Stop discarding a framework's JSON args and its measured accuracy

[fengshaoyi-amd]

What happened

An ATOM GLM-5.2-MXFP4 run (16h budget, TP8/EP1, ISL 8192 / OSL 1024, conc 64, MXFP4 on 8x MI355X) stopped at 6h09m with stop_reason=conc_sweep_failed, no accuracy anywhere in final.json, and ~9.8h of budget unspent. Three separate hardcoded, vLLM-shaped assumptions each failed silently on a framework whose flags do not match the vLLM spelling.

Every conc_sweep launch died like this:

openai_server.py: error: argument --online_quant_config/--online-quant-config:
  invalid loads value: '{global_quant_config:ptpc_fp8,exclude_layer:[lm_head,model.embed_tokens,*.mlp.gate,*expert*]}'

Root cause, narrowed to one hop

remove_server_args tokenized with a POSIX shlex.split and space-joined without re-quoting, so a JSON-valued flag lost its inner double quotes. _reserialize_json_blobs on the return path is supposed to recover that via _repair_unquoted_json, but that heuristic's bareword pattern (_JSON_BAREWORD) excludes *, so atom's exclude_layer wildcards (*.mlp.gate, *expert*) could not be re-quoted, json.loads kept failing, and the damaged blob was retained verbatim.

Reproduced as a pure-function chain against unmodified main:

ORIG                     : --online_quant_config '{"global_quant_config": "ptpc_fp8", "exclude_layer": [...]}'
compact_json_server_args : json.loads OK
remove_server_args       : json.loads FAIL   <- the only damage point
compose / dedup / lift   : FAIL (inherited)

The other three join points the incident report flagged are safe: merge_server_args never splits, and dedup_vllm_server_args / _shell_safe_dedupe both go through tokenize_server_args_preserving_json (posix=False, quotes preserved).

The hop is unconditional. compose_server_args always ends in strip_benchmark_harness_flags, whose removal list is non-empty by construction, and _lift_to_current_best re-strips both sides before merging. So no operator remove_args is needed to trigger it.

Where it actually bit, per the session artifacts. The EXPLORE variant YAMLs carry the value intact — {"global_quant_config":"ptpc_fp8","exclude_layer":[...]}, which json.loads accepts — and those variants benchmarked fine. The damage appears once SWEEP lifts current_best through the strip, and every conc_sweep YAML from that point on carries the unquoted form. An earlier revision of this description said all 12 variants were affected; the artifacts do not support that, and the corrected scope is the conc_sweep launches.

A/B against the two real trees — the incident's own checkout and this branch — confirms the hop and reproduces the rejected string byte for byte:

stage	incident tree (7b50bee1e)	this branch (f3884adb9)
compact_json_server_args	parses	parses
strip_benchmark_harness_flags	{global_quant_config:ptpc_fp8,exclude_layer:[lm_head,model.embed_tokens,*.mlp.gate,*expert*]} — identical to the string in server.log	parses
compose_server_args	corrupted (inherited)	parses

Changes

1. remove_server_args preserves JSON quotes. Both the args and the removal specs now tokenize through _tokenize_for_removal, which uses the quote-preserving tokenizer and declines — returning the input untouched — anything it cannot tokenize without dropping quote bytes.

The first revision of this commit kept a POSIX shlex.split fallback for declined inputs, justified here as safe because a declined input "cannot hold a JSON value the unquoted transport could deliver anyway". That reasoning was wrong: it is about the declined token, but the fallback applies to the whole string. One whitespace-bearing operand anywhere — --tool-call-parser 'my parser' sharing the args with --online_quant_config — sent the entire string through POSIX and reproduced the incident value byte for byte. 4e45fad3f drops the fallback; the cost is an unstripped flag, which is visible in the args, rather than a corrupted JSON value, which is not.

2. SPACE_VALUE_FLAGS deleted rather than extended. It had no reader left anywhere in the tree (only a definition, an alias, a _grid_runner re-export and an __all__ entry), yet its docstring still promised dedup-time protection for enrolled flags. That stale promise is why atom's --online_quant_config looked covered while nothing guarded it. Value shape, decided by parsing, is the property that matters. Note this removes a name whose comment claimed out-of-tree test use; flagging explicitly in case that matters to anyone.

3. The accuracy gate is unconditional. EXPLORE parsed a variant's eval result only when is_high_accuracy_risk matched a list of vLLM/SGLang flag names and VLLM_*/SGLANG_* env keys by substring. It matched none of the 12 ATOM variants: atom spells the knob --kv_cache_dtype (underscores, never matching --kv-cache-dtype), uses AITER_* env vars, and --online_quant_config — which changes numeric precision directly, the highest-risk class there is — was never enrolled at all.

