refactor(datagrid): merge sortCache into coordinator.querySortCache#937
Merged
refactor(datagrid): merge sortCache into coordinator.querySortCache#937
Conversation
The view-side @State sortCache (in MainEditorContentView) and the coordinator's @ObservationIgnored querySortCache stored byte-identical SortedRowsCache / QuerySortCacheEntry structs keyed by tab ID. Two write paths fed them: sync inline sort (small tables, ≤1000 rows) wrote to the view-side cache; async background sort (large tables) wrote to the coordinator-side cache. The read in sortedIDsForTab checked both. The split was historical, not architectural. Both caches share the same lifecycle (window/coordinator) and the row-mutation invalidations in MainContentCoordinator+RowOperations only cleared querySortCache — meaning a sync-sorted small table would silently return a stale sortedIDs after add/delete/undo. Bug fixed as a side effect of the merge. Drops: - private struct SortedRowsCache - @State var sortCache from MainEditorContentView - The two-step cache cleanup in onChange(of: tabStructureVersion) and onTeardown - The sortCache lookup branch in sortedIDsForTab The sync sort path now writes directly to coordinator.querySortCache. cleanupSortCache and the row-mutation invalidations cover both paths uniformly. -23 LOC.
Four tests cover the four mutation pathways through the now-merged querySortCache: - addNewRow invalidates the cache (was view-side sortCache before merge — silently stale) - duplicateSelectedRow invalidates - deleteSelectedRows invalidates when physically removing inserted rows - deleteSelectedRows preserves the cache on soft-delete of existing rows (sortedIDs reference rows by ID, soft-delete only changes appearance, not order) Documents the soft-delete-preserves invariant that emerged during test writing — guards a future contributor from "helpfully" invalidating on every delete and breaking sort persistence.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Drops the duplicate sort cache.
MainEditorContentViewhad a@State var sortCache: [UUID: SortedRowsCache]for the sync inline-sort path (≤1000 rows).MainContentCoordinatorhad@ObservationIgnored var querySortCache: [UUID: QuerySortCacheEntry]for the async background-sort path. The two structs were byte-identical (sortedIDs: [RowID],columnIndex: Int,direction: SortDirection,schemaVersion: Int).Both writes now go to
coordinator.querySortCache. The view-side cache is gone.Stale-sort bug fixed as a side effect
Row-mutation invalidations in
MainContentCoordinator+RowOperations(six call sites:addNewRow,deleteSelectedRows,duplicateSelectedRow,undoInsertRow,handleUndoResult,pasteRows) calledquerySortCache.removeValue(forKey: tabId)but never touched the view-sidesortCache. A small (≤1000-row) sorted table that received an add / delete / undo / paste would return stalesortedIDsuntil the user clicked the column header again. After the merge, both paths share one cache and one invalidation set.Drops
private struct SortedRowsCache(was duplicate ofQuerySortCacheEntry)@State private var sortCacheinMainEditorContentViewonChange(of: tabStructureVersion)sortCache.removeAll()from theonTeardownhandlersortedIDsForTabcoordinator.cleanupSortCache(openTabIds:)andquerySortCache.removeAll()(called on coordinator teardown at MainContentCoordinator.swift:585) cover both paths uniformly.Net −33 / +2 LOC.
Test plan