diff --git a/.github/verify-patches-applied.sh b/.github/verify-patches-applied.sh index 64ec0c88..418d6dfe 100755 --- a/.github/verify-patches-applied.sh +++ b/.github/verify-patches-applied.sh @@ -7,21 +7,26 @@ # Asserts that every llama/patches/*.patch really reached the fetched llama.cpp tree. # # WHY THIS EXISTS. The patch applier (llama/cmake/apply-llama-patches.cmake) is fail-loud on -# "does not apply", so a *stale* patch cannot ship silently. What it cannot detect is a patch -# that stops having an effect while still applying, and most patches do not need this check -# because they have a runnable guard that reds CI on every platform if they go missing: +# "does not apply", so a *stale* patch cannot ship silently. What it cannot detect is the +# applier never having run at all, or a patched tree being reverted after the fact — the stamp +# bookkeeping and the tree's dirty state are the only evidence of that, and this script asserts +# both. It runs in the always-on `C++ Tests` job, needs no model, and costs milliseconds. +# +# Every patch in the set does also have a runnable guard that reds CI on every platform if it +# goes missing, so these checks are a second line rather than the only one: # # 0003, 0006, 0007, 0008 -> jllama.cpp / native_server.cpp call the symbols they add, # so dropping one is a compile or link error. # 0012 -> src/test/cpp/test_model_split.cpp. +# 0014 -> src/test/cpp/test_common_log_callback.cpp (link error). # 0001, 0002 -> model-gated Java jobs (Windows argv, LoadProgressCallbackTest). # -# `0010` is the exception and the reason for this script. It casts one enum to int inside -# upstream's `get_res_model_info()`, which is `static` in server-context.cpp and therefore -# unreachable from jllama_test; reverting it leaves `ctest` completely green. Its only guard is -# NativeServerAttachIntegrationTest.models_reportNumericVocabType, which is model-gated — so the -# day a platform stops downloading models, the regression ships. This check runs in the -# always-on `C++ Tests` job, needs no model, and costs milliseconds. +# It used to carry a third, patch-specific check for `0010`, the one patch with no runnable +# guard (it cast an enum inside upstream's `static get_res_model_info()`, unreachable from +# jllama_test). That patch was DROPPED at the b11080 bump — upstream #28518 gave +# `common_json_value` an enum constructor, fixing the defect at its root — so the check retired +# with it. If a future patch is ever added that likewise cannot be reached from `ctest`, add a +# check for it here rather than relying on a model-gated Java test. # # Usage: .github/verify-patches-applied.sh [] # Exit codes: 0 all good, 1 a check failed. @@ -67,14 +72,4 @@ if git -C "$SRC" rev-parse --git-dir >/dev/null 2>&1; then fi fi -# --- 3. the one patch with no runnable guard ------------------------------------------------------ -VOCAB_CAST='(int) meta.model_vocab_type' -SERVER_CONTEXT="$SRC/tools/server/server-context.cpp" -[ -f "$SERVER_CONTEXT" ] || fail "not found: $SERVER_CONTEXT" -grep -qF "$VOCAB_CAST" "$SERVER_CONTEXT" \ - || fail "patches/0010 is not present in $SERVER_CONTEXT: expected '$VOCAB_CAST'. - Without the cast, common_json binds the unscoped enum to its bool constructor and - GET /models + GET /v1/models report vocab_type as true/false instead of a number. - If upstream added the cast themselves, DROP patch 0010 and update this check." - -echo "patches verified: $on_disk applied, tree dirty, patches/0010 cast present" +echo "patches verified: $on_disk applied, tree dirty" diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 61abf90c..1b7ebafb 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -2356,10 +2356,10 @@ jobs: run: | mvn -q --no-transfer-progress -f llama/pom.xml compile .github/build.sh -DBUILD_TESTING=ON - # Most patches have a runnable guard that reds this job if they go missing (a link error, or - # test_utils.cpp / test_model_split.cpp). patches/0010 has none — it casts one enum inside a - # `static` function unreachable from jllama_test, so reverting it leaves ctest fully green and - # only a model-gated Java test notices. This is the always-on, model-free check for it. + # Every patch has a runnable guard that reds this job if it goes missing (a link error, or + # test_utils.cpp / test_model_split.cpp / test_common_log_callback.cpp). This is the second + # line: it asserts the applier actually ran and nothing reverted the patched tree, which no + # per-patch guard covers directly. Model-free, milliseconds. - name: Verify llama.cpp patches are applied run: .github/verify-patches-applied.sh - name: Run C++ unit tests diff --git a/CHANGELOG.md b/CHANGELOG.md index 863aaf60..7920ca88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -101,6 +101,33 @@ from version 5.0.0 onward. Pre-fork releases (`1.x`–`4.2.0`) were authored by where the backend cannot provide it, `OFF` disables it. ### Changed +- **llama.cpp `b11069` → `b11080`, and local patch `0010` dropped — upstream fixed the + enum-to-JSON-boolean trap at its root.** Eleven upstream commits, 1244 KiB, no project-source + change. The size is one commit that does not concern this project (llama.cpp #29197 rewrites 46 + files under `ggml/src/ggml-hexagon/`; no hexagon classifier is built here); the rest of `ggml/src` + is additive ARM repack kernels, a Metal fusion-list simplification and a SYCL softmax tweak, and + `ggml/include` is byte-identical. The one that matters is llama.cpp #28518 ("json: Fixed json enum + handling"): `common_json_value` gains an `std::is_enum`-gated constructor delegating to the + underlying type, so an unscoped enum no longer binds to `common_json_value(bool)` and serialises + as `true`/`false`. That is exactly the defect `patches/0010` cast around in upstream's own + `get_res_model_info()`, so the patch became a redundant carry and was dropped rather than kept + (the `0009`/`0011`/`0013` precedent). **Nothing observable changes for consumers** — + `GET /models` and `GET /v1/models` reported a numeric `vocab_type` with the patch and still do + without it — but the drop is worth flagging because `0010` *still applied cleanly*: the fail-loud + applier can only detect "does not apply", never "upstream already fixed this", which is why that + patch carries a by-hand drop-check on every bump. Its guard was kept and re-pointed: the + `CommonJsonEnumTrap` pair in `test_json_helpers.cpp` is now the `CommonJsonEnum` trio and pins + upstream's contract (uncast enum is numeric, an explicit cast is equivalent, a real `bool` is + still a boolean), so a bump that loses the constructor reds `C++ Tests` everywhere instead of + shipping a boolean. `jllama.cpp` keeps its own two `"vocab_type"` casts — correct either way. + Also in range: six existing sampling flags gained environment defaults (#27380 — + `LLAMA_ARG_TEMPERATURE`, `_TOP_P`, `_MIN_P`, `_REPEAT_PENALTY`, `_PRESENCE_PENALTY`, + `_FREQUENCY_PENALTY`), which adds no option but does mean a host with those variables set now + inherits them; and a router no longer forwards `LLAMA_ARG_API_KEY_FILE` to spawned children + (#28938). `tools/server/`'s schema, task and context translation units are byte-identical, and + the request-field set (68), bounded-field set (23) and response-key set (142) were all verified + unchanged mechanically, so the server wire contract cannot have moved. The other eight patches + apply unchanged and all four remaining drop-checks still say "still required". - **llama.cpp `b11062` → `b11069`, and local patch `0011` dropped — malformed UTF-8 in a completion is now replaced, not truncated.** Seven upstream commits, 57 KiB, no project-source change; the one that matters is llama.cpp #29161 ("common/peg : handle invalid utf-8 sequences in diff --git a/CLAUDE.md b/CLAUDE.md index ddfc1db2..a12e2f04 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,7 +6,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co Java bindings for [llama.cpp](https://github.com/ggerganov/llama.cpp) via JNI, providing a high-level API for LLM inference in Java. The Java layer communicates with a native C++ library through JNI. -Current llama.cpp pinned version: **b11069** +Current llama.cpp pinned version: **b11080** ## Upgrading CUDA Version @@ -510,7 +510,7 @@ needs no extra step here, `build-webui` re-reads the tag and rebuilds the matchi ships no UI): ```bash # needs node/npm + network for the asset build; the embed step is plain cmake -P -git clone --depth 1 --branch b11069 https://github.com/ggml-org/llama.cpp /tmp/lc +git clone --depth 1 --branch b11080 https://github.com/ggml-org/llama.cpp /tmp/lc ( cd /tmp/lc/tools/ui && npm ci && npm run build ) mkdir -p webui-generated /tmp/ui-gen cmake -DUI_SOURCE_DIR=/tmp/lc/tools/ui -DUI_BINARY_DIR=/tmp/ui-gen \ @@ -550,7 +550,7 @@ cache lives in **Depot Cache** over sccache's **WebDAV** backend: - `SCCACHE_WEBDAV_TOKEN: ${{ secrets.DEPOT_TOKEN }}` — a Depot **organization** token, stored as the repo secret **`DEPOT_TOKEN`**. -Because `sccache` is **content-addressed** and llama.cpp is pinned (`GIT_TAG b11069`), the +Because `sccache` is **content-addressed** and llama.cpp is pinned (`GIT_TAG b11080`), the ~280 upstream object files are byte-identical every run, so a warm cache recompiles only the *changed* files. Depot's cache is **shared across all branches** (unlike GitHub's per-branch `actions/cache`), so every branch builds incrementally; a `b` version bump @@ -753,11 +753,39 @@ Current patches: | `0003-pr22393-server-add-slot-prompt-similarity-getter-setter.patch` | **Upstream-PR carry** of [ggml-org/llama.cpp#22393](https://github.com/ggml-org/llama.cpp/pull/22393) ("server : add slot_prompt_similarity getter/setter"). Purely additive: adds `server_context::get_slot_prompt_similarity()` / `set_slot_prompt_similarity(float)` (`tools/server/server-context.{cpp,h}`) so an embedding/JNI caller can query and tune the slot-selection threshold at runtime without reloading the model. Verbatim copy of the PR, which **upstream closed without merging** (rejected as exposing unsafe internal state — see the patch header). Carried permanently; it will not be droppable via a version bump. | | `0007-server-attach-http-frontend.patch` | **Adds `llama_server_attach(argc, argv, server_context&)`** so the `NativeServer` *attach mode* can serve an **already-loaded `LlamaModel`** over the upstream HTTP frontend — no second model load, no `start_loop()`; the LlamaModel's worker keeps driving the shared `server_context` and the HTTP routes post tasks to its queue (the queue is the synchronization point). Mechanically: (1) extracts the **pure core route table** (`health` … `slots`) out of `llama_server()` into `static void llama_server_register_common_routes(ctx_http, routes)` (shared, so the two entry points cannot drift on the core endpoint set). **Scope note (narrowed at the b10154 bump):** the helper deliberately carries **only** the stable, state-independent route table — **not** the resumable-streaming routes (their handlers differ between router / non-router), the GCP-compat shim, or the experimental **CORS-proxy / MCP-server / built-in-tools** wiring. b10154 (upstream MCP-server support) moved the streaming routes into the middle of that block and coupled tools/CORS to a per-call `server_mcp mcp_mgr` lifecycle, so the earlier contiguous "route-table + CORS-proxy + tools" extraction is no longer possible; `llama_server()` keeps all of that inline, **byte-identical to upstream b10154** (only the route-table block is factored out). (2) adds `llama_server_attach`, which parses only the HTTP-side argv via `common_params_parse`, starts the stream-session GC + `server_http_context`, registers the common route table, the **non-router** resumable-streaming handlers (upstream b10154 paths `/v1/stream` GET/DEL + `/v1/streams/lookup` POST), the GCP-compat shim, and **403 "disabled" stubs for `/cors-proxy` + `/tools`** (attach mode does not wire the experimental CORS-proxy / MCP / built-in-tools host — those belong to a full `llama-server`, not an embedded model), marks ready immediately (model already loaded), and blocks on the HTTP thread until `llama_server_request_shutdown()` — never calling `common_init()`, backend init, `ctx_server.terminate()` or `llama_backend_free()` (the embedding caller owns those). Applies after `0001`+`0006` (same file); closes the "NativeServer — reuse an already-loaded LlamaModel" TODO. Upstream-submittable ("server: let embedding callers attach the HTTP frontend to an existing server_context"). **Refreshed at the b10519 bump:** upstream #26347 dropped the API key from the `/models` + `/v1/models` public-endpoint set and deleted the two trailing `// public endpoint (no API key check)` comments on those route registrations. Those two lines sit inside this patch's route-table removal block, so `git apply` failed ("patch does not apply", `server.cpp:258`) at **every** tag from b10519 on; the fix was to drop the now-wrong comment from all four affected lines (2 on the `-` side, 2 in the extracted helper on the `+` side), keeping the helper byte-identical to the block it replaces. **This is the invariant to re-check on every bump:** the `+` side of `llama_server_register_common_routes()` must stay a verbatim copy of the route table it factors out of `llama_server()`. | | `0008-server-models-worker-cmd-override.patch` | **Makes router mode usable in-JVM.** The router (`server-models.cpp`) spawns each model worker by re-executing its own binary (`get_server_exec_path()` = `/proc/self/exe` & friends) — inside a JVM that binary is `java`, not a llama-server, so embedded router workers could never start. The patch adds env `LLAMA_SERVER_WORKER_CMD` (whitespace-split; read in `server_model_meta::update_args`) which replaces only the leading binary-path token of the rendered worker args, letting an embedding host relaunch workers through its own bootstrap — e.g. `java -cp app.jar net.ladenthin.llama.server.NativeServer` (each worker is then a fresh JVM running the classic single-model `NativeServer`). Exposed in Java as `NativeServer.setWorkerCommand(String...)` (JNI `setenv`); exercised by `RouterModeIntegrationTest` (Linux CI). Upstream-submittable (also useful for containerized/wrapped deployments). | -| `0010-server-cast-vocab-type-for-common-json.patch` | **Upstream regression from the b10585 `common_json` switch (#27511), one line.** `get_res_model_info()` (`tools/server/server-context.cpp`) builds the `GET /models` + `GET /v1/models` payload and emits `{"vocab_type", meta.model_vocab_type}` — an **unscoped enum**. `common_json_value`'s integral constructor template is `std::is_integral`-gated, which *excludes* enums, so the value binds to `common_json_value(bool)` and serialises as `true`/`false` instead of the numeric vocab type. It was correct while the alias was `nlohmann::ordered_json` (nlohmann serialises an enum as an integer), so upstream regressed it silently when they flipped the alias. The project ships this: `server-context.cpp` is compiled into `libjllama` and both routes are served by `NativeServer` — the default fat-jar `Main-Class` — in full **and** attach mode (`patches/0007`'s common route table registers them). The patch casts the value to `int` at the emit site, mirroring what `jllama.cpp` does for its own two `"vocab_type"` sites. Upstream-submittable; **not yet filed upstream**. Applies after `0002`/`0003` (same file) — numbered `0010` because `0009` is burned: it names the subprocess.h patch dropped at the b10280 bump (see the note below this table), and reusing the number would make that note read as if it were about this patch. **On every bump, check whether upstream cast the value themselves; if they did, DROP this patch rather than refreshing it** — the fail-loud applier only detects "does not apply", never "upstream already fixed this", and no test can catch a redundant carry here because `get_res_model_info` is `static` inside `server-context.cpp` and unreachable from `jllama_test`. See the `CommonJsonEnumTrap` tests in `test_json_helpers.cpp` for the mechanism the cast defends against. | | `0006-server-embed-native-server-jni.patch` | **Makes `server.cpp`'s `llama_server` embeddable in the JVM** so the `NativeServer` JNI bridge can run the full upstream HTTP server (WebUI included) inside `libjllama` — see "Two server modes" below. b9870 already exposes `int llama_server(int, char**)` (non-static; no `main` in the file), so the patch only adds embedded-mode support: (1) a `g_llama_server_embedded` flag + `llama_server_set_embedded()` / `llama_server_request_shutdown()` (declared in the committed `src/main/cpp/native_server_bridge.h`); (2) skips installing the process-wide SIGINT/SIGTERM handlers when embedded (they would hijack the JVM's); (3) in embedded mode parses the **forwarded** argv via `common_params_parse` instead of `common_params_parse_main` (whose `GetCommandLineW` recovery would pick up `java.exe`'s command line — the same Windows class of bug `0001` fixes). `llama_server_request_shutdown()` mirrors the SIGTERM path (invokes the installed `shutdown_handler` → `ctx_server.terminate()` unblocks `start_loop()`), giving JNI an out-of-band stop since `ctx_server` is loop-local. Applies **after `0001`** (which flips this call site to `common_params_parse_main`), so its context is the post-`0001` tree; regenerate against `0001`+source on a bump. Only touches `tools/server/server.cpp`. | | `0012-model-guard-zero-split-sum-and-name-the-device-index.patch` | **A GPU that reports zero free memory makes every model load fail with the unactionable `error loading model: vector`.** `llama_model_base::load_tensors` (`src/llama-model.cpp`) weights the per-device layer split by `ggml_backend_dev_memory()`'s `free`, then normalises: `splits[i] /= split_sum`. With a single device reporting `free == 0` that is `0/0` → **NaN** in every split point; NaN compares false against everything, so the `std::upper_bound` below returns the end iterator, `layer_gpu == n_devices()`, and `devices.at(layer_gpu)` throws `std::out_of_range` — whose libc++ `what()` is the bare string `"vector"`, which `llama.cpp`'s `catch (const std::exception &)` prints verbatim. Upstream's `free == 0 && total == 0` host-memory fallback does **not** fire, because `total` is `recommendedMaxWorkingSetSize` and is non-zero. **Reachable since b10618..b10797**: upstream `8c0b9cd04` ("metal : fix memory query under low-memory conditions", [#27701](https://github.com/ggml-org/llama.cpp/pull/27701)) changed `ggml-metal-device.m` to `*free = *total > cur ? *total - cur : 0`; before that clamp an over-committed device (`currentAllocatedSize > recommendedMaxWorkingSetSize`) *underflowed* to a huge `size_t`, which normalised fine, so the same precondition was harmless. That is why the `Java Tests macOS …` jobs went red at the b10792→b10797 step while every Linux/Windows job stayed green — **and why only a GPU build can fail this way at all**: `act_gpu_layers` is `devices.empty() ? 0 : …`, so with no GPU backend `devices` is empty, every layer returns early on `cpu_dev`, and the `.at()` line is unreachable. **Shape:** the two blocks are lifted out of `load_tensors` into free functions declared in `src/llama-model.h`, purely so they can be driven by a test — the failing state needs a real over-committed GPU and cannot be arranged through any public API. `llama_model_splits_normalize()` carries **the fix**: on `split_sum == 0` it `LLAMA_LOG_WARN`s and falls back to an even split (`splits[i] = float(i+1)/splits.size()`), the only neutral choice when no device can be preferred and exactly right for a single device. `llama_model_splits_select_device()` carries **the diagnostic**: it bounds-checks the index and throws a `std::runtime_error` naming the function, the offloaded layer, the device index, the split-point count **and the split points themselves** — with NaN splits that message prints `nan` and names the cause outright, which is precisely what was missing when this had to be diagnosed by reading source. **A second, backend-independent trigger reaches the same line**, found while writing this up and verified against the unfixed library: `--tensor-split` values are parsed with `std::stof` and never range-checked (`common/arg.cpp`), so `-ts 1,-1` cancels out, `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 just as much as on Metal, with no memory pressure involved. That is what makes this an ordinary upstream defect rather than a Metal edge case, and the warning names both causes rather than only the memory one. Also adds upstream `tests/test-model-split.cpp` (5 cases in upstream's `testing.h` style) + its `llama_build_and_test` registration. Touches `src/llama-model.{cpp,h}`, `tests/test-model-split.cpp` and `tests/CMakeLists.txt` — **none** of which any other patch touches, so it is independent of all of them. Upstream-submittable ("model: fall back to an even split when no device reports free memory"); **not yet filed upstream**. **Runnable guard: `src/test/cpp/test_model_split.cpp`** — a FetchContent subproject builds with `LLAMA_BUILD_TESTS=OFF`, so the upstream test above is applied-but-never-compiled here (same as `0001`'s test). That file drives the same two functions from `jllama_test`, which runs on **every** platform in `C++ Tests`, so a bump that drops this patch fails the build at link time everywhere instead of surfacing as one red macOS Java job. **Verification limit — read before assuming this can be dropped:** the *failing path* still cannot be reached without a GPU backend, so the guard pins the arithmetic (what actually broke), not the end-to-end load; the end-to-end proof is the macOS CI job. On a bump, re-check whether upstream added its own `split_sum == 0` guard (grep `split_sum` in `src/llama-model.cpp`) and **drop this patch rather than refreshing it** if they did — the fail-loud applier detects "does not apply", never "upstream already fixed this". | | `0014-common-log-callback-sink.patch` | **Gives `common_log` a callback sink: `common_log_set_callback(log, cb, user_data)` (`common/log.{h,cpp}`).** This is what `LlamaModel.setLogger` hooks. Before it, the Java logger was a `llama_log_set()` callback, which has two holes, both found while chasing `slot print_timing` lines interleaving with the Atmosphere agent's streamed answer: **(a)** every model load runs `common_init()`, which re-points `llama_log_set()` at `common_log_default_callback` (`common.cpp:394`), so `setLogger(…)` *before* `new LlamaModel(…)` silently lost the callback; **(b)** the server's own `SRV_*`/`SLT_*` macros are `LOG_INF` and write straight into `common_log`, which `llama_log_set()` never carried, so the per-request `slot …`/`srv …` lines could not be routed to Java at all (the reason `LlamaModelTest#testLogText/JSON` sat `@Disabled` for years). `common_log` upstream offers file, colors, prefix, timestamps, verbosity and JSONL but no hook. The patch adds one: while a callback is set, the worker thread hands every entry to it **instead of** printing to stdout/stderr (a `--log-file` still receives them); the callback gets the bare formatted message (no prefix/timestamp/colors) with the `ggml_log_callback` signature; swapping the callback pauses the worker first, so queued entries reach the *previous* sink (which is what makes `setLogger(format, null)` a synchronous drain). With the sink, `common_init()`'s `llama_log_set()` reset is harmless — it points at the default callback that feeds `common_log`, i.e. exactly the path into the sink — so the ordering problem (a) disappears without any re-install logic, and the `srv`/`slot` lines (b) arrive because they are `common_log` entries. `jllama.cpp`'s `setLogger` therefore sets `common_log_set_callback(common_log_main(), trampoline)` **plus** `llama_log_set(common_log_default_callback)` (so llama/ggml lines feed `common_log` even before the first load). Two consequences to know: the callback runs on `common_log`'s **worker thread**, a plain `std::thread` llama.cpp re-creates on every pause/resume and never attaches to the JVM — the trampoline attaches per call and detaches again (`get_jni_env_attaching`; a thread that exits while attached leaks a `JavaThread`, and this thread is not ours — the leak-free choice, not the cheapest: each attach creates a `java.lang.Thread` object, so a `thread_local` guard that detaches once at thread exit is the optimisation on file in `TODO.md`), `setLogger` must call `common_log_set_callback` **outside** `g_log_mutex`, because the pause joins the worker, which needs that mutex to read the callback, and `setLogger` callers are serialized by a **separate** `g_set_logger_mutex`: two unserialized swaps race on the worker's `std::thread` (one joins it while the other assigns a fresh thread over the still-joinable object = `std::terminate`, the whole JVM), which `LlamaLoggerTest#concurrentSetLoggerCallsDoNotRaceOnTheLogWorker` reproduced before the mutex existed. Two caveats the Javadoc carries: a caller must not hold a lock the *previous* callback needs (the drain runs it on the worker while the caller waits), and the verbosity threshold is process-wide and reset by **every** load (`common_params_parse` ends with `common_log_set_verbosity_thold(params.verbosity)`, default 3), so a load without `-lv` puts it back to 3 — a review assumed the opposite, and `LlamaLoggerTest#verbosityThresholdIsProcessWideAndEveryLoadSetsIt` now pins the measured behaviour. And the verbosity threshold applies *before* the sink: at the default (`3`) the Java logger sees errors, warnings and the server's INFO lines, while llama/ggml INFO lines (`common_log_get_verbosity` maps them to TRACE = 4) arrive only from `setLogVerbosity(4)` on — the same filtering the console gets, and a behaviour change for consumers who captured the unfiltered `llama_log_set()` stream before. **Runnable guards:** `src/test/cpp/test_common_log_callback.cpp` (6 tests over a private `common_log_init()` instance: delivery, bare text under prefix+timestamps, clear, swap-drains-to-old-sink, file kept, levels pass through) links the function on every platform, so a bump that drops the patch reds `C++ Tests` at link time; `LlamaLoggerTest` (model-free, needs only `libjllama`: a logger set before a deliberately failing load on a non-GGUF file sees the `srv … loading model` INFO line and llama's ERROR line, in TEXT and JSON) and the re-enabled `LlamaModelTest#testLogText/testLogJSON` plus `#testLoggerSetBeforeLoadSurvivesTheLoad` (vocab-only load) cover the Java side. Upstream-submittable ("common : add a callback sink to common_log for embedding hosts"); **not yet filed upstream**. Touches only `common/log.{h,cpp}`, which no other patch touches. **On a bump, check whether upstream added a hook of its own (grep `callback` in `common/log.h`) and, if so, DROP this patch and port `setLogger` to theirs rather than refreshing it.** | +**`0010` was dropped at the b11080 bump.** Upstream merged +[ggml-org/llama.cpp#28518](https://github.com/ggml-org/llama.cpp/pull/28518) +("json: Fixed json enum handling", first tagged at **b11080**), which fixes the defect at its +root instead of at the emit site: `common_json_value` gains an `std::is_enum`-gated constructor +that delegates to `std::underlying_type`, and `common_json_is_value` now accepts enums. So an +unscoped enum no longer binds to `common_json_value(bool)` anywhere, and the one-line `(int)` +cast `0010` added to upstream's `get_res_model_info()` became a **redundant carry**. + +**This is the drop-check firing, and it is the case that makes the check worth running by hand.** +`0010` still applied cleanly at b11080 — upstream never touched the emit site — so the fail-loud +applier said nothing, exactly as `CLAUDE.md` warned it could not ("the applier detects 'does not +apply', never 'upstream already fixed this'"). The standing check is phrased as *"did upstream +cast the value themselves?"*; the honest reading is *"is the defect still there?"*, and it was +not. Dropped, not refreshed, per the `0009`/`0011`/`0013` precedent. + +**The runnable guard stays, re-pointed** (the `0011` precedent again): the `CommonJsonEnumTrap` +pair in `src/test/cpp/test_json_helpers.cpp` is now the `CommonJsonEnum` trio, and it pins +upstream's contract — an **uncast** enum serialises as its numeric value, an explicit +`static_cast` is equivalent, and a real `bool` is still a boolean (the new constructor sits +next to `common_json_value(bool)`, so that one is worth pinning too). A future bump that loses +the enum constructor reds `C++ Tests` on every platform, and the response is to reinstate both +the cast and the patch. `jllama.cpp` keeps the explicit casts at its own two `"vocab_type"` +sites: they are correct either way and survive such a revert. + +`.github/verify-patches-applied.sh` lost its third check with the patch — `0010` was the only +patch with no runnable guard, which is what that check existed for. The script keeps its two +generic assertions (every patch on disk is in the stamp; the patched tree is dirty), so it still +catches an applier that never ran or a tree reverted after the fact. + **`0011` was dropped at the b11069 bump.** Upstream merged [ggml-org/llama.cpp#29161](https://github.com/ggml-org/llama.cpp/pull/29161) ("common/peg : handle invalid utf-8 sequences in the AST", commit `3d82ef62d`, first tagged at @@ -1304,7 +1332,7 @@ If the local check passes (`BUILD SUCCESS`), the `mvn package` job in - `json_helpers.hpp` — Pure JSON transformation helpers (no JNI, no llama state). Independently unit-testable. - `jni_helpers.hpp` — JNI bridge helpers (handle management + server orchestration). Includes `json_helpers.hpp`. - **The `json` alias is upstream's `common_json`, not `nlohmann::ordered_json` (since llama.cpp b10585, upstream #27511).** `tools/server/server-common.h` now says `using json = common_json;` — a deliberately small pimpl wrapper (`common/json.{h,cpp}`, compiled into `llama-common`) around the vendored nlohmann copy. Two traps this cost the project once, both of which **compile silently**: - 1. **An unscoped enum becomes a JSON boolean.** `common_json_value`'s integral constructor template is `std::is_integral`-gated, which excludes enums, so an enum binds to `common_json_value(bool)`. Always `static_cast(...)` an enum before putting it in JSON — `jllama.cpp`'s two `"vocab_type"` sites do, and `patches/0010` does the same for upstream's own `/models` handler. Guards: `test_json_helpers.cpp`'s `CommonJsonEnumTrap` pair pins the mechanism and **does** run in CI; `LlamaModelTest`'s `isIntegralNumber()` assertion pins the real wire value and now runs in CI too (the model-gated suite no longer self-skips — see "CI model policy" below). + 1. **An unscoped enum became a JSON boolean — fixed upstream at b11080, and still worth knowing.** From b10585 to b11069, `common_json_value`'s integral constructor template was `std::is_integral`-gated, which excludes enums, so an enum bound to `common_json_value(bool)` and serialised as `true`/`false`. Upstream [#28518](https://github.com/ggml-org/llama.cpp/pull/28518) added an `std::is_enum`-gated constructor delegating to the underlying type, which retired `patches/0010` (see the drop note under the patches table). **Keep casting anyway** — `jllama.cpp`'s two `"vocab_type"` sites still `static_cast(...)`, which is equivalent under the fix and immune to a revert of it. Guards, both in CI: `test_json_helpers.cpp`'s `CommonJsonEnum` trio pins the mechanism (uncast enum is numeric, explicit cast equivalent, `bool` still boolean); `LlamaModelTest`'s `isIntegralNumber()` assertion pins the real wire value (the model-gated suite no longer self-skips — see "CI model policy" below). 2. **`common_json` converts to `std::string` implicitly**, so it binds happily to a `const nlohmann::json &` parameter (via nlohmann's string-constructible converting constructor) and then throws `json::type_error 302` at runtime. Never declare a project helper as taking `nlohmann::json` when callers pass the `json` alias — `require_json_field_impl` is a template for exactly this reason. Other differences to know: no `get_ref`/`array_t`/`type_name()`; a braced list in *value* position does not build an array (write `json::array({...})`); `at(key)` needs an explicit `.get()`; errors are `common_json_error`; and `get()` is limited to the types explicitly specialised in `common/json.cpp`. `log_helpers.hpp` and `train_engine.cpp` keep their own `nlohmann::json` alias — they never touch the server's `json`. - Uses `nlohmann/json` for JSON deserialization of parameters in the two files named above; everything on the server path uses `common_json`. @@ -1613,7 +1641,7 @@ ctest --test-dir build --output-on-failure -R "ResultsToJson" |------|-------|-------| | `src/test/cpp/test_utils.cpp` | 168 | Upstream helpers: `server_tokens`, `server_grammar_trigger`, `gen_tool_call_id`, `json_value`, `json_get_nested_values`, UTF-8 helpers, `format_response_rerank`, `format_embeddings_response_oaicompat`, `oaicompat_completion_params_parse`, `oaicompat_chat_params_parse`, `are_lora_equal`, `strip_flag_from_argv`, `token_piece_value`, `json_is_array_and_contains_numbers`, `format_oai_sse`, `format_oai_resp_sse`, `format_anthropic_sse`, `parse_lora_request`, `common_chat_parse` over malformed UTF-8 (the `ContentOnlyParseUtf8` guard, which pins upstream #29161's one-U+FFFD-per-invalid-run contract — formerly the guard for the dropped `patches/0011`) | | `src/test/cpp/test_server.cpp` | 206 | Upstream result types: `server_slot_stats` (the `timings` JSON payload; replaced `result_timings` in b10408), `task_params::to_json()` (incl. `dry_sequence_breakers`, `preserved_tokens`, `timings_per_token`), `completion_token_output`, `server_task_result_cmpl_partial` (non-oaicompat + `to_json_oaicompat` + logprobs + `to_json_oaicompat_chat` + `to_json_anthropic` + dispatcher), `server_task_result_cmpl_final` (non-oaicompat + `to_json_oaicompat` + `to_json_oaicompat_chat` + `to_json_oaicompat_chat_stream` + `to_json_anthropic` + `to_json_anthropic_stream` + tool_calls + dispatcher), `server_task_result_embd`, `server_task_result_rerank`, `server_task_result_metrics` (`to_metrics()` = the `/metrics` Prometheus exposition text; its `to_json()` has been unused since b10519 and returns `json{}` = JSON null), `server_task_result_slots` (`to_json()` = the `/slots` array, fed by the b10519 `SERVER_TASK_TYPE_SLOT_GET` task), `server_task_result_slot_save_load`, `server_task_result_slot_erase`, `server_task_result_apply_lora`, `server_task_result_get_lora`, `server_task_result_error`, `format_error_response`, `server_task::need_sampling()`, `server_task::n_tokens()`, `server_schema::eval_llama_cmpl_schema()` (parsing pipeline + grammar routing + error paths + per-request `dry_*` and `sse_ping_interval` field round-trips incl. hard-limit + server-default inheritance), `response_fields` projection | -| `src/test/cpp/test_json_helpers.cpp` | 63 | All functions in `json_helpers.hpp`: `get_result_error_message`, `results_to_json`, `rerank_results_to_json` (incl. missing/out-of-range `index` rejection), `parse_encoding_format`, `extract_embedding_prompt`, `is_infill_request`, `parse_slot_prompt_similarity`, `parse_positive_int_config`, `wrap_stream_chunk`, `server_metrics_to_json` | +| `src/test/cpp/test_json_helpers.cpp` | 64 | All functions in `json_helpers.hpp`: `get_result_error_message`, `results_to_json`, `rerank_results_to_json` (incl. missing/out-of-range `index` rejection), `parse_encoding_format`, `extract_embedding_prompt`, `is_infill_request`, `parse_slot_prompt_similarity`, `parse_positive_int_config`, `wrap_stream_chunk`, `server_metrics_to_json` | | `src/test/cpp/test_log_helpers.cpp` | 13 | All functions in `log_helpers.hpp`: `log_level_name`, `format_log_as_json` | | `src/test/cpp/test_common_log_callback.cpp` | 6 | **The runnable guard for `patches/0014`**: `common_log_set_callback()` on a private `common_log_init()` instance (never `common_log_main()`, so the process-wide logger the other tests print through is untouched) — delivery of level + bare text, no prefix/timestamp even when both are on (what `common_init()` does), clearing stops delivery, a swap drains queued entries to the *previous* sink (the property behind `LlamaModel.setLogger(format, null)` being a synchronous flush), a `--log-file` keeps being written alongside the sink, and every `ggml_log_level` passes through unchanged. The Java half (`LlamaLoggerTest`, model-free) proves the JNI trampoline on top of it. | | `src/test/cpp/test_jni_helpers.cpp` | 70 | All functions in `jni_helpers.hpp` using a zero-filled `JNINativeInterface_` mock (incl. the `utf8_to_jstring_impl` byte-array string path: emoji byte-preservation, truncated-UTF-8 replace-not-throw). Seven of them pin `jni_guard_impl` — the JNI exception boundary every `Java_*` entry point runs inside — including the `catch (...)` arm that is the only backstop for a non-`std::exception` type, and its two refusals (never `ThrowNew` over a pending Java exception, never with a null class). | @@ -1623,11 +1651,11 @@ ctest --test-dir build --output-on-failure -R "ResultsToJson" | `src/test/cpp/test_model_flags.cpp` | 4 | **The contract between the Java CLI-flag registries and llama.cpp's server argument parser.** CMake reads `ModelFlag.java` + `ModelOption.java` (`cmake/extract-java-wire-names.cmake` → a generated header of `{name, contract}` pairs), and this file asserts every `SERVER_PARSER` name 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, and it declares itself `CliContract.PROJECT_PSEUDO` on its own constant rather than appearing in a list inside this file; the test asserts such a name is **still unknown** to the parser (an exemption upstream later registers would be hiding a real check) and that the exempt set is non-empty. | | `src/test/cpp/test_wire_contracts.cpp` | 6 | **The same contract for the two quieter surfaces.** `RequestField` against `server_schema::make_llama_cmpl_schema(...)` (5 tests) and `TrainingField` against `jllama_train::config_keys()` (1 test). Both receivers *silently ignore* an unknown key — the schema skips it, `train_engine.cpp` reads with `j.value(key, default)` and falls back — so a dead field produces no error anywhere and every string-mapping test keeps passing. `OAI_LAYER`-declared keys (consumed by `oaicompat_*_params_parse` before the schema) are exempt from the schema check, and are checked **both** ways: still unknown to the schema (the inverted check), and read by at least one upstream reader-shaped site (the configure-time sweep — this is what caught `chat_template`, a key a public builder wrote and nothing read). See [`docs/history/parameter-wire-surface.md`](docs/history/parameter-wire-surface.md). | -**Current total: 558 tests (all passing).** +**Current total: 559 tests (all passing).** #### Upstream source location (in CMake build tree) -llama.cpp is fetched via CMake FetchContent, pinned to `GIT_TAG b11069`. +llama.cpp is fetched via CMake FetchContent, pinned to `GIT_TAG b11080`. **GoogleTest** is a separate `BUILD_TESTING`-only FetchContent (`GIT_TAG v1.18.0`), used solely by the `jllama_test` C++ unit-test binary — not by the shipped library, and not coupled to the diff --git a/README.md b/README.md index 9ac10627..83a2f8c1 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ **Build:** ![Java 8+](https://img.shields.io/badge/Java-8%2B-informational) ![Platform](https://img.shields.io/badge/Platform-Linux%20%7C%20macOS%20%7C%20Windows%20%7C%20Android-lightgrey) -[![llama.cpp b11069](https://img.shields.io/badge/llama.cpp-%23b11069-informational)](https://github.com/ggml-org/llama.cpp/releases/tag/b11069) +[![llama.cpp b11080](https://img.shields.io/badge/llama.cpp-%23b11080-informational)](https://github.com/ggml-org/llama.cpp/releases/tag/b11080) [![JPMS](https://img.shields.io/badge/JPMS-modular%20JAR-25A162)](https://openjdk.org/projects/jigsaw/) ![JUnit](https://img.shields.io/badge/tested%20with-JUnit6-25A162) [![JSpecify](https://img.shields.io/badge/JSpecify-1.0.0%20%40NullMarked-25A162)](https://jspecify.dev) diff --git a/TODO.md b/TODO.md index dc46a9a8..c16871b5 100644 --- a/TODO.md +++ b/TODO.md @@ -139,7 +139,7 @@ workflow in `.github/workflows/`). It contributes to the `mergeable_state: block ### Upstream PR submissions — drop the carried patches (open) -There are **nine** patches today (`0001`–`0003`, `0006`–`0008`, `0010`–`0012`). **Eight are +There are **eight** patches today (`0001`–`0003`, `0006`–`0008`, `0012`, `0014`). **Seven are upstream-submittable verbatim**; each accepted PR (once the pin is bumped past it) deletes a patch from the bump checklist. The exception is **`0003`**, a carry of upstream PR #22393, which upstream **closed without merging** — it is permanent and will never be droppable via a bump. (`0003` used to @@ -158,13 +158,13 @@ be described here as "drops automatically when that merges"; it will not.) - **`0007` `llama_server_attach`** (HTTP frontend on an existing `server_context`). - **`0008` `LLAMA_SERVER_WORKER_CMD` router worker override** (also useful for containerized/wrapped deployments). -- **`0010` cast `vocab_type` for `common_json`** (one line; upstream regressed `GET /models` + - `GET /v1/models` to emit `true`/`false` instead of the numeric vocab type when they flipped the - `json` alias to `common_json` at b10585/#27511). **Not yet filed upstream.** - **`0012` guard the zero split-sum and name the device index** (a GPU reporting zero free memory — or a cancelling `--tensor-split` such as `-ts 1,-1` on any backend — makes every model load fail with the unactionable `error loading model: vector`). Ships an upstream `tests/test-model-split.cpp`. **Not yet filed upstream.** +- **`0014` add a callback sink to `common_log`** (`common_log_set_callback`, what `LlamaModel.setLogger` + hooks; upstream has file/colors/prefix/timestamps/verbosity/JSONL but no hook, so an embedding host + cannot route the server's own `SRV_*`/`SLT_*` lines anywhere). **Not yet filed upstream.** (`0009` is **not** in this list and the number is burned: upstream merged the subprocess.h fix via ggml-org/llama.cpp#26606, so the patch was dropped at the b10280 bump. `0013` is likewise gone — @@ -172,7 +172,10 @@ upstream merged this project's own PR ggml-org/llama.cpp#28775 and it was droppe went the same way at b11069: upstream fixed the invalid-UTF-8 PEG-parser failure independently and more broadly via ggml-org/llama.cpp#29161 (one U+FFFD per undecodable run, text after it kept) before the patch was ever filed, so the `ContentOnlyParseUtf8` guard now pins upstream's contract instead. -All three drops are recorded in `CLAUDE.md` under the patch table.) +`0010` followed at b11080: upstream gave `common_json_value` an enum constructor via +ggml-org/llama.cpp#28518, fixing at the root the enum-to-bool trap the patch cast around, so it became +a redundant carry — note that it still *applied* cleanly, which is why the by-hand drop-check exists. +All four drops are recorded in `CLAUDE.md` under the patch table.) ### llama.cpp upstream feature exposure (queued, deferred by policy) @@ -340,20 +343,9 @@ and have only run locally so far. A mutation pass over the branch applied 27 mutations and 26 went red on the test that claims them, so no test here passes with its subject deleted. What it did find is code with **no runnable guard**. Two of the three were closed in that PR (a model-free `jsonSchemaToGrammar` test in -`NativeLibraryLoadSmokeTest`, and `IdleSleepWakeIntegrationTest` for the `wake_and_post` path); -these are what remains. - -- **`patches/0010` has no guard that runs on a model-free host.** Reverting the patch's - `(int)` cast in the fetched `tools/server/server-context.cpp` leaves `ctest` at a clean **520/520** — - the always-run `C++ Tests` job cannot see the regression at all. The only guard is - `NativeServerAttachIntegrationTest.models_reportNumericVocabType`, which is model-gated; it *does* - run on all six CI Java jobs (the full model set is downloaded there), so this is a coverage gap - rather than a shipping risk today. It becomes one the moment a platform stops downloading models. - `CommonJsonEnumTrap` in `test_json_helpers.cpp` cannot help — it builds its own JSON literals and - calls no project code. A direct unit test is impossible as things stand: `get_res_model_info` is - `static` inside `server-context.cpp` and unreachable from `jllama_test`. Cheapest real fix is a - CI assertion in the `C++ Tests` job that the patch is present in the fetched tree - (`grep -c '(int) meta.model_vocab_type'` plus a non-empty `git -C _deps/llama.cpp-src diff`). +`NativeLibraryLoadSmokeTest`, and `IdleSleepWakeIntegrationTest` for the `wake_and_post` path); the +third was `patches/0010`'s `(int)` cast, reachable only from a model-gated Java test, and it went +away with the patch itself at the b11080 bump. This is what remains. - **`TestConstantsTest.theShippedModelConstantsGoThroughTheResolver` is vacuous when the fixture is absent.** Mutating `MODEL_PATH = resolveModelPath("models/…")` to the bare literal leaves the test diff --git a/docs/history/llama-cpp-breaking-changes.md b/docs/history/llama-cpp-breaking-changes.md index eba1e6da..807d7ff6 100644 --- a/docs/history/llama-cpp-breaking-changes.md +++ b/docs/history/llama-cpp-breaking-changes.md @@ -742,3 +742,5 @@ Used during `llama.cpp` version bumps: when upgrading, scan this file from the r | b11018–b11062 | patches + upstream verification | **Nine patches still, none dropped — but two needed a refresh, the first in several ranges.** One upstream commit is responsible: **#29125** ("server : improve startup log messages", first tagged b11053) adds an `SRV_INF("initializing ...")` line immediately above `llama_server()`'s argv parse and a two-line `TODO` comment above `common_params_parse()` in `common/arg.h`. `0001` anchors hunks on both spots and `0006` replaces the very line `0001` flips, so both went stale **on context only** — the refresh changes `@@` line numbers, three context lines and the index blob hashes, and not one added or removed line. Replayed in filename order against pristine **b11055 and b11062**: all nine apply clean at both. **`0007`'s standing invariant is intact and provably so** — its `-` side is a verbatim copy of the route table it factors out of `llama_server()`, so a clean apply *is* the proof upstream did not touch that block; #29125's edits sit above it (the CORS warning) and below it (the `warn_names` loop), never inside. **The three mechanical `tools/server/` contract greps have no input** despite `tools/server/` being touched: `server-schema.cpp`, `server-task.cpp` and `server-context.cpp` are byte-identical b11018→b11062, verified by blob hash rather than by reading a diff, so the request-field set, the field bounds and the response-key set cannot have moved. **All standing drop-checks still say "still required"**, run against the pristine tag because the fail-loud applier detects "does not apply" but never "upstream already fixed this": `0001` (`common_params_parse_main` 0 occurrences in `b11062:common/arg.h`, WIN32 override still at `common/arg.cpp:1282`), `0002` (`params_base.load_progress_callback = load_progress_callback` still unguarded at `server-context.cpp:1095`), `0010` (`{"vocab_type", meta.model_vocab_type}` still uncast at `server-context.cpp:4554`), `0011` (the `common_peg_until_parser` `INVALID` branch still returns `FAIL` unconditionally, ignoring `ctx.is_lenient()`, while the `INCOMPLETE` branch right above it honours it), `0012` (bare `splits[i] /= split_sum` at `llama-model.cpp:1518`, no zero guard), and `0003`/`0006`/`0007`/`0008` absent upstream. **Verified at the target from a fresh configure**: stamp head `3cf03257f` with nine SHA-256 lines, `verify-patches-applied.sh` green (9 applied, 0010 cast present), extraction unchanged at 138 CLI / 57 request / 15 trainer names, Release build clean (0 errors, 0 warnings), `ctest` 551/551, `nm -D` 40 `Java_*` exports and 0 mangled, `NativeLibraryLoadSmokeTest` 4/4 with 0 skipped after a `mvn clean`, `mvn test` 1763/0 (269 model-gated skips in a HF-blocked sandbox), `verify-test-counts.sh` 1763 across 119 classes, SpotBugs 0, spotless clean. **The OpenVINO SDK pin moved with it**: #29009 takes upstream's own `OPENVINO_VERSION_MAJOR`/`OPENVINO_VERSION_FULL` to 2026.4, and this project's two OpenVINO classifier jobs — which had drifted two releases behind at 2026.2.1 — now install `2026.4` / `2026.4.0.22959.99c81491cc3` from the same URL template upstream's `{linux,windows}-setup-openvino` actions use. ggml-openvino is developed against whatever pair upstream pins, so tracking it is the cheaper end of the trade: a lagging pin does not fail on the bump that introduces the drift, it fails on some later one, in a job whose runner has no Intel GPU to reproduce on. **Not verifiable from the bump sandbox** — `storage.openvinotoolkit.org` is blocked by the network policy, so neither archive URL could be HEAD-checked here; the evidence they resolve is that upstream's own release jobs download exactly these two URLs at b11062. Per the classifier policy the step is fail-loud, so a wrong URL reds the job rather than shipping a backend-less jar. Both jobs now carry a keep-in-sync note naming upstream's two variables as the source of truth, so the next bump has somewhere to look instead of rediscovering the coupling. | | b11062–b11069 | Seven commits, 21 files, **57 KiB** (`tools/ui` untouched, so the WebUI-excluded figure is the same). One touches a priority-list neighbourhood and a patch target: **#29161** ("common/peg : handle invalid utf-8 sequences in the AST", first tagged **b11063**) rewrites `common/peg-parser.{cpp,h}`, `common/unicode.{cpp,h}`, `common/chat-peg-parser.cpp` and the two upstream tests. Everything else is backend-internal: CUDA (#28912 MMVQ→MMQ crossover for SM70, #29152 FA tuning for Gemma 4 on Ampere+), Metal (#29169 arbitrary `hc` in `dsv4_hc_pre`, #29136 macOS 27 SDK deprecation warnings — warnings only, no API), WebGPU (#28976 fused GDN + cpy; not a backend this project builds), and the Python converter (#29203, not compiled). | **No project-source change, but one patch dropped.** `common/peg-parser.h` grows additively — a new `common_peg_invalid_utf8 {pos, len}` record, an `invalid_utf8` vector plus `sanitized_text()` on `common_peg_ast_node`, and a defaulted trailing parameter on `common_peg_ast_arena::add_node` and the four-argument `common_peg_parse_result` constructor — and it is on the *safe-to-skip* list anyway (nothing in `src/main/cpp` names a PEG type; `jllama.cpp` reaches the parser only through `common_chat_parse`). `common/unicode.h` changes one *comment*: `utf8_parse_result::bytes_consumed` now carries the valid-prefix length on `INVALID`/`INCOMPLETE` results instead of `0`. Neither `common/chat.h` nor any other priority row moves. **`tools/server/` is untouched** (no file in the range), so the three mechanical contract greps have no input. **`ggml/include` is byte-identical.** The behavioural change is the one `patches/0011` existed for — see the patch row below. | | b11062–b11069 | patches + upstream verification | **Eight patches now: `0011` dropped, the other eight apply unchanged.** #29161 deleted the very `INVALID` branch of `common_peg_until_parser` that `0011` patched: the until-parser now consumes an undecodable run in every mode (strict included), records it on the result, and `common_chat_peg_mapper` renders the node through `sanitized_text()` — one U+FFFD per run, per the Unicode "maximal subpart" rule (`\xE4\xB8` + `c` → one replacement, `\xFF\xFE` → two), with the text after the run kept. `0011` had returned only the text *before* the byte. The lenient incomplete-at-end branch is unchanged (trailing bytes withheld). So the applier failed loud — `patch failed: common/peg-parser.cpp:680` — exactly as designed, and the patch was **dropped, not refreshed**, per the `0009`/`0013` precedent; it had never been filed upstream, so nothing to close. Its runnable guard was kept and re-pointed: `ContentOnlyParseUtf8` in `src/test/cpp/test_utils.cpp` now pins upstream's replacement contract (six cases, one more than before, the extra one pinning the run boundary `\xFF\xFE` → two U+FFFD), so a future upstream revert to `FAIL` still reds `C++ Tests` everywhere. **Replayed in filename order against pristine b11069**: `0001` `0002` `0003` `0006` `0007` `0008` `0010` `0012` apply clean, `0011` is the only failure. **All standing drop-checks still say "still required"** at the pristine tag: `0001` (`common_params_parse_main` 0 occurrences in `b11069:common/arg.h`; the count-guarded `argv = utf8.ptrs.data()` override still at `common/arg.cpp:1281` — i.e. [ggml-org/llama.cpp#26416](https://github.com/ggml-org/llama.cpp/issues/26416) remains open upstream and is **not** what this range fixed), `0002` (`params_base.load_progress_callback = load_progress_callback` still unguarded at `server-context.cpp:1095`), `0010` (`{"vocab_type", meta.model_vocab_type}` still uncast at `server-context.cpp:4554`), `0012` (bare `splits[i] /= split_sum` at `llama-model.cpp:1518`), and `0003`/`0006`/`0007`/`0008` absent upstream. `.github/verify-patches-applied.sh`'s header comment no longer lists `0011`. **Verified at the target from a fresh configure** (`rm -rf build && cmake -B build -DBUILD_TESTING=ON`, the real `FetchContent` path): stamp head `68d9053af` with **eight** SHA-256 lines, `verify-patches-applied.sh` green (8 applied, tree dirty, 0010 cast present), extraction unchanged at 138 CLI / 57 request / 15 trainer names, Release build clean, `ctest` **552/552** (551 → 552: the re-pointed guard gained one case), `nm -D` 40 `Java_*` exports and 0 mangled, `NativeLibraryLoadSmokeTest` **4/4, 0 skipped** after a `mvn clean` — `nativeBuildInfoMatchesPinnedVersionConstant` confirms `LlamaCppVersion.LLAMA_CPP_VERSION` (`b11069`) against the linked `build-info`. `test_utils.cpp` is clean under the CI-pinned clang-format 23.1.1; `spotless:check` clean. Model-backed Java tests were not run (HF-blocked sandbox); `NativeServerAttachIntegrationTest.completion_overHttp_served`, the test that first surfaced the `0011` failure, is the CI-side confirmation for this drop. | +| b11069–b11080 | Eleven commits, 72 files, **1244 KiB** (`tools/ui` untouched, so the WebUI-excluded figure is the same). The size is almost entirely one commit: **#29197** ("hexagon: overhaul of buffer and DMA handling to support 64bit mappings") rewrites 46 files under `ggml/src/ggml-hexagon/` for +7085/−6108 on its own, and this project builds no hexagon classifier. The rest of `ggml/src` is three self-contained backend changes — `ggml-cpu` (**#23492**, ARM repack kernels for Q1_0, +762/−0, purely additive), `ggml-metal` (**#29206**, fusion-pattern op list simplification) and `ggml-sycl` (**#28918**, MKL-FA softmax load coalescing). **`ggml/include` is byte-identical**, so no public ggml header moved. Outside ggml: `.github/workflows` (**#28991**, upstream's own self-hosted CI refactor — not consumed here), `scripts/snapdragon`, `tests/test-backend-ops.cpp` (**#29204**, regex `-o` filter), `docs/backend/snapdragon`, and three tool READMEs regenerated for the new env vars. **One priority-list file changed and it is the consequential one: `common/json.h`** (**#28518**, "json: Fixed json enum handling", +4/−0) — see the patch row below. `common/arg.cpp` gains six `set_env(...)` calls (**#27380**: `LLAMA_ARG_TEMPERATURE` / `_TOP_P` / `_MIN_P` / `_REPEAT_PENALTY` / `_PRESENCE_PENALTY` / `_FREQUENCY_PENALTY`) on existing options — additive, no option added or removed, so the `ModelFlag`/`ModelOption` contract is untouched; note only that those six now read an environment default when the flag is absent, which an embedding host with those vars set would inherit. `tools/server/` moves by exactly one functional line (**#28938**: `unset_reserved_args` also unsets `LLAMA_ARG_API_KEY_FILE`, so a router no longer forwards it to spawned children) plus its README. | **No project-source change; one patch dropped.** The three mechanical server-contract greps have real input this time (`tools/server/` is in the range) and all three come back **identical**: the request-field set is 68 names, the bounded-field set 23, and the response-key set 142, unchanged between the two tags — `server-schema.cpp`, `server-task.cpp`, `server-context.cpp` and `server-common.h` are all byte-unchanged, so a contract change behind a stable signature is ruled out by construction rather than by reading. Wire-name extraction re-ran against b11080's sources and is unchanged at **138 CLI / 57 request / 15 trainer** names. `common/json.h`'s change is additive (a new constructor overload and one line in a type trait) and cannot break a caller; what it does is retire a patch. | +| b11069–b11080 | patches + upstream verification | **Eight patches now: `0010` dropped, the other eight apply unchanged.** **#28518** gives `common_json_value` an `std::is_enum`-gated constructor that delegates to `std::underlying_type`, and adds `std::is_enum` to `common_json_is_value`. That fixes at its root the trap `0010` worked around at the emit site: an unscoped enum no longer binds to `common_json_value(bool)`, so upstream's own `get_res_model_info()` emits a numeric `vocab_type` with no cast. **This drop is the case the by-hand drop-check exists for, and it is worth recording precisely: `0010` still applied cleanly at b11080** — upstream never touched the emit site — so the fail-loud applier said nothing and a redundant carry would have shipped silently. The standing check is worded "did upstream cast the value themselves?"; the answer was no and the correct verdict was still *drop*, because the defect is gone. Dropped, not refreshed, per the `0009`/`0011`/`0013` precedent; it had never been filed upstream, so nothing to close. **The runnable guard was kept and re-pointed** (the `0011` precedent): the `CommonJsonEnumTrap` pair in `src/test/cpp/test_json_helpers.cpp` is now the `CommonJsonEnum` trio and pins upstream's contract — an uncast enum serialises as its numeric value, an explicit `static_cast` is equivalent, and a real `bool` is still a boolean (the new overload sits next to `common_json_value(bool)`, so that one is worth pinning too). A bump that loses the enum constructor therefore reds `C++ Tests` on every platform, and the response is to reinstate both the cast and the patch; `jllama.cpp` keeps its own two `"vocab_type"` casts, which are correct either way. `.github/verify-patches-applied.sh` lost its third check with the patch — `0010` was the only patch with no runnable guard, which is precisely what that check was for — and keeps its two generic assertions (every patch on disk is in the stamp; the patched tree is dirty); the matching `TODO.md` coverage-gap entry is resolved and removed. **Replayed in filename order against pristine b11080**: all nine of the previous set apply clean, `0010` included — which is the point. **The remaining four standing drop-checks all say "still required"** at the pristine tag: `0001` (`common_params_parse_main` 0 occurrences in `b11080:common/arg.h`; the count-guarded `argv = utf8.ptrs.data()` override still at `common/arg.cpp:1282`, so [ggml-org/llama.cpp#26416](https://github.com/ggml-org/llama.cpp/issues/26416) remains open), `0002` (`params_base.load_progress_callback = load_progress_callback` still unguarded at `server-context.cpp:1095`), `0012` (bare `splits[i] /= split_sum` at `llama-model.cpp:1518`, no zero-sum guard), `0014` (`common_log_set_callback` 0 occurrences in `b11080:common/log.h`); `0003`/`0006`/`0007`/`0008` remain absent upstream. **Only two patch-target files were in the range at all** (`common/arg.cpp` for `0001`, `tools/server/server-models.cpp` for `0008`), both far from the patched hunks, and both proven by replay rather than by reading. **Verified at the target from a fresh configure** (`rm -rf build && cmake -B build -DBUILD_TESTING=ON`, the real `FetchContent` path): stamp head `1d72b05d3` with **eight** SHA-256 lines, `verify-patches-applied.sh` green (8 applied, tree dirty), the fetched `server-context.cpp` confirmed **uncast** at the emit site and `common/json.h` confirmed to carry the `is_enum` overload, Release build clean (0 errors, 0 warnings in project sources), `ctest` **559/559** (558 → 559: the re-pointed guard gained one case), `nm -D` 40 `Java_*` exports, `NativeLibraryLoadSmokeTest` **4/4, 0 skipped** after a `mvn clean` — `nativeBuildInfoMatchesPinnedVersionConstant` confirms `LlamaCppVersion.LLAMA_CPP_VERSION` (`b11080`) against the linked `build-info`. Full `mvn test` **1772 run, 0 failures, 0 errors** (272 skipped — the model-gated classes, no GGUF in this sandbox). `test_json_helpers.cpp` is clean under the CI-pinned clang-format 23.1.1; `spotless:check` clean; SpotBugs **0** findings. Model-backed Java tests were not run (HF-blocked sandbox); `NativeServerAttachIntegrationTest.models_reportNumericVocabType` — whose failure message now names #28518 instead of the retired patch — is the CI-side confirmation that the wire value stayed numeric across this drop. | diff --git a/llama/CMakeLists.txt b/llama/CMakeLists.txt index 240daa92..93cc3421 100644 --- a/llama/CMakeLists.txt +++ b/llama/CMakeLists.txt @@ -173,7 +173,7 @@ set(LLAMA_BUILD_APP OFF CACHE BOOL "" FORCE) FetchContent_Declare( llama.cpp GIT_REPOSITORY https://github.com/ggerganov/llama.cpp.git - GIT_TAG b11069 + GIT_TAG b11080 PATCH_COMMAND ${CMAKE_COMMAND} -DPATCH_DIR=${CMAKE_CURRENT_SOURCE_DIR}/patches -DLLAMA_SRC= diff --git a/llama/patches/0010-server-cast-vocab-type-for-common-json.patch b/llama/patches/0010-server-cast-vocab-type-for-common-json.patch deleted file mode 100644 index 24a8b4f9..00000000 --- a/llama/patches/0010-server-cast-vocab-type-for-common-json.patch +++ /dev/null @@ -1,16 +0,0 @@ -diff --git a/tools/server/server-context.cpp b/tools/server/server-context.cpp -index 15fa8a498..f2fbe2be8 100644 ---- a/tools/server/server-context.cpp -+++ b/tools/server/server-context.cpp -@@ -4439,7 +4439,10 @@ static json get_res_model_info(const server_context_meta & meta) { - {"created", std::time(0)}, - {"owned_by", "llamacpp"}, - {"meta", { -- {"vocab_type", meta.model_vocab_type}, -+ // an unscoped enum has no common_json_value ctor of its own (the integral one is -+ // is_integral-gated, which excludes enums), so it binds to common_json_value(bool) -+ // and serialises as true/false -- cast it to keep the numeric vocab type on the wire -+ {"vocab_type", (int) meta.model_vocab_type}, - {"n_vocab", meta.model_vocab_n_tokens}, - {"n_ctx", meta.slot_n_ctx}, - {"n_ctx_train", meta.model_n_ctx_train}, diff --git a/llama/src/main/java/net/ladenthin/llama/value/LlamaCppVersion.java b/llama/src/main/java/net/ladenthin/llama/value/LlamaCppVersion.java index ea561992..d4e43a04 100644 --- a/llama/src/main/java/net/ladenthin/llama/value/LlamaCppVersion.java +++ b/llama/src/main/java/net/ladenthin/llama/value/LlamaCppVersion.java @@ -10,13 +10,13 @@ * library was compiled against, exposed as a compile-time constant so callers can render a badge or * emit a startup log line without loading the native library. * - *

