Repository navigation
feat(mcp): support document-scoped knowledge search - #4463
Jarvis636431 wants to merge 1 commit into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f8933de9c
ℹ️ 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".
| target = &types.SearchTarget{ | ||
| Type: types.SearchTargetTypeKnowledge, KnowledgeBaseID: kb.ID, TenantID: kb.TenantID, | ||
| } |
There was a problem hiding this comment.
Disable recall thresholds for explicit document targets
When knowledge_ids selects a document whose best vector/keyword hits fall below the configured recall thresholds, this target leaves DisableRecallThresholds at its zero value. SearchKnowledgeTool calls SearchTarget.RecallThresholds before HybridSearch, so those hits are discarded before reranking and the scoped search can return “no matching chunks”; the existing explicit-document path in internal/application/service/session_knowledge_qa.go (buildSearchTargets) sets this flag specifically to keep a user-selected scope alive through reranking. Set it on these document targets as well.
Useful? React with 👍 / 👎.
| if seen[id] { | ||
| continue | ||
| } | ||
| k, kb, err := s.knowledgeInKnowledgeBases(ctx, kbs, id) |
There was a problem hiding this comment.
Bound or batch the document-ID validation loop
When a client supplies a large all-valid knowledge_ids list, this loop issues one synchronous GetKnowledgeByIDOnly database lookup for every distinct ID before search. The public MCP route has no request-body cap and the new schema/handler has no maxItems or runtime bound, so one rate-limited call can turn thousands of IDs into thousands of serial DB queries and then an oversized backend filter, causing request stalls and database load; enforce a finite list size and/or batch the validation.
Useful? React with 👍 / 👎.
Description
External MCP clients can locate a document with
list_documentsbut cannot restrictsearch_knowledgeto that document. Same-topic files from other dates or versions can therefore occupy the result limit.Add optional
knowledge_idsto the built-in Go MCP tool. Validate every document against the endpoint's authorized knowledge bases and the optionalknowledge_base_idsselection, then group document targets by knowledge base and pass their IDs into the existing retrieval filter before candidate truncation. Shared knowledge bases retain their owner tenant and document/tenant consistency checks. Omitted filtering preserves whole-KB search; empty, malformed, unavailable, unauthorized, or conflicting document selections return an error without issuing a retrieval call. Whitespace is trimmed and duplicate IDs are removed.Document the parameter and an example call. No database migration, new dependency, or changes to
ask, the standalone Python MCP server, or retrieval algorithms.Type of Change
Related Issue
Closes #4384
Testing
Passed:
Lint used v2.12.2 and reported 0 issues. Link checking covered 76 documents and 350 internal links, with 0 broken links.
Regression tests failed against unchanged main: explicit document selection returned a higher-ranked document outside the requested set in all three modes, and invalid selections were ignored. The fixed tests exercise the real MCP HTTP transport, endpoint guard, handler, shared search tool, and result formatting, with fixture services replacing storage and embeddings. Candidate count is set to one to verify that document filtering reaches retrieval before truncation. Coverage includes hybrid/semantic/keyword modes, grouped multi-document and cross-tenant shared-KB selection, endpoint restrictions, revoked shares, tenant mismatch, partial/empty scope conflicts, malformed arguments, schema discovery, and unchanged whole-KB/grep behavior. This is a contract test, not a live database/vector-engine integration run.
cd website-docs && npm run check:docspassed link checking but could not run the Mermaid check: the local dependency tree lacksjsdom(ERR_MODULE_NOT_FOUND). No Mermaid diagrams were changed. Full-repository checks and live model/database tests were not run.Checklist