fix: Add --org option to NGC download command

[mikeknep]

📋 Summary

Adds --org nvidia to the NGC CLI download command for Nemotron personas datasets. Without this, the existing command only works if you have a ~/.ngc/config file with a default org set. Some users might not "configure" their NGC CLI like this.

🔗 Related Issue

N/A, too small to bother

🔄 Changes

• Adds the arg to the download service

🧪 Testing

• make test passes
• Unit tests added/updated
• E2E tests added/updated (if applicable) (N/A)

Also manually tested data-designer download personas --locale ja_JP both with and without a ~/.ngc/config file present; works in both scenarios.

✅ Checklist

• Follows commit message conventions
• Commits are signed off (DCO)
• Architecture docs updated (if applicable) (N/A)

[greptile-apps]

Greptile Summary

Adds --org nvidia to the NGC CLI download-version command so that downloading Nemotron personas datasets works for users who have not set a default org in ~/.ngc/config. The change is minimal, the existing test is updated to assert the new flag, and manual testing was performed.

Confidence Score: 5/5

Safe to merge — the change is a single-flag addition that fixes a real usability gap and is covered by updated tests.

No logic errors or regressions introduced. The new --org nvidia flag is consistent with the resource path (nvidia/nemotron-personas/...) already present, the relevant success test is updated, and all other test cases still pass through the same code path.

No files require special attention.

Important Files Changed

Filename	Overview
packages/data-designer/src/data_designer/cli/services/download_service.py	Adds --org nvidia flag to the NGC CLI download-version command so it works without a pre-configured ~/.ngc/config file.
packages/data-designer/tests/cli/services/test_download_service.py	Updates test_download_persona_dataset_success to assert --org nvidia is present in the expected subprocess command; all other tests remain unaffected.

Sequence Diagram

sequenceDiagram
    participant CLI as CLI (download command)
    participant DS as DownloadService
    participant NGC as ngc CLI process
    participant FS as Filesystem

    CLI->>DS: download_persona_dataset(locale)
    DS->>DS: validate locale via PersonaRepository
    DS->>DS: create managed_assets_dir
    DS->>NGC: ngc registry resource download-version nvidia/nemotron-personas/{dataset} --org nvidia --dest {temp_dir}
    NGC-->>DS: parquet files in temp_dir
    DS->>DS: glob temp_dir for *.parquet
    DS->>FS: shutil.move each parquet → managed_assets_dir
    DS-->>CLI: return managed_assets_dir path

Loading

_{Reviews (1): Last reviewed commit: "Add --org option to NGC download command" | Re-trigger Greptile}

[github-actions]

PR #604 Review: Add --org option to NGC download command

Summary

Tiny, focused fix. The NGC CLI invocation in DownloadService.download_persona_dataset previously relied on a user having a default org configured in ~/.ngc/config. This PR hardcodes --org nvidia into the command so data-designer download personas works regardless of local NGC CLI config state. The companion test assertion is updated to match.

• packages/data-designer/src/data_designer/cli/services/download_service.py: +2 lines ("--org", "nvidia" inserted into the argv before --dest).
• packages/data-designer/tests/cli/services/test_download_service.py: +2 lines mirroring the expected argv.

Findings

Correctness

• The resource path is already nvidia/nemotron-personas/..., so hardcoding --org nvidia is consistent with what's being downloaded. No user input is passed to the new flag, so no injection surface is introduced.
• Argv order (--org nvidia before --dest temp_dir) is fine — NGC CLI accepts these flags in any position.
• Manual verification by the author covers both config-present and config-absent cases, which is the regression scenario the fix targets.

Style / conventions

• Matches surrounding code: absolute imports, from __future__ import annotations, SPDX header all already present. The change is purely data (two list entries).
• Test mirrors production argv exactly, which is the existing convention in test_download_persona_dataset_success.

Suggestions (minor, non-blocking)

• Magic string. "nvidia" now appears twice in the argv (as part of the resource path and as the --org value). Pulling the org into a module-level constant (e.g., _NGC_ORG = "nvidia") and reusing it in the f-string would make the coupling explicit. Not worth blocking on for a 2-line fix.
• Test coverage gap the PR doesn't address. The existing test only verifies the happy path's argv. There's no test exercising the original bug — i.e., behavior when ~/.ngc/config is absent. Since the CLI call is already mocked, a regression test would really only re-assert the argv, which the current test does. Fine to leave.
• Docs. If any setup/troubleshooting doc tells users to configure ngc config set, it could be updated to note this is no longer required for persona downloads. Optional.

Risks

• If NGC ever splits these datasets across a different org, the hardcoded value will need to change in two places (resource path + --org). Low-likelihood, easy to spot.
• No security, performance, or import-time impact.

Verdict

Approve. Small, well-scoped bug fix with matching test update. Aligns with the "declarative config, imperative engine" boundary (this is CLI plumbing, not engine behavior) and doesn't violate any of the layering or import invariants in AGENTS.md. Optional cleanup (constant for "nvidia") can be deferred.