fix: Accept space-separated negative values on server CLI flags #1459

Part of #1431

Problem / Background

Both mlxcel serve and mlxcel-server reject a space-separated negative value for every option that takes a value, so a llama-server command line copied verbatim fails at argument parsing before the server ever starts.

mlxcel-server --seed -1              => error: unexpected argument '-1' found
mlxcel-server --predict -1           => error: unexpected argument '-1' found
mlxcel-server --reasoning-budget -1  => error: unexpected argument '-1' found
mlxcel serve  --seed -1              => error: unexpected argument '-1' found

The = form works. mlxcel-server --seed=-1 --model /nonexistent parses and fails only on the model path, which is why every existing test and script passes: they all use =.

The scope was measured, not estimated. mlxcel-server --dump-flag-surface lists 122 value-taking long options, and a sweep passing -1 in space form to each of them was rejected on all 122. The root cause is a single missing command setting: neither allow_negative_numbers nor allow_hyphen_values appears anywhere under src/ (zero grep matches), so clap treats any token beginning with - as an unknown flag rather than as a candidate value.

The pinned reference accepts the space form. Verified against the official ggml-org/llama.cpp b10621 archive llama-b10621-bin-macos-arm64.tar.gz, SHA-256 429c8270608600188035e5e92f7d78dffb7900904fe7dd7e6a84f48068cd13cf: llama-server --seed -1 --model /nonexistent produces no argument-parsing complaint and proceeds to model loading, failing only on the missing model.

Seventeen b10621 value-taking options document a negative value in their help text and therefore break when a llama-server command line is copied to mlxcel in space form: --seed, --predict, --keep, --parallel, --threads, --threads-http, --reasoning-budget, --sleep-idle-seconds, --sse-ping-interval, --top-nsigma, --embd-normalize, --logit-bias, --adaptive-target, --yarn-attn-factor, --yarn-beta-fast, --yarn-beta-slow, and --yarn-ext-factor. (--fim-qwen-1.5b-default was also on the initial list but is a valueless preset flag: its manifest entry in compat/llama-server/b10621/logging-and-presets.json records value_hint: null, so the parser defect cannot reach it.)

Those seventeen are spread across seven different sub-issues of #1431 (#1432, #1436, #1440, #1445, #1447, #1450, #1452). Fixing this once at the command level is what keeps it from being patched seven times, and it is what lets the thirteen options that mlxcel does not implement yet inherit the correct behavior when their own sub-issues land. This directly violates the epic's core principle that mlxcel-server accepts llama-server flags so existing deployment scripts keep working, and it is invisible to any test that only uses the = form.

Current Behavior

mlxcel-server defines its root command at src/bin/mlx_server.rs:63-64 (#[derive(Parser)] with #[command(name = "mlxcel-server", args_conflicts_with_subcommands = true, flatten_help = true, ...)]) and flattens ServerArgs (src/bin/mlx_server.rs:179) into it, so every server flag lives on the root command. mlxcel serve is the Serve(ServeArgs) variant at src/main.rs:98, generated from the #[derive(Args)] struct at src/main.rs:864-900 whose #[command(after_help = ...)] already demonstrably reaches the generated serve subcommand.

Neither command sets any negative-value option, which is clap's default posture: a token starting with - that does not match a known flag is reported as unexpected argument, and clap never considers it as the pending option's value.

The four affected options mlxcel implements today are seed: i64 with default_value_t = -1 at src/bin/mlx_server.rs:704-705 and src/main.rs:1364-1366, predict: i32 with short -n and visible alias --n-predict at src/bin/mlx_server.rs:248-256, parallel: usize at src/bin/mlx_server.rs:271-276, and reasoning_budget: i32 at src/main.rs:1786-1790. parallel is worth noting as the mixed case: b10621 documents a negative in its help text, but mlxcel's field is usize, so once the command accepts the space form the type parser is what must reject --parallel -1 with a value error. The remaining thirteen have mlxcel: null in the manifest, meaning they are not implemented yet and are owned by their own sub-issues.

The b10621 compatibility manifest already records this as a divergence on exactly three entries, each phrased as "the space-separated --X -1 that llama-server accepts is rejected by mlxcel's argv parser": --seed in compat/llama-server/b10621/sampling-and-grammar.json, --predict in compat/llama-server/b10621/runtime-and-context.json, and --reasoning-budget in compat/llama-server/b10621/chat-templates.json. All three are state: deferred.

Proposed Solution

Set allow_negative_numbers = true at the command level on both entry points: on the root #[command(...)] at src/bin/mlx_server.rs:64 for mlxcel-server, because ServerArgs is flattened into the root command and its arguments therefore belong to that command; and on the ServeArgs #[command(...)] at src/main.rs:866 for mlxcel serve, which is the attribute block that already carries after_help for that subcommand. Do not assume the derive placement propagates as expected: the parser tests below are what proves it, and if the setting does not reach the generated command from the flattened struct, move it to the parent command and keep the tests unchanged.

Prefer allow_negative_numbers over allow_hyphen_values. allow_hyphen_values accepts any token beginning with - as a value, so --seed --moldel foo would silently swallow the mistyped flag as the seed value and the typo would never be reported. allow_negative_numbers admits only tokens that parse as a number, which is exactly the domain b10621 documents.

Decide command-wide versus per-argument deliberately and record the decision with its reasoning in the PR body. Command-wide is the smaller change and is the only form that automatically covers the thirteen not-yet-implemented options when their own sub-issues land, which is the whole reason this is one issue rather than seven. Its cost is that -1 becomes a candidate value for options whose domain admits no negative, so the value validation must still reject it with a useful message rather than silently accepting it. That is already the case: unsigned fields such as port: u16 (src/bin/mlx_server.rs:237) and ctx_size: usize (src/bin/mlx_server.rs:242-246) fail in the type parser, and custom parsers such as parse_unit_interval (src/bin/mlx_server.rs:151) and parse_kv_cache_budget (src/bin/mlx_server.rs:173) fail in their own validation. Confirm this rather than assume it, and assert on the resulting message.

Keep every = form working. It is what every existing test and deployment script uses today, and no test may be rewritten from = to space form as part of this change.

Scope

In scope: src/main.rs (the ServeArgs command attributes) and src/bin/mlx_server.rs (the root Cli command attributes) for the parser fix; src/main_tests.rs and the #[cfg(test)] mod tests block at src/bin/mlx_server.rs:1737 for parser tests on both binaries; and the three manifest entries in compat/llama-server/b10621/sampling-and-grammar.json, compat/llama-server/b10621/runtime-and-context.json, and compat/llama-server/b10621/chat-templates.json that carry the argv-parser divergence string.

Out of scope: implementing any of the thirteen b10621 options that mlxcel does not accept yet; each is owned by its own sub-issue (#1432, #1436, #1440, #1445, #1450, #1452). Also out of scope: the field:seed entry in compat/llama-server/b10621/sampling-and-grammar.json, whose recorded divergence is "A seed below -1 is rejected with a 400", an HTTP field-validation difference introduced by #1430 and owned by #1436, not this argv defect; leave it byte-identical. Also out of scope: the second divergence on --seed ("Every negative value is treated as the random-seed sentinel"), which #1436 owns.

Implementation Notes

• Reuse the existing parse harnesses. parse_server_args at src/bin/mlx_server.rs:1806-1813 already wraps Cli::try_parse_from for the server binary, and src/main_tests.rs:451 shows the Cli::try_parse_from(["mlxcel", "serve", ...]) pattern for the CLI. Add cases to those; do not introduce a new test harness or a process-spawning wrapper.
• Reuse --dump-flag-surface for the sweep. src/cli/flag_surface.rs:58 defines the token, the document's args array is built at src/cli/flag_surface.rs:155, and each entry carries takes_value (src/cli/flag_surface.rs:142) and long without its leading dashes (src/cli/flag_surface.rs:102-109). Enumerate value-taking options from that dump rather than hand-listing them, so the sweep keeps covering options added later. The token must be the first argument after the binary name or after serve.
• Do not change the issue field on any edited manifest entry. scripts/ci/check_llama_compat_manifest.py requires every entry's issue to belong to its own shard's owner set in compat/llama-server/b10621/pin.json (sampling-and-grammar owns 1377 and 1436, runtime-and-context owns 1449, 1450 and 1453, chat-templates owns 1447). This issue's number is in none of those sets, so writing it into an entry fails the gate. Edit only the divergence list and, where the rule below applies, state.
• Manifest state rule, applied mechanically. After deleting the argv-parser divergence string from an entry, flip state from deferred to supported only when that entry's divergence list is then empty AND the entry already satisfies the supported requirements enforced from scripts/ci/check_llama_compat_manifest.py:342 onward: a named test, an mlxcel claim block, the canonical long spelling present in mlxcel.accepted_spellings, and mlxcel.env equal to the b10621 env. --predict and --reasoning-budget satisfy all of that today. --seed must stay deferred, because its second divergence survives and check_divergence at scripts/ci/check_llama_compat_manifest.py:247-268 makes supported with a non-empty divergence a hard error.
• Keep the shards canonically serialized. Each shard must stay byte-identical to canonical_dump at scripts/ci/check_llama_compat_manifest.py:185, which is json.dumps(doc, indent=2, ensure_ascii=False) plus a trailing newline, with entries in (kind, id) order and policy keys in the fixed order at scripts/ci/check_llama_compat_manifest.py:141. Hand-editing is acceptable as long as make verify-llama-compat passes; regenerating with the extractor is not required for a policy-field-only edit.
• Edge cases. -n -1 must work (short spelling, src/bin/mlx_server.rs:250). --n-predict -1 must work (visible alias, src/bin/mlx_server.rs:252). --seed=-1 and -s -1 must both keep working. A negative float such as -1.0 must be accepted where the option's type is floating point. A bare -1 appearing where no option is awaiting a value must still be an error rather than being silently consumed. The download subcommand (src/bin/mlx_server.rs:166-170) has no numeric option today: decide explicitly whether the setting applies there and say which in the PR body.
• Error handling. --port -1 and --ctx-size -1 must fail with clap's value-parse error naming the option and the offending value, not with unexpected argument and not by silently accepting. Assert on the message content in the tests so a later switch to allow_hyphen_values (which would swallow a following flag) is caught by the suite rather than by a user.

Acceptance Criteria

• mlxcel-server --seed -1 --model /nonexistent produces no argument-parsing error and fails only on the missing model path.
• mlxcel serve --seed -1 -m /nonexistent behaves identically.
• mlxcel-server --predict -1, mlxcel-server -n -1, mlxcel-server --n-predict -1, and mlxcel-server --reasoning-budget -1 all parse in space form.
• A sweep over every takes_value long option reported by --dump-flag-surface, passing -1 in space form, produces zero unexpected argument errors on both binaries. This is the sweep that today rejects all 122.
• Every = form still parses: --seed=-1, --predict=-1, --reasoning-budget=-1, and no existing test is rewritten from = form to space form.
• --port -1 is rejected on both binaries with a value-parse error whose text names --port and -1.
• Parser tests exist in both src/main_tests.rs and the src/bin/mlx_server.rs test module, each covering the space form, the = form, and the rejection of a negative for an option whose domain is non-negative.
• The argv-parser divergence string is gone from the --seed, --predict, and --reasoning-budget entries; --seed keeps its second divergence and stays deferred; field:seed is unchanged; make verify-llama-compat passes.
• The fix lives in the clap command definitions that the shipped mlxcel and mlxcel-server binaries actually parse with, not behind a helper, a wrapper, or an opt-in flag.

Verification

cargo build --release --features metal,accelerate

# Space form must reach model loading and fail only on the missing path.
./target/release/mlxcel-server --seed -1 --model /nonexistent
./target/release/mlxcel-server --predict -1 --model /nonexistent
./target/release/mlxcel-server -n -1 --model /nonexistent
./target/release/mlxcel-server --n-predict -1 --model /nonexistent
./target/release/mlxcel-server --reasoning-budget -1 --model /nonexistent
./target/release/mlxcel serve --seed -1 -m /nonexistent

# The `=` form must keep working.
./target/release/mlxcel-server --seed=-1 --model /nonexistent

# A non-negative domain must still reject -1, naming the option and the value.
./target/release/mlxcel-server --port -1 --model /nonexistent

# Full sweep over every value-taking long option, both binaries.
for BIN in "./target/release/mlxcel-server" "./target/release/mlxcel serve"; do
  $BIN --dump-flag-surface \
    | python3 -c 'import json,sys; print("\n".join(a["long"] for a in json.load(sys.stdin)["args"] if a.get("takes_value") and a.get("long")))' \
    | while read -r OPT; do
        OUT=$($BIN "--$OPT" -1 --model /nonexistent 2>&1)
        case "$OUT" in *"unexpected argument"*) echo "REJECTED: $BIN --$OPT" ;; esac
      done
done

make verify-llama-compat
cargo fmt --all -- --check
cargo clippy --workspace --all-targets -- -D warnings
cargo test --workspace --profile test-fast --features metal,accelerate

A pass looks like this: every space-form and =-form command above fails only on the missing model path with no argument-parsing complaint; --port -1 fails with a value-parse error that names --port and -1; the sweep prints no REJECTED: line on either binary; make verify-llama-compat and the lint commands are clean; and the workspace suite is green with a total near the usual full-scope count (a bare cargo test at the root resolves to -p mlxcel and skips three of the four members, so --workspace is not optional here).

Technical Considerations

This is a prerequisite for seven sub-issues of #1431 rather than a member of any one of them. Landing it before #1432, #1436, #1440, #1445, #1447, #1450, and #1452 means each of those can register its options as supported without first rediscovering the same parser defect, and the thirteen options none of them has implemented yet inherit the correct behavior on arrival. Landing it after them means seven separate patches to the same two attribute blocks and seven separate manifest divergence strings to retract.

The --check-issues-open variant of the manifest gate runs in the llama-compat manifest CI job (.github/workflows/ci.yml:273) and fails a deferred entry that links a closed issue. Any entry flipped to supported here stops being subject to that check, so confirm the linked sub-issue does not still need the entry as a tracked item before flipping it.