feat(launchpad): ADJUDICATION.md -- the verdict contract (#118 STEP 1) - #256
Conversation
benmitchell11
left a comment
There was a problem hiding this comment.
Read the full document. This is a rigorous normative contract — the fail-closed default (UNPROVEN, never REFUTED), the deterministic dedupe/survivor tie-break, and the "escalate never approve" section's honest acknowledgment that free-text fields (verdict_evidence, severity_reason, notes) aren't mechanically constrained yet, with the exposure explicitly scoped to "nil until #119 renders them" rather than papered over, are all the right calls.
Verified the two claimed doc edits directly against the diff:
- The plan-doc correction genuinely uses the strikethrough-not-delete convention throughout, with each correction dated and backed by a real command (
git rev-list --left-right --count,git ls-tree,find -iname "*adjudic*") rather than asserted. The BUDGET section's correction is honest about inverting the actual risk (real recorded output now exists; the risk is consuming it correctly, not synthesizing from nothing). CONTAINMENT.md's change is exactly the one line claimed — points the #118 table row atADJUDICATION.mdwithout touching the underlyingcontain.findings_for(surfaces, nonce)prescription that STEP 118 structurally can't satisfy (stdin JSON, noSurfacedict). Correctly left for #120's owner rather than silently resolved here, andADJUDICATION.md's own "Containment findings are passed through" section states the mismatch plainly instead of hiding it.
Confirmed the stacking: base is feat/review-agent-recordings-v2 (#252), which I've now independently verified and approved — so this one's dependency is satisfied. Should still merge after #252, per the PR's own escalation note. Approving.
The base branch was changed.
serina-mcfall
left a comment
There was a problem hiding this comment.
Automated review — #118 STEP 1 (ADJUDICATION.md)
Not an independent cross-model review. Both the review-code pass and the review-adjudicate pass below ran on Claude (two fresh contexts, one review then one adjudication) — no Codex pass. Treat this as a self-consistency check, not the cross-model verification this issue chain's own culture calls for elsewhere.
Method: review-code against the diff. Note the diff GitHub renders for this PR is stale — its merge-base predates #252, which has since merged — so it looks far larger than it is. Verified via git merge-tree against current origin/launchpad: only three files actually changed content — launchpad/plans/2026-08-13-issue-118-adjudication.md, launchpad/review-agent/ADJUDICATION.md (new), launchpad/review-agent/CONTAINMENT.md (one line). A second, independent fresh context then ran review-adjudicate against every finding from the first pass, re-checking each against the merged code rather than trusting the reviewer's read.
CI is green (scripts, check, adr-boundary, audit, controls, Detect Changed Paths, Dead Token Reference Guard all pass).
Confirmed findings, ranked
- High —
ADJUDICATION.mdnever declares the top-levelstageskey this stage must produce (plan STEP 4) and that #119 depends on to banner an incomplete run — despite the doc opening by billing itself as "the document #119 reads to know what arrives." - Medium —
ADJUDICATION.md(§ Containment findings) says #117 placescontain.findings_for()'s return "into the merged document'scontainmentkey." It actually goes intocontainment.findings;containmentalso carries a seven-keystatesmap thatfindings_for()never returns. - Medium — the plan's "PR #252, open" / recordings "pending #252" language is stale: #252 merged as
30c0669ca, ahead of this PR. - Medium — the out-of-ladder severity fallback in
ADJUDICATION.mdsays only "the nearest legal value," undefined. The plan's STEP 6 is precise (reported_severityif legal, elseBlocker) and that precision didn't carry into the contract document a future implementer will actually read. - Low —
CONTAINMENT.md's existing #118 table row was annotated with a doc link rather than given the new row STEP 1's done-when literally asks for. Substance met, letter of the done-when not. - Low — the plan's
ls-treeinventory oflaunchpad/review-agent/reads as exhaustive but omits roughly 20 files, includingrun_controls.py, which STEP 10 depends on registering into. - Low —
ADJUDICATION.mdstates #119's sort/render behaviour (.get(severity, 9), which fields it renders) as settled fact; #119 is still an unmerged, actively-revised plan, not merged code.
Disputed — left for a human call, not resolved either way
One finding split the two independent passes:
- Blocker (reviewer) vs. Refuted (adjudicator) —
ADJUDICATION.md's claim that input validation already guaranteesreported_severityis legal before this stage touches it.- Reviewer's evidence: ran
findings.validate()against a document carrying a field literally namedreported_severity: "Info"— got[](accepted). - Adjudicator's rebuttal: that test doesn't apply — #117's input document has no field named
reported_severityat all; that name is introduced only by this stage's own output. The field that actually needs guarding on input isseverity, andfindings.validate()does rejectseverity: "Info"(confirmed by both passes). - What both passes agree on: the wording in
ADJUDICATION.mdand the plan's own STEP 3 done-when both write "an input carryingreported_severity: 'Info'" — a field name that can't exist on #117's actual output. Whether that's harmless shorthand (adjudicator's read) or a contract ambiguous enough that STEP 3's implementer validates the wrong field name and ships the gap the reviewer describes (reviewer's read) isn't something either pass could settle with more evidence — both already ran the code. Flagging rather than picking a side.
- Reviewer's evidence: ran
Not evaluated
No UI/accessibility surface — this PR is a markdown spec, no code yet. No tests to review. No mutation/control coverage — STEP 1 doesn't build the suite (that's STEP 10/11).
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>
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>
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>
a846b77 to
46acd27
Compare
benmitchell11
left a comment
There was a problem hiding this comment.
Re-reviewing after my earlier approval was dismissed by the rebase/new commits. serina-mcfall's own review-adjudicate pass (posted as a COMMENTED review) caught real things my first pass missed — the missing top-level stages entry in particular is a genuine High, and the containment.findings vs containment distinction is a real precision gap. Good catch on both, and on the disputed reported_severity-vs-severity field-name ambiguity, which was a legitimately confusing sentence even if not a functional gap.
Verified the fix commit (46acd273e) against all 7 confirmed findings plus the disputed one directly in the diff, not just the commit message's claim:
- High (missing
stageskey) — fixed, new "## Thestagesentry" section added, correctly scoped (one entry per stage, never overwrites an existingadjudicationentry, tiesstatusto nonce/total-refutation/verdict-completeness). - Medium (
containment.findingsvscontainment) — fixed, now explicit thatcontainmentalso carries thestatesmap. - Medium (stale "#252 open") — fixed, dated correction added.
- Medium ("nearest legal value" undefined) — fixed, now states the precise
reported_severity-else-Blockerrule. - Low (new row vs annotation) — fixed, wording now matches what was actually implemented.
- Low (incomplete file inventory) — fixed, now stated as a summary, not exhaustive.
- Low (#119 settled-fact language) — fixed, attributed to #119's plan rather than stated as fact.
- Disputed Blocker/Refuted — resolved with clear reasoning (
reported_severitygenuinely cannot appear on input; the wording was misleading, not a functional gap) and reworded throughout both files consistently.
Ran the tests myself in an isolated worktree rather than trusting the commit message: test_findings.py 36/36. run_controls.py shows 12/13 — the 1 failure is check_step3.py, the same pre-existing Windows-only bug I found and filed separately while reviewing #252, unrelated to this diff. Approving.
Review findings resolved — commit
|
Summary
Adds
ADJUDICATION.md, the normative verdict contract for #118 (adjudication pass over every reported finding), plus a documentation correction to its year-old implementation plan against #117's now-merged state.Related issue
Refs #118
Issue type
Task
Agent provenance
Objective
launchpad/review-agent/ADJUDICATION.md: the contract #119 will read once #118's adjudication stage runs — verdict enum, six added finding fields, escalate-never-approve prohibitions, the nine-keyadjudicationblock, dedupe, total refutation, and containment pass-through.Impacted components
launchpad/review-agent/ADJUDICATION.md (new)
launchpad/review-agent/CONTAINMENT.md (one-line: point #118's existing table row at the new document)
launchpad/plans/2026-08-13-issue-118-adjudication.md (documentation correction — see Approach)
Approach and rejected alternatives
Before writing STEP 1, dispatched independent research to re-verify the existing 2026-08-13 plan's factual claims against the actual current codebase, given this repo's own established citation-rot problem (plan documents pin line numbers, file states, and measurements that drift as the branch moves). Found the plan's entire risk framing stale: its ALREADY TRUE and BUDGET sections say "#117's tree does not exist as code at all" and "the producer of this stage's input does not exist" — both were true 2026-08-13, both are now false (all 12 of #117's steps are built, 11 of 12 merged). Corrected in place, struck through per the plan's own established convention rather than deleted, ahead of any implementation.
Did not correct
CONTAINMENT.md's existing #118 table row prescription (must call contain.findings_for(surfaces, nonce)), even though the plan's own research identifies it as unsatisfiable from this stage's actual JSON-on-stdin input — the plan's OPEN section explicitly marks that correction as "not for a builder to decide," deferred to whoever owns #120. Added only a cross-reference toADJUDICATION.mdin that row's label, and stated the mismatch plainly in the new document instead of silently resolving it.Verification
Command run:
Raw output:
(Documentation-only change; this confirms nothing else in the tree was disturbed.)
Not verified
verdicts.py,run_adjudication.py) — this PR is the contract document only (STEP 1 of the plan's 12).CONTAINMENT.mdtable-row mismatch (see Approach) — deliberately left for task: treat all pull request content as untrusted data #120's owner.Security implications
None. Documentation only — no new code path, no new credential, no new network access.
Escalations
feat/review-agent-recordings-v2/ PR feat(launchpad): fifteen recorded reviewer outputs for the deterministic suite (#117 STEP 8) #252, itself open). Should not merge before feat(launchpad): fifteen recorded reviewer outputs for the deterministic suite (#117 STEP 8) #252.## Dedupesection; an overstated claim about whatfindings_out == findings_inalone proves — reworded to point at the actual binding guarantee,finding_idset-equality). No residual findings.