Skip to content
PR Quorum
Sign inStart free
← Review gallery
ggml-org/llama.cpp #25544openreviewed 6h ago

common : add support for multiple end sequences in the reasoning budget sampler

by aldehir+497 25414 filesreviewed in 2m 8sView PR on GitHub
needs attentionThe panel surfaced issues that deserve attention before merge.2 findings · 1 high+

Reviewer panel

Correctness
deepseek/deepseek-v4-flash
errored
Security
deepseek/deepseek-v4-flash
errored
Architecture
deepseek/deepseek-v4-flash
2 found

Findings

01Hhighconf 85%

reasoning_budget_message handler depends on unreliably ordered field processing

tools/server/server-schema.cpp

The reasoning_budget_message handler now reads ctx.params.sampling.reasoning_budget_end.front() to build the forced token sequence. This vector is populated by the reasoning_budget_end_tags handler (or its alias). If the JSON object is processed in key order (which is not guaranteed per the JSON standard), the reasoning_budget_message handler may execute before the end-tags handler, resulting in an empty reasoning_budget_end and thus a missing forced sequence. The previous code was safe because it used the raw JSON string for reasoning_budget_end_tag directly, without relying on another handler's side effect.

02Mmedconf 75%

Aho-Corasick matcher resets state on match, losing longer overlapping patterns

common/reasoning-budget.cpp

The token_matcher::advance method calls ac.next(state, token) and if match_pattern(state) returns >= 0, it resets state = 0. This prevents detection of patterns that are a strict prefix of another pattern (e.g., end sequences {101} and {101, 102}). After matching the shorter sequence, the state is reset, so the next token cannot continue into the longer sequence. While the current end sequences (e.g., " response" and "<tool_call>") are not prefixes of each other, this is a latent bug that may appear when future models require overlapping end tags. The original single-sequence matcher had the same limitation, but now that multiple sequences are supported, this design decision becomes more likely to cause incorrect behavior.

Want this on your own pull requests?
The same reviewer panel runs on every PR — one focused review, no noise.
Start freeLive demo