fix(retrieval-sdg): support clean provider initialization - #59
Merged
Conversation
shan-nvidia
marked this pull request as ready for review
August 12, 2026 15:25
oliverholworthy
approved these changes
Aug 12, 2026
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
model_providers.yamlhas not been initialized yet.validate-releaseselect the workspaceddppackage explicitly so the guarded release target works from a fresh worktree.Context
The first
data-designer-retrieval-sdg/v0.2.0publication attempt failed in the isolated package-test step before any distribution or package-index asset was uploaded.What failed
Data Designer has two representations of its default providers:
~/.data-designer/model_providers.yamlfile containing those defaults plus any user edits.The
DataDesignerconstructor initializes the user-level file from the built-in definitions when the file does not exist. However, the retrieval SDG plugin resolves model providers before it constructsDataDesigner.The normal and custom-provider paths therefore behaved differently:
build_model_providers()returnsNonewithout reading the user-level file. The plugin then constructsDataDesigner(model_providers=None); its constructor createsmodel_providers.yamland safely loads the defaults.DataDesigner. It calledget_default_providers()during this earlier step. That function only readsmodel_providers.yaml; it does not initialize the file. On a new machine or clean CI runner, the file did not exist yet, so provider resolution raisedFileNotFoundErrorbefore the constructor that would have created it could run.In sequence, the failing path was:
The corrected path uses Data Designer's built-in provider definitions when the user-level file is absent. These are the same definitions that the
DataDesignerconstructor would write to the file. OnceDataDesigneris constructed, its normal initialization still creates the user-level file. Existing user files continue to take precedence, so this fallback changes only first-run behavior and does not overwrite user configuration.Why existing tests did not catch it
The bug was latent in
v0.1.0, but that release had no CLI or pipeline tests exercising custom-provider resolution. Local development also usually hadmodel_providers.yamlfrom earlier Data Designer runs.Regular repository CI masked the problem through test ordering: another plugin's tests constructed
DataDesignerfirst, which initialized the file under the runner's shared home directory. Retrieval SDG's later tests then found the file even though they ran in a separate virtual environment. Virtual-environment isolation does not isolate files under$HOME.The publication workflow tested retrieval SDG by itself on a fresh runner. Nothing had constructed
DataDesignerbeforehand, so its clean home directory exposed the ordering dependency. The new regression test explicitly simulates a missing user-level provider file and verifies fallback to the built-in definitions.The failed GitHub release and tag were removed after confirming that:
ddp-package-assetsstill contains retrieval SDG0.1.0only; andpackages.jsonwas not updated.The package version remains
0.2.0. After this PR merges, the release tag will be recreated at the corrected main commit.Validation
137 passed6 passedmake validate-release PLUGIN=data-designer-retrieval-sdgFailed publication run: https://github.com/NVIDIA-NeMo/DataDesignerPlugins/actions/runs/31535353100