fix(ui): reject an auto-router keyword rule left empty instead of dropping it

[tin-berri]

TLDR

Problem this solves:

• Adding a keyword tier override and leaving the keywords box empty created the auto router anyway, with the rule silently discarded
• Sending that same rule straight to the API wrote the row, then dropped the deployment on reload and returned a 500, leaving a model that can never load

How it solves it:

• The check that a rule carries at least one keyword now runs on every submit instead of only when semantic matching is on
• The write endpoints parse the config with the router's own model, so an unloadable one is a 400 and nothing is persisted

Relevant issues

• An unfilled keyword rule marks its own row and withholds the submit button, so the router cannot be created while one is outstanding, on both the create form and the edit modal
• serializeKeywordTierRules stops discarding empty rules, so an unfilled row can never pass for a saved one
• The management write paths now judge complexity_router_config content, closing the gap auto_router_model_naming documents as its own purpose

Linear ticket

Resolves LIT-5133

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

• I have added meaningful tests
• My PR passes all CI/CD checks (e.g., lint, format, unit tests)
• My PR's scope is as isolated as possible; it only solves 1 specific problem
• I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Screenshots / Proof of Fix

Live proxy on localhost:4000 against a real Postgres. First, what the two halves looked like before

# the dashboard stripped the empty rule client-side, so the router was created without it
$ curl -s -o /dev/null -w "HTTP %{http_code}\n" -X POST http://localhost:4000/model/new -H "Authorization: Bearer $KEY" \
  -H "Content-Type: application/json" -d '{"model_name":"router-old-ui","litellm_params":{...no keyword_tier_rules...}}'
HTTP 200

# the rule the operator actually configured, sent intact
$ curl -s -X POST http://localhost:4000/model/new -H "Authorization: Bearer $KEY" -H "Content-Type: application/json" \
  -d '{"model_name":"router-empty-rule","litellm_params":{"model":"auto_router/complexity_router",
       "complexity_router_default_model":"gpt-4o-mini","complexity_router_config":{
       "tiers":{"SIMPLE":["gpt-4o-mini"],"MEDIUM":["gpt-4o-mini"],"COMPLEX":["gpt-4o"],"REASONING":["gpt-4o"]},
       "classifier_type":"heuristic","keyword_tier_rules":[{"keywords":[],"tier":"COMPLEX"}]}},"model_info":{}}'
HTTP 500
{"error": {"message": "Model create was saved to the database, but the model id(s) ['e2afc484-...'] are not
 live in this pod's router after the reload and are not being served by this pod. ...",
 "type": "internal_server_error"}}

That 500 leaves a durable row the router will never load, which is what the client-side strip was avoiding by throwing the rule away instead

$ psql -U tin -d lit5133qa -tA -c "select model_name, litellm_params::json->'complexity_router_config'->'keyword_tier_rules' from \"LiteLLM_ProxyModelTable\";"
router-old-ui|
router-empty-rule|[{"tier": "COMPLEX", "keywords": []}]

$ curl -s http://localhost:4000/v1/models -H "Authorization: Bearer $KEY" | jq -r '.data[].id'
gpt-4o-mini
gpt-4o
router-old-ui          # created, with the operator's rule gone
                       # router-empty-rule is absent: written, never served

LiteLLM Router:WARNING: router.py:8437 - Error upserting deployment router-empty-rule (id=e2afc484-...):
  1 validation error for ComplexityRouterConfig
  List should have at least 1 item after validation, not 0 [type=too_short, input_value=[], input_type=list]
  Dropping it and continuing with other deployments.

After the change, on a fresh database. The same payload is refused at the boundary

$ curl -s -w "\nHTTP %{http_code}\n" -X POST http://localhost:4000/model/new ... same body as above ...
{
  "error": {
    "message": "complexity_router_config is invalid at keyword_tier_rules.0.keywords: List should have at
     least 1 item after validation, not 0. The router would drop this deployment at load time, so the write
     is rejected instead.",
    "type": "validation_error",
    "param": "litellm_params.model",
    "code": "400"
  }
}
HTTP 400

$ psql -U tin -d lit5133qa -tA -c "select count(*) || ' rows' from \"LiteLLM_ProxyModelTable\";"
0 rows

Whitespace-only keywords hit the router's own normalizer, and a valid rule still creates and serves

$ curl -s -X POST http://localhost:4000/model/new ... "keyword_tier_rules":[{"keywords":["   "],"tier":"COMPLEX"}] ...
"... complexity_router_config is invalid at keyword_tier_rules.0: Value error, keyword_tier_rules entries
 must contain at least one non-empty keyword. ..."

$ curl -s -o /dev/null -w "HTTP %{http_code}\n" -X POST http://localhost:4000/model/new ... "keywords":["invoice","refund","billing"] ...
HTTP 200
$ curl -s http://localhost:4000/v1/models -H "Authorization: Bearer $KEY" | jq -r '.data[].id'
gpt-4o-mini
gpt-4o
router-valid-rule

Every write shape, against a fresh database. A patch that carries only a config is covered too, which is what the dashboard's edit modal and any caller updating just the routing rules send

1. create valid:                       HTTP 200
2. create with empty rule:             HTTP 400
3. patch model + empty rule:           HTTP 400
4. patch config-only, no model:        HTTP 400
5. patch config-only, valid:           HTTP 200
6. patch unrelated field (rpm):        HTTP 200
7. rename only, against a bad stored config:  HTTP 200

$ psql -U tin -d lit5133qa -tA -c "select litellm_params::json->'complexity_router_config'->'keyword_tier_rules' from \"LiteLLM_ProxyModelTable\";"
[{"tier": "COMPLEX", "keywords": ["invoice"]}]

Case 4 is the one worth calling out: before it was closed, that patch overwrote a serving router with a config it could not build and took it out of service on the way to the 500. Case 7 is why a stored config is never judged on a write that does not carry one

The inference leg is not included because both shared provider accounts are out of credit right now; routing behaviour is untouched by this PR either way

UI runbook

Start the proxy and npm run dev in ui/litellm-dashboard, then

1. Go to http://localhost:3000/?page=llm-model-hub, open Auto Routers, click Add Auto Router
2. Name it, pick a model for all four tiers
3. Expand "Advanced: Keyword/Semantic Matching", click "+ Add keyword rule", leave Keywords 1 empty, leave the tier on Complex
4. The Keywords 1 box turns red with "At least one keyword is required" under it, and Add Auto Router greys out. Hover it and the tooltip names the row. Before this PR the button was clickable and the router was created with the rule gone
5. Type invoice into Keywords 1 and press enter. The error clears and the button comes back; click it and the rule is on the saved config
6. Reopen that router, click "+ Add keyword rule", leave it empty. Save Changes greys out the same way, with the error on the second row

Type

🐛 Bug Fix

Changes

• getKeywordTierRulesError rejects any rule whose keywords are empty after trimming and names each offending row; getSemanticConfigError drops the per-rule check it now subsumes, which only ever ran with the semantic toggle on
• serializeKeywordTierRules returns one entry per rule instead of filtering empties out, so validation and the payload agree on what a keyword is and row numbers line up with the "Keywords N" labels
• Both add_auto_router_tab.tsx and edit_auto_router_modal.tsx call the new check before building their payload
• KeywordTierRules marks an offending row with status="error" and "At least one keyword is required", driven by the same emptyKeywordTierRuleIndexes the submit message uses, so the row named and the row marked cannot diverge
• Both submit buttons are disabled while a tier is unfilled or a keyword row is empty, with the reason in a tooltip. Disable state and tooltip read one value, so a greyed button always explains itself
• Enter commits a typed keyword. open={false} leaves antd nothing for Enter to select, so the word previously only became a tag on blur, and clicking submit was what supplied that blur; with the button withheld that route is gone and the row could not be filled
• The edit modal gains the showValidationErrors state it was missing, so its classifier error renders at all
• validate_complexity_router_config_write parses a written complexity_router_config with ComplexityRouterConfig, so /model/new, /model/{id}/update and the legacy /model/update all reject an unloadable one with a 400 and persist nothing

Things a reviewer will ask about

Whether parsing at the write boundary can reject a config that works today. It cannot: resolve_complexity_router_plugins runs only on the config.yaml path, so a written config already reaches ComplexityRouterConfig.model_validate unchanged moments later at load time. The boundary calls that same model rather than a copy of its rules, and the model is extra="allow", so unknown keys stay the caller's business

