feat: add custom:mrr metric for RAG retrieval quality evaluation - #302
feat: add custom:mrr metric for RAG retrieval quality evaluation#302x86girl wants to merge 5 commits into
Conversation
Add Mean Reciprocal Rank (MRR) as a new custom metric that measures how high the first relevant context appears in the ranked list of retrieved contexts. This is a deterministic, non-LLM metric that complements existing Ragas context metrics. - Add `expected_contexts` field to TurnData for ground-truth contexts - Implement MRR evaluation with normalized containment matching - Register metric in CustomMetrics, validator, and system config - Add 26 comprehensive tests covering all edge cases Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WalkthroughAdds semantic-similarity MRR under ChangesSemantic MRR Evaluation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant NLPMetrics
participant evaluate_mrr
participant EmbeddingModel
participant ConformalCalibration
NLPMetrics->>evaluate_mrr: evaluate turn with model and configuration
evaluate_mrr->>EmbeddingModel: encode contexts
EmbeddingModel-->>evaluate_mrr: embeddings
evaluate_mrr->>ConformalCalibration: resolve threshold when calibration is configured
ConformalCalibration-->>evaluate_mrr: threshold
evaluate_mrr-->>NLPMetrics: MRR score and reason
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![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 `@src/lightspeed_evaluation/core/metrics/custom/mrr_eval.py`:
- Around line 21-23: Update the containment check in the custom MRR matching
function around `_normalize_text` so it returns `False` whenever
`norm_retrieved` or `norm_expected` is empty before evaluating substring
containment. Add regression tests covering whitespace-only retrieved and
expected contexts.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 065e6a3d-25ef-4df1-b84e-dac605a54fd1
📒 Files selected for processing (7)
config/system.yamlsrc/lightspeed_evaluation/core/metrics/custom/__init__.pysrc/lightspeed_evaluation/core/metrics/custom/custom.pysrc/lightspeed_evaluation/core/metrics/custom/mrr_eval.pysrc/lightspeed_evaluation/core/models/data.pysrc/lightspeed_evaluation/core/system/validator.pytests/unit/core/metrics/custom/test_mrr_eval.py
asamal4
left a comment
asamal4
left a comment
There was a problem hiding this comment.
Thanks !!
Please check coderabbit's comment.. and PTAL my inline comment.
Primarily I have concern about the custom implementation especially checking the relevancy part. We should either adopt a standard package or replace the matching logic with something more robust.
…_length, rename to nlp:mrr - Reject empty normalized contexts in _is_context_match before containment check - Add min_length=1 to expected_contexts field for early Pydantic validation - Rename metric from custom:mrr to nlp:mrr and move registration to NLPMetrics Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace naive substring containment in nlp:mrr with cosine similarity using sentence-transformers embeddings (default: all-MiniLM-L6-v2). Threshold 0.65 based on STS Benchmark empirical data for this model. Falls back to substring matching when sentence-transformers is not installed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add optional CRC (Angelopoulos et al., ICLR 2024) for users who want data-driven threshold calibration instead of the fixed default. Activated by providing calibration_pairs in nlp:mrr metric metadata. Algorithm vendored from github.com/aangelopoulos/conformal-risk (MIT). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/lightspeed_evaluation/core/metrics/nlp.py (1)
100-107: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
_evaluate_mrrdocstring lacks Args/Returns, unlike sibling handlers.Every other metric handler in this file (
_evaluate_bleu,_evaluate_rouge,_evaluate_semantic_similarity_distance) documents Args and Returns in Google style;_evaluate_mrrhas only a one-line docstring.This shares a root cause with a similar gap in
mrr_eval.py'sevaluate_mrr; see consolidated comment.🤖 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 `@src/lightspeed_evaluation/core/metrics/nlp.py` around lines 100 - 107, Expand the _evaluate_mrr docstring to use the same Google-style Args and Returns structure as _evaluate_bleu, _evaluate_rouge, and _evaluate_semantic_similarity_distance, documenting conv_data, turn_idx, turn_data, is_conversation, and the returned optional score and status string.Source: Coding guidelines
src/lightspeed_evaluation/core/metrics/custom/mrr_eval.py (2)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winIncomplete Google-style docstrings on the new MRR entry points. Both new/changed MRR functions in this PR fall short of the project's Google-style docstring convention used by their sibling functions.
src/lightspeed_evaluation/core/metrics/custom/mrr_eval.py#L142-168: addembedding_model_nameto theArgs:section ofevaluate_mrr's docstring.src/lightspeed_evaluation/core/metrics/nlp.py#L100-107: expand_evaluate_mrr's docstring to includeArgs:/Returns:sections, matching_evaluate_bleu/_evaluate_rouge/_evaluate_semantic_similarity_distancein the same file.🤖 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 `@src/lightspeed_evaluation/core/metrics/custom/mrr_eval.py` at line 1, Complete the Google-style docstrings for the MRR entry points: update evaluate_mrr to document embedding_model_name in its Args section, and expand _evaluate_mrr with Args and Returns sections consistent with _evaluate_bleu, _evaluate_rouge, and _evaluate_semantic_similarity_distance. Keep the documentation aligned with each function’s actual parameters and return value.Source: Coding guidelines
142-168: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
evaluate_mrrdocstring omitsembedding_model_nameparameter.The
Args:section documentsembedding_modelandmrr_configbut not the new keyword-onlyembedding_model_nameparameter added to the signature.This finding shares a root cause with a similar docstring gap in
nlp.py's_evaluate_mrr; see consolidated comment.🤖 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 `@src/lightspeed_evaluation/core/metrics/custom/mrr_eval.py` around lines 142 - 168, Update the Args section of evaluate_mrr to document the embedding_model_name keyword-only parameter, including its purpose and relationship to the embedding model. Leave the existing parameter documentation unchanged.Source: Coding guidelines
🤖 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/lightspeed_evaluation/core/metrics/nlp.py`:
- Around line 45-67: The MRR embedding state in the NLP metrics evaluator is
shared across concurrent evaluations and can be mutated by per-turn model
overrides. Update `_evaluate_mrr` and the embedding initialization/cache logic
around `_embedding_model_name` and `_embedding_model` to use a thread-safe,
model-name-keyed cache or synchronization, ensuring each evaluation uses the
model matching its requested `mrr_config["embedding_model"]` without changing
another thread’s state.
---
Nitpick comments:
In `@src/lightspeed_evaluation/core/metrics/custom/mrr_eval.py`:
- Line 1: Complete the Google-style docstrings for the MRR entry points: update
evaluate_mrr to document embedding_model_name in its Args section, and expand
_evaluate_mrr with Args and Returns sections consistent with _evaluate_bleu,
_evaluate_rouge, and _evaluate_semantic_similarity_distance. Keep the
documentation aligned with each function’s actual parameters and return value.
- Around line 142-168: Update the Args section of evaluate_mrr to document the
embedding_model_name keyword-only parameter, including its purpose and
relationship to the embedding model. Leave the existing parameter documentation
unchanged.
In `@src/lightspeed_evaluation/core/metrics/nlp.py`:
- Around line 100-107: Expand the _evaluate_mrr docstring to use the same
Google-style Args and Returns structure as _evaluate_bleu, _evaluate_rouge, and
_evaluate_semantic_similarity_distance, documenting conv_data, turn_idx,
turn_data, is_conversation, and the returned optional score and status string.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8194b0fd-d397-4399-86f6-82ab895ec867
📒 Files selected for processing (9)
config/system.yamlsrc/lightspeed_evaluation/core/metrics/custom/__init__.pysrc/lightspeed_evaluation/core/metrics/custom/conformal.pysrc/lightspeed_evaluation/core/metrics/custom/mrr_eval.pysrc/lightspeed_evaluation/core/metrics/nlp.pysrc/lightspeed_evaluation/core/models/data.pysrc/lightspeed_evaluation/core/system/validator.pytests/unit/core/metrics/custom/test_conformal.pytests/unit/core/metrics/custom/test_mrr_eval.py
Replace mutable _embedding_model/_embedding_model_name instance state with a dict[str, Any] cache keyed by model name. Each thread looks up the model by name without mutating shared state, eliminating the race condition under ThreadPoolExecutor. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lightspeed_evaluation/core/metrics/nlp.py (1)
65-93: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCache-miss race can trigger duplicate concurrent model loads.
_get_embedding_modeldoes an unsynchronized check-then-act: two threads racing on a first-time cache miss for the samemodel_namewill both pass thenot in self._modelscheck and both callSentenceTransformer(model_name)concurrently. This wastes memory/CPU loading the same model twice, and constructing the sameSentenceTransformermodel concurrently in one process is a known crash surface in some sentence-transformers versions. Correctness isn't at risk (dict writes are atomic under the GIL, so one instance simply gets discarded), but this is a real hazard worth closing with a lock, especially sinceNLPMetricsis shared underThreadPoolExecutorper the pipeline wiring.🔒 Proposed fix using double-checked locking
def __init__(self, embedding_model_name: Optional[str] = None) -> None: self._default_model_name = embedding_model_name or _DEFAULT_EMBEDDING_MODEL self._models: dict[str, Any] = {} self._load_failed: set[str] = set() + self._load_lock = threading.Lock() self.supported_metrics = { ... } def _get_embedding_model(self, model_name: str) -> Any: """Lazy-load and cache the sentence-transformers model by name.""" if model_name in self._load_failed: return None if model_name in self._models: return self._models[model_name] - try: - import sentence_transformers # type: ignore[import-not-found] # pylint: disable=import-outside-toplevel - - model = sentence_transformers.SentenceTransformer(model_name) - self._models[model_name] = model - logger.info("Loaded embedding model: %s", model_name) - return model - except ImportError: - ... - self._load_failed.add(model_name) - except (OSError, RuntimeError) as exc: - ... - self._load_failed.add(model_name) - return None + with self._load_lock: + if model_name in self._load_failed: + return None + if model_name in self._models: + return self._models[model_name] + try: + import sentence_transformers # type: ignore[import-not-found] # pylint: disable=import-outside-toplevel + + model = sentence_transformers.SentenceTransformer(model_name) + self._models[model_name] = model + logger.info("Loaded embedding model: %s", model_name) + return model + except ImportError: + ... + self._load_failed.add(model_name) + except (OSError, RuntimeError) as exc: + ... + self._load_failed.add(model_name) + return NonePlease confirm whether
sentence-transformers(as pinned in this project) documents any additional thread-safety caveats around concurrent model construction that would strengthen the case for this lock.🤖 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 `@src/lightspeed_evaluation/core/metrics/nlp.py` around lines 65 - 93, Protect the cache-miss path in _get_embedding_model with a lock so only one thread constructs a given SentenceTransformer at a time. Recheck _load_failed and _models after acquiring the lock, then perform the existing import, construction, caching, and failure handling while holding it; retain the fast-path checks before locking.
🤖 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 `@src/lightspeed_evaluation/core/metrics/nlp.py`:
- Around line 65-93: Protect the cache-miss path in _get_embedding_model with a
lock so only one thread constructs a given SentenceTransformer at a time.
Recheck _load_failed and _models after acquiring the lock, then perform the
existing import, construction, caching, and failure handling while holding it;
retain the fast-path checks before locking.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a0da8f95-137e-4b2a-81e1-158f9f9227c7
📒 Files selected for processing (1)
src/lightspeed_evaluation/core/metrics/nlp.py
|
On top of the semantic similarity matching, I also added an optional conformal risk control layer for threshold calibration. This is completely opt in and does not activate unless you provide calibration_pairs in the nlp:mrr metric metadata. The idea is simple: instead of relying on a fixed similarity threshold (0.65 by default), users who have domain specific text pairs with known matches can feed them as calibration data. The algorithm (from Angelopoulos et al., ICLR 2024) then computes a threshold with a statistical guarantee that the false negative rate stays below a configurable alpha (default 10%). Without calibration data, nothing changes. The metric uses the fixed default threshold and works exactly the same way. The conformal calibration is there for teams that want more rigor in their evaluation setup. The core algorithm is vendored from github.com/aangelopoulos/conformal-risk (MIT license) and is about 5 lines of code, so the maintenance cost is minimal. Example config with calibration enabled: "nlp:mrr":
threshold: 0.5
default_similarity_threshold: 0.65
alpha: 0.1
calibration_pairs:
- ["RHEL is a Linux distribution", "Red Hat Enterprise Linux is an operating system"]
- ["kubernetes pods", "pods are the smallest deployable units in k8s"] |
|
Regarding the original concern about the matching logic being too naive, I want to share some context on how the similarity threshold was chosen. The previous substring containment approach had a fundamental limitation: it could only match contexts that literally contained each other. Semantically equivalent texts with different wording would always fail. The new approach uses cosine similarity on sentence transformer embeddings (all-MiniLM-L6-v2 by default). The key question then becomes what threshold to use for deciding match vs no match. For this model, the empirical similarity distributions from STS Benchmark data look roughly like this: paraphrases and semantically equivalent text typically score between 0.70 and 0.90, related but different content falls in the 0.40 to 0.65 range, and unrelated text scores below 0.30. A study on literature review pipelines using this same model (arxiv.org/abs/2509.15292) calculated an empirical threshold of 0.659 using Q3 + 0.5*IQR over cosine similarities. That study was in a different domain (scientific paper filtering, not RAG context matching), so the number is a reference point rather than a definitive answer for our use case. We settled on 0.65 as the default because it sits at the boundary between "related but different" and "semantically equivalent", which is the right cutoff for RAG context matching. A retrieved context should convey the same information as the expected one, not just be topically related. The threshold is also configurable via default_similarity_threshold in the metric metadata, so teams can adjust it to their domain if needed. One thing worth noting is that all-MiniLM-L6-v2 truncates inputs at 256 word pieces, so longer paragraph level contexts will only have their first ~200 words encoded. This is generally fine for RAG chunks but something to keep in mind for unusually long contexts. |
|
@x86girl Probably I should have given more clearer comment about robust relevancy check. Current embedding approach is having some issues + it increased the scope significantly.. I will let you take the final call - 1. Are going to retain embedding approach and fix related issue, 2. Switch to n-gram approach and add embedding later in alignment with existing embedding model processing without a hardcode value.. Current model is practically unusable, most chunks will be of bigger size |
|
I am going to find more info about n-gram and try to switch to it. |
|
@x86girl let's remove the embedding approach and new threshold, Keep the old approach along with review fixes (ignore the match logic related comment).. Then we can merge this PR and we can work together to add better match logic. WDYT ? |
2 participants
Mean Reciprocal Rank (MRR) is a well-established information retrieval metric introduced by Voorhees (1999) in the TREC-8 Question Answering Track Report. It measures the rank position of the first relevant result in a ranked list, computed as 1/rank, yielding 1.0 when the
first retrieved item is relevant, 0.5 for the second, and so on.
▎ "The reciprocal rank of a query response is the multiplicative
▎ inverse of the rank of the first correct answer."
▎ Voorhees, E.M. (1999). The TREC-8 Question Answering Track Report.
▎ Proceedings of the 8th Text REtrieval Conference (TREC-8), NIST
▎ Special Publication 500-246, pp. 77–82.
In the context of RAG (Retrieval-Augmented Generation) evaluation, MRR provides a deterministic, non-LLM signal that directly measures retrieval ranking quality
It mesuares how quickly the system surfaces a relevant context chunk. This complements the existing LLM-judge-based Ragas metrics (context_recall, context_precision, context_relevance) by adding a fast, reproducible, and cost-free retrieval quality indicator.
Summary by CodeRabbit
New Features
Validation
custom:mrrtonlp:mrr.Tests