Skip to content

fix(control-bar): stop annotation search matching the middle of a column name - #488

Closed
tsenoner wants to merge 3 commits into
mainfrom
fix/annotation-search-key-tokens
Closed

tsenoner wants to merge 3 commits into
mainfrom
fix/annotation-search-key-tokens

Conversation

@tsenoner

@tsenoner tsenoner commented Sep 15, 2026

Copy link
Copy Markdown
Owner

Summary

Typing ted in the annotation dropdown returned the four Biocentral predictions above the one annotation the reader wanted.

Root cause

The filter matched the column name or the label, both as plain substrings. Every Biocentral column is keyed predic**ted**_*, so for this query all four matched on the key and none on the label — the matched text was never on screen:

MATCH predicted_subcellular_location  key:Y  label("Subcellular location"):n
MATCH predicted_membrane              key:Y  label("Membrane"):n
MATCH predicted_signal_peptide        key:Y  label("Signal peptide"):n
MATCH predicted_transmembrane         key:Y  label("Transmembrane"):n
MATCH ted_domains                     key:Y  label("TED domains"):Y

Fix

A query matches a substring of the displayed label, or begins at a word boundary of the column name. Query and column are both normalised on _/-.

The asymmetry is deliberate. A column name is a machine identifier built by joining words, so a mid-word hit is almost always an accident — ted inside predicted is the whole bug. A label is prose the reader is looking at, so a mid-word hit there is what they meant.

query matches via
ted ted_domains only label and first word
predicted all four Biocentral first word of each column
predicted_membrane itself full name, separator included
membrane predicted_membrane, predicted_transmembrane word; label substring
cellular predicted_subcellular_location label substring

What review changed

Two things were wrong with the first cut, both caught before merge:

  • The rule split the column but not the query. No word can start with a needle that still holds a separator, so every multi-word column stopped matching its own name — 16 of 38 in the registry. Typing the name you know went fine at predicted, empty at predicted_, and never recovered. That contradicted this change's own stated goal. Both sides are normalised now, and a test asserts every registry column matches its own full name.
  • "One rule, one implementation" wasn't true. annotation-select.test.ts kept an exported copy on the old semantics, with 8 tests asserting it — the suite was green for both rules at once, and two tests encoded results the shipped code no longer produced. The whole pipeline now lives in annotation-categories.ts, whose doc comment already read "Shared by annotation-select and query-condition-row". Both pickers call it; the mirror is gone.

Known limit, recorded in design.md

A match can still land on text a picker doesn't draw. The dropdown renders only the label, so predicted returns four rows reading "Membrane", "Signal peptide", … The query builder renders only the column name, so swiss returns reviewed. This removes the worst case — a hit on neither, which is what ted was — but making every match accountable needs the pickers to show what matched. query-value-picker already highlights matched substrings via _highlightMatch; that's the pattern to copy.

Tests

  • 6 cases in annotation-metadata.test.ts, including the reported query and a sweep asserting every registry column finds itself
  • the 8 dropdown filter tests now run against the real shipped function
  • pnpm test:ci (163 files), pnpm quality:ci — pass
  • strict OpenSpec validation, archived as 2026-09-15-fix-annotation-search-matching
  • verified in the running app against the reported column set

Note: this branch is also merged into #402, which is the one to land. Kept open for the focused diff.

🤖 Generated with Claude Code

https://claude.ai/code/session_01J26u3dxSKYGb8fRxchpJVa

tsenoner and others added 3 commits September 15, 2026 22:17
…umn name

Searching `ted` returned the four Biocentral predictions above the one
annotation the reader wanted, because every one of them is keyed
`predicted_*` and the filter matched any substring of the raw column name:

  MATCH predicted_subcellular_location  key:Y  label("Subcellular location"):n
  MATCH predicted_membrane              key:Y  label("Membrane"):n
  MATCH predicted_signal_peptide        key:Y  label("Signal peptide"):n
  MATCH predicted_transmembrane         key:Y  label("Transmembrane"):n
  MATCH ted_domains                     key:Y  label("TED domains"):Y

Every Biocentral hit was on the key and none on the label, so the matched
text was never on screen.

A query now matches a substring of the displayed label, or the start of a
word in the column name. The asymmetry is the point: a column name is a
machine identifier built by joining words, so a mid-word hit in it is
almost always an accident, while a label is prose the reader is looking at,
so `cellular` should still find Subcellular location.

The rule lives in one helper beside annotationLabel/annotationSource, and
both pickers call it. They disagreed before — query-condition-row matched
column names only and never labels at all, so searching for a label you
could see found nothing there.

Verified in the running app against the reported column set: ted →
ted_domains alone; predicted → all four; membrane → membrane and
transmembrane; cellular and loc → subcellular location.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J26u3dxSKYGb8fRxchpJVa
Run as the last commit before the merge, per AGENTS.md.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J26u3dxSKYGb8fRxchpJVa
Review caught two things wrong with the first cut.

The rule split the column but not the query, then asked whether any word
started with the needle. No word can start with a needle that still holds a
separator, so every multi-word column stopped matching its own name — 16 of
38 in the registry. Typing the name you know went fine to `predicted`, went
empty at `predicted_`, and never recovered. That directly contradicted this
change's own goal that searching by raw column name keeps working.

Both sides are normalised on `_`/`-` now, and the query has to begin where a
word does. `ted` still misses every `predicted_*`; `predicted_membrane`,
`ted_domains` and `go_bp` find themselves again. Verified across the whole
registry: no column fails to match its own name.

The other miss was worse for being self-inflicted: this change claimed "one
rule, one implementation" while annotation-select.test.ts kept its own
exported copy on the OLD semantics, with eight tests asserting it. The suite
was green for both rules at once, and two of those tests encoded results the
shipped code no longer produced. The whole pipeline — group, filter, drop
empty sections — now lives in annotation-categories.ts, which already said
"Shared by annotation-select and query-condition-row" in its own doc comment.
Both pickers call it, the mirror is deleted, and its tests exercise the real
function.

That also restores the empty-query short circuit query-condition-row lost,
and removes the double normalisation at both call sites.

Recorded in design.md, because it is not fixed here: a match can still land
on text a picker does not draw. The dropdown renders only the label, so
`predicted` returns four rows reading "Membrane", "Signal peptide", … The
query builder renders only the column name, so `swiss` returns `reviewed`.
This removes the worst case — a hit on neither, which is what `ted` was —
but making every match accountable needs the pickers to show what matched.
query-value-picker already highlights matched substrings; that is the
pattern to copy.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J26u3dxSKYGb8fRxchpJVa
@tsenoner

Copy link
Copy Markdown
Owner Author

Superseded by #402, which already contains every commit from this branch (merged in bc7f7e2d) — fix/annotation-search-key-tokens is an ancestor of feat/344-ted-link, so nothing is lost by closing this.

The annotation-search work is documented in #402 under section 4, and the change is archived as openspec/changes/archive/2026-09-15-fix-annotation-search-matching.

Closing to leave a single PR to review and land.

@tsenoner tsenoner closed this Sep 16, 2026
@tsenoner
tsenoner deleted the fix/annotation-search-key-tokens branch September 16, 2026 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant