Skip to content

Studio: fix tier detection for models loaded via custom folder path - #6396

Merged
danielhanchen merged 29 commits into
unslothai:mainfrom
LeoBorcherding:local-config-transformers-tier
Jun 22, 2026
Merged

danielhanchen merged 29 commits into
unslothai:mainfrom
LeoBorcherding:local-config-transformers-tier

Conversation

@LeoBorcherding

@LeoBorcherding LeoBorcherding commented Jun 17, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #6395

What

get_transformers_tier() short-circuits to "default" (transformers 4.57.x)
when a local config.json is found but its architecture isn't in the known
sidecar sets, bypassing name-based detection entirely. This misroutes any
5.3.0-tier model loaded from a custom folder path.

Changes

  • Expand _TRANSFORMERS_530_ARCHITECTURES / _MODEL_TYPES with all confirmed
    families: Qwen3.5, Qwen3 MoE, GLM-4.7-Flash, LFM2.5-VL
  • When a local config.json doesn't match any known set, fall back to
    _resolve_base_model() (already extracts _name_or_path) and run substring
    detection on the resolved HF ID, handles renamed folders without
    directory-name false positives
  • Suppress the spurious GPU-estimate warning in hardware.py that fires for
    sidecar-tier models whose config.json can't be parsed by default transformers
  • Extract _tier_from_name() to deduplicate the substring logic used by both
    the local-config fallback and the remote-path fast check
  • 79 tests passing

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

@LeoBorcherding
LeoBorcherding marked this pull request as draft June 17, 2026 07:23

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for the 5.3.0 transformers tier (e.g., for Qwen3.5) and refactors the tier detection logic to check local config.json files before applying name heuristics. It also updates GPU estimation logging to handle expected 5.x-only config parsing failures gracefully and adds comprehensive unit tests. The reviewer suggests a high-severity improvement to recursively call get_transformers_tier(resolved) instead of _tier_from_name(resolved) when falling back to the resolved base model, ensuring the full tier detection logic is applied to the resolved model.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread studio/backend/utils/transformers_version.py
LeoBorcherding and others added 7 commits June 17, 2026 14:47
…ckpoints

A local safetensors folder whose config.json did not match the Gemma4 (510/550)
architecture signals short-circuited get_transformers_tier() to "default"
(transformers 4.57.x), never reaching the name-substring check that routes
Qwen3.5 to the 5.3.0 sidecar. So a local Qwen3.5 checkpoint (model_type
"qwen3_5", needs transformers >= 5.2.0) loaded with 4.57.x and failed with
"does not support Qwen3.5". The same model as a remote HF id worked, because it
has no local config.json to trigger the short-circuit.

Detect the 5.3.0 tier from config.json (model_type "qwen3_5" / architecture
Qwen3_5ForCausalLM) in the local-config branch, mirroring the existing Gemma4
510/550 handling. This is a positive config signal, so it fixes local Qwen3.5
without weakening the directory-name false-positive guard (a llama checkpoint
under a "gemma-4-12b-*" parent still resolves to default).

Adds tests for the config-based 530 detection and local-folder tier resolution.
…lies

Expands the config.json-based tier detection to cover all known 5.3.0-tier
model families (Qwen3 MoE, GLM-4.7-Flash, LFM2.5-VL) and adds a _name_or_path
fallback so renamed local checkpoints with unrecognised model_type values still
route correctly via the HF ID embedded in their config.json.

- Expand _TRANSFORMERS_530_ARCHITECTURES / _MODEL_TYPES with verified entries
  from Qwen3MoeForCausalLM, Glm4MoeLiteForCausalLM, Lfm2VlForConditionalGeneration,
  and Qwen3_5ForConditionalGeneration (confirmed from local Qwen3.5-2B config.json)
- Extract _tier_from_name() helper, deduplicating the fast-substring logic used
  by both the remote-path branch and the new config _name_or_path fallback
- In the local-config branch: after architecture checks, resolve the tier from
  cfg._name_or_path / cfg.model_name before returning "default", preserving the
  existing directory-name false-positive guard
- 79 tests passing
@LeoBorcherding
LeoBorcherding force-pushed the local-config-transformers-tier branch from 3c83a33 to 043709b Compare June 17, 2026 19:47
@LeoBorcherding
LeoBorcherding marked this pull request as ready for review June 18, 2026 13:44

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

if _check_config_needs_550(model_name):
logger.info("Transformers tier 550 selected for %s (config.json check)", model_name)
return "550"
if _check_tokenizer_config_needs_v5(model_name):

P1 Badge Add 5.3 config detection to the HF slow path

When a model is referenced by a Hub ID whose name does not contain one of the 5.3 substrings, this slow path still only checks 510/550 config and then falls through to tokenizer/default. That leaves the newly added qwen3_moe/qwen3_5/glm4_moe_lite config signals unused for renamed/private repos; it also affects local custom folders whose _name_or_path is resolved before activation, because activate_transformers_for_subprocess() calls _resolve_base_model() before get_transformers_tier(). In those cases a valid 5.3-only config can be classified as default and loaded under transformers 4.57.x.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread studio/backend/utils/transformers_version.py Outdated
Comment thread studio/backend/utils/transformers_version.py
Comment thread studio/backend/utils/transformers_version.py
… probes

Using get_transformers_tier(resolved) on the _name_or_path fallback would
trigger up to 3 network fetches (config.json + tokenizer_config.json, 10s
each) for every ordinary checkpoint whose _name_or_path is a plain HF ID
like meta-llama/Llama-3-8B. The fallback's purpose is name-based detection
on the resolved HF ID, _tier_from_name covers all known cases without I/O.
Private or renamed HF repos whose model IDs lack a 5.3 substring were
silently routed to the default tier. _check_config_needs_530 mirrors the
existing 510/550 pattern: fetches config.json once, caches the result, and
is called after the 550 check in the slow path. Includes 5 unit tests.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fdc074fc7e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread studio/backend/utils/transformers_version.py Outdated
LeoBorcherding and others added 2 commits June 18, 2026 09:46
…ives

When _name_or_path in config.json is an absolute path to the same checkpoint
passed as a relative path, the textual resolved != model_name check passes
and _tier_from_name would scan the directory path for substrings. Split the
fallback: local directories recurse into get_transformers_tier (config check,
no network I/O); HF Hub IDs use _tier_from_name (name-based, no network).

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 29952331f4

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread studio/backend/utils/transformers_version.py
Comment thread studio/backend/utils/transformers_version.py
Comment thread studio/backend/utils/transformers_version.py
LeoBorcherding and others added 3 commits June 18, 2026 11:34
- _norm_separators(): collapse _ . whitespace to - so underscore/dot model
  ID variants (Qwen3_5, Qwen3_Next) match the canonical substring list
- _tier_from_name(): apply norm to both name and each substring so aliases
  resolve without duplicating the substring lists
- _resolve_base_model(): try model_name then _name_or_path separately so a
  self-referential Unsloth model_name doesn't hide the useful HF ID in
  _name_or_path
- Gate get_base_model_from_lora on adapter_cfg_path.is_file() to avoid
  eagerly importing transformers before the sidecar venv is on sys.path
- 17 new tests covering _norm_separators, separator-insensitive
  _tier_from_name, and the model_name/_name_or_path fallback

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: da225a669a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread studio/backend/utils/transformers_version.py Outdated
LeoBorcherding and others added 2 commits June 19, 2026 07:23
activate_transformers_for_subprocess and ensure_transformers_version were
pre-resolving all local checkpoints via _resolve_base_model before calling
get_transformers_tier. After the model_name/_name_or_path fix, a full
checkpoint with a private/offline _name_or_path and no tier substring would
resolve to that HF ID, which can't be probed, bypassing the local config.json
model_type check entirely. Gate pre-resolution on adapter_config.json so full
checkpoints go straight to get_transformers_tier, which reads config.json
directly. LoRA adapters still pre-resolve as before.
@LeoBorcherding

LeoBorcherding commented Jun 19, 2026 •

Copy link
Copy Markdown
Collaborator Author

tested again, LGTM after CI finishes

image

danielhanchen and others added 2 commits June 20, 2026 10:56
…positives

