diff --git a/CLAUDE.md b/CLAUDE.md index b5b074d5..158d58b7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1460,8 +1460,9 @@ ctest --test-dir build --output-on-failure -R "ResultsToJson" | `src/test/cpp/test_tts_wav.cpp` | 2 | The in-memory WAV writer `pcm_to_wav16_bytes` in `tts_wav.hpp` (WAV header/payload + little-endian clamping) — our own code, not upstream. The Qwen3-TTS pipeline it pairs with (`mtmd_helper::gen_audio`) is entirely upstream-owned (no project-side DSP to unit-test here). The load path is additionally covered by `test_tts_params.cpp` (3 tests over `tts_params.hpp`'s `build_tts_params`, plus 2 pinning the upstream `-1` default it depends on), which pins the CPU-thread resolution whose absence used to crash the JVM on every platform — see the `TODO.md` entry for the mechanism. End-to-end coverage is `TtsIntegrationTest`, which is model-gated. | | `src/test/cpp/test_tts_params.cpp` | 13 | The **three** builders every hand-assembled `common_params` goes through: `build_tts_params` (`tts_params.hpp`), `build_train_params` (`train_params.hpp`) and the shared `jllama::resolve_cpu_params` (`cpu_params.hpp`). Each builder is guarded separately on purpose — testing the resolver alone does **not** cover its call sites, because `train_engine.cpp` is compiled into `jllama` only, never into `jllama_test`, and `LlamaTrainerIntegrationTest` is gated on `net.ladenthin.llama.train.model`, which no CI job sets. Without these the JVM-abort bug could regress in the trainer on every platform, unseen. | | `src/test/cpp/test_model_split.cpp` | 7 | The two `load_tensors()` split helpers that `patches/0012` extracts out of llama.cpp's `src/llama-model.cpp` — `llama_model_splits_normalize` (proportional split, single device, and the zero-sum case that used to produce NaN, **and the cancelling `--tensor-split` case** — `-ts 1,-1` reaches the identical line on any backend with no GPU memory pressure at all) and `llama_model_splits_select_device` (every layer maps to a real device index; malformed split points throw a message that names the function, the layer, the index and the split values instead of libc++'s bare `"vector"`). **This is the runnable guard for `0012`**: the patch also ships an upstream `tests/test-model-split.cpp`, but a FetchContent subproject builds with `LLAMA_BUILD_TESTS=OFF`, so that one is applied-but-never-compiled here. This file is the only place the two functions are linked in CI, on every platform — so a bump that drops the patch fails the `C++ Tests` build outright rather than resurfacing as one red macOS Java job. It is the one test file that includes an **internal** upstream header (`llama-model.h`, via the `${llama.cpp_SOURCE_DIR}/src` include dir added for it), which is deliberate: a signature drift should fail loudly at compile time. | +| `src/test/cpp/test_model_flags.cpp` | 4 | **The contract between the Java flag surface and llama.cpp's server argument parser.** CMake extracts every `"--flag"` literal `ModelFlag.java` + `ModelParameters.java` can emit (`cmake/extract-java-cli-flags.cmake` → a generated header), and this file asserts each one is in `common_params_parser_init(params, LLAMA_EXAMPLE_SERVER).options`. It exists because **no Java test can catch this class**: `ModelFlagTest`/`ModelParametersExtendedTest` pin the *string mapping* (`hasKey("--mlock")`), never that llama.cpp still accepts the string, so they stay green forever while the flag is dead — and `common_params_parse` treats an unregistered option as a hard error, so the affected builder method makes the model **unloadable**, not merely ineffective. **A grep over `arg.cpp` is not a substitute**: `--grp-attn-n`/`-w` are present there at every pinned tag but `set_examples()`-scoped to `LLAMA_EXAMPLE_COMPLETION`/`PASSKEY`, so the server parser rejects them exactly like a deleted flag — only the real option table sees that. `--vocab-only` is the one exemption (a project pseudo-flag `strip_flag_from_argv` removes before the parse); the exemption list is itself asserted to stay live. | -**Current total: 527 tests (all passing).** +**Current total: 531 tests (all passing).** #### Upstream source location (in CMake build tree) @@ -1722,7 +1723,7 @@ This has actually shipped twice. Most recently the `--flash-attn` / `--lazy-mode `llama/spotbugs-exclude.xml` lists methods **by name**, so renaming `setTensorReadLazy` to `setLazyMode` left a dead entry while the new `setLazyMode` and `setFlashAttn` were uncovered. **Any rename or addition of an enum-valued `ModelParameters` setter needs that list updated in the same -commit** — the same "FQN not updated after a rename" class as the stale PIT `targetClasses` and +commit** — `setLoadMode` was added to it for exactly this reason — the same "FQN not updated after a rename" class as the stale PIT `targetClasses` and `CMakeLists.txt` OSInfo repairs. ## Spotless Formatting diff --git a/TODO.md b/TODO.md index bb48de2e..382ce06f 100644 --- a/TODO.md +++ b/TODO.md @@ -354,21 +354,22 @@ into that PR. Each item below was verified against pristine upstream tags and is real, but none is a regression introduced by the version bump — they were deferred to keep that PR landable. -- **`ModelParameters` emits five CLI flags the server arg parser rejects, so any caller of them - cannot load a model.** `--dump-kv-cache`, `--hf-repo-v` and `--hf-file-v` no longer exist anywhere - in llama.cpp (absent at both b10456 and b10649); `--grp-attn-n` and `--grp-attn-w` still exist but - are `set_examples({LLAMA_EXAMPLE_COMPLETION, ...})`, so `add_opt` never registers them for - `LLAMA_EXAMPLE_SERVER` — the example jllama parses with. An unregistered flag is not ignored: - `arg.cpp` throws, `common_params_parse` returns false, and `load_model_impl` throws - `LlamaException("Failed to parse model parameters")`. Four existing tests pin the dead literals and - would pass forever. Fix: deprecate the five members the way this PR handled - `withTfsZ`/`withPenalizeNl` (keep source compatibility, never write the map), and add a hermetic - `jllama_test` contract test that walks `common_params_parser_init(params, LLAMA_EXAMPLE_SERVER)`'s - `ctx.options` (upstream's own `test-arg-parser` pattern; the symbols already link into - `jllama_test`) and asserts every flag `ModelParameters`/`ModelFlag` can emit is in that set, - excluding only `--vocab-only`, which `strip_flag_from_argv` removes on purpose. A grep-based sweep - is **not** sufficient — it is structurally blind to example scoping, which is exactly how - `--grp-attn-w` hides. +- ~~**`ModelParameters` emits five CLI flags the server arg parser rejects, so any caller of them + cannot load a model.**~~ **DONE** — and it turned out to be **seven**, not five. The fix is the one + this entry prescribed: `cmake/extract-java-cli-flags.cmake` extracts every `"--flag"` literal + `ModelFlag.java`/`ModelParameters.java` can emit into a generated header, and + `src/test/cpp/test_model_flags.cpp` asserts each is registered in + `common_params_parser_init(params, LLAMA_EXAMPLE_SERVER).options`, exempting only `--vocab-only` + (which `strip_flag_from_argv` removes on purpose). Run against the pre-fix Java sources it reported + exactly the predicted set, which is how the count grew: the five named here plus `--mlock` and + `--no-mmap`, deleted upstream at b10878 while this entry was open. Those two have a faithful + replacement, so `enableMlock()`/`disableMmap()` were **repointed** to upstream's own deprecation-shim + mapping (`--load-mode mlock` / `--load-mode none`) behind a new `setLoadMode(LoadMode)` rather than + retired — no API loss. The other five became no-ops (`@Deprecated`, never write the map), and + `ModelFlag.MLOCK`/`NO_MMAP`/`DUMP_KV_CACHE` were removed from the enum so a broken argv is not + reachable through `setFlag` either — the same reasoning that already excluded `FLASH_ATTN`. The + replaced Java assertions now compare against a pristine `ModelParameters`, not against the old + "still has this key" shape that would have passed forever. - **`acquire_jllama_context_impl` / `release_jllama_context_impl` / `jllama_context_guard` have no model-free unit guard.** These three (`jni_helpers.hpp`) are the whole `close()`-vs-inference diff --git a/docs/history/llama-cpp-breaking-changes.md b/docs/history/llama-cpp-breaking-changes.md index 02e7f9a1..851f6918 100644 --- a/docs/history/llama-cpp-breaking-changes.md +++ b/docs/history/llama-cpp-breaking-changes.md @@ -700,5 +700,5 @@ Used during `llama.cpp` version bumps: when upgrading, scan this file from the r | **macOS-15 failure — root cause + fix, 2026-09-08** | `src/llama-model.cpp` (upstream) ← `ggml/src/ggml-metal/ggml-metal-device.m` `8c0b9cd04` ([#27701](https://github.com/ggml-org/llama.cpp/pull/27701)); carried as `llama/patches/0012` | **Found by reading, not by bisecting — the bisect was prepared and then not needed.** The chain, each link checked against the source rather than inferred: **(1)** `8c0b9cd04` "metal : fix memory query under low-memory conditions" lies inside the b10618→b10797 window and rewrote `ggml_metal_device_get_memory` to `*free = *total > cur ? *total - cur : 0`. **(2)** Both the green and the red run log `current allocated size is greater than the recommended max working set size`, i.e. `cur > total` — so that condition is the **precondition, not the discriminator**; it held on both sides of the regression. **(3)** Before the clamp, `*total - cur` **underflowed** to a huge `size_t`, which normalised harmlessly; after it, the device reports exactly `free == 0`. **(4)** `load_tensors`' `if (free == 0 && total == 0)` host-memory fallback does not fire, because `total` is `recommendedMaxWorkingSetSize` and is non-zero. **(5)** `splits[0] = 0` → `split_sum = 0` → `splits[i] /= split_sum` = **NaN**. **(6)** `std::upper_bound(…, NaN)` — every comparison false — returns the end iterator, so `layer_gpu == 1`. **(7)** `devices.at(1)` on a one-element vector throws `std::out_of_range`, whose libc++ `what()` is the bare string `"vector"`; `llama.cpp`'s `catch (const std::exception & err)` prints it verbatim. **(8)** The **discriminator** is `const int act_gpu_layers = devices.empty() ? 0 : …`: without a GPU backend `devices` is empty, every layer returns early on `cpu_dev`, and the `.at()` line is unreachable — which is exactly why only the Metal job failed. This accounts for every observation the two investigation rows above collected: Metal-only; only on *repeat* loads in a long-lived JVM (that is merely how `currentAllocatedSize` grew past the recommended set size, not a precondition of its own); `what() == "vector"`; ~30 ms in, right after the vocab warnings; and the absence of a `hyperparameters:`/`vocabulary:` prefix, since upstream's own rethrows would have added one. **The fix** is `patches/0012`: `llama_model_splits_normalize()` falls back to an even split when the weights sum to zero, and `llama_model_splits_select_device()` bounds-checks the lookup and throws a message naming the function, the layer, the device index and the split points. Both are lifted out of `load_tensors` into free functions **for testability** — the failing state needs a real over-committed GPU and cannot be arranged through any API — with an upstream `tests/test-model-split.cpp` and, because a FetchContent subproject sets `LLAMA_BUILD_TESTS=OFF`, a project-side runnable guard `src/test/cpp/test_model_split.cpp` that links the same two functions into `jllama_test` on every platform. **A second trigger, found while writing the fix up and verified against the unfixed library:** `--tensor-split` is parsed with `std::stof` and never range-checked, so `-ts 1,-1` makes the weights cancel, `split_sum` is 0 again, the split points become `[inf, -nan]`, and every layer maps one past the last device — on CUDA, Vulkan or ROCm as much as on Metal, in a fresh process with no memory pressure. The macOS failure is therefore one *instance* of a general defect, not a Metal edge case, which is what settles the question of upstream-submittability. **Two lessons worth carrying.** The `ggml/src/**` "safe to skip" rule in the review list is sound for compile/link breaks and blind to runtime ones; this is the first entry where it cost something, and the cost was ~180 builds of bisect window. And a `catch (…) { log(err.what()); }` over a library that throws `std::out_of_range` is a **diagnostic dead end** on libc++, which reports it as `"vector"` and nothing else — the second half of `0012` exists for that reason alone, and is why the message a future occurrence produces will name its own cause. | | b10850–b10870 | `common/chat.cpp` (**−2524 lines**: every model-specific chat parser split out into a new `common/parsers/` directory — 19 new files, wired in via `common/parsers/sources.cmake` + `common/CMakeLists.txt`), `common/arg.cpp` (**behaviour change, not a signature change** — see below), `common/speculative.cpp` (**behaviour change** — see below), `ggml/include/ggml.h` (**additive + one deprecation**: `ggml_prec` gains `GGML_PREC_UNDEFINED`/`BF16`/`F16`/`Q8`/`Q4`, `GGML_PREC_DEFAULT` kept as a same-value alias marked deprecated; two new `GGML_API` functions `ggml_prec_set_acc` / `ggml_prec_set_src`, and **`ggml_mul_mat_set_prec` + `ggml_flash_attn_ext_set_prec` are now `GGML_DEPRECATED`**), `tools/server/server-context.cpp` (checkpoint eviction), `tools/mtmd/clip.cpp` + two model files, `src/llama-model.cpp` (**#28160**, lazy-mode AUTO), `tests/CMakeLists.txt`. **`common/chat.h` is byte-identical in the range** | **No project-source change.** The headline number is misleading: `common/chat.cpp` losing 2524 lines is a pure **internal reorganisation** — `common/chat.h`, which `jllama.cpp` includes directly and which is #2 on the priority review list, does not change at all, so nothing the project compiles against moved. The `ggml.h` change is additive plus deprecations that break nothing: `GGML_PREC_DEFAULT` keeps its value, and the two newly-deprecated functions still exist. The project source was grepped for all of it — `GGML_PREC`, `ggml_prec_set_*`, `ggml_mul_mat_set_prec`, `ggml_flash_attn_ext_set_prec` across `src/main/cpp/**` and `src/test/cpp/**` — with **zero** references, so none of it can reach us. (A deprecation is worth naming anyway: it is the shape that becomes a removal two bumps later, and a removal is the one change that breaks a build with no diff hunk to notice.) Server contract re-checked mechanically and **byte-identical in all three dimensions** (request-field set, `set_hard_limits` bounds, response keys in both emit forms). **Two behaviour changes that a header diff cannot see, and both reach every entry point that parses argv** (`NativeServer` in both modes, `LlamaModel`'s own parameter parse): **(1)** `--mmproj-device` now **defaults to `--device`** instead of auto-selecting (`common_params_parse` assigns `params.mmproj_device = params.devices.front()` when `mmproj_use_gpu` is set and `-mmdev` was not) — a caller that sets `--device` but not `-mmdev` now pins the multimodal projector to the same device rather than letting it choose, which is the surface `MultimodalIntegrationTest` and every vision user exercises. **(2)** the **draft model inherits the global device list** the same way, and `common_speculative_init` now only overwrites `result.devices` **when the spec device list is non-empty** (it previously assigned unconditionally), plus forces `LLAMA_SPLIT_MODE_LAYER` when the draft is pinned to exactly one device. That is the speculative-decoding path `LlamaModelTest#testSpeculativeDecoding` drives — the same test that was red on macOS before `patches/0012`, so a failure there after this bump needs to be attributed carefully between the two. **Chunking, with the figures recorded rather than a verdict asserted:** the full diff is **393 KB over 20 commits**, over the runbook's 100 KiB threshold; the **review surface proper** (`common/`, `include/`, `tools/server/`, `tools/mtmd/`, `ggml/include/`, top-level `CMakeLists.txt`) is **28 files, +2686 / −2480**, but ~2500 of those lines on each side are the one mechanical parser move, so the material change is a few dozen lines. Bumped straight rather than chunked on that basis, with the raw numbers here so the call is auditable. | | b10850–b10870 | patches + upstream verification | **The intersection was NOT empty, and `0012` was the patch at risk.** The range touches `src/llama-model.cpp` and `tests/CMakeLists.txt` — both files `patches/0012` modifies, one week after that patch landed — plus `common/arg.cpp` (`0001`) and `tools/server/server-context.cpp` (`0002`/`0003`/`0010`). So this bump could not be waved through on a disjoint file list. **The `0012`-specific check `CLAUDE.md` mandates was run by hand first**, because the fail-loud applier detects "does not apply" but never "upstream already fixed this": `git show b10870:src/llama-model.cpp | grep -A3 split_sum` still shows the bare `splits[i] /= split_sum` with **no zero-sum guard**, so upstream has not adopted the fix and the patch stays rather than being dropped. Upstream's own change to that file (#28160, resolving `LLAMA_LAZY_MODE_AUTO` to `OFF` on devices without mmap support) sits ~60 lines above the patched region and is unrelated. Then the applier was run for real: fresh `rm -rf llama/build && cmake -B build -DBUILD_TESTING=ON`, configure clean, stamp written at head `1945e092030f8668ff93382799502d01490e564d` (= `b10870`), **all nine hashes recorded**. | -| b10870–b10878 | `common/arg.cpp` (**REMOVAL, and it reaches this project's public Java API**: the deprecated `--mlock`, `--mmap`, `--no-mmap`, `-dio`/`--direct-io`, `-ndio`/`--no-direct-io` options are deleted in favour of `-lm`/`--load-mode `; nothing was added — `--load-mode` already existed at b10870, so the whole deprecation window opened and closed inside a single 8-tag range), `include/llama.h` (`llama_sampler_chain_n` returns `int32_t` instead of `int` — **unreachable here**, no project TU calls it, and the two are the same type on every platform this builds for), `src/llama-model.{cpp,h}` (**additive only**: a new `LLM_TYPE_1B_A400M` enumerator for Granite3 MoE, far from `patches/0012`'s hunks), `tools/mtmd/mtmd-helper.cpp` (internal video frame-id propagation; **`mtmd-helper.h` is untouched**, so `mtmd_helper::gen_audio` and therefore `TextToSpeech` are unaffected), `src/llama-sampler.cpp`, `common/jinja/runtime.cpp`, `src/models/granite-moe.cpp`, and a ggml build-system change (`GGML_CUDA_FA_ALL_QUANTS` deprecated in favour of `GGML_CUDA_FA_QUANTS` — **not set anywhere in this repo**, so it cannot reach the CUDA/HIP jobs). 25 files, 336 insertions, 227 deletions, 50.3 KiB — under the runbook's 100 KiB chunking threshold, so bumped straight through. **The one row that needs project action is the first.** `ModelFlag.MLOCK` (`"--mlock"`) and `ModelFlag.NO_MMAP` (`"--no-mmap"`) are public constants emitted by `ModelParameters.enableMlock()` / `disableMmap()`, and `LlamaModel.loadModel(parameters.toArray())` hands that argv straight to `common_params_parse`, where an unknown option is a hard error rather than a warning — so both builder methods now produce a model load that fails. The other three removed options are not exposed here. **No test can catch this**: `ModelFlagTest` and `ModelParametersExtendedTest#testEnableMlock`/`#testDisableMmap` assert only the string mapping (`hasKey("--mlock")`), never that llama.cpp still accepts it, so they stay green while the flag is dead — the same "pins the mapping, not the contract" shape as the `getMetrics()` payload drift at b10408. Faithful replacement is `--mlock` → `--load-mode mlock` and `--no-mmap` → `--load-mode none`; deciding between re-pointing the two builders, adding a `LoadMode` value-taking setter, or removing the constants outright is a public-API call and is deliberately **not** made in the bump commit. | +| b10870–b10878 | `common/arg.cpp` (**REMOVAL, and it reaches this project's public Java API**: the deprecated `--mlock`, `--mmap`, `--no-mmap`, `-dio`/`--direct-io`, `-ndio`/`--no-direct-io` options are deleted in favour of `-lm`/`--load-mode `; nothing was added — `--load-mode` already existed at b10870, so the whole deprecation window opened and closed inside a single 8-tag range), `include/llama.h` (`llama_sampler_chain_n` returns `int32_t` instead of `int` — **unreachable here**, no project TU calls it, and the two are the same type on every platform this builds for), `src/llama-model.{cpp,h}` (**additive only**: a new `LLM_TYPE_1B_A400M` enumerator for Granite3 MoE, far from `patches/0012`'s hunks), `tools/mtmd/mtmd-helper.cpp` (internal video frame-id propagation; **`mtmd-helper.h` is untouched**, so `mtmd_helper::gen_audio` and therefore `TextToSpeech` are unaffected), `src/llama-sampler.cpp`, `common/jinja/runtime.cpp`, `src/models/granite-moe.cpp`, and a ggml build-system change (`GGML_CUDA_FA_ALL_QUANTS` deprecated in favour of `GGML_CUDA_FA_QUANTS` — **not set anywhere in this repo**, so it cannot reach the CUDA/HIP jobs). 25 files, 336 insertions, 227 deletions, 50.3 KiB — under the runbook's 100 KiB chunking threshold, so bumped straight through. **The one row that needs project action is the first.** `ModelFlag.MLOCK` (`"--mlock"`) and `ModelFlag.NO_MMAP` (`"--no-mmap"`) are public constants emitted by `ModelParameters.enableMlock()` / `disableMmap()`, and `LlamaModel.loadModel(parameters.toArray())` hands that argv straight to `common_params_parse`, where an unknown option is a hard error rather than a warning — so both builder methods now produce a model load that fails. The other three removed options are not exposed here. **No test can catch this**: `ModelFlagTest` and `ModelParametersExtendedTest#testEnableMlock`/`#testDisableMmap` assert only the string mapping (`hasKey("--mlock")`), never that llama.cpp still accepts it, so they stay green while the flag is dead — the same "pins the mapping, not the contract" shape as the `getMetrics()` payload drift at b10408. Faithful replacement is `--mlock` → `--load-mode mlock` and `--no-mmap` → `--load-mode none`. **Resolved in the follow-up PR, and it did all three:** a new `args.LoadMode` enum + `ModelParameters.setLoadMode(LoadMode)` expose the replacement option properly; `enableMlock()` / `disableMmap()` are kept and `@Deprecated`, re-pointed to `LoadMode.MLOCK` / `LoadMode.NONE` — upstream's own deprecation-shim mapping, so behaviour is unchanged and no API is lost; and `ModelFlag.MLOCK` / `NO_MMAP` are removed from the enum, because leaving them would keep the broken argv reachable through `setFlag` (the same reasoning that already excluded `FLASH_ATTN`). The “no test can catch this” half was closed at the same time and generalised: `src/test/cpp/test_model_flags.cpp` drives every flag the Java layer can emit — the list generated at configure time from the Java sources by `cmake/extract-java-cli-flags.cmake` — through the real `common_params_parser_init(params, LLAMA_EXAMPLE_SERVER)` option table. Run against the pre-fix sources it named **seven** dead flags, not two: the long-dead `--dump-kv-cache` / `--hf-repo-v` / `--hf-file-v`, and `--grp-attn-n` / `--grp-attn-w`, which are *present* in `arg.cpp` at every pinned tag but `set_examples()`-scoped away from `LLAMA_EXAMPLE_SERVER` — a case a textual sweep of upstream sources is structurally blind to. | | b10870–b10878 | patches + upstream verification | **All nine patches still apply, and `0012` is still required.** The range touches two patch targets — `common/arg.cpp` (`0001`) and `src/llama-model.{cpp,h}` (`0012`) — so both were checked against the pristine tag rather than assumed. `0001`: `b10878:common/arg.cpp` still carries the `#ifdef _WIN32` count-guarded `argv = utf8.ptrs.data()` override, and `common_params_parse_main` appears **0 times** in `b10878:common/arg.h`, so upstream has still not adopted the fix. `0012`: `b10878:src/llama-model.cpp` still normalises with a bare `splits[i] /= split_sum;` and has **no `split_sum == 0` guard** of its own — the CLAUDE.md instruction to *drop rather than refresh* this patch does not fire, and its `llama-model` diff is only the new enumerator. Verified for real: fresh `cmake -S llama -B /tmp/b10878-build -DBUILD_TESTING=ON` through the real `FetchContent` path, configure clean, stamp written at head `4850c7727fa73bbe3098e10ee369fbc3467c445f` (= `b10878`) with **all nine hashes recorded**; full `cmake --build --config Release` clean; `ctest` **527/527**, including the four `LlamaModelSplits.*` cases that are the only place `0012`'s two extracted functions are linked in CI. `nm -D` on the fresh `libjllama.so` reports **40** `Java_*` exports. `mvn -pl llama clean test -Dtest=NativeLibraryLoadSmokeTest` **4/4, 0 skipped**, including `nativeBuildInfoMatchesPinnedVersionConstant` — the end-to-end proof that the four pin sites and the linked binary agree. Run with `clean`: `LLAMA_CPP_VERSION` is a compile-time constant javac inlines into the test class, and Maven's incremental compilation cannot see that dependency. | diff --git a/llama/CMakeLists.txt b/llama/CMakeLists.txt index 90040f83..2a6d368b 100644 --- a/llama/CMakeLists.txt +++ b/llama/CMakeLists.txt @@ -560,6 +560,17 @@ if(BUILD_TESTING) enable_testing() include(GoogleTest) + # Make the Java layer's emitted CLI-flag set available to the C++ contract test. The Java + # sources are the single source of truth; see cmake/extract-java-cli-flags.cmake for why a + # textual sweep of common/arg.cpp is not a substitute (example scoping is invisible to it). + include(${CMAKE_CURRENT_SOURCE_DIR}/cmake/extract-java-cli-flags.cmake) + jllama_extract_java_cli_flags( + JAVA_FLAG_SOURCES + ${CMAKE_CURRENT_SOURCE_DIR}/src/main/java/net/ladenthin/llama/args/ModelFlag.java + ${CMAKE_CURRENT_SOURCE_DIR}/src/main/java/net/ladenthin/llama/parameters/ModelParameters.java + OUTPUT_HEADER ${CMAKE_CURRENT_BINARY_DIR}/generated/jllama_java_cli_flags.h + ) + add_executable(jllama_test src/test/cpp/test_utils.cpp src/test/cpp/test_server.cpp @@ -569,6 +580,7 @@ if(BUILD_TESTING) src/test/cpp/test_tts_wav.cpp src/test/cpp/test_tts_params.cpp src/test/cpp/test_model_split.cpp + src/test/cpp/test_model_flags.cpp ${llama.cpp_SOURCE_DIR}/tools/server/server-common.cpp ${llama.cpp_SOURCE_DIR}/tools/server/server-chat.cpp ${llama.cpp_SOURCE_DIR}/tools/server/server-context.cpp @@ -591,6 +603,8 @@ if(BUILD_TESTING) # same (tests/test-batch-alloc.cpp, tests/test-quantize-stats.cpp include "../src/..."). # No header name in that directory collides with tools/server, tools/mtmd or src/main/cpp. ${llama.cpp_SOURCE_DIR}/src + # The generated Java-CLI-flag list consumed by test_model_flags.cpp. + ${CMAKE_CURRENT_BINARY_DIR}/generated ) target_link_libraries(jllama_test PRIVATE llama-common mtmd llama nlohmann_json GTest::gtest_main) target_compile_features(jllama_test PRIVATE cxx_std_17) diff --git a/llama/cmake/extract-java-cli-flags.cmake b/llama/cmake/extract-java-cli-flags.cmake new file mode 100644 index 00000000..b58c2929 --- /dev/null +++ b/llama/cmake/extract-java-cli-flags.cmake @@ -0,0 +1,111 @@ +# SPDX-FileCopyrightText: 2026 Bernard Ladenthin +# +# SPDX-License-Identifier: MIT + +# Generates a C++ header listing every CLI flag the Java layer can emit into the argv that +# LlamaModel.loadModel() hands to llama.cpp's common_params_parse(). +# +# WHY THIS EXISTS +# --------------- +# ModelFlag / ModelParameters are the only two places that write a "--flag" key into the +# parameter map, and that map becomes argv verbatim. common_params_parse() treats an +# *unregistered* option as a hard error, not a warning -- so the moment upstream removes a +# flag (or narrows its set_examples() scope away from LLAMA_EXAMPLE_SERVER), the matching +# builder method silently turns into "this model will never load". +# +# Every Java-side test of these methods asserts the *string mapping* (hasKey("--mlock")), never +# that llama.cpp still accepts the string, so they stay green forever while the flag is dead. +# That is exactly how --mlock/--no-mmap (removed at b10878), --dump-kv-cache, --hf-repo-v and +# --hf-file-v reached main, and how --grp-attn-n/--grp-attn-w hid for even longer: those two +# still exist in arg.cpp, but are scoped to LLAMA_EXAMPLE_COMPLETION, so a grep-based sweep +# reports them alive while the server parser rejects them. A textual check is structurally +# blind to example scoping; only the real parser knows. +# +# So: this script extracts the list from the Java sources (the single source of truth), and +# src/test/cpp/test_model_flags.cpp feeds every entry to the *actual* +# common_params_parser_init(params, LLAMA_EXAMPLE_SERVER) option table. jllama_test runs on +# every platform in the "C++ Tests" job, so a llama.cpp bump that kills a flag reds CI in the +# same run that introduces it. +# +# EXTRACTION +# ---------- +# Line-oriented on purpose: any line whose trimmed form starts with "*", "//" or "/*" is a +# comment and is dropped before matching, which removes every Javadoc mention of a flag +# ({@code --flash-attn}, prose referring to --mlock, ...) without needing a real Java parser. +# What survives is `"--something"` in code position -- enum constants, putScalar/putEnum keys, +# parameters.put keys, and the private static final ARG_* constants alike. +# +# Inputs : JAVA_FLAG_SOURCES - list of .java files to scan +# OUTPUT_HEADER - path of the header to write +# Output : a header defining JLLAMA_JAVA_CLI_FLAGS[] / JLLAMA_JAVA_CLI_FLAG_COUNT + +function(jllama_extract_java_cli_flags) + cmake_parse_arguments(ARG "" "OUTPUT_HEADER" "JAVA_FLAG_SOURCES" ${ARGN}) + + if(NOT ARG_OUTPUT_HEADER) + message(FATAL_ERROR "jllama_extract_java_cli_flags: OUTPUT_HEADER is required") + endif() + if(NOT ARG_JAVA_FLAG_SOURCES) + message(FATAL_ERROR "jllama_extract_java_cli_flags: JAVA_FLAG_SOURCES is required") + endif() + + set(_flags "") + foreach(_src IN LISTS ARG_JAVA_FLAG_SOURCES) + if(NOT EXISTS "${_src}") + message(FATAL_ERROR "jllama_extract_java_cli_flags: missing source ${_src}") + endif() + file(STRINGS "${_src}" _lines) + foreach(_line IN LISTS _lines) + string(STRIP "${_line}" _trimmed) + # Drop comment lines (Javadoc continuations, // and block-comment openers). + if(_trimmed MATCHES "^(\\*|//|/\\*)") + continue() + endif() + # CMake regex has no global match, so consume the line one literal at a time. + while(_line MATCHES "\"(--[A-Za-z0-9][A-Za-z0-9._+-]*)\"") + list(APPEND _flags "${CMAKE_MATCH_1}") + string(REPLACE "\"${CMAKE_MATCH_1}\"" "" _line "${_line}") + endwhile() + endforeach() + endforeach() + + list(REMOVE_DUPLICATES _flags) + list(SORT _flags) + list(LENGTH _flags _count) + + # A silently empty list would make the contract test vacuously pass -- the same + # "nothing to scan reported as a clean pass" trap verify-bytecode-version.sh exits 2 for. + if(_count LESS 50) + message(FATAL_ERROR + "jllama_extract_java_cli_flags: only ${_count} flags extracted from " + "${ARG_JAVA_FLAG_SOURCES} -- the extractor is broken or the sources moved") + endif() + + set(_body "") + foreach(_flag IN LISTS _flags) + string(APPEND _body " \"${_flag}\",\n") + endforeach() + + set(_header "// Generated by cmake/extract-java-cli-flags.cmake -- DO NOT EDIT.\n") + string(APPEND _header "// Source of truth: the Java files listed in llama/CMakeLists.txt.\n") + string(APPEND _header "#pragma once\n\n") + string(APPEND _header "static const char * const JLLAMA_JAVA_CLI_FLAGS[] = {\n${_body}};\n\n") + string(APPEND _header "static const int JLLAMA_JAVA_CLI_FLAG_COUNT = ${_count};\n") + + # Only rewrite when the content actually changed, so an unrelated re-configure does not + # touch the header and force a needless rebuild of the test. + set(_existing "") + if(EXISTS "${ARG_OUTPUT_HEADER}") + file(READ "${ARG_OUTPUT_HEADER}" _existing) + endif() + if(NOT _existing STREQUAL _header) + file(WRITE "${ARG_OUTPUT_HEADER}" "${_header}") + endif() + + # Re-run configure (and therefore this extractor) whenever a scanned Java file changes. + foreach(_src IN LISTS ARG_JAVA_FLAG_SOURCES) + set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS "${_src}") + endforeach() + + message(STATUS "jllama: extracted ${_count} Java CLI flags -> ${ARG_OUTPUT_HEADER}") +endfunction() diff --git a/llama/spotbugs-exclude.xml b/llama/spotbugs-exclude.xml index 0644b2f4..e7f47839 100644 --- a/llama/spotbugs-exclude.xml +++ b/llama/spotbugs-exclude.xml @@ -82,6 +82,7 @@ SPDX-License-Identifier: MIT + diff --git a/llama/src/main/java/net/ladenthin/llama/args/LoadMode.java b/llama/src/main/java/net/ladenthin/llama/args/LoadMode.java new file mode 100644 index 00000000..73011843 --- /dev/null +++ b/llama/src/main/java/net/ladenthin/llama/args/LoadMode.java @@ -0,0 +1,92 @@ +// SPDX-FileCopyrightText: 2026 Bernard Ladenthin +// +// SPDX-License-Identifier: MIT + +package net.ladenthin.llama.args; + +/** + * How llama.cpp brings the model weights into memory. + * + *

The string constants are the exact values accepted by llama.cpp's {@code -lm}/{@code --load-mode} + * CLI argument, and map 1-to-1 to the {@code llama_load_mode} enum in {@code include/llama.h}. + * + *

This single option replaced the older independent switches. Upstream deprecated + * {@code --mlock}, {@code --mmap}/{@code --no-mmap} and {@code -dio}/{@code --direct-io} at b10092 + * and deleted them at b10878 — the whole deprecation window opened and closed + * inside eight tags. Because llama.cpp's argument parser treats an unknown option as a hard error + * rather than a warning, the deleted spellings do not degrade a model load, they prevent it; the + * {@code ModelParameters} methods that used to emit them now emit the matching mode here instead + * (see {@link net.ladenthin.llama.parameters.ModelParameters#enableMlock()} and + * {@link net.ladenthin.llama.parameters.ModelParameters#disableMmap()}). + * + * @see net.ladenthin.llama.parameters.ModelParameters#setLoadMode(LoadMode) + */ +public enum LoadMode implements CliArg { + + /** + * Let llama.cpp choose — mmap unless a device does not support it. + * + *

CLI string: {@code "auto"} — maps to {@code LLAMA_LOAD_MODE_AUTO = -1}. This is + * upstream's default, so passing it is equivalent to omitting the flag. + */ + AUTO("auto"), + + /** + * No special loading mode: read the weights normally, without mmap and without mlock. + * + *

CLI string: {@code "none"} — maps to {@code LLAMA_LOAD_MODE_NONE = 0}. This is what the + * removed {@code --no-mmap} mapped to in upstream's own deprecation shim. + */ + NONE("none"), + + /** + * Memory-map the model. + * + *

CLI string: {@code "mmap"} — maps to {@code LLAMA_LOAD_MODE_MMAP = 1}. Loading is fast and + * pages are shared, but pages can be evicted again under memory pressure; combine with + * {@link #MMAP_MLOCK} to prevent that. + */ + MMAP("mmap"), + + /** + * Force the system to keep the model in RAM rather than swapping or compressing it. + * + *

CLI string: {@code "mlock"} — maps to {@code LLAMA_LOAD_MODE_MLOCK = 2}. This is what the + * removed {@code --mlock} mapped to in upstream's own deprecation shim. + */ + MLOCK("mlock"), + + /** + * Memory-map the model and lock it into RAM. + * + *

CLI string: {@code "mmap+mlock"} — maps to {@code LLAMA_LOAD_MODE_MMAP_MLOCK = 3}. + */ + MMAP_MLOCK("mmap+mlock"), + + /** + * Use direct I/O if the platform supports it, bypassing the page cache. + * + *

CLI string: {@code "dio"} — maps to {@code LLAMA_LOAD_MODE_DIRECT_IO = 4}. + */ + DIRECT_IO("dio"); + + /** + * The CLI string passed to {@code --load-mode} in llama.cpp's {@code common/arg.cpp}. + */ + private final String argValue; + + LoadMode(String value) { + this.argValue = value; + } + + /** + * Returns the CLI string accepted by llama.cpp's {@code --load-mode} argument. + * + * @return the mode string ({@code "auto"}, {@code "none"}, {@code "mmap"}, {@code "mlock"}, + * {@code "mmap+mlock"} or {@code "dio"}) + */ + @Override + public String getArgValue() { + return argValue; + } +} diff --git a/llama/src/main/java/net/ladenthin/llama/args/ModelFlag.java b/llama/src/main/java/net/ladenthin/llama/args/ModelFlag.java index 5efc8197..43dc5cb2 100644 --- a/llama/src/main/java/net/ladenthin/llama/args/ModelFlag.java +++ b/llama/src/main/java/net/ladenthin/llama/args/ModelFlag.java @@ -14,11 +14,27 @@ * {@link net.ladenthin.llama.parameters.ModelParameters#clearFlag(ModelFlag)} for programmatic control, * or use the named convenience methods (e.g. {@link net.ladenthin.llama.parameters.ModelParameters#enableSwaFull()}). * - *

{@code --flash-attn} is deliberately NOT here. It looks like a flag and was modelled as one, but - * llama.cpp has required a mandatory {@code on|off|auto} value since b10273 — emitting the key alone - * makes the parser consume the next argv token. Listing it would leave that broken argv reachable - * through {@code setFlag}. Use - * {@link net.ladenthin.llama.parameters.ModelParameters#setFlashAttn(net.ladenthin.llama.args.FlashAttn)}.

+ *

A constant is only listed here while llama.cpp's server argument parser still registers + * it. That parser treats an unknown option as a hard error, not a warning, so a stale + * constant does not merely have no effect — every {@code setFlag} caller gets + * {@code "Failed to parse model parameters"} instead of a loaded model. Four constants have been + * dropped for that reason and must not be reintroduced:

+ * + *
    + *
  • {@code --flash-attn} — looks like a flag and was modelled as one, but llama.cpp has required + * a mandatory {@code on|off|auto} value since b10273, so emitting the key alone makes the + * parser consume the next argv token. Use + * {@link net.ladenthin.llama.parameters.ModelParameters#setFlashAttn(net.ladenthin.llama.args.FlashAttn)}.
    • + *
  • {@code --mlock} and {@code --no-mmap} — deprecated at b10092 and deleted at b10878. + * Use {@link net.ladenthin.llama.parameters.ModelParameters#setLoadMode(LoadMode)} + * ({@link LoadMode#MLOCK} / {@link LoadMode#NONE} are upstream's own replacements).
    • + *
  • {@code --dump-kv-cache} — removed upstream with no replacement.
    • + *
+ * + *

The rule is enforced, not just documented: {@code src/test/cpp/test_model_flags.cpp} feeds every + * flag string in this file (and in {@code ModelParameters}) to the real + * {@code common_params_parser_init(params, LLAMA_EXAMPLE_SERVER)} option table, so adding an + * unaccepted one reds the {@code C++ Tests} job on every platform.

*/ public enum ModelFlag { @@ -51,9 +67,6 @@ public enum ModelFlag { /** Ignore end-of-stream token and continue generating. */ IGNORE_EOS("--ignore-eos"), - /** Enable verbose printing of the KV cache. */ - DUMP_KV_CACHE("--dump-kv-cache"), - /** Disable KV offload. */ NO_KV_OFFLOAD("--no-kv-offload"), @@ -63,12 +76,6 @@ public enum ModelFlag { /** Disable continuous batching. */ NO_CONT_BATCHING("--no-cont-batching"), - /** Force system to keep model in RAM rather than swapping or compressing. */ - MLOCK("--mlock"), - - /** Do not memory-map model (slower load but may reduce pageouts if not using mlock). */ - NO_MMAP("--no-mmap"), - /** Enable checking model tensor data for invalid values. */ CHECK_TENSORS("--check-tensors"), diff --git a/llama/src/main/java/net/ladenthin/llama/parameters/ModelParameters.java b/llama/src/main/java/net/ladenthin/llama/parameters/ModelParameters.java index 18108fab..8c9cc710 100644 --- a/llama/src/main/java/net/ladenthin/llama/parameters/ModelParameters.java +++ b/llama/src/main/java/net/ladenthin/llama/parameters/ModelParameters.java @@ -752,30 +752,82 @@ public ModelParameters setYarnBetaFast(float yarnBetaFast) { /** * Set group-attention factor (default: 1). * + *

No longer emitted — this method is a no-op. {@code --grp-attn-n} still exists in + * {@code common/arg.cpp}, but carries {@code set_examples({LLAMA_EXAMPLE_COMPLETION, + * LLAMA_EXAMPLE_PASSKEY})}, so {@code common_params_parser_init} never registers it for + * {@code LLAMA_EXAMPLE_SERVER} — the example this binding parses with. A textual sweep of + * upstream sources cannot see that; only the real option table can. Because llama.cpp's + * argument parser treats an unregistered option as a hard error rather than a warning, still + * emitting it would make {@code loadModel()} throw {@code "Failed to parse model parameters"} + * instead of loading the model. Writing nothing keeps existing call sites compiling and + * loading. The method will be removed in a future release; the contract is enforced by + * {@code src/test/cpp/test_model_flags.cpp}, which drives every emitted flag through the real + * server option table.

+ * * @param grpAttnN the group-attention factor * @return this builder + * @deprecated upstream scopes {@code --grp-attn-n} to non-server examples, so the server + * argument parser rejects it */ + // Error Prone's InlineMeSuggester wants an @InlineMe here because the body is a single + // expression. Inlining would be exactly wrong: the point of the deprecation is that callers + // keep calling THIS method, so a later removal is one edit here and not a code search. + @SuppressWarnings("InlineMeSuggester") + @Deprecated public ModelParameters setGrpAttnN(int grpAttnN) { - return putScalar("--grp-attn-n", grpAttnN); + return this; } /** * Set group-attention width (default: 512). * + *

No longer emitted — this method is a no-op. {@code --grp-attn-w} still exists in + * {@code common/arg.cpp}, but carries {@code set_examples({LLAMA_EXAMPLE_COMPLETION})}, so + * {@code common_params_parser_init} never registers it for {@code LLAMA_EXAMPLE_SERVER} — the + * example this binding parses with. Because llama.cpp's + * argument parser treats an unregistered option as a hard error rather than a warning, still + * emitting it would make {@code loadModel()} throw {@code "Failed to parse model parameters"} + * instead of loading the model. Writing nothing keeps existing call sites compiling and + * loading. The method will be removed in a future release; the contract is enforced by + * {@code src/test/cpp/test_model_flags.cpp}, which drives every emitted flag through the real + * server option table.

+ * * @param grpAttnW the group-attention width * @return this builder + * @deprecated upstream scopes {@code --grp-attn-w} to the completion example, so the server + * argument parser rejects it */ + // Error Prone's InlineMeSuggester wants an @InlineMe here because the body is a single + // expression. Inlining would be exactly wrong: the point of the deprecation is that callers + // keep calling THIS method, so a later removal is one edit here and not a code search. + @SuppressWarnings("InlineMeSuggester") + @Deprecated public ModelParameters setGrpAttnW(int grpAttnW) { - return putScalar("--grp-attn-w", grpAttnW); + return this; } /** * Enable verbose printing of the KV cache. * + *

No longer emitted — this method is a no-op. Upstream removed {@code --dump-kv-cache} + * with no replacement; it appears nowhere in llama.cpp at the pinned build. Because llama.cpp's + * argument parser treats an unregistered option as a hard error rather than a warning, still + * emitting it would make {@code loadModel()} throw {@code "Failed to parse model parameters"} + * instead of loading the model. Writing nothing keeps existing call sites compiling and + * loading. The method will be removed in a future release; the contract is enforced by + * {@code src/test/cpp/test_model_flags.cpp}, which drives every emitted flag through the real + * server option table.

+ * * @return this builder + * @deprecated upstream removed {@code --dump-kv-cache} with no replacement */ + // Error Prone's InlineMeSuggester wants an @InlineMe here because the body is a single + // expression. Inlining would be exactly wrong: the point of the deprecation is that callers + // keep calling THIS method, so a later removal is one edit here and not a code search. + @SuppressWarnings("InlineMeSuggester") + @Deprecated public ModelParameters enableDumpKvCache() { - return setFlag(ModelFlag.DUMP_KV_CACHE); + return this; } /** @@ -845,22 +897,68 @@ public ModelParameters disableContBatching() { return setFlag(ModelFlag.NO_CONT_BATCHING); } + /** + * Select how llama.cpp brings the model weights into memory. + * + *

Maps to upstream's {@code -lm}/{@code --load-mode}, the single option that replaced the + * independent {@code --mlock}, {@code --mmap}/{@code --no-mmap} and {@code --direct-io} switches + * (deprecated at b10092, deleted at b10878). Because the modes are mutually exclusive, the last + * call wins — {@link LoadMode#MMAP_MLOCK} is the way to ask for both mmap and mlock.

+ * + *

Upstream's default is {@link LoadMode#AUTO} (mmap unless a device cannot support it), so + * omitting this call is not the same as passing {@link LoadMode#NONE}.

+ * + * @param loadMode the model-loading mode + * @return this builder + */ + public ModelParameters setLoadMode(LoadMode loadMode) { + return putEnum("--load-mode", loadMode); + } + /** * Force system to keep model in RAM rather than swapping or compressing. * + *

Now emits {@code --load-mode mlock}. Upstream deprecated {@code --mlock} + * at b10092 and deleted it at b10878; since llama.cpp's argument parser treats an unknown + * option as a hard error rather than a warning, continuing to emit it would make + * {@code loadModel()} throw {@code "Failed to parse model parameters"}. The substitution is + * upstream's own — its deprecation shim mapped {@code --mlock} to + * {@code LLAMA_LOAD_MODE_MLOCK} — so behaviour is unchanged. Prefer + * {@link #setLoadMode(LoadMode)} directly; this method will be removed in a future release.

+ * * @return this builder + * @deprecated use {@link #setLoadMode(LoadMode)} with {@link LoadMode#MLOCK} */ + // Error Prone's InlineMeSuggester wants an @InlineMe here because the body is a single + // expression. Inlining would be exactly wrong: the point of the deprecation is that callers + // keep calling THIS method, so a later removal is one edit here and not a code search. + @SuppressWarnings("InlineMeSuggester") + @Deprecated public ModelParameters enableMlock() { - return setFlag(ModelFlag.MLOCK); + return setLoadMode(LoadMode.MLOCK); } /** * Do not memory-map model (slower load but may reduce pageouts if not using mlock). * + *

Now emits {@code --load-mode none}. Upstream deprecated {@code --no-mmap} + * at b10092 and deleted it at b10878; the substitution is upstream's own deprecation-shim + * mapping ({@code --no-mmap} to {@code LLAMA_LOAD_MODE_NONE}), so behaviour is unchanged. Note + * that this is a whole loading mode, not an independent switch: a later + * {@link #setLoadMode(LoadMode)} call overrides it, and combining "no mmap" with mlock is + * expressed as {@link LoadMode#MLOCK} rather than as two calls. Prefer + * {@link #setLoadMode(LoadMode)} directly; this method will be removed in a future release.

+ * * @return this builder + * @deprecated use {@link #setLoadMode(LoadMode)} with {@link LoadMode#NONE} */ + // Error Prone's InlineMeSuggester wants an @InlineMe here because the body is a single + // expression. Inlining would be exactly wrong: the point of the deprecation is that callers + // keep calling THIS method, so a later removal is one edit here and not a code search. + @SuppressWarnings("InlineMeSuggester") + @Deprecated public ModelParameters disableMmap() { - return setFlag(ModelFlag.NO_MMAP); + return setLoadMode(LoadMode.NONE); } /** @@ -1094,22 +1192,54 @@ public ModelParameters setHfFile(String hfFile) { /** * Set the Hugging Face model repository for the vocoder model (default: unused). * + *

No longer emitted — this method is a no-op. Upstream removed {@code --hf-repo-v} with the + * OuteTTS-era two-model TTS design; it appears nowhere in llama.cpp at the pinned build. The + * current TTS pipeline takes a backbone plus an mmproj GGUF — see + * {@link net.ladenthin.llama.TextToSpeech}. Because llama.cpp's + * argument parser treats an unregistered option as a hard error rather than a warning, still + * emitting it would make {@code loadModel()} throw {@code "Failed to parse model parameters"} + * instead of loading the model. Writing nothing keeps existing call sites compiling and + * loading. The method will be removed in a future release; the contract is enforced by + * {@code src/test/cpp/test_model_flags.cpp}, which drives every emitted flag through the real + * server option table.

+ * * @param hfRepoV the Hugging Face repository for the vocoder model * @return this builder + * @deprecated upstream removed {@code --hf-repo-v}; see {@link net.ladenthin.llama.TextToSpeech} */ + // Error Prone's InlineMeSuggester wants an @InlineMe here because the body is a single + // expression. Inlining would be exactly wrong: the point of the deprecation is that callers + // keep calling THIS method, so a later removal is one edit here and not a code search. + @SuppressWarnings("InlineMeSuggester") + @Deprecated public ModelParameters setHfRepoV(String hfRepoV) { - parameters.put("--hf-repo-v", hfRepoV); return this; } /** * Set the Hugging Face model file for the vocoder model (default: unused). * + *

No longer emitted — this method is a no-op. Upstream removed {@code --hf-file-v} with the + * OuteTTS-era two-model TTS design; it appears nowhere in llama.cpp at the pinned build. The + * current TTS pipeline takes a backbone plus an mmproj GGUF — see + * {@link net.ladenthin.llama.TextToSpeech}. Because llama.cpp's + * argument parser treats an unregistered option as a hard error rather than a warning, still + * emitting it would make {@code loadModel()} throw {@code "Failed to parse model parameters"} + * instead of loading the model. Writing nothing keeps existing call sites compiling and + * loading. The method will be removed in a future release; the contract is enforced by + * {@code src/test/cpp/test_model_flags.cpp}, which drives every emitted flag through the real + * server option table.

+ * * @param hfFileV the vocoder model file within the Hugging Face repository * @return this builder + * @deprecated upstream removed {@code --hf-file-v}; see {@link net.ladenthin.llama.TextToSpeech} */ + // Error Prone's InlineMeSuggester wants an @InlineMe here because the body is a single + // expression. Inlining would be exactly wrong: the point of the deprecation is that callers + // keep calling THIS method, so a later removal is one edit here and not a code search. + @SuppressWarnings("InlineMeSuggester") + @Deprecated public ModelParameters setHfFileV(String hfFileV) { - parameters.put("--hf-file-v", hfFileV); return this; } diff --git a/llama/src/test/cpp/test_model_flags.cpp b/llama/src/test/cpp/test_model_flags.cpp new file mode 100644 index 00000000..4689f171 --- /dev/null +++ b/llama/src/test/cpp/test_model_flags.cpp @@ -0,0 +1,148 @@ +// SPDX-FileCopyrightText: 2026 Bernard Ladenthin +// +// SPDX-License-Identifier: MIT +// +// Contract test: every CLI flag the Java layer can emit must be one the llama.cpp server arg +// parser actually registers. +// +// WHY THIS IS NOT REDUNDANT WITH THE JAVA TESTS +// --------------------------------------------- +// `LlamaModel.loadModel(parameters.toArray())` hands the ModelParameters map to +// `common_params_parse(..., LLAMA_EXAMPLE_SERVER)` as argv. An option that parser does not +// know is a hard error, not a warning: `arg.cpp` throws, `common_params_parse` returns false, +// and `load_model_impl` throws `LlamaException("Failed to parse model parameters")`. So a flag +// upstream removed does not degrade the call -- it makes the model unloadable. +// +// The Java tests cannot see this. `ModelFlagTest` and `ModelParametersExtendedTest` assert the +// *string mapping* (`assertThat(p.parameters, hasKey("--mlock"))`), never that llama.cpp still +// accepts the string, so they pass forever while the flag is dead. That is how `--mlock` and +// `--no-mmap` survived their removal at b10878, and `--dump-kv-cache` / `--hf-repo-v` / +// `--hf-file-v` survived theirs for far longer. +// +// WHY A GREP OVER common/arg.cpp IS NOT A SUBSTITUTE +// --------------------------------------------------- +// `--grp-attn-n` and `--grp-attn-w` are *present* in `arg.cpp` at every tag this project has +// pinned, so any textual sweep reports them alive. They carry +// `.set_examples({LLAMA_EXAMPLE_COMPLETION, ...})`, and `common_params_parser_init`'s `add_opt` +// filters by example at registration time -- so they are never registered for +// `LLAMA_EXAMPLE_SERVER` and the server parser rejects them exactly like a deleted flag. Only +// the real option table knows this, which is why the oracle here is +// `common_params_parser_init(params, LLAMA_EXAMPLE_SERVER).options` rather than a file scan. +// +// The Java-side list is generated at configure time from ModelFlag.java + ModelParameters.java +// (cmake/extract-java-cli-flags.cmake), so the two halves cannot drift: adding a builder method +// with a bad flag string reds this test without anyone remembering to update it. +// +// Hermetic: no model, no JVM, no network. `common_params_parser_init` only fills a struct. + +#include "arg.h" +#include "common.h" + +#include "jllama_java_cli_flags.h" + +#include + +#include +#include +#include +#include + +namespace { + +// Flags the Java layer emits on purpose that llama.cpp is NOT expected to know. +// +// `--vocab-only` is this project's own pseudo-flag: `jllama.cpp`'s `loadModel` calls +// `strip_flag_from_argv(argv, argc, "--vocab-only", &vocab_only)` and removes it *before* +// `common_params_parse` ever sees the argv, using it to select the vocab-only path that owns +// its model directly and never starts a server_context. Adding an entry here is a deliberate +// statement that some project code removes the flag from argv -- never a way to silence this +// test for a flag that is simply dead. +const std::set &project_only_flags() { + static const std::set flags = {"--vocab-only"}; + return flags; +} + +// Every option string the server example registers, positive and negated forms alike. +std::set server_registered_flags() { + common_params params; + common_params_context ctx = common_params_parser_init(params, LLAMA_EXAMPLE_SERVER); + + std::set registered; + for (const auto &opt : ctx.options) { + for (const auto &arg : opt.get_args()) { + registered.insert(arg); + } + } + return registered; +} + +} // namespace + +// The extractor must never hand us an empty or obviously truncated list: a vacuous pass is the +// failure mode this whole file exists to prevent. cmake/extract-java-cli-flags.cmake enforces a +// floor of its own at configure time; this repeats it at the consuming end so a hand-edited or +// stale generated header cannot slip through either. +TEST(JavaCliFlagContract, GeneratedFlagListIsPopulated) { + ASSERT_GT(JLLAMA_JAVA_CLI_FLAG_COUNT, 50) + << "the generated Java CLI flag list is empty or truncated -- the extractor is broken"; + ASSERT_EQ(JLLAMA_JAVA_CLI_FLAG_COUNT, + static_cast(sizeof(JLLAMA_JAVA_CLI_FLAGS) / sizeof(JLLAMA_JAVA_CLI_FLAGS[0]))); +} + +// Sanity-check the oracle itself before trusting its verdict: if `common_params_parser_init` +// ever returned an empty table, every flag would "pass" the exemption path below instead. +TEST(JavaCliFlagContract, ServerOptionTableIsPopulated) { + const std::set registered = server_registered_flags(); + ASSERT_GT(registered.size(), 100u) + << "common_params_parser_init(LLAMA_EXAMPLE_SERVER) returned an implausibly small option " + "table -- the oracle is broken, not the Java layer"; + // Spot-check a flag every llama.cpp version this project supports has had. + EXPECT_EQ(registered.count("--model"), 1u); +} + +// THE contract. A failure here means a ModelParameters/ModelFlag member produces an argv that +// makes loadModel() throw -- fix the Java side (repoint, deprecate or remove it), do not add an +// exemption. +TEST(JavaCliFlagContract, EveryJavaEmittedFlagIsAcceptedByTheServerParser) { + const std::set registered = server_registered_flags(); + + std::vector rejected; + for (int i = 0; i < JLLAMA_JAVA_CLI_FLAG_COUNT; ++i) { + const std::string flag = JLLAMA_JAVA_CLI_FLAGS[i]; + if (project_only_flags().count(flag) != 0) { + continue; + } + if (registered.count(flag) == 0) { + rejected.push_back(flag); + } + } + + std::string message; + for (const auto &flag : rejected) { + message += "\n " + flag; + } + EXPECT_TRUE(rejected.empty()) + << "The Java layer emits " << rejected.size() + << " flag(s) that llama.cpp's server arg parser does not register. Every caller of the " + "matching builder method gets 'Failed to parse model parameters' instead of a loaded " + "model:" + << message + << "\nCheck whether upstream removed the option or narrowed its set_examples() away from " + "LLAMA_EXAMPLE_SERVER, then repoint or retire the Java member."; +} + +// The exemption list is not allowed to rot either: an entry that upstream later *does* register +// would silently stop being checked. (It is fine for a project-only flag to stay unknown to +// llama.cpp -- that is the point -- so this only asserts each entry is still emitted by Java.) +TEST(JavaCliFlagContract, EveryExemptedFlagIsStillEmittedByJava) { + std::set emitted; + for (int i = 0; i < JLLAMA_JAVA_CLI_FLAG_COUNT; ++i) { + emitted.insert(JLLAMA_JAVA_CLI_FLAGS[i]); + } + for (const auto &flag : project_only_flags()) { + EXPECT_EQ(emitted.count(flag), 1u) + << flag + << " is exempted from the contract but no longer emitted by the Java layer -- " + "drop the exemption"; + } +} diff --git a/llama/src/test/java/net/ladenthin/llama/args/LoadModeTest.java b/llama/src/test/java/net/ladenthin/llama/args/LoadModeTest.java new file mode 100644 index 00000000..f40675ed --- /dev/null +++ b/llama/src/test/java/net/ladenthin/llama/args/LoadModeTest.java @@ -0,0 +1,22 @@ +// SPDX-FileCopyrightText: 2026 Bernard Ladenthin +// +// SPDX-License-Identifier: MIT + +package net.ladenthin.llama.args; + +import java.util.Arrays; +import java.util.Collection; + +public class LoadModeTest extends AbstractCliArgEnumTest { + + public static Collection data() { + return Arrays.asList(new Object[][] { + {LoadMode.AUTO, "auto", 6}, + {LoadMode.NONE, "none", 6}, + {LoadMode.MMAP, "mmap", 6}, + {LoadMode.MLOCK, "mlock", 6}, + {LoadMode.MMAP_MLOCK, "mmap+mlock", 6}, + {LoadMode.DIRECT_IO, "dio", 6}, + }); + } +} diff --git a/llama/src/test/java/net/ladenthin/llama/args/ModelFlagTest.java b/llama/src/test/java/net/ladenthin/llama/args/ModelFlagTest.java index d55f7679..d55cec2e 100644 --- a/llama/src/test/java/net/ladenthin/llama/args/ModelFlagTest.java +++ b/llama/src/test/java/net/ladenthin/llama/args/ModelFlagTest.java @@ -26,12 +26,9 @@ public static Collection data() { {ModelFlag.NO_WARMUP, "--no-warmup"}, {ModelFlag.SPM_INFILL, "--spm-infill"}, {ModelFlag.IGNORE_EOS, "--ignore-eos"}, - {ModelFlag.DUMP_KV_CACHE, "--dump-kv-cache"}, {ModelFlag.NO_KV_OFFLOAD, "--no-kv-offload"}, {ModelFlag.CONT_BATCHING, "--cont-batching"}, {ModelFlag.NO_CONT_BATCHING, "--no-cont-batching"}, - {ModelFlag.MLOCK, "--mlock"}, - {ModelFlag.NO_MMAP, "--no-mmap"}, {ModelFlag.CHECK_TENSORS, "--check-tensors"}, {ModelFlag.EMBEDDING, "--embedding"}, {ModelFlag.RERANKING, "--reranking"}, @@ -66,10 +63,17 @@ public void testGetCliFlag(ModelFlag flag, String expectedCliFlag) { @Test public void testEnumCount() { - // 34 since FLASH_ATTN was removed: --flash-attn is not a valueless flag (llama.cpp b10273 - // made its on|off|auto value mandatory), so modelling it here left a broken argv reachable - // through setFlag. It lives in the FlashAttn enum instead. - assertEquals(34, ModelFlag.values().length); + // 31 after four removals, all for the same reason: llama.cpp's server arg parser rejects an + // option it does not register, so a stale constant does not merely have no effect -- it makes + // every setFlag caller's model unloadable. + // FLASH_ATTN -- --flash-attn stopped being valueless at b10273 (on|off|auto is + // mandatory), so emitting the key alone consumed the next argv token. + // Use ModelParameters#setFlashAttn. + // MLOCK, NO_MMAP -- deprecated at b10092, deleted at b10878. + // Use ModelParameters#setLoadMode. + // DUMP_KV_CACHE -- removed upstream with no replacement. + // src/test/cpp/test_model_flags.cpp enforces this against the real option table. + assertEquals(31, ModelFlag.values().length); } @ParameterizedTest(name = "{0} -> {1}") diff --git a/llama/src/test/java/net/ladenthin/llama/parameters/ModelParametersExtendedTest.java b/llama/src/test/java/net/ladenthin/llama/parameters/ModelParametersExtendedTest.java index ea51ef97..6d384c99 100644 --- a/llama/src/test/java/net/ladenthin/llama/parameters/ModelParametersExtendedTest.java +++ b/llama/src/test/java/net/ladenthin/llama/parameters/ModelParametersExtendedTest.java @@ -371,16 +371,31 @@ public void testSetYarnBetaFast() { // Group attention // ------------------------------------------------------------------------- + /** + * Asserts that a retired builder method wrote nothing at all -- compared against a pristine + * instance rather than against emptiness, because the constructor seeds defaults of its own + * ({@code --fit} today), and a future default must not quietly weaken this assertion. + */ + private static void assertWroteNothing(ModelParameters actual) { + assertThat(actual.parameters, is(new ModelParameters().parameters)); + } + + // Retired flags: llama.cpp's server arg parser does not register these, and it treats an + // unregistered option as a hard error -- so the builder methods must write NOTHING, or every + // caller's model becomes unloadable. Asserting emptiness (not just "no longer that key") is the + // point: the old assertions pinned the mapping and would have passed forever while the flag was + // dead. src/test/cpp/test_model_flags.cpp is the upstream-facing half of this guard. + // --grp-attn-n/-w still exist upstream but are set_examples()-scoped to + // LLAMA_EXAMPLE_COMPLETION/PASSKEY, so the server example never registers them. + @Test - public void testSetGrpAttnN() { - ModelParameters p = new ModelParameters().setGrpAttnN(4); - assertThat(p.parameters.get("--grp-attn-n"), is("4")); + public void testSetGrpAttnNIsRetiredAndEmitsNothing() { + assertWroteNothing(new ModelParameters().setGrpAttnN(4)); } @Test - public void testSetGrpAttnW() { - ModelParameters p = new ModelParameters().setGrpAttnW(1024); - assertThat(p.parameters.get("--grp-attn-w"), is("1024")); + public void testSetGrpAttnWIsRetiredAndEmitsNothing() { + assertWroteNothing(new ModelParameters().setGrpAttnW(1024)); } // ------------------------------------------------------------------------- @@ -422,11 +437,10 @@ public void testDisableKvOffload() { assertThat(p.parameters.get("--no-kv-offload"), is(nullValue())); } + // --dump-kv-cache was removed upstream with no replacement; see the retired-flag note above. @Test - public void testEnableDumpKvCache() { - ModelParameters p = new ModelParameters().enableDumpKvCache(); - assertThat(p.parameters, hasKey("--dump-kv-cache")); - assertThat(p.parameters.get("--dump-kv-cache"), is(nullValue())); + public void testEnableDumpKvCacheIsRetiredAndEmitsNothing() { + assertWroteNothing(new ModelParameters().enableDumpKvCache()); } @Test @@ -579,17 +593,37 @@ public void testSetDevices() { // ------------------------------------------------------------------------- @Test - public void testEnableMlock() { + public void testSetLoadModeAllValues() { + for (LoadMode mode : LoadMode.values()) { + ModelParameters p = new ModelParameters().setLoadMode(mode); + assertThat(p.parameters.get("--load-mode"), is(mode.getArgValue())); + } + } + + // enableMlock()/disableMmap() kept their names but changed what they emit: upstream deleted + // --mlock and --no-mmap at b10878, and these are the substitutions upstream's own deprecation + // shim used (LLAMA_LOAD_MODE_MLOCK / LLAMA_LOAD_MODE_NONE), so behaviour is unchanged. + + @Test + public void testEnableMlockEmitsLoadModeMlock() { ModelParameters p = new ModelParameters().enableMlock(); - assertThat(p.parameters, hasKey("--mlock")); - assertThat(p.parameters.get("--mlock"), is(nullValue())); + assertThat(p.parameters.get("--load-mode"), is("mlock")); + assertThat(p.parameters, not(hasKey("--mlock"))); } @Test - public void testDisableMmap() { + public void testDisableMmapEmitsLoadModeNone() { ModelParameters p = new ModelParameters().disableMmap(); - assertThat(p.parameters, hasKey("--no-mmap")); - assertThat(p.parameters.get("--no-mmap"), is(nullValue())); + assertThat(p.parameters.get("--load-mode"), is("none")); + assertThat(p.parameters, not(hasKey("--no-mmap"))); + } + + @Test + public void testLoadModeIsSingleValuedSoTheLastCallWins() { + // The three used to be independent switches; they are one mutually exclusive mode now, so a + // caller wanting both mmap and mlock must say MMAP_MLOCK rather than chain two calls. + ModelParameters p = new ModelParameters().disableMmap().enableMlock(); + assertThat(p.parameters.get("--load-mode"), is("mlock")); } @Test @@ -879,16 +913,17 @@ public void testSetHfToken() { assertThat(p.parameters.get("--hf-token"), is("hf_abc123")); } + // --hf-repo-v/--hf-file-v went away with the OuteTTS-era two-model TTS design; see the + // retired-flag note above. + @Test - public void testSetHfRepoV() { - ModelParameters p = new ModelParameters().setHfRepoV("org/vocoder"); - assertThat(p.parameters.get("--hf-repo-v"), is("org/vocoder")); + public void testSetHfRepoVIsRetiredAndEmitsNothing() { + assertWroteNothing(new ModelParameters().setHfRepoV("org/vocoder")); } @Test - public void testSetHfFileV() { - ModelParameters p = new ModelParameters().setHfFileV("vocoder.gguf"); - assertThat(p.parameters.get("--hf-file-v"), is("vocoder.gguf")); + public void testSetHfFileVIsRetiredAndEmitsNothing() { + assertWroteNothing(new ModelParameters().setHfFileV("vocoder.gguf")); } // -------------------------------------------------------------------------