config: publishing is the process entry's job - #36251
Merged
Merged
Conversation
ch-wan
marked this pull request as ready for review
August 25, 2026 03:18
ch-wan
requested review from
CatherineSue,
Fridge003,
JustinTong0323,
ShangmingCai,
Ying1123,
hnyls2002,
ispobock,
liusy58,
merrymercy,
slin1237 and
xiezhq-hermann
as code owners
August 25, 2026 03:18
ch-wan
force-pushed
the
cheng/gc-p1-parallel-tier
branch
from
August 25, 2026 03:47
762949f to
bd01f13
Compare
ch-wan
requested review from
Jiminator,
Qiaolin-Yu,
alexnails,
alphabetc1,
b8zhong,
fzyzcjy,
hanming-lu,
huangtingwei9988,
hzh0425,
jybsuper,
kpham-sgl,
lifuhuang,
mmangkad,
pyc96,
yizhang2077 and
yushengsu-thu
as code owners
August 25, 2026 03:47
ch-wan
force-pushed
the
cheng/gc-p2-publish-at-entry
branch
from
August 25, 2026 03:47
945cacf to
4083266
Compare
ch-wan
force-pushed
the
cheng/gc-p1-parallel-tier
branch
from
August 26, 2026 06:48
bd01f13 to
1303cea
Compare
ch-wan
force-pushed
the
cheng/gc-p2-publish-at-entry
branch
from
August 26, 2026 06:48
4083266 to
d49cb9d
Compare
ch-wan
force-pushed
the
cheng/gc-p1-parallel-tier
branch
from
August 26, 2026 07:22
1303cea to
18e7fd8
Compare
ch-wan
requested review from
mickqian,
yctseng0211,
yhyang201 and
yuan-luo
as code owners
August 26, 2026 07:22
ch-wan
force-pushed
the
cheng/gc-p2-publish-at-entry
branch
from
August 26, 2026 07:22
d49cb9d to
b1d2a8b
Compare
ch-wan
force-pushed
the
cheng/gc-p1-parallel-tier
branch
from
August 26, 2026 08:12
18e7fd8 to
27cde8d
Compare
ch-wan
force-pushed
the
cheng/gc-p2-publish-at-entry
branch
2 times, most recently
from
August 26, 2026 08:21
048d3b8 to
8431d22
Compare
ch-wan
force-pushed
the
cheng/gc-p1-parallel-tier
branch
from
August 26, 2026 09:37
27cde8d to
06caed3
Compare
ch-wan
force-pushed
the
cheng/gc-p2-publish-at-entry
branch
from
August 26, 2026 09:37
8431d22 to
62d6018
Compare
`/server_info`, its gRPC and in-process twins, and the scheduler's internal-state dump all handed out the record itself: three via `dataclasses.asdict(server_args)` and one via `dict(vars(server_args))`. Both read the fields, which carry resolution's result only for as long as declarations materialize onto the record -- and the point of declaring is that they will stop. Left alone, these endpoints would quietly start reporting what the operator typed instead of what resolution decided. `ServerArgs.resolved_dict()` is the whole-object shape of `resolution_result`: every field, read through the declarations, nested dataclasses expanded the way `asdict` expands them. The four exits report it. Values are unchanged today -- 15 launch shapes agree field for field across all 476 -- so this is the placement, not a new answer. The `vars()` base was also leaking: the resolution bookkeeping (`_raw_input`, the declaration stash, the materialization marker) and the `ModelConfig` memo crossed IPC into `/server_info`'s `internal_states` block. The projection is fields only, and a test pins that the dump is exactly the fields.
Three constructors published defensively because each could be built with nothing published before it: `ModelRunner`, `TokenizerManager`, `MMEncoder`. That made "are the config bags available here?" a question about which constructor happened to have run, which is the wrong place for it: code between the process entry and that constructor cannot read a bag, and a `publish` that lands late re-projects the bags and silently drops any `override()` taken in between. The six entries that relied on it now publish for themselves: `init_multi_tokenizer` (the multi-tokenizer worker reads its record from shared memory), the two `benchmark/one_batch` work functions (run inline for tp_size == 1 and spawned per rank otherwise), the encoder's gRPC entry, and its spawned TP and DP workers. `publish` resolves through the idempotent gate, so a spawned child that receives a resolved record is unaffected. No behavior change: the constructors' `ensure_published` becomes a no-op at each of these, and an override taken between an entry and its constructor now survives instead of being discarded. `test_publish_precedes_bag_reads` pins all six as publishing entries, so the walk checks each one's publish against the bag reads it reaches.
…t publish `ModelRunner`, `TokenizerManager` and `MMEncoder` published defensively through `ensure_published`. With every process entry publishing for itself, a constructor arriving unpublished no longer means "this is a standalone build" -- it means an entry was missed, and publishing here would work by accident while re-projecting the bags over whatever the process had. `assert_published` replaces it: same identity-and-role check, and it raises with what is published instead, naming the entry as the place to publish. The draft runner is unchanged (it deliberately does not publish, so it does not check either). The four manual tests that built one of these objects directly now publish in their setup, which is what they are: the process entry for that test. The two constructor entries leave `_KNOWN_ENTRIES`, and the constructor-publisher census is down to the two that really are entries -- `Engine` and `SchedulerActor`.
ch-wan
force-pushed
the
cheng/gc-p2-publish-at-entry
branch
from
August 26, 2026 11:52
62d6018 to
de39aea
Compare
wangfakang
added a commit
to wangfakang/sglang
that referenced
this pull request
Aug 27, 2026
Resolve conflicts from the config-bag refactor (upstream sgl-project#36251-sgl-project#36255): - kimi_k3_hook.py: keep disable_kimi_k3_symm_mem removed (revert intent). - server_args.py _handle_cuda_graph_config: drop the disable_kimi_k3_symm_mem import/call (revert intent), keep upstream's cfg = resolving_view(self) the merged warning block at the end needs. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: wangfakang <fakangwang@gmail.com>
saturn-acc
pushed a commit
to saturn-acc/sglang
that referenced
this pull request
Aug 31, 2026
nzr-niu
pushed a commit
to nzr-niu/sglang
that referenced
this pull request
Sep 1, 2026
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.
Motivation
Publishing the config bags was happening inside constructors —
TokenizerManager,ModelRunner, the encoder server. That makes "is the config published?" depend on whichobject a process happened to build first, and it means a process that legitimately builds
none of them (a spawned encoder worker, a benchmark entry) reads unpublished bags. It also
puts a process-wide side effect in an object's
__init__, so a test that builds two ofthem publishes twice.
Separately, whole-object readbacks (
/server_info, the resolved-args dict) walked therecord's fields. Once a decision lives in the declaration stash rather than in the field,
walking fields reports the wrong shape.
What changes
publish(server_args, role=...)is called by the process entry: the HTTP entry, theencoder gRPC entry, the encoder DP and TP workers after spawn, and the one-batch
benchmark entry.
assert_published(server_args, role=...)instead of publishing. Itfails closed with the role in the message, so an entry that forgot to publish says so at
startup instead of returning a stale or empty namespace.
resolution_projection(server_args)/ServerArgs.resolved_dict(), which report the declaration result for every leaf,expanding nested dataclasses and namedtuples the way
asdictdoes.test_publish_precedes_bag_reads.pywalks each known entry and asserts publishingprecedes the first bag read.
Nothing here changes what any field holds: resolution still writes as it declares, and the
end-of-resolution replay is untouched. Both go away together in the last PR of the stack.
How to verify
🤖 Generated with Claude Code
CI States
Latest PR Test (Base): ❌ Run #32965569690
Latest PR Test (Extra): ✅ Run #32966000247
Latest PR Test (AMD ROCm 7.2): ❌ Run #32965569687