Skip to content

support rsplit#24

Open
f291400 wants to merge 4 commits into
ochafik:mainfrom
f291400:main
Open

support rsplit#24
f291400 wants to merge 4 commits into
ochafik:mainfrom
f291400:main

Conversation

@f291400

@f291400 f291400 commented Dec 29, 2025

Copy link
Copy Markdown

No description provided.

@f291400

f291400 commented Dec 29, 2025

Copy link
Copy Markdown
Author

@ochafik please check again

@ochafik ochafik left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Hi @f291400 , thanks for opening this PR

The tests seem to cover something different than rsplit. Can you drop those changes and add a test for rsplit in test-syntax, and also mention in which template you've seen this? (if this unlock a new template, can you add it to the tested model ids in tests/CMakeLists.txt ?)

As for the thinking tests, I'm preparing an overhaul of thinking patterns in templates and their testing, stay tuned!

@f291400

f291400 commented Dec 30, 2025

Copy link
Copy Markdown
Author

@ochafik Currently, none of the open-source large language models use rsplit; this is used in the custom YouTu-LLM model, which will be open-sourced soon.
I just deleted the relevant test code, please check it again.

Comment thread tests/test-syntax.cpp Outdated
@f291400

f291400 commented Jan 1, 2026

Copy link
Copy Markdown
Author

Could you kindly review the current submission?

@CISC

CISC commented Jan 3, 2026

Copy link
Copy Markdown
Collaborator

@ochafik gentle ping

Comment thread include/minja/minja.hpp Outdated
Comment thread include/minja/minja.hpp Outdated
Comment thread include/minja/minja.hpp Outdated
Co-authored-by: Sigbjørn Skjæret <sigbjorn.skjaeret@scala.com>
@CISC

CISC commented May 9, 2026

Copy link
Copy Markdown
Collaborator

@ochafik Oh hey, where have you been? :)

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.

3 participants