Repository navigation
Improve operational search ranking robustness - #244
kentcdodds wants to merge 1 commit into
Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
📝 WalkthroughWalkthroughThis PR introduces token-based "operational identity" scoring to package and connector ranking in the search module. Helper functions normalize queries, extract tokens, and compute coverage across multiple fields (names, tags, exports, job names) to boost relevance scores. Tests validate the new ranking behavior for Spotify packages and named integrations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 deployed: https://kody-pr-244.kentcdodds.workers.dev Worker: Mocks:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/worker/src/mcp/tools/search.ts`:
- Around line 290-305: The ranking uses projection.exports and projection.jobs
but those are initialized empty in the registerSearchTool flow; before calling
searchUnified (and before computing
lexical/vector/scoreOperationalIdentityBoost) ensure each entry's projection is
hydrated: populate entry.projection.exports and entry.projection.jobs from the
package metadata or by invoking the existing projection-enrichment routine used
by loadSearchRowsAndRegistry, and update the registerSearchTool path to set
those fields instead of leaving them empty so searchUnified and
scoreOperationalIdentityBoost see real export/job names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fc07d110-4896-49c8-8d9e-dc30d13fee6b
📒 Files selected for processing (2)
packages/worker/src/mcp/tools/search.node.test.tspackages/worker/src/mcp/tools/search.ts
| entry.projection.exports.join(' '), | ||
| entry.projection.jobs.map((job) => job.name).join(' '), | ||
| ].join('\n') | ||
| const lexical = lexicalScore(query, doc) | ||
| const vector = cosineSimilarity( | ||
| queryEmbedding, | ||
| deterministicEmbedding(doc), | ||
| ) | ||
| const identityBoost = scoreOperationalIdentityBoost({ | ||
| query, | ||
| primaryFields: [entry.record.kodyId, entry.record.name], | ||
| secondaryFields: entry.record.tags, | ||
| tertiaryFields: [ | ||
| ...entry.projection.exports, | ||
| ...entry.projection.jobs.map((job) => job.name), | ||
| ], |
There was a problem hiding this comment.
Hydrate package projections before relying on exports/jobs in unified ranking.
Line 290, Line 291, Line 303, and Line 304 now rank packages using projection.exports and projection.jobs, but the registered search tool path initializes those fields as empty at Line 705 and Line 706. That means export/job-name ranking works only in tests or custom callers that manually provide enriched projections, not in the normal registerSearchTool flow.
Please populate these projection fields before calling searchUnified or persist them into the saved package search projection used by loadSearchRowsAndRegistry.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/worker/src/mcp/tools/search.ts` around lines 290 - 305, The ranking
uses projection.exports and projection.jobs but those are initialized empty in
the registerSearchTool flow; before calling searchUnified (and before computing
lexical/vector/scoreOperationalIdentityBoost) ensure each entry's projection is
hydrated: populate entry.projection.exports and entry.projection.jobs from the
package metadata or by invoking the existing projection-enrichment routine used
by loadSearchRowsAndRegistry, and update the registerSearchTool path to set
those fields instead of leaving them empty so searchUnified and
scoreOperationalIdentityBoost see real export/job names.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 970844c. Configure here.
| if (!field) continue | ||
| const normalizedField = extractSearchTokens(field).join(' ') | ||
| if (!normalizedField.includes(' ')) continue | ||
| if (normalizedQuery.includes(normalizedField)) return true |
There was a problem hiding this comment.
Phrase match ignores token boundaries causing false positives
Low Severity
hasPrimaryPhraseMatch joins tokens with spaces and uses String.includes() for substring matching, which doesn't enforce word boundaries. A field like "spotify-play" (normalized to "spotify play") falsely matches a query like "spotify playlist" (normalized to "spotify playlist") because the character sequence "spotify play" is a prefix of "spotify playlist". This grants an undeserved +0.2 identity boost whenever a field's final token happens to be a prefix of a different query token. A boundary-aware check (e.g., padding both strings with spaces before comparing) would prevent these false positives.
Reviewed by Cursor Bugbot for commit 970844c. Configure here.


Summary
Testing
npm run test -- packages/worker/src/mcp/tools/search.node.test.ts packages/worker/src/mcp/tools/search-handler.node.test.tsnpx oxfmt --check packages/worker/src/mcp/tools/search.ts packages/worker/src/mcp/tools/search.node.test.tsnpx oxlint packages/worker/src/mcp/tools/search.ts packages/worker/src/mcp/tools/search.node.test.tsnpm run typecheckSummary by CodeRabbit
New Features
Tests