fix: chat loading-state model placeholder (#8431)
Signed-off-by: sunilkumarvalmiki <g.sunilkumarvalmiki@gmail.com> Co-authored-by: jh-block <jhugo@block.xyz>
This commit is contained in:
@@ -0,0 +1,109 @@
|
|||||||
|
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||||
|
import { render, type RenderOptions, screen } from '@testing-library/react';
|
||||||
|
import ModelsBottomBar from './ModelsBottomBar';
|
||||||
|
import { IntlTestWrapper } from '../../../../i18n/test-utils';
|
||||||
|
|
||||||
|
const renderWithIntl = (ui: React.ReactElement, options?: RenderOptions) =>
|
||||||
|
render(ui, { wrapper: IntlTestWrapper, ...options });
|
||||||
|
|
||||||
|
const createDropdownRef = (): React.RefObject<HTMLDivElement> =>
|
||||||
|
({ current: document.createElement('div') }) as React.RefObject<HTMLDivElement>;
|
||||||
|
|
||||||
|
let mockCurrentModel: string | null = 'config-model';
|
||||||
|
let mockCurrentProvider: string | null = 'config-provider';
|
||||||
|
const mockGetProviders = vi.fn();
|
||||||
|
const mockOnModelChanged = vi.fn();
|
||||||
|
|
||||||
|
vi.mock('../../../ModelAndProviderContext', () => ({
|
||||||
|
useModelAndProvider: () => ({
|
||||||
|
currentModel: mockCurrentModel,
|
||||||
|
currentProvider: mockCurrentProvider,
|
||||||
|
}),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('../../../ConfigContext', () => ({
|
||||||
|
useConfig: () => ({
|
||||||
|
getProviders: mockGetProviders,
|
||||||
|
}),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('../modelInterface', () => ({
|
||||||
|
getProviderMetadata: vi.fn().mockResolvedValue({ display_name: 'Config Provider' }),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('../predefinedModelsUtils', () => ({
|
||||||
|
getModelDisplayName: (model: string) => `Display ${model}`,
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('../../../bottom_menu/BottomMenuAlertPopover', () => ({
|
||||||
|
default: () => null,
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('../../../ui/dropdown-menu', () => ({
|
||||||
|
DropdownMenu: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
||||||
|
DropdownMenuTrigger: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
||||||
|
DropdownMenuContent: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
||||||
|
DropdownMenuItem: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('../../localInference/ModelSettingsPanel', () => ({
|
||||||
|
ModelSettingsPanel: () => null,
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('../../../ui/scroll-area', () => ({
|
||||||
|
ScrollArea: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
||||||
|
}));
|
||||||
|
|
||||||
|
describe('ModelsBottomBar', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.clearAllMocks();
|
||||||
|
mockCurrentModel = 'config-model';
|
||||||
|
mockCurrentProvider = 'config-provider';
|
||||||
|
mockGetProviders.mockResolvedValue([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('shows a loading placeholder while the active session model is still loading', async () => {
|
||||||
|
renderWithIntl(
|
||||||
|
<ModelsBottomBar
|
||||||
|
sessionId="session-123"
|
||||||
|
dropdownRef={createDropdownRef()}
|
||||||
|
setView={vi.fn()}
|
||||||
|
onModelChanged={mockOnModelChanged}
|
||||||
|
sessionLoaded={false}
|
||||||
|
/>
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(screen.getByTestId('model-loading-state')).toHaveTextContent('Loading model...');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('shows the active session model once the session has loaded', async () => {
|
||||||
|
renderWithIntl(
|
||||||
|
<ModelsBottomBar
|
||||||
|
sessionId="session-123"
|
||||||
|
dropdownRef={createDropdownRef()}
|
||||||
|
setView={vi.fn()}
|
||||||
|
sessionModel="session-model"
|
||||||
|
sessionProvider="session-provider"
|
||||||
|
onModelChanged={mockOnModelChanged}
|
||||||
|
sessionLoaded={true}
|
||||||
|
/>
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(screen.getByText('session-model')).toBeInTheDocument();
|
||||||
|
expect(screen.queryByTestId('model-loading-state')).not.toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('shows the configured model when there is no active session', async () => {
|
||||||
|
renderWithIntl(
|
||||||
|
<ModelsBottomBar
|
||||||
|
sessionId={null}
|
||||||
|
dropdownRef={createDropdownRef()}
|
||||||
|
setView={vi.fn()}
|
||||||
|
onModelChanged={mockOnModelChanged}
|
||||||
|
/>
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(screen.getByText('config-model')).toBeInTheDocument();
|
||||||
|
expect(screen.queryByTestId('model-loading-state')).not.toBeInTheDocument();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -1,4 +1,4 @@
|
|||||||
import { Sliders, Bot, Settings } from 'lucide-react';
|
import { Sliders, Bot, LoaderCircle, Settings } from 'lucide-react';
|
||||||
import React, { useEffect, useState } from 'react';
|
import React, { useEffect, useState } from 'react';
|
||||||
import { useModelAndProvider } from '../../../ModelAndProviderContext';
|
import { useModelAndProvider } from '../../../ModelAndProviderContext';
|
||||||
import { SwitchModelModal } from '../subcomponents/SwitchModelModal';
|
import { SwitchModelModal } from '../subcomponents/SwitchModelModal';
|
||||||
@@ -26,6 +26,10 @@ const i18n = defineMessages({
|
|||||||
id: 'modelsBottomBar.currentModel',
|
id: 'modelsBottomBar.currentModel',
|
||||||
defaultMessage: 'Current model',
|
defaultMessage: 'Current model',
|
||||||
},
|
},
|
||||||
|
loadingModel: {
|
||||||
|
id: 'modelsBottomBar.loadingModel',
|
||||||
|
defaultMessage: 'Loading model...',
|
||||||
|
},
|
||||||
changeModel: {
|
changeModel: {
|
||||||
id: 'modelsBottomBar.changeModel',
|
id: 'modelsBottomBar.changeModel',
|
||||||
defaultMessage: 'Change Model',
|
defaultMessage: 'Change Model',
|
||||||
@@ -73,10 +77,13 @@ export default function ModelsBottomBar({
|
|||||||
const [isLocalModelSettingsOpen, setIsLocalModelSettingsOpen] = useState(false);
|
const [isLocalModelSettingsOpen, setIsLocalModelSettingsOpen] = useState(false);
|
||||||
const [providerDefaultModel, setProviderDefaultModel] = useState<string | null>(null);
|
const [providerDefaultModel, setProviderDefaultModel] = useState<string | null>(null);
|
||||||
|
|
||||||
// Hide label while session data is still being fetched (avoids flashing
|
// Show a visible loading placeholder while session metadata is still being fetched,
|
||||||
// the config default before the session's actual model arrives).
|
// rather than flashing the config default or leaving the footer blank.
|
||||||
const isModelLoading = sessionId && !sessionLoaded;
|
const isModelLoading = Boolean(sessionId && !sessionLoaded);
|
||||||
const displayModel = currentModel || providerDefaultModel || displayModelName;
|
const displayModel = currentModel || providerDefaultModel || displayModelName;
|
||||||
|
const loadingModelLabel = intl.formatMessage(i18n.loadingModel);
|
||||||
|
const triggerLabel = isModelLoading ? loadingModelLabel : displayModel;
|
||||||
|
const menuModelLabel = isModelLoading ? loadingModelLabel : displayModelName;
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!currentProvider) return;
|
if (!currentProvider) return;
|
||||||
@@ -121,16 +128,24 @@ export default function ModelsBottomBar({
|
|||||||
<DropdownMenuTrigger className="flex items-center hover:cursor-pointer max-w-[180px] md:max-w-[200px] lg:max-w-[380px] min-w-0 text-text-primary/70 hover:text-text-primary transition-colors">
|
<DropdownMenuTrigger className="flex items-center hover:cursor-pointer max-w-[180px] md:max-w-[200px] lg:max-w-[380px] min-w-0 text-text-primary/70 hover:text-text-primary transition-colors">
|
||||||
<div className="flex items-center truncate max-w-[130px] md:max-w-[200px] lg:max-w-[360px] min-w-0">
|
<div className="flex items-center truncate max-w-[130px] md:max-w-[200px] lg:max-w-[360px] min-w-0">
|
||||||
<Bot className="mr-1 h-4 w-4 flex-shrink-0" />
|
<Bot className="mr-1 h-4 w-4 flex-shrink-0" />
|
||||||
<span className={`truncate text-xs${isModelLoading ? ' opacity-0' : ''}`}>
|
{isModelLoading ? (
|
||||||
{displayModel}
|
<span
|
||||||
</span>
|
data-testid="model-loading-state"
|
||||||
|
className="inline-flex items-center gap-1 truncate text-xs"
|
||||||
|
>
|
||||||
|
<LoaderCircle className="h-3 w-3 animate-spin flex-shrink-0" />
|
||||||
|
<span className="truncate">{triggerLabel}</span>
|
||||||
|
</span>
|
||||||
|
) : (
|
||||||
|
<span className="truncate text-xs">{triggerLabel}</span>
|
||||||
|
)}
|
||||||
</div>
|
</div>
|
||||||
</DropdownMenuTrigger>
|
</DropdownMenuTrigger>
|
||||||
<DropdownMenuContent side="top" align="center" className="w-64 text-sm">
|
<DropdownMenuContent side="top" align="center" className="w-64 text-sm">
|
||||||
<h6 className="text-xs text-text-primary mt-2 ml-2">{intl.formatMessage(i18n.currentModel)}</h6>
|
<h6 className="text-xs text-text-primary mt-2 ml-2">{intl.formatMessage(i18n.currentModel)}</h6>
|
||||||
<p className="flex items-center justify-between text-sm mx-2 pb-2 border-b mb-2">
|
<p className="flex items-center justify-between text-sm mx-2 pb-2 border-b mb-2">
|
||||||
{displayModelName}
|
{menuModelLabel}
|
||||||
{displayProvider && ` — ${displayProvider}`}
|
{!isModelLoading && displayProvider && ` — ${displayProvider}`}
|
||||||
</p>
|
</p>
|
||||||
<DropdownMenuItem onClick={() => setIsAddModelModalOpen(true)}>
|
<DropdownMenuItem onClick={() => setIsAddModelModalOpen(true)}>
|
||||||
<span>{intl.formatMessage(i18n.changeModel)}</span>
|
<span>{intl.formatMessage(i18n.changeModel)}</span>
|
||||||
|
|||||||
@@ -2315,6 +2315,9 @@
|
|||||||
"modelsBottomBar.currentModel": {
|
"modelsBottomBar.currentModel": {
|
||||||
"defaultMessage": "Current model"
|
"defaultMessage": "Current model"
|
||||||
},
|
},
|
||||||
|
"modelsBottomBar.loadingModel": {
|
||||||
|
"defaultMessage": "Loading model..."
|
||||||
|
},
|
||||||
"modelsBottomBar.localModelSettings": {
|
"modelsBottomBar.localModelSettings": {
|
||||||
"defaultMessage": "Local Model Settings"
|
"defaultMessage": "Local Model Settings"
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -1,9 +1,9 @@
|
|||||||
import { test as base, Page, Browser, chromium } from '@playwright/test';
|
import { test as base, Page, Browser, chromium } from '@playwright/test';
|
||||||
import { spawn, ChildProcess } from 'child_process';
|
import { exec, spawn, ChildProcess } from 'child_process';
|
||||||
import { join } from 'path';
|
import { join } from 'path';
|
||||||
import { promisify } from 'util';
|
import { promisify } from 'util';
|
||||||
|
|
||||||
const execAsync = promisify(require('child_process').exec);
|
const execAsync = promisify(exec);
|
||||||
|
|
||||||
type GooseTestFixtures = {
|
type GooseTestFixtures = {
|
||||||
goosePage: Page;
|
goosePage: Page;
|
||||||
@@ -28,7 +28,8 @@ type GooseTestFixtures = {
|
|||||||
*/
|
*/
|
||||||
export const test = base.extend<GooseTestFixtures>({
|
export const test = base.extend<GooseTestFixtures>({
|
||||||
// Test-scoped fixture: launches a fresh Electron app for each test
|
// Test-scoped fixture: launches a fresh Electron app for each test
|
||||||
goosePage: async ({}, use, testInfo) => {
|
goosePage: async ({ browserName }, providePage, testInfo) => {
|
||||||
|
void browserName;
|
||||||
console.log(`Launching fresh Electron app for test: ${testInfo.title}`);
|
console.log(`Launching fresh Electron app for test: ${testInfo.title}`);
|
||||||
|
|
||||||
let appProcess: ChildProcess | null = null;
|
let appProcess: ChildProcess | null = null;
|
||||||
@@ -80,8 +81,9 @@ export const test = base.extend<GooseTestFixtures>({
|
|||||||
console.log(`Connected to Electron app on attempt ${attempt} (~${(attempt * retryDelay) / 1000}s)`);
|
console.log(`Connected to Electron app on attempt ${attempt} (~${(attempt * retryDelay) / 1000}s)`);
|
||||||
break;
|
break;
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
|
const errorMessage = error instanceof Error ? error.message : String(error);
|
||||||
if (attempt === maxRetries) {
|
if (attempt === maxRetries) {
|
||||||
throw new Error(`Failed to connect to Electron app after ${maxRetries} attempts (${(maxRetries * retryDelay) / 1000}s). Last error: ${error.message}`);
|
throw new Error(`Failed to connect to Electron app after ${maxRetries} attempts (${(maxRetries * retryDelay) / 1000}s). Last error: ${errorMessage}`);
|
||||||
}
|
}
|
||||||
// Wait before next retry
|
// Wait before next retry
|
||||||
await new Promise(resolve => setTimeout(resolve, retryDelay));
|
await new Promise(resolve => setTimeout(resolve, retryDelay));
|
||||||
@@ -92,26 +94,28 @@ export const test = base.extend<GooseTestFixtures>({
|
|||||||
throw new Error('Browser connection failed unexpectedly');
|
throw new Error('Browser connection failed unexpectedly');
|
||||||
}
|
}
|
||||||
|
|
||||||
// Get the electron app context and first page
|
// Wait for Electron to create its first window after the CDP endpoint is up.
|
||||||
const contexts = browser.contexts();
|
let page: Page | null = null;
|
||||||
if (contexts.length === 0) {
|
for (let attempt = 1; attempt <= 100; attempt++) {
|
||||||
throw new Error('No browser contexts found');
|
const contexts = browser.contexts();
|
||||||
|
page = contexts.flatMap((context) => context.pages())[0] ?? null;
|
||||||
|
if (page) {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
await new Promise((resolve) => setTimeout(resolve, 100));
|
||||||
}
|
}
|
||||||
|
|
||||||
const pages = contexts[0].pages();
|
if (!page) {
|
||||||
if (pages.length === 0) {
|
|
||||||
throw new Error('No windows/pages found');
|
throw new Error('No windows/pages found');
|
||||||
}
|
}
|
||||||
|
|
||||||
const page = pages[0];
|
|
||||||
|
|
||||||
// Wait for page to be ready
|
// Wait for page to be ready
|
||||||
await page.waitForLoadState('domcontentloaded');
|
await page.waitForLoadState('domcontentloaded');
|
||||||
|
|
||||||
// Try to wait for networkidle
|
// Try to wait for networkidle
|
||||||
try {
|
try {
|
||||||
await page.waitForLoadState('networkidle', { timeout: 10000 });
|
await page.waitForLoadState('networkidle', { timeout: 10000 });
|
||||||
} catch (error) {
|
} catch {
|
||||||
console.log('NetworkIdle timeout (likely due to MCP activity), continuing...');
|
console.log('NetworkIdle timeout (likely due to MCP activity), continuing...');
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -124,7 +128,7 @@ export const test = base.extend<GooseTestFixtures>({
|
|||||||
console.log('App ready, starting test...');
|
console.log('App ready, starting test...');
|
||||||
|
|
||||||
// Provide the page to the test
|
// Provide the page to the test
|
||||||
await use(page);
|
await providePage(page);
|
||||||
|
|
||||||
} finally {
|
} finally {
|
||||||
console.log('Cleaning up Electron app for this test...');
|
console.log('Cleaning up Electron app for this test...');
|
||||||
@@ -146,19 +150,23 @@ export const test = base.extend<GooseTestFixtures>({
|
|||||||
// First try SIGTERM for graceful shutdown
|
// First try SIGTERM for graceful shutdown
|
||||||
process.kill(-appProcess.pid, 'SIGTERM');
|
process.kill(-appProcess.pid, 'SIGTERM');
|
||||||
await new Promise(resolve => setTimeout(resolve, 2000));
|
await new Promise(resolve => setTimeout(resolve, 2000));
|
||||||
} catch (e) {
|
} catch {
|
||||||
// Process might already be dead
|
// Process might already be dead
|
||||||
}
|
}
|
||||||
// Then SIGKILL if still running
|
// Then SIGKILL if still running
|
||||||
try {
|
try {
|
||||||
process.kill(-appProcess.pid, 'SIGKILL');
|
process.kill(-appProcess.pid, 'SIGKILL');
|
||||||
} catch (e) {
|
} catch {
|
||||||
// Process already exited
|
// Process already exited
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
console.log('Cleaned up app process');
|
console.log('Cleaned up app process');
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
if (error.code !== 'ESRCH' && !error.message?.includes('No such process')) {
|
if (
|
||||||
|
error instanceof Error &&
|
||||||
|
!('code' in error && error.code === 'ESRCH') &&
|
||||||
|
!error.message.includes('No such process')
|
||||||
|
) {
|
||||||
console.error('Error killing app process:', error);
|
console.error('Error killing app process:', error);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,24 @@
|
|||||||
|
import { test, expect } from './fixtures';
|
||||||
|
|
||||||
|
test.describe('Loading State', () => {
|
||||||
|
test('shows a model placeholder while creating a new chat session', async ({ goosePage }) => {
|
||||||
|
await goosePage.waitForSelector('[data-testid="chat-input"]', { timeout: 30000 });
|
||||||
|
|
||||||
|
const chatInput = await goosePage.waitForSelector('[data-testid="chat-input"]');
|
||||||
|
await chatInput.fill('Respond with the single word hello.');
|
||||||
|
await chatInput.press('Enter');
|
||||||
|
|
||||||
|
await goosePage.waitForSelector('[data-testid="loading-indicator"]', {
|
||||||
|
state: 'visible',
|
||||||
|
timeout: 10000,
|
||||||
|
});
|
||||||
|
|
||||||
|
const loadingModel = goosePage.locator('[data-testid="model-loading-state"]');
|
||||||
|
await expect(loadingModel).toHaveText(/loading model/i, { timeout: 10000 });
|
||||||
|
|
||||||
|
await goosePage.screenshot({
|
||||||
|
path: test.info().outputPath('loading-state-fresh-session.png'),
|
||||||
|
fullPage: true,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user