fix(embedding): from_parts honors an explicit model for the mock provider

`EmbeddingConfig::from_parts(Some("mock"), _, Some(model), _)` short-circuited to
`Self::mock()` and silently dropped the provided `model`, falling back to
`OMNIGRAPH_EMBED_MODEL`. A cluster `embeddings` profile that sets
`provider: mock, model: X` (RFC-012 Phase 5) would therefore not resolve to X —
the query-time same-space check (`exec/query.rs`) compares the recorded model
against the resolved one, so a dropped model makes that check fire on the wrong
value (a spurious mismatch, or env-dependent). Mock vectors are model-independent
so this is not silent cross-space ranking, but it breaks the contract `from_parts`
documents ("model defaults exactly as from_env does") and the cluster path that
Phase 5 wiring will feed it.

Honor an explicit `model` for mock; fall back to `mock()`'s env-based model only
when none is supplied. Regression: `from_parts_mock_honors_an_explicit_model`
(red before this change).

Addresses the bot-review finding on #248.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
aaltshuler 2026-06-16 01:14:01 +03:00
parent 7d412988b5
commit 1498ed821e

View file

@ -113,7 +113,16 @@ impl EmbeddingConfig {
api_key: String,
) -> Result<Self> {
if provider == Some("mock") {
return Ok(Self::mock());
// An explicit `model` (e.g. a cluster `embeddings` profile) is
// authoritative — it is what the same-space check compares against —
// so honor it; fall back to `mock()`'s env-based model only when the
// caller supplied none. Without this, a profile's `model` is silently
// dropped and the same-space check resolves to OMNIGRAPH_EMBED_MODEL.
let mut config = Self::mock();
if let Some(model) = model {
config.model = model;
}
return Ok(config);
}
let (provider, default_base, default_model, _key_envs) = provider_profile(provider)?;
let base_url = base_url
@ -939,6 +948,25 @@ mod tests {
);
}
#[test]
#[serial]
fn from_parts_mock_honors_an_explicit_model() {
// A cluster `embeddings` profile that sets `provider: mock, model: X`
// must resolve to model X — it is what the query-time same-space check
// compares against. Env cleared so the assertion isolates the arg.
let _guard = cleared_env(&[]);
let pinned =
EmbeddingConfig::from_parts(Some("mock"), None, Some("recorded-x".to_string()), String::new())
.unwrap();
assert_eq!(pinned.provider, Provider::Mock);
assert_eq!(pinned.model, "recorded-x");
// With no explicit model, mock falls back to its env-based default (here
// empty, since the env is cleared).
let bare = EmbeddingConfig::from_parts(Some("mock"), None, None, String::new()).unwrap();
assert_eq!(bare.provider, Provider::Mock);
assert_eq!(bare.model, "");
}
#[test]
#[serial]
fn from_env_openai_compatible_prefers_openrouter_key() {