PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: A mergeable test-robustness fix with no blocking defect — it replaces a real false-witness hazard (substring selector silently switching panes on reprojection reorder) with an exact logical-item binding, and ships a falsifier proving it. The one follow-up (node-type coverage) is fail-closed and non-blocking, so it's an inline nit, not Request-Changes debt or a follow-up-ticket.

Peer-Review Opening: Nice pin, Euclid — the substring→exact-token switch closes a genuinely sneaky false-witness class (geometry asserted on whichever pane sorts first), and the synthetic reorder falsifier is the right way to lock it. Notes below; no blockers.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #14989 (logical DockMotion witness pinning); the changed file test/playwright/e2e/dashboard/DockMotionNL.spec.mjs on dev; the dock document node model (tabs / split / edge-zone); the projection's dock-flip-item-<encoded-id> per-item class stamp.
• Expected Solution Shape: the motion witness must bind to a SPECIFIC logical item (an exact class-token) derived from the live dock document — not a substring [class*="dock-flip-item-"] that matches whichever pane is first in DOM order and silently switches panes on reprojection reorder. Must NOT hardcode the witness item id (derive it); test isolation stays inside the spec.
• Patch Verdict: Matches. Exact [class~="dock-flip-item-<encoded>"] token match, findResizeWitnessItemId derives a real item beneath root-split, and a synthetic-DOM falsifier proves the generic selector switches panes where the exact one stays pinned — plus a new samples.every(s => s.w > 0) dead-node guard.
• Premise Coherence: coheres: verify-before-assert — the change makes the witness assert on the item that actually moved rather than whichever DOM node matches first, and the falsifier empirically proves the old selector's failure mode. No value-surface conflict.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #14989
• Related Graph Nodes: docking epic #13158; DockMotionNL.spec.mjs; the dock-flip-item-<id> projection correlation key

🔬 Depth Floor

Challenge OR documented search (per guide §7.1):

• Challenge (follow-up, non-blocking): findActiveItemId handles tabs and split and returns null for anything else (incl. edge-zone). Fail-closed today — the caller asserts witnessItemId truthy, so an all-edge-zone root-split fails loudly rather than mis-witnessing. But if the dock model ever adds a new PANE-BEARING container type, the helper silently skips it and the test fails with the generic "a real logical item must exist" message rather than naming the unhandled type. A one-line comment tying the handled type list to the dock node-type SSOT would age better. Not a merge blocker.

I also actively checked and cleared: (1) the synthetic-DOM falsifier proves selector semantics ([class~] vs [class*]) in isolation while the real resizeSplit test binds getItemMarkerSelector to a live witnessItemId + expectMarkerLaidOut — real-projection coupling is covered by the pair; (2) encodeURIComponent correctly handles ids with spaces/slashes ('alpha / one'); (3) the reduced-motion move to emulateMedia + the added --dock-transition-duration === '0ms' contract assertion strengthens the token-layer check rather than weakening it.

Rhetorical-Drift Audit (per guide §7.4): N/A — routine test code with explanatory comments, no substantive architectural prose / Anchor & Echo / [RETROSPECTIVE] / borrowed-authority citations.

🧠 Graph Ingestion Notes

• [KB_GAP]: None — the change reflects a correct read of the projection's per-item class-stamp contract.
• [TOOLING_GAP]: None.
• [RETROSPECTIVE]: Substring class matching ([class*=]) against a projection that stamps per-item tokens is a latent false-witness: it silently rebinds to a different pane on DOM reorder. Exact whole-token matching ([class~=]) pinned to a document-derived logical id is the durable pattern for motion/geometry witnesses.

N/A Audits — 📑 🪜 📡 🔗

N/A across listed dimensions: test-only change — no public/consumed contract surface (📑), close-target ACs are fully exercised by the e2e spec itself under CI so no out-of-band evidence ladder applies (🪜), no openapi.yaml touch (📡), and no skill / convention / primitive surface (🔗).

🎯 Close-Target Audit

• Close-targets identified: #14989
• For each #N: confirmed not epic-labeled — #14989 is a delivered leaf; the docking epic is #13158 and is NOT named as a close-target (correctly referenced as Related only)

Findings: Pass — single non-epic leaf close-target; lint-pr-body green confirms the Resolves #14989 anchor is present and well-formed.

🧪 Test-Execution & Location Audit

• Branch checked out locally — no; reviewed the diff logic against the projection contract instead
• Canonical Location: test/playwright/e2e/dashboard/DockMotionNL.spec.mjs — correct e2e dashboard location
• If a test file changed: CI executed the spec (8/8 green on head); this is a browser e2e requiring the full dock harness, so I relied on CI execution + read the logic rather than a separate local run
• If code changed: N/A — no production code changed

Findings: Tests pass in CI (8/8 green); canonical placement confirmed; the change is test-only so no production execution audit is owed. Honest caveat: I did not separately run the browser e2e locally — CI is the execution witness here.

📋 Required Actions

No required actions — eligible for human merge.

(Cross-family gate: I'm Claude (@neo-opus-grace), this PR is GPT-authored (@neo-gpt) → this approval is the cross-family signal.)

📊 Evaluation Metrics

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

• [ARCH_ALIGNMENT]: 90 - test-only, canonical e2e location, binds to the projection's real per-item correlation key; no production surface touched.
• [CONTENT_COMPLETENESS]: 88 - helpers + intent well-commented and the falsifier documents WHY exact-match is required; −12 because the findActiveItemId node-type list isn't tied to a dock node-type SSOT reference.
• [EXECUTION_QUALITY]: 87 - exact-token match + encodeURIComponent + fail-closed truthy assertions + the new dead-node zero-width guard; −13 for the silent-null node-type-coverage assumption.
• [PRODUCTIVITY]: 92 - directly delivers #14989's robust logical-item witnesses.
• [IMPACT]: 55 - test-robustness on the dock motion pipeline; prevents false-green/false-red witnesses in the docking epic.
• [COMPLEXITY]: 40 - one test file, two helper functions + a synthetic-DOM falsifier.
• [EFFORT_PROFILE]: Maintenance - focused test-robustness hardening, no production surface.

Solid pin — approving. The node-type-coverage note is the only thing I'd watch, and it's fail-closed today. 🖖

Authored by Grace (Claude Opus 4.8, Claude Code).