mirror of
https://github.com/open-jarvis/OpenJarvis.git
synced 2026-08-14 00:47:52 +00:00
fix: never auto-select embed-only models for chat (#659)
* fix: never auto-select embed-only models for chat Ollama lists nomic-embed-text alongside chat models. Auto-picking models[0] / recommending the only available id selected the embedder and every generation failed with HTTP 400 "does not support chat". - Filter embed-only ids out of GET /v1/models (chat picker) - Exclude them from /v1/recommended-model; return empty when none left - Frontend setModels prefers chat models and clears a bad embed selection - Regression tests for mixed, embed-only, and classifier cases * fix: harden chat model capability filtering --------- Co-authored-by: Elliot Slusky <elliot@slusky.com>
This commit is contained in:
co-authored by
Elliot Slusky
parent
07fcf35276
commit
2ed885eb11
@@ -31,7 +31,6 @@ export default function App() {
|
||||
const prevModelRef = useRef<string>('');
|
||||
const setModels = useAppStore((s) => s.setModels);
|
||||
const setModelsLoading = useAppStore((s) => s.setModelsLoading);
|
||||
const setSelectedModel = useAppStore((s) => s.setSelectedModel);
|
||||
const selectedModel = useAppStore((s) => s.selectedModel);
|
||||
const setServerInfo = useAppStore((s) => s.setServerInfo);
|
||||
const setSavings = useAppStore((s) => s.setSavings);
|
||||
@@ -70,7 +69,6 @@ export default function App() {
|
||||
fetchModels()
|
||||
.then((m) => {
|
||||
setModels(m);
|
||||
if (!selectedModel && m.length > 0) setSelectedModel(m[0].id);
|
||||
})
|
||||
.catch(() => setModels([]))
|
||||
.finally(() => setModelsLoading(false));
|
||||
|
||||
@@ -7,6 +7,7 @@ import {
|
||||
type SetupStatus,
|
||||
} from '../lib/api';
|
||||
import { useAppStore } from '../lib/store';
|
||||
import { isEmbedOnlyModel } from '../lib/model-capabilities';
|
||||
|
||||
const STEPS = [
|
||||
{ key: 'ollama_ready', label: 'Inference Engine', icon: Cpu, detail: 'Starting Ollama...' },
|
||||
@@ -91,12 +92,14 @@ export function SetupScreen({ onReady }: { onReady: () => void }) {
|
||||
fetchRecommendedModel().catch(() => ({ model: '', reason: '' })),
|
||||
]);
|
||||
const store = useAppStore.getState();
|
||||
const hadSelection = !!store.selectedModel;
|
||||
store.setModels(models);
|
||||
store.setModelsLoading(false);
|
||||
const recommended = rec.model && models.some((m) => m.id === rec.model)
|
||||
const chatModels = models.filter((m) => !isEmbedOnlyModel(m.id));
|
||||
const recommended = rec.model && chatModels.some((m) => m.id === rec.model)
|
||||
? rec.model
|
||||
: models[0]?.id || '';
|
||||
if (recommended && !store.selectedModel) {
|
||||
: chatModels[0]?.id || '';
|
||||
if (recommended && !hadSelection) {
|
||||
store.setSelectedModel(recommended);
|
||||
}
|
||||
} catch {
|
||||
|
||||
@@ -0,0 +1,19 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import { isEmbedOnlyModel } from './model-capabilities';
|
||||
|
||||
describe('isEmbedOnlyModel', () => {
|
||||
it.each([
|
||||
'nomic-embed-text',
|
||||
'mxbai-embed-large',
|
||||
'text-embedding-3-small',
|
||||
'all-minilm:latest',
|
||||
'hf.co/BAAI/bge-m3:latest',
|
||||
])('classifies %s as embedding-only', (modelId) => {
|
||||
expect(isEmbedOnlyModel(modelId)).toBe(true);
|
||||
});
|
||||
|
||||
it.each(['qwen3.5:4b', 'codegemma:7b'])('keeps %s available for chat', (modelId) => {
|
||||
expect(isEmbedOnlyModel(modelId)).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,22 @@
|
||||
const EMBEDDING_MODEL_PREFIXES = [
|
||||
'all-minilm',
|
||||
'bge-',
|
||||
'bge_',
|
||||
'e5-',
|
||||
'e5_',
|
||||
'gte-',
|
||||
'gte_',
|
||||
'jina-embeddings',
|
||||
'nomic-bert',
|
||||
'sentence-transformers',
|
||||
];
|
||||
|
||||
export function isEmbedOnlyModel(modelId: string): boolean {
|
||||
const name = (modelId || '').trim().toLowerCase();
|
||||
const leaf = name.slice(name.lastIndexOf('/') + 1).split(':')[0];
|
||||
return (
|
||||
leaf.includes('embed') ||
|
||||
leaf.includes('minilm') ||
|
||||
EMBEDDING_MODEL_PREFIXES.some((prefix) => leaf.startsWith(prefix))
|
||||
);
|
||||
}
|
||||
@@ -0,0 +1,64 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import type { ModelInfo } from '../types';
|
||||
|
||||
class MemoryStorage {
|
||||
private store = new Map<string, string>();
|
||||
|
||||
getItem(key: string): string | null {
|
||||
return this.store.get(key) ?? null;
|
||||
}
|
||||
|
||||
setItem(key: string, value: string): void {
|
||||
this.store.set(key, String(value));
|
||||
}
|
||||
}
|
||||
|
||||
const model = (id: string): ModelInfo => ({
|
||||
id,
|
||||
object: 'model',
|
||||
created: 0,
|
||||
owned_by: 'openjarvis',
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetModules();
|
||||
(globalThis as unknown as { localStorage: MemoryStorage }).localStorage =
|
||||
new MemoryStorage();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
(globalThis as unknown as { localStorage?: MemoryStorage }).localStorage =
|
||||
undefined;
|
||||
});
|
||||
|
||||
describe('setModels', () => {
|
||||
it('does not select an embedding-only model', async () => {
|
||||
const { useAppStore } = await import('./store');
|
||||
|
||||
useAppStore.getState().setModels([model('nomic-embed-text')]);
|
||||
|
||||
expect(useAppStore.getState().selectedModel).toBe('');
|
||||
});
|
||||
|
||||
it('clears a missing selection when no chat fallback exists', async () => {
|
||||
const { useAppStore } = await import('./store');
|
||||
useAppStore.getState().setSelectedModel('deleted-chat-model');
|
||||
|
||||
useAppStore.getState().setModels([model('nomic-embed-text')]);
|
||||
|
||||
expect(useAppStore.getState().selectedModel).toBe('');
|
||||
});
|
||||
|
||||
it('replaces an embedding selection with an available chat model', async () => {
|
||||
const { useAppStore } = await import('./store');
|
||||
useAppStore.getState().setSelectedModel('all-minilm:latest');
|
||||
|
||||
useAppStore.getState().setModels([
|
||||
model('all-minilm:latest'),
|
||||
model('qwen3.5:4b'),
|
||||
]);
|
||||
|
||||
expect(useAppStore.getState().selectedModel).toBe('qwen3.5:4b');
|
||||
});
|
||||
});
|
||||
@@ -15,6 +15,7 @@ import type {
|
||||
TokenUsage,
|
||||
} from '../types';
|
||||
import type { ManagedAgent } from './api';
|
||||
import { isEmbedOnlyModel } from './model-capabilities';
|
||||
|
||||
export interface CachedConnector {
|
||||
connector_id: string;
|
||||
@@ -444,11 +445,36 @@ export const useAppStore = create<AppState>((set, get) => {
|
||||
// ── Models & server ────────────────────────────────────────────
|
||||
|
||||
setModels: (models: ModelInfo[]) =>
|
||||
set((state) =>
|
||||
!state.selectedModel && models.length > 0
|
||||
? { models, selectedModel: models[0].id }
|
||||
: { models },
|
||||
),
|
||||
set((state) => {
|
||||
// Ollama returns embed-only models (e.g. nomic-embed-text) in the
|
||||
// same list as chat models. Auto-picking models[0] selected the
|
||||
// embedder and every chat failed with HTTP 400 "does not support
|
||||
// chat". Prefer a real chat model for selection / fallback.
|
||||
const chatModels = models.filter((m) => !isEmbedOnlyModel(m.id));
|
||||
const preferred =
|
||||
(state.settings.defaultModel &&
|
||||
chatModels.some((m) => m.id === state.settings.defaultModel) &&
|
||||
state.settings.defaultModel) ||
|
||||
chatModels[0]?.id ||
|
||||
models.find((m) => !isEmbedOnlyModel(m.id))?.id ||
|
||||
'';
|
||||
|
||||
const currentIsBad =
|
||||
!!state.selectedModel && isEmbedOnlyModel(state.selectedModel);
|
||||
const currentMissing =
|
||||
!!state.selectedModel &&
|
||||
!models.some((m) => m.id === state.selectedModel);
|
||||
|
||||
if (!state.selectedModel || currentIsBad || currentMissing) {
|
||||
// Prefer a real chat model. If none exist, clear a bad/missing
|
||||
// selection rather than keeping an embed-only id that 400s on chat.
|
||||
return {
|
||||
models,
|
||||
selectedModel: preferred,
|
||||
};
|
||||
}
|
||||
return { models };
|
||||
}),
|
||||
setModelsLoading: (loading: boolean) => set({ modelsLoading: loading }),
|
||||
setSelectedModel: (model: string) => set({ selectedModel: model }),
|
||||
setServerInfo: (info: ServerInfo | null) => set({ serverInfo: info }),
|
||||
|
||||
@@ -20,6 +20,7 @@ from openjarvis.agents.tool_resolver import (
|
||||
from openjarvis.agents.tool_resolver import (
|
||||
ensure_registries_populated as _ensure_registries_populated,
|
||||
)
|
||||
from openjarvis.server.model_capabilities import is_embed_only_model
|
||||
|
||||
try:
|
||||
from fastapi import APIRouter, HTTPException, Request
|
||||
@@ -332,16 +333,29 @@ _CLOUD_PREFIXES = ("gpt-", "claude-", "gemini-", "o1-", "o3-", "o4-")
|
||||
def _pick_recommended_model(
|
||||
model_ids: list[str],
|
||||
) -> dict[str, str]:
|
||||
"""Pick the second-largest local model from a list."""
|
||||
local = [m for m in model_ids if not any(m.startswith(p) for p in _CLOUD_PREFIXES)]
|
||||
"""Pick the second-largest local *chat* model from a list.
|
||||
|
||||
Embedding-only models (nomic-embed-text, etc.) are excluded — they return
|
||||
HTTP 400 "does not support chat" when used as the generation model.
|
||||
"""
|
||||
local = [
|
||||
m
|
||||
for m in model_ids
|
||||
if not any(m.startswith(p) for p in _CLOUD_PREFIXES)
|
||||
and not is_embed_only_model(m)
|
||||
]
|
||||
if not local:
|
||||
# Fall back to any non-cloud model, still skipping embedders.
|
||||
local = [m for m in model_ids if not is_embed_only_model(m)]
|
||||
if not local:
|
||||
# Never recommend an embed-only model — chat would 400.
|
||||
return {
|
||||
"model": model_ids[0] if model_ids else "",
|
||||
"reason": "Only model available",
|
||||
"model": "",
|
||||
"reason": "No local chat model available",
|
||||
}
|
||||
sized = sorted(local, key=_parse_param_count, reverse=True)
|
||||
if len(sized) == 1:
|
||||
return {"model": sized[0], "reason": "Only local model available"}
|
||||
return {"model": sized[0], "reason": "Only local chat model available"}
|
||||
pick = sized[1] # second-largest
|
||||
params = _parse_param_count(pick)
|
||||
return {
|
||||
|
||||
@@ -0,0 +1,34 @@
|
||||
"""Model capability helpers shared by server model-selection routes."""
|
||||
|
||||
_EMBEDDING_MODEL_PREFIXES = (
|
||||
"all-minilm",
|
||||
"bge-",
|
||||
"bge_",
|
||||
"e5-",
|
||||
"e5_",
|
||||
"gte-",
|
||||
"gte_",
|
||||
"jina-embeddings",
|
||||
"nomic-bert",
|
||||
"sentence-transformers",
|
||||
)
|
||||
|
||||
|
||||
def is_embed_only_model(model_name: str) -> bool:
|
||||
"""Return whether a model identifier denotes a non-chat embedder.
|
||||
|
||||
Ollama does not expose capabilities through its model-list response, so
|
||||
model selection needs a conservative name-based guard. Most embedding
|
||||
models contain ``embed``; the explicit prefixes cover common families
|
||||
such as MiniLM, BGE, E5, and GTE whose names do not.
|
||||
"""
|
||||
name = (model_name or "").strip().lower()
|
||||
leaf = name.rsplit("/", 1)[-1].split(":", 1)[0]
|
||||
return (
|
||||
"embed" in leaf
|
||||
or "minilm" in leaf
|
||||
or leaf.startswith(_EMBEDDING_MODEL_PREFIXES)
|
||||
)
|
||||
|
||||
|
||||
__all__ = ["is_embed_only_model"]
|
||||
@@ -12,6 +12,7 @@ from fastapi.responses import StreamingResponse
|
||||
|
||||
from openjarvis.core.paths import get_config_dir
|
||||
from openjarvis.core.types import Message, Role
|
||||
from openjarvis.server.model_capabilities import is_embed_only_model
|
||||
from openjarvis.server.models import (
|
||||
ChatCompletionChunk,
|
||||
ChatCompletionRequest,
|
||||
@@ -892,6 +893,11 @@ async def list_models(request: Request) -> ModelListResponse:
|
||||
if not model_ids:
|
||||
model_ids = await list_local_models()
|
||||
|
||||
# Keep embed-only models out of the chat model picker. They still work for
|
||||
# memory/retrieval via the embedder path; putting them in /v1/models made
|
||||
# the UI auto-select nomic-embed-text and fail every generation with 400.
|
||||
model_ids = [m for m in model_ids if not is_embed_only_model(m)]
|
||||
|
||||
return ModelListResponse(
|
||||
data=[
|
||||
ModelObject(
|
||||
|
||||
@@ -265,6 +265,28 @@ class TestModelsEndpointExtended:
|
||||
assert "qwen3.5:9b" in ids
|
||||
assert "qwen3:0.6b" in ids
|
||||
|
||||
def test_models_list_filters_embedding_only_models(self):
|
||||
engine = _make_engine(
|
||||
models=["nomic-embed-text", "all-minilm:latest", "qwen3.5:4b"],
|
||||
)
|
||||
client = TestClient(create_app(engine, "qwen3.5:4b"))
|
||||
|
||||
resp = client.get("/v1/models")
|
||||
|
||||
assert resp.status_code == 200
|
||||
assert [m["id"] for m in resp.json()["data"]] == ["qwen3.5:4b"]
|
||||
|
||||
def test_models_list_returns_empty_when_only_embedders_are_installed(self):
|
||||
engine = _make_engine(
|
||||
models=["nomic-embed-text", "hf.co/BAAI/bge-m3:latest"],
|
||||
)
|
||||
client = TestClient(create_app(engine, "nomic-embed-text"))
|
||||
|
||||
resp = client.get("/v1/models")
|
||||
|
||||
assert resp.status_code == 200
|
||||
assert resp.json()["data"] == []
|
||||
|
||||
def test_models_empty_engine(self):
|
||||
"""When engine.list_models() returns empty, endpoint still succeeds."""
|
||||
engine = _make_engine(models=[])
|
||||
|
||||
@@ -50,3 +50,42 @@ def test_parse_param_count():
|
||||
assert _parse_param_count("qwen3.5:0.8b") == 0.8
|
||||
assert _parse_param_count("qwen3.5:35b") == 35.0
|
||||
assert _parse_param_count("gpt-4o") == 0.0
|
||||
|
||||
|
||||
@pytest.mark.skipif(not HAS_FASTAPI, reason="fastapi not installed")
|
||||
def test_recommended_model_skips_embed_only():
|
||||
"""Embed-only models must never be recommended for chat."""
|
||||
from openjarvis.server.agent_manager_routes import _pick_recommended_model
|
||||
|
||||
models = [
|
||||
"nomic-embed-text",
|
||||
"qwen3.5:4b",
|
||||
"mxbai-embed-large",
|
||||
"qwen3.5:9b",
|
||||
]
|
||||
result = _pick_recommended_model(models)
|
||||
assert result["model"] == "qwen3.5:4b"
|
||||
assert "embed" not in result["model"]
|
||||
|
||||
|
||||
@pytest.mark.skipif(not HAS_FASTAPI, reason="fastapi not installed")
|
||||
def test_recommended_model_embed_only_returns_empty():
|
||||
"""If only embedders are installed, recommend nothing (not nomic-embed)."""
|
||||
from openjarvis.server.agent_manager_routes import _pick_recommended_model
|
||||
|
||||
result = _pick_recommended_model(["nomic-embed-text", "mxbai-embed-large"])
|
||||
assert result["model"] == ""
|
||||
assert "No local chat model" in result["reason"]
|
||||
|
||||
|
||||
@pytest.mark.skipif(not HAS_FASTAPI, reason="fastapi not installed")
|
||||
def test_is_embed_only_model():
|
||||
from openjarvis.server.model_capabilities import is_embed_only_model
|
||||
|
||||
assert is_embed_only_model("nomic-embed-text")
|
||||
assert is_embed_only_model("mxbai-embed-large")
|
||||
assert is_embed_only_model("text-embedding-3-small")
|
||||
assert is_embed_only_model("all-minilm:latest")
|
||||
assert is_embed_only_model("hf.co/BAAI/bge-m3:latest")
|
||||
assert not is_embed_only_model("qwen3.5:4b")
|
||||
assert not is_embed_only_model("codegemma:7b")
|
||||
|
||||
Reference in New Issue
Block a user