fix(config): retain pre-engine resolution declarations - #37195
Merged
Conversation
Signed-off-by: Ishan Dhanani <ishandhanani@gmail.com>
Collaborator
Author
|
/tag-and-rerun-ci |
ishandhanani
marked this pull request as ready for review
August 31, 2026 18:33
Collaborator
Author
|
relevant a-stage tests have passed |
nzr-niu
pushed a commit
to nzr-niu/sglang
that referenced
this pull request
Sep 1, 2026
…7195) Signed-off-by: Ishan Dhanani <ishandhanani@gmail.com>
StevenChenSE
pushed a commit
to StevenChenSE/sglang
that referenced
this pull request
Sep 6, 2026
…7195) Signed-off-by: Ishan Dhanani <ishandhanani@gmail.com>
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
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.
SGLang #36255 made
ServerArgspreserve raw operator input, so Dynamo began declaring its pre-engine settings throughdeclare_late_resolution(); the first engine resolution pass then cleared that declaration stash, silently dropping Dynamo's snapshot, GPU-memory-service, and forward-pass-metrics settings. This change preserves those declarations as inputs to resolution, allowing downstream launchers to retain their intended effective configuration while rawServerArgsvalues remain unchanged.How This Was Implemented
_resolved_overridesentries after snapshotting raw input, rather than replacing the stash at the start ofrun_resolution_pipeline().enable_forward_pass_metricsbeforeresolve_once()and verifies the resolved projection observes it while the raw field does not change.Walkthrough
Mental model
ServerArgsnow has two intentionally separate states: raw operator input on the record and effective values in the declaration stash. A launcher that needs to influence the engine before it starts callsdeclare_late_resolution(); resolution must treat that declaration exactly like the declarations it derives internally.sequenceDiagram participant D as Dynamo launcher participant SA as SGLang ServerArgs participant E as SGLang Engine resolution participant R as Resolved projection D->>SA: declare_late_resolution(FPM, snapshot, GMS) E->>SA: snapshot raw input E->>SA: retain launcher declarations E->>SA: append engine declarations R->>SA: project effective configurationDownstream failure and ordering
The raw-input change exposed two separate Dynamo integration assumptions. First, Dynamo passed raw
page_size=Noneto its KV publisher and hit aTypeError; Dynamo #13905 fixes that by reading SGLang's post-engine resolved view. During that same integration audit, Dynamo moved its own pre-engine settings to SGLang's declaration channel — but this pipeline reset discarded the entries before the engine projected them, leaving the engine to start without those settings.This patch fixes the second failure at its source. It copies the existing stash before adding declarations derived during resolution, so a launcher's pre-engine decision remains visible to
resolution_result()and the published config; declarations made later in the pipeline still win as before.Boundaries
This does not materialize defaults back into raw fields, change post-publish mutation rules, or alter Dynamo's resolved-view handoff for
page_size.declare_late_resolution()remains pre-publish only; post-publish configuration changes still use the runtime configuration bags.Validation
python -m pytest -q test/registered/unit/server_args/test_resolution_declarations.py—17 passed.pre-commit run --files python/sglang/srt/arg_groups/pipeline.py test/registered/unit/server_args/test_resolution_declarations.py.Summary
Validation
python -m pytest -q test/registered/unit/server_args/test_resolution_declarations.pypre-commit run --files python/sglang/srt/arg_groups/pipeline.py test/registered/unit/server_args/test_resolution_declarations.pyCI States
Latest PR Test (Base): 🚫 Run #33425671954
Latest PR Test (Extra): ❌ Run #33425671224
Latest PR Test (AMD ROCm 7.2): 🚫 Run #33425672198