Why the row reports itself instead of waiting for a failed submit. With the button withheld there is no failed submit left to surface anything, so a row that only spoke up after one would never speak at all. A row exists only because the caller clicked "Add keyword rule", so it is safe to have it report straight away; tiers keep their existing behaviour and rely on the tooltip, since a blank form turning red on load helps nobody

Why the serializer no longer drops empty rules. Dropping them is what made the bug invisible, and it breaks the row numbering the message depends on. Reverting the serializer alone fails five of the new tests, including the validator ones, which is the check that the call into it is real

Why the config is judged on its own rather than by classifying the deployment's model. A patch may write a config without naming a model, and the stored model is encrypted at rest, so it cannot be classified from the row. Judging the config alone is the only reading that covers that path

Why a write that carries no config is never rejected for a stored one. A row stored before this validation existed is already unloadable, and blocking its rename would break the restore path _strategy_router_write_violation documents. The repair is a write that supplies a good config, which is covered

QA runbook

Follow the UI runbook above for the dashboard half, and the curl block for the API half

• npx vitest run src/components/add_model src/components/edit_auto_router is green; the full dashboard suite passes at 5906 tests
• pytest tests/test_litellm/router_utils/test_auto_router_model_naming.py tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py tests/test_litellm/router_strategy/test_complexity_router.py passes at 397

Mutation checks run against this diff:

• reverting the two UI guard call sites fails exactly the two new workflow tests and nothing else
• reverting the inline row error fails the tests that assert it, on both forms
• forcing either submit button back to always-enabled fails six tests across both forms
• restoring the serializer's empty-rule filter fails six tests across both the create and edit payload builders
• disabling the new backend config check fails exactly the new backend tests, with the pre-existing ones still passing

Final Attestation

• The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

---

Note

Medium Risk
Touches proxy model create/update validation for auto-router deployments; behavior is aligned with existing router load rules and mainly prevents bad writes, but incorrect validation could block legitimate configs.

Overview
Fixes LIT-5133: empty keyword tier rules could be saved (or stripped client-side) and leave deployments that fail at router reload with a 500 instead of a clear validation error.

Proxy: Model create/update now runs validate_complexity_router_config_write on any incoming complexity_router_config, using the same ComplexityRouterConfig the router loads. Invalid configs (e.g. keyword_tier_rules with no keywords) return 400 before persistence. _strategy_router_write_violation validates config-only patches without requiring litellm_params.model, and does not judge stored config on renames that omit config.

Dashboard: getKeywordTierRulesError and emptyKeywordTierRuleIndexes block submit whenever a keyword rule has no non-empty keyword, including when semantic matching is off. serializeKeywordTierRules keeps empty rules in the payload instead of dropping them. KeywordTierRules shows per-row errors, disables submit via shared submitBlockedReason (tooltip on create/edit), and commits typed keywords on Enter/blur so rows can be filled with the button disabled. Create and edit flows both call the new checks before building the config.

^{Reviewed by Cursor Bugbot for commit b647947. Bugbot is set up for automated code reviews on this repo. Configure here.}

[tin-berri]

bugbot run

[tin-berri]

@greptileai can you reply here with a score and why that score?

[greptile-apps]

Score: 5/5

The fix is correct, well-scoped, and the tests validate exactly the right things.

Why the approach is sound:

The root cause was architectural: serializeKeywordTierRules was silently dropping empty rows, which made getSemanticConfigError (the only previous guard) irrelevant when the semantic toggle was off. The PR fixes this by:

1. Making the serializer a pure transformer (no filtering) — preserving row identity so error messages can name the right row
2. Extracting getKeywordTierRulesError as an unconditional guard that runs before the semantic check, calling the serializer so both functions agree on what "empty" means
3. Wiring both call sites (create + edit) to the new guard

Why the serializer change is safe:

With getKeywordTierRulesError gating both submit paths, an empty rule can only reach the payload if someone bypasses the UI (raw API), at which point the backend correctly 500s loudly instead of silently dropping the row and reporting success. That's strictly better behavior.

Test quality is high:

• The new integration tests specifically disable semantic matching to confirm the fix isn't accidentally passing because the old guard was still active
• The getKeywordTierRulesError unit tests cover: no rules (valid), whitespace-only entries, multiple offending rows (row numbering), and the trim-but-keep case
• Reverting either guard call site or restoring the filter in the serializer breaks exactly the expected tests — the PR explicitly verified this

No concerns.

[cursor]

