refactor(sdk): drop unused incrementedUnread payload from ChatMessageAppendOutcome

RidestrSDK/Sources/RidestrSDK/RoadFlare/ChatMessageStore.swift

@@ -22,13 +22,13 @@ public struct ChatMessage: Hashable, Sendable {
 
 /// Result of appending a message to a `ChatMessageStore`.
 ///
-/// `.duplicate` is returned when a message with the same `id` is already
-/// present; the store is unchanged. `.inserted` is returned when the message
-/// is accepted; `incrementedUnread` is `true` iff the message was remote
-/// (`!isMine`) and its timestamp was at or after the current unread cutoff.
+/// `.duplicate` — a message with the same `id` is already present and the
+/// store is unchanged. `.inserted` — the message was accepted. Callers
+/// that need to know whether the append also incremented `unreadCount`
+/// observe `unreadCount` directly.
 public enum ChatMessageAppendOutcome: Equatable, Sendable {
     case duplicate
-    case inserted(incrementedUnread: Bool)
+    case inserted
 }
 
 /// Platform-neutral in-memory store for an in-ride chat thread.
@@ -97,11 +97,10 @@ public final class ChatMessageStore: @unchecked Sendable {
                 let removed = messages.removeFirst()
                 messageIds.remove(removed.id)
             }
-            let incrementedUnread = !message.isMine && message.timestamp >= unreadCutoff
-            if incrementedUnread {
+            if !message.isMine && message.timestamp >= unreadCutoff {
                 unreadCount += 1
             }
-            return .inserted(incrementedUnread: incrementedUnread)
+            return .inserted
         }
     }
 

RidestrSDK/Tests/RidestrSDKTests/RoadFlare/ChatMessageStoreTests.swift

@@ -35,7 +35,7 @@ struct ChatMessageStoreTests {
     @Test func appendInsertsNewMessage() {
         let store = makeStore()
         let outcome = store.append(message(id: "a", timestamp: 100))
-        #expect(outcome == .inserted(incrementedUnread: true))
+        #expect(outcome == .inserted)
         #expect(store.messages.count == 1)
         #expect(store.messages[0].id == "a")
     }
@@ -120,7 +120,7 @@ struct ChatMessageStoreTests {
         _ = store.append(message(id: "b", isMine: true, timestamp: 200))
         _ = store.append(message(id: "c", isMine: true, timestamp: 300))
         let outcome = store.append(message(id: "a", isMine: false, timestamp: 100))
-        #expect(outcome == .inserted(incrementedUnread: true))
+        #expect(outcome == .inserted)
         #expect(store.messages.map(\.id) == ["b", "c"])
         #expect(store.unreadCount == 1)
     }
@@ -130,7 +130,7 @@ struct ChatMessageStoreTests {
     @Test func unreadDoesNotIncrementForOwnMessages() {
         let store = makeStore()
         let outcome = store.append(message(id: "a", isMine: true, timestamp: 100))
-        #expect(outcome == .inserted(incrementedUnread: false))
+        #expect(outcome == .inserted)
         #expect(store.unreadCount == 0)
     }
 
@@ -145,9 +145,10 @@ struct ChatMessageStoreTests {
         let store = makeStore()
         store.setUnreadCutoff(500)
         let stale = store.append(message(id: "old", isMine: false, timestamp: 400))
+        #expect(stale == .inserted)
+        #expect(store.unreadCount == 0)
         let live = store.append(message(id: "new", isMine: false, timestamp: 600))
-        #expect(stale == .inserted(incrementedUnread: false))
-        #expect(live == .inserted(incrementedUnread: true))
+        #expect(live == .inserted)
         #expect(store.unreadCount == 1)
     }
 
@@ -156,7 +157,7 @@ struct ChatMessageStoreTests {
         let store = makeStore()
         store.setUnreadCutoff(500)
         let atBoundary = store.append(message(id: "boundary", isMine: false, timestamp: 500))
-        #expect(atBoundary == .inserted(incrementedUnread: true))
+        #expect(atBoundary == .inserted)
         #expect(store.unreadCount == 1)
     }
 
@@ -196,7 +197,7 @@ struct ChatMessageStoreTests {
         #expect(store.unreadCount == 0)
         // Cutoff preserved: a message at timestamp 400 should NOT increment unread.
         let outcome = store.append(message(id: "b", isMine: false, timestamp: 400))
-        #expect(outcome == .inserted(incrementedUnread: false))
+        #expect(outcome == .inserted)
         #expect(store.unreadCount == 0)
     }
 

decisions/0020-chat-message-store.md

@@ -43,7 +43,7 @@ public struct ChatMessage: Hashable, Sendable {
 
 public enum ChatMessageAppendOutcome: Equatable, Sendable {
     case duplicate
-    case inserted(incrementedUnread: Bool)
+    case inserted
 }
 
 @Observable
@@ -68,7 +68,9 @@ Specific decisions captured:
 3. **Sort:** ascending `timestamp`, tiebreak ascending `id` — deterministic.
 4. **Capacity:** init parameter with default `500`. FIFO eviction (drop
    `removeFirst()` after sort).
-5. **Append API:** returns `enum AppendOutcome { duplicate, inserted(incrementedUnread:) }`.
+5. **Append API:** returns `enum AppendOutcome { duplicate, inserted }`. Callers
+   that need to know whether the append also bumped `unreadCount` observe the
+   counter directly rather than receiving it as a return-value side channel.
 6. **Unread cutoff:** caller-supplied via `setUnreadCutoff(_:)`, defaults to
    `0`, inclusive check (`timestamp >= cutoff`).
 7. **`reset()` semantics:** clears messages, message-id set, and
@@ -114,11 +116,18 @@ costs essentially nothing, lets tests exercise the boundary with small
 capacities (`capacity: 2` and `capacity: 5` in the test suite), and gives
 future consumers a config knob without forcing a refactor.
 
-**Append outcome enum.** `Bool` would have told callers *whether* a message
-was inserted but not whether it bumped the unread counter — and the
-coordinator's haptic decision needs to know both. The enum bundles the two
-signals atomically without polling the counter or recomputing the cutoff
-check externally.
+**Append outcome enum.** The enum's named cases (`.duplicate`, `.inserted`)
+read more clearly at call sites than a `Bool` would — the haptic decision
+in `ChatCoordinator.handleChatEvent` becomes a self-describing
+`if case .inserted = store.append(message), !message.isMine`. An earlier
+revision attached an `incrementedUnread: Bool` payload to `.inserted`,
+motivated by a hypothetical caller that needed to know whether the badge
+just bumped (e.g. for a "fresh-unread pulse" UI). The actual coordinator
+never read the payload — it only checks the case — and every test that
+exercised the unread side effect also asserts `store.unreadCount` directly,
+which is the canonical source. The payload was therefore dead public surface
+and was dropped; if a future consumer genuinely needs a fresh-unread signal
+it can be added back as an associated value at that time.
 
 **Default cutoff of 0.** This makes "no cutoff set" equivalent to "every
 remote message is unread," which is the safe default for a fresh store
@@ -178,10 +187,20 @@ gain and the driver coordinator will not call this type at all.
   test-seam value (running boundary tests with `capacity: 2` instead of
   pushing 501 messages) was outsized.
 
-- **`@discardableResult func append(_:) -> Bool`.** Simpler. Rejected
-  because the coordinator's haptic decision needs to know whether the
-  message also bumped unread; `Bool` would have forced callers to poll
-  `unreadCount` deltas, which is brittle.
+- **`@discardableResult func append(_:) -> Bool`.** Strictly equivalent
+  to the chosen `enum { duplicate, inserted }`. Rejected on readability —
+  `if case .inserted = store.append(msg)` at the call site documents what
+  the value means; `if store.append(msg)` requires the reader to remember
+  which boolean polarity means "inserted."
+
+- **Bundle unread-bump info into the return value** (e.g. `.inserted(incrementedUnread:)`).
+  An earlier revision did this. Dropped because the only production
+  caller — `ChatCoordinator.handleChatEvent` — never read the payload;
+  it branches solely on `.inserted` vs `.duplicate` for haptics. Tests
+  that check the unread side effect assert `store.unreadCount` directly,
+  which is the canonical source. If a future consumer genuinely needs
+  a "fresh-unread" signal it can be added back as an associated value
+  at that point.
 
 - **Construct with cutoff in init, no `setUnreadCutoff(_:)`.** Removes
   the default-0 footgun. Rejected because the subscription-managing caller