diff --git a/.claude/rules/general-code-style.md b/.claude/rules/general-code-style.md index dc09944d8..07f82c548 100644 --- a/.claude/rules/general-code-style.md +++ b/.claude/rules/general-code-style.md @@ -9,7 +9,7 @@ Default conventions for new and modified Python code. Prefer these unless there - **Prefer stateless.** Favor pure functions over methods that mutate instance state; pass inputs in, return outputs out. - **Prefer immutable.** Default to immutable data (frozen structs, tuples, read-only values); mutate only when there is a clear need. -- **Extract init-static values at construction.** When a derived value's inputs are frozen for the object's lifetime (typically configuration: constructor args, env vars, server args), compute it once in `__init__` and store it as a well-named attribute (`self.mtp_enabled`, `self.needs_cpu_seq_lens`); later code reads the attribute instead of re-deriving it. Input immutability is the hard precondition — if inputs can change, recompute in place or funnel mutation through a single override point (the frozen `ServerArgs.override()` pattern). If you can't give the value a meaningful name, the boundary is wrong — don't cache unnameable subexpressions. +- **Extract init-static values at construction.** When a derived value's inputs are frozen for the object's lifetime (typically configuration: constructor args, env vars, server args), compute it once in `__init__` and store it as a well-named attribute (`self.mtp_enabled`, `self.needs_cpu_seq_lens`); later code reads the attribute instead of re-deriving it. Input immutability is the hard precondition — if inputs can change, recompute in place or funnel mutation through a single override point (`get_context().override()` for resolved config). If you can't give the value a meaningful name, the boundary is wrong — don't cache unnameable subexpressions. - **Functions stay small.** Keep each function under ~100 LOC; split larger ones into named helpers. - **Files stay small.** Keep each file under ~2k LOC; split larger modules along cohesive boundaries. - **Core functions read like pseudocode.** The main / orchestration function of a unit should be short and read like algorithm pseudocode — push detail into well-named helpers so the top-level flow is obvious. diff --git a/.claude/skills/sglang-runtime-context/SKILL.md b/.claude/skills/sglang-runtime-context/SKILL.md index a5f68174a..a5690abcd 100644 --- a/.claude/skills/sglang-runtime-context/SKILL.md +++ b/.claude/skills/sglang-runtime-context/SKILL.md @@ -49,12 +49,41 @@ resolved configuration lives in the namespace bags.** `get_context().override(source, **fields)`. It writes the bag leaves in place (namespace readers see the new value) and records provenance in the overrides log. There is **no write-through** to the `ServerArgs` instance — it stays pristine. - `ServerArgs.override(...)` (instance-only) is being retired; its call sites are - ratcheted down and new ones are rejected. -- **Nested publishes**: a construction step that must publish a private copy (the - draft-worker build publishes the draft's rewritten config for the duration of the - build) wraps itself in `get_context().preserve_config()` — the enclosing lifecycle, - including its post-publish overrides, is value-snapshotted and reinstated on exit. + There is no in-place mutation entry on the instance at all: it is read-only after + resolution. +- **Late launcher-stage resolution (pre-publish)**: a few rules cannot run inside + `__post_init__` — LoRA normalization, and the auto-parser detection that needs a + tokenizer/chat-template load. They are resolution, not mutation, and they write + **in place** via `arg_groups.overrides.declare_late_resolution(server_args, + source, **fields)`, which refuses the published instance. In place is the point: + every holder of that object must see the resolved value — the HTTP server, the + multi-tokenizer workers it is serialized for, the schedulers it forks. Returning a + variant here is a bug: the launcher rebinds its local and everyone else keeps the + unresolved object. +- **A config another runner / worker / process is built from**: `server_args.derive( + source, **fields)` returns a variant (an encode worker's `base_gpu_id`/`tp_size`). + The receiver — and any bags projected from it — are untouched; resolution does + **not** re-run, so do not reach for it to "re-resolve" a config. +- **Per-runner values inside one process are constructor arguments, not a variant.** + The draft worker's `context_length`, load format and attention backend travel as + arguments to `TpModelWorker` / `ModelRunner` and live on the runner + (`ModelRunner.draft_attention_backend`, `kv_cache_dtype_str`, …), because target + and draft coexist and the process-wide bags can only describe one of them. + +**Why a bag override cannot stand in for the last two.** The bags are projected at +publish *from the instance's fields*, so anything the runtime must read has to be on +the instance before publish — an override afterwards puts instance and bags back out +of agreement, and whole-object readers (`ModelConfig.from_server_args`, +`build_load_config`, `MMEncoder`'s own `self.server_args.X`) never see it. And bags do +not cross a process boundary: a child publishes from the object it receives and +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. ### Reads that legitimately stay on a `ServerArgs` instance @@ -209,18 +238,18 @@ ONE thread — do not design for TBO threads that don't exist. ## Guardrails (these fail CI; what to do when they fire) 1. **Strict mutation guard** (always on): bare `server_args.x = ...` after resolution - raises unconditionally — `ServerArgs.__setattr__` no longer consults - `SGLANG_STRICT_CONFIG_MUTATION` (the env var survives only as a legacy harness - flag). Projected bags are sealed the same way (leaf assignment raises — write via - `get_context().override`). + raises unconditionally in `ServerArgs.__setattr__` — this *is* the guarantee that + no writer can desync the bags, so there is no writer ratchet any more. Change + resolved config with `get_context().override`, build a per-runner config with + `server_args.derive`. Projected bags are sealed the same way (leaf assignment + raises). 2. **Mutation ratchet** (`test_server_args_mutation_ratchet.py`, exact pin 0 over the whole package minus the pipeline / multimodal_gen): textual scan for assignment forms. Never raise the baseline. -3. **Writer ratchet** (`test_server_args_writer_ratchet.py`): `ServerArgs.override` - call sites are pinned exactly and may only shrink — instance writes never reach the - bags, so namespace readers desync from the writer. New post-publish writes go through - `get_context().override`; rerouting a writer means flipping **all its readers to the - bag in the same commit** (no transitional dual-write). +3. **Derive contract** (`test_server_args_derive.py`): deriving leaves the receiver + intact, a published config still refuses assignment, and deriving does not publish. + 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). diff --git a/python/sglang/compile_deep_gemm.py b/python/sglang/compile_deep_gemm.py index cf733c79e..7eb283d2c 100644 --- a/python/sglang/compile_deep_gemm.py +++ b/python/sglang/compile_deep_gemm.py @@ -169,21 +169,18 @@ def launch_server_process_and_send_one_request( ) -def refine_server_args(server_args: ServerArgs, compile_args: CompileArgs): - # Disable cuda graph and torch compile to save time. Writes after - # ServerArgs.__post_init__ don't propagate to cuda_graph_config via the - # legacy disable_cuda_graph field, so flip both phases directly. +def compile_server_args(args, compile_args: CompileArgs) -> ServerArgs: + """The config this script serves with: no cuda graph, no torch compile, and a + watchdog that outlives the compilation.""" + args.enable_torch_compile = False + # Watchdog timeout follows compile_args.timeout because compilation takes long. + args.watchdog_timeout = compile_args.timeout + args.warmups = "compile-deep-gemm" + server_args = ServerArgs.from_cli_args(args) server_args.cuda_graph_config[Phase.DECODE].backend = Backend.DISABLED server_args.cuda_graph_config[Phase.PREFILL].backend = Backend.DISABLED print(f"Disable CUDA Graph and Torch Compile to save time...") - - # Watchdog timeout follows compile_args.timeout because compilation takes long. - server_args.override( - "compile_deep_gemm.refine_server_args", - enable_torch_compile=False, - watchdog_timeout=compile_args.timeout, - warmups="compile-deep-gemm", - ) + return server_args def run_compile(server_args: ServerArgs, compile_args: CompileArgs): @@ -216,9 +213,7 @@ if __name__ == "__main__": ServerArgs.add_cli_args(parser) CompileArgs.add_cli_args(parser) args = parser.parse_args() - server_args = ServerArgs.from_cli_args(args) compile_args = CompileArgs.from_cli_args(args) - - refine_server_args(server_args, compile_args) + server_args = compile_server_args(args, compile_args) run_compile(server_args, compile_args) diff --git a/python/sglang/lang/backend/runtime_endpoint.py b/python/sglang/lang/backend/runtime_endpoint.py index fa5414afd..9b453f72f 100644 --- a/python/sglang/lang/backend/runtime_endpoint.py +++ b/python/sglang/lang/backend/runtime_endpoint.py @@ -384,13 +384,15 @@ class Runtime: from sglang.srt.server_args import ServerArgs from sglang.srt.utils.network import is_port_available - self.server_args = ServerArgs(*args, log_level=log_level, **kwargs) - - # Pre-allocate ports - for port in range(self.server_args.port, 40000): + # Pre-allocate a port before building the config, so the config is born + # with the port this runtime will serve on. + requested_port = kwargs.pop( + "port", ServerArgs.__dataclass_fields__["port"].default + ) + for port in range(requested_port, 40000): if is_port_available(port): break - self.server_args.override("runtime_endpoint.port_alloc", port=port) + self.server_args = ServerArgs(*args, log_level=log_level, port=port, **kwargs) self.url = self.server_args.url() self.generate_url = self.url + "/generate" diff --git a/python/sglang/srt/arg_groups/overrides.py b/python/sglang/srt/arg_groups/overrides.py index 245f5943c..f5af1ddbc 100644 --- a/python/sglang/srt/arg_groups/overrides.py +++ b/python/sglang/srt/arg_groups/overrides.py @@ -208,12 +208,47 @@ def run_post_process_pass(server_args: Any, fn: Callable[..., dict]) -> None: def _apply_fields(server_args: Any, fields: Dict[str, Any]) -> None: """Write fields on behalf of the pipeline (bypasses the strict bare- assignment guard that protects post-resolution mutation).""" - object.__setattr__(server_args, "_in_override", True) + object.__setattr__(server_args, "_internal_write", True) try: for field, value in fields.items(): setattr(server_args, field, value) finally: - object.__setattr__(server_args, "_in_override", False) + object.__setattr__(server_args, "_internal_write", False) + + +def declare_late_resolution(server_args: Any, source: str, **fields: Any) -> None: + """Resolve fields on a config that is **not published yet**. + + A few resolution rules cannot run inside ``__post_init__``: LoRA + normalization and the auto-parser detection need the launcher's validation + stage (and, for the parsers, a tokenizer / chat-template load). They still + belong to the resolution pipeline — they decide what the process will run + with — so they write the fields in place, before anything publishes the + object. Writing in place is the point: every holder of that instance (the + HTTP server, the multi-tokenizer workers it serializes for, the schedulers + it forks) must see the resolved value. + + Refuses to touch the published instance: after publish the bags exist and a + field write would desync them, which is what ``get_context().override`` is + for. + """ + from sglang.srt.runtime_context import get_context + + try: + published = get_context().server_args + except ValueError: + published = None + if published is server_args: + raise ValueError( + f"declare_late_resolution({source!r}) called on the published config; " + "post-publish changes go to the bags via get_context().override(...)" + ) + log = getattr(server_args, "_runtime_mutations", None) + if log is None: + log = [] + object.__setattr__(server_args, "_runtime_mutations", log) + log.append((source, dict(fields))) + _apply_fields(server_args, fields) def materialize_declarations(server_args: Any) -> None: diff --git a/python/sglang/srt/disaggregation/encode_server.py b/python/sglang/srt/disaggregation/encode_server.py index 84732b837..600942960 100644 --- a/python/sglang/srt/disaggregation/encode_server.py +++ b/python/sglang/srt/disaggregation/encode_server.py @@ -1,7 +1,6 @@ import asyncio import concurrent.futures import contextlib -import copy import ctypes import functools import logging @@ -3520,10 +3519,7 @@ async def run_dp_worker( # gpu_id is the device chosen by maybe_reindex_device_id in the parent: # 0 when CVD is pinned to one GPU, else the absolute id. rank=0, so # MMEncoder runs set_device(base_gpu_id). - args = copy.deepcopy(server_args) - # The copy is already resolved (read-only); route the per-worker - # specialization through the audited mutation entry. - args.override("encode_server.dp_worker", base_gpu_id=gpu_id, tp_size=1) + args = server_args.derive("encode_server.dp_worker", base_gpu_id=gpu_id, tp_size=1) enc = MMEncoder(args, dist_init_method=f"tcp://127.0.0.1:{get_free_port()}", rank=0) global encoder_metrics_collector diff --git a/python/sglang/srt/environ.py b/python/sglang/srt/environ.py index eee06bdc2..92421fae1 100644 --- a/python/sglang/srt/environ.py +++ b/python/sglang/srt/environ.py @@ -247,10 +247,6 @@ class DsparkFoldedSampling(IntEnum): class Envs: - # Raise on bare server_args field assignments after resolution; mutation - # must go through ServerArgs.override() (enabled by the test harness). - SGLANG_STRICT_CONFIG_MUTATION = EnvBool(False) - # Per-role config-namespace bookkeeping: off / record / enforce (value is # validated fail-loud in runtime_context, which resolves it once at import # so the read stays dynamo-prunable). diff --git a/python/sglang/srt/mem_cache/hi_mamba_radix_cache.py b/python/sglang/srt/mem_cache/hi_mamba_radix_cache.py index 8671a7a75..fb8815b70 100644 --- a/python/sglang/srt/mem_cache/hi_mamba_radix_cache.py +++ b/python/sglang/srt/mem_cache/hi_mamba_radix_cache.py @@ -99,16 +99,6 @@ class HiMambaRadixCache(MambaRadixCache): def __init__(self, params: CacheInitParams, server_args: ServerArgs): self._enable_metrics_flag = params.enable_metrics - if server_args.hicache_io_backend == "direct": - if server_args.hicache_mem_layout == "page_first": - server_args.override( - "hicache.mem_layout_force", hicache_mem_layout="page_first_direct" - ) - logger.warning( - "Page first layout is not supported with direct IO backend, " - "switching to page first direct layout" - ) - self.page_size = params.page_size self.hybrid_kv_cache = params.token_to_kv_pool_allocator.get_kvcache() if not isinstance(self.hybrid_kv_cache, HybridLinearKVPool): diff --git a/python/sglang/srt/parser/template_detection.py b/python/sglang/srt/parser/template_detection.py index d61c4d3ef..d353aa1c6 100644 --- a/python/sglang/srt/parser/template_detection.py +++ b/python/sglang/srt/parser/template_detection.py @@ -22,13 +22,15 @@ import logging import os import re from dataclasses import dataclass -from typing import Callable, Optional, Tuple +from typing import Callable, Dict, Optional, Tuple import jinja2 import jinja2.ext import jinja2.nodes import jinja2.sandbox +from sglang.srt.arg_groups.overrides import declare_late_resolution + logger = logging.getLogger(__name__) @@ -625,26 +627,21 @@ def detect_inline_system_support(chat_template: Optional[str]) -> bool: return False -def _resolve_auto_parser( - server_args, +def _detect_auto_parser( attr: str, ctx: TemplateDetectionContext, rules: Tuple[DetectionRule, ...], label: str, -) -> None: - """Resolve a single auto parser, updating server_args in place.""" +) -> Optional[str]: + """The parser one auto field resolves to (``None`` disables it).""" detected = match_rules(ctx, rules, label) if detected: - server_args.override(source="template-detection", **{attr: detected}) logger.info( f"Auto-detected --{attr.replace('_', '-')} as '{detected}' from chat template" ) - else: - logger.warning( - f"--{attr.replace('_', '-')}=auto specified but could not detect " - f"{label} from chat template. Disabling {label}." - ) - server_args.override(source="template-detection", **{attr: None}) + return detected + _log_undetected_parser(attr, label) + return None def _load_explicit_jinja_template(chat_template_arg: Optional[str]) -> Optional[str]: @@ -658,15 +655,15 @@ def _load_explicit_jinja_template(chat_template_arg: Optional[str]) -> Optional[ return f.read().replace("\\n", "\n") -def _disable_auto_parser(server_args, attr: str, label: str) -> None: +def _log_undetected_parser(attr: str, label: str) -> None: logger.warning( f"--{attr.replace('_', '-')}=auto specified but could not detect " f"{label} from chat template. Disabling {label}." ) - server_args.override(source="template-detection", **{attr: None}) -def _resolve_architecture_auto_parsers(server_args) -> None: +def _architecture_auto_parsers(server_args, needs: Tuple[str, ...]) -> Dict[str, str]: + """The parsers the model architecture implies, for the fields still on auto.""" from sglang.srt.utils.hf_transformers_utils import get_config config = get_config( @@ -686,30 +683,37 @@ def _resolve_architecture_auto_parsers(server_args) -> None: elif "DeepseekV3" in arch: reasoning_parser, tool_call_parser = "deepseek-v3", "deepseekv32" else: - return + return {} + resolved = {} for attr, detected in ( ("reasoning_parser", reasoning_parser), ("tool_call_parser", tool_call_parser), ): - if getattr(server_args, attr) == "auto": - server_args.override(source="template-detection", **{attr: detected}) + if attr in needs: + resolved[attr] = detected logger.info( f"Auto-detected --{attr.replace('_', '-')} as '{detected}' " f"from model architecture '{arch}'" ) + return resolved def resolve_auto_parsers(server_args) -> None: - """Resolve --reasoning-parser=auto and --tool-call-parser=auto before scheduler. + """Resolve ``--reasoning-parser=auto`` / ``--tool-call-parser=auto`` from the + chat template, in place, before anything publishes ``server_args``. - This performs a lightweight tokenizer load to detect parsers from the chat - template. Called early in engine init before scheduler subprocesses are spawned. + Performs a lightweight tokenizer load, so it runs once in engine init. In + place because everyone who holds this instance must see the resolved value: + the schedulers it forks, the HTTP server, and the tokenizer workers it is + serialized for. """ - needs_reasoning = server_args.reasoning_parser == "auto" - needs_tool_call = server_args.tool_call_parser == "auto" - - if not needs_reasoning and not needs_tool_call: + needs = tuple( + attr + for attr in ("reasoning_parser", "tool_call_parser") + if getattr(server_args, attr) == "auto" + ) + if not needs: return from sglang.srt.utils.hf_transformers_utils import get_tokenizer @@ -741,6 +745,8 @@ def resolve_auto_parsers(server_args) -> None: ctx = build_detection_context( template, tokenizer, reasoning_config, force_reasoning ) + + detected: Dict[str, Optional[str]] = {} if ctx is None: if has_explicit_template_without_detection: logger.warning( @@ -750,38 +756,26 @@ def resolve_auto_parsers(server_args) -> None: ) else: try: - _resolve_architecture_auto_parsers(server_args) + detected.update(_architecture_auto_parsers(server_args, needs)) except Exception as e: logger.warning( "Failed to load model config for architecture-based auto-detection: %s", e, ) - if needs_reasoning: - if server_args.reasoning_parser == "auto": - _disable_auto_parser( - server_args, "reasoning_parser", "reasoning parser" - ) - if needs_tool_call: - if server_args.tool_call_parser == "auto": - _disable_auto_parser( - server_args, "tool_call_parser", "tool-call parser" - ) - return + for attr, label in ( + ("reasoning_parser", "reasoning parser"), + ("tool_call_parser", "tool-call parser"), + ): + if attr in needs and attr not in detected: + _log_undetected_parser(attr, label) + detected[attr] = None + else: + for attr, rules, label in ( + ("reasoning_parser", REASONING_PARSER_RULES, "reasoning parser"), + ("tool_call_parser", TOOL_CALL_PARSER_RULES, "tool-call parser"), + ): + if attr in needs: + detected[attr] = _detect_auto_parser(attr, ctx, rules, label) - if needs_reasoning: - _resolve_auto_parser( - server_args, - "reasoning_parser", - ctx, - REASONING_PARSER_RULES, - "reasoning parser", - ) - - if needs_tool_call: - _resolve_auto_parser( - server_args, - "tool_call_parser", - ctx, - TOOL_CALL_PARSER_RULES, - "tool-call parser", - ) + if detected: + declare_late_resolution(server_args, "template-detection", **detected) diff --git a/python/sglang/srt/runtime_context.py b/python/sglang/srt/runtime_context.py index 33cd4780b..f958b3d3e 100644 --- a/python/sglang/srt/runtime_context.py +++ b/python/sglang/srt/runtime_context.py @@ -1011,8 +1011,7 @@ class _ServerArgsOverride: def install(self) -> ServerArgs: """Publish a fresh dummy-boundary ``ServerArgs`` carrying the - overrides (written through ``ServerArgs.override`` for provenance); - returns the published instance.""" + overrides; returns the published instance.""" from sglang.srt.server_args import ServerArgs assert not self._installed, "override_server_args already installed" @@ -1025,7 +1024,7 @@ class _ServerArgsOverride: self._prev_capture = ctx.flags.capture.enable_torch_compile server_args = ServerArgs(model_path="dummy") if self._fields: - server_args.override(source="test-override", **self._fields) + server_args = server_args.derive("test-override", **self._fields) # The dummy boundary skips materialization, which would leave the # strict mutation guard unarmed on the published object — mark it # materialized so bare post-publish writes raise like they do on a diff --git a/python/sglang/srt/server_args.py b/python/sglang/srt/server_args.py index a40d9607b..99bdfda2f 100644 --- a/python/sglang/srt/server_args.py +++ b/python/sglang/srt/server_args.py @@ -16,6 +16,7 @@ from __future__ import annotations import argparse +import copy import dataclasses import glob import importlib @@ -8547,56 +8548,73 @@ class ServerArgs: return resolved_view(self) - def override(self, source: str, **fields) -> None: - """The single post-resolution mutation point. + def _late_resolution(self, source: str, **fields) -> None: + """Resolve fields at the launcher's validation stage (pre-publish). - After ``__post_init__`` the configuration is resolved; the audited - runtime adjustments (load-resolved values, control-plane - reconfiguration, deployment wiring) go through here instead of - assigning fields. Whitelisted resolvable fields also join the - declaration stash, so a republish resolves the same values; - everything is recorded with its ``source`` for provenance. + See ``arg_groups.overrides.declare_late_resolution``: in place, because + every holder of this instance must see the resolved value, and refused + outright once the config is published. + """ + from sglang.srt.arg_groups.overrides import declare_late_resolution + + declare_late_resolution(self, source, **fields) + + def derive(self, source: str, **fields) -> ServerArgs: + """A copy carrying variant values: a draft worker's context length, an + encode worker's device, a launcher's late port pick. + + The receiver is untouched, so a config already published from it -- and + the namespace bags projected out of it -- stay true; the variant is a + second config, to be published in its own right or handed to whoever + owns it. Resolution does not re-run: the values being set are decided + *after* it, from inputs resolution never had, and re-resolving a + resolved config re-derives conditional decisions from the wrong ones. + + Whitelisted resolvable fields also join the copy's declaration stash so + a later re-resolution keeps them; ``source`` is recorded for provenance. """ from sglang.srt.arg_groups.arg_utils import resolvable_fields - whitelist = resolvable_fields(type(self)) + variant = copy.deepcopy(self) + whitelist = resolvable_fields(type(variant)) declared = {k: v for k, v in fields.items() if k in whitelist} rest = {k: v for k, v in fields.items() if k not in whitelist} if declared: - stash = getattr(self, "_resolved_overrides", None) + stash = getattr(variant, "_resolved_overrides", None) if stash is None: stash = [] - object.__setattr__(self, "_resolved_overrides", stash) + object.__setattr__(variant, "_resolved_overrides", stash) stash.append((source, dict(declared))) if rest: - log = getattr(self, "_runtime_mutations", None) + log = getattr(variant, "_runtime_mutations", None) if log is None: log = [] - object.__setattr__(self, "_runtime_mutations", log) + object.__setattr__(variant, "_runtime_mutations", log) log.append((source, dict(rest))) - object.__setattr__(self, "_in_override", True) + object.__setattr__(variant, "_internal_write", True) try: for field, value in fields.items(): - setattr(self, field, value) + setattr(variant, field, value) finally: - object.__setattr__(self, "_in_override", False) + object.__setattr__(variant, "_internal_write", False) + return variant def __setattr__(self, name, value): - # after materialization the fields are the resolved startup - # configuration -- the pristine, READ-ONLY record. A bare assignment - # outside ServerArgs.override() (and the resolution pipeline, which runs - # before materialization) always raises; resolved config is mutated on - # the context bags via get_context().override(...), not here. (Formerly - # gated on SGLANG_STRICT_CONFIG_MUTATION; now unconditional.) + # After materialization the fields are the resolved startup + # configuration -- the pristine, READ-ONLY record that the config bags + # were projected from. Resolved config changes go to the bags via + # get_context().override(source, ...); a config that differs per runner + # or per worker is a separate object, built with derive(). if ( not name.startswith("_") and getattr(self, "_declarations_materialized", False) - and not getattr(self, "_in_override", False) + and not getattr(self, "_internal_write", False) ): raise AttributeError( f"server_args.{name} assigned after resolution; server_args is " "read-only -- use get_context().override(source, ...) to change " - "resolved config." + "resolved config, or server_args.derive(source, ...) to build a " + "variant for one runner." ) object.__setattr__(self, name, value) @@ -8914,7 +8932,7 @@ class ServerArgs: # Enable LoRA if any LoRA paths are provided for backward compatibility. if self.lora_paths: if self.enable_lora is None: - self.override("check_lora_server_args", enable_lora=True) + self._late_resolution("check_lora_server_args", enable_lora=True) logger.warning( "--enable-lora is set to True because --lora-paths is provided." ) @@ -8925,7 +8943,7 @@ class ServerArgs: if self.enable_lora: if self.enable_lora_overlap_loading is None: - self.override( + self._late_resolution( "check_lora_server_args", enable_lora_overlap_loading=False ) @@ -8984,9 +9002,11 @@ class ServerArgs: "Expected a string or a dictionary." ) parsed_lora_paths.append(lora_ref) - self.override("check_lora_server_args", lora_paths=parsed_lora_paths) + self._late_resolution( + "check_lora_server_args", lora_paths=parsed_lora_paths + ) elif isinstance(self.lora_paths, dict): - self.override( + self._late_resolution( "check_lora_server_args", lora_paths=[ LoRARef( @@ -8999,7 +9019,7 @@ class ServerArgs: ], ) elif self.lora_paths is None: - self.override("check_lora_server_args", lora_paths=[]) + self._late_resolution("check_lora_server_args", lora_paths=[]) else: raise ValueError( f"Invalid type for --lora-paths: {type(self.lora_paths)}. " @@ -9009,7 +9029,7 @@ class ServerArgs: # Normalize target modules to a set; keep {"all"} as a sentinel # that gets resolved model-awarely in lora_manager.init_lora_shapes(). if self.lora_target_modules: - self.override( + self._late_resolution( "check_lora_server_args", lora_target_modules=set(self.lora_target_modules), ) diff --git a/python/sglang/srt/speculative/draft_worker_common.py b/python/sglang/srt/speculative/draft_worker_common.py index 2e1685da1..adc23c728 100644 --- a/python/sglang/srt/speculative/draft_worker_common.py +++ b/python/sglang/srt/speculative/draft_worker_common.py @@ -1,7 +1,6 @@ from __future__ import annotations import logging -from copy import deepcopy from typing import TYPE_CHECKING, Optional import msgspec @@ -43,11 +42,11 @@ class DraftWorkerBundle(msgspec.Struct, frozen=True): def _resolve_draft_attention_backend_fallback( - *, draft_server_args: ServerArgs, algo_label: str + *, server_args: ServerArgs, algo_label: str ) -> str: - draft_backend = draft_server_args.speculative_draft_attention_backend + draft_backend = server_args.speculative_draft_attention_backend if draft_backend is None: - draft_backend, _ = draft_server_args.get_attention_backends() + draft_backend, _ = server_args.get_attention_backends() if draft_backend is None: return "triton" if torch.version.hip else "flashinfer" if draft_backend not in _SUPPORTED_DRAFT_BACKENDS: @@ -111,8 +110,7 @@ def draft_server_args_copy(server_args: ServerArgs, target_model_config) -> Serv for _source, fields in get_context().overrides_log(): resolved.update(fields) - draft_server_args = deepcopy(server_args) - draft_server_args.override( + return server_args.derive( "draft_worker.copy", **{ **resolved, @@ -120,7 +118,6 @@ def draft_server_args_copy(server_args: ServerArgs, target_model_config) -> Serv **_draft_load_format_fields(), }, ) - return draft_server_args def build_draft_tp_worker( @@ -133,18 +130,15 @@ def build_draft_tp_worker( algo_label: str, attention_backend_override: Optional[str] = None, ) -> DraftWorkerBundle: - draft_server_args = deepcopy(server_args) # An override names a draft-specific backend the caller has already # validated (e.g. a self-drafting architecture); it skips the generic # supported-backend fallback below. draft_backend = attention_backend_override or ( _resolve_draft_attention_backend_fallback( - draft_server_args=draft_server_args, algo_label=algo_label + server_args=server_args, algo_label=algo_label ) ) - # Post-resolution ServerArgs rejects bare assignment; route the draft-copy - # adjustments through the audited mutation point. - draft_server_args.override( + draft_server_args = server_args.derive( "draft_worker.build", **draft_server_args_overrides(target_model_config, draft_backend), ) diff --git a/python/sglang/test/kits/attention_unittest/runner_modes/speculative_draft_runner.py b/python/sglang/test/kits/attention_unittest/runner_modes/speculative_draft_runner.py index eb150e7c9..46db9aebf 100644 --- a/python/sglang/test/kits/attention_unittest/runner_modes/speculative_draft_runner.py +++ b/python/sglang/test/kits/attention_unittest/runner_modes/speculative_draft_runner.py @@ -295,7 +295,6 @@ def _configure_runner_for_eagle_draft( *, speculative_attention_mode: str = "decode", ) -> None: - server_args = runner.server_args updates = { "attention_backend": case.backend, "cuda_graph_config": CudaGraphConfig( @@ -326,11 +325,12 @@ def _configure_runner_for_eagle_draft( "torch_compile_max_bs": 0, "use_mla_backend": runner.use_mla_backend, } - server_args.override(source="attention-unittest-eagle-draft", **updates) - # Re-publish so the bags pick up the overrides. from sglang.srt.runtime_context import get_context - get_context().set_server_args(server_args) + runner.server_args = runner.server_args.derive( + "attention-unittest-eagle-draft", **updates + ) + get_context().set_server_args(runner.server_args) runner.spec_algorithm = SpeculativeAlgorithm.EAGLE runner.is_draft_worker = True @@ -391,11 +391,11 @@ def _build_frozen_kv_mtp_fixture( runner_batch_size=settings.capture_batch_size, ) _configure_runner_for_eagle_draft(fixture.runner, case, settings) - fixture.runner.server_args.override( - "attention_unittest.frozen_kv_draft", speculative_algorithm="FROZEN_KV_MTP" - ) from sglang.srt.runtime_context import get_context + fixture.runner.server_args = fixture.runner.server_args.derive( + "attention_unittest.frozen_kv_draft", speculative_algorithm="FROZEN_KV_MTP" + ) get_context().set_server_args(fixture.runner.server_args) fixture.runner.spec_algorithm = SpeculativeAlgorithm.FROZEN_KV_MTP fixture.runner.draft_attn_backend = fixture.backend diff --git a/python/sglang/test/test_utils.py b/python/sglang/test/test_utils.py index efd41ed4e..2a9c1a68d 100644 --- a/python/sglang/test/test_utils.py +++ b/python/sglang/test/test_utils.py @@ -9,10 +9,6 @@ import inspect import json import logging import os - -# Registered tests run with the strict config-mutation guard: bare -# server_args assignments after resolution raise (use ServerArgs.override). -os.environ.setdefault("SGLANG_STRICT_CONFIG_MUTATION", "1") import random import re import shlex diff --git a/test/registered/attention/unittests/hybrid_linear/test_flashinfer_mla_chunk_metadata.py b/test/registered/attention/unittests/hybrid_linear/test_flashinfer_mla_chunk_metadata.py index a00851797..e5cebe281 100644 --- a/test/registered/attention/unittests/hybrid_linear/test_flashinfer_mla_chunk_metadata.py +++ b/test/registered/attention/unittests/hybrid_linear/test_flashinfer_mla_chunk_metadata.py @@ -45,13 +45,14 @@ class _ChunkKVMLARunner(MockMLAModelRunner): def __init__(self, **kwargs): super().__init__(**kwargs) - # The fixture's config is already published; adjust it through the - # audited entry point (bare writes raise under the strict guard). - self.server_args.override( - source="attention-unittest", + from sglang.srt.runtime_context import get_context + + self.server_args = self.server_args.derive( + "attention-unittest", disable_chunked_prefix_cache=False, flashinfer_mla_disable_ragged=False, ) + get_context().set_server_args(self.server_args) def _make_case() -> MLAAttentionCase: diff --git a/test/registered/unit/parser/test_template_manager.py b/test/registered/unit/parser/test_template_manager.py index b9b257b88..af5e0ea0c 100644 --- a/test/registered/unit/parser/test_template_manager.py +++ b/test/registered/unit/parser/test_template_manager.py @@ -13,6 +13,7 @@ from sglang.srt.parser.template_detection import ( detect_tool_call_parser, resolve_auto_parsers, ) +from sglang.srt.server_args import ServerArgs from sglang.test.ci.ci_register import register_cpu_ci register_cpu_ci(est_time=2.0, suite="base-a-test-cpu") @@ -769,21 +770,15 @@ class TestResolveAutoParsers(unittest.TestCase): qwen3_template = "{% set enable_thinking = enable_thinking if enable_thinking is defined else true %}" - class _Args(SimpleNamespace): - # Write-through override, per the runtime-context testing idiom: - # production adjusts parsers through override(source, ...), so the - # stand-in needs the method (a bare SimpleNamespace would raise). - def override(self, source, **fields): - for key, value in fields.items(): - setattr(self, key, value) - def _make_server_args( self, reasoning_parser=None, tool_call_parser=None, chat_template=None ): - return self._Args( + # The dummy model path skips resolution; the tokenizer / HF-config + # loads that detection performs are patched per test. + return ServerArgs( + model_path="dummy", reasoning_parser=reasoning_parser, tool_call_parser=tool_call_parser, - model_path="Qwen/Qwen3-0.6B", trust_remote_code=False, chat_template=chat_template, ) @@ -826,7 +821,11 @@ class TestResolveAutoParsers(unittest.TestCase): def test_nonexistent_model_disables_both_parsers(self): args = self._make_server_args(reasoning_parser="auto", tool_call_parser="auto") - args.model_path = "nonexistent/model-does-not-exist-xyz" + args = self._make_server_args( + reasoning_parser="auto", + tool_call_parser="auto", + ) + object.__setattr__(args, "model_path", "nonexistent/model-does-not-exist-xyz") with _patch_hf_transformers_utils( Mock(side_effect=RuntimeError("tokenizer unavailable")), Mock(side_effect=RuntimeError("config unavailable")), diff --git a/test/registered/unit/test_runtime_context.py b/test/registered/unit/test_runtime_context.py index d3ac25300..6079f9614 100644 --- a/test/registered/unit/test_runtime_context.py +++ b/test/registered/unit/test_runtime_context.py @@ -299,12 +299,13 @@ class TestServerArgsScopedOverride(_IsolatedServerArgs): def test_installed_config_arms_the_strict_guard(self): # The published dummy must behave like a resolved config: bare writes - # raise under the strict harness; override() stays the entry point. + # raise, and a differing value lives on a derived variant. published = get_context().override_server_args(tp_size=2).install() with self.assertRaises(AttributeError): published.tp_size = 4 - published.override(source="test", tp_size=4) - self.assertEqual(published.tp_size, 4) + variant = published.derive("test", tp_size=4) + self.assertEqual(variant.tp_size, 4) + self.assertEqual(published.tp_size, 2) def test_restore_resets_the_capture_seed(self): # install() seeds flags.capture from the published dummy; restore() diff --git a/test/registered/unit/test_runtime_context_override.py b/test/registered/unit/test_runtime_context_override.py index 9f99a89c1..93c344242 100644 --- a/test/registered/unit/test_runtime_context_override.py +++ b/test/registered/unit/test_runtime_context_override.py @@ -102,8 +102,8 @@ class TestContextOverride(CustomTestCase): self.assertEqual(sa.kv_cache_dtype, raw) def test_bare_server_args_write_raises_after_resolution(self): - # server_args is read-only after resolution regardless of the - # SGLANG_STRICT_CONFIG_MUTATION env; write via override instead. + # server_args is read-only after resolution: resolved config changes go + # to the bags, a per-runner config to a derived variant. sa = ServerArgs(model_path="dummy") object.__setattr__(sa, "_declarations_materialized", True) with self.assertRaises(AttributeError): @@ -118,8 +118,9 @@ class TestContextOverride(CustomTestCase): rc.get_context().override( "ModelRunner.configure_kv_cache_dtype", kv_cache_dtype="fp8_e4m3" ) - draft = ServerArgs(model_path="dummy") - draft.override(source="draft-build", kv_cache_dtype="bf16") + draft = ServerArgs(model_path="dummy").derive( + "draft-build", kv_cache_dtype="bf16" + ) with rc.get_context().preserve_config(): rc.get_context().set_server_args(draft) # Inside the scope the draft's bags are live... diff --git a/test/registered/unit/test_server_args_derive.py b/test/registered/unit/test_server_args_derive.py new file mode 100644 index 000000000..3ddca37e5 --- /dev/null +++ b/test/registered/unit/test_server_args_derive.py @@ -0,0 +1,79 @@ +"""``ServerArgs.derive`` is the only way one config becomes another. + +After resolution the instance is the process's read-only startup record and the +object the config bags were projected from, so it cannot be mutated: a change to +resolved config goes to the bags (``get_context().override``), and a config that +differs for one runner or worker — a draft's context length, an encode worker's +device — is a second object. +""" + +from sglang.test.ci.ci_register import register_cpu_ci + +register_cpu_ci(est_time=5, suite="base-a-test-cpu") + +import unittest + +from sglang.srt.runtime_context import get_context, reset_context +from sglang.srt.server_args import ServerArgs +from sglang.test.test_utils import CustomTestCase + + +class TestServerArgsDerive(CustomTestCase): + def tearDown(self): + reset_context() + + def _resolved(self) -> ServerArgs: + """A config in the post-resolution state, without a real model.""" + server_args = ServerArgs(model_path="dummy") + object.__setattr__(server_args, "_declarations_materialized", True) + return server_args + + def test_the_receiver_is_untouched(self): + server_args = self._resolved() + variant = server_args.derive("draft_worker.copy", context_length=1024) + + self.assertEqual(variant.context_length, 1024) + self.assertIsNot(variant, server_args) + self.assertIsNone(server_args.context_length) + + def test_a_published_config_rejects_assignment_but_still_derives(self): + override = get_context().override_server_args(tp_size=2) + published = override.install() + self.addCleanup(override.restore) + + with self.assertRaises(AttributeError): + published.tp_size = 4 + + self.assertEqual(published.derive("worker", tp_size=4).tp_size, 4) + self.assertEqual(published.tp_size, 2) + + def test_the_variant_is_not_published_by_deriving(self): + override = get_context().override_server_args(tp_size=2) + published = override.install() + self.addCleanup(override.restore) + + published.derive("worker", tp_size=4) + + self.assertIs(get_context().server_args, published) + + def test_the_variant_is_frozen_too(self): + variant = self._resolved().derive("worker", context_length=1024) + with self.assertRaises(AttributeError): + variant.context_length = 2048 + + def test_provenance_is_recorded(self): + variant = self._resolved().derive("draft_worker.copy", watchdog_timeout=1.0) + self.assertIn( + ("draft_worker.copy", {"watchdog_timeout": 1.0}), + getattr(variant, "_runtime_mutations", []), + ) + + def test_a_resolvable_field_joins_the_declaration_stash(self): + """So a re-resolution of the variant keeps the derived value.""" + variant = self._resolved().derive("draft_worker.build", kv_cache_dtype="bf16") + stash = getattr(variant, "_resolved_overrides", []) + self.assertIn(("draft_worker.build", {"kv_cache_dtype": "bf16"}), stash) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/registered/unit/test_server_args_mutation_ratchet.py b/test/registered/unit/test_server_args_mutation_ratchet.py index 848c2d78c..4eea1aefe 100644 --- a/test/registered/unit/test_server_args_mutation_ratchet.py +++ b/test/registered/unit/test_server_args_mutation_ratchet.py @@ -8,13 +8,13 @@ configuration; the resolution pipeline (``server_args.py`` and an exact pin: new mutations must not appear, and removals must lower the baseline to lock in the progress. -Every audited runtime adjustment goes through ``ServerArgs.override(source, -**fields)`` — the single mutation entry point, which records provenance and -keeps whitelisted fields consistent with the declaration stash. The baseline -is therefore zero. The registered test harness additionally runs with -``SGLANG_STRICT_CONFIG_MUTATION=1``, under which a bare assignment after -resolution raises at runtime; this ratchet catches sites the tests never -execute. +There is no post-resolution mutation entry point on the instance any more: +resolved config changes go to the context bags via +``get_context().override(source, **fields)``, and a config that differs for one +runner or worker is a separate object built with ``ServerArgs.derive(source, +**fields)``. The baseline is therefore zero. ``ServerArgs.__setattr__`` raises +on a bare assignment after resolution; this ratchet catches the sites the tests +never execute. """ from sglang.test.ci.ci_register import register_cpu_ci @@ -69,8 +69,9 @@ class TestServerArgsMutationRatchet(CustomTestCase): f"server_args mutations outside the resolution pipeline grew: " f"{count} > baseline {_BASELINE}. Configuration is resolved in " "ServerArgs.__post_init__; declare through the pipeline " - "(passes / declare_load_time_override) or go through " - "ServerArgs.override(source, ...) instead of assigning fields." + "(passes / declare_load_time_override), change resolved config " + "with get_context().override(source, ...), or build a variant " + "with server_args.derive(source, ...) — do not assign fields." ) if count < _BASELINE: self.fail( diff --git a/test/registered/unit/test_server_args_no_instance_mutation_entry.py b/test/registered/unit/test_server_args_no_instance_mutation_entry.py new file mode 100644 index 000000000..2656589ad --- /dev/null +++ b/test/registered/unit/test_server_args_no_instance_mutation_entry.py @@ -0,0 +1,85 @@ +"""``ServerArgs`` has no in-place mutation entry, and nothing calls one. + +``ServerArgs.override(source, **fields)`` used to mutate a resolved instance; +after resolution the fields are the record the config bags were projected from, +so such a write desyncs every namespace reader. The method is gone and the two +sanctioned replacements are ``get_context().override`` (post-publish, writes the +bags) and ``ServerArgs.derive`` (a variant for another runner / process). Late +launcher-stage resolution writes in place through +``arg_groups.overrides.declare_late_resolution``, which refuses the published +instance. + +The textual half of this guard matters because the resolution pipeline's own file +is exempt from the mutation ratchet: a ``self.override(...)`` there — exactly +what the LoRA normalization used — is invisible to it. +""" + +from sglang.test.ci.ci_register import register_cpu_ci + +register_cpu_ci(est_time=5, suite="base-a-test-cpu") + +import re +import unittest +from pathlib import Path + +import sglang +from sglang.srt.server_args import ServerArgs +from sglang.test.test_utils import CustomTestCase + +_SGLANG_ROOT = Path(next(iter(sglang.__path__))) + +# ``x.override(`` on anything that is a ServerArgs by name, including the +# pipeline's own ``self.override(`` inside server_args.py. +_PATTERNS = [ + re.compile(r"\bself\.override\("), + re.compile(r"\bserver_args\.override\("), + re.compile(r"\bsa\.override\("), + re.compile(r"\bargs\.override\("), +] + +_EXCLUDED = ("multimodal_gen",) + + +class TestNoServerArgsMutationEntry(CustomTestCase): + def test_the_method_is_gone(self): + self.assertFalse( + hasattr(ServerArgs, "override"), + "ServerArgs.override is back; post-publish changes belong on the bags " + "(get_context().override) and per-runner values on a derive() variant.", + ) + + def test_nothing_calls_an_instance_override(self): + offenders = [] + for path in sorted(_SGLANG_ROOT.rglob("*.py")): + rel = path.relative_to(_SGLANG_ROOT).as_posix() + if rel.startswith(_EXCLUDED): + continue + source = path.read_text() + for pattern in _PATTERNS: + for match in pattern.finditer(source): + line = source.count("\n", 0, match.start()) + 1 + offenders.append(f"{rel}:{line}: {match.group(0)}") + if offenders: + self.fail( + "in-place ServerArgs mutation call-sites:\n" + + "\n".join(offenders) + + "\n\nUse get_context().override(source, ...) for resolved config, " + "server_args.derive(source, ...) for a per-runner variant, or " + "declare_late_resolution(...) for pre-publish launcher resolution." + ) + + def test_late_resolution_refuses_the_published_config(self): + from sglang.srt.arg_groups.overrides import declare_late_resolution + from sglang.srt.runtime_context import get_context + + override = get_context().override_server_args(tp_size=2) + published = override.install() + self.addCleanup(override.restore) + + with self.assertRaises(ValueError): + declare_late_resolution(published, "test", tp_size=4) + self.assertEqual(published.tp_size, 2) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/registered/unit/test_server_args_writer_ratchet.py b/test/registered/unit/test_server_args_writer_ratchet.py deleted file mode 100644 index 6a611dfb6..000000000 --- a/test/registered/unit/test_server_args_writer_ratchet.py +++ /dev/null @@ -1,81 +0,0 @@ -"""Ratchet guard: ``ServerArgs.override`` call-sites may only decrease. - -``ServerArgs.override(source, **fields)`` mutates a ``ServerArgs`` *instance* -only — the resolved-config bags on the runtime context never see the write, so -any consumer reading the namespace accessors (``get_exec()`` / ``get_memory()`` -/ …) desyncs from the writer. The migration end-state removes this primitive -entirely: post-publish, process-global config changes go through -``get_context().override(source, **fields)`` (which writes the bags), and -per-runner resolved values live on the runner object rather than on a -``ServerArgs`` copy. - -Until every call-site is rerouted together with its readers, this exact pin -keeps the writer surface from growing unwatched: new writers must use -``get_context().override``, and each rerouted batch lowers the baseline to -lock in the progress. (The count is textual and includes docstring mentions -and the test-kit's private-config use — the pin tracks growth, not the exact -production-writer census.) -""" - -from sglang.test.ci.ci_register import register_cpu_ci - -register_cpu_ci(est_time=5, suite="base-a-test-cpu") - -import re -import unittest -from pathlib import Path - -import sglang -from sglang.test.test_utils import CustomTestCase - -_SGLANG_ROOT = Path(next(iter(sglang.__path__))) - -# ``server_args.override(`` also matches ``self.server_args.override(``, -# ``.server_args.override(``, and the ``draft_server_args`` / -# ``dp_server_args`` copies; ``args`` / ``sa`` are the aliases a few call-sites -# bind first. -_WRITER_PATTERNS = [ - re.compile(r"server_args\.override\("), - re.compile(r"\bargs\.override\("), - re.compile(r"\bsa\.override\("), -] - -# The resolution pipeline itself (its declare face forwards through -# ``override`` by design) and multimodal_gen, whose ServerArgs is a different -# class outside this contract. -_EXCLUDED = ( - "srt/server_args.py", - "srt/arg_groups", - "multimodal_gen", -) - -_BASELINE = 15 - - -class TestServerArgsWriterRatchet(CustomTestCase): - def test_server_args_override_call_sites_match_the_baseline(self): - count = 0 - for path in sorted(_SGLANG_ROOT.rglob("*.py")): - rel = path.relative_to(_SGLANG_ROOT).as_posix() - if rel.startswith(_EXCLUDED): - continue - source = path.read_text() - count += sum(len(p.findall(source)) for p in _WRITER_PATTERNS) - if count > _BASELINE: - self.fail( - f"ServerArgs.override call-sites grew: {count} > baseline " - f"{_BASELINE}. Instance writes never reach the resolved-config " - "bags, so namespace readers desync from the writer. Post-publish " - "process-global changes go through get_context().override(...); " - "per-runner resolved values belong on the runner object." - ) - if count < _BASELINE: - self.fail( - f"ServerArgs.override call-sites shrank: {count} < baseline " - f"{_BASELINE}. Lower the baseline in this file to lock in the " - "progress." - ) - - -if __name__ == "__main__": - unittest.main()