bench-eval-bench runs the eval on every warmup_round regardless, so all 12 scores were already on disk. Confirmed by calling parse_eval_results on the winning variant's slot after the fact: accuracy=0.9689158453373768, task=gsm8k, metric=exact_match,strict-match. The predicate's only effect was discarding a number already paid for. baseline_accuracy=0.9704321455648218 was in state.json the whole time, so keeping that precondition is sufficient.

Recovered from disk, GSM8K exact_match,strict-match vs baseline 0.9704:

variant	accuracy	delta
vendor-proven-stack-mtp3	0.9704	0.0000
ci-online-ptpc-fp8-nonexpert (winner)	0.9689	-0.0015
cudagraph-mode-full	0.9659	-0.0045
max-num-seqs-64	0.9636	-0.0068

Worst drop 0.0068 against ACCURACY_THRESHOLD = 0.05, so unconditional gating would not have rejected a single variant here. The predicate and both lists are removed.

4. conc_sweep_failed is no longer terminal. The short-circuit sat ahead of should_reloop_to_explore(), so one failure ended the whole optimization. conc_sweep is a closeout concurrency scan, not the optimization itself, so its failure carries no evidence about whether the remaining budget could still find gain. It now flows into the normal reloop decision and stays the stop_reason when reloop is blocked, so the honest outcome survives without the budget being forfeited too. Reloop evidence gains sweep_exit_reason so a downgraded failure is still distinguishable after the fact.

Test plan

• New test_json_server_args_roundtrip.py (19 cases) locks the compose -> dedup -> lift chain, asserting on json.loads rather than exact strings so any join point that stops re-quoting fails regardless of normalization shape. The three cases added by 4e45fad3f cover the shared-string case above; all three fail on f3884adb9 and pass on 4e45fad3f.
• Discriminating: run against unmodified main, the suite is 7 failed / 10 passed. The failures are all the ATOM wildcard cases; the vLLM --compilation-config cases pass on both because their values have no * and the repair heuristic recovers them. That split is exactly why vLLM never hit this and ATOM did.
• 4e45fad3f checked the same way against its own parent: the server-args / grid-runner / accuracy suites fail on an identical set of 41 pre-existing cases on both commits, zero new and zero incidentally fixed.
• Regression checked by diffing failure sets (not counts) against a clean main worktree: server-args + accuracy suites 39 vs 39 identical; phase/termination suites 185 vs 185 with zero new; explore suites 45 vs 45. Pre-existing failures are a missing pytest-asyncio in the local venv, unrelated to this change.
• ruff check clean, ruff format applied.
• Updated test_sweep_closes_on_failed_conc_sweep_even_when_reloop_available, which locked the old terminal behaviour, and added a case asserting conc_sweep_failed still surfaces as the stop_reason when reloop is blocked, so the downgrade cannot launder the outcome.
• Pending: ATOM argparse accepts the repaired value on the GPU host. The incident died inside argparse, before any weight loading, so the sink check is python -m atom.entrypoints.openai_server --online_quant_config <composed value> and takes under a minute rather than a 12h run. Script: verify_atom_argparse.sh.
• Pending: a run long enough to reach EXPLORE with a baseline accuracy, to show a variant score now lands in current_best instead of being discarded. This is the only one of the four fixes that still needs GPU wall-clock.
• Fix 4 (conc_sweep_failed reloop) is covered by unit tests only. Now that the JSON corruption is fixed, conc_sweep is not expected to fail on this workload, so there is no natural end-to-end trigger to observe. Called out rather than papered over.

Not changed, and why

The incident report also suggested making SPACE_VALUE_FLAGS value-shape-driven (moot, see change 2) and supporting RUN_EVAL from .env. The latter is by design: RUN_EVAL is listed in BLOCKED_EXTERNAL_ENV_NAMES because workload/benchmark keys are owned by the CLI flags (--eval / --no-eval), so honouring it from .env would contradict that ownership. The warning is correct; only its wording could be clearer. The warm-replay REPRODUCED label inconsistency is a separate subsystem and is left for its own PR.

[github-actions]

CI E2E report — ❌ Timeout

item	value
result	❌ Timeout
model	Qwen/Qwen3-0.6B (dense)
resources	1× GPU, TP=1
PR branch	fix/atom-json-server-args-and-unconditional-accuracy
commit	f3884adb90058c03a6ddab74b23cfef44f9c5c20
session_id	2a8d418e-d785-4c9c-9edb-1834ca3c33c6
queue → dispatch	24m 1s
run time	125m 27s
total	149m 28s
reason	Timed out — the run never reached a terminal state in time (task stuck, or the GPU stayed queued too long).
detail	not terminal after 13200s

details