diff --git a/.claude/rules/no-dataclasses.md b/.claude/rules/no-dataclasses.md index f29c7bfb9..6a30511ec 100644 --- a/.claude/rules/no-dataclasses.md +++ b/.claude/rules/no-dataclasses.md @@ -7,12 +7,12 @@ paths: Define data containers as `msgspec.Struct`. Do not add new `dataclasses.dataclass` (or `attrs`) — they weaken strict type checking and -don't map onto Rust structs for the planned Rust migration. +don't translate cleanly for multi-language support (e.g. the planned Rust migration). ```python import msgspec -class LoadSnapshot(msgspec.Struct): # frozen=, kw_only=, omit_defaults= as needed +class LoadSnapshot(msgspec.Struct): # prefer frozen= and omit_defaults=; kw_only= as needed dp_rank: int = 0 tokens: list[int] = [] # mutable defaults are safe ``` @@ -21,4 +21,3 @@ class LoadSnapshot(msgspec.Struct): # frozen=, kw_only=, omit_defaults= as nee `python/sglang/srt/managers/load_snapshot.py`. - New code only. Existing `@dataclass` is grandfathered — migrate opportunistically while editing the file, not in drive-by sweeps. -- If a third-party API forces `@dataclass`, keep it at that boundary only. diff --git a/.claude/rules/no-getattr-defensive.md b/.claude/rules/no-getattr-defensive.md new file mode 100644 index 000000000..76893d93c --- /dev/null +++ b/.claude/rules/no-getattr-defensive.md @@ -0,0 +1,39 @@ +--- +paths: + - "**/*.py" +--- + +# Don't use `getattr` / `hasattr` for defensive access + +Over-defensive `getattr(obj, "field", default)` / `hasattr(obj, "field")` hide +errors and defeat strict type checking. If a field is always present, accessing +it defensively is confusing and masks real bugs. Prefer: + +1. **`isinstance` for type narrowing** — check the type, then access fields directly: + + ```python + if ( + isinstance(obj, (TokenizedGenerateReqInput, TokenizedEmbeddingReqInput)) + and obj.mm_inputs + ): + ``` + (see `python/sglang/srt/managers/mm_utils.py`) + +2. **Always set the field (to `None` if needed), then do a `None` check** — the + field should always exist, so a `None` / non-`None` check is enough: + + ```python + obj.field = None # in __init__ / construction + ... + if obj.field is not None: + ... + ``` + +Bad — `server_args` always has `revision`, so `getattr` is misleading and swallows +a real `AttributeError` if the field is ever renamed: + +```python +revision=getattr(server_args, "revision", None), # BAD +revision=server_args.revision, # GOOD +``` +(see `python/sglang/srt/managers/template_detection.py`)