Repository navigation
chat : fix reasoning leak with force-opened bare <think> templates - #24674
Conversation
|
Hi @newjordan, thanks for your contribution! Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:
Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below. |
|
Hey there, this is a bug fix on soemthing we found from testing that is upstream of our model that can affect models with the think vs think>n - there is no lazy or gracefu ldefualt there and some models break on loading for users on the Nex2 - this was a deliberate PR, I use AI, but this decision to send this PR was hopefully a courtesy to LLama and I sincrely do not wish to produce noise or bad work. |
|
I confirm that this fixes Nex N2 for me. There's a workaround in the chat template, but changing the template can affect the final performance. Quoting one of N2 devs (https://huggingface.co/nex-agi/Nex-N2-mini/discussions/4#6a2b6c0e143fc2731e78c958):
So this fix appears to be necessary for N2 to work properly in llama.cpp |
|
This should be sufficient for this issue: diff --git a/common/chat-auto-parser-generator.cpp b/common/chat-auto-parser-generator.cpp
index 37ca55c..53bc85c 100644
--- a/common/chat-auto-parser-generator.cpp
+++ b/common/chat-auto-parser-generator.cpp
@@ -147,7 +147,8 @@ common_peg_arena autoparser::build_parser(const generation_params & inputs, cons
} else {
parser = content.build_parser(ctx);
}
- return pure_content ? p.prefix(generation_prompt, reasoning.start) + parser : p.prefix(generation_prompt, reasoning.start) << parser;
+ const std::string reasoning_start = trim_whitespace(reasoning.start);
+ return pure_content ? p.prefix(generation_prompt, reasoning_start) + parser : p.prefix(generation_prompt, reasoning_start) << parser;
});
} |
|
@newjordan can you push the simplified version suggested by @aldehir ? |
|
Applied the patch proposed by @aldehir. Worked fine with the Nex-N2-Mini model. Reasoning is adaptive for this model. So I was able to force it to produce thinking by a logic task[1] in open-webui. No custom templates needed. Also works as expected in OpenCode. [1]
|
|
Hey guys, Yes I will PR now. However, I did some work and ran into this on exact implementation - regresses an existing case: test-chat aborts on Sorry for being obtuse, I try and do a lot of testing before sending anything over and I keep getting an error that if I submit just the short fix, it breaks a nemo config upstream. I will submit just the PR as requested, with this as a notation in case it is an issue. I have the fix to this also prepared. |
c68c44b to
69dc863
Compare
The reasoning start tag inferred from prior turns can carry trailing whitespace (e.g. <think>\n) while a force-open template prefills a bare <think>. Trim the tag used for the prefix split so the bare prefill is matched instead of being swallowed into content.
69dc863 to
461630c
Compare
I'll look into it, but it seems to have surfaced an existing bug with Nemotron Nano v2. |
|
While this is not merged, I've uploaded a chat template workaround that "fools" the autoparser into using the correct delimiter: https://huggingface.co/tarruda/Nex-N2-Pro-GGUF/blob/main/chat_template.jinja#L102-L107 |
pwilkin
left a comment
There was a problem hiding this comment.
All right, since we already do << for the prefix, this won't change semantics, so it should be fine.
…gml-org#24674) * chat : fix reasoning leak with force-opened bare <think> templates The reasoning start tag inferred from prior turns can carry trailing whitespace (e.g. <think>\n) while a force-open template prefills a bare <think>. Trim the tag used for the prefix split so the bare prefill is matched instead of being swallowed into content. * chat : fix Nemotron Nano v2 regression --------- Co-authored-by: Alde Rojas <hello@alde.dev>
…gml-org#24674) * chat : fix reasoning leak with force-opened bare <think> templates The reasoning start tag inferred from prior turns can carry trailing whitespace (e.g. <think>\n) while a force-open template prefills a bare <think>. Trim the tag used for the prefix split so the bare prefill is matched instead of being swallowed into content. * chat : fix Nemotron Nano v2 regression --------- Co-authored-by: Alde Rojas <hello@alde.dev>
…gml-org#24674) * chat : fix reasoning leak with force-opened bare <think> templates The reasoning start tag inferred from prior turns can carry trailing whitespace (e.g. <think>\n) while a force-open template prefills a bare <think>. Trim the tag used for the prefix split so the bare prefill is matched instead of being swallowed into content. * chat : fix Nemotron Nano v2 regression --------- Co-authored-by: Alde Rojas <hello@alde.dev>
…gml-org#24674) * chat : fix reasoning leak with force-opened bare <think> templates The reasoning start tag inferred from prior turns can carry trailing whitespace (e.g. <think>\n) while a force-open template prefills a bare <think>. Trim the tag used for the prefix split so the bare prefill is matched instead of being swallowed into content. * chat : fix Nemotron Nano v2 regression --------- Co-authored-by: Alde Rojas <hello@alde.dev>
…gml-org#24674) * chat : fix reasoning leak with force-opened bare <think> templates The reasoning start tag inferred from prior turns can carry trailing whitespace (e.g. <think>\n) while a force-open template prefills a bare <think>. Trim the tag used for the prefix split so the bare prefill is matched instead of being swallowed into content. * chat : fix Nemotron Nano v2 regression --------- Co-authored-by: Alde Rojas <hello@alde.dev>
…gml-org#24674) * chat : fix reasoning leak with force-opened bare <think> templates The reasoning start tag inferred from prior turns can carry trailing whitespace (e.g. <think>\n) while a force-open template prefills a bare <think>. Trim the tag used for the prefix split so the bare prefill is matched instead of being swallowed into content. * chat : fix Nemotron Nano v2 regression --------- Co-authored-by: Alde Rojas <hello@alde.dev>
Fixes a reasoning leak in the PEG chat auto-parser for templates that force-open thinking by prefilling a bare
<think>(e.g. Nex-N2-mini). The reasoning start tag inferred from prior turns is<think>\n, which the prefix split fails to find in the bare-<think>generation prompt, so the tag is swallowed and the reasoning trace lands incontentinstead ofreasoning_content.Trims the start tag used for the prefix split so the bare
<think>is matched (per @aldehir's suggestion).