config: the record is not an object that gets passed around

[ch-wan]

Motivation

PR 5 of a five-PR series on top of the raw-input ServerArgs work (#36250–#36255), based on f775db03aaa. Each builds on the previous one; review them in order.

1. cheng/gc-p1 — config: resolution declares, and nothing writes a field
2. cheng/gc-p2 — config: every handler declares its cuda-graph decisions
3. cheng/gc-p3 — config: a parallel leaf with no live counterpart is read bare
4. cheng/gc-p4 — config: a parallel size has one spelling; a patched scope declares its own
5. cheng/gc-p5 — config: the record is not an object that gets passed around ← this PR

They are grouped by how they have to be read, not by topic: PR 3 is 118 files of one mechanical rewrite, reviewed by checking the rule and sampling; PR 4 is the design change that rewrite made possible, and its production files each need reading.

CI for the whole series runs on a separate vehicle PR, whose branch sits one placeholder commit above PR 5: #36623.

Three shapes of the same claim, plus the payoff that only appears when the whole
stack is together.

Modifications

``f04076d24f5 · 23 files (16 production, 7 test) · +244 / −125

1. A function does not take the record it never reads

Seven module-level functions took server_args and never named it. Each keeps a
reference to the whole record alive across a call boundary and reads as an
invitation: the next person who needs one value takes it off the parameter that
is already there.

They cascade. Removing one uncovers the next, because the caller only had a
record in order to pass it along:

release_req -> retract_all -> the scheduler call site
build_kv_host_pool -> build_kv_only_group -> build_hicache_draft_sidecars
                   -> maybe_register_hicache_draft

Iterated to a fixpoint: the initial census of 7 becomes 13 signatures.

Nine class methods still carry a server_args they never name, and the ratchet
walks module-level functions only — a method body is not evidence about its
contract. Some are plainly contract-bearing (BaseKVManager.__init__,
RadixCacheCpp.__init__, the plugin hooks
SRTPlatform.apply_server_args_defaults and CustomSpecAlgo.handle_server_args,
the StackStrategy.build / _MiniMaxSparseStrategy.build pair). Three are not
obviously anything — _DetailSinglePassGatherer.__init__,
TokenizerMetricsCollector.__init__,
RayDataParallelController.launch_tensor_parallel_group — and are exempt by the
rule rather than by argument. They are left for a pass that can look at each
class.

test_dead_server_args_parameter_ratchet.py pins the module-level census at
zero.

A mistake worth knowing about: the cascade script deleted parameters by line,
and load_kv_cache_scales had *, model, server_args: ServerArgs, kv_cache_dtype: str on one line — the whole line went, taking two live
parameters with it. Caught by running ruff --select F821 over the package and
comparing against origin/main (4 undefined names here, 0 there). Restored. A
lint pass also removed a now-unused ServerArgs import from schedule_batch.py
that elastic_ep.py was re-importing through; that importer now names the real
module.

1b. A worker publishes before it reads its own configuration

init_multi_tokenizer unpickles the record from shared memory and asserted on
server_args.api_key above its publish. Publish moves above the assert and the
assert reads get_serving().api_key; nothing between them writes, so there is
nothing to order against it.

test_publish_precedes_bag_reads.py cannot see this one — its own docstring
names the gap: "a publish moving across code that reads only the handed
server_args instance. Such code names no accessor, so there is no read for the
walk to order it against." It is ordered now because the read became an accessor.

2. Not done: the launch stand-in is left exactly as it was

RuntimeContext.override_server_args builds a bare record, resolves it,
declares the caller's fields late, and also writes them onto the record.
That last write is the one remaining use of _apply_fields, and it is unchanged
here.

Two rewrites of it were tried and both were wrong for a refactor, so this records
them rather than repeating them. Passing the caller's values to the constructor
makes resolution run over them — which rejects a value validation would reject
and derives dependent values, but also fires the handlers' process-wide side
effects (configure_media_url_security, the DG_* env writes) with nothing to
put them back, and turns a caller-supplied model_path into a network fetch.
Declaring them without the write keeps all of that away, but then the record a
test is handed no longer carries the values, and seven registered test files
break because the code under test reads them off it.

The current shape has neither problem: it declares (so the projection and the
bags see the values) and writes (so a handed record carries them), and the
caller of a test hook is the operator, so their values genuinely are both. See
"The payoff, and the one exception left" below for what closing it would take.

3. The record grows no attribute the projection cannot see

Three publicly-named non-field attributes were written on ServerArgs. Each is
invisible to every other guard — the namespace coverage walks fields, the
projection walks fields, the read ratchets watch field reads — which is how three
of them accumulated.

• moe_ep_size: written once, read once, in a log f-string. Every real reader
  reads the live group. Dropped; the log names the value that was declared.
• grpc_worker_threads: env-derived resolution output living outside the
  projection. Now a real field with Arg(no_cli=True) and NS("serving"),
  declared by the handler that reads the env; the validation reads the view and
  the one consumer reads the bag.
• model_config: the get_model_config() cache, public-named, which forced
  _CACHE_SLOTS to exist as a named exception in the read-only guard. Renamed
  _model_config; _CACHE_SLOTS is deleted.

test_no_public_non_field_slot.py is the census the backlog asked for.

Also here: seven object.__setattr__ calls that write a leading-underscore
non-field name, where plain assignment already passes the guard. The six that
are required (field names on an already-resolved copy; the tail of __setattr__
itself; ResolvedView.__init__, whose class refuses assignment) are left alone.

The payoff, and the one exception left

_apply_fields was the write channel that let resolution put its answers back on
the record. #36618 removed its pipeline caller, so no production path writes a
record field after resolution any more: the record holds the operator's input
because nothing is left that could change it, not because a guard is holding a
line.

The helper itself stays, with exactly one caller:
RuntimeContext.override_server_args, the test stand-in. That hook publishes a
context for a test that needs bags, and it both declares the caller's values and
writes them onto the record — the caller there is the operator, so their values
are the record's input as well as resolution's answer. It lives in arg_groups/,
which the mutation ratchet exempts by module.

This is stated rather than fixed on purpose. Closing it means changing the code
under test, not the hook: a handful of call sites read a config value off a
ServerArgs they were handed (pool_configurator, base_grammar_backend,
scheduler_hicache_attach, …), which is a category the read ratchet
deliberately does not count and which needs its own pass to convert to bag
reads. Doing it here would widen this PR by exactly the amount that makes it
unreviewable.

Accuracy Tests

No model-output change: this series moves where a configuration value is read
from, not what resolution decides. The equivalent check for that claim is a
resolution dump — every field's resolved value for 24 launch shapes (plain, tp2,
tp4_pp2, dp2, EAGLE, NEXTN, page32, page64_chunk2k, cuda-graph knobs,
disaggregation, deterministic, hierarchical cache, symmetric memory, …) — taken
in both trees and compared field by field:

