fix(web): keep review tab and scroll across agent turns

apps/web/src/components/DiffPanel.tsx

@@ -418,7 +418,8 @@ export default function DiffPanel({
     ? `${routeThreadRef.environmentId}:${routeThreadRef.threadId}`
     : null;
 
-  const selectedGitSource = branchDiffPreview.data?.sources.find(
+  const preview = branchDiffPreview.isSuccess ? branchDiffPreview.data : null;
+  const selectedGitSource = preview?.sources.find(
     (source) =>
       source.kind ===
       (selectedGitScope === "unstaged"
@@ -470,15 +471,16 @@ export default function DiffPanel({
   const refreshPreviewQuery = branchDiffPreview.refresh;
   const refreshDiffFromUserAction = refreshPreviewQuery;
 
-  const preview = branchDiffPreview.data;
   const gitDiff = selectedGitSource?.diff;
 
   const toolCallPreview =
     toolCallSelection && toolCallItem?.type === "file_change" ? toolCallDiff(toolCallItem) : null;
   const selectedPatch = toolCallSelection
     ? toolCallPreview?.patch
     : selectedTurn
-      ? activeCheckpointDiff.data?.diff
+      ? activeCheckpointDiff.error === null
+        ? activeCheckpointDiff.data?.diff
+        : undefined
       : gitDiff;
   const isSelectedPatchTruncated =
     toolCallPreview?.truncated === true ||
@@ -598,6 +600,7 @@ export default function DiffPanel({
     ...matchingBaseRefChoices.map(valueForBaseRefChoice),
   ];
   const {
+    family: filePatchFamily,
     isPending: areFilePatchesPending,
     fileStates,
     retry,
@@ -606,6 +609,7 @@ export default function DiffPanel({
     remainingFileCount,
     loadNextFiles,
   } = useReviewFilePatches({
+    reviewKey: collapseScopeKey,
     environmentId: activeThread?.environmentId,
     cwd: branchDiffPreview.data?.cwd,
     source: lazySource,
@@ -1450,10 +1454,11 @@ export default function DiffPanel({
                     projectId={activeThread?.projectId}
                     viewerRef={attachCodeView}
                     onScroll={codeViewScroll.onScroll}
-                    codeViewKey={codeViewMountKey}
+                    codeViewKey={`${codeViewMountKey}:${filePatchFamily}`}
                     className="h-full min-h-0 overflow-auto"
                     files={codeViewFiles}
                     pendingFiles={pendingFiles}
+                    isRefreshingDiff={isRefreshingDiff}
                     renderCodeViewFooter={renderLoadingBoundary}
                     sectionId={reviewSectionId}
                     sectionTitle={reviewSectionTitle}

apps/web/src/components/diffs/AnnotatableCodeView.browser.test.tsx

@@ -1,91 +1,60 @@
-import type { CodeViewDiffItem, FileDiffMetadata, SelectedLineRange } from "@pierre/diffs";
+import type { FileDiffMetadata, SelectedLineRange } from "@pierre/diffs";
 import { parsePatchFiles } from "@pierre/diffs/utils/parsePatchFiles";
-import { StrictMode, type ComponentProps } from "react";
+import { WorkerPoolManager } from "@pierre/diffs/worker";
+import { createRef, StrictMode } from "react";
 import { describe, expect, it, vi } from "vite-plus/test";
 
 import { act, render } from "../../../test/render";
 
 import { DraftId, useComposerDraftStore } from "~/composerDraftStore";
-import { buildFileDiffIdentityKey } from "~/lib/diffRendering";
+import { buildFileDiffContentVersion, buildFileDiffIdentityKey } from "~/lib/diffRendering";
 
-type ViewerProps = Omit<ComponentProps<typeof StyledDiffCodeView<unknown>>, "options"> & {
-  options: {
-    enableLineSelection: boolean;
-    enableGutterUtility: boolean;
-    onGutterUtilityClick: (
-      range: SelectedLineRange,
-      context: { item: CodeViewDiffItem<unknown> },
-    ) => void;
-  };
-};
-
-// Replace only the virtualized viewer; comment editing and submission use the real components.
-vi.mock("./StyledDiffCodeView", () => ({
-  StyledDiffCodeView: (props: ViewerProps) =>
-    props.items
-      ?.filter((item) => item.type === "diff")
-      .map((item) => (
-        <section key={item.id}>
-          <button
-            disabled={!props.options?.enableLineSelection || !props.options?.enableGutterUtility}
-            onClick={() => {
-              props.onSelectedLinesChange?.({ id: item.id, range });
-              props.options?.onGutterUtilityClick?.(range, { item });
-            }}
-          >
-            Comment on {item.fileDiff.name}
-          </button>
-          {props.selectedLines?.id === item.id ? (
-            <mark>Line {props.selectedLines.range.start}</mark>
-          ) : null}
-          {item.annotations?.map((annotation) => (
-            <div key={`${annotation.side}:${annotation.lineNumber}`}>
-              {props.renderAnnotation?.(annotation, item.fileDiff, item)}
-            </div>
-          ))}
-        </section>
-      )),
-}));
-
-import { AnnotatableCodeView } from "./AnnotatableCodeView";
-import type { StyledDiffCodeView } from "./StyledDiffCodeView";
+import { AnnotatableCodeView, type AnnotatableCodeViewHandle } from "./AnnotatableCodeView";
 
 const target = DraftId.make("diff-draft-test");
 const range: SelectedLineRange = { start: 1, end: 1, side: "additions" };
-let container: HTMLDivElement;
 
 function file(
   name: string,
-  fileVersion = 1,
   type: FileDiffMetadata["type"] = "change",
   prevName?: string,
+  contents = "new",
 ) {
   const fileDiff = parsePatchFiles(
-    `diff --git a/${name} b/${name}\n--- a/${name}\n+++ b/${name}\n@@ -1 +1 @@\n-old\n+new\n`,
-    { cacheKeyPrefix: name },
+    `diff --git a/${name} b/${name}\n--- a/${name}\n+++ b/${name}\n@@ -1 +1,${contents.split("\n").length} @@\n-old\n+${contents}\n`,
+    { cacheKeyPrefix: `${name}:${contents}` },
   )[0]!.files[0]!;
   fileDiff.type = type;
   if (prevName) fileDiff.prevName = prevName;
   return {
     fileDiff,
     filePath: name,
     fileKey: buildFileDiffIdentityKey(fileDiff),
-    fileVersion,
+    fileVersion: buildFileDiffContentVersion(fileDiff),
     collapsed: false,
   };
 }
 
-function view(
-  files: ReturnType<typeof file>[],
-  codeViewKey = "initial",
-  pendingFiles: FileDiffMetadata[] = [],
-) {
-  return (
+function setup() {
+  const reset = () => useComposerDraftStore.setState(useComposerDraftStore.getInitialState(), true);
+  reset();
+  const container = document.createElement("div");
+  Object.assign(container.style, { width: "800px", height: "400px", display: "grid" });
+  const viewerRef = createRef<AnnotatableCodeViewHandle>();
+  const view = (
+    files: ReturnType<typeof file>[],
+    codeViewKey = "initial",
+    pendingFiles: FileDiffMetadata[] = [],
+    isRefreshingDiff = false,
+  ) => (
     <StrictMode>
       <AnnotatableCodeView
+        viewerRef={viewerRef}
+        className="h-full min-h-0 overflow-auto"
         codeViewKey={codeViewKey}
         files={files}
         pendingFiles={pendingFiles}
+        isRefreshingDiff={isRefreshingDiff}
         sectionId="unstaged"
         sectionTitle="Uncommitted"
         composerDraftTarget={target}
@@ -96,68 +65,179 @@ function view(
       />
     </StrictMode>
   );
-}
-
-function click(label: string) {
-  const button = Array.from(container.querySelectorAll("button")).find(
-    (node) => node.textContent === label,
-  );
-  if (!button) throw new Error(`Missing button: ${label}`);
-  return act(() => button.click());
-}
-
-function editor() {
-  const textarea = container.querySelector("textarea");
-  if (!textarea) throw new Error("Comment editor is not available");
-  return textarea;
-}
-
-function typeComment(text: string) {
-  return act(() => {
-    Object.getOwnPropertyDescriptor(HTMLTextAreaElement.prototype, "value")!.set!.call(
-      editor(),
-      text,
+  const renderedFile = () => {
+    const item = viewerRef.current?.getInstance()?.getRenderedItems()[0];
+    if (item?.type !== "diff") throw new Error("Diff is not rendered");
+    return item;
+  };
+  const select = () =>
+    act(() => {
+      // Invoke Pierre's own wrappers, which resolve their context from the latest item.
+      const { instance } = renderedFile();
+      instance.options.onLineSelectionChange?.(range);
+      instance.options.onGutterUtilityClick?.(range);
+    });
+  const editor = () => {
+    const textarea = container.querySelector("textarea");
+    if (!textarea) throw new Error("Comment editor is not available");
+    return textarea;
+  };
+  const click = (label: string) => {
+    const button = Array.from(container.querySelectorAll("button")).find(
+      (node) => node.textContent === label,
     );
-    editor().dispatchEvent(new Event("input", { bubbles: true }));
-  });
-}
-
-function setup(): Disposable {
-  const reset = () => useComposerDraftStore.setState(useComposerDraftStore.getInitialState(), true);
-  reset();
-  container = document.createElement("div");
-  return { [Symbol.dispose]: reset };
+    if (!button) throw new Error(`Missing button: ${label}`);
+    return act(() => button.click());
+  };
+  const typeComment = (text: string) =>
+    act(() => {
+      Object.getOwnPropertyDescriptor(HTMLTextAreaElement.prototype, "value")!.set!.call(
+        editor(),
+        text,
+      );
+      editor().dispatchEvent(new Event("input", { bubbles: true }));
+    });
+  return {
+    container,
+    viewerRef,
+    view,
+    renderedFile,
+    select,
+    editor,
+    click,
+    typeComment,
+    [Symbol.dispose]: reset,
+  };
 }
 
 describe("diff comment drafts", () => {
   it("keeps the text and selected line through refresh and a provisional lazy patch", async () => {
-    using _drafts = setup();
-    const original = file("shared.ts", 1, "rename-pure", "old.ts");
-    await using screen = await render(view([original]), { container });
-    await click("Comment on shared.ts");
-    await typeComment("Keep this draft");
-    const refreshed = file("shared.ts", 2, "rename-pure", "old.ts");
-    await screen.rerender(view([refreshed], "refreshed"));
-    expect(editor().value).toBe("Keep this draft");
-    expect(container.querySelector("mark")?.textContent).toBe("Line 1");
+    using test = setup();
+    const original = file("shared.ts", "rename-pure", "old.ts");
+    await using screen = await render(test.view([original]), { container: test.container });
+    await expect.element(screen.getByText("new", { exact: true })).toBeVisible();
+    expect(test.renderedFile().instance.getLineIndex(1)).toBeDefined();
+    await test.select();
+    await vi.waitFor(() => expect(test.editor().value).toBe(""));
+    await test.typeComment("Keep this draft");
+    const refreshed = file("shared.ts", "rename-pure", "old.ts");
+    await screen.rerender(test.view([refreshed], "refreshed", [], true));
+    await vi.waitFor(() => expect(test.editor().value).toBe("Keep this draft"));
+    expect(test.viewerRef.current?.getSelectedLines()).toEqual(
+      expect.objectContaining({ id: original.fileKey, range }),
+    );
+    await test.click("Comment");
+    expect(useComposerDraftStore.getState().getComposerDraft(target)?.reviewComments ?? []).toEqual(
+      [],
+    );
+    await act(() => {
+      test
+        .editor()
+        .dispatchEvent(
+          new KeyboardEvent("keydown", { key: "Enter", metaKey: true, bubbles: true }),
+        );
+    });
+    expect(useComposerDraftStore.getState().getComposerDraft(target)?.reviewComments ?? []).toEqual(
+      [],
+    );
+    expect(test.editor().value).toBe("Keep this draft");
 
     await screen.rerender(
-      view([], "loading", [
+      test.view([], "loading", [
         { ...refreshed.fileDiff, type: "rename-changed", hunks: [], cacheKey: "shared:pending" },
       ]),
     );
-    expect(container.querySelector("textarea")).toBeNull();
-    await screen.rerender(view([refreshed], "loaded"));
-    expect(editor().value).toBe("Keep this draft");
-    expect(container.querySelector("mark")?.textContent).toBe("Line 1");
-    await click("Comment");
+    expect(test.container.querySelector("textarea")).toBeNull();
+    await screen.rerender(test.view([refreshed], "loaded"));
+    await vi.waitFor(() => expect(test.editor().value).toBe("Keep this draft"));
+    expect(test.viewerRef.current?.getSelectedLines()).toEqual(
+      expect.objectContaining({ id: original.fileKey, range }),
+    );
+    await test.click("Comment");
     expect(useComposerDraftStore.getState().getComposerDraft(target)?.reviewComments).toEqual([
       expect.objectContaining({
         filePath: "shared.ts",
         text: "Keep this draft",
         selection: { ...range, endSide: "additions" },
       }),
     ]);
-    expect(container.querySelector("p")?.textContent).toBe("Keep this draft");
+  });
+
+  it("rejects old line ranges and retained submission until Pierre adopts refreshed content", async () => {
+    using test = setup();
+    const original = file("adoption.ts");
+    const refreshed = file("adoption.ts", "change", undefined, "inserted\n+new");
+    const highlight = WorkerPoolManager.prototype.highlightDiffAST;
+    let resumeHighlight: (() => void) | undefined;
+    let pool: WorkerPoolManager | undefined;
+    using _highlight = vi
+      .spyOn(WorkerPoolManager.prototype, "highlightDiffAST")
+      .mockImplementation(function (this: WorkerPoolManager, instance, diff) {
+        pool = this;
+        if (diff === refreshed.fileDiff) {
+          resumeHighlight = () => highlight.call(this, instance, diff);
+          return;
+        }
+        return highlight.call(this, instance, diff);
+      });
+    await using screen = await render(test.view([original]), { container: test.container });
+    await expect.element(screen.getByText("new", { exact: true })).toBeVisible();
+    const initialPool = pool;
+    if (!initialPool) throw new Error("Diff worker pool is unavailable");
+    await act(() => initialPool.primeDiffHighlightCache(original.fileDiff));
+    expect(initialPool.getDiffResultCache(original.fileDiff)).toBeDefined();
+    expect(test.renderedFile().instance.getLineIndex(1)).toBeDefined();
+    await act(() => test.viewerRef.current?.getInstance()?.render(true));
+    await test.select();
+    await vi.waitFor(() => expect(test.editor().value).toBe(""));
+    await test.typeComment("Old coordinates");
+    const retainedSubmit = Array.from(test.container.querySelectorAll("button")).find(
+      (node) => node.textContent === "Comment",
+    )!;
+    const previousInstance = test.renderedFile().instance;
+
+    await screen.rerender(test.view([refreshed]));
+    await vi.waitFor(() => expect(resumeHighlight).toBeDefined());
+    const pending = test.renderedFile();
+    expect(pending.item.fileDiff).toBe(refreshed.fileDiff);
+    expect(pending.instance).toBe(previousInstance);
+    expect(pending.element.shadowRoot?.textContent).not.toContain("inserted");
+    expect(retainedSubmit.isConnected).toBe(true);
+    expect(
+      pending.instance.__getNextRenderData({
+        fileDiff: refreshed.fileDiff,
+        lineAnnotations: undefined,
+      }).fileDiff,
+    ).toBe(original.fileDiff);
+    await test.typeComment("Still visible old draft");
+    expect(retainedSubmit.disabled).toBe(false);
+    await act(() => retainedSubmit.click());
+    expect(useComposerDraftStore.getState().getComposerDraft(target)?.reviewComments ?? []).toEqual(
+      [],
+    );
+    await test.select();
+    expect(test.viewerRef.current?.getSelectedLines()).toBeNull();
+
+    await act(() => resumeHighlight?.());
+    await vi.waitFor(() => {
+      expect(
+        test.renderedFile().instance.__getNextRenderData({
+          fileDiff: refreshed.fileDiff,
+          lineAnnotations: undefined,
+        }).fileDiff,
+      ).toBe(refreshed.fileDiff);
+      expect(test.renderedFile().element.shadowRoot?.textContent).toContain("inserted");
+      expect(test.container.querySelector("textarea")).toBeNull();
+    });
+    await test.select();
+    await vi.waitFor(() => expect(test.editor().value).toBe(""));
+    await test.typeComment("Current coordinates");
+    await test.click("Comment");
+    expect(useComposerDraftStore.getState().getComposerDraft(target)?.reviewComments).toEqual([
+      expect.objectContaining({
+        text: "Current coordinates",
+        diff: expect.stringContaining("+inserted"),
+      }),
+    ]);
   });
 });