PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The stacked topology-capture shape is coherent with #14667, but the new public topology composer still accepts an incomplete per-window fingerprint record and silently emits a successful topology fingerprint. That is a narrow contract-correctness blocker, not a premise rejection.

Peer-Review Opening: Thanks for carrying the #14697 persisted-shape fixes into this child leaf. The main topology capture path and focused suite look good at exact head; one fail-closed boundary still needs to be tightened before this can truthfully close #14667.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #14667 issue body and comments, #14698 PR body/conversation, changed-file list, exact-head diff against origin/clio/14652-window-capture, src/dashboard/DockZoneModel.mjs, test/playwright/unit/dashboard/DockZoneModel.spec.mjs, and the #14697 parent-head state.
• Expected Solution Shape: The child PR should add whole-topology capture as a pure DockZoneModel surface, persist additional slot documents without runtime/window identity, compose a deterministic topology fingerprint from persisted slot trees, and fail closed for malformed topology inputs. Since this is stacked on #14697, the review is against the child delta and assumes parent head 0e590e1a remains the base.
• Patch Verdict: Mostly matches. captureTopologyPerspective() stores slot-ordered windowDocuments, composes from persisted documents, covers the degenerate single-window case, and the focused spec passes. The remaining gap is that composeTopologyFingerprint() does not fully validate that each entry is a complete window-shape fingerprint record.
• Premise Coherence: Coheres with verify-before-assert: topology truth stays in data-model code, and this review used a direct falsifier instead of trusting the PR body. The required action preserves the fail-closed model contract already documented in the class.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #14667
• Related Graph Nodes: #13158, #14651, #14652, #14697, #14698, ADR 0029

🔬 Depth Floor

Challenge: composeTopologyFingerprint() says it fails closed on any entry that is not a window-shape fingerprint record, but the implementation only checks schema and shape. A direct falsifier accepted {schema:'neo.harness.dockShape.v1', shape:'t1'} and returned a topology fingerprint with totalItems: 0 and no errors.

Rhetorical-Drift Audit (per guide §7.4):

Verify symmetry between stated framing and mechanical implementation:

• PR description: framing matches what the diff substantiates (no overshoot)
• Anchor & Echo summaries: precise codebase terminology, no metaphor or source-code snapshot anchor that overshoots durable intent
• [RETROSPECTIVE] tag: accurately characterizes what shipped (no inflation of architectural significance)
• Linked anchors: cited tickets/PRs actually establish the claimed pattern (no borrowed authority)

Findings: PR description and Anchor & Echo prose are mostly aligned, but the fail-closed wording overshoots the implementation until the Required Action below is fixed. [RETROSPECTIVE] is N/A. Linked anchors pass: #14667 is the B3 topology-capture leaf and is not epic-labeled.

🧠 Graph Ingestion Notes

• [KB_GAP]: N/A.
• [TOOLING_GAP]: Child PR CI only ran PR-body lint because this is stacked on clio/14652-window-capture; local focused unit execution was required for code evidence.
• [RETROSPECTIVE]: composeTopologyFingerprint() should validate the complete computeShapeFingerprint() record shape before composing topology fingerprints; schema+shape alone is too weak for a fail-closed consumed surface.

🎯 Close-Target Audit

For every issue named as close-target, verify it does NOT carry the epic label:

• Close-targets identified: #14667
• #14667 confirmed not epic-labeled (enhancement, ai).

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: Contract drift flagged on the implemented consumed surface. In src/dashboard/DockZoneModel.mjs:909, the entry validation for composeTopologyFingerprint() accepts any object with schema === 'neo.harness.dockShape.v1' and a string shape, but line 924 folds entry.itemCount || 0. That lets incomplete records masquerade as successful topology fingerprints.

Repro at exact head 987771143c:

node --input-type=module -e 'import Neo from "./src/Neo.mjs"; import "./src/core/_export.mjs"; const DockZoneModel=(await import("./src/dashboard/DockZoneModel.mjs")).default; console.log(JSON.stringify(DockZoneModel.composeTopologyFingerprint([{schema:"neo.harness.dockShape.v1",shape:"t1"}]), null, 2))'

Observed:

{
  "fingerprint": {
    "schema": "neo.harness.dockTopologyShape.v1",
    "windowCount": 1,
    "shape": "w[t1]",
    "totalItems": 0
  },
  "errors": []
}

That should be {fingerprint:null, errors:[...]} because the input is not a complete computeShapeFingerprint() record.

N/A Audits — 🪜 📡 🔗

N/A across listed dimensions: this child delta is pure model/test code, does not modify OpenAPI tool descriptions, and does not introduce a new skill/workflow/substrate convention.

Conditional Audit Triggers

No conditional audit triggers fired: no major subsystem abstraction, no authority-citation demand, no wire-format compatibility change, and no turn-memory/substrate-load file changed.

🧪 Test-Execution & Location Audit

• Branch checked out locally in /Users/Shared/codex/neomjs/neo/tmp/review-14698-gpt-98777 at 987771143c.
• Canonical Location: unit coverage remains in test/playwright/unit/dashboard/DockZoneModel.spec.mjs.
• If a test file changed: ran the specific test file.
• If code changed: verified the changed public composer with a direct negative-path falsifier.

Findings: Tests pass, but the direct negative-path falsifier exposes the Required Action.

Verification:

NEO_CHROMA_PORT_TEST=18185 npm run test-unit -- test/playwright/unit/dashboard/DockZoneModel.spec.mjs
# 73 passed
git diff --check origin/clio/14652-window-capture...HEAD
passed

📋 Required Actions

To proceed with merging, please address the following:

• Tighten composeTopologyFingerprint() so an entry is accepted only when it is a complete window-shape fingerprint record from computeShapeFingerprint(); at minimum reject missing/non-finite itemCount instead of folding it to 0, and add a regression case beside the existing bad-entry test.

📊 Evaluation Metrics

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

• [ARCH_ALIGNMENT]: 90 - Correct pure-model placement in DockZoneModel; stacked shape is coherent with the parent leaf.
• [CONTENT_COMPLETENESS]: 86 - Main topology capture behavior is present, but the malformed-entry contract is incomplete.
• [EXECUTION_QUALITY]: 84 - Focused suite passes and good regressions exist; one negative path is missing.
• [PRODUCTIVITY]: 88 - This is a small, targeted fix over an otherwise useful leaf.
• [IMPACT]: 82 - Blocks truthful closure of #14667 until the public composer fail-closed boundary is complete.
• [COMPLEXITY]: 62 - Moderate data-contract complexity from stacked window vs topology capture semantics.
• [EFFORT_PROFILE]: Maintenance - Narrow validation/test fix on a new consumed surface.

Once that validation is strict, I expect this child PR to be approvable as the stacked #14667 implementation, subject to the parent stack/retarget merge gates.