Skip to content

fix: optimize ReasoningBlock performance by isolating timer re-renders #8000

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

fix: optimize ReasoningBlock performance by isolating timer re-renders #8000

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d03bf9a
fix: optimize ReasoningBlock performance by isolating timer re-renders
roomote Sep 15, 2025
635eb98
feat: add content debouncing to ReasoningBlock
roomote Sep 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 42 additions & 19 deletions webview-ui/src/components/chat/ReasoningBlock.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useEffect, useRef, useState } from "react"
import React, { memo, useEffect, useRef, useState } from "react"
import { useTranslation } from "react-i18next"

import MarkdownBlock from "../common/MarkdownBlock"
Expand All @@ -12,45 +12,68 @@ interface ReasoningBlockProps {
metadata?: any
Copy link
Contributor Author

@roomote roomote bot Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor type safety improvement: The prop is typed as . Consider defining a proper interface for better type safety:

}

interface ElapsedTimeProps {
isActive: boolean
startTime: number
}

/**
* Render reasoning with a heading and a simple timer.
* - Heading uses i18n key chat:reasoning.thinking
* - Timer runs while reasoning is active (no persistence)
* Memoized timer component that only re-renders itself
* This prevents the entire ReasoningBlock from re-rendering every second
*/
export const ReasoningBlock = ({ content, isStreaming, isLast }: ReasoningBlockProps) => {
const ElapsedTime = memo(({ isActive, startTime }: ElapsedTimeProps) => {
const { t } = useTranslation()

const startTimeRef = useRef<number>(Date.now())
const [elapsed, setElapsed] = useState<number>(0)

// Simple timer that runs while streaming
useEffect(() => {
if (isLast && isStreaming) {
const tick = () => setElapsed(Date.now() - startTimeRef.current)
tick()
if (isActive) {
const tick = () => setElapsed(Date.now() - startTime)
tick() // Initial tick
const id = setInterval(tick, 1000)
return () => clearInterval(id)
} else {
setElapsed(0)
Copy link
Contributor Author

@roomote roomote bot Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is resetting elapsed to 0 here intentional? This might cause a brief flash if the component transitions from active to inactive and back quickly. Consider keeping the last elapsed value when inactive, or only resetting when the component unmounts.

}
}, [isLast, isStreaming])
}, [isActive, startTime])

if (elapsed === 0) return null

const seconds = Math.floor(elapsed / 1000)
const secondsLabel = t("chat:reasoning.seconds", { count: seconds })

return (
<span className="text-vscode-foreground tabular-nums flex items-center gap-1">
<Clock className="w-4" />
{secondsLabel}
</span>
)
})

ElapsedTime.displayName = "ElapsedTime"
Copy link
Contributor Author

@roomote roomote bot Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job isolating the timer re-renders! However, this component could benefit from test coverage. Given this is a performance-critical fix, consider adding tests to verify that:

  • ElapsedTime only re-renders when its props change
  • The parent ReasoningBlock doesn't re-render on timer updates
  • Timer cleanup works correctly


/**
* Render reasoning with a heading and a simple timer.
* - Heading uses i18n key chat:reasoning.thinking
* - Timer runs while reasoning is active (no persistence)
* - Timer is isolated in a memoized component to prevent full re-renders
*/
export const ReasoningBlock = ({ content, isStreaming, isLast }: ReasoningBlockProps) => {
Copy link
Contributor Author

@roomote roomote bot Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider also wrapping the ReasoningBlock component itself with React.memo. While you've isolated the timer, the parent could still benefit from memoization to prevent unnecessary re-renders when none of its props have changed:

const { t } = useTranslation()
const startTimeRef = useRef<number>(Date.now())

// Only render markdown when there's actual content
const hasContent = (content?.trim()?.length ?? 0) > 0

return (
<div className="py-1">
<div className="flex items-center justify-between mb-2.5 pr-2">
<div className="flex items-center gap-2">
<Lightbulb className="w-4" />
<span className="font-bold text-vscode-foreground">{t("chat:reasoning.thinking")}</span>
</div>
{elapsed > 0 && (
<span className="text-vscode-foreground tabular-nums flex items-center gap-1">
<Clock className="w-4" />
{secondsLabel}
</span>
)}
<ElapsedTime isActive={isLast && isStreaming} startTime={startTimeRef.current} />
</div>
{(content?.trim()?.length ?? 0) > 0 && (
{hasContent && (
<div className="px-3 italic text-vscode-descriptionForeground">
<MarkdownBlock markdown={content} />
Copy link
Contributor Author

@roomote roomote bot Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following up on nabilfreeman's comment about fast output models - have you considered adding debouncing for the markdown content updates? Fast streaming could still trigger many re-renders of the MarkdownBlock even with your timer isolation fix.

</div>
Expand Down
Loading