diff --git a/package.json b/package.json index 40cefd52..2629c509 100644 --- a/package.json +++ b/package.json @@ -42,7 +42,8 @@ "evals:smoke": "cd evals/promptfoo && npx promptfoo@0.103.3 eval", "test:release-smoke": "npx playwright test --config tests/release-smoke/playwright.config.ts", "test:release-smoke:headed": "npx playwright test --config tests/release-smoke/playwright.config.ts --headed", - "metrics:paperclip-commits": "tsx scripts/paperclip-commit-metrics.ts" + "metrics:paperclip-commits": "tsx scripts/paperclip-commit-metrics.ts", + "perf:issue-chat-long-thread": "node scripts/measure-issue-chat-long-thread.mjs" }, "devDependencies": { "@playwright/test": "^1.58.2", diff --git a/scripts/measure-issue-chat-long-thread.mjs b/scripts/measure-issue-chat-long-thread.mjs new file mode 100644 index 00000000..5bcda55e --- /dev/null +++ b/scripts/measure-issue-chat-long-thread.mjs @@ -0,0 +1,108 @@ +#!/usr/bin/env node + +import { chromium } from "@playwright/test"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const baseUrl = (process.env.PAPERCLIP_PERF_BASE_URL || "http://localhost:3100").replace(/\/$/, ""); +const companyPrefix = process.env.PAPERCLIP_PERF_COMPANY_PREFIX; +const url = companyPrefix + ? `${baseUrl}/${companyPrefix}/tests/perf/long-thread` + : `${baseUrl}/tests/perf/long-thread`; +const origin = new URL(url).origin; + +function loadBoardToken() { + const authPath = path.resolve(os.homedir(), ".paperclip/auth.json"); + try { + const auth = JSON.parse(fs.readFileSync(authPath, "utf-8")); + const credentials = auth.credentials || {}; + const matching = Object.values(credentials).find((entry) => { + if (!entry || !entry.token || !entry.apiBase) return false; + return new URL(entry.apiBase).origin === origin; + }); + if (matching?.token) return matching.token; + const fallback = Object.values(credentials).find((entry) => entry?.token); + return fallback?.token ?? null; + } catch { + return null; + } +} + +const browser = await chromium.launch({ headless: true }); +const page = await browser.newPage({ viewport: { width: 1440, height: 1000 } }); +const boardToken = process.env.PAPERCLIP_PERF_BEARER_TOKEN || loadBoardToken(); + +if (boardToken) { + await page.route(`${origin}/**`, async (route) => { + await route.continue({ + headers: { ...route.request().headers(), Authorization: `Bearer ${boardToken}` }, + }); + }); +} + +try { + const startedAt = Date.now(); + await page.goto(url, { waitUntil: "networkidle" }); + await page.waitForSelector('[data-testid="issue-chat-long-thread-perf"]', { timeout: 30_000 }); + await page.waitForFunction(() => { + const target = Number(document.querySelector('[data-testid="perf-fixture-row-target"]')?.textContent ?? "450"); + const renderedRows = document.querySelectorAll('[data-testid="issue-chat-message-row"]').length; + const virtualizer = document.querySelector('[data-testid="issue-chat-thread-virtualizer"]'); + if (!virtualizer) return renderedRows >= target; + const virtualCount = Number(virtualizer.getAttribute("data-virtual-count") ?? "0"); + return virtualCount >= target && renderedRows > 0 && renderedRows < target; + }, null, { timeout: 60_000 }); + const rowReadyMs = Date.now() - startedAt; + + const metrics = await page.evaluate(async () => { + const text = (testId) => document.querySelector(`[data-testid="${testId}"]`)?.textContent?.trim() ?? ""; + const numericMs = (testId) => { + const value = text(testId).replace(/\s*ms$/, ""); + const parsed = Number(value); + return Number.isFinite(parsed) ? parsed : null; + }; + + const rowCount = document.querySelectorAll('[data-testid="issue-chat-message-row"]').length; + const virtualizer = document.querySelector('[data-testid="issue-chat-thread-virtualizer"]'); + const virtualCount = Number(virtualizer?.getAttribute("data-virtual-count") ?? "0"); + const assistantRowCount = document.querySelectorAll('[data-testid="issue-chat-message-row"][data-message-role="assistant"]').length; + const systemRowCount = document.querySelectorAll('[data-testid="issue-chat-message-row"][data-message-role="system"]').length; + const userRowCount = document.querySelectorAll('[data-testid="issue-chat-message-row"][data-message-role="user"]').length; + const markdownRows = Number(text("perf-fixture-markdown-rows")); + const commitCount = Number(text("perf-commit-count")); + const scrollStartY = window.scrollY; + const scrollTarget = Math.max(0, document.documentElement.scrollHeight - window.innerHeight); + const scrollStartedAt = performance.now(); + window.scrollTo({ top: scrollTarget, behavior: "instant" }); + await new Promise((resolve) => requestAnimationFrame(() => resolve())); + await new Promise((resolve) => requestAnimationFrame(() => resolve())); + const scrollResponsiveMs = performance.now() - scrollStartedAt; + + return { + url: window.location.href, + fixtureRowTarget: Number(text("perf-fixture-row-target")), + virtualized: Boolean(virtualizer), + virtualCount, + rowCount, + assistantRowCount, + userRowCount, + systemRowCount, + markdownRows, + commitCount, + mountActualDurationMs: numericMs("perf-mount-duration"), + latestActualDurationMs: numericMs("perf-latest-duration"), + maxActualDurationMs: numericMs("perf-max-duration"), + totalActualDurationMs: numericMs("perf-total-duration"), + reactProfilerAvailable: commitCount > 0, + scrollResponsiveMs: Number(scrollResponsiveMs.toFixed(1)), + scrollDeltaPx: Math.round(Math.abs(window.scrollY - scrollStartY)), + documentHeightPx: Math.round(document.documentElement.scrollHeight), + }; + }); + + const elapsedMs = Date.now() - startedAt; + console.log(JSON.stringify({ ...metrics, renderReadyMs: rowReadyMs, elapsedMs }, null, 2)); +} finally { + await browser.close(); +} diff --git a/ui/src/components/IssueChatThread.test.tsx b/ui/src/components/IssueChatThread.test.tsx index 6837fe0d..977d2bed 100644 --- a/ui/src/components/IssueChatThread.test.tsx +++ b/ui/src/components/IssueChatThread.test.tsx @@ -1,6 +1,6 @@ // @vitest-environment jsdom -import { act, createRef, forwardRef, useImperativeHandle } from "react"; +import { act, createRef, forwardRef, useImperativeHandle, useState } from "react"; import type { ReactNode } from "react"; import { createRoot } from "react-dom/client"; import { MemoryRouter } from "react-router-dom"; @@ -601,6 +601,71 @@ describe("IssueChatThread", () => { scrollHost.remove(); }); + it("cancels jump-to-latest settling when the user scrolls manually", () => { + vi.useFakeTimers(); + container.remove(); + const scrollHost = document.createElement("main"); + scrollHost.id = "main-content"; + scrollHost.style.overflowY = "auto"; + scrollHost.style.overflow = "auto"; + scrollHost.style.height = "640px"; + document.body.appendChild(scrollHost); + container = document.createElement("div"); + scrollHost.appendChild(container); + + const elementScrollToMock = vi.fn(); + scrollHost.scrollTo = elementScrollToMock as unknown as typeof scrollHost.scrollTo; + const originalScrollIntoView = Element.prototype.scrollIntoView; + const scrollIntoViewMock = vi.fn(); + Element.prototype.scrollIntoView = scrollIntoViewMock as unknown as typeof Element.prototype.scrollIntoView; + + const root = createRoot(container); + act(() => { + root.render( + + {}} + enableLiveTranscriptPolling={false} + transcriptsByRunId={issueChatLongThreadTranscriptsByRunId} + hasOutputForRun={(runId) => issueChatLongThreadTranscriptsByRunId.has(runId)} + /> + , + ); + }); + + const jump = Array.from(container.querySelectorAll("button")).find( + (button) => button.textContent === "Jump to latest", + ) as HTMLButtonElement | undefined; + expect(jump).toBeDefined(); + + act(() => { + jump?.click(); + }); + + expect(elementScrollToMock.mock.calls.some(([arg]) => hasSmoothScrollBehavior(arg))).toBe(true); + const scrollCallsAfterClick = elementScrollToMock.mock.calls.length; + + act(() => { + scrollHost.dispatchEvent(new WheelEvent("wheel", { bubbles: true })); + vi.advanceTimersByTime(500); + }); + + expect(elementScrollToMock).toHaveBeenCalledTimes(scrollCallsAfterClick); + expect(scrollIntoViewMock).not.toHaveBeenCalled(); + + Element.prototype.scrollIntoView = originalScrollIntoView; + act(() => { + root.unmount(); + }); + scrollHost.remove(); + }); + // Regression for PAP-2672: when the merged feed ends with a non-comment row // (run/timeline/embedded output) we still want Jump to latest to land on the // last comment, not whichever activity row sorts last. @@ -757,6 +822,78 @@ describe("IssueChatThread", () => { }); }); + it("uses comments rendered by onRefreshLatestComments before resolving latest", async () => { + const scrolledIds: string[] = []; + const originalScrollIntoView = Element.prototype.scrollIntoView; + Element.prototype.scrollIntoView = vi.fn(function scrollIntoView(this: Element) { + scrolledIds.push(this.id); + }) as unknown as typeof Element.prototype.scrollIntoView; + + const olderComment = { + id: "comment-before-refresh", + companyId: "company-1", + issueId: "issue-1", + authorAgentId: "agent-perf-codex", + authorUserId: null, + body: "Older loaded comment", + createdAt: new Date("2026-04-06T12:00:00.000Z"), + updatedAt: new Date("2026-04-06T12:00:00.000Z"), + }; + const latestComment = { + ...olderComment, + id: "comment-after-refresh", + body: "Latest fetched comment", + createdAt: new Date("2026-04-06T12:01:00.000Z"), + updatedAt: new Date("2026-04-06T12:01:00.000Z"), + }; + + function RefreshingThread() { + const [comments, setComments] = useState([olderComment]); + return ( + {}} + enableLiveTranscriptPolling={false} + onRefreshLatestComments={async () => { + setComments([olderComment, latestComment]); + await new Promise((resolve) => window.requestAnimationFrame(resolve)); + }} + /> + ); + } + + const root = createRoot(container); + await act(async () => { + root.render( + + + , + ); + }); + + const jump = Array.from(container.querySelectorAll("button")).find( + (button) => button.textContent === "Jump to latest", + ) as HTMLButtonElement | undefined; + expect(jump).toBeDefined(); + + await act(async () => { + jump?.click(); + await new Promise((resolve) => window.requestAnimationFrame(resolve)); + }); + + expect(scrolledIds).toContain("comment-comment-after-refresh"); + + Element.prototype.scrollIntoView = originalScrollIntoView; + act(() => { + root.unmount(); + }); + }); + it("findLatestCommentMessageIndex prefers the last comment-anchored row (PAP-2672)", () => { const messages = [ { metadata: { custom: { anchorId: "comment-a" } } }, diff --git a/ui/src/components/IssueChatThread.tsx b/ui/src/components/IssueChatThread.tsx index 80d9cfdc..ed615d56 100644 --- a/ui/src/components/IssueChatThread.tsx +++ b/ui/src/components/IssueChatThread.tsx @@ -1534,7 +1534,7 @@ function IssueChatAssistantMessage({ }} > - {isStoppingRun ? "Stopping…" : "Stop run"} + {isStoppingRun ? "Stopping..." : "Stop run"} ) : null} {runHref ? ( @@ -3102,6 +3102,7 @@ export function IssueChatThread({ const spacerBaselineAnchorRef = useRef(null); const spacerInitialReserveRef = useRef(0); const latestSettleTimeoutsRef = useRef([]); + const latestSettleCleanupRef = useRef<(() => void) | null>(null); const [bottomSpacerHeight, setBottomSpacerHeight] = useState(0); const displayLiveRuns = useMemo(() => { const deduped = new Map(); @@ -3147,6 +3148,8 @@ export function IssueChatThread({ window.clearTimeout(timeout); } latestSettleTimeoutsRef.current = []; + latestSettleCleanupRef.current?.(); + latestSettleCleanupRef.current = null; }, []); useEffect(() => clearLatestSettleTimeouts, [clearLatestSettleTimeouts]); @@ -3204,6 +3207,8 @@ export function IssueChatThread({ stableMessageCacheRef.current = stabilized.cache; return stabilized.messages; }, [rawMessages]); + const latestMessagesRef = useRef(messages); + latestMessagesRef.current = messages; const isRunning = displayLiveRuns.some((run) => run.status === "queued" || run.status === "running"); const unresolvedBlockers = useMemo( @@ -3236,9 +3241,14 @@ export function IssueChatThread({ function scrollToThreadAnchor( anchorId: string, options?: { align?: "start" | "center" | "end" | "auto"; behavior?: ScrollBehavior }, + messageSnapshot: readonly ThreadMessage[] = messages, ) { - const virtualIndex = messageAnchorIndex.get(anchorId); - if (useVirtualizedThread && virtualIndex !== undefined) { + const snapshotUsesVirtualizer = messageSnapshot.length >= VIRTUALIZED_THREAD_ROW_THRESHOLD; + const virtualIndex = + messageSnapshot === messages + ? messageAnchorIndex.get(anchorId) + : findMessageAnchorIndex(messageSnapshot, anchorId); + if (snapshotUsesVirtualizer && virtualIndex !== undefined && virtualIndex >= 0) { if (!virtualizedThreadRef.current) return false; virtualizedThreadRef.current.scrollToIndex(virtualIndex, { align: options?.align ?? "center", @@ -3366,26 +3376,35 @@ export function IssueChatThread({ bottomAnchorRef.current?.scrollIntoView({ behavior: "smooth", block: "end" }); } - // Walks the thread by anchor and lands on the latest `comment-*` row, with - // a short series of settle passes. The virtualizer estimates row sizes for - // unmeasured rows, and that estimate undershoots tall markdown comments — - // so the first scroll often lands above the actual bottom and the user - // ends up clicking Jump to latest repeatedly to converge. Re-issuing the - // scroll after measurements catch up lets one click reach the actual - // latest comment (PAP-2672 follow-up). - function scrollToLatestCommentWithSettle() { - const latestCommentIndex = findLatestCommentMessageIndex(messages); + // Lands on the latest `comment-*` row and then drives the scroll the rest + // of the way home as the virtualizer's per-row measurements arrive. + // + // The virtualizer estimates 220px for unmeasured rows. On long threads + // with tall markdown comments (PAP-2536 et al.), totalSize is hugely + // underestimated until rows render and get measured. A single scroll + // lands above the actual bottom; rendered rows then expand, the layout + // grows, and the user has to keep clicking Jump-to-latest to walk closer + // to the real bottom. The convergence loop below issues `scrollIntoView` + // on the latest comment element on every tick until the DOM bottom of + // that element is at the scroll container's bottom (or scroll position + // and content height stop changing). + function scrollToLatestCommentWithSettle(messageSnapshot: readonly ThreadMessage[] = latestMessagesRef.current) { + const latestCommentIndex = findLatestCommentMessageIndex(messageSnapshot); if (latestCommentIndex < 0) { jumpToLatestFallback(); return; } - const latestCommentAnchor = issueChatMessageAnchorId(messages[latestCommentIndex]); + const latestCommentAnchor = issueChatMessageAnchorId(messageSnapshot[latestCommentIndex]); if (!latestCommentAnchor) { jumpToLatestFallback(); return; } - const initial = scrollToThreadAnchor(latestCommentAnchor, { align: "end", behavior: "smooth" }); + const initial = scrollToThreadAnchor( + latestCommentAnchor, + { align: "end", behavior: "smooth" }, + messageSnapshot, + ); if (!initial) { jumpToLatestFallback(); return; @@ -3393,44 +3412,123 @@ export function IssueChatThread({ if (typeof window === "undefined") return; + const startedAt = (typeof performance !== "undefined" ? performance.now() : Date.now()); + const MAX_DURATION_MS = 4000; + const TICK_MS = 80; + const TOLERANCE_PX = 4; + clearLatestSettleTimeouts(); - const settleDelays = [380, 760, 1140]; - settleDelays.forEach((delay) => { + const resolveScrollContainer = (): HTMLElement | null => + (document.getElementById("main-content") as HTMLElement | null); + const cancelTarget = resolveScrollContainer() ?? window; + + let lastScrollTop = -1; + let lastScrollHeight = -1; + let stableTicks = 0; + let cancelled = false; + + const cancel = () => { + cancelled = true; + }; + + const cleanup = () => { + cancelTarget.removeEventListener("wheel", cancel); + cancelTarget.removeEventListener("touchstart", cancel); + }; + + cancelTarget.addEventListener("wheel", cancel, { once: true, passive: true }); + cancelTarget.addEventListener("touchstart", cancel, { once: true, passive: true }); + latestSettleCleanupRef.current = cleanup; + + const finish = () => { + cleanup(); + latestSettleCleanupRef.current = null; + for (const timeout of latestSettleTimeoutsRef.current) { + window.clearTimeout(timeout); + } + latestSettleTimeoutsRef.current = []; + }; + + const scheduleTick = (delay: number) => { const timeout = window.setTimeout(() => { - if (typeof document === "undefined") return; - const el = document.getElementById(latestCommentAnchor); - if (el) { - el.scrollIntoView({ behavior: "smooth", block: "end" }); - return; - } - // The row may still be outside the virtualizer's render buffer; nudge - // the offset so it gets mounted, then the next pass can align with - // real DOM measurements. + latestSettleTimeoutsRef.current = latestSettleTimeoutsRef.current.filter((entry) => entry !== timeout); + tick(); + }, delay); + latestSettleTimeoutsRef.current.push(timeout); + }; + + const tick = () => { + const now = (typeof performance !== "undefined" ? performance.now() : Date.now()); + if (cancelled || now - startedAt > MAX_DURATION_MS) { + finish(); + return; + } + + const el = document.getElementById(latestCommentAnchor); + if (!el) { + // Row hasn't been rendered into the virtualizer's buffer yet — nudge + // the offset (instant) so it gets mounted, then keep settling. virtualizedThreadRef.current?.scrollToIndex(latestCommentIndex, { align: "end", behavior: "auto", }); - }, delay); - latestSettleTimeoutsRef.current.push(timeout); - }); + scheduleTick(TICK_MS); + return; + } + + const container = resolveScrollContainer(); + const containerBottom = container + ? container.getBoundingClientRect().bottom + : window.innerHeight; + const elBottom = el.getBoundingClientRect().bottom; + const offBottom = elBottom - containerBottom; + + if (Math.abs(offBottom) > TOLERANCE_PX) { + el.scrollIntoView({ behavior: "smooth", block: "end" }); + } + + const currentScrollTop = container?.scrollTop ?? window.scrollY; + const currentScrollHeight = container?.scrollHeight ?? document.documentElement.scrollHeight; + const scrollStable = Math.abs(currentScrollTop - lastScrollTop) < 1; + const heightStable = currentScrollHeight === lastScrollHeight; + const atBottom = Math.abs(offBottom) <= TOLERANCE_PX; + if (scrollStable && heightStable && atBottom) { + stableTicks += 1; + if (stableTicks >= 3) { + finish(); + return; + } + } else { + stableTicks = 0; + } + lastScrollTop = currentScrollTop; + lastScrollHeight = currentScrollHeight; + scheduleTick(TICK_MS); + }; + + // Hold the first iteration off for one frame so the initial smooth + // scroll has begun (and the virtualizer has rendered the buffer around + // the target) before we start settling. + scheduleTick(120); } function handleJumpToLatest() { if (onRefreshLatestComments) { - // Refetching from page 0 (newest first) brings any comments that - // arrived after the initial load into the cache before we scroll — - // otherwise we'd land on the latest *loaded* row rather than the - // absolute newest, which is what PAP-2672 reopened on. + // Refetching the comments query (page 0 first) brings any comment that + // arrived after the initial load — including ones live updates may + // have missed during reconnects — into the loaded set before we + // resolve the latest target. Otherwise we'd land on the latest + // *loaded* comment but not the absolute newest. (PAP-2672 follow-up.) const refreshed = onRefreshLatestComments(); if (refreshed && typeof (refreshed as Promise).then === "function") { (refreshed as Promise).then( - () => scrollToLatestCommentWithSettle(), - () => scrollToLatestCommentWithSettle(), + () => scrollToLatestCommentWithSettle(latestMessagesRef.current), + () => scrollToLatestCommentWithSettle(latestMessagesRef.current), ); return; } } - scrollToLatestCommentWithSettle(); + scrollToLatestCommentWithSettle(latestMessagesRef.current); } const stableOnVote = useStableEvent(onVote); diff --git a/ui/src/components/IssuesList.test.tsx b/ui/src/components/IssuesList.test.tsx index 9ca81665..aa13ac30 100644 --- a/ui/src/components/IssuesList.test.tsx +++ b/ui/src/components/IssuesList.test.tsx @@ -523,6 +523,153 @@ describe("IssuesList", () => { }); }); + it("hides the workflow blocker chip when a sub-issue is blocked only by its previous sibling", async () => { + const firstChild = createIssue({ + id: "issue-first-child", + identifier: "PAP-1", + parentId: "issue-parent", + title: "First child", + status: "todo", + createdAt: new Date("2026-04-01T00:00:00.000Z"), + }); + const secondChild = createIssue({ + id: "issue-second-child", + identifier: "PAP-2", + parentId: "issue-parent", + title: "Second child", + status: "blocked", + blockedBy: [ + { + id: "issue-first-child", + identifier: "PAP-1", + title: "First child", + status: "todo", + priority: "medium", + assigneeAgentId: null, + assigneeUserId: null, + }, + ], + createdAt: new Date("2026-04-02T00:00:00.000Z"), + }); + + const { root } = renderWithQueryClient( + undefined} + />, + container, + ); + + await waitForAssertion(() => { + const rows = Array.from(container.querySelectorAll('[data-testid="issue-row"]')); + expect(rows).toHaveLength(2); + expect(rows.map((row) => row.getAttribute("data-step"))).toEqual(["1", "2"]); + expect(container.textContent).not.toContain("blocked by PAP-1"); + }); + + act(() => { + root.unmount(); + }); + }); + + it("collapses multiple workflow blocker chips to the first blocker and a count", async () => { + const issueDone = createIssue({ + id: "issue-done", + identifier: "PAP-1", + title: "Done first", + status: "done", + createdAt: new Date("2026-04-01T00:00:00.000Z"), + }); + const firstBlocker = createIssue({ + id: "issue-first-blocker", + identifier: "PAP-2", + title: "First blocker", + status: "todo", + createdAt: new Date("2026-04-02T00:00:00.000Z"), + }); + const secondBlocker = createIssue({ + id: "issue-second-blocker", + identifier: "PAP-3", + title: "Second blocker", + status: "todo", + createdAt: new Date("2026-04-03T00:00:00.000Z"), + }); + const thirdBlocker = createIssue({ + id: "issue-third-blocker", + identifier: "PAP-4", + title: "Third blocker", + status: "todo", + createdAt: new Date("2026-04-04T00:00:00.000Z"), + }); + const issueBlocked = createIssue({ + id: "issue-blocked", + identifier: "PAP-5", + title: "Blocked issue", + status: "blocked", + blockedBy: [ + { + id: "issue-first-blocker", + identifier: "PAP-2", + title: "First blocker", + status: "todo", + priority: "medium", + assigneeAgentId: null, + assigneeUserId: null, + }, + { + id: "issue-second-blocker", + identifier: "PAP-3", + title: "Second blocker", + status: "todo", + priority: "medium", + assigneeAgentId: null, + assigneeUserId: null, + }, + { + id: "issue-third-blocker", + identifier: "PAP-4", + title: "Third blocker", + status: "todo", + priority: "medium", + assigneeAgentId: null, + assigneeUserId: null, + }, + ], + createdAt: new Date("2026-04-05T00:00:00.000Z"), + }); + + const { root } = renderWithQueryClient( + undefined} + />, + container, + ); + + await waitForAssertion(() => { + expect(container.textContent).toContain("blocked by PAP-2"); + expect(container.textContent).toContain("... and 2 more"); + expect(container.textContent).not.toContain("blocked by PAP-3"); + expect(container.textContent).not.toContain("blocked by PAP-4"); + const blockerButtons = Array.from(container.querySelectorAll("button")) + .filter((button) => button.textContent?.includes("blocked by")); + expect(blockerButtons).toHaveLength(1); + expect(blockerButtons[0]?.textContent).toBe("blocked by PAP-2 · step 2 ... and 2 more"); + }); + + act(() => { + root.unmount(); + }); + }); + it("uses hierarchical checklist step numbers when nested rows render inline", async () => { const firstRoot = createIssue({ id: "issue-first-root", @@ -909,7 +1056,7 @@ describe("IssuesList", () => { }); it("waits for the desktop main scroll container before rendering more local rows", async () => { - const manyIssues = Array.from({ length: 420 }, (_, index) => + const manyIssues = Array.from({ length: 120 }, (_, index) => createIssue({ id: `issue-${index + 1}`, identifier: `PAP-${index + 1}`, diff --git a/ui/src/components/IssuesList.tsx b/ui/src/components/IssuesList.tsx index 9cf1a48d..3fa43313 100644 --- a/ui/src/components/IssuesList.tsx +++ b/ui/src/components/IssuesList.tsx @@ -282,6 +282,51 @@ function buildChecklistStepNumberMap(issues: Issue[], nestingEnabled: boolean): return stepNumberByIssueId; } +function buildPreviousSiblingIssueIdMap(issues: Issue[], nestingEnabled: boolean): Map { + const previousSiblingByIssueId = new Map(); + + if (!nestingEnabled) { + const previousByParentId = new Map(); + for (const issue of issues) { + if (!issue.parentId) continue; + const previousSibling = previousByParentId.get(issue.parentId); + if (previousSibling) { + previousSiblingByIssueId.set(issue.id, previousSibling.id); + } + previousByParentId.set(issue.parentId, issue); + } + return previousSiblingByIssueId; + } + + const { roots, childMap } = buildIssueTree(issues); + const visit = (siblings: Issue[]) => { + siblings.forEach((issue, index) => { + const previousSibling = index > 0 ? siblings[index - 1] : null; + if (issue.parentId && previousSibling?.parentId === issue.parentId) { + previousSiblingByIssueId.set(issue.id, previousSibling.id); + } + visit(childMap.get(issue.id) ?? []); + }); + }; + visit(roots); + + return previousSiblingByIssueId; +} + +function shouldSuppressSinglePreviousSiblingBlockerChip( + issue: Issue, + unresolvedVisibleBlockerIds: string[], + previousSiblingIssueId: string | undefined, +): boolean { + return Boolean( + issue.parentId + && previousSiblingIssueId + && (issue.blockedBy ?? []).length === 1 + && unresolvedVisibleBlockerIds.length === 1 + && unresolvedVisibleBlockerIds[0] === previousSiblingIssueId, + ); +} + /* ── Component ── */ interface Agent { @@ -878,6 +923,7 @@ export function IssuesList({ const visibleIssueIds = new Set(filtered.map((issue) => issue.id)); const stepNumberByIssueId = buildChecklistStepNumberMap(filtered, viewState.nestingEnabled); + const previousSiblingIssueIdByIssueId = buildPreviousSiblingIssueIdMap(filtered, viewState.nestingEnabled); const unresolvedVisibleBlockersByIssueId = new Map(); filtered.forEach((issue) => { @@ -889,7 +935,12 @@ export function IssuesList({ if (!blockerIssue) return false; return blockerIssue.status !== "done" && blockerIssue.status !== "cancelled"; }); - unresolvedVisibleBlockersByIssueId.set(issue.id, unresolvedVisible); + const shouldSuppressChip = shouldSuppressSinglePreviousSiblingBlockerChip( + issue, + unresolvedVisible, + previousSiblingIssueIdByIssueId.get(issue.id), + ); + unresolvedVisibleBlockersByIssueId.set(issue.id, shouldSuppressChip ? [] : unresolvedVisible); }); const firstActionable = filtered.find((issue) => isActionableWorkflowStatus(issue.status)) ?? null; @@ -1388,36 +1439,49 @@ export function IssuesList({ const doneRowTitleClass = checklistMeta && issue.status === "done" ? "text-muted-foreground" : undefined; - const checklistDependencyChips = checklistMeta && unresolvedVisibleBlockers.length > 0 ? ( - <> - {unresolvedVisibleBlockers.map((blockerId) => { - const blockerIssue = issueById.get(blockerId); - if (!blockerIssue) return null; - const label = blockerIssue.identifier ?? blockerIssue.id.slice(0, 8); - const blockerStep = checklistMeta.stepNumberByIssueId.get(blockerId); - const blockerStepSuffix = blockerStep ? ` \u00b7 step ${blockerStep}` : ""; - const chipLabel = `blocked by ${label}${blockerStepSuffix}`; - return ( - - ); - })} - + const visibleBlockerChips = unresolvedVisibleBlockers + .map((blockerId) => { + const blockerIssue = issueById.get(blockerId); + if (!blockerIssue) return null; + const label = blockerIssue.identifier ?? blockerIssue.id.slice(0, 8); + const blockerStep = checklistMeta?.stepNumberByIssueId.get(blockerId); + const blockerStepSuffix = blockerStep ? ` \u00b7 step ${blockerStep}` : ""; + return { blockerId, chipLabel: `blocked by ${label}${blockerStepSuffix}` }; + }) + .filter((chip): chip is { blockerId: string; chipLabel: string } => chip !== null); + const firstVisibleBlockerChip = visibleBlockerChips[0] ?? null; + const additionalVisibleBlockerCount = Math.max(visibleBlockerChips.length - 1, 0); + const additionalVisibleBlockerLabel = additionalVisibleBlockerCount > 0 + ? ` ... and ${additionalVisibleBlockerCount} more` + : ""; + const firstVisibleBlockerDisplayLabel = firstVisibleBlockerChip + ? `${firstVisibleBlockerChip.chipLabel}${additionalVisibleBlockerLabel}` + : ""; + const hiddenVisibleBlockerLabels = visibleBlockerChips + .slice(1) + .map((chip) => chip.chipLabel) + .join(", "); + const firstVisibleBlockerTitle = additionalVisibleBlockerCount > 0 + ? `${firstVisibleBlockerDisplayLabel}: ${hiddenVisibleBlockerLabels}` + : firstVisibleBlockerDisplayLabel; + const checklistDependencyChips = checklistMeta && firstVisibleBlockerChip ? ( + ) : null; return ( diff --git a/ui/src/components/MarkdownBody.test.tsx b/ui/src/components/MarkdownBody.test.tsx index 0cae828d..66811fe9 100644 --- a/ui/src/components/MarkdownBody.test.tsx +++ b/ui/src/components/MarkdownBody.test.tsx @@ -366,6 +366,21 @@ describe("MarkdownBody", () => { expect(html).toContain('style="max-width:100%;overflow-x:auto"'); }); + it("renders a copy button alongside fenced code blocks", () => { + const html = renderMarkdown("```ts\nconst a = 1;\n```"); + + expect(html).toContain("paperclip-markdown-codeblock"); + expect(html).toContain("paperclip-markdown-codeblock-copy"); + expect(html).toContain('aria-label="Copy code"'); + expect(html).toContain("lucide-copy"); + }); + + it("does not render a copy button on inline code", () => { + const html = renderMarkdown("Reference `inline-code` here."); + + expect(html).not.toContain("paperclip-markdown-codeblock-copy"); + }); + it("renders internal issue links and bare identifiers as inline issue refs", () => { const html = renderMarkdown(`See PAP-42 and [linked task](${buildIssueReferenceHref("PAP-77")}) for follow-up.`, [ { identifier: "PAP-42", status: "done" }, diff --git a/ui/src/components/MarkdownBody.tsx b/ui/src/components/MarkdownBody.tsx index b155032f..f741cf5b 100644 --- a/ui/src/components/MarkdownBody.tsx +++ b/ui/src/components/MarkdownBody.tsx @@ -1,6 +1,6 @@ -import { isValidElement, useEffect, useId, useState, type ReactNode } from "react"; +import { isValidElement, useCallback, useEffect, useId, useRef, useState, type ReactNode } from "react"; import { useQuery } from "@tanstack/react-query"; -import { ExternalLink, Github } from "lucide-react"; +import { Check, Copy, ExternalLink, Github } from "lucide-react"; import Markdown, { defaultUrlTransform, type Components, type Options } from "react-markdown"; import remarkGfm from "remark-gfm"; import { cn } from "../lib/utils"; @@ -183,6 +183,83 @@ function renderLinkBody( ); } +function CodeBlock({ + children, + preProps, +}: { + children: ReactNode; + preProps: React.HTMLAttributes; +}) { + const [copied, setCopied] = useState(false); + const [failed, setFailed] = useState(false); + const preRef = useRef(null); + const timerRef = useRef>(undefined); + + useEffect(() => () => clearTimeout(timerRef.current), []); + + const handleCopy = useCallback(async () => { + const text = preRef.current?.innerText ?? flattenText(children); + try { + if (navigator.clipboard && window.isSecureContext) { + await navigator.clipboard.writeText(text); + } else { + const textarea = document.createElement("textarea"); + textarea.value = text; + textarea.style.position = "fixed"; + textarea.style.left = "-9999px"; + document.body.appendChild(textarea); + try { + textarea.select(); + const success = document.execCommand("copy"); + if (!success) throw new Error("execCommand copy failed"); + } finally { + document.body.removeChild(textarea); + } + } + setFailed(false); + setCopied(true); + } catch { + setFailed(true); + setCopied(true); + } + clearTimeout(timerRef.current); + timerRef.current = setTimeout(() => { + setCopied(false); + setFailed(false); + }, 1500); + }, [children]); + + const label = failed ? "Copy failed" : copied ? "Copied!" : "Copy"; + + return ( +
+ 
+        {children}
+
+ +
+ ); +} + function MermaidDiagramBlock({ source, darkMode }: { source: string; darkMode: boolean }) { const renderId = useId().replace(/[^a-zA-Z0-9_-]/g, ""); const [svg, setSvg] = useState(null); @@ -286,7 +363,7 @@ export function MarkdownBody({ if (mermaidSource) { return ; } - return 
{preChildren}
; + return {preChildren}; }, code: ({ node: _node, style: codeStyle, children: codeChildren, ...codeProps }) => ( diff --git a/ui/src/components/MarkdownEditor.test.tsx b/ui/src/components/MarkdownEditor.test.tsx index e41b1bab..bbbf0bc4 100644 --- a/ui/src/components/MarkdownEditor.test.tsx +++ b/ui/src/components/MarkdownEditor.test.tsx @@ -478,22 +478,34 @@ describe("MarkdownEditor", () => { }); }); - it("anchors the mention menu inside the visual viewport when mobile offsets are present", () => { + it("places the menu top on the caret line and offsets the left a space-width past the caret", () => { expect( computeMentionMenuPosition( - { viewportTop: 180, viewportLeft: 120 }, + { viewportTop: 100, viewportBottom: 118, viewportLeft: 240 }, + { offsetLeft: 0, offsetTop: 0, width: 800, height: 600 }, + ), + ).toEqual({ + top: 100, + left: 250, + }); + }); + + it("applies visual viewport offsets when present", () => { + expect( + computeMentionMenuPosition( + { viewportTop: 20, viewportBottom: 38, viewportLeft: 120 }, { offsetLeft: 24, offsetTop: 320, width: 320, height: 260 }, ), ).toEqual({ - top: 372, - left: 144, + top: 340, + left: 154, }); }); it("clamps the mention menu back into view near the viewport edges", () => { expect( computeMentionMenuPosition( - { viewportTop: 260, viewportLeft: 240 }, + { viewportTop: 260, viewportBottom: 278, viewportLeft: 240 }, { offsetLeft: 0, offsetTop: 0, width: 280, height: 220 }, ), ).toEqual({ @@ -502,16 +514,28 @@ describe("MarkdownEditor", () => { }); }); + it("flips the menu above the caret line when it would overflow below", () => { + expect( + computeMentionMenuPosition( + { viewportTop: 560, viewportBottom: 580, viewportLeft: 200 }, + { offsetLeft: 0, offsetTop: 0, width: 800, height: 600 }, + ), + ).toEqual({ + top: 372, + left: 210, + }); + }); + it("keeps a short mention menu on the same line when it fits below the caret", () => { expect( computeMentionMenuPosition( - { viewportTop: 160, viewportLeft: 120 }, + { viewportTop: 160, viewportBottom: 178, viewportLeft: 120 }, { offsetLeft: 0, offsetTop: 0, width: 320, height: 220 }, { width: 188, height: 42 }, ), ).toEqual({ - top: 164, - left: 120, + top: 160, + left: 130, }); }); @@ -619,8 +643,20 @@ describe("MarkdownEditor", () => { editable.remove(); }); - it("accepts mention selection from touchstart taps", async () => { - const handleChange = vi.fn(); + function createTouchEvent( + type: "touchstart" | "touchmove" | "touchend", + touches: Array<{ clientX: number; clientY: number }>, + ) { + const event = new Event(type, { bubbles: true, cancelable: true }); + const list = touches as unknown as TouchList; + Object.defineProperty(event, "touches", { value: type === "touchend" ? [] : list }); + Object.defineProperty(event, "changedTouches", { value: list }); + return event; + } + + async function openMentionMenuFor( + handleChange: ReturnType, + ): Promise<{ option: HTMLButtonElement; root: ReturnType }> { const root = createRoot(container); await act(async () => { @@ -645,7 +681,6 @@ describe("MarkdownEditor", () => { const editable = container.querySelector('[contenteditable="true"]'); expect(editable).not.toBeNull(); - const textNode = editable?.firstChild; expect(textNode?.nodeType).toBe(Node.TEXT_NODE); @@ -659,15 +694,24 @@ describe("MarkdownEditor", () => { act(() => { document.dispatchEvent(new Event("selectionchange")); }); - await flush(); const option = Array.from(document.body.querySelectorAll('button[type="button"]')) - .find((node) => node.textContent?.includes("Paperclip App")); + .find((node) => node.textContent?.includes("Paperclip App")) as HTMLButtonElement | undefined; expect(option).toBeTruthy(); + return { option: option!, root }; + } + + it("accepts mention selection from a touch tap", async () => { + const handleChange = vi.fn(); + const { option, root } = await openMentionMenuFor(handleChange); + const point = { clientX: 100, clientY: 50 }; act(() => { - option?.dispatchEvent(new Event("touchstart", { bubbles: true, cancelable: true })); + option.dispatchEvent(createTouchEvent("touchstart", [point])); + }); + act(() => { + option.dispatchEvent(createTouchEvent("touchend", [point])); }); expect(handleChange).toHaveBeenCalledWith( @@ -678,4 +722,44 @@ describe("MarkdownEditor", () => { root.unmount(); }); }); + + it("does not preventDefault on touchstart so the mention menu can scroll on mobile", async () => { + const handleChange = vi.fn(); + const { option, root } = await openMentionMenuFor(handleChange); + + const touchstart = createTouchEvent("touchstart", [{ clientX: 100, clientY: 50 }]); + act(() => { + option.dispatchEvent(touchstart); + }); + + expect(touchstart.defaultPrevented).toBe(false); + expect(handleChange).not.toHaveBeenCalled(); + + await act(async () => { + root.unmount(); + }); + }); + + it("does not select when the touch moves like a scroll", async () => { + const handleChange = vi.fn(); + const { option, root } = await openMentionMenuFor(handleChange); + const start = { clientX: 100, clientY: 50 }; + const moved = { clientX: 100, clientY: 90 }; + + act(() => { + option.dispatchEvent(createTouchEvent("touchstart", [start])); + }); + act(() => { + option.dispatchEvent(createTouchEvent("touchmove", [moved])); + }); + act(() => { + option.dispatchEvent(createTouchEvent("touchend", [moved])); + }); + + expect(handleChange).not.toHaveBeenCalled(); + + await act(async () => { + root.unmount(); + }); + }); }); diff --git a/ui/src/components/MarkdownEditor.tsx b/ui/src/components/MarkdownEditor.tsx index 0ca8e0b6..78fb82df 100644 --- a/ui/src/components/MarkdownEditor.tsx +++ b/ui/src/components/MarkdownEditor.tsx @@ -174,8 +174,14 @@ interface MentionState { query: string; top: number; left: number; - /** Viewport-relative coords for portal positioning */ + /** + * Caret-aligned viewport coords for portal positioning. `viewportTop` / + * `viewportBottom` describe the active text line, and `viewportLeft` is the + * caret X (right edge of the last typed character) so the menu can sit on + * the same line, just to the right of the cursor. + */ viewportTop: number; + viewportBottom: number; viewportLeft: number; textNode: Text; atPos: number; @@ -201,6 +207,8 @@ const MENTION_MENU_HEIGHT = 208; const MENTION_MENU_PADDING = 8; const MENTION_MENU_ROW_HEIGHT = 34; const MENTION_MENU_CHROME_HEIGHT = 8; +/** Roughly one space-width of breathing room between the caret and the menu. */ +const MENTION_MENU_CARET_GAP = 10; const CODE_BLOCK_LANGUAGES: Record = { txt: "Text", @@ -263,6 +271,36 @@ export function findMentionMatch( }; } +interface CaretRect { + top: number; + bottom: number; + /** Caret X — the right edge of the last typed character (or left edge of the next). */ + x: number; +} + +function measureCaretRect(textNode: Text, offset: number, atPos: number): CaretRect { + const length = textNode.textContent?.length ?? 0; + const rectFromRange = (start: number, end: number, side: "right" | "left"): CaretRect | null => { + if (start < 0 || end > length || end <= start) return null; + const range = document.createRange(); + range.setStart(textNode, start); + range.setEnd(textNode, end); + const rect = range.getBoundingClientRect(); + if (rect.width === 0 && rect.height === 0) return null; + return { top: rect.top, bottom: rect.bottom, x: side === "right" ? rect.right : rect.left }; + }; + + // Prefer the character immediately before the caret — its right edge IS the caret X + // and its top/bottom describe the active line. Falls back to the char after the caret + // and finally the @ marker if nothing else gives us a valid rect. + return ( + rectFromRange(Math.max(0, offset - 1), offset, "right") + ?? rectFromRange(offset, Math.min(length, offset + 1), "left") + ?? rectFromRange(atPos, atPos + 1, "right") + ?? { top: 0, bottom: 0, x: 0 } + ); +} + function detectMention(container: HTMLElement): MentionState | null { const sel = window.getSelection(); if (!sel || sel.rangeCount === 0 || !sel.isCollapsed) return null; @@ -277,21 +315,20 @@ function detectMention(container: HTMLElement): MentionState | null { const match = findMentionMatch(text, offset); if (!match) return null; - // Get position relative to container - const tempRange = document.createRange(); - tempRange.setStart(textNode, match.atPos); - tempRange.setEnd(textNode, match.atPos + 1); - const rect = tempRange.getBoundingClientRect(); + // Anchor the menu to the live caret so it tracks each typed character instead of + // staying glued to the @ marker. + const caret = measureCaretRect(textNode as Text, offset, match.atPos); const containerRect = container.getBoundingClientRect(); return { trigger: match.trigger, marker: match.marker, query: match.query, - top: rect.bottom - containerRect.top, - left: rect.left - containerRect.left, - viewportTop: rect.bottom, - viewportLeft: rect.left, + top: caret.top - containerRect.top, + left: caret.x - containerRect.left, + viewportTop: caret.top, + viewportBottom: caret.bottom, + viewportLeft: caret.x, textNode: textNode as Text, atPos: match.atPos, endPos: match.endPos, @@ -318,7 +355,7 @@ function getMentionMenuViewport(): MentionMenuViewport { } export function computeMentionMenuPosition( - anchor: Pick, + anchor: Pick, viewport: MentionMenuViewport, menuSize: MentionMenuSize = { width: MENTION_MENU_WIDTH, height: MENTION_MENU_HEIGHT }, ) { @@ -327,10 +364,23 @@ export function computeMentionMenuPosition( const minTop = viewport.offsetTop + MENTION_MENU_PADDING; const maxTop = viewport.offsetTop + viewport.height - menuSize.height; - return { - top: Math.max(minTop, Math.min(viewport.offsetTop + anchor.viewportTop + 4, maxTop)), - left: Math.max(minLeft, Math.min(viewport.offsetLeft + anchor.viewportLeft, maxLeft)), - }; + // Place the menu's top edge on the current line so it sits next to the caret. + // If it would overflow below, flip above so the menu's bottom hugs the line. + const desiredTop = viewport.offsetTop + anchor.viewportTop; + let top: number; + if (desiredTop > maxTop) { + const flipped = viewport.offsetTop + anchor.viewportBottom - menuSize.height; + top = Math.max(minTop, Math.min(flipped, maxTop)); + } else { + top = Math.max(minTop, desiredTop); + } + + // Place the menu's left edge a small gap to the right of the caret X so + // there's roughly a space-width of breathing room between cursor and menu. + const desiredLeft = viewport.offsetLeft + anchor.viewportLeft + MENTION_MENU_CARET_GAP; + const left = Math.max(minLeft, Math.min(desiredLeft, maxLeft)); + + return { top, left }; } function getMentionMenuSize(optionCount: number): MentionMenuSize { @@ -903,6 +953,44 @@ export const MarkdownEditor = forwardRef } }, [selectMention]); + // Touch handling for the mention menu. We deliberately do NOT preventDefault + // on touchstart so the browser can still scroll the menu vertically; instead + // we record the start point and only treat the gesture as a selection if the + // finger lifted with negligible movement (i.e., a tap, not a scroll). + const touchStartPointRef = useRef<{ x: number; y: number } | null>(null); + const TOUCH_TAP_THRESHOLD_PX = 8; + + const handleAutocompleteTouchStart = useCallback((event: ReactTouchEvent) => { + const touch = event.touches[0]; + if (!touch) return; + touchStartPointRef.current = { x: touch.clientX, y: touch.clientY }; + }, []); + + const handleAutocompleteTouchMove = useCallback((event: ReactTouchEvent) => { + const start = touchStartPointRef.current; + if (!start) return; + const touch = event.touches[0]; + if (!touch) return; + if (Math.hypot(touch.clientX - start.x, touch.clientY - start.y) > TOUCH_TAP_THRESHOLD_PX) { + touchStartPointRef.current = null; + } + }, []); + + const handleAutocompleteTouchEnd = useCallback(( + event: ReactTouchEvent, + option: AutocompleteOption, + ) => { + const start = touchStartPointRef.current; + touchStartPointRef.current = null; + if (!start) return; + const touch = event.changedTouches[0]; + if (!touch) return; + if (Math.hypot(touch.clientX - start.x, touch.clientY - start.y) > TOUCH_TAP_THRESHOLD_PX) { + return; + } + handleAutocompletePress(event, option); + }, [handleAutocompletePress]); + function hasFilePayload(evt: DragEvent) { return Array.from(evt.dataTransfer?.types ?? []).includes("Files"); } @@ -1131,26 +1219,36 @@ export const MarkdownEditor = forwardRef /> {/* Mention dropdown — rendered via portal so it isn't clipped by overflow containers */} - {mentionActive && filteredMentions.length > 0 && + {mentionActive && filteredMentions.length > 0 && mentionMenuPosition && createPortal(
{filteredMentions.map((option, i) => (