From c5e90eb192914a9690ad4473a310f9bd25207dbf Mon Sep 17 00:00:00 2001 From: AstroHan Date: Sat, 29 Aug 2026 02:30:14 +0800 Subject: [PATCH 1/6] fix(desktop): keep session workspace action identities fixed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `setActiveId` and its siblings were function declarations in the `useAppShellSessionWorkspace` body, so every AppShell render handed consumers new identities. `activateSession` alone invalidated `openSession`'s `useCallback`, then the Session navigation controller's `commands`, then the host's `rowActions` and `onSelectSession`, then `renderSessionRow` — which defeated `SessionNavRow`'s `memo` on every commit. One session switch re-rendered all 32 sidebar rows about twenty times, and each Astryx button rewrote its inline `anchor-name` per render, so a switch also produced roughly 2,500 style writes. Every dependency these actions close over is a ref box, a React state setter, or a method of the once-created session-UI controller, so they are constant by construction rather than by discipline. `createSessionWorkspaceActions` moves them out of the render body and the hook instantiates it once; `refreshSessions` and `seedSessions` get the same treatment. This is why they are not routed through `useStableActions`, whose facade exists for factories whose closures do capture changing deps. Measured by alternating the two identities inside one running instance, six switches each: row renders 459 to 85, DOM mutations 3,438 to 1,992, renderer JS 521 ms to 380 ms, and renderer CPU under a repeated-switch loop 34% to 24% with peaks falling from 77% to 55%. Two imports in the session-list hook gain their `.js` extension so the workspace module tree loads under Node, which the new identity contract test needs. Generated-by: Claude Code --- .../session-workspace-action-identity.test.ts | 88 +++++++ .../src/renderer/session-workspace-actions.ts | 241 ++++++++++++++++++ .../renderer/use-app-shell-session-list.ts | 37 +-- .../use-app-shell-session-workspace.ts | 191 +++----------- 4 files changed, 380 insertions(+), 177 deletions(-) create mode 100644 apps/desktop/src/main/__tests__/session-workspace-action-identity.test.ts create mode 100644 apps/desktop/src/renderer/session-workspace-actions.ts diff --git a/apps/desktop/src/main/__tests__/session-workspace-action-identity.test.ts b/apps/desktop/src/main/__tests__/session-workspace-action-identity.test.ts new file mode 100644 index 0000000000..19d1c56a7d --- /dev/null +++ b/apps/desktop/src/main/__tests__/session-workspace-action-identity.test.ts @@ -0,0 +1,88 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { strict as assert } from 'node:assert'; +import { afterEach, describe, it } from 'node:test'; +import { act, createElement } from 'react'; +import { LocaleProvider } from '@maka/ui'; +import { cleanupFakeDom, installReactRenderer } from './fake-dom.js'; +import { useAppShellSessionWorkspace } from '../../renderer/use-app-shell-session-workspace.js'; + +/** + * The session workspace hands its actions to consumers that put them in + * dependency arrays and pass them down as props. When `setActiveId` was a + * function declaration in the hook body it changed identity every render, which + * rebuilt the whole Session rail command chain and defeated `SessionNavRow`'s + * `memo` on every commit — measured at 20 full re-renders of a 32-row sidebar + * for a single session switch. Identity is a contract, not an implementation + * detail, so it is asserted here rather than left to review. + */ +const ACTION_KEYS = [ + 'setActiveId', + 'startNewSession', + 'clearOwnedSessionState', + 'setMessages', + 'addTransientMessage', + 'updateTransientMessage', + 'projectQueuedTransientMessages', + 'retireCancelledTransientMessages', + 'removeTransientMessage', + 'refreshSessions', + 'seedSessions', +] as const; + +type Workspace = ReturnType; + +describe('session workspace action identity', () => { + afterEach(cleanupFakeDom); + + it('keeps every action identity fixed across re-renders', () => { + const { root } = installReactRenderer(); + const reads: Workspace[] = []; + + function Probe(): null { + reads.push(useAppShellSessionWorkspace({ error: () => {} })); + return null; + } + + act(() => { + root.render( + createElement(LocaleProvider, { locale: 'en', children: createElement(Probe) }), + ); + }); + assert.equal(reads.length, 1); + + // Three unrelated state changes, each of which re-renders the hook. + act(() => reads[0]!.setActiveId('session-a')); + act(() => reads[0]!.setMessages([])); + act(() => reads[0]!.setMessageLoadPending(true)); + assert.ok(reads.length > 1, 'the probe should have re-rendered'); + + const first = reads[0]!; + for (const key of ACTION_KEYS) { + for (const later of reads.slice(1)) { + assert.equal( + later[key], + first[key], + `${key} changed identity between renders`, + ); + } + } + }); +}); diff --git a/apps/desktop/src/renderer/session-workspace-actions.ts b/apps/desktop/src/renderer/session-workspace-actions.ts new file mode 100644 index 0000000000..bef2637567 --- /dev/null +++ b/apps/desktop/src/renderer/session-workspace-actions.ts @@ -0,0 +1,241 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * Session-workspace actions: active-session selection, the durable message + * list, and the transient (optimistic) message projection. + * + * Every dependency here is a ref box, a React state setter, or a method of the + * once-created session-UI controller — all fixed for the renderer's lifetime. + * The factory therefore runs ONCE (issue #1043's `useStableActions` exists for + * factories whose closures capture changing deps; these capture none, so the + * cheaper structural guarantee applies). Declaring these in the hook body + * instead handed every consumer a fresh identity per render, and + * `activateSession` alone rebuilt the Session rail's whole command chain, which + * defeated `SessionNavRow`'s `memo` on every commit. + */ + +import type { StoredMessage } from '@maka/core/session'; +import type { TransientUserMessageProjection } from '@maka/ui'; +import { MESSAGE_QUEUE_MAX_ENTRIES } from '@maka/runtime-host/protocol'; +import { clearNewTaskReloadIntent, markNewTaskReloadIntent } from './new-task-reload-intent.js'; +import type { DesktopTranscriptRangeController } from './desktop-transcript-range-store.js'; +import { + mergeTransientMessageProjection, + projectQueuedTransientMessages as applyQueuedTransientProjection, + reconcileTransientMessages, +} from './transient-message-projection.js'; + +type RefBox = { current: T }; + +type TransientUserMessage = TransientUserMessageProjection; + +export type MessageListUpdater = ( + next: StoredMessage[] | ((current: StoredMessage[]) => StoredMessage[]), +) => void; + +export interface SessionWorkspaceActions { + setActiveId(next: string | undefined): void; + startNewSession(): void; + clearOwnedSessionState(sessionId: string): void; + setMessages: MessageListUpdater; + addTransientMessage(sessionId: string, message: TransientUserMessage): void; + updateTransientMessage(sessionId: string, message: TransientUserMessage): void; + projectQueuedTransientMessages( + sessionId: string, + messages: readonly TransientUserMessage[], + ): void; + retireCancelledTransientMessages(sessionId: string): Promise; + removeTransientMessage(sessionId: string, messageId: string): void; +} + +export function createSessionWorkspaceActions(deps: { + activeIdRef: RefBox; + messagesRef: RefBox; + transientMessagesBySessionRef: RefBox>>; + transcriptRangeRef: RefBox; + selectionRevisionRef: RefBox; + messageRetryPendingRef: RefBox>; + stopPendingRef: RefBox>; + setActiveIdState: (next: string | undefined) => void; + setMessagesState: (next: StoredMessage[]) => void; + setTransientMessagesState: (next: TransientUserMessage[]) => void; + setMessageLoadPending: (pending: boolean) => void; + clearSessionUiState: (sessionId: string) => void; +}): SessionWorkspaceActions { + const { + activeIdRef, + messagesRef, + transientMessagesBySessionRef, + transcriptRangeRef, + selectionRevisionRef, + messageRetryPendingRef, + stopPendingRef, + setActiveIdState, + setMessagesState, + setTransientMessagesState, + setMessageLoadPending, + clearSessionUiState, + } = deps; + + function projectTransientMessages( + sessionId: string, + durable: readonly StoredMessage[], + ): TransientUserMessage[] { + const pending = transientMessagesBySessionRef.current.get(sessionId); + if (!pending || pending.size === 0) return []; + let includeTransient = true; + try { + const range = transcriptRangeRef.current?.store.range(); + includeTransient = range?.sessionId !== sessionId || !range.hasNewer; + } catch { + // An unopened transcript has no historical range to hide the live tail from. + } + const projected = reconcileTransientMessages(pending, durable, { includeTransient }); + if (pending.size === 0) { + transientMessagesBySessionRef.current.delete(sessionId); + } + return projected; + } + + function reprojectActiveTransients(sessionId: string): void { + if (activeIdRef.current !== sessionId) return; + setTransientMessagesState(projectTransientMessages(sessionId, messagesRef.current)); + } + + const setMessages: MessageListUpdater = (next) => { + const projected = typeof next === 'function' ? next([...messagesRef.current]) : next; + messagesRef.current = projected; + setMessagesState(projected); + const sessionId = activeIdRef.current; + setTransientMessagesState( + sessionId ? projectTransientMessages(sessionId, projected) : [], + ); + }; + + function addTransientMessage(sessionId: string, message: TransientUserMessage): void { + let pending = transientMessagesBySessionRef.current.get(sessionId); + if (!pending) { + pending = new Map(); + transientMessagesBySessionRef.current.set(sessionId, pending); + } + pending.set(message.id, message); + reprojectActiveTransients(sessionId); + } + + function updateTransientMessage(sessionId: string, message: TransientUserMessage): void { + const pending = transientMessagesBySessionRef.current.get(sessionId); + const current = pending?.get(message.id); + if (!pending || !current) return; + pending.set(message.id, mergeTransientMessageProjection(current, message)); + reprojectActiveTransients(sessionId); + } + + function projectQueuedTransientMessages( + sessionId: string, + messages: readonly TransientUserMessage[], + ): void { + let pending = transientMessagesBySessionRef.current.get(sessionId); + if (!pending && messages.length === 0) return; + if (!pending) { + pending = new Map(); + transientMessagesBySessionRef.current.set(sessionId, pending); + } + applyQueuedTransientProjection(pending, messages); + reprojectActiveTransients(sessionId); + } + + async function retireCancelledTransientMessages(sessionId: string): Promise { + const pending = transientMessagesBySessionRef.current.get(sessionId); + if (!pending || pending.size === 0) return; + try { + // A legal Host queue already fills the protocol's per-query cap, and an + // unreconciled root Message sits beside it, so asking about every row at + // once fails the whole proof and retires nothing. + const messageIds = [...pending.keys()]; + const cancelled: string[] = []; + for (let from = 0; from < messageIds.length; from += MESSAGE_QUEUE_MAX_ENTRIES) { + const result = await window.maka.sessions.queryCancelledMessages( + sessionId, + messageIds.slice(from, from + MESSAGE_QUEUE_MAX_ENTRIES), + ); + cancelled.push(...result.cancelledMessageIds); + } + const current = transientMessagesBySessionRef.current.get(sessionId); + if (!current) return; + for (const messageId of cancelled) current.delete(messageId); + if (current.size === 0) transientMessagesBySessionRef.current.delete(sessionId); + reprojectActiveTransients(sessionId); + } catch { + // A failed proof query leaves presentation intact until canonical proof arrives. + } + } + + function removeTransientMessage(sessionId: string, messageId: string): void { + const pending = transientMessagesBySessionRef.current.get(sessionId); + if (!pending?.delete(messageId)) return; + if (pending.size === 0) transientMessagesBySessionRef.current.delete(sessionId); + reprojectActiveTransients(sessionId); + } + + function setActiveId(next: string | undefined): void { + selectionRevisionRef.current += 1; + // Clear here, not in the read effect: a layout-effect clear would wipe an + // optimistic first message before the first paint. + if (!next) { + setMessageLoadPending(false); + } else if (next !== activeIdRef.current) { + messagesRef.current = []; + setMessagesState([]); + setTransientMessagesState(projectTransientMessages(next, [])); + setMessageLoadPending(true); + } + activeIdRef.current = next; + if (next) clearNewTaskReloadIntent(); + setActiveIdState(next); + } + + function startNewSession(): void { + markNewTaskReloadIntent(); + setActiveId(undefined); + messagesRef.current = []; + setMessagesState([]); + setTransientMessagesState([]); + } + + function clearOwnedSessionState(sessionId: string): void { + messageRetryPendingRef.current.delete(sessionId); + stopPendingRef.current.delete(sessionId); + transientMessagesBySessionRef.current.delete(sessionId); + if (activeIdRef.current === sessionId) setTransientMessagesState([]); + clearSessionUiState(sessionId); + } + + return { + setActiveId, + startNewSession, + clearOwnedSessionState, + setMessages, + addTransientMessage, + updateTransientMessage, + projectQueuedTransientMessages, + retireCancelledTransientMessages, + removeTransientMessage, + }; +} diff --git a/apps/desktop/src/renderer/use-app-shell-session-list.ts b/apps/desktop/src/renderer/use-app-shell-session-list.ts index 4c664a49ca..d1c4695f55 100644 --- a/apps/desktop/src/renderer/use-app-shell-session-list.ts +++ b/apps/desktop/src/renderer/use-app-shell-session-list.ts @@ -23,11 +23,11 @@ import { getDesktopConversationCopy } from './locales/conversation-copy.js'; import { localizedShellErrorMessage } from './locales/shell-copy.js'; import { normalizeSessionSummaryForDisplay, -} from './session-status-presentation'; +} from './session-status-presentation.js'; import { createSessionListRefresher, type SessionListRefresher, -} from './session-read-state'; +} from './session-read-state.js'; import { reconcileSettledSessionTransients } from './settled-session-transients.js'; import type { DesktopSessionSummary } from '../preload/bridge-contract.js'; @@ -97,19 +97,26 @@ export function useAppShellSessionList( }); } - async function refreshSessions(): Promise { - return refresherRef.current!.refresh(); - } - - function seedSessions( - snapshotSessions: readonly DesktopSessionSummary[], - ): DesktopSessionSummary[] { - const next = snapshotSessions.map(normalizeSessionSummaryForDisplay); - commitSessions(next); - return next; - } - - + // Fixed identities for the renderer's lifetime: both close over ref boxes and + // a state setter only, and consumers list them in dep arrays and hand them + // down as props (see `session-workspace-actions.ts`). + const actionsRef = useRef<{ + refreshSessions(): Promise; + seedSessions( + snapshotSessions: readonly DesktopSessionSummary[], + ): DesktopSessionSummary[]; + } | null>(null); + actionsRef.current ??= { + async refreshSessions() { + return refresherRef.current!.refresh(); + }, + seedSessions(snapshotSessions) { + const next = snapshotSessions.map(normalizeSessionSummaryForDisplay); + commitSessions(next); + return next; + }, + }; + const { refreshSessions, seedSessions } = actionsRef.current; return { sessions, diff --git a/apps/desktop/src/renderer/use-app-shell-session-workspace.ts b/apps/desktop/src/renderer/use-app-shell-session-workspace.ts index b5d6fa92a4..d607d11621 100644 --- a/apps/desktop/src/renderer/use-app-shell-session-workspace.ts +++ b/apps/desktop/src/renderer/use-app-shell-session-workspace.ts @@ -20,30 +20,20 @@ import { useRef, useState } from 'react'; import type { StoredMessage } from '@maka/core/session'; import type { TransientUserMessageProjection } from '@maka/ui'; -import { MESSAGE_QUEUE_MAX_ENTRIES } from '@maka/runtime-host/protocol'; -import { useAppShellSessionUiState } from './app-shell-session-ui-state'; -import { useAppShellSessionList } from './use-app-shell-session-list'; -import { createBootstrapSelectionLease } from './bootstrap-selection-lease'; -import { - clearNewTaskReloadIntent, - hasNewTaskReloadIntent, - markNewTaskReloadIntent, -} from './new-task-reload-intent'; +import { useAppShellSessionUiState } from './app-shell-session-ui-state.js'; +import { useAppShellSessionList } from './use-app-shell-session-list.js'; +import { createBootstrapSelectionLease } from './bootstrap-selection-lease.js'; +import { hasNewTaskReloadIntent } from './new-task-reload-intent.js'; import type { DesktopTranscriptRangeController } from './desktop-transcript-range-store.js'; import { - mergeTransientMessageProjection, - projectQueuedTransientMessages as applyQueuedTransientProjection, - reconcileTransientMessages, -} from './transient-message-projection.js'; + createSessionWorkspaceActions, + type SessionWorkspaceActions, +} from './session-workspace-actions.js'; type ToastApi = { error(title: string, description?: string): void; }; -type MessageListUpdater = ( - next: StoredMessage[] | ((current: StoredMessage[]) => StoredMessage[]), -) => void; - type TransientUserMessage = TransientUserMessageProjection; export function useAppShellSessionWorkspace(toastApi: ToastApi) { @@ -68,167 +58,44 @@ export function useAppShellSessionWorkspace(toastApi: ToastApi) { const messageRetryPendingRef = useRef>(new Set()); const stopPendingRef = useRef>(new Set()); - function projectTransientMessages( - sessionId: string, - durable: readonly StoredMessage[], - ): TransientUserMessage[] { - const pending = transientMessagesBySessionRef.current.get(sessionId); - if (!pending || pending.size === 0) return []; - let includeTransient = true; - try { - const range = transcriptRangeRef.current?.store.range(); - includeTransient = range?.sessionId !== sessionId || !range.hasNewer; - } catch { - // An unopened transcript has no historical range to hide the live tail from. - } - const projected = reconcileTransientMessages(pending, durable, { includeTransient }); - if (pending.size === 0) { - transientMessagesBySessionRef.current.delete(sessionId); - } - return projected; - } - - const setMessagesForActiveSession: MessageListUpdater = (next) => { - const projected = typeof next === 'function' ? next([...messagesRef.current]) : next; - messagesRef.current = projected; - setMessages(projected); - const sessionId = activeIdRef.current; - setTransientMessages(sessionId ? projectTransientMessages(sessionId, projected) : []); - }; - - function addTransientMessage(sessionId: string, message: TransientUserMessage): void { - let pending = transientMessagesBySessionRef.current.get(sessionId); - if (!pending) { - pending = new Map(); - transientMessagesBySessionRef.current.set(sessionId, pending); - } - pending.set(message.id, message); - if (activeIdRef.current === sessionId) { - setTransientMessages(projectTransientMessages(sessionId, messagesRef.current)); - } - } - - function updateTransientMessage(sessionId: string, message: TransientUserMessage): void { - const pending = transientMessagesBySessionRef.current.get(sessionId); - const current = pending?.get(message.id); - if (!pending || !current) return; - pending.set(message.id, mergeTransientMessageProjection(current, message)); - if (activeIdRef.current === sessionId) { - setTransientMessages(projectTransientMessages(sessionId, messagesRef.current)); - } - } - - function projectQueuedTransientMessages( - sessionId: string, - messages: readonly TransientUserMessage[], - ): void { - let pending = transientMessagesBySessionRef.current.get(sessionId); - if (!pending && messages.length === 0) return; - if (!pending) { - pending = new Map(); - transientMessagesBySessionRef.current.set(sessionId, pending); - } - applyQueuedTransientProjection(pending, messages); - if (activeIdRef.current === sessionId) { - setTransientMessages(projectTransientMessages(sessionId, messagesRef.current)); - } - } - - async function retireCancelledTransientMessages(sessionId: string): Promise { - const pending = transientMessagesBySessionRef.current.get(sessionId); - if (!pending || pending.size === 0) return; - try { - // A legal Host queue already fills the protocol's per-query cap, and an - // unreconciled root Message sits beside it, so asking about every row at - // once fails the whole proof and retires nothing. - const messageIds = [...pending.keys()]; - const cancelled: string[] = []; - for (let from = 0; from < messageIds.length; from += MESSAGE_QUEUE_MAX_ENTRIES) { - const result = await window.maka.sessions.queryCancelledMessages( - sessionId, - messageIds.slice(from, from + MESSAGE_QUEUE_MAX_ENTRIES), - ); - cancelled.push(...result.cancelledMessageIds); - } - const current = transientMessagesBySessionRef.current.get(sessionId); - if (!current) return; - for (const messageId of cancelled) current.delete(messageId); - if (current.size === 0) transientMessagesBySessionRef.current.delete(sessionId); - if (activeIdRef.current === sessionId) { - setTransientMessages(projectTransientMessages(sessionId, messagesRef.current)); - } - } catch { - // A failed proof query leaves presentation intact until canonical proof arrives. - } - } - - function removeTransientMessage(sessionId: string, messageId: string): void { - const pending = transientMessagesBySessionRef.current.get(sessionId); - if (!pending?.delete(messageId)) return; - if (pending.size === 0) transientMessagesBySessionRef.current.delete(sessionId); - if (activeIdRef.current === sessionId) { - setTransientMessages(projectTransientMessages(sessionId, messagesRef.current)); - } - } - - function setActiveId(next: string | undefined): void { - selectionRevisionRef.current += 1; - // Clear here, not in the read effect: a layout-effect clear would wipe an - // optimistic first message before the first paint. - if (!next) { - setMessageLoadPending(false); - } else if (next !== activeIdRef.current) { - messagesRef.current = []; - setMessages([]); - setTransientMessages(projectTransientMessages(next, [])); - setMessageLoadPending(true); - } - activeIdRef.current = next; - if (next) clearNewTaskReloadIntent(); - setActiveIdState(next); - } + const actionsRef = useRef(null); + // Every dep below is a ref box, a state setter, or a method of the + // once-created session-UI controller, so one instance serves the renderer's + // lifetime. Consumers list these in dep arrays and pass them as props; a + // per-render identity there is what defeated the Session rail's memo. + actionsRef.current ??= createSessionWorkspaceActions({ + activeIdRef, + messagesRef, + transientMessagesBySessionRef, + transcriptRangeRef, + selectionRevisionRef, + messageRetryPendingRef, + stopPendingRef, + setActiveIdState, + setMessagesState: setMessages, + setTransientMessagesState: setTransientMessages, + setMessageLoadPending, + clearSessionUiState: sessionUi.clearSessionUiState, + }); + const actions = actionsRef.current; if (!bootstrapSelectionLeaseRef.current) { bootstrapSelectionLeaseRef.current = createBootstrapSelectionLease({ readActiveId: () => activeIdRef.current, readSelectionRevision: () => selectionRevisionRef.current, - select: setActiveId, + select: actions.setActiveId, }); if (hasNewTaskReloadIntent()) bootstrapSelectionLeaseRef.current.release(); } - function startNewSession(): void { - markNewTaskReloadIntent(); - setActiveId(undefined); - messagesRef.current = []; - setMessages([]); - setTransientMessages([]); - } - - function clearOwnedSessionState(sessionId: string): void { - messageRetryPendingRef.current.delete(sessionId); - stopPendingRef.current.delete(sessionId); - transientMessagesBySessionRef.current.delete(sessionId); - if (activeIdRef.current === sessionId) setTransientMessages([]); - sessionUi.clearSessionUiState(sessionId); - } - return { ...sessionList, activeId, activeIdRef, bootstrapSelectionLease: bootstrapSelectionLeaseRef.current, - setActiveId, - startNewSession, - clearOwnedSessionState, + ...actions, messages, transientMessages, - setMessages: setMessagesForActiveSession, - addTransientMessage, - updateTransientMessage, - projectQueuedTransientMessages, - retireCancelledTransientMessages, - removeTransientMessage, transcriptRangeRef, messageLoadPending, setMessageLoadPending, From e52421e1d9ddf8b9a843944107d3f026f4858189 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Sat, 29 Aug 2026 02:32:20 +0800 Subject: [PATCH 2/6] refactor: give absolute timestamps a single formatter authority MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `@maka/ui`'s `formatAbsoluteTimestamp` was a second copy of the `Intl` options `@maka/core/relative-time` already owned, and it built a formatter on every call — the session sidebar reads one per row for the tooltip and one for the row's accessible name, so a single session switch constructed roughly 1,300 of them. Core's own cache could not have absorbed that either: `getRelativeFormat` and `getAbsoluteFormat` shared one `cachedLocale` and cleared each other on a miss, so alternating readings of the same timestamp rebuilt a formatter every call. Core now caches each formatter with its own locale and exports `formatAbsoluteTimestamp`; the UI copy is re-exported rather than reimplemented, so the tooltip and the accessible name cannot drift. Profiling attributed about 33 ms per session switch to the constructions. An A/B inside one running instance, swapping a memoising `Intl.DateTimeFormat` in and out six times each, moved renderer JS by less than the run-to-run spread — this is a duplicate-authority removal, not a measurable win. Generated-by: Claude Code --- .../core/src/__tests__/relative-time.test.ts | 39 ++++++++++++++++ packages/core/src/relative-time.ts | 46 ++++++++++++------- packages/ui/src/chat-display-helpers.ts | 21 +++------ 3 files changed, 75 insertions(+), 31 deletions(-) diff --git a/packages/core/src/__tests__/relative-time.test.ts b/packages/core/src/__tests__/relative-time.test.ts index 008c4b740e..2507f2c167 100644 --- a/packages/core/src/__tests__/relative-time.test.ts +++ b/packages/core/src/__tests__/relative-time.test.ts @@ -20,6 +20,7 @@ import assert from 'node:assert/strict'; import { describe, it } from 'node:test'; import { + formatAbsoluteTimestamp, formatCompactTimestamp, formatRelativeTimestamp, formatSidebarTimestamp, @@ -114,4 +115,42 @@ describe('relative timestamp labels', () => { assert.ok(delay === null || (Number.isFinite(delay) && delay > 0 && delay <= 10 * 60_000)); } }); + + it('reuses both formatters when relative and absolute readings alternate', () => { + resetRelativeTimeFormatters(); + const OriginalDateTimeFormat = Intl.DateTimeFormat; + const OriginalRelativeTimeFormat = Intl.RelativeTimeFormat; + let constructions = 0; + function countConstructions(name: 'DateTimeFormat' | 'RelativeTimeFormat'): void { + const Original = Intl[name] as unknown as new (...args: unknown[]) => unknown; + function Counting(...args: unknown[]): unknown { + constructions += 1; + return new Original(...args); + } + Object.defineProperty(Intl, name, { value: Counting, configurable: true, writable: true }); + } + countConstructions('DateTimeFormat'); + countConstructions('RelativeTimeFormat'); + try { + for (let round = 0; round < 5; round += 1) { + // The sidebar reads both per row: the relative label and, for the + // accessible name and the tooltip, the absolute one. + formatRelativeTimestamp(NOW - 60_000, NOW, 'en'); + formatAbsoluteTimestamp(NOW - 60_000, 'en'); + } + assert.equal(constructions, 2, 'one formatter of each kind for one locale'); + } finally { + Object.defineProperty(Intl, 'DateTimeFormat', { + value: OriginalDateTimeFormat, + configurable: true, + writable: true, + }); + Object.defineProperty(Intl, 'RelativeTimeFormat', { + value: OriginalRelativeTimeFormat, + configurable: true, + writable: true, + }); + resetRelativeTimeFormatters(); + } + }); }); diff --git a/packages/core/src/relative-time.ts b/packages/core/src/relative-time.ts index b794ccc7b3..5ce6f475bd 100644 --- a/packages/core/src/relative-time.ts +++ b/packages/core/src/relative-time.ts @@ -57,31 +57,44 @@ function relativeAgeMs(ts: number, now: number): number { return Math.max(0, now - ts); } -let cachedRelativeFormat: Intl.RelativeTimeFormat | null = null; -let cachedAbsoluteFormat: Intl.DateTimeFormat | null = null; -let cachedLocale: string | null = null; +// One cache per formatter. They used to share `cachedLocale` and clear each +// other on a miss, so alternating relative and absolute reads — which is what +// the sidebar does, once per row — rebuilt an `Intl` formatter every call. +let cachedRelativeFormat: { locale: string; format: Intl.RelativeTimeFormat } | null = null; +let cachedAbsoluteFormat: { locale: string; format: Intl.DateTimeFormat } | null = null; function getRelativeFormat(uiLocale: UiLocale): Intl.RelativeTimeFormat { const locale = uiLocaleToIntlLocale(uiLocale); - if (!cachedRelativeFormat || cachedLocale !== locale) { - cachedRelativeFormat = new Intl.RelativeTimeFormat(locale, { numeric: 'auto' }); - cachedAbsoluteFormat = null; - cachedLocale = locale; + if (cachedRelativeFormat?.locale !== locale) { + cachedRelativeFormat = { + locale, + format: new Intl.RelativeTimeFormat(locale, { numeric: 'auto' }), + }; } - return cachedRelativeFormat; + return cachedRelativeFormat.format; } function getAbsoluteFormat(uiLocale: UiLocale): Intl.DateTimeFormat { const locale = uiLocaleToIntlLocale(uiLocale); - if (!cachedAbsoluteFormat || cachedLocale !== locale) { - cachedAbsoluteFormat = new Intl.DateTimeFormat(locale, { - dateStyle: 'medium', - timeStyle: 'short', - }); - cachedRelativeFormat = null; - cachedLocale = locale; + if (cachedAbsoluteFormat?.locale !== locale) { + cachedAbsoluteFormat = { + locale, + format: new Intl.DateTimeFormat(locale, { + dateStyle: 'medium', + timeStyle: 'short', + }), + }; } - return cachedAbsoluteFormat; + return cachedAbsoluteFormat.format; +} + +/** + * Date and time for `ts`, spelled out. The single authority for the absolute + * reading a relative label falls back to and a tooltip shows; `@maka/ui` had + * its own uncached copy of the same `Intl` options until this became public. + */ +export function formatAbsoluteTimestamp(ts: number, locale: UiLocale = 'zh'): string { + return getAbsoluteFormat(locale).format(new Date(ts)); } /** @@ -180,7 +193,6 @@ export function formatSidebarTimestamp( export function resetRelativeTimeFormatters(): void { cachedRelativeFormat = null; cachedAbsoluteFormat = null; - cachedLocale = null; cachedCompactSameYearFormat = null; cachedCompactOtherYearFormat = null; cachedCompactLocale = null; diff --git a/packages/ui/src/chat-display-helpers.ts b/packages/ui/src/chat-display-helpers.ts index 5831a26ff5..c4d113e357 100644 --- a/packages/ui/src/chat-display-helpers.ts +++ b/packages/ui/src/chat-display-helpers.ts @@ -40,22 +40,15 @@ * sites. */ -import { uiLocaleToIntlLocale, type UiLocale } from '@maka/core/ui-locale'; +import type { UiLocale } from '@maka/core/ui-locale'; import { getConversationCopy } from './conversation-copy.js'; -function createAbsoluteTimeFormat(locale: UiLocale): Intl.DateTimeFormat { - if (typeof Intl === 'undefined' || typeof Intl.DateTimeFormat !== 'function') { - return { format: (d: Date) => d.toISOString() } as unknown as Intl.DateTimeFormat; - } - return new Intl.DateTimeFormat( - uiLocaleToIntlLocale(locale), - { dateStyle: 'medium', timeStyle: 'short' }, - ); -} - -export function formatAbsoluteTimestamp(ts: number, locale: UiLocale): string { - return createAbsoluteTimeFormat(locale).format(new Date(ts)); -} +/* `formatAbsoluteTimestamp` used to be a second copy of the same `Intl` options + `@maka/core/relative-time` already owned, and it built a formatter per call — + the sidebar renders one per row for the row tooltip and one for the row's + accessible name. Re-exported rather than reimplemented so the two readings of + a timestamp cannot drift apart. */ +export { formatAbsoluteTimestamp } from '@maka/core/relative-time'; /* `formatClockTime` (a 24-hour `HH:mm` for the user-message time) lived here until the meta row moved to Astryx's `Timestamp`. Locking the hour cycle was From 5aed4da150b169d31c640cbdc909bb23f9f08416 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Sat, 29 Aug 2026 02:47:33 +0800 Subject: [PATCH 3/6] test(desktop): budget the Session rail's DOM writes for one switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rail's cost has had several independent causes — `setActiveId` changing identity on every AppShell render, `Intl` formatters rebuilt per row, catalog refreshes replacing unchanged row objects — and each was invisible to the others. Asserting identities pins one mechanism in one hook; the next plain function declaration upstream passes every existing check, because the dependency arrays stay correct. So the assertion is on the outcome: switching a session may write at most three inline styles per rail row. Inline `style` is the dominant term, since every Astryx button removes and re-adds its `anchor-name` per render, and it needs no React internals to observe — a `MutationObserver` over the rail is the whole probe. Measured on the new twelve-row fixture: 4 writes when the rail behaves, the leaving and the arriving row at two each, stable across runs; 336 with `setActiveId` restored to a per-render identity. The budget of 36 sits an order of magnitude clear of both. This also covers the unattributed commit cascade in #4109: whatever raises the number of commits a switch produces shows up here. Generated-by: Claude Code --- apps/desktop/e2e/fixtures.ts | 37 +++++++ .../e2e/session-rail-render-contract.spec.ts | 104 ++++++++++++++++++ 2 files changed, 141 insertions(+) create mode 100644 apps/desktop/e2e/session-rail-render-contract.spec.ts diff --git a/apps/desktop/e2e/fixtures.ts b/apps/desktop/e2e/fixtures.ts index 85db3785c0..16b077cdb5 100644 --- a/apps/desktop/e2e/fixtures.ts +++ b/apps/desktop/e2e/fixtures.ts @@ -165,6 +165,28 @@ async function seedE2eLocale(userDataDir: string, locale: 'zh' | 'en'): Promise< }); } +/** Rows for the rail-render contract: enough that a stray render is loud. */ +export const RAIL_RENDER_SESSION_COUNT = 12; + +async function seedRailRenderSessions(userDataDir: string): Promise { + const workspaceRoot = path.join(userDataDir, 'workspaces', 'default'); + const store = createSessionStore(workspaceRoot); + try { + for (let index = 0; index < RAIL_RENDER_SESSION_COUNT; index += 1) { + await store.create({ + cwd: path.join(userDataDir, 'project'), + llmConnectionSlug: 'e2e', + model: 'claude-sonnet-4-5-20250929', + permissionMode: 'ask', + name: `Rail row ${index}`, + labels: [], + }); + } + } finally { + await store.close?.(); + } +} + async function seedParentRemovalSessions(userDataDir: string): Promise { const workspaceRoot = path.join(userDataDir, 'workspaces', 'default'); const store = createSessionStore(workspaceRoot); @@ -354,6 +376,7 @@ async function withE2eWindow( invocableSkills, gitReviewExtraFiles, parentRemovalSessions, + railRenderSessions, newTaskProject, }: { seed: boolean; @@ -369,6 +392,7 @@ async function withE2eWindow( invocableSkills?: boolean; gitReviewExtraFiles?: number; parentRemovalSessions?: boolean; + railRenderSessions?: boolean; newTaskProject?: boolean; }, use: (page: Page, context: { userDataDir: string }) => Promise, @@ -384,6 +408,7 @@ async function withE2eWindow( try { if (seed) await seedE2eConnection(userDataDir); if (parentRemovalSessions) await seedParentRemovalSessions(userDataDir); + if (railRenderSessions) await seedRailRenderSessions(userDataDir); if (invocableSkills) await seedE2eInvocableSkills(userDataDir); if (gitReviewExtraFiles !== undefined) { await seedE2eGitReviewProject(userDataDir, gitReviewExtraFiles); @@ -455,6 +480,7 @@ export const test = base.extend<{ linkColorWindow: Page; projectSidebarWindow: Page; parentRemovalWindow: Page; + railRenderWindow: Page; promptRailWindow: Page; promptRailMotionWindow: Page; requestHeaderRowWindow: Page; @@ -537,6 +563,17 @@ export const test = base.extend<{ use, ); }, + railRenderWindow: async ({}, use) => { + await withE2eWindow( + { + seed: true, + readinessSelector: COMPOSER_INPUT, + locale: 'zh', + railRenderSessions: true, + }, + use, + ); + }, // A multi-prompt transcript for the prompt anchor rail. Shown, because every // assertion in prompt-rail.spec.ts is geometry the compositor has to settle. promptRailWindow: async ({}, use) => { diff --git a/apps/desktop/e2e/session-rail-render-contract.spec.ts b/apps/desktop/e2e/session-rail-render-contract.spec.ts new file mode 100644 index 0000000000..72c9b66a45 --- /dev/null +++ b/apps/desktop/e2e/session-rail-render-contract.spec.ts @@ -0,0 +1,104 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { expect } from '@playwright/test'; +import { + ensureSidebarExpanded, + RAIL_RENDER_SESSION_COUNT, + test, +} from './fixtures.js'; + +/** + * Switching a session moves one row's selection. What it must not do is rewrite + * the rest of the rail. + * + * Deliberately a budget on the OUTCOME rather than an assertion about + * identities or `memo`. The rail's cost has had several independent causes — + * `setActiveId` changing identity every AppShell render, `Intl` formatters + * rebuilt per row, catalog refreshes replacing unchanged row objects — and each + * was invisible to the others. A DOM-write budget catches all of them and the + * ones not yet found, including anything that raises the number of commits a + * switch produces (#4109). + * + * The budget counts inline `style` writes on rail buttons because that is the + * dominant term: every Astryx button removes and re-adds its `anchor-name` per + * render, so one wasted rail render is two style writes per button plus the + * style recalculation they force. + * + * Measured on this fixture: 4 writes when the rail behaves — the leaving row + * and the arriving row, two each — against 336 with `setActiveId` restored to a + * per-render identity. The budget sits an order of magnitude clear of both, so + * it fails on a regression rather than on scheduling noise. + */ +const RAIL_STYLE_WRITE_BUDGET = 3 * RAIL_RENDER_SESSION_COUNT; + +test('switching sessions does not rewrite the whole Session rail', async ({ + railRenderWindow: page, +}) => { + await ensureSidebarExpanded(page); + + const rows = page.locator('.maka-session-row'); + await expect(rows).toHaveCount(RAIL_RENDER_SESSION_COUNT); + + const target = page.locator('.maka-session-row button.astryx-side-nav-item', { + hasText: 'Rail row 3', + }); + const selected = page.locator('.maka-session-row button.astryx-side-nav-item.selected'); + await expect(target).toBeVisible(); + + // Settle first: the budget is about a switch, not about arriving. + await page.waitForTimeout(500); + + await page.evaluate(() => { + const counter = { styleWrites: 0 }; + (window as unknown as { __railWrites: typeof counter }).__railWrites = counter; + const observer = new MutationObserver((records) => { + for (const record of records) { + const target = record.target as Element; + if (!target.closest?.('.maka-session-row')) continue; + counter.styleWrites += 1; + } + }); + observer.observe(document.body, { + subtree: true, + attributes: true, + attributeFilter: ['style'], + }); + (window as unknown as { __railObserver: MutationObserver }).__railObserver = observer; + }); + + await target.click(); + await expect(selected).toHaveText(/Rail row 3/); + // Let the post-switch commit cascade finish before reading the counter. + await page.waitForTimeout(1500); + + const styleWrites = await page.evaluate(() => { + const scope = window as unknown as { + __railWrites: { styleWrites: number }; + __railObserver: MutationObserver; + }; + scope.__railObserver.disconnect(); + return scope.__railWrites.styleWrites; + }); + + expect( + styleWrites, + `rail inline-style writes for one session switch (${RAIL_RENDER_SESSION_COUNT} rows)`, + ).toBeLessThanOrEqual(RAIL_STYLE_WRITE_BUDGET); +}); From ca531515eb12651676962f1e168072a4ef267c0d Mon Sep 17 00:00:00 2001 From: AstroHan Date: Sat, 29 Aug 2026 11:02:00 +0800 Subject: [PATCH 4/6] test(desktop): fail the rail contract on a single stray render MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The budget was a total, scaled by row count, and one-sided. Each of those let a real regression through. A total of `3 * rows` is 1.5 whole-rail renders, so a change that renders the rail exactly once more than it should stayed under it — and that is the likeliest regression, because `renderSessionRow` depends on `rowActions`, `sessionMeta` and three Sets, any of which becoming a fresh object per render defeats `SessionNavRow`'s memo for every row at once. The identity test could not see it either: it reads the workspace hook's return value, not what AppShell assembles from it. Attributing each write to its row removes the hole and the row-count coupling together — a switch touches the leaving row and the arriving row, whatever the rail's length — and it asserts the fix's own missing middle, that memo holding means untouched rows do no DOM work. Counting remounts closes the other side: React sets attributes before insertion, so an attribute-only observer reads a whole rail unmounting and remounting as CHEAPER than a re-render. `styleWrites > 0` is the counter's liveness check. Every write counted comes from an Astryx ref callback with no `useCallback` around it; if that is ever memoised upstream, healthy and regressed readings both collapse to zero and a one-sided budget passes forever. The two fixed `waitForTimeout` calls were the only thing keeping a slow machine out of the measurement window, with `retries: 0` behind them. Polling until the counter is quiet for ~300ms states the actual precondition, and runs faster: 2.2s against 3.6s. The identity test now derives its keys from the hook's return value. The hand-kept list covered 11 of the 23 functions it returns and would have kept covering 11 as more were added. Verified by reverting the fix in the built renderer bundle: the run fails on rows-touched, and passes three times in a row with the fix in place. Generated-by: Claude Code --- .../e2e/session-rail-render-contract.spec.ts | 128 ++++++++++++++---- .../session-workspace-action-identity.test.ts | 36 ++--- 2 files changed, 121 insertions(+), 43 deletions(-) diff --git a/apps/desktop/e2e/session-rail-render-contract.spec.ts b/apps/desktop/e2e/session-rail-render-contract.spec.ts index 72c9b66a45..7a4dbb34b8 100644 --- a/apps/desktop/e2e/session-rail-render-contract.spec.ts +++ b/apps/desktop/e2e/session-rail-render-contract.spec.ts @@ -17,7 +17,7 @@ * under the License. */ -import { expect } from '@playwright/test'; +import { expect, type Page } from '@playwright/test'; import { ensureSidebarExpanded, RAIL_RENDER_SESSION_COUNT, @@ -36,17 +36,67 @@ import { * ones not yet found, including anything that raises the number of commits a * switch produces (#4109). * - * The budget counts inline `style` writes on rail buttons because that is the + * The counter reads inline `style` writes on rail buttons because that is the * dominant term: every Astryx button removes and re-adds its `anchor-name` per * render, so one wasted rail render is two style writes per button plus the * style recalculation they force. * - * Measured on this fixture: 4 writes when the rail behaves — the leaving row - * and the arriving row, two each — against 336 with `setActiveId` restored to a - * per-render identity. The budget sits an order of magnitude clear of both, so - * it fails on a regression rather than on scheduling noise. + * The assertion that carries the contract is `rowsTouched`, not the total. + * Attributing each write to its row makes the budget independent of how many + * rows the fixture seeds, and closes the hole a total-only budget leaves: a + * regression that re-renders the whole rail exactly ONCE stays under any total + * generous enough not to flake, but it cannot touch two rows. That is also the + * missing middle of the fix's own claim — identity is fixed so `memo` holds, and + * `memo` holding means untouched rows produce no DOM work at all. + * + * `styleWrites > 0` is the counter's own liveness check. Every write counted + * here comes from an Astryx ref callback that is not wrapped in `useCallback`; + * if that upstream detail is ever memoised, both the healthy and the regressed + * reading collapse to zero and a one-sided budget would pass forever without + * ever failing again. + */ +const RAIL_ROWS_TOUCHED_BUDGET = 2; +/** The leaving row and the arriving row, two writes each, doubled for slack. */ +const RAIL_STYLE_WRITE_BUDGET = 8; + +interface RailCounters { + /** Cumulative since the last `resetRailCounters`; what the budgets read. */ + styleWrites: number; + rowIds: string[]; + rowRemounts: number; + /** Drained by every quiet poll, so it reports only the latest interval. */ + delta: number; +} + +type RailWindow = Window & { __railCounters: RailCounters }; + +/** + * Waits until the rail has been silent for ~300ms. + * + * A fixed `waitForTimeout` would be the only thing standing between a slow + * machine and a red run: `toHaveCount` proves the rows mounted, not that the + * commits behind them are done, and one late catalog refresh writes more than + * the whole budget. `retries` is 0 in `playwright.config.ts`, so that failure + * would land on an unrelated pull request. */ -const RAIL_STYLE_WRITE_BUDGET = 3 * RAIL_RENDER_SESSION_COUNT; +async function waitForRailQuiet(page: Page): Promise { + let quietPolls = 0; + await expect + .poll( + async () => { + const delta = await page.evaluate(() => { + const counters = (window as unknown as RailWindow).__railCounters; + const seen = counters.delta; + counters.delta = 0; + return seen; + }); + quietPolls = delta === 0 ? quietPolls + 1 : 0; + return quietPolls; + }, + { timeout: 15_000, intervals: [100] }, + ) + .toBeGreaterThanOrEqual(3); +} test('switching sessions does not rewrite the whole Session rail', async ({ railRenderWindow: page, @@ -62,43 +112,69 @@ test('switching sessions does not rewrite the whole Session rail', async ({ const selected = page.locator('.maka-session-row button.astryx-side-nav-item.selected'); await expect(target).toBeVisible(); - // Settle first: the budget is about a switch, not about arriving. - await page.waitForTimeout(500); - await page.evaluate(() => { - const counter = { styleWrites: 0 }; - (window as unknown as { __railWrites: typeof counter }).__railWrites = counter; + const counters = { styleWrites: 0, rowIds: [] as string[], rowRemounts: 0, delta: 0 }; + (window as unknown as { __railCounters: typeof counters }).__railCounters = counters; const observer = new MutationObserver((records) => { for (const record of records) { - const target = record.target as Element; - if (!target.closest?.('.maka-session-row')) continue; - counter.styleWrites += 1; + if (record.type === 'childList') { + for (const node of record.addedNodes) { + const element = node as Element; + if (element.nodeType !== 1) continue; + // A row that unmounts and remounts writes its `anchor-name` once + // on the way in, from a ref callback that runs AFTER insertion — + // so an attribute-only counter reads a whole rail remount as + // cheaper than a rail re-render. Count the remounts directly. + if (element.classList?.contains('maka-session-row')) counters.rowRemounts += 1; + } + continue; + } + const row = (record.target as Element).closest?.('.maka-session-row'); + if (!row) continue; + counters.styleWrites += 1; + counters.delta += 1; + const rowId = row.getAttribute('data-session-id'); + if (rowId && !counters.rowIds.includes(rowId)) counters.rowIds.push(rowId); } }); observer.observe(document.body, { subtree: true, + childList: true, attributes: true, attributeFilter: ['style'], }); (window as unknown as { __railObserver: MutationObserver }).__railObserver = observer; }); + // Settle first: the budget is about a switch, not about arriving. + await waitForRailQuiet(page); + await page.evaluate(() => { + const counters = (window as unknown as RailWindow).__railCounters; + counters.styleWrites = 0; + counters.rowIds = []; + counters.rowRemounts = 0; + counters.delta = 0; + }); + await target.click(); await expect(selected).toHaveText(/Rail row 3/); - // Let the post-switch commit cascade finish before reading the counter. - await page.waitForTimeout(1500); + // Let the post-switch commit cascade finish before reading the counters. + await waitForRailQuiet(page); - const styleWrites = await page.evaluate(() => { - const scope = window as unknown as { - __railWrites: { styleWrites: number }; - __railObserver: MutationObserver; - }; + const counted = await page.evaluate(() => { + const scope = window as unknown as RailWindow & { __railObserver: MutationObserver }; scope.__railObserver.disconnect(); - return scope.__railWrites.styleWrites; + const { styleWrites, rowIds, rowRemounts } = scope.__railCounters; + return { styleWrites, rowIds, rowRemounts }; }); expect( - styleWrites, - `rail inline-style writes for one session switch (${RAIL_RENDER_SESSION_COUNT} rows)`, - ).toBeLessThanOrEqual(RAIL_STYLE_WRITE_BUDGET); + counted.rowIds.length, + `rail rows touched by one session switch, of ${RAIL_RENDER_SESSION_COUNT} (${counted.rowIds.join(', ')})`, + ).toBeLessThanOrEqual(RAIL_ROWS_TOUCHED_BUDGET); + expect(counted.rowRemounts, 'rail rows remounted by one session switch').toBe(0); + expect(counted.styleWrites, 'the style-write counter never fired').toBeGreaterThan(0); + expect(counted.styleWrites, 'rail inline-style writes for one session switch').toBeLessThanOrEqual( + RAIL_STYLE_WRITE_BUDGET, + ); }); diff --git a/apps/desktop/src/main/__tests__/session-workspace-action-identity.test.ts b/apps/desktop/src/main/__tests__/session-workspace-action-identity.test.ts index 19d1c56a7d..baa6e161ef 100644 --- a/apps/desktop/src/main/__tests__/session-workspace-action-identity.test.ts +++ b/apps/desktop/src/main/__tests__/session-workspace-action-identity.test.ts @@ -33,22 +33,20 @@ import { useAppShellSessionWorkspace } from '../../renderer/use-app-shell-sessio * for a single session switch. Identity is a contract, not an implementation * detail, so it is asserted here rather than left to review. */ -const ACTION_KEYS = [ - 'setActiveId', - 'startNewSession', - 'clearOwnedSessionState', - 'setMessages', - 'addTransientMessage', - 'updateTransientMessage', - 'projectQueuedTransientMessages', - 'retireCancelledTransientMessages', - 'removeTransientMessage', - 'refreshSessions', - 'seedSessions', -] as const; - type Workspace = ReturnType; +/** + * Every function the hook returns, read off the first render rather than + * listed here. A hand-kept list covers what someone remembered on the day it + * was written and silently stops covering whatever is added later, which is + * the opposite of what a contract test is for. + */ +function actionKeys(workspace: Workspace): string[] { + return Object.keys(workspace).filter( + (key) => typeof (workspace as Record)[key] === 'function', + ); +} + describe('session workspace action identity', () => { afterEach(cleanupFakeDom); @@ -75,11 +73,15 @@ describe('session workspace action identity', () => { assert.ok(reads.length > 1, 'the probe should have re-rendered'); const first = reads[0]!; - for (const key of ACTION_KEYS) { + const keys = actionKeys(first); + // A guard on the guard: if the hook's shape ever collapses, the loop below + // would pass by having nothing to check. + assert.ok(keys.length > 10, `expected the workspace to expose actions, saw ${keys.length}`); + for (const key of keys) { for (const later of reads.slice(1)) { assert.equal( - later[key], - first[key], + (later as Record)[key], + (first as Record)[key], `${key} changed identity between renders`, ); } From 1ba5aa347f7419444c0d1fd92c1c0270aeccc2e1 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Sat, 29 Aug 2026 11:02:08 +0800 Subject: [PATCH 5/6] refactor(desktop): reuse the session workspace's MessageListUpdater Extracting the workspace actions gave this type an owner and an export. Leaving the three local copies in place would have made the PR that merged one duplicate authority create another. Generated-by: Claude Code --- apps/desktop/src/renderer/app-shell-chat-actions.ts | 2 +- apps/desktop/src/renderer/app-shell-revision-actions.ts | 4 +--- apps/desktop/src/renderer/app-shell-turn-actions.ts | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/apps/desktop/src/renderer/app-shell-chat-actions.ts b/apps/desktop/src/renderer/app-shell-chat-actions.ts index 3c501053b3..fedf80b37e 100644 --- a/apps/desktop/src/renderer/app-shell-chat-actions.ts +++ b/apps/desktop/src/renderer/app-shell-chat-actions.ts @@ -67,6 +67,7 @@ import { noRealConnectionSetupDescription, } from './model-connection-errors.js'; import type { RefreshMessagesOptions } from './session-message-settlement.js'; +import type { MessageListUpdater } from './session-workspace-actions.js'; export type { RefreshMessagesOptions }; @@ -81,7 +82,6 @@ type BooleanRecordUpdater = (updater: (current: Record) => Reco type LiveTurnRecordUpdater = ( updater: (current: Record) => Record, ) => void; -type MessageListUpdater = (next: StoredMessage[] | ((current: StoredMessage[]) => StoredMessage[])) => void; type MessageLoadErrorUpdater = (updater: (current: Record) => Record) => void; type InteractionQueueUpdater = (updater: (current: InteractionQueues) => InteractionQueues) => void; diff --git a/apps/desktop/src/renderer/app-shell-revision-actions.ts b/apps/desktop/src/renderer/app-shell-revision-actions.ts index 7bd256fb07..18b722ef7a 100644 --- a/apps/desktop/src/renderer/app-shell-revision-actions.ts +++ b/apps/desktop/src/renderer/app-shell-revision-actions.ts @@ -37,11 +37,9 @@ import { type SessionCopyAttemptKey, } from './session-copy-attempt.js'; import { readSettledMessages } from './session-message-settlement.js'; +import type { MessageListUpdater } from './session-workspace-actions.js'; type RefBox = { current: T }; -type MessageListUpdater = ( - next: StoredMessage[] | ((current: StoredMessage[]) => StoredMessage[]), -) => void; type ToastApi = { info(title: string, description?: string): void; diff --git a/apps/desktop/src/renderer/app-shell-turn-actions.ts b/apps/desktop/src/renderer/app-shell-turn-actions.ts index 1c594d0cc5..80ac9e0a11 100644 --- a/apps/desktop/src/renderer/app-shell-turn-actions.ts +++ b/apps/desktop/src/renderer/app-shell-turn-actions.ts @@ -28,9 +28,9 @@ import { showSessionWorkspaceUnavailableToast, } from './session-workspace-errors.js'; import { acquireSessionCopyAttempt } from './session-copy-attempt.js'; +import type { MessageListUpdater } from './session-workspace-actions.js'; type RefBox = { current: T }; -type MessageListUpdater = (next: StoredMessage[] | ((current: StoredMessage[]) => StoredMessage[])) => void; type ToastApi = { info(title: string, description?: string): void; From 35c0ffdfcfa92ebb54c41d480644d41efec94a2e Mon Sep 17 00:00:00 2001 From: AstroHan Date: Sat, 29 Aug 2026 11:02:08 +0800 Subject: [PATCH 6/6] refactor(ui): read absolute timestamps from their core authority MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Once the implementation moved to `@maka/core/relative-time`, the export left behind in `chat-display-helpers` held nothing — it was a second name for the same function, and `relative-time.tsx` reached the one module through both names at once. Drift is prevented by there being a single implementation, not by which file the callers name. `formatAbsoluteTimestamp` is not in the package's public exports, so this moves three imports and removes a concept without changing a contract. Generated-by: Claude Code --- packages/ui/src/chat-display-helpers.ts | 11 +++++------ packages/ui/src/chat-turn.tsx | 7 ++----- packages/ui/src/relative-time.tsx | 2 +- packages/ui/src/session-history-list.tsx | 2 +- 4 files changed, 9 insertions(+), 13 deletions(-) diff --git a/packages/ui/src/chat-display-helpers.ts b/packages/ui/src/chat-display-helpers.ts index c4d113e357..fe3b64255d 100644 --- a/packages/ui/src/chat-display-helpers.ts +++ b/packages/ui/src/chat-display-helpers.ts @@ -43,12 +43,11 @@ import type { UiLocale } from '@maka/core/ui-locale'; import { getConversationCopy } from './conversation-copy.js'; -/* `formatAbsoluteTimestamp` used to be a second copy of the same `Intl` options - `@maka/core/relative-time` already owned, and it built a formatter per call — - the sidebar renders one per row for the row tooltip and one for the row's - accessible name. Re-exported rather than reimplemented so the two readings of - a timestamp cannot drift apart. */ -export { formatAbsoluteTimestamp } from '@maka/core/relative-time'; +/* `formatAbsoluteTimestamp` used to live here as a second copy of the same + `Intl` options `@maka/core/relative-time` already owned, and it built a + formatter per call — the sidebar renders one per row for the row tooltip and + one for the row's accessible name. Its callers now read the core module + directly, so there is no second reading of a timestamp to keep in step. */ /* `formatClockTime` (a 24-hour `HH:mm` for the user-message time) lived here until the meta row moved to Astryx's `Timestamp`. Locking the hour cycle was diff --git a/packages/ui/src/chat-turn.tsx b/packages/ui/src/chat-turn.tsx index a3fd9fa896..a0253207c1 100644 --- a/packages/ui/src/chat-turn.tsx +++ b/packages/ui/src/chat-turn.tsx @@ -22,11 +22,8 @@ import { useMountedRef } from './use-mounted-ref.js'; import { ICON_SIZE, Ban, Check, Copy, GitBranch, Info, Pencil, RefreshCcw, Timer } from './icons.js'; import { type ClipboardCopyPhase, useClipboardCopyFeedback } from './clipboard-feedback.js'; import { Markdown } from './markdown.js'; -import { - formatAbsoluteTimestamp, - formatTurnDuration, - turnAbortStatusLabel, -} from './chat-display-helpers.js'; +import { formatTurnDuration, turnAbortStatusLabel } from './chat-display-helpers.js'; +import { formatAbsoluteTimestamp } from '@maka/core/relative-time'; import { isTimeDrivenMotionEnabled } from './streaming-presentation.js'; import { computerRunningLabel } from './tool-activity/computer-action-label.js'; import { diff --git a/packages/ui/src/relative-time.tsx b/packages/ui/src/relative-time.tsx index 11267e79bc..836c3df895 100644 --- a/packages/ui/src/relative-time.tsx +++ b/packages/ui/src/relative-time.tsx @@ -18,8 +18,8 @@ */ import { useEffect, useState } from 'react'; -import { formatAbsoluteTimestamp } from './chat-display-helpers.js'; import { + formatAbsoluteTimestamp, formatRelativeTimestamp, nextRelativeRefreshDelay, formatSidebarTimestamp, diff --git a/packages/ui/src/session-history-list.tsx b/packages/ui/src/session-history-list.tsx index ab65178b36..dccdd1cbb6 100644 --- a/packages/ui/src/session-history-list.tsx +++ b/packages/ui/src/session-history-list.tsx @@ -44,7 +44,7 @@ import { Trash2, } from './icons.js'; import { RelativeTime } from './relative-time.js'; -import { formatAbsoluteTimestamp } from './chat-display-helpers.js'; +import { formatAbsoluteTimestamp } from '@maka/core/relative-time'; import { Badge } from '@astryxdesign/core/Badge'; import { MoreMenu } from '@astryxdesign/core/MoreMenu'; import {