Skip to content

fix(evaluations): reject boolean scorer scores - #92

Open
donei003 wants to merge 1 commit into
mainfrom
fix/scorer-rejects-boolean-scores
Open

donei003 wants to merge 1 commit into
mainfrom
fix/scorer-rejects-boolean-scores

Conversation

@donei003

@donei003 donei003 commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up to #63, now merged. Brings the scorer return contract in line with ai-sdks-monorepo TESTING.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", with True/False coerced to 1.0/0.0. Two problems with the coercion:

It 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.

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, scored 1.0 and passed:

# Before: scored 1.0, row passed, no error anywhere.
Scorer(name="quality", fn=lambda row, output: grade(output))  # grade() returns "high"

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 returns 1.0 or 0.0 itself:

def mentions_policy(row: DatasetRow, output: str | None) -> float:
    return 1.0 if "refund policy" in (output or "").lower() else 0.0

A bool, a non-finite number, or one out of range is an invalid_score result — a per-criterion ERROR event, never a raise, since the row's generation has already been paid for.

numeric_score() already excluded bool (Python's bool is an int subclass, 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.

ScorerFn drops bool from its signature, so a caller who annotates 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. 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 skipped
  • uv run mypy .../evaluations — clean; ruff check / ruff format --check — clean
  • New parametrized coverage: True, False, NaN, Infinity, 3, -1, "high", None all produce invalid_score with the value named and no score field, and the run still completes, flushes, and polls
  • Boundary coverage: 0, 1, 0.0, 1.0, 0.25 stay valid, ints included — only bool is excluded
  • The suite's own scorers were converted from bool to numeric, which is the change callers make

🤖 Generated with Claude Code


Note

Overview
Breaking: offline evaluation Scorer.fn must return a finite number in 0–1; the SDK no longer coerces True/False to 1.0/0.0.

The runner drops the boolean branch and validates everything through numeric_score(), so bool, strings, None, non-finite values, and out-of-range numbers emit a per-criterion invalid_score ERROR (run still completes). ScorerFn is typed as float | Awaitable[float] only, and README / Scorer docs tell callers to use explicit 1.0/0.0 for yes/no checks.

Tests and examples are updated accordingly, with new parametrized coverage for rejected vs accepted return values.

Reviewed by Cursor Bugbot for commit 0bc9544. Bugbot is set up for automated code reviews on this repo. Configure here.

@donei003
donei003 marked this pull request as ready for review September 21, 2026 22:55

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

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>
@donei003
donei003 force-pushed the fix/scorer-rejects-boolean-scores branch from db9bfbc to 0bc9544 Compare September 30, 2026 19:08

@jeffdupont jeffdupont left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 bool from ScorerFn means "a caller who annotates their scorer sees this at type-check time". That isn't true. I ran mypy --strict against this branch: a def f(...) -> bool scorer and lambda row, output: True both pass, and only a -> str scorer is flagged. bool is a subclass of int, and type checkers accept int where float is 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 a BREAKING CHANGE: footer. The coercion shipped in launchdarkly-ai-server 0.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 go ERROR with no note telling them why.
  • Optional: on that upgrade path the only signal is passed=False and error_rows in the summary. _criterion_error_result (runner.py:818) doesn't log anything locally. A logger.warning once per scorer on invalid_score that names the returned value would make the bool case obvious from the caller's own output.

from .types import DatasetRow

type ScorerFn = Callable[[DatasetRow, Any], float | bool | Awaitable[float | bool]]
type ScorerFn = Callable[[DatasetRow, Any], float | Awaitable[float]]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Worth narrowing, but this won't catch a bool scorer at type-check time. I ran mypy --strict on this branch: Scorer(name="a", fn=yes_no) where yes_no returns bool, and fn=lambda row, output: True, both pass. Only a -> str scorer errors. bool is a subclass of int, and int is accepted where float is expected, and typing has no way to say "float but not bool". So the runtime numeric_score check is the real guard. Could the PR body and squash message drop the type-check claim?

# 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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is the right shape: one path, and numeric_score already excludes bool. One upgrade concern. A caller whose scorers return bools today (this shipped in 0.2.4) will get ERROR on every row and nothing in their local logs, because _criterion_error_result only builds the dict. A one-time logger.warning per scorer on invalid_score that names the value would make that obvious. Not blocking.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants