From db9bfbc8a80f05136d709fd1a6da24a7ea8c9c89 Mon Sep 17 00:00:00 2001 From: Dylan O'Neill Date: Wed, 16 Sep 2026 20:12:15 -0700 Subject: [PATCH] fix(evaluations): reject boolean scorer scores MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A scorer's return contract was "a bool or a finite number in 0-1", with True/False coerced to 1.0/0.0. Two problems. Coercion put two score types on the wire. A threshold then meant different things depending on which type a scorer happened to return, even though LaunchDarkly rules on score-vs-threshold identically for both -- so the comparison a caller reasoned about and the one ingest performed could differ. Worse, it hid bugs. Every non-empty value is truthy, so a scorer that returned "high", or a stray object, scored 1.0 and passed. The failure mode was a green run, which is the one failure mode an evaluation harness must not have. The contract is now a finite number in 0-1 and nothing else. A scorer answering a yes/no question returns 1.0 or 0.0 itself. A bool, a non-finite number, or one out of range is an invalid_score result -- per-criterion ERROR, never a raise, since the row's generation has already been paid for. numeric_score already excluded bool, so the fix is deleting the coercion branch rather than adding a check. ScorerFn drops bool from its signature, so a caller annotating their scorer sees this at type-check time rather than at ingest. BREAKING: a scorer returning a bool now produces an invalid_score result instead of 1.0/0.0. Callers return the number directly. Specced in ai-sdks-monorepo TESTING.md §8.8.4. Co-Authored-By: Claude Opus 5 --- packages/ai/README.md | 2 +- packages/client/README.md | 6 +- .../evaluations/criteria.py | 17 +- .../evaluations/runner.py | 26 ++- packages/client/tests/test_evaluations_run.py | 149 +++++++++++++++++- 5 files changed, 170 insertions(+), 30 deletions(-) diff --git a/packages/ai/README.md b/packages/ai/README.md index b1b47a70..f31dfade 100644 --- a/packages/ai/README.md +++ b/packages/ai/README.md @@ -68,7 +68,7 @@ result = await evals.run( generation={"provider": "OpenAI", "model": "gpt-4o"}, criteria=[ Judge(key="accuracy-judge"), - Scorer(name="mentions-policy", fn=lambda row, output: "policy" in (output or "")), + Scorer(name="mentions-policy", fn=lambda row, output: 1.0 if "policy" in (output or "") else 0.0), ], ) ``` diff --git a/packages/client/README.md b/packages/client/README.md index 1e50a5ec..9d72240f 100644 --- a/packages/client/README.md +++ b/packages/client/README.md @@ -92,8 +92,8 @@ from launchdarkly_ai_openai_messages import create_openai_messages_handler from launchdarkly_ai_server import DatasetRow, Judge, Scorer, init_evaluations -def mentions_policy(row: DatasetRow, output: str | None) -> bool: - return "refund policy" in (output or "").lower() +def mentions_policy(row: DatasetRow, output: str | None) -> float: + return 1.0 if "refund policy" in (output or "").lower() else 0.0 result = await init_evaluations().run( @@ -112,7 +112,7 @@ result = await init_evaluations().run( ) ``` -`Scorer.fn` receives the `DatasetRow` the output was generated from plus the generated output, may be sync or async, and must return a bool or a number from 0 to 1; booleans become 1.0 or 0.0. `Judge.threshold` defaults to 0.5 and `Scorer.threshold` to 1.0 — a perfect score, which is what a boolean scorer wants — and both accept an optional `pass_rate_threshold`. Judge keys and scorer names share one `criterionType` namespace and must be unique within a run, case-insensitively, because that name is part of each result's deterministic event identity. `Judge.ground_truth_context` overrides what the judge is graded against when the dataset row's expected output is not it. +`Scorer.fn` receives the `DatasetRow` the output was generated from plus the generated output, may be sync or async, and must return a **finite number from 0 to 1** — that is the whole contract, and nothing is coerced on your behalf. A scorer answering a yes/no question returns `1.0` or `0.0` itself; a `bool`, a `NaN`, or an out-of-range number is an `invalid_score` result. Returning one score type keeps a threshold comparison meaning the same thing for binary and graded scorers, and keeps a scorer that accidentally returns a non-score from passing silently — every non-empty value is truthy, so `return "high"` would otherwise score 1.0. `Judge.threshold` defaults to 0.5 and `Scorer.threshold` to 1.0 — a perfect score, which is what a yes/no scorer wants — and both accept an optional `pass_rate_threshold`. Judge keys and scorer names share one `criterionType` namespace and must be unique within a run, case-insensitively, because that name is part of each result's deterministic event identity. `Judge.ground_truth_context` overrides what the judge is graded against when the dataset row's expected output is not it. **The SDK reports scores and never rules on them.** LaunchDarkly derives each row's verdict at ingest by comparing the score against the criterion's stored threshold and success direction, so pass/fail policy is one server-side implementation that applies to every SDK version and to runs already recorded. A judge's direction lives on its AI Config and is injected server-side, keeping the one input a verdict turns on server-attested; a `Scorer` has no LaunchDarkly-side config to read, so it declares its own `success_direction` (default `"higher_is_better"` — set `"lower_is_better"` for a scorer that counts something unwanted, like a regex hit count). diff --git a/packages/client/src/launchdarkly_ai_server/evaluations/criteria.py b/packages/client/src/launchdarkly_ai_server/evaluations/criteria.py index ee0962fd..174b1946 100644 --- a/packages/client/src/launchdarkly_ai_server/evaluations/criteria.py +++ b/packages/client/src/launchdarkly_ai_server/evaluations/criteria.py @@ -7,7 +7,7 @@ from .types import DatasetRow -type ScorerFn = Callable[[DatasetRow, Any], float | bool | Awaitable[float | bool]] +type ScorerFn = Callable[[DatasetRow, Any], float | Awaitable[float]] type SuccessDirection = Literal["higher_is_better", "lower_is_better"] @@ -70,12 +70,19 @@ class Scorer: ``fn`` may be sync or async and receives ``(row, output)``, where ``row`` is the :class:`~launchdarkly_ai_server.evaluations.types.DatasetRow` the output was generated from and ``output`` is the generated output. It must - return a boolean or a numeric score from 0 to 1. Boolean results are - converted to 1.0 or 0.0 before being emitted as evaluation events. + return a **finite number from 0 to 1** -- that is the whole contract. + Nothing is coerced on the caller's behalf, so a scorer answering a yes/no + question returns ``1.0`` or ``0.0`` itself; a ``bool`` is an + ``invalid_score`` result, as is a non-finite or out-of-range number. + + Returning one score type keeps the ``threshold`` comparison below meaning + the same thing for binary and graded scorers. It also keeps a scorer that + accidentally returns a non-score from passing silently: every non-empty + value is truthy, so ``return "high"`` scored 1.0 under the old coercion. ``threshold`` defaults to 1.0: a row passes only on a perfect score, which - matches the common case of boolean scorers. Pass a lower threshold for - graded numeric scorers. + is what a scorer answering a yes/no question wants. Pass a lower threshold + for a graded scorer. ``success_direction`` says which way the score points, and defaults to higher-is-better. Unlike a judge, a scorer has no LaunchDarkly-side config diff --git a/packages/client/src/launchdarkly_ai_server/evaluations/runner.py b/packages/client/src/launchdarkly_ai_server/evaluations/runner.py index 164ea57d..e21ea887 100644 --- a/packages/client/src/launchdarkly_ai_server/evaluations/runner.py +++ b/packages/client/src/launchdarkly_ai_server/evaluations/runner.py @@ -775,25 +775,21 @@ async def _run_scorer_for_result( return self._criterion_error_result( base, started_clock, "scorer_raised", f"scorer fn raised: {error}" ) - if isinstance(score_value, bool): - score: float = 1.0 if score_value else 0.0 - else: - maybe_score = numeric_score(score_value) - if maybe_score is None: - return self._criterion_error_result( - base, - started_clock, - "invalid_score", - "scorer fn must return a bool or a finite number, " - f"got {score_value!r}", - ) - score = maybe_score - if score < 0 or score > 1: + # A bool is rejected rather than read as 1.0/0.0. numeric_score already + # excludes it, so the whole return contract is "a finite number in + # 0-1": a scorer answering a yes/no question returns 1.0 or 0.0 itself. + # Coercing on the caller's behalf sent two score types to ingest and + # made the threshold comparison mean different things for binary and + # graded scorers -- and silently scored `return "high"`-style bugs as a + # pass, since every non-empty value is truthy. + score = numeric_score(score_value) + if score is None or score < 0 or score > 1: return self._criterion_error_result( base, started_clock, "invalid_score", - f"scorer fn score must be between 0 and 1, got {score_value!r}", + "scorer fn must return a finite number between 0 and 1, " + f"got {score_value!r}", ) completed = datetime.now(UTC) return { diff --git a/packages/client/tests/test_evaluations_run.py b/packages/client/tests/test_evaluations_run.py index c7ca278b..ed03280b 100644 --- a/packages/client/tests/test_evaluations_run.py +++ b/packages/client/tests/test_evaluations_run.py @@ -1486,11 +1486,12 @@ async def handler(*args: object) -> dict[str, Any]: "usage": {"input_tokens": 10, "output_tokens": 4}, } - def check_refund(row: DatasetRow, output: Any) -> bool: + def check_refund(row: DatasetRow, output: Any) -> float: assert row.row_index == 42 assert row.input == "Ticket A" assert output == "refund exists" - return "refund" in str(output) + # A yes/no scorer returns the score itself: the SDK coerces nothing. + return 1.0 if "refund" in str(output) else 0.0 result = await evals.run( project_key="proj", @@ -1747,7 +1748,7 @@ async def handler(*args: object) -> dict[str, Any]: generation={"provider": "OpenAI", "model": "gpt-4o"}, criteria=[ Judge(key="accuracy"), - Scorer(name="accuracy", fn=lambda row, output: True), + Scorer(name="accuracy", fn=lambda row, output: 1.0), ], ) @@ -1775,7 +1776,7 @@ async def handler(*args: object) -> dict[str, Any]: generation={"provider": "OpenAI", "model": "gpt-4o"}, criteria=[ Judge(key="Accuracy"), - Scorer(name="accuracy", fn=lambda row, output: True), + Scorer(name="accuracy", fn=lambda row, output: 1.0), ], ) @@ -1863,7 +1864,7 @@ async def handler( generation={"provider": "OpenAI", "model": "gpt-4o"}, criteria=[ Judge(key="$ld:ai:judge:accuracy"), - Scorer(name="nonempty", fn=lambda row, output: bool(output)), + Scorer(name="nonempty", fn=lambda row, output: 1.0 if output else 0.0), ], ) @@ -1888,7 +1889,7 @@ def test_criteria_reject_thresholds_outside_zero_to_one( with pytest.raises(ValueError, match=f"{field} must be a number between 0 and 1"): Judge(key="$ld:ai:judge:accuracy", **{field: value}) with pytest.raises(ValueError, match=f"{field} must be a number between 0 and 1"): - Scorer(name="nonempty", fn=lambda row, output: True, **{field: value}) + Scorer(name="nonempty", fn=lambda row, output: 1.0, **{field: value}) def judge_variation( @@ -2249,3 +2250,139 @@ async def handler( assert result.passed is True assert max_in_flight == 2 + + +def scorer_run_transport() -> SequencedTransport: + """Transport for a one-row scorer-only run.""" + return SequencedTransport( + [ + response(200, {"id": "dataset-id", "name": "golden"}), + response( + 200, + dataset_page([{"rowIndex": 7, "input": "Question"}], total=1), + ), + response(201, {"id": "evaluation-id", "name": "support-qa", "version": 3}), + response( + 201, + {"id": "run-id", "evaluationId": "evaluation-id", "state": "PENDING"}, + ), + response( + 200, + {"statusCounts": {"total": 1, "passed": 1, "error": 0, "pending": 0}}, + ), + ] + ) + + +@pytest.mark.parametrize( + "returned", + [ + pytest.param(True, id="true"), + pytest.param(False, id="false"), + pytest.param(float("nan"), id="nan"), + pytest.param(float("inf"), id="inf"), + pytest.param(3, id="above-range"), + pytest.param(-1, id="below-range"), + pytest.param("high", id="string"), + pytest.param(None, id="none"), + ], +) +@pytest.mark.asyncio +async def test_scorer_must_return_a_finite_number_in_range( + stub_sdk_client: MagicMock, + returned: Any, +) -> None: + """A bool is an invalid score, not a shortcut for 1.0/0.0. + + Coercing booleans sent two score types to ingest and made the threshold + comparison mean different things for binary and graded scorers. It also hid + bugs: every non-empty value is truthy, so a scorer that returned "high" + scored a pass. + """ + transport = scorer_run_transport() + evals = init_evaluations(api_token="token", sdk_key="sdk-key", transport=transport) + + async def handler(*args: object) -> dict[str, Any]: + return {"output": "generated"} + + result = await evals.run( + project_key="proj", + key="support-qa", + dataset="golden", + handler=handler, + generation={"provider": "OpenAI", "model": "gpt-4o"}, + criteria=[Scorer(name="binary", fn=lambda row, output: returned)], + ) + + events = [call.args[2] for call in stub_sdk_client.track.call_args_list] + scorer_event = next(event for event in events if event.get("kind") == "scorer") + assert scorer_event["status"] == "ERROR" + assert scorer_event["error"]["code"] == "invalid_score" + # The message names the offending value, so a caller can see what came back. + assert repr(returned) in scorer_event["error"]["message"] + assert scorer_event["errorMessage"] == scorer_event["error"]["message"] + assert "score" not in scorer_event + # A rejected score is a per-criterion ERROR, never a raised exception: the + # row's generation has already been paid for. + assert result.run_id == "run-id" + stub_sdk_client.flush.assert_awaited() + + +@pytest.mark.parametrize( + ("returned", "expected"), + [(1.0, 1.0), (0.0, 0.0), (0, 0.0), (1, 1.0), (0.25, 0.25)], +) +@pytest.mark.asyncio +async def test_scorer_accepts_the_range_boundaries_and_integers( + stub_sdk_client: MagicMock, + returned: Any, + expected: float, +) -> None: + """0 and 1 stay valid, as ints too -- only bool is excluded.""" + transport = scorer_run_transport() + evals = init_evaluations(api_token="token", sdk_key="sdk-key", transport=transport) + + async def handler(*args: object) -> dict[str, Any]: + return {"output": "generated"} + + await evals.run( + project_key="proj", + key="support-qa", + dataset="golden", + handler=handler, + generation={"provider": "OpenAI", "model": "gpt-4o"}, + criteria=[Scorer(name="graded", fn=lambda row, output: returned)], + ) + + events = [call.args[2] for call in stub_sdk_client.track.call_args_list] + scorer_event = next(event for event in events if event.get("kind") == "scorer") + assert scorer_event["status"] == "COMPLETE" + assert scorer_event["score"] == expected + + +@pytest.mark.asyncio +async def test_async_scorer_score_is_awaited_before_validation( + stub_sdk_client: MagicMock, +) -> None: + transport = scorer_run_transport() + evals = init_evaluations(api_token="token", sdk_key="sdk-key", transport=transport) + + async def handler(*args: object) -> dict[str, Any]: + return {"output": "generated"} + + async def graded(row: DatasetRow, output: Any) -> float: + return 0.5 + + await evals.run( + project_key="proj", + key="support-qa", + dataset="golden", + handler=handler, + generation={"provider": "OpenAI", "model": "gpt-4o"}, + criteria=[Scorer(name="graded", fn=graded)], + ) + + events = [call.args[2] for call in stub_sdk_client.track.call_args_list] + scorer_event = next(event for event in events if event.get("kind") == "scorer") + assert scorer_event["status"] == "COMPLETE" + assert scorer_event["score"] == 0.5