test(desktop): harden model-visibility toggle + dedupe default expansion

Follow-up to the salvaged #47450 fix:
- Extract expandProviderDefaults() so the curated-default expansion rule
  lives in one place (was duplicated between defaultVisibleKeys and
  resolveVisibleKeys).
- Drop the redundant new Set() wrap in toggleModelVisibility (resolveVisibleKeys
  already returns a fresh Set; effectiveVisibleKeys already relied on this).
- Document the intentional re-enable behavior (re-enabling one model of a
  hidden-all provider restores only that model, not the curated defaults) and
  tighten the toggleModelVisibility JSDoc.
- Add 7 hardening tests: re-enable-restores-only-that-model, full hide/re-enable
  round-trip, empty-non-null stored, single toggle-off from null defaults,
  zero-model provider, and direct resolveVisibleKeys null/empty assertions.

apps/desktop/src/store/model-visibility.test.ts

@@ -4,6 +4,7 @@ import type { ModelOptionProvider } from '@/types/hermes'
 
 import {
   collapseModelFamilies,
+  defaultVisibleKeys,
   effectiveVisibleKeys,
   emptyProviderSentinelKey,
   isProviderSentinel,
@@ -159,4 +160,72 @@ describe('toggleModelVisibility', () => {
     expect(visible.has(modelVisibilityKey('openai', 'gpt-a'))).toBe(true)
     expect(visible.has(modelVisibilityKey('nous', 'hermes-x'))).toBe(false)
   })
+
+  it('re-enabling one model of a hidden-all provider restores ONLY that model, not the curated defaults', () => {
+    // openai hidden-all, nous untouched.
+    let stored: Set<string> | null = new Set([emptyProviderSentinelKey('openai')])
+
+    stored = apply(stored, 'openai', 'gpt-a')
+
+    const visible = effectiveVisibleKeys(stored, providers)
+    expect(visible.has(modelVisibilityKey('openai', 'gpt-a'))).toBe(true)
+    // gpt-b is NOT restored — "you hid everything, you get back only what you re-enable".
+    expect(visible.has(modelVisibilityKey('openai', 'gpt-b'))).toBe(false)
+  })
+
+  it('re-hiding the last re-enabled model re-adds the sentinel (full round-trip)', () => {
+    let stored: Set<string> | null = new Set([emptyProviderSentinelKey('openai')])
+
+    // Re-enable gpt-a (clears sentinel, set = {gpt-a}), then toggle it back off.
+    stored = apply(stored, 'openai', 'gpt-a')
+    expect(stored.has(emptyProviderSentinelKey('openai'))).toBe(false)
+    stored = apply(stored, 'openai', 'gpt-a')
+
+    expect(stored.has(emptyProviderSentinelKey('openai'))).toBe(true)
+    expect(effectiveVisibleKeys(stored, providers).has(modelVisibilityKey('openai', 'gpt-a'))).toBe(false)
+  })
+
+  it('toggling from an empty (non-null) stored set adds the model without expanding defaults', () => {
+    // Empty-but-not-null = "everything hidden". resolveVisibleKeys short-circuits to {}.
+    const stored = new Set<string>()
+
+    const next = apply(stored, 'openai', 'gpt-a')
+
+    expect(next.has(modelVisibilityKey('openai', 'gpt-a'))).toBe(true)
+    // No curated defaults were expanded for any provider.
+    expect(next.has(modelVisibilityKey('openai', 'gpt-b'))).toBe(false)
+    expect(next.has(modelVisibilityKey('nous', 'hermes-x'))).toBe(false)
+  })
+
+  it('toggling off one default model from null stored keeps the rest of the curated defaults', () => {
+    // null = "never customized": resolveVisibleKeys expands all defaults first.
+    const next = apply(null, 'openai', 'gpt-a')
+
+    expect(next.has(modelVisibilityKey('openai', 'gpt-a'))).toBe(false)
+    expect(next.has(modelVisibilityKey('openai', 'gpt-b'))).toBe(true)
+    expect(next.has(modelVisibilityKey('nous', 'hermes-x'))).toBe(true)
+    // Other models remain, so no sentinel.
+    expect(next.has(emptyProviderSentinelKey('openai'))).toBe(false)
+  })
+
+  it('tolerates a provider with zero models (defensive — dialog filters these out)', () => {
+    const ps = [provider('empty', []), provider('openai', ['gpt-a'])]
+    const next = toggleModelVisibility(new Set([modelVisibilityKey('openai', 'gpt-a')]), ps, 'empty', 'ghost')
+
+    // No crash; the phantom key is recorded but no defaults are invented.
+    expect([...next].some(k => k.startsWith('empty::') && !isProviderSentinel(k))).toBe(true)
+    expect(next.has(modelVisibilityKey('openai', 'gpt-a'))).toBe(true)
+  })
+})
+
+describe('resolveVisibleKeys', () => {
+  const providers = [provider('openai', ['gpt-a', 'gpt-b']), provider('nous', ['hermes-x', 'hermes-y'])]
+
+  it('returns the curated defaults verbatim for null stored', () => {
+    expect(resolveVisibleKeys(null, providers)).toEqual(defaultVisibleKeys(providers))
+  })
+
+  it('returns an empty set for an empty (non-null) stored set', () => {
+    expect([...resolveVisibleKeys(new Set(), providers)]).toEqual([])
+  })
 })

apps/desktop/src/store/model-visibility.ts

@@ -106,16 +106,23 @@ export function defaultVisibleKeys(providers: readonly ModelOptionProvider[]): S
   const keys = new Set<string>()
 
   for (const provider of providers) {
-    const families = collapseModelFamilies(provider.models ?? [])
-
-    for (const family of families.slice(0, DEFAULT_VISIBLE_PER_PROVIDER)) {
-      keys.add(modelVisibilityKey(provider.slug, family.id))
-    }
+    expandProviderDefaults(provider, keys)
   }
 
   return keys
 }
 
+/** Add a provider's curated default model keys (top-N collapsed families) to
+ *  `target`. Shared by `defaultVisibleKeys` and `resolveVisibleKeys` so the
+ *  expansion rule lives in exactly one place. */
+function expandProviderDefaults(provider: ModelOptionProvider, target: Set<string>): void {
+  const families = collapseModelFamilies(provider.models ?? [])
+
+  for (const family of families.slice(0, DEFAULT_VISIBLE_PER_PROVIDER)) {
+    target.add(modelVisibilityKey(provider.slug, family.id))
+  }
+}
+
 /** 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
@@ -148,11 +155,7 @@ export function resolveVisibleKeys(
       continue
     }
 
-    const families = collapseModelFamilies(provider.models ?? [])
-
-    for (const family of families.slice(0, DEFAULT_VISIBLE_PER_PROVIDER)) {
-      next.add(modelVisibilityKey(provider.slug, family.id))
-    }
+    expandProviderDefaults(provider, next)
   }
 
   return next
@@ -180,14 +183,15 @@ export function effectiveVisibleKeys(
  *  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. */
+ *  hide-all; re-enabling a model clears THAT provider's sentinel (only). */
 export function toggleModelVisibility(
   stored: Set<string> | null,
   providers: readonly ModelOptionProvider[],
   providerSlug: string,
   model: string
 ): Set<string> {
-  const next = new Set(resolveVisibleKeys(stored, providers))
+  // `resolveVisibleKeys` always returns a fresh Set, so we can mutate it directly.
+  const next = resolveVisibleKeys(stored, providers)
   const key = modelVisibilityKey(providerSlug, model)
   const sentinel = emptyProviderSentinelKey(providerSlug)
 
@@ -203,6 +207,10 @@ export function toggleModelVisibility(
       next.add(sentinel)
     }
   } else {
+    // Re-enabling promotes a previously hidden-all provider to an explicit
+    // set of exactly the one re-enabled model — the curated defaults are NOT
+    // restored. Intentional: "you hid everything, you get back only what you
+    // re-enable." (Locked in by the sentinel-clear-on-re-enable test.)
     next.delete(sentinel)
     next.add(key)
   }