feat: populate the correctness stratum with adjudicated review cases - #66
Merged
Conversation
Batch 2 of #58's corpus population. Populates `s1-correctness-orchestrator` with the seven case classes #58 requires, every one minimized from a real review disposition in the public `shaug/atelier` or in this repository's own delivery history. Three accepted material escapes: - `dependency-strictness-propagation` — a decision helper gains a strict mode, one caller adopts it, and the untouched sibling caller keeps deciding the same question permissively. atelier PR 335, accepted in `5cb0333`. - `stale-claim-release-guard` — a collect-then-apply guard compares ownership only when the captured owner was present, so the unowned-at-capture interleaving bypasses it and releases a live claim. atelier PR 373, accepted. - `optional-tool-probe` — an optional-tool probe checks only the return code, but a missing executable raises, so the intended skip never happens in the one environment it exists for. This repository's `f544aa0`, which survived an aggregate `clean` review verdict and was then caught by CI. Four clean-expected controls, all built to the owner's settled standard: - `session-continuation-summary` — a summary flag challenged as inaccurate, kept on the merits because it is the loop-continuation signal, with only the operator wording corrected. atelier PR 486. - `dependency-hint-parser-coverage` — a coverage request for a deliberately retired alias, declined on the merits. atelier PR 279, declined in reply 2862037362. - `post-bootstrap-module-load` — a deferred module load flagged as a strange pattern, adjudicated intentional with a comment as the whole resolution. atelier PR 710. - `process-isolation-assertion` — a test whose name overstates what it proves, raised by a real review and dispositioned `defer`. This repository's #50, item 4 of PR #61's preserved deferred list. Five cases carry multi-file diffs, and three of them are only decidable by reading a consumer the diff does not touch, which satisfies #58's multi-file minimum. The owner settled the standard: a clean control must be a case where a finding was actually raised and dispositioned as not material. The recorded rejection is the evidence, and a reviewer that re-raises it is charged a false alarm. Each clean case therefore records its rejected concern as an accepted non-finding with the formulations a reviewer would use, so a non-gating mention is tolerated while a gating one is charged. The rejected concern is deliberately *not* treated as a root cause: raising it is the error being measured. Two previously identified clean-control candidates were dropped for failing this standard, and two more that looked usable were dropped after checking their dispositions; the sourcing record names all four and why. The stratum declares `scored: false` and every expectation declares `calibrated: false`, both honestly. Nothing here may be scored yet, for two independent reasons: no expectation has a second independent adjudication, and none is calibrated. Batch 1 measured what an uncalibrated expectation reports — recall 0.0 against a reviewer that found the defect on every attempt — so scoring this stratum as it stands would produce a number about the corpus rather than about the reviewer. No case in this stratum has been run through any runtime. They are unobserved, and must stay that way until the calibration-versus-result-blindness question is settled.
## Summary - Add `review-suite/scripts/evals/oracles/`: one runnable module per case exposing the reproduction as the candidate leaves it, the corrected reproduction where a root cause exists, and the packet's requirement expressed as code. - Add `test_eval_oracles.py`, which runs each requirement and asserts the polarity the case's expected verdict demands. - Record adjudication structurally: `provenance.adjudication` names the first adjudication and whether the second is an `oracle` or `owner_required`. ## Why an oracle rather than a second opinion The requirement is two independent adjudications. The first is the recorded source disposition. The obvious candidate for the second — a fresh agent context — is not independent of a reviewer it shares a model family with: the errors correlate, so a defect that model class cannot see becomes a defect the adjudicator agrees is immaterial, and recall rises without the reviewer improving. A machine that runs the code has no such correlation. So for a correctness case, materiality stops being an opinion. The stated requirement either holds against the candidate or it does not. ## What the tests assert | expected verdict | on `candidate()` | on `corrected()` | | ------------------ | ------------------------- | ------------------------- | | `changes_required` | the requirement **fails** | the requirement **holds** | | `clean` | the requirement **holds** | not applicable | The `corrected` leg is the part that makes this an adjudication of the *root cause* rather than of a symptom: correcting the cause the expectation names is what must make the requirement hold. A case whose requirement fails for some unrelated reason fails here rather than shipping as evidence. All seven correctness cases are oracle-settled: three gating cases where the requirement fails and correction fixes it, and four clean controls where the stated contract holds. A test requires every case in that stratum to ship one, so a case cannot be silently routed to the owner when a machine could have settled it. ## What an oracle does not prove It covers the requirements the change contract states, and nothing else. A clean control's oracle proves the contract holds and the raised concern was adjudicated immaterial; it does not prove the diff is free of every possible defect. Two cases record this explicitly: the oracle cannot adjudicate an aesthetic objection to a field's name, nor whether a test's name overstates what it asserts, because those are judgements about prose. `process-isolation-assertion` records a further caveat an adjudicator has to weigh: its first adjudication came from the review suite under evaluation, so a `defer` from the same contract being measured is weaker evidence than a human disposition. ## Where no oracle exists The simplicity strata have none — "this is over-engineered" has no executable form — so those cases will declare `owner_required`, and a test holds them to it rather than letting them claim two adjudications they do not have.
…s limits - Record the seven delivered cases in `SOURCING.md` with their source dispositions, and record all five candidates dropped for failing the clean-control standard. - Record the oracle outcome per case in `ADJUDICATION-PLAN.md`, including the three limits of what an oracle can settle and the three expectations that verification reversed. - Add limitations 13 to 15: false-alarm rate is a lower bound on invention; a scored stratum cannot be both calibrated and result-blind; an oracle adjudicates a requirement, not a diff. - Update `frozen-configuration.json`: the ceiling is preregistered, the clean-control standard is settled, and the remaining blocker is named. - Apply the two factual corrections carried over from batch 1. **Cost ceiling** is now recorded as preregistered at 9.00 / 3.00 / 3.00 USD, 15.00 total, with the hard-stop semantics stated: exceeding it stops further runs in that stratum and records incomplete evidence rather than quietly reducing repetitions. **Clean-control standard** — a clean control must be an adjudicated-rejected finding — invalidated five candidates, and the record says so rather than quietly replacing them. Both of batch 1's clean-control candidates failed: one rested on the reviewer praising a hunk, the other was an accepted *fix*, and acceptance says a concern was material, which is the opposite of what a clean control needs. Two negative-control candidates turned out on re-check to have been **accepted**, not deferred. A fifth was declined and then implemented in another form, so its disposition is acceptance too. Verification also reversed the plan in the other direction: atelier PR 335, which batch 1 recorded as lacking a verified acceptance, **was** accepted, which makes it a valid escape and now the multi-file propagation case. The standard measures whether a reviewer re-raises a finding a human already rejected. It does **not** measure whether a reviewer invents a novel finding on a wholly clean diff. A reviewer could score 0.0 across every control here and still gate freely on diffs where nothing was ever raised. Limitation 13 states this and requires the figure to be reported as a lower bound on invention rather than as a general false-alarm rate. Limitation 14 is new and it is not a caveat, it is a blocker. Grader matching is containment, so an uncalibrated expectation reports recall 0.0 against a reviewer that is right every time — batch 1 measured exactly that. But calibrating a case means observing its prose, and observing a scored case's prose then tuning its formulations is fitting the grader to the answer. So the stratum ships populated, `scored: false`, every expectation `calibrated: false`, and **no case run through any runtime**. Four resolutions are set out for the owner to preregister, with their costs: rely on transfer and accept a downward-biased recall; split each class between calibration and scoring; report referrals as a first-class bucket; or replace containment matching, which belongs to #59.
Closes both blocking findings and the strong recommendation from the first review cycle. One of them was a real contamination defect in a shipped case. ## The defect `optional-tool-probe` is the stratum's only validation-gap escape. Its packet said ``` `subprocess.run` raises `FileNotFoundError`, a subclass of `OSError` ```, and its private expectation accepted `subprocess.run raises FileNotFoundError` as a matching formulation. A reviewer could therefore have earned full recall by quoting its own input, and the case would have measured whether a reviewer echoes the packet rather than whether it finds the escape. The contamination audit did not catch it, and that is the more important half. The audit's text search folds case and whitespace but keeps punctuation; the grader folds punctuation away. The formulation was invisible to one and a perfect match for the other. **Whatever the grader treats as the same text is what decides a score, so an audit using a stricter definition of "the same text" than the grader is not auditing the thing that matters.** The two now share one normalisation, imported rather than restated so they cannot drift apart again. Scoped to `material_root_causes`. An accepted non-finding's formulation often does legitimately restate reviewer-visible content, and matching one only makes the grader more tolerant of an observation already judged immaterial — it cannot manufacture a correct answer. Two regression tests pin both halves: one echoes a root-cause formulation into its own packet with punctuation between every word and requires the audit to reject it, the other echoes an accepted non-finding's formulation and requires the audit to allow it. The packet keeps both environment facts a reviewer needs — that the validating machine has the tool and that CI does not — and now points at the repository's existing optional-tool probe as the worked example, instead of naming the exception. The defect remains fully derivable; it is no longer quotable. ## Also fixed - **The stratum purpose contradicted the corpus.** It still read "none has a second independent adjudication", written before the oracles landed one commit later, while every provenance record carries `adjudication.second: "oracle"` and four other records say adjudication is satisfied. The runner copies that block verbatim into every report, so the false claim would have travelled. It now names calibration as the single remaining blocker. - **A non-finding cited a packet fact that was not there.** The tolerance for an unscoped artifact glob was justified by "the packet scopes the directory to the run under test", which the packet does not say. Reworded to justify the tolerance on its own terms rather than on an invented fact; the packet is unchanged. ## Not covered, and why No mechanical check can catch a packet that describes the defect in words the formulations do not use — the packet is supposed to describe the change. That stays a curation judgement, with the oracle as the backstop: a case whose stated root cause is not the real cause fails its oracle regardless of how its packet is worded.
…record Closes the blocking finding and both strong recommendations from the second review cycle, which was the first cycle in which `review-code-simplicity` ran at all. ## The blocking defect: a packet asserting a suite it could not have `dependency-strictness-propagation` guarded its corrective action on the wrong side of a negation. `reconcile_closed` called `reopen_if_unlanded` only when integration *was* proven, so for the abandoned-merge record the diff's own added test builds, the call never ran — the test asserted `record.reopened` and could not pass, while the packet recorded that suite as `9 passed` on a diff declared complete. That is worse than a typo. The contradiction is neither a root cause nor an accepted non-finding, so a reviewer that spotted it would be classified `unexpected` at gating severity and **charged a false positive for a curation defect**, while a reviewer that missed it could reach the expected verdict with zero recall. On the case carrying #58's multi-file minimum. The guard is now negated to match the stated goal, and the oracle was strengthened to exercise the corrective action rather than only the two decision predicates — which is precisely why it passed a diff it should have rejected. ## Eleven of seventeen packet diffs were not valid patches `test_every_case_diff_is_a_parseable_patch` only ever covered the original protocol-proof corpus, so malformed hunk headers shipped in every stratum added since: eleven of seventeen packets failed `git apply --numstat`. All headers are recomputed from their own bodies, and the check now iterates `corpus_roots()`. One case needed more than a header. The propagation case carried the untouched sibling caller as a context-only file section, which `git apply` rejects outright — a diff cannot contain a file it does not change. That consumer now appears in `context.data` as the packet's statement of what the untouched module contains, which is both a valid patch and a truer rendering of the case: the whole point is that the sibling is *not* in the diff. This matters more for a scored stratum than it first appears. A packet whose diff is not a valid patch is inconsistent evidence, and a reviewer that refuses a merge verdict on it is behaving correctly under its own contract — which a scored run would record as a mismatch, charging the reviewer for a curation defect again. ## The adjudication gate was keyed on presence, not on scoring The existing check only fired when a case already declared an adjudication, so a later batch could ship scored cases with none and pass everything, while the adjudication plan claimed a test held them to it. A stratum may now not declare `scored` while any of its cases lacks a recorded second adjudication, and the plan's wording is corrected to describe what the tests actually enforce. ## Records Limitations 17 and 18 are added, both from what this cycle found: an oracle is a hand transcription of its packet with no mechanical link to it — demonstrated by the blocking defect surviving one — and diff validity is now gated where it previously was not. A mechanical packet-to-oracle link is named as a candidate v2 mechanism for #59 rather than invented here. Four deferred findings are preserved and reported rather than applied: the formulation audit walks the payload a second time, the oracle expectation lookup takes the first corpus carrying a case, two oracle `corrected` legs restate their `candidate`, and the stratum's single `ground_truth` label covers two `repository_history` cases whose provenance records the difference.
21 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.
Part of #58. This PR does not complete #58, and deliberately carries no closing
syntax. Batch 2 of 4: populates one stratum. Corpus minima remain unmet across the
other two strata, and calibration plus full independent adjudication remain
outstanding — see "Still open" below.
TL;DR
Populates
s1-correctness-orchestratorwith seven cases minimized from realadjudicated review history — three accepted escapes, four adjudicated-rejected
clean controls — and adds an executable-oracle mechanism that gives every one of
them a genuine second independent adjudication.
Summary
shaug/atelier(899 owner-authoredreview comments) or this repository's own delivery history: a multi-file
contract/propagation escape, a concurrency/data-integrity escape, a validation-gap
escape, two clean controls, and two negative controls. Five carry multi-file
diffs; three are only decidable by reading a consumer the diff does not touch.
review-suite/scripts/evals/oracles/): one runnable moduleper case exposing the candidate reproduction, the corrected reproduction where a
root cause exists, and the packet's stated requirement as code.
test_eval_oracles.pyasserts the requirement fails against every gatingcandidate and holds once corrected, and holds against every clean candidate. All
seven cases in this stratum are oracle-settled.
provenance.adjudicationnames the firstadjudication (the source disposition) and the second (
oracleorowner_required), and a test now fails closed if a scored stratum ships a casewith no recorded second adjudication.
seventeen packet diffs across all five corpora were not valid patches (malformed
hunk headers, or a context-only file section
git applyrejects); all now parse,checked across every corpus rather than only the original one. And the
contamination audit now shares its text-normalisation rule with the grader, after
review found one case's packet echoing its own accepted formulation in
grader-equivalent (though not audit-equivalent) form.
The owner's two decisions, applied
Cost ceiling — preregistered hard at 9.00/3.00/3.00 USD (15.00 total). This
batch spent 0.00 USD; no case has been run through any runtime. Full 15.00 USD
headroom remains.
Clean-control standard — a clean control must be an adjudicated-rejected
finding: a concern actually raised and dispositioned as not material by the owner,
recorded as an accepted non-finding with the formulations a reviewer would use, so a
non-gating mention is tolerated while a gating one is charged a false alarm.
Applying it invalidated five earlier candidates, all recorded with why in
SOURCING.md: both of batch 1's clean-control candidates (one rested on praise fora hunk rather than a rejection, the other was an accepted fix — acceptance says the
opposite of what a clean control needs), two negative-control candidates that
re-verification showed were actually accepted, and one that was declined and
then implemented in another form. One candidate batch 1 had flagged as unverified
turned out, on re-check, to have been accepted — it is now the multi-file
propagation escape instead of a control.
Residual gap, recorded as limitation 13: this standard measures re-raising an
adjudicated-immaterial finding. It does not measure whether a reviewer invents a
novel finding on a wholly clean diff. Report false-alarm rate under it as a lower
bound on invention, never as a general rate.
Independent adjudication: settled for correctness, still open for simplicity
The requirement is two independent adjudications per case. The source disposition
is a strong first adjudication for materiality, thin for severity, and cannot supply
formulations even in principle — measured twice in batch 1.
For a correctness case, materiality stops being an opinion: the stated requirement
either holds against the candidate or it does not. Every one of the seven cases here
is adjudicated by executable oracle — genuinely independent of the reviewer being
measured, because a machine that runs the code shares no blind spot with a model
that does not.
What an oracle does not prove: only the requirements the change contract
states. A clean control's oracle proves the contract holds and the raised concern
was immaterial, never that nothing else is wrong. It cannot adjudicate a judgement
about prose. Review found the sharper limit directly: one oracle modelled only two
decision predicates and passed a diff whose corrective action was guarded backwards
— so the diff's own added test could not have passed — because it never exercised
the action the test asserted. Fixed, and recorded as limitation 17: an oracle is a
hand transcription of its packet with no mechanical link to it, and closing that gap
is a candidate v2 mechanism for #59.
No oracle exists for the two simplicity strata. "This is over-engineered" has no
executable form. Those cases will need the owner; a test now holds them to declaring
owner_requiredrather than letting them claim an adjudication they do not have.Still open
s2-solution-simplicity-lensands3-code-simplicity-lens, remaindeclared_unpopulated. Batches 3 and 4.14, new this batch). Batch 1 measured what an uncalibrated expectation reports —
recall 0.0 against a reviewer correct every time — but calibrating a case means
observing its prose, and observing a scored case's prose to tune its formulations
is fitting the grader to the answer. This stratum ships
scored: false, everyexpectation
calibrated: false, and no case has been run through any runtime.Four resolutions are set out for the owner to preregister.
Sanitization
All seven cases are
minimized_reproductionorrepository_history, rewrittenfrom scratch against fictional subjects, retaining only the failure shape: no
business logic, domain identifier, customer context, credential, or hidden
reasoning, and no copied source text. All sources are public. Retention authority
is recorded per case in
SOURCING.md. No content from any private repositoryreached any artifact; the excluded connector-bearing repository stays unnamed and
untouched.
Preserved behaviour
Review behaviour and the v1 packet/result contracts are unchanged. No file under
skills/is modified.just sync-contractsleaves bundled mirrors byte-identical.just eval-review-suiteremains the only command that may spend money and remainsunreachable from
test,lint,check.Validation
At head
06a5679, base2687dd1(this branch was rebased ontoorigin/mainafterunrelated PR #64 merged):
just format—All checks passed!,90 files left unchangedjust lint— pass (ruff, mdformat,skills-ref validate, plugin validation)just test— pass,Ran 208 testsjust test-review-suite— pass,Ran 208 testsjust audit-review-corpus—corpus audit passed for 5 corpus(es)just sync-contractsthengit status— cleanruff==0.15.22(local is 0.11.12) run explicitly:check,check --select I,RUF022,format --checkall cleanReview
review-code-changereturned aggregate clean bound to head06a5679/ base2687dd1, all three lenses run fresh: solution simplicity clean, correctness clean(2 non-gating
defer), code simplicity clean (2 non-gatingdefer). Two earliercycles on earlier heads returned
changes_required; every blocking finding andstrong recommendation was fixed, including one genuine curation defect (a packet
whose own added test could not have passed against its diff, caught by the
strengthened oracle) and a corpus-wide diff-validity gap (11 of 17 packets were not
valid patches before this PR).
Four
deferfindings are preserved per contract: the formulation audit walks thepayload twice; the oracle case lookup takes the first corpus carrying a case id;
two oracle
correctedlegs intentionally share a predicate withcandidate(tracedindependence, not a defect); and the stratum's single
ground_truthlabel coverstwo
repository_historycases whose provenance already records the distinction.Merge
Merge is authorized once CI is green. No closing syntax anywhere in this PR body
or in any commit message. #58 must remain OPEN.