Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions apps/desktop/src/app/desktop-controller.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1017,8 +1017,11 @@ export function DesktopController() {

useEffect(() => {
if (gatewayState === 'open' && !activeSessionId && freshDraftReady) {
void refreshCurrentModel()
void refreshHermesConfig()
// Load config first so the reset-on-new-session flag is in the store
// before refreshCurrentModel decides whether to force-reseed the model.
void refreshHermesConfig().finally(() => {
void refreshCurrentModel()
})
}
}, [activeSessionId, freshDraftReady, gatewayState, refreshCurrentModel, refreshHermesConfig])
Comment on lines 1018 to 1026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Sequential fetch adds latency for the default (flag-off) case

Previously refreshCurrentModel and refreshHermesConfig raced in parallel, so the model was typically seeded as fast as the faster of the two calls. Now refreshCurrentModel is always blocked behind a full /api/config round-trip on every fresh-draft open — even when reset_model_on_new_session is false (the default for all users). On a slow connection the composer model slot will appear blank noticeably longer on startup.

One option is to fire both calls in parallel but have refreshCurrentModel re-check the flag after config settles, e.g. by awaiting the config promise first only if the flag is known to be true from a previous fetch, or by passing the flag as a parameter. The current approach is safe and correct; this is a tradeoff worth being aware of if startup latency is a concern.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


Expand Down
38 changes: 37 additions & 1 deletion apps/desktop/src/app/session/hooks/use-hermes-config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'

import { getHermesConfig } from '@/hermes'
import { persistString } from '@/lib/storage'
import { $currentCwd, setCurrentCwd } from '@/store/session'
import { $currentCwd, $resetModelOnNewSession, setCurrentCwd, setResetModelOnNewSession } from '@/store/session'

import { useHermesConfig } from './use-hermes-config'

Expand All @@ -22,6 +22,7 @@ describe('useHermesConfig refreshHermesConfig', () => {
beforeEach(() => {
// Reset atoms and localStorage between tests
setCurrentCwd('')
setResetModelOnNewSession(false)
persistString(WORKSPACE_CWD_KEY, null)
})

Expand Down Expand Up @@ -141,4 +142,39 @@ describe('useHermesConfig refreshHermesConfig', () => {

expect(refreshProjectBranch).toHaveBeenCalledWith('/workspace/attached-project')
})

it('mirrors desktop.reset_model_on_new_session=true into the store', async () => {
mockConfig({ desktop: { reset_model_on_new_session: true } })

const { result } = renderHook(() =>
useHermesConfig({
activeSessionIdRef: { current: null },
refreshProjectBranch: vi.fn().mockResolvedValue(undefined)
})
)

await act(async () => {
await result.current.refreshHermesConfig()
})

expect($resetModelOnNewSession.get()).toBe(true)
})

it('defaults the reset-model flag to false when the config key is absent', async () => {
setResetModelOnNewSession(true) // prove the refresh clears a stale true
mockConfig({})

const { result } = renderHook(() =>
useHermesConfig({
activeSessionIdRef: { current: null },
refreshProjectBranch: vi.fn().mockResolvedValue(undefined)
})
)

await act(async () => {
await result.current.refreshHermesConfig()
})

expect($resetModelOnNewSession.get()).toBe(false)
})
})
4 changes: 3 additions & 1 deletion apps/desktop/src/app/session/hooks/use-hermes-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import {
setCurrentPersonality,
setCurrentReasoningEffort,
setCurrentServiceTier,
setIntroPersonality
setIntroPersonality,
setResetModelOnNewSession
} from '@/store/session'
import { applyAutoSpeakFromConfig } from '@/store/voice-prefs'

Expand Down Expand Up @@ -87,6 +88,7 @@ export function useHermesConfig({ activeSessionIdRef, refreshProjectBranch }: He

setVoiceMaxRecordingSeconds(recordingLimit(config.voice?.max_recording_seconds))
setSttEnabled(config.stt?.enabled !== false)
setResetModelOnNewSession(config.desktop?.reset_model_on_new_session === true)
applyAutoSpeakFromConfig(config)
} catch {
// Config is nice-to-have; chat still works without it.
Expand Down
53 changes: 52 additions & 1 deletion apps/desktop/src/app/session/hooks/use-model-controls.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,14 @@ import { cleanup, render, renderHook } from '@testing-library/react'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'

import { getGlobalModelInfo } from '@/hermes'
import { $activeSessionId, $currentModel, $currentProvider, setCurrentModel, setCurrentProvider } from '@/store/session'
import {
$activeSessionId,
$currentModel,
$currentProvider,
setCurrentModel,
setCurrentProvider,
setResetModelOnNewSession
} from '@/store/session'

import { useModelControls } from './use-model-controls'

Expand Down Expand Up @@ -56,6 +63,7 @@ describe('useModelControls', () => {
$activeSessionId.set(null)
setCurrentModel('')
setCurrentProvider('')
setResetModelOnNewSession(false)
})

afterEach(() => {
Expand All @@ -64,6 +72,7 @@ describe('useModelControls', () => {
$activeSessionId.set(null)
setCurrentModel('')
setCurrentProvider('')
setResetModelOnNewSession(false)
})

it('applies the global model when there is no active runtime session', async () => {
Expand Down Expand Up @@ -179,4 +188,46 @@ describe('useModelControls', () => {
await result.current.refreshCurrentModel(true)
expect($currentModel.get()).toBe('openai/gpt-5.5')
})

it('reseeds a sticky last-picked model to the default when reset_model_on_new_session is ON', async () => {
vi.mocked(getGlobalModelInfo).mockResolvedValue({ model: 'claude-opus-4-8', provider: 'claude-apr' })
setCurrentModel('claude-fable-5')
setCurrentProvider('claude-apr')
setResetModelOnNewSession(true)

const { result } = renderHook(() =>
useModelControls({
activeSessionId: null,
queryClient: new QueryClient(),
requestGateway: vi.fn()
})
)

// Flag ON: a fresh draft snaps the sticky pick back to the profile default.
await result.current.refreshCurrentModel()
expect($currentModel.get()).toBe('claude-opus-4-8')
expect($currentProvider.get()).toBe('claude-apr')
})

it('never reseeds the composer for an ACTIVE session even when the flag is ON', async () => {
vi.mocked(getGlobalModelInfo).mockResolvedValue({ model: 'claude-opus-4-8', provider: 'claude-apr' })
setCurrentModel('claude-fable-5')
setCurrentProvider('claude-apr')
setResetModelOnNewSession(true)
$activeSessionId.set('runtime-1')

const { result } = renderHook(() =>
useModelControls({
activeSessionId: 'runtime-1',
queryClient: new QueryClient(),
requestGateway: vi.fn()
})
)

await result.current.refreshCurrentModel()

// A live session owns its model; the flag must not disturb it mid-conversation.
expect($currentModel.get()).toBe('claude-fable-5')
expect($currentProvider.get()).toBe('claude-apr')
})
})
13 changes: 10 additions & 3 deletions apps/desktop/src/app/session/hooks/use-model-controls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { useCallback } from 'react'
import { getGlobalModelInfo } from '@/hermes'
import { useI18n } from '@/i18n'
import { notifyError } from '@/store/notifications'
import { $activeSessionId, $currentModel, $currentProvider, setCurrentModel, setCurrentProvider } from '@/store/session'
import { $activeSessionId, $currentModel, $currentProvider, $resetModelOnNewSession, setCurrentModel, setCurrentProvider } from '@/store/session'
import type { ModelOptionsResponse } from '@/types/hermes'

interface ModelSelection {
Expand Down Expand Up @@ -40,19 +40,26 @@ export function useModelControls({ activeSessionId, queryClient, requestGateway
// only fills an EMPTY selection so a user's pick (plain UI state in
// $currentModel) survives the lifecycle refreshes that fire on boot / fresh
// draft / session events. A live session owns the footer, so skip entirely.
//
// When config.yaml `desktop.reset_model_on_new_session` is on, a fresh draft
// is treated as a forced reseed too: the sticky last-picked model is
// overwritten with the profile default so a new chat always starts on the
// default (opt-in; default off preserves the sticky behavior).
const refreshCurrentModel = useCallback(async (force = false) => {
try {
if ($activeSessionId.get()) {
return
}

if (!force && $currentModel.get()) {
const effectiveForce = force || $resetModelOnNewSession.get()

if (!effectiveForce && $currentModel.get()) {
return
}

const result = await getGlobalModelInfo()

if ($activeSessionId.get() || (!force && $currentModel.get())) {
if ($activeSessionId.get() || (!effectiveForce && $currentModel.get())) {
return
}

Expand Down
8 changes: 8 additions & 0 deletions apps/desktop/src/store/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,11 @@ export const $currentProvider = atom(storedString(COMPOSER_PROVIDER_KEY) ?? '')
export const $currentReasoningEffort = atom(storedString(COMPOSER_EFFORT_KEY) ?? '')
export const $currentServiceTier = atom('')
export const $currentFastMode = atom(storedBoolean(COMPOSER_FAST_KEY, false))
// Mirror of config.yaml `desktop.reset_model_on_new_session` (default false).
// When true, a fresh chat reseeds the composer model to the profile default
// instead of carrying the last-picked model forward. Refreshed from backend
// config by useHermesConfig; a plain reflection, not its own persisted state.
export const $resetModelOnNewSession = atom(false)
// Effective approval-bypass state mirrored from the gateway (session.info).
// Persistence lives in the backend config (approvals.mode), so this is a plain
// reflection of the truth the gateway reports rather than its own store.
Expand Down Expand Up @@ -331,6 +336,9 @@ export const setCurrentFastMode = (next: Updater<boolean>) => {
persistBoolean(COMPOSER_FAST_KEY, $currentFastMode.get())
}

export const setResetModelOnNewSession = (next: Updater<boolean>) =>
updateAtom($resetModelOnNewSession, next)

export const setYoloActive = (next: Updater<boolean>) => updateAtom($yoloActive, next)

export const setCurrentCwd = (next: Updater<string>) => {
Expand Down
6 changes: 6 additions & 0 deletions apps/desktop/src/types/hermes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,12 @@ export interface HermesConfig {
max_recording_seconds?: number
auto_tts?: boolean
}
desktop?: {
// When true, a fresh chat (Cmd+N / app relaunch) reseeds the composer model
// to the profile default instead of carrying the last-picked model forward.
// Default false preserves the sticky-last-pick behavior.
reset_model_on_new_session?: boolean
}
}

export type HermesConfigRecord = Record<string, unknown>
Expand Down
Loading