Update nemo-rl to latest

[smahdavi4]

Upgrading nemo-rl to the latest main:

• Vllm version 11.0
• Full support for async RL
• RL support with GB200 clusters.
• Config changes to align with nemo-rl and fix some of the bugs on our side (such as clipping)

Limitations:
The gradient clipping is not unified yet, we can merge after this is resolved.

Summary by CodeRabbit

• New Features

  • Asynchronous GRPO training mode with configurable weight update controls
  • LoRA and PEFT support for parameter-efficient fine-tuning

• Improvements

  • Updated build infrastructure with enhanced CUDA architecture targeting
  • Extended GRPO and SFT training configurations with expanded hyperparameter options
  • Enhanced generation and optimization parameter controls for improved performance

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[greptile-apps]

Greptile Overview

Greptile Summary

This PR upgrades nemo-rl to the latest version with vllm 11.0, adds full async RL support, and enables GB200 cluster support. The changes align with upstream nemo-rl improvements and fix several configuration issues.

Key Changes:

• Upgraded to vllm 11.0 with improved Docker build process using multi-stage builds
• Added CUDA architecture 10.0 support for GB200 clusters (TORCH_CUDA_ARCH_LIST="9.0 10.0")
• Implemented async GRPO training mode with configurable trajectory age and optional in-flight weight updates
• Added LoRA/PEFT support for parameter-efficient fine-tuning in SFT config
• Enhanced KL divergence controls with configurable types (k1, k2, k3) and clamping values
• Updated learning rate scheduler to include warmup period (10 steps) instead of constant LR
• Fixed import issue in utils.py by moving fire imports into function scope

Critical Issue:

• SFT config sets max_grad_norm: 0.0 which causes Megatron's clip_grad to be 0.0, potentially breaking training by clipping all gradients to zero

Minor Issues:

• CVE-2025-68973 in Dockerfile appears to be a non-existent or future CVE ID that should be verified

Confidence Score: 2/5

• This PR has a critical gradient clipping issue in SFT config that will break training
• The SFT config sets max_grad_norm to 0.0 which will cause clip_grad=0.0 in Megatron, clipping all gradients to zero and preventing the model from learning. This is a blocking issue that must be fixed before merge.
• Pay critical attention to nemo_skills/training/nemo_rl/configs/sft.yaml - the gradient clipping configuration will break training

Important Files Changed

Filename	Overview
dockerfiles/Dockerfile.nemo-rl	Updated to vllm 11.0, added GB200 support (CUDA arch 10.0), improved build process with multi-stage build, added CVE fixes
nemo_skills/training/nemo_rl/configs/grpo.yaml	Added async GRPO support, expanded generation configs, added KL divergence controls, updated scheduler warmup settings
nemo_skills/training/nemo_rl/configs/sft.yaml	Added LoRA/PEFT support, enabled gradient clipping with max_grad_norm=0.0, which may break training for Megatron backend

Sequence Diagram

sequenceDiagram
    participant Main as start_grpo.py
    participant Config as Config Validation
    participant AsyncGRPO as async_grpo_train
    participant SyncGRPO as grpo_train
    participant Policy as Policy Model
    participant Gen as Generation Engine
    participant Env as Environment

    Main->>Config: Check async_grpo.enabled
    
    alt Async Mode Enabled
        Config->>Config: Validate unsupported features
        Config-->>Config: Check use_dynamic_sampling
        Config-->>Config: Check reward_scaling
        Config-->>Config: Check reward_shaping
        
        alt Unsupported Feature Found
            Config-->>Main: Raise NotImplementedError
        else All Features Supported
            Main->>AsyncGRPO: Initialize async training
            AsyncGRPO->>Gen: Generate trajectories (async)
            Gen->>Policy: Sample responses
            Gen->>Env: Evaluate rewards
            Note over AsyncGRPO,Gen: Trajectories can be aged<br/>(max_trajectory_age_steps)
            AsyncGRPO->>Policy: Update weights (async)
            Note over AsyncGRPO,Policy: Optional in-flight updates<br/>and KV cache recompute
            AsyncGRPO->>AsyncGRPO: Train with aged trajectories
        end
    else Sync Mode (Default)
        Main->>SyncGRPO: Initialize standard training
        SyncGRPO->>Gen: Generate trajectories (blocking)
        Gen->>Policy: Sample responses
        Gen->>Env: Evaluate rewards
        SyncGRPO->>Policy: Update weights (blocking)
        SyncGRPO->>SyncGRPO: Train on fresh trajectories
    end

Loading

[coderabbitai]

📝 Walkthrough

Walkthrough

This pull request introduces multi-stage Docker build infrastructure for the NeMo RL container with conditional VLLM compilation, upgrades UV tooling, and adds comprehensive training configurations for GRPO and SFT workflows including async GRPO support, LoRA configuration, and SwanLab logging integration.

Changes

Cohort / File(s)	Summary
Docker multi-stage build refactoring dockerfiles/Dockerfile.nemo-rl	Introduces nemo-rl, hermetic, and release build stages with commit-driven NeMo RL fetching, UV 0.7.2→0.9.7 upgrade, conditional custom VLLM builds, CUDA architecture targeting (9.0, 10.0), and metadata/fingerprinting support.
GRPO training configuration nemo_skills/training/nemo_rl/configs/grpo-legacy-85eeb8d.yaml, nemo_skills/training/nemo_rl/configs/grpo.yaml	Adds legacy GRPO config template and updates primary config with async GRPO flags, expanded loss/checkpointing settings, KL clamping, refined generation configs (mcore + vllm), Megatron memory optimization, optimizer/scheduler refinement, and SwanLab logger support.
SFT training configuration nemo_skills/training/nemo_rl/configs/sft-legacy-85eeb8d.yaml, nemo_skills/training/nemo_rl/configs/sft.yaml	Introduces legacy SFT config template and updates primary config with metric tracking changes, dtensor v2 enhancements, LoRA/PEFT configuration blocks, dynamic batching refinement, sequence packing adjustments, optimizer/scheduler updates, and SwanLab logger support.
Async GRPO training support nemo_skills/training/nemo_rl/start_grpo.py	Adds conditional branching to invoke async GRPO training path when enabled, with validation for unsupported feature combinations and extended parameter passing.
Deferred fire import nemo_skills/utils.py	Moves fire and fire_decorators imports from module scope to function scope in check_no_extra_args_fire to support environments without fire installed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• update nemo-rl #972 — Modifies the same NEMO_RL_COMMIT build argument in dockerfiles/Dockerfile.nemo-rl
• Switch to building containers on-the-fly for local runs #969 — Adds dockerfile container reference resolution that pairs with updated nemo-rl Dockerfile and build stages
• Remove nemo-aligner and change all commands to nemo-rl #891 — Aligns with container renaming and widespread nemo-to-nemo-rl migration with environment variable support

Suggested labels

run GPU tests

Suggested reviewers

