chore: fix assertions for process_data post merge

[mckornfield]

Summary

Pre-Review Checklist

Ensure that the following pass:

• make format && make check or via prek validation.
• make test passes locally
• make test-e2e passes locally
• make test-ci-container passes locally (recommended)
• GPU CI status check passes -- comment /sync on this PR to trigger a run (auto-triggers on ready-for-review)

Pre-Merge Checklist

• New or updated tests for any fix or new behavior
• Updated documentation for new features and behaviors, including docstrings for API docs.

Other Notes

• Closes #

Summary by CodeRabbit

Release Notes

No user-facing changes in this release.

• Tests
  • Updated test expectations for configuration loading and runtime overrides.

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 040a7013-1d2a-49d1-8179-ca707aa08d84

📥 Commits

Reviewing files that changed from the base of the PR and between d97315b and 28f7e35.

📒 Files selected for processing (1)

• tests/sdk/test_process_data.py

📜 Recent review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)

• GitHub Check: Unit Tests (3.11)
• GitHub Check: Unit Tests (3.12)
• GitHub Check: Unit Tests (3.13)
• GitHub Check: Smoke Tests
• GitHub Check: Typecheck
• GitHub Check: Analyze (Python)
• GitHub Check: Analyze (python)

🧰 Additional context used

📓 Path-based instructions (7)

**/*.{md,markdown,py}

📄 CodeRabbit inference engine (.cursor/rules/agent-markdown-style.mdc)

**/*.{md,markdown,py}: Avoid decorative bold (**text**) in list items, body text, and docstrings; use structural cues (headers, list markers, colons, backticks) for emphasis instead
Use backticks for code identifiers, paths, and CLI commands in markdown and docstrings

Files:

• tests/sdk/test_process_data.py

tests/**

📄 CodeRabbit inference engine (.cursor/rules/repo-navigation.mdc)

Tests should mirror the src/ directory structure in tests/

Files:

• tests/sdk/test_process_data.py

tests/**/*.py

📄 CodeRabbit inference engine (.cursor/rules/repo-navigation.mdc)

Tests in tests/e2e/ should be auto-marked with e2e marker, tests in tests/smoke/ with smoke marker, others with unit marker

tests/**/*.py: Use absolute imports in tests/ (e.g., from nemo_safe_synthesizer.observability import get_logger).
Use fixture_ prefix convention for fixtures for grep-ability and to separate fixtures from test functions. Add a one-line docstring describing the fixture's purpose and data.
Use function-scoped fixtures by default. Session scope only when empirically justified by test runtime.
Use bare assert as the primary assertion style; pytest.raises() with match= for exceptions; pytest.approx() for floating-point comparisons.
Mark CUDA-dependent tests with @pytest.mark.e2e, @pytest.mark.smoke, or @pytest.mark.requires_gpu.
Mock only external boundaries, not internal implementation details.
Ensure test isolation: no shared mutable state or execution-order dependencies between tests. If something must be run first before executing a test, include it in the test or a fixture.
Use @pytest.mark.parametrize for testing multiple input combinations rather than copy-pasting similar tests.

Organize tests using pytest following the structure in tests/TESTING.md with support for unit tests, smoke tests, and end-to-end tests

Files:

• tests/sdk/test_process_data.py

---

⚙️ CodeRabbit configuration file

Review tests against tests/TESTING.md. Check marker usage, fixture naming, tmp_path usage, determinism, and GPU/vLLM process-isolation requirements. Flag slop tests that only check that code runs, assert result is not None when stronger invariants exist, over-mock internal implementation details, patch around the bug instead of reproducing it, or add broad snapshot/golden churn without a clear contract. Flag change detector tests that fail on harmless refactors, formatting, record ordering, incidental wording, or private implementation details without demonstrating a behavior regression. Prefer existing fixtures or focused new fixtures for repeated setup; keep tests DRY when reasonable without making the behavior under test opaque. print() is allowed in tests.

Files:

• tests/sdk/test_process_data.py

**/*.py

📄 CodeRabbit inference engine (STYLE_GUIDE.md)

**/*.py: Use American English spelling: 'initialize' not 'initialise', 'recognize' not 'recognise', 'color' not 'colour'.
Use observability.get_logger(__name__) for logging, never logging.getLogger() or structlog.get_logger() directly.
Use category loggers: .runtime for internals, .user for progress/results, .system for system events.
Never use print() for operational output. Use click.echo() for CLI output or sys.stdout.write() for raw output in tools.
Use extra={} in logging for structured data that downstream tools should query or aggregate; use f-strings for human-readable context.
Raise from the custom error hierarchy with dual inheritance: SafeSynthesizerError (base), UserError, DataError, ParameterError, GenerationError, InternalError.
Use NSSBaseModel for config/parameter models in config/ which define user-facing configuration. Use raw BaseModel or module-specific bases for data transfer objects and internal structures.
Use BaseSettings for env/CLI settings. Prefer AliasChoices on individual fields when a field needs to respond to both its Python name and an env var name.
Include Field(description=...) for Pydantic model fields as the canonical field docstring for API documentation and CLI help text.
Use assignment-style type = Field(default=..., description="...") as the default for Pydantic model fields because type checkers understand default, default_factory, and alias in assignment style.
Use Annotated only when the field carries additional metadata beyond Field() -- ValueValidator, AutoParam, DependsOnValidator, reusable constrained type aliases, nested-type constraints, or discriminated unions.
Put defaults as bare assignment (= value), not inside Field(default=...), when using Annotated. Exception: use assignment-style Field(default_factory=...) for defaults that cannot be expressed as bare assignments.
Use @dataclass(frozen=True) for immutable value objects and validators; mu...

Files:

• tests/sdk/test_process_data.py

**/*.{py,sh,yaml,yml}

📄 CodeRabbit inference engine (STYLE_GUIDE.md)

Include SPDX copyright header at the top: # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. and # SPDX-License-Identifier: Apache-2.0. The make format command handles this automatically.

Files:

• tests/sdk/test_process_data.py

**/*

📄 CodeRabbit inference engine (STYLE_GUIDE.md)

**/*: Include a newline at the end of all files, never trailing whitespace. This is enforced by pre-commit.
Use line length of 120 characters for code, comments, and docstrings (configured in ruff.toml).

Files:

• tests/sdk/test_process_data.py

---

⚙️ CodeRabbit configuration file

**/*: Review as a senior maintainer for NeMo Safe Synthesizer. Prioritize issues that can change behavior, break user workflows, weaken privacy guarantees, hide failures, make tests unreliable, or create maintenance risk. Avoid generic style commentary unless it points to a concrete project convention that automated tools will not catch.
Comment only when the finding is actionable and tied to changed code. For each finding, state the impact, the condition that triggers it, and the smallest practical fix. Prefer one precise comment over broad advice. Do not ask for refactors outside the PR scope unless the changed code creates the problem.
Review type guidance: - Potential issue: use for correctness bugs, data loss, privacy leaks,
security risks, broken public APIs, invalid config behavior, missing
validation, hidden failures, nondeterministic tests, or CI breakage.

• Refactor suggestion: use for local maintainability problems introduced
  by the diff when they have clear future cost, such as duplicated setup,
  unclear boundaries, over-mocking, avoidable complexity, or opaque test
  helpers.
• Nitpick: avoid in chill mode. Do not emit formatting, import-order,
  wording, or style-only comments unless automated tools cannot catch the
  issue and it affects maintainability.

Severity guidance: - Critical: security/privacy leaks, data loss, training/test/holdout
contamination, or broken release/package/core pipeline execution.

• Major: incorrect generation/training/evaluation behavior, broken
  CLI/SDK public API, invalid config defaults or validators, or GPU/vLLM
  cleanup and process-isolation bugs likely to fail CI or production
  runs.
• Minor: localized bugs, missing focused tests for changed behavior, or
  bad test patterns that weaken regression coverage.
• Trivial: small cleanup with no behavior impact. Usually suppress in
  chill mode.
• Info: context only. Avoid unless it helps reviewers understand risk.
  Safe-Synthesizer-specific review focus: - Data ...

Files:

• tests/sdk/test_process_data.py

**/*test*.py

📄 CodeRabbit inference engine (AGENTS.md)

The unit_test marker is deprecated; use unit instead

Files:

• tests/sdk/test_process_data.py

🧠 Learnings (3)

📓 Common learnings

Learnt from: CR
Repo: NVIDIA-NeMo/Safe-Synthesizer PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2026-05-20T22:25:01.000Z
Learning: Applies to .github/CODEOWNERS : Code ownership is split between NVIDIA-NeMo/safe-synthesizer-reviewers for src/ and test/ files, and NVIDIA-NeMo/safe-synthesizer-maintainers for configuration and infrastructure files

Learnt from: CR
Repo: NVIDIA-NeMo/Safe-Synthesizer PR: 0
File: tests/TESTING.md:0-0
Timestamp: 2026-05-14T21:47:20.140Z
Learning: Applies to tests/generation/conftest.py : Refer to `tests/generation/conftest.py` for JSONL/schema fixtures and `fixture_valid_iris_dataset_jsonl_and_schema` pattern.

📚 Learning: 2026-05-14T21:47:20.140Z

Learnt from: CR
Repo: NVIDIA-NeMo/Safe-Synthesizer PR: 0
File: tests/TESTING.md:0-0
Timestamp: 2026-05-14T21:47:20.140Z
Learning: Applies to tests/generation/conftest.py : Refer to `tests/generation/conftest.py` for JSONL/schema fixtures and `fixture_valid_iris_dataset_jsonl_and_schema` pattern.

Applied to files:

• tests/sdk/test_process_data.py

📚 Learning: 2026-05-27T22:20:37.354Z

Learnt from: kendrickb-nvidia
Repo: NVIDIA-NeMo/Safe-Synthesizer PR: 520
File: tests/generation/test_vllm_backend.py:556-587
Timestamp: 2026-05-27T22:20:37.354Z
Learning: In NVIDIA-NeMo/Safe-Synthesizer, `tests/conftest.py`’s `pytest_collection_modifyitems` hook applies pytest category markers automatically based on each test file’s path: tests under `/e2e/` get `pytest.mark.e2e`, tests under `/smoke/` get `pytest.mark.smoke`, and all other tests get `pytest.mark.unit`. Therefore, when reviewing pytest tests outside `tests/e2e/` and `tests/smoke/`, do not flag missing explicit `pytest.mark.unit` decorators on test classes/functions as an issue (the hook will add them during collection). If a new test directory/category is introduced, ensure the hook is updated so it’s categorized correctly.

Applied to files:

• tests/sdk/test_process_data.py

🔇 Additional comments (1)

tests/sdk/test_process_data.py (1)

460-460: LGTM!

Also applies to: 466-466, 478-478, 498-498, 510-510, 565-565, 583-583

---

Walkthrough

Test expectations in three load_from_save_path flow tests are updated to set and assert generation.structured_generation_schema_method to "auto", replacing prior expected values of "regex"/"json_schema".

Changes

Configuration schema method test updates

Layer / File(s)	Summary
Test runtime generation and evaluation config application tests/sdk/test_process_data.py	In test_load_applies_runtime_generation_and_evaluation_config, saved and runtime generation.structured_generation_schema_method config setup and the corresponding assertion are updated to use "auto".
Test preservation of saved generation without runtime overrides tests/sdk/test_process_data.py	In test_load_preserves_saved_generation_when_runtime_config_has_no_overrides, saved generation.structured_generation_schema_method config setup and the corresponding assertion are updated to use "auto".
Test full runtime config replacement tests/sdk/test_process_data.py	In test_load_full_runtime_config_replaces_supported_runtime_sections, saved generation.structured_generation_schema_method config setup and the corresponding assertion are updated to use "auto".

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested labels

test, chore

Suggested reviewers

• kendrickb-nvidia
• binaryaaron

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'chore: fix assertions for process_data post merge' directly addresses the main change: updating test assertions in process_data.py after a merge that affected structured_generation_schema_method expectations.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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 test-fixes-xgrammar-regex/mck

---

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

[greptile-apps]

Greptile Summary

This chore PR updates test assertions in tests/sdk/test_process_data.py to reflect a default-value change for structured_generation_schema_method from \"regex\" to \"auto\" that landed in a prior merge.

• All four changed fixtures now use \"auto\" for structured_generation_schema_method, which matches the current schema default, fixing CI failures.
• Three of the affected tests were specifically designed to verify config-merge behaviour (runtime override, saved-value preservation, and full-section replacement); replacing both the saved and runtime values with the same default makes those assertions pass trivially and removes their regression-detection value — \"regex\" and \"json_schema\" remain valid StructuredGenerationSchemaMethod literals and could still be used to keep the tests discriminating.

Confidence Score: 3/5

Safe to merge only if the weakened merge-behavior coverage is acceptable; production code is untouched but three test cases no longer detect regressions in the config-merge path they were written to guard.

The test assertions now use the same "auto" value for both saved and runtime configs, so they pass trivially whether the merge logic works correctly or is broken. Tests that were explicitly designed to distinguish override-wins from preserve-saved scenarios have lost that distinction, leaving the config-merge path without meaningful automated coverage for this field.

tests/sdk/test_process_data.py — specifically the three tests that changed saved and runtime values to the same default; they should use distinct enum literals (e.g. "regex" vs "json_schema") to remain effective.

Important Files Changed

Filename	Overview
tests/sdk/test_process_data.py	Updates structured_generation_schema_method from "regex"/"json_schema" to "auto" across four test fixtures; fixes broken assertions after default changed to "auto", but several tests now use identical values for both saved and runtime configs, losing their ability to detect merge regressions.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[load_from_save_path called] --> B{Runtime field explicitly set?}
    B -- Yes --> C[Use runtime value]
    B -- No --> D[Use saved config value]
    C --> E[Final merged config]
    D --> E

    subgraph Tests Updated
        T1["test_load_applies_runtime_generation_and_evaluation_config\nsaved='auto', runtime='auto'\nassert == 'auto'  ⚠️ ambiguous"]
        T2["test_load_preserves_saved_generation_when_runtime_config_has_no_overrides\nsaved='auto' (default), runtime=default\nassert == 'auto'  ⚠️ ambiguous"]
        T3["test_load_full_runtime_config_replaces_supported_runtime_sections\nsaved='auto', runtime_all_set='auto'\nassert == 'auto'  ⚠️ ambiguous"]
    end

    E -.->|tested by| T1
    E -.->|tested by| T2
    E -.->|tested by| T3

Loading

Comments Outside Diff (1)

1. tests/sdk/test_process_data.py, line 561-584 (link)

   Replacement assertion now uses the same value as saved config

   test_load_full_runtime_config_replaces_supported_runtime_sections uses model_validate(model_dump()) to mark every runtime field as explicitly set, so the entire generation section from the runtime config should replace the saved one. Before this PR the saved field was "json_schema" and the assertion checked the runtime default ("regex"), confirming the replacement. Now both saved and runtime default to "auto", so the assertion == "auto" cannot distinguish "runtime replaced saved" from "saved value happened to survive." Setting saved to a non-default value like "json_schema" while leaving the runtime config at its default "auto" would restore the test's ability to detect a broken replacement.

_{Reviews (1): Last reviewed commit: "chore: fix assertions for process_data p..." | Re-trigger Greptile}

[greptile-apps]

Test no longer verifies override behavior

test_load_applies_runtime_generation_and_evaluation_config is explicitly named and commented as testing that runtime generation overrides are honoured on resume. Setting both saved_config and runtime_config to "auto" (the field's default) means the final assertion == "auto" passes whether the merge chose the runtime value, kept the saved value, or simply applied the schema default — so a broken override path would go undetected. "regex" and "json_schema" are still valid StructuredGenerationSchemaMethod literals; using distinct values here (e.g. saved = "regex", runtime = "json_schema", assert "json_schema") is sufficient to restore the discriminating power this test was providing.

The same pattern appears in test_load_preserves_saved_generation_when_runtime_config_has_no_overrides (line ~498): storing the default "auto" as the saved value means the assertion == "auto" also passes if the merge silently falls back to the schema default instead of preserving the persisted value. A non-default saved value (e.g. "json_schema") is needed for that test to catch regressions.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!