Feat/training report metadata - #992
Merged
Merged
Conversation
blugassi
requested review from
jj10306,
podkidyshev,
rutayan-nv and
srivatsankrishnan
as code owners
August 10, 2026 11:41
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughChangesThe PR adds scenario configuration provenance, expands the training report schema and parser output, replaces the training report strategy with a multi-run reporter, and updates registration, tests, and documentation. Training report pipeline
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cloudai/report_generator/training/parser.py (1)
97-102: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle unreadable configuration artifacts.
get_model_configcan raiseOSErrororUnicodeDecodeErrorafter the file-existence check. These exceptions escapecan_parse.TrainingReporter.generatecallscan_parsebefore itstryblock, so one unreadable report artifact stops processing of later runs instead of logging a skip warning.Proposed fix
- except (json.JSONDecodeError, yaml.YAMLError) as exc: + except (OSError, UnicodeDecodeError, json.JSONDecodeError, yaml.YAMLError) as exc: logging.warning(f"{name}: invalid config artifact at '{config_path}' ({exc}); skipping training report") return False🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cloudai/report_generator/training/parser.py` around lines 97 - 102, Update can_parse around the get_model_config call to also catch OSError and UnicodeDecodeError, logging the same skip warning used for malformed JSON/YAML artifacts. Ensure unreadable configuration files return the non-parseable result so TrainingReporter.generate can continue processing subsequent runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/cloudai/report_generator/training/reporter.py`:
- Around line 42-44: In the parser selection flow, add a warning log immediately
before the continue when parser_cls is None, identifying the skipped unsupported
workload using tr.test.test_template_name. Keep the existing skip behavior
unchanged.
---
Outside diff comments:
In `@src/cloudai/report_generator/training/parser.py`:
- Around line 97-102: Update can_parse around the get_model_config call to also
catch OSError and UnicodeDecodeError, logging the same skip warning used for
malformed JSON/YAML artifacts. Ensure unreadable configuration files return the
non-parseable result so TrainingReporter.generate can continue processing
subsequent runs.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 772e9e84-6dde-4407-b145-b39d24eae80f
📒 Files selected for processing (14)
doc/training-report-schema.mdsrc/cloudai/_core/test_scenario.pysrc/cloudai/core.pysrc/cloudai/parser.pysrc/cloudai/registration.pysrc/cloudai/report_generator/training/__init__.pysrc/cloudai/report_generator/training/mappings.pysrc/cloudai/report_generator/training/models.pysrc/cloudai/report_generator/training/parser.pysrc/cloudai/report_generator/training/reporter.pytests/report_generator/training/test_training_parser.pytests/test_init.pytests/test_parser.pytests/test_test_scenario.py
blugassi
force-pushed
the
feat/training-report-metadata
branch
from
August 10, 2026 12:13
1468060 to
243c357
Compare
podkidyshev
requested changes
Aug 10, 2026
Include identity, environment, hardware, precision, and parallelism details so training reports capture the resolved run context. Signed-off-by: Ben Lugassi <blugassi@nvidia.com>
Generate per-run training reports from a scenario reporter so parsers can include scenario-level context. Signed-off-by: Ben Lugassi <blugassi@nvidia.com>
blugassi
force-pushed
the
feat/training-report-metadata
branch
from
August 12, 2026 07:38
243c357 to
328a108
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@doc/training-report-schema.rst`:
- Around line 47-50: Update the schema history section around the 0.0 entry:
either remove the incomplete 0.0 history entry, or add its release date and
document the 0.0 contract so the version history is unambiguous alongside the
1.0 entry.
- Around line 34-36: Update the field documentation in the training report
schema, including the corresponding fields around the additional referenced
section, to state each key’s requiredness and whether absent values are omitted
or encoded as null. Document that configuration path fields use resolved
absolute paths when available and empty strings when provenance is unavailable.
- Around line 4-5: Fix the broken download reference in the training report
documentation by either adding the missing unified-training-output-design.md
file under doc or updating the :download: target to an existing .rst source
accepted by doc/conf.py. Ensure the link resolves successfully in the
documentation build.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 39013e3c-3bf6-4fcc-8fa7-4698ff9d652d
📒 Files selected for processing (2)
doc/index.rstdoc/training-report-schema.rst
blugassi
force-pushed
the
feat/training-report-metadata
branch
2 times, most recently
from
August 12, 2026 07:56
80d4f06 to
99c593e
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@doc/training-report-schema.rst`:
- Around line 23-44: Add test_template_name with its string type to the
root.config field list for schema version 1.0, alongside the other
TrainingConfig fields. Ensure the documentation matches the field written by
TrainingParser._build_config.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 493b74fb-d793-4c1f-852e-5feab35df8d6
📒 Files selected for processing (1)
doc/training-report-schema.rst
Signed-off-by: Ben Lugassi <blugassi@nvidia.com>
blugassi
force-pushed
the
feat/training-report-metadata
branch
from
August 12, 2026 09:08
99c593e to
f5efccd
Compare
podkidyshev
approved these changes
Aug 12, 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
2 participants
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
training_report.jsonwith schema versioning, run identity, environment, hardware, precision, parallelism, and configuration-source metadata.TrainingReporter.1.0.Test Plan
Tested on Linux with Python 3.12.
Branch-focused result: 140 tests passed.
Both Ruff checks passed. Changed-file Pyright completed with zero errors.
Ran
cloudai generate-reportagainst existing NeMoRun, MegatronRun, and Megatron-Bridge results. All three generated valid reports containing the new metadata and resolved absolute configuration paths.Additional Notes
Unsupported workloads and runs missing required artifacts are skipped with warnings. Configuration paths use empty strings when provenance is unavailable.