diff --git a/crates/tinymemory-module/src/embedding.rs b/crates/tinymemory-module/src/embedding.rs index c9790d2..ac00e39 100644 --- a/crates/tinymemory-module/src/embedding.rs +++ b/crates/tinymemory-module/src/embedding.rs @@ -121,12 +121,14 @@ impl EmbeddingHost for BusEmbeddingHost { self.ollama_base_url.clone() } + /// The host's default is its managed-cloud embedder built from the same + /// two values it sent as `cloud_embedding_model`/`cloud_embedding_dimensions`, + /// so this provider is that embedder and names itself `cloud`: the name + /// travels first on every `Embed` call and is what the host selects the + /// credential and endpoint by. An invented label here (`module-bus`, once) + /// reaches the host as an unknown provider slug and fails every batch. fn default_embedding_provider(&self) -> Arc { - Arc::new(self.provider( - "module-bus", - &self.cloud_model.clone(), - self.cloud_dimensions, - )) + Arc::new(self.provider("cloud", &self.cloud_model.clone(), self.cloud_dimensions)) } /// Builds a provider for an explicit triple, ignoring `api_key`. @@ -257,10 +259,23 @@ impl EmbeddingProvider for BusEmbeddingProvider { self.dimensions ); + // Four positional arguments, in the order the host's `EmbeddingHost` + // interface declares them: `(provider, model, dimensions, texts)`. The + // host needs the provider name first because it is what selects the + // credential and endpoint (`cloud` is its managed embedder, `ollama` + // its local daemon, anything else a BYO-key slug). Sending three + // arguments shifted `dimensions` into `model` and every embed was + // refused at decode with "invalid type: integer, expected a string" — + // nothing ingested in module mode ever got a vector (openhuman#5820). let vectors: Vec> = proxy .call( EMBED_METHOD, - (self.model_id.clone(), self.dimensions, owned), + ( + self.name.clone(), + self.model_id.clone(), + self.dimensions, + owned, + ), ) .await .map_err(|error| anyhow::anyhow!("host embed failed: {error}"))?; diff --git a/crates/tinymemory-module/src/embedding_test.rs b/crates/tinymemory-module/src/embedding_test.rs index b42a8b1..2652c2c 100644 --- a/crates/tinymemory-module/src/embedding_test.rs +++ b/crates/tinymemory-module/src/embedding_test.rs @@ -6,7 +6,7 @@ //! unsearchable without a re-embed, and nothing fails at the time. So the //! provider checks, and these tests are what prove it checks. -use std::sync::Arc; +use std::sync::{Arc, Mutex}; use tinybus::broker::Broker; use tinybus::transport::memory::MemoryBus; @@ -17,28 +17,59 @@ use super::{BusEmbeddingHost, EMBEDDING_HOST_BUS_NAME, EMBEDDING_HOST_OBJECT_PAT use crate::config::ModuleConfig; /// A stand-in for the host's embedder, returning vectors of a chosen width. +/// +/// Its `Embed` takes the four positional arguments the real host declares, in +/// the host's order — `(provider, model, dimensions, texts)` — because argument +/// order is the part of a bus contract no compiler checks. A fake with the +/// wrong arity would pass every test here while the real host refused every +/// call at decode, which is exactly how the module shipped sending three +/// (openhuman#5820). struct FakeHostEmbedder { /// Width of each returned vector. Set to something other than the requested /// dimensionality to exercise the mismatch refusal. width: usize, /// Return this many vectors regardless of input count, when `Some`. force_count: Option, + /// Every `(provider, model, dimensions)` triple the host was asked for. + seen: Arc>>, } #[tinybus::interface(name = "ai.tinyhumans.tinymemory.EmbeddingHost")] impl FakeHostEmbedder { async fn embed( &self, - _model: String, - _dimensions: usize, + provider: String, + model: String, + dimensions: usize, texts: Vec, ) -> BusResult>> { std::future::ready(()).await; + self.seen + .lock() + .expect("seen lock") + .push((provider, model, dimensions)); let count = self.force_count.unwrap_or(texts.len()); Ok((0..count).map(|_| vec![0.5_f32; self.width]).collect()) } } +impl FakeHostEmbedder { + fn with_width(width: usize) -> Self { + Self { + width, + force_count: None, + seen: Arc::new(Mutex::new(Vec::new())), + } + } + + fn forcing_count(width: usize, count: usize) -> Self { + Self { + force_count: Some(count), + ..Self::with_width(width) + } + } +} + /// Bring up a bus with `embedder` served at the host's well-known name. /// /// The broker task is leaked deliberately: it lives as long as the test, and @@ -82,11 +113,7 @@ fn config_with_dims(dims: usize) -> ModuleConfig { #[tokio::test] async fn a_batch_is_embedded_over_the_bus() { - let connection = bus_with_host(FakeHostEmbedder { - width: 4, - force_count: None, - }) - .await; + let connection = bus_with_host(FakeHostEmbedder::with_width(4)).await; let host = BusEmbeddingHost::new(connection, &config_with_dims(4)); let provider = host.default_embedding_provider(); @@ -104,11 +131,7 @@ async fn a_wrong_width_is_refused_rather_than_written() { // The dangerous case. Accepting these would write vectors into a space they // do not belong to, and nothing would fail until a later search silently // returned nothing. - let connection = bus_with_host(FakeHostEmbedder { - width: 8, - force_count: None, - }) - .await; + let connection = bus_with_host(FakeHostEmbedder::with_width(8)).await; let host = BusEmbeddingHost::new(connection, &config_with_dims(4)); let provider = host.default_embedding_provider(); @@ -123,11 +146,7 @@ async fn a_wrong_width_is_refused_rather_than_written() { async fn a_wrong_vector_count_is_refused() { // Callers pair inputs with outputs positionally, so a short reply would // attach the wrong vector to the wrong chunk. - let connection = bus_with_host(FakeHostEmbedder { - width: 4, - force_count: Some(1), - }) - .await; + let connection = bus_with_host(FakeHostEmbedder::forcing_count(4, 1)).await; let host = BusEmbeddingHost::new(connection, &config_with_dims(4)); let provider = host.default_embedding_provider(); @@ -142,11 +161,7 @@ async fn a_wrong_vector_count_is_refused() { async fn a_zero_dimension_provider_yields_empty_vectors() { // Zero dimensions is the engine's "semantic search off" state, and the // vectors it yields are expected to be empty rather than merely unchecked. - let connection = bus_with_host(FakeHostEmbedder { - width: 0, - force_count: None, - }) - .await; + let connection = bus_with_host(FakeHostEmbedder::with_width(0)).await; let host = BusEmbeddingHost::new(connection, &config_with_dims(0)); let provider = host.default_embedding_provider(); @@ -166,11 +181,7 @@ async fn a_zero_dimension_request_answered_with_real_vectors_is_refused() { // then believe no vectors existed while the store filled with embeddings // from a space nothing tracks — the split-space failure, with the split // hidden. Zero means empty, and this is the test that says so. - let connection = bus_with_host(FakeHostEmbedder { - width: 768, - force_count: None, - }) - .await; + let connection = bus_with_host(FakeHostEmbedder::with_width(768)).await; let host = BusEmbeddingHost::new(connection, &config_with_dims(0)); let provider = host.default_embedding_provider(); @@ -230,11 +241,7 @@ async fn the_module_never_reports_a_credential() { // The central claim, asserted on the behaviour rather than the config shape: // no provider name yields a key, including ones a host would normally have // one for. - let connection = bus_with_host(FakeHostEmbedder { - width: 4, - force_count: None, - }) - .await; + let connection = bus_with_host(FakeHostEmbedder::with_width(4)).await; let host = BusEmbeddingHost::new(connection, &config_with_dims(4)); for provider in ["openai", "cohere", "voyage", "custom", "ollama", "cloud"] { @@ -250,11 +257,7 @@ async fn a_keyed_provider_request_still_builds_and_ignores_the_key() { // The engine may ask for a keyed provider with an empty key. Refusing would // break recall; forwarding a key would defeat the split. It builds, and the // key goes nowhere. - let connection = bus_with_host(FakeHostEmbedder { - width: 3, - force_count: None, - }) - .await; + let connection = bus_with_host(FakeHostEmbedder::with_width(3)).await; let host = BusEmbeddingHost::new(connection, &config_with_dims(3)); let provider = host @@ -272,11 +275,7 @@ async fn dimension_support_is_answered_from_configuration() { // A synchronous getter cannot make a bus call, so the host passes the list. // Absent means "unsupported", which is the safe direction: the engine omits // the parameter rather than writing a batch the provider rejects halfway. - let connection = bus_with_host(FakeHostEmbedder { - width: 4, - force_count: None, - }) - .await; + let connection = bus_with_host(FakeHostEmbedder::with_width(4)).await; let host = BusEmbeddingHost::new(connection, &config_with_dims(4)); assert!(host.model_supports_dimensions("test-model")); @@ -285,11 +284,7 @@ async fn dimension_support_is_answered_from_configuration() { #[tokio::test] async fn configured_getters_and_provider_factories_preserve_their_identity() { - let connection = bus_with_host(FakeHostEmbedder { - width: 6, - force_count: None, - }) - .await; + let connection = bus_with_host(FakeHostEmbedder::with_width(6)).await; let config = ModuleConfig { ollama_base_url: "http://embedder.internal:11434".to_string(), cloud_embedding_model: "cloud-default".to_string(), @@ -303,7 +298,7 @@ async fn configured_getters_and_provider_factories_preserve_their_identity() { assert_eq!(host.default_cloud_embedding_dimensions(), 6); let default_provider = host.default_embedding_provider(); - assert_eq!(default_provider.name(), "module-bus"); + assert_eq!(default_provider.name(), "cloud"); assert_eq!(default_provider.model_id(), "cloud-default"); assert_eq!(default_provider.dimensions(), 6); @@ -333,11 +328,7 @@ async fn configured_getters_and_provider_factories_preserve_their_identity() { async fn the_signature_matches_what_the_contract_formats() { // Drift between a live provider's signature and a config-derived one splits // one embedding space in two, so both must route through the same formatter. - let connection = bus_with_host(FakeHostEmbedder { - width: 4, - force_count: None, - }) - .await; + let connection = bus_with_host(FakeHostEmbedder::with_width(4)).await; let host = BusEmbeddingHost::new(connection, &config_with_dims(4)); let provider = host.default_embedding_provider(); @@ -373,14 +364,52 @@ async fn the_debug_form_carries_no_connection_and_no_key() { /// bus provider must be usable as one. #[tokio::test] async fn the_provider_is_usable_as_a_trait_object() { - let connection = bus_with_host(FakeHostEmbedder { - width: 2, - force_count: None, - }) - .await; + let connection = bus_with_host(FakeHostEmbedder::with_width(2)).await; let host = BusEmbeddingHost::new(connection, &config_with_dims(2)); let provider: Arc = host.default_embedding_provider(); let one = provider.embed_one("alpha").await.expect("embed_one works"); assert_eq!(one.len(), 2); } + +#[tokio::test] +async fn the_provider_name_travels_first_then_model_then_dimensions() { + // The host resolves credential and endpoint from the provider name, so it + // must arrive as the first argument. This pins the wire order against the + // host's `EmbeddingHost::embed(provider, model, dimensions, texts)`; the + // three-argument form the module used to send put `dimensions` where the + // host reads `model` and was refused at decode (openhuman#5820). + let embedder = FakeHostEmbedder::with_width(4); + let seen = Arc::clone(&embedder.seen); + let connection = bus_with_host(embedder).await; + let host = BusEmbeddingHost::new(connection, &config_with_dims(4)); + + host.default_embedding_provider() + .embed(&["alpha"]) + .await + .expect("the managed embedder answers"); + host.ollama_embedding_provider("http://127.0.0.1:11434", "nomic-embed-text", 4) + .expect("an ollama provider is constructible") + .embed(&["beta"]) + .await + .expect("the local embedder answers"); + host.create_embedding_provider_with_credentials("voyage", "voyage-3", 4, "", None) + .expect("a BYO-key provider is constructible") + .embed(&["gamma"]) + .await + .expect("the BYO-key embedder answers"); + + let seen = seen.lock().expect("seen lock").clone(); + assert_eq!( + seen, + vec![ + ( + "cloud".to_string(), + config_with_dims(4).cloud_embedding_model, + 4 + ), + ("ollama".to_string(), "nomic-embed-text".to_string(), 4), + ("voyage".to_string(), "voyage-3".to_string(), 4), + ] + ); +} diff --git a/crates/tinymemory-module/tests/module_e2e.rs b/crates/tinymemory-module/tests/module_e2e.rs index 315ff45..c0b2b48 100644 --- a/crates/tinymemory-module/tests/module_e2e.rs +++ b/crates/tinymemory-module/tests/module_e2e.rs @@ -147,14 +147,22 @@ impl HostChat { #[tinybus::interface(name = "ai.tinyhumans.tinymemory.EmbeddingHost")] impl HostEmbedder { + /// The host's real signature, in the host's order: `provider` first, + /// because it is what the host selects credential and endpoint by. A fake + /// declaring the module's old three-argument form passed while the real + /// host refused every batch at decode (openhuman#5820). async fn embed( &self, + provider: String, _model: String, _dimensions: usize, texts: Vec, ) -> BusResult>> { std::future::ready(()).await; EMBED_CALLS.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + // The engine's default embedder must announce itself under a slug the + // host has a factory arm for; `cloud` is the managed embedder. + assert_eq!(provider, "cloud", "unknown provider slug on the Embed wire"); // A crude content-derived vector: enough that identical text embeds // identically and different text does not, which is all recall needs // here.