diff --git a/crates/biorouter/src/agents/extension_manager.rs b/crates/biorouter/src/agents/extension_manager.rs index 728269411..cbb6d51c8 100644 --- a/crates/biorouter/src/agents/extension_manager.rs +++ b/crates/biorouter/src/agents/extension_manager.rs @@ -170,6 +170,106 @@ fn is_privacy_refusal(err: &ErrorData) -> bool { err.code == ErrorCode::INVALID_REQUEST } +/// What `list_resources` says when an extension the caller NAMED lists nothing +/// (finding F8) — the name is the caller's own, and the reach gate has already +/// admitted it. +fn no_resources_in(extension: &str, supports_resources: bool) -> String { + if supports_resources { + format!( + "`{extension}` has no resources to list: it supports resources but is not \ + publishing any right now." + ) + } else { + format!("`{extension}` has no resources to list: it does not offer resources.") + } +} + +/// Who a `list_resources` fan-out that found nothing may name, partitioned the +/// way its sentence reports them. Every name is one Gate E has already shown +/// the caller. +#[derive(Debug, Default)] +struct UnlistedResources { + /// Support resources, were asked, and had none. + asked: Vec, + /// Support resources and could not be listed. + failed: Vec, + /// Support resources and were refused to this caller's model — a + /// cross-affiliation mismatch (DR-26), which Gate E lists and marks rather + /// than hides. Without this group the sentence would call them extensions + /// that "do not offer resources". + withheld: Vec, + /// How many other reachable extensions do not support resources at all. A + /// count, not a list: the model's own tool list already names them, and + /// the sentence is read to a user. + offer_none: usize, +} + +/// What `list_resources`' fan-out says when no extension listed anything +/// (finding F8). +fn no_resources_listed(unlisted: &UnlistedResources) -> String { + let named = |names: &[String]| { + names + .iter() + .map(|name| format!("`{name}`")) + .collect::>() + .join(", ") + }; + let mut text = String::from("No resources to list."); + match unlisted.asked.as_slice() { + [] => {} + [one] => text.push_str(&format!( + " `{one}` supports resources and was asked, but is not publishing any right now." + )), + many => text.push_str(&format!( + " {} support resources and were asked, but none of them is publishing any right \ + now.", + named(many) + )), + } + match unlisted.failed.as_slice() { + [] => {} + [one] => text.push_str(&format!( + " `{one}` supports resources but could not be listed: its server returned an error." + )), + many => text.push_str(&format!( + " {} support resources but could not be listed: their servers returned an error.", + named(many) + )), + } + match unlisted.withheld.as_slice() { + [] => {} + [one] => text.push_str(&format!( + " `{one}` supports resources, but this chat's model may not read them." + )), + many => text.push_str(&format!( + " {} support resources, but this chat's model may not read them.", + named(many) + )), + } + let offer_none = unlisted.offer_none; + let other = if unlisted.asked.is_empty() + && unlisted.failed.is_empty() + && unlisted.withheld.is_empty() + { + "" + } else { + " other" + }; + match offer_none { + 0 if other.is_empty() => { + text.push_str(" No extension this chat can reach offers resources."); + } + 0 => {} + 1 => text.push_str(&format!( + " The one{other} extension this chat can reach does not offer resources." + )), + n => text.push_str(&format!( + " The{other} {n} extensions this chat can reach do not offer resources." + )), + } + text +} + /// The prefixed tool list and the extension keys that named it, taken together. /// /// Issue #56 Gate E resolves a prefixed tool name against the set of installed @@ -2566,6 +2666,12 @@ impl ExtensionManager { ) }) .map(|lr| { + // Nothing listed is NOTHING, not one empty string: both callers + // compose a sentence from an empty result, and a `""` among the + // fan-out's contents would read as a listing (finding F8). + if lr.resources.is_empty() { + return Vec::new(); + } let resource_list = lr .resources .into_iter() @@ -2599,38 +2705,65 @@ impl ExtensionManager { .await?; // Handle single extension case - self.list_resources_from_extension(extension_name, admitted, cancellation_token) + let listed = self + .list_resources_from_extension(extension_name, admitted, cancellation_token) + .await?; + if !listed.is_empty() { + return Ok(listed); + } + // The gate above admitted this name, and it is the caller's own + // word, so saying it back tells the caller nothing new. + let supports_resources = self + .extensions + .lock() .await + .get(extension_name) + .is_some_and(Extension::supports_resources); + Ok(vec![Content::text(no_resources_in( + extension_name, + supports_resources, + ))]) } None => { // Handle all extensions case using FuturesUnordered let mut futures = FuturesUnordered::new(); - // Create futures for each resource_capable_extension - self.extensions + // The extensions that declare resource support: the ones this + // fan-out asks, and — by difference — the ones it need not. + let capable: Vec = self + .extensions .lock() .await .iter() .filter(|(_name, ext)| ext.supports_resources()) .map(|(name, _ext)| name.clone()) - .for_each(|name| { - let token = cancellation_token.clone(); - futures.push(async move { - self.list_resources_from_extension(&name.clone(), admitted, token) - .await - }); + .collect(); + for name in capable.iter().cloned() { + let token = cancellation_token.clone(); + futures.push(async move { + let listed = self + .list_resources_from_extension(&name, admitted, token) + .await; + (name, listed) }); + } let mut all_resources = Vec::new(); + let mut asked = Vec::new(); + let mut failed = Vec::new(); let mut errors = Vec::new(); // Process results as they complete - while let Some(result) = futures.next().await { + while let Some((name, result)) = futures.next().await { match result { Ok(content) => { + asked.push(name); all_resources.extend(content); } Err(tool_error) => { + if !is_privacy_refusal(&tool_error) { + failed.push(name); + } errors.push(tool_error); } } @@ -2663,11 +2796,68 @@ impl ExtensionManager { ); } - Ok(all_resources) + if !all_resources.is_empty() { + return Ok(all_resources); + } + let unlisted = self + .unlisted_resources(&capable, asked, failed, admitted) + .await; + Ok(vec![Content::text(no_resources_listed(&unlisted))]) } } } + /// Who an empty `list_resources` fan-out may name, and how (finding F8, + /// 2026-09-10 composer QA run). An empty listing used to come back as + /// `""`, and the model could only report that the tool "returned an empty + /// string"; every other empty or refusing result explains itself. + /// + /// `capable` is every extension that declares resource support; `asked` + /// and `failed` are the ones the fan-out listed and could not list. + /// + /// ⚠ **Every name is filtered through Gate E's roster**, the rule #219 set + /// for the not-found message in `read_resource_tool`, and for the same + /// reason: this is composed right after a loop that declined to reach the + /// private extensions, so naming what the loop consulted — or, worse, what + /// it skipped — would hand a public caller the private roster Gate E + /// withholds. A refused extension is never named and never counted. + /// `admitted` is threaded, never resampled, and names are sorted because + /// the map's iteration order is randomised per process. + async fn unlisted_resources( + &self, + capable: &[String], + asked: Vec, + failed: Vec, + admitted: Option, + ) -> UnlistedResources { + let roster = self.allowed_extension_keys(admitted).await; + let shown = |mut names: Vec| { + names.retain(|name| roster.contains(name)); + names.sort(); + names + }; + // What is left of the capable set once the asked and the failed are + // taken out is what the loop refused. Of that, only the part Gate E + // shows this caller is named — the cross-affiliation mismatches DR-26 + // lists and marks — and the rest, the private extensions a public + // caller may not see, vanishes here exactly as it vanished from the + // tool list. + let withheld = shown( + capable + .iter() + .filter(|name| !asked.contains(name) && !failed.contains(name)) + .cloned() + .collect(), + ); + let offer_none = roster.iter().filter(|name| !capable.contains(name)).count(); + UnlistedResources { + asked: shown(asked), + failed: shown(failed), + withheld, + offer_none, + } + } + async fn prefixed_tool_name(&self, tool_name: &str) -> String { if !tool_name.contains("__") && ["execute_code", "read_module", "search_modules"].contains(&tool_name) @@ -6953,6 +7143,9 @@ mod tests { struct CountingClient { label: &'static str, calls: Arc, + /// Whether `list_resources` returns a resource. `false` is a server + /// that supports resources and is publishing none — finding F8's case. + publishes: bool, } impl CountingClient { @@ -6960,6 +7153,14 @@ mod tests { Self { label, calls: Arc::new(std::sync::atomic::AtomicUsize::new(0)), + publishes: true, + } + } + + fn publishing_nothing(label: &'static str) -> Self { + Self { + publishes: false, + ..Self::new(label) } } @@ -6990,12 +7191,17 @@ mod tests { ) -> Result { use rmcp::model::AnnotateAble; self.hit(); - Ok(ListResourcesResult { - resources: vec![rmcp::model::RawResource::new( + let resources = if self.publishes { + vec![rmcp::model::RawResource::new( "res://x", format!("{}-resource", self.sentinel()), ) - .no_annotation()], + .no_annotation()] + } else { + vec![] + }; + Ok(ListResourcesResult { + resources, next_cursor: None, meta: None, }) @@ -7404,6 +7610,248 @@ mod tests { ))); } + /// Finding F8's shape: extensions that SUPPORT resources and publish none + /// — the private `ucsfomopagent` and the public `developer` — beside the + /// public `todo`, which declares no resource support at all. The QA + /// sandbox's `computercontroller` had cached nothing, which is the + /// `developer` row; the private row is added so the roster filter can be + /// seen biting rather than assumed. + async fn quiet_resources_fixture() -> (TempDir, ExtensionManager, CountingClient) { + let dir = tempfile::tempdir().unwrap(); + let session_manager = Arc::new(crate::session::SessionManager::new( + dir.path().to_path_buf(), + )); + let em = ExtensionManager::new( + Arc::new(Mutex::new(Some(provider_at( + crate::privacy::ProviderTier::Public, + )))), + session_manager, + ); + let info = ServerInfo { + capabilities: rmcp::model::ServerCapabilities::builder() + .enable_tools() + .enable_resources() + .build(), + ..Default::default() + }; + let private = CountingClient::publishing_nothing("private"); + for (name, client) in [ + ("ucsfomopagent", private.clone()), + ("developer", CountingClient::publishing_nothing("public")), + ] { + em.add_client( + normalize(name), + ExtensionConfig::Builtin { + name: name.to_string(), + display_name: Some(name.to_string()), + description: "built-in".to_string(), + timeout: None, + bundled: None, + available_tools: vec![], + }, + Arc::new(client), + Some(info.clone()), + None, + ) + .await; + } + // No `ServerInfo`, so no declared resource support. + em.add_mock_extension("todo".to_string(), Arc::new(MockClient {})) + .await; + (dir, em, private) + } + + /// `list_resources` as the `extensionmanager__list_resources` tool calls + /// it — an admitted capability, never a fresh sample — rendered to the text + /// the model reads. + async fn listed_resources( + em: &ExtensionManager, + params: Value, + caller: crate::privacy::ProviderTier, + ) -> String { + em.list_resources( + params, + Some(crate::privacy::CallCapability::for_test(caller, true)), + CancellationToken::default(), + ) + .await + .expect("an empty listing is an answer, not an error") + .iter() + .filter_map(|c| c.as_text().map(|t| t.text.clone())) + .collect::>() + .join("\n") + } + + /// Finding F8: `list_resources` found nothing and answered `""`, so the + /// model could only report that the tool "returned an empty string". It now + /// says who was asked and that none of them had anything. + /// + /// ⚠ **Both columns are needed, as in #219's M6 test next door.** The public + /// sentence must not name `ucsfomopagent` — it is composed right after a + /// loop that declined to reach it, and naming what the loop skipped is the + /// leak #219 closed in `read_resource_tool`. And it must still name + /// `developer`, or a sentence that named nothing would pass. The private + /// column is what shows the name was FILTERED rather than never collected. + #[tokio::test] + async fn an_empty_resource_listing_says_who_was_asked_naming_only_gate_es_roster() { + let (_dir, em, private) = quiet_resources_fixture().await; + + let as_public = listed_resources( + &em, + serde_json::json!({}), + crate::privacy::ProviderTier::Public, + ) + .await; + assert_eq!( + as_public, + "No resources to list. `developer` supports resources and was asked, but is not \ + publishing any right now. The one other extension this chat can reach does not \ + offer resources." + ); + assert!(!as_public.contains("ucsfomopagent"), "{as_public}"); + assert_eq!( + private.contacted(), + 0, + "the public listing asked the private server" + ); + + let as_private = listed_resources( + &em, + serde_json::json!({}), + crate::privacy::ProviderTier::Private, + ) + .await; + assert_eq!( + as_private, + "No resources to list. `developer`, `ucsfomopagent` support resources and were \ + asked, but none of them is publishing any right now. The one other extension this \ + chat can reach does not offer resources." + ); + assert_eq!(private.contacted(), 1); + } + + /// The named branch says the named extension has none — and a public caller + /// naming a private extension is still REFUSED, in the words a name that is + /// not installed gets (finding M18). "`ucsfomopagent` has no resources" + /// would confirm to a public caller that it is installed. + #[tokio::test] + async fn a_named_extension_with_nothing_to_list_says_so() { + use crate::privacy::ProviderTier::{Private, Public}; + let (_dir, em, private) = quiet_resources_fixture().await; + + assert_eq!( + listed_resources(&em, serde_json::json!({ "extension": "developer" }), Public).await, + "`developer` has no resources to list: it supports resources but is not publishing \ + any right now." + ); + assert_eq!( + listed_resources( + &em, + serde_json::json!({ "extension": "ucsfomopagent" }), + Private + ) + .await, + "`ucsfomopagent` has no resources to list: it supports resources but is not \ + publishing any right now." + ); + assert_eq!(private.contacted(), 1, "only the private caller reached it"); + + let refused = |name: &'static str| { + let em = &em; + async move { + em.list_resources( + serde_json::json!({ "extension": name }), + Some(crate::privacy::CallCapability::for_test(Public, true)), + CancellationToken::default(), + ) + .await + .expect_err("a public caller reaches neither") + .message + .to_string() + } + }; + assert_eq!( + refused("ucsfomopagent") + .await + .replace("ucsfomopagent", "NAME"), + refused("nonexistent_ext") + .await + .replace("nonexistent_ext", "NAME"), + ); + assert_eq!(private.contacted(), 1); + } + + /// A model bound to ANOTHER institution is shown `ucsfomopagent` — DR-26 + /// lists and marks a mismatch rather than hiding it — and refused its + /// resources. The sentence must say that, not count it among the + /// extensions that "do not offer resources", which would be false. + #[tokio::test] + async fn an_empty_listing_names_a_cross_affiliation_refusal_as_withheld() { + let (_dir, em, private) = quiet_resources_fixture().await; + let elsewhere = crate::privacy::CallCapability::for_test_affiliated( + crate::privacy::ProviderTier::Private, + true, + Some(crate::privacy::affiliation::ModelAffiliation::institution( + crate::privacy::affiliation::InstitutionId::new("stanford"), + )), + ); + let text = em + .list_resources( + serde_json::json!({}), + Some(elsewhere), + CancellationToken::default(), + ) + .await + .expect("an empty listing is an answer, not an error") + .iter() + .filter_map(|c| c.as_text().map(|t| t.text.clone())) + .collect::>() + .join("\n"); + assert_eq!( + text, + "No resources to list. `developer` supports resources and was asked, but is not \ + publishing any right now. `ucsfomopagent` supports resources, but this chat's model \ + may not read them. The one other extension this chat can reach does not offer \ + resources." + ); + assert_eq!( + private.contacted(), + 0, + "Gate C refused it before any contact" + ); + } + + /// The composer's other branches, which the fixtures above do not reach: + /// no reachable extension supports resources at all, and ones that could + /// not be listed. + #[test] + fn the_empty_listing_sentence_accounts_for_every_group_it_is_given() { + let names = |names: &[&str]| names.iter().map(|n| n.to_string()).collect::>(); + assert_eq!( + no_resources_listed(&UnlistedResources::default()), + "No resources to list. No extension this chat can reach offers resources." + ); + assert_eq!( + no_resources_listed(&UnlistedResources { + offer_none: 11, + ..Default::default() + }), + "No resources to list. The 11 extensions this chat can reach do not offer resources." + ); + assert_eq!( + no_resources_listed(&UnlistedResources { + failed: names(&["files", "notes"]), + withheld: names(&["cdwagent"]), + offer_none: 2, + ..Default::default() + }), + "No resources to list. `files`, `notes` support resources but could not be listed: \ + their servers returned an error. `cdwagent` supports resources, but this chat's \ + model may not read them. The other 2 extensions this chat can reach do not offer \ + resources." + ); + } + /// An MCP prompt body is server-authored text that lands in the transcript /// verbatim, so a refusal that echoed it would defeat the point of refusing. #[tokio::test] diff --git a/crates/biorouter/src/agents/extension_manager_extension.rs b/crates/biorouter/src/agents/extension_manager_extension.rs index e3f7ff13f..493dda31d 100644 --- a/crates/biorouter/src/agents/extension_manager_extension.rs +++ b/crates/biorouter/src/agents/extension_manager_extension.rs @@ -421,6 +421,87 @@ fn marketplace_descriptor_json( payload } +/// The `search_marketplace_extensions` result, from a catalog already loaded — +/// split from the load so it is testable without the network. +/// +/// `caller` is the admitted capability's tier. Everything below — the hits, the +/// browse list and the count the guidance quotes — is taken from what that +/// caller may see, so a public model's answer never counts a private row. +fn marketplace_extensions_json( + loaded: &crate::marketplace::MarketplaceCatalogLoad, + query: Option<&str>, + caller: crate::privacy::ProviderTier, +) -> Value { + let visible = loaded.catalog.browse_extensions(caller); + // Ranked, and each hit says which query terms it matched: the search is + // shared with the skills catalog (finding F5). + let search = query.map(|query| loaded.catalog.search_extensions(caller, query)); + let extensions: Vec = match &search { + Some(search) => search + .hits + .iter() + .map(|hit| { + let mut payload = marketplace_descriptor_json(hit.entry); + if let Some(fields) = payload.as_object_mut() { + fields.insert( + "matchedTerms".to_owned(), + serde_json::json!(hit.matched_terms), + ); + } + payload + }) + .collect(), + None => visible + .iter() + .copied() + .map(marketplace_descriptor_json) + .collect(), + }; + let source = match loaded.source { + crate::marketplace::MarketplaceCatalogSource::Live => "live", + crate::marketplace::MarketplaceCatalogSource::LastGood => "lastGood", + crate::marketplace::MarketplaceCatalogSource::Embedded => "embedded", + }; + let mut body = serde_json::json!({ + "source": source, + "stale": loaded.is_stale(), + "extensions": extensions, + }); + if let (Some(query), Some(search), Some(fields)) = (query, &search, body.as_object_mut()) { + fields.insert("terms".to_owned(), serde_json::json!(&search.terms)); + if search.is_empty() { + fields.insert( + "guidance".to_owned(), + Value::String(no_marketplace_extension_matched( + &search.describe_query(query), + visible.len(), + )), + ); + } + } + body +} + +/// What an empty `search_marketplace_extensions` says instead of an empty list +/// — the extension half of finding F5, whose skills half let a model tell a +/// user the marketplace had nothing. +/// +/// ⚠ `visible` is the count shown to THIS caller. For a public model that is +/// the public rows only; the registry's full size would count the private +/// extensions Gate E keeps out of its sight. +fn no_marketplace_extension_matched(asked: &str, visible: usize) -> String { + let available = match visible { + 1 => "1 extension is".to_owned(), + n => format!("{n} extensions are"), + }; + format!( + "No marketplace extension available to this model matched {asked}. {available} \ + available to this model, so this does not mean there is nothing relevant: try a \ + shorter or more general term (one tool, data source or topic name), or call \ + search_marketplace_extensions with no query to list them all." + ) +} + fn marketplace_approval_request( mutation: MarketplaceMutation, descriptor: &crate::marketplace::MarketplaceExtensionDescriptor, @@ -2241,23 +2322,7 @@ impl ExtensionManagerClient { .map_err(|error| ExtensionManagerToolError::OperationFailed { message: error.to_string(), })?; - let entries = match query { - Some(query) => loaded.catalog.search_extensions(cap.tier(), query), - None => loaded.catalog.browse_extensions(cap.tier()), - }; - let source = match loaded.source { - crate::marketplace::MarketplaceCatalogSource::Live => "live", - crate::marketplace::MarketplaceCatalogSource::LastGood => "lastGood", - crate::marketplace::MarketplaceCatalogSource::Embedded => "embedded", - }; - let body = serde_json::json!({ - "source": source, - "stale": loaded.is_stale(), - "extensions": entries - .into_iter() - .map(marketplace_descriptor_json) - .collect::>(), - }); + let body = marketplace_extensions_json(&loaded, query, cap.tier()); Ok(vec![Content::text( serde_json::to_string_pretty(&body).unwrap_or_else(|_| "{}".to_owned()), )]) @@ -3267,7 +3332,7 @@ impl ExtensionManagerClient { tools.extend([ Tool::new( SEARCH_MARKETPLACE_EXTENSIONS_TOOL_NAME.to_owned(), - "Browse or search trusted BAAM marketplace extensions. Pass `query` to match an id, name, organization, description or tag; omit it to list everything visible to this model. Private entries are hidden from public models. Results carry `registryId` (camelCase); pass that exact value as install_extension's `registry_id` (snake_case) — the two tools spell the same field differently." + "Browse or search trusted BAAM marketplace extensions. Pass `query` to search ids, names, organizations, descriptions and tags — an entry matching any of its words is returned, best match first; omit it to list everything visible to this model. Private entries are hidden from public models. Results carry `registryId` (camelCase); pass that exact value as install_extension's `registry_id` (snake_case) — the two tools spell the same field differently." .to_owned(), Arc::new( serde_json::to_value(schema_for!(SearchMarketplaceExtensionsParams)) @@ -3572,6 +3637,82 @@ mod tests { .unwrap() } + /// The extension half of finding F5, at the tool's output: a phrase is + /// matched by any of its words and ranked, and a query matching nothing + /// explains itself instead of returning an empty list. + /// + /// ⚠ The explanation counts what THIS caller may see. A public caller + /// whose query matches only the private row it is not shown must read + /// exactly what a query matching nothing at all reads — otherwise the + /// guidance, not the hit list, becomes the private catalog's oracle. + #[test] + fn marketplace_search_ranks_a_phrase_and_explains_an_empty_result_from_visible_rows() { + let loaded = crate::marketplace::MarketplaceCatalogLoad { + catalog: marketplace_catalog(), + source: crate::marketplace::MarketplaceCatalogSource::Embedded, + cache_warning: None, + }; + let ids = |body: &Value| -> Vec { + body["extensions"] + .as_array() + .unwrap() + .iter() + .map(|entry| entry["registryId"].as_str().unwrap().to_owned()) + .collect() + }; + + let as_public = marketplace_extensions_json( + &loaded, + Some("public fixture manager"), + ProviderTier::Public, + ); + assert_eq!( + as_public["terms"], + serde_json::json!(["public", "fixture", "manager"]) + ); + assert_eq!(ids(&as_public), ["manager-public-fixture"]); + assert_eq!( + as_public["extensions"][0]["matchedTerms"], + serde_json::json!(["public", "fixture", "manager"]) + ); + let as_private = marketplace_extensions_json( + &loaded, + Some("public fixture manager"), + ProviderTier::Private, + ); + assert_eq!( + ids(&as_private), + ["manager-public-fixture", "manager-private-fixture"], + "the row matching every term ranks first" + ); + + let guidance = |query: &str| -> String { + let body = marketplace_extensions_json(&loaded, Some(query), ProviderTier::Public); + assert!(ids(&body).is_empty(), "{body}"); + body["guidance"] + .as_str() + .expect("an empty result explains itself") + .replace(&format!("`{query}`"), "`QUERY`") + }; + let hidden_match = guidance("private"); + assert!( + hidden_match.contains("1 extension is available to this model"), + "{hidden_match}" + ); + assert_eq!( + hidden_match, + guidance("zzqx"), + "a public caller's miss on a hidden private row reads differently from a miss on \ + nothing" + ); + assert!( + marketplace_extensions_json(&loaded, None, ProviderTier::Public) + .get("guidance") + .is_none(), + "browsing needs no explanation" + ); + } + #[test] fn install_schema_accepts_only_a_registry_id_and_enable_flag() { let schema = serde_json::to_value(schema_for!(InstallExtensionParams)).unwrap(); diff --git a/crates/biorouter/src/agents/skills_extension.rs b/crates/biorouter/src/agents/skills_extension.rs index 3260c1c4c..2ecd74f41 100644 --- a/crates/biorouter/src/agents/skills_extension.rs +++ b/crates/biorouter/src/agents/skills_extension.rs @@ -38,7 +38,7 @@ const SKILL_OPERATION_GUIDANCE: &[(&str, &str)] = &[ ("loadSkill", "loadSkill reads an exact installed skill"), ( "searchMarketplaceSkills", - "searchMarketplaceSkills lists trusted BAAM entries, or filters them when you pass a query", + "searchMarketplaceSkills lists trusted BAAM entries, or ranks those matching any word of a query you pass", ), ( "installMarketplaceSkill", @@ -1786,20 +1786,51 @@ impl SkillsClient { let loaded = crate::marketplace::load_marketplace_catalog() .await .map_err(|error| error.to_string())?; + Ok(vec![Content::text( + Self::marketplace_skill_page_json(&loaded, query, offset, limit).to_string(), + )]) + } + + /// One page of `searchMarketplaceSkills`, from a catalog already loaded — + /// split from the load so the page is testable without the network. + fn marketplace_skill_page_json( + loaded: &crate::marketplace::MarketplaceCatalogLoad, + query: Option<&str>, + offset: usize, + limit: usize, + ) -> serde_json::Value { let source = Self::marketplace_source_name(loaded.source); let stale = loaded.is_stale(); let cache_warning = loaded.cache_warning.clone(); - let matches = match query { - Some(query) => loaded.catalog.search_skills(query), - None => loaded.catalog.browse_skills(), + let registry_size = loaded.catalog.browse_skills().len(); + // A query is ranked, not filtered (finding F5): its terms are matched + // separately and each hit says which of them it matched, so a model + // reading a long list can tell an entry that matched every term from + // one that matched a single common word. + let search = query.map(|query| loaded.catalog.search_skills(query)); + let matches: Vec<( + &crate::marketplace::MarketplaceSkillDescriptor, + Option<&[String]>, + )> = match &search { + Some(search) => search + .hits + .iter() + .map(|hit| (hit.entry, Some(hit.matched_terms.as_slice()))) + .collect(), + None => loaded + .catalog + .browse_skills() + .into_iter() + .map(|entry| (entry, None)) + .collect(), }; let total = matches.len(); let entries: Vec<_> = matches .into_iter() .skip(offset) .take(limit) - .map(|entry| { - serde_json::json!({ + .map(|(entry, matched_terms)| { + let mut row = serde_json::json!({ "registryId": &entry.registry_id, "name": &entry.name, "category": &entry.category, @@ -1808,25 +1839,58 @@ impl SkillsClient { "tags": &entry.tags, "keywords": &entry.keywords, "license": &entry.license, - }) + }); + if let (Some(matched_terms), Some(fields)) = (matched_terms, row.as_object_mut()) { + fields.insert("matchedTerms".to_owned(), serde_json::json!(matched_terms)); + } + row }) .collect(); let returned = entries.len(); let next_offset = (offset + returned < total).then_some(offset + returned); - Ok(vec![Content::text( - serde_json::json!({ - "source": source, - "stale": stale, - "cacheWarning": cache_warning, - "total": total, - "offset": offset, - "limit": limit, - "returned": returned, - "nextOffset": next_offset, - "skills": entries, - }) - .to_string(), - )]) + let mut body = serde_json::json!({ + "source": source, + "stale": stale, + "cacheWarning": cache_warning, + "total": total, + "offset": offset, + "limit": limit, + "returned": returned, + "nextOffset": next_offset, + "skills": entries, + }); + if let (Some(query), Some(search), Some(fields)) = (query, &search, body.as_object_mut()) { + fields.insert("terms".to_owned(), serde_json::json!(&search.terms)); + if search.is_empty() { + fields.insert( + "guidance".to_owned(), + serde_json::Value::String(Self::no_marketplace_skill_matched( + &search.describe_query(query), + registry_size, + )), + ); + } + } + body + } + + /// What an empty marketplace search says instead of a bare `total: 0` + /// (finding F5). That answer let a model report *"no matching marketplace + /// skills found"* about a registry that held every skill the user named, + /// so the guidance says how big the registry is and what to try next — a + /// miss is more often the query's wording than the registry's contents. + fn no_marketplace_skill_matched(asked: &str, registry_size: usize) -> String { + let skills = if registry_size == 1 { + "skill" + } else { + "skills" + }; + format!( + "No marketplace skill matched {asked}. The registry holds {registry_size} {skills}, \ + so this does not mean there is nothing relevant: try a shorter or more general term \ + (one tool, language or topic name), or call searchMarketplaceSkills with no query \ + to list them all." + ) } /// Browse or search the trusted BAAM skill registry. @@ -2793,8 +2857,10 @@ impl SkillsClient { indoc! {r#" Browse or search the trusted skill entries published in BAAM. - Pass `query` to match an id, name, category, description, tag or keyword; - omit it to list the whole registry. This returns registry ids and metadata, + Pass `query` to search ids, names, categories, descriptions, tags and keywords: + an entry matching any of its words is returned, the entries matching the most + words first, each with the `matchedTerms` it matched. Omit `query` to list the + whole registry. This returns registry ids and metadata, never arbitrary download URLs — pass an exact returned registryId as installMarketplaceSkill's registry_id. "#} @@ -5938,6 +6004,70 @@ mod merged_surface_tests { } } + /// Finding F5 at the tool's own output. The QA run read `total: 0` for the + /// phrase below against a registry holding every skill it names, and the + /// model reported "no matching marketplace skills found". The phrase now + /// finds them, each hit says which terms it matched, and a query that + /// genuinely matches nothing explains itself instead of returning a bare + /// zero. + #[test] + fn a_marketplace_skill_page_ranks_a_phrase_and_explains_an_empty_result() { + let loaded = crate::marketplace::MarketplaceCatalogLoad::embedded_for_test(); + let registry = loaded.catalog.browse_skills().len(); + + let page = SkillsClient::marketplace_skill_page_json( + &loaded, + Some("R scripting ggplot visualization"), + 0, + 50, + ); + assert!(page["total"].as_u64().unwrap() >= 2, "{page}"); + assert_eq!( + page["terms"], + serde_json::json!(["r", "scripting", "ggplot", "visualization"]) + ); + let skills = page["skills"].as_array().unwrap(); + let ids: Vec<&str> = skills + .iter() + .map(|skill| skill["registryId"].as_str().unwrap()) + .collect(); + assert!( + ids.contains(&"ggplot-visualization") && ids.contains(&"r-scripting"), + "{ids:?}" + ); + assert!( + skills + .iter() + .all(|skill| !skill["matchedTerms"].as_array().unwrap().is_empty()), + "{page}" + ); + assert!(page.get("guidance").is_none(), "{page}"); + + let none = SkillsClient::marketplace_skill_page_json(&loaded, Some("zzqx"), 0, 50); + assert_eq!(none["total"], 0); + let guidance = none["guidance"] + .as_str() + .expect("an empty result explains itself"); + assert!( + guidance.contains(&format!("The registry holds {registry} skills")), + "{guidance}" + ); + assert!( + guidance.contains("`zzqx`") && guidance.contains("shorter"), + "{guidance}" + ); + + // Browsing is untouched: no terms, no matchedTerms, no guidance. + let browse = SkillsClient::marketplace_skill_page_json(&loaded, None, 0, 5); + assert_eq!(browse["total"], registry); + assert!(browse.get("terms").is_none(), "{browse}"); + assert!( + browse["skills"][0].get("matchedTerms").is_none(), + "{browse}" + ); + assert!(browse.get("guidance").is_none(), "{browse}"); + } + /// The retired names keep dispatching. They are not advertised — that is /// the whole point — but a persisted transcript, a stored `always allow` /// grant, or a coding-agent child that read one still calls them, and an diff --git a/crates/biorouter/src/marketplace.rs b/crates/biorouter/src/marketplace.rs index 54e1f78c7..e038fe503 100644 --- a/crates/biorouter/src/marketplace.rs +++ b/crates/biorouter/src/marketplace.rs @@ -12,6 +12,11 @@ use crate::config::paths::Paths; use crate::privacy::affiliation::InstitutionId; use crate::privacy::{ExtensionAffiliation, ProviderTier}; +mod search; + +use search::Weight; +pub use search::{MarketplaceSearch, MarketplaceSearchHit}; + pub const REGISTRY_URL: &str = "https://biorouter.ucsf.edu/registry.json"; const REGISTRY_SOURCE: &str = "https://biorouter.ucsf.edu/baam"; pub const MAX_REGISTRY_BYTES: usize = 2 * 1024 * 1024; @@ -109,56 +114,58 @@ impl MarketplaceCatalog { .collect() } + /// Rank the extensions visible to `caller` against a free-text query. How a + /// query is matched — and why a phrase is a union of its words rather than + /// one substring (finding F5) — is documented in `marketplace/search.rs`. + /// + /// The caller filter runs FIRST, so an extension hidden from `caller` is + /// never scored and cannot move, or be counted among, what it is shown. pub fn search_extensions( &self, caller: ProviderTier, query: &str, - ) -> Vec<&MarketplaceExtensionDescriptor> { - let query = query.trim().to_ascii_lowercase(); - self.browse_extensions(caller) - .into_iter() - .filter(|entry| { - query.is_empty() - || searchable( - &query, - [ - entry.registry_id.as_str(), - entry.extension_name.as_str(), - entry.name.as_str(), - entry.organization.as_str(), - entry.description.as_str(), - ] - .into_iter() - .chain(entry.tags.iter().map(String::as_str)), - ) - }) - .collect() + ) -> MarketplaceSearch<'_, MarketplaceExtensionDescriptor> { + search::rank( + query, + search::EXTENSION_NOISE, + self.browse_extensions(caller), + |entry| { + let mut fields = vec![ + (entry.registry_id.as_str(), Weight::Name), + (entry.extension_name.as_str(), Weight::Name), + (entry.name.as_str(), Weight::Name), + (entry.organization.as_str(), Weight::Label), + (entry.description.as_str(), Weight::Prose), + ]; + fields.extend(entry.tags.iter().map(|tag| (tag.as_str(), Weight::Label))); + fields + }, + ) } pub fn browse_skills(&self) -> Vec<&MarketplaceSkillDescriptor> { self.skills.values().collect() } - pub fn search_skills(&self, query: &str) -> Vec<&MarketplaceSkillDescriptor> { - let query = query.trim().to_ascii_lowercase(); - self.skills - .values() - .filter(|entry| { - query.is_empty() - || searchable( - &query, - [ - entry.registry_id.as_str(), - entry.name.as_str(), - entry.category.as_str(), - entry.description.as_str(), - ] - .into_iter() - .chain(entry.tags.iter().map(String::as_str)) - .chain(entry.keywords.iter().map(String::as_str)), - ) - }) - .collect() + /// Rank every skill against a free-text query, matched as documented in + /// `marketplace/search.rs`. + pub fn search_skills(&self, query: &str) -> MarketplaceSearch<'_, MarketplaceSkillDescriptor> { + search::rank(query, search::SKILL_NOISE, self.skills.values(), |entry| { + let mut fields = vec![ + (entry.registry_id.as_str(), Weight::Name), + (entry.name.as_str(), Weight::Name), + (entry.category.as_str(), Weight::Label), + (entry.description.as_str(), Weight::Prose), + ]; + fields.extend(entry.tags.iter().map(|tag| (tag.as_str(), Weight::Label))); + fields.extend( + entry + .keywords + .iter() + .map(|keyword| (keyword.as_str(), Weight::Label)), + ); + fields + }) } pub fn resolve_extension_for_install( @@ -219,10 +226,6 @@ impl MarketplaceCatalog { } } -fn searchable<'a>(query: &str, mut fields: impl Iterator) -> bool { - fields.any(|field| field.to_ascii_lowercase().contains(query)) -} - #[derive(Debug, Deserialize)] #[serde(deny_unknown_fields)] struct RawRegistry { @@ -565,6 +568,19 @@ impl MarketplaceCatalogLoad { pub fn is_stale(&self) -> bool { self.source != MarketplaceCatalogSource::Live } + + /// The registry snapshot shipped in the binary, loaded as the tools see it + /// offline — for tests of what a tool builds from a catalog, which must not + /// depend on the network. + #[cfg(test)] + pub(crate) fn embedded_for_test() -> Self { + Self { + catalog: MarketplaceCatalog::from_bytes(EMBEDDED_REGISTRY) + .expect("the shipped registry is a valid catalog"), + source: MarketplaceCatalogSource::Embedded, + cache_warning: None, + } + } } pub async fn load_marketplace_catalog() -> Result { @@ -877,6 +893,172 @@ mod tests { ); } + /// Seven skill rows copied VERBATIM from `landing/registry.json` at + /// 7c96d796, the registry the 2026-09-10 composer QA run measured finding + /// F5 against. Frozen here rather than read from [`EMBEDDED_REGISTRY`] so + /// the exact rankings below cannot drift when the registry gains a skill; + /// the shipped-registry test after them pins only the measured shape. + fn skills_snapshot() -> Vec { + fn row( + id: &str, + name: &str, + category: &str, + kind: &str, + description: &str, + tags: &[&str], + keywords: &[&str], + ) -> serde_json::Value { + json!({ + "id": id, + "name": name, + "category": category, + "type": kind, + "description": description, + "tags": tags, + "keywords": keywords, + "download": format!("https://github.com/BaranziniLab/biorouter-skills/releases/download/skill-{id}/{id}.zip"), + "filename": format!("{id}.zip"), + "license": "Apache-2.0" + }) + } + serde_json::to_vec(&json!({ + "version": 2, + "source": "https://biorouter.ucsf.edu/baam", + "institutions": { "ucsf": "UCSF" }, + "extensions": [], + "skills": [ + row("data-visualization", "Data Visualization", "Biomedical", "13 skills · auto-applied", + "Publication-quality plots: heatmaps, volcano, Manhattan, dimplots.", + &["ggplot2", "matplotlib", "ComplexHeatmap"], + &["data-visualization", "ggplot2", "matplotlib", "complexheatmap"]), + row("ggplot-visualization", "ggplot2 Visualization", "Core", "Auto-applied · R plotting", + "Applies ggplot2 best-practice style when writing R plotting code.", + &["R", "ggplot2"], &[]), + row("python-scripting", "Python Scripting", "Core", "Auto-applied · Python code", + "Applies Python naming, typing, error handling, and project structure conventions when writing Python code.", + &["Python"], &[]), + row("r-scripting", "R Scripting", "Core", "Auto-applied · R code", + "Applies tidyverse conventions and documentation standards when writing or reviewing R code.", + &["R", "Tidyverse"], &[]), + row("scientific-visual-communication", "Scientific Visual Communication", "Core", + "User-invocable · /scientific-visual-communication", + "Plans schematics, posters, slides, figure panels, infographics, visual abstracts, and source-to-visual traceability.", + &["Visuals", "Posters", "Apache-2.0"], + &["scientific-visual-communication", "schematics", "infographics", "posters", "slides", "visual", "abstracts", "apache"]), + row("clinical-biostatistics", "Clinical Biostatistics", "Biomedical", "6 skills · auto-applied", + "Survival, mixed models, and clinical-trial statistical analysis.", + &["survival", "R", "lme4"], &["clinical-biostatistics", "survival", "r", "lme4"]), + row("single-cell", "Single-cell", "Biomedical", "14 skills · auto-applied", + "scRNA-seq clustering, annotation, trajectory, and integration.", + &["Scanpy", "Seurat", "scVI"], &["single-cell", "scanpy", "seurat", "scvi"]), + ] + })) + .unwrap() + } + + fn skill_ids(search: &MarketplaceSearch<'_, MarketplaceSkillDescriptor>) -> Vec { + search + .hits + .iter() + .map(|hit| hit.entry.registry_id.clone()) + .collect() + } + + /// Finding F5, the three queries the QA run measured. The unfixed matcher + /// asked whether the whole query was a substring of one field, so the + /// phrase returned `total: 0` while each of its words, asked alone, found + /// the skills it names. It now returns their union, ranked: the skill + /// matching three of the four terms, then the two matching two, then the + /// single-term matches. + #[test] + fn a_natural_language_skill_query_returns_the_union_ranked() { + let catalog = MarketplaceCatalog::from_bytes(&skills_snapshot()).unwrap(); + + let phrase = catalog.search_skills("R scripting ggplot visualization"); + assert_eq!(phrase.terms, ["r", "scripting", "ggplot", "visualization"]); + assert_eq!( + skill_ids(&phrase), + [ + "ggplot-visualization", + "r-scripting", + "data-visualization", + "python-scripting", + "clinical-biostatistics", + ], + "measured before this fix: no hits at all" + ); + assert_eq!( + phrase.hits[0].matched_terms, + ["r", "ggplot", "visualization"] + ); + assert!( + !skill_ids(&phrase).contains(&"scientific-visual-communication".to_owned()), + "`visual` is not `visualization`: a long term must be found in the entry, not the \ + other way round" + ); + + // The two single-term controls, measured in the same chat as 2 and 1. + assert_eq!( + skill_ids(&catalog.search_skills("ggplot")), + ["ggplot-visualization", "data-visualization"] + ); + let id = catalog.search_skills("r-scripting"); + assert_eq!( + skill_ids(&id)[0], + "r-scripting", + "the skill the query names ranks first, ahead of the other skills about R or \ + scripting" + ); + assert_eq!(id.hits[0].matched_terms, ["r", "scripting"]); + + // The empty query stays the browse case, 129 of 129 in the QA run. + assert_eq!( + catalog.search_skills("").len(), + catalog.browse_skills().len() + ); + } + + /// The same property against the registry that ships in the binary — the + /// offline fallback, and the snapshot of the live registry the QA run read. + /// Only the measured SHAPE is pinned, so a new skill cannot fail it: the + /// phrase finds every skill that each of its words finds alone. + #[test] + fn on_the_shipped_registry_a_phrase_finds_what_each_of_its_words_finds() { + let catalog = MarketplaceCatalog::from_bytes(EMBEDDED_REGISTRY).unwrap(); + let phrase = skill_ids(&catalog.search_skills("R scripting ggplot visualization")); + for word in ["ggplot", "r-scripting", "scripting", "visualization"] { + for id in skill_ids(&catalog.search_skills(word)) { + assert!( + phrase.contains(&id), + "`{word}` alone finds `{id}`, the phrase containing it does not: {phrase:?}" + ); + } + } + assert!(phrase.len() >= 2, "{phrase:?}"); + } + + /// The extension catalog shares the matcher, and the caller filter runs + /// before the ranking: a public caller's multi-word query can match the + /// private row's words and still never be shown it. + #[test] + fn a_multi_word_extension_query_ranks_only_what_the_caller_may_see() { + let catalog = MarketplaceCatalog::from_bytes(®istry("public")).unwrap(); + let ids = |caller| -> Vec { + catalog + .search_extensions(caller, "private fixture") + .hits + .iter() + .map(|hit| hit.entry.registry_id.clone()) + .collect() + }; + assert_eq!(ids(ProviderTier::Public), ["public-agent"]); + assert_eq!( + ids(ProviderTier::Private), + ["private-agent", "public-agent"], + "the row matching both terms ranks first" + ); + } + #[test] fn a_later_public_registry_row_cannot_lower_learned_private_authority() { let mut lowered: serde_json::Value = serde_json::from_slice(®istry("public")).unwrap(); diff --git a/crates/biorouter/src/marketplace/search.rs b/crates/biorouter/src/marketplace/search.rs new file mode 100644 index 000000000..52dacae22 --- /dev/null +++ b/crates/biorouter/src/marketplace/search.rs @@ -0,0 +1,433 @@ +//! Free-text search over the trusted marketplace catalog — the matcher behind +//! both `skills__searchMarketplaceSkills` and +//! `extensionmanager__search_marketplace_extensions`. +//! +//! ⚠ **A query is a set of words, not a substring.** The matcher this replaced +//! asked whether the WHOLE lowercased query occurred inside a single field, so a +//! one-word query worked and every phrase failed. Measured in the 2026-09-10 +//! composer QA run (finding F5), one chat, one live registry: +//! `R scripting ggplot visualization` → `total: 0`, while `ggplot` → 2 and +//! `r-scripting` → 1. A model composes exactly that phrase on a user's behalf, +//! so the shape that failed was the common one, and the model went on to tell +//! the user the marketplace had nothing. +//! +//! So a query is split into terms and an entry is a hit when it matches ANY of +//! them. The union is deliberate: no single entry has to contain every word a +//! user happened to say, and an AND over a phrase is the same empty answer with +//! a different cause. Precision comes from the ranking instead, best first: +//! +//! 1. an entry containing the query **verbatim** — what the old matcher found, +//! so nothing it returned is lost; +//! 2. then by **how many terms** it matched, so an entry matching every term +//! precedes one matching some; +//! 3. then by **where** each term matched — the id or name outweighs a tag, +//! which outweighs the description — and how exactly (the whole word, the +//! start of one, or inside one); +//! 4. then registry order, which is by id, so a result never reshuffles. +//! +//! Two rules keep the union from drowning the useful hits, and both were needed +//! by the measured query itself: +//! +//! * **A term under three characters matches whole words only.** `r` has to +//! find the R language; as a substring it matched nearly every entry. +//! * **Filler words are dropped** ("a skill about R" is `r`), because in a union +//! a word like `for` or `about` inflates the term count of every entry whose +//! prose happens to use it, which ranked noise above the real hit. + +/// How much a match in one field says about an entry. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(super) enum Weight { + /// Free prose: a description. + Prose = 1, + /// Curated labels: tags, keywords, a category, an organization. + Label = 2, + /// What the entry is called: its registry id and names. + Name = 3, +} + +/// Words that say how a request is phrased, not what it is for. Dropped from a +/// query unless nothing else is left, so a query made only of them (`agent`, +/// `or`) still searches for what it says. +const FILLER: &[&str] = &[ + "a", + "about", + "an", + "and", + "any", + "are", + "baam", + "be", + "by", + "can", + "do", + "does", + "find", + "for", + "from", + "help", + "how", + "i", + "in", + "into", + "is", + "it", + "its", + "looking", + "marketplace", + "me", + "my", + "need", + "of", + "on", + "or", + "please", + "search", + "some", + "that", + "the", + "this", + "to", + "use", + "using", + "via", + "want", + "what", + "which", + "with", +]; + +/// Filler specific to the skills catalog: every entry in it is a skill. +pub(super) const SKILL_NOISE: &[&str] = &["skill", "skills"]; + +/// Filler specific to the extensions catalog: every entry in it is an extension. +pub(super) const EXTENSION_NOISE: &[&str] = &["extension", "extensions"]; + +/// Below this many characters a term matches whole words only. +const MIN_PARTIAL_CHARS: usize = 3; + +/// One entry a search returned, with the query terms it matched. +#[derive(Debug)] +pub struct MarketplaceSearchHit<'a, T> { + pub entry: &'a T, + /// The terms this entry matched, in query order. Empty only for an entry + /// that nothing but the verbatim query found. + pub matched_terms: Vec, +} + +/// A ranked search: the terms the query was split into, and every entry that +/// matched at least one of them (or the whole query verbatim), best first. +#[derive(Debug)] +pub struct MarketplaceSearch<'a, T> { + /// What the query was read as, after filler words were dropped. Reported + /// to the model so a surprising result can be traced to its input. + pub terms: Vec, + pub hits: Vec>, +} + +impl MarketplaceSearch<'_, T> { + pub fn len(&self) -> usize { + self.hits.len() + } + + pub fn is_empty(&self) -> bool { + self.hits.is_empty() + } + + /// What was searched for, as a sentence fragment for an empty result: + /// "any of the terms `r`, `ggplot`", or "the query `…`" when the query + /// held no word at all. + pub fn describe_query(&self, query: &str) -> String { + if self.terms.is_empty() { + return format!("the query `{query}`"); + } + let terms = self + .terms + .iter() + .map(|term| format!("`{term}`")) + .collect::>() + .join(", "); + format!("any of the terms {terms}") + } +} + +/// Lowercase words, split at every character that is not a letter or digit — +/// whitespace and punctuation alike, so `r-scripting` is `r` + `scripting` and +/// `ggplot2` stays one word. +fn words(text: &str) -> impl Iterator + '_ { + text.split(|c: char| !c.is_alphanumeric()) + .filter(|word| !word.is_empty()) + .map(str::to_lowercase) +} + +/// The distinct terms of `query`, in the order written, without filler. +pub(super) fn terms(query: &str, noise: &[&str]) -> Vec { + let mut all: Vec = Vec::new(); + for word in words(query) { + if !all.contains(&word) { + all.push(word); + } + } + let meaningful: Vec = all + .iter() + .filter(|term| !FILLER.contains(&term.as_str()) && !noise.contains(&term.as_str())) + .cloned() + .collect(); + if meaningful.is_empty() { + all + } else { + meaningful + } +} + +/// How well `term` matches one field word: 3 for the whole word, 2 for its +/// start, 1 for anywhere inside it (`heatmap` in `complexheatmap`), 0 for no +/// match. A short term matches whole words only. +fn strength(term: &str, word: &str) -> u32 { + if word == term { + 3 + } else if term.chars().count() < MIN_PARTIAL_CHARS { + 0 + } else if word.starts_with(term) { + 2 + } else if word.contains(term) { + 1 + } else { + 0 + } +} + +/// `term`'s strength against `word`, falling back to its singular so +/// `visualizations` finds `visualization` and `heatmaps` finds `heatmap`. +/// `class` and `gis` are left alone. +fn term_strength(term: &str, word: &str) -> u32 { + let direct = strength(term, word); + if direct > 0 { + return direct; + } + match term.strip_suffix('s') { + Some(stem) if stem.chars().count() >= MIN_PARTIAL_CHARS && !stem.ends_with('s') => { + strength(stem, word) + } + _ => 0, + } +} + +/// Rank `entries` against `query`. `fields` names the text of one entry that +/// is searched, and how much a match there counts. +/// +/// An empty (or all-whitespace) query is the browse case: every entry, in +/// registry order. +pub(super) fn rank<'a, T>( + query: &str, + noise: &[&str], + entries: impl IntoIterator, + fields: impl Fn(&'a T) -> Vec<(&'a str, Weight)>, +) -> MarketplaceSearch<'a, T> { + let phrase = query.trim().to_lowercase(); + if phrase.is_empty() { + return MarketplaceSearch { + terms: Vec::new(), + hits: entries + .into_iter() + .map(|entry| MarketplaceSearchHit { + entry, + matched_terms: Vec::new(), + }) + .collect(), + }; + } + let terms = terms(query, noise); + + let mut ranked = Vec::new(); + for entry in entries { + let fields = fields(entry); + let verbatim = fields + .iter() + .any(|(text, _)| text.to_lowercase().contains(&phrase)); + let entry_words: Vec<(String, u32)> = fields + .iter() + .flat_map(|(text, weight)| words(text).map(move |word| (word, *weight as u32))) + .collect(); + + let mut matched_terms = Vec::new(); + let mut score = 0; + for term in &terms { + let best = entry_words + .iter() + .map(|(word, weight)| term_strength(term, word) * weight) + .max() + .unwrap_or(0); + if best > 0 { + matched_terms.push(term.clone()); + score += best; + } + } + if verbatim || !matched_terms.is_empty() { + ranked.push(( + (verbatim, matched_terms.len(), score), + MarketplaceSearchHit { + entry, + matched_terms, + }, + )); + } + } + // Stable, and descending on the key: equal ranks keep registry order. + ranked.sort_by(|(left, _), (right, _)| right.cmp(left)); + MarketplaceSearch { + terms, + hits: ranked.into_iter().map(|(_, hit)| hit).collect(), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + struct Entry { + id: &'static str, + name: &'static str, + description: &'static str, + tags: &'static [&'static str], + } + + fn fields(entry: &Entry) -> Vec<(&str, Weight)> { + let mut fields = vec![ + (entry.id, Weight::Name), + (entry.name, Weight::Name), + (entry.description, Weight::Prose), + ]; + fields.extend(entry.tags.iter().map(|tag| (*tag, Weight::Label))); + fields + } + + fn ids<'a>(search: &MarketplaceSearch<'a, Entry>) -> Vec<&'a str> { + search.hits.iter().map(|hit| hit.entry.id).collect() + } + + const ENTRIES: &[Entry] = &[ + Entry { + id: "complex-plots", + name: "Complex Plots", + description: "Draws annotated heat maps with the ComplexHeatmap package.", + tags: &["ComplexHeatmap"], + }, + Entry { + id: "prose-only", + name: "Prose Only", + description: "Mentions scripting in passing.", + tags: &[], + }, + Entry { + id: "r-scripting", + name: "R Scripting", + description: "Tidyverse conventions for R code.", + tags: &["R"], + }, + ]; + + #[test] + fn a_query_is_split_at_whitespace_and_punctuation_and_lowercased() { + assert_eq!( + terms("R scripting, ggplot/Visualization", &[]), + ["r", "scripting", "ggplot", "visualization"] + ); + assert_eq!(terms("r-scripting", &[]), ["r", "scripting"]); + assert_eq!( + terms("ggplot2 ggplot2", &[]), + ["ggplot2"], + "terms are distinct" + ); + } + + #[test] + fn filler_is_dropped_unless_it_is_all_there_is() { + assert_eq!( + terms("a skill about R scripting or ggplot", SKILL_NOISE), + ["r", "scripting", "ggplot"] + ); + assert_eq!(terms("skills", SKILL_NOISE), ["skills"]); + assert_eq!( + terms("skills", EXTENSION_NOISE), + ["skills"], + "a catalog's own noise words are its own" + ); + } + + /// `r` as a substring is in nearly every word of prose; as a term it must + /// mean the R language. + #[test] + fn a_short_term_matches_whole_words_only() { + let search = rank("R scripting", &[], ENTRIES, fields); + let prose = search + .hits + .iter() + .find(|hit| hit.entry.id == "prose-only") + .expect("`scripting` is in its description"); + assert_eq!( + prose.matched_terms, + ["scripting"], + "the `r` inside `scripting` is not the R language" + ); + assert!( + !ids(&search).contains(&"complex-plots"), + "nor is the `r` inside `Draws`" + ); + } + + #[test] + fn a_long_term_matches_inside_a_word_and_a_plural_finds_its_singular() { + assert_eq!( + ids(&rank("heatmap", &[], ENTRIES, fields)), + ["complex-plots"], + "`heatmap` inside `complexheatmap`" + ); + assert_eq!( + ids(&rank("heatmaps", &[], ENTRIES, fields)), + ["complex-plots"], + "no field says `heatmaps`; its singular is inside `complexheatmap`" + ); + assert_eq!( + ids(&rank("scripts", &[], ENTRIES, fields)), + ["r-scripting", "prose-only"], + "`scripts` is not in `scripting`, but `script` starts it" + ); + } + + /// The union, ranked: every entry matching a term is returned, the one + /// matching more terms first, and a name match ahead of a prose match. + #[test] + fn hits_are_the_union_ranked_by_terms_matched_then_by_where() { + let search = rank("R scripting", &[], ENTRIES, fields); + assert_eq!(ids(&search), ["r-scripting", "prose-only"]); + assert_eq!(search.hits[0].matched_terms, ["r", "scripting"]); + assert_eq!(search.hits[1].matched_terms, ["scripting"]); + + let by_place = rank("scripting", &[], ENTRIES, fields); + assert_eq!( + ids(&by_place), + ["r-scripting", "prose-only"], + "a match in the name outranks the same match in the description" + ); + } + + /// Everything the substring matcher found is still found: a query that + /// occurs verbatim in a field is a hit even when its terms are too short + /// to match on their own. + #[test] + fn a_verbatim_occurrence_is_still_a_hit() { + let search = rank("s p", &[], ENTRIES, fields); + assert!(search.is_empty(), "neither `s` nor `p` is a whole word"); + + let search = rank("dy", &[], ENTRIES, fields); + assert_eq!(ids(&search), ["r-scripting"], "`dy` inside `Tidyverse`"); + assert!(search.hits[0].matched_terms.is_empty()); + } + + #[test] + fn an_empty_query_browses_in_registry_order() { + let search = rank(" ", &[], ENTRIES, fields); + assert!(search.terms.is_empty()); + assert_eq!(ids(&search), ["complex-plots", "prose-only", "r-scripting"]); + } +} diff --git a/docs/extensions/built-in/extension-manager.md b/docs/extensions/built-in/extension-manager.md index ba0df2358..ea7581ff6 100644 --- a/docs/extensions/built-in/extension-manager.md +++ b/docs/extensions/built-in/extension-manager.md @@ -62,12 +62,12 @@ The result is a more focused session where BioRouter has exactly the tools it ne | Tool | Description | Use Case | |------|-------------|----------| | `search_available_extensions` | List installed third-party extensions and exact names | Finding what is already installed | -| `search_marketplace_extensions` | Browse or search trusted BAAM entries visible to this model — omit the query to list everything | Finding a package to install | +| `search_marketplace_extensions` | Browse or search trusted BAAM entries visible to this model — omit the query to list everything. A query is matched word by word, so a phrase returns every entry matching any of its words, best match first; a query that matches nothing says so and suggests shorter terms | Finding a package to install | | `manage_extensions` | Enable or disable an extension by name | Loading/unloading extensions dynamically | | `install_extension` | Install a BAAM marketplace extension end to end | The extension is not installed at all | | `delete_extension_package` | Delete one or up to 50 validated marketplace packages after approval | Permanent removal of something installed from BAAM; shared credentials are retained | | `remove_extension` | Remove one or up to 50 installed extensions by installed name after approval | Permanent removal of anything else — a sideloaded `.brxt`, a hand-configured MCP server | -| `list_resources` | List resources from extensions (if supported) | Discovering available data sources | +| `list_resources` | List resources from extensions (if supported). When there are none, it says which extensions were asked — naming only extensions the model has already been shown | Discovering available data sources | | `read_resource` | Read specific resource content (if supported) | Accessing extension-provided data | > **Tip.** Not every tool in this table is offered in every session. The resource tools (`list_resources` and `read_resource`) appear only when at least one enabled extension supports resources. `install_extension`, `delete_extension_package` and `remove_extension` each wait on your approval, so they are withheld entirely where no one can be asked for it — on a daemon started by `biorouter serve` for browser access, for instance. Browsing and searching are read-only and always available.