mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-08-28 05:27:41 +00:00
refactor(llm): resolve request dispatch through the catalog route (#496)
PR 8 of the gateway refactor series (after #493) — the optional closer: `Client` dispatch goes through the route machinery #493 introduced, instead of an inline ad-hoc lookup. ## What's here `Client::resolve_provider`'s hand-rolled catalog hop (`catalog.get(model)` → provider id) becomes `adapter_registry::resolve_route`. Fallback order is byte-identical: explicit `request.provider` wins, then the model's catalog route, then the default provider, then the existing configuration error. This puts route resolution on the live request path, so the route-equivalence table from #493 now pins actual dispatch rather than a helper nothing calls: a new live-dispatch sweep asserts every built-in model's request lands on the provider its route names, alongside explicit-provider-wins and unknown-model-default pins. ## Scope notes - **No public API change** — `resolve_provider` is private; all frozen `Client` methods are untouched. - The route's `codec`/`deployment_id` still aren't handed to adapters: `ProviderAdapter::complete(&Request)` is frozen (prod-implemented in fabro-cli), and every allowed pairing equals the adapter's built-in codec until the feature PRs. This PR is deliberately just the dispatch seam, so the OpenRouter redo's Client-side wiring is a no-op. ## Verification - `cargo nextest run --workspace --no-fail-fast` (post-rebase onto #493's merge): green except the same 5 pre-existing environment-dependent fabro-workflow failures, identical on main - clippy `-D warnings` + pinned-nightly fmt clean - Wire snapshots untouched This closes the refactor series. Remaining: the already-open cost PR (#494), then the feature redos — OpenRouter (#438) and Bedrock (#459). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
23d3644787
commit
eff3a5a9cb
1 changed files with 70 additions and 8 deletions
|
|
@ -6,7 +6,7 @@ use fabro_model::{AdapterKind, Catalog, ProviderId};
|
|||
use tracing::debug;
|
||||
|
||||
use crate::adapter_registry::{
|
||||
AdapterConfig, AdapterKindOptions, OpenAiAdapterOptions, factory_for,
|
||||
self, AdapterConfig, AdapterKindOptions, OpenAiAdapterOptions, factory_for,
|
||||
};
|
||||
use crate::cost;
|
||||
use crate::error::{Error, ProviderErrorKind};
|
||||
|
|
@ -259,18 +259,18 @@ impl Client {
|
|||
)
|
||||
}
|
||||
|
||||
/// Resolve the provider for a request.
|
||||
/// Resolve the provider for a request: an explicit `request.provider`
|
||||
/// wins, then the model's catalog route, then the default provider.
|
||||
fn resolve_provider(&self, request: &Request) -> Result<Arc<dyn ProviderAdapter>, Error> {
|
||||
let catalog_provider = self.catalog.as_ref().and_then(|catalog| {
|
||||
catalog
|
||||
.get(&request.model)
|
||||
.map(|info| info.provider.to_string())
|
||||
});
|
||||
let route = self
|
||||
.catalog
|
||||
.as_ref()
|
||||
.and_then(|catalog| adapter_registry::resolve_route(catalog, &request.model));
|
||||
|
||||
let provider_name = request
|
||||
.provider
|
||||
.as_deref()
|
||||
.or(catalog_provider.as_deref())
|
||||
.or_else(|| route.as_ref().map(|route| route.provider.as_str()))
|
||||
.or(self.default_provider.as_deref())
|
||||
.ok_or_else(|| Error::Configuration {
|
||||
message: "No provider specified and no default provider set".into(),
|
||||
|
|
@ -1577,6 +1577,68 @@ reasoning = false
|
|||
assert_eq!(provider.name(), "acme");
|
||||
}
|
||||
|
||||
/// Build a Client with one registered mock per catalog provider, so
|
||||
/// dispatch tests can observe which provider a request resolves to.
|
||||
async fn client_with_all_catalog_providers(catalog: &Arc<Catalog>) -> Client {
|
||||
let mut client = Client::new(HashMap::new(), None, vec![]);
|
||||
for provider in catalog.providers() {
|
||||
client
|
||||
.register_provider(Arc::new(MockProvider::new(provider.id.as_str(), "ok")))
|
||||
.await
|
||||
.unwrap();
|
||||
}
|
||||
client.catalog = Some(Arc::clone(catalog));
|
||||
client
|
||||
}
|
||||
|
||||
/// Live-dispatch counterpart of the adapter_registry route-equivalence
|
||||
/// table: for every built-in model, `resolve_provider` lands on the same
|
||||
/// provider the resolved route names.
|
||||
#[tokio::test]
|
||||
async fn dispatch_agrees_with_resolve_route_for_every_builtin_model() {
|
||||
let catalog = catalog_with("");
|
||||
let client = client_with_all_catalog_providers(&catalog).await;
|
||||
|
||||
for model in catalog.list(None) {
|
||||
let route = adapter_registry::resolve_route(&catalog, &model.id)
|
||||
.expect("built-in model should resolve to a route");
|
||||
let mut request = test_request();
|
||||
request.model = model.id.clone();
|
||||
|
||||
let provider = client.resolve_provider(&request).unwrap();
|
||||
|
||||
assert_eq!(provider.name(), route.provider.as_str(), "{}", model.id);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn explicit_provider_wins_over_the_model_route() {
|
||||
let catalog = catalog_with("");
|
||||
let client = client_with_all_catalog_providers(&catalog).await;
|
||||
|
||||
let mut request = test_request();
|
||||
request.model = "gpt-5.4-mini".to_string();
|
||||
request.provider = Some("anthropic".to_string());
|
||||
|
||||
let provider = client.resolve_provider(&request).unwrap();
|
||||
|
||||
assert_eq!(provider.name(), "anthropic");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn unknown_model_falls_back_to_default_provider() {
|
||||
let catalog = catalog_with("");
|
||||
let client = client_with_all_catalog_providers(&catalog).await;
|
||||
let default = client.default_provider().unwrap().to_string();
|
||||
|
||||
let mut request = test_request();
|
||||
request.model = "model-not-in-any-catalog".to_string();
|
||||
|
||||
let provider = client.resolve_provider(&request).unwrap();
|
||||
|
||||
assert_eq!(provider.name(), default);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn from_credentials_registers_no_auth_provider_with_extra_headers() {
|
||||
let catalog = catalog_with(
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue