mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-24 10:52:21 +00:00
fix(desktop): preserve other providers' hide-all in model visibility dialog
#43496 added a per-provider hide-all sentinel ('provider::') so emptying a provider in the Edit Models dialog stopped re-expanding its defaults. That fixed the single-provider case, but the dialog's toggle handler seeds its working set from effectiveVisibleKeys(), which strips ALL sentinels before returning. So persisting after any toggle silently dropped every OTHER provider's hide-all sentinel; those providers then looked 'never customized' and re-enabled all their models on the next render. Split resolution into two functions: - resolveVisibleKeys(): stored keys + curated default expansion, with hide-all sentinels PRESERVED — the canonical working set the toggle handler mutates and persists. - effectiveVisibleKeys(): resolveVisibleKeys() then strips sentinels, for display only (unchanged contract). Move the toggle set-computation into a pure, unit-tested toggleModelVisibility() that seeds from resolveVisibleKeys(), so sibling sentinels survive the persist. Add regression tests that drive the real toggle handler across multiple providers. Follow-up to #43496; completes the fix for #43485 (cross-provider case).
This commit is contained in:
parent
f57ff7aef1
commit
8666fd7635
3 changed files with 120 additions and 26 deletions
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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<string> | 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<string> | 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<string> | 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<string> | 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)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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<string> | null,
|
||||
providers: readonly ModelOptionProvider[]
|
||||
): Set<string> {
|
||||
|
|
@ -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<string> | null,
|
||||
providers: readonly ModelOptionProvider[]
|
||||
): Set<string> {
|
||||
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<string> | null,
|
||||
providers: readonly ModelOptionProvider[],
|
||||
providerSlug: string,
|
||||
model: string
|
||||
): Set<string> {
|
||||
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
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue