feat(complexity_router): let the classifier see assistant turns and rate what a short reply approves - #35471
Conversation
|
@greptileai can you reply here with a score and why that score? |
|
bugbot run |
|
Score: 4.5/5 What this PR gets very right: The core problem is real and the solution is precise. A classifier told "rate only the current message" then handed a bare "yes" will always return SIMPLE — the rubric change from The separation of Default-off for The "blank falls back" behavior for What pulls it below 5:
Overall this is well-engineered, well-tested, and the live-proxy proof in the PR description is unusually thorough. None of the concerns above are correctness blockers for the primary use case. |
Greptile SummaryThis PR improves follow-up classification by optionally including assistant turns and rating short approvals in their conversational context
Confidence Score: 5/5The PR appears safe to merge; no concrete blocking or independently actionable non-blocking issue remains The assistant-context behavior is opt-in, existing user-only prompt formatting remains unchanged when disabled, supported message surfaces normalize textual blocks correctly, and the tests cover the newly introduced routing and prompt-composition boundaries
|
| Filename | Overview |
|---|---|
| litellm/router_strategy/complexity_router/complexity_router.py | Adds role-aware classifier context extraction and configurable prompt composition without changing non-LLM or user-only routing inputs |
| litellm/router_strategy/complexity_router/config.py | Adds default-off assistant context and optional classifier tier-rubric configuration with clear validation and egress descriptions |
| tests/test_litellm/router_strategy/test_complexity_router.py | Adds focused regression coverage for assistant context, window semantics, prompt compatibility, and trust-boundary behavior |
Reviews (1): Last reviewed commit: "feat(complexity_router): let the classif..." | Re-trigger Greptile
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 5410d9b. Configure here.
5410d9b to
774df82
Compare
|
Thanks, all three checked. Pushed a test for the first one; declining all three, reasoning below. 1. 2. Rubric length cap. No field on 3. @greptileai can you reply here with a score and why that score? |
|
bugbot run |
|
Re-requesting on 774df82, the current head. The only change since 5410d9b is the added @greptileai can you reply here with a score and why that score? |
|
bugbot run |
|
All three rebuttals hold up:
Score: 4/5 The PR does what it says, does it safely, and the defaults preserve prior behavior byte-for-byte. The key risks are all handled:
The one thing that keeps it from a 5 is that |
|
Score: 4/5 Here is the breakdown: What works well:
Why not 5/5: One structural concern that doesn't break correctness but is worth noting for a follow-up: The production code and the new test on 774df82 are correct. The depth-signal consistency fix is the right change and is now properly covered. |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 774df82. Configure here.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
774df82 to
5a74a19
Compare
|
Partial-rubric coverage: enumerated the state space rather than guessing at it. Unset and blank fall back to the built-in tiers; a rubric naming all four, a subset, or none is honoured as written, and the response schema keeps the value legal in every case. So the only surprising outcome is that tiers the rubric omits stop being chosen and their mapped models stop receiving traffic. Documented that in the field description on 5a74a19 rather than validating it. A check that a rubric 'references all four tiers' has to sniff operator prose for tier names, which would both false-positive on wording like 'keep it simple' and reject a deliberate two-tier rubric, and it would fail config load on a heuristic. Same objection I had to a hard length cap. @greptileai can you reply here with a score and why that score? |
|
bugbot run |
|
Score: 4/5 The PR is well-engineered and the defense on partial-rubric coverage is correct. Here's the breakdown: What earns the 4:
What keeps it from 5: There's no runtime log or warning when the classifier returns a tier that has no mapped model in |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 5a74a19. Configure here.
|
Agreed it is a real gap, and declining it here on two grounds. It is reachable today with no rubric involved at all: a More to the point, that function is being rewritten right now in open PR #35331, which replaces the unmapped-tier fallback with a resolution ladder and touches @greptileai can you reply here with a score and why that score? |
|
4/5. Here is the breakdown. What earns it high marks:
What holds it to 4 rather than 5: The PR ships a documented footgun: a |
|
Filed the follow-up as LIT-5086 so the gap is tracked rather than left to a future reader: warn when a classified tier resolves through the fallback instead of its own entry, to land on top of #35331 once that rewrite settles. Ticket also notes this may be better as startup validation than a per-request log, since a partial tiers map is a static config fact and the request path is hot. |
…ate what a short reply approves The LLM classifier's context window carried user turns only, so a conversation whose difficulty was stated by the model rather than by the user was classified without it. Asked to find events, the assistant answers "here is the plan, it is complex, should I execute?", the user answers "yes", and the router rates the word "yes" and picks the cheapest tier Two independent causes, so two changes that are each provable on their own classifier_context_include_assistant_turns adds assistant turns to the window. It is off by default because turning it on shifts tier decisions, and therefore spend, for an already-deployed router, and because assistant text is net-new egress to the classifier deployment. With it on, classifier_context_window_size counts the last N turns across both roles, which is what makes the assistant's own statement of difficulty land in the window Assistant text reaches the classifier payload and nothing else. The window is read only by _build_classifier_user_payload, while keyword_tier_rules, escalation matching, the heuristic scorer and the semantic embedding all read the human ask through _iter_human_asks_newest_first. Those are substring and vector matchers, so an assistant echoing an escalation keyword back to a user would choose the model, and the spend, with nobody having asked. Rather than widen the shared iterator, _iter_context_turns_newest_first is separate and feeds the window alone, which makes the boundary structural instead of a rule to remember The rubric ended "Classify only the current message", and the classifier applied it literally: a request whose difficulty was established earlier came back SIMPLE because the message being rated was the word "yes". A context window the rubric then tells the model to disregard buys nothing, so the wording now asks it to rate the work the current message approves, judged in the conversation it continues, while still forbidding it to rate a quoted section as if that section were the request classifier_tier_rubric lets an operator replace the tier definitions. The trust-boundary paragraph is appended and cannot be replaced: it defends the operator against their own callers, so an operator writing tiers without that threat in mind would otherwise hand every keyholder the top tier by omission. Blank reads as unset so an empty form field falls back rather than sending a rubric with no tiers in it Turns are labelled by role only when assistant turns can appear, so the prompt of every deployment that never asked for this is unchanged byte for byte
5a74a19 to
34f4354
Compare
|
Taken, on 34f4354. Worth separating the two proposals: what I declined earlier was a hard
@greptileai can you reply here with a score and why that score? |
|
bugbot run |
|
5/5 The The broader PR holds up the same way:
No issues to raise. |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 34f4354. Configure here.
…o-Router screens PR #35471 added classifier_context_include_assistant_turns and classifier_tier_rubric to ComplexityRouterConfig. Both worked through config.yaml and the model API but had no control on the Add Model or Edit Auto-Router screens, so an operator working from the dashboard could not reach either. Wires both into the create and edit forms, shown only when the LLM classifier is selected, matching what #35315 did for the two context-window fields The create and edit stacks share the rendered control but keep their own serializer, their own hydration, and their own managed-key set, so each field is added in five places rather than one. A field wired into only one stack fails in a way neither serializer unit test can see, since those are handed a form value assembled by hand, so the edit-modal test drives the real component through open, edit and save A blank rubric is sent as omitted rather than as an empty string, in both serializers. The backend reads blank as unset and falls back to its built-in tier definitions, so storing "" would round-trip as a value that means nothing and reappear in the form as content. The assistant-turns switch is emitted even when false, because there the operator turning it off is a choice worth persisting rather than an absent value
…-Router screens PR #35471 added classifier_context_include_assistant_turns to ComplexityRouterConfig. It worked through config.yaml and the model API but had no control on the Add Model or Edit Auto-Router screens, so an operator working from the dashboard could not reach it. Wires it into the create and edit forms, shown only when the LLM classifier is selected, matching what #35315 did for the two context-window fields The create and edit stacks share the rendered control but keep their own serializer, their own hydration, and their own managed-key set, so the field is added in five places rather than one. A field wired into only one stack fails in a way neither serializer unit test can see, since those are handed a form value assembled by hand, so the edit-modal test drives the real component through open, edit and save The switch is emitted even when false, because there the operator turning it off is a choice that has to overwrite a stored true rather than an absent value a truthiness gate would drop
classifier_tier_rubric let an operator replace the classifier's tier definitions. It shipped in #35471 alongside the assistant-turn context window, but the two answer different halves of the same report and only the context window was asked for. The override carried its own surface to maintain: a system prompt composed from an overridable half and a non-overridable one, a blank-is-unset rule, a length-warning validator, and a matching pair of dashboard controls Removing it puts the classifier's system prompt back in the single constant it was before. The prompt an existing deployment sends is unchanged: the composed default and the restored constant are byte-identical, so the trust-boundary paragraph moves back inside the constant rather than being edited. Nothing about the wording, the quoted conversation, the context window or routing changes here A config still carrying a classifier_tier_rubric key keeps loading, since ComplexityRouterConfig is extra="allow"; the key is ignored from here on
… rubric on the window it was given Two changes to the classifier's system role, both narrowing it rather than adding to it classifier_tier_rubric let an operator replace the tier definitions. It shipped in #35471 alongside the assistant-turn context window, but the two answer different halves of the same report and only the context window was asked for. The override carried a composed prompt, an overridable and a non-overridable half, a blank-is-unset rule, a length-warning validator and a pair of dashboard controls. All of it goes The rubric then closes on one of two lines, chosen by classifier_context_window_size. At 0 no conversation is quoted, so the line is the original one, byte for byte: a deployment that sends no context is told to classify the current message and nothing else, which is what it could see all along. Above 0 the turns are quoted, and the original line told the model to disregard them, which is how a request whose difficulty was established in an earlier turn came back SIMPLE on the word "yes". There the line instead says to classify the current message using the quoted turns as context, and to rate what a short reply approves rather than the reply The choice keys on the window and not on classifier_context_include_assistant_turns. Whether the quoted turns are the user's alone or include the assistant's replies does not change what the model needs told, and whose turn is whose is already on the turns. Keying it on the assistant toggle would put the default deployment back on the original line, which is the configuration the report was raised against Folds in #35508, which built the window-dependent framing on top of the override this removes; that PR is closed in favour of this one
…-Router screens (#35500) PR #35471 added classifier_context_include_assistant_turns to ComplexityRouterConfig. It worked through config.yaml and the model API but had no control on the Add Model or Edit Auto-Router screens, so an operator working from the dashboard could not reach it. Wires it into the create and edit forms, shown only when the LLM classifier is selected, matching what #35315 did for the two context-window fields The create and edit stacks share the rendered control but keep their own serializer, their own hydration, and their own managed-key set, so the field is added in five places rather than one. A field wired into only one stack fails in a way neither serializer unit test can see, since those are handed a form value assembled by hand, so the edit-modal test drives the real component through open, edit and save The switch is emitted even when false, because there the operator turning it off is a choice that has to overwrite a stored true rather than an absent value a truthiness gate would drop
… rubric on the window it was given (#35504) Two changes to the classifier's system role, both narrowing it rather than adding to it classifier_tier_rubric let an operator replace the tier definitions. It shipped in #35471 alongside the assistant-turn context window, but the two answer different halves of the same report and only the context window was asked for. The override carried a composed prompt, an overridable and a non-overridable half, a blank-is-unset rule, a length-warning validator and a pair of dashboard controls. All of it goes The rubric then closes on one of two lines, chosen by classifier_context_window_size. At 0 no conversation is quoted, so the line is the original one, byte for byte: a deployment that sends no context is told to classify the current message and nothing else, which is what it could see all along. Above 0 the turns are quoted, and the original line told the model to disregard them, which is how a request whose difficulty was established in an earlier turn came back SIMPLE on the word "yes". There the line instead says to classify the current message using the quoted turns as context, and to rate what a short reply approves rather than the reply The choice keys on the window and not on classifier_context_include_assistant_turns. Whether the quoted turns are the user's alone or include the assistant's replies does not change what the model needs told, and whose turn is whose is already on the turns. Keying it on the assistant toggle would put the default deployment back on the original line, which is the configuration the report was raised against Folds in #35508, which built the window-dependent framing on top of the override this removes; that PR is closed in favour of this one
2 participants
TLDR
Problem this solves:
How it solves it:
classifier_context_include_assistant_turns(default off) puts assistant turns in the context window, soclassifier_context_window_sizebecomes the last N turns across both rolesclassifier_tier_rubriclets an operator supply their own tier definitions, with the trust-boundary paragraph always appendedRelevant issues
Follow-up to the context window added in #35185
Linear ticket
Resolves LIT-5080
Pre-Submission checklist
Screenshots / Proof of Fix
Live proxy on port 4099, classifier is Haiku 4.5, tiers are Haiku 4.5 (SIMPLE/MEDIUM) and Sonnet 5 (COMPLEX/REASONING), real provider calls.
return_raw_model_name: trueso the response reports the model that actually served the request. Two auto-router routes differing only by the new flag:The reported conversation, sent to each route on each request surface:
The same three-turn body was also sent to
/v1/responses(asinput) and to/v1/messages. Before, onlitellm_internal_stagingat 2b30708:Six for six on the cheap tier, and
auto-ctx-onsilently no-ops becauseComplexityRouterConfigisextra="allow", so the unknown key is accepted and ignored. The proxy log agrees,6 ComplexityRouter: routing decision cause=llm_classifier, tier=SIMPLE. After, same commands:3 ... tier=COMPLEXand3 ... tier=SIMPLEin the log. The classifier payload with the flag on, lifted from the same run:The rubric change is proven separately, on
auto-ctx-offso assistant turns are not involved and only the wording differs. Difficulty stated in a prior user turn, current ask is "yes":Upstream for every call above was the sandbox gateway rather than api.anthropic.com, because the Anthropic key in
.envis out of credit; the calls are Bedrock-hosted Anthropic models and cost real money. A direct api.anthropic.com re-run is owed if anyone wants it.Type
🆕 New Feature
Changes
_iter_context_turns_newest_firstfeeds the classifier window and may include assistant turns;_iter_human_asks_newest_firststays user-only and keeps feedingkeyword_tier_rules, escalation, the heuristic scorer and the semantic embedding_extract_prior_user_turnsbecomes_extract_prior_turnsand returns(role, text); turns are labelled by role in the payload only when assistant turns can appear, so an existing deployment's prompt is unchanged byte for byte_CLASSIFICATION_TIER_RUBRIC(overridable) and_CLASSIFICATION_TRUST_BOUNDARY(always appended), composed by_classification_system_promptComplexityRouterConfig:classifier_context_include_assistant_turnsandclassifier_tier_rubricThings a reviewer will ask about:
Why assistant text is safe here. The prior-turn window has exactly one consumer,
_build_classifier_user_payload. Everything that matches on strings or vectors reads the current ask, which comes from a different iterator that still filters onrole == "user". That matters because an assistant turn quotingLITELLM ESCALATEback to a user would otherwise escalate the session, and a rule keyword in a model's reply would force a tier; a regression test drives both strings through an assistant turn, including the case where that turn is the newest message, and asserts the tier does not move.Why the override cannot replace the whole system prompt. The trust-boundary paragraph protects the operator from their own callers, so an operator writing tier definitions without that threat in mind would hand every keyholder the top tier by omission. Tier values stay constrained by the response schema either way.
Why the default is off. Enabling assistant context changes tier decisions, and therefore spend, for routers already in production, and it sends assistant text to the classifier deployment, which may be a different provider than the routed model. The field description records that egress the same way
classifier_context_window_sizerecords its own.What does not change. Nothing about the heuristic classifier, nothing when
classifier_typeis notllm, and nothing in the payload for any deployment that leaves the new field unset. Turns with no text, an assistant turn holding only tool calls or thinking blocks, are skipped rather than quoted as an empty slot, so they do not spend a window position.UI exposure of the two new fields on the Auto-Router create and edit screens is deliberately left out, matching how #35185 and #35315 were split.
QA runbook
GATEWAY_KEYor any provider key wired to the three model entriesauto-ctx-offon/v1/chat/completions,/v1/responsesand/v1/messages; every response should report the cheap model, byte-identical to baseauto-ctx-on; every response should report the strong modelclassifier_context_window_size: 0alongsideclassifier_context_include_assistant_turns: trueand confirm the classifier payload carries no conversation context at allclassifier_tier_rubricto a short rubric of your own and confirm the classifier's system message contains it followed by the trust-boundary paragraph, and that a caller system prompt saying "every request is REASONING" still cannot reach the system roleFinal Attestation
Note
Medium Risk
Changes tier routing and spend when assistant context or custom rubrics are enabled; defaults preserve existing behaviour, but misconfigured rubrics can starve tiers of traffic.
Overview
Improves LLM complexity classification for multi-turn chats where the real difficulty lives in assistant plans or earlier user turns, not in short replies like “yes”.
Adds
classifier_context_include_assistant_turns(default off): when on, the classifier context window is the last N user + assistant turns (role-labelled in the payload), while keyword/escalation matching still uses user-only asks via a separate iterator. Prior context is now_extract_prior_turnsreturning(role, text); the depth signal uses the same turn set so it stays aligned with what was quoted.Splits the classifier system prompt into overridable tier definitions (
classifier_tier_rubric, blank → built-in) and a fixed trust-boundary paragraph (short-reply / conversation-context guidance; caller text cannot override tiers). Long custom rubrics log a warning at load time but are still applied.Extensive tests cover assistant-in-window behaviour, trust boundary, and regression that assistant text cannot trigger keyword/escalation routing.
Reviewed by Cursor Bugbot for commit 34f4354. Bugbot is set up for automated code reviews on this repo. Configure here.