Skip to content

fix(desktop): recover composer model pill after rapid profile switches - #47537

Open
LeonSGP43 wants to merge 1 commit into
NousResearch:mainfrom
LeonSGP43:fix-47524-model-pill-refresh
Open

fix(desktop): recover composer model pill after rapid profile switches#47537
LeonSGP43 wants to merge 1 commit into
NousResearch:mainfrom
LeonSGP43:fix-47524-model-pill-refresh

Conversation

@LeonSGP43

Copy link
Copy Markdown
Contributor

Summary

  • retry forced composer model refreshes during profile swaps so transient backend startup races do not leave the model pill spinning forever
  • ignore stale refresh completions from older profile switches so late responses cannot overwrite the latest profile default
  • cover the retry path and stale-result race with focused desktop hook tests

Verification

  • npm --workspace apps/desktop exec -- vitest run --environment jsdom src/app/session/hooks/use-model-controls.test.tsx
  • npm --workspace apps/desktop exec -- eslint src/app/session/hooks/use-model-controls.ts src/app/session/hooks/use-model-controls.test.tsx
  • git diff --check

Closes #47524.

@alt-glitch alt-glitch added type/bug Something isn't working comp/tui Terminal UI (ui-tui/ + tui_gateway/) P3 Low — cosmetic, nice to have labels Jun 17, 2026
@alt-glitch alt-glitch added comp/desktop Electron desktop app (apps/desktop/*) and removed comp/tui Terminal UI (ui-tui/ + tui_gateway/) labels Jun 26, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the focused retry and request-ordering coverage. The current-main premise is still valid: the profile-switch effect calls refreshCurrentModel(true) in apps/desktop/src/app/desktop-controller.tsx:661-670, while the current hook silently drops a failed model-info request at apps/desktop/src/app/session/hooks/use-model-controls.ts:53-68.

Problems

  • The new bounded loop still returns after its final failed request at apps/desktop/src/app/session/hooks/use-model-controls.ts:96-100 without changing state or exposing recovery. Since ModelPill renders an empty $currentModel as GlyphSpinner (apps/desktop/src/app/chat/composer/model-pill.tsx:56-60), this can still leave the user with an indefinite spinner. The new test at use-model-controls.test.tsx:212-235 covers recovery before exhaustion, but not this terminal path.

Suggested changes

  • Add a retryable exhausted-refresh state and a visible action that re-runs the refresh, then cover the all-attempts-fail path. Keep the request-id ordering guard, which correctly addresses the overlapping-refresh case.

Automated hermes-sweeper review.

}
const retryDelay = retryDelays[attempt]

if (typeof retryDelay !== 'number') {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

On the fourth failed request retryDelays[attempt] is undefined, so this returns with $currentModel still empty; ModelPill then keeps rendering its spinner. Please surface a retryable failure/recovery action here and add an exhaustion-path test rather than leaving the terminal state indistinguishable from loading.

@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 area/profiles Multi-profile isolation, HERMES_HOME scoping labels Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/profiles Multi-profile isolation, HERMES_HOME scoping 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]: Composer model pill stuck in loading spinner after rapid profile switching

3 participants