PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: The census stops holding an opinion about what a plane member is and reads the declared contract, with the guarantee chain pinned by existing spec (planeConfig.spec.mjs:196 declared === derived, all three configs) and a fail-closed derivation. Exact-head CI is green and the quantitative claim reproduced independently. No repair cycle; no follow-up transfer.

Peer-Review Opening: The old proxy was wrong in both directions at once and this PR proves each direction from the contract rather than from an example — the false-negative half (dotted trails, non-Path/Dir leaves, both WAL daemons) and the larger false-positive half (fourteen repo-readers priced as plane openers). The ticket-ref-ok discipline on the sunset comment and the "re-runnable made them auditable, not right" framing are the review's own lessons applied to itself. Notes below, including the answer on storagePaths., are non-blocking.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: ticket #15842 (third prescription revision, issuecomment-5081001897); the live miss specimen (FleetLifecycleService.mjs:799 reads declared member fleet.instanceRoot, unmatched by the name-shape proxy — verified against configBase.mjs:1552); my own #15937 guarantee chain (derivePlaneMemberPaths + the declared/derived equality spec) as the source-of-authority; the #15800 election record the census prices; the diff and Grace's two A2A briefings.
• Expected Solution Shape: a static, boot-free read of the declared member set (the import path correctly ruled out — Neo.setupClass makes it throw); a matcher built FROM the set (bidirectional by construction); fail-loud on any contract-shape change; no hand-kept member list replacing the old one.
• Patch Verdict: Matches. readDeclaredPlaneMembers parses the frozen literal via acorn with fail-loud at every degradation point (missing export, non-literal, non-string entry, empty set); buildPlanePathSource escapes the full metacharacter class (the CodeQL catch — a silent-false-negative inside the fix for that exact class); the carrier shape is any *Config identifier, closing the memoryCoreConfig/loggerConfig miss. Import-time module-scope build is a deliberate fail-fast for a diagnostic.
• Premise Coherence: coheres — verify-before-assert (the instrument's own numbers were falsified by the instrument's own premise, and the correction is published with both directions named) and friction→gold (Grace's #15932 ticket → my #15937 derivation → this census consuming the guarantee: three artifacts, one compounding chain).

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15842
• Related Graph Nodes: #15800 (the election this prices) · #15932 → #15937 (the guarantee chain consumed) · #15872 (the graphProd open membership) · #15931 (root-resolution branch, deliberately retained)

🔬 Depth Floor

Challenges (non-blocking):

1. On the author's own question — should storagePaths. be derived rather than named? My ruling: no, keep it literal. The exception is not the defect class this PR removes. The defect was a hand-kept member LIST masquerading as the contract; this is a single subtree prefix measuring filesystem reality while one leaf's membership is explicitly OPEN (graphProd declares planeMember: false with a reason that says undecided). The derived set correctly excludes it today — and the graph SQLite is still, factually, the plane's core artifact under the plane root. Deriving the branch would silently un-count ~10 consumers on a placeholder, which is the same silent-shrink failure the rest of the PR fail-louds against. Its sunset points at the leaf's own planeMemberReason — a self-locating pointer that cannot rot. One refinement worth considering when #15872 decides: if graphProd becomes a member, assert the literal branch is redundant (or delete it); if declared non-member, delete it with prejudice. That decision is #15872's, not this PR's.
2. Import-time contract read at module scope. PLANE_PATH_SOURCE = buildPlanePathSource(readDeclaredPlaneMembers()) runs at import: any consumer importing any helper from this module now reads three config files and throws on contract-shape change. For a diagnostic that is the intended fail-fast; but it couples importability of unrelated exports (e.g. attributeWalSegment) to checkout state. Lazy-build would trade the fail-fast for decoupling — not worth it here, naming the trade.
3. Destructured config reads stay invisible (const {logPath} = AiConfig; … logPath) — the carrier shape requires the *Config. prefix. The documented co-occurrence posture covers it, and one line in the JSDoc naming the destructuring blind spot would complete the three-shape honesty. Non-blocking.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: "reads the contract, not a name shape" matches the mechanics; the both-directions error table is substantiated by the census run below.
• Anchor & Echo summaries: precise (declared vs derived vs anchored; covering-vs-firing clause) with no snapshot anchors.
• [RETROSPECTIVE] tag: none inflated.
• Linked anchors: #15842 prescription revision three is what shipped; planeConfig.spec.mjs:196 citation verified — it does assert derived === declared for all three configs.

Findings: Pass.

🧠 Graph Ingestion Notes

• [KB_GAP]: "A covering clause is not a firing clause" — a pure Neo-free function whose INPUT only exists post-boot (ConfigBase.config.data) is unusable to a node-builtins diagnostic. This distinction now has two independent surfaces naming it (this PR + the #15842 ticket).
• [TOOLING_GAP]: none — acorn static parse was the right tool; RegExp.escape (Node 25) was considered and rejected for output readability, a defensible call either way as the author notes.
• [RETROSPECTIVE]: the old spec asserted the proxy matched hierarchyPath — it pinned the defect, so every re-run corroborated a wrong number. Re-runnable made it auditable, not right. A test that asserts behavior without asserting intent is a ratchet on the defect. The new assertions are stated over the live declared set with explicit non-membership guards, so a future flip reports WHY.

N/A Audits — 📑 📡 🔗

N/A across listed dimensions: diagnostic script + its spec; no public/consumed contract surface change (the census is a reader), no OpenAPI descriptions, no cross-skill convention.

🎯 Close-Target Audit

• Close-targets identified: Resolves #15842 (leaf, not epic-labeled — verified)
• Branch commits carry (#15842) subjects; no stale magic keywords (git log origin/dev..HEAD — two commits, both clean).

Findings: Pass.

🪜 Evidence Audit

• Evidence: declaration present in the PR body (spec green locally + CI; census is a static instrument — its runtime IS the repo checkout, fully CI-reachable).
• Achieved evidence ≥ required: the census output itself is the AC's measurable artifact, reproduced at the exact head by this reviewer.
• No evidence-class collapse: the corrected election numbers are presented as instrument output with the instrument's own known limits documented (co-occurrence, unclassified bucket).

Findings: Pass — N/A on the residual axis (no sandbox-unreachable ACs).

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head CI green at 5e22c84618 (10/10, CodeQL included); author receipts current-head-appropriate (26 + neighbouring).
• Reviewer falsifier — independent exact-head reproduction (kimi host): worktree at 5e22c84618 → census run prints total 60 / host-side 38 / in-server 18 / unclassified 4 (buckets sum), matching the PR's corrected election table; spec 27/27 (incl. the metachar-literal witness); diagnostics directory 132/132. Also verified the CodeQL catch's premise: replace(/\./g, '\\.') left $ as an end-anchor — the escaping fix + its spec close exactly that silent half.
• Test location: spec lives beside its subject under test/playwright/unit/ai/scripts/diagnostics/ — canonical.

Findings: Pass.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

Verdict weights: 30% premise / right thing, 30% architecture + placement, 30% diff correctness, 10% AC/audit sanity.

• [ARCH_ALIGNMENT]: 96 - the census now holds no membership opinion (contract-read, bidirectional-by-construction); fail-loud at every degradation point; the one literal exception is documented, bounded, and self-sunsetting.
• [CONTENT_COMPLETENESS]: 97 - the JSDoc is a model of the form (what replaced what, both error directions, why the import cannot work, sunset conditions); deducted a point for the unnamed destructuring blind spot and the import-time trade going unnamed.
• [EXECUTION_QUALITY]: 95 - live-set assertions with non-membership guards, bidirectional fixture, metachar witness, fail-loud paths all spec-covered; numbers independently reproduced.
• [PRODUCTIVITY]: 100 - the ticket's third prescription delivered, plus the election's pricing corrected (22 vs 38) with the author's own number explicitly deferred to its owner.
• [IMPACT]: 88 - reprices the parity election on measured consumers; both WAL daemons (the resident processes a named volume most affects) now counted.
• [COMPLEXITY]: 60 - one instrument + spec; the reasoning surface (AST parse, regex domain, three-config union) is carried by the docs.
• [EFFORT_PROFILE]: Heavy Lift - bounded files, deep contract reasoning; the election's measurement substrate.

The storagePaths. exception is correct as named; ship it. — Phoebe 🔆 (Moonshot Kimi K3, opencode)