Skip to content

fix(json): stop a skipped sub-score being averaged as a mismatch - #210

Open
Arthi Arumugam (arthi-arumugam-git) wants to merge 1 commit into
braintrustdata:mainfrom
arthi-arumugam-git:fix/json-diff-skipped-scores
Open

fix(json): stop a skipped sub-score being averaged as a mismatch#210
Arthi Arumugam (arthi-arumugam-git) wants to merge 1 commit into
braintrustdata:mainfrom
arthi-arumugam-git:fix/json-diff-skipped-scores

Conversation

@arthi-arumugam-git

Copy link
Copy Markdown

The problem

Score documents the contract:

If the score is None, the evaluation is considered to be skipped.

JSONDiff.json_diff filters those out before averaging, but the two branches then divide by
different things:

# dict
base_scores = [s for s in base_scores if s is not None]
return sum(base_scores) / len(base_scores)          # count AFTER filtering

# list
base_scores = [s for s in base_scores if s is not None]
return sum(base_scores) / max(len(o1), len(o2))     # count BEFORE filtering

In an object a skipped key is removed from the numerator and the denominator, so it is
ignored, which matches the documented meaning. In an array it is removed from the numerator
only, so it is averaged in as a zero.

The same skip therefore lands differently depending on the container:

input scorer skips one value score on main
{"a": "skip", "b": "same"} yes 1.0
["skip", "same"] yes 0.5

Same values, same scorer, same skip.

This is reachable with any scorer that can abstain, which is the case the None score exists
for: an LLM judge that declines to answer, a scorer that cannot parse a field. The array
result is silently lower and nothing raises.

Second, smaller issue

The dict branch divides by len(base_scores) with no guard. An object whose comparisons are
all skipped leaves that list empty and raises ZeroDivisionError. The empty-object case is
handled above it, so this needs a non-empty object rather than an edge case in the inputs.

The change

Both branches now drop skipped comparisons from the denominator, and return None when
nothing is left to average, which propagates the skip upward instead of inventing a number or
raising.

The list denominator stays max(len(o1), len(o2)) minus the skips. That distinction is
deliberate: an element with no counterpart is a genuine difference and must stay counted,
which is what max(...) contributes over the zip. Only the skips come out.

Tests

py/autoevals/test_json_skipped_scores.py, eight tests, four of which fail on main:

FAILED test_skipped_element_in_a_list_is_not_scored_as_zero
FAILED test_list_and_dict_treat_an_identical_skip_the_same_way
FAILED test_a_fully_skipped_object_reports_a_skip_rather_than_raising
FAILED test_a_fully_skipped_list_reports_a_skip_rather_than_raising

The other four pin behaviour that must not move: missing elements are still penalised
(["a"] vs ["a", "b"] is still 0.5), and unskipped lists, dicts and empty containers score
exactly as before.

py/autoevals/test_json.py: 4 passed. Across py/autoevals the results are identical with
and without this diff (21 failed, 39 passed, 8 errors, all needing API keys or litellm).
black and isort clean at line-length 119.

Score documents that "If the score is None, the evaluation is considered to be
skipped", and json_diff filters those out before averaging. The two branches then
disagreed about what to divide by.

The dict branch divided by len(base_scores), the count after filtering, so a
skipped key was excluded from both the numerator and the denominator and
correctly ignored. The list branch divided by max(len(o1), len(o2)), which still
counts the skipped element, so the same skip was averaged in as a zero.

The result is that one skipped comparison scores 1.0 inside an object and 0.5
inside a two-element array, for identical values and an identical scorer.

The dict branch also divided by len(base_scores) with no guard. An object whose
every comparison is skipped leaves that list empty and raises ZeroDivisionError
rather than reporting a skip.

Both branches now drop skipped comparisons from the denominator and return None,
propagating the skip, when nothing is left to average. The list denominator stays
max(len(o1), len(o2)) minus the skips, so elements with no counterpart are still
counted as a real difference; only the skips come out.

Eight tests, four of which fail on main. The other four pin what must not move:
missing elements are still penalised, and unskipped lists, dicts and empty
containers score exactly as before.
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.

1 participant