From 4d620c977bd10dc8cf56586c5d3bfedf10025e87 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sat, 26 Sep 2026 21:56:14 -0500 Subject: [PATCH] fix(desktop): one-time model snapshot adoption no longer strands catalog defaults behind a stale allowlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A visible-models allowlist persisted before the known-models snapshot existed was adopted with the entire current catalog marked as already judged, so a model present in the catalog but absent from the old allowlist stayed hidden through every later refresh. Adoption now records only the curated defaults the old allowlist actually contained; the ones it omitted stay unknown and the default rule re-admits them. Explicit hide-all sentinels are honoured, and a deliberately hidden default resurfaces once at worst — the next save records the re-hide. Fixes the adoption half of https://github.com/NousResearch/hermes-agent/issues/122053 — stores already seeded by the previous adoption behavior still need the recovery action proposed in https://github.com/NousResearch/hermes-agent/pull/122055. --- .../src/store/model-visibility.test.ts | 87 ++++++++++++++++++- apps/desktop/src/store/model-visibility.ts | 40 +++++++-- 2 files changed, 120 insertions(+), 7 deletions(-) diff --git a/apps/desktop/src/store/model-visibility.test.ts b/apps/desktop/src/store/model-visibility.test.ts index 2247d676bd..e3d9aaac66 100644 --- a/apps/desktop/src/store/model-visibility.test.ts +++ b/apps/desktop/src/store/model-visibility.test.ts @@ -1,5 +1,5 @@ import type { ModelOptionProvider } from '@hermes/shared' -import { describe, expect, it } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' import { collapseModelFamilies, @@ -277,6 +277,91 @@ describe('featured defaults', () => { }) }) +describe('seedKnownModels', () => { + const featuredProvider = (slug: string, models: string[], featured_models: string[]): ModelOptionProvider => ({ + featured_models, + models, + name: slug, + slug + }) + + // Fresh module per load: the atoms read localStorage at import, so a + // re-import stands in for a renderer reload after a pre-snapshot persist. + const loadStore = () => import('./model-visibility') + + beforeEach(() => { + window.localStorage.clear() + vi.resetModules() + }) + + it('does not adopt a stale allowlist as a judgement over catalog-present defaults', async () => { + // A visible set persisted before the snapshot existed, first loaded by a + // build whose catalog already ships gpt-6 — the bug class from 122053: + // seeding everything as judged would keep gpt-6 hidden forever. + window.localStorage.setItem('hermes.desktop.visible-models', JSON.stringify(['openai-codex::gpt-5.5'])) + + const store = await loadStore() + + const providers = [ + featuredProvider('openai-codex', ['gpt-5.5', 'gpt-6'], ['gpt-5.5', 'gpt-6']), + featuredProvider('qwen', ['qwen3-coder', 'qwen4'], ['qwen3-coder']) + ] + + store.seedKnownModels(providers) + + const visible = store.effectiveVisibleKeys(store.$visibleModels.get(), providers) + + // The curated defaults the stale allowlist omitted come back… + expect(visible.has(modelVisibilityKey('openai-codex', 'gpt-5.5'))).toBe(true) + expect(visible.has(modelVisibilityKey('openai-codex', 'gpt-6'))).toBe(true) + expect(visible.has(modelVisibilityKey('qwen', 'qwen3-coder'))).toBe(true) + // …while the user's explicit hide of the non-default qwen4 holds verbatim. + expect(visible.has(modelVisibilityKey('qwen', 'qwen4'))).toBe(false) + }) + + it('re-seeding is inert once a snapshot exists (never a running union)', async () => { + window.localStorage.setItem('hermes.desktop.visible-models', JSON.stringify(['openai-codex::gpt-5.5'])) + + const store = await loadStore() + const first = [featuredProvider('openai-codex', ['gpt-5.5'], ['gpt-5.5'])] + store.seedKnownModels(first) + + // A later catalog refresh with a brand-new model must not mark it judged. + const grown = [featuredProvider('openai-codex', ['gpt-5.5', 'gpt-7'], ['gpt-5.5', 'gpt-7'])] + store.seedKnownModels(grown) + + const visible = store.effectiveVisibleKeys(store.$visibleModels.get(), grown) + expect(visible.has(modelVisibilityKey('openai-codex', 'gpt-7'))).toBe(true) + }) + + it('honours an explicit hide-all sentinel: its defaults stay unknown, not re-admitted', async () => { + window.localStorage.setItem('hermes.desktop.visible-models', JSON.stringify(['nous::'])) + + const store = await loadStore() + const providers = [featuredProvider('nous', ['hermes-4', 'hermes-5'], ['hermes-4', 'hermes-5'])] + + store.seedKnownModels(providers) + + const visible = store.effectiveVisibleKeys(store.$visibleModels.get(), providers) + expect(visible.has(modelVisibilityKey('nous', 'hermes-4'))).toBe(false) + expect(visible.has(modelVisibilityKey('nous', 'hermes-5'))).toBe(false) + }) + + it('leaves a fresh install unseeded: no stored set, nothing to adopt', async () => { + const store = await loadStore() + const providers = [featuredProvider('openai-codex', ['gpt-5.5', 'gpt-6'], ['gpt-5.5', 'gpt-6'])] + + store.seedKnownModels(providers) + + // Nothing was persisted before the snapshot existed, so there is no + // adoption to do; the curated defaults show and `known` stays null. + expect(store.$knownModels.get()).toBeNull() + expect(store.effectiveVisibleKeys(store.$visibleModels.get(), providers)).toEqual( + store.defaultVisibleKeys(providers) + ) + }) +}) + describe('setProviderVisibility', () => { const providers = [provider('openai', ['gpt-a', 'gpt-b']), provider('nous', ['hermes-x', 'hermes-y'])] diff --git a/apps/desktop/src/store/model-visibility.ts b/apps/desktop/src/store/model-visibility.ts index 46cf36b0d7..2ca7d3ce79 100644 --- a/apps/desktop/src/store/model-visibility.ts +++ b/apps/desktop/src/store/model-visibility.ts @@ -132,16 +132,44 @@ function persistKnownModels(known: Set): void { } /** One-time adoption for a visible set persisted before the known snapshot - * existed: everything in the catalog at that moment counts as judged (the - * user's hide choices are honoured verbatim); only models that appear later are - * new. Never a running union — that would mark a newcomer judged on the very - * render that first shows it. Call when the catalog has loaded. */ + * existed. The old store predates the snapshot machinery, so "absent from the + * allowlist" is ambiguous: a deliberate hide and a model that arrived after + * the user last curated look identical. Recording everything in the catalog + * as judged therefore strands catalog-present defaults behind a stale + * allowlist forever (https://github.com/NousResearch/hermes-agent/issues/122053) + * — so the curated defaults the old allowlist does NOT contain stay unknown, + * and the default rule re-admits them on the next resolve. The one-time cost + * is that a deliberately hidden default comes back once; the user's next save + * records the re-hide properly and it locks. Non-default models keep the + * verbatim-hide semantics — the default rule never showed them anyway — and + * a provider hidden outright (sentinel) is skipped entirely. Never a running + * union — that would mark a newcomer judged on the very render that first + * shows it. Call when the catalog has loaded. */ export function seedKnownModels(providers: readonly ModelOptionProvider[]): void { - if ($knownModels.get() !== null || $visibleModels.get() === null || providers.length === 0) { + const stored = $visibleModels.get() + + if ($knownModels.get() !== null || stored === null || providers.length === 0) { return } - persistKnownModels(allFamilyKeys(providers)) + const known = allFamilyKeys(providers) + + for (const provider of providers) { + if (stored.has(emptyProviderSentinelKey(provider.slug))) { + continue + } + + const defaults = new Set() + expandProviderDefaults(provider, defaults) + + for (const key of defaults) { + if (!stored.has(key)) { + known.delete(key) + } + } + } + + persistKnownModels(known) } export function setModelVisibilityOpen(open: boolean): void {