feat(code-review): mark files as viewed in diff review (#2762)

Author: MattPua

packages/core/src/archive/archiveOrchestration.test.ts

@@ -29,6 +29,7 @@ class Harness {
     stopCloudRun: vi.fn().mockResolvedValue(true),
     disconnectFromTask: vi.fn().mockResolvedValue(undefined),
     archive: vi.fn().mockResolvedValue(undefined),
+    clearViewedState: vi.fn(),
     logError: vi.fn(),
     cache: {
       cancelPathFilter: vi.fn().mockResolvedValue(undefined),
@@ -59,10 +60,19 @@ describe("archiveTask", () => {
 
     expect(harness.deps.archive).toHaveBeenCalledWith(TASK_ID);
     expect(harness.deps.disconnectFromTask).toHaveBeenCalledWith(TASK_ID);
+    expect(harness.deps.clearViewedState).toHaveBeenCalledWith(TASK_ID);
     expect(harness.ids).toContain(TASK_ID);
     expect(harness.list.some((a) => a.taskId === TASK_ID)).toBe(true);
   });
 
+  it("does not clear read state when the archive request fails", async () => {
+    harness.deps.archive = vi.fn().mockRejectedValue(new Error("boom"));
+
+    await expect(archiveTask(TASK_ID, harness.deps)).rejects.toThrow("boom");
+
+    expect(harness.deps.clearViewedState).not.toHaveBeenCalled();
+  });
+
   it("with optimistic:false, defers cache writes until archive resolves", async () => {
     let idsWhenArchiveCalled: string[] = ["sentinel"];
     harness.deps.archive = vi.fn().mockImplementation(async () => {

packages/core/src/archive/archiveOrchestration.ts

@@ -39,6 +39,7 @@ export interface ArchiveOrchestrationDeps {
   stopCloudRun(taskId: string, runId?: string): Promise<boolean>;
   disconnectFromTask(taskId: string): Promise<void>;
   archive(taskId: string): Promise<void>;
+  clearViewedState(taskId: string): void;
   logError(message: string, error: unknown): void;
   cache: ArchiveCacheWriter;
 }
@@ -103,9 +104,8 @@ export async function archiveTask(
   try {
     await deps.disconnectFromTask(taskId);
     await deps.archive(taskId);
-    // Destroying terminals is irreversible, so it waits for the archive to
-    // commit; a failed archive keeps its live terminals.
     deps.clearTerminalStates(taskId);
+    deps.clearViewedState(taskId);
     // Non-optimistic flows keep the row visible during the request, then remove
     // it the moment the archive succeeds.
     if (!optimistic) {

packages/core/src/git/router-schemas.ts

@@ -42,6 +42,7 @@ export const changedFileSchema = z.object({
   linesRemoved: z.number().optional(),
   staged: z.boolean().optional(),
   patch: z.string().optional(),
+  sha: z.string().optional(),
 });
 
 export type ChangedFile = z.infer<typeof changedFileSchema>;

packages/shared/src/domain-types.ts

@@ -364,6 +364,7 @@ export interface ChangedFile {
   linesRemoved?: number;
   staged?: boolean;
   patch?: string; // Unified diff patch from GitHub API
+  sha?: string;
 }
 
 // External apps detection types

packages/ui/src/features/archive/useArchiveTask.ts

@@ -16,6 +16,7 @@ import {
   type HostTrpcClient,
 } from "@posthog/host-router/client";
 import { useHostTRPC } from "@posthog/host-router/react";
+import { useReviewViewedStore } from "@posthog/ui/features/code-review/reviewViewedStore";
 import { useCommandCenterStore } from "@posthog/ui/features/command-center/commandCenterStore";
 import { useFocusStore } from "@posthog/ui/features/focus/focusStore";
 import { pinnedTasksApi } from "@posthog/ui/features/sidebar/taskMetaApi";
@@ -127,6 +128,8 @@ function makeOrchestrationDeps(
       ),
     archive: (taskId) =>
       hostClient.archive.archive.mutate({ taskId }).then(() => undefined),
+    clearViewedState: (taskId) =>
+      useReviewViewedStore.getState().clearTasks([taskId]),
     logError: (message, error) => log.error(message, error),
     cache: makeCacheWriter(queryClient, keys),
   };

packages/ui/src/features/code-review/components/CloudReviewPage.tsx

@@ -15,6 +15,7 @@ import {
   ReviewShell,
   useReviewState,
 } from "./ReviewShell";
+import { changedFileSignature } from "./reviewItemBuilders";
 
 interface CloudReviewPageProps {
   task: Task;
@@ -50,7 +51,19 @@ export function CloudReviewPage({ task }: CloudReviewPageProps) {
     expandAll,
     collapseAll,
     uncollapseFile,
-  } = useReviewState(reviewFiles, allPaths);
+    collapseFiles,
+    viewedRecord,
+    toggleViewed,
+  } = useReviewState(reviewFiles, allPaths, taskId);
+
+  const currentSignatures = useMemo(() => {
+    const map = new Map<string, string>();
+    for (const f of reviewFiles) {
+      const signature = changedFileSignature(f);
+      if (signature) map.set(f.path, signature);
+    }
+    return map;
+  }, [reviewFiles]);
 
   const toolCallFallbacks = useMemo(
     () =>
@@ -81,6 +94,7 @@ export function CloudReviewPage({ task }: CloudReviewPageProps) {
             commentThreads={showReviewComments ? commentThreads : undefined}
             fallback={toolCallFallbacks?.get(file.path) ?? null}
             externalUrl={githubFileUrl}
+            viewedKey={file.path}
           />
         ),
       };
@@ -130,8 +144,12 @@ export function CloudReviewPage({ task }: CloudReviewPageProps) {
       onExpandAll={expandAll}
       onCollapseAll={collapseAll}
       onUncollapseFile={uncollapseFile}
+      onCollapseFiles={collapseFiles}
       items={items}
       itemIndexByFilePath={itemIndexByFilePath}
+      currentSignatures={currentSignatures}
+      viewedRecord={viewedRecord}
+      onToggleViewed={toggleViewed}
     />
   );
 }

packages/ui/src/features/code-review/components/PatchedFileDiff.tsx

@@ -17,6 +17,7 @@ interface PatchedFileDiffProps {
   externalUrl?: string;
   prUrl?: string | null;
   commentThreads?: Map<number, PrCommentThread>;
+  viewedKey?: string;
   /** Extra controls in the file header row (e.g. a "Viewed" toggle). */
   headerTrailing?: ReactNode;
 }
@@ -31,6 +32,7 @@ export function PatchedFileDiff({
   externalUrl,
   prUrl,
   commentThreads,
+  viewedKey,
   headerTrailing,
 }: PatchedFileDiffProps) {
   const fileDiff = useMemo((): FileDiffMetadata | undefined => {
@@ -64,6 +66,7 @@ export function PatchedFileDiff({
         collapsed={collapsed}
         onToggle={onToggle}
         externalUrl={externalUrl}
+        viewedKey={viewedKey}
         commentCount={commentCount}
         headerTrailing={headerTrailing}
       />
@@ -80,6 +83,7 @@ export function PatchedFileDiff({
         collapsed={collapsed}
         onToggle={onToggle}
         externalUrl={externalUrl}
+        viewedKey={viewedKey}
         commentCount={commentCount}
         headerTrailing={headerTrailing}
       />
@@ -98,6 +102,7 @@ export function PatchedFileDiff({
           fileDiff={fd}
           collapsed={collapsed}
           onToggle={onToggle}
+          viewedKey={viewedKey}
           commentCount={commentCount}
           trailing={headerTrailing}
         />

packages/ui/src/features/code-review/components/ReviewPage.tsx

@@ -33,6 +33,8 @@ import {
   buildPatchReviewItems,
   buildRemoteReviewItems,
   buildUntrackedReviewItems,
+  changedFileSignature,
+  patchFileSignature,
 } from "./reviewItemBuilders";
 
 const EMPTY_CHANGED_FILES: ChangedFile[] = [];
@@ -138,7 +140,10 @@ export function ReviewPage({ task }: ReviewPageProps) {
     expandAll,
     collapseAll,
     uncollapseFile,
-  } = useReviewState(changedFiles, allPaths);
+    collapseFiles,
+    viewedRecord,
+    toggleViewed,
+  } = useReviewState(changedFiles, allPaths, taskId);
 
   const stagedPathSet = useMemo(
     () => new Set(stagedParsedFiles.map((f) => f.name ?? f.prevName ?? "")),
@@ -191,6 +196,9 @@ export function ReviewPage({ task }: ReviewPageProps) {
       expandAll={expandAll}
       collapseAll={collapseAll}
       uncollapseFile={uncollapseFile}
+      collapseFiles={collapseFiles}
+      viewedRecord={viewedRecord}
+      toggleViewed={toggleViewed}
       refetch={refetch}
       hasStagedFiles={hasStagedFiles}
       stagedParsedFiles={stagedParsedFiles}
@@ -224,6 +232,9 @@ function LocalReviewContent({
   expandAll,
   collapseAll,
   uncollapseFile,
+  collapseFiles,
+  viewedRecord,
+  toggleViewed,
   refetch,
   hasStagedFiles,
   stagedParsedFiles,
@@ -253,6 +264,9 @@ function LocalReviewContent({
   expandAll: () => void;
   collapseAll: () => void;
   uncollapseFile: (filePath: string) => void;
+  collapseFiles: (keys: string[]) => void;
+  viewedRecord: Record<string, string>;
+  toggleViewed: (key: string, sig: string | null) => void;
   refetch: () => void;
   hasStagedFiles: boolean;
   stagedParsedFiles: ReturnType<typeof parsePatchFiles>[number]["files"];
@@ -295,6 +309,27 @@ function LocalReviewContent({
     [filesByKey, stageToggle],
   );
 
+  const currentSignatures = useMemo(() => {
+    const map = new Map<string, string>();
+    for (const f of stagedParsedFiles) {
+      map.set(
+        makeFileKey(true, f.name ?? f.prevName ?? ""),
+        patchFileSignature(f),
+      );
+    }
+    for (const f of unstagedParsedFiles) {
+      map.set(
+        makeFileKey(false, f.name ?? f.prevName ?? ""),
+        patchFileSignature(f),
+      );
+    }
+    for (const f of untrackedFiles) {
+      const signature = changedFileSignature(f);
+      if (signature) map.set(makeFileKey(f.staged, f.path), signature);
+    }
+    return map;
+  }, [stagedParsedFiles, unstagedParsedFiles, untrackedFiles]);
+
   const items = useMemo<ReviewListItem[]>(() => {
     const reviewItems: ReviewListItem[] = [];
 
@@ -393,6 +428,7 @@ function LocalReviewContent({
       onExpandAll={expandAll}
       onCollapseAll={collapseAll}
       onUncollapseFile={uncollapseFile}
+      onCollapseFiles={collapseFiles}
       onRefresh={refetch}
       onDiscardAll={totalFileCount > 0 ? discardAllChanges : undefined}
       effectiveSource={effectiveSource}
@@ -401,6 +437,9 @@ function LocalReviewContent({
       defaultBranch={defaultBranch}
       items={items}
       itemIndexByFilePath={itemIndexByFilePath}
+      currentSignatures={currentSignatures}
+      viewedRecord={viewedRecord}
+      onToggleViewed={toggleViewed}
     />
   );
 }
@@ -455,7 +494,16 @@ function RemoteReviewPage({
     : prLoading && files.length === 0;
 
   const allPaths = useMemo(() => files.map((f) => f.path), [files]);
-  const reviewState = useReviewState(files, allPaths);
+  const reviewState = useReviewState(files, allPaths, taskId);
+
+  const currentSignatures = useMemo(() => {
+    const map = new Map<string, string>();
+    for (const f of files) {
+      const signature = changedFileSignature(f);
+      if (signature) map.set(f.path, signature);
+    }
+    return map;
+  }, [files]);
 
   const items = useMemo(
     () =>
@@ -492,13 +540,17 @@ function RemoteReviewPage({
       onExpandAll={reviewState.expandAll}
       onCollapseAll={reviewState.collapseAll}
       onUncollapseFile={reviewState.uncollapseFile}
+      onCollapseFiles={reviewState.collapseFiles}
       onRefresh={onRefresh}
       effectiveSource={effectiveSource}
       branchSourceAvailable={branchSourceAvailable}
       prSourceAvailable={prSourceAvailable}
       defaultBranch={defaultBranch}
       items={items}
       itemIndexByFilePath={itemIndexByFilePath}
+      currentSignatures={currentSignatures}
+      viewedRecord={reviewState.viewedRecord}
+      onToggleViewed={reviewState.toggleViewed}
     />
   );
 }

packages/ui/src/features/code-review/components/ReviewRows.tsx

@@ -83,9 +83,10 @@ export const PatchRow = memo(function PatchRow({
         onDiscard={onDiscard}
         onStage={onStage}
         staged={staged}
+        viewedKey={itemKey}
       />
     ),
-    [collapsed, onToggle, onOpenFile, onDiscard, onStage, staged],
+    [collapsed, onToggle, onOpenFile, onDiscard, onStage, staged, itemKey],
   );
 
   // Binary files (images, video, archives, …) have no meaningful textual diff;
@@ -176,6 +177,7 @@ export const UntrackedRow = memo(function UntrackedRow({
       onDiscard={onDiscard}
       onStage={onStage}
       taskId={taskId}
+      viewedKey={itemKey}
     />
   );
 });
@@ -215,6 +217,7 @@ export const RemoteRow = memo(function RemoteRow({
       onToggle={onToggle}
       commentThreads={commentThreads}
       externalUrl={externalUrl}
+      viewedKey={file.path}
     />
   );
 });
@@ -228,6 +231,7 @@ function UntrackedFileDiff({
   onToggle,
   onDiscard,
   onStage,
+  viewedKey,
 }: {
   file: ChangedFile;
   repoPath: string;
@@ -237,6 +241,7 @@ function UntrackedFileDiff({
   onToggle: () => void;
   onDiscard?: () => void;
   onStage?: () => void;
+  viewedKey?: string;
 }) {
   const [containerRef, inView] = useInView<HTMLDivElement>({
     rootMargin: REVIEW_PREFETCH_ROOT_MARGIN,
@@ -278,6 +283,7 @@ function UntrackedFileDiff({
         reason="line-limit"
         collapsed={collapsed}
         onToggle={onToggle}
+        viewedKey={viewedKey}
       />
     );
   }
@@ -301,6 +307,7 @@ function UntrackedFileDiff({
               onDiscard={onDiscard}
               onStage={onStage}
               staged={false}
+              viewedKey={viewedKey}
             />
           )}
         />
@@ -312,6 +319,7 @@ function UntrackedFileDiff({
           deletions={0}
           collapsed={collapsed}
           onToggle={onToggle}
+          viewedKey={viewedKey}
         />
       )}
     </div>