diff --git a/.github/models.csv b/.github/models.csv index 3fe2b67da..476405095 100644 --- a/.github/models.csv +++ b/.github/models.csv @@ -19,3 +19,4 @@ SmolVLM-500M-Instruct-Q8_0.gguf,https://huggingface.co/ggml-org/SmolVLM-500M-Ins mmproj-SmolVLM-500M-Instruct-Q8_0.gguf,https://huggingface.co/ggml-org/SmolVLM-500M-Instruct-GGUF/resolve/main/mmproj-SmolVLM-500M-Instruct-Q8_0.gguf Qwen3-TTS-12Hz-1.7B-Base-Q4_K_M.gguf,https://huggingface.co/ggml-org/Qwen3-TTS-12Hz-1.7B-Base-GGUF/resolve/main/Qwen3-TTS-12Hz-1.7B-Base-Q4_K_M.gguf mmproj-Qwen3-TTS-12Hz-1.7B-Base-Q8_0.gguf,https://huggingface.co/ggml-org/Qwen3-TTS-12Hz-1.7B-Base-GGUF/resolve/main/mmproj-Qwen3-TTS-12Hz-1.7B-Base-Q8_0.gguf +stories260K.gguf,https://huggingface.co/ggml-org/tiny-llamas/resolve/main/stories260K.gguf diff --git a/.github/verify-patches-applied.sh b/.github/verify-patches-applied.sh new file mode 100755 index 000000000..ab02c9ff8 --- /dev/null +++ b/.github/verify-patches-applied.sh @@ -0,0 +1,78 @@ +#!/usr/bin/env bash + +# SPDX-FileCopyrightText: 2026 Bernard Ladenthin +# +# SPDX-License-Identifier: MIT OR Apache-2.0 + +# 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: +# +# 0003, 0006, 0007, 0008 -> jllama.cpp / native_server.cpp call the symbols they add, +# so dropping one is a compile or link error. +# 0011 -> the ContentOnlyParseUtf8 tests in src/test/cpp/test_utils.cpp. +# 0012 -> src/test/cpp/test_model_split.cpp. +# 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. +# +# Usage: .github/verify-patches-applied.sh [] +# Exit codes: 0 all good, 1 a check failed. + +set -euo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +SRC="${1:-$ROOT/llama/build/_deps/llama.cpp-src}" +PATCH_DIR="$ROOT/llama/patches" +STAMP="$SRC/.jllama-patches-applied" + +fail() { + echo "ERROR: $*" >&2 + exit 1 +} + +[ -d "$SRC" ] || fail "llama.cpp source dir not found: $SRC (configure the build first)" +[ -f "$STAMP" ] || fail "patch stamp not found: $STAMP — the applier never ran, so the tree is unpatched" + +# --- 1. every patch on disk is named in the stamp ----------------------------------------------- +# Self-maintaining on purpose: adding a patch file needs no edit here. The stamp's first line is +# the checked-out llama.cpp commit; every other line is " ". +on_disk=0 +for p in "$PATCH_DIR"/*.patch; do + [ -e "$p" ] || fail "no *.patch files in $PATCH_DIR" + on_disk=$((on_disk + 1)) + name="$(basename "$p")" + grep -qF "$name" "$STAMP" || fail "patch '$name' is on disk but absent from the stamp $STAMP" +done + +in_stamp="$(($(wc -l < "$STAMP") - 1))" +[ "$in_stamp" -eq "$on_disk" ] \ + || fail "stamp lists $in_stamp patch(es) but $on_disk are on disk — the build dir is stale; configure into a fresh one" + +# --- 2. the tree is actually modified ------------------------------------------------------------ +# A valid stamp over a clean tree means the patches were reverted after the fact. +if git -C "$SRC" rev-parse --git-dir >/dev/null 2>&1; then + if git -C "$SRC" diff --quiet; then + fail "stamp says $on_disk patch(es) applied but '$SRC' is clean — the patched files were reverted" + 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" diff --git a/.github/verify-test-counts.sh b/.github/verify-test-counts.sh new file mode 100755 index 000000000..7bb818148 --- /dev/null +++ b/.github/verify-test-counts.sh @@ -0,0 +1,89 @@ +#!/usr/bin/env bash + +# SPDX-FileCopyrightText: 2026 Bernard Ladenthin +# +# SPDX-License-Identifier: MIT OR Apache-2.0 + +# Fails a Java test job when the suite silently stopped running tests. +# +# WHY THIS EXISTS. Surefire's working directory is the module basedir while the shared GGUF cache +# restores to the reactor root, so every model path resolved one directory too deep and EVERY +# model-gated class aborted in its @BeforeAll assumption — on every test-java-* job, for months, +# while the pipeline stayed green. Several stale assertions rode along unnoticed. +# +# The shape is what defeats the obvious guard: a CLASS-LEVEL assumption failure makes Surefire +# record tests="0" errors="0" failures="0" skipped="0" for that class. It contributes NO test +# entries at all, so "did this run skip anything?" is structurally blind to it — a skipped test is +# still a reported test. Two checks catch it: +# +# 1. Any testsuite reporting tests="0". This is the exact signature above, it is precise, and it +# is platform-independent: a class that runs nowhere is a bug on every OS. +# 2. A floor on the total number of tests executed. The backstop for a whole class file going +# missing from the run rather than reporting zero. +# +# Check 1 is the sensitive one; check 2 is deliberately slack so it never fails spuriously on a +# platform that legitimately runs fewer tests. Tighten --min-total once real per-platform numbers +# are known from a green run. +# +# Usage: .github/verify-test-counts.sh [--min-total N] +# Exit codes: 0 all good, 1 a check failed, 2 nothing to scan. + +set -euo pipefail + +REPORT_DIR="${1:-}" +MIN_TOTAL=0 +shift || true +while [ $# -gt 0 ]; do + case "$1" in + --min-total) MIN_TOTAL="$2"; shift 2 ;; + *) echo "unknown argument: $1" >&2; exit 1 ;; + esac +done + +[ -n "$REPORT_DIR" ] || { echo "usage: $0 [--min-total N]" >&2; exit 1; } +[ -d "$REPORT_DIR" ] || { echo "ERROR: no such directory: $REPORT_DIR" >&2; exit 2; } + +shopt -s nullglob +reports=("$REPORT_DIR"/TEST-*.xml) +shopt -u nullglob + +# An empty input is a failure, never a pass — the same rule verify-bytecode-version.sh follows, +# for the same reason: a job that produced no reports at all has not proved anything. +if [ "${#reports[@]}" -eq 0 ]; then + echo "ERROR: no TEST-*.xml under $REPORT_DIR — the suite did not run" >&2 + exit 2 +fi + +total=0 +empty_suites=() +for f in "${reports[@]}"; do + # The count lives on the element; take the first match so a nested element + # carrying the same attribute name cannot shift the number. + n="$(grep -o 'tests="[0-9]*"' "$f" | head -1 | grep -o '[0-9]*' || true)" + [ -n "$n" ] || n=0 + total=$((total + n)) + if [ "$n" -eq 0 ]; then + empty_suites+=("$(basename "$f")") + fi +done + +status=0 + +if [ "${#empty_suites[@]}" -gt 0 ]; then + echo "ERROR: ${#empty_suites[@]} test class(es) contributed ZERO test entries:" >&2 + printf ' %s\n' "${empty_suites[@]}" >&2 + echo " A class-level @BeforeAll assumption that fails looks exactly like this. It is NOT a" >&2 + echo " skip — the class reports no tests at all, so it is invisible to a skip check. If a" >&2 + echo " model is missing, validate-models should have failed the job before this point." >&2 + status=1 +fi + +if [ "$total" -lt "$MIN_TOTAL" ]; then + echo "ERROR: only $total test(s) executed, below the floor of $MIN_TOTAL" >&2 + status=1 +fi + +if [ "$status" -eq 0 ]; then + echo "test counts verified: $total test(s) across ${#reports[@]} class(es), none empty (floor $MIN_TOTAL)" +fi +exit "$status" diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 9219e6d4c..6edb00294 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -42,6 +42,12 @@ env: # Q4_K_M backbone (0.96 GiB), Q8_0 mmproj (0.42 GiB; no smaller mmproj quant is published). TTS_MODEL_NAME: "Qwen3-TTS-12Hz-1.7B-Base-Q4_K_M.gguf" TTS_MMPROJ_NAME: "mmproj-Qwen3-TTS-12Hz-1.7B-Base-Q8_0.gguf" + # Fine-tuning fixture for LlamaTrainerIntegrationTest. stories260K is 1.19 MB and, unlike every + # other model here, is **F32** — which is the only reason it works: llama_set_param silently skips + # any tensor that is not GGML_TYPE_F32, so a quantized model would "train" nothing but the norm + # weights and still write an output GGUF that the test's assertions accept. Its small trained + # context also suits the test's short corpus; the test pins nCtx itself so that is belt-and-braces. + TRAIN_MODEL_NAME: "stories260K.gguf" # Test image used by MultimodalIntegrationTest is committed to the repo # at src/test/resources/images/test-image.jpg (see the README in that # directory for licensing). No download step is needed; CI just points @@ -2246,6 +2252,12 @@ 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. + - name: Verify llama.cpp patches are applied + run: .github/verify-patches-applied.sh - name: Run C++ unit tests run: ctest --test-dir llama/build --output-on-failure @@ -2352,7 +2364,15 @@ jobs: -Dnet.ladenthin.llama.vision.mmproj=models/${VISION_MMPROJ_NAME} \ -Dnet.ladenthin.llama.vision.image=${VISION_IMAGE_PATH} \ -Dnet.ladenthin.llama.tts.model=models/${TTS_MODEL_NAME} \ - -Dnet.ladenthin.llama.tts.mmproj=models/${TTS_MMPROJ_NAME} + -Dnet.ladenthin.llama.tts.mmproj=models/${TTS_MMPROJ_NAME} \ + -Dnet.ladenthin.llama.train.model=models/${TRAIN_MODEL_NAME} + # Green tests are not the same as tests that ran. A class-level @BeforeAll assumption that + # fails makes Surefire record tests="0" for that class -- no entries at all, so it is + # invisible to a skip check. That is exactly how every model-gated class stayed silently + # muted on every test-java-* job for months. The floor is deliberately slack; the per-class + # zero check is the sensitive half. + - name: Verify tests actually ran + run: .github/verify-test-counts.sh llama/target/surefire-reports --min-total 1500 - uses: actions/upload-artifact@v7 if: success() with: @@ -2507,7 +2527,15 @@ jobs: -Dnet.ladenthin.llama.vision.mmproj=models/${VISION_MMPROJ_NAME} \ -Dnet.ladenthin.llama.vision.image=${VISION_IMAGE_PATH} \ -Dnet.ladenthin.llama.tts.model=models/${TTS_MODEL_NAME} \ - -Dnet.ladenthin.llama.tts.mmproj=models/${TTS_MMPROJ_NAME} + -Dnet.ladenthin.llama.tts.mmproj=models/${TTS_MMPROJ_NAME} \ + -Dnet.ladenthin.llama.train.model=models/${TRAIN_MODEL_NAME} + # Green tests are not the same as tests that ran. A class-level @BeforeAll assumption that + # fails makes Surefire record tests="0" for that class -- no entries at all, so it is + # invisible to a skip check. That is exactly how every model-gated class stayed silently + # muted on every test-java-* job for months. The floor is deliberately slack; the per-class + # zero check is the sensitive half. + - name: Verify tests actually ran + run: .github/verify-test-counts.sh llama/target/surefire-reports --min-total 1500 - name: Memory after tests if: always() run: vm_stat && sysctl hw.memsize hw.physmem @@ -2610,7 +2638,15 @@ jobs: -Dnet.ladenthin.llama.vision.mmproj=models/${VISION_MMPROJ_NAME} \ -Dnet.ladenthin.llama.vision.image=${VISION_IMAGE_PATH} \ -Dnet.ladenthin.llama.tts.model=models/${TTS_MODEL_NAME} \ - -Dnet.ladenthin.llama.tts.mmproj=models/${TTS_MMPROJ_NAME} + -Dnet.ladenthin.llama.tts.mmproj=models/${TTS_MMPROJ_NAME} \ + -Dnet.ladenthin.llama.train.model=models/${TRAIN_MODEL_NAME} + # Green tests are not the same as tests that ran. A class-level @BeforeAll assumption that + # fails makes Surefire record tests="0" for that class -- no entries at all, so it is + # invisible to a skip check. That is exactly how every model-gated class stayed silently + # muted on every test-java-* job for months. The floor is deliberately slack; the per-class + # zero check is the sensitive half. + - name: Verify tests actually ran + run: .github/verify-test-counts.sh llama/target/surefire-reports --min-total 1500 - name: Memory after tests if: always() run: vm_stat && sysctl hw.memsize hw.physmem @@ -2713,7 +2749,15 @@ jobs: -Dnet.ladenthin.llama.vision.mmproj=models/${VISION_MMPROJ_NAME} \ -Dnet.ladenthin.llama.vision.image=${VISION_IMAGE_PATH} \ -Dnet.ladenthin.llama.tts.model=models/${TTS_MODEL_NAME} \ - -Dnet.ladenthin.llama.tts.mmproj=models/${TTS_MMPROJ_NAME} + -Dnet.ladenthin.llama.tts.mmproj=models/${TTS_MMPROJ_NAME} \ + -Dnet.ladenthin.llama.train.model=models/${TRAIN_MODEL_NAME} + # Green tests are not the same as tests that ran. A class-level @BeforeAll assumption that + # fails makes Surefire record tests="0" for that class -- no entries at all, so it is + # invisible to a skip check. That is exactly how every model-gated class stayed silently + # muted on every test-java-* job for months. The floor is deliberately slack; the per-class + # zero check is the sensitive half. + - name: Verify tests actually ran + run: .github/verify-test-counts.sh llama/target/surefire-reports --min-total 1500 - name: Memory after tests if: always() run: vm_stat && sysctl hw.memsize hw.physmem @@ -2834,7 +2878,16 @@ jobs: "-Dnet.ladenthin.llama.vision.mmproj=models/$env:VISION_MMPROJ_NAME" ` "-Dnet.ladenthin.llama.vision.image=$env:VISION_IMAGE_PATH" ` "-Dnet.ladenthin.llama.tts.model=models/$env:TTS_MODEL_NAME" ` - "-Dnet.ladenthin.llama.tts.mmproj=models/$env:TTS_MMPROJ_NAME" + "-Dnet.ladenthin.llama.tts.mmproj=models/$env:TTS_MMPROJ_NAME" ` + "-Dnet.ladenthin.llama.train.model=models/$env:TRAIN_MODEL_NAME" + # Green tests are not the same as tests that ran. A class-level @BeforeAll assumption that + # fails makes Surefire record tests="0" for that class -- no entries at all, so it is + # invisible to a skip check. That is exactly how every model-gated class stayed silently + # muted on every test-java-* job for months. The floor is deliberately slack; the per-class + # zero check is the sensitive half. + - name: Verify tests actually ran + shell: bash + run: .github/verify-test-counts.sh llama/target/surefire-reports --min-total 1500 - name: Memory after tests if: always() run: Get-CimInstance Win32_OperatingSystem | Select-Object FreePhysicalMemory,TotalVisibleMemorySize | Format-List @@ -2962,7 +3015,16 @@ jobs: "-Dnet.ladenthin.llama.vision.mmproj=models/$env:VISION_MMPROJ_NAME" ` "-Dnet.ladenthin.llama.vision.image=$env:VISION_IMAGE_PATH" ` "-Dnet.ladenthin.llama.tts.model=models/$env:TTS_MODEL_NAME" ` - "-Dnet.ladenthin.llama.tts.mmproj=models/$env:TTS_MMPROJ_NAME" + "-Dnet.ladenthin.llama.tts.mmproj=models/$env:TTS_MMPROJ_NAME" ` + "-Dnet.ladenthin.llama.train.model=models/$env:TRAIN_MODEL_NAME" + # Green tests are not the same as tests that ran. A class-level @BeforeAll assumption that + # fails makes Surefire record tests="0" for that class -- no entries at all, so it is + # invisible to a skip check. That is exactly how every model-gated class stayed silently + # muted on every test-java-* job for months. The floor is deliberately slack; the per-class + # zero check is the sensitive half. + - name: Verify tests actually ran + shell: bash + run: .github/verify-test-counts.sh llama/target/surefire-reports --min-total 1500 - name: Memory after tests if: always() run: Get-CimInstance Win32_OperatingSystem | Select-Object FreePhysicalMemory,TotalVisibleMemorySize | Format-List @@ -3232,6 +3294,26 @@ jobs: compression-level: 0 retention-days: 7 if-no-files-found: error + # The two aarch64 fat jars are release assets exactly like the x86-64 pair, so they get the + # same treatment: package-fatjars emits four, and for a long time only the two x86-64 ones + # were ever launched. GitHub's free ARM runners (ubuntu-24.04-arm / windows-11-arm) are + # already used by the aarch64 build jobs, so there is no reason to leave these unrun. + - name: Upload Linux aarch64 smoke jar + uses: actions/upload-artifact@v7 + with: + name: llama-fatjar-smoke-linux-aarch64 + path: fatjars/llama-*-all-linux-aarch64-jar-with-dependencies.jar + compression-level: 0 + retention-days: 7 + if-no-files-found: error + - name: Upload Windows arm64 smoke jar + uses: actions/upload-artifact@v7 + with: + name: llama-fatjar-smoke-windows-arm64 + path: fatjars/llama-*-all-windows-aarch64-jar-with-dependencies.jar + compression-level: 0 + retention-days: 7 + if-no-files-found: error # GPU-less runners: every manifest backend must fail its load cleanly (missing vendor # runtimes) and the server must come up on the default CPU natives — this exercises @@ -3319,6 +3401,94 @@ jobs: server-err.log if-no-files-found: warn + # The aarch64 halves of the same convention. Identical in shape to the two jobs above — the only + # differences are the runner and the jar glob — and they exist because `all-linux-aarch64` and + # `all-windows-aarch64` were built, GPG-signed and attached to every release without CI ever + # launching them, which is exactly what workspace/policies/fat-jar-release-assets.md forbids + # ("No release asset is attached that CI has not run"). That rule exists because a corrupt macOS + # dylib shipped in three releases under a fully green pipeline; these two jars were the remaining + # assets in the same blind spot. + smoke-fatjar-linux-aarch64: + name: Smoke test all-backends fat jar (Linux aarch64) + needs: [package-fatjars, verify-model-cache] + runs-on: ubuntu-24.04-arm + steps: + - uses: actions/checkout@v7 + - uses: actions/download-artifact@v8 + with: + name: llama-fatjar-smoke-linux-aarch64 + path: fatjars/ + - name: Restore shared GGUF model cache (populated by download-models; no re-download) + # Restore-only: consumer jobs can NEVER write the cache, so a job running on a + # cache miss cannot re-save an empty/partial entry under the immutable key. + # download-models is the single writer; enableCrossOsArchive matches its + # cross-OS entry version so every consumer restores the same ubuntu-built entry. + uses: actions/cache/restore@v6 + with: + path: models/ + key: gguf-models-${{ hashFiles('.github/models.csv') }} + enableCrossOsArchive: true + - name: Validate model files + run: .github/validate-models.sh + - uses: actions/setup-java@v6 + with: + distribution: 'temurin' + java-version: ${{ env.JAVA_VERSION }} + # Same floor, re-checked on this assembled release asset: package-fatjars rewrites the zip + # per OS/arch, so the aarch64 jar is a different artifact from the one smoke-fatjar-linux + # verifies even though the classes are identical. + - name: Verify Java 8 bytecode (no class newer than major 52) + run: .github/verify-bytecode-version.sh --max-major 52 fatjars + - name: Run fat-jar server smoke test + run: .github/smoke-test-fatjar.sh fatjars 'llama-*-all-linux-aarch64-jar-with-dependencies.jar' "models/${DRAFT_MODEL_NAME}" + - name: Upload server logs + if: failure() + uses: actions/upload-artifact@v7 + with: + name: fatjar-smoke-linux-aarch64-logs + path: | + server-out.log + server-err.log + if-no-files-found: warn + + smoke-fatjar-windows-arm64: + name: Smoke test all-backends fat jar (Windows arm64) + needs: [package-fatjars, verify-model-cache] + runs-on: windows-11-arm + steps: + - uses: actions/checkout@v7 + - uses: actions/download-artifact@v8 + with: + name: llama-fatjar-smoke-windows-arm64 + path: fatjars/ + - name: Restore shared GGUF model cache (populated by download-models; no re-download) + # Restore-only: see the note on the Linux job above. + uses: actions/cache/restore@v6 + with: + path: models/ + key: gguf-models-${{ hashFiles('.github/models.csv') }} + enableCrossOsArchive: true + - name: Validate model files + run: .github\validate-models.bat + # temurin publishes a Windows/AArch64 JDK for this java-version; the build-windows-arm64 + # job already resolves it on this same runner. + - uses: actions/setup-java@v6 + with: + distribution: 'temurin' + java-version: ${{ env.JAVA_VERSION }} + - name: Run fat-jar server smoke test + shell: pwsh + run: .github/smoke-test-fatjar.ps1 -JarDir fatjars -JarGlob 'llama-*-all-windows-aarch64-jar-with-dependencies.jar' -Model "models/$env:DRAFT_MODEL_NAME" + - name: Upload server logs + if: failure() + uses: actions/upload-artifact@v7 + with: + name: fatjar-smoke-windows-arm64-logs + path: | + server-out.log + server-err.log + if-no-files-found: warn + # --------------------------------------------------------------------------- # macOS member of the cross-repo "no release asset is attached that CI has not run" convention # (workspace/policies/fat-jar-release-assets.md; BitcoinAddressFinder and srcmorph run the shared @@ -3407,7 +3577,7 @@ jobs: publish-snapshot: name: Publish Snapshot to Central - needs: [check-snapshot, crosscompile-linux-x86_64-cuda, crosscompile-android-aarch64-opencl, package-android-aar, test-android-emulator, code-style, test-java-llama-langchain4j, test-java-llama-kotlin, package-fatjars, smoke-fatjar-linux, smoke-fatjar-windows, smoke-fatjar-macos] + needs: [check-snapshot, crosscompile-linux-x86_64-cuda, crosscompile-android-aarch64-opencl, package-android-aar, test-android-emulator, code-style, test-java-llama-langchain4j, test-java-llama-kotlin, package-fatjars, smoke-fatjar-linux, smoke-fatjar-windows, smoke-fatjar-linux-aarch64, smoke-fatjar-windows-arm64, smoke-fatjar-macos] if: needs.check-snapshot.result == 'success' && inputs.publish_to_central runs-on: ubuntu-latest environment: maven-central @@ -3674,7 +3844,7 @@ jobs: publish-release: name: Publish Release to Central if: needs.check-tag.result == 'success' && inputs.publish_to_central - needs: [check-tag, crosscompile-linux-x86_64-cuda, crosscompile-android-aarch64-opencl, package-android-aar, test-android-emulator, code-style, test-java-llama-langchain4j, test-java-llama-kotlin, package-fatjars, smoke-fatjar-linux, smoke-fatjar-windows, smoke-fatjar-macos] + needs: [check-tag, crosscompile-linux-x86_64-cuda, crosscompile-android-aarch64-opencl, package-android-aar, test-android-emulator, code-style, test-java-llama-langchain4j, test-java-llama-kotlin, package-fatjars, smoke-fatjar-linux, smoke-fatjar-windows, smoke-fatjar-linux-aarch64, smoke-fatjar-windows-arm64, smoke-fatjar-macos] runs-on: ubuntu-latest environment: maven-central permissions: diff --git a/CLAUDE.md b/CLAUDE.md index 737302a04..c272b1803 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: **b10948** +Current llama.cpp pinned version: **b10976** ## Upgrading CUDA Version @@ -438,11 +438,19 @@ Mechanism (three pieces): property `net.ladenthin.llama.backend` forces one backend (fail-loud) or `default`/`cpu`. Jars without a manifest take the unchanged legacy path. A backend whose extra module is already resident from a previously failed attempt is skipped (by-name import cross-wiring). -3. **`publish.yml` wiring** — `smoke-fatjar-linux` / `smoke-fatjar-windows` run the - `all--x86-64` jar via real `java -jar` on GPU-less runners (cached draft model, - `--chat-template chatml`): poll `/health` to 200, assert a `/v1/chat/completions` choice, - and require the loader's backend-selection log line. `publish-snapshot`/`publish-release` - `need` `package-fatjars` + both smokes (fail-loud gating); `github-release-signed` and +3. **`publish.yml` wiring** — **all four** OS/arch fat jars are launched, one smoke job each: + `smoke-fatjar-linux` / `smoke-fatjar-windows` (x86-64) plus `smoke-fatjar-linux-aarch64` + (`ubuntu-24.04-arm`) / `smoke-fatjar-windows-arm64` (`windows-11-arm`). Each runs its jar via + real `java -jar` on a GPU-less runner (cached draft model, `--chat-template chatml`): poll + `/health` to 200, assert a `/v1/chat/completions` choice, and require the loader's + backend-selection log line — so every manifest backend failing its load and falling back to the + CPU natives is exercised on the actual release asset. The four jobs consume four small + single-jar artifacts (`llama-fatjar-smoke-{linux,windows,linux-aarch64,windows-arm64}`) rather + than the multi-GB `llama-fatjars` set. **The two aarch64 jobs close a real gap**: those jars were + built, GPG-signed and attached to every release while `publish.yml` referenced them zero times, + which is exactly what the cross-repo rule forbids — and that rule exists because a corrupt macOS + dylib shipped in three releases under a fully green pipeline. `publish-snapshot`/`publish-release` + `need` `package-fatjars` + **all four** smokes (fail-loud gating); `github-release-signed` and `github-snapshot` additionally download `llama-fatjars` into their asset directory, then **GPG-sign each fat jar** via `.github/sign-fatjars.sh` (a detached `.asc` alongside the `.sha256`), so the fat jars land signed on the tag release and the rolling `snapshot` @@ -502,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 b10948 https://github.com/ggml-org/llama.cpp /tmp/lc +git clone --depth 1 --branch b10976 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 \ @@ -542,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 b10948`), the +Because `sccache` is **content-addressed** and llama.cpp is pinned (`GIT_TAG b10976`), 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 @@ -1384,6 +1392,38 @@ Functions with `_impl` suffix are called directly from `jllama.cpp`. - If it needs upstream server types, put it in Layer B (after the `json_helpers.hpp` include). - Add tests to `src/test/cpp/test_jni_helpers.cpp`. +### The JNI exception boundary — `jni_guard_impl` + +An exception that escapes a native method and unwinds across the JNI boundary is **undefined +behaviour and aborts the JVM** on most implementations. **Every `Java_*` entry point must therefore +convert anything that escapes into a Java exception**, and there are 40 of them across three TUs — +`jllama.cpp` (34), `native_server.cpp` (5), `train_engine.cpp` (1). + +The mechanism is `jni_guard_impl(env, exception_class, [&]() -> Ret { … })` (`jni_helpers.hpp`, +Layer A). It is **additive**: an entry point that already converts `std::exception` itself keeps +doing so and never reaches the guard's handlers. What the guard adds everywhere is the +**`catch (...)` arm** — the case for an exception type not derived from `std::exception`, which +otherwise has no backstop at all. On a catch it returns the zero/`nullptr` sentinel for the entry +point's return type. + +Two rules the handler keeps, both pinned by tests in `test_jni_helpers.cpp`: + +- **Never `ThrowNew` over an already-pending Java exception.** The JNI spec forbids most calls in + that state, and the pending exception is the more precise error — so it is left in place. +- **Never `ThrowNew` with a null class.** + +**Three entry points are deliberately NOT routed through it, and each uses a function-try-block +instead** (which also avoids reindenting a `goto`-carrying body): `JNI_OnLoad` runs before +`c_llama_error` is cached and `JNI_OnUnload` after it is released, so neither has a class to throw +with — `JNI_OnLoad` returns `JNI_ERR` (the JVM surfaces that as `UnsatisfiedLinkError`) and +`JNI_OnUnload` swallows. `LlamaTrainer_finetuneNative` reports failure as its **return string** +rather than a Java exception, and `train_engine.cpp` deliberately keeps its own `nlohmann` alias +and never includes `jni_helpers.hpp`, so its backstop returns an error string to preserve that +contract. + +**When you add a native method, wrap it.** The guard is not enforced by a test — a new unguarded +entry point is invisible until something throws through it in production. + ### Parameter Flow Java parameters are serialized to JSON strings and passed to native code, which deserializes them using nlohmann/json. This avoids complex JNI field mapping for the many llama.cpp parameters. @@ -1551,18 +1591,18 @@ ctest --test-dir build --output-on-failure -R "ResultsToJson" | `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_log_helpers.cpp` | 13 | All functions in `log_helpers.hpp`: `log_level_name`, `format_log_as_json` | -| `src/test/cpp/test_jni_helpers.cpp` | 56 | 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) | +| `src/test/cpp/test_jni_helpers.cpp` | 63 | 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). The last 7 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). | | `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 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: 537 tests (all passing).** +**Current total: 544 tests (all passing).** #### Upstream source location (in CMake build tree) -llama.cpp is fetched via CMake FetchContent, pinned to `GIT_TAG b10948`. +llama.cpp is fetched via CMake FetchContent, pinned to `GIT_TAG b10976`. **GoogleTest** is a separate `BUILD_TESTING`-only FetchContent (`GIT_TAG v1.17.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 f995a214f..9f336ef4b 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 b10948](https://img.shields.io/badge/llama.cpp-%23b10948-informational)](https://github.com/ggml-org/llama.cpp/releases/tag/b10948) +[![llama.cpp b10976](https://img.shields.io/badge/llama.cpp-%23b10976-informational)](https://github.com/ggml-org/llama.cpp/releases/tag/b10976) [![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 1f1305fa0..0420720f4 100644 --- a/TODO.md +++ b/TODO.md @@ -96,9 +96,11 @@ workflow in `.github/workflows/`). It contributes to the `mergeable_state: block ### Upstream PR submissions — drop the carried patches (open) -Six of the seven `patches/` are upstream-submittable verbatim; each accepted PR (once the pin is -bumped past it) deletes a patch from the bump checklist. (`0003` is a carry of an already-open -upstream PR #22393 — it drops automatically when that merges.) +There are **nine** patches today (`0001`–`0003`, `0006`–`0008`, `0010`–`0012`). **Eight 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 +be described here as "drops automatically when that merges"; it will not.) - **`0001` Windows arg-parse embed guard** (against #24779): `common_params_parse` trusts the caller's argv; `common_params_parse_main()` keeps the standalone tools' UTF-8 recovery. Ship with the @@ -113,10 +115,22 @@ upstream PR #22393 — it drops automatically when that merges.) - **`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). -- **`0009` guard `posix_spawn_file_actions_addchdir_np` on old glibc** (b10154 cross-compile break on - manylinux2014 / glibc 2.17 and manylinux_2_28 / glibc 2.28; adds an overridable - `SUBPROCESS_HAVE_CWD` probe via `__GLIBC_PREREQ(2, 29)` — submitted as sheredom/subprocess.h#104, - drops automatically once llama.cpp bumps the vendored pin). +- **`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.** +- **`0011` lenient invalid-UTF-8 in the PEG parser** (one malformed byte from the model turns a + finished generation into an HTTP 500; the `INVALID` branch ignores leniency while the `INCOMPLETE` + branch beside it honours it). Ships an upstream `tests/peg-parser/test-unicode.cpp` case. + **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.** + +(`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 — +upstream merged this project's own PR ggml-org/llama.cpp#28775 and it was dropped at b10948. Both +drops are recorded in `CLAUDE.md` under the patch table.) ### llama.cpp upstream feature exposure (queued, deferred by policy) @@ -219,38 +233,6 @@ These are JNI plumbing items for upstream API additions. Policy: add only after - **Expose `llama_vocab::get_suppress_tokens()` via `LlamaModel.getSuppressTokens()`.** Added in b9490–b9495 alongside the new `tokenizer.ggml.suppress_tokens` GGUF key and the `LLM_KV_TOKENIZER_SUPPRESS_TOKENS` constant. When a GGUF declares this array, upstream stores it on `llama_vocab::impl::suppress_tokens` and exposes it via the new `llama_vocab::get_suppress_tokens()` accessor. The bias is **applied automatically** inside the model forward graph — the Gemma4 Unified graph (`src/models/gemma4.cpp`) reads the list and adds a `-INFINITY` logit bias to those token IDs via a new `llm_graph_input_logits_bias` input so the model cannot emit them (used to block `` / `` placeholders). A Java mirror would be `public int[] getSuppressTokens()` on `LlamaModel`: a read-only inspector returning the suppression list for debugging or for callers running their own sampling who want to replicate the same bias. Value is low (the bias is auto-applied, Java callers cannot change it; java-llama.cpp does not expose custom logit-bias hooks at this level); cost is trivial (one JNI passthrough + a `getSuppressTokens()` Java method). -### JNI safety and server hardening (from PR #251 contributor) - -Raised by [@vaiju1981](https://github.com/vaiju1981) in -[PR #251 comment](https://github.com/bernardladenthin/java-llama.cpp/pull/251#issuecomment-4761363838). -Feel free to contribute fixes — PRs welcome. - -- **Unhandled C++ exceptions cross the JNI boundary → JVM abort (UB).** Any `std::exception` - (or worse, an exception of unknown type) that escapes a native method and crosses the JNI - boundary causes undefined behaviour on most JVMs and typically aborts the process. Each native - method in `jllama.cpp` should wrap its body in `try { … } catch (const std::exception& e) { - env->ThrowNew(llamaExceptionClass, e.what()); return ; } catch (...) { env->ThrowNew(…, - "unknown C++ exception"); return ; }` so that errors surface as `LlamaException` on the - Java side instead of crashing the JVM. - -- **`parse_string_array` — null deref + JNI local-reference leak.** The helper that reads a - JSON string array from JNI can dereference a null pointer when an array element is absent, - and leaks JNI local references when an early exit skips the matching `DeleteLocalRef`. Fix: - guard every `GetObjectArrayElement` result and pair each reference acquisition with a - `DeleteLocalRef` before the next iteration or return. - -- **`close()` / native `delete()` double-free under concurrent close.** If two threads race to - call `LlamaModel.close()`, both can reach the native `delete` path and free the same - `jllama_context` pointer twice → heap corruption. Fix: use `AtomicBoolean closed` + a - `synchronized` guard (or `compareAndSet`) on the Java side so `close()` is idempotent and - the native pointer is nulled before the second caller can reach it. - -- **Unbounded request-body read → OOM DoS.** The HTTP handler reads the entire request body - into a `String`/`byte[]` before parsing it, with no size cap. A client that streams a - multi-gigabyte body can exhaust heap memory and crash the JVM. Fix: add a configurable - `maxRequestBodyBytes` limit (e.g. default 4 MB) and reject oversized requests with - `HTTP 413 Content Too Large` before buffering them. - ### Feature backlog from similar projects (remainder: jbang example) The consolidated investigation lives in @@ -348,50 +330,6 @@ these are what remains. `target/surefire-reports/TEST-*.xml` in each `test-java-*` job and failing below a pinned minimum is the one check that would have caught it directly, and it is cheap. -### Release/build robustness gaps found by the b10679 audit (PR #403) - -Both are **pre-existing** and orthogonal to a version bump, so they were recorded rather than folded -into that PR. - -- **Two `all-*-aarch64` fat jars are attached to releases with no smoke job.** - `.github/package-fatjars.sh` emits four OS/arch fat jars (`linux-x86-64`, `linux-aarch64`, - `windows-x86-64`, `windows-aarch64`), all uploaded as `llama-fatjars` and attached by - `github-release-signed` / `github-snapshot`. Only the two **x86-64** ones are smoked - (`smoke-fatjar-linux`, `smoke-fatjar-windows`); grepping `publish.yml` for `all-linux-aarch64` or - `all-windows-aarch64` returns nothing, so neither is ever downloaded or launched. - - That directly violates the cross-repo rule in - [`../workspace/policies/fat-jar-release-assets.md`](../workspace/policies/fat-jar-release-assets.md) - — *"No release asset is attached that CI has not run"* — which exists because a corrupt macOS dylib - shipped in three releases under a fully green pipeline. The fix is cheap: the workflow **already** - uses the free ARM runners elsewhere (`ubuntu-24.04-arm` for the aarch64 CPU and Vulkan builds, - `windows-11-arm` for the Windows arm64 build), so `smoke-fatjar-linux-aarch64` and - `smoke-fatjar-windows-arm64` can mirror the existing smoke jobs and join both publish jobs' - `needs:`. Not done in the bump PR because it widens a version bump into CI work and would gate that - PR on a pre-existing defect if either jar turns out to be broken. - -- **The patch applier silently accepts a partially-reverted source tree.** The stamp file records the - checked-out llama.cpp commit plus each patch's SHA-256 — **nothing about the resulting file - contents**. Reverting one patched file after a successful apply leaves the stamp valid and the tree - still dirty (the other patched files are still modified), so the dirty-tree branch reports - "already applied — skipping", exits 0, and the build compiles unpatched code. Reproduction: - - ```bash - # with the tree fully patched and the stamp written: - git -C checkout -- common/peg-parser.cpp # drops patch 0011's fix - cmake -DPATCH_DIR=... -DLLAMA_SRC=... -P llama/cmake/apply-llama-patches.cmake - # -> "8 patch(es) already applied — skipping", exit 0, patch NOT restored - ``` - - Every other path is correctly fail-loud (committed-patch state, stamp/HEAD mismatch on a dirty tree, - and a non-git-worktree re-run all exit 1). The fix is a content oracle in the manifest — cheapest is - to append `git -C diff --no-color | sha256`, or per-patched-file blob hashes — so a reverted or - hand-edited file invalidates the stamp. **CI is unaffected** (every job configures into a fresh build - directory); this only bites a local reconfigure, which is why it was not rushed. Note the stamp - format change will make every existing local build dir abort with the applier's - "configure into a fresh build directory" message — that is the designed fail-loud path, not a - regression. - ### Test-coverage debt found during the b10649 review (PR #403) Each item below was verified against pristine upstream tags and is real, but none is a regression diff --git a/docs/history/llama-cpp-breaking-changes.md b/docs/history/llama-cpp-breaking-changes.md index d1bc2215e..72c888102 100644 --- a/docs/history/llama-cpp-breaking-changes.md +++ b/docs/history/llama-cpp-breaking-changes.md @@ -728,3 +728,7 @@ Used during `llama.cpp` version bumps: when upgrading, scan this file from the r | b10934–b10938 | patches + upstream verification | End of the six-chunk walk. **All ten patches apply at pristine b10938 and all four standing drop-checks 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 `common/arg.h`; the WIN32 override still at `common/arg.cpp:1282` — unmoved despite three edits to that file in this range), `0010` (`{"vocab_type", …}` still uncast at `server-context.cpp:4554`), `0012` (bare `splits[i] /= split_sum;` still at `src/llama-model.cpp:1491`), `0013` (the three s390x helpers still unguarded at 73/77/83). **`0013` remains filed-but-unmerged upstream** as [ggml-org/llama.cpp#28775](https://github.com/ggml-org/llama.cpp/pull/28775); when it merges, the first tag carrying it aborts the configure and the response is to **delete** the patch, not refresh it. | | b10938–b10948 | 10 commits, **33 KiB** — identical excluding `tools/ui`, because the range does not touch the WebUI at all. `ggml/src/ggml-sycl/**` (**#28227**, a Level-Zero memory-query fix, a new `GGML_SYCL_DEV_DEBUG` macro in `base.hpp`, and a `LEVEL_ZERO_INCLUDE_DIR` → `LEVEL_ZERO_DEV_INCLUDE_DIR` rename plus a longer warning in `ggml-sycl/CMakeLists.txt`), `ggml/src/ggml-cpu/arch/s390/repack.cpp` (**#28775** — *this project's own patch*, see the paired row), `src/models/nemotron-h.cpp` (**#28779**, the expert-FFN-size fallback now throws instead of dividing by zero when a layer declares neither `expert_feed_forward_length` nor `expert_used_count`), plus `tests/**`, `.github/**`, `ci/run.sh`, `docs/backend/SYCL.md` and `.pi/gg/SYSTEM.md`. 16 files, 151 insertions, 95 deletions. | **No project-source change, and not one file on the priority review list moved.** Zero files under `common/`, `include/`, `tools/server/` or `tools/mtmd/`, so every row of the API-compatibility table is vacuously satisfied and the three mechanical server-contract greps have **no input to compare** — the request-field set, its `set_hard_limits` bounds and the emitted response keys cannot have moved. The two upstream *code* changes that are neither tests nor CI are both internal TUs upstream compiles itself: the nemotron-h loader guard (a new `throw` on a malformed GGUF; no signature moved) and the SYCL backend, which only the `sycl-*` classifier jobs build — `base.hpp`'s new `extern int g_ggml_sycl_dev_debug` and the `ggml-sycl/CMakeLists.txt` variable rename are local to `ggml-sycl` and add no required flag or dependency, so those classifiers build unchanged. What *is* ours in this range is a **patch deletion**, not a source edit — see the paired row. | | b10938–b10948 | patches + upstream verification | **Nine patches now, not ten: `0013` was dropped here.** Upstream merged this project's own PR [ggml-org/llama.cpp#28775](https://github.com/ggml-org/llama.cpp/pull/28775) ("ggml-cpu(s390x): guard VXE-only repack helpers", commit `6978052`, first tagged at **b10948**), so `ggml/src/ggml-cpu/arch/s390/repack.cpp` now wraps `vxe_dot_acc` / `vxe_splat_granule` / `vxe_fold` in the `#if defined(__VXE__) || defined(__VXE2__)` guard itself — byte-identical to the patch apart from the trailing `// __VXE__ || __VXE2__` comment the patch put on its `#endif`. **That is the designed signal, and the response is delete, not refresh** (the `0009` precedent at b10280): `git apply -p1` of `0013` against pristine b10948 fails with `patch failed: ggml/src/ggml-cpu/arch/s390/repack.cpp:70 … does not apply`, because the `#if` line it wants to insert is already there. The rationale that outlives the patch — why `build-linux-s390x` is a *scalar* (non-VXE) cross build, and why `-DGGML_VXE=ON` is the wrong response to a future VXE compile error — moved into the "`0013` was dropped at the b10948 bump" note in `CLAUDE.md`, which the job's own comment in `publish.yml` now points at. **The other three 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 `b10948:common/arg.h`; the WIN32 `argv = utf8.ptrs.data()` override still in `common/arg.cpp`), `0010` (`{"vocab_type", meta.model_vocab_type}` still uncast at `b10948:tools/server/server-context.cpp:4554`), `0012` (bare `splits[i] /= split_sum;` still at `b10948:src/llama-model.cpp:1491`, no zero-sum guard). **The s390x half was re-verified with the real cross toolchain rather than by inspection**, because that path has no runnable guard beyond the CI job: `s390x-linux-gnu-g++` (13.2.0) compiles pristine `b10948:ggml/src/ggml-cpu/arch/s390/repack.cpp` clean **both** with the job's own scalar flags (the `-DGGML_NATIVE=OFF` configuration, no `-mvx -mzvector`) **and** with `-mvx -mzvector -march=z15` — upstream's guard covers exactly what the patch covered and forecloses nothing. Verified end-to-end for real: `rm -rf build` then `cmake -B build -DBUILD_TESTING=ON` through the real `FetchContent` path, configure clean, stamp at head `5f436dddb440a288ee5611d7d1eca564a6aca9f4` (= `b10948`) with **nine** SHA-256 lines; extraction unchanged at **138 CLI / 57 request / 15 trainer** names; full `cmake --build --config Release` clean; `ctest` **537/537**; `nm -D` **40** `Java_*` exports, **0** mangled; `mvn -pl llama clean test -Dtest=NativeLibraryLoadSmokeTest` **4/4, 0 skipped** — which is the check that cross-validates the bumped `LLAMA_CPP_VERSION` constant against the linked `build-info`, and needs the `clean` because the constant is inlined into the already-compiled test class. | +| b10948–b10969 | 21 commits, **99 KiB** — identical excluding `tools/ui`, because the range does not touch the WebUI at all. The headline is a **reversal**: **#28892** ("cmake : remove precompiled headers") deletes every `target_precompile_headers` call from `common/`, `src/`, `tests/`, `tools/mtmd/` and `tools/server/`, and **#28882** additionally disables the ggml-cpu PCH and makes `CACHE_LINE_SIZE` include-order independent. Also here: **#28749** `common/common.cpp` (hoist the `llama_n_rs_seq` check above the probe decode in `common_context_can_seq_rm`), **#27000** a new `LLM_ARCH_MAPLE` (Maple 20B-A1B ternary MoE, CPU) with `src/models/maple.cpp`, `conversion/maple.py`, `gguf-py` constants, a `llama-graph.cpp` swiglu-clamp arm and NEOX rope; model-loader correctness (**#28868** `get_key_or_arr` misuse across gemma4 / gemma4-assistant / mimo2 / step35 / qwen4exp, **#28865** mimo2 SWA pattern, **#28896** qwen4exp rms_norm+mul fusion); **#28846** ggml-cuda F32 fallback on devices without BF16 hardware acceleration; **#28670** a SYCL radix `top_k` (new `ggml-sycl/topk-radix.{cpp,hpp}`, +555); **#26885** a `llama_grammar_advance_stack` find+insert coalesce; **#28776** s390x (see the paired row); a new upstream `scripts/check-apiabi-compat.sh` (**#28579**, not consumed here); plus upstream CI/release workflow churn, `ci/run.sh`, and the llama.cpp 0.4.1 / ggml 0.24.0 version bumps. 46 files, 1409 insertions, 273 deletions. | **No project source change, and no priority-list *header* moved** — `common/common.cpp` is the one priority-list file touched and only its implementation. Zero `tools/server/*.{cpp,h}`: `server-schema.cpp`, `server-task.cpp`, `server-context.cpp`, `server-common.h`, `server-task.h` and `server-schema.h` are **byte-identical by blob hash** across the whole b10948→b10976 walk, so the three mechanical server-contract greps have **no input to compare** — the request-field set, its `set_hard_limits` bounds and the emitted response keys cannot have moved. **The PCH removal is a heap-buffer-overflow fix this project shipped**, not housekeeping: PCH arrived with #28091 at **b10917**, so every pin from b10919 through b10948 carried it. Mechanism (upstream #28858) — the PCH force-includes `ggml-impl.h` before `ops.h`, which pulls `` and defines `__cpp_lib_hardware_interference_size`, so the **C++** kernels resolved `CACHE_LINE_SIZE` to `std::hardware_destructive_interference_size` while `ggml-cpu.c`, a **C** TU where that feature macro can never be defined, kept the 64 fallback; the rope work buffer is sized by the C side and written by the C++ side, overflowing it by `(CACHE_LINE_SIZE/4 - 16) * n_threads * 4` bytes and crashing later in `ggml_compute_forward_rope_flt`. Exposure was per-job, not universal: the deleted ggml-cpu guard skipped the PCH for **GCC on non-x86**, so `build-linux-s390x` (cross g++) and the aarch64-native GCC 14 job were never affected, while the clang / AppleClang / clang-cl / MSVC and GCC-on-x86 jobs were — and only where that toolchain's `hardware_destructive_interference_size` actually differs from 64. **On the build-system side all four changes that reach this build are pure removals**, so nothing had to be added here: `tools/server/CMakeLists.txt` is *not* processed at all (`LLAMA_BUILD_TOOLS`/`LLAMA_BUILD_SERVER` are forced OFF and the server TUs are compiled straight into `jllama`), while `common/`, `src/`, `tools/mtmd/` and `ggml/src/ggml-cpu/` are — and dropping PCH also removes the one thing that could have interfered with the sccache-over-Depot compiler launcher. #28749 changes only *when* `common_context_can_seq_rm` answers: for a recurrent/hybrid context it now returns `COMMON_CONTEXT_SEQ_RM_TYPE_RS` before the `llama_memory_clear` + throwaway `llama_decode` probe instead of after it — same enum, same signature, one fewer decode; the consumer is `server-context.cpp`'s `ctx_tgt_seq_rm_type`, compiled into `libjllama`. `LLM_ARCH_MAPLE` is a **mid-enum insert**, but `llm_arch` lives in `src/llama-arch.h` and is never exposed through public `llama.h`, so it carries no ABI consequence for a consumer. The SYCL `top_k` files are picked up by ggml-sycl's own glob (`ggml-sycl/CMakeLists.txt` did not move), so the three `sycl-*` classifiers build unchanged, and the CUDA BF16 fallback only widens device support for `cuda13-*`. | +| b10948–b10969 | patches + upstream verification | **All nine patches apply, and none is droppable.** Following the b10933–b10934 precedent the whole set was applied against the **target** tag up front — `git apply -p1` of all nine, in filename order, against a pristine `b10976` worktree, every one clean — and then independently against pristine `b10969` so this intermediate commit is a valid bisect point rather than an untested waypoint. Two patch targets move in this chunk and both are harmless: `src/llama-model.cpp` (`0012`) gains three additive lines at the `LLM_ARCH_MAPLE` dispatch and rope-type switch, far from the `load_tensors` split arithmetic the patch rewrites, and `tests/CMakeLists.txt` (`0012`) *loses* the `test-peg-parser` PCH line near the peg-parser block, nowhere near the patch's `llama_build_and_test(test-model-split.cpp)` registration. Everything else the nine patches touch — `common/arg.{cpp,h}`, `common/peg-parser.cpp`, all of `tools/server/*.cpp`, and the ~34 standalone `main()` call sites — is **untouched by the entire b10948→b10976 range**. **#28776 is the direct sequel to the previous bump's `0013` drop**: having merged this project's #28775, upstream reverted their temporary in-tree carry of it and added a **non-VXE s390x build to their own CI** (`.github/workflows/build-ibm.yml`), plus the one `UNUSED(nb)` in `ggml/src/ggml-cpu/arch/s390/quants.c` that the scalar path exposed — so the scalar, `-DGGML_NATIVE=OFF` configuration `build-linux-s390x` builds is now guarded upstream as well as here, which is the outcome the `0013` note in `CLAUDE.md` was written to anticipate. | +| b10969–b10976 | 7 commits, **20 KiB** — the final chunk, reaching the target release. Nothing under `common/`, `include/`, `src/` or `tools/server/*.{cpp,h}` at all. **#28771** ("cmake : use `PROJECT_SOURCE_DIR` instead of `CMAKE_SOURCE_DIR`") retargets five path references — `tools/server/CMakeLists.txt` (×2), `app/`, `tools/tuning/`, `examples/eval-callback/` and `tests/` — and extends `examples/test-cmake` to cover consumption as a **subproject**. Backend work: **#28897** CUDA enables `i16`/`i32` for `GGML_OP_DUP` (new `int16_t` arm in `cpy.cu`; `ggml_backend_cuda_device_supports_op` now returns `true` unconditionally for `DUP` instead of excluding those two types), and **#28576** HIP switches flash-attention MMA to **fp32 accumulation on MFMA devices** (`fattn-mma-f16.cuh`: `T_C_VKQ` becomes `tile<16,16,float>`, MFMA gets its own `VKQ_C` extent separate from WMMA, and one `GGML_CUDA_FATTN_MMA_CONFIG_CASE` drops 4→3 warps), with **#28909** relaxing upstream's own HIP spill check for it. The rest is **#28646** (WebUI: stop re-probing a disabled `/tools` endpoint on every message) and upstream CI (**#28911**, **#28936**), plus `docs/ops.md` + `docs/ops/CUDA.csv` regeneration. 22 files, 88 insertions, 42 deletions. | **No project source change and nothing on the priority review list** — the chunk touches no header on it and no implementation behind one. The `CMAKE_SOURCE_DIR` → `PROJECT_SOURCE_DIR` fix is precisely the bug class that bites a **FetchContent subproject** like this one (`CMAKE_SOURCE_DIR` resolves to the *top-level* project — here `llama/` — not llama.cpp's own root), but none of the five files it repairs is processed by this build: `LLAMA_BUILD_TOOLS`/`LLAMA_BUILD_SERVER`/`LLAMA_BUILD_APP` are forced OFF and the server TUs are compiled straight into `jllama`, `tests/` and `examples/` are never added, and `tools/mtmd` — the one subdirectory this build *does* add explicitly — was already correct. So the fix is inert here today and removes a latent trap tomorrow; upstream's new `examples/test-cmake` subproject coverage makes a regression of it their CI's problem rather than this project's. The two backend changes are classifier-scoped and additive in effect: the CUDA `DUP` widening only adds accepted types for `cuda13-*`, and the HIP fp32 accumulation is a numerical-accuracy change confined to AMD **MFMA** hardware, which no GitHub-hosted runner has — the `rocm-*` jobs are build-only, so CI proves it compiles and nothing more, as designed. The WebUI commit is auto-followed: `build-webui` rebuilds the Svelte UI from the pinned `GIT_TAG`, so it needs no action here. | +| b10969–b10976 | patches + upstream verification | End of the two-chunk walk, and **the patch set is unchanged at nine** — nothing dropped, nothing refreshed. Verified against the pristine target rather than inferred: `git apply -p1` of all nine, in filename order, into a clean `b10976` worktree, every one clean. **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 `b10976:common/arg.h`; the WIN32 `argv = utf8.ptrs.data()` override still at `common/arg.cpp:1282`), `0010` (`{"vocab_type", meta.model_vocab_type}` still uncast at `b10976:tools/server/server-context.cpp:4554`), `0012` (bare `splits[i] /= split_sum;` still at `b10976:src/llama-model.cpp:1493`, no zero-sum guard), `0002` (`params_base.load_progress_callback = load_progress_callback;` still unguarded at `server-context.cpp:1095`), and `0003`/`0006`/`0008` (`get_slot_prompt_similarity`, `llama_server_set_embedded`/`llama_server_attach` and `LLAMA_SERVER_WORKER_CMD` all absent upstream). The whole b10948→b10976 range leaves **every** patch target except `src/llama-model.cpp` and `tests/CMakeLists.txt` byte-identical, and both of those move only at a distance from the patched regions. Verified end-to-end for real: `rm -rf build` then `cmake -B build -DBUILD_TESTING=ON` through the real `FetchContent` path, configure clean, stamp at head `987498f4592a76897863cf53711dce38380c082b` (= `b10976`) with **nine** SHA-256 lines; extraction unchanged at **138 CLI / 57 request / 15 trainer** names; full `cmake --build --config Release` clean with **zero** errors; `ctest` **537/537**; `nm -D` **40** `Java_*` exports, **0** mangled; `mvn -pl llama clean test -Dtest=NativeLibraryLoadSmokeTest` **4/4, 0 skipped** — the check that cross-validates the bumped `LLAMA_CPP_VERSION` constant against the linked `build-info`, needing the `clean` because the constant is inlined into the already-compiled test class. | diff --git a/llama/CMakeLists.txt b/llama/CMakeLists.txt index bf0f1a1f7..0077bd6b6 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 b10948 + GIT_TAG b10976 PATCH_COMMAND ${CMAKE_COMMAND} -DPATCH_DIR=${CMAKE_CURRENT_SOURCE_DIR}/patches -DLLAMA_SRC= diff --git a/llama/cmake/apply-llama-patches.cmake b/llama/cmake/apply-llama-patches.cmake index ddc92cb2b..f74e8f8db 100644 --- a/llama/cmake/apply-llama-patches.cmake +++ b/llama/cmake/apply-llama-patches.cmake @@ -105,9 +105,54 @@ if(NOT head_rc EQUAL 0) return() endif() +# --------------------------------------------------------------------------- +# Content oracle for the WORKING TREE. +# +# The commit + patch hashes below describe what SHOULD be applied; they say nothing about what +# the files actually contain. Reverting one patched file after a successful apply therefore left +# the stamp valid and the tree still dirty (the other patched files are still modified), so the +# dirty-tree branch reported "already applied — skipping", exited 0, and the build compiled +# unpatched code. This fingerprint closes that: it hashes the tree's actual modifications, so a +# reverted or hand-edited file changes it and the stamp stops matching. +# +# Two sources, because neither alone is enough: `git diff` carries the CONTENT of modifications +# to tracked files, and `git status --porcelain` carries the PRESENCE of untracked files (patch +# 0012 adds tests/test-model-split.cpp, which `git diff` never sees). The stamp itself is +# untracked and is filtered out, or writing it would change the fingerprint that describes it. +# +# Residual hole, stated rather than papered over: an edit to the *body* of an untracked file a +# patch added is not detected. Closing that would mean hashing every untracked file's contents; +# the CI-side .github/verify-patches-applied.sh covers the case that actually matters. +# --------------------------------------------------------------------------- +function(compute_tree_fingerprint out_var) + execute_process( + COMMAND "${GIT_EXECUTABLE}" -C "${LLAMA_SRC}" status --porcelain + OUTPUT_VARIABLE fp_status + OUTPUT_STRIP_TRAILING_WHITESPACE + ERROR_QUIET) + set(fp_status_filtered "") + if(NOT fp_status STREQUAL "") + string(REPLACE "\n" ";" fp_lines "${fp_status}") + foreach(fp_line IN LISTS fp_lines) + string(STRIP "${fp_line}" fp_line) + if(fp_line STREQUAL "" OR fp_line MATCHES "${STAMP_NAME}$") + continue() + endif() + string(APPEND fp_status_filtered "${fp_line}\n") + endforeach() + endif() + execute_process( + COMMAND "${GIT_EXECUTABLE}" -C "${LLAMA_SRC}" diff --no-color + OUTPUT_VARIABLE fp_diff + ERROR_QUIET) + string(SHA256 fp_hash "${fp_status_filtered}${fp_diff}") + set(${out_var} "${fp_hash}" PARENT_SCOPE) +endfunction() + # --------------------------------------------------------------------------- # Build the manifest: the checked-out commit plus every patch's content hash. # Any llama.cpp version bump changes HEAD; any patch edit changes a hash. +# The tree fingerprint is appended after applying (it cannot be known before). # --------------------------------------------------------------------------- set(manifest "head ${llama_head}\n") foreach(patch IN LISTS patch_files) @@ -147,7 +192,9 @@ if(NOT tree_is_dirty) foreach(patch IN LISTS patch_files) apply_one_patch("${patch}") endforeach() - file(WRITE "${stamp_file}" "${manifest}") + # Fingerprint the result, not the intent: this is what a later reconfigure compares against. + compute_tree_fingerprint(applied_fingerprint) + file(WRITE "${stamp_file}" "${manifest}tree ${applied_fingerprint}\n") return() endif() @@ -155,10 +202,13 @@ endif() # Dirty tree: already patched. Only a stamp matching this exact commit + patch # set proves the modifications are ours and complete. # --------------------------------------------------------------------------- +compute_tree_fingerprint(current_fingerprint) +set(expected_stamp "${manifest}tree ${current_fingerprint}\n") + set(stamp_matches FALSE) if(EXISTS "${stamp_file}") file(READ "${stamp_file}" stamp_content) - if(stamp_content STREQUAL manifest) + if(stamp_content STREQUAL expected_stamp) set(stamp_matches TRUE) endif() endif() @@ -173,7 +223,9 @@ message(FATAL_ERROR "apply-llama-patches: ${LLAMA_SRC} has local modifications that do not match the current " "patch set.\n" " Patches cannot be applied on top of an already-patched tree, and the previous state is " - "unknown (the tree was patched with a different patch set or llama.cpp commit, or edited by " - "hand).\n" + "unknown: the tree was patched with a different patch set or llama.cpp commit, or one of the " + "patched files was reverted or edited by hand (the stamp records a fingerprint of the tree's " + "modifications, so a single reverted file lands here rather than silently building unpatched " + "code).\n" " Configure into a FRESH build directory so FetchContent re-checks-out a pristine " "llama.cpp, then build again.") diff --git a/llama/spotbugs-exclude.xml b/llama/spotbugs-exclude.xml index 385440973..0858fe282 100644 --- a/llama/spotbugs-exclude.xml +++ b/llama/spotbugs-exclude.xml @@ -365,20 +365,6 @@ SPDX-License-Identifier: MIT - - - - - - -