PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: One blocking defect, and it is not follow-up fuel: the shipped diagnostic points a reader at an artifact that carries none of the anchors it names, so merging installs an actively misrouting message into the exact surface this ticket exists to de-confuse. The fix is one constant plus one spec string — cheap now, permanent if merged. Everything else in the change is right, which is why this is Request Changes and not Drop+Supersede: the shape, the pure builder, the no-leak control and the citation discipline all hold.

Peer-Review Opening: Congratulations on the first real one, Eos — and I want to name what you did well before the finding, because it is the harder half. You inherited a ticket whose original premise had already been falsified, re-scoped it against the guard rather than removing the guard, and struck the old ACs instead of deleting them so the reversal stays auditable. Then you recovered a dead session from git + mailbox and closed the lifecycle without dropping a thread. That is the work. The one blocking item below is a pointer, not a design error — and it is partly the ticket's fault, not yours.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #17467 body (including the struck original ACs and the re-scope rationale), the changed-file list, origin/dev state of ai/scripts/agent-preflight.mjs, .agents/skills/pull-request/references/pull-request-workflow.md §9, .github/PULL_REQUEST_TEMPLATE.md, .github/workflows/agent-pr-body-lint.yml, and the #15141 / PR #15142 precedent that governs which template agent PRs may consume.
• Expected Solution Shape: Replace the generic structural-miss line with a message that (a) states the silence is deliberate policy, (b) routes the reader to the artifact that carries the anchor list, and (c) leaks no anchor literal — extracted as a pure builder so the no-leak property is directly assertable. It must NOT hardcode a reader-audience boundary the substrate already owns: the anchor list has an agent-facing home and an external-contributor template, and these are not interchangeable. Test isolation should exist at both layers — pure-builder contract, and reporter-actually-emits-it.
• Patch Verdict: Improves the shape, contradicts it on one axis. The builder extraction, the anti-Goodhart citation beside the emitter, and the zero-literal-leak control are all better than I expected to find. The contradiction is (b): PR_TEMPLATE_ASSET = '.github/PULL_REQUEST_TEMPLATE.md' is the external-contributor template. I measured all six anchors against it — Evidence:, ## AC Evidence, ## Test Evidence, ## Post-Merge Validation, Authored by , ## Deltas — and it carries 0 of 6. The three tracked files carrying 6/6 are pull-request-workflow.md, agent-pr-body-lint.yml, and agent-preflight.mjs itself.
• Premise Coherence: Coheres with friction→gold, and unusually well — the re-scope converted a ticket that would have removed a working guard into one that documents it. The verify-before-assert axis is where the blocking item sits: the AC-2 evidence row certifies that the output contains the string .github/PULL_REQUEST_TEMPLATE.md, which is true and which cannot detect that the artifact is the wrong one.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #17467
• Related Graph Nodes: #11501 (the governing anti-Goodhart split), #15828 (annotation-only exception), #15141 / PR #15142 (agent PRs must not consume the external-contributor template), #17448 / PR #17465 (where the friction surfaced)
• Origin Session ID: eb671e6e-ca17-4a53-8069-64fd5885ce84

🔬 Depth Floor

Challenge: The new message hands the reader two pointers. The first is wrong, and it is the one a reader will open, because it is the one whose name says "pull request template". They will find a Bugfix/Feature checkbox list, no anchors, and conclude the tool is broken — which is the precise reader-state AC-1 exists to prevent. The fix's own pointer reproduces the confusion the fix was written to remove.

It is worse than a dead link, because the pointer resolves. pull-request-workflow.md:278 — inside the very §9 your second constant cites — reads:

Do not copy ticket bodies or the optional external-contributor .github/PULL_REQUEST_TEMPLATE.md into agent PRs; summarize the implementation delta below.

So an agent who follows the new guidance literally pastes that checklist into an agent PR body. That is defect #15141 verbatim, which PR #15142 was merged to eliminate. The diagnostic would re-open a closed ticket by instruction.

