Repository navigation
Split search.ts runtime into focused modules - #883
Merged
Merged
Conversation
kody-bot
marked this pull request as ready for review
July 22, 2026 20:41
📝 WalkthroughWalkthroughThe PR modularizes the MCP search tool into shared contracts, candidate generation, scoring, data loading, memory enrichment, entity resolution, formatting, response sizing, and tool registration modules. It supports unified query search, single-entity lookup, and batched entity lookup. ChangesUnified MCP search
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant runSearchTool
participant Loaders
participant searchUnified
participant Formatter
Client->>runSearchTool: submit query or entity reference
runSearchTool->>Loaders: load registry, rows, and connector statuses
runSearchTool->>searchUnified: generate and rerank candidates
searchUnified-->>runSearchTool: matches, intent, telemetry, timings
runSearchTool->>Formatter: render Markdown and structured content
Formatter-->>Client: formatted search response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
Contributor
|
🔎 Preview deployed: https://kody-pr-883.kody-a99.workers.dev Worker: Mocks:
|
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/worker/src/mcp/tools/search-tool-runner.ts`:
- Around line 455-471: The searchConversationIdsWithPreamble tracking in the
includePreamble block grows indefinitely, especially for generated conversation
IDs. Update this state update to bound the stored IDs by retaining only the most
recent configured limit, or restrict tracking to explicitly supplied
conversation IDs, while preserving preamble inclusion behavior.
In `@packages/worker/src/mcp/tools/search.ts`:
- Around line 1-16: Update the search barrel around the search-timing export to
re-export elapsedMs alongside settleWithBudget, preserving both timing helpers
for consumers importing from search.ts.
🪄 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: 4b9a2f4f-5fe1-447b-9134-bc94a7338083
📒 Files selected for processing (23)
packages/worker/src/mcp/tools/search-candidates.tspackages/worker/src/mcp/tools/search-constants.tspackages/worker/src/mcp/tools/search-core.tspackages/worker/src/mcp/tools/search-descriptors.tspackages/worker/src/mcp/tools/search-detail.tspackages/worker/src/mcp/tools/search-format-detail.tspackages/worker/src/mcp/tools/search-format-helpers.tspackages/worker/src/mcp/tools/search-format-list.tspackages/worker/src/mcp/tools/search-format-slim.tspackages/worker/src/mcp/tools/search-format-types.tspackages/worker/src/mcp/tools/search-format.tspackages/worker/src/mcp/tools/search-loaders.tspackages/worker/src/mcp/tools/search-memory.tspackages/worker/src/mcp/tools/search-package-rows.tspackages/worker/src/mcp/tools/search-register.tspackages/worker/src/mcp/tools/search-related-capabilities.tspackages/worker/src/mcp/tools/search-response-size.tspackages/worker/src/mcp/tools/search-scoring.tspackages/worker/src/mcp/tools/search-timing.tspackages/worker/src/mcp/tools/search-tool-definition.tspackages/worker/src/mcp/tools/search-tool-runner.tspackages/worker/src/mcp/tools/search-types.tspackages/worker/src/mcp/tools/search.ts
Comment on lines
+455
to
+471
| const searchConversationIdsWithPreamble = Array.isArray( | ||
| statefulAgent.state?.searchConversationIdsWithPreamble, | ||
| ) | ||
| ? (statefulAgent.state?.searchConversationIdsWithPreamble ?? []) | ||
| : [] | ||
| const includePreamble = | ||
| !args.conversationId || | ||
| !searchConversationIdsWithPreamble.includes(conversationId) | ||
| if (includePreamble && typeof statefulAgent.setState === 'function') { | ||
| statefulAgent.setState({ | ||
| ...(statefulAgent.state ?? {}), | ||
| searchConversationIdsWithPreamble: [ | ||
| ...searchConversationIdsWithPreamble, | ||
| conversationId, | ||
| ], | ||
| }) | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
searchConversationIdsWithPreamble grows without bound.
Every call with includePreamble true appends conversationId and it is never pruned. Since calls without an explicit conversationId generate a fresh id each time, this array accumulates indefinitely in Durable Object state, steadily inflating persisted state for long-lived agents. Consider capping it (e.g. keep the most recent N ids) or only tracking client-supplied conversation ids.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/worker/src/mcp/tools/search-tool-runner.ts` around lines 455 - 471,
The searchConversationIdsWithPreamble tracking in the includePreamble block
grows indefinitely, especially for generated conversation IDs. Update this state
update to bound the stored IDs by retaining only the most recent configured
limit, or restrict tracking to explicitly supplied conversation IDs, while
preserving preamble inclusion behavior.
Comment on lines
+1
to
+16
| export * from './search-constants.ts' | ||
| export { settleWithBudget } from './search-timing.ts' | ||
| export type * from './search-types.ts' | ||
| export * from './search-package-rows.ts' | ||
| export * from './search-descriptors.ts' | ||
| export * from './search-scoring.ts' | ||
| export * from './search-candidates.ts' | ||
| export * from './search-related-capabilities.ts' | ||
| export * from './search-core.ts' | ||
| export * from './search-memory.ts' | ||
| export * from './search-response-size.ts' | ||
| export * from './search-loaders.ts' | ||
| export * from './search-detail.ts' | ||
| export * from './search-tool-definition.ts' | ||
| export * from './search-tool-runner.ts' | ||
| export * from './search-register.ts' |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1) Detect duplicate exported names across the wildcard-exported search-* modules.
fd -e ts 'search-.*' packages/worker/src/mcp/tools \
| xargs rg -nP '^\s*export\s+(async\s+)?(function|const|class|type|interface|enum)\s+\w+' \
| sed -E 's/.*(function|const|class|type|interface|enum)\s+(\w+).*/\2/' \
| sort | uniq -d
# 2) Find consumers importing from the search.ts barrel to spot dropped symbols (e.g. elapsedMs).
rg -nP "from ['\"].*mcp/tools/search(\.ts)?['\"]" packages/worker/src -g '!**/search-*.ts'Repository: kentcdodds/kody
Length of output: 153
🏁 Script executed:
#!/bin/bash
set -u
echo "== candidate search files =="
fd -e ts '^search-.*\.ts$' packages/worker/src/mcp/tools | sort
echo
echo "== barrel file =="
cat -n packages/worker/src/mcp/tools/search.ts
echo
echo "== direct consumers of search.ts barrel from packages/worker/src, excluding search-* modules =="
rg -n "from ['\"].*/search(\.ts)?['\"]" packages/worker/src/mcp packages/worker/src/routes packages/worker/src/lib || true
echo
echo "== exports naming probe for wildcard modules (selected common decorators/statements) =="
python3 - <<'PY'
import re, subprocess, pathlib
files = subprocess.check_output(['fd','-e','ts','^search-.*\\.ts$','packages/worker/src/mcp/tools'], text=True).splitlines()
pat = re.compile(r'(?:export\s+(?:const|let|var|function|class|enum|async\s+function)\s+([A-Za-z_$][\w$]*))|(?:(?:type|interface)\s+([A-Za-z_$][\w$]*))')
names={}
for f in files:
text=pathlib.Path(f).read_text(encoding='utf-8')
matches=[g for g in pat.findall(text) if g[0]|g[1]]
for a,b in matches:
name=a or b
# ignore inline block-local exports like export { X } from './X.ts'
if re.search(r'export\s*\{\s*\S*\b'+re.escape(name)+r'\b[^}]*\}', text):
continue
names.setdefault(name,[]).append(f)
dups={name:fs for name,fs in names.items() if len(fs)>1}
print("duplicate_export_names:")
for name,fs in sorted(dups.items()):
print(name, ":", sorted(fs))
print("no_duplicate_export_names_found") if not dups else None
PYRepository: kentcdodds/kody
Length of output: 3191
🏁 Script executed:
#!/bin/bash
set -u
echo "== direct consumers of search.ts barrel from packages/worker/src, excluding search-* modules (full) =="
rg -n "from ['\"].*['\"]" packages/worker/src/mcp packages/worker/src/lib packages/worker/src/routes packages/worker/src/worker 2>/dev/null | rg "from ['\"].*/search(\.ts)?['\"]" || true
echo
echo "== imports of settleWithBudget/elapsedMs from anywhere =="
rg -n "settleWithBudget|elapsedMs" . --glob '!**/node_modules/**' --glob '!**/dist/**' || true
echo
echo "== exports and import of elapsedMs/settleWithBudget in timing =="
cat -n packages/worker/src/mcp/tools/search-timing.ts | sed -n '1,220p'Repository: kentcdodds/kody
Length of output: 6254
🏁 Script executed:
#!/bin/bash
set -u
python3 - <<'PY'
from pathlib import Path
import re
files = list(Path('packages/worker/src/mcp/tools').glob('search-*.ts'))
specifiers = set()
names = {}
for path in sorted(files):
text = path.read_text(encoding='utf-8')
hits = []
pat = re.compile(
r'export\s+(?:async\s+)?(?:function|const|let|var|class|enum|type|interface)\s+([A-Za-z_$][\w$]*)'
r'|export\s+type\s+([A-Za-z_$][\w$]*)'
r'|export\s*\{\s*([A-Za-z_$][\w$]*(?:\s+as\s+[A-Za-z_$][\w$]*)?)\b'
r'|from\s+(?:\.\./)?(?:\.\/)?search-timing\.ts|from\s+["\']\.\/search-timing\.ts["\']',
re.M,
)
for m in pat.finditer(text):
if 'from' in m.group(0):
hits.append(m.group(0))
elif m.group(3):
name = m.group(3).split()
hits.append((name[0], 'from ' + name[-1] if name[-1] != name[0] else None))
else:
hits.append((m.group(1) or m.group(2), 'decl'))
lines = {
line: text[:line.start()+1].count('\n')
for line in hits
if isinstance(line, (tuple, str))
}
names[path] = []
for item, origin in lines:
if isinstance(item, str):
specifiers.add(item)
else:
specifiers.add(item)
names[path].append((item, origin, lines[item]))
for path, entries in names.items():
print(f'{path}:')
for item, origin, line in entries:
print(f' line {line}: {origin!r}: {item}')
dup = [name for name in specifiers if sum(1 for entries in names.values() for item, _, _ in entries if item == name) > 1]
print("duplicate_export_items:")
print(sorted(dup))
PYRepository: kentcdodds/kody
Length of output: 324
Keep the dropped elapsedMs re-export if you want the search barrel to preserve its public timing helpers.
elapsedMs is exported from search-timing.ts and used by core/tool runner memory paths, but this barrel only re-exports settleWithBudget; direct imports from ./search.ts that still rely on elapsedMs will fail.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/worker/src/mcp/tools/search.ts` around lines 1 - 16, Update the
search barrel around the search-timing export to re-export elapsedMs alongside
settleWithBudget, preserving both timing helpers for consumers importing from
search.ts.
1 of 2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Mechanically splits remaining
search.tsruntime into focused modules.search.tsbecomes a thin barrel; MCP handler registration is thin. Includes the search-format split commits until that lands. No ranking/output changes. No production file undermcp/tools/exceeds ~1000 lines.Risk: MEDIUM — mechanical split; search unit tests prove unchanged behavior.
Test plan
Summary by CodeRabbit