Skip to content

Fix legacy @provider session models#1129

Closed
franksong2702 wants to merge 1 commit intonesquena:masterfrom
franksong2702:franksong2702/session-model-at-prefix-normalization
Closed

Fix legacy @provider session models#1129
franksong2702 wants to merge 1 commit intonesquena:masterfrom
franksong2702:franksong2702/session-model-at-prefix-normalization

Conversation

@franksong2702
Copy link
Copy Markdown
Contributor

Thinking Path

What Changed

  • Added @provider:model handling to _resolve_compatible_session_model().
  • Normalizes redundant active-provider hints to the bare model, for example @openai-codex:gpt-5.4-mini -> gpt-5.4-mini.
  • Preserves still-routable non-active provider hints, for example visible Copilot dropdown selections.
  • Recovers stale/unroutable hints to the current active provider family or default model.
  • Added regression coverage for stale Copilot hints, active-provider hints, valid cross-provider hints, and family mismatch fallback.
  • Added a changelog entry.

Why It Matters

Older sessions can otherwise stay pinned to a provider that is no longer routable. In that state, a session can appear to send but never show an assistant reply until the stored model is manually edited. This keeps old sessions usable without a bulk migration or UI change.

Fixes #1128.
Refs #1023.
Refs #1029.

Verification

  • python -m py_compile api/routes.py
  • python -m pytest tests/test_provider_mismatch.py -k "session_model or provider_model or stale_at_provider or legacy_at_provider or active_at_provider or routable_non_active" -v
  • python -m pytest tests/test_provider_mismatch.py tests/test_model_resolver.py tests/test_credential_pool_providers.py tests/test_issue895_894_nous_prefix.py -v

Risks / Follow-ups

  • The fix is intentionally conservative: if a non-active provider hint is still visible/routable in the current catalog, it is preserved.
  • This does not bulk-migrate historical session files. Existing write-path normalization persists only when a stale explicit model is encountered.
  • No UI behavior changes, so no screenshots are included.

Model Used

  • Provider: OpenAI Codex
  • Model: GPT-5.5
  • Mode/tooling: local code inspection, shell tests, GitHub CLI

@nesquena-hermes
Copy link
Copy Markdown
Collaborator

Thanks for the PR, @franksong2702. The approach looks solid — the three-case decision tree (normalize active-provider hints, preserve still-routable non-active hints, recover stale/unroutable hints) handles the key constraint cleanly: @provider:model is a valid live format and can't be blindly stripped.

A few notes after reviewing:

Logic correctness

  • The active-provider normalization (e.g. @openai-codex:gpt-5.4-minigpt-5.4-mini when openai-codex is active) is the right conservative path.
  • Preserving routable non-active hints avoids breaking intentional cross-provider dropdown selections, which is the main footgun here.
  • The family-mismatch fallback to the current default model is safe.

Test coverage
The four regression cases (stale Copilot hint, active-provider hint, valid cross-provider hint, family mismatch fallback) cover the important boundary conditions. Good to see these added alongside the fix.

Minor

  • No UI changes and no bulk migration needed aligns with keeping this narrow — appreciated.
  • The _resolve_compatible_session_model() placement keeps this consistent with the existing #1023 / #1029 recovery chain.

Overall looks like a clean, well-scoped fix. Pending maintainer review.

@nesquena-hermes
Copy link
Copy Markdown
Collaborator

Reviewed — this is clean and the fix is correct. The four-branch logic (active-provider hint → strip, routable non-active → preserve, family match → strip, mismatch → default) is exactly right and the test coverage is solid.

Queuing for the next batch release rather than releasing solo — expect it to ship in the next round alongside a couple of other incoming PRs. No changes needed on your end.

@nesquena-hermes
Copy link
Copy Markdown
Collaborator

Absorbed and shipped in v0.50.224 (PR #1131). Thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(sessions): legacy @provider:model session models bypass stale-model recovery

2 participants