perf(web): skip session cache writes that only move activeAt (#1232)

This commit is contained in:
Haoqing Wang
2026-07-29 20:12:33 +08:00
committed by GitHub
parent e3ca31da08
commit f5673e89bd
2 changed files with 150 additions and 1 deletions
+99 -1
View File
@@ -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> = {}): 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<SessionSummary>]>)('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)
})
})
+51
View File
@@ -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<string, unknown>
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: {