From 2e1b454c09151a39f8f5a3a2d4feee814936cf46 Mon Sep 17 00:00:00 2001 From: Krishna Kumar Date: Wed, 29 Jul 2026 23:46:07 -0500 Subject: [PATCH] Fix crash: enrichment held array indices across awaits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `startEnrichment` snapshotted `bookmarks.indices`, then awaited a network call per bookmark. `bookmarks` is replaced wholesale by loads, searches and the unread filter, so by the time the loop read `bookmarks[i]` the index could be out of range — EXC_BREAKPOINT in Array._checkSubscript, straight from the read. The write path already guarded with `i < bookmarks.count`; the read did not. `enrichAll()` had the same shape and the same exposure. Both now track bookmark ids and re-resolve the position after each await, via a shared `apply(summary:tags:toId:)` that skips a bookmark that is no longer loaded rather than writing to whatever now sits at that index — which is the other half of the bug: a stale-but-in-range index would have silently attached one bookmark's summary to another. Found while investigating leftover Spotlight "translation error ... Code=1 (null)" entries after the indexing fix. Those turned out to be a symptom, not a separate bug: the crash tore the process down mid-donation and the in-flight items failed to translate. With the crash fixed they are gone, and the suite went from crashing (0 tests executed) to green across three consecutive runs. Adds a source round-trip to SpotlightRetrievalTests, so both donation paths — app entities and raw searchable items — are covered. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JgLHztZGaEHvS3KNeGQmRM --- Marks/Views/BookmarksViewModel.swift | 71 +++++++++++++++--------- MarksTests/SpotlightRetrievalTests.swift | 19 +++++++ 2 files changed, 64 insertions(+), 26 deletions(-) diff --git a/Marks/Views/BookmarksViewModel.swift b/Marks/Views/BookmarksViewModel.swift index 5b5ea18..ea3307d 100644 --- a/Marks/Views/BookmarksViewModel.swift +++ b/Marks/Views/BookmarksViewModel.swift @@ -225,52 +225,71 @@ final class BookmarksViewModel { } func enrichAll() async { - let toEnrich = bookmarks.indices.filter { bookmarks[$0].aiSummary == nil } + // Track ids, not indices. Each iteration awaits the network, and + // `bookmarks` is replaced wholesale by loads, searches and filter + // toggles — an index captured before the await can be out of range by + // the time it is used. + let toEnrich = bookmarks.filter { $0.aiSummary == nil }.map(\.id) guard !toEnrich.isEmpty else { return } enrichmentProgress = 0 - for (done, i) in toEnrich.enumerated() { + var completed = 0 + for id in toEnrich { + guard let bookmark = bookmarks.first(where: { $0.id == id }) else { continue } do { - let (summary, tags) = try await claude.enrich(bookmark: bookmarks[i]) - bookmarks[i].aiSummary = summary - bookmarks[i].aiTags = tags - saveAIData(for: bookmarks[i]) - let enriched = bookmarks[i] - Task { await SpotlightIndexer.index([enriched]) } - enrichmentProgress = Double(done + 1) / Double(toEnrich.count) + let (summary, tags) = try await claude.enrich(bookmark: bookmark) + apply(summary: summary, tags: tags, toId: id) + completed += 1 + enrichmentProgress = Double(completed) / Double(toEnrich.count) } catch { break } } - let enriched = toEnrich.count - bookmarks.indices.filter { bookmarks[$0].aiSummary == nil }.count - if enriched > 0 { Analytics.track("enrich.completed", ["count": enriched]) } + if completed > 0 { Analytics.track("enrich.completed", ["count": completed]) } enrichmentProgress = 0 } + /// Write an enrichment result back by id, skipping it if the bookmark is no + /// longer loaded (deleted, archived, or filtered away mid-flight). + private func apply(summary: String, tags: [String], toId id: Int) { + guard let i = bookmarks.firstIndex(where: { $0.id == id }) else { + Log.ai.notice("enrich result dropped: id=\(id, privacy: .public) no longer loaded") + return + } + bookmarks[i].aiSummary = summary + bookmarks[i].aiTags = tags + saveAIData(for: bookmarks[i]) + let enriched = bookmarks[i] + Task { await SpotlightIndexer.index([enriched]) } + } + // Silently enrich new bookmarks that lack summaries (background, non-blocking) + // + // Everything here is keyed by bookmark id rather than array index. This loop + // awaits a network call per bookmark, and `bookmarks` gets replaced under it + // by loads, searches and the unread filter — an index captured up front used + // to crash on the read (`bookmarks[i]`), which the write path already + // guarded against but the read did not. private func startEnrichment() { enrichTask?.cancel() enrichTask = Task { [weak self] in guard let self else { return } - let indices = await MainActor.run { bookmarks.indices.filter { bookmarks[$0].aiSummary == nil }.prefix(5) } - Log.ai.info("startEnrichment: \(indices.count, privacy: .public) to enrich") - for i in indices { + let ids = await MainActor.run { + bookmarks.filter { $0.aiSummary == nil }.prefix(5).map(\.id) + } + Log.ai.info("startEnrichment: \(ids.count, privacy: .public) to enrich") + for id in ids { guard !Task.isCancelled else { return } let claude = await MainActor.run(body: { self.claude }) + guard let bm = await MainActor.run(body: { + bookmarks.first { $0.id == id } + }) else { continue } do { - let bm = await MainActor.run { bookmarks[i] } - Log.ai.debug("startEnrichment enriching id=\(bm.id, privacy: .public)") + Log.ai.debug("startEnrichment enriching id=\(id, privacy: .public)") let (summary, tags) = try await claude.enrich(bookmark: bm) - Log.ai.debug("startEnrichment done id=\(bm.id, privacy: .public)") - await MainActor.run { - guard i < bookmarks.count else { return } - bookmarks[i].aiSummary = summary - bookmarks[i].aiTags = tags - saveAIData(for: bookmarks[i]) - let enriched = bookmarks[i] - Task { await SpotlightIndexer.index([enriched]) } - } + Log.ai.debug("startEnrichment done id=\(id, privacy: .public)") + await MainActor.run { apply(summary: summary, tags: tags, toId: id) } } catch { - Log.ai.error("startEnrichment failed idx=\(i, privacy: .public): \(error.localizedDescription, privacy: .public)") + Log.ai.error("startEnrichment failed id=\(id, privacy: .public): \(error.localizedDescription, privacy: .public)") break } } diff --git a/MarksTests/SpotlightRetrievalTests.swift b/MarksTests/SpotlightRetrievalTests.swift index 9413564..7c41759 100644 --- a/MarksTests/SpotlightRetrievalTests.swift +++ b/MarksTests/SpotlightRetrievalTests.swift @@ -37,4 +37,23 @@ struct SpotlightRetrievalTests { "url did not survive the round trip: \(hit.url)") #expect(hit.host == "github.com", "host was \(hit.host)") } + + @Test func indexedSourceIsRetrievable() async throws { + let source = IngestedSource( + kind: .text, + title: "Wqvbnmtest meeting notes", + bodyText: "A distinctive probe document about wqvbnmtest.", + tags: ["wqvbnmtest"] + ) + await SourceSpotlightIndexer.index([source]) + var hits: [RetrievedBookmark] = [] + for _ in 0..<20 { + try? await Task.sleep(for: .milliseconds(700)) + hits = await SpotlightBookmarkSearch.run(query: "wqvbnmtest", limit: 5) + if !hits.isEmpty { break } + } + await SourceSpotlightIndexer.remove(ids: [source.id]) + #expect(!hits.isEmpty, "Spotlight returned no hits for an indexed source") + #expect(hits.first?.title.contains("Wqvbnmtest") == true) + } }