Skip to content

feat(cli): write catalog-summary.json after catalog validate - #121

Open
mimran-khan wants to merge 3 commits into
NVIDIA:mainfrom
mimran-khan:feat/catalog-fleet-rollup
Open

feat(cli): write catalog-summary.json after catalog validate#121
mimran-khan wants to merge 3 commits into
NVIDIA:mainfrom
mimran-khan:feat/catalog-fleet-rollup

Conversation

@mimran-khan

Copy link
Copy Markdown
Contributor

Fixes #120

Catalog validate already runs every skill and prints a Rich scoreboard, but CI had no single JSON file to gate on. This writes catalog-summary.json at the reports root after the catalog loop finishes.

Each skill entry includes pass/fail, a failure reason when applicable, and optional fields pulled from the newest per-skill JSON report (overall_status, severity_counts, incomplete scanners). Severity totals are aggregated across skills when that data exists.

The writer creates the output directory if needed so an early skill crash (before per-skill reports land) still produces the fleet rollup and the scoreboard.

Test plan

  • pytest tests/test_commands.py -k catalog

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>
Comment thread src/skillevaluator/cli.py Outdated
"""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)

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.

[P1] Please restrict report discovery to JSON written by this validation run. Reusing the default reports directory after a failed or CLI-only run currently attaches an older report: I reproduced a current passed:false entry carrying stale overall_passed:true and high:7 counts.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed: serial catalog tracks per-run json_report_name and skips stale JSON when the current run did not write a report.

Comment thread src/skillevaluator/cli.py Outdated
}
output_dir.mkdir(parents=True, exist_ok=True)
output_path = output_dir / CATALOG_SUMMARY_FILENAME
output_path.write_text(json.dumps(summary, indent=2, default=str, allow_nan=False), encoding="utf-8")

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.

[P1] Use the checked atomic report writer here. Path.write_text follows a pre-existing catalog-summary.json symlink; I reproduced it truncating an external sentinel, whereas the other report paths reject symlink and reparse destinations.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

catalog-summary.json now goes through _write_report_atomically.

@rng1995 rng1995 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.

The fleet summary currently can attribute stale report data to a failed current run and writes through a linked destination instead of using the hardened atomic report path. The focused catalog tests passed (4 tests), with Ruff and diff checks clean. Requesting changes for the two reproducible current-run provenance and output-safety issues noted inline.

Track JSON reports per serial catalog run, skip stale report attribution when
no new JSON was written, and write catalog-summary.json through the atomic
report writer.

Signed-off-by: mimran-khan <mohammed_imran.khan@outlook.com>
# Conflicts:
#	CHANGELOG.md
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.

[FEA]: Machine-readable catalog fleet rollup report

2 participants