Skip to content

End-to-end review: dedup + graph-engine tests, VM import fix, Dashboard doc resync #45

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mayeack merged 1 commit into from
Jun 2, 2026
Merged

End-to-end review: dedup + graph-engine tests, VM import fix, Dashboard doc resync #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 27 additions & 10 deletions .cursor/pages/Dashboard.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,44 +15,61 @@ The Dashboard is the app's home screen, providing an at-a-glance summary of the
**File:** `Blackbook/Views/Dashboard/DashboardView.swift`

**Data sources:**
- `@Query(sort: \Contact.relationshipScore, order: .reverse)` — all contacts, filtered to exclude hidden
- `@Query(sort: \Reminder.dueDate)` — all reminders
- `@State private var allContacts: [Contact]` — fetched on demand via `FetchDescriptor(sortBy: \.relationshipScore, .reverse)`, filtered to exclude hidden/merged-away via the `contacts` computed property
- `@State private var reminders: [Reminder]` — fetched on demand via `FetchDescriptor(sortBy: \.dueDate)`
- `DashboardViewModel` — `@State private var viewModel`

**Lifecycle:** Calls `viewModel.recalculateScores(context:)` on appear.
> **Why not `@Query`?** As of PR #43, `DashboardView` no longer uses `@Query`. `@Query` auto-refreshed on every `ModelContext` save — including the intermediate saves a sync-pull makes — and the `_SwiftData_SwiftUI` machinery faulted the `Contact.interactions` inverse relationship while observing that partial state, crashing the app at launch (`EXC_BAD_ACCESS`). The dashboard now snapshots the store explicitly so it only ever renders settled state.

**Layout:** `NavigationStack` > `ScrollView` > `VStack(spacing: 20)` with `.padding()`. Navigation title: "Dashboard".
**Lifecycle:**
- `.task` — on first appearance waits ~500ms (so any in-flight pull-apply commits first), calls `refreshFromStore()`, sets `hasLoadedOnce = true`, then schedules `viewModel.recalculateScoresIfNeeded(context:)` ~1s later.
- `.onReceive(NotificationCenter … .blackbookSyncDidComplete)` — re-runs `refreshFromStore()` after each successful sync so newly pulled data appears without a relaunch.
- `refreshFromStore()` (`@MainActor`) — fetches contacts + reminders and recomputes `weeklyStats`.

**Layout:** `NavigationStack` > `SwiftUI.Group` gated on `hasLoadedOnce`:
- **Loading:** centered `ProgressView` until the first fetch lands.
- **Loaded:** `ScrollView` > `VStack(spacing: 20)` with `.padding()`.

Navigation title: **"Overview"**.

**Cards (top to bottom):**

1. **Weekly Stats Card** (`weeklyStatsCard`)
- Title: "This Week", icon: `chart.bar.fill`, accent gold
- Shows `StatBubble` pair: total interactions count + unique people count
- Data from `viewModel.weeklyStats(from:)`
- Data from `weeklyStats` (computed by `viewModel.computeWeeklyStats(context:)`)

2. **Fading Relationships Card** (`fadingCard`)
2. **Prioritize Card** (`prioritizeCard`)
- Title: "Prioritize", icon: `pin.fill`, accent gold
- Empty: an `AddContactChip` button → `showingPrioritizePicker` sheet (`PrioritizeContactPicker`)
- Populated: `PriorityChipFlowLayout` of `PriorityContactChip`s (each a `NavigationLink(value: contact.id)`), plus a trailing `AddContactChip`
- The chip's ✕ button clears `isPriority` (via `markLocallyEdited()`)

3. **Fading Relationships Card** (`fadingCard`)
- Title: "Fading Relationships", icon: `arrow.down.right.circle.fill`, color: `fadingRed`
- Empty state: "All relationships are healthy"
- Populated: `VStack(spacing: 8)` of contact rows — `HStack(spacing: 12)` with `ContactAvatarView(size: 36)`, name `.body.weight(.medium)`, last interaction `.caption`, `ScoreBadgeView`, `.padding(.vertical, 2)`
- Populated: `PriorityChipFlowLayout` of `ContactChip`s, each a `NavigationLink(value: contact.id)`
- Data from `viewModel.fadingContacts(from:)`

3. **Upcoming Reminders Card** (`remindersCard`)
4. **Upcoming Reminders Card** (`remindersCard`)
- Title: "Upcoming Reminders", icon: `bell.fill`, accent gold
- Shows up to 5 incomplete reminders
- Empty state: "No upcoming reminders"
- Each row: title (line limit 1), contact name, due date (red if overdue)

4. **AI Assistant Card** (`aiCard`)
5. **AI Assistant Card** (`aiCard`)
- Title: "AI Assistant", icon: `sparkles`, color: purple
- Contains a `NavigationLink` to `AIInsightsView`
- Label: "Get AI-powered outreach suggestions" with chevron

5. **Strongest Relationships Card** (`topContactsCard`)
6. **Strongest Relationships Card** (`topContactsCard`)
- Title: "Strongest Relationships", icon: `star.fill`, color: `strongGreen`
- Empty state: "Add contacts to see top relationships"
- Populated: ranked list (#1, #2, etc.) in `HStack(spacing: 12)` with rank label (`.caption.weight(.bold)`, 24pt frame), `ContactAvatarView(size: 36)`, name `.body.weight(.medium)`, `ScoreBadgeView`, `.padding(.vertical, 2)`
- Data from `viewModel.topContacts(from:)`

**Navigation:** `.navigationDestination(for: UUID.self)` resolves a contact id to `ContactDetailView` via the `contactsByID` map.

### DashboardCard (Reusable Component)

**File:** `Blackbook/Views/Dashboard/DashboardView.swift` (same file)
Expand Down
8 changes: 8 additions & 0 deletions Blackbook.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
1838B918B2D0E5BA722A3EF0 /* ServerMenuView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 107F0516F43606F715A04CAC /* ServerMenuView.swift */; };
1D1FA4DA1805F09735832F43 /* Group.swift in Sources */ = {isa = PBXBuildFile; fileRef = 59D5A4C8F3FE7AFE76757690 /* Group.swift */; };
1D5E32E9EE0B56EA56D2D7EC /* FeatureGating.swift in Sources */ = {isa = PBXBuildFile; fileRef = C0BF1D07C08681789AF0CC48 /* FeatureGating.swift */; };
1D99B1A030BE9CA3B7CF9713 /* ContactDeduplicationServiceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = BBCD54BD3218CA370A819170 /* ContactDeduplicationServiceTests.swift */; };
1E261EB5A02886029E363553 /* SyncTypesTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6169F3E4455CDCD77042D958 /* SyncTypesTests.swift */; };
1F15A9907F62206EA97F4555 /* NetworkGraphEngine.swift in Sources */ = {isa = PBXBuildFile; fileRef = 682862BCC7F0F257DF35CEB8 /* NetworkGraphEngine.swift */; };
25A80D3929B5841C1D224E29 /* TagModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 12AE2904F18728288E8C2218 /* TagModelTests.swift */; };
Expand Down Expand Up @@ -102,6 +103,7 @@
AFC41BDFEE097478165A2D0D /* ContactDetailViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 75F47D5E806EFA1DA2D5935A /* ContactDetailViewModelTests.swift */; };
B164427D17DE85E450AEA4C1 /* ActivityListView.swift in Sources */ = {isa = PBXBuildFile; fileRef = ACC2D7FC775E7A455AA78ABC /* ActivityListView.swift */; };
B383DE37B4755963A3BBD2FE /* SmartGroupsView.swift in Sources */ = {isa = PBXBuildFile; fileRef = A6DFE9C279FB07636F49D37A /* SmartGroupsView.swift */; };
B3D765199ED7FF84739621F0 /* NetworkGraphEngineTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F8CBD51D0C167CD7EB595D1F /* NetworkGraphEngineTests.swift */; };
B7A06A425624014C337FFB6E /* BiometricServiceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2AA93BA20CE88C3BC9C9317A /* BiometricServiceTests.swift */; };
B8795D426E8414EDC3028C87 /* ContactFormView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 98155954680EC712B26CB05D /* ContactFormView.swift */; };
B99B1B94442BAC565FEDAAA2 /* AIAssistantViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = A36AE5C06322D7E805B52341 /* AIAssistantViewModel.swift */; };
Expand Down Expand Up @@ -270,6 +272,7 @@
B388096991E8F365A624F046 /* BackupDetailView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BackupDetailView.swift; sourceTree = "<group>"; };
B56B474D15D62192EC7F6305 /* ModelSyncApply.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ModelSyncApply.swift; sourceTree = "<group>"; };
BB300BB5DF41935C07BC4210 /* ActivityFormView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ActivityFormView.swift; sourceTree = "<group>"; };
BBCD54BD3218CA370A819170 /* ContactDeduplicationServiceTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ContactDeduplicationServiceTests.swift; sourceTree = "<group>"; };
BBD49AF1F502FF3C26EC5CF1 /* CalendarPickerView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CalendarPickerView.swift; sourceTree = "<group>"; };
BEE4CD8BE91D0E8284B87F31 /* TagListView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TagListView.swift; sourceTree = "<group>"; };
C0BF1D07C08681789AF0CC48 /* FeatureGating.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeatureGating.swift; sourceTree = "<group>"; };
Expand All @@ -296,6 +299,7 @@
F49D100318268DC2B37475CA /* SyncTypes.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SyncTypes.swift; sourceTree = "<group>"; };
F767094E24F344D3643565AA /* LocationIconSuggestionView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LocationIconSuggestionView.swift; sourceTree = "<group>"; };
F8A766A33F57F94C9BDF7ACF /* ConstantsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ConstantsTests.swift; sourceTree = "<group>"; };
F8CBD51D0C167CD7EB595D1F /* NetworkGraphEngineTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NetworkGraphEngineTests.swift; sourceTree = "<group>"; };
F8F368EF8553F3DD542C10AE /* LocationDetailView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LocationDetailView.swift; sourceTree = "<group>"; };
FA40028F884B4B1161FA87B0 /* ContactMergeServiceTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ContactMergeServiceTests.swift; sourceTree = "<group>"; };
FA9E2FA5C2EA9DC5CC7C31F9 /* Constants.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Constants.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -509,6 +513,7 @@
2AA93BA20CE88C3BC9C9317A /* BiometricServiceTests.swift */,
EA38C3DA0A5DB92E7C98C0F0 /* ClaudeAPIServiceTests.swift */,
F8A766A33F57F94C9BDF7ACF /* ConstantsTests.swift */,
BBCD54BD3218CA370A819170 /* ContactDeduplicationServiceTests.swift */,
75F47D5E806EFA1DA2D5935A /* ContactDetailViewModelTests.swift */,
ECBCBC491C2BFAA0AABA68B3 /* ContactListViewModelTests.swift */,
FA40028F884B4B1161FA87B0 /* ContactMergeServiceTests.swift */,
Expand All @@ -522,6 +527,7 @@
367B4A2B6BD57C86106A9103 /* InteractionViewModelTests.swift */,
7DB7F634E85EAB2589E6F041 /* KeychainServiceTests.swift */,
E47D954BD6E7D82202705588 /* LocationModelTests.swift */,
F8CBD51D0C167CD7EB595D1F /* NetworkGraphEngineTests.swift */,
9111F21061CF4BB14E6FE866 /* NetworkGraphViewModelTests.swift */,
AD6C2B5FA5933483BFCEC3B4 /* NoteModelTests.swift */,
AB7B07D8F95B7197892AB8D6 /* RelationshipScoreEngineTests.swift */,
Expand Down Expand Up @@ -880,6 +886,7 @@
B7A06A425624014C337FFB6E /* BiometricServiceTests.swift in Sources */,
F7DD6583F920E520F4EEBE9D /* ClaudeAPIServiceTests.swift in Sources */,
E71E2B55D28997F49015FA8D /* ConstantsTests.swift in Sources */,
1D99B1A030BE9CA3B7CF9713 /* ContactDeduplicationServiceTests.swift in Sources */,
AFC41BDFEE097478165A2D0D /* ContactDetailViewModelTests.swift in Sources */,
F9C8E6C6BBBDE1468720591E /* ContactListViewModelTests.swift in Sources */,
28092BCCCBD27C677F28E65F /* ContactMergeServiceTests.swift in Sources */,
Expand All @@ -893,6 +900,7 @@
819102BFD7CD40272EA76C97 /* InteractionViewModelTests.swift in Sources */,
A303CE901664DED4348494BD /* KeychainServiceTests.swift in Sources */,
79377F20F80D93BE4ED30678 /* LocationModelTests.swift in Sources */,
B3D765199ED7FF84739621F0 /* NetworkGraphEngineTests.swift in Sources */,
FD484B077257640144869BCF /* NetworkGraphViewModelTests.swift in Sources */,
17FC135DAFA6AACBD52DFD7D /* NoteModelTests.swift in Sources */,
FD356F0646511FD58F59CCEF /* RelationshipScoreEngineTests.swift in Sources */,
Expand Down
1 change: 0 additions & 1 deletion Blackbook/ViewModels/ContactListViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import Foundation
import SwiftData
import SwiftUI

@Observable
final class ContactListViewModel {
Expand Down
192 changes: 192 additions & 0 deletions BlackbookTests/ContactDeduplicationServiceTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
import XCTest
import SwiftData
@testable import Blackbook

/// Tests for `ContactDeduplicationService` — the union-find duplicate detector that auto-merges
/// contacts. This service is safety-critical (it mutates the contact graph via
/// `ContactMergeService`) and was previously untested. These tests exercise the pure `findGroups`
/// scan and the `mergeAll` mutation path.
@MainActor
final class ContactDeduplicationServiceTests: XCTestCase {

private var container: ModelContainer!
private var context: ModelContext!
private let dedup = ContactDeduplicationService()
private let merger = ContactMergeService()

override func setUpWithError() throws {
container = try TestHelpers.makeContainer()
context = ModelContext(container)
}

override func tearDown() {
context = nil
container = nil
}

// MARK: - Helpers

@discardableResult
private func makeContact(
_ first: String,
_ last: String,
emails: [String] = [],
phones: [String] = []
) -> Contact {
let c = Contact(firstName: first, lastName: last)
c.emails = emails
c.phones = phones
context.insert(c)
return c
}

// MARK: - findGroups: negative cases

func testNoContactsYieldsNoGroups() throws {
XCTAssertTrue(try dedup.findGroups(in: context).isEmpty)
}

func testSingleContactYieldsNoGroups() throws {
makeContact("Ada", "Lovelace", emails: ["ada@x.com"])
try context.save()
XCTAssertTrue(try dedup.findGroups(in: context).isEmpty)
}

func testDistinctContactsAreNotGrouped() throws {
makeContact("Ada", "Lovelace", emails: ["ada@x.com"], phones: ["111-111-1111"])
makeContact("Alan", "Turing", emails: ["alan@x.com"], phones: ["222-222-2222"])
try context.save()
XCTAssertTrue(try dedup.findGroups(in: context).isEmpty)
}

// MARK: - findGroups: linking by each key

func testLinkBySameName() throws {
// nameKey folds case and diacritics (but does NOT trim per-component whitespace —
// see testNameKeyDoesNotTrimInnerWhitespace below).
makeContact("José", "García")
makeContact("JOSE", "garcia")
try context.save()
let groups = try dedup.findGroups(in: context)
XCTAssertEqual(groups.count, 1)
guard groups.count == 1 else { return }
XCTAssertEqual(groups[0].duplicates.count, 1)
XCTAssertTrue(groups[0].matchReason.contains("Same name"))
}

/// Documents a known limitation: `ContactSyncService.nameKey` trims only the *combined*
/// string's outer whitespace, not each component, so a stray inner space prevents a name
/// match. Captured as a regression guard; see review report for the proposed hardening.
func testNameKeyDoesNotTrimInnerWhitespace() throws {
makeContact("Grace", "Hopper")
makeContact("Grace", " Hopper") // leading space on last name → different key today
try context.save()
XCTAssertTrue(try dedup.findGroups(in: context).isEmpty,
"current behavior: inner whitespace is not normalized away")
}

func testLinkBySharedEmailDespiteDifferentNames() throws {
makeContact("Robert", "Smith", emails: ["shared@x.com"])
makeContact("Bob", "Smithe", emails: ["SHARED@x.com"]) // case-insensitive email match
try context.save()
let groups = try dedup.findGroups(in: context)
XCTAssertEqual(groups.count, 1)
XCTAssertTrue(groups[0].matchReason.contains("Shared email"))
}

func testLinkBySharedPhoneIgnoresFormatting() throws {
makeContact("Jenny", "A", phones: ["(415) 867-5309"])
makeContact("Jen", "B", phones: ["4158675309"]) // formatting stripped to same digits
try context.save()
let groups = try dedup.findGroups(in: context)
XCTAssertEqual(groups.count, 1)
XCTAssertTrue(groups[0].matchReason.contains("Shared phone"))
}

func testEmptyNameKeyDoesNotLink() throws {
// Two contacts with blank names must NOT be grouped on an empty name key.
makeContact("", "", emails: ["a@x.com"])
makeContact("", "", emails: ["b@x.com"])
try context.save()
XCTAssertTrue(try dedup.findGroups(in: context).isEmpty)
}

// MARK: - findGroups: transitive union-find correctness

func testTransitiveLinkingFormsSingleComponent() throws {
// A~B by name, B~C by email, C~D by phone → all four collapse into one group.
let a = makeContact("Sam", "Vimes")
let b = makeContact("Sam", "Vimes", emails: ["sv@watch.gov"])
let c = makeContact("Samuel", "V", emails: ["sv@watch.gov"], phones: ["555-0100"])
let d = makeContact("S", "Vimes2", phones: ["5550100"])
_ = (a, b, c, d)
try context.save()
let groups = try dedup.findGroups(in: context)
XCTAssertEqual(groups.count, 1)
XCTAssertEqual(groups[0].duplicates.count + 1, 4, "all four contacts should be one component")
}

func testTwoSeparateDuplicatePairsYieldTwoGroups() throws {
makeContact("Pair", "One")
makeContact("Pair", "One")
makeContact("Pair", "Two")
makeContact("Pair", "Two")
try context.save()
let groups = try dedup.findGroups(in: context)
XCTAssertEqual(groups.count, 2)
XCTAssertTrue(groups.allSatisfy { $0.duplicates.count == 1 })
}

// MARK: - findGroups: exclusions & primary selection

func testMergedAwayContactsAreExcludedFromScan() throws {
makeContact("Dup", "Erson")
let gone = makeContact("Dup", "Erson")
gone.isMergedAway = true
try context.save()
// Only one live contact with that name remains → no group.
XCTAssertTrue(try dedup.findGroups(in: context).isEmpty)
}

func testPrimaryIsRichestContact() throws {
let sparse = makeContact("Rich", "Card")
let rich = makeContact("Rich", "Card")
// Give `rich` more data so its dataRichness wins.
TestHelpers.makeInteraction(contact: rich, in: context)
TestHelpers.makeInteraction(contact: rich, in: context)
TestHelpers.makeNote(contact: rich, in: context)
rich.emails = ["rich@x.com"]
try context.save()
let groups = try dedup.findGroups(in: context)
XCTAssertEqual(groups.count, 1)
XCTAssertEqual(groups[0].primary.id, rich.id, "the data-richer contact must be primary")
XCTAssertEqual(groups[0].duplicates.first?.id, sparse.id)
}

// MARK: - mergeAll mutation path

func testMergeAllSuppressesSecondariesAndReturnsCount() throws {
let keep = makeContact("Merge", "Target")
TestHelpers.makeNote(contact: keep, in: context) // make `keep` the richer primary
let dupe = makeContact("Merge", "Target")
try context.save()

let merged = try dedup.mergeAll(using: merger, in: context)

XCTAssertEqual(merged, 1)
XCTAssertEqual(dedup.lastMergeCount, 1)
XCTAssertEqual(dedup.lastGroupCount, 1)
XCTAssertTrue(dupe.isMergedAway, "secondary should be suppressed, not deleted")
XCTAssertEqual(dupe.mergedIntoContact?.id, keep.id)
// A second pass finds nothing because the duplicate is now merged away.
XCTAssertTrue(try dedup.findGroups(in: context).isEmpty)
}

func testMergeAllOnCleanStoreIsNoOp() throws {
makeContact("Solo", "Contact", emails: ["solo@x.com"])
try context.save()
let merged = try dedup.mergeAll(using: merger, in: context)
XCTAssertEqual(merged, 0)
XCTAssertEqual(dedup.lastGroupCount, 0)
}
}
Loading
Loading