diff options
| -rw-r--r-- | backend-handoff.md | 22 | ||||
| -rw-r--r-- | src/core/chunks/reducer.test.ts | 80 | ||||
| -rw-r--r-- | src/core/chunks/reducer.ts | 11 | ||||
| -rw-r--r-- | src/core/chunks/trim.test.ts | 9 | ||||
| -rw-r--r-- | src/core/chunks/trim.ts | 32 | ||||
| -rw-r--r-- | src/features/chat/store.svelte.ts | 5 | ||||
| -rw-r--r-- | src/features/chat/store.test.ts | 8 |
7 files changed, 49 insertions, 118 deletions
diff --git a/backend-handoff.md b/backend-handoff.md index 4b3a8d6..8e86ce4 100644 --- a/backend-handoff.md +++ b/backend-handoff.md @@ -64,17 +64,17 @@ The backend now persists chunks **incrementally at step boundaries** during gene FE's existing `syncTail` already polls this — it will find new chunks as each step completes. No wire/transport-contract change needed (`StoredChunk` already has `seq`; `AgentEvent` types unchanged). -**FE adoption plan (option (c) from the CR):** -- Fold events for the **current in-progress step** only (streaming text, thinking dots) — provisional state shrinks to one step's worth of chunks, never a trim concern. -- `syncTail` for **sealed steps** — picks up committed chunks incrementally as each step completes. -- `trimTranscript` drops oldest committed chunks uniformly — no special provisional case. -- `turn-sealed` becomes a "refresh" signal — all chunks already committed. -- "Show earlier" works uniformly for all turns (including in-flight — sealed steps have seq). -- Remove `hiddenThinkingCount` (no provisional thinking blocks to track) and the `sealedTurnId` flush. - -**Not yet implemented.** The FE currently still uses the provisional/committed split. The interim -fix (frontend-only cache for dropped provisional chunks) was not built — CR-6 resolution makes it -unnecessary. Adoption is the next FE task when prioritized. +**FE adoption: NOT pursuing syncTail-during-generation.** Investigation revealed +the kernel emits `step-complete` (line 360 of `run-turn.ts`) BEFORE calling +`onStepComplete` (line 542) — the step's chunks are persisted only AFTER tool +results come back, not when `step-complete` fires. So `syncTail` triggered by +`step-complete` finds nothing. Moving the emission after `onStepComplete` would +be a kernel change. + +Instead, the FE now trims provisional chunks directly in `trimTranscript` when +committed is exhausted — no `syncTail` needed. Dropped provisional chunks are +lost temporarily (no "Show earlier" for them) but come back as committed when +the turn seals and `syncTail` fetches everything. --- diff --git a/src/core/chunks/reducer.test.ts b/src/core/chunks/reducer.test.ts index c479488..a346545 100644 --- a/src/core/chunks/reducer.test.ts +++ b/src/core/chunks/reducer.test.ts @@ -551,86 +551,6 @@ describe("applyHistory", () => { expect(s.committed).toHaveLength(2); expect(s.committed.map((c) => c.seq)).toEqual([1, 2]); }); - - it("clears provisional when new committed chunks arrive during generation (CR-6)", () => { - let s = initialState(); - s = foldEvent(s, turnStart("t1")); - s = foldEvent(s, textDelta("t1", "hello")); - s = foldEvent(s, toolCall("t1", "tc1", "run_shell", {})); - s = foldEvent(s, toolResult("t1", "tc1", "run_shell", "output")); - expect(s.generating).toBe(true); - expect(s.provisional.length).toBeGreaterThan(0); - - s = applyHistory(s, [ - storedChunk(1, "assistant", { type: "text", text: "hello" }), - storedChunk(2, "assistant", { - type: "tool-call", - toolCallId: "tc1", - toolName: "run_shell", - input: {}, - stepId: "s0" as StepId, - }), - storedChunk(3, "tool", { - type: "tool-result", - toolCallId: "tc1", - toolName: "run_shell", - content: "output", - isError: false, - stepId: "s0" as StepId, - }), - ]); - - expect(s.provisional).toEqual([]); - expect(s.generating).toBe(true); - expect(s.committed).toHaveLength(3); - }); - - it("keeps provisional when no new committed chunks arrive during generation", () => { - let s = initialState(); - s = applyHistory(s, [storedChunk(1, "user", { type: "text", text: "q" })]); - s = foldEvent(s, turnStart("t1")); - s = foldEvent(s, textDelta("t1", "hello")); - s = foldEvent(s, toolCall("t1", "tc1", "run_shell", {})); - // At this point: provisional has [text "hello", tool-call] - expect(s.provisional.length).toBeGreaterThan(0); - - // applyHistory with chunks already in committed — no new additions - s = applyHistory(s, [storedChunk(1, "user", { type: "text", text: "q" })]); - - expect(s.provisional.length).toBeGreaterThan(0); - }); - - it("keeps accumulating when clearing provisional during generation (CR-6)", () => { - let s = initialState(); - s = applyHistory(s, [storedChunk(1, "user", { type: "text", text: "q" })]); - s = foldEvent(s, turnStart("t1")); - s = foldEvent(s, toolCall("t1", "tc1", "run_shell", {})); - s = foldEvent(s, toolResult("t1", "tc1", "run_shell", "output")); - // Start accumulating text for the NEXT step - s = foldEvent(s, textDelta("t1", "streaming...")); - expect(s.accumulating).not.toBeNull(); - - s = applyHistory(s, [ - storedChunk(2, "assistant", { - type: "tool-call", - toolCallId: "tc1", - toolName: "run_shell", - input: {}, - stepId: "s0" as StepId, - }), - storedChunk(3, "tool", { - type: "tool-result", - toolCallId: "tc1", - toolName: "run_shell", - content: "output", - isError: false, - stepId: "s0" as StepId, - }), - ]); - - expect(s.provisional).toEqual([]); - expect(s.accumulating).not.toBeNull(); - }); }); describe("selectChunks", () => { diff --git a/src/core/chunks/reducer.ts b/src/core/chunks/reducer.ts index 37f0164..035846c 100644 --- a/src/core/chunks/reducer.ts +++ b/src/core/chunks/reducer.ts @@ -54,10 +54,8 @@ export function applyHistory( ): TranscriptState { const seqMap = new Map<number, StoredChunk>(); for (const c of state.committed) seqMap.set(c.seq, c); - let addedNew = false; for (const c of chunks) { if (c.seq < state.hiddenBeforeSeq) continue; - if (!seqMap.has(c.seq)) addedNew = true; seqMap.set(c.seq, c); } const committed = Array.from(seqMap.values()).sort((a, b) => a.seq - b.seq); @@ -72,15 +70,6 @@ export function applyHistory( }; } - // During generation: if new committed chunks arrived (CR-6 — backend - // persists at step boundaries), clear provisional chunks. They're - // duplicates — the same content was folded from live events but is now - // persisted with seq. Keep the accumulating chunk (current in-progress - // step, not yet persisted). - if (addedNew && state.generating) { - return { ...state, committed, provisional: [], accumulating: state.accumulating }; - } - return { ...state, committed }; } diff --git a/src/core/chunks/trim.test.ts b/src/core/chunks/trim.test.ts index 7914f35..aa4b0e3 100644 --- a/src/core/chunks/trim.test.ts +++ b/src/core/chunks/trim.test.ts @@ -86,7 +86,7 @@ describe("trimTranscript", () => { expect(next.hiddenBeforeSeq).toBe(51); }); - it("counts provisional + accumulating toward the limit but never drops them", () => { + it("counts provisional + accumulating toward the limit (drops committed first)", () => { const base = stateWith(chunks(1, 98)); const state: TranscriptState = { ...base, @@ -103,17 +103,18 @@ describe("trimTranscript", () => { expect(next.accumulating).not.toBeNull(); }); - it("caps the drop at the committed length", () => { + it("drops oldest provisional when committed is exhausted", () => { const base = stateWith(chunks(1, 2)); const provisional = Array.from({ length: 20 }, (_, i) => ({ role: "assistant" as const, chunk: { type: "text" as const, text: `p${i}` }, })); const state: TranscriptState = { ...base, provisional }; + // 2 + 20 = 22 > 10. quarter = 3. Drop 2 committed, then drop + // ceil((20-10)/3)*3 = 12 provisional → 8 remain. const next = trimTranscript(state, 10); expect(next.committed).toHaveLength(0); - expect(next.provisional).toHaveLength(20); - // Watermark advances past the last dropped committed chunk. + expect(next.provisional).toHaveLength(8); expect(next.hiddenBeforeSeq).toBe(3); }); diff --git a/src/core/chunks/trim.ts b/src/core/chunks/trim.ts index 94065b3..7791721 100644 --- a/src/core/chunks/trim.ts +++ b/src/core/chunks/trim.ts @@ -84,7 +84,9 @@ function dropOldest(state: TranscriptState, drop: number): TranscriptState { * quarters (`unloadCount(limit)` each) of the OLDEST committed chunks until back * at/under the limit — normally exactly one quarter (limit 100: 101 → 76); more * only when trimming was deferred (e.g. while the reader was scrolled up). - * At/under the limit this is the identity. Never drops provisional chunks. + * At/under the limit this is the identity. When committed chunks are + * exhausted, also drops the oldest provisional chunks (the in-flight turn) + * to keep the browser responsive during very long turns. */ export function trimTranscript(state: TranscriptState, limit: number): TranscriptState { if (!Number.isFinite(limit) || limit <= 0) return state; @@ -92,9 +94,31 @@ export function trimTranscript(state: TranscriptState, limit: number): Transcrip if (total <= limit) return state; const quarter = unloadCount(limit); const passes = Math.ceil((total - limit) / quarter); - const drop = Math.min(passes * quarter, state.committed.length); - if (drop <= 0) return state; - return dropOldest(state, drop); + + // First, drop oldest committed chunks (the usual path). + const committedDrop = Math.min(passes * quarter, state.committed.length); + let next = committedDrop > 0 ? dropOldest(state, committedDrop) : state; + + // If still over the limit and committed is exhausted, drop oldest + // provisional chunks (the in-flight turn). These chunks have no seq + // (not yet persisted) and can't be "Show earlier" — but dropping them + // keeps the browser responsive. They'll come back as committed when + // the turn seals and syncTail fetches them from the server. + const remaining = totalCount(next); + if (remaining > limit && next.provisional.length > 0) { + const provisionalDrop = Math.min( + Math.ceil((remaining - limit) / quarter) * quarter, + next.provisional.length, + ); + if (provisionalDrop > 0) { + next = { + ...next, + provisional: next.provisional.slice(provisionalDrop), + }; + } + } + + return next; } /** diff --git a/src/features/chat/store.svelte.ts b/src/features/chat/store.svelte.ts index f6c23fc..9beabfc 100644 --- a/src/features/chat/store.svelte.ts +++ b/src/features/chat/store.svelte.ts @@ -284,11 +284,6 @@ export function createChatStore(deps: ChatStoreDependencies): ChatStore { if (transcript.sealedTurnId !== null) { void syncTail(); void syncMetrics(); - } else if (msg.event.type === "step-complete") { - // CR-6: backend persists chunks at step boundaries. Fetch them - // as committed so trimTranscript can unload oldest chunks - // uniformly — no unbounded provisional growth during long turns. - void syncTail(); } }, diff --git a/src/features/chat/store.test.ts b/src/features/chat/store.test.ts index 2d75139..c1d62a6 100644 --- a/src/features/chat/store.test.ts +++ b/src/features/chat/store.test.ts @@ -1057,8 +1057,9 @@ describe("createChatStore", () => { } expect(store.chunks).toHaveLength(15); - // Reader returns to the bottom — but provisional chunks are never unloaded, - // so the deferred trim still can't shrink an all-provisional transcript. + // Reader returns to the bottom — the deferred trim now catches up. + // With no committed chunks, it drops the oldest provisional chunks + // (the in-flight turn) to stay within the limit. atBottom = true; store.handleDelta( deltaEvent({ @@ -1071,7 +1072,8 @@ describe("createChatStore", () => { stepId: "t1#15" as StepId, }), ); - expect(store.chunks).toHaveLength(16); + // 16 provisional, limit 10, quarter 3 → drop 6 oldest → 10 remain. + expect(store.chunks).toHaveLength(10); store.dispose(); }); |