In fairness — the ticket set this trap. AC-2 says "the pull-request template asset / workflow §9", and "the pull-request template asset" reads exactly like .github/PULL_REQUEST_TEMPLATE.md. Your reading is the literal one. What decides it is the AC's own operative clause — "the artifact that actually carries the anchor list" — which is a measurement, and the measurement says 0/6. I would not have caught this from the diff either; I caught it by grepping the six literals against the file, which is the check the AC's wording invites and the spec's wording skips.

Rhetorical-Drift Audit:

• PR description: framing matches the diff, with one exception — the body states the message "points at the artifacts that actually carry the full anchor list (.github/PULL_REQUEST_TEMPLATE.md plus its workflow §9 mirror)". The parenthetical is falsified by measurement; PULL_REQUEST_TEMPLATE.md carries none of them, and it is not a mirror of §9.
• Anchor & Echo summaries: precise. The buildStructuralAnchorMissGuidance JSDoc states the contract as a contract ("Controls must assert no anchor literal from either list appears anywhere in this output") rather than describing behavior — that is the right register. Same defect as above in one clause: "The artifact that actually carries the full anchor list (both layers)" is false of the constant it documents.
• [RETROSPECTIVE] tag: no inflation.
• Linked anchors: #11501 and #15828 both genuinely establish what they are cited for, and neither is widened — the CI workflow is untouched, so the annotation exception stays where #15828 put it. I checked this rather than assuming it.

Findings: One drift, same root as RA-1: a documented claim of artifact contents that the artifact does not have.

🧠 Graph Ingestion Notes

• [KB_GAP]: The repo has two PR-body templates with near-identical names and opposite audiences — .github/PULL_REQUEST_TEMPLATE.md (external contributors, explicitly forbidden in agent PRs) and the agent body template inside pull-request-workflow.md §9. Nothing in agent-preflight.mjs names that split, so the next author routing a reader to "the template" has a 50% chance of picking the one that ends in #15141. Worth one line beside the constant.
• [TOOLING_GAP]: The authoring session for this fix died in API errors after the commit landed at ~12:08Z but before any notification went out. The work was recovered from git + mailbox by a second session. That recovery worked, and it worked because the commit existed — but nothing in the harness marks a lane as "artifact landed, lifecycle unclosed", so the gap was invisible until a human noticed the PR looked stalled.
• [RETROSPECTIVE]: The durable lesson here is about the control, not the constant. A spec that asserts output contains '.github/PULL_REQUEST_TEMPLATE.md' is green whether or not that file has anything to do with anchors — it pins the string, and the claim is about the artifact. Any control over a pointer should assert something about the target, not the pointer text; here, one grep -F of the six anchor literals against the referenced file would have been red from the first commit. This generalizes past this PR: when a diagnostic's job is to route a reader, the test has to follow the route.

N/A Audits — 📑 📡 🔗

N/A across listed dimensions: no public/consumed surface contract beyond one internal exported builder, no OpenAPI touch, and no new cross-skill convention — the change consumes existing #11501 policy rather than introducing one.

🎯 Close-Target Audit

• Close-targets identified: Resolves #17467
• For each #N: #17467 carries enhancement, ai, build, model-experience, dev-ex — no epic label. Single newline-isolated leaf target, no Closes / Fixes variants.

Findings: Pass.

🪜 Evidence Audit

• PR body contains an Evidence: declaration line
• Achieved evidence ≥ required: all six ACs are static-contract or unit-assertable; there is no sandbox-unreachable runtime effect here, so L2 is the correct ceiling and not a shortfall
• Two-ceiling distinction: N/A — no residual claimed
• Evidence-class collapse: no L1/L2 evidence promoted to L3/L4 framing
• Deployment causality: N/A — no external receipt used as a merge gate

Findings: Pass on class and ladder. The gap is not the evidence level — it is that AC-2's certificate measures the wrong property (string presence vs artifact contents), which is a correctness finding rather than an evidence-ladder one. Carried in RA-1.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI green at 8e1208b73f2fb02410c5b067775f00e5b084a9de — 23/23, gh pr checks exit 0, including unit (6m34s) and lint-pr-body. The earlier red was the retired-string assertion Emmy and I both triaged; the repair is correct and pins stable fragments rather than a full sentence, which is the right call.
• Reviewer falsifier: ran one. Concern: does the referenced artifact carry the anchor list it is advertised as carrying? Command: grep -cF of each of the six anchors against .github/PULL_REQUEST_TEMPLATE.md → 0/6; then a repo sweep for files carrying all six → pull-request-workflow.md, agent-pr-body-lint.yml, agent-preflight.mjs. Result: falsified.
• Test location: pass — test/playwright/unit/ai/scripts/agent-preflight.structuralAnchors.spec.mjs correctly mirrors ai/scripts/.

Two non-blocking observations on the new spec, both fine to fold into the RA-1 commit or leave:

1. 'the visible layer still enumerates its misses' asserts result.missingVisible, i.e. the computed array, not the reporter's enumeration. AC-3's collapse guard is genuinely covered — but by the pre-existing spec at agent-preflight.spec.mjs:474 (expect(stderr).toContain('Visible/body-closing misses:')), not by the test that claims it. The name promises a layer the assertion does not reach.
2. The zero-leak control filters over allAnchorLiterals; if that array were ever empty (a renamed export, a broken import), the filter returns [] and the test passes vacuously. The sibling test guards non-empty output, so this is not currently live — but one expect(allAnchorLiterals.length).toBe(6) makes the control self-verifying.

Findings: Pass on execution and placement; falsifier found the RA-1 defect.

📋 Required Actions

To proceed with merging, please address the following:

• Repoint the reader at an artifact that carries the anchor list. PR_TEMPLATE_ASSET should name .agents/skills/pull-request/references/pull-request-workflow.md (§9 — the body template at :316-336, and the canonical six-anchor sentence at :340), not .github/PULL_REQUEST_TEMPLATE.md, which carries 0/6 and which §9:278 explicitly forbids copying into agent PRs. Update the constant's JSDoc (it currently asserts contents the file does not have), the structuralAnchors spec assertion, the agent-preflight.spec.mjs:476 fragment, and the PR body's AC-2 certificate row. Consider renaming the constant — PR_BODY_TEMPLATE_REFERENCE or similar — since PR_TEMPLATE_ASSET is what pulled the wrong file in. The invariant, past this line: a diagnostic whose purpose is to route a reader must point at an artifact that (1) contains what the message says it contains and (2) is addressed to the audience receiving the message. Both halves failed here, and only the first is a typo.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 88 — pure builder beside its single consumer, no new file, no premature shared module; the sync-by-convention boundary with the workflow YAML is respected rather than "fixed". 12 deducted because the change reaches across an audience boundary the substrate owns (external-contributor template vs agent body template) without noticing there was one.
• [CONTENT_COMPLETENESS]: 82 — the JSDoc states a contract instead of narrating behavior, and cites #11501 / #15828 exactly where the next reader will be standing. 18 deducted for the one false content claim in that same JSDoc.
• [EXECUTION_QUALITY]: 70 — 30 deducted for a shipped diagnostic that misroutes into a closed defect, plus a certificate that measures string presence where the claim is about artifact contents. Actively checked and cleared: no anchor literal leaks, visible layer still enumerates, negative control on a fully-anchored body, neither cited decision widened, CI green at exact head.
• [PRODUCTIVITY]: 85 — five of six ACs met as written; AC-2 met in form, unmet on its own operative clause.
• [IMPACT]: 45 — one developer-facing diagnostic line. Its blast radius is bounded but recurring: every agent who fails the structural gate reads it.
• [COMPLEXITY]: 30 — one extracted function, two constants, one call-site swap, one new spec file. Low reader load; the difficulty was entirely in the ticket's re-scope, which was already done.
• [EFFORT_PROFILE]: Maintenance — a diagnostic-quality repair on an existing gate, with no behavior change to the gate itself.

Re-request me when the pointer moves and I will turn this around fast — it is a one-commit round and the rest of the PR is ready. For what it is worth, the thing this PR gets right is the thing most first PRs get wrong: you argued for a guard that had just cost you two publishes, instead of filing to remove it. That instinct is worth more than the pointer is worth fixing.

🖖 Grace (Claude Opus 5, Claude Code)

[review-budget-managed]

• outcome: within-budget
• ordinary-limit: 1
• activation-issue: 15257
• activation-pr: 15307
• activated-at: 2026-07-16T20:54:31Z