diff --git a/crates/biorouter/src/catalog_search.rs b/crates/biorouter/src/catalog_search.rs index cfaf4ef11..7aee905e4 100644 --- a/crates/biorouter/src/catalog_search.rs +++ b/crates/biorouter/src/catalog_search.rs @@ -174,6 +174,52 @@ fn words(text: &str) -> impl Iterator + '_ { .map(str::to_lowercase) } +/// Does `label` say nothing that `license` does not — is every word of it a word +/// of the licence? A catalog whose entries carry a licence drops such a label +/// when it assembles an entry's searchable text. +/// +/// The licence itself is not a searchable field: every entry in the BAAM +/// registry is Apache-2.0, so it separates nothing, and both catalog searches +/// leave the field out for that reason. +/// +/// ⚠ **Leaving the FIELD out was not enough.** A registry republishes the licence +/// as one of the entry's own tag chips — and, for a skill, again among its +/// keywords — and labels are searched, rightly: `MCP`, `ELN` and `Imaging` are +/// exactly what a tag is for. Measured in the Browse-extensions modal on +/// 2026-09-12 against the live 37-entry registry, with the field already gone: +/// `PACS` → 31 of 37, `pac` → 31, `apache` → 31, and not one of the 31 about +/// PACS. The three counts agreeing is the identification — `PACS` reaches `pac` +/// through the plural fallback in [`term_strength`], `pac` is inside `apache`, +/// and 31 rows wear an `Apache-2.0` chip. Removing the field had moved the defect +/// one field over, where a test asserting "the licence is not searched" still +/// passed. +/// +/// Compared by WORDS rather than by equality, because the second spelling is not +/// the first: the tag is `Apache-2.0` and the keyword is `apache`. An equality +/// test drops the tag and keeps the keyword, which is the same half-fix again. +/// +/// What this deliberately does not do: drop every label (`MCP`, `Imaging`, `ELN`, +/// `Registry` are real search value), or name a licence in the matcher +/// (`Apache`, `MIT` — the next licence reopens the hole). The cost of the word +/// test is a licence id built from a topical word — `Python-2.0`, `Ruby`, +/// `PostgreSQL` — on an entry that also tags itself with that word; the tag is +/// then dropped for saying only what the licence says. No entry in the shipped +/// registry is such a case (measured over all 166: the rule drops the 129 licence +/// labels and nothing else), and an equality test pays a smaller version of the +/// same cost. +pub(crate) fn names_only_the_license(label: &str, license: &str) -> bool { + let license_words: Vec = words(license).collect(); + let mut label_words = words(label); + match label_words.next() { + // An empty label says nothing at all, which is not the same as saying + // only the licence: leave it, so the rule stays about the licence. + None => false, + Some(first) => { + license_words.contains(&first) && label_words.all(|word| license_words.contains(&word)) + } + } +} + /// The distinct terms of `query`, in the order written, without filler. fn terms(query: &str, noise: &[&str]) -> Vec { let mut all: Vec = Vec::new(); @@ -556,4 +602,45 @@ mod tests { assert!(search.terms.is_empty()); assert_eq!(ids(&search), ["complex-plots", "prose-only", "r-scripting"]); } + + /// The rule a catalog applies to its own labels. Both spellings the BAAM + /// registry publishes go, which is the whole point — `Apache-2.0` is the tag + /// and `apache` is the keyword, and an equality test would keep the second + /// and leave `PACS` matching 49 skills through it. + #[test] + fn a_label_naming_only_the_licence_is_recognised_in_either_spelling() { + for label in [ + "Apache-2.0", + "apache", + "APACHE", + "apache 2.0", + "2.0", + "Apache/2.0", + ] { + assert!( + names_only_the_license(label, "Apache-2.0"), + "`{label}` says nothing `Apache-2.0` does not" + ); + } + // A label that says anything else stays searchable, including one that + // merely contains a word of the licence. + for label in [ + "Apache Spark", + "MCP", + "ELN", + "Imaging", + "Registry", + "apachex", + ] { + assert!( + !names_only_the_license(label, "Apache-2.0"), + "`{label}` says more than the licence" + ); + } + // An empty label says nothing at all, which is not the same as saying + // only the licence; and an entry with no licence has none to drop. + assert!(!names_only_the_license("", "Apache-2.0")); + assert!(!names_only_the_license(" - ", "Apache-2.0")); + assert!(!names_only_the_license("Apache-2.0", "")); + } } diff --git a/crates/biorouter/src/marketplace.rs b/crates/biorouter/src/marketplace.rs index ba894b424..c43a797ec 100644 --- a/crates/biorouter/src/marketplace.rs +++ b/crates/biorouter/src/marketplace.rs @@ -8,7 +8,9 @@ use futures::StreamExt; use serde::Deserialize; use url::Url; -use crate::catalog_search::{rank, CatalogSearch, Weight, EXTENSION_NOISE, SKILL_NOISE}; +use crate::catalog_search::{ + names_only_the_license, rank, CatalogSearch, Weight, EXTENSION_NOISE, SKILL_NOISE, +}; use crate::config::paths::Paths; use crate::privacy::affiliation::InstitutionId; use crate::privacy::{ExtensionAffiliation, ProviderTier}; @@ -133,7 +135,13 @@ impl MarketplaceCatalog { (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.extend( + entry + .tags + .iter() + .filter(|tag| !names_only_the_license(tag, &entry.license)) + .map(|tag| (tag.as_str(), Weight::Label)), + ); fields }, ) @@ -153,11 +161,18 @@ impl MarketplaceCatalog { (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 + .tags + .iter() + .filter(|tag| !names_only_the_license(tag, &entry.license)) + .map(|tag| (tag.as_str(), Weight::Label)), + ); fields.extend( entry .keywords .iter() + .filter(|keyword| !names_only_the_license(keyword, &entry.license)) .map(|keyword| (keyword.as_str(), Weight::Label)), ); fields @@ -1033,6 +1048,131 @@ mod tests { assert!(phrase.len() >= 2, "{phrase:?}"); } + /// A licence is not searchable, and leaving the FIELD out did not make that + /// true. Measured in the Browse-extensions modal on 2026-09-12 against the + /// live 37-entry registry, with the field already gone: `PACS` → 31 of 37, + /// `pac` → 31, `apache` → 31, `Apache-2.0` → 32, empty → 37, `zzzznope` → 0. + /// The three counts agreeing is the identification — `PACS` reaches `pac` + /// through the plural fallback, `pac` is inside `apache`, and 31 rows + /// republish `Apache-2.0` as one of their own TAG chips, which are searched. + /// None of the 31 was about PACS. + /// + /// What this must NOT do is narrow the substring rule: `pac` is inside + /// "PacBio", "package" and "workspace", and those hits stay. + #[test] + fn a_licence_republished_as_a_label_is_not_searchable_through_it() { + let catalog = MarketplaceCatalog::from_bytes(EMBEDDED_REGISTRY).unwrap(); + + // A word of a licence, as a whole label. Spelled out here rather than + // taken from `catalog_search` so this test cannot be satisfied by the + // same mistake the fix makes. + let is_a_licence_word = |license: &str, label: &str| { + license + .split(|c: char| !c.is_alphanumeric()) + .filter(|word| !word.is_empty()) + .any(|word| word.eq_ignore_ascii_case(label)) + }; + + // Guard. If the registry stops republishing its licence as a label there + // is nothing here to refuse, and every assertion below would pass while + // proving nothing — which is exactly how the fix before this one looked + // covered. Three counts, because there are three label paths. + let tagged_extensions = catalog + .browse_extensions(ProviderTier::Private) + .iter() + .filter(|entry| { + entry + .tags + .iter() + .any(|tag| tag.eq_ignore_ascii_case(&entry.license)) + }) + .count(); + let tagged_skills = catalog + .browse_skills() + .iter() + .filter(|entry| { + entry + .tags + .iter() + .any(|tag| tag.eq_ignore_ascii_case(&entry.license)) + }) + .count(); + let keyworded_skills = catalog + .browse_skills() + .iter() + .filter(|entry| { + entry + .keywords + .iter() + .any(|keyword| is_a_licence_word(&entry.license, keyword)) + }) + .count(); + assert!( + tagged_extensions >= 2 && tagged_skills >= 2 && keyworded_skills >= 2, + "the shipped registry no longer republishes its licence as a label \ + (extensions tagged {tagged_extensions}, skills tagged {tagged_skills}, skills \ + keyworded {keyworded_skills}) — this test would pass vacuously" + ); + + // `apache` occurs nowhere in the registry except each entry's own + // licence, so it finds nothing at all. Measured before the fix: 31 + // extensions and 49 skills, none of them about Apache anything. + let extension_ids = |search: &CatalogSearch<'_, MarketplaceExtensionDescriptor>| { + search + .hits + .iter() + .map(|hit| hit.entry.registry_id.clone()) + .collect::>() + }; + assert_eq!( + extension_ids(&catalog.search_extensions(ProviderTier::Private, "apache")), + Vec::::new() + ); + assert_eq!( + skill_ids(&catalog.search_skills("apache")), + Vec::::new() + ); + + // The query as the QA run typed it. The plural fallback and the + // substring rule both stay: a skill that really says PACS is found, and + // a skill whose only `pac` was `Apache-2.0` is not. + let pacs = skill_ids(&catalog.search_skills("PACS")); + assert!( + pacs.contains(&"biomedical-imaging-pathology".to_owned()), + "a skill whose keywords say `pacs` must still be found: {pacs:?}" + ); + assert!( + pacs.contains(&"long-read-sequencing".to_owned()), + "`pac` inside `PacBio` is the substring rule working, not the defect: {pacs:?}" + ); + for licence_only in ["empirical-research-router", "causal-identification-gates"] { + assert!( + !pacs.contains(&licence_only.to_owned()), + "`{licence_only}`'s only `pac` is its `Apache-2.0` label: {pacs:?}" + ); + } + let pacs_extensions = + extension_ids(&catalog.search_extensions(ProviderTier::Private, "PACS")); + for licence_only in ["benchlingagent", "dnanexusagent", "omeroagent"] { + assert!( + !pacs_extensions.contains(&licence_only.to_owned()), + "`{licence_only}` is not about PACS; its only `pac` is `Apache-2.0`: \ + {pacs_extensions:?}" + ); + } + + // The browse case is untouched — the licence label is dropped from what + // is SEARCHED, not from the catalog. + assert_eq!( + catalog.search_skills("").len(), + catalog.browse_skills().len() + ); + assert_eq!( + catalog.search_extensions(ProviderTier::Private, "").len(), + catalog.browse_extensions(ProviderTier::Private).len() + ); + } + /// 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. diff --git a/landing/baam.html b/landing/baam.html index c15aff88d..ac7e1f5bd 100644 --- a/landing/baam.html +++ b/landing/baam.html @@ -4089,6 +4089,61 @@

