No issues found. LGTM! ✅ — gpt-5.5 via Qwen Code /review

No issues found. LGTM! ✅ — gpt-5.5 via Qwen Code /review

Hi @liqiongyu — I tried to reproduce the original duplication scenario end-to-end via tmux against both main and this PR, with a deterministic injection that fires InvalidStreamError(NO_FINISH_REASON) once per process. The PR's regression tests pass, but the actual production-path duplication still reproduces on this branch. Wanted to share the trace before this lands.

How I reproduced

I patched geminiChat.ts to throw an InvalidStreamError once per process, after the inner stream loop completes (mirroring the real processStreamResponse validation path at geminiChat.ts:1111-1124):

// Inserted right after `for await (const chunk of stream) { ... }` in the
// outer retry loop, gated by attempt === 0 and a process-global flag.
if (attempt === 0 && !globalThis.__qwenRetryFired) {
  globalThis.__qwenRetryFired = true;
  throw new InvalidStreamError(
    'forced post-stream retry for visual reproduction',
    'NO_FINISH_REASON',
  );
}

Then ran npm run dev in a 200×50 tmux pane on each branch with the same prompt asking for a 3-paragraph essay separated by blank lines.

Result

• main (no fix): duplication clearly visible — two ✦ markers, two complete essays. Expected.
• This PR (with fix): duplication still visible — two ✦ markers, two complete essays. Not expected.

Why the regression tests pass but production doesn't

The 4 new tests in useGeminiStream.test.tsx > Retry Handling use mocked stream generators where the Retry event is yielded before any Finished event:

yield Content('abcdef');
yield Retry;            // ← retry first
yield Content('abcdef');
yield Finished(STOP);

But the real geminiChat.ts retry path looks different. The chunk loop fully drains, each chunk's finishReason causes turn.ts to yield Finished, then processStreamResponse validates and throws, then the outer loop yields RETRY. So the actual production sequence is:

Content → Content → … → Content → Finished(STOP) → Retry → Content → Finished(STOP)

Finished arrives before Retry, not after. None of the regression tests cover this ordering.

Root cause

I added trace logging at the retryDiscardHistoryItemIds.add(id) call sites and inside the Retry handler. Output during attempt 1:

[TRACE] split-flush id=…782 setSize=1            # P1 flushed to <Static>, tracked
[TRACE] split-flush id=…783 setSize=2            # P2 flushed to <Static>, tracked
[TEST PATCH] forcing post-stream NO_FINISH_REASON
[TRACE] Retry fresh-restart: size=0 ids=[]       # ← Set was already cleared

The Set is empty by the time the Retry handler runs, so the removeHistoryItemsById && retryDiscardHistoryItemIds.size > 0 guard skips the cleanup. The clear happens at useGeminiStream.ts:1271-1275 (see inline comment).

If I comment out that one .clear() and re-run the same scenario, the trace shows size=2 ids=[…782, …783] at the Retry handler and the duplication mostly disappears.

A second, related gap

After fixing the clear, one paragraph still leaks — the last paragraph of attempt 1. That comes from the new pending-flush block inside handleFinishedEvent:

if (finishReason !== FinishReason.MAX_TOKENS && pendingHistoryItemRef.current) {
  addItem(pendingHistoryItemRef.current, userMessageTimestamp);   // id discarded
  setPendingHistoryItem(null);
}

The returned id isn't captured, so this final flushed item can never be removed by a subsequent Retry. Same pattern as the split-flush branches — needs the id added to retryDiscardHistoryItemIds (gated by RETRY_DISCARD_HISTORY_ITEM_TYPES).

Suggested fix

1. Drop the retryDiscardHistoryItemIds.clear() on the non-MAX_TOKENS Finished branch. Let the next attempt's first content/thought event reset the tracking instead, or rely on the Retry handler itself.
2. In handleFinishedEvent, capture the id from addItem(pendingHistoryItemRef.current, …) and add it to retryDiscardHistoryItemIds if the type is in RETRY_DISCARD_HISTORY_ITEM_TYPES.
3. Add a regression test that mirrors the production ordering — Content → Finished(STOP) → Retry → Content → Finished(STOP) — and asserts removeHistoryItemsById is called with the IDs flushed during attempt 1, including the one finalized by the first Finished. The existing should discard streamed content on Retry to avoid duplicates test interleaves Content and Retry without an intervening Finished, which is why this gap slipped through.

This also retroactively justifies @tanzhenxin's earlier ask for log-based confirmation of the root cause — the symptom matches but the cause-effect chain has a separate failure mode that the unit tests don't model.

Happy to share the patch + tmux capture files if useful.

— claude-opus-4-7 via Claude Code /qreview