Skip to content

perf(gaia): send the flagship only the tools a turn needs - #3008

Open
kovtcharov wants to merge 1 commit into
amd:mainfrom
kovtcharov:claudia/task-49822449
Open

perf(gaia): send the flagship only the tools a turn needs#3008
kovtcharov wants to merge 1 commit into
amd:mainfrom
kovtcharov:claudia/task-49822449

Conversation

@kovtcharov

Copy link
Copy Markdown
Contributor

The flagship agent shipped all 66 of its tool schemas to the model on every LLM call. A conversational turn is a ReAct loop of 2-5 calls, so a 4B model re-read ~12,200 tokens of tool definitions several times over before it could start on the user's question. It now sends a small always-on set plus whatever the turn actually matches, cutting the per-call tool prompt from 12,164 to 4,654 tokens (62%). Nothing becomes unreachable: if selection misses, the model sees a bundle menu and calls load_tools to pull the capability in mid-turn.

The loader itself already existed (#1449/#1450) but was gated to the doc profile and default-off. What was missing was bundle coverage for the flagship's 66-tool registry — DOC_BUNDLES covered 38 of them, so flipping the gate as-is would have left 29 tools unable to arrive with their cohort or be recovered through the escape hatch.

Test plan

  • python -m pytest hub/agents/gaia/python/tests/test_full_tool_bundles.py tests/unit/test_chat_tool_bundles.py -v — bundle coverage is exact in both directions, the menu renders, the cap fits two whole bundles, and the token saving is pinned
  • python -m pytest hub/agents/gaia/python/tests/ hub/agents/chat/python/tests/ tests/unit/test_tool_loader_disambiguation.py -q — 230 pass
  • Confirm gaia-agent.yaml tools_count: 67 matches the real registry (drift-guarded by test_gaia_agent.py)
  • Outstanding, not run: gaia eval agent against the committed baseline. Tool schemas are an LLM-affecting surface, so this is a required merge gate per CLAUDE.md. It needs Lemonade + Gemma-4-E4B, which were out of service on the dev box for the whole of this change.
🔍 Technical details

Measurements. tiktoken cl100k_base over the real renderers (_build_openai_tool_schemas + _format_tools_for_prompt), no model involved. Baseline 10,464 native + 1,700 text = 12,164. Mean over six queries (greeting, GitHub issues, summarize a PDF, browse Documents, CSV analysis, code search): 3,988 + 666 = 4,654. A greeting costs 2,526 — CORE only. Selections came from the loader's own select() against the live nomic embedder before Lemonade was taken down.

No seconds are claimed. The token-to-latency conversion needs a Gemma-4-E4B prefill run and has not been done. The sibling analysis measured ~387 tok/s prefill; applying it here would be arithmetic, not measurement.

End-to-end validation ran on Claude Haiku 4.5 (30/30) — valid evidence for logic, not for local-model latency or eval scores. Six scenarios: trimmed tools= really reaches the model (5 sent of 62); Haiku recovers a deliberately-missed tool via load_tools("file_browse") then calls browse_directory in the same turn; embedder-down falls back to the full registry and still answers; the cap holds and repeat turns serialise byte-identically so the KV prefix stays warm; unknown bundle names fail with the valid names listed; dynamic_tools=False restores the previous prompt exactly.

optional_tools on ToolLoader. validate_registry treated any configured-but-absent tool as drift and raised. Three legitimate absences break that: search_documentation (needs npx), the skill-library and code-index tools (GaiaAgent-only, so a plain ChatAgent on prompt_profile="full" has none), and the memory tools (MemoryMixin registers nothing when the embedder is unreachable). The last one was found by the Haiku run — a degraded-but-running agent hard-failed on its first turn. Typos still fail loudly, and the CI guard checks the other direction against a registry where all of these are present.

Cap = 26 for the flagship (10 CORE + 16 slots), swept offline across nine queries at tau in {0.18, 0.20, 0.22, 0.25} by cap in {18, 22, 26, 30}. tau barely moves the outcome because the cap binds first; 22 truncated the web bundle mid-pull on a research question, 26 lands every matched bundle whole, 30 adds cost for nothing. tau stays at the calibrated 0.20. analyze_data_file merged into the data bundle with the scratchpad tools — as a singleton it scored below 16 other tools on a CSV question and never won a slot, despite being the tool the prompt names for that case.

Composition with the duplicate-tool-list work. The base agent passes the same filter to both surfaces, so trimming shrinks the native schemas and the AVAILABLE TOOLS text block, and already moves that block to the prompt tail when a filter is active. Gating the text block off entirely is independent and additive.

Known issue, not introduced here and not fixed here. @tool writes into a process-global registry and _snapshot_tools copies all of it, so an agent built second inherits the first agent's tools — including a load_tools bound to the first agent's loader. Pre-existing; defaulting the flagship on makes it reachable. The sidecar builds one agent per process, so the shipping path is unaffected. The fix belongs in per-agent registration in src/gaia/agents/base/agent.py, which this PR deliberately does not touch.

Also fixed: build_doc_agent_skeleton re-snapshotted the global registry after _register_tools, undoing the exclusion of seven code-writing tools that generic_file_ops profiles perform. Harmless for doc (which does not set that flag), but it overstated the full skeleton by 7 and would have poisoned the measurement.

tests/unit/test_tool_loader_token_budget.py::test_harness_runs_and_pins_baseline fails on this branch and fails identically on clean main (19,533 vs 21,957 chars) — pre-existing, untouched.

@github-actions github-actions Bot added eval Evaluation framework changes tests Test changes performance Performance-critical changes agents labels Aug 19, 2026
@kovtcharov

Copy link
Copy Markdown
Contributor Author

Heads-up from #3010, which lands proactive skill discovery: when the two of us merge, a skill the agent offers to the user may become one it can no longer load in that turn.

#3010 matches each turn against installed skills and, when several are plausible but none dominant, hands the model a shortlist and tells it to call load_skill. Here load_skill lives in the skills bundle rather than CORE, and the bundle is selected from the user's words — "watch this page for me" scores nothing against "List, load, and unload the skills installed on this machine." So the model is told to call a tool that is not in tools= that turn, and has to work out load_tools("skills") on its own first.

Cheapest fix is moving load_skill alone into FULL_CORE — it is the escape hatch for the whole skill system, so it is arguably CORE on its own merits. Your call, and no rush: neither PR is merged, and #3010 does not touch tool_bundles.py.

🔍 Technical details
  • FULL_BUNDLESToolBundle(name="skills", members={list_skills, load_skill, unload_skill, skill_status}) in hub/agents/chat/python/gaia_agent_chat/tool_bundles.py.
  • feat(skills): load the right skill without the user knowing its name #3010's shortlist note names load_skill directly and never mentions load_tools, so recovery depends on the model connecting the bundle menu to a tool it was just instructed to call.
  • Auto-load is unaffected — feat(skills): load the right skill without the user knowing its name #3010 calls Agent.load_skill in Python before the request is built, so it never goes through the tool surface. Only the shortlist path is exposed.
  • Moving just load_skill costs one schema in CORE and leaves install_skill / remove_skill / search_skill_hub where they are.

@github-actions

Copy link
Copy Markdown
Contributor

Verdict: Request changes — two 🟡 items before merge

This PR extends dynamic tool loading from the doc profile to the flagship full profile (GaiaAgent). The architecture is clean: PROFILE_TOOL_CONFIGS is a natural extension of the existing pattern, the optional_tools exemption in validate_registry is implemented correctly, and the drift-guard tests are thorough. Two items need attention before this merges.


🟡 Eval results required — system prompt and tool registration both changed for GaiaAgent.
Adding "load_tools_menu" to the full profile's prompt_blocks and setting dynamic_tools: bool = True by default in GaiaAgentConfig are LLM-affecting changes (system prompt content + which tools reach the model per turn). Per the project rule, these require a gaia eval agent run against the relevant category and a comparison to the committed baseline before merge. Green unit tests don't substitute — they gate code paths, not LLM behaviour. If the PR description already links eval results, this is resolved; if not, run the eval and confirm no regression before merging.

🟡 CHANGELOG entry missing for the GaiaAgent npm package.
hub/agents/gaia/npm/CHANGELOG.md has an open [0.1.1] — unreleased section. Per the project's doc-consistency rule, a functional change to a hub agent must be named in the CHANGELOG of every package that ships it. "Per-turn skill-body selection" was documented there when that analogous feature landed; per-turn semantic tool selection for the flagship deserves a comparable entry in the same [0.1.1] block.

🔍 Technical details

Eval triggers (CLAUDE.md list):

  • profiles.py"load_tools_menu" added to full profile prompt_blocks; this flows directly into _get_system_prompt() for every GaiaAgent turn.
  • GaiaAgentConfig.dynamic_tools = True — changes the tool payload sent to Lemonade on every turn for the flagship.
  • Both are named in the CLAUDE.md list of changes that require an eval run.

CHANGELOG gap:
hub/agents/gaia/npm/CHANGELOG.md:7 has an open unreleased section. The correct place to add the entry is in the existing ### Added block under [0.1.1], mirroring the "Per-turn skill-body selection" bullet already there.

Code correctness (no issues found):

  • validate_registry fix (set(registry) | self._optional) correctly exempts environment-conditional and subclass-only tools while still failing loudly on typos — sound.
  • if agent._instance_tools is None: guard in both skeleton builders correctly preserves the profile-specific tool stripping that _register_tools does for generic_file_ops profiles — sound.
  • flagship_registry fixture in test_full_tool_bundles.py uses _isolated_registry(), sets _memory_store = object() before the second _register_tools() call, and correctly forces a full-featured registry despite CI having no embedder — matches the pattern already used in test_gaia_agent.py.
  • Memory tools in both FULL_CORE_TOOLS and the memory bundle is intentional: CORE ensures they render always (when present); the bundle gives load_tools("memory") a resolution target. select deduplicates, so no double-counting.
  • tools_count 66 → 67: the increment is load_tools, which now registers because dynamic_tools defaults True; arithmetic checks out (55 + 7 + 4 + 1 = 67).

itomek
itomek previously approved these changes Aug 24, 2026
@itomek itomek assigned itomek and unassigned itomek Aug 24, 2026
pull Bot pushed a commit to bhardwajRahul/gaia that referenced this pull request Aug 24, 2026
…md#3026)

> **Stacked branch.** Cut from the shared `integration/full-tui` base,
so the diff against `main` also shows 17 commits already covered by
amd#3022-amd#3024. This change is 7 files: `src/gaia/llm/providers/claude.py`,
`src/gaia/agents/base/turn_metrics.py`,
`tui/internal/event/canonical.go`, `tui/internal/ui/chat/turnmetrics.go`
(+ test), and the two unit-test files. It depends on amd#3024 for the
turn-metrics plumbing.

Every LLM call the Claude backend made re-sent the same ~13,700 tokens
of system prompt and tool schemas at full price — Anthropic prompt
caching is opt-in, and nothing in the provider ever asked for it. A
ReAct turn is 2-5 calls, so the largest single cost in the product was
being paid over and over for bytes that never changed. This turns it on,
and fixes the usage parsing that would otherwise have reported the win
as zero: the provider read only `input_tokens` and `output_tokens`,
never the cache counters, so the `--dev` metrics line printed `0 cached`
whether caching worked or not.

Measured on `claude-haiku-4-5` through the flagship TUI: a cold turn
writes 13,696 tokens to the cache, and **every turn after it reads them
back — 96-98% of input served from cache, on every step of a multi-step
turn**. Cached input bills at ~0.1x, so the repeated prefix now costs
about a tenth of what it did. The cache also survives a TUI restart
within its TTL, so relaunching costs nothing.

This is complementary to the open token-reduction work (amd#3007 prefill,
amd#3008 dynamic tools, the gaia-voice trim): those shrink the prefix, this
stops paying for it on every repeat. If both land the effects multiply
rather than add — a smaller prefix, charged at a tenth, once instead of
per call.

<details>
<summary>🔍 Technical details</summary>

**Two breakpoints, not one.** Anthropic renders `tools` → `system` →
`messages`, so a marker on the system block covers the whole fixed
prefill. A second marker at the end of the tools segment is there
because caching gives no partial credit: with only the system marker,
one byte of drift anywhere in the system prompt would discard the tool
schemas too. Block-level markers rather than top-level `cache_control=`
on `messages.create()` — top-level auto-placement marks the *last*
cacheable block, which in an agent loop is the newest tool result, so
every request would write a fresh entry and read almost nothing.

**`input_tokens` is the uncached remainder, not the prompt size.**
`prompt_tokens` now sums it with the cache reads and writes; left alone,
a working cache would have read as a prompt that shrank by 98%. The
streaming path takes the counters off `message_start`, the only event
that carries them.

**Metrics source.** The turn record and the `--dev` block prefer the
backend's own cache accounting wherever it reports any (a cold turn that
only *wrote* counts, so turn 1 and turn 2 are on the same scale).
Lemonade reports none, so the local prefix estimate still drives that
display unchanged — the two sources are recorded separately and never
summed.

**Prefix-stability audit — what still limits the hit rate.** Caching is
a prefix match, so anything volatile ahead of a breakpoint caps the
ceiling. Clean: no timestamp, uuid, session id, pid, or cwd anywhere in
the system prompt or tool descriptions (the one `datetime.now()` is
already correctly prepended to the *user* message, `memory.py:2114` — do
not "fix" it into the system prompt). Outstanding, in severity order,
none addressed here:

1. `dynamic_tools=True` with a 26-slot LRU cap over a ~66-tool registry
(`hub/agents/gaia/python/gaia_agent/agent.py:162,171`) rewrites the
`tools` array per turn once at the cap. Tools render first, so an
eviction invalidates everything. Worth considering pinning off under
`--use-claude`: a cached full registry at 0.1x beats an uncached
rotating subset at 1.0x.
2. `indexed_docs_section` is volatile but sits ~600 tokens from the top
of the system prompt, above ~2,500 tokens of static rules
(`gaia_agent_chat/profiles.py:141`, `agent.py:769-842`). Indexing one
document invalidates everything below it. Moving it to
`VOLATILE_PROMPT_FRAGMENTS` helps llama.cpp too.
3. The memory block prints `(confidence: 0.87)` per fact
(`memory.py:2044`), and every `recall` bumps confidence by 0.02 and can
reorder facts — a guaranteed per-turn byte change for no model benefit.
4. Splitting `system` into a static head and volatile tail with a
breakpoint between would salvage the static ~2,500 tokens from all of
the above.

The measured run above hit 96-98% because that session took the
full-registry path, so the tools array was byte-stable.
</details>

## Test plan

- [ ] `pytest tests/unit/test_claude_provider.py
tests/unit/test_turn_metrics.py -q` — 54 tests, including breakpoint
placement on the outgoing request, no mutation of the caller's tool
list, cache-field parsing on both the streaming and non-streaming paths,
and an older SDK response with no cache fields.
- [ ] `cd tui && go test ./internal/ui/chat/ ./internal/event/`
- [ ] Full `pytest tests/unit -q` was run and diffed against the
unmodified base commit: **no new failures** (605 vs 610 pre-existing
environmental failures — this branch has 5 fewer, none related).
- [ ] Live, two identical consecutive turns:
  ```
GAIA_TURN_LOG=/tmp/turns.jsonl gaia run gaia --dev --use-claude
--claude-model claude-haiku-4-5
  ```
  Before / after `cache_read_input_tokens`, straight from the turn log:

  | | prompt_tokens | cache_read | cache_write |
  |---|---:|---:|---:|
  | before (any turn) | 14,034 | **0** | 0 |
  | after, turn 1 (cold) | 14,034 | 0 | 13,696 |
  | after, turn 2 (identical) | 14,057 | **13,696** | 0 |
  | after, 2-step tool turn | 28,403 | **27,392** (both steps) | 0 |

**Not run: `gaia eval agent`.** This touches an LLM-affecting surface,
so CLAUDE.md requires it, and it has not been run — Lemonade is
currently banned on this machine (the user's PC crashes), and a
Claude-backed run is not a valid substitute for a local-model baseline.
It remains outstanding and should be run before merge on a machine where
Lemonade is available. Nothing here changes prompt *text*, tool schemas,
or the tool-call envelope — only where the cache breakpoints sit and how
usage is parsed — but that is an argument for expecting it to pass, not
evidence that it did.

---------

Co-authored-by: Ovtcharov <kovtchar@amd.com>
Co-authored-by: kovtcharov-amd <kalin.ovtcharov@amd.com>
The flagship shipped all 66 of its tool schemas to the model on every LLM
call. A conversational turn is a ReAct loop of 2-5 calls, so a 4B model
re-read ~12,200 tokens of tool definitions several times before it could
start on the user's question. It now sends a CORE set plus whatever the turn
semantically matches: 12,164 -> 4,654 tiktoken(cl100k) tokens per call, a 62%
cut, measured across six representative queries.

The machinery (ToolLoader, bundles, semantic selection, the load_tools escape
hatch) already existed from amd#1449/amd#1450 but was gated to the doc profile and
default-off. Two things had to be true before flipping it:

- FULL_CORE_TOOLS / FULL_BUNDLES now cover the flagship's registry exactly.
  DOC_BUNDLES only covered 38 of 66; enabling the loader against it would
  have left 29 tools unable to arrive with their cohort or be recovered
  through load_tools. A drift guard fails CI if the two sets ever disagree,
  so a new flagship tool forces a bundling decision instead of silently
  shipping unselected.
- ToolLoader gained optional_tools, because "absent from the registry" is not
  always drift. search_documentation needs npx, the skill-library and
  code-index tools only exist on GaiaAgent, and the memory tools do not
  register at all when the embedder is down. Validation used to turn that
  last case into a hard ValueError on the first turn of a degraded agent.

Only GaiaAgent defaults dynamic_tools on; ChatAgent keeps it off and every
profile outside doc/full still returns no loader at all. The cap is 26 for
the flagship (10 CORE + 16 slots), swept offline against nine queries: 22 cut
the web bundle in half on a research question, 26 lands every matched bundle
whole, 30 buys nothing more.

tools_count moves 66 -> 67 in the manifest and registration: load_tools
registers whenever the loader is active. That is the registry size, not the
per-turn visible size.

Composes with, rather than duplicates, the separate work to drop the
redundant AVAILABLE TOOLS block: the filter shrinks both surfaces, so the
saving here holds whichever way that lands.
@github-actions

Copy link
Copy Markdown
Contributor

Verdict: Approve with suggestions 🟢

Three coherent changes: memory tools added to FULL_OPTIONAL_TOOLS to fix a hard ValueError in degraded state (correct fix for a real crash path), _build_skeleton_tool_loader extended to accept any profile, and a new build_full_agent_skeleton for the flagship. The new token-budget regression test is a good idea and the thresholds are reasonable.

One nit worth fixing before the code ossifies:

🟢 In the new test, the comment says "CORE plus enough dynamic slots to reach the cap" but the code picks sorted(registry)[:cap] — an alphabetical slice with no relationship to CORE membership. Some CORE tools (e.g. update_memory) start late in the alphabet and won't appear in this slice. The test still fires correctly as a ratio check, but the comment misrepresents what's being measured.

🔍 Technical details

hub/agents/gaia/python/tests/test_full_tool_bundles.py:74-77

# current (misleading comment):
# A saturated session: CORE plus enough dynamic slots to reach the cap.
cap = GaiaAgentConfig().dynamic_tools_max
saturated_names = sorted(registry)[:cap]

A cleaner description of what the assertion actually tests:

# Any cap-sized subset costs less than 55% of the full registry —
# verify the ceiling scales with the filter, regardless of which tools
# are loaded.
cap = GaiaAgentConfig().dynamic_tools_max
saturated_names = sorted(registry)[:cap]

Or, if the intent really is to model a CORE-plus-dynamic session, build the set explicitly:

core_in_registry = sorted(n for n in FULL_CORE_TOOLS if n in registry)
extra = sorted(n for n in registry if n not in FULL_CORE_TOOLS)[: cap - len(core_in_registry)]
saturated_names = core_in_registry + extra

The second form makes the 55% bound meaningful as a "realistic session" claim; the first form is fine as-is as long as the comment matches.

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

Labels

agents eval Evaluation framework changes performance Performance-critical changes tests Test changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants