Match Codex compaction and model-transition behavior - #6
Conversation
There was a problem hiding this comment.
Pull request overview
Aligns compaction, model-transition, history normalization, fallback, and token-accounting behavior with Codex.
Changes:
- Adds compaction-hash-aware transitions and checkpoint recovery.
- Normalizes cross-model history and tightens fallback eligibility.
- Improves retention and approximate token accounting.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
session-coordinator.ts |
Updates transition and recovery orchestration. |
session-coordinator.test.ts |
Expands transition and normalization tests. |
scheduler.test.ts |
Tests prefix-aware token thresholds. |
remote-compaction.ts |
Persists checkpoint hashes. |
README.md |
Documents compatibility boundaries. |
package.json |
Bumps package version. |
native-compaction.ts |
Updates normalization, retention, accounting, and fallback logic. |
native-compaction.test.ts |
Tests new compaction behavior. |
index.ts |
Adds prefix accounting and signal propagation. |
docs/adr/0001-remote-native-checkpoint.md |
Updates architecture decisions. |
CONTEXT.md |
Revises transition and accounting boundaries. |
capabilities.ts |
Adds compaction-hash resolution. |
capabilities.test.ts |
Tests hash resolution. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
native-compaction.ts:785
- The fallback status whitelist omits HTTP 404, so a retired previous model returning
model_not_foundis explicitly marked ineligible and the coordinator will not try the current model. The existingnot foundbody check suggests this is intended to be an eligible model failure; include 404 and apply the same model/not-found body gate while retaining the deny list.
function canFallbackForStatus(status: number, body: string): boolean {
if (![400, 403, 408, 409, 429].includes(status) && status < 500) return false;
if (status === 403 && !/(?:model|invalid request|not found|overloaded)/i.test(body)) return false;
return !/(?:malformed|misalignment_policy|cyber_policy|invalid[_ ]image|content policy|unauthorized|forbidden|permission|api key|authentication|invalid[_ ]token|cancel(?:led|lation)?|aborted)/i.test(body);
session-coordinator.ts:55
- The broad
tokendeny term runs before the eligible context-window checks. An unmarked SSE error such as “context token limit exceeded” is therefore treated as fail-closed even though it is a context-capacity failure that should retry on the current model. Narrow this term to authentication-token forms.
if (/(?:abort|cancel|auth|token|account|malformed|invalid compaction|misalignment_policy|cyber_policy|invalid[_ ]image|content policy|unauthorized|forbidden|permission|api key)/i.test(message)) return false;
native-compaction.ts:481
- The new tool-item accounting misses standard
function_callitems because their type does not contain the wordtool; they still use the old arguments-only estimate and omit names, IDs, and structural tokens. Histories with many small calls can consequently remain substantially undercounted. Includefunction_callin the serialized-item branch.
This issue also appears on line 782 of the same file.
if (typeof item.type === "string" && item.type.includes("tool") && item.type !== "function_call_output") {
return Math.max(1, Math.ceil(JSON.stringify(item).length / 4));
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
session-coordinator.ts:389
- This lock is checked only after transition/recovery work. If model A is automatically compacting and a request for model B starts, B can run transition compaction concurrently with A. Because
conversationLeafIdignores the custom checkpoints, both operations may append; if A finishes last, B resumes here and returns A's now-latest checkpoint without rerunning the hash compatibility check above. Serialize automatic and transition compactions through one per-session operation, or await this operation before transition/recovery and restart request preparation afterward.
const activeAutomaticCompaction = automaticCompactionBySession.get(sessionId);
if (activeAutomaticCompaction) {
await activeAutomaticCompaction;
} else {
native-compaction.ts:868
- Eligibility here ignores the failure message, so any
invalid_promptresponse is explicitly marked retryable even when the message identifies a fail-closed condition such asinvalid_image,cyber_policy, or malformed input. The explicit marker bypasses the coordinator's deny-list. Apply the same fail-closed classification used for HTTP failures before setting the marker.
const code = typeof failure.code === "string" ? failure.code : "response.failed";
const message = typeof failure.message === "string" ? failure.message : "OpenAI Codex compaction ended with response.failed.";
const eligible = code === "context_length_exceeded" || code === "invalid_prompt";
throw markFallbackEligibility(new NonRetryableCompactionError(`OpenAI Codex compaction failed (${code}): ${message}`), eligible);
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
native-compaction.ts:497
- An image-bearing
function_call_outputnow contributes the fixed image weight, buttrimFunctionCallHistoryToFitContextWindowdelegates totruncateFunctionOutput, which only changes string outputs. Thus an image-only output that exceeds the remaining budget is returned unchanged (for example, 1,200 estimated tokens with a 100-token limit), so the supposedly trimmed remote request can still exceed the context window. Replace image-bearing output with bounded text when it must be trimmed.
const imageTokens = imagePartCount(item) * 1_200;
if (imageTokens > 0) return imageTokens + Math.max(0, Math.ceil(responseItemText(item).length / 4));
native-compaction.ts:805
- The fail-closed deny list still misses common machine-readable auth/policy forms. For example, a 400 body containing only
{"code":"invalid_api_key"}or{"code":"policy_violation"}does not matchapi key/content policy, so it is explicitly marked eligible and the coordinator retries it with the current model. Match underscore/hyphen variants (and generic policy-violation codes) before setting fallback eligibility.
return !/(?:malformed|misalignment_policy|cyber_policy|invalid[_ ]image|content policy|unauthorized|forbidden|permission|api key|authentication|(?:invalid|expired|bearer|refresh)[ _-]?token|cancel(?:led|lation)?|aborted)/i.test(body);
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (2)
session-coordinator.ts:94
- Content equality alone cannot identify the turn that
preservedInputbelongs to. If an earlier checkpoint preserved"hello"and a later user sends"hello"again, this returns true and lines 467-469 remove the new request from the tail; during automatic compaction, the similar check inpreserveCurrentUseralso mistakes the older copy for the current one and fails to persist it. Use branch ordering to require the checkpoint to occur after the persisted current-user entry before treating that user as already preserved.
const preservedUser = checkpoint.status === "valid" ? checkpoint.checkpoint.details.preservedInput?.findLast((item) => item.role === "user") : undefined;
return Boolean(requestUser && preservedUser && textContent(preservedUser.content) === textContent(requestUser.content));
native-compaction.ts:886
- A machine-readable SSE
errorwith an eligible code is not marked eligible here. For example,{ type: "error", code: "context_length_exceeded", message: "too large" }throws an unmarked error whose message does not match the coordinator's fallback heuristic, so the current-model fallback is skipped. Mark known request/model codes explicitly while marking unknown and fail-closed codes false, as theresponse.failedbranch already does.
const error = new NonRetryableCompactionError(event.message);
throw code && isFailClosedCompactionError(code)
? markFallbackEligibility(error, false)
: error;
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (4)
session-coordinator.ts:275
- In the unavailable-previous-model recovery path, the current user may already be part of the compaction input (for example, during a tool-turn continuation), but omitting that input from
preserveCurrentUseralways adds the matching user topreservedInput. V2 then retains the user inreplacementHistoryand replays it again frompreservedInput. Reuse the exact input sent tocreateCheckpointwhen deciding whether preservation is needed.
deps.appendCheckpoint({
...preserveCurrentUser(native.details, branch, requestInput),
modelKey: modelKey(model),
...(currentHash ? { compHash: currentHash } : {}),
});
native-compaction.ts:826
- The explicit eligibility marker returned here bypasses the coordinator's additional
auth|account|invalid compactionfail-closed check. Consequently, bodies such asaccount suspended,authentication required, orinvalid compaction stateon an otherwise eligible status are retried with the current model, contrary to the fail-closed contract. Apply the same deny terms before marking the HTTP error eligible.
function canFallbackForStatus(status: number, body: string): boolean {
if (![400, 403, 404, 408, 409, 429].includes(status) && status < 500) return false;
if (status === 403 && !/(?:model|invalid request|not found|overloaded)/i.test(body)) return false;
if (status === 404 && !/(?:model|invalid request|overloaded)/i.test(body)) return false;
return !isFailClosedCompactionError(body);
session-coordinator.ts:95
- This deduplication relies only on content in
preservedInput. A transition followed immediately by automatic compaction absorbs that preserved current user into the second checkpoint'sreplacementHistory, leavingpreservedInputempty, so lines 469-470 keep the same user in the request tail and send it twice. Conversely, a later new turn that repeats the old preserved text is removed. The checkpoint needs an unambiguous current-turn identity/marker rather than a content-only test across checkpoint generations.
function checkpointPreservesCurrentUser(branch: SessionEntry[], requestInput: ResponseItem[] | undefined): boolean {
const checkpoint = findNativeCheckpoint(branch);
const requestUser = requestInput?.findLast((item) => item.role === "user");
const preservedUser = checkpoint.status === "valid" ? checkpoint.checkpoint.details.preservedInput?.findLast((item) => item.role === "user") : undefined;
return Boolean(requestUser && preservedUser && textContent(preservedUser.content) === textContent(requestUser.content));
session-coordinator.ts:109
- Matching any user in the compaction input by content does not prove that the current turn was included. If an older user has the same text as the current persisted user,
branchBeforeCurrentUserexcludes the current entry but this check matches the older one and skipspreservedInput; later replay then loses the current user at its post-checkpoint position. Pass an explicit indication that the current branch entry was excluded instead of inferring it from content.
const currentUser = branch.findLast((entry) => entry.type === "message" && entry.message.role === "user");
if (!requestUser || !currentUser || currentUser.type !== "message") return details;
if (compactionInput?.some((item) => item.role === "user" && textContent(item.content) === textContent(requestUser.content))) return details;
return textContent(currentUser.message.content) === textContent(requestUser.content)
? { ...details, preservedInput: [structuredClone(requestUser)] }
Summary
Validation
bun test— 90 passing, 0 failinggit diff --check391f147.Known parity boundaries
Exact Codex tokenizer usage, Responses-Lite wire shape, mid-turn continuation beyond the extension seam, fresh token-budget windows, model-switch world-state injection, compaction-window metadata, and canonical provider capability metadata remain unavailable or insufficiently represented in Pi extensions.