diff --git a/.claude/skills/sglang-runtime-context/SKILL.md b/.claude/skills/sglang-runtime-context/SKILL.md index 07c534460..cd668fa1c 100644 --- a/.claude/skills/sglang-runtime-context/SKILL.md +++ b/.claude/skills/sglang-runtime-context/SKILL.md @@ -10,7 +10,7 @@ One container owns process-static runtime state: `sglang.srt.runtime_context.Run | Tier | Accessor | Holds | Lifecycle | |------|----------|-------|-----------| -| raw config seed | `get_server_args()` | the published **pristine** `ServerArgs` (resolved-at-startup record; kept for debugging, dumps, per-runner fork copies) | published at process entry; re-publish is **last-publish-wins** (in-process tokenizer build, multi-Engine) and re-projects the bags; read-only | +| raw config seed | `get_server_args()` | the published `ServerArgs` — the startup record, for debugging, dumps and provenance. **Business code does not read fields off it**: the read ratchet pins that at zero, and "Reading config: the seed is off limits" below says what to read instead, which forms the ratchet sees, and what is outside it by construction (a runtime-computed name; a whole-object hand-off) | published at process entry; re-publish is **last-publish-wins** (in-process tokenizer build, multi-Engine) and re-projects the bags; read-only | | resolved config | `get_exec()` `get_memory()` `get_schedule()` `get_model()` `get_spec()` `get_serving()` `get_observability()` `get_disagg()` `get_lora()` `get_mm()` `get_device()` | namespace **config bags** — the single source of truth for resolved config; leaves are real attributes (dynamo-traceable) | projected from `server_args` at `publish`; mutated only via `get_context().override` | | runtime flags | `get_flags()` | state that is *not* a pure function of config: `capture` (cuda-graph lifecycle), `moe` (ACTIVE backends, swappable), `dp` (DP-attention runtime flags) | materialized at subsystem init; groups offer `override()` for tests | | resources | `get_resources()`, `get_stream(name)`, `get_buffer(name, factory)` | process-level handles: graph pools, EPLB state, EP dispatcher state, named side streams, workspace buffers | lazy; cleared by `reset_context()` | @@ -79,21 +79,34 @@ re-projects its own bags, so a parent-side override is lost. Values that feed construction before any bag exists (group init reads `server_args.tp_size`) have no bag to override at all. -- **Nested publishes**: a construction step that must publish a private copy wraps - itself in `get_context().preserve_config()` — the enclosing lifecycle, including its - post-publish overrides, is value-snapshotted and reinstated on exit. The draft build - no longer needs it: per-runner values are constructor arguments now. +- **Nested publishes**: `get_context().preserve_config()` snapshots the enclosing + lifecycle (including its post-publish overrides) and reinstates it on exit. No + production caller is left — the draft build was the last one, and per-runner + values are constructor arguments now — so it survives for tests and for a future + construction step that genuinely has to publish a private copy. ### Reads that legitimately stay on a `ServerArgs` instance -- **Per-runner (fork) fields** — fields the draft-worker deepcopy rewrites - (`attention_backend`, `prefill/decode_attention_backend`, - `speculative_draft_attention_backend`, `skip_tokenizer_init`, `context_length`, - `load_format`, `json_model_override_args`, `kv_cache_dtype`): each runner's copy is - authoritative for that runner, so runner code reads `self.server_args.X`, and - *resolved* per-runner values live as runner attributes - (`model_runner.kv_cache_dtype_str` is the pattern — threaded to consumers as - constructor args, never backfilled onto shared objects). +- **Per-runner values** — there is no per-runner `ServerArgs` any more. The + draft-worker config copy is gone: every worker (`TpModelWorker`, the draft + workers in `speculative/`) is handed the *same* instance the process published, + so `self.server_args.X` and the bag leaf agree **at publish** — a + post-publish `override` moves only the bag, which is exactly why a field that + is process-wide config (`attention_backend`, `skip_tokenizer_init`, + `kv_cache_dtype`) reads from the bags like any other, and why a residual + instance read on this path is stale the moment someone overrides that leaf. + What is genuinely per-runner travels two ways, neither of them a config + instance: **constructor arguments** (`ModelRunner(draft_attention_backend=...)`, + `MMEncoder(gpu_id=...)`) and **runner attributes holding the resolved value** + (`model_runner.kv_cache_dtype_str`, `prefill_attention_backend_str`, + `num_fused_shared_experts`) — threaded to consumers as arguments, never + backfilled onto a shared object. The one sanctioned bend in that rule is + *scoped*: `ModelRunner._load_format_scope` exposes the draft's + `--speculative-draft-load-format` through `get_model().override(load_format=...)` + for exactly the duration of the draft build, because model construction + reads that bag leaf — the override restores on exit, so nothing outlives + the scope. When there is a runner in hand, read its + stamp; that is a different rule from "read the instance". - **Per-instance boundaries** — the tokenizer-manager family, everything under `entrypoints/`, and the tokenizer-process multimodal processors read `self.server_args`: several `Engine`s can share one process, and the process-global @@ -112,15 +125,128 @@ bag to override at all. Config leaves (`nccl_port`, `enable_dp_attention`, `dp_size`, `ep_size`, `dwdp_size`, ...) resolve through the parallel bag; live topology (`tp_size`, `attn_tp_group`, ranks) are `@property` and **win on name collisions**. Five topology -sizes are live-shadowed (`tp/pp/dcp/attn_cp/moe_dp_size`): a config-intent read of -those must stay on `server_args.X` — the live property always wins on the accessor. -Fail-loud is narrower: before dist init, any live size/group read raises; after it, +sizes are live-shadowed (`tp/pp/dcp/attn_cp/moe_dp_size`): the live property always +wins on the accessor, so a config-intent read of those goes through +`configured_tp_size()` / `configured_pp_size()` / `configured_moe_dp_size()` / +`configured_attn_cp_size()` (DCP: the live `get_parallel().attn_dcp_size` / +`.dcp_enabled`, which are safe with no group installed — they answer `1` / +`False` — but report the *effective* topology, never the requested size; +a config-intent read would need its own accessor, which no call site +requires today). A process-global seed field-read of one of these +sizes (`get_server_args().tp_size`, or an alias of it) is a read-ratchet failure; the +sites that legitimately go around the live property are the `configured_*_size()` +readers, and those are what the ratchet registers, each with its reason +(`_CONFIGURED_SIZE_CALL_SITES` in `test_global_config_read_ratchet.py`). A +`server_args` the object was *handed* is a different thing and not a ratchet +matter — see "Reads that legitimately stay on a ServerArgs instance". +Fail-loud is narrower: before dist init, a live size/group read raises — except +the DCP pair, which degrades instead (`dcp_enabled` → `False`, +`attn_dcp_size` → `1` when no group is installed; +`test_attn_dcp_defaults_when_group_is_uninitialized` pins this). After init, only the DCP group is optional (`_DCP` exists only when `dcp_size > 1`; attn-CP and moe-DP always install, as size-1 aliases if unused). `ParallelContext.__getattr__` is deliberately dynamo-traceable (no `object.__getattribute__`); gate helpers like `enable_moe_dense_fully_dp()` run inside compiled model forwards (`test_parallel_config_leaves_trace_under_torch_compile` pins this). +### Reading config: the seed is off limits + +`get_server_args().field` in business code is a ratchet failure. Read: + +- **a resolved leaf** → its namespace bag (`get_exec().moe.moe_runner_backend`, + `get_schedule().chunked_prefill_size`, …). Bag-backed reads — a leaf directly, or + a bag-derived accessor below, including `configured_*_size()` which reads the + parallel bag's own leaf — are what see post-publish overrides. Only the + instance-derived accessors (the ones with no leaf to read) answer from the + startup record and therefore do not. +- **the live topology** → `get_parallel()`. +- **a value derived from published leaves** → an accessor in `runtime_context` that + derives it *from the bags*: `mamba_extra_buffer_enabled()` / + `mamba_extra_buffer_lazy_enabled()` read `get_memory()` and `get_exec()`, so + they see post-publish overrides. Prefer this shape whenever the inputs are + leaves; the same-named `ServerArgs` members are the pre-publish equivalents the + resolution pipeline uses, and wrapping one of those instead would quietly cost + you override visibility. `is_ep_joiner()` / `is_ep_scale_joiner()` are the same + shape over `exec.moe.ep_join_mode`, `attention_backends()` derives the + `(prefill, decode)` pair from the three `exec.kernel` leaves, and + `max_speculative_num_draft_tokens()` / `cutedsl_moe_max_num_tokens()` derive + theirs from `spec` / `schedule` / `exec.graph`. +- **a value only the instance can compute** → the named accessor in + `runtime_context`, which is the one module allowed to read the slot: + `mamba_cache_chunk_size()`, `uses_mla_backend()`, `process_model_config()`. + These have no leaf to read — they combine several fields, the HF config, or a + property with no bag of its own. A new derived member gets an accessor here + rather than call sites reaching for the record, and only when the bag-derived + shape above cannot express it. +- **what was *configured*, where `get_parallel()` shadows it with the live value** + → `configured_{tp,pp,moe_dp,attn_cp}_size()` — the full names are + `configured_tp_size`, `configured_pp_size`, `configured_moe_dp_size`, + `configured_attn_cp_size`. They read the parallel bag's own leaf (going + around the live property that shadows those four names), so they answer with + the resolved configuration and follow a post-publish override. DCP has no configured accessor because no + config-intent DCP call site exists today — the live reads go through + `get_parallel().attn_dcp_size` / `.dcp_enabled`, which answer the effective + topology (`1` / `False` when no group is installed). A site + that must know the *requested* DCP size before dist init needs its own + `configured_dcp_size()` (and an entry in `_CONFIGURED_SIZE_CALL_SITES`, which lives + in the ratchet test, not in this skill); note the live pair does not *need* + dist init — with no group it answers `1` / `False` — it just cannot answer + with the requested size. Every (file, accessor) pair is registered + with its reason in `test_global_config_read_ratchet.py` + (`_CONFIGURED_SIZE_CALL_SITES`), and that test fails if the code and the list + disagree — a new file, or a new accessor in a listed file, has to be added — so a new site needs both an answer the live property cannot give and + an entry saying what it is. +- **this runner's resolved value** → the runner + (`prefill_attention_backend_str`, `kv_cache_dtype_str`, + `draft_attention_backend`, `num_fused_shared_experts` on the model). + +`self.server_args.field` is still right for handed per-instance config (see +"Reads that legitimately stay on a ServerArgs instance" above for the full set — +per-instance boundaries and whole-object passes; there are no per-runner config +copies to read any more). The allow-list is the +tokenizer-manager family, `entrypoints/`, the tokenizer-process multimodal +processors, `GrammarManager`, `MMEncoder` — but not for one single reason: + +- the tokenizer-manager family and `entrypoints/` are the multi-Engine case + proper: several of them can live in one process, so a bag read would answer + from whichever Engine published last; +- `GrammarManager` is a handed instance — it is constructed with the config its + owner hands it and never assumes a published namespace; +- `MMEncoder` publishes the very instance it is handed (`publish(server_args, + role="encoder")`) and takes its per-worker device as a separate `gpu_id` + argument, so its `self.server_args` reads and the bag agree today. They are on + this list as a construction-path convention rather than a semantic exception — + and the residual is real: a post-publish `override` would not reach them. + +Their tests tell you the same thing: they construct the object standalone, so a +bag read turns into "config namespace not published". + +**Test doubles publish, they do not inject.** A stand-in that carries +`server_args=SimpleNamespace(field=...)` stops working the moment production reads +the bag; seed the value with `override_server_args`, which publishes only once it is +entered or installed — the bare call just builds the override: + +```python +override = get_context().override_server_args(field=...) +override.install() +self.addCleanup(override.restore) # or: with get_context().override_server_args(...): +``` + +Five separate test files learned this the hard way during the sweep. + +The rule is about a double standing in for **config**: a `SimpleNamespace` that +pretends to be `server_args`. Prefer the context override even where a +single-accessor stub would work — `override_server_args(...)` composed with the +scoped bag / `get_parallel()` overrides expresses the *cause* (the configuration) +rather than pinning one helper's answer, and it keeps working when a reader +migrates between the accessor and the leaf. The sweep converted the last two +accessor stubs to exactly that shape (`test_attention_patching.py` publishes the +non-lazy strategy; `test_kimi_k3_vision.py` publishes `tp_size` and forces the +live topology through `get_parallel().override`), so no test stubs an accessor +today. Stubbing one *named accessor* remains a last resort for a case that +isolates one branch of one helper where no published config can reach it — +if you do it, say so in the test. + ### Mid-resolution reads (inside the pipeline only) Resolution itself still runs in `__post_init__`: handlers and hooks read the @@ -229,7 +355,10 @@ ONE thread — do not design for TBO threads that don't exist. so a faked accessor silently stops intercepting after any reader migration. Publish for real (`override_server_args(...)`), then adjust bag leaves with the scoped bag `override` where the constructed `ServerArgs` cannot carry the value (e.g. - `get_device().override(device="meta")`). + `get_device().override(device="meta")`). The one carve-out is the deliberate + single-accessor stub for isolating one predicate — the terms and the two + sanctioned examples live under "Test doubles publish, they do not inject" + above; anything wider than one named accessor is this rule. - Mocked runners/managers still need the **per-runner instance attributes** the code under test reads (`kv_cache_dtype_str`, `server_args` for whole-object passes) — set them explicitly on the mock; `MagicMock(spec=...)` raises on attributes that only @@ -257,12 +386,26 @@ ONE thread — do not design for TBO threads that don't exist. calls either form. Rerouting a writer to the bags means flipping **all its readers in the same commit** (no transitional dual-write). 4. **Legacy-accessor ratchet** (`test_legacy_global_ratchet.py`): `get_global_server_args` - call sites must not grow — new code uses `runtime_context.get_server_args()` (and - business decisions should read the bags). -5. **Module-state ratchet** (`test_module_state_ratchet.py`): `global` statements in the + call sites must not grow. The replacement for a *decision* is a bag leaf, a named + accessor, or the owning runner's stamp — not `get_server_args().field`, which the + read ratchet below pins at zero. `runtime_context.get_server_args()` is only for the + whole-object shapes (dumps, provenance, a hand-off to a callee that takes a config). +5. **Global config read ratchet** (`test_global_config_read_ratchet.py`): baselines are + **0** for both the direct `get_server_args().field` and the alias form (function-local + — including local copies of an alias, `cfg = sa` — module-level, or parked on an + instance attribute, plus the `getattr(..., "field")` spelling of each; a name + computed at runtime or indirection deeper than a local name copy is census-tool + territory, per the test's docstring). The scanners match `get_server_args` and + `configured_*_size` by their literal names, and the same file *bans* + `import ... as` renames of them so that matching stays sound. Exempt by owner + module only (`runtime_context.py`, `server_args.py`, `arg_groups/`). The same file + carries `_CONFIGURED_SIZE_CALL_SITES`, the (file, accessor) map of every + `configured_*_size()` reader with the reason the live property cannot serve it — a new + file or a new accessor in a listed file must be added there. +6. **Module-state ratchet** (`test_module_state_ratchet.py`): `global` statements in the flag-owning layers are pinned by name. A new module-level runtime global belongs on a flags group / resources slot instead; migrating a pinned survivor must shrink the pin. -6. **Namespace coverage** (`test_server_args_namespaces.py`, +7. **Namespace coverage** (`test_server_args_namespaces.py`, `test_runtime_context_config_bags.py`): every `ServerArgs` field carries `NS(...)` metadata and the projected bags must cover the fields exactly (two-way). @@ -272,13 +415,12 @@ Never module-skip a test "until the migration settles" — seed the context inst ## Hard-won pitfalls (check these before/while refactoring) - **Moving code drops first-line guards**: early returns (`if self.is_draft_worker: return`) - are the easiest thing to lose when relocating a method body. Every draft is built - under a preserved publish of its own config: the scheduler makes the copy with - `draft_server_args_copy()` (seeded from the resolved config, so load-time overrides - carry) and publishes it around the worker factory, and `build_draft_tp_worker()` - nests the same shape for dflash / dspark. The publish ends when construction does — - anything the draft reads later (`alloc_memory_pool`, `init_attention_backends`, - cuda-graph capture) is back on the target's bags. + are the easiest thing to lose when relocating a method body. A draft is built from + the target's published config — there is no draft config copy and no nested publish + any more — so a body moved out of a draft-aware call site keeps reading the target's + bags, and only that guard tells the two apart. What the draft build *does* scope is + narrower and named: `draft_model_build_scope()` for the MoE fusion gates, + `speculative_moe_backend_context()` for the runner backends. - **Registry-completeness timing**: a gate that consults an extensible list is only correct after the registrars ran (platform `init_backend()` at module import). See "load-time vs resolution-time". @@ -316,7 +458,7 @@ Key source files: `python/sglang/srt/runtime_context.py` (the container, every t `declare_late_resolution`), `python/sglang/srt/server_args.py` (`NS` metadata, `Arg(..., resolvable=True)`, `__setattr__` strict guard), and the guardrail tests under `test/registered/unit/` (`test_server_args_mutation_ratchet.py`, -`test_server_args_writer_ratchet.py`, `test_legacy_global_ratchet.py`, +`test_global_config_read_ratchet.py`, `test_legacy_global_ratchet.py`, `test_module_state_ratchet.py`, `test_server_args_namespaces.py`, `test_runtime_context.py` — the last one doubles as executable documentation of every tier's semantics).