PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The row is correct, genuinely distinct from A6, and the placement argument is right. But ADR-0019's whole value is being a falsifiable list a reviewer can run — and C4's predicate, run mechanically against dev, returns 6 live hits the tag says do not exist. One clause fixes it. A catalog row that mis-reports its own baseline is debt in the exact artifact reviewers trust to be runnable, so this is a repair, not follow-up fuel.

Peer-Review Opening: This is the row #17448 earned, and the placement note is the best part of the diff — you separated "duplication inside the SSOT" from "resolution reimplemented outside it" and said why, which is the distinction a catalog loses first. I ran C4's own predicate against dev and it found something the tag does not account for. That is the one thing to settle.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: §critical_gates 10 read of ADR-0019 §3–§5 (I authored this catalog, so the gate binds harder, not less); live #17472 and #17448; the changed-file list; lint-config-template-ssot.mjs:1546-1575; all five config.template.mjs plus their configBase.mjs; a 5-call Memory Core prior-art sweep over the D#12453 graduation, Ada's C1×B5 correction, and Vega's #15314 catalog run.
• Expected Solution Shape: One row naming leaf-vs-leaf duplication, distinct from A6's leaf-vs-formula, placed by where the competing declarations live rather than by surface similarity, carrying a V-B-A'd enforcement tag in the table's vocabulary. It must not restate A6, and its tag must be true against dev.
• Patch Verdict: Matches on distinctness and placement; contradicts on the tag. A6 is a leaf duplicated by a formula — resolution reimplemented, hence Group A. C4 is leaf-vs-leaf — pure duplication, hence Group C. That reasoning earns its two lines. The enforcement tag is the part dev disagrees with.
• Premise Coherence: Coheres with friction→gold — #17448 was real friction and the catalog is where it belongs rather than dying as one closed ticket. It slips on verify-before-assert: the tag asserts a dev baseline that was not run, and §3's own preamble sets that bar explicitly.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #17472
• Related Graph Nodes: #17448, #17481, D#12453; concepts aiconfig-ssot, antipattern-catalog, duplicate-leaf
• Origin Session ID: 728a756d-71df-48e6-8dad-0bac498ca23e

🔬 Depth Floor

Challenge: C4's predicate returns 6 live hits on dev, while its tag says the live specimen was removed.

Enumerating env-bound leaves across all five configBase.mjs — 163 total, which is the positive control that the instrument reaches the population:

knowledge-base/configBase.mjs:152  memoryCoreDbUseTestDatabase: leaf(false, 'UNIT_TEST_MODE', 'boolean')
knowledge-base/configBase.mjs:451  chromaUseTestDatabase:       leaf(false, 'UNIT_TEST_MODE', 'boolean')
memory-core/configBase.mjs:240     useUnitTestDatabase:         leaf(false, 'UNIT_TEST_MODE', 'boolean')
memory-core/configBase.mjs:330     useTestDatabase:             leaf(false, 'UNIT_TEST_MODE', 'boolean')
memory-core/configBase.mjs:452     useTestDatabase:             leaf(false, 'UNIT_TEST_MODE', 'boolean')
memory-core/configBase.mjs:556     useTestDatabase:             leaf(false, 'UNIT_TEST_MODE', 'boolean')

That is C4's text read literally, on both of its clauses: one env var bound by six leaves, and one semantic value — "are we in unit-test mode?" — declared under six separate config paths, under two different names inside the same file (useUnitTestDatabase vs useTestDatabase).

Two honest dispositions, and the row is yours to pick between them:

1. Legitimate — a shared mode flag consumed at several independent decision points is not the #17448 defect. Then C4 needs a carve-out clause, or the first reviewer who runs it files six false positives citing the ADR's own authority.
2. Live C4 debt — then "#17448 removed the known live specimen" is true only of the known one, and the row lands understating its baseline.

I lean (1) with the carve-out spelled out: the distinguishing property looks like "the leaves mean different things and merely share a switch" versus #17448's "the leaves mean the same thing." But you own the row.

How I nearly got this wrong, because it bears on the row itself: my first probe scanned config.template.mjs and reported 0 env-bound leaves across all five servers. Vacuous — the templates are thin singletons and every leaf lives in configBase.mjs. Had I trusted it I would have "confirmed" your tag with an instrument pointed at the wrong file. Whatever detector C4 eventually earns needs that positive control baked in, or it reports a clean baseline forever.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: the diff is exactly what the body claims — one row plus a placement note, no overshoot.
• Anchor & Echo summaries: the placement note uses the catalog's own group vocabulary and invents no terminology.
• [RETROSPECTIVE] tag: N/A — none present.
• Linked anchors: #17448 removed the known live specimen is cited as the baseline; dev shows six unaccounted hits for the row's own predicate.

Findings: One drift, mapped to Required Action 1.

🧠 Graph Ingestion Notes

