fix(review-suite): grader surface-in-prose defect + v2 s1 re-score comparison - #84
Merged
Merged
Conversation
## Summary - `review-suite/scripts/evals/grader.py`: split `match_strength`'s two signals into a public `match_signals(expected, finding)` helper returning `(surface_hit, signal_hit)`, and record, per referred (partial/ambiguous) candidate, whether some referring finding concretely named the candidate's actual surface (`candidate_surface_relevant_root_cause_ids`). `grade()` now additionally reports `referred_relevant_root_cause_ids` (the subset of `referred_root_cause_ids` that passes this guard) and `combined_recall` ((matched + relevant-referred) / expected). - `review-suite/scripts/evals/report.py`: surface the guard at the per-case level as `ever_referred_relevant_root_cause_ids` and `mean_combined_recall`, reading both defensively via `.get(...)` so a grade dict built before the guard existed still reports cleanly. - Add grader and report unit tests covering a referral that fires on the right surface (relevance-guard eligible), a referral that fires on formulation prose alone at the wrong location (not eligible), the always-relevant ambiguous (two full-match) case, and backward compatibility with pre-guard grade dicts. ## Why - A future re-score of `s1-correctness-orchestrator` needs to grade `dependency-strictness-propagation` and `stale-claim-release-guard` against a preregistered gate that allows a case to pass on `mean_recall >= 0.6` OR a relevance-guarded combined matched+referred rate `>= 0.8`. The existing three-way grader already computes `referred_root_cause_ids`, but a referral only earned by matching accepted-formulation prose without ever naming the right file/function/symbol must not count toward that combined rate - the existing tooling had no way to tell the two referral shapes apart before scoring, so this was implemented as the smallest additive check on top of the grader's existing surface/signal split rather than left to be eyeballed during grading. - `GRADER_VERSION` stays `1.0`: the change is additive only (new fields derived from already-computed signals), with no change to any existing matched/missed/referred classification or field, so no corpus's declared `grader_version` needs to move to keep using it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ctness-orchestrator ## Summary - Add `review-suite/evals/v2/s1-rescore-frozen-configuration.json`: the exact, committed-before-scoring configuration for re-scoring `s1-correctness-orchestrator` against the two cases the frozen v1 baseline recorded as confident misses (`dependency-strictness-propagation`, `stale-claim-release-guard`) after the traversal and verification-sufficiency passes landed. - Pins: origin/main's current head at freeze time (85ccf13), the grading-tooling commit adding the referred-path relevance guard (6195ada), runtime/model (claude, CLI 2.1.92, per gate-manifest.json's pin, verified via `claude --version`), 5 runs/case, 450s timeout, $12.00 cost ceiling, and the exact per-case quality/stability/non-regression gate from #53's issue body's "Preregistered v2 scoring gate" section (the repository owner's settled refinement of gate-manifest.json's proposal). ## Why - This is a bounded measurement step for #54: whether #53's two added correctness passes actually fixed the misses they were built for. Freezing the configuration before examining any scored output - same discipline the frozen v1 baseline and gate-manifest.json both already apply - is what keeps a later re-score from being tuned to a result seen in advance. - Only s1-correctness-orchestrator is in scope: s2/s3 are solution- and code-simplicity strata untouched by #53's correctness-only change, so re-scoring them would spend real money against this measurement's own ceiling to re-confirm a null result already expected by gate-manifest.json's non-regression floor. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e hit ## Summary - `review-suite/scripts/evals/grader.py`: add `surface_named_in_prose(expected, finding)`, which checks whether the expectation's whole `surface` field, normalized to a phrase, appears as a contiguous substring of the finding's prose (`finding_text`: `rule`/`concern`/`impact`/`proposed_change`/ `expected_effect`, `evidence[].detail`). `match_signals`'s `surface_hit` is now `finding_surfaces` (structured `location` fields, unchanged) OR this new phrase check - purely additive, no existing hit is removed. - Bump `GRADER_VERSION` to `1.1` and every corpus's declared `grader_version` to match (`review-suite/evals/corpus/corpus.json`, `review-suite/evals/calibration/rollback-guidance-render.json`, and every `review-suite/evals/strata/*/corpus.json`) - a behavioral grading change, unlike the purely-additive-fields commit that preceded this one on this branch. - Add `review-suite/scripts/evals/regrade.py` (+ `review-suite/scripts/tests/test_eval_regrade.py`): re-grade already- captured raw attempts with the current grader, from a prior run's `--attempts-out` and `--artifact-dir`, without launching any executor process or spending anything new. Exposed as `just regrade-review-suite`. - Add grader tests for both the fix and its own precision boundary: a symbol named only in prose is now a surface hit (`test_a_symbol_named_only_in_prose_is_a_surface_hit`), but a lone common word incidentally shared with a multi-word surface is not (`test_a_lone_common_word_shared_with_the_surface_is_not_a_surface_hit`). ## Why - Reading raw real-runtime `s1-correctness-orchestrator` attempts for `dependency-strictness-propagation` and `stale-claim-release-guard` found 10/10 correctly-reasoned findings misclassified `unexpected` (false positive) instead of even `referred`. Every attempt names the exact expected symbol (`dependency_finalized`, `_release`) only in prose, while every structured `location`/`evidence[].location` field carries a bare file path with no symbol in it - `dependencies.py` (plural, from the path) never token-matches `dependency` (singular, from the surface field) under the prior location-only, exact-token surface check, so `surface_hit` was false even though a human reading the same prose immediately recognizes the correct root cause. - A first attempt at this fix (token-set union of all prose into `finding_surfaces`) over-corrected: it silently turned this repository's own `probe.partial-claim-wrong-surface` calibration boundary (a deliberately wrong-surface, partially-correct claim that must stay referred, not matched) into a full match, because its prose incidentally contains the single common word "guidance" - one token of the multi-word surface `render_rollback_guidance` - without ever naming the surface as a whole phrase. Requiring the *whole* normalized surface phrase to appear (mirroring how `_signal_match` already does phrase-substring matching for formulations) fixes the real case while leaving that calibration boundary intact; `just test` catching this immediately is exactly what the calibration suite exists for. - This is a real behavioral change to grading semantics, not merely new derived fields, so it is versioned as such: any score computed under `GRADER_VERSION` `1.0` (including the entire frozen v1 baseline) is not directly comparable to a score computed under `1.1`. Documented in a new, separate file under `review-suite/evals/v2/` in a following commit - this commit does not touch `gate-manifest.json`, `DECISION-RECORD.md`, `FAILURE-TAXONOMY.md`, or any audit file, all still frozen and not owned by this change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…or s1 ## Summary - `review-suite/evals/v2/s1-correctness-orchestrator.report.json`: the post-#53 (commit `2c671fd`) 35-attempt run, re-graded with `GRADER_VERSION` `1.1` via `regrade.py` (no new executor calls). - `review-suite/evals/v2/pre53-s1-correctness-orchestrator-2case.report.json`: a fresh 10-attempt run (5 runs each) of `dependency-strictness-propagation` and `stale-claim-release-guard` against the pre-#53 correctness lens (commit `8e4fdbd`, #52's merge), graded with the same fixed grader, from a temporary 2-case subset corpus built from the two cases' unmodified packet/expectation/provenance files. - `review-suite/evals/v2/GRADER-1.1-COMPARABILITY.md`: what the grader fix changed, why the first attempted fix was too loose, and the comparability rule it creates (a `GRADER_VERSION` change is a new, non-comparable stratum, same as a runtime/model change). - `review-suite/evals/v2/s1-rescore-pre-post-comparison.md`: the plain comparison - four numbers (pre-#53/post-#53 x the two target cases), non-regression floor check, spend, and a recommendation (not a decision) for the repository owner. ## Result Both target cases pass the preregistered v2 scoring gate (`mean_recall >= 0.6` OR guarded `mean_combined_recall >= 0.8`) in both eras: `mean_combined_recall: 1.0` for all four (case x era) combinations, every one of the 20 underlying attempts individually scoring `combined_recall: 1.0`. The non-regression floor holds throughout. Total spend across the whole measurement (35 post-#53 + 10 pre-#53 attempts): $4.38 of the $12.00 ceiling. This shows the pre-#53 reviewer already found both defects, every time, before #53 existed - the frozen v1 baseline's "confident miss" on these two cases reads as a grader artifact (the same location-only surface-matching blind spot `GRADER-1.1-COMPARABILITY.md` documents), not a demonstrated reviewer capability gap. Neither `baseline/v1/` nor `gate-manifest.json`, `DECISION-RECORD.md`, `FAILURE-TAXONOMY.md`, or any audit file is modified; this adds new files alongside them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
## Summary
- `surface_named_in_prose` now delegates to the existing `_signal_match`
(called with a single-item `[expected.get("surface", "")]` formulations
list) instead of reimplementing the same normalize-then-substring check a
second time.
## Why
- Found in review-code-change's code-simplicity pass on this branch:
`surface_named_in_prose`'s body was behaviorally identical to
`_signal_match([surface], text)` in every case checked (the real
dependency_finalized/_release matches, the render_rollback_guidance
wrong-surface calibration case, and both empty/missing surface fields),
including the same falsy-formulation guard that makes an empty `surface`
never match. Two separately-named implementations of the same rule is one
more concept than a maintainer needs to hold, and a future change to the
matching rule would otherwise have to be made in both places.
- Purely internal: no observable behavior changes. `just test` (252 tests)
passes unchanged before and after.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
shaug
added a commit
that referenced
this pull request
Jul 29, 2026
…e surface-in-prose defect (#85) ## Summary - Confirm no v1 raw attempt data survives for s2-solution-simplicity-lens or s3-code-simplicity-lens (searched machine-wide; only unrelated pilot-stratum artifacts remain), matching s1's documented pattern. - Run both strata fresh (40 attempts, 5 runs/case) under the runtime/model pin unchanged (claude 2.1.92, claude-opus-4-6[1m]) and the fixed grader (1.1, commit a247d47). - Re-grade the same fresh raw attempts with the pre-fix grader (85ccf13) as a direct counterfactual: three of the four material cases were already correctly routed to `referred` under the old grader and show no material difference; `setup-service-path-gateway` is materially affected by the same surface-named-only-in-prose defect s1's two cases had, at the referred-vs-false-positive boundary rather than a full blind miss. - Confirm the four clean/negative controls are unchanged (zero false positives, perfect stability, both eras). ## Why - The grader defect fixed in #84 was known to reach s1's two target cases; whether it also reached s2/s3 (whose material cases already showed non-empty referred lists in v1, unlike s1's fully-blind cases) had never been checked. This measurement checks it directly instead of assuming the answer, and finds a real but smaller-scope effect on one case (`setup-service-path-gateway`'s false-positive rate) rather than a clean "unaffected" result. Spend: $2.15 of 40 attempts, against this task's $5.00 ceiling. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bounded measurement step for #54's evidence gate: re-score
s1-correctness-orchestrator(only;s2/s3are untouched by #53'scorrectness-only change) against the frozen v1 baseline's two confident
misses,
dependency-strictness-propagationandstale-claim-release-guard,after #53's traversal and verification-sufficiency passes landed.
While re-scoring, raw attempts showed 10/10 correctly-reasoned findings on
both cases misclassified as false positives, not because the reviewer was
wrong but because
grader.py's surface-hit detection only read structuredlocationfields and every real attempt named the affected symbol only inprose. That is a real, pre-existing grader defect (present since the
grader's original commit, not introduced by #53 or by this branch's own
relevance-guard commit) - fixed here, then used to re-grade both the
post-#53 attempts already captured and a fresh pre-#53 (commit
8e4fdbd,#52's merge) run of the same two cases, for a genuine apples-to-apples
comparison.
Changes
review-suite/scripts/evals/grader.py: a referred-path relevance guard(
referred_relevant_root_cause_ids,combined_recallongrade()) so areferred candidate only counts toward the preregistered v2 gate's
alternative pass path when it concretely names the actual surface, plus a
real fix (
surface_named_in_prose) recognizing a symbol named only in afinding's prose - constrained by, and verified against, this repository's
own
probe.partial-claim-wrong-surfacecalibration boundary.report.pysurfaces both at the per-case level.
GRADER_VERSION1.0->1.1(behavioral, versioned accordingly; every corpus/calibration file's
declared version bumped to match).
review-suite/scripts/evals/regrade.py(new) +just regrade-review-suite: re-grade already-captured raw attempts with thecurrent grader, no new executor calls, no new spend.
review-suite/evals/v2/(new files only - nothing underbaseline/v1/,gate-manifest.json,DECISION-RECORD.md,FAILURE-TAXONOMY.md, oraudits/was touched):s1-rescore-frozen-configuration.json- the re-score configuration,committed before any scored output was examined.
s1-correctness-orchestrator.report.json- post-Add correctness traversal and verification-sufficiency passes #53, 35 attempts,re-graded with the fixed grader.
pre53-s1-correctness-orchestrator-2case.report.json- a fresh 10attempts pre-Add correctness traversal and verification-sufficiency passes #53, graded with the same fixed grader.
GRADER-1.1-COMPARABILITY.md- what changed and why a grader-versionchange is a new, non-comparable stratum (extending the existing
runtime/model-change rule).
s1-rescore-pre-post-comparison.md- the four-number comparison,non-regression floor check, spend, and a recommendation (not a
decision) for the repository owner.
Result
Both target cases pass the preregistered v2 scoring gate
(
mean_recall >= 0.6OR guardedmean_combined_recall >= 0.8) in botheras -
mean_combined_recall: 1.0for all four case x era combinations,every one of the 20 underlying attempts individually at
combined_recall: 1.0. The pre-#53 reviewer already found both defects, every time, before#53 existed: the frozen v1 baseline's "confident miss" reads as the same
grader artifact this PR fixes, not a demonstrated capability gap. Full
detail, caveats, and the recommendation in
review-suite/evals/v2/s1-rescore-pre-post-comparison.md.Non-regression floor holds throughout (verdict/finding stability at the v1
floor of 1.0 on both target cases; the three clean controls at zero false
positives;
process-isolation-assertion's false-alarm rate improved from2/5 to 0/5).
Total spend: $4.38 of the $12.00 ceiling (35 post-#53 attempts + 10 fresh
pre-#53 attempts; re-grading both with the fixed grader cost nothing
further).
Review
One
review-code-changecycle: solution-simplicity and correctness cleanon the first pass; code-simplicity raised one real finding (a new helper
duplicated an existing one's exact behavior instead of reusing it) -fixed
in the second commit of this PR, re-verified clean.
Test plan
just formatjust lintjust test(252 tests,review-suite/scripts/tests)attempts, both graded with the fixed grader