ADR-017: Knowledge ingestion and retrieval (local RAG) - #306
Merged
Conversation
…ipeline Adds the local SQLite knowledge store (documents/chunks, WAL mode, corrupt-db rescue, backup cadence - mirrors chat_library.py's own connection hygiene), structure-aware token-budgeted chunking with offset-exact citation support, and an extract -> chunk -> store ingestion pipeline that reuses attachments.py's pdf/docx/text extraction plus a new markup-stripping HTML extractor. Ingestion is idempotent by content hash, scoped per collection. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds an FTS5 external-content index over chunks.text (insert/delete triggers keep it in sync, migration backfills pre-existing rows), a bm25-ranked search_chunks() with safe query sanitization against FTS5 operator syntax, and registers knowledge.search on ToolRegistry under a new knowledge.read scope - auto-approval, read-only. Tested via direct registry.invoke() calls: ADR-008's tool-use loop (the piece that offers tools to a live model and processes ToolCallEvents) is not built yet, so this tool is registered and fully working but not yet wired into a live conversation - matching ADR-007's own established precedent for capabilities built ahead of their eventual caller. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds an `embedding` capability to ProviderCapabilities and a concrete .embed() method on OllamaProvider (per-model probe via ollama.show(), mirroring ollama_supports_tools) and OpenAIProvider (client-derived, mirroring image_generation) - the local-first-default-plus-API-option pair ADR-017 names. Anthropic/Gemini/llama.cpp declare embedding=False for this stage. The embeddings table (chunk_id, model_id) migration adds the vector index; knowledge_embeddings.py owns the numpy pack/unpack and provider calls: embed_pending_chunks() only ever embeds chunks with no cached vector for that model (the exit criterion's "cache prevents re-embedding"), and vector_search() is a brute-force cosine-similarity scan (numpy, already a dependency - no sqlite-vec or new pip package) matching the ADR's own "flat index file" alternative. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…untrusted context Adds backend/knowledge_retrieval.py: reciprocal_rank_fusion() merges FTS5 and vector search by rank (never by raw score - bm25 and cosine similarity aren't comparable scales), hybrid_search() runs both and fuses when an embedding-capable provider/model is supplied, degrading gracefully to lexical-only otherwise. select_within_budget() greedily trims a ranked result list to a token allowance instead of a fixed k, so retrieval can never overflow a small local model's context window. format_untrusted_ context() builds the labeled, instruction-resistant evidence block for automatic chat-turn augmentation, reusing Web Research's own established spotlighting convention with a distinct [k...] citation marker. knowledge.search (stage 17.2's tool) now runs hybrid_search() when an embedding provider/model is registered, unchanged (lexical-only) otherwise. search_chunks()/vector_search() now also return each chunk's token_count so budget-aware selection needs no second round-trip. Fixture-set test proves the exit criterion directly: a query that only lexical search answers, a paraphrase that only vector search answers, both correctly resolved by hybrid_search() where either index alone misses one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…) + citations UI Backend: - knowledge_ingest.ingest_text(): a sibling to ingest_file() for already- in-memory text with no file path to extract from - web-research retention and branch indexing both need this, not extract_text()'s file dispatch. - Web Research retention: WebResearchRequest.retain_to_knowledge (opt-in, default False) makes WebResearchService.run() ingest each accepted source document via ingest_text() after synthesis - a retention failure is logged and swallowed, never breaks the actual research result. - Branch indexing: ChatState.index_into_knowledge (per-node opt-in, set on whichever node's branch history should be indexed) + SceneDocument. set_chat_index_into_knowledge() + the new "knowledge" topic's two WS intents (backend/api/intents_knowledge.py): search (hybrid_search(), read-only, the frontend-reachable counterpart to backend/tools_knowledge. py's ADR-008-future tool) and setChatIndexIntoKnowledge (indexes the branch via chat_branch_history()+ingest_text() BEFORE flipping the flag, so a stored true always means the write actually happened). Frontend: a new "Knowledge" search panel (AppBar chip + Dialog, mirroring DiagnosticsDialog's request/reply shape) - search results render as "N sources used", each expandable to the exact cited excerpt (document[offsetStart:offsetEnd], byte-for-byte) with an Open source link for real http(s) sources. No fabricated "jump to this local file at a byte offset" mechanism - none exists anywhere in this codebase, so none is claimed here either; the excerpt itself IS the content at that offset. Contract: ChatState.index_into_knowledge -> indexIntoKnowledge (codegen regenerated). Guard updates: node-state-migration wire-key list, undo- classification intent count (143->145) + two new classifications. Verified live: real WS round-trip through the Knowledge panel against a freshly-started backend renders the correct empty-state UI with no console or server errors. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…race A 5-dimension adversarial review of the ADR-017 diff surfaced 10 confirmed findings. This fixes the six safely-scoped ones: - embed_pending_chunks() now raises instead of silently mispairing vectors to chunk_ids when a provider returns a mismatched batch length. - vector_search() validates query-embedding count and stored-vector dimension before np.stack(), turning a would-be opaque numpy crash into a clear, diagnosable error. - KnowledgeSearchDialog's search request had no in-flight guard: the Enter-key handler could fire a second request while the first was still pending, and an out-of-order response would silently overwrite fresher results. A sequence token now discards stale responses, and the guard makes a second Enter press during an active search a no-op. - The search input now has an aria-label, matching every other search input in the app. - Both knowledge WS intent handlers (search, setChatIndexIntoKnowledge) now run their blocking SQLite calls via asyncio.to_thread instead of inline on the event loop, matching every other blocking-I/O intent handler in the codebase. The remaining four findings are real but out of scope for a review-fix pass and are documented as known gaps in the (local, gitignored) ADR-017 doc: OpenAIProvider's client-derived (not model-derived) embedding capability check, the Ollama capability cache's pre-existing permanent-negative-caching behavior, the WS search intent being lexical-only in production pending a model-selection surface for embeddings, and format_untrusted_context's lack of escaping (dormant, matches Web Research's own already-shipped precedent). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Problem
The app had no way to persist knowledge for later retrieval: chat history, attachments, and web-research results all lived and died with their originating node, with no cross-session recall and no way to ground a conversation in previously-gathered material.
Change
Adds a local knowledge store with hybrid lexical + vector retrieval, and the surfaces to ingest and search it:
knowledge.searchToolRegistry tool (ready for a future model-driven tool-use loop).Provider.embed()on Ollama and OpenAI, an embedding cache, and a brute-force cosine-similarity vector index.A subsequent adversarial review pass (5 reviewers across store/ingest, retrieval/fusion, providers, sources, and the frontend panel) found 10 confirmed issues; 6 are fixed here (embedding batch/dimension-mismatch guards, a search-request race condition in the Knowledge panel, a missing input label, and blocking SQLite calls moved off the event loop in both knowledge WS intents). The other 4 are documented as known, deliberately-deferred gaps in the project's internal ADR doc (OpenAI's capability check is client- not model-derived; the Ollama capability cache can pin a transient negative; the WS search intent is lexical-only until there's a model-selection surface for embeddings; and the untrusted-context formatter has no escaping, matching Web Research's own already-shipped precedent).
Test plan
python -m pytest -q— 2314 passed, 17 skipped (5 unrelated pre-existing failures intest_native_dialogs.py, caused by the installedpywebview5.4 not exposingwebview.FileDialog; confirmed unrelated — that file isn't touched by this branch and was last changed in an old Qt-removal commit)npm run check(typecheck, lint, tests, build, bundle-size gate) — all green