docs(architecture): sync mermaid diagrams with v4.8.2 fast-path + v4.8.3 staging dispatcher - #170
Merged
Merged
Conversation
…ture The 4 diagrams in ## Architecture (System Overview, Query Processing Flow, Document Ingestion Flow, hybrid_alpha Parameter Effect) still described the pre-v4.8 hybrid pipeline and a 4-category storage layout, missing every architectural change from the FTS5 fast-path release and the zero-downtime rebuild hotfix. This aligns the docs with what the code actually does. System Overview - Adds a dedicated SEARCH DISPATCH subgraph (search_method: auto | hybrid | fts5, ADR-006) with QueryRouter regex-based lexical detection (ADR-002) and the FTS5 fast-path branch with fallback arrow to the hybrid pipeline. - Adds SQLite FTS5 storage (data/fts5_index.db, ADR-001) alongside ChromaDB. - Corrects the category list from 4 pre-existing categories to the full 8 (redteam | blueteam | ctf | security | logscale | development | aar | general). - Adds the FTS5 CRUD sync box in the ingestion pipeline (task 05). - Marks the 13 MCP tools as frozen (LEI 1) and lists all 13. Query Processing Flow - New DISPATCH subgraph at the top: search_method decision, QueryRouter auto classification, FTS5 fast-path with ready-and-min_hits gate, and the fallback edge back to the hybrid path. - Preserves the existing hybrid pipeline (expansion, keyword routing, RRF, cross-encoder rerank, adjacent chunk expansion) for the semantic branch. - Result envelope now shows the fts5 search_method value and the fts5_router routed_by tag surfaced by the fast-path. Document Ingestion Flow - Expands the input tree to the full 8-category layout. - Adds the Write Dispatch subgraph (v4.8.3 #161) showing _write_collection routing writes to staging while nuclear_rebuild is populating and to production otherwise. - Adds the SQLite FTS5 store as a third sink alongside ChromaDB and BM25, reflecting the CRUD sync path (ADR-008). hybrid_alpha Parameter Effect - Adds an explanatory paragraph that hybrid_alpha only affects the hybrid path — the FTS5 fast-path uses SQLite bm25() natively and ignores the weight entirely. - Subgraph title updated accordingly. The 0.0 (Pure BM25) label now points readers to the FTS5 fast-path as an alternative for exact identifier queries. Unreleased changelog entry added. No code change, no test change, no version bump. LEI 1 and Pillar 7 unchanged.
📝 WalkthroughWalkthroughREADME.md now documents FTS5 lexical routing, fallback behavior, synchronized ChromaDB and SQLite storage, staging versus production writes, expanded categories, result metadata, and hybrid-only ChangesArchitecture documentation
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+314
to
+315
| FASTPATH --> NOTREADY | ||
| NOTREADY -->|no, fallback| EXPAND |
There was a problem hiding this comment.
The diagram sends explicit search_method="fts5" requests through the auto-mode fallback gate. When FTS5 is disabled or unready, forced mode returns Fts5NotReadyError instead of falling back, and forced searches also bypass min_hits, so readers receive behavior different from this documented flow.
Prompt To Fix With AI
This is a comment left during a code review.
Path: README.md
Line: 314-315
Comment:
**Forced FTS5 fallback is inaccurate**
The diagram sends explicit `search_method="fts5"` requests through the auto-mode fallback gate. When FTS5 is disabled or unready, forced mode returns `Fts5NotReadyError` instead of falling back, and forced searches also bypass `min_hits`, so readers receive behavior different from this documented flow.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+413
to
+416
| STAGING --> CHROMADB | ||
| PROD --> CHROMADB | ||
| PROD --> BM25IDX | ||
| PROD --> FTS5DB |
There was a problem hiding this comment.
These edges imply that only production writes update BM25 and that FTS5 is updated directly on every production write. Staging population also builds an isolated BM25 index, while bulk and staging paths rebuild FTS5 from ChromaDB only after the destructive rebuild or collection swap, giving operators the wrong consistency timeline during rebuilds.
Prompt To Fix With AI
This is a comment left during a code review.
Path: README.md
Line: 413-416
Comment:
**Rebuild index flows are inaccurate**
These edges imply that only production writes update BM25 and that FTS5 is updated directly on every production write. Staging population also builds an isolated BM25 index, while bulk and staging paths rebuild FTS5 from ChromaDB only after the destructive rebuild or collection swap, giving operators the wrong consistency timeline during rebuilds.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 236-246: Update the SEARCH DISPATCH diagrams in README.md to show
search.lexical_fast_path.enabled as a prerequisite for the auto route, and label
the not_ready/low_hits fallback to Hybrid as applying only to
search_method="auto". Add the explicit fts5 behavior: raise Fts5NotReadyError
when the index is unavailable and bypass min_hits, while leaving the method
table as “SQLite FTS5 only.”
- Around line 282-284: Update both README diagrams to document the full staging
promotion and index-readiness sequence: nuclear_rebuild(swap=True) writes and
validates staging, atomically promotes it to production, rebuilds BM25, then
resets and asynchronously migrates FTS5. Indicate that readers use production
until promotion and FTS5 queries fall back to hybrid until migration completes.
- Around line 363-364: Update the README search flow and Search Method Values
documentation to distinguish the request selector contract (auto, hybrid, fts5)
from the result-item search_method label contract (fts5, hybrid, semantic,
keyword). Explain that auto resolves to the selected search path, and add fts5
to the Search Method Values list.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+236
to
+246
| subgraph SEARCH["SEARCH DISPATCH (v4.8.2+)"] | ||
| direction TB | ||
| SM["search_method dispatch<br/>auto | hybrid | fts5"] | ||
| QROUTER["QueryRouter (regex)<br/>lexical vs semantic<br/>(ADR-002)"] | ||
| FTS5PATH["FTS5 Fast-Path<br/>(exact identifiers <10ms)"] | ||
| HYBRIDPATH["Hybrid Pipeline<br/>(conceptual queries)"] | ||
|
|
||
| SM --> QROUTER | ||
| QROUTER -->|lexical| FTS5PATH | ||
| QROUTER -->|semantic| HYBRIDPATH | ||
| FTS5PATH -.->|not_ready / low_hits<br/>fallback| HYBRIDPATH |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 6 --glob '!README.md' \
'search_method|QueryRouter|lexical_fast_path|not_ready|min_hits|fts5' .Repository: lyonzin/knowledge-rag
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- README dispatch and method sections ---'
sed -n '220,325p' README.md
printf '%s\n' '--- relevant implementation files ---'
rg -l --glob '*.py' \
'search_method|fts5_enabled|fts5_min_hits|Fts5NotReadyError|fast_path_fallback|QueryRouter' \
mcp_server tests | sort
printf '%s\n' '--- dispatch symbols and nearby logic ---'
rg -n -C 10 --glob '*.py' \
'search_method|fts5_enabled|fts5_min_hits|Fts5NotReadyError|fast_path_fallback|QueryRouter' \
mcp_server tests | head -n 500Repository: lyonzin/knowledge-rag
Length of output: 39068
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- README method table and FTS5 configuration text ---'
sed -n '286,322p' README.md
rg -n -C 8 --glob 'README.md' \
'lexical_fast_path|search_method|FTS5|fallback|min_hits|not.ready|not_ready' README.md
printf '%s\n' '--- override contract tests ---'
sed -n '1,260p' tests/test_search_method_override.py
sed -n '145,180p' tests/test_e2e_fts5.pyRepository: lyonzin/knowledge-rag
Length of output: 50379
Correct the FTS5 dispatch diagrams.
Show search.lexical_fast_path.enabled as a prerequisite for the auto route. Restrict not_ready and low_hits fallback to search_method="auto". Explicit search_method="fts5" raises Fts5NotReadyError when the index is unavailable and bypasses min_hits; the method table can remain “SQLite FTS5 only.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 236 - 246, Update the SEARCH DISPATCH diagrams in
README.md to show search.lexical_fast_path.enabled as a prerequisite for the
auto route, and label the not_ready/low_hits fallback to Hybrid as applying only
to search_method="auto". Add the explicit fts5 behavior: raise Fts5NotReadyError
when the index is unavailable and bypass min_hits, while leaving the method
table as “SQLite FTS5 only.”
Comment on lines
+282
to
284
| FTS5SYNC["FTS5 CRUD sync<br/>(v4.8.2 task 05)"] | ||
| PARSERS --> CHUNKER --> FTS5SYNC | ||
| end |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 --glob '!README.md' \
'_write_collection|_staging_target|nuclear_rebuild|fts5|BM25|promotion|swap' .Repository: lyonzin/knowledge-rag
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- README architecture and ingestion sections ---'
sed -n '260,310p;380,425p' README.md
printf '%s\n' '--- relevant implementation symbols ---'
rg -n -C 12 \
'def (nuclear_rebuild|index_all|_write_collection|_staging_target|_remove_document_chunks)|_staging_target|staging|swap|nuclear_rebuild|_fts5_reset_and_rebuild|start_migration_background|_fts5_sync_add_from_doc|build_index' \
mcp_server tests --glob '*.py' | head -n 1200Repository: lyonzin/knowledge-rag
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- staging and rebuild definitions ---'
rg -n '^ def (_write_collection|_swap|_cleanup_stale_staging_collections|nuclear_rebuild|_initialize_fts5_dispatch|_maybe_start_fts5_migration|_ensure_bm25_index)|nuclear_rebuild\(' mcp_server/server.py
printf '%s\n' '--- staging/write implementation ---'
sed -n '1160,1175p;1770,1885p;4090,4145p' mcp_server/server.py
printf '%s\n' '--- rebuild and FTS5 initialization implementation ---'
sed -n '2420,2670p' mcp_server/server.py
printf '%s\n' '--- all README references to staging/rebuild/index readiness ---'
rg -n -C 4 'staging|promotion|nuclear_rebuild|FTS5|BM25|queryable|ready|swap' README.mdRepository: lyonzin/knowledge-rag
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- swap/rebuild implementation ---'
sed -n '2020,2445p' mcp_server/server.py
printf '%s\n' '--- exact post-swap helpers and call sites ---'
rg -n -C 18 \
'_swap_collections_atomic|_rebuild_bm25_post_swap|_fts5_reset_and_rebuild|_staging_target|_bm25_initialized|build_index\(\)' \
mcp_server/server.py | sed -n '1,900p'Repository: lyonzin/knowledge-rag
Length of output: 49099
Document the staging promotion and index-readiness sequence.
Update both diagrams to show that nuclear_rebuild(swap=True) writes to staging, validates it, atomically promotes it to production, rebuilds BM25, then resets and asynchronously migrates FTS5. State that readers use production until promotion and that FTS5 queries fall back to hybrid until migration completes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 282 - 284, Update both README diagrams to document
the full staging promotion and index-readiness sequence:
nuclear_rebuild(swap=True) writes and validates staging, atomically promotes it
to production, rebuilds BM25, then resets and asynchronously migrates FTS5.
Indicate that readers use production until promotion and FTS5 queries fall back
to hybrid until migration completes.
Comment on lines
+363
to
+364
| NOTREADY -->|yes, hit| MINSCORE | ||
| SNIPPET --> RESULTS["Results<br/>search_method: fts5 | hybrid | semantic | keyword<br/>routed_by: fts5_router | none<br/>score + filtered_by_score + content_length"] |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 6 --glob '!README.md' \
'search_method|routed_by|filtered_by_score|content_length' .Repository: lyonzin/knowledge-rag
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- README context ---'
sed -n '345,375p' README.md
printf '%s\n' '--- implementation dispatch and result labels ---'
sed -n '2660,2705p;2832,2895p;3048,3082p;3884,3992p' mcp_server/server.py
printf '%s\n' '--- focused tests ---'
sed -n '1,15p;107,125p' tests/test_search_method_override.py
sed -n '45,65p;78,106p' tests/test_e2e_fts5.pyRepository: lyonzin/knowledge-rag
Length of output: 16912
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 'search_method|effective_method|effective route|routed_by' README.md docs/features/fts5_fast_path.mdRepository: lyonzin/knowledge-rag
Length of output: 39223
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '820,865p;1305,1325p' README.mdRepository: lyonzin/knowledge-rag
Length of output: 2788
Clarify the two search_method contracts.
Document auto | hybrid | fts5 as the request selector. Document fts5 | hybrid | semantic | keyword as the result-item label. Explain how auto maps to the selected path, and add fts5 to the Search Method Values list.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 363 - 364, Update the README search flow and Search
Method Values documentation to distinguish the request selector contract (auto,
hybrid, fts5) from the result-item search_method label contract (fts5, hybrid,
semantic, keyword). Explain that auto resolves to the selected search path, and
add fts5 to the Search Method Values list.
There was a problem hiding this comment.
Pull request overview
Updates architecture documentation to reflect FTS5 dispatch, staging-aware ingestion, and current category organization.
Changes:
- Documents FTS5 routing, fallback, and result metadata.
- Adds staging write dispatch and secondary-index flows.
- Updates categories, tools, and
hybrid_alphaguidance.
Suppressed comments (4)
README.md:310
- Forced
search_method="fts5"must not enter the auto readiness/minimum-hit fallback gate. Runtime raisesFts5NotReadyErrorwhen disabled/unready and, when ready, bypassesmin_hitsentirely. Split forced and auto FTS5 branches so the diagram does not promise a hybrid fallback for explicit requests.
METHOD -->|fts5| FASTPATH
README.md:325
- This block still depicts a word-boundary match as a hard
redteamfilter. Since the automatic keyword route is telemetry-only, both branches search the same hybrid corpus and differ only in therouted_byvalue.
subgraph ROUTING["Keyword Routing (category filter)"]
KWROUTER["Keyword Router<br/>(word boundaries)"]
README.md:394
- “Keeps prod serving queries” overstates the current isolation. Production ChromaDB remains query-visible, but
_populate_staging()rebindsself.bm25_index, andquery()reads that field dynamically, so BM25/hybrid behavior can change during population. Scope this promise specifically to ChromaDB reads.
STAGING["Staging Collection<br/>(nuclear_rebuild only,<br/>keeps prod serving queries)"]
README.md:416
- These edges omit the isolated BM25 state populated during staging and imply direct FTS5 updates throughout rebuild ingestion. After a swap, runtime rebuilds BM25 and resets/repopulates FTS5 from the swapped ChromaDB; direct FTS5 synchronization applies only to production CRUD operations. Show those distinct consistency paths.
STAGING --> CHROMADB
PROD --> CHROMADB
PROD --> BM25IDX
PROD --> FTS5DB
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+243
to
+246
| SM --> QROUTER | ||
| QROUTER -->|lexical| FTS5PATH | ||
| QROUTER -->|semantic| HYBRIDPATH | ||
| FTS5PATH -.->|not_ready / low_hits<br/>fallback| HYBRIDPATH |
| subgraph HYBRID["HYBRID PIPELINE"] | ||
| direction LR | ||
| ROUTER["Keyword Router<br/>(word boundaries)"] | ||
| KWROUTER["Keyword Router<br/>(word boundaries → category filter)"] |
| ADJ --> MINSCORE | ||
| SNIPPET --> RESULTS["Results<br/>search_method: hybrid|semantic|keyword<br/>score + filtered_by_score + content_length"] | ||
| NOTREADY -->|yes, hit| MINSCORE | ||
| SNIPPET --> RESULTS["Results<br/>search_method: fts5 | hybrid | semantic | keyword<br/>routed_by: fts5_router | none<br/>score + filtered_by_score + content_length"] |
| CHUNKER["Chunking<br/>MD: section-aware<br/>Other: 1000 chars + 200 overlap"] | ||
| PARSERS --> CHUNKER | ||
| FTS5SYNC["FTS5 CRUD sync<br/>(v4.8.2 task 05)"] | ||
| PARSERS --> CHUNKER --> FTS5SYNC |
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
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.
Summary
Syncs the 4 Mermaid diagrams in
## Architecture(System Overview, Query Processing Flow, Document Ingestion Flow, hybrid_alpha Parameter Effect) with what the code actually does in v4.8.2 (FTS5 Lexical Fast-Path) and v4.8.3 (_write_collectionstaging dispatcher,#161). The previous diagrams still described the pre-v4.8 hybrid-only pipeline and a 4-category storage layout, so any user reading the README would miss the FTS5 fast-path, thesearch_methoddispatch, the FTS5 storage box, the FTS5 CRUD sync in ingestion, and the zero-downtime staging write path.Docs-only. No code change, no test change, no version bump.
What changed
search_method: auto | hybrid | fts5, ADR-006),QueryRouterlexical detection (ADR-002), FTS5 fast-path branch with fallback edge to the hybrid pipeline, SQLite FTS5 storage box (data/fts5_index.db, ADR-001), the FTS5 CRUD sync step in ingestion, the full 8 categories (redteam | blueteam | ctf | security | logscale | development | aar | general — was showing only 4), and marks the 13 MCP tools as frozen (LEI 1) with the actual names.search_methoddecision,QueryRouter.classify()auto-routing, FTS5 fast-path withready AND hits >= min_hitsgate, and the fallback edge back to expansion + hybrid path. Result envelope updated to showsearch_method: fts5androuted_by: fts5_router.#161) showing_write_collectionrouting writes to staging duringnuclear_rebuildand to production otherwise, and added SQLite FTS5 as a third store sink (ADR-008 CRUD sync).hybrid_alphaonly affects the hybrid path — the FTS5 fast-path uses SQLitebm25()natively and ignores the weight. Subgraph title clarified,0.0 (Pure BM25)label points readers to the fast-path as an alternative for exact identifier queries.LEI 1 preserved
Docs-only. Zero change to the 13 frozen MCP tools, zero change to API surface, zero change to test count.
Test plan
flowchartopens in file)### UnreleasedSummary by CodeRabbit
hybrid_alphausage.Greptile Summary
The PR updates the README architecture documentation for FTS5 query dispatch, staging-aware ingestion, the eight-category layout, and hybrid weighting behavior.
Confidence Score: 4/5
The documentation-only PR is safe to merge, but two architecture paths should be corrected to avoid misleading users about forced FTS5 errors and rebuild-time index consistency.
Runtime behavior is unchanged; the remaining issues are inaccurate diagram branches for explicit FTS5 dispatch and BM25/FTS5 maintenance during rebuilds.
Files Needing Attention: README.md
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR Q[Query] --> D{search_method} D -->|auto| R[QueryRouter] D -->|hybrid| H[Hybrid pipeline] D -->|fts5| F[FTS5 fast path] R -->|semantic| H R -->|lexical| F F --> O[Output processing] H --> O I[Document ingestion] --> W{Write collection} W -->|default| P[Production ChromaDB] W -->|rebuild| S[Staging ChromaDB] P --> B[BM25 / FTS5 maintenance] S --> X[Post-swap index rebuild]Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "docs(architecture): sync mermaid diagram..." | Re-trigger Greptile