Skip to content

Fix/24 hybrid retriever keyword weighting - #764

Open
GarabKDorji wants to merge 7 commits into
ascherj:mainfrom
GarabKDorji:fix/24-hybrid-retriever-keyword-weighting
Open

Fix/24 hybrid retriever keyword weighting#764
GarabKDorji wants to merge 7 commits into
ascherj:mainfrom
GarabKDorji:fix/24-hybrid-retriever-keyword-weighting

Conversation

@GarabKDorji

Copy link
Copy Markdown

Summary

This PR updates the hybrid retriever’s scoring formula so that a strong keyword match cannot independently promote a chunk with little or no semantic relevance. The keyword score now acts as a controlled boost to the vector score instead of contributing as a separate additive term. This keeps semantic relevance as the foundation of the final ranking while still rewarding chunks found by both vector and keyword search.

Issue

Closes #24

Changes

-Updated the hybrid scoring formula in rag/retriever/hybrid.py.
-Changed the keyword contribution from an independent additive term to a contribution scaled by the vector score.
-Prevented keyword-only chunks with no semantic relevance from receiving a positive blended score.
-Preserved the existing default weights of 0.7 for vector search and 0.3 for keyword search.
-Added or updated regression tests in tests/unit/test_hybrid.py to cover the incorrect ranking behavior.
-Updated JOURNAL.md with implementation details, testing results, and investigation findings.

Testing

  • Unit tests pass (make test-unit)
  • Integration tests pass (make test-integration)
  • Linter passes (make lint)
  • Type checker passes (make typecheck)
  • New/updated tests cover the changes
    tests/unit/test_hybrid.py — 6 passed, 0 failed.
    The two reproduction tests committed in Week 8 now pass with no change to their data:
    -test_semantic_vector_chunk_should_outrank_keyword_only_chunk
    -test_keyword_score_boosts_weaker_vector_chunk_above_stronger_vector_chunk
    The four pre-existing tests are unaffected and the both-searches boost still produces the maximum score of 1.0, and the min_score, empty-result and max_chunks paths are unchanged.
    Pre-existing failures

Before making my changes:

  • make check reported 182 existing lint errors.
  • make test-unit reported 55 failed tests, 379 passed tests, and 5 warnings.
    The type checker reported missing return-type annotations in:
    • rag/retriever/vector_store.py
    • rag/retriever/keyword_search.py

After making my changes, I reran the relevant checks and confirmed that all six targeted hybrid retriever tests pass. My changes did not introduce additional failures. The remaining project-wide failures are pre-existing and unrelated to this contribution.

Screenshots / Demo

Notes for Reviewers

  1. keyword_searcher.index() does not appear to be called anywhere in the current application flow. retrieve() fetches all_chunks but does not use the returned value, and _get_all_chunks() states that the chunks are intended for keyword indexing. Without an initialized index, KeywordSearcher.search() may return an empty result list, which would make the keyword contribution inactive outside mocked tests. I left this behavior unchanged because properly restoring indexing may require a separate decision about caching or indexing during ingestion rather than rebuilding the BM25 index for every query.
  2. The issue description mentions equal vector and keyword weights, but the current implementation uses default weights of 0.7 for vector search and 0.3 for keyword search. This PR preserves the existing defaults and focuses only on correcting how the scores are combined.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hybrid retriever over-weights keyword results when query contains technology names

1 participant