Skip to content

common/chat : fix LFM2/LFM2.5 reasoning round-trip and <think> leak#24234

Open
tdakhran wants to merge 2 commits into
ggml-org:masterfrom
tdakhran:tarek/feat/lfm2-thinking-fix
Open

common/chat : fix LFM2/LFM2.5 reasoning round-trip and <think> leak#24234
tdakhran wants to merge 2 commits into
ggml-org:masterfrom
tdakhran:tarek/feat/lfm2-thinking-fix

Conversation

@tdakhran
Copy link
Copy Markdown
Contributor

@tdakhran tdakhran commented Jun 6, 2026

Overview

Follow-up on review comment #24178 (review) made by @aldehir.

For LFM2/LFM2.5 models, copy reasoning_content into thinking.

LFM2.5-8B-A1B is always a reasoning model. The chat template doesn't have a switch to disable it. This leads to a leak of thinking into content with reasoning disabled (-rea off). (reported here #24178 (comment)).
The workaround is to drop thinking content (as is done for Deepseek).

Additional information

Add more tests, and refactor (unify) test cases for different LFM2/LFM2.5 chat templates.

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: YES, test case refactoring and review.

@tdakhran tdakhran requested review from a team and pwilkin as code owners June 6, 2026 15:05
@github-actions github-actions Bot added the testing Everything test related label Jun 6, 2026
Comment thread common/chat.cpp Outdated
Comment on lines 1672 to 1675
} else if (extract_reasoning) {
// Thinking off, but model may still emit <think>, drop it
reasoning = p.optional(p.literal(THINK_START) + p.until(THINK_END) + p.literal(THINK_END));
}
Copy link
Copy Markdown
Contributor

@aldehir aldehir Jun 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's always parse reasoning but gate it by reasoning format and model (i.e. if the think token exists in the template). Simulating this doesn't gain anything and might seem as if the model is doing nothing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it, pushed one more commit, now reasoning parsing is always enabled if there is <think> in the chat template.

@tdakhran tdakhran requested a review from aldehir June 6, 2026 18:24
@aldehir
Copy link
Copy Markdown
Contributor

aldehir commented Jun 6, 2026

Looks good, I'll wait for CI to pass before giving approval. Thank you for your contributions!

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