test(tool-loader): pin #800 doc-profile data-vs-recall disambiguation - #1844
Conversation
Review — ApproveVerdict: Approve. This is a clean test + docs closeout for #800 with no runtime code changes. It adds a deterministic regression test, a live eval scenario, and an in-repo note that together pin a fix (the dynamic tool loader) that until now had no test guarding it. The work is solid and the claims hold up. I re-ran the 4 new unit tests against the real loader/config and they pass; the central argument — that memory One thing worth a follow-up, non-blocking: the committed eval scenario currently records a FAIL (per the PR description) for a reason unrelated to #800 — a bug in how the data tool handles the monthly-summary CSV plus an agent hallucination on turn 1. Shipping a "live proof" scenario that's red can read as "the feature is broken" to anyone who runs the suite without reading this PR. The unit test is correctly the binding gate, so this is fine to merge — but please file a tracking issue for that separate data-tool bug so the red scenario has a paper trail and eventually goes green. 🔍 Technical detailsVerification performed
Issues🟡 Committed eval scenario is currently red for an unrelated reason ( 🟢 Thin top-salesperson margin ( Strengths
|
CI failures are pre-existing Windows/Lemonade infra flakiness — not caused by this PRThe two red checks are both Windows + real-Lemonade-Server integration steps that this PR's diff (one unit test, one eval scenario YAML, one docs note) neither touches nor runs. What actually failed
Why this PR triggered them (but didn't cause the failure)Both workflows trigger on Evidence it's flaky infra, not a code defect
Recommendation
|
Consolidating every agent onto `Gemma-4-E4B-it-GGUF` is an LLM-affecting change, and CLAUDE.md requires `gaia eval agent` runs against the committed baselines for exactly that class of change — but no CI job ran them, because they need Lemonade on real AMD hardware. Today the consolidation could merge on unit tests alone, which cover code paths and say nothing about model behavior. This adds the missing job on the self-hosted `lemonade-eval` pool: it runs `rag_quality`, `context_retention` and `tool_selection` serially, compares each against its committed Gemma baseline, and uploads the scorecards and traces. It **reports** rather than blocks by default. Code-generation and data-analysis scenarios are expected to regress on a 35B MoE → ~4B swap and that tradeoff is accepted; wiring `--compare`'s exit 2 straight to the step result would make the job permanently red, and a job that is always red gates nothing. Set `enforce: true` (or put `[eval-enforce]` in the commit message) to make regressions blocking. One gate is **not** softened by report mode. `gaia eval agent --compare` only scores scenarios present on both sides, so scenarios that vanish from a run land in an `only_in_baseline` bucket outside its exit-2 verdict — I confirmed that dropping 6 of 7 `rag_quality` scenarios still exits **0**. A broken harness would otherwise sail through as "no regression". The integrity gate therefore fails the job unconditionally when a baseline scenario is missing or came back unmeasured, whatever `enforce` says. ### Verified against a live Lemonade 10.10.0 Behavior this workflow depends on, checked on real fixtures rather than assumed: - `--compare` exits **0** clean, **2** on a real regression (`PASS → FAIL`), **1** on a missing file — the workflow treats only 2 as a verdict and any other non-zero as a harness error. - `gaia eval agent --category X` exits **0** regardless of scenario scores, so the `&&` chain does not short-circuit on the expected regressions. - Scenarios present in CURRENT but absent from BASELINE are neutral — relevant because `tool_selection` now has 5 scenarios on disk vs 4 in the baseline (`data_vs_recall_disambiguation`, added by amd#1844, is currently ungated until a baseline refresh). - The integrity gate itself: passes a healthy run, fails the 6-of-7-vanished case, fails an all-`INFRA_ERROR` category. - Backend `/api/health` returns 200; the `Output: <run-dir>` parsing handles trailing slashes and multiple matches. ### Test plan - [ ] First push to a branch touching a `paths:` entry fires the job — this is the first real runner execution and the debugging pass. - [ ] Confirm the `lemonade-eval` pool is bash and has a persistent Lemonade; the preflight fails loudly with an actionable message if not. - [ ] Confirm `pip install -e ".[dev,eval,ui,api]"` resolves on the runner. - [ ] Check the wall time against the 150-min budget and tighten if there is slack. - [ ] Re-dispatch with `enforce: true` once the accepted regressions are folded into refreshed baselines. ### Notes for the reviewer - Timeout is **150**, not the siblings' 90. The three baselines sum to ~55 min of pure scenario time before install, backend boot and judge latency; a timeout cancels the job and can take the artifact with it. Happy to drop it back once a few runs establish the real number. - Time regressions are **not** comparable to this baseline — it was captured against a remote Lemonade over ngrok on different hardware (see its `meta.json`). `--compare` folds a >2x wall-clock delta into the same exit 2 as a quality regression. The summary block says so; regenerating the baselines on the runner would fix it properly. - `gemma-4-e4b-d71cd914` is the only baseline covering all three categories. `gemma-4-e4b-95e4b372` is newer but `rag_quality`-only.
2 participants
Summary
Before this PR, #800's scratchpad/memory tool collision was resolved in code (by the #688 dynamic tool loader, landed via #1449/#1450/#1451) but nothing pinned it — no test asserted that the structured-data tool and memory
recallcan't crowd each other out of the prompt, so the fix could silently regress. This adds a deterministic regression test, a live eval scenario, and an in-repo note that together lock the resolution and let #800 close with evidence. No runtime code changes — this is a closeout + regression, not a feature.Why
#800 is a coordination tracker: it exists to prove the collision is fixed, not leave it as a known gap. The decisive finding (verified on
main) is that the literal pair in the title —scratchpad.query_datavsmemory.recall— cannot occur in the loaded profile: scratchpad tools are registered only for the ChatAgentdata/fullprofiles, neverdoc, which is the only profile the loader is wired to. The real doc-profile arbitration isanalyze_data_file(structured-data, conditional) vsrecall(CORE, always-on). That asymmetry is the resolution — and until now no test encoded it.Linked issue
Closes #800
Changes
recall ∈ CORE(always present) whileanalyze_data_fileloads only when the turn's query clears the semantic threshold, using the realDOC_CORE_TOOLS/DOC_BUNDLESconfig and production τ/cap (not hand-picked literals), with a fresh loader per case so it tests the cold/empty-memory new-user state.sales_data_2025.csvcorpus, so the disambiguation is checked against a real model, not just unit logic.Deviations from the approved sketch (#800's body)
Flagged per CLAUDE.md — the landed design diverges from the original sketch in several places:
mainscratchpad.query_datavsmemory.recalldocprofile; loader isdoc-onlyanalyze_data_file(conditional) vsrecall(CORE)recallis CORE → always present; the conditional side isanalyze_data_filerecallis intentionally always-onmaintool_historyTOOL_LOADER {json}INFO lines;gaia.eval.tool_recallconsumes themscratchpad/browserindocdocprofilesmart_discovery(hardware artifact); fixture is also independently staleTest plan
python -m pytest tests/unit/test_tool_loader_disambiguation.py -v→ 4 passed (the AC Update Documentation #5 gate)python -m pytest tests/unit/test_tool_loader_selection.py tests/unit/test_chat_tool_bundles.py tests/unit/test_chat_dynamic_tools.py -q→ 67 passed (no loader regression; confirms the new test's registry/embedder assumptions match shipped config)python -m pytest tests/test_eval.py -k scenarios -q→ 27 passed (validates the new scenario YAML: required fields, sequential turns, existing corpus path)python util/lint.py --all→ cleangaia eval agent --category tool_selection --agent-type doc→ the 4 pre-existing scenarios match the Tool loader — Part 3: skill-driven signal (gated on #887) #1451 Part-3 proof (no selection regression)Checklist
Closes #800).python util/lint.py --all,pytest tests/unit/).#800 Acceptance Criteria — Proof
Verdict: all 7 ACs satisfied on
main+ this PR. The collision is resolved structurally by the #688 dynamic tool loader (landed via #1449/#1450/#1451); this PR pins it. Three ACs are satisfied with the documented reframing above.query_data&recallnot both unless justifiedAC #1 — Tool-prompt token reduction
Reframed: the ~12K figure assumed the text path; the real cost (and gate) is the native tool-schema path / TTFT. The 38-tool doc profile is capped to 14 (
DEFAULT_MAX_TOOLS).test_core_only_is_the_reduction_best_casepins the always-on CORE floor at ≤45% of the native baseline (~50–60% reduction).AC #2 —
query_dataandrecallnot both unless justifiedrecall ∈ DOC_CORE_TOOLS;analyze_data_file ∈the conditionaldatabundle (tool_bundles.py). The conditional tool loads only when the query clears τ:AC #3 — Core tools always available despite heuristic failure
CORE is admitted unconditionally and is cap-/eviction-exempt; on embedder failure the loader disables for the session and falls back to the full registry, logging loudly (
tool_loader.py).AC #4 — Decisions logged for eval/tune
Satisfied with a deliberate deviation: decisions are emitted as structured
TOOL_LOADER {json}INFO lines (not the SQLtool_historytable — avoids a UI-DB migration). Consumed bysrc/gaia/eval/tool_recall.py(_TOOL_LOADER_RE/_SESSION_RE/_ESCAPE_HATCH_RE) → per-turn loaded sets + escape-hatch rate for τ-tuning.AC #5 — End-to-end regression test (data-query vs recall) — new in this PR
tests/unit/test_tool_loader_disambiguation.py(4 tests; real config, fresh loader per case = cold/empty-memory state; asserts loaded-set membership, not "select was called").eval/scenarios/tool_selection/data_vs_recall_disambiguation.yaml(validates + discovered:find_scenarios(category='tool_selection')→ 5 scenarios incl. the new one).AC #6 — Eval suite passes, no selection regression
Live serial run on Gemma-4-E4B corroborates the #1451 Part-3 success-criteria proof — no regression:
known_path_readno_tools_neededmulti_step_plansmart_discovery* hardware artifact of the local Apple-Silicon box (Metal llama.cpp ~14–19 tok/s), not a behavior change. The new scenario records FAIL 6.58 for reasons unrelated to #800: the disambiguation works (agent routed both aggregates to
analyze_data_file, never misusedrecall; Turn 2 returned the exact answer), but Turn 1's correctness failed onanalyze_data_file's date-filter handling of the monthly-summary CSV plus an agent hallucination — flagged as a separate follow-up. Committed baseline refresh deferred to a clean run on target hardware.AC #7 — Mid-conversation re-evaluation works
Proven for in-profile pivots (the sketch's "file→web" example is out of the doc profile — browser tools aren't registered there):