Decisions

ADR-151: Persona verdict-line directive + body-parser semantics for gyrum-review-pr

`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…

#151

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:

  1. 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.
  2. CHANGES_REQUESTED persists across new commits. GitHub keeps the prior review's state until explicitly dismissed, so re-running gyrum-review-pr posts new reviews while the stale CHANGES_REQUESTED from a prior HEAD hangs around as outstanding.
  3. 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:

  1. 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.

  2. 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: APPROVE but…") are ignored; the last-line rule keeps the directive at a structural position humans and the parser agree on.

  3. 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.

  4. Body-state mismatch. If the wrapper computes a GitHub event from 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.

  5. Strict + fail-fast. Both abort cases (missing line, mismatch) are loud failures that surface in the gyrum-review-pr console 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.

  6. 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 --admin override 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_REQUESTED on 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 COMMENT if 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 it anywhere in the body and infer. Lost because prose is non-deterministic by construction; the devtools#243 body literally contained APPROVE and the wrapper still shipped CHANGES_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#NNN parser 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 --admin override to unstick a body-state mismatch on gyrum-marcus.

Supersedes: none Superseded by: (leave blank until a later ADR reverses this one)