* tests: read checked-in files as UTF-8 instead of the platform default
Path.read_text() with no encoding uses locale.getpreferredencoding(), which
is UTF-8 on the Linux runners and cp1252 on a stock Windows install. Nine
module-level reads of checked-in source files were relying on that default.
studio/backend/routes/inference.py carries the DeepSeek tool-call token
regexes, so it holds U+FF5C and U+2581. Under cp1252 that read raised
UnicodeDecodeError on byte 0x81 at position 97806, and because the reads run
at import time it took test_cancel_atomicity.py and test_cancel_id_wiring.py
out at collection, not as failures. Green on CI, permanently broken for a
Windows contributor running the suite locally.
Adds a guard: at module scope there is no tmp_path fixture, so a bare
read_text()/write_text()/open() there is always touching a checked-in file.
That makes the rule mechanical enough to enforce with no allowlist, while
staying quiet about temp-dir I/O inside test bodies where the platform
default is harmless.
The repo already spells this correctly in 464 other places; this only stops
the stragglers coming back.
* tests: cover import-time helper reads and keep the guard py3.9-safe
Follows up on the Codex review:
- add `from __future__ import annotations`, since `str | None` in
`_offender` is evaluated at import on Python 3.9 and pyproject declares
requires-python ">=3.9,<3.15".
- widen the guard from module scope to import time. Class bodies and the
bodies of module-level helpers called from an executing statement run
during collection too, so `CODE = _extract_mixed_precision_code()` was
the same hazard as an inline read. `if __name__ == "__main__":` blocks
are skipped: pytest never executes them.
- scan studio/backend/tests/ as well as tests/. Both trees are collected
on Windows by separate CI jobs, and the offender that started this,
test_tool_xml_strip.py reading routes/inference.py, lives there.
Widening it surfaced seven more import-time reads of checked-in sources;
all now name utf-8.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Harden the import-time encoding guard for PR #7438
Close the detector gaps raised in review, all of which I reproduced against
the actual AST before changing anything.
False negatives (the guard let a real hazard through):
- _is_main_guard ignored the comparison operator, so if __name__ != "__main__"
counted as script-only even though its body runs at import.
- The else arm of a main guard was discarded with the rest of the If node.
- Decorators and argument defaults on a module-level def were skipped with the
body, though both are evaluated when the def executes.
- Path.open() in text mode was invisible; only builtin open() was matched.
- encoding = None and encoding = "locale" both re-select the platform default,
but the keyword merely being present counted as pinned.
False positives (the guard would have blocked a compliant contributor):
- A non-literal mode fell through to the "r" default, so open(p, mode) was
flagged even when mode is "rb", where adding encoding= is a ValueError and
there is no edit that satisfies the rule.
- Same for open(*args) and a **kwargs splat, which hide the mode and can hide
an encoding.
- Lambda bodies and comprehension elements were walked even though neither runs
at definition.
Verified: still reports the same 22 offenders on unpatched main, green on this
branch and on the tree merged with latest main (557 files), and an adversarial
corpus of 33 cases now scores zero false positives and zero false negatives.
Also corrected two docstring claims: neither collecting job runs on Windows,
and the read is governed by locale.getencoding().
* Walk eager comprehensions and treat io.open as the builtin
Two regressions from the previous commit, both reproduced against the AST
before changing anything.
Lumping list, set and dict comprehensions in with generator expressions was
wrong. Only a genexp is lazy; the other three run their element expression,
their filters and their nested iterators immediately, so
CONTENTS = [p.read_text() for p in PATHS] at module scope is an import-time
read the guard was silently missing. Comprehensions are now walked in full and
only the genexp keeps the outermost-iterable-only treatment.
io was also in the not-a-path-opener list, but io.open is the builtin, with the
same mode position and the same platform default. io.open(CHECKED_IN_FILE) is
exactly the hazard this guard exists for, so it is matched now, with binary
modes and a pinned encoding still exempt. tarfile.open and fitz.open stay
exempt since neither has an encoding to name.
Verified: 13 targeted cases covering all five eager comprehension forms and
io.open in text, binary and pinned shapes all classify correctly; still 22
offenders on unpatched main; green on this branch and on the tree merged with
latest main.
* Close three more walker gaps in the import-time guard
All three reproduced against the AST first.
A generator expression handed straight to a call is consumed there, so
DATA = "".join(p.read_text() for p in paths) runs its element at import. Only
an unconsumed genexp bound to a name stays lazy, so the walker now follows the
consumed ones in full and keeps the outermost-iterable-only treatment for the
rest.
if "__main__" == __name__ is an equivalent and accepted spelling of the main
guard, but requiring __name__ on the left meant its body was treated as
import-time code. That is a false positive on a block pytest never runs, so
both operand orders are recognised now.
The helper table was built from module-level defs only, so a def in a class
body invoked while the class is constructed was never followed, contradicting
the walker's stated coverage of class bodies. Helpers are now collected from
the module body and from class bodies at any nesting.
Verified: 15 targeted cases including all three fixes and the earlier ones
still classify correctly; still 22 offenders on unpatched main; green on this
branch and on the tree merged with latest main.
* Handle positional read_text encodings, lazy generators and nested helpers
* Guard reads reached from test bodies, unbound Path calls and __file__ paths
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Follow derived paths, skip lazy generator helpers, cover compressed openers
* Guard the CLI tests, helper parameters and unbound Path arguments
* Discover test roots and follow literal, in-place and tuple-derived paths
* Identify module openers by import, unwrap starred paths, pin subprocess snippets
* Resolve import origins, seed helper locals, follow named generators and parametrize
* Scope imports lexically, list tracked test files, bind unpacked names
* Resolve aliased openers, keyword-only params, destructured targets, next()
* Pin the encoding on subprocess snippets, workflow lint and CLI output for PR #7438
* Harden the CLI encoding guard against detached streams for PR #7438
* Tighten the encoding guard's path and scope analysis for PR #7438
* Resolve path provenance more precisely and keep POSIX stream encodings for PR #7438
* Resolve qualified path classes and scope conditional imports for PR #7438
* Scope CLI stream setup to the entry point and align two encoding pairs for PR #7438
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: danielhanchen <danielhanchen@gmail.com>
* Keep native RoPE scaling when extending context; carry rope_theta for linear
When max_seq_length exceeds a model's native window, the loader overwrote the
model's rope_scaling with linear scaling. For models that already ship a scaled
RoPE (llama3/yarn/longrope) that is far worse for long context, and on
transformers v5 the linear dict omitted rope_theta (v5 keeps it under
rope_parameters), so the rotary base fell back to 10000 and broke past ~8K tokens.
Keep the native scaling and just widen the window; only synthesize linear for
plain-RoPE models, and carry rope_theta so v5 keeps the real base.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Only preserve native llama3 when extending context; keep linear fallback otherwise
The patched attention constructor (patch_llama_rope_scaling) rebuilds only linear,
llama3 and longrope and its longrope branch reads a top-level
original_max_position_embeddings, so preserving yarn or a nested-only longrope config
would raise during construction on transformers <= 4.47.1. Keep only llama3 native;
yarn/longrope/other types fall back to the linear override, still carrying rope_theta.
* Correct long-context extension comment to match llama3-only preservation
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* Guard RoPE scaling against the transformers v5 buffer blank; honor extended factor
Add a family-agnostic guard that builds each rotary from a scaled config,
blanks its non-persistent buffers (what transformers v5 does on load), runs
loader._fix_rope_inv_freq, and asserts every buffer is restored to its scaled
value (llama3 and longrope). This catches the whole bug class, not just the
one call site, and is validated to fail on the pre-fix repair.
Also make LlamaExtendedRotaryEmbedding read the llama3 factor from the config
instead of hardcoding 8 (wrong for Llama-3.2, factor 32), falling back to the
Llama-3.1 defaults when built without a config.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Pass config into extended rotary codegen; skip v5 round-trip on transformers 4.x
- patch_llama_rope_scaling now builds the llama3 extended rotary with
config=self.config so it reads the real factor (32 for Llama-3.2) instead
of falling back to 8; the template already references self.config.
- test_v5_blank_repair_roundtrip now skips when loader._NEEDS_ROPE_FIX is
False, since _fix_rope_inv_freq is a no-op on transformers 4.x and cannot
restore the blanked buffers there.
* Raise stream deadlock-guard timeouts from 0.2s to 5.0s in passthrough tests
These asyncio.wait_for guards bound test setup and cross-task event
signaling that complete near-instantly on success; the 0.2s budget is a
latency assertion in disguise and times out under CI scheduling load
(seen on the 3.11 matrix leg while 3.10/3.12/3.13 pass the same commit).
5.0s matches the timeout used elsewhere in the suite and still fails fast
on a real hang. No test relies on the guard expiring.
* Extended rotary reads rope_parameters as well as rope_scaling
transformers v5 stores llama3 scaling under config.rope_parameters and
exposes rope_scaling only as a back-compat property. Reading that property
works on 5.0-5.13 (verified: factor resolves to 32 for Llama-3.2), but a
future release may drop the shim, after which the subclass path would fall
back to factor 8. Read either field so the factor survives the rename.
Adds test_extended_rotary_reads_rope_parameters_v5 (fails on the old
single-field read: rope_parameters-only config resolves to 8, not 32).
* [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>
* Fix llama3 RoPE scaling dropped on transformers v5
transformers v5 loads on meta then blanks non-persistent buffers, so
_fix_rope_inv_freq rebuilds inv_freq after load. It recomputed a vanilla
inv_freq and applied _apply_inv_freq_scaling, a no-op on the base
LlamaRotaryEmbedding used by the config/llama3 path, so inv_freq ended up
divided by 1 instead of the config factor (8 for Llama 3.1, 32 for Llama
3.2). This corrupts long-range positions and inflates long-context loss
about 3-5x. transformers 4.x was unaffected.
Route __init__ and the v5 repair through one _unsloth_recompute_inv_freq
so they cannot diverge, and stash the config on the rotary module so the
repair can rebuild the same scaled value.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Add test for llama3 RoPE scaling under the transformers v5 repair
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Update RoPE drift guard for the recompute refactor and guard the v5 repair
The drift guard's AST tripwire asserted the config-scaling call lived in the
if config is not None branch of LlamaRotaryEmbedding.__init__. The fix moved
that into _unsloth_recompute_inv_freq, so follow it there (with a fallback to
the old inline branch) and add a guard that loader._fix_rope_inv_freq rebuilds
inv_freq through the same helper. Also add a CPU functional check of the helper
and drop the redundant standalone test.
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* Reduce and tighten comments and docstrings in tests
Shorten verbose comments and docstrings across the test suite without
changing any test logic. Remove narration that restates the next line,
collapse long module and test docstrings to a single line, and drop banner
separators. Keep regression context (issue and PR references, run ids),
skip reasons, mocking and timing rationale, license headers, lint and type
directives, and commented-out code.
Comments and docstrings only: an AST signature check confirms no code,
assertions, or string literals changed, and the suite byte-compiles cleanly.
* [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>
* Handle rope_type 'default' on transformers 5 to stop false RoPE warning
transformers 5 reports rope_type="default" for every plain (unscaled) config
and dropped "default" from ROPE_INIT_FUNCTIONS. _compute_config_rope_inv_freq
then did ROPE_INIT_FUNCTIONS["default"], hit KeyError, returned None and logged
"Could not apply RoPE scaling 'default'; long-context generation may degrade"
on every model load. The inv_freq was still correct (the constructor recomputes
vanilla on None), but the warning is a false alarm for unscaled models.
Compute the unscaled inv_freq directly for rope_type "default"/None instead of
going through ROPE_INIT_FUNCTIONS, so plain configs return the right value with
no warning. Scaled types (llama3/linear/yarn/...) are unchanged.
Also skip test_object_style_rope_scaling_on_config_delegates_correctly when
transformers strict-validates rope_scaling (5.x): it rejects a non-dict object
on config.rope_scaling, so the object-style delegation path cannot be set up
there. The test still runs and asserts on transformers <5.
* [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>
* Fix config.rope_scaling being dropped by the replaced rotary embedding (#2405)
On modern transformers, LlamaModel builds its rotary embedding from config
using unsloth's replacement LlamaRotaryEmbedding class, whose config path
computed vanilla inv_freq and ignored config.rope_scaling entirely. The
llama3/linear/longrope dispatch in patch_llama_rope_scaling rewrites
LlamaAttention.__init__, which no longer constructs rotary embeddings, so it
never fires; the model-level rotary is then copied onto every attention
layer. Result: Llama-3.1/3.2/3.3 ran with unscaled RoPE on the
FastLanguageModel path and collapsed into repetition loops past roughly 29K
tokens (PASS at 28867, FAIL at 31767 in needle retrieval). FastModel was
unaffected because vision.py keeps transformers' own rotary. qwen2, qwen3,
qwen3_moe, mistral and cohere assign the same base class, so any rope-scaled
config of those families was equally exposed.
The fix makes the base class config path compute inv_freq and
attention_scaling via transformers' ROPE_INIT_FUNCTIONS (covers llama3,
linear, dynamic, yarn, longrope), with an inline llama3 fallback reading
factors from config for older transformers, degrading to prior behavior on
any failure. attention_scaling is applied in _set_cos_sin_cache (1.0 default,
exact no-op for unscaled paths) and persists across extend_rope_embedding.
A type(self) guard prevents double-scaling via the legacy scaled subclasses.
Adds tests/utils/test_rope_scaling_drift.py (AST tripwire + behavioral
inv_freq/cos-cache/extension checks, validated to fail 4 of 5 on the unfixed
code) and wires it into the existing consolidated CI HARD GATE step.
Verified on GPU: 48K-token needle retrieval flips FAIL to PASS for
FastLanguageModel in bf16 and 4bit, 20K stays PASS, scaled inv_freq matches
transformers exactly, and the left-padded batch generation guard still gets
exact solo-vs-batched token matches.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Address review: normalize object-style rope_scaling, vectorize llama3 fallback
config.rope_scaling can be a config object rather than a dict on newer
transformers; _rope_scaling_as_dict normalizes it (to_dict/dict/vars
fallbacks) before any .get() access, with a regression test using a
dataclass stand-in. The inline llama3 fallback now uses torch.where instead
of a per-frequency Python loop; verified bit-for-bit equal to transformers
ROPE_INIT_FUNCTIONS for factor 8 (Llama-3.1) and factor 32 (Llama-3.2).
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Address review: CPU-safe rope guard tests, normalized config for delegation
The rotary constructor builds per-device CUDA caches, so the behavioral tests
that instantiate it cannot run on GPU-less CI. Restructured into three layers:
the AST tripwire now also asserts the constructor stays wired to
_compute_config_rope_inv_freq; the CPU layer tests that pure helper directly
(llama3 dict, llama3 object, linear object, default type) with no
instantiation; the instantiation and cache tests are gated behind a real CUDA
probe (actual tensor allocation, so import-time CUDA spoofs cannot fool the
gate). Verified: 9 passed with GPU; 5 passed 4 skipped with CUDA hidden; 5
failed 4 skipped on the unfixed code in CPU mode.
Delegation to ROPE_INIT_FUNCTIONS now retries with a shallow config copy
carrying the normalized rope_scaling dict when the original was an object the
installed transformers cannot read; covered by a linear-object test, which has
no inline fallback and passes only through that retry path.
* Tighten comments in rope scaling fix and guard test
Comment and docstring reduction only; verified code-identical with
scripts/comment_tools.py check --strip-docstrings (AST signature match on
both Python files). All guard tests unchanged: 20 passed with GPU, 5 passed
4 skipped with CUDA hidden.
* Apply repo kwarg-spacing format
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Leo Borcherding