Skip to content

Send an output cap, and shrink it when the completion reserve overflows the window - #36

Open
solvyxtech wants to merge 2 commits into
claude/wire-argsfrom
claude/completion-reserve
Open

solvyxtech wants to merge 2 commits into
claude/wire-argsfrom
claude/completion-reserve

Conversation

@solvyxtech

Copy link
Copy Markdown
Owner

Builds on #31: the stack is linear, so the #24–#31 commits show here until they merge. Only the top commit is new.

The bug

GMI Cloud serves qwen/qwen3-235b-a22b-2507 with a 131,072-token total window. The OpenAI-shaped request carried no max_tokens, so the router asked for the model's whole listed output (131,072). Even a 2,454-token prompt was refused:

Requested token count exceeds the model's maximum context length of 131072 tokens. You requested a total of 133526 tokens: 2454 tokens from the input messages and 131072 tokens for the completion.

contextOverflow() then shed history, which can never fix a completion-reserve overflow. The run ended "too large and nothing left to shed" in 11 of 34 Qwen runs.

The fix

  • completionOverflow() reads the window, the input count and the completion count from this wording and from the older OpenAI/vLLM one ("(A in the messages, B in the completion)"). When lowering the completion alone is enough (window − input − margin ≥ 1024, margin = max(256, 1% of the window)), max_tokens is lowered to that and the same request is sent again, with no shed. The lowered cap is kept for the session and reset on a model change.
  • If the prompt itself is the problem (lowering the completion leaves less than 1024 tokens), the shed path runs as before.
  • The OpenAI-shaped request now always sends max_tokens. The default is the existing DEFAULT_MAX_TOKENS (32,768), not the model's listed maximum. It doubles, up to 131,072, only when a reply is actually cut off at it. --max-tokens still wins.
  • A provider that refuses the field outright gets one retry without it, and then requests without it for the session. The trigger is narrow: the error must name max_tokens and say it is unsupported. OpenAI's max_completion_tokens models are the case.
  • The output-ceiling retry (outputCeiling) now runs on both request shapes, and also reads OpenAI's wording ("supports at most N completion tokens") and DeepSeek's ("valid range of max_tokens is [1, N]").
  • contextOverflow reads "maximum context length of W" too. It now takes the input count, not the total, as what the server counted, which is the number Maat's estimate is compared with.

Tests

test/completion-reserve.test.ts:

  • The exact GMI body. The retry asks for a smaller max_tokens (≤ 131072 − 2454 − 256) and resends the same history. Nothing is shed, and the turn answers. The next turn uses the lowered cap with no second refusal.
  • A real history overflow (GMI wording, 130,500 input tokens). It still sheds, and max_tokens is unchanged.
  • The default cap. It is 32,768, and a reply cut off at it raises the next request's cap to 65,536.
  • A provider that refuses max_tokens. Its requests go out without the field.
  • Parser unit tests for both wordings and the new ceiling wordings.

npm run check is green: 2024 tests, e2e and self-check, with ~/.local/bin off PATH.

…ws the window

GMI Cloud serves qwen/qwen3-235b-a22b-2507 with a 131072-token total
window. The OpenAI-shaped request carried no max_tokens, so the router
asked for the model's whole listed output (131072), and even a
2,454-token prompt was refused. contextOverflow then shed history, which
cannot fix a completion overflow, and 11 of 34 Qwen runs ended "too large
and nothing left to shed".

