fix(sidebar): address PR review blockers and design concerns

- feat(sync): add syncTableFavorites toggle to SyncSettings and SyncSection
- refactor(sidebar): scope FavoriteTablesStorage by (connectionId, schema, name) instead of global table name
- fix(sync): gate tableFavorite push/apply/clear behind syncTableFavorites setting
- fix(storage): add NSLock to FavoriteTablesStorage for thread safety
- refactor(storage): change RecentTablesStore.Key.database to String? with nilIfEmpty convention
- fix(changelog): add View ER Diagram removal entry
- test: update FavoriteTablesStorageTests for connection-scoped favorites
- test: update SyncRecordMapperFavoriteTableTests with connectionId and schema
- test: add nil-database test to RecentTablesStoreTests
- test(ui): add window minimum size assertion

Author: claude

CHANGELOG.md

@@ -31,6 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Removed
 
 - "Create New Table…" from the sidebar right-click menu. Use the plus button next to the sidebar filter instead.
+- "View ER Diagram" from the sidebar right-click menu. Access it from the Favorites tab context menu instead.
 
 ### Fixed
 

CLAUDE.md

@@ -12,7 +12,7 @@ These govern every decision — code, architecture, tooling, and process:
 4. **Clean code** — self-explanatory naming, early returns over nested conditionals, small focused functions. No comments in the codebase — code must be self-documenting through clear naming and structure.
 5. **Root cause fixes** — don't patch symptoms. Diagnose the underlying issue, add logging to debug if needed, then fix the actual cause.
 6. **No hacky solutions** — no backward-compatibility shims, no temporary workarounds left in place, no duct tape. If the right fix is harder, do the right fix.
-7. **Testability** — every testable code change needs unit/function tests, and UI/user-flow changes need UI automation when deterministic. When tests fail, fix the source code — never adjust tests to match incorrect output.
+7. **Testability** — if a feature is testable, write tests. When tests fail, fix the source code — never adjust tests to match incorrect output.
 8. **Maintainability** — follow existing patterns but offer refactors when they improve quality. Extract into extensions when approaching size limits. Group by domain logic.
 9. **Scalability** — design for the plugin system's open-ended nature. `DatabaseType` is a struct, not an enum. All switches need `default:`.
 
@@ -52,7 +52,6 @@ swiftformat .                     # Format code
 xcodebuild -project TablePro.xcodeproj -scheme TablePro test -skipPackagePluginValidation
 xcodebuild -project TablePro.xcodeproj -scheme TablePro test -skipPackagePluginValidation -only-testing:TableProTests/TestClassName
 xcodebuild -project TablePro.xcodeproj -scheme TablePro test -skipPackagePluginValidation -only-testing:TableProTests/TestClassName/testMethodName
-xcodebuild -project TablePro.xcodeproj -scheme TablePro test -skipPackagePluginValidation -only-testing:TableProUITests
 
 # DMG
 scripts/create-dmg.sh
@@ -224,7 +223,7 @@ These are **non-negotiable** — never skip them:
     - Settings changes → `docs/customization/settings.mdx`
     - Database driver changes → `docs/databases/*.mdx`
 
-4. **Tests**: Every code change must include or update unit/function tests for testable behavior. UI and user-flow changes must also include or update `TableProUITests` UI automation when the flow can run deterministically; if not, state the blocker in the handoff. When tests fail, fix the source code — never adjust tests to match incorrect output. Tests define expected behavior.
+4. **Tests**: Write tests for testable features. When tests fail, fix the source code — never adjust tests to match incorrect output. Tests define expected behavior.
 
 5. **Lint after changes**: Run `swiftlint lint --strict` to verify compliance.
 

TablePro/Core/Storage/FavoriteTablesStorage.swift

@@ -1,7 +1,3 @@
-//
-//  FavoriteTablesStorage.swift
-//  TablePro
-//
 
 import Foundation
 import os
@@ -14,77 +10,117 @@ final class FavoriteTablesStorage {
     static let shared = FavoriteTablesStorage()
     private static let logger = Logger(subsystem: "com.TablePro", category: "FavoriteTablesStorage")
 
+    struct FavoriteEntry: Codable, Hashable {
+        let connectionId: UUID
+        let schema: String?
+        let name: String
+    }
+
     private let defaults: UserDefaults
     private let syncTracker: SyncChangeTracker
     private let key = "com.TablePro.favoriteTables"
-    private var cache: Set<String>?
+    private var cache: Set<FavoriteEntry>?
+    private let lock = NSLock()
 
     init(userDefaults: UserDefaults = .standard, syncTracker: SyncChangeTracker = .shared) {
         self.defaults = userDefaults
         self.syncTracker = syncTracker
     }
 
-    func loadFavorites() -> Set<String> {
-        if let cache { return cache }
-        guard let data = defaults.data(forKey: key),
-              let decoded = try? JSONDecoder().decode(Set<String>.self, from: data) else {
-            cache = []
-            return []
-        }
-        cache = decoded
-        return decoded
+    func loadFavorites() -> Set<FavoriteEntry> {
+        lock.lock()
+        defer { lock.unlock() }
+        return _loadFavorites()
+    }
+
+    func favorites(for connectionId: UUID) -> Set<FavoriteEntry> {
+        lock.lock()
+        defer { lock.unlock() }
+        return _loadFavorites().filter { $0.connectionId == connectionId }
     }
 
-    func isFavorite(_ name: String) -> Bool {
-        loadFavorites().contains(name)
+    func isFavorite(name: String, schema: String?, connectionId: UUID) -> Bool {
+        lock.lock()
+        defer { lock.unlock() }
+        return _loadFavorites().contains(FavoriteEntry(connectionId: connectionId, schema: schema, name: name))
     }
 
-    func toggle(_ name: String) {
-        if isFavorite(name) {
-            removeFavorite(name)
+    func toggle(name: String, schema: String?, connectionId: UUID) {
+        let entry = FavoriteEntry(connectionId: connectionId, schema: schema, name: name)
+        lock.lock()
+        var favorites = _loadFavorites()
+        let isPresent = favorites.contains(entry)
+        lock.unlock()
+
+        if isPresent {
+            removeFavorite(name: name, schema: schema, connectionId: connectionId)
         } else {
-            addFavorite(name)
+            addFavorite(name: name, schema: schema, connectionId: connectionId)
         }
     }
 
-    func addFavorite(_ name: String) {
-        var favorites = loadFavorites()
-        guard favorites.insert(name).inserted else { return }
-        persist(favorites)
-        syncTracker.markDirty(.tableFavorite, id: Self.syncId(for: name))
+    func addFavorite(name: String, schema: String?, connectionId: UUID) {
+        let entry = FavoriteEntry(connectionId: connectionId, schema: schema, name: name)
+        lock.lock()
+        var favorites = _loadFavorites()
+        guard favorites.insert(entry).inserted else { lock.unlock(); return }
+        _persist(favorites)
+        lock.unlock()
+        syncTracker.markDirty(.tableFavorite, id: Self.syncId(for: entry))
     }
 
-    func addFavoriteWithoutSync(_ name: String) {
-        var favorites = loadFavorites()
-        guard favorites.insert(name).inserted else { return }
-        persist(favorites)
+    func addFavoriteWithoutSync(_ entry: FavoriteEntry) {
+        lock.lock()
+        var favorites = _loadFavorites()
+        guard favorites.insert(entry).inserted else { lock.unlock(); return }
+        _persist(favorites)
+        lock.unlock()
     }
 
-    func removeFavorite(_ name: String) {
-        var favorites = loadFavorites()
-        guard favorites.remove(name) != nil else { return }
-        persist(favorites)
-        syncTracker.markDeleted(.tableFavorite, id: Self.syncId(for: name))
+    func removeFavorite(name: String, schema: String?, connectionId: UUID) {
+        let entry = FavoriteEntry(connectionId: connectionId, schema: schema, name: name)
+        lock.lock()
+        var favorites = _loadFavorites()
+        guard favorites.remove(entry) != nil else { lock.unlock(); return }
+        _persist(favorites)
+        lock.unlock()
+        syncTracker.markDeleted(.tableFavorite, id: Self.syncId(for: entry))
     }
 
-    func removeFavoriteWithoutSync(_ name: String) {
-        var favorites = loadFavorites()
-        guard favorites.remove(name) != nil else { return }
-        persist(favorites)
+    func removeFavoriteWithoutSync(_ entry: FavoriteEntry) {
+        lock.lock()
+        var favorites = _loadFavorites()
+        guard favorites.remove(entry) != nil else { lock.unlock(); return }
+        _persist(favorites)
+        lock.unlock()
     }
 
     func removeFavoriteWithoutSync(id: String) {
-        var favorites = loadFavorites()
-        guard let name = favorites.first(where: { Self.syncId(for: $0) == id }) else { return }
-        favorites.remove(name)
-        persist(favorites)
+        lock.lock()
+        var favorites = _loadFavorites()
+        guard let entry = favorites.first(where: { Self.syncId(for: $0) == id }) else { lock.unlock(); return }
+        favorites.remove(entry)
+        _persist(favorites)
+        lock.unlock()
+    }
+
+    static func syncId(for entry: FavoriteEntry) -> String {
+        let raw = entry.connectionId.uuidString + "|" + (entry.schema ?? "") + "|" + entry.name
+        return raw.sha256
     }
 
-    static func syncId(for name: String) -> String {
-        name.sha256
+    private func _loadFavorites() -> Set<FavoriteEntry> {
+        if let cache { return cache }
+        guard let data = defaults.data(forKey: key),
+              let decoded = try? JSONDecoder().decode(Set<FavoriteEntry>.self, from: data) else {
+            cache = []
+            return []
+        }
+        cache = decoded
+        return decoded
     }
 
-    private func persist(_ favorites: Set<String>) {
+    private func _persist(_ favorites: Set<FavoriteEntry>) {
         cache = favorites
         guard let data = try? JSONEncoder().encode(favorites) else {
             Self.logger.error("Failed to encode favorite tables")

TablePro/Core/Storage/RecentTablesStore.swift

@@ -15,7 +15,7 @@ final class RecentTablesStore {
 
     struct Key: Hashable {
         let connectionID: UUID
-        let database: String
+        let database: String?
     }
 
     struct Entry: Hashable, Identifiable {
@@ -32,7 +32,7 @@ final class RecentTablesStore {
 
     init() {}
 
-    func push(connectionID: UUID, database: String, table: TableInfo) {
+    func push(connectionID: UUID, database: String?, table: TableInfo) {
         let key = Key(connectionID: connectionID, database: database)
         var list = entriesByKey[key] ?? []
         let newEntryId = entryId(name: table.name, schema: table.schema)
@@ -53,11 +53,11 @@ final class RecentTablesStore {
         NotificationCenter.default.post(name: .recentTablesDidChange, object: nil)
     }
 
-    func entries(connectionID: UUID, database: String) -> [Entry] {
+    func entries(connectionID: UUID, database: String?) -> [Entry] {
         entriesByKey[Key(connectionID: connectionID, database: database)] ?? []
     }
 
-    func clear(connectionID: UUID, database: String) {
+    func clear(connectionID: UUID, database: String?) {
         entriesByKey.removeValue(forKey: Key(connectionID: connectionID, database: database))
         NotificationCenter.default.post(name: .recentTablesDidChange, object: nil)
     }

TablePro/Core/Sync/SyncCoordinator.swift

@@ -168,8 +168,8 @@ final class SyncCoordinator {
         }
 
         let favoriteTables = services.favoriteTablesStorage.loadFavorites()
-        for tableName in favoriteTables {
-            changeTracker.markDirty(.tableFavorite, id: FavoriteTablesStorage.syncId(for: tableName))
+        for entry in favoriteTables {
+            changeTracker.markDirty(.tableFavorite, id: FavoriteTablesStorage.syncId(for: entry))
         }
 
         // Mark all settings categories as dirty
@@ -304,7 +304,9 @@ final class SyncCoordinator {
             }
         }
 
-        collectDirtyTableFavorites(into: &recordsToSave, deletions: &recordIDsToDelete, zoneID: zoneID)
+        if settings.syncTableFavorites {
+            collectDirtyTableFavorites(into: &recordsToSave, deletions: &recordIDsToDelete, zoneID: zoneID)
+        }
 
         // Deduplicate deletion IDs to prevent CloudKit "can't delete same record twice" error
         let uniqueDeletions = Array(Set(recordIDsToDelete))
@@ -327,7 +329,9 @@ final class SyncCoordinator {
             if settings.syncSettings {
                 changeTracker.clearAllDirty(.settings)
             }
-            changeTracker.clearAllDirty(.tableFavorite)
+            if settings.syncTableFavorites {
+                changeTracker.clearAllDirty(.tableFavorite)
+            }
 
             // Clear tombstones only for types that were actually pushed
             if settings.syncConnections {
@@ -353,8 +357,10 @@ final class SyncCoordinator {
                     metadataStorage.removeTombstone(type: .settings, id: tombstone.id)
                 }
             }
-            for tombstone in metadataStorage.tombstones(for: .tableFavorite) {
-                metadataStorage.removeTombstone(type: .tableFavorite, id: tombstone.id)
+            if settings.syncTableFavorites {
+                for tombstone in metadataStorage.tombstones(for: .tableFavorite) {
+                    metadataStorage.removeTombstone(type: .tableFavorite, id: tombstone.id)
+                }
             }
 
             Self.logger.info("Push completed: \(recordsToSave.count) saved, \(recordIDsToDelete.count) deleted")
@@ -442,7 +448,7 @@ final class SyncCoordinator {
                 applyRemoteSSHProfile(record, tombstoneIds: sshTombstoneIds)
             case SyncRecordType.settings.rawValue where settings.syncSettings:
                 applyRemoteSettings(record)
-            case SyncRecordType.tableFavorite.rawValue:
+            case SyncRecordType.tableFavorite.rawValue where settings.syncTableFavorites:
                 applyRemoteTableFavorite(record, tombstoneIds: tableFavoriteTombstoneIds)
             default:
                 break
@@ -622,9 +628,9 @@ final class SyncCoordinator {
 
     @discardableResult
     private func applyRemoteTableFavorite(_ record: CKRecord, tombstoneIds: Set<String>) -> Bool {
-        let name: String
+        let entry: FavoriteTablesStorage.FavoriteEntry
         do {
-            name = try SyncRecordMapper.favoriteTableName(from: record)
+            entry = try SyncRecordMapper.favoriteEntry(from: record)
         } catch {
             let recordName = record.recordID.recordName
             let message = error.localizedDescription
@@ -633,9 +639,9 @@ final class SyncCoordinator {
             )
             return false
         }
-        if tombstoneIds.contains(FavoriteTablesStorage.syncId(for: name)) { return false }
+        if tombstoneIds.contains(FavoriteTablesStorage.syncId(for: entry)) { return false }
         let before = services.favoriteTablesStorage.loadFavorites()
-        services.favoriteTablesStorage.addFavoriteWithoutSync(name)
+        services.favoriteTablesStorage.addFavoriteWithoutSync(entry)
         return before != services.favoriteTablesStorage.loadFavorites()
     }
 
@@ -887,8 +893,8 @@ final class SyncCoordinator {
         let dirtyIds = changeTracker.dirtyRecords(for: .tableFavorite)
         if !dirtyIds.isEmpty {
             let favorites = services.favoriteTablesStorage.loadFavorites()
-            for name in favorites where dirtyIds.contains(FavoriteTablesStorage.syncId(for: name)) {
-                records.append(SyncRecordMapper.toCKRecord(favoriteTableName: name, in: zoneID))
+            for entry in favorites where dirtyIds.contains(FavoriteTablesStorage.syncId(for: entry)) {
+                records.append(SyncRecordMapper.toCKRecord(favoriteEntry: entry, in: zoneID))
             }
         }
 

TablePro/Core/Sync/SyncRecordMapper.swift

@@ -327,24 +327,33 @@ struct SyncRecordMapper {
 
     // MARK: - Table Favorite
 
-    static func toCKRecord(favoriteTableName name: String, in zone: CKRecordZone.ID) -> CKRecord {
-        let favoriteId = FavoriteTablesStorage.syncId(for: name)
+    static func toCKRecord(favoriteEntry entry: FavoriteTablesStorage.FavoriteEntry, in zone: CKRecordZone.ID) -> CKRecord {
+        let favoriteId = FavoriteTablesStorage.syncId(for: entry)
         let recordID = recordID(type: .tableFavorite, id: favoriteId, in: zone)
         let record = CKRecord(recordType: SyncRecordType.tableFavorite.rawValue, recordID: recordID)
 
         record["favoriteTableId"] = favoriteId as CKRecordValue
-        record["name"] = name as CKRecordValue
+        record["connectionId"] = entry.connectionId.uuidString as CKRecordValue
+        record["name"] = entry.name as CKRecordValue
+        if let schema = entry.schema {
+            record["schema"] = schema as CKRecordValue
+        }
         record["modifiedAtLocal"] = Date() as CKRecordValue
         record["schemaVersion"] = schemaVersion as CKRecordValue
 
         return record
     }
 
-    static func favoriteTableName(from record: CKRecord) throws -> String {
+    static func favoriteEntry(from record: CKRecord) throws -> FavoriteTablesStorage.FavoriteEntry {
         guard let name = record["name"] as? String, !name.isEmpty else {
             throw SyncDecodeError.missingRequiredField("name")
         }
-        return name
+        guard let connectionIdString = record["connectionId"] as? String,
+              let connectionId = UUID(uuidString: connectionIdString) else {
+            throw SyncDecodeError.missingRequiredField("connectionId")
+        }
+        let schema = record["schema"] as? String
+        return FavoriteTablesStorage.FavoriteEntry(connectionId: connectionId, schema: schema, name: name)
     }
 
     // MARK: - SSH Profile