Skip to content

Commit e7b3924

Browse files
authored
Restore patch-only diff review state (#151)
* Remove per-file diff context toggle - Drop stored patch/full context mode from diff review state - Simplify diff panel rendering to always use patch context - Update review-state tests for the slimmer state shape * Add @pierre/diffs as a workspace dependency - Add the diff viewer package to the monorepo - Refresh lockfile to reflect the new dependency set
1 parent 4bedbb1 commit e7b3924

5 files changed

Lines changed: 61 additions & 205 deletions

File tree

apps/web/src/components/DiffPanel.tsx

Lines changed: 1 addition & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import { buildPatchCacheKey } from "../lib/diffRendering";
1616
import {
1717
expandDiffFile,
1818
reconcileDiffFileReviewState,
19-
setDiffFileContextMode,
2019
toggleDiffFileAccepted,
2120
toggleDiffFileCollapsed,
2221
type DiffFileReviewStateByPath,
@@ -168,92 +167,33 @@ function summarizeFileDiffStats(fileDiff: FileDiffMetadata): {
168167
);
169168
}
170169

171-
function resolveRenderableFileDiff(
172-
renderablePatch: RenderablePatch | null,
173-
filePath: string,
174-
): FileDiffMetadata | null {
175-
if (!renderablePatch || renderablePatch.kind !== "files") {
176-
return null;
177-
}
178-
return (
179-
renderablePatch.files.find((candidate) => resolveFileDiffPath(candidate) === filePath) ?? null
180-
);
181-
}
182-
183-
interface FileScopedCheckpointDiffInput {
184-
threadId: ThreadId | null;
185-
fromTurnCount: number | null;
186-
toTurnCount: number | null;
187-
cacheScope?: string | null;
188-
enabled: boolean;
189-
}
190-
191170
function DiffFileSection(props: {
192171
fileDiff: FileDiffMetadata;
193172
filePath: string;
194173
fileKey: string;
195-
checkpointDiffInput: FileScopedCheckpointDiffInput;
196174
diffRenderMode: DiffRenderMode;
197175
diffWordWrap: boolean;
198176
resolvedTheme: "light" | "dark";
199177
collapsed: boolean;
200178
accepted: boolean;
201-
contextMode: "patch" | "full";
202179
onOpenInEditor: (filePath: string) => void;
203180
onToggleCollapsed: (filePath: string) => void;
204181
onToggleAccepted: (filePath: string) => void;
205-
onContextModeChange: (filePath: string, contextMode: "patch" | "full") => void;
206182
}) {
207183
const {
208184
accepted,
209-
checkpointDiffInput,
210185
collapsed,
211-
contextMode,
212186
diffRenderMode,
213187
diffWordWrap,
214188
fileDiff,
215189
fileKey,
216190
filePath,
217-
onContextModeChange,
218191
onOpenInEditor,
219192
onToggleAccepted,
220193
onToggleCollapsed,
221194
resolvedTheme,
222195
} = props;
223196
const stats = summarizeFileDiffStats(fileDiff);
224-
const fullContextDiffQuery = useQuery(
225-
checkpointDiffQueryOptions({
226-
...checkpointDiffInput,
227-
relativePath: filePath,
228-
contextMode: "full",
229-
enabled: checkpointDiffInput.enabled && !collapsed && contextMode === "full",
230-
}),
231-
);
232-
const fullContextPatch = useMemo(
233-
() =>
234-
getRenderablePatch(
235-
contextMode === "full" ? fullContextDiffQuery.data?.diff : undefined,
236-
`diff-panel:file:${resolvedTheme}:${filePath}:full`,
237-
),
238-
[contextMode, filePath, fullContextDiffQuery.data?.diff, resolvedTheme],
239-
);
240-
const fullContextFileDiff =
241-
contextMode === "full" ? resolveRenderableFileDiff(fullContextPatch, filePath) : null;
242-
const resolvedFileDiff = contextMode === "full" ? (fullContextFileDiff ?? fileDiff) : fileDiff;
243-
const fullContextError =
244-
contextMode === "full" && fullContextDiffQuery.error
245-
? fullContextDiffQuery.error instanceof Error
246-
? fullContextDiffQuery.error.message
247-
: "Failed to load full-file context."
248-
: null;
249-
const fullContextFallbackMessage =
250-
contextMode === "full" &&
251-
!fullContextError &&
252-
!fullContextDiffQuery.isLoading &&
253-
fullContextDiffQuery.data &&
254-
fullContextFileDiff === null
255-
? "Full-file context is unavailable for this file. Showing patch context."
256-
: null;
257197

258198
return (
259199
<section
@@ -289,25 +229,6 @@ function DiffFileSection(props: {
289229
<DiffStatLabel additions={stats.additions} deletions={stats.deletions} />
290230
</span>
291231
)}
292-
<ToggleGroup
293-
className="shrink-0"
294-
variant="outline"
295-
size="xs"
296-
value={[contextMode]}
297-
onValueChange={(value) => {
298-
const next = value[0];
299-
if (next === "patch" || next === "full") {
300-
onContextModeChange(filePath, next);
301-
}
302-
}}
303-
>
304-
<Toggle aria-label={`Show patch diff for ${filePath}`} value="patch">
305-
Patch
306-
</Toggle>
307-
<Toggle aria-label={`Show full file context for ${filePath}`} value="full">
308-
Full
309-
</Toggle>
310-
</ToggleGroup>
311232
<Button
312233
size="xs"
313234
variant={accepted ? "secondary" : "outline"}
@@ -323,21 +244,8 @@ function DiffFileSection(props: {
323244
</div>
324245
{!collapsed && (
325246
<div key={fileKey}>
326-
{contextMode === "full" && fullContextDiffQuery.isLoading ? (
327-
<DiffPanelLoadingState label="Loading full file..." />
328-
) : null}
329-
{fullContextError ? (
330-
<div className="border-b border-border/60 bg-destructive/8 px-3 py-2 text-[11px] text-destructive/80">
331-
{fullContextError}
332-
</div>
333-
) : null}
334-
{fullContextFallbackMessage ? (
335-
<div className="border-b border-border/60 bg-amber-500/8 px-3 py-2 text-[11px] text-amber-700 dark:text-amber-300/90">
336-
{fullContextFallbackMessage}
337-
</div>
338-
) : null}
339247
<FileDiff
340-
fileDiff={resolvedFileDiff}
248+
fileDiff={fileDiff}
341249
options={{
342250
diffStyle: diffRenderMode === "split" ? "split" : "unified",
343251
lineDiffType: "none",
@@ -601,13 +509,6 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) {
601509
},
602510
[updateActiveReviewState],
603511
);
604-
const onChangeFileContextMode = useCallback(
605-
(filePath: string, contextMode: "patch" | "full") => {
606-
updateActiveReviewState((current) => setDiffFileContextMode(current, filePath, contextMode));
607-
},
608-
[updateActiveReviewState],
609-
);
610-
611512
const latestSelectedTurnId = orderedTurnDiffSummaries[0]?.turnId ?? null;
612513

613514
const selectTurn = useCallback(
@@ -791,29 +692,17 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) {
791692
const fileReviewState = activeReviewState[filePath] ?? {
792693
accepted: false,
793694
collapsed: true,
794-
contextMode: "patch" as const,
795695
};
796696
return (
797697
<DiffFileSection
798698
key={themedFileKey}
799699
accepted={fileReviewState.accepted}
800-
checkpointDiffInput={{
801-
threadId: activeThreadId,
802-
fromTurnCount: activeCheckpointRange?.fromTurnCount ?? null,
803-
toTurnCount: activeCheckpointRange?.toTurnCount ?? null,
804-
cacheScope: selectedTurn
805-
? `turn:${selectedTurn.turnId}`
806-
: conversationCacheScope,
807-
enabled: isGitRepo,
808-
}}
809700
collapsed={fileReviewState.collapsed}
810-
contextMode={fileReviewState.contextMode}
811701
diffRenderMode={diffRenderMode}
812702
diffWordWrap={diffWordWrap}
813703
fileDiff={fileDiff}
814704
fileKey={themedFileKey}
815705
filePath={filePath}
816-
onContextModeChange={onChangeFileContextMode}
817706
onOpenInEditor={openDiffFileInCodeViewer}
818707
onToggleAccepted={onToggleFileAccepted}
819708
onToggleCollapsed={onToggleFileCollapsed}

apps/web/src/lib/diffFileReviewState.test.ts

Lines changed: 13 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import { describe, expect, it } from "vitest";
22
import {
33
expandDiffFile,
44
reconcileDiffFileReviewState,
5-
setDiffFileContextMode,
65
toggleDiffFileAccepted,
76
toggleDiffFileCollapsed,
87
} from "./diffFileReviewState";
@@ -11,38 +10,38 @@ describe("reconcileDiffFileReviewState", () => {
1110
it("preserves existing state for known files and drops removed files", () => {
1211
expect(
1312
reconcileDiffFileReviewState(["src/a.ts"], {
14-
"src/a.ts": { accepted: true, collapsed: true, contextMode: "full" },
15-
"src/b.ts": { accepted: false, collapsed: true, contextMode: "patch" },
13+
"src/a.ts": { accepted: true, collapsed: true },
14+
"src/b.ts": { accepted: false, collapsed: true },
1615
}),
1716
).toEqual({
18-
"src/a.ts": { accepted: true, collapsed: true, contextMode: "full" },
17+
"src/a.ts": { accepted: true, collapsed: true },
1918
});
2019
});
2120

2221
it("initializes new files as unaccepted and collapsed", () => {
2322
expect(reconcileDiffFileReviewState(["src/a.ts"], undefined)).toEqual({
24-
"src/a.ts": { accepted: false, collapsed: true, contextMode: "patch" },
23+
"src/a.ts": { accepted: false, collapsed: true },
2524
});
2625
});
2726
});
2827

2928
describe("toggleDiffFileAccepted", () => {
3029
it("marks a file accepted and collapses it", () => {
3130
expect(toggleDiffFileAccepted({}, "src/a.ts")).toEqual({
32-
"src/a.ts": { accepted: true, collapsed: true, contextMode: "patch" },
31+
"src/a.ts": { accepted: true, collapsed: true },
3332
});
3433
});
3534

3635
it("clears acceptance and re-expands the file", () => {
3736
expect(
3837
toggleDiffFileAccepted(
3938
{
40-
"src/a.ts": { accepted: true, collapsed: true, contextMode: "full" },
39+
"src/a.ts": { accepted: true, collapsed: true },
4140
},
4241
"src/a.ts",
4342
),
4443
).toEqual({
45-
"src/a.ts": { accepted: false, collapsed: false, contextMode: "full" },
44+
"src/a.ts": { accepted: false, collapsed: false },
4645
});
4746
});
4847
});
@@ -52,42 +51,12 @@ describe("toggleDiffFileCollapsed", () => {
5251
expect(
5352
toggleDiffFileCollapsed(
5453
{
55-
"src/a.ts": { accepted: true, collapsed: true, contextMode: "full" },
54+
"src/a.ts": { accepted: true, collapsed: true },
5655
},
5756
"src/a.ts",
5857
),
5958
).toEqual({
60-
"src/a.ts": { accepted: true, collapsed: false, contextMode: "full" },
61-
});
62-
});
63-
});
64-
65-
describe("setDiffFileContextMode", () => {
66-
it("updates the file context mode without changing other state", () => {
67-
expect(
68-
setDiffFileContextMode(
69-
{
70-
"src/a.ts": { accepted: true, collapsed: false, contextMode: "patch" },
71-
},
72-
"src/a.ts",
73-
"full",
74-
),
75-
).toEqual({
76-
"src/a.ts": { accepted: true, collapsed: false, contextMode: "full" },
77-
});
78-
});
79-
80-
it("auto-expands a file when switching to full context", () => {
81-
expect(
82-
setDiffFileContextMode(
83-
{
84-
"src/a.ts": { accepted: false, collapsed: true, contextMode: "patch" },
85-
},
86-
"src/a.ts",
87-
"full",
88-
),
89-
).toEqual({
90-
"src/a.ts": { accepted: false, collapsed: false, contextMode: "full" },
59+
"src/a.ts": { accepted: true, collapsed: false },
9160
});
9261
});
9362
});
@@ -97,19 +66,19 @@ describe("expandDiffFile", () => {
9766
expect(
9867
expandDiffFile(
9968
{
100-
"src/a.ts": { accepted: true, collapsed: true, contextMode: "patch" },
69+
"src/a.ts": { accepted: true, collapsed: true },
10170
},
10271
"src/a.ts",
10372
),
10473
).toEqual({
105-
"src/a.ts": { accepted: true, collapsed: false, contextMode: "patch" },
74+
"src/a.ts": { accepted: true, collapsed: false },
10675
});
10776
});
10877

10978
it("returns the same object when the file is already expanded", () => {
11079
const state = {
111-
"src/a.ts": { accepted: false, collapsed: false, contextMode: "patch" as const },
112-
} satisfies Record<string, { accepted: boolean; collapsed: boolean; contextMode: "patch" }>;
80+
"src/a.ts": { accepted: false, collapsed: false },
81+
} satisfies Record<string, { accepted: boolean; collapsed: boolean }>;
11382
// File is already expanded, so the same object reference is returned.
11483
expect(expandDiffFile(state, "src/a.ts")).toBe(state);
11584
});

apps/web/src/lib/diffFileReviewState.ts

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
11
export interface DiffFileReviewState {
22
collapsed: boolean;
33
accepted: boolean;
4-
contextMode: "patch" | "full";
54
}
65

76
export type DiffFileReviewStateByPath = Record<string, DiffFileReviewState>;
87

98
const DEFAULT_DIFF_FILE_REVIEW_STATE: DiffFileReviewState = {
109
collapsed: true,
1110
accepted: false,
12-
contextMode: "patch",
1311
};
1412

1513
export function reconcileDiffFileReviewState(
@@ -34,7 +32,6 @@ export function toggleDiffFileAccepted(
3432
[path]: {
3533
accepted,
3634
collapsed: accepted,
37-
contextMode: previous.contextMode,
3835
},
3936
};
4037
}
@@ -53,25 +50,6 @@ export function toggleDiffFileCollapsed(
5350
};
5451
}
5552

56-
export function setDiffFileContextMode(
57-
current: DiffFileReviewStateByPath,
58-
path: string,
59-
contextMode: "patch" | "full",
60-
): DiffFileReviewStateByPath {
61-
const previous = current[path] ?? DEFAULT_DIFF_FILE_REVIEW_STATE;
62-
if (previous.contextMode === contextMode) {
63-
return current;
64-
}
65-
return {
66-
...current,
67-
[path]: {
68-
...previous,
69-
collapsed: contextMode === "full" ? false : previous.collapsed,
70-
contextMode,
71-
},
72-
};
73-
}
74-
7553
export function expandDiffFile(
7654
current: DiffFileReviewStateByPath,
7755
path: string,

0 commit comments

Comments
 (0)