Skip to content

Commit 852fb03

Browse files
committed
web: add regression test for brief turn changes fetch
1 parent 3e434d5 commit 852fb03

3 files changed

Lines changed: 110 additions & 17 deletions

File tree

web/src/components/AssistantChat/BriefTurnList.tsx

Lines changed: 52 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@ import { reconcileChatBlocks } from '@/chat/reconcile'
1010
import { BriefCardMarkdownPreview } from '@/components/AssistantChat/BriefCardMarkdownPreview'
1111
import { BriefFullMarkdownContent } from '@/components/AssistantChat/BriefFullMarkdownContent'
1212
import { HappyThread } from '@/components/AssistantChat/HappyThread'
13-
import { isBriefTurnLive, shouldShowLatestBriefTurnAsFullContent } from '@/components/AssistantChat/briefTurnPresentation'
13+
import {
14+
isBriefTurnLive,
15+
shouldFetchLatestTurnChangesSummary,
16+
shouldShowLatestBriefTurnAsFullContent
17+
} from '@/components/AssistantChat/briefTurnPresentation'
1418
import { Spinner } from '@/components/Spinner'
1519
import { Dialog, DialogContent, DialogHeader, DialogTitle } from '@/components/ui/dialog'
1620
import type { SessionListDensity } from '@/hooks/useSessionListDensity'
@@ -367,7 +371,8 @@ function extractTurnChangesSummary(messages: DecryptedMessage[]): TurnChangesSum
367371
if (content.type !== 'tool-call') {
368372
continue
369373
}
370-
if (content.name.trim().toLowerCase() !== 'codexturnchanges') {
374+
const normalizedToolName = content.name.trim().toLowerCase().replace(/[_-]/g, '')
375+
if (!normalizedToolName.endsWith('codexturnchanges')) {
371376
continue
372377
}
373378

@@ -408,6 +413,35 @@ function extractTurnChangesSummary(messages: DecryptedMessage[]): TurnChangesSum
408413
}
409414
}
410415

