Skip to content

feat(evaluations): normalize text behaviors - #209

Merged
aknight-ld merged 6 commits into
mainfrom
AIC-3642-match-spec-content-hash
Oct 23, 2026
Merged

aknight-ld merged 6 commits into
mainfrom
AIC-3642-match-spec-content-hash

Conversation

@aknight-ld

@aknight-ld aknight-ld commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Normalizing both python and js SDKs to use a neutral stringify for consistency in content hashing, message history, and judge reasoning. This helps us avoid shoving a bunch of python based conventions into js (and other future builds) and keep our standards language agnostic.

BEGIN_COMMIT_OVERRIDE
feat(evaluations): normalize text behaviors
fix(client): show a judge an output of 0 or false, and render non-string values as JSON
END_COMMIT_OVERRIDE


Note

Overview
Adds canonical_json (RFC 8785 / ECMAScript JSON.stringify rules) so Python matches the JavaScript SDK for offline eval event hashing and judge-facing text.

eventId and contentHash now SHA-256 JCS (canonical_json with sorted keys) instead of json.dumps(..., sort_keys=True). Tests assert digests match the JS SDK.

Judge prompts and errors use judge_text / _describe_value: structured values render as compact JSON.stringify-style JSON (no Python reprs or json.dumps spacing). build_message_history skips only truly empty parts (None, "", {}, []) so 0 and False still appear as answers. parse_judge_response applies the same rendering to reasoning / reason.

Also fixes an opencode-agents test to compare config paths with os.path.abspath.

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

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

The hashing is right. I ran 26 edge cases through this PR's canonical_json and js#155's canonicalJson, and all 26 match byte for byte: 1.0, -0.0, 1e16, 1e21, 1e-7, 5e-324, integers past 2^53, 1e400, lone surrogates, control characters, and keys whose UTF-16 and code-point orders differ. So eventId and contentHash now agree across SDKs. 249 tests pass locally.

Two things before this merges:

1. The judge text still differs between Python and JS (judge_scoring.py:58-62, runner.py:99)

The PR says a rubric now "reads the same text in every SDK", but message_history, the reasoning and the invalid_score message still render through json.dumps. That adds ", " / ": " separators and spells numbers Python's way. The same 26 cases through build_message_history differ from JS in 14. For example, a model answering {"score":1.0,"reasoning":"ok"} gives:

  • Python: {"score": 1.0, "reasoning": "ok"}
  • JS: {"score":1,"reasoning":"ok"}

A judge score of 2.0 reads got 2.0 in Python and got 2 in JS. No test catches this.

Suggested fix: canonical_json.py already has the right encoder. Give _encode a sort_keys flag, and have build_message_history, parse_judge_response and _describe_value render non-string values with canonical_json(value, sort_keys=False). Keep NaN / Infinity as a special case in _describe_value. Leave core tool_result_string alone: it feeds providers, and §1.4 doesn't need byte-identical text there. A shared fixture with js#155 (a dict with a float, a list, non-ASCII) would hold it. I'd also add the cross-SDK requirement to §3.29 in spec ai-sdks-monorepo#85.

2. feat(evaluations) hides a core judges change

build_message_history and parse_judge_response are also called by the online judges in judges.py (lines 184, 233, 441, 509). Online judges now see an output of 0, False, {} or [], and get non-string reasoning as JSON rather than a repr. Under an evaluations scope, release-please lists the whole PR under Experimental, so that core fix never reaches the core changelog (AGENTS.md, "PR Titles and the Changelog"). Could you add this to the description?

BEGIN_COMMIT_OVERRIDE
feat(evaluations): hash eventId and contentHash as RFC 8785 canonical JSON, the same in every SDK
fix(client): show a judge an output of 0, false, {} or [], and render non-string reasoning as JSON
END_COMMIT_OVERRIDE

One thing I didn't check: whether anything compares contentHash across SDK versions. It changes for outputs with floats or non-ASCII text. eventId stays the same for the usual slug-and-integer identities.

jeffdupont and others added 2 commits October 23, 2026 12:13
Judge text (message_history, response_to_evaluate, a non-string reasoning)
and error-message values now use canonical_json(v, sort_keys=False), so they
match the JS SDK: compact separators, insertion-order keys, 1 not 1.0.
_describe_value keeps NaN/Infinity. canonical_json moves out of evaluations
because judge_scoring uses it too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jeffdupont

Copy link
Copy Markdown
Contributor

Pushed 850f353 and eed4743:

  • Judge text (message_history, response_to_evaluate, non-string reasoning) and _describe_value now use canonical_json(v, sort_keys=False), which matches JSON.stringify (no whitespace, insertion-order keys, 1 not 1.0). _describe_value keeps NaN / Infinity. canonical_json moved from evaluations/ to the package root, since judge_scoring uses it too.
  • New tests with explicit expected strings: test_judge_text_is_json_stringify_text and test_describe_value_is_json_stringify_text_except_non_finite. Existing expectations updated to the compact form.
  • Kept the opencode-agents/tests/test_handler.py:1543 change from 4d24d34: process.py passes os.path.abspath(config), so .resolve() only fails where the temp dir is a symlink (macOS /var -> /private/var). It is a correct test fix, unrelated to this PR.
  • Added the requested BEGIN_COMMIT_OVERRIDE lines (replacing the old two) and merged origin/main.

CI is green. Needs a human reviewer.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit eed4743. Configure here.

return value
if value is None:
return ""
return canonical_json(value, sort_keys=False)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Online judges miss new stringify

Medium Severity

judge_text returns a string unchanged, so online invoke still shows a judge json.dumps of a structured response (spaces, 1.0, ASCII escapes). Offline now renders the same object as compact JSON.stringify text, so a rubric can see different conversation text on the two paths.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit eed4743. Configure here.

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.

Fixed in the commit above: config().invoke() now builds the judge's llm_response with judge_text, so online and offline judges read the same compact text. Test: test_online_judge_reads_a_structured_response_as_json_stringify_text.

config().invoke() rendered a structured response for its judges with
json.dumps (spaces, 1.0, ASCII escapes); it now uses judge_text, as the
offline runner does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jeffdupont

Copy link
Copy Markdown
Contributor

Pushed eaa4028 for the Bugbot finding: config().invoke() now builds the judge's llm_response with judge_text instead of json.dumps, so online and offline judges read the same text. New test: test_online_judge_reads_a_structured_response_as_json_stringify_text. CI is green. Needs a human reviewer.

@aknight-ld
aknight-ld merged commit 7cee8c8 into main Oct 23, 2026
7 checks passed
@aknight-ld
aknight-ld deleted the AIC-3642-match-spec-content-hash branch October 23, 2026 19:50
jeffdupont added a commit that referenced this pull request Oct 23, 2026
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>
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