Skip to content

fix!: retire the CLI flags llama.cpp's server parser rejects, and guard the contract - #426

Merged
bernardladenthin merged 1 commit into
mainfrom
claude/cli-flag-contract-guard
Sep 9, 2026
Merged

fix!: retire the CLI flags llama.cpp's server parser rejects, and guard the contract#426
bernardladenthin merged 1 commit into
mainfrom
claude/cli-flag-contract-guard

Conversation

@bernardladenthin

Copy link
Copy Markdown
Owner

Summary

  • Seven flags the Java layer emitted are rejected by llama.cpp's server argument parser, which makes the matching builder methods produce an unloadable model, not a merely ineffective one. LlamaModel.loadModel(parameters.toArray()) hands the ModelParameters map to common_params_parse(..., LLAMA_EXAMPLE_SERVER) as argv, and an unregistered option there is a hard error — arg.cpp throws, common_params_parse returns false, load_model_impl throws LlamaException("Failed to parse model parameters").
  • Two are fresh breakage from the b10878 bump (feat!: upgrade llama.cpp from b10870 to b10878 #425): upstream deleted --mlock and --no-mmap, deprecated at b10092 — the whole deprecation window opened and closed inside eight tags. These have a faithful replacement, so no API is lost: a new args.LoadMode enum + ModelParameters.setLoadMode(LoadMode) expose upstream's -lm/--load-mode, and enableMlock() / disableMmap() keep working, re-pointed to LoadMode.MLOCK / LoadMode.NONE — the exact mapping upstream's own deprecation shim used (LLAMA_LOAD_MODE_MLOCK / LLAMA_LOAD_MODE_NONE), so behaviour is unchanged. Both are now @Deprecated.
  • Five were already dead and are now @Deprecated no-ops that write nothing, keeping existing call sites compiling and loading. ModelFlag.MLOCK / NO_MMAP / DUMP_KV_CACHE are removed from the enum so the broken argv is not reachable through setFlag either — the same reasoning that already excluded FLASH_ATTN.
  • A contract test now enforces this, because no Java test can: ModelFlagTest and ModelParametersExtendedTest assert the string mapping (hasKey("--mlock")), never that llama.cpp still accepts the string, so they stayed green for as long as the flags were dead.

The seven, and why each is dead

Flag Java member Status upstream Now
--mlock enableMlock(), ModelFlag.MLOCK deprecated b10092, deleted b10878 emits --load-mode mlock
--no-mmap disableMmap(), ModelFlag.NO_MMAP deprecated b10092, deleted b10878 emits --load-mode none
--dump-kv-cache enableDumpKvCache(), ModelFlag.DUMP_KV_CACHE removed, no replacement no-op
--hf-repo-v setHfRepoV(String) removed with the OuteTTS-era two-model TTS design no-op
--hf-file-v setHfFileV(String) same no-op
--grp-attn-n setGrpAttnN(int) present in arg.cpp, set_examples({COMPLETION, PASSKEY}) no-op
--grp-attn-w setGrpAttnW(int) present in arg.cpp, set_examples({COMPLETION}) no-op

The last two are the interesting ones: they exist in common/arg.cpp at every tag this project has pinned, so any textual sweep reports them alive. common_params_parser_init's add_opt filters by example at registration time, so they are never registered for LLAMA_EXAMPLE_SERVER — the example this binding parses with — and the parser rejects them exactly like a deleted flag. That is precisely why the guard below drives the real option table instead of grepping upstream sources.

The guard

  • llama/cmake/extract-java-cli-flags.cmake — at configure time, extracts every "--flag" string literal ModelFlag.java + ModelParameters.java can emit into a generated header. Line-oriented so Javadoc mentions ({@code --flash-attn}, prose naming --mlock) are dropped, while enum constants, putScalar/putEnum keys, parameters.put keys and the private ARG_* constants all survive. The Java sources stay the single source of truth, so the two halves cannot drift; it fails loud below a 50-flag floor so a broken extractor cannot make the test vacuously pass.
  • llama/src/test/cpp/test_model_flags.cpp (4 tests) — asserts each extracted flag is present in common_params_parser_init(params, LLAMA_EXAMPLE_SERVER).options. Hermetic: no model, no JVM, no network — common_params_parser_init only fills a struct. It also checks the oracle (the option table must be plausibly large and contain --model) and that the exemption list has not rotted, so neither an empty table nor a stale exemption can hide a real failure. --vocab-only is the one exemption, and deliberately so: it is a project pseudo-flag that jllama.cpp removes via strip_flag_from_argv before common_params_parse sees the argv, to select the vocab-only path.

Falsified before it was trusted. Built against the pre-fix Java sources, the test named exactly the seven flags above — including the two set_examples()-scoped ones my own grep-based audit had missed. After the fix it is green.

Test plan

  • Affected unit / integration tests pass locally
  • CI is green on this branch
  • Docs / CHANGELOG updated where applicable

Run locally on Linux x86_64:

  • ctest 531/531 (was 527; +4 from the new file)
  • mvn test 1759 tests, 0 failures, 0 errors
  • PIT 320/320 mutations killed, 0 NO_COVERAGE, test strength 100% (args.* is a gate target at mutationThreshold 100, so LoadMode needed LoadModeTest — added, mirroring LazyModeTest)
  • spotbugs:check cleansetLoadMode added to the design-intent OCP_OVERLY_CONCRETE_PARAMETER list in llama/spotbugs-exclude.xml, the trap CLAUDE.md documents for any new enum-valued ModelParameters setter
  • spotless:check, clang-format 22.1.8, and javadoc:jar all clean

Not verified locally, and the reason to watch CI here: the CMake extractor uses file(STRINGS) + while(... MATCHES ...) and has only been exercised on Linux with CMake 3.28. macOS, both Windows generators, aarch64, s390x (cross + qemu) and Android are unproven until this PR's C++ Tests matrix runs. That is the main risk in this change.

Also updated: CLAUDE.md (test-file table, total 527 → 531, the SpotBugs rename/addition note), TODO.md (the "five CLI flags" entry closed, corrected to seven), and the b10870–b10878 row in docs/history/llama-cpp-breaking-changes.md, which had explicitly deferred this public-API decision to a follow-up.

Related issues / PRs

Refs #425 (the b10878 bump that deleted --mlock / --no-mmap). Implements the fix prescribed by the "ModelParameters emits five CLI flags the server arg parser rejects" entry in TODO.md, added during the b10649 review (#403).

Checklist

  • I have read CONTRIBUTING.md and CODE_OF_CONDUCT.md
  • My commits follow Conventional Commits
  • No security-sensitive changes

Breaking change

ModelFlag.MLOCK, ModelFlag.NO_MMAP and ModelFlag.DUMP_KV_CACHE are removed (source-breaking for anyone calling setFlag(ModelFlag.MLOCK)), and ModelFlag.values().length drops 34 → 31. Every affected constant guaranteed a failed model load, so there is no working behaviour being taken away. All seven builder methods are retained and still compile.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AnNYn8W1xuVxVJtyL34GyH


Generated by Claude Code

…rd the contract

`LlamaModel.loadModel(parameters.toArray())` hands the ModelParameters map to
`common_params_parse(..., LLAMA_EXAMPLE_SERVER)` as argv, where an unregistered
option is a hard error rather than a warning. Seven flags the Java layer emitted
are no longer registered, so every caller of the matching builder method got
"Failed to parse model parameters" instead of a loaded model.

Two of them are fresh: b10878 deleted `--mlock` and `--no-mmap` (deprecated at
b10092 -- the whole deprecation window opened and closed inside eight tags).
Those have a faithful replacement, so nothing is lost: a new `args.LoadMode`
enum + `ModelParameters.setLoadMode(LoadMode)` expose upstream's `-lm`/
`--load-mode`, and `enableMlock()`/`disableMmap()` keep working, re-pointed to
`LoadMode.MLOCK` / `LoadMode.NONE` -- the exact mapping upstream's own
deprecation shim used. Both are now `@Deprecated`.

Five were already dead: `--dump-kv-cache`, `--hf-repo-v` and `--hf-file-v` are
gone from llama.cpp entirely; `--grp-attn-n` and `--grp-attn-w` are still in
`arg.cpp` but `set_examples()`-scoped to LLAMA_EXAMPLE_COMPLETION/PASSKEY, so
the server parser rejects them exactly like a deleted flag. Their setters are
now `@Deprecated` no-ops that write nothing, keeping call sites compiling *and*
loading. `ModelFlag.MLOCK`/`NO_MMAP`/`DUMP_KV_CACHE` are removed from the enum
so the broken argv is not reachable through `setFlag` either -- the same
reasoning that already excluded `FLASH_ATTN`.

No Java test could catch any of this: `ModelFlagTest` and
`ModelParametersExtendedTest` assert the string mapping (`hasKey("--mlock")`),
never that llama.cpp still accepts the string, so they stayed green while the
flags were dead. The guard closes that:

  - `cmake/extract-java-cli-flags.cmake` extracts every `"--flag"` literal
    `ModelFlag.java`/`ModelParameters.java` can emit into a generated header at
    configure time, so the Java sources stay the single source of truth.
  - `src/test/cpp/test_model_flags.cpp` asserts each one is registered in the
    real `common_params_parser_init(params, LLAMA_EXAMPLE_SERVER).options`,
    exempting only `--vocab-only` (a project pseudo-flag `strip_flag_from_argv`
    removes before the parse). It also checks the oracle and the exemption list
    themselves, so an empty option table or a stale exemption cannot make it
    pass vacuously.

A grep over `arg.cpp` would not have worked -- it is structurally blind to
example scoping, which is exactly where `--grp-attn-n`/`-w` hid. Run against the
pre-fix sources the test named all seven; after the fix it is green.

Verified locally: ctest 531/531 (was 527), `mvn test` 1759 tests green,
PIT 320/320 killed with 0 NO_COVERAGE, spotbugs clean (`setLoadMode` added to
the design-intent OCP suppression list), spotless, clang-format and
`javadoc:jar` all clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AnNYn8W1xuVxVJtyL34GyH
@bernardladenthin
bernardladenthin merged commit 1b42e9b into main Sep 9, 2026
9 of 15 checks passed
@bernardladenthin
bernardladenthin deleted the claude/cli-flag-contract-guard branch September 9, 2026 19:47
@sonarqubecloud

sonarqubecloud Bot commented Sep 9, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants