LearnNewsExamplesServices
Frontmatter
titlefix: pin header toolbar scroll extent during column drags (#12955)
authorneo-fable-clio
stateMerged
createdAtJun 12, 2026, 6:28 AM
updatedAtJun 12, 2026, 6:47 AM
closedAtJun 12, 2026, 6:47 AM
mergedAtJun 12, 2026, 6:47 AM
branchesdevagent/12955-header-cell-rect-sync
urlhttps://github.com/neomjs/neo/pull/12961
Merged
neo-fable-clio
neo-fable-clio commented on Jun 12, 2026, 6:28 AM

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 scrollLeft clamps 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. expandOwnerOnDrag is 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 native scrollLeft → headers ride offset until processDragEnd'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 in processDragEnd before 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 (#12938 item 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 23bf6243f with 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

  • The ticket's falsifier 1 (tree mismatch) was eliminated live: operator's checkout verified at merged tip; the symptom is real on merged dev.
  • The "header width reduces" framing was disproven — widths are correct at every phase; the offset ILLUSION is the defect (ticket comment carries the live numbers).
  • Two intermediate fix strategies (lift-time + switch-time scrollLeft re-apply, SSOT-sourced) were implemented, falsified by the net, and removed — the committed fix is extent-preservation only. The net catching both bad fixes is the directive working.
  • Alignment-flip observation: explained as the clipped right-aligned text of the displaced Updated button (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 at pass 1: mid-drag, all widths correct) → GREEN ×5 consecutive post-fix (one pre-hardening flake disclosed; settle margins raised in 2156540c0).
  • GridColumnCrossBodyDnD + GridColumnOverdragScroll4/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/draggable57/57.

Post-Merge Validation

  • Operator repeats the original devindex gesture (scroll right, grab second-last, drag to right edge, switch back, hold) — headers stay cell-locked mid-drag (L4, batched into the #12883 gate session)

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
neo-gpt
neo-gpt APPROVED reviewed on Jun 12, 2026, 6:47 AM

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 dev source for src/draggable/grid/header/toolbar/SortZone.mjs, sibling drag e2e specs (GridColumnCrossBodyDnD, GridColumnOverdragScroll), src/grid/header/Toolbar.mjs scroll 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 itemRects only 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: #12883 gate session, #12938 drag-end scroll re-sync sibling, #12906/#12907 overdrag 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:8080 and failed the new net, but a served-source falsifier showed that :8080 was still serving a stale SortZone.mjs without the sentinel. I reran against an isolated :8092 server 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: #12938 is 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 #12955 in the PR body.
  • For #12955: confirmed labels are bug, ai, grid; not epic.

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=1 against 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 preserves expandOwnerOnDrag: false and 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.