Repository navigation
fix(knowledge): repair 4 broken KB-search call sites found by #13009's audit - #13038
Merged
Merged
Conversation
…arch call sites (#13024) KnowledgeBase.search() (knowledge/search.py) has no n_results or search_mode kwargs -- every call site using them raised TypeError, swallowed by a broad except and silently returned no/empty results. Fixed the kwarg names to the canonical top_k, and for grounded_agent.py also dropped limit= (which routes to the Enhanced dict-returning path this caller can't consume as a List). Applied RESEARCH_QUARANTINE_FILTER (#13009/#12622) at the two live sites (api/chat_knowledge.py, services/grounded_agent.py) and the KBLibrarian tool-discovery site, since fixing the TypeError makes them real general-purpose reads for the first time.
…ning search path (#13025) self.knowledge_base.search(query, limit=limit) routed the canonical search() (#10666 consolidation) to its "Enhanced" path, which returns a Dict, not the List[Dict] this method iterates over -- iterating a dict yields its keys, so result.get(...) raised AttributeError on every non-empty search, silently caught and turned into []. Switched to top_k= to stay on the Basic (List) path, and applied RESEARCH_QUARANTINE_FILTER (#13009/#12622) now that this live path (api/kb_librarian.py, api/workflow.py, agent_orchestration/agent_execution.py) actually returns content.
…er raises TypeError (#13026) get_knowledge_base = lazy_singleton(lambda: KnowledgeBase(config_manager=get_config())) raised TypeError on every call -- KnowledgeBase.__init__(self) takes no parameters (config is read internally via SSOT config). Every route in this file wraps the call in a broad except returning a generic error payload, so all 10 MCP knowledge tool endpoints (search, summarize, vector similarity, QA chain, resource read, ...) silently never worked. Fixed the factory to lazy_singleton(KnowledgeBase), matching the dependencies.get_knowledge_base() pattern used elsewhere, and applied RESEARCH_QUARANTINE_FILTER (#13009/#12622) at every kb.search() call in the file now that they actually execute.
…f relying on dead constructor injection (#13027) get_tool_registry = lazy_singleton(ToolRegistry) is constructed with no args by every production call site (services/llm_service.py, api/image_generation.py), so knowledge_base was permanently None and search_knowledge_base()/get_fact() always reported "Knowledge base is not available". Added _resolve_knowledge_base(), which falls back to the shared knowledge_factory.get_knowledge_base_async() singleton on first use and caches the result, regardless of construction order. Also fixed a pre-existing #13024-class bug discovered in the same method: search_knowledge_base() itself called .search(query, n_results=...), which would have just traded one silent failure for another (a caught TypeError) once the KB was wired in. Applied the same fix + RESEARCH_QUARANTINE_FILTER (#13009/#12622) to add_file_to_knowledge_base()/store_fact(), which share the identical dead-injection defect.
Contributor
✅ SSOT Configuration Compliance: Passing🎉 No hardcoded values detected that have SSOT config equivalents! |
This was referenced Jul 30, 2026
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
1 participant
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.
Closes #13024, Closes #13025, Closes #13026, Closes #13027
Thinking Path
Read all four issues in full, then re-verified each premise independently against the actual code rather than trusting the filed text (this session has repeatedly found reports wrong in both directions).
knowledge/search.py:194(SearchMixin.search): the canonical signature has no**kwargscatch-all and non_results/search_modeparameters --agents/kb_librarian/librarian.py:236,358,agents/knowledge_retrieval_agent.py:241,275,api/chat_knowledge.py:456, andservices/grounded_agent.py:420-423all pass one of those and raiseTypeError, swallowed by broadexcept Exceptionblocks. Reachability differs per site:api/chat_knowledge.py(wired to livePOST /search) andservices/grounded_agent.py(live viaapi/knowledge_grounding.py, Tier-4 claim classifier) are genuinely broken in production.agents/kb_librarian/librarian.py's two methods (search_tool_knowledge,get_tool_instructions) have zero callers anywhere (not even tests) -- fixed for correctness per the issue's own affected-site list, but still dead code today.agents/knowledge_retrieval_agent.pyis the same dead class Dead code: ai_hardware_accelerator._gpu_semantic_search and KnowledgeRetrievalAgent have zero production callers #13028 covers (zero production instantiation callers) -- fixed the literal kwarg only, left the wire-in/deletion decision to Dead code: ai_hardware_accelerator._gpu_semantic_search and KnowledgeRetrievalAgent have zero production callers #13028 as instructed.agents/kb_librarian_agent.py:132:limit=limit(non-None) routessearch()to its "Enhanced" path (knowledge/search.py:253), which returns aDict, not theList[Dict]this method iterates --result.get(...)on a dict's string keys raisedAttributeErroron every non-empty result, caught and turned into[]. Reachability: live, viaapi/kb_librarian.py:92,api/workflow.py:61, andagent_orchestration/agent_execution.py:292/311'sprocess_query()->search_knowledge()chain.api/knowledge_mcp.py:62:KnowledgeBase.__init__(self)(knowledge/_composed.py:54) truly takes zero parameters --config_manager=raisedTypeErroron the first call, masked by each of the 10 MCP handlers' own broadexcept Exceptionreturning a generic error payload.tools/tool_registry.py:874:lazy_singleton(ToolRegistry)is invoked with no args by every production caller (services/llm_service.py:766/770/774,api/image_generation.py:110), soknowledge_basestaysNoneforever via constructor injection --search_knowledge_base()/get_fact()always hit their availability guard.All four premises held with only reachability nuance corrected (dead-code call sites called out honestly rather than folded silently into "fixed").
What Changed
top_k=replacesn_results=/search_mode=at all 4 call sites;services/grounded_agent.pyandagents/kb_librarian_agent.py-adjacent sites additionally droplimit=in favor oftop_k=to stay on the List-returning Basic search path.agents/kb_librarian_agent.pyswitcheslimit=totop_k=.api/knowledge_mcp.py's local singleton factory becomeslazy_singleton(KnowledgeBase)(matchesdependencies.get_knowledge_base()'s no-arg construction).ToolRegistry._resolve_knowledge_base(), which lazily resolves the KB via the sharedknowledge_factory.get_knowledge_base_async()singleton on first use if none was injected at construction, and caches it. Applied to all 4 KB-touching methods (search_knowledge_base,get_fact,add_file_to_knowledge_base,store_fact-- the latter two share the identical dead-injection defect, discovered along the way and fixed in the same change since tools/tool_registry.py singleton never receives knowledge_base — search_knowledge_base/get_fact tools always report KB unavailable #13027 isn't fully delivered without it). Also fixed a second, independent bug insearch_knowledge_baseitself: it also called.search(query, n_results=...)-- would have traded "KB unavailable" for a caughtTypeErroronce the KB was wired in.services/claim_verifier.py,services/research/orchestrator.py,services/research/planner.py, untouched). PR fix(knowledge): exclude quarantined research facts at every reachable KB search site (#13009) #13029 explicitly deferred applyingRESEARCH_QUARANTINE_FILTERat these exact files ("layering a quarantine fix on dead code"); now that the code isn't dead,filters=RESEARCH_QUARANTINE_FILTER(merged with any caller-supplied filter viarequest.filters or RESEARCH_QUARANTINE_FILTERwhere one exists) is applied at every touched.search()call.ai_hardware_accelerator._gpu_semantic_searchandKnowledgeRetrievalAgent) -- explicitly out of scope per instruction, not referenced inCloses.Verification
py_compile,flake8 --max-line-length=120,black --check --line-length=120 --target-version py312: clean on all 14 touched/new files.agents/kb_librarian/librarian_test.py,agents/kb_librarian_agent_test.py,agents/knowledge_retrieval_agent_test.py,api/chat_knowledge_test.py,api/knowledge_mcp_test.py,tools/tool_registry_test.py, plus 1 added to the pre-existingtests/services/test_grounded_agent.py): each usesunittest.mock.create_autospec(KnowledgeBase, instance=True)(not a bareAsyncMock) so an incompatible kwarg reproduces the exact realTypeError/AttributeErrorthese bugs caused in production, then asserts real content is returned (not just "no exception") and that the quarantine filter is threaded through.cp-swapped baseline (notgit stash, per worktree rules): swapped all 7 modified source files + the pre-existingtest_grounded_agent.pyback toorigin/Dev_new_gui, ran the pre-existing test set (test_tool_registry_web_research.py,test_tool_registry_gui.py,test_knowledge_mcp_web_research.py,test_grounded_agent.py,tool_registry_debug_e2e_test.py,quarantine_test.py,test_claim_verifier.py,research/orchestrator_test.py,research/planner_test.py,research/quarantine_boundary_test.py): 141 passed, 1 pre-existing failure (tool_registry_debug_e2e_test.py::test_orchestrator_initialization--Orchestratorhas notool_registryattribute, unrelated to this PR, filed separately as tool_registry_debug_e2e_test.py asserts Orchestrator.tool_registry, an attribute that doesn't exist #13037). Restored PR files, reran the same set + the 29 new tests: 156 passed, same 1 pre-existing failure. Identical failing set before/after.tests/test_startup_imports.py: 350/350 both before and after the swap.Model Used
Claude Sonnet 5