Merge pull request #1244 from Hy4ri/fix/local-chat-notice-ordering
fix(chat): keep local notices in transcript order
This commit is contained in:
2 files changed
+129
-8
No files matched your search
@@ -337,10 +337,15 @@ internal fun mergeCachedTranscriptPage(
|
||||
displayKind = message.displayKind ?: match.displayKind,
|
||||
)
|
||||
}.toMap()
|
||||
val resolvedOrders =
|
||||
incoming.indices
|
||||
.mapNotNull { index ->
|
||||
matches[index]?.id?.let { id -> incoming[index].canonicalOrder?.let { id to it } }
|
||||
}.toMap()
|
||||
return (
|
||||
current.map { replacements[it.id] ?: it } +
|
||||
incoming.filterIndexed { index, _ -> matches[index] == null }
|
||||
).dedupeById().inTranscriptOrder().reconcileReasoningRows()
|
||||
).dedupeById().inTranscriptOrder(current, resolvedOrders).reconcileReasoningRows()
|
||||
}
|
||||
|
||||
/** Merge one page with the current snapshot without consuming repeated results more than once. */
|
||||
@@ -398,15 +403,56 @@ internal fun mergeTranscriptWithLive(
|
||||
// copy is also present. Fold that now-confirmed echo without changing the live key.
|
||||
// The page has already consumed its content matches; consuming another would
|
||||
// collapse a separate repeated occurrence. Only confirmed identities can fold here.
|
||||
return dedupeCachedMessages(transcript.inTranscriptOrder(), confirmedOnly = true).reconcileReasoningRows()
|
||||
val resolvedOrders =
|
||||
incoming.indices
|
||||
.mapNotNull { index ->
|
||||
matches[index]?.id?.let { id -> incoming[index].canonicalOrder?.let { id to it } }
|
||||
}.toMap()
|
||||
return dedupeCachedMessages(transcript.inTranscriptOrder(current, resolvedOrders), confirmedOnly = true)
|
||||
.reconcileReasoningRows()
|
||||
}
|
||||
|
||||
/** Numeric canonical history first, then unconfirmed local insertion order; clocks never sort a transcript. */
|
||||
private fun List<ChatMessage>.inTranscriptOrder(): List<ChatMessage> =
|
||||
sortedWith(
|
||||
compareBy<ChatMessage> { it.canonicalOrder == null }
|
||||
.thenBy { it.canonicalOrder ?: it.localOrder ?: Long.MAX_VALUE },
|
||||
)
|
||||
/** Keep server order; place local notices after their last preceding confirmed message, not at the transcript tail. */
|
||||
private fun List<ChatMessage>.inTranscriptOrder(
|
||||
previous: List<ChatMessage>,
|
||||
resolvedOrders: Map<String, Long>,
|
||||
): List<ChatMessage> {
|
||||
val latestCanonical = mapNotNull { it.canonicalOrder }.maxOrNull() ?: -1L
|
||||
var precedingCanonical: Long? = null
|
||||
var hasPendingPredecessor = false
|
||||
var pendingLocalOrder: Long? = null
|
||||
val noticeAnchors = mutableMapOf<String, Long>()
|
||||
val pendingOrderByNotice = mutableMapOf<String, Long>()
|
||||
previous.forEach { message ->
|
||||
val order = resolvedOrders[message.id] ?: message.canonicalOrder
|
||||
if (order != null) {
|
||||
precedingCanonical = order
|
||||
hasPendingPredecessor = false
|
||||
pendingLocalOrder = null
|
||||
} else if (message.role != MessageRole.SYSTEM) {
|
||||
hasPendingPredecessor = true
|
||||
pendingLocalOrder = message.localOrder
|
||||
} else {
|
||||
noticeAnchors[message.id] =
|
||||
if (hasPendingPredecessor) {
|
||||
Long.MAX_VALUE
|
||||
} else {
|
||||
precedingCanonical?.takeIf { it >= 0L }
|
||||
?: latestCanonical
|
||||
}
|
||||
if (hasPendingPredecessor) pendingOrderByNotice[message.id] = pendingLocalOrder ?: Long.MAX_VALUE
|
||||
}
|
||||
}
|
||||
val previousIndices = previous.withIndex().associate { it.value.id to it.index }
|
||||
return withIndex()
|
||||
.sortedWith(
|
||||
compareBy<IndexedValue<ChatMessage>> {
|
||||
it.value.canonicalOrder ?: noticeAnchors[it.value.id] ?: Long.MAX_VALUE
|
||||
}.thenBy { if (noticeAnchors[it.value.id]?.let { anchor -> anchor != Long.MAX_VALUE } == true) 1 else 0 }
|
||||
.thenBy { it.value.localOrder ?: pendingOrderByNotice[it.value.id] ?: Long.MAX_VALUE }
|
||||
.thenBy { previousIndices[it.value.id] ?: it.index },
|
||||
).map { it.value }
|
||||
}
|
||||
|
||||
internal fun ChatMessage.isSessionStartMarker(): Boolean =
|
||||
role == MessageRole.SYSTEM && (content == "Session created" || content == "Session branched")
|
||||
|
||||
@@ -673,6 +673,81 @@ class ChatPagingMergeTest {
|
||||
assertEquals("Here is the Python script", merged[2].content)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun localNoticesKeepTheirPositionAcrossRepeatedTranscriptSyncs() {
|
||||
val first = ChatMessage(id = "rest-s-1", role = MessageRole.USER, content = "hello")
|
||||
val answer = ChatMessage(id = "rest-s-2", role = MessageRole.ASSISTANT, content = "hi")
|
||||
val resumed = ChatMessage(id = "resume", role = MessageRole.SYSTEM, content = "Session resumed")
|
||||
val review =
|
||||
ChatMessage(id = "review", role = MessageRole.SYSTEM, content = "💾 Self-improvement review: skill updated")
|
||||
val next = ChatMessage(id = "rest-s-3", role = MessageRole.USER, content = "next")
|
||||
val reply = ChatMessage(id = "rest-s-4", role = MessageRole.ASSISTANT, content = "done")
|
||||
|
||||
val afterResume = mergeTranscriptWithLive(listOf(first, answer), listOf(first, answer, resumed))
|
||||
assertEquals(listOf("rest-s-1", "rest-s-2", "resume"), afterResume.map { it.id })
|
||||
val afterReview = mergeTranscriptWithLive(listOf(first, answer), afterResume + review)
|
||||
assertEquals(listOf("rest-s-1", "rest-s-2", "resume", "review"), afterReview.map { it.id })
|
||||
val afterNextTurn = mergeTranscriptWithLive(listOf(next, reply), afterReview)
|
||||
assertEquals(
|
||||
listOf("rest-s-1", "rest-s-2", "resume", "review", "rest-s-3", "rest-s-4"),
|
||||
afterNextTurn.map { it.id },
|
||||
)
|
||||
assertEquals(afterNextTurn, mergeTranscriptWithLive(listOf(next, reply), afterNextTurn))
|
||||
assertEquals(
|
||||
afterNextTurn.map { it.id },
|
||||
mergeCachedTranscriptPage(listOf(first, answer, next, reply), afterNextTurn).map { it.id },
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun localNoticeOnColdResumeFollowsHydratedHistoryButNotFutureReplies() {
|
||||
val resumed = ChatMessage(id = "resume", role = MessageRole.SYSTEM, content = "Session resumed")
|
||||
val prior = ChatMessage(id = "rest-s-1", role = MessageRole.USER, content = "earlier")
|
||||
val later = ChatMessage(id = "rest-s-2", role = MessageRole.ASSISTANT, content = "later")
|
||||
val created = ChatMessage(id = "created", role = MessageRole.SYSTEM, content = "Session created")
|
||||
val hydrated = mergeTranscriptWithLive(listOf(prior), listOf(created, resumed))
|
||||
assertEquals(listOf("created", "rest-s-1", "resume"), hydrated.map { it.id })
|
||||
val synced = mergeTranscriptWithLive(listOf(later), hydrated)
|
||||
assertEquals(listOf("created", "rest-s-1", "resume", "rest-s-2"), synced.map { it.id })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun unconfirmedUserAndAssistantRemainAfterCanonicalHistory() {
|
||||
val user = ChatMessage(id = "uuid-user", role = MessageRole.USER, content = "pending")
|
||||
val assistant = ChatMessage(id = "uuid-assistant", role = MessageRole.ASSISTANT, content = "streaming")
|
||||
val canonical = ChatMessage(id = "rest-s-1", role = MessageRole.USER, content = "older")
|
||||
val merged = mergeTranscriptWithLive(listOf(canonical), listOf(user, assistant))
|
||||
assertEquals(listOf("rest-s-1", "uuid-user", "uuid-assistant"), merged.map { it.id })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun olderPageDoesNotMoveLocalNoticeOrReverseCanonicalRows() {
|
||||
val old = ChatMessage(id = "rest-s-1", role = MessageRole.USER, content = "old")
|
||||
val current = ChatMessage(id = "rest-s-3", role = MessageRole.ASSISTANT, content = "current")
|
||||
val notice = ChatMessage(id = "notice", role = MessageRole.SYSTEM, content = "Session interrupted")
|
||||
val next = ChatMessage(id = "rest-s-4", role = MessageRole.USER, content = "next")
|
||||
|
||||
val paged = mergeTranscriptWithLive(listOf(old), listOf(current, notice), chronological = false)
|
||||
assertEquals(listOf("rest-s-1", "rest-s-3", "notice"), paged.map { it.id })
|
||||
val synced = mergeTranscriptWithLive(listOf(next), paged)
|
||||
assertEquals(listOf("rest-s-1", "rest-s-3", "notice", "rest-s-4"), synced.map { it.id })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun reviewNoticeStaysAfterAssistantWhenItsRestEchoArrivesLater() {
|
||||
val user = ChatMessage(id = "rest-s-1", role = MessageRole.USER, content = "prompt")
|
||||
val liveReply = ChatMessage(id = "live-reply", role = MessageRole.ASSISTANT, content = "answer")
|
||||
val review = ChatMessage(id = "review", role = MessageRole.SYSTEM, content = "💾 Self-improvement review")
|
||||
val beforeEcho = mergeTranscriptWithLive(listOf(user), listOf(user, liveReply, review), preserveLiveIds = true)
|
||||
assertEquals(listOf("rest-s-1", "live-reply", "review"), beforeEcho.map { it.id })
|
||||
val restReply = liveReply.copy(id = "rest-s-2")
|
||||
val afterEcho = mergeTranscriptWithLive(listOf(restReply), beforeEcho, preserveLiveIds = true)
|
||||
assertEquals(listOf("rest-s-1", "live-reply", "review"), afterEcho.map { it.id })
|
||||
val next = ChatMessage(id = "rest-s-3", role = MessageRole.USER, content = "next")
|
||||
val synced = mergeTranscriptWithLive(listOf(next), afterEcho)
|
||||
assertEquals(listOf("rest-s-1", "live-reply", "review", "rest-s-3"), synced.map { it.id })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun sessionCreatedMarkerStaysAtStartOfTranscript() {
|
||||
val sessionCreated =
|
||||
|
||||
Reference in new issue
Block a user