- Add Qwen3.5 MoE (qwen3_5_moe / Qwen3_5MoeForConditionalGeneration) and
  Qwen3-Next to the 5.3.0 config sets, so renamed local checkpoints route to
  the sidecar instead of default transformers
- Let a 510/550 name match override a 530 config match, so Qwen3.6 (which
  reuses qwen3_5 / qwen3_5_moe config ids) still routes to the 5.5.0 sidecar
- Stop normalizing version dots to hyphens so size names like Qwen3-5B and
  Qwen3-6B are not promoted to a 5.x sidecar; underscore aliases still match
- Skip name matching for resolved values that look like stale local paths
@danielhanchen

Copy link
Copy Markdown
Member

Pushed a follow-up commit (cf72db5) on top of this to close a few gaps I hit while testing the tier detection against the live Hub configs.

1. Qwen3.5 MoE and Qwen3-Next were missing from the 5.3.0 config sets. The flagship Qwen3.5 MoE checkpoints (Qwen3.5-35B-A3B, Qwen3.5-122B-A10B) use model_type: qwen3_5_moe / Qwen3_5MoeForConditionalGeneration, and Qwen3-Next uses qwen3_next / Qwen3NextForCausalLM. A renamed local folder for those still fell through to default transformers, which is the same failure this PR fixes for the dense case. Added both to _TRANSFORMERS_530_ARCHITECTURES / _TRANSFORMERS_530_MODEL_TYPES.

2. Qwen3.6 precedence. Qwen3.6 reuses the Qwen3.5 config ids (Qwen3.6-27B is qwen3_5, Qwen3.6-35B-A3B is qwen3_5_moe), so once the 530 config check exists, a local Qwen3.6 folder matches it and routes to the 5.3.0 sidecar instead of 5.5.0. Added a name-based override so a 510/550 name match wins over a 530 config match.

3. Dot-version false positives. _norm_separators collapsed . to -, so qwen3.6 matched Qwen3-6B and qwen3.5 matched Qwen3-5B, promoting plain size names to a 5.x sidecar (this returned default on main). Stopped normalizing the dot; underscore aliases like Qwen3_5 still resolve to the dotted form.

4. Stale _name_or_path paths. The local-config fallback could name-match a stale or renamed local path, so a non-5.x checkpoint whose saved origin path contained a 5.x substring got promoted. Added a small guard so only Hub-id-shaped values are name-matched.

Added regression tests for each; the suite is at 109 passing (studio/backend/tests/test_transformers_version.py).

One thing I left out on purpose: I did not add mistral3 / Mistral3ForConditionalGeneration to the config set even though Ministral-3 is a 5.3 model, since that arch and model_type are shared with Mistral-Small-3 (default tier) and would misroute it. Ministral-3 stays on the name rule (ministral-3-), which is specific enough.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 99b8d7f0bf

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread studio/backend/utils/transformers_version.py Outdated
LeoBorcherding and others added 2 commits June 20, 2026 09:09
…ath-hint guard

- adapter_model-only LoRA: add import-light _is_lora_adapter_dir/_has_adapter_weights
  and gate activation/export pre-resolve on them, so LoRA dirs with
  adapter_model*.safetensors but no adapter_config.json still resolve to their base
  model (via _resolve_base_model's new unsloth_<model>_<ts> directory-name parse)
  instead of tiering off the adapter folder.
- 530 override: only treat a resolved value as a name hint when it is a real Hub id;
  a stale/renamed local path in model_name/_name_or_path can no longer flip a correct
  530 config to 550. Current folder basename still allowed.

Added 7 regression tests; suite at 116 passing.
@LeoBorcherding

Copy link
Copy Markdown
Collaborator Author

Tested the latest commit end to end on Windows: a Qwen3.5-2B checkpoint loaded
via a custom folder path now loads and responds normally. Before this PR the same
model failed with "This model is not supported yet. Try a different model." since
tier detection skipped the 5.3.0 sidecar for local checkpoints.

Before:
image

After:
image

Also pushed fixes for the two open codex P2s on top of Daniel's commit:

  • adapter_model-only LoRA: dirs with adapter_model*.safetensors but no
    adapter_config.json now resolve to their base model (via the
    unsloth__ directory name) instead of tiering off the adapter
    folder, so they activate the base model's sidecar.
  • 530 override: a stale or renamed local path in model_name / _name_or_path can
    no longer flip a correct 530 config to 550. Only a real Hub id (or the current
    folder basename) is trusted as a name hint.

Added 7 regression tests, suite is at 116 passing.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1bae385d44

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +662 to +665
hint_src = (
base
if (base != model_name and _looks_like_hf_id(base))
else Path(model_name).name

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restrict 530 config overrides to real model hints

When a local checkpoint's config positively matches the 530 set but has no usable HF ID, this falls back to the arbitrary folder basename and lets any higher-tier substring override the config. For example, a Qwen3.5 checkpoint saved in a custom directory named gemma-4-12b-experiment will select tier 510 even though its model_type/architecture says qwen3_5, defeating the local-config guard this change adds. The override should be limited to the specific Qwen3.6 signal or a trusted HF ID, rather than all 510/550 name matches from the local folder name.

Useful? React with 👍 / 👎.


# Architecture classes / model_type values that require transformers 5.3.0.
# Checked via config.json (local or HuggingFace).
_TRANSFORMERS_530_ARCHITECTURES: set[str] = {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include Mistral3 config identifiers in 530 tier

The new 530 config allow-list still omits the actual Ministral-3 identifiers: the upstream mistralai/Ministral-3-8B-Instruct-2512 config uses architectures: ["Mistral3ForConditionalGeneration"] and model_type: "mistral3". A locally saved Ministral-3 checkpoint in a custom folder without a useful _name_or_path therefore enters the local-config path, misses _config_needs_530, and returns default, so the worker won't activate the 5.3 sidecar for one of the families this module explicitly lists as 5.3-only.

Useful? React with 👍 / 👎.

@LeoBorcherding

Copy link
Copy Markdown
Collaborator Author

@codex review

@LeoBorcherding

Copy link
Copy Markdown
Collaborator Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for a new transformers tier ('530') to accommodate models like Qwen3.5, Qwen3 MoE, GLM-4.7, and LFM2.5-VL. It refactors the tier detection and base model resolution logic in utils/transformers_version.py to be more robust, improves LoRA adapter detection for weight-only directories, and updates hardware configuration logging to gracefully handle sidecar-dependent models. The review feedback highlights several opportunities to enhance robustness and performance, such as leveraging the cached _load_config_json helper to avoid redundant disk I/O, defensively validating types and resolving paths during base model resolution, ensuring _looks_like_hf_id handles empty strings, and expanding exception handling in _is_lora_adapter_dir to prevent potential permission or OS-level crashes.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines 288 to +302
try:
with open(config_json_path) as f:
cfg = json.load(f)
# Unsloth writes "model_name"; HF writes "_name_or_path"
base = cfg.get("model_name") or cfg.get("_name_or_path")
if base and base != str(local_path):
logger.info(
"Resolved checkpoint '%s' → base model '%s' (via config.json)",
model_name,
base,
)
return base
# Unsloth writes "model_name"; HF writes "_name_or_path". Try both:
# if "model_name" is self-referential (equals the local path), the
# useful base id may still live in "_name_or_path".
for _key in ("model_name", "_name_or_path"):
base = cfg.get(_key)
if base and base != str(local_path):
logger.info(
"Resolved checkpoint '%s' → base model '%s' (via config.json)",
model_name,
base,
)
return base

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To avoid redundant synchronous disk I/O and leverage the existing _config_json_cache, use the _load_config_json helper instead of manually opening and parsing config.json again.

Suggested change
try:
with open(config_json_path) as f:
cfg = json.load(f)
# Unsloth writes "model_name"; HF writes "_name_or_path"
base = cfg.get("model_name") or cfg.get("_name_or_path")
if base and base != str(local_path):
logger.info(
"Resolved checkpoint '%s' → base model '%s' (via config.json)",
model_name,
base,
)
return base
# Unsloth writes "model_name"; HF writes "_name_or_path". Try both:
# if "model_name" is self-referential (equals the local path), the
# useful base id may still live in "_name_or_path".
for _key in ("model_name", "_name_or_path"):
base = cfg.get(_key)
if base and base != str(local_path):
logger.info(
"Resolved checkpoint '%s' → base model '%s' (via config.json)",
model_name,
base,
)
return base
try:
cfg = _load_config_json(model_name)
if cfg is not None:
# Unsloth writes "model_name"; HF writes "_name_or_path". Try both:
# if "model_name" is self-referential (equals the local path), the
# useful base id may still live in "_name_or_path".
for _key in ("model_name", "_name_or_path"):
base = cfg.get(_key)
if base and base != str(local_path):
logger.info(
"Resolved checkpoint '%s' → base model '%s' (via config.json)",
model_name,
base,
)
return base
References
  1. Centralize recurring or complex logical checks into a single helper function and reuse it across the codebase to ensure consistency and simplify maintenance.

Comment on lines +295 to +296
base = cfg.get(_key)
if base and base != str(local_path):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Defensively verify that base is a string (isinstance(base, str)) before comparing and returning it. If a malformed or custom config.json contains an unexpected type (like a list or dict) for model_name or _name_or_path, it could cause a TypeError crash later when passed to Path() or other string-based operations.

Suggested change
base = cfg.get(_key)
if base and base != str(local_path):
base = cfg.get(_key)
if base and isinstance(base, str) and base != str(local_path):

# useful base id may still live in "_name_or_path".
for _key in ("model_name", "_name_or_path"):
base = cfg.get(_key)
if base and base != str(local_path):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

When base is a local path, comparing it directly as a string with str(local_path) can fail to detect that they point to the same directory if one is relative and the other is absolute. To make this comparison robust and avoid redundant recursion, resolve both paths when base is a local path.

Suggested change
if base and base != str(local_path):
if base and isinstance(base, str):
try:
base_path = Path(base)
if base_path.exists() and base_path.resolve() == local_path.resolve():
continue
except Exception:
pass
if base != str(local_path):

Comment on lines +579 to +584
def _looks_like_hf_id(value: str) -> bool:
"""True if *value* looks like a Hub id (``org/name``) rather than a local
filesystem path, so a stale/renamed checkpoint path isn't name-matched."""
if os.path.isabs(value) or value.startswith((".", "~")) or "\\" in value:
return False
return value.count("/") <= 1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Ensure that value is not empty or whitespace-only before performing the checks. An empty string would currently return True as a valid Hugging Face ID.

Suggested change
def _looks_like_hf_id(value: str) -> bool:
"""True if *value* looks like a Hub id (``org/name``) rather than a local
filesystem path, so a stale/renamed checkpoint path isn't name-matched."""
if os.path.isabs(value) or value.startswith((".", "~")) or "\\" in value:
return False
return value.count("/") <= 1
def _looks_like_hf_id(value: str) -> bool:
"""True if *value* looks like a Hub id (``org/name``) rather than a local
filesystem path, so a stale/renamed checkpoint path isn't name-matched."""
if not value or not value.strip():
return False
if os.path.isabs(value) or value.startswith((".", "~")) or "\\" in value:
return False
return value.count("/") <= 1

Comment on lines +243 to +256
def _is_lora_adapter_dir(path: Path) -> bool:
"""True if *path* is a local LoRA adapter directory.

Mirrors ``utils.models._looks_like_lora_adapter`` but stays import-light so it
can run during subprocess activation without dragging in transformers. Detects
both ``adapter_config.json`` adapters and adapter_model-only LoRAs (weights
present, config absent) that a config-only check would miss.
"""
try:
if not path.is_dir():
return False
except OSError:
return False
return (path / "adapter_config.json").is_file() or _has_adapter_weights(path)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Wrap the entire check in the try-except block to prevent potential OSError or PermissionError crashes when calling is_file() or checking adapter weights on directories with restricted permissions.

Suggested change
def _is_lora_adapter_dir(path: Path) -> bool:
"""True if *path* is a local LoRA adapter directory.
Mirrors ``utils.models._looks_like_lora_adapter`` but stays import-light so it
can run during subprocess activation without dragging in transformers. Detects
both ``adapter_config.json`` adapters and adapter_model-only LoRAs (weights
present, config absent) that a config-only check would miss.
"""
try:
if not path.is_dir():
return False
except OSError:
return False
return (path / "adapter_config.json").is_file() or _has_adapter_weights(path)
def _is_lora_adapter_dir(path: Path) -> bool:
"""True if *path* is a local LoRA adapter directory.
Mirrors ``utils.models._looks_like_lora_adapter`` but stays import-light so it
can run during subprocess activation without dragging in transformers. Detects
both ``adapter_config.json`` adapters and adapter_model-only LoRAs (weights
present, config absent) that a config-only check would miss.
"""
try:
if not path.is_dir():
return False
return (path / "adapter_config.json").is_file() or _has_adapter_weights(path)
except OSError:
return False

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1bae385d44

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +582 to +584
if os.path.isabs(value) or value.startswith((".", "~")) or "\\" in value:
return False
return value.count("/") <= 1

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject relative local paths before name tiering

In the local-config fallback, _resolve_base_model() can return _name_or_path exactly as saved by Transformers; relative local values such as runs/qwen3.5-source (or a basename) are not absolute, do not start with ./~, and contain at most one slash, so this predicate treats them as Hub IDs and the fallback then applies _tier_from_name. A plain local Llama checkpoint with "_name_or_path": "runs/qwen3.5-source" is therefore routed to the 5.3 sidecar even though the config itself has no 5.x architecture, reintroducing the path-substring false positive the local config guard is meant to avoid.

Useful? React with 👍 / 👎.

if _check_config_needs_550(model_name):
logger.info("Transformers tier 550 selected for %s (config.json check)", model_name)
return "550"
if _check_config_needs_530(model_name):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor Qwen3.6 name hints in remote configs

For a renamed/private Hub repo whose model ID lacks qwen3.6 but whose fetched config has model_type: qwen3_5 and _name_or_path: "Qwen/Qwen3.6-...", this slow-path check returns 530 immediately. The local-config branch above has an explicit Qwen3.6 override because those configs reuse Qwen3.5 IDs but require the 5.5 sidecar; the remote path should apply the same _name_or_path hint before selecting 530, otherwise these renamed Qwen3.6 repos load under transformers 5.3.0.

Useful? React with 👍 / 👎.

Comment on lines +118 to +125
_TRANSFORMERS_530_MODEL_TYPES: set[str] = {
"qwen3_5",
"qwen3_5_moe",
"qwen3_moe",
"qwen3_next",
"glm4_moe_lite",
"lfm2_vl",
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include Qwen3.5 text config model types

The Qwen3.5 text-only configs use model types such as qwen3_5_text (and the MoE text config uses qwen3_5_moe_text), but this model-type set only includes the multimodal wrapper IDs. If a local fine-tuned/custom folder has one of those text config model_type values and its architectures entry is missing or stripped, _config_needs_530 returns false and the local-config path can fall back to default transformers, which cannot parse these Qwen3.5 text configs.

Useful? React with 👍 / 👎.

- Add Qwen3.5 text-tower model types (qwen3_5_text / qwen3_5_moe_text) to the
  5.3.0 config set so text-only configs with stripped architectures still route
  to the sidecar
- Apply the Qwen3.6 name override on the remote slow path too, so a renamed or
  private repo whose config reuses qwen3_5 ids but names Qwen3.6 in
  _name_or_path selects 5.5.0 instead of 5.3.0
- Treat an existing local path (or empty value) as a path, not a Hub id, in
  _looks_like_hf_id so a real local checkpoint folder is not name matched
- Guard _resolve_base_model against non-string config values and compare paths
  by realpath so relative or absolute self references resolve correctly
- Keep the LoRA adapter is_file check inside the OSError guard
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.

@danielhanchen

Copy link
Copy Markdown
Member

Pushed bde4f3f addressing the latest codex and gemini comments.

Qwen3.5 text towers (codex P2). Added qwen3_5_text and qwen3_5_moe_text to the 5.3.0 model_type set. Confirmed against the transformers config sources (the qwen3_5 module defines qwen3_5_text, qwen3_5_moe defines qwen3_5_moe_text), so a text-only config with stripped architectures now routes to the sidecar.

Remote Qwen3.6 override (codex P2). The slow config path now applies the same _name_or_path name override as the local path, so a renamed or private repo whose config reuses the qwen3_5 ids but names Qwen3.6 selects 5.5.0 instead of 5.3.0. Factored the override into a shared helper so both paths stay in sync.

Relative path guard (codex P2). _looks_like_hf_id now rejects empty/whitespace values and anything that exists as a local path, mirroring how transformers itself resolves a repo id vs a local path. A fully stale, deleted relative path that still happens to be org/name shaped is indistinguishable from a Hub id by string form, and transformers would treat it as a repo id too, so I left that case as is.

_resolve_base_model hardening (gemini). Added an isinstance(base, str) guard so a malformed config (list/dict for model_name/_name_or_path) cannot crash, and switched the self-reference check to compare realpaths so relative vs absolute references to the same folder resolve correctly.

_is_lora_adapter_dir (gemini). Moved the adapter_config.json is_file() check inside the OSError guard.

On the gemini suggestion to route _resolve_base_model's config read through _load_config_json: I kept the explicit local read there on purpose. That function runs during subprocess activation and must stay strictly local, and _load_config_json would attempt a Hub fetch for any value that is not an on-disk config, so I would rather not widen its behavior on that path.

Suite is at 123 passing.

@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: bde4f3f4fd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

- _config_matches_tier no longer raises TypeError when a malformed config.json
  carries a non-string model_type (e.g. a list) or non-list architectures; it
  fails open to no-match
- guard the model_name-derived is_file/is_dir probes with _safe_is_file /
  _safe_is_dir so a pathological or over-long path (e.g. a Windows long path)
  fails open to the default tier instead of raising OSError

No routing changes for any valid model; purely defensive. Verified by a
cross-platform simulation (POSIX + NT path semantics) and a before/after tier
matrix that is unchanged for all previously supported models.
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.

@danielhanchen

Copy link
Copy Markdown
Member

Ran an isolated cross-platform simulation of the tier detection (uv venv, POSIX and Windows path semantics via posixpath/ntpath, plus the caller and hardware.py paths) to make sure this does not regress anything.

Before/after, across 32 representative cases: 12 previously-broken local custom-folder loads now route to the correct sidecar (Qwen3.5 dense/MoE/text, Qwen3-Next, Qwen3 MoE, GLM-4.7-Flash, LFM2.5-VL, Qwen3.6, and renamed folders), 20 previously-working cases are unchanged, and 0 mismatches. Mistral-Small-3 (which shares the mistral3 arch with Ministral-3) correctly stays on default, and Qwen3-5B/Qwen3-6B size names are no longer promoted.

While simulating edge cases I found and fixed two robustness gaps (810d3aa):

  • _config_matches_tier raised TypeError on a malformed config.json with a non-string model_type (e.g. a list). It now fails open to no-match.
  • the model_name-derived is_file/is_dir probes could raise OSError on a pathological or over-long path (matters for Windows long paths). They now fail open to the default tier.

Both are purely defensive: the before/after tier matrix is identical for every valid model. Suite is at 128 passing.

Shorten/remove over-long comments and docstrings, mainly on internal helpers,
without changing behavior. Verified code-only via comment_tools.py check; suite
unchanged at 128 passing.
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.

1 similar comment
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.

Reconcile tier detection with main's NemotronH 5.10 routing (unslothai#6541) and SSM
kernel auto-install (unslothai#6535). The production transformers_version.py auto-merged
cleanly; resolve the test import block to the union of both symbol sets and
update the adapter path-name recheck test to build a real adapter dir, so the
resolved base drives the tier (path name never upgrades it).
@danielhanchen
danielhanchen force-pushed the local-config-transformers-tier branch from 013a108 to 2d50693 Compare June 22, 2026 12:31
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.

@danielhanchen
danielhanchen merged commit 040858c into unslothai:main Jun 22, 2026
27 of 35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Studio: models loaded via custom folder path fail — tier detection skips sidecar for local checkpoints

3 participants