Repository navigation
fix(evals): take the midpoint of the two middle ratings for Critical Thinking Overall - #420
Conversation
…Thinking Overall With one source passage, indicator 2.1 is omitted and Overall is the median of four ratings. Rounding that median up to the higher middle level let a strong essay with fabricated citations (Exploring, Exploring, Extending, Extending) land on Extending. Overall now takes the level halfway between the two middle ratings, rounding up only when no level sits exactly halfway. Neighboring ratings resolve as before. - system.txt: rewrite decision rule 6 with worked examples; update its hash. - fixtures.json: add a single-source fabricated-citation case expecting integrating. - config.json: set allow_adjacent_levels to false so that case needs an exact match; a neighboring level would accept the slide back to Extending. - SDK tests: the median helpers follow the new rule, and both live suites read the contract's tolerance for Critical Thinking. Refs DSCR-2633 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain for the midpoint-scoring update.
No open issues 🎉
What changed in this PR
Updates Critical Thinking’s Overall rating to use the midpoint of the two middle indicator levels for even counts, rounding upward only when the midpoint falls between levels. Odd-count behavior remains unchanged.
Changes:
- Revises the evaluator prompt and synchronizes the generated Python contract and prompt hashes.
- Adds a single-source fabricated-citation regression fixture and requires exact fixture matches.
- Updates Python and TypeScript scoring helpers, midpoint unit cases, and integration-test tolerance handling.
| File | Description |
|---|---|
sdks/typescript/tests/unit/evaluators/critical-thinking-score.test.ts |
Implements midpoint scoring in the helper and adds unit cases. |
sdks/typescript/tests/integration/critical-thinking.integration.test.ts |
Reads adjacency tolerance from the contract. |
sdks/python/tests/unit/evaluators/durable_skills/ela_writing/test_critical_thinking_score.py |
Implements midpoint scoring in the helper and adds unit cases. |
sdks/python/tests/integration/test_pilot_evaluators.py |
Honors Critical Thinking’s configured adjacency tolerance. |
sdks/python/src/learning_commons_evaluators/contracts/_generated/durable_skills/ela_writing/critical_thinking/system.txt |
Synchronizes the generated prompt’s midpoint rule. |
sdks/python/src/learning_commons_evaluators/contracts/_generated/durable_skills/ela_writing/critical_thinking/config.json |
Synchronizes the prompt hash and exact-match tolerance. |
evals/durable-skills/ela-writing/critical-thinking/system.txt |
Defines the midpoint rule with examples. |
evals/durable-skills/ela-writing/critical-thinking/fixtures.json |
Adds the fabricated-citation case expecting integrating. |
evals/durable-skills/ela-writing/critical-thinking/config.json |
Updates the prompt hash and disables adjacent-level matches. |
Review effort: Balanced
There was a problem hiding this comment.
Reviewed against DSCR-2633. The PR covers every item in the ticket's scope and acceptance criteria, CI is green, and the validation is upfront about its limits. I'm fine with this shipping Wednesday. A couple of things I'd recommend:
Suggested change (non-blocking):
Rule 5 says "can live", rule 6 is now prescriptive, and both live suites assert equality. Rule 5 reads "Overall Critical Thinking can live at the median", while the SDK tests now assert the model's Overall equals the helper's median on every run. The permissive wording invites the model to deviate, and the suite fails when it does. Since rule 6 is being rewritten anyway, tighten rule 5 to "is the median". (Prompt change, so the hash moves again; a few more runs on the fabricated case would be good.)
Not sure, want to confirm:
The new Python integration branch confirmation The exact-match path in test_pilot_evaluators.py only runs when all three attempts miss, and live tests weren't run in CI. The accessor is safe (Fixtures.tolerance is a plain dict), but please confirm the Python live suite was run locally for Critical Thinking, since validation used standalone scripts. Also, when exact match is required the failure message should say so; "expected integrating, got ['extending', ...]" otherwise reads like a flake.
Questions on the validation
- How many of the 15 Round 3 essays were single-source? The rule never fired on them, so the regression shows the prompt edit didn't perturb indicator ratings (useful), but says nothing about how often the midpoint rule fires on the population it targets.
- Across the 10 new-prompt runs on Mike's essay, what was the indicator split each run? "10/10 Integrating, self-consistent" is compatible with Exploring/Exploring/Extending/Extending every time, and also with 3.1 or 4.1 drifting down. The first means the new fixture is fragile under exact match (one indicator moving one level flips Overall to Analyzing); the second means it's more robust than it looks.
Design notes, non-blocking
- The fixture's expected label is the rule's output, not an expert label. The other two fixtures carry Round 3 consensus; this one's
integratingis what the split resolves to under the new rule. Legitimate as a regression guard, but a different kind of fixture, and nobody has asked an expert whether an essay that fabricates every citation and its opposing view should be Integrating. Flag it in the description as a rule guard, or get one expert rating. The one-line source passage and the obviously non-student prose are fine for an adversarial case but not representative. - Exact match is now evaluator-wide to protect one fixture. Keep the semantics sentence in
notesalongside the rationale. Longer term a per-fixture tolerance override would let the expert-labeled fixtures keep adjacency. - This treats the symptom. The split happens because rules 10 and 15 penalize fabrication only in 2.2 and 3.2, rule 18 exempts 4.1, and nothing gates 3.1 (the essay invents "Coach Reyes" and still earns Extending there). Smallest safe change for Wednesday, agreed. But worth recording on the ticket that the midpoint rule doesn't touch multi-source prompts (five ratings, odd count), and that the durable fix is either 3.1 sourcing rules or DSCR-2634.
Nits
test_four_ratings_round_up/ its TS twin now test the neighbors case under a name describing the old rule.- The Python special-casing by evaluator id could read
allow_adjacent_levelsfrom every contract, defaulting to true when absent, with no behavior change for the text-complexity and graphics contracts (alltrue). - Rule 6 wording and the four worked examples (gaps 1 to 4) are clear. Generated Python copies match the source hash.
… rule-guard fixture - config.json: the tolerance note states what the flag means again, then why Critical Thinking turns it off. - fixtures.json: the fabricated-citation case says its expected label comes from the midpoint rule, not from expert raters. - Python live suite: every contract's allow_adjacent_levels decides whether a neighboring level passes (absent means it does, so other evaluators are unchanged), and the failure message says when an exact match was required. - Rename the neighboring-middle-ratings unit test in both SDKs. Refs DSCR-2633 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Answers to the validation questions in the review summary:
🤖 Addressed by Claude Code |
Rule 5 said Overall "can live at" the median, which read as optional next to rule 6 and the tests, which require it. It now says Overall is the median. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
More runs on the final prompt (2a82580) to put numbers on the remaining review questions. Every run below is a single attempt with no retry. How often each fixture matches exactly on one attempt (the notebook case, and how fragile exact match is)
Each count is 10 new runs plus the notebook run on this commit, and the fabricated case also includes the 10 rule 5 validation runs. Overall equalled the midpoint of that run's own indicators in all 30 new runs. With zero misses in 11 runs the per-attempt miss rate could still be as high as about 25% (95% bound), so for the suites, which retry 3 times, the worst case is about a 2% chance per fixture of missing on all three. For the notebook, which tries once and is advisory, a miss would show up as one How often the midpoint rule applies to single-source essays 30 single-source PERSUADE 2.0 essays not used before (15 Driverless cars, 15 Face on Mars), grades 8 to 10, two per holistic score 1 to 6 plus extras at 1 to 3, one run each:
So on ordinary student essays the rule seems to apply rarely: a ratings split that wide looks specific to essays like the fabricated-citation one, which is strong on some skills and weak on others. That also makes it unlikely the change moves Overall scores on typical single-source essays. Still open, and runs can't answer it: the exact-match failure branch in Cost of these 60 calls: about $8.65. 🤖 Addressed by Claude Code |
adnanrhussain
left a comment
adnanrhussain
left a comment
There was a problem hiding this comment.
Prompt changes were already reviewed by DS.
Refs DSCR-2633.
Why
With one source passage, indicator 2.1 (synthesizing_sources) is omitted, so Overall is the median of four ratings. Decision rule 6 rounded that median up to the higher of the two middle levels. A strong essay whose citations don't appear in its only source could be rated Exploring, Exploring, Extending, Extending and get Overall = Extending.
What changes
Overall is now the level exactly halfway between the two middle ratings, rounding up only when no level sits exactly halfway (
ceil((a + b) / 2)on the scale). Odd counts are unchanged.system.txt: rule 6 rewritten with these examples;config.jsonhash updated; Python contracts regenerated.fixtures.json: addsfabricated_citations_single_source, expectingintegrating.config.json:allow_adjacent_levelsis nowfalsefor Critical Thinking. A neighboring level would accept a slide back to Extending on the new case. Both live suites now read this setting for Critical Thinking (TS already claimed to; Python hard-coded adjacency). The two existing fixtures came out exactly as expected in every validation run, so exact match doesn't make them flaky.median_rating/medianRating) follow the new rule, with unit cases for each row above. The live suites use them to check that every run's Overall matches its own indicator ratings.Validation
Python SDK, built from
main(current prompt) and from this branch (new prompt). Claude Opus 5 has no temperature control, so results are over repeated runs.Fabricated-citation case, 10 runs per prompt
The new rule changed the result on all 10 new-prompt runs; the old rule would have given Extending on each.
Regression: 15 expert-annotated essays (84 essay × dimension cells)
Two runs per essay per prompt, a third where they disagreed, then the round-up median of the runs per cell.
No run on these 15 essays had middle ratings more than one level apart, so the new rule never fired on them, and Overall is unchanged. The three cells that differ are indicator ratings, which rule 6 doesn't touch; about 11 of 84 cells vary between runs of the same prompt.
Repo checks,
make verifyand the TS lint, typecheck and unit suites pass. Live integration tests were not run in CI (no key).Moving the Overall computation into code is a separate follow-up (DSCR-2634).
🤖 Generated with Claude Code