fix: model picker stays usable during provider loading (#8900)
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Kalvin Chau <kalvin@block.xyz>
This commit is contained in:
@@ -75,40 +75,6 @@ export function useAppStartup() {
|
||||
}
|
||||
};
|
||||
|
||||
const refreshConfiguredProviderInventory = async (
|
||||
initialEntries?: Awaited<ReturnType<typeof loadProvidersAndInventory>>,
|
||||
) => {
|
||||
try {
|
||||
const entries =
|
||||
initialEntries && initialEntries.length > 0
|
||||
? initialEntries
|
||||
: await (async () => {
|
||||
const { getProviderInventory } = await import(
|
||||
"@/features/providers/api/inventory"
|
||||
);
|
||||
return getProviderInventory();
|
||||
})();
|
||||
const configuredProviderIds = entries
|
||||
.filter((entry) => entry.configured)
|
||||
.map((entry) => entry.providerId);
|
||||
if (configuredProviderIds.length === 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
const { syncProviderInventory } = await import(
|
||||
"@/features/providers/api/inventorySync"
|
||||
);
|
||||
await syncProviderInventory(configuredProviderIds, {
|
||||
onEntries: (entries) => inventoryStore.mergeEntries(entries),
|
||||
});
|
||||
} catch (err) {
|
||||
console.error(
|
||||
"Failed to refresh provider inventory on startup:",
|
||||
err,
|
||||
);
|
||||
}
|
||||
};
|
||||
|
||||
const loadSessionState = async () => {
|
||||
const t0 = performance.now();
|
||||
perfLog("[perf:startup] loadSessionState start");
|
||||
@@ -128,9 +94,19 @@ export function useAppStartup() {
|
||||
providersAndInventoryLoad,
|
||||
loadSessionState(),
|
||||
]);
|
||||
void providersAndInventoryLoad.then((entries) =>
|
||||
refreshConfiguredProviderInventory(entries),
|
||||
);
|
||||
void providersAndInventoryLoad.then(async (entries) => {
|
||||
try {
|
||||
const { backgroundRefreshInventory } = await import(
|
||||
"@/features/providers/api/inventory"
|
||||
);
|
||||
await backgroundRefreshInventory(inventoryStore, entries);
|
||||
} catch (err) {
|
||||
console.error(
|
||||
"Failed to refresh provider inventory on startup:",
|
||||
err,
|
||||
);
|
||||
}
|
||||
});
|
||||
perfLog(
|
||||
`[perf:startup] useAppStartup complete in ${(performance.now() - tStartup).toFixed(1)}ms`,
|
||||
);
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { useCallback, useMemo } from "react";
|
||||
import { useCallback, useMemo, useRef } from "react";
|
||||
import type { AcpProvider } from "@/shared/api/acp";
|
||||
import { useProviderInventory } from "@/features/providers/hooks/useProviderInventory";
|
||||
import { useProviderInventoryStore } from "@/features/providers/stores/providerInventoryStore";
|
||||
import {
|
||||
getCatalogEntry,
|
||||
resolveAgentProviderCatalogIdStrict,
|
||||
@@ -162,6 +163,24 @@ export function useAgentModelPickerState({
|
||||
[availableModels, onModelSelected],
|
||||
);
|
||||
|
||||
const refreshingRef = useRef(false);
|
||||
const handlePickerOpen = useCallback(() => {
|
||||
if (refreshingRef.current || useProviderInventoryStore.getState().loading) {
|
||||
return;
|
||||
}
|
||||
refreshingRef.current = true;
|
||||
import("@/features/providers/api/inventory")
|
||||
.then(({ backgroundRefreshInventory }) =>
|
||||
backgroundRefreshInventory(useProviderInventoryStore.getState()),
|
||||
)
|
||||
.catch((err) =>
|
||||
console.error("Failed to background-refresh inventory:", err),
|
||||
)
|
||||
.finally(() => {
|
||||
refreshingRef.current = false;
|
||||
});
|
||||
}, []);
|
||||
|
||||
return {
|
||||
selectedAgentId,
|
||||
pickerAgents,
|
||||
@@ -170,5 +189,6 @@ export function useAgentModelPickerState({
|
||||
modelStatusMessage,
|
||||
handleProviderChange,
|
||||
handleModelChange,
|
||||
handlePickerOpen,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -230,6 +230,7 @@ export function useChatSessionController({
|
||||
modelStatusMessage,
|
||||
handleProviderChange,
|
||||
handleModelChange,
|
||||
handlePickerOpen,
|
||||
effectiveModelSelection,
|
||||
} = useResolvedAgentModelPicker({
|
||||
providers,
|
||||
@@ -819,6 +820,7 @@ export function useChatSessionController({
|
||||
modelsLoading,
|
||||
modelStatusMessage,
|
||||
handleModelChange: handleModelChangeWithContextReset,
|
||||
handlePickerOpen,
|
||||
selectedProjectId: effectiveProjectId,
|
||||
availableProjects,
|
||||
handleProjectChange,
|
||||
|
||||
@@ -184,6 +184,7 @@ export function useResolvedAgentModelPicker({
|
||||
modelStatusMessage,
|
||||
handleProviderChange,
|
||||
handleModelChange,
|
||||
handlePickerOpen,
|
||||
} = useAgentModelPickerState({
|
||||
providers,
|
||||
selectedProvider,
|
||||
@@ -463,6 +464,7 @@ export function useResolvedAgentModelPicker({
|
||||
modelStatusMessage,
|
||||
handleProviderChange,
|
||||
handleModelChange,
|
||||
handlePickerOpen,
|
||||
effectiveModelSelection,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -64,6 +64,7 @@ export interface ChatInputProps {
|
||||
modelsLoading?: boolean;
|
||||
modelStatusMessage?: string | null;
|
||||
onModelChange?: (modelId: string) => void;
|
||||
onPickerOpen?: () => void;
|
||||
selectedProjectId?: string | null;
|
||||
availableProjects?: ProjectOption[];
|
||||
onProjectChange?: (projectId: string | null) => void;
|
||||
|
||||
@@ -7,8 +7,6 @@ import {
|
||||
} from "@tabler/icons-react";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import type { AcpProvider } from "@/shared/api/acp";
|
||||
import { getProviderInventory } from "@/features/providers/api/inventory";
|
||||
import { useProviderInventoryStore } from "@/features/providers/stores/providerInventoryStore";
|
||||
import { cn } from "@/shared/lib/cn";
|
||||
import { Button } from "@/shared/ui/button";
|
||||
import { Popover, PopoverContent, PopoverTrigger } from "@/shared/ui/popover";
|
||||
@@ -34,6 +32,7 @@ interface AgentModelPickerProps {
|
||||
loading?: boolean;
|
||||
isCompact?: boolean;
|
||||
showSelectedModelInTrigger?: boolean;
|
||||
onOpen?: () => void;
|
||||
}
|
||||
|
||||
function getModelDisplayName(model: ModelOption) {
|
||||
@@ -321,14 +320,11 @@ export function AgentModelPicker({
|
||||
loading = false,
|
||||
isCompact = false,
|
||||
showSelectedModelInTrigger = true,
|
||||
onOpen,
|
||||
}: AgentModelPickerProps) {
|
||||
const { t } = useTranslation("chat");
|
||||
const [open, setOpen] = useState(false);
|
||||
const [modelView, setModelView] = useState<ModelView>("recommended");
|
||||
const mergeInventoryEntries = useProviderInventoryStore(
|
||||
(s) => s.mergeEntries,
|
||||
);
|
||||
|
||||
const selectedAgentLabel =
|
||||
agents.find((agent) => agent.id === selectedAgentId)?.label ??
|
||||
formatProviderLabel(selectedAgentId);
|
||||
@@ -358,52 +354,32 @@ export function AgentModelPicker({
|
||||
}
|
||||
}, [open]);
|
||||
|
||||
useEffect(() => {
|
||||
if (!open) {
|
||||
return;
|
||||
}
|
||||
|
||||
let cancelled = false;
|
||||
|
||||
const syncInventory = async () => {
|
||||
try {
|
||||
const entries = await getProviderInventory();
|
||||
if (cancelled) {
|
||||
return;
|
||||
}
|
||||
mergeInventoryEntries(entries);
|
||||
} catch (error) {
|
||||
console.error("Failed to sync provider inventory from picker:", error);
|
||||
}
|
||||
};
|
||||
|
||||
void syncInventory();
|
||||
|
||||
return () => {
|
||||
cancelled = true;
|
||||
};
|
||||
}, [open, mergeInventoryEntries]);
|
||||
|
||||
// When in "all" view, expand the popover to full width for the search experience.
|
||||
const isAllView = modelView === "all";
|
||||
|
||||
return (
|
||||
<Popover open={open} onOpenChange={setOpen}>
|
||||
<Popover
|
||||
open={open}
|
||||
onOpenChange={(nextOpen) => {
|
||||
setOpen(nextOpen);
|
||||
if (nextOpen) onOpen?.();
|
||||
}}
|
||||
>
|
||||
<PopoverTrigger asChild>
|
||||
<Button
|
||||
type="button"
|
||||
variant="toolbar"
|
||||
size="sm"
|
||||
aria-label={t("toolbar.chooseAgentModel")}
|
||||
disabled={loading}
|
||||
disabled={loading && !selectedAgentLabel}
|
||||
leftIcon={getProviderIcon(selectedAgentId, "size-3.5")}
|
||||
rightIcon={<IconChevronDown className="opacity-50" />}
|
||||
className="min-w-0"
|
||||
>
|
||||
<span className={cn("truncate", isCompact ? "max-w-32" : "max-w-56")}>
|
||||
{loading
|
||||
? t("toolbar.loading")
|
||||
: (triggerModelLabel ?? selectedAgentLabel)}
|
||||
{triggerModelLabel ??
|
||||
selectedAgentLabel ??
|
||||
(loading ? t("toolbar.loading") : null)}
|
||||
</span>
|
||||
</Button>
|
||||
</PopoverTrigger>
|
||||
@@ -512,9 +488,25 @@ export function AgentModelPicker({
|
||||
className="flex min-h-0 min-w-0 overflow-hidden p-1"
|
||||
>
|
||||
{modelsLoading ? (
|
||||
<div className="flex min-h-0 flex-1 items-center gap-2 px-2 py-2 text-sm text-muted-foreground">
|
||||
<Spinner className="size-4" />
|
||||
<span>{t("toolbar.loadingModels")}</span>
|
||||
<div className="flex min-h-0 min-w-0 flex-1 flex-col">
|
||||
<div className="shrink-0 px-2 py-1.5 text-sm font-semibold">
|
||||
{t("toolbar.model")}
|
||||
</div>
|
||||
{currentModelName || currentModelId ? (
|
||||
<div className="space-y-0.5 p-1">
|
||||
<PickerItem selected disabled>
|
||||
<div className="min-w-0 flex-1 truncate">
|
||||
{currentModelName ?? currentModelId}
|
||||
</div>
|
||||
<Spinner className="size-3.5 shrink-0" />
|
||||
</PickerItem>
|
||||
</div>
|
||||
) : (
|
||||
<div className="flex min-h-0 flex-1 items-center gap-2 px-2 py-2 text-sm text-muted-foreground">
|
||||
<Spinner className="size-4" />
|
||||
<span>{t("toolbar.loadingModels")}</span>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
) : availableModels.length > 0 ? (
|
||||
modelView === "recommended" ? (
|
||||
|
||||
@@ -48,6 +48,7 @@ export function ChatInput({
|
||||
modelsLoading = false,
|
||||
modelStatusMessage = null,
|
||||
onModelChange,
|
||||
onPickerOpen,
|
||||
selectedProjectId = null,
|
||||
availableProjects = [],
|
||||
onProjectChange,
|
||||
@@ -455,6 +456,7 @@ export function ChatInput({
|
||||
modelsLoading={modelsLoading}
|
||||
modelStatusMessage={modelStatusMessage}
|
||||
onModelChange={onModelChange}
|
||||
onPickerOpen={onPickerOpen}
|
||||
selectedProjectId={selectedProjectId}
|
||||
availableProjects={availableProjects}
|
||||
onProjectChange={onProjectChange}
|
||||
|
||||
@@ -63,6 +63,7 @@ interface ChatInputToolbarProps {
|
||||
modelsLoading?: boolean;
|
||||
modelStatusMessage?: string | null;
|
||||
onModelChange?: (modelId: string) => void;
|
||||
onPickerOpen?: () => void;
|
||||
// Project
|
||||
selectedProjectId: string | null;
|
||||
availableProjects: ProjectOption[];
|
||||
@@ -108,6 +109,7 @@ export function ChatInputToolbar({
|
||||
modelsLoading = false,
|
||||
modelStatusMessage = null,
|
||||
onModelChange,
|
||||
onPickerOpen,
|
||||
selectedProjectId,
|
||||
availableProjects,
|
||||
onProjectChange,
|
||||
@@ -231,6 +233,7 @@ export function ChatInputToolbar({
|
||||
modelsLoading={modelsLoading}
|
||||
modelStatusMessage={modelStatusMessage}
|
||||
onModelChange={onModelChange}
|
||||
onOpen={onPickerOpen}
|
||||
loading={providersLoading}
|
||||
isCompact={isCompact}
|
||||
showSelectedModelInTrigger={selectedPersonaId === null}
|
||||
|
||||
@@ -139,6 +139,7 @@ export function ChatView({
|
||||
modelsLoading={controller.modelsLoading}
|
||||
modelStatusMessage={controller.modelStatusMessage}
|
||||
onModelChange={controller.handleModelChange}
|
||||
onPickerOpen={controller.handlePickerOpen}
|
||||
selectedProjectId={controller.selectedProjectId}
|
||||
availableProjects={controller.availableProjects}
|
||||
onProjectChange={controller.handleProjectChange}
|
||||
|
||||
@@ -92,6 +92,7 @@ function HomeComposer({
|
||||
modelsLoading={controller.modelsLoading}
|
||||
modelStatusMessage={controller.modelStatusMessage}
|
||||
onModelChange={controller.handleModelChange}
|
||||
onPickerOpen={controller.handlePickerOpen}
|
||||
selectedProjectId={controller.selectedProjectId}
|
||||
availableProjects={controller.availableProjects}
|
||||
onProjectChange={controller.handleProjectChange}
|
||||
|
||||
@@ -0,0 +1,97 @@
|
||||
import type { ProviderInventoryEntryDto } from "@aaif/goose-sdk";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { backgroundRefreshInventory } from "./inventory";
|
||||
|
||||
const mockClient = vi.hoisted(() => ({
|
||||
GooseProvidersList: vi.fn(),
|
||||
GooseProvidersInventoryRefresh: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@/shared/api/acpConnection", () => ({
|
||||
getClient: vi.fn(async () => ({
|
||||
goose: mockClient,
|
||||
})),
|
||||
}));
|
||||
|
||||
function providerEntry(
|
||||
overrides: Partial<ProviderInventoryEntryDto>,
|
||||
): ProviderInventoryEntryDto {
|
||||
return {
|
||||
providerId: "openai",
|
||||
providerName: "OpenAI",
|
||||
description: "",
|
||||
defaultModel: "",
|
||||
configured: false,
|
||||
providerType: "Preferred",
|
||||
configKeys: [],
|
||||
setupSteps: [],
|
||||
supportsRefresh: false,
|
||||
refreshing: false,
|
||||
models: [],
|
||||
stale: false,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe("backgroundRefreshInventory", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("merges fetched inventory before returning when no providers are configured", async () => {
|
||||
const entries = [
|
||||
providerEntry({ providerId: "openai", providerName: "OpenAI" }),
|
||||
];
|
||||
const inventoryStore = { mergeEntries: vi.fn() };
|
||||
mockClient.GooseProvidersList.mockResolvedValue({ entries });
|
||||
|
||||
await backgroundRefreshInventory(inventoryStore);
|
||||
|
||||
expect(inventoryStore.mergeEntries).toHaveBeenCalledWith(entries);
|
||||
expect(mockClient.GooseProvidersInventoryRefresh).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("merges fetched inventory before returning when no refresh starts", async () => {
|
||||
const entries = [
|
||||
providerEntry({
|
||||
providerId: "openai",
|
||||
providerName: "OpenAI",
|
||||
configured: true,
|
||||
}),
|
||||
];
|
||||
const inventoryStore = { mergeEntries: vi.fn() };
|
||||
mockClient.GooseProvidersList.mockResolvedValue({ entries });
|
||||
mockClient.GooseProvidersInventoryRefresh.mockResolvedValue({
|
||||
started: [],
|
||||
});
|
||||
|
||||
await backgroundRefreshInventory(inventoryStore);
|
||||
|
||||
expect(inventoryStore.mergeEntries).toHaveBeenCalledWith(entries);
|
||||
expect(mockClient.GooseProvidersInventoryRefresh).toHaveBeenCalledWith({
|
||||
providerIds: ["openai"],
|
||||
});
|
||||
});
|
||||
|
||||
it("does not re-merge entries supplied by a caller that already stored them", async () => {
|
||||
const entries = [
|
||||
providerEntry({
|
||||
providerId: "openai",
|
||||
providerName: "OpenAI",
|
||||
configured: true,
|
||||
}),
|
||||
];
|
||||
const inventoryStore = { mergeEntries: vi.fn() };
|
||||
mockClient.GooseProvidersInventoryRefresh.mockResolvedValue({
|
||||
started: [],
|
||||
});
|
||||
|
||||
await backgroundRefreshInventory(inventoryStore, entries);
|
||||
|
||||
expect(mockClient.GooseProvidersList).not.toHaveBeenCalled();
|
||||
expect(inventoryStore.mergeEntries).not.toHaveBeenCalled();
|
||||
expect(mockClient.GooseProvidersInventoryRefresh).toHaveBeenCalledWith({
|
||||
providerIds: ["openai"],
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -30,3 +30,43 @@ export async function refreshProviderInventory(
|
||||
);
|
||||
return response;
|
||||
}
|
||||
|
||||
/**
|
||||
* Refresh configured provider inventories in the background, polling until
|
||||
* all providers finish refreshing. If no entries are supplied, fetch and merge
|
||||
* the current inventory snapshot first so the UI sees fresh cached data even
|
||||
* when no refresh starts.
|
||||
*
|
||||
* Does NOT set the store's `loading` flag, so the UI keeps showing cached data
|
||||
* during the refresh.
|
||||
*/
|
||||
export async function backgroundRefreshInventory(
|
||||
inventoryStore: {
|
||||
mergeEntries: (entries: ProviderInventoryEntryDto[]) => void;
|
||||
},
|
||||
initialEntries?: ProviderInventoryEntryDto[],
|
||||
): Promise<void> {
|
||||
const entries = initialEntries?.length
|
||||
? initialEntries
|
||||
: await getProviderInventory();
|
||||
|
||||
if (!initialEntries?.length) {
|
||||
inventoryStore.mergeEntries(entries);
|
||||
}
|
||||
|
||||
const configuredProviderIds = entries
|
||||
.filter((entry) => entry.configured)
|
||||
.map((entry) => entry.providerId);
|
||||
if (configuredProviderIds.length === 0) return;
|
||||
|
||||
const refresh = await refreshProviderInventory(configuredProviderIds);
|
||||
if (refresh.started.length === 0 && (refresh.skipped ?? []).length === 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
const { syncProviderInventory } = await import("./inventorySync");
|
||||
await syncProviderInventory(configuredProviderIds, {
|
||||
initialRefresh: refresh,
|
||||
onEntries: (entries) => inventoryStore.mergeEntries(entries),
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user