Skip to content

Commit 7df36b6

Browse files
datlechinclaude
andauthored
refactor(perf): datagrid signal architecture and row data extraction (#919)
* refactor(perf): decouple persistence and inspector from tabs writes Phase A: persistence - Add tabStructureVersion on QueryTabManager, bumped only on tab add/remove/rename/replaceTabContent and on user query dispatch. - Replace .onChange(of: tabManager.tabs) with .onChange(of: tabManager.tabStructureVersion). Remove handleTabsChange; add handleStructureChange. - Remove per-keystroke saveLastQuery from queryTextBinding; remove now-unused saveLastQuery on TabPersistenceCoordinator. Phase B: inspector - Move updateSidebarEditState inside the 50ms inspector debounce so per-row-click N×M field configuration coalesces. - Drop coordinator.tableMetadata?.tableName from inspectorTrigger; metadata-driven inspector refresh now flows through the existing .task(id: tableName) modifier with an explicit scheduleInspectorUpdate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(perf): split resultVersion and route row ops through delegate deltas - Rename QueryTab.resultVersion to schemaVersion (column shape only). ResultSet keeps its own resultVersion for result-set tabs. - Add dataGridDidInsertRows/dataGridDidRemoveRows/dataGridDidReplaceAllRows to DataGridViewDelegate. DataTabGridDelegate forwards to the table view coordinator. - TableViewCoordinator gains applyInsertedRows/applyRemovedRows/applyFullReplace that call NSTableView.insertRows / removeRows / reloadData directly, refresh visual state, and invalidate identity so subsequent updateNSView passes don't short-circuit. - Row ops (add, delete, duplicate, undo, redo, paste) drive the new delta path instead of bumping a counter. RowOperationsManager.deleteSelectedRows now returns physicallyRemovedIndices so soft-deletes stay on the existing reloadVersion path while inserted-row removals get NSTableView animation. - querySortCache invalidates per tab on row-count changes. - Sort completion routes a full reload via the delegate; remove the redundant changeManager.reloadVersion bumps. - applyMultiStatementResults: keep the schemaVersion bump, drop the redundant reloadVersion bump. - Pin toggle no longer mutates a tab counter; TabDisplayState.resultSets is already @observable. - Wire MainContentCoordinator.dataTabDelegate weak ref in MainEditorContentView.onAppear / clear in teardown. * refactor(perf): broaden DataGridIdentity and stop re-wiring delegate per render - Extend DataGridIdentity with tabType, tableName, and primaryKeyColumns so updateNSView's identity guard catches all configuration fields that should drive a column rebuild. Drop the legacy initializer. - Move DataTabGridDelegate property assignments out of the dataGridView(tab:) body. Stable refs are set once in onAppear; isEditable / isView / tableName / safeModeLevel drive a focused refresh via .onChange. - DataGridConfiguration was already Equatable; no change. * refactor(perf): move row data out of QueryTab into RowDataStore QueryTab is now pure metadata. Row data (RowBuffer) lives in a RowDataStore keyed by tab.id, owned by MainContentCoordinator. Mutations to row data no longer flow through SwiftUI's @observable tracking on tabManager.tabs. - New RowDataStore (@mainactor, @observable, store @ObservationIgnored): buffer(for:), existingBuffer(for:), setBuffer, removeBuffer, evict, evictAll(except:), tearDown. - Drop rowBuffer and the resultRows/resultColumns/columnTypes/columnDefaults/columnForeignKeys/columnEnumValues/columnNullable proxy properties from QueryTab. Update == and init(from:) accordingly. - Migrate every read and write site (QueryHelpers, MultiStatement, LoadMore, RowOperations, Filtering, FKNavigation, Navigation, Discard, SaveChanges, SidebarActions, SidebarSave, TabSwitch, WindowLifecycle, CommandActions, plus inspector and editor views) to use coordinator.rowDataStore.buffer(for: tab.id). - replaceTabContent on QueryTabManager no longer resets a per-tab buffer; callers reset via setBuffer. - Eviction routes through rowDataStore.evictAll(except:); teardown clears the store. - Tests updated to read row data from the coordinator's rowDataStore. * fix(perf): correct cachedRowCount ordering and remove tabStructureVersion double-bump - DataGridCoordinator.applyInsertedRows / applyRemovedRows / applyFullReplace now call updateCache() before mutating the table view, so numberOfRows(in:) returns the post-mutation count when NSTableView synchronously validates. - QueryTabManager: drop the explicit tabStructureVersion bump from each add* method. The didSet on tabs already bumps when the ID array changes, and the explicit bump made every add count as 2 increments. replaceTabContent keeps its bump because same-id mutation does not change the ID array. * docs: remove internal datagrid refactor design doc * test(perf): cover refactor with unit tests, extract RowDeltaApplying protocol - New protocol RowDeltaApplying captures the row-delta surface (applyInsertedRows/applyRemovedRows/applyFullReplace) so DataTabGridDelegate can be tested with a fake without standing up an NSTableView. TableViewCoordinator conforms via empty extension. - DataTabGridDelegate.tableViewCoordinator switches to (any RowDeltaApplying)?. - Unit tests added: RowDataStore (10 cases), RowProviderCache (9 cases), QueryTabManager.tabStructureVersion semantics (10 cases), DataTabGridDelegate forwarding (4 cases). - Drop a stale comment in QueryTabManager.init() (CLAUDE.md no-comments rule). - Guard the selectedTabId .onChange against caching a provider over an evicted RowBuffer. * fix(perf): use existingBuffer in eviction guard, add reorder test - MainEditorContentView: the selectedTabId onChange and onAppear hooks called rowDataStore.buffer(for:) inside the eviction guard. buffer(for:) creates an empty RowBuffer on miss, so the guard could leak ghost entries (e.g. on first switch to a tab that has not loaded yet, or on view re-appear after teardown). Use existingBuffer(for:) so the guard only runs when a buffer actually exists, and skip cacheRowProvider for fresh / evicted tabs. - TabStructureVersionTests: cover drag-reorder (tabs.swapAt) so the didSet ID-array-change check is regression-tested. * refactor: drop dead ResultSet.resultVersion + saveLastQuery feature, fix tests Dead code removal: - ResultSet.resultVersion was write-only (3 sites set it, 0 sites read). Field gone, 5 dead writes removed in QueryHelpers, QueryParameters, MultiStatement, and two LoadMore paths. - TabPersistenceCoordinator.saveLastQuery was deleted in the persistence-decoupling phase but loadLastQuery was left orphaned. Drop loadLastQuery on the coordinator, drop saveLastQuery / loadLastQuery / lastQueryFileURL / lastQueryDirectory on TabDiskActor, drop the legacy lastQuery migration loop and key prefix. Per-keystroke crash recovery is gone; user-dispatched queries still bump tabStructureVersion so committed work persists. Test fixes: - TriggerStructTests: drop metadataTableName, rename resultVersion -> schemaVersion. Old signature was broken by phases B and C. - DataGridIdentityTests: convert hiddenColumns: argument to configuration: DataGridConfiguration. Phase D added tabType / tableName / primaryKeyColumns to the identity through the configuration init. - CommandActionsDispatchTests: drop selectedRowIndices binding, pass coordinator.selectionState. - MainStatusBarLayoutTests: pass StatusBarSnapshot instead of QueryTab. - SaveCompletionTests: drop selectedRowIndices inout from row op calls; assert on coordinator.selectionState.indices instead. - RowOperationsManagerTests: deleteSelectedRows now returns DeleteRowsResult; assert on .nextRowToSelect. - AnyChangeManagerTests: drop dataManager: / structureManager: argument labels (single any ChangeManaging init), and switch the changes property to rowChanges. - TabDiskActorTests + TabPersistenceCoordinatorTests: drop the lastQuery round-trip / nil / empty / whitespace / 500KB-cap tests since the feature is gone. * test(perf): cover physicallyRemovedIndices, drop stale TabDiskActor doc - Add 4 RowOperationsManagerTests cases for the DeleteRowsResult.physicallyRemovedIndices contract: empty selection returns empty, deleting only existing rows leaves it empty (soft-delete via change manager), deleting inserted rows reports indices descending, mixed selection reports only the inserted indices. - Drop the stale 'Last-query strings are stored in a sibling directory' doc comment on TabDiskActor; the directory and feature are gone. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 6dd9e44 commit 7df36b6

52 files changed

Lines changed: 1279 additions & 721 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2424
- OpenSSL shared as dylib across app and plugins, saving ~15MB in bundle size
2525
- Data grid uses single cell reuse identifier with typed stored properties instead of 3 identifiers and viewWithTag
2626
- Boolean dropdown menu includes Set NULL option for nullable columns
27+
- Tab persistence triggers on a structural counter, not on every tabs write. Cell edits, row mutations, and per-keystroke query text no longer invoke disk I/O.
28+
- Inspector sidebar edit state runs inside the existing 50ms debounce instead of synchronously per row click.
29+
- Row add, delete, duplicate, undo, redo, and paste drive NSTableView insertRows / removeRows directly through the data grid delegate. SwiftUI no longer re-evaluates the editor view tree on row mutations.
30+
- QueryTab.resultVersion split: schemaVersion (column shape) on QueryTab, row mutations through delegate deltas, sort completion through a single delegate replace call. Pin toggle, sort completion, and applyMultiStatementResults no longer fan out a redundant reload signal.
31+
- Row data lives in a per-coordinator RowDataStore keyed by tab.id rather than on QueryTab itself, so SwiftUI's @Observable tracking on tabManager.tabs no longer fires for row writes.
32+
- DataGridConfiguration is Equatable; DataGridIdentity covers tabType, tableName, and primaryKeyColumns so updateNSView short-circuits when nothing structural changed. DataTabGridDelegate properties are wired in onAppear / onChange instead of in the body.
2733

2834
### Fixed
2935

TablePro/Core/Services/Infrastructure/TabPersistenceCoordinator.swift

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -120,21 +120,6 @@ internal final class TabPersistenceCoordinator {
120120
)
121121
}
122122

