Sync apply / conflict-resolution unit tests (#43/#44 regression surface) - #46
Merged
Merged
Conversation
#43/#44) The sync-apply layer — ContactSyncApply.applyRemoteContact and ModelSyncApply.applyRemoteInteraction — is the exact code behind the recent drift (#44) and launch-crash (#43) incidents and had zero unit coverage. These functions are pure (payload dict + ModelContext), so no protocol seam or network mock is needed; "remote" payloads are built with the real contactToDict/interactionToDict serializers so ISO8601 formatting matches the parser. SyncApplyTests (11): - insert path: absent record inserted + marked synced; round-trip preserves name/emails/phones/score/priority; idempotent re-apply is a UUID upsert (no dup). - conflict resolution: newer remote overwrites; NEWER+PENDING local is protected from a stale remote; NEWER-but-SYNCED local IS clobbered by an older remote (regression guard documenting exactly why edits must flip syncStatus to .pending via markLocallyEdited — the #44 root cause). - malformed payloads: missing id / missing updatedAt are ignored without throwing. - interactions: link to resolved contact; unknown contactId persists with nil contact (heals later) instead of crashing; newer pending local survives stale remote. Network-path integration (URLSession injection into LocalServerSyncService) is the remaining step noted in docs/CODE_REVIEW_2026-06-02.md finding #6. Test count 206 -> 217 XCTest (+ 13 Swift Testing), all green iOS + macOS. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Acts on finding #6 of
docs/CODE_REVIEW_2026-06-02.md: the sync-apply layer that caused this week's drift (#44) and launch-crash (#43) incidents had zero unit coverage.ContactSyncApply.applyRemoteContactandModelSyncApply.applyRemoteInteractionare pure functions (payload[String: Any]+ModelContext), so this needs no protocol seam or network mock — "remote" payloads are built with the realcontactToDict/interactionToDictserializers so ISO8601 formatting matches the parser exactly.SyncApplyTests(11 tests)synced; round-trip preserves name/emails/phones/score/priority; idempotent re-apply is a UUID upsert (no duplicate).syncedlocal is clobbered by an older remote — the regression guard that documents exactly why edits must flipsyncStatusto.pending(markLocallyEdited), i.e. the Fix sync drift: mark every Contact edit as locally edited + filter merged-away from iMessage lookup #44 root cause.id/ missingupdatedAtignored without throwing.contactIdpersists withnilcontact (heals on a later pull) instead of crashing; newer pending local survives a stale remote.Not in scope (noted for later)
Full network-path integration (URLSession injection into
LocalServerSyncService) — the further half of finding #6.Test plan
xcodegen generateregistered the new file🤖 Generated with Claude Code