From 7f25c4636cbb060e9dcf4a2cf3deda5723f32965 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ng=C3=B4=20Qu=E1=BB=91c=20=C4=90=E1=BA=A1t?= Date: Wed, 29 Apr 2026 11:58:54 +0700 Subject: [PATCH 1/2] refactor(datagrid): merge sortCache into coordinator.querySortCache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../Main/Child/MainEditorContentView.swift | 35 ++----------------- 1 file changed, 2 insertions(+), 33 deletions(-) diff --git a/TablePro/Views/Main/Child/MainEditorContentView.swift b/TablePro/Views/Main/Child/MainEditorContentView.swift index 6114d9b18..fd75ffc9a 100644 --- a/TablePro/Views/Main/Child/MainEditorContentView.swift +++ b/TablePro/Views/Main/Child/MainEditorContentView.swift @@ -10,17 +10,6 @@ import AppKit import CodeEditSourceEditor import SwiftUI -/// Cache for sorted query result rows to avoid re-sorting on every SwiftUI body evaluation. -/// Stores a permutation of `RowID` so the grid keeps the same display order even after -/// inserts and deletes mutate the underlying TableRows storage. -private struct SortedRowsCache { - let sortedIDs: [RowID] - let columnIndex: Int - let direction: SortDirection - let schemaVersion: Int -} - -/// Main editor content with tab bar and content switching struct MainEditorContentView: View { // MARK: - Dependencies @@ -58,10 +47,6 @@ struct MainEditorContentView: View { let onOffsetChange: (Int) -> Void let onPaginationGo: () -> Void - // MARK: - Sort Cache - - @State private var sortCache: [UUID: SortedRowsCache] = [:] - @State private var cachedChangeManager: AnyChangeManager? @State private var erDiagramViewModels: [UUID: ERDiagramViewModel] = [:] @State private var serverDashboardViewModels: [UUID: ServerDashboardViewModel] = [:] @@ -118,14 +103,7 @@ struct MainEditorContentView: View { favoriteDialogQuery = FavoriteDialogQuery(query: query) } .onChange(of: tabManager.tabStructureVersion) { _, _ in - let newIds = tabManager.tabIds - guard !sortCache.isEmpty || !erDiagramViewModels.isEmpty - || !serverDashboardViewModels.isEmpty else { - coordinator.cleanupSortCache(openTabIds: Set(newIds)) - return - } - let openTabIds = Set(newIds) - sortCache = sortCache.filter { openTabIds.contains($0.key) } + let openTabIds = Set(tabManager.tabIds) coordinator.cleanupSortCache(openTabIds: openTabIds) erDiagramViewModels = erDiagramViewModels.filter { openTabIds.contains($0.key) } serverDashboardViewModels = serverDashboardViewModels.filter { openTabIds.contains($0.key) } @@ -140,7 +118,6 @@ struct MainEditorContentView: View { refreshDataTabDelegateMutableRefs() coordinator.dataTabDelegate = dataTabDelegate coordinator.onTeardown = { [self] in - sortCache.removeAll() cachedChangeManager = nil coordinator.dataTabDelegate = nil } @@ -636,14 +613,6 @@ struct MainEditorContentView: View { return nil } - if let cached = sortCache[tab.id], - cached.columnIndex == (tab.sortState.columnIndex ?? -1), - cached.direction == tab.sortState.direction, - cached.schemaVersion == tab.schemaVersion - { - return cached.sortedIDs - } - let sortColumns = tab.sortState.columns let storageRows = resolvedRows.rows let sortedIndices = Array(storageRows.indices).sorted { idx1, idx2 in @@ -666,7 +635,7 @@ struct MainEditorContentView: View { } let sortedIDs = sortedIndices.map { storageRows[$0].id } - sortCache[tab.id] = SortedRowsCache( + coordinator.querySortCache[tab.id] = QuerySortCacheEntry( sortedIDs: sortedIDs, columnIndex: tab.sortState.columnIndex ?? -1, direction: tab.sortState.direction, From b64d0b45b528d88faf9aa540759c988a8a989ddc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ng=C3=B4=20Qu=E1=BB=91c=20=C4=90=E1=BA=A1t?= Date: Wed, 29 Apr 2026 12:06:20 +0700 Subject: [PATCH 2/2] test(datagrid): lock sort cache invalidation on row mutations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../Main/SortCacheInvalidationTests.swift | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 TableProTests/Views/Main/SortCacheInvalidationTests.swift diff --git a/TableProTests/Views/Main/SortCacheInvalidationTests.swift b/TableProTests/Views/Main/SortCacheInvalidationTests.swift new file mode 100644 index 000000000..69ebcd2ce --- /dev/null +++ b/TableProTests/Views/Main/SortCacheInvalidationTests.swift @@ -0,0 +1,98 @@ +// +// SortCacheInvalidationTests.swift +// TableProTests +// +// Locks the contract that row mutations invalidate querySortCache for the +// affected tab. Pre-merge, only the coordinator-side cache was invalidated; +// the view-side @State sortCache stayed stale, so a sorted small table +// returned out-of-date sortedIDs after add / undo / paste / delete. After +// the merge there is one cache and these tests guard the invalidation set. +// + +import Foundation +import Testing +@testable import TablePro + +@Suite("querySortCache invalidation on row mutations") +@MainActor +struct SortCacheInvalidationTests { + private func makeCoordinator() -> (MainContentCoordinator, QueryTabManager, UUID) { + let tabManager = QueryTabManager() + let coordinator = MainContentCoordinator( + connection: TestFixtures.makeConnection(), + tabManager: tabManager, + changeManager: DataChangeManager(), + filterStateManager: FilterStateManager(), + columnVisibilityManager: ColumnVisibilityManager(), + toolbarState: ConnectionToolbarState() + ) + tabManager.addTableTab(tableName: "users") + let tabIndex = tabManager.selectedTabIndex ?? 0 + tabManager.tabs[tabIndex].tableContext.isEditable = true + let tabId = tabManager.tabs[tabIndex].id + return (coordinator, tabManager, tabId) + } + + private func seedCache(_ coordinator: MainContentCoordinator, for tabId: UUID) { + coordinator.querySortCache[tabId] = QuerySortCacheEntry( + sortedIDs: [.existing(0), .existing(1), .existing(2)], + columnIndex: 1, + direction: .ascending, + schemaVersion: 0 + ) + } + + private func seedRows(_ coordinator: MainContentCoordinator, for tabId: UUID, count: Int) { + let columns = ["id", "name"] + let rows = (0..