[NA] [SDK] fix: correct NLTK usage in METEOR and chrF metrics

sdks/python/src/opik/evaluation/metrics/heuristics/chrf.py

@@ -85,20 +85,31 @@ def __init__(
                     " `pip install nltk` or provide `chrf_fn`."
                 )
 
-            def _compute(candidate: Sequence[str], references: Sequence[str]) -> float:
-                try:
-                    return float(
-                        nltk_chrf_score.sentence_chrf(
-                            references,
-                            candidate,
-                            max_len=self._char_order,
-                            beta=self._beta,
-                            ignore_whitespace=self._ignore_whitespace,
-                        )
+            def _score_single(candidate: Sequence[str], reference: str) -> float:
+                # `sentence_chrf` has exposed this exact signature since NLTK 3.5,
+                # so every version installable on the Python versions this SDK
+                # supports accepts these keywords. Catching TypeError here would
+                # only mask genuine errors (and silently drop char_order/beta/
+                # ignore_whitespace), so let it propagate.
+                return float(
+                    nltk_chrf_score.sentence_chrf(
+                        reference,
+                        candidate,
+                        max_len=self._char_order,
+                        beta=self._beta,
+                        ignore_whitespace=self._ignore_whitespace,
                     )
-                except TypeError:
-                    # Older NLTK versions expose the helper with fewer keyword arguments.
-                    return float(nltk_chrf_score.sentence_chrf(references, candidate))
+                )
+
+            def _compute(candidate: Sequence[str], references: Sequence[str]) -> float:
+                # NLTK's sentence_chrf scores against a *single* reference; handing
+                # it the whole list makes NLTK join the references into one string,
+                # which drags the score down instead of rewarding the best match.
+                # Score each reference on its own and keep the highest, the standard
+                # multi-reference behaviour for chrF.
+                return max(
+                    _score_single(candidate, reference) for reference in references
+                )
 
             self._chrf_fn = _compute
 

sdks/python/src/opik/evaluation/metrics/heuristics/meteor.py

@@ -7,9 +7,15 @@
     nltk = None
     wordnet = None
 
+from opik import semantic_version
 from opik.exceptions import MetricComputationError
 from opik.evaluation.metrics import base_metric, score_result
 
+# NLTK 3.6.5 switched `meteor_score` to pre-tokenized input; 3.6.4 and earlier
+# expect untokenized strings. Supporting both would mean branching on a release
+# from 2021, so the default backend requires the modern API and says so clearly.
+MINIMUM_NLTK_VERSION = "3.6.5"
+
 try:
     from nltk.translate import meteor_score as nltk_meteor_score
 except ImportError:  # pragma: no cover - optional dependency
@@ -33,9 +39,13 @@ class METEOR(base_metric.BaseMetric):
         https://huggingface.co/spaces/evaluate-metric/meteor
 
     Args:
-        meteor_fn: Optional callable with the same interface as
-            ``nltk.translate.meteor_score.meteor_score``. When omitted the
-            function from NLTK is used.
+        meteor_fn: Optional callable ``(references, hypothesis) -> float`` that
+            receives **untokenized** text: a sequence of reference strings and a
+            single hypothesis string. Note this deliberately differs from
+            ``nltk.translate.meteor_score.meteor_score``, which requires
+            pre-tokenized input — passing that function in directly will not
+            work. When omitted, NLTK is used through an adapter that tokenizes
+            on your behalf.
         alpha: Precision weight.
         beta: Penalty exponent.
         gamma: Fragmentation penalty weight.
@@ -65,6 +75,18 @@ def __init__(
                     " `pip install nltk` or provide `meteor_fn`."
                 )
 
+            if (
+                nltk is not None
+                and semantic_version.SemanticVersion.parse(nltk.__version__)
+                < MINIMUM_NLTK_VERSION
+            ):
+                raise ImportError(
+                    f"METEOR metric requires nltk >= {MINIMUM_NLTK_VERSION}, but "
+                    f"{nltk.__version__} is installed. Earlier versions expect "
+                    "untokenized input. Upgrade via `pip install -U nltk` or supply "
+                    "`meteor_fn`."
+                )
+
             if nltk is not None and wordnet is not None:
                 try:
                     wordnet.ensure_loaded()  # type: ignore[attr-defined]
@@ -82,10 +104,21 @@ def __init__(
                         ) from download_error
 
             def _scorer(references: Sequence[str], hypothesis: str) -> float:
+                # NLTK's meteor_score expects pre-tokenized input: an iterable of
+                # token lists for the references and a token list for the
+                # hypothesis. Handing it raw strings raises TypeError, so tokenize
+                # here (whitespace split, matching BLEU/GLEU) while keeping the
+                # public `meteor_fn` contract string-based.
+                tokenized_references = [reference.split() for reference in references]
+                tokenized_hypothesis = hypothesis.split()
                 try:
                     return float(
                         nltk_meteor_score.meteor_score(
-                            references, hypothesis, alpha=alpha, beta=beta, gamma=gamma
+                            tokenized_references,
+                            tokenized_hypothesis,
+                            alpha=alpha,
+                            beta=beta,
+                            gamma=gamma,
                         )
                     )
                 except LookupError as error:

sdks/python/tests/unit/evaluation/metrics/test_heuristics.py

@@ -1,4 +1,5 @@
 import re
+import types
 
 import pytest
 
@@ -411,6 +412,17 @@ def test_kl_divergence_avg_direction():
     assert result.value >= 0.0
 
 
+def _skip_without_wordnet() -> None:
+    """Skip when the optional `nltk` dependency or its WordNet corpus is missing."""
+    pytest.importorskip("nltk")
+    from nltk.corpus import wordnet
+
+    try:
+        wordnet.ensure_loaded()
+    except LookupError:
+        pytest.skip("NLTK WordNet corpus is not available")
+
+
 def test_meteor_metric_with_custom_fn():
     captured = []
 
@@ -433,6 +445,84 @@ def test_meteor_rejects_empty_inputs():
         metric.score(output="hyp", reference="   ")
 
 
+def test_meteor_metric__default_backend__hands_nltk_pretokenized_input(monkeypatch):
+    # Locks the tokenization contract with a stubbed NLTK, so this regression stays
+    # covered on a bare CI runner with neither `nltk` nor the WordNet corpus
+    # installed. Reverting the fix makes the recorded call raw strings and fails here.
+    from opik.evaluation.metrics.heuristics import meteor as meteor_module
+
+    recorded = {}
+
+    class _StubMeteor:
+        @staticmethod
+        def meteor_score(references, hypothesis, alpha, beta, gamma):
+            recorded["references"] = references
+            recorded["hypothesis"] = hypothesis
+            return 0.5
+
+    monkeypatch.setattr(meteor_module, "nltk_meteor_score", _StubMeteor)
+    monkeypatch.setattr(meteor_module, "nltk", None)
+    monkeypatch.setattr(meteor_module, "wordnet", None)
+
+    METEOR(track=False).score(output="the cat sat", reference="the cat ran")
+
+    assert recorded["hypothesis"] == ["the", "cat", "sat"]
+    assert recorded["references"] == [["the", "cat", "ran"]]
+
+
+def test_meteor_metric__legacy_nltk__raises_actionable_import_error(monkeypatch):
+    # NLTK <= 3.6.4 expects untokenized input, so the tokenizing adapter cannot
+    # work there. Fail at construction with a clear message instead of a confusing
+    # TypeError at score time.
+    from opik.evaluation.metrics.heuristics import meteor as meteor_module
+
+    monkeypatch.setattr(meteor_module, "nltk_meteor_score", object())
+    monkeypatch.setattr(
+        meteor_module, "nltk", types.SimpleNamespace(__version__="3.6.4")
+    )
+    monkeypatch.setattr(meteor_module, "wordnet", None)
+
+    with pytest.raises(ImportError, match="requires nltk >= 3.6.5"):
+        METEOR(track=False)
+
+
+def test_meteor_metric__default_nltk_backend__scores_without_error():
+    # NLTK's meteor_score requires pre-tokenized input; before the fix the default
+    # backend passed raw strings and every call raised
+    # `TypeError: "hypothesis" expects pre-tokenized hypothesis`, so the metric was
+    # unusable outside of dependency-injected tests. Needs the WordNet corpus.
+    _skip_without_wordnet()
+
+    metric = METEOR(track=False)
+
+    identical = metric.score(
+        output="the cat sat on the mat", reference="the cat sat on the mat"
+    ).value
+    different = metric.score(
+        output="the cat sat on the mat", reference="a completely unrelated sentence"
+    ).value
+
+    # METEOR always applies a fragmentation penalty, so an exact match tops out
+    # just below 1.0 rather than at it.
+    assert identical > 0.99
+    assert identical > different
+
+
+def test_meteor_metric__multiple_references__picks_best_match():
+    _skip_without_wordnet()
+
+    metric = METEOR(track=False)
+    hypothesis = "the cat sat on the mat"
+
+    best_reference_only = metric.score(output=hypothesis, reference=hypothesis).value
+    with_distractor = metric.score(
+        output=hypothesis,
+        reference=["a completely unrelated sentence", hypothesis],
+    ).value
+
+    assert with_distractor == pytest.approx(best_reference_only)
+
+
 def test_gleu_metric_with_custom_fn():
     def gleu_fn(references, hypothesis):
         return 0.5
@@ -525,6 +615,52 @@ def test_chrf_metric__char_order_and_ignore_whitespace_vary__change_score():
     assert order_1 != order_6
 
 
+def test_chrf_metric__default_backend__scores_each_reference_separately(monkeypatch):
+    # Locks the per-reference contract with a stubbed NLTK so it runs without the
+    # optional dependency. Before the fix NLTK received the reference list in one
+    # call and joined it; now it must be called once per reference, best score kept.
+    from opik.evaluation.metrics.heuristics import chrf as chrf_module
+
+    seen_references = []
+
+    class _StubChrf:
+        @staticmethod
+        def sentence_chrf(
+            reference,
+            hypothesis,
+            min_len=1,
+            max_len=6,
+            beta=3.0,
+            ignore_whitespace=True,
+        ):
+            seen_references.append(reference)
+            return 0.25 if reference == "first ref" else 0.75
+
+    monkeypatch.setattr(chrf_module, "nltk_chrf_score", _StubChrf)
+
+    result = ChrF(track=False).score(
+        output="hypothesis", reference=["first ref", "second ref"]
+    )
+
+    assert seen_references == ["first ref", "second ref"]
+    assert result.value == pytest.approx(0.75)
+
+
+def test_chrf_metric__multiple_references__scores_against_best_reference():
+    # NLTK's sentence_chrf takes a single reference. Before the fix the whole list
+    # was handed to it, so NLTK joined the references into one string and an exact
+    # match against one of them scored ~0.42 instead of 1.0.
+    pytest.importorskip("nltk")
+
+    metric = ChrF(track=False)
+    result = metric.score(
+        output="the cat sat on the mat",
+        reference=["totally unrelated words here", "the cat sat on the mat"],
+    )
+
+    assert result.value == pytest.approx(1.0)
+
+
 def test_spearman_ranking_metric():
     metric = SpearmanRanking(track=False)
     result = metric.score(output=["b", "a", "c"], reference=["a", "b", "c"])