Repository navigation
fix: preserve causal delays after AgentX live warmup - #492
Conversation
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
…x/feat-agentx-live-warmup Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
…x/feat-agentx-live-warmup Signed-off-by: hongkuanz <hongkuanz@nvidia.com> # Conflicts: # .github/codeowners/areas.yaml
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
… tests Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
…x/feat-agentx-live-warmup Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📜 Recent review details
📝 Summary
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Cross-Layer Contract |  |
The live policy input is traced through Python validation, serialization, CLI gates, runner capability checks, Rust JSON, and native tests. The output/API contract is incomplete. `AgenticPhaseEvidence… | Document agentic_phases.warmup_policy, including omission for repeated_prefixes and the value for live_trajectory_v1, in the replay result documentation. Document WorkloadDriver::new_agentic_warmup_with_policy and its policy and idl… |
| Modeling And Data Evidence | |
The PR changes timing formulas and dependency/handoff selection logic in crates/core/src/replay/loadgen/phase.rs and driver.rs. It supplies pinned AIPerf and AgentX methodology provenance (`phase.… |
Add reproducible before/after evidence for the changed timing behavior. Use the same pinned AgentX inputs and imported cuts with the base and head implementations, then check in a machine-readable summary containing per-request deadlines, s… |
-
Check name Status Explanation Title check The title precisely states the behavioral change: preserving causal delays after AgentX live warmup. It is specific despite using the fix:prefix.Linked Issues check Issue #464is closed and supplies historical context only. The linked-issue record states that no active directly linked issue objectives or coding acceptance criteria apply. No coding requirement rem…Out of Scope Changes check The reported changes stay within the current pull request intent. Runtime changes implement selectable live AgentX warmup and causal-frontier timing. Tests cover the new policy and validation. Documen… Compatibility Boundaries PASS. The new agentic_warmup_policyuses the same two values andrepeated_prefixesdefault in Python configuration, Rust deserialization, workload forwarding, runner validation, and documentation.…Review Evidence The description names commands and results, including focused Python tests, cargo test --workspace --locked --offline, public Rust probes, and frozen-golden cases. It includes negative and boundary …Description check The description clearly states the problem, implementation scope, causal-timing behavior, validation results, limitations, and integration state. It does not use the required Review map, Modeling or d… Full details: Linked Issues check
-
Explanation
Issue
#464is closed and supplies historical context only. The linked-issue record states that no active directly linked issue objectives or coding acceptance criteria apply. No coding requirement remains for this pull request.
Full details: Out of Scope Changes check
-
Explanation
The reported changes stay within the current pull request intent. Runtime changes implement selectable live AgentX warmup and causal-frontier timing. Tests cover the new policy and validation. Documentation, SDK capability and migration updates describe those changes. CODEOWNERS and third-party notices support the touched implementation. The summary identifies unrelated streaming work, private frontend delay, package publication, benchmarking, and calibration as excluded.
Full details: Compatibility Boundaries
-
Explanation
PASS. The new
agentic_warmup_policyuses the same two values andrepeated_prefixesdefault in Python configuration, Rust deserialization, workload forwarding, runner validation, and documentation. Default serialization omits the field, and Rust report serialization omits the default policy, preserving legacy inputs and reports. The native Rust boundary accepts the policy and enforces the same prerequisites documented by Python. Package versions remain synchronized at0.13.0; the PR changes no package, dependency, binding, or release-artifact files. Maturin still builds oneaisimulatewheel fromaisimulate-core, whose crate types remainrlibandcdylib. The docs state the wheel/crate version contract and keep Dynamo integration as downstream qualification, with no new Dynamo dependency or coupling introduced.
Full details: Review Evidence
-
Explanation
The description names commands and results, including focused Python tests,
cargo test --workspace --locked --offline, public Rust probes, and frozen-golden cases. It includes negative and boundary checks for invalid speedups, unsupported stacks, 2x/0.5x scaling, caps, and timing cases. It separates exact-head local/native results, independent probes, prior-head parity evidence, and excluded AgentX/GPU production qualification. It also states that hosted checks must qualify the exact current head and that prior-head Full CI results do not transfer. The PR changes no workflow files and cites no inactive nested workflow as hosted CI evidence.
Full details: Description check
-
Explanation
The description clearly states the problem, implementation scope, causal-timing behavior, validation results, limitations, and integration state. It does not use the required Review map, Modeling or data provenance, or Tracking sections, and it does not explicitly identify the public or serialized contract change or provide the required Codex-reviewed commit. These are template omissions, but the description remains substantially complete and actionable.
-
Full details: Cross-Layer Contract
-
Explanation
The live policy input is traced through Python validation, serialization, CLI gates, runner capability checks, Rust JSON, and native tests. The output/API contract is incomplete.
AgenticPhaseEvidence.warmup_policyis a new serialized field atcrates/core/src/replay/loadgen/phase.rs:96-108, but the result documentation only lists genericagentic_phasescontents atdocs/replay/workloads.md:515-522and does not definewarmup_policy. The new publicWorkloadDriver::new_agentic_warmup_with_policyatcrates/core/src/replay/loadgen/driver.rs:1440-1475is absent fromdocs/replay/api/rust.md:117-127, and the public API test covers onlynew_agentic_warmupatcrates/tests/public-api/src/lib.rs:641-673.Resolution
Document
agentic_phases.warmup_policy, including omission forrepeated_prefixesand the value forlive_trajectory_v1, in the replay result documentation. DocumentWorkloadDriver::new_agentic_warmup_with_policyand its policy and idle-cap arguments in the Rust API/warmup documentation. Extend the public API test to importAgenticWarmupPolicy, construct the live-policy driver through the public method, and assert the live evidence field.
Full details: Modeling And Data Evidence
-
Explanation
The PR changes timing formulas and dependency/handoff selection logic in
crates/core/src/replay/loadgen/phase.rsanddriver.rs. It supplies pinned AIPerf and AgentX methodology provenance (phase.rs:7-16,THIRD_PARTY_NOTICES.md:16-45) and deterministic unit tests, including the causal-delay regression and speedup/cap cases (phase.rs:1263-1402). However, the tests use self-authored synthetic rows with hand-coded expected timestamps. They are not an independent oracle, held-out comparison, or machine-readable before/after summary with anomaly checks. The PR changes no golden or data artifact, and its documentation explicitly limits validation to native functional qualification rather than parity or performance evidence (docs/replay/agentic/warmup.md:228-236). Reported test counts do not provide the required explained before/after evidence.Resolution
Add reproducible before/after evidence for the changed timing behavior. Use the same pinned AgentX inputs and imported cuts with the base and head implementations, then check in a machine-readable summary containing per-request deadlines, speedup/cap cases, expected causal delays, and anomaly checks for monotonicity, finite values, and cap handling. Alternatively, compare the supported behavior against the pinned AIPerf oracle and include an explained golden diff that documents intentional native-scheduler differences.
✨ Finishing Touches: 💡 2
-
⚔️ Resolve merge conflicts: 💡
-
- Resolve merge conflict in branch
hzhou-codex/fix-agentx-live-handoff-delays
- Resolve merge conflict in branch
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Harrilee
left a comment
Harrilee
left a comment
There was a problem hiding this comment.
LGTM, approving. The same condition applies: #463's fixes plus a green stacked Full CI.
The fix is right. Only the current handoff frontier keeps a source-clock deadline. Blocked nodes with a pending Sequence/Spawn predecessor are skipped, so their timers come from the real dependency event (max(stale ≤ now, now + delay) after compressed_warmup_timing is cleared). Join/barrier gates keep their deadlines. Classification is a single O(nodes + edges) pass at handoff, not in the hot path. The regression reproduces the 4 s continuation behind a 300 s idle cap exactly.
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Signed-off-by: hongkuanz <hongkuanz@nvidia.com>
Summary
#465 is merged; this PR now targets main. Fix causal delays after live AgentX warmup without changing engine scheduling or FPM timing.
Live warmup reconstructed source-clock deadlines for every future request. After an idle cap advanced a current timer, dependency release could restore a later request's stale absolute deadline: a four-second continuation could wait for another 300-second cap.
Updated integration
Current head:
5507c2ad4b58100ab3eea60d643739e4d2e07299, with main00f0eff747801c00af98388dd0028df9eb95c53fmerged into the existing branch using a signed, history-preserving commit.The complete tree matches the independently computed old-parent-relative integration. The PR still contains only its original two-file causal-handoff delta (237 additions / 10 deletions) against main; its stable patch ID is unchanged. Main's live-warmup primer-speedup correction, GLM readiness/pricing changes and HF evidence plumbing are preserved. No force push or performance-data/golden change was made.
Validation
cargo test --workspace --locked --offline: 2,174 passed, one pre-existing R17 local-artifact test ignored; 16 excluded public-API tests passed.Boundaries
This is a functional causal-timing correction, not hardware accuracy qualification. The historical controlled AgentX attempt stopped on a genuine uncovered FPM coordinate; no new full AgentX/GPU benchmark was run for this synchronization. Data coverage, empirical model calibration and the separate frontend experiment are outside this PR.