diff --git a/web/src/hooks/useSSE.test.ts b/web/src/hooks/useSSE.test.ts index e098f9ef..80f44660 100644 --- a/web/src/hooks/useSSE.test.ts +++ b/web/src/hooks/useSSE.test.ts @@ -1,5 +1,28 @@ import { describe, expect, it } from 'vitest' -import { isGlobalScopedMessageStreamEvent } from './useSSE' +import type { SessionSummary } from '@/types/api' +import type { Session } from '@/types/api' +import { isGlobalScopedMessageStreamEvent, isRenderIrrelevantPatch, isRenderIrrelevantSessionPatch } from './useSSE' + +function makeSummary(overrides: Partial = {}): SessionSummary { + return { + id: 'session-1', + active: true, + thinking: false, + activeAt: 1_000, + updatedAt: 2_000, + metadata: null, + todoProgress: null, + pendingRequestsCount: 0, + pendingRequestKinds: [], + pendingRequests: [], + backgroundTaskCount: 0, + futureScheduledMessageCount: 0, + nextScheduledAt: null, + model: null, + effort: null, + ...overrides + } as SessionSummary +} describe('useSSE scope handling', () => { it('treats message stream events as global-scoped skips', () => { @@ -19,3 +42,78 @@ describe('useSSE scope handling', () => { expect(isGlobalScopedMessageStreamEvent('full', 'message-received')).toBe(false) }) }) + +describe('isRenderIrrelevantPatch', () => { + it('treats a keep-alive that only moves activeAt as irrelevant', () => { + const current = makeSummary({ activeAt: 1_000 }) + const next = makeSummary({ activeAt: 11_000 }) + + expect(isRenderIrrelevantPatch(current, next)).toBe(true) + }) + + it('treats an identical summary as irrelevant', () => { + expect(isRenderIrrelevantPatch(makeSummary(), makeSummary())).toBe(true) + }) + + it.each([ + ['active', { active: false }], + ['thinking', { thinking: true }], + ['updatedAt', { updatedAt: 9_999 }], + ['backgroundTaskCount', { backgroundTaskCount: 3 }], + ['model', { model: 'opus' }], + ['modelReasoningEffort', { modelReasoningEffort: 'high' }], + ['effort', { effort: 'medium' }], + ['pendingRequestsCount', { pendingRequestsCount: 2 }] + ] as Array<[string, Partial]>)('reports %s changes as relevant', (_field, change) => { + const current = makeSummary() + const next = makeSummary({ ...change, activeAt: 11_000 }) + + expect(isRenderIrrelevantPatch(current, next)).toBe(false) + }) +}) + +describe('isRenderIrrelevantSessionPatch', () => { + const session = { + id: 'session-1', + active: true, + thinking: false, + activeAt: 1_000, + updatedAt: 2_000, + model: 'opus', + effort: null, + permissionMode: 'default', + serviceTier: null + } as unknown as Session + + it('treats a keep-alive that only moves activeAt as irrelevant', () => { + expect(isRenderIrrelevantSessionPatch(session, { + active: true, + thinking: false, + activeAt: 11_000, + model: 'opus', + effort: null, + permissionMode: 'default', + serviceTier: null + })).toBe(true) + }) + + it('reports a changed field as relevant even alongside a new activeAt', () => { + expect(isRenderIrrelevantSessionPatch(session, { + thinking: true, + activeAt: 11_000 + })).toBe(false) + }) + + it('reports a field the session does not carry yet as relevant', () => { + // scratchlistUpdatedAt is absent from the cached session, so the patch + // genuinely adds information and must not be dropped. + expect(isRenderIrrelevantSessionPatch(session, { + activeAt: 11_000, + scratchlistUpdatedAt: 5_000 + })).toBe(false) + }) + + it('treats an empty patch as irrelevant', () => { + expect(isRenderIrrelevantSessionPatch(session, {})).toBe(true) + }) +}) diff --git a/web/src/hooks/useSSE.ts b/web/src/hooks/useSSE.ts index d27455c1..b86eb22f 100644 --- a/web/src/hooks/useSSE.ts +++ b/web/src/hooks/useSSE.ts @@ -55,10 +55,51 @@ function sortSessionSummaries(left: SessionSummary, right: SessionSummary): numb return right.updatedAt - left.updatedAt } +/** + * True when applying `patch` to `session` would change nothing that renders. + * + * Same reasoning as {@link isRenderIrrelevantPatch}, for the session-detail + * cache: the keep-alive patch repeats every field it knows about, so compare + * each one against the value already stored and ignore `activeAt`, which has + * no reader. + */ +export function isRenderIrrelevantSessionPatch(session: Session, patch: SessionPatch): boolean { + const current = session as unknown as Record + for (const [key, value] of Object.entries(patch)) { + if (key === 'activeAt') { + continue + } + if (current[key] !== value) { + return false + } + } + return true +} + function isSessionRecord(value: unknown): value is Session { return SessionSchema.safeParse(value).success } +/** + * True when the only difference between two summaries is `activeAt`. + * + * The CLI keep-alive makes the hub re-broadcast a full session patch about + * every 10s per active session even when nothing changed, and `activeAt` is + * the sole field that moves. No component reads `activeAt` - the list shows + * `updatedAt` and sorts on active/pendingRequestsCount/updatedAt - so storing + * it costs a new object identity and a full list re-render for nothing. + */ +export function isRenderIrrelevantPatch(current: SessionSummary, next: SessionSummary): boolean { + return current.active === next.active + && current.thinking === next.thinking + && current.updatedAt === next.updatedAt + && current.backgroundTaskCount === next.backgroundTaskCount + && current.model === next.model + && current.modelReasoningEffort === next.modelReasoningEffort + && current.effort === next.effort + && current.pendingRequestsCount === next.pendingRequestsCount +} + function getSessionPatch(value: unknown): SessionPatch | null { const parsed = SessionPatchSchema.safeParse(value) if (!parsed.success) { @@ -370,6 +411,13 @@ export function useSSE(options: { } patched = true + // The keep-alive patch repeats every field every ~10s per active + // session, and `activeAt` is the only one that actually moves. + // Nothing renders `activeAt`, so writing a new object for it just + // hands React Query a fresh reference and re-renders the list. + if (isRenderIrrelevantPatch(current, nextSummary)) { + return previous + } nextSessions[index] = nextSummary nextSessions.sort(sortSessionSummaries) return { ...previous, sessions: nextSessions } @@ -384,6 +432,9 @@ export function useSSE(options: { return previous } patched = true + if (isRenderIrrelevantSessionPatch(previous.session, patch)) { + return previous + } return { ...previous, session: {