123-
// MARK: - Last Query
124-
125-
/// Save the editor's last query text for this connection.
126-
internal func saveLastQuery(_ query: String) {
127-
let connId = connectionId
128-
Task {
129-
await TabDiskActor.shared.saveLastQuery(query, for: connId)
130-
}
131-
}
132-
133-
/// Load the editor's last query text for this connection.
134-
internal func loadLastQuery() async -> String? {
135-
await TabDiskActor.shared.loadLastQuery(for: connectionId)
136-
}
137-
138123
// MARK: - Private
139124

140125
private func convertToPersistedTab(_ tab: QueryTab) -> PersistedTab {
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import Foundation
2+
3+
@MainActor
4+
@Observable
5+
final class RowDataStore {
6+
@ObservationIgnored private var store: [UUID: RowBuffer] = [:]
7+
8+
func buffer(for tabId: UUID) -> RowBuffer {
9+
if let existing = store[tabId] {
10+
return existing
11+
}
12+
let buffer = RowBuffer()
13+
store[tabId] = buffer
14+
return buffer
15+
}
16+
17+
func existingBuffer(for tabId: UUID) -> RowBuffer? {
18+
store[tabId]
19+
}
20+
21+
func setBuffer(_ buffer: RowBuffer, for tabId: UUID) {
22+
store[tabId] = buffer
23+
}
24+
25+
func removeBuffer(for tabId: UUID) {
26+
store.removeValue(forKey: tabId)
27+
}
28+
29+
func evict(for tabId: UUID) {
30+
store[tabId]?.evict()
31+
}
32+
33+
func evictAll(except activeTabId: UUID?) {
34+
for (id, buffer) in store where id != activeTabId {
35+
if !buffer.rows.isEmpty && !buffer.isEvicted {
36+
buffer.evict()
37+
}
38+
}
39+
}
40+
41+
func tearDown() {
42+
store.removeAll()
43+
}
44+
}

TablePro/Core/Services/Query/RowOperationsManager.swift

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -91,16 +91,18 @@ final class RowOperationsManager {
9191

9292
// MARK: - Delete Rows
9393

94-
/// Delete selected rows
95-
/// - Parameters:
96-
/// - selectedIndices: Indices of rows to delete
97-
/// - resultRows: Current rows (will be mutated)
98-
/// - Returns: Next row index to select after deletion, or -1 if no rows left
94+
struct DeleteRowsResult {
95+
let nextRowToSelect: Int
96+
let physicallyRemovedIndices: [Int]
97+
}
98+
9999
func deleteSelectedRows(
100100
selectedIndices: Set<Int>,
101101
resultRows: inout [[String?]]
102-
) -> Int {
103-
guard !selectedIndices.isEmpty else { return -1 }
102+
) -> DeleteRowsResult {
103+
guard !selectedIndices.isEmpty else {
104+
return DeleteRowsResult(nextRowToSelect: -1, physicallyRemovedIndices: [])
105+
}
104106

105107
var insertedRowsToDelete: [Int] = []
106108
var existingRowsToDelete: [(rowIndex: Int, originalRow: [String?])] = []
@@ -118,40 +120,40 @@ final class RowOperationsManager {
118120
}
119121
}
120122

121-
// Process inserted rows deletion
122-
if !insertedRowsToDelete.isEmpty {
123-
let sortedInsertedRows = insertedRowsToDelete.sorted(by: >)
123+
let sortedInsertedRows = insertedRowsToDelete.sorted(by: >)
124124

125-
// Remove from resultRows first (descending order)
125+
if !sortedInsertedRows.isEmpty {
126126
for rowIndex in sortedInsertedRows {
127127
guard rowIndex < resultRows.count else { continue }
128128
resultRows.remove(at: rowIndex)
129129
}
130-
131-
// Update changeManager for ALL deleted inserted rows at once
132130
changeManager.undoBatchRowInsertion(rowIndices: sortedInsertedRows)
133131
}
134132

135-
// Record batch deletion for existing rows (single undo action for all rows)
136133
if !existingRowsToDelete.isEmpty {
137134
changeManager.recordBatchRowDeletion(rows: existingRowsToDelete)
138135
}
139136

140-
// Calculate next row selection, accounting for deleted inserted rows
141137
let totalRows = resultRows.count
142-
let rowsDeleted = insertedRowsToDelete.count
138+
let rowsDeleted = sortedInsertedRows.count
143139
let adjustedMaxRow = maxSelectedRow - rowsDeleted
144-
let adjustedMinRow = minSelectedRow - insertedRowsToDelete.count(where: { $0 < minSelectedRow })
140+
let adjustedMinRow = minSelectedRow - sortedInsertedRows.count(where: { $0 < minSelectedRow })
145141

142+
let nextRow: Int
146143
if adjustedMaxRow + 1 < totalRows {
147-
return min(adjustedMaxRow + 1, totalRows - 1)
144+
nextRow = min(adjustedMaxRow + 1, totalRows - 1)
148145
} else if adjustedMinRow > 0 {
149-
return adjustedMinRow - 1
146+
nextRow = adjustedMinRow - 1
150147
} else if totalRows > 0 {
151-
return 0
148+
nextRow = 0
152149
} else {
153-
return -1
150+
nextRow = -1
154151
}
152+
153+
return DeleteRowsResult(
154+
nextRowToSelect: nextRow,
155+
physicallyRemovedIndices: sortedInsertedRows
156+
)
155157
}
156158

157159
// MARK: - Undo/Redo

TablePro/Core/Storage/TabDiskActor.swift

Lines changed: 11 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,6 @@ internal struct TabDiskState: Codable {
2020
///
2121
/// Data is stored as individual JSON files per connection in:
2222
/// `~/Library/Application Support/TablePro/TabState/`
23-
///
24-
/// Last-query strings are stored in a sibling directory:
25-
/// `~/Library/Application Support/TablePro/LastQuery/`
2623
internal actor TabDiskActor {
2724
internal static let shared = TabDiskActor()
2825

@@ -31,39 +28,26 @@ internal actor TabDiskActor {
3128
// MARK: - Legacy UserDefaults Keys (for migration)
3229

3330
private static let legacyTabStateKeyPrefix = "com.TablePro.tabs."
34-
private static let legacyLastQueryKeyPrefix = "com.TablePro.lastquery."
3531
private static let migrationCompleteKey = "com.TablePro.tabStateMigrationComplete"
3632

3733
// MARK: - File Storage
3834

3935
private let tabStateDirectory: URL
40-
private let lastQueryDirectory: URL
4136
private let encoder: JSONEncoder
4237
private let decoder: JSONDecoder
4338

4439
private init() {
45-
tabStateDirectory = Self.resolvedTabStateDirectory()
46-
47-
let baseDirectory = tabStateDirectory.deletingLastPathComponent()
48-
lastQueryDirectory = baseDirectory.appendingPathComponent("LastQuery", isDirectory: true)
49-
40+
let directory = Self.resolvedTabStateDirectory()
41+
tabStateDirectory = directory
5042
encoder = JSONEncoder()
5143
decoder = JSONDecoder()
5244

53-
// Directory creation and migration run synchronously at init.
54-
// Safe because init is the only caller and runs before any concurrent access.
55-
let fm = FileManager.default
56-
for directory in [tabStateDirectory, lastQueryDirectory] {
57-
do {
58-
try fm.createDirectory(at: directory, withIntermediateDirectories: true)
59-
} catch {
60-
Self.logger.error("Failed to create directory \(directory.path): \(error.localizedDescription)")
61-
}
45+
do {
46+
try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true)
47+
} catch {
48+
Self.logger.error("Failed to create directory \(directory.path): \(error.localizedDescription)")
6249
}
63-
Self.performMigrationIfNeeded(
64-
tabStateDirectory: tabStateDirectory,
65-
lastQueryDirectory: lastQueryDirectory
66-
)
50+
Self.performMigrationIfNeeded(tabStateDirectory: directory)
6751
}
6852

6953
// MARK: - Public API
@@ -111,52 +95,6 @@ internal actor TabDiskActor {
11195
}
11296
}
11397

114-
/// Save the last query text for a connection. Skips if query exceeds 500KB.
115-
internal func saveLastQuery(_ query: String, for connectionId: UUID) {
116-
guard (query as NSString).length < TabQueryContent.maxPersistableQuerySize else { return }
117-
118-
let fileURL = lastQueryFileURL(for: connectionId)
119-
let trimmed = query.trimmingCharacters(in: .whitespacesAndNewlines)
120-
121-
if trimmed.isEmpty {
122-
if FileManager.default.fileExists(atPath: fileURL.path) {
123-
do {
124-
try FileManager.default.removeItem(at: fileURL)
125-
} catch {
126-
Self.logger.error(
127-
"Failed to remove last query for \(connectionId): \(error.localizedDescription)"
128-
)
129-
}
130-
}
131-
} else {
132-
do {
133-
let data = Data(trimmed.utf8)
134-
try data.write(to: fileURL, options: .atomic)
135-
} catch {
136-
Self.logger.error(
137-
"Failed to save last query for \(connectionId): \(error.localizedDescription)"
138-
)
139-
}
140-
}
141-
}
142-
143-
/// Load the last query text for a connection.
144-
internal func loadLastQuery(for connectionId: UUID) -> String? {
145-
let fileURL = lastQueryFileURL(for: connectionId)
146-
147-
guard FileManager.default.fileExists(atPath: fileURL.path) else {
148-
return nil
149-
}
150-
151-
do {
152-
let data = try Data(contentsOf: fileURL)
153-
return String(data: data, encoding: .utf8)
154-
} catch {
155-
Self.logger.error("Failed to load last query for \(connectionId): \(error.localizedDescription)")
156-
return nil
157-
}
158-
}
159-
16098
/// List all connection IDs that have saved tab state on disk.
16199
internal func connectionIdsWithSavedState() -> [UUID] {
162100
let fm = FileManager.default
@@ -217,28 +155,22 @@ internal actor TabDiskActor {
217155
tabStateDirectory.appendingPathComponent("\(connectionId.uuidString).json")
218156
}
219157

220-
private func lastQueryFileURL(for connectionId: UUID) -> URL {
221-
lastQueryDirectory.appendingPathComponent("\(connectionId.uuidString).txt")
222-
}
223-
224158
// MARK: - Migration from UserDefaults
225159

226-
/// One-time migration: reads existing tab state and last-query data from UserDefaults,
160+
/// One-time migration: reads existing tab state from UserDefaults,
227161
/// writes it to file storage, then clears the old UserDefaults keys.
228162
/// This is a static method to avoid actor-isolation issues during init.
229-
private static func performMigrationIfNeeded(tabStateDirectory: URL, lastQueryDirectory: URL) {
163+
private static func performMigrationIfNeeded(tabStateDirectory: URL) {
230164
let defaults = UserDefaults.standard
231165

232166
guard !defaults.bool(forKey: migrationCompleteKey) else { return }
233167

234168
logger.trace("Starting one-time migration of tab state from UserDefaults to file storage")
235169

236170
var migratedTabStates = 0
237-
var migratedLastQueries = 0
238171

239172
let allKeys = defaults.dictionaryRepresentation().keys
240173
let tabStateKeys = allKeys.filter { $0.hasPrefix(legacyTabStateKeyPrefix) }
241-
let lastQueryKeys = allKeys.filter { $0.hasPrefix(legacyLastQueryKeyPrefix) }
242174

243175
for key in tabStateKeys {
244176
let uuidString = String(key.dropFirst(legacyTabStateKeyPrefix.count))
@@ -255,28 +187,10 @@ internal actor TabDiskActor {
255187
}
256188
}
257189

258-
for key in lastQueryKeys {
259-
let uuidString = String(key.dropFirst(legacyLastQueryKeyPrefix.count))
260-
guard let connectionId = UUID(uuidString: uuidString),
261-
let query = defaults.string(forKey: key) else { continue }
262-
263-
let fileURL = lastQueryDirectory.appendingPathComponent("\(connectionId.uuidString).txt")
264-
do {
265-
let data = Data(query.utf8)
266-
try data.write(to: fileURL, options: .atomic)
267-
defaults.removeObject(forKey: key)
268-
migratedLastQueries += 1
269-
} catch {
270-
logger.error("Failed to migrate last query for \(uuidString): \(error.localizedDescription)")
271-
}
272-
}
273-
274190
defaults.set(true, forKey: migrationCompleteKey)
275191

276-
if migratedTabStates > 0 || migratedLastQueries > 0 {
277-
logger.trace(
278-
"Migration complete: \(migratedTabStates) tab states, \(migratedLastQueries) last queries"
279-
)
192+
if migratedTabStates > 0 {
193+
logger.trace("Migration complete: \(migratedTabStates) tab states")
280194
} else {
281195
logger.trace("Migration complete: no legacy data found")
282196
}

0 commit comments

Comments
 (0)