0 differences across 24 shapes × 478 shared fields, against f775db03aaa.

The one field the series has and the base does not is grpc_worker_threads: on
main it is a public non-field slot assigned in _handle_deprecated_args, and
this series makes it a declared field. Its value is 4 on both sides.

Every guard also runs at each commit of the series, not only at the head — a
PR that is green only on top of its successors is not reviewable on its own. The
set is the config guards plus every registered test the series touches, ~33 files
per boundary, all green.

No GPU accuracy run. Everything above is CPU-side: resolution, projection and the
guards. A launch-path change that only shows up with real process groups is not
covered by any of it.

Speed Tests and Profiling

No benchmark run, and none is expected to move: nothing here changes a kernel, a
schedule, or the shape of any batch. What changes is the source of a
configuration read — a published dataclass attribute instead of a process-group
getter or an accessor hop.

The one place that could have mattered is torch.compile: gate helpers read
parallel leaves inside compiled forwards, and object.__getattribute__
graph-breaks. That was measured rather than assumed — the reads this series
introduces trace under torch.compile(fullgraph=True), which is pinned by a
regression test.

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

1. Ping Merge Oncalls to start the process. See the PR Merge Process.
2. Get approvals from CODEOWNERS and other reviewers.
3. Trigger CI tests with comments or contact authorized users to do so.
   • Common commands include /tag-and-rerun-ci, /tag-run-ci-label, /rerun-failed-ci
4. After green CI and required approvals, ask Merge Oncalls or people with Write permission to merge the PR.

🤖 Generated with Claude Code

---

CI States

Latest PR Test (Base): 🚫 Run #33110923787
Latest PR Test (Extra): 🚫 Run #33110923514
Latest PR Test (AMD ROCm 7.2): 🚫 Run #33110923733

[ch-wan]

@codex

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3af172c613

ℹ️ 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]

Revalidate divergent values before publishing

When an override supplies a value that resolution replaces before validating, this late declaration restores the caller's value without running that validation against the final configuration. For example, override_server_args(grpc_port=50051, grpc_worker_threads=0).install() validates the environment-derived worker count (normally 4), then declares 0 here and publishes it despite _handle_deprecated_args requiring the count to be at least 1. This lets tests silently install configurations that a real launch rejects; validate the divergent result before publishing or preserve the validated resolution for such fields.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dfcc101d00

ℹ️ 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]

Preserve override values for direct record consumers

When an overridden field is still intentionally read from the returned ServerArgs, declaring it only in the resolution stash leaves that field at its dummy default. A concrete in-repo case is test_real_path_loads_table: it overrides speculative_dspark_sps_table_path, but build_sps_cost_table reads server_args.speculative_dspark_sps_table_path directly, so it now sees None and returns the uninitialized table instead of loading the JSON file. Keep caller values on the raw test record or migrate every such consumer to the published bag before removing the field writes.

Useful? React with 👍 / 👎.