✅ 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 0460803. Configure here.}

[greptile-apps]

Greptile Summary

The PR prevents empty auto-router keyword rules from being silently discarded during create or edit.

• Adds unconditional keyword-rule validation to both submit workflows.
• Keeps one serialized entry per configured rule so validation and payload row numbering remain aligned.
• Adds focused validator, serializer, create-flow, and edit-flow regression tests.

Confidence Score: 5/5

The PR appears safe to merge, with create and edit paths consistently validating empty keyword rules before their payload builders run.

The validator and both payload builders share the same serializer semantics, every current production submit path applies the guard, and the regression tests cover rejection, successful submission, whitespace handling, and row numbering.

Important Files Changed

Filename	Overview
ui/litellm-dashboard/src/components/add_model/add_auto_router_tab.tsx	Adds a pre-submit empty-keyword-rule guard before semantic validation and payload construction.
ui/litellm-dashboard/src/components/add_model/build_complexity_router_config.ts	Centralizes per-rule keyword validation using the same serialization semantics as payload generation.
ui/litellm-dashboard/src/components/add_model/complexity_router_keywords.ts	Preserves empty serialized rule rows so they cannot disappear silently and row indexes remain stable.
ui/litellm-dashboard/src/components/edit_auto_router/edit_auto_router_modal.tsx	Applies the same keyword-rule guard to edits before constructing and sending the update.
ui/litellm-dashboard/src/components/add_model/add_auto_router_tab.test.tsx	Verifies creation is blocked for an empty rule and succeeds with the expected payload once populated.
ui/litellm-dashboard/src/components/add_model/build_complexity_router_config.test.ts	Covers retained empty rows, whitespace normalization, validation messages, and row numbering.
ui/litellm-dashboard/src/components/edit_auto_router/build_updated_complexity_router_config.test.ts	Updates edit-builder coverage to ensure empty caller-provided rows are not silently removed.
ui/litellm-dashboard/src/components/edit_auto_router/edit_auto_router_modal.test.tsx	Verifies editing cannot save a newly added empty rule and does not issue the patch request.

_{Reviews (1): Last reviewed commit: "fix(ui): reject an auto-router keyword r..." | Re-trigger Greptile}

[tin-berri]

Force-pushed. Folded in the backend half: validate_strategy_router_model_write now parses the merged complexity_router_config with the router's own ComplexityRouterConfig, so all three write routes reject an unloadable config with a 400 and persist nothing, instead of writing the row and returning a 500 once the reload drops the deployment. Live before/after curls and the PATCH case are in the description.

bugbot run

@greptileai can you reply here with a score and why that score?

[greptile-apps]

5/5

This is a well-reasoned, minimal fix with tight mutation-verified tests. Here's why it earns the top score:

Correctness of approach

The backend check delegates to ComplexityRouterConfig.model_validate — the router's own model — rather than duplicating its rules. That means the boundary rejects exactly what the load would drop, and the two can never drift apart as the model evolves. The fallback logic in _strategy_router_write_violation (incoming if non-None else stored) correctly reflects that complexity_router_config is written wholesale, not merged field-by-field.

Serializer inversion is justified

Dropping empty rules in serializeKeywordTierRules was what made the original bug invisible: the create succeeded, the rule was gone, and nobody said anything. Keeping empty rules now means the validator and the payload are always looking at the same thing, and row numbers in the error message align with the UI labels. The test comment ("reverting the serializer alone fails six tests including the validator ones") confirms this coupling is tested end-to-end.

Frontend check placement

getKeywordTierRulesError runs before getSemanticConfigError, which is correct — a missing keyword is a structural problem independent of whether semantic matching is on. The removed per-rule check from getSemanticConfigError was the only previous guard, and it was conditional on the toggle, which is why the bug existed.

One operational note worth knowing

test_patch_that_only_renames_still_judges_stored_config documents that a rename-only patch is rejected if the stored config is already bad. Users who stored a broken config before this PR (empty keyword rule, router never loading) won't be able to rename or update the deployment without also fixing the config. That's the correct behavior — it forces the operator to fix a permanently broken deployment — but it's a breaking change for those edge-case rows. Since the PR description shows those rows are already un-served, it's the right call; just worth flagging in any support runbook.

Everything else — mutation checks, whitespace handling, edit modal test with semantic toggle OFF, the 400-with-nothing-persisted contract — is solid.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[tin-berri]

