feat(knowledge): scope local_knowledge_search to a namespace

[chenmingwei23]

Problem / Motivation

The knowledge store already models a namespace label on every item -- items.namespace TEXT DEFAULT 'default' with a CREATE INDEX idx_items_namespace, and the dashboard already lists namespaces (GET /api/knowledge/namespaces), assigns them (upload picker, PATCH /api/knowledge/items/{id} accepts namespace) and browse-filters the item list by them (?namespace=).

Search was the one verb the label never reached. HybridRetriever.search() and the local_knowledge_search MCP tool scoped only by source_id, so an agent (or a user driving the KB) could organise documents into namespaces and filter the browse view by them, but could not tell a search "only look in this namespace". A single flat retrieval surface is exactly what #8266 asks to fix for the per-client / per-project / per-topic separation use cases.

Why it matters to the user

Someone who has bothered to file documents under namespaces wants retrieval to honour that split -- keeping a topic's reference docs from diluting an unrelated query's results, or scoping an agent's local_knowledge_search to the namespace it is working in. Today they can see the label everywhere except where it would actually change what a search returns.

What changed

• knowledge/retrieval.py: search(), _keyword_search() and _vector_search() take an optional namespace. It is applied to the same SEED legs as source_id (FTS5 keyword + vector similarity) as a parameterized items.namespace = ? match. The graph leg stays unfiltered, exactly as it is for source_id, so cross-namespace entity connections still contribute traversal context. source_id and namespace compose when both are given.
• mcp_tools/knowledge.py: local_knowledge_search gains an optional namespace argument (schema + validation + passthrough), mirroring source_id.
• validation.py: a namespace FieldSpec on LOCAL_KNOWLEDGE_SEARCH_SCHEMA, max_len=64 to match the cap the ingest handler already enforces.

Design note, stated up front so it is not mistaken: knowledge namespaces are an ORGANISATIONAL LABEL, not a security boundary. There is no authorization anywhere in the tree keyed on the knowledge namespace column -- it is a plain SQL filter with a 'default' fallback. This change is a relevance/organisation filter with the same trust properties as the existing ?namespace= browse filter; it does not and must not be read as confidentiality enforcement. Chain from symptom to root cause: the symptom (search ignores namespaces) traces directly to the seed-leg WHERE clauses only knowing source_id; the fix adds the parallel clause the label already earned everywhere else.

Scope is deliberately the smallest coherent slice -- search scoping only. The issue's other verbs (a CLI create/delete surface, and binding a namespace to a workspace, its item 4) are separate, larger changes, so this is Refs, not Closes.

Tests

Only the touched test files, run at -n0:

• test/test_knowledge.py: new TestHybridRetrieverNamespaceFilter -- keyword-leg narrowing, vector-leg narrowing, omitted-namespace regression (keeps all), unknown-namespace returns nothing, and namespace + source_id composing.
• test/test_mcp_knowledge_search.py: namespace optional in the advertised schema, rejects non-string, rejects overlong (>64), and passes through to the retriever. Two existing search(...) call assertions were updated to carry the new namespace=None kwarg.

293 passed. Each new enforcement site was mutation-verified by hand and reddened a DISTINCT set on a real assertion: neutering the keyword-leg filter reddened the keyword + compose tests (not the vector test); neutering the vector-leg filter reddened only the vector test; neutering the MCP passthrough reddened only the passthrough test. flake8, isort, mypy (my files) and the diff-scoped black gate are clean; merge-tree against main is clean.

Pattern harvest

Mirrored the existing source_id seed-leg scoping verbatim rather than inventing a new mechanism, including its "seed legs only, graph leg unfiltered" contract and its parameterized-bind discipline. The one intentional divergence from the source_id shape: namespace is a direct items.namespace column with no source_locations ownership-OR-location join, because a namespace lives on the item itself.

Refs #8266

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 8ee357fb4066e6644b56221c1e53540642a64055 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 8ee357f

Verdict parsed from the review's SHA-scoped output markers for commit 8ee357fb4066e6644b56221c1e53540642a64055.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 8ee357fb4066e6644b56221c1e53540642a64055: <one-sentence reason>

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 8ee357fb4066e6644b56221c1e53540642a64055 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 8ee357f

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 8ee357fb4066e6644b56221c1e53540642a64055: <one-sentence reason>

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

Design-level review of 8ee357fb4066e6644b56221c1e53540642a64055 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Namespace scoping is seed-only, so scoped searches still return other namespaces' items via the graph leg — the opposite of the dashboard's strict filter.

Watch

