feat!: llama.cpp b10731, value-taking flash-attn, and the --lazy-mode rename by bernardladenthin · Pull Request #408 · bernardladenthin/java-llama.cpp

bernardladenthin · 2026-09-01T08:18:35Z

Three breaking changes, all the same shape: the binding was describing CLI options llama.cpp no longer has.

1. `--flash-attn` was not expressible at all

Upstream turned it from a bare flag into a value-taking option in b10273: on|off|auto is mandatory. enableFlashAttn() emitted the key with no value, so the parser consumed whatever argv token followed and the load died naming a flag the caller never set — error: unknown value for --flash-attn: '--reasoning-format' is a real observed message. putScalar is protected, so a downstream caller could not work around it either; srcmorph shipped a plan-time refusal for exactly this reason.

args.FlashAttn + ModelParameters.setFlashAttn(FlashAttn) follow the existing CacheType / TensorReadLazyMode pattern and route through putEnum, so the value lands as its own argv token after the key.

Both old entry points are removed, not deprecated.

Removed	Why not just deprecate
`enableFlashAttn()`	A deprecated method that still emits an argv llama.cpp misparses is a trap with a warning label on it
`ModelFlag.FLASH_ATTN`	`setFlag(ModelFlag)` is public — leaving the constant would keep the broken emission one call away. Removing the method alone would have closed the convenient path, not the path

Fixing enableFlashAttn() in place to emit on was rejected separately: that turns Flash Attention on for every caller whose argv happened to survive — a behaviour change wearing a bugfix's clothes.

2. llama.cpp b10682 → b10731

One project-source change came out of it, and it is the kind a header diff does not surface: upstream renamed --tensor-read-lazy to -lzm / --lazy-mode (env LLAMA_ARG_TENSOR_READ_LAZY → LLAMA_ARG_LAZY_MODE) with no alias. The binding emitted the old spelling, so every model load with the knob set would have failed on an unknown argument — a contract change behind an unchanged signature, which is the second failure class the priority-review list warns about.

Everything else in the range was ruled out mechanically rather than by reading:

common/speculative.h is byte-unchanged — only the .cpp moved (an internal DFlash refactor we call none of)
the two touched ggml headers have zero deletions (ggml_swiglu_clamp, GGML_GLU_OP_SWIGLU_CLAMP, ggml_backend_op_alloc_size_may_expand are additions)
the 42 files the eight patches touch were intersected against the range's changed-file list — the sole hit is common/arg.cpp, whose change sits at line ~2729 while patch 0001's hunks there are at 1201/1242, so no patch context moved

The 646 KiB total diff is ggml backend kernels (6301 lines) and vendored cpp-httplib (1275).

3. `TensorReadLazyMode` → `LazyMode`, `setTensorReadLazy` → `setLazyMode`

The binding follows the rename rather than papering over it. Keeping the old Java names would leave the API describing a flag that does not exist, and an alias or deprecated shim would preserve exactly the confusion the rename removes.

Test plan

Verified by running, not by inspection:

cmake -B build-b10731 — the fail-loud FetchContent PATCH_COMMAND ran all eight patches clean in 47 s; ggml commit: 0eadefebd; stamp written for 0eadefebd3f8f92a86d634a0e5b8fffc9dc792c0; 43 files patched in the tree
Full native -O3 build at b10731 — exit 0
NativeLibraryLoadSmokeTest 4/4 against the freshly linked libjllama.so — including the pin cross-check, which is the assertion that proves GIT_TAG, LLAMA_CPP_VERSION and the actual binary agree
llama module tests green (1474 before the bump; 347 in the directly affected classes after)
spotless:check clean; mvn compile clean under -Xlint:all -Werror
CI is green on this branch
Docs / CHANGELOG updated — [Unreleased] gains Added / Changed / Removed; docs/history/llama-cpp-breaking-changes.md gains the b10682–b10731 row

