Skip to content

fix(compaction): auto-continue after OpenAI Responses output-budget underflow - #1690

Merged
lavaman131 merged 4 commits into
mainfrom
fix/auto-compaction-output-budget-continue
Jul 9, 2026
Merged

fix(compaction): auto-continue after OpenAI Responses output-budget underflow#1690
lavaman131 merged 4 commits into
mainfrom
fix/auto-compaction-output-budget-continue

Conversation

@lavaman131

@lavaman131 lavaman131 commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes Atomic auto-compaction so a session continues automatically after an OpenAI Responses output-budget underflow error (Invalid 'max_output_tokens': integer below minimum value. Expected a value >= 16, but got 1 instead.), instead of compacting and then sitting idle until the user manually types Continue.

Root cause

_checkCompaction() runs after every assistant turn. Threshold compaction could fire for an assistant that ended with stopReason: "error", but the retry-worthiness check (shouldRetryAfterThresholdCompaction) only recognized stopReason: "length" turns. The underflow error therefore compacted with willRetry: false and left no automatic continuation path, so the session appeared stuck right after Auto-compacting....

Transcript evidence

A user-provided session transcript showed the manual-continuation pattern repeating at lines 387-390, 694-697, 886-889, and 1085-1088: an assistant stopReason:"error" (invalid max_output_tokens, got 1), followed by context_compaction, followed by a manual continue from the user, followed by the assistant resuming successfully.

Changes

  • agent-session-auto-compaction.ts: Split the retry check into isRetryWorthyLengthStop (the existing stopReason:"length" case) and a new isRetryWorthyOutputBudgetError, which matches stopReason:"error" turns on the openai-responses API whose error message names an output-token budget parameter (max_output_tokens) and underflow/minimum-token wording (e.g. >= 16, got 1 instead), parsing structured JSON error bodies where present. shouldRetryAfterThresholdCompaction now returns true for either case, so threshold compaction passes willRetry: true for this specific error family, drops the trailing error assistant, and schedules automatic continuation. Generic invalid_request_body errors (e.g. malformed tool schemas) remain non-retryable.
  • Bounded recovery: Added MAX_OUTPUT_BUDGET_ERROR_CONTINUATION_ATTEMPTS = 1 and a new _outputBudgetErrorContinuationAttempts counter (reset once an assistant turn completes without failing) so output-budget-triggered compact-and-retry only auto-continues once per failure streak. If the same underflow error recurs after that single retry, _checkCompaction now emits a compaction_end event with willRetry: false and an explanatory errorMessage instead of looping indefinitely.
  • Renamed overflow-specific post-compaction continuation tracking (_pendingOverflowPostCompactionContinuation, _overflowPostCompactionContinuationToken, _awaitPendingOverflowPostCompactionContinuation) to generic post-compaction names, since the continuation path is no longer overflow-only. Updated call sites in agent-session-methods.ts, agent-session-prompt.ts, and agent-session.ts.
  • openai-responses-payload-sanitizer.ts: Hardened payload sanitization so a finite numeric max_output_tokens below the provider minimum (MIN_RESPONSES_MAX_OUTPUT_TOKENS = 16) is clamped up to 16 before the request is sent, preventing the underflow error at its source for future requests.
  • Docs: Updated docs/compaction.md and docs/json.md to describe the new output-budget-underflow retry path and clarify that compaction_end.willRetry now also covers this case.
  • Changelog: Added an entry under packages/coding-agent/CHANGELOG.md [Unreleased] > Fixed.
  • Tests: Extended agent-session-auto-compaction-queue-03.suite.ts and openai-responses-payload-sanitizer.test.ts to cover the new retry classification, the attempt-bounding/recovery-exhaustion path, and the max_output_tokens clamping. Also touched first-run-onboarding.test.ts and interactive-mode-clone-command.test.ts for the rename of the post-compaction continuation fields.

