Pre-push review: T-1165 Block ID Cache Eviction

diff --git a/prism/Models/MarkdownBlock.swift b/prism/Models/MarkdownBlock.swiftindex 335d88f..b40644e 100644--- a/prism/Models/MarkdownBlock.swift+++ b/prism/Models/MarkdownBlock.swift@@ -9,50 +9,121 @@ import Foundation import CryptoKit import Synchronization -/// Thread-safe cache for memoizing MarkdownBlock ID computations.+/// Storage and eviction policy for memoizing MarkdownBlock ID computations. /// /// The ID computation involves SHA-256 hashing which is expensive when called /// repeatedly. SwiftUI's ForEach and LazyVStack access block IDs many times /// per layout pass, causing severe performance degradation with large documents. ///-/// This cache stores computed IDs keyed by the content hash input string,-/// ensuring each unique block's ID is computed only once.-private enum BlockIDCache {-    /// The underlying cache storage, protected by a Mutex for thread safety.-    private static let cache = Mutex<[String: String]>([:])+/// Entries are held across two generations (`primary` + `secondary`). New+/// inserts always land in `primary`; when `primary` reaches `maxEntries` it is+/// rotated into `secondary` (discarding the older generation) and a fresh+/// `primary` is started. Lookups check both generations and return a hit+/// *without* promoting it, so memory is bounded at ~2×`maxEntries` and the+/// most-recent generation always survives an overflow — replacing the former+/// clear-on-overflow wipe that dropped every entry at once (T-1165).+///+/// Not thread-safe on its own: the process-global instance in ``BlockIDCache``+/// is guarded by a `Mutex`. This type is extracted so its eviction behaviour+/// can be unit-tested in isolation with a small `maxEntries`, free of the+/// shared static state that parallel tests would race on.+///+/// Explicitly `Sendable`: the process-global instance is shared across threads+/// through ``BlockIDCache``'s `Mutex`, so a future non-`Sendable` stored+/// property must be a compile-time error at this declaration, not a silent+/// thread-safety hole at the `Mutex` use site.+struct BlockIDStore: Sendable {+    /// Maximum entries in the primary generation before a rotation.+    let maxEntries: Int++    /// Most-recent generation; receives every new insert.+    private var primary: [String: String] = [:]++    /// Previous generation, retained across one rotation so recently-used+    /// entries survive an overflow instead of being wiped.+    private var secondary: [String: String] = [:]++    init(maxEntries: Int) {+        self.maxEntries = maxEntries+    }++    /// Returns the cached ID for the given hash input, computing and inserting+    /// it on a miss.+    ///+    /// - Parameter hashInput: The content string to hash for ID generation.+    /// - Returns: The cached or newly computed ID.+    mutating func id(for hashInput: String) -> String {+        if let cached = primary[hashInput] {+            return cached+        }+        if let cached = secondary[hashInput] {+            // Non-promoting: return the hit without copying it into `primary`.+            // Promotion would fill `primary` faster under a large working set,+            // triggering more rotations for no worst-case gain (Decision 2).+            return cached+        }++        let computedId = Self.computeId(for: hashInput)++        // Rotate before inserting when the primary generation is full so the+        // previous generation survives rather than being discarded all at once.+        if primary.count >= maxEntries {+            secondary = primary+            primary = [:]+        }+        primary[hashInput] = computedId+        return computedId+    }++    /// Empties both generations.+    mutating func removeAll() {+        primary.removeAll()+        secondary.removeAll()+    } -    /// Maximum number of cached entries before cleanup.-    /// Set high enough to handle large documents without eviction during normal use.+    /// Total entries currently retained across both generations.+    ///+    /// Test support: lets eviction tests assert memory stays bounded.+    var count: Int { primary.count + secondary.count }++    /// Whether `hashInput` is currently cached in either generation.+    ///+    /// Test support: lets eviction tests assert generation retention after a+    /// rotation without recomputing the value.+    func contains(_ hashInput: String) -> Bool {+        primary[hashInput] != nil || secondary[hashInput] != nil+    }++    private static func computeId(for hashInput: String) -> String {+        let data = Data(hashInput.utf8)+        let hash = SHA256.hash(data: data)+        return hash.compactMap { String(format: "%02x", $0) }.prefix(16).joined()+    }+}++/// Thread-safe, process-global cache for memoizing MarkdownBlock ID+/// computations. Delegates storage and eviction to a shared ``BlockIDStore``+/// guarded by a `Mutex`.+private enum BlockIDCache {+    /// Maximum entries per generation. Memory is bounded at ~2× this value.+    /// Set high enough to handle large documents without eviction during+    /// normal use.     private static let maxEntries = 10_000 +    /// The shared store, protected by a Mutex for thread safety.+    private static let store = Mutex<BlockIDStore>(BlockIDStore(maxEntries: maxEntries))+     /// Returns the cached ID for the given hash input, computing it if not present.     ///     /// - Parameter hashInput: The content string to hash for ID generation.     /// - Returns: The cached or newly computed ID.     static func id(for hashInput: String) -> String {-        cache.withLock { storage in-            if let cached = storage[hashInput] {-                return cached-            }--            // Compute the ID-            let data = Data(hashInput.utf8)-            let hash = SHA256.hash(data: data)-            let computedId = hash.compactMap { String(format: "%02x", $0) }.prefix(16).joined()--            // Simple cache eviction: clear if too large-            if storage.count >= maxEntries {-                storage.removeAll(keepingCapacity: true)-            }--            storage[hashInput] = computedId-            return computedId-        }+        store.withLock { $0.id(for: hashInput) }     }      /// Clears the cache. Call when documents are closed or memory pressure occurs.     static func clear() {-        cache.withLock { $0.removeAll() }+        store.withLock { $0.removeAll() }     } } 

diff --git a/prismTests/BlockIDCacheTests.swift b/prismTests/BlockIDCacheTests.swiftindex 9b21a98..9ff2b27 100644--- a/prismTests/BlockIDCacheTests.swift+++ b/prismTests/BlockIDCacheTests.swift@@ -63,3 +63,81 @@ struct BlockIDCacheTests {         #expect(originalIDs == recomputedIDs)     } }++/// Unit tests for ``BlockIDStore`` eviction (T-1165).+///+/// These exercise a local, small-cap store instance rather than the+/// process-global ``BlockIDCache``, so the assertions are deterministic and+/// free of the shared static state that parallel tests would race on.+struct BlockIDStoreEvictionTests {++    @Test("Overflow bounds memory at ~2x the cap and never drops to zero")+    func overflowKeepsMemoryBoundedWithoutFullWipe() {+        let maxEntries = 8+        var store = BlockIDStore(maxEntries: maxEntries)++        // Insert well past two full generations to force repeated rotations.+        for index in 0..<(maxEntries * 3) {+            _ = store.id(for: "key-\(index)")+        }++        // Memory stays bounded across both generations...+        #expect(store.count <= maxEntries * 2)+        // ...and, crucially, a rotation never wipes everything the way the old+        // clear-on-overflow strategy did (which would leave a single entry).+        #expect(store.count > maxEntries)+    }++    @Test("Most-recent generation survives one rotation, evicted after two")+    func generationRetentionAcrossRotations() {+        let maxEntries = 4+        var store = BlockIDStore(maxEntries: maxEntries)++        // Cache a known entry, then capture its ID for stability comparison.+        let originalId = store.id(for: "anchor")++        // Fill the primary generation to the cap and trigger exactly one+        // rotation, which moves "anchor" into the surviving secondary.+        for index in 0..<maxEntries {+            _ = store.id(for: "first-\(index)")+        }++        // Retained without recomputation, and the value is unchanged.+        #expect(store.contains("anchor"))+        #expect(store.id(for: "anchor") == originalId)++        // Trigger a second rotation; this discards the generation holding+        // "anchor", so it is no longer cached.+        for index in 0..<maxEntries {+            _ = store.id(for: "second-\(index)")+        }+        #expect(!store.contains("anchor"))++        // Even when re-derived after eviction, the ID is content-stable.+        #expect(store.id(for: "anchor") == originalId)+    }++    @Test("removeAll empties both generations")+    func removeAllClearsBothGenerations() {+        let maxEntries = 4+        var store = BlockIDStore(maxEntries: maxEntries)++        // Populate enough to occupy both generations.+        for index in 0..<(maxEntries * 2) {+            _ = store.id(for: "key-\(index)")+        }+        #expect(store.count > 0)++        store.removeAll()+        #expect(store.count == 0)+    }++    @Test("Identical content yields a stable ID, matching MarkdownBlock.id")+    func storeIdMatchesBlockId() {+        var store = BlockIDStore(maxEntries: 10_000)+        // The store hashes the same "paragraph:<text>" input MarkdownBlock uses.+        let storeId = store.id(for: "paragraph:Hello world")+        let blockId = MarkdownBlock.paragraph(markdown: "Hello world").id+        #expect(storeId == blockId)+    }+}

diff --git a/prismTests/VisibleBlockIdentityTests.swift b/prismTests/VisibleBlockIdentityTests.swiftnew file mode 100644index 0000000..aa6cb60--- /dev/null+++ b/prismTests/VisibleBlockIdentityTests.swift@@ -0,0 +1,62 @@+//+//  VisibleBlockIdentityTests.swift+//  prismTests+//+//  Created by Claude on 2/6/2026.+//++import Testing+@testable import prism++/// Regression tests locking in the view-layer protection against duplicate+/// content (T-1165).+///+/// `MarkdownBlock.id` is content-only by design, so blocks with identical+/// content share an `id`. SwiftUI `ForEach` identity at the document level is+/// instead keyed on `VisibleBlock.id` — the composite `"\(block.id)-\(sourceIndex)"`+/// — so duplicate content still renders with distinct identities. These tests+/// guard that composite so the protection cannot silently regress (for example+/// if `VisibleBlock.id` were ever reduced to `block.id`).+struct VisibleBlockIdentityTests {++    /// A document with three pairs of duplicate content: identical paragraphs,+    /// identical fenced code blocks, and repeated thematic breaks.+    private static let duplicateContentMarkdown = """+    Repeated paragraph.++    Repeated paragraph.++    ---++    ---++    ```swift+    let x = 1+    ```++    ```swift+    let x = 1+    ```+    """++    @Test("Duplicate-content blocks get distinct VisibleBlock identities")+    func duplicateContentBlocksHaveDistinctVisibleIds() {+        let blocks = MarkdownBlockParser.parse(Self.duplicateContentMarkdown)+        let structure = MarkdownSectionBuilder.build(from: blocks)+        let visible = structure.visibleBlocks(collapsedSectionIds: [])++        // Sanity: the document actually contains duplicate content.+        #expect(visible.count >= 6)++        // The underlying content-only block IDs DO collide for duplicates —+        // this is the intended design and the reason the composite ID exists.+        let blockIds = visible.map { $0.block.id }+        #expect(Set(blockIds).count < blockIds.count,+                "Expected duplicate content to share block IDs")++        // The view identities used by ForEach must nonetheless be unique.+        let visibleIds = visible.map { $0.id }+        #expect(Set(visibleIds).count == visibleIds.count,+                "VisibleBlock.id must be unique across duplicate-content blocks")+    }+}

diff --git a/CHANGELOG.md b/CHANGELOG.mdindex 52ebd21..6ce3ca6 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -34,6 +34,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0  ### Changed +- Replaced `BlockIDCache`'s clear-on-overflow eviction with a non-promoting two-generation scheme (T-1165). The block-ID memoization cache (`prism/Models/MarkdownBlock.swift`) previously wiped all 10,000 entries at once on overflow, which could recur within a single SwiftUI layout pass for documents with more unique blocks than the cap (each block's `id` is read many times per pass), recomputing SHA-256 hashes repeatedly. The storage and eviction logic is extracted into a new internal `BlockIDStore` value type — `primary`/`secondary` generations; a lookup checks both and returns on hit without promoting; on overflow the `primary` rotates into `secondary` (discarding the older generation) and a fresh `primary` starts — guarded by the existing `Mutex`. Memory is bounded at ~2× the cap and the most-recent generation always survives a rotation, removing the cliff for working sets up to ~2× the cap. `block.id` values are unchanged (same content-only SHA-256), so persisted notes and saved scroll positions still resolve by exact match. T-1165 also proposed incorporating positional data (array index / byte offset) into the hash to fix duplicate-content `ForEach` collisions; that was deliberately dropped — the collision is already handled at the view layer (the document `ForEach` keys on the composite `VisibleBlock.id` = `block.id` + `sourceIndex`; nested rendering uses enumerated offsets), and adding positional data would change every `id` and orphan existing persisted notes and scroll positions. New `BlockIDStoreEvictionTests` cover bounded growth, generation retention across a rotation, and ID stability; new `VisibleBlockIdentityTests` lock in distinct view identities for duplicate paragraphs, code blocks, and thematic breaks. Smolspec and decision log (3 ADRs) under `specs/block-id-cache-eviction/`. - Extracted the imported-notes load path from `NotesManager` into a new `ImportedNotesProcessor` `@MainActor` enum (T-1217)   - `NotesManager.swift` drops from 1333 to ~1077 lines; `loadImportedNotes` becomes a thin wrapper that delegates to `ImportedNotesProcessor.process`   - No public-API change: `loadImportedNotes` signature and the published `importedNotes` state surface are preserved