docs(review-suite): run the preregistered v2 ablation and integration closeout (#57) - #88
Merged
Merged
Conversation
…runs
## Summary
- Add an optional `--skill-root` argument to `runner.py`'s evaluation CLI (and
a `skill_root` keyword on `evaluate()`/`target_skill_documents()`/
`target_skill_prompt()`), defaulting to the real `skills/` tree so every
existing call site and report is unaffected.
- Record the resolved root verbatim as `configuration.skill_root` in every
report, so a run against an alternate root can never be mistaken for a
standard-configuration one.
- Add two eval-only `review-correctness/SKILL.md` overlays under
`review-suite/evals/v2/ablation-skill-roots/{traversal-only,verification-only}/`,
each disabling exactly one of the two required passes (consumer/impact
traversal, verification-sufficiency) for measurement purposes only. Neither
overlay is installed, distributed, or referenced by the shipped skill.
## Why
- #57 (the epic's acceptance-gate ticket) requires scoring
`s1-correctness-orchestrator` in three configurations preregistered by
`gate-manifest.json`: traversal-only, verification-sufficiency-only, and
both together. The shipped `review-correctness` skill deliberately performs
both passes unconditionally, by design (`SKILL.md`: "not routed specialist
modules"), so no runtime toggle exists or should be added to the shipped
skill itself.
- This gives the ablation measurement a way to isolate each pass's
contribution without touching the shipped skill, without duplicating the
full `skills/` tree in git (only the one changed file per configuration is
committed; the rest is mirrored from the real tree at run time), and without
changing behavior for any existing caller of `runner.py`.
## Summary - Backfill 16fc32a's SHA onto the previous top entry and add a new bullet for the skill-root ablation override commit, per AGENTS.md's changelog convention. ## Why - review-code-change's initial pass on this checkpoint flagged the missing entry as a gating strong_recommendation finding; this is the exact, smallest fix.
…ntegration closeout (#57) ## Summary - Score `s1-correctness-orchestrator` in the two ablation configurations `gate-manifest.json` still needed (traversal-pass-only, verification-sufficiency-pass-only), 35 attempts each under the fixed (`1.1`) grader, using the new `--skill-root` override. Reuse the already-run "both passes together" configuration and the `s2`/`s3` non-regression figures from predecessor measurement tasks in this epic rather than re-spending on them. - Add `FROZEN-V2-CONFIGURATION.md` (the exact scored configuration, the reviewed grader-version transition, and the ablation mechanism), `S1-ABLATION-MATRIX.md` (the three-configuration comparison against the settled per-case gate, including the honest finding that neither pass demonstrates unique causal contribution to its own target case, and the real non-regression floor violation found under the isolated verification-only configuration), `DETERMINISTIC-AND-INTEGRATION-EVIDENCE.md` (every required deterministic gate and caller-integration scenario cited against its exact existing #51-#56 test), and `CLOSEOUT-REPORT.md` (the full synthesis, mechanism-by-mechanism recommendation, residual risks, and spend against the $20 ceiling). - Update `review-suite/evals/v2/README.md` and the top-level `README.md` to point at the new scored closeout documents. ## Why - #57 is the epic's (#49) acceptance-gate ticket: it must run the preregistered scored comparison, verify deterministic and caller-integration invariants, and report - per mechanism - whether it earned its complexity. This candidate does the reporting; it does not remove any mechanism or file any follow-up issue, since #57 is explicitly withheld that authority. ## Spend - $7.44 of this ticket's own $20.00 ceiling ($0.009 runtime/model sanity check + $3.560 traversal-only + $3.867 verification-only); $12.56 headroom never approached.
…ation runs
## Summary
- The committed `ablation-skill-roots/{traversal-only,verification-only}/`
directories only ever held the one changed `review-correctness/SKILL.md`
file per configuration, not a full mirror of the four-skill dependency
closure `--skill-root` requires. The two scored runs actually pointed
`--skill-root` at a fresh local mirror of the real `skills/` tree with that
one file overlaid, and each report's own `configuration.skill_root` field
records that ephemeral, machine-local path, not the committed directory.
- Correct `FROZEN-V2-CONFIGURATION.md` and `S1-ABLATION-MATRIX.md` to state
this accurately: what path was actually recorded, why the committed
directories can't be pointed at directly, and the exact steps to reproduce
either scored configuration from the committed overlay files.
## Why
- A fresh full review-code-change pass on the complete candidate found that
the previous wording claimed the committed overlay directories were used
directly as `--skill-root`, which both reports' own `configuration` field
contradicts. The scored numbers themselves are unaffected (the ephemeral
content used is verified byte-identical to the committed overlay files
modulo a later `mdformat` line-wrap pass), but the epic's final,
permanent acceptance-gate record must describe its own methodology
accurately.
Merged
4 tasks
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
Runs the preregistered v2 scored ablation and reports the epic's (#49)
acceptance-gate closeout for #57. Verifies every required deterministic gate
and caller-integration scenario against existing #51-#56 evidence, adds the
one piece of new infrastructure needed to score
s1-correctness-orchestrator'stwo remaining ablation configurations (an eval-only
--skill-rootoverride),runs them for real ($7.44 of this ticket's own $20 ceiling, $12.56 headroom
never approached), and reports a mechanism-by-mechanism recommendation.
This PR reports; it does not remove any mechanism or file any follow-up
issue - both are explicitly withheld authority for #57. #51/#52/#53/#56 are
untouched, and no new GitHub issue was created.
What's new
review-suite/scripts/evals/runner.py: optional--skill-rootoverride(default: the real
skills/tree, every existing caller unaffected), so amechanism ablation can be scored without editing the shipped
review-correctnessskill, which deliberately performs both its requiredpasses unconditionally.
review-suite/evals/v2/ablation-skill-roots/{traversal-only,verification-only}/:two eval-only
review-correctness/SKILL.mdoverlays, each disablingexactly one required pass, used only as scored-measurement fixtures.
review-suite/evals/v2/FROZEN-V2-CONFIGURATION.md,S1-ABLATION-MATRIX.md,DETERMINISTIC-AND-INTEGRATION-EVIDENCE.md,CLOSEOUT-REPORT.md: the frozen configuration, the three-configurationablation comparison, the deterministic/integration evidence citations, and
the full synthesis report.
(
s1-correctness-orchestrator-ablation-{traversal,verification}-only.report.json).Key finding (reported plainly, not smoothed over)
Both
s1target cases pass their settled gate in every configuration tested,including the one where their nominal owning pass is disabled - extending
a predecessor task's own finding that even a pre-#53, zero-pass reviewer
already resolved both cases once graded by the fixed (
1.1) grader. Passingthe numeric gate is real; neither pass demonstrates unique causal
contribution to its own target case on this evidence. Separately, the
verification-sufficiency pass, when isolated (no traversal pass alongside
it), regresses
session-continuation-summary's false-positive floor (3/5against a required 0/5) - a real, disclosed quality cost that does not appear
in the as-shipped combined configuration. See
CLOSEOUT-REPORT.mdfor thefull mechanism-by-mechanism recommendation.
Test plan
just formatjust lint(ruff, mdformat,skills-ref validateon all 8 skills,plugin packaging)
just test(all skill suites + 313review-suite/scripts/teststests)
review-code-changepasses (one on thedeterministic/infra-only checkpoint, one final full pass on the
complete diff) both returned
cleanafter one fix cycle eachgate-manifest.json's pinexactly before any scored spend
baseline/v1/,gate-manifest.json,DECISION-RECORD.md,FAILURE-TAXONOMY.md,audits/confirmed untouchedcarve-changesetsconsulted for its guardrail text only (not invokedto act); candidate classified as fitting the ordinary single-PR path
Closes #57.