Validation

  • Targeted transcript inspection confirmed the manual Continue pattern at 387-390, 694-697, 886-889, and 1085-1088.
  • bun run test -- test/agent-session-auto-compaction-queue-03.suite.ts test/openai-responses-payload-sanitizer.test.ts — passed.
  • bun run typecheck — passed.
  • bun run check:file-length — passed.
  • bun packages/coding-agent/src/cli.ts --help | head -40 — passed.
  • git diff --check main — clean.
  • Pre-commit hooks during commit/push — passed (used BUN_OPTIONS=--timeout=10000 to avoid an unrelated Windows full-suite child-process timeout flake while still running hooks).

Notes

No version bumps; main remains versionless as required.

Treat retry-worthy max_output_tokens/max_tokens underflow errors as threshold compaction continuations, drop the trailing error assistant before rebuilding retry context, and track post-compaction continuations generically. Also clamp OpenAI Responses max_output_tokens to the provider minimum.

Assistant-model: GPT-5.5
@mintlify

mintlify Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
bastani 🟢 Ready View Preview Jul 9, 2026, 8:57 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@claude claude Bot changed the title fix(compaction): continue after output budget underflow fix(compaction): continue automatically after OpenAI Responses output-budget underflow Jul 9, 2026
@claude

claude Bot commented Jul 9, 2026

Copy link
Copy Markdown

Code Review — PR #1690: continue after OpenAI Responses output-budget underflow

Thanks for the detailed writeup and transcript evidence — the root-cause analysis is clear and the two-layer fix (classify the error as retry-worthy and clamp max_output_tokens at the source) is the right shape. Overall this is clean, well-tested, and docs/changelog are properly updated. A few observations, mostly minor.

Strengths

  • Defense in depth: fixing the source (openai-responses-payload-sanitizer.ts clamp) and the recovery path (retry classification) is the correct approach — the clamp prevents recurrence while the classifier heals sessions where the error already landed.
  • The rename from overflow* -> generic postCompaction* naming is consistent, and I confirmed no stale references remain across the packages.
  • _reason is correctly underscore-prefixed to satisfy noUnusedParameters while keeping the interface signature stable.
  • Good, targeted test coverage: the reported error shape, the generic invalid_request_body negative case, and the input-absent sanitizer path are all exercised.

