Repository navigation
feat(skills): add the catalog entry skill for Hyperloom bootstrap - #1515
Conversation
amd/skills now federates every catalog skill from the product repo that owns it, so `hyperloom-workload-optimizer` has to live here and the catalog vendors a copy nightly. What it holds is only the bootstrap: confirm the workspace, install the wheel, run `/hyperloom-setup`, then hand the run to the skill that owns it. Everything after setup already ships with the runtime -- `hyperloom-setup` for credentials and run mode, the demo skills for a workload preset, `inference_optimizer` for the launcher gates, resume and monitoring -- so they stay in step with the installed version by construction. This is the agent-facing form of examples/README.md, which stays as the human quickstart. The catalog copy of this skill was written against an empty workspace and carries its own launch, resume and GPU-preflight scripts. Those are not imported: a second launch path in the product repo would drift from the CLI it wraps. The skill says so explicitly rather than leaving it to the reader. No packaging change. The entry point earns its keep before the wheel is installed, so shipping it in the wheel would only overwrite the copy the user installed from the catalog.
… hands off to The skill's whole job is to reach the demo skills in examples/, and its prose is the agent-facing form of examples/README.md, so it reads better next to both than at the repository root. It stays out of pyproject's data-files on purpose, unlike the four demo skills one level up: this entry point is what a user follows before the wheel exists, so shipping it would only overwrite the copy they installed from the catalog.
The run skills report the plan before starting; the walkthrough in amd/skills already promises the user is asked, and an unattended run that holds the GPU for hours should not begin on a plan nobody accepted.
…ates amd/skills imports this folder nightly and validates it there, so until now a broken edit here would surface as a red bot pull request in that repo, where nobody on this side is watching. Nothing in this repo would have caught it either: the skill is markdown, and lint.yml and tests-coverage.yml both ignore **/*.md, so the one file that ships to the catalog was the one file no job read. The new workflow carries no paths-ignore for exactly the reason packaging.yml carries none. The four assertions are the rules the catalog enforces, and the description is the one with no room left: at 943 of 1024 characters, a single added trigger sentence takes it to 1098. Each assertion was checked against the break it exists for -- an over-long description, a name that no longer matches the directory, and a moved folder, which is the case that also needs a federation.json pull request upstream.
The workflow and its test are named for the relationship they guard -- this folder is federated out of here -- rather than for the catalog on the other end of it. Both keep their own name rather than #1482's `AMD Skills Checks`, which belongs to a workflow that really does call the catalog's skillscope harness. This one asserts four rules itself, so borrowing that name would show a green check for a harness that never ran.
The rename commit carried the git mv but not the edits inside the file, so the workflow still named itself Catalog skill and ran a test path that no longer existed.
72b2e55 to
d9a3e58
Compare
What this PR doesAdds BlockingThe PR description is out of sync with the diff. Three points:
Please update the description (and the title, if the CI gate is meant to stay in this PR). What I checked
|
xiaofei-zheng
left a comment
xiaofei-zheng
left a comment
There was a problem hiding this comment.
Approving. The code itself is sound — the contract test passes locally on the PR head (4 passed), the new workflow is correctly scoped, and the four run skills named in the handoff all have matching [tool.setuptools.data-files] entries.
Please still fix the description before merge as noted in my earlier comment: it does not mention the new CI workflow or contract test, the "Markdown-only change, so Lint and Tests with Coverage are skipped" claim is false (both ran on this PR), and the Placement rationale describes the opposite of the actual path.
2 participants
Description
amd/skillsnow federates every catalog skill from the product repo that owns it: the skill lives here, and the catalog imports a copy nightly. This addshyperloom-workload-optimizerto this repo so that import has a source, plus the check that keeps it importable.This is an alternative to #1482, which copies the catalog's current tree into this repo. Please take this one instead and close that.
Registering the path is amd/skills#213, which also brings the catalog copy in step with this one. Merge this PR first — the catalog's nightly importer clones
mainand raisesFileNotFoundErroron a path that is not there yet, and its own PR check reads the schema without cloning, so it would go green either way.What this PR adds
examples/skills/hyperloom-workload-optimizer/SKILL.mdscripts/tests/test_federated_skill_contract.py.github/workflows/federated-skill.ymlThe skill
Only the bootstrap: confirm the workspace,
pip install --target ., run/hyperloom-setup, then hand the run to the skill that owns it.Everything after setup already ships with the runtime, so it stays in step with the installed version by construction:
hyperloom-setup— credentials,USER_DATA_PATH, run mode, Docker target host, bare-metal framework installinference_optimizer— launcher gates, resume, monitoringIt is the agent-facing form of
examples/README.md, which stays as the human quickstart.Placement
examples/skills/hyperloom-workload-optimizer/, beside the README it mirrors and one level up from the demo skills it hands off to.Unlike those four demo skills, it has no
data-filesentry inpyproject.toml, and that is deliberate rather than an omission: a user follows this skill before the wheel exists, so shipping it in the wheel would only overwrite the copy they installed from the catalog.What it deliberately leaves out
The catalog copy was written for a workspace with no Hyperloom in it, so it carries its own
launch.sh,resume.sh,preflight.pyand their tests. Importing those would give this repo a second launch path that drifts from the CLI it wraps, so they are not here, and the skill says so rather than leaving it to the reader.Also out:
skill-card.md(the catalog synthesizes one on import, as it does for the federated TraceLens skill) andevals/(federation does not carry it in either direction — the datasets stay inamd/skills).The check, and why this repo needs one
The catalog validates the skill on its side, so without a check here a broken edit lands as a red bot pull request over there, where nobody on this side is watching.
Nothing already in this repo would have caught it either: the skill is markdown, and
lint.ymlandtests-coverage.ymlboth ignore**/*.md, so the one file that ships to the catalog was the one file no job read.federated-skill.ymltherefore carries nopaths-ignore, for the same reasonpackaging.ymlcarries none, and the job is seconds long.It asserts what the catalog enforces: the frontmatter parses,
namematches the directory, the description fits 1024 characters, and the body fits 500 lines. The description is the one with no room left — at 943 characters, a single added trigger sentence takes it to 1098.The catalog's own
skillscopeharness is not reused here: its discovery step only treats a directory as a skill once anevals/evals.jsonsits besideSKILL.md, and federation deliberately leaves that dataset inamd/skills. Naming this workflow after the catalog's checks would also show a green tick for a harness that never ran, which is why it is named for the relationship it guards instead.Notes
descriptionis kept byte-identical to the catalog's, because the catalog's routing cases are graded against it.SKILL.mdmatches the copy in Federatehyperloom-workload-optimizerfrom AMD-AGI/Hyperloom amd/skills#213 apart from one line: the link out toexamples/README.mdis relative here and absolute there, which is the rewrite the importer performs.Test plan
test_federated_skill_contract.py: 4 passed, and each assertion was checked against the break it exists for — an over-long description (fails at 1098 characters), anamethat no longer matches the directory, and a moved foldertest_packaging_lint.pypasses (7 passed); no asset undersrc/changed, so nothing new to declareruff format --checkandruff checkclean on the new testexamples/README.mdresolves from the skill's locationnamematches the directory, description 943/1024 characters, body 127/500 linesmain; 30 checks pass, the one skip isupdate test durations (main), which only runs onmain