Repository navigation
fix(bestax-mcp): rank the component a search is about above passing mentions - #970
Conversation
…entions
search_bestax("date picker"), the example in the tool's own schema, put DateInput 33rd.
Every word of a multi-word query scored the same in a name as in prose, so a prop that
mentions a date picker in its description outranked the component whose name says
"date", and a one-word "datepicker" matched nothing at all. A prop named like a
component (DateInput.popover) tied with Popover itself and won on alphabetical order.
In a query of several words, a word in the name now counts for more than the same word
in the prose. A component the query is about, one it names or whose summary has every
word, ranks above props and CSS variables that match equally. A small table of what
other libraries call a component (date picker, time picker, combobox, toggle, drawer,
and the like) finds it as if it had been named, however the phrase is spaced, hyphenated
or cased, and get_component suggests from the same table, so DatePicker offers DateInput.
Examples that share a heading are listed once rather than once per example.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 10 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Preview DeploymentPreview URL: https://9f11a028.bestax.pages.dev |
|
deep-review: fresh The previous run was cancelled at the runner limit before it posted anything, so this asks for the same full review again. The change is the ranking and alias logic in |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Deep review — 1 blocking · 2 advisory
| # | Severity | Area | Finding | Location |
|---|---|---|---|---|
| 1 | 🟡 Minor | Correctness | Multi-word name bonus is un-gated for props/examples while COMPONENT_BONUS needs full prose coverage, so a partly-matching natural-language query now ranks a prop or example above the right component (Block, Reveal regress vs main) |
bestax-mcp/src/search.ts:242 |
| 2 | 🔵 Advisory | Correctness | An alias word inside a longer query (110–128) outweighs the component that query names outright (~80): breadcrumbs with separators → Divider |
bestax-mcp/src/search.ts:163 |
| 3 | 🔵 Advisory | Correctness | t.length >= 3 && lowerName.includes(t) is boundary-free, so tab scores on Table exactly as on Tabs and tab panel returns Table before Tabs |
bestax-mcp/src/search.ts:101 |
Overall: Sound, and a large measured improvement — I reimplemented the origin/main scorer alongside HEAD and ran both over the committed index. Every claim in #934 reproduces on main (date picker → Popover.closeOnEscape first, DateInput at 33; datepicker → nothing; the four *.popover props tied with Popover at 130 and won alphabetically) and every one is fixed at HEAD. A sweep deriving a 3-word query from each of the 90 catalog purposes found zero cases where a prop, example, CSS variable or skill outranks the component the query describes, which is the central claim held empirically. The riskiest part is the one place the new scoring is asymmetric between hit kinds (finding 1): the name bonus in score() applies to props and example titles, including function words like that and into, while a component is denied COMPONENT_BONUS unless its summary covers every term — and the 26-row pinned table is all short keyword queries, so nothing covers that shape. Start there; findings 2 and 3 are ranking trade-offs worth knowing but not worth holding the merge for.
Residual risk:
- Prose-only queries — open, and posted as finding 1.
vertical spacing between elementsandelement that animates into viewboth move the right component out of #1 relative tomain. A majority-coverage gate fixes both and keeps the pinned table, thecloseOnEscapeexact-prop case and thepopovercomponent-above-prop case green (verified by running all three against a patched scorer). - Alias dominance in compound queries — open, posted as finding 2; bounded to component-vs-component ordering, never component-vs-prop.
- Alias table drift — refuted. The
names only components that existtest holds every target inALIASESto the catalog, so a rename fails the suite rather than silently pointing nowhere.Object.hasOwnguards the lookup, so a query word likeconstructorortoStringcannot reach a prototype key. - Dedup dropping distinct results — refuted. The key is
kind+ NUL +name;part.pathis unique across all 90 component files (checked — every path occurs exactly once), so no two prop hits can collide, andcssVarIndex/ component / skill names are unique by construction. For examples the collision is real and intended (DateInput has threeMonth and Year Pickersbodies and threeLauncher Iconbodies) and nothing is lost: the surviving row namesget_examples({ component, query: title }), which filters ontitle.includes(q)and returns all of them. - Performance — refuted.
aliasMatchesruns once per search rather than once per component, andphrasesis O(4·words). A full-length 200-character query (theMAX_QUERYceiling) over all five kinds completes in ~35 ms anddate pickerin ~7 ms, nowhere near the 5.8 s the file cache note is about.STEM_RE_CACHEis untouched. - Coverage and gates — refuted.
pnpm --filter bestax-mcp test:coverageis green at 288 tests, withsearch.tsat 98.56% statements / 92.85% branches against the 95/78 floor;tsc --noEmitandprettier --checkboth clean. Nothing underbestax-mcp/data/changed, sogen:mcp:checkis unaffected, anddocs/docs/guides/llms/index.mddescribessearch_bestaxonly as "components, props, examples, CSS variables and skills in one ranked list", which stays true. - PR body — accurate. Spot-checked the two non-obvious claims:
cssVarIndex["--bulma-dateinput-cell-color"]really isDateTimeInput(the attribution problem the body defers), and theCollapsespurpose really does say "one item open at a time", so thetime pickerexample is the right one.
🏄 Pretty clean set, this one — paddled straight out past the
--bulma-dateinput-*soup that used to swamp "date picker" and came back with DateInput on the nose. One little shorebreak where the long chatty queries lose the component to a stray prop, so patch that and it is all glass from here.
…nce-shaped search A query written as a sentence lost its component again. A component got its bonus only when its summary had every word, so Block, which says "margin" where the query said "spacing", lost "vertical spacing between elements" to a prop named spacing, and an example titled with the word "that" beat Reveal. A summary with most of the query's words now earns the bonus, and words like "that", "with" and "into" are no longer terms at all. An alias also outweighed a component the query named outright, so "breadcrumbs with separators" answered with Divider. Where a query names another component outright, an alias is now worth what that name is, and a plural counts as a name.
|
deep-review: verify Every open thread has a reply: fixed, with the commit, or refuted with a reason. This settles them on 2e2a1ef. |
Preview DeploymentPreview URL: https://cc96de89.bestax.pages.dev |
There was a problem hiding this comment.
Deep review (verify) — 3 resolved · 0 open
| # | Severity | Area | Thread | Outcome | Location |
|---|---|---|---|---|---|
| 1 | 🟡 Minor | Correctness | Name bonus un-gated for props/examples while COMPONENT_BONUS was all-or-nothing on prose coverage |
✅ Verified fixed | bestax-mcp/src/search.ts:311 |
| 2 | 🔵 Advisory | Correctness | An alias buried in a longer query outweighed the component that query named outright | ✅ Verified fixed | bestax-mcp/src/search.ts:260 |
| 3 | 🔵 Advisory | Correctness | A three-letter term scores anywhere inside a name (tab in Table) |
✅ Reason accepted | bestax-mcp/src/search.ts:149 |
What I checked
- Thread 1. The
aboutgate is now a strict majority (covered * 2 > terms.length) with a name/alias path, andthat/with/intoare inSTOP_WORDS, so a function word never becomes a term. Both queries I reported are pinned atsearch.test.ts:138-139. The load-bearing detail: that table block loads the full component corpus in its ownbeforeAll(catalog.components.map(loadComponent), line 101), not the four records the outer suite loads — soprop:Avatars.spacingand theClientOnlyexample really are in the haystack those rows score against. Both now return the component at #1. - Thread 2.
aliasMatchestakes the catalog names and drops the alias toNAME_WORD_SCOREwhen the query names a component the alias does not reach. Tracedbreadcrumbs with separatorsby hand:withis a stop word,breadcrumbsreachesBreadcrumbthrough the new plural-awareisName, andseparatorreaches onlyDivider— so Divider takes 30 rather than 118 and Breadcrumb is #1.star rating(line 124) still passes, which is what holds the empty-outrightcase whereeveryis vacuously true and the fullALIAS_SCOREsurvives. - Thread 3. Refutation from the PR author (
allxsmith, live role admin). It holds on its own terms: names are lower-cased before matching, which erases the word boundaries a boundary-anchored rule needs, so such a rule could not match the term input against dateinput for exactly the reason this file already documents for them against anthem — anddate inputhas to reach DateInput. Table versus Tabs is the shared-prefix collision my own follow-up comment had already withdrawn the fix for. - Tests.
pnpm --filter bestax-mcp testgreen at 295/295 (7 suites);search.test.tsalone at 60/60. The two guards that bound the widened bonus —closeOnEscapeanswering with a prop, andpopoverputting the component aboveDateInput.popover— both still pass, so the looseraboutgate is not lifting components over props that genuinely win.
Overall: This pass settled threads and reviewed no commits. All three of my open threads are resolved: two verified fixed against the current code and the real committed index, one accepted on a reasoned deliberate-choice refutation from the author. Nothing is left open, and the two fixes are pinned by assertions that run over the full corpus rather than a reduced fixture, which is what makes them durable.
Residual risk: Not re-assessed — a verify pass settles what the prior review left open and raises nothing new. A reader who wants the current ranking logic reviewed afresh should ask for a fresh pass.
🏄 Three threads paddled out, three came back in clean — two real fixes pinned against the whole catalog, and one gnarly tab-slash-Table prefix thing nobody can surf around, which the author called straight. All mellow, good to go.
github-actions
Bot
added
the
review-converged
allxsmith
added
the
needs-human-review
📸 Story screenshots at handoff —
|
|
🎉 This PR is included in version 5.27.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 4.2.14 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 2.25.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.14.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
1 participant
search_bestax({ query: "date picker" }), the example in the tool's own schema, ranked DateInput below a prop that mentions a date picker in passing and below every--bulma-dateinput-*variable. Every word of a multi-word query scored the same in a name as in prose. A one-word "datepicker" matched nothing. A prop named like a component (DateInput.popover) tied with Popover itself and won on alphabetical order. Andget_component({ name: "DatePicker" })offered no suggestion, since edit distance cannot see that DatePicker means DateInput.In a multi-word query, a word in a name now counts for more than the same word in prose. A component the query is about, one it names or whose summary has every word, ranks above props and CSS variables that match equally. A summary that shares one word gets no lift, so "at a time" does not pull Collapses into "time picker". A small table of what other libraries call a component (date picker, time picker, combobox, toggle, drawer and the like) finds it as if it had been named, however the phrase is spaced, hyphenated or cased. Unknown names are suggested from the same table first. A test holds every name in the table to the catalog. Examples that share a heading are listed once.
The tests rank a table of realistic queries, including the issue's, and check that an exact prop name still answers with that prop.
Left out: the CSS-variable index attributes a variable to the last component that declares it, so
--bulma-dateinput-*reads "declared by DateTimeInput". That is a separate attribution problem in the generator.pnpm allpasses locally.Fixes #934