scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-10-09 03:20:56 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

refactor(llm): simplify the Modal provider tests and fixtures

Browse source 
Reduce duplication and over-specification introduced with the Modal
provider, without changing shipped behavior.

- Extract enabled_provider_catalog and assert_deep_tool_round_trip in
  the fabro-llm integration tests. The Poolside, Fireworks, OpenRouter,
  and Modal deep round trips were four near-identical copies.
- Add ApiCredential::with_extra_headers for providers that authenticate
  with request headers instead of an API key.
- Replace the unreachable require_env guards in the Modal e2e test with
  the std::env::var form used by every sibling test, and register
  MODAL_TOKEN_ID and MODAL_TOKEN_SECRET in EnvVars.
- Collapse modal_requires_both_vault_proxy_tokens to a single case. The
  loop rebuilt the whole built-in catalog per iteration.
- Drop tautological and over-specified assertions: the api_key_url doc
  URL, the forced default/probe lookups on a single-model provider, and
  the get_on_provider loop that could not fail.
- Inline the single-use modal_env_catalog fixture and note why it
  overrides the shipped secrets templates.
- Sort the MODAL_* keys in .env.example, and record in modal.toml why
  api_id keeps the Hugging Face capitalization.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-07-28 18:06:07 -04:00
parent 8771c971d7
commit 8e4251bb07
No known key found for this signature in database
8 changed files with 135 additions and 200 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
.env.example
View file

@ -5,10 +5,10 @@ FIREWORKS_API_KEY=
GEMINI_API_KEY=
INCEPTION_API_KEY=
KIMI_API_KEY=
MINIMAX_API_KEY=
MODAL_KIMI_K3_BASE_URL=
MODAL_TOKEN_ID=
MODAL_TOKEN_SECRET=
MINIMAX_API_KEY=
OPENAI_API_KEY=
OPENROUTER_API_KEY=
POOLSIDE_API_KEY=

13
lib/components/fabro-llm/src/client.rs
View file

@ -1207,18 +1207,13 @@ base_url = "{}/v1"
));
let modal = ProviderId::new("modal");
let client = Client::from_credentials(
vec![ApiCredential {
provider: modal.clone(),
auth_header: None,
extra_headers: HashMap::from([
vec![ApiCredential::with_extra_headers(
modal.clone(),
HashMap::from([
("Modal-Key".to_string(), "wk-test".to_string()),
("Modal-Secret".to_string(), "ws-test".to_string()),
]),
base_url: None,
codex_mode: false,
org_id: None,
project_id: None,
}],
)],
catalog,
)
.await

176
lib/components/fabro-llm/tests/integration.rs
View file

@ -40,6 +40,48 @@ fn make_request(model: &str) -> Request {
}
}
/// Build the built-in catalog with `provider` enabled, plus an operator base
/// URL for providers such as Modal that do not ship one.
fn enabled_provider_catalog(provider: &ProviderId, base_url: Option<String>) -> Arc<Catalog> {
let mut settings = LlmCatalogSettings::default();
settings
.providers
.insert(provider.to_string(), ProviderCatalogSettings {
enabled: Some(true),
base_url,
..ProviderCatalogSettings::default()
});
Arc::new(
Catalog::from_builtin_with_overrides(&settings)
.unwrap_or_else(|err| panic!("enabled {provider} catalog should build: {err}")),
)
}
/// Drive the shared deep tool round trip for one catalog offering.
async fn assert_deep_tool_round_trip(
catalog: &Arc<Catalog>,
provider: &ProviderId,
model_id: &str,
credential: ApiCredential,
) {
let client = Arc::new(
Client::from_credentials(vec![credential], Arc::clone(catalog))
.await
.unwrap_or_else(|err| panic!("{provider} client should build from the catalog: {err}")),
);
let model = catalog
.get_on_provider(provider, model_id)
.unwrap_or_else(|| panic!("{provider} {model_id} should be present"));
let outcome = run_model_test(model, ModelTestMode::Deep, client).await;
assert_eq!(
outcome.status,
ModelTestStatus::Ok,
"{provider} {model_id} deep test failed: {:?}",
outcome.error_message
);
}
#[fabro_macros::e2e_test(live("ANTHROPIC_API_KEY"))]
async fn anthropic_complete() {
let api_key = std::env::var(EnvVars::ANTHROPIC_API_KEY).expect("ANTHROPIC_API_KEY must be set");
@ -318,25 +360,11 @@ async fn bedrock_openai_frontier_complete() {
async fn poolside_laguna_xs_deep_tool_round_trip() {
let api_key = std::env::var(EnvVars::POOLSIDE_API_KEY).expect("POOLSIDE_API_KEY must be set");
let provider = ProviderId::new("poolside");
let catalog = Arc::new(Catalog::from_builtin().expect("built-in catalog should be valid"));
let credential = ApiCredential::from_api_key(provider, api_key, &catalog)
let catalog = enabled_provider_catalog(&provider, None);
let credential = ApiCredential::from_api_key(provider.clone(), api_key, &catalog)
.expect("Poolside credential should resolve from the catalog");
let client = Arc::new(
Client::from_credentials(vec![credential], Arc::clone(&catalog))
.await
.expect("Poolside client should build from the catalog"),
);
let model = catalog
.get_on_provider(&ProviderId::new("poolside"), "laguna-xs-2.1")
.expect("direct Poolside Laguna XS should be present");
let outcome = run_model_test(model, ModelTestMode::Deep, client).await;
assert_eq!(
outcome.status,
ModelTestStatus::Ok,
"direct Poolside Laguna XS deep test failed: {:?}",
outcome.error_message
);
assert_deep_tool_round_trip(&catalog, &provider, "laguna-xs-2.1", credential).await;
}
#[fabro_macros::e2e_test(live("OPENROUTER_API_KEY"))]
@ -575,35 +603,11 @@ async fn fireworks_complete() {
async fn fireworks_kimi_k2_7_code_deep_tool_round_trip() {
let api_key = std::env::var(EnvVars::FIREWORKS_API_KEY).expect("FIREWORKS_API_KEY must be set");
let provider = ProviderId::new("fireworks");
let mut settings = LlmCatalogSettings::default();
settings
.providers
.insert(provider.to_string(), ProviderCatalogSettings {
enabled: Some(true),
..ProviderCatalogSettings::default()
});
let catalog = Arc::new(
Catalog::from_builtin_with_overrides(&settings)
.expect("enabled Fireworks catalog should build"),
);
let credential = ApiCredential::from_api_key(provider, api_key, &catalog)
let catalog = enabled_provider_catalog(&provider, None);
let credential = ApiCredential::from_api_key(provider.clone(), api_key, &catalog)
.expect("Fireworks credential should resolve from the catalog");
let client = Arc::new(
Client::from_credentials(vec![credential], Arc::clone(&catalog))
.await
.expect("Fireworks client should build from the catalog"),
);
let model = catalog
.get_on_provider(&ProviderId::new("fireworks"), "kimi-k2.7-code")
.expect("Fireworks Kimi K2.7 Code should be present");
let outcome = run_model_test(model, ModelTestMode::Deep, client).await;
assert_eq!(
outcome.status,
ModelTestStatus::Ok,
"Fireworks Kimi K2.7 Code deep test failed: {:?}",
outcome.error_message
);
assert_deep_tool_round_trip(&catalog, &provider, "kimi-k2.7-code", credential).await;
}
#[fabro_macros::e2e_test(live("OPENROUTER_API_KEY"))]
@ -611,35 +615,11 @@ async fn openrouter_kimi_k3_deep_tool_round_trip() {
let api_key =
std::env::var(EnvVars::OPENROUTER_API_KEY).expect("OPENROUTER_API_KEY must be set");
let provider = ProviderId::new("openrouter");
let mut settings = LlmCatalogSettings::default();
settings
.providers
.insert(provider.to_string(), ProviderCatalogSettings {
enabled: Some(true),
..ProviderCatalogSettings::default()
});
let catalog = Arc::new(
Catalog::from_builtin_with_overrides(&settings)
.expect("enabled OpenRouter catalog should build"),
);
let credential = ApiCredential::from_api_key(provider, api_key, &catalog)
let catalog = enabled_provider_catalog(&provider, None);
let credential = ApiCredential::from_api_key(provider.clone(), api_key, &catalog)
.expect("OpenRouter credential should resolve from the catalog");
let client = Arc::new(
Client::from_credentials(vec![credential], Arc::clone(&catalog))
.await
.expect("OpenRouter client should build from the catalog"),
);
let model = catalog
.get_on_provider(&ProviderId::new("openrouter"), "kimi-k3")
.expect("OpenRouter Kimi K3 should be present");
let outcome = run_model_test(model, ModelTestMode::Deep, client).await;
assert_eq!(
outcome.status,
ModelTestStatus::Ok,
"OpenRouter Kimi K3 deep test failed: {:?}",
outcome.error_message
);
assert_deep_tool_round_trip(&catalog, &provider, "kimi-k3", credential).await;
}
#[fabro_macros::e2e_test(
@ -648,56 +628,22 @@ async fn openrouter_kimi_k3_deep_tool_round_trip() {
live("MODAL_TOKEN_SECRET")
)]
async fn modal_kimi_k3_deep_tool_round_trip() {
let Some(base_url) = fabro_test::require_env("MODAL_KIMI_K3_BASE_URL") else {
return;
};
let Some(token_id) = fabro_test::require_env("MODAL_TOKEN_ID") else {
return;
};
let Some(token_secret) = fabro_test::require_env("MODAL_TOKEN_SECRET") else {
return;
};
let base_url =
std::env::var("MODAL_KIMI_K3_BASE_URL").expect("MODAL_KIMI_K3_BASE_URL must be set");
let token_id = std::env::var(EnvVars::MODAL_TOKEN_ID).expect("MODAL_TOKEN_ID must be set");
let token_secret =
std::env::var(EnvVars::MODAL_TOKEN_SECRET).expect("MODAL_TOKEN_SECRET must be set");
let provider = ProviderId::new("modal");
let mut settings = LlmCatalogSettings::default();
settings
.providers
.insert(provider.to_string(), ProviderCatalogSettings {
enabled: Some(true),
base_url: Some(base_url),
..ProviderCatalogSettings::default()
});
let catalog = Arc::new(
Catalog::from_builtin_with_overrides(&settings)
.expect("enabled Modal catalog should build"),
);
let credential = ApiCredential {
provider: provider.clone(),
auth_header: None,
extra_headers: HashMap::from([
let catalog = enabled_provider_catalog(&provider, Some(base_url));
let credential = ApiCredential::with_extra_headers(
provider.clone(),
HashMap::from([
("Modal-Key".to_string(), token_id),
("Modal-Secret".to_string(), token_secret),
]),
base_url: None,
codex_mode: false,
org_id: None,
project_id: None,
};
let client = Arc::new(
Client::from_credentials(vec![credential], Arc::clone(&catalog))
.await
.expect("Modal client should build from the catalog"),
);
let model = catalog
.get_on_provider(&provider, "kimi-k3")
.expect("Modal Kimi K3 should be present");
let outcome = run_model_test(model, ModelTestMode::Deep, client).await;
assert_eq!(
outcome.status,
ModelTestStatus::Ok,
"Modal Kimi K3 deep test failed: {:?}",
outcome.error_message
);
assert_deep_tool_round_trip(&catalog, &provider, "kimi-k3", credential).await;
}
async fn run_multi_turn_cache_test(

29
lib/foundation/fabro-auth/src/env_source.rs
View file

@ -195,20 +195,6 @@ reasoning_effort = "levels"
))
}
fn modal_env_catalog() -> Catalog {
catalog_with(
r#"
[providers.modal]
enabled = true
base_url = "https://example--kimi-k3.modal.run/v1"
[providers.modal.extra_headers]
"Modal-Key" = "{{ env.MODAL_TOKEN_ID }}"
"Modal-Secret" = "{{ env.MODAL_TOKEN_SECRET }}"
"#,
)
}
#[tokio::test]
async fn configured_providers_reads_injected_env() {
let source = test_source(&[("ANTHROPIC_API_KEY", "anthropic-key")]);
@ -347,7 +333,20 @@ x-portkey-provider = "@bedrock-prod"
#[tokio::test]
async fn env_source_resolves_modal_proxy_headers_when_explicitly_configured() {
let catalog = modal_env_catalog();
// The shipped `modal.toml` reads `{{ secrets.* }}`, which this source
// cannot resolve. Operators who want env-backed Modal credentials must
// override both header sources, as documented in `reference/sdk.mdx`.
let catalog = catalog_with(
r#"
[providers.modal]
enabled = true
base_url = "https://example--kimi-k3.modal.run/v1"
[providers.modal.extra_headers]
"Modal-Key" = "{{ env.MODAL_TOKEN_ID }}"
"Modal-Secret" = "{{ env.MODAL_TOKEN_SECRET }}"
"#,
);
let source = test_source(&[
("MODAL_TOKEN_ID", "wk-test"),
("MODAL_TOKEN_SECRET", "ws-test"),

78
lib/foundation/fabro-auth/src/resolve.rs
View file

@ -70,6 +70,24 @@ impl ApiCredential {
project_id: None,
})
}
/// Build an `ApiCredential` for a provider that authenticates with request
/// headers instead of an API key, such as Modal's proxy-token pair.
#[must_use]
pub fn with_extra_headers(
provider: impl Into<ProviderId>,
extra_headers: HashMap<String, String>,
) -> Self {
Self {
provider: provider.into(),
auth_header: None,
extra_headers,
base_url: None,
codex_mode: false,
org_id: None,
project_id: None,
}
}
}
const OPENAI_CODEX_BASE_URL: &str = "https://chatgpt.com/backend-api/codex";
@ -970,7 +988,6 @@ reasoning = false
.unwrap();
let ResolvedCredential::Api(api) = resolved;
assert_eq!(api.provider, modal);
assert!(api.auth_header.is_none());
assert_eq!(
api.extra_headers,
@ -986,40 +1003,35 @@ reasoning = false
}
#[tokio::test]
async fn modal_requires_both_vault_proxy_tokens() {
for (present_name, present_value, missing_name) in [
("MODAL_TOKEN_ID", "wk-present", "MODAL_TOKEN_SECRET"),
("MODAL_TOKEN_SECRET", "ws-present", "MODAL_TOKEN_ID"),
] {
let catalog = modal_catalog();
let dir = tempfile::tempdir().unwrap();
let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap();
vault_set_token(&mut vault, present_name, present_value).unwrap();
let resolver = test_resolver(vault, Arc::new(|_| None));
let modal = ProviderId::new("modal");
async fn modal_is_not_configured_with_only_one_vault_proxy_token() {
let catalog = modal_catalog();
let dir = tempfile::tempdir().unwrap();
let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap();
vault_set_token(&mut vault, "MODAL_TOKEN_ID", "wk-present").unwrap();
let resolver = test_resolver(vault, Arc::new(|_| None));
let modal = ProviderId::new("modal");
{
let vault = resolver.vault.read().await;
assert!(
!resolver
.configured_providers(&vault, &catalog)
.contains(&modal)
);
}
let err = resolver
.resolve(modal.clone(), CredentialUsage::ApiRequest, &catalog)
.await
.unwrap_err();
assert!(matches!(
err,
ResolveError::Interpolation { ref provider, .. } if provider == &modal
));
let message = err.to_string();
assert!(message.contains(missing_name));
assert!(!message.contains(present_value));
{
let vault = resolver.vault.read().await;
assert!(
!resolver
.configured_providers(&vault, &catalog)
.contains(&modal)
);
}
let err = resolver
.resolve(modal.clone(), CredentialUsage::ApiRequest, &catalog)
.await
.unwrap_err();
assert!(matches!(
err,
ResolveError::Interpolation { ref provider, .. } if provider == &modal
));
let message = err.to_string();
assert!(message.contains("MODAL_TOKEN_SECRET"));
assert!(!message.contains("wk-present"));
}
#[tokio::test]

30
lib/foundation/fabro-model/src/catalog.rs
View file

@ -3515,11 +3515,6 @@ enabled = true
assert_eq!(provider.billing_policy, BillingPolicy::OpenAi);
assert_eq!(provider.priority, 30);
assert!(provider.auth.is_none());
assert!(provider.base_url.is_none());
assert_eq!(
provider.api_key_url.as_deref(),
Some("https://modal.com/docs/guide/endpoints#proxy-tokens")
);
assert_eq!(
provider.extra_headers,
HashMap::from([
@ -3533,19 +3528,10 @@ enabled = true
),
])
);
assert_eq!(
catalog
.default_for_provider(&modal)
.map(|model| model.id.as_str()),
Some("kimi-k3")
);
assert_eq!(
catalog
.probe_for_provider(&modal)
.map(|model| model.id.as_str()),
Some("kimi-k3")
);
// Modal assigns the endpoint URL per deployment, so the built-in entry
// ships without one and the operator supplies it through settings.
assert!(provider.base_url.is_none());
let catalog = Catalog::from_builtin_with_overrides(&minimal_settings(
r#"
[providers.modal]
@ -3642,7 +3628,7 @@ enabled = true
}
#[test]
fn builtin_kimi_k3_slug_is_portable_across_direct_and_gateway_providers() {
fn builtin_kimi_k3_selection_prefers_direct_kimi_then_modal_over_openrouter() {
let kimi = ProviderId::new("kimi");
let modal = ProviderId::new("modal");
let openrouter = ProviderId::new("openrouter");
@ -3657,14 +3643,6 @@ enabled = true
))
.expect("enabled Modal and OpenRouter overrides should build");
for provider in [&kimi, &modal, &openrouter] {
let model = catalog
.get_on_provider(provider, "kimi-k3")
.unwrap_or_else(|| panic!("Kimi K3 should resolve on provider '{provider}'"));
assert_eq!(model.id, "kimi-k3", "{provider}");
assert_eq!(&model.provider, provider, "{provider}");
}
let selected = catalog
.select(
"kimi-k3",

5
lib/foundation/fabro-model/src/catalog/providers/modal.toml
View file

@ -11,7 +11,7 @@ enabled = false
"Modal-Secret" = "{{ secrets.MODAL_TOKEN_SECRET }}"
# Modal assigns an endpoint URL when the Shared API or an Auto Endpoint is
# created. To enable Modal, add the endpoint URL to settings.toml:
# created. To enable Modal, add the endpoint URL to ~/.fabro/settings.toml:
#
# [llm.providers.modal]
# enabled = true
@ -22,6 +22,9 @@ enabled = false
# fabro secret set MODAL_TOKEN_ID wk-...
# fabro secret set MODAL_TOKEN_SECRET ws-...
# Modal serves the Hugging Face repository id, so `api_id` keeps that
# capitalization. OpenRouter routes the same model under its own lowercase
# slug (`moonshotai/kimi-k3`).
[providers.modal.models."kimi-k3"]
api_id = "moonshotai/Kimi-K3"
display_name = "Kimi K3 (via Modal)"

2
lib/foundation/fabro-static/src/env_vars.rs
View file

@ -56,6 +56,8 @@ impl EnvVars {
pub const INCEPTION_API_KEY: &'static str = "INCEPTION_API_KEY";
pub const KIMI_API_KEY: &'static str = "KIMI_API_KEY";
pub const MINIMAX_API_KEY: &'static str = "MINIMAX_API_KEY";
pub const MODAL_TOKEN_ID: &'static str = "MODAL_TOKEN_ID";
pub const MODAL_TOKEN_SECRET: &'static str = "MODAL_TOKEN_SECRET";
pub const OPENAI_API_KEY: &'static str = "OPENAI_API_KEY";
pub const OPENAI_BASE_URL: &'static str = "OPENAI_BASE_URL";
pub const OPENAI_ORGANIZATION: &'static str = "OPENAI_ORGANIZATION";