From e2a0af7427f7f34d41f323c9b478c65866d0c448 Mon Sep 17 00:00:00 2001 From: Glenn Neuber Date: Fri, 18 Sep 2026 17:40:51 +1000 Subject: [PATCH 1/2] docs(compat): 004 is not redundant at b10864, and why MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The v0.34.1 fold moved llama.cpp b10630 -> b10864, and at b10864 upstream changed its own gemma4 vision defaults from set_limit_image_tokens(40, 280) to (70, 1120) — exactly the endpoints of the ladder 004 enforces. That invites the conclusion that the patch has been upstreamed. It has not, and the next payload bump will invite it again, so the answer belongs in the file rather than in a thread. Checked at b10864: - Those bounds are inputs to calc_size_preserved_ratio, which rounds to align and then clamps down over max_pixels or scales up under min_pixels. It neither snaps to the ladder nor fills to the budget: an under-budget image keeps its natural rounded grid. 004 replaces that call for gemma4 with calc_size_budget_fill. Upstream changed the bounds fed to an algorithm; the patch changes the algorithm. - 004 never touches set_limit_image_tokens. That line appears in the patch only as context, which is also why the patch still applies cleanly. - The fork passes --image-min-tokens 70 --image-max-tokens 1120 explicitly on every gemma4 launch, so the payload's own defaults never bound anything here regardless of what they are. Also records that the MLX path does not use this patch at all — the mlx-metal profiles carry patchset = [] — and gets the same geometry from llm.BudgetFillSize, which must stay in lockstep with the C++ function. Changing one and not the other splits GGUF and MLX onto different grids, and only the GGUF half is covered by this patch's tests. No behaviour change. go test ./llm/ gemma4 budget tests pass. Co-Authored-By: Claude Opus 5 --- llama/compat/README.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/llama/compat/README.md b/llama/compat/README.md index 0ae14dd5812..5b3be29d54f 100644 --- a/llama/compat/README.md +++ b/llama/compat/README.md @@ -37,6 +37,32 @@ intentionally skipped so a developer can iterate on a local llama.cpp tree. measurably break `box_2d` vertical grounding; see `docs/maxusai/gemma4-bbox-investigation-findings.md`. Makes `--image-min-tokens` a no-op for gemma4. + + **Upstream converging on 70/1120 is not redundancy (checked 2026-09-18, at + b10864).** llama.cpp moved its own gemma4 defaults from + `set_limit_image_tokens(40, 280)` to `(70, 1120)` at b10864 — exactly the + endpoints of the ladder above — which invites the question of whether this + patch can now go. It cannot, and the reason is that the two changes are + orthogonal. Those bounds are inputs to `calc_size_preserved_ratio`, which + rounds each axis to align and then clamps DOWN when over `max_pixels` or + scales UP when under `min_pixels`; it neither snaps to the ladder nor fills + to the budget, so an under-budget image keeps its natural rounded grid. 004 + replaces that call for gemma4 with `calc_size_budget_fill`. Upstream changed + the bounds fed to an algorithm; this patch changes the algorithm. Two further + checks: 004 never touches `set_limit_image_tokens` at all — that line appears + only as patch context — and the fork passes `--image-min-tokens 70 + --image-max-tokens 1120` explicitly on every gemma4 launch, so the payload's + own defaults never bound anything here in the first place. Re-open this + question only if upstream adds ladder snapping or budget filling to the + dyn_size preprocessor itself, not merely because the numbers match. + + **The MLX path does not use this patch.** The `mlx-metal` preflight profiles + carry `patchset = []` because the compat patches do not apply to MLX at all. + Apple Silicon gets the same geometry from `llm.BudgetFillSize` + (`llm/llama_server.go`), a Go mirror of `calc_size_budget_fill` called from + `x/models/gemma4/vision.go`. The two must stay in lockstep: changing one and + not the other silently splits GGUF and MLX onto different grids, and only the + GGUF half is covered by this patch's tests. - `005-llama-cpp-dynres-pinned-overshoot.patch` - shared dyn_size sizing: when a pinned budget (min ~= max) makes the min_pixels ceil overshoot max_pixels, floor just under min instead. The budget is a hard ceiling; measured From 2e359359a8a6532c5c8b19e1e8f06040f29e570c Mon Sep 17 00:00:00 2001 From: Glenn Neuber Date: Fri, 18 Sep 2026 17:54:46 +1000 Subject: [PATCH 2/2] docs(compat): the resize-algo change predates b10864 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A b9888..b10864 diff of the gemma4 branch shows image_resize_algo moving to RESIZE_ALGO_BICUBIC alongside set_limit_image_tokens moving to (70, 1120), which reads as two simultaneous b10864 changes. Only the token limits are. b10630 — the payload behind 0.33.2 — already set BICUBIC, so the resize kernel is identical on both sides of any 0.33.2-vs-0.34.0 comparison, and the MLX path has always used CatmullRom (Keys a=-0.5, the bicubic family) regardless. Also records that 004's context was re-cut for this bump in fb18f5c94, which is why it still applies: at 2b95b4a5 the context read (40, 280) and on main it reads (70, 1120). Documentation only. Co-Authored-By: Claude Opus 5 --- llama/compat/README.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/llama/compat/README.md b/llama/compat/README.md index 5b3be29d54f..fd2936c5bb2 100644 --- a/llama/compat/README.md +++ b/llama/compat/README.md @@ -56,6 +56,16 @@ intentionally skipped so a developer can iterate on a local llama.cpp tree. question only if upstream adds ladder snapping or budget filling to the dyn_size preprocessor itself, not merely because the numbers match. + Two related facts, so the same ground is not re-walked. The patch context + *was* re-cut for this bump (`fb18f5c94`, "re-cut 004 for llama.cpp b10864"), + which is why it still applies: at `2b95b4a5` its context read + `set_limit_image_tokens(40, 280)` and on `main` it reads `(70, 1120)`. + Separately, the gemma4 branch's `image_resize_algo = RESIZE_ALGO_BICUBIC` is + NOT a b10864 change — b10630 already set it, so it predates the 0.33.2 + baseline. A diff of the gemma4 branch spanning b9888..b10864 shows the + resize-algo and token-limit changes together and invites treating both as + new; only the token limits are. + **The MLX path does not use this patch.** The `mlx-metal` preflight profiles carry `patchset = []` because the compat patches do not apply to MLX at all. Apple Silicon gets the same geometry from `llm.BudgetFillSize`