fix(client-core): keep image prompts in place after conversation reseed - #464
fix(client-core): keep image prompts in place after conversation reseed#464lucas77778 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Important
The enrichment advance registers a second replay entry under the seed's messageId, which breaks conversation-rewind for exactly the prompts this PR fixes. Reproduced locally — details inline.
Reviewed changes — the coveredBySeed → foldPreCutEvent inversion in packages/client/core/src/conversation-store.ts plus the new integration test, reviewed against the four agent adapters' history-read paths.
- Coverage check inverted —
coveredBySeed(event, …) => boolean(consulted by the caller) becomesfoldPreCutEvent(builder, event, receivedAt, …) => void, which callsbuilder.advanceitself per branch. I traced this branch by branch and it is semantically faithful:default:still advances, and theseq > uptoSeqshort-circuit still skips seed-queue consumption for post-cut events. - Seed rows retained, not counted —
Map<string, number>becomesMap<string, SeedUserMessageQueue>({ messages, nextIndex }), so the matched seed event itself is recoverable. In-order consumption for repeated identical prompts is preserved. - Image enrichment — a live
user-messagecarrying animageblock whose full content key misses retries with the image-filtered content, then re-advances the live content under the seed'smessageIdandbranchCursor. OmittingreceivedAtis what keeps the seed timestamp (the projection doesreceivedAt ?? item.receivedAt) — subtle, and correct. - Test — asserts the enriched message keeps the seed id/cursor, carries the image blocks, and that no duplicate lands at the bottom. It genuinely fails with the bug present.
I confirmed the premise holds where it needs to: textHistoryEvent (packages/host/agent-adapter/src/history-util.ts:67) funnels user history rows through textFromUnknown and emits exactly [textBlock(text)], so claude-code and opencode do drop image blocks, and the filtered live content serializes identically to the seed row. codex preserves images (native/codex/history.ts:153), so its first lookup hits and the fallback never fires. No adapter substitutes a placeholder block, and none drops non-image block kinds — so the image-only filter is the right scope.
ℹ️ Image-only prompts still land at the bottom
textHistoryEvent returns undefined when the text is empty (history-util.ts:76), so a prompt that is only an image produces no seed row at all on claude-code and opencode. Neither the full-content key nor the image-filtered key ("[]") can match, so the live echo falls through to the plain builder.advance and is appended below the reply it prompted. The composer permits this input, so the reported symptom survives for that case.
This isn't fixable from conversation-store.ts — there is nothing to enrich — so it's a scope question rather than a defect in this diff. Worth deciding explicitly whether it's out of scope for CODE-602 or whether the real fix belongs in the adapters.
Technical details
# Image-only prompts have no seed row to enrich
## Affected sites
- `packages/host/agent-adapter/src/history-util.ts:76` — `if (text.trim().length === 0) return undefined;` drops the entire history row for a prompt whose only content was an image, so no `user-message` seed event is emitted.
- `packages/client/core/src/conversation-store.ts:71-85` — both lookups miss (full key has an image the seed never had; filtered key is `"[]"` and no seed row exists), so control reaches `builder.advance(event, receivedAt)` at line 86 and the prompt is appended at the tail.
- `packages/presentation/ui/src/shell/composer.tsx:1053` — send is enabled on `hasReadyAttachment` with empty text, so an image-only prompt is reachable from the UI.
## Required outcome
- Either an explicit decision that image-only prompts are out of scope for this change (with the residual behavior noted), or a history-read that emits a `user-message` row whenever the prompt had *any* content, so there is a seed row for the client to enrich.
## Open questions for the human
- Is the intended long-term fix client-side reconstruction, or should `textHistoryEvent` stop discarding non-text blocks so the seed is faithful and the `JSON.stringify` matching becomes unnecessary? The current approach is a value-equality workaround for a lossy upstream read, and it only works while the two construction sites happen to serialize identically.ℹ️ Nitpicks
- The deleted
coveredBySeedJSDoc carried the reason thedefault:branch must advance (ephemeral events — a pending permission request — are never inhistory.read, so cutting them erases them outright). The replacement one-liner drops that, leaving a barebuilder.advanceindefault:with nothing to stop a future reader from "optimizing" it. A one-line note on that branch would preserve the trap without restoring the essay. - Reflowing the
createConversationStoreJSDoc leftconversation-store.ts:105over-long (… (see {@link foldPreCutEvent}). The sync is idempotent and monotone with a stable snapshot identity). Formatters don't rewrap comments, so it needs a manual wrap.
Claude Opus | 𝕏
| builder.advance({ | ||
| ...event, | ||
| messageId: seedMessage.messageId, | ||
| branchCursor: seedMessage.branchCursor, | ||
| }); |
There was a problem hiding this comment.
This advance registers a second entries row under the seed's messageId, which breaks conversation-rewind for enriched prompts. createConversationBuilder.advance appends every event to a replay log, and the rewind handler scans that log backwards for the first matching user-message id (conversation.ts:211-219) — so the cut lands on this enriched row instead of the seed row, leaving the rewound prompt and the agent reply between them on screen.
I reproduced it: with a lossy seed the timeline after a rewind + replacement is ['provider-prompt', 'reply', 'replacement']; with a faithful seed (so the fallback never fires) the same sequence correctly yields ['replacement'].
Technical details
# Enrichment advance corrupts the `conversation-rewind` replay log
## Affected sites
- `packages/client/core/src/conversation-store.ts:78-82` — advances the live content under `seedMessage.messageId`, after the seed fold at line 158 already advanced the seed event under that same id. `entries` now holds two `user-message` rows sharing one `messageId`.
- `packages/client/core/src/conversation.ts:211-219` — the rewind scan walks `entries` from the end and cuts at the first id match, i.e. the enriched row. `entries.slice(0, cut)` therefore retains the original lossy seed row and every event that landed between the two.
## Reachability
- `packages/presentation/ui/src/chat/user-message.tsx:98` — `onEditPrompt(item.id, item.branchCursor, …)`. After enrichment `item.id` *is* the seed `messageId` and `item.branchCursor` is the seed cursor, so edit is enabled and keyed on the enriched identity.
- `packages/client/workbench/src/surface/workbench.tsx:346` — forwards that id as `sourceMessageId`; the engine echoes it back as `conversation-rewind` with the same `messageId`.
## Reproduction
Seed `[{ user-message 'provider-prompt', content: [text] }, { agent-message 'reply' }]`, `uptoSeq: 2`; live buffer holds the image-bearing echo `'host-prompt'` plus the same reply. Enrichment fires. Then send `conversation-rewind` for `'provider-prompt'` followed by a replacement prompt:
```
lossy seed (fallback fires): ['provider-prompt', 'reply', 'replacement'] // wrong
faithful seed (no fallback): ['replacement'] // correct
```
## Required outcome
- After a rewind targeting an enriched prompt, that prompt and every later item must leave the timeline, exactly as they do when the fallback never fires.
## Suggested approach (optional)
- Enrich the seed event's `content` *before* the initial fold rather than re-advancing afterwards, so `entries` keeps one row per message identity. The live buffer is already reachable from `sync()`, which folds the seed and then walks the live events on the same call.
- Alternatively, make the rewind scan cut at the *first* matching entry instead of the last. Since a user `messageId` is a single identity, forward and backward scans differ only when one id was folded twice — precisely this case.
## Open questions for the human
- A prompt rewrite also emits `session-ref` with a new `historyId`, which re-keys `useSeededConversation` and would build a fresh store from a fresh seed. How reliably does that reseed land? It probably bounds this to a transient window rather than a permanent wrong state, but it depends on the reseed arriving and on SWR yielding a new seed identity, so I would not lean on it.| id: 'provider-prompt', | ||
| role: 'user', | ||
| blocks: livePrompt.content, | ||
| branchCursor: 'provider-cursor', |
There was a problem hiding this comment.
The seed row here has no ts, so nothing in this test pins timestamp preservation even though the PR description claims it. That matters because the preservation is implicit: omitting receivedAt from the enrichment advance is the only reason the seed timestamp survives (receivedAt ?? item.receivedAt). A future refactor that threads receivedAt through would silently stamp the live receive time onto a history message with no test failing.
Giving the seed row a ts and asserting receivedAt alongside the existing fields would close that.
| branchCursor: 'provider-cursor', | |
| branchCursor: 'provider-cursor', | |
| receivedAt: 1_700_000_000_000, |

Summary
CODE-602
Verification
pnpm check:cipnpm test— 2974 passed, 1 skippedChecklist
pnpm check:ciandpnpm testboth pass (pluscargo fmt/clippy/testfor Rust changes)WIRE_PROTOCOL_VERSIONis bumped (not applicable; no wire change)