chore(release): v4.8.2 — FTS5 Lexical Fast-Path opt-in (default OFF) - #168
Conversation
v4.8.2 bundles Fases 1-5 of the FTS5 Lexical Fast-Path plan (.compozy/tasks/fts5-lexical-fast-path/) as an opt-in release - search.lexical_fast_path.enabled remains false by default, so users who do not touch config.yaml keep v4.8.1 behaviour byte-for-byte (LEI 1 preserved). The v4.9.0 default flip is deferred to the CI perf-gate adjudication documented in ADR-009 (bench harness ships in this release; the flip PR follows once G1 >=15x and G2 <=2% both pass on CI hardware). - Version bump 4.8.1 -> 4.8.2 (3 files atomic: pyproject.toml + mcp_server/__init__.py + npm/package.json) - New bench/test_bench_fts5_lexical.py (BENCH-001 cold + BENCH-002 hot) - New docs/features/fts5_fast_path.md user guide (6.5K) - README Configuration section: new Search Method (v4.8.2+) subsection - CHANGELOG entry v4.8.2 with LEI 1 + ADR-009 references Zero MCP tool signature change. Default OFF preserves v4.8.1 behavior.
📝 WalkthroughWalkthroughThe pull request documents the opt-in FTS5 lexical fast-path, exposes its configuration and search method, adds cold and warmed benchmarks, and updates the project versions to 4.8.2. ChangesFTS5 lexical fast-path
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @pytest.fixture(scope="module") | ||
| def fts5_index(tmp_path_factory): | ||
| """Fresh FTS5 index seeded with 3865 rows, closed on teardown.""" | ||
| tmp = tmp_path_factory.mktemp("fts5_bench") | ||
| idx = Fts5LexicalIndex( | ||
| db_path=tmp / "fts5_index.db", | ||
| state_path=tmp / "fts5_migration.state", | ||
| ) | ||
| _seed_fts5_corpus(idx) | ||
| yield idx | ||
| idx.close() |
There was a problem hiding this comment.
When CI runs the parametrized cold benchmark, calibration and measurement calls reuse the same module-scoped SQLite connection, warming its page cache before recorded samples and causing BENCH-001 to report warm or partially warm latency instead of the required cold latency.
Prompt To Fix With AI
This is a comment left during a code review.
Path: bench/test_bench_fts5_lexical.py
Line: 54-64
Comment:
**Cold benchmark reuses warm cache**
When CI runs the parametrized cold benchmark, calibration and measurement calls reuse the same module-scoped SQLite connection, warming its page cache before recorded samples and causing BENCH-001 to report warm or partially warm latency instead of the required cold latency.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| BENCH-003 and BENCH-004 are executed by re-running the existing | ||
| ``bench/test_bench_search.py`` suite with the FTS5 feature toggled via | ||
| env override (see ``bench_v4_9_0_gate.md`` for the CI procedure). They | ||
| are documented here — not duplicated — so the baseline metric IDs stay | ||
| stable between comparisons. |
There was a problem hiding this comment.
When CI adjudicates G2, it runs the ordinary branch and master benchmark suites without the documented FTS5 environment toggle, while test_bench_search.py does not exercise orchestrator fast-path dispatch. The resulting comparison cannot detect a hybrid regression caused by enabling FTS5.
Prompt To Fix With AI
This is a comment left during a code review.
Path: bench/test_bench_fts5_lexical.py
Line: 13-17
Comment:
**Hybrid regression gate is absent**
When CI adjudicates G2, it runs the ordinary branch and master benchmark suites without the documented FTS5 environment toggle, while `test_bench_search.py` does not exercise orchestrator fast-path dispatch. The resulting comparison cannot detect a hybrid regression caused by enabling FTS5.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| ## Configuration Reference | ||
|
|
||
| Full field docs live in `config.example.yaml` under | ||
| `search.lexical_fast_path`. The five knobs: |
There was a problem hiding this comment.
The guide says there are five knobs, but the following table and runtime schema define only four, leaving users searching for a nonexistent setting or wondering whether the reference is incomplete.
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/features/fts5_fast_path.md
Line: 47
Comment:
**Configuration knob count is incorrect**
The guide says there are five knobs, but the following table and runtime schema define only four, leaving users searching for a nonexistent setting or wondering whether the reference is incomplete.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 `@bench/test_bench_fts5_lexical.py`:
- Around line 78-80: Strengthen the assertions in the parameterized benchmark
tests around benchmark(run) so each query must return a non-empty result or the
expected seeded hits, while retaining the existing isinstance(result, list)
check. Apply the same assertion update to both benchmark cases covered by the
comment, including the occurrence around the second referenced location.
- Around line 54-64: Make the FTS5 benchmarks independent by changing fts5_index
to function scope so each case gets a fresh index, and ensure cold benchmark
runs create or reopen the index within each benchmark invocation rather than
reusing warmed state. Update _seed_fts5_corpus to distribute all
_LEXICAL_QUERIES using (i // 100) % len(_LEXICAL_QUERIES), and assert every
query returns non-empty results; make the hot benchmark explicitly perform its
warm-up instead of relying on collection order.
In `@config.example.yaml`:
- Around line 286-292: Update the default lexical regex patterns near the
configuration block in config.example.yaml (lines 286-292) so they match every
documented identifier, including H1-P4-XXX, MDR-AD002, CWE-79, and MS17-010;
alternatively remove any unsupported examples. Synchronize the corresponding
guide examples and coverage in docs/features/fts5_fast_path.md (lines 56-63),
ensuring each documented identifier is covered by the default patterns.
In `@docs/features/fts5_fast_path.md`:
- Around line 29-33: Update the migration behavior statement in the FTS5
fast-path guide to limit transparent fallback to queries using
search_method="auto" or dispatched through the router; do not imply that
explicitly forced search_method="fts5" queries fall back.
- Around line 102-108: The troubleshooting section should use the endpoint’s
actual `"no_results"` status and separate the diagnoses: verify router
classification independently, treat low min_hits as a fallback-threshold issue
after lexical dispatch, and use an explicit search_method="fts5" query only to
determine whether the index contains the chunk. Update the recommended actions
for each case without claiming forced FTS5 hits validate the regex pattern list.
- Around line 44-54: Correct the configuration reference in the FTS5 fast-path
documentation by changing the “five knobs” count to four, preserving the
existing fields and descriptions.
In `@README.md`:
- Around line 1263-1267: Update the FTS5 latency entry in README.md (lines
1263-1267) to identify the listed values as p95 targets on reference CI hardware
rather than typical latency. Also update docs/features/fts5_fast_path.md (lines
8-13) to describe those figures as hardware-dependent targets, not observed or
guaranteed latency; no other method latency descriptions need changing.
- Around line 1269-1270: Align the public search_knowledge signature and
documentation: use Literal["auto", "hybrid", "fts5"] so MCP metadata exposes an
enum, while preserving runtime validation. Update the corresponding
documentation in README.md lines 1269-1270 and docs/features/fts5_fast_path.md
lines 82-83; both sites require consistent Literal wording.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1daccb8c-15cf-47b2-aa96-431ffe7fc751
📒 Files selected for processing (7)
README.mdbench/test_bench_fts5_lexical.pyconfig.example.yamldocs/features/fts5_fast_path.mdmcp_server/__init__.pynpm/package.jsonpyproject.toml
| @pytest.fixture(scope="module") | ||
| def fts5_index(tmp_path_factory): | ||
| """Fresh FTS5 index seeded with 3865 rows, closed on teardown.""" | ||
| tmp = tmp_path_factory.mktemp("fts5_bench") | ||
| idx = Fts5LexicalIndex( | ||
| db_path=tmp / "fts5_index.db", | ||
| state_path=tmp / "fts5_migration.state", | ||
| ) | ||
| _seed_fts5_corpus(idx) | ||
| yield idx | ||
| idx.close() |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 \
'pytest-benchmark|benchmark\(|--benchmark|--random-order|xdist' \
.Repository: lyonzin/knowledge-rag
Length of output: 16435
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- benchmark file ---'
cat -n bench/test_bench_fts5_lexical.py
printf '%s\n' '--- related fixture and index definitions ---'
rg -n -C 5 \
'def fts5_index|def _seed_fts5_corpus|class Fts5LexicalIndex|def search|BENCH-001|cold|hot' \
bench tests src .Repository: lyonzin/knowledge-rag
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- FTS5 files ---'
fd -i 'fts5' .
printf '%s\n' '--- FTS5 index structure ---'
fts5_file="$(fd -i -t f 'fts5' . | head -n 1)"
if [ -n "${fts5_file}" ]; then
ast-grep outline "${fts5_file}"
printf '%s\n' '--- FTS5 index implementation ---'
rg -n -C 8 'class Fts5LexicalIndex|def __init__|def add_document|def search|sqlite3.connect|PRAGMA|close' "${fts5_file}"
fi
printf '%s\n' '--- benchmark identifiers and ordering references ---'
rg -n -C 3 'BENCH-00[12]|test_bench_fts5_lexical_(cold|hot)|pytest_collection|order' bench .github pyproject.toml pytest.ini setup.cfg 2>/dev/null || trueRepository: lyonzin/knowledge-rag
Length of output: 5578
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="mcp_server/fts5_index.py"
printf '%s\n' '--- FTS5 implementation outline ---'
ast-grep outline "$file"
printf '%s\n' '--- FTS5 lifecycle and search code ---'
rg -n -C 12 \
'class Fts5LexicalIndex|def __init__|def add_document|def search|sqlite3.connect|PRAGMA|def close' \
"$file"
printf '%s\n' '--- benchmark fixture data-shape check ---'
python3 - <<'PY'
queries = ["CVE-2021-4034", "MDR-AD002", "T1078.001", "CWE-79"]
counts = {q: 0 for q in queries}
for i in range(3865):
marker = queries[i % len(queries)] if i % 100 == 0 else ""
for query in queries:
if marker == query:
counts[query] += 1
print(counts)
PYRepository: lyonzin/knowledge-rag
Length of output: 9342
🌐 Web query:
pytest-benchmark official documentation benchmark fixture calibration repeated calls benchmark(function)
💡 Result:
The pytest-benchmark plugin provides a benchmark fixture that automatically calibrates and executes functions to produce accurate performance statistics [1][2]. Calibration and Repeated Calls In standard usage, when you call benchmark(function, *args, kwargs), the plugin automatically manages the number of repeated calls [3]. It performs a calibration phase to determine how many iterations are necessary to fit a chosen duration (based on your timer's resolution), ensuring the measurements are robust even for very fast code [3]. This process groups your function calls into "rounds," which are then used to calculate the statistics [3]. To control this process, you can use several command-line options and marker settings: - benchmark-min-time: Sets the minimum duration for the measurement rounds [4][5]. - benchmark-max-time: Sets the maximum duration for the measurement rounds [4][5]. - benchmark-min-rounds: Sets the minimum number of rounds to perform [4][5]. - benchmark-calibration-precision: Adjusts the precision used when calibrating the number of iterations [5]. Pedantic Mode If you require precise control over the number of iterations and rounds, avoiding automatic calibration, you can use the benchmark.pedantic method [6][7][8]: benchmark.pedantic(target, args=, kwargs=None, setup=None, rounds=1, warmup_rounds=0, iterations=1) In this mode: - iterations: The number of times the target function is called within each round [8]. - rounds: The total number of measurement rounds to execute [8]. - setup: An optional function to run before the target in each round [8]. Be cautious when manually setting iterations in pedantic mode, especially for very fast functions, as the default value of 1 may lead to inaccurate results if the execution time is below your timer's resolution [8]. Top results: [6][1][3][5][8]
Citations:
- 1: https://pytest-benchmark.readthedocs.io/
- 2: https://pytest-benchmark.readthedocs.io/en/stable/
- 3: https://pytest-benchmark.readthedocs.io/en/stable/calibration.html
- 4: https://pytest-benchmark.readthedocs.io/en/v5.2.3/usage.html
- 5: https://pytest-benchmark.readthedocs.io/en/latest/usage.html
- 6: https://pytest-benchmark.readthedocs.io/en/stable/usage.html
- 7: https://github.com/ionelmc/pytest-benchmark
- 8: https://pytest-benchmark.readthedocs.io/en/latest/pedantic.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import ast
path = Path("bench/test_bench_fts5_lexical.py")
tree = ast.parse(path.read_text())
fixture_scope = None
tests = []
for node in tree.body:
if isinstance(node, ast.FunctionDef):
if node.name == "fts5_index":
for decorator in node.decorator_list:
if isinstance(decorator, ast.Call) and getattr(decorator.func, "attr", None) == "fixture":
for kw in decorator.keywords:
if kw.arg == "scope":
fixture_scope = ast.literal_eval(kw.value)
if node.name.startswith("test_bench_fts5_lexical_"):
tests.append((node.name, node.lineno))
print({"fixture_scope": fixture_scope, "test_definition_order": tests})
queries = ["CVE-2021-4034", "MDR-AD002", "T1078.001", "CWE-79"]
corpus = []
for i in range(3865):
marker = queries[i % len(queries)] if i % 100 == 0 else ""
corpus.append(
f"document {i} kerberoast bloodhound impacket privilege escalation "
f"lateral movement notes on {marker}"
)
print({
query: sum(query in content for content in corpus)
for query in queries
})
PYRepository: lyonzin/knowledge-rag
Length of output: 356
Make the FTS5 benchmarks independent and seed every query.
- Create or reopen the index for each cold case. The module-scoped fixture and repeated
benchmark(run)calls measure warmed state. The hot benchmark also depends on collection order. - In
_seed_fts5_corpus,i % 100is always divisible by 4. OnlyCVE-2021-4034is inserted, so the other three queries return empty lists while the list assertion passes. Distribute markers with(i // 100) % len(_LEXICAL_QUERIES)and assert non-empty results.
🤖 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 `@bench/test_bench_fts5_lexical.py` around lines 54 - 64, Make the FTS5
benchmarks independent by changing fts5_index to function scope so each case
gets a fresh index, and ensure cold benchmark runs create or reopen the index
within each benchmark invocation rather than reusing warmed state. Update
_seed_fts5_corpus to distribute all _LEXICAL_QUERIES using (i // 100) %
len(_LEXICAL_QUERIES), and assert every query returns non-empty results; make
the hot benchmark explicitly perform its warm-up instead of relying on
collection order.
| result = benchmark(run) | ||
| assert isinstance(result, list) | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Require each benchmark query to return seeded hits.
isinstance(result, list) also passes for []. A tokenizer, escaping, or seed regression can therefore produce a fast but meaningless benchmark and still satisfy BENCH-001 or BENCH-002. Keep the type assertion and add a non-empty or expected-hit assertion for every parameterized query.
Minimal assertion change
assert isinstance(result, list)
+ assert result, f"Expected at least one seeded FTS5 hit for {query}"Also applies to: 95-96
🤖 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 `@bench/test_bench_fts5_lexical.py` around lines 78 - 80, Strengthen the
assertions in the parameterized benchmark tests around benchmark(run) so each
query must return a non-empty result or the expected seeded hits, while
retaining the existing isinstance(result, list) check. Apply the same assertion
update to both benchmark cases covered by the comment, including the occurrence
around the second referenced location.
| # First-match-wins regex patterns that classify a query as "lexical". | ||
| # Ordering matters (PRD OQ-2). Default set targets bug bounty + SOC/DFIR | ||
| # identifier vocabulary. Add custom project-specific codes below. | ||
| patterns: | ||
| - "[A-Z]{2,}-\\d+" # H1-P4-XXX, MDR-AD002, CWE-79, MS17-010 | ||
| - "CVE-\\d{4}-\\d+" # canonical CVE identifiers | ||
| - "^[a-f0-9]{32,64}$" # md5/sha1/sha256 file hashes |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Align the default lexical regex contract with its documented identifiers.
The shipped regex does not match H1-P4-XXX, MDR-AD002, or MS17-010, so those queries will not receive the documented lexical routing by default.
config.example.yaml#L286-L292: add patterns or remove unsupported examples.docs/features/fts5_fast_path.md#L56-L63: keep the guide synchronized and add coverage for every documented identifier.
📍 Affects 2 files
config.example.yaml#L286-L292(this comment)docs/features/fts5_fast_path.md#L56-L63
🤖 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 `@config.example.yaml` around lines 286 - 292, Update the default lexical regex
patterns near the configuration block in config.example.yaml (lines 286-292) so
they match every documented identifier, including H1-P4-XXX, MDR-AD002, CWE-79,
and MS17-010; alternatively remove any unsupported examples. Synchronize the
corresponding guide examples and coverage in docs/features/fts5_fast_path.md
(lines 56-63), ensuring each documented identifier is covered by the default
patterns.
| 2. Restart the server. On first start with the flag on, a background | ||
| daemon thread rebuilds the FTS5 index from your existing ChromaDB | ||
| corpus. Progress is checkpointed every 100 rows to | ||
| `data/fts5_migration.state`; queries during migration transparently | ||
| fall back to the hybrid pipeline. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Limit the migration fallback claim to automatic routing.
The guide later documents that forced search_method="fts5" returns a readiness error. Change “queries during migration transparently fall back” to specify search_method="auto" or router-dispatched queries.
🤖 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 `@docs/features/fts5_fast_path.md` around lines 29 - 33, Update the migration
behavior statement in the FTS5 fast-path guide to limit transparent fallback to
queries using search_method="auto" or dispatched through the router; do not
imply that explicitly forced search_method="fts5" queries fall back.
| ## Configuration Reference | ||
|
|
||
| Full field docs live in `config.example.yaml` under | ||
| `search.lexical_fast_path`. The five knobs: | ||
|
|
||
| | Field | Default | What it controls | | ||
| | ---------------- | ------- | ------------------------------------------------------------- | | ||
| | `enabled` | `false` | Master toggle. Off → zero runtime cost, no FTS5 index open. | | ||
| | `min_hits` | `3` | Minimum FTS5 hits to skip hybrid fallback (recall safety). | | ||
| | `rerank_enabled` | `false` | Layer cross-encoder rerank on FTS5 hits. ADR-003 keeps off. | | ||
| | `patterns` | *(see)* | First-match-wins regex list that classifies "lexical". | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the configuration field count.
The guide says “five knobs” but lists four fields: enabled, min_hits, rerank_enabled, and patterns. Add the missing field or change the count to four.
🤖 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 `@docs/features/fts5_fast_path.md` around lines 44 - 54, Correct the
configuration reference in the FTS5 fast-path documentation by changing the
“five knobs” count to four, preserving the existing fields and descriptions.
| **2. Lexical query returns `NO_RESULTS` but hybrid finds the chunk.** | ||
| Router probably misclassified. Confirm with an explicit | ||
| `search_method="fts5"` call — if that returns hits, the pattern list is | ||
| right and `min_hits` may be too aggressive; lower to `1`. If the forced | ||
| `"fts5"` call also empty, the FTS5 index does not contain that chunk — run | ||
| `python scripts/build_fts5_index.py --data-dir <path> --force` to rebuild | ||
| from ChromaDB. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the troubleshooting decision tree.
An explicit search_method="fts5" call bypasses QueryRouter, so returned hits cannot prove that the regex list is correct. min_hits only controls fallback after lexical dispatch. The supplied endpoint returns status "no_results", not NO_RESULTS. Rewrite this section to distinguish router misclassification, low-hit fallback, and an empty index.
🤖 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 `@docs/features/fts5_fast_path.md` around lines 102 - 108, The troubleshooting
section should use the endpoint’s actual `"no_results"` status and separate the
diagnoses: verify router classification independently, treat low min_hits as a
fallback-threshold issue after lexical dispatch, and use an explicit
search_method="fts5" query only to determine whether the index contains the
chunk. Update the recommended actions for each case without claiming forced FTS5
hits validate the regex pattern list.
| | Method | Path | Latency (typical) | Best For | | ||
| |-------------------------|-------------------|-----------------------|----------------------------------------------| | ||
| | `auto` **(default)** | Router decides | Router adds ~0.1ms | Mixed workloads — safe default | | ||
| | `hybrid` | BM25 + semantic | 50-150ms (with rerank)| Prose queries, "how does X work", exploration| | ||
| | `fts5` | SQLite FTS5 only | <10ms cold / <2ms hot | CVE / MITRE / CWE / hash lookups | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)(README\.md|fts5_fast_path\.md)$|benchmark|perf|latency|fts5' | head -200
printf '%s\n' '--- referenced documentation ---'
sed -n '1248,1275p' README.md
sed -n '1,80p' docs/features/fts5_fast_path.md
printf '%s\n' '--- benchmark and CI references ---'
rg -n -i --glob '!node_modules' --glob '!dist' \
'cold|hot|latency|perf.?gate|performance|benchmark|3865|10ms|2ms|50-150ms|under ~' \
.github . 2>/dev/null | head -300Repository: lyonzin/knowledge-rag
Length of output: 37927
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- FTS5 benchmark ---'
cat -n bench/test_bench_fts5_lexical.py
printf '%s\n' '--- performance-gate workflow ---'
sed -n '470,530p' .github/workflows/quality-gate.yml
sed -n '530,590p' .github/workflows/quality-gate.yml
printf '%s\n' '--- ADR references and documentation wording ---'
rg -n -i 'BENCH-00[12]|10ms|2ms|target|measur|gate|CI|hardware|typical|guarante' \
docs/adr README.md docs/features/fts5_fast_path.md \
--glob '0004*' --glob '0009*' --glob '*.md' | head -300
printf '%s\n' '--- relevant README release notes ---'
sed -n '1435,1468p' README.mdRepository: lyonzin/knowledge-rag
Length of output: 49515
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- FTS5 decision records and gate procedure ---'
find docs .compozy -type f \( -iname '*004*' -o -iname '*009*' -o -iname '*bench*v4*' -o -iname '*fts5*' \) -print 2>/dev/null | sort
rg -n -i 'G1|G2|BENCH-001|BENCH-002|p95|15.?x|2%|10ms|2ms|assert|threshold' \
docs .compozy scripts bench .github/workflows \
--glob '*.md' --glob '*.py' --glob '*.yml' | head -400
printf '%s\n' '--- regression checker semantics ---'
cat -n scripts/check_perf_regression.py | sed -n '1,150p'Repository: lyonzin/knowledge-rag
Length of output: 10069
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
import ast
import re
bench = Path("bench/test_bench_fts5_lexical.py").read_text(encoding="utf-8")
checker = Path("scripts/check_perf_regression.py").read_text(encoding="utf-8")
workflow = Path(".github/workflows/quality-gate.yml").read_text(encoding="utf-8")
tree = ast.parse(bench)
tests = [
node for node in ast.walk(tree)
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef))
and node.name.startswith("test_bench_fts5_lexical_")
]
print("FTS5 benchmark tests:", [node.name for node in tests])
print("Target assertions in FTS5 benchmark:",
sum(isinstance(node, ast.Assert) for node in ast.walk(tree)))
print("FTS5 p95 target declarations:",
re.findall(r"Target:\s*p95\s*<=\s*[^.\n]+", bench))
print("Checker compares medians:", '"median"' in checker and '"mean"' not in checker)
print("Checker intersects benchmark names:",
"set(master) & set(branch)" in checker)
print("Workflow runs FTS5 benchmark explicitly:",
"test_bench_fts5_lexical.py" in workflow)
PYRepository: lyonzin/knowledge-rag
Length of output: 521
Label FTS5 latency values as targets, not typical latency.
The benchmark does not assert the p95 limits. The CI checker compares only median results shared with master, so it does not enforce these new FTS5 targets.
README.md#L1263-L1267: identify the values as p95 targets on reference CI hardware.docs/features/fts5_fast_path.md#L8-L13: describe them as hardware-dependent targets, not observed or guaranteed latency.
📍 Affects 2 files
README.md#L1263-L1267(this comment)docs/features/fts5_fast_path.md#L8-L13
🤖 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 1263 - 1267, Update the FTS5 latency entry in
README.md (lines 1263-1267) to identify the listed values as p95 targets on
reference CI hardware rather than typical latency. Also update
docs/features/fts5_fast_path.md (lines 8-13) to describe those figures as
hardware-dependent targets, not observed or guaranteed latency; no other method
latency descriptions need changing.
| The `search_knowledge` MCP tool exposes `search_method: Literal["auto", | ||
| "hybrid", "fts5"] = "auto"` (ADR-006, additive on the LEI 1 contract). The |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)(README\.md|server\.py|fts5_fast_path\.md)$|mcp|search' | head -200
printf '%s\n' '--- relevant symbols and documentation ---'
rg -n -C 4 'search_knowledge|search_method|Literal\["auto"|hybrid.*fts5|fts5.*hybrid' README.md docs mcp_server 2>/dev/null | head -300
printf '%s\n' '--- server outline ---'
if command -v ast-grep >/dev/null 2>&1 && [ -f mcp_server/server.py ]; then
ast-grep outline mcp_server/server.py
fiRepository: lyonzin/knowledge-rag
Length of output: 46732
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- imports and public tool implementation ---'
sed -n '1,80p' mcp_server/server.py
sed -n '3725,3875p' mcp_server/server.py
printf '%s\n' '--- all search_method declarations and validation ---'
rg -n -C 5 'search_method|Literal' mcp_server tests docs README.md | head -500
printf '%s\n' '--- relevant tests ---'
sed -n '1,260p' tests/test_search_method_override.py
rg -n -C 8 'signature|search_knowledge|schema|search_method|Literal' tests/test_backwards_compat.py tests | head -500
printf '%s\n' '--- dependency/configuration references ---'
rg -n -C 3 'fastmcp|mcp\[|mcpserver|MCPServer|tool\(' pyproject.toml requirements* setup* mcp_server 2>/dev/null | head -300Repository: lyonzin/knowledge-rag
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- public signature and decorator definitions ---'
sed -n '3680,3760p' mcp_server/server.py
sed -n '1,180p' mcp_server/ratelimit.py
sed -n '1,180p' mcp_server/metrics.py
printf '%s\n' '--- focused compatibility and override tests ---'
if [ -f tests/test_backwards_compat.py ]; then
rg -n -C 12 'search_knowledge|signature|annotation|schema|tool' tests/test_backwards_compat.py
fi
sed -n '1,260p' tests/test_search_method_override.py
printf '%s\n' '--- project dependency declarations ---'
for f in pyproject.toml requirements.txt requirements-dev.txt setup.py setup.cfg; do
if [ -f "$f" ]; then
echo "### $f"
rg -n -C 5 'mcp|fastmcp|typing|python' "$f"
fi
done
printf '%s\n' '--- ADR-006 and related documentation ---'
fd -i 'ADR-006' . --type f -x sh -c 'echo "### $1"; cat -n "$1"' sh {} \; 2>/dev/null | head -260Repository: lyonzin/knowledge-rag
Length of output: 29689
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import ast
from pathlib import Path
server = ast.parse(Path("mcp_server/server.py").read_text())
public = next(
node for node in ast.walk(server)
if isinstance(node, ast.FunctionDef) and node.name == "search_knowledge"
)
param = next(arg for arg in public.args.args if arg.arg == "search_method")
annotation = ast.unparse(param.annotation)
default = ast.unparse(public.args.defaults[-1])
membership = []
for node in ast.walk(public):
if isinstance(node, ast.Compare) and isinstance(node.ops[0], ast.In):
left = ast.unparse(node.left)
right = ast.unparse(node.comparators[0])
if left == "search_method":
membership.append(right)
docs = {}
for filename in ("README.md", "docs/features/fts5_fast_path.md"):
text = Path(filename).read_text()
docs[filename] = {
"has_literal_claim": 'Literal["auto", "hybrid", "fts5"]' in text,
"has_all_values": all(f'"{x}"' in text for x in ("auto", "hybrid", "fts5")),
}
print({
"public_search_method_annotation": annotation,
"public_search_method_default": default,
"public_runtime_membership_checks": membership,
"documentation": docs,
"typing_literal_imported": any(
isinstance(node, ast.ImportFrom)
and node.module == "typing"
and any(alias.name == "Literal" for alias in node.names)
for node in server.body
),
})
PYRepository: lyonzin/knowledge-rag
Length of output: 492
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import ast
from pathlib import Path
server = ast.parse(Path("mcp_server/server.py").read_text())
public = next(
node for node in ast.walk(server)
if isinstance(node, ast.FunctionDef) and node.name == "search_knowledge"
)
param = next(arg for arg in public.args.args if arg.arg == "search_method")
default = ast.unparse(public.args.defaults[-1])
checks = []
for node in ast.walk(public):
if (
isinstance(node, ast.Compare)
and len(node.ops) == 1
and isinstance(node.ops[0], (ast.In, ast.NotIn))
and ast.unparse(node.left) == "search_method"
):
checks.append({
"operator": type(node.ops[0]).__name__,
"values": ast.unparse(node.comparators[0]),
})
def normalized_literal_claim(path):
text = Path(path).read_text()
normalized = " ".join(text.split())
return 'search_method: Literal["auto", "hybrid", "fts5"] = "auto"' in normalized
print({
"public_annotation": ast.unparse(param.annotation),
"public_default": default,
"runtime_checks": checks,
"typing_literal_imported": any(
isinstance(node, ast.ImportFrom)
and node.module == "typing"
and any(alias.name == "Literal" for alias in node.names)
for node in server.body
),
"normalized_doc_literal_claims": {
path: normalized_literal_claim(path)
for path in ("README.md", "docs/features/fts5_fast_path.md")
},
})
PY
printf '%s\n' '--- MCP schema/test references in repository ---'
rg -n -C 5 'inputSchema|input_schema|json_schema|tool metadata|MCPServer|mcp\.list_tools|list_tools|search_method' \
tests mcp_server docs README.md | rg -v 'README.md:14[6-9][0-9]' | head -350Repository: lyonzin/knowledge-rag
Length of output: 38505
🌐 Web query:
Python MCP SDK 2.0 MCPServer tool decorator function annotations JSON schema Literal enum input schema
💡 Result:
In the Python MCP SDK 2.0 (often used with the MCPServer or FastMCP interface), the @mcp.tool decorator automatically generates a JSON Schema for tool arguments by inspecting Python type hints, Pydantic Field configurations, and standard library primitives [1][2][3]. Key behaviors for defining input schemas include: 1. Automatic Schema Inference: The SDK introspects function signatures to build a Pydantic model for validation, which is then converted into a JSON Schema for the MCP host [2]. Name, description, and parameter types are derived directly from the function definition [3]. 2. Enums and Literal Types: - Literal: Using Literal["a", "b"] in a function signature is the recommended way to represent enums [1][4]. This produces an inline enum schema in the generated JSON, which is widely compatible across different MCP clients [4]. - Enum Classes: While Python Enum classes (e.g., StrEnum) are supported by Pydantic, they are often serialized as JSON Schema references ($ref) [5][4]. Some MCP clients struggle with these $ref definitions, leading to issues where values are interpreted as null or cause crashes [5][4]. If you encounter compatibility issues with Enum classes, replacing them with inline Literal[...] type hints is the established workaround [4]. 3. Annotations and Field Metadata: - Use typing.Annotated combined with pydantic.Field to customize the schema beyond default type inference [1][2]. - Field(description="..."): Adds a human-readable description for LLMs [1]. - Field(ge=N, le=M): Enforces numeric constraints (minimum/maximum) that are automatically included in the JSON Schema [1]. 4. Decorator Customization: Everything inferred by the SDK can be overridden using arguments in the @mcp.tool decorator, such as title, description, or custom annotations [1][6]. For example: @mcp.tool(title="Tool Title") def my_tool(param: Annotated[str, Field(description="Description")]) -> str:... These mechanisms allow you to define complex, validated input interfaces without manually writing JSON schema definitions [3].
Citations:
- 1: https://py.sdk.modelcontextprotocol.io/v2/servers/tools/
- 2: https://deepwiki.com/modelcontextprotocol/python-sdk/2.4-function-metadata-and-schema-generation
- 3: https://py.sdk.modelcontextprotocol.io/v2/get-started/first-steps/
- 4: [BUG] MCP tool parameters using $ref enum types are serialized as null anthropics/claude-code#58794
- 5: Tool with argumemnt annotated as Enum string requires input to be explicitly quoted? modelcontextprotocol/python-sdk#1373
- 6: https://github.com/modelcontextprotocol/python-sdk/blob/main/src/mcp/server/mcpserver/server.py
Align search_method documentation with the MCP schema.
search_knowledge uses search_method: str = "auto" and validates "auto", "hybrid", and "fts5" at runtime. Both documents claim a Literal signature, so generated MCP metadata exposes a generic string instead of an enum. Use Literal["auto", "hybrid", "fts5"] in the public signature, or document the accepted strings without claiming a Literal type.
📍 Affects 2 files
README.md#L1269-L1270(this comment)docs/features/fts5_fast_path.md#L82-L83
🤖 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 1269 - 1270, Align the public search_knowledge
signature and documentation: use Literal["auto", "hybrid", "fts5"] so MCP
metadata exposes an enum, while preserving runtime validation. Update the
corresponding documentation in README.md lines 1269-1270 and
docs/features/fts5_fast_path.md lines 82-83; both sites require consistent
Literal wording.
lyonzin
changed the title
1 participant
Summary
v4.8.2 - FTS5 Lexical Fast-Path opt-in release (default OFF, LEI 1 preserved).
Bundles Fases 1-5 of the FTS5 Lexical Fast-Path plan as a conservative patch release.
search.lexical_fast_path.enabledremainsfalseby default - users who do not touchconfig.yamlkeep v4.8.1 behaviour byte-for-byte.What ships
pyproject.toml+mcp_server/__init__.py+npm/package.json)bench/test_bench_fts5_lexical.py- BENCH-001 cold (p95 <=10ms target) + BENCH-002 hot (p95 <=2ms target) against 3865-row FTS5 corpusdocs/features/fts5_fast_path.md(6.5K) - overview, quick-start, config reference, troubleshooting, when-to-use## Configurationsection: new### Search Method (v4.8.2+)subsection with 3-method comparison table## v4.8.2with LEI 1 + ADR-004 + ADR-009 referencesPath C - Deferred gate (ADR-009 emergent decision)
The ADR-004 v4.9.0 gate (G1 >=15x lexical / G2 <=2% hybrid regression) was not adjudicated locally because the release-authoring workstation cannot fairly baseline against CI hardware (see ADR-004 Risks). The gate is deferred to the CI perf-gate adjudication documented in ADR-009. The v4.9.0 default flip PR follows once G1 + G2 both pass on CI hardware.
What does NOT ship
enabled: falsepreserves v4.8.1 behaviorsearch_knowledgesignature addedsearch_methodin v4.8.2 (PR feat: wire FTS5 fast-path (default OFF) #160), unchanged hereLEI 1 preserved
Zero change to the 13 frozen MCP tools. Default OFF preserves v4.8.1 byte-for-byte.
Rollback
git revert <sha>- feature is opt-in default-off; revert is safe. Standalonescripts/build_fts5_index.py(from PR #167) continues to work for manual FTS5 rebuild.Summary by CodeRabbit
New Features
auto,hybrid, andfts5search method options.Documentation
Tests
Chores
Greptile Summary
The PR releases version 4.8.2 and documents the opt-in FTS5 lexical fast path while adding its performance harness. The version and default-off example configuration are consistent, but the benchmark does not produce a valid cold measurement and the advertised hybrid regression gate is not executable as shipped.
Confidence Score: 3/5
The release should not merge until the cold benchmark isolates cache state and the advertised feature-on hybrid regression gate is made executable.
The new harness measures warmed SQLite searches as cold and does not implement the enabled-versus-disabled hybrid comparison required to adjudicate the future default flip.
Files Needing Attention: bench/test_bench_fts5_lexical.py, docs/features/fts5_fast_path.md, README.md
Important Files Changed
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "release: v4.8.2 - FTS5 Lexical Fast-Path..." | Re-trigger Greptile