From 58d800218f60e3fc88761b98bc4d993cf62b8cca Mon Sep 17 00:00:00 2001 From: Meet Patel Date: Thu, 12 Mar 2026 23:53:29 +0530 Subject: [PATCH 1/3] Fix sparse multi-turn checkpoint diffs - tolerate sparse checkpoint turn counts in diff queries - keep completed turn captures from being dropped on activeTurnId mismatch - dedupe repeated provider diff placeholder events per turn - retry checkpoint-ref unavailable errors in the web diff query - add focused regression coverage for each path --- .../Layers/CheckpointDiffQuery.test.ts | 120 +++++++++++++++--- .../Layers/CheckpointDiffQuery.ts | 64 ++++++---- .../Layers/CheckpointReactor.test.ts | 27 ++-- .../orchestration/Layers/CheckpointReactor.ts | 12 -- .../Layers/ProviderRuntimeIngestion.test.ts | 56 ++++++++ .../Layers/ProviderRuntimeIngestion.ts | 62 +++++++-- apps/web/src/lib/providerReactQuery.test.ts | 1 + apps/web/src/lib/providerReactQuery.ts | 1 + 8 files changed, 268 insertions(+), 75 deletions(-) diff --git a/apps/server/src/checkpointing/Layers/CheckpointDiffQuery.test.ts b/apps/server/src/checkpointing/Layers/CheckpointDiffQuery.test.ts index 2f79ea9d5..e479e41f4 100644 --- a/apps/server/src/checkpointing/Layers/CheckpointDiffQuery.test.ts +++ b/apps/server/src/checkpointing/Layers/CheckpointDiffQuery.test.ts @@ -20,8 +20,11 @@ function makeSnapshot(input: { readonly threadId: ThreadId; readonly workspaceRoot: string; readonly worktreePath: string | null; - readonly checkpointTurnCount: number; - readonly checkpointRef: CheckpointRef; + readonly checkpoints: ReadonlyArray<{ + readonly turnId: TurnId; + readonly checkpointTurnCount: number; + readonly checkpointRef: CheckpointRef; + }>; }): OrchestrationReadModel { return { snapshotSequence: 0, @@ -62,17 +65,15 @@ function makeSnapshot(input: { messages: [], activities: [], proposedPlans: [], - checkpoints: [ - { - turnId: TurnId.makeUnsafe("turn-1"), - checkpointTurnCount: input.checkpointTurnCount, - checkpointRef: input.checkpointRef, - status: "ready", - files: [], - assistantMessageId: null, - completedAt: "2026-01-01T00:00:00.000Z", - }, - ], + checkpoints: input.checkpoints.map((checkpoint) => ({ + turnId: checkpoint.turnId, + checkpointTurnCount: checkpoint.checkpointTurnCount, + checkpointRef: checkpoint.checkpointRef, + status: "ready" as const, + files: [], + assistantMessageId: null, + completedAt: "2026-01-01T00:00:00.000Z", + })), session: null, }, ], @@ -96,8 +97,13 @@ describe("CheckpointDiffQueryLive", () => { threadId, workspaceRoot: "/tmp/workspace", worktreePath: null, - checkpointTurnCount: 1, - checkpointRef: toCheckpointRef, + checkpoints: [ + { + turnId: TurnId.makeUnsafe("turn-1"), + checkpointTurnCount: 1, + checkpointRef: toCheckpointRef, + }, + ], }); const checkpointStore: CheckpointStoreShape = { @@ -194,4 +200,88 @@ describe("CheckpointDiffQueryLive", () => { ), ).rejects.toThrow("Thread 'thread-missing' not found."); }); + + it("falls back to the nearest earlier checkpoint when an exact turn checkpoint is missing", async () => { + const projectId = ProjectId.makeUnsafe("project-1"); + const threadId = ThreadId.makeUnsafe("thread-1"); + const checkpointRef2 = checkpointRefForThreadTurn(threadId, 2); + const checkpointRef4 = checkpointRefForThreadTurn(threadId, 4); + const hasCheckpointRefCalls: Array = []; + const diffCheckpointsCalls: Array<{ + readonly fromCheckpointRef: CheckpointRef; + readonly toCheckpointRef: CheckpointRef; + readonly cwd: string; + }> = []; + + const snapshot = makeSnapshot({ + projectId, + threadId, + workspaceRoot: "/tmp/workspace", + worktreePath: null, + checkpoints: [ + { + turnId: TurnId.makeUnsafe("turn-2"), + checkpointTurnCount: 2, + checkpointRef: checkpointRef2, + }, + { + turnId: TurnId.makeUnsafe("turn-4"), + checkpointTurnCount: 4, + checkpointRef: checkpointRef4, + }, + ], + }); + + const checkpointStore: CheckpointStoreShape = { + isGitRepository: () => Effect.succeed(true), + captureCheckpoint: () => Effect.void, + hasCheckpointRef: ({ checkpointRef }) => + Effect.sync(() => { + hasCheckpointRefCalls.push(checkpointRef); + return true; + }), + restoreCheckpoint: () => Effect.succeed(true), + diffCheckpoints: ({ fromCheckpointRef, toCheckpointRef, cwd }) => + Effect.sync(() => { + diffCheckpointsCalls.push({ fromCheckpointRef, toCheckpointRef, cwd }); + return ""; + }), + deleteCheckpointRefs: () => Effect.void, + }; + + const layer = CheckpointDiffQueryLive.pipe( + Layer.provideMerge(Layer.succeed(CheckpointStore, checkpointStore)), + Layer.provideMerge( + Layer.succeed(ProjectionSnapshotQuery, { + getSnapshot: () => Effect.succeed(snapshot), + }), + ), + ); + + const result = await Effect.runPromise( + Effect.gen(function* () { + const query = yield* CheckpointDiffQuery; + return yield* query.getTurnDiff({ + threadId, + fromTurnCount: 2, + toTurnCount: 3, + }); + }).pipe(Effect.provide(layer)), + ); + + expect(hasCheckpointRefCalls).toEqual([checkpointRef2, checkpointRef2]); + expect(diffCheckpointsCalls).toEqual([ + { + cwd: "/tmp/workspace", + fromCheckpointRef: checkpointRef2, + toCheckpointRef: checkpointRef2, + }, + ]); + expect(result).toEqual({ + threadId, + fromTurnCount: 2, + toTurnCount: 3, + diff: "", + }); + }); }); diff --git a/apps/server/src/checkpointing/Layers/CheckpointDiffQuery.ts b/apps/server/src/checkpointing/Layers/CheckpointDiffQuery.ts index 5cf26c86d..1929d2d43 100644 --- a/apps/server/src/checkpointing/Layers/CheckpointDiffQuery.ts +++ b/apps/server/src/checkpointing/Layers/CheckpointDiffQuery.ts @@ -1,5 +1,7 @@ import { + CheckpointRef, OrchestrationGetTurnDiffResult, + ThreadId, type OrchestrationGetFullThreadDiffInput, type OrchestrationGetFullThreadDiffResult, type OrchestrationGetTurnDiffResult as OrchestrationGetTurnDiffResultType, @@ -21,6 +23,34 @@ const make = Effect.gen(function* () { const projectionSnapshotQuery = yield* ProjectionSnapshotQuery; const checkpointStore = yield* CheckpointStore; + const resolveCheckpointRefAtOrBeforeTurnCount = (input: { + readonly threadId: ThreadId; + readonly requestedTurnCount: number; + readonly checkpoints: ReadonlyArray<{ + readonly checkpointTurnCount: number; + readonly checkpointRef: CheckpointRef; + }>; + }): CheckpointRef => { + if (input.requestedTurnCount <= 0) { + return checkpointRefForThreadTurn(input.threadId, 0); + } + + const matchingCheckpoint = input.checkpoints.reduce<{ + readonly checkpointTurnCount: number; + readonly checkpointRef: CheckpointRef; + } | null>((resolved, checkpoint) => { + if (checkpoint.checkpointTurnCount > input.requestedTurnCount) { + return resolved; + } + if (resolved && resolved.checkpointTurnCount >= checkpoint.checkpointTurnCount) { + return resolved; + } + return checkpoint; + }, null); + + return matchingCheckpoint?.checkpointRef ?? checkpointRefForThreadTurn(input.threadId, 0); + }; + const getTurnDiff: CheckpointDiffQueryShape["getTurnDiff"] = (input) => Effect.gen(function* () { const operation = "CheckpointDiffQuery.getTurnDiff"; @@ -73,30 +103,16 @@ const make = Effect.gen(function* () { }); } - const fromCheckpointRef = - input.fromTurnCount === 0 - ? checkpointRefForThreadTurn(input.threadId, 0) - : thread.checkpoints.find( - (checkpoint) => checkpoint.checkpointTurnCount === input.fromTurnCount, - )?.checkpointRef; - if (!fromCheckpointRef) { - return yield* new CheckpointUnavailableError({ - threadId: input.threadId, - turnCount: input.fromTurnCount, - detail: `Checkpoint ref is unavailable for turn ${input.fromTurnCount}.`, - }); - } - - const toCheckpointRef = thread.checkpoints.find( - (checkpoint) => checkpoint.checkpointTurnCount === input.toTurnCount, - )?.checkpointRef; - if (!toCheckpointRef) { - return yield* new CheckpointUnavailableError({ - threadId: input.threadId, - turnCount: input.toTurnCount, - detail: `Checkpoint ref is unavailable for turn ${input.toTurnCount}.`, - }); - } + const fromCheckpointRef = resolveCheckpointRefAtOrBeforeTurnCount({ + threadId: input.threadId, + requestedTurnCount: input.fromTurnCount, + checkpoints: thread.checkpoints, + }); + const toCheckpointRef = resolveCheckpointRefAtOrBeforeTurnCount({ + threadId: input.threadId, + requestedTurnCount: input.toTurnCount, + checkpoints: thread.checkpoints, + }); const [fromExists, toExists] = yield* Effect.all( [ diff --git a/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts b/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts index 09773b71d..f86b47807 100644 --- a/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts +++ b/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts @@ -110,7 +110,7 @@ async function waitForThread( engine: OrchestrationEngineShape, predicate: (thread: { latestTurn: { turnId: string } | null; - checkpoints: ReadonlyArray<{ checkpointTurnCount: number }>; + checkpoints: ReadonlyArray<{ checkpointTurnCount: number; turnId: string }>; activities: ReadonlyArray<{ kind: string }>; }) => boolean, timeoutMs = 5000, @@ -118,7 +118,7 @@ async function waitForThread( const deadline = Date.now() + timeoutMs; const poll = async (): Promise<{ latestTurn: { turnId: string } | null; - checkpoints: ReadonlyArray<{ checkpointTurnCount: number }>; + checkpoints: ReadonlyArray<{ checkpointTurnCount: number; turnId: string }>; activities: ReadonlyArray<{ kind: string }>; }> => { const readModel = await Effect.runPromise(engine.getReadModel()); @@ -403,7 +403,7 @@ describe("CheckpointReactor", () => { ).toBe("v2\n"); }); - it("ignores auxiliary thread turn completion while primary turn is active", async () => { + it("captures completed turns even when session activeTurnId points at a different turn", async () => { const harness = await createHarness({ seedFilesystemCheckpoints: false }); const createdAt = new Date().toISOString(); @@ -453,11 +453,12 @@ describe("CheckpointReactor", () => { }); await harness.drain(); - const midReadModel = await Effect.runPromise(harness.engine.getReadModel()); - const midThread = midReadModel.threads.find( - (entry) => entry.id === ThreadId.makeUnsafe("thread-1"), + const midThread = await waitForThread( + harness.engine, + (entry) => + entry.checkpoints.length === 1 && entry.checkpoints[0]?.turnId === asTurnId("turn-aux"), ); - expect(midThread?.checkpoints).toHaveLength(0); + expect(midThread.checkpoints[0]?.checkpointTurnCount).toBe(1); harness.provider.emit({ type: "turn.completed", @@ -472,9 +473,13 @@ describe("CheckpointReactor", () => { const thread = await waitForThread( harness.engine, - (entry) => entry.latestTurn?.turnId === "turn-main" && entry.checkpoints.length === 1, + (entry) => entry.latestTurn?.turnId === "turn-main" && entry.checkpoints.length === 2, ); - expect(thread.checkpoints[0]?.checkpointTurnCount).toBe(1); + expect(thread.checkpoints.map((checkpoint) => checkpoint.checkpointTurnCount)).toEqual([1, 2]); + expect(thread.checkpoints.map((checkpoint) => checkpoint.turnId)).toEqual([ + asTurnId("turn-aux"), + asTurnId("turn-main"), + ]); }); it("appends capture failure activity when turn diff summary cannot be derived", async () => { @@ -786,7 +791,9 @@ describe("CheckpointReactor", () => { threadId: ThreadId.makeUnsafe("thread-1"), numTurns: 1, }); - expect(fs.readFileSync(path.join(harness.cwd, "README.md"), "utf8")).toBe("v2\n"); + expect( + fs.readFileSync(path.join(harness.cwd, "README.md"), "utf8").replaceAll("\r\n", "\n"), + ).toBe("v2\n"); expect( gitRefExists(harness.cwd, checkpointRefForThreadTurn(ThreadId.makeUnsafe("thread-1"), 2)), ).toBe(false); diff --git a/apps/server/src/orchestration/Layers/CheckpointReactor.ts b/apps/server/src/orchestration/Layers/CheckpointReactor.ts index ab38c1033..4387d49fe 100644 --- a/apps/server/src/orchestration/Layers/CheckpointReactor.ts +++ b/apps/server/src/orchestration/Layers/CheckpointReactor.ts @@ -40,13 +40,6 @@ function toTurnId(value: string | undefined): TurnId | null { return value === undefined ? null : TurnId.makeUnsafe(String(value)); } -function sameId(left: string | null | undefined, right: string | null | undefined): boolean { - if (left === null || left === undefined || right === null || right === undefined) { - return false; - } - return left === right; -} - function checkpointStatusFromRuntime(status: string | undefined): "ready" | "missing" | "error" { switch (status) { case "failed": @@ -334,11 +327,6 @@ const make = Effect.gen(function* () { return; } - // When a primary turn is active, only that turn may produce completion checkpoints. - if (thread.session?.activeTurnId && !sameId(thread.session.activeTurnId, turnId)) { - return; - } - // Only skip if a real (non-placeholder) checkpoint already exists for this turn. // ProviderRuntimeIngestion may insert placeholder entries with status "missing" // before this reactor runs; those must not prevent real git capture. diff --git a/apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.test.ts b/apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.test.ts index b6b48c7ed..458e733db 100644 --- a/apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.test.ts +++ b/apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.test.ts @@ -1327,6 +1327,62 @@ describe("ProviderRuntimeIngestion", () => { expect(checkpoint?.checkpointRef).toBe("provider-diff:evt-turn-diff-updated"); }); + it("dedupes repeated turn diff placeholder updates for the same turn", async () => { + const harness = await createHarness(); + const now = new Date().toISOString(); + + harness.emit({ + type: "turn.diff.updated", + eventId: asEventId("evt-turn-diff-updated-1"), + provider: "codex", + createdAt: now, + threadId: asThreadId("thread-1"), + turnId: asTurnId("turn-p1"), + itemId: asItemId("item-p1-assistant"), + payload: { + unifiedDiff: "diff --git a/file.txt b/file.txt\n+hello\n", + }, + }); + + harness.emit({ + type: "turn.diff.updated", + eventId: asEventId("evt-turn-diff-updated-2"), + provider: "codex", + createdAt: now, + threadId: asThreadId("thread-1"), + turnId: asTurnId("turn-p1"), + itemId: asItemId("item-p1-assistant"), + payload: { + unifiedDiff: "diff --git a/file.txt b/file.txt\n+hello again\n", + }, + }); + + const thread = await waitForThread(harness.engine, (entry) => + entry.checkpoints.some( + (checkpoint: ProviderRuntimeTestCheckpoint) => checkpoint.turnId === "turn-p1", + ), + ); + + await harness.drain(); + const readModel = await Effect.runPromise(harness.engine.getReadModel()); + const updatedThread = readModel.threads.find( + (entry) => entry.id === ThreadId.makeUnsafe("thread-1"), + ); + + expect(updatedThread).toBeDefined(); + const matchingCheckpoints = updatedThread?.checkpoints.filter( + (checkpoint: ProviderRuntimeTestCheckpoint) => checkpoint.turnId === "turn-p1", + ); + expect(matchingCheckpoints).toHaveLength(1); + expect(matchingCheckpoints?.[0]?.checkpointTurnCount).toBe(1); + expect(matchingCheckpoints?.[0]?.checkpointRef).toBe("provider-diff:evt-turn-diff-updated-1"); + expect( + thread.checkpoints.filter( + (checkpoint: ProviderRuntimeTestCheckpoint) => checkpoint.turnId === "turn-p1", + ), + ).toHaveLength(1); + }); + it("projects Codex task lifecycle chunks into thread activities", async () => { const harness = await createHarness(); const now = new Date().toISOString(); diff --git a/apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts b/apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts index 417e93c8d..2c1a0c0e6 100644 --- a/apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts +++ b/apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts @@ -484,6 +484,7 @@ const make = Effect.gen(function* () { const assistantDeliveryModeRef = yield* Ref.make( DEFAULT_ASSISTANT_DELIVERY_MODE, ); + const placeholderCheckpointTurnsRef = yield* Ref.make(new Set()); const turnMessageIdsByTurnKey = yield* Cache.make>({ capacity: TURN_MESSAGE_IDS_BY_TURN_CACHE_CAPACITY, @@ -780,6 +781,17 @@ const make = Effect.gen(function* () { const now = event.createdAt; const eventTurnId = toTurnId(event.turnId); + if (eventTurnId && (event.type === "turn.completed" || event.type === "turn.aborted")) { + const eventTurnKey = providerTurnKey(thread.id, eventTurnId); + yield* Ref.update(placeholderCheckpointTurnsRef, (seen) => { + if (!seen.has(eventTurnKey)) { + return seen; + } + const next = new Set(seen); + next.delete(eventTurnKey); + return next; + }); + } const activeTurnId = thread.session?.activeTurnId ?? null; const conflictsWithActiveTurn = @@ -1053,13 +1065,22 @@ const make = Effect.gen(function* () { if (event.type === "turn.diff.updated") { const turnId = toTurnId(event.turnId); if (turnId && (yield* isGitRepoForThread(thread.id))) { + const turnKey = providerTurnKey(thread.id, turnId); + const placeholderAlreadyQueued = yield* Ref.get(placeholderCheckpointTurnsRef).pipe( + Effect.map((seen) => seen.has(turnKey)), + ); // Skip if a checkpoint already exists for this turn. A real // (non-placeholder) capture from CheckpointReactor should not // be clobbered, and dispatching a duplicate placeholder for the // same turnId would produce an unstable checkpointTurnCount. - if (thread.checkpoints.some((c) => c.turnId === turnId)) { + if (placeholderAlreadyQueued || thread.checkpoints.some((c) => c.turnId === turnId)) { // Already tracked; no-op. } else { + yield* Ref.update(placeholderCheckpointTurnsRef, (seen) => { + const next = new Set(seen); + next.add(turnKey); + return next; + }); const assistantMessageId = MessageId.makeUnsafe( `assistant:${event.itemId ?? event.turnId ?? event.eventId}`, ); @@ -1067,19 +1088,32 @@ const make = Effect.gen(function* () { (max, c) => Math.max(max, c.checkpointTurnCount), 0, ); - yield* orchestrationEngine.dispatch({ - type: "thread.turn.diff.complete", - commandId: providerCommandId(event, "thread-turn-diff-complete"), - threadId: thread.id, - turnId, - completedAt: now, - checkpointRef: CheckpointRef.makeUnsafe(`provider-diff:${event.eventId}`), - status: "missing", - files: [], - assistantMessageId, - checkpointTurnCount: maxTurnCount + 1, - createdAt: now, - }); + yield* orchestrationEngine + .dispatch({ + type: "thread.turn.diff.complete", + commandId: providerCommandId(event, "thread-turn-diff-complete"), + threadId: thread.id, + turnId, + completedAt: now, + checkpointRef: CheckpointRef.makeUnsafe(`provider-diff:${event.eventId}`), + status: "missing", + files: [], + assistantMessageId, + checkpointTurnCount: maxTurnCount + 1, + createdAt: now, + }) + .pipe( + Effect.tapError(() => + Ref.update(placeholderCheckpointTurnsRef, (seen) => { + if (!seen.has(turnKey)) { + return seen; + } + const next = new Set(seen); + next.delete(turnKey); + return next; + }), + ), + ); } } } diff --git a/apps/web/src/lib/providerReactQuery.test.ts b/apps/web/src/lib/providerReactQuery.test.ts index b7e770799..b3b5ca0e3 100644 --- a/apps/web/src/lib/providerReactQuery.test.ts +++ b/apps/web/src/lib/providerReactQuery.test.ts @@ -131,6 +131,7 @@ describe("checkpointDiffQueryOptions", () => { expect( retry(12, new Error("Filesystem checkpoint is unavailable for turn 2 in thread thread-1.")), ).toBe(false); + expect(retry(1, new Error("Checkpoint ref is unavailable for turn 3."))).toBe(true); expect(retry(2, new Error("Something else failed."))).toBe(true); expect(retry(3, new Error("Something else failed."))).toBe(false); }); diff --git a/apps/web/src/lib/providerReactQuery.ts b/apps/web/src/lib/providerReactQuery.ts index 0547aa9e5..ab037139e 100644 --- a/apps/web/src/lib/providerReactQuery.ts +++ b/apps/web/src/lib/providerReactQuery.ts @@ -84,6 +84,7 @@ function isCheckpointTemporarilyUnavailable(error: unknown): boolean { const message = asCheckpointErrorMessage(error).toLowerCase(); return ( message.includes("exceeds current turn count") || + message.includes("checkpoint ref is unavailable for turn") || message.includes("checkpoint is unavailable for turn") || message.includes("filesystem checkpoint is unavailable") ); From 552a430062e7eca3321bd9050516c243e32cabdb Mon Sep 17 00:00:00 2001 From: Meet Patel Date: Fri, 13 Mar 2026 02:12:04 +0530 Subject: [PATCH 2/3] Add checkpoint diff availability message logic and tests --- apps/web/src/components/DiffPanel.tsx | 27 ++++-- .../src/components/diffPanel.logic.test.ts | 95 +++++++++++++++++++ apps/web/src/components/diffPanel.logic.ts | 27 ++++++ 3 files changed, 140 insertions(+), 9 deletions(-) create mode 100644 apps/web/src/components/diffPanel.logic.test.ts create mode 100644 apps/web/src/components/diffPanel.logic.ts diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx index 34ad78881..c5d4e748b 100644 --- a/apps/web/src/components/DiffPanel.tsx +++ b/apps/web/src/components/DiffPanel.tsx @@ -23,6 +23,7 @@ import { useTheme } from "../hooks/useTheme"; import { buildPatchCacheKey } from "../lib/diffRendering"; import { resolveDiffThemeName } from "../lib/diffRendering"; import { useTurnDiffSummaries } from "../hooks/useTurnDiffSummaries"; +import { getUnavailableCheckpointDiffMessage } from "./diffPanel.logic"; import { useStore } from "../store"; import { useAppSettings } from "../appSettings"; import { formatShortTimestamp } from "../timestampFormat"; @@ -206,6 +207,10 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) { ? undefined : (orderedTurnDiffSummaries.find((summary) => summary.turnId === selectedTurnId) ?? orderedTurnDiffSummaries[0]); + const unavailableCheckpointDiffMessage = getUnavailableCheckpointDiffMessage({ + activeThread, + selectedTurn, + }); const selectedCheckpointTurnCount = selectedTurn && (selectedTurn.checkpointTurnCount ?? inferredCheckpointTurnCountByTurnId[selectedTurn.turnId]); @@ -257,7 +262,7 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) { fromTurnCount: activeCheckpointRange?.fromTurnCount ?? null, toTurnCount: activeCheckpointRange?.toTurnCount ?? null, cacheScope: selectedTurn ? `turn:${selectedTurn.turnId}` : conversationCacheScope, - enabled: isGitRepo, + enabled: isGitRepo && unavailableCheckpointDiffMessage === null, }), ); const selectedTurnCheckpointDiff = selectedTurn @@ -266,13 +271,15 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) { const conversationCheckpointDiff = selectedTurn ? undefined : activeCheckpointDiffQuery.data?.diff; - const isLoadingCheckpointDiff = activeCheckpointDiffQuery.isLoading; + const isLoadingCheckpointDiff = + unavailableCheckpointDiffMessage === null && activeCheckpointDiffQuery.isLoading; const checkpointDiffError = - activeCheckpointDiffQuery.error instanceof Error + unavailableCheckpointDiffMessage ?? + (activeCheckpointDiffQuery.error instanceof Error ? activeCheckpointDiffQuery.error.message : activeCheckpointDiffQuery.error ? "Failed to load checkpoint diff." - : null; + : null); const selectedPatch = selectedTurn ? selectedTurnCheckpointDiff : conversationCheckpointDiff; const hasResolvedPatch = typeof selectedPatch === "string"; @@ -532,11 +539,13 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) { ref={patchViewportRef} className="diff-panel-viewport min-h-0 min-w-0 flex-1 overflow-hidden" > - {checkpointDiffError && !renderablePatch && ( -
-

{checkpointDiffError}

-
- )} + {checkpointDiffError && + !renderablePatch && + unavailableCheckpointDiffMessage === null && ( +
+

{checkpointDiffError}

+
+ )} {!renderablePatch ? ( isLoadingCheckpointDiff ? ( diff --git a/apps/web/src/components/diffPanel.logic.test.ts b/apps/web/src/components/diffPanel.logic.test.ts new file mode 100644 index 000000000..c170de7ca --- /dev/null +++ b/apps/web/src/components/diffPanel.logic.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, it } from "vitest"; +import { MessageId, ProjectId, ThreadId, TurnId } from "@t3tools/contracts"; + +import { getUnavailableCheckpointDiffMessage } from "./diffPanel.logic"; +import type { Thread, TurnDiffSummary } from "../types"; + +const turnId = TurnId.makeUnsafe("turn-1"); +const otherTurnId = TurnId.makeUnsafe("turn-2"); + +function makeThread(overrides: Partial = {}): Thread { + return { + id: ThreadId.makeUnsafe("thread-1"), + codexThreadId: null, + projectId: ProjectId.makeUnsafe("project-1"), + title: "Thread", + model: "gpt-5-codex", + runtimeMode: "full-access", + interactionMode: "default", + session: null, + messages: [], + proposedPlans: [], + error: null, + createdAt: new Date().toISOString(), + latestTurn: null, + branch: null, + worktreePath: null, + turnDiffSummaries: [], + activities: [], + ...overrides, + }; +} + +function makeTurnSummary(overrides: Partial = {}): TurnDiffSummary { + return { + turnId, + completedAt: new Date().toISOString(), + status: "ready", + files: [], + checkpointRef: "checkpoint:1" as never, + assistantMessageId: MessageId.makeUnsafe("assistant:1"), + checkpointTurnCount: 1, + ...overrides, + }; +} + +describe("getUnavailableCheckpointDiffMessage", () => { + it("returns an unavailable message for historical missing checkpoints", () => { + const thread = makeThread({ + latestTurn: { + turnId: otherTurnId, + state: "completed", + requestedAt: new Date().toISOString(), + startedAt: new Date().toISOString(), + completedAt: new Date().toISOString(), + assistantMessageId: null, + }, + }); + + const message = getUnavailableCheckpointDiffMessage({ + activeThread: thread, + selectedTurn: makeTurnSummary({ status: "missing", checkpointRef: undefined }), + }); + + expect(message).toBe("Checkpoint diff is unavailable for this turn."); + }); + + it("allows the latest turn to keep retrying while checkpoint capture catches up", () => { + const thread = makeThread({ + latestTurn: { + turnId, + state: "completed", + requestedAt: new Date().toISOString(), + startedAt: new Date().toISOString(), + completedAt: new Date().toISOString(), + assistantMessageId: null, + }, + }); + + const message = getUnavailableCheckpointDiffMessage({ + activeThread: thread, + selectedTurn: makeTurnSummary({ status: "missing", checkpointRef: undefined }), + }); + + expect(message).toBeNull(); + }); + + it("does not block ready checkpoints", () => { + const message = getUnavailableCheckpointDiffMessage({ + activeThread: makeThread(), + selectedTurn: makeTurnSummary(), + }); + + expect(message).toBeNull(); + }); +}); diff --git a/apps/web/src/components/diffPanel.logic.ts b/apps/web/src/components/diffPanel.logic.ts new file mode 100644 index 000000000..6d7dc5a15 --- /dev/null +++ b/apps/web/src/components/diffPanel.logic.ts @@ -0,0 +1,27 @@ +import type { Thread, TurnDiffSummary } from "../types"; + +interface CheckpointDiffAvailabilityInput { + activeThread: Thread | undefined; + selectedTurn: TurnDiffSummary | undefined; +} + +export function getUnavailableCheckpointDiffMessage({ + activeThread, + selectedTurn, +}: CheckpointDiffAvailabilityInput): string | null { + if (!selectedTurn) { + return null; + } + + const checkpointMissing = selectedTurn.status === "missing" || !selectedTurn.checkpointRef; + if (!checkpointMissing) { + return null; + } + + const isLatestTurn = activeThread?.latestTurn?.turnId === selectedTurn.turnId; + if (isLatestTurn) { + return null; + } + + return "Checkpoint diff is unavailable for this turn."; +} From b0fa2585156a045fcaab7db370f1074bdfa6f621 Mon Sep 17 00:00:00 2001 From: Meet Patel Date: Sat, 14 Mar 2026 01:06:10 +0530 Subject: [PATCH 3/3] Refactor checkpoint diff availability logic and update related tests --- apps/web/src/components/DiffPanel.tsx | 126 +++++++++++++----- .../src/components/diffPanel.logic.test.ts | 65 ++------- apps/web/src/components/diffPanel.logic.ts | 11 +- 3 files changed, 103 insertions(+), 99 deletions(-) diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx index c5d4e748b..7b0ab929a 100644 --- a/apps/web/src/components/DiffPanel.tsx +++ b/apps/web/src/components/DiffPanel.tsx @@ -3,7 +3,7 @@ import { FileDiff, type FileDiffMetadata, Virtualizer } from "@pierre/diffs/reac import { useQuery } from "@tanstack/react-query"; import { useNavigate, useParams, useSearch } from "@tanstack/react-router"; import { ThreadId, type TurnId } from "@t3tools/contracts"; -import { ChevronLeftIcon, ChevronRightIcon, Columns2Icon, Rows3Icon } from "lucide-react"; +import { AlertCircleIcon, ChevronLeftIcon, ChevronRightIcon, Columns2Icon, Rows3Icon } from "lucide-react"; import { type WheelEvent as ReactWheelEvent, useCallback, @@ -27,7 +27,7 @@ import { getUnavailableCheckpointDiffMessage } from "./diffPanel.logic"; import { useStore } from "../store"; import { useAppSettings } from "../appSettings"; import { formatShortTimestamp } from "../timestampFormat"; -import { DiffPanelLoadingState, DiffPanelShell, type DiffPanelMode } from "./DiffPanelShell"; +import { DiffPanelShell, type DiffPanelMode } from "./DiffPanelShell"; import { ToggleGroup, Toggle } from "./ui/toggle-group"; type DiffRenderMode = "stacked" | "split"; @@ -99,14 +99,14 @@ const DIFF_PANEL_UNSAFE_CSS = ` type RenderablePatch = | { - kind: "files"; - files: FileDiffMetadata[]; - } + kind: "files"; + files: FileDiffMetadata[]; + } | { - kind: "raw"; - text: string; - reason: string; - }; + kind: "raw"; + text: string; + reason: string; + }; function getRenderablePatch( patch: string | undefined, @@ -208,7 +208,6 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) { : (orderedTurnDiffSummaries.find((summary) => summary.turnId === selectedTurnId) ?? orderedTurnDiffSummaries[0]); const unavailableCheckpointDiffMessage = getUnavailableCheckpointDiffMessage({ - activeThread, selectedTurn, }); const selectedCheckpointTurnCount = @@ -218,14 +217,15 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) { () => typeof selectedCheckpointTurnCount === "number" ? { - fromTurnCount: Math.max(0, selectedCheckpointTurnCount - 1), - toTurnCount: selectedCheckpointTurnCount, - } + fromTurnCount: Math.max(0, selectedCheckpointTurnCount - 1), + toTurnCount: selectedCheckpointTurnCount, + } : null, [selectedCheckpointTurnCount], ); const conversationCheckpointTurnCount = useMemo(() => { const turnCounts = orderedTurnDiffSummaries + .filter((summary) => summary.status !== "missing" && summary.checkpointRef) .map( (summary) => summary.checkpointTurnCount ?? inferredCheckpointTurnCountByTurnId[summary.turnId], @@ -241,9 +241,9 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) { () => !selectedTurn && typeof conversationCheckpointTurnCount === "number" ? { - fromTurnCount: 0, - toTurnCount: conversationCheckpointTurnCount, - } + fromTurnCount: 0, + toTurnCount: conversationCheckpointTurnCount, + } : null, [conversationCheckpointTurnCount, selectedTurn], ); @@ -256,13 +256,17 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) { } return `conversation:${orderedTurnDiffSummaries.map((summary) => summary.turnId).join(",")}`; }, [orderedTurnDiffSummaries, selectedTurn]); + const isCheckpointAvailable = !( + selectedTurn && + (selectedTurn.status === "missing" || !selectedTurn.checkpointRef) + ); const activeCheckpointDiffQuery = useQuery( checkpointDiffQueryOptions({ threadId: activeThreadId, fromTurnCount: activeCheckpointRange?.fromTurnCount ?? null, toTurnCount: activeCheckpointRange?.toTurnCount ?? null, cacheScope: selectedTurn ? `turn:${selectedTurn.turnId}` : conversationCacheScope, - enabled: isGitRepo && unavailableCheckpointDiffMessage === null, + enabled: isGitRepo && unavailableCheckpointDiffMessage === null && isCheckpointAvailable, }), ); const selectedTurnCheckpointDiff = selectedTurn @@ -539,25 +543,77 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) { ref={patchViewportRef} className="diff-panel-viewport min-h-0 min-w-0 flex-1 overflow-hidden" > - {checkpointDiffError && - !renderablePatch && - unavailableCheckpointDiffMessage === null && ( -
-

{checkpointDiffError}

-
- )} {!renderablePatch ? ( - isLoadingCheckpointDiff ? ( - - ) : ( -
-

- {hasNoNetChanges - ? "No net changes in this selection." - : "No patch available for this selection."} -

-
- ) +
+ {isLoadingCheckpointDiff ? ( +
+
+

Loading checkpoint diff...

+
+ ) : checkpointDiffError ? ( +
+
+ +
+
+

+ Checkpoint Unavailable +

+

+ {checkpointDiffError} +

+
+
+ ) : hasNoNetChanges ? ( +
+
+ + + +
+
+

No Changes

+

+ No net changes in this selection. +

+
+
+ ) : ( +
+
+ + + +
+
+

No Diff Available

+

+ No patch available for this selection. +

+
+
+ )} +
) : renderablePatch.kind === "files" ? ( = {}): Thread { - return { - id: ThreadId.makeUnsafe("thread-1"), - codexThreadId: null, - projectId: ProjectId.makeUnsafe("project-1"), - title: "Thread", - model: "gpt-5-codex", - runtimeMode: "full-access", - interactionMode: "default", - session: null, - messages: [], - proposedPlans: [], - error: null, - createdAt: new Date().toISOString(), - latestTurn: null, - branch: null, - worktreePath: null, - turnDiffSummaries: [], - activities: [], - ...overrides, - }; -} function makeTurnSummary(overrides: Partial = {}): TurnDiffSummary { return { @@ -44,49 +20,24 @@ function makeTurnSummary(overrides: Partial = {}): TurnDiffSumm } describe("getUnavailableCheckpointDiffMessage", () => { - it("returns an unavailable message for historical missing checkpoints", () => { - const thread = makeThread({ - latestTurn: { - turnId: otherTurnId, - state: "completed", - requestedAt: new Date().toISOString(), - startedAt: new Date().toISOString(), - completedAt: new Date().toISOString(), - assistantMessageId: null, - }, - }); - + it("returns an unavailable message for missing checkpoints", () => { const message = getUnavailableCheckpointDiffMessage({ - activeThread: thread, selectedTurn: makeTurnSummary({ status: "missing", checkpointRef: undefined }), }); - expect(message).toBe("Checkpoint diff is unavailable for this turn."); + expect(message).toBe("Checkpoint is marked as missing and cannot be restored."); }); - it("allows the latest turn to keep retrying while checkpoint capture catches up", () => { - const thread = makeThread({ - latestTurn: { - turnId, - state: "completed", - requestedAt: new Date().toISOString(), - startedAt: new Date().toISOString(), - completedAt: new Date().toISOString(), - assistantMessageId: null, - }, - }); - + it("returns an unavailable message when checkpoint ref is missing", () => { const message = getUnavailableCheckpointDiffMessage({ - activeThread: thread, - selectedTurn: makeTurnSummary({ status: "missing", checkpointRef: undefined }), + selectedTurn: makeTurnSummary({ status: "ready", checkpointRef: undefined }), }); - expect(message).toBeNull(); + expect(message).toBe("Checkpoint reference is unavailable for this turn."); }); - it("does not block ready checkpoints", () => { + it("does not block ready checkpoints with valid refs", () => { const message = getUnavailableCheckpointDiffMessage({ - activeThread: makeThread(), selectedTurn: makeTurnSummary(), }); diff --git a/apps/web/src/components/diffPanel.logic.ts b/apps/web/src/components/diffPanel.logic.ts index 6d7dc5a15..ad45b6d9e 100644 --- a/apps/web/src/components/diffPanel.logic.ts +++ b/apps/web/src/components/diffPanel.logic.ts @@ -1,12 +1,10 @@ -import type { Thread, TurnDiffSummary } from "../types"; +import type { TurnDiffSummary } from "../types"; interface CheckpointDiffAvailabilityInput { - activeThread: Thread | undefined; selectedTurn: TurnDiffSummary | undefined; } export function getUnavailableCheckpointDiffMessage({ - activeThread, selectedTurn, }: CheckpointDiffAvailabilityInput): string | null { if (!selectedTurn) { @@ -18,10 +16,9 @@ export function getUnavailableCheckpointDiffMessage({ return null; } - const isLatestTurn = activeThread?.latestTurn?.turnId === selectedTurn.turnId; - if (isLatestTurn) { - return null; + if (selectedTurn.status === "missing") { + return "Checkpoint is marked as missing and cannot be restored."; } - return "Checkpoint diff is unavailable for this turn."; + return "Checkpoint reference is unavailable for this turn."; }