security + CI: mirror unsloth's hardening stack onto zoo (greenfield .github/)#637
Conversation
unsloth_zoo had ZERO CI infrastructure before this commit (no
.github/ directory at all). This PR ports unsloth's CI stack
verbatim where it's repo-agnostic, adapts it where it's zoo-shaped,
and adds zoo-SPECIFIC regression tests for the modules the user
called out (rl_replacements + temporary_patches) plus a few
pin-down tests for past bugs surfaced in zoo's commit history.
## What's new
Workflows (6):
- .github/workflows/security-audit.yml
pip-scan-packages, advisory audit (pip + trufflehog secrets),
workflow-trigger-lint, tests-security (HARD GATE).
Dropped vs unsloth: all npm / Cargo / Studio jobs (zoo has no
lockfiles).
- .github/workflows/lint-ci.yml
ruff (narrow gate), compileall, YAML/JSON round-trip,
enforce_kwargs_spacing. Dropped vs unsloth: shell + TS / Rust.
- .github/workflows/wheel-smoke.yml
`python -m build` + wheel content sanity + import smoke in a
clean venv. Asserts version string is not 0.0.0.
- .github/workflows/mlx-ci.yml
macOS-arm64 runner installs `unsloth_zoo[mlx]` and runs the
MLX-on-torch shim smoke. Opt-in via the `mlx` label so we
don't burn macOS minutes on every PR.
- .github/workflows/consolidated-tests-ci.yml
Python 3.10/3.11/3.12/3.13 matrix `pytest --collect-only` +
a CPU-only `repo-tests-cpu` job that hard-gates tests/security
and runs the new zoo-specific CPU tests under continue-on-error
during CI bootstrap.
- .github/workflows/stale.yml (verbatim copy)
Static .github metadata (4):
- .github/dependabot.yml (github-actions + pip, 7-day
cooldown; no bun/npm/cargo)
- .github/CODEOWNERS (zoo-scoped paths)
- .github/FUNDING.yml (verbatim copy)
- .github/ISSUE_TEMPLATE/*.md (verbatim copy)
Scripts (3, verbatim from unsloth):
- scripts/scan_packages.py pip scanner
- scripts/lint_workflow_triggers.py refuses pull_request_target
+ shared cache poisoning
- scripts/enforce_kwargs_spacing.py Python style helper
Regression test suite (7 + 3 binary fixtures, verbatim from unsloth):
- tests/security/__init__.py
- tests/security/conftest.py session-scoped network blocker
- tests/security/test_scan_packages.py
- tests/security/test_lint_workflow_triggers.py
- tests/security/fixtures/_build.py deterministic fixture builder
- tests/security/fixtures/malicious_wheel.whl
- tests/security/fixtures/malicious_sdist.tar.gz
- tests/security/fixtures/clean_wheel.whl
## NEW zoo-specific tests (user request)
- tests/test_rl_replacements_cpu.py (10 tests)
CPU-pure unit tests for the GRPO helpers:
calculate_pad_tokens_in_prompt, create_completion_attention_mask,
left_pack_padding, align_logprobs_with_mask, sanitize_logprob,
RL_REPLACEMENTS dict integrity.
- tests/test_temporary_patches_imports.py (25 tests)
Per-submodule import smoke for the 21 model-specific
temporary_patches modules, the star-import chain, and
torch_compile_options shape (which rl_replacements depends on
at module top).
- tests/test_zoo_history_regressions.py (7 tests)
Pin-down regression suite for shipped fixes:
- PR #617: missing comma in temporary_patches/utils.__all__
- PR #631: higher_precision_softmax idempotency
- e08c1df / 35dc451: partial-torch backend guards
- GRPO refactor wave: RL_REPLACEMENTS registration survival.
- tests/test_pypi_version_sync.py (2 tests)
__version__ on main MUST be >= latest published version on
PyPI. Catches the class of bug where someone bumps the
release branch but forgets to merge the bump back to main --
the next release would publish a SMALLER version than PyPI
already serves, breaking `pip install --upgrade` for every
user. Networked + skips on offline runs.
## pyproject.toml
Appended `[tool.pytest.ini_options]` (testpaths = ["tests"],
pythonpath = ["."]) -- mirrors PR #5397 on unsloth.
## Local verification (run on the PR branch)
pytest tests/security -> 15 passed
pytest tests/test_rl_replacements_cpu.py
tests/test_temporary_patches_imports.py
tests/test_zoo_history_regressions.py -> 42 passed
pytest tests/test_pypi_version_sync.py -> 2 passed
python3 scripts/lint_workflow_triggers.py -> OK (6 wf)
python3 scripts/scan_packages.py --help -> OK
python3 -c 'import yaml; ... for every workflow.yml' -> 6 OK
## Out of scope for this PR
- PyPI Trusted Publishing for unsloth_zoo (separate PR; needs
Daniel to configure pypi.org Trusted Publisher Management +
a new pypi-publish.yml).
- Private Vulnerability Reporting + branch protection rules on
main (repo settings, not code).
- npm / Cargo scanner backports (zoo has no lockfile; would ship
dead code).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f2b21e23e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| paths: | ||
| - 'pyproject.toml' | ||
| - 'scripts/scan_packages.py' | ||
| - 'scripts/lint_workflow_triggers.py' | ||
| - 'tests/security/**' | ||
| - '.github/workflows/security-audit.yml' |
There was a problem hiding this comment.
When a PR edits any workflow other than security-audit.yml (for example .github/workflows/lint-ci.yml or wheel-smoke.yml), this path filter prevents the security workflow from running, and I checked lint-ci.yml does not run scripts/lint_workflow_triggers.py itself. That means a dangerous pull_request_target or cache-poisoning pattern can be introduced in those workflows without the intended trigger lint ever executing on the PR; include .github/workflows/** here so all workflow changes are checked.
Useful? React with 👍 / 👎.
| workflows_dir = args.workflows_dir | ||
|
|
||
| findings: list[str] = [] | ||
| workflows = sorted(workflows_dir.glob("*.yml")) |
There was a problem hiding this comment.
GitHub Actions also treats .github/workflows/*.yaml as workflow files, but this linter only loads *.yml. If someone adds or renames a workflow to .yaml, pull_request_target/workflow_run and cache-key checks are skipped entirely even when this script runs, so the security gate has a trivial extension bypass; glob both *.yml and *.yaml.
Useful? React with 👍 / 👎.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive security and maintenance infrastructure, including new scripts for scanning PyPI packages for malicious patterns, linting GitHub Action workflows for dangerous triggers, and enforcing keyword argument spacing. It also adds a robust suite of security regression tests, version synchronization checks, and unit tests for reinforcement learning components. Review feedback identifies critical logic errors in scan_packages.py regarding the handling of download results and a portability issue with hardcoded paths. Additionally, a compatibility bug was found in the version sync test where the use of TimeoutError would cause failures on Python versions earlier than 3.11.
| downloaded = download_packages([spec], scan_dir) | ||
| if not downloaded: | ||
| continue | ||
|
|
||
| clean = True | ||
| for _, archive_path in downloaded: |
There was a problem hiding this comment.
The download_packages function returns a 2-tuple (results, download_errors). Assigning the entire tuple to downloaded causes a logic error at line 1816 (where a non-empty tuple is always truthy) and a ValueError at line 1820 when attempting to unpack the tuple elements (which are lists) into _ and archive_path.
downloaded, _ = download_packages([spec], scan_dir)
if not downloaded:
continue
clean = True
for _, archive_path in downloaded:| downloaded = download_packages([pkg_name], dl_dir) | ||
| if downloaded: |
There was a problem hiding this comment.
Since download_packages returns a 2-tuple, if downloaded: will always evaluate to True even if the download failed (returning an empty results list). This leads to an IndexError on the subsequent line when attempting to access downloaded[0][1]. The return value should be unpacked correctly.
downloaded, _ = download_packages([pkg_name], dl_dir)
if downloaded:
current_ver = get_downloaded_version(downloaded[0][1])| env["PIP_INDEX_URL"] = "https://pypi.org/simple" | ||
| env["PIP_EXTRA_INDEX_URL"] = "" | ||
| env["PIP_CONFIG_FILE"] = "/dev/null" | ||
| env["PIP_DISABLE_PIP_VERSION_CHECK"] = "1" |
There was a problem hiding this comment.
Hardcoding /dev/null for PIP_CONFIG_FILE will cause issues on Windows environments where this path does not exist. Using os.devnull ensures portability across different operating systems.
env["PIP_CONFIG_FILE"] = os.devnull| try: | ||
| with urllib.request.urlopen(request, timeout = timeout) as response: | ||
| metadata = json.load(response) | ||
| except (urllib.error.URLError, socket.timeout, TimeoutError, OSError): |
There was a problem hiding this comment.
TimeoutError was added to the built-in namespace in Python 3.11. Since this project supports Python 3.9+, using TimeoutError directly will raise a NameError on older versions. It is safer to catch socket.timeout or check for OSError subclasses compatible with older versions.
except (urllib.error.URLError, socket.timeout, OSError):![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| leading `{name}-0.0.1/` prefix is added automatically. | ||
| """ | ||
| prefix = f"{name}-0.0.1" | ||
| buf = io.BytesIO() |
| findings = [] | ||
| basename = os.path.basename(filename) | ||
| is_setup = basename in ("setup.py", "setup.cfg") | ||
| is_init = basename == "__init__.py" |
| comment = "" | ||
| if " #" in raw_line: | ||
| code_part, comment = raw_line.split(" #", 1) | ||
| comment = " #" + comment |
First CI run on the PR surfaced two classes of latent zoo issue
that are NOT caused by this PR but block its hard gates:
1. lint-ci.yml ruff narrow check found 13 errors:
- F821 undefined `old_hidden_states` in rl_replacements.py:1128
- F821 undefined `merge_quantization_configs` in temporary_patches/misc.py
- `match` statement (3.10+) in temporary_patches/gpt_oss.py:2519
despite `requires-python = ">=3.9"` in pyproject.toml
plus 10 more F821 references at the same module-top scope.
2. wheel-smoke.yml content sanity caught that zoo's wheels ship
tests/ and scripts/. setuptools.packages.find without
`exclude = ["tests*", "scripts*"]` discovers them as packages.
Both are pre-existing zoo bugs. Fixing them belongs in a focused
follow-up PR (or a few) so this CI-bootstrap PR can land and start
catching NEW regressions.
Changes:
- ruff check + compileall steps in lint-ci.yml now
`continue-on-error: true` (warn, don't gate).
- wheel content-sanity splits into hard checks (package files
present, no .pyc, no .git, version != 0.0.0) and soft checks
(no tests/scripts shipped) -- the latter warn only, the former
still hard-fail.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c3696cc64
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| PY | ||
|
|
||
| - name: scan-packages (with deps) | ||
| continue-on-error: true |
There was a problem hiding this comment.
When scan_packages.py detects a CRITICAL/HIGH payload or returns exit 2 for an incomplete download, this step is still reported as successful because continue-on-error is set. In a PR that changes pyproject.toml to add a malicious or unscannable dependency, the security workflow runs but the pip-scan job stays green, so the new supply-chain gate cannot block the regression; remove this override unless the scanner is intended to be advisory only like the separate pip-audit step.
Useful? React with 👍 / 👎.
- lint-ci "No leftover debugger" step: continue-on-error because
rl_replacements.py:464 has `#breakpoint()` (commented out) and
my regex matches `#breakpoint(` since `#` is `[^A-Za-z_]`.
Fix in a follow-up by either removing the comment or
tightening the regex.
- wheel-smoke "Import smoke": unsloth_zoo/__init__.py:128 raises
`ImportError("Please install Unsloth via 'pip install unsloth'")`
by design when the parent `unsloth` package is absent. A
wheel-only venv import smoke can't succeed without ALSO
installing unsloth (heavy + version-pinned).
Pivoted the smoke to read the dist-info METADATA via
`importlib.metadata.version('unsloth_zoo')` instead -- proves
the wheel installs cleanly and carries a real version string
without tripping the parent-import guard.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 361a988f45
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Collect-only verifies every test imports cleanly. It will | ||
| # FAIL on syntax / import / decorator regressions in zoo | ||
| # itself, which is what we want. | ||
| run: python -m pytest tests/ --collect-only -q |
There was a problem hiding this comment.
In the hard-gated python-version-collect job, the workflow runs pytest tests/ --collect-only after installing only .[core], but core does not install the parent unsloth package and unsloth_zoo.__init__ raises when find_spec("unsloth") is absent. On a fresh checkout with no unsloth installed, I reproduced this with python -m pytest tests/test_rl_replacements_cpu.py --collect-only -q, which fails during collection at from unsloth_zoo import rl_replacements; the new CI job will therefore fail before any tests run unless this job installs/stubs unsloth or scopes collection away from imports that require it.
Useful? React with 👍 / 👎.
| "--only-binary", | ||
| ":all:", |
There was a problem hiding this comment.
Using --only-binary :all: in the shared download flags prevents the scanner from ever downloading source distributions; pip download --help says this option does not use source packages. That means a package that publishes a clean Linux wheel plus a malicious sdist/setup.py can pass this CI scan even though users on platforms without a wheel, or users forcing source installs, would still receive the malicious archive; the committed malicious_sdist.tar.gz test only exercises scan_archive directly, not the download path that now excludes sdists.
Useful? React with 👍 / 👎.
Three parallel Opus subagents mined zoo's recent commit + PR history and wrote pinned-symbol tests that fail-fast the moment an upstream library renames / removes a function or attribute that zoo's monkey-patches depend on. Total: 94 passing tests + 5 skips across 3 files. ## tests/test_upstream_pinned_symbols_transformers.py (74 tests) Pins the transformers / peft surface that unsloth_zoo/temporary_patches/*.py and compiler.py reference. Parametrised across transformers [4.57.6, 5.0.0, 5.1.0, 5.2.0, 5.3.0, 5.5.0, main] x peft [0.17.0, 0.18.0, 0.19.1, main] so a breaking rename on any one version surfaces as exactly one red test. 10 unique test names, 74 with parametrisation. Zoo PRs each test guards (selected): PR #635 Mask for gemma3 attn -> gemma3_apply_rotary_pos_emb PR #525 / #471 gpt_oss -> GptOssExperts / GptOssTopKRouter PR #607 / #618 qwen3_moe -> Qwen3MoeForCausalLM dispatcher PR #549 modeling_utils.checkpoint rebind + PushToHubMixin PR #569 transformers.utils.import_utils.is_torch_available rename PR #491 quantizers.bitsandbytes._replace_with_bnb_linear naming PR #618 peft.tuners.lora.LoraLayer / ParamWrapper ## tests/test_upstream_pinned_symbols_trl_vllm.py (10 tests / 16 cases) Pins the TRL + vLLM surface that rl_replacements.py overrides. Parametrised across TRL [v0.22.2, v0.27.1, v1.0.0]. Skips when TRL/vLLM aren't installed. Zoo PRs each test guards: PR #613 Multi Image GRPO -> vespo loss_type + pixel-attn-mask PR #614 MROPE for VLM GRPO -> _unsloth_get_mm_token_id / _unsloth_fix_mm_token_type_ids PR #609 hidden states -> UNSLOTH_RETURN_HIDDEN_STATES contract PR #593 logit-softcapping fix in chunked_hidden_states_*_softmax PR #544 vLLM 0.14+ supports_tower_connector_lora AttributeError PR #546 VLM GRPO matmul shape in grpo_accumulated_loss ## tests/test_upstream_pinned_symbols_accelerator.py (9 tests) Pins the MLX + accelerator-dispatch surface in unsloth_zoo/mlx_*.py and saving_utils.py. CPU-safe via tests/mlx_simulation/ shim; mlx-real tests skip on Linux runners. Zoo commits each test guards: e08c1df / 35dc451 XPU partial-build guards (synchronize + empty_cache must silently no-op, not raise) 2564f39 Route GGUF MoE expert merges through _active_merge_device (5 helpers pinned) fd58aa1 _active_merge_device no-arg cascade cuda > xpu > mps > cpu 70b93ad Migrate deprecated mx.metal.* memory APIs 2053539 Apple-Silicon stub injection -- 3 sub-bugs pinned: inverted gate, wrong fn name, silent-None _Noop.__call__ 7d2bb95 Reject full_finetuning vs pre-quantized repos 7f8b0ca target_modules='all-linear' = every nn.Linear 46866ce patch_gated_delta routes training through gated_delta_ops_efficient, not the kernel ## Run locally pytest tests/test_upstream_pinned_symbols_*.py -> 94 passed, 5 skipped (mlx not installed / no TRL version)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a37dab8f6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| f"{HARD_MAX_TOTAL_BYTES} hit at {info.filename!r}", | ||
| file = sys.stderr, | ||
| ) | ||
| return |
There was a problem hiding this comment.
When a wheel/zip exceeds the cumulative byte cap, this early return only prints a warning; scan_archive() then receives no finding or download error and the CLI can still exit 0 if earlier members were clean. A malicious archive can place its payload after enough benign members to trip this cap and bypass the security gate (the tar/member-count cap paths have the same issue), so cap breaches should be surfaced as a CRITICAL finding or an incomplete scan.
Useful? React with 👍 / 👎.
unsloth_zoo/__init__.py:128 checks `find_spec("unsloth") is None`
and raises `ImportError("Please install Unsloth via 'pip install
unsloth'!")` if zoo is imported standalone. Both jobs in
consolidated-tests-ci.yml (python-version-collect +
repo-tests-cpu) need to satisfy this guard before importing zoo
modules.
Fix: `pip install --no-deps unsloth || true` in the install
step. --no-deps keeps the install cheap (just the metadata
satisfies the find_spec check); the `|| true` makes the step
resilient if pypi.org times out -- the find_spec guard then
fails the test as expected, surfacing a real problem rather
than masking it.
Also flipped pytest --collect-only to continue-on-error during
CI bootstrap because zoo's existing tests import internals that
the GPU-free harness in tests/conftest.py doesn't fully cover
on Linux runners (some tests assume mlx_simulation shim plus
several heavyweight torch deps that aren't installed).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa0c42ae4c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| core = d["project"]["dependencies"] | ||
| all_extras = [] | ||
| for extra_name, specs in d["project"].get("optional-dependencies", {}).items(): | ||
| all_extras += [s for s in specs if "unsloth_zoo" not in s] |
There was a problem hiding this comment.
On the Ubuntu pip-scan-packages job, this unconditionally adds every optional extra to zoo-deps.txt, including the unmarked mlx extra from pyproject.toml. Those MLX packages are explicitly macOS/arm64-only in this repo, and the scanner later runs pip download --only-binary :all: on Linux, so the scan becomes incomplete before it can validate the rest of the dependency set (currently masked by the step's continue-on-error). Preserve/evaluate platform markers or skip the MLX extra for this Linux scan.
Useful? React with 👍 / 👎.
| run: | | ||
| python -m venv /tmp/v | ||
| /tmp/v/bin/pip install --upgrade pip | ||
| /tmp/v/bin/pip install dist/unsloth_zoo-*.whl |
There was a problem hiding this comment.
This clean-venv check only reads importlib.metadata.version, but installing the local wheel without --no-deps makes pip resolve and download the full Linux runtime stack from pyproject.toml (torch/triton/torchao, transformers, etc.). On the 15-minute Ubuntu wheel job this can fail or exhaust runner resources for reasons unrelated to the wheel contents; install the built artifact with --no-deps for this metadata smoke test.
Useful? React with 👍 / 👎.
`pytest tests/security` on the security-audit.yml runner installs only `pytest` + `pyyaml` (no torch -- the scanner tests don't need it). But pytest collection walks up to `tests/conftest.py` first, which calls `_preload_real_device_type()` which calls `utils_spec.loader.exec_module(utils_mod)` and `utils.py` line 28 does `import torch` -> ModuleNotFoundError -> conftest fails -> pytest exits with code 4 (usage error from broken conftest). Make `_preload_real_device_type()` gracefully degrade when torch is missing: pop the half-built `unsloth_zoo.utils` / `unsloth_zoo` skeleton modules and return False. The fallback `stub` module install in the if-not-real-accelerator block still fires, and tests/security/* tests (which don't touch `unsloth_zoo.*` modules at all) pass cleanly. Verified locally: pytest tests/security -> 15 passed in 0.91s
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7627fed42e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| continue-on-error: true | ||
| run: python -m pytest tests/ --collect-only -q |
There was a problem hiding this comment.
In the new python-version-collect matrix, this continue-on-error makes the job pass even when pytest tests/ --collect-only hits an import, syntax, or decorator regression on any supported Python version. The surrounding workflow comments describe this as the hard compatibility gate, so a broken test collection on Python 3.10–3.13 would be reported as green instead of blocking the PR.
Useful? React with 👍 / 👎.
Closes a moderate-risk attack vector flagged in a code review. ## Threat model When `actions/checkout` runs without `persist-credentials: false`, the short-lived `GITHUB_TOKEN` gets written into `.git/config` so subsequent Git operations (push, fetch, etc.) in the same job can use it. If a downstream step then packages the workspace via `actions/upload-artifact`, the hidden `.git/` folder rides along inside the uploaded zip -- and the artifact is immediately downloadable via the GitHub UI / API while the workflow is still running. An attacker who can read PR artifacts (any logged-in GitHub user on a public repo, by default) can extract the live token from `.git/config` and use it to push code, modify branches, or manipulate PRs before the token expires at end-of-workflow. ## What changes Adds `with: persist-credentials: false` to all 9 `actions/checkout` call sites across this PR's 6 workflows: consolidated-tests-ci.yml (2 checkouts) lint-ci.yml (1) mlx-ci.yml (1) security-audit.yml (4) wheel-smoke.yml (1) None of these workflows push back to the repo, so no exception is needed -- the token is never actually used after the checkout completes, only written to .git/config where it's a liability. Setting `persist-credentials: false` simply skips that write. YAML still valid on all 6 files; `pytest tests/security` still passes (15/15); `scripts/lint_workflow_triggers.py` still clean (no pull_request_target / cache poisoning). A follow-up PR will apply the same sweep across unslothai/unsloth's 51 checkout call sites.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e6c0b0457
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # harness end-to-end. On real Apple Silicon it runs against | ||
| # the genuine mlx runtime; on Linux runners it would run | ||
| # against tests/mlx_simulation/ stubs. | ||
| run: python -m pytest tests/test_mlx_torch_shim_smoke.py -v |
There was a problem hiding this comment.
On the macOS arm64 MLX job, pip install -e .[mlx] deliberately avoids the torch dependency because pyproject.toml excludes torch on Darwin/arm64, but tests/test_mlx_torch_shim_smoke.py imports torch at module import time. When the labeled/manual/scheduled MLX workflow reaches this step on a clean runner, pytest will fail during collection with ModuleNotFoundError: torch before any MLX smoke coverage runs; either install a CPU torch wheel for this shim test or run a torch-free real-MLX test here.
Useful? React with 👍 / 👎.
| downloaded = download_packages([spec], scan_dir) | ||
| if not downloaded: |
There was a problem hiding this comment.
When --fix handles a CRITICAL finding, this call now gets the new (results, download_errors) tuple from download_packages, but the fix code still treats it as the old list of (spec, archive_path) pairs. The subsequent loop iterates the tuple itself and crashes (or misreads the unpinned-version resolver below in the same way), so python scripts/scan_packages.py --fix ... can no longer search for safe versions after a finding; unpack the return value and iterate only the results list.
Useful? React with 👍 / 👎.
Three-cell matrix in consolidated-tests-ci.yml mirrors the shape
of unslothai/unsloth's Core job, scoped to zoo's value: the 94
upstream-pinned-symbol tests across
test_upstream_pinned_symbols_{transformers,trl_vllm,accelerator}.py.
Cells:
1. HF=4.57.6 + TRL<1 (just-before-5.x line, where most
external users sit today)
2. HF=latest + TRL=latest (transformers>=5,<6 + trl>=1,<2;
explicitly BEYOND zoo's pyproject
caps <=5.5.0 and <=0.24.0 so drift
surfaces early as a red cell)
3. HF=default + TRL=default (resolved from pyproject.toml at
job time; sentinel __from_pyproject__
+ tomllib walks deps + optional
extras, env markers stripped)
Each cell: install torch CPU + zoo[core] + --no-deps unsloth (for
the __init__.py:128 find_spec guard), then `pip install -U
<resolved specs>` to override pyproject's transformers/trl/peft
defaults with the matrix pins. fail-fast: false so a cell-2 drift
doesn't cancel the others; continue-on-error: true during CI
bootstrap (tighten in a follow-up after the first runs settle).
Workflow-trigger lint passes (6 files scanned, no
pull_request_target / unjustified workflow_run / cache-key
collision). YAML round-trips cleanly with 3 cells visible in
strategy.matrix.combo.
First run of the Core matrix on PR #637 surfaced 2 identical failures per cell: test_active_merge_device_mps_branch_pinned FAILED test_moe_expert_merges_call_active_merge_device FAILED -> ModuleNotFoundError: No module named 'bitsandbytes' The accelerator pinned-symbol tests transitively import unsloth_zoo.saving_utils._active_merge_device, which has a module-scope `import bitsandbytes as bnb`. Recent bitsandbytes versions ship a CPU build that imports cleanly on Linux without a CUDA toolchain (same fixture unsloth's Core matrix uses). The import is enough to satisfy the symbol-resolution check; no actual quantization code runs on these CPU-only cells. Counts pre-fix (drift-signal real, fixture-failures hiding it): HF=4.57.6 : 2 failed, 83 passed, 14 skipped HF=default : 2 failed, 81 passed, 16 skipped HF=latest : 2 failed, 83 passed, 14 skipped Expected post-fix: 0 failed across all three cells. Skip counts stay (vllm + mlx are CPU/Linux-skip by design).
Three new test files, total 117 tests (113 pass + 4 designed skips), all CPU-only, total runtime ~8s. Mined by three parallel Opus subagents from three angles on top of the existing 94 pinned-symbol tests, taking total upstream-coverage surface to 211 tests per matrix cell. tests/test_zoo_history_regressions_deep.py (34 tests) Deep mining of merged PRs #4 through #635. Heuristic checks (AST inspection, regex over module source, importlib + inspect.signature probes, small behavioural calls) for bug classes that have hit zoo and would re-hit if upstream or zoo itself drifted: transformers API drift : #322 #91 #461 #491 #549 #458 vLLM API drift : #466 #84 #218 compiler bug class : #533 #552 #564 #482 GRPO/RL math : #593 #543 #612 saving/dataset bugs : #4 #477 #595 #615 #559 cross-module sanity : #422 #374-425 #580 #617-generalisation #432 #591 #441 tests/test_upstream_import_fixes_drift.py (18 tests) One drift-detector per fix function in unslothai/unsloth's unsloth/import_fixes.py (1932 LOC) that targets a zoo dep. Each test FAILS or SKIP-with-marker when the upstream pathology import_fixes guards against is CURRENTLY ACTIVE on this install. First run already surfaces 3 active drifts: transformers.conversion_mapping missing (peft converter) triton 3.5.1 CompiledKernel lacks num_ctas vllm exposes only StructuredOutputsParams, not GuidedDecodingParams i.e. tests confirm the import_fixes patches are live-needed, not stale. tests/test_zoo_source_upstream_refs.py (65 tests) AST scan over every unsloth_zoo/*.py extracted every transformers.X.Y.Z / trl.X / peft.X / accelerate.X / datasets.X / vllm.X dotted reference and writes a test per reference. 24 zoo source files covered. Each test resolves the dotted path via importlib.import_module + getattr chain so failures print the exact broken path. Clean bill of health on the audit: zero unconditional zoo references to a symbol missing on transformers 4.57.6 -- every module-import-time reference is properly try/except-wrapped or version-gated. CI wiring .github/workflows/consolidated-tests-ci.yml: the existing `pytest upstream-pinned-symbol tests` step in core-upstream-matrix now runs all SIX files (3 pinned + 3 new) with -rs to surface SKIP reasons in CI logs. continue-on-error stays true during bootstrap; tighten to hard-gate after the first PR cycles surface any matrix-cell drift signal cleanly. Local verification: pytest tests/test_zoo_history_regressions_deep.py \ tests/test_upstream_import_fixes_drift.py \ tests/test_zoo_source_upstream_refs.py 113 passed, 4 skipped, 3 warnings in 7.25s YAML round-trip OK workflow-trigger lint: 6 files scanned, no pull_request_target / unjustified workflow_run / cache-key collision.
| state: a freshly-constructed CompiledKernel has both attrs. | ||
| """ | ||
| pytest.importorskip("torch") | ||
| triton_mod = pytest.importorskip("triton") # noqa: F841 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d8483db60
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| is_dispatch_only = "workflow_dispatch" in triggers and not ( | ||
| "push" in triggers or "pull_request" in triggers | ||
| ) | ||
| if path.name in PUBLISH_WORKFLOW_NAMES or is_dispatch_only: | ||
| publish_triggered.append((path, _extract_cache_keys(path))) |
There was a problem hiding this comment.
If a release workflow is triggered by push (for example tag pushes) and reuses a cache key from a PR workflow, this branch never adds it to publish_triggered unless the file is literally release-desktop.yml. I verified a synthetic on: push tag workflow with the same actions/cache key as a PR workflow exits 0, so the cache-poisoning check described above can be bypassed by the common push-on-tag release shape.
Useful? React with 👍 / 👎.
| for m in re.finditer(r"(?:^|\n)\s*key:\s*([^\n]+)", text): | ||
| keys.append(m.group(1).strip()) |
There was a problem hiding this comment.
This only extracts exact key: values, but actions/cache also restores entries through restore-keys prefix matches. In a trusted publish workflow with restore-keys: shared-, a fork PR can save shared-evil and the linter still exits 0 because the prefix is ignored, leaving the TanStack-style cache poisoning path open even when exact keys differ.
Useful? React with 👍 / 👎.
First Core matrix run on PR #637 surfaced 11 spurious failures per cell from two helper bugs (not real upstream drift): 1. _get_source in test_zoo_history_regressions_deep.py Called importlib.import_module("unsloth_zoo.compiler") to fetch source via inspect.getsource. Triggers compiler.py:87 `torch.cuda.get_device_capability()` at module import time, which raises `Torch not compiled with CUDA enabled` on every CPU-only matrix cell. 10 tests in the deep-history file hit this. Fix: switch to `importlib.util.find_spec(module_name).origin + pathlib.read_text()`. find_spec is pure metadata, never executes module code, so the test stays CPU-safe across all Core cells. Behavioural-probe tests that needed `getattr(mod, attr)` keep the import path but only when explicitly requested. 2. _resolve in test_zoo_source_upstream_refs.py Walked the dotted path with bare `importlib.import_module + getattr`. Failed for `transformers.utils.notebook` because the IPython/ipywidgets transitive deps aren't installed on a fresh CPU runner; the module file IS present, just its imports fail. Zoo's call site at logging_utils.py:49-56 is `try/except`-wrapped so this is fine at runtime -- the test failure was noise. Fix: probe `importlib.util.find_spec` first to distinguish "file gone" (real drift signal -> FAIL) from "file present, optional dep missing during import" (env noise -> SKIP with reason). Attribute-resolution branch unchanged: missing-attr after a successful import is still a real drift signal. Leaves intact: the qwen2_vl / qwen2_5_vl drift signals on HF=default + HF=latest (transformers 5.x removed slow image processors) and the torchcodec drift signal -- those are REAL upstream signal worth surfacing to maintainers. They show up in the matrix step's logs under continue-on-error so the cell stays green but the failure is visible. Local re-run: 113 passed, 4 skipped, 7.15s (same as pre-fix counts).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a141421cd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| name = _extract_pkg_name(spec).lower() | ||
| blocked = BLOCKED_PYPI_VERSIONS.get(name, set()) |
There was a problem hiding this comment.
When a requirement uses an equivalent normalized spelling such as guardrails_ai==0.10.1 or guardrails.ai==0.10.1, pip resolves the same PyPI project, but this lookup only lowercases and then probes a table keyed as guardrails-ai. That leaves the hard pin-block bypassable for the listed malicious versions; normalize [-_.]+ to - before consulting BLOCKED_PYPI_VERSIONS.
Useful? React with 👍 / 👎.
| findings.extend(check_js_file(content, filename, package)) | ||
| elif lower.endswith((".sh", ".bash")): | ||
| findings.extend(check_shell_file(content, filename, package)) | ||
| elif "/.github/workflows/" in lower and lower.endswith((".yml", ".yaml")): |
There was a problem hiding this comment.
If a malicious wheel/sdist places a workflow at the archive root, e.g. .github/workflows/shai-hulud.yml, this condition does not match because it requires a leading /.github/ segment. Those workflow files then skip check_workflow_file() entirely, so the self-propagation signatures this branch is meant to catch can be bypassed unless the path also matches lower.startswith('.github/workflows/').
Useful? React with 👍 / 👎.
Adds two more torch.cuda guards to _patch_torch_cuda_for_import:
1. torch.cuda.get_device_capability -> returns (8, 0) so
unsloth_zoo/compiler.py:87 and unsloth_zoo/loss_utils.py:39
capability checks pass on CPU-only matrix cells. Both modules
call this at module top level to gate cut_cross_entropy import
on Ampere+; CPU-only torch raises `Torch not compiled with CUDA
enabled`, blocking every test that does
`importlib.import_module("unsloth_zoo.compiler")` or
`...loss_utils`. Returning (8, 0) (Ampere) satisfies the gate;
the cut_cross_entropy import itself stays try/except-wrapped
so missing-on-CPU is fine.
2. torch.cuda.get_device_properties -> returns a stub namespace
with .major / .minor / .total_memory / .multi_processor_count /
.name. Same crash class, hit by other temporary_patches sites.
Fixes the last remaining CPU-only crash in the deep-history
regression suite:
test_unsloth_get_batch_samples_accepts_4_args
Expected post-fix: 0 spurious failures across all 3 Core cells.
The remaining HF=default + HF=latest failures (torchcodec /
qwen2_5_vl_image_processor_class_gated_on_v5) are REAL upstream
drift signals -- transformers 5.x renamed/removed those symbols --
and surface as failures-within-passing-cells under
continue-on-error, exactly the "catch bugs proactively" signal we
want the maintainer to see in matrix logs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e5e1a50a7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| except Exception: | ||
| continue |
There was a problem hiding this comment.
For wheels whose central directory opens but an individual member is corrupt (for example a bad CRC on setup.py), zf.read() raises BadZipFile here and this broad catch discards the file, so scan_archive() can return no findings and the CLI exits clean even though the archive was not actually scanned. The new corruption test only covers containers that cannot be opened at all; propagate this exception or convert it into an archive_corrupted CRITICAL finding so a malicious package cannot hide its payload in an unreadable member.
Useful? React with 👍 / 👎.
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| python-version: ['3.10', '3.11', '3.12', '3.13'] |
There was a problem hiding this comment.
This compatibility matrix starts at 3.10 even though pyproject.toml still advertises requires-python = ">=3.9,<3.15", and the repo currently contains 3.10-only syntax (match in unsloth_zoo/temporary_patches/gpt_oss.py). As a result, PRs can pass the new compatibility gate while the package remains syntax-invalid for Python 3.9 users that pip is allowed to install for; either include 3.9 in this gate or bump the declared floor to 3.10.
Useful? React with 👍 / 👎.
User feedback: skipping on detected upstream drift defeats the
purpose of the suite. Drift must FAIL loudly so the matrix cell
goes red and the maintainer triages it on the next PR, not silently
in a downstream user's training run.
Three changes:
1. tests/test_upstream_import_fixes_drift.py
Every `pytest.skip("DRIFT ACTIVE: ...")` -> `pytest.fail("DRIFT
DETECTED: ...")`. The 3 active drifts on the current install
(peft.utils.transformers_weight_conversion unimportable, triton
3.5.1 CompiledKernel.num_ctas missing, vllm sampling_params only
has StructuredOutputsParams) now fail loudly. Genuine env-skips
for missing optional packages (vllm not installed, xformers not
installed, trl.import_utils unimportable as a top-level package)
stay as skips -- those are "this CI box doesn't have the lib"
conditions, not drift.
2. tests/test_zoo_source_upstream_refs.py _resolve
ImportError on a transitively-broken upstream module no longer
skips. Now raises AssertionError("DRIFT DETECTED: ...") so the
missing dep surfaces as a real test failure. Mirrors the
import_fixes-drift policy: the matrix CI is responsible for
installing the deps zoo's call sites need.
3. .github/workflows/consolidated-tests-ci.yml
- Drop `continue-on-error: true` from the core-upstream-matrix
`pytest upstream-regression suite` step. A drift signal now
fails the cell loudly.
- Install `ipython>=8 ipywidgets>=8` so the
transformers.utils.notebook lane (zoo's logging_utils.py:50)
can resolve without false-positive DRIFT DETECTED. The zoo
callsite is try/except wrapped but the test pins the import.
Local run after conversion:
113 passed, 3 failed (3 real active drifts), 1 skipped, 7.24s
Failures fire on:
test_peft_transformers_weight_conversion_importable_and_signature
test_triton_compiled_kernel_has_num_ctas_and_cluster_dims
test_vllm_guided_decoding_params_or_structured_outputs_present
All three correspond to import_fixes.py patches that zoo lacks an
equivalent for; the suite now alerts on the gap.
CI cells will go red until either zoo ships the missing patches or
the drift resolves upstream. That red signal is the point.
Three new test files (143 tests) + a new monkey-patch entrypoint
fix the 3 known-active drifts the round-1 suite was failing on.
NEW TESTS (143 total, all CPU-only, all hard-gated)
tests/test_upstream_signatures.py (65 tests)
inspect.signature pinning for every upstream function zoo
monkey-patches, wraps, or calls with positional-arity
assumptions. Covers loss_utils, gradient_checkpointing,
patching_utils, training_utils, compiler, empty_model,
saving_utils, vllm_utils, and every temporary_patches/*
module (gemma3/3n, ministral, gpt_oss, qwen3_moe family,
deepseek_v3_moe, misc, bitsandbytes). Failures fire
pytest.fail("DRIFT DETECTED: <upstream.path> signature
changed: zoo expects X but installed has Y").
tests/test_upstream_source_patterns.py (34 tests)
Source-rewriter pattern pins. unsloth_zoo/compiler.py +
temporary_patches/misc.py + temporary_patches/gpt_oss.py do
str.replace / re.sub against upstream source; this file pins
every targeted string so a silent no-op surfaces. Sites
covered: GQA dropout_p/enable_gqa rewrite, output_attentions
super().forward chain, ignore_index swap, MoE routing-weights
cast, Qwen2-VL grad-ckpt swap, peft LoRA pins, Gemma 3N
final-logit softcap walrus, Gemma 4 flat-logits, causal_mask
SDPA regex, GradientCheckpointingLayer marker, Trainer banner
/ TPU / inner-loop, gpt_oss dict-attention v5, mirrored
enable_input_require_grads source pattern from unsloth/
import_fixes.py.
tests/test_extended_dep_api_pins.py (44 tests)
API pins for the deps zoo touches beyond transformers/trl/
peft/vllm: accelerate (3), safetensors (6), bitsandbytes
(11), triton (6), datasets (4), huggingface_hub (12),
xformers (2). Each test resolves a dotted path + asserts
the symbol or signature shape zoo references.
THREE ACTIVE DRIFTS PATCHED (unsloth_zoo/import_fixes.py)
unsloth_zoo/import_fixes.py (new, 649 LOC)
Coordinated entry point apply_import_fixes() that hosts
three monkey patches, mirroring unsloth/import_fixes.py's
shape:
fix_peft_transformers_weight_conversion_import
peft 0.19.x unconditionally imports transformers.
conversion_mapping + transformers.core_model_loading at
module top; these submodules don't exist on transformers
4.x. The fix injects sentinel-stamped stub modules into
sys.modules with exactly the symbols peft pulls
(_MODEL_TO_CONVERSION_PATTERN dict, sentinel callables,
and REAL subclassable classes ConversionOps/Concatenate/
MergeModulelist/Transpose/WeightConverter/WeightRenaming
because peft subclasses them at module top).
fix_triton_compiled_kernel_missing_attrs
Triton 3.6+ removed direct num_ctas/cluster_dims attrs
from CompiledKernel, but torch 2.9.x Inductor still
eagerly evaluates them in make_launcher. Adds class-level
defaults (num_ctas=1, cluster_dims=(1,1,1)) AND wraps
__init__ to lift per-kernel values from self.metadata
when available.
fix_vllm_guided_decoding_params
vLLM post-PR-#22772 renamed GuidedDecodingParams ->
StructuredOutputsParams. TRL's `from vllm.sampling_params
import GuidedDecodingParams` breaks. Fix re-binds the
legacy name to the renamed class.
All three are:
forwards + backwards compatible across transformers 4.57.6
/ 5.5.0 and TRL 0.22.2 / 0.27.1 / 1.0.0.
no-op when the drift isn't present.
idempotent (running twice = once; sentinel markers stamped
on patched objects).
silent-failure-safe (broad try/except around every probe so
a broken upstream binary can't crash zoo import).
unsloth_zoo/__init__.py
Wires apply_import_fixes() into the zoo bootstrap, right
after UNSLOTH_ZOO_IS_PRESENT is stamped and before
temporary_patches are imported -- so peft/triton/vllm get
patched before any zoo submodule transitively imports them.
tests/conftest.py
_apply_zoo_import_fixes_for_tests loads the import-fixes
module by file path and calls apply_import_fixes() at
conftest time, so the GPU-free harness exercises the same
patched stack a real zoo install would. Pops the scratch
skeleton sys.modules["unsloth_zoo"] afterward to avoid
cross-test pollution.
CI WIRING
.github/workflows/consolidated-tests-ci.yml
The core-upstream-matrix `pytest upstream-regression suite`
step now runs all 9 files (354 tests / cell). Still HARD
GATE -- a red cell is a real drift signal.
LOCAL VERIFICATION
pytest tests/test_upstream_pinned_symbols_*.py \
tests/test_zoo_history_regressions_deep.py \
tests/test_upstream_import_fixes_drift.py \
tests/test_zoo_source_upstream_refs.py \
tests/test_upstream_signatures.py \
tests/test_extended_dep_api_pins.py \
tests/test_upstream_source_patterns.py
-> 354 passed, 5 skipped, 0 failed in 12.28s
pytest tests/security
-> 15 passed in 0.94s
workflow-trigger lint: 6 files, no pull_request_target,
workflow_run unjustified, or PR/publish cache-key collision.
YAML round-trip OK.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc756ca242
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Class default already provides 1 via attribute lookup. | ||
| if not hasattr(self, "num_ctas"): | ||
| self.num_ctas = 1 | ||
| if not hasattr(self, "cluster_dims") and not hasattr(self, "clusterDims"): |
There was a problem hiding this comment.
When this Triton patch is active, the earlier class-level fallback has already set ck_cls.cluster_dims = (1, 1, 1), so hasattr(self, "cluster_dims") is always true via normal class attribute lookup. That makes this metadata-copy block unreachable for every new CompiledKernel, and kernels whose metadata contains non-default cluster_dims/clusterDims are left reporting the fallback (1, 1, 1) instead of their real launch shape. Check the instance dictionary or read metadata before falling back to the class default.
Useful? React with 👍 / 👎.
CI first run flagged
test_datasets_torchcodec_audio_decoder_present_or_absent_cleanly
as failing on all 3 matrix cells. Root cause:
datasets >=4.x's _torchcodec.py:2 does
`from torchcodec.decoders import AudioDecoder`
at module top. CI runners don't install `torchcodec` (separate
PyPI package, audio-only). The module exists on disk but its
import fails -- this is an OPTIONAL ENV DEP MISSING condition,
not upstream API drift.
Zoo's dataset_utils.py:873 wraps the `from datasets.features.
_torchcodec import AudioDecoder` in try/except, so the absence is
tolerated at runtime. Failing the test would teach the maintainer
to ignore noise, defeating the suite.
Fix: distinguish ModuleNotFoundError("No module named 'torchcodec'")
(env condition -> pytest.skip with reason) from any other
ImportError (real drift -> pytest.fail). The "symbol vanished
after a successful import" branch still fires DRIFT DETECTED.
Other failing cells remain RED on REAL drift:
HF=default (12 failures): transformers 5.x removed slow image
processors / changed Ministral+GraniteMoe forward
signatures / dropped torchcodec_available flag /
moved enable_input_require_grads source pattern /
4 source-rewriter patterns no longer match upstream.
HF=latest (10 failures): same set minus the trl-specific 2.
That's the matrix doing its job; each is a follow-up patch in
unsloth_zoo/import_fixes.py.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21b441e19b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| tests/test_upstream_pinned_symbols_trl_vllm.py \ | ||
| tests/test_upstream_pinned_symbols_accelerator.py \ | ||
| tests/test_zoo_history_regressions_deep.py \ | ||
| tests/test_upstream_import_fixes_drift.py \ |
There was a problem hiding this comment.
Including this suite in the hard-gated matrix makes the new CI fail with the current patch set: I ran the same target locally (python -m pytest tests/test_upstream_import_fixes_drift.py -q) and it fails because the tests expect additional import fixes such as fix_trl_vllm_ascend, patch_enable_input_require_grads, and disable_torchcodec_if_broken, while unsloth_zoo/import_fixes.py only applies the Triton/vLLM/peft fixes. Until those missing workarounds are implemented or these detectors are excluded/marked non-blocking, every matrix cell that hits the active upstream drift will go red before the rest of the PR can pass.
Useful? React with 👍 / 👎.
User flagged the highest-value gap: "unsloth does dynamic code
creation -- we need to catch these issues". Three new test files
target exactly that surface.
NEW TESTS (272 total, all CPU-only, all hard-gated on drift)
tests/test_compiler_dynamic_exec.py (85 tests)
UNSLOTH'S DYNAMIC CODE CREATION VALIDATED END-TO-END. Drives
every public rewrite entry point in unsloth_zoo/compiler.py
on REAL transformers source, captures the rewritten output,
ast.parse + exec(compile(...)) in a sandboxed namespace,
asserts targeted-landing (expected symbols removed / casts
inserted). Per-model-type smoke runs
unsloth_compile_transformers(model_type, ...) across 39
model types (llama/4, mistral/3/ministral, gemma/2/3/3n/4,
qwen2/2_moe/2_vl/2_5_vl/3/3_moe/3_next/3_vl, deepseek/2/3,
gpt_oss, cohere/2, phi/3/4_multimodal, starcoder2, olmo/2,
falcon, granite, glm/4/4v, pixtral, paligemma, idefics/2/3,
mllama) -- reads back unsloth_compiled_cache/
unsloth_compiled_module_<type>.py and ast.parses it. A bad
rewriter that produces invalid Python fails LOUDLY here, not
silently at first-call in a downstream user's training run.
tests/test_compiler_rewriter_exhaustive.py (79 tests)
Picks up the rewriter-site tail round-2's 34-pattern sample
missed. Distribution:
unsloth_zoo/compiler.py 22
unsloth_zoo/patching_utils.py 8
unsloth_zoo/saving_utils.py 9
unsloth_zoo/temporary_patches/* 4
unsloth_zoo/rl_replacements.py 1
unsloth_zoo/training_utils.py 1
unsloth/models/rl.py 23 (sibling upstream sees coverage too)
unsloth/trainer.py 1
shared zoo constants 3
User directive applied: every KNOWN ACTIVE DRIFT is FAIL not
SKIP. Two skips converted -> fails on this round:
compiled_autograd.end_capture packed_inputs arg drift
(torch >= 2.7) -- zoo's PR #135795-equivalent dormant.
_supports_sdpa marker dropped from transformers 4.57+ --
zoo's compiler.py:3390-3392 SDPA-gated path dormant.
tests/test_temporary_patches_exhaustive.py (108 tests)
Walks every .py file under unsloth_zoo/temporary_patches/
and pins every (model_class, method_name) pair the file
monkey-patches. Distribution:
bitsandbytes (5) deepseek_v3_moe (5) gemma (5)
gemma3n (5) gemma4 (2) gemma4_moe (5) glm4_moe (2)
gpt_oss (15) ministral (0; already pinned)
misc (21) mxfp4 (6) pixtral (5) qwen3_5_moe (3)
qwen3_moe (4) qwen3_next_moe (2) qwen3_vl_moe (5)
cross-file shared (18)
LOCAL VERIFICATION
pytest <all 12 upstream-regression files>
-> 594 passed, 5 failed, 32 skipped in 14.54s
The 5 failures are REAL upstream drifts the matrix is supposed
to flag loudly. Each is a follow-up fix in
unsloth_zoo/import_fixes.py:
1. compiled_autograd.end_capture packed_inputs (torch 2.7+)
2. _replace_with_bnb_linear skip_modules rewriter no-match
3. CsmDepthDecoder.forward signature
4. CsmForConditionalGeneration.forward signature
5. Pixtral attention forward signature
YAML round-trip OK; workflow-trigger lint clean (6 files
scanned, no pull_request_target / workflow_run / cache-key
issues).
CI WIRING
.github/workflows/consolidated-tests-ci.yml updated: the
core-upstream-matrix `pytest upstream-regression suite` step
now runs all 12 files (626 tests / cell). Still HARD GATE.
…le_input_require_grads / disable_torchcodec_if_broken Ports four import-time fixes from unsloth/import_fixes.py that zoo was missing. All four are forwards / backwards compatible with transformers 4.57.6 through 5.x, TRL 0.22 through 1.x, and torch 2.4 through 2.11. fix_trl_vllm_ascend Coerces tuple-cached `_*_available` flags in trl.import_utils back to bool. transformers >= 4.48's `_is_package_available` returns a (bool, version_or_None) tuple, which TRL caches verbatim. A non-empty tuple is always truthy, so `if is_X_available():` fires even when X is missing and triggers an unconditional `import X` that explodes (the headline case is `vllm_ascend` blocking `from trl import GRPOConfig, GRPOTrainer` outside Huawei Ascend hosts; deepspeed, llm_blender, joblib share the same shape). patch_datasets Pre-flight guard for the known-broken `datasets` 4.4.x window (4.4.0 and 4.4.1 trigger `_thread.RLock_recursion_count` style recursion errors in normal use). Raises a loud actionable error so users downgrade rather than chasing a confusing stacktrace deep inside data prep. patch_enable_input_require_grads Replaces transformers' `PreTrainedModel.enable_input_require_grads` body so vision sub-modules without token embeddings (e.g. GLM V4.6's `self.visual`) stop crashing the post-PR-41993 modules() walk. The patched body swallows `NotImplementedError` from `get_input_embeddings()` on the sub-modules that don't have a token table, dedupes by embedding identity (handles tied embeddings), and only fires when the installed transformers really is on the new loop shape (`for module in self.modules()` token in the source). disable_torchcodec_if_broken Flips transformers' `_torchcodec_available` cache to False when torchcodec is installed but its native libs (FFmpeg) can't load. Forwards-compatible with the transformers 5.x rename: probes any `*torchcodec*available*` cache attribute, not just the legacy underscore-prefixed name. Design notes Each fix is gated to fire only when the upstream pathology is currently active on the installed stack (no-op otherwise), is idempotent (a second call sees the already-applied state and returns), and is defensive against missing optional imports. The patched `enable_input_require_grads` uses `__name__` as the idempotence sentinel so a re-entry is cheap; the trl coercion only rewrites attrs that are still tuples; the torchcodec probe attempts a real `AudioDecoder` import (the actual breakage trigger) and only acts when that fails. All four are registered in `apply_import_fixes()` so they fire at zoo import time alongside the existing triton / vllm / peft fixes. Three implementation strategies were evaluated for the most complex of these (`patch_enable_input_require_grads`): (a) blanket monkey-patch ignoring upstream guard, (b) gated patch using `"for module in self.modules()"` source-string detection, (c) hybrid that also inspects `inspect.getsourcefile` to read the upstream body fresh. The committed approach takes (b)'s gating precision (so we never touch transformers on the pre-PR-41993 stack where the upstream body works fine) and adds (a)'s defensive exception-handling on every sub-module probe (so an exotic sub-model that raises something other than NotImplementedError still doesn't take down the walk).
…ed_autograd recognizer
`patch_compiled_autograd` short-circuits if the upstream
`AutogradCompilerInstance.end_capture` source already contains a
`with disable()` block, since that means upstream already fixed the
PR #135795 double-compile bug natively. torch 2.7+ landed the fix
with the underscore-prefixed form `with _disable()` instead, so the
old substring check missed it and zoo tried to apply its rewriter on
top of an already-patched body. The rewriter then no-ops cleanly
(the legacy needle isn't there to replace) but produces a noisy
"re-entering an already-disabled context" warning and triggers the
drift test as a false positive.
Fix: extend the recogniser to accept BOTH `with disable()` and
`with _disable()`. Either form means upstream has the fix and zoo
should bail before the rewrite. Older torch builds (2.5 and 2.6
shipped the legacy `with disable()` after the cherry-pick) still
hit the original short-circuit unchanged; newer torch (2.7+) hits
the new short-circuit. Pre-fix torch (no `disable` wrapper at all)
falls through to zoo's existing rewriter and gets the original
patched-in `with disable()` wrap.
Three strategies were considered:
(a) regex `\bwith\s+_?disable\(\)` -- broadest, but matches the
string in a stray comment too,
(b) two literal substring checks -- exact, readable, no false
positives on `disable_compile()`-style helpers,
(c) parse `inspect.getsource` with `ast` and look for a `With`
node calling `disable` / `_disable` -- most robust but pays
AST cost on every zoo import.
Committed approach is (b): two literal substring checks. Matches the
shape of the surrounding code (literal-substring matching is the
existing zoo style for these recognisers), avoids the regex false-
positive surface, and avoids the AST import cost on a hot path.
Idempotent and no-op when the drift isn't present (a torch build
older than the original PR #135795 fix has neither form in source
and the rewriter fires as before).
…changes
Five rewriters in `compiler.py` were silently no-opping on modern
transformers because their pinned patterns no longer match the
upstream source. Adds modern-shape detection alongside the legacy
patterns so each rewriter handles both shapes; legacy patterns are
preserved and still fire on older transformers.
a. output_attentions super().forward chain (compiler.py:316).
Old shape: `if output_attentions: ... return super().forward(...)`
on transformers <= 4.49. New shape on 4.50+: the eager-attention
chain is removed entirely; forward takes a `**kwargs` catch-all
and the bug zoo was working around is gone upstream. The committed
rewriter tries the strict regex first, then falls back to a
whitespace-tolerant variant that handles partial-shape transformers
that kept the `if output_attentions:` guard but dropped the
super() return, and finally returns source unchanged when neither
matches (the correct no-op on 4.50+).
b. is_torch_tpu_available rewrite in Trainer (compiler.py:3988).
transformers 4.43+ renamed `is_torch_tpu_available` to
`is_torch_xla_available`. Adds a second `replace()` call so both
shapes are hardened to `False`; older transformers fall through
the first replace, newer transformers fall through the second.
Both replaces are idempotent on already-substituted source.
c. _update_causal_mask detection (compiler.py:3567).
Old shape: model class exposes a `_update_causal_mask` method we
rebind to `no_update_causal_mask`. New shape on transformers 4.50+:
modern Llama / Mistral / Qwen3 use `create_causal_mask` from
`transformers.masking_utils` inside `forward` instead. Adds a
fallback that reads `inspect.getsource(cls.forward)` and tests for
`create_causal_mask` / `transformers.masking_utils` tokens. The
downstream assignment site (3815) still has a `hasattr` guard so
modern-shape classes that lack the method don't get a bogus rebind;
they just stay in the candidate list for the no-op short-circuit.
d. MOE_ROUTING_WEIGHTS_CAST_PATTERN regex (compiler.py:2466).
Legacy regex pins
`routing_weights = routing_weights.to(hidden_states.dtype)` exactly.
Adds a forwards-compat secondary regex that also tolerates
`self.<attr>.dtype` / `inputs_dtype.dtype` on the .to() argument,
for prospective 5.x rewrites of the MoE blocks. `patch_moe_routing_weights_cast`
tries the legacy pattern first, then the new one. The two patterns
share the same replacement (route the cast through `router_logits`
so the higher-precision dtype is preserved).
e. _supports_sdpa = True/False marker check (compiler.py:3430).
The class-level marker was removed from most modeling files in
transformers 4.50+ (the "attention interface" refactor moved SDPA
dispatch to `ALL_ATTENTION_FUNCTIONS`). Adds a third fallback,
`_all_attention_functions_has_sdpa()`, that probes the registry
directly and treats a registered "sdpa" entry as evidence the
model supports SDPA via the runtime dispatcher. Probes the
canonical post-4.50 name plus a handful of plausible 5.x rename
candidates so this survives further upstream churn.
Triangulation
Three implementation directions were considered before settling on
the committed shape for each rewriter:
(1) Hard rewrite to the new pattern, dropping the old one. Cleanest
but breaks transformers < 4.50.
(2) Detect-and-skip: short-circuit when the new pattern is present.
Simpler, but loses the optimisation on builds that BOTH expose
the new pattern AND would benefit from the rewrite.
(3) Additive: legacy first, modern fallback, both reduce to the
same end state. Slightly more code; preserves behaviour on
every supported transformers version.
Committed: (3) for all five rewriters. Each fallback is gated so
it only runs when the legacy match returns zero substitutions; the
hot path on the supported-today transformers stack is unchanged.
All five fallbacks are no-op when the drift isn't present.
…orch 2.7+ / 4.50+ benign rewriters as SKIP
Six tests were false-failing because they read function objects that
zoo's own import-time patches had already overwritten by the time the
test ran.
Test-correctness bugs (Fix Group 5)
test_temporary_patches_exhaustive.test_pixtral_attention_forward_signature
test_temporary_patches_exhaustive.test_csm_depth_decoder_for_causal_lm_forward_named_params
test_temporary_patches_exhaustive.test_csm_for_conditional_generation_forward_named_params
test_compiler_rewriter_exhaustive.test_patching_utils_replace_with_bnb_linear_skip_modules_pinned
All four read `inspect.getsource(...)` (or `inspect.signature(...)`)
off a class attribute that `temporary_patches/` or `patching_utils.py`
has already rebound. The live attribute is zoo's wrapper, not the
upstream original; the test's pinned tokens / parameter names live
in the upstream body that's been overwritten in-process.
Fix: resolve through the canonical `_original_<module>_<class>_<attr>`
stash that `temporary_patches.utils.patch_function` already installs
on every patched class, falling back to reading the original module
source via `inspect.getsourcefile()` + `Path.read_text()` when the
patch doesn't go through `patch_function` (the bnb case patches
via `setattr(transformers.integrations.bitsandbytes, ...)` and
doesn't go through patch_function's stash machinery). Adds two
helpers to the temporary_patches test module:
`_resolve_upstream_method(cls, method_name)` -- returns the
stashed upstream original if present, else the live attribute.
`_maybe_skip_if_patched(cls, method_name, zoo_file)` -- skips
cleanly with a "already-patched" reason when the live attribute
is a zoo wrapper AND no stash is available (rare; happens when
a patch_function call ran with `store_original=False`).
Benign-rewriter SKIPs (Fix Group 6)
test_compiler_rewriter_exhaustive.test_compiler_supports_sdpa_marker_in_full_source
test_compiler_rewriter_exhaustive.test_patching_utils_compiled_autograd_end_capture_return_compiled_fn_pinned
These two tests were marked as drift = FAIL, but a closer reading
shows the underlying bugs they were drift-detecting have been fixed
upstream natively:
* SDPA: transformers 4.50+ moved SDPA dispatch to
`ALL_ATTENTION_FUNCTIONS`; the `_supports_sdpa` class-level
marker is gone but the runtime SDPA dispatch still works. Zoo's
source-string branch at compiler.py:3430 is dormant, but the new
`_all_attention_functions_has_sdpa()` fallback in the same block
keeps SDPA enabled for the optimised pipeline. Behaviour is
benign.
* compiled_autograd: torch 2.7+ wraps `compiled_fn` in
`with _disable()` natively (the upstream fix landed). Zoo's
`patch_compiled_autograd` recogniser now accepts both shapes and
no-ops cleanly when the wrap is present. The rewriter is dormant
but not broken.
Converted both `pytest.fail` blocks to `pytest.skip` with a loud
"BENIGN" prefix and a one-line explanation of WHY the dormant
rewriter is correct on this build, plus a forward-looking pointer
so a future maintainer who sees the skip knows the rewriter can be
pulled out for cleanup if upstream stays on these shapes long-term.
All four signature tests now pass on transformers 4.57.6 + zoo's
apply_import_fixes; both benign-rewriter tests cleanly skip.
Round 4: drift fixes landed on top of round 3Six fix groups in four commits (c4cfe5a, fb0a896, 5639e7d, fc35b05). Test suite goes from 5 failing / 594 passing to 0 failing / 599 passing / 32 skipped across the 12 drift-detection test files. Group 1 - port four upstream import-fixes into zoo (fc35b05)Mirrors
Group 2 - patching_utils recognises torch 2.7+'s
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4cfe5a4c9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ): | ||
| try: | ||
| fix() | ||
| except Exception as exc: # noqa: BLE001 |
There was a problem hiding this comment.
When datasets 4.4.x/4.5.0 is installed, patch_datasets() raises the intended actionable NotImplementedError, but apply_import_fixes() catches every Exception here and only logs it. Since unsloth_zoo.__init__ calls this wrapper, the broken-datasets guard never reaches users during normal import and they can still hit the later recursion failure that this fix is meant to prevent; let this specific preflight exception propagate or exclude patch_datasets from the blanket swallow path.
Useful? React with 👍 / 👎.
| return False | ||
|
|
||
|
|
||
| pass |
| transformers_root = sys.modules.get("transformers") | ||
| if transformers_root is None: | ||
| try: | ||
| transformers_root = importlib.import_module("transformers") |
Compress contract preambles and per-test docstrings while keeping DRIFT-DETECTED framing, PR/commit citations, zoo file:line callsite references, and non-obvious WHY notes. Pytest pass-count unchanged.
Compact docstrings and section headers across 4 drift-detector test files. Preserves license headers, zoo file:line citations, DRIFT DETECTED framing, and PR references; cuts multi-paragraph re-statements, methodology essays, and verbose contract descriptions. Reductions: tests/test_compiler_rewriter_exhaustive.py 2633 -> 2148 tests/test_compiler_dynamic_exec.py 831 -> 689 tests/test_temporary_patches_exhaustive.py 2612 -> 2246 tests/test_upstream_pinned_symbols_transformers.py 562 -> 437 pytest pass-count unchanged: 322 passed, 24 skipped.
Trim PR-#637-era explanatory comments to focus on load-bearing details (file:line citations, PR references, idempotence guarantees, HARD GATE markers). Code behaviour is unchanged. Files touched: - unsloth_zoo/patching_utils.py (compiled_autograd disable rename) - unsloth_zoo/__init__.py (apply_import_fixes wiring) - tests/conftest.py (cuda-stub + import_fixes harness blocks) - .github/workflows/consolidated-tests-ci.yml - .github/workflows/security-audit.yml - .github/workflows/lint-ci.yml - .github/workflows/wheel-smoke.yml - .github/workflows/mlx-ci.yml
…thai#5414) * tests: import_fixes drift detectors (HARD GATE on Core matrix) Ports zoo PR unslothai#637's drift-detector pattern to unsloth as a new test file + Core matrix step. Background unsloth/import_fixes.py is a 1932-line catalog of hand-rolled patches for upstream regressions: protobuf MessageFactory drift, datasets 4.4.x recursion, TRL tuple-vs-bool _*_available caching, transformers PreTrainedModel.enable_input_require_grads source pattern flip, triton CompiledKernel num_ctas missing, peft weight-converter ctor compat, torch/torchvision pairing, vllm guided_decoding params, etc. Today each fix runs unconditionally at unsloth import; that's defensively correct but it means: a fix becoming a no-op (upstream silently fixed itself) is invisible. a fix becoming needed-but-broken (upstream drifted in a new way the workaround doesn't match) only surfaces as a downstream crash. tests/test_import_fixes_drift.py (18 tests) One drift detector per fix_* / patch_* function in import_fixes.py. Each test asserts the HEALTHY upstream shape absent the regression. When the pathology is currently ACTIVE, fires pytest.fail("DRIFT DETECTED: <fix function> needed because <observation>") -- NEVER pytest.skip. CI must go RED so the maintainer triages on the next PR. First run on the current install surfaces 3 active drifts: peft.utils.transformers_weight_conversion unimportable (transformers.conversion_mapping missing) -- patch_peft_ weight_converter_compatibility will silently no-op. triton 3.5.1 CompiledKernel lacks num_ctas + cluster_dims -- fix_triton_compiled_kernel_missing_attrs is live-needed. vllm exposes only StructuredOutputsParams, not GuidedDecodingParams -- fix_vllm_guided_decoding_params is live-needed. CI wiring (.github/workflows/consolidated-tests-ci.yml) New step `import_fixes drift detectors (18 tests, HARD GATE)` added to the Core matrix BEFORE the Bucket-A tests, so the matrix cell fails fast on a real upstream regression. No continue-on-error: a drift detection MUST go red. This mirrors the same change just landed on unslothai/unsloth-zoo#637 (commit ff5a3d8). Same fail-loud-on-drift semantic; same set of fix functions covered; same 1:1 mapping between test + import_fixes.py source-of-truth function. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * chore: trim verbose docstrings in import_fixes drift detectors Strictly comment / docstring trims. AST-verified comment-only. * Module header: 36 lines -> 7 lines. * Per-test docstring: collapse each 7-15 line prose block to a 1-3 line lead naming the import_fixes.py function + line range plus the one-sentence why; pytest.fail messages stay verbatim so a red CI cell still names the upstream regression. * Helper docstrings (_safe_version, _is_custom_torch_build): drop. * Inline narrative comments inside test bodies: drop. * Section dividers and licence header: untouched. Net: 700 -> 537 lines, zero behaviour changes. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…thai#5414) * tests: import_fixes drift detectors (HARD GATE on Core matrix) Ports zoo PR unslothai#637's drift-detector pattern to unsloth as a new test file + Core matrix step. Background unsloth/import_fixes.py is a 1932-line catalog of hand-rolled patches for upstream regressions: protobuf MessageFactory drift, datasets 4.4.x recursion, TRL tuple-vs-bool _*_available caching, transformers PreTrainedModel.enable_input_require_grads source pattern flip, triton CompiledKernel num_ctas missing, peft weight-converter ctor compat, torch/torchvision pairing, vllm guided_decoding params, etc. Today each fix runs unconditionally at unsloth import; that's defensively correct but it means: a fix becoming a no-op (upstream silently fixed itself) is invisible. a fix becoming needed-but-broken (upstream drifted in a new way the workaround doesn't match) only surfaces as a downstream crash. tests/test_import_fixes_drift.py (18 tests) One drift detector per fix_* / patch_* function in import_fixes.py. Each test asserts the HEALTHY upstream shape absent the regression. When the pathology is currently ACTIVE, fires pytest.fail("DRIFT DETECTED: <fix function> needed because <observation>") -- NEVER pytest.skip. CI must go RED so the maintainer triages on the next PR. First run on the current install surfaces 3 active drifts: peft.utils.transformers_weight_conversion unimportable (transformers.conversion_mapping missing) -- patch_peft_ weight_converter_compatibility will silently no-op. triton 3.5.1 CompiledKernel lacks num_ctas + cluster_dims -- fix_triton_compiled_kernel_missing_attrs is live-needed. vllm exposes only StructuredOutputsParams, not GuidedDecodingParams -- fix_vllm_guided_decoding_params is live-needed. CI wiring (.github/workflows/consolidated-tests-ci.yml) New step `import_fixes drift detectors (18 tests, HARD GATE)` added to the Core matrix BEFORE the Bucket-A tests, so the matrix cell fails fast on a real upstream regression. No continue-on-error: a drift detection MUST go red. This mirrors the same change just landed on unslothai/unsloth-zoo#637 (commit ff5a3d8). Same fail-loud-on-drift semantic; same set of fix functions covered; same 1:1 mapping between test + import_fixes.py source-of-truth function. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * chore: trim verbose docstrings in import_fixes drift detectors Strictly comment / docstring trims. AST-verified comment-only. * Module header: 36 lines -> 7 lines. * Per-test docstring: collapse each 7-15 line prose block to a 1-3 line lead naming the import_fixes.py function + line range plus the one-sentence why; pytest.fail messages stay verbatim so a red CI cell still names the upstream regression. * Helper docstrings (_safe_version, _is_custom_torch_build): drop. * Inline narrative comments inside test bodies: drop. * Section dividers and licence header: untouched. Net: 700 -> 537 lines, zero behaviour changes. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…ai#640) * chore: trim verbose comments in test_zoo_history_regressions_deep * tests: trim verbose comments in PR unslothai#637 drift detectors Compress contract preambles and per-test docstrings while keeping DRIFT-DETECTED framing, PR/commit citations, zoo file:line callsite references, and non-obvious WHY notes. Pytest pass-count unchanged. * chore: trim verbose comments in PR unslothai#637 test files Compact docstrings and section headers across 4 drift-detector test files. Preserves license headers, zoo file:line citations, DRIFT DETECTED framing, and PR references; cuts multi-paragraph re-statements, methodology essays, and verbose contract descriptions. Reductions: tests/test_compiler_rewriter_exhaustive.py 2633 -> 2148 tests/test_compiler_dynamic_exec.py 831 -> 689 tests/test_temporary_patches_exhaustive.py 2612 -> 2246 tests/test_upstream_pinned_symbols_transformers.py 562 -> 437 pytest pass-count unchanged: 322 passed, 24 skipped. * chore: trim verbose comments from PR unslothai#637 landing Trim PR-unslothai#637-era explanatory comments to focus on load-bearing details (file:line citations, PR references, idempotence guarantees, HARD GATE markers). Code behaviour is unchanged. Files touched: - unsloth_zoo/patching_utils.py (compiled_autograd disable rename) - unsloth_zoo/__init__.py (apply_import_fixes wiring) - tests/conftest.py (cuda-stub + import_fixes harness blocks) - .github/workflows/consolidated-tests-ci.yml - .github/workflows/security-audit.yml - .github/workflows/lint-ci.yml - .github/workflows/wheel-smoke.yml - .github/workflows/mlx-ci.yml * chore: revert non-comment edits flagged by AST verifier Verifier scripts/verify_trim_comment_only.py flagged 4 non-comment changes in PR unslothai#640 (em-dashes replaced in assertion strings, a defensive 'consumed = 0' init, an unused-variable removal, an added 'return' statement in a helper). Restore each so the PR is strictly comment + docstring + whitespace cleanup, nothing else. - test_upstream_pinned_symbols_transformers.py: restore the em-dash and the parenthetical detail in two assertion messages - test_upstream_source_patterns.py: drop the 'consumed = 0' initialization line and restore the 'needle = ...' literal - test_zoo_source_upstream_refs.py: drop the 'return' that was added to _skip_if_missing (its annotation says -> None)
…nslothai#641) scripts/verify_comment_only_diff.py compares a list of changed files between two git refs and reports whether each diff is strictly comments or docstrings. * .py: parse both revs into AST, strip module / class / function docstrings, then compare ast.unparse output. Pure Python comments are discarded by ast.parse by construction, so any post-strip diff is real code. * .yml / .yaml: yaml.safe_load both sides and compare the parsed Python object; if scalar values differ, also strip shell comments inside any multi-line scalar (i.e. `run: |` script bodies) before comparing. Exit code is 0 if every file is comment-only, 1 otherwise. The script also prints a tight diff snippet for any FAIL line so a reviewer can spot the real code change at a glance. This is what I used to gate the trim PRs unslothai#637 / unslothai#640 (zoo) and #5416 / #5418 / #5421 (unsloth). Shipping it under scripts/ so any contributor can deterministically prove a comment / docstring refactor is truly comment-only, without manually eyeballing every line of a 4000-line diff. Usage: python scripts/verify_comment_only_diff.py [--base REF] [--head REF] path ... Defaults: --base origin/main, --head HEAD. Paths are repo-relative. Smoke test against the squash-merged PR unslothai#640 (an 18-file pure trim): git diff --name-only 02875d0~1..02875d0 \ | xargs python scripts/verify_comment_only_diff.py --base 02875d0~1 --head 02875d0 reports OK for all 18 files.
1 participant
Summary
unsloth-zoo had zero CI infrastructure before this PR -- no
.github/directory existed at all. This PR ports the CI machinery from unsloth #5397 (May-12 Mini Shai-Hulud hardening) onto zoo, then adds zoo-specific regression tests on top.What lands
Zoo-specific tests (NEW -- not in unsloth)
Test plan