From b3c830237206a319348a0eb7078b49e20f60883b Mon Sep 17 00:00:00 2001 From: Adam Malczewski Date: Mon, 22 Jun 2026 14:18:56 +0900 Subject: fix: trim provisional chunks during long turns (browser stays responsive) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit trimTranscript now drops oldest provisional chunks (the in-flight turn) when committed chunks are exhausted. Previously it bailed with drop=0 when committed was empty, allowing unbounded provisional growth during long generating turns (300+ chunks → browser crawls). Root cause of the syncTail approach failing: the kernel emits step-complete (line 360) BEFORE calling onStepComplete (line 542) — chunks are persisted only after tool results come back, not when step-complete fires. So syncTail on step-complete found nothing. Reverted the applyHistory + syncTail-on-step-complete changes from 4e1d041. The new approach is simpler: trim provisional directly in trimTranscript. Dropped chunks are lost temporarily (no Show Earlier) but come back as committed when the turn seals and syncTail fetches everything from the server. 686 tests green. --- backend-handoff.md | 22 +++++------ src/core/chunks/reducer.test.ts | 80 --------------------------------------- src/core/chunks/reducer.ts | 11 ------ src/core/chunks/trim.test.ts | 9 +++-- src/core/chunks/trim.ts | 32 ++++++++++++++-- src/features/chat/store.svelte.ts | 5 --- 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(); 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(); }); -- cgit v1.2.3