mirror of
https://github.com/blakeblackshear/frigate.git
synced 2026-09-27 08:28:58 +03:00
fix genai settings keeping a stale model and dropping roles after save
Switching a GenAI entry's provider left the previous provider's model selected, so saving wrote a model the new provider doesn't serve. llama.cpp can't find that model in `/v1/models`, so the backend reported every capability as false for the entry, and once the save refetched `genai/models` the roles widget stripped `transcribe` from the form on its own. The section showed unsaved changes right after saving, and saving again would have dropped the role. Switching provider now clears the model, and the roles widget only strips a role for a model or provider picked in the form, since the entry-level capability flags only describe the saved model. A selected role stays visible when the provider can't confirm it, so it can still be switched off. The llama.cpp model list also no longer repeats a model whose alias matches its id, which is what `--alias` produces.
This commit is contained in:
@@ -560,13 +560,12 @@ class LlamaCppClient(GenAIClient):
|
||||
|
||||
def list_models(self) -> list[str]:
|
||||
"""Return available model IDs from the llama.cpp server."""
|
||||
models = []
|
||||
models: set[str] = set()
|
||||
|
||||
# llama-server lists the id among the aliases when --alias is set
|
||||
for m in self._fetch_models_data():
|
||||
models.append(m.get("id", "unknown"))
|
||||
|
||||
for alias in m.get("aliases", []):
|
||||
models.append(alias)
|
||||
models.add(m.get("id", "unknown"))
|
||||
models.update(m.get("aliases", []))
|
||||
|
||||
return sorted(models)
|
||||
|
||||
|
||||
@@ -580,6 +580,15 @@ class TestLlamaCppProvider(unittest.TestCase):
|
||||
client = self._validated_client(4096, {"context_size": 32768})
|
||||
self.assertEqual(client.get_context_size(), 32768)
|
||||
|
||||
def test_list_models_dedupes_alias_matching_id(self):
|
||||
client = self._client()
|
||||
models_data = [
|
||||
{"id": "qwen3-asr", "aliases": ["qwen3-asr"]},
|
||||
{"id": "gemma", "aliases": ["gemma", "g4"]},
|
||||
]
|
||||
with patch.object(client, "_fetch_models_data", return_value=models_data):
|
||||
self.assertEqual(client.list_models(), ["g4", "gemma", "qwen3-asr"])
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# transcribe role
|
||||
|
||||
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
@@ -0,0 +1,157 @@
|
||||
/**
|
||||
* Generative AI provider settings tests -- MEDIUM tier.
|
||||
*
|
||||
* A model name belongs to its provider, so switching provider clears the model
|
||||
* field. The roles widget strips a role only for a model or provider picked in
|
||||
* the form, never when capability data arrives for the saved entry, which would
|
||||
* dirty the section on load and silently drop the role on the next save.
|
||||
*/
|
||||
|
||||
import { readFileSync } from "node:fs";
|
||||
import { resolve, dirname } from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { test, expect } from "../../fixtures/frigate-test";
|
||||
import type { Page } from "@playwright/test";
|
||||
import { configFactory } from "../../fixtures/mock-data/config";
|
||||
|
||||
const __dirname = dirname(fileURLToPath(import.meta.url));
|
||||
const CONFIG_SCHEMA = JSON.parse(
|
||||
readFileSync(
|
||||
resolve(__dirname, "../../fixtures/mock-data/config-schema.json"),
|
||||
"utf-8",
|
||||
),
|
||||
);
|
||||
|
||||
const ENTRY = "audio";
|
||||
const SETTINGS_URL = "/settings?page=integrationGenerativeAi";
|
||||
const UNSAVED = "You have unsaved changes";
|
||||
const MODEL_PLACEHOLDER = "Select or enter a model…";
|
||||
|
||||
type Entry = {
|
||||
provider: string;
|
||||
model: string;
|
||||
base_url?: string;
|
||||
roles: string[];
|
||||
};
|
||||
|
||||
type ProviderInfo = {
|
||||
models: string[];
|
||||
supports_transcription: boolean;
|
||||
model_capabilities?: Record<string, { supports_transcription?: boolean }>;
|
||||
};
|
||||
|
||||
async function installRoutes(page: Page, entry: Entry, info: ProviderInfo) {
|
||||
const config = configFactory({ genai: { [ENTRY]: entry } });
|
||||
|
||||
await page.route("**/api/config/schema.json", (route) =>
|
||||
route.fulfill({ json: CONFIG_SCHEMA }),
|
||||
);
|
||||
await page.route("**/api/config", (route) => {
|
||||
if (route.request().method() === "GET") {
|
||||
return route.fulfill({ json: config });
|
||||
}
|
||||
return route.fulfill({ json: { success: true } });
|
||||
});
|
||||
await page.route("**/api/config/raw_paths", (route) =>
|
||||
route.fulfill({ json: { genai: { [ENTRY]: entry } } }),
|
||||
);
|
||||
await page.route("**/api/genai/models", (route) =>
|
||||
route.fulfill({
|
||||
json: {
|
||||
[ENTRY]: {
|
||||
roles: entry.roles,
|
||||
supports_toggleable_thinking: false,
|
||||
supports_embeddings: true,
|
||||
model_capabilities: {},
|
||||
...info,
|
||||
},
|
||||
},
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
function roleSwitch(page: Page, role: string) {
|
||||
return page.locator(`#root_${ENTRY}_roles-${role}`);
|
||||
}
|
||||
|
||||
test.describe("genai provider settings @medium", () => {
|
||||
test("a saved role the provider cannot confirm stays and is not dirty", async ({
|
||||
frigateApp,
|
||||
}) => {
|
||||
// The server does not serve the saved model, so the backend reports every
|
||||
// capability as false for the entry.
|
||||
await installRoutes(
|
||||
frigateApp.page,
|
||||
{
|
||||
provider: "llamacpp",
|
||||
model: "stale-model",
|
||||
base_url: "http://llama:8080",
|
||||
roles: ["transcribe"],
|
||||
},
|
||||
{ models: ["qwen3-asr"], supports_transcription: false },
|
||||
);
|
||||
await frigateApp.goto(SETTINGS_URL);
|
||||
|
||||
await expect(roleSwitch(frigateApp.page, "transcribe")).toBeVisible();
|
||||
await expect(roleSwitch(frigateApp.page, "transcribe")).toBeChecked();
|
||||
|
||||
// Give any stripping effect time to fire, then confirm the section stayed
|
||||
// clean.
|
||||
await frigateApp.page.waitForTimeout(1000);
|
||||
await expect(frigateApp.page.getByText(UNSAVED)).toBeHidden();
|
||||
await expect(roleSwitch(frigateApp.page, "transcribe")).toBeChecked();
|
||||
});
|
||||
|
||||
test("switching provider clears the model", async ({ frigateApp }) => {
|
||||
await installRoutes(
|
||||
frigateApp.page,
|
||||
{
|
||||
provider: "openai",
|
||||
model: "gpt-4o",
|
||||
roles: ["descriptions"],
|
||||
},
|
||||
{ models: ["gpt-4o"], supports_transcription: true },
|
||||
);
|
||||
await frigateApp.goto(SETTINGS_URL);
|
||||
|
||||
const model = frigateApp.page.locator(`#root_${ENTRY}_model`);
|
||||
await expect(model).toHaveText("gpt-4o");
|
||||
|
||||
await frigateApp.page.locator(`#root_${ENTRY}_provider`).click();
|
||||
await frigateApp.page.getByRole("option", { name: "llamacpp" }).click();
|
||||
|
||||
await expect(model).toHaveText(MODEL_PLACEHOLDER);
|
||||
await expect(frigateApp.page.getByText(UNSAVED)).toBeVisible();
|
||||
});
|
||||
|
||||
test("picking a model that cannot transcribe strips the role", async ({
|
||||
frigateApp,
|
||||
}) => {
|
||||
await installRoutes(
|
||||
frigateApp.page,
|
||||
{
|
||||
provider: "llamacpp",
|
||||
model: "qwen3-asr",
|
||||
base_url: "http://llama:8080",
|
||||
roles: ["transcribe"],
|
||||
},
|
||||
{
|
||||
models: ["qwen3-asr", "text-only"],
|
||||
supports_transcription: true,
|
||||
model_capabilities: {
|
||||
"qwen3-asr": { supports_transcription: true },
|
||||
"text-only": { supports_transcription: false },
|
||||
},
|
||||
},
|
||||
);
|
||||
await frigateApp.goto(SETTINGS_URL);
|
||||
|
||||
await expect(roleSwitch(frigateApp.page, "transcribe")).toBeChecked();
|
||||
|
||||
await frigateApp.page.locator(`#root_${ENTRY}_model`).click();
|
||||
await frigateApp.page.getByRole("option", { name: "text-only" }).click();
|
||||
|
||||
await expect(roleSwitch(frigateApp.page, "transcribe")).toBeHidden();
|
||||
await expect(frigateApp.page.getByText(UNSAVED)).toBeVisible();
|
||||
});
|
||||
});
|
||||
@@ -59,20 +59,30 @@ export function GenAIModelWidget(props: WidgetProps) {
|
||||
|
||||
const formContext = registry?.formContext as ConfigFormContext | undefined;
|
||||
|
||||
// Build a fingerprint from the saved config's provider + base_url so the
|
||||
// SWR key changes (and models are refetched) whenever those fields are saved.
|
||||
const configFingerprint = useMemo(() => {
|
||||
if (!providerKey) return "";
|
||||
const savedEntry = useMemo<Record<string, unknown> | null>(() => {
|
||||
if (!providerKey) return null;
|
||||
const genai = (
|
||||
formContext?.fullConfig as Record<string, unknown> | undefined
|
||||
)?.genai;
|
||||
if (!genai || typeof genai !== "object" || Array.isArray(genai)) return "";
|
||||
if (!genai || typeof genai !== "object" || Array.isArray(genai)) {
|
||||
return null;
|
||||
}
|
||||
const entry = (genai as Record<string, unknown>)[providerKey];
|
||||
if (!entry || typeof entry !== "object" || Array.isArray(entry)) return "";
|
||||
const e = entry as Record<string, unknown>;
|
||||
return `${e.provider ?? ""}|${e.base_url ?? ""}`;
|
||||
if (!entry || typeof entry !== "object" || Array.isArray(entry)) {
|
||||
return null;
|
||||
}
|
||||
return entry as Record<string, unknown>;
|
||||
}, [providerKey, formContext?.fullConfig]);
|
||||
|
||||
const savedProvider =
|
||||
typeof savedEntry?.provider === "string" ? savedEntry.provider : null;
|
||||
|
||||
// Build a fingerprint from the saved config's provider + base_url so the
|
||||
// SWR key changes (and models are refetched) whenever those fields are saved.
|
||||
const configFingerprint = savedEntry
|
||||
? `${savedEntry.provider ?? ""}|${savedEntry.base_url ?? ""}`
|
||||
: "";
|
||||
|
||||
const { data: allModels, mutate: mutateModels } = useSWR<GenAIModelsResponse>(
|
||||
"genai/models",
|
||||
{
|
||||
@@ -148,6 +158,17 @@ export function GenAIModelWidget(props: WidgetProps) {
|
||||
typeof formEntry?.provider === "string" ? formEntry.provider : null;
|
||||
const canProbe = Boolean(formProvider) && !probing;
|
||||
|
||||
// A model name belongs to its provider, so switching provider clears it.
|
||||
// Returning to the saved provider (including a form reset) leaves it alone.
|
||||
const prevFormProvider = useRef(formProvider);
|
||||
useEffect(() => {
|
||||
const previous = prevFormProvider.current;
|
||||
prevFormProvider.current = formProvider;
|
||||
|
||||
if (previous === formProvider || formProvider === savedProvider) return;
|
||||
if (typeof value === "string" && value) onChange("");
|
||||
}, [formProvider, savedProvider, value, onChange]);
|
||||
|
||||
const probe = async () => {
|
||||
if (!formEntry || !formProvider) return;
|
||||
if (probeSuccessTimerRef.current) {
|
||||
|
||||
@@ -25,6 +25,22 @@ function normalizeValue(value: unknown): string[] {
|
||||
return [];
|
||||
}
|
||||
|
||||
function getString(value: unknown): string | undefined {
|
||||
return typeof value === "string" && value ? value : undefined;
|
||||
}
|
||||
|
||||
function getEntry(
|
||||
entries: unknown,
|
||||
providerKey: string | undefined,
|
||||
): Record<string, unknown> | undefined {
|
||||
if (!providerKey || !entries || typeof entries !== "object") return undefined;
|
||||
const entry = (entries as Record<string, unknown>)[providerKey];
|
||||
if (!entry || typeof entry !== "object" || Array.isArray(entry)) {
|
||||
return undefined;
|
||||
}
|
||||
return entry as Record<string, unknown>;
|
||||
}
|
||||
|
||||
function getProviderKey(widgetId: string): string | undefined {
|
||||
const prefix = "root_";
|
||||
const suffix = "_roles";
|
||||
@@ -51,21 +67,26 @@ export function GenAIRolesWidget(props: WidgetProps) {
|
||||
// The model currently chosen in the form, which is what the roles have to
|
||||
// reflect. Reading the saved config instead would keep reporting the previous
|
||||
// model's capabilities until a save and a refetch.
|
||||
const selectedModel = useMemo(() => {
|
||||
if (!providerKey) return undefined;
|
||||
const formData = formContext?.formData as
|
||||
Record<string, unknown> | undefined;
|
||||
const entry = formData?.[providerKey];
|
||||
if (!entry || typeof entry !== "object" || Array.isArray(entry)) {
|
||||
return undefined;
|
||||
}
|
||||
const model = (entry as Record<string, unknown>).model;
|
||||
return typeof model === "string" && model ? model : undefined;
|
||||
}, [formContext?.formData, providerKey]);
|
||||
const formEntry = useMemo(
|
||||
() => getEntry(formContext?.formData, providerKey),
|
||||
[formContext?.formData, providerKey],
|
||||
);
|
||||
const savedEntry = useMemo(
|
||||
() => getEntry(formContext?.fullConfig?.genai, providerKey),
|
||||
[formContext?.fullConfig?.genai, providerKey],
|
||||
);
|
||||
|
||||
const selectedModel = getString(formEntry?.model);
|
||||
|
||||
// The entry-level capability flags describe the saved provider and model
|
||||
// only, so they apply while the form still matches the saved entry.
|
||||
const matchesSaved =
|
||||
savedEntry !== undefined &&
|
||||
getString(formEntry?.provider) === getString(savedEntry.provider) &&
|
||||
selectedModel === getString(savedEntry.model);
|
||||
|
||||
// Capabilities the provider reported for that specific model. Absent when the
|
||||
// provider cannot describe a model it has not loaded, in which case the
|
||||
// entry-level flags (which describe the saved model) are the best available.
|
||||
// provider cannot describe a model it has not loaded.
|
||||
const modelCapabilities: GenAIModelCapabilities | undefined = useMemo(() => {
|
||||
if (!providerKey || !selectedModel) return undefined;
|
||||
return genaiInfo?.[providerKey]?.model_capabilities?.[selectedModel];
|
||||
@@ -76,7 +97,7 @@ export function GenAIRolesWidget(props: WidgetProps) {
|
||||
): boolean => {
|
||||
const perModel = modelCapabilities?.[key];
|
||||
if (perModel !== undefined) return perModel;
|
||||
if (!providerKey) return true;
|
||||
if (!providerKey || !matchesSaved) return true;
|
||||
const info = genaiInfo?.[providerKey];
|
||||
// assume supported when nothing is known, so a role is never hidden on
|
||||
// missing information alone
|
||||
@@ -95,9 +116,13 @@ export function GenAIRolesWidget(props: WidgetProps) {
|
||||
return unsupported;
|
||||
}, [embeddingsSupported, transcriptionSupported]);
|
||||
|
||||
// a selected role stays visible so it can still be switched off
|
||||
const availableRoles = useMemo(
|
||||
() => GENAI_ROLES.filter((role) => !unsupportedRoles.has(role)),
|
||||
[unsupportedRoles],
|
||||
() =>
|
||||
GENAI_ROLES.filter(
|
||||
(role) => !unsupportedRoles.has(role) || selectedRoles.includes(role),
|
||||
),
|
||||
[unsupportedRoles, selectedRoles],
|
||||
);
|
||||
|
||||
const occupiedRoles = useMemo(() => {
|
||||
@@ -123,13 +148,16 @@ export function GenAIRolesWidget(props: WidgetProps) {
|
||||
return occupied;
|
||||
}, [formContext?.formData, providerKey]);
|
||||
|
||||
// strip every unsupported role in a single onChange; two effects each
|
||||
// rewriting the same value would race and lose one of the edits
|
||||
// Strip every unsupported role in a single onChange; two effects each
|
||||
// rewriting the same value would race and lose one of the edits. Only a
|
||||
// model or provider picked in the form can rule a role out, so capability
|
||||
// data arriving for the saved entry never edits the form on its own.
|
||||
useEffect(() => {
|
||||
if (matchesSaved) return;
|
||||
if (!selectedRoles.some((role) => unsupportedRoles.has(role))) return;
|
||||
|
||||
onChange(selectedRoles.filter((role) => !unsupportedRoles.has(role)));
|
||||
}, [unsupportedRoles, selectedRoles, onChange]);
|
||||
}, [matchesSaved, unsupportedRoles, selectedRoles, onChange]);
|
||||
|
||||
const toggleRole = (role: string, enabled: boolean) => {
|
||||
if (enabled) {
|
||||
|
||||
Reference in New Issue
Block a user