fix: preserve original model path for frontend config downloads by yurekami · Pull Request #5102 · ai-dynamo/dynamo

yurekami · 2025-12-27T22:09:47Z

Overview

Fixes the frontend model download bug where using --served-model-name causes config download to fail because the customized name doesn't match any HuggingFace repository.

Details

Root Cause

When --served-model-name is used, set_name() overwrites display_name with the custom name. Later, download_config() uses display_name to fetch from HuggingFace, but the custom name isn't a valid HuggingFace repository path.

Solution

Add source_model field to ModelDeploymentCard - preserves the original HuggingFace repository path
Update set_name() - automatically saves the original display_name to source_model before overwriting (addresses CodeRabbit's review feedback)
Update download_config() - uses source_model when available, falls back to display_name for backward compatibility
Initialize source_model: None in from_repo_checkout() - for local paths where no HF download is needed

Key Improvement

The set_name() fix ensures that whenever --served-model-name is called, the original model path is automatically preserved. This is more robust than requiring callers to manually set source_model.

Where to start

lib/llm/src/model_card.rs - All changes are in this file

Related Issues

Fixes #4748

Test Plan

Deploy a model with --served-model-name custom-name
Verify frontend can still download config files from the original HuggingFace repository
Verify backward compatibility when source_model is not set

Note: This PR is based on the fork's main branch. There may be merge conflicts with upstream due to recent changes. If so, please use "Update with merge" or squash-merge after resolving conflicts in lib/llm/src/model_card.rs.

Summary by CodeRabbit

Release Notes

Improvements
- Enhanced model download reliability through improved handling of model source path resolution
- Strengthened tracking of model origins for better resource management

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

copy-pr-bot · 2025-12-27T22:09:51Z

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.

github-actions · 2025-12-27T22:09:54Z

👋 Hi yurekami! Thank you for contributing to ai-dynamo/dynamo.

Just a reminder: The NVIDIA Test Github Validation CI runs an essential subset of the testing framework to quickly catch errors.Your PR reviewers may elect to test the changes comprehensively before approving your changes.

🚀

coderabbitai · 2025-12-27T22:13:07Z

Walkthrough

Added source_model: Option<String> field to ModelDeploymentCard to preserve the original HuggingFace model identifier. The set_name method now saves the current display name before renaming, and download_config uses this original identifier for model downloads, falling back to display name when unavailable.

Changes

Cohort / File(s)	Summary
Model Card Source Tracking `lib/llm/src/model_card.rs`	Added `source_model` field to store original HuggingFace repository path. Updated `set_name()` to preserve original model path when renaming. Modified `download_config()` to resolve downloads using `source_model` (if available) instead of potentially renamed `display_name`. Updated all struct initialization paths to include `source_model: None`.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A field was born to remember the source,
When names got changed, the download stayed true,
No more chasing the served-name, of course!
The original path shines bright and brand new. 🌟

Pre-merge checks

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: preserving the original model path for frontend config downloads, which directly addresses the core issue of the PR.
Description check	✅ Passed	The description covers all required template sections: Overview, Details (including root cause and solution), Where to start, Related Issues, and Test Plan. It is comprehensive and well-structured.
Linked Issues check	✅ Passed	The changes implement the required fix: adding source_model field to preserve HuggingFace path, updating set_name() to save original display_name, and modifying download_config() to use source_model when available, directly addressing issue #4748 requirements.
Out of Scope Changes check	✅ Passed	All changes are confined to lib/llm/src/model_card.rs and directly relate to fixing the model path preservation issue. No out-of-scope modifications detected.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

📜 Recent 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 7053bb2 and 6606329.

📒 Files selected for processing (1)

lib/llm/src/model_card.rs

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-09-02T16:46:54.015Z

Learnt from: GuanLuo
Repo: ai-dynamo/dynamo PR: 2714
File: lib/llm/src/discovery/model_entry.rs:38-42
Timestamp: 2025-09-02T16:46:54.015Z
Learning: In lib/llm/src/discovery/model_entry.rs, GuanLuo prefers not to add serde defaults for model_type and model_input fields to keep the specification explicit and avoid user errors, relying on atomic deployment strategy to avoid backward compatibility issues.

Applied to files:

lib/llm/src/model_card.rs

🧬 Code graph analysis (1)

lib/llm/src/model_card.rs (1)

lib/llm/src/hub.rs (1)

from_hf (18-75)

🔇 Additional comments (5)

lib/llm/src/model_card.rs (5)

173-178: Well-designed field addition for backward compatibility.

The source_model field is properly documented and uses appropriate serde attributes (default, skip_serializing_if) to ensure backward compatibility with existing serialized cards. The documentation clearly explains the use case and fallback behavior.

369-376: LGTM! Correct preservation logic.

The implementation correctly preserves the original display_name in source_model before overwriting it. The conditional logic ensures that:

source_model is only set once (prevents overwriting on subsequent calls)

Empty display_name values are not preserved (defensive check)

This aligns perfectly with the use case where --served-model-name customizes the display name after initial model loading.

409-410: Correct use of source_model with proper fallback.

The download logic correctly prioritizes source_model for HuggingFace downloads while maintaining backward compatibility by falling back to display_name when source_model is not set. This ensures that:

New cards with customized names download from the original repository

Old cards without source_model continue to work as before

540-540: Appropriate initialization for local models.

Setting source_model: None for models loaded from local paths is correct, as these models don't require HuggingFace downloads. The has_local_files() check at line 394 ensures download_config() returns early for such models, so source_model won't be consulted.

173-178: Note: source_model intentionally excluded from checksum calculation.

The source_model field is not included in the mdcsum() calculation (lines 297-336), which appears intentional. Since source_model is metadata indicating where to download configuration files rather than what was downloaded, the actual model identity is determined by the checksums of the downloaded artifacts (model_info, tokenizer, etc.). This design is correct: two cards with identical model files but different source_model values should be considered equivalent.

Also applies to: 297-336

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

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

rmccorm4 · 2025-12-30T19:16:11Z

Hi @yurekami, thanks for the contribution! Can you fix the merge conflict? No need to open a new PR for it, please fix it on this existing branch/PR.

rmccorm4 · 2025-12-30T19:16:34Z

/ok to test 6606329

When --served-model-name is used to customize the display name, the frontend's download_config() method was using the display_name to download from HuggingFace, which fails because the custom name doesn't match any HuggingFace repository. This fix: 1. Adds source_model field to ModelDeploymentCard to preserve the original HuggingFace repository path 2. Updates set_name() to automatically preserve the original model path in source_model before overwriting display_name 3. Updates download_config() to use source_model when available, falling back to display_name for backward compatibility Fixes ai-dynamo#4748 Signed-off-by: yurekami <yurekami@users.noreply.github.com>

rmccorm4 · 2026-01-02T19:55:19Z

/ok to test 8ff8f78

grahamking · 2026-01-08T20:42:38Z

/ok to test 8ff8f78

grahamking · 2026-01-08T20:48:09Z

Thanks @yurekami !

…ynamo#5102) Signed-off-by: yurekami <yurekami@users.noreply.github.com> Co-authored-by: yurekami <yurekami@users.noreply.github.com>

yurekami requested a review from a team as a code owner December 27, 2025 22:09

pull-request-size Bot added the size/S label Dec 27, 2025

github-actions Bot added external-contribution Pull request is from an external contributor fix labels Dec 27, 2025

copy-pr-bot Bot had a problem deploying to GITLAB December 30, 2025 19:16 Failure

yurekami force-pushed the fix-frontend-download-v3 branch from 6606329 to 8ff8f78 Compare December 31, 2025 07:12

copy-pr-bot Bot had a problem deploying to GITLAB January 2, 2026 19:55 Failure

rmccorm4 requested review from GuanLuo and grahamking January 2, 2026 19:55

grahamking approved these changes Jan 8, 2026

View reviewed changes

grahamking merged commit f7ba417 into ai-dynamo:main Jan 8, 2026
36 of 37 checks passed

nnshah1 mentioned this pull request Apr 27, 2026

DEP (light): Worker self-hosted metadata files #8749

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: preserve original model path for frontend config downloads#5102

fix: preserve original model path for frontend config downloads#5102
grahamking merged 1 commit into
ai-dynamo:mainfrom
yurekami:fix-frontend-download-v3

yurekami commented Dec 27, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

copy-pr-bot Bot commented Dec 27, 2025

Uh oh!

github-actions Bot commented Dec 27, 2025

Uh oh!

coderabbitai Bot commented Dec 27, 2025

Uh oh!

rmccorm4 commented Dec 30, 2025

Uh oh!

rmccorm4 commented Dec 30, 2025

Uh oh!

rmccorm4 commented Jan 2, 2026

Uh oh!

grahamking commented Jan 8, 2026

Uh oh!

Uh oh!

grahamking commented Jan 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

yurekami commented Dec 27, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Details

Root Cause

Solution

Key Improvement

Where to start

Related Issues

Test Plan

Summary by CodeRabbit

Release Notes

Uh oh!

copy-pr-bot Bot commented Dec 27, 2025

Uh oh!

github-actions Bot commented Dec 27, 2025

Uh oh!

coderabbitai Bot commented Dec 27, 2025

Walkthrough

Changes

Estimated code review effort

Poem

Pre-merge checks

Uh oh!

rmccorm4 commented Dec 30, 2025

Uh oh!

rmccorm4 commented Dec 30, 2025

Uh oh!

rmccorm4 commented Jan 2, 2026

Uh oh!

grahamking commented Jan 8, 2026

Uh oh!

Uh oh!

grahamking commented Jan 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yurekami commented Dec 27, 2025 •

edited by coderabbitai Bot

Loading