Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions docs/plans/2026-09-27-codex-ghost-item-identity.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Codex text ghosts are never superseded (#1231)

## Evidence (verified 2026-09-27)
- **The mismatch.** Codex ghosts are keyed by the live semantic turn id, which is the proxy response id (`resp_…`, `ghostsFromSemanticTurn` → `ghostUuid(turn.turnId, blockIndex)`).
- `reconcileUpstream` matches Codex entries on `codexTurnId`, but the mapper stamps that from `turn_context.turn_id`, a rollout UUID (`providers/codex/renderer/transcript/mapper.ts`, `stampCodexTurnId`). It never equals a `resp_…` id, so that branch never matches.
- Text blocks have no tool id for the fallback, so every Codex text ghost orphans after 30 s. It stays hidden only by render rules 3/4 in `selectMergedEntries`.
- **The two ids that do agree.** Every rollout `response_item` carries the provider item id (`msg_…`, `rs_…`, `ctc_…`, `fc_…`), and the live block already has it: `CodexResponsesAdapter` emits `itemId` on `block_started`, and `foldEvent` keeps it on `SemanticLiveBlock.itemId`.
- Checked on 6 real Agent Code 0.157.1 sessions (proxy dumps against their rollouts, 2026-09-27): every `msg_…` id seen on the proxy stream, 45 of 45, appears verbatim in the rollout.
- **The other route and why not.** `token_usage_record` rows carry `{turn_id, response_id}` after each response's items, which would allow a retroactive match by response. The item id is exact per block and needs no lookahead, so it is the route taken (issue comment, route 2).

## Change
- `mapCodexRolloutToFeedEntries` stamps `codexItemId` (the rollout item's `payload.id`) on every entry it maps from a `response_item`. This is an Agent Code-local field, like `codexTurnId`.
- `ghostContextForBlock` records the block's `itemId` in the ghost's `context`, atp's free-form slot. No package change is needed.
- `reconcileUpstream` supersedes a ghost when `context.itemId === entry.codexItemId`.
- The `codexTurnId === ghost.turnId` branch compares a rollout UUID with a response id, so it can never match. It is removed, and the stale comments that claim the mapper stamps the response id are corrected.

## Tests
A recorded pair from a real 0.157.1 session: the response's SSE frames for one assistant message (`response.created` id, `output_item.added`/`delta`/`done`) and the rollout `response_item` for the same `msg_…` id. The chain:
1. The frames drive the real `CodexResponsesAdapter` (codex-headless adapter harness) and the real `foldSemanticEvent`.
2. The ghost is minted by `ghostsFromSemanticTurn`.
3. The rollout record goes through the real mapper and `reconcileUpstream`.

The ghost must be superseded by that entry's uuid. It is red on main. A second rollout message with a different id must NOT supersede it.

## Out of scope
- Reasoning ghosts get the same match for free, where a mapped entry exists.
- The `token_usage_record` route (parser#38 tracks the type itself).
57 changes: 43 additions & 14 deletions src/providers/codex/renderer/transcript/rollout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,10 @@ type CodexMessageBlock =
//
// Ordering with `stampCodexTurnId`: Codex rollout doesn't put
// `turn_id` on per-item entries — only on `task_started` /
// `turn_started` and `turn_context`. Without tracking it here the
// ghost reconciler has nothing to match Codex assistant-text ghosts
// against (they don't carry `message.id`, don't carry `tool_use_id`).
// `turn_started` and `turn_context`. The feed's committed-text keys
// (renderUnits.ts) use it. Ghost reconciliation does NOT: a ghost is keyed
// by the proxy response id, which is not this turn UUID. Ghosts match on
// the item id instead (`stampCodexItemId`, #1231).

// Codex injects two synthetic user messages on the first turn of
// every conversation, both meant for the model and not the human:
Expand Down Expand Up @@ -228,36 +229,64 @@ function codexCompactSummaryEntry(
}

/**
* Extract the Codex rollout's per-turn response id from a
* `turn_context` side-channel entry. Returns null for any other
* Extract the Codex rollout's per-turn id (a UUID, NOT the proxy response
* id) from a `turn_context` side-channel entry. Returns null for any other
* entry type. Call sites that iterate a rollout stream use this to
* keep a rolling "current turn id" that subsequent `response_item`
* entries get stamped with via `stampCodexTurnId`.
*
* WHY: Codex rollout doesn't put `turn_id` on per-item entries —
* only on `task_started`/`turn_started` and `turn_context`. Without
* tracking it here the ghost reconciler in `reconcileUpstream` has
* nothing to match Codex assistant-text ghosts against (they don't
* carry `message.id`, don't carry `tool_use_id`).
* only on `task_started`/`turn_started` and `turn_context`. The feed's
* committed-text keys use it. It is NOT the id a Codex ghost is keyed by
* (that is the proxy response id), so `reconcileUpstream` matches ghosts
* by item id instead (#1231).
*/
export function codexTurnIdFromRollout(entry: Record<string, unknown>): string | null {
if (entry.type !== 'turn_context') return null
return stringField(asRecord(entry.payload), 'turn_id')
}

/**
* Stamp a mapped Codex feed entry with the rollout turn id so the
* ghost reconciler can supersede by turn id. The field is added as
* an Agent Code-local extension to the shared `Entry` type via cast —
* consumers that don't care about it ignore it, and
* `reconcileUpstream` reads it defensively.
* Stamp a mapped Codex feed entry with the rollout turn id. Its consumer is
* the feed's committed-text ownership keys (features/feed/ui/semantic/
* renderUnits.ts). The ghost reconciler does NOT use it: a ghost is keyed by
* the proxy response id, never this turn UUID, so ghosts match on
* `codexItemId` instead (`stampCodexItemId`, #1231). The field is an Agent
* Code-local extension to the shared `Entry` type via cast; consumers that
* don't care about it ignore it.
*/
export function stampCodexTurnId(entry: Entry, turnId: string | null): Entry {
if (turnId === null) return entry
return { ...entry, codexTurnId: turnId } as Entry
}

/**
* Stamp mapped Codex feed entries with the provider ITEM id (`msg_…`, `rs_…`,
* `fc_…`, `ctc_…`) of the rollout `response_item` they came from.
*
* WHY (#1231): this is the one id the live stream and the rollout share. A
* Codex ghost is keyed by the proxy response id (`resp_…`), while the rollout
* knows each item's id and its turn UUID, never the response id. So no
* committed entry could supersede a Codex text ghost, and every one orphaned.
* The proxy adapter already carries the same item id on the live block
* (`SemanticLiveBlock.itemId`); `reconcileUpstream` matches on it. It was
* verified on six real 0.157.1 sessions: every `msg_…` id on the proxy stream
* appears verbatim in the rollout. An Agent Code-local field like
* `codexTurnId`; consumers that don't care ignore it.
*/
export function stampCodexItemId(entry: Entry, itemId: string | null): Entry {
if (itemId === null) return entry
return { ...entry, codexItemId: itemId } as Entry
}

export function mapCodexRolloutToFeedEntries(entry: Record<string, unknown>): Entry[] {
const mapped = mapCodexRolloutToFeedEntriesUnstamped(entry)
if (entry.type !== 'response_item' || mapped.length === 0) return mapped
const itemId = stringField(asRecord(entry.payload), 'id')
return itemId === null ? mapped : mapped.map(mappedEntry => stampCodexItemId(mappedEntry, itemId))
}

function mapCodexRolloutToFeedEntriesUnstamped(entry: Record<string, unknown>): Entry[] {
const payload = asRecord(entry.payload)
// Shared with the marker below and main's history loader (#1288): the old
// `ts:id|call_id|type` rule collided for a call and its output, and for
Expand Down
7 changes: 3 additions & 4 deletions src/renderer/src/features/feed/ui/Feed.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -258,10 +258,9 @@ export type FeedProps = Props
//
// That duplicate class is now prevented at its source. The ghost
// reducer (`reconcileUpstream` in src/renderer/src/session-runtime/ghosts.ts)
// supersedes Codex text ghosts by rollout response id once the
// rollout mapper stamps `codexTurnId` on committed entries
// (src/renderer/src/workspace/workspaceStore.ts::codexTurnIdFromRollout
// + stampCodexTurnId). The live view and the merged feed are split
// supersedes Codex text ghosts by provider item id, which the rollout
// mapper stamps on committed entries as `codexItemId` (#1231; the earlier
// turn-id match could never succeed). The live view and the merged feed are split
// by turn ownership (src/renderer/src/session-runtime/mergedEntries.ts), so
// there is no longer any path by which the same assistant text can
// reach both surfaces at once.
Expand Down
57 changes: 57 additions & 0 deletions src/renderer/src/session-runtime/ghosts.codexItemIdentity.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { describe, expect, it } from 'vitest'

import fixture from '../../../../testing/fixtures/ghosts/codex/message-item-identity.json'

import { ghostsFromSemanticTurn, reconcileUpstream } from '@renderer/session-runtime/ghosts'
import { mountCodexPane } from '@renderer/session-runtime/semantic/testing/proxyPaneDrivers'
import type { Frame } from '@renderer/session-runtime/semantic/testing/proxyPaneDrivers'
import { createCodexTranscriptEntryMapper } from '@providers/codex/renderer/transcript/mapper'

// #1231: a Codex text ghost is keyed by the proxy response id (`resp_…`), and
// the committed rollout entry never carried anything that equals it, so no
// Codex text ghost was ever superseded. Both sides do carry the provider ITEM
// id (`msg_…`): the proxy stream on output_item.added, the rollout on the
// response_item. The fixture is one real 0.157.1 assistant message seen from
// both sides (see its `evidence`). It drives the REAL codex proxy adapter and
// the REAL semantic fold, as the desktop hook does, then the REAL rollout
// mapper, through the same stateful wrapper all three production ingest
// surfaces use (review c: a bare mapper call would stay green if stamping
// moved into the wrapper).
const mapRollout = (record: unknown) =>
createCodexTranscriptEntryMapper().map(record as Record<string, unknown>).entries

function ghostFromRecordedStream() {
const pane = mountCodexPane()
pane.request('req-1')
pane.frames('req-1', fixture.sseFrames as Frame[])
const turn = pane.reducer.pane.semantic.currentTurn
if (!turn) throw new Error('recorded frames opened no semantic turn')
const ghosts = ghostsFromSemanticTurn(turn, 'recorded-session', new Map())
const textGhosts = [...ghosts.values()].filter(ghost =>
Array.isArray(ghost.message.content) &&
ghost.message.content.some(block => (block as { type?: string }).type === 'text'))
return { turn, ghosts, textGhosts }
}

describe('Codex text ghosts are superseded by their committed rollout entry (#1231)', () => {
it('supersedes the recorded message ghost when the rollout record for the same item lands', () => {
const { turn, ghosts, textGhosts } = ghostFromRecordedStream()
// The ghost is keyed by the response id; the rollout knows only the item.
expect(turn.turnId).toBe((fixture.sseFrames[0] as { response: { id: string } }).response.id)
expect(textGhosts).toHaveLength(1)

const [entry] = mapRollout(fixture.rolloutRecord)
expect(entry?.uuid).toBeTruthy()
const next = reconcileUpstream(entry!, ghosts)
expect(next.get(textGhosts[0]!.uuid)?._atp.supersededBy).toBe(entry!.uuid)
})

it('does not supersede it with a different message', () => {
const { ghosts, textGhosts } = ghostFromRecordedStream()
const other = structuredClone(fixture.rolloutRecord) as { payload: { id: string } }
other.payload.id = other.payload.id.replace(/.$/, c => (c === '0' ? '1' : '0'))
const [entry] = mapRollout(other)
const next = reconcileUpstream(entry!, ghosts)
expect(next.get(textGhosts[0]!.uuid)?._atp.supersededBy).toBeUndefined()
})
})
55 changes: 30 additions & 25 deletions src/renderer/src/session-runtime/ghosts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// `ClaudeEntry` records via atp's `createGhost`. When the
// authoritative JSONL entry lands (Claude's batched 100 ms drain;
// Codex's mpsc flush), `reconcileUpstream` matches by message.id /
// codexTurnId / tool_use_id and supersedes the ghost. If JSONL
// Codex item id / tool_use_id and supersedes the ghost. If JSONL
// never matches (Claude Code's auxiliary calls — title gen,
// predict-next-prompt, branch-name gen — are not written to the
// rollout), `orphanStale` flags the ghost after the TTL.
Expand All @@ -37,11 +37,16 @@
// so one upstream entry supersedes every ghost block for that
// turn at once.
//
// Codex: rollout emits one entry per content block, with the
// rollout response_id stamped onto the mapped entry by
// `stampCodexTurnId` in ../codex/rollout.ts. Match is by
// (turnId, blockIndex). When that fails, both providers fall back
// to tool_use_id / call_id pairing for tool blocks.
// Codex: the ghost's turnId is the proxy response id (`resp_…`), which
// the rollout never records, so turn-level matching is impossible
// (#1231). Instead each ghost records its block's provider item id
// (`msg_…`, `fc_…`, `ctc_…`) in `context.itemId`, and the rollout mapper
// stamps the same id on each committed entry as `codexItemId`
// (`stampCodexItemId` in providers/codex/renderer/transcript/rollout.ts).
// Match is by that id, one block at a time. Reasoning ghosts (`rs_…`) are
// NOT matched: a reasoning rollout item maps to no feed entry, so they still
// orphan and stay hidden by the render rules, as before. Tool blocks can also still
// pair by tool_use_id / call_id.
//
// -----------------------------------------------------------------------------
// Reference stability
Expand Down Expand Up @@ -341,6 +346,9 @@ function ghostContextForBlock(
if (block.messagePhase) out.messagePhase = block.messagePhase
if (block.toolUseId) out.toolUseId = block.toolUseId
if (block.callId) out.callId = block.callId
// The Codex provider item id (#1231): the only id the live block and
// its committed rollout entry share. `reconcileUpstream` matches on it.
if (block.itemId) out.itemId = block.itemId
return out
}

Expand All @@ -360,11 +368,10 @@ function ghostContextForBlock(
* landing validates all provisional blocks for that turn.
*
* Codex: rollout emits one entry per content block with its own
* uuid. We match by (turnId, blockIndex). turnId for Codex
* comes from the response id when available; Agent Code's
* Codex ingest path already exposes it on the mapped
* entry. When the mapping is not available we fall back
* to tool_use id pairing for tool blocks.
* uuid, and never the proxy response id a ghost is keyed by.
* We match one block at a time on the provider item id: the
* ghost's `context.itemId` against the entry's `codexItemId`
* (#1231). Tool blocks can also pair by tool_use id.
*
* Both: if the upstream entry has a tool_use block whose id
* matches a ghost's `context.toolUseId`, supersede that
Expand Down Expand Up @@ -396,15 +403,13 @@ export function reconcileUpstream(
typeof messageRecord?.id === 'string'
? messageRecord.id
: null
// Codex rollout-sourced entries don't carry message.id (the Codex
// response id lives elsewhere on the rollout payload). Plumbing it
// through the mapper to this matcher is Task 6 of the rendering-
// fixes plan; the field is read defensively here so the match path
// lights up the moment `mapCodexRolloutToFeedEntries` stamps it.
// Codex rollout-sourced entries carry no message.id and never the
// proxy response id a Codex ghost is keyed by. They do carry the
// provider item id, stamped by the rollout mapper (#1231).
const codexRecord = asRecord(entry)
const codexTurnId =
typeof codexRecord?.codexTurnId === 'string'
? codexRecord.codexTurnId
const codexItemId =
typeof codexRecord?.codexItemId === 'string'
? codexRecord.codexItemId
: null

// Gather tool_use ids carried by this upstream entry — used for
Expand All @@ -428,12 +433,12 @@ export function reconcileUpstream(
// Claude match by message.id → turnId equality.
if (messageId && ghost._atp.turnId === messageId) match = true

// Codex match by response id → turnId equality. Ghosts are minted
// with `turnId = responseId` when the live source is Codex rollout,
// so a committed entry carrying the same responseId supersedes
// every ghost for that turn in one shot — matching the Claude
// message.id contract.
if (!match && codexTurnId && ghost._atp.turnId === codexTurnId) match = true
// Codex match by provider item id (#1231). This replaced a
// `codexTurnId === ghost.turnId` comparison that could never be
// true: the entry's codexTurnId is the rollout turn UUID, and the
// ghost's turnId is the proxy response id. Every Codex text ghost
// therefore orphaned. The item id is exact per block.
if (!match && codexItemId && ghost._atp.context?.itemId === codexItemId) match = true

// Shared: tool_use id equality. Works for both providers; wins
// over message-id in ambiguous cases (the ghost knows the exact
Expand Down
4 changes: 2 additions & 2 deletions src/renderer/src/workspace/hook/ipc/useIpcSubscriptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2321,8 +2321,8 @@ export function useIpcSubscriptions(
: current.limitHit

// Ghost reconciliation — when authoritative entries land,
// supersede any live ghost whose `(turnId, blockIndex)`
// they replace. Runs per appended entry so ghost→real
// supersede any live ghost they replace (Claude: message id;
// Codex: provider item id, #1231; either: tool_use id). Runs per appended entry so ghost→real
// handoff is synchronous with the entry becoming visible;
// the ghost drops out of the merged view in the same
// render as the real entry appears.
Expand Down
Loading
Loading