Skip to content

fix(agent): tell the user where their conversation is actually processed - #3706

Merged
itomek merged 3 commits into
amd:mainfrom
kovtcharov:fix/inference-destination
Sep 25, 2026
Merged

itomek merged 3 commits into
amd:mainfrom
kovtcharov:fix/inference-destination

Conversation

@kovtcharov

@kovtcharov kovtcharov commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor

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.

#3672 has merged, so this is no longer stacked — it is one commit against main.

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, isort
  • Live TUI on Fireworks-via-Lemonade: "locally or a cloud provider? Name it" → names Fireworks AI via Lemonade, distinguishes cloud chat from local tools and memory, 1 step, 0 tools, 3.9s. No filesystem inspection.
  • gaia eval agent — not run. The prompt is an eval-required surface per CLAUDE.md. It needs ANTHROPIC_API_KEY exported (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_prompt was a constant. gaia.llm.inference_location resolves 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 calls rebuild_system_prompt() inside _apply_switch's rollback guard, so the next answer moves with it.

_inference_location reads self.config before AgentSDK exists — MCP registration composes the prompt during __init__ — which also carries use_chatgpt and claude_model, neither of which is on the agent itself at that point.

stdio.run_model_command now builds its switch message from the same helper instead of its own copy of the provider-display map.

Review follow-ups

  • The local sentence no longer over-promises. It said "nothing in this conversation goes to a cloud provider"; the classifier only knows two provider prefixes, and this repo's own SPEC.md says 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".
  • Classification prefers the live client. LemonadeProvider.cloud_model_provider reads 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.
  • Rows 12–14 of the prompt goldens are deliberately untouched and, as asked, confirmed to fail identically on plain main in 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.
  • The Go ineffassign the review reproduced was fixed in feat(tui): choose local, Fireworks, or AMD Gateway via Lemonade #3672 before it merged.

@github-actions github-actions Bot added documentation Documentation changes llm LLM backend changes audio Audio (ASR/TTS) changes tests Test changes performance Performance-critical changes agents tui Go terminal UI (gaia-tui) labels Sep 12, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Request changes

The 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 golangci-lint fails the build on it. One-line fix; it comes in with the stacked branch, but it is failing here.

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 evidence

No 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 details

Scope. Reviewed abf74ce8, this branch's own commit (inference_location.py, ChatAgent._get_system_prompt, stdio.run_model_command, goldens, tests). The Go TUI, lemonade_client.py cloud plumbing, and resolve_lemonade_api_key(base_url=…) threading arrive from the stacked #3672 and belong to that review — except its lint failure, which is red on this branch.

🟡 Important

1. golangci-lint fails: ineffectual assignment (tui/internal/ui/providers/model_test.go:45)

Reproduced locally with the repo's own config (golangci-lint v2.12.2): ineffectual assignment to cmd (ineffassign). cmd is overwritten at line 49 before use.

	next, _ := m.Update(modelsMsg{models: []lemonade.Model{{ID: "fireworks.z"}, {ID: lemonade.FireworksModel}}})

(then line 49 becomes _, cmd := m.Update(tea.KeyMsg{Type: tea.KeyEnter}).)

2. Prompt change without an eval run

CLAUDE.md lists _get_system_prompt() as eval-required, and the rag_quality + context_retention + tool_selection vs Gemma baselines job reports skipping on this PR. The hardware-mismatch argument against diffing the committed Gemma baseline is fair, but it doesn't rule out the cheaper form: gaia eval agent --category tool_selection (and context_retention) on this machine, once on the base commit and once on the head, comparing the two scorecards to each other rather than to the committed baseline. That still catches "the new block displaced attention from the tool rules", which is the regression class this insertion risks — it lands four lines into the prompt, above WHO YOU ARE.

3. The local branch of describe() asserts more than the classifier verifies (src/gaia/llm/inference_location.py:60-65)

resolve_inference_location calls the module-level cloud_model_provider(model_id) with no metadata, so it only recognises fireworks.* and amd.* (lemonade_client.py:237-256); a catalog-discovered provider outside that pair, or a Lemonade instance configured to route a plainly-named model upstream, resolves to lemonade and the prompt then states "nothing in this conversation goes to a cloud provider." This PR's own hub/agents/gaia/npm/SPEC.md says the opposite: "Lemonade may independently be configured to route a model remotely, so a local server URL alone does not establish local inference."

Two ways out, your call on which:

  • Scope the claim — e.g. "GAIA is not sending this conversation to a cloud provider" instead of the absolute "nothing … goes to", which is the only part that can be false.
  • Give the resolver the metadata-aware classifier. LemonadeClient.cloud_model_provider() (lemonade_client.py:2631) already consults _model_metadata; it isn't reachable from ChatAgent._inference_location today because chat.llm_client is a LemonadeProvider wrapping _backend privately, so this needs a small passthrough rather than a one-liner.

The remote branch has no equivalent problem — over-claiming remote is harmless; over-claiming local is the one that misleads.

🟢 Minor

4. Three goldens left stale (tests/fixtures/profilespec_characterization/row12-14)

Rows 12–14 still carry the pre-change opening and no WHERE THIS SESSION IS PROCESSED block, while the shared base_prompt now emits it for every profile — so those rows now differ for two reasons at once, which is precisely what a byte-for-byte contract exists to prevent. The module's docstring warns that a corrupted golden corrupts the whole contract. I couldn't run the suite here (no pytest in this environment) to confirm they already fail on main; if they do, an issue link in the commit body makes the "left alone deliberately" call auditable for the next person.

5. Claude attribution (commit trailer + PR body)

CLAUDE.md → "No Claude Attribution of Any Kind" prohibits both Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> on abf74ce8 and the 🤖 Generated with [Claude Code] footer on the PR description.

Strengths

  • The resolver is the right shape. One pure function over (model, use_claude, use_openai), no I/O, so the prompt, the stdio switch message, and the status event can't disagree — and run_model_command dropping its private provider-display map for it is exactly the duplication this should remove.
  • The test file tests the behaviour, not the implementation. local_user_ns and local_dotted_version pin the two ways a local id can look cloud-shaped; test_switching_provider_changes_the_next_prompt pins the thing the bug report was actually about; and the pre-client cases cover the MCP-registration ordering that makes _inference_location's fallback branch real rather than defensive.
  • The "what is sent vs. what is stored" distinction is the correct call. Saying memory and documents stay local, without noting that whatever the agent reads out of them travels with the turn, would have replaced one misleading claim with a subtler one.

@kovtcharov
kovtcharov force-pushed the fix/inference-destination branch 2 times, most recently from 90b0e48 to dee065c Compare September 12, 2026 03:04
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.
@kovtcharov
kovtcharov force-pushed the fix/inference-destination branch from dee065c to 6c5025e Compare September 12, 2026 19:32
@kovtcharov-amd

Copy link
Copy Markdown
Collaborator

Four of the five findings are already fixed on the current head (6c5025e17); the review was written against abf74ce8, before the rebase. I re-verified each one rather than taking the PR body's word for it. The eval is the one thing still outstanding, and I can't run it here.

Go lint is green. The ineffectual assignment came in with #3672 and left with it. ineffassign over the whole tui/ module reports nothing, and go vet ./... is clean.

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 analyze_image and friends, not just the one added block. Re-capturing them would have baked one box's dep set into the contract. The commit body records the call.

Still outstanding: gaia eval agent. The prompt is an eval-required surface and I am not running it. Lemonade is down on this machine and the CI judge is spend-capped, so any number I produced would be fabricated rather than measured. Your suggestion — a before/after pair on one box rather than a diff against the committed Gemma baseline — is the right shape; it needs a machine with a free Lemonade model slot. This PR should not merge on my say-so; it needs that run.

🔍 Technical details

1. ineffassign — tui/internal/ui/providers/model_test.go:45 is already next, _ := m.Update(...) with line 50 _, cmd := m.Update(...), exactly the suggested shape.

$ go run github.com/gordonklaus/ineffassign@latest ./...   # in tui/
(no output, exit 0)
$ go vet ./...                                              # exit 0
$ go test ./internal/ui/providers/                          # ok, 0.969s

3. describe() local branch — src/gaia/llm/inference_location.py:

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 resolve_inference_location(..., cloud_provider_lookup=...) routes through _ask(), which prefers the injected classifier and falls back to the module-level cloud_model_provider prefix rule — logging and discarding a lookup that answers with a non-string rather than putting it in the user-visible sentence.

pytest tests/unit/test_inference_location.py → 27 passed.

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 (/fake/home vs \fake\home) that is unrelated to this PR, so I compared failure reasons instead: rows 01–11 differ only by that separator, row 12 additionally differs in its tool list. That is the independent pre-existing cause the commit body describes.

5. Attribution — git log -1 --format=%B 6c5025e17 has no Co-Authored-By trailer; the PR body has no Generated with Claude Code footer.

@kovtcharov-amd

Copy link
Copy Markdown
Collaborator

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: row12_full_indexed_n1, row13_full_no_docs_enable_sd_tools, row14_data_no_docs_baseline. The other twelve picked up the new block.

Please don't regenerate them on a dev box. A machine with the optional extras installed captures four tools CI doesn't have (read_tool_output, run_python, transcribe_media, refine_transcript) and a different capability sentence along with them, so a locally-captured golden bakes in a tool list that is wrong for CI — worse than the drift it fixes. Either regenerate in an environment matching the lane, or hand-apply only the changed block to those three.

🔍 Technical details
  • git diff origin/main...<head> --name-only -- tests/fixtures/profilespec_characterization/ lists 12 of 15 fixtures — rows 01–11 and 15.
  • Grepping the merged goldens for WHERE THIS SESSION IS PROCESSED finds it in those 12 and misses it in rows 12/13/14, while capturing live on the merged tree emits the block for all three. First divergence at offset 305.
  • The test is tests/unit/test_profilespec_characterization.py, gated by a module-level pytest.importorskip("gaia_agent_chat"). test_unit.yml collects it and skips it (no hub wheel); test_chat_agent.yml installs the wheel but runs an explicit file list that doesn't name it. That zero-lane gap is now ci: nothing verifies that a test file is actually run by any CI lane #4121 — it covers 15 files, not just this one.
  • Second local-run caveat: the goldens embed POSIX /fake/home, so on Windows every row also "fails" on path separators alone. Normalise before believing a diff.

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.
@itomek

itomek commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

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 main, not the base this branch was cut from.

What changed here: rows 12/13/14 now carry the same WHERE THIS SESSION IS PROCESSED block as the other twelve. Hand-applied exactly as asked, so none of the optional-extras tooling leaks into the fixtures. The branch was also 234 commits behind and has been updated from main, which matters more than it sounds: the previous green run was against a base from 12 Sept, and main has since retired the OpenAI/LiteLLM providers and added a dedicated Fireworks one — the exact surface this change classifies against.

Still outstanding, unchanged: gaia eval agent has not run. That was flagged honestly in the description and it stays flagged — it is not claimed below.

🔍 Technical details

The golden fix

Rows 12/13/14 asserted a prompt this branch no longer produces. Only the one added block moves; tools arrays untouched (3 files changed, 3 insertions(+), 3 deletions(-)).

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:

15 rows: row01..row15
  BEFORE: ' `lspci | grep VGA`, Memory: `free -h`\n\n'
  AFTER : '\n\n\n**WHO YOU ARE:**\n- You\'re GAIA. Not "'

Rows 12–14 still fail, and it is not this branch

pytest tests/unit/test_profilespec_characterization.py → 3 failed, 14 passed. The divergence moved from the block (offset 305) to a capability sentence much later in the prompt — this box has the audio/video extras, so the live prompt advertises transcribe_media while the committed golden says audio/video is unavailable.

Verified against plain main in a separate clean worktree (imports proven to resolve inside it, no PYTHONPATH): at this branch's merge-base, main gives the same 3 failed, 14 passed, same three rows, byte-identical divergence string, uniform +113-char delta, offsets row12@10451 / row13@6874 / row14@4500. So the claim that they fail identically on main holds up.

Worth knowing separately: at the current main tip that same test is 11 failed, 6 passed (rows 05–15). The goldens are stale well beyond this PR — read_tool_output and run_python have since spread to more profiles and no golden anywhere contains them. That is #4121's real scope.

Zero-lane claim confirmed independently: no workflow mentions profilespec; test_unit.yml runs pytest tests/unit/ wholesale but never installs gaia_agent_chat, so the module-level importorskip skips it; test_chat_agent.yml runs hub/agents/chat/python/tests/, a different directory.

Runtime behaviour, on the merged tree against today's main

model='Gemma-4-E4B-it-GGUF'
  remote=False provider='lemonade'
  Chat inference runs through Lemonade on this machine, model `Gemma-4-E4B-it-GGUF`.
  GAIA is not sending this conversation to any cloud provider.

model='fireworks.accounts/fireworks/models/kimi-k2'
  remote=True provider='fireworks'
  Chat inference runs on Fireworks AI via Lemonade — a cloud provider — using model
  `fireworks.accounts/...`. Everything in this conversation is sent there to be answered,
  including whatever you read out of files, documents or memory. Those are stored and
  searched on this machine.

model='amd.Llama-3.3-70B'          remote=True  provider='amd'
model='user.embeddinggemma-300m-GGUF'  remote=False provider='lemonade'   # `user.` not mistaken for cloud
model='Qwen3-VL-4B-Instruct-GGUF'      remote=False provider='lemonade'
model='claude-opus-5' (use_claude)     remote=True  provider='claude'

That is #3674's reported symptom resolved, and it still resolves after main's provider redesign — cloud_model_provider's second parameter is optional and it already recognises {fireworks, amd}, so the one-arg call in inference_location.py is unaffected.

pytest tests/unit/test_inference_location.py tests/unit/test_chat_system_prompt_budget.py → 34 passed, run against the merged tree (post-main-merge), not just the old base.

The "switch rebuilds the prompt" claim checks out: rebuild_system_prompt() is called inside _apply_switch's rollback guard (hub/agents/gaia/python/gaia_agent/stdio.py:531), so a prompt-composition failure rolls the client swap back rather than leaving a half-composed prompt.

Why the eval still has not run

Checked three places rather than assuming:

  • Locally — no Lemonade at all: no lemonade/lemond/llama-server/flm process, and no plausible listener in ss -ltn.
  • Fleet — t-nx-strx-halo and t-nx-radeon answer SSH, but neither exposes an ANTHROPIC_API_KEY to a non-interactive session; the judge credential was never provisioned there. (Caveat: a login-shell profile could still hold one — this is a non-interactive check.) t-nx-strx-pnt does not resolve.
  • CI — every eval lane on this run reports skipping.

So the number is unavailable rather than unfavourable. Nothing above is presented as an eval result.

@itomek

itomek commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

CI finished on the updated branch: 24 pass, 14 skipped, 2 red — and neither red one is this change. Both were checked against plain main rather than assumed.

  • Unit Tests (Windows smoke) fails on main too, on the same single test, with the same counts. Unrelated to this PR.
  • Unit Tests (macOS smoke) hit the job timeout. That lane takes 24m47s on main against a 30-minute budget, so it is already running at 83% of its allowance — a slower runner tips it over regardless of what is in the PR.

The second one is worth a separate look at some point: a lane that close to its ceiling will keep flapping.

🔍 Technical details

Windows smoke — identical on main.

this PR (job 108121156753) main @ 641e40b6 (job 107993971658)
result 1 failed, 2302 passed, 115 skipped 1 failed, 2302 passed, 114 skipped
test tests/unit/chat/ui/test_toctou.py::TestSafeOpenDocument::test_safe_open_rejects_symlinked_directory_escape same
reason Failed: DID NOT RAISE HTTPException same

The one extra skip on this branch is the profilespec module skipping, consistent with it having no lane.

macOS smoke — timeout, not a test failure. conclusion: cancelled, and the step list shows every setup step green with only Run unit tests (macOS smoke) cancelled at the 30-minute mark (timeout-minutes: 30, test_unit.yml:252). Re-run once on its own to rule out matrix fail-fast; it timed out the second time as well. main's own run of the same job: 08:04:52 → 08:29:39.

Unit Tests (Linux, py3.12): success — that is the lane that actually exercises the changed code.

Bot findings from the first review, current status:

# Finding Status
1 Go ineffassign lint red Fixed — arrived and left with #3672
2 gaia eval agent not run Still outstanding, not claimed
3 Local wording over-promises Fixed — now scoped to GAIA's own routing, and classification prefers the live client
4 Claude attribution on commit/body Removed
5 Rows 12–14 goldens stale Fixed in e9d5240b

Runner note (not a defect): several jobs sat queued ~30 minutes. They require devlab-dispatch + strix-halo, which only the 8 ephemeral runners carry, and all 8 were busy; the 5 idle stx runners cannot take them. A peer PR was queued in the same lane at the same time, so it was pool-wide saturation rather than anything to do with this branch.

@itomek itomek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@itomek
itomek added this pull request to the merge queue Sep 25, 2026
Merged via the queue into amd:main with commit 69f76ab Sep 25, 2026
39 of 43 checks passed
@itomek

itomek commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

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. src/gaia/eval/claude.py reads only ANTHROPIC_API_KEY, but the Anthropic SDK picks up ANTHROPIC_BASE_URL and ANTHROPIC_CUSTOM_HEADERS from the environment on its own, and a live call through the AMD gateway returned 200. Lemonade is serving on three fleet boxes; it is bound to localhost there, which is why my port scan found nothing — an SSH tunnel reaches it. So the eval was a tunnel away the whole time, not blocked. It still has not been run.

#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 — /model could switch you to a Fireworks-backed model and then tell you "GAIA is not sending this conversation to any cloud provider." Same contradiction, opposite direction. One-line fix with a regression test.

🔍 Technical details

Eval feasibility, corrected. claude.py:68 reads ANTHROPIC_API_KEY only and passes no base_url/default_headers — but anthropic 1.7.0's __init__ reads ANTHROPIC_BASE_URL and ANTHROPIC_CUSTOM_HEADERS from env itself. Verified by instantiating the real client and issuing a call, not by reading code:

base_url    = https://llm-api.amd.com/Anthropic/
custom hdrs = ['Ocp-Apim-Subscription-Key']
POST https://llm-api.amd.com/Anthropic/v1/messages  HTTP/1.1 200 OK
first block type = thinking

Lemonade on the fleet is bound to 127.0.0.1:13305, hence invisible to a LAN port scan. t-nx-strx-halo, t-nx-radeon and t-nx-acer-swft all serve Gemma-4-E4B-it-GGUF at ctx 65536 with an idle slot (t-nx-strx-pnt is unreachable — it defaults to Windows). None of them can run the judge — llm-api.amd.com is unroutable from the home LAN, HTTP 000 — so the split is: judge local, Lemonade tunnelled.

Worth recording for whoever runs it: the pre-merge branch also lacked 995bafb2 (#3885, "read past thinking blocks in the Claude judge response"), and the probe above shows the gateway returns a thinking block at index 0 — so every judge call on that branch would have died on content[0].text. Merging main cleared that; an eval attempted on the old head would have failed for a reason unrelated to this change.

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.

@itomek

itomek commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

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 main at the commit immediately preceding this merge.

before (9d2d4c3f) after (69f76ab5)
Pass rate (all) 58% 75%
Pass rate (judged) 78% 90%
Avg score 8.8 8.9

The comparison flagged two things and neither reproduced when re-run:

  • skill_summarize_styles looked like a 3.1-point drop. Re-running it scored 7.5 instead of 5.4 — and it fails on the before side too. The scenario is just noisy; it was already failing.
  • budget_query looked 3.7× slower (53s → 197s). Re-running it took 51s. It was the first scenario of the run, so the 197s was warm-up.

Two scenarios produced no measurement on both sides (skill_document_brief_gaps, skill_document_brief_standing) — pre-existing, unrelated to this change, and excluded from the verdict rather than counted as passes.

🔍 Technical details

Setup. t-nx-radeon, leased for the run and released after. Lemonade 2026.39.1 serving Gemma-4-E4B-it-GGUF at ctx 65536 on the Vulkan backend (not ROCm — ROCm degenerates this GGUF). Reached over an SSH tunnel, since Lemonade binds 127.0.0.1:13305 there. Judge claude-opus-5 via the AMD gateway. --agent-type left at its default, so both runs scored the flagship gaia agent; runs were strictly serial against the one Lemonade slot. Each backend was confirmed pinned to its own worktree by reading /proc/<pid>/environ and cwd, because the editable install resolves gaia to a different checkout.

Full diff (gaia eval agent --compare <before> <after>):

METRIC                           BASELINE    CURRENT      DELTA
Pass rate (all)                       58%        75%       +17%
Pass rate (judged)                    78%        90%       +12%
Avg score                             8.8        8.9       +0.1

[+] IMPROVED (2) — FAIL → PASS:
    negation_handling                        4.7 → 6.2 (+1.5)
    skill_summarize_action_items             8.4 → 9.7 (+1.3)
[~] SCORE REGRESSION (1):
    skill_summarize_styles                   8.5 → 5.4 (-3.1)
[~] TIME REGRESSION (1):
    budget_query                             53.2s → 197.4s
[=] UNCHANGED (6): cross_section_rag, csv_analysis, hallucination_resistance,
    safety_handbook_water, simple_factual_rag, table_extraction
[?] NOT MEASURED (2): skill_document_brief_gaps, skill_document_brief_standing

Reproduction attempts on the two flags:

Scenario before after (run 1) after (run 2)
skill_summarize_styles FAIL 8.5 FAIL 5.4 FAIL 7.5
budget_query PASS 9.8, 53.2s PASS 9.8, 197.4s PASS 9.8, 51.0s

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 rag_quality was run — not context_retention or tool_selection. The claim is bounded to: nothing in this category regressed in a way that reproduces.

Run ids: after eval-20260925-163008, before eval-20260925-165249, re-runs eval-20260925-171300 and eval-20260925-171507.

pull Bot pushed a commit to bhardwajRahul/gaia that referenced this pull request Sep 29, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents audio Audio (ASR/TTS) changes documentation Documentation changes llm LLM backend changes performance Performance-critical changes tests Test changes tui Go terminal UI (gaia-tui)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(agent): cloud sessions incorrectly describe inference as local

3 participants