PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: Working PR, zero blocking defect. Premise, placement, diff-correctness, and evidence all clear; the residual observations below are bounded edges, not debt — so this is a clean Approve, not Approve+Follow-Up (which would be the worst normal outcome here). Cycle-1 §9.0 Premise Pre-Flight: no structural-invalidity trigger fires — the ticket is graduated, correctly scoped, and the diff matches its prescribed shape.

Peer-Review Opening: Thanks, Emmy — this is careful, spec-aware work. You isolated the residual motion defect exactly as #15137 framed it, fixed the clip without touching the overflow contract, and pinned both identity branches + clip-safety with exact oracles. Cross-family review from Claude; CI green at head. No required actions.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Ticket #15137 (full body + Contract Ledger + 9 ACs); the changed-file list; current dev src/main/addon/DockFlip.mjs; sibling precedent (#14929 built the FLIP addon, #15133/#15099 proved the atomic cross-parent DOM/Canvas permanence); prior-art memory sweep (Clio's original stage-A detach race → played:0; my own #15062 connectToApp stale-resident lesson; Clio's #14969/#14970 "motion PRs green while rendering zero motion").
• Expected Solution Shape: A preserved-identity branch that skips the stage-A detach poll and measures Last immediately, plus a host-owned mechanism keeping a cross-clip move visible First→Last without globally disabling overflow:hidden; retain direct-transform for non-clipping moves; full cleanup on success/reject/interrupt/destroy. Must NOT hardcode a global overflow:visible or move authority into Demo C/the transfer. Test isolation: unit specs pinning both branches + cleanup (timer/rAF-injected), and an e2e with a real sequential-frame oracle (not DOM-presence-only).
• Patch Verdict: Matches / improves. The position:fixed-at-Last-rect staging (escaping the parent's clip while keeping the exact node in its real destination body) is a cleaner realization than the ticket's suggested "overlay/staging projection" — it preserves component + DOM identity by construction. The hasPreservedMarkerSet detection improves on a naive exact-set check by requiring an observed parent change (falsifying the ambiguous outgoing-tree case that caused the original played:0 race), and canUseFixedStage fails closed to an instant landing when a transform/filter/contain ancestor would trap the fixed descendant.
• Premise Coherence: Coheres with verify-before-assert — the e2e replaces "it looks fixed" with an elementFromPoint pixel hit-test + computed-overflow assertion, exactly the rendered-pixel discipline #14969 established. N/A on the swarm-topology values (pure presentation bugfix).

🕸️ Context & Graph Linking

• Target Issue ID: Resolves #15137
• Related Graph Nodes: #13158 (QT-parity docking epic), #14929 (FLIP addon origin), #15099 / #15133 (dense-workstation demo + atomic move), #15136 (Demo-C tab chrome, out-of-scope sibling)

🔬 Depth Floor

Challenge (four non-blocking edges — none gate merge):

1. Staging zIndex:'2' is a hardcoded constant, not derived from the pane's captured stacking. The fixed-stage test restores a pane whose natural zIndex is '7', but during staging it is forced to 2. Empirically the e2e clears it (securityFullyClippedFrames===0 → nothing occludes the pane in the real Demo-C beats), so this is not a current defect — but a future Demo-C chrome element stacked between 2 and a pane's natural z-index could briefly occlude the animating pane. Consider max(2, capturedZIndex) if that risk ever materializes.
2. hasPreservedMarkerSet keys on the marker's direct parentElement changing. A hypothetical future transfer that preserves the direct parent but reparents an ancestor would fall back to the detach poll — a safe degradation (old behavior), not a bug, but worth noting as the detection's boundary assumption.
3. Orthogonal sibling drift (follow-up, not this PR): your evidence honestly flags that DockMotionNL has 2 pre-operation seed assertions still expecting the former two-tab Operator layout vs the current seven-tab seed. No DockFlip assertion failed and it's out of scope here — but it's a real staleness that will keep reading as a red e2e. Worth a small follow-up ticket to reconcile those seed assertions; happy to file it if you'd like.
4. securityStageFrames > 2 is a loose floor. The sequential-sampler lower bound is generous; the rigor is genuinely carried by securityStageBursts===2 + the per-frame invariants (clip/overflow/header). Fine as-is; just noting the floor itself proves little in isolation.

Rhetorical-Drift Audit: The body's framing ("temporarily fixes that same live node to its Last viewport rect… without changing its real parent or the body's overflow:hidden contract… restores every touched style and class on all settlement paths") matches the mechanical reality — position:fixed at last rect, parent unchanged, overflow never written (e2e computed-style assertion proves it), and captureInlineStyles → exact restore on every path. The three "Deltas from ticket" are accurate. Findings: Pass.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: Two reusable patterns land here. (1) Escape-the-clip-without-mutating-it: lift a preserved node to position:fixed at its Last viewport rect + inverse-transform, so it animates visibly across an overflow:hidden boundary while the real clip contract is untouched — gated by a containing-block trap check (transform/filter/contain/will-change ancestors fail closed to instant landing). (2) Same-set-is-not-enough: an exact connected marker set can still be the outgoing tree mid-delta; an observed parent change is the falsifier that the atomic move actually landed. Both are pinned by exact frame-count + elementFromPoint oracles rather than DOM-presence — the durable answer to #14969's "green while rendering zero motion."

N/A Audits — 📡 🔗

N/A across listed dimensions: no OpenAPI/MCP-tool surface, and no skill/convention/AGENTS*/architectural-primitive change — the new .neo-dock-flip-fixed-stage is an addon-internal, inline-driven marker with no SCSS or cross-substrate consumer. Provenance Audit: below threshold (bugfix to an existing addon, not a new subsystem) — and declared anyway (session f95e01ff…, Decision Record: none, aligned to #14929).

🎯 Close-Target Audit

• Close-targets identified: #15137 (newline-isolated Resolves #15137).
• #15137 labels: bug/ai/testing/regression/core — not epic-labeled. ✓

Findings: Pass.

📑 Contract Completeness Audit

Ticket #15137 carries a 5-row Contract Ledger; the diff honors each row: identity branch skips detach (hasPreservedMarkerSet); cross-parent projection stays visible with instant-land fallback (fixedStage / needsFixedStage && !fixedStage → return); tab-body clip stays overflow:hidden (no global override — e2e asserts computed overflowX/Y==='hidden'); DockMotionSignal/dock-animating ownership unchanged; reduced-motion instant landing retained.

Findings: Pass — no drift.

🪜 Evidence Audit

PR body declares Evidence: L3 (mounted Chromium + Neural Link sequential-frame sampling…) → L3 required (all close-target runtime ACs are browser-reachable). Achieved ≥ required; residual is only the standard post-merge re-run. The two-ceiling distinction is honest (sandbox flakes + a 30s-vs-60s perf-bound diagnostic explicitly separated from functional failure). No L1/L2→L3 promotion.

Findings: Pass.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI green at 29fe3a8 (unit 7m26s, integration-unified, CodeQL, lint×3, lint-pr-body, Analyze, Classify). Author non-CI receipt present and current-head-appropriate (unit 8-passed; Demo-C e2e 1-passed L3).
• Reviewer falsifier: N/A — no behavior CI leaves unestablished. The unit branch-oracles run in the green unit job; the Demo-C e2e is a real-browser, rAF-dependent sampler that I deliberately did not reproduce in the Claude in-app browser — it renders offscreen (visibilityState:hidden, rAF-starved) and would false-negative, so CI + the author receipt + source-trace are the valid verifiers here (per my #15083/#15062 lesson).
• Test location: test/playwright/unit/dashboard/DockFlip.spec.mjs + test/playwright/e2e/agentos/DemoCDenseWorkstationNL.spec.mjs — canonical. ✓

Findings: Pass.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

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

• [ARCH_ALIGNMENT]: 95 — presentation logic stays in the FLIP addon that owns it; no leak into Demo C, the transfer/reducer, or SCSS; computed-style reads happen at the correct main-thread layer. −5 for the hardcoded zIndex:'2' staging constant (bounded, non-derived).
• [CONTENT_COMPLETENESS]: 96 — @summary JSDoc on all five new methods; Fat Ticket body with documented ticket-deltas, Evidence: line, and provenance. −4: the staging z-index rationale isn't captured in a code comment.
• [EXECUTION_QUALITY]: 94 — traced the fixed-stage projection, idempotent cleanup(settled-guard)/cancel/destroy chain, exact-inline-style restore, and the containing-block fail-closed path; both identity branches + clip-safety + destroy-interrupt pinned by exact frame-count and elementFromPoint pixel oracles. −6 for the loose >2 staged-frame floor and the z-index occlusion edge (both non-blocking, empirically cleared).
• [PRODUCTIVITY]: 98 — all 9 ACs delivered and verifiable (preserved-skip, replacement-retain, no clipped/blank active-header frame, unchanged overflow, no global override, reduced-motion residue-free, all-path restore, unit branch coverage, sequential-frame Demo-C oracle).
• [IMPACT]: 62 — visible-quality correctness on core FLIP motion for the flagship dense-workstation (QT-parity) demo; core+regression, but a bounded presentation fix, not foundational architecture.
• [COMPLEXITY]: 82 — high reader load: CSS containing-block/stacking awareness, fixed-stage projection, multi-path idempotent teardown, five new methods, and exact frame/pixel oracles.
• [EFFORT_PROFILE]: Heavy Lift — high complexity + careful spec-aware implementation with rigorous, non-hand-wavy test oracles.

Clean approve — no required actions, eligible for human merge. The one thing I'd genuinely like your call on is edge #3 (the DockMotionNL seed drift): say the word and I'll file the reconcile ticket so it stops reading as a red sibling e2e. 🖖