{@link #LLAMA_CPP_VERSION} is a pure-Java string ({@code "b11069"}) that mirrors the + *

{@link #LLAMA_CPP_VERSION} is a pure-Java string ({@code "b11080"}) that mirrors the * {@code GIT_TAG} in {@code llama/CMakeLists.txt}. It is available even when {@code libjllama} is * absent (pure-Java checkout, before {@code System.load}), which is what makes it suitable for a * lightweight version badge in Android or other UIs.

* *

For the authoritative value that is baked into the native binary — the build number - * plus the resolved upstream commit, e.g. {@code "b11069-"} — call + * plus the resolved upstream commit, e.g. {@code "b11080-"} — call * {@link net.ladenthin.llama.LlamaModel#getLlamaCppBuildInfo()} instead; that reads llama.cpp's own * {@code build-info} through JNI and therefore cannot drift from the compiled library (but requires * the native library to be loaded).

@@ -24,14 +24,14 @@ public final class LlamaCppVersion { /** - * The pinned llama.cpp release tag this library was built against, e.g. {@code "b11069"}. + * The pinned llama.cpp release tag this library was built against, e.g. {@code "b11080"}. * *

Kept in lockstep with {@code GIT_TAG} in {@code llama/CMakeLists.txt} — see the * "Upgrading/Downgrading llama.cpp Version" checklist in {@code CLAUDE.md}. This is the * compile-time pin; use {@link net.ladenthin.llama.LlamaModel#getLlamaCppBuildInfo()} for the * value actually linked into the native binary.

*/ - public static final String LLAMA_CPP_VERSION = "b11069"; + public static final String LLAMA_CPP_VERSION = "b11080"; // Constants holder — not instantiable. private LlamaCppVersion() {} diff --git a/llama/src/test/cpp/test_json_helpers.cpp b/llama/src/test/cpp/test_json_helpers.cpp index d82f47f3..9deb64e0 100644 --- a/llama/src/test/cpp/test_json_helpers.cpp +++ b/llama/src/test/cpp/test_json_helpers.cpp @@ -357,30 +357,47 @@ TEST(ExtractEmbeddingPrompt, ArrayPrompt_ReturnedAsIs) { } // ============================================================ -// common_json enum trap (llama.cpp b10585, upstream #27511) +// common_json enum handling (llama.cpp b11080, upstream #28518) // -// The upstream `json` alias is `common_json`, whose value constructors cover -// bool / integral / floating-point / string / container — but the integral one -// is `std::is_integral`-gated, and an *enum* is not integral. An unscoped enum -// therefore binds to `common_json_value(bool)` and silently serialises as -// true/false. Project code must cast enum values to `int` before putting them -// in JSON; `jllama.cpp`'s two "vocab_type" emit sites do exactly that, and -// `ModelMeta.getVocabType()` reads the result with Jackson's `asInt(0)`. +// The upstream `json` alias is `common_json`. From b10585 (#27511) until +// b11080 its integral value constructor was `std::is_integral`-gated, which +// excludes enums, so an unscoped enum bound to `common_json_value(bool)` and +// silently serialised as true/false. Upstream #28518 fixed that at the root: +// `common_json_value` now has an `std::is_enum`-gated constructor delegating +// to the underlying type, and `common_json_is_value` accepts enums. That is +// what retired `patches/0010`, which used to cast the enum at upstream's own +// `get_res_model_info()` emit site. +// +// These three tests are the re-pointed guard. The first is the tripwire: if +// a future bump loses the enum constructor, an uncast enum goes back to being +// a JSON boolean and `ModelMeta.getVocabType()` (Jackson `asInt(0)`) reports 1 +// for every non-SPM model — reinstate the cast (and `patches/0010`) then. The +// second pins that an explicit `static_cast` is still equivalent, which +// is what `jllama.cpp`'s two "vocab_type" emit sites keep doing. The third +// pins that the new overload did not swallow real booleans on its way in. // ============================================================ -TEST(CommonJsonEnumTrap, UncastEnumBecomesBoolean) { - // documents the trap this guard exists for (tripwire: if upstream ever adds an - // enum constructor, this flips and the cast convention can be revisited) +TEST(CommonJsonEnum, UncastEnumKeepsTheNumericValue) { const json j = json::object({{"vocab_type", LLAMA_VOCAB_TYPE_WPM}}); - EXPECT_TRUE(j.at("vocab_type").is_boolean()); + EXPECT_FALSE(j.at("vocab_type").is_boolean()); + EXPECT_TRUE(j.at("vocab_type").is_number_integer()); + EXPECT_EQ(j.at("vocab_type").get(), static_cast(LLAMA_VOCAB_TYPE_WPM)); } -TEST(CommonJsonEnumTrap, ExplicitIntCastKeepsTheNumericValue) { +TEST(CommonJsonEnum, ExplicitIntCastKeepsTheNumericValue) { const json j = json::object({{"vocab_type", static_cast(LLAMA_VOCAB_TYPE_WPM)}}); EXPECT_TRUE(j.at("vocab_type").is_number_integer()); EXPECT_EQ(j.at("vocab_type").get(), static_cast(LLAMA_VOCAB_TYPE_WPM)); } +TEST(CommonJsonEnum, BoolIsStillABoolean) { + // the enum constructor sits next to common_json_value(bool); make sure it did + // not swallow real booleans on its way in + const json j = json::object({{"stream", true}}); + EXPECT_TRUE(j.at("stream").is_boolean()); + EXPECT_FALSE(j.at("stream").is_number_integer()); +} + // ============================================================ // is_infill_request // ============================================================ diff --git a/llama/src/test/java/net/ladenthin/llama/server/NativeServerAttachIntegrationTest.java b/llama/src/test/java/net/ladenthin/llama/server/NativeServerAttachIntegrationTest.java index 5581825b..6ad48ecc 100644 --- a/llama/src/test/java/net/ladenthin/llama/server/NativeServerAttachIntegrationTest.java +++ b/llama/src/test/java/net/ladenthin/llama/server/NativeServerAttachIntegrationTest.java @@ -143,7 +143,7 @@ public void models_reportNumericVocabType() throws IOException { .path("vocab_type"); assertThat( "vocab_type must be the numeric llama_vocab_type, not a JSON boolean - " - + "patches/0010 reverted, dropped, or no longer applying? body: " + response.body, + + "did common_json lose the enum constructor upstream #28518 added? body: " + response.body, vocabType.isIntegralNumber(), is(true)); }