fix: handle root scope in shared search#70
Conversation
Treat root scope aliases in the shared scoped-search helper so callers like plan_change can search from the repository root with scope='.' or an explicit empty scope. This fixes the failure mode where the shared hierarchy collector treated '.' as a literal hierarchy path and returned no entities, even though the same query succeeded when unscoped or when given a concrete semantic path. Keep the change in rpg-nav search instead of adding MCP- or planner-specific normalization so all shared search consumers inherit the same behavior automatically. The scope parser now maps whole-input root aliases to the full graph, preserves '.' as a root alias inside comma-separated scope lists, and ignores empty trailing scope fragments so malformed lists do not widen unexpectedly. Add search-layer regressions for root-scope parity and empty-fragment handling, plus a planner-level regression covering the reported user-visible failure mode.
ebb5ceb to
53bfb24
Compare
userFRM
left a comment
There was a problem hiding this comment.
Thanks for the clean fix — correct architectural layer, minimal diff, and the trailing-comma regression test shows good defensive thinking.
Two things needed before merge:
Missing test coverage for two of the three new code paths
1. scope="" (empty string) — no test
The top-level guard at search.rs:565 handles this, and it's half the stated intent of the PR (plan_change(scope="") should behave like unscoped). Please add a test analogous to test_root_scope_matches_unscoped_search but with Some("") instead of Some(".").
2. scope="Security/auth,." (dot inside comma-separated list) — no test
The return at search.rs:574-576 is the most novel code path in this PR. When "." appears mid-list, it early-returns the full graph, discarding previously collected scoped entities. That's semantically correct (union with "everything" = "everything"), but it's the path most likely to break silently in a future refactor. Please add a test that verifies Some("Security/auth/token,.") produces the same results as unscoped search.
Everything else looks good
"."mid-list early return is correct — no point continuing to collect subsets when we already have everything- Empty-segment skip is the right call
- The
scope.trim()at the top handles whitespace-padded input correctly - CI is green
Minor note for future follow-up (not blocking): when scope=Some("."), the caller in search_with_params still builds a HashSet<String> of all entity keys then filters every entity against it — same result as the None path but with extra allocation. Could normalize Some(".")/Some("") → None at the call site. Not this PR's problem though.
userFRM
left a comment
There was a problem hiding this comment.
CI green, code correct, no bugs. The test gaps I flagged earlier are nice-to-haves — the fix itself is solid. Merging.
PR: Fix root scope handling for
plan_changeand shared scoped searchSummary
This change fixes root-scope handling in shared navigation search so
plan_change(scope=".")andplan_change(scope="")behave like unscoped repository-root searches.The fix is intentionally placed in the shared scoped-search helper rather than in MCP glue or planner-specific logic, so all downstream search consumers inherit consistent scope semantics.
Problem
plan_changecould returnNo relevant entities foundwhen called withscope=".", even though the same query succeeded with no scope or with an explicit semantic path.That happened because the shared scope collector treated
.as a literal hierarchy path instead of a repository-root alias.What Changed
Production code
Updated
crates/rpg-nav/src/search.rs:scope.trim()equal to"."or""now maps to the full graph"."segment inside comma-separated scopes also maps to the full graphThis keeps existing multi-scope union behavior intact while making root scope work consistently.
Tests
Added shared search-layer regressions in
crates/rpg-nav/tests/search.rs:test_root_scope_matches_unscoped_searchtest_multi_scope_with_empty_segment_does_not_widenAdded planner-level symptom coverage in
crates/rpg-nav/src/planner.rs:test_plan_root_scope_matches_unscoped_searchWhy This Approach
The bug was caused by shared scope interpretation, not by MCP parameter parsing.
Fixing it in
rpg-navsearch has a few advantages:Validation
Targeted local tests
Passed:
Workspace checks
Passed:
Note: full
cargo test --workspacecan be blocked on Windows when the worktree-builtrpg-mcp-server.exeis actively running, because Cargo cannot replace the locked executable. When the MCP server process is stopped, the workspace test run can proceed normally.Live MCP verification
Verified against the rebuilt
rpg_developmentMCP server:Both returned valid change plans.
Files Changed
crates/rpg-nav/src/search.rscrates/rpg-nav/src/planner.rscrates/rpg-nav/tests/search.rsReview Notes
crates/rpg-mcp/src/tools.rs