diff --git a/.agents/adding-gallery-models.md b/.agents/adding-gallery-models.md index 155495e62..a52daddbb 100644 --- a/.agents/adding-gallery-models.md +++ b/.agents/adding-gallery-models.md @@ -113,9 +113,9 @@ Rules: - A variant references another gallery entry **by name**. That entry must exist and must not declare `variants` of its own. - **A referenced entry keeps its own gallery row by default.** It is hidden only - in the collapsed listing (`collapse_variants=true`, the UI's "One row per - model" toggle), where the declaring entry stands in for it, so referencing an - entry never makes it unreachable. + in the collapsed listing (`collapse_variants=true`, which the web UI always + requests), where the declaring entry stands in for it. A search term bypasses + the collapse, so referencing an entry never makes it unfindable by name. - **Order carries no meaning.** Do not try to encode a preference; write the list in whatever order reads best. - **A variant may be smaller than the declaring entry.** Offering a downgrade diff --git a/core/http/react-ui/e2e/models-gallery.spec.js b/core/http/react-ui/e2e/models-gallery.spec.js index 40ac56acb..5556fa239 100644 --- a/core/http/react-ui/e2e/models-gallery.spec.js +++ b/core/http/react-ui/e2e/models-gallery.spec.js @@ -737,13 +737,18 @@ const COLLAPSED_RESPONSE = { currentPage: 1, }; -test.describe("Models Gallery - Collapse Variants Filter", () => { - let listingUrls; +// What a search for the hidden build gets back: the build itself, even though +// browsing would have collapsed it away behind its parent. +const SEARCH_HIT_RESPONSE = { + ...MOCK_MODELS_RESPONSE, + models: MOCK_MODELS_RESPONSE.models.filter((m) => m.name === "whisper-model"), + availableModels: 1, + totalPages: 1, + currentPage: 1, +}; - const collapseToggle = (page) => - page - .locator("label.filter-bar-group__toggle", { hasText: "One row per model" }) - .locator(".toggle__track"); +test.describe("Models Gallery - Collapsed Listing", () => { + let listingUrls; test.beforeEach(async ({ page }) => { listingUrls = []; @@ -753,7 +758,7 @@ test.describe("Models Gallery - Collapse Variants Filter", () => { // Only the gallery's own listing. Sibling routes like // /api/models/estimate share the prefix, and the recommended-models // panel queries /api/models itself with its own page size, so neither - // must pollute the record of what the filter sent, nor pick up the + // must pollute the record of what the page sent, nor pick up the // narrowed bodies below. const isListing = url.pathname.endsWith("/api/models") && @@ -761,12 +766,17 @@ test.describe("Models Gallery - Collapse Variants Filter", () => { if (isListing) { listingUrls.push(url); } - const collapsed = url.searchParams.get("collapse_variants") === "true"; + const term = (url.searchParams.get("term") || "").trim(); + const collapsed = + url.searchParams.get("collapse_variants") === "true" && term === ""; const tag = url.searchParams.get("tag"); - // A usecase filter matches nothing in this fixture either way, so the - // empty state is reachable in both toggle states and the specs can pin - // down what it says about each. + // Stands in for the server: an explicit search term bypasses the + // collapse, so a build a parent already offers is still findable by + // name. Anything else browsing-shaped stays collapsed. let body = collapsed ? COLLAPSED_RESPONSE : MOCK_MODELS_RESPONSE; + if (isListing && term === "whisper-model") body = SEARCH_HIT_RESPONSE; + // A usecase filter matches nothing in this fixture, so the empty state + // stays reachable and the specs can pin down what it says. if (isListing && tag) body = EMPTY_FILTERED_RESPONSE; route.fulfill({ contentType: "application/json", @@ -780,18 +790,23 @@ test.describe("Models Gallery - Collapse Variants Filter", () => { }); }); - test("the toggle is visible", async ({ page }) => { - await expect(page.getByText("One row per model")).toBeVisible(); + test("there is no collapse toggle to find", async ({ page }) => { + // The control is redundant once searching bypasses the collapse, and a + // toggle whose only job is "let me find things" is a worse answer than + // the search box already being able to find them. + await expect(page.getByText("One row per model")).toHaveCount(0); + await expect( + page.locator("label.filter-bar-group__toggle", { + hasText: "One row per model", + }), + ).toHaveCount(0); }); - test("defaults to on, sending collapse_variants=true and collapsing the list", async ({ + test("browsing collapses: the parent stays, the build it offers drops", async ({ page, }) => { - await expect(page.getByLabel("One row per model")).toBeChecked(); - - // The parent keeps its row, the build it already offers drops out, and an - // entry that is nobody's variant is untouched. A filter that kept only the - // entries declaring variants would wrongly drop stablediffusion-model too. + // A filter that kept only the entries declaring variants would wrongly + // drop stablediffusion-model too. await expect(page.locator("tr", { hasText: "llama-model" })).toBeVisible(); await expect(page.locator("tr", { hasText: "whisper-model" })).toHaveCount( 0, @@ -801,55 +816,71 @@ test.describe("Models Gallery - Collapse Variants Filter", () => { ).toBeVisible(); // Asserted over every listing request, so a first paint that fetched the - // uncollapsed listing before settling on the default would still fail. + // uncollapsed listing before settling would still fail. expect(listingUrls.length).toBeGreaterThan(0); for (const url of listingUrls) { expect(url.searchParams.get("collapse_variants")).toBe("true"); } }); - test("a stored preference from before the default flipped is not honoured", async ({ + test("searching a build the collapse hides still finds it", async ({ page, }) => { - // The previous build wrote '0' from an effect that runs on mount, so it - // marks a visit rather than a choice. Reading it as one would leave every - // earlier visitor on the old default forever. - await page.evaluate(() => { - localStorage.setItem("localai-models-collapse-variants-filter", "0"); - }); - await page.reload(); + // The regression this whole change exists to prevent: typing the name of + // an entry the gallery does hold must never answer "no models found", + // which reads as "that model does not exist". + await page.locator(".search-bar input").fill("whisper-model"); + + await expect( + page.locator("tr", { hasText: "whisper-model" }), + ).toBeVisible(); + await expect(page.locator(".empty-state")).toHaveCount(0); + }); + + test("the search term is sent alongside the collapse, not instead of it", async ({ + page, + }) => { + // The server decides what an active search means. The page keeps asking + // for the collapsed listing so that decision lives in one place, and so + // clearing the box goes straight back to the browsing view. + await page.locator(".search-bar input").fill("whisper-model"); + await expect.poll( + () => listingUrls[listingUrls.length - 1].searchParams.get("term"), + ).toBe("whisper-model"); + + const searched = listingUrls[listingUrls.length - 1]; + expect(searched.searchParams.get("collapse_variants")).toBe("true"); + }); + + test("clearing the search box returns to the collapsed listing", async ({ + page, + }) => { + await page.locator(".search-bar input").fill("whisper-model"); + await expect( + page.locator("tr", { hasText: "whisper-model" }), + ).toBeVisible(); + + await page.locator(".search-bar input").fill(""); - await expect(page.getByLabel("One row per model")).toBeChecked(); await expect(page.locator("tr", { hasText: "whisper-model" })).toHaveCount( 0, ); + await expect(page.locator("tr", { hasText: "llama-model" })).toBeVisible(); }); - test("turning it off drops the param and restores the full list", async ({ + test("a stored preference from the removed toggle is inert", async ({ page, }) => { - await collapseToggle(page).click(); - - await expect( - page.locator("tr", { hasText: "whisper-model" }), - ).toBeVisible(); - await expect( - page.locator("tr", { hasText: "stablediffusion-model" }), - ).toBeVisible(); - - // Omitted rather than sent as false, so opting out asks for exactly the - // listing every other API client gets. - const last = listingUrls[listingUrls.length - 1]; - expect(last.searchParams.has("collapse_variants")).toBe(false); - }); - - test("turning it back on collapses the list again", async ({ page }) => { - await collapseToggle(page).click(); - await expect( - page.locator("tr", { hasText: "whisper-model" }), - ).toBeVisible(); - - await collapseToggle(page).click(); + // The key outlives the control it belonged to. A user who left the toggle + // off gets the collapsed view like everyone else rather than a listing + // shaped by a setting they can no longer see or change. + await page.evaluate(() => { + localStorage.setItem("localai-models-collapse-variants-filter", "off"); + }); + await page.reload(); + await expect(page.locator("th", { hasText: "Backend" })).toBeVisible({ + timeout: 10_000, + }); await expect(page.locator("tr", { hasText: "whisper-model" })).toHaveCount( 0, @@ -858,29 +889,7 @@ test.describe("Models Gallery - Collapse Variants Filter", () => { expect(last.searchParams.get("collapse_variants")).toBe("true"); }); - test("resets to page 1 when toggled", async ({ page }) => { - await collapseToggle(page).click(); - const last = listingUrls[listingUrls.length - 1]; - // A filter that widened or narrowed the rows while holding page 3 would - // strand the user on a page the new set may not have. - expect(last.searchParams.get("page") || "1").toBe("1"); - }); - - test("turning it off persists after reload, like the other filters", async ({ - page, - }) => { - await collapseToggle(page).click(); - await page.reload(); - - await expect(page.getByLabel("One row per model")).not.toBeChecked(); - await expect( - page.locator("tr", { hasText: "whisper-model" }), - ).toBeVisible(); - const last = listingUrls[listingUrls.length - 1]; - expect(last.searchParams.has("collapse_variants")).toBe(false); - }); - - test("mentions the collapsed view in the empty state as a hint, not a cause", async ({ + test("the empty state no longer blames a toggle nobody can reach", async ({ page, }) => { await page.locator(".filter-btn", { hasText: "Chat" }).click(); @@ -888,37 +897,22 @@ test.describe("Models Gallery - Collapse Variants Filter", () => { await expect(page.locator(".empty-state-title")).toHaveText( "No models found", ); - // Collapsing is the default, so it cannot be blamed for the empty result - // the way an opted-into filter could; the filters get the top line and the - // toggle only gets a hint underneath. await expect(page.locator(".empty-state-text")).toHaveText( "No models match your current search or filters.", ); - await expect(page.locator(".empty-state-hint")).toHaveText( - 'Alternative builds that another entry already offers are hidden. Turn off "One row per model" to include them.', - ); - }); - - test("no collapsed-view hint once the toggle is off", async ({ page }) => { - await collapseToggle(page).click(); - await page.locator(".filter-btn", { hasText: "Chat" }).click(); - await expect(page.locator(".empty-state")).toBeVisible(); - - // Nothing is hidden any more, so the hint would be pointing at a toggle - // that is already doing what it suggests. + // The old hint told the user to turn off a control that is gone, and it + // is no longer even true for a search: searching sees every build. await expect(page.locator(".empty-state-hint")).toHaveCount(0); }); - test("clear filters restores the collapsed default", async ({ page }) => { - await collapseToggle(page).click(); + test("clear filters returns to the collapsed browsing view", async ({ + page, + }) => { await page.locator(".filter-btn", { hasText: "Chat" }).click(); await expect(page.locator(".empty-state")).toBeVisible(); await page.getByRole("button", { name: "Clear filters" }).click(); - // Clearing filters means going back to the view a fresh visit gets, which - // is now the collapsed one. - await expect(page.getByLabel("One row per model")).toBeChecked(); await expect(page.locator("tr", { hasText: "llama-model" })).toBeVisible(); await expect(page.locator("tr", { hasText: "whisper-model" })).toHaveCount( 0, diff --git a/core/http/react-ui/public/locales/de/models.json b/core/http/react-ui/public/locales/de/models.json index 8047c187f..9142a1b8b 100644 --- a/core/http/react-ui/public/locales/de/models.json +++ b/core/http/react-ui/public/locales/de/models.json @@ -24,7 +24,6 @@ "embedding": "Embedding", "rerank": "Rerank", "fitsGpu": "Passt in die GPU", - "collapseVariants": "Eine Zeile pro Modell", "allBackends": "Alle Backends", "searchBackends": "Backends suchen..." }, @@ -72,7 +71,6 @@ "empty": { "title": "Keine Modelle gefunden", "withFilters": "Keine Modelle entsprechen den aktuellen Such- oder Filterkriterien.", - "collapsedVariantsHint": "Alternative Builds, die ein anderer Eintrag bereits anbietet, sind ausgeblendet. Schalte \"Eine Zeile pro Modell\" aus, um sie einzubeziehen.", "noFilters": "Die Modellgalerie ist leer." }, "deleteDialog": { diff --git a/core/http/react-ui/public/locales/en/models.json b/core/http/react-ui/public/locales/en/models.json index ea86cc6aa..ec5180f3b 100644 --- a/core/http/react-ui/public/locales/en/models.json +++ b/core/http/react-ui/public/locales/en/models.json @@ -43,7 +43,6 @@ "vad": "VAD", "ner": "NER", "fitsGpu": "Fits in GPU", - "collapseVariants": "One row per model", "allBackends": "All Backends", "searchBackends": "Search backends..." }, @@ -91,7 +90,6 @@ "empty": { "title": "No models found", "withFilters": "No models match your current search or filters.", - "collapsedVariantsHint": "Alternative builds that another entry already offers are hidden. Turn off \"One row per model\" to include them.", "noFilters": "The model gallery is empty." }, "deleteDialog": { diff --git a/core/http/react-ui/public/locales/es/models.json b/core/http/react-ui/public/locales/es/models.json index 13e8428c7..69ca19dae 100644 --- a/core/http/react-ui/public/locales/es/models.json +++ b/core/http/react-ui/public/locales/es/models.json @@ -24,7 +24,6 @@ "embedding": "Embedding", "rerank": "Rerank", "fitsGpu": "Cabe en la GPU", - "collapseVariants": "Una fila por modelo", "allBackends": "Todos los backends", "searchBackends": "Buscar backends..." }, @@ -72,7 +71,6 @@ "empty": { "title": "No se encontraron modelos", "withFilters": "Ningún modelo coincide con la búsqueda o filtros actuales.", - "collapsedVariantsHint": "Las compilaciones alternativas que otra entrada ya ofrece están ocultas. Desactiva \"Una fila por modelo\" para incluirlas.", "noFilters": "La galería de modelos está vacía." }, "deleteDialog": { diff --git a/core/http/react-ui/public/locales/id/models.json b/core/http/react-ui/public/locales/id/models.json index 32356075c..3acff3afb 100644 --- a/core/http/react-ui/public/locales/id/models.json +++ b/core/http/react-ui/public/locales/id/models.json @@ -31,7 +31,6 @@ "detection": "Deteksi", "vad": "VAD", "fitsGpu": "Muat di GPU", - "collapseVariants": "Satu baris per model", "allBackends": "Semua Backend", "searchBackends": "Cari backends..." }, @@ -79,7 +78,6 @@ "empty": { "title": "Model tidak ditemukan", "withFilters": "Tidak ada model yang cocok dengan pencarian atau filter Anda.", - "collapsedVariantsHint": "Build alternatif yang sudah ditawarkan entri lain disembunyikan. Matikan \"Satu baris per model\" untuk menyertakannya.", "noFilters": "Galeri model kosong." }, "deleteDialog": { diff --git a/core/http/react-ui/public/locales/it/models.json b/core/http/react-ui/public/locales/it/models.json index 564eeaa73..15eca936f 100644 --- a/core/http/react-ui/public/locales/it/models.json +++ b/core/http/react-ui/public/locales/it/models.json @@ -24,7 +24,6 @@ "embedding": "Embedding", "rerank": "Rerank", "fitsGpu": "Entra nella GPU", - "collapseVariants": "Una riga per modello", "allBackends": "Tutti i backend", "searchBackends": "Cerca backend..." }, @@ -72,7 +71,6 @@ "empty": { "title": "Nessun modello trovato", "withFilters": "Nessun modello corrisponde ai filtri attuali.", - "collapsedVariantsHint": "Le build alternative gia offerte da un'altra voce sono nascoste. Disattiva \"Una riga per modello\" per includerle.", "noFilters": "La galleria modelli è vuota." }, "deleteDialog": { diff --git a/core/http/react-ui/public/locales/zh-CN/models.json b/core/http/react-ui/public/locales/zh-CN/models.json index 3d9c2a3ca..88cd49d7d 100644 --- a/core/http/react-ui/public/locales/zh-CN/models.json +++ b/core/http/react-ui/public/locales/zh-CN/models.json @@ -24,7 +24,6 @@ "embedding": "嵌入", "rerank": "重排", "fitsGpu": "适合 GPU", - "collapseVariants": "每个模型一行", "allBackends": "所有后端", "searchBackends": "搜索后端..." }, @@ -72,7 +71,6 @@ "empty": { "title": "未找到模型", "withFilters": "没有模型与当前搜索或筛选条件匹配。", - "collapsedVariantsHint": "其他条目已经提供的替代构建已被隐藏。关闭“每个模型一行”即可将其包含在内。", "noFilters": "模型库为空。" }, "deleteDialog": { diff --git a/core/http/react-ui/src/pages/Models.jsx b/core/http/react-ui/src/pages/Models.jsx index feac21a52..e5665b44e 100644 --- a/core/http/react-ui/src/pages/Models.jsx +++ b/core/http/react-ui/src/pages/Models.jsx @@ -23,31 +23,6 @@ import React from 'react' const CONTEXT_SIZES = [8192, 16384, 32768, 65536, 131072, 262144] const CONTEXT_LABELS = ['8K', '16K', '32K', '64K', '128K', '256K'] const FITS_FILTER_STORAGE_KEY = 'localai-models-fits-filter' -// Renamed alongside the filter it persists: the old key's stored value meant -// "show only entries declaring variants", which is not what this filter does. -const COLLAPSE_VARIANTS_STORAGE_KEY = 'localai-models-collapse-variants-filter' -// The deduplicated gallery is what a user asking "what can I install" wants, so -// the UI asks for it unless told otherwise. Only the UI decides this: the server -// still returns the full listing when the parameter is absent, because other API -// clients depend on that response. -const COLLAPSE_VARIANTS_DEFAULT = true - -// The stored vocabulary changed with the default. The previous build wrote -// '1'/'0' from an effect that runs on mount, so a stored '0' recorded that the -// page had been opened, not that anyone chose the expanded view. Honouring it -// would pin every earlier visitor to a default they never picked, so only the -// 'on'/'off' written since counts as a choice. -const readCollapseVariantsPreference = () => { - try { - const stored = localStorage.getItem(COLLAPSE_VARIANTS_STORAGE_KEY) - if (stored === 'on') return true - if (stored === 'off') return false - return COLLAPSE_VARIANTS_DEFAULT - } catch { - return COLLAPSE_VARIANTS_DEFAULT - } -} - const FILTERS = [ { key: '', labelKey: 'filters.all', icon: 'fa-layer-group' }, @@ -111,12 +86,6 @@ export default function Models() { return false } }) - // Collapses the listing to one row per model by hiding the individual builds - // another entry already offers as variants. Server-side, unlike fitsFilter, - // because the listing paginates and a client-side narrowing would leave the - // page count describing the unfiltered set. - const [collapseVariants, setCollapseVariants] = useState(readCollapseVariantsPreference) - // Total GPU memory for "fits" check const totalGpuMemory = resources?.aggregate?.total_memory || 0 @@ -127,17 +96,20 @@ export default function Models() { const filtersVal = params.filters !== undefined ? params.filters : filters const sortVal = params.sort !== undefined ? params.sort : sort const backendVal = params.backendFilter !== undefined ? params.backendFilter : backendFilter - const collapseVal = params.collapseVariants !== undefined ? params.collapseVariants : collapseVariants const queryParams = { page: params.page || page, items: 9, + // The deduplicated gallery is what a user asking "what can I install" + // wants, so the UI always asks for it. There is no control for this + // because a search term already bypasses the collapse server-side, so + // a build hidden behind a parent is still findable by name. The + // parameter stays optional on the API: other clients want either view, + // and an absent parameter still returns the full listing. + collapse_variants: 'true', } if (filtersVal.length > 0) queryParams.tag = filtersVal.join(',') if (searchVal) queryParams.term = searchVal if (backendVal) queryParams.backend = backendVal - // Omitted entirely when off rather than sent as false, so opting out asks - // for exactly the listing every other API client gets. - if (collapseVal) queryParams.collapse_variants = 'true' if (sortVal) { queryParams.sort = sortVal queryParams.order = params.order || order @@ -155,11 +127,11 @@ export default function Models() { } finally { setLoading(false) } - }, [page, search, filters, sort, order, backendFilter, collapseVariants, addToast, t]) + }, [page, search, filters, sort, order, backendFilter, addToast, t]) useEffect(() => { fetchModels() - }, [page, filters, sort, order, backendFilter, collapseVariants]) + }, [page, filters, sort, order, backendFilter]) // Fetch backend→usecase mapping once on mount useEffect(() => { @@ -324,14 +296,6 @@ export default function Models() { } }, [fitsFilter]) - useEffect(() => { - try { - localStorage.setItem(COLLAPSE_VARIANTS_STORAGE_KEY, collapseVariants ? 'on' : 'off') - } catch { - // Ignore storage errors (e.g., private browsing restrictions). - } - }, [collapseVariants]) - const visibleModels = models.filter((model) => { if (!fitsFilter) return true const name = model.name || model.id @@ -403,14 +367,6 @@ export default function Models() { ) })} - {totalGpuMemory > 0 && (