Skip to content

test(spec): cover speculative TP synchronization contract - #37264

Open
seongyun1104 wants to merge 1 commit into
sgl-project:mainfrom
seongyun1104:test/dspark-tp-sync-contract
Open

seongyun1104 wants to merge 1 commit into
sgl-project:mainfrom
seongyun1104:test/dspark-tp-sync-contract

Conversation

@seongyun1104

@seongyun1104 seongyun1104 commented Aug 31, 2026

Copy link
Copy Markdown

Summary

  • cover the SGLANG_SPEC_TP_SYNC parser, presets, numeric/sluggified selectors, and negation
  • pin the default all-sites synchronization contract introduced by [Spec] Fix Dspark and Dflash state divergence across TP rank #33614
  • cover enabled/disabled and single-rank broadcast behavior
  • cover distributed-min versus local available-memory calls

Why

SpecTpSync protects DSpark and DFlash decisions that must stay identical across TP ranks, including graph-tier planning and acceptance state. The helper currently has no direct unit coverage, so a parser/default regression or an accidental single-rank collective could silently weaken that correctness boundary.

Validation

  • ruff check test/registered/unit/spec/test_spec_tp_sync.py
  • ruff format --check test/registered/unit/spec/test_spec_tp_sync.py
  • python3 -m py_compile test/registered/unit/spec/test_spec_tp_sync.py
  • 10/10 unittest cases passed with the production modules loaded through an isolated CPU harness
  • upstream lint workflow passed

Repository-level pytest collection was unavailable on the local macOS host because SGLang imports Triton during package initialization. The PR is ready for the registered Linux CPU test once project CI is authorized.


CI States

Latest PR Test (Base): ❌ Run #33407379655
Latest PR Test (Extra): ❌ Run #33407379145
Latest PR Test (AMD ROCm 7.2): ❌ Run #33407379685

@seongyun1104
seongyun1104 marked this pull request as ready for review September 1, 2026 09:09
@seongyun1104

Copy link
Copy Markdown
Author

Ready for review. The earlier matrix was blocked only because the PR was draft; upstream lint passed, and the focused parser/broadcast suite passes 10/10 locally. Could a maintainer please authorize the registered Linux CI when convenient?

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.

1 participant