- completionOverflow reads the window, input and completion counts from
  the vLLM/OpenAI wordings ("You requested a total of X tokens: A tokens
  from the input messages and B tokens for the completion", and "(A in
  the messages, B in the completion)"). When lowering the completion
  alone fits (window - input - margin >= 1024), max_tokens is lowered to
  that and the same request is sent again, with no shed; the cap is kept
  for the session. A prompt that is itself too big still sheds.
- The OpenAI-shaped request now always sends max_tokens: the default is
  DEFAULT_MAX_TOKENS (32768), doubled up to 131072 only when a reply is
  cut off at it; --max-tokens still wins.
- A provider that refuses the field itself (max_completion_tokens
  models) gets requests without it after one retry. The output-ceiling
  retry now runs on both request shapes and reads OpenAI's and
  DeepSeek's wordings too.
- contextOverflow reads "maximum context length of W", and takes the
  input count, not the total, as what the server counted.

@solvyxtech solvyxtech left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grok review: changes requested (posted as a comment because GitHub won't let the shared account request changes on its own PR). Two small fixes, then this is good to go.

Reviewed only the new commit a48570ddb (everything under it is #24–#31, reviewed in their own PRs). Pairing rules are clean: no subscription backends, no competitor names in product copy (the new user-facing info line names no product), nothing touching fine-tune data, and the base is claude/sync-local-work, not main. CI (typecheck-and-test) was still pending when I looked.

The diagnosis and the fix are right: a completion-reserve overflow can't be fixed by shedding, so refitting max_tokens and resending the same history is the correct move, and keeping the prompt-too-big case on the shed path is the right split. Taking the input count as sent in contextOverflow is also a real fix, since that's the number Maat's estimate is compared with.

Please fix

  1. The truncation note and nudge report the doubled cap, not the cap the reply hit. In the finishReason === "length" branch, outputCap is doubled before the log.append note, the [molt: that reply hit the output ceiling of ${this.maxTokensFor()} tokens…] transcript nudge and the info line are built. So on the first cut-off at 32,768 all three say 65,536. The nudge goes to the model, and the note goes into the run log Maat's receipts rely on, so it should be accurate. Capture const hit = this.maxTokensFor() before doubling and use hit in those three strings (and maybe add "next request asks for N" to the note). Worth a one-line assertion in the "doubles only when a reply hits it" test.

  2. refusedMaxTokens is broader than its doc comment says. The comment says it must name the field and say it's unsupported, but the last alternative, |max_completion_tokens, matches any body that mentions both fields, with no "unsupported" wording needed. (It also makes use 'max_completion_tokens' redundant.) A value error like "max_tokens / max_completion_tokens must be ≤ N" that slips past outputCeiling would drop the cap for the session. The one-retry-then-revert guard limits the damage, but I'd drop the bare alternative so the code matches the comment.

Suggestions (non-blocking)

  • Always sending max_tokens changes behavior for every OpenAI-shaped endpoint, not only GMI. Servers that count the reserve against the window but word the refusal differently will now fall through to the shed path, which is the exact failure this PR fixes. Hugging Face TGI-style servers are the one I'd check (their refusal reads roughly "inputs tokens + max_new_tokens must be <= N"). Either add that wording to completionOverflow, or at least make sure a refusal that names max_tokens/max_new_tokens never goes to shed.
  • outputCap isn't reset in setModel, while modelMaxTokens, fittedCompletion and maxTokensUnsupported are. A cap doubled to 131,072 on one model carries over to the next. That's self-correcting through the refusal paths, but it costs a refused request, and resetting it to DEFAULT_MAX_TOKENS on a model change would match the comment above it.
  • Reasoning models that refuse max_tokens end up uncapped, which is the pre-PR behavior. Sending max_completion_tokens: this.maxTokensFor() on the retry would keep a cap there too. Fine as a follow-up.
  • Tests: the refused-field test could also assert that the second turn doesn't pay the refusal again, and that a retry-without-field that also fails puts max_tokens back (the revert path has no test).
  • Minor: the two-pass refit loop doesn't check the abort signal between sends. That's harmless at two iterations; I'm only noting it.

Not merging from here. Once 1 and 2 are in and CI is green, this is an approve from me.

@solvyxtech
solvyxtech changed the base branch from claude/sync-local-work to claude/wire-args October 7, 2026 23:28

@solvyxtech solvyxtech left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: CHANGES REQUESTED

(Posted as a comment review: GitHub won't let this shared account request changes on its own PR.)

Fresh review at head a48570d. This is the same commit my 22:37 UTC review looked at, so neither of the two items from that review has been fixed. I re-read the whole commit and probed it with mock providers built on the test/completion-reserve.test.ts harness.

Checks run: typecheck is clean. The full unit suite passes 2024/2024 on Linux with node 20.19. The runner still exits 1, but only because node's coverage reporter crashes; that is unrelated to this PR.

Pairing terms: clean. Vendor names appear only in code comments and regexes, there are no subscription backends, and fine-tune data is untouched.

Blocking

1. Always sending max_tokens: 32768 breaks endpoints whose refusal wording isn't one of the two parsed ones. src/engine.ts:6157 and src/engine.ts:356 (completionOverflow). High confidence: reproduced with mocks.

  • OpenRouter. When a request doesn't fit, OpenRouter answers "This endpoint's maximum context length is 32768 tokens. However, you requested about 34668 tokens (1000 of text input, 900 of tool input, 32768 in the output)…". completionOverflow returns null, refusedMaxTokens returns false, and contextOverflow sheds with sent: 0. On turn 1 the run then ends with "this endpoint serves 32768 tokens of context and there is nothing left to shed … Restart the server with a larger context". In the mock, the first request was refused and the turn never answered.
  • Current vLLM (wording since vllm#36197). It answers "This model's maximum context length is 32768 tokens. However, you requested 32768 output tokens and your prompt contains 1000 input tokens, for a total of 33768 tokens…". That hits the same dead end on turn 1. It is worse, because contextOverflow's /you requested (?:a total of )?(\d+)/ reads the output count as sent, so tokenScale jumps to about 8x and every later decision about what to shed is skewed.
  • Why it matters: before this PR no max_tokens was sent, and both servers fit the output themselves. OpenRouter's documented maximum is the context minus the prompt, and vLLM defaults to max_model_len - input. So this is a regression for every OpenRouter model with a context of about 32k–64k (the bench routes through OpenRouter) and for self-hosted vLLM at the common --max-model-len 32768. Both fail on the first request.
  • Fix:
    • Teach completionOverflow these wordings: (\d+) of text input, an optional (\d+) of tool input, (\d+) in the output, and you requested (\d+) output tokens and your prompt contains (\d+) input tokens.
    • Make contextOverflow stop reading an output count as sent.
    • As a fallback: when a 400 is about the context window and the max_tokens we sent was our default (not --max-tokens), retry once without max_tokens before shedding.
    • Add mocks for all three wordings.

2. The vLLM 0.8–0.16 wording only works by accident, through the over-broad refusedMaxTokens. src/engine.ts:373. High confidence: reproduced.

  • What happens: vLLM 0.8–0.16 answers "'max_tokens' or 'max_completion_tokens' is too large: 32768. This model's maximum context length is 32768 tokens and your request has 1000 input tokens (32768 > 32768 - 1000)." completionOverflow misses it. The bare |max_completion_tokens alternative matches, so the session drops max_tokens entirely and journals "provider refused max_tokens", which is false. The session also runs uncapped from then on.
  • Correction to my earlier advice: in my last review I asked for that alternative to be removed. Doing that alone would turn this case into the turn-1 failure from item 1. Please add this wording to completionOverflow first, then narrow refusedMaxTokens to what its comment says it matches.

3. The cut-off note and the model nudge still report the doubled cap. src/engine.ts:7197, 7200 and 7207. High confidence: reproduced.

  • What happens: outputCap is doubled before the note, the info line and the [molt: that reply hit the output ceiling of N tokens…] nudge are built. With a reply cut at 32,768, the mock printed "cut off at the 65536-token output ceiling", and the transcript carries "output ceiling of 65536".
  • Why it matters: the model is told the wrong number, and so are the receipts that read the journal.
  • Fix: capture const hit = this.maxTokensFor() before doubling and use it in all three strings.

Non-blocking

  • The fitted cap is sticky after the prompt shrinks. src/engine.ts:2046 and 6323. Medium confidence. Say the fit is taken near a full window: 125k input on a 131k window gives 4,761. After a shed or compaction the prompt shrinks, but the cap never comes back up until the model changes. Because maxTokensFor() !== outputCap, the doubling path never fires either. A large write_file reply then truncates on every try. The comment "the prompt only grows" isn't true once shedding exists. Store the window and recompute from the current estimate, or clear fittedCompletion after a shed.
  • outputCap is not reset in setModel. src/engine.ts:2358. Medium-high confidence. This was raised before and is still open. A cap of 131,072 carried over to a model whose limit is lower costs a refused request.
  • One extra refused round trip every time the prompt grows about 1% past the fit. Low confidence. The fit is computed against the old input count, so each turn that grows past the 1% margin pays a refusal and a resend. Computing the fit from the window and the current estimate would avoid it.
  • Test gaps: the refused-field revert path has no test, and neither do the three wordings above.

Nothing merged.

…hedding

Sending max_tokens on every request broke endpoints whose refusal wording
was not one of the two parsed ones: OpenRouter ("1000 of text input, 900 of
tool input, 32768 in the output") and current vLLM ("you requested 32768
output tokens and your prompt contains 1000 input tokens") fell through to
the shed path and ended turn 1 with nothing to shed.

- completionOverflow reads OpenRouter, current vLLM, vLLM 0.8-0.16
  ("'max_tokens' or 'max_completion_tokens' is too large: N ... your request
  has M input tokens"), OpenAI (including "N in the functions"), Anthropic
  ("input length and `max_tokens` exceed context limit: A + B > W") and TGI.
  llama.cpp never refuses for the reserve, so its refusal stays a prompt
  overflow.
- contextOverflow takes the prompt count from those wordings and never reads
  an output count as what was sent (current vLLM scaled every estimate 8x);
  it also reads Anthropic's "prompt is too long: N tokens > W maximum".
- A window refusal in wording none of these read, from a request with a cap
  above 8192, gets one request with a smaller cap (fitted from the window
  and the estimate when the server named a window) before shedding; kept
  only if it works, and dropped if a reply is later cut off at it. A prompt
  known to be too big goes straight to the shed path.
- refusedMaxTokens no longer matches a body that merely names
  max_completion_tokens; it needs the field refused.
- The cut-off note, the nudge and the info line report the cap the reply
  hit, not the doubled one; the note says what the next request asks for.
- A model change resets the doubled output cap; a shed clears the fitted
  cap, since the smaller prompt leaves room for a larger reply.
- Tests for each wording, end to end on turn 1, the llama.cpp shed path,
  the fallback and its revert, and the refused-field revert.
@solvyxtech

Copy link
Copy Markdown
Owner Author

Addressed in 34c91ca (new commit on top of a48570d; nothing under it changed). npm run check green with ~/.local/bin off PATH: 2043/2043 unit tests, e2e PASS, self-drive PASS, self-check PASS.

Blocking

  1. Unparsed refusal wordings / always sending max_tokens. Fixed.
    • completionOverflow now reads OpenRouter (N of text input, N of tool input (every of … input part is summed), N in the output), current vLLM (you requested N output tokens and your prompt contains M input tokens), vLLM 0.8–0.16 ('max_tokens' or 'max_completion_tokens' is too large: N … your request has M input tokens), OpenAI (with or without N in the functions), Anthropic (input length and \max_tokens` exceed context limit: A + B > W) and TGI (must be <= W. Given: A `inputs` tokens and B `max_new_tokens``). The GMI wording still works.
    • llama.cpp never refuses for the reserve (it stops the reply when the window fills), so its n_ctx/n_prompt_tokens refusal stays a prompt overflow and sheds. There's a test for that.
    • contextOverflow takes the prompt count from those wordings first and never reads an output count as sent (the generic you requested N no longer matches N output). It also reads Anthropic's prompt is too long: N tokens > W maximum.
    • Fallback: if a 400 is about the window in a wording none of these parse, and the cap sent was above 8192, Maat tries one request with a smaller cap before shedding. When the server named a window, the cap is fitted from that window and the scaled estimate; otherwise it is 8192. The smaller cap is kept only if that request succeeds. It is dropped if a later reply is cut off at it, and restored if the retry is refused too. If the server said the prompt alone doesn't fit, the request goes straight to the shed path. I chose a smaller cap over sending no max_tokens because dropping the field brings back the GMI failure this PR fixes.
  2. vLLM 0.8–0.16 only working by accident. That wording is now read by completionOverflow, which runs before refusedMaxTokens. refusedMaxTokens has lost the bare |max_completion_tokens alternative, so it matches only what its comment says. Tests: that body is refusedMaxTokens === false, and max_tokens / max_completion_tokens must be <= 4096 is false too.
  3. Note and nudge reported the doubled cap. Fixed. const hit = this.maxTokensFor() is captured before doubling, and the note, the transcript nudge and the info line all use hit. The note also says the next request asks for N when the cap changes. The doubling test now asserts output ceiling of 32768 tokens in the nudge and the info line, and asserts that 65536 appears in neither.

Non-blocking, also done

  • setModel resets outputCap to DEFAULT_MAX_TOKENS, and the doubling test switches model and asserts 32768.
  • Sticky fitted cap: shed() clears fittedCompletion, so a smaller prompt gets a larger reply again. If the reserve is still too big, the next refusal fits it again.
  • Tests: the refused-field test now asserts that the second turn doesn't pay the refusal again. There's a new test for the revert path: when the retry without the field also fails, max_tokens is back on the next request.

Tests added (test/completion-reserve.test.ts, 26 in the file): a unit test per wording (window/input/completion, contextOverflow.sent = prompt, not a refused field, fit in range). Turn-1 end-to-end mocks for OpenRouter, current vLLM, vLLM 0.8–0.16, OpenAI, Anthropic wording and an unknown wording each check four things: the turn answers, nothing is shed, the second request has a smaller cap with the same history, and the next turn pays no refusal. Also covered: llama.cpp sheds with the cap unchanged, and the fallback is forgotten when it is refused too.

Not done (follow-up): sending max_completion_tokens on the retry for reasoning models that refuse max_tokens. They still run uncapped, which is the pre-PR behavior. The refit loop still doesn't check the abort signal between its two sends.

The stacked branches (#32, #33, #34) merge cleanly onto the new head (git merge-tree). They haven't been rebased yet.

@solvyxtech

Copy link
Copy Markdown
Owner Author

Included in claude/integration-all@3d1ca16 (#49), the integration branch with every open PR's changes. That branch is now the standard build and supersedes this one. The conflict choices are listed in #49.

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