feat(skills): make image generation reachable from the flagship - #3073

🔴 Critical

Selection tests fail without a live embedder — test_gaia_agent.yml has none (hub/agents/gaia/python/tests/test_sd_tools_do_not_rewrite_the_prompt.py:110, :123)

_select_fresh calls the real ToolLoader.select, which embeds the query via MemoryMixin._embed_text → Lemonade. tool_loader.py:266-277 catches the failure, sets _session_disabled, and returns None. The workflow runs pytest hub/agents/gaia/python/tests/ on a plain ubuntu-latest with no Lemonade (.github/workflows/test_gaia_agent.yml:85), so assert selected is not None fails for all three cases. Sibling tests in this package avoid it by injecting a fake embed_fn (test_lazy_skill_activation.py:102-109). Cheapest fix keeps the assertion meaningful where an embedder exists:

 selected = _select_fresh(flagship, query)
 if selected is None:
 pytest.skip("dynamic tool selection needs a live embedder")
 assert SD_TOOLS <= set(selected)

Injecting a deterministic embed_fn would be stronger — a skip means CI never checks the behaviour the test was written for.

🟡 Important

Remedy commands name the removed legacy CLI (src/gaia/sd/mixin.py:473-474, :479-481; hub/skills/image-gen/SKILL.md:99)

describe_client_hint/describe_start_hint exist for exactly this and resolve modern vs. legacy tooling per machine — mixin.py:34 already imports the first and uses it at :599. test_remedy_commands_are_runnable.py:113-117 asserts a modern install never sees lemonade-server load; the new text names it unconditionally, so on a modern install both branches point at a binary that isn't there.

 if "timed out" in lowered or "timeout" in lowered:
 return (
 f"Timed out waiting for {model}; the server is running but did "
 "not answer in time. First use of an SD model both downloads "
 "and loads several GB. "
 f"{describe_client_hint('pull', model).instruction} Then confirm "
 f"it loads: {describe_client_hint('load', model).instruction} ({raw})"
 )
 if "connection refused" in lowered or "failed to establish" in lowered:
 return (
 f"Cannot reach Lemonade Server. {describe_start_hint().instruction} "
 "Or set LEMONADE_BASE_URL to a running server. "
 f"({raw})"
 )

Needs describe_start_hint added to the lemonade_launcher import. Note this also changes what tests/unit/test_sd_error_messages.py:46,:37 should assert — they currently pin the literal lemonade-server serve / pull strings, so they'd need to mock resolve_lemonade the way test_remedy_commands_are_runnable.py does, or assert against the helper's output. As written those assertions are environment-dependent: with no client binary resolved, describe_client_hint emits prose containing no pull at all.

Generation history never records the seed the skill tells the model to reuse (src/gaia/sd/mixin.py:396; hub/skills/image-gen/SKILL.md:74-79)

result["seed"] = seed stores the requested seed, which is None on any call that didn't specify one — the normal case. The response is parsed for b64_json only (:379), so the server-chosen seed is discarded. get_generation_history() therefore returns "seed": null, and the skill's "keep everything else — including the seed" instruction has nothing to keep. Recording one at call time fixes it (add import random at the top):

 start_time = time.time()
 # Record a concrete seed — history-driven iteration ("same but at
 # sunset") needs one, and the server never returns the one it picked.
 if seed is None:
 seed = random.randrange(2**32)

Alternative, if pinning the seed is undesirable: drop the seed-reuse paragraph from the skill and say the second image will differ.

The absolute output dir may break the row13 golden (hub/agents/chat/python/gaia_agent_chat/agent.py:1486-1491, tests/unit/test_profilespec_characterization.py:266-271)

That harness builds a full-profile ChatAgent with enable_sd_tools=True under patch.object(Path, "home", return_value=Path("/fake/home")) and a cleared os.environ. GAIA_CONFIG_DIR is computed at import of gaia.config, which nothing in the harness's import chain pulls in at module level — so if that first import happens inside the patch, the new sd_output_dir is /fake/home/.gaia/cache/sd/images, mkdir raises PermissionError, the new except Exception swallows it, and the SD tools are missing from a golden that lists them. If instead the module was already imported, the test writes into the developer's real ~/.gaia. I could not run pytest in this review environment — please run pytest tests/unit/test_profilespec_characterization.py -k row13 both standalone and as part of the full file to confirm which.

LLM-affecting change with no eval run shown (hub/agents/gaia/python/gaia_agent/agent.py:178, src/gaia/sd/mixin.py:147)

Three tools added to the flagship registry, a changed model parameter description, and a system-prompt composition change all sit on CLAUDE.md's "requires gaia eval agent before merge" list. evidence-bundle.md contains no scorecard comparison. Link the run if it happened elsewhere.

🟢 Minor

A connect-timeout is reported as "the server is running" (src/gaia/sd/mixin.py:469) — requests renders a connect timeout as Connection to host timed out. (connect timeout=5), which hits the timeout branch and tells the user a server that never answered a SYN is healthy. Same substring-ordering class of bug the helper was written to fix. Checking "connect timeout" before the general timeout test covers it.

The characterization harness's litter guard is now dead (tests/unit/test_profilespec_characterization.py:266-269) — it redirects cwd to tmp_path solely because init_sd used to mkdir a relative .gaia/. With the absolute path that redirect does nothing; see the 🟡 above for why it may do worse than nothing.

Strengths

• The prompt/tools split is the right call and is pinned properly. Suppressing the 5K SD persona while keeping the one-line image_gen bundle entry gets the capability without the identity, and test_the_capability_is_still_advertised_as_a_bundle stops a future "cleanup" from removing the only thing that makes it discoverable.
• Bundle/manifest bookkeeping is complete. image_gen added to FULL_BUNDLES, the three names added to FULL_OPTIONAL_TOOLS with the reason documented, and tools_count moved in both the manifest and build_gaia() — so the existing drift guards stay meaningful rather than being worked around.
• The starter-skill honesty guard was extended, not bypassed. Wiring init_sd into the registry fixture (test_starter_skills.py:199) and adding generate_image to the guards-the-guard assertion keeps tools_required validation from going vacuous for the new skill.
• The image-gen skill is written for the constraint that actually matters — generation is slow, so spend the thinking before the call — and it explicitly forbids silent fallback to a smaller model, matching the repo's fail-loudly rule.

Silent-fallback note (hub/agents/chat/python/gaia_agent_chat/agent.py): the except Exception as _sd_err: logger.warning(...) handler discards the error and lets the agent proceed without SD tools. init_sd in practice can only fail with filesystem errors (directory creation) or unexpected SDK exceptions — the SD client doesn't contact the server at init time, so "Lemonade not running" is not a trigger. Still, CLAUDE.md §"No Silent Fallbacks" says: "add a specific exception type, log with context, or re-raise." exc_info=True adds context; narrowing to OSError (the realistic failure) or re-raising would complete the fix.

_get_mixin_prompts is now unconditional — even a deliberately standalone SD agent would get the persona stripped. This is intentional and pinned by test_the_sd_persona_stays_out_of_the_system_prompt, but authors composing SDToolsMixin in isolation should know the prompt is now opt-in via skill, not auto-included.

Selection skip logic (_select_fresh in the new test file): if select() returns None for a reason other than session_disabled, the caller does set(None) → TypeError. In practice this can't happen (the loader only returns None when disabled), but a guard would make the skip path more robust.

Conflict — hub/agents/gaia/python/gaia_agent/agent.py only. Main's #3010 inserted skill_discovery / skill_discovery_threshold directly above the enable_sd_tools block; resolution keeps both, with enable_sd_tools: bool = True. Merged rather than rebased so nothing already pushed gets rewritten.

except Exception → except OSError (gaia_agent_chat/agent.py). Verified before narrowing, not inferred:

• LemonadeClient.__init__ is URL parsing plus attribute assignment — no network call. Constructed against a dead port in 0.2 ms, so "Lemonade not running" cannot reach this handler.
• resolve_lemonade_api_key and _get_lemonade_config have no raise paths.
• @tool assigns into _TOOL_REGISTRY with no duplicate check.
• That leaves sd_output_dir.mkdir(parents=True, exist_ok=True) → OSError.

Both branches exercised: injected PermissionError → caught, WARNING naming the path and remedy, agent builds without SD tools; injected RuntimeError → propagates. GAIA_CONFIG_DIR is a real env override (src/gaia/config.py:24), so the suggested remedy is honest.

_select_fresh — pytest.fail on None with the session enabled, instead of falling through to set(None). Preserves the property that a real selection regression fails rather than skips.

Validation — 484 passed, 16 skipped across hub/agents/gaia/python/tests/, test_chat_tool_bundles, test_sd_error_messages, test_starter_skills. util/lint.py --all: Black, isort, Flake8 pass. mypy reports 13 pre-existing errors, all in files untouched here (factory/harvest/* from #3111, ui/sse_translation.py from #3026); src/gaia/sd/mixin.py is clean.

Eval — gaia eval agent not run. Lemonade was not listening on 13305 or 8000, and ANTHROPIC_API_KEY is set to the placeholder dummy, which the judge would pass straight to the SDK and 401 on rather than falling back to subscription auth. The author's own run hit the 900 s cap on every tool_selection scenario, so there is still no scorecard to diff against gemma-4-e4b-d71cd914. Separately, no generated PNG has been produced end to end — image output remains unproven.

tests/unit/test_sd_error_messages.py:741:

assert "lemonade-server serve" in message

This assertion mirrors the literal string in SDToolsMixin._describe_client_error (src/gaia/sd/mixin.py:681-684). The prior review asked for both the mixin.py error messages and the SKILL.md copy to use describe_start_hint() / describe_client_hint() instead of the hardcoded binary. If that fix lands, this assertion will need to change too — but as written it pins the wrong string and will fail once the mixin is corrected, which may cause the author to revert the fix rather than update both together.

The seed-not-recorded issue (result["seed"] = seed stores None for default calls, SKILL.md promises the seed is always reusable) is unchanged from the prior review — not a new regression, but still open.

src/gaia/sd/mixin.py:478-487 — the new _describe_client_error method:

"Pre-fetch it with `lemonade-server pull "
f"{model}`, confirm it loads with `lemonade-server load "
f"{model}`, then retry. ({raw})"

"Cannot reach Lemonade Server. Start it with "
"`lemonade-server serve`, or set LEMONADE_BASE_URL to a running "

The helper that resolves the right binary already exists and is already imported in this file (describe_client_hint, describe_start_hint from gaia.llm.lemonade_launcher; mixin.py:34 already imports describe_client_hint and uses it at line ~600 for the non-SD path). Use them here the same way.

tests/unit/test_sd_error_messages.py:741 pins assert "lemonade-server serve" in message — this will need to be updated to assert against whatever describe_start_hint().instruction returns, the same pattern test_remedy_commands_are_runnable.py uses.

hub/skills/image-gen/SKILL.md:555-558 also names lemonade-server pull <model> directly in the failure-handling section.