[staging CI] unslothai/unsloth#7273 - #411
Closed
danielhanchen wants to merge 237 commits into
Closed
danielhanchen wants to merge 237 commits into
danielhanchen wants to merge 237 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…
…unslothai#6917) The pip scan-packages gate (SCAN_ENFORCE=1) blocks on non-baselined CRITICAL/HIGH findings. Recent upstream releases of transitive dependencies added new files/loops that trip the pattern scanner, so all three shards (extras, hf-stack, studio) red-failed on legitimate library code. Add the 7 reviewed findings to scripts/scan_packages_baseline.json. Each entry is genuine upstream code from the official PyPI archive: - huggingface-hub huggingface_hub/_sandbox.py (staged dropper + C2 loop): the HF Jobs sandbox bootstrap string and its host-pool reservation loop. New in huggingface_hub 1.x (pulled via huggingface_hub>=0.34.0). - huggingface-hub huggingface_hub/hf_api.py, utils/_http.py (C2 loop): standard polling / retry while True loops. - fastapi fastapi/routing.py (C2 loop): websocket receive loop. - fastmcp-slim fastmcp/cli/apps_dev.py (fs enum + network): the FastMCP dev CLI (PrefectHQ) making httpx/socket calls. - cffi cffi/_cffi_gen_src.py (compile + exec): cffi generating and running C extension source, its core purpose. Additive only: no existing baseline entry is changed or removed. Verified by re-running the scanner over the full closure on Python 3.12.13 (the CI interpreter); it now exits 0 with only MEDIUM findings remaining.
…slothai#5704) * Studio: parse Mistral [TOOL_CALLS] and rehearsal tool-call shapes Extends the rescue parsers in core/tool_healing.py and core/inference/tool_call_parser.py to recognise two extra serialisations local models commonly emit when bypassing native function calling: * [TOOL_CALLS]name{json_args} (Devstral-Small-2, Mistral-Small-3.x). * name[ARGS]{json_args} (reasoning-model rehearsal). Both extractors use a brace-balance scan that honours escapes and quoted strings so nested JSON args stay intact. Also pre-strips <think>...</think> and [THINK]...[/THINK] blocks before matching so calls emitted after a reasoning preamble are recognised regardless of position. Streaming gates (TOOL_XML_SIGNALS, llama_cpp.py _TOOL_XML_SIGNALS) and the SSE strip regex (routes/inference.py _TOOL_XML_RE) gain the new sentinels so the parser is actually invoked and the raw markup never leaks to the UI. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Strip unclosed think blocks and catch rehearsal [ARGS] mid-buffer The pre-existing ``_THINK_TAG_RE`` only matched closed thinking blocks (``<think>...</think>`` or ``[THINK]...[/THINK]``). During streaming the model is still inside the open block when the parser runs, so any tool-shaped markup the model is REHEARSING inside that block survived the strip and could be executed as a real call. Switch both copies of the regex (parser + healing) to accept the trailing block being terminated by end-of-string in addition to the explicit closer. The ``_TOOL_XML_SIGNALS`` list on the llama_cpp streaming buffer included ``[ARGS]`` to catch rehearsal syntax, but the gate used a ``startswith`` check against the buffer head -- rehearsal is shaped ``name[ARGS]{json}``, so the buffer never STARTS with ``[ARGS]`` and the signal had no effect. Add a substring fallback for the bracket-style signals so the BUFFERING window can still divert the stream into DRAINING when rehearsal markup arrives mid-buffer. Adds three regression tests covering rehearsal inside unclosed ``<think>`` / ``[THINK]`` blocks (must yield no calls) and the positive case after a closed think block (still parsed). * [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: harden bracket-tag tool-call parsing and streaming strip Address review findings on the Mistral [TOOL_CALLS] / rehearsal [ARGS] paths: - Accept hyphenated tool names in the bracket parsers and strip patterns. _MISTRAL_BRACKET_RE and _REHEARSAL_RE used \w+, which dropped or truncated MCP function names containing dashes (mcp__srv__list-issues). Use [\w-]+ to match the XML and Gemma parsers. - Strip a partial bracket marker streamed before its opening brace. The trailing-unclosed patterns required the {, so a [TOOL_CALLS]web_search or python[ARGS] split across deltas leaked the raw marker to the UI. Match the bare marker to end-of-text, mirroring how the bare open tags are stripped. Closed pairs are unchanged so in-progress markup stays buffered until parsed. - Strip a truncated bracket tail in the route-level display regex. _TOOL_XML_RE required a balanced JSON object; a tool call truncated by EOS now strips up to \Z, like the orphan-opening XML shapes. Complete calls still strip only their balanced JSON so following prose survives. Add regression tests for hyphenated names, the streaming partial-marker strip, and the unclosed-tail route strip. * Studio: preserve XML parameter indentation in tool_healing The chat template emits <parameter=k>\nVALUE\n</parameter>; the parameter-start regex consumed the wrapping newline AND the value's first-line indentation via a trailing \s*, then str.strip() removed the rest, corrupting code/diff arguments. Narrow the trailing class to horizontal whitespace and trim exactly one wrapping newline (_trim_param_value), preserving indentation. Matches SGLang's qwen3_coder detector and the same fix on the multi-format parser. Add a regression test. * Studio: tighten Mistral/rehearsal tool-call comments Compress the comments in the Mistral [TOOL_CALLS] / rehearsal [ARGS] healing shim and its callers to one or two lines, keeping the bracket-tag stripping rationale, the thinking-block handling note, and the forge attribution intact. Comment-only: no code or behavior change (verified with comment_tools.py check --strip-docstrings; tests green). * Studio: fix think-strip arg corruption and nested bracket-JSON strip Review follow-up for the Mistral/rehearsal healing shim: - The <think>/[THINK] strip ran unconditionally over the whole content before parsing, so a real tool argument that legitimately contained a <think> / [THINK] literal was silently corrupted. Don't delete the blocks: compute the reasoning-block spans and skip any tool-call candidate that STARTS inside one, across all parse paths (JSON, Gemma, XML, bracket, rehearsal). A rehearsed call inside reasoning is still ignored; a real call after </think> still parses. - The bracket-tag display strip used a fixed one-level-nesting regex, so a call with two-level-nested JSON args either leaked raw markup or, in final mode, let the catch-all eat the trailing prose. Add a balanced-brace _strip_bracket_tag_calls pass (any nesting depth) used by strip_tool_call_markup and the route display strip. Add regressions: <think>/[THINK] literal inside a real argument, rehearsal-inside- think with a real call after, and two-level-nested bracket/rehearsal strip keeping trailing prose. * Studio: correct think-block comments to match span-skip behavior The think-strip fix replaced the unconditional think-block strip with a span-skip (the block is kept and any tool-call candidate starting inside it is ignored), but two comments still described the old strip-first behavior. Update the _THINK_TAG_RE comment and the parse_tool_calls_from_text docstring. * Studio: parse Mistral arrays and call-ids, unify bracket parse/strip, keep it linear - Parse the canonical Mistral array form (TOOL_CALLS followed by a JSON list of calls) and emit every call; parse the v11 shape that carries an opaque CALL_ID token between the name and ARGS (the function name is the token after TOOL_CALLS, never the call-id); and parse a Mistral call plus a rehearsal call in one message (the second was dropped yet still stripped from display). - One shared balanced forward scan (_iter_bracket_spans) backs both the parser and the strip path, so they no longer diverge. It is linear: each regex is re-searched only once its cached match falls behind the cursor, replacing the per-match full-tail re-scan that was O(n^2) (O(n^3) over a stream). A length cap before the scan is a backstop. - strip_tool_call_markup preserves think/reasoning blocks verbatim (the parser skips tool markup inside them), stripping only the visible text around them. - _in_think uses bisect over the sorted think spans (was a linear scan per candidate). - GGUF streaming strip runs the balanced bracket pre-pass before the regex patterns so nested-arg calls do not leak or eat trailing prose, and the BUFFERING ARGS detector requires the rehearsal name-ARGS shape. - Tests: canonical array, array string-args, array strip keeps prose, Mistral plus rehearsal multi-call, v11 call-id name, think-rehearsal strip preservation, and bracket-strip linearity. * Studio: preserve reasoning blocks in the route and streaming strip paths too Addresses Gemini/Codex review: making strip_tool_call_markup preserve think blocks left the route display strip and the GGUF streaming strip inconsistent, so a rehearsed call inside a reasoning block was still deleted from the visible text on those paths. - Extract the think-block segmentation into one shared helper (strip_outside_think) and route all three strip paths through it: strip_tool_call_markup, _strip_tool_xml_for_display, and the GGUF _strip_tool_markup_streaming closure. - Add a route-strip regression test that a rehearsal inside a reasoning block is preserved while a real call outside it is still stripped. * Studio: fix bracket-tag strip/buffer review findings Address the live code-review findings on the Mistral bracket-tag / rehearsal tool-call rescue path: - tool_healing: a literal think block inside a tool-call argument is no longer treated as a reasoning block. strip_outside_think now excludes think spans that sit inside a complete tool-call span, so the call is stripped whole instead of the split hiding its open/close pair and leaking the raw call. - tool_healing: the rehearsal trailing-strip pattern requires a following brace or end-of-text, so prose that merely mentions name[ARGS] is not truncated as a phantom call. The bracket strip patterns are aligned with the parser regexes (whitespace, v11 [CALL_ID]/[ARGS] metadata, and the [CALL_ID] lookbehind). - routes: strip a truncated canonical Mistral array ([TOOL_CALLS] [{... with no closing bracket) that the balanced scan cannot remove, align the display regex with the parser regexes, and apply the same rehearsal-prose guard. - safetensors loop: mirror the GGUF [ARGS] rehearsal-substring check during BUFFERING so a rehearsal name does not stream before its [ARGS] arrives. Adds regression tests for each; existing parser suite stays green. * Studio: hold split rehearsal tool-name prefix in both streaming loops A reasoning-model rehearsal call can stream the tool name and its [ARGS] arm in separate chunks (web_search then [ARGS]{...}). The buffering detector only recognised the rehearsal once [ARGS] was present, so the bare tool name was emitted as visible content before the call drained and executed. Add _is_rehearsal_prefix (mirrored in the safetensors loop and the GGUF loop): when a no-signal buffer is a bare active-tool name -- or a partial prefix of NAME[ARGS] -- hold it as a prefix instead of streaming it, so the next chunk's [ARGS] flips it to a drain. A whitespace in the buffer means prose, not a split call, so ordinary text still streams. Adds regression tests for the split rehearsal in both loops and a guard that a plain non-tool word still streams. * Studio: route Anthropic tool-call cleanup through the protected display strip The Anthropic stream, non-stream, and passthrough paths cleaned content with raw _TOOL_XML_RE.sub instead of _strip_tool_xml_for_display, so a rehearsal call inside <think> was deleted from the reasoning and a nested [TOOL_CALLS] call dropped its trailing prose (the OpenAI-compatible paths already use the helper). Route all four sites (prior-assistant cleanup, streaming content events, non-stream aggregation, passthrough conversion) through the protected helper, and add a source-level guard test so raw _TOOL_XML_RE.sub stays confined to the helper itself. * Studio: stop split rehearsal tool names leaking once streaming, uncapped, or unrestricted The split-rehearsal guard (NAME in one chunk, [ARGS]{...} in the next) only held the name in the initial BUFFERING state. Three gaps remained where the bare tool name still streamed as visible content before the call drained: - STREAMING: after prose had already streamed, both loops emitted a trailing active-tool-name token (and the GGUF/safetensors [ARGS] boundary was not pulled back over the name). Hold the trailing rehearsal token and release it on the next chunk, with an end-of-stream flush so a plain answer that merely ends on a tool-name word is never dropped. - Buffer cap: a realistic MCP name longer than the 32-char _MAX_BUFFER_CHARS cap defeated the BUFFERING hold. A rehearsal prefix is self-bounding (it stops matching once it grows past NAME[ARGS]), so the generic cap no longer applies to it. - Unrestricted mode (tools=[]): with no declared tool list, any bare identifier may be a NAME[ARGS] rehearsal, so the prefix check now recognises one instead of leaking the name and mis-parsing the call. Regression tests cover the streaming, long-name, and unrestricted cases plus the plain-prose paths that must not be held or corrupted. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio tools: protect think blocks in safetensors streaming, hold split rehearsal on initial flush, advertise Mistral tools Pass-3 review follow-ups on the Mistral [TOOL_CALLS] / rehearsal [ARGS] work: - Safetensors streaming display strip now preserves think / [THINK] reasoning verbatim (routes through strip_outside_think like the GGUF path). A call rehearsed inside a reasoning block was stripped mid-stream and then restored by the final strip, a non-monotonic shrink/grow that corrupted append-by-length stream consumers and the visible reasoning. - The first flush out of BUFFERING (safetensors and GGUF) now applies the same trailing-name hold the STREAMING branch uses, so a split rehearsal (prose plus a trailing active tool name in one chunk, [ARGS]{...} in the next) no longer leaks the bare name before the call drains. - Safetensors capability gate no longer suppresses tools for Mistral [TOOL_CALLS] templates, which the shared bracket-tag parser now handles end to end. Llama python_tag stays suppressed (still unparseable). - Route display strip applies the open-ended / bare-marker tail arms only on the segment after the last reasoning block (closed-only regex before it), matching strip_tool_call_markup, so a bare foo[ARGS] before a reasoning block is preserved while complete calls are still removed in every segment. Adds regression tests for each and updates the now-stale Mistral capability test. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix tool-call think-marker and bracket-wrapper edge cases Round-1 review follow-ups on the Mistral/rehearsal tool-call healing: - tool_healing: a reasoning marker that opens INSIDE a tool call's arguments is argument data, not a reasoning block. Add _think_spans_outside_tool_markup (start-inside test) and use it in both parse_tool_calls_from_text and strip_outside_think so a literal marker in one call's args no longer hides a later call (parse) or leaks the raw markup (strip) when the greedy match runs past the call's closer. - tool_healing: strip the orphan Mistral v11 [/TOOL_CALLS] closer left behind after the balanced scan removes the call body. Add a route arm for the same closer in _TOOL_XML_RE / _TOOL_XML_CLOSED_RE. - safetensors + llama_cpp streaming strip: run the open-ended (EOS anchored) tail patterns only on the last segment; segments before a reasoning block use the closed-only patterns, matching the final strip and the route strip. A bare foo[ARGS] before a reasoning block is prose, not a truncated call. - safetensors streaming detector: validate each [ARGS] hit before draining. A bare foo[ARGS] in prose (no active tool name in front) no longer drains the rest of the turn; a later real NAME[ARGS] call is still found and the prose in between is preserved. Regression tests added for each case across the parser, strip helpers, and both streaming loops. * Strip incomplete-XML tool markup with literal think tags; widen render-html detector Round-2 review follow-ups. - tool_healing: an UNCLOSED <tool_call> / <function= call that the parser still executes via allow_incomplete leaked its markup when an argument contained a literal think marker. _tool_call_markup_spans only covered closed calls, so the literal was treated as a reasoning block to preserve. Extend it to the open-ended XML tail forms (shared as _TOOL_OPEN_XML_TAIL_PATS) so a think marker inside an unclosed call is argument data and the call's markup is stripped. A complete call's opener stays bounded to its closed span, and a real reasoning block with no tool call is still preserved. - safetensors render-html provisional card: _detect_render_html_tool_start was XML-only, so a Mistral [TOOL_CALLS]render_html or rehearsal render_html[ARGS] call executed but skipped the early card. Detect the earliest tool-call marker across every serialization the loop executes and fire when it is render_html. Regression tests added for both. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio tools: gate [ARGS] on active tools and skip think-block render_html rehearsal Round 3 review fixes for the Mistral / rehearsal tool-call parsing path. Both are asymmetric-fix bugs where one code path applied a guard the analogous paths did not. - [ARGS] active-tool gating: the streaming state already validates a rehearsal NAME[ARGS] against the active tool list before draining, but the BUFFERING detection and the end-of-stream safety-net checks (safetensors and GGUF) treated any word[ARGS] substring as a tool boundary. An answer containing a literal foo[ARGS]{...} in prose, where foo is not an enabled tool, was drained, parsed into a disabled foo no-op, and forced an extra generation turn. Gate those checks on the active tool name too (unrestricted mode still accepts any name), so inactive-name prose is neither drained nor parsed. Adds a shared _has_genuine_tool_signal helper (safetensors) and _gguf_rehearsal_signal_pos / _gguf_has_genuine_tool_signal (GGUF). - render_html provisional card vs think blocks: the parser skips tool candidates that start inside a <think>/[THINK] reasoning block, but the provisional render_html detector scanned raw content. A render_html rehearsed inside <think> followed by a real non-render_html call emitted a provisional render_html tool_start (reusing the later call's id) that the loop never executed. Drop candidates that start inside a think span and use the first marker of each shape outside the blocks. Also resolve the [TOOL_CALLS] [{...}] array shape through the parser so a nested "name" argument key no longer fires a false provisional card ahead of the real top-level tool name. Adds regression tests for both loops: inactive-name foo[ARGS]{...} is not drained into a disabled no-op or a retry turn, a think-block render_html rehearsal emits no provisional card, and the array top-level name is read correctly. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Gate ambiguous bare-rehearsal parse and strip on the active tool list A bare NAME[ARGS]{json} is a genuine rehearsal call only when NAME is an active tool; otherwise it is prose. The earlier round gated only detection (so an inactive foo[ARGS] no longer drained the buffer or forced a retry turn), but the parse and strip stayed unrestricted, which produced two regressions: 1. An inactive foo[ARGS]{...} placed immediately before a real web_search[ARGS]{...} in the same content span made the real call fail to execute (parse consumed the phantom foo call). 2. An inactive foo[ARGS]{...} in a prose answer had its markup stripped from the visible text, corrupting the sentence to " is just syntax." Thread enabled_tool_names through the shared parser/strip so parse and strip apply the SAME active-tool gate as detection: - core/tool_healing.py: _iter_bracket_spans skips an inactive rehearsal span; parse_tool_calls_from_text, _strip_bracket_tag_calls, _strip_markup_segment and strip_tool_call_markup accept and thread the gate; apply_tool_strip_patterns keeps an inactive rehearsal match. - core/inference/tool_call_parser.py: wrappers forward the gate. - core/inference/safetensors_agentic.py and core/inference/llama_cpp.py: compute the gate from the active tool list (None when unrestricted, to keep the legacy strip-all behavior) and thread it into every parse and streaming/final strip site. - routes/inference.py: _strip_tool_xml_for_display accepts the gate and keeps an inactive rehearsal via a capture group on its rehearsal arm, so the display cleanup does not re-strip the already-correct loop output. The [TOOL_CALLS] control-token arms still strip unconditionally. Wire the current turn's active tool names into the GGUF and safetensors content-display sites. Tests: parse and strip gate coverage in test_tool_call_parser_strict.py, test_tool_xml_strip.py and test_safetensors_tool_loop.py; end-to-end GGUF coverage for the real-call-after-inactive-rehearsal case and a strengthened assertion that the inactive rehearsal prose survives intact. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * 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: skip tool calls rehearsed in prefilled reasoning Reasoning models (Qwen3.5 enable_thinking) open <think> in the prompt, so the generated text starts inside the thought and emits only a closing </think> with no opener. _think_spans_outside_tool_markup only found spans with an explicit opener, so a NAME[ARGS]{...} or [TOOL_CALLS] call rehearsed in that leading thought was parsed and executed as a real call. Add a leading think span (offset 0 through the first close marker) when the content opens with a bare close, so the rehearsed call is skipped and the reasoning is preserved by strip_outside_think. Guarded by the existing call-span check: a literal </think> inside a real call's arguments does not trigger the span, so a genuine leading call still fires. Tests for both cases. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: do not start prefilled reasoning mode when reasoning_effort is none enable_thinking_effort models (e.g. GLM-5.2) express thinking-off via reasoning_effort="none" rather than enable_thinking=False, but _sf_reasoning_prefill_mode only looked at enable_thinking, so such a request started the extractor in prefilled mode. With thinking off the model never emits </think>, so the whole answer was captured as reasoning_content and the visible content/stream came back empty. Thread reasoning_effort through and return False when it is "none". Tests for none vs a real effort level. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: only treat a leading bare </think> as prefilled reasoning when a real call follows The prefilled-reasoning virtual span fired on any unmatched leading close marker, so a non-prefilled turn that emits a real call before a stray </think> (for example "Now web_search[ARGS]{...}</think> answer") had the call swallowed by the span and dropped. Require that a real tool call also appear after the close (the actual turn that follows the thought) before adding the span, so a stray close in a normal answer no longer suppresses a genuine leading call. The rehearse-then- call case still skips the rehearsal. Test for the stray-close case. * Studio: trim redundant comments (comment-only, AST-verified) * studio: keep tool_healing importable on Python 3.9 _balanced_json_span was annotated -> int | None. With no from __future__ import annotations, that PEP 604 union is evaluated at import time, so on Python 3.9 (which the package still supports, requires-python >=3.9, and where external inference servers import this module standalone) the def raises TypeError and the whole module fails to import before any parsing runs. Add from __future__ import annotations so annotations stay lazy strings, matching the prevailing convention across studio/backend. No behavior change: the module has no runtime annotation introspection. * Studio: gate the Anthropic tool-stream display strip on declared tools The Anthropic streaming and non-streaming tool paths called _strip_tool_xml_for_display without enabled_tool_names, so with the default strip-all behavior a final answer that literally contains an inactive-name NAME[ARGS]{json} (prose, not a call) lost those bytes in the delivered text. The GGUF and safetensors paths already pass _display_tool_name_gate(tools); these two sites were missed when that gate was threaded through. Compute the gate from the declared tools and pass it at both sites (threading openai_tools into _anthropic_tool_non_streaming and its caller), so an inactive-name rehearsal survives while an active-name one is still stripped. Add a regression test. * Studio: hold a split unrestricted rehearsal prefix at the bracket In unrestricted tool mode (tools=[]) the rehearsal-prefix regex required [A after the bracket, so a chunk boundary landing right after NAME[ (e.g. web_search[ then ARGS]{...}) failed the prefix check and streamed the partial tool markup web_search[ to the client before the call drained. Restricted mode already holds this via a startswith check. Make the bracket and each ARGS letter individually optional so NAME[ is held too, matching the documented intent. Add a regression test. * Studio: gate rehearsal detection and history strip on the original tool set Two display/loop gate fixes so a spent one-shot tool is handled consistently: - Rehearsal DETECTION (safetensors and GGUF loops) now uses the ORIGINAL tool list, matching the strip gate, instead of the post-removal active_tools. After a one-shot tool (render_html) runs it is dropped from active_tools; a repeat render_html[ARGS]{...} while another tool is still active was stripped from display yet never detected, so it was not routed to the render_html_repeat no-op and the turn ended as a blank continuation. Detection now fires for it. - The GGUF assistant-history sanitiser forwards the enabled-tool-name gate (like the live-response strip), so a prior turn documenting an inactive foo[ARGS]{...} shape is preserved in the replayed prompt context instead of being deleted. Add regression tests for both loops and the history strip. * Studio: thread the tool-name gate through the remaining rehearsal/history sites Follow-up to the rehearsal-detection and history-strip gate fixes, covering the sibling sites that were missed: - GGUF loop: the rehearsal-prefix and trailing-name hold checks now use the original tool list (_detect_tools) like the detection path, so a spent one-shot's split repeat (bare render_html then [ARGS]{...}) is held instead of flushed as visible text. - The safetensors and Anthropic assistant-history sanitisers and the Anthropic non-streaming passthrough now forward the enabled-tool-name gate to _strip_tool_xml_for_display, matching the GGUF history sanitiser and the live strips, so a prior turn documenting an inactive foo[ARGS]{...} example is preserved in the replayed prompt / final text instead of deleted. Add regression tests. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tile bracket-call spans per array item and include the v11 closer Two with_spans fixes for the Mistral bracket parser, both hit through the client-tool passthrough healers: - A multi-call [TOOL_CALLS] array carried its whole markup span on the first call and zero-width spans after, so a consumer that filters promotions by the declared tool set either re-emitted the full raw array as text next to the promoted call or silently dropped a filtered call's bytes. The region is now tiled across the call-producing items (each call's span covers its own JSON object plus the separator bytes before it; the last span runs to the region end), so promoted markup strips exactly once and a skipped call's bytes stay visible. - The v11 wrapper closer [/TOOL_CALLS] sat outside the reported span and leaked as stray text after promotion; the region now extends over an immediately-following closer. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Address review: decouple healer signals from the loop signal set The passthrough healer buffered on every TOOL_XML_SIGNALS entry, so the bare [ARGS] rehearsal marker this branch adds for the loops (where it is gated on active tool names) put legitimate prose like 'Use foo[ARGS] in templates' into the holding state and stalled the stream until finalization. The healer can never promote a bare rehearsal call, so it now buffers only on formats its parser promotes: <tool_call>, <|tool_call>, <function=, [TOOL_CALLS]. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Condense comments in the Mistral tool-call rescue to contract essentials * 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. * Drain the whole Mistral [TOOL_CALLS] array in streaming passthrough healing StreamToolCallHealer._drain promoted only the first parsed call per pass and dropped the rest of the buffer past that one span. For a well-formed Mistral parallel-tool-call array streamed through client-tool passthrough ([TOOL_CALLS][{...},{...}]), the per-item spans are contiguous, so after the first call was promoted the residue began with ,{...}] (no leading signal) and was flushed as raw text: every call after the first was lost. _drain now walks the contiguous run of parsed calls (adjacent tiled spans = one array), promoting each declared call and relaying undeclared ones as data, and stops at the first gap (prose) or incomplete trailing block so separate blocks still stream incrementally in document order. This mirrors the non-streaming heal_openai_message / finalize promote-or-flush loop and the server-side safetensors loop, which already handled multi-call arrays. Added regression tests: 2-call array in one feed and char-by-char, an undeclared middle call kept as text, and an array followed by trailing prose. * Drain comma-less Mistral tool-call arrays and normalize null arguments The array branch fed the whole body to a single json.loads, which rejects the comma-less multi-call form the repo's own Mistral/Ollama templates render (the range loop in ollama_template_mappers.py emits the objects with no separator) and so dropped every call. Decode elements individually with the existing comma-tolerant raw_decode helper, now _decode_array_items, which also returns the objects, so all calls are recovered while the span tiling is unchanged. Also normalize a non-object array argument such as arguments null to an empty object, matching the wrapped tool_call path, instead of serializing None to the string "null" that auto-heal would turn into a bogus query of "null". * Gate safetensors reasoning prefill on the rendered generation prompt reasoning_always_on fires on any paired <think></think> in the template, including markup that only renders PAST assistant history (Kimi-K2-Thinking) while the generation prompt opens no <think>. Starting the reasoning extractor in prefilled mode there captured a normal answer entirely as reasoning_content and returned blank visible content. Prefill only when rendering the generation prompt actually leaves <think> open (DeepSeek-R1 / QwQ / Qwen3-Thinking); history-only templates start the extractor in normal mode and parse the model's own <think>...</think>. Adds a Kimi-shape regression test. * Keep bare scalar Mistral array arguments raw instead of double-encoding A scalar string argument in the canonical Mistral [TOOL_CALLS] array (for example [TOOL_CALLS][{"name":"web_search","arguments":"weather"}]) was run through json.dumps, turning weather into the JSON string "weather". The downstream argument healer then wrapped that quoted form, so a single-string tool like web_search searched for the literal "weather" with quotes. The <tool_call> path already keeps a scalar argument raw; mirror it here so only a dict is serialized. Add a regression test asserting both paths yield the same healed arguments. * Tighten tool-call rescue and reasoning-prefill comments * [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 --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…pple Silicon) (unslothai#6803) * Studio: exclude mlx-lm 0.31.3 (broke gemma4/qwen3_5 QK-norm load) mlx-lm 0.31.3 regressed the QK-norm archs: its strict load_weights rejects the q_norm/k_norm tensors with "Received N parameters not in model", so gemma4 and qwen3_5 checkpoints fail to load. Studio installs the MLX stack unpinned at latest, which pulls 0.31.3. Verified on a real macos-14 runner: gemma4 fails to load on 0.31.3 but loads and generates coherently on 0.31.2 and on git-main (future 0.31.4). See mlx-lm #1242. Exclude just that release (!=0.31.3) in the installer and the self-heal floor so --upgrade still resolves to the newest good build, and treat an already-installed 0.31.3 as unsatisfied so the self-heal replaces it. * Studio MLX: cover fresh-install path + robust bad-version compare Address PR review: - Fresh install.sh (Apple Silicon) runs the base 'uv pip install unsloth' with SKIP_STUDIO_BASE=1, skipping the guarded MLX-stack step, so transitive resolution could still pull mlx-lm 0.31.3. install.sh already exports UV_OVERRIDE -> overrides-darwin-arm64.txt before that install, so exclude mlx-lm 0.31.3 there too; this also strengthens the self-heal (same override). - Match the known-bad version with parsed packaging.Version so 0.31.3 == 0.31.3.0 (trailing-zero normalization) instead of raw string equality. * Studio: exclude mlx-lm 0.31.3 on the fresh Apple Silicon install too The overrides file only applies via UV_OVERRIDE when it exists relative to the script, which is not true for a curl-piped install, and the guarded MLX step in install_python_stack.py is skipped there (SKIP_STUDIO_BASE=1). So the base install could still resolve the transitive mlx-lm to the broken 0.31.3. Append mlx-lm!=0.31.3 to the base install on Apple Silicon (empty elsewhere), so the fresh path pins away from 0.31.3 without waiting for the runtime self-heal. * Studio: exclude mlx-lm 0.31.3 on the migrated install; keep the >=0.22.0 floor The with-deps migrated install did not append ${_MLX_LM_EXCLUDE_ARG:-}, so a curl-piped Apple Silicon migration (no repo overrides file, UV_OVERRIDE unset) could resolve mlx-lm 0.31.3 transitively. Append the exclusion there, matching the fresh install path. The no-torch migration is left alone since --no-deps never resolves mlx-lm (same as the fresh no-torch path). Also restore the >=0.22.0 floor in overrides-darwin-arm64.txt: a uv override replaces the transitive constraint, so a bare !=0.31.3 could let the resolver drop below the supported minimum that mlx_repair.py enforces at runtime. * Triage huggingface_hub 1.22.0 / fastapi / multiprocess scanner false positives The scan-packages gate red-failed on all three shards after transitive deps bumped. Every new CRITICAL is a benign false positive, verified against upstream: - huggingface_hub 1.22.0 added _sandbox.py for the remote HF sandbox feature. Its job-startup bootstrap string (fetch sbx-server into the container /tmp and exec it) and the SandboxPool host-reservation loop trip the staged-dropper and C2-loop heuristics; that script runs inside a remote HF container, not on the user machine. The bump also re-hashed the already-reviewed benign polling loops in hf_api.py and utils/_http.py. The PyPI artifact is byte-identical to the official v1.22.0 tag. - fastapi 0.139.0 routing.py re-hashed the websocket keepalive while-True loop; byte-identical to upstream 0.139.0. - multiprocess 0.70.19 forkserver.py and tests/__init__.py re-hashed the AF_UNIX fork-server IPC and fd-inheritance tests; genuine uqfoundation release, local IPC not network. Added 7 reviewed allowlist entries (no blind regenerate). All three shards (hf-stack, studio, extras) exit 0 locally. * Tighten mlx-lm 0.31.3 exclusion comments * Trim mlx-lm 0.31.3 exclusion comments
…othai#6883) * Studio chat: tool-call nudging on by default (API stays opt-in) Healing is already default-on everywhere and the nudge retry from the client-tool passthrough is opt-in on the API. Studio chat had neither signal: the frontend never sent nudge_tool_calls, and the safetensors and MLX server-side loop lacked the GGUF loop's plan-without-action re-prompt entirely. Backend: the re-prompt helpers move from llama_cpp.py into tool_call_parser.py (shared, cycle-free; the GGUF loop imports them under its old names with zero behavior change) and run_safetensors_tool_loop now re-prompts once at the streaming no-tool-call exit, gated on Auto-Heal, active tools, nothing executed yet, and short forward-looking text. Re-prompts do not consume tool iterations. Frontend: the chat adapter sends nudge_tool_calls from a new nudgeToolCalls runtime setting (default true) with the same persistence, hydration, and settings toggle plumbing as Auto-Heal. Request-model defaults are untouched, so raw API callers stay opt-in. * Address review: persist the nudge setting, consume the flag in the loops, skip the re-prompt after RAG autoinject ChatSettingsPayload uses extra forbid, so a settings patch containing nudgeToolCalls failed to persist any settings; the field is now typed and round-trips. nudge_tool_calls now plumbs into both server-side tool loops and gates the plan-without-action re-prompt with None meaning on, so API callers keep today's behavior, explicit false disables it, and Studio's default-on flag actually controls the path Studio chat runs. The safetensors loop no longer re-prompts after RAG autoinject: the injected retrieval bypasses the tool controller, so the nothing-executed gate saw an empty history and re-asked after a successful retrieval. * Safetensors loop: the plan-without-action retry requires an explicit nudge flag The retry is new on this loop, so an omitted nudge_tool_calls must not change existing API behavior; Studio opts in explicitly. The GGUF loop keeps None as on because its re-prompt predates the flag. * Suppress the plan-without-action re-prompt after a denied tool confirmation A denial appends TOOL_REJECTED_MESSAGE but records nothing in the tool controller history, so the nothing-executed gate re-prompted the model to call the tool the user had just rejected, producing another confirmation prompt. A denial now suppresses the re-prompt for the rest of the request, mirroring the RAG autoinject handling. * Tighten plan-without-action re-prompt comments * Tighten plan-without-action re-prompt comments * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: match unified plan-without-action nudge cap to GGUF default of 3 The shared MAX_ACT_REPROMPTS was set to 1, but GGUF's established default (llama_cpp.py) has re-prompted a stalling model up to 3 times since unslothai#5620. Restore the GGUF-matched cap so safetensors and MLX inherit the same behavior, and update the safetensors cap test to assert the cap dynamically. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Fix six race conditions when a user switches or cancels a model while a previous load or generation is still in flight, across the inference orchestrator and the /load and /unload routes: - Cancel an in-flight generation on a safetensors/MLX model switch and serialize unload with load under the inference lifecycle gate. - Cancel an in-flight load off the lifecycle gate so a Stop-loading cancel does not wait out the multi-minute load; guard the dispatched mailbox against a racing unload. - Recheck the loading marker after spawn and again after the load response before publishing, so a load cancelled mid-flight is reaped instead of going live. - Discard the loading marker before tearing the subprocess down in cancel_load, closing a spawn-after-cancel window and an orphaned compare-mode dispatcher during unload. - Match the unload target before canceling an in-flight GGUF load and add an off-gate fast path for the still-loading GGUF case. - Run the Unsloth unload off the event loop so a paused SSE stream holding _gen_lock cannot block the loop. Adds studio/backend/tests/test_orchestrator_unload_cancel.py covering the unload/cancel/switch race paths.
…s None (unslothai#7199) * fix(chat_templates): bind loop_messages when default_system_message is None construct_chat_template(default_system_message=None) built a system part that binds loop_messages only inside the `{% if messages[0]['role'] == 'system' %}` arm. The `Fix missing loop_messages` step right below then found no unconditional `{% set loop_messages = messages %}`, concluded loop_messages was missing, and rewrote `{% for message in loop_messages %}` back to `{% for message in messages %}` -- undoing the `messages[1:]` skip. A caller-supplied system message therefore reached the loop and tripped raise_exception: Only user and assistant roles are supported! Add the `{% else %}` arm so loop_messages is always bound, mirroring the default_system_message is not None branch minus the default text. That also stops the rewrite from firing, since the unconditional binding is now present. Renders before / after, same template, same inputs: default_system_message input before after None system msg raise_exception 'Be terse.\n### User: Hi\n' None no system '### User: Hi\n' unchanged 'You are helpful.' system msg 'Be terse.\n### User: Hi\n' unchanged 'You are helpful.' no system 'You are helpful.\n...' unchanged The rewrite still fires for templates with no {SYSTEM} part, which is what it was there for -- verified unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Scope loop_messages binding to {SYSTEM} templates for PR unslothai#7199 The None branch now only adds the else arm when system_part contains {SYSTEM}, so a static prefix with no {SYSTEM} placeholder keeps raising on a caller system message instead of silently dropping it. Strengthen the tests: assert the default does not leak when a caller system message is present, and add a regression test for the static prefix case. --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: danielhanchen <danielhanchen@gmail.com>
…d vendor on re-runs (unslothai#7250) * install: preserve the previous torch release across every flavor and vendor A re-run of curl | sh over an existing install was supposed to keep the user's validated torch release, but the pin required the old build's local flavor tag to match the freshly chosen index leaf. That gate was wrong in practice: a PyPI-sourced torch reports a BARE version (on Linux the PyPI wheel IS a CUDA build), which classified as cpu and never matched a cu leaf, so a healthy 2.10 on a cu130 host was silently moved to 2.11 (reproduced end to end); the same happened for any flavor drift such as cu128 to cu130 after a driver upgrade, and AMD ROCm leaves were excluded from preservation entirely. The rule is now release-based and flavor-agnostic: the probed previous release is pinned whenever it sits inside the final constraint window, and the pin installs from the freshly chosen index, so the flavor always follows the machine (NVIDIA cu*, AMD rocm/gfx, Intel/CPU, mac) while the release follows the user. The pin is evaluated AFTER every index and constraint decision including the Strix reroute, so raised floors (rocm7.2 / Strix gfx need torch 2.11 for the _grouped_mm fix) correctly reject an older release and win. UNSLOTH_TORCH_UPGRADE=1 still opts out, out-of-window releases are never kept, and probe noise never becomes a pin. The kept-release install with its range fallback (for indexes that do not carry the exact release) is factored into _install_torch_default_index and used by every --default-index torch path: the default NVIDIA/CPU/mac path and all three ROCm-index fallbacks, which previously bypassed the fallback. The Radeon-repo direct-wheel path keeps its curated per-arch wheel set (those wheels are already exact-pinned per rocm release). Platform coverage: install.sh serves Linux, WSL (including the WoA fallback), and macOS for all vendors; native Windows install.ps1 still caps at <2.11.0 everywhere, so the silent 2.10-to-2.11 move cannot occur there (2.11 alignment is a separate follow-up). Verified: 35-check unit suite rewritten to the new spec (any-flavor keep, floor rejection, noise, window edges, opt-out, wiring including pin-after-reroute and helper coverage); end-to-end matrix against sandboxed UNSLOTH_STUDIO_HOME installs on a cu130 host covering PyPI bare, cu128 drift, cu130 same-flavor, out-of-window 2.3, the upgrade opt-out, the hidden-GPU cpu leaf, and a fresh-install control. * install: honor the kept torch release on the Radeon direct-wheel path The Radeon repo path installs an explicit wheel trio selected by _pick_radeon_wheel, bypassing --default-index, so the kept-release pin only took effect when the listing failed and the install fell back to the ROCm index. On a re-run over an in-window Radeon install the trio search started at the newest common minor and silently moved the user forward (2.9 to 2.10 whenever the repo offered both). The trio search now starts at the kept release's minor when _PREV_TORCH_PIN is set and the listing still offers a torch wheel for that minor. Radeon wheels are patch-curated per rocm release, so the minor is the unit of preservation there; the raised rocm7.2 / Strix floors still win because the pin is window-checked against the final constraint before this point, and gaps keep the existing downward search / ROCm-index fallback. Verified with a simulated listing carrying both a 2.9 and a 2.10 trio: no pin selects the 2.10 trio, a kept 2.9 release selects the matched 2.9 / 0.24 / 2.9 trio, and an unavailable minor degrades to the newest trio. Added a structural wiring check to test_previous_torch_pin.sh (now 36 checks). * install: tighten comments in the torch preservation paths * install: exact kept release on the Radeon path, pin fallback in ROCm repairs The minor-level clamp on the Radeon direct-wheel path still allowed patch drift (a kept 2.10.0 could become 2.10.1 when the listing carried both) and the downward gap search could settle below the kept minor, both breaking the exact preservation guarantee the other vendor paths honor. The kept release now gets an exact-first trio attempt before the newest-trio search: pick the kept patch (else the newest patch of the kept minor, for listings that pruned the exact patch) together with the paired torchvision/torchaudio wheels for that minor. Any gap warns and falls back to the unchanged newest-trio search, mirroring _install_torch_default_index, so a rerun installs either the kept release or the same set a fresh install would choose, never something in between. The two ROCm torch repair sites (torch overwritten by dependency resolution, on the migrated and fresh paths) installed TORCH_CONSTRAINT directly, so a pinned release missing from the generic ROCm index would abort the rerun instead of falling back. Both now route through _install_torch_default_index, which passes extra uv args through (--force-reinstall) and clears the pin once the fallback fires so later paths stay consistent. Verified against synthetic listings: both patches listed keeps exactly 2.10.0; a kept minor missing vision/audio warns and yields the newest complete trio rather than a silent undercut; a pruned patch stays on the kept minor; no pin keeps the existing newest-trio behavior. Unit suite now 39 checks, all passing. * install: never pin nightly/dev/source torch builds on a rerun A survey of published torch version strings (PyPI bare, +cpu, +cu116 through +cu132, +rocmX.Y and +rocmX.Y.Z, +xpu, nightly .devYYYYMMDD, source a0+git, rc tags) showed one gap: nightly, dev, rc, and source builds passed the loose release-shape check, producing a pin such as torch==2.11.0.dev20250704 that no stable index carries. The range fallback rescued the install, but it printed "keeping it" and then burned a doomed resolve first. The base must now be a plain numeric X.Y[.Z] release, so those builds skip the pin and go straight to the newest supported release. Added unit checks for +xpu and three-component +rocm7.2.1 tags (both already preserved correctly) and for nightly, a0 source, and rc builds (never pinned). Suite now 44 checks, all passing. * install: pair kept-release companions, protect the flavor repair, note substitutions Three fixes from a 12-way review pass over the preservation work: The kept-release install left torchvision and torchaudio unconstrained next to the exact torch pin. torchvision exact-pins its torch in wheel metadata so it always paired correctly, but torchaudio no longer does: a kept torch 2.9.0 on cu130 resolved torchaudio 2.11.0 (verified with uv dry-runs). The helper now pairs both companions to the kept minor (torchvision 0.minor+15, torchaudio 2.minor); if the index lacks the paired set the existing range fallback fires. Verified resolving correctly on cu130, cu126, and rocm6.4. The wrong-flavor repair at the end of the install was the one remaining default-index torch install outside the helper. It runs under set -e, so a retained pin absent from the repair index (reachable when the Radeon direct-wheel path installed the kept release and dependency resolution later overwrote it) aborted the installer at the last step instead of falling back. It now routes through the helper with its reinstall flags passed through. The Radeon kept-release path installed a same-series build silently when the listing had pruned the exact patch; it now prints what it is substituting. Unit suite extended with wiring checks for all three (46 checks, all passing).
…location (unslothai#7252) unslothai#6414 moved the llama_extra_args inheritance out of the GGUF branch in _load_model_impl into _guard_chat_load_against_training, which runs before the branch, so 'if request.llama_extra_args is None' is no longer inside the gguf_branch slice that test_load_marker_precedes_hub_guard_and_unload checks. The assertion failed on that now-missing landmark even though the guarantee it protects (the gguf_load_in_flight marker is entered before the hub-download guard and the unload) is intact. Drop the relocated landmark from the ordering so the test matches the current structure. Co-authored-by: danielhanchen <unslothshared@gmail.com>
Route --yolo to OpenCode native --auto for the default TUI and run; keep the config permission fallback for no-auto subcommands (including hidden console/generate) and for --mini, which ignores --auto.
Pin the fetched Hermes install.sh/install.ps1 and the checkout they perform to an immutable upstream commit, and distinguish pinned from unpinned sources in the consent warning.
Bridge PWD through WSLENV /p when launching a Windows npm shim from WSL so project-root discovery uses the live cwd. The no-launch recipe adds PWD/p without freezing PWD; the concrete cwd override applies only on direct launch.
…e) (unslothai#7204) * Studio: persist llama.cpp KV cache across idle auto-unload (slot save/restore) * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: address KV persistence review feedback * Studio: guard KV restore on launch config * Studio: fix KV resume purge race, fingerprint requested ctx, purge on disable * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: re-check idle/keep-KV settings after slot save, ns file identity * Studio: shard-aware KV guard, honor user --no-cache-prompt, early save cap * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: honor LLAMA_ARG_CACHE_PROMPT env in slot-save guard * Studio: derive prompt-cache state from final argv for slot saves * Studio: stat LoRA/control-vector sidecars in KV restore fingerprint * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: parse csv and FNAME:SCALE sidecar syntax in KV fingerprint * Studio: address codex review on idle-unload KV resume * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: harden slot-save cleanup, cap accounting, stale-KV guard, save timeout * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: treat unavailable KV estimate as full-cap for slot-save disk check --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Han <danielhanchen@gmail.com>
* Fix text-only VLM CPT packing truncation * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Handle streaming vision datasets in packing * Harden multimodal packing detection * Preserve safe packing boundaries * Scope stream packing checks to VLMs * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Narrow VLM packing detection * Align packing mode and eval safety * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Add qwen3_5/qwen3_next to PADDING_FREE_BLOCKLIST to avoid packed-sequence contamination * Detect hybrid linear-attention models structurally instead of by name for packing guard * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Install wrapped-packing setup at the signature, not the Zoo license comment The _unsloth_wrapped_packing / _inspect setup block was injected by matching the exact 'All Unsloth Zoo code licensed under LGPLv3' comment line in the sourced sft_prepare_dataset. The unsloth_zoo dependency is only lower-bounded, so a newer Zoo that moves or drops that header made the setup a silent no-op while the truncation and pack_dataset rewrites still emitted references to those names, raising NameError on every SFT dataset preparation. Anchor the setup on the function signature instead (a structural location that always exists) and fail loudly if it cannot be found, so the helper variables are always defined before they are referenced across Zoo versions. Adds a regression test that patches in a Zoo source without the license header. * [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: Etherl <61019402+Etherll@users.noreply.github.com> Co-authored-by: danielhanchen <danielhanchen@gmail.com>
…back unslothai#7213 (unslothai#7228) * test(studio): add e2e test for cpu-fallback overriding vulkan * feat(studio): add UNSLOTH_LLAMA_CPP_BACKEND env var * feat(studio): add UNSLOTH_LLAMA_CPP_BACKEND env var * Preserve UNSLOTH_LLAMA_CPP_BACKEND=cpu across llama.cpp updates for PR unslothai#7228 The in-app updater rebuilt the installer command without --cpu-fallback and only re-asserted Vulkan, so accepting a llama.cpp update after forcing CPU on an Intel iGPU host re-ran host detection and routed back to the crashing Vulkan bundle (unslothai#7213). Record install_kind in the prebuilt marker and re-assert --cpu-fallback on update when the installed bundle is CPU. Also make setup.sh's UNSLOTH_LLAMA_CPP_BACKEND check case-insensitive to match setup.ps1, and add tests for the updater CPU preservation and the setup.sh flag plumbing. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Trim and validate UNSLOTH_LLAMA_CPP_BACKEND, warn on unknown values for PR unslothai#7228 Trim surrounding whitespace and lowercase the value in both setup.sh and setup.ps1, so values like ' cpu ' or 'CPU' still force the CPU-only prebuilt. An unrecognized value (e.g. 'gpu') now prints a warning instead of silently falling back to auto. Extend test_setup_llama_cpp_backend.py to cover both scripts, including trimmed, empty and unknown values. * Preserve arm64 CPU installs on update and honor CPU override in Windows prune for PR unslothai#7228 The update-path CPU preservation only matched install_kind ending in -cpu, so arm64 CPU bundles (linux-arm64, windows-arm64) were re-routed to a GPU or source build on update. Match the full set of CPU-only kinds instead. Persisting install_kind also activated the previously inert Windows mismatch-prune in setup.ps1: on a GPU host with UNSLOTH_LLAMA_CPP_BACKEND=cpu it saw the windows-cpu marker as mismatched and deleted it every rerun. Normalize the override once and make CPU expected so a deliberate CPU install is kept. Extend the tests to cover both. * Document legacy llama.cpp markers keep heal-to-GPU on update for PR unslothai#7228 Legacy prebuilt markers written before install_kind was persisted intentionally do not force --cpu-fallback on update: the in-app updater lets them re-resolve (heal to a GPU bundle) per the existing behavior from unslothai#6097, and only markers that explicitly record a CPU install_kind are pinned to CPU. Add a comment and a regression case documenting the boundary. * Tighten llama.cpp CPU-fallback comments for PR unslothai#7228 * Fix Windows install-prune to keep valid Intel/fallback bundles for PR unslothai#7228 Persisting install_kind activated the setup.ps1 mismatch-prune, whose expectedKinds was incomplete: the non-NVIDIA/non-AMD branch omitted windows-vulkan (the Intel auto-route) and the GPU branches omitted the windows-cpu/windows-arm64 fallback the installer uses when a GPU prebuilt is missing. That made every setup rerun delete and re-download a valid Intel Vulkan (or CPU-fallback) install. List all kinds the installer can produce per host so only a bundle the host cannot run is pruned. Cover the full matrix in tests. * Persist force_cpu marker flag so only forced CPU installs re-assert on update for PR unslothai#7228 * Add --force-cpu for deliberate CPU installs and warn on macOS for PR unslothai#7228 * Record force_cpu when reusing a matching CPU bundle for PR unslothai#7228 * Accept force_cpu keyword in installer test validator fakes for PR unslothai#7228 --------- Co-authored-by: danielhanchen <unslothai@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Han <danielhanchen@gmail.com>
…on models (unslothai#7249) * Fix text-only VLM CPT packing truncation * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Handle streaming vision datasets in packing * Harden multimodal packing detection * Preserve safe packing boundaries * Scope stream packing checks to VLMs * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Narrow VLM packing detection * Align packing mode and eval safety * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Add qwen3_5/qwen3_next to PADDING_FREE_BLOCKLIST to avoid packed-sequence contamination * Detect hybrid linear-attention models structurally instead of by name for packing guard * Add experimental varlen packing for hybrid linear-attention models Feed seq_idx to the causal conv and cu_seqlens to the gated-delta scan so sample packing / padding-free reset state at sequence boundaries for hybrid linear-attention models (Qwen3.5, Qwen3-Next). Gated behind UNSLOTH_EXPERIMENTAL_HYBRID_PACKING and fail-closed: when the flag is off or the accelerated kernels (causal_conv1d + fla) are unavailable, the guard keeps these models on the padded path. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Harden hybrid linear-attention varlen packing shim Make patch_hybrid_linear_attention_varlen robust across transformers 4.57.6 through 5.x and TRL 0.22.2 through 1.x, following the import_fixes.py style: - Read UNSLOTH_EXPERIMENTAL_HYBRID_PACKING at call time so the flag takes effect when set after importing unsloth. - Idempotent: repeat calls on a patched model return True without re-validating the wrappers or double-wrapping; signatures are checked on captured originals. - Prefer the authoritative packed_seq_lengths (via get_packed_info_from_kwargs) over position_ids resets, handling pad_to_multiple_of trailing tokens. - Suppress injection for cached forwards (use_cache / past_key_values) so generation and eval are left on the untouched decode path. - Validate every gated-delta module before mutating any (transactional). - Bind position_ids / use_cache from both positional and keyword args. - Verify dispatch at runtime (Unsloth wraps each module forward, so the mixer source is not statically inspectable) and warn once if the shim is never hit. - Emit one deduped diagnostic on each fail-closed path. Add CPU unit tests covering the hybrid guard detection, the boundary builders, and the shim (fail-closed, active, idempotent, cached no-op, runtime handshake). * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Abort hybrid packing when the varlen shim is not fully dispatched The runtime handshake used a single per-module hit flag written by both the conv and scan wrappers, so a partial dispatch (only one kernel routed through self.<kernel>) passed the any() check and trained on contaminated data, and a missing dispatch only logged a warning. Track conv and scan dispatch separately, require both on every gated-delta module on the first packed forward, and raise before loss/backward when either is missing (the batch is already flattened, so there is no padded recovery at that point). Also skip an empty packed_seq_lengths before it reaches max(), and document the position_ids fallback's left-pad assumption. Add tests for no-dispatch and partial (conv-only / scan-only) abort, the packed_seq_lengths preference over a competing position_ids, MRoPE 3D position ids, and the pad_to_multiple_of trailing-segment path through the metadata builder. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Import the hybrid packing patch from its submodule to satisfy the import-hoist lint * Fail closed for hybrid packing on encoder-decoder, chunked-loss, and string-name models The varlen shim only helps decoder-only hybrid models that run their mixer through self.<kernel> on a live nn.Module forward. Three cases slipped past the guard: - Encoder-decoder configs (is_encoder_decoder) reached the packing path even though flattening a cross-attention batch is unsound. Block them explicitly. - TRL's chunked_nll loss (the 1.x default) calls the backbone directly and bypasses model.forward, so the per-instance forward wrapper that refreshes the varlen stash never runs. Detect that path and keep the model padded. - A string model_name reaches the trainer before the module exists, so the instance shim has nothing to patch. Resolve the config up front and keep string hybrids on the padded path. Adds encoder-decoder / decoder-only / chunked-loss / string-model tests. * Harden the SFT source-injection replacements and forward auth args for string models The wrapped-packing injection rewrote the sourced unsloth_zoo sft_prepare_dataset with str.replace anchored on the exact 'All Unsloth Zoo code licensed under LGPLv3' comment. str.replace never raises on a missing anchor, so a supported newer unsloth_zoo (the dependency is only lower-bounded) that moved that header would silently drop the setup while the truncation and pack_dataset edits still referenced _unsloth_wrapped_packing / _inspect, raising NameError on every SFT dataset preparation. - Install the setup at the sft_prepare_dataset signature via re.subn (a structural anchor that always exists) and raise if even that is missing. - Route the remaining edits through a _require_replace helper that fails loudly on a missing required anchor (or warns once for an optional one), formalizing the verify-then-replace idiom the DPO patchers in this file already use. - Reuse the guarded _unsloth_pack_has_strategy at the pack_dataset call instead of re-calling inspect.signature(pack_dataset) unguarded, so a non-introspectable pack_dataset cannot crash there after the setup already handled it. - _resolve_string_model_config now forwards token / use_auth_token / cache_dir / code_revision, so a private hybrid resolves its config instead of falling through as non-hybrid and enabling packing without the varlen shim. Adds regression tests for the drift-resistant injection, the helper, and the string-model auth forwarding. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Honor top-level SFTConfig.trust_remote_code when resolving a string model TRL merges the top-level args.trust_remote_code into the load via model_init_kwargs.setdefault("trust_remote_code", args.trust_remote_code) before create_model_from_path, so a remote-code hybrid is commonly set with SFTConfig(trust_remote_code=True) rather than inside model_init_kwargs. The config probe only read model_init_kwargs, so AutoConfig could fail for such a model, leave model_config None, and let the guard treat it as non-hybrid, enabling packing without the varlen shim. Mirror TRL's setdefault (model_init_kwargs wins). * Tighten hybrid-packing comments for concision * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: alkinun <alkinunl@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Etherl <61019402+Etherll@users.noreply.github.com>
…tection (unslothai#6692) * install: let UNSLOTH_TORCH_INDEX_FAMILY / _URL override CUDA wheel detection get_torch_index_url (and the studio-update mirror _detect_cuda_torch_index_url) chose the torch wheel family solely by probing the host GPU, with no override. In a headless / container / CI build the host driver is visible via the /proc/driver/nvidia/gpus fallback but nvidia-smi cannot report a CUDA version, so the function fell back to its cu126 default and installed the wrong wheels (e.g. a cu128 image got cu126 torch). Add an explicit override checked before any probing, in both the shell installer and the Python studio-update path: - UNSLOTH_TORCH_INDEX_URL full index URL, used verbatim (wins) - UNSLOTH_TORCH_INDEX_FAMILY family (cpu, cu128, rocm6.4, ...) appended to the mirror base (UNSLOTH_PYTORCH_MIRROR still honoured) This matches how the published GPU images select CUDA -- vLLM and SGLang take the CUDA version from an explicit build ARG rather than detecting it, and the Unsloth Docker base image already pins the cu128 index directly. Desktop installs are unchanged: with no override set, detection runs exactly as before. Adds test_get_torch_index_url.sh cases for the override (family, full URL, precedence, mirror base, trailing-slash strip, empty-ignored). * install: make the torch-index override authoritative across ROCm paths Address review feedback on the override added in this PR so a pinned index is honoured everywhere, not just in get_torch_index_url: - Skip the WSL ROCm bootstrap (root privilege + large downloads, probes /dev/dxg) when UNSLOTH_TORCH_INDEX_URL / _FAMILY is set; it previously ran before the override was consulted. - Skip the Radeon/Strix rerouting (which re-probes the GPU and overwrites the resolved URL with repo.radeon.com / repo.amd.com) when the index is pinned, so an explicit ROCm override (e.g. UNSLOTH_TORCH_INDEX_FAMILY=rocm6.4) is kept. - install_python_stack.py: derive _TORCH_BACKEND from the override when UNSLOTH_TORCH_BACKEND is unset (standalone studio update), so _ensure_rocm_torch / _ensure_cuda_torch repair to the requested family instead of re-detecting. - Strip ALL leading/trailing slashes in the shell override to match the Python side (avoids 404s on strict pip proxies). Adds test cases for double-slash and leading/trailing-slash overrides. * install: honor pinned torch index in CUDA/ROCm repair paths Follow-up to the override work in this PR: the get_torch_index_url / install.sh reroute already respect a pinned UNSLOTH_TORCH_INDEX_URL / _FAMILY, but the Python repair helpers in install_python_stack.py still re-probed the GPU and could overwrite the pinned family. Make the pin authoritative there too: - _ensure_cuda_torch: an explicit cu* pin commits to CUDA wheels, so repair a ROCm-poisoned venv even when no NVIDIA GPU is visible here (headless / container / CI cross-install), instead of bailing on the GPU-presence gate. - _ensure_rocm_torch: skip the AMD per-gfx (Strix) reroute when a ROCm index is pinned, and in the generic reinstall path install from the pinned URL verbatim rather than re-detecting the host ROCm version. gfx*/rocm7.2 indexes serve torch 2.11+, so select the 2.11 package specs for a gfx leaf. - install.sh: raise the torch constraint to 2.11 for */gfx* indexes too, matching rocm7.2, so a pinned full-URL/family override that returns early keeps a valid constraint. Add _explicit_torch_index_url / _explicit_rocm_torch_index_url helpers and tests covering the no-GPU CUDA pin repair and the explicit gfx index honored verbatim. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: honor torch-index override on the Windows installers too The pinned-index work landed for install.sh and install_python_stack.py, but the Windows installers still picked the wheel index from GPU probing. Extend the same UNSLOTH_TORCH_INDEX_URL / _FAMILY contract so a pinned index wins on every platform: - install.ps1: Get-TorchIndexUrl returns the pinned URL/family before nvidia-smi probing; the AMD ROCm reroute is skipped when the index is pinned, so an explicit cpu/cu* pin on an AMD host is not overwritten. - studio/setup.ps1: add shared Get-PinnedTorchIndexUrl / Get-TorchIndexLeaf helpers; the stale-venv check, the install selection and the AMD reroute all honor the pin, and the CPU/CUDA install pulls from the resolved index URL. - tests: parity test that all four installers read both override vars and the two Windows installers gate the AMD reroute on the pinned flag. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: complete pinned-index handling for ROCm/Windows edge cases Follow-ups to the override work flagged in review: - install.ps1: a pinned gfx*/rocm>=7.2 index previously skipped the AMD reroute that sets the torch>=2.11 floor, so the generic install used torch>=2.4,<2.11 and could resolve the known-bad _grouped_mm wheel. Route a pinned ROCm index through the ROCm install path with the 2.11 floor + companions, and guard the companion-spec lookup so a skipped reroute block cannot null-deref. - studio/setup.ps1: the stale-venv check compared the installed flavor (cuXXX/cpu, with +rocm misread as cpu) against the raw pinned leaf (gfx1151 / rocm6.4), so a correct pinned ROCm venv was always marked stale. Classify +rocm wheels as the generic 'rocm' flavor and normalize a pinned rocm*/gfx* leaf to 'rocm' before comparing (cu* stays specific so cu126-vs-cu128 still rebuilds). - install_python_stack.py: _ensure_cuda_torch now also reinstalls from a pinned CUDA index when the venv carries a CPU wheel (headless CPU-venv-to-CUDA cross-install via 'studio update'), not only when it finds a ROCm build. - tests: parity assertions already cover all four installers honoring the override. * install: finish pinned ROCm/CUDA edge cases on Windows + repair path Follow-ups to the previous round: - studio/setup.ps1: a pinned gfx*/rocm>=7.2 index now routes through the ROCm install path with the 2.11 floor + companions (it previously fell through to the CUDA branch with bare torch/torchvision/torchaudio against the ROCm index). The CPU/CUDA fallback index is forced to the CPU wheel index when a ROCm index is active, so a failed pinned-ROCm install does not retry the ROCm mirror. - studio/setup.ps1: the stale-venv check no longer treats an unrecognized pinned URL leaf (e.g. a PEP 503 mirror ending in /simple) as a torch flavor tag, which was marking a correct venv stale; cu*/cpu/rocm/gfx leaves are still compared. - install.ps1: the post-failure CPU fallback uses an explicit CPU index instead of , which for a pinned ROCm index was the ROCm mirror itself (so the 'fallback' just retried the failing index and aborted the installer). - install_python_stack.py: _ensure_cuda_torch now also reinstalls when the venv's CUDA family differs from a pinned one (installed cu126 vs pinned cu128), not only CPU->CUDA; the probe reports the installed cuXXX tag for the comparison. * install: keep the ROCm to CPU fallback install inside the retry-helper window The pinned-ROCm CPU fallback computes an explicit CPU index, but the comment explaining why it cannot reuse $TorchIndexUrl pushed the actual Invoke-InstallCommandRetry / --force-reinstall call more than 600 chars past the "ROCm PyTorch install failed" message, so test_pr5940_followups's window check no longer saw the retry helper. Move the CPU-index computation and its comment above the failure substep so the retrying force-reinstall stays adjacent to the message. No behavior change: same explicit CPU index, same retry, same --force-reinstall. * install: address #6692 review round 5 (ROCm/CPU pin edge cases) setup.ps1: - Stale-venv check: treat an AMD/ROCm host (HasROCm or a resolved gfx arch) with no explicit pin as expecting "rocm", not "cpu", so a healthy +rocm venv is not flagged stale (which made installer-managed setup exit and direct update rebuild). - Pinned-ROCm install failure now routes into the force-reinstall CPU branch: CuTag stays the rocm/gfx leaf on failure, so the condition also checks ROCmCpuFallback; otherwise the CUDA branch installed from the CPU index without --force-reinstall and kept the partial ROCm torch. - Explicit ROCm pin compare no longer collapses gfx*/rocm* to a generic "rocm": it compares the +rocmX.Y version (and the torch 2.11 line for gfx pins) so changing the pinned family (e.g. rocm6.4 -> gfx1151) rebuilds and applies it. install_python_stack.py: - _ensure_rocm_torch: an explicit ROCm wheel-index pin now bypasses the NVIDIA-present / no-AMD-GPU / unreadable-ROCm gates (headless/container/CI cross-install), mirroring the explicit-CUDA-pin bypass in _ensure_cuda_torch. - Add _ensure_cpu_torch: an explicit CPU pin (FAMILY=cpu or /cpu URL) now has a repair path that reinstalls CPU torch over an existing CUDA/ROCm build on a standalone update (which skips install.sh's flavor enforcement). install.sh: - Pin torchvision/torchaudio companions alongside torch for the rocm7.2 / per-gfx index and the Strix reroute (those AMD indexes publish companions independently and a bare name can resolve a torch-2.12-built wheel, an ABI mismatch). * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * torch-index override: classify CUDA pin by leaf; trim blank shell overrides _ensure_cuda_torch only overrode the NVIDIA-presence gate for *any* pinned index, so a non-CUDA mirror URL (or a ROCm/CPU pin) on a non-NVIDIA host with ROCm torch could force a CUDA reinstall over a working ROCm venv. Add _explicit_cuda_torch_index_url() (leaf cu*), matching the ROCm/CPU helpers, and gate on it instead. install.sh::get_torch_index_url treated a whitespace-only UNSLOTH_TORCH_INDEX_URL / _FAMILY as authoritative (yielding an invalid index), unlike the Python .strip() and PowerShell IsNullOrWhiteSpace paths; trim leading/trailing whitespace first. * install: honor pinned torch index over CVD/GPU gates and fix leaf-based ROCm classification - install_python_stack.py: an explicit cu* pin now clears the CUDA_VISIBLE_DEVICES empty/-1 hide gate as well as the NVIDIA-presence gate, so CVD=-1 UNSLOTH_TORCH_INDEX_FAMILY=cu128 studio update repairs to CUDA wheels (parity with install.sh's get_torch_index_url override, which skips all GPU probing). Unpinned CVD=-1 still skips. - install_python_stack.py: _ensure_cpu_torch installs the bounded _CPU_TORCH_PKG_SPEC instead of a bare torch/torchvision/torchaudio trio; the /cpu index now also serves torch 2.11+, which is outside the supported <2.11 range. - install.sh: the torch>=2.11 constraint case matches the index leaf (rocm7.2|gfx*) instead of the whole URL, so a mirror base path containing a gfx/rocm7.2 segment with a cu*/cpu family is not false-matched onto the 2.11 line. - setup.ps1: the stale-venv check expects rocm torch only for arches the install path maps to a repo.amd.com wheel index; an unmapped/unreadable arch installs CPU, so a correct CPU venv is no longer marked stale. - Tests for each of the above. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: tighten pinned torch-index override edge cases - install.sh: trim whitespace-only UNSLOTH_TORCH_INDEX_URL/_FAMILY before the _torch_index_pinned guard, matching get_torch_index_url, so a blank override no longer skips the WSL bootstrap and Radeon/Strix reroutes while detection still picks the normal index. - install.sh / install.ps1 / setup.ps1 / install_python_stack.py: force the torch 2.11 floor only for the gfx families with the <2.11 _grouped_mm bug (gfx120X-all, gfx1151, gfx1150). A pinned override to gfx110X-all/gfx90a/gfx908 stays on the default range, matching the automatic AMD path. - install_python_stack.py _ensure_cuda_torch: treat an untagged CUDA build under a CUDA pin as a family mismatch (reinstall), and match cuXXX pins narrowly (cu + digits) so a custom/current mirror leaf no longer forces CUDA over a CPU/ROCm venv. - install_python_stack.py _ensure_rocm_torch: reinstall when an explicit ROCm pin names a different ROCm family than the already-installed ROCm torch (the ROCm analogue of the CUDA cuXXX mismatch repair). Adds tests for each case. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: fix second-order edge cases in pinned torch-index ROCm/CUDA handling Parse the ROCm torch probe positionally so an empty HIP marker is kept: CPU/CUDA torch no longer reads as HIP, so the ROCm reinstall is not skipped. Emit one "<marker>|<version>" line (like the CUDA probe) for a robust parse. Limit the gfx torch 2.11 expectation to the install allowlist (gfx120X-all/gfx1151/gfx1150). A pinned gfx110X-all/gfx90a/gfx908 index stays on the default <2.11 specs, so a correct 2.10+rocm wheel is no longer judged a mismatch and force-reinstalled every update. Distinguish an AMD per-arch wheel (three-part +rocmA.B.C) from a generic pytorch.org wheel (two-part +rocmA.B): a gfx per-arch pin over a generic 2.11 wheel now reinstalls the per-arch wheel, while an already-installed per-arch wheel is not re-flagged (no reinstall loop). Mirror all of the above in setup.ps1 via new Test-RocmGfx211Leaf / Test-CudaFamilyLeaf / Get-RocmPinStaleTags helpers, reused by both the install-spec path and the stale-venv check so they cannot diverge again. Require a digit after "cu" (^cu[0-9]) in setup.ps1, install.ps1 and install.sh so a mirror leaf like /custom or /current is not branded CUDA and does not rebuild the venv every run. Add tests: CPU/CUDA probe -> has_hip_torch False; gfx110X-all pin + 2.10 wheel not stale; gfx1151 pin + generic 2.11 wheel stale; gfx1151 pin + per-arch wheel not stale; /custom and /current not CUDA; plus cross-language allowlist and cu-digit parity guards, and a PowerShell unit test for the new setup.ps1 helpers. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix ROCm/gfx pin case normalization, ROCm-tag requirement, and CUDA-leaf classification Normalize torch-index leaves to lowercase before the gfx*/rocm*/cu* allowlist matches so the canonical gfx120X-all (capital X) gets the torch 2.11 floor in install.sh (leaf, flavor and repairable helpers). Require an installed +rocm local tag before a rocmX.Y or non-2.11 gfx pin is judged satisfied in setup.ps1 Get-RocmPinStaleTags and the Python _rocm_pin_family_mismatch, so an untagged CPU/CUDA wheel never leaves the pin unapplied. Classify a leaf as CUDA only via ^cu[0-9]: the Python _TORCH_BACKEND derivation now uses _is_cuda_family_leaf, and install.sh brands cuda only on cu[0-9]* (unset on an unknown /current /custom mirror leaf) so the stack probes the GPU instead of skipping ROCm repair. Add bash, Python and PowerShell tests for capital gfx120X-all floor, current/custom not-cuda, and untagged-wheel ROCm pins. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: converge torch-index pin detection via a per-venv marker Introduce a torch-index MARKER that records the exact wheel --index-url used after each successful torch install, so `unsloth studio update` / repair makes the "did the pinned index change?" decision by an EXACT string compare rather than inferring it from the wheel +rocm/+cu version tag. The tag cannot encode the AMD per-arch gfx family (two 2.11 gfx indexes both install +rocm7.13.0), so the tag heuristic missed a gfx1151 -> gfx120X-all switch and a custom-URL swap. Marker path is per-venv (.unsloth-torch-index), one line = the resolved index URL, written atomically (temp + rename). Path, format and normalization are shared across all four installers (install.sh, install_python_stack.py, setup.ps1, install.ps1). - Reapply gfx pins on a per-arch target change: the marker's exact compare reinstalls when the pinned index differs, even when both wheels share a tag. - Honor custom ROCm URL pins during repair: an explicit index whose leaf is not rocm/gfx/cu/cpu (e.g. simple, current) now reinstalls torch VERBATIM from the pin when it differs from the marker ("URL wins verbatim"). - Align the KNOWN-2.11 rocm/gfx set to exactly rocm7.2 plus the gfx allowlist gfx120x-all/gfx1151/gfx1150 in every language; stop treating an unknown newer rocm (rocm7.3, which does not exist) as the 2.11 line speculatively. Backward compatible: with no marker (old venvs, torch installed out-of-band) the existing +rocm/version-tag heuristics still decide, and a matching marker never reinstall-loops. A cu128 CUDA pin stays a CUDA pin; custom and current leaves are not CUDA. Adds marker tests (py/sh/ps) plus cross-installer parity checks. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: keep the torch-index marker additive to flavor validation Three narrow fixes in the marker-based stale-venv detection: - setup.ps1: a matching marker no longer overwrites the detected installed flavor. The marker compare is now an additional rebuild trigger, so a stale wheel (torch swapped to a +cpu build while the marker still records a cuXXX pin) is still caught by the flavor check instead of being masked as up to date. - setup.ps1: a supported AMD arch carrying CPU torch is no longer marked stale and wiped. The downstream AMD Windows ROCm override upgrades CPU torch to ROCm in place, so wiping first would delete the venv and abort with "Virtual environment not found". Only a genuinely wrong CUDA wheel still rebuilds. - install.sh: the Radeon --find-links path records its repo.radeon.com base in the marker instead of the generic pytorch.org ROCm fallback index, so a later pin to that generic family correctly reinstalls rather than comparing equal. Mirrors install.ps1/setup.ps1, which already record the real AMD index. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: honor custom pins and repair pinned venvs in place Four follow-ups to the torch-index marker work: - install_python_stack.py: _ensure_cuda_torch/_ensure_rocm_torch now bail when an explicit custom-index pin names no known torch family, so a verbatim URL override (a private/simple mirror) is not clobbered by auto-detected CUDA/ROCm wheels before _ensure_verbatim_torch_index applies it. - install_python_stack.py: the ROCm marker is additive, not a substitute -- a matching marker still runs the family/version check so a wheel swapped after the marker was written is caught. Mirrors setup.ps1. - setup.ps1: a stale venv under an explicit pin, whose torch still imports, is repaired in place (force-reinstall torch from the pin in the dependency pass) instead of wiped. The wipe path only delegates to install.ps1, so on a direct update it stranded the user at "Virtual environment not found" instead of applying the new pin. A broken venv or unpinned drift still wipes/delegates. - install.ps1: when a pinned ROCm install fails over to a CPU base, the marker now records the CPU index actually used instead of the ROCm pin, so the next managed setup does not see CPU torch under a ROCm pin and abort as stale. * setup.ps1: keep the ROCm CPU-fallback force line the pr5940 test guards 5c93ffd4 folded the pin-change force-reinstall into the ROCm CPU-fallback condition on one line, so the exact literal that test_pr5940_followups.py checks (if ($ROCmCpuFallback) { $cpuForce = @("--force-reinstall") }) no longer appeared and the test failed. Split the two conditions into separate if lines: the ROCm fallback line is restored verbatim and the pin-change force is its own line. Both still set $cpuForce to the array, so @splat passes one arg. * install: honor exact CUDA/custom index URL pins in the torch-index marker Address three Codex review findings on the torch-index marker mechanism: - install.sh: after the ROCm CPU repair reinstalls torch from the generic $TORCH_INDEX_URL, record that as the marker source. A Radeon --find-links install set _TORCH_MARKER_INDEX_URL to its repo.radeon.com base earlier, so leaving it made the marker misreport Radeon wheels and a later Radeon pin would compare equal and skip a needed reinstall. - install_python_stack.py: _ensure_cuda_torch now consults the exact-URL marker (_marker_pin_mismatch) when the installed +cuXXX tag matches the pinned leaf, so a same-leaf CUDA mirror change (official cu128 to an internal cu128 mirror) is reinstalled and re-recorded instead of skipped. - _normalize_index_url / _normalize_family_leaf (install.sh, setup.ps1, install_python_stack.py): lowercase only KNOWN wheel-family leaves (rocm/gfx/ cpu/cuXXX) so gfx120X-all still matches gfx120x-all, while a custom (unknown-family) leaf keeps its case so a verbatim URL pin like /Current does not compare equal to /current. Tests updated to assert the refined behavior. * install: fix 3 torch-index marker edge cases (CPU mirror pin, Radeon leaf, migrated venv) Addresses three review findings on the torch-index override path: 1. CPU index URL change on an already-CPU venv. _ensure_cpu_torch returned early whenever torch was already a CPU build, so a standalone update that moved the pin (official /cpu -> a private UNSLOTH_PYTORCH_MIRROR /cpu, same +cpu tag) never reinstalled. It now consults the exact-URL marker and reinstalls only when _marker_pin_mismatch reports a different index, mirroring the CUDA/ROCm same-family handling. A matching marker (or none) still leaves CPU torch untouched, so there is no reinstall loop. 2. Radeon find-links directory misclassified as a pip ROCm family. A repo.radeon.com/.../rocm-rel-7.2.1 leaf starts with "rocm" but is a find-links listing, not a pip --index-url. The old startswith(("rocm", "gfx")) test routed it into a --index-url reinstall that fails against find-links. New _is_pip_rocm_family_leaf gates on ^rocm\d / gfx (matching install.sh's rocm[0-9]* and setup.ps1's ^(rocm[0-9]|gfx)), so a Radeon URL routes to the verbatim/marker path instead. 3. Migrated venv rewriting its marker to a pin it did not install. install.sh and install.ps1 write the marker unconditionally, so a migration that preserves existing torch recorded the newly requested pin and a later update then found a matching marker and skipped the reinstall the pin needs (e.g. a per-arch gfx1151 -> gfx120X-all switch, identical +rocm tag). Both now track _TORCH_INSTALLED_THIS_RUN and write the marker only when torch was actually installed or repaired this run. Also add Get-NormalizedFamilyLeaf to the setup.ps1 helper-extraction list in test_torch_index_marker.ps1 (it was added to setup.ps1 and the shell test in an earlier round but missed here) and add two unit tests covering findings 1 and 2. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: keep pinned torch repairs on the pinned index Two fixes for explicit index pins (UNSLOTH_TORCH_INDEX_FAMILY / _URL): 1. install_python_stack.py's repair paths ran uv without clearing the inherited uv index env vars. uv resolves the default index (--index-url or --default-index) at the LOWEST priority, so a UV_INDEX or UV_EXTRA_INDEX_URL mirror in the environment won for any package it served: a cu128-pinned repair could install torch from the mirror and then record the cu128 marker it never used. Verified empirically: with UV_EXTRA_INDEX_URL=.../cu126 exported, uv pip install torch --index-url .../cu128 resolves torch 2.13.0+cu126. Strip the four uv index env vars for pinned-index commands only, mirroring the gate install.sh, install.ps1 and setup.ps1 already have; non-pinned installs keep the user's mirror. 2. install.ps1 routed any pinned leaf matching rocm* through the ROCm --default-index path, so a custom find-links leaf like rocm-rel-7.2.1 was treated as a PEP 503 ROCm index and could silently fall back to CPU torch on resolution failure. Require a digit after rocm, matching install.sh's rocm[0-9]* and install_python_stack.py's ^rocm\d. Adds parity + unit tests for both (11 new tests). * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: keep pinned repairs off UV_TORCH_BACKEND and narrow setup.ps1's rocm pin match Round 2 of the pinned-index hardening: 1. _build_uv_cmd converted UV_TORCH_BACKEND into --torch-backend before the new env isolation could act, and uv's torch backend redirects torch resolution to its own per-backend index even when --index-url is given (verified: a cu128-pinned dry run with UV_TORCH_BACKEND=cpu resolves torch 2.13.0+cpu). Pinned-index commands now never receive the flag and UV_TORCH_BACKEND joins the stripped env vars, so uv cannot re-read it. 2. setup.ps1's pinned reroute had the same bare rocm* glob install.ps1 had: a custom find-links leaf like rocm-rel-7.2.1 was routed through the ROCm --index-url path instead of the verbatim unknown-pin path. Now requires a digit after rocm, matching install.ps1, install.sh and _is_pip_rocm_family_leaf. 3. The marker test's case-normalization checks used -eq, which is case-insensitive in PowerShell, making them vacuous, and the unknown-leaf expectation was written lowercased while the implementation deliberately preserves custom-leaf case. Tightened to -ceq with the case-preserving expected value. Adds unit + parity tests for 1 and 2 (5 new tests). * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: extend the pinned-index guards to every remaining surface Round 3 of the pinned-index hardening, closing the same holes on the surfaces the earlier rounds missed: 1. install.sh's pinned-install env scrub now clears UV_TORCH_BACKEND (uv's torch backend redirects torch resolution to its own per-backend index even against --default-index), and both PowerShell wrappers clear it in their pinned-install scrubs, matching install_python_stack.py. 2. setup.ps1's marker stale check still classified any rocm* leaf as a PyTorch ROCm family while the install selection is digit-gated, so a custom rocm-current / rocm-rel-7.2.1 pin stale-compared as not-rocm vs rocm and force-reinstalled on every studio update. The stale check now uses the same ^rocm\d gate. 3. install_python_stack.py's pinned-command scrub also strips PIP_EXTRA_INDEX_URL for the pip fallback: pip adds the env extra index in addition to --index-url, so an inherited mirror could satisfy torch off the pin while the marker recorded the pinned URL. PIP_INDEX_URL needs no strip since the explicit --index-url flag overrides it. Parity + unit tests extended (4 new tests). * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: scrub find-links and carry the pinned scrub through pip fallbacks Round 4 of the pinned-index hardening: 1. UV_FIND_LINKS joins every pinned-install scrub (install.sh, install.ps1, setup.ps1, install_python_stack.py): uv's --find-links locations can satisfy torch off the pinned index the same way an extra index does. 2. setup.ps1's Fast-Install restored the scrubbed vars in its finally BEFORE the pip fallback ran, and never touched the pip env vars at all, so a failed uv attempt fell back to python -m pip with an inherited PIP_EXTRA_INDEX_URL / PIP_FIND_LINKS able to win over the pinned --index-url. The scrub now wraps the whole function (uv attempt + pip fallback) and includes the pip vars; restore happens after both. 3. install_python_stack.py's scrub also strips PIP_FIND_LINKS for its own pip fallback, completing the PIP_EXTRA_INDEX_URL fix from round 3. Parity tests extended (2 new tests). * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: digit-gate rocm leaves in marker normalization and ROCm side effects Round 5 of the pinned-index hardening (three custom-rocm-leaf edge cases): 1. _normalize_family_leaf lowercased every leaf starting with rocm, so a custom mirror leaf like rocm-Current compared equal to its lowercase form and a case-only pin change was skipped. URL paths can be case-sensitive. The rocm prefix is now digit-gated (rocm[0-9]*, matching _is_pip_rocm_family_leaf) in install.sh, setup.ps1 and install_python_stack.py, so only true family leaves (rocm7.2) are lowercased; a custom rocm-* leaf keeps its case. 2. setup.ps1 Test-MarkerPinMismatch compared normalized URLs with -ne, which is case-insensitive in PowerShell, so a case-only marker change (Simple vs simple) was treated as matching and the reinstall skipped. Now -cne. 3. install.sh gated the AMD bitsandbytes install and the "repair ROCm torch" --default-index reinstall on a bare whole-URL rocm glob, so a custom CPU/CUDA/private index whose leaf merely starts with rocm (rocm-current) was force-repaired from the wrong ROCm-only path whenever torch.version.hip was empty. Both now gate on _torch_index_is_rocm_family, computed once from the digit-gated leaf (rocm[0-9]*/gfx*). Tests: 4 new parity assertions plus 2 case-sensitivity marker checks. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: apply an explicit custom torch-index pin on the first update Round 6: an explicitly-set custom (unknown-family) UNSLOTH_TORCH_INDEX_URL was silently ignored on the first `studio update` of a venv that predates the marker feature, on both platforms, because the no-marker case was treated as "do nothing" and the version-tag heuristics cannot judge an unknown leaf. 1. install_python_stack.py _ensure_verbatim_torch_index now reinstalls verbatim when the marker is ABSENT (None), not only when it differs, and short-circuits only when the marker already records this exact pin. It then writes the marker, so every later update is a no-op. A user who did not set the override gets pin=None and is untouched, so an out-of-band torch install is never clobbered. 2. setup.ps1: for an unknown-family pin on a marker-less venv the stale-venv check now sets PinChangedForceReinstall so the torch block reinstalls in place from the pin. It deliberately does NOT set shouldRebuild, which would wipe the venv and strand a direct `studio update`. 3. setup.sh (the Linux `studio update` entry point) skipped install_python_stack.py entirely when unsloth was already current, so the marker-driven reinstall (both the verbatim custom pin and the cu/rocm flavor and family-change repair, e.g. gfx1151 to gfx120X-all) never ran. It now forces the dependency pass when a torch-index pin env var is set; the pass is idempotent and no-ops when the marker already matches. This mirrors setup.ps1's stale-venv pre-check. Tests: 3 new parity assertions. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * test: expect first-update reinstall for a no-marker custom index pin Follow-up to d671d8fb2: _ensure_verbatim_torch_index now applies an explicit unknown-family URL pin verbatim on the first update when the marker is absent (instead of no-op), so the old test_verbatim_custom_url_no_marker_is_noop assertion was stale. Rewritten as test_verbatim_custom_url_no_marker_reinstalls_once: asserts the one verbatim reinstall from the pinned URL, that the marker is written, and that a second call with the pin still set is idempotent (no reinstall loop). * install: gate the pinned update pass on the marker and record a pin baseline Round 8, two follow-ups to the round-6 first-update pin fix: 1. setup.sh forced the full dependency pass on EVERY `studio update` while a torch-index pin stayed exported, even after the marker already recorded the same pin, turning quick updates into the expensive pass every time. It now probes install_python_stack.py --torch-pin-needs-apply (which reuses the exact marker normalization) and forces the pass only when the pin is not yet applied (marker absent or different); an already-applied persistent pin keeps the fast path. A probe error fails safe toward running the pass. setup.ps1 gets the same probe in its fast path for parity. 2. A known-family full-URL pin on a venv predating the marker (e.g. an installed cu128 build and UNSLOTH_TORCH_INDEX_URL pointing at a same-family mirror) left the marker absent forever: the _ensure_* helpers deliberately do not force a multi-GB reinstall of identical-family wheels on an old venv, so nothing recorded the pin and every update re-entered the pass. _record_torch_index_pin_baseline now records the resolved pin as a baseline after the ensure sequence when the family already matches and no marker exists, so the pin is tracked (a later genuine change is detected and applied) and the update loop is broken, without the redundant reinstall. Tests: 3 new baseline unit tests, 4 new parity assertions, and the CLI probe. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * setup.sh: keep the pin probe's exit 1 from killing the update under set -e The --torch-pin-needs-apply probe deliberately exits 1 for the common steady-state answer (pin already recorded, keep the fast path), but it ran as a bare command under set -euo pipefail, so the whole studio update aborted before the exit code was even captured. Absorb the status with || _PIN_NEEDS_APPLY=$? and pre-seed 0 so all three outcomes route as documented: 0 runs the pass, 1 keeps the fast path, anything else fails safe into the pass. Parity test asserts the guard. * install: strip pin credentials, disable uv config discovery, bound verbatim installs Four verified fix groups from a 12-reviewer audit of the torch-index override feature, each reproduced before fixing: 1. Credential persistence: all four marker writers stored the raw pin URL, so an authenticated pin (https://user:token@mirror/simple) persisted its credentials in .unsloth-torch-index (mode 0644 under a default POSIX umask) and install_python_stack.py printed pin URLs verbatim in repair messages. Userinfo is now stripped before persisting and in every log/substep that interpolates a pin, via lockstep helpers (_strip_index_url_credentials in install.sh / install_python_stack.py, Remove-IndexUrlCredentials in install.ps1 / setup.ps1). The three normalizers strip too, so an OLD marker that already carries credentials still compares equal to the same pin: no reinstall loop on upgrade. Query strings deliberately stay in the marker; two indexes distinguished only by query must not compare equal. 2. uv configuration discovery beat the explicit pin: with a discovered uv.toml declaring torch-backend = "cpu" or a [[index]] entry, uv 0.10.12 resolves torch 2.13.0+cpu against an explicit --index-url/.../cu126 pin; UV_NO_CONFIG=1 restores +cu126 (reproduced both ways). The pinned-install scrub in all four installers now sets UV_NO_CONFIG=1 and drops UV_CONFIG_FILE. 3. The verbatim custom-index update path installed a bare, unconstrained torch trio while fresh installs from the same unknown-leaf pin apply the supported range; _ensure_verbatim_torch_index now installs the bounded trio spec, closing the fresh-vs-update asymmetry. 4. Query-bearing pins (.../cu128?token=x) classified by raw leaf split and force-reinstalled on every update (the installed cu128 never equals cu128?token=x). Query/fragment are now stripped before leaf classification in all four implementations; the marker comparison keeps the query per (1). Rejected after verification (no change): the pin-baseline record cannot produce a wrong later decision (every pin change still mismatches and reinstalls from the new pin); the venv temp-file symlink scenarios require an attacker who already owns the environment; pathological inputs like " / cu128 / " have no realistic caller and fail loudly. Parity, stack, rocm-support, marker (sh + ps1), pin-stale, index-url and flavor suites all pass (455 python + full shell/ps1 batteries). * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: harden custom-pin repair against clobber, broken torch, and pip config Four follow-ups to the pinned-index audit fixes: 1. setup.ps1 routed an unknown-leaf custom pin through the CUDA branch with a bare torch trio while install.ps1 (fresh) and the Python verbatim path bound the supported range; the pinned unknown-leaf route now applies the same torch>=2.4,<2.11.0 bound. Known cu* leaves and unpinned runs are unchanged. 2. The final torch safety pass could not repair a clobbered unknown-family pin: intermediate dependency steps can pull torch from PyPI (the pass exists for exactly that reason), but the verbatim helper short-circuited on marker==pin and no flavor tag exists to probe. The helper now keeps a per-run snapshot of the installed trio (taken after a verbatim reinstall or on the first matching-marker pass) and reinstalls from the pin when the final pass sees the trio drifted. Probe failure skips the comparison; a reinstall refreshes the snapshot, so no loop. 3. _record_torch_index_pin_baseline could freeze a known-family pin as applied on a venv whose torch is missing or broken (every family helper returns without reinstalling when its probe fails), making --torch-pin-needs-apply report done forever. The baseline now probes the installed flavor and records only on a match: a cuXXX pin requires the matching +cuXXX tag, cpu requires a cpu build, rocm/gfx requires hip; probe failure records nothing. 4. The pinned pip fallback stripped PIP_* env vars but user/site pip config files still applied (a configured global.extra-index-url can satisfy torch off the pin). PIP_CONFIG_FILE is now pointed at the null device for pinned commands (pip loads no config files then), in _install_env_for_cmd and setup.ps1's Fast-Install pinned scrub. install.sh / install.ps1 have no pip fallback (uv-only), verified. Tests: 7 new rocm_support tests (snapshot reset fixture), 1 stack test, 2 parity tests. Full battery green (464 python, sh and ps1 suites). * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: complete the pin-repair coverage across the fast path and platforms Three cross-platform follow-ups to the round-2 pin-repair fixes: 1. The --torch-pin-needs-apply probe only compared marker==pin, so a torch trio clobbered to the wrong family (a cpu wheel replacing cu128 via a later pip install) with a still-matching marker reported "already applied" and the _ensure_{cuda,rocm,cpu} repair never ran on the Linux fast path. The probe is now a testable _torch_pin_needs_apply() that also checks the installed flavor against a known-family pin (via a shared _torch_flavor_matches_pin() helper, so the baseline and the probe cannot drift). An unknown-family pin has no flavor to validate and a failed probe cannot prove drift, so both keep the fast path. 2. macOS ARM (real CPU/MPS torch, not NO_TORCH) never applied an unknown- family custom pin on update: both the verbatim path and the baseline returned on IS_MACOS while fresh install.sh honors the pin, so the marker was never written and setup.sh forced the dependency pass on every update forever. The guards are now IS_MAC_INTEL (Intel mac is already NO_TORCH), and the final pass applies the pin on macOS ARM. 3. The round-2 final verbatim repair sat in the step-13 sequence guarded not IS_WINDOWS, so on Windows a dependency step that clobbered torch after the pin was applied was masked by the matching marker (setup.ps1 does not re-validate the main venv's torch after calling this script -- verified). Step 13 now runs the verbatim snapshot-drift repair on Windows and macOS ARM too; the Linux-oriented cuda/rocm/cpu family helpers stay Linux-only. Tests: 13 new rocm_support cases (flavor drift, macOS ARM, Windows repair), parity updates. Full battery green (475 python, sh and ps1 suites). * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: strip query tokens from the marker and tighten the pin-drift probe Four follow-ups to the round-3 pin-repair fixes: 1. The credential stripper feeding the torch-index marker and the logged repair messages dropped only user:pass@ userinfo, so a private feed that carries its auth token in the query string (.../simple?token=SECRET) persisted the token in the world-readable marker (mode 0644 under a default umask) and printed it in substep output. All four strippers (install.sh, install.ps1, studio/setup.ps1, install_python_stack.py) now drop the query and fragment before building the sanitized URL. A query is not part of a PEP 503 index's identity, so this also stops a rotated token from spuriously mismatching the marker and forcing a needless reinstall. 2. The --torch-pin-needs-apply fast-path probe accepted an untagged CUDA build (no +cuXXX local tag) under a specific cuXXX pin, but _ensure_cuda_torch reinstalls exactly that build to enforce the pin. The probe was more lenient than the repair, so the repair pass was skipped on the fast path. _torch_flavor_matches_pin now reports a mismatch for an untagged build under a cuXXX pin, forcing the pass. 3. The probe's ROCm branch accepted any HIP build for a rocm/gfx pin, while _ensure_rocm_torch decides a reinstall with the per-arch _rocm_pin_family_mismatch predicate (a generic +rocm7.2 wheel under a per-arch gfx pin, or a wrong ROCm version, is a mismatch). The probe now reuses that predicate, so it is as strict as the repair. This needs the installed torch version, so _probe_torch_flavor now returns (marker, cutag, version) and _torch_flavor_matches_pin takes the pin URL (extracting the leaf internally). 4. On Windows a known-family cu*/cpu pin is applied to the main venv by setup.ps1 before install_python_stack.py runs; a later dependency step can clobber it, and the GPU-aware _ensure_{cuda,cpu}_torch self-skip on Windows while the verbatim helper handles only unknown-family pins, so nothing repaired the clobber (setup.ps1 does not re-validate the main venv's torch afterward, verified). New _ensure_pinned_known_family_torch reinstalls a drifted cu*/cpu pin in the step-13 Windows/macOS-ARM branch; rocm/gfx per-arch specs stay owned by setup.ps1, unknown-family by the verbatim helper. A speculative ROCm 2.11 floor was also raised but is unreachable: the rocm7.2 index publishes no 2.x wheel below 2.11.0, and an unknown newer rocm is not floored speculatively. Tests: query/fragment strip cases in the sh + ps1 marker suites and the Python strip/marker tests; the tri-state helper and the probe/baseline harnesses moved to the (marker, cutag, version) flavor with matching versions; new probe cases (untagged CUDA, generic-rocm-under-gfx) and 8 _ensure_pinned_known_family_torch tests; a four-way query-strip parity assertion. Full battery green (1150 python, sh 26/26 marker, ps1 marker/flavor/pin-stale). * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: reinstall markerless gfx pins and cap custom-index updates at torch 2.11 Two follow-ups from the pin-marker audit: 1. A markerless venv with a gfx per-arch 2.11 pin trusted the wheel version tag, which is byte-identical (+rocm7.13.0) across gfx120X-all / gfx1151 / gfx1150. A pre-marker install holding one gfx arch's wheel that is now pinned to a DIFFERENT gfx index was therefore never switched: _rocm_pin_family_mismatch returns no-mismatch for any three-part +rocm 2.11 wheel, and _ensure_rocm_torch's absent-marker branch fell through to that heuristic. _ensure_rocm_torch now forces a one-time reinstall when the marker is absent AND the pin leaf is a 2.11 gfx per-arch index; the reinstall writes the marker, so the next update compares exactly and does not loop (the correctly-pinned no-reinstall guarantee then comes from the exact marker compare, not the ambiguous tag). Non-gfx-2.11 pins (rocmX.Y, non-2.11 gfx) stay on the tag heuristic -- their tags are distinguishable. 2. The verbatim custom-index update path used _CUDA_TORCH_PKG_SPEC (torch <2.12.0) while a FRESH install of the same unknown leaf caps torch at <2.11.0 (install.sh's default TORCH_CONSTRAINT, and setup.ps1's custom-pin branch), so a private /simple mirror publishing torch 2.11 could upgrade a `studio update` to a state the fresh installer never produces. Added _CUSTOM_INDEX_TORCH_PKG_SPEC (torch>=2.4,<2.11.0), used only by the verbatim path; companions stay pinned for the same exclusive --index-url ABI reason as _CUDA_TORCH_PKG_SPEC (a bare name could pull a torch-2.12-built torchvision). _CUDA_TORCH_PKG_SPEC is unchanged (known-family cu/cpu repair correctly tracks install.sh's widened cu ceiling). Tests: 2 new markerless-gfx cases (one-time reinstall + marker write + no-loop second run, and the rocmX.Y absent-marker no-op), the pre-existing markerless gfx no-reinstall test flipped to assert the one-time reinstall (it had encoded the old tag-trusting behavior), and the custom-index bound assertions. 488 passed. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: a matching marker must not mask a broken, clobbered, or misclassified torch Four round-6 follow-ups, all closing cases where a matching torch-index marker wrongly vouched for a torch that is not actually the pinned one: 1. _is_cuda_family_leaf matched cu+digits by PREFIX (^cu[0-9]), so a custom mirror leaf like cu128-private classified as CUDA family; the flavor check then compared the installed cu128 tag to the whole leaf cu128-private and forced a reinstall on EVERY update (never converging). The cu family is now matched EXACTLY (re.fullmatch cu[0-9]+), so a cu-suffixed custom leaf routes through the verbatim/unknown path with a stable marker. Mirrored in install.sh (_normalize_family_leaf: strip cu, require an all-digit remainder) and setup.ps1 / install.ps1 (^cu[0-9]+$). 2. _torch_pin_needs_apply returned False on a failed torch probe (missing or unimportable) under a matching marker, so setup.sh kept the fast path and a broken torch was never repaired. A failed probe now forces the pass: the marker cannot vouch for a torch that does not import, forcing is idempotent, and once torch imports again the probe succeeds and the forcing stops (self-resolving). Reverses the round-4 conservative choice for this case. 3. _ensure_verbatim_torch_index snapshotted the installed trio on the first pass with a matching marker and treated an unimportable torch (snapshot None) as "no drift, skip", so a torch clobbered to a broken state before the run was masked. A None snapshot now reapplies the pin. A torch clobbered to a WORKING-but-wrong build under an unknown-family pin remains undetectable from metadata (no flavor tag; reinstalling every update would be the loop this avoids) and is documented as a known limitation. 4. The step-13 Windows final repair reran only the verbatim (unknown-family) and known-family cu*/cpu paths, so a clobbered explicit rocm/gfx pin (the wheel setup.ps1 installed from AMD's per-arch index) was left in place. The branch now also runs _ensure_rocm_torch on Windows for an explicit rocm/gfx pin; it has a Windows path and no-ops when torch already links HIP, so it only reinstalls a genuinely clobbered ROCm venv (loop-safe). Tests: the round-4 failed-probe-trusts-marker test flipped to force the pass; new cases for the cu-suffix no-loop, the broken-torch verbatim reinstall, and the Windows rocm final-repair structure; item-2 exact-cu parity assertions. 490 passed. sh/ps1 marker + flavor + pin-stale suites all green. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: repair Windows ROCm pins from the pinned URL and honor NO_TORCH Four round-7 review items, two of them regressions in the round-6 work: 1. _torch_pin_needs_apply ignored UNSLOTH_NO_TORCH. With a torch-index env var set and no marker, the failed-probe branch forced the dependency pass on every `studio update`, and the pass (which also honors NO_TORCH) never installs torch or writes a marker, so nothing could ever stop the forcing. It now returns False immediately under NO_TORCH: the pin only matters once torch is actually installed. 2. The step-13 Windows final repair (round-6) restored a clobbered explicit rocm/gfx pin by calling _ensure_rocm_torch, whose Windows path reinstalls from the arch AUTO-DETECTED via hipinfo, not from the pin. A user pinning a different gfx family or a private mirror was restored from the wrong source (and the wrong marker written), and a headless box was skipped entirely (the arch probe returns nothing). The repair now goes through _ensure_pinned_known_family_torch, which reinstalls from the PINNED url with the same per-arch floor setup.ps1 uses (2.11-line gfx leaves) or a bare trio (older arches, rocmN mirrors). It is gated on IS_WINDOWS since macOS ARM has no ROCm, and the existing flavor check keeps it loop-safe (a matching HIP wheel is left alone). 3. _ensure_verbatim_torch_index's broken-torch check (round-6) used "_installed_trio_snapshot() is None", but that helper reports a REMOVED torch as "torch==absent" (a non-None tuple) and a broken import as the stale on-disk version, so a missing or unimportable torch under a matching marker was read as "no drift" and skipped. The matching-marker path now confirms torch health with an import probe (_probe_torch_flavor): a torch that does not import reapplies the pin, while a healthy torch keeps the snapshot-based intra-run drift detection. 4. A unit test for _ensure_cpu_torch did not pin NO_TORCH False like its siblings, so a suite run with UNSLOTH_NO_TORCH=1 in the environment made the guard return early and the reinstall assertions fail spuriously. Tests: the round-6 broken-torch verbatim test re-encodes the non-None "torch==absent" snapshot case (the exact state the old "is None" check missed); new Windows-ROCm pinned-repair cases (reinstall from the pin, per-arch floor vs bare spec, matching-wheel no-op, off-Windows no-op); a NO_TORCH fast-path probe case; the parity test now asserts the Windows final branch does not auto-detect the ROCm index and that the helper reinstalls from the explicit pin. 494 passed. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: floor the rocm7.2 index in the Windows pin repair; isolate marker tests Three round-8 review items, two of them downstream of the round-7 changes: 1. _ensure_pinned_known_family_torch gave a rocm<d> index leaf a bare torch/torchvision/torchaudio trio while flooring only gfx* leaves, so a Windows venv clobbered under an explicit rocm7.2 pin could reinstall an unbounded or ABI-mismatched trio from that exclusive --index-url. It now mirrors the spec the initial ROCm paths pin: the rocm7.2 floor for 2.11-line gfx leaves and rocm<d> leaves that serve torch 2.11, the <2.11 default for older rocm versions, and a bare trio only for older gfx per-arch leaves (which publish no floor), matching _ROCM_TORCH_PKG_SPECS / _ensure_rocm_torch. 2. test_verbatim_custom_url_no_marker_reinstalls_once called _ensure_verbatim_torch_index twice; the second call now hits the matching-marker health probe, and with pip_install mocked torch never becomes importable, so in a no-torch environment _probe_torch_flavor returned None and forced another reinstall, failing the idempotence assertion. The test now pins a healthy flavor so the idempotence check is about the marker, not ambient torch. 3. The TestEnsureRocmTorchMarker fixture patched os.environ per test but not _TORCH_BACKEND, which install_python_stack.py computes once at import from UNSLOTH_TORCH_BACKEND. A runner starting with a cuda/cpu backend made _ensure_rocm_torch early-return and skip the mocked repair these tests exercise. The fixture now neutralizes _TORCH_BACKEND so the marker tests are independent of the caller's installer-pin environment. Tests: the Windows floor-spec test now asserts a rocm7.2 mirror pin uses the rocm7.2 floor (not bare), plus a new rocm7.1 case that must fall back to the <2.11 default; the marker suite passes under a hostile UNSLOTH_TORCH_BACKEND=cuda / UNSLOTH_TORCH_INDEX_URL env. 495 passed. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: apply same-flavor pin repoints, keep ROCm fallback nonfatal, bound custom companions Four round-9 review items, two of them regressions in the round-7 pin helper: 1. _ensure_pinned_known_family_torch returned as satisfied whenever the installed flavor matched the pin, so a same-flavor SOURCE change (one /cpu or /cu128 mirror to another, or a gfx1151 -> gfx120x-all per-arch switch, both carrying the same wheel tag) was never applied, while _torch_pin_needs_apply kept forcing the pass on the marker mismatch forever. It now also reinstalls when the marker records a DIFFERENT index of the same flavor, rewriting the marker so the next update matches (no loop), exactly as the Linux _ensure_{cuda,cpu}_torch helpers do. An absent marker on an already-matching venv is still left to the baseline recorder (no forced reinstall of a correct pre-marker venv). 2. That helper reinstalled a Windows ROCm pin with the FATAL pip_install, so when setup.ps1 had taken its CPU fallback (the pinned AMD index unavailable), the final repair re-hit the same missing index and aborted the whole install. The ROCm reinstall is now nonfatal (pip_install_try): on failure it leaves the CPU base in place and writes no ROCm marker, so the install completes -- matching _ensure_rocm_torch's Windows path. cu*/cpu pins stay fatal (authoritative source). 3. install.sh left torchvision/torchaudio bare for a pinned custom/unknown-leaf index (a private /simple mirror), unlike the Python update path's _CUSTOM_INDEX_TORCH_PKG_SPEC, so a mirror also exposing newer companion wheels could resolve a torch-2.12-built torchvision against the capped <2.11 torch. It now bounds the companions (torchvision>=0.19,<0.26.0 / torchaudio>=2.4,<2.11.0) for a custom leaf, gated on an empty _expected_torch_flavor_tag so known families keep their curated bare/floored companions. 4. install.sh's _expected_torch_flavor_tag matched cu[0-9]* by prefix, so a custom leaf like cu128-private classified as the cu128 family and force-reinstalled a correct +cu128 wheel on every run. It now requires exact cu+digits (routing the suffixed leaf to the custom path), matching the Python re.fullmatch(cu[0-9]+) and PowerShell, and feeding item 3's custom-leaf detection. Tests: new cases for the same-flavor marker-change reinstall, the nonfatal ROCm fallback (no marker on failure), the rocm7.2/older-rocm floor selection now split across the nonfatal path, cu-suffixed custom leaves in test_torch_flavor.sh, and the custom-leaf companion bounds in test_torch_constraint.sh. 497 python + 143 shell assertions pass; the marker suite still passes under a hostile UNSLOTH_TORCH_BACKEND=cuda env. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: bound custom-pin companions on the Windows setup path; isolate pin-probe tests Two round-10 review items: 1. setup.ps1's custom/unknown-leaf pin branch capped only torch ($cudaTorchSpec) and still asked the exclusive index for bare torchvision/torchaudio, so a private mirror that also serves newer companion wheels could install a torch<2.11 wheel alongside a torchvision>=0.26 / torchaudio>=2.11 built for a newer torch ABI, after which the marker records the pin as applied. It now bounds the whole trio (torch>=2.4,<2.11.0 / torchvision>=0.19,<0.26.0 / torchaudio>=2.4,<2.11.0) for a pinned non-cu-family leaf, matching install.sh, install.ps1's fresh pinned install, and install_python_stack.py's _CUSTOM_INDEX_TORCH_PKG_SPEC. This completes the companion-bounds fix across all three installers; known cu* leaves keep bare specs (the family index bounds them). 2. The _torch_pin_needs_apply probe tests did not pin NO_TORCH False, so a test process launched with UNSLOTH_NO_TORCH=1 short-circuited the probe (the round-7 guard) and returned False for cases that expect the pass to run. The _needs_apply helper now patches NO_TORCH (default False) around the call, and the dedicated no-torch case passes no_torch=True explicitly. Tests: the cross-platform parity test now asserts setup.ps1 bounds the full trio (not just torch) for a custom leaf; the pin-probe suite passes under a hostile UNSLOTH_NO_TORCH=1 environment. setup.ps1 parses clean; 497 python + shell suites green. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: bound custom rocm-* pins, redact diag tokens, snapshot custom pins before base update Three round-11 review items, all reproduced before fixing: 1. install.sh's custom-index companion bounds gated on _expected_torch_flavor_tag returning empty, but that helper returned "rocm" for ANY rocm* leaf, so a custom mirror whose leaf starts with rocm but is not a pip family (a private rocm-current mirror, a Radeon find-links rocm-rel-7.2.1) escaped the bounds and installed bare torchvision/torchaudio. It now digit-gates rocm to rocm[0-9]* (matching the Python _is_pip_rocm_family_leaf ^rocm\d), so those custom leaves return "" and the <2.11 companion caps apply; real rocm7.2 / gfx per-arch indexes still classify as rocm. 2. _tauri_torch_index_family classified by the raw last path segment, so a pinned URL carrying auth in the query (.../rocm7.2?token=SECRET) had the token echoed verbatim into the emitted [TAURI:DIAG] line. It now strips query/fragment before classifying (mirroring the marker/log credential stripping), so no token reaches the diagnostic output; as a side effect .../cu128?token=x now classifies as cu128 instead of auto. 3. On studio update, the core package step (a newer unsloth can require a torch the custom pin does not satisfy, pulling a default PyPI trio) runs BEFORE the step-2b verbatim check, which then recorded the already-clobbered trio as the baseline for a matching marker and left the pin unapplied. A new _capture_verbatim_baseline() records the pre-clobber trio before the core step, so the verbatim pass detects the drift and reapplies the pin. Captures only for a matching custom pin with importable torch; a mismatched/absent marker or broken torch is left to _ensure_verbatim_torch_index. Tests: _expected_torch_flavor_tag rocm-current / rocm-rel cases; _tauri_torch_index_family token/fragment redaction with a no-leak regression guard; _capture_verbatim_baseline record/skip cases plus an end-to-end clobber-detection scenario; a structural guard that the capture runs before the core step. 501 python + shell suites pass; install.sh bash -n clean, shellcheck unchanged from base. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: match rocm family leaves exactly, enforce the rocm7.2 torch line, repair a broken pinned torch A pinned index is a pip ROCm --index-url family only when its leaf is an exact rocm<digits> / rocm<digits>.<digits> (rocm7.2) or a gfx* per-arch leaf. The prior ^rocm[0-9] prefix match also caught suffixed private-mirror leaves (rocm7.2-private, rocm7-current), routing them through the ROCm/companion-family path instead of the verbatim pin: the companion bounds were skipped and, on a pre-marker venv with a compatible +rocm wheel, the pin was never applied. Match the family exactly through one shared helper at every site: - install_python_stack.py: _is_pip_rocm_family_leaf (re.fullmatch), plus the two other loose gates it feeds (_normalize_family_leaf, _torch_flavor_matches_pin). - install.sh: a new _is_pip_rocm_family_leaf routes _expected_torch_flavor_tag, _torch_index_repairable, _normalize_family_leaf and the ROCm side-effect gate. - setup.ps1: a new Test-PipRocmFamilyLeaf routes Get-NormalizedFamilyLeaf and both pinned reroutes; install.ps1 anchors its reroute regex. _rocm_pin_family_mismatch (and its setup.ps1 mirror Get-RocmPinStaleTags) compared only the ROCm version, so a +rocm7.2 wheel whose torch release drifted off the 2.11 line (2.12/2.13 from an out-of-band upgrade or a custom rocm7.2 mirror) satisfied the family check while violating _ROCM_TORCH_PKG_SPECS['rocm7.2'] (torch>=2.11,<2.12). Flag it stale so the repair reinstalls to floor; >=2.11 alone is not enough, so the release is compared exactly against the 2.11 line for a KNOWN-2.11 rocm pin. _ensure_pinned_known_family_torch returned on a failed import probe, but _torch_pin_needs_apply forces the dependency pass on that same failed probe: a broken torch under a known-family pin was left in place and the pass was forced on every update. Treat an unimportable torch as drift and reinstall the pinned trio (the spec and marker derive from the pinned leaf, not the absent flavor); once it lands the probe succeeds and the fast path returns. Tests: exact-match cases across test_torch_flavor.sh, test_rocm_support.py, test_cross_platform_parity.py and the two .ps1 helper suites; the rocm7.2 release-line and broken-probe-reinstall cases; extraction lists updated for the new helpers. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: anchor the PS pinned-ROCm floor gate and bound install.ps1 custom-pin companions Round 12 made every family CLASSIFIER exact, but the Windows install-flow floor gate reads $_pinRocm211 directly from the raw pinned leaf with an unanchored -match '^rocm(\d+)\.(\d+)' BEFORE any exact classification runs. A suffixed custom leaf (rocm7.2-private) matches that rocm7.2 prefix, so it takes the 2.11-floor branch and is force-routed through the ROCm install path before the exact-match elseif can send it to the verbatim install. Anchor the match ($) in both install.ps1 and setup.ps1 so only an exact rocmX.Y leaf is floored; a suffixed or newer-suffix leaf falls through to the verbatim path. The Python floor selection is already exact (dict lookups gated on _is_pip_rocm_family_leaf), so only the two PS scripts needed this. install.ps1's custom (non-cu-family) pinned-torch install bounded torch>=2.4,<2.11.0 but left torchvision/torchaudio bare, so a private mirror serving newer companions could pull a wheel built for a newer torch ABI while the marker records the pin as applied. Bound both companions (torchvision>=0.19,<0.26.0 / torchaudio>=2.4,<2.11.0) when the leaf is not a cu<digits> family index (a cu index bounds its own resolution), matching setup.ps1's Test-CudaFamilyLeaf gate and _CUSTOM_INDEX_TORCH_PKG_SPEC. Tests: parity guards for the anchored floor gate in both PS scripts and for install.ps1's bounded custom-pin companions. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * install: tighten comments in the torch-index-override paths Collapse the verbose comment and docstring blocks added across the installer scripts and their tests to fewer, clearer lines without changing behaviour. Remove a duplicated CUDA-spec comment block. Comments/docstrings only; no code changes (AST-verified). * install: repair a broken pinned torch on Linux, strip trailing slash in tauri family, count the final step _ensure_cuda_torch / _ensure_cpu_torch returned on a failed import probe (torch present but unimportable). With an explicit CUDA/CPU pin, _torch_pin_needs_apply forces the dependency pass on that same failed probe, and the base package update does not force-reinstall an already-installed torch distribution, so the broken torch was left in place and the pass reran every update without repairing it. Treat a failed probe under a pin as drift and reinstall from the pinned index (the reinstall rewrites the marker and the next probe imports, so no loop). This is the Linux counterpart of the known-family repa…
… chat image preview fix (unslothai#7029) * Studio: Data settings tab, uploaded files manager, quant pinning, image preview fix Settings - New Data tab in the settings sidebar, under Connections. Chat data management (archived chats, confirm before deleting, exports, import, clear all) moved there from the Chat tab. - New Archive all chats action with confirmation. Archives every chat in Recents and Projects; compare pairs count as one chat. - New Uploaded files manager listing RAG documents (chats, projects, knowledge bases) and chat message attachments with location, size and date. Files can be opened in a new tab or deleted. Deleting a chat attachment keeps the message text. Backend - GET /api/rag/documents lists all uploaded RAG documents with file size plus KB and project names. - GET /api/chat/attachments lists chat message attachments; per attachment file and delete endpoints included. Model selector - Downloaded GGUF quants can be pinned from the quant row (next to the settings and delete actions). Pinned quants show at the top of On Device under a Pinned heading as model name plus a grey quant chip and load directly with one click. Non GGUF cached repos pin as a whole. - Toned down the green of the downloaded label. Fix - Clicking an image attachment in chat now opens the preview overlay. The tooltip trigger wrapper called preventDefault before composed handlers ran, which made Radix DialogTrigger skip opening. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: image previews and file type chips in uploaded files list Image attachments now show a small thumbnail (lazy loaded from the stored bytes, object URL revoked on unmount) and every row shows a grey uppercase type chip derived from the extension or content type. Non image rows keep a file icon. Name cell floors its width and clips overflow so narrow dialogs stay aligned. * Harden attachment serving, add tests, and polish pinned rows and previews - Strict base64 decoding for attachment files: corrupt payloads now return 422 instead of silently serving empty or garbled bytes; whitespace, missing padding, the URL-safe alphabet, and RFC 2397 percent-encoded data URLs are all handled - New backend test suite covering attachment listing, size accounting, malformed rows, deletion semantics, and every file-serving edge case - Pinned quant rows show a Loaded tag when that exact quant is active, and reveal unpin, settings, and delete actions on hover - Uploaded files dialog is wider and chat locations link straight to the thread the attachment belongs to - Chat image preview is now a chrome-free lightbox: dimmed backdrop, rounded image, corner close button, click outside to dismiss - File opens go through a synchronous window.open so Safari and Firefox popup blockers do not eat them * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Uploaded files: click a file to jump to its chat, square thumbs, new Data icon - Clicking a file row (thumbnail or name) now goes straight to the chat it belongs to; files without a chat open directly as before - File thumbnails pin a small 7px radius: the theme scales rounded-md up to a near circle at this size - Settings Data tab now uses the database-setting icon * Uploaded files is now a Data tab subpage instead of a popup - Manage swaps the tab body for an inline Uploaded files page with a back header, matching the rest of settings navigation - Size column header and values are left aligned like the other columns - Column widths tightened so the table fits the settings panel * Lightbox polish and Data tab row order - Image preview close button is transparent until hovered - Preview image no longer rounds its corners - Import chats now sits below Clear all chats in the Data tab * Data tab: export chats as fine-tuning data and open them in Recipes - New Fine-tuning section in Settings > Data converts every chat into a JSONL dataset in the OpenAI messages format, one conversation per line with string-only system/user/assistant turns - The Train tab detects this file as chatml natively: no column mapping and no standardization pass, and it works with train on completions since every assistant turn sits behind the chat template response marker - Consecutive same-role turns merge, trailing turns without an assistant reply drop, and reasoning, tool calls, and images are excluded so chat templates format the data cleanly - Open in Recipes stages the JSONL as a local seed upload, creates a new Data Recipe with the seed block preconfigured, and jumps to the editor * Data tab: load chats straight into the Train tab, row moved to the top - New Load in Train tab button uploads the fine-tuning JSONL through the training dataset endpoint, selects it in the training config store, and opens the Train tab with the dataset loaded and format-checked - Use chats as training data now sits at the very top of the Data tab - The Chats subheading is gone; chat rows flow directly under it * Address review findings on the uploads manager and quant pins - Deleting the last attachment stores '[]' instead of NULL: a NULL reads back as a missing field and triggers the legacy IndexedDB backfill, which resurrected the deleted attachment on the next chat load - The attachment file endpoint now serves audio: adapter parts store {data, format} raw base64 and compare chats store a bare base64 string; media type comes from the attachment contentType or the format - Compare-chat uploads live in message content parts, not attachments; the uploads list now includes those blobs via synthetic content-part ids that the same get and delete routes resolve - Deleting a quant from the expanded repo row also unpins it so a pinned row cannot try to load a file that no longer exists - Thumbnails in the uploads list fetch their blob only once the row is visible, so a long screenshot history does not download everything - Nine new backend tests cover audio serving, content-part listing, serving, deletion, and the empty-list delete behavior * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Data tab: single action dropdown with format choices for chat training data - The three fine-tune buttons collapse into one dropdown plus a run button; pick Load in Train tab, Open in Recipes, or Export JSONL, then click the arrow to run it - The dropdown's Format section adds ShareGPT and Alpaca alongside the default OpenAI messages format, ticked like a checklist; all three shapes are auto-detected by the Train tab's format check - Alpaca is single-turn, so each user to assistant pair becomes its own record with the system prompt and earlier turns carried in the input column - Shorter description on the training data row - Uploaded files rows show the size under the file name instead of a separate column, matching the tighter layout * Polish the training data action control - Run button is a true circle (icon-sm plus rounded-full) with a heavier arrow stroke - Dropdown trigger uses the shared standard chevron and a fixed width so switching actions no longer resizes the control * Shorten the training data row description * Use the standard chevron for the run button and enlarge the ticks - Run button uses the shared standard right chevron so it matches the dropdown chevron instead of the hugeicons arrow - Dropdown ticks bumped up a size for legibility * Reword the training data row description * Shorten Data Recipes to Recipes in the training data description * List Export JSONL first and rename the default format to Chat Completions * Handle legacy string content in fine-tune exports and gate Train on chat-only hosts - messageToPlainText now accepts plain-string message content, the shape legacy and imported histories store, so those conversations export instead of being skipped as having no exchange - The Load in Train tab action is disabled on chat-only hosts the same way the sidebar gates Train; the default action falls back to Export JSONL there so the run button never uploads a dataset that /studio would immediately redirect away from * Narrow the training data action dropdown slightly * Drop the format picker from the training data dropdown Chat Completions (OpenAI messages) is the only export format we ship, so the ShareGPT and Alpaca options and the Format section are removed. The export always uses the OpenAI messages shape. * Address the second round of review findings Security - Chat attachment data URLs no longer echo their embedded media type: anything that is not a plain raster image serves as octet-stream, so imported text/html or SVG payloads cannot render under the app origin - Uploaded .html/.htm RAG documents serve as text/plain for the same reason; the preview sheet only uses the file URL for PDFs Uploads manager - Remote image URLs in imported chats are no longer listed as stored uploads (nothing to serve, and delete would strip the chat reference); the delete guard mirrors the same data:-only rule - Deleting a content-part upload refetches the list since the remaining parts re-index, keeping sibling row ids current - Deleting a project document from the Data tab invalidates the project sources cache like the sources panel does - Data-tab deletions now patch the loaded thread's in-memory copy via a small event, so a later repo sync cannot write the attachment back Fine-tune export - Branch siblings from retries stay out of the exported conversation; only the selected chain converts (full exports still keep everything) - Assistant turns before the first user turn drop, preserving leading system prompts, so no unconditioned assistant targets are emitted Four new backend tests cover the media type clamp and remote-URL rows; two existing tests updated for the clamped types * Fix uploaded file lifecycle and model state * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Make archived chats a Data settings subpage * Studio: fix attachment route tests and pinned quant edge cases - test_chat_attachments: drop asyncio.run around the synchronous /attachments routes (list/get/delete are plain def, so asyncio.run raised 'a coroutine was expected' and failed the Repo tests CI job). - test_chat_attachments: align compare-chat content-part assertions with the stable content-hash id scheme (content-part-sha256-...) instead of the removed array-index ids; resolve ids from the listing. - pickers: pass disabled={deleteDisabled} to the pinned-quant delete action so a quant cannot be deleted mid model-load, matching the expanded variant rows. - pickers: build the pinned-quant existence set from the query-unfiltered cached GGUF repos (format filter still applied) so a pinned quant stays findable when the search term matches only its quant name. * Fix Studio review regressions * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Guard fine-tune export content blocks * Add Export button for archived chats Adds an Export action to the Archived chats view in Settings > Data that downloads only the archived chats as a JSON backup (their threads, messages and projects). The button sits in the archived header row and appears only when archived chats exist. * Refactor archived export into pure, testable units Split the archived-chats export into a dependency-free filter (archived-chat-export.ts) and a shared JSON download helper (download-json.ts). Skip the download when nothing is archived so a stray call never drops an empty file. No behavior change to the button. --------- Co-authored-by: shimmyshimmer <shimmyshimmer@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Etherll <61019402+Etherll@users.noreply.github.com> Co-authored-by: Unsloth <michaelhan@Michaels-MacBook-Pro.local> Co-authored-by: Lee Jackson <130007945+Imagineer99@users.noreply.github.com>
* Studio: fix per-GPU VRAM reporting on Windows ROCm On Windows ROCm without a HIP SDK, amd-smi is disabled and the System tab fell back to torch mem_get_info, which reports free==total there (ROCm/legacy-rocm-build#1909), so used VRAM showed as 0. The perf-counter fallback also summed every adapter into a single device with only GPU 0's total, hiding the second GPU. Read per-adapter Dedicated Usage (LUID-instanced) for used and take each GPU's total from torch properties, and treat the free==total case as unknown rather than 0, so every GPU shows real usage. NVIDIA, Linux ROCm, Apple and CPU paths are unchanged. Final validation needs a real Windows AMD box. Fixes unslothai#7072 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: report unknown VRAM instead of fabricating or zeroing it Two gaps in the Windows ROCm VRAM path. When more adapters are actively using VRAM than are visible to the process (a GPU outside the visibility mask), the per-adapter attribution paired usage by size and fabricated a per-GPU value; report unknown for every device in that case rather than mis-assign. And the System API turned an unknown (None) used value into 0 with ``or 0``, then reported the full card as free, re-hiding the exact case this change surfaces; keep None so the UI shows unknown. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Render unknown VRAM as Unknown instead of zero in the System tab The backend reports null usage when it is unknown (e.g. the Windows ROCm perf counter is unavailable or localized), but the System tab coerced null to 0 and derived free from it, fabricating a 0-used/full-free total. Preserve null and render the translated Unknown for per-device used, free and utilization, and mark the aggregate VRAM tile unknown when any device is unknown. * Render unknown VRAM as Unknown in the floating monitor and the util tile The floating VRAM monitor and the aggregate utilization ring both still coerced a null usage to 0, showing a fabricated 0.00 GiB / full free / 0% on the same Windows ROCm no-counter case the resources tab already handles. Guard both on whether every device reports a finite usage and render Unknown (value and percent) instead of a concrete 0. * Attribute per-adapter VRAM usage only when capacity forces the mapping On Windows/ROCm there is no shared key between LUID performance-counter instances and torch ordinals, so usage was paired to devices purely by capacity ranking. That pairing is only trustworthy when capacity forces it (a usage larger than every smaller device can sit on one card). When a smaller-capacity device could equally hold a strictly larger usage (for example an 8 GiB card near full beside a lightly used 48 GiB card), the two values are swappable without violating any capacity, so the ranking is a guess with no key to break the tie. A wrong guess both mislabels the System tab and feeds routes/training_vram.py a wrong per-index free value, driving a wrong keep-resident decision. Report unknown for every device when the assignment is ambiguous, keeping the attribution only for the capacity-forced case. Returning None is the conservative direction: training_vram treats a missing index as zero free, so it never keeps a chat model into an OOM. Add regression tests for the not-capacity-ordered, same-capacity, single-fits-both, and capacity-forced cases. * Report unknown VRAM usage when a hidden adapter survives the noise filter When HIP_VISIBLE_DEVICES exposes a subset of the physical adapters, the LUID usage counters cover cards outside the visibility mask too. The sub-64 MiB noise filter could drop a genuinely-idle visible card's real usage while keeping a hidden larger card's high usage, which was then clamped onto the smaller visible device and reported as fully used (for example a hidden 48 GiB card at 40 GiB shown as a visible 8 GiB card fully used, with its true 10 MiB usage filtered out). That fabricated reading also feeds routes/training_vram.py a wrong per-index free value. Flag extra adapters on the raw counter count (before the noise filter, since an idle visible card can itself fall below the floor) and, when a kept usage exceeds its ranked visible capacity, report unknown rather than clamp a hidden card's usage onto a visible device. The genuinely-idle-noise and capacity-forced single-model cases are unchanged. Add a regression test for the hidden high-use-adapter case in both counter orders. * Report unknown when only a placeholder adapter counter survives the noise filter When more raw counters than visible devices are present but every counter sits below the 64 MiB noise floor (an idle real GPU alongside a Windows Basic Render Driver placeholder), the non_trivial-or-raw fallback resurrected the raw magnitude-sorted counters and could attribute the placeholder to a real GPU while dropping a real card's reading. With a single visible device the swap-ambiguity check cannot catch it (it needs at least two ranks), so the fabricated value reached the System tab and automatic GPU selection. Return unknown for every device in that case instead of falling back to raw counters. With the earlier guards this completes the invariant: a concrete per-GPU usage is emitted only when the assignment is capacity-forced, and every ambiguous, extra-adapter, placeholder-fallback, or count-mismatch path reports unknown. Add a regression test for the placeholder fallback in both counter orders and the two-idle-GPU case. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: attribute Windows/ROCm VRAM only when capacity forces a clean bijection With more raw adapter counters than visible devices, a survivor that merely fits a visible card was pinned to it by magnitude ranking, fabricating a hidden GPU's usage onto an idle visible card whose true reading was dropped by the sub-threshold noise filter (two visible 48/8 GiB cards using 40 GiB / 10 MiB beside a hidden 6 GiB adapter returned [40, 6]). Emit a concrete per-device value only when the supra-threshold counters number exactly the visible devices (every visible card has one real reading, the extras were sub-threshold placeholders) AND the ranked usage strictly exceeds every smaller visible card's capacity. When a visible card is idle (fewer supra-threshold counters than devices) a survivor could be the hidden GPU's usage, so every device reports unknown; more active counters than visible cards, the smallest card, and any merely-fitting usage stay unknown too. The reporter's loaded-card display is preserved (40 GiB / 0.5 GiB across 48/8 GiB -> [40, None]). Adds a regression test for the reported case plus an exhaustive capacity-forced/bijection matrix. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: keep the unified-memory total when Windows-ROCm used is unknown _apply_unified_memory_correction gated both the total and the used update on torch_used_gb being known, so on a unified-memory APU (Strix Halo) where torch reports used=None (the Windows-ROCm free==total sentinel) but an authoritative full-GTT total, the device kept amd-smi's small dedicated carve-out and underreported its capacity on the System tab. Adopt torch's larger total independently of used; overwrite used only when torch's is known (otherwise keep amd-smi's dedicated-usage figure) and recompute utilization against the corrected total. Adds regression tests. * Tighten comments in the ROCm/Windows VRAM reporting path * Tighten comments further in the ROCm/Windows VRAM reporting path --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <unslothshared@gmail.com>
…el Hub (unslothai#7245) * Studio: do not show Run for embedding-only non-GGUF models in the Model Hub A downloaded embedding-only repo (sentence-transformers, feature-extraction) reports canChat by safetensors format and classifies as supported, so the Model Hub showed a Run button that dead-ends at load. Keep embedding-only non-GGUF models out of the Run gate. GGUF is unaffected (llama.cpp resolves embed vs generate at load time), and these models stay trainable. * Gate embedding-only models on the pipeline tag, not just capabilities An embedding repo whose name or tags also imply code, vision, audio or reasoning (e.g. jina-embeddings-v2-base-code picks up code from its -code suffix) slipped the embedding-only Run gate and dead-ended on a chat load. Treat a feature-extraction or sentence-similarity pipeline tag as authoritative for the gate; the change only ever widens it. * studio: tighten comments in the hub embedding run guard * Tighten comments in the hub embedding run guard --------- Co-authored-by: danielhanchen <unslothshared@gmail.com>
…bly (unslothai#7236) * Studio: make Stop and stall deadlines interrupt a wedged stream portably The cancel watcher unblocks a stalled read by shutting the socket down from another thread, which works on POSIX but not reliably on native Windows, where Winsock does not dependably wake a recv() already in progress on another thread. Wrap the httpcore network stream so the reader loops each read in short slices and polls the cancel event itself. Stop and the stall deadlines now interrupt a wedged mid-stream read without any cross-thread socket teardown, and a slow but still-alive stream is never torn down. The POSIX shutdown path is preserved. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Honor the post-first-token stall timeout in the cancel-aware read httpcore snapshots request.extensions timeout read once when the body starts, so lowering it to the stall timeout after the first token never reached the socket read and a one-token-then-silent server hung for the full prefill window. Re-read the live extensions timeout per call and bound each read by it, falling back to the httpcore-passed timeout when absent so prefill and normal completion are unchanged. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: tighten comments in the llama.cpp stall timeout path * Tighten comments in the stream stall cancel path --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <unslothshared@gmail.com>
* Document local agent connections * Refresh README features and news * Tighten README feature copy * Restore selective README emphasis * Add Unsloth Start quickstart * Update * Mention * Update README.md * Reduce * Restore-inference-order * Split-agent-API-features
…ocal model selection (unslothai#7257) * unsloth start: fix Windows agent install/launch and local model selection - claude: pin availableModels to the served model in the session --settings overlay so a user's ~/.claude/settings.json allowlist no longer substitutes the org default for the local Unsloth model. The allowlist covers --model, ANTHROPIC_MODEL and the model setting, and an empty [] is ignored, so the pin lists the model explicitly. - installs: run the Windows installer under -ExecutionPolicy Bypass (process-scoped, nothing persistent) so npm's npm.ps1 and irm|iex scripts run under the default Restricted policy; on failure, hint at Set-ExecutionPolicy -Scope CurrentUser -ExecutionPolicy RemoteSigned for a hand-run retry. - PATH: resolve agents installed to ~/.local/bin (claude) and %APPDATA%\npm (npm agents) in-process, so a fresh install launches without opening a new shell and an already-installed agent is not re-prompted for install. - load message: "Loading <model> - please wait" while a model loads. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * unsloth start: resolve agent version against the launch PATH The claude/codex/opencode version probes ran shutil.which while building the command, before _launch augments PATH with the known install dirs. An agent present only in ~/.local/bin or %APPDATA%\npm was therefore missed, assumed to be a current build, and launched with flags an older build rejects (claude aborts on the unknown flags). Route the three probes through a new _which_with_install_dirs() so each resolves the same binary _launch will, restoring PATH afterward so only _launch persists the augmentation. Add regression tests for the three probes (POSIX and the Windows npm dir) and make the Windows-branch tests run on POSIX hosts (pinning Path to the native flavour so a simulated os.name does not make pathlib build WindowsPath). * unsloth start: keep os.defpath when augmenting an unset PATH _augment_path_with_install_dirs collapsed an unset PATH to just the install dirs, dropping the os.defpath fallback (/bin:/usr/bin) that shutil.which and exec*p* use when PATH is absent. A system-installed agent then looked missing and the launched child lost its normal PATH. Seed os.defpath when PATH is unset; an explicitly empty PATH is left as-is (search nothing), matching shutil.which. Add regression tests for the augment helper and the version-probe wrapper. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* Fix on-device model picker startup ordering * Fix on-device picker cached local remounts * fix(studio): stabilize on-device picker readiness * fix(studio): prevent stale picker refreshes * fix(studio): retry incomplete picker scans * fix(studio): preserve replacement picker readiness * fix(studio): preserve slow local scans --------- Co-authored-by: Long Yixing <longyixing331@gmail.com>
* Studio: validate Hugging Face tokens before use * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: keep token validation failures non-blocking * Studio: harden Hugging Face token preflight * Studio: make token validation effect lint-safe --------- 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>
…7260) * Studio (Windows): keep prompt caching on full GPU offload (unslothai#5692 follow-up) The unslothai#5692 full-offload tuning also added --no-cache-prompt, which disables in-VRAM prompt-prefix reuse. That is unrelated to the host-RAM KV checkpoints unslothai#5692 fixed (--cache-ram 0 / --ctx-checkpoints 0): a fully offloaded model keeps its KV cache in VRAM, so reusing a common prefix does not copy to system RAM and does not cause the PCI-E overhead. --no-cache-prompt only forces every request to re-prefill the whole prompt, which is small for short chats but severe for large stable system prompts reused across calls (coding agents, long multi-turn chats). Remove --no-cache-prompt; keep the checkpoint disables and the thread/OMP tuning. _prompt_cache_disabled stays False (its default), so slot save/restore is intact. Verified on a fully offloaded gemma GGUF: an identical repeated prompt reprefills 1 token instead of 2220. * Guard against re-adding --no-cache-prompt to any llama-server command Add a backend-wide test that AST-scans studio/backend and fails if --no-cache-prompt is appended/extended/+= into a command. This locks in the unslothai#7260 fix across every code path, not just load_model. Detecting the flag or honouring a user-supplied one stays allowed. * [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>
PyPI release unsloth 2026.7.4 is live; bump the pinned floor so fresh installs resolve to the new wheel.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+223
to
+224
| "Note: --password is visible in the process list and shell history; " | ||
| f"prefer {SUPPLIED_PASSWORD_ENV} or --password - (stdin).\n" |
| while True: | ||
| password = read_masked("New password: ", out) | ||
| if len(password) < MIN_PASSWORD_LENGTH: | ||
| out.write(f"Password must be at least {MIN_PASSWORD_LENGTH} characters. Try again.\n") |
Comment on lines
+1241
to
+1242
| f"Error: password must be at least {_auth_storage.MIN_PASSWORD_LENGTH} " | ||
| "characters; not starting.", |
Comment on lines
+277
to
+278
| "Note: --password is visible in the process list and shell history; " | ||
| f"prefer {SUPPLIED_PASSWORD_ENV} or --password - (stdin).\n" |
![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 rebrands 'Studio' to 'Unsloth' across the codebase, introduces persistent session management for stdio MCP servers, adds a standalone Vulkan VRAM probe, and implements presence penalty support for safetensors and MLX inference. It also enhances the training backend with an MLX trainer adapter for Apple Silicon and improves RAG embedding model security. The review identified two critical runtime bugs: one in embeddings.py where dtype_kwargs is passed a string instead of a torch.dtype object, and another in mlx_inference.py where a Python list is incorrectly treated as an array, which will raise an AttributeError.
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.
| name, device = device, model_kwargs = {"torch_dtype": "float16"} | ||
| ) | ||
| _guard_model_security(name) | ||
| _model = SentenceTransformer(name, device = device, model_kwargs = dtype_kwargs("float16")) |
There was a problem hiding this comment.
The dtype_kwargs helper expects a torch.dtype object (such as torch.float16), but is being called with the string "float16". This will fail at runtime.
Suggested change
| _model = SentenceTransformer(name, device = device, model_kwargs = dtype_kwargs("float16")) | |
| _model = SentenceTransformer(name, device = device, model_kwargs = dtype_kwargs(torch.float16)) |
Comment on lines
+264
to
+270
| if state["prompt_len"] is None: | ||
| # First call is prompt-only; latch its length. | ||
| state["prompt_len"] = int(tokens.shape[0]) | ||
| return logits | ||
| generated = tokens[state["prompt_len"] :] | ||
| if generated.size == 0: | ||
| return logits |
There was a problem hiding this comment.
In mlx_lm's generate_step, tokens is passed as a Python list of integers, so calling tokens.shape[0] and generated.size will raise AttributeError. Using len(tokens) and converting generated to an MLX array is safer and more robust.
Suggested change
| if state["prompt_len"] is None: | |
| # First call is prompt-only; latch its length. | |
| state["prompt_len"] = int(tokens.shape[0]) | |
| return logits | |
| generated = tokens[state["prompt_len"] :] | |
| if generated.size == 0: | |
| return logits | |
| if state["prompt_len"] is None: | |
| # First call is prompt-only; latch its length. | |
| state["prompt_len"] = len(tokens) | |
| return logits | |
| import mlx.core as mx | |
| generated = mx.array(tokens[state["prompt_len"] :]) | |
| if generated.size == 0: | |
| return logits |
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
20 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#7273. Do not merge; closed after CI.