Skip to content

fix(auto): honor cost_saving in inventory routing#4486

Merged
Hmbown merged 1 commit into
mainfrom
agent/091-issue4405-cost-saving
Jul 17, 2026
Merged

fix(auto): honor cost_saving in inventory routing#4486
Hmbown merged 1 commit into
mainfrom
agent/091-issue4405-cost-saving

Conversation

@Hmbown

@Hmbown Hmbown commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Summary

  • wire the parsed auto.cost_saving opt-in into the provider-aware heuristic
  • tell the network classifier the exact validated strong and fast sibling pair
  • keep providers without a known fast sibling on their active model
  • derive fallback candidates from the fallback provider, preventing cross-provider model invention
  • leave default routing and child-agent model-strength policy unchanged

Verification

  • cargo fmt --all -- --check
  • git diff --check
  • cargo check -p codewhale-tui --locked
  • focused model-routing tests: 22 passed
  • full TUI binary suite before the final provider-fallback follow-up: 7,123 passed, 2 ignored
  • cargo build -p codewhale-tui --locked
  • exact fallback-provider regression passed after the follow-up

Refs #4405

This is intentionally a partial fix. Effective-pair diagnostics and per-turn strong versus fast route receipts remain in #4405.

The inventory Auto resolver retained the parsed cost_saving setting but called the balanced heuristic and emitted no classifier policy for it.

Feed the setting into the shared provider-aware heuristic and describe the exact runnable strong/fast pair to the classifier. Providers without a known sibling stay on their active route, and child model-strength policy is unchanged.

Add regression coverage proving direct Z.ai changes from GLM-5.2 to GLM-5-Turbo for a borderline request only when cost saving is enabled.

Refs #4405

Signed-off-by: Hunter B <hmbown@gmail.com>
Copilot AI review requested due to automatic review settings July 17, 2026 19:13
@cursor

cursor Bot commented Jul 17, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@Hmbown Hmbown added this to the v0.9.1 milestone Jul 17, 2026
@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Hmbown's task in 2m 53s —— View job


Review: fix(auto): honor cost_saving in inventory routing

Summary

This is a solid, well-scoped fix. The core change replaces provider_router_candidates(config.api_provider(), &active.model) with provider_router_candidates(active.provider, &active.model) in auto_route_from_inventory_heuristic (crates/tui/src/model_routing.rs:447).

That single-argument swap actually fixes a real latent bug beyond just wiring cost_saving: when ModelInventory::active_default() falls back to self.candidates.first() (the "active provider has no auth, route to whatever authenticated provider is first in the inventory" case, e.g. configured for zai without ZAI_API_KEY but DEEPSEEK_API_KEY set), the old code paired the configured provider (zai) with the fallback model (deepseek-v4-flash). Since provider_router_candidates(Zai, "deepseek-v4-flash") normalizes to a cheap: None pair (the string doesn't match any known Zai model), router_candidates.cheap.is_some() was false, so the heuristic branch was skipped entirely and chosen was pinned to active.model — meaning auto-mode escalation/de-escalation silently stopped working whenever a fallback provider was in play, regardless of cost_saving. Using active.provider fixes this for both balanced and cost-saving modes. Good catch, and the new inventory_auto_route_uses_fallback_candidates_from_their_provider test (model_routing.rs:894) pins this down well.

Provider/model safety (EPIC #2608)

No concerns. The cost-saving prompt addition in inventory_auto_router_system_prompt (model_routing.rs:516) only names a strong/fast pair to the classifier after confirming both ids exist in the inventory via inventory.candidate(active.provider, ...) (model_routing.rs:530-534) — it never infers a provider switch from a model-string prefix. The classifier's response is still independently validated against the inventory in parse_inventory_auto_route_recommendation (model_routing.rs:569), so even a hallucinated provider/model pair from the LLM classifier is rejected. When no validated sibling exists, the prompt explicitly instructs "do not invent a model or cross-provider downgrade" (model_routing.rs:548-551) rather than silently omitting guidance. This matches the stated invariant well.

Correctness

  • The heuristic bias logic in auto_model_heuristic_with_bias_for_candidates (model_routing.rs:150) is correct: cost_saving both suppresses the borderline-keyword escalation (implement/analyze/CJK equivalents) and raises the long-input escalation threshold from 500 to 1,000 chars. Traced through the new inventory_auto_route_cost_saving_changes_borderline_zai_route test manually — the "Please implement a binary search" prompt correctly resolves to GLM-5.2 (balanced) vs GLM-5-Turbo (cost-saving).
  • ZAI_GLM_5_2_MODEL/ZAI_GLM_5_TURBO_MODEL constants (config/models.rs:148-149) match the string literals asserted in the new tests exactly.

Minor / non-blocking observation

  • crates/tui/src/tui/ui.rs:7958 has a legacy fallback (auto_model_heuristic(&message.display, &app.model)) that still hardcodes RouterCandidates::deepseek() and never consults cost_saving. Tracing should_resolve_auto_model_selection (tui/auto_router.rs:19, just app.auto_model), this branch appears unreachable in practice today since it's gated by the same app.auto_model flag that already guarantees auto_selection is Some(..) (or the turn errors out first) — so this is likely dead code rather than a live gap. Worth a follow-up cleanup, but out of scope for this PR and not something this diff needs to touch.

Tests

Coverage is good: provider-aware candidate derivation, prompt content for the validated pair vs. no-sibling case, fallback-provider heuristic routing, and the balanced-vs-cost-saving borderline-keyword routing decision are all exercised. No gaps identified for the stated scope of this partial fix (per-turn strong/fast route receipts are explicitly deferred to #4405, which is reasonable).

Verdict

No blocking issues found. This is a clean, well-tested fix that also resolves an unrelated latent bug (fallback-provider heuristic routing getting stuck) as a side effect of the correct fix. Note: I was unable to execute cargo test/cargo check in this sandboxed environment to independently re-run the verification steps listed in the PR description — the analysis above is from static tracing of the diff and surrounding code.

@Hmbown
Hmbown merged commit 7d64a3c into main Jul 17, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants