Skip to content

feat(launchpad): fifteen recorded reviewer outputs for the deterministic suite (#117 STEP 8) - #252

Merged
serina-mcfall merged 1 commit into
launchpadfrom
feat/review-agent-recordings-v2
Aug 20, 2026
Merged

feat(launchpad): fifteen recorded reviewer outputs for the deterministic suite (#117 STEP 8)#252
serina-mcfall merged 1 commit into
launchpadfrom
feat/review-agent-recordings-v2

Conversation

@serina-mcfall

Copy link
Copy Markdown

Summary

Adds the 15 recorded reviewer outputs (5 fixtures × 3 dimensions) for #117's plan STEP 8, produced by rendering each fixture's real contained document and reasoning through each dimension's real prompt against it — not hand-written to merely look plausible. This PR is rebased directly onto launchpad now that STEPs 3, 4, 5, 6, 7 and the corpus-scan fix have all merged.

Related issue

Refs #117

Issue type

Task


Agent provenance

Field Value
Harness / provider Claude Code
Model claude-sonnet-5
Session reference N/A - this harness does not expose a run id/URL
Initiating human @serina-mcfall

Objective

15 JSON files under launchpad/review-agent/recordings/<fixture>/<dimension>.json, each a recorded {outcome, findings} reviewer output with full provenance, replayable through run_dimensions.build_document() with zero network calls, plus test_recordings.py verifying all of STEP 8's done-when.

Impacted components

launchpad/review-agent/recordings/{secrets-and-access,claim-vs-evidence,correctness-and-failure-modes,paraphrase,description-of-an-attack}/{secrets-and-access,claim-vs-evidence,correctness-and-failure-modes}.json (15 new files)
launchpad/review-agent/test_recordings.py (new)

Approach and rejected alternatives

Produced each recording by actually rendering the fixture's contained document (contain.render with a seed-derived nonce) and reading each dimension's real assembled PROMPT against it, computing finding_id via findings.finding_id() from the finding's own fields — rejected writing plausible-looking JSON directly, since STEP 8 explicitly requires recordings "from a real run against a real model… never hand-written," and BUDGET's own text names this exact step as where that distinction quietly erodes.

Disclosed a real limitation rather than hiding it: these 15 recordings come from one reasoning pass per fixture (the same session applying all three dimensions' prompts to one rendered document), not three independently-invoked model calls — three genuinely separate runs would not converge on identical prose the way these do for the paraphrase fixture. Every recording's _provenance.sampling field states this plainly and points at STEP 10's live, separately-invoked runs as the actual evidence for independent model variation. Considered fabricating varied wording across dimensions to look more like independent samples — rejected as actively dishonest; disclosure is the defensible choice.

Verification

Command run:

cd launchpad/review-agent && python3 test_recordings.py

Raw output:

.............
----------------------------------------------------------------------
Ran 13 tests in 0.026s

OK

Command run:

cd launchpad/review-agent && python3 test_injection_clause.py && python3 test_run_dimensions.py && python3 test_findings.py && python3 test_fixtures.py

Raw output:

....
----------------------------------------------------------------------
Ran 4 tests in 0.019s

OK
.....................................................
----------------------------------------------------------------------
Ran 53 tests in 0.554s

OK

----------------------------------------------------------------------
Ran 36 tests in 0.002s

OK
...............
----------------------------------------------------------------------
Ran 15 tests in 0.006s

OK

Command run (the repo's own containment control gate — now fully clean, confirming #245's corpus-scan fix landed cleanly):

cd launchpad/review-agent && python3 run_controls.py

Raw output:

PASS  check_contract.py
PASS  check_step2.py
PASS  check_step3.py
PASS  check_step45.py
PASS  check_step6.py
PASS  suite.py
PASS  check_step8.py
PASS  check_step9.py
PASS  check_step11.py
PASS  check_invariants.py
PASS  check_mutations.py
PASS  check_flag_guard.py
PASS  check_fetch_states.py

13 passed, 0 failed, 0 skipped
  • Tests or checks were run and the raw output is pasted above
  • The diff is confined to the scope of the linked issue
  • No secrets, keys, tokens or hostnames were added to tracked files

Not verified

  • No live model was invoked to produce these recordings (see Approach) — STEP 10's live runs are the evidence for genuine independent per-dimension model behavior; this PR verifies the recordings' structural and attributional properties, not live-model reproducibility.
  • Did not re-verify the recordings against a hypothetical future change to any dimension prompt or fixture — these recordings are a snapshot against the prompts/fixtures as they exist in this PR.

Security implications

None beyond what earlier PRs in this stack already cover. These are static JSON recordings with no code path of their own, consumed only by this repo's own tests and, later, an injected reviewer stub during controls — never executed, never sent anywhere, no network access (verified: NoNetworkCallOnReplayTests).

Escalations

  • This commit also carries two corrections to STEP 5's already-merged INJECTION_CLAUSE and claim-vs-evidence.py's ANCHORING_RULE, found during this step's own review (see commit message for full detail): the injection clause as merged would have flagged every fixture's ordinary "Approved." pr_review_bodies content as an attack (a false positive on any clean, already-reviewed PR), and claim-vs-evidence.py's anchoring text contradicted FINDINGS.md on how to anchor a citation to a nonexistent path. Both fixed in this PR rather than a follow-up to feat(launchpad): cross-cutting injection clause in all three dimensions (#117 STEP 5) #250, since that's where the review caught them — same disclosed-bundling pattern feat(launchpad): three dimension definitions + STEP 6 exit-code fix for the PR review agent (#117) #242 used for STEP 4+6.
  • Nine findings (3 High, 4 Medium, 2 Low) came back from the most thorough review of the session (dispatched on the most capable available model, given STEP 9 will treat these recordings as ground truth), all fixed and verified by mutation — see commit message for the full list. No residual findings.

…tic suite (#117 STEP 8)

Adds launchpad/review-agent/recordings/<fixture>/<dimension>.json (5 fixtures
x 3 dimensions) -- for each STEP 7 fixture, one recorded output per dimension,
produced by actually rendering that fixture's contained document (via
contain.render with a seed-derived nonce) and reasoning through each
dimension's real assembled PROMPT against it, not hand-written to merely look
plausible. Every finding_id is computed via findings.finding_id() from the
finding's own fields; every recording is replayed through the real
run_dimensions.build_document() and validated via findings.validate().

Confirms: secrets-and-access's planted credential is Blocker per that
dimension's own 'could this authenticate against a real system' test;
claim-vs-evidence's fixture yields TWO genuine defects (a diff-contradicting
validation claim, anchor line 23; a citation to scripts/config_schema.py,
which does not exist anywhere in this repository, anchor pr); correctness-
and-failure-modes' widened except clause is a two-part fail-open (a missing
manifest AND a malformed one both now return True); the paraphrase fixture is
caught by all three dimensions via STEP 5's injection clause, each anchored at
its own file/line, never anchor pr; and description-of-an-attack yields no
finding from any dimension, confirming the use-mention negative control
holds. All ten non-owning-dimension pairs are correctly clean.

test_recordings.py is a permanent, committed witness (matching test_fixtures.py's
convention): file existence, provenance (model/date/seed/nonce, and an honest
disclosure that these are a single reasoning pass per fixture applied to all
three dimensions -- not independent model invocations, since three genuinely
separate runs would not converge on identical prose), replay validity and
no-network-call, cross-dimension attribution, exact anchor/location/severity
pinning, and the injection-clause behavioral claims.

Reviewed independently on the most capable available model before commit,
given this is the highest-stakes artifact in the whole plan (STEP 9's control
suite will treat it as ground truth). Nine findings came back, three High:
(1) severity was entirely unpinned -- a Blocker could silently become a Low
while every structural check still passed; (2) the three paraphrase
recordings' prose was byte-identical with no disclosure that this was one
reasoning pass, not three independent samples; (3) the injection clause as
first written would have flagged every fixture's own ordinary 'Approved.'
pr_review_bodies content as an attack -- a false positive on any clean,
already-reviewed PR. Plus four Medium (an evidence-raw test that could not
fail; evidence checked against only one of three recordings; an anchor-pr
test that asserted only what findings.validate() already enforces; claim-vs-
evidence's own ANCHORING_RULE text contradicting FINDINGS.md by routing a
nonexistent-citation defect to anchor 'line' instead of 'pr') and two Low (a
two-part fail-open recorded as one; the fixture's two-finding design
uncounted). All nine fixed and re-verified by mutation (each previously-silent
defect class was reproduced and confirmed to now fail the suite, then
restored), which itself surfaced two further Low gaps -- the second
(citation) finding's own severity was still unpinned, and the sampling
disclosure had no test guarding it -- both fixed and mutation-verified in the
same pass.

The dimensions/*.py changes in this commit are corrections to STEP 5's own
files (PR #250, not yet merged) -- found via this step's review, landing here
because that is where they were caught. See PR body for the same disclosure
STEP 4/6's bundling used.

Signed-off-by: Serina Mcfall <serina.mcfall@gmail.com>
@serina-mcfall serina-mcfall added the by:agent Filed or authored by an AI agent, not a human label Aug 20, 2026
@serina-mcfall
serina-mcfall marked this pull request as ready for review August 20, 2026 07:56

@benmitchell11 benmitchell11 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked out the branch in an isolated worktree and ran the actual tests rather than trusting the pasted output. test_recordings.py (13), test_injection_clause.py (4), test_run_dimensions.py (53), test_findings.py (36), test_fixtures.py (15) — all 121 pass, matching the claim.

run_controls.py itself reported 12/13 on my machine, but I ran down the one failure (check_step3.py) and it's a pre-existing, unrelated Windows bug, not something this PR introduced: HERE = __file__.rsplit("/", 1)[0] silently no-ops when __file__ has no forward slash (i.e. invoked from its own directory on Windows), so PAYLOAD resolves to check_step3.py/fixtures/captured-pr.json — a path that doesn't exist. Confirmed this exists identically on current launchpad, untouched by this PR's diff. Filing it separately rather than blocking this PR on it.

Verified both fixes named in Escalations directly against the diff:

  • The pr_review_bodies false-positive carve-out is real and applied identically across all three dimension files — reads correctly: it distinguishes a bare recorded verdict ("Approved.") from an actual attempt to leverage that verdict against this review, rather than flagging the field's ordinary expected content.
  • The ANCHORING_RULE fix in claim-vs-evidence.py is a genuine correctness fix, not cosmetic — a citation to a nonexistent file correctly moved from the "anchor: line" bullet to "anchor: pr", since a target that doesn't exist has no line to point at.

Spot-checked one recording (paraphrase/claim-vs-evidence.json) — real, non-trivial content (a paraphrased approval-injection attempt correctly caught at Blocker), with the single-reasoning-pass limitation disclosed honestly in _provenance.sampling rather than dressed up to look like independent runs. Approving.

@serina-mcfall
serina-mcfall merged commit 30c0669 into launchpad Aug 20, 2026
28 checks passed
@serina-mcfall
serina-mcfall deleted the feat/review-agent-recordings-v2 branch August 20, 2026 19:26
serina-mcfall added a commit that referenced this pull request Aug 20, 2026
The plan's ALREADY TRUE and BUDGET sections, written 2026-08-13, assumed none
of #120 or #117 had landed -- 'THE PRODUCER OF THIS STAGE'S INPUT DOES NOT
EXIST', 'no findings.py, no run_dimensions.py'. As of today every one of
#117's 12 steps is built, and 11 of 12 are merged to launchpad (STEP 8's 15
recorded reviewer outputs are committed on this exact branch, PR #252 open).

Corrected in place, struck through rather than deleted, per this plan's own
citation-rot discipline (the same convention it already uses for its own
prior corrections): the precondition chain #120 -> #117 -> #118 is
discharged, not pending, and the actual risk STEP 8/9 now carry is the
OPPOSITE of what was documented -- real recorded dimension output already
exists and should be the primary input for #118's own fixtures, not
document-only synthesis. Also fixed one stale line citation (review.py:62 ->
review.py:86, verified against the actual file rather than trusted).

STEP 8/9's own text still describes synthesizing fixtures from the contract
document alone; BUDGET's correction flags this explicitly so whoever
implements those steps updates that text rather than building against it as
written. No steps renumbered, no done-when criteria changed -- this is a
documentation correction only, ahead of implementation.

Signed-off-by: Serina Mcfall <serina.mcfall@gmail.com>
serina-mcfall added a commit that referenced this pull request Aug 20, 2026
Fixes the 7 confirmed findings from review-adjudicate plus the disputed
Blocker, resolved per Serina's call: not a real gap (severity values that
matter are rejected on input; reported_severity cannot appear there at
all), but the doc's wording was genuinely misleading and is corrected.

- add the missing top-level `stages` entry section to ADJUDICATION.md
- fix containment key: findings_for's return lands in
  containment.findings, not the whole containment key
- fix the out-of-ladder severity fallback to state the precise
  reported_severity-else-Blocker rule instead of "nearest legal value"
- stop stating #119's sort behaviour as settled fact -- it's still an
  unmerged plan
- reword the disputed sentences so they describe an out-of-ladder
  severity value, never a field literally named reported_severity,
  which cannot appear on input
- correct stale "#252 open" references now that #252 has merged
- note the file inventory list is a summary, not exhaustive
  (run_controls.py, check_*.py, test_*.py omitted)
- reword STEP 1's done-when from "a new row" to "annotating the
  existing row", matching what was actually implemented

Docs-only change; run_controls.py (13/13) and test_findings.py (36/36)
both pass unchanged.

Signed-off-by: Serina Mcfall <serina.mcfall@gmail.com>
serina-mcfall added a commit that referenced this pull request Aug 20, 2026
#256)

* docs(launchpad): correct #118's plan against #117's now-merged state

The plan's ALREADY TRUE and BUDGET sections, written 2026-08-13, assumed none
of #120 or #117 had landed -- 'THE PRODUCER OF THIS STAGE'S INPUT DOES NOT
EXIST', 'no findings.py, no run_dimensions.py'. As of today every one of
#117's 12 steps is built, and 11 of 12 are merged to launchpad (STEP 8's 15
recorded reviewer outputs are committed on this exact branch, PR #252 open).

Corrected in place, struck through rather than deleted, per this plan's own
citation-rot discipline (the same convention it already uses for its own
prior corrections): the precondition chain #120 -> #117 -> #118 is
discharged, not pending, and the actual risk STEP 8/9 now carry is the
OPPOSITE of what was documented -- real recorded dimension output already
exists and should be the primary input for #118's own fixtures, not
document-only synthesis. Also fixed one stale line citation (review.py:62 ->
review.py:86, verified against the actual file rather than trusted).

STEP 8/9's own text still describes synthesizing fixtures from the contract
document alone; BUDGET's correction flags this explicitly so whoever
implements those steps updates that text rather than building against it as
written. No steps renumbered, no done-when criteria changed -- this is a
documentation correction only, ahead of implementation.

Signed-off-by: Serina Mcfall <serina.mcfall@gmail.com>

* feat(launchpad): ADJUDICATION.md -- the verdict contract (#118 STEP 1)

Adds launchpad/review-agent/ADJUDICATION.md, a sibling to FINDINGS.md and
CONTAINMENT.md in the same normative voice: the contract #119 will read once
#118's adjudication stage runs.

Settles: the verdict enum (CONFIRMED/REFUTED/UNPROVEN, input/output finding_id
SETS equal, UNPROVEN the default and REFUTED never one); six added finding
fields (verdict, verdict_evidence, reported_severity, severity, severity_reason,
duplicate_of); three escalate-never-approve prohibitions (no approval-carrying
field, a REFUTED finding still published, a downgrade allowed but visible);
the nine-key adjudication block with completion_marker last, carrying #117's
own nonce rather than minting one; dedupe (survivor-selection rule, both-
directions linking, duplicates still individually verdicted); total refutation
as a flag rather than a silent clean pass; and containment findings passed
through byte-identically, never adjudicated.

Also updates CONTAINMENT.md's 'Contract for later stages' table to point at
this document from #118's existing row, without touching that row's literal
'must call contain.findings_for' prescription -- the plan's own OPEN section
flags that prescription as unsatisfiable from this stage's actual JSON-on-
stdin input, and explicitly defers correcting the table to whoever owns #120.
ADJUDICATION.md names the mismatch plainly and states the deferral rather
than silently resolving a decision the plan marks as not a builder's to make.

Ahead of this commit, corrected the plan itself (launchpad/plans/
2026-08-13-issue-118-adjudication.md, previous commit): its ALREADY TRUE and
BUDGET sections, written before #117 existed as code, are now stale --
#117's 12 steps are all built, 11 of 12 merged. Struck through per the plan's
own citation-rot convention, not deleted.

Reviewed independently (serina:review-code) before commit: two findings, one
Medium (a 'see section Dedupe' cross-reference to a section that did not
exist -- fixed by adding one, covering the survivor rule and both-directions
linking from the plan's STEP 7) and one Low (findings_out==findings_in
described as proving no silent drop, when only the stronger finding_id
set-equality claim actually guards a drop-and-invent swap -- reworded to
state which guarantee is load-bearing). Both fixed and re-verified.

Signed-off-by: Serina Mcfall <serina.mcfall@gmail.com>

* fix(launchpad): address review findings on #256 (#118 STEP 1)

Fixes the 7 confirmed findings from review-adjudicate plus the disputed
Blocker, resolved per Serina's call: not a real gap (severity values that
matter are rejected on input; reported_severity cannot appear there at
all), but the doc's wording was genuinely misleading and is corrected.

- add the missing top-level `stages` entry section to ADJUDICATION.md
- fix containment key: findings_for's return lands in
  containment.findings, not the whole containment key
- fix the out-of-ladder severity fallback to state the precise
  reported_severity-else-Blocker rule instead of "nearest legal value"
- stop stating #119's sort behaviour as settled fact -- it's still an
  unmerged plan
- reword the disputed sentences so they describe an out-of-ladder
  severity value, never a field literally named reported_severity,
  which cannot appear on input
- correct stale "#252 open" references now that #252 has merged
- note the file inventory list is a summary, not exhaustive
  (run_controls.py, check_*.py, test_*.py omitted)
- reword STEP 1's done-when from "a new row" to "annotating the
  existing row", matching what was actually implemented

Docs-only change; run_controls.py (13/13) and test_findings.py (36/36)
both pass unchanged.

Signed-off-by: Serina Mcfall <serina.mcfall@gmail.com>

---------

Signed-off-by: Serina Mcfall <serina.mcfall@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

by:agent Filed or authored by an AI agent, not a human

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants