fix(search): scope local-search graph expansion to top-hit docs (RAN-35)#80
Merged
fix(search): scope local-search graph expansion to top-hit docs (RAN-35)#80
Conversation
`search.LocalSearch` narrowed the initial entity set to the top-hit documents, but its BFS walk called `RelationshipsForEntity` without any doc filter. That re-expanded through unrelated relationships anywhere in the project, so a scoped query could leak off-scope entities and edges into the result set. Add `Store.RelationshipsForEntityInDocs(entityID, depth, docIDs)` — a BFS variant that constrains every hop to `doc_id IN (...)` and dedups relationships across hops. Use it from `LocalSearch` so the expansion stays inside the top-hit doc set. Both the REST search path (`/search/local`) and the MCP `docsiq_search_local` tool go through `search.LocalSearch`, so the REST/MCP behaviour stays aligned after the fix. Entity-focused paths (`/entities/:id`, MCP entity-graph tools) intentionally keep the unscoped walk — they take an entity as input, not a document set. Regression coverage: - store: `TestRelationshipsForEntityInDocs_OnlyReturnsEdgesFromScopedDocs` proves a relationship from an unrelated document cannot appear in a scoped BFS result; depth and empty-input cases covered too. - search: `TestLocalSearch_GraphExpansionScopedToTopHitDocs` exercises the full `LocalSearch` path end-to-end. Co-Authored-By: Paperclip <noreply@paperclip.ing>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ebcb85fd5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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
search.LocalSearchnarrowed the initial entity set to top-hit docs but then calledRelationshipsForEntitywith no doc filter, so BFS expansion could leak off-scope entities/edges into a scoped result. Fixes RAN-35.Store.RelationshipsForEntityInDocs(entityID, depth, docIDs)— BFS that constrains every hop todoc_id IN (...)and dedups across hops.LocalSearchnow uses it so the expansion stays inside the top-hit doc set./search/local) and MCP (docsiq_search_local) both route throughsearch.LocalSearch, so behaviour stays aligned after the fix. Entity-focused endpoints that take an entity ID (not a doc set) keep the unscoped walk.Regression coverage
internal/store/relationships_for_entity_in_docs_test.goTestRelationshipsForEntityInDocs_OnlyReturnsEdgesFromScopedDocs— proves a relationship from an unrelated document cannot appear in a scoped BFS result (directly exercises the leak described in RAN-35).TestRelationshipsForEntityInDocs_EmptyDocsReturnsNil— empty doc set → no expansion.TestRelationshipsForEntityInDocs_RespectsDepthLimit— depth=1 from a seed only returns the direct edge.internal/search/local_scope_test.goTestLocalSearch_GraphExpansionScopedToTopHitDocs— end-to-end: top chunk's doc set scopes the graph walk; a relationship a seed entity participates in via an unrelated doc does not leak through.Test plan
CGO_ENABLED=1 go build -tags sqlite_fts5 ./...CGO_ENABLED=1 go vet -tags sqlite_fts5 ./...CGO_ENABLED=1 go test -tags sqlite_fts5 -timeout 300s ./...— 662 passedmain(verified by asserting that unscopedRelationshipsForEntitystill surfaces the out-of-scope edge in the fixture)