fix(tui): avoid duplicate assistant reply after history replay#96986
Closed
snowzlmbot wants to merge 1 commit into
Closed
fix(tui): avoid duplicate assistant reply after history replay#96986snowzlmbot wants to merge 1 commit into
snowzlmbot wants to merge 1 commit into
Conversation
Contributor
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Keep open: this is a plausible small TUI ChatLog fix, but the contributor proof is still a dependency-free ChatLog harness rather than a real TUI/Gateway/Telegram run, and sibling PRs still target the same open bug. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Canonical path: Close this PR as superseded by #93562. So I’m closing this here and keeping the remaining discussion on #93562. Review detailsBest possible solution: Close this PR as superseded by #93562. Do we have a high-confidence way to reproduce the issue? Yes at source level: current main has a no-run-id history replay path and a run-id live final path that can append identical assistant text. I did not establish a live Telegram/TUI reproduction in this read-only review. Is this the best way to solve the issue? Unclear: ChatLog is a plausible narrow owner boundary for visible row dedupe, but sibling PRs propose alternatives and no maintainer has selected the canonical patch yet. Security review: Security review cleared: The diff only changes in-process TUI ChatLog state and tests, with no dependency, workflow, secret, package, or code-execution surface changes. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against 1cd6f81a46ae. |
clawsweeper
Bot
added
rating: 🦪 silver shellfish
This was referenced Jun 26, 2026
Contributor
Author
|
Updated the PR body and branch after review: history replay now uses an explicit ChatLog API, and the regression coverage exercises the loadHistory + live final event-handler path with a real ChatLog. @clawsweeper re-review |
Contributor
|
🦞🧹 I asked ClawSweeper to review this item again. |
xialonglee
added a commit
to xialonglee/openclaw
that referenced
this pull request
Jun 29, 2026
…racking Drop the lastHistoryAssistant text-matching dedupe in ChatLog (posed P1 false-suppression risk). Instead, track active runIds surrendered to loadHistory() on sessions.changed 'new' — late events for surrendered runs are suppressed by exact runId match. Add ChatLog.hasStreamingRun() so the event handler can detect when loadHistory restored a surrendered run as in-flight streaming. Retain chatFinalizedRuns / chatFinalizedRunsWithDisplay from the original fix for post-reload delta/error dedup of finalized runs. Incorporates the precise surrender-tracking approach from closed PRs openclaw#96979/openclaw#96986 while keeping the scoped ChatLog API minimal.
steipete
pushed a commit
to xialonglee/openclaw
that referenced
this pull request
Jul 7, 2026
…racking Drop the lastHistoryAssistant text-matching dedupe in ChatLog (posed P1 false-suppression risk). Instead, track active runIds surrendered to loadHistory() on sessions.changed 'new' — late events for surrendered runs are suppressed by exact runId match. Add ChatLog.hasStreamingRun() so the event handler can detect when loadHistory restored a surrendered run as in-flight streaming. Retain chatFinalizedRuns / chatFinalizedRunsWithDisplay from the original fix for post-reload delta/error dedup of finalized runs. Incorporates the precise surrender-tracking approach from closed PRs openclaw#96979/openclaw#96986 while keeping the scoped ChatLog API minimal.
steipete
pushed a commit
to xialonglee/openclaw
that referenced
this pull request
Jul 7, 2026
…racking Drop the lastHistoryAssistant text-matching dedupe in ChatLog (posed P1 false-suppression risk). Instead, track active runIds surrendered to loadHistory() on sessions.changed 'new' — late events for surrendered runs are suppressed by exact runId match. Add ChatLog.hasStreamingRun() so the event handler can detect when loadHistory restored a surrendered run as in-flight streaming. Retain chatFinalizedRuns / chatFinalizedRunsWithDisplay from the original fix for post-reload delta/error dedup of finalized runs. Incorporates the precise surrender-tracking approach from closed PRs openclaw#96979/openclaw#96986 while keeping the scoped ChatLog API minimal.
steipete
added a commit
that referenced
this pull request
Jul 7, 2026
…ad (#96980) * fix(tui): suppress adjacent history-replay/live-final duplicate in ChatLog (#96967) When sessions.changed triggers loadHistory(), assistant rows are replayed via finalizeAssistant(text) without a runId. A late live final with the same text and a runId can land immediately afterward, producing a visible duplicate assistant reply. Track the most recent history-replay assistant component in ChatLog and suppress a following live final when the text matches and no other component was appended in between. This is simpler and more targeted than the prior event-handler approach because it operates at the rendering layer where the duplicate is actually visible. Fixes #96967 * fix(tui): bound replay-final dedupe window in ChatLog * fix(tui): bound replay-final dedupe marker with TTL expiration * fix(tui): satisfy no-promise-executor-return lint rule in chat-log test * fix(tui): replace ChatLog text heuristic with runId-based surrender tracking Drop the lastHistoryAssistant text-matching dedupe in ChatLog (posed P1 false-suppression risk). Instead, track active runIds surrendered to loadHistory() on sessions.changed 'new' — late events for surrendered runs are suppressed by exact runId match. Add ChatLog.hasStreamingRun() so the event handler can detect when loadHistory restored a surrendered run as in-flight streaming. Retain chatFinalizedRuns / chatFinalizedRunsWithDisplay from the original fix for post-reload delta/error dedup of finalized runs. Incorporates the precise surrender-tracking approach from closed PRs #96979/#96986 while keeping the scoped ChatLog API minimal. * fix(tui): render displayable late final for surrendered non-streaming run When a sessions.changed 'new' surrenders an in-flight run and loadHistory does not restore it as streaming, a late displayable final must still be rendered — otherwise the user never sees the assistant reply. Only suppress non-displayable finals (empty message, no errorMessage). Add focused test for non-displayable final suppression. * test(tui): add coverage for empty in-flight restore + late displayable final Covers the rank-up scenario from #96979: surrendered run restored by loadHistory as empty streaming, late displayable final passes through surrender check and renders to chatLog. * fix(tui): suppress late finals for surrendered finalized runs to prevent history-replay duplicate When sessions.changed 'new' fires, now surrender chatFinalizedRuns entries too (as 'finalized'), not just sessionRuns ('in-flight'). This prevents the finalized-before-reload history-replay duplicate: a late final for an already-displayed run gets suppressed because the history-replay static row covers the visible content. In-flight surrendered runs still render displayable finals because the user may not have seen the streaming text before the reload. SurrenderedToHistoryRunIds is now Map<string,'in-flight'|'finalized'> so the surrender check can branch on source. Move surrender check before the finalizedRuns/chatFinalizedRuns guard so surrendered finalized runs are routed correctly. Add focused tests for history-replayed-visible (suppress) and history-replay-missing (render) late finals. * fix(tui): extend surrender coverage to sessions.changed reset reload path Reset also calls clearTrackedRunState() + loadHistory() with the same sessionKey, so late chat events for runs from the old session window can race with the history replay the same way as 'new'. Surrender sessionRuns and chatFinalizedRuns on both reasons (#96979 P1 rank-up). * fix(tui): keep surrendered finalized marker after late delta to block later final For finalized surrendered runs, a late delta must not clear the surrender marker — otherwise a subsequent late final would slip through and produce a duplicate. Only in-flight surrendered runs clear the marker on delta (their final needs to render). * fix(tui): gate session history reloads on persistence Co-authored-by: xialonglee <[email protected]> * docs(changelog): note TUI external-run dedupe * test(tui): type history load harness results * test(tui): defer history results without async mocks * chore(changelog): defer TUI note to release --------- Co-authored-by: Peter Steinberger <[email protected]>
github-actions Bot
pushed a commit
to Desicool/openclaw
that referenced
this pull request
Jul 7, 2026
…ad (openclaw#96980) * fix(tui): suppress adjacent history-replay/live-final duplicate in ChatLog (openclaw#96967) When sessions.changed triggers loadHistory(), assistant rows are replayed via finalizeAssistant(text) without a runId. A late live final with the same text and a runId can land immediately afterward, producing a visible duplicate assistant reply. Track the most recent history-replay assistant component in ChatLog and suppress a following live final when the text matches and no other component was appended in between. This is simpler and more targeted than the prior event-handler approach because it operates at the rendering layer where the duplicate is actually visible. Fixes openclaw#96967 * fix(tui): bound replay-final dedupe window in ChatLog * fix(tui): bound replay-final dedupe marker with TTL expiration * fix(tui): satisfy no-promise-executor-return lint rule in chat-log test * fix(tui): replace ChatLog text heuristic with runId-based surrender tracking Drop the lastHistoryAssistant text-matching dedupe in ChatLog (posed P1 false-suppression risk). Instead, track active runIds surrendered to loadHistory() on sessions.changed 'new' — late events for surrendered runs are suppressed by exact runId match. Add ChatLog.hasStreamingRun() so the event handler can detect when loadHistory restored a surrendered run as in-flight streaming. Retain chatFinalizedRuns / chatFinalizedRunsWithDisplay from the original fix for post-reload delta/error dedup of finalized runs. Incorporates the precise surrender-tracking approach from closed PRs openclaw#96979/openclaw#96986 while keeping the scoped ChatLog API minimal. * fix(tui): render displayable late final for surrendered non-streaming run When a sessions.changed 'new' surrenders an in-flight run and loadHistory does not restore it as streaming, a late displayable final must still be rendered — otherwise the user never sees the assistant reply. Only suppress non-displayable finals (empty message, no errorMessage). Add focused test for non-displayable final suppression. * test(tui): add coverage for empty in-flight restore + late displayable final Covers the rank-up scenario from openclaw#96979: surrendered run restored by loadHistory as empty streaming, late displayable final passes through surrender check and renders to chatLog. * fix(tui): suppress late finals for surrendered finalized runs to prevent history-replay duplicate When sessions.changed 'new' fires, now surrender chatFinalizedRuns entries too (as 'finalized'), not just sessionRuns ('in-flight'). This prevents the finalized-before-reload history-replay duplicate: a late final for an already-displayed run gets suppressed because the history-replay static row covers the visible content. In-flight surrendered runs still render displayable finals because the user may not have seen the streaming text before the reload. SurrenderedToHistoryRunIds is now Map<string,'in-flight'|'finalized'> so the surrender check can branch on source. Move surrender check before the finalizedRuns/chatFinalizedRuns guard so surrendered finalized runs are routed correctly. Add focused tests for history-replayed-visible (suppress) and history-replay-missing (render) late finals. * fix(tui): extend surrender coverage to sessions.changed reset reload path Reset also calls clearTrackedRunState() + loadHistory() with the same sessionKey, so late chat events for runs from the old session window can race with the history replay the same way as 'new'. Surrender sessionRuns and chatFinalizedRuns on both reasons (openclaw#96979 P1 rank-up). * fix(tui): keep surrendered finalized marker after late delta to block later final For finalized surrendered runs, a late delta must not clear the surrender marker — otherwise a subsequent late final would slip through and produce a duplicate. Only in-flight surrendered runs clear the marker on delta (their final needs to render). * fix(tui): gate session history reloads on persistence Co-authored-by: xialonglee <[email protected]> * docs(changelog): note TUI external-run dedupe * test(tui): type history load harness results * test(tui): defer history results without async mocks * chore(changelog): defer TUI note to release --------- Co-authored-by: Peter Steinberger <[email protected]>
sheyanmin
pushed a commit
to sheyanmin/openclaw
that referenced
this pull request
Jul 8, 2026
…ad (openclaw#96980) * fix(tui): suppress adjacent history-replay/live-final duplicate in ChatLog (openclaw#96967) When sessions.changed triggers loadHistory(), assistant rows are replayed via finalizeAssistant(text) without a runId. A late live final with the same text and a runId can land immediately afterward, producing a visible duplicate assistant reply. Track the most recent history-replay assistant component in ChatLog and suppress a following live final when the text matches and no other component was appended in between. This is simpler and more targeted than the prior event-handler approach because it operates at the rendering layer where the duplicate is actually visible. Fixes openclaw#96967 * fix(tui): bound replay-final dedupe window in ChatLog * fix(tui): bound replay-final dedupe marker with TTL expiration * fix(tui): satisfy no-promise-executor-return lint rule in chat-log test * fix(tui): replace ChatLog text heuristic with runId-based surrender tracking Drop the lastHistoryAssistant text-matching dedupe in ChatLog (posed P1 false-suppression risk). Instead, track active runIds surrendered to loadHistory() on sessions.changed 'new' — late events for surrendered runs are suppressed by exact runId match. Add ChatLog.hasStreamingRun() so the event handler can detect when loadHistory restored a surrendered run as in-flight streaming. Retain chatFinalizedRuns / chatFinalizedRunsWithDisplay from the original fix for post-reload delta/error dedup of finalized runs. Incorporates the precise surrender-tracking approach from closed PRs openclaw#96979/openclaw#96986 while keeping the scoped ChatLog API minimal. * fix(tui): render displayable late final for surrendered non-streaming run When a sessions.changed 'new' surrenders an in-flight run and loadHistory does not restore it as streaming, a late displayable final must still be rendered — otherwise the user never sees the assistant reply. Only suppress non-displayable finals (empty message, no errorMessage). Add focused test for non-displayable final suppression. * test(tui): add coverage for empty in-flight restore + late displayable final Covers the rank-up scenario from openclaw#96979: surrendered run restored by loadHistory as empty streaming, late displayable final passes through surrender check and renders to chatLog. * fix(tui): suppress late finals for surrendered finalized runs to prevent history-replay duplicate When sessions.changed 'new' fires, now surrender chatFinalizedRuns entries too (as 'finalized'), not just sessionRuns ('in-flight'). This prevents the finalized-before-reload history-replay duplicate: a late final for an already-displayed run gets suppressed because the history-replay static row covers the visible content. In-flight surrendered runs still render displayable finals because the user may not have seen the streaming text before the reload. SurrenderedToHistoryRunIds is now Map<string,'in-flight'|'finalized'> so the surrender check can branch on source. Move surrender check before the finalizedRuns/chatFinalizedRuns guard so surrendered finalized runs are routed correctly. Add focused tests for history-replayed-visible (suppress) and history-replay-missing (render) late finals. * fix(tui): extend surrender coverage to sessions.changed reset reload path Reset also calls clearTrackedRunState() + loadHistory() with the same sessionKey, so late chat events for runs from the old session window can race with the history replay the same way as 'new'. Surrender sessionRuns and chatFinalizedRuns on both reasons (openclaw#96979 P1 rank-up). * fix(tui): keep surrendered finalized marker after late delta to block later final For finalized surrendered runs, a late delta must not clear the surrender marker — otherwise a subsequent late final would slip through and produce a duplicate. Only in-flight surrendered runs clear the marker on delta (their final needs to render). * fix(tui): gate session history reloads on persistence Co-authored-by: xialonglee <[email protected]> * docs(changelog): note TUI external-run dedupe * test(tui): type history load harness results * test(tui): defer history results without async mocks * chore(changelog): defer TUI note to release --------- Co-authored-by: Peter Steinberger <[email protected]>
giodl73-repo
pushed a commit
to giodl73-repo/openclaw
that referenced
this pull request
Jul 8, 2026
…ad (openclaw#96980) * fix(tui): suppress adjacent history-replay/live-final duplicate in ChatLog (openclaw#96967) When sessions.changed triggers loadHistory(), assistant rows are replayed via finalizeAssistant(text) without a runId. A late live final with the same text and a runId can land immediately afterward, producing a visible duplicate assistant reply. Track the most recent history-replay assistant component in ChatLog and suppress a following live final when the text matches and no other component was appended in between. This is simpler and more targeted than the prior event-handler approach because it operates at the rendering layer where the duplicate is actually visible. Fixes openclaw#96967 * fix(tui): bound replay-final dedupe window in ChatLog * fix(tui): bound replay-final dedupe marker with TTL expiration * fix(tui): satisfy no-promise-executor-return lint rule in chat-log test * fix(tui): replace ChatLog text heuristic with runId-based surrender tracking Drop the lastHistoryAssistant text-matching dedupe in ChatLog (posed P1 false-suppression risk). Instead, track active runIds surrendered to loadHistory() on sessions.changed 'new' — late events for surrendered runs are suppressed by exact runId match. Add ChatLog.hasStreamingRun() so the event handler can detect when loadHistory restored a surrendered run as in-flight streaming. Retain chatFinalizedRuns / chatFinalizedRunsWithDisplay from the original fix for post-reload delta/error dedup of finalized runs. Incorporates the precise surrender-tracking approach from closed PRs openclaw#96979/openclaw#96986 while keeping the scoped ChatLog API minimal. * fix(tui): render displayable late final for surrendered non-streaming run When a sessions.changed 'new' surrenders an in-flight run and loadHistory does not restore it as streaming, a late displayable final must still be rendered — otherwise the user never sees the assistant reply. Only suppress non-displayable finals (empty message, no errorMessage). Add focused test for non-displayable final suppression. * test(tui): add coverage for empty in-flight restore + late displayable final Covers the rank-up scenario from openclaw#96979: surrendered run restored by loadHistory as empty streaming, late displayable final passes through surrender check and renders to chatLog. * fix(tui): suppress late finals for surrendered finalized runs to prevent history-replay duplicate When sessions.changed 'new' fires, now surrender chatFinalizedRuns entries too (as 'finalized'), not just sessionRuns ('in-flight'). This prevents the finalized-before-reload history-replay duplicate: a late final for an already-displayed run gets suppressed because the history-replay static row covers the visible content. In-flight surrendered runs still render displayable finals because the user may not have seen the streaming text before the reload. SurrenderedToHistoryRunIds is now Map<string,'in-flight'|'finalized'> so the surrender check can branch on source. Move surrender check before the finalizedRuns/chatFinalizedRuns guard so surrendered finalized runs are routed correctly. Add focused tests for history-replayed-visible (suppress) and history-replay-missing (render) late finals. * fix(tui): extend surrender coverage to sessions.changed reset reload path Reset also calls clearTrackedRunState() + loadHistory() with the same sessionKey, so late chat events for runs from the old session window can race with the history replay the same way as 'new'. Surrender sessionRuns and chatFinalizedRuns on both reasons (openclaw#96979 P1 rank-up). * fix(tui): keep surrendered finalized marker after late delta to block later final For finalized surrendered runs, a late delta must not clear the surrender marker — otherwise a subsequent late final would slip through and produce a duplicate. Only in-flight surrendered runs clear the marker on delta (their final needs to render). * fix(tui): gate session history reloads on persistence Co-authored-by: xialonglee <[email protected]> * docs(changelog): note TUI external-run dedupe * test(tui): type history load harness results * test(tui): defer history results without async mocks * chore(changelog): defer TUI note to release --------- Co-authored-by: Peter Steinberger <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #96967
Summary
sessions.changednew/resetreload behavior intact so external session refreshes still rebuild the current session.ChatLogunit coverage for the replay/live duplicate case and the adjacent cases that must still render duplicate text.What Problem This Solves
Fixes an issue where users viewing the TUI could see the same assistant reply twice when a Telegram-originated turn caused history replay and the live final message to meet in the current chat log.
Root Cause
History replay renders assistant messages without a run id, while live final events render with a run id. The existing run-id dedupe cannot relate those two paths, so a replayed assistant row and its matching live final can be appended as separate visible rows when the events arrive adjacent to each other.
Why This Change Was Made
The fix is scoped to
ChatLog: it records only the most recent no-run-id assistant row from history replay and suppresses a live final with a run id only when it immediately follows that replayed row, has identical text, and arrives within a short TTL. The marker is cleared when other content is appended, the log is cleared, or the tracked component is pruned, so intentional repeated messages remain visible.This avoids a broader event-handler change and preserves the
sessions.changedreload path needed for external sessionnew/resetupdates.User Impact
TUI users should no longer see a duplicated assistant reply for the reported replay/live-final interleaving. Legitimate repeated assistant output remains visible when separated by user content or when both messages are live finals.
Validation / Pre-checks
git diff --check— passed as a local pre-check.git diff --cached --check— passed before committing.node_modulesis absent), and no dependency install was performed for the local pre-check.Evidence
Focused tests added in
src/tui/components/chat-log.test.tscover:Evidence source map
src/tui/tui-session-actions.tsrebuilds assistant history viachatLog.finalizeAssistant(text)without a run id.src/tui/tui-event-handlers.tsfinal chat events render viachatLog.finalizeAssistant(finalText, evt.runId).src/tui/components/chat-log.tsnow dedupes only the adjacent history-replay/live-final pair.src/tui/components/chat-log.test.tscovers the suppression and allowed-duplicate boundaries.Risk / Rollback / Docs
Follow-up proof added after ClawSweeper review
Terminal behavior proof: sessions.changed replay + live final leaves one visible assistant row
A local, dependency-free Node 24 harness was run against this branch at
6acf71618376655a80a6f2f1104a256cd3e14c20. The harness loads the PR'ssrc/tui/components/chat-log.tssource into a temporary module with minimal rendering stubs only, then exercises the same TUI boundary ordering described in the issue:tui-session-actions.ts) callschatLog.finalizeAssistant(text)with no run id;tui-event-handlers.ts) callschatLog.finalizeAssistant(text, runId)with a run id;Output:
This avoids external messaging and does not mutate any active user/Gateway/Telegram/TUI configuration. A true native Telegram/TUI visual capture would require maintainer-controlled Mantis/Telegram desktop infrastructure; the useful maintainer trigger remains:
CI / check evidence
6acf71618376655a80a6f2f1104a256cd3e14c20).test/vitest/vitest.tui-pty.config.tsstarted at 08:19:17 UTC and passedsrc/tui/tui-pty-harness.e2e.test.tsplussrc/tui/tui-pty-local.e2e.test.ts.Real behavior proofworkflow check passed in run https://github.com/openclaw/openclaw/actions/runs/28226954824 with log lineExternal PR includes problem context and evidence.src/tui/components/chat-log.test.ts; the local targeted Vitest command below could not run because this checkout intentionally has no installed dependencies.Local targeted test attempt
No dependency install was performed.
OpenGrep changed paths diagnosis
The failed OpenGrep PR-diff check did not reach repository scanning. It failed while installing OpenGrep, before the changed-path scan step:
That appears to be an OpenGrep install/network workflow failure, not an actionable finding in this PR's two changed TUI files.