diff --git a/apps/desktop/src/components/model-visibility-dialog.tsx b/apps/desktop/src/components/model-visibility-dialog.tsx index 0b92dba36fb..05a5e92cb3a 100644 --- a/apps/desktop/src/components/model-visibility-dialog.tsx +++ b/apps/desktop/src/components/model-visibility-dialog.tsx @@ -14,10 +14,9 @@ import { $visibleModels, collapseModelFamilies, effectiveVisibleKeys, - emptyProviderSentinelKey, - isProviderSentinel, modelVisibilityKey, - setVisibleModels + setVisibleModels, + toggleModelVisibility } from '@/store/model-visibility' import type { ModelOptionProvider, ModelOptionsResponse } from '@/types/hermes' @@ -61,25 +60,7 @@ export function ModelVisibilityDialog({ const visible = effectiveVisibleKeys(stored, providers) const toggle = (provider: ModelOptionProvider, model: string) => { - const next = new Set(effectiveVisibleKeys($visibleModels.get(), providers)) - const key = modelVisibilityKey(provider.slug, model) - const sentinel = emptyProviderSentinelKey(provider.slug) - - if (next.has(key)) { - next.delete(key) - - // Check if this was the last real model for this provider. - const remainingForProvider = [...next].some(k => k.startsWith(`${provider.slug}::`) && !isProviderSentinel(k)) - - if (!remainingForProvider) { - next.add(sentinel) - } - } else { - next.delete(sentinel) - next.add(key) - } - - setVisibleModels(next) + setVisibleModels(toggleModelVisibility($visibleModels.get(), providers, provider.slug, model)) } const q = search.trim().toLowerCase() diff --git a/apps/desktop/src/store/model-visibility.test.ts b/apps/desktop/src/store/model-visibility.test.ts index 90eccdf457e..446a61f874e 100644 --- a/apps/desktop/src/store/model-visibility.test.ts +++ b/apps/desktop/src/store/model-visibility.test.ts @@ -7,7 +7,9 @@ import { effectiveVisibleKeys, emptyProviderSentinelKey, isProviderSentinel, - modelVisibilityKey + modelVisibilityKey, + resolveVisibleKeys, + toggleModelVisibility } from './model-visibility' const provider = (slug: string, models: string[]): ModelOptionProvider => ({ @@ -96,4 +98,65 @@ describe('model visibility', () => { expect(isProviderSentinel('openai::')).toBe(true) expect(isProviderSentinel('openai::gpt-4o')).toBe(false) }) + + it('resolveVisibleKeys preserves sentinels that effectiveVisibleKeys strips', () => { + const stored = new Set([emptyProviderSentinelKey('nous')]) + const providers = [provider('nous', ['hermes-x', 'hermes-y']), provider('ollama', ['qwen3:latest'])] + + const resolved = resolveVisibleKeys(stored, providers) + expect(resolved.has(emptyProviderSentinelKey('nous'))).toBe(true) + expect(resolved.has(modelVisibilityKey('nous', 'hermes-x'))).toBe(false) + // Un-customized providers still expand to their defaults. + expect(resolved.has(modelVisibilityKey('ollama', 'qwen3:latest'))).toBe(true) + + // Display variant drops the sentinel. + expect(effectiveVisibleKeys(stored, providers).has(emptyProviderSentinelKey('nous'))).toBe(false) + }) +}) + +describe('toggleModelVisibility', () => { + const providers = [provider('openai', ['gpt-a', 'gpt-b']), provider('nous', ['hermes-x', 'hermes-y'])] + + // Drive the handler the way the dialog does: feed each result back in as the + // next `stored`, so the persisted set is what the next toggle starts from. + const apply = (stored: Set | null, slug: string, model: string) => + toggleModelVisibility(stored, providers, slug, model) + + it('records a hide-all sentinel when the last model of a provider is toggled off', () => { + let stored: Set | null = null + stored = apply(stored, 'openai', 'gpt-a') + stored = apply(stored, 'openai', 'gpt-b') + + expect(stored.has(emptyProviderSentinelKey('openai'))).toBe(true) + expect(effectiveVisibleKeys(stored, providers).has(modelVisibilityKey('openai', 'gpt-a'))).toBe(false) + expect(effectiveVisibleKeys(stored, providers).has(modelVisibilityKey('openai', 'gpt-b'))).toBe(false) + }) + + it('keeps a hidden provider hidden when a different provider is toggled (regression for #43485)', () => { + // Hide ALL of nous — its sentinel is now stored. + let stored: Set | null = null + stored = apply(stored, 'nous', 'hermes-x') + stored = apply(stored, 'nous', 'hermes-y') + expect(stored.has(emptyProviderSentinelKey('nous'))).toBe(true) + + // Toggle a model in another provider. nous must NOT snap back on. + stored = apply(stored, 'openai', 'gpt-a') + + expect(stored.has(emptyProviderSentinelKey('nous'))).toBe(true) + const visible = effectiveVisibleKeys(stored, providers) + expect(visible.has(modelVisibilityKey('nous', 'hermes-x'))).toBe(false) + expect(visible.has(modelVisibilityKey('nous', 'hermes-y'))).toBe(false) + }) + + it('clears only the toggled provider sentinel when a model is re-enabled', () => { + let stored: Set | null = new Set([emptyProviderSentinelKey('openai'), emptyProviderSentinelKey('nous')]) + + stored = apply(stored, 'openai', 'gpt-a') + + expect(stored.has(emptyProviderSentinelKey('openai'))).toBe(false) + expect(stored.has(emptyProviderSentinelKey('nous'))).toBe(true) + const visible = effectiveVisibleKeys(stored, providers) + expect(visible.has(modelVisibilityKey('openai', 'gpt-a'))).toBe(true) + expect(visible.has(modelVisibilityKey('nous', 'hermes-x'))).toBe(false) + }) }) diff --git a/apps/desktop/src/store/model-visibility.ts b/apps/desktop/src/store/model-visibility.ts index 5c2b568c596..c5611dc274f 100644 --- a/apps/desktop/src/store/model-visibility.ts +++ b/apps/desktop/src/store/model-visibility.ts @@ -116,9 +116,12 @@ export function defaultVisibleKeys(providers: readonly ModelOptionProvider[]): S return keys } -/** Resolve which keys are currently visible: the user's explicit set when - * configured, otherwise the curated default for the given providers. */ -export function effectiveVisibleKeys( +/** Resolve the canonical working set: the user's stored keys plus the curated + * default expansion for any provider they haven't customized. Hide-all + * sentinels are PRESERVED here — this is the set the toggle handler mutates and + * persists, so dropping a sentinel would silently re-enable a provider the user + * emptied. Use `effectiveVisibleKeys` for display (sentinels stripped). */ +export function resolveVisibleKeys( stored: Set | null, providers: readonly ModelOptionProvider[] ): Set { @@ -134,9 +137,11 @@ export function effectiveVisibleKeys( for (const provider of providers) { const providerPrefix = `${provider.slug}::` + const hasStoredProvider = [...stored].some( key => key.startsWith(providerPrefix) && !isProviderSentinel(key) ) + const hasSentinel = stored.has(emptyProviderSentinelKey(provider.slug)) if (hasStoredProvider || hasSentinel) { @@ -150,6 +155,17 @@ export function effectiveVisibleKeys( } } + return next +} + +/** Resolve which keys are currently visible for DISPLAY: the resolved working + * set with bookkeeping sentinels stripped (they are not real models). */ +export function effectiveVisibleKeys( + stored: Set | null, + providers: readonly ModelOptionProvider[] +): Set { + const next = resolveVisibleKeys(stored, providers) + // Strip sentinel keys — they are bookkeeping, not real visibility entries. for (const key of [...next]) { if (isProviderSentinel(key)) { @@ -159,3 +175,37 @@ export function effectiveVisibleKeys( return next } + +/** Compute the next persisted visibility set when one model row is toggled. + * Seeds from `resolveVisibleKeys` (NOT `effectiveVisibleKeys`) so other + * providers' hide-all sentinels survive the persist. When the last visible + * model of a provider is toggled off, a sentinel records the explicit + * hide-all; re-enabling any model clears that provider's sentinel. */ +export function toggleModelVisibility( + stored: Set | null, + providers: readonly ModelOptionProvider[], + providerSlug: string, + model: string +): Set { + const next = new Set(resolveVisibleKeys(stored, providers)) + const key = modelVisibilityKey(providerSlug, model) + const sentinel = emptyProviderSentinelKey(providerSlug) + + if (next.has(key)) { + next.delete(key) + + // Check if this was the last real model for this provider. + const remainingForProvider = [...next].some( + k => k.startsWith(`${providerSlug}::`) && !isProviderSentinel(k) + ) + + if (!remainingForProvider) { + next.add(sentinel) + } + } else { + next.delete(sentinel) + next.add(key) + } + + return next +}