commit 759dc2e4f9c78c356a774f54c57e583b0bc4693b
parent c9bdc969b929a91466e0b8753dedb389a68c74a3
Author: Michael Camilleri <[email protected]>
Date: Wed, 10 Jun 2026 11:58:57 +0900
Split the read watermark from the presence lease
A single field, Player.readAt (locally GameEntity.lastReadOtherMoveAt),
served two incompatible roles: a forward-dated presence lease (now + 10
min) that drives PeerPresence, and a read watermark that the session-end
push and unread badge treat as 'seen through T'.
The forward-dating works while the user is looking at the puzzle but
breaks the moment the user stops: the lease stays in the future for up
to ten minutes after backgrounding. A peer computing what the user
missed sees that future readAt, concludes the user has already read the
later moves, and suppresses the 'stopped solving' summary push. The
lease frequently doesn't collapse cleanly on background either, so the
watermark has to be its own value.
This commit adds a true read watermark, never forward-dated:
- GameEntity.readThroughAt / PlayerEntity.readThrough (+ wire field
readThrough on the Player record).
- Watermark writers target readThroughAt: markOtherMovesRead (open),
the noteIncomingMovesUpdate lockstep (viewing), and a new
advanceReadThrough on .currentTime leave. Monotonic, never future.
- readAt / lastReadOtherMoveAt keep their exact meaning -- the presence
lease -- so presence behaviour is unchanged.
- Push summary windows on the peer's readThrough (pushPlan ->
PushRecipient.readThrough -> SessionPushPlanner cutoff).
- Unread badge keys off readThroughAt in both computeHasUnread and the
shared unreadOtherMovesPredicate, so moves that land after backgrounding
now badge instead of being swallowed.
- RecordApplier converges our the user watermark across sibling devices.
readAt is now purely a presence horizon. It is noted that the name will
be changed a more clearly presence-related name when the schema version
is next bumped.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
Diffstat:
10 files changed, 226 insertions(+), 84 deletions(-)
diff --git a/Crossmate/Models/CrossmateModel.xcdatamodeld/CrossmateModel.xcdatamodel/contents b/Crossmate/Models/CrossmateModel.xcdatamodeld/CrossmateModel.xcdatamodel/contents
@@ -25,6 +25,7 @@
<attribute name="lastReadOtherMoveAt" optional="YES" attributeType="Date" usesScalarValueType="NO" renamingIdentifier="lastSeenOtherMoveAt"/>
<attribute name="lastSyncedAt" optional="YES" attributeType="Date" usesScalarValueType="NO"/>
<attribute name="latestOtherMoveAt" optional="YES" attributeType="Date" usesScalarValueType="NO"/>
+ <attribute name="readThroughAt" optional="YES" attributeType="Date" usesScalarValueType="NO"/>
<attribute name="puzzleResourceID" optional="YES" attributeType="String"/>
<attribute name="hasPushPending" attributeType="Boolean" defaultValueString="NO" usesScalarValueType="YES"/>
<attribute name="puzzleCmVersion" optional="YES" attributeType="Integer 64" defaultValueString="0" usesScalarValueType="YES"/>
@@ -46,6 +47,7 @@
<attribute name="notifiedThrough" optional="YES" attributeType="Date" usesScalarValueType="NO"/>
<attribute name="pushAddress" optional="YES" attributeType="String"/>
<attribute name="readAt" optional="YES" attributeType="Date" usesScalarValueType="NO"/>
+ <attribute name="readThrough" optional="YES" attributeType="Date" usesScalarValueType="NO"/>
<attribute name="selCol" optional="YES" attributeType="Integer 64" usesScalarValueType="NO"/>
<attribute name="selDir" optional="YES" attributeType="Integer 64" usesScalarValueType="NO"/>
<attribute name="selRow" optional="YES" attributeType="Integer 64" usesScalarValueType="NO"/>
diff --git a/Crossmate/Persistence/GameStore.swift b/Crossmate/Persistence/GameStore.swift
@@ -113,7 +113,11 @@ struct GameSummary: Identifiable, Equatable {
isShared: self.isShared,
completedAt: entity.completedAt,
latest: entity.latestOtherMoveAt,
- lastRead: entity.lastReadOtherMoveAt
+ // The unread badge keys off the read *watermark*, not the presence
+ // lease. The lease (`lastReadOtherMoveAt`) is forward-dated while
+ // present, which used to suppress the badge for ~10 min after the
+ // user backgrounded — swallowing moves that arrived in that window.
+ readThrough: entity.readThroughAt
)
}
@@ -121,11 +125,11 @@ struct GameSummary: Identifiable, Equatable {
isShared: Bool,
completedAt: Date?,
latest: Date?,
- lastRead: Date?
+ readThrough: Date?
) -> Bool {
guard isShared, completedAt == nil, let latest else { return false }
- guard let lastRead else { return true }
- return latest > lastRead
+ guard let readThrough else { return true }
+ return latest > readThrough
}
}
@@ -151,7 +155,7 @@ final class GameSummaryCache {
let updatedAt: Date?
let completedAt: Date?
let latestOther: Date?
- let lastReadOther: Date?
+ let readThrough: Date?
let scope: Int16
let shareName: String?
let revoked: Bool
@@ -163,7 +167,7 @@ final class GameSummaryCache {
updatedAt: entity.updatedAt,
completedAt: entity.completedAt,
latestOther: entity.latestOtherMoveAt,
- lastReadOther: entity.lastReadOtherMoveAt,
+ readThrough: entity.readThroughAt,
scope: entity.databaseScope,
shareName: entity.ckShareRecordName,
revoked: entity.isAccessRevoked
@@ -364,13 +368,15 @@ final class GameStore {
// even when the user is sitting on the library list, suppressing
// the badge that should appear there.
// Suppressed = viewing here (incl. the local leave-grace) — the
- // user has eyes on these moves, so keep the read horizon at
- // least caught up. Do not shorten an active future read lease.
- // Sibling devices learn about the read via `Player.readAt`;
- // AppServices publishes the active read lease after this merge.
+ // user has eyes on these moves, so advance the *read watermark*
+ // (`readThroughAt`) to what they're looking at. This is monotonic
+ // and never forward-dated, so it cannot claim moves that arrive
+ // after the user backgrounds. Sibling devices learn about the read
+ // via `Player.readThrough`; the forward-dated presence lease
+ // (`lastReadOtherMoveAt`) is published separately by AppServices.
if NotificationState.isSuppressed(gameID: gameID),
- (entity.lastReadOtherMoveAt ?? .distantPast) < latest {
- entity.lastReadOtherMoveAt = latest
+ (entity.readThroughAt ?? .distantPast) < latest {
+ entity.readThroughAt = latest
}
}
@@ -529,11 +535,15 @@ final class GameStore {
}
private var unreadOtherMovesPredicate: NSPredicate {
+ // Keyed off the read *watermark* (`readThroughAt`), not the forward-
+ // dated presence lease (`lastReadOtherMoveAt`) — matches
+ // `GameSummary.computeHasUnread`. Using the lease here suppressed the
+ // badge for moves that arrived after the user backgrounded.
NSPredicate(
format: "(databaseScope == 1 OR ckShareRecordName != nil) "
+ "AND completedAt == nil "
+ "AND latestOtherMoveAt != nil "
- + "AND (lastReadOtherMoveAt == nil OR latestOtherMoveAt > lastReadOtherMoveAt)"
+ + "AND (readThroughAt == nil OR latestOtherMoveAt > readThroughAt)"
)
}
@@ -989,10 +999,19 @@ final class GameStore {
_ = setReadCursor(gameID: gameID, readAt: readAt)
}
- /// Sets the per-account read horizon for other-author moves. When
- /// `minimumExistingReadAt` is provided, the write is skipped if the
- /// current horizon already reaches that floor; active sessions use this
- /// to refresh a future read lease only when it is getting close to expiry.
+ /// Sets the per-account **presence lease** for `gameID` — the forward-dated
+ /// "the user is actively present on this puzzle" horizon stored in
+ /// `lastReadOtherMoveAt`. When `minimumExistingReadAt` is provided, the
+ /// write is skipped if the current lease already reaches that floor; active
+ /// sessions use this to refresh a future lease only when it is close to
+ /// expiry.
+ ///
+ /// This is *not* the read watermark — see `advanceReadThrough`. The two were
+ /// historically the same field, which is why `lastReadOtherMoveAt` ships on
+ /// the wire as `Player.readAt`.
+ /// TODO(v4): rename the `readAt` CloudKit field (and `lastReadOtherMoveAt`)
+ /// to something like `presenceLeaseUntil` — it is a presence horizon, not a
+ /// read position, now that `readThrough` carries the latter.
@discardableResult
func setReadCursor(
gameID: UUID,
@@ -1017,11 +1036,36 @@ final class GameStore {
return true
}
+ /// Advances the per-account **read watermark** (`readThroughAt`) to
+ /// `through`, monotonically — the latest other-author move time this
+ /// account has actually observed. Unlike the presence lease it is never
+ /// forward-dated, so a peer computing what we've seen (and our own unread
+ /// badge) never credits us with moves made after we stopped looking.
+ /// Returns `true` if the watermark moved.
+ @discardableResult
+ func advanceReadThrough(gameID: UUID, through: Date) -> Bool {
+ let request = NSFetchRequest<GameEntity>(entityName: "GameEntity")
+ request.predicate = NSPredicate(format: "id == %@", gameID as CVarArg)
+ request.fetchLimit = 1
+ guard let entity = try? context.fetch(request).first else { return false }
+ let isShared = entity.ckShareRecordName != nil || entity.databaseScope == 1
+ guard isShared else { return false }
+ guard (entity.readThroughAt ?? .distantPast) < through else { return false }
+ entity.readThroughAt = through
+ saveContext("advanceReadThrough")
+ onUnreadOtherMovesChanged?()
+ return true
+ }
+
private func markOtherMovesRead(for entity: GameEntity) {
let isShared = entity.ckShareRecordName != nil || entity.databaseScope == 1
guard isShared, let latest = entity.latestOtherMoveAt else { return }
- if (entity.lastReadOtherMoveAt ?? .distantPast) < latest {
- entity.lastReadOtherMoveAt = latest
+ // Advances the *read watermark* (`readThroughAt`), not the presence
+ // lease (`lastReadOtherMoveAt`). Opening the game means the user has now
+ // seen every other-author move up to `latest`; the lease is a separate,
+ // forward-dated "actively present" horizon owned by `setReadCursor`.
+ if (entity.readThroughAt ?? .distantPast) < latest {
+ entity.readThroughAt = latest
saveContext("markOtherMovesRead")
onUnreadOtherMovesChanged?()
}
diff --git a/Crossmate/Services/AppServices.swift b/Crossmate/Services/AppServices.swift
@@ -1351,7 +1351,13 @@ final class AppServices {
/// callers consult before emitting.
struct PushRecipient: Sendable, Equatable {
let authorID: String
- let readAt: Date?
+ /// The recipient's read watermark (`Player.readThrough`): the latest
+ /// other-author move time they've actually seen. The session-end tally
+ /// windows on this — never the forward-dated presence lease — so a peer
+ /// who was "present" (leased) but backgrounded before our moves still
+ /// gets a summary for what they missed. `nil` when they've recorded no
+ /// read yet, which tallies their whole backlog.
+ let readThrough: Date?
/// Sender-local watermark: the latest authored move we've already told
/// this recipient about via a previous pause. The session-end tally
/// windows on the later of this and `readAt`, so we never re-report a
@@ -1390,7 +1396,7 @@ final class AppServices {
gReq.predicate = NSPredicate(format: "id == %@", gameID as CVarArg)
gReq.fetchLimit = 1
guard let game = try? ctx.fetch(gReq).first else { return .empty }
- var byAuthor: [String: (readAt: Date?, notifiedThrough: Date?, pushAddress: String?)] = [:]
+ var byAuthor: [String: (readThrough: Date?, notifiedThrough: Date?, pushAddress: String?)] = [:]
let pReq = NSFetchRequest<PlayerEntity>(entityName: "PlayerEntity")
pReq.predicate = NSPredicate(format: "game == %@", game)
for p in (try? ctx.fetch(pReq)) ?? [] {
@@ -1399,12 +1405,12 @@ final class AppServices {
!a.isEmpty
else { continue }
if let authorID, a == authorID { continue }
- byAuthor[a] = (p.readAt, p.notifiedThrough, p.pushAddress)
+ byAuthor[a] = (p.readThrough, p.notifiedThrough, p.pushAddress)
}
let recipients = byAuthor.map {
PushRecipient(
authorID: $0.key,
- readAt: $0.value.readAt,
+ readThrough: $0.value.readThrough,
notifiedThrough: $0.value.notifiedThrough,
pushAddress: $0.value.pushAddress
)
@@ -3307,7 +3313,14 @@ final class AppServices {
await publishAccountSeenPush(gameID: gameID, readAt: readAt)
}
case .currentTime:
- didUpdate = store.setReadCursor(gameID: gameID, readAt: now)
+ // Leaving / backgrounding: collapse the presence lease to now and,
+ // in lockstep, advance the read watermark to now — the user was
+ // looking right up to here, so they've seen everything through now.
+ // The watermark write is what stops a peer re-summarising moves we
+ // saw live just before leaving; it never reaches into the future.
+ let collapsed = store.setReadCursor(gameID: gameID, readAt: now)
+ let advanced = store.advanceReadThrough(gameID: gameID, through: now)
+ didUpdate = collapsed || advanced
}
guard didUpdate else { return }
let reason: String
diff --git a/Crossmate/Services/SessionPushPlanner.swift b/Crossmate/Services/SessionPushPlanner.swift
@@ -52,7 +52,7 @@ enum SessionPushPlanner {
/// device's recorded moves, in seq order). Counts are derived from it —
/// not the merged grid — so the summary can name *gestures*: net letter
/// `fills` / `clears` plus the number of `check` / `reveal` gestures, each
- /// windowed to entries newer than the recipient's `readAt`. The journal is
+ /// windowed to entries newer than the recipient's `readThrough`. The journal is
/// this device's only, so a session run on another of the author's devices
/// is described by *that* device's own pause; "eventual consistency is OK"
/// covers the gap.
@@ -74,13 +74,15 @@ enum SessionPushPlanner {
return recipients.compactMap { recipient -> PushClient.Addressee? in
guard let address = recipient.pushAddress else { return nil }
- // Window on the later of what the recipient has read in-app and
- // what we've already notified them about. `readAt` alone misses a
- // recipient who stayed backgrounded across two of our pauses — it
- // never advances, so the second pause would re-tally the same
- // moves. The watermark closes that: a session that added nothing
- // new tallies to zero ("stopped solving") rather than repeating.
- let cutoff = [recipient.readAt, recipient.notifiedThrough]
+ // Window on the later of what the recipient has actually read
+ // (`readThrough`) and what we've already notified them about
+ // (`notifiedThrough`). `readThrough` alone misses a recipient who
+ // stayed backgrounded across two of our pauses — it never advances,
+ // so the second pause would re-tally the same moves. The
+ // notified-through watermark closes that: a session that added
+ // nothing new tallies to zero ("stopped solving") rather than
+ // repeating.
+ let cutoff = [recipient.readThrough, recipient.notifiedThrough]
.compactMap { $0 }
.max()
let counts = tally(history: history, since: cutoff ?? .distantPast)
diff --git a/Crossmate/Sync/CloudQuery.swift b/Crossmate/Sync/CloudQuery.swift
@@ -408,7 +408,7 @@ extension SyncEngine {
database: database,
zoneID: zone.zoneID,
since: since,
- desiredKeys: ["authorID", "name", "updatedAt", "selRow", "selCol", "selDir", "readAt", "sessionSnapshot", "pushAddress"]
+ desiredKeys: ["authorID", "name", "updatedAt", "selRow", "selCol", "selDir", "readAt", "readThrough", "sessionSnapshot", "pushAddress"]
)
let activities = playerRecords.compactMap { record in
Session.parseRecord(record, puzzleTitle: zone.title)
@@ -568,7 +568,7 @@ extension SyncEngine {
database: database,
zoneID: zoneID,
since: nil,
- desiredKeys: ["authorID", "name", "updatedAt", "selRow", "selCol", "selDir", "readAt", "sessionSnapshot", "pushAddress"]
+ desiredKeys: ["authorID", "name", "updatedAt", "selRow", "selCol", "selDir", "readAt", "readThrough", "sessionSnapshot", "pushAddress"]
)
let (m, p) = try await (moves, players)
return PerZoneResult(records: games + m + p, hasGame: true)
@@ -675,7 +675,7 @@ extension SyncEngine {
database: database,
zoneID: info.zoneID,
since: since,
- desiredKeys: ["authorID", "name", "updatedAt", "selRow", "selCol", "selDir", "readAt", "sessionSnapshot", "pushAddress"]
+ desiredKeys: ["authorID", "name", "updatedAt", "selRow", "selCol", "selDir", "readAt", "readThrough", "sessionSnapshot", "pushAddress"]
)
(gameResults, moves, players) = try await (gameResultsTask, movesTask, playersTask)
} catch {
diff --git a/Crossmate/Sync/RecordApplier.swift b/Crossmate/Sync/RecordApplier.swift
@@ -237,10 +237,22 @@ extension SyncEngine {
// lowers it once no sibling leases above it.
let incomingReadAt = RecordSerializer.parsePlayerReadAt(from: record)
entity.readAt = incomingReadAt
+ let incomingReadThrough = RecordSerializer.parsePlayerReadThrough(from: record)
+ entity.readThrough = incomingReadThrough
entity.sessionSnapshot = RecordSerializer.parsePlayerSessionSnapshot(from: record)
if authorID == localAuthorID, let readAt = incomingReadAt {
onReadCursor(gameID, readAt, entity.sessionSnapshot)
}
+ // Our own record coming back from another of this account's devices
+ // carries that device's read watermark. Adopt it monotonically onto the
+ // shared game so the unread badge clears here once any device has read,
+ // mirroring how the presence lease converges via `onReadCursor` above.
+ if authorID == localAuthorID,
+ let incomingReadThrough,
+ let game = entity.game,
+ (game.readThroughAt ?? .distantPast) < incomingReadThrough {
+ game.readThroughAt = incomingReadThrough
+ }
// The remaining value fields are only adopted when the incoming record
// is at least as new as what we have locally; otherwise a stale-but-
diff --git a/Crossmate/Sync/RecordBuilder.swift b/Crossmate/Sync/RecordBuilder.swift
@@ -133,6 +133,7 @@ extension SyncEngine {
updatedAt: updatedAt,
selection: selection,
readAt: entity.game?.lastReadOtherMoveAt,
+ readThrough: entity.game?.readThroughAt,
sessionSnapshot: entity.sessionSnapshot,
pushAddress: entity.pushAddress,
zone: zoneID,
diff --git a/Crossmate/Sync/RecordSerializer.swift b/Crossmate/Sync/RecordSerializer.swift
@@ -379,6 +379,7 @@ enum RecordSerializer {
updatedAt: Date,
selection: PlayerSelection?,
readAt: Date? = nil,
+ readThrough: Date? = nil,
sessionSnapshot: Data? = nil,
pushAddress: String? = nil,
zone: CKRecordZone.ID,
@@ -404,11 +405,21 @@ enum RecordSerializer {
record["selCol"] = nil
record["selDir"] = nil
}
+ // `readAt` is the forward-dated presence lease (see `GameStore`'s
+ // TODO(v4): this field should be renamed to a presence name).
if let readAt {
record["readAt"] = readAt as CKRecordValue
} else {
record["readAt"] = nil
}
+ // `readThrough` is the true read watermark — the latest other-author
+ // move time this account has actually seen. Never forward-dated, so a
+ // peer's session-end summary windows only on moves we genuinely missed.
+ if let readThrough {
+ record["readThrough"] = readThrough as CKRecordValue
+ } else {
+ record["readThrough"] = nil
+ }
if let sessionSnapshot {
record["sessionSnapshot"] = sessionSnapshot as CKRecordValue
} else {
@@ -435,6 +446,16 @@ enum RecordSerializer {
record["readAt"] as? Date
}
+ /// Reads `readThrough` off an inbound Player record — the per-account read
+ /// watermark: the latest other-author move time this user has actually
+ /// seen. Unlike `readAt` it is never leased into the future, so the
+ /// session-end push uses it to decide what a recipient still hasn't seen.
+ /// Returns `nil` for records that predate the field or a slot that has not
+ /// recorded a read yet.
+ static func parsePlayerReadThrough(from record: CKRecord) -> Date? {
+ record["readThrough"] as? Date
+ }
+
/// Reads `selRow`/`selCol`/`selDir` off an inbound Player record. These
/// fields carry the peer's cursor track start, not their exact local
/// reticle. Returns `nil` if any field is missing — the peer either hasn't
diff --git a/Tests/Unit/GameStoreUnreadMovesTests.swift b/Tests/Unit/GameStoreUnreadMovesTests.swift
@@ -125,7 +125,7 @@ struct GameStoreUnreadMovesTests {
#expect(!summary.hasUnreadOtherMoves)
}
- @Test("Opening a game advances lastReadOtherMoveAt to latestOtherMoveAt")
+ @Test("Opening a game advances readThroughAt to latestOtherMoveAt")
func openingGameMarksOtherMovesSeen() throws {
let persistence = makeTestPersistence()
let store = makeTestStore(persistence: persistence)
@@ -137,7 +137,7 @@ struct GameStoreUnreadMovesTests {
_ = try store.loadGame(id: entity.id!)
- #expect(entity.lastReadOtherMoveAt == latest)
+ #expect(entity.readThroughAt == latest)
let summary = try #require(GameSummary(entity: entity))
#expect(!summary.hasUnreadOtherMoves)
}
@@ -162,11 +162,11 @@ struct GameStoreUnreadMovesTests {
currentAuthorID: Self.localAuthorID
)
- // Seen: shared game whose lastSeen catches up to latest.
+ // Seen: shared game whose watermark catches up to latest.
let (gameB, _) = try makeSharedGame(in: ctx)
let seenLatest = Date(timeIntervalSinceNow: -30)
gameB.latestOtherMoveAt = seenLatest
- gameB.lastReadOtherMoveAt = seenLatest
+ gameB.readThroughAt = seenLatest
try ctx.save()
#expect(store.unreadOtherMovesGameCount() == 1)
@@ -176,7 +176,7 @@ struct GameStoreUnreadMovesTests {
#expect(store.unreadOtherMovesGameCount() == 0)
}
- @Test("Inbound moves while the puzzle is visible advance lastReadOtherMoveAt")
+ @Test("Inbound moves while the puzzle is visible advance readThroughAt")
func inboundMovesWhilePuzzleVisibleMarkSeen() throws {
let persistence = makeTestPersistence()
let store = makeTestStore(persistence: persistence)
@@ -198,7 +198,7 @@ struct GameStoreUnreadMovesTests {
currentAuthorID: Self.localAuthorID
)
- #expect(entity.lastReadOtherMoveAt == updatedAt)
+ #expect(entity.readThroughAt == updatedAt)
let summary = try #require(GameSummary(entity: entity))
#expect(!summary.hasUnreadOtherMoves)
}
@@ -298,12 +298,12 @@ struct GameStoreUnreadMovesTests {
currentAuthorID: Self.localAuthorID
)
- #expect(entity.lastReadOtherMoveAt == updatedAt)
+ #expect(entity.readThroughAt == updatedAt)
let summary = try #require(GameSummary(entity: entity))
#expect(!summary.hasUnreadOtherMoves)
}
- @Test("A sibling's readAt is adopted last-writer-wins")
+ @Test("A sibling's readAt presence lease is adopted last-writer-wins")
func incomingReadCursorSetsBadgeHorizon() throws {
let persistence = makeTestPersistence()
let store = makeTestStore(
@@ -316,45 +316,66 @@ struct GameStoreUnreadMovesTests {
let earlier = Date(timeIntervalSinceNow: -30)
let later = Date(timeIntervalSinceNow: -10)
let future = Date(timeIntervalSinceNow: 10 * 60)
- try addMovesRow(
- for: entity,
- gameID: gameID,
- authorID: Self.otherAuthorID,
- updatedAt: later,
- in: ctx
- )
- store.noteIncomingMovesUpdate(
- gameIDs: [gameID],
- currentAuthorID: Self.localAuthorID
- )
- #expect(entity.lastReadOtherMoveAt == nil)
- #expect(store.unreadOtherMovesGameCount() == 1)
// All of an account's devices share one Player record, so the inbound
- // `readAt` is the account's resolved horizon: adopt it verbatim.
- // A sibling read up to `earlier` — older than the latest move, badge fires.
+ // `readAt` is the account's resolved *presence lease*: adopt it verbatim
+ // under last-writer-wins. This is the presence horizon only; the unread
+ // badge is driven by the read watermark (see the watermark test below).
store.noteIncomingReadCursor(gameID: gameID, readAt: earlier)
#expect(entity.lastReadOtherMoveAt == earlier)
- #expect(store.unreadOtherMovesGameCount() == 1)
- // A sibling reads up to `later` — catches the latest move, badge clears.
store.noteIncomingReadCursor(gameID: gameID, readAt: later)
#expect(entity.lastReadOtherMoveAt == later)
- #expect(store.unreadOtherMovesGameCount() == 0)
// A sibling opens an active session and leases the horizon into the
- // future; the lease is adopted and keeps the badge suppressed.
+ // future; the lease is adopted verbatim.
store.noteIncomingReadCursor(gameID: gameID, readAt: future)
#expect(entity.lastReadOtherMoveAt == future)
- #expect(store.unreadOtherMovesGameCount() == 0)
// That sibling leaves and publishes the current time. Under
// last-writer-wins the single shared scalar simply moves back to the
- // close value — there is no per-device lease here to protect it. The
- // collapse is bounded and self-healing: a still-present device re-asserts
- // its own lease from `AppServices`'s incoming-cursor drain, not here.
+ // close value — there is no per-device lease here to protect it.
store.noteIncomingReadCursor(gameID: gameID, readAt: later)
#expect(entity.lastReadOtherMoveAt == later)
+ }
+
+ @Test("The unread badge tracks the read watermark, not the presence lease")
+ func badgeTracksWatermarkNotLease() throws {
+ let persistence = makeTestPersistence()
+ let store = makeTestStore(
+ persistence: persistence,
+ authorIDProvider: { Self.localAuthorID }
+ )
+ let ctx = persistence.viewContext
+ let (entity, gameID) = try makeSharedGame(in: ctx)
+
+ let earlier = Date(timeIntervalSinceNow: -30)
+ let latest = Date(timeIntervalSinceNow: -10)
+ let future = Date(timeIntervalSinceNow: 10 * 60)
+ try addMovesRow(
+ for: entity,
+ gameID: gameID,
+ authorID: Self.otherAuthorID,
+ updatedAt: latest,
+ in: ctx
+ )
+ store.noteIncomingMovesUpdate(gameIDs: [gameID], currentAuthorID: Self.localAuthorID)
+ #expect(entity.readThroughAt == nil)
+ #expect(store.unreadOtherMovesGameCount() == 1)
+
+ // A future presence lease must NOT clear the badge — this is the bug:
+ // a leased-but-backgrounded reader had moves silently swallowed.
+ #expect(store.setReadCursor(gameID: gameID, readAt: future))
+ #expect(entity.lastReadOtherMoveAt == future)
+ #expect(store.unreadOtherMovesGameCount() == 1)
+
+ // The watermark, older than the latest move, still leaves it unread.
+ #expect(store.advanceReadThrough(gameID: gameID, through: earlier))
+ #expect(store.unreadOtherMovesGameCount() == 1)
+
+ // The watermark catching the latest move is what clears the badge.
+ #expect(store.advanceReadThrough(gameID: gameID, through: latest))
+ #expect(entity.readThroughAt == latest)
#expect(store.unreadOtherMovesGameCount() == 0)
}
diff --git a/Tests/Unit/SessionPushPlannerTests.swift b/Tests/Unit/SessionPushPlannerTests.swift
@@ -72,12 +72,12 @@ struct SessionPushPlannerTests {
private func recipient(
_ address: String?,
- readAt: Date?,
+ readThrough: Date?,
notifiedThrough: Date? = nil
) -> AppServices.PushRecipient {
AppServices.PushRecipient(
authorID: "peer",
- readAt: readAt,
+ readThrough: readThrough,
notifiedThrough: notifiedThrough,
pushAddress: address
)
@@ -90,7 +90,7 @@ struct SessionPushPlannerTests {
entry("X", at: edit, seq: 1, col: 0),
entry("", at: edit, seq: 2, col: 1)
]
- let seenEverything = recipient("addr-1", readAt: edit.addingTimeInterval(60))
+ let seenEverything = recipient("addr-1", readThrough: edit.addingTimeInterval(60))
let addressees = SessionPushPlanner.sessionEndAddressees(
recipients: [seenEverything],
@@ -116,7 +116,7 @@ struct SessionPushPlannerTests {
entry("Y", at: edit.addingTimeInterval(-120), seq: 1, col: 1),
entry("", at: edit, seq: 3, col: 1)
]
- let behind = recipient("addr-2", readAt: edit.addingTimeInterval(-60))
+ let behind = recipient("addr-2", readThrough: edit.addingTimeInterval(-60))
let addressees = SessionPushPlanner.sessionEndAddressees(
recipients: [behind],
@@ -135,7 +135,7 @@ struct SessionPushPlannerTests {
@Test("A move already notified isn't re-counted, even if the recipient never read it")
func notifiedThroughWindowsOutPriorMoves() {
- // The bounce case: the recipient stayed backgrounded (readAt stuck far
+ // The bounce case: the recipient stayed backgrounded (readThrough stuck far
// in the past), but we already paused to them once covering `edit`. The
// second pause must not re-report that fill — it tallies to zero and
// reads as a presence-only "stopped solving", not a duplicate summary.
@@ -145,7 +145,7 @@ struct SessionPushPlannerTests {
// First pause (never notified): the fill is news to the recipient.
let firstPause = SessionPushPlanner.sessionEndAddressees(
- recipients: [recipient("addr", readAt: staleReadAt)],
+ recipients: [recipient("addr", readThrough: staleReadAt)],
journalEntries: entries,
playerName: "Bunny",
puzzleTitle: "Tuesday"
@@ -153,10 +153,10 @@ struct SessionPushPlannerTests {
#expect(firstPause[0].payload
== PushPayload(event: .pause(fills: 1, clears: 0, checks: 0, reveals: 0)))
- // Second pause after a bounce with no new move: readAt is still stale,
+ // Second pause after a bounce with no new move: readThrough is still stale,
// but the watermark already covers `edit`, so nothing is re-reported.
let secondPause = SessionPushPlanner.sessionEndAddressees(
- recipients: [recipient("addr", readAt: staleReadAt, notifiedThrough: edit)],
+ recipients: [recipient("addr", readThrough: staleReadAt, notifiedThrough: edit)],
journalEntries: entries,
playerName: "Bunny",
puzzleTitle: "Tuesday"
@@ -167,6 +167,32 @@ struct SessionPushPlannerTests {
#expect(secondPause[0].body == "Bunny stopped solving the puzzle 'Tuesday'.")
}
+ @Test("A present-but-backgrounded recipient still gets a summary (lease ≠ watermark)")
+ func presenceLeaseDoesNotSuppressSummary() {
+ // Regression for the "moves via banner, no push" bug. The recipient held
+ // a *presence lease* dated into the future (they were "present"), then
+ // backgrounded before our edit. Their read *watermark* (`readThrough`)
+ // is what the planner sees, and it sits before the edit — so the fill is
+ // still news and must be summarised. The forward-dated lease is no
+ // longer an input here, which is the whole point of the split.
+ let edit = Date(timeIntervalSince1970: 10_000)
+ let entries = [entry("X", at: edit, seq: 1, col: 0)]
+ // Watermark from when they last actually looked, well before the edit.
+ let watermarkBeforeEdit = edit.addingTimeInterval(-600)
+
+ let addressees = SessionPushPlanner.sessionEndAddressees(
+ recipients: [recipient("addr", readThrough: watermarkBeforeEdit)],
+ journalEntries: entries,
+ playerName: "Bunny",
+ puzzleTitle: "Tuesday"
+ )
+
+ #expect(addressees.count == 1)
+ #expect(addressees[0].payload
+ == PushPayload(event: .pause(fills: 1, clears: 0, checks: 0, reveals: 0)))
+ #expect(addressees[0].body == "Bunny filled 1 letter in the puzzle 'Tuesday'")
+ }
+
@Test("A whole-grid check reports one check gesture, not a wall of letter churn")
func checkGestureCounted() {
let edit = Date(timeIntervalSince1970: 1_000)
@@ -179,7 +205,7 @@ struct SessionPushPlannerTests {
entry("A", at: edit, seq: 3, col: 0, kind: .check, batch: batch),
entry("B", at: edit, seq: 4, col: 1, kind: .check, batch: batch)
]
- let behind = recipient("addr-check", readAt: edit.addingTimeInterval(-60))
+ let behind = recipient("addr-check", readThrough: edit.addingTimeInterval(-60))
let addressees = SessionPushPlanner.sessionEndAddressees(
recipients: [behind],
@@ -201,7 +227,7 @@ struct SessionPushPlannerTests {
entry("Z", at: edit, seq: 1, col: 0, kind: .reveal, batch: batch),
entry("Q", at: edit, seq: 2, col: 1, kind: .reveal, batch: batch)
]
- let behind = recipient("addr-reveal", readAt: edit.addingTimeInterval(-60))
+ let behind = recipient("addr-reveal", readThrough: edit.addingTimeInterval(-60))
let addressees = SessionPushPlanner.sessionEndAddressees(
recipients: [behind],
@@ -233,7 +259,7 @@ struct SessionPushPlannerTests {
// reveal
entry("D", at: edit, seq: 13, col: 3, kind: .reveal, batch: revealBatch)
]
- let behind = recipient("addr-mixed", readAt: edit.addingTimeInterval(-60))
+ let behind = recipient("addr-mixed", readThrough: edit.addingTimeInterval(-60))
let addressees = SessionPushPlanner.sessionEndAddressees(
recipients: [behind],
@@ -255,7 +281,7 @@ struct SessionPushPlannerTests {
entry("A", at: edit, seq: 1, col: 0),
entry("", at: edit.addingTimeInterval(1), seq: 2, col: 0)
]
- let behind = recipient("addr-noop", readAt: edit.addingTimeInterval(-60))
+ let behind = recipient("addr-noop", readThrough: edit.addingTimeInterval(-60))
let addressees = SessionPushPlanner.sessionEndAddressees(
recipients: [behind],
@@ -269,13 +295,13 @@ struct SessionPushPlannerTests {
#expect(addressees[0].payload?.marksUnread == false)
}
- @Test("Diagnostics ride each recipient's payload, stamped with that recipient's readAt")
+ @Test("Diagnostics ride each recipient's payload, stamped with that recipient's readThrough")
func diagnosticsStampedPerRecipient() {
let edit = Date(timeIntervalSince1970: 1_000)
let entries = [entry("X", at: edit, seq: 1, col: 0)]
let aReadAt = edit.addingTimeInterval(-60)
- let recipientA = recipient("addr-a", readAt: aReadAt)
- let recipientB = recipient("addr-b", readAt: nil)
+ let recipientA = recipient("addr-a", readThrough: aReadAt)
+ let recipientB = recipient("addr-b", readThrough: nil)
let base = PushPayload.Diagnostics(
gridWidth: 15,
gridHeight: 15,
@@ -306,7 +332,7 @@ struct SessionPushPlannerTests {
func unaddressableDropped() {
let edit = Date(timeIntervalSince1970: 1_000)
let addressees = SessionPushPlanner.sessionEndAddressees(
- recipients: [recipient(nil, readAt: nil)],
+ recipients: [recipient(nil, readThrough: nil)],
journalEntries: [entry("X", at: edit, seq: 1, col: 0)],
playerName: "Bunny",
puzzleTitle: "Tuesday"
@@ -319,8 +345,8 @@ struct SessionPushPlannerTests {
func mixedRecipientsAllIncluded() {
let edit = Date(timeIntervalSince1970: 1_000)
let entries = [entry("X", at: edit, seq: 1, col: 0)]
- let caughtUp = recipient("addr-caught-up", readAt: edit.addingTimeInterval(60))
- let behind = recipient("addr-behind", readAt: edit.addingTimeInterval(-60))
+ let caughtUp = recipient("addr-caught-up", readThrough: edit.addingTimeInterval(60))
+ let behind = recipient("addr-behind", readThrough: edit.addingTimeInterval(-60))
let addressees = SessionPushPlanner.sessionEndAddressees(
recipients: [caughtUp, behind],
