fix(datagrid): keep the column window with the viewport on a wide result (#2381)

[datlechin]

Fixes #2381.

Root cause

DataGridColumnPool.applyColumnWindow picks the mounted column range with ColumnWindowResolver, which models the whole un-windowed column run measured from data column 0. The live viewport has to be rebased into that model first, and the rebase subtracted the width of every non-hidden column sitting ahead of the first data column.

The leading spacer is one of those columns. Its width is, by construction, exactly the width of the columns the window left out, and the resolver's model already carries that width. Subtracting it a second time makes the rebased offset converge on roughly half the real offset, so the window walks left while the reader scrolls right.

One mechanism, all three reported symptoms: the window alternates between two ranges and re-mounts columns on every scroll event (the flicker), it covers only part of the viewport (the empty gap in the screenshot), and once it parks far enough away the resolver's keep-current branch holds it there, so the grid paints nothing until the table is reopened.

Horizontal windowing shipped in 752019a (#2254), which is in v0.67.0, the version the report is filed against.

Measured

A standalone AppKit harness drives the shipping decision code over a 100-column result at a 1200pt viewport with the grid's own settings (style = .plain, intercellSpacing = (1, 0)), then asks NSTableView.rect(ofColumn:) what actually paints:

	shipping 0.67.x	this change
scroll positions rendering nothing	36 of 74	0
worst unpainted viewport	1200pt of 1200pt	0pt
positions where the window never settles	45 of 74	0

Two supporting measurements the fix rests on: a hidden NSTableColumn contributes exactly zero width and zero intercell spacing, so the spacers reproduce the document width exactly; and rect(ofColumn:) returns NSZeroRect for a hidden column, which is why scrollColumnToVisible on an unmounted column scrolls to x = 0 (measured: clip origin 900 to 0 hidden, 900 to 1212 mounted).

Performance

The bug was arithmetic, but it had a price. An alternating window unmounts one set of columns and mounts another on almost every scroll event, so fixing it does strictly less work. Same scroll sweep through both variants of the real decision, on a view-based NSTableView with a delegate building real cell views:

100 columns, 30 rows, full-width sweep	before	after
re-windows	23	10
column mount and unmount toggles	509	155
main-thread time spent windowing	962ms	378ms
worst single frame	56ms	42ms
frames with an unpainted gap	38 of 60	0 of 60
frames painting nothing at all	29 of 60	0 of 60

500 columns	before	after
re-windows	160	57
column mount and unmount toggles	3,558	954
main-thread time spent windowing	7,628ms	3,374ms
frames painting nothing at all	289 of 320	0 of 320

Two numbers move the other way and should. Cell views built rises (2,370 to 3,090 at 100 columns) and the 500-column worst frame goes from 66ms to 76ms, because the fixed build renders the columns the broken one skipped by painting nothing at all. Building the content is the job.

The window itself still earns its place at every size, which settles whether it should be switched off for narrower results:

columns	worst frame, every column attached	worst frame, windowed
25	32ms	25ms
50	66ms	37ms
100	163ms	47ms
200	470ms	63ms
500	2,223ms	137ms

Windowing costs more total work across a sweep and buys a bounded worst frame, which is what a reader feels. A column-count gate would trade a 47ms hitch at 100 columns for a 163ms one.

The rest of the same regression

#2254 also put two spacer columns into tableView.tableColumns and changed what isHidden means there, and four call sites still read the old shape. They all bite on exactly the wide results this change makes usable, so they ship with it.

• Find, the arrow keys and the inline editor could not reach a column scrolled off the side. rect(ofColumn:) and frameOfCell(atColumn:row:) are both empty for an unmounted column, so Find scrolled to the document origin instead of the match, and the inline editor's own empty-frame guard opened nothing. There is now a single chokepoint, TableViewCoordinator.scrollColumnToVisible(tableColumnIndex:), that mounts the column before anything reads its frame. It re-centres a bounded window rather than stretching the mounted range out to the target, which would mount every column in between.
• DataGridView.firstDataTableColumnIndex was a hardcoded 1, which is now the leading spacer, and isDataTableColumn accepted the trailing spacer for the same reason. The cell cursor was seeded onto a spacer whenever the selection moved without a click, so on any table Down then Return did nothing while the Edit menu item still validated as enabled. Both constants are gone; presentsColumn is now the single answer to "is this position one of the columns the result shows", and the pool grew firstPresentedColumnIndex / lastPresentedColumnIndex / nextPresentedColumnIndex / previousPresentedColumnIndex beside it.
• Tab out of a row's last cell and Shift+Tab out of its first were swallowed, because both wrapped onto a fixed end of tableColumns, which is a spacer at one end and a spacer or a surplus pool slot at the other.
• Size All Columns to Fit left the far columns unreachable. It resizes every presented column, deliberately including the unmounted ones, but never dropped the window, so the spacers kept standing in at the pre-resize total and the document came up short. It now invalidates and re-resolves, the shape applyAccessoryWidthChanges already used.

Tests

New deterministic unit tests on a real NSTableView inside a real NSScrollView:

• the mounted columns cover the viewport at every horizontal scroll offset, measured through rect(ofColumn:) rather than through the resolver, so the test cannot agree with the bug;

• resolving again at one offset settles instead of alternating;

• the document keeps its width at every window position, and across an on-demand mount;

• the last column is mounted at the far right;

• an unmounted column can be mounted on demand, without mounting everything in between, and a user-hidden one never is;

• the ends of the data run skip the spacers, including a zero-column grid (the create-table state) and a single-column one;

• resizing unmounted columns restores the full document width;

• a selection with no cell cursor seeds one on a data column rather than a spacer.

• Return opens the editor on the column a keyboard selection seeded, driven through tableViewSelectionDidChange and then insertNewline(_:) on a real KeyHandlingTableView inside an NSWindow, so the whole keystroke is covered rather than just the seed.

Four of these fail on the shipped code and pass here, checked by reverting each fix in place: the viewport-coverage sweep and the settle test fail on the old leadingChrome, and both the cursor-seed and the Return-path test fail on the old 1.

DataGridColumnPoolTests also now builds its table view the way the grid does (style = .plain, intercellSpacing = (1, 0)); it was measuring a geometry the app never has. DataGridRowViewCopyTests builds its columns through the pool rather than attaching them by hand, so the positions it focuses are the ones the grid really has.

No UI automation. Seven attempts, and the environment is healthy: TableProLaunchUITests passes 4 of 4 as a control. Two real harness defects turned up and are worth recording. A normalized-offset click on a grid row element lands at the window's top-left, on its close button, because a row reports an empty frame there, and every later query then fails as though the feature were broken. And filter(\.isSelected) on [XCUIElement] does not compile under Swift 6. With both fixed a row does select, but Return still produced no textViews element against the sample database and the cause was never established, so nothing red is being pushed. The unit test above covers the same flow in 0.06s and fails on the pre-fix seed. The scrolling behaviour has no deterministic XCUITest form at all: the assertion is about which columns paint, which is what rect(ofColumn:) answers directly.

Verified

• verify.sh build PASS
• verify.sh test PASS, 101 cases across the six affected suites
• verify.sh lint TablePro TableProTests PASS, 0 violations
• Reproduced and confirmed in the shipping app, not only in a harness: a Debug build in an isolated sandbox opened on a 100-column SQLite table, swept across 95% of its width in 200 steps with every frame captured and scored for the widest unpainted band. Worst frame before, 2,238px unpainted, which is 100% of the grid, across 296 frames. Worst frame after, 40px, one column gutter, across 254. On the broken build the grid goes completely blank mid-scroll while the row striping is still drawn and the status bar still reports "1-25 of 25 rows".
• /code-review high found three defects in the first draft, all fixed here: an on-demand mount that stretched the window to the target and materialised every column in between (measured 848ms and 3,081 cell views at 100 columns, 4.8s at 500), a cursor seed that still could not open an editor once the grid was scrolled sideways, and a drag clamp that changed behaviour rather than restoring it.

A note for anyone reproducing this by hand: a single jump to a scroll offset does not show the bug. The rebase error compounds across successive slides, so it takes a sweep of many small steps, which is what a trackpad produces and what one drag of the scroller does not.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[datlechin]

Follow-up: the scroll lag, measured live, plus three loose ends closed

The reported gap, captured frame by frame

The earlier numbers came from a standalone AppKit harness. This is the shipping app: a Debug build in an isolated sandbox, opened on a 100-column SQLite table, scrolled across 95% of its width in 200 steps while every frame was captured and scored for the widest unpainted band inside the grid.

	worst frame	frames captured
shipped 0.67.x	2238px unpainted, 100% of the grid	296
this branch	40px, one column gutter	254

On the shipped build the grid goes completely blank mid-scroll, row striping still drawn, status bar still reporting "1-25 of 25 rows". That is the reporter's "columns eventually stop loading", reproduced.

Note for anyone repeating this: a single jump to the same scroll offset does not reproduce it. The rebase error compounds across successive slides, so it takes a sweep of many small steps, which is what a person does with a trackpad.

Does the window keep up with a fast fling?

The arithmetic fix puts the window in the right place; this is the other half of the reported symptom. Measured on a real view-based NSTableView with a delegate building real cell views, timing each re-window including the layout pass:

	100 columns	500 columns
cost per re-window	median 32ms, worst 35ms	median 47ms, worst 77ms
frames at 16.7ms	2.1	4.6
viewport travel during the worst one, at 240pt/frame	498pt	1108pt
overscan headroom each side	~1450pt	~1450pt

At the reported size the headroom is about 3x the distance the viewport can travel during a stall, so the gap is gone. At 500 columns the margin is 1.09x and a hard fling can still show a transient band that fills when the scroll stops. That is inherent to mount-on-demand with synchronous cell views, not a tuning problem: sweeping overscan and slideMargin makes it strictly worse, because the per-re-window cost grows faster than the headroom it buys.

overscan	margin at 100 columns	margin at 500
10 (current)	2.92x	1.09x
15	2.29x	0.62x
20	1.90x	0.39x
30	1.66x	0.34x

Does windowing earn its place at 100 columns?

Yes, and this refutes the idea of switching it off below some column count. Same table, scrolled the full width both ways:

columns	worst frame, every column attached	worst frame, windowed
25	32ms	25ms
50	66ms	37ms
100	163ms	47ms
200	470ms	63ms
500	2223ms	137ms

Windowing costs more total work over a full sweep (440ms against 184ms at 100 columns) and buys a bounded worst frame, which is the thing a reader feels. A count gate would trade a 47ms hitch for a 163ms one.

Also in this branch now

• KeyHandlingTableView.mouseDown classified a clicked column as data by testing identifier != rowNumberIdentifier, which is true for the spacers as well. The dataColumnIndex lookup right after it made the outcome correct, and a spacer always occupies the scrolled-past region so it cannot be clicked, so there is no behaviour change: it now asks presentsColumn, which makes that rule hold everywhere rather than nearly everywhere. No CHANGELOG entry, because nothing observable changes.
• The Return path is now covered end to end by a unit test that drives tableViewSelectionDidChange and then insertNewline(_:) on a real KeyHandlingTableView inside an NSWindow, asserting the overlay editor opens. It fails on the pre-fix seed and runs in 0.06s.
• CLAUDE.md gains two entries in ### Invariants: the rebase counts chrome only, and no fixed position in tableColumns names a data column.

Still no XCUITest

Seven attempts. The environment is healthy, confirmed by running TableProLaunchUITests as a control (4/4). Two real harness defects were found and are recorded: a normalized-offset click on a grid row element lands at the window's top-left, on the close button, because the row reports an empty frame, which makes every later query fail as though the feature were broken; and filter(\.isSelected) does not compile under Swift 6. With both fixed a row does select, but Return still produced no textViews element against the sample database and the cause was not established, so nothing red is being pushed. The unit test above covers the same flow deterministically.