From 5e51e70585a6f1446736e8e482d67238b236bced Mon Sep 17 00:00:00 2001 From: Michael Yeack Date: Wed, 3 Jun 2026 17:53:27 -0700 Subject: [PATCH] Hotfix: stop Import-All crash loop + make recent interactions raise the score MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two app-breaking issues, shipped together as an urgent hotfix. 1. Import-All crash / app won't reopen ContactSyncService imported on the MAIN ModelContext and saved once there. "Import All" inserts hundreds of Contacts; that main-context save fires ContactListView's @Query mid-transaction and faults the Contact.interactions inverse (EXC_BAD_ACCESS) — the exact failure PR #43 fixed for the pull path. Because startAutoSync re-imports on every .onAppear, it became a crash loop so the app couldn't reopen. Fix: importContacts/importSelected apply mergeOrInsert on a background ModelContext (autosaveEnabled=false) with a single settled save, so the main context's @Queries only ever see one committed state. startAutoSync also defers the launch import by 1s so the first frame renders first. Posts .blackbookSyncDidComplete after import so the Dashboard refetches. 2. Recent interactions don't raise the relationship score (Hugo Dooner) The score reads the denormalized Contact.lastInteractionDate. Synced iMessage interaction *records* arrive cleanly, but the contact-field update carrying lastInteractionDate is rejected by conflict resolution when the local copy is newer + pending — so recency stays 0 and the score sits at priority-only (exactly 20 / "Fading"). Fix: RelationshipScoreEngine.recalculateAll now re-derives each contact's lastInteractionDate from the Interaction records this device already holds (single FetchDescriptor, grouped by the to-one interaction.contact?.id — the same controlled pattern the server uses; never touches the faulting Contact.interactions inverse), taking max() so a newer manual date is never lowered. recalculateAll is now also invoked right after each sync pull on the background context, so scores refresh regardless of the visible tab (on macOS the Dashboard's own recalc wouldn't run otherwise). Tests: +3 regression cases in RelationshipScoreEngineTests covering the heal-from-records behavior (stale date healed, priority contact recovers above the boost, newer manual date preserved). Full macOS suite green (220 tests). Co-Authored-By: Claude Opus 4.8 --- Blackbook/Services/ContactSyncService.swift | 32 +++++++++--- .../Services/LocalServerSyncService.swift | 6 +++ .../Services/RelationshipScoreEngine.swift | 33 ++++++++++++- .../RelationshipScoreEngineTests.swift | 49 +++++++++++++++++++ 4 files changed, 111 insertions(+), 9 deletions(-) diff --git a/Blackbook/Services/ContactSyncService.swift b/Blackbook/Services/ContactSyncService.swift index 4193c6d..6e8116a 100644 --- a/Blackbook/Services/ContactSyncService.swift +++ b/Blackbook/Services/ContactSyncService.swift @@ -45,9 +45,16 @@ final class ContactSyncService { return } - // Run synchronously on the main thread to keep ModelContext safe. - importContacts(into: modelContext) - startObservingChanges() + // Defer the launch import by one second so the first frame renders before the import + // runs. Combined with the background-context apply in importContacts, this prevents the + // launch-time re-import from faulting a live @Query mid-insert — the crash loop that + // left the app unable to reopen after an "Import All" (work_log: import crash). + Task { @MainActor [weak self] in + try? await Task.sleep(for: .seconds(1)) + guard let self else { return } + self.importContacts(into: modelContext) + self.startObservingChanges() + } } func startObservingChanges() { @@ -95,9 +102,15 @@ final class ContactSyncService { let started = Date() do { let cnContacts = try fetchAllSystemContacts() - let outcome = try mergeOrInsert(cnContacts, into: modelContext) - try modelContext.save() + // Apply on a background context with a single settled commit so the main context's + // @Queries (ContactListView, etc.) never observe the partial-state insert window — + // the SwiftData inverse fault that crashed "Import All" (mirrors the PR #43 pull fix). + let bgContext = ModelContext(modelContext.container) + bgContext.autosaveEnabled = false + let outcome = try mergeOrInsert(cnContacts, into: bgContext) + try bgContext.save() lastSyncDate = Date() + NotificationCenter.default.post(name: .blackbookSyncDidComplete, object: nil) logger.info("Contact sync completed — processed=\(cnContacts.count) updated=\(outcome.identifierMatched) reattached=\(outcome.reattached) inserted=\(outcome.inserted)") let durationMs = Int(Date().timeIntervalSince(started) * 1000) Log.action("contacts.sync", metadata: [ @@ -153,9 +166,14 @@ final class ContactSyncService { do { let allSystem = fetchSystemContacts() let selected = allSystem.filter { identifiers.contains($0.identifier) } - let outcome = try mergeOrInsert(selected, into: modelContext) - try modelContext.save() + // Background-context apply + single commit, as in importContacts (avoids the + // partial-state @Query fault during bulk insert). + let bgContext = ModelContext(modelContext.container) + bgContext.autosaveEnabled = false + let outcome = try mergeOrInsert(selected, into: bgContext) + try bgContext.save() lastSyncDate = Date() + NotificationCenter.default.post(name: .blackbookSyncDidComplete, object: nil) logger.info("Selective import completed — \(selected.count) contacts imported") let durationMs = Int(Date().timeIntervalSince(started) * 1000) Log.action("contacts.sync.selective", metadata: [ diff --git a/Blackbook/Services/LocalServerSyncService.swift b/Blackbook/Services/LocalServerSyncService.swift index 3721643..bcfa810 100644 --- a/Blackbook/Services/LocalServerSyncService.swift +++ b/Blackbook/Services/LocalServerSyncService.swift @@ -466,6 +466,12 @@ final class LocalServerSyncService { } try bgContext.save() + + // Re-derive lastInteractionDate from the interaction records this device now holds and + // recompute relationship scores, so health reflects freshly-synced interactions even + // when the Dashboard isn't the visible tab (on macOS its @State recalc wouldn't run). + // Runs on the background context → the main context's @Queries see one settled commit. + RelationshipScoreEngine().recalculateAll(context: bgContext) } private func fetchPendingContacts(context: ModelContext) throws -> [Contact] { diff --git a/Blackbook/Services/RelationshipScoreEngine.swift b/Blackbook/Services/RelationshipScoreEngine.swift index 9a9a2ed..53053fc 100644 --- a/Blackbook/Services/RelationshipScoreEngine.swift +++ b/Blackbook/Services/RelationshipScoreEngine.swift @@ -12,14 +12,43 @@ final class RelationshipScoreEngine { UserDefaults.standard.object(forKey: "scoring.recencyWeight") as? Double ?? AppConstants.Scoring.recencyWeight } - /// Recalculates scores and trends for all contacts using only direct - /// Contact properties. Never accesses lazy SwiftData relationships. + /// Recalculates scores and trends for all contacts. + /// + /// First re-derives each contact's `lastInteractionDate` from its actual `Interaction` + /// records on *this* device, then scores from that field. This decouples the score from + /// cross-device propagation of the denormalized `lastInteractionDate`: synced interaction + /// rows arrive cleanly, but the contact-field update can be rejected by conflict resolution + /// (`ContactSyncApply.applyRemoteContact`) when the local copy is newer + pending, leaving + /// recency stuck at 0 even though the texts are present (work_log: Hugo Dooner). + /// + /// We deliberately read interactions via a single `FetchDescriptor` and group + /// by the to-one `interaction.contact?.id` — the same controlled service-side pattern used + /// by the server's `IMessageSyncService`. We never touch the `Contact.interactions` to-many + /// inverse, which is what faults under `@Query` re-renders (work_log 2026-06-01/02). func recalculateAll(context: ModelContext) { do { let contacts = try context.fetch(FetchDescriptor()) let sevenDaysAgo = Date.daysAgo(7) + // Latest interaction date per contact, derived from the records this device holds. + var latestInteraction: [UUID: Date] = [:] + for ix in try context.fetch(FetchDescriptor()) { + guard let cid = ix.contact?.id else { continue } + if let current = latestInteraction[cid] { + if ix.date > current { latestInteraction[cid] = ix.date } + } else { + latestInteraction[cid] = ix.date + } + } + for contact in contacts { + // Heal a stale/missing denormalized date from real interaction records. + // Local-only correction — no markLocallyEdited(); each device derives its own. + if let derived = latestInteraction[contact.id], + contact.lastInteractionDate == nil || derived > contact.lastInteractionDate! { + contact.lastInteractionDate = derived + } + // Score based on recency of last interaction + priority boost var score: Double = 0 if let lastDate = contact.lastInteractionDate { diff --git a/BlackbookTests/RelationshipScoreEngineTests.swift b/BlackbookTests/RelationshipScoreEngineTests.swift index 31acd26..e199213 100644 --- a/BlackbookTests/RelationshipScoreEngineTests.swift +++ b/BlackbookTests/RelationshipScoreEngineTests.swift @@ -239,4 +239,53 @@ final class RelationshipScoreEngineTests: XCTestCase { XCTAssertGreaterThan(contact1.relationshipScore, contact2.relationshipScore, "Recent contact should score higher than old") XCTAssertGreaterThan(contact2.relationshipScore, contact3.relationshipScore, "Old contact should score higher than no-interaction") } + + // MARK: - 14. Heal stale/missing lastInteractionDate from interaction records + // + // Regression: synced iMessage interaction *records* arrive, but the contact-field update that + // carries lastInteractionDate is rejected by conflict resolution (local newer + pending), so the + // device shows the texts yet recency stays 0 and the score sits at priority-only (Hugo Dooner: 20). + // recalculateAll must re-derive the date from the records the device already holds. + + func testHealsStaleDateFromInteractionRecord() throws { + let contact = makeContact() + addInteraction(to: contact, type: .text, date: Date.daysAgo(2)) + // Simulate the rejected field update: record present, denormalized date stale. + contact.lastInteractionDate = nil + try context.save() + + let score = recalculateAndGetScore(for: contact) + + XCTAssertNotNil(contact.lastInteractionDate, "date should be re-derived from the record") + XCTAssertGreaterThan(score, 80.0, "a 2-day-old interaction must lift recency, not leave the score at 0") + XCTAssertEqual(contact.scoreTrend, .up) + } + + func testPriorityContactRecoversAboveBoostAfterHeal() throws { + // The exact Hugo Dooner shape: priority + recent texts but a stale date pinning the score at 20. + let contact = makeContact(isPriority: true) + addInteraction(to: contact, type: .text, date: Date.daysAgo(2)) + contact.lastInteractionDate = nil + contact.relationshipScore = AppConstants.Scoring.priorityBoost // the stuck "20 / Fading" value + try context.save() + + let score = recalculateAndGetScore(for: contact) + + XCTAssertGreaterThan(score, AppConstants.Scoring.priorityBoost + 40, + "recent interactions must contribute recency on top of the priority boost") + } + + func testHealNeverLowersNewerManualDate() throws { + // A manual edit set a date newer than any record; recalc uses max() and must not regress it. + let contact = makeContact() + addInteraction(to: contact, type: .text, date: Date.daysAgo(30)) + let newer = Date.daysAgo(1) + contact.lastInteractionDate = newer + try context.save() + + engine.recalculateAll(context: context) + + XCTAssertEqual(contact.lastInteractionDate?.timeIntervalSince1970 ?? 0, + newer.timeIntervalSince1970, accuracy: 1.0) + } }