[CI vehicle, do not merge] config: round-4 ServerArgs series by ch-wan · Pull Request #36925 · sgl-project/sglang

ch-wan · 2026-08-28T22:53:06Z

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
#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 ← you are here

Motivation

The five PRs of this series are stacked, and each targets the one below it. That is the right shape
for review — every boundary is self-sufficient and green on its own — but it means no single PR ever
runs CI against the whole change, and the labelled runs are expensive enough that putting them on
all five is waste.

This branch is one empty commit on top of the last PR of the series, targeting main. A run here
exercises all five at once, against the same base they will land on.

It carries no change of its own and is not for merge. git diff main... on this branch is
exactly the union of the five; the eleventh commit is empty.

Modifications

cheng/gc-r4-ci                 (empty commit — this branch)
└─ cheng/gc-r4e-eager-imports  1 commit
   └─ cheng/gc-r4d-dead-params 1 commit
      └─ cheng/gc-r4c-bag-readers 2 commits
         └─ cheng/gc-r4b-callbacks 5 commits
            └─ cheng/gc-r4a-pipeline 1 commit
               └─ main

The five are linked at the top of this description.

This is the only PR of the series that carries run-ci / run-ci-extra / bypass-fastfail. The
five review PRs carry none of them, so their gate jobs report as skipped rather than run — that is
by design, not a failure to look into.

What the series does

ServerArgs stops being an object that resolution and the runtime call back into. It goes from
5656 lines and 59 def members on main to 4653 and 25; arg_groups/ touches nothing on it —
no method call and no property read — where a census found 96 call sites across 24 members.

Accuracy Tests

Locally, per boundary: the 62-shape resolution probe (resolve_once() + check_server_args(),
exception type and full message) is byte-identical to main at each of the five branch tips,
and each tip also passes a scope check — every touched file parses, no conflict marker survives,
the package imports, and ruff reports nothing this series introduced. The registered config
suites pass at each boundary.

A parallel differential over 565 registered test files (tip vs main, same machine, same shards)
reports the same failure set on both sides.

The point of this branch is the part that cannot be checked locally — the GPU, AMD, NPU and
disaggregation jobs.

Speed Tests and Profiling

None. A dummy resolution is unchanged at ~9 ms and loads the same six arg_groups modules.

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

Do not merge. Close it once the series has landed; read the five PRs it sits on instead.

CI States

Latest PR Test (Base): 🚫 Run #33244103801
Latest PR Test (Extra): ❌ Run #33244103690
Latest PR Test (AMD ROCm 7.2): 🚫 Run #33244103855

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e35f10448b

ℹ️ 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:24:30Z

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


Route Qwen fusion through the extracted token-bound helper

When the Qwen3.5 FlashInfer MNNVL CuTe DSL fusion is enabled, prepare_qwen35_flashinfer_fusion() calls resolve_max_m(), which still invokes server_args.cutedsl_moe_max_num_tokens() on a ResolvingConfig. That view delegates to the underlying ServerArgs, but this commit deletes that member in favor of this free function, so model initialization raises AttributeError before preparing the fusion workspace. Update that remaining call site to invoke the extracted helper with the underlying server arguments.

Useful? React with 👍 / 👎.

`_run_resolution_pipeline` was the last piece of resolution living on `ServerArgs`: 331 lines of ordered dispatch plus the four handler methods it still reached. Every step it calls already lives in `arg_groups/`, and none of them is a member of the record -- a step decides *about* the record. The dispatcher becomes `arg_groups.pipeline.run_resolution_pipeline`. Three of the four remaining handlers were one-line forwards to `run_post_process_pass`, like the ninety-three the previous PR cut, and are now that call; the fourth is the MLX opt-in check, which joins the platform family as `handle_hardware_runtime_validation`. `resolve_once` keeps the gate and calls the function. `ServerArgs` loses 366 lines (5643 -> 5277) and hosts no resolution handler at all. `test_no_handler_reads_a_field_off_self` said a handler on the record must not read a raw field off `self`; there is none left, so the guard becomes the stronger `test_the_record_hosts_no_resolution_handler`, and the package-side `test_no_hook_reads_a_field_off_the_record` covers what the steps read. Five guards follow the dispatcher to its new file: the two that count pipeline entries patch the module rather than the class, the "one caller" scan matches a bare-name call, the laziness and ModelConfig-ordering checks parse `arg_groups/pipeline.py`, and the platform write-capture plugin is installed in the namespace the capture now runs in -- without that last one the plugin's defaults silently stopped reaching the projection. The MLX check runs before the dummy short circuit, so `platform_hook` is imported at its call site like every other family. A dummy resolution loads two more modules (`pipeline`, `platform_hook`) and is unchanged at ~9ms. The 62-shape resolution probe reports byte-identical results. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

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>

