commit 2329fba9234266b08cd5288f10f2339bc5a330f5
parent adef3fa3892c2cf2567ee8b79b181e13033b4ab6
Author: Michael Camilleri <[email protected]>
Date: Sun, 24 May 2026 06:53:04 +0900
Coalesce Player-record sends and log send success correctly
A diagnostics log review of a normal puzzle-open flow showed five separate
'private sent: 1 saved (Player=1)' lines within six seconds with no user input,
and a 'Last Success' timestamp that ignored every one of them. This commit
addresses this in two ways.
First, enqueuePlayerRecord now takes a `reason: String` that is traced at
enqueue time, and the outbound drain is routed through a 400 ms per-scope
coalescing window (CKSyncEngine state already dedupes repeated .saveRecord for
the same record ID, so several writes in the same tick ride one round-trip
instead of one round-trip each). Reasons distinguish PlayerSelectionPublisher
('selection'), PlayerNamePublisher's open vs. rename paths ('name-open',
'name-rename'), and the two publishReadCursor modes ('readCursor(activeLease)',
'readCursor(currentTime)').
Second, a new SyncMonitor.noteSuccess() now fires from the three delegate
handlers (handleFetchedDatabaseChanges, handleFetchedRecordZoneChanges and
handleSentRecordZoneChanges when failedRecordSaves is empty) via a
successCheckpoint callback wired in AppServices alongside the existing
tracer. Error-clearing stays with recordSuccess(_ phase:) — it relies on the
failing/recovering phase names matching, which a generic delegate event cannot
provide.
Co-Authored-By: Claude Opus 4.7 <[email protected]>
Diffstat:
6 files changed, 105 insertions(+), 16 deletions(-)
diff --git a/Crossmate/Services/AppServices.swift b/Crossmate/Services/AppServices.swift
@@ -208,7 +208,11 @@ final class AppServices {
sink: { gameID, authorID in
let isEnabled = await MainActor.run { preferences.isICloudSyncEnabled }
guard isEnabled else { return }
- await syncEngine.enqueuePlayerRecord(gameID: gameID, authorID: authorID)
+ await syncEngine.enqueuePlayerRecord(
+ gameID: gameID,
+ authorID: authorID,
+ reason: "selection"
+ )
}
)
self.friendController = FriendController(
@@ -258,6 +262,10 @@ final class AppServices {
syncMonitor.note(message)
}
+ await syncEngine.setSuccessCheckpoint { [syncMonitor] in
+ syncMonitor.noteSuccess()
+ }
+
await syncEngine.setLocalAuthorIDProvider { [identity] in
identity.currentID
}
@@ -383,10 +391,14 @@ final class AppServices {
preferences: preferences,
persistence: persistence,
authorIdentity: identity,
- enqueuePlayerRecord: { [preferences, syncEngine] gameID, authorID in
+ enqueuePlayerRecord: { [preferences, syncEngine] gameID, authorID, reason in
let isEnabled = await MainActor.run { preferences.isICloudSyncEnabled }
guard isEnabled else { return }
- await syncEngine.enqueuePlayerRecord(gameID: gameID, authorID: authorID)
+ await syncEngine.enqueuePlayerRecord(
+ gameID: gameID,
+ authorID: authorID,
+ reason: reason
+ )
}
)
@@ -1621,7 +1633,16 @@ final class AppServices {
didUpdate = store.setReadCursor(gameID: gameID, readAt: now)
}
guard didUpdate else { return }
- await syncEngine.enqueuePlayerRecord(gameID: gameID, authorID: authorID)
+ let reason: String
+ switch mode {
+ case .activeLease: reason = "readCursor(activeLease)"
+ case .currentTime: reason = "readCursor(currentTime)"
+ }
+ await syncEngine.enqueuePlayerRecord(
+ gameID: gameID,
+ authorID: authorID,
+ reason: reason
+ )
}
/// Sets the app icon badge to the number of shared games with unread
diff --git a/Crossmate/Services/DebuggingMonitors.swift b/Crossmate/Services/DebuggingMonitors.swift
@@ -252,6 +252,17 @@ final class SyncMonitor {
log.note(message)
}
+ /// Bumps `lastSuccessAt` without writing a `succeeded` log line. Used by
+ /// the CKSyncEngine delegate handlers so that auto-drained sends and
+ /// framework-initiated fetches — which never go through `run(_:_:)` —
+ /// still count as "the engine is alive and round-tripping with the
+ /// server". Without this the timestamp pins to whichever instrumented
+ /// phase last ran and silently lies about engine health when only
+ /// auto-drain activity is happening.
+ func noteSuccess() {
+ lastSuccessAt = Date()
+ }
+
func updateSnapshot(_ snapshot: SyncEngine.DiagnosticSnapshot) {
self.snapshot = snapshot
}
diff --git a/Crossmate/Services/PlayerNamePublisher.swift b/Crossmate/Services/PlayerNamePublisher.swift
@@ -14,7 +14,7 @@ final class PlayerNamePublisher {
private let preferences: PlayerPreferences
private let persistence: PersistenceController
private let authorIdentity: AuthorIdentity
- private let enqueuePlayerRecord: (UUID, String) async -> Void
+ private let enqueuePlayerRecord: (UUID, String, String) async -> Void
private var debounceTask: Task<Void, Never>?
private var observationTask: Task<Void, Never>?
@@ -27,7 +27,7 @@ final class PlayerNamePublisher {
preferences: PlayerPreferences,
persistence: PersistenceController,
authorIdentity: AuthorIdentity,
- enqueuePlayerRecord: @escaping (UUID, String) async -> Void
+ enqueuePlayerRecord: @escaping (UUID, String, String) async -> Void
) {
self.preferences = preferences
self.persistence = persistence
@@ -89,7 +89,7 @@ final class PlayerNamePublisher {
name: preferences.name
) else { return }
- await enqueuePlayerRecord(gameID, authorID)
+ await enqueuePlayerRecord(gameID, authorID, "name-open")
}
private func fanOut(newName: String) async {
@@ -104,7 +104,7 @@ final class PlayerNamePublisher {
)
for gameID in touchedGameIDs {
- await enqueuePlayerRecord(gameID, authorID)
+ await enqueuePlayerRecord(gameID, authorID, "name-rename")
}
onFanOutForTesting?(newName)
diff --git a/Crossmate/Sync/SyncEngine.swift b/Crossmate/Sync/SyncEngine.swift
@@ -104,6 +104,12 @@ actor SyncEngine {
var onIncomingReadCursor: (@MainActor @Sendable ([(UUID, Date)]) async -> Void)?
private var localAuthorIDProvider: (@MainActor @Sendable () -> String?)?
private var tracer: (@MainActor @Sendable (String) -> Void)?
+ /// Fires when a delegate event reports a successful round-trip with the
+ /// CloudKit server (fetched DB changes, fetched zone changes, or sent
+ /// zone changes with no failures). Bumps `Last Success` in the
+ /// diagnostics view so the timestamp reflects actual engine activity
+ /// rather than only instrumented phases run through `SyncMonitor.run`.
+ private var successCheckpoint: (@MainActor @Sendable () -> Void)?
var liveQueryCheckpoints: [String: Date] = [:]
let liveQueryCheckpointOverlap: TimeInterval = 5
@@ -126,6 +132,15 @@ actor SyncEngine {
tracer = t
}
+ func setSuccessCheckpoint(_ cb: @MainActor @Sendable @escaping () -> Void) {
+ successCheckpoint = cb
+ }
+
+ private func noteRoundTripSuccess() async {
+ guard let successCheckpoint else { return }
+ await successCheckpoint()
+ }
+
func setOnRemoteMovesUpdated(_ cb: @MainActor @Sendable @escaping (Set<UUID>) async -> Void) {
onRemoteMovesUpdated = cb
}
@@ -271,6 +286,34 @@ actor SyncEngine {
Task.detached { [engine] in try? await engine.sendChanges() }
}
+ /// Per-scope debounce slot for the coalesced Player-record drain.
+ /// Keyed by `GameEntity.databaseScope` (0 = private, 1 = shared).
+ private var playerSendDebounce: [Int16: Task<Void, Never>] = [:]
+ /// How long to wait after the first Player enqueue before firing
+ /// `sendChanges`. Long enough to absorb a puzzle-open burst (read cursor
+ /// + name + cursor track land within ~500 ms), short enough that a true
+ /// solo edit doesn't perceptibly defer its push.
+ private static let playerSendCoalesceWindow: Duration = .milliseconds(400)
+
+ /// Coalescing variant of `sendChangesDetached` for the Player-record
+ /// path. The first enqueue in a window schedules a deferred drain;
+ /// subsequent enqueues for the same scope are absorbed into that same
+ /// drain (CKSyncEngine state dedupes repeated `.saveRecord` for the same
+ /// record ID, and one `sendChanges` ships them all in a single batch).
+ private func scheduleCoalescedPlayerSend(on engine: CKSyncEngine, scope: Int16) {
+ if playerSendDebounce[scope] != nil { return }
+ let window = Self.playerSendCoalesceWindow
+ playerSendDebounce[scope] = Task.detached { [engine, weak self] in
+ try? await Task.sleep(for: window)
+ await self?.clearPlayerSendDebounce(scope: scope)
+ try? await engine.sendChanges()
+ }
+ }
+
+ private func clearPlayerSendDebounce(scope: Int16) {
+ playerSendDebounce.removeValue(forKey: scope)
+ }
+
// MARK: - Outbound
/// Registers each game's local-device Moves record as a pending save and
@@ -638,10 +681,15 @@ actor SyncEngine {
sendChangesDetached(on: engine)
}
- /// Registers a Player record as a pending send. Used by `PlayerNamePublisher`
- /// when the local user renames; one record per (game, authorID), so
- /// participants only ever write their own slot.
- func enqueuePlayerRecord(gameID: UUID, authorID: String) {
+ /// Registers a Player record as a pending send. One record per
+ /// (game, authorID), so participants only ever write their own slot.
+ /// `reason` is logged so the diagnostics view can attribute each enqueue
+ /// to its caller (rename, read cursor, cursor track, …); it does not
+ /// affect routing. The outbound drain is coalesced via
+ /// `scheduleCoalescedPlayerSend` so several enqueues in the same tick
+ /// (puzzle-open fans out name + read cursor + cursor track in <1s) ship
+ /// as one batch instead of one round-trip per record.
+ func enqueuePlayerRecord(gameID: UUID, authorID: String, reason: String) async {
let ctx = persistence.container.newBackgroundContext()
guard let info = zoneInfo(forGameID: gameID, in: ctx) else { return }
let engine = info.scope == 1 ? sharedEngine : privateEngine
@@ -649,7 +697,8 @@ actor SyncEngine {
let recordName = RecordSerializer.recordName(forPlayerInGame: gameID, authorID: authorID)
let recordID = CKRecord.ID(recordName: recordName, zoneID: info.zoneID)
engine.state.add(pendingRecordZoneChanges: [.saveRecord(recordID)])
- sendChangesDetached(on: engine)
+ await trace("enqueue Player[\(gameID.uuidString.prefix(8))] reason=\(reason)")
+ scheduleCoalescedPlayerSend(on: engine, scope: info.scope)
}
/// Registers a Game record as a pending send and ensures its zone is
@@ -782,6 +831,7 @@ actor SyncEngine {
"\(isPrivate ? "private" : "shared") db changes [src=\(src)]: " +
"\(event.modifications.count) zone mods, \(event.deletions.count) zone deletions"
)
+ await noteRoundTripSuccess()
// Private-DB zone deletions reflect the user removing one of their own
// games on another device — hard-delete locally so the row stops
@@ -882,6 +932,7 @@ actor SyncEngine {
"\(isPrivate ? "private" : "shared") fetch [src=\(src)]: " +
"\(event.modifications.count) modifications, \(event.deletions.count) deletions"
)
+ await noteRoundTripSuccess()
if !isPrivate, !loggedFirstSharedPushPayload, src == "push",
event.modifications.count + event.deletions.count > 0 {
loggedFirstSharedPushPayload = true
@@ -1086,6 +1137,12 @@ actor SyncEngine {
"\(event.deletedRecordIDs.count) deleted" +
"\(deletedSummary.isEmpty ? "" : " (\(deletedSummary))")"
)
+ // Only bump on a clean batch: a round trip with per-record failures
+ // is not "success" from the user's point of view, and the existing
+ // SyncMonitor.recordError path owns reporting whatever did break.
+ if event.failedRecordSaves.isEmpty {
+ await noteRoundTripSuccess()
+ }
for record in event.savedRecords {
let name = record.recordID.recordName
if name.hasPrefix("ping-") {
diff --git a/Tests/Unit/PlayerNamePublisherTests.swift b/Tests/Unit/PlayerNamePublisherTests.swift
@@ -73,7 +73,7 @@ struct PlayerNamePublisherTests {
preferences: preferences,
persistence: persistence,
authorIdentity: AuthorIdentity(testing: authorID),
- enqueuePlayerRecord: { _, _ in }
+ enqueuePlayerRecord: { _, _, _ in }
)
}
@@ -184,7 +184,7 @@ struct PlayerNamePublisherTests {
preferences: prefs,
persistence: p,
authorIdentity: AuthorIdentity(testing: "_local"),
- enqueuePlayerRecord: { gameID, _ in enqueued.append(gameID) }
+ enqueuePlayerRecord: { gameID, _, _ in enqueued.append(gameID) }
)
await broadcaster.publishName(for: secondID)
diff --git a/Tests/Unit/Sync/ZoneOrphaningTests.swift b/Tests/Unit/Sync/ZoneOrphaningTests.swift
@@ -93,7 +93,7 @@ struct ZoneOrphaningTests {
let (gameID, zoneName, owner) = try makeSharedGame(in: ctx)
let engine = await makeEngine(persistence: persistence)
- await engine.enqueuePlayerRecord(gameID: gameID, authorID: "_localAuthor")
+ await engine.enqueuePlayerRecord(gameID: gameID, authorID: "_localAuthor", reason: "test")
let beforeNames = await engine.pendingSaveRecordNames(scope: .shared)
let playerRecordName = RecordSerializer.recordName(forPlayerInGame: gameID, authorID: "_localAuthor")
#expect(beforeNames.contains(playerRecordName))
