From afb924d841700fe1620dfb5124736ff60de10eac Mon Sep 17 00:00:00 2001 From: PRATHAMESH75 Date: Sat, 26 Sep 2026 15:16:17 +0530 Subject: [PATCH] test(desktop): cover personalityOptions' root read and pin GUI ordering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review on #123378: - Add enumOptionsFor('display.personality', …) tests with a root-level `personalities` block, deriving expected built-ins from BUILTIN_PERSONALITIES (change-detector rule). Reverting the new root read now fails these, closing the coverage gap the reviewer flagged. - Replace the Set-based clash assertion in personalityNamesFromConfig with direct array equality so it pins membership, dedupe, and the root-before-agent ordering the CLI listing uses. --- apps/desktop/src/app/settings/helpers.test.ts | 42 +++++++++++++++++++ apps/desktop/src/lib/chat-runtime.test.ts | 6 ++- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/apps/desktop/src/app/settings/helpers.test.ts b/apps/desktop/src/app/settings/helpers.test.ts index d9c2d49811..fa22fb5205 100644 --- a/apps/desktop/src/app/settings/helpers.test.ts +++ b/apps/desktop/src/app/settings/helpers.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest' import type { HermesConfigRecord } from '@/types/hermes' +import { BUILTIN_PERSONALITIES } from './constants' import { defineFieldCopy, fieldCopyForSchemaKey, schemaKeyToFieldCopyKey } from './field-copy' import { clearsEnabledToolsets, @@ -266,6 +267,47 @@ describe('settings helpers', () => { }) }) + describe('enumOptionsFor — display.personality dropdown', () => { + it('lists a root-level `personalities` block alongside the built-ins (#123297)', () => { + // The Python spec (`hermes_cli.personality.available_personalities`) overlays + // the built-ins with the root `personalities` block then `agent.personalities`; + // the dropdown must surface a root-registered persona the CLI/gateway resolve. + const config: HermesConfigRecord = { personalities: { root_persona: { prompt: 'hi' } } } + const opts = enumOptionsFor('display.personality', '', config) + + // Derive the expected built-ins from the source of truth, per the repo's + // change-detector rule — adding a built-in must not silently break this. + for (const builtin of BUILTIN_PERSONALITIES) { + expect(opts).toContain(builtin) + } + expect(opts).toContain('') // the "unset" sentinel + expect(opts).toContain('root_persona') + }) + + it('merges root and agent personalities, deduping a clashing name', () => { + const config: HermesConfigRecord = { + personalities: { root_persona: {}, shared: {} }, + agent: { personalities: { agent_persona: {}, shared: {} } } + } + const opts = enumOptionsFor('display.personality', '', config)! + expect(opts).toContain('root_persona') + expect(opts).toContain('agent_persona') + // a name in both blocks is offered exactly once + expect(opts.filter(o => o === 'shared')).toHaveLength(1) + }) + + it('ignores a non-object or array `personalities` block', () => { + for (const bad of [[], 'nope', 42, null]) { + const opts = enumOptionsFor('display.personality', '', { personalities: bad } as HermesConfigRecord)! + // still the built-ins + empty sentinel, no crash on a malformed block + expect(opts).toContain('') + for (const builtin of BUILTIN_PERSONALITIES) { + expect(opts).toContain(builtin) + } + } + }) + }) + describe('sectionFieldEntries', () => { it('renders memory.provider from config even when the backend schema omits it', () => { const schema = { 'memory.memory_enabled': { type: 'boolean' as const } } diff --git a/apps/desktop/src/lib/chat-runtime.test.ts b/apps/desktop/src/lib/chat-runtime.test.ts index d6f0d45bd6..33d4eb6308 100644 --- a/apps/desktop/src/lib/chat-runtime.test.ts +++ b/apps/desktop/src/lib/chat-runtime.test.ts @@ -263,7 +263,11 @@ describe('personalityNamesFromConfig', () => { agent: { personalities: { agent_persona: 'a', shared: 'agent' } } }) - expect(new Set(names)).toEqual(new Set(['root_persona', 'shared', 'agent_persona'])) + // Direct array equality pins membership, dedupe, AND order in one assertion: + // `available_personalities()` inserts the root block before `agent.personalities`, + // and a clashing name keeps its first-insert (root) position, so the GUI listing + // must match that exact order. + expect(names).toEqual(['root_persona', 'shared', 'agent_persona']) }) it('ignores non-object or array blocks', () => {