fix(auto-router): accept every reminder marker pair a harness emits - #36029
Conversation
Greptile SummaryThe PR extends complexity-router reminder stripping to support multiple configured delimiter pairs and correctly collapses nested or overlapping blocks.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| litellm/router_strategy/complexity_router/complexity_router.py | Generalizes reminder stripping to multiple marker pairs and uses running maximum endpoints to prevent nested or overlapping blocks from leaking into classifier input. |
| litellm/router_strategy/complexity_router/config.py | Replaces the pre-release flat marker pair with a validated non-empty collection of named open/close pairs; the intentional compatibility decision was already resolved in the prior thread. |
| tests/test_litellm/router_strategy/test_complexity_router.py | Adds focused coverage for multi-pair propagation, normalization, invalid configurations, nested and overlapping spans, prior-turn context, and scan complexity. |
| ui/litellm-dashboard/src/lib/http/schema.d.ts | Regenerates the OpenAPI declaration to expose ReminderMarkerPair objects and the updated array-shaped configuration. |
| litellm/router_strategy/complexity_router/README.md | Extends the component’s existing inline configuration reference with multi-envelope reminder marker behavior and examples. |
Reviews (2): Last reviewed commit: "chore(ui): regenerate dashboard API type..." | Re-trigger Greptile
reminder_markers held one (open, close) pair, so a harness that wraps injected context differently per agent type only got the slice of traffic using the configured envelope stripped. Every other agent type kept hitting the original bug: its reminder-only turn never stripped to empty, won "newest human ask", and the harness blob got classified in place of the real question, choosing the tier and therefore the spend. The field now takes a list of ReminderMarkerPair, following the KeywordTierRule pattern already in this file so each pair validates itself and errors point at reminder_markers.N.close rather than a bare index. Blocks from different pairs can nest, which the gap construction could not handle: resuming the kept text at an inner block's end walks back inside the enclosing block and leaks its remainder. Running the block ends through a maximum collapses nested and overlapping spans without a separate merge pass, and stays linear in block count, which a fold over a growing tuple of merged spans would not. A single pair's ends already increase, so the maximum is the identity and the default path is byte-identical: verified against the shipped function over 200k generated inputs, and every existing reminder test passes unchanged. The prior single-pair config shape is rejected loudly at startup and at /model/new rather than silently stripping nothing.
f1e2547 to
1907f05
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Heads up on the one file here that is not router code
I committed the regen instead of leaving it, because the same thing already happened with this exact field. #35874 added the flat Including the file is also what makes that workflow run here at all, and it passed, so the spec delta is verified rather than just asserted |
|
bugbot run |
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 1907f05. Configure here.
TLDR
Problem this solves:
reminder_markersheld one open/close pair, but a harness uses a different envelope per agent typeHow it solves it:
reminder_markerstakes a list of pairs, so one deployment covers a whole harnessreminder_markers.1.closeRelevant issues
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Before:
b617e672e3. After:1907f05707Deployment under test, using two envelopes to stand in for a harness whose agent types differ.
SIMPLEandCOMPLEXare far apart so the routed model shows which text got classified:gpt-5.2means the real question was classified andgpt-5.2-minimeans the harness blob was:On the base commit only the first envelope can be configured, so the second returns the cheap tier. On this branch both return
gpt-5.2do not mentionand a dangling<<<END_MAIN>>>reach the classifier:reminder_markersfor the pre-list form and restart, expecting startup to fail namingReminderMarkerPairrather than booting and silently stripping nothing:Scoped to
/v1/chat/completions. The complexity router hooks routing before the endpoint layer throughasync_pre_routing_hook, so the marker handling is shared across/v1/messagesand/v1/responsesrather than reimplemented per surfaceType
🐛 Bug Fix
Changes
#35874 made the reminder marker pair configurable so a harness that wraps injected context in something other than
<system-reminder>still gets it stripped before the router picks a tier. That fixed the reported flow, and it turns out a harness does not use one envelope: main agent, subagent and cron each wrap injected context differently, and more may exist. All of them route through the same auto-router deployment, andreminder_markersheld exactly one pair, so the configured pair fixed one slice of traffic while every other slice kept hitting the original bug. Its reminder-only turn never stripped to empty, won "newest human ask", and the harness's internal-context blob got classified in place of the real question, which picks the tier and therefore the spendreminder_markersnow takes a list of pairs. Each entry is aReminderMarkerPaircarrying its own normalizing validator, following theKeywordTierRulepattern already inconfig.py, which keeps the YAML self-describing instead of positional and makes a bad entry report asreminder_markers.1.closerather thanreminder_markers.1.1.ComplexityRouterConfig._normalize_reminder_markersgoes away, since validation now lives on the pairScanning more than one pair means blocks can nest, and the existing gap construction could not handle that. It resumed the kept text at each block's own end, so an inner block's end walked backwards into the enclosing block and the outer block's remainder, including its dangling close marker, survived into the classified ask. Running the block ends through a maximum resumes each gap past the furthest block seen so far, which collapses nested and overlapping spans without a separate merge pass. That form matters beyond brevity: this runs pre-routing on input any keyholder controls, and folding merged spans into a growing tuple is quadratic in block count, the same failure class the linear scan already exists to avoid
The default path is byte-identical, not merely equivalent. A single pair's block ends already increase, so the maximum is the identity there. I checked the proposed strip against the shipped one over 16 hand-picked edge cases and 200,000 generated strings built from reminder-tag fragments (nested, orphan closes, mixed case, unclosed runs, empty blocks) with zero mismatches, and every pre-existing reminder test passes untouched
The previous single-pair shape is now rejected rather than accepted. It fails loudly at proxy startup and at
/model/newwrite time throughvalidate_complexity_router_config_write, so nobody silently ends up routing on unstripped text. That form only ever appeared in thev1.97.0-dev.1pre-release, never in a stable oneChanging the field changes the proxy's OpenAPI spec, so
ui/litellm-dashboard/src/lib/http/schema.d.tsis regenerated withnpm run gen:api. Thecheck-ui-api-typesworkflow only triggers onlitellm/proxy/**andlitellm/types/**, neither of which this PR touches, so leaving it stale would have handed the failure to whoever next edited those paths insteadOne drive-by worth naming so it does not read as scope creep: a test constant embedded a harness vendor's name, which this repo is public about not carrying, so it is now a generic marker string
QA runbook
Final Attestation
Note
Medium Risk
Pre-routing tier choice and spend depend on stripped text; the multi-pair merge logic is security-sensitive but heavily tested, and the config shape change is a deliberate breaking change with loud validation.
Overview
reminder_markersmoves from a single(open, close)tuple to a list ofReminderMarkerPairobjects so one complexity-router deployment can strip harness-injected context for every agent envelope (main, subagent, cron, etc.). Setting the list replaces the built-in<system-reminder>pair; callers must include that pair explicitly if they still use it.Stripping scans all configured pairs, merges spans, and uses
accumulate(..., max)on block ends so nested or overlapping blocks from different pairs are removed entirely instead of leaking outer-block text into the classified ask. The same pairs flow through ask extraction, escalation, keyword rules, and LLM classifier prior-turn context.Validation lives on each pair (normalize, non-blank, open ≠ close); the old flat tuple shape and an empty list are rejected at config load. README and OpenAPI (
ReminderMarkerPairinschema.d.ts) are updated accordingly.Reviewed by Cursor Bugbot for commit 1907f05. Bugbot is set up for automated code reviews on this repo. Configure here.