[RFC] Sentinel-default precedence for stage engine args

TL;DR

Collapse the three sources of truth for engine-arg defaults (argparse parser, OmniEngineArgs dataclass, StageDeployConfig dataclass) down to one — StageDeployConfig. Make argparse parser defaults and OmniEngineArgs field defaults all None (sentinel). The merge layer's existing if value is None: continue guard becomes the entire precedence rule. Eliminates the cli_explicit_keys parameter, the default_overrides bucket, and the _infer_explicit_keys_from_defaults heuristic introduced by #3006.

Targets: post-0.20.0. Pure refactor; no user-visible behavior change for correctly-configured deploy YAMLs.

Background

#2383 split per-platform stage YAMLs into pipeline.py (frozen topology) + deploy/<model>.yaml (deployment knobs) + caller overrides, with the precedence ladder:

pipeline.py → deploy.yaml (+ platforms overlay) → global CLI → per-stage CLI

To distinguish "user typed --gpu-memory-utilization 0.9" from "argparse filled the default 0.9," we introduced cli_explicit_keys — a set of dests captured from sys.argv via detect_explicit_cli_keys, threaded through OmniBase → AsyncOmniEngine → StageConfigFactory._create_from_registry.

This works for argparse callers but leaves a sharp edge: programmatic Omni(...) / AsyncOmni(...) callers have no parser, so cli_explicit_keys=None. Today the merge layer treats None as "everything is explicit," which silently clobbers deploy YAML values when callers forward vars(args) or OmniEngineArgs() defaults (#2958 thread, reported by @Sy0307).

#3006 attempts to fix this by inferring explicitness from OmniEngineArgs dataclass-default comparison. @xiaohajiayou pushed back: the right fix is a precedence-definition change, not a heuristic. Both proposals leave caller intent ambiguous in different rows of the precedence table.

The root cause: three layers all carry real defaults. Argparse ships default=0.9, OmniEngineArgs.gpu_memory_utilization=0.9, and StageDeployConfig.gpu_memory_utilization=0.9. They compete during merge, and we need cli_explicit_keys only because the parser-default layer is indistinguishable from real user input by the time it reaches the merge.

Goals

• Single source of truth for defaults. StageDeployConfig (already the design per #2383) becomes the only place real defaults live.
• Trivial precedence rule. Caller overrides YAML; un-set fields fall back to YAML; YAML gaps fall back to StageDeployConfig dataclass defaults. Period.
• Symmetric semantics for CLI and programmatic callers. vllm serve --gpu-mem 0.5 and Omni(model="x", gpu_memory_utilization=0.5) behave identically.
• Anti-pattern safety. Omni(**vars(args)) with sentinel-defaulted parser doesn't clobber YAML.
• CI-enforced contract. Three invariants prevent regression (parser defaults, dataclass defaults, boolean flag form).

Non-goals

• No change to topology semantics — pipeline.py stays frozen.
• No change to deploy YAML schema or platform overlay logic.
• No new CLI flags for end users. --stage-overrides, --deploy-config, etc. all keep existing semantics.
• Does not address the engine_args=OmniEngineArgs(...) kwarg path — separate decision documented in Open Questions.

Design

Layered split

flowchart TB
    subgraph CaptureLayer["Capture layer (None defaults)"]
        Parser["argparse: default=None per flag"]
        OmniArgs["OmniEngineArgs: field default=None"]
    end
    subgraph MergeLayer["Merge layer (cli_overrides → stage)"]
        Guard["if value is None: continue"]
        Apply["explicit_overrides[key] = value"]
    end
    subgraph EffectiveLayer["Effective config (real defaults)"]
        Stage["StageDeployConfig: real defaults"]
    end
    Parser --> Guard
    OmniArgs --> Guard
    Guard --> Apply
    Apply --> Stage

Each layer has one job:

Layer	Defaults	None means
OmniEngineArgs (user input dataclass)	None for all fields	"user did not set this"
argparse parser (CLI capture)	None for all stage-level flags	"user did not type this"
vars(args) / kwargs into Omni()	mostly None when nothing typed	"skip in merge"
deploy/<model>.yaml stages: + platforms:	per-stage real values	"this is the deployed value"
StageDeployConfig (effective config)	Real defaults (0.9, auto, ...)	"fallback when nobody else set it"

Precedence chain (replaces #2383's ladder)

For topology fields (stage list, edges, processor functions): pipeline.py is absolute, never overridden.

For engine knob fields (gpu_memory_utilization, max_num_seqs, dtype, ...):

Per-stage caller kwarg (stage_N_<field> / --stage-overrides JSON)   [strongest]
↓
Global caller kwarg (<field> typed on CLI or in Omni(...) call)
↓
deploy.yaml platforms.<auto-detected>.stages
↓
deploy.yaml stages
↓
StageDeployConfig dataclass default                                  [weakest]

Single rule: anything in cli_overrides with value is not None wins over YAML for its key.

What disappears

Today	Post-RFC
cli_explicit_keys parameter on _create_from_registry	Removed
_cli_explicit_keys plumbing through OmniBase → AsyncOmniEngine	Removed
default_overrides bucket in merge	Removed
_infer_explicit_keys_from_defaults (#3006)	Removed
is_explicit = cli_explicit_keys is None or key in cli_explicit_keys or is_per_stage	Replaced by if value is None: continue
tools/smoke_cli_explicit_keys.py	Replaced by tools/smoke_sentinel_defaults.py
Parser-default vs dataclass-default divergence risk	Eliminated by single source of truth

What stays

• detect_explicit_cli_keys survives but is no longer load-bearing for precedence — it can be used by tooling/diagnostics if needed.
• from_cli_args survives as a 2-line ergonomic alias: cls(model=args.model, **{k:v for k,v in vars(args).items() if k != "model"}).
• All of #2383's deploy YAML structure, platform overlay merge, per-stage override JSON.

All call paths under the new design

Invocation	cli_overrides content	Stage 2 gpu_memory_utilization
vllm serve --omni --gpu-mem 0.5	{gpu_memory_utilization: 0.5, max_num_seqs: None, ...}	0.5 (caller wins)
vllm serve --omni	{gpu_memory_utilization: None, max_num_seqs: None, ...}	YAML's 0.1 (None-guard skips)
Omni(model="x", gpu_memory_utilization=0.5)	{gpu_memory_utilization: 0.5}	0.5
Omni(model="x", gpu_memory_utilization=0.9) (matches dataclass)	{gpu_memory_utilization: 0.9}	0.9 (intent honored, no heuristic)
Omni(model="x")	{}	YAML's 0.1
Omni(model="x", **vars(args)) (anti-pattern)	mostly None	YAML's 0.1 (None-guard saves us)
Omni(model="x", async_chunk=False) (BoolOptAction)	{async_chunk: False}	YAML async_chunk: True flipped to False

Same end state as today's correct behavior, without the explicit/implicit branching.

Migration plan (4 PRs, post-0.20.0)

#	Title	Surface	Risk
1	Convert parser defaults to None	OmniEngineArgs.add_cli_args, help formatter	Low — --help cosmetics + 1 CI invariant
2	Convert OmniEngineArgs field defaults to None	dataclass field defaults, type annotations	Low — explicit user-input vs effective-config split
3	Drop cli_explicit_keys from merge layer	StageConfigFactory._create_from_registry, OmniBase.__init__, AsyncOmniEngine.__init__, all tests	Medium — load-bearing deletion, biggest test churn
4	from_cli_args → thin alias	OmniBase.from_cli_args, examples	Trivial

Each PR is independently mergeable and reversible. PR-3 is the load-bearing one; PR-1 + PR-2 set the stage so PR-3 is a deletion rather than a rewrite.

PR-3 will ship with a one-version DeprecationWarning shim that accepts and ignores cli_explicit_keys= for compatibility with out-of-tree forks; the shim is removed in the release after.

Test plan

Three load-bearing CI invariants (added in PR-1 and PR-2)

def test_no_stage_engine_flag_has_nondefault_parser_default():
    """Every --flag from OmniEngineArgs.add_cli_args has default=None.
    Server-level dests in SERVER_LEVEL_DESTS are exempt."""

def test_no_engine_arg_uses_store_true_or_store_false():
    """Boolean flags use BooleanOptionalAction(default=None)."""

def test_omniengineargs_user_input_fields_default_to_none():
    """OmniEngineArgs is the user-input layer; required fields use MISSING,
    optional fields use None. StageDeployConfig holds the real defaults."""

These three tests are what makes the design self-defending. Without them, the next contributor to add a flag with default=0.9 re-introduces the bug.

TestSentinelDefaultPrecedence (added in PR-3)

Replaces TestCLIExplicitPrecedence. Covers all rows of the call-path table above. _stages helper drops its cli_explicit_keys argument.