• The diff filters only the keyword/vector seeds ("the graph leg stays unfiltered") while the dashboard's ?namespace= hybrid-search branch post-filters results strictly (handlers/knowledge.py:401). Cause → mechanism → consequence: a graph-only match from another namespace enters _rrf_fuse and is returned verbatim, so the same query + namespace yields different result sets in the MCP tool vs the dashboard — and for the per-client/per-project separation use case Make knowledge-base namespaces user-manageable (create, assign, scope search) instead of everything landing in 'default' #8266 names, client A's scoped search can surface client B's document content. It is documented, but "leaks by design" is a surprising contract for the very use case cited; a post-fusion namespace filter (graph still contributes ranking, results stay in-namespace) matches user expectation and the dashboard's existing semantics.
• docs/system-specs/modules/knowledge.md §4 specifies the local_knowledge_search contract (it documents source_id seed-scoping in detail) but this commit adds the namespace param without updating it — AGENTS.md requires the owning spec in the same commit; the spec is now stale on new public tool surface.

Suggestions

• If seed-only filtering is kept, state in the tool description that results may include out-of-namespace items (not just "connections") — the current wording implies traversal context, not returned documents.

[DESIGN-REVIEWED] 8ee357f

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 8ee357fb4066e6644b56221c1e53540642a64055 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Verification done. I checked the retriever's call sites (3 in src/: mcp_tools/knowledge.py, dashboard/handlers/knowledge.py:382 and :1922, plus the eval bench), the dashboard's existing namespace handling, and the 64-char ingest cap (handlers/knowledge.py:874). Emitting the review.

First-Principles-Verdict: CONCERNS

Namespace scoping earns its place, but the dashboard's own search already namespace-filters — post-retrieval, defectively — and this PR's fix reaches neither dashboard call site.

What this change ships

Intent: let an agent (and any retriever caller) restrict knowledge search to one namespace, per #8266. ADDITION.

1. local_knowledge_search accepts an optional namespace argument — justified (Make knowledge-base namespaces user-manageable (create, assign, scope search) instead of everything landing in 'default' #8266)
2. Search seeds (keyword + vector) narrow to one namespace; graph leg stays open — justified, mirrors source_id contract
3. Non-string / >64-char namespace rejected before dispatch — justified, derived from the ingest cap (handlers/knowledge.py:874)
4. Unknown namespace silently returns empty (no existence probe, unlike source_id) — declared
5. HybridRetriever.search(namespace=) public param — one consumer (mcp_tools/knowledge.py:240), minimal form

Watch

• "Search was the one verb the label never reached" overstates: GET /api/knowledge/items?q=&namespace= already narrows search results by namespace via a post-retrieval filter (dashboard/handlers/knowledge.py:401, inside the q branch). The genuinely new part is agent/MCP access plus seed-time scoping.
• Counted unfixed sibling (grep retriever.search( in src/: 3 call sites, 1 gains the param): that dashboard post-filter discards from a fixed limit * 3 window after global ranking — the exact defect the comment at handlers/knowledge.py:372-378 documents and _search_until_exhausted was built to fix for source_id. Seed-leg scoping isn't drop-in there (graph leg would leak cross-namespace items into a strict browse filter), so deferral is defensible — but the description should not imply search scoping is now done.

[FIRST-PRINCIPLES-REVIEWED] 8ee357f

[chenmingwei23]

Disposition of the First Principles CONCERNS (advisory -- no BLOCK-MERGE, so this does not gate):

Accepted as a correct observation, declined for this PR as out of scope.

The reviewer is right that dashboard/handlers/knowledge.py:401 reaches namespace for the browse/list view via an item.get("namespace") != namespace post-filter over a limit * 3 window, and that the seed-leg namespace= parameter this PR adds is the cleaner mechanism. Two reasons it stays out of this change:

1. Scope. This PR is the smallest coherent slice: namespace scoping on the SEARCH path (HybridRetriever + local_knowledge_search). list_items is a different surface (paged browse, not hybrid retrieval) with a different contract -- it returns a filtered page, not RRF-fused seeds -- so swapping its filter for the retriever's namespace= is a behaviour change to an endpoint this PR otherwise never touches. That belongs in its own change against Make knowledge-base namespaces user-manageable (create, assign, scope search) instead of everything landing in 'default' #8266's remaining verbs, not bundled here.

2. The post-filter is correct, just less efficient. It is a strict filter today (it does not leak; namespaces are an organisational label with no trust boundary, so there is nothing to leak), so this is an optimization, not a bug fix. Folding an optimization of a second surface into a search-scoping PR is exactly the padding the review process discourages.

I have adjusted the mental model but not the code: the description line "Search was the one verb the label never reached" is about the hybrid-retrieval search path (local_knowledge_search), which genuinely never scoped by namespace; the list/browse endpoint reaches it by post-filter, as the reviewer notes. Refs #8266 (not Closes) precisely because these sibling surfaces remain.