Skip to content

refactor(agents): migrate chat to hub (#1102)#1456

Merged
kovtcharov merged 6 commits into
mainfrom
claudia/task-247f4a89
Jun 26, 2026
Merged

refactor(agents): migrate chat to hub (#1102)#1456
kovtcharov merged 6 commits into
mainfrom
claudia/task-247f4a89

Conversation

@kovtcharov-amd

Copy link
Copy Markdown
Collaborator

Why this matters

ChatAgent — the flagship conversational agent powering gaia chat (and gaia chat --ui) under its chat, doc, and file profiles — was the last family of registry builtins still hardcoded inside the core framework wheel. It now ships as the standalone gaia-agent-chat wheel under hub/agents/python/chat/, with chat/doc/file registered as three gaia.agent entry points that the registry discovers automatically. The core wheel no longer hardcodes them, so the chat family versions independently like every other migrated agent. gaia chat resolves the agent through the registry and fails loudly with an install hint if the wheel is absent (mirroring how #1446 made gaia browse/gaia analyze resolve). All three profiles, their full+lite model tiers (via registry.build_model_tiers), and their per-profile prompt/tool wiring are preserved exactly.

Continues the #1102 Agent Hub restructure after connectors-demo (#1442) and analyst/browser (#1446). The shared FileToolsMixin (directory monitoring) stays framework-side — promoted to gaia.agents.tools.file_monitor_tools — since tool mixins are framework code (#1396).

Test plan

  • python util/lint.py --all — black/isort clean (the only pylint hit, os.geteuid in lemonade_installer.py, is a pre-existing Windows-only false positive; mypy is warning-only)
  • pytest tests/unit/agents/test_registry.py tests/unit/test_agents_split.py tests/unit/cli/test_cli_smoke.py — registry + split green; the 4 cli_smoke gaia-code/gaia-emr console-script hits are pre-existing shared-env pollution (verified passing on the base checkout), unrelated to chat
  • pip install -e hub/agents/python/chat && pytest hub/agents/python/chat/tests/ — 7 passed (registration shapes for chat/doc/file, lazy re-exports, discovery, installed: namespacing)
  • pytest across the chat-touched framework suites (chat agent, integration, system-prompt budget, browser tools, multi-device, required-connectors, registry factory, chat-UI helpers) — 182 passed with the wheel installed; all skip cleanly via importorskip in a framework-only env
  • Registry discovery lists chat/doc/file as source=installed with installed:chat namespace; builder/email remain the only resident builtins
  • CI: the Chat Agent Tests workflow installs the wheel and runs both the package smoke tests and the framework session/RAG/path-validation suites

@github-actions github-actions Bot added dependencies Dependency updates devops DevOps/infrastructure changes cli CLI changes tests Test changes agents labels Jun 4, 2026
@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Code Review — refactor(agents): migrate chat to hub (#1102)

Verdict up front: Approve with suggestions. This is a clean, well-tested lift of the ChatAgent family (chat/doc/file) out of the core wheel into the standalone gaia-agent-chat package, faithfully mirroring the analyst/browser migration (#1446). No blocking issues. Two small framework-side staleness items and the doc import examples are the only follow-ups worth tracking.

Summary

ChatAgent now ships as three gaia.agent entry points discovered by the registry, the core wheel drops the hardcoded builtins, and gaia chat fails loudly with an actionable install hint when the wheel is absent. The model tiers, per-profile wiring, consumes_mcp_servers guard, and installed:chat namespacing are all preserved, and the FileToolsMixin (directory monitoring) was correctly promoted to gaia.agents.tools so framework code (DocumentQAAgent) no longer reaches into the chat package. Test migration is thorough — every chat-touching suite gained an importorskip("gaia_agent_chat") so a framework-only env skips cleanly rather than erroring.

Issues Found

🟢 Stale FRAMEWORK_PATHS entry — chat frames no longer filtered from user errors (src/gaia/agents/base/errors.py:21)

format_user_error filters framework frames out of user-facing tracebacks using these path substrings. After the move, "gaia/agents/chat" matches nothing (the dir is gone) and the new location gaia_agent_chat isn't listed — so an exception raised inside chat-agent code now surfaces as "user code" in error output. Note "gaia/agents/code" on the next line carries the identical staleness from the code migration, so this is shared #1102 debt rather than something this PR introduced. Worth repointing in the same pass:

    "gaia/agents/base",
    "gaia/agents/blender",
    "gaia_agent_chat",
    "gaia/agents/code",

(or, more robustly for the whole hub effort, also filter hub/agents/python/.)

🟢 Stale source-path reference in docstring (src/gaia/mcp/mixin.py:28)

The docstring points at src/gaia/agents/chat/agent.py, which now lives at hub/agents/python/chat/gaia_agent_chat/agent.py. Trivial, but it'll mislead the next reader who greps for it.

🟢 Doc import examples now broken (~20 files under docs/)

docs/sdk/agents/specialized.mdx, docs/spec/chat-agent.mdx, docs/playbooks/chat-agent/*, docs/sdk/sdks/rag.mdx, and others still show from gaia.agents.chat.agent import ChatAgent (and gaia.agents.chat.tools...) as runnable examples that no longer resolve. CLAUDE.md treats doc accuracy as mandatory, but this mirrors the analyst/browser/code migrations' existing doc debt, so it's reasonable to land this PR and sweep all hub-migrated import paths in one follow-up rather than block here. Flagging so it gets tracked, not as a merge gate.

Strengths

  • Fail-loudly done right. The CLI ImportError → RuntimeError translation (src/gaia/cli.py) names what failed, exactly what to run (pip install gaia-agent-chat / amd-gaia[agents]), and the next step — textbook CLAUDE.md "actionable error," no silent fallback.
  • Framework→chat dependency severed cleanly. Promoting FileToolsMixin to gaia.agents.tools.file_monitor_tools and repointing DocumentQAAgent means the core wheel no longer imports the chat package — the actual point of the migration, and it holds.
  • The consumes_mcp_servers guard survived the move intact, with the package smoke test (test_build_chat_registration_shape) asserting it, so the feat(connectors): bind MCP connectors to specific agents (per-agent activation) #1005 activation-filter contract stays enforced from the new home.
  • Test discipline. importorskip everywhere chat is touched, _register_builtin_agents()discover() swapped where chat is now entry-point-discovered, and the reserved-id trojan test correctly re-pointed to builder now that chat is no longer reserved.

Verdict

Approve with suggestions. Nothing blocks merge. The two framework-side staleness nits are quick same-PR cleanups; the doc sweep is fair to defer as shared #1102 follow-up. Nice, faithful continuation of the hub restructure.

@kovtcharov-amd kovtcharov-amd self-assigned this Jun 8, 2026
@kovtcharov-amd kovtcharov-amd requested a review from itomek June 8, 2026 18:35
itomek
itomek previously approved these changes Jun 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🟡 tests/verify_path_validator.py:16 and tests/verify_shell_security.py:7 — bare imports with no guard

Every other file changed in this PR wraps gaia_agent_chat imports in pytest.importorskip or a try/except. These two standalone scripts can't use importorskip, but they get no guard at all:

# verify_path_validator.py line 16
from gaia_agent_chat.agent import ChatAgent, ChatAgentConfig

Running either script in a framework-only environment now throws a bare ModuleNotFoundError instead of a helpful message. Before this PR, both worked out of the box.

try:
    from gaia_agent_chat.agent import ChatAgent, ChatAgentConfig
except ImportError:
    raise SystemExit(
        "gaia-agent-chat is not installed — run: pip install gaia-agent-chat"
    )

Same fix applies to verify_shell_security.py:7. The rest of the migration looks correct.

@github-actions

Copy link
Copy Markdown
Contributor

🟡 agent_loop.py:388ImportError from the missing chat wheel is swallowed silently

When gaia-agent-chat isn't installed, from gaia_agent_chat.agent import ChatAgent, ChatAgentConfig at line 333 raises ModuleNotFoundError. The bare except Exception at line 388 catches it, logs "AgentLoop tick execution failed: No module named 'gaia_agent_chat'", and moves on — the user sees no install instruction.

cli.py wraps the same import with a try/except ImportError that raises a RuntimeError with the actionable message "Install it with pip install gaia-agent-chat". The Agent UI path should do the same:

            try:
                from gaia_agent_chat.agent import ChatAgent, ChatAgentConfig
            except ImportError as e:
                raise RuntimeError(
                    "The chat agent is not installed. "
                    'Run `pip install gaia-agent-chat` (or `pip install "amd-gaia[agents]"`), '
                    "then restart the server."
                ) from e

Same gap exists in the two lazy-import sites in _chat_helpers.py (_get_chat_response and _stream_chat_response) — those would surface as opaque 500 errors to the Agent UI instead of the helpful message the CLI shows.

kovtcharov added a commit that referenced this pull request Jun 26, 2026
Resolves the actionable items from the PR #1456 bot review:

- agent_loop.py + the two _chat_helpers lazy-import sites translated a missing
  gaia-agent-chat wheel into a swallowed ImportError / opaque 500; they now
  raise an actionable RuntimeError naming the install command, matching the CLI.
- verify_path_validator.py / verify_shell_security.py guarded their bare
  gaia_agent_chat import with a SystemExit install hint (importorskip is
  unavailable in standalone scripts).
- FRAMEWORK_PATHS dropped the now-dead gaia/agents/chat substring and filters
  the hub package generically (hub/agents/python/ + gaia_agent_chat) so
  chat-agent frames stay out of user-facing tracebacks; test updated to match.
@kovtcharov kovtcharov force-pushed the claudia/task-247f4a89 branch from 8d29835 to 3442b53 Compare June 26, 2026 02:48
@github-actions github-actions Bot added documentation Documentation changes mcp MCP integration changes rag RAG system changes eval Evaluation framework changes performance Performance-critical changes labels Jun 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🟡 Five broken GitHub hyperlinks in the published docs will 404 as soon as this merges. The path src/hub/agents/python/chat/… was used in several docs but the directory lives at hub/agents/python/chat/… (repo root, no leading src/). Dropping the src/ prefix fixes them.

🔍 Technical details

src/hub/ does not exist in the repo (ls src/hub/ → not found); the actual location is hub/agents/python/chat/gaia_agent_chat/.

Broken clickable links (will 404):

  • docs/plans/tool-loader.mdx — three <Info> / prose tool_bundles.py hrefs:

    https://github.com/amd/gaia/blob/main/src/hub/agents/python/chat/gaia_agent_chat/tool_bundles.py
    

    → should be …/main/hub/agents/python/chat/gaia_agent_chat/tool_bundles.py

  • docs/playbooks/chat-agent/part-3-deployment.mdx — Card href:

    https://github.com/amd/gaia/blob/main/src/hub/agents/python/chat/gaia_agent_chat/agent.py
    

    → should be …/main/hub/agents/python/chat/gaia_agent_chat/agent.py

  • docs/spec/chat-agent.mdx<Info> Source Code link (same wrong prefix)

Non-linked prose references in docs/reference/agent-core-loop-architecture.md, docs/spec/browser-tools.mdx, docs/spec/file-change-handler.mdx, docs/spec/file-system-agent.mdx, and docs/plans/agent-ui-eval-benchmark.md use src/hub/… too — those are informational-only but worth fixing for consistency.

@kovtcharov

Copy link
Copy Markdown
Collaborator

Rebased onto current main (was 122 commits behind) and addressed the review feedback. The migration commit replayed cleanly via rename-tracking, so main's evolution of the chat code is preserved rather than reverted to the original branch point.

Review items resolved:

  • Swallowed/opaque missing-wheel errorsagent_loop.py and both _chat_helpers.py lazy-import sites now raise an actionable RuntimeError with the install command instead of a swallowed ImportError / opaque 500, matching the CLI.
  • Unguarded standalone scriptsverify_path_validator.py / verify_shell_security.py now exit with an install hint when the wheel is absent.
  • Stale FRAMEWORK_PATHS — dropped the dead gaia/agents/chat substring and added a generic hub filter (hub/agents/python/ + gaia_agent_chat) so chat-agent frames stay out of user tracebacks; test updated.
  • Stale docstring path in mcp/mixin.py repointed to the wheel.
  • Doc import examples — swept all ~23 docs referencing gaia.agents.chat to the correct targets (gaia_agent_chat for the agent/session/bundles, gaia.agents.tools for the promoted mixins, hub/agents/python/chat/ for source links), including a long-stale PathValidator import.

Post-rebase drift the original branch never saw (new main touch-points added after the original commit): the scheduled-run executor (ui/server.py), the dynamic-tools toggle (ui/routers/system.py), and eval/tool_cost.py imported ChatAgent from the now-deleted module — all repointed to gaia_agent_chat with the same lazy-import + install-hint pattern, and the affected test modules guarded with importorskip.

Verified: registry discovers chat/doc/file via entry points; the Agent UI backend boots and serves all three via /api/agents; affected unit suites green locally. No ChatAgent prompt/tool/model wiring changed (pure relocation), so the eval gate doesn't apply, and the #696 GaiaAgent rename has not landed — no collision.

@kovtcharov kovtcharov force-pushed the claudia/task-247f4a89 branch from 3442b53 to 81a4b15 Compare June 26, 2026 06:27
kovtcharov added a commit that referenced this pull request Jun 26, 2026
Resolves the actionable items from the PR #1456 bot review:

- agent_loop.py + the two _chat_helpers lazy-import sites translated a missing
  gaia-agent-chat wheel into a swallowed ImportError / opaque 500; they now
  raise an actionable RuntimeError naming the install command, matching the CLI.
- verify_path_validator.py / verify_shell_security.py guarded their bare
  gaia_agent_chat import with a SystemExit install hint (importorskip is
  unavailable in standalone scripts).
- FRAMEWORK_PATHS dropped the now-dead gaia/agents/chat substring and filters
  the hub package generically (hub/agents/python/ + gaia_agent_chat) so
  chat-agent frames stay out of user-facing tracebacks; test updated to match.
@github-actions github-actions Bot added the security Security-sensitive changes label Jun 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🟡 Several GitHub source-code links in the updated docs will 404 — they use a src/hub/… prefix that doesn't exist in the repo. The wheel landed at hub/agents/python/chat/, not src/hub/agents/python/chat/.

Affected files: docs/plans/tool-loader.mdx (4 links), docs/spec/chat-agent.mdx (the <Info> block link), and docs/playbooks/chat-agent/part-3-deployment.mdx (the ChatAgent Card href). Every link should drop the leading src/.

🔍 Technical details

docs/plans/tool-loader.mdx — four occurrences of:

https://github.com/amd/gaia/blob/main/src/hub/agents/python/chat/gaia_agent_chat/tool_bundles.py

should be:

https://github.com/amd/gaia/blob/main/hub/agents/python/chat/gaia_agent_chat/tool_bundles.py

docs/spec/chat-agent.mdx — the <Info> Source Code href:

https://github.com/amd/gaia/blob/main/src/hub/agents/python/chat/gaia_agent_chat/agent.py

docs/playbooks/chat-agent/part-3-deployment.mdx — the ChatAgent Card:

href="https://github.com/amd/gaia/blob/main/src/hub/agents/python/chat/gaia_agent_chat/agent.py"

Quick fix — in all three files, replace blob/main/src/hub/ with blob/main/hub/.

docs/reference/agent-core-loop-architecture.md, docs/spec/file-change-handler.mdx, docs/spec/browser-tools.mdx and the plan docs use src/hub/… only in plain text / comments, not as hyperlinks, so those are cosmetically inaccurate but don't 404.

kovtcharov
kovtcharov previously approved these changes Jun 26, 2026
@kovtcharov

Copy link
Copy Markdown
Collaborator

Fixed in a4fe24a — good catch, this was a sweep bug on my end (the rewrite of src/gaia/agents/chat/ → the hub path also fired on lines carrying a src/ prefix, yielding the bogus src/hub/...).

Dropped the stray src/ from every occurrence repo-wide, not just the three hyperlinked ones — including the plain-text/comment references in agent-core-loop-architecture.md, file-change-handler.mdx, browser-tools.mdx, and the plan docs you noted were only cosmetically inaccurate.

Verified:

  • git grep -nE "src/hub/" → no matches anywhere in the repo.
  • python util/check_doc_links.py --internal-only949 OK, Broken: 0.
  • The target file exists at the linked path (hub/agents/python/chat/gaia_agent_chat/agent.py), so the links resolve once this merges — same convention as the sibling hub-agent docs.

kovtcharov
kovtcharov previously approved these changes Jun 26, 2026
Ovtcharov and others added 6 commits June 26, 2026 14:26
ChatAgent — the flagship conversational agent backing the chat/doc/file
profiles — was the last family of registry builtins still hardcoded in the
core framework wheel. It now ships as the standalone gaia-agent-chat wheel
under hub/agents/python/chat/, exposing chat/doc/file as three gaia.agent
entry points discovered by the registry. The core wheel no longer hardcodes
them and the agent versions independently. `gaia chat` (incl. --ui) resolves
through the registry and fails loudly with an install hint when the wheel is
absent. Full+lite model tiers and the per-profile prompt/tool wiring are
preserved exactly via registry.build_model_tiers.

Continues the #1102 hub restructure after analyst/browser (#1446).
…t wheel

The original chat→hub migration (#1102) was authored 122 commits back. Main
has since grown new ChatAgent touch-points the replayed commit never saw:
the scheduled-run executor and the dynamic-tools UI toggle imported ChatAgent
from the now-deleted gaia.agents.chat, and tool_cost plus several tool-loader
tests imported tool_bundles/ChatAgentConfig from the same dead path — every one
would ImportError the moment the wheel landed.

Repoint them at gaia_agent_chat (matching the migration's own lazy-import +
install-hint pattern), guard the affected test modules with
pytest.importorskip("gaia_agent_chat") so framework-only envs skip rather than
error, and move tool_bundles into the hub package alongside session.
Resolves the actionable items from the PR #1456 bot review:

- agent_loop.py + the two _chat_helpers lazy-import sites translated a missing
  gaia-agent-chat wheel into a swallowed ImportError / opaque 500; they now
  raise an actionable RuntimeError naming the install command, matching the CLI.
- verify_path_validator.py / verify_shell_security.py guarded their bare
  gaia_agent_chat import with a SystemExit install hint (importorskip is
  unavailable in standalone scripts).
- FRAMEWORK_PATHS dropped the now-dead gaia/agents/chat substring and filters
  the hub package generically (hub/agents/python/ + gaia_agent_chat) so
  chat-agent frames stay out of user-facing tracebacks; test updated to match.
…1102)

The chat→hub migration moved ChatAgent's runnable examples out from under
gaia.agents.chat. Repoint every doc reference so copy-paste examples resolve:
agent/session/tool_bundles/lite_agent → gaia_agent_chat; the promoted
FileToolsMixin/RAGToolsMixin/ShellToolsMixin → gaia.agents.tools; source-code
links and file paths → hub/agents/python/chat/. Also corrects a long-stale
PathValidator import (gaia.agents.chat.path_validator → gaia.security).
…ests

CI's framework-only jobs (Unit Tests on `.[api]`, Security Tests on `.[dev,rag]`)
run without gaia-agent-chat, which surfaced regressions the wheel-present local
run masked:

- The dynamic-tools UI toggle 500'd without the wheel: `dynamic_tools_env_override`
  had moved into the chat wheel but the core UI settings router imports it. Moved
  the helper to the core `tool_loader` (its feature home) and re-export it from
  the wheel's agent.py — the router no longer depends on the wheel, one truthy
  set preserved.
- `test_chat_dynamic_tools` stubbed faiss/sentence_transformers as MagicMocks
  BEFORE the `importorskip`, so a skip (no wheel) leaked the stubs into the run
  and broke later memory/code-index tests. Moved the importorskip above the stub
  block so a skip leaves sys.modules untouched.
- Guarded `test_dynamic_tool_filtering` (transitively builds a ChatAgent via
  `build_doc_agent_skeleton`) with importorskip.
- Updated stale hub tests for the migration: chat/doc are no longer reserved ids
  or builtins, so `test_reserved_id_rejected` keeps only `builder`,
  `test_uninstall_builtin_refused` targets `builder`, and the agent-hub-metadata
  tests run full discovery + importorskip for the now-installed chat/doc agents.
- test_security.yml installs the chat wheel (the verify_*.py guardrail scripts
  instantiate ChatAgent).
The earlier doc sweep rewrote `src/gaia/agents/chat/` → the hub path on lines
that carried a `src/` prefix, producing `src/hub/agents/python/chat/...` — a
path that does not exist in the repo, so the GitHub source-code links 404.
Drop the stray `src/` so they point at the real `hub/agents/python/chat/...`
tree. Verified: no `src/hub/` left in the repo, check_doc_links --internal-only
reports 0 broken (949 OK), and the target file exists at the linked path.
@github-actions

Copy link
Copy Markdown
Contributor

🟡 The rename from "builtin:chat""installed:chat" was not carried through the connector docs and CLI help text. Every connector guide uses builtin:chat as the canonical example agent ID (grants, activations, revocations). After this PR those commands silently target a dead ID — and any user who ran them before upgrading finds their MCP server activations silently dropped.

Files that were missed and need the same builtin:chatinstalled:chat update:

  • src/gaia/connectors/cli.py (two --help strings, lines 140 and 186)
  • docs/sdk/infrastructure/connectors.mdx (seven occurrences, including the note "builtin:chat is the canonical example")
  • docs/connectors/index.mdx, google.mdx, github.mdx, microsoft.mdx, tavily.mdx
  • docs/reference/cli.mdx

Additionally, tests/unit/agents/test_tool_visibility_filter.py uses "builtin:chat" throughout its activation-filter tests — those tests still pass (they're self-consistent) but now document the wrong production ID.

🔍 Technical details

_chat_helpers.py and the new gaia_agent_chat/__init__.py correctly stamp "installed:chat", and _discover_installed_agents at src/gaia/agents/registry.py:700 coerces the id to f"installed:{registration.id}". But the external-facing references were not updated:

src/gaia/connectors/cli.py:140:  help="Namespaced agent id, e.g. 'builtin:chat' or ..."
src/gaia/connectors/cli.py:186:  help="Namespaced agent id, e.g. 'builtin:chat' or ..."
docs/sdk/infrastructure/connectors.mdx:
  gaia connectors grants grant google builtin:chat \
  gaia connectors activations activate mcp-github builtin:chat \
  activate("mcp-github", "builtin:chat", ...)
  # and 4 more occurrences

Any user who runs gaia connectors activations activate mcp-github builtin:chat is writing to the old key; MCPClientManager.servers_for_agent is called with "installed:chat", finds no matching entries, and the agent falls back to no MCP servers visible. The CLAUDE.md rule ("A functional change must update EVERY doc that describes it") applies directly here.

@kovtcharov kovtcharov added this pull request to the merge queue Jun 26, 2026
Merged via the queue into main with commit 390361e Jun 26, 2026
61 of 62 checks passed
@kovtcharov kovtcharov deleted the claudia/task-247f4a89 branch June 26, 2026 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents cli CLI changes dependencies Dependency updates devops DevOps/infrastructure changes documentation Documentation changes eval Evaluation framework changes mcp MCP integration changes performance Performance-critical changes rag RAG system changes security Security-sensitive changes tests Test changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants