ADR-151: Persona verdict-line directive + body-parser semantics for gyrum-review-pr
Status: Accepted Date: 2026-05-06
Context
gyrum-review-pr runs the 3-persona AI review panel
(Priya / Marcus / Lin) against an open PR. Each persona is implemented
as an LLM that produces a review BODY (Markdown prose) and a wrapper
that translates that body into a GitHub Reviews API call carrying both
body and event (one of APPROVE, REQUEST_CHANGES, COMMENT).
The merge gate (the gyrum-review-pr post-condition that
gyrum-complete-pr consumes) fires on the GitHub-side review state,
not on the body.
Tonight's PR
gyrum-labs/devtools#243
(warp#1448 kanban-move) hit the failure mode this ADR locks shut: the
gyrum-marcus body opened with APPROVE — reasoning is sound, but the
wrapper translated that to event: REQUEST_CHANGES and the resulting
review carried state CHANGES_REQUESTED. The merge gate read state,
saw an outstanding changes-request, and refused to clear. The operator
had to authorise --admin to unblock — burning a structural rule
(no admin overrides without written authorisation per CLAUDE.md) on a
problem the wrapper itself created.
This is a recurring class. Three independent failure modes share one root cause:
- Persona-PAT review-state inconsistency. LLM emits prose-level approval; wrapper code (warp#1392 cluster, persona-as-employee with PAT-per-persona) misclassifies body→state. Multiple instances observed today.
- CHANGES_REQUESTED persists across new commits. GitHub keeps the
prior review's state until explicitly dismissed, so re-running
gyrum-review-prposts new reviews while the staleCHANGES_REQUESTEDfrom a prior HEAD hangs around as outstanding. - Empty-commit re-trigger wastes CI. Pushing an empty re-trigger commit re-fires the persona panel even though the diff is empty; warp#1499 quick-mode catches trivial diffs but not the literal no-diff case.
The structural cause is one shape: gyrum-review-pr has two sources
of truth — body prose (LLM judgment) and GitHub review state (the
gate signal) — that can disagree, with no convergence mechanism. This
is the same bug-class as warp#1330 (auto-tally Refs warp#NNN parser
treated Refs as Closes; prose said one thing, action took another;
fix was to narrow the parser) and warp#1407 (changelog-entry-validate
gate trusted both prose claim and structured flag; fix was to validate
the structured block alone). Both shipped fixes followed the same
pattern: the structured directive in the body is the source of truth;
structural action follows it deterministically.
EFF-7 (warp#1559) is the EPIC that closes this class for the persona panel. This ADR is Phase 0 — the design lock that Phases 1–4 implement against. Phase 1 ships the wrapper change; Phase 2 ships auto-dismiss-on-fresh-HEAD; Phase 3 ships the no-diff quick-tier; Phase 4 ships the regression corpus including the devtools#243 fixture.
Decision
Every persona review body produced by gyrum-review-pr's persona panel
MUST end with a verdict line of exactly one of three forms, on its
own line, as the last non-blank line of the body:
## VERDICT: APPROVE
## VERDICT: CHANGES_REQUESTED
## VERDICT: COMMENT
The wrapper parses that line and MUST set the GitHub event
parameter to match:
| Verdict line | GitHub event |
|---|---|
## VERDICT: APPROVE |
APPROVE |
## VERDICT: CHANGES_REQUESTED |
REQUEST_CHANGES |
## VERDICT: COMMENT |
COMMENT |
Concretely:
Format. The verdict line matches the regex
^##\s+VERDICT:\s+(APPROVE|CHANGES_REQUESTED|COMMENT)\s*$(case-sensitive on the keyword; one verdict token; no trailing prose on the line). Variation is rejected.Parser semantics. The parser scans the body bottom-up and takes the last matching line as the verdict. Earlier matches (e.g. quoted in narrative — "I considered
## VERDICT: APPROVEbut…") are ignored; the last-line rule keeps the directive at a structural position humans and the parser agree on.Missing verdict line. If no line matches the regex, the wrapper aborts before calling the GitHub Reviews API. No review is posted; the persona panel fails fast with a structured error pointing at the malformed body. The operator's recourse is to re-run the persona (which re-prompts with the directive) or open a Warp ticket on the persona prompt template if the failure recurs.
Body-state mismatch. If the wrapper computes a GitHub
eventfrom any source other than the parsed verdict (e.g. a sentiment classifier on the prose, a cached default), and the two disagree, the wrapper aborts. There is no "tie-breaker" rule; disagreement is a bug, not a judgment call. After this ADR ships, the only path from body to event is: parse the verdict line; map per the table above; pass to the API. No other classifier runs.Strict + fail-fast. Both abort cases (missing line, mismatch) are loud failures that surface in the
gyrum-review-prconsole output and the persona panel's structured log. Soft-fallback ("treat as COMMENT", "default to APPROVE on missing") is rejected: it re-introduces the convergence-without-evidence shape this ADR exists to remove.Scope. This rule binds the persona panel only — the gyrum-priya / gyrum-marcus / gyrum-lin family of persona-PAT reviewers under the warp#1392 employee model. External collaborator reviews, Dependabot reviews, and human reviewer reviews are untouched by this ADR; the verdict-line directive is appended to persona prompt templates only.
The four cases the regression corpus (Phase 4) MUST cover:
| Body verdict | GitHub state passed | Wrapper behaviour |
|---|---|---|
## VERDICT: APPROVE |
APPROVE |
Posts review (legit approve). |
## VERDICT: CHANGES_REQUESTED |
REQUEST_CHANGES |
Posts review (legit changes-requested). |
## VERDICT: APPROVE |
REQUEST_CHANGES |
Aborts (mismatch); does NOT post. |
| (no verdict line) | (any) | Aborts (malformed); does NOT post. |
Phase 4's devtools#243 fixture covers row 3 directly: body said APPROVE, state was CHANGES_REQUESTED. Under this ADR the wrapper would have aborted before posting; the operator would have re-run the persona instead of admin-overriding the gate.
Consequences
Easier.
- Merge gate becomes deterministic. The state on a posted persona review is, by construction, the verdict the persona's body declared. Operators stop having to read both surfaces and reconcile them.
- The
--adminoverride class shrinks. Every "Marcus said APPROVE but state is CHANGES_REQUESTED → admin merge" cycle is now caught by the wrapper before the bad review lands. - Auto-dismiss (Phase 2) becomes safe to ship. Once the wrapper
guarantees state-matches-body, a stale
CHANGES_REQUESTEDon a prior HEAD can be dismissed without worrying that its body actually said APPROVE all along. - The regression corpus (Phase 4) has a clear acceptance shape: feed body+state pairs in, assert the wrapper's posted-or-aborted decision matches the table above. No prose-grading needed.
Harder.
- Persona prompt templates must be updated to require the directive, and the corpus that pins each persona's prompt version must be re-run when the directive lands. This is one-time and falls under Phase 1.
- LLM prompt drift can produce a missing-verdict-line failure on a PR that "should have" reviewed cleanly. The fail-fast choice means the operator sees a clear error and re-runs, instead of silently shipping a wrong-state review. We accept the operational cost as the price of determinism.
- The wrapper loses the option to "soften" a persona's verdict via a separate classifier (e.g. "Marcus said CHANGES_REQUESTED but the issues are minor; downgrade to COMMENT"). Such softening is out of scope for the persona panel; if the operator wants a softer gate they configure a different review tier, not a body→state translator.
Signed up to operate.
- The persona prompt templates and their regression corpora become load-bearing artefacts. A prompt change that drops the verdict directive is a structural regression and ships with corpus re-run, per the broker / prompt-version rules already in CLAUDE.md.
- Phase 4's fixture must stay current with the devtools#243 case shape; if the persona-as-employee wrapper changes its body→state computation in any future EPIC, the corpus regenerates.
- This ADR scopes only the persona panel. Future review-state-drift failure modes on adjacent surfaces (e.g. CodeRabbit's review state, a Copilot Reviews integration) require their own ADR — this rule does not silently extend.
Alternatives considered
- Sentiment classifier on the prose. Run a second LLM/regex pass over the body to derive an "intended verdict", then either trust it or use it as a tie-breaker against the wrapper's computed state. Lost because it adds a third source of truth on top of the two we are trying to converge — every additional classifier is one more place state can disagree, and the goal of EFF-7 is to reduce sources, not multiply them.
- Trust the GitHub state, ignore the body. Skip body parsing entirely; whatever state the wrapper passes to the API wins, prose-narrative is decorative. Lost because the body is the artefact the LLM actually produces — the state is a derived field — and the operator-readable record (the merged PR's review thread) is the prose. Trusting the derived field over the artefact gives us a deterministic gate but a misleading audit trail when the two disagree, exactly the devtools#243 failure mode pointing the wrong way.
- Soft-fallback on missing verdict line. Default to
COMMENTif no line matches; the persona "didn't decide", treat the review as a no-op. Lost because a missing directive is a prompt failure, not a verdict; downgrading to COMMENT silently turns a structural bug into shipped-but-mute output, hiding exactly the regressions this ADR exists to surface. - Free-form verdict text scanned by regex. Allow
APPROVE/LGTM/ship itanywhere in the body and infer. Lost because prose is non-deterministic by construction; the devtools#243 body literally containedAPPROVEand the wrapper still shippedCHANGES_REQUESTED. The strict-format rule is the only variant that mechanically forecloses the failure mode.
Refs
- EPIC: warp#1559 — EFF-7 gyrum-review-pr persona body-parser as source-of-truth + auto-dismiss-on-fresh-HEAD + no-diff skip.
- This phase (P0 child): warp#1571 — E7.0 ADR.
- Originator: warp#1392 — persona-as-employee with PAT-per-persona; this ADR fixes a structural gap in that wrapper.
- Sister (predecessor): warp#1499
— quick-mode
gyrum-review-pr; Phase 3 of EFF-7 extends quick-mode to the no-diff tier. - Sister bug-class: warp#1330
— auto-tally
Refs warp#NNNparser drift; same prose-vs-action shape, fix was parser narrowing. - Sister gate: warp#1407 — changelog-entry-validate; same structured-block-as-truth pattern.
- Concrete trigger: devtools#243
/ warp#1448 — required
--adminoverride to unstick a body-state mismatch on gyrum-marcus.
Supersedes: none Superseded by: (leave blank until a later ADR reverses this one)