Skip to content

fix(desktop): refresh default-derived composer model - #58527

Closed
kohoj wants to merge 1 commit into
NousResearch:mainfrom
kohoj:codex/fix-desktop-composer-model-source-58498
Closed

kohoj wants to merge 1 commit into
NousResearch:mainfrom
kohoj:codex/fix-desktop-composer-model-source-58498

Conversation

@kohoj

@kohoj kohoj commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Problem

Fixes #58498.

Desktop can persist the composer model/provider in localStorage and then send that pair on session.create as a per-session override. The same persistent slot was used for two different meanings:

  • a real manual model-picker choice, which should persist across new chats/restarts
  • a mirror of the profile default loaded from /api/model/info, which should keep following config.yaml

When the profile default changed outside the renderer (for example CLI/config now says provider: openai-codex, default: gpt-5.5), a stale default-derived composer value could still be sent as a no-session override. That matches the reporter's dump: config.yaml showed OpenAI Codex, while the desktop turn was built as provider=nous model=openai/gpt-5.5.

Root Cause

refreshCurrentModel() skipped any non-empty composer model, because it could not tell a user picker choice from a default mirror. The second post-fetch race guard had the same unconditional skip.

Fix

Add a tiny persisted source marker for the composer model:

  • manual for explicit desktop model-picker selections
  • default for profile-default refreshes and Settings -> Model saves
  • missing/legacy source is treated as default-derived, so stale v0.18.0 localStorage can self-heal

refreshCurrentModel() now preserves only manual choices. Default-derived and legacy composer state refreshes from /api/model/info, so new Desktop chats stop overriding the correct OpenAI Codex config with a stale Nous route.

Tests

  • npm --workspace apps/desktop run test:ui -- src/app/session/hooks/use-model-controls.test.tsx
  • npm --workspace apps/desktop run typecheck
  • cd apps/desktop && npx eslint src/app/desktop-controller.tsx src/app/session/hooks/use-model-controls.ts src/app/session/hooks/use-model-controls.test.tsx src/store/session.ts

@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have labels Jul 4, 2026
@yingliang-zhang

Copy link
Copy Markdown
Contributor

Replaced by #62486 — same fix re-opened from yingliang-zhang fork so CI can run. Thanks @kohoj for the original work.

@yingliang-zhang

Copy link
Copy Markdown
Contributor

Replaced by #62486 — same fix re-opened from yingliang-zhang fork so CI can run. Thanks @kohoj for the original work. Would a maintainer please close this PR in favor of #62486?

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for isolating the persisted-state distinction. The premise is confirmed on current main: refreshCurrentModel() returns for every non-empty composer model at apps/desktop/src/app/session/hooks/use-model-controls.ts:49 and repeats that condition after the async fetch at :55. New sessions then forward those stored values as explicit overrides at apps/desktop/src/app/session/hooks/use-session-actions/index.ts:211-228.

The source-marker change in commit 5828a4099eae addresses this exact path: preserve only picker-originated state, let legacy/default-derived state refresh from /api/model/info, and restore the previous source on failed active-session changes. The regression test covers legacy state transitioning to the current profile default.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
@embwl0x

embwl0x commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

The source marker here fixes the stale/default-derived state path: legacy or default-derived composer state refreshes from the active profile, while an intentional picker choice remains sticky.

One complementary invariant from #62055 is still worth retaining: a valid manual picker choice can differ from Settings and silently affect the model and billing for every new chat. The Desktop should disclose that intentional override before session.create, for example with a warning state and tooltip on the model pill whenever the manual selection differs from the current profile default.

I validated that approach locally by carrying the profile default into Desktop memory only, deriving the mismatch in the model controls, and covering the pill/control behavior. The focused Desktop suite passed 36 tests, plus typecheck and the production build. I am closing #62072 rather than racing this canonical fix, but wanted to preserve the tested UX invariant here.

@OutThisLife

Copy link
Copy Markdown
Contributor

Closing as superseded by #65896 (merged), which salvaged this fix (and #62486) for #58498 — refresh default-derived composer model/provider so sticky localStorage mirrors of the profile default keep following config.yaml.

Your diagnosis and approach are what landed; authorship was preserved in the salvage. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Hermes Desktop ignores OpenAI Codex provider and routes GPT-5.5 requests through Nous Portal (CLI works correctly)

6 participants