chore: prepare AISimulate 0.13.0 release - #268
Conversation
Signed-off-by: Harrison King Saturley-Hall <hsaturleyhal@nvidia.com>
|
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 (10)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review. 📜 Recent review details🧰 Additional context used📓 Path-based instructions (10)Enforce the mapped collector guidelines.⚙️ CodeRabbit configuration file Files:
Enforce the single-oracle and golden-diff rules in python/aisimulate/.claude/rules/rust-core/parity.md.⚙️ CodeRabbit configuration file Files:
Require coverage of the changed behavior and its negative or boundary cases.⚙️ CodeRabbit configuration file Files:
Check commands, defaults, supported runtimes, public names, and claims against executable behavior.⚙️ CodeRabbit configuration file Files:
Read REVIEW.md before commenting.⚙️ CodeRabbit configuration file Files:
A legal branch changes HOW a case runs.📄 CodeRabbit inference engine (python/aisimulate/.claude/rules/collector/layer_permissions.md) Files:
Core doctrine: **observe, don't predict.**📄 CodeRabbit inference engine (python/aisimulate/.claude/rules/collector/failure_handling.md) Files:
The declaration surface is exactly two kinds of YAML plus one capability table — if you feel the need for a new kind of rule, re-read `layer_permissions.md` first.📄 CodeRabbit inference engine (python/aisimulate/.claude/rules/collector/case_authoring.md) Files:
Do not reintroduce them.📄 CodeRabbit inference engine (python/aisimulate/.claude/rules/rust-core/parity.md) Files:
Before making any change under: `python/aisimulate/src/aiconfigurator/generator/**` MUST read: `python/aisimulate/.claude/rules/generator-development.md` Before making any change under `python/aisimulate/collector/**` MUST read: `python/ais...📄 CodeRabbit inference engine (AGENTS.md) Files:
🪛 ast-grep (0.45.3)tests/test_ci_qualification.py[error] 118-118: Avoid command injection (command-injection-python) [error] 118-118: Command coming from incoming request (subprocess-from-request) [info] 120-120: use jsonify instead of json.dumps for JSON output (use-jsonify) [error] 121-133: Command coming from incoming request (subprocess-from-request) [error] 140-145: Command coming from incoming request (subprocess-from-request) 🔀 Multi-repo context ai-dynamo/dynamo, ai-dynamo/aiconfiguratorLinked repositories findingsai-dynamo/dynamo
ai-dynamo/aiconfigurator
🔇 Additional comments (5)
📝 SummaryRisk: Medium. Merge readiness still depends on validation results not supplied. Most human attention:
Changed behavior and public contracts
Evidence supplied
Technical qualityThe supplied evidence supports alignment of release metadata, CI baseline handling, compatibility messaging, and current runtime contracts. The baseline resolver has focused safety tests. No specific defect is established. Merge readinessBefore merge, confirm current CI results, WalkthroughThe release version changed from 0.12.0 to 0.13.0 across Rust and Python metadata, release tooling, artifact documentation, compatibility messaging, CI workflows, and validation tests. Numerical CI now resolves recorded prediction baselines before running checks. ChangesRelease version update
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The release and validation changes have no concrete remaining merge-blocking risk identified. 🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 passed)
Comment |
Signed-off-by: Harrison King Saturley-Hall <hsaturleyhal@nvidia.com>
jasonqinzhou
left a comment
jasonqinzhou
left a comment
There was a problem hiding this comment.
The version metadata and lockfiles are synchronized, and the five focused nightly-version/historical-manifest tests pass locally with a working Node runtime. Before closeout, the contracts jobs need their missing tools installed: Full CI run 35304255738 fails on both architectures with missing huggingface_hub during FPM test collection and missing Node.js in _nightly_version. These requirements predate this PR. I will add explicit contract-job setup and rerun the exact updated head.
…ct CI Signed-off-by: Jason Zhou (Engrg-Hardware 1) <jasonzho@nvidia.com>
Signed-off-by: Jason Zhou (Engrg-Hardware 1) <jasonzho@nvidia.com>
jasonqinzhou
left a comment
jasonqinzhou
left a comment
There was a problem hiding this comment.
Current-main integration exposed a separate reproducible CI provenance setup failure: Fast CI 35357873986 cannot resolve numerical baseline af91885. GitHub still serves this historical PR commit, but normal branch checkout does not include it after squash merge. Preserve the manifest SHA, all eight expected values, and their tolerances; explicitly fetch that validated immutable baseline before Fast CI contract tests, Full CI repository contracts, and native numerical qualification.
Signed-off-by: Jason Zhou (Engrg-Hardware 1) <jasonzho@nvidia.com>
|
@coderabbitai full review |
Signed-off-by: Harrison King Saturley-Hall <hsaturleyhal@nvidia.com>
jasonqinzhou
left a comment
jasonqinzhou
left a comment
There was a problem hiding this comment.
Codex review of 81e214e against current main d9f1580: no remaining actionable findings. Product and artifact versions agree at 0.13.0; the legacy command remains available with removal targeted for 0.14.0 subject to verified replacements. Current docs and warnings agree, and historical publication records remain intact. The numerical-baseline repair validates the full SHA, explicitly fetches only missing commits, preserves checkout HEAD/branch and the frozen manifest, and fails on unavailable sources. Local validation includes 356 workflow/FPM cases (three native-runtime cases deferred to hosted CI), all 304 Fast CI workflow/qualification/release-FPE cases after the fetch fix, release-manifest/lock checks, lint, and warning behavior. Hosted Fast CI 35358464789 passed on this SHA. Current-head CodeRabbit and Full CI remain pending; this is not merge readiness.
|
/ok to test 81e214e |
@jasonqinzhou, there was an error processing your request: See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/2/ |
jasonqinzhou
left a comment
jasonqinzhou
left a comment
There was a problem hiding this comment.
The workflow-contract exclusion can rely on the existing exact-head Fast CI coverage, but the FPM accuracy exclusion removes installed-wheel coverage and needs correction before merge.
Signed-off-by: Jason Zhou (Engrg-Hardware 1) <jasonzho@nvidia.com>
|
/ok to test dd23ccb |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
jasonqinzhou
left a comment
jasonqinzhou
left a comment
There was a problem hiding this comment.
Full CI on dd23ccb exposed two inherited contract mismatches that need correction before approval:
- [P1] Pure-Rust engine/FPM test fixtures load only direct b200_sxm/vllm/0.24.0 tables, but the corrected database now supplies fp8_block through its declared reuse layer. Sixteen tests fail before reaching their behavioral assertions. Load the fixtures through the supported reuse-aware database path; preserve assertions and all production data/math.
- [P1] The installed-wheel verifier still expects the pre-migration SDK facade, rejecting the newly public ForwardPassPerfModelConfig and ForwardPassPerfOptions on all three wheel platforms. Update its exact export contract to the current canonical SDK.
The prediction comparison and engine golden gates already pass. I am investigating the remaining Full CI failures before pushing the corrections.
jasonqinzhou
left a comment
jasonqinzhou
left a comment
There was a problem hiding this comment.
Further Full CI findings on dd23ccb:
- [P1] Collector backend facts were not synchronized with merged PR #244. All 227 differences map to parquet files changed by ba64235: 221 new vLLM 0.25.0 slices (B300/GB200/GB300/H100/H200) and six intentionally removed direct 0.24.0 FP8-block GEMM slices. Existing shared slices have no changed backend/kernel mappings. Update only those facts; preserve the curated translation map, measurements, and strict drift check.
- [P1] Three replay contract tests still assert retired aic_forward_model/aic_system rank arguments after PR #242 moved that identity into forward_pass_estimators. Assert canonical per-role estimator identity instead. The AFD artifact golden also needs its replay-spec hash updated for the newly serialized empty forward_pass_estimators field; verify that this field alone explains the digest change before updating it. No performance golden values need to change.
|
Honor declared vLLM reuse in Rust fixtures, synchronize only the 227 backend facts changed by PR #244, and verify the canonical SDK and replay configuration from PR #242. The AFD golden changes only its serialization digest for the added empty forward_pass_estimators field; production math, measurements, and numerical goldens are unchanged. Signed-off-by: Jason Zhou (Engrg-Hardware 1) <jasonzho@nvidia.com>
jasonqinzhou
changed the title
jasonqinzhou
left a comment
jasonqinzhou
left a comment
There was a problem hiding this comment.
Codex review of bc222e6 against main d9f1580: no remaining actionable findings in the complete diff. The inherited failures reported above are corrected in this commit. Clarification: prediction rank identity is asserted through canonical timing_model.config; forward_pass_estimators is the additional serialized deployment field responsible for the AFD digest change.
Local evidence: 698 Rust performance-model tests passed (one existing ignored test), 65 replay/CLI tests passed, 45 registry/layering/deprecation tests passed, and the strict registry check passed for 2,137 slices. Every registry change maps to a parquet file changed by #244; existing mappings and production measurements are untouched. Removing only the new empty serialization field exactly reproduces the old AFD hash, with all other golden content equal. SDK export/native-engine smoke and lint/format/whitespace checks pass. Earlier baseline-fetch regression coverage remains in the final tree.
Release versions remain 0.13.0 and the retained compatibility command targets removal in 0.14.0 subject to verified replacements. Numerical sentinel baselines, values, tolerances, and engine goldens remain unchanged. Final-head CodeRabbit and hosted CI are still required before the requested approval and merge.
jasonqinzhou
left a comment
jasonqinzhou
left a comment
There was a problem hiding this comment.
Validated the two warnings in CodeRabbit's completed dd23ccb review against current bc222e6:
- [P2] The active API mapping and temporary Cargo alias in docs/repository-history.md still target 0.12 even though the current release is 0.13. Update those current-use examples while preserving the explicitly historical migration-start version.
- [P2] The new baseline-fetch helper has real Git coverage, but its CLI mode-selection boundary lacks a direct regression test. Add subprocess coverage proving --fetch-baseline-only succeeds with only a baseline manifest (no prediction cases), produces no prediction artifact, and rejects missing or conflicting mode arguments.
Both are narrow release-validation corrections and will be fixed before final approval.
Signed-off-by: Jason Zhou (Engrg-Hardware 1) <jasonzho@nvidia.com>
jasonqinzhou
left a comment
jasonqinzhou
left a comment
There was a problem hiding this comment.
Codex review refreshed for 031d8db against unchanged main d9f1580: no remaining actionable findings. The two CodeRabbit cross-layer warnings are fixed. The latest delta updates current-use 0.13 compatibility examples and adds direct CLI boundary coverage; all 40 numerical-qualification tests, lint, and whitespace checks pass. The full-diff assessment and 698 Rust / 65 replay-CLI / 45 layering-registry-deprecation test evidence from bc222e6 remain applicable to unchanged files. Final-head CodeRabbit, Fast CI, and Full CI are still required before approval and merge.
|
@coderabbitai review Please refresh coverage for the current head |
|
/ok to test 031d8db |
|
@coderabbitai The description-template warning is addressed. The PR body now includes every required section and field: review map, medium risk, contract/compatibility impact, exact reviewed commit, commands and results, negative cases, digest derivation, data provenance, and tracking. Full CI is running on the unchanged reviewed head 031d8db: https://github.com/ai-dynamo/aisimulate/actions/runs/35363166376. Please recheck the description warning. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
gh pr view 268 --repo ai-dynamo/aisimulate --json number,headRefOid,body,url,statusCheckRollup \
--jq '{number, url, headRefOid, body, checks: [.statusCheckRollup[]? | {name, status, conclusion, detailsUrl}]}'Length of output: 12478
The body includes the required review map, medium-risk assessment, contract and compatibility impact, reviewed commit, validation evidence, negative cases, digest derivation, data provenance, and tracking. The description-template warning is addressed. Full CI run You are interacting with an AI system. |
|
|
jasonqinzhou
left a comment
jasonqinzhou
left a comment
There was a problem hiding this comment.
Approved after the requested release/compatibility corrections and exact-head verification. The compatibility command remains in 0.13.0 with removal targeted for 0.14.0 after verified replacements. CodeRabbit and Codex reviewed 031d8db with all findings addressed; all review threads are resolved. Fast CI passed, and Full CI 35363166376 passed all 54 jobs on this head, including the previously failing Rust, wheel, Collector, and replay-contract checks. Current main remains d9f1580 and the reviewed branch includes it. Release publication and downstream Dynamo qualification remain separate gates.
2 participants
Why and what changed
Prepare AISimulate 0.13.0 by synchronizing wheel/crate versions, lockfiles, release checks, and artifact documentation. Retain the
aiconfiguratorcompatibility command in 0.13.0 and target removal for 0.14.0 once all remaining workflows have verified unified-CLI replacements. Align current examples and runtime warnings; preserve dated publication records.Repair release validation exposed by integrating current main. Keep workflow-only contracts in Fast CI and installed-wheel FPM accuracy coverage in Full CI, with joint application/FPM dependency resolution and
pip check. Fetch the numerical manifest's immutable historical commit when a fresh clone lacks it after squash merge. Align Rust fixtures, the backend-facts registry, the wheel verifier, and replay tests with merged #244 and #242.Review map
scripts/check_prediction_numerics.pyand the two active CI workflows;aiconfigurator/deprecation.pyand migration docs; the Rust fixture loaders, wheel SDK verifier, and canonical replay assertions. The Collector registry diff is limited to 221 additions and six removals traced to fix: add vLLM 0.25.0 measurements and correct FP8-block reuse #244.Evidence
cargo test -p aisimulate-core --lib perfmodel::passed 698 tests (one existing ignored test).python -m pytest -p no:timeout tests/test_afd_artifacts.py tests/test_unified_traffic_runtime.py tests/e2e/test_unified_cli_engine.py -qpassed 65 tests. Backend-facts, package-layering, and deprecation unit suites passed 45 tests.python -m pytest -p no:timeout tests/test_ci_qualification.py -qpassed all 40 tests on the final tree.scripts/build_release_artifacts.py --check-only; Ruff check/format;cargo fmt --all --check;git diff --check; current SDK-facade/native-engine smoke in the editable environment. Earlier workflow/FPM validation passed 356 cases (three installed-native cases deferred to hosted CI), and the earlier Fast CI test set passed 304 cases.031d8db3a81e4e42ffe43c37161517f2ec05157d.031d8db3a81e4e42ffe43c37161517f2ec05157d, including both architectures, all three platform wheels, installed-wheel FPM/replay contracts, Collector validation, and engine/prediction regression gates.031d8db3a81e4e42ffe43c37161517f2ec05157d, completed with no actionable code comments. Its description-template warning is addressed by these sections.031d8db3a81e4e42ffe43c37161517f2ec05157d, no remaining actionable findings after the full diff and follow-up review.bbbd762b...to3f53f3ae...; removing only refactor(perfmodel): unify estimator selection and configuration #242's newly serialized emptyforward_pass_estimatorsfield exactly reproduces the old digest.timing_model.config; wheel verification expects the two canonical public FPM exports. All other AFD golden content is identical.Modeling or data provenance
No new measurements, proxies, or production modeling changes. All 227 changed Collector facts map to parquet files changed by #244: 221 vLLM 0.25.0 slices (49 each B300/GB200/GB300; 37 each H100/H200) and six deliberately removed direct 0.24.0 FP8-block GEMM slices. Existing backend/kernel mappings and the curated translation map are unchanged.
python tools/perf_database/backend_facts.py --checkpasses for all 2,137 slices. The collection provenance, physical-key and anomaly checks, and declared-reuse limits remain documented in the #244 evidence bundle.Tracking