Skip to content

feat(context): gate reranking behind retrieval config#436

Open
Steve-too wants to merge 3 commits into
vouchdev:testfrom
Steve-too:feat/context-rerank-config
Open

feat(context): gate reranking behind retrieval config#436
Steve-too wants to merge 3 commits into
vouchdev:testfrom
Steve-too:feat/context-rerank-config

Conversation

@Steve-too

@Steve-too Steve-too commented Jul 7, 2026

Copy link
Copy Markdown

Summary

  • add retrieval.rerank.enabled / retrieval.rerank.top_k config for kb.context
  • rerank only the already scoped hybrid retrieval window, preserving membership
  • gracefully fall back to fused order when reranker extras are unavailable

Tests

  • PYTHONPATH=src python3 -m pytest tests/test_retrieval_backend.py tests/embeddings/test_rerank.py tests/test_context.py -q
  • uv run --with ruff python -m ruff check src tests
  • uv run --with mypy --with types-PyYAML --with jinja2 python -m mypy src

Closes #429

Summary by CodeRabbit

  • New Features

    • Added optional result reranking for hybrid retrieval, improving the order of returned matches when enabled.
    • Reranking affects only the first portion of results (configurable) while preserving the scoped set and keeping the remainder in its original order.
  • Bug Fixes

    • If reranking is unavailable or not configured correctly, retrieval now cleanly falls back to the existing ordering.
    • Added automated coverage for disabled/enabled reranking, configurable top_k, and repeated-call caching behavior.

@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 872b93ea-b239-4e67-a096-11cb04793c93

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds an optional rerank stage to hybrid context retrieval, gated by retrieval.rerank config. It reads rerank settings, reranks a scoped window of fused hits without changing membership, and falls back to the original order if reranking is unavailable. Tests cover the enabled, disabled, cached, and fallback paths.

Changes

Config-gated retrieval reranking

Layer / File(s) Summary
Rerank config and cache
src/vouch/context.py
Adds a module cache for the default reranker and helpers that read retrieval.rerank.enabled and retrieval.rerank.top_k from config with fallback to the caller limit.
Hybrid retrieval rerank wiring
src/vouch/context.py
Applies reranking to the filtered hybrid hit list before returning hybrid-tagged context items.
Rerank behavior tests
tests/test_retrieval_backend.py
Adds a config helper and tests covering disabled ordering, enabled reordering, top_k fallback to limit, cached reranker reuse, and ImportError fallback.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant context as context._retrieve
  participant config as _configured_rerank
  participant cache as _default_reranker_cached
  participant rerank as embeddings.rerank.rerank

  context->>config: read retrieval.rerank.enabled/top_k
  config-->>context: enabled flag, top_k
  context->>cache: load default reranker
  cache-->>context: cached reranker
  context->>rerank: rerank(query, hits, reranker, top_k)
  rerank-->>context: reordered hits
Loading

Possibly related PRs

  • vouchdev/vouch#42: Adds the reranking implementation that this PR imports and invokes from the context retrieval path.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR adds config-gated reranking to _retrieve, preserves default ordering, and degrades gracefully when reranker extras are missing.
Out of Scope Changes check ✅ Passed The changes stay within retrieval reranking and test coverage, with no unrelated feature work evident.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: reranking in context retrieval is gated by retrieval config.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added retrieval context, search, synthesis, and evaluation tests tests and fixtures size: S 50-199 changed non-doc lines labels Jul 7, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@src/vouch/context.py`:
- Around line 131-145: The reranking logic in do_rerank() is incorrectly reusing
the tuples returned by do_rerank(), which swaps in cross-encoder scores instead
of preserving the original hybrid scores. Update the ordering/reconciliation
step to use the original window tuples when building ordered results, and only
use reranked output for relative ordering via the hit keys (kind, artifact_id),
so items[*].score and explain[*].score stay unchanged for hybrid results.
- Around line 100-101: The rerank top_k normalization in the context setup is
incorrectly accepting boolean values because bool is a subclass of int, so True
can slip through as 1 instead of falling back to limit. Update the top_k
handling in the rerank logic to explicitly reject booleans before the integer
check, keeping the existing fallback behavior to limit when the value is
missing, non-numeric, or non-positive.
- Around line 119-126: The reranker is being recreated on every call to
build_context_pack(), which adds unnecessary CrossEncoder initialization cost.
Update the build_context_pack flow in context.py to reuse a single cached
reranker instance instead of calling default_reranker() inline each time, and
pass that cached instance into do_rerank so repeated retrievals avoid
reinitialization.
🪄 Autofix (Beta)

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: 63e86fa2-916b-4536-8ebc-e8039204c7c2

📥 Commits

Reviewing files that changed from the base of the PR and between 5c98a3a and 09bb4b5.

📒 Files selected for processing (2)
  • src/vouch/context.py
  • tests/test_retrieval_backend.py

Comment thread src/vouch/context.py Outdated
Comment thread src/vouch/context.py Outdated
Comment thread src/vouch/context.py
@github-actions github-actions Bot added size: M 200-499 changed non-doc lines and removed size: S 50-199 changed non-doc lines labels Jul 7, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
tests/test_retrieval_backend.py (1)

178-179: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reset _RERANKER_CACHE via monkeypatch instead of direct mutation.

All four rerank tests set context._RERANKER_CACHE = None directly rather than through monkeypatch.setattr. This bypasses pytest's automatic teardown, so if a test fails before completion (or tests run in a different order/under xdist), the cache can leak a mocked reranker object() into unrelated tests instead of being restored to its original state.

♻️ Suggested fix (repeat for each occurrence)
-    context._RERANKER_CACHE = None
+    monkeypatch.setattr(context, "_RERANKER_CACHE", None)

Also applies to: 224-224, 259-259, 294-294

🤖 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 `@tests/test_retrieval_backend.py` around lines 178 - 179, The rerank tests are
mutating context._RERANKER_CACHE directly, which can leak the mocked reranker
across tests if teardown is skipped. Update each rerank test to reset
context._RERANKER_CACHE using monkeypatch.setattr instead of direct assignment,
alongside the existing monkeypatch on rerank_mod.default_reranker, so pytest
restores the cache automatically after each test.
🤖 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.

Nitpick comments:
In `@tests/test_retrieval_backend.py`:
- Around line 178-179: The rerank tests are mutating context._RERANKER_CACHE
directly, which can leak the mocked reranker across tests if teardown is
skipped. Update each rerank test to reset context._RERANKER_CACHE using
monkeypatch.setattr instead of direct assignment, alongside the existing
monkeypatch on rerank_mod.default_reranker, so pytest restores the cache
automatically after each test.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 131c6b76-25e4-4371-98b6-7278ae4fc4da

📥 Commits

Reviewing files that changed from the base of the PR and between 09bb4b5 and cb9dafe.

📒 Files selected for processing (2)
  • src/vouch/context.py
  • tests/test_retrieval_backend.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/vouch/context.py

@Tet-9

Tet-9 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

@Steve-too , it should be targeting 'test' not 'main', otherwise it won't get merged

@Tet-9

Tet-9 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

@Steve-too , it should be targeting 'test' not 'main', otherwise it won't get merged

Try checking the contributing.MD

@Steve-too Steve-too changed the base branch from main to test July 7, 2026 19:17
@Steve-too Steve-too force-pushed the feat/context-rerank-config branch from b5228ca to 6d236de Compare July 7, 2026 19:17
@Steve-too

Copy link
Copy Markdown
Author

Retargeted to test and rebuilt the branch on top of origin/test, per CONTRIBUTING.md. The PR is now 3 commits ahead / 0 behind test, with the diff still limited to src/vouch/context.py and tests/test_retrieval_backend.py. Targeted local check passed: PYTHONPATH=src python3 -m pytest tests/test_retrieval_backend.py -q.

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

Labels

retrieval context, search, synthesis, and evaluation size: M 200-499 changed non-doc lines tests tests and fixtures

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(retrieval): wire the cross-encoder reranker into the kb.context path (config-gated)

2 participants