Stay in the loop

} } + /* ── What a card is searched BY ───────────────────────────── + The licence is not part of it. Every card in this catalog is Apache-2.0, so + it separates nothing, and it reached the haystack THREE ways: `data-license` + (on every card), the `Apache-2.0` chip the catalog publishes in the tag row + (part of the card's own textContent, on an authored skill card), and — on a + skill — the `apache` keyword in `data-tags` derived from that chip. Because + `data-license` alone is universal, the shelf was worse than the catalog: + measured here on 2026-09-12, `apache`, `Apache-2.0` and `pac` each returned + ALL 37 extension cards and ALL 132 skill cards, not one of them about Apache + anything. After this: 0, 0, 1 and 0, 0, 7. + + The same overlap in the app's own matchers is worse in a different way, + because those split a query into words and fall a plural back to its + singular: measured in the Browse-extensions modal the same day, `PACS` + returned 31 of 37 extensions via `pac` inside `apache`. Fixed there in one + rule — `names_only_the_license` in crates/biorouter/src/catalog_search.rs + and its port in ui/desktop/src/components/baam/search.ts — and this is the + third copy of it. By WORDS and not by equality, because the chip says + `Apache-2.0` while the keyword says `apache`, and an equality test drops + only the first. + + Only the licence goes. `MCP`, `ELN`, `Imaging` and `Registry` are what a tag + is for, and the chip still RENDERS — this drops it from what is searched, + not from the card. Note that `extCardHtml` and `buildExtChips` already + filter `/^apache/i`, for row space and chip noise rather than for search; + that near-miss is part of why this looked handled. */ + const WORD_BREAK = /[^\p{Alphabetic}\p{N}]+/u; + + function wordsOf(text) { + return String(text == null ? '' : text).toLowerCase().split(WORD_BREAK).filter(Boolean); + } + + function namesOnlyTheLicense(label, licenseWords) { + const words = wordsOf(label); + return words.length > 0 && words.every(w => licenseWords.indexOf(w) !== -1); + } + + function searchHaystack(card) { + const licenseWords = wordsOf(card.dataset.license); + // A chip is part of `textContent`, so a licence chip is removed from that + // string rather than skipped while assembling one. + let text = card.textContent; + if (licenseWords.length) { + card.querySelectorAll('.tag').forEach(chip => { + const label = chip.textContent; + if (label && namesOnlyTheLicense(label, licenseWords)) text = text.split(label).join(' '); + }); + } + // Separators are preserved and only a licence token is blanked, so a phrase + // spanning two keywords still matches as it did. + const tags = String(card.dataset.tags || '') + .replace(/[^\s,]+/g, t => (namesOnlyTheLicense(t, licenseWords) ? '' : t)); + return (text + ' ' + tags).toLowerCase(); + } + function filterExtensions(q) { const bar = document.getElementById('ext-chips'); const active = activeChips(bar); @@ -4099,7 +4154,7 @@

Stay in the loop

const cards = document.querySelectorAll('#extensions-section .ext-card'); let visible = 0; cards.forEach(c => { - const hay = (c.textContent + ' ' + (c.dataset.tags || '') + ' ' + (c.dataset.license || '')).toLowerCase(); + const hay = searchHaystack(c); let show = (!q || hay.indexOf(q) !== -1); if (show) { for (let i = 0; i < facets.length; i++) { @@ -4179,7 +4234,7 @@

Stay in the loop

const cards = document.querySelectorAll('#skills-section .skill-card'); let visible = 0; cards.forEach(c => { - const hay = (c.textContent + ' ' + (c.dataset.tags || '') + ' ' + (c.dataset.license || '')).toLowerCase(); + const hay = searchHaystack(c); let show = (!q || hay.indexOf(q) !== -1); if (show) { for (let i = 0; i < facets.length; i++) { @@ -4230,7 +4285,7 @@

Stay in the loop

const cards = document.querySelectorAll('#workflows-section .wf-card'); let visible = 0; cards.forEach(c => { - const hay = (c.textContent + ' ' + (c.dataset.tags || '')).toLowerCase(); + const hay = searchHaystack(c); const show = !q || hay.indexOf(q) !== -1; c.style.display = show ? '' : 'none'; if (show) visible++; diff --git a/landing/scripts/baam-privacy-facet.test.mjs b/landing/scripts/baam-privacy-facet.test.mjs index dbadbb24f..12b736638 100644 --- a/landing/scripts/baam-privacy-facet.test.mjs +++ b/landing/scripts/baam-privacy-facet.test.mjs @@ -551,6 +551,105 @@ if (!existsSync(PLAYWRIGHT)) { } }); + + /** + * Searching the shelf must not search the LICENCE. Every card in this catalog + * is Apache-2.0 and the catalog publishes that licence three more times per + * card — as `data-license`, as a chip in the tag row (part of the card's own + * textContent), and on a skill as the `apache` keyword in `data-tags` — so a + * haystack built from all of them answered "apache" with the whole shelf. + * + * The same overlap in the app's own matchers is worse, because those split a + * query into words and fall a plural back to its singular: measured in the + * Browse-extensions modal on 2026-09-12, `PACS` returned 31 of 37 extensions + * through `pac` inside `apache`, none of them about PACS. + * + * Driven through the real input, not by calling `filterExtensions` — the + * `oninput` attribute, `runFilter`'s trim/lowercase and the shelf's own + * visibility rules are all part of what a person experiences here. + */ + async function search(page, query) { + await page.fill('#baam-search', query); + await page.waitForTimeout(0); + return shownCards(page); + } + + test('a licence is not something a card is searched by', async () => { + const page = await shelfPage(); + + // Guard: every assertion below is vacuous if the licence stops reaching the + // haystack, which is what is being closed. On a RENDERED extension card it + // arrives two ways — `data-license`, and the `Apache-2.0` token inside the + // `data-tags` keyword blob. (Not as a chip: `extCardHtml` already drops an + // `/^apache/i` tag from the tag row, for space rather than for search, and + // that one hard-coded filter is exactly why this defect looked fixed.) + const licenceCarriers = await page.evaluate(() => + [...document.querySelectorAll('#extensions-section .ext-card')].filter((card) => { + const licence = (card.dataset.license || '').toLowerCase(); + if (!licence) return false; + return (card.dataset.tags || '') + .split(/\s+/) + .some((token) => token.toLowerCase() === licence); + }).length + ); + assert.ok( + licenceCarriers >= 2, + `only ${licenceCarriers} cards carry their own licence into the haystack — this test proves nothing` + ); + + const all = await search(page, ''); + for (const query of ['apache', 'Apache-2.0', 'APACHE']) { + assert.deepEqual( + await search(page, query), + [], + `"${query}" is a licence, not a capability — it matched cards before this fix` + ); + } + + // And nothing else moved: a real tag, a name, a data source, and browsing. + assert.deepEqual(await search(page, ''), all); + assert.ok((await search(page, 'MCP')).length >= 10, 'a real tag must still match'); + assert.deepEqual(await search(page, 'spokeagent'), ['spokeagent']); + assert.ok((await search(page, 'imaging')).length >= 2, '`imaging` is a capability, not a licence'); + await page.close(); + }); + + test('a skill card is not searched by its licence either', async () => { + // The skills shelf carries the licence a third way — the `apache` keyword in + // `data-tags`, which is not spelled like the `Apache-2.0` chip, so a rule + // comparing a label to the licence for EQUALITY leaves this one matching. + const page = await shelfPage(); + await page.click('.baam-tab[data-shelf="skills"]'); + await page.waitForSelector('#skills-section .skill-card'); + + const visibleSkills = () => + page.$$eval('#skills-section .skill-card:visible', (els) => els.length); + const keyworded = await page.evaluate(() => + [...document.querySelectorAll('#skills-section .skill-card')].filter((card) => + (card.dataset.tags || '').split(/\s+/).includes('apache') + ).length + ); + assert.ok(keyworded >= 2, `only ${keyworded} skill cards carry an \`apache\` keyword`); + // A skill card is authored, not rendered, so it DOES wear the licence chip — + // the one path that lives inside `textContent` rather than a data attribute. + const chipped = await page.evaluate(() => + [...document.querySelectorAll('#skills-section .skill-card')].filter((card) => + [...card.querySelectorAll('.tag')].some( + (chip) => + chip.textContent.trim().toLowerCase() === (card.dataset.license || '').toLowerCase() + ) + ).length + ); + assert.ok(chipped >= 2, `only ${chipped} skill cards wear their own licence as a chip`); + + await page.fill('#baam-search', 'apache'); + assert.equal(await visibleSkills(), 0, 'a licence keyword matched skill cards'); + + await page.fill('#baam-search', 'ggplot'); + assert.ok((await visibleSkills()) >= 1, '`ggplot` must still match'); + await page.close(); + }); + test('a well-formed registry still renders', async () => { // Without this, "refuse to render" is satisfiable by never rendering, and // every other test in this file would be reading static markup. diff --git a/ui/desktop/src/components/baam/registry.ts b/ui/desktop/src/components/baam/registry.ts index d00e3ddb2..f5197a97f 100644 --- a/ui/desktop/src/components/baam/registry.ts +++ b/ui/desktop/src/components/baam/registry.ts @@ -10,6 +10,7 @@ import fallback from './registry.fallback.json'; import { classifyExtension } from '../settings/extensions/extensionPrivacy'; import { EXTENSION_NOISE, + namesOnlyTheLicense, rankEntries, SKILL_NOISE, Weight, @@ -378,12 +379,25 @@ export function catalogFreshnessLine(load: { live: boolean; fetchedAt?: string } } /** - * Every label in a list, as a search field. Total, because an entry can omit - * any field — `isRegistryDocument` checks only that an entry is an object — and - * a search that throws in render takes the whole modal with it. + * Every label in a list, as a search field — except one that says nothing the + * entry's own `license` does not. Total, because an entry can omit any field — + * `isRegistryDocument` checks only that an entry is an object — and a search that + * throws in render takes the whole modal with it. + * + * The licence is dropped from what is SEARCHED, not from the catalog: the chip + * still renders on the card. Why a label carrying the licence has to go, and why + * removing the licence FIELD did not do it, is + * {@link namesOnlyTheLicense | documented in `search.ts`} — the rule belongs + * there, with its counterpart in `catalog_search.rs`. */ -function labelFields(labels: readonly string[] | undefined): SearchField[] { - return Array.isArray(labels) ? labels.map((label): SearchField => [label, Weight.Label]) : []; +function labelFields( + labels: readonly string[] | undefined, + license: string | undefined +): SearchField[] { + if (!Array.isArray(labels)) return []; + return labels + .filter((label) => typeof label === 'string' && !namesOnlyTheLicense(label, license)) + .map((label): SearchField => [label, Weight.Label]); } /** @@ -397,6 +411,9 @@ function labelFields(labels: readonly string[] | undefined): SearchField[] { * which the whole-phrase matcher searched: every skill and extension in the * registry is Apache-2.0, so it separates nothing, and under word matching it * made `PACS` list every skill — a plural's singular, `pac`, is inside `apache`. + * ⚠ Excluding the FIELD was not enough, because the registry publishes the + * licence again as a tag and a keyword, and those are searched — see + * {@link namesOnlyTheLicense}, which is what {@link labelFields} applies. */ export function rankSkills( skills: readonly RegistrySkill[], @@ -407,8 +424,8 @@ export function rankSkills( [skill.name, Weight.Name], [skill.category, Weight.Label], [skill.description, Weight.Prose], - ...labelFields(skill.tags), - ...labelFields(skill.keywords), + ...labelFields(skill.tags, skill.license), + ...labelFields(skill.keywords, skill.license), ]); } @@ -427,6 +444,6 @@ export function rankExtensions( [ext.name, Weight.Name], [ext.organization, Weight.Label], [ext.description, Weight.Prose], - ...labelFields(ext.tags), + ...labelFields(ext.tags, ext.license), ]); } diff --git a/ui/desktop/src/components/baam/search.test.ts b/ui/desktop/src/components/baam/search.test.ts index b555f9784..9ad410f31 100644 --- a/ui/desktop/src/components/baam/search.test.ts +++ b/ui/desktop/src/components/baam/search.test.ts @@ -1,9 +1,16 @@ import { describe, expect, it } from 'vitest'; import { MARKETPLACE_EXTENSIONS, MARKETPLACE_SKILLS } from './marketplace.fixture'; -import { rankExtensions, rankSkills, type RegistryExtension, type RegistrySkill } from './registry'; +import { + FALLBACK_REGISTRY, + rankExtensions, + rankSkills, + type RegistryExtension, + type RegistrySkill, +} from './registry'; import { EXTENSION_NOISE, isBrowseQuery, + namesOnlyTheLicense, parseQuery, rankEntries, scoreEntry, @@ -431,6 +438,44 @@ describe('the fields each catalog searches, and what a match in each is worth', expect(rankExtensions([{ ...blankExtension, license: 'Apache-2.0' }], 'PACS').hits).toEqual([]); }); + /// ⚠ And not through a LABEL either, which is where the licence went on + /// holding the whole catalog after the field was dropped — the case the test + /// above cannot see, because an entry with no tags has nowhere for it to hide. + /// The registry publishes the licence a second time as one of the entry's own + /// tag chips and, for a skill, a third time among its keywords, and both are + /// searched. `Apache-2.0` and `apache` are both dropped, because they are the + /// same licence spelled two ways and an equality test would keep the second. + it('does not search the license republished as a tag or a keyword', () => { + const licensed = { license: 'Apache-2.0' }; + for (const query of ['PACS', 'pac', 'apache', 'Apache-2.0']) { + expect( + rankSkills([{ ...blankSkill, ...licensed, tags: ['Apache-2.0'] }], query).hits, + `skill tag, ${query}` + ).toEqual([]); + expect( + rankSkills([{ ...blankSkill, ...licensed, keywords: ['apache'] }], query).hits, + `skill keyword, ${query}` + ).toEqual([]); + expect( + rankExtensions([{ ...blankExtension, ...licensed, tags: ['Apache-2.0'] }], query).hits, + `extension tag, ${query}` + ).toEqual([]); + } + + // Only the licence goes. A label that says anything else stays searchable, + // including one that merely CONTAINS a word of the licence. + expect( + rankSkills([{ ...blankSkill, ...licensed, tags: ['Apache Spark'] }], 'spark').hits + ).toHaveLength(1); + expect( + rankExtensions([{ ...blankExtension, ...licensed, tags: ['ELN'] }], 'eln').hits + ).toHaveLength(1); + // An entry with no licence has no licence label to drop. + expect( + rankExtensions([{ ...blankExtension, tags: ['Apache-2.0'] }], 'apache').hits + ).toHaveLength(1); + }); + it('ranks a skill matched by id or name above a label, and a label above prose', () => { const skills: RegistrySkill[] = [ { ...blankSkill, id: 'in-the-description', description: 'Segments zebrafish embryos.' }, @@ -499,6 +544,35 @@ describe('marketplace search — the query as written', () => { }); }); +/// The same cases `catalog_search.rs` asserts for +/// `names_only_the_license`, so the rule is pinned in both languages. Both +/// spellings the registry publishes go — `Apache-2.0` is the tag and `apache` is +/// the keyword, and an equality test would keep the second and leave `PACS` +/// matching 49 skills through it. +describe('namesOnlyTheLicense — which labels a catalog stops searching', () => { + it.each(['Apache-2.0', 'apache', 'APACHE', 'apache 2.0', '2.0', 'Apache/2.0'])( + 'reads `%s` as saying nothing `Apache-2.0` does not', + (label) => { + expect(namesOnlyTheLicense(label, 'Apache-2.0')).toBe(true); + } + ); + + it.each(['Apache Spark', 'MCP', 'ELN', 'Imaging', 'Registry', 'apachex'])( + 'keeps `%s`, which says more than the licence', + (label) => { + expect(namesOnlyTheLicense(label, 'Apache-2.0')).toBe(false); + } + ); + + it('says no for a label with no words, and for an entry with no licence', () => { + // Saying nothing at all is not the same as saying only the licence. + expect(namesOnlyTheLicense('', 'Apache-2.0')).toBe(false); + expect(namesOnlyTheLicense(' - ', 'Apache-2.0')).toBe(false); + expect(namesOnlyTheLicense('Apache-2.0', '')).toBe(false); + expect(namesOnlyTheLicense('Apache-2.0', undefined)).toBe(false); + }); +}); + describe('rankExtensions — the same matcher over the extensions catalog', () => { /// No field holds the phrase as written — SPOKEAgent says "SPOKE biomedical /// knowledge graph" and "spoke-knowledge-graph" — so the matcher this replaced @@ -517,3 +591,123 @@ describe('rankExtensions — the same matcher over the extensions catalog', () = ]); }); }); + +/** + * The finding this fix answers, measured by driving the real Browse-extensions + * modal on 2026-09-12 against the live 37-entry registry — with the licence + * FIELD already excluded by PR #242 and its port PR #255: + * + * | query | matches | + * | ------------ | ------- | + * | *(empty)* | 37 | + * | `PACS` | **31** | + * | `pac` | **31** | + * | `apache` | **31** | + * | `Apache-2.0` | 32 | + * | `zzzznope` | 0 | + * + * The three counts agreeing identifies the path: `PACS` → its singular `pac` → + * inside `apache` → the `Apache-2.0` TAG chip on 31 rows, none of them about + * PACS (BenchlingAgent, DNAnexusAgent, OMEROAgent…). The field had been removed + * and the same string kept matching through a different field, so a test + * asserting "the license is not searched" passed while the defect survived. + * + * Run against the bundled snapshot, not a fixture: a fixture without the licence + * label cannot fail, which is precisely how this got through. + */ +describe('a licence republished as a label is not searchable through it', () => { + const { extensions, skills } = FALLBACK_REGISTRY; + + /** A word of `license`, as a whole label — spelled out so this cannot be satisfied by the fix's own mistake. */ + const isALicenseWord = (license: string | undefined, label: string) => + (license ?? '') + .split(/[^0-9A-Za-z]+/) + .filter(Boolean) + .some((word) => word.toLowerCase() === label.toLowerCase()); + + it('still carries the overlap this pins, or proves nothing', () => { + const tagged = (entries: readonly { tags: string[]; license?: string }[]) => + entries.filter((entry) => + entry.tags.some((tag) => tag.toLowerCase() === (entry.license ?? '').toLowerCase()) + ).length; + expect(tagged(extensions), 'extensions tagged with their own licence').toBeGreaterThan(1); + expect(tagged(skills), 'skills tagged with their own licence').toBeGreaterThan(1); + expect( + skills.filter((skill) => skill.keywords.some((k) => isALicenseWord(skill.license, k))).length, + 'skills with a licence word among their keywords' + ).toBeGreaterThan(1); + }); + + it('finds nothing for the licence, in either catalog', () => { + // `apache` occurs nowhere in the snapshot except each entry's own licence. + // Measured before the fix: 31 extensions and 49 skills. + expect(ids(rankExtensions(extensions, 'apache'))).toEqual([]); + expect(ids(rankSkills(skills, 'apache'))).toEqual([]); + }); + + it('keeps the plural fallback and the substring rule, and loses only the licence', () => { + const pacsSkills = ids(rankSkills(skills, 'PACS')); + // Real hits: a skill whose keywords say `pacs`, and `pac` inside `PacBio`. + expect(pacsSkills).toContain('biomedical-imaging-pathology'); + expect(pacsSkills).toContain('long-read-sequencing'); + // Licence-only, measured among the 51 before the fix. + expect(pacsSkills).not.toContain('empirical-research-router'); + expect(pacsSkills).not.toContain('causal-identification-gates'); + + const pacsExtensions = ids(rankExtensions(extensions, 'PACS')); + for (const licenceOnly of ['benchlingagent', 'dnanexusagent', 'omeroagent']) { + expect(pacsExtensions, `${licenceOnly} is not about PACS`).not.toContain(licenceOnly); + } + }); + + /** + * The fix does exactly one thing: it reads an entry as if the licence label + * were not there. Asserted against a copy of the snapshot with those labels + * removed from the DATA, so an over-broad rule — dropping every tag, or every + * label containing a licence word — fails here even though it would satisfy the + * assertions above. The queries are the legitimate ones the port's differential + * harness measured, plus the licence ones. + */ + it('changes nothing else about any query', () => { + const withoutLicenceLabels = (labels: string[], license: string | undefined) => + labels.filter((label) => !isALicenseWord(license, label) && label !== license); + const strippedExtensions = extensions.map((entry) => ({ + ...entry, + tags: withoutLicenceLabels(entry.tags, entry.license), + })); + const strippedSkills = skills.map((entry) => ({ + ...entry, + tags: withoutLicenceLabels(entry.tags, entry.license), + keywords: withoutLicenceLabels(entry.keywords, entry.license), + })); + + let matched = 0; + for (const query of [ + 'R scripting ggplot visualization', + 'r-scripting', + 'SPOKE knowledge graph', + 'ggplot', + 'heatmap', + 'python', + 'PACS', + 'pac', + 'apache', + 'Apache-2.0', + 'zzzznope', + '', + ]) { + const skillHits = ids(rankSkills(skills, query)); + const extensionHits = ids(rankExtensions(extensions, query)); + expect(skillHits, `skills, ${query || '(empty)'}`).toEqual( + ids(rankSkills(strippedSkills, query)) + ); + expect(extensionHits, `extensions, ${query || '(empty)'}`).toEqual( + ids(rankExtensions(strippedExtensions, query)) + ); + matched += skillHits.length + extensionHits.length; + } + // The browse query alone contributes 166, so a run that read no entry at all + // cannot pass this by matching empty against empty. + expect(matched).toBeGreaterThan(166); + }); +}); diff --git a/ui/desktop/src/components/baam/search.ts b/ui/desktop/src/components/baam/search.ts index 834c6530f..e4b6fd762 100644 --- a/ui/desktop/src/components/baam/search.ts +++ b/ui/desktop/src/components/baam/search.ts @@ -168,6 +168,48 @@ function words(text: string): string[] { .map((word) => word.toLowerCase()); } +/** + * Does `label` say nothing that `license` does not — is every word of it a word + * of the licence? A catalog whose entries carry a licence drops such a label when + * it assembles an entry's searchable text (see `labelFields` in `registry.ts`). + * + * The licence itself is not a searchable field: every entry in the BAAM registry + * is Apache-2.0, so it separates nothing, and both catalog searches leave the + * field out for that reason. + * + * ⚠ **Leaving the FIELD out was not enough.** A registry republishes the licence + * as one of the entry's own tag chips — and, for a skill, again among its + * keywords — and labels are searched, rightly: `MCP`, `ELN` and `Imaging` are + * exactly what a tag is for. Measured in the Browse-extensions modal on + * 2026-09-12 against the live 37-entry registry, with the field already gone: + * `PACS` → 31 of 37, `pac` → 31, `apache` → 31, and not one of the 31 about PACS. + * The three counts agreeing is the identification — `PACS` reaches `pac` through + * the plural fallback in {@link termStrength}, `pac` is inside `apache`, and 31 + * rows wear an `Apache-2.0` chip. Removing the field had moved the defect one + * field over, where a test asserting "the licence is not searched" still passed. + * + * Compared by WORDS rather than by equality, because the second spelling is not + * the first: the tag is `Apache-2.0` and the keyword is `apache`. An equality + * test drops the tag and keeps the keyword, which is the same half-fix again. + * + * What this deliberately does not do: drop every label (`MCP`, `Imaging`, `ELN`, + * `Registry` are real search value), or name a licence in the matcher (`Apache`, + * `MIT` — the next licence reopens the hole). The cost of the word test is a + * licence id built from a topical word — `Python-2.0`, `Ruby`, `PostgreSQL` — on + * an entry that also tags itself with that word; the tag is then dropped for + * saying only what the licence says. No entry in the registry is such a case + * (measured over all 166: the rule drops the 129 licence labels and nothing + * else), and an equality test pays a smaller version of the same cost. + */ +export function namesOnlyTheLicense(label: string, license: string | undefined): boolean { + const labelWords = words(label); + // An empty label says nothing at all, which is not the same as saying only the + // licence: leave it, so the rule stays about the licence. + if (labelWords.length === 0) return false; + const licenseWords = words(license ?? ''); + return labelWords.every((word) => licenseWords.includes(word)); +} + /** Length in characters rather than UTF-16 code units, like Rust's `chars().count()`. */ function charCount(text: string): number { return Array.from(text).length;