bugbot run

[tin-berri]

Good catch on the rename-only patch. A row with a bad config is already un-served, and a patch carrying a valid config clears it (pinned by test_incoming_config_replaces_stored_rather_than_merging), so the only blocked move is renaming a deployment that cannot load either way. Model-string corruption stays restorable.

[tin-berri]

Bugbot's config-only patch finding was real; I reproduced it against a live proxy, where that PATCH took a working router out of service before returning the 500. Two things came out of it. The check now runs on the incoming config whenever a write carries one, so it no longer depends on the payload naming a model; and it is judged on the config alone rather than by classifying the stored model, because that field is encrypted at rest and could never have been classified from the row.

All six write shapes verified live: create valid 200, create empty rule 400, patch with model 400, patch config-only 400, patch config-only valid 200, patch of an unrelated field 200. Nothing persists on the rejections.

bugbot run

@greptileai can you reply here with a score and why that score?

[greptile-apps]

4/5

The fix is correct and well-executed on all fronts. Here is the breakdown.

What is solid:

The three-way fix is internally consistent. serializeKeywordTierRules keeping empty rules (instead of dropping them) is the keystone — it means getKeywordTierRulesError and the payload agree on what row numbers mean, so the error message is accurate. Without that change, the UI check could pass a row the payload drops, or vice versa. The tests prove exactly this with the mutation-check attestation in the PR description.

validate_complexity_router_config_write delegating to ComplexityRouterConfig.model_validate rather than reimplementing the rules is the right call — the boundary rejects exactly what the load would, and extra="allow" means future unknown keys stay the caller's business.

The config-only patch path (incoming_params.model is None) is the fix for the Bugbot finding. Judging it on the config alone, not by classifying a possibly-encrypted stored model, is the only reading that closes the gap regardless of what is stored.

One real edge to note:

The rename path (model present, no config) falls back to the stored config. That is correct — the test test_patch_that_only_renames_still_judges_stored_config confirms it. But it means users who somehow already have a bad config stored (pre-validation rows) are now blocked from making any model-naming update until they also repair the config. The error message names the violation, so it is recoverable, but it is a behaviorally new rejection for a class of existing rows. Given the rule against backwards-incompatible surprises without a flag, that is worth noting even if the breakage is narrow and intentional.

Tests:

Coverage is thorough. All six live write shapes verified, mutation checks confirm the guards are load-bearing, and the edit-modal test correctly uses semantic matching off to hit the pre-fix gap.

The one point off is the rename-blocks-existing-bad-config edge case being a silent behavioral change for stored rows rather than a graceful migration path. Everything else is clean.

[tin-berri]

Took the 4/5 point. A config is now judged only when the write actually carries one, so a rename never inherits a stored config it does not touch; that also keeps the restore path this function documents intact. The repair for a pre-existing bad row is a write that supplies a good config, which is covered.

Verified live on a fresh database: create valid 200, create empty rule 400, patch with model 400, patch config-only 400, patch config-only valid 200, patch rpm 200, rename-only against a bad stored config 200. Backend suites pass at 397.

bugbot run

@greptileai can you reply here with a score and why that score?

[greptile-apps]

5/5.

The concern that kept it at 4/5 is fully resolved. Here's the final accounting:

What was resolved:

• _strategy_router_write_violation now checks incoming_params.complexity_router_config independently of the model, so a rename-only patch (model present, no new config) passes through without touching a stored config it doesn't own
• The restore path documented in the docstring is intact: a write against an already-corrupted row can supply a good config to fix it, and a write that touches nothing config-related is never blocked
• Live evidence covers all seven cases: create valid (200), create empty rule (400), patch with model (400), patch config-only (400), patch config-only valid (200), patch rpm (200), rename-only against bad stored config (200)

Implementation is clean throughout:

• validate_complexity_router_config_write delegates to the router's own ComplexityRouterConfig.model_validate rather than duplicating its rules — future validator changes are free
• serializeKeywordTierRules dropping the filter is the right call; keeping empty rows ensures getKeywordTierRulesError row numbers and the payload are always in sync, eliminating the invisible-drop path that was the root bug
• The deferred import (from litellm.router_strategy.complexity_router.config import ComplexityRouterConfig) is the right pattern for the circular-import concern here
• The double-call to validate_complexity_router_config_write (once eagerly in _strategy_router_write_violation, once at the tail of validate_strategy_router_model_write) is harmless — the first short-circuits on invalid, the second is a no-op on valid — and the ordering is intentional: config rejection surfaces before field-shape rejection at the endpoint level, field-shape before content within validate_strategy_router_model_write directly

