config: the resolution callbacks into the record go to zero by ch-wan · Pull Request #36972 · sgl-project/sglang

ch-wan · 2026-08-29T06:21:12Z

The series

Stacked, each PR based on the one above it. Read them in order; every boundary is
self-sufficient and green on its own.

#36896 — the resolution pipeline's dispatcher leaves the record
#36972 — the callbacks into the record go to zero ← you are here
#36973 — six more runtime readers ask the bags
#36974 — the dead record parameters go
#36975 — the lazy imports that buy nothing become eager

#36925 — CI vehicle — runs the whole series against main, not for merge

Motivation

After #36792 the resolution pipeline lived in arg_groups/, but it still reached back into the
record for a great deal of what it needed. A census of the package found 96 call sites across 24
ServerArgs members, 503 lines — a hook handed the record, then calling a method on it to reach
a function that belongs in its own package. That first census counted method calls; a property
read is an attribute access, so two of them were invisible to it and are included here.

Splitting on "what is this member for" gives two very different shapes:

Fourteen members have no other caller at all — not elsewhere in srt, not in a test. 352
lines of resolution-time helper that ended up on the record for historical reasons.
The rest are read by the runtime too, and are small: get_model_config is 27 lines with 56
call sites. There the work is the callers, not the code.

This PR takes both to zero.

Modifications

Five steps, one per commit.

The resolution-only helpers move to the family that calls them.
generate_{decode,prefill,cpu}_*_batch_sizes and apply_cuda_graph_disaggregation_roles join
cuda_graph_hook; reserve_for_graph_mb, reserve_for_deepep_a2a_mb and
adjust_mem_fraction_for_vlm join memory_hook; the two dispatch-token budgets join moe_hook;
is_mistral_native_format joins model_path_hook; is_attention_backend_not_set and
get_default_attn_backend join overrides. The two _set_default_dsa_* members were one-line
forwards to a post-process pass and are now that call.

The declaration seams stop being members. _resolved, _declare and _late_resolution were
one-line forwards to resolved_view, declare_resolution and declare_late_resolution; 61 call
sites went through them. Every call site names the function.

The attention-backend pair is one function. _resolved_attention_backends and
get_attention_backends were two names for attention_backends_of(resolved_view(record)).
use_mla_backend and should_report_expert_balancedness become free functions.

The model configuration is built by a function. get_model_config becomes
overrides.model_config_of. The memo stays on the record beside _resolved_overrides and
_cuda_graph_config_locked — it is resolution scratch, it has to survive the pickle to a child,
and there is no per-record store anywhere else at resolution time. What leaves is the callback.

The last three computed reads follow. post_capture_kv_sizing_planned,
cutedsl_moe_max_num_tokens and max_prefill_buffer_tokens are pure functions of the resolving
view, and each already had a published sibling in runtime_context for readers that run after
publish. Only the pre-publish half was still a method.

And the last two properties. mamba_cache_chunk_size and max_speculative_num_draft_tokens
are the same shape — a derived value with a published sibling — and were only missed because a
property read is not a call. Lifting them is what makes the census claim true for reads as well as
calls; the post-publish readers in pool_configurator and tokenspeed_mla_backend move to the
runtime_context siblings, which is where they belonged.

arg_groups/ now touches nothing on ServerArgs — no method call and no property read. The
record goes from 5277 to 4653 lines and from 54 methods to 25.

model_hook and moe_hook also pick up the import hoist that PR 5 does everywhere else: the two
lifted properties join the same module-scope import block, and splitting the two changes by hunk
would leave an intermediate commit that does not compile.

What this makes visible

Two defects that only a package-side guard can see, because both were invisible while the code was
a method on the class:

get_default_attn_backend read tp_size off the record — raw input, not what resolution
decided. test_no_hook_reads_a_field_off_the_record walks arg_groups/ and now reports it. It
reads through the view.
hisparse_hook._is_hip imported is_hip from server_args, so the tests would have a stable
patch target. That made a configuration module the home of a platform probe. It asks
utils.common now.

