Repository navigation
fix(agent): tell the user where their conversation is actually processed - #3706
Conversation
Request changesThe agent no longer claims to be local when it isn't — the system prompt now names the real destination and tells the agent to answer provider questions from that line instead of rummaging through config. The design is clean and the test coverage is genuinely good; three things stand between it and merge. The Go lint check is red on this branch. A test variable is assigned and never read, and The eval the project requires for a prompt change hasn't run. You flagged this yourself in the test plan — the flagship's system prompt is exactly the surface that gate exists for, and CI's eval job is skipping on this PR. A before/after pair on a single box is enough; it doesn't have to be the committed baseline's hardware. The local wording promises more than the code can check. On a local model the prompt tells the agent "nothing in this conversation goes to a cloud provider" — but the classifier only recognises two named cloud providers, and this same PR's own spec text says a local Lemonade URL doesn't by itself establish local inference. A user on a Lemonade instance routed to some other provider would get a confident, wrong privacy assurance from the one line meant to fix exactly that problem. Either scope the sentence to what GAIA actually knows, or have it ask the client that holds the catalog metadata. Two smaller things: the commit carries a Claude co-author trailer and the PR body a "Generated with Claude Code" footer, both of which the project's guidelines prohibit; and three of the fifteen prompt goldens were deliberately left stale — worth confirming those rows already fail on main so a future reader can tell the two drifts apart. Real-world evidenceNo automated evidence bundle was produced for this run, so nothing was exercised on my side; the verdict on the prompt's runtime behaviour rests on static review plus what the PR reports. The PR description does carry an author-run TUI session on Fireworks-via-Lemonade — asked "locally or a cloud provider? Name it", the agent named Fireworks AI via Lemonade, distinguished cloud chat from local tools and memory, and did it in 1 step, 0 tools, 3.9s with no filesystem inspection. That is the reported fix working on its surface, and it supports the change. I reproduced only the CI lint failure locally. 🔍 Technical detailsScope. Reviewed 🟡 Important1. Reproduced locally with the repo's own config ( (then line 49 becomes 2. Prompt change without an eval run
3. The local branch of
Two ways out, your call on which:
The remote branch has no equivalent problem — over-claiming remote is harmless; over-claiming local is the one that misleads. 🟢 Minor4. Three goldens left stale ( Rows 12–14 still carry the pre-change opening and no 5. Claude attribution (commit trailer + PR body)
Strengths
|
90b0e48 to
dee065c
Compare
Pick Fireworks in the TUI and the flagship still described itself as "running locally on the user's machine" — while the header two lines above it correctly said remote. Asked outright which provider was serving the session, it spent a turn reading configuration files and never answered. The opening line of the system prompt was a constant. It now names the real destination, resolved from the live client, and the prompt carries one authoritative line the agent answers provider questions from instead of going looking. Switching provider rebuilds the prompt, so the next answer moves with it. Two things that line is careful about: - It says what is SENT, not merely what is stored. Memory and documents live on the machine, but whatever the agent reads out of them to answer travels with the conversation. - On a local model it claims only GAIA's own routing — "GAIA is not sending this conversation to any cloud provider" — because Lemonade can itself be configured to route a plainly-named model upstream, which nothing here can see. Classification prefers the live client's own classifier, which reads the catalog metadata and so recognises a provider discovered at runtime; the id prefix is the fallback, and it is the rule the TUI's remote badge uses, so the two always agree. stdio's switch message is built from the same helper instead of its own copy of the provider-display map. The prompt characterization goldens are re-captured with the module's own harness — the drift is this one block, added, in 12 of 15 rows, tool sets untouched. Rows 12-14 are left alone: they fail identically on main in this environment, and re-capturing them here would bury that.
dee065c to
6c5025e
Compare
|
Four of the five findings are already fixed on the current head ( Go lint is green. The ineffectual assignment came in with #3672 and left with it. The local sentence no longer over-promises. It now says "GAIA is not sending this conversation to any cloud provider" — a claim about GAIA's own routing, which is all GAIA can actually verify — and classification prefers the live client's metadata-aware classifier, so a provider discovered at runtime is recognised. Both remedies you offered, not one. No Claude attribution remains on either the commit or the PR body. Rows 12–14 check out. They fail for a second, independent reason — a differing optional-dependency tool set, visible in the diff as Still outstanding: 🔍 Technical details1. ineffassign — 3. if not self.remote:
return (
f"Chat inference runs through Lemonade on this machine, model "
f"`{self.model}`. GAIA is not sending this conversation to any "
"cloud provider."
)and
4. Goldens — the commit touches 12 files (rows 01–11, 15); rows 12–14 are untouched. On this Windows box all 15 rows fail on a path-separator artifact ( 5. Attribution — |
|
Three of the fifteen golden fixtures didn't get regenerated, so the prompt-composition test now asserts a prompt this PR no longer produces — and nothing in CI will tell you, because no lane actually runs that test. The three that kept the old prompt: Please don't regenerate them on a dev box. A machine with the optional extras installed captures four tools CI doesn't have ( 🔍 Technical details
|
The prompt-composition goldens asserted a prompt this branch no longer produces: 12 of 15 rows picked up the new "WHERE THIS SESSION IS PROCESSED" block and rows 12/13/14 kept the old one. Nothing in CI catches it — no lane actually runs the test (amd#4121). Hand-applied rather than re-captured, as asked: a dev box with the optional extras registers four tools CI lacks (read_tool_output, run_python, transcribe_media, refine_transcript) and a different capability sentence with them, so a locally-captured golden would bake in a tool list wrong for CI. Only the one added block moves; the tools arrays are untouched. The inserted text and its position are byte-identical to the twelve rows the capture harness produced, verified by comparing the 40 characters either side of the block across all fifteen. Rows 12-14 still fail here, unchanged, on a separate pre-existing drift: this box has the audio/video extras, so the live prompt advertises transcribe_media while the committed golden says audio/video is unavailable. Confirmed byte-identical on plain main at this branch's merge-base — same three rows, same divergence, same offsets. That belongs to amd#4121, not here.
|
Picked up the one outstanding item and brought the branch current. The three stale goldens are fixed by hand, not re-captured — and the agent's provider answer is verified working against today's What changed here: rows 12/13/14 now carry the same Still outstanding, unchanged: 🔍 Technical detailsThe golden fixRows 12/13/14 asserted a prompt this branch no longer produces. Only the one added block moves; tools arrays untouched ( Correctness was checked by comparing the inserted text and its position across all fifteen rows — the 40 characters either side of the block are byte-identical everywhere, so the three hand-applied rows are indistinguishable from the twelve the capture harness produced: Rows 12–14 still fail, and it is not this branch
Verified against plain Worth knowing separately: at the current Zero-lane claim confirmed independently: no workflow mentions Runtime behaviour, on the merged tree against today's
|
|
CI finished on the updated branch: 24 pass, 14 skipped, 2 red — and neither red one is this change. Both were checked against plain
The second one is worth a separate look at some point: a lane that close to its ceiling will keep flapping. 🔍 Technical detailsWindows smoke — identical on
The one extra skip on this branch is the macOS smoke — timeout, not a test failure. Unit Tests (Linux, py3.12): success — that is the lane that actually exercises the changed code. Bot findings from the first review, current status:
Runner note (not a defect): several jobs sat queued ~30 minutes. They require |
itomek
left a comment
There was a problem hiding this comment.
Verified independently rather than on the description's word: the stale goldens are fixed and consistent across all fifteen rows, the provider line resolves correctly against today's main (Fireworks names Fireworks, user.-namespaced and dotted local ids stay local), and both red checks reproduce on plain main. The eval gap is real and stays flagged — it is genuinely unrunnable here, not skipped for convenience.
|
Two corrections to what I posted above, and one follow-up. The eval is runnable after all — my "unavailable" claim was wrong. I checked for the judge credential on the fleet boxes, which is the wrong place to look: the judge runs locally, and it works here. #4121 is closed, not open. I cited it as the open tracking issue for the zero-lane gap in both the commit message and my first comment. It was closed 2026-09-24 as completed and split: #4124 covers files no lane names, while the collected-then-skipped case — which is what this file actually hits — is explicitly still unguarded and has no issue of its own. Follow-up filed: #4365. Reviewing this PR turned up one path it missed — 🔍 Technical detailsEval feasibility, corrected. Lemonade on the fleet is bound to Worth recording for whoever runs it: the pre-merge branch also lacked What the correction does not change. The eval still has not run, so nothing here is presented as an eval result, and no regression claim is made either way. Worth noting there is no committed flagship baseline to diff against regardless — a run would report scores, not a verdict. |
|
The eval has now run — the gate this PR declared outstanding. No regression attributable to this change, and the headline numbers moved up. I ran it the way the first review suggested: a before/after pair on one machine, rather than a diff against a committed baseline (there isn't one for the flagship). Same box, same Lemonade, same category, one after the other. "Before" is
The comparison flagged two things and neither reproduced when re-run:
Two scenarios produced no measurement on both sides ( 🔍 Technical detailsSetup. Full diff ( Reproduction attempts on the two flags:
A 3.1-point spread across two runs of the same scenario on the same code is the measurement's own noise floor, which is worth knowing before anyone reads a single-pair diff as a verdict. What this does not establish. One before/after pair per scenario cannot separate a small real effect from that noise, and only Run ids: after |
…sage (amd#4365) 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 amd#3674 and amd#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 amd#3706. ## Test plan - [x] `pytest hub/agents/gaia/python/tests/test_stdio.py` — **79 passed** - [x] `pytest tests/unit/test_inference_location.py` — **27 passed** - [x] `python util/lint.py --black --isort --flake8` — 3/3 passed - [x] 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 ``` <details> <summary>🔍 Technical details</summary> ### 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. </details> --------- Co-authored-by: Kalin Ovtcharov <kalin.ovtcharov@amd.com>
Pick Fireworks in the TUI and the flagship still describes itself as "running locally on the user's machine" — while the header two lines above it correctly says remote. Asked outright which provider is serving the session, it spends a turn reading configuration files and never answers. Users get two contradictory accounts of where their chat is going, and the wrong one comes from the agent itself.
Fixes #3674
Test plan
pytest tests/unit/test_inference_location.py(27 passed) — local,user.-namespaced and dotted-version local ids, Fireworks, AMD Gateway, Claude, OpenAI; a runtime-discovered provider; a broken lookup; the prompt for each; the switch; and composition before the client exists.pytest tests/unit/test_chat_system_prompt_budget.py hub/agents/gaia/python/tests/test_stdio.py tests/unit/test_lemonade_cloud_models.py(140 passed)python util/lint.py --pylint, black, flake8, isortgaia eval agent— not run. The prompt is an eval-required surface per CLAUDE.md. It needsANTHROPIC_API_KEYexported (the judge reads it directly and cannot ride a Claude Code subscription from its subprocess) and a box with no TUI driving the single Lemonade model slot. Neither held here. The reviewer's suggestion — a before/after pair on one machine rather than a diff against the committed Gemma baseline — is the right shape for it; flagging as outstanding rather than claiming it.🔍 Technical details
The opening line of
ChatAgent._get_system_promptwas a constant.gaia.llm.inference_locationresolves the real destination from the live client (effective_model+use_claude/use_chatgpt), and the prompt carries one authoritative line the agent answers provider questions from, with an instruction never to go looking. Every model switch already callsrebuild_system_prompt()inside_apply_switch's rollback guard, so the next answer moves with it._inference_locationreadsself.configbeforeAgentSDKexists — MCP registration composes the prompt during__init__— which also carriesuse_chatgptandclaude_model, neither of which is on the agent itself at that point.stdio.run_model_commandnow builds its switch message from the same helper instead of its own copy of the provider-display map.Review follow-ups
SPEC.mdsays a local Lemonade URL does not establish local inference. It now claims only GAIA's own routing — "GAIA is not sending this conversation to any cloud provider".LemonadeProvider.cloud_model_providerreads the catalog metadata, so a provider discovered at runtime is recognised; the id-prefix rule is the documented fallback, and it is what the TUI's own remote badge uses, so the two always agree. A lookup that answers with anything but a provider name is logged and ignored rather than put into the sentence the user reads.mainin this environment (they turn on optional deps, not on this change). The other 12 are re-captured with the module's own harness, guarded to refuse anything but this one added block; tool sets untouched.ineffassignthe review reproduced was fixed in feat(tui): choose local, Fireworks, or AMD Gateway via Lemonade #3672 before it merged.