From 856cfacc7ac54ecea6ebd3fad0b923033016b25b Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 02:02:33 -0700 Subject: [PATCH 1/6] fix(semantic): bootstrap-complete archives a reopened replayed turn without duplicating it #1290: the bootstrap-complete reconciler archived a still-open replayed turn with a raw append while every other path replaces by turnId, so a replayed turn already in history that reopened as currentTurn was held twice (duplicate sem:T:i candidates and semantic-block:T:i keys). It now uses appendSemanticHistory. Red before on the real subscription hub. Co-Authored-By: Claude Opus 5.5 --- ...09-27-semantic-history-bootstrap-dedupe.md | 10 +++ .../bootstrapHistoryDedupe.renderer.test.tsx | 82 +++++++++++++++++++ .../workspace/hook/ipc/useIpcSubscriptions.ts | 14 ++-- 3 files changed, 100 insertions(+), 6 deletions(-) create mode 100644 docs/plans/2026-09-27-semantic-history-bootstrap-dedupe.md create mode 100644 src/renderer/src/workspace/hook/ipc/bootstrapHistoryDedupe.renderer.test.tsx diff --git a/docs/plans/2026-09-27-semantic-history-bootstrap-dedupe.md b/docs/plans/2026-09-27-semantic-history-bootstrap-dedupe.md new file mode 100644 index 000000000..211a280fd --- /dev/null +++ b/docs/plans/2026-09-27-semantic-history-bootstrap-dedupe.md @@ -0,0 +1,10 @@ +# Bootstrap-complete archives a reopened turn without duplicating it (#1290) + +## Problem +The bootstrap-complete reconciler (`useIpcSubscriptions.ts`) archives a still-open replayed turn with a raw `[...history, row]` append. Every other archive path uses `appendSemanticHistory`, which replaces by `turnId`. A replayed turn T already in history can reopen as `currentTurn`, and the ledger hides the copy while T is live. Bootstrap-complete then appended T again: two history rows with one turnId, repeated `sem:T:i` candidate ids, and duplicate `semantic-block:T:i` React keys. Found by the Stage 3 C8 hunt (`temp/quality-loop/hunt-c8.md`). + +## Fix +The reconciler uses `appendSemanticHistory(next.semantic.history, closedTurn)`, the same helper as the other paths. It also applies the same `SEMANTIC_HISTORY_CAP`. + +## Test +`bootstrapHistoryDedupe.renderer.test.tsx` drives the real `useIpcSubscriptions` through the fake session feed. It seeds T archived AND reopened, sends a replay burst, and lets the bootstrap-complete timer fire. It asserts one `turn-T` row. It was red before the fix (`['turn-T', 'turn-T']`). diff --git a/src/renderer/src/workspace/hook/ipc/bootstrapHistoryDedupe.renderer.test.tsx b/src/renderer/src/workspace/hook/ipc/bootstrapHistoryDedupe.renderer.test.tsx new file mode 100644 index 000000000..ab65f6779 --- /dev/null +++ b/src/renderer/src/workspace/hook/ipc/bootstrapHistoryDedupe.renderer.test.tsx @@ -0,0 +1,82 @@ +import { afterEach, expect, it, vi } from 'vitest' +import { render } from '@testing-library/react' +import { act, useRef } from 'react' + +import { createFakeSessionFeed } from '@renderer/features/sessionFeed/FakeSessionFeed' +import { emptyRuntime } from '@renderer/session-runtime/state' +import type { SessionRuntime } from '@renderer/session-runtime/state' +import { semanticHistoryRow } from '@renderer/session-runtime/semantic/helpers' +import type { SessionId, WorkspaceState } from '@renderer/workspace/types' +import type { WorkspaceRefs } from '@renderer/workspace/hook/refs' + +import { useIpcSubscriptions } from './useIpcSubscriptions' +import { makeWorkspaceRefsForTest as makeRefs } from './testing/workspaceRefsForTest' + +// #1290: bootstrap-complete archives a still-open replayed turn with a RAW +// append, while every other archive path (appendSemanticHistory) replaces by +// turnId. A replayed turn T already in history reopens as currentTurn; the +// ledger hides the copy while T is live, then bootstrap-complete appended T +// again: two history rows with one turnId, repeated `sem:T:i` candidate ids +// and duplicate `semantic-block:T:i` React keys. +const originalWindowApi = window.api +afterEach(() => { + vi.useRealTimers() + Object.defineProperty(window, 'api', { configurable: true, value: originalWindowApi }) +}) + +const turn = (endedAt: number | null) => ({ + turnId: 'turn-T', source: 'rollout' as const, text: 'replayed answer', blocks: {}, blockOrder: [], + stopReason: null, usage: null, + task: { todos: [], doneCount: 0, totalCount: 0, inProgressToolUseIds: [], activeToolNames: [] }, + startedAt: 1, endedAt, + lookups: { toolCallsById: {}, toolUseIdsInOrder: [], resolvedToolUseIds: [], erroredToolUseIds: [] }, +}) + +it('archives a reopened replayed turn at bootstrap-complete without duplicating it in history', () => { + vi.useFakeTimers() + const fake = createFakeSessionFeed() + const sessionId = 'bootstrap-dedupe' as SessionId + let workspaceState = { sessions: { [sessionId]: { cwd: '/repo', kind: 'claude' } } } as unknown as WorkspaceState + let runtimes: Record = { + [sessionId]: { + ...emptyRuntime(), + semantic: { + ...emptyRuntime().semantic, + // T is already archived AND reopened as the current turn by replay. + history: [semanticHistoryRow(turn(2) as never)], + currentTurn: turn(null) as never, + }, + }, + } + let refsForTest!: WorkspaceRefs + const commitRuntimes = (updater: Record | ((current: Record) => Record)): void => { + runtimes = typeof updater === 'function' ? updater(runtimes) : updater + refsForTest.latestRuntimesRef.current = runtimes + } + function Harness(): React.JSX.Element { + const refs = useRef(null) + if (refs.current === null) { + refs.current = makeRefs(workspaceState) + refs.current.latestRuntimesRef.current = runtimes + refsForTest = refs.current + } + useIpcSubscriptions(fake, refs.current, updater => { + workspaceState = typeof updater === 'function' ? updater(workspaceState) : updater + refs.current!.stateRef.current = workspaceState + refs.current!.latestStateRef.current = workspaceState + }, commitRuntimes, () => {}, () => {}) + return
+ } + Object.defineProperty(window, 'api', { configurable: true, value: { gitWorktrees: vi.fn(async () => ({ ok: false })) } }) + render() + + // A replay burst, then quiet: the bootstrap-complete reconciler runs. + act(() => { + fake.emitJsonlEntries({ sessionId, entries: [{ file: '/s/claude.jsonl', entry: { type: 'user', uuid: 'u1', message: { role: 'user', content: 'hi' }, timestamp: '2026-09-27T00:00:00.000Z' } as never }] }) + }) + act(() => { vi.advanceTimersByTime(1_000) }) + + const semantic = runtimes[sessionId]!.semantic + expect(semantic.currentTurn).toBeNull() + expect(semantic.history.map(row => row.turnId)).toEqual(['turn-T']) +}) diff --git a/src/renderer/src/workspace/hook/ipc/useIpcSubscriptions.ts b/src/renderer/src/workspace/hook/ipc/useIpcSubscriptions.ts index 7c48f18fa..c91024b1d 100644 --- a/src/renderer/src/workspace/hook/ipc/useIpcSubscriptions.ts +++ b/src/renderer/src/workspace/hook/ipc/useIpcSubscriptions.ts @@ -22,10 +22,9 @@ import { appendFeedDebugLog } from '@renderer/session-runtime/feedDebug' import type { FeedDebugInput } from '@renderer/session-runtime/feedDebug' import type { SessionId } from '@renderer/workspace/types' import { + appendSemanticHistory, hasPendingSemanticTools, isSemanticTurnRunning, - semanticHistoryRow, - SEMANTIC_HISTORY_CAP, withDerivedSessionStatus, } from '@renderer/session-runtime/semantic/helpers' import { stepLiveSemantic } from '@renderer/session-runtime/ingest/liveSemantic' @@ -2781,10 +2780,13 @@ export function useIpcSubscriptions( ...next, semantic: { ...next.semantic, - history: [ - ...next.semantic.history, - semanticHistoryRow(closedTurn), - ].slice(-SEMANTIC_HISTORY_CAP), + // WHY appendSemanticHistory and not a raw append (#1290): + // every other archive path replaces by turnId. A replayed + // turn already in history can reopen as currentTurn (the + // ledger hides the copy while it is live); a raw append here + // then held that turn twice, repeating its `sem:T:i` + // candidate ids and `semantic-block:T:i` React keys. + history: appendSemanticHistory(next.semantic.history, closedTurn), currentTurn: null, }, } From 07c675c45d989cb1d35ea608853850d8aa569e53 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 02:15:09 -0700 Subject: [PATCH 2/6] fix(semantic): a thinner replay copy never displaces the archived turn at bootstrap-complete #1391 review a: a replayed turn_started reopens T empty; if replay quiets before its content re-arrives, replacing by turnId swapped the full archived T for the empty copy and the answer vanished from the feed. archiveReplayedTurn keeps the archived row unless the replay copy renders at least as much (text and blocks); still one row per turnId. Live fold paths keep replace-by-id (a newer live copy is authoritative). Text and block rules each pinned; red before on the reviewer's sequence. Co-Authored-By: Claude Opus 5.5 --- .../semantic/archiveReplayedTurn.test.ts | 34 +++++++++++++++++++ .../src/session-runtime/semantic/helpers.ts | 29 ++++++++++++++++ .../bootstrapHistoryDedupe.renderer.test.tsx | 23 ++++++++++--- .../workspace/hook/ipc/useIpcSubscriptions.ts | 6 ++-- 4 files changed, 85 insertions(+), 7 deletions(-) create mode 100644 src/renderer/src/session-runtime/semantic/archiveReplayedTurn.test.ts diff --git a/src/renderer/src/session-runtime/semantic/archiveReplayedTurn.test.ts b/src/renderer/src/session-runtime/semantic/archiveReplayedTurn.test.ts new file mode 100644 index 000000000..37883ca80 --- /dev/null +++ b/src/renderer/src/session-runtime/semantic/archiveReplayedTurn.test.ts @@ -0,0 +1,34 @@ +import { expect, it } from 'vitest' + +import { archiveReplayedTurn, semanticHistoryRow } from './helpers' + +// #1391 review a: a replay copy only displaces the archived turn when it +// renders at least as much (text AND blocks); either way one row per turnId. +const turn = (text: string, blocks: string[], endedAt: number | null = 2) => ({ + turnId: 'T', source: 'rollout' as const, text, + blocks: Object.fromEntries(blocks.map(id => [id, { id }])), blockOrder: blocks, + stopReason: null, usage: null, + task: { todos: [], doneCount: 0, totalCount: 0, inProgressToolUseIds: [], activeToolNames: [] }, + startedAt: 1, endedAt, + lookups: { toolCallsById: {}, toolUseIdsInOrder: [], resolvedToolUseIds: [], erroredToolUseIds: [] }, +}) as never + +it('keeps the archived turn when the replay copy has fewer blocks', () => { + const history = [semanticHistoryRow(turn('answer', ['b1', 'b2']))] + expect(archiveReplayedTurn(history, turn('answer', []))).toBe(history) +}) + +it('keeps the archived turn when the replay copy has less text', () => { + const history = [semanticHistoryRow(turn('answer', ['b1']))] + expect(archiveReplayedTurn(history, turn('', ['b1']))).toBe(history) +}) + +it('lets an equal or richer replay copy replace the archived one, once', () => { + const history = [semanticHistoryRow(turn('answer', ['b1']))] + const next = archiveReplayedTurn(history, turn('answer, continued', ['b1', 'b2'])) + expect(next.map(row => [row.turnId, row.text])).toEqual([['T', 'answer, continued']]) +}) + +it('archives a turn that is not in history yet', () => { + expect(archiveReplayedTurn([], turn('answer', ['b1'])).map(row => row.turnId)).toEqual(['T']) +}) diff --git a/src/renderer/src/session-runtime/semantic/helpers.ts b/src/renderer/src/session-runtime/semantic/helpers.ts index 14d8c3942..a9722bcda 100644 --- a/src/renderer/src/session-runtime/semantic/helpers.ts +++ b/src/renderer/src/session-runtime/semantic/helpers.ts @@ -138,6 +138,35 @@ export function appendSemanticHistory( ].slice(-SEMANTIC_HISTORY_CAP) } +/** + * Archive a turn that REPLAY reopened and left open, keeping whichever copy + * of it renders more (#1391 review a). + * + * WHY not plain appendSemanticHistory here: replay can reopen an archived turn + * T with a repeated `turn_started`, which starts it EMPTY (foldEvent). If + * replay then quiets before T's content re-arrives, replacing by turnId swapped + * the full archived T for the empty copy, and with no durable assistant entry + * yet the answer vanished from the feed. Here the reopened copy only replaces + * the archived one when it carries at least as much renderable content (text + * and blocks). Otherwise the archived row stays, still exactly one per turnId. + * + * Only the bootstrap-complete path uses this: on the live fold paths a newer + * copy of the same turn is the authoritative one and should win. + */ +export function archiveReplayedTurn( + history: SemanticRuntimeState['history'], + turn: SemanticLiveTurn, +): SemanticRuntimeState['history'] { + const archived = history.find(existing => existing.turnId === turn.turnId) + if ( + archived && + (turn.text.length < archived.text.length || turn.blockOrder.length < archived.blockOrder.length) + ) { + return history + } + return appendSemanticHistory(history, turn) +} + /** True when the turn is still live — hasn't received its * terminal `turn_stopped`/`turn_completed` yet. Used by session- * status derivation and the foldSemanticEvent branch that decides diff --git a/src/renderer/src/workspace/hook/ipc/bootstrapHistoryDedupe.renderer.test.tsx b/src/renderer/src/workspace/hook/ipc/bootstrapHistoryDedupe.renderer.test.tsx index ab65f6779..263b09efa 100644 --- a/src/renderer/src/workspace/hook/ipc/bootstrapHistoryDedupe.renderer.test.tsx +++ b/src/renderer/src/workspace/hook/ipc/bootstrapHistoryDedupe.renderer.test.tsx @@ -24,15 +24,15 @@ afterEach(() => { Object.defineProperty(window, 'api', { configurable: true, value: originalWindowApi }) }) -const turn = (endedAt: number | null) => ({ - turnId: 'turn-T', source: 'rollout' as const, text: 'replayed answer', blocks: {}, blockOrder: [], +const turn = (endedAt: number | null, text = 'replayed answer') => ({ + turnId: 'turn-T', source: 'rollout' as const, text, blocks: {}, blockOrder: [], stopReason: null, usage: null, task: { todos: [], doneCount: 0, totalCount: 0, inProgressToolUseIds: [], activeToolNames: [] }, startedAt: 1, endedAt, lookups: { toolCallsById: {}, toolUseIdsInOrder: [], resolvedToolUseIds: [], erroredToolUseIds: [] }, }) -it('archives a reopened replayed turn at bootstrap-complete without duplicating it in history', () => { +function run(reopened: ReturnType) { vi.useFakeTimers() const fake = createFakeSessionFeed() const sessionId = 'bootstrap-dedupe' as SessionId @@ -44,7 +44,7 @@ it('archives a reopened replayed turn at bootstrap-complete without duplicating ...emptyRuntime().semantic, // T is already archived AND reopened as the current turn by replay. history: [semanticHistoryRow(turn(2) as never)], - currentTurn: turn(null) as never, + currentTurn: reopened as never, }, }, } @@ -76,7 +76,20 @@ it('archives a reopened replayed turn at bootstrap-complete without duplicating }) act(() => { vi.advanceTimersByTime(1_000) }) - const semantic = runtimes[sessionId]!.semantic + return runtimes[sessionId]!.semantic +} + +it('archives a reopened replayed turn at bootstrap-complete without duplicating it in history', () => { + const semantic = run(turn(null)) expect(semantic.currentTurn).toBeNull() expect(semantic.history.map(row => row.turnId)).toEqual(['turn-T']) }) + +// #1391 review a (major): a replayed turn_started reopens T EMPTY; if replay +// quiets before T's content re-arrives, the empty copy must not replace the +// archived one, or the answer vanishes from the feed (no durable entry yet). +it('keeps the richer archived copy when the reopened replay copy has less content', () => { + const semantic = run(turn(null, '')) + expect(semantic.currentTurn).toBeNull() + expect(semantic.history.map(row => [row.turnId, row.text])).toEqual([['turn-T', 'replayed answer']]) +}) diff --git a/src/renderer/src/workspace/hook/ipc/useIpcSubscriptions.ts b/src/renderer/src/workspace/hook/ipc/useIpcSubscriptions.ts index c91024b1d..a54df4356 100644 --- a/src/renderer/src/workspace/hook/ipc/useIpcSubscriptions.ts +++ b/src/renderer/src/workspace/hook/ipc/useIpcSubscriptions.ts @@ -22,7 +22,7 @@ import { appendFeedDebugLog } from '@renderer/session-runtime/feedDebug' import type { FeedDebugInput } from '@renderer/session-runtime/feedDebug' import type { SessionId } from '@renderer/workspace/types' import { - appendSemanticHistory, + archiveReplayedTurn, hasPendingSemanticTools, isSemanticTurnRunning, withDerivedSessionStatus, @@ -2786,7 +2786,9 @@ export function useIpcSubscriptions( // ledger hides the copy while it is live); a raw append here // then held that turn twice, repeating its `sem:T:i` // candidate ids and `semantic-block:T:i` React keys. - history: appendSemanticHistory(next.semantic.history, closedTurn), + // …and never lets a THINNER replay copy displace the archived + // one (#1391 review a): see archiveReplayedTurn. + history: archiveReplayedTurn(next.semantic.history, closedTurn), currentTurn: null, }, } From f7b65430374fa33d56804bbf83556202298073bf Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 02:26:04 -0700 Subject: [PATCH 3/6] fix(semantic): compare renderable content, not counts, before a replay copy replaces the archived turn #1391 review a, round 2: a replay can reopen the same block EMPTY (block_started then quiet before text_delta), so equal block counts still let an empty block displace the archived answer. archiveReplayedTurn now compares renderable size: turn text plus each block's text, thinking, tool input and tool result. Each term, the equal-replaces rule and the reviewer's probe are pinned. Co-Authored-By: Claude Opus 5.5 --- .../semantic/archiveReplayedTurn.test.ts | 55 ++++++++++++++----- .../src/session-runtime/semantic/helpers.ts | 27 ++++++--- 2 files changed, 60 insertions(+), 22 deletions(-) diff --git a/src/renderer/src/session-runtime/semantic/archiveReplayedTurn.test.ts b/src/renderer/src/session-runtime/semantic/archiveReplayedTurn.test.ts index 37883ca80..c400aac47 100644 --- a/src/renderer/src/session-runtime/semantic/archiveReplayedTurn.test.ts +++ b/src/renderer/src/session-runtime/semantic/archiveReplayedTurn.test.ts @@ -2,33 +2,60 @@ import { expect, it } from 'vitest' import { archiveReplayedTurn, semanticHistoryRow } from './helpers' -// #1391 review a: a replay copy only displaces the archived turn when it -// renders at least as much (text AND blocks); either way one row per turnId. -const turn = (text: string, blocks: string[], endedAt: number | null = 2) => ({ +// #1391 review a (rounds 1 and 2): a replay copy only displaces the archived +// turn when it renders at least as much, measured as content (turn text plus +// block text, thinking, tool input and result), not counts. Always one row per +// turnId. +type Block = { text?: string; thinking?: string; inputJson?: string; resultContent?: string } +const turn = (text: string, blocks: Block[], endedAt: number | null = 2) => ({ turnId: 'T', source: 'rollout' as const, text, - blocks: Object.fromEntries(blocks.map(id => [id, { id }])), blockOrder: blocks, + blocks: Object.fromEntries(blocks.map((block, index) => [index, { blockIndex: index, kind: 'text', ...block }])), + blockOrder: blocks.map((_, index) => index), stopReason: null, usage: null, task: { todos: [], doneCount: 0, totalCount: 0, inProgressToolUseIds: [], activeToolNames: [] }, startedAt: 1, endedAt, lookups: { toolCallsById: {}, toolUseIdsInOrder: [], resolvedToolUseIds: [], erroredToolUseIds: [] }, }) as never -it('keeps the archived turn when the replay copy has fewer blocks', () => { - const history = [semanticHistoryRow(turn('answer', ['b1', 'b2']))] - expect(archiveReplayedTurn(history, turn('answer', []))).toBe(history) +it('keeps the archived turn when the replay copy has less turn text', () => { + const history = [semanticHistoryRow(turn('answer', []))] + expect(archiveReplayedTurn(history, turn('', []))).toBe(history) }) -it('keeps the archived turn when the replay copy has less text', () => { - const history = [semanticHistoryRow(turn('answer', ['b1']))] - expect(archiveReplayedTurn(history, turn('', ['b1']))).toBe(history) +it('keeps the archived turn when the replay copy has fewer or emptier blocks', () => { + const history = [semanticHistoryRow(turn('', [{ text: 'first' }, { text: 'second' }]))] + expect(archiveReplayedTurn(history, turn('', [{ text: 'first' }]))).toBe(history) +}) + +// Round 2: equal block COUNT, but the replay's block is still empty. +it('keeps the archived answer when the replay reopened the same block empty', () => { + const history = [semanticHistoryRow(turn('', [{ text: 'answer' }]))] + expect(archiveReplayedTurn(history, turn('', [{ text: '' }]))).toBe(history) +}) + +it('counts thinking as content', () => { + const history = [semanticHistoryRow(turn('', [{ thinking: 'reasoning' }]))] + expect(archiveReplayedTurn(history, turn('', [{ thinking: '' }]))).toBe(history) +}) + +it('counts tool input as content', () => { + const history = [semanticHistoryRow(turn('', [{ inputJson: '{"a":1}' }]))] + expect(archiveReplayedTurn(history, turn('', [{ inputJson: '' }]))).toBe(history) +}) + +it('counts a tool result as content', () => { + const history = [semanticHistoryRow(turn('', [{ resultContent: 'exit 0' }]))] + expect(archiveReplayedTurn(history, turn('', [{ resultContent: '' }]))).toBe(history) }) it('lets an equal or richer replay copy replace the archived one, once', () => { - const history = [semanticHistoryRow(turn('answer', ['b1']))] - const next = archiveReplayedTurn(history, turn('answer, continued', ['b1', 'b2'])) - expect(next.map(row => [row.turnId, row.text])).toEqual([['T', 'answer, continued']]) + const history = [semanticHistoryRow(turn('', [{ text: 'answer' }]))] + const equal = archiveReplayedTurn(history, turn('', [{ text: 'ANSWER' }])) + expect(equal.map(row => [row.turnId, (row.blocks as Record)[0]?.text])).toEqual([['T', 'ANSWER']]) + const richer = archiveReplayedTurn(history, turn('', [{ text: 'answer, continued' }])) + expect(richer.map(row => [row.turnId, (row.blocks as Record)[0]?.text])).toEqual([['T', 'answer, continued']]) }) it('archives a turn that is not in history yet', () => { - expect(archiveReplayedTurn([], turn('answer', ['b1'])).map(row => row.turnId)).toEqual(['T']) + expect(archiveReplayedTurn([], turn('answer', [])).map(row => row.turnId)).toEqual(['T']) }) diff --git a/src/renderer/src/session-runtime/semantic/helpers.ts b/src/renderer/src/session-runtime/semantic/helpers.ts index a9722bcda..d3be0a33c 100644 --- a/src/renderer/src/session-runtime/semantic/helpers.ts +++ b/src/renderer/src/session-runtime/semantic/helpers.ts @@ -147,8 +147,15 @@ export function appendSemanticHistory( * replay then quiets before T's content re-arrives, replacing by turnId swapped * the full archived T for the empty copy, and with no durable assistant entry * yet the answer vanished from the feed. Here the reopened copy only replaces - * the archived one when it carries at least as much renderable content (text - * and blocks). Otherwise the archived row stays, still exactly one per turnId. + * the archived one when it carries at least as much renderable content; + * otherwise the archived row stays, still exactly one per turnId. + * + * WHY content size and not counts (#1391 review a, round 2): the reopened + * copy's BLOCKS can be empty too. Replay re-emits `block_started` for index 0 + * and quiets before `text_delta`, so an equal block count held an empty block + * where the archived turn held the answer. renderableSize measures what the + * feed can actually paint (turn text plus each block's text, thinking, tool + * input and tool result). * * Only the bootstrap-complete path uses this: on the live fold paths a newer * copy of the same turn is the authoritative one and should win. @@ -158,15 +165,19 @@ export function archiveReplayedTurn( turn: SemanticLiveTurn, ): SemanticRuntimeState['history'] { const archived = history.find(existing => existing.turnId === turn.turnId) - if ( - archived && - (turn.text.length < archived.text.length || turn.blockOrder.length < archived.blockOrder.length) - ) { - return history - } + if (archived && renderableSize(turn) < renderableSize(archived)) return history return appendSemanticHistory(history, turn) } +function renderableSize(turn: SemanticLiveTurn): number { + let size = turn.text.length + for (const block of Object.values(turn.blocks)) { + size += (block.text?.length ?? 0) + (block.thinking?.length ?? 0) + + (block.inputJson?.length ?? 0) + (block.resultContent?.length ?? 0) + } + return size +} + /** True when the turn is still live — hasn't received its * terminal `turn_stopped`/`turn_completed` yet. Used by session- * status derivation and the foldSemanticEvent branch that decides From f6cae4ba986fb76a1667e5c9490083c8cefdff98 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 02:37:48 -0700 Subject: [PATCH 4/6] fix(semantic): merge a replayed turn into its archived copy field by field (#1391 q93) Three size proxies for 'the replay covers the archive' failed review. The bootstrap path now merges: strings take the replay only when it extends the archived value, objects merge per key, id lists union. Tests go through the real ledger adapter, including both round-3 sequences and a mixed-progress case. Co-Authored-By: Claude Opus 5.5 --- .../semantic/archiveReplayedTurn.test.ts | 110 ++++++++++++------ .../src/session-runtime/semantic/helpers.ts | 83 +++++++++---- 2 files changed, 135 insertions(+), 58 deletions(-) diff --git a/src/renderer/src/session-runtime/semantic/archiveReplayedTurn.test.ts b/src/renderer/src/session-runtime/semantic/archiveReplayedTurn.test.ts index c400aac47..9e34dc126 100644 --- a/src/renderer/src/session-runtime/semantic/archiveReplayedTurn.test.ts +++ b/src/renderer/src/session-runtime/semantic/archiveReplayedTurn.test.ts @@ -1,15 +1,22 @@ import { expect, it } from 'vitest' +import { createLedgerInputAdapter } from '@renderer/rendering/adapter/collectLedgerInput' import { archiveReplayedTurn, semanticHistoryRow } from './helpers' -// #1391 review a (rounds 1 and 2): a replay copy only displaces the archived -// turn when it renders at least as much, measured as content (turn text plus -// block text, thinking, tool input and result), not counts. Always one row per -// turnId. -type Block = { text?: string; thinking?: string; inputJson?: string; resultContent?: string } +// #1391 (review a rounds 1-3, steering q93): at bootstrap-complete a replay +// copy of an archived turn is MERGED field-wise (longer string wins, objects +// merge key by key, id lists union), so nothing the archived row painted can +// vanish, and anything the replay added is kept. Asserted through the REAL +// ledger adapter: what matters is what the feed would paint. +// +// Limit (q93): no recorded bootstrap-overlap sequence exists in the fixture +// corpus, so the turn states are built deterministically from the event shapes +// the reviewer traced (foldEvent's turn_started/block_started with no deltas +// yet). No claim is made about how often they occur. +type Block = Record const turn = (text: string, blocks: Block[], endedAt: number | null = 2) => ({ turnId: 'T', source: 'rollout' as const, text, - blocks: Object.fromEntries(blocks.map((block, index) => [index, { blockIndex: index, kind: 'text', ...block }])), + blocks: Object.fromEntries(blocks.map((block, index) => [index, { blockIndex: index, kind: 'text', status: 'completed', finalized: true, ...block }])), blockOrder: blocks.map((_, index) => index), stopReason: null, usage: null, task: { todos: [], doneCount: 0, totalCount: 0, inProgressToolUseIds: [], activeToolNames: [] }, @@ -17,45 +24,82 @@ const turn = (text: string, blocks: Block[], endedAt: number | null = 2) => ({ lookups: { toolCallsById: {}, toolUseIdsInOrder: [], resolvedToolUseIds: [], erroredToolUseIds: [] }, }) as never -it('keeps the archived turn when the replay copy has less turn text', () => { - const history = [semanticHistoryRow(turn('answer', []))] - expect(archiveReplayedTurn(history, turn('', []))).toBe(history) +/** What the feed would paint for this history: candidate id -> painted text. */ +function painted(history: ReturnType, provider: 'claude' | 'codex' = 'claude'): Record { + const bundle = createLedgerInputAdapter()({ + provider, sessionId: 's', entries: [], semanticCurrent: null, semanticHistory: history as never, + ghosts: new Map(), streamPhase: 'idle', lastJsonlEntryAtMs: null, + }) + return Object.fromEntries(bundle.input.live.map(candidate => [candidate.id, candidate.textKey ?? ''])) +} + +// Round 1: a repeated turn_started reopens T with empty turn text (blockless). +it('keeps a blockless archived answer when the replay reopened it empty', () => { + const history = archiveReplayedTurn([semanticHistoryRow(turn('answer', []))], turn('', [], 3)) + expect(history.map(row => row.turnId)).toEqual(['T']) + expect(Object.values(painted(history))).toContain('answer') }) -it('keeps the archived turn when the replay copy has fewer or emptier blocks', () => { - const history = [semanticHistoryRow(turn('', [{ text: 'first' }, { text: 'second' }]))] - expect(archiveReplayedTurn(history, turn('', [{ text: 'first' }]))).toBe(history) +// Round 2: the replay re-emitted block_started for index 0, no text_delta yet. +it('keeps the archived block when the replay reopened the same block empty', () => { + const history = archiveReplayedTurn([semanticHistoryRow(turn('', [{ text: 'answer' }]))], turn('', [{ text: '', status: 'in_progress', finalized: false }], 3)) + expect(painted(history)['sem:T:0']).toBe('answer') }) -// Round 2: equal block COUNT, but the replay's block is still empty. -it('keeps the archived answer when the replay reopened the same block empty', () => { - const history = [semanticHistoryRow(turn('', [{ text: 'answer' }]))] - expect(archiveReplayedTurn(history, turn('', [{ text: '' }]))).toBe(history) +// Round 3 (a): a Codex reasoning block painted from reasoningSummary. +it('keeps a provider-specific drawable field (Codex reasoningSummary)', () => { + const archived = turn('', [{ kind: 'thinking', reasoningSummary: 'reasoned answer' }]) + const reopened = turn('', [{ kind: 'thinking', reasoningSummary: '' }], 3) + const history = archiveReplayedTurn([semanticHistoryRow(archived)], reopened) + expect((history[0]!.blocks[0] as Block).reasoningSummary).toBe('reasoned answer') }) -it('counts thinking as content', () => { - const history = [semanticHistoryRow(turn('', [{ thinking: 'reasoning' }]))] - expect(archiveReplayedTurn(history, turn('', [{ thinking: '' }]))).toBe(history) +// Round 3 (b): the ledger ignores turn text when blocks exist, so longer turn +// text must not stand in for the missing answer block. +it('keeps the answer block even when the replay carries longer (unpainted) turn text', () => { + const history = archiveReplayedTurn( + [semanticHistoryRow(turn('', [{ text: 'answer' }]))], + turn('answer with extra words', [{ text: '', status: 'in_progress', finalized: false }], 3), + ) + expect(painted(history)['sem:T:0']).toBe('answer') }) -it('counts tool input as content', () => { - const history = [semanticHistoryRow(turn('', [{ inputJson: '{"a":1}' }]))] - expect(archiveReplayedTurn(history, turn('', [{ inputJson: '' }]))).toBe(history) +// q93: mixed progress. The archive has block 0; the replay has block 0 still +// empty but ALSO a block the archive never saw. Both must paint. +it('keeps archived content and the replay\'s new block together (mixed progress)', () => { + const history = archiveReplayedTurn( + [semanticHistoryRow(turn('', [{ text: 'first part' }]))], + turn('', [{ text: '', status: 'in_progress', finalized: false }, { text: 'second part' }], 3), + ) + expect(painted(history)).toMatchObject({ 'sem:T:0': 'first part', 'sem:T:1': 'second part' }) + expect(history).toHaveLength(1) }) -it('counts a tool result as content', () => { - const history = [semanticHistoryRow(turn('', [{ resultContent: 'exit 0' }]))] - expect(archiveReplayedTurn(history, turn('', [{ resultContent: '' }]))).toBe(history) +it('takes the replay copy where it is longer, and archives a turn not in history yet', () => { + const history = archiveReplayedTurn([semanticHistoryRow(turn('', [{ text: 'answer' }]))], turn('', [{ text: 'answer, continued' }], 3)) + expect(painted(history)['sem:T:0']).toBe('answer, continued') + expect(archiveReplayedTurn([], turn('answer', [])).map(row => row.turnId)).toEqual(['T']) }) -it('lets an equal or richer replay copy replace the archived one, once', () => { - const history = [semanticHistoryRow(turn('', [{ text: 'answer' }]))] - const equal = archiveReplayedTurn(history, turn('', [{ text: 'ANSWER' }])) - expect(equal.map(row => [row.turnId, (row.blocks as Record)[0]?.text])).toEqual([['T', 'ANSWER']]) - const richer = archiveReplayedTurn(history, turn('', [{ text: 'answer, continued' }])) - expect(richer.map(row => [row.turnId, (row.blocks as Record)[0]?.text])).toEqual([['T', 'answer, continued']]) +// The string rule is prefix-extension, not "longer wins": a reopened block's +// status 'in_progress' is LONGER than 'completed', and taking it would drop +// the block's text ownership key (textKey is set only for completed text). +// The archived block is completed WITHOUT finalized, the Codex shape +// semantic.ts documents (#492), so status is the only terminal evidence. +it('does not reopen a completed block through its longer status string', () => { + const history = archiveReplayedTurn( + [semanticHistoryRow(turn('', [{ text: 'answer', finalized: undefined }]))], + turn('', [{ text: 'answer', status: 'in_progress', finalized: false }], 3), + ) + expect(painted(history)['sem:T:0']).toBe('answer') }) -it('archives a turn that is not in history yet', () => { - expect(archiveReplayedTurn([], turn('answer', [])).map(row => row.turnId)).toEqual(['T']) +// A replay that reopened only block 0 must not drop the archived block 1: +// blockOrder is unioned, not replaced. +it('keeps an archived block the replay has not re-emitted yet', () => { + const history = archiveReplayedTurn( + [semanticHistoryRow(turn('', [{ text: 'first part' }, { text: 'second part' }]))], + turn('', [{ text: 'first part' }], 3), + ) + expect(painted(history)).toMatchObject({ 'sem:T:0': 'first part', 'sem:T:1': 'second part' }) }) diff --git a/src/renderer/src/session-runtime/semantic/helpers.ts b/src/renderer/src/session-runtime/semantic/helpers.ts index d3be0a33c..fdf4b4f7d 100644 --- a/src/renderer/src/session-runtime/semantic/helpers.ts +++ b/src/renderer/src/session-runtime/semantic/helpers.ts @@ -139,43 +139,76 @@ export function appendSemanticHistory( } /** - * Archive a turn that REPLAY reopened and left open, keeping whichever copy - * of it renders more (#1391 review a). + * Archive a turn that REPLAY reopened and left open, WITHOUT ever erasing + * anything the archived copy of it could paint (#1391, steering q93). * - * WHY not plain appendSemanticHistory here: replay can reopen an archived turn - * T with a repeated `turn_started`, which starts it EMPTY (foldEvent). If - * replay then quiets before T's content re-arrives, replacing by turnId swapped - * the full archived T for the empty copy, and with no durable assistant entry - * yet the answer vanished from the feed. Here the reopened copy only replaces - * the archived one when it carries at least as much renderable content; - * otherwise the archived row stays, still exactly one per turnId. + * Source-of-truth rule: when the same turnId is already archived, the result + * is a FIELD-WISE MONOTONIC MERGE of the archived row and the replay copy: + * - two strings at the same path: the replay's only when it EXTENDS the + * archived one (starts with it and is longer), else the archived one. + * Streamed content only grows by appending, so this takes a replay that + * got further, while an enum-like string (`status: 'completed'` vs a + * reopened `'in_progress'`, which is LONGER) or a replay that diverged + * keeps the archived value. A first draft used "longer string wins" and + * would have flipped a completed block back to in_progress, which drops + * its text ownership key (semantic.ts keys it on finalized/completed) + * and double-renders it next to JSONL; + * - two plain objects (the turn, `blocks`, each block, `lookups`, + * `toolCallsById`, `usage`, ...): merged key by key, recursively, so a + * key only one copy has survives; + * - two arrays of primitives (`blockOrder`, the lookups' id lists): their + * union, archived order first; + * - anything else (numbers, booleans, arrays of objects such as todos): the + * archived value unless it is missing, then the replay's. + * INVARIANT: every string in the archived row survives at its path, unchanged + * or extended, and every archived block index and id stays. The ledger renders + * only from these fields (turn text when blockless, each block's text / + * thinking / reasoningSummary / reasoningText / arguments / input / result), + * so no field visible before can vanish, while content the replay ADDED + * (a block the archive lacked, longer text) is kept too. * - * WHY content size and not counts (#1391 review a, round 2): the reopened - * copy's BLOCKS can be empty too. Replay re-emits `block_started` for index 0 - * and quiets before `text_delta`, so an equal block count held an empty block - * where the archived turn held the answer. renderableSize measures what the - * feed can actually paint (turn text plus each block's text, thinking, tool - * input and tool result). + * WHY not a proxy: three proxies failed review (#1391 review a, rounds 1-3). + * Turn-text length missed empty blocks; block counts missed an empty block + * at the same index; one content total missed provider fields + * (reasoningSummary) and let turn text the ledger never paints outweigh a + * missing answer block. A merge needs no judgement of which copy is + * "richer": it keeps both. * - * Only the bootstrap-complete path uses this: on the live fold paths a newer - * copy of the same turn is the authoritative one and should win. + * Only the bootstrap-complete path uses this. On the live fold paths a newer + * copy of a turn is authoritative (appendSemanticHistory's plain replace). */ export function archiveReplayedTurn( history: SemanticRuntimeState['history'], turn: SemanticLiveTurn, ): SemanticRuntimeState['history'] { const archived = history.find(existing => existing.turnId === turn.turnId) - if (archived && renderableSize(turn) < renderableSize(archived)) return history - return appendSemanticHistory(history, turn) + const merged = archived ? (mergeMonotonic(archived, turn) as SemanticLiveTurn) : turn + return appendSemanticHistory(history, merged) } -function renderableSize(turn: SemanticLiveTurn): number { - let size = turn.text.length - for (const block of Object.values(turn.blocks)) { - size += (block.text?.length ?? 0) + (block.thinking?.length ?? 0) + - (block.inputJson?.length ?? 0) + (block.resultContent?.length ?? 0) +function mergeMonotonic(archived: unknown, replay: unknown): unknown { + if (archived === undefined || archived === null) return replay + if (replay === undefined || replay === null) return archived + if (typeof archived === 'string' && typeof replay === 'string') { + return replay.length > archived.length && replay.startsWith(archived) ? replay : archived } - return size + if (Array.isArray(archived) && Array.isArray(replay)) { + const primitive = (value: unknown) => value === null || typeof value !== 'object' + if (archived.every(primitive) && replay.every(primitive)) { + return [...archived, ...replay.filter(value => !archived.includes(value))] + } + return archived.length > 0 ? archived : replay + } + if (isPlainObject(archived) && isPlainObject(replay)) { + const out: Record = { ...archived } + for (const [key, value] of Object.entries(replay)) out[key] = mergeMonotonic(archived[key], value) + return out + } + return archived +} + +function isPlainObject(value: unknown): value is Record { + return value !== null && typeof value === 'object' && !Array.isArray(value) } /** True when the turn is still live — hasn't received its From 8aa9ab02be14a3c1f399b440544d09ca60b67524 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 02:55:12 -0700 Subject: [PATCH 5/6] fix(semantic): lifecycle fields take the more advanced value in the replay merge (#1391 review a round 4) Keeping archived lifecycle values blocked a replay that completed a block or tool the archive saw pending. status/messagePhase now rank, booleans OR, and the derived in-progress tool lists are recomputed after the merge. Co-Authored-By: Claude Opus 5.5 --- .../semantic/archiveReplayedTurn.test.ts | 55 +++++++++++++++++++ .../src/session-runtime/semantic/helpers.ts | 52 ++++++++++++++++-- 2 files changed, 101 insertions(+), 6 deletions(-) diff --git a/src/renderer/src/session-runtime/semantic/archiveReplayedTurn.test.ts b/src/renderer/src/session-runtime/semantic/archiveReplayedTurn.test.ts index 9e34dc126..7c2b1e9b8 100644 --- a/src/renderer/src/session-runtime/semantic/archiveReplayedTurn.test.ts +++ b/src/renderer/src/session-runtime/semantic/archiveReplayedTurn.test.ts @@ -1,6 +1,8 @@ import { expect, it } from 'vitest' import { createLedgerInputAdapter } from '@renderer/rendering/adapter/collectLedgerInput' +import { groupSemanticActivity } from '@renderer/features/feed/ui/semantic/renderUnits' +import type { SemanticLiveTurn } from '@renderer/session-runtime/state' import { archiveReplayedTurn, semanticHistoryRow } from './helpers' // #1391 (review a rounds 1-3, steering q93): at bootstrap-complete a replay @@ -103,3 +105,56 @@ it('keeps an archived block the replay has not re-emitted yet', () => { ) expect(painted(history)).toMatchObject({ 'sem:T:0': 'first part', 'sem:T:1': 'second part' }) }) + +// Round 4 (review a): lifecycle fields take the MORE advanced value from +// either copy. The archive can be cut while a block is still streaming and the +// replay can complete it; keeping the archived lifecycle left the merged block +// without its ownership key, so it double-rendered beside the JSONL answer. +it('takes a completion the replay reached (Codex status)', () => { + const history = archiveReplayedTurn( + [semanticHistoryRow(turn('', [{ text: 'answer', status: 'in_progress', finalized: false }], null))], + turn('', [{ text: 'answer', status: 'completed', finalized: false }], 3), + ) + expect(painted(history)['sem:T:0']).toBe('answer') +}) + +it('takes a completion the replay reached (Claude finalized, no status)', () => { + const history = archiveReplayedTurn( + [semanticHistoryRow(turn('', [{ text: 'answer', status: undefined, finalized: false }], null))], + turn('', [{ text: 'answer', status: undefined, finalized: true }], 3), + ) + expect(painted(history)['sem:T:0']).toBe('answer') +}) + +// A Read the archive saw pending and the replay resolved must not paint as +// running; nor may a reopened replay make a resolved one run again. +function readTurn(state: 'in_progress' | 'completed', endedAt: number | null) { + const base = turn('', [{ kind: 'tool_use', toolName: 'Read', toolUseId: 'r1', inputJson: '{"file_path":"/a"}', status: undefined, finalized: true }], endedAt) as ReturnType & Record + const t = base as unknown as SemanticLiveTurn + return { + ...t, + task: { ...t.task, inProgressToolUseIds: state === 'in_progress' ? ['r1'] : [], activeToolNames: state === 'in_progress' ? ['Read'] : [] }, + lookups: { + toolCallsById: { r1: { toolUseId: 'r1', blockIndex: 0, kind: 'tool_use', toolName: 'Read', status: state, inputJson: '{"file_path":"/a"}', resultContent: state === 'completed' ? 'contents' : null } }, + toolUseIdsInOrder: ['r1'], + resolvedToolUseIds: state === 'completed' ? ['r1'] : [], + erroredToolUseIds: [], + }, + } as SemanticLiveTurn +} +const running = (history: ReturnType) => { + const row = history[0] as unknown as SemanticLiveTurn + const blocks = row.blockOrder.map(index => row.blocks[index]!) + return groupSemanticActivity(blocks, row).some(unit => unit.type === 'collapsed_activity' && unit.isRunning) +} + +it.each([ + ['archived pending, replay resolved', 'in_progress', 'completed'], + ['archived resolved, replay reopened', 'completed', 'in_progress'], +] as const)('%s: the merged Read is not running', (_label, archivedState, replayState) => { + const history = archiveReplayedTurn([semanticHistoryRow(readTurn(archivedState, 2))], readTurn(replayState, 3)) + expect(running(history)).toBe(false) + const row = history[0] as unknown as SemanticLiveTurn + expect(row.task.inProgressToolUseIds).toEqual([]) + expect(row.task.activeToolNames).toEqual([]) +}) diff --git a/src/renderer/src/session-runtime/semantic/helpers.ts b/src/renderer/src/session-runtime/semantic/helpers.ts index fdf4b4f7d..b36331f6b 100644 --- a/src/renderer/src/session-runtime/semantic/helpers.ts +++ b/src/renderer/src/session-runtime/semantic/helpers.ts @@ -158,14 +158,30 @@ export function appendSemanticHistory( * key only one copy has survives; * - two arrays of primitives (`blockOrder`, the lookups' id lists): their * union, archived order first; - * - anything else (numbers, booleans, arrays of objects such as todos): the - * archived value unless it is missing, then the replay's. + * - LIFECYCLE fields take the more advanced value, whichever copy has it + * (#1391 review a, round 4): a `status` (block, tool lookup, image/shell + * call) ranks pending < in_progress < terminal; `messagePhase` ranks + * commentary < final_answer; booleans OR (`finalized`, `inputJsonValid`, + * `resultIsError` only ever become true as evidence arrives). The round-3 + * rule kept the ARCHIVED lifecycle value, which protected a completed + * archive from a reopened replay but also blocked the opposite and + * equally real case: an archive cut while a block or tool was still + * pending, completed by the replay. That merged row had no text ownership + * key (double-rendered beside JSONL) and painted a resolved tool as + * running; + * - `task.inProgressToolUseIds` is DERIVED, so it is recomputed after the + * merge (the union of both copies minus every id either copy resolved or + * errored), and `activeToolNames` keeps only names both copies agree are + * active. A plain union would resurrect a tool the replay resolved; + * - anything else (numbers, arrays of objects such as todos): the archived + * value unless it is missing, then the replay's. * INVARIANT: every string in the archived row survives at its path, unchanged * or extended, and every archived block index and id stays. The ledger renders * only from these fields (turn text when blockless, each block's text / * thinking / reasoningSummary / reasoningText / arguments / input / result), * so no field visible before can vanish, while content the replay ADDED - * (a block the archive lacked, longer text) is kept too. + * (a block the archive lacked, longer text) is kept too. No lifecycle field + * moves backwards, and none is held back when either copy has progressed. * * WHY not a proxy: three proxies failed review (#1391 review a, rounds 1-3). * Turn-text length missed empty blocks; block counts missed an empty block @@ -182,14 +198,30 @@ export function archiveReplayedTurn( turn: SemanticLiveTurn, ): SemanticRuntimeState['history'] { const archived = history.find(existing => existing.turnId === turn.turnId) - const merged = archived ? (mergeMonotonic(archived, turn) as SemanticLiveTurn) : turn + const merged = archived ? withDerivedTask(mergeMonotonic(archived, turn) as SemanticLiveTurn, archived, turn) : turn return appendSemanticHistory(history, merged) } -function mergeMonotonic(archived: unknown, replay: unknown): unknown { +// Unknown values rank as -1, which keeps the archived value (see below). +const STATUS_RANK: Record = { + pending: 0, queued: 0, + in_progress: 1, running: 1, streaming: 1, searching: 1, generating: 1, + completed: 2, failed: 2, error: 2, errored: 2, cancelled: 2, canceled: 2, incomplete: 2, interrupted: 2, +} +const PHASE_RANK: Record = { commentary: 0, final_answer: 1 } + +function mergeMonotonic(archived: unknown, replay: unknown, key?: string): unknown { if (archived === undefined || archived === null) return replay if (replay === undefined || replay === null) return archived + if (typeof archived === 'boolean' && typeof replay === 'boolean') return archived || replay if (typeof archived === 'string' && typeof replay === 'string') { + const rank = key === 'status' ? STATUS_RANK : key === 'messagePhase' ? PHASE_RANK : null + if (rank) { + // Replay wins only when it is strictly MORE advanced. Two different + // terminal values (completed vs error), or a value this table does not + // know, keep the archived one. + return (rank[replay] ?? -1) > (rank[archived] ?? -1) ? replay : archived + } return replay.length > archived.length && replay.startsWith(archived) ? replay : archived } if (Array.isArray(archived) && Array.isArray(replay)) { @@ -201,12 +233,20 @@ function mergeMonotonic(archived: unknown, replay: unknown): unknown { } if (isPlainObject(archived) && isPlainObject(replay)) { const out: Record = { ...archived } - for (const [key, value] of Object.entries(replay)) out[key] = mergeMonotonic(archived[key], value) + for (const [childKey, value] of Object.entries(replay)) out[childKey] = mergeMonotonic(archived[childKey], value, childKey) return out } return archived } +function withDerivedTask(merged: SemanticLiveTurn, archived: SemanticLiveTurn, replay: SemanticLiveTurn): SemanticLiveTurn { + const settled = new Set([...merged.lookups.resolvedToolUseIds, ...merged.lookups.erroredToolUseIds]) + const inProgress = [...new Set([...archived.task.inProgressToolUseIds, ...replay.task.inProgressToolUseIds])] + .filter(id => !settled.has(id)) + const activeToolNames = archived.task.activeToolNames.filter(name => replay.task.activeToolNames.includes(name)) + return { ...merged, task: { ...merged.task, inProgressToolUseIds: inProgress, activeToolNames } } +} + function isPlainObject(value: unknown): value is Record { return value !== null && typeof value === 'object' && !Array.isArray(value) } From 39c712b6d42550875cae4309a156df37fe1176b1 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 03:06:24 -0700 Subject: [PATCH 6/6] fix(semantic): take a tool result as one unit from the later copy (#1391 review a round 5, q95) resultContent/resultIsError/resultAt and the tool's lookup state now come together from the copy with the later resultAt (replay on a tie). Merged field by field, an archived error followed by a corrected result painted the success red. Co-Authored-By: Claude Opus 5.5 --- .../semantic/archiveReplayedTurn.test.ts | 54 ++++++++++++++++++ .../src/session-runtime/semantic/helpers.ts | 56 ++++++++++++++++++- 2 files changed, 107 insertions(+), 3 deletions(-) diff --git a/src/renderer/src/session-runtime/semantic/archiveReplayedTurn.test.ts b/src/renderer/src/session-runtime/semantic/archiveReplayedTurn.test.ts index 7c2b1e9b8..fb13b95c8 100644 --- a/src/renderer/src/session-runtime/semantic/archiveReplayedTurn.test.ts +++ b/src/renderer/src/session-runtime/semantic/archiveReplayedTurn.test.ts @@ -158,3 +158,57 @@ it.each([ expect(row.task.inProgressToolUseIds).toEqual([]) expect(row.task.activeToolNames).toEqual([]) }) + +// Round 5 (review a) / steering q95: a tool result is ONE unit taken from the +// copy with the later resultAt (the replay on a tie). Field by field, an +// archived error followed by a corrected result kept resultIsError (OR) and +// the `error` lookup status beside the corrected text: a success painted red. +function resultTurn(result: { content: string; isError: boolean; at: number }, endedAt: number) { + const t = turn('', [{ + kind: 'tool_use', toolName: 'Bash', toolUseId: 'r1', inputJson: '{}', status: undefined, finalized: true, + resultContent: result.content, resultIsError: result.isError, resultAt: result.at, + }], endedAt) as unknown as SemanticLiveTurn + return { + ...t, + lookups: { + toolCallsById: { r1: { toolUseId: 'r1', blockIndex: 0, kind: 'tool_use', toolName: 'Bash', status: result.isError ? 'error' : 'completed', inputJson: '{}', resultContent: result.content } }, + toolUseIdsInOrder: ['r1'], + resolvedToolUseIds: result.isError ? [] : ['r1'], + erroredToolUseIds: result.isError ? ['r1'] : [], + }, + } as SemanticLiveTurn +} +const resultOf = (history: ReturnType) => { + const row = history[0] as unknown as SemanticLiveTurn + const block = row.blocks[0]! + return { + content: block.resultContent, isError: block.resultIsError, at: block.resultAt, + status: row.lookups.toolCallsById.r1?.status, + errored: row.lookups.erroredToolUseIds.includes('r1'), + resolved: row.lookups.resolvedToolUseIds.includes('r1'), + } +} + +it('takes a corrected result the replay received later as one unit (review a round 5)', () => { + const history = archiveReplayedTurn( + [semanticHistoryRow(resultTurn({ content: 'error', isError: true, at: 2 }, 2))], + resultTurn({ content: 'error corrected', isError: false, at: 3 }, 3), + ) + expect(resultOf(history)).toEqual({ content: 'error corrected', isError: false, at: 3, status: 'completed', errored: false, resolved: true }) +}) + +it('keeps the archived result when it is the later one', () => { + const history = archiveReplayedTurn( + [semanticHistoryRow(resultTurn({ content: 'fixed', isError: false, at: 5 }, 5))], + resultTurn({ content: 'fixed, but failed earlier', isError: true, at: 3 }, 6), + ) + expect(resultOf(history)).toEqual({ content: 'fixed', isError: false, at: 5, status: 'completed', errored: false, resolved: true }) +}) + +it('takes the replay result on a tie', () => { + const history = archiveReplayedTurn( + [semanticHistoryRow(resultTurn({ content: 'ok', isError: false, at: 4 }, 4))], + resultTurn({ content: 'failed', isError: true, at: 4 }, 5), + ) + expect(resultOf(history)).toEqual({ content: 'failed', isError: true, at: 4, status: 'error', errored: true, resolved: false }) +}) diff --git a/src/renderer/src/session-runtime/semantic/helpers.ts b/src/renderer/src/session-runtime/semantic/helpers.ts index b36331f6b..81f05299c 100644 --- a/src/renderer/src/session-runtime/semantic/helpers.ts +++ b/src/renderer/src/session-runtime/semantic/helpers.ts @@ -161,14 +161,25 @@ export function appendSemanticHistory( * - LIFECYCLE fields take the more advanced value, whichever copy has it * (#1391 review a, round 4): a `status` (block, tool lookup, image/shell * call) ranks pending < in_progress < terminal; `messagePhase` ranks - * commentary < final_answer; booleans OR (`finalized`, `inputJsonValid`, - * `resultIsError` only ever become true as evidence arrives). The round-3 + * commentary < final_answer; booleans OR (`finalized`, `inputJsonValid` + * only ever become true as evidence arrives). The round-3 * rule kept the ARCHIVED lifecycle value, which protected a completed * archive from a reopened replay but also blocked the opposite and * equally real case: an archive cut while a block or tool was still * pending, completed by the replay. That merged row had no text ownership * key (double-rendered beside JSONL) and painted a resolved tool as * running; + * - a TOOL RESULT is one unit, not separate fields (#1391 review a round + * 5, steering q95): `resultContent`, `resultIsError`, `resultAt` and the + * tool's lookup state (`toolCallsById[id]` and its membership in + * `resolvedToolUseIds` / `erroredToolUseIds`) all come from the copy + * with the LATER `resultAt`, and from the replay on a tie. Merged field + * by field, an archived error followed by a corrected result kept + * `resultIsError` (OR) and the error lookup status (equal terminal rank) + * next to the corrected text, so a success painted red. The live reducer + * lets a later result clear the error (foldEvent.ts's tool_result + * paths), so no single field of a result is monotonic; only the tuple's + * time is; * - `task.inProgressToolUseIds` is DERIVED, so it is recomputed after the * merge (the union of both copies minus every id either copy resolved or * errored), and `activeToolNames` keeps only names both copies agree are @@ -198,7 +209,9 @@ export function archiveReplayedTurn( turn: SemanticLiveTurn, ): SemanticRuntimeState['history'] { const archived = history.find(existing => existing.turnId === turn.turnId) - const merged = archived ? withDerivedTask(mergeMonotonic(archived, turn) as SemanticLiveTurn, archived, turn) : turn + const merged = archived + ? withDerivedTask(withToolResultUnits(mergeMonotonic(archived, turn) as SemanticLiveTurn, archived, turn), archived, turn) + : turn return appendSemanticHistory(history, merged) } @@ -239,6 +252,43 @@ function mergeMonotonic(archived: unknown, replay: unknown, key?: string): unkno return archived } +function withToolResultUnits(merged: SemanticLiveTurn, archived: SemanticLiveTurn, replay: SemanticLiveTurn): SemanticLiveTurn { + const blocks = { ...merged.blocks } + const lookups = { + ...merged.lookups, + toolCallsById: { ...merged.lookups.toolCallsById }, + resolvedToolUseIds: [...merged.lookups.resolvedToolUseIds], + erroredToolUseIds: [...merged.lookups.erroredToolUseIds], + } + for (const key of Object.keys(blocks)) { + const index = Number(key) + const a = archived.blocks[index] + const r = replay.blocks[index] + if (!a || !r || (a.resultAt === undefined && r.resultAt === undefined)) continue + // Later result wins; the replay on a tie or when only it has a time. + const source = a.resultAt !== undefined && (r.resultAt === undefined || a.resultAt > r.resultAt) ? a : r + const sourceTurn = source === a ? archived : replay + blocks[index] = { + ...blocks[index]!, + resultContent: source.resultContent, + resultIsError: source.resultIsError, + resultAt: source.resultAt, + } + const id = blocks[index]!.toolUseId ?? blocks[index]!.callId + if (id === undefined) continue + const call = sourceTurn.lookups.toolCallsById[id] + if (call) lookups.toolCallsById[id] = call + const setMembership = (list: string[], member: boolean) => { + const at = list.indexOf(id) + if (member && at < 0) list.push(id) + if (!member && at >= 0) list.splice(at, 1) + } + setMembership(lookups.resolvedToolUseIds, sourceTurn.lookups.resolvedToolUseIds.includes(id)) + setMembership(lookups.erroredToolUseIds, sourceTurn.lookups.erroredToolUseIds.includes(id)) + } + return { ...merged, blocks, lookups } +} + function withDerivedTask(merged: SemanticLiveTurn, archived: SemanticLiveTurn, replay: SemanticLiveTurn): SemanticLiveTurn { const settled = new Set([...merged.lookups.resolvedToolUseIds, ...merged.lookups.erroredToolUseIds]) const inProgress = [...new Set([...archived.task.inProgressToolUseIds, ...replay.task.inProgressToolUseIds])]