Test coverage is thorough:
Mutation checks isolate each guard, backend suite at 397, UI suite at 5901 — the fix is well-bounded and the regressions are covered.

[tin-berri]

bugbot run

[cursor]

✅ 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 f52e2c6. Configure here.}

[codspeed-hq]

Merging this PR will not alter performance

✅ 31 untouched benchmarks

---

_{Comparing litellm_lit5133_autorouter_keyword_validation (b647947) with litellm_internal_staging (ba1bde7)¹}

Footnotes

1. No successful run was found on litellm_internal_staging (d4d0bf0) during the generation of this report, so ba1bde7 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

[tin-berri]

Added the inline error on the keyword row, matching how a missing tier already reports. The row turns red with "At least one keyword is required" under it, and both that marker and the submit message read the same emptyKeywordTierRuleIndexes, so the row named can never be a different row from the one marked. The edit modal never passed showValidationErrors, which is why none of its inline errors rendered; it has that state now, so tier, classifier and keyword errors all show there too.

bugbot run

@greptileai can you reply here with a score and why that score?

[greptile-apps]

Score: 5/5

This is a well-executed bug fix. Here's why it earns the top score:

The core fix is correct. validate_complexity_router_config_write calls ComplexityRouterConfig.model_validate — the exact same model the router uses at load time — rather than duplicating its rules. That means the boundary rejects precisely what would be dropped, no more and no less. extra="allow" on that model ensures unknown fields from future callers aren't blocked.

The ordering in _strategy_router_write_violation is right. Config check runs before the model naming check, which is the only reading that covers a patch carrying a config but no model. The early-return on incoming_params is None (not gated on .model is None any more) is the key structural change that opens that path.

The serializer change is the right call. Dropping empty rules was what made the original bug invisible — the round-trip looked clean because the offending row never reached the payload. Keeping empty rules means the validator and the payload agree on what exists, and row numbers in the toast match the rows that light up. Five of the new tests directly verify this contract.

The edit modal fix was a real gap. showValidationErrors was never set in the edit modal, so every inline error (tier, classifier, keyword) silently didn't render. That's now closed.

The "rename only" case (case 7) is correctly preserved. A write that carries no complexity_router_config exits validate_complexity_router_config_write at the None guard, so a rename against a row stored before this validation existed stays legal. That's the documented restore path and the tests cover it explicitly.

Test coverage is solid. The mutation check inventory in the PR description (reverting each guard site fails exactly the tests for that site, not others) is the right kind of evidence. The six backend test cases map directly to the six curl-verified scenarios in the QA runbook.

One minor note: validate_complexity_router_config_write is called twice when a write carries both model and config — once at the top of _strategy_router_write_violation and once at the tail of validate_strategy_router_model_write. In practice the first call short-circuits before the second is reached for invalid configs, and for valid configs calling it twice is harmless. It does mean validate_strategy_router_model_write works correctly as a standalone function too, which the naming tests verify directly. Not worth changing.

[tin-berri]

bugbot run

[cursor]

✅ 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 155e513. Configure here.}

[tin-berri]

Both submit buttons are now withheld while a tier is unfilled or a keyword row is empty, with the reason in a tooltip so a greyed button always explains itself; the disable state and the tooltip read the same value.

That surfaced two things worth calling out. The row error had been gated on a failed submit, which can no longer happen, so it now reports on the invalid state itself. And Enter never committed a typed keyword: the dropdown is deliberately closed, leaving antd nothing for Enter to select, so the word only became a tag on blur, and clicking submit was what supplied that blur. With the button withheld a caller could type a keyword, watch nothing happen and have no way forward, so Enter commits explicitly now.

bugbot run

@greptileai can you reply here with a score and why that score?

[greptile-apps]

Score: 4.5 / 5

This is a well-executed fix that addresses the root cause at both layers rather than papering over it.

What's done well:

