From 024e94f9d5e399ff5292ef6caac4b06bf6ff30b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20LIARD?= Date: Sun, 26 Apr 2026 20:40:03 +0200 Subject: [PATCH] fix(routing): auto-map respects explicit [[models]] entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Router::route` step 3 (auto-mapping) was rewriting `request.model` to `router.default` for any name matching `auto_map_regex` — even when the user had an explicit `[[models]]` virtual entry with that name. The virtual entry's fallback chain was therefore bypassed entirely; in practice the virtual name `default-model` then leaked to pass-through providers downstream, producing errors like: Provider API error: 400 - {"error":{"message":"default-model is not a valid model ID","code":400},"user_id":"user_..."} The error surfaces specifically when: 1. Client sends `claude-sonnet-4-6`. 2. Auto-map regex `^claude-` matches → `request.model` rewritten to `default-model`. 3. The user's explicit `[[models]] name = "claude-sonnet-4-6"` with fallbacks (anthropic native → OR `anthropic/claude-sonnet-4.6` → `deepseek-v4-flash`) is never consulted. 4. `default-model` chain runs; on exhaustion `try_direct_provider_lookup` forwards `model = "default-model"` to a pass-through provider (OpenRouter), which 400s. Fix: skip the auto-map rewrite when `config.models` already contains an entry whose name equals `request.model`. The check is regex-agnostic, so it covers every auto_map_regex the user might configure (claude, gpt, gemini, mixed, etc.) — there is no provider-specific code path here. Auto-map continues to rewrite the model when no explicit virtual exists, preserving the original "catch-all rewrite for unconfigured models" intent that `test_auto_map_claude_models` and `test_auto_map_custom_regex` exercise. Adds two regression tests: - `test_auto_map_skips_explicit_virtual_model` — the new behaviour. - `test_auto_map_still_rewrites_unmapped_claude` — counter-test confirming the original catch-all path is intact. Tests: 9/9 auto-map tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/routing/classify/mod.rs | 30 +++++++++++++++++++----- src/routing/classify/tests.rs | 44 +++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/src/routing/classify/mod.rs b/src/routing/classify/mod.rs index 9556ca6..3cb9f67 100644 --- a/src/routing/classify/mod.rs +++ b/src/routing/classify/mod.rs @@ -312,14 +312,32 @@ impl Router { } } - // 3. Auto-mapping (model name transformation, after background check) + // 3. Auto-mapping (model name transformation, after background check). + // + // If the user has an explicit `[[models]]` entry whose name matches + // the incoming request model, that entry takes precedence — auto-map + // is a "catch-all rewrite for unconfigured models", not a forced + // override. Without this guard, a request for `claude-sonnet-4-6` + // would be rewritten to `router.default` even when the user defined + // a virtual `claude-sonnet-4-6` model with its own fallback chain; + // the chain would never run and pass-through would leak the virtual + // name "default-model" to the upstream provider. if let Some(ref mapper) = self.auto_mapper { if mapper.is_match(&request.model) { - info!( - "Auto-mapped model '{}' → '{}'", - request.model, self.config.router.default - ); - request.model.clone_from(&self.config.router.default); + let has_explicit_virtual = + self.config.models.iter().any(|m| m.name == request.model); + if has_explicit_virtual { + info!( + "Auto-map skipped for '{}' — explicit [[models]] entry takes precedence", + request.model + ); + } else { + info!( + "Auto-mapped model '{}' → '{}'", + request.model, self.config.router.default + ); + request.model.clone_from(&self.config.router.default); + } } } diff --git a/src/routing/classify/tests.rs b/src/routing/classify/tests.rs index 75a68da..e75c6da 100644 --- a/src/routing/classify/tests.rs +++ b/src/routing/classify/tests.rs @@ -200,6 +200,50 @@ fn test_auto_map_custom_regex() { assert_eq!(decision.model_name, "default.model"); // Auto-mapped to default } +#[test] +fn test_auto_map_skips_explicit_virtual_model() { + // Regression: a request that matches the auto_map regex must NOT be + // rewritten to `router.default` when the user has an explicit + // `[[models]]` entry with the same name. Without this guard the + // virtual entry's fallback chain would be bypassed entirely and the + // virtual name would leak to a pass-through provider downstream. + use crate::cli::ModelConfig; + + let mut config = create_test_config(); + // Add a virtual model entry with the same name as the incoming model. + config.models.push(ModelConfig { + name: "claude-sonnet-4-6".to_string(), + mappings: vec![], + budget_usd: None, + strategy: Default::default(), + fan_out: None, + deprecated: None, + }); + let router = Router::new(config); + + let mut request = create_simple_request("Hello"); + request.model = "claude-sonnet-4-6".to_string(); + + let decision = router.route(&mut request).unwrap(); + assert_eq!(decision.route_type, RouteType::Default); + // Must use the explicit virtual name, NOT the auto-mapped default. + assert_eq!(decision.model_name, "claude-sonnet-4-6"); +} + +#[test] +fn test_auto_map_still_rewrites_unmapped_claude() { + // Counter-test: when the user has no `[[models]]` entry for the + // incoming claude-* name, auto-map continues to rewrite as before. + let config = create_test_config(); + let router = Router::new(config); + + let mut request = create_simple_request("Hello"); + request.model = "claude-some-unmapped-variant".to_string(); + + let decision = router.route(&mut request).unwrap(); + assert_eq!(decision.model_name, "default.model"); +} + #[test] fn test_no_auto_map_non_matching() { let config = create_test_config();