chore: update e2e safe-synthesizer tests

[mckornfield]

Summary by CodeRabbit

• Tests
  • Reworked Safe Synthesizer end-to-end coverage to run via a local subprocess smoke harness by default (replacing container-completion assertions).
  • Added Safe Synthesizer API health/status readiness checks.
  • Added Fileset upload/download round-trip validation with temporary test datasets and automatic cleanup.
  • Added Safe Synthesizer job lifecycle coverage: create, list, retrieve, cancel, and delete, with more robust status waiting.
  • Kept GPU/Kubernetes workflow tests opt-in, including cancel transitions, PII replacement completion, and full workflow artifact downloads.

[crookedstorm]

LGTM (not that you need my approval in this folder)

[github-actions]

Suite	Lines Covered	Line Rate	Branch Rate
Unit Tests	25436/32661	77.9%	62.5%
Integration Tests	14702/31310	47.0%	19.3%

[coderabbitai]

📝 Walkthrough

Walkthrough

The Safe Synthesizer E2E module now validates REST health, fileset transfers, job lifecycle operations, cancellation, PII replacement, and full workflow artifacts. GPU/Kubernetes workflows remain opt-in, with dynamic model-fileset setup and fixture-managed cleanup.

Changes

Safe Synthesizer E2E suite

Layer / File(s)	Summary
API helpers and test configuration e2e/test_safe_synthesizer.py	Adds REST URL, header, dataset upload, job operation, result retrieval, and diagnostic status-polling helpers.
Platform fileset and dataset fixtures e2e/test_safe_synthesizer.py	Dynamically locates model-fileset setup and manages temporary dataset filesets and created jobs.
Service and job lifecycle smoke tests e2e/test_safe_synthesizer.py	Checks readiness, fileset upload/download integrity, and job create/list/retrieve/cancel/delete behavior.
Opt-in Kubernetes and workflow validation e2e/test_safe_synthesizer.py	Validates cancellation transitions, PII replacement results, synthesis artifacts, timing fields, CSV data, and evaluation reports.

Sequence Diagram(s)

sequenceDiagram
  participant SmokeTest
  participant FilesAPI
  participant SafeSynthesizerAPI
  SmokeTest->>FilesAPI: Upload generated CSV
  FilesAPI-->>SmokeTest: Return fileset data source
  SmokeTest->>SafeSynthesizerAPI: Create and inspect job
  SmokeTest->>SafeSynthesizerAPI: Cancel job
  SafeSynthesizerAPI-->>SmokeTest: Return terminal status
  SmokeTest->>SafeSynthesizerAPI: Delete job

Loading

Possibly related PRs

• NVIDIA-NeMo/nemo-platform#654: Modifies the same Safe Synthesizer E2E module and related model-fileset and job coverage.

Suggested reviewers: tylersbray

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: updating the Safe Synthesizer E2E tests.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch nss-e2e-updates/mck

---

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

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

e2e/test_safe_synthesizer.py (1)

338-355: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Add a timeout to the subprocess.run call.

No timeout= set. A hang in setup_model_filesets.py (e.g. stuck network call) can block this module-scoped fixture indefinitely; pytest's own test-level timeout is not guaranteed to reliably terminate a blocked subprocess or run fixture teardown when it fires.

🔧 Proposed fix

     result = subprocess.run(
         [
             "uv",
             "run",
             "--project",
             str(platform_root),
             "python",
             str(script),
             "--files-api-url",
             str(sdk.base_url).rstrip("/"),
             "--workspace",
             "default",
         ],
         cwd=platform_root,
+        timeout=300,
         check=False,
         capture_output=True,
         text=True,
     )

🤖 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 `@e2e/test_safe_synthesizer.py` around lines 338 - 355, Add a finite timeout to
the subprocess.run invocation in the module-scoped fixture using the existing
setup_model_filesets flow, and handle subprocess.TimeoutExpired consistently so
the fixture fails promptly with useful captured-output context instead of
hanging indefinitely.

🤖 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 `@e2e/test_safe_synthesizer.py`:
- Around line 204-213: Update _cancel_nss_job to treat HTTP 409 responses as a
successful best-effort cleanup outcome, alongside the existing 404 handling, so
cleanup does not assert or mask the original test failure. Preserve the current
200 response parsing and assertion behavior for other unexpected status codes.

---

Outside diff comments:
In `@e2e/test_safe_synthesizer.py`:
- Around line 338-355: Add a finite timeout to the subprocess.run invocation in
the module-scoped fixture using the existing setup_model_filesets flow, and
handle subprocess.TimeoutExpired consistently so the fixture fails promptly with
useful captured-output context instead of hanging indefinitely.

🪄 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: 64eb6309-b74b-4440-8249-df3c2a517cbc

📥 Commits

Reviewing files that changed from the base of the PR and between 3576fe7 and 63957e7.

📒 Files selected for processing (1)

• e2e/test_safe_synthesizer.py

[tylersbray]

Adding one more non-blocking approval modulo those hopefully easy feedback items, thanks.

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@e2e/test_safe_synthesizer.py`:
- Around line 389-393: Update the model fileset setup in the relevant test
fixture or helper to register models in the configured workspace from the
workspace fixture instead of hardcoding `"default"` in the `--workspace`
argument. Keep the existing workspace behavior for default runs while ensuring
jobs using non-default workspace values resolve their model filesets.

🪄 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: 14e0c970-8c85-4752-be7e-660d32270b88

📥 Commits

Reviewing files that changed from the base of the PR and between 203e20b and 2894ca1.

📒 Files selected for processing (1)

• e2e/test_safe_synthesizer.py