Skip to content

Federate flydsl-kernel-trace-analysis from ROCm/FlyDSL - #274

Merged
danielholanda merged 7 commits into
mainfrom
federate-flydsl-kernel-trace-analysis
Oct 26, 2026
Merged

danielholanda merged 7 commits into
mainfrom
federate-flydsl-kernel-trace-analysis

Conversation

@johnl-amd

@johnl-amd johnl-amd commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Vendors flydsl-kernel-trace-analysis from ROCm/FlyDSL, adding it to the federation source that #273 introduces.

Upstream the skill folder is kernel-trace-analysis, so the entry carries as: flydsl-kernel-trace-analysis to keep the catalog name Felix used in #267 and #270. The importer rewrites the name: field in the vendored copy to match.

This supersedes #267 and #270, which were closed pending FlyDSL #1304 and #1305. Both merged on Oct 18, so the upstream skill now carries its own skill card, evals and offline fixtures. Original work is Felix Li's.

Stacked on #273 and based on its branch, since both skills share one ROCm/FlyDSL block in federation.json. Draft until that merges, then I'll rebase onto main.

One follow-up for the FlyDSL side rather than here: the skill description ends with Usage: /kernel-trace-analysis <cmd>, which is the upstream name. Installed from the catalog the skill is flydsl-kernel-trace-analysis, so that line will point users at a command they don't have. Worth fixing upstream, where the nightly import will pick it up.

The README catalog row is deliberately not here. Every row links to blob/main/skills/<name>/SKILL.md, which 404s on the external-reference check until the skill is actually on main. Same split as #248 then #255: the row follows in a separate PR once this merges.

Behavioral evals are failing, and it looks like a skill issue rather than a flake

Two runs, both 12/13, but failing a different expectation each time, and both of the same kind: the agent did not invoke the bundled script as prescribed.

Run flydsl-trace-offline-hotspots flydsl-trace-offline-pmc
1 FAIL 6/7, split hotspot_analyzer.py into two invocations instead of one combining --kernel and source detail PASS 6/6
2 PASS 7/7 FAIL 5/6, computed the result with an inline Python heredoc and never ran scripts/pmc_l2_analyzer.py with the required flags

So SKILL.md does not pin the agent to running the bundled scripts with the given flags, and roughly one expectation of that class misses per run, landing on whichever case rolls badly. Worth tightening upstream in FlyDSL, where the nightly import will pick it up, rather than patching the vendored copy here.

For contrast, #273 went green on a re-run, so the authoring skill's single miss does look like judge nondeterminism.

Testing

  • federate_skills.py --only flydsl-kernel-trace-analysis: vendors at 6245f9b.
  • publish.sh then check.sh: 0 errors, 13 skills, Cursor and Codex manifests up to date.

@johnl-amd
johnl-amd force-pushed the federate-flydsl-kernel-authoring branch from bbe3ff7 to 420856a Compare October 19, 2026 08:40
@johnl-amd
johnl-amd force-pushed the federate-flydsl-kernel-trace-analysis branch from 5f5a38e to cfe5c34 Compare October 19, 2026 08:40
johnl-amd and others added 2 commits October 26, 2026 11:39
Picks up ROCm/FlyDSL#1322, which tells the skill to run the bundled
pmc_l2_analyzer.py rather than recompute its metrics inline. That was
the one behavioral expectation failing here (12/13); upstream scored
13/13 on three consecutive runs with the fix.
@danielholanda
danielholanda marked this pull request as ready for review October 26, 2026 21:47
@danielholanda
danielholanda changed the base branch from federate-flydsl-kernel-authoring to main October 26, 2026 22:10
…l-trace-analysis

Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	.github/federation.json

@danielholanda danielholanda left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and passing all tests!

@danielholanda
danielholanda merged commit bdd53a5 into main Oct 26, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants