feat(launchpad): adjudicator.md -- what the judge is told (#118 STEP 5) - #265
Conversation
Normative sibling to ADJUDICATION.md, FINDINGS.md, and CONTAINMENT.md: states the adjudicator's scope, its four exclusions (no restating a claim as evidence, no hunting new defects, no approval-shaped output, no refuting for want of evidence -- UNPROVEN is the default), that anchor "pr" is a legitimate finding shape rather than a malformed one, and a worked restatement-vs-independent-check contrast. Names no model, per #117/#118's own framing. This step's done-when is textual on purpose, per the plan: whether the exclusion clauses work behaviorally is STEP 9's job (live recordings), not this one's, so no test/build command applies here. The .claude/.verified stamp for this commit was touched, not earned by a suite -- nothing in this worktree runs, imports, or lints this markdown file (verdicts.py and run_adjudication.py, which would consume it, live on sibling unmerged branches per this build's own scope: STEP 5 needs only STEP 1). Signed-off-by: Serina Mcfall <serina.mcfall@gmail.com>
… STEP 5) review-code found that adjudicator.md's citation of run_adjudication.py's _location_description behavior named an unmerged sibling branch without a commit SHA, unlike ADJUDICATION.md's own citation convention. This plan's own text has been burned three times by exactly this failure mode -- a true-when-written cross-branch claim going stale before merge -- so pin it to 2be90c6 (feat/review-agent-adjudication-nonce, PR #264, CI-green) rather than leaving it to drift silently if that branch is revised again before #118 merges. No test suite applies to this docs-only file -- .claude/.verified touched per verify-gate's own stated escape hatch for a project with no runnable suite, stated here rather than silently worked around. Signed-off-by: Serina Mcfall <serina.mcfall@gmail.com>
Review pipeline — PR #265Stages run: Not applicable, declared rather than faked:
STEP 5's done-when, clause by clause
All six met. I checked each mechanically rather than reading for impression. Findings1. High — the document normatively directs the judge to write to
|
ciaran-slow
left a comment
There was a problem hiding this comment.
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.
The document itself needs no change. I checked all six of STEP 5's done-when clauses mechanically and every one is met: four exclusions each with its reason, UNPROVEN stated in those words, a worked restatement-vs-independent-check contrast, anchor pr declared legitimate, and no model named anywhere (grepped for every vendor and family). The worked contrast at :41-59 is the strongest part — it distinguishes restatement from real evidence by a criterion that is actually operable: "whether the evidence could have been written by someone who never opened the file."
I also expected the commit citation to be stale and it is not. 2be90c629 exists, is an ancestor of #264, and _location_description at that commit does branch on anchor before touching file/line, exactly as claimed. Given the plan records getting this class of claim wrong three times and deriving a standing rule from it, that is worth naming.
Two holds, neither a defect in this diff: a dependency and a merge-order constraint.
-
:66is contingent on #263. It normatively instructs the judge to record new observations inadjudication.notes, and the runner provides no channel —notes=[]is hardcoded and the judge protocol has no such key. Probed: a judge returningnoteshas it silently dropped, leavingverdict_evidenceas its only outlet, which is the field the plan's OPEN item flags as unguarded. If #263 gains the wire, this line is correct as written and needs nothing. If the channel is deferred instead, this line needs to say so. Either way the resolution happens on #263, so this PR should not merge ahead of that decision. -
Merge after #264. This PR's base is
launchpad, andrun_adjudication.pydoes not exist there — I verified. The document cites it at:112-118and discloses that honestly, so it is not a hidden dangling reference, but landing ahead of #261→#263→#264 leaveslaunchpadcarrying a normative document citing a function no file onlaunchpaddefines.
…es deferral (#118 STEP 3) Two findings adjudicated on #263. **`_run_judge_safely` did not fail closed on blank evidence.** The guard was `verdict not in verdicts.VERDICTS or not evidence` -- a truthiness test, so `not " "` is False and whitespace passed as usable output. Reproduced through the shipped `--replay` flag, no code injection: a recording carrying `verdict_evidence: " \n "` published a CONFIRMED verdict at exit 0, and `verdicts.validate` returned zero violations because the contract check used the same idiom. A CONFIRMED Blocker is what blocks a merge, so this could publish a merge-blocking verdict with no stated reason. The rule is now `verdicts.is_nonempty_str`, imported rather than re-implemented. That matters more than the strip() itself: this producer guard and the contract check drifted apart precisely because each had its own copy, and each admitted whitespace because the other did. The docstring said "empty verdict_evidence", which was literally accurate -- `" "` is not empty -- so it has been corrected to say what the guard enforces. The promise it actually broke was the clause before it, "fail closed on anything unusable", and ADJUDICATION.md's own words it quotes. **`adjudication.notes` was hardcoded empty with no deferral stated.** Every other hardcoded-empty field is named in the module docstring's STEP 6/7 deferral list; `notes` was the one that was not, while ADJUDICATION.md declares it and `verdicts.py` carries it -- so a reader had every reason to assume the channel worked. Now documented, at the docstring and at the assignment. Deliberately NOT resolved here: `adjudicator.md` (#265) normatively tells a judge to "record it in `adjudication.notes`", against a protocol that drops the key. Plumbing `notes` means designing how notes are collected and attributed, which is a STEP 6/7 decision, and amending #265 is #265's call. The tension is now stated in the code so it cannot merge past unnoticed rather than silently picked. 6 tests added, each failing before the change. One of them originally passed for the WRONG reason and was fixed: the replay recording format is a mapping `finding_id -> {...}`, not a flat record, so the first version missed the lookup entirely and asserted UNPROVEN against "no recorded judge output". It now carries a control proving the lookup HITS and returns CONFIRMED on good evidence, so the UNPROVEN in the blank cases is the guard firing. 25 tests in test_run_adjudication (was 19), 176 across launchpad/review-agent. Refs #118 Signed-off-by: Serina Mcfall <serina.mcfall@gmail.com>
Brings #264's `StagesShapeError` / `_input_stages` and #261/#263's `verdicts.is_nonempty_str` up to the chain tip. Both of this PR's findings were homed on earlier branches -- the `stages` Blocker on #264 and the `notes` drift on #263 -- so this branch is cleared by propagation rather than by any change of its own, which is what the adjudication asked for. Clean merge, no conflicts. 230 tests across launchpad/review-agent. `notes` remains empty and is now documented as deferred at STEP 6/7, with the unresolved tension against `adjudicator.md` (#265) stated in the code. #265 should not merge ahead of that decision. Refs #118 Signed-off-by: Serina Mcfall <serina.mcfall@gmail.com>
…118 STEP 5) Prohibition 2 told the judge to "record it in `adjudication.notes`". That is a normative instruction against a channel that silently drops its input: `run_adjudication.py` hardcodes `notes=[]` and its judge protocol reads only `verdict`, `verdict_evidence`, `severity` and `severity_reason`. Probed on the STEP 3 branch -- a judge returning a `notes` key produces `adjudication.notes: []`, and the document validates clean against both contracts, so nothing catches the loss. `ADJUDICATION.md` declares the field and `verdicts.py` carries it, which is what made the instruction look supported. Nothing between the declaration and the producer connects them. Adjudicated across #263 and #267 as one cross-step drift. Resolved on this side rather than by plumbing `notes` through the protocol, because collecting and attributing notes -- per-finding or per-document, deduped, ordered -- is STEP 6/7's design, and building that surface here would pre-empt a decision those steps own. The deferral is now recorded from both sides: this document and `run_adjudication.py`'s module docstring. Also states the consequence the old wording left implicit: with the channel deferred, a new observation has nowhere to go from this stage, so `verdict_evidence` must not be stretched to carry it. That field is the reason for the verdict on the finding the judge was given, and it is the one field with no structural guard -- exactly the wrong place for overflow. Doc-only. 121 tests across launchpad/review-agent green on this branch. Refs #118 Signed-off-by: Serina Mcfall <serina.mcfall@gmail.com>
benmitchell11
left a comment
There was a problem hiding this comment.
Independent review — first pass on this one, no standing review to cross-check against.
Checked out feat/review-agent-adjudicator-md and ran the suite: Ran 121 tests ... OK, matching the PR body's own claim of "confirms nothing regressed" (this is a docs-only change, correctly not claiming to prove anything about the diff itself).
Stacking note, for the record rather than as a problem: this PR's base is launchpad directly, not feat/review-agent-adjudication-nonce (STEP 4) like the review-in-step-order assignment would suggest. The PR body states this is deliberate ("Depends only on STEP 1 ... so it branches directly off launchpad rather than stacking on the in-flight STEPs 2-4") and that holds up — it's a pure markdown addition with no code dependency on STEPs 2-4. Worth flagging only because it means this PR does not carry #264's nonce/stages fixes or #263's whitespace-evidence fix, so a reader diffing branches shouldn't expect them here.
Content review:
- The four prohibitions are each given a stated reason, and prohibition 1's worked contrast (restatement vs. independent check) is genuinely useful — it's concrete enough that a reader could apply the test to a real judge transcript.
- Prohibition 3 ("no approval-shaped output") is honest about being only partially mechanical: the doc itself says
_FORBIDDEN_KEYSlives inverdicts.validate, which nothing calls automatically on the runner's own output before printing, so "no such key can exist" is stated as not proven, only "the runner writes only known keys, so it cannot produce a forbidden one today." That's the right amount of honesty for a spec document and matches what I saw readingrun_adjudication.pydirectly on the other branches —adjudicate()/main()indeed never callverdicts.validateon their own output. - The
notesamendment (commit05a960478) is real and matches what it claims: the original text told the judge to "record it inadjudication.notes", and this PR corrects that to say the channel is deferred and not to useverdict_evidenceas a substitute dumping ground either. I confirmed independently (readingrun_adjudication.pyonfeat/review-agent-adjudication-run/-escalate/-dedupe) thatnotes=[]is hardcoded and the judge protocol has nonoteskey — so this document's corrected claim is accurate. Worth noting for the author: #267's standing review flagged this exact cross-step drift and suggested "resolution belongs on #263" — this PR resolves it from the adjudicator.md side instead, which is arguably the more natural place (it's the instruction that was wrong), but the two PRs' resolutions should be cross-referenced so a reviewer of #267 doesn't think it's still open. - The
anchor: "pr"section's citation torun_adjudication.pypins a specific commit (2be90c629) on the still-unmergedfeat/review-agent-adjudication-noncebranch rather than citing the branch alone, explicitly to avoid citation rot before that branch merges — I confirmed that commit exists on that branch's history and does contain the_location_descriptionfunction as described (anchor branched on first,prcase never falls through to aNone:Noneformat string).
No blockers. Clean.
Not an independent review — I wrote part of this PRFlagging a conflict rather than posting a verdict, because a review is worth nothing if the reviewer wrote the thing. Commit So I cannot assess this PR's central document. I would be marking my own work, and the one thing that made the earlier adjudication of this stack worth anything was that a fresh context checked what the authoring context had claimed. What I can say without judging the content:
What it needs: a reviewer who did not write The specific question a real reviewer should press on: the deferral I wrote resolves the contradiction by telling the judge not to rely on the channel, rather than by making the channel work. That was @serina-mcfall's explicit decision between three options, and it is defensible — but it means a judge that genuinely notices a new defect has nowhere to put it until STEP 6/7, and the document now says so plainly. Whether that is acceptable, or whether |
) (#430) Reviewing a batch of PRs has two halves. One is judgement -- is this claim true, does the conclusion depend on this defect. The other is bookkeeping applied identically to every PR. This extracts the second half, which is the line ADR-0019 draws and the same extraction pr_body_check.py and adr_boundary_check.py already did for their own rules. Six classifiers, each one a rule applied by hand across three review batches on 2026-08-21/22, and each one applied WRONGLY at least once: - STALE/MISFILED reviews. Four PRs carried change-requests already satisfied. #262's blockers were fixed at 03:21 and the review restating them arrived at 03:57. #271's change-request was #275's review MISFILED -- textually identical including a "same as #271's" self-reference. No change to #271 could have addressed it. - CI triage. #268's red CI was setup-mold timing out on a one-markdown-file PR. #288's log printed four inherited warnings above the real blocker. - Independence. #265 carried a commit written in the reviewing session. - Leak scan. #281 quoted a private hook's header in a public file. - Placement, per AGENTS.md section 3. - Drift calibration. #374's "796 files" was reported REFUTED by a reviewer who measured at the live tip and got 912. The script emits the pinned SHA so a count that does not reproduce reads as drift, not error. It emits no severity. Five proposed blockers were demoted and one upheld across those batches, each turning on whether a conclusion depended on the defect; a script guessing that is the model-gating ADR-0019 forbids wearing automation's clothes. test_the_briefing_states_no_severity_anywhere asserts the absence. It also posts nothing, so it can run read-only. Three defects found by running it against live PRs rather than by reading it, each now a regression test: 1. FALSE MISFILED on #374. The only path token in a genuine review was `launchpad/ARCHITECTURE.md`, cited as corroborating evidence rather than as a defect site. Reviews cite files outside the diff constantly -- that is what checking a claim looks like. MISFILED now needs two or more cited paths, none in the diff, and no mention of any changed file. 2. Selecting log lines BY POSITION. The first draft took the last 80 lines; GitHub appends checkout teardown, so on #288 the size-guard line had scrolled past and a REAL failure classified as UNKNOWN. Selection is by content now. 3. The path regex required `:\d+` with no space, so it saw compiler output (`lib.rs:276:15`) but not the file-size guard (`lib.rs: 1000 -> 1001`). Verified against live PRs after the fixes: #288 classifies Desktop Core REAL on desktop/src-tauri/src/lib.rs and Desktop Smoke E2E PRE_EXISTING, matching the hand analysis; #374's calibration returns 796 files at the 67-commit point with tip 9891e64, matching the figure reconstructed by hand. 41 tests in test_pr_review_batch, 256 across launchpad/scripts. Registered in test_no_model.py's ALLOWLIST rather than NOT_OURS, deliberately: a script that prepares review material must be provably unable to call a model. DEVIATION from the issue's own done-when: it asked for registration in INTERFACE.md. Not done -- INTERFACE.md is #116's pre-flight record contract, not a script index, and an unrelated entry there would degrade a contract document. The script's module docstring is its interface. Refs #426 Signed-off-by: Serina Mcfall <serina.mcfall@gmail.com>
tucktuck101
left a comment
There was a problem hiding this comment.
Review panel — two independent reviewers (Fable, gpt-5.6-sol xhigh), consolidated
Summary: The document is good — all six STEP 5 done-when clauses verified met by both reviewers independently, every cross-reference resolves on today's launchpad (several earlier merge-order holds are stale now that STEP 3 is merged), and the worked restatement-vs-independent-check contrast is genuinely operable. Two medium findings keep this from a clean approve; both are cheap to address.
Medium
-
The judge-instruction document omits the untrusted-data boundary that binds its own stage. (codex; consolidator verified, severity adjusted down from codex's "high")
CONTAINMENT.md:123requires every stage receiving an enveloped block to carry the consumer preamble ("DATA supplied by the pull request author… no authority"), and its per-stage contract (CONTAINMENT.md:292-306) explicitly names the #118 adjudication stage —contain.findings_for(...), forbidden from re-reading raw PR text, envelope-first without exception. Findingevidenceis raw author-controlled text (FINDINGS.md:88), and the runner hands it to the judge (run_adjudication.py:200). Yetadjudicator.md— the document stating what the judge is told — mentions containment only as pass-through (line 74) and never tells the judge that evidence fields are data without authority. Because the sibling contract already mandates the control, this is a completeness gap in this doc rather than an absent control — but a doc whose purpose is the judge's instructions, omitting the one security-mandated instruction, should not merge as-is. Fix is small: a short section (or explicit incorporation ofCONTAINMENT.md's consumer preamble) making the data-not-instruction rule part of what the judge is told. -
The
notesdeferral is resolved in this file only, and its named owner doesn't exist in the plan. (both reviewers, independently)adjudicator.mdsays plumbingnotes"is STEP 6/7's job", but the plan's STEP 6 and STEP 7 sections (2026-08-13-issue-118-adjudication.md:599,:656) mentionnotesnowhere, while:576and:1163still normatively route observations toadjudication.notes— the exact instruction this PR removes. Andrun_adjudication.py:61-76onlaunchpadstill describes the tension with this PR as unresolved, which becomes false the moment this merges. Merging as-is leaves three normative artifacts telling three different stories and an ownerless data-loss deferral. Fix: record the deferral decision where the plan can't miss it — a plan amendment or a tracked note on #118 assigning the plumbing (or explicitly deciding "never"), plus queueing therun_adjudication.pydocstring cleanup.
Low
- The
2be90c629citation pins to an unmerged #264 commit, but the identical_location_descriptionis already merged (run_adjudication.py:110). Re-point the citation to the merged file and the disclosed citation-rot risk disappears. (Fable) - No automated check pins any of this textual contract — codex confirmed the suite passes identically with the file absent, and no CI runs these suites. The PR body already says so; noted for the record, not actionable in this step. (codex)
- Missing
by:agentlabel — the body carries an Agent provenance block. Onegh pr edit --add-label. (Fable)
What looks good
- Done-when: four exclusions each with a stated reason;
UNPROVENin those words as the fail-closed default; the worked contrast ("could this evidence have been written by someone who never opened the file?"); anchorprlegitimacy; no model named — all six verified by both reviewers. - The
notesamendment box itself is a model of honest spec-writing: states the deferral, the consequence, and forbids stuffingverdict_evidence— precisely the unguarded field the plan worries about. - The escalation trail (self-disclosed conflict of interest, named human decision) is exactly what the cohort's process asks for, and the prior benmitchell11 review satisfies serina's independent-reviewer condition.
Verdict: request changes — items 1 and 2. Both are small; item 2 can even be satisfied by a tracked follow-up comment on #118 plus one line in this doc naming it.
Panel: fable + gpt-5.6-sol (codex, xhigh), independent reviews consolidated by the lane; the consolidator re-verified the deciding findings against the repo (including downgrading one) before issuing the verdict. Posted under Jeff's standing authorization.
…118) Found by re-running the trial merge after `launchpad` moved 88 commits ahead. The merge is still conflict-free and there is no file-level overlap -- which is exactly why this needed looking for: the two halves live in different files, so a clean merge cannot see it. `run_adjudication.py`'s docstring said: **This deferral is in tension with ``adjudicator.md`` (#265), which normatively tells a judge to "record it in ``adjudication.notes``".** `adjudicator.md` was amended on `launchpad` in `05a960478` ("stop mandating a notes channel the runner discards"). It now says the channel is deferred and a judge "must not rely on it", and records that its own earlier paragraph said otherwise. So the claim above is already false on the merge target: this branch would have landed a docstring describing a document in the same directory as saying the opposite of what it says. Rewritten to record that the tension is resolved, and in which direction, rather than deleting the paragraph -- the two facts that outlived the tension are the reason it existed: - a judge's only free-text outlet is still `verdict_evidence`, the field ADJUDICATION.md identifies as having no structural guard - "deferred to STEP 6/7" was already wrong on its own terms: both steps are in this branch and neither plumbed `notes` So the field is now described as what it is -- empty, with no step plumbing it. Plumbing it remains unowned. Calling it "deferred to STEP 6/7" made a live gap look like a scheduled one. Also loosens the test that pinned the stale sentence. `test_run_adjudication.py:507` asserted `notes.*(defer|STEP 6/7|left empty)`, which accepted the wrong wording -- so correcting the docstring meant editing a green test, making a fix look like a regression. It now asserts the FACT (the field is empty and the docstring says so) and additionally asserts the stale shape does NOT return, so the claim cannot silently regain a scheduled-sounding owner. A test that pins prose it cannot verify is a test that defends a stale claim. Proved the new one is load-bearing in both directions, in scratch copies: restoring the "deferred to STEP 6/7" wording turns it red, and deleting the notes paragraph entirely turns it red. 265 tests OK, run_controls.py 13 passed 0 failed 0 skipped. Trial merge onto `9e39bab36` conflict-free. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Serina Mcfall <serina.mcfall@gmail.com>
Summary
Implements STEP 5 of
launchpad/plans/2026-08-13-issue-118-adjudication.md:adjudicator.md, the document stating what the judge is told. Depends only on STEP 1 (ADJUDICATION.md, merged via #256), so it branches directly offlaunchpadrather than stacking on the in-flight STEPs 2-4. Docs-only.Related issue
Refs #118
Issue type
Task
Agent provenance
Objective
Add
launchpad/review-agent/adjudicator.md— a prompt-definition file, sibling toADJUDICATION.md/FINDINGS.md/CONTAINMENT.mdand in the same normative voice.Impacted components
launchpad/review-agent/adjudicator.mdApproach and rejected alternatives
States the judge's scope and four prohibitions: do not restate a claim as its own evidence, do not hunt for new defects, do not emit approval-shaped output, and do not refute for want of evidence (UNPROVEN is the fail-closed default). Includes a worked contrast between a restatement and an independent check, because the distinction is the one a reader is most likely to get wrong, and states that anchor
pr(file and line both null) is a legitimate finding shape rather than malformed.Rejected: building a behavioural test that the prohibitions hold under a live model. Whether an exclusion clause survives contact with a real adjudicator is a property of output, which needs STEP 9's recorded before/after pairs — and tagging this step against STEP 9 would make the dependency circular. This step's own plan text says its done-when is deliberately textual, and no harness was expected here.
Rejected on the
notesprohibition: leaving it as originally written. See Escalations — it mandated a channel the runner discards.Verification
Command run:
Raw output:
Docs-only, so that run confirms nothing regressed rather than proving anything about this diff — nothing in it touches
launchpad/review-agent/*.py.The
notesclaim this PR now corrects, probed on the STEP 3 branch where the runner lives — a judge returning anoteskey alongside a valid verdict:The value is discarded and both contract checks pass, so nothing catches the loss.
Not verified
Nothing here is verified against a live model, by design. Every claim in this document is about what a judge is instructed to do; whether an instruction changes model behaviour is unmeasured and unmeasurable until STEP 9 exists. Treat this as a specification, not as evidence of behaviour.
The worked restatement-vs-independent-check contrast is illustrative, not sampled. It was written to make the distinction legible, not drawn from real adjudicator output — so it may not resemble the failure mode a real judge actually exhibits.
The four prohibitions have no mechanical enforcement. Prohibition 3 (no approval-shaped output) is the only one with any:
verdicts.py's_FORBIDDEN_KEYSwalk. And that walk lives inverdicts.validate, which neitheradjudicate()normain()calls on its own output before printing — so it is a validator a downstream caller may run, not an enforcement in the producer. The conclusion still holds (the runner writes only known keys, so it cannot produce a forbidden one), but "no such key can exist" would be stronger than the code supports. Prohibitions 1, 2 and 4 are textual only.Security implications
Adds one markdown file; no change to exposure. What it records is security-relevant: prohibition 3 is the document half of "escalate, never approve", which is the property that keeps an agent from approving work — forbidden by
launchpad/AGENTS.mdrule 1. Prohibition 4 makes UNPROVEN the default, so a judge that cannot establish something escalates rather than clearing it.The
notesamendment closes a small integrity gap rather than opening one. Instructing a judge to write into a channel that silently discards its input meant a genuine new observation had exactly one remaining outlet —verdict_evidence, the field with no structural guard. The amendment says so, and says not to stretch that field instead.Escalations
adjudication.notes".ADJUDICATION.mddeclares that field andverdicts.pycarries it, which made the instruction look supported — butrun_adjudication.pyhardcodesnotes=[]and its judge protocol never reads the key. Adjudicated across feat(launchpad): run_adjudication.py -- the adjudication CLI (#118 STEP 3) #263 and feat(launchpad): dedupe via an injectable second judge (#118 STEP 7) #267 as one cross-step drift. Resolved on this side rather than by plumbingnotesthrough the protocol, because collecting and attributing notes is STEP 6/7's design and building it here would pre-empt a decision those steps own. Decision taken by @serina-mcfall.notes, not to relax prohibition 2.run_adjudication.pycitation is pinned to2be90c629rather than to a branch name, after a Low finding on the citation-rot risk. That SHA is on feat(launchpad): nonce check and stages manifest (#118 STEP 4) #264, which is not merged — so the citation will need re-pinning if feat(launchpad): nonce check and stages manifest (#118 STEP 4) #264's history is rewritten.🤖 Drafted by Claude Code (
claude-opus-5) for @serina-mcfall.