Rebuild message location index on non-append growth

The incremental index refresh assumed growth meant appended messages,
but PublicMessagePipeline inserts out-of-order arrivals by timestamp:
the count grows while the tail ID stays put, so the inserted message
never entered the index and later delivery updates for it silently
no-op'd (and, with retain-until-ack routing, left it queued for
resend). Detect non-append growth by checking the previously indexed
tail kept its position, and rebuild when it hasn't. Same check on the
per-peer private chat arrays, which re-sort by timestamp.

Found by Codex review on #1331.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
jack
2026-06-10 16:32:36 +01:00
co-authored by Claude Fable 5
parent 3a995e20b6
commit 19b28cf49d
2 changed files with 68 additions and 0 deletions
@@ -238,6 +238,15 @@ private extension ChatDeliveryCoordinator {
}
if context.messages.count > indexedPublicMessageCount {
// Growth is only a pure append if the previously indexed tail kept
// its position; a middle insertion (out-of-order timestamp arrival)
// shifts it and invalidates every indexed location after the
// insertion point.
if indexedPublicMessageCount > 0,
context.messages[indexedPublicMessageCount - 1].id != indexedPublicTailMessageID {
rebuildMessageLocationIndex()
return true
}
for index in indexedPublicMessageCount..<context.messages.count {
add(.publicTimeline(index: index), for: context.messages[index].id)
}
@@ -265,6 +274,12 @@ private extension ChatDeliveryCoordinator {
}
guard messages.count > indexedCount else { continue }
// Same append-only check as the public timeline above.
if indexedCount > 0,
messages[indexedCount - 1].id != indexedPrivateTailMessageIDs[peerID] {
rebuildMessageLocationIndex()
return true
}
for index in indexedCount..<messages.count {
add(.privateChat(peerID: peerID, index: index), for: messages[index].id)
}
@@ -483,6 +483,59 @@ struct ChatDeliveryCoordinatorContextTests {
#expect(context.notifyUIChangedCount == 1)
}
@Test @MainActor
func middleInsertedMessage_isFoundAfterIndexWasBuilt() async {
let context = MockChatDeliveryContext()
let coordinator = ChatDeliveryCoordinator(context: context)
let makePublic = { (id: String) in
BitchatMessage(
id: id,
sender: "me",
content: "Public message",
timestamp: Date(),
isRelay: false,
isPrivate: false,
deliveryStatus: .sending
)
}
context.messages = [makePublic("public-a"), makePublic("public-b")]
// Build the incremental index.
#expect(coordinator.updateMessageDeliveryStatus("public-a", status: .sent))
// Out-of-order arrival: PublicMessagePipeline inserts by timestamp,
// so the count grows while the tail ID stays the same.
context.messages.insert(makePublic("public-mid"), at: 1)
// The inserted message must be locatable, and the shifted tail must
// not be updated through a stale index entry.
#expect(coordinator.updateMessageDeliveryStatus("public-mid", status: .sent))
#expect(isSent(context.messages[1].deliveryStatus))
#expect(coordinator.updateMessageDeliveryStatus("public-b", status: .sent))
#expect(isSent(context.messages[2].deliveryStatus))
}
@Test @MainActor
func middleInsertedPrivateMessage_isFoundAfterIndexWasBuilt() async {
let context = MockChatDeliveryContext()
let coordinator = ChatDeliveryCoordinator(context: context)
let peerID = PeerID(str: "0102030405060708")
context.privateChats[peerID] = [
makePrivateMessage(id: "pm-a", status: .sending),
makePrivateMessage(id: "pm-b", status: .sending)
]
#expect(coordinator.updateMessageDeliveryStatus("pm-a", status: .sent))
// Timestamp re-sort (sanitizeChat) can place a late arrival mid-array.
context.privateChats[peerID]?.insert(makePrivateMessage(id: "pm-mid", status: .sending), at: 1)
#expect(coordinator.updateMessageDeliveryStatus("pm-mid", status: .sent))
#expect(isSent(context.privateChats[peerID]?[1].deliveryStatus))
#expect(coordinator.updateMessageDeliveryStatus("pm-b", status: .sent))
#expect(isSent(context.privateChats[peerID]?[2].deliveryStatus))
}
@Test @MainActor
func cleanupOldReadReceipts_prunesReceiptsAgainstMockContext() async {
let context = MockChatDeliveryContext()