Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .changelog/next/fixed-issue-4157.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Vision pickers in the universe builder (image describe, style-reference analysis, corrective reference) now offer installed vision models the client id regex doesn't recognize, instead of showing a "no vision model" blocker
7 changes: 7 additions & 0 deletions client/src/components/universe/VisionDescribeModal.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ vi.mock('../../hooks/useProviderModels', () => ({
}),
}));

// VisionProviderPicker unions the server's authoritative VLM set into its model
// filter, so it runs the capability scan on mount — stub it out (this suite is
// about the modal's actions, and the picker's own suite covers the union).
vi.mock('../../hooks/useVisionModelIds', () => ({
default: () => ({ idsByProvider: null, loaded: true }),
}));

// The gallery picker pulls in the media/socket layer — stub it.
vi.mock('../imageGen/GalleryImagePicker', () => ({ default: () => null }));
vi.mock('../ProviderModelSelector', () => ({ default: () => null }));
Expand Down
37 changes: 30 additions & 7 deletions client/src/components/universe/VisionProviderPicker.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
*
* Owns a `useProviderModels` instance scoped to enabled API providers, with
* LOCAL backends (Ollama / LM Studio) restricted to vision-capable models (cloud
* providers' lists are left intact). Renders the provider+model dropdowns plus
* providers' lists are left intact) — by the client id regex UNIONED with the
* server's authoritative per-provider VLM set (`useVisionModelIds`), like every
* other vision picker. Renders the provider+model dropdowns plus
* the "no vision model" / "no provider" guidance, and lifts the current
* selection to the parent via `onChange` so the caller can submit the chosen
* `{ providerId, model }` and gate its action on a vision model being present.
Expand All @@ -15,27 +17,48 @@
* it's actually needed.
*/

import { useEffect } from 'react';
import { useCallback, useEffect } from 'react';
import ProviderModelSelector from '../ProviderModelSelector';
import useProviderModels from '../../hooks/useProviderModels';
import { enabledApiProviderFilter, visionLocalModelFilter } from '../../utils/providers';
import useVisionModelIds from '../../hooks/useVisionModelIds';
import { enabledApiProviderFilter, localBackendForProvider, visionLocalModelFilter } from '../../utils/providers';

export default function VisionProviderPicker({ label = 'Vision provider', onChange }) {
// The server's authoritative per-provider VLM set, unioned into the filter:
// the client id regex only knows the multimodal families it was written
// against, so on its own it hides installed VLMs from newer ones (`gemma4`).
// Every mount of this picker is behind a modal/conditional render, so the
// capability scan is already deferred until it's needed — no `enabled` gate.
const { idsByProvider: visionIds, loaded: visionLoaded } = useVisionModelIds();
const modelFilter = useCallback(
(id, provider) => visionLocalModelFilter(id, provider, visionIds),
[visionIds],
);

const {
providers, selectedProviderId, selectedModel, availableModels,
setSelectedProviderId, setSelectedModel, loading,
} = useProviderModels({ filter: enabledApiProviderFilter, modelFilter: visionLocalModelFilter, silent: true });
} = useProviderModels({ filter: enabledApiProviderFilter, modelFilter, silent: true });

const hasProviders = providers.length > 0;
// While the capability scan is in flight the filter is regex-only, so an empty
// selection on a LOCAL backend is "don't know yet", not "none installed" —
// asserting the blocker here would flash it and then flip. Cloud providers are
// never filtered, so their empty selection is already a final answer.
const visionPending = !visionLoaded
&& !!localBackendForProvider(providers.find((p) => p.id === selectedProviderId));
// A provider is selected but exposes no vision-capable model (all of a local
// backend's models were filtered out) — block the run with an explanation.
const noVisionModel = hasProviders && !selectedModel;
const noVisionModel = hasProviders && !selectedModel && !visionPending;

// Lift the selection so the caller can submit it and gate on a vision model.
// `onChange` should be a stable setter; deps are bounded (load + user picks).
// `loading` covers the capability scan too — the auto-picked model can still
// change when it lands, so the selection isn't final until it settles.
const resolving = loading || visionPending;
useEffect(() => {
onChange?.({ providerId: selectedProviderId, model: selectedModel, hasProviders, noVisionModel, loading });
}, [onChange, selectedProviderId, selectedModel, hasProviders, noVisionModel, loading]);
onChange?.({ providerId: selectedProviderId, model: selectedModel, hasProviders, noVisionModel, loading: resolving });
}, [onChange, selectedProviderId, selectedModel, hasProviders, noVisionModel, resolving]);

if (!hasProviders) {
return (
Expand Down
104 changes: 104 additions & 0 deletions client/src/components/universe/VisionProviderPicker.test.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { render, screen, waitFor, within } from '@testing-library/react';

vi.mock('../../services/api', () => ({ getProviders: vi.fn() }));
vi.mock('../../services/apiLocalLlm', () => ({ getVisionModels: vi.fn(), getToolUseModels: vi.fn() }));

import VisionProviderPicker from './VisionProviderPicker';
import { getProviders } from '../../services/api';
import { getVisionModels } from '../../services/apiLocalLlm';

// A local backend whose ONLY vision-capable model belongs to a family the
// client id regex predates — the `gemma4` gap, restaged with a placeholder id
// so it keeps testing the GAP rather than whichever families the regex has
// since learned. Regex-only, this picker renders empty.
const VLM_ID = 'muse-glimmer:30b';
const OLLAMA = {
id: 'ollama',
name: 'Ollama',
type: 'api',
enabled: true,
endpoint: 'http://127.0.0.1:11434',
defaultModel: 'qwen3.6:35b',
models: ['qwen3.6:35b', VLM_ID, 'nomic-embed-text'],
};

// Cloud providers are never id-filtered — the regex is a local-name heuristic.
const CLOUD = {
id: 'cloud-api',
name: 'Cloud API',
type: 'api',
enabled: true,
defaultModel: 'omni-1',
models: ['omni-1', 'omni-1-mini'],
};

const modelSelect = () => screen.getByLabelText('Model');

beforeEach(() => {
vi.clearAllMocks();
getProviders.mockResolvedValue({ providers: [OLLAMA] });
getVisionModels.mockResolvedValue({ models: [] });
});

describe('VisionProviderPicker', () => {
it('offers a VLM the client id regex does not recognize, once the server list lands', async () => {
getVisionModels.mockResolvedValue({
models: [{ id: VLM_ID, backend: 'ollama', providerId: 'ollama', vision: true }],
});
const onChange = vi.fn();
render(<VisionProviderPicker onChange={onChange} />);

// The auto-pick is lifted, so the caller's "Run" gate (`!!vision.model`) opens.
await waitFor(() => expect(onChange).toHaveBeenLastCalledWith(
expect.objectContaining({ providerId: 'ollama', model: VLM_ID, noVisionModel: false }),
));
expect(modelSelect()).toHaveValue(VLM_ID);
expect(within(modelSelect()).getByRole('option', { name: VLM_ID })).toBeInTheDocument();
expect(screen.queryByText(/no vision-capable model installed/i)).not.toBeInTheDocument();
});

it('still blocks with an explanation when the backend really has no VLM', async () => {
// The scan settles reporting nothing installed — regex-only is then the best
// answer available, and it also finds nothing.
render(<VisionProviderPicker onChange={vi.fn()} />);

await waitFor(() => expect(screen.getByText(/no vision-capable model installed/i)).toBeInTheDocument());
expect(screen.queryByLabelText('Model')).not.toBeInTheDocument();
});

it('does not flash the blocker while the capability scan is still in flight', async () => {
let settle;
getVisionModels.mockReturnValue(new Promise((resolve) => { settle = resolve; }));
const onChange = vi.fn();
render(<VisionProviderPicker onChange={onChange} />);

// Providers have loaded and the regex found nothing — but "don't know yet"
// must not render as "none installed".
await waitFor(() => expect(onChange).toHaveBeenCalled());
expect(screen.queryByText(/no vision-capable model installed/i)).not.toBeInTheDocument();
expect(onChange).toHaveBeenLastCalledWith(
expect.objectContaining({ noVisionModel: false, loading: true }),
);

settle({ models: [] });
await waitFor(() => expect(screen.getByText(/no vision-capable model installed/i)).toBeInTheDocument());
});

it('leaves a cloud provider unfiltered while the scan is pending', async () => {
getProviders.mockResolvedValue({ providers: [CLOUD] });
getVisionModels.mockReturnValue(new Promise(() => {}));
render(<VisionProviderPicker onChange={vi.fn()} />);

// No local backend selected → nothing to wait for; the cloud list is final.
await waitFor(() => expect(modelSelect()).toHaveValue('omni-1'));
expect(screen.queryByText(/no vision-capable model installed/i)).not.toBeInTheDocument();
});

it('explains the empty case when no provider is configured at all', async () => {
getProviders.mockResolvedValue({ providers: [] });
render(<VisionProviderPicker onChange={vi.fn()} />);

await waitFor(() => expect(screen.getByText(/No API provider with a vision-capable model/i)).toBeInTheDocument());
});
});
58 changes: 55 additions & 3 deletions client/src/hooks/useProviderModels.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,15 @@ const sourceModels = (provider, withEffort) => {
* set, the auto-selected / provider-change model is the first model that
* passes the filter rather than the provider's `defaultModel` (which may not
* qualify). Omit for the full selectable list.
*
* A `modelFilter` whose IDENTITY changes is supported and expected: a vision
* picker starts on the client-side id regex and widens once the server's
* authoritative capability list resolves (`useVisionModelIds`). The hook then
* re-runs its initial pick — without refetching the provider list — so a
* selection the first, blinder filter couldn't make isn't frozen at `''`.
* Once the user picks (or clears) a model, that wins and the re-pick stands
* down. Memoize the predicate (`useCallback`) so it only changes when its
* inputs do.
* @param {boolean} [options.withEffort] - Set when the caller also renders an
* effort control (`ProviderModelSelector`'s `effort`/`onEffortChange`) and
* threads the value to the server. Providers whose CLI bakes the reasoning
Expand All @@ -50,6 +59,12 @@ export default function useProviderModels({ filter, allowDefault = false, silent
const [selectedModel, setSelectedModel] = useState('');
const [loading, setLoading] = useState(true);
const hasSetInitialRef = useRef(false);
// Latched once the model is chosen deliberately — a picker change, or a
// caller restoring a saved pin. The auto re-pick below then stands down for
// good, so a deliberate CLEAR (`''`) is not silently refilled when the filter
// later widens; `selectedModel === ''` alone can't carry that distinction,
// since it is also what a filter that matched nothing produces.
const userPickedModelRef = useRef(false);

// Resolve the model to pin when a provider is (auto-)selected. With a
// modelFilter, the provider's defaultModel may not qualify (e.g. a vision
Expand All @@ -62,6 +77,16 @@ export default function useProviderModels({ filter, allowDefault = false, silent
return models[0] || '';
}, [modelFilter, withEffort]);

// `load` must NOT depend on `pickInitialModel` (and so on `modelFilter`): a
// caller whose filter identity changes when a capability list resolves would
// otherwise re-run the whole `api.getProviders()` fetch for a change that
// needs no new data. The ref hands the async body the freshest picker without
// pulling it into the dependency list — synced in an effect (never mutated
// during render), which is soon enough: `load` only reads it after awaiting
// the fetch, and the initial value covers the mount.
const pickInitialModelRef = useRef(pickInitialModel);
useEffect(() => { pickInitialModelRef.current = pickInitialModel; }, [pickInitialModel]);

const load = useCallback(async () => {
setLoading(true);
const data = await api.getProviders(silent ? { silent: true } : undefined).catch((err) => {
Expand All @@ -76,10 +101,10 @@ export default function useProviderModels({ filter, allowDefault = false, silent
if (!allowDefault && filtered.length > 0 && !hasSetInitialRef.current) {
hasSetInitialRef.current = true;
setSelectedProviderId(filtered[0].id);
setSelectedModel(pickInitialModel(filtered[0]));
setSelectedModel(pickInitialModelRef.current(filtered[0]));
}
setLoading(false);
}, [filter, allowDefault, silent, pickInitialModel]);
}, [filter, allowDefault, silent]);

useEffect(() => { load(); }, [load]);

Expand Down Expand Up @@ -107,8 +132,35 @@ export default function useProviderModels({ filter, allowDefault = false, silent
[currentProvider, modelFilter, selectedModel, withEffort]
);

// Re-run the initial pick when the `modelFilter`'s identity changes. Without
// this, `hasSetInitialRef` freezes the auto-pick at whatever the FIRST filter
// produced: a vision picker running on the client id regex alone returns `''`
// for a backend whose only VLM the regex doesn't know (`gemma4`), and the
// authoritative list landing a moment later never gets a say — leaving a
// "no vision model" blocker next to a now-populated dropdown. Scoped to
// filtered pickers (an unfiltered one pins `defaultModel`, which needs no
// revision) and to a selection that is still the hook's own.
useEffect(() => {
if (!modelFilter || allowDefault || userPickedModelRef.current) return;
if (!hasSetInitialRef.current || !currentProvider) return;
// Still valid under the current filter → nothing to revise.
if (selectedModel && availableModels.includes(selectedModel)) return;
const next = pickInitialModel(currentProvider);
if (next !== selectedModel) setSelectedModel(next);
}, [modelFilter, allowDefault, currentProvider, availableModels, selectedModel, pickInitialModel]);

// A user pick latches: the re-pick above never overrides it, in either
// direction (a chosen model, or a deliberate clear).
const handleModelChange = useCallback((model) => {
userPickedModelRef.current = true;
setSelectedModel(model);
}, []);

const handleProviderChange = useCallback((id) => {
setSelectedProviderId(id);
// The model that follows a provider change is auto-picked, not user-picked,
// so it stays eligible for the re-pick above if the filter widens later.
userPickedModelRef.current = false;
if (allowDefault) {
// Empty model = "use the default model" — don't pin the provider's
// defaultModel, which would suppress the empty-sentinel choice.
Expand All @@ -131,7 +183,7 @@ export default function useProviderModels({ filter, allowDefault = false, silent
availableModels,
selectedProvider,
setSelectedProviderId: handleProviderChange,
setSelectedModel,
setSelectedModel: handleModelChange,
loading
};
}
98 changes: 98 additions & 0 deletions client/src/hooks/useProviderModels.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,3 +90,101 @@ describe('useProviderModels — Antigravity base models', () => {
]);
});
});

// A capability-scoped picker (vision) starts on the client-side id regex and
// widens once the server's authoritative list resolves, so its `modelFilter`
// identity changes AFTER the first load. Stand-in filters here: the contract
// under test is the identity change, not any particular capability rule.
describe('useProviderModels — a modelFilter whose identity changes', () => {
const LOCAL = {
id: 'local-backend',
name: 'Local Backend',
type: 'api',
enabled: true,
defaultModel: 'text-a',
models: ['text-a', 'vlm-b', 'vlm-c'],
};
const OTHER = { ...LOCAL, id: 'other-backend', name: 'Other Backend', defaultModel: 'text-a' };

// The blind first pass: knows no vision family this backend has installed.
const matchesNothing = () => false;
const matchesB = (id) => id === 'vlm-b';
const matchesBandC = (id) => id === 'vlm-b' || id === 'vlm-c';

const mountFiltered = async (modelFilter, providers = [LOCAL], extra = {}) => {
api.getProviders.mockResolvedValue({ providers });
const hook = renderHook(
({ filterFn }) => useProviderModels({ modelFilter: filterFn, ...extra }),
{ initialProps: { filterFn: modelFilter } },
);
await waitFor(() => expect(hook.result.current.loading).toBe(false));
return hook;
};

beforeEach(() => vi.clearAllMocks());

it('re-picks the initial model once the filter widens', async () => {
const { result, rerender } = await mountFiltered(matchesNothing);
// The blind first pass finds nothing — the bug this guards is that the
// hook used to freeze here forever.
expect(result.current.selectedModel).toBe('');
expect(result.current.availableModels).toEqual([]);

rerender({ filterFn: matchesB });
await waitFor(() => expect(result.current.selectedModel).toBe('vlm-b'));
expect(result.current.availableModels).toEqual(['vlm-b']);
});

it('does not refetch the provider list for a filter identity change', async () => {
const { rerender, result } = await mountFiltered(matchesNothing);
expect(api.getProviders).toHaveBeenCalledTimes(1);

rerender({ filterFn: matchesB });
await waitFor(() => expect(result.current.selectedModel).toBe('vlm-b'));
expect(api.getProviders).toHaveBeenCalledTimes(1);
});

it('leaves a deliberate user clear alone when the filter widens', async () => {
const { result, rerender } = await mountFiltered(matchesB);
expect(result.current.selectedModel).toBe('vlm-b');

act(() => result.current.setSelectedModel(''));
rerender({ filterFn: matchesBandC });
// A clear and a "the filter matched nothing" both read as `''` — only the
// user-pick latch tells them apart, and the user's wins.
await waitFor(() => expect(result.current.availableModels).toEqual(['vlm-b', 'vlm-c']));
expect(result.current.selectedModel).toBe('');
});

it('leaves a user-picked model alone when the filter changes', async () => {
const { result, rerender } = await mountFiltered(matchesBandC);
act(() => result.current.setSelectedModel('vlm-c'));

rerender({ filterFn: matchesB });
await waitFor(() => expect(result.current.availableModels).toEqual(['vlm-b']));
expect(result.current.selectedModel).toBe('vlm-c');
});

it('re-arms the auto-pick after a provider change', async () => {
const { result, rerender } = await mountFiltered(matchesNothing, [LOCAL, OTHER]);
// A provider change picks through the CURRENT (still blind) filter…
act(() => result.current.setSelectedProviderId('other-backend'));
expect(result.current.selectedModel).toBe('');

// …so the widened filter must still get its say on that provider.
rerender({ filterFn: matchesB });
await waitFor(() => expect(result.current.selectedModel).toBe('vlm-b'));
expect(result.current.selectedProviderId).toBe('other-backend');
});

it('keeps the empty-model sentinel under allowDefault', async () => {
const { result, rerender } = await mountFiltered(matchesNothing, [LOCAL], { allowDefault: true });
expect(result.current.selectedProviderId).toBe('');
expect(result.current.selectedModel).toBe('');

rerender({ filterFn: matchesB });
await waitFor(() => expect(result.current.availableModels).toEqual([]));
// `''` is the "use the default model" choice here, never an auto-pick target.
expect(result.current.selectedModel).toBe('');
});
});