feat: freeze the v1 review baseline configuration and calibrate the grader - #62
Merged
Conversation
## Summary - Add `review-suite/evals/strata/`, one directory per baseline stratum, with an optional `stratum` block in the corpus index declaring the stratum id, the ground truth its expectations came from, whether it is scored, and its purpose. - Discover every shipped corpus in `just audit-review-corpus`, so a stratum added by a later population batch is gated without editing the recipe. - Add three unscored pilot corpora carrying one byte-identical minimized case, so the declared skill closure is the only variable, and record the measured per-stratum cost and latency envelope from a real-runtime run. - Add grader calibration: a versioned calibration contract, committed probe reviews, and a test that replays each probe through the real grader and asserts its classification, matched root causes, recall, false positives, and adjudication referrals. - Add the frozen v1 baseline record: the immutable configuration, a per-stratum cost-ceiling proposal built from the pilot numbers, the calibration and adjudication record, the ground-truth sourcing and sanitization record, and the baseline limitations. ## Why The evaluator could measure a review but could not yet say what it measured against. Three things were missing and each would have corrupted a baseline quietly. **A stratum had no identity.** The corpus index declares exactly one target skill, and the payload for the orchestrator ships eight documents against a single lens's two. Without a labelled stratum, two incomparable runs could be reported as one figure. Ground truth had no label either, which matters here more than usual: this repository has no pull-request review history at all, so the connector stratum is deferred rather than satisfied, and a human-review figure must never be reported as a connector figure. A test now asserts no shipped corpus claims connector ground truth. **The grader was uncalibrated, which is not the same as conservative.** Matching is containment on normalised text, so formulations written before any real prose existed recognised nothing: the pilot found the real root cause on all three runs and scored recall 0.0. Calibrating against the observed prose — pilot output only, never scored output — moved three fresh runs to recall 1.0 with zero false positives. Calibration also found that a `surface` written as a path prefix shares a token with almost every location in a packet, which made a deliberately wrong gating finding an unfalsifiable partial and left the false-positive rate unmeasurable until the surface was narrowed to a symbol. **The cost envelope could not be inherited.** The earlier smoke figure predates the usage-accounting fix and rests on a denominator of one. Measured fresh, the orchestrator stratum costs about 1.8 times a lens stratum per attempt, and the first attempt of any stratum costs three to four times the ones after it because it pays prompt-cache creation. A ceiling built from a mean would be exceeded by any run whose cache does not stay warm, so the proposal is built from the cold figure. ## Preserved behaviour Review behaviour and the v1 packet and result contracts are unchanged. No file under `skills/` is modified, `review-suite/CONTRACT.md`, both v1 schemas, `validate.py`, the lens prompts, and `review-suite/fixtures/` are untouched, and `just sync-contracts` leaves the bundled mirrors byte-identical. The existing protocol-proof corpus is unchanged. `just eval-review-suite` remains the only command that may spend money and remains unreachable from `test`, `lint`, and `check`. ## Deliberately not done The scored strata are declared and **not populated**, and no baseline figure exists. Two inputs cannot come from an implementing context: a per-stratum cost ceiling preregistered by the repository owner before any scored output is examined, and two independent adjudications per private expectation from genuinely separate parties. The configuration record states this as its status rather than implying a freeze that has not happened, and the sourcing record names every required case class together with the real adjudicated review already identified for it, so nothing is silently omitted.
Closes the two blocking findings and three strong recommendations from the initial review, and expands the pilot now that spend is not the binding constraint. ## Auditable citations The calibration set and `CALIBRATION.md` cited the raw run their formulations were drawn from by a path the runner had since overwritten: an artifact is named for its case and run number only, so re-running a stratum into the same directory replaced output a committed record already cited. The loss is recorded rather than papered over, and two things stop it recurring. - `runner.refuse_to_overwrite_artifacts` fails **before any attempt launches** when a run would overwrite retained output, so the failure is a refusal that costs nothing rather than a silent replacement on the one command that can spend money. - Retained output goes to `artifacts/<stratum>/<corpus_version>/`, which is the path recorded in the frozen per-stratum invocations. The committed `observed` probes are now verbatim, byte for byte, from a retained post-calibration run, so every citation resolves to a file that exists. ## The false-positive boundary was overstated The record claimed that narrowing a root cause's `surface` from a path to a symbol restored the false-positive boundary. Measured against the shipped expectation, it did not: `grader.match_strength` counts **one** shared normalised token as a surface hit, so a wrong gating finding at the changed file, at its test, or at an untouched neighbour is still referred rather than charged. Narrowing shrank the unchargeable region; it did not remove it. `false_positive_rate` is therefore recorded as a lower bound rather than a rate, a `surface_token_collision` probe pins the residual gap in the calibration set, and the one-token rule is escalated to #59 as a candidate v2 gate rather than recorded as solved — repairing the grader is outside this ticket. ## Calibration claims are now enforced, not declared A probe's `kind` was only checked for presence, so a set could ship a `plausible_false_positive` probe whose own expectation declared `partial` and pass. `PROBE_KIND_CONTRACT` asserts the required outcome per kind against the real grader, not against the set's own claim. ## The frozen configuration is executable `eval-review-suite` took only an executor, so it could not reach any stratum: `--corpus` defaults to the protocol-proof corpus and `--runs` to 1, meaning the documented command silently evaluated a different corpus once. It now forwards runner arguments, the justfile parser in the command-contract test understands just's variadic prefix, and the exact per-stratum invocation is recorded for the pilot and for each declared scored stratum. ## A target-mismatched grade cannot be misread as quality Both lens pilot reports record recall 0.0 and false-clean 1.0 for contract-faithful behaviour, because they carry the orchestrator's case to isolate payload size. The caveat is now in-band: `stratum.grading_is_signal`, a test forbidding a scored stratum from declaring it false, annotated `pilot_reports` entries, and a README beside the reports. ## A larger pilot, and what it measured Five runs per case, 20 attempts, zero evaluation failures, zero timeouts, 1.2266 USD. A second, materially larger case was added to the orchestrator stratum, which produced two measurements the ceiling now rests on. - **The first attempt of a case costs three to four times the rest**, paying prompt-cache creation. A ceiling from the mean is exceeded by any run whose cache does not stay warm, so the proposal assumes every attempt is cold. - **Cost tracks output volume, not packet size.** The larger packet raised input tokens 2.9% and raised warm cost 28% and mean latency 45%, because output grew 44%. Per-attempt cost scales with how much a reviewer has to say. The second case is also deliberately left **uncalibrated**, as a control. Over five attempts it scored recall 0.0 with nine adjudication referrals while the reviewer gated the change every time and verdict stability was 1.0. Together with the calibrated case's recall 1.0, that is two independent confirmations that an uncalibrated expectation reports a number about itself. `expectation.calibrated` is now machine-readable and a test refuses a scored case without it. ## Added: how the adjudication gate can honestly be satisfied `ADJUDICATION-PLAN.md` separates the requirement into ground-truth, fidelity, and grading adjudications, because the available evidence differs completely. Source review threads are a strong first adjudication of a root cause, thin on severity, usually silent on accepted non-findings, and cannot supply formulations even in principle. A blind agent context is legitimate for fidelity and grading, and is **not** a legitimate second adjudicator for materiality when it shares a model family with the reviewer being measured: the errors correlate, so recall would rise without the reviewer improving. An executable oracle is stronger than any opinion where a reproduction can be made to run. Every case where the two adjudications are expected to disagree is named, including that neither identified clean-control candidate is an adjudicated clean review.
## Summary - Carry the corpus's declared `stratum` verbatim into the aggregate report's `configuration`, alongside the distinct executor model identities the run actually observed. - Assert the overwrite refusal by its launch log rather than only by its exception, and assert that a report names its stratum. ## Why The limitations record claimed every stratum is labelled "in `corpus.json` and in every report". No report carried a stratum id, its ground truth, whether it is scored, or whether its grading is a signal at all — the runner held `loaded.stratum` and never copied it. So the epic's most emphatic prohibition, that a human-review figure must never be reported as a connector figure, was maintained only as prose in other files, and a report quoted on its own silently lost it. Model identity had the same shape of gap: a stratum that cannot name the model that answered is not comparable with any other. The overwrite-refusal test asserted the exception, its message, and byte-identity. All three would still pass if the check moved into `_write_artifacts` — which would bill a paid attempt before refusing, defeating the entire point of the guard. The launch log is what distinguishes the pre-launch guard from the one that is too late to help, and the sibling contamination tests already use that idiom. ## Sequencing note Committed before the pilot is re-run, deliberately. A run records the commit its tree was at, so running from an uncommitted tree produces a report attributing itself to a commit that cannot reproduce it. This commit is the tree the recorded pilot runs from.
…ords Closes the remaining review findings against the frozen deliverable. Every figure in every record now resolves to a committed report or a retained artifact. ## The pilot batch was attributed to a commit that could not run it Three prose records credited the frozen pilot to the base commit, while the reports recorded the branch's first commit — and neither could reproduce the batch, because the corpora were uncommitted working-tree state at run time. One key was carrying two different meanings across six hand-copies. - The pilot was re-run from the committed tree at `2ae0d23`, so the reports now record a commit whose tree contains the corpora they evaluated. Later commits on this branch change records only and leave every payload byte-identical. - The frozen record's single `suite_commit` is replaced by two explicit fields: `v1_review_behaviour_commit`, the pre-v2 commit whose review behaviour a scored baseline must evaluate, and `pilot_suite_commit`, what the pilot actually ran from. The note on the first says what is really load-bearing — this candidate touches no file under `skills/`, so the evaluated closure text is identical at every commit on this branch, and the closure digest moves when that text moves while a commit can move without it. - The superseded batches are now cited by corpus version and closure digest rather than by a commit, because no commit reproduces them, and the calibration record says so. ## Per-attempt figures now have a retained source The pilot invocations omitted `--attempts-out`, so four per-case latency ranges and the output-volume conclusion had no retained source. Per-attempt records are now retained beside the raw output, and both paths carry the commit as well as the corpus version. ## Records corrected - `controlled_artifacts` omitted the version scoping that the same file prescribes as the remedy for the overwrite it describes. - The adjudication plan said the corpus "contains" the CI-adjudicated case; the sourcing record only *identifies* it as batch-2 ground truth. - `evals/README.md` built a 0.0-to-1.0 recall comparison whose "before" was #50's smoke figure, which this ticket's non-goals forbid citing as any kind of signal. Re-anchored on this ticket's own two-case measurement inside one batch, which is a legitimate anchor and a sharper one. - The stratum-label claim is now true rather than aspirational, and names the report field that carries it. ## Re-measured envelope Suite commit `2ae0d23`, corpus `1.3-pilot-*`, 20 attempts, zero evaluation failures, zero timeouts, **1.2536 USD**. Both pilot measurements held on fresh data: the first attempt of a case still costs three to four times the rest, and the larger packet still raised input tokens 3.0% while raising cache-read cost 21% and cache-read latency 22%, because cache-read output grew 29%. Worst observed cold cost per attempt moved, so the per-stratum ceiling proposal is rebuilt on it: 9.00 USD for the orchestrator-targeted stratum, 3.00 USD for each lens stratum, 15.00 USD overall, against an expected spend of 5.08 USD. Total pilot spend across this ticket is 4.6312 USD over 72 attempts, reported in full.
Closes both gating findings from the second review cycle. Both were figures in a record that did not match the artifact the record cites. ## Corrected - **Referral count.** The limitations record said `status-label-normalization` produced nine adjudication referrals. The committed orchestrator report carries ten, two per run across five runs, and six sibling records already said ten. The limitations record was the sole outlier, and it is the one document #58 designates as an explicit input to #59. - **Input-token series.** The recorded drift series opened with 32,507, which appears in no retained artifact: it came from the earliest batch, whose raw output was overwritten before the artifact path was version-scoped. The retained series is 32,573, 32,955, 33,166, then 32,464. Both records now list the retained values and disclose the unretained one as not re-derivable, the same disclosure the calibration record already makes for that batch. - **Timeout rationale.** It justified the 300 s timeout against a 55.4 s worst latency. A raw stdout record carries no latency field, and that observation came from a superseded batch whose per-attempt records were never written under the artifact path, so it cannot be re-derived. Restated against 51.2 s, which is both the worst in any committed report and the worst in any retained per-attempt record, making the margin 5.9x rather than 5.4x. - **Overwrite-guard claim.** The frozen record implied the runner refuses to overwrite every retained output. It refuses on the raw stdout artifacts; the per-attempt, report, and baseline-report paths are still written unconditionally and are protected only because the frozen invocation gives them the same `<commit>-<corpus_version>` stem, so the raw collision fires first. The record now says exactly that. ## Deferred findings recorded rather than fixed Two limitations are added instead of code changes, because the review did not gate on either. - Every measured figure has several hand-maintained homes and nothing checks the restatements against the reports. Two cycles each found one drifted figure. The duplicated *prose* caveats must stay — a reader reaching one report should not have to find another file to learn its grading is not a signal. The duplicated *numbers* should not, and two remedies are named: one canonical home per figure, or a test asserting documented figures match their report. - Widening the overwrite guard to every output path. Both are inputs to #59. Until one lands, a number in a prose record is a restatement and the committed report is the source.
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 — see "Why #58 stays open" below.
TL;DR
Turns the review-replay evaluator from #50 into something that can say what it
measured against: labelled baseline strata, a calibrated grader, a measured
per-stratum cost and latency envelope from a real unscored pilot, and the frozen v1
configuration record. The scored corpus is declared and deliberately not
populated.
Summary
review-suite/evals/strata/, one directory per stratum, eacha complete corpus declaring its own
target_skillplus astratumblock namingits id, its ground truth, whether it is scored, and whether its grading is a
signal at all.
just audit-review-corpusnow discovers every shipped corpus, so astratum added by a later batch is gated without editing the recipe.
taken verbatim from retained real runs, and a test that replays each probe through
the real grader and asserts its classification, matched root causes, recall, false
positives, and adjudication referrals. A probe's
kindis a claim about behaviour:the required outcome is asserted per kind against the grader, not against the
calibration set's own expectation.
failures, zero timeouts, 1.2536 USD, run from the committed tree at
2ae0d23.review-suite/evals/baseline/v1/— the immutableconfiguration, a per-stratum cost-ceiling proposal built from the pilot numbers, the
calibration and adjudication record, a plan for satisfying the
two-independent-adjudication gate, the ground-truth sourcing and sanitization
record, and the baseline limitations.
are carried into every report's
configuration, so a report quoted on its own stillstates its ground truth.
What the pilot actually measured
Two findings shape the ceiling, and neither was predictable from #50:
prompt-cache creation while later attempts read the cache. A ceiling built from the
mean is exceeded by any run whose cache does not stay warm, so the proposal assumes
every attempt is cold.
two cases specifically to measure this. The larger packet raised input tokens
3.0% and raised cache-read cost 21% and cache-read latency 22%, because
cache-read output grew 29% — it warranted more findings, and output tokens
dominate.
Per-stratum cost-ceiling proposal
s1-correctness-orchestrators2-solution-simplicity-lenss3-code-simplicity-lensIt is a proposal. No scored run may launch until the repository owner
preregisters a ceiling, because a ceiling fixed after outputs are visible is not a
ceiling.
Calibration is not optional, and now it is measurable
Two cases in one batch, differing only in whether their formulations had ever been
confronted with real reviewer prose:
calibratedrollback-guidance-rendertruestatus-label-normalizationfalseThe uncalibrated case's reviewer gated the change on every attempt, stably, and the
grader matched neither root cause. An uncalibrated expectation reports a number about
itself.
expectation.calibratedis now machine-readable and a test refuses a scoredcase without it.
Ground truth, and the stratum label it forces
This repository has no pull-request review history: zero reviews, zero review
threads, zero comments across every PR. There was no in-repository connector material
to curate, and none was invented. Ground truth comes from real adjudicated human
review in the public
shaug/atelier(899 owner-authored review comments) and fromthis suite's own delivery history.
shaug/eldritchdarkwas not read at all. Only its visibility was checked: it isprivate, this repository is public.
shaug/ateliersupplies every required caseclass with clean provenance, so reading a private repository would have added
disclosure risk for no coverage gain. No content from any private repository
reached any artifact here.
excluded on third-party authority and disclosure grounds. Not named, not read, not
derived from.
recall has never been measured. A test asserts no shipped corpus claims
connector-reviewground truth, and no human-review figure may be reported as aconnector figure.
Every retained case is a minimized reproduction rewritten from scratch against a
fictional subject, carrying only the failure shape — no business logic, domain
identifier, customer context, credential, or hidden reasoning, and no copied source
text. Per-case provenance, retention authority, and every exclusion are in
review-suite/evals/baseline/v1/SOURCING.md.Why #58 stays open
Two prerequisites cannot come from an implementing context, and #58's own "Frozen
baseline protocol" places steps 3–6 outside an implementing run for exactly this
reason:
scored output is examined.
severity, and allowed formulation, from genuinely separate parties. One context
generating both sides is not independence, and nothing here pretends otherwise.
frozen-configuration.jsonrecords its own status asincomplete_pending_owner_preregistrationrather than implying a freeze that has nothappened.
ADJUDICATION-PLAN.mdproposes how the gate can honestly be satisfied — andconcludes that a blind agent context is legitimate for judging a minimized case's
fidelity and a formulation's tightness, but not for judging whether a defect is
materially real when it shares a model family with the reviewer being measured, since
those errors correlate and recall would rise without the reviewer improving.
The three scored strata are
declared_unpopulated. Population is split intoevidence-preserving batches, as #58's delivery boundary directs, with every required
case class and its identified real ground truth recorded so nothing is silently
omitted.
Preserved behaviour
Review behaviour and the v1 packet and result contracts are unchanged. No file under
skills/is modified;review-suite/CONTRACT.md, both v1 schemas,validate.py, thelens prompts, and
review-suite/fixtures/are untouched;just sync-contractsleavesthe bundled mirrors byte-identical; the existing protocol-proof corpus loads and audits
unchanged against purely additive fields.
just eval-review-suiteremains the onlycommand that may spend money and remains unreachable from
test,lint, andcheck.Validation
At head
d013507, base16560d80:just format—All checks passed!,79 files left unchangedjust lint— pass (ruff, mdformat,skills-ref validate× 8 skills, plugin validation)just test— pass,Ran 194 testsjust test-review-suite— pass,Ran 194 testsjust audit-review-corpus—corpus audit passed for 4 corpus(es)just sync-contractsthengit status— cleanruff==0.15.22while local ruff is 0.11.12, so the pinned version was runexplicitly:
check,check --select I,RUF022, andformat --checkall clean.Local
mdformatis 1.0.0, matching CI.Review
review-code-changereturned aggregatecleanbound to headd013507and base16560d80, with all three lenses run in fresh isolated read-only contexts:solution simplicity clean, correctness clean, code simplicity clean. Two earlier
cycles on earlier heads returned
changes_required; their blocking findings — acalibration citation pointing at overwritten output, an overstated false-positive
boundary, a frozen configuration that could not actually be executed per stratum, and
two figures that did not match the artifacts they cited — were fixed and are visible in
the commit history.
Six
deferfindings are preserved rather than applied, as the contract requires:consistency check. Recorded as limitation 11.
--attempts-out,--report-out, or--baseline-report. Recorded as limitation 12.frozen-configuration.jsonsay the frozen invocations give every output the shared<commit>-<corpus_version>stem. Every--report-outis in fact a stemless fixedpath, so a re-run replaces a committed report without the guard firing; its only
protection is git history. Left unapplied because fixing it would change the head and
void the head-bound clean review after the three-cycle budget closed.
CALIBRATION.mdstates the artifacttemplate as
artifacts/<stratum>/<corpus_version>/, omitting the<commit>-component that the same file's own concrete citation,
frozen-configuration.json,and
strata/README.mdall use. Same reason.probe_resultcouldtake the case rather than a pre-projected dict.
cases_by_idreturns a pair whose first element every consumer discards, and therunner-evaluate test setup is open-coded once more.
Items 3 and 4 are factual corrections to records #59 will read; they are stated here
so the information is not lost, and both underlying gaps are already disclosed as
limitations.
Merge
Merge is authorized for this candidate once CI is green.
The
Part of #58wording at the top is deliberate, and so is the absence of anyauto-linking keyword anywhere in this description. Merging this PR must leave #58
OPEN, because the owner-gated scored baseline is still outstanding. If you see #58
listed as an issue this PR would auto-resolve, that is a defect in this description —
please tell me rather than merging.
🤖 Generated with Claude Code