From 778f51d2e3152642ea4da5529ebecfecbc194ac0 Mon Sep 17 00:00:00 2001 From: Xiao Liu Date: Fri, 4 Sep 2026 05:37:38 +0800 Subject: [PATCH] fix(desktop): show side-conversation message + progress immediately MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The side conversation (quote-companion side chat) gave no immediate feedback on send: it rendered the user's message and the running-status line only after the turn settled, and — because it forks lazily — nothing at all during the first send's fork round trip. - Render the user's message optimistically on send (transientMessages), matching the main conversation, instead of only after the turn completes. The bubble is armed before the fork exists, so a cold first send is not blank. - Render that optimistic content even before a session exists: ChatView's no-activeSession branch now shows transientMessages + the running-status line, so the first question and progress cue appear during the lazy fork creation (they were previously dropped until the fork committed). - Drive the running-status line from `streaming || transientMessages.length > 0` with the same rising-edge delay as the main chat. The admission (and the Composer's Stop button) is armed only in onBeforeSend, once the fork exists and stop() can act on it — so Stop never appears while it would be a no-op. - Wait for the just-sent user message to be durable before the settled read so the turn materializes promptly. The Host mints the message id from the turn id, so one identity gates every path. Per-session workbar collapse (item 3 of #4654) is deferred to its own issue (#4693): it needs a decision on whether the collapse preference persists across restart, and should hold the session key in the layout reducer. Refs #4654 Generated-by: Claude Code --- apps/desktop/e2e/slash-command-menu.spec.ts | 7 +- .../chat-view-optimistic-render.test.ts | 96 ++++++++++++++ .../__tests__/quote-companion-retry.test.ts | 79 ++++++++++++ .../tools/side-chat/quote-companion-panel.tsx | 40 +++++- .../tools/side-chat/use-quote-companion.ts | 121 +++++++++++++++--- packages/ui/src/chat-view.tsx | 48 ++++++- 6 files changed, 364 insertions(+), 27 deletions(-) create mode 100644 apps/desktop/src/main/__tests__/chat-view-optimistic-render.test.ts diff --git a/apps/desktop/e2e/slash-command-menu.spec.ts b/apps/desktop/e2e/slash-command-menu.spec.ts index d78016aa9b..9f0210ce8d 100644 --- a/apps/desktop/e2e/slash-command-menu.spec.ts +++ b/apps/desktop/e2e/slash-command-menu.spec.ts @@ -233,7 +233,12 @@ test('dispatches /side instead of steering it into a running turn', async ({ await composer.press('Enter'); await expect(page.locator('.maka-quote-workbar-panel')).toHaveCount(1); - await page.getByRole('button', { name: '停止' }).click(); + // The side conversation now shows its own running/停止 state the instant it is + // dispatched, so a bare getByRole('停止') matches two buttons (the held-open + // main turn AND the side turn). Scope to the main column — the side panel + // lives in the sibling WorkbarHost — and stop the reliably-open main turn (the + // side turn's short prompt settles too fast to click deterministically). + await page.locator('.mainColumn').getByRole('button', { name: '停止' }).click(); }); test('an open menu keeps its container and skills group across projection refreshes', async ({ diff --git a/apps/desktop/src/main/__tests__/chat-view-optimistic-render.test.ts b/apps/desktop/src/main/__tests__/chat-view-optimistic-render.test.ts new file mode 100644 index 0000000000..bf71bb0372 --- /dev/null +++ b/apps/desktop/src/main/__tests__/chat-view-optimistic-render.test.ts @@ -0,0 +1,96 @@ +/* + * 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 assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { type ComponentProps, createElement } from 'react'; +import { renderToStaticMarkup } from 'react-dom/server'; +import { + AstryxLocaleProvider, + ChatSurfaceLayout, + ChatView, + LocaleProvider, + type TransientUserMessageProjection, +} from '@maka/ui'; + +// A side conversation forks lazily: its first send arms the optimistic bubble +// (and, after the delay, the running-status line) BEFORE the fork commits, so +// `activeSession` is still undefined. These tests pin that `ChatView` renders +// that optimistic content in its no-session branch — the render-layer half of +// #4654 that the hook-only tests could not prove. The panel wires it up: +// `activeSession={companion.companionSession}` (undefined pre-fork) and +// `transientMessages`/`runningStatus` from the same hook. +function renderNoSessionChatView( + props: Partial>, +): string { + const view = createElement(ChatView, { + messages: [], + activeSession: undefined, + onNew: () => {}, + // A marker standing in for the empty-state content a caller supplies (the + // side panel's placeholder, the main chat's onboarding surface / hero). It + // must render when there is no optimistic content, and be suppressed when a + // bubble/running turn takes over — the ChatMessageList shows `emptyState` + // only while it has no children, so empty optimistic fragments must not + // count as children (regression: onboarding stopped rendering otherwise). + emptyOverride: createElement('div', { 'data-testid': 'empty-state-marker' }), + ...props, + } as ComponentProps); + const layout = createElement(ChatSurfaceLayout, { + scrollOwner: 'host', + composer: null, + children: view, + }); + const astryx = createElement(AstryxLocaleProvider, { children: layout }); + return renderToStaticMarkup( + createElement(LocaleProvider, { locale: 'en', children: astryx }), + ); +} + +const OPTIMISTIC_BUBBLE: TransientUserMessageProjection = { + id: 'turn-1', + text: 'why does this fail?', + ts: 1, + transientPlacement: 'current_turn', +}; + +test('ChatView renders the optimistic bubble and running status before a session exists', () => { + const markup = renderNoSessionChatView({ + transientMessages: [OPTIMISTIC_BUBBLE], + runningStatus: true, + }); + // The user's question is on screen immediately, before the fork/session lands. + assert.match(markup, /why does this fail\?/); + // The running-status line rides alongside it (the no-turn bare-turn fallback). + assert.match(markup, /data-live-streaming="true"/); + // The optimistic content takes over from the empty state. + assert.doesNotMatch(markup, /empty-state-marker/); +}); + +test('ChatView shows the empty state when there is neither a bubble nor a running turn', () => { + const markup = renderNoSessionChatView({ + transientMessages: [], + runningStatus: false, + }); + assert.doesNotMatch(markup, /why does this fail\?/); + assert.doesNotMatch(markup, /data-live-streaming="true"/); + // The empty state (onboarding surface / hero) must still render — the empty + // optimistic fragments must not suppress it. + assert.match(markup, /empty-state-marker/); +}); diff --git a/apps/desktop/src/main/__tests__/quote-companion-retry.test.ts b/apps/desktop/src/main/__tests__/quote-companion-retry.test.ts index cb7dbce1e9..00e7a3214a 100644 --- a/apps/desktop/src/main/__tests__/quote-companion-retry.test.ts +++ b/apps/desktop/src/main/__tests__/quote-companion-retry.test.ts @@ -376,6 +376,83 @@ test('first send after a completed turn forks through the settled turn', async ( assert.equal(probe.getAttribute('data-error'), ''); }); +test('a first send shows the question bubble immediately but arms Stop only once the fork exists', async () => { + // `branchFromTurn` is the Host round trip a first send waits on. Holding it + // open lets us observe the panel while the fork is still being created. + const branch = deferred<{ ok: true; session: SessionSummary }>(); + const rendered = await renderOwnershipProbe({ + listTurns: async () => [settledTurn('done-turn')], + branchFromTurn: () => branch.promise, + send: async () => ({ ok: true as const, turnId: 'first-turn' }), + }); + const probe = rendered.container.firstElementChild; + assert.ok(probe); + // Nothing sent yet: no fork, no bubble, not streaming. + assert.equal(probe.getAttribute('data-companion-id'), ''); + assert.equal(probe.getAttribute('data-transient-count'), '0'); + assert.equal(probe.getAttribute('data-streaming'), 'false'); + + // Kick off the send but leave fork creation pending (branch unresolved). + let sendResult!: Promise; + await act(async () => { + sendResult = rendered.send('why does this fail?'); + await Promise.resolve(); + }); + await waitUntil(() => probe.getAttribute('data-transient-count') === '1'); + + // The fork has NOT committed yet, but the question bubble is already on screen + // — the instant feedback #4654 asked for, and what the panel's running-status + // line rides on (`streaming || transientMessages.length > 0`) before a turn + // exists. Crucially `streaming` is still false, so the Composer does NOT render + // a Stop button during the window where `stop()` is a no-op (companionIdRef is + // only set at commitFork). Arming the admission early would show a dead Stop. + assert.equal(probe.getAttribute('data-companion-id'), ''); + assert.equal(probe.getAttribute('data-transient-text'), 'why does this fail?'); + assert.equal(probe.getAttribute('data-streaming'), 'false'); + + // Once the fork commits and the send goes in flight, the admission arms: + // streaming turns true, so Stop appears exactly when it can act on the turn. + await act(async () => { + branch.resolve({ ok: true as const, session: session('side-conversation') }); + assert.equal(await sendResult, true); + await Promise.resolve(); + }); + await awaitCompanion(rendered.container); + await waitUntil(() => probe.getAttribute('data-streaming') === 'true'); + assert.equal(probe.getAttribute('data-error'), ''); + assert.equal(probe.getAttribute('data-transient-count'), '1'); + + // The running state rides the whole turn and only retires on completion. + await act(async () => { + rendered.emit(completeEvent('c1', 'first-turn', 2)); + await Promise.resolve(); + }); + await waitUntil(() => probe.getAttribute('data-streaming') === 'false'); +}); + +test('a failed first send retires the optimistic bubble without ever arming Stop', async () => { + // The fork never materializes: `branchFromTurn` throws. The optimistic bubble + // must be unwound so nothing is stranded with no turn to reconcile it away, and + // Stop must never have appeared (the admission is armed only in onBeforeSend). + const rendered = await renderOwnershipProbe({ + listTurns: async () => [settledTurn('done-turn')], + branchFromTurn: async () => { + throw new Error('fork setup exploded'); + }, + }); + const probe = rendered.container.firstElementChild; + assert.ok(probe); + + await act(async () => { + assert.equal(await rendered.send('why does this fail?'), false); + await Promise.resolve(); + }); + assert.equal(probe.getAttribute('data-companion-id'), ''); + assert.equal(probe.getAttribute('data-transient-count'), '0'); + assert.equal(probe.getAttribute('data-streaming'), 'false'); + assert.equal(probe.getAttribute('data-live-turn-id'), ''); +}); + test('dispatches /compact to the committed companion fork without sending model input', async () => { const compactCalls: string[] = []; let sendCalls = 0; @@ -1968,6 +2045,8 @@ function QuoteCompanionOwnershipProbe(props: { 'data-processing': String(companion.processing), 'data-model-ready': String(companion.modelReady), 'data-permission-mode': companion.permissionMode ?? '', + 'data-transient-count': String(companion.transientMessages.length), + 'data-transient-text': companion.transientMessages[0]?.text ?? '', }); } diff --git a/apps/desktop/src/renderer/features/workbar/tools/side-chat/quote-companion-panel.tsx b/apps/desktop/src/renderer/features/workbar/tools/side-chat/quote-companion-panel.tsx index feeb77b8cf..0e9f30963a 100644 --- a/apps/desktop/src/renderer/features/workbar/tools/side-chat/quote-companion-panel.tsx +++ b/apps/desktop/src/renderer/features/workbar/tools/side-chat/quote-companion-panel.tsx @@ -17,7 +17,7 @@ * under the License. */ -import { useCallback, useEffect, useRef, type ComponentProps } from 'react'; +import { useCallback, useEffect, useRef, useState, type ComponentProps } from 'react'; import { Banner } from '@astryxdesign/core/Banner'; import { ChatView, @@ -57,6 +57,28 @@ import type { CompanionForkVisibilityEvent } from './quote-companion-visibility' import { readScrollMotionBehavior } from '../../../../scroll-motion-policy'; import { useWorkbarServices } from '../../services-context.js'; +const RUNNING_STATUS_DELAY_MS = 200; + +/** + * A boolean that turns true only after `condition` has held for `delayMs`, and + * false the moment it drops — the rising-edge delay that keeps a fast turn from + * flashing the running-status line. A feature-local copy of the shell's + * useDelayedFlag: the renderer-legacy original is walled off from feature code + * by the architecture budget, and this is only a few lines of timer plumbing. + */ +function useDelayedFlag(condition: boolean, delayMs: number): boolean { + const [visible, setVisible] = useState(false); + useEffect(() => { + if (!condition) { + setVisible(false); + return; + } + const handle = window.setTimeout(() => setVisible(true), delayMs); + return () => window.clearTimeout(handle); + }, [condition, delayMs]); + return visible; +} + /** * The side-conversation workbar tab: a transient read-only fork of the main session. * It renders with the SAME surface as the main conversation — the real @@ -169,6 +191,19 @@ export function QuoteCompanionPanel(props: { useEffect(() => { props.onContentStateChange?.(props.panelId, companion.hasContent); }, [companion.hasContent, props.onContentStateChange, props.panelId]); + // The transcript's running-status line ("正在琢磨… · Ns"). Like the main chat + // (useShellLiveTurn → showRunningStatus) it rides the whole active turn, not + // just the pre-first-token wait, with the same rising-edge delay so a fast + // turn never flashes it. The companion's `processing` only covers the wait + // window, which is why the side panel used to show almost no progress cue. + // `transientMessages` covers the first-send window BEFORE the fork commits and + // the admission is armed: the optimistic bubble is on screen but `streaming` + // is still false, and the cue must already be up (the admission is deliberately + // armed late so the Stop button never appears before `stop()` can act on it). + const showRunningStatus = useDelayedFlag( + companion.streaming || companion.transientMessages.length > 0, + RUNNING_STATUS_DELAY_MS, + ); useEffect(() => { props.onActivityStateChange?.( props.panelId, @@ -371,9 +406,10 @@ export function QuoteCompanionPanel(props: { >