fix: history load notifies and never overwrites unreadable file; dictionary keeps ids across external edits, undo safety

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Sources/SstData/DictionaryFile.swift

@@ -70,6 +70,21 @@ public struct DictionaryFile: Equatable, Sendable {
         return .entry(DictionaryEntry(kind: .term(trimmed)))
     }
 
+    /// Reuses the ids of `old` for entries with equal content (first unused match, in order), so ids survive a reload.
+    public func preservingIDs(from old: DictionaryFile) -> DictionaryFile {
+        var available = old.entries
+        var result = self
+        result.lines = lines.map { line in
+            guard case .entry(let entry) = line else { return line }
+            if let index = available.firstIndex(where: { $0.kind == entry.kind }) {
+                let reused = available.remove(at: index)
+                return .entry(DictionaryEntry(id: reused.id, kind: entry.kind))
+            }
+            return line
+        }
+        return result
+    }
+
     public func serialized() -> String {
         let out = lines.map { line -> String in
             switch line {

Sources/SstData/DictionaryStore.swift

@@ -14,6 +14,8 @@ public final class DictionaryStore {
     public let fileURL: URL
     public private(set) var file = DictionaryFile()
     public var onChange: (() -> Void)?
+    /// Fired (after `onChange`) when the file was changed by something other than this store.
+    public var onExternalChange: (() -> Void)?
 
     private var lastText = ""
     private var watcher: DispatchSourceFileSystemObject?
@@ -53,8 +55,9 @@ public final class DictionaryStore {
     public func reloadIfChanged() {
         guard let text = try? String(contentsOf: fileURL, encoding: .utf8), text != lastText else { return }
         lastText = text
-        file = DictionaryFile(parsing: text)
+        file = DictionaryFile(parsing: text).preservingIDs(from: file)
         onChange?()
+        onExternalChange?()
     }
 
     /// Watches the folder (editors replace files instead of editing them in place).

Sources/SstData/History.swift

@@ -35,6 +35,8 @@ public final class HistoryStore {
         case empty
         case loaded
         case recoveredCorrupt(backup: URL)
+        /// The file exists but cannot be read; it is left alone and nothing is written over it.
+        case unreadable(String)
     }
 
     private struct Document: Codable {
@@ -47,6 +49,7 @@ public final class HistoryStore {
     public var onChange: (() -> Void)?
 
     private let fileURL: URL
+    private var readOnly = false
     private var saveTask: Task<Void, Never>?
 
     public init(fileURL: URL) {
@@ -68,18 +71,29 @@ public final class HistoryStore {
 
     @discardableResult
     public func load() -> LoadOutcome {
-        guard let data = try? Data(contentsOf: fileURL) else {
-            entries = []
-            return .empty
+        let data: Data
+        do {
+            data = try Data(contentsOf: fileURL)
+        } catch {
+            if !FileManager.default.fileExists(atPath: fileURL.path) {
+                entries = []
+                onChange?()
+                return .empty
+            }
+            readOnly = true
+            lastError = error.localizedDescription
+            return .unreadable(error.localizedDescription)
         }
         do {
             entries = try Self.makeDecoder().decode(Document.self, from: data).entries
+            onChange?()
             return .loaded
         } catch {
             let stamp = ISO8601DateFormatter().string(from: Date()).replacingOccurrences(of: ":", with: "-")
             let backup = fileURL.deletingLastPathComponent().appending(path: "history.corrupt-\(stamp).json")
             try? FileManager.default.moveItem(at: fileURL, to: backup)
             entries = []
+            onChange?()
             return .recoveredCorrupt(backup: backup)
         }
     }
@@ -103,6 +117,7 @@ public final class HistoryStore {
     public func flush() {
         saveTask?.cancel()
         saveTask = nil
+        guard !readOnly else { return }
         do {
             try FileManager.default.createDirectory(
                 at: fileURL.deletingLastPathComponent(), withIntermediateDirectories: true)

Sources/SstData/ViewSupport.swift

@@ -161,16 +161,23 @@ public final class DictionaryViewModel: ObservableObject {
     }
 
     public func undo() {
-        guard let snapshot = undoStack.popLast() else { return }
-        canUndo = !undoStack.isEmpty
+        guard let snapshot = undoStack.last else { return }
         do {
             try store.apply { $0 = snapshot }
+            undoStack.removeLast()
+            canUndo = !undoStack.isEmpty
         } catch {
             errorMessage = error.localizedDescription
         }
         reload()
     }
 
+    /// The file was edited elsewhere: older snapshots no longer describe it, so undo is dropped.
+    public func externalChangeDetected() {
+        undoStack.removeAll()
+        canUndo = false
+    }
+
     private func mutate(_ change: (inout DictionaryFile) -> Void) {
         let snapshot = store.file
         do {

Tests/SstDataTests/DictionaryFileTests.swift

@@ -49,4 +49,23 @@ struct DictionaryFileTests {
         #expect(file.entries.isEmpty)
         #expect(file.serialized() == "# c\n")
     }
+
+    @Test func preservingIDsKeepsIdsOfUnchangedLines() {
+        let old = DictionaryFile(parsing: "a\nb => c\nd\n")
+        let new = DictionaryFile(parsing: "z\na\nb => c\nd2\n").preservingIDs(from: old)
+        let oldIDs = old.entries.map(\.id), newIDs = new.entries.map(\.id)
+        #expect(newIDs[1] == oldIDs[0])
+        #expect(newIDs[2] == oldIDs[1])
+        #expect(!oldIDs.contains(newIDs[0]))
+        #expect(!oldIDs.contains(newIDs[3]))
+        #expect(new.serialized() == "z\na\nb => c\nd2\n")
+    }
+
+    @Test func preservingIDsMapsDuplicatesOneToOneInOrder() {
+        let old = DictionaryFile(parsing: "a\na\n")
+        let new = DictionaryFile(parsing: "a\na\na\n").preservingIDs(from: old)
+        let oldIDs = old.entries.map(\.id), newIDs = new.entries.map(\.id)
+        #expect(Array(newIDs[0...1]) == oldIDs)
+        #expect(!oldIDs.contains(newIDs[2]))
+    }
 }

Tests/SstDataTests/DictionaryStoreTests.swift

@@ -43,6 +43,23 @@ struct DictionaryStoreTests {
         #expect(changes == 1)
     }
 
+    @Test func externalChangeFiresBothCallbacksAndKeepsIDs() throws {
+        let url = tempFile()
+        let store = DictionaryStore(fileURL: url)
+        try store.load()
+        try store.apply { $0.append(.term("Vercel")) }
+        let id = store.entries[0].id
+        var changes = 0, external = 0
+        store.onChange = { changes += 1 }
+        store.onExternalChange = { external += 1 }
+        try "Nieuw\nVercel\n".write(to: url, atomically: true, encoding: .utf8)
+        store.reloadIfChanged()
+        #expect(changes == 1 && external == 1)
+        #expect(store.entries[1].id == id)
+        try store.apply { $0.append(.term("Eigen")) }
+        #expect(changes == 2 && external == 1)
+    }
+
     @Test func ourOwnWriteDoesNotTriggerAReload() throws {
         let store = DictionaryStore(fileURL: tempFile())
         try store.load()

Tests/SstDataTests/HistoryTests.swift

@@ -57,6 +57,35 @@ struct HistoryStoreTests {
         #expect(store.entries.isEmpty)
     }
 
+    @Test func loadNotifiesOnChange() {
+        let url = tempFile()
+        let writer = HistoryStore(fileURL: url)
+        writer.add(entry("a", at: 0)); writer.add(entry("b", at: 1))
+        writer.flush()
+
+        let store = HistoryStore(fileURL: url)
+        var calls = 0
+        store.onChange = { calls += 1 }
+        #expect(store.load() == .loaded)
+        #expect(calls == 1)
+        #expect(store.entries.count == 2)
+    }
+
+    @Test func unreadableFileIsReportedAndNeverOverwritten() throws {
+        let url = tempFile()
+        try FileManager.default.createDirectory(at: url, withIntermediateDirectories: true)   // history.json is a directory
+        let store = HistoryStore(fileURL: url)
+        guard case .unreadable = store.load() else {
+            Issue.record("expected unreadable"); return
+        }
+        store.add(entry("a", at: 0))
+        store.flush()
+        var isDirectory: ObjCBool = false
+        #expect(FileManager.default.fileExists(atPath: url.path, isDirectory: &isDirectory))
+        #expect(isDirectory.boolValue)
+        #expect(store.lastError != nil)
+    }
+
     @Test func onChangeFiresForEdits() {
         let store = HistoryStore(fileURL: tempFile())
         var calls = 0

Tests/SstDataTests/ViewSupportTests.swift

@@ -67,6 +67,20 @@ struct HistoryViewModelTests {
         return store
     }
 
+    @Test func daysAreFilledAfterTheStoreLoads() {
+        let url = FileManager.default.temporaryDirectory.appending(path: "sst-vm-\(UUID().uuidString)/h.json")
+        let writer = HistoryStore(fileURL: url)
+        writer.add(HistoryEntry(date: .now, duration: 1, rawText: "x", finalText: "x"))
+        writer.flush()
+        let store = HistoryStore(fileURL: url)
+        let model = HistoryViewModel(store: store)
+        store.onChange = { model.refresh() }
+        #expect(model.days.isEmpty)
+        store.load()
+        #expect(!model.days.isEmpty)
+        #expect(model.totalCount == 1)
+    }
+
     @Test func listsAllEntriesNewestFirstAndFiltersByQuery() {
         let model = HistoryViewModel(store: store(["eerste", "tweede zin", "derde"]))
         #expect(model.totalCount == 3)
@@ -152,6 +166,31 @@ struct DictionaryViewModelTests {
         #expect(store.entries == [term("Supabase")])
     }
 
+    @Test func externalChangeClearsUndo() throws {
+        let (model, store) = try makeModel("Supabase\n")
+        model.add(.term("Vercel"))
+        #expect(model.canUndo)
+        model.externalChangeDetected()
+        #expect(!model.canUndo)
+        model.undo()
+        #expect(store.entries == [term("Supabase"), term("Vercel")])
+    }
+
+    @Test func failedUndoKeepsTheSnapshot() throws {
+        let (model, store) = try makeModel("Supabase\n")
+        model.add(.term("Vercel"))
+        let folder = store.fileURL.deletingLastPathComponent()
+        try FileManager.default.setAttributes([.posixPermissions: 0o500], ofItemAtPath: folder.path)
+        model.undo()
+        try FileManager.default.setAttributes([.posixPermissions: 0o700], ofItemAtPath: folder.path)
+        #expect(model.errorMessage != nil)
+        #expect(model.canUndo)
+        model.errorMessage = nil
+        model.undo()
+        #expect(store.entries == [term("Supabase")])
+        #expect(!model.canUndo)
+    }
+
     @Test func searchMatchesBothSidesIgnoringCaseAndDiacritics() throws {
         let (model, _) = try makeModel("cloud code => Claude Code\nCafé\nVercel\n")
         model.query = "CLAUDE"