1c91f49d83* fix: unblock 4 tests deselected/skipped in #5312 (real bugs) PR #5312 surfaced two real regressions by turning previously-silent skips into explicit `--deselect` / `pytest.skip(...)` blocks. Both were left as follow-ups rather than fixed in that PR. This PR fixes the underlying bugs so the suppressions can be dropped. 1. studio/backend/requirements/no-torch-runtime.txt: pin tokenizers Installing with `--no-deps -r no-torch-runtime.txt` (the path install.sh takes for the no-torch / GGUF-only mode) resolves transformers to 5.3.0 and tokenizers to the latest available (0.23.1). transformers 5.3.0 requires `tokenizers>=0.22.0,<=0.23.0`, so `from transformers import AutoConfig` then fails at import time: ImportError: tokenizers>=0.22.0,<=0.23.0 is required for a normal functioning of this module, but found tokenizers==0.23.1. Pin `tokenizers>=0.22.0,<=0.23.0` to match the constraint embedded inside every transformers version in the allowed window (4.56.0..5.3.0). Verified locally: a fresh `uv venv` + `uv pip install --no-deps -r no-torch-runtime.txt` followed by `from transformers import AutoConfig` now succeeds. Unblocks 3 deselected cases in studio-backend-ci.yml: - TestE2ETokenizersFix::test_autoconfig_works_with_no_torch_runtime (parametrized py 3.12 + 3.13 -> 2 cases) - TestE2EFullNoTorchSandbox::test_autoconfig_succeeds 2. unsloth/models/rl.py: defensive wrapper for _patch_trl_rl_trainers _patch_trl_rl_trainers has many internal `try: ... except: ... return` branches, but several paths (notably inspect.getsource on the thin wrappers TRL 1.x leaves in trl.trainer for trainers that moved to trl.experimental) can still propagate exceptions. The umbrella patch_trl_rl_trainers() ring-fences each call with try/except + warning_once, but direct callers (the CI shim in consolidated-tests-ci.yml, downstream tools, end-user scripts) used to see the raw exception, which forced #5312's CI heredoc to ring-fence with: except Exception as e: # TRL 1.x renames break the patch helper internally; we # accept that here and skip rather than fail the cell. pytest.skip(f"_patch_trl_rl_trainers raised: ...") Rename the existing implementation to _patch_trl_rl_trainers_impl and make _patch_trl_rl_trainers a thin wrapper that catches any uncaught exception and routes it through logger.info, matching the umbrella wrapper's behaviour. Power users who want the raw raising behaviour for their own diagnostics can still call _patch_trl_rl_trainers_impl directly. Adds tests/python/test_patch_trl_rl_trainers_defensive.py to lock the contract: the wrapper must never raise, and it must delegate to the impl on the happy path. Unblocks 1 skip in consolidated-tests-ci.yml's test_compile_sft_trainer_patch. Follow-up for #5312 once this lands: drop the two `--deselect` lines in studio-backend-ci.yml's repo-cpu-tests step and drop the `except Exception ... pytest.skip(f"_patch_trl_rl_trainers raised: ")` block in consolidated-tests-ci.yml's test_compile_sft_trainer_patch. * chore: tighten comments and docstrings in the new code Drop verbose justifications down to one or two lines per site. The PR description carries the full context; in-file comments only need to point at the WHY. * chore(no-torch-runtime): drop redundant lower bound on tokenizers tokenizers 0.23.0 was never published to PyPI (versions go 0.22.2 -> 0.23.1), so `tokenizers<=0.23.0` resolves to 0.22.2 in practice, the same version the explicit >=0.22.0,<=0.23.0 pin resolved to. Verified on Python 3.12 and 3.13.
69 lines
2 KiB
Python
69 lines
2 KiB
Python
# SPDX-License-Identifier: AGPL-3.0-only
|
|
# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved.
|
|
|
|
"""Regression tests: _patch_trl_rl_trainers must never raise.
|
|
|
|
The wrapper in unsloth/models/rl.py ring-fences the impl so direct
|
|
callers (CI shims, downstream tools) don't have to. Lock that
|
|
contract here.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import pytest
|
|
|
|
|
|
pytest.importorskip("trl")
|
|
|
|
|
|
def _import_helpers():
|
|
try:
|
|
from unsloth.models.rl import (
|
|
_patch_trl_rl_trainers,
|
|
_patch_trl_rl_trainers_impl,
|
|
)
|
|
except ImportError as e:
|
|
pytest.skip(f"unsloth.models.rl helpers not importable: {e}")
|
|
return _patch_trl_rl_trainers, _patch_trl_rl_trainers_impl
|
|
|
|
|
|
def test_patch_trl_rl_trainers_swallows_unknown_trainer_name():
|
|
wrapper, _impl = _import_helpers()
|
|
assert wrapper("definitely_not_a_real_trainer_xyz") is None
|
|
|
|
|
|
def test_patch_trl_rl_trainers_swallows_garbage_input():
|
|
wrapper, _impl = _import_helpers()
|
|
for bad in ("", "..", "trainer with space", "sft_trainer; rm -rf /"):
|
|
assert wrapper(bad) is None, f"raised on input: {bad!r}"
|
|
|
|
|
|
def test_impl_is_separately_exposed():
|
|
# Power users can still call the impl directly for the raising path.
|
|
_wrapper, impl = _import_helpers()
|
|
assert callable(impl)
|
|
|
|
|
|
def test_wrapper_delegates_to_impl(monkeypatch):
|
|
from unsloth.models import rl as _rl
|
|
|
|
sentinel = object()
|
|
calls = []
|
|
|
|
def _fake_impl(trainer_file):
|
|
calls.append(trainer_file)
|
|
return sentinel
|
|
|
|
monkeypatch.setattr(_rl, "_patch_trl_rl_trainers_impl", _fake_impl)
|
|
assert _rl._patch_trl_rl_trainers("sft_trainer") is sentinel
|
|
assert calls == ["sft_trainer"]
|
|
|
|
|
|
def test_wrapper_swallows_impl_exception(monkeypatch):
|
|
from unsloth.models import rl as _rl
|
|
|
|
def _boom(_trainer_file):
|
|
raise RuntimeError("simulated TRL 1.x rename failure")
|
|
|
|
monkeypatch.setattr(_rl, "_patch_trl_rl_trainers_impl", _boom)
|
|
assert _rl._patch_trl_rl_trainers("sft_trainer") is None
|