feat(router): add max_output_tokens condition to ModelRoute - #3898
connectsudhindra wants to merge 4 commits into
Conversation
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
PR governanceThis PR follows the template and is marked ready for human review. |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
JerrettDavis
left a comment
There was a problem hiding this comment.
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.
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.
|
@JerrettDavis I fixed this in 8872c6e by appending |
JerrettDavis
left a comment
There was a problem hiding this comment.
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.
Description
ModelRoutecould bound the estimated input size (max_input_tokens) but had no condition on the request's ownmax_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'smax_tokensis<= N. As with the other conditions,Nonemeans "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
Changes Made
headroom/proxy/model_router.py: newModelRoute.max_output_tokensfield.matches()andModelRouter.select()take an optionalmax_tokenskeyword, which defaults toNone, so existing callers are unchanged.max_tokensis added to the decision reason.max_output_tokensis added to the allowed route keys and parsed with the existing_strict_opt_int, so a malformed, boolean or negative value skips the route. Newrequest_max_tokens(body)helper.headroom/proxy/handlers/anthropic.py:_maybe_route_modelpassesrequest_max_tokens(body)to the router.docs/content/docs/configuration.mdx: new field in the rule table, plus a note on combining it withmax_input_tokens.Testing
pytest)ruff check .)mypy headroom): not run successfully. My local mypy stops on an unrelated numpy stub syntax error in this envTest Output
Real Behavior Proof
from_models: [claude-opus-4-8],max_input_tokens: 4096,max_output_tokens: 512,require_no_tools,to_model: claude-haiku-4-5) built viaModelRouterConfig.from_env. I then ranselect()for the issue's "Reply with the single word OK." prompt with three differentmax_tokensvalues.max_tokens=4000stays onclaude-opus-4-8,max_tokens=256routes toclaude-haiku-4-5, and a request with nomax_tokensstays onclaude-opus-4-8:Runtime Rollout Safety
HEADROOM_MODEL_ROUTER_ENABLED).max_output_tokensbehave exactly as before.HEADROOM_MODEL_ROUTER_ENABLED, or remove the field from the rule.Review Readiness
Checklist
CHANGELOG.mdAdditional Notes
Routing still only applies on the Anthropic
/v1/messagespath, wheremax_tokensis required. If routing is later extended to the OpenAI paths (#2354),request_max_tokensis the single place to addmax_completion_tokens/max_output_tokens.