refactor(datagrid): correctness, a11y, perf, tests

CHANGELOG.md

@@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Click a focused cell to start editing without a second click
 - Data grid focus ring follows the system accent color and contrast settings
 - Data grid cells expose accessibility row and column index ranges to VoiceOver on all dataset sizes
+- Data grid column headers announce sort direction and multi-sort priority to VoiceOver
 - Multi-cell paste: paste TSV data from the clipboard into the grid starting from the focused cell, grouped as a single undo action
 - Shift+Tab navigates to the previous cell in the data grid
 - Copy rows writes TSV, HTML table, and plain text to the clipboard for richer paste in spreadsheet apps
@@ -54,6 +55,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Date picker popover font follows the data grid font setting
 - Data grid undo/redo uses the window's UndoManager instead of a private instance, unifying Cmd+Z across editor and grid
 - Right-click during cell editing shows the native text context menu instead of the row menu
+- Multiline cell overlay editor discards the in-progress edit when a column resize fires, instead of silently committing partial text
+- Data grid cell focus ring redraws when the user toggles Light or Dark mode mid-session, picking up the system's appearance-aware focus indicator color
+- Data grid keeps sortedIDs and cachedRowCount paired by calling updateCache() immediately after the SwiftUI bridge writes new sortedIDs to the coordinator, removing a window where the cached count and the sort permutation could disagree
+- Display formats memoized per tab on MainContentCoordinator keyed by schema version, smart-detection setting, and format-overrides version, so ValueDisplayDetector.detect runs once per result schema instead of on every SwiftUI body evaluation
 
 ### Fixed
 

TablePro/Core/Services/Formatting/ValueDisplayFormatService.swift

@@ -18,6 +18,8 @@ final class ValueDisplayFormatService {
     /// Auto-detected formats keyed by "connectionId.tableName.columnName" for per-connection isolation.
     private var autoDetectedFormats: [String: ValueDisplayFormat] = [:]
 
+    private(set) var overridesVersion: Int = 0
+
     private init() {}
 
     // MARK: - Format Application
@@ -102,6 +104,8 @@ final class ValueDisplayFormatService {
         } else {
             ValueDisplayFormatStorage.shared.save(overrides, for: tableName, connectionId: connectionId)
         }
+
+        overridesVersion &+= 1
     }
 
     // MARK: - Private Formatting

TablePro/Views/Main/Child/MainEditorContentView.swift

