fix(email): ship with Agent Skills off until the eval gate covers them - #2848
fix(email): ship with Agent Skills off until the eval gate covers them#2848kovtcharov-amd wants to merge 3 commits into
Conversation
github-actions
Bot
added
documentation
|
Verdict: Approve ✅ This ships the email agent's Agent Skills switched off — commenting out the The bottom line: nothing is deleted — the six skill files, the selection machinery, and the budget accounting all still ship, inert — so re-enabling is a one-line manifest edit, and the tests are structured to keep full coverage behind that switch. No blocking issues. One tiny thing to fix while you're in the file: the npm Real-world evidenceThe CI evidence stage ran the gaia-testing skill and wrote $ gaia-agent-email serve --skill-set work
gaia-agent-email: error: --skill-set requested skill set 'work', but this agent declares no skill sets — Agent Skills are switched off in this build. Drop the option, or uncomment the 'skill_sets:' and 'default_skill_set:' blocks in gaia-agent.yaml.
EXIT: 2The Deferred with reason: live model triage with/without a set active (needs Lemonade — pending strix-halo lane). No Agent-UI screenshot is applicable — skill sets have no UI surface (no 🔍 Technical details🟢 Minor — stale tool count in npm SPEC.md ( The line reads "tool count is unchanged (59)", but the agent's canonical, test-guarded count is 65 ( (An older Python CHANGELOG entry, Correctness — the guard ( Clean. Test structure Well thought through: the Minor coverage note (not blocking): the new Strengths
|
|
🟡 The new CHANGELOG entry claims 🔍 Technical details
This PR modifies the paragraph around that SPEC.md line but leaves |
|
Verdict: Approve (one optional doc nit) This PR ships the email agent's Agent Skills switched off — the The bottom line: it's correct, the docs are kept in lockstep across every surface, and the tests cleanly separate "what ships today" (skills off) from "how the machinery behaves when re-enabled" so turning them back on stays a one-line manifest edit with coverage already behind it. The only thing worth touching is a stale tool-count number in the npm SPEC. Real-world evidence
🔍 Technical details🟢 Minor — stale tool count in npm SPEC ( (The older Correctness — verified clean:
Strengths:
|
eval evidence behind them. Loading the `personal` set costs ~1,334 prompt tokens, which cuts the bulk-triage result envelope from 6144 to 4810 (the `work` set, to 4070) — a fifth to a third of the room the agent has for tool results, traded for a quality improvement nothing has measured yet. Comment out `skill_sets:` and `default_skill_set:` in gaia-agent.yaml so the agent resolves no set and loads no skills. `envelope_budget_tokens()` returns to 6144, byte-identical to pre-skills. Nothing is deleted. The six SKILL.md bodies still ship in the wheel and the frozen binary, and SKILL_DIRS, select_skill_set(), the account-type mapping, `--skill-set`, and the skill_prompt_tokens accounting are all intact but inert. Re-enabling is uncommenting the two blocks, together — a non-empty skill_sets: with no default_skill_set: fails validation. With no sets declared a pinned set is now a startup error rather than a silent no-op. The sidecar previously rendered that as "Valid sets: ." on an empty list; it now names the real cause and the fix. The set-resolution tests move to a fixture manifest that switches the blocks back on, so the generic machinery keeps full coverage and re-enabling lands on tests that already pass. New tests pin the shipped state: zero skills, no active set, a 6144 envelope, and a loud refusal for --skill-set.
server.main() pins a requested skill set by writing straight into os.environ — that is how the override reaches per-request agent sessions, so it is deliberate. monkeypatch cannot undo it: delenv(..., raising=False) records nothing when the variable was already unset, so the --skill-set flag tests left 'work' set for the rest of the session. Harmless while the manifest declared 'work'. Now that it declares nothing, the next EmailTriageAgent built by any later test file raises SkillSetError — which is why test_trash_restore, test_undo_reachable, and test_zero_connector_construction failed in CI but passed when run alone. Restore the variable in the email package's conftest, next to the other order-dependence guard it already owns.
e5d102b to
9b1563c
Compare
2 participants
Every email agent launch on
maintoday loads three skill bodies into its system prompt. That costs ~1,334 prompt tokens and cuts the bulk-triage result envelope from 6144 to 4810 — a fifth of the room the agent has to hand back a tool result (theworkset, a third: 4070) — traded for a triage quality improvement that no eval has measured. #2695 turned this on by default and it is live now. After this PR the agent resolves no skill set, loads nothing, and the envelope is back to 6144, byte-identical to pre-skills. The six skills stay in the wheel and the frozen binary, inert; re-enabling is uncommenting two blocks ingaia-agent.yaml, once an eval backs them.Two things worth flagging for review:
--skill-setnow fails at startup instead of quietly doing nothing. That is the intended no-silent-fallbacks behaviour. The sidecar used to render the empty case asValid sets: .; it now names the real cause and the fix.SCORECARD.mdis deliberately not regenerated, and is now accurate again. It contains no reference to skills or a skill-adjusted budget, and itsgenerated_atis 2026-07-16 — three weeks before the skills landed in bbf69fd. Every number in it, includingtokens_per_triage1906.03, was measured with zero skills loaded and a full 6144 envelope. Between feat(email,skills): bundled skills + account-keyed skill-set selection #2695 merging and this PR it described pre-skills behaviour while the shipped agent had a set active; with the blocks commented out the runtime matches its measurement conditions again.The set-resolution tests move to a fixture manifest that switches the blocks back on, so the generic skill-set machinery keeps full coverage and re-enabling lands on tests that already pass, rather than on a rewrite.
Test plan
python -m pytest hub/agents/email/python/tests/test_skill_sets_2466.py— 42 passed, covering both the shipped state (zero skills, no active set, 6144 envelope, loud--skill-setrefusal) and the generic machinery against the fixture manifestpython -m pytest tests/unit/test_skill_sets.py— framework coverage, unchanged by this PR (it builds inline manifests and never reads the email agent's). One pre-existing Windows-only path-separator failure intest_manifest_error_names_the_file, present identically onmainpython -c "from gaia.hub.manifest import parse; from gaia_agent_email.agent import EmailTriageAgent as A; s=parse(A.SKILL_MANIFEST).skill_sets; print(bool(s), s.set_names, s.default_set)"→False [] Nonepython -c "from gaia_agent_email.context_budget import envelope_budget_tokens as e; print(e())"→6144SKILL.mdfiles undergaia_agent_email/skills/andSKILL_DIRSstill pointing at themgaia-agent-email serve --skill-set personalexits non-zero naming the cause and the fixVerified locally: the 42 skill-set tests above;
envelope_budget_tokens()== 6144 proven by an actual run, not by reading the code; the shipped manifest parsing to zero sets; the real--skill-setstartup error text; and the ~1,334 / 4810 / 4070 token figures quoted throughout, measured rather than assumed (an earlier "278 tokens of headroom" figure did not reproduce and was corrected everywhere it had propagated).Not finished: a full-repo baseline comparison was still running when this PR opened, so unrelated pre-existing full-suite failures are not yet distinguished from anything this change might cause. The affected test files run in isolation are byte-identical with and without this change (2 failed / 130 passed either way, both pre-existing), but CI's run against
mainis the authoritative comparison.util/lint.py --allcould not complete locally (its tools are fetched viauvxand the sandbox blocks PyPI); note it lintssrc/gaiaandtestsonly, and this PR changes no Python under either.