Skip to content

Commit ff12fce

Browse files
committed
fix: completed messages skip live streaming-stats subscription
Codex P1 on PR #3219: every TypewriterMarkdown instance was subscribing to useWorkspaceStreamingStats(workspaceId) regardless of streaming state. Long transcripts of completed assistant messages then re-rendered on every stream-delta of the new live message, undoing part of the cascade-rerender fix. Subscribe to the real workspace key only while the message is actively streaming; completed messages subscribe to the empty-string sentinel which is never bumped. Hook still runs unconditionally per rules of hooks — only the key changes.
1 parent a476613 commit ff12fce

2 files changed

Lines changed: 54 additions & 3 deletions

File tree

src/browser/features/Messages/TypewriterMarkdown.test.tsx

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { UseSmoothStreamingTextOptions } from "@/browser/hooks/useSmoothStreamingText";
22
import { useSmoothStreamingText as importedUseSmoothStreamingText } from "@/browser/hooks/useSmoothStreamingText";
3+
import { useWorkspaceStreamingStats as importedUseWorkspaceStreamingStats } from "@/browser/stores/WorkspaceStore";
34
import { afterAll, afterEach, beforeEach, describe, expect, mock, test } from "bun:test";
45
import { cleanup, render } from "@testing-library/react";
56
import { GlobalWindow } from "happy-dom";
@@ -8,6 +9,7 @@ import { TypewriterMarkdown } from "./TypewriterMarkdown";
89

910
const actualMarkdownCore = ImportedMarkdownCore;
1011
const actualUseSmoothStreamingText = importedUseSmoothStreamingText;
12+
const actualUseWorkspaceStreamingStats = importedUseWorkspaceStreamingStats;
1113

1214
const mockUseSmoothStreamingText = mock(
1315
(options: UseSmoothStreamingTextOptions): { visibleText: string; isCaughtUp: boolean } => ({
@@ -16,6 +18,8 @@ const mockUseSmoothStreamingText = mock(
1618
})
1719
);
1820

21+
const mockUseWorkspaceStreamingStats = mock((_workspaceId: string) => null);
22+
1923
function MarkdownCoreStub(props: { content: string }) {
2024
return <div data-testid="markdown-core">{props.content}</div>;
2125
}
@@ -29,6 +33,9 @@ async function installTypewriterMarkdownModuleMocks() {
2933
await mock.module("@/browser/hooks/useSmoothStreamingText", () => ({
3034
useSmoothStreamingText: mockUseSmoothStreamingText,
3135
}));
36+
await mock.module("@/browser/stores/WorkspaceStore", () => ({
37+
useWorkspaceStreamingStats: mockUseWorkspaceStreamingStats,
38+
}));
3239
}
3340

3441
async function restoreTypewriterMarkdownModuleMocks() {
@@ -40,6 +47,9 @@ async function restoreTypewriterMarkdownModuleMocks() {
4047
await mock.module("@/browser/hooks/useSmoothStreamingText", () => ({
4148
useSmoothStreamingText: actualUseSmoothStreamingText,
4249
}));
50+
await mock.module("@/browser/stores/WorkspaceStore", () => ({
51+
useWorkspaceStreamingStats: actualUseWorkspaceStreamingStats,
52+
}));
4353
}
4454

4555
describe("TypewriterMarkdown", () => {
@@ -59,6 +69,7 @@ describe("TypewriterMarkdown", () => {
5969
globalThis.document = globalThis.window.document;
6070
await installTypewriterMarkdownModuleMocks();
6171
mockUseSmoothStreamingText.mockClear();
72+
mockUseWorkspaceStreamingStats.mockClear();
6273
});
6374

6475
afterEach(async () => {
@@ -108,4 +119,38 @@ describe("TypewriterMarkdown", () => {
108119
expect.objectContaining({ bypassSmoothing: true })
109120
);
110121
});
122+
123+
// Regression: completed historical messages must not subscribe to live
124+
// streaming stats for their workspace, otherwise every assistant message in a
125+
// long transcript re-renders on every stream-delta of an active stream and
126+
// re-introduces the cascade jitter this PR is supposed to eliminate.
127+
test("completed messages subscribe with empty key (no live-stats updates)", () => {
128+
render(
129+
<TypewriterMarkdown
130+
content="Historical reply"
131+
isComplete={true}
132+
streamKey="msg-old"
133+
streamSource="live"
134+
workspaceId="ws-active"
135+
/>
136+
);
137+
138+
// Hook still runs (rules of hooks), but the key must be the no-op sentinel.
139+
expect(mockUseWorkspaceStreamingStats).toHaveBeenCalledWith("");
140+
expect(mockUseWorkspaceStreamingStats).not.toHaveBeenCalledWith("ws-active");
141+
});
142+
143+
test("streaming messages subscribe with the real workspace key", () => {
144+
render(
145+
<TypewriterMarkdown
146+
content="Streaming reply"
147+
isComplete={false}
148+
streamKey="msg-live"
149+
streamSource="live"
150+
workspaceId="ws-active"
151+
/>
152+
);
153+
154+
expect(mockUseWorkspaceStreamingStats).toHaveBeenCalledWith("ws-active");
155+
});
111156
});

src/browser/features/Messages/TypewriterMarkdown.tsx

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,15 @@ export const TypewriterMarkdown: React.FC<TypewriterMarkdownProps> = ({
4646
// Read the live model emission rate (chars/sec) for the active stream of this
4747
// workspace. The hook subscribes to its own MapStore so per-delta updates
4848
// re-render this component WITHOUT cascading through the parent — see
49-
// useWorkspaceStreamingStats. Hooks must run unconditionally; pass an empty
50-
// string when no workspace is provided so the subscription is a stable no-op.
51-
const streamingStats = useWorkspaceStreamingStats(workspaceId ?? "");
49+
// useWorkspaceStreamingStats.
50+
//
51+
// Subscribe to the real workspace key ONLY while this message is actively
52+
// streaming. Completed historical messages subscribe to the stable empty-key
53+
// sentinel, which is never bumped — so a long transcript of finished
54+
// assistant messages does not re-render on every delta of a new stream.
55+
// (Hooks must run unconditionally; we toggle the key, not the call site.)
56+
const subscriptionKey = isStreaming && workspaceId ? workspaceId : "";
57+
const streamingStats = useWorkspaceStreamingStats(subscriptionKey);
5258
const liveCharsPerSec = isStreaming && workspaceId ? (streamingStats?.charsPerSec ?? 0) : 0;
5359

5460
// Two-clock streaming: ingestion (content) vs presentation (visibleText).

0 commit comments

Comments
 (0)