@@ -536,16 +536,25 @@ struct MainEditorContentView: View {
     }
 
     private func displayFormats(for tab: QueryTab) -> [ValueDisplayFormat?] {
+        let settings = AppSettingsManager.shared.dataGrid
+        let service = ValueDisplayFormatService.shared
+        let smartDetectionEnabled = settings.enableSmartValueDetection
+        let overridesVersion = service.overridesVersion
+
+        if let cached = coordinator.displayFormatsCache[tab.id],
+           cached.schemaVersion == tab.schemaVersion,
+           cached.smartDetectionEnabled == smartDetectionEnabled,
+           cached.overridesVersion == overridesVersion {
+            return cached.formats
+        }
+
         let tableRows = coordinator.tableRowsStore.existingTableRows(for: tab.id)
         let columns = tableRows?.columns ?? []
         let columnTypes = tableRows?.columnTypes ?? []
         guard !columns.isEmpty else { return [] }
 
-        let settings = AppSettingsManager.shared.dataGrid
-        let service = ValueDisplayFormatService.shared
-
         var detected: [ValueDisplayFormat?] = Array(repeating: nil, count: columns.count)
-        if settings.enableSmartValueDetection {
+        if smartDetectionEnabled {
             let sampleRows: [[String?]]? = {
                 let rows = tableRows?.rows.prefix(10).map(\.values) ?? []
                 return rows.isEmpty ? nil : Array(rows)
@@ -582,7 +591,14 @@ struct MainEditorContentView: View {
             }
         }
 
-        return merged.contains(where: { $0 != nil }) ? merged : []
+        let result = merged.contains(where: { $0 != nil }) ? merged : []
+        coordinator.displayFormatsCache[tab.id] = DisplayFormatsCacheEntry(
+            schemaVersion: tab.schemaVersion,
+            smartDetectionEnabled: smartDetectionEnabled,
+            overridesVersion: overridesVersion,
+            formats: result
+        )
+        return result
     }
 
     /// Returns the display order as a permutation of `RowID`, or nil when no sort applies.

TablePro/Views/Main/MainContentCoordinator.swift

@@ -31,6 +31,13 @@ struct QuerySortCacheEntry {
     let schemaVersion: Int
 }
 
+struct DisplayFormatsCacheEntry {
+    let schemaVersion: Int
+    let smartDetectionEnabled: Bool
+    let overridesVersion: Int
+    let formats: [ValueDisplayFormat?]
+}
+
 /// Sidebar table loading state — single source of truth for sidebar UI
 enum SidebarLoadingState: Equatable {
     case idle
@@ -147,6 +154,8 @@ final class MainContentCoordinator {
     /// Cache for async-sorted query tab rows (large datasets sorted on background thread)
     @ObservationIgnored var querySortCache: [UUID: QuerySortCacheEntry] = [:]
 
+    @ObservationIgnored var displayFormatsCache: [UUID: DisplayFormatsCacheEntry] = [:]
+
     @ObservationIgnored var pendingScrollToTopAfterReplace: Set<UUID> = []
 
     // MARK: - Internal State
@@ -351,6 +360,9 @@ final class MainContentCoordinator {
         if querySortCache.keys.contains(where: { !openTabIds.contains($0) }) {
             querySortCache = querySortCache.filter { openTabIds.contains($0.key) }
         }
+        if displayFormatsCache.keys.contains(where: { !openTabIds.contains($0) }) {
+            displayFormatsCache = displayFormatsCache.filter { openTabIds.contains($0.key) }
+        }
         for (tabId, task) in activeSortTasks where !openTabIds.contains(tabId) {
             task.cancel()
             activeSortTasks.removeValue(forKey: tabId)
@@ -585,6 +597,7 @@ final class MainContentCoordinator {
 
         tableRowsStore.tearDown()
         querySortCache.removeAll()
+        displayFormatsCache.removeAll()
         cachedTableColumnTypes.removeAll()
         cachedTableColumnNames.removeAll()
 

TablePro/Views/Results/CellOverlayEditor.swift

@@ -140,7 +140,7 @@ final class CellOverlayEditor: NSObject, NSTextViewDelegate {
             queue: .main
         ) { [weak self] _ in
             Task { @MainActor [weak self] in
-                self?.dismiss(commit: true)
+                self?.dismiss(commit: false)
             }
         }
     }

TablePro/Views/Results/DataGridCellView.swift

@@ -13,7 +13,7 @@ final class DataGridCellView: NSTableCellView {
     var isFocusedCell: Bool = false {
         didSet {
             guard oldValue != isFocusedCell else { return }
-            updateFocusBorder()
+            updateFocusRing()
         }
     }
 
@@ -46,18 +46,35 @@ final class DataGridCellView: NSTableCellView {
     override var backgroundStyle: NSView.BackgroundStyle {
         didSet {
             backgroundView.isHidden = (backgroundStyle == .emphasized) || (changeBackgroundColor == nil)
-            if isFocusedCell { updateFocusBorder() }
+            if isFocusedCell { updateFocusRing() }
         }
     }
 
-    private func updateFocusBorder() {
+    override var focusRingMaskBounds: NSRect { bounds }
+
+    override func drawFocusRingMask() {
+        NSBezierPath(rect: bounds).fill()
+    }
+
+    override func draw(_ dirtyRect: NSRect) {
+        super.draw(dirtyRect)
+        guard isFocusedCell, backgroundStyle != .emphasized else { return }
+        NSGraphicsContext.saveGraphicsState()
+        NSFocusRingPlacement.only.set()
+        drawFocusRingMask()
+        NSGraphicsContext.restoreGraphicsState()
+    }
+
+    override func viewDidChangeEffectiveAppearance() {
+        super.viewDidChangeEffectiveAppearance()
         if isFocusedCell {
-            layer?.borderWidth = 2
-            layer?.borderColor = backgroundStyle == .emphasized
-                ? NSColor.white.withAlphaComponent(0.8).cgColor
-                : NSColor.keyboardFocusIndicatorColor.cgColor
-        } else {
-            layer?.borderWidth = 0
+            needsDisplay = true
         }
     }
+
+    private func updateFocusRing() {
+        focusRingType = isFocusedCell ? .exterior : .none
+        noteFocusRingMaskChanged()
+        needsDisplay = true
+    }
 }

TablePro/Views/Results/DataGridView.swift

@@ -142,6 +142,7 @@ struct DataGridView: NSViewRepresentable {
         context.coordinator.tableRowsProvider = tableRowsProvider
         context.coordinator.tableRowsMutator = tableRowsMutator
         context.coordinator.sortedIDs = sortedIDs
+        context.coordinator.updateCache()
         context.coordinator.syncDisplayFormats(displayFormats)
         context.coordinator.delegate = delegate
         delegate?.dataGridAttach(tableViewCoordinator: context.coordinator)
@@ -227,6 +228,7 @@ struct DataGridView: NSViewRepresentable {
         coordinator.tableRowsProvider = tableRowsProvider
         coordinator.tableRowsMutator = tableRowsMutator
         coordinator.sortedIDs = sortedIDs
+        coordinator.updateCache()
         coordinator.syncDisplayFormats(displayFormats)
         coordinator.delegate = delegate
         delegate?.dataGridAttach(tableViewCoordinator: coordinator)

TablePro/Views/Results/SortableHeaderCell.swift

@@ -73,6 +73,23 @@ final class SortableHeaderCell: NSTableHeaderCell {
         priority: Int
     ) {}
 
+    override func accessibilityLabel() -> String? {
+        let baseLabel = super.accessibilityLabel() ?? stringValue
+        guard let direction = sortDirection else { return baseLabel }
+        let directionSuffix: String
+        switch direction {
+        case .ascending:
+            directionSuffix = String(localized: "Sorted ascending")
+        case .descending:
+            directionSuffix = String(localized: "Sorted descending")
+        }
+        guard let sortPriority, sortPriority >= 2 else {
+            return "\(baseLabel), \(directionSuffix)"
+        }
+        let prioritySuffix = String(format: String(localized: "Priority %d"), sortPriority)
+        return "\(baseLabel), \(directionSuffix), \(prioritySuffix)"
+    }
+
     private func priorityNumberString() -> String? {
         guard let sortPriority, sortPriority >= 2 else { return nil }
         return String(sortPriority)

TablePro/Views/Results/SortableHeaderView.swift

@@ -119,7 +119,6 @@ final class SortableHeaderView: NSTableHeaderView {
 
     override func viewDidMoveToWindow() {
         super.viewDidMoveToWindow()
-        window?.acceptsMouseMovedEvents = true
         window?.invalidateCursorRects(for: self)
     }
 

TableProTests/Models/UI/ColumnIdentitySchemaTests.swift

@@ -132,4 +132,70 @@ struct ColumnIdentitySchemaTests {
         #expect(before.dataIndex(from: columnId) == 2)
         #expect(after.dataIndex(from: columnId) == 0)
     }
+
+    @Test("Duplicate names ignore the duplicate column name when looking up by raw name")
+    func positionalSchemaIgnoresDuplicateRawName() {
+        let schema = ColumnIdentitySchema(columns: ["a", "b", "a"])
+        #expect(!schema.isNameBased)
+
+        #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("a")) == nil)
+        #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("b")) == nil)
+    }
+
+    @Test("Positional schema only resolves col_N identifiers within range")
+    func positionalSchemaResolvesOnlyValidPositions() {
+        let schema = ColumnIdentitySchema(columns: ["a", "b", "a"])
+
+        #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("col_0")) == 0)
+        #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("col_1")) == 1)
+        #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("col_2")) == 2)
+        #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("col_3")) == nil)
+        #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("col_-1")) == nil)
+    }
+
+    @Test("Name-based schema does not resolve positional identifiers like col_0")
+    func nameBasedSchemaDoesNotResolvePositional() {
+        let schema = ColumnIdentitySchema(columns: ["id", "name", "email"])
+
+        #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("col_0")) == nil)
+        #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("col_1")) == nil)
+        #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("col_2")) == nil)
+    }
+
+    @Test("Duplicate-name positional schema disagrees with the layout key for that name")
+    func positionalSchemaIdentifiersDoNotMatchColumnNames() {
+        let schema = ColumnIdentitySchema(columns: ["id", "name", "name"])
+
+        #expect(!schema.isNameBased)
+        #expect(schema.identifier(for: 0)?.rawValue == "col_0")
+        #expect(schema.identifier(for: 1)?.rawValue == "col_1")
+        #expect(schema.identifier(for: 2)?.rawValue == "col_2")
+    }
+
+    @Test("Single column with duplicate-trigger reserved name produces positional id")
+    func singleReservedNameTriggersPositional() {
+        let schema = ColumnIdentitySchema(columns: ["__rowNumber__"])
+
+        #expect(!schema.isNameBased)
+        #expect(schema.identifier(for: 0)?.rawValue == "col_0")
+        #expect(schema.dataIndex(from: ColumnIdentitySchema.rowNumberIdentifier) == nil)
+    }
+
+    @Test("Schema with three duplicates uses positional fallback consistently")
+    func tripleDuplicateUsesPositionalFallback() {
+        let schema = ColumnIdentitySchema(columns: ["x", "x", "x"])
+
+        #expect(!schema.isNameBased)
+        for index in 0..<3 {
+            #expect(schema.identifier(for: index)?.rawValue == "col_\(index)")
+        }
+    }
+
+    @Test("Empty array column input is name-based and resolves nothing")
+    func emptyColumnsInputIsNameBased() {
+        let schema = ColumnIdentitySchema(columns: [])
+        #expect(schema.isNameBased)
+        #expect(schema.identifiers.isEmpty)
+        #expect(schema.dataIndex(from: NSUserInterfaceItemIdentifier("anything")) == nil)
+    }
 }