fix: move nss jobs into plugin, update docs - #195
Merged
Conversation
Signed-off-by: mkornfield <mkornfield@nvidia.com>
Contributor
|
Contributor
Documentation preview is readyPreview: https://nvidia-nemo.github.io/nemo-platform/pr-preview/pr-195/pr-195/ Built from This preview is deployed from this PR branch, updates when docs changes are pushed, and will be removed when the PR closes. |
Contributor
|
Lost in the diff? Review this PR in Change Stack to follow the change map from intent to exact ranges. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Safe Synthesizer plugin SDK (resources, job builder, job wrapper), job-config validators for pretrained-adapter reuse, tests, packaging/entry-point and manifest updates, and documentation clarifying local execution and Jobs API/SDK workflows. ChangesSafe Synthesizer SDK rollout
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/safe-synthesizer/about/host-local-development.md`:
- Line 4: Add a blank line before the heading text "# Local and Subprocess
Execution" in the file to satisfy MD022; locate the heading string and insert
one empty line immediately above it so the heading is separated from prior
content.
In `@docs/safe-synthesizer/tutorials/safe-synthesizer-101.md`:
- Around line 77-79: The error message hardcodes product and plugin names;
replace the literal strings "Safe Synthesizer" and "safe-synthesizer" in the
quoted message with the appropriate Sphinx substitutions (e.g. |product| or the
project-specific substitution such as |safe_synthesizer_plugin|) so docs use
configured substitutions consistently; update the three concatenated string
pieces to use the substitution tokens (keeping the same wording and path
structure but substituting the product/plugin tokens) so Sphinx renders the
correct product name across builds.
- Around line 123-124: The current code assumes provider_name contains "/",
causing split("/", 1) to crash for unqualified names; update the logic around
provider_name, provider_workspace, and provider_id so both "provider" and
"workspace/provider" work: if "/" in provider_name then split into
provider_workspace and provider_id, otherwise set provider_id = provider_name
and provider_workspace = None (or an empty value), and call
client.inference.providers.retrieve(provider_id, workspace=provider_workspace)
only when provider_workspace is present (or pass None/omit the workspace
argument as appropriate).
In `@mkdocs.yml`:
- Around line 222-229: Remove the two hidden_docs.paths entries that reference
host-local-development.md and safe-synthesizer-101.md so those files are no
longer included in hidden_docs.paths; specifically edit the mkdocs.yml
hidden_docs.paths list to delete the items
"safe-synthesizer/tutorials/host-local-development.md" and
"safe-synthesizer/tutorials/safe-synthesizer-101.md" (or the exact matching path
strings present) so the hide_unready_docs.py hook no longer filters them out and
the nav entries for host-local-development.md and safe-synthesizer-101.md remain
usable.
In
`@plugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/sdk/job_builder.py`:
- Around line 159-163: The path branch currently forces all file-path sources
through pd.read_csv which will break parquet/json/jsonl inputs; update the
loader in the method using self._data_source (in job_builder.py / JobBuilder or
the method containing that if-block) to detect the file type via
Path(self._data_source).suffix.lower() and call the appropriate pandas reader:
pd.read_parquet for .parquet, pd.read_json(..., lines=True) for .jsonl,
pd.read_json for .json, and pd.read_csv for .csv (fall back to pd.read_csv for
unknown extensions or raise a clear error). Apply the same change to the other
identical branch around the later occurrence (the block at the 235-240 region).
In `@plugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/sdk/job.py`:
- Around line 98-103: The status-change prints execute unconditionally (using
current_status_info and previous_status_info), ignoring the verbose flag; wrap
the print block so it only executes when the function/class-level verbose is
True (e.g., if verbose: or if self.verbose:) and ensure the surrounding function
(where current_status_info/previous_status_info are compared) accepts or
references that verbose flag so behavior is unchanged when verbose is False.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a2f01659-19de-4e38-b14a-6be48c8f8b2b
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (28)
docs/_snippets/nvidia-build-model-provider.mddocs/safe-synthesizer/.gitignoredocs/safe-synthesizer/about/host-local-development.mddocs/safe-synthesizer/about/jobs.mddocs/safe-synthesizer/about/reference.mddocs/safe-synthesizer/getting-started.mddocs/safe-synthesizer/llms.txtdocs/safe-synthesizer/tutorials/differential-privacy.mddocs/safe-synthesizer/tutorials/safe-synthesizer-101.mdmkdocs.ymlpackages/nemo_platform/pyproject.tomlplugins/nemo-safe-synthesizer/README.mdplugins/nemo-safe-synthesizer/pyproject.tomlplugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/api/v2/jobs/endpoints.pyplugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/sdk/config.pyplugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/sdk/http_utils.pyplugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/sdk/job.pyplugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/sdk/job_builder.pyplugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/sdk/resources.pyplugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/skills/safe-synthesizer/workflows/diagnose.mdplugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/skills/safe-synthesizer/workflows/results.mdplugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/skills/safe-synthesizer/workflows/run.mdplugins/nemo-safe-synthesizer/tests/unit/test_jobs.pyplugins/nemo-safe-synthesizer/tests/unit/test_sdk.pypyproject.tomlthird_party/licenses.jsonlthird_party/osv-licenses.jsonthird_party/requirements-main.txt
Signed-off-by: mkornfield <mkornfield@nvidia.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
mckornfield
commented
Jun 4, 2026
anastasia-nesterenko
approved these changes
Jun 5, 2026
anastasia-nesterenko
left a comment
anastasia-nesterenko
left a comment
Contributor
There was a problem hiding this comment.
left some nit-picks
tylersbray
reviewed
Jun 5, 2026
tylersbray
left a comment
tylersbray
left a comment
Contributor
There was a problem hiding this comment.
agent (
pr-review): Full review summary
Solid PR overall — plugin SDK relocation, _runtime_job_config, and scheduler workspace fix are the right moves. Inline comments below are ordered roughly by severity.
Merge blockers / high priority
safe-synthesizer-101.md— classify provider configured but never submitted (missing.with_replace_pii())nvidia-build-model-provider.md—default/vssystem/workspace mismatch with platform-seed
Should fix in this PR
3. Stale "Host-Local Development and Testing" link titles in jobs.md, reference.md, about/index.md
4. Add docs/safe-synthesizer/sdk-resources.md (parity with anonymizer/evaluator/data-designer)
5. AsyncSafeSynthesizerJobsResource.get_logs should be async def
6. Expand tests for SafeSynthesizerJob and _runtime_job_config edge cases
Confirm intent / lower priority
7. mkdocs.yml nav pruning — jobs/reference/DP tutorial removed from published nav
8. job_builder.py — with_classify_model_provider() only applies when PII replacement enabled
9. job.py — broad except Exception in wait_for_completion
10. config.py — silent ImportError leaves empty __all__
11. packages/nemo_platform/BUNDLING.md still references removed safe_synthesizer_sdk vendoring (file not in this PR)
Looks good
enabled-pluginsaddition, SDK entry points,test_sdk.pyresource/builder coverage, scheduler workspace test
tylersbray
approved these changes
Jun 5, 2026
tylersbray
left a comment
tylersbray
left a comment
Contributor
There was a problem hiding this comment.
Left a few comments...
Signed-off-by: mkornfield <mkornfield@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/safe-synthesizer/sdk-resources.md (1)
10-57: ⚖️ Poor tradeoffAdd CLI examples alongside Python SDK examples.
Coding guidelines require both Python SDK and CLI examples in tab-sets. Document CLI equivalents for job creation and monitoring where available.
As per coding guidelines, provide both Python SDK and CLI examples in tab-sets for consistency and to support multiple user workflows.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/safe-synthesizer/sdk-resources.md` around lines 10 - 57, Add CLI examples next to the Python SDK examples using tab-sets: keep the existing Python block (NeMoPlatform and SafeSynthesizerJobBuilder usage) and add a parallel CLI tab that shows equivalent commands for client.safe_synthesizer operations (create/list/retrieve/get_status/get_logs) and for the SafeSynthesizerJobBuilder flow (upload/data-source, synthesize, create-job), naming commands and flags that map to the SDK method signatures (create with --spec/--name/--workspace/--project/--timeout, list with --workspace, retrieve/get_status/get_logs with <name> and --workspace, plus CLI flags for classify model provider and replace-pii used in the builder). Ensure the docs mention the async namespace only for SDK and keep CLI examples in their own tab, matching the same examples/parameters and brief usage notes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/safe-synthesizer/about/index.md`:
- Line 109: The Markdown list item "- **[Local and Subprocess
Execution](host-local-development.md)**" has three spaces after the list marker;
change it so there is exactly one space after the dash (i.e., "- **[Local and
Subprocess Execution](host-local-development.md)**") to conform to Markdown list
spacing conventions.
---
Nitpick comments:
In `@docs/safe-synthesizer/sdk-resources.md`:
- Around line 10-57: Add CLI examples next to the Python SDK examples using
tab-sets: keep the existing Python block (NeMoPlatform and
SafeSynthesizerJobBuilder usage) and add a parallel CLI tab that shows
equivalent commands for client.safe_synthesizer operations
(create/list/retrieve/get_status/get_logs) and for the SafeSynthesizerJobBuilder
flow (upload/data-source, synthesize, create-job), naming commands and flags
that map to the SDK method signatures (create with
--spec/--name/--workspace/--project/--timeout, list with --workspace,
retrieve/get_status/get_logs with <name> and --workspace, plus CLI flags for
classify model provider and replace-pii used in the builder). Ensure the docs
mention the async namespace only for SDK and keep CLI examples in their own tab,
matching the same examples/parameters and brief usage notes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 8218c7dd-ef80-45a0-b5bd-e48ef1360886
📒 Files selected for processing (18)
docs/_snippets/nvidia-build-model-provider.mddocs/safe-synthesizer/about/index.mddocs/safe-synthesizer/about/jobs.mddocs/safe-synthesizer/about/reference.mddocs/safe-synthesizer/getting-started.mddocs/safe-synthesizer/sdk-resources.mddocs/safe-synthesizer/tutorials/safe-synthesizer-101.mdmkdocs.ymlpackages/nemo_platform/BUNDLING.mdplugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/api/v2/jobs/endpoints.pyplugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/job_config.pyplugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/sdk/config.pyplugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/sdk/job.pyplugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/sdk/job_builder.pyplugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/sdk/resources.pyplugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/tasks/safe_synthesizer/__main__.pyplugins/nemo-safe-synthesizer/tests/unit/test_jobs.pyplugins/nemo-safe-synthesizer/tests/unit/test_sdk.py
✅ Files skipped from review due to trivial changes (6)
- docs/safe-synthesizer/about/jobs.md
- packages/nemo_platform/BUNDLING.md
- plugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/tasks/safe_synthesizer/main.py
- docs/_snippets/nvidia-build-model-provider.md
- docs/safe-synthesizer/getting-started.md
- docs/safe-synthesizer/tutorials/safe-synthesizer-101.md
🚧 Files skipped from review as they are similar to previous changes (5)
- docs/safe-synthesizer/about/reference.md
- plugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/api/v2/jobs/endpoints.py
- mkdocs.yml
- plugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/sdk/resources.py
- plugins/nemo-safe-synthesizer/src/nemo_safe_synthesizer_plugin/sdk/job.py
Signed-off-by: mkornfield <mkornfield@nvidia.com>
Signed-off-by: mkornfield <mkornfield@nvidia.com>
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
4 participants
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 by CodeRabbit