diff --git a/.agents/skills/add-jit-kernel/SKILL.md b/.agents/skills/add-jit-kernel/SKILL.md index f68b43dd58..77e7225cf0 100644 --- a/.agents/skills/add-jit-kernel/SKILL.md +++ b/.agents/skills/add-jit-kernel/SKILL.md @@ -742,7 +742,7 @@ cd test && python3 run_suite.py --hw cuda --suite base-b-kernel-benchmark-test-1 - `docs/docs/developer_guide/development_jit_kernel_guide.mdx` - `test/run_suite.py` — suite names, discovery of `test/registered/`, execution entrypoint for CI - `python/sglang/test/ci/ci_register.py` — `register_cuda_ci` and AST registration rules -- `python/sglang/kernels/jit/utils/compile.py` — `load_jit`, `make_cpp_args` +- `python/sglang/kernels/jit/utils/compile/` — `load_jit` (`loader.py`), `make_cpp_args` (`cpp_args.py`) - `python/sglang/kernels/jit/utils/common.py` — `cache_once`, `should_run_full_tests`, `get_ci_test_range` - `python/sglang/kernels/jit/include/sgl_kernel/tensor.h` — `TensorMatcher`, `SymbolicSize/DType/Device`, `is_type` - `python/sglang/kernels/jit/include/sgl_kernel/ffi.h` — `ffi::empty`, `ffi::empty_like`, `ffi::from_blob` diff --git a/.agents/skills/clean-startup-log/references/noise-sources.md b/.agents/skills/clean-startup-log/references/noise-sources.md index f59d55f048..1d96a5d7b0 100644 --- a/.agents/skills/clean-startup-log/references/noise-sources.md +++ b/.agents/skills/clean-startup-log/references/noise-sources.md @@ -16,7 +16,7 @@ These are investigation hints, not a list of changes to apply. | `No platform detected. Using base SRTPlatform with defaults.` | Platform detection | Decide whether fallback is expected on this machine or indicates a missing plugin. | | `Unexpected error during package walk` in CUTE_DSL | Installed cutlass package, its handler, root propagation, and `CUTE_DSL_LOG_LEVEL` | Inspect the actual exception and duplicate-handler path; do not assume the current log level or hide all warnings. | | Repeated dtype, hybrid-model, or tokenizer fallback messages | Model-config and tokenizer construction in launcher, scheduler, and detokenizer | Multiple constructors may explain repetition. Retain meaningful per-process information and distinguish these from handler duplication. | -| Repeated template detection messages | `managers/template_detection.py` and `managers/template_manager.py` | Check whether a summary already contains the same information. | +| Repeated template detection messages | `parser/template_detection.py` and `parser/template_manager.py` | Check whether a summary already contains the same information. | | KV cache dtype logged separately from allocation | Model runner and memory pool | Check whether the allocation summary already carries dtype; retain useful allocation and SWA details. | | CUTLASS backend disabled during graph capture | Attention backend selection | Expected fallback may be useful information; retain the reason and selected behavior. | | `Ignore import error when loading ...` | Model, multimodal, or algorithm registries | Distinguish an unused optional model dependency from failure to load the requested model. | diff --git a/.agents/skills/cookbook-review-pr/SKILL.md b/.agents/skills/cookbook-review-pr/SKILL.md index 536ec8ea64..ba5d31c0a4 100644 --- a/.agents/skills/cookbook-review-pr/SKILL.md +++ b/.agents/skills/cookbook-review-pr/SKILL.md @@ -205,7 +205,7 @@ than restating. `python/sglang/srt/function_call/function_call_parser.py` (ToolCallParserEnum) — prose naming a near-miss id (e.g. the reasoning id where the tool id differs) is a factual bug. `--…-parser auto` is acceptable ONLY if the template-detection rules - (`python/sglang/srt/managers/template_detection.py`) actually resolve THIS model's + (`python/sglang/srt/parser/template_detection.py`) actually resolve THIS model's chat template to the right parser — no rule match means auto silently disables the parser; when in doubt require explicit ids (the DSv4 page pins explicit ids). diff --git a/.agents/skills/llm-torch-profiler-analysis/references/source-map.md b/.agents/skills/llm-torch-profiler-analysis/references/source-map.md index 88e0219027..56f15ecf1a 100644 --- a/.agents/skills/llm-torch-profiler-analysis/references/source-map.md +++ b/.agents/skills/llm-torch-profiler-analysis/references/source-map.md @@ -18,7 +18,7 @@ Use these upstream files when the workflow or behavior needs to be justified fro ## Scheduler-side trace writing -- `python/sglang/srt/managers/scheduler_profiler_mixin.py` +- `python/sglang/srt/managers/scheduler_components/profiler_manager.py` - actual trace start/stop behavior - filename pattern for `TP/DP/PP/EP` and optional stage suffixes - `CUDA_PROFILER` and torch profiler handling @@ -37,6 +37,3 @@ Use these upstream files when the workflow or behavior needs to be justified fro - `test/registered/profiling/test_start_profile.py` - validates `/start_profile` behavior, including `CUDA_PROFILER` - -- `test/registered/profiling/test_profile_v2.py` - - validates stage-scoped trace outputs under `SGLANG_PROFILE_V2` diff --git a/.agents/skills/sglang-runtime-context/SKILL.md b/.agents/skills/sglang-runtime-context/SKILL.md index b441c292b3..99628611e0 100644 --- a/.agents/skills/sglang-runtime-context/SKILL.md +++ b/.agents/skills/sglang-runtime-context/SKILL.md @@ -15,7 +15,7 @@ One container owns process-static runtime state: `sglang.srt.runtime_context.Run | 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()` | | per-forward | `get_forward()` | forward-scoped flags (multi-stream switch, MoE output buffer, attn-TP inputs, extend-in-batch) | contextvar-backed; `scoped(**kw)` restores on exit; new threads see defaults | -| parallel | `get_parallel()` | one spelling per name: ranks and group handles are the live topology (`@property`, read-through); every other name, sizes included, is a leaf of the parallel config bag | ranks/groups: after dist init; leaves: after publish | +| parallel | `get_parallel()` | one spelling per name: scoped override, then permanent stamp, then published parallel config leaf | ranks: stamped at publish; group handles: stamped at distributed init; configured and derived widths: after publish | `reset_context()` (unit-test teardown) drops the published config and installs fresh flags/resources/forward tiers. @@ -87,13 +87,12 @@ with what the operator typed, not with what resolution decided.** `ModelRunner.update_model_fields` overrides `model_path` / `load_format` for the target runner. - **Late launcher-stage resolution (pre-publish)**: a few rules cannot run inside - `__post_init__` — LoRA normalization, and the auto-parser detection that needs a + `ServerArgs.resolve_once` — LoRA normalization, and the auto-parser detection that needs a tokenizer/chat-template load. They are resolution, not mutation, and they **declare** via `arg_groups.overrides.declare_resolution(server_args, source, **fields)`, the same call the rest of the pipeline makes; there is no `declare_late_resolution` any more. *When* a declaration is made is not - something the code marks — the guardrails that used to read that marker - cover these sites through the ordinary keyword scan instead. The declaration lands + something the code marks. The declaration lands in the stash on that very object, so every holder of it carries the decision — the HTTP server, the multi-tokenizer workers it is serialized for, the schedulers it forks — and each of them publishes bags projected from it. The @@ -185,8 +184,8 @@ bag to override at all. ### Four ways a config sweep breaks something no test runs -Each of these shipped in a review round and cost a real defect; each now has a -guard, named here so the next sweep checks the same four things by hand first. +Each of these shipped in a review round and cost a real defect. Where a test +guards one it is named; the next sweep checks all four by hand first. 1. **The other implementations of an interface.** Dropping a parameter means auditing implementers, not just callers: `CustomSpecAlgo` is the plugin @@ -198,7 +197,8 @@ guard, named here so the next sweep checks the same four things by hand first. `publish` says nothing about whether a given read runs before it. Spawned workers (`MMEncoder` for encoder DP/TP, the Ray scheduler actor) start with an empty context, so a bag read above the publish raises only there. - Guard: `test_publish_precedes_bag_reads.py`. + Guarded on the launch path only (`test_pre_publish_readers.py`); other + entries, spawned workers included, are checked by review. 3. **The role namespace a process publishes under.** `ROLE_NAMESPACE_SETS` narrows what each role may read; the DP controller is audited for `exec` alone. A helper that reaches for another namespace passes every default-mode @@ -208,7 +208,9 @@ guard, named here so the next sweep checks the same four things by hand first. means enumerating the others: HTTP, gRPC and in-process `Engine` each have their own server-info and model-info, and each passes its own tests while its users lose the field. - Guard: `test_effective_state_surfaces.py`. + Guarded for server info across HTTP, both gRPC roles and the Rust frontend + (`TestServerInfoTransportParity` in `entrypoints/test_server_info.py`); + `Engine.get_server_info` and model info are checked by review. A fifth, from the same rounds: the accessor **name itself**. Called as an object member (`manager.get_disagg()`), or shadowed by a same-named import @@ -230,12 +232,13 @@ slack (`>= len(...) - 1`) or compared key names instead of value sources. ### `get_parallel()`: one spelling per name -**There is no `.config` hop.** Ranks and group handles are `@property` -read-through over the canonical getters, so they answer with the live process -groups. Everything else — `tp_size`, `pp_size`, `attn_cp_size`, `dcp_size`, -`moe_dp_size` included, alongside config-only leaves like `nccl_port`, -`dp_size`, `ep_size`, `dwdp_size` — is answered from the -published `parallel` bag. Reading a leaf before publish raises a `ValueError` +**There is no `.config` hop.** `ParallelContext._read` answers every name in +one order: a scoped override, then a permanent stamp, then the published +`parallel` bag leaf. `publish` stamps ranks from the launcher; distributed init +stamps group handles and validates their widths. Configured widths such as +`tp_size`, `pp_size`, `attn_cp_size`, `dcp_size` and `moe_dp_size`, and config-only +leaves such as `nccl_port`, `dp_size`, `ep_size` and `dwdp_size`, fall back to the +published bag. Reading a config leaf before publish raises a `ValueError` naming the namespace; an unknown name is an `AttributeError`. A size reads from the configuration because the groups are built at exactly the @@ -252,38 +255,33 @@ configured widths — checked at every assignment to `_TP` / `_PP` / `_ATTN_CP` coordinators keep their construction width. Those are different names, not two answers to one name. -DCP keeps its own pair: `get_parallel().attn_dcp_size` / `.dcp_enabled` answer the -*effective* topology (`1` / `False` with no group installed), while `dcp_size` is +DCP keeps its own pair: `get_parallel().attn_dcp_size` / `.dcp_enabled` are derived +from the published configuration unless explicitly overridden; `dcp_size` is what the launch requested. 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. 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 *rank/group* read raises. The six +Fail-loud is narrower: an unstamped rank or group handle with no declared default +raises; ranks are available after publish, before dist init. The six parallel quotients are not live reads at all — `attn_tp_size`, `attn_dp_size`, `attn_dcp_size`, `moe_ep_size`, `moe_tp_size`, `dcp_enabled` are a function of the configured leaves, computed once at publish into bag leaves, and answered override → stamp → published leaf. So `dcp_enabled` means "the launch configured -DCP" (`dcp_size > 1`), not "a DCP group is installed here"; in a scheduler the -stamp makes the two identical, in a process that publishes without dist init they -differ. `test_a_topology_is_stated_by_naming_the_width` and its neighbours in -`test_runtime_context.py` pin this; they replaced +DCP" (`dcp_size > 1`), not "a DCP group is installed here". Normal distributed +initialization builds the groups to match the configured widths; it does not +restamp these DCP leaves. `test_a_topology_is_stated_by_naming_the_width` and its +neighbours in `test_runtime_context.py` pin this; they replaced `test_attn_dcp_defaults_when_group_is_uninitialized`. One consequence for tests: overriding a leaf no longer moves its quotient — state a topology by publishing a config, or by naming the width. 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). The `config` hop is -deliberately dynamo-traceable (a plain property over a slot, no -`object.__getattribute__`); gate helpers like `enable_moe_dense_fully_dp()` run inside +moe-DP always install, as size-1 aliases if unused). Bare parallel leaf reads are +dynamo-traceable; gate helpers like `enable_moe_dense_fully_dp()` run inside compiled model forwards (`test_parallel_config_leaves_trace_under_torch_compile` pins this). -A third surface carries the same names: `ParallelState` (`self.ps` / `mr.ps`), the -frozen per-process snapshot built once in `Scheduler.__init__` from these configured -sizes plus this process's ranks, and handed down (draft runners included). Prefer it -where an object was handed one; it is not a global accessor. - ### Reading config: the seed is off limits `get_server_args().field` in business code is a ratchet failure. Read: @@ -329,9 +327,9 @@ where an object was handed one; it is not a global accessor. follows a post-publish override. Two questions are *not* that, and have their own spelling: the width of the MoE communicator you are about to collectively operate on is `get_moe_cp_size()` (the `_MOE_DP = _ATTN_CP` alias makes it - differ), and the effective DCP topology is `get_parallel().attn_dcp_size` / - `.dcp_enabled` (`1` / `False` when no group is installed), which does not need - dist init to answer. + differ), and DCP's derived width and enablement are + `get_parallel().attn_dcp_size` / `.dcp_enabled` (published configuration + unless explicitly overridden). - **this runner's resolved value** → the runner (`prefill_attention_backend_str`, `kv_cache_dtype_str`, `draft_attention_backend`); for what the model's layers bake in at @@ -343,10 +341,9 @@ per-instance boundaries and whole-object passes; there are no per-runner config copies to read any more). The allow-list is `GrammarManager` and `MMEncoder`; what sits beside it is residue, not a family — and not for one single reason: -- the tokenizer-manager family and `entrypoints/` **read the bags**; what is - left of them in the exposure ratchet is a handful of individually-dispositioned - pairs, not a family awaiting conversion. Read the ratchet for the current set - rather than assuming a directory is off-limits; +- the tokenizer-manager family and `entrypoints/` **read the bags**; review a new + instance read there against the raw-input contract rather than assuming a + directory is off-limits; - `GrammarManager` is a handed instance for its residual `self.server_args` reads, but backend selection is **not** on the instance any more: `create_grammar_backend` reads `get_exec().kernel.grammar_backend`, and @@ -392,15 +389,15 @@ if you do it, say so in the test. ### Mid-resolution reads (inside the pipeline only) -Resolution runs in `__post_init__` and **writes nothing onto the record**: a -handler declares (`self._declare` / `declare_resolution`), the declaration goes +Resolution runs in `ServerArgs.resolve_once` and **writes nothing onto the record**: a +handler declares (`declare_resolution`), the declaration goes into the stash, and the fields keep what the caller passed. So a mid-resolution read of a field answers with the *raw input* — every reader in the pipeline goes through a view instead: -- `resolving_view(server_args)` / `self._resolved()` — the live view (walks the +- `resolving_view(server_args)` — the live view (walks the stash per read). This is what handlers and hooks bind, conventionally as - `cfg = resolving_view(self)` at the top of the handler. + `cfg = resolving_view(server_args)` at the top of the handler. - `resolved_view(server_args)` — snapshots the overlay when built, which is what a post-process pass wants: it reads the state at *its* slot. @@ -479,12 +476,12 @@ dataclass. A declaration against a non-whitelisted field fails at its slot. ### Load-time vs resolution-time (critical) -`__post_init__` runs in the launcher process before any model/platform import. Logic that +`ServerArgs.resolve_once` runs in the launcher process before any model/platform import. Logic that consults an **extensible registry** (e.g. out-of-tree platforms registering attention backends in `init_backend()`, which runs at `model_runner` import) must stay at load time (ModelRunner init), writing through `get_context().override()`. Before moving any load-time logic into resolution, verify everything it reads is already complete at -construction time. +resolution time. ## Runtime flags (`get_flags()`) @@ -556,8 +553,9 @@ ONE thread — do not design for TBO threads that don't exist. them explicitly on the mock; `MagicMock(spec=...)` raises on attributes that only exist post-`__init__`, which is the fastest way to find a missed stub. - `reset_context()` in teardown when a test publishes outside a scoped override. -- `ServerArgs(model_path="dummy")` early-returns the pipeline (few declarations, no - strict guard) — fine for lightweight fixtures. +- `ServerArgs(model_path="dummy")` resolves through the pipeline's early return (few + declarations) — fine for lightweight fixtures. The strict guard still applies once + `resolve_once` has run, as it does at `publish`. - **Asserting what resolution decided reads `resolution_result(sa, "field")`**, not `sa.field`: the field is the raw input. Assert the field only when the point of the case *is* that the record stayed pristine (the FA4 page-size and waterfill @@ -566,7 +564,7 @@ ONE thread — do not design for TBO threads that don't exist. pytest run lets a context published by an earlier file mask a missing-publish bug in a later one. -## Guardrails (these fail CI; what to do when they fire) +## Guardrails and review rules 1. **Strict mutation guard** (always on, and with no exception): bare `server_args.x = ...` after resolution raises unconditionally in @@ -576,10 +574,9 @@ ONE thread — do not design for TBO threads that don't exist. resolved config with `get_context().override`; hand a per-runner value to its runner as a constructor argument. 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. **No-copy contract** (`test_server_args_no_instance_mutation_entry.py`): neither +2. **No textual mutation ratchet**: the strict guard above is the only mutation + check. +3. **No-copy contract** (review only, no automated guard): neither `ServerArgs.override` nor `ServerArgs.derive` exists, and nothing in the package calls either form. Rerouting a writer to the bags means flipping **all its readers in the same commit** (no transitional dual-write). @@ -590,23 +587,23 @@ ONE thread — do not design for TBO threads that don't exist. 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 +5. **Global config read check** (`test_global_config_read_ratchet.py`): no business code + reads a field off `get_server_args()`, directly or through an alias (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 scanner matches `get_server_args` by its + computed at runtime or indirection deeper than a local name copy is outside the + check, per the test's docstring). The scanner matches `get_server_args` by its literal name — bare or module-qualified (`ctx.get_server_args()`) — and `TestNoRenamedAccessorImports` in the same file *bans* `import ... as` renames of it, which is what makes literal-name matching sound. Exempt by owner module only (`runtime_context.py`, `server_args.py`, `arg_groups/`). Two classes, - no more: `TestGlobalConfigReadRatchet` holds the two baselines and + no more: `TestGlobalConfigReads` holds the zero check and `TestNoRenamedAccessorImports` holds the ban. There is no configured-size registry here any longer — `get_parallel()` has one spelling per name, so a size read is not a choice between two answers and nothing needs registering. -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. **Module-state rule** (review only, no automated guard): a new module-level + runtime global in the flag-owning layers belongs on a flags group / resources slot + instead. 7. **Namespace coverage** (`test_server_args_namespaces.py`, `test_runtime_context_config_bags.py`): every `ServerArgs` field resolves to a namespace — from the `arg_groups/fields/` class that declares it — and the @@ -662,8 +659,7 @@ Key source files: `python/sglang/srt/runtime_context.py` (the container, every t `python/sglang/srt/arg_groups/overrides.py` (override registry, passes, `declare_resolution` and the spellings around it), `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_global_config_read_ratchet.py`, -`test_module_state_ratchet.py`, `test_server_args_namespaces.py`, +`test/registered/unit/` (`test_global_config_read_ratchet.py`, +`test_server_args_namespaces.py`, `test_runtime_context.py` — the last one doubles as executable documentation of every tier's semantics). diff --git a/.claude/rules/no-getattr-defensive.md b/.claude/rules/no-getattr-defensive.md index 76893d93c1..ce4124db03 100644 --- a/.claude/rules/no-getattr-defensive.md +++ b/.claude/rules/no-getattr-defensive.md @@ -36,4 +36,4 @@ a real `AttributeError` if the field is ever renamed: revision=getattr(server_args, "revision", None), # BAD revision=server_args.revision, # GOOD ``` -(see `python/sglang/srt/managers/template_detection.py`) +(see `python/sglang/srt/parser/template_detection.py`) diff --git a/.claude/rules/unit-test-admission.md b/.claude/rules/unit-test-admission.md index 4731a596e4..1aaa47d902 100644 --- a/.claude/rules/unit-test-admission.md +++ b/.claude/rules/unit-test-admission.md @@ -27,8 +27,8 @@ A new unit test case must fall into one of these categories: 3. **Critical-path bookkeeping.** Defends conventions that are easy to break by forgetting to sync -- registry completeness, field lifecycle, serialization compatibility. Enumerating assertions are fine here; the guarded failure - mode is "someone extended X without updating Y". Example: the ratchet tests - (`test/registered/unit/test_module_state_ratchet.py`). + mode is "someone extended X without updating Y". Example: + `test/registered/unit/test_global_config_read_ratchet.py`. Not admissible: diff --git a/python/sglang/multimodal_gen/.agents/skills/sglang-diffusion-benchmark-profile/existing-fast-paths.md b/python/sglang/multimodal_gen/.agents/skills/sglang-diffusion-benchmark-profile/existing-fast-paths.md index 5b7f51b127..35f9ce7920 100644 --- a/python/sglang/multimodal_gen/.agents/skills/sglang-diffusion-benchmark-profile/existing-fast-paths.md +++ b/python/sglang/multimodal_gen/.agents/skills/sglang-diffusion-benchmark-profile/existing-fast-paths.md @@ -322,7 +322,7 @@ framework-specific optimization workflow. layouts (B > 1, SP > 1, LoRA-wrapped or quantized projections, head_dim != 128) and first-sight mismatches keep the Triton `qknorm_complex_rope` / `qknorm_complex_rope_kv` chain. -- Validation: `test/registered/kernels/ops/diffusion/test_qknorm_complex_rope.py`, +- Validation: `test/registered/kernels/ops/diffusion/test_complex_rope.py`, the `test_qwen21_cuda_qk_rope_*` cases in `test_model_fast_paths.py`, and `python/sglang/multimodal_gen/test/unit/test_qwen_image21_cuda.py`. - Workflow rule: if a Qwen-Image 2.1 trace still shows