Skip to content

test: preserve Weka future-parent rekey regression - #234

Merged
Harrilee merged 3 commits into
mainfrom
codex/weka-future-parent-regression
Sep 29, 2026
Merged

Harrilee merged 3 commits into
mainfrom
codex/weka-future-parent-regression

Conversation

@Harrilee

Copy link
Copy Markdown
Contributor

Summary

Restore the Weka regression removed when Dynamo delegated ingestion to AISimulate (ai-dynamo/dynamo#14355). When seam resolution chooses a later request with a deeper prefix overlap as a continuation, an earlier fork must keep its original parent; reparenting it to that future continuation creates a cycle.

The test uses AISimulate's public graph loader, verifies the continuation was selected, and checks that the earlier fork retains its original dependency. Source attribution records the immutable Dynamo revision in the test file and both packaged notice copies.

Validation

  • cargo test --locked -p aisimulate-core --lib replay::loadgen::weka::tests:: — 27 passed.
  • Mutation check: removing the future-parent guard makes the regression fail with a cycle between the earlier fork and later continuation; the guard was restored afterward.
  • rustfmt --check --edition 2024 crates/core/src/replay/loadgen/weka.rs
  • python3 scripts/check_packaged_legal_files.py
  • git diff --check

Signed-off-by: Harry Lee <harrli@nvidia.com>
@Harrilee
Harrilee requested review from a team as code owners September 16, 2026 20:51
@coderabbitai

coderabbitai Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

Important

Review skipped

We couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting @coderabbitai full review.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: ed7e6878-ed43-4cb3-b08f-3f01d4b186eb

📥 Commits

Reviewing files that changed from the base of the PR and between cbaf51b and 37ace07.

📒 Files selected for processing (3)
  • THIRD_PARTY_NOTICES.md
  • crates/core/src/replay/loadgen/weka.rs
  • python/aisimulate/THIRD_PARTY_NOTICES.md
🔗 Linked repositories identified

CodeRabbit 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
⏰ Context from checks skipped due to timeout. (13)
  • GitHub Check: Prediction Regression / Collect prediction snapshot (old)
  • GitHub Check: Prediction Regression / Collect prediction snapshot (new)
  • GitHub Check: Platform Wheels / Build wheels (macosx_arm64)
  • GitHub Check: Platform Wheels / Build wheels (manylinux_2_28_aarch64)
  • GitHub Check: Platform Wheels / Build wheels (manylinux_2_28_x86_64)
  • GitHub Check: Collector Data / Check collector data
  • GitHub Check: Release Artifact Contract (arm64)
  • GitHub Check: Rust feature modes
  • GitHub Check: Engine Golden Regression
  • GitHub Check: Release Artifact Contract (amd64)
  • GitHub Check: Python 3.13 compatibility
  • GitHub Check: Python 3.11 compatibility
  • GitHub Check: Application Test Wheel (arm64)
🧰 Additional context used
📓 Path-based instructions (4)
Preserve deterministic Replay semantics and explicit validation.

⚙️ CodeRabbit configuration file

Files:

  • crates/core/src/replay/loadgen/weka.rs
Verify upstream provenance, license text, collection and overlay hashes, local modification notices, and generated derivative records remain complete and internally consistent.

⚙️ CodeRabbit configuration file

Files:

  • THIRD_PARTY_NOTICES.md
Read REVIEW.md before commenting.

⚙️ CodeRabbit configuration file

Files:

  • python/aisimulate/THIRD_PARTY_NOTICES.md
  • crates/core/src/replay/loadgen/weka.rs
  • THIRD_PARTY_NOTICES.md
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:

  • python/aisimulate/THIRD_PARTY_NOTICES.md
  • crates/core/src/replay/loadgen/weka.rs
  • THIRD_PARTY_NOTICES.md
🔀 Multi-repo context ai-dynamo/dynamo, ai-dynamo/aiconfigurator

Linked repositories findings

ai-dynamo/dynamo

Inspected the branch of open PR #14355 (HEAD 5910d2b), not the default branch.

  • lib/mocker/src/loadgen/mod.rs:8 re-exports aisimulate_core::replay::loadgen::*; Dynamo’s replay loader is therefore a direct consumer of AISimulate’s loadgen/graph API. [::ai-dynamo/dynamo::]
  • Cargo.toml:58-60, pyproject.toml:109-111, and container/deps/requirements.aisimulate.txt:4-6 pin the same AISimulate revision (d71203e...) and explicitly note that it must export the Weka graph-loading API. [::ai-dynamo/dynamo::]
  • tests/dependencies/test_aisimulate_consistency.py:39-167 enforces that all Python, Cargo, container, and lockfile references use that exact revision. Any API changes accompanying this regression should preserve compatibility with that pinned public loader contract or update all coordinated pins. [::ai-dynamo/dynamo::]
  • Dynamo’s Python replay boundary passes Weka traces into the shared loader and relies on embedded source metadata (components/src/dynamo/replay/simulation.py:382-405; lib/bindings/python/rust/llm/replay.rs:971-983,1076-1105). The added regression is directly relevant to this delegated Weka ingestion path. [::ai-dynamo/dynamo::]

ai-dynamo/aiconfigurator

No Weka replay or graph-loader consumers were found in the broad search. Its repository guidance and migration documentation direct active development to AISimulate rather than treating AIC as a runtime dependency. [::ai-dynamo/aiconfigurator::]


📝 Summary

Risk: Low.

Human attention should focus on:

  1. Regression-test coverage for future-parent seam rekeying.
  2. Correctness of the Dynamo provenance and packaged notices.
  3. Validation scope beyond the targeted tests.

Changes:

  • Adds weka_seam_rekey_never_uses_a_future_parent through AISimulate’s public graph loader.
  • Confirms that a later request with deeper prefix overlap becomes the continuation.
  • Confirms that an earlier fork keeps its original dependency and does not form a cycle.
  • Updates both notice files with immutable source provenance and Apache-2.0 attribution.
  • Changes no exported or public entities.

Technical quality:

  • 27 targeted tests passed.
  • Mutation testing confirmed regression failure when the future-parent guard is removed.
  • rustfmt --check, packaged legal-file checks, and git diff --check passed.

Merge readiness:

  • Targeted validation supports the change.
  • Repository-wide test results and independent review findings were not supplied.
  • Review severity counts are unavailable.

Walkthrough

Adds a Weka seam-rekey regression test and documents its NVIDIA Dynamo provenance, source commit, licensing, copyright, and attribution details.

Changes

Weka seam-rekey regression

Layer / File(s) Summary
Seam-rekey regression test
crates/core/src/replay/loadgen/weka.rs
Adds weka_seam_rekey_never_uses_a_future_parent and documents its adapted upstream source. The test checks session reuse and parent selection across overlapping forks.
Provenance notices
THIRD_PARTY_NOTICES.md, python/aisimulate/THIRD_PARTY_NOTICES.md
Adds matching NVIDIA Dynamo provenance notices with the upstream commit, source path, behavioral assertions, copyright, Apache-2.0 license, and disclaimer.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 37ace

The PR adds valid targeted regression coverage and consistent provenance documentation without introducing production risk.

🚥 Pre-merge checks | ✅ 7 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the problem, behavior change, affected consumer, and validation results. It does not follow the required template and omits the Review map, Modeling or data provenance, and Tr… Rewrite the description using the required headings: Why and what changed, Review map, Evidence, Modeling or data provenance, and Tracking. Include risk level, contract impact, compatibility or rollback concerns, exact evidence fields, prov…
✅ Passed checks (7 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cross-Layer Contract ✅ Passed The PR changes no public input, schema field, default, output, serialization contract, CLI path, or Python interface. The Rust diff adds provenance comments and one unit test only. The test exercises …
Modeling And Data Evidence ✅ Passed PASS. The pull request adds no production selection-logic change. It adds a deterministic, machine-readable Weka fixture with explicit graph assertions for the continuation and parent relationships. T…
Compatibility Boundaries ✅ Passed PASS. The pull request adds one Rust regression test, provenance comments, and two identical notice sections. It does not change Python or Rust public declarations, schemas, versions, bindings, defaul…
Review Evidence ✅ Passed The description names the targeted cargo test --locked -p aisimulate-core --lib replay::loadgen::weka::tests:: command and reports 27 passed. It includes negative validation through the mutation che…
Title check ✅ Passed The title precisely describes the behavioral change: preserving the Weka future-parent rekey regression. It avoids vague wording.
Full details: Description check

Explanation

The description explains the problem, behavior change, affected consumer, and validation results. It does not follow the required template and omits the Review map, Modeling or data provenance, and Tracking sections. The Evidence section is also replaced by a less detailed Validation section.

Resolution

Rewrite the description using the required headings: Why and what changed, Review map, Evidence, Modeling or data provenance, and Tracking. Include risk level, contract impact, compatibility or rollback concerns, exact evidence fields, provenance status or N/A, and issue or PR references.

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands.

@Harrilee Harrilee changed the title test(replay): preserve Weka future-parent rekey regression test: preserve Weka future-parent rekey regression Sep 24, 2026
@Harrilee

Copy link
Copy Markdown
Contributor Author

Checked the failed run 35149065556 against the current merged head 93356ae0.

The old failures were collector registry drift (49 differences) and stale GEMM case-count, DeepSeek-V4 FPM capability, and B200/SGLang GDN expectations, in both architecture lanes. They were unrelated to the added Weka test. The main merge already brought in the relevant fixes; no additional code change was necessary.

Fresh local validation at 93356ae0, with its native extension rebuilt:

  • Previously failing Python test selections: 5 passed (including the current parameterized FPM cases).
  • Collector data invariants: all rules passed.
  • Backend facts: 2,139 fact slices match.
  • Weka Rust tests: 27 passed, including the new regression.
  • Packaged legal files match.

New Full CI is still running, with no failures observed at this check. This is not a claim that hosted CI has completed.

Scope clarification for ai-dynamo/dynamo#14355: this PR only restores a regression test and its attribution. The runtime future-parent guard already exists in the pinned nightly; there is no runtime fix here that Dynamo must wait to consume in a new release.

…ent-regression

# Conflicts:
#	THIRD_PARTY_NOTICES.md
#	python/aisimulate/THIRD_PARTY_NOTICES.md
@Harrilee
Harrilee enabled auto-merge (squash) September 29, 2026 21:13
@Harrilee
Harrilee merged commit 7108a0a into main Sep 29, 2026
67 checks passed
@Harrilee
Harrilee deleted the codex/weka-future-parent-regression branch September 29, 2026 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-ready Ready for automated and human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants