[ngxson]

Overview

Move the suppress_tokens logits bias handling to common, so that it can be reused by other models (for example, #26254)

Requirements

• I have read and agree with the contributing guidelines
• AI usage disclosure: yes, pi with glm-5.2

[ngxson]

/bot review

[ggml-gh-bot]

Automated code review

Reviewed pr.diff (4 files: common/sampling.cpp, include/llama.h, src/llama-vocab.cpp, src/models/gemma4.cpp). The change is small and cleanly scoped: it removes the gemma4-specific graph-side logits-bias suppression and relocates it into common_sampler_init, exposing the existing internal vocab.get_suppress_tokens() via a new thin C API.

Will slow the review

(point 1) Behavioral change worth confirming in the description. Previously suppression was applied inside the gemma4 graph, so it was visible in the raw logits returned by llama_get_logits_ith / llama_get_logits. After this PR, suppression only happens inside the common_sampler chain (via llama_sampler_logit_bias). Consequences:

• Tools that read raw logits bypass suppression entirely: the server logprobs path (tools/server/server-common.cpp:1267 llama_get_logits_ith) will now report non--INFINITY logprobs for <image|>/<audio|> tokens, and any low-level caller that does not build its sampler via common_sampler_init (or that runs the logit-bias sampler) silently loses the suppression the gemma4_unified patch added.
• The original gemma4 code even had a TODO: maybe handle this inside the sampling system in the future, so the move looks intentional, but the description should state that raw-logit / logprobs consumers are expected to see unsuppressed logits, and that generation-only suppression is the new contract. A sentence pointing at the motivating discussion (PR mtmd: support Qwen3-TTS (note: breaking change to llama-tts binary) #26254) would help reviewers.

Nits / convention

(point 2) int32_t n_suppress receives tokens.size() (a size_t). It cannot overflow in practice, but per project style add an explicit cast / comment rather than an implicit narrowing, e.g. *n_suppress_tokens = (int32_t) tokens.size(); (already cast) and in sampling.cpp the loop is fine; just keep the cast visible. Minor.

(point 3) Public API addition (include/llama.h llama_vocab_get_suppress_tokens). It's a reasonable, thin accessor over an existing internal method and it's exercised through common_sampler_init into llama-cli/server, which satisfies the "working example" expectation. Per the public-API convention, add a one-line rationale in the PR body (why expose rather than keep internal) so it's not relitigated later. Naming/signature follow conventions (snake_case, sized int32_t * out-param, null-checked). Returning tokens.data() is safe for an empty vector since the caller only dereferences 0..n_suppress.

Security / bounds

(point 4) Attacker-controlled GGUF: suppress_tokens are read straight from GGUF array data (src/llama-vocab.cpp:2578-2581) as raw int32_t, and the loading path does not bound them. The new consumer in sampling.cpp does the right thing: if (0 <= id && id < llama_vocab_n_tokens(vocab)) before indexing, matching the original gemma4 guard, so the moved path itself is safe. (Optional follow-up, out of this PR's scope: the vocab loader at src/llama-vocab.cpp:2581 could also range-check before storing, so future consumers don't have to remember.)

No other correctness issues found. The merged-vector approach is fine even with duplicate ids (logit-bias is additive, -INFINITY + x == -INFINITY), and llama_sampler_init_logit_bias copies the array internally so the temporary merged lifetime is correct. Removed llm_graph_input_logits_bias class has no remaining references.

This review was generated automatically by pi coding agent using zai-org/GLM-5.2. It may contain mistakes. Maintainers make the final call.

[ggerganov]

The common_params_sampling::has_logit_bias() method can now be removed.