fix: prevent NaN thinking timers (#11556)
* fix: prevent NaN thinking timers * test: cover thinking timer fallback and cleanup
This commit is contained in:
@@ -102,10 +102,12 @@ const ThinkingBlock: React.FC<Props> = ({ block }) => {
|
||||
)
|
||||
}
|
||||
|
||||
const normalizeThinkingTime = (value?: number) => (typeof value === 'number' && Number.isFinite(value) ? value : 0)
|
||||
|
||||
const ThinkingTimeSeconds = memo(
|
||||
({ blockThinkingTime, isThinking }: { blockThinkingTime: number; isThinking: boolean }) => {
|
||||
const { t } = useTranslation()
|
||||
const [displayTime, setDisplayTime] = useState(blockThinkingTime)
|
||||
const [displayTime, setDisplayTime] = useState(normalizeThinkingTime(blockThinkingTime))
|
||||
|
||||
const timer = useRef<NodeJS.Timeout | null>(null)
|
||||
|
||||
@@ -121,7 +123,7 @@ const ThinkingTimeSeconds = memo(
|
||||
clearInterval(timer.current)
|
||||
timer.current = null
|
||||
}
|
||||
setDisplayTime(blockThinkingTime)
|
||||
setDisplayTime(normalizeThinkingTime(blockThinkingTime))
|
||||
}
|
||||
|
||||
return () => {
|
||||
@@ -132,10 +134,10 @@ const ThinkingTimeSeconds = memo(
|
||||
}
|
||||
}, [isThinking, blockThinkingTime])
|
||||
|
||||
const thinkingTimeSeconds = useMemo(
|
||||
() => ((displayTime < 1000 ? 100 : displayTime) / 1000).toFixed(1),
|
||||
[displayTime]
|
||||
)
|
||||
const thinkingTimeSeconds = useMemo(() => {
|
||||
const safeTime = normalizeThinkingTime(displayTime)
|
||||
return ((safeTime < 1000 ? 100 : safeTime) / 1000).toFixed(1)
|
||||
}, [displayTime])
|
||||
|
||||
return isThinking
|
||||
? t('chat.thinking', {
|
||||
|
||||
@@ -255,6 +255,20 @@ describe('ThinkingBlock', () => {
|
||||
unmount()
|
||||
})
|
||||
})
|
||||
|
||||
it('should clamp invalid thinking times to a safe default', () => {
|
||||
const testCases = [undefined, Number.NaN, Number.POSITIVE_INFINITY]
|
||||
|
||||
testCases.forEach((thinking_millsec) => {
|
||||
const block = createThinkingBlock({
|
||||
thinking_millsec: thinking_millsec as any,
|
||||
status: MessageBlockStatus.SUCCESS
|
||||
})
|
||||
const { unmount } = renderThinkingBlock(block)
|
||||
expect(getThinkingTimeText()).toHaveTextContent('0.1s')
|
||||
unmount()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('collapse behavior', () => {
|
||||
|
||||
@@ -254,6 +254,17 @@ const HomeWindow: FC<{ draggable?: boolean }> = ({ draggable = true }) => {
|
||||
|
||||
let blockId: string | null = null
|
||||
let thinkingBlockId: string | null = null
|
||||
let thinkingStartTime: number | null = null
|
||||
|
||||
const resolveThinkingDuration = (duration?: number) => {
|
||||
if (typeof duration === 'number' && Number.isFinite(duration)) {
|
||||
return duration
|
||||
}
|
||||
if (thinkingStartTime !== null) {
|
||||
return Math.max(0, performance.now() - thinkingStartTime)
|
||||
}
|
||||
return 0
|
||||
}
|
||||
|
||||
setIsLoading(true)
|
||||
setIsOutputted(false)
|
||||
@@ -291,6 +302,7 @@ const HomeWindow: FC<{ draggable?: boolean }> = ({ draggable = true }) => {
|
||||
case ChunkType.THINKING_START:
|
||||
{
|
||||
setIsOutputted(true)
|
||||
thinkingStartTime = performance.now()
|
||||
if (thinkingBlockId) {
|
||||
store.dispatch(
|
||||
updateOneBlock({ id: thinkingBlockId, changes: { status: MessageBlockStatus.STREAMING } })
|
||||
@@ -315,9 +327,13 @@ const HomeWindow: FC<{ draggable?: boolean }> = ({ draggable = true }) => {
|
||||
{
|
||||
setIsOutputted(true)
|
||||
if (thinkingBlockId) {
|
||||
if (thinkingStartTime === null) {
|
||||
thinkingStartTime = performance.now()
|
||||
}
|
||||
const thinkingDuration = resolveThinkingDuration(chunk.thinking_millsec)
|
||||
throttledBlockUpdate(thinkingBlockId, {
|
||||
content: chunk.text,
|
||||
thinking_millsec: chunk.thinking_millsec
|
||||
thinking_millsec: thinkingDuration
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -325,14 +341,17 @@ const HomeWindow: FC<{ draggable?: boolean }> = ({ draggable = true }) => {
|
||||
case ChunkType.THINKING_COMPLETE:
|
||||
{
|
||||
if (thinkingBlockId) {
|
||||
const thinkingDuration = resolveThinkingDuration(chunk.thinking_millsec)
|
||||
cancelThrottledBlockUpdate(thinkingBlockId)
|
||||
store.dispatch(
|
||||
updateOneBlock({
|
||||
id: thinkingBlockId,
|
||||
changes: { status: MessageBlockStatus.SUCCESS, thinking_millsec: chunk.thinking_millsec }
|
||||
changes: { status: MessageBlockStatus.SUCCESS, thinking_millsec: thinkingDuration }
|
||||
})
|
||||
)
|
||||
}
|
||||
thinkingStartTime = null
|
||||
thinkingBlockId = null
|
||||
}
|
||||
break
|
||||
case ChunkType.TEXT_START:
|
||||
@@ -404,6 +423,8 @@ const HomeWindow: FC<{ draggable?: boolean }> = ({ draggable = true }) => {
|
||||
if (!isAborted) {
|
||||
throw new Error(chunk.error.message)
|
||||
}
|
||||
thinkingStartTime = null
|
||||
thinkingBlockId = null
|
||||
}
|
||||
//fall through
|
||||
case ChunkType.BLOCK_COMPLETE:
|
||||
|
||||
@@ -41,8 +41,19 @@ export const processMessages = async (
|
||||
|
||||
let textBlockId: string | null = null
|
||||
let thinkingBlockId: string | null = null
|
||||
let thinkingStartTime: number | null = null
|
||||
let textBlockContent: string = ''
|
||||
|
||||
const resolveThinkingDuration = (duration?: number) => {
|
||||
if (typeof duration === 'number' && Number.isFinite(duration)) {
|
||||
return duration
|
||||
}
|
||||
if (thinkingStartTime !== null) {
|
||||
return Math.max(0, performance.now() - thinkingStartTime)
|
||||
}
|
||||
return 0
|
||||
}
|
||||
|
||||
const assistantMessage = getAssistantMessage({
|
||||
assistant,
|
||||
topic
|
||||
@@ -79,6 +90,7 @@ export const processMessages = async (
|
||||
switch (chunk.type) {
|
||||
case ChunkType.THINKING_START:
|
||||
{
|
||||
thinkingStartTime = performance.now()
|
||||
if (thinkingBlockId) {
|
||||
store.dispatch(
|
||||
updateOneBlock({ id: thinkingBlockId, changes: { status: MessageBlockStatus.STREAMING } })
|
||||
@@ -102,9 +114,13 @@ export const processMessages = async (
|
||||
case ChunkType.THINKING_DELTA:
|
||||
{
|
||||
if (thinkingBlockId) {
|
||||
if (thinkingStartTime === null) {
|
||||
thinkingStartTime = performance.now()
|
||||
}
|
||||
const thinkingDuration = resolveThinkingDuration(chunk.thinking_millsec)
|
||||
throttledBlockUpdate(thinkingBlockId, {
|
||||
content: chunk.text,
|
||||
thinking_millsec: chunk.thinking_millsec
|
||||
thinking_millsec: thinkingDuration
|
||||
})
|
||||
}
|
||||
onStream()
|
||||
@@ -113,6 +129,7 @@ export const processMessages = async (
|
||||
case ChunkType.THINKING_COMPLETE:
|
||||
{
|
||||
if (thinkingBlockId) {
|
||||
const thinkingDuration = resolveThinkingDuration(chunk.thinking_millsec)
|
||||
cancelThrottledBlockUpdate(thinkingBlockId)
|
||||
store.dispatch(
|
||||
updateOneBlock({
|
||||
@@ -120,12 +137,13 @@ export const processMessages = async (
|
||||
changes: {
|
||||
content: chunk.text,
|
||||
status: MessageBlockStatus.SUCCESS,
|
||||
thinking_millsec: chunk.thinking_millsec
|
||||
thinking_millsec: thinkingDuration
|
||||
}
|
||||
})
|
||||
)
|
||||
thinkingBlockId = null
|
||||
}
|
||||
thinkingStartTime = null
|
||||
}
|
||||
break
|
||||
case ChunkType.TEXT_START:
|
||||
@@ -190,6 +208,7 @@ export const processMessages = async (
|
||||
case ChunkType.ERROR:
|
||||
{
|
||||
const blockId = textBlockId || thinkingBlockId
|
||||
thinkingStartTime = null
|
||||
if (blockId) {
|
||||
store.dispatch(
|
||||
updateOneBlock({
|
||||
|
||||
@@ -284,6 +284,54 @@ describe('processMessages', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('thinking timer fallback', () => {
|
||||
it('should use local timer when thinking_millsec is missing', async () => {
|
||||
const nowValues = [1000, 1500, 2000]
|
||||
let nowIndex = 0
|
||||
const performanceSpy = vi.spyOn(performance, 'now').mockImplementation(() => {
|
||||
const value = nowValues[Math.min(nowIndex, nowValues.length - 1)]
|
||||
nowIndex += 1
|
||||
return value
|
||||
})
|
||||
|
||||
const mockChunks = [
|
||||
{ type: ChunkType.THINKING_START },
|
||||
{ type: ChunkType.THINKING_DELTA, text: 'Thinking...' },
|
||||
{ type: ChunkType.THINKING_COMPLETE, text: 'Done thinking' },
|
||||
{ type: ChunkType.TEXT_START },
|
||||
{ type: ChunkType.TEXT_COMPLETE, text: 'Final answer' },
|
||||
{ type: ChunkType.BLOCK_COMPLETE }
|
||||
]
|
||||
|
||||
vi.mocked(fetchChatCompletion).mockImplementation(async ({ onChunkReceived }: any) => {
|
||||
for (const chunk of mockChunks) {
|
||||
await onChunkReceived(chunk)
|
||||
}
|
||||
})
|
||||
|
||||
await processMessages(
|
||||
mockAssistant,
|
||||
mockTopic,
|
||||
'test prompt',
|
||||
mockSetAskId,
|
||||
mockOnStream,
|
||||
mockOnFinish,
|
||||
mockOnError
|
||||
)
|
||||
|
||||
const thinkingDeltaCall = vi.mocked(throttledBlockUpdate).mock.calls.find(([id]) => id === 'thinking-block-1')
|
||||
const deltaPayload = thinkingDeltaCall?.[1] as { thinking_millsec?: number } | undefined
|
||||
expect(deltaPayload?.thinking_millsec).toBe(500)
|
||||
|
||||
const thinkingCompleteUpdate = vi
|
||||
.mocked(updateOneBlock)
|
||||
.mock.calls.find(([payload]) => (payload as any)?.changes?.thinking_millsec !== undefined)
|
||||
expect((thinkingCompleteUpdate?.[0] as any)?.changes?.thinking_millsec).toBe(1000)
|
||||
|
||||
performanceSpy.mockRestore()
|
||||
})
|
||||
})
|
||||
|
||||
describe('stream with exceptions', () => {
|
||||
it('should handle error chunks properly', async () => {
|
||||
const mockError = new Error('Stream processing error')
|
||||
|
||||
Reference in New Issue
Block a user