Skip to content

common : replace wrap_for_generation with a prefix convenience function + gpt-oss fixes#20912

Merged
aldehir merged 1 commit intoggml-org:masterfrom
aldehir:tidy-generation-prompt
Mar 24, 2026
Merged

common : replace wrap_for_generation with a prefix convenience function + gpt-oss fixes#20912
aldehir merged 1 commit intoggml-org:masterfrom
aldehir:tidy-generation-prompt

Conversation

@aldehir
Copy link
Copy Markdown
Contributor

@aldehir aldehir commented Mar 23, 2026

Overview

Remove wrap_for_generation function and replace with a prefix() parser that generates a literal up to a given delimiter.

Clean up gpt-oss and fix required tool calling. Injecting the generation prompt greatly simplifies the gpt-oss parser rules. This assumes <|start|>assistant is always a prefix of generation_prompt, which is a reasonable assumption. It will work even if users append extra tokens, i.e. <|start|>assistant<|channel|>analysis<|message|>I am thinking of chocolate<|end|>.

Requirements

cc @pwilkin

@pwilkin
Copy link
Copy Markdown
Member

pwilkin commented Mar 23, 2026

Dunno, the creator of the PEG parser said that it's not a good idea to add more helper builders ;)

@aldehir
Copy link
Copy Markdown
Contributor Author

aldehir commented Mar 23, 2026

I meant inside the core peg parser so it's still useful for general parsing tasks. Inside the chat builder, anything goes!

Sorry for not making that distinction. I do prefer more composable patterns though, for example we're just matching a prefix here.

@pwilkin
Copy link
Copy Markdown
Member

pwilkin commented Mar 23, 2026

Aight, might want to take this out of draft :)

@aldehir
Copy link
Copy Markdown
Contributor Author

aldehir commented Mar 23, 2026

Wanted to run through a few models before removing out of draft, ministral-3 thinks forever.

@aldehir aldehir marked this pull request as ready for review March 23, 2026 20:42
@aldehir aldehir requested a review from a team as a code owner March 23, 2026 20:42
@pwilkin
Copy link
Copy Markdown
Member

pwilkin commented Mar 23, 2026

BTW you might want to take a look at #20844 - I scrapped the previous approach and did the regexp generator for the full pattern, but also expanded the tests to check whether the grammar is actually triggered.

@aldehir aldehir merged commit 312d870 into ggml-org:master Mar 24, 2026
47 of 48 checks passed
Seunghhon pushed a commit to Seunghhon/llama.cpp that referenced this pull request Apr 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants