Skip to content

fix(ui): classify chat helper models with the shared cloud catalog - #6926

Merged
kovtcharov-amd merged 1 commit into
amd:mainfrom
ityashag:fix/chat-helpers-shared-cloud-catalog
Nov 7, 2026
Merged

kovtcharov-amd merged 1 commit into
amd:mainfrom
ityashag:fix/chat-helpers-shared-cloud-catalog

Conversation

@ityashag

@ityashag ityashag commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Six cloud-model checks in src/gaia/ui/_chat_helpers.py now use the existing _ui_cloud_provider, which also reads the shared model catalog. Before, a cloud model from a provider GAIA only learns about at runtime, like opencode.*, counted as local in those places.

Why

The review on #6907 pointed out these call sites. They called cloud_model_provider(model_id) with no metadata, which only recognizes the fireworks and amd prefixes. The module already has _ui_cloud_provider for this, and other callers in the file use it.

The most important one is _title_model_for. It exists so "a local chat's content never leaves the PC just to name it." With a guard model as the session model and a runtime-discovered cloud model as the default, that check missed the cloud default, so a local chat could be sent to it just to generate a title.

Linked issue

Refs #4784, which #6907 fixes in LemonadeClient. This PR is independent of #6907 and doesn't touch lemonade_client.py.

Changes

These call sites now use _ui_cloud_provider(model_id):

  • _title_model_for: both the title model and the session model.
  • _generate_session_title: the local/cloud switch for title request options.
  • _title_when_model_free: whether to wait for a local slot.
  • _maybe_update_session_title: whether to title at turn start.
  • _empty_answer_backend_problem: whether to check that the model is loaded locally.
  • The context-size check before a turn: it already checked the health payload's recipe, and now uses the same helper as the rest.

The PR also removes the cloud_model_provider imports those functions no longer need.

Test plan

Run in a python:3.12-slim container with .[ui] and the test tools installed:

  • pytest tests/unit/chat/ui/test_chat_helpers.py tests/unit/chat/ui/test_chat_title_on_send.py tests/unit/chat/ui/test_model_slot_discipline.py: 231 passed.
  • python util/lint.py --all: passes, with the existing mypy warning count.
  • pytest tests/unit: 34 failures, but 17 test workers crashed under load. The 20 test files with failures that main didn't have pass when rerun on their own (1,967 passed). Most of the other failures also fail on main in my environment.

With the source change reverted, both new tests fail:

FAILED test_chat_helpers.py::TestEmptyAnswerBackendProblem::test_a_discovered_cloud_model_skips_the_loaded_check
FAILED test_chat_title_on_send.py::test_a_local_guard_chat_is_not_titled_by_a_discovered_cloud_default
2 failed

Evidence

The two new tests cover the title-model privacy check and the empty-answer diagnosis, with a model known only from the catalog (opencode.big-model). Without the fix, the guard chat gets titled by that cloud model, and the diagnosis checks whether it's loaded locally. With the fix, both treat it as cloud. The other changed call sites use the same helper, and their existing tests pass.

Not tested against a real cloud provider or a running Agent UI.

  • Agent exposed in the Agent UI: N/A, no visible change apart from the title and diagnosis behavior above.
  • MCP tools / servers: N/A
  • CLI: N/A
  • HTTP API / REST: N/A

Checklist

  • I have linked a GitHub issue above (Closes #N / Fixes #N / Refs #N).
  • I have described why this change is being made, not just what changed.
  • I have run linting and tests locally (python util/lint.py --all, pytest tests/unit/).
  • I have attached real-world evidence matched to the surface I changed (see Evidence above), or marked each surface N/A.
  • I have updated documentation if user-visible behavior changed (see CONTRIBUTING.md). N/A: no documented behavior changes.

@github-actions github-actions Bot added the tests Test changes label Nov 7, 2026
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

Verdict: Approve with suggestions

The fix is correct. Chat titling, the slot-wait check, the empty-answer diagnosis and the pre-turn context check now recognize cloud models that GAIA only learns about at runtime (like opencode.*), so a local guard-model chat can no longer be sent to a cloud default just to get a title. Each swapped call site now classifies at least as many models as cloud as it did before. Both new tests fail without the fix, so they cover the bug.

  • Follow-up: the same gap still exists outside this file. The most important case is memory profile inference. It promises "this data stays local" but uses the old check, so a runtime-discovered cloud default model would get the user's memory data. Fix it here or file an issue before merge.

Real-world evidence

N/A: no evidence bundle was produced for this run. The change is internal model classification with no new CLI, API or UI surface. The verdict rests on static review and the author's unit-test results. I couldn't re-run the tests here because pytest isn't installed on this runner.

🔍 Technical details

Issues

🟡 Follow-up: "Route remaining UI cloud checks through the shared catalog". These callers outside the diff still call cloud_model_provider(...) with no metadata, so they only recognize the fireworks/amd prefixes. I found them by grepping cloud_model_provider( under src/gaia/ui/:

  • src/gaia/ui/routers/memory.py:1544: profile inference guards "this data stays local" with cloud_model_provider(model). A discovered cloud default passes this guard, which is the same privacy class as _title_model_for. This is the one that matters.
  • src/gaia/ui/routers/chat.py:962: "Retry with…" rejects only built-in cloud ids, so a discovered cloud model gets past the "local models only" 400.
  • src/gaia/ui/server.py:668: the warm-up skip misses discovered cloud models and waits on a local load lease for nothing.
  • src/gaia/ui/routers/system.py:629: default_model_is_cloud reports a discovered cloud default as local.

_ui_cloud_provider lives in _chat_helpers, so the routers can import it the same way other callers already do:

            if _ui_cloud_provider(model):

(This would go in memory.py, with from gaia.ui._chat_helpers import _ui_cloud_provider.)

Notes

  • _preflight_load changed cloud_model_provider(model_id, None) to _ui_cloud_provider(model_id). That's a strict superset: it now also catches catalog ids the /health payload didn't list as recipe == "cloud".
  • The new tests reset the shared catalog with record_cloud_models({"data": []}) instead of restoring the previous state. That matches how the other suites do it, so it's fine as long as no test relies on a catalog left over from an earlier test.
  • No doc drift: nothing documents which prefixes these checks recognize.

Strengths

  • The PR reuses the existing _ui_cloud_provider helper instead of adding a new classifier, so the file now uses one rule throughout.
  • Each new test is a regression check that fails without the fix, as shown in the PR body, and it targets the privacy-relevant path directly.
  • The PR also removes the imports that no longer have a caller.

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

Labels

tests Test changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants