Skip to content

Fix browser chat provider fallback on fresh sessions#3087

Merged
1 commit merged into
nesquena:masterfrom
george-andraws:fix/chat-start-provider-fallback
May 28, 2026
Merged

Fix browser chat provider fallback on fresh sessions#3087
1 commit merged into
nesquena:masterfrom
george-andraws:fix/chat-start-provider-fallback

Conversation

@george-andraws
Copy link
Copy Markdown
Contributor

Thinking Path

  • Hermes WebUI should honor the composer model selection on the first browser turn, not just after session metadata is fully hydrated.
  • The bug path was in the frontend send flow: it already fell back to the dropdown for the model ID, but not for the provider.
  • That meant a fresh or partially hydrated session could send grok-4.3 with model_provider=null, letting backend resolution drift to the wrong default provider.
  • The safest fix was frontend-only: keep S.session.model authoritative, and only derive a provider fallback when it belongs to the exact model being sent.
  • This keeps the change low risk and avoids touching backend routing logic.

What Changed

  • Added static/ui.js::_modelProviderForSend(modelId).
    • Preserves S.session.model_provider when present.
    • Falls back to an explicit provider encoded in the model value when available.
    • Otherwise falls back to dropdown or persisted model state only when that state matches the outgoing model exactly.
  • Added small payload helpers in static/messages.js so browser chat start and queued follow-up payloads all use the same model/provider resolution.
  • Added regression coverage in tests/test_chat_start_provider_fallback.py.
  • Added an Unreleased changelog note.

Why It Matters

  • Fixes wrong-provider routing on fresh browser turns and other partial-hydration cases.
  • Keeps session-model precedence intact.
  • Avoids stealing a provider from an unrelated dropdown selection.

Verification

Ran from the isolated worktree with /Users/georgeandraws/hermes-webui/.venv/bin/python:

  • python -m pytest tests/test_chat_start_provider_fallback.py -q5 passed
  • python -m pytest tests/test_new_chat_default_model_frontend.py::test_new_chat_does_not_send_stale_dropdown_model_when_session_has_default_model tests/test_regressions.py::test_send_uses_session_model_as_authoritative_source tests/test_chat_start_provider_fallback.py -q7 passed
  • node -c static/ui.js && node -c static/messages.js
  • python -m pytest tests/test_parallel_session_switch.py::TestGitInfoParallel::test_parallel_faster_than_serial -q1 passed
  • python -m pytest tests -q
    • Result after fix: only two remaining failures, both reproduced unchanged on pristine origin/master:
      • tests/test_workspace_git.py::test_git_commit_selected_rejects_conflicts_and_path_traversal
      • tests/test_workspace_git.py::test_git_stash_and_checkout_is_explicit
    • These fail because local git now defaults temp repos away from master, so git checkout master errors before product code is exercised.

Risks / Follow-ups

  • This PR intentionally stays in the browser chat path. It does not broaden the same fallback into unrelated flows.
  • The two workspace_git failures are baseline test assumptions unrelated to this diff and should be fixed separately.

AI Usage Disclosure

  • OpenAI Codex CLI via npx -y @openai/codex exec
  • Model: gpt-5.5
  • Mode: workspace-write / full-auto, reasoning effort medium
  • Parent-session verification, test triage, and PR authoring were done manually in Hermes.

Model Used

  • Codex CLI gpt-5.5 for the implementation pass
  • Hermes parent session gpt-5.4 for verification and PR prep

@nesquena-hermes
Copy link
Copy Markdown
Collaborator

Read the diff against origin/master. The fix is small, well-scoped, and the test approach is solid — wiring up a real Node-based driver for _modelProviderForSend rather than reimplementing the logic in Python. Approving the direction; a few notes.

Logic verified

_modelProviderForSend at static/ui.js:900-927 keeps the precedence sensible:

const sessionProvider=(S&&S.session&&S.session.model_provider)||null;
if(sessionProvider) return sessionProvider;
const model=String(modelId||'').trim();
if(!model) return null;
const explicitProvider=typeof _providerFromModelValue==='function'
  ? _providerFromModelValue(model)
  : '';
if(explicitProvider) return explicitProvider;

Order: session model_provider (authoritative) → encoded @provider:model in the model string → dropdown state but only when the dropdown value matches the outgoing model exactly → persisted state under the same equality guard → null.

The "same outgoing model" guard at line 911 is the key correctness fix — without it, a fresh session reading the dropdown could grab the provider of whatever was previously selected (e.g. user opened the dropdown to browse Gemini, clicked back to a session that has grok-4.3 typed in, and sends would otherwise attach model_provider=google). The string equality check prevents that.

One concern: leading-zero _providerFromModelValue interaction

Looking at static/ui.js:866-880 (the existing helper), _providerFromModelValue extracts a provider hint from @provider:model syntax. The new helper calls it before checking dropdown state. That's fine, but it means a model_provider typed by the user in @openai:gpt-5.5 form will override anything the dropdown thinks, even when the dropdown is showing @anthropic:gpt-5.5. That's almost certainly the desired behavior (explicit beats inferred), but worth a comment in _modelProviderForSend to make the precedence intentional rather than incidental.

Tests

tests/test_chat_start_provider_fallback.py has two parts:

  1. A grep-based source assertion (lines 20-37) that catches drift if the payload construction stops going through _chatPayloadModelState.
  2. A Node driver (_DRIVER_SRC from line 40 onward) that loads static/ui.js, extracts _modelProviderForSend, and runs scenarios. Smart — this is closer to a real unit test than the Python-port pattern other recent PRs have used.

One gap: there's no test for the case where S.session.model and the dropdown disagree, e.g. session=grok-4.3 session.model_provider=null, dropdown=gpt-5.5 provider=openai. With the new code, sending should produce model=grok-4.3, model_provider=null (not openai), because _chatPayloadModel() returns S.session.model first and the dropdown-state branch's equality guard then fails. Worth an explicit assertion — it's the exact "wrong-provider routing on fresh browser turns" case the PR description calls out.

Cross-repo

The backend at the agent side (hermes_cli/auth.py or agent/agent_init.py depending on which provider you're routing to) will still receive model_provider=null and have to resolve a default. This PR doesn't change that — it just stops corrupting the field with a mismatched dropdown selection. Good scope discipline.

Other queue/interrupt sites

The diff updates four payload-construction sites consistently:

  • static/messages.js:300 (queue mode)
  • static/messages.js:360 (interrupt mode)
  • static/messages.js:374 (default queue)
  • static/messages.js:539 (the actual /api/chat/start POST)
  • static/messages.js:601 (busy-retry queue)
  • static/messages.js:1778, :2010 (live-stream continuation paths)

All routed through _chatPayloadModelState. That coverage is what I'd expect for the fix to be complete. The PR description says "intentionally stays in the browser chat path" — confirmed by the diff.

Recommend merge once the additional regression test for session.model != dropdown.value is added.

@nesquena-hermes nesquena-hermes closed this pull request by merging all changes into nesquena:master in 5ec136a May 28, 2026
@nesquena-hermes
Copy link
Copy Markdown
Collaborator

Shipped in v0.51.156 / Release EB (stage-batch38, commit 5ec136a). Thanks for the contribution!

AJV20 pushed a commit to AJV20/hermes-webui that referenced this pull request May 28, 2026
# Conflicts:
#	CHANGELOG.md
AJV20 pushed a commit to AJV20/hermes-webui that referenced this pull request May 28, 2026
2-PR Tier B cleanup:
- nesquena#3084 harden WebUI request/session/runtime edges (auth.py thread-safety + body validation + frontend storage guards + sw.js vendor precache + i18n key fills + workspace_git test default-branch)
- nesquena#3087 model_provider fallback only when same model — fixes fresh-session model_provider=null bug
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.

2 participants