From bddfbb12c8096a5988d64e368111eb0fb6ebc198 Mon Sep 17 00:00:00 2001
From: sanil-23 <sanil@vezures.xyz>
Date: Wed, 13 May 2026 20:47:54 +0530
Subject: [PATCH] fix(chat): stop duplicating assistant replies on
 multi-segment turns (#1648)

---
 app/src/providers/ChatRuntimeProvider.tsx     | 15 +++--
 .../__tests__/ChatRuntimeProvider.test.tsx    | 66 +++++++++++++++----
 2 files changed, 62 insertions(+), 19 deletions(-)

diff --git a/app/src/providers/ChatRuntimeProvider.tsx b/app/src/providers/ChatRuntimeProvider.tsx
index a627894a3..0425ea057 100644
--- a/app/src/providers/ChatRuntimeProvider.tsx
+++ b/app/src/providers/ChatRuntimeProvider.tsx
@@ -120,6 +120,13 @@ function deleteSegmentDelivery(deliveries: Map<string, SegmentDelivery>, key: st
   deliveries.delete(key);
 }
 
+// Delivery is complete iff every expected segment_index arrived. Do NOT also
+// compare reconstructed segments against event.full_response — the server
+// trims each segment and normalises joiners during segmentation
+// (presentation.rs::segment_for_delivery), while full_response keeps the raw
+// LLM text. A byte-equality check therefore fails on virtually every
+// multi-segment turn and triggers the reconciliation path, producing a
+// duplicate assistant message.
 function hasCompleteSegmentDelivery(
   event: ChatDoneEvent,
   delivery: SegmentDelivery | undefined
@@ -127,14 +134,10 @@ function hasCompleteSegmentDelivery(
   const expected = event.segment_total ?? 0;
   if (expected <= 0 || !delivery) return false;
   if (delivery.segments.size < expected) return false;
-
-  let reconstructed = '';
   for (let i = 0; i < expected; i += 1) {
-    const segment = delivery.segments.get(i);
-    if (segment === undefined) return false;
-    reconstructed += segment;
+    if (!delivery.segments.has(i)) return false;
   }
-  return reconstructed === event.full_response;
+  return true;
 }
 
 function chatDoneExtraMetadata(event: ChatDoneEvent): Record<string, unknown> | undefined {
diff --git a/app/src/providers/__tests__/ChatRuntimeProvider.test.tsx b/app/src/providers/__tests__/ChatRuntimeProvider.test.tsx
index f4dba11cd..14370a0b9 100644
--- a/app/src/providers/__tests__/ChatRuntimeProvider.test.tsx
+++ b/app/src/providers/__tests__/ChatRuntimeProvider.test.tsx
@@ -324,21 +324,26 @@ describe('ChatRuntimeProvider — dedupe, proactive resolution, mid-turn invaria
       expect(threadApi.appendMessage).toHaveBeenCalledTimes(2);
     });
 
-    it('reconciles when segments are present but not in full_response order', async () => {
+    it('does not reconcile when all segments arrived even if full_response differs (trim/joiner)', async () => {
+      // Regression: the server's segmenter trims each segment and joins with
+      // "\n\n", while chat_done.full_response is the raw LLM text. A strict
+      // byte-equality check used to fire reconciliation on every multi-segment
+      // turn — once we have all expected segment_index values, delivery is
+      // complete regardless of full_response content.
       const listeners = renderProvider();
 
       act(() => {
         listeners.onSegment?.({
-          thread_id: 't-out-of-order',
-          request_id: 'r-out-of-order',
-          full_response: 'Alpha',
+          thread_id: 't-trim',
+          request_id: 'r-trim',
+          full_response: 'Hello.',
           segment_index: 0,
           segment_total: 2,
         });
         listeners.onSegment?.({
-          thread_id: 't-out-of-order',
-          request_id: 'r-out-of-order',
-          full_response: 'Beta',
+          thread_id: 't-trim',
+          request_id: 'r-trim',
+          full_response: 'World.',
           segment_index: 1,
           segment_total: 2,
         });
@@ -348,9 +353,44 @@ describe('ChatRuntimeProvider — dedupe, proactive resolution, mid-turn invaria
 
       act(() => {
         listeners.onDone?.({
-          thread_id: 't-out-of-order',
-          request_id: 'r-out-of-order',
-          full_response: 'BetaAlpha',
+          thread_id: 't-trim',
+          request_id: 'r-trim',
+          // Raw LLM output: leading/trailing whitespace + paragraph break.
+          // segments.join('') === 'Hello.World.' !== full_response, but
+          // delivery is still complete.
+          full_response: '\nHello.\n\nWorld.\n',
+          rounds_used: 1,
+          total_input_tokens: 10,
+          total_output_tokens: 20,
+          segment_total: 2,
+        });
+      });
+
+      await waitFor(() => expect(mockRefetchSnapshot).toHaveBeenCalledTimes(1));
+      expect(threadApi.appendMessage).toHaveBeenCalledTimes(2);
+    });
+
+    it('reconciles when a segment is missing', async () => {
+      const listeners = renderProvider();
+
+      act(() => {
+        listeners.onSegment?.({
+          thread_id: 't-missing',
+          request_id: 'r-missing',
+          full_response: 'Part one.',
+          segment_index: 0,
+          segment_total: 2,
+        });
+        // segment_index 1 never arrives.
+      });
+
+      await waitFor(() => expect(threadApi.appendMessage).toHaveBeenCalledTimes(1));
+
+      act(() => {
+        listeners.onDone?.({
+          thread_id: 't-missing',
+          request_id: 'r-missing',
+          full_response: 'Part one.\n\nPart two.',
           rounds_used: 1,
           total_input_tokens: 10,
           total_output_tokens: 20,
@@ -360,11 +400,11 @@ describe('ChatRuntimeProvider — dedupe, proactive resolution, mid-turn invaria
 
       await waitFor(() =>
         expect(threadApi.appendMessage).toHaveBeenCalledWith(
-          't-out-of-order',
-          expect.objectContaining({ content: 'BetaAlpha', sender: 'agent' })
+          't-missing',
+          expect.objectContaining({ content: 'Part one.\n\nPart two.', sender: 'agent' })
         )
       );
-      expect(threadApi.appendMessage).toHaveBeenCalledTimes(3);
+      expect(threadApi.appendMessage).toHaveBeenCalledTimes(2);
     });
 
     it('expires stale segment delivery state before chat_done reconciliation', async () => {