[staging CI] unslothai/unsloth#6749 - #301
Closed
danielhanchen wants to merge 86 commits into
Closed
danielhanchen wants to merge 86 commits into
danielhanchen wants to merge 86 commits into
Conversation
…unslothai#6869) * Fix export-time trust_remote_code bypass in FP8/INT8/GGUF-LoRA export The torchao, compressed-tensors, and LoRA GGUF export paths re-read the merged checkpoint and used to set trust_remote_code from the checkpoint config's static auto_map (the torchao path also scanned the staged tokenizer/processor configs). A model that loads with built-in Transformers classes can carry an auto_map entry, which skips the load-time remote-code consent scan (that only runs when the load already requested trust_remote_code) yet flips trust_remote_code on at export, running unvetted custom code. Derive the reload trust_remote_code from the approved load decision instead: a new _loaded_via_remote_code() checks whether the in-memory model / tokenizer was itself loaded from custom code (its class lives in the transformers_modules package), walking PEFT / wrapper layers. Built-in-loaded models no longer gain trust from config metadata; genuine custom-code models (loaded with consent) still reload correctly. Add CPU-only regression tests. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Harden _loaded_via_remote_code against a None/missing __module__ Read type(node).__module__ via getattr and require a string before startswith, so a dynamically created or C-extension class with a None module does not raise during export. Add a regression test. * Split model and tokenizer trust for the compressed subprocess, walk processor components The compressed-tensors export collapsed model and tokenizer trust into one --trust-remote-code flag, so an approved custom tokenizer would have let an unapproved model's custom code run inside the quantization subprocess. The subprocess now takes --trust-remote-code-tokenizer for the processor load and keeps --trust-remote-code for the model loads, matching the torchao path's separate model_trust / tok_trust. _loaded_via_remote_code now also walks processor components (tokenizer, image_processor, feature_extractor, video_processor), so an approved custom tokenizer held inside a built-in ProcessorMixin keeps its trust on the export reload instead of failing with trust_remote_code=False. The walk is a bounded BFS with a seen set so wrapper cycles terminate. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
The setup.ps1 unit-tests job intermittently fails on the windows-latest runner with 'No repository with the name PSGallery was found.' when the default PowerShell Gallery is not registered, so Set-PSRepository throws before Pester can be installed. Register the default gallery first when it is missing, then set its policy and install Pester as before.
… fallback via unsloth_zoo (unslothai#6638)
…unslothai#6738) * GRPO: optional sequence packing for the no-grad old/ref logp path Add an opt-in sequence-packing fast path to _get_per_token_logps_and_entropies, enabled with UNSLOTH_GRPO_SEQ_PACKING=1. When the batch is text-only, the padded [B, Lmax] per-chunk forward is replaced by a single varlen [1, sum L] forward (BlockDiagonalCausalMask via packed_seq_lengths with reset position_ids). Per-token logps use the same float32 chunked_hidden_states_selective_log_softmax as the padded path, so the old and reference logps are bit-for-bit identical. Safety: the packed path is self-verified once against the padded ground truth on a batch that has at least two rows with real completion tokens (self._unsloth_seq_packing_nograd_ok), so cross-sample contamination would actually manifest; a degenerate all-pad / fully tool-masked batch leaves the verdict unset and re-verifies later. If a backend silently ignores packed_seq_lengths (flat batch run under a normal causal mask, samples leaking across boundaries), the packed logps will not match and packing is disabled instead of corrupting logps. It also forces use_cache=False (a populated past_key_value disables varlen packing), skips packing when a sliding window is shorter than the packed stream, runs the same GPT-OSS offload device_synchronize the padded loop uses, and falls back on any exception (UNSLOTH_GRPO_SEQ_PACKING_DEBUG=1 prints the reason). Default off, so existing behavior is unchanged. Pairs with the matching gradient-path change in unsloth_zoo so the full GRPO logp + loss + backward can run packed. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * GRPO no-grad packing: address review feedback - Cache the packed-vs-padded verdict per unwrapped model instead of on the trainer, so a separately forwarded reference model is verified on its own forward path rather than inheriting the policy model's verdict. - Force the padded path when token_type_ids or mm_token_type_ids are present, matching the extra vision kwargs the padded loop forwards. - Require the xformers varlen backend before packing. Without it the packed mask falls back to a dense O(T^2) SDPA mask that can OOM on the flattened batch, so we keep the padded loop in that case. - On any packed-forward failure (missing backend, OOM, unsupported forward) empty the cache on OOM, disable packing for that model, and fall back to the chunked padded loop instead of retrying every step. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * GRPO no-grad packing: default-on, verify against per-row reference Redesign of the optional sequence-packing fast path for the no-grad old/ref logprob recompute, after establishing that the packed forward is the exact per-row computation and the padded batch forward is the side that mis-positions left-padded rows on long completions. - Default the packing on (UNSLOTH_GRPO_SEQ_PACKING, disable with 0). - Verify the packed logprobs against the per-row clean forward (each row's real tokens alone, reset 0-based positions, no padding), not the padded batch which is itself wrong for left-padding. Cross-sample contamination (a backend ignoring packed_seq_lengths) shows up as a large mismatch and falls back to the padded loop. - Make the trust decision shape and RoPE aware: re-verify whenever the packed total length or the longest segment grows past what was verified, so a later batch crossing a LongRoPE short/long cache boundary is re-checked instead of trusted blindly. - Run lm_head only on completion-prediction positions instead of every packed prompt token, so long-prompt/short-completion batches do not pay for projecting the whole packed prompt. - Drop the hard xformers import so the path also runs in FlashAttention-only environments; the per-row verification guards correctness regardless of backend. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * GRPO no-grad packing: disable entirely on cross-sample mismatch When the per-row verification fails, distinguish the two failure modes by magnitude instead of by sequence length: - A large mismatch (>= 1.5) is the cross-sample contamination signature: the model's attention does not honor the block-diagonal packed mask (seen on some MoE / custom-attention models, e.g. qwen2_moe). Disable packing entirely for the model so later batches do not pay the verification cost again. - A moderate mismatch is more likely a length-boundary effect (a LongRoPE short/long cache switch): keep marking just that length region unsafe so packing still runs for smaller shapes. Validated: Qwen1.5-MoE falls back after a single verification (grad and no-grad ok flags go False, no re-verify on later steps); dense Llama-3.2 and Qwen3 still verify and engage packing. * GRPO no-grad packing: trim comments to be concise * GRPO no-grad packing: fix per-row completion boundary for left-padded rows The completion-target selection used a single global boundary (col >= L - logits_to_keep). After left-packing, each row's completion starts at (L - logits_to_keep) - left_pad[row], so for left-padded rows the first left_pad completion tokens fall below the global boundary and were dropped, leaving 0 logprobs at real completion positions that the loss mask keeps. Use the per-row boundary so packed coverage matches create_completion_attention_mask exactly, and widen the self-verify mask to the full per-row completion region so it can catch coverage gaps. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * GRPO no-grad packing: gate verification on real completion rows Count active rows via create_completion_attention_mask (the same mask the loss uses) instead of any non-pad token in the packed window. Prompt-only rows carry prompt-overflow tokens in the window and could otherwise satisfy the >= 2 verification guard, letting a batch with a single real completion row cache a trust decision. This matches the gradient path, which already gates on the completion mask. The same mask is reused for the self-verify comparison. * GRPO no-grad packing: gate debug logging on UNSLOTH_ENABLE_LOGGING Use the shared UNSLOTH_ENABLE_LOGGING global (import_fixes, re-exported by _utils) instead of a bespoke UNSLOTH_GRPO_SEQ_PACKING_DEBUG env var for the packing debug prints, matching the rest of the codebase. * GRPO packing: import UNSLOTH_ENABLE_LOGGING inside the injected logp function _get_per_token_logps_and_entropies is copied verbatim into the generated GRPO trainer via inspect.getsource, and that module never imported UNSLOTH_ENABLE_LOGGING, so the default-on packing verify path raised NameError (and the except handler re-raised it). Import the flag locally, before the try, so the name is defined in the generated module too. Drop it from the now-unused module-level import. * GRPO no-grad packing: harden unsafe-length skip, verify guard, fallback cleanup Three fixes to the no-grad logp packing path, mirroring the grad path: - skip the packed forward for known-unsafe lengths by reading unsafe_T and gating on it before the forward, instead of running the full packed pass and the result build only to discard them (wastes a pass, can OOM at large T) - only widen the verified T/seg envelope when >= 2 completion rows actually exercised cross-sample packing; a < 2 row batch cannot expose leakage, so it must not extend the trusted shape that later multi-row batches skip verify for - drop the packed intermediates (hidden/sel/result/ref) before the padded fallback loop so it does not run with the flattened hidden state still resident * GRPO no-grad packing: cap the flattened forward at one mini-batch budget The packed path built a single [1, sum L] forward over every row before any size check, so a large batch could exceed the memory the padded path bounds per mini-batch. Gate packing on _pk_T <= _pk_cap (B * seq_len, one padded mini-batch's token budget); larger batches fall back to the chunked padded loop. * GRPO no-grad packing: disable unless unsloth_zoo has the masked-column guard The packed path leaves masked prompt/pad logprob columns at 0, which only stays finite if unsloth_zoo grpo_compute_loss zeroes them before exp() (zoo#840). An older unsloth_zoo without that guard would NaN. Detect the guard once (cached on the model) via inspect.getsource and gate packing on it, so unslothai#6738 is safe with any unsloth_zoo version and re-enables packing automatically once a guarded zoo is installed, independent of the pinned lower bound. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * GRPO packing: hoist env gates and zoo-guard detection to one-time module checks Read UNSLOTH_GRPO_SEQ_PACKING and detect the unsloth_zoo masked-column guard once at import time (module constants plus RL_PRE_ITEMS for the generated trainer cache) instead of per call, and drop the in-function UNSLOTH_ENABLE_LOGGING import for a module-top one. The UNSLOTH_GRPO_SEQ_PACKING_VERIFY force-verify debug knob is commented out, kept in place for hand re-enable; the first-use and envelope-growth self-verify stays active. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * GRPO packing: cap the flattened forward by the padded chunk rows B counts chunks at this point, so B * seq_len understated (small runs) or overstated (large runs) the padded mini-batch token budget; use batch_size * seq_len, the rows the padded loop actually forwards per chunk. * GRPO sequence packing: tighten comments * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Lee Jackson <130007945+Imagineer99@users.noreply.github.com>
…completions (unslothai#6871) * GRPO: optional sequence packing for the no-grad old/ref logp path Add an opt-in sequence-packing fast path to _get_per_token_logps_and_entropies, enabled with UNSLOTH_GRPO_SEQ_PACKING=1. When the batch is text-only, the padded [B, Lmax] per-chunk forward is replaced by a single varlen [1, sum L] forward (BlockDiagonalCausalMask via packed_seq_lengths with reset position_ids). Per-token logps use the same float32 chunked_hidden_states_selective_log_softmax as the padded path, so the old and reference logps are bit-for-bit identical. Safety: the packed path is self-verified once against the padded ground truth on a batch that has at least two rows with real completion tokens (self._unsloth_seq_packing_nograd_ok), so cross-sample contamination would actually manifest; a degenerate all-pad / fully tool-masked batch leaves the verdict unset and re-verifies later. If a backend silently ignores packed_seq_lengths (flat batch run under a normal causal mask, samples leaking across boundaries), the packed logps will not match and packing is disabled instead of corrupting logps. It also forces use_cache=False (a populated past_key_value disables varlen packing), skips packing when a sliding window is shorter than the packed stream, runs the same GPT-OSS offload device_synchronize the padded loop uses, and falls back on any exception (UNSLOTH_GRPO_SEQ_PACKING_DEBUG=1 prints the reason). Default off, so existing behavior is unchanged. Pairs with the matching gradient-path change in unsloth_zoo so the full GRPO logp + loss + backward can run packed. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * GRPO no-grad packing: address review feedback - Cache the packed-vs-padded verdict per unwrapped model instead of on the trainer, so a separately forwarded reference model is verified on its own forward path rather than inheriting the policy model's verdict. - Force the padded path when token_type_ids or mm_token_type_ids are present, matching the extra vision kwargs the padded loop forwards. - Require the xformers varlen backend before packing. Without it the packed mask falls back to a dense O(T^2) SDPA mask that can OOM on the flattened batch, so we keep the padded loop in that case. - On any packed-forward failure (missing backend, OOM, unsupported forward) empty the cache on OOM, disable packing for that model, and fall back to the chunked padded loop instead of retrying every step. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * GRPO no-grad packing: default-on, verify against per-row reference Redesign of the optional sequence-packing fast path for the no-grad old/ref logprob recompute, after establishing that the packed forward is the exact per-row computation and the padded batch forward is the side that mis-positions left-padded rows on long completions. - Default the packing on (UNSLOTH_GRPO_SEQ_PACKING, disable with 0). - Verify the packed logprobs against the per-row clean forward (each row's real tokens alone, reset 0-based positions, no padding), not the padded batch which is itself wrong for left-padding. Cross-sample contamination (a backend ignoring packed_seq_lengths) shows up as a large mismatch and falls back to the padded loop. - Make the trust decision shape and RoPE aware: re-verify whenever the packed total length or the longest segment grows past what was verified, so a later batch crossing a LongRoPE short/long cache boundary is re-checked instead of trusted blindly. - Run lm_head only on completion-prediction positions instead of every packed prompt token, so long-prompt/short-completion batches do not pay for projecting the whole packed prompt. - Drop the hard xformers import so the path also runs in FlashAttention-only environments; the per-row verification guards correctness regardless of backend. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * GRPO no-grad packing: disable entirely on cross-sample mismatch When the per-row verification fails, distinguish the two failure modes by magnitude instead of by sequence length: - A large mismatch (>= 1.5) is the cross-sample contamination signature: the model's attention does not honor the block-diagonal packed mask (seen on some MoE / custom-attention models, e.g. qwen2_moe). Disable packing entirely for the model so later batches do not pay the verification cost again. - A moderate mismatch is more likely a length-boundary effect (a LongRoPE short/long cache switch): keep marking just that length region unsafe so packing still runs for smaller shapes. Validated: Qwen1.5-MoE falls back after a single verification (grad and no-grad ok flags go False, no re-verify on later steps); dense Llama-3.2 and Qwen3 still verify and engage packing. * GRPO no-grad packing: trim comments to be concise * GRPO no-grad packing: fix per-row completion boundary for left-padded rows The completion-target selection used a single global boundary (col >= L - logits_to_keep). After left-packing, each row's completion starts at (L - logits_to_keep) - left_pad[row], so for left-padded rows the first left_pad completion tokens fall below the global boundary and were dropped, leaving 0 logprobs at real completion positions that the loss mask keeps. Use the per-row boundary so packed coverage matches create_completion_attention_mask exactly, and widen the self-verify mask to the full per-row completion region so it can catch coverage gaps. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * GRPO no-grad packing: gate verification on real completion rows Count active rows via create_completion_attention_mask (the same mask the loss uses) instead of any non-pad token in the packed window. Prompt-only rows carry prompt-overflow tokens in the window and could otherwise satisfy the >= 2 verification guard, letting a batch with a single real completion row cache a trust decision. This matches the gradient path, which already gates on the completion mask. The same mask is reused for the self-verify comparison. * GRPO no-grad packing: gate debug logging on UNSLOTH_ENABLE_LOGGING Use the shared UNSLOTH_ENABLE_LOGGING global (import_fixes, re-exported by _utils) instead of a bespoke UNSLOTH_GRPO_SEQ_PACKING_DEBUG env var for the packing debug prints, matching the rest of the codebase. * GRPO packing: import UNSLOTH_ENABLE_LOGGING inside the injected logp function _get_per_token_logps_and_entropies is copied verbatim into the generated GRPO trainer via inspect.getsource, and that module never imported UNSLOTH_ENABLE_LOGGING, so the default-on packing verify path raised NameError (and the except handler re-raised it). Import the flag locally, before the try, so the name is defined in the generated module too. Drop it from the now-unused module-level import. * GRPO no-grad packing: harden unsafe-length skip, verify guard, fallback cleanup Three fixes to the no-grad logp packing path, mirroring the grad path: - skip the packed forward for known-unsafe lengths by reading unsafe_T and gating on it before the forward, instead of running the full packed pass and the result build only to discard them (wastes a pass, can OOM at large T) - only widen the verified T/seg envelope when >= 2 completion rows actually exercised cross-sample packing; a < 2 row batch cannot expose leakage, so it must not extend the trusted shape that later multi-row batches skip verify for - drop the packed intermediates (hidden/sel/result/ref) before the padded fallback loop so it does not run with the flattened hidden state still resident * GRPO no-grad packing: cap the flattened forward at one mini-batch budget The packed path built a single [1, sum L] forward over every row before any size check, so a large batch could exceed the memory the padded path bounds per mini-batch. Gate packing on _pk_T <= _pk_cap (B * seq_len, one padded mini-batch's token budget); larger batches fall back to the chunked padded loop. * GRPO no-grad packing: disable unless unsloth_zoo has the masked-column guard The packed path leaves masked prompt/pad logprob columns at 0, which only stays finite if unsloth_zoo grpo_compute_loss zeroes them before exp() (zoo#840). An older unsloth_zoo without that guard would NaN. Detect the guard once (cached on the model) via inspect.getsource and gate packing on it, so unslothai#6738 is safe with any unsloth_zoo version and re-enables packing automatically once a guarded zoo is installed, independent of the pinned lower bound. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * GRPO packing: hoist env gates and zoo-guard detection to one-time module checks Read UNSLOTH_GRPO_SEQ_PACKING and detect the unsloth_zoo masked-column guard once at import time (module constants plus RL_PRE_ITEMS for the generated trainer cache) instead of per call, and drop the in-function UNSLOTH_ENABLE_LOGGING import for a module-top one. The UNSLOTH_GRPO_SEQ_PACKING_VERIFY force-verify debug knob is commented out, kept in place for hand re-enable; the first-use and envelope-growth self-verify stays active. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * GRPO packing: cap the flattened forward by the padded chunk rows B counts chunks at this point, so B * seq_len understated (small runs) or overstated (large runs) the padded mini-batch token budget; use batch_size * seq_len, the rows the padded loop actually forwards per chunk. * Add PrefixGrouper for GRPO: dedup the shared prompt across a group's completions In GRPO every prompt spawns G=num_generations completions that share the prompt prefix, so the trunk logprob forward re-encodes that prefix G times. PrefixGrouper stores the prefix once and concatenates only the G suffixes behind a FlexAttention shared-prefix mask, cutting the forward from G*(P+R) to P+G*R tokens across both the no-grad old/ref forwards and the grad logp forward. Default off behind the UNSLOTH_GRPO_PREFIX_GROUPER env gate, so the gate-unset path is byte-identical to today. A tok_r auto-gate and a first-use self-verify (fall back and mark the shape unsafe on mismatch) keep it from ever shipping wrong logprobs silently. Wired for llama, mistral, qwen3, gemma2, cohere, granite and falcon_h1, plus qwen2 and gemma through the shared LlamaAttention_fast_forward. Stacked on the GRPO sequence-packing PR (unslothai#6738); the grad path lands in a companion unsloth-zoo PR. Also fixes a latent UNSLOTH_ENABLE_LOGGING NameError in the seq-packing no-grad verify path by defining the name as a generated-cache pre-item. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * PrefixGrouper: enforce the sliding-window cap, gate softcap models, bound the mask cache Add a max_segment_cap kwarg to build_group_layout so it falls back when a group's span (prefix + longest suffix) exceeds the model's local window, and pass the config sliding_window into the no-grad engage gate the same way the packed _pk guard derives it. Skip PrefixGrouper entirely for attn_logit_softcapping models, since the FlexAttention kernel never applies logit softcapping. Bound _BLOCK_MASK_CACHE to a FIFO of 8 so per-step lengths cannot pin BlockMasks forever, release the PG hidden before the verify forward, and align the UNSLOTH_ENABLE_LOGGING pre-item truthiness with the canonical form. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * PrefixGrouper: vectorize the real-column scan in build_group_layout Replace the per-row O(B*L) Python scan of the keep mask with a GPU-derived contiguous-run fast path (first real column + count per row), keeping the general scan only as a fallback for non-contiguous rows. Works for both call sites: the no-grad layout (left-padded prompt + right-padded completion, run does not start at column 0) and the grad layout (left-packed). * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * PrefixGrouper: hoist the gate and kernel imports to one-time module checks, AGPLv3 headers Read UNSLOTH_GRPO_PREFIX_GROUPER and resolve the prefix_grouper imports once at module level (source constants plus an RL_PRE_ITEMS entry for the generated trainer cache) instead of per call, matching the sequence-packing gates. The prefix_grouper env helpers become one-time module reads with unchanged signatures, and attention_dispatch resolves the FlexAttention kernel once behind the same gate (lazy fallback kept). The two new prefix_grouper files move to AGPLv3 headers. * PrefixGrouper: length-envelope trust and hybrid SSM exclusion Verified signatures now record (max T, max segment) and re-verify when either grows, matching the packed path's envelope. Hybrid SSM models (FalconH1 etc.) are excluded at the gate since only attention gets the shared-prefix isolation, and the FalconH1 wiring is removed. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * PrefixGrouper: defer the unverified no-grad forward until the packed reference exists Unverified shapes no longer run the whole-batch shared-prefix forward up front; it now runs at the verify site, only when the packed path produced a reference. A declined packed path (budget, window) therefore costs no wasted PG forward per step. Trusted shapes still run it first to skip the full-row forward, with the same fallback. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * PrefixGrouper: disable under vLLM (fast_inference=True) With colocated vLLM generation the rollout dominates the GRPO step, so the shared-prefix training forward saves little end-to-end and its first-use self-verify (which also runs the full-row path) is net overhead. Gate PG on not use_vllm so it only engages on the raw transformers path, where the training forward is on the critical path. Packing is unaffected. * PrefixGrouper: compile the FlexAttention kernel with dynamic shapes GRPO changes the packed length T almost every batch. With dynamic=False the flex forward+backward kernel recompiled on every new T (~14s each on a 4B trunk), which dominated the step and made PG a net loss. dynamic=True compiles once, then reuses the kernel across all lengths recompile-free (a new shape drops from ~14s to ~1.4ms after a two-graph warmup). T is still padded to a multiple of 128 for the backward block assertion. * PrefixGrouper: default on Enable PrefixGrouper by default (UNSLOTH_GRPO_PREFIX_GROUPER defaults to 1; set 0 to disable). Still auto-disabled under vLLM (fast_inference=True) and by the arch/softcap/ SSM/tok_r gates, and the first-use self-verify falls back on any mismatch, so this is a memory-first default on the raw-transformers path with no correctness risk. * GRPO PrefixGrouper: gate on zoo masked-column guard and exclude MoE - Require the zoo masked-column guard (zoo#840) before PrefixGrouper can engage. PG rides the sequence-packing path, so when the first-step self-verify is off the fast path trusts PG output directly; without the guard those masked columns feed NaN into the packed loss. Gate PG on the same UNSLOTH_ZOO_HAS_MASKED_COL_GUARD the packing path already checks. - Exclude MoE configs (num_experts, num_local_experts, n_routed_experts, moe_intermediate_size) alongside the hybrid-SSM markers. Only the threaded attention forwards carry the shared-prefix isolation, so a MoE decoder that does not forward prefix_seg_info would let suffixes leak across completions. - Refresh the stale default-off comments now that UNSLOTH_GRPO_PREFIX_GROUPER is on by default. * GRPO PrefixGrouper: import chunked_hidden_states_selective_log_softmax The shared-prefix forward passes chunked_hidden_states_selective_log_softmax into extract_logps, but the name was only ever provided by the generated trainer cache (rl.py injects grpo_selective_log_softmax_code), never bound in this module. Import it from unsloth_zoo.rl_replacements next to its sibling chunked_selective_log_softmax so the source resolves the name in every scope (the new _pg_run_forward closure included). No runtime change: the cache still defines the function via template injection. * GRPO PrefixGrouper: dropout gate, device-safe layout, Mistral mask skip Addresses three review findings on the shared-prefix path: - Skip PrefixGrouper when the model sets a nonzero attention_dropout. The normal backends apply config.attention_dropout while training (e.g. Granite dense flash/sdpa/xformers), but the FlexAttention shared-prefix path is deterministic, so gate PG off for those configs rather than train on mismatched activations. - Move the shared-prefix mask labels to the consumer (Q) device in get_block_mask and the target index maps to hidden.device in extract_logps, mirroring the packed path moving its metadata to the consumer device. Prevents cross-device indexing when the model is sharded across GPUs. - Do not synthesize a causal attention_mask in the Mistral forward when prefix_seg_info is present. On the no-xFormers path that synthetic mask tripped resolve_prefix_seg_info and forced PG to always fall back to the packed forward. * GRPO sequence packing: tighten comments * GRPO PrefixGrouper: tighten comments * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * GRPO PrefixGrouper: persistent disable on runtime failure; build block-mask labels with inference mode disabled - rl_replacements: on a PG forward exception (FlexAttention/Triton compile failure or OOM), set a model-level _unsloth_prefix_grouper_nograd_disabled flag and consult it in the engage gate, mirroring the seq-packing handler, so a GPU-wide failure is not retried and re-paid every step. - prefix_grouper_kernel: move the .to(device) label copies inside the inference_mode(False) block so a cross-device (model-parallel shard) first build does not capture inference tensors, which otherwise cannot be saved for backward when the grad training forward reuses the cached BlockMask. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Lee Jackson <130007945+Imagineer99@users.noreply.github.com>
…hai#6848) * Handle odd shapes and non-float scales in FP8BlockQuantLinear Small fp8 checkpoints (e.g. tiny test models) break the block-quantized linear in three ways: weight scales stored in a float8 dtype such as float8_e8m0fnu have no triton dtype mapping; activations whose hidden dim is not a multiple of the activation quant block fail act_quant's divisibility assert; and weights whose dims are not multiples of the weight block cannot be tiled by the triton dequant kernel. Cast non-float scales to float32 on entry, and when the hidden dim does not divide into the activation block, dequantize the weight and run a plain matmul instead of the fp8 block matmul. The dequant goes through a new shape-safe helper that falls back to a torch-native scale expansion when the weight does not tile evenly; backward uses the same helper so the gradient path works for every shape the forward accepts. Full-size checkpoints are unaffected. * Add tiny / e8m0 fp8 block-quant regression test * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix FP8 block-quant fallback: real block size in dequant and scalar-scale fast path * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Route rectangular fp8 blocks through torch dequant and keep block_size across e8m0 upcast The triton weight_dequant kernel uses one BLOCK_SIZE for both axes, so rectangular blocks (block_size[0] != block_size[1]) mis-index the column scale and corrupt grad_X. Route those through the torch scale expansion, which handles each dimension independently, and keep the triton path for square blocks only. Also preserve a block_size attribute carried on the scale tensor across the e8m0 -> float32 upcast so the later lookup no longer falls back to [128, 128]. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…lothai#6849) * Scope MoE expert LoRA detection to actual MLP projection targets _moe_target_set_from_string treated any regex containing the substring mlp or ffn as targeting the expert MLP projections. Unsloth's auto-generated attention-only regex lists mlp, ffn and feed_forward as allowed intermediate path segments while its final group matches only q_proj/k_proj/v_proj/o_proj, so attention-only finetuning on MoE models silently enabled expert LoRA as well: the experts were trained and every MoE layer paid the extra expert LoRA grouped matmuls. Detect expert intent from the projection names themselves (gate_proj/up_proj/down_proj/gate_up_proj) instead of the mlp substring. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten comments * Detect MoE expert LoRA via mlp path segment, not proj names The auto-generated target regex always lists every projection leaf (q/k/v/o and gate/up/down), so keying detection on a proj name mis-fired: it enabled expert LoRA for attention-only regexes and dropped the mlp/ffn path regexes. Key on the mlp/ffn/feed_forward/experts path segment instead, which is present only when the MLP/experts are actually targeted. Add a regression test for the attention-only case. * Scope expert LoRA targets to the leaves a regex names An mlp path alternative with attention-only leaves, for example (mlp|self_attn).(q_proj|o_proj), no longer enables expert LoRA, and a regex naming a single expert leaf such as .*experts.*down_proj now targets only that projection instead of the whole broad set. Generic mlp projections (.*mlp.*proj) and the auto regex mlp tag block keep the broad set for fused-expert models whose leaves are plain Parameters. * Route explicit leaf list into MoE expert detection An attention-only explicit target_modules list routed through get_peft_regex for family scoping (e.g. FastVisionModel with vision layers off) yields a regex carrying the full mlp|feed_forward|ffn|dense component block even though its leaf group only names q/k/v/o_proj. Keying expert detection on that regex trained the experts for a language-only/attention-only request. Use the caller's original leaf list for detection; only the auto path uses the regex, where the mlp block is the sole MLP-intent signal on fused-expert models. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Respect finetune_mlp_modules and finetune_language_layers scope for MoE expert detection When an explicit leaf list that names MLP projections (gate_proj/up_proj/down_proj) is routed through get_peft_regex under finetune_mlp_modules=False, the scoped regex correctly drops the MLP leaves, but MoE expert detection was still keyed on the original list and re-added mlp.experts.* via target_parameters, training the experts the caller had frozen. Same gap for finetune_language_layers=False on vision-only runs. Prefer the original list only when MLP and language families are both in scope (preserving the attention-only fix); otherwise honor the scoped result so the frozen family is respected. Factored the choice into _select_moe_detection_targets with unit tests over the full selection matrix. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…ed (unslothai#6847) * Honor an explicit sdpa or flex_attention request when flash is disabled When flash attention is disabled for a model, the fallback selection could downgrade a caller who explicitly passed attn_implementation='sdpa' or 'flex_attention' to a different backend, because the disable reason is flash-specific. Keep an explicit non-flash request as-is; flash requests still fall back as before. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten comments * Gate honor-explicit attention on provenance and flex support Only honor an explicit non-flash attention request when it comes from the caller argument, not from a config value the loaders synthesize (the language path seeds attn_implementation=sdpa). Honor explicit flex_attention only when supports_flex_attention is True so excluded/broken configs (e.g. gpt_oss) fall back instead of selecting a known-broken backend. Explicit sdpa stays honored. * Honor explicit sdpa through the resolver guard * Keep SDPA exclusions when honoring an explicit sdpa request An explicit attn_implementation="sdpa" was re-enabling sdpa for models in _SDPA_EXCLUDED_MODELS (e.g. gpt_oss) where sdpa is known-broken: the helper honored the request and the resolver's final not-supports_sdpa guard skipped the eager downgrade for any explicit request. Honor an explicit sdpa only when the model is not sdpa-excluded, mirroring the flex guard that already falls back for _FLEX_EXCLUDED_MODELS via supports_flex_attention. Conservative supports_sdpa=False (large head dim / attention-sink models) still honors an explicit sdpa; a synthesized/default sdpa still downgrades to eager. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Honor DISABLE_SDPA_MODEL_NAMES when honoring explicit sdpa The honor-explicit-sdpa guard only skipped the sdpa->eager downgrade for models in _SDPA_EXCLUDED_MODELS (gpt_oss). Gemma3/Gemma3Text disable SDPA through the loader's DISABLE_SDPA_MODEL_NAMES (their bundled SDPA modules are wrong), so an explicit sdpa request bypassed the downgrade and re-enabled a known-wrong path. Extend _is_sdpa_excluded to also treat DISABLE_SDPA_MODEL_NAMES membership as excluded, replicating the loader's trailing-comma substring match so gemma3 and gemma3_text match but gemma3n does not. Move the constant into _utils.py (single source of truth, re-exported from loader.py) to avoid a loader -> _utils cycle. Conservative supports_sdpa=False models not in either list still honor explicit sdpa. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…slothai#6727) * Auto-enable grouped MoE on loaded / PEFT'd models via loader hook Wraps the FastLlamaModel and FastBaseModel from_pretrained / get_peft_model leaves with wrap_loader_for_grouped_moe so the grouped-GEMM MoE forward is installed on the live instance after the model and its compiled module are built. Gated by UNSLOTH_MOE_GROUPED and wrapped in try/except, so it is a no-op when the unsloth_zoo module is absent or no eligible MoE block exists. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Install grouped-MoE loader wrappers before PatchFastRL * Re-evaluate grouped MoE after loading a PEFT adapter When loading an existing adapter through FastLanguageModel.from_pretrained, the base model is evaluated for grouped MoE when the wrapped from_pretrained leaf returns, but the adapter is attached afterwards via PeftModel and patch_peft_model. Re-run auto_enable_grouped_moe on the final model so blocks whose experts gained LoRA are restored to the original loop, attention-only adapters keep the grouped path on their frozen experts, and recompute is re-derived from the final gradient-checkpointing state. Guarded so it never blocks adapter loading. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Trim comments in the grouped MoE loader hooks Shorten the loader re-eval and llama.py wrapper comments; code is unchanged (verified comment-only). * Re-evaluate grouped MoE after loading a PEFT adapter on the vision path --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
) A model path ending in -bf16 unconditionally forced 16-bit loading, so a LOCAL checkpoint directory whose name happens to end in -bf16 could never be loaded in 4-bit, 8-bit or fp8: the suffix rule silently overrode the caller's quantization flags. Hub repo ids keep the existing behavior (the suffix is a publishing convention there), but for a local directory (expanduser-aware, so tilde paths are detected too) the requested quantization is preserved unless the caller explicitly passes load_in_16bit=True.
…_moe) (unslothai#6865) * Add gemma4, glm4_moe and qwen3_moe to the FORCE_FLOAT32 fallback list Keeps the fallback list (used only if the unsloth_zoo import fails) in sync with unsloth_zoo/model_lists.py, which now force-float32s these MoE archs so a float16 request loads bf16 and trains finite instead of NaNing the grad_norm. * Union FORCE_FLOAT32 fallback so new archs force float32 with older unsloth_zoo
…unslothai#6850) * Note the bundled flash-linear-attention kernels for gated-deltanet models Unsloth Zoo now bundles the flash-linear-attention (fla) gated-delta Triton kernels and injects them automatically, so gated-deltanet models (Qwen3-Next, Qwen3.5, Kimi-Linear) get the fast path with no pip install. Replace the old install advisory with a one-time note that fires only when the bundled kernels could not be enabled on the current setup (no CUDA, or torch < 2.7 / triton < 3.3), i.e. exactly when transformers falls back to the slow pure PyTorch path. * Tighten comments * Normalize model_types in fla install advisory for None and single string * Cover olmo_hybrid in the gated-deltanet fla advisory
The live resource monitor and GPU readouts derive memory from binary byte counts (bytes / 1024**3 for torch and psutil, MiB / 1024 for the nvidia-smi path), which is GiB, but the UI labeled the values "GB". On a B200 this showed "178.35 GB" for a card whose nvidia-smi total is 183359 MiB (179 GiB), so it looked like memory was missing. Relabel the measured RAM and VRAM readouts to GiB across the floating monitor, the resources tab, the studio live GPU panel, the hub header, the about tab and the onboarding summary. The numeric values are unchanged, so the training GPU selection and memory-fit logic that read the same fields are unaffected. Disk stays labeled GB because the backend reports it in decimal GB (bytes / 1e9), and model file sizes and download progress keep their decimal GB labels to match Hugging Face.
* Fix llama3 RoPE scaling dropped on transformers v5 transformers v5 loads on meta then blanks non-persistent buffers, so _fix_rope_inv_freq rebuilds inv_freq after load. It recomputed a vanilla inv_freq and applied _apply_inv_freq_scaling, a no-op on the base LlamaRotaryEmbedding used by the config/llama3 path, so inv_freq ended up divided by 1 instead of the config factor (8 for Llama 3.1, 32 for Llama 3.2). This corrupts long-range positions and inflates long-context loss about 3-5x. transformers 4.x was unaffected. Route __init__ and the v5 repair through one _unsloth_recompute_inv_freq so they cannot diverge, and stash the config on the rotary module so the repair can rebuild the same scaled value. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Add test for llama3 RoPE scaling under the transformers v5 repair * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Update RoPE drift guard for the recompute refactor and guard the v5 repair The drift guard's AST tripwire asserted the config-scaling call lived in the if config is not None branch of LlamaRotaryEmbedding.__init__. The fix moved that into _unsloth_recompute_inv_freq, so follow it there (with a fallback to the old inline branch) and add a guard that loader._fix_rope_inv_freq rebuilds inv_freq through the same helper. Also add a CPU functional check of the helper and drop the redundant standalone test. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
ruff-format requires two blank lines before a top-level function. loader.py carried only one, so the ruff-format-with-kwargs pre-commit hook reformats it and the run fails. This restores the expected spacing.
…n safetensors + MLX (unslothai#5620) * studio: tool calling for Llama-3, Mistral, Gemma 4 on safetensors + MLX (unslothai#5615) Adds tool calling for Llama-3, Mistral (pre-v11 + v11+ + [ARGS]), and Gemma 4 to the safetensors / transformers and MLX backends. Parser patched against llama.cpp / vLLM / SGLang per-family parsers and normalises to OpenAI shape. 96 targeted unit tests + cross-OS staging CI (ubuntu / macos-14 / windows) green on the multi-format probe. * studio: tool-call healing parity between safetensors / MLX and GGUF After the multi-format parser landed in unslothai#5615, the safetensors / MLX agentic loop and the GGUF loop still differed on healing behaviour. This commit closes the gaps in both directions so the two backends react the same way to identical model output. Changes: 1. core/inference/llama_cpp.py -- the GGUF BUFFERING state machine now wakes on every emission marker the shared parser knows. Was ("<tool_call>", "<function="); is now the five-tuple imported from core.inference.tool_call_parser (Qwen / Qwen3.5 / Llama-3 <|python_tag|> / Mistral [TOOL_CALLS] / Gemma 4 <|tool_call>). Stream cleanup is delegated to the same shared strip_tool_markup so leaked markup from any family is removed from assistant content. 2. core/inference/llama_cpp.py -- per-tool canonical heal key. When a tool arguments field is a bare string and JSON parsing fails, the GGUF path now heals to {"code": raw_args} for python, {"command": raw_args} for terminal, and {"query": raw_args} for everything else. Was hard-coded to {"query": raw_args}, which silently routed every python / terminal emission through web_search. Mirrors safetensors_agentic._CANONICAL_HEAL_ARG. 3. core/inference/safetensors_agentic.py -- re-prompt on plan- without-action. When the model emits a short forward-looking intent ("I'll search for that", "Let me check", "First, I will...") and no tool call, the loop nudges the model to act instead of silently returning a plan-only answer. Up to _MAX_REPROMPTS=3 (matches GGUF). The intent regex, character cap, and instruction text are byte-identical to the GGUF path. The buffer-end fall-through is unified so a buffered intent emission that never exits the BUFFERING state still triggers the re-prompt. 4. core/inference/safetensors_agentic.py -- extra iteration slots for re-prompts. The loop now budgets max_tool_iterations + _MAX_REPROMPTS + 1 total iterations and tracks the tool-call count separately, so a stalling model can be nudged 3x without eating the caller's tool-call budget. Mirrors the _extra slot reservation in the GGUF path. Tests (14 new safetensors-side units; 5 GGUF parity pins): TestLoopRePrompt -- intent-trigger, plain-answer, no-tools, cap-at-three, budget preserved, buffer-end intent. TestLoopCanonicalHealKey -- python / terminal / unknown. TestGGUFSafetensorsHealingParity -- shared markers used, shared strip used, canonical heal keys identical, intent regex matches same phrases, _MAX_REPROMPTS equal on both backends. All 110 targeted tests pass locally; the broader tool / inference / model-config / sandbox / anthropic / mlx suites stay green. Why this matters Without this parity, Llama-3.2 / Mistral / Gemma 4 emissions on Mac (MLX) and Linux-safetensors stop the agentic loop as soon as the model says "Let me...", because the GGUF re-prompt logic never existed on these backends. The two-marker GGUF BUFFERING tuple also let non-Qwen tool emissions stream out as plain prose when llama-server's structured channel did not pick them up. Both paths now drain the same way, heal the same way, and re-prompt the same way -- so a tool call that works on GGUF works identically on safetensors / MLX. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: fix tool-call parser bugs from gemini review on unslothai#5620 Three high-priority gemini findings on the tool-call parsing additions: 1. unicode_escape on UTF-8 bytes corrupts non-ASCII literals (e.g. ✨ becomes â\x9c¨). Replace with json.loads on a quoted string -- preserves emoji / CJK / RTL while still handling \n \t \uXXXX escapes. 2. Llama-3 sentinel stripping is order-dependent. A leading `<|eot_id|><|begin_of_text|>` left `<|begin_of_text|>` behind because the loop had already passed that sentinel. Loop until no sentinel matches at the start. 3. Mistral v11+ `[TOOL_CALLS] name { json }` regex uses non-greedy `\{.*?\}` which truncates at the first `}` of a nested JSON argument, leaking the tail (e.g. `}}`) into user-visible streamed text. Same problem for the v0.3 array pattern with nested brackets. Strip those with balanced brace/bracket scanning via a new `_strip_mistral_closed_calls` helper called from `strip_tool_markup`. Also fix the inference routes' parallel `_TOOL_XML_RE`: - Same nested-JSON truncation in the Mistral patterns; route the strip through the parser's balanced-scan helper via a thin `_strip_tool_xml` wrapper that all existing callers now use. - Llama-3 `<|python_tag|>[^\n<]*` stopped at any `<`, leaking the tail of any tool call whose argument contained a literal `<` (queries, code snippets). Relax to `[^\n]*` which keeps the strip confined to the actual end-of-line. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio/routes: make python_tag strip multi-line aware Earlier revisions of _TOOL_XML_RE in studio.backend.routes.inference oscillated between two bug shapes: 5615 r"<\|python_tag\|>[^\n<]*" -- stopped at any literal "<" so code='if x < 10: pass' leaked '< 10: pass)' to the user. 5620.1 r"<\|python_tag\|>[^\n]*" -- single-line only; the second line of python.call(code="a\nb") leaked. The full parser (_parse_llama3_python_tag) already handles both via balanced-brace scanning, so the parsing path was fine; the LEAK was in the streaming strip path that runs on every cumulative emission while content is still arriving. Switch to r"<\|python_tag\|>(?:[^<]|<(?!\|))*" so the strip consumes: * any character that is not a "<" (newlines, JSON, code, ...), * a "<" only when it is NOT followed by "|" (i.e. NOT a Llama-3 sentinel start like <|eot_id|>, <|eom_id|>, <|begin_of_text|>). This means: * code='if x < 10' stays inside the strip (5615 fix preserved), * multi-line code stays inside the strip (5620 round 2), * the strip terminates at the next Llama-3 sentinel so trailing assistant content survives. Tests: TestRoutesPythonTagStrip (8 cases) pytest test_safetensors_tool_loop.py test_safetensors_capability_advertise.py -> 118 passed in 1.81s (was 110). * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: tighten verbose comments in tool-call parser sections Comments were narrating what the code already says. Cut historical "earlier revisions used X, then Y" narratives down to one-line WHY notes where the footgun still matters (canonical heal-key parity, balanced-brace vs non-greedy regex, ``(?:[^<]|<(?!\|))*`` over ``[^\n<]*``/``[^\n]*``). Drop section-header banners. No behaviour change. Re-ran: pytest studio/backend/tests/test_safetensors_tool_loop.py \ studio/backend/tests/test_safetensors_capability_advertise.py -q -> 118 passed. Regression replay (parser + _coerce_arguments on the 5 unslothai#5615 inputs) -> 21/21. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: parser robustness fixes for PR unslothai#5620 Three surgical extensions to the multi-format tool-call parser, each covering a real fine-tune / template emission shape that the current parser silently drops. No path narrows; all changes widen what is accepted. 1. `_parse_tool_call_json` now accepts both `arguments` and `parameters` keys. A Hermes / Qwen `<tool_call>{json}</tool_call>` wrapper around a Llama-3.2 fine-tune that emits the `parameters` key was extracting the tool name and silently discarding the args, producing a working-shaped call with an empty payload. The bare-JSON and python_tag paths already accepted both keys; this path now matches them. 2. `_TC_FUNC_START_RE`, `_TC_PARAM_START_RE`, and `_TC_PARAM_CLOSE_RE` now also match the attribute form `<function name="..."><param name="...">v</param></function>` used by MiniCPM-5 and MiniMax-M2. Names land in either capture group, and `</param>` is accepted as a short close. 3. `_parse_llama3_bare_json` sentinel-strip now consumes the role label inserted between `<|start_header_id|>` and `<|end_header_id|>` by Meta's official Llama-3.x chat template. Without this, every assistant turn re-fed through the template prefix `<|start_header_id|>assistant<|end_header_id|>\n\n{json}` parsed to zero calls, so any history-with-tool-call round-trip in production silently dropped. Tests in `studio/backend/tests/test_safetensors_tool_loop.py`: * `TestParserRobustness::test_tool_call_json_accepts_parameters_key` * `TestParserRobustness::test_function_xml_attribute_form` * `TestParserRobustness::test_function_xml_attribute_form_multi_param` * `TestParserRobustness::test_function_xml_legacy_equals_form_still_works` (regression guard for the existing `<function=name>` syntax) * `TestParserRobustness::test_llama3_chat_template_round_trip` * `TestParserRobustness::test_llama3_round_trip_all_roles` * `TestParserRobustness::test_llama3_round_trip_with_eot_prefix` `pytest studio/backend/tests/test_safetensors_tool_loop.py studio/backend/tests/test_safetensors_capability_advertise.py -q` goes from 118 to 125 passed. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: terminate function-XML body at </function>, not just </tool_call> `_parse_function_xml` was looking for `</tool_call>` (the Hermes wrapper) as the body terminator. When a model emits a standalone `<function=NAME><parameter=K>v</parameter></function>` followed by explanatory prose (which models routinely do), no `</tool_call>` is present, so the body extended to end-of-string and the trailing prose leaked into the LAST parameter value. Pre-existing on main (the legacy `<function=NAME>` form had this bug too). Same affects PR unslothai#5620's new attribute-form `<function name="NAME"><param name="K">v</param></function>` emission used by MiniCPM-5 / MiniMax-M2. Fix: `_TC_END_TAG_RE` now matches either `</tool_call>` OR `</function>`. The existing `_TC_FUNC_CLOSE_RE` / `_TC_PARAM_CLOSE_RE` strips are unchanged. Multi-call inputs still bound each function at the next `<function=` start, so no over-eager consumption. New tests: * `test_function_xml_followed_by_prose` (legacy form + prose) * `test_function_attribute_xml_followed_by_prose` (attribute form + prose) Existing `test_code_with_embedded_xml` still passes (a parameter value containing literal `<a></a>` is preserved because the embedded close tag is `</a>`, not `</function>`). `pytest studio/backend/tests/test_safetensors_tool_loop.py studio/backend/tests/test_safetensors_capability_advertise.py -q` goes from 125 to 127 passed. * Studio: tighten Llama-3.2 bare-JSON guard A fuzz pass on PR unslothai#5811 turned up that ``_parse_llama3_bare_json`` accepted ``parameters`` as a string, contradicting the docstring's "parameters or arguments is a dict" guard. Prose JSON like ``{"name":"foo","parameters":"a sentence"}`` would wrongly fire the parser, which the agentic loop would then heal into a real ``foo(query="a sentence")`` call. Same code lives on this branch, so the same fix applies here. Tightened guard: - ``parameters`` must be a dict (Llama-3 spec). - ``arguments`` may be a dict, or a JSON-encoded string that decodes to a dict (OpenAI shape, e.g. ``"arguments":"{\"q\":\"x\"}"``). Plain non-JSON strings or JSON-strings of lists / scalars / null no longer pass. Mirrors the fix landed in PR unslothai#5811 commit 615b860. Adds the same 4 regression tests under TestParserMultiFormat. Existing test suite stays green: 127 -> 131 passing. * studio: fix safetensors tool-call parser gaps vs llama.cpp (Mistral CALL_ID / THINK, attribute-form signal) Three GGUF-parity fixes to the safetensors tool-call parser, each matching llama.cpp's reference behaviour: - Mistral Small 3.2 emits [TOOL_CALLS]name[CALL_ID]<id>[ARGS]{json}. The parser stopped after the name on seeing [CALL_ID] (neither [ARGS] nor {), dropping the call. Skip an optional [CALL_ID]<id> segment in both the parse and strip paths. llama.cpp parses this (test-chat.cpp:4785). - Magistral wraps reasoning in [THINK]...[/THINK]. A [TOOL_CALLS] inside the reasoning was parsed as a real call, producing a phantom call. Strip a leading [THINK] block before scanning so only the post-reasoning call counts (test-chat.cpp:2285); a literal [THINK] inside a later argument is left intact. - The standalone MiniCPM-5 / MiniMax-M2 <function name="..."> attribute form parsed correctly but was absent from TOOL_XML_SIGNALS and the markup strip patterns, so the streaming safety-net parse was gated off (dropping the call) and markup leaked into displayed text. Add the signal and broaden the strip regexes. Adds regression tests for all three. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: fire safetensors tool calls for the bare-JSON (Llama-3.2) form The agentic loop's streaming safety-net parse was gated on has_tool_signal(), which is False for the Llama-3.1 / 3.2 bare-JSON tool form {"name":..,"parameters":..} (no XML marker). Real tool calls were therefore dropped: the loop logged "model planned without calling tools", re-prompted three times, then gave up with zero tool calls, while GGUF's llama-server parses the same emission natively. Run parse_tool_calls_from_text() unconditionally in the safety net. The parser is strict (only fires on a valid tool-call shape) so plain answers are unaffected. Reproduced on a real unsloth/Llama-3.1-8B-Instruct run: the model emits {"name":"web_search","parameters":{...}} which now executes the tool instead of being re-prompted into a no-op. Adds a loop regression test for the bare-JSON form. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: complete strict-mode contract and fix parser import paths Address review findings on the multi-format tool-call parser: - Honor allow_incomplete=False in the remaining sub-parsers. The Llama-3 <|python_tag|>NAME.call(...) parser, the pre-v11 Mistral [TOOL_CALLS] array parser, and the Gemma 4 <|tool_call> parser ignored strict mode, so a truncated call (missing closing paren, ], or <tool_call|>) was still healed and executed with Auto-Heal disabled. Thread strictness through and reject the unclosed forms, matching the JSON and function-XML paths. - Drop the duplicate tool_call_parser import block in llama_cpp.py and the redundant un-aliased TOOL_XML_SIGNALS; only the _SHARED_TOOL_XML_SIGNALS alias is used as a value. - Import _strip_mistral_closed_calls from core.inference.tool_call_parser in routes/inference.py instead of studio.backend.core... The self-contained run.py launch mode only puts studio/backend on sys.path, so the absolute package path raised ModuleNotFoundError on the server-tool strip path. Add strict-mode regression tests for the truncated Llama-3 dot-call and the unclosed Mistral array. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: preserve XML param indentation and alias Mistral array parameters Two parser-correctness fixes found by auditing against the model chat templates and the SGLang / vLLM reference parsers: - Qwen3.5 XML parameter values lost their leading indentation. The chat template emits <parameter=k>\nVALUE\n</parameter>, but the parameter-start regex ate the wrapping newline AND the value's first-line indentation with a trailing \s*, then str.strip() removed the rest. Narrow the trailing class to horizontal whitespace only and trim exactly one wrapping newline (via _trim_param_value), preserving indentation in code/diff arguments. Matches SGLang's qwen3_coder detector. Applies to both _parse_function_xml (tool_call_parser.py) and the XML path in tool_healing.py. - Mistral pre-v11 array objects keyed on parameters dropped their payload. _consume_mistral_call read only the arguments key; alias parameters the same way the JSON/XML paths and SGLang's base detector do. Add regression tests for preserved multi-line indentation and the array parameters alias. * Studio: tighten tool-call parser comments Make the comments in the multi-format tool-call parser and its callers succinct: compress verbose docstrings/blocks to one or two lines, drop ones that restate the code, and trim the tiny balanced-scanner helpers. Correctness rationale and upstream provenance (SGLang/llama.cpp parity, the strict-mode / Auto-Heal contract, whitespace-preservation, and the Unicode / full-width-pipe notes) are kept in compact form. Comment-only: no code or behavior change (verified with comment_tools.py check --strip-docstrings; parser suite green). * Studio: make Llama-3 .call and Mistral-array healing parsing linear Two more O(n^2) ReDoS paths in the multi-format parser, both reachable from the agentic loop on a long truncated body with no length cap: - _LLAMA3_KV_RE.finditer over a .call(...) body retried at every offset of a long word run / unterminated quote (40K -> 14s). Replace with a hand-scan that reuses the same key/number/literal sub-regexes via anchored match and walks the string body by hand, so an unterminated quote is O(n). Verified byte-identical to the old regex over 200K fuzzed inputs. - _parse_mistral_array healing ran _balanced_brace_end from every { in the body (20K -> 17s). Walk top-level objects, advancing past each balanced {...}; this also drops the phantom call the old scan emitted from a nested argument object. Add adversarial-length linearity regressions plus positive .call kwargs and unclosed-array recovery coverage. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: honor strict mode in safety-net, keep empty Gemma args, strip attribute-form function XML - safetensors safety-net parser now forwards allow_incomplete=auto_heal_tool_calls, matching the draining path, so a late incomplete tool call is not healed and executed when Auto-Heal is off. - Gemma empty bare value ({k:}) now serialises as "" instead of invalid {"k":}, which previously dropped the whole call. - Route _TOOL_XML_RE also strips the <function name="..."> attribute form (MiniCPM-5 / MiniMax-M2) so it no longer leaks to the UI. * Studio: fix attribute-form function-XML literal close tag and zero-arg strict call Addresses Codex review of the <function name="..."> attribute form in _parse_function_xml (MiniCPM-5 / MiniMax-M2): - End the call body at the LAST </function> / </tool_call> within the call's window, so a literal close tag inside a code/search argument (e.g. print("</function>")) is preserved instead of truncating the call. - Accept a closed call with no parameters as a valid zero-argument call in strict mode (the function close is already required), instead of rejecting it as a truncated call. - Tests for both, mirroring the legacy <function=...> coverage. * Studio: fix tool-call parser/loop review findings on the multi-format path Address the live code-review findings on the safetensors/MLX + GGUF tool path: - routes: include the attribute form <function name="..."> in the safetensors capability whitelist so MiniCPM-5 / MiniMax-M2 templates keep the tool pill (parser already handles the form; the post-filter wrongly suppressed it). - safetensors loop: build the plan-without-action re-prompt from the active tools instead of a hardcoded web_search/python string, and gate it on auto_heal_tool_calls, matching the GGUF loop. - safetensors loop: hold a leading bare-JSON object ({"name":..,"parameters":..}) during BUFFERING until it closes, then drain it as a tool call instead of streaming the raw JSON to clients. The DRAINING/STREAMING resolvers still recover a plain JSON answer, so this can never drop content. - parser: anchor the Llama-3 <|python_tag|>NAME.call(...) scan to the tag and chain ; -separated calls, so all semicolon-separated built-ins parse and a literal <|python_tag|>x.call(...) inside a JSON string argument no longer fires the wrong tool. - parser: consume the optional trailing </s> after a named Mistral [TOOL_CALLS]name{json} call, mirroring the array shape. - GGUF streaming strip: use the shared parser patterns (which know [TOOL_CALLS] and <|python_tag|>) so a textual tool call entering DRAINING is stripped instead of leaking the marker to streaming clients. - routes: hoist the _strip_mistral_closed_calls import to module level. Adds regression tests covering each fix; existing parser suite stays green. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: harden multi-format tool-call detection from review findings Apply five targeted fixes from the review pass over the multi-format tool path: - routes: route display strip delegates to _strip_tool_xml so Mistral [TOOL_CALLS] blocks with nested JSON are removed from streamed display text, not just the XML forms. - tool_call_parser: skip function/parameter starts that fall inside an already-open parameter block (_inside_open_parameter) so nested example payloads are not mis-parsed as new calls; extract strip_llama3_leading_sentinels so the bare-JSON guard is shared. - safetensors_agentic: probe bare JSON through strip_llama3_leading_sentinels before the balanced-brace check so a leaked header sentinel does not defeat the guard. - tool_healing: allow dotted tool names in the Gemma wrapped start pattern. - llama_cpp (GGUF): buffer wrapper-less Llama-3.2 {"name":..} calls that carry no XML signal, drain a complete object silently and hold an incomplete one, and run the end-of-stream safety net unconditionally so markerless calls are detected and never leak the raw JSON (including truncated fragments). Adds regression tests for the GGUF bare-JSON streaming path and the Mistral display strip. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: stop bare-JSON tool calls leaking at EOF, oversized, and into history The second review pass flagged that the Llama-3.2 bare-JSON tool-call handling still leaked raw JSON in several spots; ``strip_tool_markup`` only knows XML/bracket markup, so the bare-JSON form survived it. Fix them symmetrically across the safetensors and GGUF loops: - Safetensors stream-end resolver now routes a held bare-JSON fragment to DRAINING (mirroring GGUF) so a truncated ``{"name":..`` cut off by the end of the stream is dropped instead of flushed as assistant content. The 7/10 reviewer finding. - Both loops now drain (suppress) an oversized still-open bare-JSON call once it passes ``_MAX_BARE_JSON_BUFFER`` instead of streaming the raw prefix, gated on a ``"name"`` key so a giant plain JSON answer still streams; a complete oversized call still executes via the safety net. - Add a shared ``strip_leading_bare_json_call`` helper and apply it to the content kept for the assistant turn in both loops, so an executed bare-JSON call is not replayed as visible text or fed back as next-turn history. Plain JSON answers without a ``"name"`` key are untouched throughout. Adds regression tests for the EOF, oversized, and next-turn cases on both backends plus unit tests for the helper. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: bound the Llama-3 python_tag strip on real control sentinels The route display strip's <|python_tag|> arm ran to the next <| of any kind. A tool-call argument carrying a literal <|...|> token (for example <|cite|> inside a string value) truncated the strip early and leaked the call tail into the visible response. Narrow the stop condition to the genuine Llama control sentinels (eot_id, eom_id, python_tag, start/end_header_id, begin_of_text, finetune_right_pad_id) so embedded markup and JSON are consumed while real header/turn boundaries still bound the strip. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: gate markerless bare JSON on enabled tools and close parser/strip asymmetries The Llama-3.2 custom_tools bare-JSON form has no marker, so any JSON object with a name key was read as a tool call. An ordinary JSON answer like {"name":"Alice","parameters":{"age":30}} was misclassified as a call to a disabled tool and dropped from the visible response. Gate the markerless form on the enabled tool names (threaded through parse_tool_calls_from_text and strip_leading_bare_json_call, supplied by both streaming loops): an object whose name is not an enabled tool is ordinary content. The marker-based forms keep their name-agnostic behaviour (an explicit signal is a real call attempt), and unrestricted mode stays ungated. Also fix two parser/strip asymmetries the parser already tolerated: - A literal </function> inside a parameter value (print("</function>")) truncated both the core and route strips at the first close, leaking the tail. Extend the strip to the call's real close (last </function> before the next opener), mirroring the parser, without merging separate calls. - The single-object Mistral [TOOL_CALLS]{...} shape parsed but _strip_mistral_closed_calls left it, leaking the raw object into display. Strip the balanced object while keeping trailing prose, matching the array and name shapes. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio tools: gate GGUF bare-JSON suppression on enabled tools and fix python-tag exponent parsing Pass-4 review follow-ups on the GGUF tool loop and Llama-3 parser: - The GGUF bare-JSON suppression sites still keyed off a raw "name" substring, so an ordinary JSON answer whose name is not an enabled tool was dropped when it was truncated, oversized, or reached the no-tool DRAINING fallback (the parser, helper, and safetensors paths were already gated). All three sites now use the shared enabled-name gate, and a held bare-JSON buffer that turns out not to be an enabled call is shown as the answer instead of dropped at stream end. - The Llama-3 python-tag numeric kwarg regex matched only the mantissa, so scientific notation was truncated to its leading digits (1e-3 parsed as 1) and a tool executed with the wrong value. The regex now accepts exponent and decimal forms, and the int/float classification keys off the exponent too. Adds regression tests for the truncated / oversized disabled-name JSON cases (and a counterpart that a truncated enabled call still does not leak) plus the scientific-notation kwargs. * Studio tools: gate safetensors bare-JSON drain, fix nested-name gate and function-XML strip Pass-4 review follow-ups on the shared parser / safetensors loop: - The safetensors oversized and end-of-stream bare-JSON drain branches keyed off a raw "name" substring, so a large or truncated ordinary JSON answer whose name is not an enabled tool was drained instead of streamed. Both now use the shared enabled-tool-name gate, matching the GGUF path. - strip_leading_bare_json_call matched the first "name" anywhere, so a plain JSON answer with a nested name equal to an enabled tool ({"result":{"name":"web_search"}}) was wrongly suppressed. It now extracts the TOP-LEVEL name only, walking past nested objects/arrays and keeping the text when a top-level value is truncated. - The function-XML display strip used a regex negative-lookahead that stopped at a literal <function=...> opener inside a parameter value and then dropped the rest of the answer to EOF. A scan-based strip mirrors the parser (ignores openers inside an open <parameter> via _inside_open_parameter) and closes each call at its real </function>, so trailing assistant text after such a call survives. Adds regression tests for each. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tool parsing: 3.9 import safety, disabled-Auto-Heal contract, capability gate Round-2 review follow-ups on the multi-format tool-call parser: - tool_call_parser: add `from __future__ import annotations`. The module is dependency-light by design (external llama-server wrappers import it standalone) and the package targets python >=3.9, where its PEP 604 `int | None` return annotations would raise TypeError on import. - safetensors + GGUF drain fallback: gate the leading bare-JSON strip on auto_heal_tool_calls. With Auto-Heal off, a truncated enabled-name fragment that did not parse now stays visible, matching the XML strip in the same branch and the disabled-Auto-Heal contract. With Auto-Heal on it is still suppressed. - safetensors capability gate: match the bare-JSON `{"name":` template marker with a whitespace/escape-tolerant regex so a pretty-printed `{ "name" :` or JSON-escaped `{\"name\":` template is not mis-classified as tool-less. The parser already accepts that whitespace via raw_decode, so the gate must too. Regression tests added for each case. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tool parsing: symmetric "function" bare-JSON alias and route strip parity Round-3 review follow-ups, all parser/strip symmetry fixes. - Bare-JSON "function" alias: the markerless parser accepts a call name via obj.get("name") or obj.get("function"), but the strip/gates only knew "name", so a {"function":<enabled tool>} call executed while its raw JSON leaked. Teach _top_level_bare_json_name the alias (with "name" precedence and the same nested and truncated-name guards), and widen the guards in strip_leading_bare_json_call, the safetensors and GGUF _looks_like_enabled_bare_json gates, and the route capability marker regex. - Route display/history cleanup: strip a tail-only </param> alias close (the parser accepts <param name="...">...</param>), and run the parser's guarded function-XML scan (_inside_open_parameter) before _TOOL_XML_RE so a literal nested <function=...></function> inside an argument value does not truncate the strip and leak the tail. Regression tests added for each. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio tools: honor tool budget in GGUF loop and guard function-XML streaming strip Round 4 review fixes. Both are asymmetric-fix bugs where the final/steady path got a guard the analogous streaming/loop path did not. - GGUF tool-call budget: the safetensors loop counts real tool-call turns against max_tool_iterations (re-prompt stalls excepted), but the GGUF loop only bounded the turn count by the enlarged range (max_tool_iterations + _MAX_REPROMPTS). Since this PR raised _MAX_REPROMPTS from 1 to 3, a model that keeps making valid tool calls could run up to three extra tool rounds (with max_tool_iterations=1, four rounds instead of one). Add a _tool_iters_done counter that increments only when a tool actually executed in the turn, and stop once the caller's budget is spent so the post-loop final-answer nudge fires. A duplicate/disabled no-op turn is a correction turn (like a plan-without-action re-prompt) and does not consume budget, preserving the existing "already completed" re-prompt behavior. - Streaming display strip: the final strip runs the guarded _strip_function_xml_calls scanner (a literal <function=...> inside a parameter value is data, not a nested call), but the GGUF and safetensors streaming strips still used only the open-ended regex arms. When a tool-call argument contained literal function markup, the regex tail ate everything to end-of-text and dropped the real trailing prose after the call's true </function>. Run the guarded scanner (and the balanced Mistral strip) before the regex arms in both streaming paths so streaming and final display agree. Adds regression tests: GGUF valid tool calls respect max_tool_iterations, and the streaming strip keeps trailing prose after a function-XML call with a literal marker. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio tools: safetensors tool budget counts only executed turns (GGUF parity) Follow-up to the GGUF budget fix. The safetensors loop charged max_tool_iterations per non-re-prompt iteration (iteration + 1 - reprompt_count), so a duplicate/disabled no-op turn spent a budget slot even though no tool ran. With a small cap this dropped real work: for max_tool_iterations=2, a model that made a valid call, repeated it (an internal no-op correction turn), then made a distinct valid call executed only the first -- the third turn was sent with no tools and the distinct call was ignored. Track whether a turn actually executed a tool (set on record_result) and count only those turns against the cap, matching the GGUF loop. A duplicate/disabled no-op is a correction turn -- like a plan-without-action re-prompt -- and no longer consumes budget, so the model still gets its "already completed" nudge and another tool-enabled turn. Adds a regression test for the small-cap duplicate-then-distinct-call flow. * Studio: render the reasoning block for safetensors and MLX like GGUF enable_thinking chat templates (Qwen3/Qwen3.5/GLM) prefill an unclosed <think> into the generation prompt, so the model emits only the closing </think> then the answer. The safetensors/MLX chat stream emitted that as plain content, so the reasoning showed inline with no collapsible thinking block, while GGUF (which surfaces reasoning via reasoning_content) rendered one. This brings safetensors and MLX to parity. - _ResponsesReasoningExtractor gains a reasoning_prefilled mode that starts inside the reasoning block and splits on the first </think>; default False keeps GGUF and every existing caller byte-identical. It suppresses a stray re-emitted <think> and holds partial markers back across chunk boundaries. - _sf_reasoning_prefill_mode gates the mode on reasoning being enabled for the request, an enable_thinking or enable_thinking_effort style, and the template actually using the standard <think>/</think> markers. Models with a bespoke reasoning channel (e.g. gemma's <|think|>/<|channel>) are excluded so their answer is never swallowed; gpt-oss (Harmony) and thinking-off requests are excluded too. - sf_tool_stream and stream_chunks (the latter also serves MLX) feed text through the extractor, emitting reasoning_content then content deltas, with a per-turn reset in the tool loop and a flush before each tool_start; only the visible delta reaches the monitor reply. The two non-streaming drains split reasoning_content the same way. - Tests: extractor prefilled mode (streaming and edge cases), the gate matrix including the gemma-style exclusion, and a route-replay of the tool-loop reasoning stream. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: don't force a tool re-prompt on a negated intent (safetensors parity) The safetensors _INTENT_SIGNAL claimed to mirror GGUF but was missing the negative lookahead, so a refusal like "I will not search the web for that" matched the "i will" intent and triggered the plan-without-action re-prompt (STOP... you MUST call a tool), overriding a valid no-tool answer. GGUF already excludes not/never. Add the same (?!\s+(?:not|never)\b) lookahead so both backends agree. Extends the intent parity test with negated refusals. * Studio: trim redundant comments (comment-only, AST-verified) * Studio: prevent Gemma tool-parser DoS on stray delimiters _gemma_parse_value returned the input index unchanged when text[i] was a stray delimiter (,}]), so the list and mapping caller loops that advance on the returned index spun forever at 100% CPU on malformed input such as [},]. Advance past the delimiter so parsing always terminates. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: strip Magistral [THINK] reasoning from final display/history strip_tool_markup removed [TOOL_CALLS] and <function> markup but left a leading Magistral [THINK]...[/THINK] block intact, so its bracket-form reasoning (not the <think> the reasoning channel renders) leaked into the safetensors display and conversation history while GGUF/llama.cpp routes it natively. Drop the leading reasoning block at end-of-turn (final=True) via the existing _strip_mistral_reasoning helper; streaming is untouched. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Honor reasoning_effort none in safetensors prefill; strip Magistral reasoning while streaming Two safetensors/MLX reasoning fixes surfaced in review: _sf_reasoning_prefill_mode only checked enable_thinking, so an enable_thinking_effort (GLM-5.2) request that disables thinking via reasoning_effort=none (without enable_thinking=False) still began in prefilled-<think> mode. A plain answer with no </think> was then swallowed whole into reasoning_content and the visible response came back empty. Thread reasoning_effort into the predicate and treat none as disabled, mirroring _request_reasoning_kwargs. strip_tool_markup_streaming stripped tool markup but not the leading Magistral [THINK]...[/THINK] bracket block, so the raw chain-of-thought leaked into the streamed safetensors content instead of the reasoning drawer (GGUF routes it natively). Apply _strip_mistral_reasoning first, matching the final strip; an unclosed [THINK] is held from the marker on so nothing flickers. * Mistral outer call wins over XML literals; align healer signals with its parser Two follow-ups on the shared-parser ordering after the healing-passthrough merge: - A well-formed [TOOL_CALLS] call whose JSON arguments quote tool XML parsed the literal instead of the outer call (executing the wrong tool). When the first XML signal sits inside a leading balanced Mistral body it is argument data, so the Mistral parser now runs first; an XML signal before the trigger keeps the normal order, so a [TOOL_CALLS] literal inside an XML call's arguments still stays data. - passthrough_healing buffered streams on the parser module's broadened signal list (now including <|python_tag|> and [TOOL_CALLS]) but promotes with core.tool_healing, which does not parse those forms: a streamed Mistral or Llama text call was held until finalization and flushed as prose. The healer keeps its own signal list limited to the formats it can promote, restoring immediate streaming for the rest. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Address review: leading envelopes win over rehearsed literals - New _first_foreign_tool_signal shared by the leading-envelope guards adds <|python_tag|> to the protected signal set: the spelled-out literal inside a Mistral call's arguments (a query about Llama built-in tool syntax) executed the inner literal instead of the outer call. - New _xml_signal_inside_leading_bare_json guard, sibling of the Mistral one: a leading bare-JSON call whose string argument quotes tool XML (a code value citing <function=...>) had the literal promoted by the shared XML pass before the bare-JSON parser ran. - Magistral [THINK]...[/THINK] is dropped once at parse entry instead of only inside the Mistral parser, so a call rehearsed in the think block in a foreign format can no longer be promoted while the real call after the block is lost. Parse now agrees with the display strip. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Address review: a disabled leading bare-JSON object keeps its literals as data When the leading bare-JSON object is ordinary content (name not an enabled tool), the guard proved the first tool signal sits inside it, so falling through to the XML/python_tag passes promoted quoted string data as a real call. Drop the object and parse only the tail: a real call after the object still parses, nothing inside it can be promoted. * Address review: Mistral literals inside leading JSON, whitespace-tolerant wrapped Gemma opener - The leading bare-JSON guard now treats the [TOOL_CALLS] trigger as a foreign signal: the Mistral parser runs before the bare-JSON one, so a literal quoted inside the leading object's strings was promoted over the outer call (or over ordinary JSON content). - tool_healing's wrapped Gemma opener tolerates whitespace around call and the colon: sampling drift emits call: name{ and call : name{, and rejecting those lost the call entirely because no fallback re-parses the wrapped form. Strict mode still requires the closing tag. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Address review: accept dotted Gemma argument keys in the key-quoting scanner The scanner quoted keys of [alnum_-] only, so a dotted key (user.name:...) was left unquoted, json.loads failed, and the whole wrapped call was lost (parse empty, strip wipes the markup). Dots now match the parser's own key/name charset. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Address review: leading Mistral call owns the turn, dotted keys after bare values - A LEADING parseable [TOOL_CALLS] call now runs the Mistral parser first unconditionally: literal XML in trailing prose after the call was promoted by the earlier shared XML pass, executing the quoted example instead of the real leading call. XML leading keeps the normal order. - _GEMMA_NEXT_KEY_RE accepts dots so a dotted key after a bare value (query:foo,user.name:bob) ends the value at the comma instead of being swallowed into it, matching the round-earlier key-quoting charset. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Address review: markup quoted inside a nameless leading JSON answer stays data The leading bare-JSON guard required a top-level name, so a structured JSON answer quoting tool markup in its strings (a response_format turn documenting a tool's syntax) had the literal promoted by the later passes. A nameless leading object that parses as real JSON now routes through the same decline-then-parse-the-tail path; non-JSON braced prose keeps the old behaviour, and a real call after the answer still parses. * Compress docstrings in the multi-format tool parser to their contract essence * verify_import_hoist: exempt __future__ imports and same-diff relocations Two false positives fired on this PR's refactor. A from __future__ import is a compiler directive whose name never appears as a runtime load, so HOISTED-IMPORT-UNUSED can never see it used, yet the file requires it for PEP 604 annotations on Python 3.9. TARGET-CHANGED flagged the deliberate move of the strip-pattern constants into core.inference.tool_call_parser as a silent re-point even though the old module-level target was removed and the new one added in the same diff. Both get narrow exemptions; a re-point to a pre-existing target is still caught, and the self-test negative controls all pass unchanged. * Leading bare-JSON calls own the turn; function calls end at the first balanced close The XML-signal guard for a leading bare-JSON call required the signal strictly inside the object, so a trailing XML example stole the turn from the leading call; it now applies the same inside-or-after rule as the Mistral guard. Function-XML calls also ended at the LAST close tag, which let prose after a closed call that mentions a literal close tag get swallowed into the final parameter value; calls now end at the first close tag that is not inside an open parameter, and the strip mirrors the same rule so parse and strip agree. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Attribute-form calls end at the first balanced close; bare-JSON strip requires the call shape The attribute form parser still kept the last close tag in the call window, folding prose after a closed call into the final parameter value. It now takes the first close not inside an open parameter, the same rule the equals form and the strip already use. The leading bare-JSON strip deleted any closed object whose top-level name matched an enabled tool, including plain JSON answers the parser correctly rejects as non-calls. The strip (and the drain gate that delegates to it) now requires the parser's exact call shape, so answers like {"name":"web_search","result":...} stream and display intact. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * False-alarm markers keep the answer; the bare-JSON strip consumes the whole chain The trailing strip arms dropped everything from a bare marker to EOF, so a normal answer that mentions [TOOL_CALLS] or another marker literally was truncated (or fully swallowed when it started with the literal) after the no-call drain fallback. Those arms now require a call-shaped lookahead or marker-at-EOF before dropping; truncated real calls still strip. Chained bare-JSON turns executed both calls but stripped only the first object, so the second call's raw JSON replayed into the next assistant history message alongside the structured tool_calls. The strip now consumes the entire chained run of call-shaped enabled objects while non-call answers, disabled names, and trailing prose stay intact. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Attribute-form containment, parameter-close-decides rule, preamble-tolerant Mistral guard, strict strip shape Four document-order and containment fixes. A leading attribute-form call now parses before the shared XML pass, so markup quoted in its parameter stays data. The open-parameter scan lets the parameter's own close tag decide, so any number of literal function closes inside one value stay data, restoring the pre-close-scan behavior for multi-close arguments. The leading-Mistral guard tolerates a visible preamble, with the leading-bare-JSON guard running first so a trigger quoted inside a leading JSON object stays data. The bare-JSON strip requires the parser's top-level name in every mode, so nested-name JSON answers survive name-agnostic stripping. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Let a leading <|python_tag|> call own the turn over quoted XML literals The leading-call ownership contract (a leading executable call owns the turn; foreign markup quoted in its string arguments or trailing prose stays data) was enforced for the bare-JSON, Mistral and attribute-form leading calls but not for the Llama-3 <|python_tag|> form. The shared tool_healing XML pass runs before _parse_llama3_python_tag and does not recognise <|python_tag|>, so a <function=...> / <tool_call> / [TOOL_CALLS] literal quoted inside a <|python_tag|> .call(...) string argument (or its JSON parameters) was promoted and the wrong tool executed. Well-formed single-format examples: <|python_tag|>web_search.call(query="... <function=foo> ...") -> foo <|python_tag|>python.call(code="<function=render_html>..</function>") -> render_html both returned the phantom inner tool instead of the real leading call. Add a leading-<|python_tag|> guard mirroring the other leading-call guards: when the tag is the first tool signal, parse it before tool_healing so quoted foreign markup stays data. A foreign signal before the tag keeps normal document order. Added TestPythonTagOuterOverXmlLiteral (7 cases). * studio: tighten tool-calling comments to be shorter and clearer * studio: shorten tool-format comments in changed files --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <michaelhan2050@gmail.com> Co-authored-by: Daniel Han <info@unsloth.ai> Co-authored-by: danielhanchen <danielhanchen@users.noreply.github.com>
…wen3.5 loop) (unslothai#6804) * Studio: stop chat generation on the assistant-turn-end token A small chat model (e.g. Qwen3.5-0.8B) looped on the safetensors path: it emitted a valid response or tool call, then ran past its turn and re-emitted the call, hallucinating <|im_start|>user turns. Root cause: the model's tokenizer.eos_token is synced to the config document terminator (<|endoftext|>, 248044) while chat turns actually end with <|im_end|> (248046), so generate_stream's single eos_token_id never stopped at the turn boundary. Stop on every assistant-turn-end marker the vocab defines (tokenizer.eos plus <|im_end|>, <|eot_id|>, <end_of_turn>, ...). Verified on the real weights: the single-eos control loops (400 tokens) while the fixed set yields a clean 38-token tool call and a clean answer from the tool result. No-op when eos is already the turn-ender (the id just dedups). * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: repair chat generation_config.eos_token_id at load time Qwen3.5 / Qwen3.6 small chat checkpoints declare the chat turn-end as tokenizer.eos_token (<|im_end|>) but ship config.eos_token_id = <|endoftext|> and no generation_config.json (upstream shipped generation_config only on the large chat models). So every .generate() path that reads generation_config -- the vision path and tool loops, not just generate_stream -- never stops at the turn boundary and loops. At load time, when the tokenizer's own eos is a chat turn-end marker but generation_config.eos_token_id omits it, add it. This fixes the config once for all generation paths and complements the generate_stream turn-end stop. No-op for base models (eos is a plain document terminator) and already-correct configs. Verified on unsloth/Qwen3.5-0.8B: 248044 -> [248044, 248046]. * Studio: derive chat turn-end eos from the template, resolve once at load Address PR review of the turn-end stop handling: - Do not call tokenizer.get_vocab() per generation request (serializes the whole 100k+ vocab). Resolve the turn-end tokens once at load and cache them on model_info; generate_stream reads the cache. - Derive turn-end markers from the chat_template the model actually uses, not raw vocab membership, so a base/coder model that merely carries ChatML control tokens in a shared vocab is not stopped early, and a loader that synced tokenizer.eos to the document terminator is still covered. - Skip harmony/gpt-oss templates: <|end|> there is an intra-message channel delimiter, not the turn end (dropped <|return|> from the marker list too). - Move the logic to a dependency-light module (core.inference.chat_eos) so the unit test does not import the full unsloth/torch inference stack. Verified on unsloth/Qwen3.5-0.8B (gen_config 248044 -> [248044, 248046], clean 38-token tool call with generation_config-only stopping), Phi-3.5 (adds <|end|>), Llama-3 / Qwen3 (unchanged), and a harmony template (left untouched). * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: refresh turn-end eos after the mapper installs its template For a MODEL_TO_TEMPLATE_MAPPER model whose own tokenizer ships no chat_template, the effective template is applied at generate time via get_chat_template, but the turn-end eos ids were resolved once at load when the template was still empty, so only the document eos was cached. Qwen2.5 / Yi base checkpoints (eos <|endoftext|>, ChatML turns end with <|im_end|>) then run past the assistant boundary in generate_stream and loop. Re-resolve the turn-end eos from the now-templated tokenizer and refresh the cached ids right after applying the mapper template, so generate_stream stops at the ChatML turn end. Add a regression test. * Studio: union turn-end eos refresh into load-time cache instead of overwriting get_chat_template can return a different tokenizer whose vocab was remapped (Gemma folds <end_of_turn> onto the eos id), while generate_stream re-reads the original model_info tokenizer. Overwriting the cache with the refreshed set dropped a valid load-time id (e.g. <end_of_turn>=107) and let generation run past the real turn marker. Union the refresh into the existing cache so it can only add ids, never drop a valid one. Add a regression test covering the destructive-swap case the prior test missed. * Studio: resolve refreshed turn-end ids on the generation tokenizer, add Gemma-4 marker Two residual gaps in the turn-end eos refresh: - For map_eos_token=True mapped templates (e.g. chatml on a Yi-6B base), get_chat_template returns a tokenizer whose vocab folds the turn-end token onto the document eos id, while generate_stream re-reads the original tokenizer. The refresh resolved ids on the returned tokenizer, so it stored the doc eos and missed the real turn-end id, and generation ran past the boundary. Read the turn-end marker strings from the mapped template but resolve their ids on the original generation tokenizer (new resolve_chat_turn_end_eos_ids_using). - Add Gemma-4's <turn|> turn terminator to the marker allowlist; those templates keep a document eos so resolve otherwise missed the real turn marker. Add regression tests for both. * Fix turn-end detection for Starling, multi-variant and vision templates; keep tests collectable The turn-end marker set missed OpenChat/Starling's barred <|end_of_turn|> (distinct from Gemma's unbarred form), so Starling generations ran past the assistant boundary. A dict/list chat_template (Hermes-3 style default+tool_use variants) hit an early non-string return and skipped detection; flatten and scan every variant. Vision models carry the chat_template on the ProcessorMixin, not the unwrapped inner tokenizer, so read markers from the template-carrying container while resolving ids on the generation tokenizer. The refresh test constructs the real backend, so it is guarded with a module-level skip when unsloth/unsloth_zoo is absent (the lightweight pytest matrix), and core.inference package init is made lazy so the dependency-light chat_eos tests collect without the heavy stack. * Studio: tighten chat turn-end eos comments * Studio: condense chat turn-end eos comments --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* studio: deterministic backend tool-calling wiring test Add a deterministic, download-free test that exercises the shared tool-calling seam both inference backends use. InferenceBackend (transformers) and MLXInferenceBackend both render the prompt through apply_chat_template_for_generation(..., tools=...) and stream cumulative text into run_safetensors_tool_loop. The existing test_safetensors_tool_loop.py covers the parser and the loop state machine with fake generators but does not cover the backend's own tool-injection seam, so a regression that drops the tool schema before the tokenizer, or fails to feed a tool result back into generation, would slip through. The test drives that seam with fakes: a tokenizer that records the tools it is handed, a canned tool-call generation, and a stub executor. It asserts the full chain: tools reach the chat template, the loop parses the call, the tool is dispatched once with the parsed arguments, the result is fed back, generation re-enters, and the final answer streams after the tool result. It also guards that the raw tool-call markup never leaks to the client as content. The test imports no torch, unsloth, or mlx, so it runs in the portable Backend CI alongside the tool-call parser tests and stays sub-second. Follow-up to the parser test PRs unslothai#5620 and unslothai#5704. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: assert the tool result is fed back before the final turn Strengthen the wiring test so single_turn records each turn's conversation and the test asserts the tool result message is present in the conversation handed to the final generation turn. Event ordering alone did not catch a loop that stops appending the tool output before re-entering generation, because the fake generation ignores the conversation; this closes that gap. * studio: tighten comments in tool-calling wiring test * studio: shorten comments in tool-calling wiring test --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…xes MLX tool follow-up error) (unslothai#6807) * Studio: coerce tool_call arguments to dict before chat templating Strict tool chat templates (e.g. mlx-community Qwen3.5 checkpoints) iterate arguments.items() and raise "TypeError: Can only get item pairs from a mapping" when a prior assistant tool call is re-rendered on the next turn. The agentic loop stores arguments in the OpenAI JSON-string form (as_assistant_tool_call), which is correct on the wire and for llama-server, but the transformers / MLX paths apply_chat_template directly and hit the strict Jinja templates. Normalize each assistant tool_call's function.arguments from a JSON string to a dict inside apply_chat_template_for_generation (shared by both the MLX and safetensors paths). A dict renders on strict and lenient templates alike; non-JSON / non-dict values are left untouched, and the OpenAI-format as_assistant_tool_call (used by the GGUF path + API responses) is unchanged. Verified against the real mlx-community/Qwen3.5-2B-8bit template: string args raised the tester's error, the fix renders cleanly, and the lenient unsloth/Qwen3.5-0.8B template still works. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: make tool-arg coercion a string-first fallback (non-regressive) Render the original OpenAI string-arg form first and only coerce arguments to a dict when the template raises the mapping TypeError, instead of always coercing. Any template that already renders is now byte-identical (a template that emits arguments verbatim keeps the JSON string, not a Python dict repr). Verified across Llama-3, Qwen2.5, Qwen3, Qwen3.5, Phi-3.5 (byte-identical) and mlx-community/Qwen3.5-2B-8bit (strict -> fixed). Gemma-3 / Mistral tool-template errors are unrelated (role alternation / tool-id length) and identical with or without the change. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Make core.inference package init lazy so dependency-light helpers import standalone Importing any core.inference submodule ran the package __init__, which eagerly imported orchestrator and llama_cpp; both pull loggers -> structlog (and httpx), so a dependency-light helper like chat_template_helpers dragged in the full heavy stack and its unit test failed to collect in a backend env without structlog. Defer those imports to attribute access via PEP 562 __getattr__, mirroring the lazy pattern already in core/__init__.py. The re-exports resolve unchanged on first access. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Retry dict-coercion for strict templates that raise non-TypeError apply_chat_template_for_generation only retried the OpenAI JSON-string arguments coercion when the first render raised TypeError (the arguments.items() form). The bundled gemma-4.jinja instead rejects string arguments with raise_exception, which surfaces as a Jinja error, so a second tool turn with string function.arguments propagated and failed rather than retrying with the parsed dict. Broaden the outer catch to Exception, still gated on there being a string arg to normalize (normalized is messages -> re-raise), so unrelated template errors and templates that already render are unaffected. * Tighten comments in tool-call argument coercion helper and tests * Tighten tool-call argument coercion comments --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…nslothai#6476) (unslothai#6611) * Quote-aware Gemma strip, symmetric unstarted cleanup, ReDoS anchor Address review findings on the tool-strip and streaming paths: - strip_tool_call_markup stripped Gemma-native spans with a plain regex that stops at the first <tool_call|>, so a literal close marker inside a <|"|>-quoted argument truncated the span and leaked its suffix into visible text. A brace/quote-aware _strip_gemma_native_spans now removes complete spans (keeping an incomplete one unless final), matching the parser's own balance logic. - The Gemma close pattern this PR added (<\|tool_call>.*?<tool_call\|>) had no \Z fallback, so a run of unclosed markers backtracked from every open position (quadratic, and the streaming stripper re-scans per token). It is now anchored to (?:<tool_call|>|\Z) like routes/inference.py's _TOOL_XML_RE, linear with identical output on well-formed input. - _SameTaskStreamingResponse added unstarted_cleanup for the OpenAI passthrough, but the local GGUF/safetensors streams that enter _TrackedCancel before returning only unregister in the generator finally, which never runs if the client disconnects before the body iterator starts, leaking cancel-registry entries. Each such stream now passes unstarted_cleanup to exit its tracker. - __call__ reads _unstarted_cleanup via getattr so a response built through __new__ (the cancel-timing test) without __init__ does not raise AttributeError; the test also sets the attribute explicitly. - Document that the verbatim /v1/chat/completions passthrough delegates <think>/<|tool_call> splitting to llama-server (--jinja, --reasoning-format auto) and is intentionally not re-parsed locally, noting the llama.cpp dependency. Adds a regression test for the close-marker-inside-quoted-argument strip. * Tighten comments on the tool-strip and streaming paths Compress the verbose comment blocks added with the Gemma tool-call / streaming work to crisp one or two liners, drop restatements of obvious code, and shorten docstrings, keeping the load-bearing rationale (ReDoS anchor, quote-aware strip, unstarted-cleanup, llama.cpp passthrough dependency). Code is unchanged (verified comment-only via AST/ast signature, docstrings stripped). * Harden Gemma parse/strip: span-aware XML fallback and quote-aware streaming - Security: the XML fallback in parse_tool_calls_from_text scanned the whole content for <function=...> markers and only skipped those inside an open XML parameter, not those inside a collected JSON/Gemma candidate span. A balanced but unparsable Gemma call whose argument data contained XML tool markup (<|tool_call>call:outer{code:<function=terminal>...}<tool_call|>) therefore fell through to the fallback and returned an executable terminal call. The fallback now also excludes <function=> markers inside any candidate span, including ones that failed to parse. - strip_tool_call_markup no longer skips the generic Gemma regex after running the quote-aware _strip_gemma_native_spans, so a closed Gemma span the helper cannot match (malformed, e.g. <|tool_call>{"name":"x"}<tool_call|>) is still stripped instead of leaking its opener and payload into visible text. - _strip_gemma_native_spans stops at the first unbalanced start instead of re-scanning every later start to EOF, keeping it linear on a run of unclosed markers rather than quadratic. - The GGUF and safetensors streaming strippers run _strip_gemma_native_spans before the regex patterns, so a well-formed streamed call whose quoted argument contains a literal close marker no longer leaks its suffix into incremental display. Adds regression tests for the nested-XML escape and the malformed-span strip. * Avoid remainder copy in _strip_gemma_native_spans Match the Gemma close marker with re pos directly on the buffer instead of slicing tail = text[brace_end + 1:] on every span. The streaming strippers re-scan a growing cumulative buffer per token, so the per-span remainder copy was quadratic. Behavior is unchanged. * Exclude unclosed Gemma/JSON starts from the XML tool-call fallback The nested-XML guard only skipped <function=> markers inside recorded candidate spans, but a span is recorded only when the braces balance. An unbalanced call such as <|tool_call>call:outer{code:<function=terminal>... recorded no span, so the fallback still promoted the inner <function=> to an executable terminal call. Treat unclosed JSON/Gemma starts as exclusion spans through EOF before scanning. Standalone <function=> calls with no preceding unclosed start still parse. Regression tests added. * Skip doomed tool-strip passes to avoid quadratic rescans The lazy closed-pair strip patterns (<tool_call>.*?</tool_call>, <function=...>.*?</function>) rescan to EOF from every opener when their close token is absent, which is O(n^2) and re-runs per streamed token. Add strip_tool_patterns, which skips a pass whose close token is not present in the text; output is identical to the per-pattern loop (verified by fuzz), and a degenerate run drops from ~minutes to milliseconds. Used by strip_tool_call_markup and the GGUF/safetensors streaming strippers. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Use full tool-call envelopes to close nested-XML escape variants Key the parser and stripper off the full <|tool_call>...<tool_call|> / <tool_call>...</tool_call> envelope (start to close marker, searched after the braces; EOF if unclosed) instead of just the braces: - XML between the closing brace and the close marker (call:outer{broken:{x}}<function=terminal>...<tool_call|>) is now inside the envelope, so the fallback no longer promotes it to a tool call. - A balanced inner call inside an unclosed outer (call:outer{code:<|tool_call>call:terminal{...}<tool_call|>) is skipped via the envelope nested check, not just the XML fallback. - strip_tool_call_markup searches for the close marker after the braces, so junk before <tool_call|> is stripped through the close and text after it is preserved instead of truncated to EOF; a no-close run stops early (linear). Regression tests added; standalone XML and well-formed calls unaffected. * Fix non-final Gemma strip and missing-close recovery for PR unslothai#6611 Split the nested-skip from the XML fallback exclusion: nesting is decided by each marker's brace region, so a balanced call after one with a missing close marker is recovered instead of being swallowed to EOF. Only the XML fallback keeps the search-to-close envelope, so trailing nested markup still cannot escape as an executable call. Use a closed-only Gemma pattern in the non-final strip list so an incomplete block is preserved (matching the JSON and function paths); the final list keeps the close-or-EOF Gemma pattern in its original position, so streaming display output is byte-for-byte unchanged. Add regression tests for both cases. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Block gap-nested tool markers and fix XML strip order for PR unslothai#6611 Decide candidate nesting by a per-marker coverage region paired with a per-format stack (a close after the braces pops the nearest still-open marker of that format). A closed outer call now covers up to its own close marker, so a JSON or Gemma tool marker smuggled between the outer braces and that close is treated as data instead of being executed. An outer that balances but has no close of its own covers only its brace region, so a later sibling after an omitted close marker is still recovered (adjacent calls use an exclusive end bound so the next call is not misread as nested). Strip every closed pair (JSON, Gemma, function) before any to-EOF sweep, so a closed function call whose parameter text contains a bare Gemma opener is removed as a unit and the to-EOF sweep can no longer drop the visible text after the close. Add regression tests for both. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Strip closed tool blocks before the Gemma final sweep for PR unslothai#6611 The final display strip ran the quote-aware Gemma helper before the closed JSON/function patterns. A closed <tool_call>...</tool_call> or <function=...>...</function> block whose argument data held a call-form Gemma opener (e.g. a "<|tool_call>call:t{" string) was read as an incomplete Gemma span and truncated to EOF, dropping the block's close and any visible text after it. Strip closed JSON/function blocks first, so such a block is removed as a unit before the helper runs. Centralize the final strip order in a shared strip_tool_markup_final so strip_tool_call_markup and both streaming display wrappers (safetensors, llama_cpp) stay in sync, and apply the same closed-block pre-pass to the non-final path. Add regression tests for the JSON and function variants. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Recover XML/JSON siblings after a close-less tool marker for PR unslothai#6611 Two fixes so the XML fallback and marker coverage recover a later valid call after an earlier marker omits its close, matching the candidate loop: Reuse the candidate marker-coverage in the XML fallback instead of a separate search-to-close-or-EOF envelope. A balanced but close-less marker now covers only its brace region there too, so a following <function=...> sibling is recovered rather than filtered as nested data; an unbalanced marker still covers to EOF and a closed one still covers through its close, so nested XML stays blocked. Ignore a close token that falls inside another call's balanced braces when pairing closes in _marker_coverage. Such a token is that call's quoted argument data, so it no longer pops an earlier close-less marker and extends its coverage over a later valid sibling. Add regression tests for both. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Make the closed-block strip pre-pass Gemma-span-aware The final display strip ran the closed JSON/function regex pre-pass before removing Gemma-native spans, so a literal <function=...> quoted inside a Gemma argument plus any later </function> (a real call's close or even prose) was deleted across the Gemma boundary. That mangled the Gemma close marker, the quote-aware helper then saw an unclosed opener, and the whole visible tail after the call was truncated. The pre-pass now skips matches that start inside a complete Gemma span (that text is the span's argument data) and resumes scanning at the end of the covering span, so a real function-XML call after the Gemma call is still stripped. The original ordering rationale is preserved: a Gemma opener inside a JSON or function argument still cannot truncate that block, covered by regression tests for both directions. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Trim comments in the Gemma streaming and strip pipeline to essentials * Tighten comments in the Gemma strip and streaming disconnect paths * Fold marker-collection comment to two lines --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…afetensors + MLX (unslothai#5624) * studio: tool calling for Llama-3, Mistral, Gemma 4 on safetensors + MLX (#5615) Adds tool calling for Llama-3, Mistral (pre-v11 + v11+ + [ARGS]), and Gemma 4 to the safetensors / transformers and MLX backends. Parser patched against llama.cpp / vLLM / SGLang per-family parsers and normalises to OpenAI shape. 96 targeted unit tests + cross-OS staging CI (ubuntu / macos-14 / windows) green on the multi-format probe. * studio: tool-call healing parity between safetensors / MLX and GGUF After the multi-format parser landed in #5615, the safetensors / MLX agentic loop and the GGUF loop still differed on healing behaviour. This commit closes the gaps in both directions so the two backends react the same way to identical model output. Changes: 1. core/inference/llama_cpp.py -- the GGUF BUFFERING state machine now wakes on every emission marker the shared parser knows. Was ("<tool_call>", "<function="); is now the five-tuple imported from core.inference.tool_call_parser (Qwen / Qwen3.5 / Llama-3 <|python_tag|> / Mistral [TOOL_CALLS] / Gemma 4 <|tool_call>). Stream cleanup is delegated to the same shared strip_tool_markup so leaked markup from any family is removed from assistant content. 2. core/inference/llama_cpp.py -- per-tool canonical heal key. When a tool arguments field is a bare string and JSON parsing fails, the GGUF path now heals to {"code": raw_args} for python, {"command": raw_args} for terminal, and {"query": raw_args} for everything else. Was hard-coded to {"query": raw_args}, which silently routed every python / terminal emission through web_search. Mirrors safetensors_agentic._CANONICAL_HEAL_ARG. 3. core/inference/safetensors_agentic.py -- re-prompt on plan- without-action. When the model emits a short forward-looking intent ("I'll search for that", "Let me check", "First, I will...") and no tool call, the loop nudges the model to act instead of silently returning a plan-only answer. Up to _MAX_REPROMPTS=3 (matches GGUF). The intent regex, character cap, and instruction text are byte-identical to the GGUF path. The buffer-end fall-through is unified so a buffered intent emission that never exits the BUFFERING state still triggers the re-prompt. 4. core/inference/safetensors_agentic.py -- extra iteration slots for re-prompts. The loop now budgets max_tool_iterations + _MAX_REPROMPTS + 1 total iterations and tracks the tool-call count separately, so a stalling model can be nudged 3x without eating the caller's tool-call budget. Mirrors the _extra slot reservation in the GGUF path. Tests (14 new safetensors-side units; 5 GGUF parity pins): TestLoopRePrompt -- intent-trigger, plain-answer, no-tools, cap-at-three, budget preserved, buffer-end intent. TestLoopCanonicalHealKey -- python / terminal / unknown. TestGGUFSafetensorsHealingParity -- shared markers used, shared strip used, canonical heal keys identical, intent regex matches same phrases, _MAX_REPROMPTS equal on both backends. All 110 targeted tests pass locally; the broader tool / inference / model-config / sandbox / anthropic / mlx suites stay green. Why this matters Without this parity, Llama-3.2 / Mistral / Gemma 4 emissions on Mac (MLX) and Linux-safetensors stop the agentic loop as soon as the model says "Let me...", because the GGUF re-prompt logic never existed on these backends. The two-marker GGUF BUFFERING tuple also let non-Qwen tool emissions stream out as plain prose when llama-server's structured channel did not pick them up. Both paths now drain the same way, heal the same way, and re-prompt the same way -- so a tool call that works on GGUF works identically on safetensors / MLX. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: fix tool-call parser bugs from gemini review on #5620 Three high-priority gemini findings on the tool-call parsing additions: 1. unicode_escape on UTF-8 bytes corrupts non-ASCII literals (e.g. ✨ becomes â\x9c¨). Replace with json.loads on a quoted string -- preserves emoji / CJK / RTL while still handling \n \t \uXXXX escapes. 2. Llama-3 sentinel stripping is order-dependent. A leading `<|eot_id|><|begin_of_text|>` left `<|begin_of_text|>` behind because the loop had already passed that sentinel. Loop until no sentinel matches at the start. 3. Mistral v11+ `[TOOL_CALLS] name { json }` regex uses non-greedy `\{.*?\}` which truncates at the first `}` of a nested JSON argument, leaking the tail (e.g. `}}`) into user-visible streamed text. Same problem for the v0.3 array pattern with nested brackets. Strip those with balanced brace/bracket scanning via a new `_strip_mistral_closed_calls` helper called from `strip_tool_markup`. Also fix the inference routes' parallel `_TOOL_XML_RE`: - Same nested-JSON truncation in the Mistral patterns; route the strip through the parser's balanced-scan helper via a thin `_strip_tool_xml` wrapper that all existing callers now use. - Llama-3 `<|python_tag|>[^\n<]*` stopped at any `<`, leaking the tail of any tool call whose argument contained a literal `<` (queries, code snippets). Relax to `[^\n]*` which keeps the strip confined to the actual end-of-line. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: tool calling for DeepSeek (R1/V3/V3.1), GLM 4.x, Kimi K2 Adds three more emission-family parsers to tool_call_parser.py so the shared safetensors / MLX / GGUF agentic loop covers the major open- weight reasoning families. Patterns ported from llama.cpp (common/chat-parser.cpp legacy pre-PEG branch), vLLM (tool_parsers/deepseekv3*, glm4_moe, kimi_k2), and SGLang (function_call/deepseekv31_detector, glm4_moe_detector, kimik2_detector). All three references are MIT (llama.cpp) or Apache-2.0 (vLLM, SGLang). Formats covered: DeepSeek R1 <｜tool▁calls▁begin｜><｜tool▁call▁begin｜>function <｜tool▁sep｜>NAME\n```json\n{...}\n```<｜tool▁call▁end｜> <｜tool▁calls▁end｜> -- args wrapped in a Markdown json fence, ``function`` literal prefix per llama.cpp common_chat_parse_ deepseek_r1 (chat-parser.cpp:801-820) DeepSeek V3/V3.1 <｜tool▁calls▁begin｜><｜tool▁call▁begin｜>NAME <｜tool▁sep｜>{json}<｜tool▁call▁end｜><｜tool▁calls▁end｜> -- bare JSON, no code fence, no ``function`` prefix per llama.cpp common_chat_parse_deepseek_v3_1 (chat-parser.cpp:822-879) GLM 4.5/4.6/4.7 <tool_call>NAME\n<arg_key>k1</arg_key> \n<arg_value>v1</arg_value>...</tool_call> -- strings raw, non-strings JSON-encoded per chat_template.jinja; multi-call is back-to-back blocks. Per llama.cpp common_chat_parse_glm_4_5 (chat-parser.cpp:1040-1052) Kimi K2 <|tool_calls_section_begin|><|tool_call_begin|> functions.NAME:IDX<|tool_call_argument_begin|>{json} <|tool_call_end|><|tool_calls_section_end|> -- bare name recovered by stripping ``functions.`` prefix and ``:IDX`` suffix; full id preserved as tool_calls[i].id so the roundtrip replays verbatim. Per llama.cpp common_chat_parse_kimi_k2 (chat-parser.cpp:896-913) Marker collisions GLM uses the same ``<tool_call>`` opener as Qwen but with a bare function name + ``<arg_key>`` body (Qwen has ``\s*{`` after the tag). The dispatch keeps Qwen first; Qwen's _TC_JSON_START_RE returns no matches on a GLM emission, so the fall-through to _parse_glm_tool_ calls handles it correctly. Existing Qwen tests confirm zero regression. Streaming buffer TOOL_XML_SIGNALS extended from 5 markers to 12 so the BUFFERING state machine wakes on every new family's section opener. Added the DeepSeek alternative markers (ASCII underscores, short ``<｜tool▁calls｜>`` form) because real checkpoints emit those variants. Strip patterns _TOOL_CLOSED_PATS adds DeepSeek envelope (``<｜tool▁calls▁begin｜>... <｜tool▁calls▁end｜>``) and Kimi section (``<|tool_calls_section_begin|> ...<|tool_calls_section_end|>``). _TOOL_ALL_PATS adds the same plus the unclosed-tail variants so a truncated stream does not leak markup. Route gate _detect_safetensors_features._PARSER_MARKERS grows to include DeepSeek and Kimi markers plus ``<arg_key>`` (the unique GLM signal). _TOOL_XML_RE (the route-layer markup-strip regex) gets DeepSeek and Kimi closed-pair patterns. _TOOL_TEMPLATE_MARKERS in llama_cpp.py adds ``message['role'] == 'tool'``, ``message['tool_calls']``, and ``tool_calls is defined`` so the classifier recognises DeepSeek's subscripted-access template style (it has no top-level ``{% if tools %}`` block). Tests (39 new): TestParserDeepSeek (7) -- R1 fence, short-form opener, V3.1 bare, multi-call, with-reasoning, strip, signal-wakes-streaming TestParserGLM (6) -- single, mixed types, multi-call, unclosed-heal, no-Qwen-regression, strip TestParserKimi (6) -- single, multi-call, dotted-name, unclosed, strip, signal-wakes-streaming TestParserCrossFormatRouting (2) -- dispatch routing, signal coverage TestLoopBasic loop integration (3) -- DeepSeek / GLM / Kimi end-to-end Capability advertise (3) -- DeepSeek / GLM / Kimi templates flip supports_tools=True All 398 targeted tests pass locally (115 safetensors + 27 capability + rest of tool / inference / sandbox / model-config suites). Builds on PR #5620 (parser + healing parity for Llama-3 / Mistral / Gemma 4); will rebase cleanly onto main once #5620 lands. PR opened as draft - do not merge until validated against real models for each family. Sources - llama.cpp common/chat-parser.cpp lines 801-913, 1040-1052 (MIT) - vLLM vllm/tool_parsers/deepseekv31_tool_parser.py (Apache-2.0) - vLLM vllm/tool_parsers/glm4_moe_tool_parser.py (Apache-2.0) - vLLM vllm/tool_parsers/kimi_k2_tool_parser.py (Apache-2.0) - SGLang python/sglang/srt/function_call/{deepseekv31,glm4_moe,kimik2}_ detector.py (Apache-2.0) - Live chat templates: deepseek-ai/DeepSeek-V3.1, zai-org/GLM-4.6, moonshotai/Kimi-K2-Instruct, unsloth/DeepSeek-V3-0324, unsloth/GLM-4.5-Air, unsloth/Kimi-K2-Instruct * studio/routes: make python_tag strip multi-line aware Earlier revisions of _TOOL_XML_RE in studio.backend.routes.inference oscillated between two bug shapes: 5615 r"<\|python_tag\|>[^\n<]*" -- stopped at any literal "<" so code='if x < 10: pass' leaked '< 10: pass)' to the user. 5620.1 r"<\|python_tag\|>[^\n]*" -- single-line only; the second line of python.call(code="a\nb") leaked. The full parser (_parse_llama3_python_tag) already handles both via balanced-brace scanning, so the parsing path was fine; the LEAK was in the streaming strip path that runs on every cumulative emission while content is still arriving. Switch to r"<\|python_tag\|>(?:[^<]|<(?!\|))*" so the strip consumes: * any character that is not a "<" (newlines, JSON, code, ...), * a "<" only when it is NOT followed by "|" (i.e. NOT a Llama-3 sentinel start like <|eot_id|>, <|eom_id|>, <|begin_of_text|>). This means: * code='if x < 10' stays inside the strip (5615 fix preserved), * multi-line code stays inside the strip (5620 round 2), * the strip terminates at the next Llama-3 sentinel so trailing assistant content survives. Tests: TestRoutesPythonTagStrip (8 cases) pytest test_safetensors_tool_loop.py test_safetensors_capability_advertise.py -> 118 passed in 1.81s (was 110). * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: review follow-ups for DeepSeek / GLM / Kimi tool calling Four fixes addressing review of the parent commit: 1. GLM <arg_value> coercion: tighten the json.loads -> ast.literal_eval -> raw cascade to only deserialize when the body unambiguously looks like a JSON literal (object, array, JSON-encoded string, true/false/null, or numeric). Strings like ``True`` / ``None`` (Python literals, not JSON) and arbitrary prose now stay raw. The bare-numeric / bare-boolean ambiguity with string args remains an inherent limitation of the template without schema access -- documented in the new comment. Drops the ast import entirely (closes Gemini's :1036 suggestion). 2. Kimi K2 bare-counter ids (e.g. ``<|tool_call_begin|>3``) are now dropped rather than surfaced as a tool literally named "3". Matches vLLM behaviour; SGLang's schema-infer fallback is out of scope at the parse site. Real Kimi K2 emissions use ``functions.NAME:IDX`` so this is the exception path. 3. Restore the elaborate ``<|python_tag|>(?:[^<]|<(?!\|))*`` clause in routes.inference._TOOL_XML_RE -- the simpler ``[^\n<]*`` form regressed PR #5620's multi-line / literal-``<`` python_tag fix. Restore ``TestRoutesPythonTagStrip`` (8 tests) adapted to call ``_TOOL_XML_RE.sub`` directly since the ``_strip_tool_xml`` helper was inlined this PR. 4. Add the spaced and backslash-escaped DeepSeek opener variants (``<｜tool calls begin｜>``, ``<｜tool\_calls\_begin｜>``) to ``TOOL_XML_SIGNALS`` for streaming-gate parity with ``_DEEPSEEK_BEGIN_RE``. Also updates the llama.cpp / vLLM citations in the parser docstrings: ``common/chat-parser.cpp`` was split into ``common/chat.cpp`` + ``common/chat-peg-parser.cpp`` by llama.cpp PR #18675, and vLLM moved the tool parsers from ``vllm/entrypoints/openai/tool_parsers/`` to ``vllm/tool_parsers/``. Pin to pre-refactor commit ``51fa458a92d6`` where the cited line numbers still resolve. New regression tests in ``test_pr5624_regressions.py`` cover the GLM coercion heuristic shapes, GLM literal-``<`` in arg_value, Kimi K2 dotted name, Kimi K2 bare-counter drop, DeepSeek V3.1 truncated mid-stream, and routes-layer strip across all three new families. Tests: pytest studio/backend/tests/test_safetensors_tool_loop.py studio/backend/tests/test_safetensors_capability_advertise.py studio/backend/tests/test_pr5624_regressions.py -q -> 170 passed in 1.91s * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: tighten verbose comments in tool-call parser sections Comments were narrating what the code already says. Cut historical "earlier revisions used X, then Y" narratives down to one-line WHY notes where the footgun still matters (canonical heal-key parity, balanced-brace vs non-greedy regex, ``(?:[^<]|<(?!\|))*`` over ``[^\n<]*``/``[^\n]*``). Drop section-header banners. No behaviour change. Re-ran: pytest studio/backend/tests/test_safetensors_tool_loop.py \ studio/backend/tests/test_safetensors_capability_advertise.py -q -> 118 passed. Regression replay (parser + _coerce_arguments on the 5 #5615 inputs) -> 21/21. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: GLM 4.7 no-newline emission + Kimi multi-section parity Two fixes surfaced by triple-confirm verification against the live HF chat templates and upstream llama.cpp / vLLM / SGLang parsers. 1. GLM 4.7 silent drop ``zai-org/GLM-4.7/chat_template.jinja`` line 65 uses ``{{- '<tool_call>' + tc.name -}}`` which Jinja strips trailing whitespace from, so the first ``<arg_key>`` follows the function name with NO ``\n`` between them. Real emissions look like ``<tool_call>get_weather<arg_key>city</arg_key><arg_value>London </arg_value></tool_call>``. The previous ``_GLM_TC_OPEN_RE`` ended the name with ``\n`` so GLM-4.7 calls were silently dropped (parser returned ``[]``). Fix: relax the name terminator to a lookahead that accepts EITHER ``\n`` OR the next ``<arg_key>``: _GLM_TC_OPEN_RE = re.compile( r"<tool_call>\s*([^\n<{][^\n<]*?)\s*(?=\n|<arg_key>)" ) The first-char restriction ``[^\n<{]`` still excludes Qwen's ``<tool_call>{json}`` form so the Qwen-vs-GLM dispatch remains mutually exclusive. 2. Kimi multi-section parity with vLLM / SGLang ``vllm/tool_parsers/kimi_k2_tool_parser.py`` and SGLang's ``kimik2_detector.py`` both use ``re.findall`` and so collect every ``<|tool_calls_section_begin|>...<|tool_calls_section_end|>`` block in a single stream. The previous implementation stopped at the first ``<|tool_calls_section_end|>``. Kimi K2 doesn't emit multi-section in practice, but parity is cheap. Fix: wrap the existing per-call body parser in an outer loop that advances past each ``<|tool_calls_section_end|>`` and continues to the next ``<|tool_calls_section_begin|>``. Body parsing extracted to ``_parse_kimi_section_body`` for clarity. Truncated final section is still surfaced via the existing in-body balanced-brace walk. Verified independently against the live HF templates: * GLM-4.7 emission constructed from the live template parses to the expected ``{name, arguments}`` shape. * GLM-4.5 / 4.6 newline shape continues to parse (the lookahead also matches ``\n``). * Qwen ``<tool_call>{json}`` still dispatches to the Qwen path -- the first-char restriction stops the GLM regex from biting JSON bodies. * Kimi two-section stream surfaces both calls in order with full ids preserved. * Bare-counter Kimi ids still drop. Tests added in ``test_pr5624_regressions.py``: * ``test_glm_4_7_no_newlines_between_name_and_arg_key`` * ``test_glm_4_7_no_newlines_multi_call`` * ``test_glm_4_7_does_not_break_qwen_path`` * ``test_kimi_two_sections_in_one_stream_both_parse`` pytest studio/backend/tests/test_safetensors_tool_loop.py studio/backend/tests/test_safetensors_capability_advertise.py studio/backend/tests/test_pr5624_regressions.py -q -> 174 passed in 1.93s pytest studio/backend/tests/ -q -k 'not gpu and not llama_cpp_integration' -> 2038 passed, 15 failed (pre-existing CI gaps). * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: parser robustness fixes for PR #5620 Three surgical extensions to the multi-format tool-call parser, each covering a real fine-tune / template emission shape that the current parser silently drops. No path narrows; all changes widen what is accepted. 1. `_parse_tool_call_json` now accepts both `arguments` and `parameters` keys. A Hermes / Qwen `<tool_call>{json}</tool_call>` wrapper around a Llama-3.2 fine-tune that emits the `parameters` key was extracting the tool name and silently discarding the args, producing a working-shaped call with an empty payload. The bare-JSON and python_tag paths already accepted both keys; this path now matches them. 2. `_TC_FUNC_START_RE`, `_TC_PARAM_START_RE`, and `_TC_PARAM_CLOSE_RE` now also match the attribute form `<function name="..."><param name="...">v</param></function>` used by MiniCPM-5 and MiniMax-M2. Names land in either capture group, and `</param>` is accepted as a short close. 3. `_parse_llama3_bare_json` sentinel-strip now consumes the role label inserted between `<|start_header_id|>` and `<|end_header_id|>` by Meta's official Llama-3.x chat template. Without this, every assistant turn re-fed through the template prefix `<|start_header_id|>assistant<|end_header_id|>\n\n{json}` parsed to zero calls, so any history-with-tool-call round-trip in production silently dropped. Tests in `studio/backend/tests/test_safetensors_tool_loop.py`: * `TestParserRobustness::test_tool_call_json_accepts_parameters_key` * `TestParserRobustness::test_function_xml_attribute_form` * `TestParserRobustness::test_function_xml_attribute_form_multi_param` * `TestParserRobustness::test_function_xml_legacy_equals_form_still_works` (regression guard for the existing `<function=name>` syntax) * `TestParserRobustness::test_llama3_chat_template_round_trip` * `TestParserRobustness::test_llama3_round_trip_all_roles` * `TestParserRobustness::test_llama3_round_trip_with_eot_prefix` `pytest studio/backend/tests/test_safetensors_tool_loop.py studio/backend/tests/test_safetensors_capability_advertise.py -q` goes from 118 to 125 passed. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Trim verbose comments in tool-call parser sections for PR #5624 Pure comment / docstring tightening on top of the GLM 4.7 + Kimi multi-section fixes. No behavioural change. * Drop multi-paragraph prelude and post-refactor citation chatter in the DeepSeek, GLM and Kimi parser docstrings; keep the shape and upstream-commit pin. * Collapse ``parse_tool_calls_from_text``'s 9 per-family blocks into a single ordered loop with one combined comment. * Tighten the GLM coercion, Kimi bare-counter and ``_TOOL_XML_RE`` comments to one or two lines each. * Same trim pass on ``_PARSER_MARKERS`` and the regression-test docstrings. Tests: pytest studio/backend/tests/test_safetensors_tool_loop.py studio/backend/tests/test_safetensors_capability_advertise.py studio/backend/tests/test_pr5624_regressions.py -q -> 174 passed in 2.00s * Fix O(N^2) DeepSeek V3.1 backtracking for PR #5624 Adversarial input ``<｜tool▁calls▁begin｜><｜tool▁call▁begin｜>fn<｜tool▁sep｜>`` followed by a long body that does NOT contain a closing brace caused the V3 path's ``([^\n<]+?)<｜tool▁sep｜>`` regex to backtrack quadratically: at each position the lazy quantifier extends one char at a time looking for a sep that isn't there, taking ~19s on 50k chars. Replace the regex search with ``str.find`` on the sep marker plus a left-walk to recover the name. ``str.find`` is O(N); the walk stops on ``\n`` (turn boundary), ``<`` (start of a tag), or ``>`` (end of an optional ``<｜tool▁call▁begin｜>`` prefix). Same observable behaviour as the regex on every canonical input. Tests: test_deepseek_v3_1_huge_truncated_body_is_linear (new) -- 50k chars must parse in < 1s. pytest studio/backend/tests/test_safetensors_tool_loop.py studio/backend/tests/test_safetensors_capability_advertise.py studio/backend/tests/test_pr5624_regressions.py -q -> 175 passed in 1.97s pytest studio/backend/tests/ -q -k 'not gpu and not llama_cpp_integration' -> 2038 passed, 15 pre-existing failures unchanged. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: terminate function-XML body at </function>, not just </tool_call> `_parse_function_xml` was looking for `</tool_call>` (the Hermes wrapper) as the body terminator. When a model emits a standalone `<function=NAME><parameter=K>v</parameter></function>` followed by explanatory prose (which models routinely do), no `</tool_call>` is present, so the body extended to end-of-string and the trailing prose leaked into the LAST parameter value. Pre-existing on main (the legacy `<function=NAME>` form had this bug too). Same affects PR #5620's new attribute-form `<function name="NAME"><param name="K">v</param></function>` emission used by MiniCPM-5 / MiniMax-M2. Fix: `_TC_END_TAG_RE` now matches either `</tool_call>` OR `</function>`. The existing `_TC_FUNC_CLOSE_RE` / `_TC_PARAM_CLOSE_RE` strips are unchanged. Multi-call inputs still bound each function at the next `<function=` start, so no over-eager consumption. New tests: * `test_function_xml_followed_by_prose` (legacy form + prose) * `test_function_attribute_xml_followed_by_prose` (attribute form + prose) Existing `test_code_with_embedded_xml` still passes (a parameter value containing literal `<a></a>` is preserved because the embedded close tag is `</a>`, not `</function>`). `pytest studio/backend/tests/test_safetensors_tool_loop.py studio/backend/tests/test_safetensors_capability_advertise.py -q` goes from 125 to 127 passed. * Studio: tighten Llama-3.2 bare-JSON guard A fuzz pass on PR #5811 turned up that ``_parse_llama3_bare_json`` accepted ``parameters`` as a string, contradicting the docstring's "parameters or arguments is a dict" guard. Prose JSON like ``{"name":"foo","parameters":"a sentence"}`` would wrongly fire the parser, which the agentic loop would then heal into a real ``foo(query="a sentence")`` call. Same code lives on this branch, so the same fix applies here. Tightened guard: - ``parameters`` must be a dict (Llama-3 spec). - ``arguments`` may be a dict, or a JSON-encoded string that decodes to a dict (OpenAI shape, e.g. ``"arguments":"{\"q\":\"x\"}"``). Plain non-JSON strings or JSON-strings of lists / scalars / null no longer pass. Mirrors the fix landed in PR #5811 commit 615b8608. Adds the same 4 regression tests under TestParserMultiFormat. Existing test suite stays green: 127 -> 131 passing. * Studio: skip non-scalar args in python_tag JSON form The JSON sub-path of ``_parse_llama3_python_tag`` was fabricating ``{"value": args}`` when the model emitted a non-dict / non-string ``arguments`` value (e.g. ``42``, ``[1,2,3]``, ``null``, ``true``). This silently turned a malformed emission into a real tool call, which the agentic loop would then execute with arguments the model never intended. Tightened: skip the call instead of fabricating. The same behaviour now matches the bare-JSON guard tightened earlier (strict-guard merge from PR #5620, inherited via merge here). Added a regression test covering the four non-scalar shapes. Pass count on this branch: 158 -> 159. Sites in ``_parse_tool_call_json`` and ``_consume_mistral_call`` keep the existing looser behaviour for now; both are reached only after explicit ``<tool_call>`` / ``[TOOL_CALLS]`` markers so the false-positive surface there is much narrower. * studio: fix safetensors tool-call parser gaps vs llama.cpp (Mistral CALL_ID / THINK, attribute-form signal) Three GGUF-parity fixes to the safetensors tool-call parser, each matching llama.cpp's reference behaviour: - Mistral Small 3.2 emits [TOOL_CALLS]name[CALL_ID]<id>[ARGS]{json}. The parser stopped after the name on seeing [CALL_ID] (neither [ARGS] nor {), dropping the call. Skip an optional [CALL_ID]<id> segment in both the parse and strip paths. llama.cpp parses this (test-chat.cpp:4785). - Magistral wraps reasoning in [THINK]...[/THINK]. A [TOOL_CALLS] inside the reasoning was parsed as a real call, producing a phantom call. Strip a leading [THINK] block before scanning so only the post-reasoning call counts (test-chat.cpp:2285); a literal [THINK] inside a later argument is left intact. - The standalone MiniCPM-5 / MiniMax-M2 <function name="..."> attribute form parsed correctly but was absent from TOOL_XML_SIGNALS and the markup strip patterns, so the streaming safety-net parse was gated off (dropping the call) and markup leaked into displayed text. Add the signal and broaden the strip regexes. Adds regression tests for all three. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: fix GLM and Kimi K2 safetensors tool-call parser gaps vs llama.cpp Four GGUF-parity fixes for the GLM and Kimi K2 families: - GLM 4.7 zero-argument inline call <tool_call>name</tool_call> was dropped: the open-tag lookahead only allowed \n or <arg_key> after the name. Allow </tool_call> too so a no-arg call parses to empty args (vLLM / SGLang / llama.cpp all parse it). - GLM string argument values were stripped, losing significant leading / trailing whitespace in code / diff arguments. Keep the raw value for the string fallback and only strip the copy used to probe for a JSON literal, matching vLLM glm4_moe which never strips string args. - Kimi K2 calls emitted without the <|tool_calls_section_begin|> wrapper were dropped. llama.cpp makes the section optional (Kimi can call a tool straight after reasoning without opening a section); parse a bare <|tool_call_begin|> when no section is present. - Kimi K2 malformed / truncated JSON in one call dropped every later call in the section. Skip the bad call and keep parsing so valid subsequent calls are recovered (vLLM parity). Adds regression tests for all four. * studio: fire safetensors tool calls for the bare-JSON (Llama-3.2) form The agentic loop's streaming safety-net parse was gated on has_tool_signal(), which is False for the Llama-3.1 / 3.2 bare-JSON tool form {"name":..,"parameters":..} (no XML marker). Real tool calls were therefore dropped: the loop logged "model planned without calling tools", re-prompted three times, then gave up with zero tool calls, while GGUF's llama-server parses the same emission natively. Run parse_tool_calls_from_text() unconditionally in the safety net. The parser is strict (only fires on a valid tool-call shape) so plain answers are unaffected. Reproduced on a real unsloth/Llama-3.1-8B-Instruct run: the model emits {"name":"web_search","parameters":{...}} which now executes the tool instead of being re-prompted into a no-op. Adds a loop regression test for the bare-JSON form. * studio: fire safetensors tool calls for Gemma 4 (native template + stripped parser) Gemma-4 safetensors fired no tools while its GGUF fired reliably. Three gaps: - The Studio swaps in the Unsloth "gemma-4" chat template, which does not render the tools schema (the model's native template does), so the model never saw the tools. Fall back to the model's native template when the override template renders identically with and without tools. Same fix helps any family whose override template drops tools. - skip_special_tokens strips the <|tool_call> wrapper and <|"|> string markers, so a streamed Gemma-4 call arrives as a bare call:NAME{k:v, ...} with unquoted values. Parse that form, keeping commas/braces inside a code or command value, normalising surrounding quotes, and stripping the leaked markup from the final answer. - Without a grammar a small model can loop, repeating one call for the whole tool budget. Collapse exact-duplicate calls within a turn and force a final answer after a turn that made no new tool progress (llama-server's lazy grammar prevents this loop on the GGUF side). Adds parser tests for the bare/stripped Gemma-4 form. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: complete strict-mode contract and fix parser import paths Address review findings on the multi-format tool-call parser: - Honor allow_incomplete=False in the remaining sub-parsers. The Llama-3 <|python_tag|>NAME.call(...) parser, the pre-v11 Mistral [TOOL_CALLS] array parser, and the Gemma 4 <|tool_call> parser ignored strict mode, so a truncated call (missing closing paren, ], or <tool_call|>) was still healed and executed with Auto-Heal disabled. Thread strictness through and reject the unclosed forms, matching the JSON and function-XML paths. - Drop the duplicate tool_call_parser import block in llama_cpp.py and the redundant un-aliased TOOL_XML_SIGNALS; only the _SHARED_TOOL_XML_SIGNALS alias is used as a value. - Import _strip_mistral_closed_calls from core.inference.tool_call_parser in routes/inference.py instead of studio.backend.core... The self-contained run.py launch mode only puts studio/backend on sys.path, so the absolute package path raised ModuleNotFoundError on the server-tool strip path. Add strict-mode regression tests for the truncated Llama-3 dot-call and the unclosed Mistral array. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: harden DeepSeek/Kimi tool-call parsing and strip Address review findings on the DeepSeek and Kimi parsers: - Honor allow_incomplete=False for DeepSeek. An envelope with no closing <｜tool▁calls▁end｜> is truncated mid-stream; reject it in strict mode instead of healing the body out to EOF, matching the strict XML and Mistral paths. - Do not skip a following tool call when the current call's end marker is missing. The DeepSeek V3 and Kimi loops advanced by searching forward for the next <｜tool▁call▁end｜> / <|tool_call_end|>, which could land on a later call's end marker and drop the call in between. Advance by the JSON end; the loop re-locates the next call marker from there. - Strip truncated DeepSeek and Kimi section blocks in the route-level display regex. The patterns required the closing marker; add the end-of-text alternative so a block truncated by EOS does not leak raw markup to the UI. Add regression tests for the truncated DeepSeek envelope, and for DeepSeek and Kimi multi-call recovery when the first call's end marker is missing. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: preserve XML param indentation and alias Mistral array parameters Two parser-correctness fixes found by auditing against the model chat templates and the SGLang / vLLM reference parsers: - Qwen3.5 XML parameter values lost their leading indentation. The chat template emits <parameter=k>\nVALUE\n</parameter>, but the parameter-start regex ate the wrapping newline AND the value's first-line indentation with a trailing \s*, then str.strip() removed the rest. Narrow the trailing class to horizontal whitespace only and trim exactly one wrapping newline (via _trim_param_value), preserving indentation in code/diff arguments. Matches SGLang's qwen3_coder detector. Applies to both _parse_function_xml (tool_call_parser.py) and the XML path in tool_healing.py. - Mistral pre-v11 array objects keyed on parameters dropped their payload. _consume_mistral_call read only the arguments key; alias parameters the same way the JSON/XML paths and SGLang's base detector do. Add regression tests for preserved multi-line indentation and the array parameters alias. * Studio: DeepSeek strip sync, Gemma nested args, GLM/Kimi strict mode Parser-correctness fixes found by auditing DeepSeek/GLM/Kimi against vLLM, SGLang, and the model chat templates: - DeepSeek: the short <｜tool▁calls｜> opener (and the space / escaped-underscore spellings) was parsed but never stripped, so a short-opener envelope leaked raw markup to the UI. Share one opener alternation between _DEEPSEEK_BEGIN_RE and the strip patterns (and the route-level display regex) so a signal we parse can never be left un-stripped. - Gemma wrapper-less stream: a nested object/array argument (loc:{city:NYC}, labels:[bug,ui]) was kept as a literal string. Parse it recursively when the bare value is a balanced {} / [], falling back to the raw string for a truncated value. - GLM and Kimi ignored allow_incomplete. With Auto-Heal off, a GLM block with no </tool_call>, a Kimi section with no <|tool_calls_section_end|>, or a Kimi call with no <|tool_call_end|> are truncated and must be rejected, matching the strict behavior of the JSON/XML/Mistral/DeepSeek paths and vLLM/SGLang. Add regression tests for the short-opener strip, the Gemma nested args, and GLM / Kimi strict-mode rejection. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: tighten tool-call parser comments Make the comments in the multi-format tool-call parser and its callers succinct: compress verbose docstrings/blocks to one or two lines, drop ones that restate the code, and trim the tiny balanced-scanner helpers. Correctness rationale and upstream provenance (SGLang/llama.cpp parity, the strict-mode / Auto-Heal contract, whitespace-preservation, and the Unicode / full-width-pipe notes) are kept in compact form. Comment-only: no code or behavior change (verified with comment_tools.py check --strip-docstrings; parser suite green). * Studio: tighten DeepSeek/GLM/Kimi parser comments Compress the comments added for the DeepSeek/GLM/Kimi parsers and the Gemma wrapper-less helpers to one or two lines, keeping the upstream provenance (llama.cpp 51fa458a92d6), the O(N^2) / strict-mode rationale, and the vLLM parity notes intact. Comment-only: no code or behavior change (verified with comment_tools.py check --strip-docstrings; parser suite green). * Studio: make DeepSeek R1 / GLM parsing linear and close routes strip gaps Review follow-up for the DeepSeek/GLM/Kimi parser: - DeepSeek R1 detection used a greedy ``([^\n]+)\n```json`` regex that backtracks O(N^2) on a fence-less truncated body; scan with str.find instead (mirrors the V3 path). - GLM arg pairs used a lazy-group finditer that rescanned to EOF from each bare <arg_key> in an unclosed body (O(N^2)); walk pairs with str.find. - The route display strip (_TOOL_XML_RE) accepted fewer DeepSeek openers than the parser (missed the space / escaped-underscore spellings) and missed bare section-less Kimi calls, so a call we parse could leak raw markup to the UI. Reuse the parser's shared _DEEPSEEK_OPEN_RE_SRC and add a bare-Kimi arm. Add ReDoS-linearity regressions for the R1 and GLM paths, a positive R1 fenced-json parse test, and routes-strip tests for the space/escaped DeepSeek openers and the bare Kimi call. * Studio: fix test_mcp_servers _TOOL_XML_RE reconstruction after _DS_OPEN_SRC reuse The routes strip fix made _TOOL_XML_RE reference the module-level _DS_OPEN_SRC variable. test_mcp_servers reconstructs the regex by exec-ing the extracted compile() source in a namespace that only defined _re, so it raised NameError. Inject _DS_OPEN_SRC into that namespace, matching the same fix already applied in test_tool_xml_strip. * Studio: make Llama-3 .call and Mistral-array healing parsing linear Two more O(n^2) ReDoS paths in the multi-format parser, both reachable from the agentic loop on a long truncated body with no length cap: - _LLAMA3_KV_RE.finditer over a .call(...) body retried at every offset of a long word run / unterminated quote (40K -> 14s). Replace with a hand-scan that reuses the same key/number/literal sub-regexes via anchored match and walks the string body by hand, so an unterminated quote is O(n). Verified byte-identical to the old regex over 200K fuzzed inputs. - _parse_mistral_array healing ran _balanced_brace_end from every { in the body (20K -> 17s). Walk top-level objects, advancing past each balanced {...}; this also drops the phantom call the old scan emitted from a nested argument object. Add adversarial-length linearity regressions plus positive .call kwargs and unclosed-array recovery coverage. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: strengthen #5624 regression assertions and strip-test harness guards - test_strip_tool_markup_handles_deepseek_envelope used `A or B` where B was the preservation property the next line already asserts, masking the real check. Replace with an explicit assertion that the call name and args are stripped. - The test_tool_xml_strip source-extraction harness reconstructs _TOOL_XML_RE and _strip_tool_xml_for_display from routes/inference.py via lazy regexes that could silently grab a shorter slice. Assert the extracted regex carries the DeepSeek / bare-Kimi arms and the helper body reached the _TOOL_XML_RE.sub call. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: honor strict mode in safety-net, keep empty Gemma args, strip attribute-form function XML - safetensors safety-net parser now forwards allow_incomplete=auto_heal_tool_calls, matching the draining path, so a late incomplete tool call is not healed and executed when Auto-Heal is off. - Gemma empty bare value ({k:}) now serialises as "" instead of invalid {"k":}, which previously dropped the whole call. - Route _TOOL_XML_RE also strips the <function name="..."> attribute form (MiniCPM-5 / MiniMax-M2) so it no longer leaks to the UI. * Studio: linearize wrapper-less Gemma nested-arg parsing and correct parser provenance - _gemma_parse_value/_gemma_parse_mapping/_gemma_parse_array now parse nested {}/[] in a single forward pass instead of pre-scanning each subtree with a balanced-brace walk and re-parsing it. Deeply nested wrapper-less Gemma args were O(n^2); they are now ~linear (and ~40x faster at depth 400). - Correct the DeepSeek/GLM/Kimi provenance comments: the cited commit 51fa458a92d6 is unrelated, and GLM/Kimi were never standalone common_chat_parse_* functions (llama.cpp uses common_chat_params_init_glm_4_5 plus a generalized XML parser, PRs #15904 / #16932). - Add tests: Gemma deep-nesting linearity, nested object/array preservation, same-turn distinct-call cap, and the native-template tool-render fallback. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: guard Gemma value parser against non-advancement and missing tokenizer Addresses Gemini review: - _gemma_parse_value now consumes one character when a stray }/]/, sits where a value is expected, so _gemma_parse_array can never stall at the same index on malformed input (a latent infinite loop). - _render_with_native_template returns None when neither a tokenizer nor a processor is present instead of raising AttributeError. - Tests for both. * Studio: fix attribute-form function-XML literal close tag and zero-arg strict call Addresses Codex review of the <function name="..."> attribute form in _parse_function_xml (MiniCPM-5 / MiniMax-M2): - End the call body at the LAST </function> / </tool_call> within the call's window, so a literal close tag inside a code/search argument (e.g. print("</function>")) is preserved instead of truncating the call. - Accept a closed call with no parameters as a valid zero-argument call in strict mode (the function close is already required), instead of rejecting it as a truncated call. - Tests for both, mirroring the legacy <function=...> coverage. * Studio: drop scratch review/planning artifacts from the branch * Studio: fix tool-call parser/loop review findings on the multi-format path Address the live code-review findings on the safetensors/MLX + GGUF tool path: - routes: include the attribute form <function name="..."> in the safetensors capability whitelist so MiniCPM-5 / MiniMax-M2 templates keep the tool pill (parser already handles the form; the post-filter wrongly suppressed it). - safetensors loop: build the plan-without-action re-prompt from the active tools instead of a hardcoded web_search/python string, and gate it on auto_heal_tool_calls, matching the GGUF loop. - safetensors loop: hold a leading bare-JSON object ({"name":..,"parameters":..}) during BUFFERING until it closes, then drain it as a tool call instead of streaming the raw JSON to clients. The DRAINING/STREAMING resolvers still recover a plain JSON answer, so this can never drop content. - parser: anchor the Llama-3 <|python_tag|>NAME.call(...) scan to the tag and chain ; -separated calls, so all semicolon-separated built-ins parse and a literal <|python_tag|>x.call(...) inside a JSON string argument no longer fires the wrong tool. - parser: consume the optional trailing </s> after a named Mistral [TOOL_CALLS]name{json} call, mirroring the array shape. - GGUF streaming strip: use the shared parser patterns (which know [TOOL_CALLS] and <|python_tag|>) so a textual tool call entering DRAINING is stripped instead of leaking the marker to streaming clients. - routes: hoist the _strip_mistral_closed_calls import to module level. Adds regression tests covering each fix; existing parser suite stays green. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: fix DeepSeek/GLM/Gemma tool-call review findings Address the live code-review findings specific to the DeepSeek / GLM / Kimi and native-template additions: - parser: in strict mode (Auto-Heal off) require the per-call <｜tool▁call｜end｜> terminator for DeepSeek V3 calls instead of executing on a bare balanced object closed only by the envelope end. - parser: keep GLM string arguments that begin with a quote verbatim (drop the leading-quote case from the JSON-decode probe) so a quoted search query is not decoded down to its inner text. - parser: reject a GLM call with an unclosed <arg_value> in strict mode, and under Auto-Heal keep the partial value rather than dropping it to a no-arg call. - parser: add a balanced wrapper-less Gemma strip (call:NAME{...}) so a nested object/array argument is removed whole instead of leaving a trailing brace; run the balanced Mistral and Gemma strips on the streaming display paths too. - safetensors loop: buffer a leading wrapper-less Gemma call:NAME{...} so it drains and executes instead of streaming the raw call text. - inference: render the native-template fallback on a shallow tokenizer copy instead of mutating the shared tokenizer outside the generation lock, and load the native template from base_model for LoRA adapters. Adds regression tests for each; existing parser suite stays green. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: harden multi-format tool-call detection from review findings Apply five targeted fixes from the review pass over the multi-format tool path: - routes: route display strip delegates to _strip_tool_xml so Mistral [TOOL_CALLS] blocks with nested JSON are removed from streamed display text, not just the XML forms. - tool_call_parser: skip function/parameter starts that fall inside an already-open parameter block (_inside_open_parameter) so nested example payloads are not mis-parsed as new calls; extract strip_llama3_leading_sentinels so the bare-JSON guard is shared. - safetensors_agentic: probe bare JSON through strip_llama3_leading_sentinels before the balanced-brace check so a leaked header sentinel does not defeat the guard. - tool_healing: allow dotted tool names in the Gemma wrapped start pattern. - llama_cpp (GGUF): buffer wrapper-less Llama-3.2 {"name":..} calls that carry no XML signal, drain a complete object silently and hold an incomplete one, and run the end-of-stream safety net unconditionally so markerless calls are detected and never leak the raw JSON (including truncated fragments). Adds regression tests for the GGUF bare-JSON streaming path and the Mistral display strip. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: stop bare-JSON tool calls leaking at EOF, oversized, and into history The second review pass flagged that the Llama-3.2 bare-JSON tool-call handling still leaked raw JSON in several spots; ``strip_tool_markup`` only knows XML/bracket markup, so the bare-JSON form survived it. Fix them symmetrically across the safetensors and GGUF loops: - Safetensors stream-end resolver now routes a held bare-JSON fragment to DRAINING (mirroring GGUF) so a truncated ``{"name":..`` cut off by the end of the stream is dropped instead of flushed as assistant content. The 7/10 reviewer finding. - Both loops now drain (suppress) an oversized still-open bare-JSON call once it passes ``_MAX_BARE_JSON_BUFFER`` instead of streaming the raw prefix, gated on a ``"name"`` key so a giant plain JSON answer still streams; a complete oversized call still executes via the safety net. - Add a shared ``strip_leading_bare_json_call`` helper and apply it to the content kept for the assistant turn in both loops, so an executed bare-JSON call is not replayed as visible text or fed back as next-turn history. Plain JSON answers without a ``"name"`` key are untouched throughout. Adds regression tests for the EOF, oversized, and next-turn cases on both backends plus unit tests for the helper. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: bound the Llama-3 python_tag strip on real control sentinels The route display strip's <|python_tag|> arm ran to the next <| of any kind. A tool-call argument carrying a literal <|...|> token (for example <|cite|> inside a string value) truncated the strip early and leaked the call tail into the visible response. Narrow the stop condition to the genuine Llama control sentinels (eot_id, eom_id, python_tag, start/end_header_id, begin_of_text, finetune_right_pad_id) so embedded markup and JSON are consumed while real header/turn boundaries still bound the strip. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: harden GLM/Gemma parsing, cap GGUF textual calls, share native-template fallback GLM 4.x parser walked a body pre-bounded by the first </tool_call>, so a string argument containing a literal </tool_call> (e.g. code that prints it) was truncated. Walk arg_key/arg_value pairs against the full content instead, since each <arg_value> is delimited by its own </arg_value> and the call's real close is the </tool_call> that precedes the next <arg_key>. Add a truncated wrapper-less Gemma pattern (call:NAME{... with no closing brace) to the markup strip so a call cut off mid-arguments does not leak raw into the visible stream. It runs after the closed form, so a complete call keeps trailing prose. Cap and dedup tool calls parsed from the GGUF TEXTUAL fallback at _MAX_TOOL_CALLS_PER_TURN, mirroring the safetensors loop. Structured delta.tool_calls are grammar-bounded by llama-server, but text parsed straight from content is not, so one runaway turn could fan out into dozens of executions. Extract the native-chat-template fallback into chat_template_helpers (render_native_template / render_with_native_template_fallback) so the transformers and MLX text backends share one implementation. The MLX text path now applies it too, so an Unsloth override template that drops the tools schema no longer silently stops MLX from advertising tools. The MLX VLM path renders via the processor for image tokens and is intentionally left on its own render. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: gate markerless bare JSON on enabled tools and close parser/strip asymmetries The Llama-3.2 custom_tools bare-JSON form has no marker, so any JSON object with a name key was read as a tool call. An ordinary JSON answer like {"name":"Alice","parameters":{"age":30}} was misclassified as a call to a disabled tool and dropped from the visible response. Gate the markerless form on the enabled tool names (threaded through parse_tool_calls_from_text and strip_leading_bare_json_call, supplied by both streaming loops): an object whose name is not an enabled tool is ordinary content. The marker-based forms keep their name-agnostic behaviour (an explicit signal is a real call attempt), and unrestricted mode stays ungated. Also fix two parser/strip asymmetries the parser already tolerated: - A literal </function> inside a parameter value (print("</function>")) truncated both the core and route strips at the first close, leaking the tail. Extend the strip to the call's real close (last </function> before the next opener), mirroring the parser, without merging separate calls. - The single-object Mistral [TOOL_CALLS]{...} shape parsed but _strip_mistral_closed_calls left it, leaking the raw object into display. Strip the balanced object while keeping trailing prose, matching the array and name shapes. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio tools: fix strip/parse symmetry and native-template token for DeepSeek/GLM/Kimi Pass-3 review follow-ups on the multi-format tool parser: - Bare Kimi call (<|tool_call_begin|>...<|tool_call_end|> with no section wrapper) is accepted by the parser, so add it to the closed strip patterns so the streaming (non-final) display strip removes it instead of leaking the markup mid-generation. - Route display strip now also runs the wrapper-less Gemma cleanup, so a Gemma 4 call:NAME{..} no longer leaks into the visible answer. - MLX model record carries base_model for a LoRA adapter so the native-template fallback loads the base repo template rather than the adapter's (often template-less) tokenizer. - Native-template reload forwards the load-time HF token so a gated/private model's repo template can still be fetched (transformers and MLX text paths). - GGUF end-of-stream bare-call heuristic is gated on the enabled tool names so a truncated ordinary JSON object ({"name":"Alice","age":) streams as the answer instead of being dropped as a tool call. Adds regression tests for each case. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio tools: gate GGUF bare-JSON suppression on enabled tools and fix python-tag exponent parsing Pass-4 review follow-ups on the GGUF tool loop and Llama-3 parser: - The GGUF bare-JSON suppression sites still keyed off a raw "name" substring, so an ordinary JSON answer whose name is not an enabled tool was dropped when it was truncated, oversized, or reached the no-tool DRAINING fallback (the parser, helper, and safetensors paths were already gated). All three sites now use the shared enabled-name gate, and a held bare-JSON buffer that turns out not to be an enabled call is shown as the answer instead of dropped at stream end. - The Llama-3 python-tag numeric kwarg regex matched only the mantissa, so scientific notation was truncated to its leading digits (1e-3 parsed as 1) and a tool executed with the wrong value. The regex now accepts exponent and decimal forms, and the int/float classification keys off the exponent too. Adds regression tests for the truncated / oversized disabled-name JSON cases (and a counterpart that a truncated enabled call still does not leak) plus the scientific-notation kwargs. * Studio: drop accidentally committed async worker transcripts Eight generated reviewer / async-worker transcripts were committed under studio/backend/async_task_outputs/. They are not imported or referenced by any code and carry only internal task state, so they should never ship in the repo. Remove them and gitignore the directory so they cannot be re-added. * Studio tools: gate safetensors bare-JSON drain, fix nested-name gate and function-XML strip Pass-4 review follow-ups on the shared parser / safetensors loop: - The safetensors oversized and end-of-stream bare-JSON drain branches keyed off a raw "name" substring, so a large or truncated ordinary JSON answer whose name is not an enabled tool was drained instead of streamed. Both now use the shared enabled-tool-name gate, matching the GGUF path. - strip_leading_bare_json_call matched the first "name" anywhere, so a plain JSON answer with a nested name equal to an enabled tool ({"result":{"name":"web_search"}}) was wrongly suppressed. It now extracts the TOP-LEVEL name only, walking past nested objects/arrays and keeping the text when a top-level value is truncated. - The function-XML display strip used a regex negative-lookahead that stopped at a literal <function=...> opener inside a parameter value and then dropped the rest of the answer to EOF. A scan-based strip mirrors the parser (ignores openers inside an open <parameter> via _inside_open_parameter) and closes each call at its real </function>, so trailing assistant text after such a call survives. Adds regression tests for each. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: keep tools prompt when native-template probe raises; make helper tests hermetic Pass-4 review follow-ups on the native-template fallback: - render_with_native_template_fallback re-renders the live template with tools=None to detect whether it dropped the schema. A template that requires tools can raise on that probe; that must not discard the already-valid tools prompt. The probe is now wrapped so any error returns the original formatted_prompt (transformers would otherwise fall back to manual formatting and lose the schema; MLX would let the exception escape). - The native-template helper tests imported InferenceBackend just to reach the thin wrapper, which pulls in unsloth and its optional vllm package metadata. They now call the dependency-light render_native_template helper directly so they pass in a backend/test environment without vllm. Adds a probe-raises regression test. * Tool parsing: 3.9 import safety, disabled-Auto-Heal contract, capability gate Round-2 review follow-ups on the multi-format tool-call parser: - tool_call_parser: add `from __future__ import annotations`. The module is dependency-light by design (external llama-server wrappers import it standalone) and the package targets python >=3.9, where its PEP 604 `int | None` return annotations would raise TypeError on import. - safetensors + GGUF drain fallback: gate the leading bare-JSON strip on auto_heal_tool_calls. With Auto-Heal off, a truncated enabled-name fragment that did not parse now stays visible, matching the XML strip in the same branch and the disabled-Auto-Heal contract. With Auto-Heal on it is still suppressed. - safetensors capability gate: match the bare-JSON `{"name":` template marker with a whitespace/escape-tolerant regex so a pretty-printed `{ "name" :` or JSON-escaped `{\"name\":` template is not mis-classified as tool-less. The parser already accepts that whitespace via raw_decode, so the gate must too. Regression tests added for each case. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * GLM tool-call display strip: treat literal close tag in arg value as data Round-2 review follow-up on the GLM 4.x tool-call format. The GLM call shape is <tool_call>NAME<arg_key>k</arg_key><arg_value>v </arg_value>...</tool_call>. The parser was hardened to walk arg_key / arg_value pairs so a literal </tool_call> inside an argument value (e.g. print("</tool_call>")) is treated as data and the call's real close is the </tool_call> that precedes the next <arg_key>. The display strips still used a non-greedy <tool_call>.*?</tool_call> regex, which stopped at the literal and leaked the call's tail into visible content and stale history. Add _strip_glm_calls, a scan that mirrors the parser's close detection, and run it before the regex arms in every strip pipeline: the core strip_tool_markup, the route _strip_tool_xml display/history cleanup, and the safetensors + GGUF streaming strips. Qwen / Hermes <tool_call>{json} has no NAME token after the opener, so it is left to the regex arms unchanged. Regression tests cover the literal-close-tag leak (core + route), normal GLM calls, back-to-back GLM calls, zero-arg GLM, truncated GLM, and untouched Qwen. * Tool parsing: symmetric "function" bare-JSON alias and route strip parity Round-3 review follow-ups, all parser/strip symmetry fixes. - Bare-JSON "function" alias: the markerless parser accepts a call name via obj.get("name") or obj.get("function"), but the strip/gates only knew "name", so a {"function":<enabled tool>} call executed while its raw JSON leaked. Teach _top_level_bare_json_name the alias (with "name" precedence and the same nested and truncated-name guards), and widen the guards in strip_leading_bare_json_call, the safetensors and GGUF _looks_like_enabled_bare_json gates, and the route capability marker regex. - Route display/history cleanup: strip a tail-only </param> alias close (the parser accepts <param name="...">...</param>), and run the parser's guarded function-XML scan (_inside_open_parameter) before _TOOL_XML_RE so a literal nested <function=...></function> inside an argument value does not truncate the strip and leak the tail. Regression tests added for each. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio tools: fix DeepSeek strict recovery, Kimi dotted names, Gemma spaced streaming Round 3 review fixes for the DeepSeek / GLM / Kimi tool-call parsing path. - DeepSeek R1 and V3/V3.1 strict parsing (Auto-Heal off): when a call is truncated (missing closing fence or <tool_call_end> terminator), skip it and keep scanning for later well-formed calls instead of breaking out and dropping the rest of the envelope. This matches the Kimi strict parser's recovery behaviour. - Kimi dotted tool names: keep the full name after stripping only the functions. prefix and :idx suffix, e.g. functions.mcp.server-list:0 stays mcp.server-list. The previous split on "." truncated dotted MCP names to their last segment. This matches current vLLM (tool_id.split(":")[0].removeprefix("functions.")) and SGLang (^(?:functions\.)?(?P<name>[\w.\-]+):(?P<index>\d+)$). - Gemma wrapper-less call streaming: hold the whitespace-tolerant prefix (call : NAME) in the streaming suppression buffer, matching the parser's _GEMMA_BARE_TC_RE, so the spaced spelling split across chunks is buffered instead of leaking as visible text. Applied to both the safetensors and llama.cpp streaming paths. - Remove dead _render_with_native_template method and the now-unused copy import from inference.py; the live path uses render_with_native_template_fallback. Adds regression tests for DeepSeek R1/V3 strict recovery, Kimi full dotted name preservation, and the Gemma spaced-call streaming suppression. * Studio tools: honor tool budget in GGUF loop and guard function-XML streaming strip Round 4 review fixes. Both are asymmetric-fix bugs where the final/steady path got a guard the analogous streaming/loop path did not. - GGUF tool-call budget: the safetensors loop counts real tool-call turns against max_tool_iterations (re-prompt stalls excepted), but the GGUF loop only bounded the turn count by the enlarged range (max_tool_iterations + _MAX_REPROMPTS). Since this PR raised _MAX_REPROMPTS from 1 to 3, a model that keeps making valid tool calls could run up to three extra tool rounds (with max_tool_iterations=1, four rounds instead of one). Add a _tool_iters_done counter that increments only when a tool actually executed in the turn, and stop once the caller's budget is spent so the post-loop final-answer nudge fires. A duplicate/disabled no-op turn is a correction turn (like a plan-without-action re-prompt) and does not consume budget, preserving the existing "already completed" re-prompt behavior. - Streaming display strip: the final strip runs the guarded _strip_function_xml_calls scanner (a literal <function=...> inside a parameter value is data, not a nested call), but the GGUF and safetensors streaming strips still used only the open-ended regex arms. When a tool-call argument contained literal function markup, the regex tail ate everything to end-of-text and dropped the real trailing prose after the call's true </function>. Run the guarded scanner (and the balanced Mistral strip) before the regex arms in both streaming paths so streaming and final display agree. Adds regression tests: GGUF valid tool calls respect max_tool_iterations, and the streaming strip keeps trailing prose after a function-XML call with a literal marker. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio tools: safetensors tool budget counts only executed turns (GGUF parity) Follow-up to the GGUF budget fix. The safetensors loop charged max_tool_iterations per non-re-prompt iteration (iteration + 1 - reprompt_count), so a duplicate/disabled no-op turn spent a budget slot even though no tool ran. With a small cap this dropped real work: for max_tool_iterations=2, a model that made a valid call, repeated it (an internal no-op correction turn), then made a distinct valid call executed only the first -- the third turn was se…
TRL 1.7.0 enables the MoE router load-balancing aux loss by default (router_aux_loss_coef = 0.001). Unsloth's optimized GRPO forward does not compute it, so default the coefficient to 0, matching pre-1.7.0 behaviour. Users can still opt in with router_aux_loss_coef > 0. No-op on TRL < 1.7.0.
…slothai#6908) * Add DeepSeek-V4-Flash-GGUF to Studio with none/high/max reasoning Adds unsloth/DeepSeek-V4-Flash-GGUF as a default selectable model with the recommended decoding defaults (temperature 1.0, top_p 1.0 from the official generation_config.json) and its three tier reasoning control. The high/max ladder is surfaced for deepseek-v4 model ids and flows through the existing enable_thinking_effort reasoning style via chat_template_kwargs, so no frontend changes are needed. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio DeepSeek-V4: segment-scope high, enable thinking for lone effort, render tests Match deepseek-v4 on whole repo-name segments so a future deepseek-v40 or deepseek40 cannot false-match the synthetic 'high'. In _request_reasoning_kwargs, emit enable_thinking when a named effort level is sent without it, so the newly exposed High mode renders thinking-on over the API (the UI already sent it explicitly). Add a none/high/max render-path test file (jinja behind importorskip) with a lone-high regression. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: danielhanchen <michaelhan2050@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* Studio: add assistant response details panel * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Hide model badge by default, show on hover/focus Wrap MessageResponseModelBadge in a span with hidden/group-hover visibility classes to reduce visual clutter. The badge now only displays when hovering or focusing on the assistant message, improving the UI presentation. Updated corresponding tests to verify the new CSS classes. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
for more information, see https://pre-commit.ci
…floating-owner scale dtype for PR unslothai#6749
…nslothai#6940) * Studio: account for DeepSeek-V4 compute buffer in context auto-fit DeepSeek-V4-Flash's lightning indexer plus compressed sparse attention reserve a large context-scaling compute buffer that _compute_buffer_ctx_bytes did not model (the KQ-mask and dequant-scratch rates both miss it, even with an f16 cache). Measured on UD-Q4_K_XL at ub 512 it is about 65.5 GiB at 1M context, which the mask estimate puts near 1.5 GiB, so the auto-fit kept the full 1M train context and llama-server OOM'd allocating the ~70 GB buffer, then spilled to CPU (~4 tok/s). Add a deepseek4-gated flat plus per-token term so the fit caps the context (about 256k on a B200) and the model stays fully on GPU. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: danielhanchen <unslothai@gmail.com>
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
* show chat by by last activity * Update chat thread updated_at logic and enhance sidebar chat item handling * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Lee Jackson <130007945+Imagineer99@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…esponse pattern (unslothai#6926) * fix: match qwen3-thinking chat template double-newline in response pattern The Qwen3-thinking chat template generates `<think>\n\n` (double newline) after the think tag, but `train_on_responses_only` was looking for `<think>\n` (single newline). `\n\n` is token 271 while `\n` is token 198 -- different tokens, so the pattern match in `train_on_responses_only` fails, masking ALL tokens and dropping 100% of training samples. Update the response pattern from `<think>\n` to `<think>\n\n` to match what the actual qwen3-thinking template generates. Fixes unslothai#6919 * fix qwen3 thinking response marker --------- Co-authored-by: Ayushman Paul <ayushman@HP> Co-authored-by: Etherll <61019402+Etherll@users.noreply.github.com>
…pSeek thinking not streaming with a pill on) (unslothai#6947)
* Speed up Studio startup path * Studio: recheck managed binary executability on preflight cache hit and ignore stale unauthenticated platform fetches Preflight: a matching capability cache fingerprint no longer skips the runnability check when the managed binary's executable bit was cleared (size and mtime unchanged, since chmod bumps ctime not mtime). The cache fast path now confirms the binary is still executable, otherwise it falls back to the CLI help probe so preflight reports Stale and can repair, instead of returning Ready and failing later at backend start. Adds a regression test. Frontend: now that first render is no longer gated on fetchDeviceType, the initial unauthenticated health call can resolve after an authenticated platform fetch. Guard the store so a late unauthenticated or failed non-forced response cannot overwrite an already authoritative device type, tunnel URL, or secure flag. Forced refreshes and the first unauthenticated load are unaffected. * Studio: use access(X_OK) for the preflight cache executability guard A mode bitmask treats any execute bit as launchable, but the executable bits can be set only for another owner or group, or be denied by an ACL, so the current user could still hit PermissionDenied at launch and the cached fast path would wrongly return Ready. access(X_OK) checks real executability for the calling user, so an ownership or permission change correctly falls back to the CLI help probe and the Stale repair path. * Studio: ignore any stale non-forced platform fetch once authoritative Extend the platform store guard so a non-forced health response never overwrites an already authoritative result, not only unauthenticated ones. With a saved token the post-render non-forced request can be authenticated but older than a later forced refresh that already picked up the tunnel URL and secure flag; if that earlier request resolves last it would null those fields. Now any non-forced response is dropped once the store holds a server-reported platform. Forced refreshes and the first authoritative write are unaffected. * Studio: run the managed CLI help probe before trusting the preflight cache Restore running the managed CLI help probe before returning Ready from the desktop capability cache, so a managed install whose venv interpreter or a runtime dependency is broken (while path, size, mtime, and markers are unchanged) is reported Stale for repair rather than proceeding to a backend start that cannot spawn. The capability cache still skips the heavier desktop-capabilities probe on a hit, so a warm cache runs one probe instead of two. Removes the executable-access shortcut, which the help probe now subsumes. --------- Co-authored-by: Daniel Han <danielhanchen@gmail.com>
for more information, see https://pre-commit.ci
…lback, force safetensors for FP8 siblings in PR unslothai#6749
… for PEFT FP8 base remaps in PR unslothai#6749
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several critical improvements, including enhanced security for RAG embedding models, a fix for ChatML turn-end EOS resolution, and robustness improvements for the inference subprocess lifecycle and tool-call parsing. I have reviewed the provided comments and identified several actionable improvements regarding resource management and error handling in the new modules.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+84
to
+89
| try: | ||
| tid = id_tokenizer.convert_tokens_to_ids(marker) | ||
| except Exception: | ||
| tid = None | ||
| if tid is not None and tid != unk and int(tid) >= 0: | ||
| ids.add(int(tid)) |
There was a problem hiding this comment.
The conversion of tid to int is performed outside the try-except block. If convert_tokens_to_ids returns a value that cannot be converted to an integer (or if int(tid) raises a ValueError/TypeError), it will crash the loading process. Since this function is intended to never block a model load on EOS resolution, the entire check and conversion should be wrapped inside the try block.
Suggested change
| try: | |
| tid = id_tokenizer.convert_tokens_to_ids(marker) | |
| except Exception: | |
| tid = None | |
| if tid is not None and tid != unk and int(tid) >= 0: | |
| ids.add(int(tid)) | |
| try: | |
| tid = id_tokenizer.convert_tokens_to_ids(marker) | |
| if tid is not None and tid != unk and int(tid) >= 0: | |
| ids.add(int(tid)) | |
| except Exception: | |
| pass |
| native_tpl = model_info.get("native_chat_template") | ||
| if native_tpl is None: | ||
| # A LoRA adapter's native template lives on the base model, not the adapter id. | ||
| template_source = model_info.get("base_model") or active_model_name |
There was a problem hiding this comment.
If both base_model and active_model_name are missing or empty, template_source will be falsy, causing AutoTokenizer.from_pretrained to fail. Adding a guard to return None early prevents unnecessary exceptions.
Suggested change
| template_source = model_info.get("base_model") or active_model_name | |
| template_source = model_info.get("base_model") or active_model_name | |
| if not template_source: | |
| return None |
| derived from the chat_template (see chat_eos.resolve_chat_turn_end_eos_ids), | ||
| so base/coder models and harmony templates are left untouched. | ||
| """ | ||
| info = self.models.get(model_name) or {} |
There was a problem hiding this comment.
If model_name is not present in self.models, self.models.get(model_name) or {} returns a new empty dictionary. Modifying this temporary dictionary later in the method will not update self.models. It is safer to check if info is None or empty and return early.
Suggested change
| info = self.models.get(model_name) or {} | |
| info = self.models.get(model_name) | |
| if not info: | |
| return |
| local = hf_hub_download(name, "modules.json", token = token or None) | ||
| except EntryNotFoundError: | ||
| return () | ||
| data = json.loads(open(local).read()) |
There was a problem hiding this comment.
Opening a file using open(local).read() without a with statement or explicitly closing it can cause a resource leak (unclosed file descriptor). It is highly recommended to use a with statement to ensure the file is closed properly.
Suggested change
| data = json.loads(open(local).read()) | |
| with open(local, "r", encoding="utf-8") as f: | |
| data = json.load(f) |
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
10 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.
Disposable CI run for unslothai#6749. Do not merge; closed after CI.