Skip to content

fix: stop nameMatchBonus awarding a spurious +10 on a whitespace-only query#984

Open
JOhnsonKC201 wants to merge 1 commit into
colbymchenry:mainfrom
JOhnsonKC201:fix/name-match-bonus-empty-query
Open

fix: stop nameMatchBonus awarding a spurious +10 on a whitespace-only query#984
JOhnsonKC201 wants to merge 1 commit into
colbymchenry:mainfrom
JOhnsonKC201:fix/name-match-bonus-empty-query

Conversation

@JOhnsonKC201

Copy link
Copy Markdown

Summary

nameMatchBonus (search ranking, src/search/query-utils.ts) awards a spurious flat +10 "name match" bonus to every node when the query is empty or whitespace-only.

The bug

The function collapses the query into a single comparable token and then checks startsWith:

const queryLower = query.replace(/[\s]+/g, '').toLowerCase();
...
if (nameLower.startsWith(queryLower)) {
  const ratio = queryLower.length / nameLower.length;
  return Math.round(10 + 30 * ratio);   // → 10 when queryLower === ''
}

For an empty / whitespace-only query, queryLower is '', and in JS anyString.startsWith('') is always true. So the branch fires for every node, with ratio = 0, returning Math.round(10 + 0) = 10 — a uniform, undeserved name-match bonus for a query that matches no name at all.

Reachability

This is reachable end-to-end through the public searchNodes API:

  • searchNodes(' ')parseQuery(' ') yields text = '', while the raw query is ' ' (truthy).
  • The rescoring guard in src/db/queries.ts is if (results.length > 0 && (text || query)), so a whitespace-only query passes the guard ('' || ' ' → truthy).
  • scoringQuery = text || query = ' ' is then handed to nameMatchBonus(name, ' ') for every candidate.

scorePathRelevance already handles this case correctly (it returns 0 for a whitespace-only query); nameMatchBonus did not.

The fix

A one-line early guard, before the always-true startsWith('') check:

if (!queryLower) return 0;

Repro / tests

Added a nameMatchBonus empty/whitespace query block to __tests__/context-ranking.test.ts:

expect(nameMatchBonus('authenticate', '')).toBe(0);     // was 10
expect(nameMatchBonus('authenticate', '   ')).toBe(0);  // was 10
expect(nameMatchBonus('authenticate', 'authenticate')).toBe(80); // unchanged — guard isn't over-broad
  • Before the fix: the empty/whitespace cases return 10 (test fails).
  • After the fix: they return 0, and a real exact-match query is unaffected.

Verification

  • npx vitest run __tests__/context-ranking.test.ts __tests__/search-query-parser.test.ts39 passed.
  • Confirmed the new tests fail on main (stash the source change → expected 10 to be 0) and pass with it.
  • tsc --noEmit clean.

Diff is two files, +31/-4. No behavior change for any non-empty query.

… query

nameMatchBonus collapsed the query to a single token (queryLower) and then
hit `nameLower.startsWith(queryLower)`. For an empty/whitespace-only query,
queryLower is '' and `String.startsWith('')` is always true, so every node
got a flat +10 'name match' bonus despite nothing matching its name.

This is reachable end-to-end: searchNodes('   ') yields text='' and a
truthy raw query, so the rescoring guard `text || query` lets the
whitespace-only query through to nameMatchBonus.

Guard with an early `if (!queryLower) return 0;`. Adds regression tests
covering empty and whitespace-only queries (fail before, pass after) plus an
exact-match case to confirm real queries are unaffected.
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