fix(kap-server): render fork seed user messages in the unnumbered seed turn - #3759
Conversation
|
commit: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a26b190b3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const attachmentIds: string[] = []; | ||
| for (const part of content.slice(skipBlocks)) { | ||
| if (part.type === 'text') continue; | ||
| if (daemonFileRefFromPart(part) === undefined) continue; | ||
| entry.attachmentSeq += 1; | ||
| attachmentIds.push(attachmentIdOf(seedTurnId, entry.attachmentSeq)); |
There was a problem hiding this comment.
Count origin-backed seed attachments
When a fork seed prompt carries a regular file attachment in message.origin.attachments, this loop inspects only content parts, so the rendered seed user receives no attachment_ids. These files are stored separately from message.content—the numbered-turn path handles them via promptAttachmentCount(input, origin)—so fork history silently loses their attachment indicators; include the origin attachment count when allocating the seed IDs.
Useful? React with 👍 / 👎.
| const messageId = | ||
| typeof message.id === 'string' | ||
| ? message.id | ||
| : `${seedTurnId}.u${(phantomUserSeq += 1)}`; |
There was a problem hiding this comment.
Keep seed fallback IDs out of the numbered-turn counter
When a seed user message lacks message.id, incrementing phantomUserSeq here leaks the seed's t-1.uN allocation into the first numbered turn because onTurnPrompt carries that counter into its serverUserSeq. A subsequent ID-less steer in t0 is therefore folded as t0.u2 (or later), while the live projector—whose timeline seed does not restore this counter—emits t0.u1; reloading history can consequently change that entity's ID. Allocate missing seed IDs from the seed turn's own scratch sequence instead.
Useful? React with 👍 / 👎.
4a26b19 to
5f88ae4
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f88ae40f2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return; | ||
| } | ||
| if (!isUndoAnchorOrigin(message.origin)) return; | ||
| if (!seedEnded) { |
There was a problem hiding this comment.
Render shell-command inputs in the seed turn
When a fork seed contains a shell_command input, this new block is never entered because the preceding isUndoAnchorOrigin check excludes that origin. The engine persists shell input as a user message, and forkTurnSlice.ts explicitly treats phase: 'input' as user-visible, so the forked history drops the command and may omit t-1 entirely when there is no assistant message. Extend seed eligibility to shell inputs while continuing to exclude shell outputs.
Useful? React with 👍 / 👎.
| attachmentIds: attachmentIds.length > 0 ? attachmentIds : undefined, | ||
| skillActivations: skillActivationsOf(message.origin), | ||
| }; | ||
| users.set(messageId, draft); |
There was a problem hiding this comment.
Remove seed entities during context clears
When a forked agent containing one of these newly stored seed users later receives context.clear, the seed user survives cold folding: onClear only purges IDs accepted by turnOrdinalOf, whose ^t\d+$ check rejects t-1. The clear event therefore reports t-1 as removed while a subsequent history reload still emits the seed turn, user, and any associated details, unlike the live projector's full clear; handle the seed turn explicitly when removing timeline entities.
Useful? React with 👍 / 👎.
5f88ae4 to
61ef69f
Compare
|
Addressing the second-round review comments:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61ef69fbdf
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!visibleTurnOrdinals.has(SEED_TURN_RAW_ID)) { | ||
| visibleTurnOrdinals.add(SEED_TURN_RAW_ID); | ||
| timelineIds.push(turnIdOf(SEED_TURN_RAW_ID)); | ||
| } |
There was a problem hiding this comment.
Re-add the seed turn after a pre-boundary clear
When a legacy append-only wire contains a seed user, context.clear, and then another seed user before any turn.prompt or agent.fork, the clear empties timelineIds but leaves visibleTurnOrdinals containing -1. This branch therefore skips re-adding t-1, while foldWireHistory recreates that turn and user, so the live projector is seeded with a different timeline from /history and subsequent timeline rewrites omit t-1. Clear the seed ordinal with the timeline or determine visibility from the current timeline.
Useful? React with 👍 / 👎.
Related Issue
Follow-up to #3754 (bug-201). That PR aligned btw fork seed turns with engine turn ids but deliberately left seed-zone user messages unrendered; this PR closes that gap.
Problem
In a btw (side question) agent's history, the seed section — the main conversation copied into the fork — showed only assistant replies. Every user question from the main conversation was missing, because the history fold treats
role: 'user'append_message records with undo-anchor origins as pure undo-anchor registrations (correct dedup for live turns, where the user entity comes from theturn.promptrecord). The fork seed contains only append_message records and no turn.prompt records, so seed user messages never produced entities.What changed
agent.forkboundary record or, for legacy wires, the firstturn.prompt), user append_message records now render as user message entities inside the unnumberedt-1seed turn — text, timestamp, origin, attachments, and skill activations preserved, interleaved in wire order. They no longer register degenerate undo anchors pointing at rawId 0. The orphan-turn synthesis is extracted into a shared helper used by both the assistant and user paths.foldTimelineSeed): same seed-zone handling — seed user messages materialize thet-1skeleton and skip anchor registration, staying consistent with the cold fold.Checklist
/approve).gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.