Fix crash: enrichment held array indices across awaits
CI / build-and-deploy (pull_request) Successful in 30s
CI / build-and-deploy (pull_request) Successful in 30s
`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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JgLHztZGaEHvS3KNeGQmRM
This commit is contained in:
co-authored by
Claude Opus 5
parent
1995b38b52
commit
2e1b454c09
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user