diff --git a/apps/desktop/src/renderer/settings/general-settings-page.tsx b/apps/desktop/src/renderer/settings/general-settings-page.tsx index 1bf53b36d9..64ce616df8 100644 --- a/apps/desktop/src/renderer/settings/general-settings-page.tsx +++ b/apps/desktop/src/renderer/settings/general-settings-page.tsx @@ -667,7 +667,6 @@ function GeneralDefaultsCard(props: { renderProviderMark={(type) => } ariaLabel={copy.defaultModel} disabled={saving || !props.connectionsInteractive} - loading={saving} triggerClassName="settingsModelPickerTrigger" onValueChange={persistDefault} /> diff --git a/packages/ui/src/__tests__/use-pending-selection.test.tsx b/packages/ui/src/__tests__/use-pending-selection.test.tsx new file mode 100644 index 0000000000..3a4b4321ff --- /dev/null +++ b/packages/ui/src/__tests__/use-pending-selection.test.tsx @@ -0,0 +1,141 @@ +/* + * 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 { act, createElement } from 'react'; +import { createRoot } from 'react-dom/client'; +import { parseHTML } from 'linkedom'; +import { usePendingSelection } from '../use-pending-selection.js'; + +interface Deferred { + resolve(): void; + reject(): void; +} + +interface Harness { + value(): string; + pick(next: string): Promise; + render(authoritative: string): Promise; + settle(index?: number): Promise; + reject(index?: number): Promise; +} + +const flush = () => new Promise((r) => setTimeout(r, 0)); + +async function mount(initial: string): Promise { + const { document, window } = parseHTML('
'); + Object.assign(globalThis, { + document, + window, + IS_REACT_ACT_ENVIRONMENT: true, + }); + const container = document.querySelector('#root'); + assert.ok(container); + + const writes: Deferred[] = []; + let handle: { value: string; onChange: (n: string) => void } | null = null; + const onValueChange = (_next: string) => + new Promise((resolve, reject) => { + writes.push({ resolve, reject }); + }); + + function Host({ authoritative }: { authoritative: string }) { + handle = usePendingSelection(authoritative, onValueChange); + return null; + } + + const root = createRoot(container as unknown as Element); + await act(() => { + root.render(createElement(Host, { authoritative: initial })); + }); + + return { + value: () => handle!.value, + pick: async (next) => { + await act(async () => { + handle!.onChange(next); + await flush(); + }); + }, + render: async (authoritative) => { + await act(() => { + root.render(createElement(Host, { authoritative })); + }); + }, + settle: async (index = writes.length - 1) => { + await act(async () => { + writes[index]!.resolve(); + await flush(); + }); + }, + reject: async (index = writes.length - 1) => { + await act(async () => { + writes[index]!.reject(); + await flush(); + }); + }, + }; +} + +test('a pick shows immediately, before the write settles', async () => { + const h = await mount('A'); + assert.equal(h.value(), 'A'); + await h.pick('B'); + assert.equal(h.value(), 'B'); +}); + +test('the pick clears to authority once the write resolves and value catches up', async () => { + const h = await mount('A'); + await h.pick('B'); + await h.render('B'); // caller's refresh lands the new authority + await h.settle(); + assert.equal(h.value(), 'B'); +}); + +test('a rejected write rolls back to the authoritative value', async () => { + const h = await mount('A'); + await h.pick('B'); + assert.equal(h.value(), 'B'); + await h.reject(); + assert.equal(h.value(), 'A'); +}); + +test('the pick holds across an unrelated authority change while the write is in flight', async () => { + const h = await mount('A'); + await h.pick('B'); + // An unrelated event pushes a different authoritative value mid-write; the + // user's pick still shows until their own write settles. + await h.render('C'); + assert.equal(h.value(), 'B'); + await h.settle(); + assert.equal(h.value(), 'C'); +}); + +test('latest pick wins: a slower earlier write settling does not wipe a newer pick', async () => { + const h = await mount('A'); + await h.pick('B'); // write #0 + await h.pick('C'); // write #1 (newer) + assert.equal(h.value(), 'C'); + await h.settle(0); // the older B write resolves late + assert.equal(h.value(), 'C'); // still C, not cleared + await h.render('C'); + await h.settle(1); + assert.equal(h.value(), 'C'); +}); diff --git a/packages/ui/src/model-picker.tsx b/packages/ui/src/model-picker.tsx index df29709df4..764c15d865 100644 --- a/packages/ui/src/model-picker.tsx +++ b/packages/ui/src/model-picker.tsx @@ -44,6 +44,7 @@ import { } from './model-picker-internals.js'; import { useUiLocale } from './locale-context.js'; import { getSharedUiCopy } from './shared-ui-copy.js'; +import { usePendingSelection } from './use-pending-selection.js'; export interface ModelPickerProps { groups: readonly ModelMenuGroup[]; @@ -51,7 +52,6 @@ export interface ModelPickerProps { onValueChange(value: string): void | Promise; renderProviderMark?(type: ProviderType): ReactNode; disabled?: boolean; - loading?: boolean; /** * An ordinary option placed before the catalog for product values such as * “not set” or a current model that is no longer listed. Astryx search treats @@ -80,6 +80,10 @@ export function ModelPicker(props: ModelPickerProps) { [locale, props.groups], ); + // Reflect the pick immediately and hold it until the caller's write settles, + // then defer to the authoritative `value`. See usePendingSelection. + const selection = usePendingSelection(props.value, props.onValueChange); + // size=md matches the other settings-row selectors. Settings is the only // production host since the composer footer moved to ghost DropdownMenus, // so the size is a fact of the component, not a prop. @@ -89,15 +93,20 @@ export function ModelPicker(props: ModelPickerProps) { label={props.ariaLabel} isLabelHidden options={options} - value={props.value} + value={selection.value} hasSearch searchPlaceholder={props.searchPlaceholder ?? copy.searchPlaceholder} size="md" placement="above" isDisabled={props.disabled} - isLoading={props.loading} className={props.triggerClassName} - changeAction={props.onValueChange} + // `onChange`, not `changeAction`: the async `changeAction` path wraps + // the caller's save in a transition and spins the trigger (Astryx's + // built-in optimistic `isBusy`) for the whole round-trip. On the + // fire-and-forget `onChange` path the trigger never enters that busy + // state; usePendingSelection shows the pick at once and settles it when + // the write finishes. + onChange={selection.onChange} renderOption={(option: SelectorOptionData) => { const providerType = providerTypes.get(option.value); const providerMark = diff --git a/packages/ui/src/use-pending-selection.ts b/packages/ui/src/use-pending-selection.ts new file mode 100644 index 0000000000..c067045661 --- /dev/null +++ b/packages/ui/src/use-pending-selection.ts @@ -0,0 +1,76 @@ +/* + * 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 { useCallback, useRef, useState } from 'react'; + +export interface PendingSelection { + /** + * The value to render: the just-picked value while its write is unsettled, + * otherwise the authoritative value. + */ + value: string; + /** + * Show `next` at once and fire the write; the pick clears when that write + * settles — by when the authoritative `value` has caught up on success, or + * falling back to it on failure. + */ + onChange(next: string): void; +} + +/** + * Reflect a just-picked value immediately and hold it until the caller's write + * settles, then defer to the authoritative `value`. No spinner and no lag: the + * pick shows from the click and clears the moment `onValueChange` resolves (by + * when `value` has caught up) or rejects (rolling back to `value`). + * + * A monotonic token makes the latest pick win, so a slower earlier write's + * settle cannot wipe a newer pick. The state is deliberately local and + * write-scoped: it carries no cross-read generation and no state that outlives + * the component, so reopening the surface always starts clean. + * + * Limitation: the pick clears when the write settles, not when `authoritative` + * is confirmed to carry it — so if the write resolves but the caller never + * lands the new value into `authoritative` (e.g. its refresh is silently + * dropped), the trigger falls back to the prior `authoritative` until the + * caller next updates it, self-healing on that next update and never wrong + * durably. + */ +export function usePendingSelection( + authoritative: string, + onValueChange: (next: string) => void | Promise, +): PendingSelection { + const [pending, setPending] = useState(null); + const tokenRef = useRef(0); + const onChange = useCallback( + (next: string) => { + const token = (tokenRef.current += 1); + setPending(next); + // Clear on either outcome — success (authoritative caught up) or failure + // (roll back to authoritative) — and only if this is still the latest + // pick. Two-arg `then` (not `finally`) so a rejected write is consumed + // here rather than surfacing as an unhandled rejection. + const settle = () => { + if (tokenRef.current === token) setPending(null); + }; + Promise.resolve(onValueChange(next)).then(settle, settle); + }, + [onValueChange], + ); + return { value: pending ?? authoritative, onChange }; +} diff --git a/packages/ui/stories/model-picker.stories.tsx b/packages/ui/stories/model-picker.stories.tsx index 7774685f20..d7ac2758da 100644 --- a/packages/ui/stories/model-picker.stories.tsx +++ b/packages/ui/stories/model-picker.stories.tsx @@ -26,8 +26,6 @@ import type { SessionSummary } from '@maka/core/session'; import { ChatModelSwitcher, ModelChipStatic, NewChatModelPicker, ThinkingLevelSelector } from '../src/chat-model-switcher.js'; import { exactModelChoiceValue, - modelChoiceValue, - modelMenuGroups, type ChatModelChoice, } from '../src/chat-model-helpers.js'; import { ModelPicker } from '../src/model-picker.js'; @@ -344,30 +342,6 @@ export const ThinkingLevelSeparate: Story = { }, }; -// Real path: Settings → 通用 → default model, while the just-picked model is -// being saved. Production drives `ModelPicker.loading` from the save in flight -// (general-settings-page.tsx `loading={saving}`), with the catalog present and -// the row disabled — not an empty catalog. (When the catalog itself is -// unavailable the settings row renders a skeleton, which is a different -// component, so that is not modelled here.) -export const SavingDefaultModel: Story = { - render: () => ( -
- {}} - /> -
- ), -}; - // Real path: composer left footer when no connection yields a usable model — // what a failed / offline / unauthorised catalog fetch all collapse to. The // picker cannot exist without choices, so the composer swaps in an honest