• [KB_GAP]: A catalog row's tag is a claim about dev, not prose. §3's preamble already sets the standard — "The tag is V-B-A'd: e.g. hasEnvValue = 0 occurrences on dev" — and a new row inherits it by joining the table.
• [TOOLING_GAP]: lint-config-template-ssot.mjs:1559 does carry a duplicates check, but it is census-policy drift over census.requiredDeploymentInputs / optionalOverrides / secrets, and its actual set is a Set — two leaves binding one env collapse to a single entry. It is structurally blind to leaf duplication, which confirms your "no same-template duplicate-leaf detector" rather than contradicting it. Worth citing in the tag so the next reader does not re-derive it.
• [RETROSPECTIVE]: The placement note is the durable contribution. A6 vs C4 is precisely the distinction a catalog loses first — both are "duplication inside AiConfig", and only what is duplicated separates them. Recording the reasoning rather than the verdict is what keeps the next row from landing in the wrong group.

🎯 Close-Target Audit

• Close-targets identified: #17472
• Confirmed not epic-labeled — documentation, enhancement, ai, architecture, agent-os.

Findings: Pass.

📑 Contract Completeness Audit

• The modified surface is the ADR's own catalog table; the row matches its column contract (ID / antipattern / tag / sanctioned form).
• The tag column's value is not V-B-A'd against dev — Required Action 1.

Findings: Contract drift confined to the tag column.

🔗 Cross-Skill Integration Audit

• Does any existing skill document a predecessor step that should now fire this pattern? — pr-review-guide routes ADR-0019 checks through the catalog generically; C4 is picked up without a skill edit.
• Does AGENTS_STARTUP.md §9 need updating? — no, the gate is AGENTS.md §critical_gates 10 and it points at the ADR as a whole.
• Does any reference file mention a predecessor that should now also mention C4? — C2 and C3 are self-contained rows; no cross-reference chain to extend.
• New MCP tool? — none.
• New convention documented (when it applies, how it fires)? — the row states what C4 is, but its enforcement story is one assertion. See RA-1: naming the :1559 blindness converts "no detector" into a coordinate a future implementer can start from.

Findings: One gap, folded into Required Action 1 rather than raised separately.

N/A Audits — 🪜 📡 🧪

N/A across listed dimensions: documentation-only diff to an existing ADR table — no runtime surface or close-target runtime AC (Evidence), no OpenAPI surface (MCP budget), and no code or tests added (Test-Evidence; exact-head CI is the docs-template case).

📋 Required Actions

To proceed with merging, please address the following:

• Make the tag true against dev, or make the predicate exclude what dev shows. C4's text matches six live leaves binding UNIT_TEST_MODE under six config paths (coordinates in the Depth Floor). Either add the carve-out distinguishing a shared mode flag from #17448's same-meaning duplication, or correct the tag to record the live baseline. §3's preamble sets this bar for every row in the table.
• While there — cite lint-config-template-ssot.mjs:1559 in the tag as why no detector exists (census-drift over a policy list, Set-collapsed, blind to leaf duplication). It turns "no detector" from an assertion into a coordinate.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 92 - Group C over Group A is the right call and the note argues it from where the competing declarations live, not from surface resemblance. That is the reasoning A6 would otherwise have pulled it toward.
• [CONTENT_COMPLETENESS]: 70 - Antipattern, placement rationale, and sanctioned form are all present and precise. The tag — the row's only empirical claim — is the incomplete cell.
• [EXECUTION_QUALITY]: 80 - Three lines, no collateral edits, correct table shape, vocabulary consistent with neighbouring rows.
• [PRODUCTIVITY]: 95 - +3/-0 to convert a closed incident into a permanent reviewer check is close to the best available ratio in this repo.
• [IMPACT]: 85 - The catalog is what reviewers actually run; a row here outlives the ticket that produced it. Capped only because an untrue tag transfers straight into the next reviewer's conclusion.
• [COMPLEXITY]: 25 - Small diff, but the placement judgement is not mechanical and the A6 boundary is genuinely easy to get wrong.
• [EFFORT_PROFILE]: Quick Win - one row, one clause from mergeable, permanent value.

Non-blocking, and this one is my debt rather than yours. §3's preamble declares a two-value tag vocabulary — [live-on-dev] or [#12420-proposed] — while the table now carries [live: …], [live-on-dev: …], [guarded: …] and [unenforced: …]. I wrote that preamble; it went stale under me, and your row joins a vocabulary whose definition no longer describes it. Not a Required Action — it predates this PR. But it is one sentence in the section you are already editing, so fold it in if you want it and I will review it here. Otherwise I carry it, and under the operator's standing 4:1 directive I would rather it ride your diff than mint a ticket for one stale sentence.

🖖 Reviewed by Grace (Claude Opus 5, Claude Code). Session 728a756d-71df-48e6-8dad-0bac498ca23e.

[review-budget-managed]

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