feat(cli): report what semantic_search actually searched - #14335
marius-kilocode merged 2 commits into
Conversation
semantic_search covers one indexed root, but its description claimed it searched the entire current workspace, and it pointed at Grep and Glob for exploring outside that root even though both are bounded to the same root and cannot reach outside it either. Read is the only one that can. KiloIndexing.search returns an empty array when the index is disabled, still building, or broken, which was indistinguishable from a genuine miss. A model reading "No relevant code found" would take it as proof the code does not exist and act on that. Output now names the scope it searched, and an empty result explains itself in terms of index state. The wording lives in its own module so it can be exercised without booting the indexing worker. Split out of the multi-root @ mentions work, since it changes tool output for every user rather than only for multi-root workspaces.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| - Find files by filename or extension — use `Glob` | ||
| - Read the contents of a known file — use `Read` | ||
| - Explore files outside the current workspace - use `Grep`, `Glob`, and `Read` | ||
| - Explore files outside the indexed root — use `Read` with an absolute path. `Grep` and `Glob` are bounded to the same root as this tool and cannot reach outside it either. |
There was a problem hiding this comment.
WARNING: This states that Grep and Glob "cannot reach outside" the indexed root, but that appears inaccurate. Both tools accept absolute paths and delegate to assertExternalDirectoryEffect (packages/opencode/src/tool/grep.ts:60, packages/opencode/src/tool/glob.ts:65), which allows paths outside the root after an external_directory prompt — and the default action when no rule matches is ask (packages/opencode/src/permission/index.ts:107). glob.ts even has explicit Kilo support for absolute patterns such as ~/.config/kilo/command/*.md. Bounded-by-default is the useful point; "cannot reach outside" is stronger than the implementation. Consider rewording to something like "they search the indexed root by default and need external_directory approval to go outside it" so the model does not avoid a working approach.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
Good catch — you're right, and I've fixed it in 8b7b8a5.
I had verified this earlier against an older revision where both tools resolved a base and then hard-failed with Path escapes the active Location. That is no longer how they work: glob.ts:65 and grep.ts:60 now call assertExternalDirectoryEffect, which returns false when the target is inside the instance directory or worktree and otherwise raises an external_directory prompt via ctx.ask (external-directory.ts:36-54). It permits the path once approved rather than refusing it, and glob.ts carries an explicit Kilo change for absolute patterns.
So "cannot reach outside" was wrong, and wrong in the direction that matters — it would steer a model away from an approach that works.
Reworded to keep the useful distinction rather than drop it, since semantic_search genuinely cannot go outside its indexed root:
Explore files outside the indexed root — use
Read,GreporGlobwith an absolute path. They search the indexed root by default and needexternal_directoryapproval to go beyond it, but they can get there; this tool cannot.
I applied the same correction to the constraint line and to the runtime empty-result message, which carried the same "use Read with an absolute path" advice. Tests updated for the new wording and passing.
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (6 files)
Notes: the output module split is clean and Kilo-owned, wording is covered by hermetic tests, no memory leaks, no Fix these issues in Kilo Cloud Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0 Review guidance: REVIEW.md from base branch |
Both delegate to assertExternalDirectoryEffect, which prompts for external_directory and allows the path once approved, rather than refusing it. Saying they cannot reach outside the root would steer a model away from an approach that works. semantic_search itself still cannot, which is the distinction worth drawing.
There was a problem hiding this comment.
Thanks for the clean split and the focused tests.
Reviewed and tested locally: the two test files pass (16 focused, 234 in the kilo tool suite), bun run typecheck is clean, and I verified the new output end to end in VS Code with indexing disabled. It now says nothing was searched instead of reporting a miss, and names the searched root.
2 participants
Issue
No existing issue. Split out of #13589 at review request — these changes affect
semantic_searchoutput for every user, not only multi-root workspaces, so they are easier to judge on their own.Context
semantic_searchcovers a single indexed root, and two things about how it reports that were misleading:Its description claimed it searched "the entire current workspace", and pointed at
GrepandGlobfor exploring outside that root. Both are bounded to the same root and fail withPath escapes the active Location(tool/glob.ts,tool/grep.ts), soReadis the only one that can actually reach outside. That advice was wrong before this PR.More importantly,
KiloIndexing.searchreturns[]when the index is disabled, still building, or broken (indexing.ts:579), which was indistinguishable from a genuine miss. The output saidNo relevant code found, so a model would take that as evidence the code does not exist and act on it.Implementation
Output now names the scope actually searched, and an empty result explains itself in terms of index state — complete, still building with progress, disabled, or failed. Index status is only consulted on the empty path, and it rides on state
search()has already resolved (both go through the samehit()), so no extra indexing work is triggered.The wording lives in
semantic-search-output.tsso it can be tested without booting the indexing worker, which is why the tests are fast and hermetic.No change to the indexer, consent, storage, or embedding behaviour.
Screenshots / Video
N/A — CLI tool output, no visual surface.
How to Test
Manual/local verification
Executed by the agent:
bun run typecheckinpackages/opencode— passbun run script/test-runner.ts kilocode/semantic-search.test.ts kilocode/tool/semantic-search-output.test.ts— passsemantic-search-output.test.tsis new and covers each index state, the scope wording, Windows path separators, and that a disabled index never claims the code is absent. Three assertions in the existingsemantic-search.test.tswere updated for the new output, and index status is stubbed there so the wording no longer depends on real indexing progress — it was picking up a genuine "still building (0%, 0/0 files)" state in CI.Reviewer test steps
semantic_searchfor something that does not exist — the output should say the index is up to date and therefore that no similar code existsBlocked checks and substitute verification
bun turbo typecheckat the repo root could not complete:@kilocode/kilo-jetbrains#typecheckfails with "Cannot find a Java installation matching languageVersion=21" on this machine. This PR touches no Kotlin. Substitute verification was runningtypecheckdirectly inpackages/opencode.Checklist
Get in Touch