Skip to content

Commit 6192b03

Browse files
xialongleesteipete
xialonglee
and
steipete
authored
fix(tui): deduplicate assistant messages across sessions.changed reload (#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]>
1 parent a2f95de commit 6192b03

6 files changed

Lines changed: 536 additions & 12 deletions

‎src/tui/tui-command-handlers.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ type CommandHandlerContext = {
5555
openOverlay: (component: Component) => OverlayHandle;
5656
closeOverlay: (handle?: OverlayHandle) => void;
5757
refreshSessionInfo: () => Promise<void>;
58-
loadHistory: () => Promise<void>;
58+
loadHistory: () => Promise<unknown>;
5959
setSession: (key: string) => Promise<void>;
6060
refreshAgents: () => Promise<void>;
6161
abortActive: (params?: { preferActive?: boolean }) => Promise<void>;

‎src/tui/tui-event-handlers.test.ts‎

Lines changed: 348 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import type {
77
BtwEvent,
88
ChatEvent,
99
SessionChangedEvent,
10+
TuiHistoryLoadResult,
1011
TuiStateAccess,
1112
} from "./tui-types.js";
1213

@@ -100,7 +101,10 @@ describe("tui-event-handlers: handleAgentEvent", () => {
100101
const btw = createMockBtwPresenter();
101102
const tui = { requestRender: vi.fn() } as unknown as MockTui & HandlerTui;
102103
const setActivityStatus = vi.fn();
103-
const loadHistory = vi.fn();
104+
const loadHistory = vi.fn<() => Promise<TuiHistoryLoadResult>>(async () => ({
105+
loaded: true,
106+
inFlightRunId: null,
107+
}));
104108
const localRunIds = new Set<string>();
105109
const localBtwRunIds = new Set<string>();
106110
const noteLocalRunId = (runId: string) => {
@@ -1526,9 +1530,10 @@ describe("tui-event-handlers: handleAgentEvent", () => {
15261530
});
15271531

15281532
noteLocalRunId("run-local-empty");
1529-
loadHistory.mockImplementation(() => {
1533+
loadHistory.mockImplementation(async () => {
15301534
expect(state.activeChatRunId).toBeNull();
15311535
expect(state.activityStatus).toBe("idle");
1536+
return { loaded: true, inFlightRunId: null };
15321537
});
15331538

15341539
handleChatEvent({
@@ -1861,6 +1866,347 @@ describe("tui-event-handlers: handleAgentEvent", () => {
18611866

18621867
expect(loadHistory).toHaveBeenCalledTimes(1);
18631868
});
1869+
1870+
describe("sessions.changed history reload", () => {
1871+
const startRun = (
1872+
state: TuiStateAccess,
1873+
handleChatEvent: ReturnType<typeof createHandlersHarness>["handleChatEvent"],
1874+
runId: string,
1875+
) => {
1876+
handleChatEvent({
1877+
runId,
1878+
sessionKey: state.currentSessionKey,
1879+
state: "delta",
1880+
message: { content: [{ type: "text", text: "typing" }] },
1881+
});
1882+
};
1883+
1884+
const changeSession = (
1885+
state: TuiStateAccess,
1886+
handleSessionsChangedEvent: ReturnType<
1887+
typeof createHandlersHarness
1888+
>["handleSessionsChangedEvent"],
1889+
sessionId = state.currentSessionId,
1890+
) => {
1891+
handleSessionsChangedEvent({
1892+
sessionKey: state.currentSessionKey,
1893+
reason: "new",
1894+
sessionId: sessionId ?? undefined,
1895+
updatedAt: 200,
1896+
} satisfies SessionChangedEvent);
1897+
};
1898+
1899+
const finishPersistence = (
1900+
state: TuiStateAccess,
1901+
handleSessionsChangedEvent: ReturnType<
1902+
typeof createHandlersHarness
1903+
>["handleSessionsChangedEvent"],
1904+
runId: string,
1905+
) => {
1906+
handleSessionsChangedEvent({
1907+
sessionKey: state.currentSessionKey,
1908+
phase: "end",
1909+
runId,
1910+
} satisfies SessionChangedEvent);
1911+
};
1912+
1913+
const deferNextHistoryLoad = (loadHistory: MockFn) => {
1914+
let resolveHistory: (result: TuiHistoryLoadResult) => void = () => {};
1915+
const result = new Promise<TuiHistoryLoadResult>((resolve) => {
1916+
resolveHistory = resolve;
1917+
});
1918+
loadHistory.mockReturnValueOnce(result);
1919+
return (loaded: boolean, inFlightRunId: string | null = null) =>
1920+
resolveHistory(loaded ? { loaded: true, inFlightRunId } : { loaded: false });
1921+
};
1922+
1923+
it("waits for terminal persistence before rebuilding an active external run", async () => {
1924+
const { state, chatLog, loadHistory, handleChatEvent, handleSessionsChangedEvent } =
1925+
createHandlersHarness({ state: { activeChatRunId: "run-active" } });
1926+
startRun(state, handleChatEvent, "run-active");
1927+
chatLog.finalizeAssistant.mockClear();
1928+
loadHistory.mockClear();
1929+
1930+
changeSession(state, handleSessionsChangedEvent);
1931+
expect(loadHistory).not.toHaveBeenCalled();
1932+
1933+
handleChatEvent({
1934+
runId: "run-active",
1935+
sessionKey: state.currentSessionKey,
1936+
state: "final",
1937+
message: { content: [{ type: "text", text: "reply" }] },
1938+
});
1939+
expect(chatLog.finalizeAssistant).toHaveBeenCalledTimes(1);
1940+
1941+
finishPersistence(state, handleSessionsChangedEvent, "run-active");
1942+
await vi.waitFor(() => expect(loadHistory).toHaveBeenCalledTimes(1));
1943+
await vi.waitFor(() => expect(state.activeChatRunId).toBeNull());
1944+
expect(state.activityStatus).toBe("idle");
1945+
1946+
handleChatEvent({
1947+
runId: "run-active",
1948+
sessionKey: state.currentSessionKey,
1949+
state: "final",
1950+
message: { content: [{ type: "text", text: "reply" }] },
1951+
});
1952+
expect(chatLog.finalizeAssistant).toHaveBeenCalledTimes(1);
1953+
});
1954+
1955+
it("suppresses a final that arrives while persisted history is rebuilding", async () => {
1956+
const { state, chatLog, loadHistory, handleChatEvent, handleSessionsChangedEvent } =
1957+
createHandlersHarness({ state: { activeChatRunId: "run-active" } });
1958+
const resolveHistory = deferNextHistoryLoad(loadHistory);
1959+
startRun(state, handleChatEvent, "run-active");
1960+
chatLog.finalizeAssistant.mockClear();
1961+
changeSession(state, handleSessionsChangedEvent);
1962+
finishPersistence(state, handleSessionsChangedEvent, "run-active");
1963+
1964+
handleChatEvent({
1965+
runId: "run-active",
1966+
sessionKey: state.currentSessionKey,
1967+
state: "final",
1968+
message: { content: [{ type: "text", text: "history-owned" }] },
1969+
});
1970+
expect(chatLog.finalizeAssistant).not.toHaveBeenCalled();
1971+
1972+
resolveHistory(true);
1973+
await Promise.resolve();
1974+
expect(chatLog.finalizeAssistant).not.toHaveBeenCalled();
1975+
});
1976+
1977+
it("replays a deferred final when the history rebuild fails", async () => {
1978+
const { state, chatLog, loadHistory, handleChatEvent, handleSessionsChangedEvent } =
1979+
createHandlersHarness({ state: { activeChatRunId: "run-active" } });
1980+
const resolveHistory = deferNextHistoryLoad(loadHistory);
1981+
startRun(state, handleChatEvent, "run-active");
1982+
chatLog.finalizeAssistant.mockClear();
1983+
changeSession(state, handleSessionsChangedEvent);
1984+
finishPersistence(state, handleSessionsChangedEvent, "run-active");
1985+
handleChatEvent({
1986+
runId: "run-active",
1987+
sessionKey: state.currentSessionKey,
1988+
state: "final",
1989+
message: { content: [{ type: "text", text: "fallback reply" }] },
1990+
});
1991+
1992+
resolveHistory(false);
1993+
await vi.waitFor(() =>
1994+
expect(chatLog.finalizeAssistant).toHaveBeenCalledWith("fallback reply", "run-active"),
1995+
);
1996+
});
1997+
1998+
it("terminates a persisted run when history fails before its final arrives", async () => {
1999+
const {
2000+
state,
2001+
chatLog,
2002+
loadHistory,
2003+
setActivityStatus,
2004+
handleChatEvent,
2005+
handleSessionsChangedEvent,
2006+
} = createHandlersHarness({ state: { activeChatRunId: "run-active" } });
2007+
const resolveHistory = deferNextHistoryLoad(loadHistory);
2008+
startRun(state, handleChatEvent, "run-active");
2009+
chatLog.finalizeAssistant.mockClear();
2010+
changeSession(state, handleSessionsChangedEvent);
2011+
finishPersistence(state, handleSessionsChangedEvent, "run-active");
2012+
2013+
resolveHistory(false);
2014+
await vi.waitFor(() => expect(state.activeChatRunId).toBeNull());
2015+
expect(setActivityStatus).toHaveBeenCalledWith("idle");
2016+
2017+
handleChatEvent({
2018+
runId: "run-active",
2019+
sessionKey: state.currentSessionKey,
2020+
state: "final",
2021+
message: { content: [{ type: "text", text: "late fallback" }] },
2022+
});
2023+
expect(chatLog.finalizeAssistant).toHaveBeenCalledWith("late fallback", "run-active");
2024+
});
2025+
2026+
it("uses an already-observed persistence barrier for a recently finalized run", async () => {
2027+
const { state, chatLog, loadHistory, handleChatEvent, handleSessionsChangedEvent } =
2028+
createHandlersHarness({ state: { activeChatRunId: "run-done" } });
2029+
handleChatEvent({
2030+
runId: "run-done",
2031+
sessionKey: state.currentSessionKey,
2032+
state: "final",
2033+
message: { content: [{ type: "text", text: "done" }] },
2034+
});
2035+
finishPersistence(state, handleSessionsChangedEvent, "run-done");
2036+
loadHistory.mockClear();
2037+
2038+
changeSession(state, handleSessionsChangedEvent);
2039+
await vi.waitFor(() => expect(loadHistory).toHaveBeenCalledTimes(1));
2040+
expect(chatLog.finalizeAssistant).toHaveBeenCalledTimes(1);
2041+
});
2042+
2043+
it("preserves finalized-run dedupe across a delayed session reload", async () => {
2044+
const now = vi.spyOn(Date, "now").mockReturnValue(1_000);
2045+
const { state, chatLog, loadHistory, handleChatEvent, handleSessionsChangedEvent } =
2046+
createHandlersHarness({ state: { activeChatRunId: "run-done" } });
2047+
handleChatEvent({
2048+
runId: "run-done",
2049+
sessionKey: state.currentSessionKey,
2050+
state: "final",
2051+
message: { content: [{ type: "text", text: "done" }] },
2052+
});
2053+
finishPersistence(state, handleSessionsChangedEvent, "run-done");
2054+
loadHistory.mockClear();
2055+
now.mockReturnValue(12_000);
2056+
2057+
changeSession(state, handleSessionsChangedEvent);
2058+
await vi.waitFor(() => expect(loadHistory).toHaveBeenCalledTimes(1));
2059+
handleChatEvent({
2060+
runId: "run-done",
2061+
sessionKey: state.currentSessionKey,
2062+
state: "final",
2063+
message: { content: [{ type: "text", text: "done" }] },
2064+
});
2065+
2066+
expect(chatLog.finalizeAssistant).toHaveBeenCalledTimes(1);
2067+
now.mockRestore();
2068+
});
2069+
2070+
it("keeps a later terminal reload queued behind the current rebuild", async () => {
2071+
const { state, loadHistory, handleChatEvent, handleSessionsChangedEvent } =
2072+
createHandlersHarness({ state: { activeChatRunId: "run-a" } });
2073+
const resolveFirstHistory = deferNextHistoryLoad(loadHistory);
2074+
startRun(state, handleChatEvent, "run-a");
2075+
startRun(state, handleChatEvent, "run-b");
2076+
changeSession(state, handleSessionsChangedEvent);
2077+
2078+
finishPersistence(state, handleSessionsChangedEvent, "run-a");
2079+
finishPersistence(state, handleSessionsChangedEvent, "run-b");
2080+
expect(loadHistory).toHaveBeenCalledTimes(1);
2081+
2082+
resolveFirstHistory(true);
2083+
await vi.waitFor(() => expect(loadHistory).toHaveBeenCalledTimes(2), { timeout: 3_000 });
2084+
});
2085+
2086+
it("preserves immediate reload behavior when new replaces a known session", () => {
2087+
const { state, loadHistory, setActivityStatus, handleChatEvent, handleSessionsChangedEvent } =
2088+
createHandlersHarness({
2089+
state: {
2090+
activeChatRunId: "run-old",
2091+
currentSessionId: "session-old",
2092+
activityStatus: "streaming",
2093+
},
2094+
});
2095+
startRun(state, handleChatEvent, "run-old");
2096+
loadHistory.mockClear();
2097+
2098+
changeSession(state, handleSessionsChangedEvent, "session-new");
2099+
2100+
expect(loadHistory).toHaveBeenCalledTimes(1);
2101+
expect(state.currentSessionId).toBe("session-new");
2102+
expect(state.activeChatRunId).toBeNull();
2103+
expect(setActivityStatus).toHaveBeenCalledWith("idle");
2104+
});
2105+
2106+
it("preserves an in-flight run adopted by reset history", async () => {
2107+
const { state, loadHistory, handleChatEvent, handleSessionsChangedEvent } =
2108+
createHandlersHarness({
2109+
state: { activeChatRunId: "run-reset", activityStatus: "streaming" },
2110+
});
2111+
startRun(state, handleChatEvent, "run-reset");
2112+
loadHistory.mockImplementationOnce(async () => {
2113+
state.activeChatRunId = "run-reset";
2114+
state.activityStatus = "streaming";
2115+
return { loaded: true as const, inFlightRunId: "run-reset" };
2116+
});
2117+
2118+
handleSessionsChangedEvent({
2119+
sessionKey: state.currentSessionKey,
2120+
reason: "reset",
2121+
sessionId: state.currentSessionId ?? undefined,
2122+
} satisfies SessionChangedEvent);
2123+
2124+
await vi.waitFor(() => expect(loadHistory).toHaveBeenCalledTimes(1));
2125+
expect(state.activeChatRunId).toBe("run-reset");
2126+
expect(state.activityStatus).toBe("streaming");
2127+
});
2128+
2129+
it("preserves finalized-run dedupe after reset history reload", async () => {
2130+
const { state, chatLog, loadHistory, handleChatEvent, handleSessionsChangedEvent } =
2131+
createHandlersHarness({ state: { activeChatRunId: "run-reset" } });
2132+
handleChatEvent({
2133+
runId: "run-reset",
2134+
sessionKey: state.currentSessionKey,
2135+
state: "final",
2136+
message: { content: [{ type: "text", text: "done" }] },
2137+
});
2138+
chatLog.finalizeAssistant.mockClear();
2139+
loadHistory.mockClear();
2140+
2141+
handleSessionsChangedEvent({
2142+
sessionKey: state.currentSessionKey,
2143+
reason: "reset",
2144+
sessionId: state.currentSessionId ?? undefined,
2145+
} satisfies SessionChangedEvent);
2146+
await vi.waitFor(() => expect(loadHistory).toHaveBeenCalledTimes(1));
2147+
handleChatEvent({
2148+
runId: "run-reset",
2149+
sessionKey: state.currentSessionKey,
2150+
state: "final",
2151+
message: { content: [{ type: "text", text: "done" }] },
2152+
});
2153+
2154+
expect(chatLog.finalizeAssistant).not.toHaveBeenCalled();
2155+
});
2156+
2157+
it("preserves displayed-run dedupe when reset history fails", async () => {
2158+
const { state, chatLog, loadHistory, handleChatEvent, handleSessionsChangedEvent } =
2159+
createHandlersHarness({ state: { activeChatRunId: "run-reset" } });
2160+
const resolveHistory = deferNextHistoryLoad(loadHistory);
2161+
handleChatEvent({
2162+
runId: "run-reset",
2163+
sessionKey: state.currentSessionKey,
2164+
state: "final",
2165+
message: { content: [{ type: "text", text: "done" }] },
2166+
});
2167+
chatLog.finalizeAssistant.mockClear();
2168+
2169+
handleSessionsChangedEvent({
2170+
sessionKey: state.currentSessionKey,
2171+
reason: "reset",
2172+
sessionId: state.currentSessionId ?? undefined,
2173+
} satisfies SessionChangedEvent);
2174+
resolveHistory(false);
2175+
await Promise.resolve();
2176+
handleChatEvent({
2177+
runId: "run-reset",
2178+
sessionKey: state.currentSessionKey,
2179+
state: "final",
2180+
message: { content: [{ type: "text", text: "done" }] },
2181+
});
2182+
2183+
expect(chatLog.finalizeAssistant).not.toHaveBeenCalled();
2184+
});
2185+
2186+
it("gates late run events while reset history is rebuilding", async () => {
2187+
const { state, chatLog, loadHistory, handleChatEvent, handleSessionsChangedEvent } =
2188+
createHandlersHarness({ state: { activeChatRunId: "run-reset" } });
2189+
startRun(state, handleChatEvent, "run-reset");
2190+
chatLog.finalizeAssistant.mockClear();
2191+
loadHistory.mockClear();
2192+
2193+
handleSessionsChangedEvent({
2194+
sessionKey: state.currentSessionKey,
2195+
reason: "reset",
2196+
sessionId: state.currentSessionId ?? undefined,
2197+
} satisfies SessionChangedEvent);
2198+
handleChatEvent({
2199+
runId: "run-reset",
2200+
sessionKey: state.currentSessionKey,
2201+
state: "final",
2202+
message: { content: [{ type: "text", text: "stale after reset" }] },
2203+
});
2204+
2205+
await vi.waitFor(() => expect(loadHistory).toHaveBeenCalledTimes(1));
2206+
expect(chatLog.finalizeAssistant).not.toHaveBeenCalled();
2207+
expect(state.activeChatRunId).toBeNull();
2208+
});
2209+
});
18642210
});
18652211

18662212
describe("tui-event-handlers: streaming watchdog", () => {

0 commit comments

Comments
 (0)