From f8860add83cc3692aa6a71199b876fa60656179c Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Thu, 7 May 2026 08:32:49 +0000 Subject: [PATCH] CI(Core): filter TRL trainer/config sweep to actual submodules only The trainer-discovery sweep tripped on TRL 0.x (cell HF=4.57.6+TRL<1) and TRL 1.x (cell HF=latest+TRL=latest) with: AST FAIL trl.trainer.get_peft_config: no spec AST FAIL trl.trainer.get_quantization_config: no spec TRL re-exports those as utility FUNCTIONS in trl.trainer.__init__. Their names end with `_config` so my `endswith("_config")` filter swept them up alongside real `*_config.py` submodules; importlib.util. find_spec then returns None because they are not files on disk and the AST stage records `no spec` -> failure. Add `_is_real_submodule(qual_name)` that tests `find_spec().origin` non-None and apply it to both `_trainer_files()` and `_config_files()`. Re-exported utility functions are silently filtered out -- they are NOT modules and unsloth's auto-discovery in rl.py:patch_trl_rl_trainers does not pretend they are. Note: rl.py:1939-1943 has the same `endswith("_trainer")` filter without a submodule check; it gets away with it today only because TRL has no public `_trainer`-suffixed function exports. If TRL ever adds one, the same gap appears upstream. Cell HF=default+TRL=default succeeded on the previous run because its TRL pin (resolved via pyproject) happens to ship a different public surface that does not include the `get_*_config` re-exports. Verified locally on TRL 0.25.1: 16/16 raw `_config` names are real submodules; 0 non-module exports filtered. Filter is a no-op on versions without the trap and a corrective skip on versions with it. --- .github/workflows/consolidated-tests-ci.yml | 27 +++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/.github/workflows/consolidated-tests-ci.yml b/.github/workflows/consolidated-tests-ci.yml index 003f8bee71..07b3e5c392 100644 --- a/.github/workflows/consolidated-tests-ci.yml +++ b/.github/workflows/consolidated-tests-ci.yml @@ -962,20 +962,43 @@ jobs: import trl.trainer - # Replicate rl.py:1939-1943 verbatim. + def _is_real_submodule(qual_name: str) -> bool: + """True iff `qual_name` resolves to an importable submodule + with a file on disk (i.e. has a non-None find_spec().origin). + + TRL re-exports utility FUNCTIONS into `trl.trainer.__init__` + whose names happen to end with `_config` (e.g. + `get_peft_config`, `get_quantization_config`). Without this + filter the `endswith` check below picks them up as if they + were submodules and the AST stage fails on `no spec`. The + same trap exists for `_trainer` (none today, but defensive). + """ + try: + spec = importlib.util.find_spec(qual_name) + except (ImportError, ValueError): + return False + return spec is not None and bool(getattr(spec, "origin", None)) + + + # Replicate rl.py:1939-1943 verbatim, then filter to actual + # submodules so re-exported utility functions (e.g. + # `get_peft_config`) do not pollute the AST sweep. def _trainer_files(): return [ x for x in dir(trl.trainer) if x.islower() and x.endswith("_trainer") and x != "base_trainer" + and _is_real_submodule(f"trl.trainer.{x}") ] def _config_files(): return [ x for x in dir(trl.trainer) - if x.islower() and x.endswith("_config") + if x.islower() + and x.endswith("_config") + and _is_real_submodule(f"trl.trainer.{x}") ]