Observations / discussion points

  1. Behavioral broadening of the continuation await (worth confirming it's intended). _schedulePostAutoCompactionContinuationProbe now routes all willRetry cases through the token-tracked _pendingPostCompactionContinuation path, and _runAgentPrompt awaits it via _awaitPendingPostCompactionContinuation. Previously the threshold+willRetry length-truncation continuation was fire-and-forget (void this._resumeAfterAutoCompaction()) and was not awaited by prompt(). This is arguably more correct — prompt() now resolves only after the length-truncation continuation completes — but it changes prompt() resolution/latency semantics for that pre-existing path too, not just the new error path. If deliberate, great; flagging it since the PR description frames the change as error-specific.

  2. Output-budget errors below the compaction threshold are not auto-continued. At agent-session-auto-compaction.ts:131 only isRetryWorthyLengthStop triggers a direct resume; a stopReason:\"error\" output-budget underflow that lands below threshold falls through with no continuation. This is intended per the description, and the source-side clamp makes a sub-threshold underflow unlikely — but the asymmetry with the length-stop path is worth keeping in mind if the clamp ever misses a code path that sets max_output_tokens.

  3. Regex anchors on provider-specific wording. OUTPUT_BUDGET_UNDERFLOW_PATTERN includes got 1 instead and >= 16 (the latter interpolated from the min constant). Those alternatives are brittle to wording/value changes, but integer below minimum value is a robust primary anchor covering the general case, so this is fine in practice. Minor: got 1 instead only matches the literal value 1 — if the provider ever reports got 3 instead, the primary anchor still catches it, so no functional gap, just dead-ish specificity.

  4. Clamp floor interaction. When the computed budget is genuinely tiny (context near cap), clamping max_output_tokens up to 16 lets the request through but may yield a near-empty length-stopped response -> resume. That's bounded by MAX_LENGTH_CONTINUATION_ATTEMPTS, so no infinite loop, but recovery in that corner is a few tiny turns rather than an immediate compact. Acceptable given the alternative was a hard error.

Nits

  • test/openai-responses-payload-sanitizer.test.ts:72-73 has a double blank line before the first new test, and around line 97-98 there is no blank line before does not change non-Responses payloads (the new block runs directly into the existing test). Purely cosmetic.

Security / performance

  • No security concerns — this is input clamping and error classification, no new external surface. The added regexes run once per assistant turn on error, so no perf impact.

Nice fix. The behavioral-await broadening in point 1 is the only thing I'd explicitly confirm is intentional before merge.

@claude

claude Bot commented Jul 9, 2026

Copy link
Copy Markdown

Code Review — fix(compaction): continue automatically after OpenAI Responses output-budget underflow

Thanks for the detailed writeup and transcript evidence — the root-cause analysis is clear and the fix is well-scoped. Overall this is solid, well-tested defensive work. A few things worth considering before merge.

🔴 Potential unbounded compact→retry loop on the new error path

The overflow path is guarded by _overflowRecoveryAttempted (one attempt), and the length-stop path is bounded by MAX_LENGTH_CONTINUATION_ATTEMPTS. The new output-budget-error retry path has no equivalent bound. In _checkCompaction, a stopReason:"error" output-budget underflow that still crosses the threshold triggers _runAutoCompaction("threshold", true)_resumeAfterAutoCompaction() → continue. If compaction can't actually free enough room (e.g. protected/pinned entries dominate the context), the continuation re-issues, underflows again, and re-enters the same branch.

Two failure shapes are possible depending on where the usage anchor lands:

  • Loop: compact → continue → underflow → compact → … indefinitely.
  • Silent stall: on the second occurrence estimateContextTokens may resolve lastUsageIndex to a message at/before the fresh compaction boundary (lines 112–119), causing an early return with no compaction and no length-resume — the session goes idle again, which is the exact symptom this PR set out to fix.

Consider a small consecutive-attempt counter for this family (mirroring _overflowRecoveryAttempted / MAX_LENGTH_CONTINUATION_ATTEMPTS), reset on a successful non-error turn, so a context that genuinely can't be compacted below the budget terminates with a visible error rather than looping or stalling.

🟡 Clamp-to-16 interaction & coupling

  • sanitizeOpenAIResponsesPayload clamping max_output_tokens up to 16 prevents the hard error at source — good belt-and-suspenders. Worth confirming the follow-on behavior is benign: a clamped request yields a ~16-token length-stopped turn, which then routes through the length-resume/threshold path. That's bounded, but it does mean a near-full context can burn a tiny extra turn before compacting. Acceptable, just flagging the emergent flow.
  • OUTPUT_BUDGET_UNDERFLOW_PATTERN bakes the literal >= 16 from MIN_RESPONSES_MAX_OUTPUT_TOKENS into the message regex. Nicely DRY via the template literal, but it couples "the provider's advertised minimum in the error text" to "our clamp constant." If the provider ever reports a different minimum, that alternative silently stops matching (the integer below minimum value / got 1 instead alternatives still cover the common case, so low-risk).

🟢 Nits

  • JsonValue / isJsonRecord / stringField in agent-session-auto-compaction.ts duplicate JSON typing already present in openai-responses-payload-sanitizer.ts. Minor, but a shared tiny json-util would avoid drift.
  • openai-responses-payload-sanitizer.test.ts: the new block adds a double blank line at the top and the following does not change non-Responses payloads test lost its leading blank line — cosmetic.

👍 Strengths

  • The structured-JSON parse (parseProviderErrorDetails) with a text-scan fallback, plus the api === "openai-responses" narrowing, makes the classifier appropriately conservative — the generic-invalid_request_body and non-Responses negative tests confirm malformed-tool-schema errors stay non-retryable.
  • Renaming the overflow-specific continuation tracking to generic names is the right call now that the awaited pending-promise path serves threshold retries too.
  • Good coverage of both the flat and nested {"error":{…}} shapes, and hoisting estimateContextTokens into the mock so it can be re-stubbed per test is a clean improvement.
  • Docs + changelog updates are thorough and accurate.

Nice work — the loop/stall guard is the one item I'd want addressed (or explicitly reasoned about) before merge; the rest are optional.

Reviewed against CLAUDE.md conventions (Bun, versionless main, 500-line gate — all satisfied).

@claude

claude Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review — fix(compaction): continue after OpenAI Responses output-budget underflow

Reviewed the full diff against the CLAUDE.md conventions. This is a well-scoped, well-tested fix. The root-cause analysis (a stopReason: "error" output-budget underflow slipping past shouldRetryAfterThresholdCompaction, which only recognized "length") is convincing, and the classification logic is nicely defensive. A few things worth confirming before merge.

Strengths

  • Narrow, layered matching. Gating on api === "openai-responses" + requiring both the max_output_tokens parameter token and underflow wording (structured JSON first, raw-text fallback) keeps this from catching generic invalid_request_body failures. The explicit malformed-tool-schema-stays-non-retryable test locks that in.
  • Robust regex. Including the generic integer below minimum value alternative alongside the literal >= 16 / got 1 instead means it will not silently stop matching if the provider reports a different observed value or minimum.
  • Bounded retry. MAX_OUTPUT_BUDGET_ERROR_CONTINUATION_ATTEMPTS = 1 with the counter reset in agent-session-events.ts on a non-failed assistant mirrors the existing _overflowRecoveryAttempted pattern — consistent and prevents loops.
  • Sanitizer clamp is a sensible source-level guard; the refactor to sanitize max_output_tokens even when payload.input is absent (previously an early return) is correct, and the changed-tracking preserves the no-op identity return.

Points to confirm

  1. Un-flagged behavior change to the existing length-truncation path. The _schedulePostAutoCompactionContinuationProbe refactor now routes all willRetry continuations (overflow and threshold) through the tracked _pendingPostCompactionContinuation promise that prompt() awaits via _awaitPendingPostCompactionContinuation. Previously the threshold length-stop case took the fire-and-forget void this._resumeAfterAutoCompaction() branch that prompt() did NOT await. Net effect: prompt() now blocks until a length-stop threshold continuation finishes, where before it could resolve earlier. Arguably more correct/consistent, but it changes resolution timing for the already-shipped length-truncation feature and is not called out in the description. Worth a changelog note and a check that no SDK caller relied on the earlier resolve point.
  2. Cap-exceeded branch skips compaction entirely. When _outputBudgetErrorContinuationAttempts >= 1, _checkCompaction emits compaction_end (willRetry:false) and returns without calling _runAutoCompaction, even though shouldCompact was true — so the over-threshold context is left uncompacted. This matches the overflow-exhaustion precedent (lines ~78-90), so it is consistent, but confirm that leaving the context un-shrunk (rather than compacting once more without retry) is the intended terminal state. Note it also emits compaction_end with no preceding compaction_start; the interactive handler tolerates it (loaders are conditional), again matching the overflow path — just flagging.
  3. Two out-of-scope test-only edits. first-run-onboarding.test.ts and interactive-mode-clone-command.test.ts add ensureDeferredStartupComplete mocks unrelated to compaction. The source already calls this on main, so these look like test-sync fixes rather than part of the fix — harmless, but ideally a separate commit/PR (or a one-line note) so the diff stays focused.

Minor / nits

  • parseProviderErrorDetails slices first brace to last brace; fine for the observed shapes, but if a message ever embeds prose containing braces after the JSON it could mis-slice. The raw-text fallback covers that case, so low risk.
  • Only the overflow path sets unresolvedOverflow: true, so external orchestration that branches on unresolvedOverflow to fall back to another model will not treat this new exhaustion the same way. Confirm that asymmetry is intended.

Verification

  • Static review only — I could not execute the suite in this environment (command-approval gate). The PR reports the two targeted test files green (18 tests) plus typecheck and check:file-length passing. Coverage of the new classification (reported shape, structured shape, non-Responses API, generic invalid-request, cap exhaustion, auto-continue) is solid and reads well.

Overall: looks good to merge once (1) is documented and (2) / the unresolvedOverflow asymmetry are confirmed intentional.

Automated review — generated with Claude Code.

@claude claude Bot changed the title fix(compaction): continue automatically after OpenAI Responses output-budget underflow fix(compaction): auto-continue after OpenAI Responses output-budget underflow Jul 9, 2026
@claude

claude Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review: fix(compaction) — continue after OpenAI Responses output-budget underflow

Nice, well-scoped fix with clear root-cause analysis. The defense-in-depth approach (clamp at the source in the sanitizer and classify the error as retry-worthy in auto-compaction) is the right call, the renames are consistent, and the test coverage is genuinely thorough — structured vs. unstructured error bodies, the exhaustion path, the non-openai-responses exclusion, and the generic-invalid_request_body exclusion are all exercised. Changelog and docs (compaction.md, json.md) are comprehensive. I verified the sanitizer is wired in as the last step of the before_provider_request payload chain (sdk.ts:414), which is the correct placement so it also covers extension-injected payloads.

A few observations, all minor / non-blocking:

1. Scope is openai-responses-only for both halves — confirm this is intended.
Both isRetryWorthyOutputBudgetError (requires api === "openai-responses") and the sanitizer clamp (isOpenAIResponsesModel) are Responses-scoped. That is internally consistent, but if the same max_output_tokens minimum-underflow ever surfaces on openai-completions (which also enforces a minimum), neither the source clamp nor the retry path will catch it. Deliberate narrowing to avoid over-matching is defensible — just flagging so it is a conscious decision.

2. Two unrelated test files in the diff.
test/first-run-onboarding.test.ts and test/interactive-mode-clone-command.test.ts only add an ensureDeferredStartupComplete mock and have nothing to do with output-budget recovery. Looks like a rebase/merge artifact — worth confirming they belong in this PR (and that there is a corresponding production change they are covering, since interactive-mode.ts is not in the changeset).

3. isRetryWorthyOutputBudgetError is evaluated twice per errored turn — once inside shouldRetryAfterThresholdCompaction() and again in the if (willRetry && isRetryWorthyOutputBudgetError(...)) guard (agent-session-auto-compaction.ts:131-132). Each call re-runs JSON.parse + regexes. The cost is trivial (once per turn), but you could compute the boolean once and reuse it for slightly cleaner control flow.

4. Attempt counter increments before the compaction actually runs.
In the threshold branch, _outputBudgetErrorContinuationAttempts += 1 happens at classification time (:145), then _runAutoCompaction is called. If that compaction itself fails (!result -> emits willRetry:false), the single retry attempt has still been "consumed", so a subsequent identical underflow will not get its retry. Edge case (compaction failure is rare), low impact — just noting the ordering.

5. Redundant/over-specific regex alternative.
OUTPUT_BUDGET_UNDERFLOW_PATTERN includes got\s+1\s+instead, which is both redundant with the integer\s+below\s+minimum\s+value alternative (both phrases appear in the real error) and hard-coded to the literal value 1. If the provider ever reports got 3 instead, the first alternative already covers it, so the got 1 branch adds little and reads as if 1 were special. Harmless, but could be dropped for clarity.

6. Consistency note (not a defect): the exhaustion branch emits compaction_end (reason: "threshold", willRetry: false) without a preceding compaction_start, and returns without compacting. This mirrors the existing overflow-exhaustion branch (:78-90), so it is consistent with established behavior — any listener assuming paired start/end events should already handle this.

Overall this looks solid and safe to merge once the stray test-file changes (#2) are confirmed intentional.

Automated review by Claude.

@lavaman131
lavaman131 merged commit 7c87ed1 into main Jul 9, 2026
11 checks passed
@lavaman131
lavaman131 deleted the fix/auto-compaction-output-budget-continue branch July 9, 2026 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant