Skip to content

fix(agent): stop /model calling a cloud model local in the switch message - #4365

Merged
itomek merged 3 commits into
mainfrom
fix/switch-message-cloud-lookup
Sep 29, 2026
Merged

itomek merged 3 commits into
mainfrom
fix/switch-message-cloud-lookup

Conversation

@itomek

@itomek itomek commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Switching to a cloud model in the TUI could tell you your conversation was staying on your machine when it was being sent to Fireworks. /model answered with an affirmative privacy assurance — "GAIA is not sending this conversation to any cloud provider" — for a model it had just routed to the cloud. That contradiction is the exact thing #3674 and #3706 set out to remove; this closes the one path they missed.

It only bites when the model's id prefix isn't fireworks. or amd. — the switchable-model filter accepts any prefix so long as the catalog's own cloud_provider field names a known provider, but the switch message was classifying from the prefix alone. Both the switch message and the system prompt rebuilt alongside it now read the catalog before they answer.

Found while reviewing #3706.

Test plan

  • pytest hub/agents/gaia/python/tests/test_stdio.py — 79 passed
  • pytest tests/unit/test_inference_location.py — 27 passed
  • python util/lint.py --black --isort --flake8 — 3/3 passed
  • The regression test fails without the production fix. Remove the refresh_model_catalog warm-up from _apply_local_switch and test_switch_message_classifies_from_the_client_not_the_id_prefix fails:
    E assert 0 == 1 E + where 0 = <_CatalogAwareClient object>.refresh_calls
🔍 Technical details

The bug

_lemonade_models (stdio.py) filters with the two-argument
cloud_model_provider(m["id"], m), which returns
metadata.get("cloud_provider") or provider for a recipe: cloud entry — so an
id like custom.gemma-4-31b-it carrying cloud_provider: "fireworks" is
switchable.

run_model_command then called the one-argument form, which only matches
provider in {"fireworks", "amd"} and returns None otherwise. None is what
resolve_inference_location reads as local, so the local sentence was emitted
for a cloud session.

Why passing the live client wasn't enough on its own

The first commit passed cloud_provider_lookup from agent.chat.llm_client.
That client is the one _apply_local_switch had just built via create_client,
and cloud_model_provider only recognises a runtime-discovered provider once
list_models() has populated _model_metadata. Nothing warmed it, so it still
answered None for exactly the target case — a no-op, confirmed against a real
Lemonade client.

The same cold client is handed to _apply_switch, which calls
rebuild_system_prompt(). So the system prompt had the identical gap: the
original framing here — that the prompt already consulted the client and only
the switch message disagreed — was wrong. It was a two-site problem.

The fix

LemonadeProvider.refresh_model_catalog() (providers/lemonade.py) reads the
catalog once. _apply_local_switch calls it immediately after create_client
and before _apply_switch, so the warm client reaches both the switch
message and the prompt rebuild. The callable() guard keeps the id-prefix rule
as the documented fallback for a client that has no such method.

Deliberately not calling agent._inference_location() even though it would
be shorter: that method lives on ChatAgent, and reaching for it here would
couple gaia's stdio to the chat package's private surface and break every
_ModelSwitchAgent fake in the suite.

No doc surface asserts the old behaviour — the sentence is composed at runtime,
and grep over docs/, hub/ and src/ finds it only in
inference_location.py and this test file. hub/agents/gaia/python/ ships no
README/SPEC/SKILL/CHANGELOG to keep in sync.

…sage

Switching to a cloud model whose id prefix is not `fireworks.` or `amd.`
answered with the LOCAL sentence:

    Switched to **custom.gemma-4-31b-it**. Chat inference runs through
    Lemonade on this machine, model `custom.gemma-4-31b-it`. GAIA is not
    sending this conversation to any cloud provider.

That model is served by Fireworks. The user gets an affirmative privacy
assurance that is false, and the system prompt — which does consult the
client — then says the opposite, which is the contradiction #3674 set out
to remove.

The switchable-model filter calls `cloud_model_provider(id, metadata)` and
honours the catalog's own `cloud_provider`, so a model is switchable under
any prefix. The switch message called the one-argument form, which knows
only the two hard-coded prefixes and returns None for everything else —
and None means local. It now passes the live client's classifier, exactly
as `ChatAgent._inference_location` already does.

The regression test drives `/model` with a catalog entry whose
`cloud_provider` is `fireworks` behind a `custom.` prefix, and fails on the
old code with the message quoted above.
@github-actions

Copy link
Copy Markdown
Contributor

Request changes

This PR sets out to stop /model telling you a cloud model is running locally — a real and worth-fixing privacy-message bug. As written, though, the change doesn't actually fix it, and the new test passes only because its stand-in client behaves in a way the real one doesn't.

The classifier the fix now consults belongs to the client the switch just created. That client only learns which models are cloud-hosted after it has fetched the catalog, and nothing between creating it and composing the message makes it do that — so it answers "not a cloud model" for exactly the case this PR targets, and the same local-sounding sentence goes out as before. The catalog was read moments earlier, by the throwaway client that validated the model id; carrying that answer forward (or warming the new client once before the message is composed) is what would close the gap. Warming it would also cover the system prompt, which is rebuilt during the same switch from the same cold client and so likely repeats the claim — worth checking, because if so this isn't a stdio-only gap and the PR's "the prompt already says the opposite" framing needs revisiting.

The test's stub answers "Fireworks" unconditionally, which the real client would only do after reading the catalog. That's the pattern CLAUDE.md calls out from #1655: the stub proves the call was made, not that it returns anything useful. Whatever the fix ends up being, the test needs a stand-in that starts cold like the real one.

Real-world evidence

The evidence stage ran the changed path against a real Lemonade HTTP client and reports the fix is a no-op for its target case — output byte-identical before and after:

