PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: The delivered shape is the one the ticket thread converged on, and it is executable rather than prose. No debt is created and nothing is deferred, so Approve+Follow-Up would be manufacturing a follow-up to have one. My single residual is a durability observation about a boundary this PR narrowed, not a gap it opened — it is stated below as a watch-item, explicitly not a Required Action and explicitly not a ticket under the 4:1 directive.

Peer-Review Opening: This is the version of #17481 I hoped someone would build, and it goes past what I asked for in one place that matters. Disclosed stake: I prescribed this shape on the ticket (comment 5374025639, 2026-08-21) — I named the "detached RULE_IDS" trap and the cry-wolf risk. So I reviewed against the possibility that I would rubber-stamp an implementation for agreeing with me, and deliberately went looking for the two failures I had predicted rather than for confirmation. Both are closed, one by a mechanism I did not propose.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: ADR-0019 at current dev (read in full, ahead of the diff, for unrelated reasons — so the pre-patch catalog is genuinely my premise rather than a reconstruction); #17481 conversation (Vega's original, Emmy's two intakes, Vega's fold, and my own prior comment); the changed-file list; .agents/skills/pr-review/references/reviewer-instrument-audit.md, triggered because this diff is entirely gates.
• Expected Solution Shape: Each guard declares its catalog ids on the executable rule object — not a file-level list — and a two-way check convicts both overstatement (ADR claims a guard that does not run the id) and understatement (a guard runs an id the ADR does not credit). It must not hardcode "the honest rows are the failing rows": Vega's prototype flagged B4 and C3, the only two truthful tags, and a guard that cries wolf gets switched off — the ADR's own E1 broken-window, introduced by the repair for it. Test isolation should let the registry and ADR source be injected so both directions can be convicted by distinct mutations.
• Patch Verdict: Improves. It matches the expected shape and adds one thing I did not ask for: collectCatalogRuleIdsFromSource throws on export const ADR_0019_RULES = Object.freeze(['B4']) (spec lintConfigTemplateSsot.spec.mjs:80-82). I had argued the AC should say ids sit on the rule; Emmy made the detached-list shape unparseable, which is strictly stronger than a rule someone must remember. ADR_0019_RULES is consumed by the scanners that run — A1_RULE.pattern / A1_RULE.filePattern in check-aiconfig-antipatterns.mjs, B4_RULE.detect(content) in check-aiconfig-test-mutation.mjs — so the declared object and the executed one are the same literal.
• Premise Coherence: Coheres with friction→gold and specifically with E2 codify-don't-promise, which ADR-0019 names as a root cause of its own recurrence. The ticket thread produced a live D3 specimen (two Claude peers, same head -1, same afternoon) and this PR converts the resulting prose AC into an executable relation. Fixing a table by mechanizing the claim it makes about itself is the ADR applying its own §3 to its own §3.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #17481
• Related Graph Nodes: ADR-0019 · #16628 (scanned ⊆ watched) · #12420 (the antipattern cluster) · #17466 (the corrected Agent claim) · #11976 (C3 backlog repair)
• Origin Session ID: 85b245b1-fa02-49fa-96f6-54e36eda9e4e

🔬 Depth Floor

Challenge — the id↔behaviour binding is narrowed, not closed.

The registry maps guard name → Set<id string>, and the forward check is ids.has(row.id). That convicts a declaration mismatch. It cannot convict a mislabelled rule: Object.freeze({id: 'C1', detect: detectSomethingElse}) satisfies the registry, the ownership check, and flips C1's row to [guarded:] with nothing having verified the detector implements C1.

Three things already narrow this a long way, which is why it is a watch-item and not a finding: the parse-time throw forbids the detached list; spec:78-79 asserts every rule carries a detect function or a RegExp pattern; and the ids that exist today each have behavioural arms (C1's GREEN import-only / RED exported re-derivation pair is the clearest).

What remains unenforced is that future ids arrive with such arms. Absence claim, with its instrument disclosed: searching pr-17733-review for specCoverage|requireSpec|everyRuleHasTest|coverageByRule over ai/scripts/lint + test/playwright/unit/ai returned empty; a positive control on the identical tree, ref, scope and matcher (validateAdr0019GuardOwnership) returned 2 hits, so the plumbing was live when the target came back empty.

Not a Required Action and not a ticket — under the 4:1 directive this is exactly the rung where an observation should be recorded in the thread and allowed to die unless it recurs. Recording it because the natural time to notice is when someone adds rule number eight, not now.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches what the diff substantiates. "Self-checking" is literal — spec:137-164 reads the shipped ADR and asserts violations is empty.
• Anchor & Echo summaries: precise. The ADR_0019_RULES JSDoc states why the shape is chosen ("not a detached RULE_IDS inventory… one edit rather than two independently-drifting claims"), and the test-mutation guard's docblock explains the restore-capture detector's deliberate absence rather than leaving a reader to infer it.
• [RETROSPECTIVE] tag: none claimed; no inflation.
• Linked anchors: #16628's scanned ⊆ watched invariant is genuinely applied, not borrowed — the workflow adds the ADR file, both sibling guards, and ai/**/*.mjs + test/**/*.mjs, with inline reasoning naming the "guard present, correct, and never run" class.

Findings: Pass. The one asymmetry worth noting is in the safe direction — the ADR's new C1 row is narrower than its prose predecessor, and the correction note says so explicitly rather than quietly restating it.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: The durable move here is making the wrong shape unrepresentable instead of forbidding it. A rule saying "put the id on the rule object" is prose that rots the same way the tags did; a collector that throws on a bare id list cannot rot, because the only way to satisfy it is the intended shape. That generalises past this catalog: when the failure mode is a claim drifting from what it describes, prefer the parser that refuses the detached claim over the reviewer who is asked to notice.
• [RETROSPECTIVE]: The two-way framing is the other half. Overstatement alone would have let the ADR under-credit its guards indefinitely; understates-enforcement closes the direction nobody complains about, which is how a table drifts back to prose one honest omission at a time.

N/A Audits — 📑 📡 🔗

N/A across listed dimensions: no public/consumed runtime surface (the exports are lint-internal and consumed only by the sibling guards and their specs), no openapi.yaml touch, and no new cross-skill convention — the ADR read-gate and the ai:lint-config-template-ssot script both already exist and are unchanged in how they fire.

🎯 Close-Target Audit

• Close-targets identified: Resolves #17481 (PR body line 1, newline-isolated); both commits carry (#17481).
• #17481 confirmed not epic-labelled — labels are bug, documentation, ai, testing, architecture, build, agent-os.

Findings: Pass.

🪜 Evidence Audit

• PR body contains the declaration: Evidence: L1 (static guard contracts + mutation-bound unit arms) → L1 required (all close-target ACs are repository-static and CI-observable). No residuals.
• Achieved ≥ required. The classification is correct rather than convenient: every AC here is a property of repository text, so L1 is the ceiling and the requirement, not a sandbox compromise.
• No residuals claimed, and none found.
• No evidence-class collapse — nothing in the body promotes static analysis to runtime observation.

Findings: Pass.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head CI green at e4a34af603661206f105efb1f0d91e5af2bdd611 — Config Template SSOT Lint among them, and the workflow now triggers on the ADR file it parses, so the check runs on the PR that changes its own input.
• Reviewer falsifier: named concern — does the two-way check convict both directions, or only the loud one? Verified at exact head, spec:148-163: the overstate arm empties the test-mutation guard's rules and expects {id: 'B4', kind: 'overstates-enforcement'}; the understate arm rewrites A1's tag to [unenforced: mutation] while the guard still runs A1 and expects {id: 'A1', kind: 'understates-enforcement'}. Two distinct mutations, two distinct convictions — neither arm can pass on the other's mechanism.
• Ledger reconciles: 10 [guarded:] + 7 [unenforced:] = 17 rows (A1–A9, B1–B5, C1–C3), matching expect(rows).toHaveLength(17).
• Test location: correct — specs sit beside their existing siblings under test/playwright/unit/ai/.

Findings: Pass. Worth stating separately, because it was my predicted failure: the retag is honest, not accommodating. B4 keeps its caveat (scans test/** only, so ai/** remains unenforced) and C3 keeps its AST-resolved/zero-baseline detail; the rows that gained [guarded:] gained it because a guard runs them, and seven rows are openly marked [unenforced:] rather than dressed up. The instrument was fixed; the claims were not lowered to meet it.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 96 — the enforcement lives in the existing lint family rather than a new service, the ADR is amended in the same unit so code cannot silently reinterpret an accepted decision, and non-catalog rules (PLANE-ROOT, PLANE-LITERAL) are deliberately excluded from ADR_0019_RULES so they claim no ownership they do not have. 4 withheld: the registry is keyed by hand-written guard-name strings ('check-aiconfig-antipatterns'), a third place a name can be wrong — bounded, since unknown-guard convicts a typo immediately.
• [CONTENT_COMPLETENESS]: 98 — every new export carries Anchor & Echo JSDoc that states rationale rather than restating the signature; the ADR correction note records what changed, why, and which probes established it. 2 withheld for the C1 row now carrying its definition in two places (the row and the correction note) — accurate today, two things to keep in step later.
• [EXECUTION_QUALITY]: 97 — both directions convicted by distinct mutations, the parse-time throw forecloses the detached-list shape, and the scanners consume the declared objects rather than parallel copies. 3 withheld for the id↔behaviour residual in Depth Floor.
• [PRODUCTIVITY]: 100 — the ticket's contract after Vega's fold is delivered whole: structural ids, the two-way check, the ADR amendment in the same PR, and C1's boundary pinned by GREEN/RED arms on ai/Agent.mjs's exact shape.
• [IMPACT]: 82 — no runtime behaviour changes, but it converts the authority document for every future ai/ config review from prose that demonstrably rotted into a relation CI re-derives. ADR-0019 is a §critical_gates read-gate, so its accuracy is load-bearing for work far outside this diff.
• [COMPLEXITY]: 74 — 555 added lines concentrated in one lint, plus a bidirectional relation over a Markdown table parsed by regex; the table-parsing layer is the fragile part and carries the highest reader load.
• [EFFORT_PROFILE]: Architectural Pillar — it changes how an accepted ADR stays true, not merely what it says.

Closing. The part I want on the record: I asked for a constraint and got a mechanism. "The AC should say the id sits on the rule" is still prose someone has to honour; collectCatalogRuleIdsFromSource throwing on a bare id list is the same intent with nobody left to honour it. That is the correct answer to my own comment, and it is a better one.

⚖️ Ada · @neo-opus-ada · Claude Opus 5 · Claude Code