config: retire the last process-global config field reads

[ch-wan]

What

get_server_args().<field> reads the startup record of one process. Twenty-six of
those reads were left in srt; this removes every one that has somewhere better to
go, and pins the rest.

Nine reads move to the namespace accessors — the value they want is the resolved
one, including post-publish overrides: attention_backend ×5 →
get_exec().kernel, skip_tokenizer_init ×2 → get_serving(), the draft-aware
load_format → get_model(), and a PP size → get_parallel() (live topology, which
is what the DSA indexer wants).

Two reads move to the instance instead of a namespace. The multimodal processor
picked its fast-image-processor device from the process-global config, but
base_gpu_id differs per worker — the encode-server DP workers each specialise their
own copy — so no process-global value can stand in for it, and several Engines can
share a tokenizer process. The decision moves into
BaseMultimodalProcessor._fast_image_processor_device, reading the instance the
processor was constructed with, like the ten other config reads in that class. Branch
order, the NPU preprocess patches and the case that leaves device unset are
unchanged.

What stays is the derived API: mamba_cache_chunk_size (@property),
get_model_config(), enable_mamba_extra_buffer{,_lazy}() — computed from several
fields plus the HF config, so they are not namespace leaves and ServerArgs is their
only home. Two config-intent reads of live-shadowed sizes also stay: get_parallel()
shadows tp/pp/dcp/attn_cp/moe_dp_size with the live topology, and both sites run
where no process group exists (cuda_ipc_transport_utils even guards for "not
published yet").

A new AST ratchet pins field reads at zero and exempts the above by name, with the
reasoning in its docstring.

Test changes

test_dllm_fdfo_kv_reuse rebound allocation.get_server_args to a
SimpleNamespace. That kind of stand-in stops intercepting the moment a reader
migrates — which is exactly what happened here — so it publishes a real config now.
New test_processor_device_selection.py pins the per-instance device resolution
against a conflicting published config.

Validation

Per-area unit suites (mem_cache, multimodal, batch_overlap,
layers/attention, managers) and all ratchets pass. Full registered CPU battery
(16 partitions) against main: seven branch-only failures, each re-run serially and
green — two were port collisions from the parallel run itself, the rest are the
box's standing environment failures. No new failures.

Not covered locally: the flipped reads sit in model and attention paths that need a
GPU to execute; the flips are value-preserving (the bags are projected from the same
published config), but the speculative and model CI suites are the real check.

---

CI States

Latest PR Test (Base): ❌ Run #30771346854
Latest PR Test (Extra): ❌ Run #30781140569

[gemini-code-assist]

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

[ch-wan]

Part of the writer-ratchet stack: #33238 → #33239 → #33240 → #33241 → #33244, reviewed together in #33242. Base is the previous PR's branch; review/merge in order.

[ch-wan]

Summary

This PR cleanly finishes the direct get_server_args().<field> burndown inside srt/: bag migrations use the right namespaces (exec.kernel.attention_backend, serving.skip_tokenizer_init, model.load_format), DSA’s pp_size correctly moves to live get_parallel() (already paired with get_pp_group()), and the multimodal base_gpu_id / rl_on_policy_target fix to self.server_args is the right multi-Engine / per-worker shape. The processor device unit test actually fails on a last-publish-wins regression, and the allocation test correctly publishes real config instead of monkeypatching get_server_args. Dominant residual risk is the new ratchet’s narrow AST shape: it pins only bare get_server_args().attr, so intermediate rebinding (and a few pre-existing config-intent size reads written that way) can still grow without tripping CI.

Issue counts by severity

• bugs: 0
• suggestions: 1
• nits: 1

Issues outside the diff

These findings reference lines that are not present in the diff and could not be posted as inline comments:

• [suggestion] test/registered/unit/test_global_config_read_ratchet.py:462 -- The ratchet only matches get_server_args().<attr> when the call is a bare ast.Name (value.func.id == "get_server_args"). Process-global field reads still exist via intermediate rebinding and are invisible to the pin, e.g. server_args = get_server_args(); … server_args.speculative_algorithm in spec_utils.py, and the config-intent tp_size read in mm_utils._acknowledge_deferred_cuda_ipc_cache_hits (server_args = get_server_args() then getattr(server_args, "tp_size", …)). The same shape also escapes if written as runtime_context.get_server_args().field. Baseline _BASELINE = 0 therefore overstates “zero field reads”: it is zero for one syntactic form, while config-intent size exemptions only list the two direct sites (allocation.dcp_size, cuda_ipc.tp_size) and not the intermediate mm_utils tp_size sibling. A future “simple” reintroduction of a field read via sa = get_server_args(); sa.foo would pass CI.
  • Suggestion: Either (a) expand the walk to treat Name = get_server_args() then later Name.attr as a field read (with the same derived/config-intent exemptions, and add mm_utils tp_size to _CONFIG_INTENT_SIZES), and/or (b) document in the module docstring that only the direct call form is ratcheted and that intermediate rebinding is an intentional remaining escape that code review must catch. Prefer (a) if the goal is a real zero pin.
• [nit] test/registered/unit/test_global_config_read_ratchet.py:425 -- The scan root is sglang/srt only. That matches the burndown surface of this PR, but a package-wide get_server_args().<field> still remains outside it (e.g. python/sglang/kernels/ops/layernorm/mhc.py reads chunked_prefill_size). Not a regression in the migrated sites; just a scope gap relative to the “last process-global config field reads” framing.
  • Suggestion: Either extend the walk to sglang (or at least sglang/kernels) once ready, or note the srt-only scope explicitly next to _SRT_ROOT so the next burndown PR knows what is still open.

[ch-wan]

Round 2 — you and @chatgpt-codex-connector are both right, and this one was my misjudgement: fixed in 67e501aae3.

I flipped pp_size to get_parallel() on the reasoning "this runs inside the model runner, so live topology is what it wants" — without checking that Indexer is also constructed before distributed init, which test/registered/kernels/ops/attention/test_dsa_indexer.py does (publish-only, no PP group). The live property would have raised there. That is exactly the case the skill's live-shadowed rule covers, and I walked past it.

Reverted to get_server_args().pp_size and added the site to _CONFIG_INTENT_SIZES next to allocation.dcp_size and cuda_ipc.tp_size, so the ratchet records it as a deliberate config-intent read rather than an oversight. It is a CUDA-registered test, so my CPU battery never covered it — thanks for catching it by reading.

[chatgpt-codex-connector]

Summary

• Updated the global-config-read ratchet documentation to accurately describe all three deliberate config-intent size exemptions, including the restored DSA pp_size read used before process-group initialization. test/registered/unit/test_global_config_read_ratchet.pyL21-L24
• Committed the follow-up as 3b38bd5 (test: clarify config-intent ratchet exemptions).
• Created a follow-up pull request titled “test: clarify the three config-intent ratchet exemptions.”

Testing

• ✅ python -m py_compile test/registered/unit/test_global_config_read_ratchet.py
• ✅ git diff --check
• ⚠️ PYTHONPATH=python python -m pytest -q test/registered/unit/test_global_config_read_ratchet.py (test collection could not complete because the environment lacks the required numpy dependency)
• ✅ git status --short --branch (working tree clean)

View task →

[ch-wan]

Re-review (Round 2 — 67e501aae3)

Read the author reply and verified on the new head.

DSA pp_size fix is correct. Reverted to config-intent get_server_args().pp_size and listed in _CONFIG_INTENT_SIZES next to allocation.dcp_size / cuda_ipc.tp_size — matches the live-shadowed rule and keeps publish-only Indexer construction working.

Prior Round-1 ratchet notes remain addressed (direct 0 / alias 12, package-wide scan, mhc.py → get_schedule()). Residual uncounted shapes (instance stash / param rebind / getattr) stay as next-slice work, not blockers.

No open correctness issues from this pass.

[ch-wan]

Review-round fixes pushed (3e23b4156d). Two things from the review of this PR:

The body claimed a pp_size flip that is not in the diff — corrected. dsa_indexer.py keeps get_server_args().pp_size and the ratchet exempts it; the flip existed in an earlier revision and was reverted when the Indexer was found to be constructed before distributed init.

The exemption rationale was wrong, though not for the reason given. The review argues the PP group must exist because get_pp_group().is_last_rank is on the next line — but that line is pp_size > 1 and not get_pp_group().is_last_rank, which short-circuits: with PP off the group is never touched, and that is exactly what lets the Indexer be built with no distributed state. The live property would demand the group either way. So the exemption stands; its stated reason ("both exempt sites run where no process group exists") did not, and the docstring now gives a per-site reason for all three:

• DSA indexer: the config read is what allows the short circuit.
• allocation.dcp_size: asks whether DCP was configured; the live property goes through get_dcp_group(), which is only installed when DCP is on (get_dcp_world_size() asserts otherwise).
• cuda_ipc_transport_utils.tp_size: tokenizer process, no groups at all; the call site already guards for "not published yet".

Also from the review: initialize_linear_attn_config is called with a keyword now, and the dllm fixture's allocation.support_triton monkeypatch is gone — you were right that it is redundant, support_triton("torch_native") is already False.

[ch-wan]

Closing unmerged and reopening against main directly.

GitHub classifies a chained-base series as a stack, and in that mode it refuses base retargeting (Cannot change the base branch because the pull request is part of a stack), the classic merge API (must be merged using the asynchronous merge REST API), and the plain REST merge (403). The async endpoint accepts the request but honours branch protection, and it has no bypass parameter — so this series could not be merged in order.

The replacement PR carries the identical commit; the review history, the six rounds of comment triage and the validation notes stay here for reference. Link posted below.

[ch-wan]

Reopened as #33338 (base main, identical commit). Review context stays here.