Repository navigation
Conversation
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 <noreply@anthropic.com>
db9bfbc to
0bc9544
Compare
jeffdupont
left a comment
There was a problem hiding this comment.
Reviewed with the 1.0 freeze in mind. Nothing blocking from me. This matches ai-sdks-monorepo TESTING.md §8.8.4 as tightened in 1402609 ("there is no boolean shortcut and nothing is coerced on the caller's behalf"), and the invalid_score row in the §8.8.4 failure table. Tested on 0bc9544: make test exit 0 (1410 passed, 11 skipped), make typecheck exit 0. That head is 2 commits behind main, and both are release commits.
On the contract question, reject is the right choice for 1.0. I went through the three options:
- Coerce is the bug this PR fixes.
return "high"scored a pass. - Drop doesn't work. The backend needs one result per (row, criterion) to finish the run, so a dropped result stalls it until the §8.9 timeout.
- Raise throws away a generation that has already been paid for.
A per-criterion ERROR is what §8.8.3 already does for judge scores, so judges and scorers now behave the same way. It is also the right direction to freeze. If we want a boolean shortcut later, loosening the contract is additive. Tightening it after 1.0 would be a breaking change.
JS parity: there's nothing to port. JS has no evaluations on main (deferred to 1.1 in the GA plan), and its judge path already rejects booleans, because isFiniteScore checks typeof score === 'number' (packages/client/src/judges.ts:68). When evals land in JS, §8.8.4 covers it.
Smaller notes:
- The PR body says dropping
boolfromScorerFnmeans "a caller who annotates their scorer sees this at type-check time". That isn't true. I ranmypy --strictagainst this branch: adef f(...) -> boolscorer andlambda row, output: Trueboth pass, and only a-> strscorer is flagged.boolis a subclass ofint, and type checkers acceptintwherefloatis expected. The runtime check is the only guard. Narrowing the type is still worth doing, but the claim should come out of the PR body and the squash message. Details inline. - Please mark it as breaking when you squash:
fix(evaluations)!:or aBREAKING CHANGE:footer. The coercion shipped inlaunchdarkly-ai-server0.2.4. As titled, release-please will list this under Bug Fixes as a patch, and a caller with bool scorers will see every row goERRORwith no note telling them why. - Optional: on that upgrade path the only signal is
passed=Falseanderror_rowsin the summary._criterion_error_result(runner.py:818) doesn't log anything locally. Alogger.warningonce per scorer oninvalid_scorethat names the returned value would make the bool case obvious from the caller's own output.
A scorer that returns a bool now produces an invalid_score result. Log a warning once for each scorer. The warning names the scorer and the value it returned. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Track the scorers that already logged a warning for each run. A module that runs more than once now logs the warning in every run. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
@donei003 this is approved but conflicts with |
…olean-scores # Conflicts: # packages/ai/README.md # packages/client/tests/test_evaluations_run.py
Keeps both sides of the test_evaluations_run.py conflict. The new scorer tests now route through tagged() handlers and a mode, which main requires. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Pushed bc596c0: merged origin/main, keeping both sides of the test_evaluations_run.py conflict. The new scorer tests now use Not done: the invalid_score message and warning still use The PR body makes no claim that mypy catches a bool scorer (it says the opposite), so it is unchanged. Needs a human reviewer. |
Keep this branch's rule that a bool is an invalid score, against main's coercion. The invalid_score message and warning now name the value with _describe_value from #209 (true, null, "high", NaN), and the test asserts that rendering. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@aknight-ld, could you re-approve? There are new commits since your approval.
Locally the full suite passes (3379 passed), along with lint, format, typecheck and the API report check. @donei003, FYI since it's your branch: I merged main in rather than rebasing, so nothing was force-pushed. |
jeffdupont
left a comment
There was a problem hiding this comment.
Re-approving after the main merge in 52f69be: bools still reject, and the invalid_score message now uses _describe_value.
| if score is None or score < 0 or score > 1: | ||
| message = ( | ||
| "scorer fn must return a finite number between 0 and 1, " | ||
| f"got {_describe_value(score_value)}" |
There was a problem hiding this comment.
🔴 Invalid scorer values abort evaluation runs
When a scorer returns a mapping with tuple keys, _describe_value raises instead of recording invalid_score. The exception escapes _run_scorer_for_result, aborting the run after generation.
Learn more
The scorer's return value is supplied by customer code. An invalid return must become a per-criterion ERROR, leaving the run free to emit events for all rows. Here numeric_score rejects a mapping, but _describe_value calls canonical_json, whose json.dumps step rejects mappings with tuple keys. The exception occurs after the scorer invocation's exception handler and propagates through _run_criteria_for_results, preventing criterion event emission.
Example: A scorer returns {('precision', 'recall'): 0.8} instead of a score. The expected invalid_score event never appears; run() raises TypeError after generating the rows.
Recommended fix: Make _describe_value safely fall back to repr(value) when JSON serialization fails, while retaining canonical formatting for ordinary JSON-like values. Test a scorer returning a mapping with tuple keys and assert it produces an invalid_score event and completes the run.
Was this helpful? React with 👍 or 👎 to provide feedback.
Follow-up to #63, now merged. Brings the scorer return contract in line with
ai-sdks-monorepoTESTING.md §8.8.4, which was tightened after #63 was written.Problem
A scorer's contract was "a bool or a finite number in
0–1", withTrue/Falsecoerced to1.0/0.0. Two problems with the coercion:It put two score types on the wire. A
thresholdthen 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.It hid bugs, and the failure mode was a green run. Every non-empty value is truthy, so a scorer that returned
"high", or a stray object, scored1.0and passed:A false pass is the one failure mode an evaluation harness must not have — the point of the gate is that a broken check fails loudly.
Change
The contract is now a finite number in
0–1, and nothing else. A scorer answering a yes/no question returns1.0or0.0itself:A
bool, a non-finite number, or one out of range is aninvalid_scoreresult — a per-criterionERRORevent, never a raise, since the row's generation has already been paid for.numeric_score()already excludedbool(Python'sboolis anintsubclass, which is exactly the trap), so the fix deletes the coercion branch rather than adding a check — the two error paths collapse into one whose message names the offending value.ScorerFndropsboolfrom its signature. This documents the contract, but type checkers still accept aboolhere becauseboolis a subclass ofint. The runtime check innumeric_score()is the guard.The first
invalid_scorefrom each scorer in a run also logs a warning that names the scorer and the returned value. A caller who upgrades with bool scorers sees the cause in their own logs.A scorer returning a bool now produces an
invalid_scoreresult instead of1.0/0.0. Callers return the number directly. This is deliberate per §8.8.4 ("there is no boolean shortcut and nothing is coerced on the caller's behalf") — but it is a behavior change and worth a release note.Every scorer in the examples repo is converted to match in launchdarkly-labs/ai-sdk-evaluations-example#4.
Validation
uv run pytest -q— 1264 passed, 11 skippeduv run mypy .../evaluations— clean;ruff check/ruff format --check— cleanTrue,False,NaN,Infinity,3,-1,"high",Noneall produceinvalid_scorewith the value named and noscorefield, and the run still completes, flushes, and polls0,1,0.0,1.0,0.25stay valid, ints included — onlyboolis excluded🤖 Generated with Claude Code
Note
Overview
Breaking: evaluation
Scorerfunctions must return a finite number in0–1;True/Falseare no longer coerced to1.0/0.0and instead emit per-criterioninvalid_scoreERROR events (the run still completes).The runner drops the boolean coercion path and validates all scorer returns through
numeric_score, with a once-per-scorer-per-run warning when a scorer returns something invalid (bools, strings, NaN/Inf, out-of-range).ScorerFntyping and README/Scorerdocs now describe the numeric-only contract and show yes/no scorers returning1.0/0.0explicitly. Tests were updated and expanded for invalid vs boundary-valid scores.Reviewed by Cursor Bugbot for commit 52f69be. Bugbot is set up for automated code reviews on this repo. Configure here.