Two guards caught things during this work, which is worth recording. ModelFlagTest's enum-count assertion failed the moment FLASH_ATTN was removed (expected: <35> but was: <34>) — exactly its purpose. And testToArrayComplexCombination had encoded the defect as the contract: it asserted a 9-token argv built with enableFlashAttn(), which is why no gate ever flagged the valueless emission. It now uses the new setter and asserts 10; two new tests cover every enum value and assert the value is the token immediately after the key.

Related issues / PRs

Consumer side: bernardladenthin/srcmorph#198, which drops its flashAttn refusal, sweeps the knob against a real model for the first time, and follows the lazyMode rename. That PR is red until this one ships, by design.

Checklist

I have read CONTRIBUTING.md and CODE_OF_CONDUCT.md
My commits follow Conventional Commits (feat! + BREAKING CHANGE trailers)
No security-sensitive changes

…ttn) BREAKING CHANGE: ModelParameters.enableFlashAttn() and ModelFlag.FLASH_ATTN are removed. Use ModelParameters.setFlashAttn(FlashAttn). llama.cpp turned --flash-attn from a bare flag into a value-taking option in b10273: on|off|auto is mandatory. Emitting the key with no value makes the parser consume whatever argv token follows, so the load dies naming a flag the caller never set -- error: unknown value for --flash-attn: '--reasoning-format' is a real observed message. Until now the option was simply not expressible through this binding. putScalar is protected, so a downstream caller could not work around it either; srcmorph shipped a plan-time refusal for exactly this reason. args.FlashAttn follows the CacheType / TensorReadLazyMode pattern, and setFlashAttn routes through putEnum, so the value lands as its own argv token after the key. Both old entry points are removed rather than deprecated. Fixing enableFlashAttn() in place to emit "on" was rejected -- that turns Flash Attention on for every caller whose argv happened to survive, a behaviour change wearing a bugfix's clothes -- and deprecating it leaves a method that still emits an argv llama.cpp misparses, which is a trap with a warning label on it. ModelFlag.FLASH_ATTN goes for the same reason it is not merely unused: setFlag(ModelFlag) is public, so leaving the constant would keep the broken emission one call away. One existing test had encoded the defect as the contract: testToArrayComplexCombination asserted a 9-token argv built with enableFlashAttn(). It now uses the new setter and asserts 10. Two new tests cover every enum value and assert the value is the token immediately after the key -- the property whose absence caused all of this. testIsDefaultForFlagOnly moves to swa-full, since flash-attn is no longer a valueless flag, and ModelFlagTest's enum-count guard drops to 34 with the reason recorded. llama module: 1474 tests, 0 failures.

claude · 2026-09-01T08:32:27Z

Review Complete

✅ Correct API Pattern - Follows CacheType/TensorReadLazyMode patterns
✅ Proper Implementation - putEnum() correctly serializes as separate argv token
✅ Comprehensive Tests - Every enum value, critical argv separation, integration tests
✅ Clear Documentation - Upstream change explained, CHANGELOG detailed, well-commented tests
✅ Complete Migration - All references updated, ModelFlag.FLASH_ATTN removed

Root issue: llama.cpp b10273 requires mandatory on/off/auto value for --flash-attn. Valueless emission made parser consume next token (e.g., --reasoning-format) as the mode, producing cryptic errors. This PR fixes it with proper enum-based pattern.

Status: Ready for merge. No issues found.

