Repository navigation
Improve MCP search intelligence - #246
Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 45 minutes and 32 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughA comprehensive refactor of the search system introducing a multi-phase pipeline: query understanding to extract intent/entities/constraints, candidate generation with improved scoring, and reranking with entity/action matching signals. Telemetry and entity references are now tracked throughout the pipeline, with a new module for structured query parsing. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SearchTool as Search Tool
participant QueryParser as Query Parser
participant CandidateGen as Candidate<br/>Generator
participant Reranker as Reranker
participant Formatter as Result<br/>Formatter
User->>SearchTool: search({ query, entity })
SearchTool->>QueryParser: understandSearchQuery(query, entities)
QueryParser->>QueryParser: normalize & tokenize
QueryParser->>QueryParser: match tokens against<br/>entity aliases
QueryParser-->>SearchTool: SearchIntent<br/>(task, entities, actions)
SearchTool->>CandidateGen: generate candidates<br/>(lexical + vector)
CandidateGen->>CandidateGen: score base matches<br/>for capabilities,<br/>packages, values,<br/>connectors, secrets
CandidateGen-->>SearchTool: scored candidates
SearchTool->>Reranker: rerankCandidates<br/>(base scores + signals)
Reranker->>Reranker: apply entity match,<br/>action match,<br/>task affinity,<br/>constraint signals
Reranker-->>SearchTool: final ranked<br/>candidates
SearchTool->>Formatter: formatSearchMarkdown<br/>(matches, intent, telemetry)
Formatter->>Formatter: add entityRef,<br/>markdown details,<br/>telemetry/timings
Formatter-->>SearchTool: formatted result
SearchTool-->>User: { matches, intent,<br/>telemetry, phaseTimings }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 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-246.kentcdodds.workers.dev Worker: Mocks:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ec7218b. Configure here.
| ] | ||
| const confidence = | ||
| confidenceInputs.reduce((sum, value) => sum + value, 0) / | ||
| confidenceInputs.length |
There was a problem hiding this comment.
Overall confidence double-counts task signal via actions
Medium Severity
The overall confidence averages task.confidence, entities[0]?.confidence, and actions[0]?.confidence. However, actions[0]?.confidence is always identical to task.confidence because inferTaskSignals returns the strongest task as both task and as the first element of actions (sorted descending by score). This double-counts the task signal, inflating intent confidence, which in turn inflates rerankWeight in rerankCandidates and amplifies all reranking adjustments (entity match, task affinity, app availability, constraints).
Reviewed by Cursor Bugbot for commit ec7218b. Configure here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/worker/src/mcp/tools/search.ts (1)
877-897:⚠️ Potential issue | 🟡 MinorKeep app-opening help text in sync with the new
kody_idguidance.The structured search output now recommends
open_generated_ui({ kody_id }), but the tool description and example still only showpackage_id.📝 Proposed fix
-Open package apps with `open_generated_ui({ package_id })` or hosted package URLs. +Open package apps with `open_generated_ui({ kody_id })`, `open_generated_ui({ package_id })`, or hosted package URLs. @@ -- To open a saved package app: `open_generated_ui({ "package_id": "<id>" })` +- To open a saved package app: `open_generated_ui({ "kody_id": "<kody-id>" })`🤖 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 877 - 897, Update the help text and examples so they reference the new kody_id guidance instead of package_id: replace occurrences of "open_generated_ui({ package_id })" and example strings that use "package_id" with "open_generated_ui({ kody_id })" and corresponding example JSON; ensure descriptive sentences mentioning package_id are updated to mention kody_id as the recommended identifier (search output, example arguments, and any sentences referencing opening a saved package app). Verify the symbol open_generated_ui and keys kody_id/package_id in this comment block are consistent and update all example argument lines accordingly.
🤖 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 684-717: The code currently calls understandSearchQuery twice and
measures query understanding only for the non-empty path; move the creation of
emptyIntent into the if (!query) branch so you only call understandSearchQuery
once for empty queries, and for non-empty queries call understandSearchQuery
after building entityDescriptors (buildSearchableEntityDescriptors) and after
recording queryUnderstandingStart (performance.now()) so queryUnderstandingMs =
performance.now() - queryUnderstandingStart is accurate; reference the symbols
emptyIntent, understandSearchQuery, queryUnderstandingStart, performance.now(),
buildSearchableEntityDescriptors, entityDescriptors, and queryUnderstandingMs
when making this change.
In `@packages/worker/src/mcp/tools/understand-search-query.ts`:
- Around line 278-293: The issue is that the special-case match for the phrase
"set up" (when term === 'setup') only contributes one hit so queries like "set
up github" tie with the "operate" match and lose by lexicon order; in
inferTaskSignals, after computing matchedTerms for each [task, terms] entry,
detect if terms includes 'setup' and normalizedQuery.includes('set up') and if
so increment the matchedTerms count (or treat it as an extra match) before
computing score so the setup task gets a boost; update the score calculation (in
inferTaskSignals using tokenSet, matchedTerms, normalizedQuery and taskLexicon)
to use matchedTerms.length + (setupBoost ? 1 : 0) when computing score.
---
Outside diff comments:
In `@packages/worker/src/mcp/tools/search.ts`:
- Around line 877-897: Update the help text and examples so they reference the
new kody_id guidance instead of package_id: replace occurrences of
"open_generated_ui({ package_id })" and example strings that use "package_id"
with "open_generated_ui({ kody_id })" and corresponding example JSON; ensure
descriptive sentences mentioning package_id are updated to mention kody_id as
the recommended identifier (search output, example arguments, and any sentences
referencing opening a saved package app). Verify the symbol open_generated_ui
and keys kody_id/package_id in this comment block are consistent and update all
example argument lines accordingly.
🪄 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: 73efe349-3f70-4d72-aab3-dd8181c3725b
📒 Files selected for processing (5)
packages/worker/src/mcp/tools/search-format.node.test.tspackages/worker/src/mcp/tools/search-format.tspackages/worker/src/mcp/tools/search-handler.node.test.tspackages/worker/src/mcp/tools/search.tspackages/worker/src/mcp/tools/understand-search-query.ts
| const offline = isCapabilitySearchOffline(input.env) | ||
| const query = input.query.trim() | ||
| const emptyIntent = understandSearchQuery({ | ||
| query, | ||
| entities: [], | ||
| }) | ||
| if (!query) { | ||
| return { | ||
| matches: [], | ||
| offline, | ||
| intent: emptyIntent, | ||
| telemetry: buildCandidateTelemetry({ | ||
| intent: emptyIntent, | ||
| candidates: [], | ||
| matches: [], | ||
| }), | ||
| phaseTimings: { | ||
| queryUnderstandingMs: 0, | ||
| candidateGenerationMs: 0, | ||
| rerankingMs: 0, | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| const limit = Math.max(1, input.limit) | ||
| const queryUnderstandingStart = performance.now() | ||
| const entityDescriptors = buildSearchableEntityDescriptors({ | ||
| registry: input.registry, | ||
| optionalRows: input.optionalRows, | ||
| }) | ||
| const intent = understandSearchQuery({ | ||
| query, | ||
| entities: entityDescriptors, | ||
| }) |
There was a problem hiding this comment.
Move emptyIntent inside the empty-query branch.
Non-empty searches currently call understandSearchQuery twice, and the first call is outside queryUnderstandingMs, so phase timing under-reports the work.
♻️ Proposed fix
const offline = isCapabilitySearchOffline(input.env)
const query = input.query.trim()
- const emptyIntent = understandSearchQuery({
- query,
- entities: [],
- })
if (!query) {
+ const emptyIntent = understandSearchQuery({
+ query,
+ entities: [],
+ })
return {
matches: [],
offline,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const offline = isCapabilitySearchOffline(input.env) | |
| const query = input.query.trim() | |
| const emptyIntent = understandSearchQuery({ | |
| query, | |
| entities: [], | |
| }) | |
| if (!query) { | |
| return { | |
| matches: [], | |
| offline, | |
| intent: emptyIntent, | |
| telemetry: buildCandidateTelemetry({ | |
| intent: emptyIntent, | |
| candidates: [], | |
| matches: [], | |
| }), | |
| phaseTimings: { | |
| queryUnderstandingMs: 0, | |
| candidateGenerationMs: 0, | |
| rerankingMs: 0, | |
| }, | |
| } | |
| } | |
| const limit = Math.max(1, input.limit) | |
| const queryUnderstandingStart = performance.now() | |
| const entityDescriptors = buildSearchableEntityDescriptors({ | |
| registry: input.registry, | |
| optionalRows: input.optionalRows, | |
| }) | |
| const intent = understandSearchQuery({ | |
| query, | |
| entities: entityDescriptors, | |
| }) | |
| const offline = isCapabilitySearchOffline(input.env) | |
| const query = input.query.trim() | |
| if (!query) { | |
| const emptyIntent = understandSearchQuery({ | |
| query, | |
| entities: [], | |
| }) | |
| return { | |
| matches: [], | |
| offline, | |
| intent: emptyIntent, | |
| telemetry: buildCandidateTelemetry({ | |
| intent: emptyIntent, | |
| candidates: [], | |
| matches: [], | |
| }), | |
| phaseTimings: { | |
| queryUnderstandingMs: 0, | |
| candidateGenerationMs: 0, | |
| rerankingMs: 0, | |
| }, | |
| } | |
| } | |
| const limit = Math.max(1, input.limit) | |
| const queryUnderstandingStart = performance.now() | |
| const entityDescriptors = buildSearchableEntityDescriptors({ | |
| registry: input.registry, | |
| optionalRows: input.optionalRows, | |
| }) | |
| const intent = understandSearchQuery({ | |
| query, | |
| entities: entityDescriptors, | |
| }) |
🤖 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 684 - 717, The code
currently calls understandSearchQuery twice and measures query understanding
only for the non-empty path; move the creation of emptyIntent into the if
(!query) branch so you only call understandSearchQuery once for empty queries,
and for non-empty queries call understandSearchQuery after building
entityDescriptors (buildSearchableEntityDescriptors) and after recording
queryUnderstandingStart (performance.now()) so queryUnderstandingMs =
performance.now() - queryUnderstandingStart is accurate; reference the symbols
emptyIntent, understandSearchQuery, queryUnderstandingStart, performance.now(),
buildSearchableEntityDescriptors, entityDescriptors, and queryUnderstandingMs
when making this change.
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
2 participants
Summary
Testing
npm run test -- packages/worker/src/mcp/tools/search.node.test.ts packages/worker/src/mcp/tools/search-handler.node.test.ts packages/worker/src/mcp/tools/search-format.node.test.ts packages/worker/src/mcp/observability.workers.test.tsnpm run typechecknpx oxfmt --check packages/worker/src/mcp/tools/search.ts packages/worker/src/mcp/tools/search-format.ts packages/worker/src/mcp/tools/understand-search-query.ts packages/worker/src/mcp/tools/search.node.test.ts packages/worker/src/mcp/tools/search-handler.node.test.ts packages/worker/src/mcp/tools/search-format.node.test.tsSummary by CodeRabbit
New Features
Improvements