feat(launchpad): verdict contract in code (#118 STEP 2)

[serina-mcfall]

Summary

Implements STEP 2 of launchpad/plans/2026-08-13-issue-118-adjudication.md, following STEP 1 (ADJUDICATION.md, merged in #256). Adds launchpad/review-agent/verdicts.py and test_verdicts.py: the Verdict/Adjudication dataclasses matching ADJUDICATION.md's six added finding fields and nine adjudication block keys, plus a pure validate(input_document, output_document) -> list[str] that returns every violation rather than stopping at the first.

Related issue

Refs #118

Issue type

Task

---

Agent provenance

Field	Value
Harness / provider	Claude Code
Model	claude-opus-5 (this body); the code and its commits were authored in an earlier session — see the note below
Session reference	N/A - harness does not expose a stable run id/URL for this session
Initiating human	@serina-mcfall

Provenance caveat, stated rather than papered over: this body was filled in by a different session from the one that wrote the code, after CI's pr_body_check.py failed the PR for missing provenance, a missing ### Not verified section, and no fenced output block. The authoring session's own model id is not recorded anywhere I can read, so I have not guessed at it — the original body said only "Drafted by Claude Code". Everything below the provenance table is either quoted from the original body or re-verified by me directly; nothing here is inferred about what the authoring session did.

Objective

Turn ADJUDICATION.md's prose verdict contract into executable dataclasses plus a validator, so later steps can check an adjudication document instead of trusting it.

Impacted components

launchpad/review-agent/verdicts.py
launchpad/review-agent/test_verdicts.py

Approach and rejected alternatives

SEVERITY_ORDER is imported from review.py rather than redeclared, so the ordering cannot drift between the two modules. validate() is pure — no subprocess, no network, no model call — and returns the full list of violations rather than raising on the first, because STEP 10's planned controls feed it deliberately malformed documents and need to see every violation each one produces. It re-runs #117's own findings.validate over the output rather than reimplementing those checks.

One Medium finding was resolved by scoping rather than extending: validate() does not check 3 of the 9 adjudication keys (schema_version, verdict_counts, notes). Per STEP 1's own text, per-key controls for the adjudication block are STEP 10's job (check_adjudication.py), and STEP 2's done-when never names those three. The fix (77065b95b) was to state that boundary explicitly in validate()'s docstring rather than pull STEP 10's scope forward — the rejected alternative being to widen validate() now and leave STEP 10 with nothing to do and no record of why.

A serina:review-code pass before this PR found one Blocker: validate() raised TypeError on a non-string field value (e.g. severity: ["Blocker"]) instead of returning a violation, which would have broken exactly those STEP 10 malformed-input controls. Fixed in f79aa3d64 and independently re-confirmed against a fresh repro.

Verification

Command run — both suites, at this PR's head (77065b95b, confirmed to match gh pr view 261 --json headRefOid):

python3 -m unittest discover -s launchpad/review-agent -p "test_verdicts.py"
python3 -m unittest discover -s launchpad/review-agent -p "test_findings.py"

Raw output:

.................
----------------------------------------------------------------------
Ran 17 tests in 0.002s

OK

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

OK

test_verdicts.py's 17 cases cover every item in STEP 2's own done-when list: four independent violations reported at once, drop-vs-invent detected via id sets rather than counts, both severity fields independently, both downgrade directions, both total_refutation directions, both dedupe directions, the findings.validate re-run, no-mutation of the input, and SEVERITY_ORDER identity with review.py. test_findings.py (36 tests, from #117) confirms this change leaves that contract untouched.

• 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

I did not re-derive the review-code Blocker (f79aa3d64) or the Medium scoping finding myself — those are quoted from the authoring session's own account, and I have not independently reproduced the original TypeError. I did not check validate() against a real adjudicator's output, only against the fixtures in test_verdicts.py, so nothing here proves it accepts a document a live adjudication run would actually produce. The three unchecked adjudication keys (schema_version, verdict_counts, notes) are deliberately unvalidated at this step and remain unverified until STEP 10 builds check_adjudication.py. No mutation testing was run against validate(), so I cannot say the 17 cases would catch a weakening of the checks they cover. launchpad/review-agent's tests are run in CI by launchpad-review-agent-controls.yml only on paths under launchpad/review-agent/** — I confirmed that path filter matches this diff, but did not watch the job itself go green for this specific head.

Security implications

None that change exposure. validate() is pure and offline by construction — no subprocess, no network, no model call — and it reads documents rather than executing anything in them. It is a containment mechanism in the weak sense that it is the thing that will later refuse a malformed or adversarial adjudication document, so the Blocker fixed in f79aa3d64 (raising TypeError instead of returning a violation on a non-string field) mattered: a validator that crashes on hostile input fails open in any caller that catches exceptions broadly.

Escalations

The provenance caveat above is the one item raised rather than decided: I filled this body in for a PR I did not author, and I have deliberately not invented the authoring session's model id or re-stated its findings as my own observations. If @serina-mcfall wants the provenance table to name the actual authoring model, that has to come from her or that session — I cannot read it.

[ciaran-slow]

Review pipeline — PR #261

Stages run: review-code, review-tests, review-a11y, review-adjudicate, review-final.
Plan read first: launchpad/plans/2026-08-13-issue-118-adjudication.md, STEP 2 at :362.

Not applicable, declared rather than faked: review-a11y — the plan's LEFT OUT section puts accessibility out of scope for #118 and states why (a definition plus a CLI printing JSON; no UI). check-ledger.sh — the plan uses STEP N, not ### Task N:, and no .superpowers/sdd/ ledger exists; the checker exits 1 on its vacuity guard, so I walked the step graph by hand.

I ran the two mechanical clauses of STEP 2's done-when myself, in a clean worktree at 77065b95b:

$ python3 -c "import verdicts"                                     → import OK
$ python3 -c "import verdicts, review;
              print(verdicts.SEVERITY_ORDER is review.SEVERITY_ORDER)"  → True
$ python3 -m unittest test_verdicts                                → Ran 17 tests, OK

The import clause matters more than it looks: the plan specifies it instead of py_compile, because py_compile compiles without resolving imports and would pass on a module whose import review cannot be satisfied. It resolves.

Everything below came from probing the validator with mutated copies of the suite's own make_well_formed_pair() fixture, so the only variable in each case is the mutation.

---

Findings

1. High — whitespace-only verdict_evidence satisfies "non-empty" at every guard in the pipeline

launchpad/review-agent/verdicts.py:268 and launchpad/review-agent/run_adjudication.py:516 (the latter on the #263→#267 chain)

The plan requires "verdict_evidence present and non-empty on all three" verdicts. Both places that enforce it test falsiness, and a whitespace string is truthy.

verdicts.py:268 — if not finding.get("verdict_evidence"):. Probed:

mutation: reports[0].findings[0].verdict_evidence = "   "
verdicts.validate(input, output) → 0 violations

run_adjudication.py:516 — if verdict not in verdicts.VERDICTS or not evidence: — has the same shape, and its docstring claims the stronger property: "fail closed to UNPROVEN on anything unusable — a raised exception, a non-dict return, an illegal/missing verdict, or empty verdict_evidence." not " " is False, so the whitespace passes and is forwarded verbatim into safe_result["verdict_evidence"] at :527.

Concrete failure: a judge returns {"verdict": "CONFIRMED", "verdict_evidence": "\n"} — the shape a truncated model response or a stripped formatting step actually produces. _run_judge_safely accepts it rather than failing closed. verdicts.validate accepts it. The document publishes a CONFIRMED finding whose evidence is blank, and per ADJUDICATION.md a CONFIRMED Blocker is what blocks a merge. So a blank-evidence confirmation can block a merge with no stated reason — while the ADJUDICATION.md default both docstrings invoke by name, "returns unusable output yields UNPROVEN with a reason", silently does not apply to the most likely form of degenerate output.

Nothing fails today: stub_judge always returns real prose. It goes live with --replay, which loads recorded verdicts straight from JSON files, and with any real judge — STEP 9 being the next planned step.

Fix, both sites: test the stripped value — if not str(evidence).strip(): in the runner, and if not str(finding.get("verdict_evidence") or "").strip(): in the validator. Two lines. Worth doing at both ends rather than one: the runner's guard is the fail-closed promise and the validator's is the contract, and the plan's own reasoning for re-running findings.validate on the output — "a stage that quietly breaks its input's own rules is the same defect as one that drops a finding" — argues for not relying on a single checkpoint.

One defect, two sites, counted once. Reported here because this PR defines what "non-empty" means for the contract; cross-referenced from #263's review rather than filed twice.

2. Medium — a duplicate_groups entry with no duplicates never has its survivor validated at all

launchpad/review-agent/verdicts.py:425

The survivor is only ever checked indirectly. :433 loops over duplicates, and for each one confirms the finding exists and that its own duplicate_of equals survivor. So a bad survivor is normally caught through a duplicate pointing at it. With an empty duplicates list, that loop never runs and nothing else looks at survivor. Probed:

duplicate_groups = [{"survivor": "no-such-id", "duplicates": []}]   → 0 violations
duplicate_groups = [{"duplicates": []}]      (no survivor key)      → 0 violations

Both validate clean. The second is worse than the first: the group has no survivor field whatsoever.

Concrete failure: the runner emits duplicate_groups: [{"survivor": "abc123", "duplicates": []}] after a dedupe pass that grouped nothing — an off-by-one in the grouping logic, or a survivor whose duplicates were filtered out upstream. verdicts.validate reports the document clean. #119 then renders a duplicate group naming a finding_id that may not exist in the document, and there is no finding anywhere marked as its duplicate, so the group asserts a relationship with one end missing. The plan's stated intent is the opposite: "the grouping is in the output rather than in the stage's head… discoverable from the finding as well as from the block." A group with no members is discoverable from neither.

Fix: two checks in the loop at :425, before the duplicates handling — that survivor is a string present in output_ids, and that duplicates is non-empty. The plan's own done-when says a run that dedupes nothing emits an empty duplicate_groups array, so an empty group inside a non-empty array is never a legal shape and can be rejected outright.

3. Medium — a boolean passes every integer check on the count fields

launchpad/review-agent/verdicts.py:322 and :324

isinstance(True, int) is True in Python — bool subclasses int — and True == 1. So both the type guard and the equality chain accept booleans. Probed:

adjudication.findings_in  = True
adjudication.findings_out = True
every report's findings_count = True   (with one finding each)
verdicts.validate(input, output) → 0 violations

Zero. The document asserts "findings_in": true and passes the contract check clean.

Concrete failure: the plan's own STEP 10 is "one control per done-criterion", and this PR's body records a Blocker already found and fixed in the same family — validate() raising TypeError on severity: ["Blocker"] — explicitly "which would have broken STEP 10's planned 'feed every field malformed' controls." findings_in: true is exactly such a malformed value, and it is the one that does not raise and does not get reported. When STEP 10 feeds every field malformed, this is a control that will report the validator as accepting garbage.

The asymmetry is the evidence that strictness was intended: total_refutation is checked with isinstance(declared_total_refutation, bool) — a strict bool test that correctly rejects 1 — while the count fields next to it accept True. One of the two reflects the author's intent and it is not the looser one.

Fix: isinstance(x, int) and not isinstance(x, bool) at both sites, or a small _is_count() helper used by all three, since the same test is needed in three places.

4. Low — missing by:agent label

No labels on this PR; its body carries an Agent provenance block. launchpad/AGENTS.md §5 rule 3 requires by:agent.

gh pr edit 261 --repo launchpad-26/buzz --add-label by:agent

---

What I looked for and did not find

Four things I expected to be defects and confirmed are not. Two were probed, not reasoned about.

• A missing top-level nonce sliding through. :463 compares output_document.get("nonce") against input_document.get("nonce"), so if both lack the key, None != None is False and the "present" half of the plan's requirement looks unenforced. It is enforced — by task: the parallel review dimensions that produce findings #117. Probed by deleting nonce from both documents: 3 violations, the first being document: missing or empty top-level 'nonce' from findings_module.validate at :232. The guard is real and upstream, exactly as the plan said it would be.
• Containment findings polluting total_refutation. The plan requires containment findings to pass through with no verdict field, and total_refutation is all(v == "REFUTED") over _iter_findings. Had _iter_findings walked containment.findings, every verdict-less containment finding would force the flag to False and total refutation would be unreportable on any PR with a containment catch. It does not: :138-150 reads document["reports"][*]["findings"] only. Correctly scoped.
• keys[-1] on an empty mapping. :478 indexes list(adjudication.keys())[-1], which would raise on {}. Unreachable: the else branch only runs when "completion_marker" in adjudication, so the list is non-empty, and the non-dict path at :311 substitutes {} which fails that test first. No crash.
• Prohibition 1 implemented as a grep. _find_forbidden_keys at :184 walks the parsed structure instead, and the docstring gives the right reason: a grep over serialised text would also flag those words inside verdict_evidence prose, which prohibition 1 does not bind. Recursive over dicts and lists, so a forbidden key nested at any depth is caught.
• Self-duplication. Probed a finding named as its own survivor and its own duplicate: caught, duplicate_of names itself.
• Tests that cannot fail. All 17. Every fixture and expectation is a literal; the make_* helpers build dicts from literals with keyword overrides and compute nothing. test_four_independent_violations_surface_at_once is the right test for the "returns EVERY violation" requirement, and it asserts on the count rather than on a mock.
• An undeclared scope gap. validate's docstring at :207-213 states plainly that schema_version, verdict_counts and notes are unchecked, why (STEP 1 assigns one-control-per-key to STEP 10), and the consequence: "A document with a fabricated verdict_counts or a wrong-typed schema_version passes this function with zero violations today." STEP 2's done-when names none of the three. A limitation the code documents is not a finding, and this one is documented better than most.

On CI coverage — different from #260 and #262

I reported on #260 and #262 that no CI job runs their test directories. The same is mechanically true here — run_controls.py's CONTROLS list is hardcoded and names no test_*.py, and suite.py is #120's containment suite, not a discoverer — but this PR states the decision and names its owner, at test_verdicts.py:9-13:

"deliberately not wired into run_controls.py's CONTROLS list — that is STEP 10's control suite, over the full adjudication surface (run_adjudication.py included), not this module in isolation."

That is a real plan step (:741, [needs 4, 6, 7, 9]), not a hand-wave, so I am not filing it as a finding here. The distinction is worth stating because it does not hold on #260/#262, where nothing was said. What nothing currently enforces is that STEP 10 actually wires them — worth carrying into STEP 10's own review rather than blocking this.

Triage of deferred items

Nothing arrived deferred or parked; no prior reviews or comments on this PR. The PR body records one Blocker found and fixed by the author's own pre-PR review pass (f79aa3d64, validate() raising TypeError on non-string field values). I re-probed that family and it holds: severity: ["Blocker"] now yields a violation rather than a TypeError. Finding 3 is the surviving member of that same family, in the opposite direction — a value too permissive rather than one that crashes.

Merge readiness

A reader of #118 STEP 2 would find a validator that does what the step asked, including the parts that are easy to half-do. It returns every violation rather than raising on the first. It takes both documents, and the docstring at :215-221 reconstructs why the earlier one-document revision could not work — a count cannot distinguish a drop from an invention. Both-directions checking is genuinely both-directions in all four places the plan demanded it: downgrades, total_refutation, duplicate_groups, and the finding_id set. SEVERITY_ORDER is the same object as review's, not a copy, which is the difference between a shared ladder and two that can drift.

They would also find three holes where a type or an emptiness test is looser than the contract it enforces, all in the same family: falsiness standing in for non-emptiness, int accepting bool, and a validation that only runs when a list is non-empty. Finding 1 is the one that matters, because it defeats a fail-closed guarantee that two modules state explicitly and because it can publish a merge-blocking verdict with no evidence. All three fixes are one to three lines.

What I could not check: whether verdict_counts and schema_version are correct in practice, since nothing validates them yet by design. And I did not review run_adjudication.py here beyond the two lines finding 1 cites — that is #263's and #264's diff, reviewed separately.

Independence and tools

Independent of the code under review: I did not write it. Not independent across pipeline stages — one context ran the reviewers, the adjudicator and the final pass, where the skills call for a fresh context per stage. All four findings are self-adjudicated. Treat that as a limit on this report.

Tools actually held and used: Bash (git, git grep, git worktree, gh, python3 for the probes), Read, Edit, Write. No Grep or Glob tool was available in this session.

Nothing found at Blocker.

CONFIRMED	High	launchpad/review-agent/verdicts.py:268	whitespace-only verdict_evidence passes "non-empty"; same bug at run_adjudication.py:516 defeats fail-closed
CONFIRMED	Medium	launchpad/review-agent/verdicts.py:425	duplicate_groups entry with empty duplicates never validates its survivor
CONFIRMED	Medium	launchpad/review-agent/verdicts.py:322	bool passes every int check on findings_in/findings_out/findings_count
CONFIRMED	Low	PR #261 (labels)	missing required by:agent label

Handed 4 findings, confirmed 4, refuted 0, merged 1 pair — finding 1's validator and runner halves are one defect on one row. Four further candidates were REFUTED by probe and are recorded above rather than dropped silently: missing-nonce (caught upstream by findings.validate), containment findings in total_refutation (_iter_findings correctly excludes them), keys[-1] on an empty mapping (unreachable), and self-duplication (caught). No reviewer report arrived without its REVIEW COMPLETE marker, because all stages ran in one context; stated as a limit, not a pass. I did not author any of the code under review.

ADJUDICATION COMPLETE

REVIEW COMPLETE

---

Per launchpad/AGENTS.md §5 rule 1 — an agent drafts and raises, never approves or clears. This is a report, not an approval; the merge decision is @ciaran-slow's.

[ciaran-slow]

Correction to finding 2 above — reachability, not the finding itself

Reviewing #267 (STEP 7) put the producer side of duplicate_groups in front of me, and it changes one thing I wrote.

What stands: verdicts.validate does accept a duplicate_groups entry whose duplicates list is empty, and in that case nothing validates survivor at all. Re-probed just now, unchanged:

duplicate_groups = [{"survivor": "no-such-id", "duplicates": []}]   → 0 violations
duplicate_groups = [{"duplicates": []}]      (no survivor key)      → 0 violations

What I got wrong: my failure scenario said the runner might emit such a group "after a dedupe pass that grouped nothing". It cannot. _build_duplicate_groups in #267 drops any group left with fewer than two distinct, real, unclaimed finding_ids:

if len(candidate_ids) < 2:
    continue

I probed that too — a dedupe judge asked to group a finding with itself, and one returning the same pair twice, both yield a correct single group or none, never an empty one.

So the corrected reading: this is a validator-only gap, reachable from a hand-written or forged document, or from STEP 10's planned "feed every field malformed" controls — not from the current producer. That makes it materially less urgent than I framed it. I would still fix it, because catching what a producer might do wrong is the validator's whole job and STEP 10 is going to feed it exactly this, but it is not a live path.

Findings 1 and 3 are unaffected. Finding 1 in particular I re-verified end-to-end through adjudicate() on the chain tip while reviewing #266 and #267: a judge returning {"verdict": "CONFIRMED", "verdict_evidence": " \n "} still yields a CONFIRMED verdict with whitespace evidence and verdicts.validate still reports 0 violations. That one is real and live.

Revised severity for finding 2 only: Medium → Low.

CONFIRMED	High	launchpad/review-agent/verdicts.py:268	whitespace-only verdict_evidence passes "non-empty"; same bug at run_adjudication.py:516 defeats fail-closed
CONFIRMED	Low	launchpad/review-agent/verdicts.py:425	duplicate_groups entry with empty duplicates never validates its survivor — validator-only, current producer cannot emit it
CONFIRMED	Medium	launchpad/review-agent/verdicts.py:322	bool passes every int check on findings_in/findings_out/findings_count
CONFIRMED	Low	PR #261 (labels)	missing required by:agent label

This block supersedes the one in my earlier comment. Ranking is unchanged apart from finding 2 dropping below finding 3.

ADJUDICATION COMPLETE

REVIEW COMPLETE

[ciaran-slow]

Comment review recording the pipeline result. Not an approval and not a change-request — the merge decision is unchanged by this.

Reviewed via the full pipeline — detail in my comment on this PR. I ran STEP 2's two mechanical clauses myself: python3 -c "import verdicts" resolves, verdicts.SEVERITY_ORDER is review.SEVERITY_ORDER is True, and Ran 17 tests ... OK.

The validator is good work — it returns every violation rather than raising on the first, takes both documents for the reason the docstring reconstructs correctly, and checks all four of the plan's both-directions rules in both directions.

One High finding — must be fixed before this merges.

verdicts.py:268 — whitespace-only verdict_evidence satisfies "non-empty". if not finding.get("verdict_evidence") is a falsiness test and not " " is False. Probed: setting a finding's evidence to " " yields 0 violations. The same shape sits at run_adjudication.py:516 (#263), where the docstring promises to fail closed on "empty verdict_evidence" and does not — so a judge returning {"verdict": "CONFIRMED", "verdict_evidence": "\n"} publishes a CONFIRMED verdict with blank evidence, and a CONFIRMED Blocker is what blocks a merge. Fix at both ends: test the stripped value.

Also for the author, non-blocking:

• Medium, :322/:324 — isinstance(True, int) is True, so findings_in: true with every findings_count: true validates with 0 violations. total_refutation next to it uses a strict isinstance(..., bool), which shows strictness was intended. Use isinstance(x, int) and not isinstance(x, bool).
• Low, :425 — a duplicate_groups entry with an empty duplicates list never has its survivor validated at all. Downgraded from Medium after I confirmed on #267 that _build_duplicate_groups cannot emit that shape — so it is validator-only, reachable from a forged document or STEP 10's planned malformed-field controls. Correction posted on this PR.

I checked four other candidates and refuted all four — missing nonce (caught upstream by findings.validate), containment findings in total_refutation, keys[-1] on an empty mapping, and self-duplication. Recorded in my comment rather than dropped.

[ciaran-slow]

Change-request review. The finding(s) below are the author's to resolve before this merges — full detail, probes and line citations are in my pipeline comment on this PR.

Reviewed via the full pipeline — detail in my comment on this PR. I ran STEP 2's two mechanical clauses myself: python3 -c "import verdicts" resolves, verdicts.SEVERITY_ORDER is review.SEVERITY_ORDER is True, and Ran 17 tests ... OK.

The validator is good work — it returns every violation rather than raising on the first, takes both documents for the reason the docstring reconstructs correctly, and checks all four of the plan's both-directions rules in both directions.

One High finding — must be fixed before this merges.

verdicts.py:268 — whitespace-only verdict_evidence satisfies "non-empty". if not finding.get("verdict_evidence") is a falsiness test and not " " is False. Probed: setting a finding's evidence to " " yields 0 violations. The same shape sits at run_adjudication.py:516 (#263), where the docstring promises to fail closed on "empty verdict_evidence" and does not — so a judge returning {"verdict": "CONFIRMED", "verdict_evidence": "\n"} publishes a CONFIRMED verdict with blank evidence, and a CONFIRMED Blocker is what blocks a merge. Fix at both ends: test the stripped value.

Also for the author, non-blocking:

• Medium, :322/:324 — isinstance(True, int) is True, so findings_in: true with every findings_count: true validates with 0 violations. total_refutation next to it uses a strict isinstance(..., bool), which shows strictness was intended. Use isinstance(x, int) and not isinstance(x, bool).
• Low, :425 — a duplicate_groups entry with an empty duplicates list never has its survivor validated at all. Downgraded from Medium after I confirmed on #267 that _build_duplicate_groups cannot emit that shape — so it is validator-only, reachable from a forged document or STEP 10's planned malformed-field controls. Correction posted on this PR.

I checked four other candidates and refuted all four — missing nonce (caught upstream by findings.validate), containment findings in total_refutation, keys[-1] on an empty mapping, and self-duplication. Recorded in my comment rather than dropped.

[serina-mcfall]

Independent adjudication of the change-request findings

Adjudicated in a fresh context, separate from the pipeline that produced the findings. That matters here because the review states its own limitation plainly — one context ran the reviewers, the adjudicator and the final pass, so its findings were self-adjudicated. This pass supplies the independent adjudication that was missing. It does not supply an independent second reviewer, so anything both passes missed is still missed.

Head is unchanged since the review (77065b95b, last commit 2026-08-20T21:38Z, review 2026-08-21T03:56Z). All three code findings are live.

The three mechanical claims verified: import verdicts resolves, verdicts.SEVERITY_ORDER is review.SEVERITY_ORDER is True, and Ran 17 tests ... OK.

Findings

1. verdicts.py:268 — whitespace verdict_evidence — CONFIRMED, High (unmoved).

The hole is wider than reported. There is no type check at all, so every truthy value passes:

'   ' -> 0 violations      42    -> 0 violations      True     -> 0 violations
['x'] -> 0 violations      {'a':1} -> 0 violations    ''       -> 1 violation  <-- control

The empty-string control firing is what proves the check exists and that everything else slips past it specifically. This also removes the one available defence — that " " is literally non-empty and the code implements the plan's wording faithfully. verdict_evidence: 42 cannot be defended under any reading, and :263 five lines above does not isinstance(verdict, str) or verdict not in VERDICTS — the file demonstrating its own intended strictness.

Not documented: validate's docstring names exactly three unchecked keys (schema_version, verdict_counts, notes) and verdict_evidence is not among them. grep -n strip verdicts.py returns nothing.

High rather than Blocker: nothing on this branch calls validate except the test suite, and no producer of verdict_evidence exists in this diff. It becomes wrong the moment STEP 3's runner lands — which is #263, based directly on this branch.

2. verdicts.py:322/:324 — bool passes the int checks — CONFIRMED, Medium (unmoved).

Reproduced: findings_in/findings_out/findings_count all true -> 0 violations. The asymmetry in one run — total_refutation: 0 is rejected as an int where True is accepted as an int two checks earlier.

One narrowing the review did not mention: findings.validate catches findings_count: False because it compares against len(findings) by equality. That only helps when the count is not 1 — with a single finding, True == 1 == len(findings) and the guard is silent. So the hole needs the bool to numerically match.

Medium because the producer cannot emit it (findings_in is computed with len(...)), but a maintainer reading :324 next to the strict :396 will conclude the count checks are equally strict, and they are not.

3. verdicts.py:425 — empty duplicates leaves survivor unvalidated — CONFIRMED, Low (unmoved).

Three shapes validate clean, including a missing survivor key and an integer survivor. Two controls confirm the mechanism: an entry with no duplicates key at all is caught by the isinstance guard at :427, and a bogus survivor with a non-empty list is caught indirectly — the survivor is only ever checked through a member pointing back at it.

The reviewer's own Medium -> Low downgrade was re-verified rather than accepted: on the chain tip, _build_duplicate_groups gates on len(candidate_ids) < 2 and selects survivor from candidate_ids, every member filtered on fid in findings_by_id. Producer-unreachable.

Where the fix belongs, and one row un-merged

The review merged verdicts.py:268 with the runner's guard on #263 as "one defect, two sites, counted once." Splitting them, on two grounds:

• run_adjudication.py does not exist on this branch, so a merged row here has no actionable second half.
• They enforce different promises. verdicts.py:268 is the contract validator (ADJUDICATION.md's "non-empty on all three"). The runner's guard is a fail-closed degradation path promising to substitute UNPROVEN. Fix only the validator and the runner still forwards whitespace; fix only the runner and the validator still accepts a forged or --replay-loaded document.

The primary fix belongs here, on the lowest branch in the chain — str(...).strip() plus a type check at :268 propagates to #263 -> #264 -> #266 -> #267 and catches a whitespace verdict from any future producer. The Medium and the Low are cheap to take in the same change.

Two notes for the reviewer harness

• Citation correction: the review cites the runner site as run_adjudication.py:516. That is the line number on the chain tip (feat/review-agent-adjudication-dedupe); on feat(launchpad): run_adjudication.py -- the adjudication CLI (#118 STEP 3) #263's head the same guard is at :188. The file grows from 335 to ~700 lines across the chain. The site is real, the number is off by a chain's worth of insertions.
• Two verdict blocks were emitted across two comments, the second superseding the first. A gate scraping comments sees two authoritative-looking blocks with nothing to tell them apart. Worth fixing in the harness.

Verdict

state	severity	file:line	summary
CONFIRMED	High	launchpad/review-agent/verdicts.py:268	Falsiness test admits whitespace and any truthy non-string as "non-empty"
CONFIRMED	Medium	launchpad/review-agent/verdicts.py:322	bool passes the int checks on the three count fields
CONFIRMED	Low	launchpad/review-agent/verdicts.py:425	Empty duplicates list leaves survivor unvalidated

Confirmed 3, refuted 0, resolved 1 (the by:agent label, applied 02:02Z). One row un-merged from the reviewer's block. No severity moved. Every finding reproduced on a probe against the branch's own test fixture, with a control mutation in each case proving the check under test fires on neighbouring input.

The by:agent label finding was real at review time and is fixed.

🤖 Adjudicated by Claude Code (claude-opus-5) for @serina-mcfall. I authored none of these findings and none of the code under review; this pass was read-only.

[benmitchell11]

Independent pass (not the pipeline that left the earlier reviews on this PR).

Checked out feat/review-agent-adjudication-verdicts locally and ran python3 -m unittest discover -s launchpad/review-agent -p 'test_*.py' myself: Ran 150 tests ... OK, in a clean worktree. That's more than the Ran 17 tests the PR body's own Verification section quotes, because this branch's tip already carries fix commits (e2e953faa, f79aa3d64, 77065b95b) added after that section was written -- worth updating the PR body so Verification reflects the current tip, but not a blocker.

Re-verified the three findings from the standing review myself rather than trusting either account:

• Whitespace-only verdict_evidence -- verdicts.py:37-48 now defines is_nonempty_str (isinstance(value, str) and bool(value.strip())) and it's used at the evidence check (:295) and the severity_reason check (:313). I built a minimal output document with verdict_evidence: " " and called validate() directly: it now correctly reports "verdict_evidence must be a non-empty string, got ' '". Fixed.
• isinstance(True, int) admitting booleans as counts -- is_int (:51-61) now explicitly excludes bool, used at both findings_count (:356) and findings_in/findings_out (:366). Fixed.
• Empty-duplicates group skipping survivor validation -- :472-481 now checks survivor unconditionally, before the loop over duplicates, with a comment explaining exactly why it moved. Fixed.

I didn't find anything new. The module is well-scoped (three adjudication keys deliberately left to STEP 10, documented rather than silently gapped) and validate()'s never-raise, collect-everything design holds up under the probes above.

Not verified: no live model or subprocess involved here (pure dataclasses/validation), so nothing beyond the test run and my own probes applies.

[serina-mcfall]

Requested changes are done — but I am not the right party to certify it

Disclosure first: I wrote some of the commits on this branch, including the fix for the blocker @ciaran-slow raised. So this is a status report, not an independent review, and I am not clearing it.

@ciaran-slow's change-request (2026-08-21T03:56Z) is stale — the head has moved to 8455ddb5b since, most recently by merging #263. What landed against his three findings:

• High, verdicts.py:268 — whitespace and any truthy non-string satisfying "non-empty". Fixed: is_nonempty_str is present at head and the guard now tests it. The hole was wider than reported — there was no type check at all, so 42, True, ['x'] and {'a':1} all validated clean.
• Medium, :322/:324 — bool passing the int checks. Fixed: is_int present, excluding bools.
• Low, :425 — empty duplicates leaving survivor unvalidated. Fixed in the same pass.

Also fixed, and worth flagging because nobody reported it: severity_reason at :283 carried the identical idiom eleven lines from the reported site, so a severity re-rating could be justified by whitespace. Found while fixing its neighbour.

Both predicates were made public deliberately so #263's producer guard imports the rule rather than re-implementing it — two private copies of one contract rule is how these drifted apart in the first place.

What this needs: a re-review from @ciaran-slow, whose findings these were, rather than a clearance from me. His change-request is stale and will not clear itself, and I should not be the one dismissing findings against a fix I wrote. I have re-requested his review.

Non-blocking follow-ups from the same pass are filed and need nothing here.

[tucktuck101]

Reviewed current head and the existing review history. The previously confirmed High/Medium findings have been fixed and independently re-verified; current CI is green, and the accumulated STEP 2 + STEP 3 scope is intentional. Approving this revision

reviewed changes myself