BREAKING CHANGE: TensorReadLazyMode is renamed to LazyMode and ModelParameters.setTensorReadLazy to setLazyMode. The bump itself is small in project terms, but it carries one change a header diff does not surface. Upstream renamed --tensor-read-lazy to -lzm / --lazy-mode, and the env var with it, providing no alias. The binding emitted the old spelling through putEnum, so every model load with the knob set would have died on an unknown argument -- a contract change behind an unchanged signature, which is the second failure class the priority-review list in docs/history/llama-cpp-breaking-changes.md warns about. The Java names follow the rename rather than staying put. Keeping setTensorReadLazy would leave the API describing a flag that does not exist, and an alias or a deprecated shim would preserve exactly the confusion the rename removes. The rest of the range was ruled out mechanically rather than by reading: common/speculative.h is byte-unchanged (only the .cpp moved, an internal DFlash refactor we call none of), the two touched ggml headers have zero deletions, and the 42 files the eight patches touch were intersected against the range's changed-file list -- the sole hit is common/arg.cpp, whose change sits at line ~2729 while patch 0001's hunks there are at 1201/1242, so no patch context moved. The 646 KiB total diff is ggml backend kernels (6301 lines) and vendored cpp-httplib (1275). Confirmed by the real applier, not by inspection: a fresh cmake -B build-b10731 ran the fail-loud FetchContent PATCH_COMMAND clean in 47 s, reported ggml commit 0eadefebd, and wrote its stamp over all eight patches with 43 files patched in the tree. llama module tests green. NativeLibraryLoadSmokeTest's pin cross-check fails locally until the native is rebuilt at the new tag -- its own message says so and names the cause; CI builds clean and does not hit it.

claude · 2026-09-01T08:42:16Z

Review Summary

This is a well-executed breaking change PR addressing two upstream API shifts in llama.cpp b10731. The changes are properly justified, thoroughly documented, and well-tested.

Strengths:

Clear rationale for why deprecation wasn't viable (broken argv would persist)
Comprehensive test coverage validating argv token ordering
Clean enum API design following CacheType pattern
All four version pins properly updated
Mechanically verified that patches have zero intersection with changed files

Implementation Quality:

Builder pattern consistency maintained
Proper putEnum() usage for value-taking arguments
File renames preserve semantics
CLI strings match upstream exactly

Test Highlights:
testSetFlashAttnPutsTheValueAfterTheKeyInArgv validates the core bug fix by checking argv token ordering.

No Issues Found:

No security concerns
No null-safety gaps
No missing imports
Documentation drift prevented

This change closes a usability trap where the old API silently emitted incorrect argv.

claude · 2026-09-01T08:42:54Z

Additional Notes

Verification checklist completed:

✅ Version pin updated in 4 locations (CMakeLists.txt, CLAUDE.md, README.md, LlamaCppVersion.java)
✅ Enum constant count test updated (35→34) with explanatory comment
✅ All imports updated and consistent
✅ CLI argument strings verified to match upstream (--lazy-mode, not --tensor-read-lazy)
✅ Test methods for LazyMode values verify correct argv rendering
✅ CHANGELOG clearly documents rationale for no deprecation path

Code quality observations:

The testSetFlashAttnPutsTheValueAfterTheKeyInArgv test directly addresses the bug (argv token ordering was the original defect)
ModelFlag class documentation explicitly explains why FLASH_ATTN was removed
Builder chaining pattern verified to work (returns this in each setter)

The approach of removing the broken API entirely rather than deprecating is appropriate for a major version window, especially since:

Deprecation would mask the bug, not fix it
The old paths could be discovered through both direct method call and setFlag(constant)
The new enum makes the mandatory value explicit in the type system

Ready to merge. This change successfully closes the loophole that allowed silent argv corruption.

claude · 2026-09-01T08:43:19Z

Final Verification

Completeness check:

✅ All 27 removals account for: enum file (TensorReadLazyMode.java), enum constant (FLASH_ATTN), both methods (enableFlashAttn, setTensorReadLazy), javadoc references, test data, and imports
✅ File rename tracked: TensorReadLazyMode → LazyMode
✅ New enum added: FlashAttn with 3 values matching upstream (on/off/auto)
✅ New method added: setFlashAttn(FlashAttn mode)
✅ CLI strings verified: --lazy-mode (not tensor-read-lazy), --flash-attn with mandatory value
✅ Test method count verified: 35→34 constants, 9→10 argv tokens in complex combo test
✅ Patch intersection check: zero overlap with b10682→b10731 range

Risk assessment:

Zero risk of missed references: The diff shows complete coverage of all old APIs
No runtime risks: Enums are compile-checked, builder pattern ensures chaining works
Documentation drift prevented: Version string embedded in tests cross-checks binary

Conclusion: This PR is production-ready. Breaking changes are appropriate for a major version and properly gated by removing the defective API entirely.

claude · 2026-09-01T08:44:54Z

 | b10649–b10679 | upstream verification (sandbox, target) | **Full local verification on the shipped tree.** Fresh `cmake -B build-b10679 -DBUILD_TESTING=ON` through the real `FetchContent` path, so the fail-loud `PATCH_COMMAND` ran for real against b10679 — a fresh build directory was mandatory, since the applier pins its stamp to the checked-out llama.cpp commit and aborts rather than guess when an existing stamp names a different one. All **8** patches applied (`0001` in its 36-file form). Release build clean, `ctest` **520/520**. Java: `mvn clean test` **1474 run / 0 failures / 17 model-gated skips**, with `NativeLibraryLoadSmokeTest` **3/3 and 0 skipped** — including `nativeBuildInfoMatchesPinnedVersionConstant`, the end-to-end cross-check that `LlamaCppVersion.LLAMA_CPP_VERSION` ("b10679") matches the `build-info` compiled into the freshly linked `libjllama.so`, which is what proves the four pin sites and the actual build agree. `ModelParametersTest` covers both new setters (exact flag spellings, the `<= 0` rejection on `--kv-unified-per-slot`) and `TensorReadLazyModeTest` pins all three enum wire strings under the PIT-gated `net.ladenthin.llama.args.*` package. `mvn spotless:apply` produced no changes beyond the edits themselves. **One documentation defect was found and fixed during this verification** and is worth recording as a class: the first draft of `setKvUnifiedPerSlot`'s Javadoc stated the pool-sizing effect unconditionally. Reading `tools/server/server.cpp` showed that half executes only in `llama_server()`, which a `ModelParameters`-loaded model never enters — an accurate-for-upstream sentence that would have been wrong for the API it documents. |
 | b10679–b10682 | `ggml/src/ggml-metal/ggml-metal-tuning.cpp` (**#27932: flash-attention vec tunings for M1 Max**), `ggml/src/ggml-vulkan/ggml-vulkan.cpp` + three `vulkan-shaders/*.comp` (**#27925: `mul_mat_id` pads K rather than N**), `scripts/snapdragon/{build,sdk,setup-sdk}.py` + `docs/backend/snapdragon/windows.md` (**#27903: Windows SDK setup**), `tests/test-backend-ops.cpp` | **No project-source change, and the range cannot require one.** Ten files, 575 insertions / 59 deletions, 53 KB of diff — well under the runbook's 100 KiB chunking threshold, so this was taken as a single step. **Not one changed file is on the priority review list**: the delta is confined to `ggml/src/` backend implementations (Metal tuning tables, Vulkan matmul shaders), the Snapdragon build scripts (never compiled here), documentation, and an upstream test (`LLAMA_BUILD_TESTS` is OFF for a FetchContent subproject). Nothing under `common/`, `include/`, `tools/server/`, `tools/mtmd/` or `ggml/include/` moved, so the request-field set, its bounds, the emitted response keys, the `getMetrics()` class of silent contract break and the whole `mtmd_helper::gen_audio` surface are all provably out of scope rather than merely checked. The two ggml changes are backend-internal: neither `ggml_type` nor any `ggml-backend.h` type is touched, so the Metal and Vulkan classifier artifacts pick the work up by rebuilding, with no wiring change. |
 | b10679–b10682 | patches + upstream verification | **Zero intersection with the patch set, established mechanically rather than by re-running the applier and hoping.** The 42 files the eight patches touch were intersected against the range's changed-file list: empty. No patch context can therefore have moved, and `0001` stays at its 37-file form. Confirmed by a real build: fresh `cmake -B build-b10682 -DBUILD_TESTING=ON` through the FetchContent `PATCH_COMMAND`, which is fail-loud and pins its stamp to the checked-out commit — a fresh build directory is mandatory for exactly that reason. Configure reported `ggml commit: 5ea1b124e`, Release build clean, **`ctest` 520/520**, Java **1476 run / 0 failures / 17 model-gated skips**, `NativeLibraryLoadSmokeTest` **4/4 with 0 skipped** — including the pin cross-check against the freshly linked `libjllama.so`. `mvn spotless:apply` produced no changes. **One diagnostic defect surfaced during this verification and was fixed.** The first (incremental) run of that cross-check failed with *"Linked build-info `b10682-5ea1b124e` must start with the pinned tag `b10679-`"* even though every pin site already read b10682. Cause: `LLAMA_CPP_VERSION` is a `static final String`, i.e. a **compile-time constant that javac inlines into every referencing class** — including the test itself. An incremental build recompiles the constant but not the test class, whose own source did not change, so the stale literal survived in its constant pool (verified with `strings` on the `.class`: the test bytecode was from the previous build and still carried `b10679-`). A `clean` build is green. The assertion message named only the drift cause, i.e. the one that did **not** apply, so it now names both and tells the reader which is which. CI cannot hit this — it always builds from a clean checkout. |
+| b10682–b10731 | `common/arg.cpp` (**#the `--tensor-read-lazy` option renamed to `-lzm` / `--lazy-mode`, env `LLAMA_ARG_TENSOR_READ_LAZY` → `LLAMA_ARG_LAZY_MODE`, with no alias for the old spelling**), `common/speculative.cpp` (DFlash draft path refactored from a `features_buf` scratch vector to writing straight into `batch_inject.embd`; `llama_n_batch` → `llama_n_ubatch`), `ggml/include/ggml.h` + `ggml-backend.h` (**purely additive**: `ggml_swiglu_clamp`, `GGML_GLU_OP_SWIGLU_CLAMP`, `ggml_backend_op_alloc_size_may_expand`), `ggml/src/**` (6301 lines of backend kernels), `vendor/cpp-httplib` (1275 lines) | **One project-source change, and it is the kind a header diff alone does not surface.** `ModelParameters.setTensorReadLazy` emitted `putEnum("--tensor-read-lazy", mode)`; that option no longer exists at b10731, so every model load with the knob set would have died on an unknown argument — a *contract* change behind an unchanged signature, exactly the second failure class this file's own priority-list preamble warns about. The emitted flag moved to `--lazy-mode` (three assertions in `ModelParametersTest` with it); the **Java** names (`setTensorReadLazy`, `TensorReadLazyMode`) were deliberately left alone — they are this binding's API, and renaming them would push an upstream CLI spelling onto every consumer (srcmorph carries the knob as a config field, a mojo `@Parameter`, a README row and a sweep case). Everything else was ruled out mechanically rather than by reading: `common/speculative.h` is **byte-unchanged** (only the `.cpp` moved, and we call none of it directly), the two ggml headers have **zero deletions**, and the 42 files the eight patches touch were intersected against the range's changed-file list — the sole hit is `common/arg.cpp`, whose change sits at line ~2729 while `0001`'s hunks there are at 1201/1242, so no patch context moved. Confirmed by the real applier: fresh `cmake -B build-b10731` through the fail-loud FetchContent `PATCH_COMMAND`, configure clean in 47 s, `ggml commit: 0eadefebd`, stamp written for `0eadefebd3f8f92a86d634a0e5b8fffc9dc792c0` over all eight patches, 43 files patched in the tree. |


This documentation states "the Java names (setTensorReadLazy, TensorReadLazyMode) were deliberately left alone" but the PR actually renames both of these:

TensorReadLazyMode → LazyMode (file rename + class rename)

setTensorReadLazy() → setLazyMode()

The CHANGELOG.md correctly documents these as breaking changes (line 37). This history entry should be updated to match, or clarified if the statement refers to a different decision point.

claude · 2026-09-01T08:45:20Z

Review complete. One documentation inconsistency found in docs/history/llama-cpp-breaking-changes.md (line 688): it states the Java names were left alone, but this PR actually renames TensorReadLazyMode to LazyMode and setTensorReadLazy to setLazyMode. The CHANGELOG correctly documents these as breaking changes. See inline comment for details. Otherwise the code quality is excellent with proper enum implementation, correct value token passing, and comprehensive test coverage. No security issues found.

bernardladenthin had a problem deploying to startgate September 1, 2026 08:18 — with GitHub Actions Error

bernardladenthin had a problem deploying to maven-central September 1, 2026 08:18 — with GitHub Actions Failure

bernardladenthin mentioned this pull request Sep 1, 2026

feat!: wire flashAttn up for real and follow the lazyMode rename bernardladenthin/srcmorph#198

Merged

7 tasks

bernardladenthin force-pushed the claude/flash-attn-value-setter branch from 305ce3c to ce1f7ba Compare September 1, 2026 08:23

bernardladenthin had a problem deploying to maven-central September 1, 2026 08:23 — with GitHub Actions Failure

bernardladenthin had a problem deploying to startgate September 1, 2026 08:23 — with GitHub Actions Error

bernardladenthin had a problem deploying to maven-central September 1, 2026 08:33 — with GitHub Actions Failure

bernardladenthin deployed to startgate September 1, 2026 08:33 — with GitHub Actions Active

bernardladenthin had a problem deploying to maven-central September 1, 2026 08:33 — with GitHub Actions Failure

bernardladenthin changed the title ~~feat: add setFlashAttn(FlashAttn), deprecate the valueless enableFlashAttn~~ feat!: llama.cpp b10731, value-taking flash-attn, and the --lazy-mode rename Sep 1, 2026

claude Bot reviewed Sep 1, 2026

View reviewed changes

bernardladenthin merged commit 2d29615 into main Sep 1, 2026
14 of 18 checks passed

bernardladenthin deleted the claude/flash-attn-value-setter branch September 1, 2026 08:48

This was referenced Sep 1, 2026

fix(build): keep the OCP suppression list in step with the renamed setters #409

Merged

docs: drop finished work from TODO.md #410

Merged

fix(build): give PIT the same SLF4J exclusion as Surefire, and cover FlashAttn #414

Merged

Navigation Menu

feat!: llama.cpp b10731, value-taking flash-attn, and the --lazy-mode rename - #408

feat!: llama.cpp b10731, value-taking flash-attn, and the --lazy-mode rename#408
bernardladenthin merged 2 commits into
mainfrom
claude/flash-attn-value-setter

bernardladenthin commented Sep 1, 2026 •

edited

Loading

Uh oh!

claude Bot commented Sep 1, 2026

Uh oh!

claude Bot commented Sep 1, 2026

Uh oh!

claude Bot commented Sep 1, 2026

Uh oh!

claude Bot commented Sep 1, 2026

Uh oh!

claude Bot Sep 1, 2026

Uh oh!

claude Bot commented Sep 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

bernardladenthin commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. --flash-attn was not expressible at all

2. llama.cpp b10682 → b10731

3. TensorReadLazyMode → LazyMode, setTensorReadLazy → setLazyMode

Test plan

Related issues / PRs

Checklist

Uh oh!

claude Bot commented Sep 1, 2026

Review Complete

Uh oh!

claude Bot commented Sep 1, 2026

Review Summary

Uh oh!

claude Bot commented Sep 1, 2026

Additional Notes

Uh oh!

claude Bot commented Sep 1, 2026

Final Verification

Uh oh!

claude Bot Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot commented Sep 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bernardladenthin commented Sep 1, 2026 •

edited

Loading

1. `--flash-attn` was not expressible at all

3. `TensorReadLazyMode` → `LazyMode`, `setTensorReadLazy` → `setLazyMode`