Repository navigation
fix: pass chat_template_kwargs to the chat template as sent - #260
Merged
solderzzc merged 2 commits intoNov 6, 2026
Merged
Conversation
The field decoded as [String: Bool], so a string or a number anywhere in it answered 400, and every key but enable_thinking was dropped before the template saw it. Templates read other names and types: DeepSeek-V3.1 `thinking`, gpt-oss `reasoning_effort` and `model_identity`, MiniMax-M2 `model_identity`. The kwargs now decode as any JSON value (AnyCodable, as tool parameters already do) and go into the template context whole. enable_thinking keeps its precedence (top-level field, then the kwarg, then --thinking) and must still be a boolean; a request that sends it as anything else gets a 400 naming the param, as the whole field did before. Closes SharpAI#259.
Cancelling the debounce task does not stop a page fetch that is already awaiting the network, and a loadMore() fetch is not in that task at all. The replaced fetch's request fails with URLError.cancelled, which the generic catch turned into errorMessage = "Search failed: cancelled", and the fetch then set isSearching = false and appended its results, all over the search that replaced it. That is the CI flake in HFModelSearchTests.testStrictMLXFilterDisabled: whenever an earlier test leaves a fetch in flight (the pagination test skips mid-fetch under rate limiting), the next search inherits the stale error. Each search bumps a generation; a fetch publishes only while it is the current one, and a cancelled request is not an error to report.
Member
|
Thanks for the fix, and for the detailed write-up in #259. The reproduction table and the list of families that read their own kwarg names made the scope very clear. Reviewing it now. |
This was referenced Nov 6, 2026
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
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.
Closes #259.
chat_template_kwargsdecoded as[String: Bool], so a string or a number anywhere in it answered the whole request with 400, and every key butenable_thinkingwas dropped before the template saw it.enable_thinkingis the Qwen and Gemma name for the thinking switch; other families SwiftLM runs read their own: DeepSeek-V3.1thinking, gpt-ossreasoning_effortandmodel_identity, MiniMax-M2model_identity. None of those could be set per request.What changes
chat_template_kwargsdecodes as any JSON value, through the sameAnyCodablethe tool parameters use, and goes into the template context whole (chatTemplateContext). Nulls inside it are stripped bysanitizeForJinjaon the way to the template, as they already are for tools.enable_thinkingkeeps its precedence exactly as it was: the top-level field, then the kwarg, then--thinking. The Gemma 4 tool-call override is untouched.enable_thinkinginside the kwargs must still be a boolean. A string or a number there is answered with400 invalid_request_errornamingchat_template_kwargs.enable_thinking, which is what the whole field answered before.docs/api-response-formats.mdgain thechat_template_kwargsrow.Requests that send only
enable_thinking, or no kwargs, render the same prompt as before.Verified
ChatRequestParsingTests(4 new): a kwargs object with a boolean, a string, a number and a nested array decodes and keeps its types; an absent field staysnil;chatTemplateContextcarries every kwarg and setsenable_thinkingto the decided value; with no kwargs the context holdsenable_thinkingalone. The decode test is red onmain(Expected to decode Bool but found a string instead).tests/test-contract.shgained block 17: string, number, boolean and nested kwargs are accepted (red onmain: 400), and a non-booleanenable_thinkingis still a 400 that names the param.swift test --filter SwiftLMTests: 342 tests, 0 failures.tests/test-contract.shagainstmlx-community/Qwen2.5-0.5B-Instruct-4bit(M1 Max, macOS 27.2): 28 passed, 0 failed, 2 skipped (the two capability skips for the tiny model).Second commit: the CI flake in
HFModelSearchTestsCI failed on the first push in
SwiftBuddyTests.HFModelSearchTests.testStrictMLXFilterDisabledwithSearch failed: cancelled, the same failure as onmainafter #257, and it does the same on my machine. The cause is inHFModelSearchService: cancelling the debounce task does not stop a page fetch that is already awaiting the network (and aloadMore()fetch is not in that task at all), so the replaced fetch's request fails withURLError.cancelled, the genericcatchrecords it asSearch failed: cancelled, and the fetch goes on to clearisSearchingand append its results, all over the search that replaced it. In the suite that happens whenever an earlier test leaves a fetch in flight, which the pagination test does when it skips under rate limiting.The fix: each
search()bumps a generation, a fetch publishes only while it is the current one, and a cancelled request is not an error to report.testSupersededSearchPublishesNothingpins the scenario, in the shape of its neighbours (live API, skip when rate limited). LocallytestStrictMLXFilterDisabledfailed in 2 of 5 runs onmainbefore the rate limit turned the rest into skips, and passes 5 of 5 with the fix.