refactor(complexity_router): drop the tier-rubric override, close the rubric on the window it was given - #35504
Conversation
|
@greptileai can you reply here with a score and why that score? |
|
bugbot run |
|
5/5 This is a clean, well-reasoned refactoring with a net negative diff (49 insertions / 138 deletions). Why 5:
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 025f003. Configure here.
Greptile SummaryThis PR removes the recently introduced operator-controlled classifier rubric and restores a single repository-owned classification prompt. It also clarifies the quoted-conversation header when assistant turns are enabled.
Confidence Score: 5/5The PR appears safe to merge, with no concrete changed-code failure identified. The classifier continues to send a fixed trusted system rubric, legacy override keys retain the documented permissive loading behavior, and the assistant-context header follows the same configuration flag that controls role labeling and assistant-turn eligibility.
|
| Filename | Overview |
|---|---|
| litellm/router_strategy/complexity_router/complexity_router.py | Consolidates the classifier rubric into one constant and conditionally clarifies the conversation header without introducing an actionable defect. |
| litellm/router_strategy/complexity_router/config.py | Removes the rubric override and its validator; permissive extra-field handling preserves loading of legacy configurations as explicitly intended. |
| tests/test_litellm/router_strategy/test_complexity_router.py | Removes tests for deleted override behavior while retaining end-to-end trust-boundary coverage and adding assertions for both header modes. |
Reviews (1): Last reviewed commit: "refactor(complexity_router): drop the op..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
025f003 to
4f5e075
Compare
|
Narrowed on 4f5e075 to a pure removal. The quoted-conversation header change I had included is reverted, so this PR now touches no prompt text at all: the composed default and the restored single constant hash identically (07d59e1d28f43648), and the trust-boundary paragraph appears on both a minus and a plus line only because it moves back inside the constant. @greptileai can you reply here with a score and why that score? |
|
bugbot run |
|
Score: 5/5 Reasons:
|
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 4f5e075. Configure here.
… 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
4f5e075 to
8e1b41f
Compare
|
@greptileai can you reply here with a score and why that score? |
|
bugbot run |
|
4/5 The refactor is clean, well-reasoned, and the test suite properly pins the wording. Here's the breakdown: What's solid (pushes toward 5):
What prevents a 5:
Neither is a blocker. The silent-drop concern is the more substantive one, but given |
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 8e1b41f. Configure here.
…will not receive The classifier's system role closes on a line chosen by the window, so a deployment at classifier_context_window_size: 0 is told to classify only the current message. The trust boundary paragraph above it was not conditional, and it names prior turns as quoted material, so the same system role still promised sections the payload never carries. That is the defect #35504 fixed for the closing line, one paragraph earlier. The boundary now tracks the window the same way. The half that defends against a caller's own system prompt stays unconditional, because that block is quoted at every window setting and dropping it would let a key scoped to the router pin itself to the top tier through its own system prompt. At a window above 0 the system role is byte-identical to before.
TLDR
Problem this solves:
classifier_tier_rubricshipped in feat(complexity_router): let the classifier see assistant turns and rate what a short reply approves #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 controlsclassifier_context_window_sizewas set to, so a deployment sending no conversation was told to weigh what a short reply approves, and a deployment sending a window was told to disregard itHow it solves it:
Relevant issues
classifier_tier_rubricoperator override fromComplexityRouterConfigclassifier_context_window_sizerather than on the assistant-turns toggle, so the default deployment keeps the fixFolds in #35508, closed in favour of this. Follow-up to #35471; the dashboard half is #35500
Linear ticket
Resolves LIT-5100
Pre-Submission checklist
Screenshots / Proof of Fix
There are exactly two prompts. With the window at 0 the classifier receives the original pre-#35471 rubric, byte for byte:
The two closing lines, which is the whole diff to the prompt:
classifier_context_window_sizeEverything before that line is identical in both, and identical to what ships today. The user payload is untouched, so with the window on the classifier still reads
Recent conversation ...thenClassify this message: yes., and with assistant turns on those quoted turns still carryuser:andassistant:labels.The end-to-end tier flip was proven against a live proxy in #35471 and is unaffected: nothing here touches extraction, the window, the payload or routing.
Type
🧹 Refactoring
Changes
complexity_router.pydrops_CLASSIFICATION_TRUST_BOUNDARYand the override composer, and_classification_system_prompt(context_window_size)now appends one of_CLASSIFICATION_CURRENT_MESSAGE_ONLYor_CLASSIFICATION_WITH_CONVERSATIONconfig.pydropsclassifier_tier_rubric,CLASSIFIER_TIER_RUBRIC_WARN_CHARS,_warn_on_long_tier_rubric, and the logger import that existed only for that validatorThings a reviewer will ask about:
Why the window and not the assistant toggle.
classifier_context_window_sizedefaults to 3, so quoting prior turns is on for every LLM-classifier deployment;classifier_context_include_assistant_turnsdefaults to false. The reported misclassification happens at window 3 with assistant turns off, so keying the wording on the assistant toggle would put that exact configuration back on the line that caused it. Whether the quoted turns are the user's alone or include the assistant's does not change what the model needs told, and whose turn is whose is already on the turns.Why two static strings rather than a formatted count. The window size is not named in the prompt, so there are exactly two system-role variants, both cacheable, and no per-N string to keep true.
Existing configs.
ComplexityRouterConfigisextra="allow", so a config still carrying aclassifier_tier_rubrickey keeps loading and the key is ignored rather than erroring on startup.QA runbook
pytest tests/test_litellm/router_strategy/test_complexity_router.py, 252 passed with 14 pre-existing failures that needsemantic_routerinstalled and are red on basegrep -rn "tier_rubric" litellm/ tests/returns nothingclassifier_type: llmandclassifier_context_window_size: 0, and confirm the classifier's system message ends on "Classify only the current message; use the other sections to disambiguate its difficulty."classifier_context_include_assistant_turnsdoes not change the system message at allclassifier_tier_rubrickey and confirm the proxy still boots and ignores itFinal Attestation
Note
Medium Risk
Changes LLM classifier instructions for all
classifier_type: llmdeployments, which can shift tier routing and spend; configs with staleclassifier_tier_rubricare ignored but still load.Overview
Removes the
classifier_tier_rubricoperator override (and its length warning validator) so the LLM classifier always uses the built-in tier definitions._classification_system_promptnow takesclassifier_context_window_sizeinstead of a custom rubric. It appends one of two fixed closing lines: with window 0, the pre-context wording (“classify only the current message”); with window > 0, wording that tells the model to use quoted prior turns and to rate short replies like “yes” on the work they approve—not the reply alone. The choice is keyed on window size, not the assistant-turns toggle, so default deployments keep the fix.Tests drop override/rubric-warning cases and pin the window-dependent system prompt behavior.
Reviewed by Cursor Bugbot for commit 8e1b41f. Bugbot is set up for automated code reviews on this repo. Configure here.