Skip to content

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

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

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

Conversation

@yingliang-zhang

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

Replaces #58527 (source branch was on kohoj fork, re-opened from yingliang-zhang fork to enable CI). Original author: @kohoj

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) area/config Config system, migrations, profiles duplicate This issue or pull request already exists labels Jul 11, 2026
@alt-glitch

alt-glitch commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Related: #65896 merged the source-marker repair on the current Desktop paths. This older branch still edits the deleted desktop-controller.tsx entrypoint, so it is no longer a valid duplicate anchor; #58527 remains historical related work.

@yingliang-zhang

Copy link
Copy Markdown
Contributor Author

Thanks for the triage. To clarify: #58527 was opened from the kohoj fork, which does not have CI access (GitHub Actions can't run on fork PRs without explicit maintainer approval). #62486 is the re-opened version from the yingliang-zhang fork specifically to enable CI runs — the PR body already notes "Replaces #58527". The code is identical; this is not a competing duplicate but a CI-enabled replacement. #58527 can be closed once #62486 is merged.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state 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 11, 2026
@OutThisLife

Copy link
Copy Markdown
Contributor

Rebased onto current main in #65896 (authorship preserved — commit by @kohoj; thanks @yingliang-zhang for re-opening it for CI). After ~715 commits it no longer applied:

  • desktop-controller.tsx (the file this patched) was deleted on main — refactored into contrib/wiring.tsx. Moved the onMainModelChangedsetCurrentModelSource('default') write there.
  • use-model-controls.ts conflicted (main renamed activeSessionIdliveSessionId and dropped the hook's activeSessionId prop). Re-applied the manual/default source logic on the current shape and fixed the new test to the current signature.

Behavior is identical to your fix; only the wiring moved. Green on #65896: full vitest --project ui (1314), typecheck, lint, build. Superseding here; will close once #65896 merges. Nice diagnosis — the source-marker split is exactly right.

@alt-glitch alt-glitch added needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists and removed duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have labels Jul 16, 2026
@OutThisLife

Copy link
Copy Markdown
Contributor

Landed via #65896 (merged). Closing as superseded — thanks @kohoj / @yingliang-zhang.

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/*) needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists 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 sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state 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)

5 participants