fix(sdk): propagate enable_thinking from TextMessages and add set_sampler_repetition_penalty#1967
Open
jm-observer wants to merge 2 commits intoEricLBuehler:masterfrom
Open
fix(sdk): propagate enable_thinking from TextMessages and add set_sampler_repetition_penalty#1967jm-observer wants to merge 2 commits intoEricLBuehler:masterfrom
jm-observer wants to merge 2 commits intoEricLBuehler:masterfrom
Conversation
…pler_repetition_penalty - Fix enable_thinking field not being forwarded when converting TextMessages to RequestBuilder (was always None) - Add RequestBuilder::set_sampler_repetition_penalty replacing the non-existent set_sampler_frequency_penalty used in websocket handler
Code Metrics Report━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Language Files Lines Code Comments Blanks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ C Header 5 305 210 52 43 CSS 2 1181 1036 34 111 CUDA 46 13150 9967 1467 1716 Dockerfile 1 39 22 8 9 HTML 2 235 197 14 24 JavaScript 16 3546 2676 482 388 Jinja2 7 694 656 5 33 JSON 21 409 406 0 3 Makefile 1 6 5 0 1 Metal Shading Lan| 31 11519 8914 1048 1557 PowerShell 1 300 227 30 43 Python 119 8050 6579 405 1066 Shell 2 424 274 92 58 Plain Text 3 3723 0 2413 1310 TOML 27 1290 1124 35 131 YAML 3 25 23 2 0 ───────────────────────────────────────────────────────────────────────────────── Jupyter Notebooks 3 122 83 23 16 |- Markdown 1 60 30 22 8 |- Python 1 122 113 1 8 (Total) 304 226 46 32 ───────────────────────────────────────────────────────────────────────────────── Markdown 98 10470 0 7673 2797 |- BASH 48 708 535 107 66 |- JSON 16 666 666 0 0 |- PowerShell 2 2 2 0 0 |- Python 19 740 563 83 94 |- Rust 46 1826 1519 51 256 |- TOML 6 207 164 0 43 |- YAML 2 9 8 1 0 (Total) 14628 3457 7915 3256 ───────────────────────────────────────────────────────────────────────────────── Rust 524 221388 194812 6095 20481 |- Markdown 339 8370 451 6865 1054 (Total) 229758 195263 12960 21535 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Total 912 289586 231262 27008 31316 ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ |
EricLBuehler
requested changes
Mar 21, 2026
Owner
EricLBuehler
left a comment
EricLBuehler
left a comment
There was a problem hiding this comment.
Hi @jm-observer! Looks good, just 1 small comment?
| .set_sampler_topp(top_p) | ||
| .set_sampler_topk(top_k) | ||
| .set_sampler_max_len(max_tokens) | ||
| .set_sampler_frequency_penalty(rep_penalty); |
Owner
There was a problem hiding this comment.
Why was this removed in favor of set_sampler_repetition_penalty? I think it should be both.
Author
There was a problem hiding this comment.
😅 I’m not sure—this was modified by AI.
Owner
There was a problem hiding this comment.
No worries! Lets just make sure its both . set_sampler_repetition_penalty and . set_sampler_frequency_penalty.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two small but impactful bug fixes in the Rust SDK:
enable_thinkingwas silently dropped when convertingTextMessagesinto
RequestBuilder. TheFrom<TextMessages> for RequestBuilderimplalways set
enable_thinking: None, ignoring whatever the caller hadconfigured on the
TextMessagesinstance. This meant calling.enable_thinking(true/false)on aTextMessageshad no effect.set_sampler_frequency_penaltydoes not exist onRequestBuilder,but the websocket UI handler was calling it. This would fail to compile
unless the method happened to be defined elsewhere. The correct method
for repetition penalty is
set_sampler_repetition_penalty, which is nowadded to
RequestBuilder.Changes
mistralrs/src/messages.rs: Forwardvalue.enable_thinkingin theFrom<TextMessages>conversion instead of hardcodingNone.mistralrs/src/messages.rs: AddRequestBuilder::set_sampler_repetition_penalty(sets
sampling_params.repetition_penalty).mistralrs-cli/src/ui/handlers/websocket.rs: Callset_sampler_repetition_penaltyinstead of the non-existentset_sampler_frequency_penalty.