Skip to content

feat(router): add max_output_tokens condition to ModelRoute - #3898

Open
connectsudhindra wants to merge 4 commits into
headroomlabs-ai:mainfrom
connectsudhindra:feat/2765-model-route-max-output-tokens
Open

connectsudhindra wants to merge 4 commits into
headroomlabs-ai:mainfrom
connectsudhindra:feat/2765-model-route-max-output-tokens

Conversation

@connectsudhindra

@connectsudhindra connectsudhindra commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Description

ModelRoute could bound the estimated input size (max_input_tokens) but had no condition on the request's own max_tokens. That meant a tiny prompt asking for a long answer (for example, max_tokens: 4000) still matched a "trivial turn" downshift rule and was sent to the cheaper model.

This PR adds max_output_tokens, using the shape proposed in the issue. A rule with it set matches only when the request's max_tokens is <= N. As with the other conditions, None means "ignore this condition", set conditions AND together, and the first matching rule wins.

Open question from the issue, decided here: a request with no max_tokens, or with a non-integer or boolean value, does not match. An unbounded response is not a trivial turn, so the rule can never widen to cover unbounded requests. This follows the same fail-closed approach as the existing parser.

Closes #2765

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Performance improvement
  • Code refactoring (no functional changes)

Changes Made

  • headroom/proxy/model_router.py: new ModelRoute.max_output_tokens field. matches() and ModelRouter.select() take an optional max_tokens keyword, which defaults to None, so existing callers are unchanged. max_tokens is added to the decision reason. max_output_tokens is added to the allowed route keys and parsed with the existing _strict_opt_int, so a malformed, boolean or negative value skips the route. New request_max_tokens(body) helper.
  • headroom/proxy/handlers/anthropic.py: _maybe_route_model passes request_max_tokens(body) to the router.
  • docs/content/docs/configuration.mdx: new field in the rule table, plus a note on combining it with max_input_tokens.
  • Tests for matching, absent budget, select and reason, env parsing (valid and malformed), the helper, and handler wiring.

Testing

  • Unit tests pass (pytest)
  • Linting passes (ruff check .)
  • Type checking passes (mypy headroom): not run successfully. My local mypy stops on an unrelated numpy stub syntax error in this env
  • New tests added for new functionality
  • Manual testing performed

Test Output

$ pytest tests/test_proxy/test_model_router.py tests/test_proxy/test_model_router_wiring.py -q
54 passed, 1 warning in 5.22s

$ pytest tests/test_proxy -q -k "anthropic or router"
119 passed, 237 deselected, 1 warning in 14.41s

$ ruff check headroom/proxy/model_router.py headroom/proxy/handlers/anthropic.py tests/test_proxy/test_model_router*.py
All checks passed!

Real Behavior Proof

  • Environment: macOS, Python 3.12, branch head
  • Exact command / steps: the rule from the issue (from_models: [claude-opus-4-8], max_input_tokens: 4096, max_output_tokens: 512, require_no_tools, to_model: claude-haiku-4-5) built via ModelRouterConfig.from_env. I then ran select() for the issue's "Reply with the single word OK." prompt with three different max_tokens values.
  • Observed result: max_tokens=4000 stays on claude-opus-4-8, max_tokens=256 routes to claude-haiku-4-5, and a request with no max_tokens stays on claude-opus-4-8:
    max_tokens=4000: routed_model=claude-opus-4-8 reason=no rule matched
    max_tokens=256:  routed_model=claude-haiku-4-5 reason=matched rule 'trivial-opus->haiku': claude-opus-4-8 -> claude-haiku-4-5 (input_tokens=7, has_tools=False, max_tokens=256)
    max_tokens=None: routed_model=claude-opus-4-8 reason=no rule matched
    
  • Not tested: an end-to-end request through a running proxy against a live upstream.

Runtime Rollout Safety

  • Rollout-managed feature(s): none. This only extends the opt-in model router (HEADROOM_MODEL_ROUTER_ENABLED).
  • Minimum rollout channel: N/A
  • Stable/default behavior changed: No. The router is still off by default, and rules without max_output_tokens behave exactly as before.
  • Kill switch / disable path: unset HEADROOM_MODEL_ROUTER_ENABLED, or remove the field from the rule.
  • Unsafe override required: No
  • Qualification impact: None
  • Rollback path: revert this PR

