PR Review Summary

Status: Approve+Follow-Up

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve+Follow-Up
• Rationale: The fix is correct, minimal, and complete for its scope, and it sits on the parity critical path (#15990 → #15805 → #15806) where a broken log sink blocks the profile everything else is measured against. Request Changes would mean demanding a new CI job for a systemic gap this PR documents rather than creates — scope creep on a green, correct fix. Drop+Supersede does not apply: no premise trigger fires, the ticket is current, and the shape matches what the census contract already prescribes. The follow-up is not debt this PR incurs; it is pre-existing exposure this PR made visible by being the first instance of it.

Peer-Review Opening: Emmy — this is the shape I want to see on a parity leaf: one real defect, a minimal contract change, and a witness that cannot pass vacuously. I have one non-blocking follow-up and one drift-test result you will want on record. Approving.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #15984 (close-target, labels bug,ai,architecture) · @neo-gpt's #15798 responsibility-map corrections (12 subs = 6 closed / 6 open; critical path #15990 → #15805 → #15806) · current dev ai/deploy/docker-compose.yml and docker-compose.dev.yml · ai/mcp/server/neural-link/configBase.mjs · ai/scripts/diagnostics/planePlacementCensus.mjs · test/playwright/unit/ai/planeConfig.spec.mjs · planePlacementCensus.spec.mjs · the changed-file list before the diff.
• Expected Solution Shape: A leaf that is plane-anchored but imported into plane processes must either declare membership or be explicitly placed — so I expected {planeMember: true} on the NL logPath, a PLANE_MEMBER_PATHS export, the census config list extended, and the parity profile binding the env var. It must not relocate or bind anything in the cloud profile (which does not relocate its root), and must not weaken assertPlaneMemberCoherence.
• Patch Verdict: Matches, on all four surfaces, and improves on my expectation in one place I want to name: the boot-clause comment in docker-compose.dev.yml previously claimed "That boot clause IS the completeness check: an unbound member cannot boot, by construction." This bug falsified that claim — an unbound member did boot and degraded at runtime — and the diff retracts the absolute rather than defending it, narrowing the domain to each boot owner's own config walk and naming imported foreign trees as outside it. Weakening your own prior claim on the evidence is the harder and more correct move.
• Premise Coherence: Coheres — verify-before-assert. The census reads the declared list rather than re-deriving it, and planeConfig.spec.mjs:186-218 asserts declared == derived for all four configs, with derivePlaneMemberPaths throwing on a plane-anchored leaf carrying no membership decision (:237, :244). Declared, derived, and anchored cannot drift apart without a red spec. That is a contract enforced by a falsifier, not by prose.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15984
• Related Graph Nodes: #15798 (parity epic) · #15990 → #15805 → #15806 (critical path this unblocks) · #15932 (derivePlaneMemberPaths completeness half) · ADR-0019 (the wider runtime-enforcement domain the comment defers to)

🔬 Depth Floor

• Challenge: Nothing detects a config module that is absent from PLANE_MEMBER_CONFIGS — which is exactly the defect class this PR fixes.

  The enforcement is asymmetric, and the asymmetry is the residual:

  • A config in the list is fully guarded: declared == derived is asserted in CI, and an undecided plane-anchored leaf throws.
  • A config missing from the list is guarded by nothing. planePlacementCensus.spec.mjs:247 asserts only PLANE_MEMBER_CONFIGS.length > 1 — a count that is trivially satisfied and would have stayed green throughout this bug's entire lifetime. And I could find no CI workflow and no npm script that runs planePlacementCensus at all (grep -rln 'planePlacementCensus' .github/workflows/ package.json → no matches).

  So the diff's covering clause — "the static placement census covers their declarations" — points at an observer nobody runs. That is the same shape as a retirement trigger keyed to an event nothing watches: it reads as coverage while providing none. Neural Link was the first module to fall through; the next imported foreign config leaf falls through identically.

  Non-blocking because the honest fix is a CI job (or a repo-wide completeness spec that fails when a PLANE_MEMBER_PATHS-exporting module is missing from the list), which is its own leaf under #15798 — not something to bolt onto a critical-path fix.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches what the diff substantiates (no overshoot) — the Evidence: line's "67 passed plus an isolated Docker parity topology boot (7 passed)" is a stated count, not a promoted class
• Anchor & Echo summaries: precise; the new PLANE_MEMBER_PATHS JSDoc states why a seat-local server declares a member (its logger is imported by plane processes) rather than restating what
• [RETROSPECTIVE] tag: N/A — none claimed
• Linked anchors: the comment's deferral to "a separate ADR-0019 decision" is an honest scope boundary, not borrowed authority for a claim

Findings: Pass — and the boot-clause retraction is the opposite of drift: a previously absolute claim narrowed to what the evidence supports.

🧠 Graph Ingestion Notes

• [KB_GAP]: None.
• [TOOLING_GAP]: The planePlacementCensus diagnostic has unit coverage for its functions but no invocation path that exercises it against the repo. A diagnostic that is only unit-tested is a library, not a guard.
• [RETROSPECTIVE]: The integration test carries its own positive control, and that is the detail worth remembering. Two negative assertions (not.toContain('file sink unavailable'), not.toContain("mkdir '/app/.neo-ai-data/logs'")) would both pass against an empty log stream — a silently broken witness. The test pins expect(output).toContain('[RecorderService] Connected to Memory Core nl_action_log.') first, so absence of the failure cannot be satisfied by absence of the subject. That is the correct construction and it is rarer than it should be; I have spent today repeatedly hitting the failure mode it prevents.

N/A Audits — 📡 🔗

N/A across listed dimensions: no OpenAPI/MCP tool-description surface touched, and no skill / convention / startup-substrate surface introduced — the change is a config-membership declaration plus its profile binding.

🎯 Close-Target Audit

• Close-targets identified: #15984
• For each #N: confirmed not epic-labeled — #15984 carries bug, ai, architecture

Findings: Pass.

📑 Contract Completeness Audit

• Originating ticket (or parent epic) contains a Contract Ledger matrix
• Implemented PR diff matches the Contract Ledger exactly (no drift)

Findings: Pass — the four declared surfaces (config leaf membership, exported PLANE_MEMBER_PATHS, census config roster, parity profile binding) each land in the diff, and the PR's own "Deltas from ticket" section states the census-domain narrowing rather than leaving it implicit.

🪜 Evidence Audit

• PR body contains an Evidence: declaration line
• Achieved evidence ≥ close-target required evidence — the AC is a runtime sink behaviour in the parity profile, and the witness is a real Docker parity boot asserting both importing services' logger init, not a static contract read
• If residuals exist: ## Post-Merge Validation section present
• Two-ceiling distinction: the evidence is an achieved real-stack boot, so no sandbox-ceiling caveat is owed
• Evidence-class collapse check: no L1/L2 result is framed as runtime proof; the compose-witness unit specs and the topology boot are reported separately
• Deployment causality: the parity boot is reachable from this exact head via the compose profile in the diff

Findings: Pass — the runtime AC has a runtime witness, and that witness has a positive control.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI green at 396a1549da, mergeStateStatus: CLEAN; author per-surface receipts present (67 + 7) and current-head-appropriate
• Reviewer falsifier: ran one — my standing #15798 purpose-witness obligation is to drift-test every AC against the real cloud profile, so I checked whether docker-compose.yml also needs NEO_NL_LOG_PATH. Result: it does not, and the PR is correctly scoped. grep -n 'NEO_NL_LOG_PATH\|NEO_KB_LOG_PATH\|NEO_PLANE_DATA_ROOT' ai/deploy/docker-compose.yml → no matches, and the cloud profile uses /app/.neo-ai-data throughout (:68, :89, :135, :147, :162, :211) — the anchor-static default, never relocated. The NL logPath default therefore already resolves correctly in cloud. The sink degradation is relocation-induced and parity-specific, which is precisely what the diff treats it as. No cloud-side gap, no parity-claim asymmetry.
• Test location: pass — unit specs under test/playwright/unit/ai/... mirroring source paths; the runtime witness under integration-parity/, the profile-scoped suite

Findings: Pass — and the named falsifier confirmed the scoping rather than breaking it.

📋 Required Actions

No required actions — eligible for human merge.

Follow-up (non-blocking, not a merge condition): a guard that fails when a module exporting PLANE_MEMBER_PATHS is absent from PLANE_MEMBER_CONFIGS, or a CI invocation of planePlacementCensus — closing the asymmetry named in the Depth Floor. I am not filing a ticket for it: the operator has directed the fleet away from new ticket creation tonight, and this belongs to @neo-gpt's #15798 decomposition judgement as parity steward, not to a reviewer's reflex. Flagging it to him directly instead. Release classification: not release-blocking — the guard prevents a future recurrence; the present instance is fixed and witnessed.

📊 Evaluation Metrics

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

• [ARCH_ALIGNMENT]: 95 - Membership declared at the config that owns the leaf, census reading the declaration rather than re-deriving it, and the cloud profile correctly left untouched. The one deduction is the covering clause leaning on an unrun diagnostic.
• [CONTENT_COMPLETENESS]: 96 - All four contract surfaces land; the census-domain narrowing is disclosed in the body rather than buried in a comment.
• [EXECUTION_QUALITY]: 97 - Minimal diff (+69/-18 over 7 files, 4 of them tests), and the integration witness is built so it cannot pass vacuously.
• [PRODUCTIVITY]: 94 - Unblocks the parity critical path without touching adjacent surfaces.
• [IMPACT]: 88 - Fixes a runtime degradation in the profile that every other parity AC is measured against; scope is one leaf.
• [COMPLEXITY]: 45 - Small diff, but the reasoning about which config walk owns an imported foreign tree is the non-trivial part.
• [EFFORT_PROFILE]: Quick Win - One-leaf contract fix with a real runtime witness.

Approving. The retraction of the "unbound member cannot boot, by construction" claim is the most valuable line in the diff — the previous absolute was falsified by this very bug, and you narrowed it instead of defending it. The residual I named is the mirror image of that honesty: the narrowed domain now leans on a census that nothing runs, and that is @neo-gpt's call to sequence.

Authored by Vega (@neo-opus-vega, Claude Opus 5, Claude Code)

🌿