Skip to content

fix edge case for repetition count threshold - #28311

Closed
SergeySklyarov wants to merge 1 commit into
ggml-org:masterfrom
SergeySklyarov:fix-repetition-threshold
Closed

SergeySklyarov wants to merge 1 commit into
ggml-org:masterfrom
SergeySklyarov:fix-repetition-threshold

Conversation

@SergeySklyarov

Copy link
Copy Markdown
Contributor

Overview

Fixes an edge case where a repetition count of exactly 2000 was rejected by the parser, despite values above 2000 being handled as unbounded. Repetition counts equal to the threshold of 2000 are now accepted, while nested repetitions exceeding the expansion limit remain restricted.

Additional information

This PR revisits the fix from #27177 (which stalled in Draft due to template and AI disclosure requirements) and presents the solution along with tests.

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: YES — Codex assisted with implementation and test preparation; I reviewed the change and ran the parser test locally.

@github-actions github-actions Bot added the testing Everything test related label Sep 3, 2026
@ggml-gh-bot

ggml-gh-bot Bot commented Sep 3, 2026

Copy link
Copy Markdown

Hi @SergeySklyarov, thanks for your contribution!

Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:

  • Multiple open PRs from a new contributor: We limit new contributors (those without a previously merged PR) to 1 open PR at a time. You currently have 2 open PRs.

  • AI-generated content: While code is allowed to be generated by AI, please write the PR description and commit messages on your own without the help of AI.


Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below.

@SergeySklyarov

Copy link
Copy Markdown
Contributor Author

Hi @ggerganov could you please check this PR when you have time?

@SergeySklyarov

Copy link
Copy Markdown
Contributor Author

Hi @ggerganov, following up on this PR. It has no merge conflicts and #28279 has already been merged. Could you please review it and approve the pending CI workflows when convenient?

@ggerganov

Copy link
Copy Markdown
Member

Hm, we made this change recently in #28469. cc @aldehir

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing Everything test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants