Skip to content

fix(agent): stop collapsing the output cap to 1 token near a full context window - #300

Merged
OmarB97 merged 1 commit into
mainfrom
fix/output-fit-floor-fork-20260801
Aug 2, 2026
Merged

fix(agent): stop collapsing the output cap to 1 token near a full context window#300
OmarB97 merged 1 commit into
mainfrom
fix/output-fit-floor-fork-20260801

Conversation

@OmarB97

@OmarB97 OmarB97 commented Aug 2, 2026

Copy link
Copy Markdown
Owner

What does this PR do?

output_tokens_that_fit() returned its min_output floor (1) when the reserved input filled the window, and both callers consumed that fabricated 1 as a real budget — sending max_tokens=1 to the provider.

The reservation is multiplicative — int(est * 1.2) + 1024 + 512 — so it exhausts the window once the rough estimate passes ~(ctx - 1536) / 1.2, about 82.7% fill. Past that point:

  • the pre-flight clamp in build_api_kwargs set max_tokens=1 on every request, with no provider error involved at all; and
  • the reactive output-cap retry clamped a healthy provider-authoritative cap down to 1 — directly contradicting the branch immediately above it, which deliberately falls back to the provider's number precisely because "the rough local estimate can overshoot the real request size".

Measured against a 200,000-token window:

prompt fill provider available_tokens cap actually sent
80% 35,000 6,455
83% 30,000 1
85% 25,000 1

A cliff, not a degradation — and it applies to any window size, not just this shape.

max_tokens=1 is the worst available failure mode: the provider accepts it and returns a single truncated token, so the turn reports success while handing back output the user cannot use. Nothing fails loudly.

Why this approach: the fix is to report None"no usable cap; leave max_tokens to your own budget logic" — instead of fabricating a floor. Both call sites already guarded None correctly, so neither needed a code change. min_output has no external callers, so the semantic change is fully contained in one function.

Regression introduced in #271. That PR's vLLM / deepseek-v4-flash-w2 convergence property is unaffected — that regime sits at ~44% fill, where the fit stays positive and the reactive retry still converges in one step.

Relationship to #295

#295 refreshed two assertions. One was right, one was not:

  • test_codex_content_filter_incomplete_routes_to_policy_fallbackcorrect, and stands. conversation_loop.py:2174 really does pass the classified FailoverReason into the fallback switch since fix: make provider fallback policy explicit and visible #269; asserting it is strictly stronger.
  • test_output_cap_retry_request_pressure_lower_boundreverted here. That assertion was loosened to max(1, min(expected_cap, local_fit)), which amounts to asserting whatever the code did. The original arithmetic was right; it now passes for the right reason (903, not 1).

Known tradeoff

Above 82.7% fill the pre-flight clamp now no-ops, so those turns eat a provider 400 before the reactive retry corrects it with the provider's authoritative budget. That is strictly better than silently returning a 1-token answer, but it costs a round-trip. Bounding the safety reservation so the clamp degrades gracefully instead of going silent is worth a follow-up; it is a reservation-policy redesign and does not belong in a defect fix.

Related Issue

Regression from #271. Test-side follow-up to #295.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • agent/model_metadata.pyoutput_tokens_that_fit() returns None instead of the min_output floor when the reserved input leaves no positive cap. Docstring now states that None means "no usable cap" (never a licence to shrink), that min_output is the smallest fit worth reporting rather than a fallback value, and documents the ~82.7% fill threshold explicitly.
  • agent/conversation_loop.py — comment only, at the reactive output-cap retry: records why the None case must keep the provider-authoritative safe_out so the floor is not reinstated.
  • tests/test_output_fit_preflight_clamp.py — new TestNearFullWindowNeverYieldsAUselessCap (5 cases) covering the 82.7–100% fill band at both call sites.
  • tests/run_agent/test_run_agent.py — reverts the test half of test(agent): refresh two assertions that predate current retry/fallback behavior #295 for test_output_cap_retry_request_pressure_lower_bound.

How to Test

  1. Reproduce the defect — on main, output_tokens_that_fit(131072, [{"role": "user", "content": "x" * 460000}]) returns 1. That prompt is ~115k tokens in a 131,072 window, leaving ~16k of genuine headroom.
  2. See it reach the wirescripts/run_tests.sh tests/test_output_fit_preflight_clamp.py -q with this PR's tests but main's model_metadata.py fails 4 of them, each on the value 1:
    • test_reports_none_rather_than_a_floorassert 1 is None
    • test_preflight_leaves_cap_alone_instead_of_clamping_to_oneassert 1 is None
    • test_reactive_retry_keeps_provider_authoritative_capassert 1 > 1000
    • test_every_reported_fit_is_a_real_fit → reported fit does not fit
  3. Verify the fixscripts/run_tests.sh tests/test_output_fit_preflight_clamp.py tests/run_agent/test_run_agent.py -q → 14 + 436 pass.

test_every_reported_fit_is_a_real_fit sweeps the whole fill range and holds every non-None answer to the safety property the function documents (a server tokenizing ~15% denser must still leave the request in-window). A fabricated floor fails that by construction, so a reintroduction is caught at the root rather than by pattern-matching the literal 1.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix
  • I've run the suite and all tests pass — see below
  • I've added tests for my changes
  • I've tested on my platform: macOS 15 (Darwin 25.6.0)

Full-suite status: scripts/run_tests.sh reports 34 pre-existing failures on unmodified origin/main in this environment (systemd/WSL paths on macOS, absent Codex credentials, one concurrency flake in test_base_environment.py). This branch reproduces exactly that set — no new failures. tests/agent/test_anthropic_adapter.py fails identically (3) on both refs. The directly affected files are fully green: tests/run_agent/test_run_agent.py 436/436, tests/test_output_fit_preflight_clamp.py 14/14.

Documentation & Housekeeping

  • I've updated relevant documentation (docstrings) — the output_tokens_that_fit contract is the thing that was misread, so it is now explicit about what None means and what min_output is not
  • cli-config.yaml.example — N/A, no config keys
  • CONTRIBUTING.md / AGENTS.md — N/A, no architecture or workflow change
  • Cross-platform impact — N/A, pure arithmetic
  • Tool descriptions/schemas — N/A

🤖 Generated with Claude Code

…text window

output_tokens_that_fit() returned its ``min_output`` floor (1) when the
reserved input filled the window, and both callers consumed that fabricated
1 as a real budget.

The reservation is multiplicative — int(est * 1.2) + 1024 + 512 — so it
exhausts the window once the rough estimate passes ~(ctx - 1536) / 1.2, about
82.7% fill. Past that point:

  * the pre-flight clamp in build_api_kwargs set max_tokens=1 on EVERY request,
    with no provider error involved at all; and
  * the reactive output-cap retry clamped a healthy provider-authoritative cap
    down to 1 — directly contradicting the branch immediately above it, which
    deliberately falls back to the provider's number precisely because "the
    rough local estimate can overshoot the real request size".

Measured against a 200,000-token window: a ~170k-token prompt (85% fill) with
the provider reporting available_tokens=25,000 produced a retry cap of 1
instead of 24,936. At 80% fill the cap is 6,455; at 83% it is 1 — a cliff, not
a degradation.

max_tokens=1 is the worst kind of failure here: the provider accepts it and
returns a single truncated token, so the turn reports success while handing
back output the user cannot use. Nothing fails loudly.

Fix: report None ("no usable cap — leave max_tokens to your own budget logic")
instead of fabricating a floor. Both call sites already guard None correctly,
so neither needed a code change; the reactive one gains a comment so the floor
is not reinstated. Above 82.7% fill the pre-flight clamp now no-ops and the
provider reports its own authoritative budget on the reactive path.

Introduced in #271; the vLLM/deepseek-v4-flash-w2 convergence property that
motivated #271 is unaffected — that regime sits at ~44% fill, where the fit
stays positive and the retry still converges in one step.

Also reverts the test half of #295 for test_output_cap_retry_request_pressure_
lower_bound. That assertion was loosened to match the defect; the original
arithmetic was right and now passes for the right reason. #295's other change
(threading FailoverReason into the fallback switch) is correct and stands.

Tests: 5 cases in tests/test_output_fit_preflight_clamp.py covering the
82.7%-100% fill band at both call sites. The sweep case holds every reported
fit to the documented "server tokenizes ~15% denser" safety property, which a
fabricated floor fails by construction. Verified they fail without the
production change and pass with it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@OmarB97
OmarB97 merged commit 10818f0 into main Aug 2, 2026
33 of 35 checks passed
OmarB97 added a commit that referenced this pull request Aug 2, 2026
…f giving up (#302)

#300 stopped output_tokens_that_fit() from fabricating a max_tokens=1 floor and
made it return None once the reserved input filled the window. That was right,
but it left a gap: above ~82.7% fill the pre-flight clamp in build_api_kwargs
stops firing entirely, so every turn in that regime sends the full configured
max_tokens, eats a provider 400, and only then gets a usable cap from the
reactive retry in conversation_loop. A guaranteed extra round-trip per turn.

The 1.2x factor is an uncertainty allowance for servers that tokenize denser
than the ~4 chars/token heuristic, not a real token cost — but near a full
window that allowance (0.2 * est) exceeds the entire remaining headroom, so
holding it reports "nothing fits" while thousands of output tokens genuinely
do. The documented safety property is looser than the allowance: a server
tokenizing ~15% denser must still leave the clamped request in-window. At
est=166,008 in a 200,000-token window the 1.2x reservation admits nothing while
the 1.15x contract admits ~9,000 tokens.

Fix: reserve in two tiers. The preferred 1.2x + 1024 cushion is kept whenever
the window can pay for it and still leave a usable cap — below ~82.5% fill the
returned value is byte-identical to before, so nothing that works today gets
looser. Past that point the reservation drops to exactly what the safety
contract requires (23/20, kept rational so the ceiling is exact integer
arithmetic) and hands back the rest, tapering from ~9,700 tokens at 82.5% fill
down to the usable floor rather than falling off a cliff. The fixed pad is
dropped in the degraded tier on purpose: it is a rounding cushion for small
prompts, and there the 0.15 * est term is orders of magnitude larger.

None is still the answer past the 1/1.15 ceiling (~87% fill), where the
contract admits nothing usable — swept over every estimate in a 200,000-token
window, the largest contract headroom ever declined is 1,024 tokens, exactly
the usable floor plus the window margin we deliberately hold back.

min_output now defaults to _OUTPUT_FIT_MIN_USABLE (512) rather than 1, so the
"never hand back a cap too small to use" rule is the default rather than
something each caller has to know. Neither call site needed a code change.

The #271 vLLM/deepseek-v4-flash-w2 convergence property is strengthened, not
weakened: in the 82.5%-87% band the local anchor used to be None, so the
reactive retry fell back to whatever the provider's max_tokens-dependent lower
bound reported. It now exists and pulls the retry straight to a fitting cap in
one step from any starting value.

Tests: 7 cases in tests/test_output_fit_preflight_clamp.py covering the
82.7%-100% fill band at both call sites — the reported fit where the preferred
cushion admits none, band coverage without a hole, the no-cliff property
(wherever None is still reported, the contract really does admit nothing
usable, swept in 500-token steps), the no-unusable-cap property, a pin that
low fill is unchanged, the pre-flight clamp firing in the band, and one-step
reactive convergence in the band. Verified 6 of the 7 fail against the previous
formula; the seventh is the low-fill pin, which must pass on both.

Also re-runs green: tests/run_agent/test_run_agent.py,
tests/test_output_cap_parsing.py, tests/test_ctx_halving_fix.py.

Co-authored-by: Omar Baradei <omar@kostudios.io>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@OmarB97
OmarB97 deleted the fix/output-fit-floor-fork-20260801 branch August 2, 2026 13:39
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