• Kipok
• wedu-nvidia

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'Update nemo-rl to latest' is generic and vague, failing to convey specific information about what was actually updated or why.	Consider a more descriptive title like 'Update nemo-rl with async RL support and vllm 11.0' that highlights the main changes and value of the update.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@nemo_skills/training/nemo_rl/configs/grpo-legacy-85eeb8d.yaml`:
- Line 279: Remove the duplicate root-level key `checkpoint_must_save_by` and
rely on the existing `checkpointing.checkpoint_must_save_by` configuration; edit
the YAML to delete the top-level `checkpoint_must_save_by: null` entry so there
is only one authoritative `checkpointing.checkpoint_must_save_by` setting, and
scan for any other duplicate top-level keys to avoid OmegaConf resolution
conflicts.

In `@nemo_skills/training/nemo_rl/configs/sft.yaml`:
- Around line 96-108: The YAML key lora_dtype in the peft config is currently
set to the literal None which may be parsed as a string; update the value to an
explicit null if you mean "unset" (set lora_dtype: null) or to a quoted string
if you mean the string "None" (set lora_dtype: "None"); locate the peft block
(peft.enabled, peft.dim, etc.) and make the change to lora_dtype accordingly.

🧹 Nitpick comments (2)

dockerfiles/Dockerfile.nemo-rl (1)

149-149: Consider pinning NeMo-Skills to a specific commit for reproducibility.

Unlike the NeMo-RL repository which uses NEMO_RL_COMMIT for version pinning, NeMo-Skills is cloned from the default branch without a specific commit reference. This means builds at different times may include different NeMo-Skills code.

If reproducibility is important, consider adding a similar NEMO_SKILLS_COMMIT build argument:

♻️ Suggested improvement

+ARG NEMO_SKILLS_COMMIT=main
-RUN git clone https://github.com/NVIDIA-NeMo/Skills.git /opt/NeMo-Skills && cd /opt/NeMo-Skills && uv pip install .
+RUN git clone https://github.com/NVIDIA-NeMo/Skills.git /opt/NeMo-Skills && cd /opt/NeMo-Skills && git checkout ${NEMO_SKILLS_COMMIT} && uv pip install .

nemo_skills/training/nemo_rl/start_grpo.py (1)

331-350: Async GRPO feature validation looks good, but verify importance sampling requirement.

The feature compatibility checks correctly prevent unsupported features from being used with async GRPO. However, the config comment in grpo-legacy-85eeb8d.yaml (lines 45-47) states:

"Async GRPO requires importance sampling correction enabled. Set to true when async_grpo.enabled is true"

Consider adding a validation or warning if async_grpo.enabled is true but loss_fn.use_importance_sampling_correction is false, as this configuration dependency isn't enforced here.

Suggested validation

         for feature in unsupported_features:
             if feature not in config["grpo"]:
                 continue

             if feature == "use_dynamic_sampling":
                 if config["grpo"][feature]:
                     raise NotImplementedError(f"{feature} is not supported with async GRPO")
             else:
                 if config["grpo"][feature]["enabled"]:
                     raise NotImplementedError(f"{feature} is not supported with async GRPO")

+        # Warn if importance sampling correction is not enabled for async GRPO
+        if not config["loss_fn"].get("use_importance_sampling_correction", False):
+            print("⚠️  Warning: Async GRPO typically requires use_importance_sampling_correction=true in loss_fn")
+
         from nemo_rl.algorithms.grpo import async_grpo_train

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a2571d0 and b8c0aaf.

📒 Files selected for processing (7)

• dockerfiles/Dockerfile.nemo-rl
• nemo_skills/training/nemo_rl/configs/grpo-legacy-85eeb8d.yaml
• nemo_skills/training/nemo_rl/configs/grpo.yaml
• nemo_skills/training/nemo_rl/configs/sft-legacy-85eeb8d.yaml
• nemo_skills/training/nemo_rl/configs/sft.yaml
• nemo_skills/training/nemo_rl/start_grpo.py
• nemo_skills/utils.py

🧰 Additional context used

🪛 Hadolint (2.14.0)

dockerfiles/Dockerfile.nemo-rl

[error] 10-10: invalid flag: --keep-git-dir

(DL1000)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: Greptile Review
• GitHub Check: pre-commit
• GitHub Check: unit-tests

🔇 Additional comments (26)

dockerfiles/Dockerfile.nemo-rl (7)

7-11: BuildKit-specific ADD flag requires recent Docker versions.

The --keep-git-dir=true flag on line 10 is a BuildKit feature for ADD with Git URLs. The # syntax=docker/dockerfile:1 directive on line 1 properly enables this, but be aware that:

1. Older Docker versions or builds without BuildKit may fail
2. Hadolint's warning is a false positive—this flag is valid with BuildKit

The stage looks good for fetching the NeMo RL repository with git metadata preserved.

---

41-42: Good: CVE remediation for gnupg.

Proactive security fix for CVE-2025-68973. The --only-upgrade flag ensures it only upgrades the existing package without pulling new dependencies.

---

49-49: UV version upgrade from 0.7.2 to 0.9.7.

This is a significant version bump. If any compatibility issues arise with dependency resolution or lock file handling, this would be a likely suspect.

---

79-80: CUDA architecture targeting for H100 and B200.

Setting TORCH_CUDA_ARCH_LIST="9.0 10.0" targets:

• 9.0: Hopper (H100)
• 10.0: Blackwell (B200/GB200)

This aligns with the PR objective of adding GB200 cluster support.

---

90-112: Conditional VLLM build and CVE mitigation look good.

The conditional custom VLLM build is properly guarded, and the CVE mitigation for aiohttp (GHSA-mqqc-3gqh-h2x8) using find -exec rm -rf is an effective approach.

One minor note: if BUILD_CUSTOM_VLLM is set but the nemo-rl.env file doesn't exist (e.g., build script failure), line 94 will fail the build. This is likely the desired behavior, but worth being aware of.

---

134-138: COPY --exclude and git unshallow logic.

The --exclude flag is another BuildKit feature (properly enabled by the syntax directive). The conditional unshallow logic on line 138 is well-designed—it checks if the repo is shallow before attempting to fetch full history.

Note: git fetch --unshallow requires network access. If the build environment is network-isolated after the initial clone, this could fail. The || true fallback handles this gracefully, though the fingerprint generation may produce different results for shallow vs. full repos.

---

146-147: OSS attribution notice generation.

Good compliance practice for NVIDIA open-source distribution requirements.

nemo_skills/utils.py (1)

508-512: LGTM! Lazy import pattern for optional dependency.

Moving fire and fire.decorators imports inside the function is a good approach for environments where fire is not installed. The comment clearly documents the rationale.

nemo_skills/training/nemo_rl/start_grpo.py (2)

351-371: LGTM! Async GRPO training integration.

The lazy import of async_grpo_train and the parameter passing looks correct. The additional max_trajectory_age_steps parameter properly sources from the async config section.

---

372-389: LGTM!

The synchronous GRPO training path is preserved correctly as the fallback when async mode is not enabled.

nemo_skills/training/nemo_rl/configs/sft-legacy-85eeb8d.yaml (2)

1-14: LGTM! Legacy SFT configuration.

The configuration provides sensible defaults. The large max_num_epochs and max_num_steps values (100000000) ensure that one of them can be set to control training duration without the other being a limiting factor.

---

56-115: Configuration aligns with PR objectives.

The Megatron configuration with empty_unused_memory_level: 0 is documented with the appropriate OOM warning. The commented-out clip_grad aligns with the PR description noting that "gradient clipping is not unified yet."

nemo_skills/training/nemo_rl/configs/grpo-legacy-85eeb8d.yaml (2)

33-50: Async GRPO configuration is properly documented.

The async GRPO section correctly defaults to disabled and includes helpful comments about importance sampling correction requirements. This aligns well with the validation in start_grpo.py.

---

82-89: The configuration does not present a conflict. Although both dtensor_cfg.enabled and megatron_cfg.enabled are set to true in the YAML file, the nemo-rl pipeline code (nemo_skills/pipeline/nemo_rl/sft.py and nemo_skills/pipeline/nemo_rl/grpo.py) explicitly enforces mutual exclusivity through CLI overrides: when backend=="megatron", dtensor is disabled; otherwise, megatron is disabled. Only one backend is active per training run by design.

nemo_skills/training/nemo_rl/configs/sft.yaml (3)

56-68: LGTM! Well-documented LoRA configuration.

The LoRA configuration section is well-structured with helpful comments explaining each parameter, including the important note about disabling Triton when tensor_parallel_size > 1.

---

216-218: LGTM! SwanLab logger integration.

The SwanLab configuration follows the established pattern used by wandb and other loggers, maintaining consistency across the logging options.

---

19-19: LGTM!

The metric name format change to "val:val_loss" with the explanatory comment about prefix options is clear and helpful.

nemo_skills/training/nemo_rl/configs/grpo.yaml (9)

1-31: Good traceability with upstream commit reference.

The source commit hash comment provides clear lineage for tracking configuration changes against upstream nemo-rl. The GRPO algorithm settings appear reasonable for math training workloads.

---

32-37: LGTM!

Conservative defaults for async GRPO with clear inline documentation. The disabled state is appropriate for synchronous training mode.

---

39-57: LGTM!

The KL divergence configuration with k3 approximation and clamping values is well-documented. The reference to the Joschu blog provides helpful context for understanding the KL type choices.

---

59-68: LGTM!

The safetensors format is a good choice for model serialization due to its safety and performance benefits. The metric naming convention with colon separator is consistent with the documented format.

---

70-100: LGTM!

The policy configuration provides good flexibility with hf_config_overrides and clear documentation for the optimizer offloading behavior. The dtensor _v2: true flag aligns with the enhanced distributed tensor tooling.

---

154-161: Verify the warmup configuration values.

lr_warmup_iters: 13 is an unusual number. Typically warmup iterations are round numbers or percentages of total training. If this is intentional (e.g., matching a specific upstream configuration), the value is fine. Otherwise, consider whether this should be a rounder number like 10 or 100.

---

175-218: LGTM!

The optimizer configuration correctly disables foreach and fused for DTensor compatibility with clear documentation. The sequential scheduler setup (LinearLR warmup → ConstantLR) with milestone at iteration 10 provides a clean warmup phase.

---

281-302: LGTM!

Good observability defaults with GPU monitoring enabled and consistent naming across logging backends. The SwanLab integration provides additional logging flexibility.

---

227-254: vLLM 0.11 compatibility confirmed for these configuration settings.

All cited vllm_cfg parameters (kv_cache_dtype: "auto", enforce_eager: False, use_deep_gemm: False) are supported in vLLM 0.11.0 and correctly configured. The mcore_generation_config defaults for KV cache management are reasonable, and the inline comment referencing convergence considerations with torch.compile provides good context for understanding the enforce_eager setting.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Duplicate checkpoint_must_save_by key at root level.

This key already exists under checkpointing.checkpoint_must_save_by (line 59). The root-level duplicate may be unintentional or could cause configuration loading issues depending on how OmegaConf resolves it.

Suggested fix

 cluster:
   gpus_per_node: 1
   num_nodes: 1
-
-checkpoint_must_save_by: null

🤖 Prompt for AI Agents

In `@nemo_skills/training/nemo_rl/configs/grpo-legacy-85eeb8d.yaml` at line 279,
Remove the duplicate root-level key `checkpoint_must_save_by` and rely on the
existing `checkpointing.checkpoint_must_save_by` configuration; edit the YAML to
delete the top-level `checkpoint_must_save_by: null` entry so there is only one
authoritative `checkpointing.checkpoint_must_save_by` setting, and scan for any
other duplicate top-level keys to avoid OmegaConf resolution conflicts.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

lora_dtype: None may not parse as expected.

In YAML, None is typically a string literal, not a null value. If the intention is to represent a null/unset value, use null instead. If it's meant to be the string "None", wrap it in quotes.

Suggested fix (if null is intended)

       a2a_experimental: false
-      lora_dtype: None
+      lora_dtype: null

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	peft:
	enabled: false
	target_modules: []
	exclude_modules: []
	dim: 8
	alpha: 32
	dropout: 0.0
	dropout_position: "post"
	lora_A_init_method: "xavier"
	lora_B_init_method: "zero"
	a2a_experimental: false
	lora_dtype: None
	peft:
	enabled: false
	target_modules: []
	exclude_modules: []
	dim: 8
	alpha: 32
	dropout: 0.0
	dropout_position: "post"
	lora_A_init_method: "xavier"
	lora_B_init_method: "zero"
	a2a_experimental: false
	lora_dtype: null

🤖 Prompt for AI Agents

In `@nemo_skills/training/nemo_rl/configs/sft.yaml` around lines 96 - 108, The
YAML key lora_dtype in the peft config is currently set to the literal None
which may be parsed as a string; update the value to an explicit null if you
mean "unset" (set lora_dtype: null) or to a quoted string if you mean the string
"None" (set lora_dtype: "None"); locate the peft block (peft.enabled, peft.dim,
etc.) and make the change to lora_dtype accordingly.

[Kipok]

let's maybe keep original defaults? Otherwise it will be harder to set LR decay. While cosine can work as both constant schedule as well as decay by just adjusting min lr

[Kipok]

and same here, ideally keep original

[greptile-apps]

_{3 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

max_grad_norm: 0.0 will set clip_grad: 0.0 in Megatron config (line 130), causing all gradients to be clipped to 0 and breaking training. The comment says "Zero means no clipping" but Megatron's clip_grad parameter interprets 0 as a clipping threshold. Should be null to disable clipping, or a positive value like 1.0.

Suggested change

	max_grad_norm: 0.0 # megatron: Zero means no clipping, FSDP: null means no clipping
	max_grad_norm: null # megatron: null means no clipping, FSDP: null means no clipping

[greptile-apps]

CVE-2025-68973 doesn't exist yet. CVE IDs follow the format CVE-YYYY-NNNNN where YYYY is the year of assignment. 2025 IDs would only be assigned in 2025. Verify this is the correct CVE number.

[greptile-apps]

Changed from constant LR (start_factor=1.0, end_factor=1.0, total_iters=1) to warmup schedule (start_factor=0.1 for 10 steps). This significantly changes training behavior - LR now starts at 10% and warms up over 10 steps.

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

[greptile-apps]

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

grpo.lr_warmup_steps doesn't exist in nemo_skills/training/nemo_rl/configs/grpo.yaml. The test will fail with this parameter.

Suggested change

	"++grpo.lr_warmup_steps=2 "
	"++grpo.num_prompts_per_step=2 "

[greptile-apps]

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

tests fail

[greptile-apps]

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

logic error: should be or not and. If .hf_metadata exists as a file (not directory), this check won't catch it.

Suggested change

	if not os.path.exists(hf_metadata_dir) and not os.path.isdir(hf_metadata_dir):
	if not os.path.exists(hf_metadata_dir) or not os.path.isdir(hf_metadata_dir):

[greptile-apps]

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

logic error: should be or not and. If .hf_metadata exists as a file (not directory), this check won't catch it.

Suggested change

	if not os.path.exists(hf_metadata_dir) and not os.path.isdir(hf_metadata_dir):
	if not os.path.exists(hf_metadata_dir) or not os.path.isdir(hf_metadata_dir):

[greptile-apps]

_{1 file reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

missing tokenizer files when tokenizer_path is not a local directory

When tokenizer_path is a HuggingFace model ID (e.g., meta-llama/Llama-3.2-1B), os.path.exists(src) will fail for all files since it's not a local path. This will silently skip copying tokenizer files, potentially breaking the converted checkpoint.

Consider downloading tokenizer files from HF first or handling the case where tokenizer_path is a model ID.

[greptile-apps]

os.path.isdir() check is insufficient

If .hf_metadata exists as a file (not directory), this will incorrectly return False and use the wrong conversion path. Use:

Suggested change

	hf_metadata_path = os.path.join(weights_path, "model", ".hf_metadata")
	return os.path.isdir(hf_metadata_path)
	return os.path.exists(hf_metadata_path) and os.path.isdir(hf_metadata_path)

[greptile-apps]

_{No files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}