Skip to content

Commit f9a735e

Browse files
authored
Add per-file full-context diff viewing (#138)
* Add file-scoped full-context diff viewing - Propagate relative file paths and context mode through diff queries - Add per-file patch/full toggle in the diff panel and persist review state - Extend orchestration contracts and tests for scoped full-context diffs * Omit unset diff query fields - Skip undefined checkpoint diff inputs when building queries - Tighten diff panel formatting and test typing * Auto-expand diff files and dedupe effect installs - Collapse removal now resets when switching a diff file to full context - Add Bun hoisted linker config and dedupe nested effect node_modules on prepare * Show fallback notice for unavailable full-file diffs - Display an amber notice when full-file context cannot be rendered - Simplify checkpoint diff query tests around patch vs full-context keys * Harden missing-translation error handling - Safely narrow `I18nProvider`'s `onError` input before checking `code` - Refresh Bun lockfile metadata and workspace versions
1 parent 927ebf8 commit f9a735e

7 files changed

Lines changed: 129 additions & 145 deletions

File tree

apps/web/src/components/DiffPanel.tsx

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -237,16 +237,23 @@ function DiffFileSection(props: {
237237
),
238238
[contextMode, filePath, fullContextDiffQuery.data?.diff, resolvedTheme],
239239
);
240-
const resolvedFileDiff =
241-
contextMode === "full"
242-
? (resolveRenderableFileDiff(fullContextPatch, filePath) ?? fileDiff)
243-
: fileDiff;
240+
const fullContextFileDiff =
241+
contextMode === "full" ? resolveRenderableFileDiff(fullContextPatch, filePath) : null;
242+
const resolvedFileDiff = contextMode === "full" ? (fullContextFileDiff ?? fileDiff) : fileDiff;
244243
const fullContextError =
245244
contextMode === "full" && fullContextDiffQuery.error
246245
? fullContextDiffQuery.error instanceof Error
247246
? fullContextDiffQuery.error.message
248247
: "Failed to load full-file context."
249248
: 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;
250257

251258
return (
252259
<section
@@ -324,6 +331,11 @@ function DiffFileSection(props: {
324331
{fullContextError}
325332
</div>
326333
) : 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}
327339
<FileDiff
328340
fileDiff={resolvedFileDiff}
329341
options={{

apps/web/src/i18n/I18nProvider.tsx

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,13 @@ export function I18nProvider({ children }: { children: ReactNode }) {
8888
locale={resolvedLocale}
8989
defaultLocale="en"
9090
messages={activeMessages}
91-
onError={(error) => {
92-
if ("code" in error && error.code === "MISSING_TRANSLATION") {
91+
onError={(error: unknown) => {
92+
if (
93+
typeof error === "object" &&
94+
error !== null &&
95+
"code" in error &&
96+
error.code === "MISSING_TRANSLATION"
97+
) {
9398
return;
9499
}
95100

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,20 @@ describe("setDiffFileContextMode", () => {
7676
"src/a.ts": { accepted: true, collapsed: false, contextMode: "full" },
7777
});
7878
});
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" },
91+
});
92+
});
7993
});
8094

8195
describe("expandDiffFile", () => {

apps/web/src/lib/diffFileReviewState.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ export function setDiffFileContextMode(
6666
...current,
6767
[path]: {
6868
...previous,
69+
collapsed: contextMode === "full" ? false : previous.collapsed,
6970
contextMode,
7071
},
7172
};
Lines changed: 27 additions & 139 deletions
Original file line numberDiff line numberDiff line change
@@ -1,161 +1,49 @@
1-
import { ThreadId, type NativeApi } from "@okcode/contracts";
2-
import { QueryClient } from "@tanstack/react-query";
3-
import { afterEach, describe, expect, it, vi } from "vitest";
4-
import { checkpointDiffQueryOptions, providerQueryKeys } from "./providerReactQuery";
5-
import * as nativeApi from "../nativeApi";
1+
import { describe, expect, it } from "vitest";
2+
import { ThreadId } from "@okcode/contracts";
3+
import { providerQueryKeys, checkpointDiffQueryOptions } from "./providerReactQuery";
64

7-
const threadId = ThreadId.makeUnsafe("thread-id");
8-
9-
function mockNativeApi(input: {
10-
getTurnDiff: ReturnType<typeof vi.fn>;
11-
getFullThreadDiff: ReturnType<typeof vi.fn>;
12-
}) {
13-
vi.spyOn(nativeApi, "ensureNativeApi").mockReturnValue({
14-
orchestration: {
15-
getTurnDiff: input.getTurnDiff,
16-
getFullThreadDiff: input.getFullThreadDiff,
17-
},
18-
} as unknown as NativeApi);
19-
}
20-
21-
afterEach(() => {
22-
vi.restoreAllMocks();
23-
});
5+
const threadId = ThreadId.makeUnsafe("thread-1");
246

257
describe("providerQueryKeys.checkpointDiff", () => {
26-
it("includes cacheScope so reused turn counts do not collide", () => {
27-
const baseInput = {
8+
it("distinguishes patch and full-context file queries", () => {
9+
const patchKey = providerQueryKeys.checkpointDiff({
2810
threadId,
2911
fromTurnCount: 1,
3012
toTurnCount: 2,
31-
} as const;
32-
33-
expect(
34-
providerQueryKeys.checkpointDiff({
35-
...baseInput,
36-
cacheScope: "turn:old-turn",
37-
}),
38-
).not.toEqual(
39-
providerQueryKeys.checkpointDiff({
40-
...baseInput,
41-
cacheScope: "turn:new-turn",
42-
}),
43-
);
44-
});
45-
});
46-
47-
describe("checkpointDiffQueryOptions", () => {
48-
it("forwards checkpoint range to the provider API", async () => {
49-
const getTurnDiff = vi.fn().mockResolvedValue({ diff: "patch" });
50-
const getFullThreadDiff = vi.fn().mockResolvedValue({ diff: "patch" });
51-
mockNativeApi({ getTurnDiff, getFullThreadDiff });
52-
53-
const options = checkpointDiffQueryOptions({
54-
threadId,
55-
fromTurnCount: 3,
56-
toTurnCount: 4,
57-
cacheScope: "turn:abc",
58-
});
59-
60-
const queryClient = new QueryClient();
61-
await queryClient.fetchQuery(options);
62-
63-
expect(getTurnDiff).toHaveBeenCalledWith({
64-
threadId,
65-
fromTurnCount: 3,
66-
toTurnCount: 4,
67-
});
68-
expect(getFullThreadDiff).not.toHaveBeenCalled();
69-
});
70-
71-
it("uses explicit full thread diff API when range starts from zero", async () => {
72-
const getTurnDiff = vi.fn().mockResolvedValue({ diff: "patch" });
73-
const getFullThreadDiff = vi.fn().mockResolvedValue({ diff: "patch" });
74-
mockNativeApi({ getTurnDiff, getFullThreadDiff });
75-
76-
const options = checkpointDiffQueryOptions({
77-
threadId,
78-
fromTurnCount: 0,
79-
toTurnCount: 2,
80-
cacheScope: "thread:all",
81-
});
82-
83-
const queryClient = new QueryClient();
84-
await queryClient.fetchQuery(options);
85-
86-
expect(getFullThreadDiff).toHaveBeenCalledWith({
87-
threadId,
88-
toTurnCount: 2,
89-
});
90-
expect(getTurnDiff).not.toHaveBeenCalled();
91-
});
92-
93-
it("fails fast on invalid range and does not call provider RPC", async () => {
94-
const getTurnDiff = vi.fn().mockResolvedValue({ diff: "patch" });
95-
const getFullThreadDiff = vi.fn().mockResolvedValue({ diff: "patch" });
96-
mockNativeApi({ getTurnDiff, getFullThreadDiff });
97-
98-
const options = checkpointDiffQueryOptions({
99-
threadId,
100-
fromTurnCount: 4,
101-
toTurnCount: 3,
102-
cacheScope: "turn:invalid",
13+
relativePath: "src/a.ts",
14+
contextMode: "patch",
10315
});
104-
105-
const queryClient = new QueryClient();
106-
107-
await expect(queryClient.fetchQuery(options)).rejects.toThrow(
108-
"Checkpoint diff is unavailable.",
109-
);
110-
expect(getTurnDiff).not.toHaveBeenCalled();
111-
expect(getFullThreadDiff).not.toHaveBeenCalled();
112-
});
113-
114-
it("retries checkpoint-not-ready errors longer than generic failures", () => {
115-
const options = checkpointDiffQueryOptions({
16+
const fullKey = providerQueryKeys.checkpointDiff({
11617
threadId,
11718
fromTurnCount: 1,
11819
toTurnCount: 2,
119-
cacheScope: "turn:abc",
20+
relativePath: "src/a.ts",
21+
contextMode: "full",
12022
});
121-
const retry = options.retry;
122-
expect(typeof retry).toBe("function");
123-
if (typeof retry !== "function") {
124-
throw new Error("Expected retry to be a function.");
125-
}
12623

127-
expect(retry(1, new Error("Checkpoint turn count 2 exceeds current turn count 1."))).toBe(true);
128-
expect(
129-
retry(11, new Error("Filesystem checkpoint is unavailable for turn 2 in thread thread-1.")),
130-
).toBe(true);
131-
expect(
132-
retry(12, new Error("Filesystem checkpoint is unavailable for turn 2 in thread thread-1.")),
133-
).toBe(false);
134-
expect(retry(2, new Error("Something else failed."))).toBe(true);
135-
expect(retry(3, new Error("Something else failed."))).toBe(false);
24+
expect(patchKey).not.toEqual(fullKey);
13625
});
26+
});
13727

138-
it("backs off longer for checkpoint-not-ready errors", () => {
28+
describe("checkpointDiffQueryOptions", () => {
29+
it("stays enabled for full-thread file-scoped full-context queries", () => {
13930
const options = checkpointDiffQueryOptions({
14031
threadId,
141-
fromTurnCount: 1,
32+
fromTurnCount: 0,
14233
toTurnCount: 2,
143-
cacheScope: "turn:abc",
34+
relativePath: "src/a.ts",
35+
contextMode: "full",
14436
});
145-
const retryDelay = options.retryDelay;
146-
expect(typeof retryDelay).toBe("function");
147-
if (typeof retryDelay !== "function") {
148-
throw new Error("Expected retryDelay to be a function.");
149-
}
15037

151-
const checkpointDelay = retryDelay(
152-
4,
153-
new Error("Checkpoint turn count 2 exceeds current turn count 1."),
38+
expect(options.queryKey).toEqual(
39+
providerQueryKeys.checkpointDiff({
40+
threadId,
41+
fromTurnCount: 0,
42+
toTurnCount: 2,
43+
relativePath: "src/a.ts",
44+
contextMode: "full",
45+
}),
15446
);
155-
const genericDelay = retryDelay(4, new Error("Network failure"));
156-
157-
expect(typeof checkpointDelay).toBe("number");
158-
expect(typeof genericDelay).toBe("number");
159-
expect((checkpointDelay ?? 0) > (genericDelay ?? 0)).toBe(true);
47+
expect(options.enabled).toBe(true);
16048
});
16149
});

bunfig.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
[install]
2+
linker = "hoisted"
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
import fs from "node:fs/promises";
2+
import path from "node:path";
3+
import { fileURLToPath } from "node:url";
4+
5+
const scriptDir = path.dirname(fileURLToPath(import.meta.url));
6+
const repoRoot = path.resolve(scriptDir, "..");
7+
const effectRoot = path.join(repoRoot, "node_modules", "effect");
8+
const effectScopeDir = path.join(repoRoot, "node_modules", "@effect");
9+
10+
async function exists(targetPath) {
11+
try {
12+
await fs.lstat(targetPath);
13+
return true;
14+
} catch {
15+
return false;
16+
}
17+
}
18+
19+
async function dedupeNestedEffect(packageDir) {
20+
const nestedEffectDir = path.join(packageDir, "node_modules", "effect");
21+
if (!(await exists(nestedEffectDir))) {
22+
return false;
23+
}
24+
25+
const nestedStat = await fs.lstat(nestedEffectDir);
26+
if (nestedStat.isSymbolicLink()) {
27+
const linkTarget = await fs.readlink(nestedEffectDir);
28+
const resolvedTarget = path.resolve(path.dirname(nestedEffectDir), linkTarget);
29+
if (resolvedTarget === effectRoot) {
30+
return false;
31+
}
32+
}
33+
34+
await fs.rm(nestedEffectDir, { recursive: true, force: true });
35+
const relativeTarget = path.relative(path.dirname(nestedEffectDir), effectRoot);
36+
await fs.symlink(relativeTarget, nestedEffectDir, "dir");
37+
return true;
38+
}
39+
40+
async function main() {
41+
if (!(await exists(effectRoot)) || !(await exists(effectScopeDir))) {
42+
return;
43+
}
44+
45+
const entries = await fs.readdir(effectScopeDir, { withFileTypes: true });
46+
let dedupedCount = 0;
47+
for (const entry of entries) {
48+
if (!entry.isDirectory()) {
49+
continue;
50+
}
51+
const changed = await dedupeNestedEffect(path.join(effectScopeDir, entry.name));
52+
if (changed) {
53+
dedupedCount += 1;
54+
}
55+
}
56+
57+
if (dedupedCount > 0) {
58+
console.log(`[dedupe-effect-node-modules] linked root effect into ${dedupedCount} package(s)`);
59+
}
60+
}
61+
62+
await main();

0 commit comments

Comments
 (0)