Skip to content

fix(server): reject unsupported GGML runtime flags and KV formats - #1469

Merged
inureyes merged 2 commits into
mainfrom
fix/issue-1445-ggml-runtime-rejection
Aug 27, 2026
Merged

inureyes merged 2 commits into
mainfrom
fix/issue-1445-ggml-runtime-rejection

Conversation

@inureyes

@inureyes inureyes commented Aug 27, 2026 •

Copy link
Copy Markdown
Member

Summary

Every option in b10621's GGML runtime, placement, and memory group describes the GGML backend: which CPU cores its thread pool runs on, how many layers to copy into VRAM, how to split a model across GPUs, whether to mmap or mlock the GGUF file, which RPC servers to farm work out to, and which GGML quantizer stores the KV cache. mlxcel runs every tensor operation through MLX on one Metal or CUDA device and has no GGML backend, so almost none of it has a counterpart.

Before this change six of those options parsed and did nothing while the other thirty-seven were rejected by clap as unknown arguments. Neither group told the operator anything true: a deployment passing --n-gpu-layers 20 believed twelve layers were on the CPU, and -ngl auto (b10621's own default spelling) did not start at all.

What changed

  • src/cli/ggml_compat_args.rs (new): one flattened clap group carrying all forty-three spellings, hidden from --help, that classifies every value at startup before a weight is read.
    • Inert, accepted silently: --split-mode none, --threads -1, --cpu-strict 0, --poll 50, --prio 0, --flash-attn on|auto, --gpu-layers all, --main-gpu 0, --load-mode auto|mmap, --mmap, --no-direct-io, both halves of --repack, --kv-offload, --op-offload, --no-perf, --n-cpu-moe 0, --fit off, and --backend-sampling, which asks for the on-accelerator sampler mlxcel always uses.
    • Rejected with the option, the value, the limitation, and the alternative: --split-mode row and --tensor-split point at docs/distributed.md, --rpc at --node-role / --peers, --mlock and --load-mode mlock at MLXCEL_WIRED_LIMIT, --device at MLXCEL_DEVICE, --perf at --metrics, --fit on at --estimate-memory, --no-kv-offload at --cache-type-k / --cache-type-v, --check-tensors and --list-devices at mlxcel inspect.
  • src/cli/turbo_args.rs: --cache-type-k / --cache-type-v gain a value-specific diagnostic. A GGML quantizer name now explains why the vocabularies do not carry over and lists the mlxcel modes that do exist; anything else gets the plain accepted list.
  • src/main.rs, src/bin/mlx_server.rs, src/commands/serve.rs: the group is flattened into both binaries, replacing the four ad-hoc _n_gpu_layers / _flash_attn / _mlock / _no_mmap fields that parsed and did nothing. Classification runs in two stages, before and after model resolution.
  • compat/llama-server/b10621/ggml-runtime.json: all forty-two entries move off deferred.
  • tests/llama_compat_manifest.rs: the two census floors become one partition invariant.
  • docs/llama-server-compat.md, docs/environment-variables.md: the operator-facing boundary and the environment vocabulary.

KV cache formats: exactly one alias, and no invented equivalence

f16 names the same thing on both sides, unquantized half-precision KV storage, so it maps onto KVCacheMode::Fp16. That is the only value the two vocabularies share.

Every other GGML name is a different quantizer, not a different name for the same one. q8_0 is block-wise with a per-block scale; mlxcel's int8 is per-token absmax; the Turbo modes are PolarQuant with a Walsh-Hadamard rotation. None of q8_0, q4_0, q4_1, iq4_nl, q5_0, q5_1, f32 or bf16 is mapped onto a TurboQuant mode, because no numerical or storage equivalence has been demonstrated and naming similarity is not evidence. Each is rejected at startup with a diagnostic naming the value, why the vocabularies do not carry over, and the mlxcel modes that do exist. mlxcel's own int8, fp16+turbo4, fp16+turbo3, turbo4 and turbo4-delegated stay on the same flag and stay visible in --help.

No new KV representation or mapping is introduced, so the issue's teacher-forced logit-trace requirement has nothing to measure here. If anyone later wants one of those GGML types aliased, that procedure is what would have to run first; the doc says so.

Two design points worth review

  • --gpu-layers is classified after the model resolves. Only the checkpoint's num_hidden_layers separates a full offload (inert: mlxcel always runs every layer on the accelerator) from a partial one. Everything else is classified before, so --numa distribute is reported immediately instead of after a multi-gigabyte download. When the layer count cannot be read, a non-negative budget is unsupported rather than guessed at.
  • Nine options bind their environment variable at runtime, not through clap. b10621 fires a value-less option from the environment only for on/enabled/true/1 (common_arg_utils::is_truthy, case-sensitive) and reads a --x / --no-x pair through parse_bool_value plus a LLAMA_ARG_NO_* alias meaning false. clap's boolish parser accepts a wider vocabulary and errors outside it, so binding through clap would both enable options on spellings b10621 ignores and refuse to start on values it silently drops. LLAMA_ARG_CPU_MOE=0 therefore does not enable --cpu-moe, and LLAMA_ARG_PERF=sometimes stops startup exactly as upstream throws.

Behavior change for existing deployments

Five options that mlxcel previously accepted and ignored can now stop startup: --n-gpu-layers <partial>, --flash-attn off, --mlock, --no-mmap, and the GGML KV cache types. That is the point of the issue ("No option with an observable value is accepted and ignored"), but it is a deployment-visible change for anyone copying a llama-server command line, so every rejection names what to pass instead and docs/environment-variables.md says so.

Manifest census

The two assertions in tests/llama_compat_manifest.rs floored the claimed and unclaimed halves of the option inventory at hand-picked numbers. This PR converts forty-two entries from unclaimed to claimed, which is exactly what epic #1431 exists to do, so the unclaimed floor tripped. Rather than lowering a magic number that will trip again on the next chain, the two halves now have to add up to b10621's frozen 249 help entries. That invariant cannot go stale and still catches a shard that loses entries.

Test plan

  • cargo test --profile test-fast --features metal,accelerate --test llama_ggml_runtime_cli (9 passed) — every upstream spelling reaches mlxcel's own classification rather than clap; inert values fail only for the missing model; unsupported values name option, value, limitation and alternative on both binaries; a GGML KV quantizer is refused with the mlxcel vocabulary; the environment vocabulary matches b10621's; nothing appears in --help.
  • cargo test --profile test-fast --features metal,accelerate --lib cli::ggml_compat_args (27 passed) and --lib cli::turbo_args (19 passed)
  • cargo test --profile test-fast --features metal,accelerate --test llama_compat_manifest (4), --test llama_model_source_cli (16), --test cli_help_consistency (25)
  • cargo test --profile test-fast --features metal,accelerate --lib cli:: (107), --lib server::cli_input (120), --lib server::startup (61), --lib downloader:: (162)
  • cargo test --profile test-fast --features metal,accelerate --bin mlxcel (214) and --bin mlxcel-server (28)
  • make verify-llama-compat
  • cargo clippy --profile test-fast --features metal,accelerate --lib --tests --bins -- -D warnings
  • cargo fmt --all -- --check

Review round two

A review found two false rejections that would refuse a command line llama-server starts, a diagnostic class naming flags mlxcel does not have, and the short spellings still going nowhere. All fixed in the second commit.

  • --defrag-thold is a no-op upstream, so it is inert here. b10621's handler is literally GGML_UNUSED(params); GGML_UNUSED(value); plus a deprecation warning. Rejecting it refused a command line that starts upstream, and the manifest note claimed a defragmentation pass upstream no longer has.
  • read_model_layer_count reproduced the fix: KV estimation misses n_layer / n_embd for GPT-2 and GPT-BigCode #927 gap. It read num_hidden_layers by hand; execution::config_fields::LAYER_COUNT_KEYS exists because that alias chain was already copied five times. gpt2 and gpt_bigcode use n_layer, dbrx / falcon_ocr / jina_vlm / molmo use n_layers, exaone uses num_layers; on all of them every non-negative --gpu-layers was rejected, including a full offload, and this PR newly binds LLAMA_ARG_N_GPU_LAYERS, so a container carrying LLAMA_ARG_N_GPU_LAYERS=99 would have stopped starting.
  • --load-mode is now the authority over the deprecated loading flags. All four write one field upstream in command-line order, and b10621 deprecates the three in favour of the first while warning that combining them is last-wins, so --no-mmap --load-mode mmap asks for mmap. The residue (a deprecated flag written after --load-mode) is recorded as a divergence on all four entries rather than reconstructed from argv order.
  • The diagnostics named flags that do not exist. --tensor-parallel, --pipeline-parallel and --peers are now --tp-size, --pp-size and --cluster-peers. An alternative that produces "unexpected argument" misses the issue's own acceptance criterion.
  • All twenty-eight upstream short spellings now reach their option. -ngl is the most common llama-server flag and clap read every one of these as a cluster of one-letter shorts. SHORT_ALIASES in the argv pre-pass gains them, whether the rewritten option consumes the next entry is now read from the command rather than assumed, and a test asserts no table token shadows an mlxcel short.
  • f32 and bf16 are float types, not quantizers, and get their own sentence naming the real reason (mlxcel's KV cache has no f32 or bf16 storage).
  • Smaller: the KV cache type is validated before the model resolves, so --cache-type-k q8_0 --hf-repo … reports the value instead of downloading first; --threads accepts any value <= 0 (upstream's own "use hardware concurrency"); inert numeric values compare as numbers so 00 and +0 work; --numa "" is b10621's distribute; and --load-mode none|dio no longer offers MLXCEL_WIRED_LIMIT, which is a residency control.

The manifest validator now checks the function half of a file::name test pointer. Several entries here pointed at an integration test that did not cover them and nothing noticed, because only the file half was validated. scripts/ci/check_llama_compat_manifest_test.sh gains the negative case and a positive control, and every entry's pointer now names the test that actually exercises it.

Closes #1445

Every option in b10621's GGML runtime, placement, and memory group describes the GGML backend: which CPU cores its thread pool runs on, how many layers to copy into VRAM, how to split a model across GPUs, whether to mmap or mlock the GGUF file, which RPC servers to farm work out to, and which GGML quantizer stores the KV cache. mlxcel runs every tensor operation through MLX on one Metal or CUDA device and has no GGML backend, so almost none of it has a counterpart. Before this change six of those options parsed and did nothing and the other thirty-seven were rejected by clap as unknown arguments, so neither group told the operator anything true: a deployment passing `--n-gpu-layers 20` believed twelve layers were on the CPU, and one passing `-ngl auto` did not start at all.

`src/cli/ggml_compat_args.rs` is one flattened clap group carrying all forty-three spellings, hidden so `--help` never implies a GGML backend, and it classifies every value at startup before a weight is read. A value whose b10621 meaning mlxcel already satisfies is inert and accepted silently: `--split-mode none`, `--threads -1`, `--cpu-strict 0`, `--poll 50`, `--prio 0`, `--flash-attn on|auto`, `--gpu-layers all`, `--main-gpu 0`, `--load-mode auto|mmap`, `--mmap`, `--no-direct-io`, both halves of `--repack`, `--kv-offload`, `--op-offload`, `--no-perf`, `--n-cpu-moe 0`, `--fit off`, and `--backend-sampling`, which asks for the on-accelerator sampler mlxcel always uses. Anything else stops startup with a diagnostic naming the option, the value, the platform limitation, and the mlxcel alternative: `--split-mode row` and `--tensor-split` point at docs/distributed.md, `--rpc` at `--node-role` / `--peers`, `--mlock` and `--load-mode mlock` at `MLXCEL_WIRED_LIMIT`, `--device` at `MLXCEL_DEVICE`, `--perf` at `--metrics`, `--fit on` at `--estimate-memory`, `--no-kv-offload` at `--cache-type-k` / `--cache-type-v`, and `--check-tensors` / `--list-devices` at `mlxcel inspect`.

`--gpu-layers` is the one option whose classification needs the checkpoint, because only `num_hidden_layers` separates a full offload (inert: mlxcel always runs every layer on the accelerator) from a partial one. It is checked after the model reference resolves; everything else is checked before, so `--numa distribute` is reported immediately instead of after a multi-gigabyte download. When the layer count cannot be read the value is unsupported rather than guessed at. `--flash-attn` also changes shape: b10621 requires a value (`on|off|auto`) and mlxcel had it as a value-less boolean, so `--flash-attn on` did not parse.

KV cache formats keep exactly one alias. `f16` names the same thing on both sides, unquantized half-precision KV storage, so it maps onto `KVCacheMode::Fp16`. Every other GGML name is a different quantizer, not a different name for the same one: `q8_0` is block-wise with a per-block scale, mlxcel's `int8` is per-token absmax, and the Turbo modes are PolarQuant with a Walsh-Hadamard rotation. None of `q8_0`, `q4_0`, `q4_1`, `iq4_nl`, `q5_0`, `q5_1`, `f32` or `bf16` is mapped onto a TurboQuant mode, because no numerical or storage equivalence has been demonstrated and naming similarity is not evidence; each is rejected with a diagnostic naming the value, why the vocabularies do not carry over, and the mlxcel modes that do exist. mlxcel's own `int8` and Turbo modes stay on the same flag and stay visible in `--help`.

Nine of these options bind their environment variable at runtime rather than through clap. b10621 fires a value-less option from the environment only for `on`/`enabled`/`true`/`1` (`common_arg_utils::is_truthy`, case-sensitive) and reads a `--x` / `--no-x` pair through `parse_bool_value` plus a `LLAMA_ARG_NO_*` alias meaning false, while clap's boolish parser accepts a wider vocabulary and errors outside it. Binding through clap would both enable options on spellings b10621 ignores and refuse to start on values it silently drops.

The manifest's two census assertions become one partition invariant. They floored the claimed and unclaimed halves at hand-picked numbers, and this change converts forty-two entries from unclaimed to claimed, which is exactly what epic #1431 exists to do; the halves now have to add up to b10621's frozen 249 help entries instead, which cannot go stale and still catches a shard that loses entries.

All forty-two `ggml-runtime.json` entries move off `deferred`: `--cache-type-k` and `--cache-type-v` to `aliased` with their value-domain divergences recorded, and the other forty to `not_applicable`, each naming its diagnostic test.

Validation: `cargo test --test llama_ggml_runtime_cli` (9), `--test llama_compat_manifest` (4), `--test llama_model_source_cli` (16), `--test cli_help_consistency` (25), `--lib cli::ggml_compat_args` (27), `--lib cli::turbo_args` (19), `--lib server::cli_input` (110), `--lib server::startup` (61), `--lib downloader::` (162), `--bin mlxcel` (214), `--bin mlxcel-server` (28); `make verify-llama-compat`; `cargo clippy --lib --tests --bins -- -D warnings`; `cargo fmt --all -- --check`.

Closes #1445
@inureyes inureyes added status:review Under review type:bug Bug fixes, error corrections, or issue resolutions priority:high High priority area:docs User and developer documentation area:core mlxcel-core: MLX FFI, primitives, KV cache, layers area:cli Command-line interface / CLI flags labels Aug 27, 2026
Review of the first pass found two false rejections that would refuse a command line llama-server starts, a diagnostic class that named flags mlxcel does not have, and the short spellings still going nowhere.

`--defrag-thold` was rejected as unsupported, but b10621's handler is literally `GGML_UNUSED(params); GGML_UNUSED(value);` plus a deprecation warning: no value of it changes anything upstream, which makes it the clearest provably-inert value in the shard. It is now accepted, and the manifest note no longer claims upstream defragments a KV cache on a threshold it removed.

`read_model_layer_count` read `num_hidden_layers` and `text_config.num_hidden_layers` by hand. `execution::config_fields::LAYER_COUNT_KEYS` exists precisely so that alias list is written once, after the same hand-rolled chain reproduced the #927 gap in five places; gpt2 and gpt_bigcode spell it `n_layer`, dbrx, falcon_ocr, jina_vlm and molmo `n_layers`, exaone `num_layers`. On those families the layer count came back `None` and every non-negative `--gpu-layers` was rejected, including one covering the whole model, and this change newly binds `LLAMA_ARG_N_GPU_LAYERS` through clap, so a container carrying the near-universal `LLAMA_ARG_N_GPU_LAYERS=99` would have stopped starting. It now goes through `config_fields`.

`--load-mode`, `--mlock`, `--mmap` and `--direct-io` write one `params.load_mode` field upstream in command-line order, and b10621 deprecates the last three in favour of the first while warning that combining them is last-wins. Classifying them as four independent flags rejected `--no-mmap --load-mode mmap`, which asks for mmap upstream. `--load-mode` is now the authority when present and the deprecated three are not classified at all; the residue, a deprecated flag written after `--load-mode`, is recorded as a divergence on all four entries rather than reconstructed from argv order.

The multi-device and RPC diagnostics named `--tensor-parallel`, `--pipeline-parallel` and `--peers`, none of which exist. They now name `--tp-size`, `--pp-size` and `--cluster-peers`. The issue's acceptance criterion asks for the supported mlxcel alternative, so a name that produces "unexpected argument" was a criterion miss.

All twenty-eight upstream short spellings in this shard now reach their option. `-ngl` is the single most common llama-server flag and clap reads a single dash as a cluster of one-letter shorts, so every one of them was an unknown argument. `SHORT_ALIASES` in the argv pre-pass gains them; whether the rewritten option consumes the next entry is now read from the command rather than assumed, so value-less tokens such as `-cmoe` and value-taking ones such as `-ngl` share one table. A test asserts no table token shadows an mlxcel short, since the pass rewrites before consulting clap.

`f32` and `bf16` were described as GGML quantizers. They are unquantized float types, and the real reason mlxcel refuses them is that its KV cache has no f32 or bf16 storage; they get their own sentence.

The KV cache type is now validated before the model reference resolves, so `--cache-type-k q8_0 --hf-repo …` reports the value instead of downloading first. `--threads` accepts any value `<= 0`, which upstream replaces with the hardware concurrency; the numeric inert values are compared as numbers, so `00` and `+0` are the same request as `0`; `--numa ""` is b10621's `distribute` rather than an absent value; and `--load-mode none|dio` no longer offers `MLXCEL_WIRED_LIMIT`, which is a residency control relevant only to the residency modes.

Finally, the manifest validator now checks the function half of a `file::name` test pointer, not just the file. Several entries here pointed at an integration test that did not cover them, and nothing noticed; `scripts/ci/check_llama_compat_manifest_test.sh` gains the negative case and a positive control. Every entry's pointer now names the test that actually exercises it.

Validation: `cargo test --test llama_ggml_runtime_cli` (11), `--test llama_compat_manifest` (4), `--test llama_model_source_cli` (16), `--test cli_help_consistency` (25), `--lib cli::` (116), `--lib server::cli_input` (120), `--lib server::startup` (61), `--lib downloader::` (162), `--lib server::llama_compat` (3), `--bin mlxcel` (214), `--bin mlxcel-server` (28); `make verify-llama-compat` including the validator's own negative coverage; `cargo clippy --lib --tests --bins -- -D warnings`; `cargo fmt --all -- --check`.

Refs #1445
@inureyes
inureyes merged commit b5d29a6 into main Aug 27, 2026
12 checks passed
@inureyes
inureyes deleted the fix/issue-1445-ggml-runtime-rejection branch August 27, 2026 12:12
@inureyes inureyes added status:done Completed and removed status:review Under review labels Aug 27, 2026
@inureyes inureyes self-assigned this Aug 31, 2026
be-free-snowboy pushed a commit to freestylejs/snowbrain-mlxcel that referenced this pull request Oct 27, 2026
…blup#1469)

## Summary

Every option in b10621's GGML runtime, placement, and memory group describes the GGML backend: which CPU cores its thread pool runs on, how many layers to copy into VRAM, how to split a model across GPUs, whether to `mmap` or `mlock` the GGUF file, which RPC servers to farm work out to, and which GGML quantizer stores the KV cache. mlxcel runs every tensor operation through MLX on one Metal or CUDA device and has no GGML backend, so almost none of it has a counterpart.

Before this change six of those options parsed and did nothing while the other thirty-seven were rejected by clap as unknown arguments. Neither group told the operator anything true: a deployment passing `--n-gpu-layers 20` believed twelve layers were on the CPU, and `-ngl auto` (b10621's own default spelling) did not start at all.

## What changed

- **`src/cli/ggml_compat_args.rs`** (new): one flattened clap group carrying all forty-three spellings, hidden from `--help`, that classifies every value at startup before a weight is read.
  - **Inert, accepted silently**: `--split-mode none`, `--threads -1`, `--cpu-strict 0`, `--poll 50`, `--prio 0`, `--flash-attn on|auto`, `--gpu-layers all`, `--main-gpu 0`, `--load-mode auto|mmap`, `--mmap`, `--no-direct-io`, both halves of `--repack`, `--kv-offload`, `--op-offload`, `--no-perf`, `--n-cpu-moe 0`, `--fit off`, and `--backend-sampling`, which asks for the on-accelerator sampler mlxcel always uses.
  - **Rejected with the option, the value, the limitation, and the alternative**: `--split-mode row` and `--tensor-split` point at `docs/distributed.md`, `--rpc` at `--node-role` / `--peers`, `--mlock` and `--load-mode mlock` at `MLXCEL_WIRED_LIMIT`, `--device` at `MLXCEL_DEVICE`, `--perf` at `--metrics`, `--fit on` at `--estimate-memory`, `--no-kv-offload` at `--cache-type-k` / `--cache-type-v`, `--check-tensors` and `--list-devices` at `mlxcel inspect`.
- **`src/cli/turbo_args.rs`**: `--cache-type-k` / `--cache-type-v` gain a value-specific diagnostic. A GGML quantizer name now explains why the vocabularies do not carry over and lists the mlxcel modes that do exist; anything else gets the plain accepted list.
- **`src/main.rs`, `src/bin/mlx_server.rs`, `src/commands/serve.rs`**: the group is flattened into both binaries, replacing the four ad-hoc `_n_gpu_layers` / `_flash_attn` / `_mlock` / `_no_mmap` fields that parsed and did nothing. Classification runs in two stages, before and after model resolution.
- **`compat/llama-server/b10621/ggml-runtime.json`**: all forty-two entries move off `deferred`.
- **`tests/llama_compat_manifest.rs`**: the two census floors become one partition invariant.
- **`docs/llama-server-compat.md`, `docs/environment-variables.md`**: the operator-facing boundary and the environment vocabulary.

## KV cache formats: exactly one alias, and no invented equivalence

`f16` names the same thing on both sides, unquantized half-precision KV storage, so it maps onto `KVCacheMode::Fp16`. That is the only value the two vocabularies share.

Every other GGML name is a **different quantizer, not a different name for the same one**. `q8_0` is block-wise with a per-block scale; mlxcel's `int8` is per-token absmax; the Turbo modes are PolarQuant with a Walsh-Hadamard rotation. None of `q8_0`, `q4_0`, `q4_1`, `iq4_nl`, `q5_0`, `q5_1`, `f32` or `bf16` is mapped onto a TurboQuant mode, because no numerical or storage equivalence has been demonstrated and naming similarity is not evidence. Each is rejected at startup with a diagnostic naming the value, why the vocabularies do not carry over, and the mlxcel modes that do exist. mlxcel's own `int8`, `fp16+turbo4`, `fp16+turbo3`, `turbo4` and `turbo4-delegated` stay on the same flag and stay visible in `--help`.

No new KV representation or mapping is introduced, so the issue's teacher-forced logit-trace requirement has nothing to measure here. If anyone later wants one of those GGML types aliased, that procedure is what would have to run first; the doc says so.

## Two design points worth review

- **`--gpu-layers` is classified after the model resolves.** Only the checkpoint's `num_hidden_layers` separates a full offload (inert: mlxcel always runs every layer on the accelerator) from a partial one. Everything else is classified before, so `--numa distribute` is reported immediately instead of after a multi-gigabyte download. When the layer count cannot be read, a non-negative budget is unsupported rather than guessed at.
- **Nine options bind their environment variable at runtime, not through clap.** b10621 fires a value-less option from the environment only for `on`/`enabled`/`true`/`1` (`common_arg_utils::is_truthy`, case-sensitive) and reads a `--x` / `--no-x` pair through `parse_bool_value` plus a `LLAMA_ARG_NO_*` alias meaning false. clap's boolish parser accepts a wider vocabulary and *errors* outside it, so binding through clap would both enable options on spellings b10621 ignores and refuse to start on values it silently drops. `LLAMA_ARG_CPU_MOE=0` therefore does not enable `--cpu-moe`, and `LLAMA_ARG_PERF=sometimes` stops startup exactly as upstream throws.

## Behavior change for existing deployments

Five options that mlxcel previously accepted and ignored can now stop startup: `--n-gpu-layers <partial>`, `--flash-attn off`, `--mlock`, `--no-mmap`, and the GGML KV cache types. That is the point of the issue ("No option with an observable value is accepted and ignored"), but it is a deployment-visible change for anyone copying a llama-server command line, so every rejection names what to pass instead and `docs/environment-variables.md` says so.

## Manifest census

The two assertions in `tests/llama_compat_manifest.rs` floored the claimed and unclaimed halves of the option inventory at hand-picked numbers. This PR converts forty-two entries from unclaimed to claimed, which is exactly what epic lablup#1431 exists to do, so the unclaimed floor tripped. Rather than lowering a magic number that will trip again on the next chain, the two halves now have to add up to b10621's frozen 249 help entries. That invariant cannot go stale and still catches a shard that loses entries.

## Test plan

- [x] `cargo test --profile test-fast --features metal,accelerate --test llama_ggml_runtime_cli` (9 passed) — every upstream spelling reaches mlxcel's own classification rather than clap; inert values fail only for the missing model; unsupported values name option, value, limitation and alternative on both binaries; a GGML KV quantizer is refused with the mlxcel vocabulary; the environment vocabulary matches b10621's; nothing appears in `--help`.
- [x] `cargo test --profile test-fast --features metal,accelerate --lib cli::ggml_compat_args` (27 passed) and `--lib cli::turbo_args` (19 passed)
- [x] `cargo test --profile test-fast --features metal,accelerate --test llama_compat_manifest` (4), `--test llama_model_source_cli` (16), `--test cli_help_consistency` (25)
- [x] `cargo test --profile test-fast --features metal,accelerate --lib cli::` (107), `--lib server::cli_input` (120), `--lib server::startup` (61), `--lib downloader::` (162)
- [x] `cargo test --profile test-fast --features metal,accelerate --bin mlxcel` (214) and `--bin mlxcel-server` (28)
- [x] `make verify-llama-compat`
- [x] `cargo clippy --profile test-fast --features metal,accelerate --lib --tests --bins -- -D warnings`
- [x] `cargo fmt --all -- --check`

## Review round two

A review found two false rejections that would refuse a command line llama-server starts, a diagnostic class naming flags mlxcel does not have, and the short spellings still going nowhere. All fixed in the second commit.

- **`--defrag-thold` is a no-op upstream, so it is inert here.** b10621's handler is literally `GGML_UNUSED(params); GGML_UNUSED(value);` plus a deprecation warning. Rejecting it refused a command line that starts upstream, and the manifest note claimed a defragmentation pass upstream no longer has.
- **`read_model_layer_count` reproduced the lablup#927 gap.** It read `num_hidden_layers` by hand; `execution::config_fields::LAYER_COUNT_KEYS` exists because that alias chain was already copied five times. gpt2 and gpt_bigcode use `n_layer`, dbrx / falcon_ocr / jina_vlm / molmo use `n_layers`, exaone uses `num_layers`; on all of them every non-negative `--gpu-layers` was rejected, including a full offload, and this PR newly binds `LLAMA_ARG_N_GPU_LAYERS`, so a container carrying `LLAMA_ARG_N_GPU_LAYERS=99` would have stopped starting.
- **`--load-mode` is now the authority over the deprecated loading flags.** All four write one field upstream in command-line order, and b10621 deprecates the three in favour of the first while warning that combining them is last-wins, so `--no-mmap --load-mode mmap` asks for mmap. The residue (a deprecated flag written *after* `--load-mode`) is recorded as a divergence on all four entries rather than reconstructed from argv order.
- **The diagnostics named flags that do not exist.** `--tensor-parallel`, `--pipeline-parallel` and `--peers` are now `--tp-size`, `--pp-size` and `--cluster-peers`. An alternative that produces "unexpected argument" misses the issue's own acceptance criterion.
- **All twenty-eight upstream short spellings now reach their option.** `-ngl` is the most common llama-server flag and clap read every one of these as a cluster of one-letter shorts. `SHORT_ALIASES` in the argv pre-pass gains them, whether the rewritten option consumes the next entry is now read from the command rather than assumed, and a test asserts no table token shadows an mlxcel short.
- **`f32` and `bf16` are float types, not quantizers**, and get their own sentence naming the real reason (mlxcel's KV cache has no f32 or bf16 storage).
- Smaller: the KV cache type is validated before the model resolves, so `--cache-type-k q8_0 --hf-repo …` reports the value instead of downloading first; `--threads` accepts any value `<= 0` (upstream's own "use hardware concurrency"); inert numeric values compare as numbers so `00` and `+0` work; `--numa ""` is b10621's `distribute`; and `--load-mode none|dio` no longer offers `MLXCEL_WIRED_LIMIT`, which is a residency control.

**The manifest validator now checks the function half of a `file::name` test pointer.** Several entries here pointed at an integration test that did not cover them and nothing noticed, because only the file half was validated. `scripts/ci/check_llama_compat_manifest_test.sh` gains the negative case and a positive control, and every entry's pointer now names the test that actually exercises it.

Closes lablup#1445
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:cli Command-line interface / CLI flags area:core mlxcel-core: MLX FFI, primitives, KV cache, layers area:docs User and developer documentation priority:high High priority status:done Completed type:bug Bug fixes, error corrections, or issue resolutions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(server): reject unsupported GGML runtime flags and KV formats explicitly

1 participant