fix(datagrid): follow the grid's display order in JSON result mode

[datlechin]

Fixes #2251.

JSON result mode showed rows in fetch order and resolved a Data-mode selection to the wrong rows. Two of the issue's own claims turned out to be wrong, and the part it did not mention is worse than the part it did.

Root cause

The display order, and the per-column value filter that produces it, lived on TableViewCoordinator, which is the Coordinator of the DataGridView NSViewRepresentable. activeGridDisplayIDs reached it through two weak hops.

Measured with a standalone SwiftUI harness, and reproduced independently by a second one:

COORDINATOR-INIT grid / MAKE-NSVIEW grid
--- switching to json mode (grid leaves the view tree) ---
UPDATE-NSVIEW grid
COORDINATOR-DEINIT grid          <- destroyed, despite strongly holding its NSTableView
--- switching back to data mode ---
COORDINATOR-INIT grid            <- a brand new one

So switching to JSON did not hide the display order, it deleted it, and every reader fell back to storage order.

What the issue got wrong

Sorting was never affected. handleSortStateChanged rebuilds the query with ORDER BY and re-runs it, so a sorted result arrives already in display order. DataGridView.sortedIDs was passed nil by its only production caller and omitted by every other one, so TableViewCoordinator.sortedIDs was nil in the shipping app. The per-column value filter was the only production source of a display order that differs from storage order.

The blast radius was not just JSON. GridSelectionOwner.resolve(.table, .json) is .dataGrid, so the row inspector is live in JSON mode and resolved the same dead reference. It showed the wrong row, and MainContentCoordinator+SidebarSave wrote an edit built from the wrong row's values.

The filter itself was lost on any switch away from Data mode and on any tab switch.

The fix

GridValueFilterState moves onto QueryTab, next to sortState, columnLayout and chartConfiguration, and is threaded into DataGridView as a binding exactly like those. The order becomes a pure function, GridDisplayOrderResolver.resolve(tableRows:valueFilter:displayFormats:databaseType:), memoized per tab in MainContentCoordinator on dataRevision plus the filter plus the formats, using the same stamped-cache shape as the displayFormatsCache sitting beside it. A row mutation ticks dataRevision, so the memo misses and the order cannot go stale.

The patch alternative, keeping a strong reference to the coordinator from outside the view tree, is rejected: dismantleNSView has already nil'd its cancellables while its NotificationCenter observers stay live, so a retained coordinator is a stale one. Apple frames it the same way ("Cleans up the presented AppKit view (and coordinator) in anticipation of their removal"), and CLAUDE.md's #2236 invariant says the same thing from the other side.

The grid keeps valueFilteredIDs as its own cache for the ~15 hot AppKit call sites, refreshed through the same resolver, so those still read a stored array. It cannot diverge from the model's copy because both are the same pure function of the same inputs.

displayFormats(for:) moves from MainEditorContentView to MainContentCoordinator, where its cache already lives, so the order resolves with no view mounted.

A grid with no owner, meaning the structure, create-table and inspector grids that also offer Filter Values…, keeps the filter on its own coordinator and behaves exactly as before.

The filter's lifetime

Giving the filter a real owner means giving it a real scope, which the grid's accidental death used to provide. It is cleared inside resetSelectionForNewResult, the existing chokepoint for a wholesale row replacement: a new query, a page turn, a refresh, or moving between a script's result sets. The filter stores the displayed strings the user picked out of the rows being replaced, so it means nothing once they are gone. That is also the guard against the failure DataGrip shipped as DBE-20501 (rated Critical there): a client-side filter that outlived its result set left later queries showing an empty grid with no visible cause.

Pending deletions

ResultJsonSerializer.serialize gains deletedDisplayIndices: Set<Int> = [] and skips those display positions. PendingChanges.deletedRowIndices is already keyed by display position in both delete paths, and SQLStatementGenerator builds the DELETE from RowChange.originalRow and the primary key rather than from the index, so no re-keying was needed.

The JSON view passes the set; the grid's Copy as JSON omits the parameter and is byte-identical. The two answer different questions: the JSON view shows what a Save would leave behind, and a JSON document has no way to mark a row, which is what made a delete look like it did nothing. Copy as JSON copies the rows you selected, like every other member of the grid's Copy as family, none of which consults pending state. A test pins that by asserting the omitted-parameter output equals the empty-set output.

The count line discloses the held-back rows so they never vanish silently: "9 of 10 rows, 1 marked for deletion". That number comes back from the serializer rather than being recomputed by the view, so it can never claim a row was held back that was not in the document to begin with. The string is in Localizable.xcstrings with all five translations.

The regression this had to avoid

deleteFilteredRows and duplicateFilteredRow wrote selectionState.indices without the selectionPointsTheGrid guard every other row command has. That was safe only by accident: activeGridDisplayIDs != nil implied "the grid is mounted, therefore Data mode". Once the order survives the unmount those branches run in JSON mode and collapse the document to one row, which is the bug #2250 just fixed. They now carry the guard, pinned by RowEditingCoordinatorJsonModeTests.

Also removed

TableViewCoordinator.sortedIDs and DataGridView.sortedIDs. They were nil in production and were a second, phantom source of display order wired into the very expression being redefined. RowVisualIndex's sortedIDs: label becomes displayIDs:, which is what it always meant.

Found by self-review, fixed before pushing

clearValueFilter cleared the tab but not the mounted grid's own mirror of the filter, and setActiveTableRows drives the grid's full reload on the very next line. The reload resolved the new rows through the old filter, and pruning that filter against the new columns wrote it back onto the tab that had just been cleared, permanently narrowing the new result. The clear now reaches the mounted coordinator synchronously, and newResultClearsTheFilterOnTheMountedGridToo pins it.

Verification

• Debug build PASS.
• 175 executed, 175 passed, 0 failed across GridDisplayOrderResolverTests, ResultJsonSerializerTests, ResultsJsonViewTests, RowEditingCoordinatorJsonModeTests, RowEditingCoordinatorCopyTests, TableViewCoordinatorValueFilterTests, TableViewCoordinatorRowCountCacheTests, TableViewCoordinatorDisplayCacheTests, DataGridUpdateSnapshotTests, DataGridRowViewCopyTests, RowVisualIndexTests, DisplayRowMappingTests, GridValueFilterStateTests, MainContentCoordinatorTabSwitchTests, CoordinatorColumnVisibilityTests, MainStatusBarLayoutTests, GridSelectionOwnerTests.
• swiftlint --strict clean over the app target and every changed test file, 0 violations.
• The SwiftUI coordinator-lifetime probe above, built and run against the Xcode-beta toolchain.

No UI automation

Driving this needs a live connection with a loaded table, a column value filter applied from a header popover, and a result-mode switch. TableProUITests has no deterministic fixture for that. The behaviour is pinned by the resolver's unit tests, the serializer tests, and the JSON-mode selection guard tests instead.

Screenshots

Not included: reproducing the before state needs a live database with a filtered result, and the visible difference is which rows a JSON document contains, which the serializer tests assert exactly rather than approximately.

Found while investigating, not fixed here

Two verified defects that predate this change and reproduce without it. This change makes both more likely to bite, because the filter now lives longer.

1. Undoing a cell edit made under a value filter writes into the wrong storage row. RowOperationsManager.applyUndoResult case .cellEdit calls tableRows.edit(row: rowIndex) with a display position, and TableRows.edit subscripts rows[] directly. The whole undo family has it. The tempting one-line fix, recording storageRow at DataGridView+CellCommit.swift:51, is wrong: RowVisualIndex.states is looked up by display row from rowViewForRow, so storage-keying cell edits alone would break the modified-cell tint and isRowDeleted.
2. Add Row and Paste write a storage index into selectionState.indices, which holds display positions (RowEditingCoordinator.swift:52, :328-334), unlike delete and duplicate which got a branch in Faulty Details for Json column #1837. Under a filter the new row is neither selected nor put into edit mode, and a later Save emits an INSERT of defaults plus an UPDATE matching nothing.
3. A pending deletion is keyed by the display position it was recorded at, with no epoch. Changing the value filter afterwards points that key at a different row, so the grid strikes through the wrong one and JSON now omits the wrong one. Both are the same aliasing, and the JSON view stays consistent with the grid, but excluding a row from a copyable document makes it more visible than the strikethrough did. Same root fix as (1): resolve display to storage at apply time, or re-key PendingChanges by RowID.

[mintlify]

Preview deployment for your docs. Learn more about Mintlify Previews.

Project	Status	Preview	Updated (UTC)
TablePro	🟢 Ready	View Preview	Aug 20, 2026, 1:14 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

[chatgpt-codex-connector]

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