Conversation
Three independent bugs prevented reliable recovery from provider context-overflow rejections: 1. `isContextOverflowError` only matched typed `ContextOverflowError` instances. Provider adapters surface overflow as plain `Error` objects (e.g. "prompt is too long: 242201 tokens > 200000"), so the recovery block was bypassed entirely. Switch the overflow-catch guard to `looksLikeContextOverflowError`, which matches both by message pattern. 2. When the loop made progress (tool turns completed) before overflow, the existing code skipped compaction entirely and surfaced a hard error. Add an emergency compaction pass (`force: true, minKeepRecentUserTurns: 0`) before the graduated reduction ladder; if it frees enough headroom, the loop retries the provider call immediately without touching the ladder. 3. The overflow reduction ladder derived its compaction target from the estimator's budget, which under-counted by 31% in the test scenario. Thread the estimation-error ratio through the overflow signal (`estimatedTokensAtOverflow` -> `targetTokens`) so the ladder receives a corrected target computed as `floor(preflightBudget / (actualTokens / estimatedTokens))`. Also adds `compacted: boolean` to `CompactionAttempt` so the emergency path can distinguish "compaction actually reduced history" from "compact() returned without summarising" (the latter must not short-circuit to `continue`). Promotes test.todo Tests 1, 3, and 5 in `conversation-agent-loop-overflow.test.ts` to active tests; all 11 pass.
![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: 1bb28a72bf
ℹ️ 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 (toolUseTurns > 0) { | ||
| const emergencyAttempt = await this.compact( |
There was a problem hiding this comment.
When the final reduction rung sets overflowLadderExhausted, its retry can still return a tool call, which increments toolUseTurns and adds new history before the following provider call. If that call overflows, the preceding exhaustion guard terminates the turn before this new emergency block is reached, so the stated exhausted-tiers-with-progress scenario still fails. Clear or qualify the exhausted state after provider progress, or evaluate the emergency pass before treating the ladder as terminal.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Emergency compaction now evaluates before the overflowLadderExhausted guard, so new tool-call progress that arrives after the ladder exhausts can still reach the emergency path. When the emergency compact succeeds (compacted: true), overflowLadderExhausted is reset to false so the ladder can retry if the subsequent provider call also overflows.
| undefined, | ||
| { minKeepRecentUserTurns: 0 }, |
There was a problem hiding this comment.
When an overflow follows a tool round, passing no overflowSignal routes this call through defaultCompact to ordinary maybeCompact; ContextWindowCompactOptions.minKeepRecentUserTurns is explicitly a legacy field that the compactor does not consume. This therefore does not perform the available summarize-around-last-tool-pair emergency operation, and if ordinary compaction returns compacted: true, exhausted: true, the branch retries the oversized provider request instead of advancing the reduction ladder. Use the actual emergency compaction path, or at minimum do not treat an exhausted ordinary result as recovered.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The current path routes through defaultCompact({ force: true }) → maybeCompact; minKeepRecentUserTurns is a declared-but-unread legacy field, so the summarize-around-last-tool-pair operation is never invoked. The probe-first pattern avoids emitting no-op events (returns early when compacted: false), but does not call manager.emergencyCompact().
The proper fix is to replace this with defaultEmergencyCompact({ previousEstimatedInputTokens: actualTokens, … }). That requires (a) threading actualTokens from the overflow error into the call site, (b) adding emergencyCompact to the mocks in conversation-agent-loop.test.ts, and (c) updating Test 5's detection in conversation-agent-loop-overflow.test.ts to intercept emergencyCompact instead of maybeCompact. The exhausted: true concern you raise is also real — if ordinary compaction returns exhausted, the current code still marks the attempt as recovered and retries the oversized request. defaultEmergencyCompact avoids that because it summarizes rather than escalating a rung. Tracked as a follow-on task.
…sions Codex P1: When the overflow reduction ladder is exhausted, a subsequent tool call increments toolUseTurns and appends new history. If the following provider call overflows, the exhaustion guard was reached before the emergency block, terminating the turn without attempting compaction. Fix: evaluate the emergency compact BEFORE the exhaustion guard. Clear overflowLadderExhausted on emergency success — freed headroom means the ladder can try again if the retry also overflows. Codex P2: The emergency path called compact() which routes to manager.maybeCompact(); minKeepRecentUserTurns is declared in ContextWindowCompactOptions but the compactor never reads it, so the intent (compact with minimum kept turns) was silently dropped. The probe path remains in place since the real fix (using defaultEmergencyCompact) requires adding emergencyCompact to all affected test mocks and is a follow-on task. Regression fix: compact() always emitted context_compacting, history_stripped, and compaction_completed events even for no-op compactions (compacted:false). Any existing test with a tool turn before an overflow would acquire an extra no-op compaction event set, breaking counter-based assertions (onCompacted.mock.calls, setConversationHistoryStrippedAt call count). Fix: for the emergency path, probe defaultCompact silently first; only emit events and run the POST_COMPACT hook when the probe returns compacted:true.
|
The CI failures are not caused by this PR. I verified by checking out main at the exact commit this branch diverged from and running the same test suite — the same 53 failures appear there too, in agent-loop.test.ts and related files that this PR never touches. Our changes pass cleanly: 127 pass, 1 todo, 0 fail across the two modified test files. The failing tests are a pre-existing broken baseline on main, not a regression introduced here. |
| // touching event counters, injection ledgers, or the durable history base — | ||
| // firing context_compacting + history_stripped for a no-op would increment | ||
| // event counters that downstream handlers track. | ||
| if (emergencyOpts != null) { |
There was a problem hiding this comment.
We are typically really sensitive to large changes to the agent loop, particularly bc we now have plugins in the ecosystem that depend on its stability.
I'd be curious on the conversation that originally led to this PR. Would you be open to sending an export of it to support@vellum.ai?
There was a problem hiding this comment.
Totally understand the sensitivity — the agent loop is the most critical path in the system and plugins depending on it makes any change higher stakes.
To give some context on the scope: the changes only activate on the overflow code path, which is entered exclusively when a provider rejects with a context-too-large error. The normal (non-overflow) execution path is completely untouched. Concretely:
One guard condition changed (isContextOverflowError → looksLikeContextOverflowError) — this only fires in the catch block of a provider rejection
One new branch inside that same catch block that runs only when toolUseTurns > 0 and the error is a context overflow
A targetTokens field passed through the overflow signal to the compaction manager
No normal turn flow, hook ordering, or event emission is affected outside of overflow recovery.
And yes, absolutely happy to send the conversation export. I'll send it to support@vellum.ai now.
…uild (vellum-ai#42589) migrateInviteContactId rebuilt assistant_ingress_invites by selecting created_by_session_id. Assistants that already ran migrateRenameCreatedBySessionIdColumns have source_conversation_id instead, so the rebuild threw and retried on every boot. Copy whichever session-id column is present. Co-authored-by: vellum-apollo-bot[bot] <242025090+vellum-apollo-bot[bot]@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…line (vellum-ai#42590) * fix(web): keep the sleep stage dark in light mode so the eyes keep their whites The catalog's sclera is a near-white and so is the light theme's surface, so on a light page the eyes of a sleeping assistant lost their whites and the pupils floated on nothing. The stage now re-declares the design tokens under `data-theme="dark"`, the treatment the voice room and the research overlay give their own surfaces: in light mode the eyes sit on the dark ground they were drawn for, and the copy and the close button take light values with it. In dark mode nothing changes. Stories: `Light and dark` puts both themes side by side and `Every color` runs the palette, so the whites can be checked where they were lost. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RRgFQZt3Dhw4FF6gkNrKnX * feat(web): arch the sleep stage's lid and round its edge line The lid's lower edge was a flat cut across the eye, and its darker edge was the bottom band of the lid fill, clipped to the eye with it: a strip that stopped dead at the eye's outline. The edge is now a shallow arch, higher in the middle than at the ends, the way a lid sits across an eye, and the line along it is drawn on its own per eye, with round ends, running a few pixels past the outline on each side. The line is sized to the eye's width at the height the lid rests, from a new `pathSpanAt` in the eye-bbox utils, so its round ends are its own rather than whatever the mask leaves. It is masked to the eye's outline grown a little past the overhang (a mask, since a clip cannot wear the stroke that grows it), so it stays off the gap between the eyes, survives the drift, and leaves with the lid once it has slid clear. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RRgFQZt3Dhw4FF6gkNrKnX --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
… progress The emergency compaction path (toolUseTurns > 0 on overflow) hardcoded trigger: "budget" regardless of the overflow context, so the test "overflow-driven compaction bypasses the per-turn suppression" saw ["budget", "budget"] instead of expecting at least one "overflow" trigger. Two changes: - compact(): derive emergencyTrigger from overflowSignal when emergencyOpts is set, mirroring the non-emergency path's trigger computation. - Overflow catch at call site: parse actualTokens from the error and pass an explicit overflow signal to the emergency compact call so the trigger resolves to "overflow".
3 participants
Three independent bugs prevented reliable recovery from provider context-overflow rejections:
isContextOverflowErroronly matched typedContextOverflowErrorinstances. Provider adapters surface overflow as plainErrorobjects (e.g. "prompt is too long: 242201 tokens > 200000"), so the recovery block was bypassed entirely. Switch the overflow-catch guard tolooksLikeContextOverflowError, which matches both by message pattern.When the loop made progress (tool turns completed) before overflow, the existing code skipped compaction entirely and surfaced a hard error. Add an emergency compaction pass (
force: true, minKeepRecentUserTurns: 0) before the graduated reduction ladder; if it frees enough headroom, the loop retries the provider call immediately without touching the ladder.The overflow reduction ladder derived its compaction target from the estimator's budget, which under-counted by 31% in the test scenario. Thread the estimation-error ratio through the overflow signal (
estimatedTokensAtOverflow->targetTokens) so the ladder receives a corrected target computed asfloor(preflightBudget / (actualTokens / estimatedTokens)).Also adds
compacted: booleantoCompactionAttemptso the emergency path can distinguish "compaction actually reduced history" from "compact() returned without summarising" (the latter must not short-circuit tocontinue).Promotes test.todo Tests 1, 3, and 5 in
conversation-agent-loop-overflow.test.tsto active tests; all 11 pass.Prompt / plan
Test plan
CLI verb checklist