Skip to content

docs(agents): clear stale YAML-manifest references after #914#918

Merged
itomek merged 2 commits intomainfrom
claude/nice-bouman-06340f
Apr 28, 2026
Merged

docs(agents): clear stale YAML-manifest references after #914#918
itomek merged 2 commits intomainfrom
claude/nice-bouman-06340f

Conversation

@itomek
Copy link
Copy Markdown
Collaborator

@itomek itomek commented Apr 27, 2026

Summary

Follow-up to #914. The github-actions review-bot left a batch of doc-cleanup suggestions that landed after merge — onboarding files (CLAUDE.md, .github/workflows/claude.yml, .claude/agents/*.md) and a couple of SDK doc subsections still told contributors to register agents via YAML manifests, directly contradicting the Python-only registry shipped in #914. Same drift class the TestRemovedSymbols regression guard exists to prevent — it just didn't extend to prose docs and .claude/agents/.

Also folds in the two silent-drop polish items the bot flagged: companion YAML models: filtering and export_custom_agents skipping legacy YAML-only directories.

Threads

  • Onboarding/agent-doc cleanupCLAUDE.md (4 spots), .github/workflows/claude.yml (review-bot guidance), .claude/agents/{gaia-agent-builder,sdk-architect,code-reviewer,architecture-reviewer,rag-specialist}.md. Why: contributors and the gaia-agent-builder Claude agent read these as authoritative — leaving them stale would steer new agents back into the deleted YAML path.
  • SDK doc subsections — drops the dead "Opting in from a YAML-manifest agent" snippet in docs/sdk/sdks/code-index.mdx and rewrites the equivalent line in docs/guides/code-index.mdx to describe Python-class composition. Why: example showed a YAML format that no longer loads.
  • Linter docstringutil/check_agent_conventions.py listed "Manifest JSON Schema is not stale" under hard checks, but the corresponding _check_manifest_schema() was deleted in refactor(agents): remove YAML manifest agent support (#912) #914. Drop the stale bullet.
  • Loud-failure polishregistry.py now warns when companion YAML models: contains non-string entries instead of silently filtering them; export_import.py emits one log.warning per skipped YAML-only directory during export so users get the same migration signal the discovery path already gives. Why: matches the "no silent fallback / actionable error" rule from CLAUDE.md.

Two new test cases guard the warnings.

Test plan

  • python -m pytest tests/unit/agents/test_registry.py tests/unit/test_export_import.py -xvs — 46 passed
  • python util/check_agent_conventions.py — 0 errors
  • python util/lint.py --black --isort — pass
  • grep -rn "YAML-manifest agents can|or YAML manifest|registered via YAML manifest|AgentManifest" CLAUDE.md .claude/ .github/ docs/sdk/ docs/guides/code-index.mdx util/ — zero hits (migration guide and release notes intentionally retained)

Refs #914.

Address review-bot suggestions on #914 that landed after merge: onboarding
docs (CLAUDE.md, .github/workflows/claude.yml, .claude/agents/*.md) and SDK
guides still recommended registering agents via YAML manifests, contradicting
the Python-only registry shipped in #914.

Also clears the two silent-drop polish items the bot flagged:
- registry.py warns when companion YAML `models:` list contains non-string
  entries instead of filtering them silently.
- export_import.py warns once per legacy YAML-only directory it skips during
  export, matching the DeprecationWarning the discovery path already emits.

Drops the obsolete "Manifest JSON Schema is not stale" bullet from the
agent-conventions linter docstring (the corresponding check was removed in
#914) and deletes the dead "Opting in from a YAML-manifest agent" subsection
in docs/sdk/sdks/code-index.mdx.

Test plan:
- pytest tests/unit/agents/test_registry.py tests/unit/test_export_import.py: 46 passed
- python util/check_agent_conventions.py: 0 errors
- python util/lint.py --black --isort: pass
@itomek itomek requested a review from kovtcharov-amd as a code owner April 27, 2026 22:44
@github-actions github-actions Bot added documentation Documentation changes devops DevOps/infrastructure changes tests Test changes agents labels Apr 27, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Summary

Tight, well-scoped follow-up to #914. The PR sweeps stale YAML-manifest guidance out of contributor-facing prose (CLAUDE.md, .claude/agents/*.md, .github/workflows/claude.yml, two docs/.../code-index.mdx pages, the check_agent_conventions.py docstring) and folds in two small loud-failure fixes flagged by the review bot (registry.py warns on non-string models: entries; export_custom_agents logs a per-directory warning when skipping legacy YAML-only dirs). Both behavior changes have new test coverage, and a post-PR grep across the touched scope returns zero stale hits — exactly the drift class the existing TestRemovedSymbols regression guard prevents in code, now extended to docs.

Issues

None blocking. A couple of minor observations worth a glance, neither of which justifies churn on its own:

🟢 Logged repr of bad could be noisy for unusual payloads (src/gaia/agents/registry.py:283-288)
The %r formatting of bad is fine for typical malformed YAML (ints, None, small dicts), but if a user pastes a giant nested structure the warning line could balloon. Not a real problem in practice — flagging only because bad is unbounded user-controlled YAML content. Safe to leave as is.

🟢 YAML-only export skip silently passes for non-dirs / dirs with neither agent.py nor agent.yaml (src/gaia/installer/export_import.py:147-157)
Confirmed correct: _is_custom_agent_dir requires is_dir() and agent.py, and the new elif requires is_dir() and agent.yaml. Stray files and empty dirs fall through cleanly with no spurious warning. No change needed — noting it because the loop's branching is easy to mis-read.

Strengths

  • The right fix at the right layer. The "no silent fallback" rule from CLAUDE.md is upheld in two more spots (registry models filtering and export skipping), each with a one-line warning that names the offender and points at https://amd-gaia.ai/guides/custom-agent — meets the "what failed / what to do / where to look" bar.
  • New tests assert the warning, not just the side-effect. test_companion_yaml_models_non_string_entries_warn and the updated test_export_skips_yaml_only_dirs both use caplog to verify the user-visible signal is emitted, including a v0.17.5 substring check that would catch a future copy-paste regression of the migration message.
  • PR description matches the diff. The four threads called out in the body are exactly what changed; the post-PR grep claim verifies (committed CLAUDE.md and .claude/agents/*.md are clean, with only intentional historical "removed in v0.17.5" callouts retained in gaia-agent-builder.md and claude.yml).
  • util/check_agent_conventions.py docstring sync (util/check_agent_conventions.py:17) — small, but the kind of stale internal doc bullet that quietly misleads contributors. Good catch.

Verdict

Approve. Ready to merge as-is. The two minor observations above are FYI only.

Two adversarial-review agents (verification + scope) flagged
`assert "v0.17.5" in message` as brittle: any future version bump or
message rewording breaks the test for no signal. Switch to substring
assertions on the *actionable* content of the warning ("yaml" and
"agent.py") so the test verifies the user gets a useful migration hint
without coupling to the version stamp.

Also cap the registry's non-string `models:` warning to the first 5
entries with an "(and N more)" suffix. A malformed companion YAML
with a large list would otherwise produce an unbounded log line via
`%r`. Bounded preview gives the same diagnostic information.

Test plan:
- pytest tests/unit/agents/test_registry.py tests/unit/test_export_import.py: 46 passed
- python util/lint.py --black --isort: pass
@github-actions
Copy link
Copy Markdown
Contributor

Summary

Tightly-scoped follow-up to #914 that finishes the YAML-manifest removal: 6 onboarding/doc files no longer steer contributors back into the deleted path, and two silent-drop spots (registry.py models: filtering, export_custom_agents legacy-dir skip) now emit actionable warnings — matching CLAUDE.md's "no silent fallbacks" rule. Test coverage was updated to assert content of the warnings (not just presence), which is the right shape. The PR description's verification step (grep -rn "YAML-manifest agents can\|or YAML manifest\|registered via YAML manifest\|AgentManifest" CLAUDE.md .claude/ .github/ docs/sdk/ docs/guides/code-index.mdx util/) was reproducible against the diff.

Issues Found

🟢 Minor

Two-pass list iteration in registry.py:281,295raw_models is walked twice (once to collect bad, once to filter models). On well-formed YAML this is cheap, but a single pass keeps intent visually grouped:

                    if isinstance(raw_models, list):
                        bad: List[object] = []
                        models = []
                        for m in raw_models:
                            (models if isinstance(m, str) else bad).append(m)
                        if bad:
                            preview = bad[:5]
                            suffix = (
                                f" (and {len(bad) - 5} more)" if len(bad) > 5 else ""
                            )
                            logger.warning(
                                "registry: companion YAML %s: 'models' contains "
                                "%d non-string entries — ignoring (sample: %r%s)",
                                yaml_file,
                                len(bad),
                                preview,
                                suffix,
                            )

Skip if you'd rather keep the diff minimal — the current form reads fine and the lists are tiny.

export_custom_agents skip path doesn't catch agent.yaml with a sibling agent.py (src/gaia/installer/export_import.py:147-157) — the elif only fires when the directory has agent.yaml and is not a custom-agent dir. That's intentional for the "no agent.py at all" case, but a directory containing both files exports silently with the stale YAML still inside. Probably out of scope for this PR (the loader already warns on manifest-style keys at load time), so just noting it — no change requested.

Strengths

  • Surgical doc sweep — every spot the github-actions bot flagged is addressed, and the verification grep in the PR body proves zero residual hits. Updating .github/workflows/claude.yml's review-bot prompt alongside the human-facing docs is the right call; otherwise the bot would keep recommending YAML for new PRs.
  • Tests assert what the warning says, not just that one firedtests/unit/test_export_import.py:526-530 checks the message contains both yaml and agent.py (so users get a migration cue), and tests/unit/agents/test_registry.py:311-313 checks for "non-string entries". This is exactly the kind of assertion that survives refactors without going stale, and it's the pattern the prior commit (9e9d9e34) was reinforcing.
  • util/check_agent_conventions.py docstring trimmed in lockstep — easy thing to miss after a deletion, and stale linter docstrings tend to outlive the code they describe.

Verdict

Approve — no blocking issues. The minor two-pass nit is cosmetic; everything else is well-scoped, well-tested, and consistent with the post-#914 architecture.

@itomek itomek self-assigned this Apr 28, 2026
@itomek itomek linked an issue Apr 28, 2026 that may be closed by this pull request
9 tasks
@itomek itomek enabled auto-merge April 28, 2026 00:40
@itomek itomek added this pull request to the merge queue Apr 28, 2026
Merged via the queue into main with commit 667fa5e Apr 28, 2026
30 checks passed
@itomek itomek deleted the claude/nice-bouman-06340f branch April 28, 2026 02:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents devops DevOps/infrastructure changes documentation Documentation changes tests Test changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor: Remove YAML manifest agent support; consolidate on Python custom agents

2 participants