`ExpertDistributionRecorder` was handed a `ServerArgs` and threaded it through six constructors -- recorder, gatherer, accumulator and their subclasses -- to reach four reads. Two of the classes stored it; one subclass took it and never touched it. The file already asked `get_exec().moe` for the recorder mode, so the record was travelling alongside a bag that already answered. The four reads go to the bag, and the parameter goes with them: five call sites in `model_runner.py`, six signatures, two stored attributes. `eplb_min_rebalancing_utilization_threshold` is an `exec.moe` leaf, so it is a direct bag read. The three balancedness predicates were members of the record -- `should_log_expert_balancedness_to_server_log` and its Prometheus sibling, read from four places across three files, each through a record someone was holding for that purpose. They become published accessors in `runtime_context` (`reports_expert_balancedness`, `logs_...`, `exports_...`), the way `uses_mla_backend` and `cutedsl_moe_max_num_tokens` already pair with their pre-publish equivalents. Both members are gone from `ServerArgs`. `resolve_collector_class` was the one thing keeping the record in the chain: it took a `ServerArgs` to read `stat_loggers`, which is an `observability` leaf. Three of its six callers were reaching for `get_server_args()` to pass it straight back. It asks the bag now, and `server_args=None` -- which meant "no config, use the default" -- becomes "the namespace is not published", which is the same case said in the context's own terms. `TestResolveCollectorClass` drove that with a `_StubArgs` stand-in. A stand-in proves nothing about a function that reads a bag, so the cases force the leaf through `override_server_args` and the stand-in is deleted. The 62-shape resolution probe is byte-identical to the base commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The EPLB recorder was one instance of a shape. A census of the whole tree finds 295 raw-field reads off a held `ServerArgs` in 53 files -- every field of them in a published namespace. Not all are convertible: the launcher trio (`http_server`, `engine`, `data_parallel_controller`) reads the record *before* its process publishes, and those reads have to stay. These six do not. Each already reads its other configuration from a bag; the raw reads are what was left behind: hybrid_pool_assembler 6 hicache_host_memory_mode -> memory scheduler 3 hicache_host_memory_mode -> memory kt_ep_wrapper 7 six kt_* leaves -> exec.moe expert_backup_client 4 nnodes, node_rank -> parallel dspark_planner 3 three speculative_dspark_* -> spec prefill_delayer 3 three prefill-delay leaves -> schedule Three of them were carrying the record only for those reads, so it goes: `PrefillDelayer` and `ExpertBackupClient` lose the constructor parameter and the stored attribute, and `build_sps_cost_table` -- whose body was already entirely `get_spec()` and `get_schedule()` -- loses its keyword-only `server_args`, which takes it out of `DsparkPlanner` too. Two `ServerArgs` imports were left with no user. `kt_ep_wrapper` and `hybrid_pool_assembler` keep theirs: the first still calls `get_hf_config()` on the record, the second is handed one for other reasons. The 62-shape resolution probe is byte-identical to the base commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

A census of the tree finds 256 functions that take a `server_args` they never read. Most are not dead: `multimodal_gen`'s pipeline stages take it as an interface contract -- 72 implementations of `forward` use it and 33 do not, so the parameter belongs to the signature, not to the body. The same holds for `SRTPlatform.apply_server_args_defaults` (implemented out of tree), `CustomSpecAlgo.handle_server_args`, the model-override providers, and the `StackStrategy.build` family. Ten in `srt/` are genuinely dead -- free functions called by name, nothing polymorphic about them: generate_prefill_cuda_graph_batch_sizes validate_ib_devices validate_buckets_rule handle_hardware_runtime_validation build_kv_only_stack build_hybrid_swa_stack build_hybrid_mamba_stack build_hybrid_mamba_swa_stack build_anchor_sidecar_stack build_deepseek_v4_hicache_stack The parameter goes from each, and the argument from all eighteen call sites. The eleventh candidate was not dead, and it is #36921: `create_kt_config_from_server_args` reached the record through `server_args.get_hf_config()`, a method `ServerArgs` does not have, under a bare `except Exception: pass` -- so `num_layers` was always None and the last-layer branch never fired. That fix landed on its own; what is left here is its call site following `get_model_config` out of the record. The 62-shape resolution probe is byte-identical to the base commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

