[staging CI] unslothai/unsloth#7140#366
Closed
danielhanchen wants to merge 169 commits into
Closed
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 unslothai#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.
) * Studio: expose Windows drive roots in the folder browser The model-selection folder browser bounds navigation to the roots returned by _build_browse_allowlist(), which exposed Linux removable-media mounts via linux_run_media_mount_roots() but had no Windows analog. As a result a user on C: could not browse to D:/E: to pick a model directory. Add windows_drive_roots(), a Windows-only companion to linux_run_media_mount_roots() that lists readable logical drive roots, and wire it into both browse-allowlist builders and their suggestion chips so other drives are both navigable and offered as quick-picks. The helper is a no-op on Linux/macOS, so existing platforms are unaffected. Closes unslothai#6368 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: cover the Windows drive-root browse wiring with an integration test Add an allowlist integration test mirroring the Linux side's test_legacy_browse_allowlist_includes_linux_run_media_mounts: it extracts _build_browse_allowlist from routes/models.py, stubs external_media so windows_drive_roots() yields a fake drive root, and asserts that root becomes browsable through the built allowlist. Proves the wiring, not just the helper. * Studio: skip inactive drives via GetLogicalDrives before probing Resolve active logical drives from GetLogicalDrives() before probing each letter with os.path.isdir. Probing a drive letter mapped to a disconnected network share can otherwise block the async backend for tens of seconds per letter. The call degrades gracefully (falls back to probing all letters) when ctypes/windll is unavailable, so behavior is unchanged on Linux/macOS. Tests override the bitmask source to stay deterministic on real Windows hosts. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: allow browsing descendants of a drive-root allowlist entry routes/models.py _is_path_inside_allowlist() checked descendants with startswith(root_real + os.sep). A drive root ("D:\") already ends in a separator, so the prefix became "D:\\" and a child like "D:\models" was rejected with 403 after the browser opened the drive root. Only append a separator when the root does not already end in one. folder_browser.py already uses commonpath and was unaffected. Adds a regression test covering the separator-terminated-root descendant case. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: enforce the system-directory denylist during folder browsing Exposing whole Windows drive roots (and any legacy-registered filesystem root) widened the browse allowlist above system directories, but the browse resolvers only re-applied the credential/config denylist, not the _denied_path_prefixes() system-dir denylist that scan-folder registration enforces. That let browse-folders enumerate C:\Windows, C:\Program Files, /etc and /proc. - Add is_denied_system_path() to both storage modules and enforce it in both browse resolvers (legacy routes/models.py and hub folder_browser.py), on each resolved child and on the final target, keeping the /run/media carve-out. - Rework the legacy _is_path_inside_allowlist to use splitdrive + commonpath so a Windows drive root authorizes its descendants while a bare POSIX / does not, and to compare case-insensitively like the hub browser. - Reject the filesystem root in the legacy add_scan_folder, matching the hub. - Hide denied system dirs from browse listings and suggestion chips. - Add tests/test_browse_denylist.py and update the external-media path tests. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: make browse-denylist tests OS-portable The browse-time denylist tests used real /etc and tmp_path locations; on macOS tmp lives under the (legitimately denied) /private/var and /etc resolves to /private/etc, so three tests failed there. Pin the platform / use a tmp-based denied prefix so they assert the same behavior on Linux, macOS and Windows. * Studio: apply the bare POSIX-root guard to the hub folder browser too The _is_path_inside_allowlist guard that stops a legacy-registered '/' scan folder from authorizing every absolute path lived only in the legacy browser. The hub browser used commonpath without it, so a stale '/' row let it descend into /var, /root, /home -- which the system-directory denylist (/proc /sys /dev /etc /boot /run) does not cover, while the legacy browser blocked them. Mirror the legacy guard so both browsers treat '/' identically. Also resolve each directory entry before the denylist check in both listing loops, so a symlink or junction pointing into a denied dir is hidden instead of rendered as a row that 403s on descent. Adds legacy-vs-hub parity tests. * Studio: bound Windows drive probing so a disconnected mapping can't stall the browser GetLogicalDrives includes mapped network drives, so a disconnected but still mapped drive (e.g. Z: -> \\nas\share) stays set in the bitmask and reaches os.path.isdir, which can block for tens of seconds while Windows tries to reconnect. Because windows_drive_roots() runs synchronously while building both folder-browser responses, one stale mapping stalled every browse request. Probe each surviving drive in a daemon thread bounded by a short timeout and skip it if it does not answer in time, so a hung mapping is dropped instead of blocking the caller. Connected drives (local or network) still respond well within the timeout, so drive discovery is unchanged. Corrects the GetLogicalDrives docstring, which claimed the bitmask alone prevented the stall. * Studio: probe drive/media roots once per browse request, not twice Both folder browsers called windows_drive_roots() (and linux_run_media_mount_roots()) twice per browse request: once to seed the allowlist in _build_browse_allowlist() and again to build the suggestion chips. With the bounded drive probe, a disconnected mapped network drive then paid the timeout twice per folder click. Probe both once in the request handler and pass the results into _build_browse_allowlist(), reusing them for the chips, in both the legacy and hub browsers. Adds a test asserting the roots are reused, not re-probed. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: run the legacy browse endpoint in the threadpool, fix its stale test Two follow-ups from review of the drive-probe changes: - browse_folders was 'async def' but does only blocking filesystem I/O (the timeout-bounded drive probe, iterdir, realpath). On the event loop a disconnected mapped drive waiting out its probe timeout stalled every other request. Declare it sync 'def' so FastAPI runs it in the threadpool, matching the hub browse endpoint. No await was used in the body. - test_browse_folders_hides_sensitive_dirs monkeypatched _build_browse_allowlist with a zero-arg lambda; the once-per-request refactor now calls it with (media_roots, drive_roots), so the lambda raised TypeError. Accept and ignore the args. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: probe Windows drive roots concurrently so multiple dead mappings don't stack timeouts windows_drive_roots() probed each candidate serially, so N disconnected-but-mapped network drives each paid the full per-drive timeout in turn (e.g. four stale mappings added ~8s to every folder-browser request). Collect the candidate roots first, then probe them all at once under a single overall deadline, so the added delay stays at ~one timeout regardless of how many drives are disconnected. _readable_dir_within stays as a thin single-path wrapper for its existing callers/tests. * Studio: tighten comments in the folder-browser drive-root changes Condense the comments and docstrings added by the Windows drive-root and system-directory denylist work to be shorter and clearer while keeping the security and correctness rationale intact. Comment and docstring text only; no code changes. * Studio: iterate the input, not the results dict, when collecting readable drive probes _readable_dirs_within returned {path for path, ok in results.items()...}, but a probe thread that exceeded the join deadline is still alive and can insert its key into results during that iteration, raising 'dictionary changed size during iteration' -- reachable exactly in the disconnected-mapped-drive case the probe exists for. Iterate the fixed input list and read results.get(path) (an atomic read) instead. * Studio: keep the browse-route containment tests denylist-inert so they pass on macOS test_browse_folders_route.py exercises allowlist containment and the file-vs-directory guard, not the system-directory denylist. On macOS pytest tmp_path resolves under /private/var, a denied prefix, so _resolve_browse_target 403s the fixture dirs before the containment logic runs (4 failures). Add an autouse fixture that makes is_denied_system_path inert in this file; the denylist keeps its own coverage in test_browse_denylist.py. * Studio: keep the hub browse tests denylist-inert so they pass on macOS * Studio: register a UNC share root; only reject local filesystem roots * Studio: reject device drive roots and browse a registered UNC share root * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: treat device-namespace volume GUID roots as local filesystem roots --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <danielhanchen@gmail.com>
…unslothai#7137) * Studio CI: make tool-calling SSE probes resilient to transport stalls * Studio CI: bound tool-probe SSE stalls to the job timeout and stop accepting partial tool events * Studio CI: surface HTTP errors from the seed loop and bound the Mac best-effort probes * Studio CI: keep completed tool results on a stall and cap seed reads by the remaining budget
…nslothai#7099) * Studio: force-terminate a stuck training stop after a grace period A Stop-with-save only signals the worker and waits for it to save and exit; force_terminate() was reachable only from the /reset cancel path. On Windows + ROCm the worker saves the adapter fine but then wedges in post-save GPU/HIP teardown and never exits, so the run stays in "Stopping..." forever, is_training stays true, and /reset returns 409. Add a stop watchdog: when a stop is requested, a daemon escalates to force_terminate() a short grace after the worker's "complete" (save done), or after an absolute cap covering a hang during save. After escalation the parent state is finalized (is_training=False, "Training stopped.") even if the OS never reaps the wedged worker, so the UI leaves "Stopping..." and a new run can start. No behavior change on a clean quick exit. Grace and timeout are configurable via UNSLOTH_STUDIO_TRAINING_STOP_GRACE_S (15) and UNSLOTH_STUDIO_TRAINING_STOP_TIMEOUT_S (120). * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: harden the training stop watchdog per review Address review feedback so a stop can never corrupt a checkpoint or leave the run stuck: - Never force-kill an in-progress save. The absolute cap is now a last-resort backstop: raise the save default to 600s and only kill past that long window; a not-yet-complete save is not treated as a hang. Cancels have nothing to save, so they keep a shorter 120s cap via UNSLOTH_STUDIO_TRAINING_CANCEL_TIMEOUT_S. The save vs cancel path is now explicit and the backstop logs a clear warning. - Always finalize even if force_terminate raises on a wedged child (try/finally), so the watchdog never dies leaving the run in "Stopping...". - Preserve output_dir when the watchdog finalizes so a saved checkpoint is still recorded in run history. - Track the watched process per watchdog: a new run always gets its own watcher, and a stale watchdog on an old proc no longer suppresses it. - Terminate only the captured proc; force_terminate revalidates under the lock that it is still the current worker, so it can never kill a fresh run. - Name the watchdog thread for debuggability. * studio: tighten training-stop watchdog comments Comment-only pass: collapse the watchdog docstrings and inline notes to fewer lines while keeping the rationale. No behavior change. * Studio: make the stop watchdog safe against concurrent runs and the pump Target-scope the escalation finalize so a stale watchdog can never clobber a run that replaced its worker: capture the watched proc and job id, and no-op the finalize (handle, progress, and DB) when a new run has already taken over. Honor a later cancel by tightening an in-flight save watchdog to the shorter cancel cap. Serialize the DB helpers on the lock so the watchdog and pump can no longer double-create, double-finalize, or corrupt the metric buffer when a force-terminate hands off to a still-finalizing pump. Add regression tests: finalize no-ops when superseded, finalize runs for its own worker, a later cancel tightens the cap, finalize is single-winner under concurrency, finalize honors expected_job_id, and concurrent flushes claim each metric exactly once. * Studio: close the remaining stop-watchdog vs start/pump races Guard the escalation finalize by the watched job id in addition to the proc: start_training sets current_job_id before it installs the new _proc, so a stale watchdog entering during that startup window still sees the old dead handle and was not caught by the proc-only guard. Capture the job id when the watchdog starts and require it to still match before touching state. Snapshot the run id and final progress under the finalize lock and thread them through the flush and finish_run calls, so a new run that starts between the finalize claim and the DB writes cannot be flushed or marked stopped under the old run's finalizer. Publish _db_run_created only after create_run commits, gated by a dedicated in-progress flag, so a concurrent finalize can no longer run finish_run against a not-yet-inserted row and leave the run stuck as running. Add regression tests for the startup-window job-id guard, run-id pinned flush, snapshot-based finalize across a new run, and create-not-published-before-insert. * Studio: finalize a force-stopped run by its captured id If a new run starts in the gap after the watchdog clears _proc and marks the backend idle, current_job_id changes, so the previous expected_job_id guard made the finalize skip and left the stopped run recorded as running. Capture the run id, metrics, and final progress under the lock (where current_job_id is still the watched run) and finalize by that captured id via _finish_stopped_run: finish_run is an idempotent UPDATE and insert_metrics_batch upserts, so a concurrent pump finalize of the same run is harmless and a newly started run is never touched. Add a test that the watched run is finalized by id with its buffered metrics, and update the escalation tests to assert finalize goes through _finish_stopped_run. * Studio: keep force-stop finalization retryable and unclaimed until the row exists Only claim _run_finalized in the escalation when the DB row already exists; if an early create failed and the pump is retrying it, claiming would make the pump's later finalize no-op and strand the row as running, so leave the finalize to that create-then-finalize path. On a DB error in _finish_stopped_run (e.g. a transient SQLite lock), unclaim the finalize and requeue the drained metrics when the run is still current, so the pump or a later retry can still record the run stopped instead of leaving history with an active run and lost metrics. A superseded run's state is never touched. Add tests: no claim before the row exists, requeue+unclaim on a DB error, and a superseded run left untouched on error. * Studio: tighten stop-watchdog comments Reduce the wording of the docstrings and inline comments added by this PR without dropping any of the concurrency invariants (dual proc/job-id supersession guard, finalize-by-captured-id, publish-after-commit, snapshot-under-lock, unclaim and requeue on error). Comments and docstrings only; no code change. * Studio: record the stopped run's DB state before dropping _proc A wedged worker still reports alive, so the pump never reaches its own finalize and bails on its _proc-is-None guard once the escalation drops the handle. So the watchdog is the sole finalizer: record the terminal DB state (create the row if a start-time create failed, then finish by captured id) BEFORE dropping _proc. While the handle is held is_training_active() stays true, so no new run can start and current_job_id stays the watched run for the write; _proc is dropped last, guarded on target_proc so a run that did replace the worker keeps its handle. _finish_stopped_run retries a transient DB error a few times (the pump can no longer retry once _proc is gone) and unclaims on final failure only when the run is still current. Add tests for create-then-finalize, retry-then-unclaim, and not dropping a new run's handle. * Studio: job-guard the DB create flags against a racing new run _ensure_db_run_created publishes backend-wide _db_run_created and _db_create_in_progress flags. When the watchdog creates a missing row for an escalated stop, the killed worker lets a new /start proceed mid-create, so the stale create could publish those flags against the new current_job_id, making the new run skip inserting its own row (metric/finalize then target a missing run). Publish the flags only when the captured job id is still current; the row is still created by id, and the new run owns/creates its own. Also reset _db_create_in_progress in start_training so a stale claim can't block a new run. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* fix(studio): recover mlx vlm image prompts * fix(studio): detect serialized vlm media items * studio: recover MLX VLM prompts when model_type only lives on _config _mlx_vlm_model_config only fell back to _config when config was entirely missing, so a model that exposes a config without a model_type (while _config carries it) skipped model-aware recovery. Prefer whichever of config / _config actually has a model_type. Adds a focused test. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Daniel Han <danielhanchen@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
unslothai#7138) * fix: replace bare except clauses and remove duplicate MAX_FUSED_SIZE definition * Also catch NameError in mllama RMSNorm patch/unpatch fallbacks If mllama exists but MllamaTextRMSNorm is missing, the module-level import fails so Unsloth_MllamaTextRMSNorm/MllamaTextRMSNorm stay undefined. The patch/unpatch module imports then succeed and reference the undefined name, raising NameError. Add NameError so these fallbacks stay no-ops as before. --------- Co-authored-by: lxcxjxhx <lxcxjxhx@users.noreply.github.com> Co-authored-by: Daniel Han <23090290+danielhanchen@users.noreply.github.com>
…ents (unslothai#7131) * Studio: scope the seeded bootstrap password auto-fill to loopback clients * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: block bootstrap injection through Cloudflare tunnels * Studio: require loopback host for bootstrap injection * Studio: add regression test for unparseable Host in bootstrap loopback gate * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Scope bootstrap auto-fill to a direct-loopback client (block proxy/tunnel headers and malformed Host) * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Reject scope-id addresses in loopback check (fail closed on ::1%zone Host) * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Reject malformed bracketed Host in loopback check (e.g. [::1]evil) * [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: Daniel Han <danielhanchen@gmail.com> Co-authored-by: danielhanchen <23090290+danielhanchen@users.noreply.github.com>
…e.py (unslothai#7090) * Single-pass GGUF export for direct outtypes + parallel multi-quant save_to_gguf defaulted first_conversion to model_dtype before the block that picks the optimal base conversion, leaving that block dead since it landed (unslothai#3356). Every default export (fast_quantized -> q8_0) therefore ran two passes: convert HF -> 16-bit GGUF, then llama-quantize -> q8_0, writing a 2x-size intermediate that the cleanup step deletes again. - Route single-output exports whose type convert_hf_to_gguf.py emits directly (f32/f16/bf16/q8_0) through one conversion pass with no 16-bit intermediate. Measured on Qwen2.5-0.5B-Instruct (8-core CPU): bytes written 1525 MB -> 531 MB (2.9x less), peak extra disk 994 MB -> 0, wall time neutral on local NVMe (14.8s vs 15.4s). The dequantized q8_0 tensors are bit-identical to the two-pass output (max diff 0 over all 290 tensors, same quant-type table). On disk-capped runtimes (Kaggle 20 GB, Colab) the removed intermediate is the difference between an export that fits and one that dies - see the Kaggle error text this file already carries. imatrix runs keep the two-pass route since only llama-quantize can apply one; explicit first_conversion is still honored. - Run independent llama-quantize passes two at a time when several quant methods are requested (thread budget split between workers, outputs byte-identical, order preserved). Measured 1.38x wall-clock on q4_k_m+q5_k_m+q6_k. Sequential under UNSLOTH_ENABLE_LOGGING=1 to keep subprocess logs readable; kill switch UNSLOTH_PARALLEL_GGUF_QUANTS=0. Duplicate methods now quantize once. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Guard parallel GGUF quant on Kaggle and make multi-quant failures atomic Each llama-quantize pass loads the whole model into RAM, so running two at once on Kaggle can OOM a host that succeeded sequentially; skip the parallel path there. On a failed multi-quant export, stop launching queued passes and remove orphaned quant outputs so a failure leaves no partial GGUFs behind, keeping the 16-bit base for retry. Also accept 0/false/no/off/empty for UNSLOTH_PARALLEL_GGUF_QUANTS so a well-meant 'false' actually disables parallelism, and add tests/saving/test_gguf_single_pass_export.py to the CI saving bucket so the new tests run. * Preserve pre-existing outputs for canceled quant passes on failure The parallel cleanup unlinked every requested output name, so a failed rerun could delete a valid model.<METHOD>.gguf left by an earlier successful export for a method whose pass was canceled and never ran this session. Skip canceled futures and only remove outputs from passes that actually executed. * Gate parallel GGUF quant on available memory and preserve prior outputs Skip the two-worker path when RAM cannot hold two full-model quantizations at once (and on Colab as well as Kaggle), so a multi-quant export that fit sequentially no longer OOMs. On failure, remove only outputs this run newly created, tracked against a pre-launch snapshot, so a rerun into an existing _gguf directory never deletes a valid artifact from an earlier export. --------- Co-authored-by: djs <dschroers2@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <michaelhan2050@gmail.com>
…ures (unslothai#7056) * Studio: offer the latest transformers release for brand-new architectures When a model's config.json model_type is absent from every installed transformers overlay (base 4.57.x and the .venv_t5_530/550/510 sidecars), Studio now checks, unauthenticated and cached, whether the newest transformers ships it: - utils/transformers_latest.py fetches the latest release version from https://pypi.org/pypi/transformers/json and the CONFIG_MAPPING_NAMES sources for that tag and for main from raw.githubusercontent.com (never api.github.com), parsing them with the same AST extractor the static router uses (no code execution, no trust_remote_code). Results are cached in memory and in a JSON snapshot under studio_root()/cache with a one day ttl; fetches are bounded to 5s with one retry and a failure backoff, and offline mode or the new kill switch UNSLOTH_STUDIO_NO_LATEST_TRANSFORMERS=1 short-circuits to None. - POST /api/inference/validate gains requires_transformers_upgrade plus a transformers_upgrade payload (model_type, pypi_version, supported_in_pypi, supported_in_main) so the frontend can raise the install consent dialog before /load, mirroring the existing remote-code consent flow. The check fires only when the model_type is unknown to all installed overlays and the hardcoded tier tables. - POST /api/inference/install-latest-transformers provisions a new persistent .venv_t5_latest sidecar after user consent, pinned to the exact PyPI version (re-verified server-side) with the same --target/--no-deps recipe as the fixed sidecars. A JSON pin marker inside the dir records the installed package set, so restarts revalidate it and routing resolves the new highest-ranked tier automatically. A dependency preflight (compat_plan) compares the release's requires_dist against the running env: unsatisfied tokenizers/safetensors floors are shadow-installed as exact pins into the sidecar, anything else unsatisfied blocks the install with a clear message. Routing for every already-supported model_type is unchanged: the hardcoded lists and the 530/550/510 static resolver run first, the new tier only participates once its venv exists, and the probe order gains the latest sidecar only when provisioned. Verified against live PyPI and GitHub (transformers 5.13.0: 674 model_types, 26 absent from all installed overlays, e.g. cosmos3_omni; 4 dev-only on main) and with a real sidecar install plus restart persistence. 64 new tests; the existing 200-test transformers_version suite passes unchanged. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Latest-transformers check: fetch outside the lock, serialize installs Release the module lock during the network refresh so a slow fetch cannot stall other threads in the ASGI pool; concurrent callers during a fetch get None (the graceful fallthrough) via an in-flight flag instead of stacking fetches. Serialize install_latest_transformers with an in-progress flag so concurrent consents cannot race the sidecar delete and recreate; the loser gets a structured already-in-progress refusal. * Latest-transformers check: LoRA bases, pin-gated mapping, live reverify Run the upgrade check over the [adapter, base] target set so a LoRA whose base model is a brand-new architecture surfaces the prompt (the worker activates transformers for the base, not the adapter). Gate the latest overlay's mapping lookup on a valid pin marker, matching activation and the probe order, so a partial or manual .venv_t5_latest dir cannot be routed to and then refused at activation. Re-verify the requested version against a live PyPI snapshot at install time, falling back to the cached one on fetch failure, so a release published inside the cache TTL is not silently missed. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Latest-transformers check: nested config types and latest-tier vision probe Collect every model_type in the config (top level plus each nested sub-config) and signal on the first one missing from all installed overlays, so a supported wrapper carrying a brand-new backbone still surfaces the upgrade prompt; wrappers instantiate sub-configs through CONFIG_MAPPING and would fail on the nested type. Route the vision capability subprocess through the pinned latest sidecar when the model resolves to the latest tier, so latest-only VLMs are not misclassified as text-only; every other tier keeps the 5.5 sidecar used today. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Latest tier: nested routing, vision probe after raw miss, safe upgrades Route by every model_type in the config: a nested sub-config type can raise the tier (wrappers instantiate sub-configs through CONFIG_MAPPING), so a supported wrapper with a latest-only backbone routes to latest once installed instead of staying on default. An unknown nested type never vetoes; the primary type keeps its previous semantics. The collector is shared with the upgrade checker. Vision detection: when the raw heuristics say False for a model that routes to the latest tier, run the AutoConfig subprocess under the pinned latest sidecar instead of trusting heuristics built from older transformers. Provisioning: stage-and-swap. Build the new sidecar in .venv_t5_latest.staging and swap it in only when the install and pin marker are complete, so a failed upgrade never destroys a previously working sidecar; restore the old dir if the final swap fails. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Latest-transformers checker, vision subprocess, and cache fixes Require the latest release to support every missing model_type (the primary included) before prompting; a nested-only match cannot make the model loadable, so no install is offered for it. The vision-check subprocess now unions the active sidecar's own registry mappings into the inlined parent-process detection sets, so architectures only the sidecar knows classify correctly. A successful sidecar install clears the tier probe cache, the latest tier's model_type mapping, and the vision-detection cache so the new venv takes effect without a restart. Tests for all three. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Aggregate upgrade support flags and keep install off /v1 The upgrade signal now reports supported_in_pypi only when the latest release covers every missing model_type; a mix with a main-only nested type surfaces as dev-only so no PyPI install is offered that would still fail at load. The consented install endpoint moves to studio_router so it is not reachable through the OpenAI-compatible /v1 mount. Tests for both. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Honor the latest-transformers kill switch in routing With UNSLOTH_STUDIO_NO_LATEST_TRANSFORMERS set after the sidecar was provisioned, the latest tier still joined mapping and probe routing because only the pin was checked. Both admission points now also check the kill switch, so operators can roll back a problematic sidecar without deleting files. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Repair the latest sidecar through stage-and-swap The lazy repair path installed into the live .venv_t5_latest, which _ensure_venv_dir wipes first, so a failed repair deleted the pinned sidecar and its marker. Both the consented install and the repair now share one stage-and-swap helper: the incomplete-but-pinned dir survives any failure and a later attempt can still repair it. * Tighten comments * Remove the staging dir when a latest-sidecar install fails A pip failure inside _ensure_venv_dir returns False without raising, so the except cleanup never ran and the partial .venv_t5_latest.staging leaked until a later attempt. Also note on the validate response fields that frontend consumption ships in the follow-up PR. * Add the transformers-upgrade consent dialog to the frontend When /validate reports requires_transformers_upgrade, every explicit load path (chat runtime and the compare composer) now pauses on a consent dialog modeled on the remote-code one: it names the model_type and the latest PyPI transformers version, and on Accept calls /api/inference/install-latest-transformers itself, shows an installing state, and resumes the original load automatically on success. Errors surface in the dialog with a retry; Cancel aborts the load like the trust dialog's deny path. Architectures shipped only on transformers main get a dev-only notice with no install button. Background auto-load skips upgrade-requiring candidates instead of prompting, mirroring the trust_remote_code rule. The dialog mounts once in the root layout and runs before the security dialogs, since no load can proceed without the runtime. * Route a non-installable new architecture to the custom-code consent as a last resort When the upgrade dialog has no installable PyPI release (the architecture is only on transformers main, which Studio never installs), the dialog now says so explicitly, and when the model also declares custom (auto_map) code it offers Continue with custom code: resolving the paused load into the existing trust_remote_code consent gate instead of hard-aborting. Models with no custom code keep the Cancel-only notice. The backend returns no upgrade signal at all for architectures unknown to both PyPI and main, so those still route straight to the unchanged security gate. * Force a 16-bit load for models on the latest-transformers sidecar Live validation with Zyphra/ZAYA1-8B (model_type zaya, shipped by transformers 5.13.1 but unknown to every installed tier) surfaced a generation crash when the consented sidecar load kept the default bnb 4-bit quantization: transformers' grouped-MoE kernels feed the packed uint8 expert weights straight into torch._grouped_mm, and generation dies (plain 16-bit works). New latest_tier_active_for() mirrors the sidecar activation's tier resolution and never raises; the inference worker flips load_in_4bit off when it reports true, and the load route applies the same flip so the pre-load VRAM guard and the worker command agree. Fixed tiers are untouched. With the guard, ZAYA1-8B loads and generates correctly in Studio chat. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Offer the custom-code fallback when a latest-sidecar install fails * Fail remote mapping fetches wholesale and mirror the 16-bit flip in validate A transient fetch or parse failure of one auto-mapping file no longer caches a partial latest-release map for the TTL (a real 404 on pre-5.10 tags is still tolerated), and validate_model now applies the same latest-sidecar 16-bit sizing flip as /load before the training guard so the two agree. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten comments in the latest-transformers changes * Resolve remote LoRA bases, fold nested tiers, and guard the sidecar swap latest_tier_active_for now resolves a remote adapter's base model the same way worker pre-activation does (and returns early without a sidecar pin), a hardcoded fast-path tier is raised when a nested sub-config's model_type needs a higher sidecar, and the install route refuses to swap .venv_t5_latest while training runs on it and unloads a latest-tier chat model first. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Gate the sidecar install on worker liveness and size installable upgrades 16-bit The install route now refuses while any training or export runs (tier re-resolution without the load token is unreliable for gated repos), holds the inference lifecycle gate across the unload and the swap so no load can interleave, and passes the model name to unload_model. validate_model runs the upgrade check before the training guard and sizes an installable upgrade as 16-bit, matching what /load and the worker will force after the consented install. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Close the sidecar install races and honor the kill switch over cached mappings Training starts and mutating export routes now refuse while a transformers install is in progress (shared is_install_in_progress flag), the chat unload and idle export-worker teardown moved into a before_swap hook that runs only once the staged install succeeded, and _config_model_types checks the kill switch before returning a cached latest mapping. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Reserve the sidecar swap before the gate wait and abort it on failed teardown The install-in-progress flag moved into a shared sidecar swap reservation in transformers_version, taken by the install route before awaiting the inference lifecycle gate (so training and export starts see it for the whole window) and by the lazy .venv_t5_latest repair path. The before_swap hook now raises when the chat unload or export teardown reports failure, leaving the previous sidecar untouched. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Back the sidecar swap reservation with a cross-process lock file The lazy repair runs inside worker subprocesses, where a module-level flag is invisible to the parent's route checks. The reservation now also creates a lock file next to .venv_t5_latest (O_EXCL, owner-only removal, stale after two hours for crashed owners), so is_install_in_progress sees a repair from any Studio process. * Hand the swap reservation to the installer thread and harden pre-swap teardown A cancelled install request no longer releases the reservation while the installer thread is still staging (the thread owns and releases it, shielded from cancellation). The route refuses while another inference request is generating, export teardown runs before the chat unload and is judged by worker liveness rather than the cleanup return value, and a live inference worker with no active model (failed load residue) is shut down before the swap. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Keep the lifecycle gate with the installer and recheck the swap at spawn time The gate moved into the shielded install task so a cancelled POST cannot release the guard /load honors while the installer still runs, cached latest probe results are ignored while the kill switch is set, and the training and export subprocess spawns recheck the sidecar swap reservation right before spawning (the route-level guards are one-shot and validation can outlast an install's start). * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Close the spawn-registration windows against the sidecar install Training marks the spawn in progress before its reservation recheck and is_training_active honors the flag, so the install route sees a start that has passed proc.start() but not yet recorded _proc. Export load-checkpoint rechecks the reservation after setting _export_active and before tearing down the old worker, so losing the race keeps the loaded checkpoint instead of surfacing a 500. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Refine the install-window interleavings around worker teardown The inference busy count is rechecked under the lifecycle gate (streams start by taking that gate, so nothing slips past a held gate), the training handshake moved ahead of the VRAM-freeing before_spawn hook so a lost race leaves chat/export intact, the export spawn-time check is op-aware (inside an active op the install is the side that aborts), and the Xet-stall respawn waits out a transient reservation instead of stranding the run. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Track the install's server-side unload and guard export ops against the swap The upgrade dialog store records when its install actually ran (the server unloads the active chat model before swapping), and the load flow then marks the previous model as unloaded so a later cancelled gate still triggers rollback; the custom-code fallback leaves the flag unset. _run_export gained the same reservation handshake as load_checkpoint so an install cannot block behind an hours-long export op instead of returning 409. * Tighten comments in the install-guard and upgrade-consent changes * Surface install-race refusals cleanly and roll back after a failed swap unload /load refuses while the sidecar swap is reserved so a load cannot succeed and immediately be unloaded by the pre-swap teardown, worker starts that lose the install race raise a typed SidecarSwapInProgress mapped to 409 instead of a 500, the install response reports model_unloaded even on a structured failure so the client can restore its state, and the compare flow tracks the server-side unload like the primary load path and clears a stale checkpoint on abort. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Type the export install races, scope the lock release, and keep the unload signal Export load-checkpoint and export ops raise SidecarSwapInProgress (mapped to 409 in every export route) instead of a 400-shaped failure, the export spawn check distinguishes repair reservations (always refused) from install ones (op-aware), the swap lock release only unlinks a lock this process wrote so a stale-superseded owner cannot drop the new owner's live lock, and the frontend unload signal survives a superseding consent via read-and-clear consumption instead of a reset. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Finalize a stalled run when the respawn loses the install race and latch the unload signal The Xet-stall respawn timeout now finalizes the run as a failure instead of raising into the pump's broad finalization catch (which stranded it in a training state with no worker), and a successful install retry ORs the model_unloaded signal with the latched value so a failed-after-unload first attempt still triggers rollback. * Recheck the swap under the load gate and latch the unload before resolver checks /load rechecks the sidecar reservation after acquiring the lifecycle gate (an install can reserve while the load queues on it), and the dialog store latches model_unloaded as soon as the install response arrives, before any resolver-identity guard, so a superseded consent's unload still reaches whichever load consumes the signal next. * Report cleared-state unload failures, guard queued installs, and fold name tiers A failed chat unload that still cleared the orchestrator's model state now reports model_unloaded so the client rolls back, the installer aborts with a 409 when a model load completed while it waited on the lifecycle gate, and the fixed-tier name fast path consults the config mapping when a latest sidecar is pinned so an accepted upgrade routes to the sidecar it installed (no I/O added to the unpinned path). * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Report cleared-state unload failures and harden the spawn handshake flag The failed-unload branch in before_swap now detects that the orchestrator cleared its model state and reports model_unloaded before aborting (the earlier commit claimed this fix but a scripting error dropped the edit), the installer's queued-load check compares a load generation counter so a same-model reload is caught, and both training spawn sites wrap everything after the handshake in a guard that resets _spawn_in_progress on any exception so a failed start cannot wedge is_training_active. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Bump the load generation when the load is published, not at load start A start-time bump is already visible when the installer snapshots mid-load, so a same-model reload completing after the snapshot looked unchanged and could be unloaded by the swap. The counter now increments alongside the active_model_name publish. * Self-heal a broken pinned sidecar, guard lazy repairs, and refresh stale retries A valid pin whose transformers source dir vanished now triggers the repair from the routing path (with a five minute backoff after failures) instead of silently routing latest-only models to older tiers, the lazy repair refuses while parent-visible chat/training/export workers are active since it has no teardown of its own, and a version-mismatch install failure carries the superseding release so the dialog's Retry re-requests a version that can succeed. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Flip latest-tier loads to 16-bit outside chat and protect export state Training and export workers now apply the same latest-sidecar 16-bit flip as the chat worker so a brand-new grouped-MoE architecture cannot reach bnb 4-bit through those paths, the latest-tier vision override returns None on an inconclusive probe so a transient failure is not cached as not-vision, and the install route refuses while an idle export checkpoint is loaded rather than discard it with no rollback signal on a failed swap. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Address parallel-review findings on the sidecar guards and install checks The training route sizes latest-tier jobs 16-bit before GPU selection, the inference subprocess spawn rechecks the swap reservation like training and export (covering the OpenAI auto-switch path) with the typed error mapped to a retryable 409, compat_plan blocks the install when dependency metadata cannot be fetched instead of proceeding unverified, snapshot model-type lists must contain only strings, and pin-marker package specs are validated against the sidecar's own package set before ever reaching pip. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Parent-only repairs, live-owner locks, remote-base activation, pre-teardown recheck Lazy sidecar repairs now refuse inside worker children (whose empty backend singletons cannot see live siblings) and run only in the parent where the active-worker guard is real, swap-lock staleness requires the owner pid to be dead so a slow live install is never superseded, both activation entry points resolve a remote adapter's base model like the inference worker and latest_tier_active_for already do, and load_model rechecks the reservation before tearing down the old worker so losing the race keeps the current model loaded. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Check workers under the repair reservation and keep state on refused swaps The lazy repair now reserves first and checks workers under the reservation (worker starts set their active markers before rechecking, so every interleaving aborts one side), with export ops and in-flight inference loads counted as active. The inference pre-teardown and spawn guards refuse only repair reservations since an install shares the load's lifecycle gate and aborts via its queued-load snapshot, a SidecarSwapInProgress raised before teardown no longer clears the live model mirrors, and an export spawn abort after teardown clears current_checkpoint so the page cannot claim a loaded checkpoint with no worker. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Repair a present-but-incomplete latest sidecar from routing The routing self-heal only fired when the pinned sidecar's transformers/ dir was missing. A sidecar that kept transformers/ but lost another pinned package still routed models to the latest tier, and workers refuse parent-only repairs, so every load failed until a manual reinstall. Routing now validates the full pin (via _venv_dir_is_valid) and repairs any incomplete sidecar under the same swap reservation and 5-minute backoff. * Treat an unrepaired latest sidecar as unavailable in routing When the pinned sidecar is incomplete and the lazy repair fails (offline, pip failure, workers active) or is inside the backoff window, routing returned the source dir anyway, sending models to a tier whose worker activation is known to fail. Return None instead so models an older tier supports keep loading there until a repair succeeds, matching the behavior when the sidecar dir is missing entirely. * Harden sidecar swap and repair against crash, survivor, and 16-bit paths Reclaim a swap lock as soon as its recorded owner PID is dead instead of waiting out the two-hour cutoff, so a crash mid-install no longer wedges /load, training, export, and repair for hours. A lock whose PID cannot be read yet still uses the long cutoff so the create-before-write window is never mistaken for dead. Probe process liveness with OpenProcess on Windows: os.kill(pid, 0) there is CTRL_C_EVENT (a real Ctrl+C via GenerateConsoleCtrlEvent), not a harmless check, and psutil is not always present. Return whether _shutdown_subprocess actually killed the worker and keep the live handle when it survives terminate/kill (an uninterruptible CUDA syscall can outlive SIGKILL). The pre-swap liveness guard now trusts that result, so the destructive .venv_t5_latest rename cannot proceed while a live worker still holds sidecar modules. Recover a sidecar stranded at .old when a swap's activation rename and its rollback both fail: reading the pin restores it when no swap holds the reservation, so latest-tier models are not permanently broken. Resolve the latest tier in the parent for export loads and for explicitly 16-bit training runs, not only 4-bit ones: tier resolution self-heals an incomplete sidecar, and repairs are parent-only, so those paths could not recover before. Sidecar integrity and quantization are independent. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Revert the parent-side latest-tier repair probe on training and export loads The probe ran before the route freed VRAM, so a resident chat or export worker made _workers_active_for_repair() refuse the parent-only repair; the route then tore that worker down and spawned a child that also cannot repair, so an incomplete sidecar still failed to load. Repairing correctly requires running the repair between the worker teardown and the child spawn, decoupled from VRAM sizing, which is a larger change tracked separately. Restore the prior behavior so these paths match the reviewed form and do not partially attempt a repair that cannot complete while workers are resident. * Honor failed worker shutdowns on load and revalidate the cached latest mapping The fresh-load paths spawned a new worker straight after _shutdown_subprocess without checking its result, so a worker that outlived terminate/kill (a wedged CUDA syscall) had its handle overwritten by the replacement while it still held GPU memory, and is_worker_alive/the pre-swap guard could no longer see it. Both the inference load and the export checkpoint load now abort when the old worker did not exit, so the load can be retried once it does. _config_model_types returned a cached latest mapping without re-checking the sidecar, so a sidecar deleted or broken in-process after its first parse was never re-validated: routing kept sending latest-only models to the stale latest tier while activation failed. The cached latest mapping is now dropped and re-resolved (self-healing) when the sidecar is no longer intact. * Drop cached latest mapping when the pin is gone; keep 4-bit for custom-code fallback _latest_sidecar_intact now returns False when the pin marker itself is gone, not just when a pinned package is missing. Otherwise a cached latest mapping outlived a deleted pin: _config_model_types kept returning it, so routing sent latest-only models to a tier whose worker activation then failed (no pinned version) until restart. It now drops the cache and re-resolves to no latest tier. The _overlay_transformers_dir caller already gates on a present pin, so it is unaffected. validate_model forced 16-bit sizing whenever a PyPI upgrade was merely offered, even for a model that can fall back to its own auto_map code. /load loads such a model 4-bit without the install, and the install route refuses while training is active, so 16-bit sizing here returned a VRAM 409 for the only viable 4-bit path. The offered-upgrade flip is now gated on the absence of a custom-code fallback; an already-active latest sidecar still always sizes 16-bit. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
for more information, see https://pre-commit.ci
) * Fix coding agent workspace and resume handling * Handle attached Hermes flags and OpenClaw paths * Add Codex model metadata catalog * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix Codex reasoning summary metadata * Preserve Hermes hook approval on resumed one-shots --------- Co-authored-by: Lee Jackson <130007945+Imagineer99@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…ff, Full access) (unslothai#7079) * Studio: permission levels for chat tool calls (Ask, Approve for me, Off, Full access) Replace the Bypass permissions on/off toggle with a four level permission selector, available in Settings > General (new Permissions section above Notifications), the chat settings panel, the composer plus menu, and a new always visible composer pill. Levels: - Ask for approval: every local tool call pauses for allow/deny. - Approve for me: only calls detected as potentially unsafe pause; the python/terminal sandbox stays on. - Off: never pauses; sandbox stays on (previous default behavior). - Full access: never pauses and the sandbox is disabled. Still requires the danger confirmation and is never restored across reloads. Backend adds permission_mode to the OpenAI compatible and Anthropic passthrough payloads and threads it through both tool loops. Auto mode uses a fail closed classifier in tools.py: terminal commands must be on a read only allowlist with no redirection or substitution, python code is AST scanned for writes, exec, process and network use, MCP tools auto run only with read only style names. Unknown tools always ask. Legacy bypass_permissions and confirm_tool_calls keep their exact behavior for existing API callers. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio permissions: Off is a plain toggle below Full access Off moves to the bottom of the level menu with a short description and acts as the feature-off state: the composer pill is hidden entirely while Off, and reselecting the active level toggles back to Off. * Studio permissions: higher contrast composer pill text The permission pill uses a foreground based grey instead of the shared muted pill color, so it reads darker in light mode and lighter in dark mode. Full access keeps the danger yellow. * Studio permissions: panel dropdown layout and shorter tooltip Chat settings panel: the Bypass permissions label sits on one line with a full width dropdown underneath, styled like the other panel selects. Tooltip shortened and wording uses Unsloth instead of Studio. * Studio permissions: harden auto-mode unsafe detection Extend the Approve for me classifier to catch write and exec paths that slipped through: - terminal: sort -o, tree -o, xxd -r, find -exec/-execdir/-ok/-delete and find -fprint/-fprintf/-fls now ask; plain read-only forms still auto-run. awk is no longer allowlisted since its program can write and call system(). - python: from-imports of mutating names (from os import remove [as rm]) and star imports now ask. Found by a fuzz and edge-case simulation matrix; pinned in test_permission_mode.py. * Studio permissions: split multi-line terminal commands in auto detection A shell runs each line as its own command, but shlex reads newlines as whitespace, so "ls\nrm -rf x" demoted rm to argument position and auto-ran. Normalize newlines and CR to separators, and treat any all separator token as a command boundary so runs of blank lines still split. Found by the simulation matrix; pinned in tests. * Studio permissions: address review feedback on auto-mode detection Auto-mode (Approve for me) safety classifier hardening: - Python: flag any reference to a mutating attribute, not only direct calls, so indirect refs (f = os.remove; f(x)) and aliases ask. Detect Path.open(mode) write modes and wrap the AST walk to fail closed. - Terminal: match attached short output flags (sort -o/tmp/out) and keep find context across grouping parens so find ( -delete ) asks. - Both: ask before reads that escape the sandbox workdir via parent traversal or hit credential paths (.ssh, .aws, id_rsa, .pem, etc.). permission_mode plumbing: - Fold permission_mode=full into bypass_permissions at the request model so route-level confirm-gate guards see it as bypass. - Reject ask/auto on the Anthropic Messages server-tools path, which has no confirmation channel (mirrors the confirm_tool_calls rejection). - Keep forced RAG autoinject in auto mode: the safe search_knowledge_base retrieval never gates, so derive the skip from the real confirm need. - Reset all local preferences now also clears the legacy confirm key so a reset restores the fresh default instead of the old level. Regression tests added for each case. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio permissions: close auto-mode classifier gaps from review round 2 Auto mode ("Approve for me") let a few mutating calls through as safe: - os.open(...) always creates/writes a descriptor, so treat it as unsafe even though builtin open in read mode stays safe. - fd -x/--exec/-X/--exec-batch runs a command per match; scan for these alongside find's -exec/-delete. - tempfile writes artefacts and hands back writable handles, so importing it now asks. - Calling the result of a call (getattr(os, "remove")("x"), partials) is a dynamic target the AST can't vet, so fail closed. - An MCP tool whose name pairs a read verb with a mutating one (get_or_create_issue, read_and_delete_file) no longer auto-runs on the read prefix alone. Also fold permission_mode="off" into confirm_tool_calls=False on both request models so the non-stream route guard sees the disabled gate, and drive the Confirm tool calls toggle off permission_mode="ask" so auto no longer shows it on. * Harden auto-mode classifier and normalize bypass to full for PR #7079 Approve for me now asks for a few cases it previously auto-ran: - os.open via an os alias (import os as o; o.open(path, O_CREAT)) - pathlib symlink_to / hardlink_to / link_to - importlib.import_module dynamic imports - os.mkfifo / os.mknod / os.utime Also fold bypass_permissions into full when a stale ask/auto permission_mode is sent alongside it, so the Anthropic route guard no longer 400s those legacy callers. Adds classifier and request-model regression tests. * Close more auto-mode classifier gaps for PR #7079 Approve for me now asks for cases the review surfaced: - builtin open aliased to a name (f = open; from builtins import open as w) or looked up dynamically (globals()['open']) - pickle / marshal / shelve / dill deserialization - io.FileIO write handles - sort --compress-program (runs an external program) - MCP names carrying save/archive/submit/commit/push/sync/register verbs Also refine the attribute open() write check so an explicit read mode (ZipFile.open(name, "r")) stays auto while os.open flags still ask. Adds test coverage for each case. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Close three more auto-mode gaps for PR #7079 - rg runs an arbitrary program per file via --pre / --hostname-bin, so "Approve for me" now asks for those flags (rg is on the read-only allowlist). - A path-qualified command token (./ls, /tmp/cat) is an arbitrary executable, not the trusted utility its basename matches, so it asks before running. - A direct /chat/completions caller that sets permission_mode ask/auto but omits the legacy confirm_tool_calls flag now self-enables the confirmation gate, so tools can no longer run ungated on that path. Adds classifier and request-model tests for each case. * Close auto-mode classifier gaps from review round 3 for PR #7079 Approve for me now asks for cases the latest pass surfaced: - short-option clusters bundling a write flag (sort -uo out => -u -o) - procfs reads that leak a process env/args/memory (cat /proc/self/environ, /proc/PID/cmdline, maps) - env-assignment prefixes that change command lookup/loading (LD_PRELOAD=x ls, PATH=. ls, IFS=x ls); benign FOO=1 cmd stays auto - os.open imported as a bare callable (from os import open as o) Also drops ps from the safe terminal allowlist: its BSD environment flags (ps auxe, ps eww) dump a parent process's unscrubbed env and cannot be flag-parsed reliably, so ps always asks now. Adds classifier tests for each case. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Close auto-mode classifier gaps from review round 4 for PR #7079 Terminal (Approve for me now asks for these): - cd dropped from the safe allowlist: cd /; cat etc/passwd moves the shell out of the session workdir so a later relative read escapes it - env -C/--chdir (workdir escape) and -S/--split-string (builds a fresh command line); wrapper flags are now checked - /etc//passwd and /etc/./passwd normalize to /etc/passwd before the sensitive-path scan - a sensitive path split across an assignment and an argument (p=/etc; cat $p/passwd) via best-effort NAME=value expansion Python: - builtins.exec / builtins.eval attribute calls (dynamic code execution) - destructured open aliases (f, _ = (open, print); f('out', 'w')) - a sensitive path composed from literals (os.path.join('/etc','passwd'), '/etc' + '/passwd') - ZipFile/TarFile write modes (ZipFile(name, 'w')); the reader stays auto Adds classifier tests for each case. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Close auto-mode classifier gaps from review round 5 for PR #7079 Terminal (Approve for me now asks for these): - procfs reads hidden by shell quotes (cat /proc/$PPID/enviro''n) or quoted/nested-variable assignments (p="/proc/$PPID"; cat $p/environ): quotes are stripped and NAME=value prefixes expanded before the scan - LESSOPEN/LESSCLOSE, which make less run an input preprocessor command Python: - os.chdir / os.fchdir, which move the cwd so a later relative read escapes the sandbox workdir - sensitive paths composed via a pathlib / chain (Path('/etc') / 'passwd') or an f-string of literals (f'/proc/{pid}/environ') - runpy (import) and runpy.run_path / run_module, which run arbitrary code Adds classifier tests for each case. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Close auto-mode classifier gaps from review round 6 for PR #7079 Approve for me now asks for these: - a mutating callable reached through a getattr alias (rm = getattr(os, "remove"); rm("f")): calls through a getattr-bound name fail closed - compound MCP tool names carrying clone/checkout/comment/fork/tag/ invite/share, which start with a read verb but still mutate - a sensitive path hidden behind a glob (cat /e??/passwd, cat /e[t]c/passwd): a ? / * / [..] token is matched against the sensitive-file set and bracket classes are de-obfuscated; benign globs (ls *.py) stay auto Also run first-pass RAG retrieval in off mode: like auto, off never prompts, so a direct caller passing a stale confirm flag should not lose document retrieval (both tool loops). Adds classifier tests for each case. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Close auto-mode classifier gaps from review round 7 for PR #7079 Approve for me now asks for these: - __builtins__.exec / __builtins__.eval (dynamic code via the dunder) - terminal reads that hide a credential path behind a backslash escape (cat /et\c/passwd) - read-named MCP filesystem calls pointed at a credential path (mcp__fs__read_file {"path": "/etc/passwd"}) - compound MCP names carrying append / prepend - open aliased through a subscript or builtins attribute (f = globals()["open"]; f = builtins.open) then called to write - open(..., **{"mode": "w"}) where a kwargs splat hides the write mode - a sensitive path with a dynamic segment (open(f"/etc/{name}"), os.path.join("/etc", name)); /tmp/{name} stays auto - urllib3 networking Also stop folding permission_mode ask/auto into confirm_tool_calls for external-provider requests: that branch rejects confirm_tool_calls with tools, and the mode only governs local tool calls. Local requests still self-gate. Adds tests for each case. * Close auto-mode classifier gaps from review round 8 for PR #7079 Approve for me now asks for these: - dbm on the unsafe-module list: dbm.open(file, "c"/"n") creates files, and importing the family signals a persistence writer - reads of ~/.azure and ~/.config/gh credential stores (Azure/GitHub tokens), in terminal, MCP arguments, and Python literals - compound MCP names carrying upsert / assign Adds classifier tests for each case. * Gate secret mounts and fix the composer pill count for PR #7079 - Add Docker/Kubernetes secret mount dirs (/run/secrets, /var/run/secrets) to the sensitive-path checks, so Approve for me asks before reading injected credentials (terminal, MCP args, Python). - Count the always-visible permission pill in the composer's compact threshold so labels collapse at the intended width instead of overflowing by one pill. Adds classifier tests for the secret mount paths. * Close auto-mode classifier gaps from review round 10 for PR #7079 Approve for me now asks for these: - qualified pathlib constructors (pathlib.Path('/etc') / name), folded the same as bare Path(...), so a dynamic sensitive path is detected - open aliased through an annotated assignment (f: object = open; f('out', 'w')), tracked like a plain assignment - recursive searches rooted at an absolute path (grep -R TOKEN /home, rg TOKEN /, fd pattern /etc), which read host files outside the sandbox tree; sandbox-relative searches stay auto Adds classifier tests for each case. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Close auto-mode classifier gaps from review round 11 for PR #7079 Approve for me now asks for these terminal reads, which bash would expand into a sensitive path only after the classifier had approved: - a glob that resolves into a secret mount or credential dir (cat /r?n/secrets/hf_token, cat /root/.s??/id_rsa) - a recursive search rooted at a tilde home (grep -R TOKEN ~root, grep -R TOKEN ~/logs) - a brace expansion that builds a credential path (cat /etc/pass{w,}d) - a default/alternate parameter expansion that builds one (cat /etc/pass${x:-wd}) - an input redirection that hides a glob (cat </e??/passwd) And these python calls: - a str.format-built sensitive path (open('/etc/{}'.format('passwd'))) - writer methods that persist to disk without open() (numpy.save, Image.save, plt.savefig, DataFrame.to_csv, json.dump) Segment-wise directory matching keeps benign globs (ls /home/*/projects) auto. Adds regression tests for each case and its safe counterpart. * Close auto-mode classifier gaps from review round 12 for PR #7079 Approve for me now asks for these too: - a terminal read whose parent traversal hides behind a redirection with no following space (cat <../../notes) - a python read whose path is built with str.join (open(''.join(['/etc', '/passwd']))), told apart from os.path.join - a dynamic-code builtin reached through an alias (from builtins import eval as e; e(...); x = builtins.exec; x(...)) Adds regression tests for each case and its safe counterpart. * Close auto-mode classifier gaps from review round 13 for PR #7079 Approve for me now asks for these too: - a recursive search whose root is hidden behind an assignment (p=/; grep -R TOKEN $p): the recursive-root test now runs on the assignment-expanded tokens as well - a python read whose sensitive path is split through a literal variable (base = '/etc'; open(base + '/passwd')), including via an f-string - numpy ndarray.tofile, which persists without open() - a sequence brace read (cat /etc/pass{w..w}d), expanded alongside the comma brace form before the sensitive-path scan Adds regression tests for each case and its safe counterpart. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Close auto-mode classifier gaps from review round 14 for PR #7079 Approve for me now asks for these python reads that assemble a sensitive path in a form the fold did not yet recognize: - a pathlib object reused through a name (p = Path('/etc'); p / 'passwd') - old-style percent formatting ('%s/%s' % ('/etc', 'passwd')) - Path.joinpath ('/etc'.joinpath('passwd')) - a bytes path literal (open(b'/etc/passwd')) And these terminal reads, which bash expands into a sensitive path only after the classifier had approved: - a substring parameter expansion off an assignment (p=passwd; cat /etc/${p:0:6}) - an ANSI-C quoted path (cat $'/etc/pass\x77d') - a glob into an Azure or GitHub CLI config dir (cat /home/*/.az?re/..., cat /home/*/.config/g?/...) Adds regression tests for each case and its safe counterpart. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Close auto-mode classifier gaps from review round 15 for PR #7079 Approve for me now asks for these terminal reads, which bash expands into a sensitive path only after the classifier had approved: - a per-thread procfs env alias (cat /proc/$PPID/task/$PPID/environ) - a recursive root behind a default parameter (grep -R TOKEN ${root:-/home}) - a path built by pattern replacement (p=passXd; cat /etc/${p/X/w}) And these python reads: - a pathlib .parent/.parents chain that escapes the session workdir ((Path.cwd().parent / 'other' / 'notes').read_text()) - a sensitive path resolved through glob (glob.glob('/e??/passwd')[0]) Adds regression tests for each case and its safe counterpart. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Close auto-mode classifier gaps from review round 16 for PR #7079 Approve for me now asks for these terminal reads, which bash expands into a sensitive path only after the classifier had approved: - a case-modifying parameter expansion (p=PASSWD; cat /etc/${p,,}) - a mutating find action hidden behind an assignment (f=-delete; find . $f) - a glob assembled through an assignment (g=e??; cat /$g/passwd) - a POSIX bracket class glob (cat /etc/pass[[:lower:]]d) And these python reads/writes: - a glob pattern folded from a literal variable (base='/e??'; glob.glob(base + '/passwd')) - a directly imported os.path.join (from os.path import join; join('/etc', 'passwd')) - a directly imported writer (from numpy import save; save(...)) - an aliased pathlib constructor (from pathlib import Path as P; P('/etc') / 'passwd') The find/fd and glob scans now run on the assignment/parameter-expanded command, and pathlib/join/writer import aliases are tracked. Adds regression tests for each case and its safe counterpart. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Close auto-mode gaps from review round 17 for PR #7079 Two fixes: - Gate sqlite3 in auto mode. sqlite3.connect(path) creates or mutates a database file (and runs DDL/DML) with no open()/writer attribute for the AST checks to catch, so treat the module like dbm and ask. - Only self-enable confirm_tool_calls for Studio's own tool loop. The ask/auto fold previously set confirm on every non-provider request, including a plain client-tool passthrough (client-supplied tools that Studio does not execute), which then tripped the local-tool streaming-confirm route guard and rejected the passthrough. Restrict the fold to requests that actually ask Studio to run tools (enable_tools / enabled_tools / mcp_enabled). Adds regression tests for the sqlite3 write and for the passthrough vs tool-loop confirm behavior. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Close auto-mode gaps from review round 18 for PR #7079 Classifier (auto mode asks for these): - os.open through a module alias (import os as o; o.open(...)); os/posix aliases are tracked like the literal module name. - less/more pagers, whose escapes (+cmd, !shell, -o/--log-file, LESSOPEN) can run a command or write a file the command-name allowlist cannot see, so they are no longer auto-approved. - a read-named MCP tool carrying a mutating query (query_database {"query": "DELETE FROM runs"}); DML/DDL statements are matched as whole statements so a natural-language query that merely contains "delete" stays safe. - ML persistence helpers (save_pretrained / save_file / save_model / save_weights / save_lora / save_checkpoint) that export weights to disk. Route: - Honor CLI-forced tools when deriving the confirm gate. When a process policy (unsloth run --enable-tools) opens the local tool loop without a request-level tool signal, a permission_mode ask/auto request now derives confirm at the route (GGUF and safetensors paths) so the mode still gates the call, and a non-streaming ask/auto request is rejected rather than running unprompted. A plain client-tool passthrough (no local loop) is unaffected. Adds regression tests for each case and its safe counterpart. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Close auto-mode classifier gaps from review round 19 for PR #7079 Approve for me now asks for these too: - a terminal read whose path is built by indirect parameter expansion (x=passwd; p=x; cat /etc/${!p}) - a bash /dev/tcp or /dev/udp redirection, which opens a network socket (cat </dev/tcp/host/port) - a python read via pathlib's receiver-plus-pattern glob (Path('/etc').glob('passw?')) - a python read whose sensitive root passes through a normalizer (os.path.abspath('/etc'), Path('/etc').resolve()) - a pickle-backed loader that can execute code on load (torch.load, joblib.load, pandas.read_pickle), tracked through module import aliases - compiled code wrapped into a callable (compile(...) + types.FunctionType) Adds regression tests for each case and its safe counterpart. * Honor unset permission_mode as ask across the local tool loop for PR #7079 Three gaps where an omitted permission_mode did not behave as the documented default ("ask"): - The frontend only sent permission_mode / confirm_tool_calls / bypass_permissions when a tool pill was on. A process policy (unsloth run --enable-tools) can open the tool loop with no pill, so the backend never saw the selected gate. Send the three permission fields at the top level of every local chat payload instead. - The backend read payload.confirm_tool_calls directly at the pre-switch guard and both late per-backend derivations, so an unset mode fell through as no-gate even for an explicit ask/auto. Add _permission_mode_confirm(payload): explicit confirm_tool_calls wins, explicit ask/auto engage the gate, off/full never prompt, and an unset mode defaults to ask only where realizable (streaming), keeping the legacy no-gate run for non-streaming unset requests. - A forced ask/auto tool loop (CLI --enable-tools) with no stream now 400s at the pre-switch guard before evicting the resident model, matching the existing confirm-without-stream rejection. Adds test_permission_mode_confirm_derivation covering the derivation truth table. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Declare permission_mode and bypass_permissions on the local chat request type The previous change moved permission_mode, confirm_tool_calls and bypass_permissions to the top level of the local chat payload. They had lived inside a conditional spread, which is not subject to excess property checking, so the fields were never declared on OpenAIChatCompletionsRequest. At the top level tsc flagged permission_mode as unknown (TS2322), failing the frontend build and every job whose Studio install builds the frontend. Add permission_mode and bypass_permissions to the request interface (confirm_tool_calls was already present). * Close auto-mode classifier gaps from review round 21 for PR #7079 Auto mode ("Approve for me") now asks for these too: - a pathlib read built from a concrete constructor (PosixPath, WindowsPath and their Pure* forms), which the folder previously ignored so PosixPath('/etc') / 'passwd' lost its /etc root and ran unprompted - a terminal or python read of the ssh host keys under /etc/ssh, which the sensitive-path regex only covered for passwd/shadow/sudoers - a read whose path variable is reassigned: the whole-tree pre-scan kept the last binding, so base = '/etc'; open(base + '/passwd'); base = 'data' folded to data/passwd and ran even though execution reads /etc/passwd; any multiply-bound name now folds to the escape sentinel and asks Also stop the pre-switch guard from rejecting a plain client-tool passthrough. permission_mode only implies the confirm gate for Studio's own local tool loop (enable_tools / enabled_tools / mcp_enabled); a non-streaming client-tool passthrough that carries permission_mode ask/auto (confirm_tool_calls left unset by the validator) must forward to the provider branch. Only an explicit confirm_tool_calls=True still forces the local-confirm rejection there. Adds regression tests for each case and its safe counterpart. * Fix permission-pill compaction count and Full-access confirm sync for PR #7079 Two frontend consistency issues in the permission-level UI: - The composer collapses tool pills to icons above four, but the count left out the permission pill, which renders in every mode except off. With one optional pill also shown the row reached five pills without collapsing and could overflow. Count the pill when it is visible (permission_mode != off). - Entering Full access via setPermissionMode('full') or setBypassPermissions(true) left confirmToolCalls at its previous value, so a Full-access run (which sends confirm_tool_calls=false) could still report confirmations as enabled in response metadata. Set confirmToolCalls false at both entry points. * Close auto-mode classifier gaps from review round 23 for PR #7079 Auto mode ("Approve for me") now asks for these too: - a command using an abbreviated GNU long option that reaches a write/exec action (sort --out= for --output, env --ch= for --chdir, fd --base-dir= for --base-directory); a prefix of an unsafe long flag now fails closed - printf -v NAME, which assigns to a shell variable, so printf -v PATH %s .; ls can rewrite PATH and run ./ls unprompted - fd --base-directory / --search-path, which move the search root outside the session workdir without any positional slash token - an MCP tool whose compound read name carries a copy-style mutator (read_and_copy_file, get_and_snapshot_volume): copy, duplicate, import, export, download, backup, restore, snapshot, mirror Also treat an omitted permission_mode as its documented default ("ask") on the Anthropic Messages server-tool path. That branch has no confirmation channel and already rejects explicit ask/auto, so an omitted mode now falls into the same rejection instead of silently running server tools unprompted, unless the caller opted out with confirm_tool_calls=false (the legacy equivalent of "off"). off/full and that opt-out still run; the two routing tests that relied on the old implicit run now set permission_mode="off". Adds regression tests for each case and its safe counterpart. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Refine permission gating from review round 24 for PR #7079 Four fixes from the latest review: - Anthropic Messages server tools: an omitted permission_mode no longer rejects a request that only runs safe server tools (web_search), so existing Anthropic callers keep working. It still rejects an omitted mode when a local tool (terminal/python) is selected, and an explicit ask/auto is still rejected outright. off/full and a confirm_tool_calls=false opt-out always run. - Pre-switch confirm-without-stream guard: use _explicit_studio_tool_loop_requested (the same predicate the passthrough router uses) instead of the policy-inclusive _effective_enable_tools, so a process --enable-tools policy no longer turns a client-tool passthrough into a local-loop rejection. - Auto mode now asks for `uniq INPUT OUTPUT`: uniq writes its second file positional, so a second positional (numeric flag values skipped) is treated like `sort -o`. A lone `uniq file` or piped `... | uniq` stays safe. - MCP mutation check now strips SQL comments before matching, so DELETE/**/FROM and UPDATE/**/users (comment-as-whitespace) no longer slip past the DML/DDL denylist. Adds regression tests for each case and its safe counterpart. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Close auto-mode gaps from review round 25 for PR #7079 Auto mode ("Approve for me") now asks for these Python cases too: - a bare archive constructor with a write mode (from zipfile import ZipFile; ZipFile('out.zip', 'w')), tracked through import aliases like the zipfile.ZipFile attribute call already was - a dynamic lookup aliased through getattr (g = getattr; rm = g(os, 'remove'); rm('file')), not just direct getattr(...) calls - a callable that wraps open or a writer via functools.partial (w = partial(open, mode='w'); w('out.txt')), which hides the write mode Also: - Always-safe tools (render_html) stream their early provisional canvas card in auto mode again. The provisional-card guard mirrored the raw confirm flag, which suppressed the early card under Approve-for-me; it now reuses the auto-mode safety decision (is_always_safe_tool). - The assistant-ui composer no longer counts the permission pill toward its collapse threshold when the level is Off (the pill renders null there), matching the other composer. Adds regression tests for each case and its safe counterpart. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Align permission-mode confirm guards with the router (review round 26) Three pre-switch confirm-gate checks disagreed with how the tool loop actually enters, so a valid request could 400 (or an invalid one could evict the resident model) at the wrong point: - The /chat/completions pre-switch guard only looked at explicit request fields, so a process --enable-tools policy that forces the loop on (request omits enable_tools, no client tools) slipped past it and only 400ed after _maybe_auto_switch_model had swapped the model. It now mirrors the router's own loop-entry gate (_effective_enable_tools or mcp, tool_choice="none" disabling it unless explicitly asked) while still deferring to client-tool passthrough, so the policy-forced case is caught before the switch. - The ChatCompletionRequest full/off fold treated enabled_tools by itself as a local-loop request and set confirm_tool_calls=True. The router never starts the loop on enabled_tools alone (it only filters which tools run), so a non-streaming passthrough carrying client tools plus enabled_tools 400ed instead of routing verbatim. The fold now keys off the same enable_tools / mcp_enabled signals. - The Anthropic /v1/messages unsupported-mode rejection (ask/auto, or an omitted mode selecting terminal/python) ran inside the post-switch server-tools block, so an invalid request evicted the resident model before the 400. It now runs before the auto-switch, determined from the requested server tools, like the neighboring malformed- and mixed-tool guards. Adds regressions for each: a policy-forced non-streaming ask/auto guard rejection that never reaches the switch, an enabled_tools-only passthrough that keeps confirm unset, and an Anthropic rejection that precedes _maybe_auto_switch_model. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Close auto-mode classifier gaps from review round 27 for PR #7079 Auto mode ("Approve for me") now asks for these host-mutating or host-reading cases it previously ran unprompted (the sandbox does not jail filesystem reads, and terminal commands can change host state): - Destructured string literals fold into the scanned path now, so base, leaf = ('/etc', 'passwd'); open(base + '/' + leaf).read() resolves to /etc/passwd and asks, like the single-assignment form already did. The tuple/list unpacking branch tracked only aliases to open; it now also binds literal and folded-path elements. - pathlib name rewrites fold to the rewritten path: Path('/etc/x').with_name('passwd').read_text() (and with_stem / with_suffix) spell no literal /etc/passwd but resolve to it, so they are folded and caught. Benign in-sandbox rewrites stay safe. - hostname NAME (or -F/--file, -b/--boot) sets the hostname, so a positional or a set flag asks; bare hostname and the display flags (-f/-i/-I/...) stay read-only. - date -s/--set STRING and the bare MMDDhhmm... positional set the system clock and now ask; the display forms stay read-only (+FORMAT, -u/-R, and -d/-r/-f whose following value is skipped so date -d tomorrow is not mistaken for a clock-setting positional). Adds regression rows for each gap and its safe counterpart. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Close more auto-mode classifier gaps from review round 28 for PR #7079 Auto mode ("Approve for me") now asks for these cases too: - Mapping-style %-formatted paths. '/etc/%(f)s' % {'f': 'passwd'} folds to /etc/passwd and asks; a dynamic value or a non-literal mapping leaves the NUL marker so /etc/<dynamic> still fails closed. The path folder previously handled only tuple/scalar % right-hand sides and returned None for a dict, hiding the sensitive segment. - A read-named MCP database tool carrying PostgreSQL COPY. COPY ... FROM bulk-loads a table and COPY ... TO writes a server-side file, so both are matched as mutating queries like DELETE/UPDATE already were. A 'copy' substring in a column name stays safe (word boundary). - logging file handlers. logging.FileHandler('out.log', mode='w') (and the default append mode, RotatingFileHandler/TimedRotatingFileHandler/ WatchedFileHandler, and the bare from-import form) create or truncate a file like open(..., 'w'), so they are classified as writer calls. StreamHandler / NullHandler and logging reads stay safe. Adds regression rows for each gap and its safe counterpart. * Fix writer aliases, GraphQL mutations, and auto server tools (review round 29) - Auto-mode Python: an aliased writer or archive constructor is tracked like the existing open alias, so from numpy import save; s = save; s('out.npy', arr) (and z = ZipFile; z('a.zip', 'w'), incl. the destructured forms) ask instead of running the write unprompted. A benign builtin alias (x = len) stays safe. - Auto-mode MCP: a read-named tool carrying a GraphQL mutation now asks. query_graphql {"query": "mutation { deleteIssue(id: 1) }"} matches a leading mutation keyword (GraphQL uses # comments, so it scans the raw payload); GraphQL read queries stay safe. - Anthropic /v1/messages: permission_mode "auto" no longer 400s a safe-only server-tool selection. auto only needs a confirmation channel for an unsafe call, so like the omitted default it runs for web_search / RAG / render and rejects only when a gate-needing local terminal/python tool is selected. ask still always rejects (it asks per call, which this passthrough cannot honor). The rejection stays ahead of the model auto-switch. Adds regression rows/cases for each. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Gate asyncio spawn, net clients, default-captured open; allow safe-only auto (round 30) Auto-mode Python now asks for more process/network/write vectors: - asyncio process spawners (asyncio.create_subprocess_exec/shell and a loop's subprocess_exec/shell) run an arbitrary program without the terminal blocklist, so they gate like os.system/subprocess. - stdlib network clients imaplib / poplib / nntplib / xmlrpc(.client) / webbrowser open outbound connections the sandbox does not namespace off, so their import asks like the other network modules. - a callable captured as a function or lambda parameter default (def f(o=open): o('out', 'w')) now binds that parameter into the same alias set, so the later write through it is gated. A benign default (o=len) stays safe. Also, permission_mode "auto" no longer 400s a non-streaming local tool request whose selection is always-safe-only (web_search / RAG / render). auto only prompts for a classifier-flagged call, so a safe-only auto request needs no stream, while ask, an explicit confirm_tool_calls=true, MCP, and an unrestricted or unsafe selection still require it. Applied via a shared _confirm_gate_needs_stream helper at the pre-switch, GGUF, and safetensors confirm-stream guards; the loop's per-call confirm flag is unchanged. Adds regression rows/cases for each. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Catch brace-glob paths and attribute writer aliases; unfold auto (round 31) - Terminal auto mode now runs the glob-sensitive scan over every expansion candidate, so a brace-expanded glob (cat /e{t,}c/pass?d, which bash expands to /etc/pass?d and then globs to /etc/passwd) asks. Brace expansion alone spells no literal /etc/passwd and the glob only resolves once the brace group is expanded, so scanning both together is required. A benign brace + glob stays safe. - Python auto mode now tracks a mutating attribute captured as a plain name: s = np.save; s('out.npy', arr) binds a writer alias, a captured .open bound method (p = Path('out').open; p('w')) fails closed on any call since its mode position varies, and z = zipfile.ZipFile is gated like the bare import. A benign attribute alias (x = np.mean) stays safe. - permission_mode "auto" is no longer folded to confirm_tool_calls=true on the request model. Folding it defeated the safe-only-selection exception in _confirm_gate_needs_stream (an explicit confirm forces stream=true), so a non-streaming safe-only auto request was rejected. Leaving it unset lets the route apply the exception; the mode still drives the loop's per-call gate. "ask" still folds (it gates every call). Adds regression rows/cases for each. * Harden SQL/GraphQL/writer classification and passthrough guards (round 32) MCP argument mutation detection (read-named query tools): - CREATE DDL now matches modifiers and the broader object set, so CREATE OR REPLACE VIEW, CREATE UNIQUE INDEX, CREATE TEMP TABLE, CREATE MATERIALIZED VIEW and CREATE FUNCTION ask. - Stored-procedure invocation (CALL proc(...), EXEC/EXECUTE) and VACUUM ask; a natural-language "call me back" stays safe via the trailing "(" / ";" / end lookahead. - GraphQL # comments are stripped before the mutation match, so mutation # note\n { deleteIssue(id: 1) } no longer hides the mutation. Python auto-mode classification: - numpy.memmap / open_memmap and pandas ExcelWriter / HDFStore create or truncate a file on construction, so they gate like open(..., "w"). - asyncio networking (asyncio.open_connection, loop.create_connection / create_server and unix variants) opens outbound connections/listeners the sandbox does not isolate, so it gates like socket.connect. Terminal auto-mode: file -C / --compile writes a compiled magic database. Routing: - A JSON-schema response_format is guided-decoding passthrough, not a local tool loop, so a --enable-tools policy no longer 400s a non-streaming ask/auto structured-output request at the confirm guard. - An explicit confirm_tool_calls=False opts out of the Anthropic Messages server-tool gate entirely (it wins over the mode, mirroring _permission_mode_confirm and the GGUF path), so it runs even under ask. Adds regression rows/cases for each. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Track path-ctor aliases, exempt empty selection and safe safetensors card (round 33) - Python auto mode now propagates path constructor / join aliases, so assigning Path or os.path.join to another local name is still folded: P = Path; (P('/etc') / 'passwd').read_text() and j = os.path.join; open(j('/etc', 'passwd')) ask, while a benign /tmp alias stays safe. - _confirm_gate_needs_stream now distinguishes an omitted enabled_tools (None, all tools) from an explicit empty list ([], no tools). An empty selection runs no built-in tool and cannot prompt, so a non-streaming auto request with enable_tools=true, enabled_tools=[] is no longer 400ed under a --enable-tools policy. - The safetensors provisional render_html card now uses permission_mode: render_html is always safe and never prompts, so its early canvas card streams under auto (which ships confirm_tool_calls=true) instead of being suppressed, matching the GGUF path's is_always_safe_tool exemption. Adds regression rows/cases for each. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Extend auto-mode classifier: SQLite mutations, more net/xattr/compressed writers Additional fail-closed gaps found by a fresh adversarial pass, each with a reproduction and a benign control: - MCP read-named tools now ask on SQLite-flavored writes the base DML/DDL regex missed: ATTACH / DETACH DATABASE, a write-form PRAGMA (PRAGMA journal_mode=WAL / user_version=42 / foreign_keys(0), while the read-form PRAGMA journal_mode stays safe), and load_extension() which loads and runs an arbitrary shared library. - Python auto mode now gates the remaining asyncio network entry points (start_server, open_unix_connection, loop.create_datagram_endpoint, sock_connect), os.setxattr / os.removexattr metadata writes, the gzip / bz2 / lzma single-stream writers (GzipFile / BZ2File / LZMAFile, mode-gated like ZipFile so a read stays safe), pandas to_xml, and the websockets client. Benign controls (SELECT 1, read-form PRAGMA, asyncio.sleep, gzip read, numpy read, natural-language "attach"/"analyze") stay safe. Regression rows added to test_permission_mode.py. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Close follow-up auto-mode gaps: SQLite/GraphQL variants, more writers and net A fresh adversarial pass on the previous round found consistent extensions of the same fail-closed rules, each reproduced with a benign control: - MCP read-named tools: DROP / ALTER now cover the same broad object set as CREATE (DROP FUNCTION, ALTER INDEX, DROP MATERIALIZED VIEW); ATTACH is caught without the optional DATABASE keyword via its quoted-path form; a schema-qualified write PRAGMA (PRAGMA main.user_version=1) is matched; and a GraphQL mutation carrying directives (mutation M @audit { ... }) is treated as a mutation. - Python auto mode: os.startfile (Windows program launch), asyncio start_unix_server, and the socketserver framework now ask; a gzip/bz2/lzma open imported under an alias (from gzip import open as gopen) is gated like builtin open; and a dynamic path prefix that can form a sensitive absolute root (open(chr(47) + "etc/passwd"), open(os.sep + "etc/passwd")) is treated as sensitive, while a dynamic prefix with a benign suffix stays safe. Benign controls (read-form PRAGMA, natural-language "attach ... as", "drop the idea", SELECT dropped_at, query @cached, gzip read alias, dynamic prefix + data/file suffix) stay safe. Regression rows added to test_permission_mode.py. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Gate GNU time -o, basicConfig/methodcaller/fileinput, and more SQL mutations Another adversarial pass surfaced further consistent fail-closed gaps, each reproduced with a benign control: - Terminal: GNU time -o/--output/-a/--append truncate or append to a file with timing output; time is a wrapper, so the flag is checked before the wrapped command like env -C. - Python auto mode: logging.basicConfig(filename=...) opens a log file for write; operator.methodcaller("write_text"/...) hides a writer method behind a string and is now treated as dynamic dispatch (like getattr/partial); fileinput.input(..., inplace=True) rewrites a file in place (the default read form stays safe). - MCP read-named tools: UPDATE now matches quoted, bracketed, and schema-qualified targets (UPDATE "users" / public.users / ONLY public.users / [users] / `users` SET); SELECT ... INTO OUTFILE/DUMPFILE writes a server file; and state-changing SQL functions inside a SELECT (pg_terminate_backend, setval, pg_write_file, lo_export, ...) ask. Benign controls (time ls / time -p, basicConfig(level=), methodcaller("upper"), fileinput read, NL "update ... set", setval_col column, PL/pgSQL SELECT INTO var) stay safe. Regression rows added to test_permission_mode.py. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten auto-mode classifier comments Collapse the multi-line rationale blocks in the permission classifier to one or two lines each without dropping the exploit each branch closes. Comments and whitespace only (no code change); the classifier tests are unchanged and pass. * Retry transient SSE stalls in the tool-calling smoke probes The tool-calling job flaked with a bare "TimeoutError: timed out": the server-side python/bash probes stream over post_sse(), which (unlike post()) had no transport-level retry, so a single stalled stream on a shared CI runner hard-failed the whole step even though function calling had already passed. post_sse() now mirrors post(): a transport-level stall (stream open or a mid-stream read timing out) is retried once with a fresh request capped at 300s, while HTTP status errors still surface immediately. The Linux _run_tool_probe caps each attempt at 360s and treats a stall that outlives the retry as a failed attempt (rotate to the next seed) instead of raising, and the web_search probe uses the same 360s cap. A genuine server wedge still fails (the retry also times out), so real regressions are not masked. Applied to the Linux, macOS, and Windows inference-smoke workflows, which share the probe. * Close five more auto-mode classifier gaps from review Each reproduces with a benign control: - Path constructor aliased through an attribute (P = pathlib.Path) now folds like the bare-name alias, so (P('/etc') / 'passwd').read_text() asks while a /tmp alias stays safe. - Callable defaults that are not plain names now bind the parameter: an attribute writer (def f(s=np.save)), an archive constructor, a captured .open, and partial(open, mode='w') fold like the equivalent assignment; a benign default (np.mean) does not. - A dynamic piece inside a sensitive name (open('/et' + chr(99) + '/passwd'), which folds to '/et\x00/passwd') now asks: the literals around each dynamic segment are matched against a credential target with the segment as any run of non-separator chars, so an all-dynamic ('1 + 1') or segment-spanning (a + '/' + b) path stays safe. - MCP read-named tools now ask on REFRESH MATERIALIZED VIEW and REINDEX; a 'refresh' column or natural-language 'refresh' stays safe. - A writer/open alias handed to a higher-order invoker (map(open, names, modes), starmap(np.save, ...)) is gated even without a direct call site; a benign map(len, ...) is unaffected. Regression rows added to test_permission_mode.py. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Default tool pills off on model load so tool execution is opt-in resolveToolsEnabledOnLoad turned the web-search and code pills on for any tool-capable model when the user had expressed no preference. Default them off instead, so tool execution is enabled only when the person clicks the pill to turn it on; a saved preference (on or off) is still honoured, so a user who already enabled tools keeps them on. * Gate mark/subscribe MCP verbs and qualified higher-order writer invokers - A read-prefixed MCP tool name carrying mark / subscribe / unsubscribe (get_and_mark_read, get_and_subscribe) now asks; a 'mark' substring inside one token (list_bookmarks) stays safe. - The higher-order writer check now also fires for a qualified invoker (itertools.starmap(open, ...), functools.reduce(open, ...)), matching the bare-name map/filter form; the writer-check on the first arg keeps a benign itertools.starmap(len, ...) or itertools.chain(...) safe. Regression rows added to test_permission_mode.py. * Close more auto-mode gaps and align the ask confirm fold across paths Each classifier change reproduces with a benign control: - MCP read-named tools now ask on reply / notify verbs (get_and_reply_email, list_and_notify_users), on catalog writes COMMENT ON / SECURITY LABEL / LOCK TABLE and CREATE|DROP|ALTER POLICY, and on state-changing PostgreSQL functions inside a read-shaped SELECT (nextval, set_config, pg_notify, the advisory-lock family). A 'comment' column, a 'locks' table, and a 'nextval' column prefix stay safe; the natural-language NOTIFY/SET ROLE statement forms are left out because SET/NOTIFY overlap ordinary prose. - Python auto mode now gates loader.exec_module (runs a module's code), archive extractall (zip-slip file writes), the ensurepip / venv modules (install pip / build an environment), and pydoc.writedoc. The Hugging Face login token (~/.cache/huggingface/token and stored_tokens) is now a sensitive path, while the rest of that cache (model data) stays readable. - ChatCompletionRequest no longer overwrites an explicit confirm_tool_calls=false when permission_mode='ask': the fold only self-enables the gate when the flag is unset, so an explicit opt-out wins on the chat path exactly as it already does via _permission_mode_confirm and the Anthropic pre-switch guard. Regression rows added to test_permission_mode.py. * Gate sort -T, xxd outfile positional, and the legacy HF token path - sort -T / --temporary-directory writes spill files to a caller-chosen dir, so it joins -o / --output in sort's unsafe-flag set. - xxd [infile [outfile]] writes its second positional, like uniq; xxd now uses the same second-positional-write handling (xxd in.bin out.hex asks, xxd in.bin and xxd -c 16 in.bin stay read-only). - The sensitive-path regex now also covers the legacy ~/.huggingface/token location (optional leading dot), not just ~/.cache/huggingface/token; an unrelated dir like myhuggingface/token stays safe. Regression rows added to test_permission_mode.py. * Catch multi-char SQL mutation targets, globbed credential names, digit outfiles Three fail-open gaps in the auto-mode classifier, each with a benign control: - SQL: the trailing word boundary on the MCP mutation regex meant a bare \w stopped at the first character, so TRUNCATE users, GRANT SELECT ON t, and REVOKE ALL ON t (multi-character names) slipped through while single-letter targets matched. Match the whole identifier instead, and accept an explicit AS alias on UPDATE (UPDATE users AS u SET). The implicit-alias form is left out because it is indistinguishable from the prose "update <noun> <noun> set". A truncate_log column and a grants table stay safe. - A glob that resolves to a credential basename anywhere (cat ~/.huggingface/tok?n -> token, cat proj/.netr? -> .netrc, cat repo/.aws/cred*) now asks; the fixed target list only covered a handful of home paths. notes/dra?t.txt and token_counts.tx? stay safe. - uniq / xxd counted file positionals but skipped every numeric token to ignore a flag value, so a file literally named with digits (uniq 123 out) hid the output positional. Track each command's value-taking flags and consume only the value, so uniq -f 2 in stays safe while uniq 123 out asks. Regression rows added to test_permission_mode.py. * Isolate the permission-mode loop tests from process-global state The loop-driving tests (auto/off/full/bypass) drove run_safetensors_tool_loop against a process-global approval registry (state.tool_approvals._pending) keyed by a single shared session id, and read os.environ. Other backend test modules mutate both, some at import time, so in the full-suite ordering a stale pending approval or a leaked env var could make the loop deny or skip a call these tests expect to run. It passed when the file ran alone but failed only in the complete tests/ run on CI. Add an autouse fixture that snapshots and restores os.environ and the approval registry around each test, and give every _drive call a unique session id so a leaked approval can never collide. Attach a compact event-stream dump to the loop assertions so any residual full-suite-only failure reports what the loop actually did instead of a bare diff. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: harden auto-mode classifier for recursive listers, sort file lists, aliased invokers, single-member extract Close four fail-open gaps in is_potentially_unsafe_tool_call: - terminal: tree/du (always recursive) and ls -R rooted at an absolute or tilde path now ask, matching the existing grep/rg/find recursive-read gate; relative walks stay safe. - terminal: sort --files0-from=F reads the file list named in F, so it can read arbitrary host files indirectly; added to sort's unsafe flags. - python: track aliases of the higher-order invokers (m = map; from itertools import starmap as sm) so an aliased invoker handed open/a writer is still gated; a benign callable (map(len, ...)) stays safe. - python: single-member archive extract (ZipFile/TarFile.extract) writes to disk like extractall and is vulnerable to a crafted member path, so gate it. Also update the stale _FakeExecuteTool in test_permission_mode.py to accept the thread_id keyword that run_safetensors_tool_loop now forwards to execute_tool after the main merge, which had broken the five tool-loop tests. Adds regression rows covering each gap plus benign controls. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: normalize unknown permission_mode to 'ask' instead of a 422 The request models validated permission_mode with Literal[ask, auto, off, full], so an unrecognized value from a newer UI/client was rejected with a 422 before the tool loops could apply their unknown -> ask fallback (safetensors_agentic.py:464, llama_cpp.py:9001). That made the intended forward-compat degradation unreachable at the API boundary for both Chat Completions and the analogous Anthropic field. Accept a plain string on both ChatCompletionRequest and AnthropicMessagesRequest and normalize in a before-validator: None stays unset, the four known modes pass through, and any other value degrades to the safest gate ('ask'), matching the loops. Adds a regression test covering unknown/None/known across both models. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: close five more auto-mode classifier gaps - terminal: xargs is no longer a safe wrapper. It appends arguments read from stdin that the scan never sees, so `echo -o out /etc/passwd | xargs sort` forwards to `sort -o out /etc/passwd` (a write + sensitive read) while only the allow-listed literals are visible. Any xargs command now asks. - terminal: ionice -p/-P/-u change the I/O priority of an already running process / group / user instead of forwarding to a wrapped read-only command, so `ionice -c 3 -p <pid>` now asks. ionice -c 3 <cmd> stays safe. - MCP: gate ALTER SYSTEM, which persists PostgreSQL server configuration and was not one of the DDL objects the mutation detector matched. - MCP: a credential noun in a read-named tool (read_secret, list_tokens, get_credentials, fetch_api_key) is a sensitive disclosure, so it asks even without a mutating verb or a path/SQL argument. Scoped *_key nouns keep a primary_key / keyboard lookup safe. - render_html: no longer unconditionally safe. A static canvas still auto-runs, but one whose HTML/JS reaches the network (fetch/WebSocket/remote script) asks, since it can egress under the canvas CSP when artifact network access is on. Its early provisional card is suppressed under the auto confirm gate, and the confirm-without-stream guard now requires a stream when render_html is selectable. Adds regression rows and benign controls for each, and updates the render_html provisional-card and confirm-gate tests to the new behavior. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: extend auto-mode gates for indirect file lists, dynamic lookups, HTML network loads, and Anthropic render_html Follow-ups on the previous classifier round: - terminal: wc/du/find --files0-from (and find's -files0-from primary) read a NUL-separated list of input paths from a file, the same indirect mechanism as sort --files0-from, so a crafted list reads arbitrary host files past the literal path/root checks. Gate them like sort. - python: a namespace lookup through a dict-style call (f = __builtins__.__dict__.get('open'), globals().get('open'), vars(x).get(...)) can return open/eval/a mutator, so poison the bound name like getattr/subscript lookups already are. An ordinary dict .get or os.environ.get stays safe. - render_html: broaden the network detector so a canvas that loads a resource via CSS url()/@import, srcset, or a root-relative (/path) or protocol-relative (//host) src/href is treated as networked, not just fetch/WebSocket/remote script. Relative ./x and url(#id)/data: refs stay static/safe. - Anthropic /v1/messages: drop render_html from the unprompted-safe server-tool set. Since it can prompt (networked canvas) and this channel invokes the loop without confirm, selecting it under ask/auto/omitted now rejects like terminal/python; off/full (or an explicit confirm opt-out) run it. Adds regression rows and benign controls for each, plus an Anthropic route test. * Studio: close six more auto-mode classifier gaps - terminal: a glob that expands to a project .env (cat .e?v) now asks; .env joins the sensitive glob-basename set, matching the literal-path gate. - python: an open bound onto an attribute (box.f = open; box.f('out','w')) is tracked by attribute name, and open invoked via .__call__ (open.__call__('out','w'), unwrapped to the underlying callable) is gated, so neither slips past the name-based open-alias checks. Benign attribute callables and .__call__ on non-writers stay safe. - python: a namespace lookup via .get/.pop/.setdefault already covered the builtins case; unchanged here. - MCP: a mutating HTTP verb in a method/verb argument (get_url {"method": "DELETE"|"POST"|"PUT"|"PATCH"}) now asks, so a generic HTTP tool cannot mutate an external service unprompted; GET/HEAD stay safe. - MCP: a credential/secret environment-variable value (get_env {"name": "OPENAI_API_KEY"}) is treated as a sensitive read via the same credential-noun match used for tool names; PATH/HOME stay safe. - render_html: self-navigation sinks (location.assign/replace, window.open, assigning a URL to (window.)location(.href)) join the network detector, so a canvas that navigates itself to an external URL asks; location.reload() / history.back() stay static. Adds regression rows and benign controls for each. * Studio: gate obfuscated canvas egress, sensitive-dir iteration, and MCP metadata-host reads - render_html: strip block comments before the network scan so fetch/*x*/(...) cannot hide egress, and match bracket-access forms (window['fetch'](...), self['open'](...)). Line // comments are left alone so the // in an https URL is not eaten. A comment-only canvas stays static. - python: enumerating a directory outside the sandbox (Path('/etc').iterdir(), os.scandir('/etc'), os.listdir('/home'), os.walk('/')) reads host filenames the direct /etc/passwd checks would prompt for, so gate it when the target dir folds to an absolute/tilde/sensitive path; a relative dir stays safe and an unresolved dynamic dir is left to other checks. - MCP: a read-named HTTP tool pointed at a cloud-metadata / link-local host (fetch_url {"url": "http://169.254.169.254/..."}, metadata.google.internal) reads instance credentials, so classify those URL arguments as sensitive, mirroring the sandbox SSRF blocklist; ordinary and localhost URLs stay safe. Adds regression rows and benign controls for each. * Studio: gate meta-refresh navigation, pandas HTML/markdown exporters, absolute glob roots, and checksum verify mode * Studio: gate starred open writes, builtins.__import__, computed render_html sinks, and procfs fd reads in auto mode * Studio: gate remote worker canvases, huggingface_hub downloads, and write callables passed to user helpers in auto mode * [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: Unsloth <michaelhan@Michaels-MacBook-Pro.local> Co-authored-by: Daniel Han <danielhanchen@gmail.com>
…#7046) * Studio: make the Cloudflare tunnel opt-in (off by default) A wildcard bind (`-H 0.0.0.0`) auto-started a public trycloudflare.com tunnel, so exposing Studio on the LAN also published it to the public internet. Flip the default so the tunnel is opt-in. - `--cloudflare` is now tri-state (Optional[bool], default None = off), mirroring the existing --enable-tools/--disable-tools handling. Pass --cloudflare to expose a public HTTPS link for a wildcard bind; --secure still implies the tunnel. - --secure + --no-cloudflare is still rejected as a contradiction. - Update the parent-command guard, re-exec forwarding, startup-banner wording, the colab comment, README, and tests. * Studio: update installer/setup launch hints for opt-in Cloudflare The post-install launch hints only mentioned --secure for a public link. Now that the tunnel is opt-in, clarify that -H 0.0.0.0 exposes the raw port on the LAN (not a public URL), and surface --cloudflare as the explicit opt-in for a public HTTPS link (--secure keeps the raw port private). Applied to install.ps1, install.sh, and studio/setup.sh. * Studio: address review - keep cloudflare tri-state + harden run re-exec Two review points from the bots: - Gemini: keep `cloudflare` as Optional[bool] in run_server instead of casting None -> False, so the startup banner can distinguish "OFF (default)" (unset) from "OFF (--no-cloudflare)" (explicit). `_cloudflare_flag` and the banner branch now carry the tri-state. - Codex (P1): `unsloth studio run` re-execs the studio venv's console script, which can be an older build whose --cloudflare defaulted on; omitting the flag let it re-enable the tunnel. That path now forwards the default polarity explicitly (--no-cloudflare, or nothing under --secure since --secure implies the tunnel). The plain `unsloth studio` path runs the same-version in-tree run.py (resolved via _find_run_py), so it keeps forwarding only an explicit polarity and still shows the accurate "(default)" banner. Tests updated for the tri-state banner labels, the None gate cases, and the new re-exec forwarding. * Studio: forward --no-cloudflare on plain re-exec too (mixed install) Codex follow-up: _find_run_py falls back to STUDIO_HOME/.../studio/backend/ run.py when the package copy is absent, so the plain `unsloth studio` re-exec can land on an older run.py whose --cloudflare defaults on. Forward the default polarity explicitly there too (--no-cloudflare, or nothing under --secure), matching the run subcommand. The common in-venv launch skips the re-exec and still shows the tri-state "(default)" banner. * Studio: fix launch hint - --cloudflare needs the wildcard bind Codex P3: the launch hint listed --cloudflare next to the loopback `unsloth studio -p 8888` command, but the tunnel only starts for wildcard binds, so `--cloudflare` alone on 127.0.0.1 does nothing. Show `-H 0.0.0.0 --cloudflare` in the hints (install.ps1, install.sh, studio/setup.sh) and clarify the same in the README. * Studio: cross-platform masked terminal password prompt helper Per-keystroke '*' echo (POSIX termios cbreak / Windows msvcrt.getwch), backspace editing, Ctrl-C abort, EOF handling, confirmation loop with re-prompt on mismatch or policy failure. Pure should_prompt gate for the --secure/--cloudflare exposure paths. * Studio CLI: force a terminal password change before public tunnel exposure When a launch will start the Cloudflare tunnel (--secure, or --cloudflare on a non-api-only wildcard bind) and the admin account still has its seeded bootstrap password, prompt for a new password in the terminal (masked with '*', confirmed, re-prompting until valid) before any re-exec or server exists. The change is committed in the parent so it never crosses argv or the environment and older studio-venv children see it immediately. Without a terminal, warn and fall back to the backend bootstrap shutdown timer. Mirrors backend update_password semantics in one transaction: rehash, rotate the JWT secret, clear must_change_password, revoke refresh tokens, drop the desktop secret, then remove the stale credential files. * Studio: terminal password gate before the public tunnel (backend backstop) Never publish a trycloudflare URL while the seeded admin password is active: run_server now runs a terminal password-change gate after the tunnel decision and strictly before start_studio_tunnel. Interactive refusal fails closed (shutdown + exit 1, mirroring the secure gate); without a tty it warns and keeps the bootstrap deadline. Success applies the same effects as the change-password route (update_password + revoke_user_refresh_tokens) and drops the stale app.state.bootstrap_password. MIN_PASSWORD_LENGTH centralised in auth/storage.py and referenced by the HTTP schema. terminal_prompt.py carries the pure gate helper (interactive loop stubbed; supplied by the masked-input module). Also migrates the studio/setup.ps1 launch footer that still showed the bare wildcard hint. * README: reconcile remote-access section with opt-in Cloudflare tunnel * Studio: harden the terminal password gate after review - run.py: run the gate BEFORE the uvicorn socket binds. On a wildcard --cloudflare launch the served HTML injects the bootstrap credential for first login, so a pre-gate listener would hand the default password to anyone who reaches the raw port while the operator is still typing. The gate now also seeds the admin row itself (it can run before lifespan startup). - Headless launches that nothing would protect now fail closed: the bootstrap deadline never arms for api-only serving and UNSLOTH_STUDIO_BOOTSTRAP_TIMEOUT=0 disables it, so warn-and-proceed would have promised a shutdown that never comes. Both the CLI and the backend refuse to publish in that case; the ordinary headless path still warns and relies on the 1h deadline, and no longer auto-fills the default credential into HTML served on a public URL. - storage.update_password gains revoke_refresh_tokens to delete the user's refresh tokens in the SAME transaction as the password commit; the change-password route and the backend gate use it (a separable follow-up delete could fail after the commit and leave a stale refresh token able to mint access tokens under the rotated secret). - clear_bootstrap_password is best-effort: a locked/undeletable file must not surface as a failed password change. - CLI masked reader: disable ISIG like the backend so Ctrl-Z cannot suspend the process with the shared terminal stuck in no-echo mode; handle Ctrl-C/Ctrl-Z as characters; treat stream EOF mid-line as an abort instead of submitting a partial password. Both readers restore terminal attrs from a SIGTERM/SIGHUP handler since a finally block cannot run when a default-disposition signal terminates the process. - Backend reader: decode byte-at-a-time through an incremental UTF-8 decoder so multi-byte characters split across read boundaries are no longer dropped; isatty checks tolerate closed/None streams. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: persist bootstrap suppression through lifespan startup The pre-bind password gate nulled app.state.bootstrap_password, but the FastAPI lifespan runs after it and re-reads the bootstrap password into app.state on both admin paths, so a headless public launch could still serve the injected credential in HTML. Carry a persistent suppress_bootstrap_injection flag that the lifespan honors instead. Also drop the quoted Tuple annotation on _terminal_password_gate that tripped the import-hoist lint (the typing import looked unused). * Studio CLI: keep the pre-exec auth DB private (0700 dir, 0600 db) On a fresh install the pre-exposure password gate creates auth/ and auth.db through the CLI before the backend ever runs, and sqlite3.connect leaves the DB 0644 under a 022 umask. Mirror backend storage.get_connection's chmod so the committed password hash and JWT secret are never world-readable, even if the launch aborts before the backend applies its own modes. * Tighten pre-exposure password gate comments * Studio: delete seeded bootstrap password before headless public re-exec The headless warn-and-proceed path returns with the default admin password still active, then re-execs a child Studio process. An old studio-venv child (mixed-version install) predates the pre-bind gate and its injection-suppress flag, so its lifespan reads .bootstrap_password and injects the seeded credential into the public HTML for up to the bootstrap deadline. A CLI-flag handshake cannot fix this uniformly: the studio run path uses ignore_unknown_options and an old in-venv child runs in-process, so it would never reject the flag. Delete the seeded .bootstrap_password file in the parent before re-exec so a fresh child of any version reads None and never serves it. This covers both re-exec paths and both child versions. must_change_password stays set, so the login page still forces a change and the bootstrap shutdown timer still arms; only the plaintext-on-disk copy is removed. Recovery is via a terminal-attached run or reset-password. Backend gate and CLI warnings updated to match. * Studio: commit the seeded admin before headless public re-exec The headless-warn path deletes the seeded .bootstrap_password so a re-exec'd child cannot inject it, but _ensure_cli_default_admin's INSERT was never committed and rolled back on conn.close(). On a fresh STUDIO_HOME an old studio-venv child then found no admin, regenerated a fresh bootstrap password + file, and injected THAT into the public page, defeating the deletion. Commit the seeded admin right after _ensure_cli_default_admin so any re-exec'd child sees the existing account and does not regenerate. Regression tests cover both re-exec paths on a fresh (unseeded) DB. * Studio: fail closed when the bootstrap password file cannot be removed On the headless public path, deleting .bootstrap_password is the protection against an old re-exec'd child injecting the seeded credential. If unlink fails (locked file, read-only auth dir) the file is still on disk, so warning and proceeding would still leak it for the bootstrap-timeout window. Abort with a clear error instead. Regression test covers the unlink-failure fail-closed path. * Studio: hold no-echo for the whole password line, not per keystroke The POSIX masked reader set cbreak/no-echo inside _getch_posix and restored the terminal to echo-on in a finally after every single keystroke, because _read_password calls _getch once per character. Between one char returning and the next call re-entering cbreak, ECHO was on, so a keystroke arriving in that window echoed the password in cleartext. Move the terminal mode into a _prompt_raw_mode context that _read_password holds around the entire line (mirroring unsloth_cli/commands/_password_prompt.py, which already did this), restoring once when the line completes or aborts. _getch_posix now only reads, since the mode is held by the caller. The context is a no-op when stdin is not a real terminal, keeping the _getch test seam. Add a regression test asserting the raw-mode context wraps the read exactly once and every keystroke is read while it is active. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: strip the seeded bootstrap password when the auth DB check fails The pre-exposure gate returned early on two auth-DB inspection failures and proceeded to re-exec without removing the seeded .bootstrap_password: - _connect_auth_db() failure: a seeded credential from a prior run may still be on disk. - the must_change_password read-back failure: worse, _ensure_cli_default_admin had already seeded the admin and the code committed it (writing .bootstrap_password) right before the failing SELECT. In the mixed-version case (a new outer CLI re-execing an old studio-venv child that predates the pre-bind gate), that child would read the file back and inject the default admin credential into the public Cloudflare page. The sibling headless branch already deletes the file for exactly this reason, so these returns were an inconsistent gap. Factor the delete-or-fail-closed logic into _strip_seeded_bootstrap_password_or_exit and call it on both inspection failures (and reuse it in the headless branch): strip the seeded file first (version-independent protection), failing closed if the removal itself fails. must_change_password stays set, so the login page still forces a change and the bootstrap shutdown timer still arms. Add tests for both new paths (connect failure and post-commit read-back failure strip the file and proceed; a failed strip fails closed). * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: fail closed when the seeded admin cannot be committed before exposure The pre-exposure gate wrapped _ensure_cli_default_admin (the INSERT), its conn.commit(), and the must_change_password read-back in one try, and the except recovered by stripping .bootstrap_password and proceeding to re-exec on the assumption the admin was already committed. That assumption only holds when the failing statement is the SELECT. When the INSERT or the commit itself fails (e.g. a write lock held past the busy timeout on a fresh install), no admin row is committed: it rolls back on conn.close(), and a re-exec'd old studio-venv child (no pre-bind gate) then finds no admin, regenerates a fresh bootstrap password + file, and serves that default credential on the public Cloudflare page. Stripping the file cannot stop a regeneration. Split the seed+commit into its own try that fails closed (refuse the public launch, best-effort removing any half-written seed file) since we cannot prove a committed admin; keep the separate read-back failure on the strip-and-proceed path, where the admin is committed so an old child finds it and will not regenerate. Add a test for the seed-commit-failure path. * Studio: decode the CLI masked password reader with errors="replace" The CLI reader read keystrokes with text-mode sys.stdin.read(1), which raises UnicodeDecodeError on a pasted non-UTF-8 password (e.g. Latin-1 bytes), or under PYTHONUTF8 yields a lone surrogate that later crashes the pbkdf2 encode -- either aborts the launch with a traceback. The backend mirror (terminal_prompt.py) already reads raw bytes through an incremental decoder with errors="replace". Mirror that here: read with os.read and an incremental decoder so invalid bytes map to U+FFFD, iterating over each emitted char (one byte can complete a replacement plus the next char). * Studio: resolve the child launcher before the pre-exposure gate The gate strips the seeded .bootstrap_password on a headless public launch, and it ran before the re-exec launchability check (studio venv / run.py / console script present). So a headless launch with an incomplete studio setup would seed the admin, delete the bootstrap password, then abort because the child could not be found, leaving the admin at must_change_password=1 with no password ever shown or injectable: locked out until `unsloth studio reset-password`. Resolve and validate the child launcher first, in both `studio` (studio_default) and `studio run`, and only then run the gate, so an unlaunchable setup exits before anything is stripped. Add a regression test that a missing venv exits without removing the seeded file. * Studio: fail closed when the auth DB cannot be opened before exposure The connect-failure branch of the pre-exposure gate stripped .bootstrap_password and proceeded, on the assumption a committed admin from a prior run made an old child find it and not regenerate. But on a fresh public launch whose _connect_auth_db() itself fails (transient lock during the schema/seed step, or an unwritable home), no admin is committed, so a mixed-version re-exec child that predates the backend gate can find no user, generate a fresh bootstrap password, and serve it on the public Cloudflare page. Stripping a file we cannot vouch for cannot stop a regeneration. Make this branch fail closed like the seed/commit failure path: we only continue past the DB inspection once a committed admin is confirmed. The existing file is left untouched so a retry (after a transient lock clears) can still prompt. Update the connect-failure test to assert fail-closed, and give the in-venv --secure flag test a real STUDIO_HOME with an already-changed admin so the gate is a no-op rather than relying on a DB-open failure. * Studio: invalidate seeded bootstrap files before deleting auth.db on reset reset-password deleted auth.db first, then best-effort unlinked the seeded .bootstrap_password and desktop secret. unlink() only ignores FileNotFoundError, so a locked or read-only file (Windows AV, read-only auth dir) survived while auth.db was gone. The next server start then re-seeded from that stale plaintext and re-validated the exact credential the reset was meant to revoke. Invalidate the credential files first, truncating any that cannot be unlinked, then delete the DB, so a surviving file can never carry a reusable secret. clear_bootstrap_password now truncates on unlink failure for the same reason, and its warning says the contents were cleared rather than claiming the stale password is already invalid. * Studio: require a servable frontend before the pre-exposure gate can strip the seeded password A headless public launch strips the seeded .bootstrap_password before the re-exec'd child starts. If the child then cannot serve the login page (the only in-band way to change the seeded password) the admin is locked out (must_change_password=1, no file, no UI) until reset-password. Add _require_servable_frontend_or_exit and call it before the gate on both `unsloth studio` and `unsloth studio run` public launches: fail closed if a non-api-only public launch has no built frontend dist, before anything is stripped. A user-supplied --frontend is validated to contain index.html so a bad path cannot silently bypass the check; an auto-resolved dist is trusted (_find_frontend_dist already requires index.html) and forwarded to the child. Model-load aborts on `studio run` remain a residual: the parent must strip for mixed-version safety (an old studio-venv child has no pre-bind gate) and model loadability cannot be proven before exec, so that path stays recoverable via reset-password. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: harden reset-password ordering and validate the in-venv backend before the strip Three follow-ups to the pre-exposure hardening: reset-password now deletes auth.db FIRST and proves it is gone before touching the seeded credential files. If the DB cannot be removed (a running Studio or Windows holds it open, or a read-only auth dir) it aborts with the credential files untouched, so a forgotten-password reset is not left half-done with the recovery credentials deleted while an un-resettable must_change_password=1 DB survives. After the DB is gone it invalidates the stale credential files (unlink, else truncate) and fails closed if a file can be neither removed nor truncated, since a surviving plaintext would be re-seeded by generate_bootstrap_password() and re-validate the revoked password. The in-venv (in-process) launch path had no analogue of the re-exec launcher check: a headless public launch would seed the admin and strip the seeded .bootstrap_password in the gate before _load_run_module() later failed on a broken/partial venv, leaving must_change_password=1 with no password to log in. Add _validate_inproc_backend_before_strip, called on the in-venv path (both `unsloth studio` and `unsloth studio run`) before the gate on the headless public path, so a broken backend fails cleanly before anything is stripped. It is scoped to the headless path so an interactive prompt is not delayed behind a full backend import. * Studio: validate the frontend and tunnel before the strip on every public path Five follow-ups closing the remaining pre-exposure-strip lockouts: The in-venv (in-process) paths of both `unsloth studio` and `unsloth studio run` validated the backend but not the frontend before the gate, so a headless public launch with a missing/bad dist would strip the seeded .bootstrap_password and then abort in run_server() during frontend setup, leaving must_change_password=1 with no login page. Both now validate a servable frontend before the strip (cheap check first, backend import after) and serve the resolved dist in-process. The `studio run` re-exec discarded the dist that satisfied the pre-strip check and only forwarded a user-supplied --frontend. In a shadowed install where the parent finds a built dist the child cannot, it stripped and exec'd without the path, and the child aborted during frontend setup. It now forwards the resolved dist, matching `unsloth studio`. On a headless --secure launch the bind is loopback, so the Cloudflare tunnel is the only public exposure. If cloudflared is provably unavailable (found nowhere and undownloadable) the tunnel cannot start, so stripping the recovery credential would just lock the user out with no public URL ever served. Add _tunnel_binary_confirmed_unavailable and, on --secure only, refuse the launch with the credential preserved rather than strip. Wildcard --cloudflare binds 0.0.0.0 publicly regardless of the tunnel, so it still strips; any uncertainty (helper not loadable) also still strips, since a possible credential leak outweighs a recoverable lockout. clear_bootstrap_password no longer claims it cleared the file's contents when both unlink and truncate failed; it now reports the stale password is still on disk and asks the user to remove it manually. * Studio: fix cloudflared probe path and skip the bootstrap strip for a self-suppressing child Two follow-ups to the --secure pre-exposure hardening: The cloudflared availability probe loaded cloudflare_tunnel by file path but not its backend deps: ensure_cloudflared() -> _cache_path() lazily imports utils.paths.storage_roots, which only resolves when studio/backend is on sys.path. From the outer CLI it is not, so the probe saw ensure_cloudflared() return None (cache unresolvable) and wrongly treated the tunnel as unavailable, refusing --secure even when cloudflared was cached or downloadable. Add the backend dir to sys.path for the probe (and remove it after) so the cache path resolves as it will in the child. A headless --secure launch stripped the seeded .bootstrap_password before the child proved the tunnel could actually connect, so a cloudflared that is present but cannot establish the tunnel (blocked connectivity, Cloudflare outage) left must_change_password=1 with no recovery credential. But the strip is only needed when the re-exec'd child is an OLD studio-venv backend with no pre-bind suppression: this install's own run.py sets app.state.suppress_bootstrap_injection before binding and never serves the seeded credential publicly. Add _child_self_suppresses (true in-process, or when the re-exec target is this install's own run.py by path identity) and skip the strip in that case, keeping .bootstrap_password as a local recovery credential; the strip stays fully in force for the studio-venv console-script path and any venv-fallback run.py, where an old child is actually possible. * Studio: reword the pre-exposure terminal password prompt * Studio: warn when -H is overridden by --secure; align pre-exposure prompt wording - --secure/--secure run: emit a Note (not an error) when -H is a non-loopback host, since --secure forces the loopback bind and would otherwise discard -H silently. - Reword the pre-exposure terminal prompt to 'exposed on the public internet' in both the backend gate and the CLI mirror. - Align the CLI success line with the backend ("Password updated for '<user>'."). - Tests for the new -H warning (present when overridden, absent on loopback). * Studio: add non-interactive --password to set the initial admin password Headless hosts (CI, containers, systemd units) have no TTY, so the forced first-exposure password change could not be completed unattended. Add a non-interactive way to set the INITIAL admin password before the server binds: - --password <value>, the UNSLOTH_STUDIO_PASSWORD env var, or --password - (read one line from stdin). Off by default; unset falls back to the normal interactive terminal prompt / browser setup. - Applies on any launch (public --secure/--cloudflare or a headless -H 0.0.0.0 bind), only when the account still has its seeded bootstrap password. An already-set password is a hard error, never an override; an invalid value (too short, or equal to the bootstrap) fails closed before bind. - The CLI applies the change in the parent, never forwards --password to the re-exec child, and strips UNSLOTH_STUDIO_PASSWORD from the child env so the secret never crosses to the child. run.py does the same on the direct path and strips the env var so spawned subprocesses (cloudflared, llama-server, tools) cannot inherit it. Mirrors resolve_supplied_password across the CLI and backend, documents the option in the README (including the argv-visibility caveat), and covers all flows (env/stdin/literal, fail-closed cases, no-forward, env-strip, reset-password roundtrip) in the CLI, backend, and unit suites. * Studio: truncate the stale bootstrap file when unlink fails on a CLI password change The post-change cleanup in _cli_update_password only warned when .bootstrap_password could not be unlinked but was still writable (locked file, read-only auth dir), leaving the old plaintext on disk. If auth.db is later reset or removed, generate_bootstrap_password() reads that file back and re-validates the revoked bootstrap password. Truncate the file on unlink failure so its stale plaintext cannot be re-seeded, mirroring the backend clear_bootstrap_password(); the password change is already committed, so this never rolls it back. The warning now states truthfully whether the contents were cleared or the file must be removed manually. * Studio: tighten comments --------- Co-authored-by: Daniel Han <danielhanchen@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
The edge fades toggle in Settings > Appearance let users swap the panel edge gradients for thin divider lines. It added little on top of the default look, so this removes the setting and all of its wiring while leaving the default edge fades in place. - drop the edgeFades field, default, and no-edge-fades class from the appearance customization store - remove the settings row, switch, and search entry - drop the html.no-edge-fades rules from index.css and hub.css - remove the edgeFades label and description from all locales - drop the edgeFades field from the personalization backend model and its test references
…OS dataset prep (unslothai#7087) * Studio: exclude /api/export/status from request access logs The frontend polls /api/export/status every 5s to detect export start, so it fires continuously even when idle. Each poll emitted an info request_completed access line, making up most of the server access logs. Add it to _EXCLUDED_PATHS alongside /api/train/status. The endpoint is unchanged; export state is still logged by the export modules and streamed over SSE, so no signal is lost. * Studio: collapse hub download-progress polls in the access log download-status and gguf-download-progress (plus the dataset equivalents) are polled about twice a second for the whole download, so each emitted an info request_completed line. Add them to _QUIET_POLL_PATHS so they collapse to one heartbeat line per 10s instead of one per poll. * Studio: log hub download progress at 10% steps The access log carried no real progress, only poll pings. Emit one hub_download_progress line per 10% step from the shared snapshot progress reader, so an active download shows actual percentage without a line per poll. Throttled per job and resynced if the same download restarts. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: drop successful chat thread/project CRUD from the access log A single chat turn fans out about twenty requests under /api/chat/threads and /api/chat/projects (list, fetch, per-message forks, and the message writes) that only reflect the UI re-rendering. Suppress their 2xx access line so the log keeps the signal (generation, tool calls, code execution, engine stats) and errors. Non-2xx on these paths still log. * Studio: silence transformers torch_dtype deprecation warning transformers logs "`torch_dtype` is deprecated! Use `dtype` instead!" once at model-config load via logger.warning_once (logging, not warnings), so a warnings filter cannot catch it. Attach a small logging.Filter in setup_logging, which runs before any model config is parsed, to drop that record on the transformers loggers that emit it. * Studio: quiet inference load-progress polls and log throttled load progress The frontend polls /api/inference/load-progress about twice a second for the whole model load, so each emitted a request_completed line. Add it to _QUIET_POLL_PATHS (heartbeat) and emit one inference_load_progress line per 10% step from the load-progress route, so a load shows real percentage instead of a line per poll. * Studio: fully suppress download/load progress poll access lines The download-status, download-progress, gguf-download-progress, active-downloads and transport-status polls (model and dataset), plus inference load-progress, fire ~2x/s for the whole download or load. Their progress is now reported by the hub_download_progress / inference_load_progress events (and the viewer's progress line), so the per-poll access line adds nothing. Drop it on 2xx and keep it on errors, instead of the prior 10s heartbeat. Chat CRUD suppression is folded into the same _is_quiet_success helper. * Studio: suppress training-tab model/dataset download-progress polls The training tab polls /api/models/download-progress and /api/datasets/download-progress about twice a second for the whole prep phase. These are separate routes from the /api/hub equivalents and only scan the cache, so their 2xx access line adds nothing (on Windows they always read 0 since the bytes live in snapshots/, not blobs/). Suppress the 2xx line and keep errors, alongside /api/models/gguf-download-progress. * Studio: drop transient pre-auth 401 on chat thread/project polls On first load the SPA fires chat thread/project GETs before the initial token refresh, so they 401 until /api/auth/refresh runs and the retries succeed. That pre-auth 401 is a bootstrap artifact, not an error; suppress it alongside the already-quiet 2xx line. Genuine 4xx/5xx on these paths, the download/load poll 401s, and all /api/auth/* still log. * Studio: quiet tab-switch list polls and per-poll scan/reconnect logs Switching between the Train, Export, and Chat tabs refetches list endpoints on a timer, and each hit re-logs internal detail. Heartbeat /api/train/runs, /api/models/checkpoints, /api/models/local and /api/rag/knowledge-bases (10s window, first hit and errors still log), and downgrade two per-poll INFO lines to debug: the checkpoints scan summary ("Found N training runs") and the per-reconnect SSE resume line. The meaningful "replayed N missed steps" line, logged only when steps were actually replayed, stays at info. * Studio: enable tokenizer parallelism for dataset prep on Windows/macOS TOKENIZERS_PARALLELISM was forced off everywhere to stop datasets' forked map() workers from deadlocking, but that fork only happens on Linux. On spawn platforms (Windows/macOS) dataset.map() runs in-process (dataset_map_num_proc returns None), so disabling tokenizer parallelism leaves the fast tokenizer single-threaded and dataset prep runs serially on one core. Keep it off on Linux (fork safety) and on for spawn platforms, where there is no fork to deadlock. Measured ~7x faster tokenization (12.5s -> 1.7s for 20k rows on a 32-core Windows box). * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: log throttled training status to the server log Training step/loss/epoch only went to the UI via SSE, so the server log showed inference engine_stats and train/runs heartbeats but nothing about the actual run. Emit one throttled training_progress line (step/total, percent, loss, epoch, eta) from the CUDA event pump: the first step, then at most every 30s, plus the final step, resyncing when a new run restarts the counter. Per-step UI streaming is unchanged. * Studio: quiet llama.cpp update-status polls and log throttled update progress The prebuilt llama.cpp update polls /api/llama/update-status about twice a second for the whole download and install. Suppress its 2xx access line (errors still log) and emit one throttled llama_update_progress line per 10% step from the status route, so the update shows progress without a line per poll. The existing "llama update: installing" and "llama update: success" events still bracket it. * Studio: quiet the export log-tail poll The Export tab polls /api/export/logs about once a second to stream the export subprocess output into the UI panel. Suppress its 2xx access line; the real progress is already logged as event-driven "Export subprocess status: <phase>" lines plus the subprocess start and checkpoint-loaded events, and errors still log. * studio: keep errors and mutations visible in access-log suppression Make the quiet-success access-log suppression GET-only so chat thread/project mutations (POST/PUT/DELETE) still log; only their list-poll 2xx and the transient pre-auth 401 are dropped. Suppress /api/export/status 2xx only (move it out of the all-status exclude set) so a 401/403/500 on it stays visible. Legacy /api/models and /api/datasets download-progress polls emit no hub_download_progress events, so heartbeat them via the 10s quiet-poll window instead of suppressing outright, keeping download visibility (notably on Linux). The event-emitting /api/hub download polls stay fully suppressed. Update and extend the middleware tests to cover GET-only suppression, the export-status error path, and the legacy download heartbeat. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * studio: tighten access-log and training-progress comments Comment-only pass: collapse the multi-line explanations in the logging middleware and the throttled training-progress logger to fewer lines while keeping the rationale. No behavior change. * studio: log structured export_progress phases Emit a structured export_progress event per phase (consolidated in the server log like training and download progress) instead of a plain status string, and add a phase milestone at the start of the heavy export step so the merge/save/convert is visible in the server log, not only in the forwarded stdout panel. * Studio: reset training-progress log throttle on each new run start_training rebuilds the per-run progress state but left _last_progress_log_ts/_last_progress_log_step at their prior values. A run started within 30s of a previous one whose last logged step matched the new run's first step would hit the step == prev short-circuit and drop the promised first training_progress line, then stay suppressed until the old 30s window expired. Reset both fields when a new job is accepted. * Studio: keep post-bootstrap chat 401s visible in the access log The chat thread/project 401 suppression dropped every GET 401 on those prefixes, so a genuine expired-session 401 vanished alongside the transient pre-auth race. Gate the 401 drop on a per-middleware bootstrap latch that flips once /api/auth/refresh first succeeds: before that the 401s are the pre-refresh race and are suppressed; after it any chat 401 is a real failure and logs. Add a test for the post-refresh case. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: limit chat access-log suppression to the exact list polls The chat thread/project suppression matched by startswith, so it also dropped the 2xx access line for detail and message reads (/threads/{id}, /threads/{id}/messages, /threads/{id}/messages/{id}, /projects/{id}) that are not the high-frequency list polls, losing their access and latency logging. Match the two list paths exactly instead, so only the intended list polls (and their pre-auth 401 race) are suppressed while detail and message reads keep their access line. Add a regression test. * Studio: reset inference load-progress throttle for each load The load-progress throttle (_last_load_progress_step) is a module global that persisted across loads, so a cached or small load whose first sampled /api/inference/load-progress response already reported fraction=1.0 hit step == prev (10) from a prior completed load and emitted no inference_load_progress line, while that endpoint's access log is suppressed, leaving the new load with no progress signal. Arm the throttle at load initiation in _load_model_impl so each load's first step always logs. Add a regression test. * Studio: tighten logging comments Collapse a few verbose comments (tokenizer-parallelism note, torch_dtype filter, legacy download-poll heartbeat, chat list-path suppression) to fewer lines without changing intent or code. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* Show concise NVFP4 inference load error * Cover direct NVFP4 load errors * Handle NVFP4 validation errors
# Conflicts: # studio/backend/main.py
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several critical improvements to the training and inference infrastructure, including support for distributed MLX training, a robust sidecar-swap mechanism for transformers upgrades, and enhanced security gates for folder browsing and model loading. Key changes include adding _MLXTrainerAdapter to support distributed training, implementing a _stop_watchdog to prevent zombie training processes, and adding security checks to is_denied_system_path to prevent unauthorized file access. I have also addressed a potential race condition in the training finalization logic where a failed DB write could incorrectly unclaim a new run's finalization status.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+1967
to
+1968
| with self._lock: | ||
| self._run_finalized = False # unclaim so a later flush can retry |
There was a problem hiding this comment.
There's a potential race condition here. If finish_run fails and a new training job starts before this except block acquires the lock, this code will incorrectly set _run_finalized = False for the new run. This could lead to the new run being finalized twice.
You should guard this write to only unclaim the finalization if the job ID is still the one this finalization attempt was for, similar to the logic in _finish_stopped_run.
Suggested change
| with self._lock: | |
| self._run_finalized = False # unclaim so a later flush can retry | |
| with self._lock: | |
| if self.current_job_id == run_id: | |
| self._run_finalized = False # unclaim so a later flush can retry |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
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" |
Comment on lines
+1241
to
+1242
| f"Error: password must be at least {_auth_storage.MIN_PASSWORD_LENGTH} " | ||
| "characters; not starting.", |
| 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
+223
to
+224
| "Note: --password is visible in the process list and shell history; " | ||
| f"prefer {SUPPLIED_PASSWORD_ENV} or --password - (stdin).\n" |
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#7140. Do not merge; closed after CI.