From 3f2886d38e9d36e407d4a6b70ec99190d6316222 Mon Sep 17 00:00:00 2001 From: Yash Singh Date: Tue, 3 Nov 2026 20:34:27 -0600 Subject: [PATCH 1/5] fix(web): open selected git diffs before untracked discovery --- apps/server/src/vcs/GitVcsDriverCore.test.ts | 68 ++++++++++++++++++++ apps/server/src/vcs/GitVcsDriverCore.ts | 16 +++-- apps/web/src/components/DiffPanel.tsx | 22 ++++++- packages/client-runtime/src/state/review.ts | 15 +++++ packages/contracts/src/review.ts | 3 + 5 files changed, 117 insertions(+), 7 deletions(-) diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index 7d9d68b28566a..710def2b0b8b7 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -2396,6 +2396,68 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { }), ); + it.effect( + "returns an incomplete selected tracked preview while untracked discovery is held", + () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + yield* initRepoWithCommit(cwd); + yield* writeTextFile(cwd, "README.md", "changed\n"); + yield* writeTextFile(cwd, "new.txt", "untracked\n"); + const started = yield* Deferred.make(); + const release = yield* Deferred.make(); + let untrackedReads = 0; + const delegate = yield* ChildProcessSpawner.ChildProcessSpawner; + const spawner = ChildProcessSpawner.make((command) => + Effect.gen(function* () { + if (ChildProcess.isStandardCommand(command) && command.args.includes("--others")) { + assert.equal( + ++untrackedReads, + 1, + "tracked preview must not enumerate untracked files", + ); + yield* Deferred.succeed(started, undefined); + yield* Deferred.await(release); + } + return yield* delegate.spawn(command); + }), + ); + const driver = yield* makeGitVcsDriverCore().pipe( + Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), + Effect.provide(layerServerConfig), + ); + const full = yield* driver + .getReviewDiffPreview({ cwd, sourceKind: "working-tree" }) + .pipe(Effect.forkChild); + yield* Deferred.await(started); + yield* Effect.gen(function* () { + const preview = yield* driver.getReviewDiffPreview({ + cwd, + sourceKind: "working-tree", + trackedOnly: true, + }); + assert.isTrue(preview.incomplete); + assert.deepStrictEqual( + preview.sources.map((source) => source.kind), + ["working-tree"], + ); + assert.include(preview.sources[0]!.diff, "+changed"); + assert.notInclude(preview.sources[0]!.diff, "untracked"); + }).pipe(Effect.ensuring(Deferred.succeed(release, undefined))); + const complete = yield* Fiber.join(full); + assert.isUndefined(complete.incomplete); + assert.include(complete.sources[0]!.diff, "+untracked"); + yield* writeTextFile(cwd, "README.md", "edited again\n"); + const refreshed = yield* driver.getReviewDiffPreview({ + cwd, + baseRef: "missing-base", + sourceKind: "working-tree", + trackedOnly: true, + }); + assert.include(refreshed.sources[0]!.diff, "+edited again"); + }), + ); + it.effect("propagates patch failures instead of reporting an empty complete diff", () => Effect.gen(function* () { const cwd = yield* makeTmpDir(); @@ -2734,6 +2796,12 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { assert.isTrue(dirty.truncated); assert.notInclude(branch.diff, "z-last.txt"); for (const source of [branch, dirty]) { + const selected = yield* driver.getReviewDiffPreview({ + cwd, + baseRef: initialBranch, + sourceKind: source.kind === "working-tree" ? "working-tree" : "branch-range", + }); + assert.deepStrictEqual(selected.sources, [source]); for (const file of source.files ?? []) { const individual = yield* driver.getReviewDiffPreview({ cwd, diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 5e5aaa0fddce2..c9a279529eb3b 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -3409,10 +3409,11 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* const patchLimit = input.file ? REVIEW_DIFF_FILE_MAX_OUTPUT_BYTES : REVIEW_DIFF_PATCH_MAX_OUTPUT_BYTES; + const sourceKind = input.file?.sourceKind ?? input.sourceKind; const needsCurrentBranch = input.commitSha === undefined && input.file?.sourceKind !== "commit" && - input.file?.sourceKind !== "working-tree"; + sourceKind !== "working-tree"; const repository = yield* ( needsCurrentBranch ? resolveRepositoryPathsUncached(input.cwd) @@ -3490,9 +3491,9 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* }; } - const dirtyRef = input.file?.sourceKind === "branch-range" ? null : "HEAD"; + const dirtyRef = sourceKind === "branch-range" ? null : "HEAD"; const review = - input.file?.sourceKind === "working-tree" + sourceKind === "working-tree" ? { baseRef: input.baseRef ?? null, mergeBase: null } : yield* resolveReviewMergeBase(cwd, repository.currentBranch, input.baseRef); @@ -3547,7 +3548,9 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* return { ...patch, files: stat.files }; }); const [dirtyTrackedResult, baseResult] = yield* Effect.gen(function* () { - const untracked = yield* prepareUntrackedReviewIndex(cwd, pathArgs, input.file?.path); + const untracked = input.trackedOnly + ? { env: undefined } + : yield* prepareUntrackedReviewIndex(cwd, pathArgs, input.file?.path); const [dirty, base] = review.mergeBase === dirtyRef ? yield* readTrackedDiff(dirtyRef, untracked?.env).pipe( @@ -3602,7 +3605,10 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* return { cwd: input.cwd, generatedAt: yield* DateTime.now, - sources, + sources: input.sourceKind + ? sources.filter((source) => source.kind === input.sourceKind) + : sources, + ...(input.trackedOnly ? { incomplete: true } : {}), }; }); diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx index 0f1d1e9e28379..54698c324c8a7 100644 --- a/apps/web/src/components/DiffPanel.tsx +++ b/apps/web/src/components/DiffPanel.tsx @@ -362,6 +362,14 @@ export default function DiffPanel({ ...(selectedBaseRef ? { baseRef: selectedBaseRef } : {}), ...(selectedCommitSha !== null ? { commitSha: selectedCommitSha } : {}), ignoreWhitespace: diffIgnoreWhitespace, + ...(selectedCommitSha === null + ? { + sourceKind: + selectedGitScope === "unstaged" + ? ("working-tree" as const) + : ("branch-range" as const), + } + : {}), } : undefined; const canRefreshGitDiff = @@ -472,7 +480,8 @@ export default function DiffPanel({ ? activeCheckpointDiff.error : branchDiffPreview.error; const hasResolvedPatch = typeof selectedPatch === "string"; - const hasNoNetChanges = hasResolvedPatch && selectedPatch.trim().length === 0; + const hasNoNetChanges = + hasResolvedPatch && selectedPatch.trim().length === 0 && !preview?.incomplete; const lazySource = isGitSelection && selectedGitSource?.truncated && selectedGitSource.files ? selectedGitSource @@ -1273,10 +1282,19 @@ export default function DiffPanel({ ) : ( <>
+ {preview?.incomplete && ( +

+ {selectedPatchError + ? "Showing tracked changes. Untracked changes could not be loaded. Refresh to retry." + : "Showing tracked changes. Checking untracked files..."} +

+ )} {isSelectedPatchTruncated && !lazySource && (

This preview exceeds the size limit. Changes shown are incomplete. - {isGitSelection && selectedGitSource?.files ? " Totals include all changes." : ""} + {isGitSelection && selectedGitSource?.files && !preview?.incomplete + ? " Totals include all changes." + : ""}

)} {omittedToolCallFiles > 0 && ( diff --git a/packages/client-runtime/src/state/review.ts b/packages/client-runtime/src/state/review.ts index 6700be9af657a..296b08135f57f 100644 --- a/packages/client-runtime/src/state/review.ts +++ b/packages/client-runtime/src/state/review.ts @@ -35,6 +35,21 @@ export function createReviewEnvironmentAtoms( label: "environment-data:review:diff-preview", tag: WS_METHODS.reviewGetDiffPreview, staleTimeMs: 5_000, + execute: Effect.fn("review.diffPreview")(function* ( + input: ReviewDiffPreviewInput, + emit: (value: ReviewDiffPreviewResult) => Effect.Effect, + ) { + if (input.sourceKind && !input.file && !input.commitSha && !input.trackedOnly) { + const tracked = yield* request(WS_METHODS.reviewGetDiffPreview, { + ...input, + trackedOnly: true, + }); + // Older servers ignore trackedOnly and already return the complete preview. + if (!tracked.incomplete) return tracked; + yield* emit(tracked); + } + return yield* request(WS_METHODS.reviewGetDiffPreview, input); + }), ...(options.onDiffPreviewAnswer === undefined ? {} : { onAnswer: options.onDiffPreviewAnswer }), }); // A file patch is a slice of the comparison its preview generated, so a newer preview diff --git a/packages/contracts/src/review.ts b/packages/contracts/src/review.ts index a40bddfc1e33e..080619443ce58 100644 --- a/packages/contracts/src/review.ts +++ b/packages/contracts/src/review.ts @@ -8,6 +8,8 @@ export const ReviewDiffPreviewInput = Schema.Struct({ baseRef: Schema.optional(TrimmedNonEmptyString), commitSha: Schema.optional(TrimmedNonEmptyString), ignoreWhitespace: Schema.optionalKey(Schema.Boolean), + sourceKind: Schema.optionalKey(Schema.Literals(["working-tree", "branch-range"])), + trackedOnly: Schema.optionalKey(Schema.Boolean), file: Schema.optionalKey( Schema.Struct({ path: Schema.NonEmptyString, @@ -86,6 +88,7 @@ export const ReviewDiffPreviewResult = Schema.Struct({ cwd: TrimmedNonEmptyString, generatedAt: Schema.DateTimeUtc, sources: Schema.Array(ReviewDiffPreviewSource), + incomplete: Schema.optionalKey(Schema.Boolean), }); export type ReviewDiffPreviewResult = typeof ReviewDiffPreviewResult.Type; From 4d147403cb8754b8e7359291f0358a44127410a3 Mon Sep 17 00:00:00 2001 From: Yash Singh Date: Tue, 3 Nov 2026 21:18:46 -0600 Subject: [PATCH 2/5] fix(web): hydrate diff metadata without resetting scroll --- apps/server/src/vcs/GitVcsDriverCore.test.ts | 48 +++++++- apps/server/src/vcs/GitVcsDriverCore.ts | 34 ++++-- apps/web/src/components/DiffPanel.tsx | 78 ++++++++----- .../useReviewFilePatches.browser.test.tsx | 105 ++++++++++++++++++ .../components/diffs/useReviewFilePatches.ts | 46 ++++---- packages/client-runtime/src/state/review.ts | 10 +- .../client-runtime/src/state/runtime.test.ts | 60 ++++++++++ packages/client-runtime/src/state/runtime.ts | 14 ++- packages/contracts/src/review.ts | 2 + 9 files changed, 334 insertions(+), 63 deletions(-) create mode 100644 apps/web/src/components/diffs/useReviewFilePatches.browser.test.tsx diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index 710def2b0b8b7..d00301e96e7d8 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -2170,6 +2170,14 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { .sources[0]!; assert.equal(rootSource.baseRef, emptyTree); assert.include(rootSource.diff, "+# test"); + const rootMetadata = (yield* driver.getReviewDiffPreview({ + cwd: upstream, + commitSha: root, + metadataOnly: true, + })).sources[0]!; + assert.deepStrictEqual(rootMetadata.files, rootSource.files); + assert.isTrue(rootMetadata.patchesDeferred); + assert.isEmpty(rootMetadata.diff); yield* writeTextFile(upstream, "README.md", "# boundary\n"); yield* git(upstream, ["commit", "-am", "boundary"]); @@ -2443,6 +2451,16 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { ); assert.include(preview.sources[0]!.diff, "+changed"); assert.notInclude(preview.sources[0]!.diff, "untracked"); + const metadata = yield* driver.getReviewDiffPreview({ + cwd, + sourceKind: "working-tree", + trackedOnly: true, + metadataOnly: true, + }); + assert.isTrue(metadata.incomplete); + assert.deepStrictEqual(metadata.sources[0]!.files, preview.sources[0]!.files); + assert.isTrue(metadata.sources[0]!.patchesDeferred); + assert.isEmpty(metadata.sources[0]!.diff); }).pipe(Effect.ensuring(Deferred.succeed(release, undefined))); const complete = yield* Fiber.join(full); assert.isUndefined(complete.incomplete); @@ -2478,6 +2496,16 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { if (Result.isFailure(result)) { assert.equal(result.failure.operation, "GitVcsDriver.getReviewDiffPreview.patch"); } + const metadata = yield* driver.getReviewDiffPreview({ + cwd, + sourceKind: "working-tree", + metadataOnly: true, + }); + assert.isTrue(metadata.sources[0]!.patchesDeferred); + assert.isEmpty(metadata.sources[0]!.diff); + assert.deepStrictEqual(metadata.sources[0]!.files, [ + { path: "README.md", previousPath: null, additions: 1, deletions: 1 }, + ]); }), ); @@ -2659,9 +2687,17 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { ); yield* writeTextFile(cwd, "README.md", "# tracked change\n"); - const preview = yield* driver.getReviewDiffPreview({ cwd, ignoreWhitespace: false }); + const preview = yield* driver.getReviewDiffPreview({ + cwd, + ignoreWhitespace: false, + metadataOnly: true, + }); const diff = preview.sources.find((source) => source.kind === "working-tree")?.diff ?? ""; + const source = preview.sources.find((source) => source.kind === "working-tree")!; + assert.isUndefined(source.patchesDeferred); + assert.isUndefined(source.files); + assert.isTrue(source.truncated); assert.include(diff, "-# test"); assert.include(diff, "+# tracked change"); }), @@ -2802,6 +2838,16 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { sourceKind: source.kind === "working-tree" ? "working-tree" : "branch-range", }); assert.deepStrictEqual(selected.sources, [source]); + const metadata = (yield* driver.getReviewDiffPreview({ + cwd, + baseRef: initialBranch, + sourceKind: source.kind === "working-tree" ? "working-tree" : "branch-range", + metadataOnly: true, + })).sources[0]!; + assert.deepStrictEqual(metadata.files, source.files); + assert.isTrue(metadata.patchesDeferred); + assert.isFalse(metadata.truncated); + assert.isEmpty(metadata.diff); for (const file of source.files ?? []) { const individual = yield* driver.getReviewDiffPreview({ cwd, diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index c9a279529eb3b..25a757104db32 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -3464,12 +3464,15 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* { maxOutputBytes: REVIEW_METADATA_MAX_OUTPUT_BYTES }, ); const files = parseReviewNumstat(stat.stdout); - const patch = yield* executeGit( - "GitVcsDriver.getReviewDiffPreview.commitPatch", - cwd, - [...diffArgs, "--patch", commit.parent, commit.sha, "--", ...pathArgs], - { maxOutputBytes: patchLimit, appendTruncationMarker: true }, - ); + const patch = + input.metadataOnly && !input.file + ? { stdout: "", stdoutTruncated: false } + : yield* executeGit( + "GitVcsDriver.getReviewDiffPreview.commitPatch", + cwd, + [...diffArgs, "--patch", commit.parent, commit.sha, "--", ...pathArgs], + { maxOutputBytes: patchLimit, appendTruncationMarker: true }, + ); const diffHash = yield* hashDiff(patch.stdout, files); return { cwd: input.cwd, @@ -3486,6 +3489,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* files, diffHash, truncated: patch.stdoutTruncated, + ...(input.metadataOnly && !input.file ? { patchesDeferred: true } : {}), }, ], }; @@ -3535,10 +3539,13 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* const readTrackedDiff = Effect.fn("GitVcsDriver.getReviewDiffPreview.tracked")(function* ( ref: string | null, env?: NodeJS.ProcessEnv, + metadataOnly = false, ) { if (ref === null) return { stdout: "", stdoutTruncated: false, files: [] }; const stat = yield* readStats(ref, env); - if (stat.files.length === 0) return { stdout: "", stdoutTruncated: false, files: [] }; + if (metadataOnly || stat.files.length === 0) { + return { stdout: "", stdoutTruncated: false, files: stat.files }; + } const patch = yield* executeGit( "GitVcsDriver.getReviewDiffPreview.patch", cwd, @@ -3551,15 +3558,16 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* const untracked = input.trackedOnly ? { env: undefined } : yield* prepareUntrackedReviewIndex(cwd, pathArgs, input.file?.path); + const metadataOnly = input.metadataOnly === true && !input.file && untracked !== null; const [dirty, base] = review.mergeBase === dirtyRef - ? yield* readTrackedDiff(dirtyRef, untracked?.env).pipe( + ? yield* readTrackedDiff(dirtyRef, untracked?.env, metadataOnly).pipe( Effect.map((result) => [result, result] as const), ) : yield* Effect.all( [ - readTrackedDiff(dirtyRef, untracked?.env), - readTrackedDiff(review.mergeBase, untracked?.env), + readTrackedDiff(dirtyRef, untracked?.env, metadataOnly), + readTrackedDiff(review.mergeBase, untracked?.env, metadataOnly), ], { concurrency: 2 }, ); @@ -3588,6 +3596,9 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* ...(dirtyFiles === undefined ? {} : { files: dirtyFiles }), diffHash: dirtyDiffHash, truncated: dirtyTrackedResult.stdoutTruncated, + ...(input.metadataOnly && !input.file && dirtyFiles !== undefined + ? { patchesDeferred: true } + : {}), }, { id: "branch-range", @@ -3599,6 +3610,9 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* ...(baseFiles === undefined ? {} : { files: baseFiles }), diffHash: baseDiffHash, truncated: baseResult.stdoutTruncated, + ...(input.metadataOnly && !input.file && baseFiles !== undefined + ? { patchesDeferred: true } + : {}), }, ]; diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx index 54698c324c8a7..ee5372ae5fa30 100644 --- a/apps/web/src/components/DiffPanel.tsx +++ b/apps/web/src/components/DiffPanel.tsx @@ -102,6 +102,7 @@ import { vcsEnvironment } from "../state/vcs"; import { buildBaseRefChoices, filterBaseRefChoices } from "../lib/baseRefChoices"; import { createGitDiffFileContentsLoader } from "../lib/diffFileContents"; +import { observeIntersection } from "~/lib/observeIntersection"; import { useReviewFilePatches } from "./diffs/useReviewFilePatches"; import { DiffFileLoadingBoundary } from "./diffs/DiffFileLoadingBoundary"; import { DiffFileStatus } from "./diffs/DiffFileStatus"; @@ -135,13 +136,13 @@ function DiffFileCollapseToggle({ filePath, fileKey, collapsed, - unavailable, onToggle, + load, }: { filePath: string; fileKey: string; collapsed: boolean; - unavailable: boolean; + load?: () => void; onToggle: (fileKey: string) => void; }) { return ( @@ -154,7 +155,16 @@ function DiffFileCollapseToggle({ className="-ms-0.5" aria-label={collapsed ? `Expand ${filePath}` : `Collapse ${filePath}`} aria-expanded={!collapsed} - disabled={unavailable} + ref={(node) => { + if (!node || !load || collapsed) return; + return observeIntersection( + node, + (entry) => { + if (entry.isIntersecting) load(); + }, + { rootMargin: "600px" }, + ); + }} onClick={(event) => { event.stopPropagation(); onToggle(fileKey); @@ -362,6 +372,7 @@ export default function DiffPanel({ ...(selectedBaseRef ? { baseRef: selectedBaseRef } : {}), ...(selectedCommitSha !== null ? { commitSha: selectedCommitSha } : {}), ignoreWhitespace: diffIgnoreWhitespace, + metadataOnly: true, ...(selectedCommitSha === null ? { sourceKind: @@ -481,9 +492,14 @@ export default function DiffPanel({ : branchDiffPreview.error; const hasResolvedPatch = typeof selectedPatch === "string"; const hasNoNetChanges = - hasResolvedPatch && selectedPatch.trim().length === 0 && !preview?.incomplete; + hasResolvedPatch && + selectedPatch.trim().length === 0 && + !preview?.incomplete && + (!selectedGitSource?.patchesDeferred || selectedGitSource.files?.length === 0); const lazySource = - isGitSelection && selectedGitSource?.truncated && selectedGitSource.files + isGitSelection && + (selectedGitSource?.truncated || selectedGitSource?.patchesDeferred) && + selectedGitSource.files ? selectedGitSource : null; const renderablePatch = getRenderablePatch( @@ -575,12 +591,10 @@ export default function DiffPanel({ ...matchingBaseRefChoices.map(valueForBaseRefChoice), ]; const { - scope: filePatchScope, isPending: areFilePatchesPending, fileStates, retry, requestFile, - readyFilePaths, renderableFiles, settledFileCount, loadNextFiles, @@ -618,7 +632,16 @@ export default function DiffPanel({ ); const isRefreshingDiff = branchDiffPreview.isPending || areFilePatchesPending; - const renderableFileEntries = renderableFiles.map(getCachedFileEntry); + const pathOccurrences = new Map(); + const renderableFileEntries = renderableFiles.map((fileDiff) => { + const entry = getCachedFileEntry(fileDiff); + if (!isGitSelection) return entry; + // Metadata has no change type; keep each path's identity when its patch arrives. + const pathKey = JSON.stringify([fileDiff.prevName ?? fileDiff.name, fileDiff.name]); + const occurrence = pathOccurrences.get(pathKey) ?? 0; + pathOccurrences.set(pathKey, occurrence + 1); + return { ...entry, fileKey: `${pathKey}:${occurrence}` }; + }); const diffFileKeys = renderableFileEntries.map((file) => file.fileKey); const testFileCount = renderableFiles.filter((file) => isTestChangedFile(resolveFileDiffPath(file)), @@ -652,19 +675,14 @@ export default function DiffPanel({ count={renderableFiles.length - settledFileCount} /> ) : null; - const codeViewFiles = shownFileEntries - .filter( - ({ fileDiff }) => - !lazySource || !readyFilePaths || readyFilePaths.has(resolveFileDiffPath(fileDiff)), - ) - .map(({ fileDiff, fileKey, fileVersion }) => ({ - fileDiff, - filePath: resolveFileDiffPath(fileDiff), - fileKey, - fileVersion, - collapsed: - collapsedDiffFileKeys.has(fileKey) || fileDiff.cacheKey?.endsWith(":pending") === true, - })); + const codeViewFiles = shownFileEntries.map(({ fileDiff, fileKey, fileVersion }) => ({ + fileDiff, + filePath: resolveFileDiffPath(fileDiff), + fileKey, + fileVersion, + collapsed: + collapsedDiffFileKeys.has(fileKey) || fileDiff.cacheKey?.endsWith(":pending") === true, + })); const pendingFiles = renderableFiles.filter( (file) => file.cacheKey?.endsWith(":pending") && isFileShown(file), ); @@ -732,6 +750,8 @@ export default function DiffPanel({ }); }; const toggleDiffFileCollapsed = useStableCallback((fileKey: string) => { + const index = renderableFileEntries.findIndex((file) => file.fileKey === fileKey); + if (lazySource && index >= 0) requestFile(index); updateCollapsedDiffFiles((current) => { const next = new Set(current); if (next.has(fileKey)) { @@ -1407,7 +1427,7 @@ export default function DiffPanel({ { - const unavailable = fileDiff.cacheKey?.endsWith(":pending") === true; return ( + requestFile( + renderableFileEntries.findIndex( + (file) => file.fileKey === fileKey, + ), + ), + } + : {})} /> ); }} diff --git a/apps/web/src/components/diffs/useReviewFilePatches.browser.test.tsx b/apps/web/src/components/diffs/useReviewFilePatches.browser.test.tsx new file mode 100644 index 0000000000000..cfbf3c5a53d55 --- /dev/null +++ b/apps/web/src/components/diffs/useReviewFilePatches.browser.test.tsx @@ -0,0 +1,105 @@ +import { RegistryContext } from "@effect/atom-react"; +import type { ReviewDiffPreviewSource } from "@t3tools/contracts"; +import { EnvironmentId } from "@t3tools/contracts"; +import { AsyncResult, Atom, AtomRegistry } from "effect/reactivity"; +import * as Option from "effect/Option"; +import { expect, it, vi } from "vite-plus/test"; +import { userEvent } from "vite-plus/test/browser"; +import { act, render } from "../../../test/render"; + +type PatchAtom = Atom.Writable>; +const state = vi.hoisted(() => ({ patches: new Map() })); +vi.mock("~/state/review", () => ({ + reviewEnvironment: { + diffFilePatch: (target: { + input: { cacheKey: string; request: { file: { path: string } } }; + }) => { + const key = JSON.stringify([target.input.cacheKey, target.input.request.file.path]); + let atom = state.patches.get(key); + if (!atom) { + atom = Atom.make>( + AsyncResult.initial(), + ); + state.patches.set(key, atom); + } + return atom; + }, + }, +})); + +import { useReviewFilePatches } from "./useReviewFilePatches"; + +function source(paths: string[], hash: string): ReviewDiffPreviewSource { + return { + id: "working-tree", + kind: "working-tree", + title: "Uncommitted", + baseRef: "HEAD", + headRef: null, + diff: "", + diffHash: hash, + truncated: false, + patchesDeferred: true, + files: paths.map((path) => ({ path, previousPath: null, additions: 1, deletions: 1 })), + }; +} + +function Probe({ source }: { source: ReviewDiffPreviewSource }) { + const files = useReviewFilePatches({ + environmentId: EnvironmentId.make("environment"), + cwd: "/project", + source, + baseRef: "HEAD", + ignoreWhitespace: false, + theme: "dark", + previewInput: { cwd: "/project", metadataOnly: true }, + preview: null, + }); + return ( + <> + + {files.renderableFiles.map((file) => ( +
+ {file.name}:{file.hunks.length ? "loaded" : "pending"} +
+ ))} + + ); +} + +it("retains an opened later file through metadata insertion, refresh failure and retry", async () => { + state.patches.clear(); + const registry = AtomRegistry.make(); + using _registry = { [Symbol.dispose]: () => registry.dispose() }; + const tracked = source(["b.txt", "c.txt", "d.txt", "e.txt", "f.txt"], "tracked"); + const view = (source: ReviewDiffPreviewSource) => ( + + + + ); + await using screen = await render(view(tracked)); + await userEvent.click(screen.getByRole("button", { name: "Open f" })); + const atom = [...state.patches].find(([key]) => JSON.parse(key)[1] === "f.txt")![1]; + const loaded = AsyncResult.success({ + ...tracked, + diff: "diff --git a/f.txt b/f.txt\n--- a/f.txt\n+++ b/f.txt\n@@ -1 +1 @@\n-old\n+new\n", + files: tracked.files!.filter((f) => f.path === "f.txt"), + }); + act(() => registry.set(atom, loaded)); + await expect.element(screen.getByText("f.txt:loaded")).toBeVisible(); + await screen.rerender( + view(source(["a-new.txt", ...tracked.files!.map((f) => f.path)], "complete")), + ); + await expect.element(screen.getByText("f.txt:loaded")).toBeVisible(); + act(() => + registry.set( + atom, + AsyncResult.fail(new Error("refresh failed"), { previousSuccess: Option.some(loaded) }), + ), + ); + await expect.element(screen.getByText("f.txt:loaded")).toBeVisible(); + act(() => registry.set(atom, loaded)); + await expect.element(screen.getByText("f.txt:loaded")).toBeVisible(); +}); diff --git a/apps/web/src/components/diffs/useReviewFilePatches.ts b/apps/web/src/components/diffs/useReviewFilePatches.ts index 69dafc2dd7823..82907050c382e 100644 --- a/apps/web/src/components/diffs/useReviewFilePatches.ts +++ b/apps/web/src/components/diffs/useReviewFilePatches.ts @@ -5,6 +5,7 @@ import type { ReviewDiffPreviewInput, ReviewDiffPreviewSource, } from "@t3tools/contracts"; +import * as Option from "effect/Option"; import * as AsyncResult from "effect/reactivity/AsyncResult"; import * as Atom from "effect/reactivity/Atom"; import { useContext, useState } from "react"; @@ -16,7 +17,7 @@ const filePathCollator = new Intl.Collator(undefined, { numeric: true, sensitivity: "base", }); -const DEFAULT_INDICES = [0, 1, 2, 3]; +const INITIAL_FILE_COUNT = 4; export function useReviewFilePatches({ environmentId, @@ -43,11 +44,12 @@ export function useReviewFilePatches({ environmentId, cwd, source?.kind, - source?.diffHash, + source?.baseRef, + source?.headRef, baseRef, ignoreWhitespace, ]); - const [requested, setRequested] = useState(() => ({ scope, indices: DEFAULT_INDICES })); + const [requested, setRequested] = useState(() => ({ scope, paths: [] as string[] })); const queries = reviewFileQueries({ environmentId, cwd, @@ -81,9 +83,12 @@ export function useReviewFilePatches({ : 0; const requestFiles = (indices: number[]) => setRequested((current) => { - const previous = current.scope === scope ? current.indices : [0, 1, 2, 3]; - const added = indices.filter((index) => !previous.includes(index)); - return added.length === 0 ? current : { scope, indices: [...previous, ...added] }; + const previous = current.scope === scope ? current.paths : []; + const added = indices.flatMap((index) => { + const path = files[index]?.path; + return path !== undefined && !previous.includes(path) ? [path] : []; + }); + return added.length === 0 ? current : { scope, paths: [...previous, ...added] }; }); // The first unsettled file already requested means a batch is in flight; wait for it. const loadNextFiles = () => { @@ -94,8 +99,9 @@ export function useReviewFilePatches({ const renderableFiles = source ? files.map((file, index): FileDiffMetadata => { const result = patches.get(index); - if (result?._tag === "Success" && result.value.patch?.kind === "files") { - const loaded = result.value.patch.files.find( + const value = result && Option.getOrUndefined(AsyncResult.value(result)); + if (value?.patch?.kind === "files") { + const loaded = value.patch.files.find( (candidate) => resolveFileDiffPath(candidate) === file.path, ); if (loaded) return loaded; @@ -136,21 +142,11 @@ export function useReviewFilePatches({ ] as const; }), ); - const readyFilePaths = new Set( - files - .filter((_, index) => { - const patch = patches.get(index); - return patch && patch._tag !== "Initial"; - }) - .map((file) => file.path), - ); return { - scope, fileStates, isPending: [...patches.values()].some((patch) => patch._tag === "Initial" || patch.waiting), retry, requestFile, - readyFilePaths, renderableFiles, settledFileCount, loadNextFiles, @@ -174,17 +170,19 @@ function reviewFileQueries({ ignoreWhitespace: boolean; previewInput: ReviewDiffPreviewInput | undefined; scope: string; - requested: { scope: string; indices: number[] }; + requested: { scope: string; paths: string[] }; }) { - const indices = requested.scope === scope ? requested.indices : DEFAULT_INDICES; const files = sortedReviewFiles(source); + const paths = new Set([ + ...files.slice(0, INITIAL_FILE_COUNT).map((file) => file.path), + ...(requested.scope === scope ? requested.paths : []), + ]); const queries = !environmentId || !cwd || !source ? [] - : indices - .filter((index) => index < files.length) - .map((index) => { - const file = files[index]!; + : files + .flatMap((file, index) => (paths.has(file.path) ? [{ file, index }] : [])) + .map(({ file, index }) => { const target: ReviewPatchTarget = { environmentId, input: { diff --git a/packages/client-runtime/src/state/review.ts b/packages/client-runtime/src/state/review.ts index 296b08135f57f..a374e51b1df89 100644 --- a/packages/client-runtime/src/state/review.ts +++ b/packages/client-runtime/src/state/review.ts @@ -38,8 +38,16 @@ export function createReviewEnvironmentAtoms( execute: Effect.fn("review.diffPreview")(function* ( input: ReviewDiffPreviewInput, emit: (value: ReviewDiffPreviewResult) => Effect.Effect, + previous: Option.Option, ) { - if (input.sourceKind && !input.file && !input.commitSha && !input.trackedOnly) { + const hasCompletePreview = Option.exists(previous, (preview) => !preview.incomplete); + if ( + !hasCompletePreview && + input.sourceKind && + !input.file && + !input.commitSha && + !input.trackedOnly + ) { const tracked = yield* request(WS_METHODS.reviewGetDiffPreview, { ...input, trackedOnly: true, diff --git a/packages/client-runtime/src/state/runtime.test.ts b/packages/client-runtime/src/state/runtime.test.ts index 519ccd025b070..04b6ec570dbc7 100644 --- a/packages/client-runtime/src/state/runtime.test.ts +++ b/packages/client-runtime/src/state/runtime.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "@effect/vitest"; import { EnvironmentId, + type ReviewDiffPreviewInput, type PreviewEvent, type TerminalAttachInput, type TerminalAttachStreamEvent, @@ -9,6 +10,7 @@ import { WS_METHODS, } from "@t3tools/contracts"; import * as Cause from "effect/Cause"; +import * as DateTime from "effect/DateTime"; import * as Deferred from "effect/Deferred"; import * as Effect from "effect/Effect"; import * as Exit from "effect/Exit"; @@ -33,6 +35,7 @@ import * as EnvironmentRegistry from "../connection/registry.ts"; import * as EnvironmentSupervisor from "../connection/supervisor.ts"; import { EnvironmentRpcUnavailableError } from "../rpc/client.ts"; import type * as RpcSession from "../rpc/session.ts"; +import { createReviewEnvironmentAtoms } from "./review.ts"; import { createPreviewEnvironmentAtoms } from "./preview.ts"; import type { QueryPersistence } from "./queryPersistence.ts"; import { createTerminalEnvironmentAtoms } from "./terminal.ts"; @@ -184,6 +187,63 @@ const mountEnvironmentQuery = Effect.fn("TestEnvironmentQuery.mount")(function* return registry; }); +it.effect("keeps complete review metadata visible while a refresh is held", () => + Effect.scoped( + Effect.gen(function* () { + const started = yield* Deferred.make(); + const release = yield* Deferred.make(); + let fullReads = 0; + const rpcSession = { + client: { + [WS_METHODS.reviewGetDiffPreview]: (input: ReviewDiffPreviewInput) => + Effect.gen(function* () { + if (!input.trackedOnly && ++fullReads > 1) { + yield* Deferred.succeed(started, undefined); + yield* Deferred.await(release); + } + return { + cwd: input.cwd, + generatedAt: DateTime.makeUnsafe(0), + ...(input.trackedOnly ? { incomplete: true } : {}), + sources: [ + { + id: "working-tree", + kind: "working-tree" as const, + title: "Uncommitted", + baseRef: "HEAD", + headRef: null, + diff: "", + diffHash: String(fullReads), + truncated: false, + patchesDeferred: true, + files: (input.trackedOnly ? ["tracked"] : ["tracked", "untracked"]).map( + (path) => ({ path, previousPath: null, additions: 1, deletions: 0 }), + ), + }, + ], + }; + }), + }, + } as unknown as RpcSession.RpcSession; + const harness = yield* makeEnvironmentQueryHarness(Effect.void, { rpcSession }); + const atom = createReviewEnvironmentAtoms(harness.runtime).diffPreview({ + environmentId: QUERY_ENVIRONMENT.environmentId, + input: { cwd: "/project", sourceKind: "working-tree", metadataOnly: true }, + }); + const registry = yield* mountEnvironmentQuery(atom); + expect((yield* readEnvironmentQuery(registry, atom)).sources[0]!.files).toHaveLength(2); + registry.refresh(atom); + yield* Deferred.await(started); + yield* Effect.sync(() => { + const value = AsyncResult.getOrElse(registry.get(atom), () => null); + expect(value?.incomplete).not.toBe(true); + expect(value?.sources[0]!.files).toHaveLength(2); + }).pipe(Effect.ensuring(Deferred.succeed(release, undefined))); + expect((yield* readEnvironmentQuery(registry, atom)).sources[0]!.diffHash).toBe("2"); + }), + ), +); + describe("preview event subscriptions", () => { it.effect("delivers every open and close when RPC events arrive in batches", () => Effect.scoped( diff --git a/packages/client-runtime/src/state/runtime.ts b/packages/client-runtime/src/state/runtime.ts index fde1790d338a3..58c37070fd8d6 100644 --- a/packages/client-runtime/src/state/runtime.ts +++ b/packages/client-runtime/src/state/runtime.ts @@ -91,6 +91,7 @@ interface EnvironmentQueryAtomOptions extends Omit< readonly execute: ( input: Input, emit: (value: A) => Effect.Effect, + previous: Option.Option, ) => Effect.Effect; readonly persistence?: QueryPersistence; readonly staleTimeMs?: number; @@ -713,7 +714,14 @@ export function createEnvironmentQueryAtomFamily( ? runInEnvironment( target.environmentId, options - .execute(target.input, emit) + .execute( + target.input, + emit, + Option.flatMap( + get.self>(), + AsyncResult.value, + ), + ) .pipe(pendingOnceSessionReplaced(session.value)), ) : Effect.never; @@ -864,6 +872,7 @@ export function createEnvironmentRpcQueryAtomFamily< readonly execute?: ( input: EnvironmentRpcInput, emit: (value: EnvironmentRpcSuccess) => Effect.Effect, + previous: Option.Option>, ) => Effect.Effect< EnvironmentRpcSuccess, EnvironmentRpcFailure | EnvironmentRpcUnavailableError, @@ -895,7 +904,8 @@ export function createEnvironmentRpcQueryAtomFamily< execute: ( input: EnvironmentRpcInput, emit: (value: EnvironmentRpcSuccess) => Effect.Effect, - ) => options.execute?.(input, emit) ?? request(options.tag, input), + previous: Option.Option>, + ) => options.execute?.(input, emit, previous) ?? request(options.tag, input), }); } diff --git a/packages/contracts/src/review.ts b/packages/contracts/src/review.ts index 080619443ce58..2608e635ba906 100644 --- a/packages/contracts/src/review.ts +++ b/packages/contracts/src/review.ts @@ -10,6 +10,7 @@ export const ReviewDiffPreviewInput = Schema.Struct({ ignoreWhitespace: Schema.optionalKey(Schema.Boolean), sourceKind: Schema.optionalKey(Schema.Literals(["working-tree", "branch-range"])), trackedOnly: Schema.optionalKey(Schema.Boolean), + metadataOnly: Schema.optionalKey(Schema.Boolean), file: Schema.optionalKey( Schema.Struct({ path: Schema.NonEmptyString, @@ -63,6 +64,7 @@ export const ReviewDiffPreviewSource = Schema.Struct({ diff: Schema.String, diffHash: TrimmedNonEmptyString, truncated: Schema.Boolean, + patchesDeferred: Schema.optionalKey(Schema.Boolean), files: Schema.optionalKey(Schema.Array(ReviewDiffFileStat)), }); export type ReviewDiffPreviewSource = typeof ReviewDiffPreviewSource.Type; From bd8c34c4577d11cb9eba91d9555e061a51e5979c Mon Sep 17 00:00:00 2001 From: Yash Singh Date: Tue, 3 Nov 2026 21:26:52 -0600 Subject: [PATCH 3/5] fix(web): retain automatic diff reads and show empty comparisons --- .../web/src/components/DiffPanel.browser.test.tsx | 2 ++ apps/web/src/components/DiffPanel.tsx | 7 +++++-- .../diffs/useReviewFilePatches.browser.test.tsx | 15 ++++++++++++++- .../src/components/diffs/useReviewFilePatches.ts | 10 ++++++++-- 4 files changed, 29 insertions(+), 5 deletions(-) diff --git a/apps/web/src/components/DiffPanel.browser.test.tsx b/apps/web/src/components/DiffPanel.browser.test.tsx index 505c11130d692..4f5475218d153 100644 --- a/apps/web/src/components/DiffPanel.browser.test.tsx +++ b/apps/web/src/components/DiffPanel.browser.test.tsx @@ -101,6 +101,8 @@ function preview(): ReviewDiffPreviewResult { diff: "", diffHash: "empty", truncated: false, + patchesDeferred: true, + files: [], }, ], }; diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx index ee5372ae5fa30..f025a0e268fc7 100644 --- a/apps/web/src/components/DiffPanel.tsx +++ b/apps/web/src/components/DiffPanel.tsx @@ -484,7 +484,10 @@ export default function DiffPanel({ : selectedTurn ? activeCheckpointDiff : branchDiffPreview; - const isLoadingSelectedPatch = selectedPatchQuery.isPending && selectedPatchQuery.data === null; + const isLoadingSelectedPatch = + selectedPatchQuery.isPending && + (selectedPatchQuery.data === null || + (isGitSelection && preview?.incomplete === true && selectedGitSource?.files?.length === 0)); const selectedPatchError = toolCallSelection ? toolCallDetail.error : selectedTurn @@ -499,7 +502,7 @@ export default function DiffPanel({ const lazySource = isGitSelection && (selectedGitSource?.truncated || selectedGitSource?.patchesDeferred) && - selectedGitSource.files + selectedGitSource.files?.length ? selectedGitSource : null; const renderablePatch = getRenderablePatch( diff --git a/apps/web/src/components/diffs/useReviewFilePatches.browser.test.tsx b/apps/web/src/components/diffs/useReviewFilePatches.browser.test.tsx index cfbf3c5a53d55..ee2ba493a5875 100644 --- a/apps/web/src/components/diffs/useReviewFilePatches.browser.test.tsx +++ b/apps/web/src/components/diffs/useReviewFilePatches.browser.test.tsx @@ -69,7 +69,7 @@ function Probe({ source }: { source: ReviewDiffPreviewSource }) { ); } -it("retains an opened later file through metadata insertion, refresh failure and retry", async () => { +it("retains automatic and explicitly opened files through metadata insertion, refresh failure and retry", async () => { state.patches.clear(); const registry = AtomRegistry.make(); using _registry = { [Symbol.dispose]: () => registry.dispose() }; @@ -89,9 +89,22 @@ it("retains an opened later file through metadata insertion, refresh failure and }); act(() => registry.set(atom, loaded)); await expect.element(screen.getByText("f.txt:loaded")).toBeVisible(); + const automatic = [...state.patches].find(([key]) => JSON.parse(key)[1] === "e.txt")![1]; + act(() => + registry.set( + automatic, + AsyncResult.success({ + ...loaded.value, + diff: loaded.value.diff.replaceAll("f.txt", "e.txt"), + files: tracked.files!.filter((file) => file.path === "e.txt"), + }), + ), + ); + await expect.element(screen.getByText("e.txt:loaded")).toBeVisible(); await screen.rerender( view(source(["a-new.txt", ...tracked.files!.map((f) => f.path)], "complete")), ); + await expect.element(screen.getByText("e.txt:loaded")).toBeVisible(); await expect.element(screen.getByText("f.txt:loaded")).toBeVisible(); act(() => registry.set( diff --git a/apps/web/src/components/diffs/useReviewFilePatches.ts b/apps/web/src/components/diffs/useReviewFilePatches.ts index 82907050c382e..36c5888e75e69 100644 --- a/apps/web/src/components/diffs/useReviewFilePatches.ts +++ b/apps/web/src/components/diffs/useReviewFilePatches.ts @@ -49,7 +49,14 @@ export function useReviewFilePatches({ baseRef, ignoreWhitespace, ]); - const [requested, setRequested] = useState(() => ({ scope, paths: [] as string[] })); + const files = sortedReviewFiles(source); + const automaticPaths = files.slice(0, INITIAL_FILE_COUNT).map((file) => file.path); + const [requested, setRequested] = useState(() => ({ scope, paths: automaticPaths })); + const previousPaths = requested.scope === scope ? requested.paths : []; + const paths = [...new Set([...previousPaths, ...automaticPaths])]; + if (requested.scope !== scope || paths.length !== previousPaths.length) { + setRequested({ scope, paths }); + } const queries = reviewFileQueries({ environmentId, cwd, @@ -69,7 +76,6 @@ export function useReviewFilePatches({ const query = queries.find(({ index }) => files[index]?.path === path)?.query; if (query) registry.refresh(query); }); - const files = sortedReviewFiles(source); const pendingIndex = files.findIndex((_, index) => { const patch = patches.get(index); return !patch || patch._tag === "Initial"; From e9b07f623a628b923ca725037ab7fb700ed47cae Mon Sep 17 00:00:00 2001 From: Yash Singh Date: Tue, 3 Nov 2026 21:51:20 -0600 Subject: [PATCH 4/5] fix(web): preserve both patches for file type changes --- apps/web/src/components/DiffPanel.tsx | 24 ++++------- .../useReviewFilePatches.browser.test.tsx | 37 ++++++++++++++--- .../components/diffs/useReviewFilePatches.ts | 41 ++++++++++--------- 3 files changed, 61 insertions(+), 41 deletions(-) diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx index f025a0e268fc7..3c2d3302a50c8 100644 --- a/apps/web/src/components/DiffPanel.tsx +++ b/apps/web/src/components/DiffPanel.tsx @@ -599,7 +599,7 @@ export default function DiffPanel({ retry, requestFile, renderableFiles, - settledFileCount, + remainingFileCount, loadNextFiles, } = useReviewFilePatches({ environmentId: activeThread?.environmentId, @@ -672,11 +672,8 @@ export default function DiffPanel({ .updateCollapsedFileKeys(collapseScopeKey, defaultCollapsedDiffFileKeys, update); }; const renderLoadingBoundary = () => - settledFileCount < renderableFiles.length ? ( - + remainingFileCount > 0 ? ( + ) : null; const codeViewFiles = shownFileEntries.map(({ fileDiff, fileKey, fileVersion }) => ({ fileDiff, @@ -704,9 +701,7 @@ export default function DiffPanel({ next.delete(file.fileKey); return next; }); - if (lazySource && index >= settledFileCount) { - requestFile(index); - } + if (lazySource) requestFile(filePath); codeViewReveal.request(file.fileKey); return true; }; @@ -753,8 +748,8 @@ export default function DiffPanel({ }); }; const toggleDiffFileCollapsed = useStableCallback((fileKey: string) => { - const index = renderableFileEntries.findIndex((file) => file.fileKey === fileKey); - if (lazySource && index >= 0) requestFile(index); + const file = renderableFileEntries.find((file) => file.fileKey === fileKey); + if (lazySource && file) requestFile(resolveFileDiffPath(file.fileDiff)); updateCollapsedDiffFiles((current) => { const next = new Set(current); if (next.has(fileKey)) { @@ -1476,12 +1471,7 @@ export default function DiffPanel({ onToggle={toggleDiffFileCollapsed} {...(fileDiff.cacheKey?.endsWith(":pending") ? { - load: () => - requestFile( - renderableFileEntries.findIndex( - (file) => file.fileKey === fileKey, - ), - ), + load: () => requestFile(resolveFileDiffPath(fileDiff)), } : {})} /> diff --git a/apps/web/src/components/diffs/useReviewFilePatches.browser.test.tsx b/apps/web/src/components/diffs/useReviewFilePatches.browser.test.tsx index ee2ba493a5875..d6a0622ea474a 100644 --- a/apps/web/src/components/diffs/useReviewFilePatches.browser.test.tsx +++ b/apps/web/src/components/diffs/useReviewFilePatches.browser.test.tsx @@ -57,12 +57,14 @@ function Probe({ source }: { source: ReviewDiffPreviewSource }) { }); return ( <> - + + Pending files: {files.remainingFileCount} {files.renderableFiles.map((file) => ( -
- {file.name}:{file.hunks.length ? "loaded" : "pending"} +
+ + {file.name}:{file.hunks.length ? "loaded" : "pending"} + + {file.type}
))} @@ -116,3 +118,28 @@ it("retains automatic and explicitly opened files through metadata insertion, re act(() => registry.set(atom, loaded)); await expect.element(screen.getByText("f.txt:loaded")).toBeVisible(); }); + +it("preserves deletion and addition patches for a file replaced by a symlink", async () => { + state.patches.clear(); + const registry = AtomRegistry.make(); + using _registry = { [Symbol.dispose]: () => registry.dispose() }; + const metadata = source(["entry"], "type-change"); + await using screen = await render( + + + , + ); + const atom = [...state.patches.values()][0]!; + act(() => + registry.set( + atom, + AsyncResult.success({ + ...metadata, + diff: "diff --git a/entry b/entry\ndeleted file mode 100644\n--- a/entry\n+++ /dev/null\n@@ -1 +0,0 @@\n-before\ndiff --git a/entry b/entry\nnew file mode 120000\n--- /dev/null\n+++ b/entry\n@@ -0,0 +1 @@\n+target\n\\ No newline at end of file\n", + }), + ), + ); + await expect.element(screen.getByText("deleted", { exact: true })).toBeVisible(); + await expect.element(screen.getByText("new", { exact: true })).toBeVisible(); + await expect.element(screen.getByText("Pending files: 0")).toBeVisible(); +}); diff --git a/apps/web/src/components/diffs/useReviewFilePatches.ts b/apps/web/src/components/diffs/useReviewFilePatches.ts index 36c5888e75e69..728f43433b010 100644 --- a/apps/web/src/components/diffs/useReviewFilePatches.ts +++ b/apps/web/src/components/diffs/useReviewFilePatches.ts @@ -101,31 +101,34 @@ export function useReviewFilePatches({ if (queries.some(({ index }) => index === settledFileCount)) return; requestFiles(Array.from({ length: 4 }, (_, index) => settledFileCount + index)); }; - const requestFile = (index: number) => requestFiles([index]); + const requestFile = (path: string) => + requestFiles([files.findIndex((file) => file.path === path)]); const renderableFiles = source - ? files.map((file, index): FileDiffMetadata => { + ? files.flatMap((file, index): FileDiffMetadata[] => { const result = patches.get(index); const value = result && Option.getOrUndefined(AsyncResult.value(result)); if (value?.patch?.kind === "files") { - const loaded = value.patch.files.find( + const loaded = value.patch.files.filter( (candidate) => resolveFileDiffPath(candidate) === file.path, ); - if (loaded) return loaded; + if (loaded.length > 0) return loaded; } - return { - name: file.path, - ...(file.previousPath ? { prevName: file.previousPath } : {}), - type: file.previousPath ? "rename-changed" : "change", - hunks: [], - additionLines: [], - deletionLines: [], - additionCharacterCount: 0, - deletionCharacterCount: 0, - splitLineCount: 0, - unifiedLineCount: 0, - isPartial: true, - cacheKey: `${scope}:${file.path}:pending`, - }; + return [ + { + name: file.path, + ...(file.previousPath ? { prevName: file.previousPath } : {}), + type: file.previousPath ? "rename-changed" : "change", + hunks: [], + additionLines: [], + deletionLines: [], + additionCharacterCount: 0, + deletionCharacterCount: 0, + splitLineCount: 0, + unifiedLineCount: 0, + isPartial: true, + cacheKey: `${scope}:${file.path}:pending`, + }, + ]; }) : (preview?.kind === "files" ? preview.files : []).toSorted((a, b) => filePathCollator.compare(resolveFileDiffPath(a), resolveFileDiffPath(b)), @@ -154,7 +157,7 @@ export function useReviewFilePatches({ retry, requestFile, renderableFiles, - settledFileCount, + remainingFileCount: source ? files.length - settledFileCount : 0, loadNextFiles, }; } From b4dcf6476ca4f45f310f0c1fde084e91c2e47610 Mon Sep 17 00:00:00 2001 From: Yash Singh Date: Tue, 3 Nov 2026 22:08:34 -0600 Subject: [PATCH 5/5] fix(web): expand pending diff files on the first click --- .../src/components/DiffPanel.browser.test.tsx | 62 +++++++++++++++++-- apps/web/src/components/DiffPanel.tsx | 12 ++-- 2 files changed, 64 insertions(+), 10 deletions(-) diff --git a/apps/web/src/components/DiffPanel.browser.test.tsx b/apps/web/src/components/DiffPanel.browser.test.tsx index 4f5475218d153..3157c3e5da2a7 100644 --- a/apps/web/src/components/DiffPanel.browser.test.tsx +++ b/apps/web/src/components/DiffPanel.browser.test.tsx @@ -8,6 +8,7 @@ import { type ClientSettings, type ReviewBranchCommitsResult, type ReviewDiffPreviewResult, + type ReviewDiffPreviewSource, type ScopedThreadRef, type VcsListRefsResult, type VcsStatusResult, @@ -18,7 +19,7 @@ import { expect, it, vi } from "vite-plus/test"; import { page, userEvent } from "vite-plus/test/browser"; import "~/index.css"; -import { render } from "../../test/render"; +import { act, render } from "../../test/render"; import { makeThreadFixture } from "~/test-fixtures"; type ResultAtom = Atom.Atom>; @@ -27,6 +28,8 @@ const state = vi.hoisted(() => ({ thread: null as ReturnType | null, threadRef: null as ScopedThreadRef | null, previews: new Map>(), + patch: null as unknown as ResultAtom, + diffFilesCollapsed: false, config: null as unknown as Atom.Atom, status: null as unknown as ResultAtom, localRefs: null as unknown as ResultAtom, @@ -42,6 +45,7 @@ vi.mock("~/state/review", () => ({ diffPreview: ({ input }: { input: { baseRef?: string } }) => state.previews.get(input.baseRef ?? null)!, branchCommits: () => state.commits, + diffFilePatch: () => state.patch, diffFileContents: {}, }, })); @@ -64,10 +68,14 @@ vi.mock("~/state/filesystem", () => ({ vi.mock("~/hooks/useThreadRouteTarget", () => ({ useThreadRouteRef: () => state.threadRef })); vi.mock("~/hooks/useSettings", () => ({ useClientSettings: (select: (settings: ClientSettings) => unknown) => - select(DEFAULT_CLIENT_SETTINGS), + select({ ...DEFAULT_CLIENT_SETTINGS, diffFilesCollapsed: state.diffFilesCollapsed }), updateClientSettings: async () => {}, })); -vi.mock("~/hooks/useTheme", () => ({ useRenderedAppearance: () => "dark" })); +vi.mock("~/hooks/useTheme", () => ({ + useRenderedAppearance: () => "dark", + getRenderedAppearance: () => "dark", + subscribeRenderedAppearance: () => () => {}, +})); vi.mock("~/state/queries", () => ({ useCheckpointDiff: () => ({ data: null, error: null, isPending: false }), useTurnItemDetail: () => ({ data: null, error: null, isPending: false }), @@ -76,8 +84,6 @@ vi.mock("~/state/use-atom-command", () => ({ useAtomCommand: () => async () => { vi.mock("~/editorPreferences", () => ({ useOpenInPreferredEditor: () => async () => {} })); vi.mock("~/fileContextMenu", () => ({ useFileContextMenuHandler: () => () => {} })); vi.mock("~/browser/useOpenLink", () => ({ useOpenLink: () => async () => {} })); -// The test drives comparison controls, without mounting virtualized diff viewers or their workers. -vi.mock("./diffs/AnnotatableCodeView", () => ({ AnnotatableCodeView: () => null })); vi.mock("./diffs/DiffFileTree", () => ({ DiffFileTree: () => null })); vi.mock("./DiffFilePathCopyButton", () => ({ DiffFilePathCopyButton: () => null })); @@ -118,6 +124,11 @@ function setup(failedBaseRef: string | null) { useDiffPanelSessionStore.setState({ collapsedFileKeysByScopeKey: {} }); }; reset(); + state.diffFilesCollapsed = false; + const patch = Atom.make>( + AsyncResult.initial(), + ); + state.patch = patch; state.threadRef = { environmentId: EnvironmentId.make("environment"), threadId: ThreadId.make("thread"), @@ -169,6 +180,7 @@ function setup(failedBaseRef: string | null) { const registry = AtomRegistry.make(); return { registry, + patch, threadRef: state.threadRef, [Symbol.dispose]() { registry.dispose(); @@ -214,3 +226,43 @@ it.each([ }); }, ); + +it.each([false, true])( + "expands an unloaded file on the first click with default collapse %s", + async (collapsed) => { + using context = setup("origin/orphan"); + state.diffFilesCollapsed = collapsed; + const path = `pending-${collapsed}.txt`; + const metadata = preview(); + const source = { + ...metadata.sources[0]!, + files: [{ path, previousPath: null, additions: 1, deletions: 1 }], + }; + state.previews.set("main", Atom.make(AsyncResult.success({ ...metadata, sources: [source] }))); + useDiffPanelStore.getState().selectBranchBaseRef(context.threadRef, "main"); + await using screen = await render( + +
+ +
+ , + ); + const toggle = screen.getByRole("button", { name: `Expand ${path}`, exact: true }); + await expect.element(toggle).toHaveAttribute("aria-expanded", "false"); + await userEvent.click(toggle); + act(() => + context.registry.set( + context.patch, + AsyncResult.success({ + ...source, + diff: `diff --git a/${path} b/${path}\n--- a/${path}\n+++ b/${path}\n@@ -1 +1 @@\n-before\n+after\n`, + }), + ), + ); + const loadedToggle = screen.getByRole("button", { name: `Collapse ${path}`, exact: true }); + await expect.element(loadedToggle).toHaveAttribute("aria-expanded", "true"); + await expect.element(screen.getByText("after", { exact: true })).toBeVisible(); + await userEvent.click(loadedToggle); + await expect.element(toggle).toHaveAttribute("aria-expanded", "false"); + }, +); diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx index 3c2d3302a50c8..f40760f847ef0 100644 --- a/apps/web/src/components/DiffPanel.tsx +++ b/apps/web/src/components/DiffPanel.tsx @@ -156,7 +156,7 @@ function DiffFileCollapseToggle({ aria-label={collapsed ? `Expand ${filePath}` : `Collapse ${filePath}`} aria-expanded={!collapsed} ref={(node) => { - if (!node || !load || collapsed) return; + if (!node || !load) return; return observeIntersection( node, (entry) => { @@ -750,9 +750,10 @@ export default function DiffPanel({ const toggleDiffFileCollapsed = useStableCallback((fileKey: string) => { const file = renderableFileEntries.find((file) => file.fileKey === fileKey); if (lazySource && file) requestFile(resolveFileDiffPath(file.fileDiff)); + const pending = file?.fileDiff.cacheKey?.endsWith(":pending") === true; updateCollapsedDiffFiles((current) => { const next = new Set(current); - if (next.has(fileKey)) { + if (pending || next.has(fileKey)) { next.delete(fileKey); } else { next.add(fileKey); @@ -1462,14 +1463,15 @@ export default function DiffPanel({ } : {})} onRevealSearchMatch={unfoldDiffFile} - renderHeaderPrefix={(fileDiff, fileKey) => { + renderHeaderPrefix={(fileDiff, fileKey, collapsed) => { return ( requestFile(resolveFileDiffPath(fileDiff)), }