fix(skills): rank an installed-skill search by the words of the query, not all of them (F5) by Broccolito · Pull Request #266 · BaranziniLab/biorouter

Broccolito · 2026-09-12T02:22:35Z

QA finding F5 — a multi-word query returns nothing — measured in the 2026-09-10
composer run, applied to the installed skill catalog. PR #255 fixes the desktop BAAM
Browse modals; this fixes skills__searchSkills, the tool a model calls to find a skill
the user already has. Zero file overlap between the two, so they land independently.

searchSkills kept a skill only when its name, description and bundle held every
word of the query as a substring. A model composing R scripting ggplot visualization on
a user's behalf got total: 0 with a ggplot skill and an R-scripting skill installed, and
told the user nothing was installed for the job.

Before / after, measured

The same searchSkills tool call, run against six installed fixture skills (ggplot,
python-scripting, r-scripting, rna-qc, variant-calling, spoke-knowledge-graph)
written as real SKILL.md files and read back by the real scanner. Before is the
pre-branch parent d6b39693 with the same harness checked out onto it — not a
reconstruction:

query	before	before order	after	after order
`R scripting ggplot visualization`	0	—	3	ggplot, r-scripting, python-scripting
`r-scripting`	2	r-scripting, python-scripting	3	r-scripting, python-scripting, ggplot
`SPOKE knowledge graph`	1	spoke-knowledge-graph	1	spoke-knowledge-graph
`R`	6	python-scripting, r-scripting, rna-qc, spoke-knowledge-graph, variant-calling, ggplot	2	r-scripting, ggplot

Two things to read out of the last row. Before, a one-letter R returned every fixture —
rna-qc and variant-calling have no R in them, only the letter — and it ranked
python-scripting first, because both names contain the letter and the tie broke
alphabetically. After, R is the two skills that are actually about R, name first,
description second. SPOKE knowledge graph is the control: a query whose words all sit in
one skill was never broken and is unchanged.

Fail-before evidence

Each behaviour the branch claims is pinned by a test that fails on the parent:

an_installed_skill_search_ranks_every_skill_matching_a_word_of_the_phrase asserts
total == 3; measured 0 at d6b39693 (table above).
a_one_letter_installed_skill_query_matches_whole_words_only asserts
["r-scripting", "ggplot"]; measured 6 rows in the wrong order at d6b39693.
a_one_letter_query_matches_whole_words_only and
a_phrase_found_only_inside_other_words_is_not_the_query_as_written pin the matcher's own
regression, which appeared the moment the installed search moved onto it: the whole-query
check was a plain substring test, so R came back through every word containing the
letter, three rows with matchedTerms: [].
test_search_skills_matches_name_description_and_bundle changes on purpose:
bio-bundle rna returned 1, now returns all 5 in the bundle with rna-qc first as the
only one matching all three words. That assertion was the AND.

What the move changed for the marketplace path

e26eb59b moves marketplace/search.rs to catalog_search.rs so a second caller can use
it. Proof it is behaviour-neutral — normalize the three renames
(MarketplaceSearch→CatalogSearch, MarketplaceSearchHit→CatalogSearchHit,
pub(super)→pub(crate)) and diff the file across the move:

$ sed -e 's/MarketplaceSearchHit/CatalogSearchHit/g' -e 's/MarketplaceSearch/CatalogSearch/g' \
      -e 's/pub(super)/pub(crate)/g' before.rs | diff -u - after.rs

The entire output is the module doc comment, plus terms() narrowing from pub(super) to
private. Not one executable line differs. marketplace.rs changes only its imports, two
return types and a doc path; both fields closures are untouched. Nothing outside the
crate ever referenced MarketplaceSearch/MarketplaceSearchHit (git grep at the parent:
no hits outside the module), so the rename breaks no consumer.

The marketplace's behaviour does change, in 4de29d1a, and deliberately: the whole-query
rank check goes from text.contains(phrase) to written_in(text, phrase) — the query
counts only where it starts and ends at a word boundary, the same edges the short-term rule
already enforced. Quantified against the live landing/registry.json (129 skills + 37
extensions), 38 queries × both catalogs = 76 searches, full ranking both ways:

