From 1efcc3efd1270df39bb5c3f733dd32d4a27628f7 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Tue, 7 Jul 2026 23:01:36 +0200 Subject: [PATCH 1/2] =?UTF-8?q?feat(rendering):=20block-level=20un-collaps?= =?UTF-8?q?e=20=E2=80=94=20retire=20SemanticStreamingTurn,=20ledger=20is?= =?UTF-8?q?=20sole=20live-turn=20decider=20(#491)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last slice of the rendering rewrite. The ledger already decided live-turn visibility at BLOCK grain, but the view bridge collapsed every turn back into one item so SemanticStreamingTurn could render it — and that component then RE-DERIVED the same suppression (buildSemanticRenderUnits: text dedup, committed-ownership, collapsed-running). Two decision-makers for live turns, kept in sync by hand. This kills the duplicate. - View bridge (ledgerFeedItems.ts) no longer collapses: it emits ONE FeedRenderItem per ledger-approved semantic block — `semantic-block`, `semantic-collapsed-activity` (churn fold), or blockless `semantic-text`. Groups by turnId (not adjacency) so equal-timestamp interleaved turns still emit contiguously. Fetches each block's rich payload from turnsById (the candidate carries only turnId+blockIndex). - `groupSemanticActivity` (renderUnits.ts): pure presentation grouping extracted from buildSemanticRenderUnits — NO suppression (the ledger owns that now). - Feed renders the three block items via the dumb SemanticLiveBlockRow / SemanticCollapsedActivityRow / MarkerRow+StreamingProse. history/current debug + work-hint signals re-derived from the items' `owner` tag. - DELETED: SemanticStreamingTurn (StreamingTurn.tsx), and the now-dead deriveFeedCommittedProjection / committedClaudeMessageTurnIds / committedMessageIds / visibleDecisionForEntry it only fed. - RETAINED with non-render owners (documented in the manifest): buildSemanticRenderUnits (the #239 semanticTurnHasRenderableContent predicate + debug bundles), selectMergedEntries (Feed's ghost-folded entries source), ghost-semantic-owned (a ghost-vs-live-turn dedup, not a component artifact). RISK: shadow parity only ever verified TURN grain — this exposes the ledger's per-block decisions to the pixels for the first time. Guarded by a new block-grain bridge test (text + churn-fold + non-churn block + payload identity) and the corpus; the residual is VISUAL and needs an eyeball on a build before merge. Gates: tsc node+web clean; rendering 164, unit 430, renderer 14, integration 28. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/rendering/legacy-deletion-manifest.md | 31 +++- .../src/features/feed/model/renderModel.ts | 151 +++++---------- src/renderer/src/features/feed/ui/Feed.tsx | 85 ++++++--- .../feed/ui/semantic/StreamingTurn.tsx | 149 --------------- .../src/features/feed/ui/semantic/index.ts | 5 +- .../features/feed/ui/semantic/renderUnits.ts | 74 ++++++++ src/renderer/src/rendering/model/types.ts | 10 +- .../rendering/view/ledgerFeedItems.test.ts | 81 ++++++-- .../src/rendering/view/ledgerFeedItems.ts | 174 +++++++++++++----- 9 files changed, 401 insertions(+), 359 deletions(-) delete mode 100644 src/renderer/src/features/feed/ui/semantic/StreamingTurn.tsx diff --git a/docs/rendering/legacy-deletion-manifest.md b/docs/rendering/legacy-deletion-manifest.md index a57090050..87567ae33 100644 --- a/docs/rendering/legacy-deletion-manifest.md +++ b/docs/rendering/legacy-deletion-manifest.md @@ -36,16 +36,27 @@ blocked until the phone also fed the ledger. `shadowParity.test.ts` CI diff. `shadow/shadowDiff.ts` KEPT (pure diff util — bundleCorpus/recordingCorpus still assert ledger output through it). -**STILL ALIVE — the block-level un-collapse slice (next PR):** the view bridge -still COLLAPSES the ledger's block-level semantic rows back into turn-level -items so `SemanticStreamingTurn` can render them. Killing that component (and -with it ghost rule 3, `selectMergedEntries` as Feed's entries source, and -`committedClaudeMessageTurnIds`) requires the ledger to own live turns -block-by-block through the shared registry dispatch — the hardest slice, and -the one that needs live visual verification the turn-grain corpus can't give. -Until then the completeness grep below still returns `SemanticStreamingTurn` / -`selectMergedEntries` / `committedClaudeMessageTurnIds` hits BY DESIGN; -`deriveFeedRenderModel` / `buildSemanticRenderUnits` are already clean. +**DONE — the block-level un-collapse (#491):** the view bridge no longer +collapses; it emits ONE `FeedRenderItem` per ledger-approved semantic block +(`semantic-block` / `semantic-collapsed-activity` / blockless `semantic-text`), +drawn by the dumb `SemanticLiveBlockRow` / `SemanticCollapsedActivityRow`. The +ledger (observations/semantic.ts + model/ownership.ts) is now the SOLE decider +of live-turn visibility; the bridge applies only the presentation grouping +(`groupSemanticActivity`, a pure fold — no suppression). **`SemanticStreamingTurn` +is DELETED**, and with it Feed's `deriveFeedCommittedProjection` / +`committedClaudeMessageTurnIds`. + +Three of the old grep tokens SURVIVE, and after this slice they are NOT +SemanticStreamingTurn artifacts — each has a non-render owner: +- `buildSemanticRenderUnits` — retained for `semanticTurnHasRenderableContent`, + the NON-render #239 prompt-ownership predicate (streaming.ts) + debug bundles. + Rendering no longer calls it (that was the duplicate-decider #491 killed). +- `selectMergedEntries` — still Feed's ghost-folded `entries` SOURCE (tool + indices / scroll), used by 6 non-render call sites. The manifest's "ABSORB" + is a separate, entangled change. +- `ghost-semantic-owned` (ghost rule 3) — retained: it is a ghost-vs-live-turn + dedup (rejects a ghost whose turnId is the live turn), load-bearing regardless + of which component draws the turn. See its updated note in model/types.ts. ## Deleted at Stage 3 cutover (the big reviewed PR) diff --git a/src/renderer/src/features/feed/model/renderModel.ts b/src/renderer/src/features/feed/model/renderModel.ts index bbfb2ab33..632157fe1 100644 --- a/src/renderer/src/features/feed/model/renderModel.ts +++ b/src/renderer/src/features/feed/model/renderModel.ts @@ -1,11 +1,5 @@ import { getRendererProviderCapabilities } from '@providers/registry.renderer.capabilities' -import { - isCompactBoundaryEntry, - isCompactSummaryEntry, - isConversationEntry, - type Entry, -} from '@shared/types/transcript' -import { asRecord } from '@shared/lib/asRecord' +import { type Entry } from '@shared/types/transcript' import type { SemanticLiveTurn, @@ -16,31 +10,17 @@ import type { DebugVisibleRow, VisibleDecision, } from '@renderer/features/feed/types' -import { - debugKeyForEntry, - debugLabelForEntry, -} from '@renderer/features/feed/lib/helpers' -import { - buildCommittedAssistantText, - type CommittedAssistantText, -} from '@renderer/features/feed/ui/semantic/renderUnits' +import { debugLabelForEntry } from '@renderer/features/feed/lib/helpers' +import type { SemanticRenderUnit } from '@renderer/features/feed/ui/semantic/types' -// NOTE (Stage 3 cutover, 2026-07): `deriveFeedRenderModel` — the legacy in-Feed -// decision core that partitioned committed + semantic planes into an ordered -// item list — lived here and is deleted. The ownership ledger -// (src/renderer/src/rendering/) is now the sole decision-maker; both the -// desktop and the phone hand Feed the ledger's pre-ordered items. What remains -// is deliberately dumb: the FeedRenderItem contract, `feedRenderModelFromItems` -// (attaches debug side-products to the ledger's list), and -// `deriveFeedCommittedProjection` (still feeds SemanticStreamingTurn's committed -// dedup until the block-level un-collapse retires that component too). - -export type FeedCommittedProjection = { - visibleDecisions: VisibleDecision[] - visibleEntries: Entry[] - committedClaudeMessageTurnIds: ReadonlySet - committedAssistantText: CommittedAssistantText -} +// NOTE (Stage 3 cutover + #491 block-level un-collapse): the legacy in-Feed +// decision core lived here and is fully deleted — `deriveFeedRenderModel` (the +// plane partitioner) AND `deriveFeedCommittedProjection` (which only fed +// SemanticStreamingTurn's committed dedup, now that the ledger owns that and the +// component is gone). The ownership ledger (src/renderer/src/rendering/) is the +// sole decision-maker; desktop and phone hand Feed the ledger's pre-ordered +// items. What remains is deliberately dumb: the FeedRenderItem contract and +// `feedRenderModelFromItems` (attaches debug side-products to the ledger's list). export type FeedRenderModel = { items: FeedRenderItem[] @@ -64,16 +44,37 @@ export type FeedRenderItem = entryOrdinal: number order: FeedRenderItemOrder } + // #491 block-level un-collapse: the view bridge no longer hands Feed a whole + // turn (which SemanticStreamingTurn then re-suppressed block-by-block). The + // ledger already decided which blocks are visible, so the bridge emits ONE + // item per ledger-approved block (or a collapsed churn receipt / a blockless + // text row), drawn by a dumb row. `owner` is kept so Feed can still derive the + // history/current debug + work-hint signals it used to read off the turn item. + | { + type: 'semantic-block' + key: string + turnId: string + owner: 'semantic-current' | 'semantic-history' + block: SemanticLiveTurn['blocks'][number] + toolState: SemanticLiveTurn['lookups']['toolCallsById'][string] | null + order: FeedRenderItemOrder + } | { - type: 'semantic-history' + type: 'semantic-collapsed-activity' key: string - turn: SemanticLiveTurn + turnId: string + owner: 'semantic-current' | 'semantic-history' + unit: Extract order: FeedRenderItemOrder } | { - type: 'semantic-current' + // Blockless turn text (Codex/opencode deliver text ONLY on turn.text with + // an empty block map — semantic.ts mints a `sem::text` candidate). + type: 'semantic-text' key: string - turn: SemanticLiveTurn + turnId: string + owner: 'semantic-current' | 'semantic-history' + text: string order: FeedRenderItemOrder } | { @@ -91,67 +92,16 @@ export type FeedRenderItem = order: FeedRenderItemOrder } -function committedMessageIds(entries: Entry[]): Set { - const ids = new Set() - for (const entry of entries) { - if (entry.type !== 'assistant') continue - const record = asRecord(entry) - const message = asRecord(record?.message) - const messageId = message?.id - if (typeof messageId === 'string') ids.add(messageId) - } - return ids -} - -function visibleDecisionForEntry(entry: Entry, index: number): VisibleDecision { - if (isCompactBoundaryEntry(entry)) { - return { - key: debugKeyForEntry(entry, index), - entry, - visible: true, - reason: 'compact_boundary', - } - } - if (isCompactSummaryEntry(entry)) { - return { - key: debugKeyForEntry(entry, index), - entry, - visible: true, - reason: 'compact_summary', - } - } - if (!isConversationEntry(entry)) { - return { - key: debugKeyForEntry(entry, index), - entry, - visible: false, - reason: 'not_conversation', - } - } - if (asRecord(entry)?.isMeta === true) { - return { - key: debugKeyForEntry(entry, index), - entry, - visible: false, - reason: 'meta_filtered', - } - } - return { - key: debugKeyForEntry(entry, index), - entry, - visible: true, - reason: 'conversation', - } -} - function labelForItem(item: FeedRenderItem, provider: AgentProvider): string { switch (item.type) { case 'entry': return debugLabelForEntry(item.entry) - case 'semantic-history': - return `semantic history ${item.turn.turnId.slice(0, 12)} · ${item.turn.source ?? 'unknown'}` - case 'semantic-current': - return `semantic turn ${item.turn.turnId.slice(0, 12)} · ${item.turn.source ?? 'unknown'}` + case 'semantic-block': + return `semantic ${item.owner === 'semantic-current' ? 'current' : 'history'} ${item.turnId.slice(0, 12)}#${item.block.blockIndex} · ${item.block.kind}` + case 'semantic-collapsed-activity': + return `collapsed ${item.turnId.slice(0, 12)} · ${item.unit.count} tools${item.unit.isRunning ? ' (running)' : ''}` + case 'semantic-text': + return `semantic text ${item.owner === 'semantic-current' ? 'current' : 'history'} ${item.turnId.slice(0, 12)}` case 'work': return item.toolName && ( item.phase === 'tool-input' || @@ -169,8 +119,9 @@ function slotForItem(item: FeedRenderItem): DebugVisibleRow['slot'] { switch (item.type) { case 'entry': return 'entry' - case 'semantic-history': - case 'semantic-current': + case 'semantic-block': + case 'semantic-collapsed-activity': + case 'semantic-text': return 'semantic' case 'work': return 'work' @@ -210,15 +161,3 @@ export function feedRenderModelFromItems( return { items, visibleDecisions, debugRows: debugRowsForItems(items, provider) } } -export function deriveFeedCommittedProjection(entries: Entry[]): FeedCommittedProjection { - const visibleDecisions = entries.map(visibleDecisionForEntry) - const visibleEntries = visibleDecisions - .filter(item => item.visible) - .map(item => item.entry) - return { - visibleDecisions, - visibleEntries, - committedClaudeMessageTurnIds: committedMessageIds(entries), - committedAssistantText: buildCommittedAssistantText(entries), - } -} diff --git a/src/renderer/src/features/feed/ui/Feed.tsx b/src/renderer/src/features/feed/ui/Feed.tsx index 295d12922..681cba799 100644 --- a/src/renderer/src/features/feed/ui/Feed.tsx +++ b/src/renderer/src/features/feed/ui/Feed.tsx @@ -43,11 +43,15 @@ import { debugLabelForEntry, } from '@renderer/features/feed/lib/helpers' import { - deriveFeedCommittedProjection, feedRenderModelFromItems, type FeedRenderItem, } from '@renderer/features/feed/model/renderModel' -import { SemanticStreamingTurn } from '@renderer/features/feed/ui/semantic' +import { + SemanticLiveBlockRow, + SemanticCollapsedActivityRow, +} from '@renderer/features/feed/ui/semantic' +import { MarkerRow } from '@renderer/features/feed/ui/MarkerRow' +import { StreamingProse } from '@renderer/features/feed/ui/markdown' import { semanticTurnScrollSignal } from '@renderer/workspace/semantic/helpers' import { EAGER_TAIL, @@ -779,14 +783,11 @@ function FeedImpl({ return out }, [entries]) - const committedProjection = useMemo(() => { - // Semantic deltas can arrive many times per second while committed - // JSONL entries stay unchanged. Keep all committed-entry scans behind - // an entries-only memo so live semantic text/tool updates do not keep - // rebuilding visible decisions, Claude message-id ownership, and the - // committed assistant text index for old rows. - return deriveFeedCommittedProjection(entries) - }, [entries]) + // #491: `committedProjection` (deriveFeedCommittedProjection) is deleted — it + // existed ONLY to feed SemanticStreamingTurn's committedAssistantText for its + // in-component dedup. The ledger now owns that suppression, and + // SemanticStreamingTurn is gone, so Feed no longer derives any committed + // projection of its own. // Stage 3 cutover (2026-07): the ownership ledger is the SOLE decision core. // Feed no longer derives its own render model — both the desktop (TileLeaf) @@ -805,20 +806,41 @@ function FeedImpl({ const renderItems = renderModel.items const renderedRows = renderModel.debugRows const visibleEntryCount = renderItems.filter(item => item.type === 'entry').length - const renderedSemanticHistoryTurnIds = useMemo( - () => - renderItems - .filter(item => item.type === 'semantic-history') - .map(item => item.turn.turnId), - [renderItems], - ) - const renderedSemanticHistorySignature = renderedSemanticHistoryTurnIds.join('\u0000') - const renderedSemanticTurn = useMemo(() => { + // #491: semantic items are now block-level (semantic-block/-collapsed-activity/ + // -text), each tagged with its turn's owner. Derive the same debug signals the + // old turn-level items exposed — unique history turnIds, and "is the current + // turn on screen" — from the block items' owner+turnId. + const renderedSemanticHistoryTurnIds = useMemo(() => { + const seen = new Set() + const ids: string[] = [] for (const item of renderItems) { - if (item.type === 'semantic-current') return item.turn + if ( + (item.type === 'semantic-block' || + item.type === 'semantic-collapsed-activity' || + item.type === 'semantic-text') && + item.owner === 'semantic-history' && + !seen.has(item.turnId) + ) { + seen.add(item.turnId) + ids.push(item.turnId) + } } - return null + return ids }, [renderItems]) + const renderedSemanticHistorySignature = renderedSemanticHistoryTurnIds.join('\u0000') + // The current live turn IS rendered iff any block item carries owner + // 'semantic-current'. WorkIndicator's tool-hint reads the full turn, which + // Feed already has as the `semanticTurn` prop — return that when present. + const renderedSemanticTurn = useMemo(() => { + const currentOnScreen = renderItems.some( + item => + (item.type === 'semantic-block' || + item.type === 'semantic-collapsed-activity' || + item.type === 'semantic-text') && + item.owner === 'semantic-current', + ) + return currentOnScreen ? semanticTurn : null + }, [renderItems, semanticTurn]) const previousRenderedRowsRef = useRef(null) const previousRenderDebugSignatureRef = useRef(null) @@ -910,15 +932,26 @@ function FeedImpl({ ) } - case 'semantic-history': - case 'semantic-current': + case 'semantic-block': + // #491: the ledger already decided this block is visible; the row is a + // pure drawer (no suppression). SemanticLiveBlockRow renders the exact + // per-kind streaming affordances it always did. return ( - ) + case 'semantic-collapsed-activity': + return + case 'semantic-text': + // Blockless Codex/opencode turn text — the legacy no-blocks path. + return ( + + + + ) case 'work': return ( 0 - // Committed tool_use index is threaded down from Feed via context. - // buildSemanticRenderUnits uses it to skip live blocks whose - // `toolUseId` / `callId` is already committed — kills the bottom- - // of-feed dupe where the live turn keeps rendering tool rounds - // that the committed transcript already painted above. - const committedToolUseIndex = useContext(ToolUseIndexContext) - const committedToolResultIndex = useContext(ToolResultIndexContext) - const units = buildSemanticRenderUnits( - turn, - committedToolUseIndex, - committedToolResultIndex, - committedAssistantText, - ) - // Compaction-synthesis placeholder. - // - // When Claude Code triggers `/compact` (manual or auto), the proxy - // sees a normal `/v1/messages` POST whose last user message is the - // fixed compact prompt. The mitm addon sniffs that signature and - // the proxy adapter tags the resulting semantic turn with - // `isCompactionSynthesis: true` at turn_started. The model's - // response body is `……` XML - // — not user-visible text — and rendering it raw is the visible - // bug we're fixing. - // - // Returning a placeholder here is the simplest correct UI: the - // user-facing artefacts of compaction (the `compact_boundary` - // system entry and the `isCompactSummary: true` user entry) land - // via JSONL later and already have proper renderers - // (`CompactBoundaryRow` and `CompactSummaryRow` in feed/ui/rows/). - // The placeholder just covers the seconds-to-minutes streaming - // window where the synthesis turn would otherwise leak XML into - // the feed. Mirrors `CompactBoundaryRow` styling so the live and - // committed states share visual language. - // - // Placed AFTER the hooks (useContext/useMemo above) on purpose: - // moving it earlier would break Rules of Hooks because the hook - // count would differ between compaction and non-compaction renders - // of the same component instance. The wasted hook work is cheap — - // `committedAssistantText` memoizes on `committedEntries` which - // doesn't grow during the compaction window. - if (turn.isCompactionSynthesis) { - return ( - -
- Compacting conversation… -
- - ) - } - - if (!hasBlocks) { - // WHY collapse to null instead of rendering an empty MarkerRow: - // - // For Codex the rollout stream does not emit per-block events, so - // `turn.blocks` stays empty and this branch owns the entire live - // view. `turn.text` is cleared to '' by the Codex adapter the - // moment a `response_item` commits the current assistant message, - // because the committed `:message` entry in the feed then owns - // display of that text. During that transient — and between two - // messages in the same Codex turn more broadly — there is - // genuinely nothing the ghost should paint. An empty MarkerRow - // would still render a solitary ⏺ bullet under the committed row, - // which reads as a second speaker to the user. - // - // Returning null is correct because WorkIndicator below the feed - // carries the "agent is working" signal on its own. See - // docs/superpowers/plans/2026-04-18-thinking-indicator-rework.md. - if (!turn.text) return null - if ( - committedAssistantText.keys.has(`${turn.turnId}\u0000${turn.text}`) || - committedAssistantText.texts.has(turn.text) - ) { - return null - } - return ( - - - - ) - } - - return ( - <> - {units.map(unit => ( - unit.type === 'collapsed_activity' ? ( - - ) : ( - - ) - ))} - - ) -}) diff --git a/src/renderer/src/features/feed/ui/semantic/index.ts b/src/renderer/src/features/feed/ui/semantic/index.ts index b13397b5d..bc38bb9da 100644 --- a/src/renderer/src/features/feed/ui/semantic/index.ts +++ b/src/renderer/src/features/feed/ui/semantic/index.ts @@ -1,13 +1,12 @@ // Barrel for the feed's semantic-streaming components. // -// Only SemanticStreamingTurn is consumed by Feed.tsx directly — the -// rest of the exports are here because they're addressable from +// The block-level rows (SemanticLiveBlockRow / SemanticCollapsedActivityRow) +// are consumed by Feed.tsx directly since the #491 un-collapse; the rest are here because they're addressable from // WorkIndicator / debug surfaces and from each other through this // barrel. Keeping Feed's import surface to one module means the // whole semantic section can grow/shrink without Feed having to // track per-file paths. -export { SemanticStreamingTurn } from '@renderer/features/feed/ui/semantic/StreamingTurn' export { SemanticLiveBlockRow } from '@renderer/features/feed/ui/semantic/BlockRow' export { SemanticCollapsedActivityRow } from '@renderer/features/feed/ui/semantic/CollapsedActivityRow' export { SemanticTodoList } from '@renderer/features/feed/ui/semantic/TodoList' diff --git a/src/renderer/src/features/feed/ui/semantic/renderUnits.ts b/src/renderer/src/features/feed/ui/semantic/renderUnits.ts index 3a4f76971..3933bb552 100644 --- a/src/renderer/src/features/feed/ui/semantic/renderUnits.ts +++ b/src/renderer/src/features/feed/ui/semantic/renderUnits.ts @@ -372,3 +372,77 @@ export function buildSemanticRenderUnits( flush() return units } + +/** + * Pure collapsed-activity grouping over an ALREADY-APPROVED, block-index-ordered + * list of semantic blocks — the grouping HALF of buildSemanticRenderUnits with + * the suppression half removed (#491 block-level un-collapse). + * + * WHY this exists separately from buildSemanticRenderUnits: the ownership ledger + * (observations/semantic.ts + model/ownership.ts) is now the sole decider of + * which live blocks are visible — text dedup, committed-ownership skips, + * empty-block and collapsed-running suppression all happen there. The view + * bridge must NOT re-run any of that (re-deriving suppression in the row layer + * was the exact duplicate-decision-maker #491 kills). It only needs to fold + * consecutive low-signal churn tools (Read/Grep/Glob/Bash) into one + * "worked: N reads" receipt — a pure PRESENTATION concern. `buildSemanticRenderUnits` + * still carries the suppression pass because `semanticTurnHasRenderableContent` + * (a NON-render #239 prompt-ownership predicate) depends on it; rendering does not. + * + * INPUT CONTRACT: `blocks` are the ledger-approved blocks for ONE turn, already + * in block-index order. AskUserQuestion is never folded (belt-and-suspenders + * over classifySemanticToolActivity's own collapsible:false). + */ +export function groupSemanticActivity( + blocks: readonly SemanticLiveTurn['blocks'][number][], + turn: SemanticLiveTurn, +): SemanticRenderUnit[] { + const units: SemanticRenderUnit[] = [] + let pending: Extract | null = null + const flush = () => { + if (!pending) return + units.push(pending) + pending = null + } + for (const block of blocks) { + const toolState = block.toolUseId + ? turn.lookups.toolCallsById[block.toolUseId] ?? null + : null + const activity = classifySemanticToolActivity(block) + const isCollapsibleTool = + (block.kind === 'tool_use' || + block.kind === 'server_tool_use' || + block.kind === 'mcp_tool_use') && + block.toolName !== 'AskUserQuestion' && + activity.collapsible && + activity.category !== null + if (!isCollapsibleTool) { + flush() + units.push({ type: 'block', block, toolState }) + continue + } + if (!pending) { + pending = { + type: 'collapsed_activity', + count: 0, + searchCount: 0, + readCount: 0, + listCount: 0, + bashCount: 0, + latestHint: null, + blockIndices: [], + isRunning: false, + } + } + pending.count += 1 + pending.blockIndices.push(block.blockIndex) + pending.latestHint = activity.hint ?? pending.latestHint + if (toolState?.status === 'in_progress') pending.isRunning = true + if (activity.category === 'search') pending.searchCount += 1 + else if (activity.category === 'read') pending.readCount += 1 + else if (activity.category === 'list') pending.listCount += 1 + else if (activity.category === 'bash') pending.bashCount += 1 + } + flush() + return units +} diff --git a/src/renderer/src/rendering/model/types.ts b/src/renderer/src/rendering/model/types.ts index 686237e3b..c021578e1 100644 --- a/src/renderer/src/rendering/model/types.ts +++ b/src/renderer/src/rendering/model/types.ts @@ -145,10 +145,12 @@ export type RenderCandidate = { * least one fixture in testing/fixtures/rendering/ proving when it fires * (plan §7 rule 15). If you need a new reason, write the fixture first. * - * `ghost-semantic-owned` carries a death sentence: it exists only because - * SemanticStreamingTurn still owns the live turn in the legacy renderer. - * It is deleted at Stage 3 cutover TOGETHER with SemanticStreamingTurn - * (plan D6 — never one without the other). + * `ghost-semantic-owned` was originally slated to die with SemanticStreamingTurn + * (plan D6), but the #491 block-level un-collapse showed it is NOT a component + * artifact: it rejects a GHOST whose turnId is the currently-live turn so the + * ghost cannot double-render against that turn's live semantic-current blocks. + * That ghost-vs-live dedup is load-bearing regardless of which component draws + * the live turn, so it is RETAINED. (SemanticStreamingTurn itself is deleted.) */ export type RenderReason = | 'selected' diff --git a/src/renderer/src/rendering/view/ledgerFeedItems.test.ts b/src/renderer/src/rendering/view/ledgerFeedItems.test.ts index 3b280d426..2241ac8ca 100644 --- a/src/renderer/src/rendering/view/ledgerFeedItems.test.ts +++ b/src/renderer/src/rendering/view/ledgerFeedItems.test.ts @@ -20,7 +20,7 @@ import { // --------------------------------------------------------------------------- // View bridge: the ledger must drive Feed's EXISTING row components. These // tests run full runtime scenarios through adapter→ledger→bridge and assert -// the resulting item list structurally (type + order) plus payload identity. +// the resulting BLOCK-LEVEL item list (#491) structurally + payload identity. // // (Before the Stage 3 cutover these compared against the legacy // deriveFeedRenderModel as an oracle; that renderer is deleted, so the @@ -28,9 +28,9 @@ import { // scenario/fixture coverage lives in bundleCorpus/recordingCorpus.) // // The payload identity assertions matter as much as the order ones: the -// bridge must hand Feed the SAME Entry / SemanticLiveTurn object references -// the runtime holds, or every downstream memo (LazyEntry keyed on entry -// identity, SemanticStreamingTurn on turn identity) re-renders per tick — +// bridge must hand Feed the SAME Entry / block object references the runtime +// holds, or every downstream memo (LazyEntry on entry identity, the block row +// on block identity) re-renders per tick — // the exact churn class D11 exists to prevent. // --------------------------------------------------------------------------- @@ -88,9 +88,12 @@ const shape = (i: FeedRenderItem): string => { switch (i.type) { case 'entry': return `entry:${typeof i.entry.uuid === 'string' ? i.entry.uuid : '?'}` - case 'semantic-history': - case 'semantic-current': - return `${i.type}:${i.turn.turnId}` + case 'semantic-block': + return `semantic-block:${i.turnId}#${i.block.blockIndex}` + case 'semantic-collapsed-activity': + return `semantic-collapsed:${i.turnId}[${i.unit.blockIndices.join(',')}]` + case 'semantic-text': + return `semantic-text:${i.turnId}` case 'work': return 'work' case 'empty': @@ -98,8 +101,8 @@ const shape = (i: FeedRenderItem): string => { } } -describe('view bridge: ledger rows drive legacy FeedRenderItems', () => { - it('matches the legacy item list on the streaming scenario, with identical payload references', () => { +describe('view bridge: ledger rows drive block-level FeedRenderItems', () => { + it('emits block-level items on the streaming scenario, with identical payload references', () => { const rt = emptyRuntime() rt.entries = [userEntry('u1', T, 'prompt'), assistantEntry('a1', 'msg_1', T + 100, 'answer')] rt.lastJsonlEntryAt = T + 100 @@ -108,18 +111,70 @@ describe('view bridge: ledger rows drive legacy FeedRenderItems', () => { const { items, dropped } = bridgeItems(rt) expect(dropped).toEqual([]) + // #491: the live turn's single text block emits as ONE semantic-block item + // (not a collapsed turn item). owner tags it current. expect(items.map(shape)).toEqual([ 'entry:u1', 'entry:a1', - 'semantic-current:turn_live', + 'semantic-block:turn_live#0', 'work', ]) - // Payload identity: the bridge hands back the RUNTIME's objects. + // Payload identity: the bridge hands back the RUNTIME's own objects — the + // entry, and the exact block instance (keyed downstream, so a fresh copy + // per tick would bust every memo — the D11 churn class). const entryItem = items.find(i => i.type === 'entry') expect(entryItem?.type === 'entry' && entryItem.entry).toBe(rt.entries[0]) - const semItem = items.find(i => i.type === 'semantic-current') - expect(semItem?.type === 'semantic-current' && semItem.turn).toBe(rt.semantic.currentTurn) + const blockItem = items.find(i => i.type === 'semantic-block') + expect(blockItem?.type === 'semantic-block' && blockItem.owner).toBe('semantic-current') + expect(blockItem?.type === 'semantic-block' && blockItem.block).toBe( + rt.semantic.currentTurn!.blocks[0], + ) + }) + + it('un-collapses a multi-block turn: text + a non-churn tool paint per block, churn folds into a collapsed-activity receipt (#491)', () => { + const rt = emptyRuntime() + rt.entries = [userEntry('u1', T, 'prompt')] + rt.lastJsonlEntryAt = T + // A history turn: text block, then 3 churn tools (Read/Grep/Read → collapse), + // then a non-churn Edit tool (paints its own block). All resolved so nothing + // is hidden by the collapsed-running rule. + const done = { status: 'completed' as const } + rt.semantic.history = [ + { + turnId: 'turn_h', + text: '', + source: 'proxy', + blocks: { + 0: { blockIndex: 0, kind: 'text', text: 'thinking about it', finalized: true }, + 1: { blockIndex: 1, kind: 'tool_use', toolName: 'Read', toolUseId: 'r1', resultAt: T + 1 }, + 2: { blockIndex: 2, kind: 'tool_use', toolName: 'Grep', toolUseId: 'g1', resultAt: T + 2 }, + 3: { blockIndex: 3, kind: 'tool_use', toolName: 'Read', toolUseId: 'r2', resultAt: T + 3 }, + 4: { blockIndex: 4, kind: 'tool_use', toolName: 'Edit', toolUseId: 'e1', resultAt: T + 4 }, + }, + blockOrder: [0, 1, 2, 3, 4], + stopReason: null, + usage: null, + task: {} as SemanticLiveTurn['task'], + lookups: { + toolCallsById: { r1: done, g1: done, r2: done, e1: done }, + } as unknown as SemanticLiveTurn['lookups'], + startedAt: T + 10, + endedAt: T + 20, + } as SemanticLiveTurn, + ] + + const { items, dropped } = bridgeItems(rt) + expect(dropped).toEqual([]) + expect(items.map(shape)).toEqual([ + 'entry:u1', + 'semantic-block:turn_h#0', // the text block + 'semantic-collapsed:turn_h[1,2,3]', // Read/Grep/Read folded + 'semantic-block:turn_h#4', // Edit paints its own block + ]) + const collapsed = items.find(i => i.type === 'semantic-collapsed-activity') + expect(collapsed?.type === 'semantic-collapsed-activity' && collapsed.unit.readCount).toBe(2) + expect(collapsed?.type === 'semantic-collapsed-activity' && collapsed.unit.searchCount).toBe(1) }) it('renders ghost fallback rows through the entry item path (legacy fold parity)', () => { diff --git a/src/renderer/src/rendering/view/ledgerFeedItems.ts b/src/renderer/src/rendering/view/ledgerFeedItems.ts index c8ac5ff45..275132a62 100644 --- a/src/renderer/src/rendering/view/ledgerFeedItems.ts +++ b/src/renderer/src/rendering/view/ledgerFeedItems.ts @@ -10,30 +10,32 @@ import type { StreamPhase, } from '@renderer/workspace/workspaceState' import type { AgentProviderKind } from '@shared/types/providerKind' -import type { RenderLedger } from '@renderer/rendering/model/types' +import type { RenderLedger, RenderRow } from '@renderer/rendering/model/types' +import { groupSemanticActivity } from '@renderer/features/feed/ui/semantic/renderUnits' // --------------------------------------------------------------------------- // The view bridge: RenderLedger rows → FeedRenderItem[]. // -// WHY this exists instead of new row components: the Stage 3 cutover swaps -// the DECISION CORE (what is visible, who owns it, in what order) while -// Feed's row rendering — LazyEntry, the memo structure, autoscroll's -// tail-signature, TextProse/StreamingProse — survives untouched. Emitting -// the legacy FeedRenderItem shape means `Feed.tsx` behind -// AGENT_CODE_RENDER_PIPELINE=1 changes ONE call site. SemanticStreamingTurn -// keeps rendering semantic items until the final cutover slice deletes it -// together with ghost rule 3 (plan D6) — deleting it here would couple the -// decision-core swap to a full row-component rewrite, exactly the big-bang -// this staging exists to avoid. +// #491 BLOCK-LEVEL UN-COLLAPSE: this bridge used to COLLAPSE every semantic +// turn's block-rows back into ONE turn-level item (semantic-current / +// semantic-history) so the legacy `SemanticStreamingTurn` could render it — +// and that component then RE-DERIVED all the suppression the ledger already +// made (buildSemanticRenderUnits). That was two decision-makers for live +// turns. Now the bridge emits ONE item per ledger-APPROVED block: the ledger +// (observations/semantic.ts + model/ownership.ts) is the sole decider of which +// blocks are visible, and the bridge only applies the PRESENTATION grouping +// (fold churn tools into a "worked: N reads" receipt via groupSemanticActivity) +// before handing dumb rows their block. // -// ORDER CONTRACT: items are emitted in LEDGER order, and the synthesized -// `order` fields are engineered so legacy sortFeedRenderItems is a NO-OP -// on them: phase matches the ledger's own phase partition (the two systems -// share PHASE_RANK semantics), timeMs is null (all-equal at MAX_SAFE_INT in -// the comparator), source is a constant (all-equal rank), sequence is the -// emission index (the comparator's final tiebreak — strictly increasing). -// The ledger's D4 chronological merge is thereby authoritative even though -// the legacy sorter still runs over the result. +// GROUP-BY-TURN, NOT BY-ADJACENCY: order.ts tiebreaks equal-timestamp turns by +// per-turn block index, so two turns sharing a timestamp INTERLEAVE +// (A0,B0,A1,B1). We therefore pre-group semantic rows by turnId and emit each +// turn's blocks contiguously at its FIRST row's position — safe because a +// committed row can never sort strictly between a turn's equal-timestamp blocks. +// +// ORDER CONTRACT: items are emitted in LEDGER order; the synthesized `order` +// fields keep timeMs null + a constant source so the legacy sorter is a no-op — +// the ledger's D4 chronological merge stays authoritative. // --------------------------------------------------------------------------- export type LedgerFeedContext = { @@ -106,6 +108,87 @@ function visibleDecisionFor(entry: Entry): VisibleDecision { } } +// Build the FeedRenderItems for ONE semantic turn from its ledger-approved +// rows. The ledger already decided WHICH blocks are visible; here we only fetch +// each block's rich payload (turn.blocks — the ledger candidate carries only +// turnId+blockIndex, not the drawable input/result) and apply the presentation +// grouping. Emits: `semantic-text` for a blockless text-only turn, else one +// `semantic-block` / `semantic-collapsed-activity` per grouped unit. +function buildSemanticTurnItems( + turnId: string, + rows: readonly RenderRow[], + turn: SemanticLiveTurn, + baseSeq: number, +): { items: FeedRenderItem[]; dropped: string[] } { + const out: FeedRenderItem[] = [] + const dropped: string[] = [] + const owner: 'semantic-current' | 'semantic-history' = + rows[0]?.candidate.owner === 'semantic-current' ? 'semantic-current' : 'semantic-history' + const orderFor = (): FeedRenderItemOrder => orderAt(baseSeq + out.length, 'content') + + // Blockless turn (Codex/opencode deliver text ONLY on turn.text with an empty + // block map): semantic.ts mints a single `sem::text` candidate with no + // blockIndex. Render turn.text directly — the legacy no-blocks path. + const textCandidate = rows.find(r => r.candidate.blockIndex === undefined) + if (textCandidate) { + if (typeof turn.text === 'string' && turn.text.length > 0) { + out.push({ + type: 'semantic-text', + key: `semantic-text:${turnId}`, + turnId, + owner, + text: turn.text, + order: orderFor(), + }) + } else { + dropped.push(textCandidate.candidate.id) + } + return { items: out, dropped } + } + + // Real blocks: map each approved blockIndex to its rich block (in block order), + // then apply presentation grouping. A missing block is a lookup gap — surface + // it in `dropped`, never silently omit (#239 class). + const byIndex = new Map() + for (const b of Object.values(turn.blocks)) byIndex.set(b.blockIndex, b) + const approved: SemanticLiveTurn['blocks'][number][] = [] + for (const r of [...rows].sort( + (a, b) => (a.candidate.blockIndex ?? 0) - (b.candidate.blockIndex ?? 0), + )) { + const bi = r.candidate.blockIndex + if (bi === undefined) continue + const block = byIndex.get(bi) + if (!block) { + dropped.push(r.candidate.id) + continue + } + approved.push(block) + } + for (const unit of groupSemanticActivity(approved, turn)) { + if (unit.type === 'collapsed_activity') { + out.push({ + type: 'semantic-collapsed-activity', + key: `semantic-collapsed:${turnId}:${unit.blockIndices.join(',')}`, + turnId, + owner, + unit, + order: orderFor(), + }) + } else { + out.push({ + type: 'semantic-block', + key: `semantic-block:${turnId}:${unit.block.blockIndex}`, + turnId, + owner, + block: unit.block, + toolState: unit.toolState, + order: orderFor(), + }) + } + } + return { items: out, dropped } +} + export function ledgerToFeedItems( ledger: RenderLedger, ctx: LedgerFeedContext, @@ -113,19 +196,21 @@ export function ledgerToFeedItems( const items: FeedRenderItem[] = [] const dropped: string[] = [] let entryOrdinal = 0 - // Semantic rows arrive block-level from the ledger but the legacy row - // component (SemanticStreamingTurn) renders whole turns — collapse a turn's - // blocks into ONE item. - // - // #442 finding-C6: this used to be a `lastSemanticTurnId` guard that only - // caught CONSECUTIVE same-turn rows. But a semantic candidate's `sequence` - // is just its per-turn block index, so two history turns sharing the same - // `timestampMs` order as A0,B0,A1,B1 — interleaved, not consecutive — and the - // adjacency guard then emitted turn A (and B) twice. One row per turnId is - // ALWAYS correct (the row renders the whole turn), so re-emission is always a - // bug: track every emitted turnId in a set, not just the previous one. No - // resets on entry/process rows are needed — a turn is emitted at most once - // for the whole feed. + + // Pre-group every semantic row by turnId so a turn's blocks emit CONTIGUOUSLY + // even when order.ts interleaved two equal-timestamp turns (A0,B0,A1,B1). Each + // turn is then emitted once, at the position of its FIRST row in ledger order. + // (The old collapse used a per-turn emitted-set on a single item; block-level + // needs the full row list per turn, hence the map.) + const semanticRowsByTurn = new Map() + for (const row of ledger.rows) { + if (row.candidate.sourcePlane !== 'semantic') continue + const t = row.candidate.turnId + if (!t) continue + const arr = semanticRowsByTurn.get(t) + if (arr) arr.push(row) + else semanticRowsByTurn.set(t, [row]) + } const emittedTurnIds = new Set() for (const row of ledger.rows) { @@ -157,27 +242,20 @@ export function ledgerToFeedItems( break } if (emittedTurnIds.has(turnId)) break + emittedTurnIds.add(turnId) const turn = ctx.turnsById.get(turnId) if (!turn) { dropped.push(c.id) break } - emittedTurnIds.add(turnId) - if (c.owner === 'semantic-current') { - items.push({ - type: 'semantic-current', - key: `semantic:${turnId}`, - turn, - order: orderAt(items.length, 'content'), - }) - } else { - items.push({ - type: 'semantic-history', - key: `semantic-history:${turnId}`, - turn, - order: orderAt(items.length, 'content'), - }) - } + const built = buildSemanticTurnItems( + turnId, + semanticRowsByTurn.get(turnId) ?? [row], + turn, + items.length, + ) + for (const it of built.items) items.push(it) + for (const d of built.dropped) dropped.push(d) break } case 'process': { From 65845e057904f57a52927fde5bd3f8a0065633ec Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Tue, 7 Jul 2026 23:24:09 +0200 Subject: [PATCH 2/2] =?UTF-8?q?fix(rendering):=20address=204-agent=20revie?= =?UTF-8?q?w=20of=20#492=20=E2=80=94=20block-grain=20suppression=20parity?= =?UTF-8?q?=20+=20bridge=20accounting?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A 2-Claude + 2-Codex orchestration review of the un-collapse found 4 real issues (and validated the grouping/interleave/block-fetch structure as correct). All fixed + locked with block-grain tests: - semantic.ts finding 1 (Claude, CONFIRMED): a write_stdin tool block always has text:'' — the chars ride parsedInput/argumentsJson/inputJson. The collector checked b.text and so suppressed EVERY write_stdin, hiding real Codex stdin from the live feed. Now reads the chars via writeStdinChars (mirrors the retired renderUnits helper). Latent ledger bug the un-collapse exposed. - semantic.ts finding 2 (Claude): text ownership keyed on finalized===true only, but a Codex message can be status:'completed' without finalized (the isTerminalProxyBlock equivalence). A completed-but-unfinalized reply escaped text dedup and could double-render under the resp_* vs rollout id split. Now keys on `finalized || status==='completed'`. - ledgerFeedItems.ts finding (Codex): a RUNNING collapsed_activity was emitted as an item, but CollapsedActivityRow null-paints while running — a null-painting row the ledger counted as content (false-ownership). The bridge now skips it (mirrors semanticRenderUnitPaintsDom). - ledgerFeedItems.ts finding (both Codex): a missing multi-block turn only pushed its first candidate to dropped[] (turnId marked emitted before the lookup), so the rest were silently skipped — the #239 accounting gap. Now pushes all the turn's candidate ids. Codex #1's "grouping reorders ledger rows" was REFUTED by Claude: order.ts breaks equal-timestamp ties on SOURCE_RANK before per-turn sequence, so a committed row can never fall between a turn's blocks — group-by-turn is safe. New tests: write_stdin chars (parsedInput + argumentsJson), status:completed text key, running-churn emits no collapsed item. Gates: tsc clean; rendering 166, unit 432, renderer 14, integration 28. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../rendering/observations/semantic.test.ts | 33 ++++++++++-- .../src/rendering/observations/semantic.ts | 54 +++++++++++++++++-- .../rendering/view/ledgerFeedItems.test.ts | 37 +++++++++++++ .../src/rendering/view/ledgerFeedItems.ts | 23 +++++--- 4 files changed, 131 insertions(+), 16 deletions(-) diff --git a/src/renderer/src/rendering/observations/semantic.test.ts b/src/renderer/src/rendering/observations/semantic.test.ts index 5185bdb06..4b84a20c4 100644 --- a/src/renderer/src/rendering/observations/semantic.test.ts +++ b/src/renderer/src/rendering/observations/semantic.test.ts @@ -72,17 +72,23 @@ describe('semantic collector', () => { expect(decisions.find(d => d.candidateId === 'sem:turn-1:0')?.reason).toBe('empty-thinking') }) - it('empty write_stdin is Codex poll noise; non-empty renders (dump invariants 12/13)', () => { + it('empty write_stdin is Codex poll noise; non-empty renders (dump invariants 12/13, #492 finding 1)', () => { + // A write_stdin TOOL block ALWAYS has text: '' — the chars payload lives in + // parsedInput/argumentsJson/inputJson as {chars:"…"}. The old collector + // checked b.text and so suppressed EVERY write_stdin (hiding real stdin); + // these fixtures use the true shape so the non-empty one must survive. const { candidates } = collectSemanticCandidates( turn({ blocks: [ - { blockIndex: 0, kind: 'custom_tool_call', toolName: 'write_stdin', text: '' }, - { blockIndex: 1, kind: 'custom_tool_call', toolName: 'write_stdin', text: 'y\n' }, + { blockIndex: 0, kind: 'custom_tool_call', toolName: 'write_stdin', text: '', parsedInput: { chars: '' } }, + { blockIndex: 1, kind: 'custom_tool_call', toolName: 'write_stdin', text: '', parsedInput: { chars: 'y\n' } }, + // chars via raw argumentsJson (parsedInput absent) must also survive. + { blockIndex: 2, kind: 'custom_tool_call', toolName: 'write_stdin', text: '', argumentsJson: '{"chars":"n\\n"}' }, ] }), [], 'codex', 's1', ) - expect(candidates.map(c => c.blockIndex)).toEqual([1]) + expect(candidates.map(c => c.blockIndex)).toEqual([1, 2]) }) it('text keys only on FINALIZED text — streaming text must not be suppressible mid-growth', () => { @@ -99,6 +105,25 @@ describe('semantic collector', () => { expect(candidates.find(c => c.blockIndex === 1)?.textKey).toBe('done text') }) + it('text keys also on status:completed text (not just finalized) — #492 finding 2 resp_* vs rollout dedup', () => { + // A Codex message block can reach status:'completed' WITHOUT finalized:true + // (one block_started, no block_completed). Such text IS terminal and must + // get an ownership key, or its committed rollout twin double-renders (Codex + // has no whole-turn message-id suppression). Mirrors the retired + // renderUnits equivalence `finalized || status==='completed'`. + const { candidates } = collectSemanticCandidates( + turn({ blocks: [ + { blockIndex: 0, kind: 'text', text: 'still growing', finalized: false, status: 'in_progress' }, + { blockIndex: 1, kind: 'text', text: 'terminal reply', finalized: false, status: 'completed' }, + ] }), + [], + 'codex', + 's1', + ) + expect(candidates.find(c => c.blockIndex === 0)?.textKey).toBeUndefined() + expect(candidates.find(c => c.blockIndex === 1)?.textKey).toBe('terminal reply') + }) + it('tool identity keys (toolUseId/callId/itemId) ride the candidate for ownership matching', () => { const { candidates } = collectSemanticCandidates( turn({ blocks: [ diff --git a/src/renderer/src/rendering/observations/semantic.ts b/src/renderer/src/rendering/observations/semantic.ts index 167516344..5979604a7 100644 --- a/src/renderer/src/rendering/observations/semantic.ts +++ b/src/renderer/src/rendering/observations/semantic.ts @@ -1,4 +1,5 @@ import type { AgentProviderKind } from '@shared/types/providerKind' +import { asRecord, parseJsonRecord } from '@shared/lib/asRecord' import { normalizeTextKey } from '@renderer/rendering/observations/committed' import type { OwnershipDecision, @@ -57,6 +58,21 @@ export type SemanticBlockLike = { * when no result was paired into the block — the 2026-07-07 soak * bundle showed completed Agent tools with empty block results. */ lookupStatus?: string + /** Block lifecycle status from the semantic reducer (foldEvent). A + * text/message block can reach status:'completed' WITHOUT finalized:true + * (single block_started event, no block_completed) — the retired render + * layer's committed-text dedup used `finalized || status==='completed'` + * (the isTerminalProxyBlock equivalence), so the ledger must too or a + * completed-but-unfinalized Codex reply escapes text ownership and + * double-renders (the resp_* vs rollout id split). #492 review finding 2. */ + status?: string + /** Codex tool-call input payloads. For a write_stdin tool block b.text is + * ALWAYS '' — the actual chars live here as {chars:"…"}. These already + * flow through the adapter's {...b} spread; typing them lets the collector + * read the real payload instead of the wrong field (#492 review finding 1). */ + parsedInput?: unknown + argumentsJson?: string + inputJson?: string } export type SemanticTurnLike = { @@ -120,6 +136,18 @@ function blockContentKind(b: SemanticBlockLike): RenderCandidate['contentKind'] return 'assistant-text' } +/** The non-empty `chars` payload of a Codex write_stdin tool block, or ''. + * Mirrors the retired renderUnits.writeStdinChars: b.text is always '' for a + * tool block, so the real chars must be read from parsedInput / argumentsJson / + * inputJson (#492 review finding 1). */ +function writeStdinChars(b: SemanticBlockLike): string { + const parsed = asRecord(b.parsedInput) + if (typeof parsed?.chars === 'string') return parsed.chars + const raw = b.argumentsJson ?? b.inputJson ?? '' + const rawParsed = parseJsonRecord(raw) + return typeof rawParsed?.chars === 'string' ? rawParsed.chars : '' +} + function collectTurn( turn: SemanticTurnLike, owner: 'semantic-current' | 'semantic-history', @@ -177,17 +205,33 @@ function collectTurn( } // Codex uses empty write_stdin as poll/continuation noise (dump // invariant 12/13): empty renders nothing, non-empty renders. - if (b.toolName === 'write_stdin' && !(b.text && b.text.length > 0)) { + // + // #492 review finding 1: a write_stdin TOOL block always has b.text === '' + // (the chars payload rides parsedInput/argumentsJson/inputJson as + // {chars:"…"}), so the old `!(b.text)` check suppressed EVERY write_stdin — + // hiding real interactive stdin from the live feed. Read the actual chars, + // mirroring the retired renderUnits.isInvisibleWriteStdinBlock; suppress + // only when they are genuinely empty. + if (b.toolName === 'write_stdin' && writeStdinChars(b).length === 0) { out.decisions.push({ candidateId: id, selected: false, reason: 'empty-write-stdin', evidence: [] }) return } - // Text ownership keys ONLY on finalized text (dump: suppression applies - // to "finalized/completed semantic text only"). A still-streaming block - // that currently equals committed text may legitimately keep growing — + // Text ownership keys on FINALIZED/COMPLETED text (dump: suppression + // applies to "finalized/completed semantic text only"). A still-streaming + // block that currently equals committed text may legitimately keep growing — // suppressing it mid-stream blanked live output in production. + // + // #492 review finding 2: a Codex message block can reach status:'completed' + // WITHOUT finalized:true (single block_started, no block_completed). The + // retired renderUnits dedup keyed on `finalized || status==='completed'` + // (the isTerminalProxyBlock equivalence); keying on finalized alone lets a + // completed-but-unfinalized reply escape text ownership and double-render + // under the resp_*/rollout id split (wholeTurnByMessageId is false for Codex, + // so exact/normalized TEXT is the only defense). Mirror the old equivalence. + const textTerminal = b.finalized === true || b.status === 'completed' const finalizedText = - kind === 'assistant-text' && b.finalized === true && b.text ? b.text : undefined + kind === 'assistant-text' && textTerminal && b.text ? b.text : undefined out.candidates.push({ id, diff --git a/src/renderer/src/rendering/view/ledgerFeedItems.test.ts b/src/renderer/src/rendering/view/ledgerFeedItems.test.ts index 2241ac8ca..023ed19be 100644 --- a/src/renderer/src/rendering/view/ledgerFeedItems.test.ts +++ b/src/renderer/src/rendering/view/ledgerFeedItems.test.ts @@ -177,6 +177,43 @@ describe('view bridge: ledger rows drive block-level FeedRenderItems', () => { expect(collapsed?.type === 'semantic-collapsed-activity' && collapsed.unit.searchCount).toBe(1) }) + it('a RUNNING churn fan-out emits no collapsed-activity item — it would null-paint (#492 review finding)', () => { + const rt = emptyRuntime() + rt.entries = [userEntry('u1', T, 'go')] + rt.lastJsonlEntryAt = T + // Current turn with two in-progress churn tools (no results). The ledger + // SELECTS current-turn churn (collapsed-running is history-only), the bridge + // groups them into a RUNNING collapsed_activity, which CollapsedActivityRow + // paints as null — so the bridge must NOT emit an item for it (else a + // null-painting row sits in the list while the ledger counts it as content). + rt.semantic.currentTurn = { + turnId: 'turn_run', + text: '', + source: 'proxy', + blocks: { + 0: { blockIndex: 0, kind: 'tool_use', toolName: 'Read', toolUseId: 'r1' }, + 1: { blockIndex: 1, kind: 'tool_use', toolName: 'Grep', toolUseId: 'g1' }, + }, + blockOrder: [0, 1], + stopReason: null, + usage: null, + task: {} as SemanticLiveTurn['task'], + lookups: { + toolCallsById: { r1: { status: 'in_progress' }, g1: { status: 'in_progress' } }, + } as unknown as SemanticLiveTurn['lookups'], + startedAt: T + 10, + endedAt: null, + } as SemanticLiveTurn + rt.streamPhase = 'responding' + + const { items, dropped } = bridgeItems(rt) + expect(dropped).toEqual([]) + // No semantic-collapsed-activity item (running → null paint, skipped); the + // WorkIndicator ('work') carries the busy signal instead. + expect(items.some(i => i.type === 'semantic-collapsed-activity')).toBe(false) + expect(items.map(shape)).toEqual(['entry:u1', 'work']) + }) + it('renders ghost fallback rows through the entry item path (legacy fold parity)', () => { const rt = emptyRuntime() rt.entries = [userEntry('u1', T, 'prompt'), assistantEntry('a1', 'msg_1', T + 100, 'done')] diff --git a/src/renderer/src/rendering/view/ledgerFeedItems.ts b/src/renderer/src/rendering/view/ledgerFeedItems.ts index 275132a62..d1c3134e4 100644 --- a/src/renderer/src/rendering/view/ledgerFeedItems.ts +++ b/src/renderer/src/rendering/view/ledgerFeedItems.ts @@ -166,6 +166,14 @@ function buildSemanticTurnItems( } for (const unit of groupSemanticActivity(approved, turn)) { if (unit.type === 'collapsed_activity') { + // A RUNNING collapsed run paints nothing — SemanticCollapsedActivityRow + // returns null while running (WorkIndicator owns the "busy" surface). + // Emitting an item for it would put a null-painting row in the list while + // the ledger counts its blocks as content — the false-ownership class the + // pipeline exists to kill (#492 review finding). Skip it; only the + // finished "worked: N reads" receipt emits. Mirrors the retired + // semanticRenderUnitPaintsDom. + if (unit.isRunning) continue out.push({ type: 'semantic-collapsed-activity', key: `semantic-collapsed:${turnId}:${unit.blockIndices.join(',')}`, @@ -243,17 +251,18 @@ export function ledgerToFeedItems( } if (emittedTurnIds.has(turnId)) break emittedTurnIds.add(turnId) + const turnRows = semanticRowsByTurn.get(turnId) ?? [row] const turn = ctx.turnsById.get(turnId) if (!turn) { - dropped.push(c.id) + // #492 review finding: the turn is marked emitted above, so the OTHER + // rows of this turn will be skipped by the emittedTurnIds guard — we + // must account for ALL of them in dropped here, not just this row's + // candidate, or a missing multi-block turn silently under-reports + // selected-but-invisible rows (the #239 class). + for (const r of turnRows) dropped.push(r.candidate.id) break } - const built = buildSemanticTurnItems( - turnId, - semanticRowsByTurn.get(turnId) ?? [row], - turn, - items.length, - ) + const built = buildSemanticTurnItems(turnId, turnRows, turn, items.length) for (const it of built.items) items.push(it) for (const d of built.dropped) dropped.push(d) break