416+
async function fetchTurnChangesSummaryByPaging(
417+
api: ApiClient,
418+
sessionId: string,
419+
turnId: string
420+
): Promise<TurnChangesSummary | null> {
421+
let beforeSeq: number | null = null
422+
let pageCount = 0
423+
424+
while (pageCount < 8) {
425+
const response = await api.getConversationTurnMessages(sessionId, turnId, {
426+
limit: TURN_CHANGES_SUMMARY_FETCH_LIMIT,
427+
beforeSeq
428+
})
429+
const summary = extractTurnChangesSummary(response.messages)
430+
if (summary) {
431+
return summary
432+
}
433+
434+
if (!response.page.hasMore || response.page.nextBeforeSeq === null) {
435+
return null
436+
}
437+
438+
beforeSeq = response.page.nextBeforeSeq
439+
pageCount += 1
440+
}
441+
442+
return null
443+
}
444+
411445
function dedupeAndSortMessages(messages: DecryptedMessage[]): DecryptedMessage[] {
412446
const dedupedById = new Map<string, DecryptedMessage>()
413447
for (const message of messages) {
@@ -633,6 +667,7 @@ export function BriefTurnList(props: {
633667
const [liveActivityByTurnId, setLiveActivityByTurnId] = useState<Record<string, string>>({})
634668
const [turnChangesSummaryByTurnId, setTurnChangesSummaryByTurnId] = useState<Record<string, TurnChangesSummary | null>>({})
635669
const turnChangesSummaryInFlightRef = useRef(new Set<string>())
670+
const turnChangesSummaryFetchedUpdatedAtRef = useRef<Record<string, number>>({})
636671
const [isMobileViewport, setIsMobileViewport] = useState(() => (
637672
typeof window !== 'undefined' && window.matchMedia(MOBILE_BRIEF_BREAKPOINT_QUERY).matches
638673
))
@@ -885,29 +920,27 @@ export function BriefTurnList(props: {
885920

886921
useEffect(() => {
887922
const latestTurn = props.turns[props.turns.length - 1]
888-
if (!latestTurn || latestTurn.status !== 'closed') {
923+
const latestTurnId = latestTurn?.id ?? null
924+
const latestTurnUpdatedAt = latestTurn?.updatedAt ?? null
925+
926+
if (!shouldFetchLatestTurnChangesSummary({
927+
latestTurnId,
928+
latestTurnUpdatedAt,
929+
fetchedUpdatedAtByTurnId: turnChangesSummaryFetchedUpdatedAtRef.current,
930+
inFlightTurnIds: turnChangesSummaryInFlightRef.current
931+
})) {
889932
return
890933
}
891934

892-
const turnId = latestTurn.id
893-
if (Object.prototype.hasOwnProperty.call(turnChangesSummaryByTurnId, turnId)) {
894-
return
895-
}
896-
if (turnChangesSummaryInFlightRef.current.has(turnId)) {
897-
return
898-
}
935+
const turnId = latestTurnId as string
899936

900937
let cancelled = false
901938
turnChangesSummaryInFlightRef.current.add(turnId)
902939

903-
void props.api.getConversationTurnMessages(props.session.id, turnId, {
904-
limit: TURN_CHANGES_SUMMARY_FETCH_LIMIT,
905-
beforeSeq: null
906-
}).then((response) => {
940+
void fetchTurnChangesSummaryByPaging(props.api, props.session.id, turnId).then((summary) => {
907941
if (cancelled) {
908942
return
909943
}
910-
const summary = extractTurnChangesSummary(response.messages)
911944
setTurnChangesSummaryByTurnId((previous) => ({
912945
...previous,
913946
[turnId]: summary
@@ -921,13 +954,16 @@ export function BriefTurnList(props: {
921954
[turnId]: null
922955
}))
923956
}).finally(() => {
957+
if (latestTurnUpdatedAt !== null) {
958+
turnChangesSummaryFetchedUpdatedAtRef.current[turnId] = latestTurnUpdatedAt
959+
}
924960
turnChangesSummaryInFlightRef.current.delete(turnId)
925961
})
926962

927963
return () => {
928964
cancelled = true
929965
}
930-
}, [props.api, props.session.id, props.turns, turnChangesSummaryByTurnId])
966+
}, [props.api, props.session.id, props.turns])
931967

932968
useEffect(() => {
933969
if (typeof window === 'undefined') {

web/src/components/AssistantChat/briefTurnPresentation.test.ts

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { describe, expect, it } from 'vitest'
2-
import { isBriefTurnLive, shouldShowLatestBriefTurnAsFullContent } from './briefTurnPresentation'
2+
import {
3+
isBriefTurnLive,
4+
shouldFetchLatestTurnChangesSummary,
5+
shouldShowLatestBriefTurnAsFullContent
6+
} from './briefTurnPresentation'
37

48
describe('brief turn presentation', () => {
59
it('latest closed turn uses full content (non-card) even when thinking', () => {
@@ -36,4 +40,40 @@ describe('brief turn presentation', () => {
3640
isLiveTurn: false
3741
})).toBe(false)
3842
})
43+
44+
it('fetches turn changes for latest turn even when turn is open (regression)', () => {
45+
expect(shouldFetchLatestTurnChangesSummary({
46+
latestTurnId: 'turn-1',
47+
latestTurnUpdatedAt: 100,
48+
fetchedUpdatedAtByTurnId: {},
49+
inFlightTurnIds: new Set()
50+
})).toBe(true)
51+
})
52+
53+
it('does not fetch when same updatedAt already fetched', () => {
54+
expect(shouldFetchLatestTurnChangesSummary({
55+
latestTurnId: 'turn-1',
56+
latestTurnUpdatedAt: 100,
57+
fetchedUpdatedAtByTurnId: { 'turn-1': 100 },
58+
inFlightTurnIds: new Set()
59+
})).toBe(false)
60+
})
61+
62+
it('re-fetches when latest turn updatedAt changes', () => {
63+
expect(shouldFetchLatestTurnChangesSummary({
64+
latestTurnId: 'turn-1',
65+
latestTurnUpdatedAt: 101,
66+
fetchedUpdatedAtByTurnId: { 'turn-1': 100 },
67+
inFlightTurnIds: new Set()
68+
})).toBe(true)
69+
})
70+
71+
it('does not fetch while in-flight', () => {
72+
expect(shouldFetchLatestTurnChangesSummary({
73+
latestTurnId: 'turn-1',
74+
latestTurnUpdatedAt: 100,
75+
fetchedUpdatedAtByTurnId: {},
76+
inFlightTurnIds: new Set(['turn-1'])
77+
})).toBe(false)
78+
})
3979
})

web/src/components/AssistantChat/briefTurnPresentation.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,20 @@ export function shouldShowLatestBriefTurnAsFullContent(params: {
1414
}): boolean {
1515
return params.isLatestTurn && !params.isLiveTurn
1616
}
17+
18+
export function shouldFetchLatestTurnChangesSummary(params: {
19+
latestTurnId: string | null
20+
latestTurnUpdatedAt: number | null
21+
fetchedUpdatedAtByTurnId: Record<string, number>
22+
inFlightTurnIds: Set<string>
23+
}): boolean {
24+
if (!params.latestTurnId || params.latestTurnUpdatedAt === null) {
25+
return false
26+
}
27+
28+
if (params.inFlightTurnIds.has(params.latestTurnId)) {
29+
return false
30+
}
31+
32+
return params.fetchedUpdatedAtByTurnId[params.latestTurnId] !== params.latestTurnUpdatedAt
33+
}

0 commit comments

Comments
 (0)