0 entries start being returned; 198 stop.
7 of 76 searches change at all — and in every one of the 7, every term is under
three characters: R, dy, s p, ml, AI. The other 31 queries — ggplot,
heatmap, complexheatmap, python, rna, single cell, knowledge graph,
differential expression, spatial transcriptomics, c++, ucsf … — return the
identical list in the identical order.
Nothing legitimate is demoted. Long terms still match inside words (heatmap in
complexheatmap) through term matching, which is untouched. In the 7 that move, the top
is preserved or improved: skills R 125→8 with an unchanged top-5; extensions R
37→1 (codegraphagent, which was already UCSF (and maybe external) token usage mismatch #1); ml 9→3, top-3 unchanged; AI 24→2,
top-2 unchanged; dy 6→0 and s p 6→1. Only two searches change their first result,
both for s p.

a_verbatim_occurrence_is_still_a_hit is replaced rather than deleted. It asserted two
things: that rank("s p") is empty (kept verbatim in the replacement) and that rank("dy")
finds r-scripting inside "Tidyverse" with matchedTerms: [] (inverted). The second was
the leak, asserted as a feature — it is the same substring the three-character rule exists
to refuse. No real assertion was lost.

Verification

cargo test -p biorouter --lib -- catalog_search marketplace   # 32 passed, 0 failed
cargo test -p biorouter --lib -- skill                        # 172 passed, 0 failed
cargo test -p biorouter-server --lib -- routes::skills        #   4 passed, 0 failed
cargo test -p biorouter --test privacy_capability --test privacy_guard_wiring  # 4 + 3 passed
cargo fmt --check                                             # clean

Merged origin/main (6 commits, no conflicts — they touch bridge.rs and
tests/privacy_capability.rs only).

Deliberately out of scope

Settings → Skills filter (ui/desktop/src/components/skills/SkillsView.tsx, lines
74–85) and the composer skill picker
(ui/desktop/src/components/bottom_menu/BottomMenuSkillSelection.tsx, lines 91–98) both
still ask name/description/bundle.toLowerCase().includes(wholeQuery) — the same defect,
a third and fourth time. They are renderer TypeScript with no shared code with this
Rust matcher, so fixing them here would mean writing a third implementation of the rule.
The right move is to point both at the TypeScript port PR Desktop: the Browse modals match a search word by word, ranked like the model's search (F5) #255 adds
(ui/desktop/src/components/baam/search.ts) once it lands; filed as a follow-up rather
than grown into this PR, which touches no renderer file.
PR Desktop: the Browse modals match a search word by word, ranked like the model's search (F5) #255's port will be stale in two ways after this merges, and it is Desktop: the Browse modals match a search word by word, ranked like the model's search (F5) #255's file to
change, not this branch's: its header cites
crates/biorouter/src/marketplace/search.rs (now catalog_search.rs) and says "A change
to a rule below is a change to both files"; and search.ts:231 implements the
whole-query check as text.toLowerCase().includes(query.phrase) — the substring rule
4de29d1a replaces. Porting written_in is a few lines and belongs on that branch, so
its Rust↔TS parity harness keeps passing.

🤖 Generated with Claude Code

The tokenising, ranking matcher written for the two marketplace searches is about to serve the installed-skill search as well, which has nothing to do with the marketplace. It moves from `marketplace/search.rs` to a neutral `catalog_search.rs`; its result types drop the `Marketplace` prefix (`CatalogSearch`, `CatalogSearchHit`), and `rank`, `Weight` and the noise lists become crate-visible so a second module can call them. No behaviour changes. The matcher's own seven tests and every marketplace test pass unchanged.

`skills__searchSkills` kept an installed skill only when its name, description and bundle held EVERY word of the query as a substring, so the phrase a model composes on a user's behalf found nothing unless one skill happened to say all of it. Measured with a ggplot skill ("Publication-quality ggplot2 visualization guide for R...") and an r-scripting skill installed: searchSkills {query: "R scripting ggplot visualization"} -> {"total": 0, ..., "skills": []} That is QA finding F5 again, on the installed catalog instead of the marketplace and through different code. It is fixed the same way, through `catalog_search::rank` rather than a second matcher: an any-term union, ranked by terms matched and then by where (name > bundle > description), with short terms matching whole words only and filler dropped. The same query now returns ggplot (3 terms), r-scripting (2) and python-scripting (1), and never rna-qc or variant-calling, whose text is full of the letter r but never says R. The page keeps its shape (total/offset/limit/returned/next_offset/skills and every provenance and removal field) and adds what the marketplace page carries: `terms`, a per-row `matchedTerms`, and, on zero hits, a `guidance` sentence counting the skills this conversation has enabled. It names only searchSkills, because an app agent holds searchSkills and loadSkill alone and this handler cannot see the roster. Unchanged, and now pinned on the search path too: the conversation's switches run before the ranking (a switched-off skill is neither returned nor counted), `removable` and `removalTarget` come through a ranked row, pagination walks the ranking, and an empty or wordless query is still the listing, carrying none of the new fields. One existing assertion encoded the AND and changes on purpose: `bio-bundle rna` returned 1 skill; it now returns all 5 in the bundle, with `rna-qc` first as the only one matching all three words.

The matcher ranks an entry that holds the whole query first, and it tested that with a plain substring check. That let a query which is itself one short term back in through every word containing it, undoing the rule that a term under three characters matches whole words only. Measured once the installed-skill search moved onto the matcher: searchSkills {query: "R"} -> all 5 fixture skills; python-scripting, rna-qc and variant-calling came back with `matchedTerms: []`, found by the letter r alone In the matcher's own fixtures `rank("R")` returned complex-plots (through "Draws") and prose-only as well. The same check ranked noise first inside a phrase: for `R scripting`, an entry saying "snippets for scripting" outranked one that said both words, because "fo[r scripting]" contains the query. The whole query now counts as written only where it starts and ends at a word boundary. Term matching is untouched, so every entry a word of the query matches is still returned; what goes is an entry found by nothing but a fragment of a word, and the rank such a fragment bought. The two queries above now return r-scripting then ggplot, and put the entry matching both words first. Every marketplace and extension search test passes unchanged. `a_verbatim_occurrence_is_still_a_hit` asserted the leak as a feature (`dy` found r-scripting inside "Tidyverse"). It is replaced by a test that the query as written still ranks first, but only as whole words.

The skill-catalog reference described what a searchSkills row carries but not how a query picks the rows. It now says: ranked by any word through the shared catalog_search matcher, short words whole-word only, the conversation's switches first, and the terms/matchedTerms/guidance fields a ranked page adds, with the F5 measurement that motivated it.

…d-66b908

PR #266 moves crates/biorouter/src/marketplace/search.rs to crates/biorouter/src/catalog_search.rs so the installed-skill search can be its second caller, renaming MarketplaceSearch/MarketplaceSearchHit to CatalogSearch/CatalogSearchHit. This file's header cited the old path, and the sentence directly under it — "A change to a rule below is a change to both files" — is the whole reason the port is maintainable. A header that names a path which no longer exists retires that sentence silently, which is the one outcome it exists to prevent. So: the path, the two type names, and the reason the file moved. The sentence itself is kept word for word, and the rule change it obliged is the commit after this one.

PR #266 changed the whole-query rank rule in the Rust matcher this file ports: a query counts as written only where it starts and ends at a word boundary (`written_in`), not wherever it occurs as a substring. The old test that asserted the substring behaviour, `a_verbatim_occurrence_is_ still_a_hit`, had pinned the leak as a feature. `search.ts` still asked `text.includes(query.phrase)`, so the Browse modals and the model's own search were about to disagree on exactly the queries #266 changed. Measured over the live landing/registry.json (129 skills, 37 extensions), 38 queries x both catalogs = 76 searches, full ranking both ways, the same file bundled before and after: 0 entries start being returned, 198 stop, 7 of 76 searches change skills `R` 125 -> 8 all 8 in the same order skills `dy` 6 -> 0 skills `s p` 6 -> 1 skills `ml` 9 -> 3 top 3 unchanged skills `AI` 24 -> 2 top 2 unchanged extensions `R` 37 -> 1 codegraphagent, already first extensions `s p` 7 -> 1 Every one of the 7 is a query whose every term is under three characters; the 33 realistic queries — ggplot, complexheatmap, knowledge graph, `R scripting ggplot visualization`, r-scripting, c++ ... — return the identical list in the identical order in both catalogs. That reproduces #266's own numbers from the TypeScript side, drop for drop. A one-letter `R` used to return 125 of 129 skills with 117 of them matching no term at all, ranking empirical-paper-submission-rr above r-scripting. `writtenIn` is a transliteration of `written_in`, not a regular expression, and the reason is measured: a `\b`-built test disagrees with Rust on 4 of the 11 cases now asserted. JavaScript defines `\b` over `[A-Za-z0-9_]` alone, so it reads `_` as a letter where Rust does not (`case` IS written in "snake_case") and every non-ASCII letter as a boundary where Rust does not (`naï` is NOT written in "naïve"); and `++` in "c++" needs no boundary at all, since an edge that is not a letter or digit is one. The scan compares code points, like Rust's `char_indices`: read as UTF-16 units the trailing half of an astral letter is a lone surrogate that matches no letter class, so `rna` would be read as written in "\u{1D41A}rna". `WORD_CHAR` is spelled as the explicit complement of the `WORD_BREAK` that `words` already splits on, so a term and the query around it can never be held to different edges. Three existing tests change, the same three #266 changed in Rust, and no modal test moves: - the `dy` half of `still finds a verbatim occurrence` is inverted, and the test becomes three: a one-letter query as a whole word, a phrase found only inside longer words, and `dy`/`s p` both empty. - `ranks a verbatim match above every term match, even one matching more terms` loses its premise. A phrase written in a field has every one of its own words in that field as a whole word, so a written entry now always matches EVERY term; the tier is re-pitted against placement instead, prose against two names, and `catalog_search.rs`'s own `tidy code` -> [styler, code-tidy] ranking is asserted beside it. - `R` alone over the fixture goes from 6 hits, 3 of them matching no term, to the 3 skills that are about R. `writtenIn` is exported so its boundary cases can be asserted directly, the way Rust asserts them from inside the module: its six `written_in` cases port over verbatim, plus the four characters `\b` gets wrong. Reverting the body to `text.includes(phrase)` fails 6 of the 43 tests, both boundary tests among them.

…d-66b908

…re two counts Follow-up to merging 133 commits of main into this branch. Nothing here changes behaviour a user can see; each item is a claim in the six commits that the merge made false. **The canonical matcher moved, so the port's three citations were dead.** PR #266 renamed `crates/biorouter/src/marketplace/search.rs` to `crates/biorouter/src/catalog_search.rs`. `landing/marketplace-search.js`, `landing/scripts/baam-search.test.mjs` and `landing/baam.html` each named the old path as the module to read for the reasoning -- the one pointer a future reader of a port most needs. All three now name the new path and say when it moved, the way `ui/desktop/src/components/baam/search.ts` already does on main. **And PR #266 did not only move it -- it changed a rule the port mirrors.** The verbatim-phrase bonus became `written_in`: present as a substring AND not buried inside a longer word at either end, with the boundary imposed only by an end that is itself a word character. The port still asked `indexOf(...) !== -1`, so it had drifted from the module it claims to mirror rule for rule. Ported, with that module's own six assertions run against it. Worth being exact about the blast radius, in both directions: the shelves call `matching`, which collapses `rank` to a membership set, so **nothing on the page moves** -- a visitor sees the same cards in the same order either way. That is also why the drift could have sat there indefinitely, and why the rule is now pinned by a test rather than left to the next reader to notice. **`declared_model_names`' own doc contradicted the comment 100 lines below it.** The route's `Ok(None)` arm still said "six of those nine ship a curated `with_models(...)` catalog" -- the figure produced by the instrument the function's doc-comment exists to warn against, which undercounts by three because `ProviderMetadata::new` also takes a `model_names` list. In a commit whose subject is copy that does not contradict itself, that is the wrong comment to leave standing. It now reads nine, measured off `known_models`, and points at the function that explains the measurement. **The subagent count moved 197 -> 198 while this branch waited.** The whole point of that CLAUDE.md line is that a wrong figure is worse than none, because this repo asserts "pre + N" against it. Re-measured on the merged tree: BIOROUTER_DISABLE_KEYRING=true cargo test -p biorouter --lib -- subagent test result: ok. 198 passed; 0 failed; 3794 filtered out so the line carries 198 and today's date, and now also records that it moved by one in a single week -- which is the argument for re-measuring rather than for trusting any figure written here, this one included. Verified: node --test landing/scripts/baam-search.test.mjs 17/17 (the browser half drives the real page, so the baam.html edit is covered); baam-privacy-facet.test.mjs 14/14; build-registry.mjs --check reports all three outputs current, and no registry datum is touched.

Broccolito added 5 commits September 11, 2026 14:32

Merge remote-tracking branch 'origin/main' into claude/relaxed-volhar…

7e17d2e

…d-66b908

Broccolito mentioned this pull request Sep 12, 2026

Desktop: the Browse modals match a search word by word, ranked like the model's search (F5) #255

Merged

Merge remote-tracking branch 'origin/main' into claude/relaxed-volhar…

6a8f42b

…d-66b908

Broccolito merged commit c2d7556 into main Sep 12, 2026
16 checks passed

Broccolito deleted the claude/relaxed-volhard-66b908 branch September 12, 2026 06:47

Navigation Menu

Uh oh!

fix(skills): rank an installed-skill search by the words of the query, not all of them (F5) - #266

fix(skills): rank an installed-skill search by the words of the query, not all of them (F5)#266
Broccolito merged 6 commits into
mainfrom
claude/relaxed-volhard-66b908

Broccolito commented Sep 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

Broccolito commented Sep 12, 2026

Before / after, measured

Fail-before evidence

What the move changed for the marketplace path

Verification

Deliberately out of scope

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant