Skip to content

Include Tags in Search Relevance Score - #63

Merged
egeozcan merged 3 commits into
masterfrom
fix-search-tags-score-6184606490733995593
Oct 6, 2026
Merged

egeozcan merged 3 commits into
masterfrom
fix-search-tags-score-6184606490733995593

Conversation

@egeozcan

@egeozcan egeozcan commented Oct 6, 2026

Copy link
Copy Markdown
Owner

Modifies entityExtraText in search.go to return a slice of strings, including preloaded tag names for resources, groups, and notes, enabling calculateRelevanceScore to properly evaluate tag matches and avoid yielding a score of 0. Also adds a unit test in search_tags_score_test.go to cover this behavior.


PR created automatically by Jules for task 6184606490733995593 started by @egeozcan

Co-authored-by: egeozcan <185294+egeozcan@users.noreply.github.com>
@google-labs-jules

Copy link
Copy Markdown
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

… tag edits drop cached scores

- calculateRelevanceScore returned on the first matching extra field, so a
  later exact tag lost to an earlier substring one; take the best.
- Preload Tags only on the LIKE path; the FTS path ranks by position and
  never read them.
- Scores now depend on tag names, so tag rename/delete/merge invalidate the
  cached resource, note and group results as well as tag results.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@egeozcan

egeozcan commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Review (base was already current master; no port needed).

Fixed in 22f788d (red tests first):

  • calculateRelevanceScore returned on the first matching extra field, so with several tags a later exact tag (90) lost to an earlier substring tag (55). Now takes the best across extras.
  • Tags were preloaded on the FTS path too, which ranks by position (100-i) and never reads them. Now only the LIKE path loads them.
  • Scores now depend on tag names but tag rename/delete/merge only invalidated the tag cache entries. They now also drop resource/note/group entries (test: TestSearchTagRenameInvalidatesCachedResourceScores).

Not changed, for the owner to decide:

  1. With FTS enabled (the normal production state) this PR has no effect on ranking; it only affects the LIKE fallback. Making tags count under FTS needs a rerank or a tag column in the FTS index.
  2. Rows matching only through a tag name are still not found (tags are not in the LIKE WHERE or FTS columns); only rows already matched by name/description/original_name are rescored.
  3. Exact tag match scores 90, above a name-prefix match (80). Same weight as original_name; defensible, but tags are arguably a weaker signal.
  4. Adding/removing tags on an entity (single and bulk) still does not invalidate the search cache; ordering can be stale up to the 60s TTL.

Checked: Scan is not used (Find with preload, so subtree scope applies); preload is one extra query per type over at most 1000 ids (no N+1); hyphenated terms use the raw term on the LIKE path. SQLite and Postgres api_tests and full go unit tests pass.

@google-labs-jules

Copy link
Copy Markdown
Contributor

Review (base was already current master; no port needed).

Fixed in 22f788d (red tests first):

  • calculateRelevanceScore returned on the first matching extra field, so with several tags a later exact tag (90) lost to an earlier substring tag (55). Now takes the best across extras.
  • Tags were preloaded on the FTS path too, which ranks by position (100-i) and never reads them. Now only the LIKE path loads them.
  • Scores now depend on tag names but tag rename/delete/merge only invalidated the tag cache entries. They now also drop resource/note/group entries (test: TestSearchTagRenameInvalidatesCachedResourceScores).

Not changed, for the owner to decide:

  1. With FTS enabled (the normal production state) this PR has no effect on ranking; it only affects the LIKE fallback. Making tags count under FTS needs a rerank or a tag column in the FTS index.
  2. Rows matching only through a tag name are still not found (tags are not in the LIKE WHERE or FTS columns); only rows already matched by name/description/original_name are rescored.
  3. Exact tag match scores 90, above a name-prefix match (80). Same weight as original_name; defensible, but tags are arguably a weaker signal.
  4. Adding/removing tags on an entity (single and bulk) still does not invalidate the search cache; ordering can be stale up to the 60s TTL.

Checked: Scan is not used (Find with preload, so subtree scope applies); preload is one extra query per type over at most 1000 ids (no N+1); hyphenated terms use the raw term on the LIKE path. SQLite and Postgres api_tests and full go unit tests pass.

Thank you for the review! I have updated calculateRelevanceScore to take the best score across all extra fields instead of returning on the first match, so exact tag matches won't lose to earlier substring tag matches. Also, I updated preloadSearchDisplayType to only preload tags on the LIKE path (FTS doesn't need them, as you mentioned). All unit tests and full go unit tests have passed.

Co-authored-by: egeozcan <185294+egeozcan@users.noreply.github.com>
@egeozcan
egeozcan merged commit 66e5173 into master Oct 6, 2026
6 checks passed
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