refactor(agent-core-v2): carry the context fold cursor in state and converge fold/projection internals - #2875
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". |
Apply settleModelOpenStep inside context.apply_compaction so a marker only ever lands on a settled frame: no partial survives a marker and nothing mutates the log behind one, making the append-only invariant structural (the identity-prefix check in historySafeToCompact relied on it) instead of timing-dependent. Mirrors the transcript's settle-at-marker. Also freeze the derived visible window before caching it (an in-place consumer mutation now throws instead of silently polluting the shared cache), and drop the production-unreachable legacy branch of buildContextCompactionShape so the legacy tail layout lives only in deriveCompactionWindow.
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? 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. Keep them coming! 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: |
…y internals - Slim file headers to the package header-only comment convention - Rename PR-introduced identifiers for clarity: getMessageLog, ProjectionPolicy.structure, pendingToolCallIds/deferredEntries, removedEntryCount, deriveVisibleWindowAfterCompaction, compactedWindowMessageCount - Extract nextProjectionPolicyForError, removeUndoOwnedEntries and summarizeProjectionRepairs; name fold intermediates after their business stage - Regroup splice-replay tests by topic and unify projection-call recording in llmRequester tests
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 641157e922
ℹ️ 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 next = state.slice(); | ||
| next[index] = update(next[index]!); | ||
| return next; | ||
| const messages = state.messages.slice(); |
There was a problem hiding this comment.
After the first compaction, ContextState.messages now retains the entire pre-compaction log, but every content.part event reaches updateOpenAssistant and copies that whole array here. Consequently, streamed responses in long or repeatedly compacted sessions incur work proportional to all messages ever recorded for every token/part; over the session lifetime this becomes quadratic allocation/copying and removes the memory/CPU bound that compaction previously provided. Keep the active frame separate or use a structurally shared log so updating the open assistant does not clone the retained history.
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: 2c36afe6dd
ℹ️ 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".
| /** | ||
| * `contextMemory` domain — `IAgentContextMemoryService` implementation. | ||
| * | ||
| * Owns per-agent conversation history through `wire`, maintains measurements | ||
| * with `tokenCounting`, and broadcasts live mutations through `event`. Every | ||
| * splice-shaped mutation (`clear` / `applyCompaction` / `undo`, plus verified | ||
| * cross-model trailing removal) publishes `context.spliced` from the live path | ||
| * only — replay rebuilds silently — and truncates the measured-anchor ledger | ||
| * when a cut crosses an anchor, letting `tokenCounting` restore the surviving | ||
| * prefix's REAL size from the remaining anchors. Bound at Agent scope. | ||
| * Owns the Agent-scoped bounded context window, persists its mutations, keeps | ||
| * token anchors aligned, and publishes live splice events to consumers. | ||
| */ |
There was a problem hiding this comment.
The shortened header no longer starts with the required `contextMemory` domain — ... identity line and omits the implementation’s cross-domain collaborators (wire, tokenCounting, and event) and Agent binding scope. Restore those boundary details here, and in the other source headers shortened by this change, so the module documentation follows the package convention.
AGENTS.md reference: packages/agent-core-v2/AGENTS.md:L38-L40
Useful? React with 👍 / 👎.
| return foldAppendMessage(state, record['message'] as ContextMessage); | ||
| case 'context.append_loop_event': | ||
| return foldLoopEvent(state, record['event'] as LoopRecordedEvent); | ||
| case 'context.undo': | ||
| return contextUndo.apply(state, { count: record['count'] as number }); |
There was a problem hiding this comment.
These new parity tests construct the expected model by directly calling foldAppendMessage, foldLoopEvent, and the same op reducers used by production replay. Because both sides therefore share the implementation under test, a defect in the common fold kernel can produce identical wrong results and leave the suite green. Feed the records through the real wire/model public surface and compare its observable state with the transcript reducer instead.
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: a08542892c
ℹ️ 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".
| /** | ||
| * `contextMemory` domain helper — derives the v1-compatible full-compaction | ||
| * handoff shape for live rewrites, wire replay, and snapshot reducers. | ||
| * | ||
| * Token budgeting runs through an injectable {@link TokenEstimate}: the live | ||
| * path (`AgentContextMemoryService.applyCompaction`) passes the estimator | ||
| * from `IAgentTokenCountingService` (the raw heuristics — the | ||
| * `[token_counting]` strategy never gates internal estimates); the pure | ||
| * wire-replay / reducer paths keep the same heuristics — their estimate | ||
| * fallback only fires when a record lacks `tokensAfter`, so the measured | ||
| * chain is unaffected. | ||
| * Builds the bounded context window produced by compaction and exposes the | ||
| * shared user-message selection rules used by live execution and replay. |
There was a problem hiding this comment.
This rewritten header no longer begins with the required `contextMemory` domain — ... identity line and omits the helper’s cross-domain roles, including token estimation through kosong and reminder rendering through systemReminder. Restore those ownership details in the top-of-file block so the module remains discoverable under the package’s mandatory header convention.
AGENTS.md reference: packages/agent-core-v2/AGENTS.md:L38-L40
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! 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:
The initial refactor also exposed two boundary bugs: clearing an open empty step could corrupt transcript folded length on the next step, and legacy compaction could recreate the summary prefix identity before a swarm reminder pop was published. Its append-only context model additionally retained the complete session history and repeatedly scanned it to derive the visible window.
What changed
ContextModelnow stores{ messages, fold }; undo, clear, compaction, and swarm-mode removal reset the cursor structurally. Blob replay also covers deferred messages.ContextModeldestructively replaces its state with the compacted window. Legacy tail records keep their original layout, and async compaction still rejects histories whose identity prefix changed.project(messages, policy)entry point, and the requester accumulates recovery policies across retries.Verification: agent-core-v2 full suite green (327 files / 5260 tests), typecheck, import-boundary check, generated wire-manifest check, plus focused context/compaction/requester tests (249 tests).
No persisted journal record format changes; legacy v1/v2 compaction records continue to replay into the same bounded layouts.
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.)