• The core design is correct: client-side guard prevents the user from getting into the broken state, and the server-side 400 closes the gap for direct API callers or future regressions. Both layers read from the same source of truth rather than duplicating logic.
• emptyKeywordTierRuleIndexes being shared between the inline row error and the submit-time message is the right call — the row named in the toast and the row that lights up red are guaranteed to match.
• validate_complexity_router_config_write calling ComplexityRouterConfig.model_validate directly rather than reimplementing the rules means the write boundary rejects exactly what the load would, no more, no less.
• The rename-doesn't-judge-stored-config behaviour is important and correctly preserved: blocking a rename on a legacy bad row would trap operators with no way out except a DB edit.
• Test coverage is thorough, including the mutation checks described in the PR description.

One redundant call worth noting:

In _strategy_router_write_violation, validate_complexity_router_config_write is called once unconditionally on incoming_params.complexity_router_config, and then again inside validate_strategy_router_model_write for complexity-router models (the if kind != "complexity": return None branch). When the model is a complexity router and the config is valid, the config is parsed by Pydantic twice. It's harmless — both return None — but the second call is redundant. The first call exists to cover the config-only-patch path (no model in payload); the second is left over from threading the config through validate_strategy_router_model_write. You could drop the final call in validate_strategy_router_model_write and rely solely on the one in _strategy_router_write_violation, since that function always runs before it. Not a blocker, just a minor tidying opportunity.

Minor UX note:

The edit modal's tier guard (every tier empty) differs from the create form's (any tier empty). The comment documents this as intentional — editing a router that was saved with only two tiers filled must stay legal — so the asymmetry is correct, just worth knowing.

Overall the fix is sound and the implementation is clean.

[tin-berri]

Took the redundant-parse note further than the note itself. Dropping the tail call left complexity_router_config as an unused parameter on validate_strategy_router_model_write, so that went too and the function is back to answering only the naming question its name and docstring describe; the config's contents are validate_complexity_router_config_write's alone. Confirmed one ComplexityRouterConfig.model_validate per write now, down from two, and the config-content tests moved onto the function that owns the rule.

All seven write shapes still behave, plus a missing-required-field create to prove the naming check is untouched: create valid 200, create empty rule 400, create missing field 400, patch with model 400, patch config-only 400, patch config-only valid 200, rename-only 200. Backend suites 398.

bugbot run

@greptileai can you reply here with a score and why that score?

[greptile-apps]

Score: 5/5

This is a well-engineered, correctly scoped fix. Here's the breakdown:

Root cause and fix are properly matched. The original bug had two independent failure modes: (1) the client silently dropped empty rules so the operator's config was discarded on a "successful" create, and (2) if the raw payload reached the API, the row was written but the router rejected it at load time, leaving a zombie deployment and a 500. Both paths are now closed.

Backend validation is correct by construction. validate_complexity_router_config_write calls ComplexityRouterConfig.model_validate — the same model the router runs at load time under ignore_invalid_deployments. The boundary rejects precisely what the load would reject, with no risk of false positives from future config keys because the model is extra="allow". Reporting only the first error is standard and acceptable.

The rename escape hatch is preserved correctly. _strategy_router_write_violation only validates complexity_router_config when incoming_params.complexity_router_config is non-None. A rename that doesn't carry a new config is never held hostage by a bad stored one — the PR's case 7 proves this explicitly.

The config-only patch gap is closed. Previously, incoming_params.model is None would short-circuit before the config was ever checked. Now the config is checked before that guard, which is exactly the fix needed for case 4 (dashboard's edit modal sending a config with no model field).

The serializer change is the right call. Reverting to filtering empties out was what made the original bug invisible. Keeping empty rules in the serialized output means validation row numbers and inline error row numbers are derived from the same source (emptyKeywordTierRuleIndexes → serializeKeywordTierRules), so the row that's named and the row that lights up are the same row by construction.

UI UX is handled correctly. The open={false} + custom onInputKeyDown Enter handler is necessary because antd's tag Select only commits a typed word on blur or dropdown selection; with the button disabled while the row is empty, neither path was available. The draft state per-rule is clean and doesn't leak across rules.

Test coverage is strong. The mutation test results described in the PR (reverting each call site fails exactly the tests it should, and nothing else) are the highest-quality signal that the tests actually guard what they claim. Both UI and backend tests cover the specific before/after scenarios documented in the curl runbook.

No issues to flag.

[tin-berri]

bugbot run

[cursor]

✅ 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 b647947. Configure here.}