Review Readiness

  • I have performed a self-review
  • This PR is ready for human review

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I did not edit CHANGELOG.md

Additional Notes

Routing still only applies on the Anthropic /v1/messages path, where max_tokens is required. If routing is later extended to the OpenAI paths (#2354), request_max_tokens is the single place to add max_completion_tokens / max_output_tokens.


Devin Review

A ModelRoute could bound the estimated input size but not the request's
own max_tokens, so a tiny prompt asking for a long answer still matched a
"trivial turn" downshift rule. Add max_output_tokens: the rule matches only
when the request declares max_tokens <= N. A request without max_tokens
does not match, so the rule can never widen to unbounded responses.

Closes headroomlabs-ai#2765
@github-actions

github-actions Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

PR governance

This PR follows the template and is marked ready for human review.

@github-actions github-actions Bot added status: needs author action Pull request body or readiness checklist still needs author updates status: ready for review Pull request body is complete and the author marked it ready for human review and removed status: needs author action Pull request body or readiness checklist still needs author updates labels Oct 1, 2026
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@JerrettDavis JerrettDavis added status: has conflicts Pull request has merge conflicts with the base branch and removed status: ready for review Pull request body is complete and the author marked it ready for human review labels Oct 1, 2026
@github-actions github-actions Bot added status: needs rebase Pull request branch is behind the base branch on files it also changes status: ready for review Pull request body is complete and the author marked it ready for human review and removed status: has conflicts Pull request has merge conflicts with the base branch status: ready for review Pull request body is complete and the author marked it ready for human review labels Oct 2, 2026
devin-ai-integration[bot]

This comment was marked as resolved.

@github-actions github-actions Bot added status: ready for review Pull request body is complete and the author marked it ready for human review and removed status: needs rebase Pull request branch is behind the base branch on files it also changes labels Oct 3, 2026

@JerrettDavis JerrettDavis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed refreshed head 47bcf62; 54 router/wiring tests passed with the native core. The unresolved positional-constructor finding is confirmed against main:

ModelRoute("cheap", 4000, None, True) previously sets require_no_tools=True and rejects a tool-bearing request. This head instead sets max_output_tokens=True and leaves require_no_tools=False; the same request with max_tokens=1 matches and downshifts. Adding the field in the middle silently changes existing constructor semantics.

Please append the new field after the existing fields or make the new field keyword-only, preserving existing positional arguments, and add a regression covering the existing positional constructor. The output-budget matching and request wiring otherwise look sound.

@github-actions github-actions Bot added status: needs rebase Pull request branch is behind the base branch on files it also changes and removed status: ready for review Pull request body is complete and the author marked it ready for human review labels Oct 3, 2026
Inserting the field before require_no_tools shifted every later
positional constructor argument, so ModelRoute("cheap", 4000, None, True)
set max_output_tokens=True instead of require_no_tools=True and let
tool-bearing requests downshift. Move the field to the end and add a
regression test for positional construction.
@connectsudhindra

Copy link
Copy Markdown
Contributor Author

@JerrettDavis I fixed this in 8872c6e by appending max_output_tokens after the existing ModelRoute fields, so the positional constructor works as it did on main. I added a regression test for ModelRoute("cheap", 4000, None, True) and a full positional call. Results: 55 router/wiring tests pass, pytest tests/test_proxy -k "anthropic or router" passes 120, and ruff is clean.

@github-actions github-actions Bot added status: ready for review Pull request body is complete and the author marked it ready for human review and removed status: ready for review Pull request body is complete and the author marked it ready for human review labels Oct 6, 2026

@JerrettDavis JerrettDavis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The positional-constructor compatibility blocker is resolved: max_output_tokens is appended after the original dataclass fields, so existing no-tools/require-tools/from-models/name arguments keep their meanings. The added regression covers both short and complete legacy positional constructors and their matching behavior. The output-budget predicate and request wiring remain unchanged. Validation: 55 model-router/wiring tests passed with the existing native core build, including the new positional regression; Ruff lint/format and diff checks passed. Hosted checks on this exact head are terminal: 29 successful, 13 conditional skips, no failures or pending checks. No merge performed.

This branch has not been deployed

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

Labels

status: needs rebase Pull request branch is behind the base branch on files it also changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] ModelRoute: a condition on the request's own max_tokens (max_output_tokens)

3 participants