refactor(agent-core-v2): carry the context fold cursor in state and converge fold/projection internals - #2875
refactor(agent-core-v2): carry the context fold cursor in state and converge fold/projection internals#28757Sageer wants to merge 12 commits into
Conversation
…onverge fold/projection internals
- ContextModel state is now { messages, fold }: the loop-event fold cursor
(openStepUuid / pending / deferred) lives in the state instead of a
module-level WeakMap keyed by array identity, so wholesale replacements
(undo / clear / compaction / swarm exit) reset it structurally via
EMPTY_FOLD instead of a manual resetFold at five call sites.
- The display transcript and the wire model now share one generic fold
kernel (FoldFrame / FoldEntryAdapter), eliminating the mirrored second
implementation. Events tagged with a non-open step uuid are dropped and
step.end settles only the step it names — defensive in abnormal streams,
identical on well-formed ones (v1 replay unaffected).
- IAgentContextProjectorService converges to project(messages, policy) with
a ProjectionPolicy data object; llmRequester builds the policy from retry
state instead of selecting among four methods.
- Blob rehydrate now also covers messages still deferred in the fold cursor.
- ContextState is deeply frozen at the op boundary to preserve the consumer
immutability the wire's shallow freeze gave the bare array state.
|
|
@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: 8780d6a0d8
ℹ️ 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".
| // Asserts after EVERY prefix of the stream that the transcript's current | ||
| // conversation (the tail of `foldedLength` entries) matches the model fold. | ||
| // Compaction pauses the check until the next clear: the display's | ||
| // foldedLength is a UI collapse metric (kept users + summary marker, and | ||
| // legacy records put the summary first in the model but last in the | ||
| // display), so the two views diverge there BY DESIGN; clear realigns them. |
There was a problem hiding this comment.
This newly added inline explanation sits inside a helper body, and there are similar scenario notes in the added test case below; package guidance for agent-core-v2 requires comments to live only in the top-of-file /** */ block, so please move the parity rationale into the file header or encode it in helper/test names instead.
AGENTS.md reference: packages/agent-core-v2/AGENTS.md:L36-L38
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d419454df
ℹ️ 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".
| openStepUuid: string | undefined; | ||
| pending: Set<string>; | ||
| deferred: ContextMessage[]; | ||
| /** A fold position: the entries reduced so far plus the fold cursor. */ |
There was a problem hiding this comment.
This new declaration-level JSDoc sits outside the top-of-file module header; in agent-core-v2, comments are restricted to the file header rather than beside declarations. Please move the fold-frame explanation into the module header or encode it in the exported names, and apply the same cleanup to the other added declaration comments in this change.
AGENTS.md reference: packages/agent-core-v2/AGENTS.md:L36-L38
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. 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". |
|
@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". |
…s in context memory - isVacuousContentPart and dehydrateRecord now switch exhaustively over ContentPart / LoopRecordedEvent variants, so a new variant fails compilation until it takes an explicit position - the transcript/model parity comparator spreads whole messages and masks only summary content, so new ContextMessage fields join the comparison automatically - correct two stale header comments: local message ids persist with append_message records, and undo's prompt-owned-injection pairing depends on them after a resume
|
@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". |
The model Op and the display transcript each walked the undo anchors with their own loop, and the transcript partially removed the tail when an undo was blocked (compaction summary / clear floor / too few anchors) while the model side no-ops at the precheck. Move the walk into conversationTime as computeUndoCut/computeUndoCutFrom applied destructively by the context.undo Op and non-destructively by the transcript reducer, so a blocked undo reads identically on both sides. Also: make isUndoAnchor exhaustive over origin kinds with a never assertion, mirrors the compaction result message count via compactionHandoff, and extend UndoCut with anchorIndex distinguishing the counted anchor from the injection-extended cut point.
…anscript undo The transcript's kept-loop retained every injection after the oldest counted anchor, so with count > 1 a prompt-owned injection of a newer removed prompt (e.g. an image-compression caption) survived the display undo while the model Op removed it. Collect the removed anchors' ids on the same pass and keep only injections not owned by them, so the header's 'prompt-owned ones leave with their prompt' holds for every count.
The llmRequester retry chain kept a RequestProjection union and translated
it into a ProjectionPolicy per attempt; repairs were mutually exclusive,
so a strict resend rejected again for body size or image format either
aborted or silently dropped the strict repair. Retry state is now the
ProjectionPolicy itself: each rejection adds its repair on its own axis
(media: 413 -> degraded -> strip; wire: structure -> strict) without
discarding the other, requestInput's translation layer and the unreachable
snapshot ??= disappear, and the persisted llm.request projection name
derives from the policy (the op enum gains strict-media-degraded /
strict-media-stripped). Also narrows ProjectionPolicy to the variants
actually produced (wire 'strict'; media 'degraded' | { strip }), dropping
the dead 'default'/'keep' literals and their guard.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8ee42ce8f
ℹ️ 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 assistant: ContextMessage = { role: 'assistant', content: [], toolCalls: [], partial: true }; | ||
| ctx.openStepUuid = event.uuid; | ||
| return bind([...settled, assistant], ctx); | ||
| const settled = settleOpenStep(state, adapter, makeEntry); |
There was a problem hiding this comment.
When a user clears while a turn is streaming, the transcript reducer keeps the pre-clear entries but resets only fold; that leaves the aborted assistant entry with partial: true. The next step.begin reaches this unconditional settle path, scans the retained messages, and drops/seals that pre-clear partial before appending the new step, so foldedLength does not count the new post-clear assistant and the messages endpoint can lose or mis-merge the live tail after /clear during a run. Only settle when the fold cursor actually has an open step, or otherwise keep the shared fold from seeing entries before the clear floor.
Useful? React with 👍 / 👎.
…pend-only log context.apply_compaction now appends a summary marker carrying the record fields as CompactionMeta instead of replacing the folded history; the model-visible window is derived at read time (visibleWindow) with the same [head, elision?, tail, summary] layout, one deterministic derivation for live dispatch and replay alike. The record format is unchanged, so v1- and v2-written sessions keep replaying identically both ways. - undo maps the visible-window cut back to a log position (the verbatim legacy-summary edge falls back to the pre-append-only destructive cut) - replay rehydrate only loads blobs the window derivation can surface - contextInjector drops position tracking; injection positions become a read-time scan that splices can never desync - fullCompaction's safety check moves to the stable-identity log
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ddab0cd2d
ℹ️ 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 result: ContextMessage[] = []; | ||
| for (let i = 0; i < messages.length; i++) { | ||
| const message = messages[i]!; | ||
| if (i > lastMarker || message.compaction !== undefined || isRealUserInput(message)) { |
There was a problem hiding this comment.
When restoring a session with a legacy context.apply_compaction record, legacyTail makes deriveCompactionWindow keep window.slice(compactedCount) visible; that tail can contain assistant/tool media that sits before the appended marker. This condition only rehydrates pre-marker real user inputs plus the marker, so those visible legacy-tail entries stay as blobref: URLs after replay and can be surfaced or resent unresolved. Include the legacy-tail survivor range when deciding which pre-marker messages to transform.
Useful? React with 👍 / 👎.
… surfaces The first append-only rehydrate rule kept pre-marker real user input plus markers, but a legacyTail derivation keeps window.slice(compactedCount) visible — assistant/tool media in that range stayed blobref after replay and could be resent unresolved. Decide survivors by identity membership in the derived window instead, which covers every derivation branch at once and skips the unselected pre-marker pool as a bonus.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8892e84eb1
ℹ️ 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 last = state.messages.at(-1); | ||
| if (last === undefined) return state; | ||
| const origin = last.origin; | ||
| if (origin?.kind !== 'injection' || origin.variant !== 'swarm_mode') return state; |
There was a problem hiding this comment.
When a restored legacy compaction record has no keptUserMessageCount, the visible window is [summary, ...window.slice(compactedCount)]; if that legacy tail contains the swarm_mode reminder, SwarmService.exit() sees it via context.get() and publishes a pop splice, but this reducer checks only the raw append-only log tail, which is the compaction marker, so the model context keeps the stale swarm reminder after exit. Remove the visible tail entry (or map it back to the log) instead of only testing state.messages.at(-1).
Useful? React with 👍 / 👎.
…cy marker SwarmService.exit decides the pop on the derived window's tail, but the swarm_mode.exit reducer tested the raw log tail — after a legacyTail compaction the survivor reminder is visible at the window tail while the marker is the log tail, so the pop silently skipped and the stale reminder stayed in the model context. Mirror the visible-tail decision in the reducer and remove the entry by its stable log identity.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ba63a4978
ℹ️ 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".
| state = { | ||
| messages: [...state.messages, { message: summary, time: record.time }], | ||
| fold: EMPTY_FOLD, | ||
| }; | ||
| foldedLength = recoverFoldedLength(record, state.messages, clearFloor, foldedLength); |
There was a problem hiding this comment.
When auto-compaction is triggered by a context-overflow failure after beginStep has already appended an empty partial assistant, this branch appends the compaction summary and resets fold while leaving that partial in state.messages. The retried step's next step.begin then settles/removes the stale partial before adding the new assistant, so applyKernel sees no length increase and foldedLength stays at the summary count; kap-server's live-tail merge uses that count, so the post-compaction retry tail can be duplicated or merged incorrectly in message history. Settle/drop the open transcript frame as part of compaction, or otherwise keep pre-compaction partials from being settled by the next step.
Useful? React with 👍 / 👎.
…ds mid-fold An overflow-triggered compaction arrives with the failed attempt's vacuous partial still open. The transcript appended the summary marker and reset the fold but left the frame; the retried step's step.begin then settled it (-1) alongside the new frame (+1), so foldedLength stayed one short of the model-visible window and kap-server's live-tail merge could duplicate the retry tail. Settle the frame at the marker through the shared kernel; recoverFoldedLength recomputes the absolute count right after either way.
|
@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". |
2 participants
Related Issue
No linked issue — this is a preventive internal refactor of the agent-core-v2 context subsystem; the problem is explained below.
Problem
Three structural gaps in the context subsystem were held together by convention rather than by structure:
openStepUuid/pending/deferred) lived in a module-levelWeakMapkeyed by state-array identity. Every wholesale state replacement (undo / clear / compaction / swarm-mode exit) had to remember to callresetFold— five call sites in a different module from the cursor's definition, with no compile-time signal if forgotten.loopEventFold) and the display transcript fold (contextTranscript) — kept in sync only by a doc comment ("semantics mirror the v1 fold exactly"), so any semantic change could silently drift the display from the live view.project/projectStrict/projectMediaDegraded/projectMediaStripped), and the LLM requester selected among them with nested ternaries keyed by retry state.What changed
ContextModelstate is now{ messages, fold }. Wholesale replacements reset the cursor by returningEMPTY_FOLDin the samereturnthat replaces the messages — there is no out-of-band reset left to forget.ContextStateis deeply frozen at the op boundary to preserve the consumer immutability the wire's shallow freeze gave the bare array state. Blob rehydrate now also covers messages still deferred in the cursor (previously their blob references were never resolved after a restore).FoldFrame<E>+FoldEntryAdapter<E>); the wire model folds bare messages and the display transcript folds time-stamped entries through the same kernel, keeping only its display bookkeeping (times,foldedLength,clearFloor). Two deliberate defensive tightenings fall out of the unification: events tagged with a non-open step uuid are dropped instead of mis-appended to a retried step, andstep.endsettles only the step it names. Well-formed streams (including v1 replay records) are unaffected.project(messages, policy). Projection variability is now data (ProjectionPolicy { wire, media }); the requester builds the policy from retry state instead of selecting among four methods. Repair telemetry is unchanged.Verification: agent-core-v2 full suite green (312 files / 4906 tests, including new transcript↔model prefix-parity tests and fold-reset assertions), typecheck, build, lint (0 errors), import-boundary check, plus downstream
kap-server/klienttypechecks.No journal record format changes (v1/v2 replay compatibility preserved); the
IAgentContextMemoryServicecontract is untouched (its ~20 consumers needed no changes).Checklist
gen-changesetsskill, or this PR needs no changeset. (No changeset: agent-core-v2 internal refactor, not user-perceivable.)gen-docsskill, or this PR needs no doc update. (No user-facing behavior change.)