And one thing about test seams worth stating, because it decides where every future patch goes.
use_mla_backend and attention_backends_of were bound at module scope in six modules each. A
from ... import name is a copy, so patching the source module reached none of them, and
patching one hook reached only that one — which is why the tests patched
ServerArgs.use_mla_backend, the class attribute, instead. Both are imported at their call sites
now, so patch("...overrides.use_mla_backend") is one seam.

Three things the move made visible in the runtime:

flashinfer_autotune reads the published bags. It is unambiguously a post-publish reader —
it already asks get_disagg() two lines down — so the pre-publish helper would have missed a
post-publish override of chunked_prefill_size or max_prefill_tokens, which is the whole
reason the pair of spellings exists. Its test patches the two module bindings, the way it already
patches get_disagg.
post_capture_kv_sizing_planned calls use_mla_backend unconditionally. It used to prefer
getattr(server_args, "use_mla_backend", None) on the theory that ModelRunner writes that name
onto the record; it writes it onto the runner (model_runner.py:370), so the branch never fired
in production — and on a fixture still carrying the old stub, bool(callable) was True without
calling anything.
is_post_capture_kv_active keeps the resolution spelling on purpose. Unlike the other two
corrected sites it has no published sibling to ask, so the pre-publish function is the only
existing spelling — worth stating, because the pattern elsewhere is the opposite.
server_args_variant validated its keyword names against ServerArgs members, which let
use_mla_backend through only because it was a method. It is not a member — the runner computes
it on itself and the attention kits copy that bool onto the record they hand in — so the
validator names that case explicitly rather than relying on a method to exist.

Thirty accessor stubs on fake records are retired. Where a fixture stubbed
is_attention_backend_not_set, it now carries the three backend fields the predicate reads; where
it stubbed get_model_config, it seeds _model_config, which is the affordance the memo already
documented. A stubbed accessor stops intercepting the moment its member moves, so these were
hiding what the fixtures actually said.

Accuracy Tests

The 62-shape resolution probe is byte-identical to the base commit at every one of the five
commits, and again after the review round above. The registered config suites report the same failure set as main, file for file (the
differential is run per commit, not only at the tip).

Speed Tests and Profiling

None. A moved function is the same code one frame over.

Checklist

Format your code according to the Format code with pre-commit.
Add unit tests according to the Run and add unit tests.
Update documentation according to Write documentations.
Provide accuracy and speed benchmark results according to Test the accuracy and Benchmark the speed.
Follow the SGLang code style guidance.

Review and Merge Process

The largest of the four, and the one to read commit by commit — each is a single kind of move and
the boundaries are green on their own. The two defects above and the module-scope-copy seam are
the parts that are not mechanical.

CI States

Latest PR Test (Base): 🚫 Run #33249798874
Latest PR Test (Extra): 🚫 Run #33249798768
Latest PR Test (AMD ROCm 7.2): ❌ Run #33249798866

chatgpt-codex-connector · 2026-08-29T06:21:20Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-08-29T06:39:45.051306Z	`e961cff`	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9dc0fcde79

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-08-29T06:23:57Z

+    return True
+
+
+def cutedsl_moe_max_num_tokens(server_args: Any) -> int:


Update the remaining CuTeDSL budget caller

When Qwen3.5 FlashInfer MNNVL CuTeDSL fusion prepares its workspace, resolve_max_m() still calls server_args.cutedsl_moe_max_num_tokens() in qwen35_flashinfer_fusion.py:40. This commit removes that ServerArgs method and only introduces this free function, so a real ServerArgs instance raises AttributeError during prepare_qwen35_flashinfer_fusion() before CUDA-graph capture. Update that caller to invoke the new helper.

Useful? React with 👍 / 👎.

Twenty-four members of `ServerArgs` were still called back into from `arg_groups/` -- 96 call sites, 503 lines. Fourteen of them have no other caller at all: not elsewhere in `srt`, not in a test. They are resolution-time helpers that ended up on the record for historical reasons, and they move to the family that calls them. `generate_{decode,prefill,cpu}_*_batch_sizes` and `apply_cuda_graph_disaggregation_roles` join `cuda_graph_hook`; `reserve_for_graph_mb`, `reserve_for_deepep_a2a_mb` and `adjust_mem_fraction_for_vlm` join `memory_hook`; the two dispatch-token budgets join `moe_hook`; `is_mistral_native_format` joins `model_path_hook`; `is_attention_backend_not_set` and `get_default_attn_backend` join `overrides`. The two `_set_default_dsa_*` members were one-line forwards to a post-process pass, like the ninety-three slots #36792 cut, and are now that call. `ServerArgs` goes from 5277 to 4896 lines. `is_attention_backend_not_set` takes the view rather than the record. Every read in it is a view read, three of its call sites hold a view rather than a record, and nine of the other ten already had one in scope. `get_default_attn_backend` needs both overlays, so it keeps the record; the one call site that has only a view reaches the record through `record_of`. Two things this makes visible: - `get_default_attn_backend` read `tp_size` off the record -- raw input, not what resolution decided. `test_no_hook_reads_a_field_off_the_record` walks `arg_groups/` and could not see it while it was a method on the class. It reads through the view now. - `hisparse_hook._is_hip` imported `is_hip` *from `server_args`*, so that the tests would have a stable patch target. That made a configuration module the home of a platform probe. It asks `utils.common` now, and the eight patches name `hisparse_hook._is_hip`. Twelve `is_attention_backend_not_set` accessor stubs on fake records are retired. Four of them recomputed the predicate from the fake's own fields -- exactly what the function now does -- and the rest are replaced by the field values that make it true or false. A stubbed accessor stops intercepting the moment its member moves, so it was hiding what these fixtures actually said. The 62-shape resolution probe is byte-identical to the base commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

`_resolved`, `_declare` and `_late_resolution` were one-line forwards to `arg_groups.overrides`: `resolved_view`, `declare_resolution` and `declare_late_resolution`. Sixty-one call sites went through them -- a hook handed the record, then calling back into it to reach a function in its own package. Every call site now names the function. `ServerArgs` keeps no declaration member, and the record is one step closer to holding only what the operator typed. The scanners that pin the declaration channels follow. Each declarer is called by bare name now, so their node-type guards say `ast.Name`; `_DECLARERS` drops the member spelling. One assertion changes meaning rather than shape: the late channel used to be invisible to the keyword scan, because `server_args._late_resolution(...)` was not one of the declarer names, and the census asserted it added fields the keyword scan missed. The two spellings have converged, so it is now a subset by construction -- the census says that instead, and its own floor is what pins it. Forty call sites were *not* rewritten. Three test classes define their own `_resolved` / `_declare` helper, and `self._resolved(model_path, **kwargs)` in a `TestCase` is that helper, not the record's seam. A rewrite keyed on the attribute name alone takes both; the receiver has to be checked against what the enclosing class defines. The 62-shape resolution probe is byte-identical to the base commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

