fix: blend hybrid retriever scores with weighted RRF instead of batch-max normalization

[GutamaKev]

Summary

HybridRetriever.retrieve() in rag/retriever/hybrid.py normalized keyword scores by dividing by the max BM25 score in the current result batch, so a chunk that simply repeats query terms (e.g. a tech-stack list) could inflate its own score to 1.0 and outrank a genuinely relevant chunk. It now blends vector and keyword results with weighted Reciprocal Rank Fusion (RRF) instead, so no single outlier score can dominate the batch.

Issue

Closes #24

Changes

Root cause: BM25 raw scores are unbounded and grow with in-chunk term repetition. The old normalization divided each keyword score by the max score in the batch, so whichever chunk repeated query terms the most became the denominator, guaranteeing itself a normalized score of 1.0 regardless of actual relevance. A secondary bug compounded it: _tokenize() only split on whitespace, so "Python," and "React." (trailing punctuation) never matched the query tokens "python"/"react", suppressing genuinely relevant chunks' scores further.

• rag/retriever/hybrid.py — replaced batch-max normalization with weighted RRF (score = vector_weight/(k+vector_rank) + keyword_weight/(k+keyword_rank), k=10); added a deterministic _rank_map() helper (ties broken by chunk id, since Python's set/dict iteration order for strings is hash-randomized); chunks absent from one side's results get that side's worst rank instead of a raw score of 0; changed min_score default from 0.3 to 0.0 since RRF scores are small and always positive, not 0-1 percentages
• rag/retriever/keyword_search.py — _tokenize() now strips leading/trailing punctuation per token, so "Python," and "React." match, while preserving tokens like "c++"/"node.js"
• rag/retriever/vector_store.py — added a missing return type annotation on get_collection(), required by the mypy pre-commit hook on files touched here
• tests/unit/test_hybrid_retriever.py — removed the xfail from the issue Hybrid retriever over-weights keyword results when query contains technology names #24 regression test (now genuinely passes); added a test confirming an exact rare-term keyword match still beats a near-tied vector competitor; added a determinism test for tied scores
• PLAN.md — solution plan with root cause, approach, and risks

Testing

• Unit tests pass (make test-unit) — test_hybrid_retriever.py (3/3) and test_keyword_search.py (19/20; the 1 failure is pre-existing and unrelated, see Notes)
• New/updated tests cover the changes

Reproduce the original bug (before the fix):

Run on main:

from unittest.mock import Mock
from rag.retriever.hybrid import HybridRetriever
from rag.retriever.keyword_search import KeywordSearcher

chunks = [
    {"id": "resume_1", "text": "Led backend architecture for a fintech platform, owning the "
                                "migration from a monolith to services built in Python, with "
                                "a customer dashboard built in React."},
    {"id": "resume_2", "text": "Managed a team of four engineers and ran quarterly planning "
                                "for the platform roadmap, prioritizing reliability work over "
                                "new features."},
    {"id": "readme_1", "text": "Tech stack Python React Python React Docker Python React "
                                "PostgreSQL Python React"},
    {"id": "readme_2", "text": "This project has no license and is not accepting "
                                "contributions at this time"},
]
vector_scores = {"resume_1": 1.0, "resume_2": 0.8, "readme_1": 0.85, "readme_2": 0.5}
# resume_1 is the genuinely relevant chunk; readme_1 just repeats tech names

store = Mock()
store.get_collection.return_value.get.return_value = {"ids": [], "documents": [], "metadatas": []}
store.query.return_value = [
    {"id": c["id"], "text": c["text"], "metadata": {}, "score": vector_scores[c["id"]]} for c in chunks
]
ks = KeywordSearcher()
ks.index(chunks)
retriever = HybridRetriever(store, ks)

results = retriever.retrieve(
    query="What is this candidate's Python and React experience?",
    profile_id="test", query_embedding=[1.0, 0.0, 0.0], max_chunks=10, min_score=0.0,
)
print(results[0]["id"])

(Note: this needs at least 4 chunks with mixed term-document-frequencies to reproduce —
with only 2 chunks, "python"/"react" each land in exactly 1 of 2 documents, and BM25's IDF
for that ratio computes to exactly log(1.5) - log(1.5) = 0, masking the bug entirely.)

Observe: prints readme_1 — the keyword-stuffed, irrelevant chunk outranks the genuinely relevant one.

Verify the fix (on this branch):

Run the same snippet on fix/24-hybrid-retriever-keyword-weighting.
Expected: prints resume_1 — the genuinely relevant chunk now wins.

Or just run: pytest tests/unit/test_hybrid_retriever.py -v

Notes for Reviewers

make check/make test-unit report pre-existing, unrelated failures (53 failing unit tests across other modules, plus repo-wide ruff/black/mypy debt) — confirmed present on main before this change (via git stash) and unaffected by it. RRF k=10 was validated against this reproduction case at k=60/20/10/5; it should be re-checked against real profile data before this ships. Considered downweighting terms common across a profile's own chunks instead of RRF, but verified numerically it wouldn't have fixed this case — the defect is repetition within one chunk, not commonality across chunks (details in PLAN.md).