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..7a4dbb34b8 --- /dev/null +++ b/apps/desktop/e2e/session-rail-render-contract.spec.ts @@ -0,0 +1,180 @@ +/* + * 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, type Page } 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 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. + * + * 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. + */ +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, +}) => { + 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(); + + await page.evaluate(() => { + 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) { + 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 counters. + await waitForRailQuiet(page); + + const counted = await page.evaluate(() => { + const scope = window as unknown as RailWindow & { __railObserver: MutationObserver }; + scope.__railObserver.disconnect(); + const { styleWrites, rowIds, rowRemounts } = scope.__railCounters; + return { styleWrites, rowIds, rowRemounts }; + }); + + expect( + 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 new file mode 100644 index 0000000000..baa6e161ef --- /dev/null +++ b/apps/desktop/src/main/__tests__/session-workspace-action-identity.test.ts @@ -0,0 +1,90 @@ +/* + * 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. + */ +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); + + 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]!; + 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 as Record)[key], + (first as Record)[key], + `${key} changed identity between renders`, + ); + } + } + }); +}); 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; 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, 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..fe3b64255d 100644 --- a/packages/ui/src/chat-display-helpers.ts +++ b/packages/ui/src/chat-display-helpers.ts @@ -40,22 +40,14 @@ * 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 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 {