… leave the record `_resolved_attention_backends` and `get_attention_backends` were two names for one computation -- `attention_backends_of(resolved_view(record))` -- with fifteen call sites between them. They are gone; every site names the function. `use_mla_backend` and `should_report_expert_balancedness` become free functions in `arg_groups.overrides`, where the rest of the resolution vocabulary lives. `ServerArgs` goes from 4868 to 4844 lines and answers 25 fewer callbacks. The interesting part is the seams, not the move. `use_mla_backend` and `attention_backends_of` were bound at module scope in six and six modules. A `from ... import` name is a *copy*, so a test that patched the source module reached none of them, and one that patched a single hook reached only that one. Both are imported at their call sites now, so `patch("...overrides.use_mla_backend")` is the one seam, and the twelve tests that patched `ServerArgs.use_mla_backend` name it. Three shapes the mover could not see, each caught by a different check: - `post_capture_kv_sizing_planned` read `self.use_mla_backend` as a *value*, not a call, because `ModelRunner` overwrites the name with a bool on the record. An AST scan for calls does not see that. The probe did. - `kv_cache_hook` had `use_mla_backend = server_args.use_mla_backend()`, so the rewrite made the local shadow the function it now calls. The local is `uses_mla`. - `ModelRunner`, `kv_cache_configurator` and the attention registry carry their own `use_mla_backend` attribute -- forty-odd references that must NOT move. Six fixtures stop stubbing the accessors and supply what the functions read: a model configuration whose `attention_arch` is or is not MLA, and the three attention-backend fields. Two patch the function at its module instead. The 62-shape resolution probe is byte-identical to the base commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…cord `get_model_config` was the last big callback: fifty-six call sites reached into `ServerArgs` for it, and it is the one member that memoises -- `_model_config` plus `_model_config_built_from`, the path the memo was filled at. It becomes `arg_groups.overrides.model_config_of(record)`. The memo stays on the record, alongside `_resolved_overrides` and `_cuda_graph_config_locked`: it is resolution scratch, it has to survive the pickle to a child, and there is no per-record store anywhere else at resolution time. What leaves is the callback. `model_config_of` takes a view as readily as the record. Ten of the call sites are override providers that hold a `ResolvedView`, and a view is a read overlay of exactly one record -- unwrapping it in the function beats making ten callers reach back through it. The imports are per call site, so `patch("...overrides.model_config_of")` is one seam rather than twelve module-level copies. Eighteen fixtures stop stubbing the accessor and seed the memo instead -- `_model_config=<config>` rather than `get_model_config=lambda: <config>`. That is the affordance the memo already documented ("a configuration a fixture supplied carries no key and is handed back as it is"), so the fixtures now use the mechanism instead of standing in front of it. Two cases could not: one wanted `None` as the answer, which as a memo means "not built yet", so it patches the function; two seeded configurations made a `patch.object` redundant and it is gone. Three guards scanned for a `.get_model_config()` attribute call to find where the configuration is first built -- the pin that keeps resolution-decided fields ahead of the build. They match a bare-name call now. The 62-shape resolution probe is byte-identical to the base commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

`post_capture_kv_sizing_planned`, `cutedsl_moe_max_num_tokens` and `max_prefill_buffer_tokens` were the last members `arg_groups/` called back into. Each is a pure function of the resolving view -- no memo, no state -- and each already had a published sibling in `runtime_context` for the readers that run after publish. Only the pre-publish half was still a method. `mamba_cache_chunk_size` and `max_speculative_num_draft_tokens` follow them. Both are the same shape, and both were missed by the first census because a property read is an attribute access, not a call. Their published siblings stay where they are; the post-publish readers in `pool_configurator` and `tokenspeed_mla_backend` switch to those siblings, which is where they belonged. They all join the resolution vocabulary in `arg_groups.overrides`. `flashinfer_autotune` was on the wrong side of publish and is corrected rather than moved: it already asks `get_disagg()`, so the pre-publish helper would have missed a post-publish override of `chunked_prefill_size` or `max_prefill_tokens` -- which is what the pair of spellings exists to prevent. Its test patches the two module bindings the way it already patches `get_disagg`. `post_capture_kv_sizing_planned` now calls `use_mla_backend` unconditionally. It used to prefer `getattr(server_args, "use_mla_backend", None)` on the theory that `ModelRunner` writes that name onto the record; it writes it onto the *runner*, so the branch never fired in production -- and on a fixture still carrying the old stub, `bool(callable)` was True without calling anything. `is_post_capture_kv_active` keeps the resolution spelling: unlike the others, this one has no published sibling to ask. `server_args_variant` validated its keyword names against `ServerArgs` members, which let `use_mla_backend` through only because it was a method. It is not a member -- the runner computes it on itself and the attention kits copy that bool onto the record -- so the validator names that case explicitly. `arg_groups/` now touches **nothing** on `ServerArgs`: no method call and no property read. The record is 4663 lines and 27 `def` members here, down from 5656 and 59 on main; the two balancedness predicates that remain leave in the next PR, which is what takes it to 4653 and 25. The 62-shape resolution probe is byte-identical to the base commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…ect#36972) Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

ch-wan requested review from BBuf, CatherineSue, Edwardf0t1, Fridge003, HaiShaw, JustinTong0323, Qiaolin-Yu, Ying1123, b8zhong, fzyzcjy, hebiao064, hnyls2002, iforgetmyname, ispobock, merrymercy, mmangkad, ping1jing2, slin1237, whybeyoung and xutizhou as code owners August 29, 2026 06:21

github-actions Bot added lora Multi-modal multi-modal language model speculative-decoding hicache Hierarchical Caching for SGLang npu labels Aug 29, 2026

chatgpt-codex-connector Bot reviewed Aug 29, 2026

View reviewed changes

ch-wan force-pushed the cheng/gc-r4a-pipeline branch from 92e4892 to 4c96ae3 Compare August 29, 2026 06:35

ch-wan force-pushed the cheng/gc-r4b-callbacks branch from 9dc0fcd to e961cff Compare August 29, 2026 06:35

ch-wan mentioned this pull request Aug 29, 2026

config: the resolution pipeline's dispatcher leaves the record #36896

Merged

5 tasks

This was referenced Aug 29, 2026

config: six more runtime readers ask the bags #36973

Merged

config: the dead record parameters go #36974

Merged

config: the lazy imports that buy nothing become eager #36975

Merged

[CI vehicle, do not merge] config: round-4 ServerArgs series #36925

Closed

ch-wan force-pushed the cheng/gc-r4a-pipeline branch from 4c96ae3 to 0f598b2 Compare August 29, 2026 07:14

ch-wan force-pushed the cheng/gc-r4b-callbacks branch 2 times, most recently from 5f745be to 71dc8ec Compare August 29, 2026 07:37

ch-wan force-pushed the cheng/gc-r4a-pipeline branch from 0f598b2 to 829770e Compare August 29, 2026 08:11

ch-wan force-pushed the cheng/gc-r4b-callbacks branch 3 times, most recently from fce9c77 to 93788d5 Compare August 29, 2026 08:50

Base automatically changed from cheng/gc-r4a-pipeline to main August 29, 2026 11:16

ch-wan and others added 5 commits August 29, 2026 11:17

ch-wan force-pushed the cheng/gc-r4b-callbacks branch from 93788d5 to 8c0c14f Compare August 29, 2026 11:17

ch-wan merged commit b65e677 into main Aug 29, 2026
16 of 20 checks passed

ch-wan deleted the cheng/gc-r4b-callbacks branch August 29, 2026 11:18

kediwu0331 pushed a commit to Zhylkaaa/sglang that referenced this pull request Aug 30, 2026

config: the resolution callbacks into the record go to zero (sgl-proj…

eed69d4

…ect#36972) Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

saturn-acc pushed a commit to saturn-acc/sglang that referenced this pull request Aug 31, 2026

config: the resolution callbacks into the record go to zero (sgl-proj…

a1b47c3

…ect#36972) Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

PaddyXj mentioned this pull request Aug 31, 2026

feat: add XingChen4 model support #37228

Draft

9 tasks

chunyuan-w mentioned this pull request Sep 1, 2026

[CPU] Fix shm allreduce collision and sglang-router import #37179

Merged

nzr-niu pushed a commit to nzr-niu/sglang that referenced this pull request Sep 1, 2026

config: the resolution callbacks into the record go to zero (sgl-proj…

8e17240

…ect#36972) Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

trevor-m mentioned this pull request Sep 2, 2026

fix(sglang): Sglang compatbility fixes for get_model_config / use_mla_backend ai-dynamo/dynamo#14234

Merged

1 task

Navigation Menu

Uh oh!

config: the resolution callbacks into the record go to zero - #36972

config: the resolution callbacks into the record go to zero#36972
ch-wan merged 5 commits into
mainfrom
cheng/gc-r4b-callbacks

ch-wan commented Aug 29, 2026 •

edited by github-actions Bot

Loading

Uh oh!

chatgpt-codex-connector Bot commented Aug 29, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Aug 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		return True


		def cutedsl_moe_max_num_tokens(server_args: Any) -> int:

Uh oh!

Conversation

ch-wan commented Aug 29, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The series

Motivation

Modifications

What this makes visible

Accuracy Tests

Speed Tests and Profiling

Checklist

Review and Merge Process

CI States

Uh oh!

chatgpt-codex-connector Bot commented Aug 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 29, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ch-wan commented Aug 29, 2026 •

edited by github-actions Bot

Loading

chatgpt-codex-connector Bot commented Aug 29, 2026 •

edited

Loading