From b84aa591aaa064c278c62582f30d9ba928e71371 Mon Sep 17 00:00:00 2001 From: umi008 Date: Fri, 10 Jul 2026 20:23:54 -0600 Subject: [PATCH 1/4] fix(webview): anchor thinking timer to message timestamp to survive remounts The ReasoningBlock previously anchored its elapsed-time display to Date.now() at component mount time. When Virtuoso's virtual list recycles or remounts the component (e.g. during expand/collapse toggles or scroll), the timer reset to 0 because a fresh Date.now() was used and the effect guard (isLast && isStreaming) was false on remount. Now the component uses the message creation timestamp (ts, already passed as a prop but previously discarded) as the timer anchor. On remount after streaming has finished, elapsed is initialized from Date.now() - ts, giving a reasonable approximation of the thinking duration instead of showing 0s. This also explains the zh_CN-specific reproduction: CJK character widths in the header created slightly different DOM geometry, pushing the layout past Virtuoso's recycle threshold in certain viewports. Fixes #656 --- webview-ui/src/components/chat/ReasoningBlock.tsx | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/webview-ui/src/components/chat/ReasoningBlock.tsx b/webview-ui/src/components/chat/ReasoningBlock.tsx index 11166f5ae1..271655475b 100644 --- a/webview-ui/src/components/chat/ReasoningBlock.tsx +++ b/webview-ui/src/components/chat/ReasoningBlock.tsx @@ -14,14 +14,19 @@ interface ReasoningBlockProps { metadata?: any } -export const ReasoningBlock = ({ content, isStreaming, isLast }: ReasoningBlockProps) => { +export const ReasoningBlock = ({ content, isStreaming, isLast, ts }: ReasoningBlockProps) => { const { t } = useTranslation() const { reasoningBlockCollapsed } = useExtensionState() const [isCollapsed, setIsCollapsed] = useState(reasoningBlockCollapsed) - const startTimeRef = useRef(Date.now()) - const [elapsed, setElapsed] = useState(0) + // Anchor the elapsed timer to the message creation timestamp (ts) + // rather than component mount time. When Virtuoso recycles or + // remounts this component (e.g. during expand/collapse in a + // virtualized list), the timer survives because ts is a stable + // prop from the message data rather than a fresh Date.now(). + const startTimeRef = useRef(ts) + const [elapsed, setElapsed] = useState(() => (isStreaming ? 0 : Math.max(0, Date.now() - ts))) const contentRef = useRef(null) useEffect(() => { From 7898c54053c594a0f67a0a7127a6aa8aa5477ebd Mon Sep 17 00:00:00 2001 From: umi008 Date: Sun, 12 Jul 2026 21:47:43 -0600 Subject: [PATCH 2/4] test(webview): add ReasoningBlock timer remount coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Covers the ts-anchored elapsed timer behavior including: - Non-streaming elapsed anchored to message timestamp - Streaming timer that ticks with real time - Remount survival (Virtuoso recycle) — elapsed anchored to ts, not mount - Timer stops when streaming ends - Collapse behavior - Zero-elapsed label hiding --- .../chat/__tests__/ReasoningBlock.spec.tsx | 151 ++++++++++++++++++ 1 file changed, 151 insertions(+) create mode 100644 webview-ui/src/components/chat/__tests__/ReasoningBlock.spec.tsx diff --git a/webview-ui/src/components/chat/__tests__/ReasoningBlock.spec.tsx b/webview-ui/src/components/chat/__tests__/ReasoningBlock.spec.tsx new file mode 100644 index 0000000000..e1f76d5c6b --- /dev/null +++ b/webview-ui/src/components/chat/__tests__/ReasoningBlock.spec.tsx @@ -0,0 +1,151 @@ +// npx vitest src/components/chat/__tests__/ReasoningBlock.spec.tsx + +import React from "react" +import { render, screen, act } from "@/utils/test-utils" + +import { ReasoningBlock } from "../ReasoningBlock" + +// Mock i18n +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ + t: (key: string, options?: any) => { + if (key === "chat:reasoning.seconds") { + return `${options?.count ?? 0}s` + } + return key + }, + }), + initReactI18next: { + type: "3rdParty", + init: vi.fn(), + }, +})) + +// Mock ExtensionStateContext +const mockExtensionState = { + reasoningBlockCollapsed: false, +} + +vi.mock("@src/context/ExtensionStateContext", () => ({ + useExtensionState: () => mockExtensionState, +})) + +const defaultProps = { + content: "Let me think about this...", + ts: Date.now() - 30_000, // 30 seconds ago + isStreaming: false, + isLast: false, +} as const + +describe("ReasoningBlock", () => { + beforeEach(() => { + vi.useFakeTimers() + vi.setSystemTime(Date.now()) + }) + + afterEach(() => { + vi.useRealTimers() + }) + + it("renders the thinking label", () => { + render() + expect(screen.getByText("chat:reasoning.thinking")).toBeInTheDocument() + }) + + it("shows elapsed time for non-streaming blocks anchored to ts", () => { + const thirtySecAgo = Date.now() - 30_000 + render() + + // Should show ~30s since the timestamp + expect(screen.getByText("30s")).toBeInTheDocument() + }) + + it("starts timer at 0 when streaming and is last message", () => { + const ts = Date.now() + render() + + // Initially 0 while streaming (just started) — no label shown when elapsed is 0 + expect(screen.queryByText(/^\d+s$/)).not.toBeInTheDocument() + + // Advance time by 5 seconds + act(() => { + vi.advanceTimersByTime(5000) + }) + + expect(screen.getByText("5s")).toBeInTheDocument() + }) + + it("preserves elapsed time on remount with same ts (Virtuoso recycle)", () => { + // ts is 1 second in the past so elapsed starts at 1s (not 0, since 0 hides the label) + const ts = Date.now() - 1000 + + const { unmount } = render() + + // Initially 1s since ts is 1s ago + expect(screen.getByText("1s")).toBeInTheDocument() + + // Advance time by 10 seconds + act(() => { + vi.advanceTimersByTime(10_000) + }) + + // Unmount (simulating Virtuoso recycling the component) + unmount() + + // Remount with the same ts + render() + + // Should still show ~11s, not 1 — timer survived the remount + // (elapsed is computed from Date.now() - ts on each mount, and fake timers keep advancing) + expect(screen.getByText("11s")).toBeInTheDocument() + }) + + it("stops timer when streaming ends", () => { + const ts = Date.now() - 5000 + + const { rerender } = render( + , + ) + + // Streaming — timer is active + expect(screen.getByText("5s")).toBeInTheDocument() + + // Advance time while streaming + act(() => { + vi.advanceTimersByTime(3000) + }) + + expect(screen.getByText("8s")).toBeInTheDocument() + + // Streaming stops — rerender with isStreaming=false + rerender() + + // Timer should stop at current value + act(() => { + vi.advanceTimersByTime(5000) + }) + + // Should NOT have advanced + expect(screen.getByText("8s")).toBeInTheDocument() + }) + + it("collapses when reasoningBlockCollapsed is true", () => { + mockExtensionState.reasoningBlockCollapsed = true + + render() + + // Content and markdown should be hidden + expect(screen.queryByText(defaultProps.content)).not.toBeInTheDocument() + + // Reset for other tests + mockExtensionState.reasoningBlockCollapsed = false + }) + + it("hides elapsed label when elapsed is 0", () => { + const ts = Date.now() + render() + + // No seconds label because elapsed is 0 + expect(screen.queryByText(/^\d+s$/)).not.toBeInTheDocument() + }) +}) From f957b59b075ed38c1e3a898aed09aea13a6a3105 Mon Sep 17 00:00:00 2001 From: umi008 Date: Fri, 17 Jul 2026 03:46:18 +0000 Subject: [PATCH 3/4] fix(webview): persist reasoning timer across Virtuoso remounts via module cache Adds a module-level Map that caches the final elapsed reasoning duration keyed by message timestamp (ts). This solves the edge case where a remount minutes after streaming finished would recompute Date.now() - ts and show an inflated elapsed time. - elapsedCache survives React unmount/remount cycles since module state persists for the page lifetime - Cache is populated on each timer tick while streaming - Cache is also populated when streaming changes to false - On component init, cache is checked first before any computation - Map size is bounded by the number of reasoning blocks per session --- .../src/components/chat/ReasoningBlock.tsx | 34 ++++++++++++++++- .../chat/__tests__/ReasoningBlock.spec.tsx | 37 ++++++++++--------- 2 files changed, 51 insertions(+), 20 deletions(-) diff --git a/webview-ui/src/components/chat/ReasoningBlock.tsx b/webview-ui/src/components/chat/ReasoningBlock.tsx index 271655475b..30835b86a8 100644 --- a/webview-ui/src/components/chat/ReasoningBlock.tsx +++ b/webview-ui/src/components/chat/ReasoningBlock.tsx @@ -14,6 +14,16 @@ interface ReasoningBlockProps { metadata?: any } +/** + * Module-level cache that persists elapsed reasoning durations across + * React component remounts. When Virtuoso recycles a ReasoningBlock + * (e.g., during expand/collapse or scroll), the cache provides the + * final elapsed value from the previous mount cycle instead of + * recomputing from Date.now() - ts, which would be wrong if minutes + * have passed since the message was created. + */ +const elapsedCache = new Map() + export const ReasoningBlock = ({ content, isStreaming, isLast, ts }: ReasoningBlockProps) => { const { t } = useTranslation() const { reasoningBlockCollapsed } = useExtensionState() @@ -26,7 +36,14 @@ export const ReasoningBlock = ({ content, isStreaming, isLast, ts }: ReasoningBl // virtualized list), the timer survives because ts is a stable // prop from the message data rather than a fresh Date.now(). const startTimeRef = useRef(ts) - const [elapsed, setElapsed] = useState(() => (isStreaming ? 0 : Math.max(0, Date.now() - ts))) + // On init, prefer cached elapsed (survives remounts). If not cached, + // use Date.now() - ts for a reasonable initial estimate. This handles + // the first mount where no cached value exists yet. + const [elapsed, setElapsed] = useState(() => { + const cached = elapsedCache.get(ts) + if (cached !== undefined) return cached + return isStreaming ? 0 : Math.max(0, Date.now() - ts) + }) const contentRef = useRef(null) useEffect(() => { @@ -35,13 +52,26 @@ export const ReasoningBlock = ({ content, isStreaming, isLast, ts }: ReasoningBl useEffect(() => { if (isLast && isStreaming) { - const tick = () => setElapsed(Date.now() - startTimeRef.current) + const tick = () => { + const current = Date.now() - startTimeRef.current + setElapsed(current) + elapsedCache.set(ts, current) + } tick() const id = setInterval(tick, 1000) return () => clearInterval(id) } }, [isLast, isStreaming]) + // Cache the final elapsed value when streaming stops so it survives + // future remounts even if the component unmounts before the timer + // effect cleanup runs. + useEffect(() => { + if (!isStreaming && elapsed > 0) { + elapsedCache.set(ts, elapsed) + } + }, [isStreaming, ts, elapsed]) + const seconds = Math.floor(elapsed / 1000) const secondsLabel = t("chat:reasoning.seconds", { count: seconds }) diff --git a/webview-ui/src/components/chat/__tests__/ReasoningBlock.spec.tsx b/webview-ui/src/components/chat/__tests__/ReasoningBlock.spec.tsx index e1f76d5c6b..4e119f8127 100644 --- a/webview-ui/src/components/chat/__tests__/ReasoningBlock.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/ReasoningBlock.spec.tsx @@ -75,30 +75,31 @@ describe("ReasoningBlock", () => { expect(screen.getByText("5s")).toBeInTheDocument() }) - it("preserves elapsed time on remount with same ts (Virtuoso recycle)", () => { - // ts is 1 second in the past so elapsed starts at 1s (not 0, since 0 hides the label) - const ts = Date.now() - 1000 + it("preserves elapsed time on remount with same ts (Virtuoso recycle)", () => { + // ts is 1 second in the past so elapsed starts at 1s (not 0, since 0 hides the label) + const ts = Date.now() - 1000 - const { unmount } = render() + const { unmount } = render() - // Initially 1s since ts is 1s ago - expect(screen.getByText("1s")).toBeInTheDocument() + // Initially 1s since ts is 1s ago + expect(screen.getByText("1s")).toBeInTheDocument() - // Advance time by 10 seconds - act(() => { - vi.advanceTimersByTime(10_000) - }) + // Advance time by 10 seconds + act(() => { + vi.advanceTimersByTime(10_000) + }) - // Unmount (simulating Virtuoso recycling the component) - unmount() + // Unmount (simulating Virtuoso recycling the component) + unmount() - // Remount with the same ts - render() + // Remount with the same ts + render() - // Should still show ~11s, not 1 — timer survived the remount - // (elapsed is computed from Date.now() - ts on each mount, and fake timers keep advancing) - expect(screen.getByText("11s")).toBeInTheDocument() - }) + // Should still show 1s — the elapsed value was cached at module level + // and survived the remount, instead of recomputing from Date.now() - ts + // (which would give 11s and inflate the timer after a scroll away) + expect(screen.getByText("1s")).toBeInTheDocument() + }) it("stops timer when streaming ends", () => { const ts = Date.now() - 5000 From 5c1d0b1f6795c5ac671917cf7977d58f72f9a900 Mon Sep 17 00:00:00 2001 From: umi008 Date: Fri, 17 Jul 2026 04:06:41 +0000 Subject: [PATCH 4/4] fix(webview): add ts to useEffect dependency array to fix lint warning the timer effect closes over ts (for elapsedCache.set) but was missing it from the dependency array, triggering react-hooks/exhaustive-deps --- webview-ui/src/components/chat/ReasoningBlock.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webview-ui/src/components/chat/ReasoningBlock.tsx b/webview-ui/src/components/chat/ReasoningBlock.tsx index 30835b86a8..cfc52e00a6 100644 --- a/webview-ui/src/components/chat/ReasoningBlock.tsx +++ b/webview-ui/src/components/chat/ReasoningBlock.tsx @@ -61,7 +61,7 @@ export const ReasoningBlock = ({ content, isStreaming, isLast, ts }: ReasoningBl const id = setInterval(tick, 1000) return () => clearInterval(id) } - }, [isLast, isStreaming]) + }, [isLast, isStreaming, ts]) // Cache the final elapsed value when streaming stops so it survives // future remounts even if the component unmounts before the timer