fix: skip redundant HfFileSystem().glob() calls in loader.py

[rolandtannous]

Summary

• Guard SUPPORTS_LLAMA32 glob blocks in FastLanguageModel.from_pretrained and FastModel.from_pretrained with is_model and is_peft so the HfFileSystem HTTP call is only made when both configs could actually exist
• Prevents indefinite hangs on slow/unreliable networks since the glob result is redundant when either AutoConfig or PeftConfig already failed to load
• Adds 7 unit tests verifying glob is skipped/called in the correct scenarios

Test plan

• pytest tests/test_loader_glob_skip.py -v — all 7 tests pass
• Train any HF model (e.g. unsloth/Qwen3.5-2B) — should no longer hang on slow networks
• Train a LoRA adapter repo — should still detect conflicting configs correctly
• Train a local model path — behavior unchanged

[gemini-code-assist]

Code Review

This pull request introduces guards in loader.py to skip redundant HfFileSystem().glob() calls when is_model or is_peft is false, and adds a test suite to verify this behavior. Feedback indicates that the logic being modified is effectively dead code because the both_exist variable updated within these blocks is never used, suggesting that the entire manual check logic should be simplified or removed.

[danielhanchen]

Thank you for the PR! The goal of this PR is to avoid unnecessary HfFileSystem().glob() network calls in the loader when they cannot produce useful results. As a summary, this PR adds and is_model and is_peft guards to both SUPPORTS_LLAMA32 blocks in FastLanguageModel.from_pretrained and FastModel.from_pretrained, so the remote glob call is only made when both AutoConfig and PeftConfig have successfully loaded -- skipping it otherwise to prevent hangs on slow/unreliable networks.

I ran 10 independent reviews (7 automated + 3 manual). Findings:

Reviewers	Severity	Finding
10/10	Info (pre-existing)	both_exist is set inside the SUPPORTS_LLAMA32 block but never read afterward -- the "two config files" RuntimeError never fires on transformers > 4.45.0. This is dead code that predates this PR.
10/10	None	The guard and is_model and is_peft is logically correct: when either config failed to load, the glob result would be written to both_exist which is never consumed, so skipping it is safe.
3/10	False positive	Three reviewers flagged a regression where a mixed repo with a malformed config would no longer be rejected. However, the both_exist check (lines 483/1097) runs before the SUPPORTS_LLAMA32 block, and there is no check after it. Even without this PR, the error never fires on new transformers. No behavioral change.

Verified compatibility with transformers 4.57.6, 5.4.0, TRL 0.22.2, 0.27.1 -- SUPPORTS_LLAMA32 is True for all of these, and the guard behaves identically.

Tests moved to #4854 to keep this PR minimal.

[danielhanchen]

/gemini review

[gemini-code-assist]

Code Review

This pull request modifies unsloth/models/loader.py to add is_model and is_peft checks to the SUPPORTS_LLAMA32 condition. The review feedback indicates that these manual file existence checks are redundant because the successful initialization of is_model and is_peft already confirms the presence of the necessary configuration files. Additionally, the reviewer noted that the both_exist variable updated in these blocks is never used, suggesting a more unified and simplified logic to handle mixed repositories across all model versions.

[gemini-code-assist]

The manual check for file existence using os.path.exists or HfFileSystem().glob() is redundant when guarded by is_model and is_peft. If both AutoConfig.from_pretrained and PeftConfig.from_pretrained succeeded (setting is_model and is_peft to True), it already confirms that both configuration files exist and are reachable for the given model_name.

Additionally, the both_exist variable set within this block (lines 508 and 517) is never checked again in this function. The only check for both_exist occurs at line 484, which is before this block executes. This means that for SUPPORTS_LLAMA32, the "mixed repo" error is currently never raised, making this entire block a no-op that only serves to potentially cause the network hangs you are fixing.

A cleaner solution would be to unify the logic at line 481 by removing the and not SUPPORTS_LLAMA32 condition. This would fix the hang, remove the redundancy, and ensure the error is correctly raised for all versions.

[gemini-code-assist]

Similar to the check in FastLanguageModel.from_pretrained, this manual file check is redundant when is_model and is_peft are both True. Moreover, the both_exist variable updated here (lines 1284 and 1292) is not used anywhere after this block. The error check at line 1097 happens before this manual check, so the conflict is never actually flagged for Llama 3.2+ models.

Simplifying the logic at line 1095 to both_exist = is_model and is_peft (removing the version check) would be more efficient and correct.

[danielhanchen]

/gemini review

[gemini-code-assist]

Code Review

This pull request updates unsloth/models/loader.py to refine the conditional logic for Llama 3.2 support. The changes ensure that the manual check for local adapter configurations only occurs when both is_model and is_peft are true. I have no feedback to provide.

[danielhanchen]

/gemini review

[gemini-code-assist]

Code Review

This pull request updates unsloth/models/loader.py to include is_model and is_peft guards for Llama 3.2 manual configuration checks. However, the review identifies that these checks are currently no-ops because the error-handling logic for configuration conflicts occurs before these blocks are reached. The feedback suggests either moving the logic or raising errors directly within these blocks to ensure the manual checks are functional.

I am having trouble creating individual review comments. Click here to see my feedback.

unsloth/models/loader.py (501)

The manual check for conflicting config files (both_exist) is performed here for SUPPORTS_LLAMA32, but the result is never used to raise an error. The only check for both_exist occurs at line 483, which is before this block updates the variable. This means that for Llama 3.2 and newer transformers, the conflict detection is effectively disabled.

Additionally, guarding this block with is_model and is_peft might defeat the purpose of a "manual" check if it was intended to catch cases where transformers fails to detect one of the configs. If both is_model and is_peft are already True, a conflict is already known and the expensive glob call is redundant. You should probably raise the RuntimeError if both_exist is set to True within this block, or move this logic before the check at line 483.

unsloth/models/loader.py (1277)

Similar to the issue at line 501, the result of the manual check for both_exist is ignored here because the error check at line 1097 occurs before this block updates the variable. For SUPPORTS_LLAMA32, conflict detection is currently a no-op.

Furthermore, if the manual check was meant to be more robust than the is_model/is_peft detection, guarding it with is_model and is_peft defeats that purpose. If both are True, the conflict is already evident. Consider raising the RuntimeError directly if both_exist is determined to be True inside this block.

[danielhanchen]

Reviewed and tested this PR thoroughly. Merged the PR branch with the latest main (clean merge, no conflicts -- only unrelated studio/backend/colab.py change).

Testing summary (62 tests, all pass):

• 14 static analysis tests (syntax, guard lines, code consistency)
• 20 before/after simulation tests (all 8 boolean combos, edge cases, network timeout scenarios)
• 7 original PR tests from Add unit tests for loader glob skip guard (from PR #4852) #4854 (glob skip/call verification)
• 12 integration tests with real transformers 5.5.0 / peft / huggingface_hub
• 9 real loader verification tests (dead code confirmation, merge safety)

Key conclusions:

1. The PR correctly avoids unnecessary HfFileSystem().glob() calls in 3 of 4 scenarios (when is_model=False or is_peft=False)
2. No observable behavior change -- the both_exist variable written inside the guarded block is dead code (never read afterward, pre-existing issue)
3. No merge conflicts with latest main
4. No backwards/forwards compatibility issues with transformers 4.57.6 / 5.4.0 / 5.5.0 or TRL 0.22.2 / 0.27.1 / 1.0.0
5. Cross-platform path handling is correct (HfFileSystem returns POSIX paths, os.path.split handles them fine)

The change is safe to merge.