fix(ui): reset thinking time display when new session starts#13450
fix(ui): reset thinking time display when new session starts#13450GeorgeDong32 wants to merge 2 commits intomainfrom
Conversation
There was a problem hiding this comment.
Reviewed the fix and the added regression test. The reset condition is scoped narrowly to genuinely new thinking sessions (isThinking with blockThinkingTime === 0), so it addresses the stale display-time carryover without changing the resumed-session path. I don't see new blocking issues in the current revision. LGTM.
|
But how could this happen? Did you reproduce it? If a new thinking session is started, it should create a new ThinkingBlock. |
It's not how react's function component works. Component is a function, and concrete component has its independent state. |
|
Note This issue/comment/review was translated by Claude. Considering that the component's responsibility is only to render the actual data in the block, I think the issue might be caused by not correctly resetting Original Content考虑到组件的职责仅是渲染 block 中实际的数据,我认为可能是在处理 thinking block 的上游的某处没有正确地重置 thinkingStartTIme 导致了此问题。 |
Note This issue/comment/review was translated by Claude. OK, I'll try to reproduce this issue later Original ContentOK,我晚点试试复现这个问题 |
What this PR does
Before this PR:
When sending a prompt with a thinking model, the displayed thinking time counter doesn't start from 0s on subsequent prompts. Instead, it accumulates from the previous thinking session's value because the
displayTimestate inThinkingTimeSecondscomponent is not reset when a new thinking session starts.After this PR:
Added logic to reset
displayTimeto 0 whenisThinkingbecomestrueandblockThinkingTime === 0(indicating a new thinking session). This ensures the thinking time counter starts fresh from 0s for each new prompt.Fixes #13449
Why we need it and why it was done in this way
Root cause: The
ThinkingTimeSecondscomponent usesuseStateto trackdisplayTime, which is only initialized once when the component mounts. When React reuses the component instance for a new thinking block, the previousdisplayTimevalue persists, causing the timer to accumulate from the old value.The fix: Added a conditional reset in the
useEffectthat handles theisThinkingstate change:Tradeoffs made:
blockThinkingTime === 0to preserve behavior for resumed/in-progress sessionsAlternatives considered:
useRefto track timer start time: rejected as it would require more extensive refactoringBreaking changes
None. This is a bug fix that only affects the display behavior of the thinking time counter.
Special notes for your reviewer
isThinkingtransitions fromfalsetotrue)blockThinkingTime === 0ensures we only reset for genuinely new sessions, not resumed onesrerender-dependenciesandrerender-functional-setstateguidelinesChecklist
/gh-pr-review,gh pr diff, or GitHub UI) before requesting review from othersRelease note