Repository navigation
docs(design): salvage Gemma 4 vision-budget design notes before branch retirement - #5
Merged
Merged
Conversation
…h retirement Rescues the two design records from feat/gemma4-visual-token-budgets-last-go-runner, the only place they existed, so the branch can be deleted without losing the reasoning behind the shipped feature. Both describe the ORIGINAL approach — implemented against Ollama's Go-native inference runner, which upstream deleted in ollama#16031 (runner/ollamarunner) and ollama#17007 (model/, model/models/gemma4/). That code was never merged and cannot be revived. What shipped instead (PR #2, d06138a) passes --image-min-tokens/--image-max-tokens to llama-server, with different defaults: 40/1120, not the plan's 70/560. Each file therefore gets a HISTORICAL banner up top so neither reads as live guidance. Two inline annotations correct claims that later proved wrong: - The rebase note's "Forward-porting" section concluded a forward-port would require patching C++ mtmd/clip because Gemma ignores the image-token levers. It does not — PR #2 passes them through with no C++ change and prompt_eval_count rises 1,435 -> 2,233, matching the reference server. - Its "Known risk" (unclamped vision position-embedding lookup) is in a file that no longer exists. Flagged moot for the shipped path, while noting the wide-image smoke test it recommends was never actually run against llama-server. Dropped docs/design/PR_BODY.md: submission scaffolding for the closed upstream PR, and it restates the superseded 70/560 defaults as fact. Note: the banners link to docs/maxusai/gemma4-budget-image.md, which arrives in PR #3. Merge that first or these two links dangle. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rescues two design records from
feat/gemma4-visual-token-budgets-last-go-runner— theonly place they exist — so that branch can be retired without losing the reasoning behind
the gemma4 vision-budget work.
Why this is worth keeping
Both documents describe the original approach: implemented against Ollama's Go-native
inference runner, which upstream deleted in two stages (ollama#16031 removed
runner/ollamarunner/, ollama#17007 removedmodel/model.goandmodel/models/gemma4/). Thatcode was never merged and cannot be revived.
What shipped instead — PR #2 (
d06138a9) — passes--image-min-tokens/--image-max-tokenstollama-server, touching onlyapi/types.goandllm/llama_server.go. Different layer, different numbers:ollamarunner)llama-serverflags (C++mtmd){70,140,280,560,1120}What survives the rewrite is the reasoning: the Google visual-token ladder, why a budget
change must force a scheduler reload, the option-naming rationale, and the base-selection
trade-off behind the
-last-go-runnerbranch.Not a verbatim copy
Dropped verbatim into
main, a 422-line implementation plan withtodos: status: pendingfrontmatter reads as live guidance for work that can never be done. So:
docs/design/PR_BODY.md— submission scaffolding for the closed upstream PR,and it restates the superseded 70/560 defaults as fact.
README.mdrewritten as an archive index pointing atdocs/maxusai/for currentbehavior.
Two inline annotations correct claims that later proved wrong. I left the original text
unedited underneath both, so the record stays honest:
main" concluded that a forward-port would requirepatching C++
mtmd/clip, because Gemma supposedly ignores the image-token levers.It doesn't. PR feat(gemma4): tunable per-request vision image-token budget #2 passes them straight through with no C++ change, and
prompt_eval_countgoes 1,435 (~220 image tokens) → 2,233 (~1,015), matching thereference server. The section's closing instinct — "first confirm what the pinned
mtmdactually does for Gemma today" — is exactly what resolved it.model/models/gemma4/model_vision.go, which no longer exists, so it's moot for theshipped path. Annotated as such — while noting the wide/extreme-aspect-ratio smoke test
it recommends has still never been run against
llama-server. The deployed 1120 confighas been exercised on exactly one image.
The banners link to
docs/maxusai/gemma4-budget-image.md, which arrives in #3. Merge thatone first or these two links dangle. No file overlap, so no conflict either way.
Once this lands, both
feat/gemma4-visual-token-budgetsbranches can be deleted — thesecond is a strict superset of the first, and this PR takes everything durable from it.
🤖 Generated with Claude Code