$ (stdin) /model custom.gemma-4-31b-it
{"type":"final","answer":"Switched to **custom.gemma-4-31b-it**. Chat inference runs
 through Lemonade on this machine, model `custom.gemma-4-31b-it`. GAIA is not sending
 this conversation to any cloud provider."}
[live client] LemonadeProvider; cloud_model_provider('custom.gemma-4-31b-it') -> None

Same events on PR HEAD and on its parent. The known-prefix case (fireworks.kimi-k2-instruct) is announced as remote on both refs, unchanged. A component probe shows the wiring itself is sound — the same classifier returns fireworks after the client reads the catalog:

=== component probe: real LemonadeProvider, before/after it reads the catalog ===
fresh client   : None
after catalog  : 'fireworks'

Spot regression on adjacent operations was clean: no-arg /model list, unknown-model error, Lemonade-down error, and entry --help all behaved. Deferred: the full python -m gaia_agent.stdio process (agent construction preloads a model, impossible on this runner) and the TUI/Agent UI rendering — pending strix-halo lane. The /model claude-* branch and the HTTP sidecar switch were N/A here (credentials / fastapi absent). This evidence is the basis for the verdict; the static review agrees with it.

🔍 Technical details

🔴 The fix doesn't change the behaviour it targets (hub/agents/gaia/python/gaia_agent/stdio.py:700-707)

agent.chat.llm_client at that point is the client _apply_local_switch built at stdio.py:590 via create_client. LemonadeProvider.cloud_model_provider delegates to LemonadeClient.cloud_model_provider (lemonade_client.py:2857), which passes self._model_metadata.get(model) — and _model_metadata is populated only by list_models (lemonade_client.py:2851-2854). Nothing calls list_models on that client: _lemonade_models (stdio.py:421) builds its own throwaway LemonadeClient for the availability check, and _apply_switch does no catalog read. So metadata is None and the module rule applies — None for custom.*, i.e. the local sentence. Confirmed empirically by the evidence bundle above.

Two shapes that would work:

  1. Carry the catalog answer forward. _lemonade_models already computes cloud_model_provider(m["id"], m) per entry (stdio.py:446-448) against a client that has the metadata. Return {id: provider} alongside the id list, and pass cloud_provider_lookup=lambda _id, p=provider: p in run_model_command. No extra HTTP.
  2. Warm the new client once inside _apply_local_switch, right after create_client and before _apply_switch runs rebuild_system_prompt(). One extra models?show_all=true round-trip, but it fixes the switch message and the system prompt together — see below.

Shape 2 is the stronger one if the prompt is affected.

🟡 The system prompt may have the same cold-client problem (hub/agents/chat/python/gaia_agent_chat/agent.py:886-908)

ChatAgent._inference_location reads the same chat.llm_client, and rebuild_system_prompt() is invoked from _apply_switch (stdio.py:533) with that same never-warmed client. If so, the prompt says "local" too, and the PR description's premise — that the switch message contradicts a prompt which does consult the client — doesn't hold. Please verify before settling on the fix; it decides whether this is a one-site or two-site problem.

🟡 The new test's stub can't fail (hub/agents/gaia/python/tests/test_stdio.py:1058-1063)

class _CatalogAwareClient:
    def cloud_model_provider(self, model_id):
        return "fireworks" if model_id == model else None

Its docstring says "What create_client really returns — a LemonadeClient, which carries cloud_model_provider and reads the catalog metadata." The real one carries the method but answers None until list_models has run; this stub answers correctly from birth. That is CLAUDE.md's "mocks prove we called it, not the call is valid" — the #1655 pattern verbatim.

A stub that starts cold and only classifies after a list_models call would have caught this, and would keep catching it:

class _CatalogAwareClient:
    """Mirrors LemonadeClient: classifies a discovered cloud model only
    once it has read the catalog."""

    def __init__(self):
        self._metadata = {}

    def list_models(self, show_all=False):
        self._metadata[model] = {"recipe": "cloud", "cloud_provider": "fireworks"}
        return stub_lemonade.catalog

    def cloud_model_provider(self, model_id):
        meta = self._metadata.get(model_id)
        return meta and (meta.get("cloud_provider"))

Strengths

  • The bug is real and well-diagnosed: the switchable-model filter uses the two-argument cloud_model_provider(id, metadata) while the message used the one-argument form, and the two genuinely disagree for a catalog-declared cloud model under an unknown prefix.
  • The callable() guard and the decision not to reach for ChatAgent._inference_location from gaia's stdio are both right calls — the PR description explains the coupling it avoided.
  • The PR description does the doc-surface sweep (docs/, hub/, src/) instead of asserting it.

The previous fix passed the newly switched client's cloud_model_provider
into the switch message, but that client had never called list_models(),
so its catalog metadata was empty and it answered "not a cloud model"
for exactly the case being fixed — a no-op confirmed against a real
Lemonade client. The system prompt, rebuilt from the same client
immediately after a switch, had the identical gap.

_apply_local_switch now reads the catalog once (LemonadeProvider.
refresh_model_catalog) right after building the new client and before
handing it to _apply_switch, so both the switch message and the
rebuilt system prompt classify a runtime-discovered cloud model
correctly.

The regression test now starts its stub client cold, like the real one,
and only classifies after its own refresh_model_catalog call — the old
version answered correctly from construction and could not have caught
this.
@github-actions github-actions Bot added llm LLM backend changes performance Performance-critical changes labels Sep 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Skill audit

Skill Verdict Claimed tier Cleared tiers Findings Rules
.claude/skills/benchmarking-the-agent ✅ ALLOW experimental experimental, community none —
.claude/skills/gaia-executive-presentation ✅ ALLOW experimental experimental, community none —
.claude/skills/gaia-technical-presentation ✅ ALLOW experimental experimental, community none —
.claude/skills/github-issue-response ✅ ALLOW experimental experimental, community none —
.claude/skills/weekly-audit-patterns ✅ ALLOW experimental experimental, community none —
hub/agents/email/npm ✅ ALLOW experimental experimental, community 2 medium, 1 info code.suppression, supply.undeclared_dependency
hub/agents/email/python/gaia_agent_email/skills/meeting-scheduling ✅ ALLOW experimental experimental, community none —
hub/agents/gaia/npm ✅ ALLOW experimental experimental 1 high body.injection.credential_request
hub/agents/gaia/python/gaia_agent/skills/document-extract ✅ ALLOW community experimental, community none —
hub/agents/gaia/python/gaia_agent/skills/gaia-voice ✅ ALLOW community experimental, community none —
hub/skills/coding ✅ ALLOW community experimental, community 4 info permission.unused
hub/skills/github-triage ✅ ALLOW community experimental, community 1 info permission.unused
hub/skills/image-gen ✅ ALLOW community experimental, community none —
hub/skills/inbox-triage ✅ ALLOW community experimental, community none —
hub/skills/transcribe-meeting ✅ ALLOW community experimental, community none —

✅ All audited skills cleared the tier they claim.

Per-finding detail is withheld here on purpose. Read it in the Security > Code scanning tab, or download the skill-audit-reports artifact from this run. Offending source text is withheld from CI everywhere — reproduce it locally with gaia skill audit <dir> --show-snippets.

@itomek

itomek commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

The earlier review was right, and the fix it asked for landed in ded78fde — four days after that review was written, so the review reads against the wrong commit. The client the switch message consults is now warmed before it answers, which fixes the switch message and the rebuilt system prompt together. Every remaining red check is already red on main and none of them touch this diff.

The three points from that review, each resolved:

  1. "The fix is a no-op — the client is never warmed." Correct against 897f0c94. ded78fde adds LemonadeProvider.refresh_model_catalog() and calls it in _apply_local_switch right after create_client, before the message is composed. This is that review's own suggested shape 2.
  2. "The system prompt probably has the same cold-client problem." It did. The warm-up sits before _apply_switch, which is what calls rebuild_system_prompt(), so one fix covers both sites. The PR description has been corrected — it previously claimed the prompt already consulted the client, which was wrong.
  3. "The test's stub can't fail." Also correct, and also fixed. The stub now starts cold and only classifies after its own catalog read. Verified by removing the production warm-up and watching the test fail.
🔍 Verification and CI triage

The regression test is real

Removing the refresh_model_catalog warm-up from _apply_local_switch and re-running:

$ pytest hub/agents/gaia/python/tests/test_stdio.py -k switch_message
FAILED test_switch_message_classifies_from_the_client_not_the_id_prefix
E   assert 0 == 1
E    +  where 0 = <_CatalogAwareClient object>.refresh_calls
1 failed, 1 passed, 77 deselected

Restored, everything green:

$ pytest hub/agents/gaia/python/tests/test_stdio.py tests/unit/test_inference_location.py -q
106 passed in 2.92s

$ python util/lint.py --black --isort --flake8
3/3 passed

Ordering confirmed by reading the source: _apply_local_switch warms the
catalog, then calls _apply_switch(..., new_client=new_client), which calls
agent.rebuild_system_prompt(). One gap worth naming: the tests assert the
warm-up happened, not the rebuilt prompt's content.

Red checks — all pre-existing on main

The branch was 153 commits behind main; it has been updated, so the run below
is against current ground truth.

Check Cause Evidence
8 × Scenario eval - *, gaia_* judged categories vs committed baselines Repo-wide eval infra eval_flagship.yml on main has failed 5 consecutive runs since 2026-09-26; last success was 36113705061 on 2026-09-25
Unit Tests (Windows smoke) test_client_disconnect_cancels_the_agent — a threading race (assert False = wait(timeout=5)), 1 failed / 1948 passed Failing on main in run 36576942529. #4421 is open against exactly that file
Unit Tests (macOS smoke) test_memory_turn_recall.py::test_the_relevant_note_reaches_the_turn_with_its_age, 1 failed / 12994 passed Failing on main in the same run
Test Agent SDK on Windows (Lemonade Integration) test_performance_integration — live model returned empty response.text Passed on this PR's own previous run

Worth flagging for anyone reading check status on this repo: main run
36576942529 reports a run-level conclusion: success while its job list shows
both Unit Tests (Windows smoke) and Unit Tests (macOS smoke) as failure.
Only Unit Tests (Linux, py3.12) actually passed. Reading the run conclusion
alone gives the wrong answer about whether main is green.

This diff touches three files — stdio.py, test_stdio.py, and
providers/lemonade.py (purely additive: one new method plus a docstring). No
failing test exercises any of them.

@itomek
itomek added this pull request to the merge queue Sep 29, 2026
Merged via the queue into main with commit 4d883aa Sep 29, 2026
44 of 56 checks passed
@itomek
itomek deleted the fix/switch-message-cloud-lookup branch September 29, 2026 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llm LLM backend changes performance Performance-critical changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants