mirror of
https://github.com/unslothai/unsloth.git
synced 2026-08-23 15:53:46 +00:00
20 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Daniel Han
|
35672fc9b3
|
Studio: keep pandas out of the backend startup import graph (#8962)
* Studio: keep pandas out of the backend startup import graph * Hold the resolved pandas module in a global so a repeat call is not an import statement * Resolve the unstructured seed plugin on first use, not at startup Importing data_designer_unstructured_seed.chunking runs the package __init__ first, and that re-exports .config and .impl, which import the data designer engine, which imports pandas and pyarrow. Dropping the module-scope import inside chunking.py therefore left the whole cost in import main's graph wherever the plugin is installed: measured 534ms cumulative for routes.data_recipe.seed, with pandas and pyarrow both loaded. The route resolves the plugin through _chunking() on first use instead. None still means "not installed", so the existing 500 and the raw-text fallback are unchanged, and the failed probe is remembered rather than retried per request. The runtime guards in test_startup_defers_torch.py pass vacuously wherever the optional plugin is not installed, which is most CI jobs, so add a source-level guard that reads seed.py and fails on a module-scope import of the plugin either way. test_data_recipe_seed.py covers the availability cases: unavailable without the plugin on both preview entry points, probed once, raw text fallback, and the installed path. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the comments on the deferred imports * [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 * Tighten comments in the deferred pandas import paths --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: oobabooga <112222186+oobabooga@users.noreply.github.com> Co-authored-by: danielhanchen <unslothshared@gmail.com> |
||
|
Daniel Han
|
2748b15eb3
|
One interpreter leg on a pull request, and a floor lint that reads more than syntax (#9100)
* One interpreter leg on a pull request, and a floor lint that reads more than syntax A pull request ran 3.10 and 3.13. It now runs 3.13 only. Main still runs all four, so anything that needs a real run is caught at merge rather than never. The leg that goes is worth something, so this pays for it rather than dropping it. What a dropped floor leg actually stops catching is not syntax: it is reaching for a stdlib name that does not exist yet. core/research_runs.py already uses anext, which is 3.10, and that parses perfectly on every version and fails only when the line runs, so the existing ast.parse floor check would not have seen it. scripts/lint_backend_python_floor.py asks vermin instead, which reads syntax AND stdlib API availability, and takes its target from the workflow's own matrix rather than a number written in the script. Adding a call to itertools.batched, which is 3.12, fails it in seconds. It runs from workflow-trigger-lint.yml, which carries no paths filter, so it sees the pull requests that touch only backend source -- the ones that most need it now. The single leg has to be the NEWEST. Removals and deprecations land on the newest interpreter first and on the oldest never, so running only the oldest would be the wrong single choice; the guard asserts which end it is. What is genuinely given up, kept visible rather than deleted along with the old guard: the backend has version_info branches at 3.10, 3.11 and 3.12 boundaries, and a pull request no longer takes both sides of any of them. Nothing static covers that -- a parse reads both sides and runs neither. test_the_boundaries_the_subset_stops_executing_are_still_run_on_main lists them and fails if main ever stops running the full matrix, at which point this stops being a trade and becomes a straight loss. Mutation tested: making the single leg the floor fails one test, dropping the lint invocation fails another, removing vermin from the install fails it too, and taking Backend CI off push-to-main fails two. That vermin check needed a second pass: the first version looked for the string anywhere in the workflow and was satisfied by a comment mentioning it. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Scan the backend tree, not a list of packages I remembered The floor lint named core, utils and routes, and silently missed 116 shipped files: all of hub, plugins, models, storage, auth, picker and state, plus _platform_compat.py, which main.py imports directly. It also named "loggers.py", which is a directory, so that entry matched nothing at all. With the 3.10 leg dropped this lint is the only thing looking at the floor before a merge, and a check that covers most of a tree reads exactly like one that covers all of it. It now scans studio/backend and excludes only tests and vendored code, which takes it from 307 files to 422. Verified by putting an itertools.batched call, which is 3.12, into each of hub, auth, picker, state, storage, models, plugins and _platform_compat.py in turn: every one is caught now, and none of them was before. Widening it immediately found something real, which is the point: locale.getencoding is 3.11 and the floor is 3.10. It turns out to be correctly guarded, in a try/except AttributeError whose fallback is locale.getpreferredencoding(False), commented "Python < 3.11". vermin reads names rather than control flow, so a guarded attribute lookup is indistinguishable from an unguarded one. That file is exempt with its reason printed on every run, and an exemption naming a path that no longer exists fails the lint, so it cannot outlive the guard it was written for. The guard test counts what the lint would hand to vermin against what is on disk, so narrowing the input back to a package list fails rather than quietly shrinking coverage. * [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 * Suppress the guarded call, not the file it lives in Excluding state_store.py wholesale left everything else in it permanently unchecked, which is the package-allowlist mistake from the previous commit one level down: a new unguarded 3.12 call anywhere in that module would have passed the floor lint on a pull request that runs only 3.13. The suppression moves to the site. vermin honours a `# novermin` annotation, so comment parsing is on now and the one guarded call carries the annotation with a note saying the except below IS the guard and that vermin reads names rather than control flow. The file is back in the scan, which is 422 files again rather than 421, and adding an unguarded itertools.batched call elsewhere in it now fails. The coverage test no longer permits any file-level exemption at all, rather than permitting a recorded one, so reintroducing the exclusion fails it. Separately: the assertion that the lint step exists was only collected by Backend CI, whose paths cover its own YAML and studio/**, not .github/workflows/**. A pull request editing only workflow-trigger-lint.yml could therefore delete the step without failing anything, which is the one change the assertion exists to reject. It runs from that unfiltered workflow now, alongside the three guards already there for the same reason. * Scan unsloth_cli on the floor as well, since the matrix runs it studio-backend-ci lists unsloth_cli/** in its own paths filter and runs pytest unsloth_cli/tests as a step on every leg, so the 3.10 leg this replaces was executing shipped CLI code on the floor interpreter, not only backend code. A lint aimed at studio/backend alone covers part of that while reading like it covers all of it, which is the same shape as the package allowlist the previous round removed, one level up. ROOTS is now both trees and targets() walks each of them, 441 files rather than 422, and the run is still clean at 3.10. The guard asserts on what the lint would actually hand to vermin rather than on its source, and dropping unsloth_cli back out fails it. * Lint the test code at the floor too, since the matrix executes it The first version dropped tests on the theory that they are not shipped. Shipping is not the question, execution is: studio-backend-ci runs pytest tests/ from studio/backend on every leg, so a 3.11 API in a test file is executed by the 3.10 leg exactly as one in a shipped module is. With the pull request down to a single 3.13 leg, that leg and this lint would both pass and the failure would arrive on the push to main, which is the gap this exists to close. Only vendored code comes out now, pinned to its own support range. 1093 files rather than 441, still clean at 3.10, so this costs nothing today and closes the hole. Putting tests back into EXCLUDE_PARTS fails the new guard. * Run one interpreter and defend the floor statically The 3.10, 3.11 and 3.12 legs are gone from Backend CI, on pull requests and on main alike. Measured on one runner over the same tree, the four legs collected the same 26,320 tests and differed by exactly one: the >= 3.12 gate on test_demonstrates_the_underlying_stdlib_regression. 3.10 and 3.11 reported 26193 passed / 127 skipped, 3.12 and 3.13 reported 26194 / 126. That is 97 runner-minutes per push to run one identical suite four times and learn the value of a single skip marker, into a queue that has been observed 195 deep, and queue depth is wall-clock for every other workflow in the repo. What the older legs were really defending is that nothing reaches for a symbol newer than the floor, which is static. scripts/lint_backend_python_floor.py now checks exactly that, on every pull request, in seconds, across 1093 shipped and executed files, reading stdlib API availability rather than syntax alone. The floor is DECLARED, as PYTHON_FLOOR in the workflow, next to where the legs used to be. Deriving it from the matrix was right while the matrix ran several interpreters and becomes self-defeating with one: a 3.13-only matrix would move the floor to 3.13 and leave the lint asserting that code written for 3.13 runs on 3.13. 3.10 rather than the 3.9 pyproject.toml declares, because 3.9 is not true today. unsloth/models/_utils.py already uses dataclasses.dataclass(kw_only) and tempfile.TemporaryDirectory(ignore_cleanup_errors), both 3.10, so a 3.9 target fails on the tree as it stands. Either the declaration or those two call sites has to give, and that is worth its own change; this lint is what made the mismatch visible rather than what hides it. The cost is stated rather than buried. A static check does not run anything, so the sys.version_info branches in sitecustomize.py, native_path_leases.py, third_party_source.py and worker.py are now covered by reading and by the lint's view of the names they use, not by execution. The guard that used to assert main still ran them asserts instead that every file carrying such a branch is inside the lint's scan, since that is the only check left on them. * Keep executing the pre-3.12 branches, and pin the ceiling by name Two from review. The first is the honest objection to a 3.13-only matrix on push as well as on pull requests: a break in a supported older runtime path that uses no newer stdlib name passes the lint and is then executed nowhere. So the branches were counted rather than argued about. Seven backend files carry a sys.version_info comparison, at 3.10, 3.12 and 3.14. The 3.10 ones were never straddled even by the old matrix, whose oldest leg WAS 3.10, so every leg took the same side of them and dropping legs loses nothing there. 3.14 is above every leg there has ever been. What is genuinely lost is the pre-3.12 side of three files, and that is small enough to keep running: a second matrix entry on 3.11, the newest version that still takes that side, running those three files and nothing else. 57 tests in under seven seconds, beside the full leg rather than in front of it, so the critical path is the full leg either way. It is not a second copy of the suite, and the four legs it replaces are still gone. The second is that asserting the sole leg is merely above the floor let 3.11 or 3.12 satisfy it, which would give up the removals-and-deprecations coverage that is the entire reason the single leg is the newest one. The ceiling is now written down and compared by name, so moving it is a decision somebody makes and defends in the same change. Both new assertions fail when mutated: pointing the full leg at 3.12 fails the ceiling test, and pointing the spot-check leg at 3.13 fails the pre-3.12 test because it would then re-test what the full leg already covers. * [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> |
||
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> |
||
|
Daniel Han
|
1daaa5cbb4
|
Let a decode failure degrade instead of escaping a fail-closed helper (#7487)
* Let a decode failure degrade instead of escaping a fail-closed helper Pinning utf-8 makes a read that used to return mojibake on Windows raise instead. 33 of those reads sit under a handler catching OSError or json.JSONDecodeError but not UnicodeDecodeError, which subclasses ValueError, so a corrupt file would now escape a helper written to return a default. Adds UnicodeDecodeError to those tuples only. * Treat an undecodable install lock as stale instead of retrying forever |
||
|
Daniel Han
|
3fd948eb95
|
Pin utf-8 on shipping-code text I/O instead of the operator locale (#7486)
* Pin utf-8 on shipping-code text I/O instead of the operator locale 113 read_text/write_text/open call sites across unsloth, studio and unsloth_cli let locale.getencoding() decide the encoding. That is utf-8 on the Linux and macOS runners and cp1252 on a stock Windows install, so the same file decodes differently for a Windows user and silently produces mojibake or raises UnicodeDecodeError. Adds tests/test_runtime_text_encoding.py to keep it that way. It resolves openers through each file's own imports rather than a fixed list of module names, so an aliased tarfile.open or a local from PIL.Image import open is not asked for an encoding it does not take. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Scan tracked files only and resolve the unbound Path calling forms * Honour PEP 263 when scanning sources and migrate a legacy JSONL before appending * Scope guard imports lexically and only migrate a legacy file when it round-trips * Leave a legacy JSONL untouched and resolve path aliases in the foreign-opener check * Tighten comments --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
Michael Han
|
6d8c18cd1a
|
Replace standalone Studio wording with Unsloth (#7221)
* Replace standalone Studio wording with Unsloth Replace the single word Studio with Unsloth wherever it is used as shorthand for Unsloth Studio in docs, CLI output, UI strings, i18n locales, workflow display names, comments and docstrings. Kept unchanged: the full name Unsloth Studio, third party product names (LM Studio, Visual Studio, Mac Studio), feature names (Recipe Studio, Fine-tuning Studio and its translations), and all identifiers such as env vars, commands, paths and filenames. * Address review feedback on the Studio wording rename Use "an" before Unsloth where the rename left the article as "a". Restore the split brand where Unsloth and Studio render as two halves of the full product name: the onboarding sidebar subtitle and the IPv6 localhost warning. Scope two messages to the full name Unsloth Studio where plain Unsloth was misleading: the AMD README bullet and the CLI studio setup error. |
||
|
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. |
||
|
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. |
||
|
Daniel Han
|
3ce187da02
|
Formatting: ruff line-length 100, kwarg-spacing passes, drop blank after short local imports (#6079)
Raise ruff line-length to 100 and extend the local pre-commit format pipeline (def-signature magic-comma normalization, short multi-line assert collapse, kwarg '=' spacing, blank-line-after-short-import removal, adjacent string-literal / f-string+plain merge, redundant-pass pruning). Every transform re-checks the file AST and is dropped if it would differ; the whole-repo reformat is verified AST-identical per file and idempotent. |
||
Tai An
|
b364080225
|
fix(gh_client): fail fast on 401/403 auth errors instead of retrying forever (#5325) (#5329)
Some checks failed
Studio GGUF CI / Studio boots, loads a GGUF, answers a chat completion (push) Has been cancelled
Backend CI / (Python 3.10) (push) Has been cancelled
Backend CI / (Python 3.11) (push) Has been cancelled
Backend CI / (Python 3.12) (push) Has been cancelled
Backend CI / (Python 3.13) (push) Has been cancelled
Backend CI / Repo tests (CPU) (push) Has been cancelled
Backend CI / Backend ruff lint (non-blocking) (push) Has been cancelled
Frontend CI / Frontend build + bundle sanity (push) Has been cancelled
Studio Tauri CI / Tauri Linux debug build (no codesign) (push) Has been cancelled
Wheel CI / Wheel build + content sanity + import smoke (push) Has been cancelled
* fix(gh_client): fail fast on 401/403 auth errors instead of retrying forever (#5325) Fixes #5325. The Studio data-recipe GitHub Crawler swallows 401 Unauthorized (and 403 Forbidden without rate-limit headers) into the generic "network error" retry path, so a job with a stale or wrong-scoped GitHub token spins indefinitely emitting "Retry." lines until the user cancels. Changes: - Add GitHubAuthError. Raised on 401, and on 403 unless the response carries a clear rate-limit signal (Retry-After header for secondary limits, or X-RateLimit-Remaining: 0 for primary limits). - Track which token source resolved at construction time: explicit argument (recipe-level field), GH_TOKEN, or GITHUB_TOKEN. Surfaced in the error message so the user knows which credential to rotate. - Insert the auth-failure check before the existing 403/429 rate-limit branch in both .graphql() and .rest() so auth failures bypass the sleep-and-retry loop and abort the recipe immediately. Genuine rate limiting still retries via the existing path. requests.RequestException handling is unchanged because GitHubAuthError does not inherit from it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * style: apply black formatting per pre-commit.ci * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix GitHub auth failure handling Preserve GitHub token source through the repo seed scraper and fail fast on non-rate-limit auth errors while keeping genuine rate-limit retries. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Wasim Yousef Said <wasimysdev@gmail.com> |
||
|
Daniel Han
|
b09aa82a3a
|
Studio: add github_repo seed reader and GitHub Support Bot recipe (#5169)
* Studio: add github_repo seed reader and GitHub Support Bot recipe
Adds a first-party Data Designer seed reader that scrapes GitHub issues,
pull requests, and commits from one or more repositories via the GraphQL
API, and a learning recipe (GitHub Support Bot) that turns those rows into
synthetic support Q&A pairs for fine-tuning.
Backend (new plugin studio/backend/plugins/data-designer-github-repo-seed):
* GitHubRepoSeedSource config: repos, token (falls back to GH_TOKEN /
GITHUB_TOKEN env var), item_types (issues / pulls / commits),
per-resource limit (0 means all), max_comments_per_item.
* Rate-limit-aware GraphQL client (GitHubClient + RepoScraper) shared
across repos; flattens each item into a uniform row with columns
item_type, repo, number, title, body, state, author, created_at,
closed_at, url, labels, comments.
* Registered via the data_designer.plugins entry point.
Frontend:
* New seed_github block variant so the seed node card shows
"GitHub repositories" instead of the generic "Document file"
placeholder, with its own icon and inline summary (repo count +
item-type list).
* Rewritten seed dialog github_repo form: repos textarea pre-filled with
unslothai/unsloth + unslothai/unsloth-zoo, password input for the GH
token, items-per-repo number with an "All" toggle, and the noisier
options (item types, max comments, include comments) tucked under an
Advanced collapsible.
* Local model auto-load on Run: if a recipe uses an is_local provider
and the inference server is not already serving that model, the
executions hook calls /api/inference/load first. Removes the "open
/chat to load a model" prerequisite that users kept tripping on.
* Honor the recipe's run.rows value in the Run dialog (previously the
store reset to 5 regardless of what the template shipped).
Recipe (studio/frontend/src/features/data-recipes/learning-recipes/
github-support-bot.json):
* Defaults to the Local Model provider + unsloth/gemma-4-E2B-it-GGUF.
* Scrapes unslothai/unsloth and unslothai/unsloth-zoo, issues and pulls,
up to 100 items per resource.
* Two LLM blocks: normalized_question (llm-text) rewrites each thread
into a clean support question, support_answer (llm-structured)
produces JSON with answer / diagnosis_questions / cites / confidence.
* Run defaults to 10 rows for a quick smoke test.
Verified end-to-end on a running Studio: card renders, source-data
dialog is pre-populated, All toggle disables the limit input, the
recipe executes and produces rows against a loaded local GGUF.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* fix: improve GitHub recipe support
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: speed up GitHub scraper and harden the support-bot recipe
Addresses a perf issue found while demoing the github_repo seed reader:
Scraper is too slow at scale. The PRs GraphQL query pulls deeply nested
fields (reviewThreads, reviews, commits, timelineItems, etc.) so the
page size was pinned at 3 to stay under GitHub's node-count ceiling. 100
PRs meant 34 serial round trips. Added lighter query variants
(PRS_PAGE_QUERY_LIGHT, ISSUES_PAGE_QUERY_LIGHT) that drop the fields the
Studio flatten layer does not use (it only reads title, body, state,
author, labels, comments). With the light query PR pages can safely go
to 25 per page and issues to 50. The plugin scraper now passes
light=True to RepoScraper so Studio always uses the fast path; the heavy
query remains available for other callers.
Recipe defaults are now demo-ready with production knobs called out:
- max_parallel_requests: 1 and max_tokens: 800 so small local models
stay stable when running the support_answer structured column.
- support_answer prompt trimmed to 80-200 words so gemma-4-E2B GGUF can
actually comply with the schema. The canonical 150-300 word codex
prompt is still documented in the node3 markdown note for
production upgrades.
* Studio: rename GitHub recipe to 'GitHub Scraper' and add Easy mode
Changes the recipe framing from a single-purpose 'Support Bot' pipeline
to a general-purpose scraper that produces {user_request,
grounded_response} training pairs. Aligns with the canonical
github_data_gatherer dataset (11 enrichment tasks mirrored in pr_requests_20
/ issue_requests_20 on the input side and explain_pr / issue_fix_plan /
issue_solution on the output side).
Recipe JSON changes:
- columns[0] renamed normalized_question -> user_request, prompt now
inverts a GitHub thread into a realistic user ask instead of
normalising it.
- columns[1] renamed support_answer -> coauthor_response, emits
{response, followups, cites, task, confidence} and branches on
issue vs PR thread type.
- Notes rewritten to document the 11-task catalog and the canonical
production prompt to paste in for a full dataset backfill.
Frontend: Easy mode for github_repo recipes. The drag-and-drop canvas is
hidden behind an 'Advanced' tab; Easy mode is the default for any recipe
whose seed_source_type is github_repo. The Easy form reuses the existing
GithubRepoSeedForm (promoted to exported), adds a rows input bound to
previewRows, a model field bound to the model_config, and a single Run
button that calls runPreview() directly (no modal). Non-github recipes
see the same Editor / Runs tabs as before.
View mode persists per-recipe-id in localStorage under
recipe-studio:view-mode:<recipeId>.
* Studio: auto-detect server GH_TOKEN and widen Easy-mode detection
The GitHub seed form now fetches /api/data-recipe/seed/github/env-token
on mount and, when the server exposes a GH_TOKEN / GITHUB_TOKEN env var
and the token field is blank, shows a small 'Using server env var' badge
and swaps the placeholder text. The token value itself is never returned
to the UI.
Widens Easy-mode detection in recipe-studio-page.tsx so that recipes
saved before ui.seed_source_type was persisted also get the Easy tab:
falls back to recipe.seed_config.source.seed_type, which is always
present for github_repo seeds.
* fix: polish GitHub recipe UI
* Studio: default llama-server --threads to -1 (auto)
Previously we passed --threads only when the caller set an explicit
value, which meant llama-server fell back to its internal default.
That default has varied across llama.cpp builds (some versions use
hardware concurrency including hyperthreads, which hurts throughput on
CPU-heavy inference). Always passing --threads -1 pins the behaviour
to llama.cpp's auto-detect (physical cores).
Caller-supplied n_threads still wins when non-None.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: auto-switch Easy mode to Runs pane on run start
Easy mode had no progress island or canvas overlay, so after clicking Run
the only visible state was the button label flipping to "Running..." while
the screen otherwise stayed identical. This reads as stuck even though the
job is progressing.
Wire an onExecutionStart callback from recipe-studio-page.tsx through to
useRecipeExecutions so that when a run is kicked off from easy mode, the
page flips to the executions view where the Runs sidebar, progress bar,
rate/ETA panel, and live log are rendered. Advanced/editor mode keeps its
existing behavior and stays on the canvas (it already has the floating
ExecutionProgressIsland).
* fix: clean up GitHub scraper layout
* Studio: forward llm-structured output_format as llama-server response_format
Local GGUF runs of llm-structured columns used to generate the full
max_tokens budget before the prompt-level "return JSON in a ```json
fence" instruction got parsed. Small models (e.g. gemma-4-E2B-it)
routinely broke format, so each row took ~65s and frequently failed
with "No parsable JSON structure within ```json markdown fence".
For any local-provider model_config referenced by an llm-structured
column, clone the model_config and inject response_format into the
clone's inference_parameters. Uses llama.cpp server's flat shape
(tools/server/README.md):
{"type": "json_schema", "schema": <output_format>}
Not the OpenAI-nested form; data_designer's OpenAI adapter forwards
response_format verbatim via facade._COMPLETION_REQUEST_FIELDS, and
llama-server's documented schema path expects the flat variant.
The clone is per (model_alias, column) so:
- llm-text / llm-judge columns that share the same alias keep
free-form sampling.
- Each structured column gets its own schema, so columns with
different output_formats don't collide.
Effect on gemma-4-E2B-it demos: every row parses cleanly, and the
model terminates immediately after the closing brace instead of
running to max_tokens. Net wall-clock is usually faster even though
grammar-constrained sampling is slightly slower per token.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: flip Easy to Runs pane before validation scrape, not after
Previously onExecutionStart fired inside runExecution, which runs AFTER
validateRecipe() -- and validation re-invokes the seed reader. For the
github_repo reader that is a full GraphQL scrape, so the user sat on a
"Running..." button with an otherwise unchanged Easy form for 10-15s
before anything moved.
Call onExecutionStart at the top of runWithValidation, right after we
have a payload to send. The view flips immediately; ensureLocalModelLoaded
+ validateRecipe now run against the Runs pane instead of a frozen Easy
form. runExecution still calls onExecutionStart downstream, but the
callback is idempotent (the page's easy -> executions guard skips the
second call), so no behaviour change for runs that pass validation.
If validation fails the toast + runErrors path still fires; the Easy
form's error banner still reads runErrors when the user switches back.
* Studio: unify data-recipe workflow auth on sk-unsloth-* keys
The previous commit (
|
||
|
Daniel Han
|
3998f67680
|
Bump Data Designer to 0.5.4 (removes litellm dependency) (#4569)
* Bump Data Designer to 0.5.4 (removes litellm dependency) NVIDIA Data Designer v0.5.4 removes litellm entirely and replaces it with native OpenAI and Anthropic adapters. This follows the litellm supply chain incident where versions 1.82.7 and 1.82.8 were compromised with a credential stealer. Release notes: https://github.com/NVIDIA-NeMo/DataDesigner/releases/tag/v0.5.4 Changes: - Bump data-designer, data-designer-config, data-designer-engine to 0.5.4 - Sync data-designer-deps.txt with 0.5.4 engine requirements: - Added: chardet, fsspec, mcp - Removed: python-json-logger, pymupdf, pymupdf4llm, mammoth (these remain in the unstructured-seed plugin which still needs them) - duckdb constraint relaxed from <1.5 to <2 (upstream fixed record_batch) - Bump plugin lower bound to >=0.5.4 * Keep pymupdf, pymupdf4llm, mammoth in data-designer-deps The unstructured-seed plugin is installed with --no-deps, so its pyproject.toml dependencies are not auto-resolved. These three packages are needed by the seed route (studio/backend/routes/ data_recipe/seed.py) and must remain in the explicit deps list. |
||
Wasim Yousef Said
|
dd283b0605
|
feat(studio): multi-file unstructured seed upload with better backend extraction (#4468)
* fix(recipe-studio): prevent fitView from zooming to wrong location on recipe load * feat: add pymupdf/python-docx deps and unstructured uploads storage root * feat: add POST /seed/upload-unstructured-file endpoint * feat: add multi-file chunking with source_file column * feat: update frontend types and API layer for multi-file upload * feat: round-robin preview rows across source files Ensures every uploaded file is represented in the preview table by cycling through sources instead of just taking the first N rows. * fix: disable OCR, fix auto-load timing, fix persistence on reload - Disable pymupdf4llm OCR with write_images=False, show_progress=False - Replace onAllUploaded callback with useEffect that detects uploading→done transition (avoids stale closure reading empty file IDs) - Fix importer to preserve file IDs from saved recipes instead of clearing (clearing only happens at share time via sanitizeSeedForShare) * fix: harden unstructured upload with input validation and state fixes Validate block_id/file_id with alphanumeric regex to prevent path traversal, use exact stem match for file deletion, add error handling for metadata writes and empty files, fix React stale closures and object mutations in upload loop, and correct validation logic for unstructured seed resolved_paths. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix: address PR review - legacy path import, share sanitizer, sync effect Promote legacy source.path into resolved_paths for old unstructured recipes, clear source.paths in share sanitizer to prevent leaking local filesystem paths, and gate file sync effect to dialog open transition so users can actually delete all uploaded files. * fix: CSV column fix (BOM + whitespace + unnamed index re-save) for #4470 * fix: harden unstructured upload flow and polish dialog UX * [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> |
||
Roland Tannous
|
47654cb91c | Final cleanup | ||
|
Roland Tannous
|
a2baf80511 | Update license headers | ||
|
Shine1i
|
904e440513 | feat(studio): studio storage roots path utilities | ||
|
Roland Tannous
|
daa50d0756 |
Revert "Merge pull request #347 from unslothai/feature/studio-storage-roots"
This reverts commit |
||
|
Shine1i
|
5301514775 | feat(studio): studio storage roots path utilities | ||
|
Roland Tannous
|
d882678fe4 | Add AGPL-3.0 SPDX headers to all source files | ||
|
Shine1i
|
c88cce8185 | refactor(seed): package unstructured seed reader as local Data Designer plugin |