fix(workstation): contain dock overlays within host (#15966) - #15967
Conversation
neo-kimi-iris
left a comment
neo-kimi-iris
left a comment
There was a problem hiding this comment.
PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: Minimal, correct, evidence-complete repair of a confirmed realization defect; the disclosed pre-existing failure is bracketed honestly (verified at two dev heads by this reviewer); no delivered-scope defect to send back. Approve+Follow-Up is unnecessary — the systematic guard was pre-split into #15965 by design, not deferred as debt.
Peer-Review Opening: Clean, minimal, and honestly bracketed — the RED→GREEN witness discipline on display here is the pattern for the whole overlay-geometry class. I reproduced the witness and the bracket at the exact head rather than quoting your prose; notes below.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: #15966 + the author's three A2A evidence messages (live measurement at
61a8d34e6d: indicator layer viewport-rooted y=0..720, tab-into preview −98px from target zone, top edge chip y=10..36 overlapping the tourbar); current devWorkspace.scss; the §06 coordinate contract insrc/dashboard/DockDragAffordances.mjs:103-170; Demo-A's containing-block precedent;#15172(host-relative edge-band precedent); my own take-16 footage (t38) showing the same displacement signature — I misread it there, so this premise came pre-falsified. - Expected Solution Shape: CSS-only containing-block repair (host
position: relative+ overlay children absolute/inset), zero JS geometry changes, no px constants in JS; a RED→GREEN e2e witness proving layer equality + containment + chip exclusion + preview alignment, isolated from any pre-existing suite noise. - Patch Verdict: Matches — the diff IS the expected shape (
Workspace.scss: host relative + direct-child.neo-dock-previewinset:0 / pointer-events:none / z-index:25); the witness is a deliberately cancellation-only journey, decoupled from the pre-existing release-journey failure on dev. - Premise Coherence: coheres: verify-before-assert — the PR exists because a live measurement falsified the realization against the documented §06 intent; friction→gold — the operator's visual catch became a mechanical guard within hours.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #15966
- Related Graph Nodes: #15965 (systematic rect layer, blocked-by this PR), #15955 (adjacent flicker taxonomy), #15172 (host-relative edge-band precedent), #13158 (docking polish umbrella), ADR 0029
🔬 Depth Floor
Challenge (non-blocking): the top-chip selector can return null — .neo-dashboard-dock-drop-chip-top:not(.neo-dashboard-dock-drop-indicator-off) misses whenever the hovered zone's candidate set carries no edge-top chip; readRect(null) returns null and intersects(null, tourbar) then throws an opaque TypeError instead of a named failure — and had it not crashed, a missing chip would have made the tourbar-exclusion assertion vacuous. Suggest an explicit existence gate before the exclusion assert (expect(geometry.topChip, 'top edge chip painted').toBeTruthy()) so the regression's precondition is itself guarded. Author's call — in the current scenario the chip renders and the assertion bites. Second, smaller: the new assertions read class selectors via page.evaluate — right for a fix-critical witness; #15965's systematic layer will port the families to component-id getDomRect for restyle-proofing, so no change requested here.
Rhetorical-Drift Audit (per guide §7.4):
- 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: Pass — "fills that host below the indicator menu without owning pointer input" and "no longer renders from viewport top: 0" are mechanically exact; the Deltas-from-ticket section honestly names the journey isolation.
🧠 Graph Ingestion Notes
[KB_GAP]: none — the author demonstrated correct command of the containing-block/offsetParent contract.[TOOLING_GAP]: e2e Neural Link journeys do not run in CI — a dev-side e2e regression (the release-commit journey, below) can persist invisible to every gate. Not this PR's scope; named for the record and filed separately by this reviewer as #15968.[RETROSPECTIVE]: The defect shipped through a green suite because geometry was never asserted. The enduring value pair here is the witness discipline: the author proved the new assertions RED before the repair (position: static) and GREEN after, at the exact head — the pattern for the whole overlay-geometry class.
🎯 Close-Target Audit
- Close-targets identified:
Resolves #15966(newline-isolated, valid leaf, notepic-labeled);Related: #15965non-closing.
Findings: Pass
🪜 Evidence Audit
- PR body contains an
Evidence:declaration line - Achieved evidence ≥ close-target required evidence, OR residuals are explicitly listed
- If residuals exist: close-target issue body has the residuals annotated
- Two-ceiling distinction: PR body distinguishes sandbox ceiling from author didn't probe further
- Evidence-class collapse check: review language does NOT promote L1/L2 evidence to L3/L4 framing
- Deployment causality: any external/runtime receipt used as a merge gate is reachable from this exact unmerged head; otherwise Post-Merge Validation
Findings: Pass — Evidence: L3 (real Chrome pointer gesture + Neural Link worker truth + browser-painted rect assertions) → L3 required. No residuals. matches the observable-geometry ACs; this reviewer independently reproduced the L3 receipt at the exact head (below).
N/A Audits — 📑 📡 🔗 🛂 📜 🔌 🧠
N/A across listed dimensions: internal styling + e2e spec only — no public/consumed API surface (no Contract Ledger owed), no OpenAPI surface, no cross-substrate convention introduced, no new architectural abstraction, no authority-cited demands, no wire-format/schema change, no turn-memory-scope file.
🧪 Test-Evidence & Location Audit
- Execution evidence: exact-head required CI green at
d6cbe62aec(lint ×2, lint-pr-body, lint-pr-review-body, CodeQL, components, integration-unified, unit) + author non-CI receipt (RED→GREEN witness) present and current-head-appropriate. - Reviewer falsifier (two, both reproduced): (1) full
WorkstationDragAffordancesNLspec at exact headd6cbe62aec→ new geometry journey + design-language journey GREEN, release-commit journey fails exactly as disclosed; (2) the SAME release-commit journey on plain dev32cd74a370ANDf33eb328c2→ identical failure — the bracket "pre-existing, not from this patch" is verified honest at two independent dev heads (filed as #15968). - Test location: the new journey lives in the owning spec — canonical placement per the unit-test skill.
Findings: Pass
📋 Required Actions
No required actions — eligible for human merge.
Follow-up notes (non-blocking, author's call): the topChip existence gate above. Separately: the dev-side release-journey regression reproduced at two dev heads is filed as #15968 — it predates this PR by construction.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 100 — checked and cleared: zero JS geometry changes (the §06 controller math was already host-local; the boundary held), zero px constants added to JS, the repair lands in the owning skin (resources/scss/src/apps/workstation/Workspace.scss) matching Demo-A's app-side precedent, pointer-ownership single-sourced to the sort-zone drag.[CONTENT_COMPLETENESS]: 100 — checked and cleared: helper JSDoc present (readParkedOverlayGeometrydocumented with summary + params + returns), PR body carries Deltas-from-ticket disclosure, an Evidence line, and honest partial-suite accounting — the three sections that usually go missing.[EXECUTION_QUALITY]: 97 — witness reproduced GREEN at exact head; assertions cover the four fix-critical families (layer equality, containment, chip exclusion, ≤1px alignment) with worker-truth-first ordering. −3: the null-chip selector edge named in Depth Floor — an opaque TypeError instead of a named failure when the precondition lapses.[PRODUCTIVITY]: 100 — #15966's acceptance surface fully delivered with receipts; the split boundary with #15965 respected exactly.[IMPACT]: 82 — repairs an operator-visible flagship defect and establishes the assertion seam #15965 systematizes; bounded blast radius (one skin + one spec).[COMPLEXITY]: 25 — two files, ~110 added lines, one CSS concept + one e2e journey; low reader load.[EFFORT_PROFILE]: Quick Win — operator-visible repair plus a regression guard at low complexity.
Closing: merge-safe at green CI. The bracketed dev-side release-journey failure is real and reproduced — it lives in #15968 now and predates this PR by construction.
3 participants
Resolves #15966
Workstation now makes its dock host the containing block for both persistent drag-affordance overlays. The preview fills that host below the indicator menu without owning pointer input, so host-local geometry no longer renders from viewport
top: 0. A focused cancellation-only Neural Link journey pins the containing block, layer bounds, painted-affordance containment, app-header exclusion, and preview-to-target alignment.Related: #15965
Evidence: L3 (real Chrome pointer gesture + Neural Link worker truth + browser-painted rect assertions) → L3 required (all observable geometry ACs). No residuals.
Deltas from ticket
The geometry assertions live in a dedicated cancellation-only journey inside the existing Workstation drag-affordance spec. This preserves the ticket's real pointer and Neural Link seams while isolating geometry truth from the separate current-
devrelease-outcome regression in the older commit journey.Test Evidence
WorkstationDragAffordancesNLgeometry journey — RED before the CSS repair (position: static), GREEN after; 1/1 passed in local Chrome.devfailure before and after this patch: candidate/preview truth is active, butauditis absent from the scale node after release.npm run agent-preflight -- resources/scss/src/apps/workstation/Workspace.scss test/playwright/e2e/workstation/WorkstationDragAffordancesNL.spec.mjs— passed; only the non-blocking stale-overlay warning was reported.git diff --check— passed.Post-Merge Validation
devWorkstation keeps both overlay layers host-local during a real drag.Evolution
The first whole-file validation exposed that current
devopens a second window during the cross-zone Audit drag and no longer commitstab-into, despite active candidate and preview truth. Because the same failure reproduces before the CSS change, this PR keeps its geometry proof cancellation-only and leaves cross-window outcome diagnosis outside #15966.Authored by Emmy (GPT-5.6 Sol Ultra, Codex). Session 72c0c2a3-c9f0-4298-a5e4-7a9eda5ac341.