Conversation
…(Phase D) Cell edits, row inserts and removals now drive NSTableView through Delta dispatch directly, so the SwiftUI counter-driven reload path is redundant. Removes: - lastReapplyVersion (already dead) - lastReloadVersion field on TableViewCoordinator - versionChanged branch in reloadAndSyncSelection (delta path already covers this) - reloadVersion field from DataGridIdentity composition - consumeChangedRowIndices on AnyChangeManager / DataChangeManager / StructureChangeManager - changedRowIndices accounting on PendingChanges and StructureChangeManager (14 insert calls and the consume/clear sites) To keep the modified-cell yellow marker correct without the SwiftUI rerun, commitCellEdit and TableViewCoordinator.applyDelta(.cellChanged / .cellsChanged) now call rebuildVisualStateCache before the controller dispatch. This was previously implicit via the versionChanged branch's second reload. Net -259 LOC. Build green, smoke-tested cell edit/undo, add-row, table switch.
datlechin
changed the title
… sweep
The bulk sed deletion of changedRowIndices.insert(...) lines in StructureChangeManager.deleteColumn / deleteIndex / deleteForeignKey left six empty blocks behind: three 'if let rowIndex = workingX.firstIndex(...) { }' blocks where the binding is now unused, and three 'if let rowIndex { for i in rowIndex..<workingX.count { } }' loops with empty bodies. Removed all six.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0bd7e81bb9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let currentIdentity = DataGridIdentity( | ||
| reloadVersion: changeManager.reloadVersion, | ||
| schemaVersion: schemaVersion, | ||
| metadataVersion: metadataVersion, | ||
| paginationVersion: paginationVersion, |
There was a problem hiding this comment.
Dropping reloadVersion from DataGridIdentity means updateNSView now early-returns for changes that do not alter row/column counts or metadata, even when the underlying ChangeManaging state changed. In this repo, schema undo/redo paths (for example CreateTableGridDelegate.dataGridUndo/Redo calling StructureChangeManager.undo/redo) do not dispatch a Delta, and many of those edits keep the same row/column shape; after this commit, those updates can leave stale cell values/row visual state on screen until a later full reload (query rerun, sort, tab switch).
Useful? React with 👍 / 👎.
1 participant
Summary
Phase D of the DataGrid refactor. Drops the SwiftUI counter-driven reload bridge that's now redundant — Phase C.2 made mutations dispatch
Deltadirectly toNSTableView, so the SwiftUIreloadVersionrerun was a duplicate reload pass.Net −259 LOC across 12 files, no behavior change.
What's removed
lastReapplyVersionfield onTableViewCoordinator(already dead, never read or written elsewhere)lastReloadVersionfield onTableViewCoordinatorversionChangedbranch inreloadAndSyncSelection(delta path already covers cell-edit reloads)reloadVersionfromDataGridIdentitycompositionconsumeChangedRowIndicesfromChangeManagingprotocol,AnyChangeManagerwrapper,DataChangeManager,StructureChangeManagerchangedRowIndicesfield onPendingChangesandStructureChangeManager, plus 14+insert(...)andremoveAll()call sitesWhy the visual-state cache still works
The previous
versionChangedbranch implicitly rebuiltrowVisualStateCacheon every cell edit via its second reload. With that gone,commitCellEditandTableViewCoordinator.applyDelta(.cellChanged / .cellsChanged)now callrebuildVisualStateCache()before the controller dispatch. The cache is guarded bylastVisualStateCacheVersion != changeManager.reloadVersion, so calling it liberally is cheap.What stays
reloadVersiononDataChangeManagerandStructureChangeManager— still used byrebuildVisualStateCache's version guard, and byCreateTableView's.onChangefor SQL preview regenerationlastIdentityearly-return inupdateNSView— still useful as a no-op short-circuit for non-data SwiftUI rerendersmetadataChangedandpaginationChangedbranches — no delta-dispatch equivalents yet (parking-lot for a future PR)Test plan
xcodebuild ... build)main)Follow-up parking lot (deferred)
lastIdentityearly-return entirely once the remaining branches (metadataChanged FK reload, paginationChanged scroll) are converted to delta dispatchmetadataChangedFK column reload with an explicit delta when FK metadata refreshescachedTableRows(mirrorstableRowsProvider()— two sources of truth)