mirror of
https://github.com/unslothai/unsloth.git
synced 2026-08-21 14:53:58 +00:00
* 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>
165 lines
7.8 KiB
Python
165 lines
7.8 KiB
Python
#!/usr/bin/env python3
|
|
# SPDX-License-Identifier: AGPL-3.0-only
|
|
# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0
|
|
|
|
"""Refuse backend source that needs a newer interpreter than the matrix floor.
|
|
|
|
A pull request runs Backend CI on the NEWEST interpreter only. Every older leg still runs
|
|
on the push to main, so a version-specific break is caught at merge rather than never, but
|
|
between opening a pull request and merging it nothing executes the backend on the oldest
|
|
one. This closes as much of that gap as a static check can.
|
|
|
|
Syntax is the easy half, and ``tests/test_python39_compatibility.py`` already covers it by
|
|
parsing at the version ``pyproject.toml`` declares. Syntax is also not the shape this
|
|
regression takes. The realistic mistake 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 this asks vermin, which reads both syntax and stdlib API availability, and compares the
|
|
answer against the oldest leg the workflow's own matrix declares rather than a number
|
|
written here. Raise the floor in the matrix and this follows; use a symbol from above it
|
|
and this fails in seconds, on every pull request, instead of on main in 23 minutes.
|
|
|
|
What it cannot do, stated so nobody mistakes it for the legs it partly replaces: it does
|
|
not run anything. Two interpreters that both accept a line can still behave differently on
|
|
it, and a ``sys.version_info`` branch is only ever parsed here, never taken. That is what
|
|
the full matrix on main is for.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import json
|
|
import re
|
|
import shutil
|
|
import subprocess
|
|
import sys
|
|
from pathlib import Path
|
|
|
|
REPO = Path(__file__).resolve().parents[1]
|
|
WORKFLOW = REPO / ".github" / "workflows" / "studio-backend-ci.yml"
|
|
|
|
# Both trees the matrix legs actually execute. studio-backend-ci lists 'unsloth_cli/**'
|
|
# in its own paths filter and runs `pytest unsloth_cli/tests` as a step on every leg, so
|
|
# a post-floor stdlib name on a shipped CLI path was covered by the old 3.10 leg exactly
|
|
# as a backend one was. Scanning only the backend would have moved that coverage to the
|
|
# push to main while looking like it had replaced it.
|
|
ROOTS = (
|
|
REPO / "studio" / "backend",
|
|
REPO / "unsloth_cli",
|
|
)
|
|
|
|
# Everything shipped under studio/backend is scanned. The first version of this listed the
|
|
# packages instead, and that is exactly the wrong shape for a floor check: it named core,
|
|
# utils and routes and silently missed 116 files, including 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.
|
|
# A check that covers most of the tree reads exactly like one that covers all of it.
|
|
#
|
|
# So the tree is the input and only vendored code comes out, pinned to its own support
|
|
# range. Tests are IN, which the first version had wrong 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 whole gap this exists to close.
|
|
EXCLUDE_PARTS = ("vendor", "node_modules", "__pycache__", ".venv")
|
|
|
|
# An above-floor symbol reached deliberately is suppressed AT THE SITE, with `# novermin`
|
|
# and a comment saying why, not by dropping its file from the scan. Excluding the file
|
|
# would leave everything else in it permanently unchecked, which is the same mistake as
|
|
# the package allowlist this replaced, one level down.
|
|
#
|
|
# The one live case is locale.getencoding() in the data-designer plugin's state_store,
|
|
# inside a try/except AttributeError with a pre-3.11 fallback. vermin reads names rather
|
|
# than control flow, so it cannot see that the guard is already there.
|
|
#
|
|
# Comment parsing is therefore ON, which is what makes the annotation work.
|
|
|
|
|
|
# The floor is DECLARED, in the workflow, next to where the legs used to be.
|
|
#
|
|
# It used to be derived from the matrix, which was right while the matrix ran several
|
|
# interpreters and became self-defeating the moment it ran one: a 3.13-only matrix would
|
|
# have moved the floor to 3.13 and left this check asserting that code written for 3.13
|
|
# runs on 3.13. Deriving it from pyproject.toml is not the answer either, because that
|
|
# says >= 3.9 and 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. That mismatch is worth
|
|
# fixing, in its own change, and this lint is what makes it visible rather than what
|
|
# hides it.
|
|
#
|
|
# So it is a number, written down once, in the workflow that would otherwise have tested
|
|
# it, and read from there.
|
|
FLOOR_KEY = "PYTHON_FLOOR"
|
|
|
|
|
|
def declared_floor() -> tuple[int, int]:
|
|
"""The floor the workflow declares, as (major, minor)."""
|
|
text = WORKFLOW.read_text(encoding = "utf-8")
|
|
found = re.search(rf"^\s*{FLOOR_KEY}:\s*['\"]?(\d+)\.(\d+)['\"]?\s*$", text, re.M)
|
|
if not found:
|
|
raise SystemExit(
|
|
f"{WORKFLOW.name} declares no {FLOOR_KEY}, so this lint has no target. It is "
|
|
f"declared there rather than here so that the number lives with the CI that "
|
|
f"used to test it."
|
|
)
|
|
return int(found.group(1)), int(found.group(2))
|
|
|
|
|
|
def targets() -> list[str]:
|
|
"""Every .py the matrix legs ship or execute, found rather than listed."""
|
|
found = []
|
|
for root in ROOTS:
|
|
if not root.is_dir():
|
|
raise SystemExit(f"{root} is gone; the scan would silently stop covering it")
|
|
found.extend(
|
|
str(path)
|
|
for path in sorted(root.rglob("*.py"))
|
|
if not any(part in EXCLUDE_PARTS for part in path.relative_to(root).parts)
|
|
)
|
|
if not found:
|
|
raise SystemExit(f"no python files found under {ROOTS}; the scan would pass on nothing")
|
|
return found
|
|
|
|
|
|
def main() -> int:
|
|
floor = declared_floor()
|
|
target = f"{floor[0]}.{floor[1]}"
|
|
# The console script, not `python -m vermin`: the package has no __main__, so that
|
|
# form exits nonzero for the wrong reason and this lint would fail on every run while
|
|
# looking like it had found something.
|
|
vermin = shutil.which("vermin")
|
|
if vermin is None:
|
|
raise SystemExit(
|
|
"vermin is not installed, so the backend floor is unchecked. Install it in "
|
|
"the job that runs this, rather than letting the check quietly pass."
|
|
)
|
|
files = targets()
|
|
print(f"[floor] {len(files)} files must run on Python {target}, " f"the declared floor")
|
|
command = [
|
|
vermin,
|
|
"--no-tips",
|
|
"--violations",
|
|
f"-t={target}",
|
|
*files,
|
|
]
|
|
result = subprocess.run(command, capture_output = True, text = True)
|
|
sys.stdout.write(result.stdout)
|
|
sys.stderr.write(result.stderr)
|
|
if result.returncode == 0:
|
|
print(f"[floor] OK: nothing needs more than {target}")
|
|
return 0
|
|
print(
|
|
f"::error title=Backend needs a newer Python than the matrix floor::"
|
|
f"something under studio/backend or unsloth_cli requires more than Python {target}, "
|
|
f"which is the "
|
|
f"floor studio-backend-ci declares. Nothing runs that interpreter any more, so "
|
|
f"this check is the only thing standing between an above-floor symbol and a user "
|
|
f"on that version. Either guard the usage behind a sys.version_info check, or "
|
|
f"raise {FLOOR_KEY} in the workflow and say why."
|
|
)
|
|
return 1
|
|
|
|
|
|
if __name__ == "__main__":
|
|
raise SystemExit(main())
|