Skip to content

fix: clamp chat completion output cap to context window - #58869

Open
ckorhonen wants to merge 1 commit into
NousResearch:mainfrom
ckorhonen:fix/vllm-output-cap-clamp
Open

fix: clamp chat completion output cap to context window#58869
ckorhonen wants to merge 1 commit into
NousResearch:mainfrom
ckorhonen:fix/vllm-output-cap-clamp

Conversation

@ckorhonen

Copy link
Copy Markdown
Contributor

Summary

  • clamp chat-completions max_tokens/max_completion_tokens against estimated input + total context before sending
  • stop treating vLLM's prompt contains at least N input tokens lower-bound wording as an exact available-output budget
  • add regressions for the vLLM lower-bound failure and preflight output-cap clamp

Root cause

Two real sessions (8e394ce78f32 and 1c340ab19f1f) failed on Spark/vLLM with a 131,072-token context window because Hermes requested 65,536 output tokens while the prompt was already around or above the remaining half-window. The recovery parser trusted vLLM's at least N input tokens wording as exact, so it stair-stepped max_tokens down by ~65 tokens per retry (65536 → 65471 → 65406 → 65341) and exhausted recovery instead of compressing or clamping safely.

Validation

  • scripts/run_tests.sh tests/test_ctx_halving_fix.py -q — 31 passed
  • scripts/run_tests.sh tests/agent/test_context_compressor.py -q — 146 passed
  • smoke: exact vLLM lower-bound wording parses as None, Spark/Qwen-shaped request clamps 65536 → 60040
  • git diff --check

Prevent local/vLLM sessions from exhausting compression recovery when input plus the default output cap exceeds the model context window.\n\nThe vLLM lower-bound error wording is no longer treated as an exact prompt token count, and chat-completions requests now clamp outgoing max_tokens against the estimated prompt budget before sending.\n\nValidation:\n- scripts/run_tests.sh tests/test_ctx_halving_fix.py -q\n- scripts/run_tests.sh tests/agent/test_context_compressor.py -q\n- git diff --check
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels Jul 5, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Approved

Clamp chat completion output cap to context window. 151 additions, 2 deletions — ensures output length never exceeds the model's context window limit. Safety fix that prevents truncation or overflow errors.


Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing the vLLM lower-bound wording; current main does treat that lower bound as exact in agent/model_metadata.py:1363-1369.

Problems

  • The parser change conflicts with the existing regression in tests/test_output_cap_parsing.py:135-146, which uses the same at least 65537 input tokens wording and currently requires 65535. This PR does not update that assertion.
  • tests/run_agent/test_run_agent.py:5768-5816 also requires a successful max-token retry for that lower-bound message with compression disabled. After the proposed None result, agent/conversation_loop.py:3105-3113 routes that fixture to the compression-disabled overflow path instead, so the existing two-call assertion no longer matches the behavior.

Suggested changes

  • Consolidate the parser expectation in tests/test_output_cap_parsing.py and update the recovery test to state the intended lower-bound contract.
  • Add an end-to-end recovery test covering preflight clamping when estimable and compression fallback otherwise.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 15, 2026

@GottZ GottZ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This was generated by AI during triage.

Summary

Four PRs address related context-window overflow paths: #14858 guards untrusted context-tier shrinkage, #58869 adds a broad preflight output clamp and rejects vLLM lower-bound parsing, #58981 combines the same clamp/parser changes with provider-cap propagation, and #61228 clamps custom-profile defaults while adding bounded retry-to-compression recovery.

Related pull requests

  • #14858 [closed] related — (+120/-0) — distinct closed reference: guards against replacing a known large context window with an untrusted probe tier below the active prompt estimate; it remains relevant as the narrower fix for probe-collapse loops, not output-cap overflow.
  • #58869 related — (+151/-2) — superseded: preflight-clamps all chat-completions output caps and treats vLLM's “at least N” input count as non-exact, but the diff does not update the existing parser and recovery regressions identified by the contributor review. Despite the keep_open review on #58869, #61228 covers the strict-endpoint cause with narrower default-cap clamping and tested retry escalation without breaking that lower-bound contract.
  • #58981 related — (+557/-8) — partially overlapping, needs scope reduction: duplicates #58869's clamp/parser patch while adding distinct custom-provider cap preservation and propagation across cron, background review, and TUI paths. Keep the contributor's keep_open verdict only for that propagation work after removing the prohibited delegation cap knob, already-landed delegate overlap, and unrelated OpenViking batching.
  • #61228 related — (+355/-21) — preferred fix: clamps the custom profile's default output budget against estimated input, stops non-converging output-cap reduction by escalating to compression, and adds transport and loop regressions. Its current diff explicitly resets the burst counter after accepted responses and shares the 512-token margin with tests, addressing the blocking points from the keep_open review on #61228.

Duplicates

#58869 and the output-clamp/vLLM-parser portion of #58981 are substantially the same change; #61228 overlaps their strict-endpoint objective but implements a narrower default-cap clamp plus recovery-spiral guard. #14858 is not a duplicate.

Suggested consolidation

Merge #61228 after final CI/review because its current diff addresses the contributor review and directly tests both first-request clamping and non-converging recovery; close #58869 as superseded despite its keep_open review, because its unchanged conflicting parser/recovery tests remain unaddressed. Retain #58981 only as a narrowed provider-cap preservation/propagation PR after removing the reviewed-out delegation knob, landed delegate overlap, duplicate clamp/parser code, and unrelated OpenViking change; keep #14858 closed as a distinct historical probe-collapse reference.

Cross-PR triage: Reviewed 4 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 88 kB of PR diffs, 8 kB of issue/PR text, 7 kB of discussion (9 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants