ci: pr-2384

[chtruong814]

What does this PR do ?

ci: pr-2384

Changelog

• Add specific line by line info of high level changes in this PR.

GitHub Actions CI

See the CI sectionin the Contributing doc for how to trigger the CI. A Nvidia developer will need to approve and trigger the CI for external contributors.

Before your PR is "Ready for review"

Pre checks:

• Make sure you read and followed Contributor guidelines
• Did you write any new necessary tests?
• Did you add or update any necessary documentation?
• Does the PR affect components that are optional to install? (Ex: Numba, Pynini, Apex etc)
  • Reviewer: Does the PR have correct import guards for all optional libraries?

If you haven't finished some of the above items you can still open "Draft" PR.

Additional Information

• Related to # (issue)

Summary by CodeRabbit

• Bug Fixes

  • Improved handling of None values and missing fields in model configuration conversion to prevent unintended field assignments.

• Tests

  • Added and updated unit tests to validate proper preservation of None values across model configuration bridging for DeepSeek, Gemma, Nemotron, and Qwen models.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[chtruong814]

/ok to test 21a1316

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6679c17 and 21a1316.

📒 Files selected for processing (5)

• src/megatron/bridge/models/conversion/model_bridge.py
• tests/unit_tests/models/deepseek/test_deepseek_bridges.py
• tests/unit_tests/models/gemma_vl/test_gemma3_vl_bridge.py
• tests/unit_tests/models/nemotron_vl/test_nemotron_vl_bridge.py
• tests/unit_tests/models/qwen_vl/test_qwen25_vl_bridge.py

---

📝 Walkthrough

Walkthrough

The pull request changes the field mapping logic in the model bridge from value-driven (checking if a value is not None) to presence-driven (verifying if a field actually exists). This prevents unintended None values from being written to configurations when fields are explicitly set to None. Updates also include test fixtures using stricter mock specifications to validate the new presence-aware behavior.

Changes

Cohort / File(s)	Summary
Bridge Conversion Logic src/megatron/bridge/models/conversion/model_bridge.py	Refactored hf_config_to_provider_kwargs and megatron_to_hf_config to use presence-driven field mapping via has_value flags and hasattr checks instead of None-value checks. Nested dot notation now validates parent dict/attribute existence before assignment.
DeepSeek Bridge Tests tests/unit_tests/models/deepseek/test_deepseek_bridges.py	Added unit tests validating that None values for q_lora_rank in DeepSeekV2 and DeepSeekV3 are preserved through the bridging workflow (hf_config_to_provider_kwargs, provider_bridge, and megatron_to_hf_config).
Vision-Language Model Test Fixtures tests/unit_tests/models/gemma_vl/test_gemma3_vl_bridge.py, tests/unit_tests/models/nemotron_vl/test_nemotron_vl_bridge.py, tests/unit_tests/models/qwen_vl/test_qwen25_vl_bridge.py	Updated mock fixtures to use Mock(spec=[]) for stricter attribute restriction, removed MLA-specific field assignments from test scaffolding, and added torch_dtype attributes to align with real HF config behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• [model, refactor] refactor: Centralize provider_bridge config mapping in base class for VLM models #2250: Modifies provider_bridge config mappings and ACTIVATION_MAPPING in the same model_bridge.py file with related configuration handling changes.

Suggested labels

bug, Run CICD

Suggested reviewers

• yaoyu-33
• cuichenx
• yashaswikarnati

🚥 Pre-merge checks | ✅ 1 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Test Results For Major Changes	⚠️ Warning	PR includes comprehensive unit tests validating the None-value preservation fix, but PR description lacks documented test results or execution confirmation.	Add explicit test results to PR description: confirmation that unit tests pass, test runner output, and verification that the regression is resolved.
Title check	❓ Inconclusive	The PR title 'ci: pr-2384' is vague and does not clearly describe the actual changes, which involve fixing a regression in config bridging logic and adjusting test cases. The title reads like a placeholder referencing an issue number rather than summarizing the substantive work.	Consider using a more descriptive title that captures the main change, such as 'Fix regression in config bridging to preserve None-valued fields' or 'Preserve None values in HF config to provider kwargs bridging'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch chtruong/cp-2384

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

---

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.}