diff --git a/tests/perf/issue-detail/README.md b/tests/perf/issue-detail/README.md index a5bfb872c7..6e29177b36 100644 --- a/tests/perf/issue-detail/README.md +++ b/tests/perf/issue-detail/README.md @@ -25,3 +25,47 @@ PAPERCLIP_ISSUE_PERF_RUNS=7 PAPERCLIP_ISSUE_PERF_PORT=3210 pnpm exec playwright ``` Outputs include `baseline.md`, `baseline.json`, and a Chrome trace for the first run of each scenario/profile. Open `*.trace.json` in Chrome DevTools Performance to inspect the `issue-detail:*` user-timing marks. + +## Task layout stability + +`layout-stability.spec.ts` exercises the real task route, including Inbox +navigation, with 160 Markdown comments and a resolved interaction. It delays +initial comments, tests failures and retry, tracks a reading anchor across +media growth, composer resizing, properties toggles, older-page loading and +Back navigation, and covers mobile reduced motion and explicit comment links. +The eight scenarios also exercise same-task hash navigation without remounting +on desktop and mobile, and stalled native/log reads that expose Retry after +the 15-second request deadline. +Videos and per-frame `layout.json` measurements are written under each test's +output directory. The two-pixel assertion measures a logical row's viewport +offset; total scroll offset legitimately changes when history is prepended. + +To run against an existing **disposable local test-drive instance**: + +```sh +PAPERCLIP_ISSUE_PERF_BASE_URL=http://127.0.0.1:3102 \ + pnpm exec playwright test --config tests/perf/issue-detail/playwright.config.ts layout-stability.spec.ts +``` + +The suite creates its own company and fixtures. The URL override accepts only +loopback origins and rejects remote hosts. Use a disposable local instance, +not a shared or production instance. Without the override, the harness starts its own +isolated instance as before. Live provider walkthroughs additionally require a +configured native Paperclip runner and Codex authentication; deterministic +browser fixtures do not substitute for watching an actual provider run. + +For the live walkthrough, first assign a disposable task to a native Paperclip +runner configured with the Codex provider. Its scratch project should contain +a small `sum.mjs` fixture. The script sends a paced job, scrolls up, disconnects +and reconnects, steers a follow-up, and records through completion: + +```sh +PAPERCLIP_LAYOUT_LIVE_URL=http://127.0.0.1:3102/LAY/issues/LAY-8 \ + node tests/perf/issue-detail/live-feed.walkthrough.mjs +``` + +Inspect `test-results/task-layout/live-acceptance/` for the video, screenshots, +per-frame positions, layout shifts, browser timing counters, and reading-anchor +measurement. The live script rejects non-local URLs and uses real provider +capacity. Review the video as well as the assertions: layout-shift scores alone +do not capture programmatic scrolling or replacement flashes. diff --git a/tests/perf/issue-detail/layout-stability.spec.ts b/tests/perf/issue-detail/layout-stability.spec.ts new file mode 100644 index 0000000000..5c06655b53 --- /dev/null +++ b/tests/perf/issue-detail/layout-stability.spec.ts @@ -0,0 +1,240 @@ +import fs from "node:fs/promises"; +import { expect, test, type Page } from "@playwright/test"; + +test.setTimeout(90_000); + +test.use({ video: "on", trace: "retain-on-failure", viewport: { width: 1440, height: 900 } }); + +let prefix: string; +let issue: { id: string; identifier: string }; +let other: { id: string; identifier: string }; +let oldestCommentId: string; + +test.beforeAll(async ({ request }) => { + const company = await (await request.post("/api/companies", { data: { name: `Layout acceptance ${Date.now()}` } })).json(); + prefix = company.issuePrefix; + const createIssue = async (title: string) => { + const response = await request.post(`/api/companies/${company.id}/issues`, { + data: { title, status: "backlog", description: "A rich task with a long conversation.\n\n".repeat(15) }, + }); + expect(response.ok(), await response.text()).toBeTruthy(); + return response.json(); + }; + issue = await createIssue("Stable conversation acceptance"); + other = await createIssue("Rapid navigation destination"); + for (let i = 0; i < 160; i++) { + const response = await request.post(`/api/issues/${issue.id}/comments`, { + data: { body: `## Historical message ${i}\n\n${"Long Markdown with **emphasis**, detail, and enough text to wrap across several lines. ".repeat(8)}\n\n\`\`\`js\nconsole.log(${i});\n\`\`\`\n\n| State | Result |\n|---|---|\n| Complete | Stable |${i === 159 ? `\n\n[Read the oldest message](#comment-${oldestCommentId})` : ""}` }, + }); + expect(response.ok()).toBeTruthy(); + if (i === 0) oldestCommentId = (await response.json()).id; + } + const interaction = await (await request.post(`/api/issues/${issue.id}/interactions`, { + data: { kind: "request_confirmation", title: "Geometry review", payload: { version: 1, prompt: "Inspect the conversation." }, continuationPolicy: "none" }, + })).json(); + await request.post(`/api/issues/${issue.id}/interactions/${interaction.id}/accept`, { data: {} }); +}); + +async function ready(page: Page) { + await expect(page.getByTestId("task-chat-thread").locator('[aria-busy="false"]')).toBeVisible({ timeout: 90_000 }); +} + +async function installMeasurements(page: Page) { + await page.addInitScript(() => { + const evidence = { frames: [] as unknown[], shifts: [] as unknown[] }; + Object.assign(window, { taskLayoutEvidence: evidence }); + new PerformanceObserver((list) => { + evidence.shifts.push(...list.getEntries().map((e) => e.toJSON())); + }).observe({ type: "layout-shift", buffered: true }); + const sample = () => { + const scroller = document.querySelector('[data-testid="task-chat-scroller"]'); + const busy = document.querySelector('[data-testid="task-chat-thread"] [aria-busy]'); + if (scroller) { + const viewport = scroller.getBoundingClientRect(); + const row = [...scroller.querySelectorAll("[data-thread-anchor]")].find((el) => el.getBoundingClientRect().bottom > viewport.top); + evidence.frames.push({ time: performance.now(), busy: busy?.getAttribute("aria-busy"), top: scroller.scrollTop, width: viewport.width, fonts: document.fonts.status, rows: scroller.querySelectorAll("[data-thread-anchor]").length, height: scroller.scrollHeight, anchor: row?.dataset.threadAnchor, offset: row ? row.getBoundingClientRect().top - viewport.top : null }); + } + requestAnimationFrame(sample); + }; + requestAnimationFrame(sample); + }); +} + +test.afterEach(async ({ page }, info) => { + const evidence = await page.evaluate(() => (window as unknown as { taskLayoutEvidence?: unknown }).taskLayoutEvidence).catch(() => null); + if (evidence) { + const file = info.outputPath("layout.json"); + await fs.writeFile(file, JSON.stringify(evidence)); + await info.attach("layout-frames-and-shifts", { path: file, contentType: "application/json" }); + } +}); + +test("Inbox click reveals once after reordered initial responses; refresh retains rows", async ({ page }) => { + await installMeasurements(page); + await page.goto(`/${prefix}/inbox/all`); + const link = page.getByRole("link", { name: new RegExp(`Open ${issue.identifier}:`) }); + await expect(link).toBeVisible({ timeout: 90_000 }); + let release!: () => void; + const held = new Promise((resolve) => { release = resolve; }); + await page.route("**/api/issues/*/comments?*", async (route) => { await held; await route.continue(); }); + await link.click(); + await expect(page.getByTestId("task-chat-thread").locator('[aria-busy="true"]')).toBeVisible(); + await expect(page.getByText("Historical message 159", { exact: true })).not.toBeVisible(); + release(); + await ready(page); + const scroller = page.getByTestId("task-chat-scroller"); + await expect.poll(() => scroller.evaluate((el) => el.scrollHeight - el.scrollTop - el.clientHeight)).toBeLessThan(2); + await page.waitForTimeout(1200); + const frames = await page.evaluate(() => (window as unknown as { taskLayoutEvidence: { frames: { busy: string; top: number; anchor: string; offset: number }[] } }).taskLayoutEvidence.frames); + const visibleFrames = frames.filter((frame) => frame.busy === "false"); + expect(visibleFrames.length).toBeGreaterThan(2); + expect(new Set(visibleFrames.map((f) => f.anchor)).size).toBe(1); + expect(Math.max(...visibleFrames.map((f) => f.offset)) - Math.min(...visibleFrames.map((f) => f.offset))).toBeLessThanOrEqual(2); +}); + +test("reading anchor survives late media, composer resizing, older history and browser Back", async ({ page }) => { + await installMeasurements(page); + await page.goto(`/${prefix}/issues/${issue.identifier}`); + await ready(page); + const scroller = page.getByTestId("task-chat-scroller"); + await expect(page.getByText("Historical message 10", { exact: true })).toBeAttached(); + await scroller.evaluate((el) => { el.scrollTop = el.scrollHeight / 2; }); + await page.waitForTimeout(100); + const anchor = await scroller.evaluate((el) => { + const top = el.getBoundingClientRect().top; + const row = [...el.querySelectorAll("[data-thread-anchor]")].find((r) => r.getBoundingClientRect().bottom > top)!; + return { id: row.dataset.threadAnchor!, top: row.getBoundingClientRect().top - top }; + }); + // Deterministic late media sizing above the reader, after the initial data + // has arrived. This exercises the actual route's ResizeObserver owner. + await scroller.evaluate((el) => { + const media = document.createElement("div"); + media.style.height = "360px"; + el.querySelector("[data-thread-anchor]")!.append(media); + }); + const offset = () => scroller.evaluate((el, id) => { + const row = [...el.querySelectorAll("[data-thread-anchor]")].find((r) => r.dataset.threadAnchor === id)!; + return row.getBoundingClientRect().top - el.getBoundingClientRect().top; + }, anchor.id); + await expect.poll(async () => Math.abs(await offset() - anchor.top)).toBeLessThanOrEqual(2); + await page.getByRole("button", { name: "Hide properties", exact: true }).click(); + await page.waitForTimeout(300); + await expect.poll(async () => Math.abs(await offset() - anchor.top)).toBeLessThanOrEqual(2); + await page.getByRole("button", { name: "Show properties", exact: true }).click(); + await page.waitForTimeout(300); + await expect.poll(async () => Math.abs(await offset() - anchor.top)).toBeLessThanOrEqual(2); + await page.locator('[contenteditable="true"]').last().fill("A multiline draft\n".repeat(8)); + await expect.poll(async () => Math.abs(await offset() - anchor.top)).toBeLessThanOrEqual(2); + // Invoke the real older-history control without scrolling it into view. + await page.getByRole("button", { name: /Load earlier/ }).evaluate((el) => (el as HTMLButtonElement).click()); + await expect(page.getByText("Historical message 0", { exact: true })).toBeAttached(); + await expect.poll(async () => Math.abs(await offset() - anchor.top)).toBeLessThanOrEqual(2); + + await page.getByRole("link", { name: "Tasks", exact: true }).first().click(); + await page.goBack(); + await ready(page); + await expect.poll(async () => Math.abs(await offset() - anchor.top)).toBeLessThanOrEqual(2); + await page.getByRole("button", { name: "Scroll to latest" }).click(); + await page.locator('[contenteditable="true"]').last().fill("Change the composer during the glide\n".repeat(5)); + await expect.poll(() => scroller.evaluate((el) => el.scrollHeight - el.scrollTop - el.clientHeight)).toBeLessThan(2); +}); + +test("explicit comment links resolve older pages before revealing at the target", async ({ page }) => { + await page.goto(`/${prefix}/issues/${issue.identifier}#comment-${oldestCommentId}`); + await ready(page); + const target = page.locator(`[id="comment-${oldestCommentId}"]`); + await expect(target).toBeVisible(); + await expect.poll(async () => { + const row = await target.boundingBox(); + const viewport = await page.getByTestId("task-chat-scroller").boundingBox(); + return Math.abs(row!.y - viewport!.y); + }).toBeLessThanOrEqual(2); +}); + +test("failed initial comments expose retry, then recover without an indefinite skeleton", async ({ page }) => { + await page.route("**/api/issues/*/comments?*", (route) => route.fulfill({ status: 503, contentType: "application/json", body: '{"error":"Temporarily unavailable"}' })); + await page.goto(`/${prefix}/issues/${issue.identifier}`); + await expect(page.getByRole("button", { name: "Retry", exact: true })).toBeVisible({ timeout: 30_000 }); + await page.unroute("**/api/issues/*/comments?*"); + await page.getByRole("button", { name: "Retry", exact: true }).click(); + await ready(page); + await expect(page.getByText("Historical message 159", { exact: true })).toBeAttached(); + await expect(page.getByRole("button", { name: "Retry", exact: true })).toBeHidden(); +}); + +test("mobile reduced-motion cold open and rapid task switching", async ({ page }) => { + await page.setViewportSize({ width: 390, height: 844 }); + await page.emulateMedia({ reducedMotion: "reduce" }); + await page.goto(`/${prefix}/issues/${issue.identifier}`); + await ready(page); + await expect.poll(() => page.evaluate(() => document.documentElement.scrollHeight - window.scrollY - window.innerHeight)).toBeLessThan(2); + await page.evaluate(() => window.scrollTo(0, 1200)); + await page.waitForTimeout(100); + const saved = await page.evaluate(() => window.scrollY); + await page.reload(); + await ready(page); + await expect.poll(() => page.evaluate(() => window.scrollY)).toBeGreaterThan(1200); + // A fresh navigation must show the new task, never retain the previous feed. + await page.goto(`/${prefix}/issues/${other.identifier}`); + await ready(page); + await expect(page.getByText("Historical message 159", { exact: true })).not.toBeAttached(); + expect(saved).toBe(1200); +}); + +for (const mobile of [false, true]) { + test(`same-task comment links and Back restore the mounted ${mobile ? "mobile" : "desktop"} conversation`, async ({ page }) => { + if (mobile) await page.setViewportSize({ width: 390, height: 844 }); + await page.goto(`/${prefix}/issues/${issue.identifier}`); + await ready(page); + await page.getByText("Historical message 10", { exact: true }).waitFor({ state: "attached" }); + await expect(page.locator(`[id="comment-${oldestCommentId}"]`)).toHaveCount(0); + const originalThread = await page.getByTestId("task-chat-thread").elementHandle(); + const before = await page.evaluate((mobile) => { + const root = document.querySelector(mobile ? '[data-testid="task-chat-thread"]' : '[data-testid="task-chat-scroller"]')!; + const top = mobile ? 0 : root.getBoundingClientRect().top; + const row = [...root.querySelectorAll("[data-thread-anchor]")].find((el) => el.getBoundingClientRect().bottom > top)!; + return { id: row.dataset.threadAnchor!, offset: row.getBoundingClientRect().top - top }; + }, mobile); + await page.getByRole("link", { name: "Read the oldest message", exact: true }).click(); + const target = page.locator(`[id="comment-${oldestCommentId}"]`); + await expect(target).toBeAttached(); + const targetOffset = () => target.evaluate((el, mobile) => el.getBoundingClientRect().top - (mobile ? 0 : document.querySelector('[data-testid="task-chat-scroller"]')!.getBoundingClientRect().top), mobile); + await expect.poll(async () => Math.abs(await targetOffset())).toBeLessThanOrEqual(2); + expect(await originalThread!.evaluate((el) => el === document.querySelector('[data-testid="task-chat-thread"]'))).toBe(true); + await page.goBack(); + await expect.poll(async () => Math.abs(await page.evaluate(({ mobile, before }) => { + const root = document.querySelector(mobile ? '[data-testid="task-chat-thread"]' : '[data-testid="task-chat-scroller"]')!; + const row = [...root.querySelectorAll("[data-thread-anchor]")].find((el) => el.dataset.threadAnchor === before.id)!; + return row.getBoundingClientRect().top - (mobile ? 0 : root.getBoundingClientRect().top) - before.offset; + }, { mobile, before }))).toBeLessThanOrEqual(2); + }); +} + +test("stalled native and log history reveal loaded content with Retry after the request deadline", async ({ page }) => { + const runId = "20000000-0000-4000-8000-000000000001"; + const at = new Date().toISOString(); + await page.route("**/api/issues/*/runs", (route) => route.fulfill({ json: [{ runId, runtimeMode: "native", status: "succeeded", agentId: "20000000-0000-4000-8000-000000000002", adapterType: "paperclip_runner", createdAt: at, startedAt: at, finishedAt: at }] })); + let stalled = true; + let release!: () => void; + const held = new Promise((resolve) => { release = resolve; }); + await page.route(`**/api/heartbeat-runs/${runId}/events?*`, async (route) => { + if (stalled) await held; + await route.fulfill({ json: [] }).catch(() => {}); + }); + await page.route(`**/api/heartbeat-runs/${runId}/log?*`, async (route) => { + if (stalled) await held; + await route.fulfill({ json: { runId, content: "", nextOffset: 0 } }).catch(() => {}); + }); + try { + await page.goto(`/${prefix}/issues/${issue.identifier}`); + await expect(page.getByTestId("task-chat-history-loading")).toBeVisible(); + await expect(page.getByText("Some task history could not be loaded.")).toBeVisible({ timeout: 25_000 }); + await ready(page); + await expect(page.getByText("Historical message 159", { exact: true })).toBeVisible(); + stalled = false; + await page.getByRole("button", { name: "Retry", exact: true }).click(); + await expect(page.getByText("Some task history could not be loaded.")).not.toBeVisible(); + } finally { + release(); + } +}); diff --git a/tests/perf/issue-detail/live-feed.walkthrough.mjs b/tests/perf/issue-detail/live-feed.walkthrough.mjs new file mode 100644 index 0000000000..9a2e853458 --- /dev/null +++ b/tests/perf/issue-detail/live-feed.walkthrough.mjs @@ -0,0 +1,110 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import assert from "node:assert/strict"; +import { chromium } from "@playwright/test"; + +// Opt-in: this sends jobs to an already configured agent in a disposable local +// test-drive project. No credentials, provider mocks, or production fixtures. +const target = process.env.PAPERCLIP_LAYOUT_LIVE_URL; +if (!target || !["localhost", "127.0.0.1", "[::1]"].includes(new URL(target).hostname)) { + throw new Error("Set PAPERCLIP_LAYOUT_LIVE_URL to a disposable localhost task with a native Codex assignee."); +} +const output = path.resolve("test-results/task-layout/live-acceptance"); +const api = (pathname) => new URL(pathname, target).href; +await fs.mkdir(output, { recursive: true }); +const browser = await chromium.launch(); +const context = await browser.newContext({ viewport: { width: 1440, height: 900 }, recordVideo: { dir: output } }); +const page = await context.newPage(); +const cdp = await context.newCDPSession(page); +await cdp.send("Performance.enable"); +await page.addInitScript(() => { + const evidence = { frames: [], shifts: [] }; + window.taskLayoutEvidence = evidence; + new PerformanceObserver((list) => evidence.shifts.push(...list.getEntries().map((entry) => entry.toJSON()))) + .observe({ type: "layout-shift", buffered: true }); + function sample() { + const viewport = document.querySelector('[data-testid="task-chat-scroller"]'); + if (viewport) { + const rect = viewport.getBoundingClientRect(); + const row = [...viewport.querySelectorAll("[data-thread-anchor]")].find((el) => el.getBoundingClientRect().bottom > rect.top); + evidence.frames.push({ time: performance.now(), top: viewport.scrollTop, height: viewport.scrollHeight, anchor: row?.dataset.threadAnchor, offset: row ? row.getBoundingClientRect().top - rect.top : null, textLength: viewport.textContent.length, loading: Boolean(document.querySelector('[data-testid="task-chat-history-loading"]')) }); + } + requestAnimationFrame(sample); + } + requestAnimationFrame(sample); +}); + +try { + await page.goto(target); + await page.locator('[data-testid="task-chat-thread"] [aria-busy="false"]').waitFor({ timeout: 90_000 }); + const issueKey = new URL(target).pathname.split("/").filter(Boolean).at(-1); + const issueResponse = await page.request.get(api(`/api/issues/${issueKey}`)); + assert.ok(issueResponse.ok(), "Disposable task must be accessible"); + const issue = await issueResponse.json(); + const agent = await (await page.request.get(api(`/api/agents/${issue.assigneeAgentId}`))).json(); + assert.equal(agent.adapterType, "paperclip_runner"); + assert.equal(agent.adapterConfig.provider, "codex"); + const startedAt = Date.now(); + const editor = page.locator('[contenteditable="true"]').last(); + await editor.fill("Run a paced layout acceptance check only in this disposable project. Send three substantial progress messages explaining chat scroll stability, separated by read-only terminal checks of sum.mjs and a ten-second pause. Include a Markdown table and a JavaScript code block in your last progress message. Do not change files. Finish with a detailed answer in the conversation. Keep the run open for this sequence so I can test a follow-up."); + await page.getByRole("button", { name: "Send", exact: true }).click(); + await page.getByText(/Working for/).last().waitFor({ timeout: 30_000 }); + await page.screenshot({ path: path.join(output, "startup.png") }); + await page.waitForTimeout(12_000); + const viewport = page.getByTestId("task-chat-scroller"); + await viewport.evaluate((el) => { el.scrollTop = Math.max(0, el.scrollTop - 800); }); + await page.waitForTimeout(150); + const anchor = await viewport.evaluate((el) => { + const rect = el.getBoundingClientRect(); + const row = [...el.querySelectorAll("[data-thread-anchor]")].find((item) => item.getBoundingClientRect().bottom > rect.top); + return { id: row.dataset.threadAnchor, top: row.getBoundingClientRect().top - rect.top }; + }); + await page.screenshot({ path: path.join(output, "reading.png") }); + await page.waitForTimeout(10_000); + await context.setOffline(true); + await page.waitForTimeout(3000); + await context.setOffline(false); + await page.waitForTimeout(10_000); + const offset = await viewport.evaluate((el, id) => { + const row = [...el.querySelectorAll("[data-thread-anchor]")].find((item) => item.dataset.threadAnchor === id); + return row.getBoundingClientRect().top - el.getBoundingClientRect().top; + }, anchor.id); + assert.ok(Math.abs(offset - anchor.top) <= 2, `Reading anchor moved ${offset - anchor.top}px`); + await page.screenshot({ path: path.join(output, "reconnected.png") }); + await fs.writeFile(path.join(output, "reading-anchor.json"), JSON.stringify({ anchor, finalOffset: offset, delta: offset - anchor.top }, null, 2)); + await page.getByRole("button", { name: "Scroll to latest" }).click(); + await editor.fill("Include how expanded tool details should remain open after the run becomes persisted history."); + await page.waitForFunction(() => { + const el = document.querySelector('[data-testid="task-chat-scroller"]'); + return el && el.scrollHeight - el.scrollTop - el.clientHeight < 2; + }); + await page.getByRole("button", { name: "Send", exact: true }).click(); + const steer = page.getByRole("button", { name: "Steer", exact: true }); + await steer.waitFor({ timeout: 30_000 }); + const deadline = Date.now() + 60_000; + while (!(await steer.isEnabled()) && Date.now() < deadline) await page.waitForTimeout(500); + await steer.click(); + await page.screenshot({ path: path.join(output, "steered.png") }); + await page.getByText(/Continued after steering/).last().waitFor({ timeout: 30_000 }); + await page.waitForFunction(() => !document.querySelector('[data-testid="task-chat-live-transcript"]')?.textContent?.includes("Working for"), undefined, { timeout: 180_000 }); + await page.waitForTimeout(5000); + await page.screenshot({ path: path.join(output, "completed.png") }); + const runs = await (await page.request.get(api(`/api/issues/${issue.id}/runs`))).json(); + const run = runs.find((candidate) => new Date(candidate.createdAt).getTime() >= startedAt); + assert.ok(run, "The submitted job must create a run"); + assert.equal(run.runtimeMode, "native"); + assert.equal(run.adapterType, "paperclip_runner"); + assert.equal(run.status, "succeeded"); + await fs.writeFile(path.join(output, "runtime.json"), JSON.stringify({ + runId: run.runId, runtimeMode: run.runtimeMode, adapterType: run.adapterType, + provider: agent.adapterConfig.provider, model: agent.adapterConfig.model, + status: run.status, startedAt: run.startedAt, finishedAt: run.finishedAt, + }, null, 2)); + await fs.writeFile(path.join(output, "reading-anchor.json"), JSON.stringify({ anchor, finalOffset: offset, delta: offset - anchor.top }, null, 2)); + console.log(`Reading anchor delta: ${offset - anchor.top}px`); +} finally { + await fs.writeFile(path.join(output, "layout.json"), JSON.stringify(await page.evaluate(() => window.taskLayoutEvidence))); + await fs.writeFile(path.join(output, "performance.json"), JSON.stringify(await cdp.send("Performance.getMetrics"), null, 2)); + await context.close(); + await browser.close(); +} diff --git a/tests/perf/issue-detail/playwright.config.ts b/tests/perf/issue-detail/playwright.config.ts index 57c1ecd82b..6e2d57dd23 100644 --- a/tests/perf/issue-detail/playwright.config.ts +++ b/tests/perf/issue-detail/playwright.config.ts @@ -4,7 +4,18 @@ import path from "node:path"; import { defineConfig } from "@playwright/test"; const PORT = Number(process.env.PAPERCLIP_ISSUE_PERF_PORT ?? 3201); -const BASE_URL = `http://127.0.0.1:${PORT}`; +const EXTERNAL_URL = process.env.PAPERCLIP_ISSUE_PERF_BASE_URL; +if (EXTERNAL_URL) { + const target = new URL(EXTERNAL_URL); + if ( + !["http:", "https:"].includes(target.protocol) || + !["localhost", "127.0.0.1", "[::1]"].includes(target.hostname) || + target.username || target.password || target.search || target.hash || target.pathname !== "/" + ) { + throw new Error("PAPERCLIP_ISSUE_PERF_BASE_URL must be a loopback origin for a disposable local instance; these tests create fixtures."); + } +} +const BASE_URL = EXTERNAL_URL ?? `http://127.0.0.1:${PORT}`; const PAPERCLIP_HOME = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-issue-perf-home-")); const PAPERCLIP_INSTANCE_ID = "playwright-issue-perf"; const PAPERCLIP_CONFIG = path.join(PAPERCLIP_HOME, "instances", PAPERCLIP_INSTANCE_ID, "config.json"); @@ -14,7 +25,7 @@ process.env.PAPERCLIP_CONFIG = PAPERCLIP_CONFIG; export default defineConfig({ testDir: ".", - testMatch: "issue-detail.perf.spec.ts", + testMatch: "*.spec.ts", timeout: 30 * 60_000, workers: 1, fullyParallel: false, @@ -23,7 +34,7 @@ export default defineConfig({ browserName: "chromium", headless: true, }, - webServer: { + webServer: EXTERNAL_URL ? undefined : { command: "pnpm paperclipai onboard --yes --run", url: `${BASE_URL}/api/health`, reuseExistingServer: false, diff --git a/ui/src/api/heartbeats.ts b/ui/src/api/heartbeats.ts index 68b7926ed3..4d2863655b 100644 --- a/ui/src/api/heartbeats.ts +++ b/ui/src/api/heartbeats.ts @@ -7,7 +7,7 @@ import type { ProviderTraceMetadata, } from "@paperclipai/shared"; import { tenantSessionRecovery } from "@/lib/tenant-session-recovery"; -import { api } from "./client"; +import { api, type RequestOptions } from "./client"; export interface RunLivenessFields { livenessState: HeartbeatRun["livenessState"]; @@ -128,11 +128,12 @@ export const heartbeatsApi = { ); }, get: (runId: string) => api.get(`/heartbeat-runs/${runId}`), - events: (runId: string, afterSeq = 0, limit = 200) => + events: (runId: string, afterSeq = 0, limit = 200, options?: RequestOptions) => api.get( `/heartbeat-runs/${runId}/events?afterSeq=${encodeURIComponent(String(afterSeq))}&limit=${encodeURIComponent(String(limit))}`, + options, ), - log: (runId: string, offset = 0, limitBytes = 256000) => + log: (runId: string, offset = 0, limitBytes = 256000, options?: RequestOptions) => api.get<{ runId: string; store: string; @@ -141,6 +142,7 @@ export const heartbeatsApi = { nextOffset?: number; }>( `/heartbeat-runs/${runId}/log?offset=${encodeURIComponent(String(offset))}&limitBytes=${encodeURIComponent(String(limitBytes))}`, + options, ), workspaceOperations: (runId: string) => api.get( diff --git a/ui/src/components/TaskChatThread.test.tsx b/ui/src/components/TaskChatThread.test.tsx index 9f8406f665..599aa6193c 100644 --- a/ui/src/components/TaskChatThread.test.tsx +++ b/ui/src/components/TaskChatThread.test.tsx @@ -141,6 +141,32 @@ function render(ui: ReactElement) { ); } +it("coordinates first reveal while keeping the composer and visible history mounted through refresh", async () => { + const props = { issueId: "coordinated-issue", comments: [], onAdd: async () => {} }; + render(); + const composer = container.querySelector('[data-testid="mock-editor"]'); + expect(composer).not.toBeNull(); + expect(container.querySelector('[aria-busy="true"]')).not.toBeNull(); + render(); + await act(async () => { await new Promise((resolve) => requestAnimationFrame(resolve)); }); + expect(container.querySelector('[aria-busy="false"]')).not.toBeNull(); + expect(container.querySelector('[data-testid="mock-editor"]')).toBe(composer); + render(); + expect(container.querySelector('[data-testid="task-chat-history-loading"]')).toBeNull(); + expect(container.querySelector('[data-testid="mock-editor"]')).toBe(composer); +}); + +it("keeps an acknowledged optimistic bubble mounted with its canonical comment target", () => { + const comment = { companyId: "company", issueId: "issue", authorAgentId: null, presentation: null, metadata: null, updatedAt: new Date("2026-09-09T12:00:00Z"), id: "optimistic-one", clientId: "optimistic-one", body: "Keep this message in place", authorType: "user" as const, authorUserId: "board", createdAt: new Date("2026-09-09T12:00:00Z") }; + render( {}} />); + const row = container.querySelector('[data-thread-anchor="optimistic-one"]'); + expect(row).not.toBeNull(); + render( {}} />); + expect(container.querySelector('[data-thread-anchor="optimistic-one"]')).toBe(row); + expect(row?.id).toBe("comment-canonical-one"); + expect(container.textContent?.match(/Keep this message in place/g)).toHaveLength(1); +}); + function fakeScrollGeometry( element: HTMLElement, { scrollHeight = 1000, clientHeight = 400, scrollTop = 600 } = {}, diff --git a/ui/src/components/TaskChatThread.tsx b/ui/src/components/TaskChatThread.tsx index f91d797dab..d08d0ebca3 100644 --- a/ui/src/components/TaskChatThread.tsx +++ b/ui/src/components/TaskChatThread.tsx @@ -1,4 +1,6 @@ import { requiresExecutionReconciliation } from "@paperclipai/shared"; +import { TaskChatExpansionState } from "@/components/task-chat/expansion-state"; +import { TaskChatScrollReady } from "@/components/task-chat/scroll-navigation"; import { useCallback, useEffect, @@ -84,10 +86,11 @@ import { useRunnerGoalControl, } from "@/components/task-chat/RunnerGoalWidget"; import { TaskChatQueuedMessages } from "@/components/task-chat/TaskChatQueuedMessages"; -import { useWindowAutoFollow } from "@/components/task-chat/useWindowAutoFollow"; +import { TaskChatWindowScroll } from "@/components/task-chat/useWindowAutoFollow"; import { useSidebar } from "@/context/SidebarContext"; import { useStreamlinedUiEnabled } from "@/hooks/useStreamlinedUiEnabled"; import { cn } from "@/lib/utils"; +import { Skeleton } from "@/components/ui/skeleton"; import { Button } from "@/components/ui/button"; import { useIssuePlanDocument } from "@/hooks/useIssuePlanDocument"; import { latestSameRunHandoffTimestamp } from "@/lib/issue-chat-messages"; @@ -389,7 +392,11 @@ function resolvedWithoutUserFacingResponse(value: unknown): boolean { ); } -export type TaskChatThreadProps = ComponentProps; +export type TaskChatThreadProps = ComponentProps & { + initialHistoryPending?: boolean; + initialHistoryError?: boolean; + onRetryInitialHistory?: () => void; +}; type PendingComposerInput = | { @@ -456,6 +463,9 @@ function durableInputLabel( export function TaskChatThread(props: TaskChatThreadProps) { const { enabled: streamlinedUiEnabled } = useStreamlinedUiEnabled(); const { + initialHistoryPending = false, + initialHistoryError = false, + onRetryInitialHistory, comments, interactions, documents = [], @@ -815,6 +825,9 @@ export function TaskChatThread(props: TaskChatThreadProps) { const { transcriptByRun: logTranscriptByRun, isInitialHydrating: logsAreInitiallyHydrating, + hydratedRunIds: hydratedLogRunIds, + errorsByRun: logErrorsByRun, + retry: retryLogs, } = useLiveRunTranscripts({ // Native events are authoritative, but the persisted/live log remains a // compatibility source when an upgraded server has no event history or @@ -825,20 +838,34 @@ export function TaskChatThread(props: TaskChatThreadProps) { const { transcriptByRun: nativeTranscriptByRun, errorsByRun: nativeTranscriptErrorsByRun, + isInitialHydrating: nativeEventsAreInitiallyHydrating, + hydratedRunIds: hydratedNativeRunIds, + retry: retryNativeEvents, } = useNativeRunTranscripts(nativeRuns); + const fallbackByRunRef = useRef(new Map>>()); const transcriptByRun = useMemo(() => { const next = new Map(logTranscriptByRun); for (const run of nativeRuns) { - const logTranscript = logTranscriptByRun.get(run.id) ?? []; const nativeTranscript = nativeTranscriptByRun.get(run.id) ?? []; - const nativeEventsUnavailable = nativeTranscriptErrorsByRun.has(run.id); - if ( - nativeTranscript.length > 0 && - (!nativeEventsUnavailable || logTranscript.length === 0) - ) { + const logTranscript = logTranscriptByRun.get(run.id) ?? []; + if (nativeTranscriptErrorsByRun.has(run.id) && logTranscript.length > 0) { + fallbackByRunRef.current.set(run.id, logTranscript); + } + const fallback = fallbackByRunRef.current.get(run.id); + const lastTimestamp = (entries: typeof nativeTranscript) => + entries.reduce((latest, entry) => Math.max(latest, toMs(entry.ts)), 0); + // Keep a newer fallback visible through native transport recovery. An + // empty successful poll must not rewind a response the reader just saw. + if (fallback && (nativeTranscriptErrorsByRun.has(run.id) || lastTimestamp(nativeTranscript) < lastTimestamp(fallback))) { + next.set(run.id, fallback); + } else if (nativeTranscript.length > 0) { + fallbackByRunRef.current.delete(run.id); next.set(run.id, nativeTranscript); } } + for (const id of fallbackByRunRef.current.keys()) { + if (!nativeRuns.some((run) => run.id === id)) fallbackByRunRef.current.delete(id); + } return next; }, [ logTranscriptByRun, @@ -958,7 +985,7 @@ export function TaskChatThread(props: TaskChatThreadProps) { return map; }, [comments]); - const { data: planDocument } = useIssuePlanDocument(issueId); + const { data: planDocument, isLoading: planLoading, isError: planError, refetch: retryPlan } = useIssuePlanDocument(issueId); // A native-runner Plan is part of the turn that wrote its current revision. // Legacy adapters do not expose the semantic write_document boundary needed @@ -2426,6 +2453,7 @@ export function TaskChatThread(props: TaskChatThreadProps) { : runtimeRequestReplacesComposerSkip(selectedPendingInput.item), } : null; + const expansionState = useRef(new Map()); const autoFollowContentKey = `${threadContentKey}:${composerTakeover?.id ?? "composer"}`; // Mobile (PAP-360): the app shell scrolls the DOCUMENT (Layout's main is @@ -2435,9 +2463,53 @@ export function TaskChatThread(props: TaskChatThreadProps) { // and track auto-follow against window scroll. Both paths include takeover // state so opening or closing composer input preserves bottom pinning. const { isMobile } = useSidebar(); - useWindowAutoFollow(isMobile ? autoFollowContentKey : 0, isMobile); + const initialCommentWindow = useRef<{ oldestAt: number; ids: Set } | null>(null); + if (!initialCommentWindow.current && comments.length > 0) { + initialCommentWindow.current = { + oldestAt: Math.min(...comments.map((comment) => toMs(comment.createdAt))), + ids: new Set(comments.map((comment) => comment.id)), + }; + } + const initialCommentRunIds = new Set(comments + .filter((comment) => initialCommentWindow.current?.ids.has(comment.id)) + .flatMap((comment) => [comment.runId, comment.createdByRunId, comment.derivedCreatedByRunId])); + const initialRuns = runs.filter((run) => { + if (!initialCommentWindow.current || !isTerminalRunStatus(run.status) || initialCommentRunIds.has(run.id)) return true; + const metadata = linkedRunMetaById.get(run.id); + return !metadata || toMs(metadata.finishedAt ?? metadata.startedAt ?? metadata.createdAt) >= initialCommentWindow.current.oldestAt; + }); + const historyPending = initialHistoryPending || planLoading || + initialRuns.some((run) => { + if (run.runtimeMode === "native" && (hydratedNativeRunIds ? !hydratedNativeRunIds.has(run.id) : nativeEventsAreInitiallyHydrating)) return true; + if (run.runtimeMode === "native" && (nativeTranscriptByRun.get(run.id)?.length ?? 0) > 0) return false; + return run.status !== "queued" && hydratedLogRunIds ? !hydratedLogRunIds.has(run.id) : logsAreInitiallyHydrating; + }); + const historyError = initialHistoryError || planError || runs.some((run) => + run.runtimeMode === "native" + ? nativeTranscriptErrorsByRun.has(run.id) && (logTranscriptByRun.get(run.id)?.length ?? 0) === 0 + : Boolean(logErrorsByRun?.has(run.id)), + ); + const [revealedIssue, setRevealedIssue] = useState(() => historyPending ? undefined : issueId); + const historyRevealed = revealedIssue === issueId; + // Mount and measure the real thread while concealed, then reveal in one + // commit. A frame also lets ancestor navigation scroll restoration finish. + // Readiness is latched per issue: refetches never hide existing conversation. + useEffect(() => { + if (historyRevealed || historyPending) return; + const frame = requestAnimationFrame(() => setRevealedIssue(issueId)); + return () => cancelAnimationFrame(frame); + }, [historyPending, historyRevealed, issueId]); + const retryHistory = () => { + onRetryInitialHistory?.(); + retryLogs?.(); + retryNativeEvents?.(); + void retryPlan(); + }; return ( + + + @@ -2448,7 +2520,24 @@ export function TaskChatThread(props: TaskChatThreadProps) { )} data-testid="task-chat-thread" > -
+
+ {historyError ? ( +
+ Some task history could not be loaded. + +
+ ) : null} + {!historyRevealed ? ( +
+
+ {threadHeader} + + + +
+
+ ) : null} +
{items.length === 0 && !tailRunId ? (
) : null } - contentKey={autoFollowContentKey} + contentKey={`${autoFollowContentKey}:${historyRevealed}`} className={isMobile ? undefined : "pt-3"} scroll={!isMobile} /> )} +
{assignedAgentForNotice?.status === "paused" ? (
@@ -2686,5 +2776,7 @@ export function TaskChatThread(props: TaskChatThreadProps) { ) : null}
+ + ); } diff --git a/ui/src/components/task-chat/TaskChatActivityPhase.test.tsx b/ui/src/components/task-chat/TaskChatActivityPhase.test.tsx index bdcc4ae5d7..f37edf6e18 100644 --- a/ui/src/components/task-chat/TaskChatActivityPhase.test.tsx +++ b/ui/src/components/task-chat/TaskChatActivityPhase.test.tsx @@ -3,8 +3,47 @@ import { flushSync } from "react-dom"; import { createRoot } from "react-dom/client"; import { afterEach, describe, expect, it } from "vitest"; import { TaskChatActivityPhase } from "./TaskChatActivityPhase"; +import { TaskChatToolCard } from "./TaskChatToolCard"; +import { TaskChatExpansionState } from "./expansion-state"; +import type { TaskChatToolItem } from "./task-chat-model"; describe("TaskChatActivityPhase", () => { + it("retains expanded phase and tool details when live activity becomes persisted history", () => { + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + const memory = new Map(); + const render = (host: string, status: TaskChatToolItem["status"]) => flushSync(() => root.render( + + } + /> + , + )); + render("live", "in_progress"); + flushSync(() => container.querySelector('[data-testid="task-chat-phase-summary"]')!.click()); + flushSync(() => container.querySelector('[data-testid="task-chat-tool-card"] button')!.click()); + render("history", "completed"); + expect(container.querySelector('[data-testid="task-chat-phase-summary"]')?.getAttribute("aria-expanded")).toBe("true"); + expect(container.querySelector('[data-testid="task-chat-tool-card"] button')?.getAttribute("aria-expanded")).toBe("true"); + expect(container.textContent).toContain("Source contents stay visible"); + flushSync(() => container.querySelector('[data-testid="task-chat-phase-summary"]')!.click()); + flushSync(() => root.render( + + null} /> + , + )); + expect(container.querySelector('[data-testid="task-chat-phase-summary"]')?.getAttribute("aria-expanded")).toBe("false"); + flushSync(() => root.unmount()); + }); + afterEach(() => { document.body.innerHTML = ""; }); diff --git a/ui/src/components/task-chat/TaskChatActivityPhase.tsx b/ui/src/components/task-chat/TaskChatActivityPhase.tsx index 837cef2638..f2ebeff67e 100644 --- a/ui/src/components/task-chat/TaskChatActivityPhase.tsx +++ b/ui/src/components/task-chat/TaskChatActivityPhase.tsx @@ -1,4 +1,5 @@ -import { useEffect, useState, type ReactNode } from "react"; +import { TaskChatExpansionState, useTaskChatExpansion } from "./expansion-state"; +import { useContext, useEffect, type ReactNode } from "react"; import { Brain, ChevronRight, CircleEllipsis } from "lucide-react"; import { cn } from "@/lib/utils"; import { MarkdownBody } from "@/components/MarkdownBody"; @@ -62,10 +63,11 @@ export function TaskChatActivityPhase({ child.surface === "runtime_request" && child.status === "pending"), ))); - const [open, setOpen] = useState(shouldAutoOpen); + const [open, setOpen] = useTaskChatExpansion(item.id, shouldAutoOpen); + const expansionMemory = useContext(TaskChatExpansionState); useEffect(() => { - if (shouldAutoOpen) setOpen(true); - }, [shouldAutoOpen]); + if (shouldAutoOpen && !expansionMemory?.has(item.id)) setOpen(true); + }, [shouldAutoOpen, expansionMemory, item.id, setOpen]); const expandable = item.items.length > 0; const runnerAppearance = appearance === "runner"; const SummaryIcon = runnerAppearance ? representativeIcon(item) : null; diff --git a/ui/src/components/task-chat/TaskChatLiveTail.test.tsx b/ui/src/components/task-chat/TaskChatLiveTail.test.tsx index b38466f953..e2a43a4a3f 100644 --- a/ui/src/components/task-chat/TaskChatLiveTail.test.tsx +++ b/ui/src/components/task-chat/TaskChatLiveTail.test.tsx @@ -72,7 +72,7 @@ describe("TaskChatLiveTail", () => { expect(container.textContent).not.toContain("const x = 1;"); expect(container.textContent).not.toContain("+1 −1"); - const tool = container.querySelector(".tc-enter-tool button"); + const tool = container.querySelector('[data-testid="task-chat-tool-card"] button'); expect(tool).not.toBeNull(); flushSync(() => tool?.click()); @@ -195,7 +195,7 @@ describe("TaskChatLiveTail", () => { render(items); const tool = container.querySelector( - ".tc-enter-tool button", + '[data-testid="task-chat-tool-card"] button', ); const collapsedTarget = tool?.querySelector( ".task-chat-collapsed-line-fade", diff --git a/ui/src/components/task-chat/TaskChatRunnerTurn.tsx b/ui/src/components/task-chat/TaskChatRunnerTurn.tsx index 8def0fb34c..9eab3a83ba 100644 --- a/ui/src/components/task-chat/TaskChatRunnerTurn.tsx +++ b/ui/src/components/task-chat/TaskChatRunnerTurn.tsx @@ -15,7 +15,9 @@ import type { TaskChatThinkingItem, TaskChatToolItem, } from "./task-chat-model"; -import { TaskChatAgentIdentity } from "./TaskChatBubble"; +import { TaskChatAgentIdentity, TaskChatBubble } from "./TaskChatBubble"; +import { TaskChatBubbleActions } from "./TaskChatBubbleActions"; +import { formatTaskChatTimestamp } from "./task-chat-adapter"; import { TaskChatActivityPhase } from "./TaskChatActivityPhase"; import { TaskChatProtocolActivityRow } from "./TaskChatProtocolActivityRow"; import { TaskChatProtocolCard } from "./TaskChatProtocolCard"; @@ -238,7 +240,7 @@ function RunnerActivityTimeline({ items }: { items: readonly TaskChatItem[] }) {
  • {item.kind === "message" ? (
    @@ -562,6 +564,7 @@ export function TaskChatRunnerTurn({ key={`${runId ?? "run"}:${row.id}`} data-testid="task-chat-turn-timeline-row" data-timeline-row-id={row.id} + data-thread-anchor={row.id} > {row.kind === "activity_phase" ? ( -
    - - {final.text} - -
    + } + />
    ) : null} - {(!execution || execution.phase === "working") ? : null} + {!final && (!execution || execution.phase === "working") ? : null}
  • ); } diff --git a/ui/src/components/task-chat/TaskChatThreadView.tsx b/ui/src/components/task-chat/TaskChatThreadView.tsx index 789d387479..1d3e5305c8 100644 --- a/ui/src/components/task-chat/TaskChatThreadView.tsx +++ b/ui/src/components/task-chat/TaskChatThreadView.tsx @@ -134,12 +134,10 @@ function renderItem( return ( (
    @@ -381,7 +381,9 @@ export function TaskChatThreadView({ )) : items.map((item, index) => (
    0 && item.kind === "interaction" && diff --git a/ui/src/components/task-chat/TaskChatToolCard.tsx b/ui/src/components/task-chat/TaskChatToolCard.tsx index ba40af19a2..4c28dca373 100644 --- a/ui/src/components/task-chat/TaskChatToolCard.tsx +++ b/ui/src/components/task-chat/TaskChatToolCard.tsx @@ -1,4 +1,4 @@ -import { useState } from "react"; +import { useTaskChatExpansion } from "./expansion-state"; import { cn } from "@/lib/utils"; import { Check, @@ -32,11 +32,11 @@ const STATUS_ICON = { export function TaskChatToolCard({ item }: { item: TaskChatToolItem }) { const { Icon, spin, tone } = STATUS_ICON[item.status]; const RowIcon = toolTaxonomy(item.rawName ?? item.name).icon; - const [showDetail, setShowDetail] = useState(false); + const [showDetail, setShowDetail] = useTaskChatExpansion(item.id, false); const expandable = Boolean(item.target || item.detail || item.diff); return ( -
    +
    diff --git a/ui/src/components/task-chat/TaskMessageScroller.test.tsx b/ui/src/components/task-chat/TaskMessageScroller.test.tsx index 2213217ef6..88d297f764 100644 --- a/ui/src/components/task-chat/TaskMessageScroller.test.tsx +++ b/ui/src/components/task-chat/TaskMessageScroller.test.tsx @@ -4,6 +4,7 @@ import { flushSync } from "react-dom"; import { createRoot, type Root } from "react-dom/client"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { TaskMessageScroller } from "./TaskMessageScroller"; +import { TaskChatScrollNavigation } from "./scroll-navigation"; const PILL_SELECTOR = 'button[aria-label="Scroll to latest"]'; @@ -124,6 +125,39 @@ describe("TaskMessageScroller", () => { vi.unstubAllGlobals(); }); + it("applies same-task hash changes and restores each history entry without remounting", async () => { + let initialized = false; + function navigate(key: string, hash: string, restore = false) { + flushSync(() => root.render( + + +
    { + if (node && !initialized) { + fakeGeometry(node.parentElement!); + initialized = true; + } + }}> + {[100, 500].map((top, index) =>
    { + if (node) node.getBoundingClientRect = () => ({ top: top - scroller().scrollTop, bottom: top + 100 - scroller().scrollTop, height: 100 } as DOMRect); + }}>Comment {index}
    )} +
    +
    +
    , + )); + } + navigate("desktop-entry-one", "#nav-comment-0"); + const viewport = scroller(); + expect(viewport.scrollTop).toBe(100); + await scrollTo(viewport, 150); + navigate("desktop-entry-two", "#nav-comment-1"); + expect(scroller()).toBe(viewport); + expect(viewport.scrollTop).toBe(500); + navigate("desktop-entry-one", "#nav-comment-0", true); + expect(viewport.scrollTop).toBe(150); + navigate("desktop-entry-one", "#nav-comment-1", true); + expect(viewport.scrollTop).toBe(500); + }); + it("renders children inside the scroll container, pill hidden, scrolled to bottom on mount", () => { render(); const el = scroller(); @@ -266,6 +300,7 @@ describe("TaskMessageScroller", () => { }); it("clicking the pill smooth-scrolls, ignores intermediate scroll events, re-pins on arrival", async () => { + vi.stubGlobal("matchMedia", vi.fn().mockReturnValue({ matches: false })); render(1); const el = scroller(); fakeGeometry(el); @@ -287,6 +322,7 @@ describe("TaskMessageScroller", () => { // Arrival within the threshold re-pins and hides the pill (immediately // here: no matchMedia in jsdom → reduced-motion/unmount-now path). + vi.stubGlobal("matchMedia", vi.fn().mockReturnValue({ matches: true })); await scrollTo(el, 600); expect(pill()).toBeNull(); @@ -295,7 +331,23 @@ describe("TaskMessageScroller", () => { expect(el.scrollTop).toBe(1000); }); + it("finishes following at the new bottom when content changes during the latest glide", async () => { + vi.stubGlobal("matchMedia", vi.fn().mockReturnValue({ matches: false })); + render(1); + const el = scroller(); + fakeGeometry(el); + el.scrollTo = vi.fn() as unknown as typeof el.scrollTo; + await scrollTo(el, 100); + await waitForPill(true); + pill()!.click(); + await flushEvents(); + await scrollTo(el, 250); + render(2); + expect(el.scrollTop).toBe(1000); + }); + it("a wheel gesture during the glide cancels easing and stays unpinned", async () => { + vi.stubGlobal("matchMedia", vi.fn().mockReturnValue({ matches: false })); render(1); const el = scroller(); fakeGeometry(el); diff --git a/ui/src/components/task-chat/TaskMessageScroller.tsx b/ui/src/components/task-chat/TaskMessageScroller.tsx index 8d5869cf80..ed75caa295 100644 --- a/ui/src/components/task-chat/TaskMessageScroller.tsx +++ b/ui/src/components/task-chat/TaskMessageScroller.tsx @@ -1,8 +1,10 @@ import { useCallback, useEffect, useLayoutEffect, useRef, useState, type ReactNode } from "react"; +import { readThreadScrollAnchor, threadScrollAnchorDelta, type ThreadScrollAnchor } from "./scroll-anchor"; import { cn } from "@/lib/utils"; import { useStreamlinedTaskChatPresentation } from "./presentation-mode"; import { ArrowDown } from "lucide-react"; import { parseCssTimeMs } from "./motion-tokens"; +import { useTaskChatScrollNavigation } from "./scroll-navigation"; const PIN_THRESHOLD_PX = 48; @@ -44,7 +46,11 @@ interface TaskMessageScrollerProps { */ export function TaskMessageScroller({ children, contentKey, className }: TaskMessageScrollerProps) { const streamlined = useStreamlinedTaskChatPresentation(); + const navigation = useTaskChatScrollNavigation(); + const initialPositionApplied = useRef(false); + const appliedNavigation = useRef({ key: navigation.key, hash: navigation.hash }); const ref = useRef(null); + const anchorRef = useRef(null); const pinnedRef = useRef(true); const easingRef = useRef(false); const clientHeightRef = useRef(null); @@ -110,7 +116,35 @@ export function TaskMessageScroller({ children, contentKey, className }: TaskMes return true; }, [scrollToBottom]); + const rememberAnchor = useCallback(() => { + const el = ref.current; + if (!el) return; + const rect = el.getBoundingClientRect(); + anchorRef.current = readThreadScrollAnchor(el, rect.top, rect.bottom); + if (initialPositionApplied.current) navigation.remember(el.scrollTop, anchorRef.current); + }, [navigation.key, navigation.hash, navigation.ready]); + + const reconcileContent = useCallback(() => { + const el = ref.current; + if (!el) return; + // Clicking latest is an explicit follow intent. If content or the composer + // changes during its glide, finish at the new bottom instead of restoring + // the old reading anchor and cancelling the browser's smooth scroll. + if (easingRef.current) { + easingRef.current = false; + pinnedRef.current = true; + hidePill(); + } + if (pinnedRef.current) scrollToBottom(); + else { + const delta = threadScrollAnchorDelta(el, anchorRef.current, el.getBoundingClientRect().top); + if (delta) el.scrollTop += delta; + } + rememberAnchor(); + }, [rememberAnchor, scrollToBottom, hidePill]); + const handleScroll = useCallback(() => { + rememberAnchor(); showScrollbarWhileScrolling(); // A growing composer shrinks this viewport. Some browsers dispatch the // resulting scroll event before ResizeObserver, so preserve the previous @@ -135,6 +169,7 @@ export function TaskMessageScroller({ children, contentKey, className }: TaskMes if (pinned) hidePill(); else showPill(); }, [ + rememberAnchor, followViewportResize, isPinned, hidePill, @@ -152,7 +187,7 @@ export function TaskMessageScroller({ children, contentKey, className }: TaskMes return; } easingRef.current = true; - if (typeof el.scrollTo === "function") { + if (typeof el.scrollTo === "function" && !motionDisabled()) { el.scrollTo({ top: el.scrollHeight, behavior: "smooth" }); } else { // Environments without scrollTo (older jsdom): fall back to instant. @@ -195,20 +230,33 @@ export function TaskMessageScroller({ children, contentKey, className }: TaskMes if (typeof ResizeObserver === "undefined") return; const observer = new ResizeObserver(() => { if (followViewportResize()) hidePill(); + reconcileContent(); }); observer.observe(el); + // The viewport itself does not resize when an image or historical row + // grows. Observe the content box too, before the browser paints it. + if (el.firstElementChild) observer.observe(el.firstElementChild); return () => observer.disconnect(); - }, [followViewportResize, hidePill]); + }, [followViewportResize, hidePill, reconcileContent]); // Follow new content only when already pinned; otherwise hold position. useLayoutEffect(() => { - if (pinnedRef.current) scrollToBottom(); - }, [contentKey, scrollToBottom]); - - useEffect(() => { - scrollToBottom(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + const el = ref.current; + if (appliedNavigation.current.key !== navigation.key || appliedNavigation.current.hash !== navigation.hash) { + appliedNavigation.current = { key: navigation.key, hash: navigation.hash }; + initialPositionApplied.current = false; + } + if (el && navigation.ready && !initialPositionApplied.current) { + const top = navigation.initialPosition(el, el.getBoundingClientRect().top, el.scrollTop); + if (top !== null) { + el.scrollTop = top; + pinnedRef.current = isPinned(); + rememberAnchor(); + } + initialPositionApplied.current = true; + } + reconcileContent(); + }, [contentKey, reconcileContent, navigation.key, navigation.hash, navigation.ready]); return (
    @@ -222,7 +270,7 @@ export function TaskMessageScroller({ children, contentKey, className }: TaskMes // right gutter; matching padding preserves the message column while // placing the scrollbar against the properties-panel boundary. className={cn( - "scrollbar-while-scrolling absolute inset-y-0 left-0 overflow-y-auto", + "task-chat-scroll-viewport scrollbar-while-scrolling absolute inset-y-0 left-0 overflow-y-auto", streamlined ? "-right-4 overflow-x-hidden pr-4 md:-right-6 md:pr-6" : "right-0", diff --git a/ui/src/components/task-chat/expansion-state.tsx b/ui/src/components/task-chat/expansion-state.tsx new file mode 100644 index 0000000000..883513503c --- /dev/null +++ b/ui/src/components/task-chat/expansion-state.tsx @@ -0,0 +1,18 @@ +import { createContext, useCallback, useContext, useState, type Dispatch, type SetStateAction } from "react"; + +// A phase can move from the live tail into persisted history. Its expansion +// belongs to the logical phase, not whichever component currently hosts it. +export const TaskChatExpansionState = createContext | null>(null); + +export function useTaskChatExpansion(id: string, initial: boolean): [boolean, Dispatch>] { + const memory = useContext(TaskChatExpansionState); + const [open, setOpen] = useState(() => memory?.get(id) ?? initial); + const update = useCallback>>((value) => { + setOpen((previous) => { + const next = typeof value === "function" ? value(previous) : value; + memory?.set(id, next); + return next; + }); + }, [id, memory]); + return [open, update]; +} diff --git a/ui/src/components/task-chat/scroll-anchor.ts b/ui/src/components/task-chat/scroll-anchor.ts new file mode 100644 index 0000000000..978edecf4c --- /dev/null +++ b/ui/src/components/task-chat/scroll-anchor.ts @@ -0,0 +1,26 @@ +/** Keep a reading position by logical row identity, not total scroll height. */ +export interface ThreadScrollAnchor { + id: string; + top: number; +} + +export function readThreadScrollAnchor(root: Element, viewportTop: number, viewportBottom: number): ThreadScrollAnchor | null { + for (const row of root.querySelectorAll("[data-thread-anchor]")) { + const rect = row.getBoundingClientRect(); + if (rect.height > 0 && rect.bottom > viewportTop && rect.top < viewportBottom) { + return { id: row.dataset.threadAnchor!, top: rect.top - viewportTop }; + } + } + return null; +} + +export function threadScrollAnchorDelta(root: Element, anchor: ThreadScrollAnchor | null, viewportTop: number): number { + if (!anchor) return 0; + // IDs are opaque and need not be safe CSS selectors. + for (const row of root.querySelectorAll("[data-thread-anchor]")) { + if (row.dataset.threadAnchor === anchor.id) { + return row.getBoundingClientRect().top - viewportTop - anchor.top; + } + } + return 0; +} diff --git a/ui/src/components/task-chat/scroll-navigation.tsx b/ui/src/components/task-chat/scroll-navigation.tsx new file mode 100644 index 0000000000..a72ca32093 --- /dev/null +++ b/ui/src/components/task-chat/scroll-navigation.tsx @@ -0,0 +1,44 @@ +import { type ThreadScrollAnchor, threadScrollAnchorDelta } from "./scroll-anchor"; +import { createContext, useContext } from "react"; + +export const TaskChatScrollNavigation = createContext<{ key: string; restore: boolean; hash: string } | null>(null); +export const TaskChatScrollReady = createContext(true); + +// Scoped to browser-history entries, not issues: opening the same task from a +// new Inbox click starts at latest, while Back restores the previous reading. +const positions = new Map(); + +export function useTaskChatScrollNavigation() { + const navigation = useContext(TaskChatScrollNavigation); + const ready = useContext(TaskChatScrollReady); + // Native hash links can reuse React Router's history key. Keep those entries + // separate so their POP event does not restore the previous hash's position. + const positionKey = navigation ? JSON.stringify([navigation.key, navigation.hash]) : null; + return { + key: navigation?.key, + hash: navigation?.hash, + ready, + initialPosition(root: Element, viewportTop: number, scrollTop: number): number | null { + if (!navigation) return null; + if (navigation.restore && positionKey && positions.has(positionKey)) { + const saved = positions.get(positionKey)!; + if (saved.anchor && [...root.querySelectorAll("[data-thread-anchor]")].some((row) => row.dataset.threadAnchor === saved.anchor?.id)) { + return scrollTop + threadScrollAnchorDelta(root, saved.anchor, viewportTop); + } + return saved.top; + } + if (navigation.hash) { + let id: string; + try { id = decodeURIComponent(navigation.hash.slice(1)); } catch { return null; } + const target = document.getElementById(id); + if (target && root.contains(target)) return scrollTop + target.getBoundingClientRect().top - viewportTop; + } + return null; + }, + remember(top: number, anchor: ThreadScrollAnchor | null) { + if (!positionKey || !ready) return; + positions.set(positionKey, { top, anchor }); + if (positions.size > 100) positions.delete(positions.keys().next().value!); + }, + }; +} diff --git a/ui/src/components/task-chat/task-chat-adapter.ts b/ui/src/components/task-chat/task-chat-adapter.ts index a910dea511..07d1a99531 100644 --- a/ui/src/components/task-chat/task-chat-adapter.ts +++ b/ui/src/components/task-chat/task-chat-adapter.ts @@ -119,6 +119,7 @@ export function commentsToTaskChatItems( ?? null; items.push({ id: comment.id || comment.clientId || `${comment.createdAt}`, + renderKey: comment.clientId ?? comment.id, kind: "message", author: kind, authorName, diff --git a/ui/src/components/task-chat/task-chat-model.ts b/ui/src/components/task-chat/task-chat-model.ts index 48ac72a79a..a92adbf7aa 100644 --- a/ui/src/components/task-chat/task-chat-model.ts +++ b/ui/src/components/task-chat/task-chat-model.ts @@ -100,6 +100,8 @@ export interface TaskChatTokenUsage { /** A human/agent/system message bubble. */ export interface TaskChatMessageItem { + /** Stable UI identity through optimistic acknowledgement; id remains canonical. */ + renderKey?: string; id: string; kind: "message"; author: TaskChatAuthorKind; diff --git a/ui/src/components/task-chat/useWindowAutoFollow.test.tsx b/ui/src/components/task-chat/useWindowAutoFollow.test.tsx index 9d269015f1..ba43d080b9 100644 --- a/ui/src/components/task-chat/useWindowAutoFollow.test.tsx +++ b/ui/src/components/task-chat/useWindowAutoFollow.test.tsx @@ -4,6 +4,7 @@ import { flushSync } from "react-dom"; import { createRoot, type Root } from "react-dom/client"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { useWindowAutoFollow } from "./useWindowAutoFollow"; +import { TaskChatScrollNavigation } from "./scroll-navigation"; function Host({ contentKey, enabled }: { contentKey: unknown; enabled: boolean }) { useWindowAutoFollow(contentKey, enabled); @@ -81,6 +82,40 @@ describe("useWindowAutoFollow", () => { vi.unstubAllGlobals(); }); + it("reapplies same-task targets and POP positions on mobile without remounting", async () => { + vi.stubGlobal("scrollTo", (options: ScrollToOptions) => setWindowScrollY(Math.min(1200, options.top ?? 0))); + function MobileThread() { + useWindowAutoFollow("unchanged", true); + return
    {[100, 500].map((top, index) => ( +
    { + if (node) node.getBoundingClientRect = () => ({ top: top - window.scrollY, bottom: top + 100 - window.scrollY, height: 100 } as DOMRect); + }}>Comment {index}
    + ))}
    ; + } + const navigate = (key: string, hash: string, restore = false) => flushSync(() => root.render( + , + )); + navigate("mobile-entry-one", "#mobile-comment-0"); + const thread = container.firstElementChild; + expect(window.scrollY).toBe(100); + await scrollWindowTo(150); + navigate("mobile-entry-two", "#mobile-comment-1"); + expect(container.firstElementChild).toBe(thread); + expect(window.scrollY).toBe(500); + navigate("mobile-entry-one", "#mobile-comment-0", true); + expect(window.scrollY).toBe(150); + navigate("mobile-entry-one", "#mobile-comment-1", true); + expect(window.scrollY).toBe(500); + }); + + it("owns browser restoration only while the mobile thread is enabled", () => { + window.history.scrollRestoration = "auto"; + render(0); + expect(window.history.scrollRestoration).toBe("manual"); + render(0, false); + expect(window.history.scrollRestoration).toBe("auto"); + }); + it("scrolls the window to the bottom on mount", () => { render(0); expect(scrollToCalls).toContain(2000); diff --git a/ui/src/components/task-chat/useWindowAutoFollow.ts b/ui/src/components/task-chat/useWindowAutoFollow.ts index 58f6c3c5da..df17a1dcfc 100644 --- a/ui/src/components/task-chat/useWindowAutoFollow.ts +++ b/ui/src/components/task-chat/useWindowAutoFollow.ts @@ -1,4 +1,6 @@ +import { readThreadScrollAnchor, threadScrollAnchorDelta, type ThreadScrollAnchor } from "./scroll-anchor"; import { useEffect, useLayoutEffect, useRef } from "react"; +import { useTaskChatScrollNavigation } from "./scroll-navigation"; const PIN_THRESHOLD_PX = 48; @@ -25,23 +27,41 @@ function scrollWindowToBottom(): void { * bottom (within a small threshold) content growth follows with INSTANT * scroll; once the user scrolls up we hold their position. * - * The initial follow runs in a passive effect plus one rAF: Layout's - * navigation scroll handling (reset-to-top on PUSH, scroll-memory re-apply on - * POP) runs in ancestor layout effects, which fire AFTER this component's - * layout effects in the same commit — deferring past them keeps the reset - * from clobbering the follow. + * The conversation enables this hook after navigation has settled and before + * its coordinated reveal, so initial positioning happens before paint. */ export function useWindowAutoFollow(contentKey: unknown, enabled: boolean): void { const pinnedRef = useRef(true); + const navigation = useTaskChatScrollNavigation(); + const initialPositionApplied = useRef(false); + const appliedNavigation = useRef({ key: navigation.key, hash: navigation.hash }); + const anchorRef = useRef(null); + const rememberAnchor = () => { + const root = document.querySelector('[data-testid="task-chat-thread"]'); + if (root) anchorRef.current = readThreadScrollAnchor(root, 0, window.innerHeight); + if (initialPositionApplied.current) navigation.remember(window.scrollY, anchorRef.current); + }; + const reconcile = () => { + if (pinnedRef.current) scrollWindowToBottom(); + else { + const root = document.querySelector('[data-testid="task-chat-thread"]'); + if (root) { + const delta = threadScrollAnchorDelta(root, anchorRef.current, 0); + if (delta) window.scrollTo({ top: window.scrollY + delta, behavior: "auto" }); + } + } + rememberAnchor(); + }; useEffect(() => { if (!enabled) return; const onScroll = () => { pinnedRef.current = windowPinned(); + rememberAnchor(); }; window.addEventListener("scroll", onScroll, { passive: true }); return () => window.removeEventListener("scroll", onScroll); - }, [enabled]); + }, [enabled, navigation.key, navigation.hash, navigation.ready]); useLayoutEffect(() => { if (!enabled || typeof ResizeObserver === "undefined") return; @@ -56,27 +76,48 @@ export function useWindowAutoFollow(contentKey: unknown, enabled: boolean): void return; } previousScrollHeight = nextScrollHeight; - if (pinnedRef.current) scrollWindowToBottom(); + reconcile(); }); observer.observe(observed); return () => observer.disconnect(); - }, [enabled]); + }, [enabled, navigation.key, navigation.hash, navigation.ready]); // Follow new content only when already pinned; otherwise hold position. useLayoutEffect(() => { if (!enabled) return; - if (pinnedRef.current) scrollWindowToBottom(); - }, [contentKey, enabled]); + if (appliedNavigation.current.key !== navigation.key || appliedNavigation.current.hash !== navigation.hash) { + appliedNavigation.current = { key: navigation.key, hash: navigation.hash }; + initialPositionApplied.current = false; + } + if (navigation.ready && !initialPositionApplied.current) { + const root = document.querySelector('[data-testid="task-chat-thread"]'); + const top = root ? navigation.initialPosition(root, 0, window.scrollY) : null; + if (top !== null) { + window.scrollTo({ top, behavior: "auto" }); + pinnedRef.current = windowPinned(); + rememberAnchor(); + } + initialPositionApplied.current = true; + } + reconcile(); + }, [contentKey, enabled, navigation.key, navigation.hash, navigation.ready]); - useEffect(() => { + useLayoutEffect(() => { if (!enabled) return; - scrollWindowToBottom(); - const raf = requestAnimationFrame(() => { - scrollWindowToBottom(); - pinnedRef.current = true; - }); - return () => cancelAnimationFrame(raf); - // Initial follow only — content-driven follow is the layout effect above. - // eslint-disable-next-line react-hooks/exhaustive-deps + // One owner for document-flow compensation, as on the desktop viewport. + const previousRestoration = window.history.scrollRestoration; + window.history.scrollRestoration = "manual"; + document.documentElement.classList.add("task-chat-window-scroll"); + return () => { + document.documentElement.classList.remove("task-chat-window-scroll"); + window.history.scrollRestoration = previousRestoration; + }; }, [enabled]); } + +/** Mount below TaskChatScrollReady so mobile navigation also waits for targets + * fetched after the conversation's first reveal. */ +export function TaskChatWindowScroll({ contentKey, enabled }: { contentKey: unknown; enabled: boolean }) { + useWindowAutoFollow(contentKey, enabled); + return null; +} diff --git a/ui/src/components/transcript/read-transcript-request.ts b/ui/src/components/transcript/read-transcript-request.ts new file mode 100644 index 0000000000..eb695afc3e --- /dev/null +++ b/ui/src/components/transcript/read-transcript-request.ts @@ -0,0 +1,35 @@ +export const TRANSCRIPT_REQUEST_TIMEOUT_MS = 15_000; + +/** Bound history reads, including body consumption, and release coalesced GETs + * on timeout so Retry starts a fresh request. Late responses cannot commit. */ +export function readTranscriptRequest(request: (signal: AbortSignal) => Promise, signal: AbortSignal): Promise { + if (signal.aborted) return Promise.reject(new DOMException("History read cancelled", "AbortError")); + return new Promise((resolve, reject) => { + const controller = new AbortController(); + let settled = false; + const finish = (complete: () => void) => { + if (settled) return; + settled = true; + window.clearTimeout(timer); + signal.removeEventListener("abort", onAbort); + complete(); + }; + const onAbort = () => { + finish(() => reject(new DOMException("History read cancelled", "AbortError"))); + controller.abort(); + }; + const timer = window.setTimeout(() => { + finish(() => reject(new Error("Run history took too long to load. Retry to load it."))); + controller.abort(); + }, TRANSCRIPT_REQUEST_TIMEOUT_MS); + signal.addEventListener("abort", onAbort, { once: true }); + try { + request(controller.signal).then( + (value) => finish(() => resolve(value)), + (error) => finish(() => reject(error)), + ); + } catch (error) { + finish(() => reject(error)); + } + }); +} diff --git a/ui/src/components/transcript/useLiveRunTranscripts.test.tsx b/ui/src/components/transcript/useLiveRunTranscripts.test.tsx index eac7a5a01b..e78b8077d8 100644 --- a/ui/src/components/transcript/useLiveRunTranscripts.test.tsx +++ b/ui/src/components/transcript/useLiveRunTranscripts.test.tsx @@ -5,6 +5,7 @@ import { createRoot } from "react-dom/client"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { ApiError } from "../../api/client"; import { useLiveRunTranscripts } from "./useLiveRunTranscripts"; +import { TRANSCRIPT_REQUEST_TIMEOUT_MS } from "./read-transcript-request"; const { useQueryMock, logMock, buildTranscriptMock } = vi.hoisted(() => ({ useQueryMock: vi.fn(() => ({ data: { censorUsernameInLogs: false } })), @@ -198,6 +199,36 @@ describe("useLiveRunTranscripts", () => { container.remove(); }); + it("releases stalled log hydration and permits a fresh retry without accepting late data", async () => { + vi.useFakeTimers(); + const container = document.createElement("div"); + const root = createRoot(container); + let latest!: ReturnType; + let resolveLate!: (result: Awaited>) => void; + const empty = { runId: "run-1", store: "memory", logRef: "log-1", content: "", nextOffset: 0 }; + logMock.mockImplementationOnce(() => new Promise((resolve) => { resolveLate = resolve; })); + function Harness() { + latest = useLiveRunTranscripts({ companyId: "company-1", runs: [{ id: "run-1", status: "succeeded", adapterType: "codex_local" }] }); + return null; + } + try { + await act(async () => { root.render(); }); + expect(latest.isInitialHydrating).toBe(true); + await act(async () => { await vi.advanceTimersByTimeAsync(TRANSCRIPT_REQUEST_TIMEOUT_MS); }); + expect(latest.isInitialHydrating).toBe(false); + expect(latest.errorsByRun.get("run-1")?.message).toContain("too long"); + await act(async () => { resolveLate(empty); }); + expect(latest.errorsByRun.has("run-1")).toBe(true); + logMock.mockResolvedValue(empty); + await act(async () => { latest.retry(); }); + expect(latest.errorsByRun.size).toBe(0); + expect(logMock).toHaveBeenCalledTimes(2); + } finally { + act(() => root.unmount()); + vi.useRealTimers(); + } + }); + it("stops retrying terminal runs whose persisted log never existed", async () => { logMock.mockReset(); logMock.mockRejectedValue(new ApiError("Run log not found", 404, { error: "Run log not found" })); @@ -261,7 +292,7 @@ describe("useLiveRunTranscripts", () => { }); expect(logMock).toHaveBeenCalledTimes(1); - expect(logMock).toHaveBeenCalledWith("run-queued", 0, 256_000); + expect(logMock).toHaveBeenCalledWith("run-queued", 0, 256_000, expect.objectContaining({ signal: expect.any(AbortSignal) })); expect(FakeWebSocket.instances).toHaveLength(1); act(() => { @@ -291,7 +322,7 @@ describe("useLiveRunTranscripts", () => { }); expect(FakeWebSocket.instances).toHaveLength(0); - expect(logMock).toHaveBeenCalledWith("run-1", 0, 64_000); + expect(logMock).toHaveBeenCalledWith("run-1", 0, 64_000, expect.objectContaining({ signal: expect.any(AbortSignal) })); act(() => { root.unmount(); @@ -319,7 +350,7 @@ describe("useLiveRunTranscripts", () => { await Promise.resolve(); }); - expect(logMock).toHaveBeenCalledWith("run-1", 36_000, 64_000); + expect(logMock).toHaveBeenCalledWith("run-1", 36_000, 64_000, expect.objectContaining({ signal: expect.any(AbortSignal) })); act(() => { root.unmount(); diff --git a/ui/src/components/transcript/useLiveRunTranscripts.ts b/ui/src/components/transcript/useLiveRunTranscripts.ts index a092d445e9..a7fcd6bb8f 100644 --- a/ui/src/components/transcript/useLiveRunTranscripts.ts +++ b/ui/src/components/transcript/useLiveRunTranscripts.ts @@ -1,4 +1,5 @@ -import { useEffect, useMemo, useRef, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { readTranscriptRequest } from "./read-transcript-request"; import { useQuery } from "@tanstack/react-query"; import type { LiveEvent } from "@paperclipai/shared"; import { ApiError } from "../../api/client"; @@ -123,6 +124,12 @@ export function useLiveRunTranscripts({ const normalizedRuns = useMemo(() => runs.map((run) => ({ ...run })), [runsKey]); const [chunksByRun, setChunksByRun] = useState>(new Map()); const [hydratedRunIds, setHydratedRunIds] = useState>(new Set()); + const [errorsByRun, setErrorsByRun] = useState>(new Map()); + const [retryGeneration, setRetryGeneration] = useState(0); + const retry = useCallback(() => { + missingTerminalLogRunIdsRef.current.clear(); + setRetryGeneration((value) => value + 1); + }, []); const seenChunkKeysRef = useRef(new Set()); // Highest sequenced chunk trimmed out of a run's retained window; older // records re-delivered by the other transport are dropped instead of being @@ -242,6 +249,11 @@ export function useLiveRunTranscripts({ return next.size === prev.size ? prev : next; }); + setErrorsByRun((previous) => { + const next = new Map([...previous].filter(([id]) => retainedRunIds.has(id))); + return next.size === previous.size ? previous : next; + }); + for (const key of pendingLogRowsByRunRef.current.keys()) { const runId = key.replace(/:records$/, ""); if (!retainedRunIds.has(runId)) { @@ -285,16 +297,28 @@ export function useLiveRunTranscripts({ if (readableRuns.length === 0) return; let cancelled = false; + const controller = new AbortController(); + const inFlightRunIds = new Set(); const readRunLog = async (run: RunTranscriptSource) => { - if (missingTerminalLogRunIdsRef.current.has(run.id)) { + if (missingTerminalLogRunIdsRef.current.has(run.id) || inFlightRunIds.has(run.id)) { return; } + inFlightRunIds.add(run.id); const offset = logOffsetByRunRef.current.get(run.id) ?? resolveInitialLogOffset(run, logReadLimitBytes); try { - const result = await heartbeatsApi.log(run.id, offset, logReadLimitBytes); + const result = await readTranscriptRequest( + (signal) => heartbeatsApi.log(run.id, offset, logReadLimitBytes, { signal }), + controller.signal, + ); if (cancelled) return; + setErrorsByRun((previous) => { + if (!previous.has(run.id)) return previous; + const next = new Map(previous); + next.delete(run.id); + return next; + }); appendChunks(run.id, parsePersistedLogContent(run.id, result.content, pendingLogRowsByRunRef.current)); if (result.nextOffset !== undefined) { @@ -305,10 +329,26 @@ export function useLiveRunTranscripts({ logOffsetByRunRef.current.set(run.id, offset + result.content.length); } } catch (error) { - if (error instanceof ApiError && error.status === 404 && isTerminalStatus(run.status)) { - missingTerminalLogRunIdsRef.current.add(run.id); + if (cancelled) return; + if (error instanceof ApiError && error.status === 404) { + setErrorsByRun((previous) => { + if (!previous.has(run.id)) return previous; + const next = new Map(previous); + next.delete(run.id); + return next; + }); + // A newly started run may not have created its log yet. + if (isTerminalStatus(run.status)) missingTerminalLogRunIdsRef.current.add(run.id); + } else { + setErrorsByRun((previous) => { + if (previous.has(run.id)) return previous; + const next = new Map(previous); + next.set(run.id, error instanceof Error ? error : new Error("Run history could not be loaded")); + return next; + }); } } finally { + inFlightRunIds.delete(run.id); if (!cancelled) { setHydratedRunIds((prev) => { if (prev.has(run.id)) return prev; @@ -340,9 +380,10 @@ export function useLiveRunTranscripts({ return () => { cancelled = true; + controller.abort(); if (interval !== null) window.clearInterval(interval); }; - }, [enableRealtimeUpdates, logPollIntervalMs, logReadLimitBytes, normalizedRuns, runIdsKey]); + }, [enableRealtimeUpdates, logPollIntervalMs, logReadLimitBytes, normalizedRuns, runIdsKey, retryGeneration]); useEffect(() => { if (!enableRealtimeUpdates) return; @@ -519,6 +560,7 @@ export function useLiveRunTranscripts({ return { transcriptByRun, + hydratedRunIds, errorsByRun, retry, isInitialHydrating: normalizedRuns.some((run) => canReadPersistedLog(run) && !hydratedRunIds.has(run.id)), hasOutputForRun(runId: string) { return (chunksByRun.get(runId)?.length ?? 0) > 0 || runById.get(runId)?.hasStoredOutput === true; diff --git a/ui/src/components/transcript/useNativeRunTranscripts.test.tsx b/ui/src/components/transcript/useNativeRunTranscripts.test.tsx index 347cbd9bd6..4666e242cf 100644 --- a/ui/src/components/transcript/useNativeRunTranscripts.test.tsx +++ b/ui/src/components/transcript/useNativeRunTranscripts.test.tsx @@ -4,6 +4,7 @@ import { act } from "react"; import { createRoot, type Root } from "react-dom/client"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { useNativeRunTranscripts } from "./useNativeRunTranscripts"; +import { TRANSCRIPT_REQUEST_TIMEOUT_MS } from "./read-transcript-request"; const eventsMock = vi.hoisted(() => vi.fn()); @@ -86,3 +87,83 @@ describe("useNativeRunTranscripts", () => { expect(eventsMock.mock.calls.at(-1)?.[0]).toBe("failed-run"); }); }); + +// Initial readiness is separate from an empty transcript: the task shell must +// not reveal empty history while the first durable page is still in flight. +describe("native history readiness and stable projection", () => { + let container: HTMLDivElement; + let root: Root; + let latest: ReturnType; + function StateProbe({ runs = [{ id: "one", status: "running", runtimeMode: "native" as const }] }) { + latest = useNativeRunTranscripts(runs); + return null; + } + beforeEach(() => { + vi.useFakeTimers(); + eventsMock.mockReset(); + container = document.createElement("div"); + root = createRoot(container); + }); + afterEach(() => { + act(() => root.unmount()); + vi.useRealTimers(); + }); + + it("distinguishes pending, loaded-empty and failed, and supports explicit retry", async () => { + let resolve!: (rows: never[]) => void; + eventsMock.mockImplementationOnce(() => new Promise((done) => { resolve = done; })); + await act(async () => { root.render(); }); + expect(latest.isInitialHydrating).toBe(true); + await act(async () => { resolve([]); }); + expect(latest.isInitialHydrating).toBe(false); + expect(latest.hydratedRunIds.has("one")).toBe(true); + eventsMock.mockRejectedValueOnce(new Error("offline")); + await act(async () => { await vi.advanceTimersByTimeAsync(2_000); }); + expect(latest.errorsByRun.has("one")).toBe(true); + expect(latest.isInitialHydrating).toBe(false); + eventsMock.mockResolvedValue([]); + await act(async () => { latest.retry(); }); + expect(latest.errorsByRun.size).toBe(0); + }); + + it("does not rebuild unchanged history on empty polls", async () => { + eventsMock.mockResolvedValueOnce([{ seq: 1, payload: {}, eventType: "log" }]).mockResolvedValue([]); + await act(async () => { root.render(); }); + const projection = latest.transcriptByRun; + const rows = projection.get("one"); + await act(async () => { await vi.advanceTimersByTimeAsync(2_000); }); + expect(latest.transcriptByRun).toBe(projection); + expect(latest.transcriptByRun.get("one")).toBe(rows); + }); + + it("times out stalled history, aborts its request, and ignores a late response before retry", async () => { + let resolveLate!: (rows: never[]) => void; + eventsMock.mockImplementationOnce(() => new Promise((resolve) => { resolveLate = resolve; })); + await act(async () => { root.render(); }); + const signal = eventsMock.mock.calls[0][3].signal as AbortSignal; + expect(latest.isInitialHydrating).toBe(true); + await act(async () => { await vi.advanceTimersByTimeAsync(TRANSCRIPT_REQUEST_TIMEOUT_MS); }); + expect(signal.aborted).toBe(true); + expect(latest.isInitialHydrating).toBe(false); + expect(latest.errorsByRun.get("one")?.message).toContain("too long"); + await act(async () => { resolveLate([]); }); + expect(latest.errorsByRun.has("one")).toBe(true); + eventsMock.mockResolvedValue([]); + await act(async () => { latest.retry(); }); + expect(latest.errorsByRun.size).toBe(0); + expect(eventsMock.mock.calls[1][3].signal.aborted).toBe(false); + }); + + it("commits a resolved run independently of a stalled sibling and retains its cursor", async () => { + const rows = [{ seq: 7, payload: {}, eventType: "log" }]; + eventsMock.mockImplementation((id) => id === "slow" ? new Promise(() => {}) : Promise.resolve(rows)); + const one = { id: "one", status: "succeeded", runtimeMode: "native" as const }; + await act(async () => { root.render(); }); + expect(latest.hydratedRunIds.has("one")).toBe(true); + expect(latest.hydratedRunIds.has("slow")).toBe(false); + expect(latest.transcriptByRun.has("one")).toBe(true); + await act(async () => { root.render(); }); + expect(eventsMock.mock.calls.filter(([id]) => id === "one").map(([, cursor]) => cursor)).toEqual([0, 7]); + expect(latest.isInitialHydrating).toBe(false); + }); +}); diff --git a/ui/src/components/transcript/useNativeRunTranscripts.ts b/ui/src/components/transcript/useNativeRunTranscripts.ts index 0f1247131c..a8dd3553e5 100644 --- a/ui/src/components/transcript/useNativeRunTranscripts.ts +++ b/ui/src/components/transcript/useNativeRunTranscripts.ts @@ -1,8 +1,9 @@ -import { useEffect, useMemo, useRef, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import type { HeartbeatRunEvent } from "@paperclipai/shared"; import type { TranscriptEntry } from "@/adapters"; import { heartbeatsApi } from "@/api/heartbeats"; import { nativeRunEventsToTranscript } from "./native-run-events"; +import { readTranscriptRequest } from "./read-transcript-request"; const EVENT_PAGE_SIZE = 1_000; const EVENT_POLL_INTERVAL_MS = 2_000; @@ -36,90 +37,116 @@ export function useNativeRunTranscripts(runs: readonly NativeRunTranscriptSource ); const [eventsByRun, setEventsByRun] = useState>(new Map()); const [errorsByRun, setErrorsByRun] = useState>(new Map()); + const [hydratedRunIds, setHydratedRunIds] = useState>(new Set()); + const [retryGeneration, setRetryGeneration] = useState(0); + const retry = useCallback(() => setRetryGeneration((value) => value + 1), []); + const projectionCacheRef = useRef(new Map()); const cursorByRunRef = useRef(new Map()); useEffect(() => { let cancelled = false; - let timer: number | null = null; + const controller = new AbortController(); + const timers = new Set(); + const retainedIds = new Set(nativeRuns.map((run) => run.id)); + const retainMap = (previous: Map) => { + const next = new Map([...previous].filter(([id]) => retainedIds.has(id))); + return next.size === previous.size ? previous : next; + }; + setEventsByRun(retainMap); + setErrorsByRun(retainMap); + setHydratedRunIds((previous) => { + const next = new Set([...previous].filter((id) => retainedIds.has(id))); + return next.size === previous.size ? previous : next; + }); + for (const id of cursorByRunRef.current.keys()) { + if (!retainedIds.has(id)) cursorByRunRef.current.delete(id); + } - const refresh = async (runsToRefresh: readonly NativeRunTranscriptSource[]) => { - const updates = new Map(); - const errors = new Map(); - await Promise.all(runsToRefresh.map(async (run) => { - try { - let cursor = cursorByRunRef.current.get(run.id) ?? 0; - const incoming: HeartbeatRunEvent[] = []; - for (;;) { - const page = await heartbeatsApi.events(run.id, cursor, EVENT_PAGE_SIZE); - if (cancelled) return; - const last = page.at(-1); - const nextCursor = last ? Math.max(cursor, last.seq) : cursor; - incoming.push(...page); - if (page.length < EVENT_PAGE_SIZE || nextCursor === cursor) { - cursor = nextCursor; - break; - } + const refreshRun = async (run: NativeRunTranscriptSource) => { + let failed = false; + try { + let cursor = cursorByRunRef.current.get(run.id) ?? 0; + const incoming: HeartbeatRunEvent[] = []; + for (;;) { + const page = await readTranscriptRequest( + (signal) => heartbeatsApi.events(run.id, cursor, EVENT_PAGE_SIZE, { signal }), + controller.signal, + ); + if (cancelled) return; + const last = page.at(-1); + const nextCursor = last ? Math.max(cursor, last.seq) : cursor; + incoming.push(...page.filter((event) => event.seq > cursor)); + if (page.length < EVENT_PAGE_SIZE || nextCursor === cursor) { cursor = nextCursor; + break; } - if (incoming.length > 0) updates.set(run.id, incoming); - cursorByRunRef.current.set(run.id, cursor); - } catch (error) { - // Keep the last durable cursor; the next poll retries this run only. - errors.set(run.id, { + cursor = nextCursor; + } + // Commit this run's cursor with its rows. A slow sibling must neither + // hold its readiness hostage nor stall live polling for this run. + cursorByRunRef.current.set(run.id, cursor); + if (incoming.length > 0) setEventsByRun((previous) => { + const next = new Map(previous); + next.set(run.id, [...(previous.get(run.id) ?? []), ...incoming]); + return next; + }); + setErrorsByRun((previous) => { + if (!previous.has(run.id)) return previous; + const next = new Map(previous); + next.delete(run.id); + return next; + }); + } catch (error) { + if (cancelled) return; + failed = true; + setErrorsByRun((previous) => { + if (previous.has(run.id)) return previous; + const next = new Map(previous); + next.set(run.id, { message: error instanceof Error ? error.message : "Native run activity could not be loaded", failedAt: new Date().toISOString(), }); - } - })); - - if (cancelled) return; - const retainedIds = new Set(nativeRuns.map((run) => run.id)); - for (const runId of cursorByRunRef.current.keys()) { - if (!retainedIds.has(runId)) cursorByRunRef.current.delete(runId); + return next; + }); } - setEventsByRun((previous) => { - const next = new Map(); - for (const runId of retainedIds) { - const current = previous.get(runId) ?? []; - const incoming = updates.get(runId) ?? []; - next.set(runId, incoming.length > 0 ? [...current, ...incoming] : current); - } - return next; - }); - setErrorsByRun((previous) => { - const next = new Map(); - for (const runId of retainedIds) { - const error = errors.get(runId); - if (error) next.set(runId, previous.get(runId) ?? error); - } - return next; - }); - - if (nativeRuns.some((run) => isLive(run.status)) || errors.size > 0) { - const retryRuns = nativeRuns.filter( - (run) => isLive(run.status) || errors.has(run.id), - ); - timer = window.setTimeout( - () => void refresh(retryRuns), - EVENT_POLL_INTERVAL_MS, - ); + if (cancelled) return; + setHydratedRunIds((previous) => previous.has(run.id) ? previous : new Set([...previous, run.id])); + if (isLive(run.status) || failed) { + const timer = window.setTimeout(() => { + timers.delete(timer); + void refreshRun(run); + }, EVENT_POLL_INTERVAL_MS); + timers.add(timer); } }; - - void refresh(nativeRuns); + for (const run of nativeRuns) void refreshRun(run); return () => { cancelled = true; - if (timer !== null) window.clearTimeout(timer); + controller.abort(); + for (const timer of timers) window.clearTimeout(timer); }; - }, [nativeRuns]); + }, [nativeRuns, retryGeneration]); const transcriptByRun = useMemo(() => { const transcripts = new Map(); for (const run of nativeRuns) { - transcripts.set(run.id, nativeRunEventsToTranscript(eventsByRun.get(run.id) ?? [])); + const events = eventsByRun.get(run.id); + if (!events) continue; + let cached = projectionCacheRef.current.get(run.id); + if (!cached || cached.events !== events) { + cached = { events, transcript: nativeRunEventsToTranscript(events) }; + projectionCacheRef.current.set(run.id, cached); + } + transcripts.set(run.id, cached.transcript); + } + for (const id of projectionCacheRef.current.keys()) { + if (!transcripts.has(id)) projectionCacheRef.current.delete(id); } return transcripts; }, [eventsByRun, nativeRuns]); - return { transcriptByRun, errorsByRun }; + return { + transcriptByRun, errorsByRun, hydratedRunIds, retry, + isInitialHydrating: nativeRuns.some((run) => !hydratedRunIds.has(run.id)), + }; } diff --git a/ui/src/index.css b/ui/src/index.css index 58eaaed0d5..fc8a2e3017 100644 --- a/ui/src/index.css +++ b/ui/src/index.css @@ -2836,3 +2836,18 @@ span.paperclip-mention-chip[data-mention-kind="external-object"] { > div:has([role="switch"]) { @apply sm:col-span-2; } + +/* Task feeds own their row anchor and follow position. Browser anchoring must + not race the same correction after a prepend or transcript reconciliation. */ +.task-chat-scroll-viewport { + overflow-anchor: none; + scrollbar-gutter: stable; +} + +.task-chat-window-scroll { + overflow-anchor: none; +} + +.task-chat-loading-shell .animate-pulse { + animation: none; +} diff --git a/ui/src/lib/optimistic-issue-comments.test.ts b/ui/src/lib/optimistic-issue-comments.test.ts index 94e8b62f99..a7730faed4 100644 --- a/ui/src/lib/optimistic-issue-comments.test.ts +++ b/ui/src/lib/optimistic-issue-comments.test.ts @@ -127,6 +127,7 @@ describe("optimistic issue comments", () => { ); expect(merged.map((comment) => comment.id)).toEqual(["comment-1"]); + expect(merged[0]).toMatchObject({ clientId: optimistic.clientId }); }); it("reconciles repeated identical comments one-for-one", () => { diff --git a/ui/src/lib/optimistic-issue-comments.ts b/ui/src/lib/optimistic-issue-comments.ts index 1c76a400df..9acfb5a49e 100644 --- a/ui/src/lib/optimistic-issue-comments.ts +++ b/ui/src/lib/optimistic-issue-comments.ts @@ -153,6 +153,8 @@ export function mergeIssueComments( ); if (matchingPersistedComment) { reconciledPersistedIds.add(matchingPersistedComment.id); + const acknowledged = { ...matchingPersistedComment, clientId: comment.clientId }; + merged[merged.findIndex((entry) => entry.id === matchingPersistedComment.id)] = acknowledged; continue; } diff --git a/ui/src/pages/IssueDetail.tsx b/ui/src/pages/IssueDetail.tsx index 7ff63b7518..91ada40769 100644 --- a/ui/src/pages/IssueDetail.tsx +++ b/ui/src/pages/IssueDetail.tsx @@ -1,3 +1,4 @@ +import { TaskChatScrollNavigation } from "@/components/task-chat/scroll-navigation"; import { memo, useCallback, @@ -1004,7 +1005,7 @@ function IssueDetailLoadingState({
    @@ -1086,7 +1087,7 @@ function IssueDetailLoadingState({ // Chat shell: the thread is the whole surface — alternating bubble // placeholders followed by the docked composer, no tab strip or // properties-card chrome (those don't exist in the chat layout). -
    +
    @@ -1250,6 +1251,9 @@ type IssueDetailChatTabProps = { } | null; comments: IssueDetailComment[]; commentsInitialLoading?: boolean; + initialHistoryPending?: boolean; + initialHistoryError?: boolean; + onRetryInitialHistory?: () => void; locallyQueuedCommentRunIds: ReadonlyMap; interactions: IssueThreadInteraction[]; documents: IssueDocumentSummary[]; @@ -1377,6 +1381,9 @@ const IssueDetailChatTab = memo(function IssueDetailChatTab({ legacyRecoverySourceIssue, comments, commentsInitialLoading = false, + initialHistoryPending = false, + initialHistoryError = false, + onRetryInitialHistory, locallyQueuedCommentRunIds, interactions, documents, @@ -1443,13 +1450,15 @@ const IssueDetailChatTab = memo(function IssueDetailChatTab({ ? IssueChatThread : TaskChatThread; const queryClient = useQueryClient(); + const scrollLocation = useLocation(); + const scrollNavigationType = useNavigationType(); const { pushToast } = useToastActions(); - const { data: activity } = useQuery({ + const { data: activity, isPending: activityPending, isError: activityError, refetch: refetchActivity } = useQuery({ queryKey: queryKeys.issues.activity(issueId), queryFn: () => activityApi.forIssue(issueId), placeholderData: keepPreviousDataForSameQueryTail(issueId), }); - const { data: liveRuns, isFetched: liveRunsFetched } = useQuery({ + const { data: liveRuns, isFetched: liveRunsFetched, isError: liveRunsError, refetch: refetchLiveRuns } = useQuery({ queryKey: queryKeys.issues.liveRuns(issueId), queryFn: () => heartbeatsApi.liveRunsForIssue(issueId), refetchInterval: 1000, @@ -1460,7 +1469,7 @@ const IssueDetailChatTab = memo(function IssueDetailChatTab({ const liveRunCount = resolvedLiveRuns.length; const activeRunQueryEnabled = !!executionRunId || issueStatus === "in_progress"; - const { data: activeRun = null, isFetched: activeRunFetched } = useQuery({ + const { data: activeRun = null, isFetched: activeRunFetched, isError: activeRunError, refetch: refetchActiveRun } = useQuery({ queryKey: queryKeys.issues.activeRun(issueId), queryFn: () => heartbeatsApi.activeRunForIssue(issueId), enabled: activeRunQueryEnabled, @@ -1524,7 +1533,7 @@ const IssueDetailChatTab = memo(function IssueDetailChatTab({ setLocalSteeringPlacements(new Map()); }, [issueId]); const hasLiveRuns = liveRunCount > 0 || !!resolvedActiveRun; - const { data: linkedRuns } = useQuery({ + const { data: linkedRuns, isPending: linkedRunsPending, isError: linkedRunsError, refetch: refetchLinkedRuns } = useQuery({ queryKey: queryKeys.issues.runs(issueId), queryFn: () => activityApi.runsForIssue(issueId), refetchInterval: @@ -2266,26 +2275,21 @@ const IssueDetailChatTab = memo(function IssueDetailChatTab({ {/* Chat-style: the button rides inside the thread's scroll viewport with the header so nothing sits above the thread in the page flow. */} {classicTaskInterfaceEnabled ? loadOlderButton : null} - {commentsInitialLoading && - commentsWithRunMeta.length === 0 && - interactions.length === 0 ? ( - classicTaskInterfaceEnabled ? ( - - ) : ( - // Chat shell: center the bubbles at the thread cap (mirrors - // TaskChatThreadView) and dock a composer placeholder beneath them. -
    - - -
    - ) + {classicTaskInterfaceEnabled && commentsInitialLoading && commentsWithRunMeta.length === 0 && interactions.length === 0 ? ( + ) : ( + { + onRetryInitialHistory?.(); + void refetchActivity(); + void refetchLinkedRuns(); + void refetchLiveRuns(); + if (activeRunQueryEnabled) void refetchActiveRun(); + }} composerRef={composerRef} composerAccessory={composerAccessory} threadHeader={ @@ -2408,6 +2412,7 @@ const IssueDetailChatTab = memo(function IssueDetailChatTab({ externalReferences={externalReferences} linkCaseReferences={linkCaseReferences} /> + )}
    ); @@ -2857,6 +2862,7 @@ export function IssueDetail() { const lastScrollIssueIdRef = useRef(undefined); const commentComposerRef = useRef(null); const cancelledQueuedOptimisticCommentIdsRef = useRef(new Set()); + const commentRenderKeys = useRef(new Map()); const resolvedIssueDetailState = useMemo( () => readIssueDetailLocationState(issueId, location.state, location.search), @@ -2920,6 +2926,7 @@ export function IssueDetail() { const { data: commentPages, isLoading: commentsLoading, + isError: commentsError, isFetchingNextPage: commentsLoadingOlder, hasNextPage: hasOlderComments, fetchNextPage: fetchOlderComments, @@ -2969,6 +2976,8 @@ export function IssueDetail() { ISSUE_DETAIL_CONTENT_MEASURE, ); }, [commentsLoading, issue?.id]); + const linkedCommentId = location.hash.startsWith("#comment-") ? location.hash.slice("#comment-".length) : null; + const linkedCommentPending = Boolean(linkedCommentId && !comments.some((comment) => comment.id === linkedCommentId) && !commentsError && (commentsLoading || hasOlderComments)); const shouldPrefetchOlderComments = useMemo( () => shouldAutoloadOlderIssueComments({ @@ -2987,7 +2996,7 @@ export function IssueDetail() { hasOlderComments, ], ); - const { data: interactions = [] } = useQuery({ + const { data: interactions = [], isLoading: interactionsLoading, isError: interactionsError, refetch: refetchInteractions } = useQuery({ queryKey: queryKeys.issues.interactions(issueId!), queryFn: () => issuesApi.listInteractions(issueId!), enabled: !!issueId, @@ -2999,7 +3008,7 @@ export function IssueDetail() { ), }); - const { data: attachments, isLoading: attachmentsLoading } = useQuery({ + const { data: attachments, isLoading: attachmentsLoading, isError: attachmentsError, refetch: refetchAttachments } = useQuery({ queryKey: queryKeys.issues.attachments(issueId!), queryFn: () => issuesApi.listAttachments(issueId!), enabled: !!issueId, @@ -3008,10 +3017,14 @@ export function IssueDetail() { ), }); - const { data: workProducts } = useQuery({ + const { data: workProducts, isLoading: workProductsLoading, isError: workProductsError, refetch: refetchWorkProducts } = useQuery({ queryKey: queryKeys.issues.workProducts(issueId!), queryFn: () => - issuesApi.listWorkProducts(issueId!, { refreshPullRequests: true }), + issuesApi.listWorkProducts(issueId!, { + // Initial geometry needs stored artifacts, not a network round-trip to + // GitHub. Enrich PR status after the stored list has painted. + refreshPullRequests: queryClient.getQueryData(queryKeys.issues.workProducts(issueId!)) !== undefined, + }), enabled: !!issueId, refetchOnMount: "always", placeholderData: keepPreviousDataForSameQueryTail( @@ -3019,6 +3032,13 @@ export function IssueDetail() { ), }); + const enrichedWorkProductsIssue = useRef(null); + useEffect(() => { + if (!issueId || enrichedWorkProductsIssue.current === issueId || !workProducts?.some((product) => product.type === "pull_request")) return; + enrichedWorkProductsIssue.current = issueId; + void refetchWorkProducts(); + }, [issueId, workProducts, refetchWorkProducts]); + const { data: liveRunCount = 0 } = useQuery( { queryKey: queryKeys.issues.liveRuns(issueId!), @@ -3452,7 +3472,11 @@ export function IssueDetail() { ); const threadComments = useMemo( - () => mergeIssueComments(comments ?? [], optimisticComments), + () => mergeIssueComments(comments ?? [], optimisticComments).map((comment) => { + if ("clientId" in comment && comment.clientId) commentRenderKeys.current.set(comment.id, comment.clientId); + const clientId = commentRenderKeys.current.get(comment.id); + return clientId ? { ...comment, clientId } : comment; + }), [comments, optimisticComments], ); const breadcrumbTitle = issue?.title ?? issueId ?? "Task"; @@ -4365,6 +4389,9 @@ export function IssueDetail() { queryKey: queryKeys.issues.queuedComments(issueId!), }); } + if (context?.optimisticCommentId) { + commentRenderKeys.current.set(comment.id, context.optimisticCommentId); + } queryClient.setQueryData>( queryKeys.issues.comments(issueId!), (current) => @@ -4738,6 +4765,7 @@ export function IssueDetail() { }); } if (comment) { + if (context?.optimisticCommentId) commentRenderKeys.current.set(comment.id, context.optimisticCommentId); queryClient.setQueryData>( queryKeys.issues.comments(issueId!), (current) => @@ -5433,7 +5461,7 @@ export function IssueDetail() { [openIssueGallery], ); - useEffect(() => { + useLayoutEffect(() => { if (!panelIssue || suppressPanelUntilPlan) { closePanel(); return; @@ -5989,9 +6017,9 @@ export function IssueDetail() { }); }, [issueId, queryClient, refetchComments]); useEffect(() => { - if (!shouldPrefetchOlderComments) return; + if (!shouldPrefetchOlderComments && !(linkedCommentPending && hasOlderComments && !commentsLoadingOlder)) return; void fetchOlderComments(); - }, [fetchOlderComments, shouldPrefetchOlderComments]); + }, [fetchOlderComments, shouldPrefetchOlderComments, linkedCommentPending, hasOlderComments, commentsLoadingOlder]); const handleCommentVote = useCallback( async ( commentId: string, @@ -7751,6 +7779,14 @@ export function IssueDetail() { legacyRecoverySourceIssue={legacyRecoverySourceIssue} comments={threadComments} commentsInitialLoading={commentsLoading} + initialHistoryPending={linkedCommentPending || interactionsLoading || attachmentsLoading || workProductsLoading} + initialHistoryError={commentsError || interactionsError || attachmentsError || workProductsError} + onRetryInitialHistory={() => { + void refetchComments(); + void refetchInteractions(); + void refetchAttachments(); + void refetchWorkProducts(); + }} locallyQueuedCommentRunIds={locallyQueuedCommentRunIds} interactions={interactions} documents={issue.documentSummaries ?? []}