Commit graph

13 commits

Author SHA1 Message Date
Michael Han
a00fe86c13
Studio: read model text as utf-8 so umlauts survive on Windows (#7467)
* Studio: read model text as utf-8 so umlauts survive on Windows

Chat rejects or mangles non-ASCII on Windows: "ä ö ü" in a prompt, a chat
template, or a model path comes back as mojibake, or the load dies with
UnicodeDecodeError.

open() and Path.read_text() fall back to locale.getencoding() when no encoding
is passed. On Windows that is the ANSI codepage (cp1252, cp932, cp1251, ... by
system locale), never UTF-8. Hugging Face writes these files as raw UTF-8, so
every read of one decodes with the wrong codec:

- tokenizer_config.json, which holds the chat template. Templates routinely
  carry -> arrows, smart quotes and CJK, so this is the common path into chat
- config.json and adapter_config.json
- modules.json, Ollama manifests, and the .py sources the remote-code scanner
  reads before a model is allowed to load

The llama-server and embedding-server stdout readers have the same problem via
subprocess(text = True); they now decode utf-8 with errors = "replace" so a
stray byte cannot kill a log reader.

Encoding arguments only, no logic changes.

tests/test_chat_text_encoding.py covers a config.json and a chat template
holding umlauts, arrows and CJK, plus the remote-code scanner reading a source
file with umlauts. Those pass anywhere the locale is already UTF-8, so a fourth
test re-runs the readers under -X warn_default_encoding and fails on any
platform if an encoding argument goes missing again.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Studio: name utf-8 explicitly on the remaining text I/O, with an AST guard (#7465)

* Studio: name utf-8 explicitly on the remaining text I/O

Follow-up to the model-text reads in #7467, covering the rest of the backend:
system probes (nvidia-smi, amd-smi, powershell, git, node), package installers,
/proc and /sys readers, and internal marker files (pid, install id, bootstrap
password, Colab credentials).

Same reason as #7467. open(), Path.read_text()/write_text() and
subprocess(text = True) fall back to locale.getencoding(), which on Windows is
the ANSI codepage rather than UTF-8. These paths are mostly ASCII today, so this
is hardening, not a live bug. Encoding arguments only, no logic changes.

Adds tests/test_text_io_encoding.py: an AST guard walking every backend source
and asserting text I/O names its encoding, so the class of bug cannot creep back
in one call at a time. 275 files.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Catch aliased subprocess and positional Path.open, migrate legacy JSONL

The guard only matched a receiver literally named subprocess, so worker.py's
`import subprocess as _sp` hid three text = True installs that decode pip
output with the ANSI codepage. It also skipped any .open() with more than one
positional argument, though Path.open takes buffering/encoding/errors/newline
positionally.

Resuming a scrape written by an older release is the other half: those JSONL
lines are in the locale codepage, so the UTF-8 preload raised, the dedup keys
were silently forgotten and duplicates were appended to a now mixed-encoding
file. Decode with the locale codepage as fallback and rewrite as UTF-8 before
the append handle opens, since Windows cannot replace a file it holds open.

* Stream the JSONL preload and keep a torn line from relabelling the shard

Reading the whole shard to migrate it was wrong twice over. These files reach
gigabytes on a large scrape, so the preload now streams line by line and the
rewrite streams through a temp file.

Worse, one interrupted append used to condemn the file: the whole-file UTF-8
decode failed, every byte was retried as cp1252, and the rewrite persisted
mojibake over records that were fine. A line now counts as legacy only if the
locale codepage both decodes it and yields valid JSON, which a torn UTF-8 line
does not. Damaged lines are skipped and copied through byte for byte.

When the rewrite cannot be written at all, the append handle opens with the
legacy encoding rather than mixing UTF-8 into the file.

install_wheel takes run = subprocess.run as a parameter, so the guard cannot
see it. Both wheel installs there now name their encoding.

* Decide the shard's encoding from the file, not one line at a time

Some byte strings parse both ways. cp1251 `Р°` is D0 B0, which is also valid
UTF-8 for `а`, so a UTF-8-first parse quietly showed the wrong text instead of
migrating it.

A line now yields both readings, and the file decides. Any line that parses
under the codepage but not as UTF-8 is unambiguous evidence, and ambiguous lines
then follow that verdict, which is enough for any real shard: ordinary Cyrillic
or Japanese prose is invalid UTF-8 several times per line. Keys for ambiguous
lines are re-derived from the legacy reading during the rewrite.

A shard is undecidable only if every line is ambiguous, and nothing can tell
those apart.

latin-1 is also tried after the locale codepage, so a scrape carried from
Windows to a UTF-8 machine still has a reading rather than none. Requiring valid
JSON, not just a decode, keeps that from claiming torn lines.

* Weigh the whole shard, and never lose a record on the fallback path

One structurally valid JSON line carrying a stray 0x96 parses as cp1252, so a
single-line verdict let it relabel a healthy shard and mojibake every good
record in it. Each line with non-ASCII bytes now votes: parsing only under the
codepage is evidence for legacy, parsing as UTF-8 is evidence against, since
codepage text rarely forms valid multibyte UTF-8. Ties leave the file alone.

When the migration cannot be written the append handle uses the legacy codepage,
and errors = "replace" quietly turned characters it cannot hold into question
marks while write() still reported success. That path now escapes to \uXXXX
instead, which is ASCII, so every codepage holds it and json.loads returns the
exact characters. Nothing needs replacing, so errors = "strict" is safe.

stream_installer runs sys.executable, so its output is now decoded as UTF-8 by
utf8_child_env rather than read as the ANSI codepage.

* Only rewrite a shard we can attribute, and append ASCII when we cannot

latin-1 was doing too much work. It reads any byte, so it gave a moved shard a
reading, but it is the right text only for cp1252: cp1251 Привет came back as
Ïðèâåò and the rewrite made that permanent. The codepage is now trusted only
when it is the locale's, and an untrusted reading is never written back.

That leaves three cases where the file holds bytes UTF-8 cannot read and we are
not converting it: no codepage to attribute it to, ambiguous lines outvoting the
unambiguous ones, and a preload that could not read the file at all. All three
used to append UTF-8 into it. They now append pure ASCII, which every
ASCII-compatible codepage stores identically, so the file keeps decoding exactly
as it did and no record is lost.

Keys from the two readings are also kept apart. A damaged line in a healthy
shard was marked seen through its codepage reading, so the retry that would have
replaced the unreadable record was refused as a duplicate.

* Let the flash-attn install stub take the kwargs the installer now passes

_run_kwargs gained encoding and errors, so the one stub in this file that
spelled its signature out rejected the call. The other four here already take
**kwargs; this one now matches.

* Do not let a stuck temp file mask the migration failure

unlink() on the failure path could raise in its own right, on a stale
.utf8.tmp directory or a temp another process holds. That escaped the
constructor instead of returning False, so the caller never reached the ASCII
append fallback that keeps the shard single-encoding.

The pip fallback in install_wheel also spawns a Python child, so it gets
utf8_child_env like the probe above it already had. The uv and nvidia-smi
children are native binaries, where PYTHONIOENCODING would do nothing.

* Stop converting legacy shards; the encoding that wrote them is unknowable

trusted only ever meant that the bytes parse under this machine's codepage,
which for a single-byte codepage is nearly always true. A cp1251 shard opened on
a cp1252 Windows box decodes cleanly and would have been rewritten with Привет
as Ïðèâåò. That is the fourth way this rewrite could corrupt a shard, and the
common cause is that a file's encoding cannot be recovered from its bytes.

So the rewrite is gone. The shard is left exactly as found, and appends are pure
ASCII whenever it holds bytes UTF-8 cannot read, which is what actually
delivered the no-mixed-encoding guarantee the rewrite was added for. Dedup keys
still come from whichever reading parses, since ids are ASCII either way.

This also removes the temp file, so there is no longer any file mode or ACL to
carry across.

* Scan the sandbox shim; it is shipped code, not a build artifact

sandbox_site is on the sandboxed child's PYTHONPATH for every Python run
(tools.py:332, 2660), so excluding it let two unannotated text calls through in
code we ship. Both read and write the remap sidecar, which holds file paths.

The exclusion list is meant for build output only, so the directory comes off
it and the two calls name their encoding.

* Force the worker's pip children to UTF-8, and read DBCS keys with a DBCS codec

The three installer calls run sys.executable -m pip with an inherited
environment, so the parent decoded UTF-8 while the child emitted the ANSI
codepage. They now go through utf8_child_env like the other Python children.

Two tests asserted no env kwarg was passed as a stand-in for no HIP flag being
injected. They now assert the flag itself, which is the guarantee they were
written for and does not depend on how the env is delivered.

Separately, latin-1 cannot stand in for a double-byte codepage while recovering
dedup keys: cp932 表 is 95 5C, and the trail byte reads as a JSON backslash, so
the record failed to parse and its id was forgotten, appending a duplicate on
resume. cp932, cp936, cp949 and cp950 are tried too. The reading is still only
ever used for keys, which are ASCII and identical whichever codec parses.

* Require more than one legacy line before trusting its dedup keys

A shard whose valid records are all ASCII casts no UTF-8 votes, so a single
damaged line won the vote by itself, its key was remembered, and the retry that
would have replaced the unreadable record was refused.

One such line is genuinely undecidable: a legacy record with one accented
character and an ASCII record with one stray byte are the same shape. Reading it
as damage costs a duplicate; reading it as legacy loses the record for good.
Only one of those is recoverable, so it is now read as damage.

A real legacy shard has a legacy line for every record carrying an umlaut, so
its dedup is unaffected.

* Append ASCII whenever the shard already holds non-ASCII bytes

The gate asked whether any line was undecodable as UTF-8, which misses a shard
where every legacy line happens to be valid UTF-8 too. A cp1251 shard of Р°
records is bytes D0 B0 throughout, so appending 世界 as UTF-8 left a file where
cp1251 reads the old records correctly and the new one as mojibake, and UTF-8
does the reverse. No single decoding recovered the whole scrape.

The gate is now simply whether the shard holds any non-ASCII byte at all, which
covers both cases and is easier to reason about: if what is already there reads
differently under different encodings, do not add more bytes that do.

Appending ASCII costs only \uXXXX escapes, which json.loads turns back into the
exact characters, and it leaves the new record correct under either reading.

* Skip the two Linux-gated flash-attn tests off Linux

_should_try_runtime_flash_attn_install ends in sys.platform.startswith(
"linux"), and the threshold test one line above already asserts exactly that,
so the two tests that drive _ensure_flash_attn_for_long_context past the gate
cannot pass anywhere else: the call returns before it reports a status. They
were written on Linux and only surface once the suite actually runs on Windows
or macOS, where both fail on an empty status list. This PR is about making the
backend behave on Windows, so its own suite should be runnable there.

* Fail closed when a KFD topology node does not decode

This PR pins that read to utf-8, which turns an undecodable byte into
UnicodeDecodeError. That is a ValueError, not an OSError, so it slips past the
handler one line below and escapes a helper whose docstring promises to fail
closed on any unreadable node. The caller would then lose the whole HIP-order
map on a machine that has AMD GPUs, and the reason the helper fails closed is
that dropping a node shifts every later ordinal and lets a similar-capacity GPU
pass the total-size guard while showing another card's usage.

Widening the handler is the same one-line change main already made in #7487, so
the two agree and the eventual merge is clean.

* Tighten the comments added in this branch

* Treat an undecodable marker and undecodable metadata as malformed, not fatal

Two more places where pinning the decode changed the failure mode. A
UnicodeDecodeError is a ValueError, so neither `except OSError` nor
`except (JSONDecodeError, OSError)` catches it, and both sites had a documented
fallback that stopped being reached.

An undecodable .transport marker used to read as an unknown value, and the
caller then safely purged and restarted the partial download. It now aborts
prepare_cache_for_transport instead, so the transfer fails rather than retrying.

Undecodable .meta.json used to fall back to the file's own name, the same way
invalid JSON does. It now aborts URI construction for the entire unstructured
seed, so one corrupt byte in original_filename takes out the whole dataset.

Both handlers are widened, matching the KFD fix earlier on this branch.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Widen two more decode guards, and pin the kernel installer's pipe

Same shape as the ones already fixed here: the read was pinned to UTF-8 while
the handler around it still only catches OSError, and UnicodeDecodeError is a
ValueError.

hf_cache_snapshot_dir answers whether a model is already on disk, and the
offline embedding checks turn a raise into a 500. A torn refs/main used to
decode into a nonsense commit and miss the snapshot dir; it now skips that cache
root and keeps looking. _remove_pid_file runs first in _graceful_shutdown, so a
corrupt studio.pid raising there abandoned the inference, export, training and
tunnel children the rest of that function exists to kill.

ssm_runtime's source-build path builds its subprocess kwargs in a dict and
splats them through _run_with_heartbeat, so neither the encoding guard nor the
earlier sweep saw the text = True in it: pip's output was still decoded with the
Windows ANSI codepage, where a non-ASCII path or a compiler diagnostic mojibakes
or raises over an install that was going fine. It now pins the same
utf-8/replace pair install_wheel uses, and the HIP branch extends that env
rather than replacing it. The guard learned the dict-literal shape and reddens
on the old code (ssm_runtime.py:253).

* Tighten the comments around the UTF-8 text I/O pins

Collapse the multi-line rationales added with the encoding pins down to a
line or two each, drop what the code already says, and use one wording for
the repeated child-env note.

* Do not let an unreadable bootstrap password stop startup, and narrow the kwargs guard

ensure_default_admin calls _load_bootstrap_password for every existing admin and
the lifespan calls that with no handler, so pinning the decode turned a damaged
or pre-pin .bootstrap_password file into a backend that will not start. We write
that file ourselves in UTF-8, so a byte that will not decode belongs to a file
whose plaintext is worthless anyway; it now reads as no bootstrap password, the
same answer as an absent file. A readable one still loads.

The new kwargs check also judged every dict literal in the tree, so an unrelated
payload carrying "text": True would have been reported as subprocess
configuration with a misleading message, and a dict that fills in its encoding on
a later line would have been reported too. It now only judges a dict that
actually reaches a call, either splatted through a name or written at the call
site, and treats a later kw["encoding"] assignment as satisfying it. The
ssm_runtime shape it was written for is still caught, and a test pins both
directions.

* Stop reading a UTF-8 record a second time

_read_line always parsed the line under the codepage as well, even when it had
already read as UTF-8. Both callers take the UTF-8 reading when there is one and
never look at the other, so on a healthy shard the second parse is pure waste,
and this file reads all of one on every resume of a scrape it expects to reach
gigabytes. Measured on 200,000 records, 76 MB: 1.96s before, 0.81s after, so the
double reading was costing 2.8x.

The early return is limited to a record, since the key lookup deliberately falls
through to the codepage reading when UTF-8 yields something that is not one. A
line UTF-8 cannot read still tries the codepage, latin-1 and the double-byte
encodings as before, which is what the second reading is for.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Pin the scanned source fixture's line endings

test_remote_code_scan_reads_non_ascii_sources compared a file's contents against
the string it wrote, but wrote it in text mode, so Windows translated the line
ends on the way out and the read back differed by a carriage return. That is the
writer's doing, not the encoding the test is about, and it was the one failure on
the Windows runner that belonged to this branch. The fixture now writes with
newline = "" so the bytes on disk are the string on every platform.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Trim the newer comments to their point

Shorten the widened-guard and state store notes added since the last pass,
and collapse the line-ending note on the scanned source fixture.

* Read the scraper checkpoint as UTF-8 only, never as a codepage

A checkpoint holds nothing but base64 cursors and booleans, so one written by
an older locale-encoded release is byte-identical to a UTF-8 one and already
reads back. The codepage fallback can therefore only ever contribute non-ASCII:
if a single-byte reading of the file were all ASCII, the UTF-8 read would have
succeeded first.

So the only file it changes the answer for is a damaged one, and there it turns
a safe reset into a resume on a mojibaked cursor. GitHub answers that with
INVALID_CURSOR_ARGUMENTS at HTTP 200, gh_client returns the partial document,
and the scraper reads zero nodes and an empty pageInfo, which marks the stream
done. Every later resume then skips it entirely.

Reading UTF-8 only restores the earlier behaviour of dropping a checkpoint that
will not decode, which re-scrapes from the first page while the writers dedup
the replay. The shard scan below keeps its codepage reading; those records do
carry non-ASCII.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Gate the remaining tilelang install tests to Linux

_tilelang_platform_supported() returns False off Linux, so _ensure_tilelang_backend
returns before the install and the subprocess mock these six assert on is never
called. They fail on macOS runners for that reason alone. The rest of the file
already carries this marker; these were missed.

* Gate the Windows-incompatible worker and ROCm tests

Two different gates, because the production code has two. The causal-conv1d and
flash-linear-attention installers bail out on sys.platform == 'win32' alone and
run everywhere else including macOS, so those cases get not_on_windows; marking
them linux_only would skip tests that legitimately pass off Linux. The DRM and
KFD readers return early unless platform.system() is Linux, and their fixtures
build a fake sysfs tree needing PCI addresses like 0000:00:02.0 as directory
names, which Windows cannot represent, so those get linux_only.

The two visible-utilization cases failed for a different reason: on Windows
get_visible_gpu_utilization takes the AMD adapter branch ahead of the torch
fallback under test, and probing it imports torch, which the runner lacks.
Stubbing that branch empty leaves every other platform unchanged.

* Treat unparseable JSON nesting as a parse failure, and guard os.fdopen

json.loads answers nesting it cannot descend with RecursionError, a
RuntimeError, so _parse let it escape where the catch-all it replaced
discarded the record. Both callers run _parse outside any further handler,
so one damaged checkpoint or shard line aborted the scraper at startup.

The encoding guard also missed os.fdopen, which is open() on a descriptor
and takes the same locale default in text mode. It flags exactly the two
text-mode calls that were left unencoded; the swap lock file's reader was
already pinned to UTF-8 while its writer still used the codepage.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Write the non-ASCII source fixture without a 3.10-only argument

Path.write_text() only grew newline in 3.10, and pyproject declares
requires-python >=3.9, so this raised TypeError there. open() takes the same
argument on every supported version and pins the bytes on disk the same way.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Tighten encoding comments

* Follow subprocess calls through callable aliases in the encoding guard

---------

Co-authored-by: Unsloth <michaelhan@Michaels-MacBook-Pro.local>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: danielhanchen <unslothshared@gmail.com>

---------

Co-authored-by: Unsloth <michaelhan@Michaels-MacBook-Pro.local>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: danielhanchen <unslothshared@gmail.com>
2026-07-28 21:27:27 -07:00
Daniel Han
1781770bee
Studio: detect an interrupted dependency install instead of launching a backend that cannot import (#7492)
* Studio: detect an interrupted dependency install instead of launching a backend that cannot import

An installer killed part-way leaves a venv with a working CLI but without
studio.txt's dependencies. Nothing recorded that, so three separate places all
reported it healthy:

- the desktop preflight probed only `unsloth -h` (typer + rich) and a hardcoded
  desktop-capabilities dict, neither of which touches studio.backend, so it
  returned ManagedReady and spawned a backend that died on `import structlog`;
- setup.sh's fast path compared the installed unsloth version against PyPI,
  which matches on a half-built venv because unsloth is installed early, so
  `unsloth studio update` printed "up to date" and repaired nothing;
- start_managed_repair calls that update and then re-checks with the same blind
  probes, so Repair reported success without fixing anything.

install_python_stack.py now clears a completion manifest before the dependency
pass and writes it only after the final step. `unsloth studio verify-install`
and desktop-capabilities' new studio_install_ok field read it, the preflight
turns a false answer into ManagedStale so auto-repair runs, and setup.sh /
setup.ps1 gain an escape hatch next to the existing anyio one.

Separately, the wheel ships studio/ and studio.backend* but declared none of
their dependencies, so `unsloth train`, `export`, `chat`, `inference` and
`studio` all ended in a rich traceback after a plain pip install. structlog is
the only hard module-level import that chain reaches once starlette's
annotation-only import moves under TYPE_CHECKING, so it becomes a core
dependency and the rest of the server stack becomes a [studio] extra mirroring
studio.txt. The CLI import sites now report missing dependencies as a sentence
with two remedies.

Fixes #4701, #5260, #7147

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Match the trimmed comments merged on the pip branch

* Put the install manifest in the preflight fingerprint for PR #7492

The capability cache keyed the venv on pyvenv.cfg, uv.lock, requirements.txt,
the interpreter and site-packages/unsloth_cli/commands/studio.py, none of which
a repair touches when it only reinstalls studio.txt. So an entry cached while
the install was healthy stayed valid after the manifest was dropped, and the
probe returned Ready on exactly the half-built venv this is meant to catch.

* Address the review findings on PR #7492

Fail the install when the completion manifest cannot be written, instead of
exiting 0 without the record every later check requires, which is a repair
loop by construction.

Compare the version of the package the manifest names, so `studio update
--package X` does not read as a permanent version change.

Read the manifest from the venv that owns it when the CLI runs outside the
managed venv, and drop the dependency verdict in that case: the walk ran
against the wrong interpreter and says nothing about that venv.

Name the import that actually failed. `unsloth train` reaches torch through
the same guard, and the studio extra does not carry it, so recommending that
extra alone left the command failing in the same place.

* Declare click, which typer stopped providing, for PR #7492

unsloth_cli/commands/start.py imports click at module scope and
unsloth_cli/__init__.py imports that module, so every unsloth command needs
it. typer carried click through 0.19 and dropped it in 0.27, and the declared
floor is typer>=0.12.0, so a fresh resolve gets no click. On the published
wheel it still arrives because huggingface_hub requires click<9,>=8.4.2, which
is luck rather than a declaration. A wheel built from this branch's
dependency list has neither, and every command dies at import.

Verified: before, `unsloth --help` on a fresh venv raised ModuleNotFoundError
for click; after, it exits 0. The drift test now covers it.

* Keep a running backend from the previous app version manageable

The manageability bump gated two unrelated things through one constant. For
the managed CLI probe 2 is right: a CLI reporting 1 cannot answer
studio_install_ok. For a RUNNING backend it is wrong, because a process
already started cannot change what it reports, so bumping studio/backend/main.py
in lockstep does not help one the previous app version spawned.

That backend is proven ours by root id and ownership token, but
lifecycle_control_block_reason returned Unmanageable, and that branch never
calls adopt_verified_backend. has_owned_backend() stays false, so Repair falls
into block_external_conflict, which finds the same process and refuses: the app
could no longer stop a backend it owns the token for. The same regression in
backend.rs turned a terminal-launched same-root server from AttachedReady into
ExternalConflict.

Split the constant: DESKTOP_BACKEND_MANAGEABILITY_VERSION = 1 for the two
live-backend probes, DESKTOP_MANAGEABILITY_VERSION = 2 for the CLI probe. Every
real gate (protocol, auth, ownership, desktop-login, MIN_DESKTOP_BACKEND_VERSION)
is untouched, so an old backend still reaches OwnedStale, adopt, stop, repair.

Also stop the installer when the stale manifest cannot be removed. Windows
raises on a read-only or locked file, and the pass would then run behind a
marker that still names this version and these digests, so a run killed
part-way would verify as complete.

* Answer for the managed venv, not the one the CLI happens to run in

The guard matched ModuleNotFoundError.name, an import name, against
missing_requirements(), which returns distribution names. So a missing PyJWT
printed 'pip install jwt', and jwt, docx and fitz are each a real but unrelated
PyPI project (fitz is a neuroimaging workflow tool), so following the advice
installed the wrong package and left the backend just as broken. Map the import
to its distribution before deciding, and never offer the import itself.

install_state() verified the caller's own prefix. The wheel ships studio/, so a
CLI installed outside the managed venv always finds its own copy of the helper
first, and a healthy managed install reported studio_install_incomplete with a
missing list copied from the wrong venv. Selecting the root is not enough:
_installed_version() reads the running interpreter and req_root defaults to the
caller's studio.txt, so both checks still answered for the wrong venv. Hand
verify_install() that venv's own metadata, enumerated through
Distribution.discover(context = ...path), which does not fall back to sys.path.
The candidate order is untouched, so shadowed-tree detection is unchanged.

setup.ps1 replaces pip, torch and triton before install_python_stack.py runs,
so the manifest it drops is not dropped before the first mutation. A run killed
in between kept a marker that still verifies while torch was half-replaced;
drop it at the top of the dependency pass instead. setup.sh is unaffected, the
stack is the first thing its pass runs, and a test now pins both.

pip uninstall rewrites nothing that was fingerprinted, and cache_matches
re-reads the cached studio_install_ok rather than re-checking, so a venv that
lost a studio.txt package kept being served the healthy verdict. Fold a sorted
hash of the installed dist-info names into the marker hash.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* A missing manifest helper is a torn install, not an old one

studio/install_manifest.py ships in the same wheel as _studio_deps.py, so
nothing legitimately has one without the other: a CLI predating both never
reaches this code, and the desktop already calls such a CLI stale on
desktop_manageability_version.

Returning ok=true there reported a healthy install for a tree the package
update had half replaced, and the preflight then launched a backend whose
own run.py could be just as absent. Report it incomplete so repair runs.

* Tighten comments across the install-detection changes

* Validate Studio dependency readiness

---------

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>
Co-authored-by: Wasim Yousef Said <wasimysdev@gmail.com>
2026-07-28 10:57:20 +02:00
Daniel Han
a6aa4fff10
Studio: quiet noisy logs, log real progress, and speed up Windows/macOS dataset prep (#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>
2026-07-15 06:49:52 -07:00
Daniel Han
9a966adf51
Studio: trim serving-log noise and surface llama-server engine stats (#6377)
* Studio: trim serving-log noise and surface llama-server engine stats

Studio prints one structured line per HTTP request, so the SPA's polling and
per-invalidation fan-out bury the lines that matter.

- Dedup identical successful GETs within a short window (default 300ms,
  UNSLOTH_STUDIO_ACCESS_LOG_DEDUP_MS) so a burst logs once. The dedup key
  includes the query string, so distinct query-driven GETs are not collapsed.
  Runs after the response is sent, so it adds no request latency; mutations,
  non-2xx, and loading polls are untouched.
- Collapse pure-liveness polls (/api/health, /api/auth/status,
  /api/inference/status, /api/inference/monitor) to a longer heartbeat
  (default 10s, UNSLOTH_STUDIO_ACCESS_LOG_POLL_DEDUP_MS). The API monitor
  console polls /monitor every 1.5s while open.
- Translate llama-server's Prometheus /metrics into a periodic vLLM-style
  engine_stats line (generation/prompt throughput and requests in flight) from
  a daemon poller, gated on UNSLOTH_STUDIO_ENGINE_STATS. Throughput uses
  llama-server's predicted_tokens_seconds / prompt_tokens_seconds gauges, with
  a tokens_predicted_total / prompt_tokens_total counter-delta fallback; it does
  not use n_decode_total (which counts llama_decode() calls, not tokens). No KV
  field is emitted, since llama.cpp does not expose kv_cache_usage_ratio.
  --metrics is added only when probe_server_capabilities reports the binary
  supports it, so older/custom binaries still load. The poller keeps retrying
  through transient scrape failures (stop() drives shutdown) and a malformed
  sample cannot crash its thread.
- api_monitor.append_reply: once the preview cap is reached, skip the per-chunk
  re-concat (avoids O(n^2) on long generations) while still recording the "..."
  truncation marker for a reply that lands exactly on the cap.
- unsloth studio --verbose and unsloth studio run --verbose both restore every
  per-request log; --verbose before a subcommand is rejected with guidance
  (matching --secure / --parallel). run --verbose still forwards --log-verbose
  to llama-server, preserving the pre-existing pass-through verbosity.

* [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>
2026-06-17 05:37:57 -07:00
Wasim Yousef Said
31943d3c21
Studio: rewrite logging middleware as pure ASGI (#6337) 2026-06-15 17:03:00 +02:00
Daniel Han
187144d4e7
Reduce and tighten code comments and docstrings repo-wide (#6095)
Trim and tighten code comments and docstrings across the repository. Comment-only: every changed file verified code-identical to main via AST/token comparison.
2026-06-08 23:09:51 -07:00
Daniel Han
8292e699e4
Studio: make code comments and docstrings more succinct (#6029)
Trim and tighten code comments and docstrings across studio/ Python. Comment-only: every changed file verified code-identical to main via AST/token comparison.
2026-06-08 23:07:28 -07:00
Roland Tannous
c57a97958a
Studio: stop truncating long log lines as suspected base64 (#5335)
* Studio: stop truncating long log lines as suspected base64

filter_sensitive_data carried a heuristic from the original Studio
import that truncated any string >100 chars containing ',' or '/'
to value[:20] + '...'. The block was dormant until #5246 wired
filter_sensitive_data into the structlog processor chain to redact
native-path leases. Once active, the heuristic ate normal log lines
- llama_cpp_backend's GGUF size summary, mmproj selection, the full
llama-server command line, and any traceback containing a path -
all rendered as a 20-char prefix, defeating debugging of llama-server
exceptions and GPU selection.

Drop the base64 truncation. No call site in the codebase logs raw
base64; if one ever does, it should truncate at the source rather
than in a global filter. Native-path lease redaction added by #5246
is preserved.

* Studio: regression test for filter_sensitive_data truncation

Pins two properties in studio/backend/loggers/handlers.py:

1. Long log messages with ',' or '/' (the GGUF size summary, mmproj
   selection, full llama-server command, exception tracebacks) flow
   through filter_sensitive_data unchanged. Exercises the exact call
   sites that regressed when #5246 wired the processor in.

2. Native-path lease redaction still fires for both the inline
   native_path_lease=... regex form and the nativePathLease dict-key
   form, so a future cleanup of the truncation logic can't quietly
   strip #5246's redaction along with it.
2026-05-08 13:07:18 +04:00
Wasim Yousef Said
e35cbfb454
Add native GGUF intake to Studio (#5246)
* feat(studio): add Tauri native GGUF intake

* feat(studio): polish native GGUF intake

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix(studio): load backend helpers during local setup

* fix(studio): acquire native load lease before unload

* Studio: harden native path lease verification and Tauri intake

- Wrap path.resolve(strict=True) and Path.stat() in NativePathLeaseError so a deleted or unmounted GGUF returns 400 instead of leaking the full filesystem path through the generic load_model/validate_model handler.
- Re-apply _reject_network_or_device_path to the resolved canonical path for defense in depth after symlink resolution.
- Replace try/except ValueError pattern in the device-path guard with Path.is_relative_to; the previous shape silently swallowed NativePathLeaseError (which subclasses ValueError) so /dev,/proc,/sys were never actually rejected.
- Broaden the lease redaction regex and dict-key check (Python and Rust diagnostics) to cover both native_path_lease and nativePathLease so the camelCase form emitted by Tauri/frontend payloads is also redacted.
- Hoist the redact_native_paths import to module top in loggers/handlers; the recursive filter no longer pays a per-record import lookup.
- Persist activeNativePathToken in the chat runtime store so the rollback branch can mint a fresh lease and reload the previous native GGUF when a new load fails after unload; clear it in clearCheckpoint and overwrite it on each successful load.
- use-native-drop: read options through a ref so the Tauri onDragDropEvent listener is registered once and stays attached across option changes; reject ambiguous multi-file drops up front instead of silently registering only the first GGUF.
- pick_native_model: use an async pick_file with a tokio oneshot channel instead of blocking_pick_file so the Tokio worker is not held for the duration of the OS dialog.
- registerNativeModelPath: drop the duplicate sourceKind argument; the Rust command parameter is source_kind.
- install_python_stack: insert the script directory (studio/) on sys.path; the previous insert pointed at studio/backend/ which does not satisfy `from backend.utils.wheel_utils import ...`.

* install_python_stack: keep _BACKEND_DIR on sys.path

Restore the studio/backend insertion. Although the immediately following `from backend.utils.wheel_utils import (...)` is satisfied by studio/ already being on sys.path[0] when invoked as `python studio/install_python_stack.py`, wheel_utils itself runs `from utils.native_path_leases import ...`, which requires studio/backend/ to be importable. Without the backend insertion, the existing tests/python/test_install_python_stack.py collection fails with ModuleNotFoundError: No module named 'utils'.

* Studio: tighten native path lease lifecycle and Tauri intake IPC

- register_native_model_path now hardcodes NativePathSourceKind::Drop on the Rust side and the frontend stops sending source_kind. The previous JS payload (source_kind only) never reached the Rust deserializer because Tauri's default ArgumentCase::Camel maps the Rust parameter source_kind to the JS key sourceKind, so drag/drop registration silently failed. Hardcoding the source kind also keeps audit metadata trustworthy on this command.
- Add native_path_secret_removed_for_child_start context manager and wrap multiprocessing.Process.start() at the inference, export, training, and data-recipe job spawn sites. The previous wrapper-only scrub left UNSLOTH_STUDIO_NATIVE_PATH_LEASE_SECRET visible to spawn-platform import-time worker code. The wrapper run_without_native_path_secret stays as defense-in-depth inside the child.
- Stop passing exc_info=True from the native-grant load/validate error logs in routes/inference.py. The structlog filter_sensitive_data processor runs before the renderer, so ConsoleRenderer formatted tracebacks bypassed redaction; the redacted str(e) preserves the message text.
- Replace the os.path.normcase string equality on the resolved canonical path with Path.samefile (with a normcase fallback) so Windows leases that differ only in extended-length \\?\ prefix or short-name spelling are accepted.
- Wrap consumeNativePathToken in its own try/catch in the chat runtime rollback. If the previous native-model token has aged out of TOKEN_TTL we now surface a clear modelsError instead of silently swallowing the rollback inside the outer catch.
- Reject non-ASCII lease strings in _split_lease and convert UnicodeEncodeError / binascii.Error / ValueError raised by _b64decode into NativePathLeaseError so verify_native_path_lease never escapes raw exceptions to the route handler.
- Tighten dropStateForPaths to mark multi-file payloads invalid so the overlay matches the post-fix drop handler that rejects the same payload.
- Replace the one-shot fetch in useNativePathLeasesSupported with a delayed-retry loop so the picker/drop becomes available once the backend is up rather than staying disabled for the rest of the session after a transient failure.
- Drop the unused setActiveNativePathToken setter; the value is set via setState directly in use-chat-model-runtime.
- Add a toast on auto-load failure in use-native-drop so a collapsed model selector does not hide the error.
- Burn the lease nonce before _validate_current_stat so a stat-failed lease is single-use even if a later state change happens to match the original size/mtime.

* Studio: cache lease secret, harden native path stat checks, polish intake UX

- Cache the decoded UNSLOTH_STUDIO_NATIVE_PATH_LEASE_SECRET on first verify and validate that it is base64-decodable and at least 32 bytes. Subsequent _decode_secret calls return from the cache and never touch os.environ, so concurrent /api/inference/load and /api/health requests no longer race with native_path_secret_removed_for_child_start scrubbing the env. native_path_leases_supported now wraps _decode_secret so the health flag matches what verify_native_path_lease actually accepts.
- Replace path.is_file()/is_dir() + path.stat() with os.lstat() in _validate_current_stat and explicitly reject S_ISLNK; size and mtime checks now refer to the link itself, closing the same-size+same-mtime symlink-swap window that the prior follow-symlink stat() left open.
- Add an issued_at_ms < expires_at_ms sanity check in _validate_payload to reject internally inconsistent (HMAC-protected) lease payloads.
- Sort _NATIVE_PATH_REDACTIONS by length (descending) before iterating in redact_native_paths so a longer registered path is replaced before a shorter prefix path; otherwise logs containing /foo/X.gguf.bak after only /foo/X.gguf was registered would leak the .bak suffix.
- classify_existing_path now re-checks the canonical path with symlink_metadata after canonicalize, so a regular file that is replaced with a symlink in the small canonicalize window is rejected at registration.
- ModelSelector renders the local file picker as its own block (not in the eject ternary), so a user with an active model can still replace it via the picker rather than only via drag/drop.
- useNativePathLeasesSupported caps the readiness probe at MAX_READINESS_POLLS (60 = ~5 minutes) and aborts the in-flight fetch on unmount via AbortController, so a permanently-disabled backend stops generating sustained traffic and hot-reload no longer leaks open connections.
- useChooseNativeModel returns a stable useCallback closure and guards the OS dialog with a useRef so rapid double-clicks cannot open multiple dialogs and orphan Rust tokens.
- Branch the multi-file drop toast: if no GGUF was present we say "Only .gguf model files can be dropped here." and otherwise "Drop a single .gguf model file." so users dropping non-GGUF attachments get an accurate explanation.

* native_path_leases: lstat the signed canonical path before resolving

The earlier change to lstat inside _validate_current_stat operates on grant.canonical_path, which is the post-resolve target. If the user atomically replaces the originally-signed file with a symlink to a different file of identical size and mtime, path.resolve(strict=True) follows the symlink, samefile returns True (both ends share the new inode), and the lstat in _validate_current_stat sees the regular target file rather than the symlink, so the swap goes undetected.

Add an os.lstat on the signed canonical path before path.resolve(strict=True), and reject S_ISLNK there. The lstat in _validate_current_stat stays as defense-in-depth for swaps that occur strictly between resolve and stat.

* Studio: scrub native lease secret before mp.Queue spawn and tighten lease lifecycle

- Move _CTX.Queue / _CTX.Event / _CTX.Process construction inside native_path_secret_removed_for_child_start at the inference, export, training and data-recipe spawn sites. The first Queue creation lazily spawns Python's multiprocessing.resource_tracker child, so when it ran outside the scrub context the tracker process inherited the lease secret. Reproduced via the proc filesystem environ entry; the wrapped order keeps the tracker clean.
- native_path_secret_removed_for_child_start now refcounts entries: the env var is popped on the first entry and restored only when the last context exits. Concurrent training/inference/export starts no longer serialize on the env lock across the entire proc.start yield, while still guaranteeing the env stays empty for the duration of every overlapping spawn.
- run_without_native_path_secret now also nulls the module-level cached lease secret. With the existing spawn-only multiprocessing context the cache is irrelevant in practice, but a future fork caller would otherwise inherit the in-memory secret even though the env var was scrubbed.
- filter_sensitive_data now applies the native lease key check on the top-level event_dict, not only on nested dicts, so a logger call that includes a lease value as a top-level keyword field actually redacts it (the bare value does not match the prefix-anchored regex).
- chat-page loadNativeModelIntent now passes intent.id to clearModelIntent so a second drag-drop during an in-flight first auto-load is not wiped from the chip area when the first resolves.
- Bump useNativePathLeasesSupported's MAX_READINESS_POLLS from 60 to 720 so first-run installs that compile llama.cpp from source or download large CUDA wheels (well past 5 minutes) don't permanently disable the native picker.

* native_path_leases: serialize first-decode against scrub context

_decode_secret used a separate _SECRET_INIT_LOCK from the env scrub's _NATIVE_PATH_ENV_LOCK, so the very first decode (before the cache is populated) could race a concurrent native_path_secret_removed_for_child_start and read os.environ during the env-empty window, raising "Native path grants require the managed desktop backend." Subsequent calls hit the cache and were already safe.

Acquire _NATIVE_PATH_ENV_LOCK around the env read inside _SECRET_INIT_LOCK and fall back to _SCRUB_SAVED_SECRET when the scrub has temporarily popped the env var. Lock ordering (init then env) is consistent with no other caller, so no deadlock.

* Studio: surface native model load errors and harden native path label cache

- Native model load and validate now bubble up the actual exception (with
  paths redacted) and apply the same friendly-error rewrite the non-native
  path uses, so users see "CUDA OOM", "trust_remote_code required", etc.
  instead of a generic "Failed to load native model: <label>".
- run_without_native_path_secret now also nulls _SCRUB_SAVED_SECRET so a
  forked grandchild that imports native_path_leases cannot recover the
  secret via the scrub-aware fallback in _decode_secret.
- _NATIVE_PATH_LABELS now has its own 10000-entry cap independent of the
  100-entry redaction list, so display_label_for_native_path no longer
  falls back to returning the raw canonical path after 101 native paths
  in one session. Redaction list keeps the 100-entry cap for log-scan
  performance.
- _validate_payload now also rejects null bytes in display_label, which
  is echoed back in HTTP responses and log lines.

* Studio: harden native path lease validation and chained native rollback

- child_env_without_native_path_secret now copies os.environ under
  _NATIVE_PATH_ENV_LOCK so a concurrent scrub-context env pop cannot
  raise RuntimeError: dictionary changed size during iteration in a
  background hardware scan or other env reader.
- _validate_payload and grant construction route every signed numeric
  field (version, issued_at_ms, expires_at_ms, size_bytes, modified_ms)
  through new _required_int / _optional_int helpers that wrap raw int()
  ValueError into NativePathLeaseError. The single upstream catcher
  produces 400 instead of 500 for malformed signed payloads.
- verify_native_path_lease now runs _validate_current_stat before
  _consume_nonce, so a transient stat error on the canonical path no
  longer permanently burns the nonce. Concurrent verifies still
  serialize through _consume_nonce, so single-use is preserved.
- Chained native model rollback now restores activeNativePathToken in
  the chat runtime store after a successful rollback loadModel. Without
  this, a second consecutive failed switch could not re-roll-back
  because the store token had been overwritten by the failed attempt.
- validate_model now applies the same not_supported_hints friendly
  rewrite to native model errors that load_model already does, so a
  native .gguf that fails validation with an upstream "is not supported"
  message gets the same actionable wording as the non-native branch.

* Studio: harden native path log redaction, status disclosure, and chip lifecycle

- structlog processor chain now runs format_exc_info before
  filter_sensitive_data so traceback strings are produced (and then
  redacted) rather than passed through as untouched (type, value, tb)
  tuples that the JSON or console renderer formats after the redaction
  filter has already finished.
- native_path_secret_removed_for_child_start clears _CACHED_LEASE_SECRET
  in addition to popping the env var, so a fork during the scrub window
  cannot inherit the cached bytes via the parent's heap. Parent verify
  calls during the window keep working through the existing scrub-aware
  fallback in _decode_secret.
- load_model's except ValueError handler now redacts native paths and
  uses the native model log label when native_grant_backed is true.
  Previously a ValueError raised after lease verification (e.g. from
  ModelConfig.from_identifier or downstream GGUF parsing) returned the
  raw exception string in the HTTP response body.
- llama_cpp_backend now records the native display label at GGUF load
  time, and /api/inference/status prefers it over the redaction store.
  After a Python backend restart the redaction store is empty; the
  attribute keeps the friendly label, and an absolute model_identifier
  with no other label source falls back to the basename so the canonical
  path no longer appears in active_model.
- reveal_path_token uses native "reveal and select" commands on macOS
  (open -R) and Windows (explorer /select,) so the file is highlighted
  in the file manager. Linux keeps the existing parent-directory open.
- Native model rollback that fails because the previous token cannot be
  consumed now throws a rollback-specific Error, and the outer empty
  catch was replaced with one that re-throws the rollback error. The
  rollback-specific message now reaches the user instead of being
  overwritten by the original load error message.
- NativeModelChip tracks the Rust token's expiresAtMs on a single
  setTimeout, disables the Load button at expiry, and relabels it
  "Select again" with an explanatory tooltip so users do not click into
  a guaranteed-failure path after the 15-minute TTL elapses.

* Studio: tighten native artifact policy, mmproj sibling check, and intake UX

- is_open_safe_artifact no longer grants Open for directories. Reveal
  already handles directory navigation, so the change closes the
  attack surface where a macOS .app artifact could be launched via
  open_path_token + open::that_detached.
- Display labels are sanitized in classify_existing_path. Control
  characters in filenames (newlines, tabs, NUL et al.) are replaced
  with spaces and the label is trimmed and capped, so a file named
  with embedded newlines cannot inject forged log lines or scramble
  the UI status panel.
- validate_entry_path skips the size_bytes/modified_ms equality check
  when the operation is Reveal or Open. Cloud-sync agents (Dropbox,
  iCloud Drive, OneDrive) routinely rewrite extended-attribute
  metadata which bumps mtime, and the user expects Reveal/Open to
  remain available for files in synced folders.
- llama_cpp_backend gains a _native_grant_backed flag at GGUF load
  success. /api/inference/status only applies the absolute-path
  basename fallback when that flag is true, so a non-native absolute
  local GGUF still reports its canonical model_identifier and unload
  by identifier keeps working.
- Native vision GGUFs now run through _validate_native_mmproj_companion
  before llama-server starts: the companion mmproj must be a regular
  file, not a symlink, and must live in the same resolved directory as
  the granted GGUF. This stops a hostile sibling or symlinked mmproj
  from being loaded under a single-file lease.
- Chained native rollback restructured: the rollback loadModel + state
  + refresh runs inside its own try/catch that swallows so the outer
  throw error surfaces the ORIGINAL load failure. The native-token
  consume-failure case still throws the rollback-specific message
  early, before the inner block runs, so its actionable guidance is
  preserved.
- Loading-model state and the duplicate-load guard in the chat runtime
  hook now compare both the model id and the native path token. Two
  drops or picks with the same basename in different folders no longer
  silently dedup; the second token is honored.
- chat-page loadNativeModelIntent awaits selectModel before clearing
  the pending intent. If selectModel returns early via dedup or
  throws, the chip and its token stay so the user can retry instead
  of losing the selection.
- NativeModelChip's Reveal button is disabled when the lease has
  expired (Rust would reject it anyway), and the Load button label
  reads "Expired" instead of "Select again" so the disabled element
  no longer promises an action it cannot perform.

* [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>
2026-05-04 11:46:18 +02:00
Wasim Yousef Said
a5eb2e3d50
Add tauri (#5144)
* add unsloth studio desktop app

* Fix review findings

- studio/src-tauri/tauri.conf.json: retarget updater to staging repo
  (danielhanchen/unsloth-staging-2); switch to unslothai/unsloth on upstream merge.
- studio/src-tauri/linux/postremove.sh: drop the interactive read loop and the
  /home/* iteration. Package maintainer scripts must stay non-interactive and
  must not touch other users' data.
- studio/frontend/src/app/auth-guards.ts: honor tauriAutoAuth() boolean. Failed
  auto-auth now redirects to /login; requireGuest/requirePasswordChangeFlow
  only redirect to /chat when auth succeeds. The new early-return on failed
  auth is intentional so the login / change-password flows remain reachable
  when desktop auth is not yet established.
- studio/frontend/src/config/env.ts: keep fetched=false on health failure so
  later calls retry instead of caching the client-side platform guess.
- studio/src-tauri/src/install.rs: pick the available system package manager
  (apt-get, dnf, zypper, pacman); AppImage bundles run on non-Debian distros.
- studio/frontend/src/lib/open-link.ts + markdown-text/sources callers: return
  boolean from openLink so callers only preventDefault on handled URLs; relative
  hrefs now navigate natively.
- studio/frontend/src/features/settings/tabs/about-tab.tsx: fetch(apiUrl(...))
  so the version request targets the backend port in desktop mode. The bare
  /api/health predates the Tauri webview (blame: the earlier onboarding commit,
  which ran with same-origin frontend/backend); in desktop mode the webview
  origin is tauri://localhost so the bare path fails.
- install.ps1: gate the install_python_stack.py hotfix on a sentinel comment
  instead of a content regex; append the sentinel after applying so reruns
  are unambiguous.
- unsloth_cli/commands/studio.py _write_auth_secret: use the atomic mkstemp +
  os.replace path on Windows too; chmod calls are wrapped in try/except OSError.
- studio/src-tauri/src/preflight.rs probe_existing_backends: fan out the health
  probes concurrently; desktop-auth status still runs sequentially per candidate.
  reqwest::Client is internally Arc-wrapped so the in-loop .clone() is a
  refcount bump, not a deep clone; annotated inline.
- studio/src-tauri/src/preflight.rs run_cli_probe: wait() after kill() to reap
  the child, matching probe_cli_capability.
- studio/src-tauri/src/process.rs + main.rs: add stop_backend_detached and use
  it from the tray quit handler so the 5s graceful-wait does not block the
  Tauri main loop. RunEvent::Exit keeps the synchronous safety-net call.
- studio/backend/main.py: drop the permissive localhost CORS regex in
  api-only mode; the explicit allow_origins list is sufficient.
- .github/workflows/release-desktop.yml: drop max-parallel: 1 so platform
  builds run in parallel, and lift releaseBody to an env var so the three
  tauri-action invocations share one source of truth.

* Fix review findings (loop 2)

- studio/backend/auth/storage.py update_password: clear_desktop_secret()
  alongside clear_bootstrap_password() so rotating the admin password
  also revokes any previously provisioned .desktop_secret. Without this,
  an old local desktop credential keeps minting fresh admin tokens via
  /api/auth/desktop-login after a password rotation.
- studio/src-tauri/src/desktop_auth.rs provision_desktop_auth: wrap
  cmd.output().await in tokio::time::timeout(30s). DESKTOP_AUTH_LOCK is
  held across the whole desktop_auth flow, and previously a hanging
  `unsloth studio provision-desktop-auth` subprocess would pin the lock
  indefinitely and freeze every subsequent desktop_auth call.

* Add review tests

* Consolidate review tests

Merge review-added tests into the existing studio/backend/tests/test_desktop_auth.py
(the PR's authoritative desktop-auth test file). Drops three scaffolding files under
tests/python/ in favor of five focused tests next to the tests they extend:
- test_update_password_clears_desktop_secret (runtime)
- test_update_password_on_unknown_user_leaves_desktop_secret_intact (runtime)
- test_cli_provisioning_delegates_to_storage_create_desktop_secret (source-level)
- test_cli_connect_auth_db_reads_storage_db_path (source-level)
- test_desktop_auth_provision_has_bounded_timeout (Rust source-level)

* Revert auth-guards.ts Tauri branches to unconditional form

The review loop on PR 5144 introduced a regression: the isTauri branch of
requireAuth redirected to /login when tauriAutoAuth() returned false, and
requireGuest / requirePasswordChangeFlow silently fell through on the same
condition. The Tauri desktop app authenticates via a local auto-generated
secret; it must never surface /login or /change-password to the user. A
failed auto-auth should let the startup layer retry, not expose a password
form.

Restore the three Tauri branches to the author's original unconditional
form (requireAuth: return; requireGuest / requirePasswordChangeFlow: throw
redirect({to: '/chat'})). Keep the rest of the review fixes -- the
apiUrl() fetch wrapping, authRedirect helper, and fetchAuthStatus refactor
are all legitimate improvements and are preserved.

* Revert release-desktop.yml to author's version

The review loop's workflow-file tweaks (drop max-parallel: 1, lift releaseBody
to an env var) are cosmetic. OAuth tokens cannot push workflow-file changes,
and fine-grained PATs cannot honor maintainerCanModify on a third-party fork.
Reverting the workflow file to wasimysaid's version lets the push go through
without needing a classic PAT with both repo and workflow scopes.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: Lee Jackson <130007945+Imagineer99@users.noreply.github.com>
Co-authored-by: Daniel Han <danielhanchen@gmail.com>
Co-authored-by: Daniel Han <unslothai@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
2026-04-23 04:50:10 -07:00
Roland Tannous
47654cb91c Final cleanup 2026-03-12 18:28:04 +00:00
Roland Tannous
817f2e8dcc feat: integrate structlog, configure workers for prod logging, and migrate print statements 2026-03-11 12:33:16 +00:00
Roland Tannous
544d6944d1 root studio folder 2026-02-02 09:13:49 +00:00