Repository navigation
feat(skills): Add critical-path analysis to the user plugin - #13
Merged
Merged
Conversation
The user plugin reads a PyPTO program at two altitudes: the IR trace covers lowering, and the in-core profile covers one kernel's instruction stream. Neither answers why a whole run took as long as it did. That analysis already ships as a wheel CLI, so what is missing is not the computation but the contract around it — which artifacts it needs, which of its own outputs prove the result is sound, and what each number licenses a reader to conclude. Add `critical-path-analysis` to the user plugin. It resolves the rank directories under a run tree, invokes the tool, and gates interpretation on evidence the tool itself emits: the compute-plus-stall tiling check must read `exact`, kernel families must carry resolved names rather than `cid<N>` placeholders, and the capture's reconciliation lines must report no dropped records. Only past those gates does it classify a run as dependency-bound, resource-bound, or compute-bound, and each classification names the stall kind that supports it, so `data-wait` sends the reader to the upstream producer, `core-wait` to resource serialization, and `front-gap` to launch cost rather than to any kernel. Two operational facts decide whether those numbers mean anything, and neither is visible from the tool's help. Split capture — dependency graph in one run, timing in another — leaves the two artifacts in different directories, and the critical-path tool has no flag to join across them, so the graph must be copied into the measured run's directory first. And with several rounds, records are collected on the first round only, so the analyzed makespan carries warm-up cost and is not a steady-state number. Introduce `plugins/pypto-user/lib/dfx/capture.md` to own the capture rules, following the developer plugin's `lib/github` pattern. Capture is shared by every analysis over the same artifacts, so a second analysis skill reading the same graph consumes that contract instead of restating it. Extend `DEPLOYABLE_ROOTS` so the banned-text portability guard covers the user plugin's `lib` as well as its skills. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Walking the skill against a real chip-swimlane run, rather than against the tool's own contract tests, invalidated the capture gate the analysis depends on. The gate told a reader to grep the run log for the collector's reconciliation line and to treat `counts match` as proof that the capture was complete. That line is emitted at info level and the runner's default level is quieter, so on a run that was in fact clean the grep returned nothing. A gate whose passing signal is silence fails open: a capture that dropped records reads exactly like one that did not, and every percentage computed from it inherits the error. State what the run actually prints. Positive confirmation needs the log level raised, and it arrives as one line per collector pool — PERF, SCHED_PHASE, ORCH_PHASE — plus a separate dep_gen reconciliation carrying an `overflow` counter the gate never mentioned. A pool missing from that list is not a pass. Failures are warnings and stay visible at the default level, which is why they were the half of the contract that was already right. Two further corrections from the same walk. A level-1 capture carries no AICPU task records at all and still produces a complete critical-path report, so capturing at the lowest useful level is now a measured recommendation rather than an inferred one. And two captures of one unchanged case differed by several points of stall share, so interpretation now treats a single capture as a single sample and refuses to compare configurations from one capture each. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Exercising the split-capture bridge showed the skill misreading the tool's own error. A directory holding timing records and a name map but no dependency graph exits 2 with a message naming the chip-swimlane flag, and the skill repeated that attribution as "capture was off for that run". Following it after a split capture sends the reader to re-run the measurement they already have, when the missing artifact is the graph the other run produced. Say that any one of the three absent artifacts produces that message, and that after a split capture the missing one is the graph. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Hzfengsy
approved these changes
Aug 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The user plugin reads a PyPTO program at two altitudes — IR lowering, and one
kernel's in-core instruction stream — with nothing that explains a whole run's
makespan.
critical-path-analysisadds that level. It resolves the rankdirectories under a run tree, invokes the wheel's critical-path tool, and gates
every conclusion on evidence the tool itself emits before classifying the run
as dependency-bound, resource-bound, or compute-bound. A reader comes away with
a bounding classification and the stall kind that supports it, or with a named
validation failure — not with a percentage of unknown provenance.
Changes
plugins/pypto-user/skills/critical-path-analysis/: the skill. Artifactresolution over both run layouts, invocation and the meaning of each exit
status, four validation gates (the compute-plus-stall tiling check must read
exact; kernel families must have resolved names rather thancid<N>;capture reconciliation must report no dropped records; every rank must
appear), and an interpretation table that maps each signal to where to act —
data-waitto the upstream producer,core-waitto resource serialization,front-gapto launch cost rather than to any kernel.plugins/pypto-user/lib/dfx/capture.md: the capture contract, shared byevery analysis over the same artifacts and following the developer plugin's
lib/githubpattern. It records the two facts that decide whether thenumbers mean anything: split capture leaves the dependency graph and the
timing in different directories while the tool has no flag to join across
them, and a multi-round run collects on the first round only, so the analyzed
makespan carries warm-up cost.
plugins/pypto-user/.codex-plugin/plugin.json: the long description nowcovers the fourth workflow. No version bump — that belongs to the release
workflow, from a clean tree on the default branch.
tests/: register the skill inEXPECTED_USER_SKILLS, and extendDEPLOYABLE_ROOTSso the banned-text portability guard covers the userplugin's
libas well as its skills.No behavior changes to any existing skill, and no migration step for anyone who
has the plugin installed; the new skill is additive.
Verification
python -m unittest discover -s tests: 232 tests, OK. Run inside the pushtransaction at each pushed commit, together with
ruff check tests,ruff format --check tests, andpyright— all clean, 0 errors.64-consumer explicit-fanin scene test, captured with
--enable-dep-gen --enable-chip-swimlane, then analyzed by following the skill literally: thepaired capture produced the artifact triple plus a merged trace; the analysis
exited 0 over one rank, wrote its report beside the records, emitted both
Perfetto traces, and passed the gates — 128 tasks, 4096 happens-before edges,
tiling check ... (exact), resolved kernel names. It classified the run asresource-bound on the evidence the skill asks for: a static CPM path at 11.6%
of the makespan with 37.7% core-wait stall.
reconciliation check as first written returned nothing on a demonstrably
clean capture, because the confirming line is info-level. See that commit
message for what the run actually prints.
while still producing a complete report, so the lowest-level recommendation
is measured.
python -m simpler_setup.tools.critical_patherror paths, each confirmingwhat the skill states:
--reportgiven a path exits 2; a missing root exits2; a directory without the artifact triple exits 2 naming the capture flag; a
missing merged trace warns and still exits 0; an incomplete name map renders
families as
cid<N>.