`arg_groups.overrides` is imported at module scope by thirty-five files already, `server_args` among them, so importing it is not a cycle and not a cost -- by the time any resolution step runs, the module is loaded. Ninety-three `from ...overrides import` statements were nonetheless written inside the functions that use them. The ones that buy nothing become module-scope imports, including the twenty-three that were **mixed**: a name a test patches, plus names riding along on the same statement that nothing patches. Splitting those frees the riders -- the `_dsa_*` pass callables are the shape of it, only ever handed to `run_post_process_pass` as arguments, never looked up through the module. Three groups were kept lazy for a test seam, which is the wrong way round, and each is fixed at the test rather than in the code the runtime executes: - The four declaration paths. `test_no_entry_changes_after_it_is_recorded` wrapped `declare_resolution`, `declare_late_resolution`, `declare_direct_writes` and `run_post_process_pass` by `setattr` on the module, purely to snapshot the stash after each append. The property it checks belongs to the stash, so the seam is now the stash: a list that snapshots on append, installed by a test-local `ServerArgs` subclass that survives the pipeline's own reset of it. The guard is strictly stronger -- it records at the append rather than after the call -- and still fails on an injected in-place edit. - `use_mla_backend`. Eight cases patched the answer; it is `model_config_of(server_args).attention_arch == AttentionArch.MLA`, so the fixtures now state the model instead. Five of the patches were redundant outright -- their stand-in already answered "not MLA" -- and the rest set `_model_config.attention_arch`, which is the input the value is derived from. - `model_config_of` had no patch site at all and was lazy on a hunch. What is left is load-bearing, for exactly two reasons and nothing else: `attention_backends_of` (3 patch sites) and `supports_mamba_cache_extra_buffer` (2) are still patched through the module, and `runtime_context` is imported by `utils.common`, which `overrides` needs, so it sits *below* `overrides` and cannot import it at module scope -- a cycle, not a preference. A function-local `overrides` import that is neither of those: zero. A dummy resolution is unchanged at ~9 ms and loads the same six `arg_groups` modules; the 62-shape resolution probe is byte-identical to the base commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Placeholder commit. This branch is one empty commit on top of the last PR of the series, so a run here exercises all five against main at once. It carries no change of its own and is not for merge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

ch-wan · 2026-08-29T11:22:25Z

The series has landed (#36896, #36972, #36973, #36974, #36975). This vehicle carried no change of its own; closing as designed.

ch-wan added run-ci run-ci-extra bypass-fastfail labels Aug 28, 2026

ch-wan requested review from HydraQYH, sogalin, wisclmy0611, yuan-luo and zijiexia as code owners August 29, 2026 06:12

ch-wan force-pushed the cheng/gc-r4-ci branch from 46b5c49 to e35f104 Compare August 29, 2026 06:21

chatgpt-codex-connector Bot reviewed Aug 29, 2026

View reviewed changes

ch-wan force-pushed the cheng/gc-r4-ci branch from e35f104 to aa3a1da Compare August 29, 2026 06:35

ch-wan force-pushed the cheng/gc-r4-ci branch 2 times, most recently from 8f6f347 to 53e93ad Compare August 29, 2026 07:37

ch-wan removed the run-ci-extra label Aug 29, 2026

ch-wan force-pushed the cheng/gc-r4-ci branch 2 times, most recently from 63c401b to 30b4655 Compare August 29, 2026 08:32

ch-wan and others added 10 commits August 29, 2026 08:44

Navigation Menu

Uh oh!

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

[CI vehicle, do not merge] config: round-4 ServerArgs series#36925
ch-wan wants to merge 11 commits into
mainfrom
cheng/gc-r4-ci

ch-wan commented Aug 28, 2026 •

edited by github-actions Bot

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Aug 29, 2026

Uh oh!

ch-wan commented Aug 29, 2026

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 28, 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 the series does

Accuracy Tests

Speed Tests and Profiling

Checklist

Review and Merge Process

CI States

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!

ch-wan commented Aug 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ch-wan commented Aug 28, 2026 •

edited by github-actions Bot

Loading