refactor(agent-core-v2): unify the loop-event fold into one core with two materializations - #3018
refactor(agent-core-v2): unify the loop-event fold into one core with two materializations#30187Sageer wants to merge 7 commits into
Conversation
… two materializations The loop-event stream was reduced by two hand-mirrored state machines: loopEventFold.ts for the live/replayed context and contextTranscript.ts for the full transcript behind the messages endpoints, kept in sync by comments alone and already drifted (transcript dropped tool-result note metadata and never closed a dangling tool exchange at step.end). createLoopEventFold now owns the shared state machine once (settle, pending tool exchanges, deferred appends, vacuous tracking) and both views plug in as LoopEventFoldSink materializations. New parity tests pin the foldedLength === live length invariant the endpoints splice on.
|
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8da25da5e8
ℹ️ 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".
| @@ -1,3 +1,46 @@ | |||
| /** | |||
There was a problem hiding this comment.
This top-of-file JSDoc is not attached to an exported symbol, so scripts/check-no-comments.mjs reports it and makes pnpm lint fail; the newly added module block in contextTranscript.ts has the same issue and should also be removed.
AGENTS.md reference: AGENTS.md:L49-L51
Useful? React with 👍 / 👎.
| if (bound === undefined || bound.sink.current() !== key) { | ||
| const sink = createImmutableFoldSink(key); | ||
| bound = { fold: createLoopEventFold(sink), sink }; |
There was a problem hiding this comment.
When the fold binding is missing, this recreates the core with openStepUuid unset even though createImmutableFoldSink discovers an existing partial assistant. This occurs after restore whenever blob rehydration replaces the context array (for example, a conversation containing an uploaded image whose journal ends during an output-free step). A subsequently appended prompt and step.begin cannot settle the stale partial assistant, while the journal-based transcript does settle it; foldedLength then differs from the live context length and mergeLiveTail can duplicate or misalign messages. The rebuilt fold must recover enough open-step state or explicitly settle the discovered partial before processing later records.
Useful? React with 👍 / 👎.
…urn transcript undo The transcript undo only walked prompt-owned injections off the oldest counted anchor, so with count > 1 an injection owned by a newer removed prompt (e.g. an image-compression caption) survived the display undo while the live context removed it. Collect every counted anchor's id during the walk and sweep their owned injections afterwards, keeping the transcript's 'prompt-owned ones leave with their prompt' contract for every count and matching the live view.
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
…dules The comment-free zone lint only allows JSDoc on exported symbols.
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
commit: |
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
…ds mid-fold An overflow-triggered compaction arrives with the failed attempt's frame still open. The transcript appended the summary marker and reset the fold but left the frame, so a vacuous partial stayed in the entries while the live context dropped it, and a pending tool exchange lost its interrupted result. Settle through the shared fold core at the marker instead: close pending tool calls, drop or seal the open frame, then append the summary. recoverFoldedLength recomputes the absolute count right after either way.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd77f9f86f
ℹ️ 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".
| break; | ||
| } | ||
| case 'context.apply_compaction': { | ||
| fold.settle(record.time); |
There was a problem hiding this comment.
When replaying a legacy context.apply_compaction record that has compactedCount but no keptUserMessageCount while a tool call is open, this settlement adds an interrupted tool message and increases foldedLength before recoverFoldedLength evaluates 1 + (foldedLength - compactedCount). The live compaction path instead applies the legacy tail shape to the unsettled state and resets the fold, so the transcript count becomes one larger than the live context; mergeLiveTail in packages/kap-server/src/services/messages/messageHistory.ts can consequently omit the newest unflushed message. Preserve the pre-settlement live length for legacy recovery or otherwise calculate the count from the live compaction shape.
Useful? React with 👍 / 👎.
…ment count A legacy context.apply_compaction record (compactedCount without keptUserMessageCount) recovers foldedLength as 1 + (foldedLength - compactedCount), and the live legacy tail shape keeps the unsettled open frame inside history.slice(compactedCount). Settling the fold for those records shifted foldedLength by the settlement delta before the recovery read it, leaving the transcript count one off the live context. Gate the settle to modern records; legacy records keep the previous freeze-and-reset behavior.
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
2 participants
Related Issue
No linked issue — the problem is explained below.
Problem
agent-core-v2reduces the loop-event stream (step.begin/content.part/tool.call/tool.result/step.end, interleaved withcontext.append_message) twice:loopEventFold.tsfolds it into the live / replayedContextMessage[], andcontextTranscript.tsfolds the same records again into the full transcript (entries+times+foldedLength) behind the messages / transcript endpoints. The two state machines were hand-mirrored copies kept in sync by comments alone, and they had already drifted in small ways (the transcript side dropped tool-resultnotemetadata and never closed an interrupted tool exchange atstep.end). The invariant the endpoints rely on —foldedLength === live context length, used to splice the not-yet-flushed live tail — was enforced by nothing.What changed
One fold core, two materializations:
createLoopEventFold(sink)inloopEventFold.tsnow owns the shared state machine exactly once: step settle (drop output-free assistants / seal otherwise), pending tool exchanges, deferred appends, vacuous-content tracking. Materialization is behind aLoopEventFoldSinkinterface.foldLoopEvent/foldAppendMessage/resetFoldare unchanged in signature, implemented as an immutable sink whose cross-record state rides in aWeakMapkeyed by each committed state array (immer drafts resolve to theiroriginal).contextTranscript.tsdeletes its copy of the state machine and drives the same core with a transcript sink; undo / clear / compaction stay local since they rewrite the materialization itself. kap-server consumers are untouched.note(matching the live view and v1), andstep.endcloses a dangling tool exchange before settling.computeUndoCutboundary. This fixes both the earliercount > 1survivor and the repeated-prompt-ID case, where an ID-wide cleanup could incorrectly remove an older prompt's injection.context.apply_compactionnow settles a frame left open by a failed attempt (overflow compaction lands mid-fold) through the shared core before the summary marker is appended: pending tool calls close with their interrupted result and a vacuous partial is dropped instead of surviving in the entries while the live context replaces its state wholesale. Legacy records (nokeptUserMessageCount) skip the settle: their recovery derives from the pre-settlement count and the live legacy tail shape carries the unsettled frame. This re-applies the transcript side of a7a0b45 from refactor(agent-core-v2): carry the context fold cursor in state and converge fold/projection internals #2875, which was dropped when that branch's append-only-log approach was reverted before merge.Parity tests drive the same record stream through both views and pin the splicing invariant (
foldedLength === live.length), including multi-turn undo and repeated prompt IDs, so future divergence fails loudly instead of corrupting the messages endpoint.No behavior change is intended for well-formed journals beyond correcting transcript/live divergence. This is an internal
agent-core-v2refactor and bug fix, so there is no changeset (repo changeset rules skip v2-internal architecture changes) and no doc update.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.