Skip to content

KBLibrarianAgent.search_knowledge silently returns no results (Enhanced-mode dict vs expected list) #13025

Closed
Closed
KBLibrarianAgent.search_knowledge silently returns no results (Enhanced-mode dict vs expected list)#13025
Labels
backendinfrastructuretesting

Description

@mrveiss
mrveiss
opened on Jul 30, 2026

Context

Discovered while auditing every kb.search() call site for #13009 (research-quarantine leak audit).

autobot-backend/agents/kb_librarian_agent.py:132 (KBLibrarianAgent.search_knowledge) calls:

results = await self.knowledge_base.search(query, limit=limit)

KnowledgeBase.search() (autobot-backend/knowledge/search.py:194) dispatches on parameter presence: passing limit (not None) sets enhanced = True (knowledge/search.py:253), which routes to the "Enhanced" search path and returns a Dict[str, Any], not a List[Dict].

The caller then does:

if results:
    for result in results:
        formatted_results.append({"content": result.get("content", ""), ...})

Iterating a dict yields its keys (strings), so result.get(...) raises AttributeError on every non-empty result. This is caught by the method's broad except Exception, logged, and [] is returned — so KBLibrarianAgent.search_knowledge() never surfaces real KB content in production, it just silently returns empty results every time it's called with a non-empty KB.

Reachability

KBLibrarianAgent is live: api/kb_librarian.py:92, api/workflow.py:56/61, and agents/agent_orchestration/agent_execution.py:292/311 (the AgentType.KNOWLEDGE_RETRIEVAL and RAG handlers in the multi-agent orchestration engine) all call .process_query() / .search_knowledge() on it.

Suggested fix

Call .search(query, top_k=limit) instead of limit=limit (matching the "Basic" search-path contract this method's return-type handling expects), or explicitly handle the Enhanced dict shape if Enhanced-mode features (tags/category/min_score) are actually wanted here. Add a regression test with a non-empty mock KB asserting real content is returned, not [].

Model Used

Claude Sonnet 5

Activity

  1. mrveiss
    added
    backend
    on Jul 30, 2026
  2. github-actions
    added
    infrastructure
    testing
    on Jul 30, 2026
  3. mrveiss
    added a commit that references this issue on Jul 30, 2026

    fix(knowledge): repair 4 broken KB-search call sites found by #13009'…

    1296170

  4. github-actions commented on Jul 30, 2026

    @github-actions
    github-actionsbot
    on Jul 30, 2026 – with GitHub Actions
    Contributor

    PR #13038 (merged to Dev_new_gui) references this issue with a close keyword.

    fix(knowledge): repair 4 broken KB-search call sites found by #13009's audit

    If this issue is fully resolved, close it manually. If work remains, no action is needed.

  5. mrveiss commented on Jul 30, 2026

    @mrveiss
    mrveiss
    on Jul 30, 2026
    OwnerAuthor

    Fixed by PR #13038, merged to Dev_new_gui as 129617080 (batched with #13024, #13025, #13026, #13027 — all four fully delivered).

    All four premises confirmed against the canonical signature read directly (knowledge/search.py:194, SearchMixin.search — no **kwargs, no n_results, no search_mode), rather than inferred:

    Two defects the issues did not mention, found by reading and fixed in-scope:

    1. services/grounded_agent.py needed top_k= rather than merely dropping the stale kwarg — limit= routes to the Enhanced dict path this caller cannot consume, so a kwarg rename alone would have left it broken in a new way.
    2. tool_registry's dead-injection defect also affected add_file_to_knowledge_base and store_fact, not just the two methods reported. A _resolve_knowledge_base() helper (lazy fallback to knowledge_factory.get_knowledge_base_async()) now covers all four KB-touching methods.

    Quarantine gap closed as a consequence. PR #13029 deliberately skipped applying RESEARCH_QUARANTINE_FILTER to these files — layering a quarantine filter onto code that never returns content would have been meaningless. Now that these paths work, the filter is applied at every repaired general-purpose read; research-internal readers left untouched.

    Test technique worth noting. The 29 new regression tests use create_autospec(KnowledgeBase, instance=True) rather than bare mocks. A bare Mock accepts any keyword argument silently — which is precisely how n_results= and config_manager= survived in this codebase. The autospec reproduces the real production TypeError/AttributeError, so these tests would have caught the original defects. Each asserts real content is returned, not merely that no exception was raised — the failure mode here is silent emptiness, so "it didn't throw" proves nothing.

    Verification: cp-swapped baseline (never git stash) 141 passed / 1 pre-existing failure → 156 passed / same 1 pre-existing failure, identical failing set; tests/test_startup_imports.py 350/350 unchanged; py_compile / flake8 --max-line-length=120 / black --check --line-length=120 clean across all 14 files.

    Filed separately: #13037 (tools/tool_registry_debug_e2e_test.py:25 asserts Orchestrator.tool_registry, an attribute that class has never had). #13028 was deliberately excluded from this batch — dead-code removal versus wire-in is a separate decision under the never-delete rule.

  6. mrveiss
    closed this as completedon Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    backendinfrastructuretesting

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions