feat(cli): parallel catalog validation and fleet rollup - #123
feat(cli): parallel catalog validation and fleet rollup#123mimran-khan wants to merge 8 commits into
Conversation
Emit a machine-readable fleet rollup at the reports root with per-skill status, optional severity totals from child JSON reports, and report paths. Create the output directory when needed so summary writes survive early skill failures. Fixes NVIDIA#120 Signed-off-by: mimran-khan <mohammed_imran.khan@outlook.com>
Catalog validate accepts --workers N to run skills in isolated child processes. Values above 1 skip the per-skill pipeline view and rebuild per-skill argv from the parent Click context or sys.argv. Fixes NVIDIA#122 Signed-off-by: mimran-khan <mohammed_imran.khan@outlook.com>
| """Return the newest per-skill machine-readable report when present.""" | ||
| if not skill_report_dir.is_dir(): | ||
| return None | ||
| candidates = sorted(skill_report_dir.glob("skillevaluator-output-*.json"), reverse=True) |
There was a problem hiding this comment.
[P1] Restrict report discovery to this run. Reusing -o leaves timestamped child JSON in place; if the current child fails before writing a report, this glob selects the old file and copies it into the current entry. Track each job’s newly produced report, or isolate/clean run outputs, before aggregating.
There was a problem hiding this comment.
Fixed: each worker tracks reports before and after the run and only aggregates newly written JSON for that job.
There was a problem hiding this comment.
This is still reachable when the current worker produces no JSON. json_report_name remains absent and the catalog entry falls back to the newest file in the reused per-skill directory, so I reproduced an old successful overall status and severity counts being attached to a current failure. Please pass only a report produced by this worker invocation, or no report, rather than searching historical files.
There was a problem hiding this comment.
Removed _latest_skill_json_report fallback; workers only aggregate JSON written for that run.
Rebuild child argv without dropping positional catalog paths, track fresh per-skill JSON reports instead of stale files, write catalog-summary.json atomically, and fix --include-skills forwarding for context fallback. Signed-off-by: mimran-khan <mohammed_imran.khan@outlook.com>
|
Worker argv, stale JSON, atomic summary, and --include-skills fixes are pushed. Ready for re-review. |
|
@mimran-khan : Re-reviewed at current head
The atomic summary writer and |
| if params.get("harbor_keep_jobs"): | ||
| argv.append("--harbor-keep-jobs") | ||
| for fmt in params.get("report_formats") or ("cli",): | ||
| if fmt == "cli": |
There was a problem hiding this comment.
[P2] Do not discard an explicitly selected CLI report here. With report_formats containing only cli, the fallback emits no -r option, so the child treats reporting as implicit and writes HTML plus JSON instead of honoring CLI-only output.
There was a problem hiding this comment.
Child argv is rebuilt from Click context now so --checks and --min-score values survive, and -r cli is forwarded when explicitly selected.
rng1995
left a comment
rng1995
left a comment
There was a problem hiding this comment.
Parallel catalog execution still loses real CLI option values, changes explicit CLI-only reporting, and can reuse stale per-skill JSON after a worker produces no report. Focused catalog tests and Ruff/diff checks passed; the single Python 3.12 CI failure is an unrelated flaky NVIDIA bridge socket test. Requesting changes for the reproducible worker-boundary defects in the review threads.
| continue | ||
| if arg.startswith("--workers=") or arg.startswith("--output-dir=") or arg.startswith("-o="): | ||
| continue | ||
| if not arg.startswith("-"): |
There was a problem hiding this comment.
[P1] This drops option values along with the catalog positional. A real --checks quality invocation becomes bare --checks, and --min-score 95 -r json loses both values, so every worker exits 2. The CliRunner tests only exercise the context fallback. Please rebuild from parsed Click parameters or preserve each option's arity instead of filtering every non-option token.
There was a problem hiding this comment.
Fixed argv reconstruction from Click context; option values are no longer dropped.
Rebuild child argv from Click params, preserve -r cli when selected, track per-run JSON in serial catalog mode, and stop attaching stale reports when no new JSON was produced this run. Signed-off-by: mimran-khan <mohammed_imran.khan@outlook.com>
# Conflicts: # CHANGELOG.md
3 participants
Fixes #122
Related: #120, #121
This branch includes two catalog features that stack cleanly:
catalog-summary.json([FEA]: Machine-readable catalog fleet rollup report #120 / feat(cli): write catalog-summary.json after catalog validate #121): fleet rollup after catalog validate--workers N([FEA]: Parallel catalog validation with --workers #122): parallel child-process validation for catalogs--workersdefaults to 1 (serial, same per-skill pipeline view as today). Above 1, skills validate in isolated processes with per-skill output dirs under-o. The parent Click context (orsys.argvwhen launched from the real CLI) is used to rebuild each childvalidateinvocation, so pytest and production entry points both work.Parallel mode skips the per-skill Rich pipeline view and prints a single fleet scoreboard at the end. Exit code stays nonzero if any skill failed.
If #121 lands first, I can rebase this to workers-only; happy to split either way.
Test plan
pytest tests/test_commands.py -k catalog--workers 1(default)--workers 2integration test on two fixture skills