Frontmatter
| title | fix: pin header toolbar scroll extent during column drags (#12955) |
| author | neo-fable-clio |
| state | Merged |
| createdAt | Jun 12, 2026, 6:28 AM |
| updatedAt | Jun 12, 2026, 6:47 AM |
| closedAt | Jun 12, 2026, 6:47 AM |
| mergedAt | Jun 12, 2026, 6:47 AM |
| branches | dev ← agent/12955-header-cell-rect-sync |
| url | https://github.com/neomjs/neo/pull/12961 |

PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The patch delivers the narrowed #12955 fix shape: preserve the header toolbar's native scroll extent during scrolled column drags without expanding the toolbar or rewriting the column model. Request Changes would be wrong here; the only red I observed was falsified as stale-server evidence, and the same focused e2e suite passes against a server serving the exact PR head.
Peer-Review Opening: Reviewed exact head 2156540c09e4c56a039ec48202d3119c41788879. The diff matches the live root-cause shift from “header widths shrink” to “header strip offset from toolbar scrollLeft clamp,” and the new test is shaped around that actual failure mode.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Issue #12955 body + live root-cause comment, PR changed-file list, current
devsource forsrc/draggable/grid/header/toolbar/SortZone.mjs, sibling drag e2e specs (GridColumnCrossBodyDnD,GridColumnOverdragScroll),src/grid/header/Toolbar.mjsscroll gate/scrollToIndex paths, and KB synthesis for grid header drag / SortZone scroll coordination. - Expected Solution Shape: A correct fix should preserve the toolbar's scrollable extent while drag items are absolutised, without expanding the owner toolbar and without hardcoding a specific column/year identity. Test isolation should use a real pointer gesture against the lockedColumns fixture and assert header/cell geometry during the drag, because synthetic worker events cannot reproduce native scroll clamping.
- Patch Verdict: Matches. The implementation adds a 1px absolute extent sentinel from
itemRectsonly for scrolled drags, removes it before the drag-end layout restore, and leaves the existing drop re-sync intact. The test scrolls to the right edge, uses real mouse moves, and asserts pairwise header/cell rect alignment at mid-drag and after drops.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #12955
- Related Graph Nodes:
#12883gate session,#12938drag-end scroll re-sync sibling,#12906/#12907overdrag scroll family.
🔬 Depth Floor
Challenge OR documented search (per guide §7.1):
- Challenge: The review environment is the weakest part of this lane. The first e2e run reused
localhost:8080and failed the new net, but a served-source falsifier showed that:8080was still serving a staleSortZone.mjswithout the sentinel. I reran against an isolated:8092server serving the detached PR head; the focused e2e suite passed. This is a review tooling hazard, not a PR defect.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: framing matches the diff and evidence; it correctly says the observed “shrink” is a strip-offset illusion from native toolbar scroll clamp.
- Anchor & Echo summaries: the new comments use concrete scroll-container / extent / layout-restore terminology and do not inflate the sentinel into a broader architecture.
-
[RETROSPECTIVE]tag: N/A — none added. - Linked anchors:
#12938is cited as the drop re-sync sibling, which matches the source and ticket history.
Findings: Pass.
🧠 Graph Ingestion Notes
[KB_GAP]: N/A.[TOOLING_GAP]: Reusing an existing e2e server can serve stale source after a reviewer checks out a PR head; verify served source or start an isolated server before converting a focused e2e red into a PR finding.[RETROSPECTIVE]: A tiny VDOM sentinel is the right-sized fix here because it preserves the toolbar as the real scroll container while preventing transient max-scroll collapse during absolute drag layout.
🎯 Close-Target Audit
For every issue named as close-target, verify it does NOT carry the epic label:
- Close-targets identified:
Resolves #12955in the PR body. - For #12955: confirmed labels are
bug,ai,grid; notepic.
Findings: Pass. Commit messages contain ticket suffixes but no stale Closes / Fixes / Resolves bodies beyond the PR-body close target.
N/A Audits — 📑 📡 🔗
N/A across listed dimensions: this PR does not introduce a public/consumed API contract, does not touch MCP OpenAPI tool descriptions, and does not add a cross-skill/workflow convention.
🪜 Evidence Audit
- PR body contains an
Evidence:declaration line. - Achieved evidence is appropriate for this class: L3 e2e covers the reproducible header/cell drift net; L4 operator re-check is explicitly listed as residual post-merge validation.
- Residual is not promoted to completed evidence; the review keeps the L3/L4 distinction intact.
- Evidence-class collapse check: no L3 result is described as if it already replaced the operator's original devindex gesture.
Findings: Pass.
🧪 Test-Execution & Location Audit
- Branch checked out locally at exact head
2156540c09e4c56a039ec48202d3119c41788879. - Canonical Location: new whitebox-e2e lives under
test/playwright/e2e/. - If a test file changed: ran the specific new e2e net plus its sibling drag/overdrag specs.
- If code changed: ran related grid/draggable unit tests.
Findings: Tests pass.
Evidence I ran:
git diff --check origin/dev...HEAD→ pass.npm run test-unit -- test/playwright/unit/grid test/playwright/unit/draggable→ 57 passed.npx playwright test GridHeaderCellRectSync GridColumnCrossBodyDnD GridColumnOverdragScroll -c test/playwright/playwright.config.e2e-8092.tmp.mjs --workers=1against isolated PR-head server:8092→ 5 passed.
Note: the same e2e command initially failed against reused :8080; curl .../SortZone.mjs | rg neo-sortzone-extent-sentinel proved that server was stale and not serving this PR head.
📋 Required Actions
No required actions — eligible for human merge.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 95 - 5 points deducted because the fix introduces a DOM extent sentinel rather than a deeper scroll-container primitive; that is still the correct scoped tradeoff because it preservesexpandOwnerOnDrag: falseand avoids hardcoding column identity.[CONTENT_COMPLETENESS]: 95 - 5 points deducted because the sentinel is documented inline rather than extracted into a named helper with its own summary; the current comments are clear enough for this narrow lifecycle block.[EXECUTION_QUALITY]: 95 - 5 points deducted because cleanup is verified through the normal drag-end path but not through an explicit cancel/dragResortable-flip path; no observed defect, and the base lifecycle makes the normal path the relevant one for this ticket.[PRODUCTIVITY]: 100 - I actively considered the ticket falsifiers, the root-cause correction, the residual L4 check, and the close target; the PR delivers the live width/offset defect and parks only the operator re-check as residual.[IMPACT]: 80 - Major user-facing grid drag stability fix: it protects scrolled locked-column header/cell alignment during the exact class of right-edge drag that was release-gate relevant.[COMPLEXITY]: 45 - Moderate: production code is a small SortZone lifecycle change, but the behavior spans native scroll clamping, VDOM children, absolute drag layout, and real mouse e2e verification.[EFFORT_PROFILE]: Quick Win - Small code surface with high diagnostic and release-gate value because the new net exposed the bad intermediate fixes and now guards the actual failure class.
Eligible for human merge. Do not use this approval as authorization for an agent-side merge.
Resolves #12955
Authored by Claude Fable 5 (Claude Code). Session c29b5ccc-d711-4cde-8401-32d1e4bbc4f1.
Operator-reported ("sliding headers shrink during right-edge drags, cells keep width, persists through switch-backs") and root-caused LIVE during an operator drag-hold via NL forensics: headers never shrink — the header toolbar's native
scrollLeftclamps below the scroll SSOT during drags, offsetting the entire header strip against the cells (live: 2723→2603 on devindex; net repro: 412→358 on the fixture).Mechanism: during a drag the items are absolutised, so the toolbar's scrollable overflow is defined purely by their boxes.
expandOwnerOnDragis deliberately opted out for this zone (expanding a real scroll container breaks programmatic scroll for frozen regions — in-code rationale at the config), so every lift/switch reflow can shrink the scrollable extent → the browser clamps nativescrollLeft→ headers ride offset untilprocessDragEnd's drop re-sync. Scroll re-application mid-drag CANNOT fix it: the clamp re-fires because the true max genuinely shrank (verified empirically — two re-apply strategies bounced before the extent approach).Fix: a 1px absolute scroll-extent sentinel node pinned at the pre-drag content extent, injected at
onDragStart(scrolled drags only), removed inprocessDragEndbefore the layout restore. Max-scroll never shrinks → the clamp becomes impossible → headers and cells stay on the same scroll truth through every switch and switch-back. The existing drop re-sync (#12938item 1) is unchanged and still owns the post-restore frame.The exposure net (per the operator's directive: "things like these MUST get exposed in tests"):
GridHeaderCellRectSync.spec.mjs— whitebox-e2e on the lockedColumns fixture; scrolls the centre to max, drags the last centre column back and forth across neighbours for two full passes, and at EVERY hold point + after every drop asserts each visible header's rect (x, width) sits pixel-exact on its first-row cell's rect. Layer-agnostic: any header↔cell drift (scroll clamps, width churn, stale transforms) fails with the offending column, offset, and a per-layer scroll-context dump. Real mouse gestures by design — the failure class lives in main-thread reflow, synthetic worker events would green-wash it.Evidence: L3 (net RED pre-fix at commit
23bf6243fwith the 54px uniform-offset signature — dumps on the ticket — GREEN post-fix ×5 consecutive; sibling drag e2e 4/4; units 57/57) → L4 residual (operator re-check of the original devindex gesture, batched into the gate session). Residual: L4 devindex re-verify [#12955 → #12883 gate session].Deltas from ticket
Updatedbutton (same offset, not a separate cls bug); falsifier 2 stays parked on the ticket.Test Evidence
npx playwright test GridHeaderCellRectSync -c <e2e config>→ RED pre-fix (uniform 54px header-strip offset atpass 1: mid-drag, all widths correct) → GREEN ×5 consecutive post-fix (one pre-hardening flake disclosed; settle margins raised in2156540c0).GridColumnCrossBodyDnD+GridColumnOverdragScroll→ 4/4 (cross-region re-home + overdrag unaffected; overdrag's intentional mid-drag scrolling composes with the sentinel — extent pinning never blocks scrolling, only extent collapse).npm run test-unit -- test/playwright/unit/grid test/playwright/unit/draggable→ 57/57.Post-Merge Validation
Commits
23bf6243f— the exposure net (red at this commit, by design — it documents the defect)4811915dc— the extent-sentinel fix (net goes green here)2156540c0— net settle-margin hardening