feat: add optional cross-encoder rerank in FTS5 fast-path - #166
Merged
Conversation
Wires config.fts5_rerank_enabled (default False per ADR-003) to trigger cross-encoder rerank on FTS5 hit results when the toggle is on. Default behavior unchanged - fast-path stays lean (1-5ms) unless operator opts in to trade latency for ranking quality. - New private helper _rerank_fts5_results in KnowledgeOrchestrator - Conditional wire in _run_fts5_search - 4 tests (UT-062, UT-063, IT-024, IT-025) covering both toggle states - Test count baseline 304 -> 308 (+4) Fase 3.5 of FTS5 Lexical Fast-Path plan. Zero LEI 1 impact - internal helper only, no MCP tool signature change.
📝 WalkthroughWalkthroughThe FTS5 fast path now supports optional cross-encoder reranking. Disabled reranking records a skip metric. Enabled reranking populates scores, reorders results, and preserves FTS5 metadata. Tests cover both modes, ordering, and latency. ChangesFTS5 reranking
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant FTS5FastPath
participant _rerank_fts5_results
participant CrossEncoderReranker
FTS5FastPath->>_rerank_fts5_results: pass formatted FTS5 results
_rerank_fts5_results->>CrossEncoderReranker: rerank mapped documents
CrossEncoderReranker-->>_rerank_fts5_results: return scores and ordering
_rerank_fts5_results-->>FTS5FastPath: return restored FTS5 results
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
2455
to
+2457
| formatted = self._format_fts5_results(hits, max_results, category_filter) | ||
| if config.fts5_rerank_enabled and formatted: | ||
| formatted = self._rerank_fts5_results(query_text, formatted, max_results) |
There was a problem hiding this comment.
When a cross-encoder-preferred result ranks below max_results in the wider FTS5 candidate pool, _format_fts5_results discards it before reranking, so it cannot be promoted into the returned results.
Prompt To Fix With AI
This is a comment left during a code review.
Path: mcp_server/server.py
Line: 2455-2457
Comment:
**Reranker receives truncated candidates**
When a cross-encoder-preferred result ranks below `max_results` in the wider FTS5 candidate pool, `_format_fts5_results` discards it before reranking, so it cannot be promoted into the returned results.
---
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!
Comment on lines
+2456
to
+2459
| if config.fts5_rerank_enabled and formatted: | ||
| formatted = self._rerank_fts5_results(query_text, formatted, max_results) | ||
| else: | ||
| metrics.inc(FAST_PATH_RERANK_SKIPPED_TOTAL) |
There was a problem hiding this comment.
If fts5_rerank_enabled is true while the independent global reranker setting is false, this branch treats reranking as applied even though CrossEncoderReranker.rerank returns the unchanged documents, leaving scores null and failing to increment the skipped counter.
Prompt To Fix With AI
This is a comment left during a code review.
Path: mcp_server/server.py
Line: 2456-2459
Comment:
**Disabled reranker becomes silent no-op**
If `fts5_rerank_enabled` is true while the independent global reranker setting is false, this branch treats reranking as applied even though `CrossEncoderReranker.rerank` returns the unchanged documents, leaving scores null and failing to increment the skipped counter.
---
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: 1
🤖 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 `@mcp_server/server.py`:
- Around line 2456-2459: Establish one consistent contract for FTS5 reranking
across server.py lines 2439-2443 and 2456-2459: either let fts5_rerank_enabled
override reranker_enabled, or require both flags and increment
FAST_PATH_RERANK_SKIPPED_TOTAL when reranking is unavailable. Document the
chosen contract in README.md line 1417 and add mixed-flag coverage in
tests/test_search.py lines 932-933.
🪄 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: 5aca34ca-aea1-4eff-ac5c-d4b4df971be0
📒 Files selected for processing (4)
.github/test-count-baseline.txtREADME.mdmcp_server/server.pytests/test_search.py
Comment on lines
+2456
to
+2459
| if config.fts5_rerank_enabled and formatted: | ||
| formatted = self._rerank_fts5_results(query_text, formatted, max_results) | ||
| else: | ||
| metrics.inc(FAST_PATH_RERANK_SKIPPED_TOTAL) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 \
'fts5_rerank_enabled|reranker_enabled' \
-g '*.py' -g '*.yaml' -g '*.yml' -g '*.md' .Repository: lyonzin/knowledge-rag
Length of output: 17913
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- CrossEncoderReranker and FTS5 helper ---'
sed -n '730,810p' mcp_server/server.py
sed -n '2380,2485p' mcp_server/server.py
printf '%s\n' '--- FTS5 rerank tests and metric assertions ---'
sed -n '880,1025p' tests/test_search.py
rg -n -C 5 'FAST_PATH_RERANK_SKIPPED_TOTAL|rerank_skipped|reranker_score|_rerank_fts5_results' \
mcp_server tests
printf '%s\n' '--- Configuration documentation and defaults ---'
sed -n '620,675p' mcp_server/config.py
sed -n '890,915p' mcp_server/config.py
sed -n '1000,1025p' mcp_server/config.pyRepository: lyonzin/knowledge-rag
Length of output: 42081
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
source = Path("mcp_server/server.py").read_text()
start = source.index("class CrossEncoderReranker")
rerank_start = source.index("def rerank", start)
rerank_end = source.index("\n def ", rerank_start + 1)
rerank_body = source[rerank_start:rerank_end]
fts_start = source.index("def _run_fts5_search")
fts_body = source[fts_start:source.index("\n def ", fts_start + 1)]
required = [
"if not documents or not config.reranker_enabled:",
"if config.fts5_rerank_enabled and formatted:",
"else:\n metrics.inc(FAST_PATH_RERANK_SKIPPED_TOTAL)",
]
for text in required:
assert text in source, f"missing source condition: {text!r}"
print("rerank early-return:", "config.reranker_enabled" in rerank_body)
print("fts5 caller gate:", "config.fts5_rerank_enabled and formatted" in fts_body)
print("mixed flags (fts5=True, shared=False): caller enters =", True)
print("mixed flags (fts5=True, shared=False): reranker returns unchanged =", True)
print("mixed flags (fts5=True, shared=False): caller skip metric branch =", False)
PYRepository: lyonzin/knowledge-rag
Length of output: 414
Define one configuration contract for FTS5 reranking.
When fts5_rerank_enabled=True and reranker_enabled=False, CrossEncoderReranker.rerank returns the original order with reranker_score=None. FAST_PATH_RERANK_SKIPPED_TOTAL does not increment. Either make the FTS5 flag override the shared gate or require both flags and record the skip. Update both documentation sites and add mixed-flag coverage.
📍 Affects 3 files
mcp_server/server.py#L2456-L2459(this comment)mcp_server/server.py#L2439-L2443README.md#L1417-L1417tests/test_search.py#L932-L933
🤖 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 `@mcp_server/server.py` around lines 2456 - 2459, Establish one consistent
contract for FTS5 reranking across server.py lines 2439-2443 and 2456-2459:
either let fts5_rerank_enabled override reranker_enabled, or require both flags
and increment FAST_PATH_RERANK_SKIPPED_TOTAL when reranking is unavailable.
Document the chosen contract in README.md line 1417 and add mixed-flag coverage
in tests/test_search.py lines 932-933.
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.
Summary
Adds an opt-in cross-encoder rerank pass on FTS5 fast-path results, gated by
config.fts5_rerank_enabled(defaultFalseper ADR-003).Fase 3.5 of FTS5 Lexical Fast-Path plan (workflow artifacts local-only, gitignored).
What ships
KnowledgeOrchestrator._rerank_fts5_results- applies existing cross-encoder to FTS5 hit list when the toggle is on_run_fts5_search: rerank only fires whenconfig.fts5_rerank_enabled=TrueAND we have formatted resultstests/test_search.py:What does NOT ship
search_knowledgesignature intactfts5_rerank_enabledalready introduced in Task 03 (PR feat: wire FTS5 fast-path (default OFF) #160)LEI 1 preserved
Zero change to the 13 frozen MCP tools.
mcp_server/server.pychange is internal helper + conditional call inside existing_run_fts5_search.Local validation
ruff check+ruff format --check= cleanpython scripts/check_api_surface.py --check= OK (no breaking changes)MCPServerimport failure (Python 3.14 local, same story as PR feat: add FTS5 lexical index module (opt-in, unused) #158/feat: add query lexical/semantic classifier (opt-in, unused) #159/feat: wire FTS5 fast-path (default OFF) #160). CI env with pinned deps validates.Rollback
git revert <sha>- feature is opt-in default-off, revert is safe.Summary by CodeRabbit
New Features
Documentation
Tests
Greptile Summary
Adds an optional cross-encoder pass to FTS5 fast-path results while preserving the default-off behavior.
_run_fts5_searchbehindfts5_rerank_enabled.Confidence Score: 3/5
The PR should be fixed before merging because enabled FTS5 reranking discards surplus candidates before scoring and can silently do nothing under a valid configuration.
The new path cannot promote results outside the raw FTS5 top limit, and its independent toggle reports reranking as applied even when the global reranker disables the operation.
Files Needing Attention: mcp_server/server.py, tests/test_search.py
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[FTS5 search: fetch 3x candidates] --> B[Format and truncate to max_results] B --> C{FTS5 rerank enabled?} C -->|Yes| D[Cross-encoder rerank] C -->|No| E[Increment skipped counter] D --> F[Expand adjacent chunks] E --> F F --> G[Return FTS5 results]Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat: add optional cross-encoder rerank ..." | Re-trigger Greptile