fix: address third-party deprecation warnings - #704
Conversation
|
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:
WalkthroughThe pull request renames the training warmup setting with backward compatibility, updates GLiNER API handling, adjusts CUDA and package setup, and suppresses expected warnings in the tutorial notebook. ChangesCompatibility and runtime updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThe PR updates third-party integrations and configuration naming to remove expected deprecation warnings while preserving legacy warmup configuration compatibility.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains. No blocking failure remains. Important Files Changed
Reviews (11): Last reviewed commit: "fix(brev): align setup with launchable r..." | Re-trigger Greptile |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 26bfb997-28a4-4382-b896-0f29d0f8d29b
📒 Files selected for processing (8)
docs/tutorials/safe-synthesizer-101.ipynbscript/brev/setup.shsrc/nemo_safe_synthesizer/config/training.pysrc/nemo_safe_synthesizer/pii_replacer/data_editor/detect.pysrc/nemo_safe_synthesizer/training/huggingface_backend.pytests/conftest.pytests/pii_replacer/test_detect.pytests/training/test_huggingface_backend.py
📜 Review details
⏰ Context from checks skipped due to timeout. (9)
- GitHub Check: Unit Tests (3.14)
- GitHub Check: Unit Tests (3.12)
- GitHub Check: Smoke Tests
- GitHub Check: Unit Tests (3.11)
- GitHub Check: Unit Tests (3.13)
- GitHub Check: End-user Wheel Install
- GitHub Check: Greptile Review
- GitHub Check: Analyze (Python)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (20)
**/*.{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/training/test_huggingface_backend.pysrc/nemo_safe_synthesizer/training/huggingface_backend.pytests/conftest.pytests/pii_replacer/test_detect.pysrc/nemo_safe_synthesizer/config/training.pysrc/nemo_safe_synthesizer/pii_replacer/data_editor/detect.py
tests/**
📄 CodeRabbit inference engine (.cursor/rules/repo-navigation.mdc)
tests/**: Mirrorsrc/directory structure intests/directory for test organization
Auto-mark tests by directory:tests/e2e/→e2e,tests/smoke/→smoke, otherwise default tounit
Files:
tests/training/test_huggingface_backend.pytests/conftest.pytests/pii_replacer/test_detect.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Useuvfor Python dependency management and execution; never usepipor rawpython. Target Python 3.11–3.14 and use modern syntax such asX | Y,list[str], andSelf.
Run project tooling throughmisetasks or wrapper scripts intools/; do not invokeruffortydirectly. Useuv runfor Python execution.
Put durable implementation guidance in public function and class docstrings; put local invariants in source comments.
Use Python 3.11–3.14-compatible modern type syntax, includingX | Y,list[str], andSelf.
**/*.py: Use American English spelling in Python code, comments, and documentation; usefrom __future__ import annotationsin every module.
UseBaseSettingsfor environment/CLI settings; preferAliasChoicesfor fields accepting both Python and environment-variable names.
Pydantic model fields must includeField(description=...); prefer assignment-styleField()and useAnnotatedonly for additional metadata or constraints.
Use@dataclass(frozen=True)for immutable value objects and validators, andfield(default_factory=...)for mutable defaults; never use mutable default values directly.
UseStrEnumfor string-valued configuration or serialization enums and plainEnumfor internal-only constants.
Obtain loggers withobservability.get_logger(__name__); do not calllogging.getLogger()orstructlog.get_logger()directly.
Do not useprint()for operational library output; use the approved logger,click.echo()for CLI output, orsys.stdout.write()for raw tool output.
Use loggerextra={}for metrics, counts, durations, and other data intended for machine querying; use f-strings for human-readable context.
Raise known errors through the Safe Synthesizer custom hierarchy, using dual inheritance where callers should also catch a built-in exception.
UseX | Y, built-in generic types,Selffor fluent returns, collection ABCs for arguments,Protocolfor structural boundaries, and avoidAny.
Pre...
Files:
tests/training/test_huggingface_backend.pysrc/nemo_safe_synthesizer/training/huggingface_backend.pytests/conftest.pytests/pii_replacer/test_detect.pysrc/nemo_safe_synthesizer/config/training.pysrc/nemo_safe_synthesizer/pii_replacer/data_editor/detect.py
**/tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use the
unitpytest marker instead of the deprecatedunit_testmarker; async tests do not need@pytest.mark.asynciobecauseasyncio_mode = auto.
Files:
tests/training/test_huggingface_backend.pytests/conftest.pytests/pii_replacer/test_detect.py
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: ReadAGENTS.local.mdif it exists and give its instructions top priority.
When a task matches a repository-specific skill, read the corresponding skill under.agents/skills/instead of duplicating its workflow instructions.
Do not commit unless the user explicitly asks for a commit or PR work.
When committing, require DCO sign-off and GPG signing usinggit commit --signoff --gpg-sign(or-s -S); never manually addSigned-off-byor use--no-gpg-sign.
Use feature branches based onmain; branch names commonly include an issue-number prefix such as<author>/123-short-name.
For recurring testing, building, syncing, bootstrapping, worktree, and GitHub workflows, use the matching skill under.agents/skills/.
For a full GPU/development environment, useuv sync --frozen --extra cu129 --extra engine --group dev; bareuv sync --frozenis incomplete and can causety, import-check, and GPU-test failures.End files with a newline, contain no trailing whitespace, use one space between sentences, and keep code, comments, and docstrings within 120 characters.
Files:
tests/training/test_huggingface_backend.pysrc/nemo_safe_synthesizer/training/huggingface_backend.pytests/conftest.pyscript/brev/setup.shtests/pii_replacer/test_detect.pysrc/nemo_safe_synthesizer/config/training.pysrc/nemo_safe_synthesizer/pii_replacer/data_editor/detect.pydocs/tutorials/safe-synthesizer-101.ipynb
⚙️ 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/training/test_huggingface_backend.pysrc/nemo_safe_synthesizer/training/huggingface_backend.pytests/conftest.pyscript/brev/setup.shtests/pii_replacer/test_detect.pysrc/nemo_safe_synthesizer/config/training.pysrc/nemo_safe_synthesizer/pii_replacer/data_editor/detect.pydocs/tutorials/safe-synthesizer-101.ipynb
**/*.{md,py,sh,toml,yml,yaml,Dockerfile}
📄 CodeRabbit inference engine (AGENTS.md)
Follow the detailed language and file-format conventions defined in
STYLE_GUIDE.md.
Files:
tests/training/test_huggingface_backend.pysrc/nemo_safe_synthesizer/training/huggingface_backend.pytests/conftest.pyscript/brev/setup.shtests/pii_replacer/test_detect.pysrc/nemo_safe_synthesizer/config/training.pysrc/nemo_safe_synthesizer/pii_replacer/data_editor/detect.py
tests/**/*.py
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
tests/**/*.py: Use absolute imports in tests.
Name test filestest_*.py, classesTest*, and functionstest_<module>_<expected_behavior>; prefix fixtures withfixture_and give each fixture a one-line purpose docstring.
Use function-scoped fixtures by default, bareassert,pytest.raises(match=...),pytest.approx(), andpytest.mark.parametrizefor input combinations.
Usetmp_pathfor file operations, mock only external boundaries, avoid shared mutable state and order dependencies, and mark CUDA-dependent tests appropriately.All existing tests must pass before submitting a pull request; new features must include tests and bug fixes must include regression tests.
tests/**/*.py: Every test must have exactly one category marker:unit,smoke, ore2e; use modifier markers such asslowandrequires_gpuonly in addition to a category marker.
Usepytest.importorskipfor optional dependencies that require specific extras, such assentence_transformersandvllm.
Usefixture_mock_processororfixture_mock_processor_without_valid_recordsforParsedResponsetest data, withvalid_records,invalid_records,errors, and integerprompt_numberfields.
Use the sharedload_test_dataset(filename)andload_test_dataframe(filename)helpers for loading test datasets where applicable.
Tests should mirror the source structure, such astests/training/,tests/generation/, and corresponding source modules.
print()is permitted in tests for debug output; Ruff ruleT201is suppressed for thetests/directory.
Do not import helpers directly from another file undertests/; for shared methods, use a relative import fromconftest.py(pytest fixtures are available automatically).
When adding a vLLM GPU smoke-test file, addpytest.mark.vllm, create a dedicatedtest:smoke:gpu:*mise task for per-file process isolation, and include that task intest:smoke:gpu.
GPU tests must use therequires_gpumarker; vLLM tests additionally r...
Files:
tests/training/test_huggingface_backend.pytests/conftest.pytests/pii_replacer/test_detect.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/training/test_huggingface_backend.pytests/conftest.pytests/pii_replacer/test_detect.py
**/*.{py,sh,yaml,yml,md}
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
Include the required SPDX copyright and Apache-2.0 license headers, using comment syntax appropriate to the file format.
Include SPDX copyright headers in all source files, except files explicitly listed in
.copyrightignore.
Files:
tests/training/test_huggingface_backend.pysrc/nemo_safe_synthesizer/training/huggingface_backend.pytests/conftest.pyscript/brev/setup.shtests/pii_replacer/test_detect.pysrc/nemo_safe_synthesizer/config/training.pysrc/nemo_safe_synthesizer/pii_replacer/data_editor/detect.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{py,pyi}: Keep shared Python package code compatible with Python 3.11; do not use Python 3.12-only syntax such as PEP 695 type statements or bracketed generic parameters.
Use the repository's pinned Ruff tasks for Python formatting, import sorting, and linting rather than invoking unpinned tools directly.
Run the repository's pinnedtytype checker and maintain type-correct Python code.
Files:
tests/training/test_huggingface_backend.pysrc/nemo_safe_synthesizer/training/huggingface_backend.pytests/conftest.pytests/pii_replacer/test_detect.pysrc/nemo_safe_synthesizer/config/training.pysrc/nemo_safe_synthesizer/pii_replacer/data_editor/detect.py
**/*.{py,sh,yaml,yml,toml,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use
misetasks with the repository's pinned tool versions for formatting, checking, and testing before submitting changes.
Files:
tests/training/test_huggingface_backend.pysrc/nemo_safe_synthesizer/training/huggingface_backend.pytests/conftest.pyscript/brev/setup.shtests/pii_replacer/test_detect.pysrc/nemo_safe_synthesizer/config/training.pysrc/nemo_safe_synthesizer/pii_replacer/data_editor/detect.py
src/**/*.py
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
src/**/*.py: Keep shared package code compatible with Python 3.11; do not use Python 3.12-only syntax such as PEP 695 type statements or bracketed generic parameters.
UseTYPE_CHECKINGguards for heavy imports such aspandas,torch, andtransformers; use relative imports insrc/.
Do not add__init__.pyto test directories; every directory undersrc/containing Python files must have one.
Comments should explain why rather than narrate what; do not add redundant docstrings, defensiveexcept Exceptionaround trusted internal calls, unjustified type ignores, or casts/Anyto hide type errors.
Usepathlib.Pathinstead ofos.path, and use explicitif/raisevalidation instead ofassertin library code.
Files:
src/nemo_safe_synthesizer/training/huggingface_backend.pysrc/nemo_safe_synthesizer/config/training.pysrc/nemo_safe_synthesizer/pii_replacer/data_editor/detect.py
⚙️ CodeRabbit configuration file
Review library code against STYLE_GUIDE.md. Focus on behavior, API contracts, error handling, resource cleanup, typing, logging, and user-facing failures. Public APIs and nontrivial functions need Google-style docstrings.
Files:
src/nemo_safe_synthesizer/training/huggingface_backend.pysrc/nemo_safe_synthesizer/config/training.pysrc/nemo_safe_synthesizer/pii_replacer/data_editor/detect.py
src/nemo_safe_synthesizer/training/**/*.py
⚙️ CodeRabbit configuration file
Review training changes for dataset preprocessing, model path handling, artifact writes, LoRA/DP behavior, GPU memory usage, reproducibility, and cleanup on failure.
Files:
src/nemo_safe_synthesizer/training/huggingface_backend.py
tests/conftest.py
📄 CodeRabbit inference engine (.cursor/rules/repo-navigation.mdc)
Store shared test fixtures in
tests/conftest.py
Files:
tests/conftest.py
tests/**/*conftest.py
📄 CodeRabbit inference engine (tests/TESTING.md)
Use the
fixture_prefix for dataset and tokenizer fixtures; CLI helpers may use descriptive names such asmock_workdir.
Files:
tests/conftest.py
**/*.sh
📄 CodeRabbit inference engine (AGENTS.md)
Never use
~inside double-quoted shell strings because it does not expand; use$HOMEor an absolute path instead.Use
#!/usr/bin/env bash,set -euo pipefailunless incompatible, quote variables, use${VAR:-default}, use snake_case functions with_for private helpers, and lint with ShellCheck.
Files:
script/brev/setup.sh
⚙️ CodeRabbit configuration file
Review shell scripts for #!/usr/bin/env bash, set -euo pipefail where appropriate, quoting, repo root detection, and shellcheck compliance.
Files:
script/brev/setup.sh
script/**
⚙️ CodeRabbit configuration file
Review standalone scripts for reproducibility and operational safety. Check argument validation, quoting, repo-root detection, environment variables, generated artifacts, external commands, GPU/cluster assumptions, and whether the script should be wired through Makefile or documented in README/docs.
Files:
script/brev/setup.sh
src/**/config/**/*.py
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
Use
NSSBaseModelfor user-facing configuration and parameter models inconfig/; useBaseModelor module-specific bases for DTOs and internal structures.
Files:
src/nemo_safe_synthesizer/config/training.py
src/nemo_safe_synthesizer/config/**/*.py
⚙️ CodeRabbit configuration file
Treat config changes as user-facing API changes. Check Pydantic field descriptions, defaults, validators, aliases, override behavior, CLI help text impact, YAML compatibility, and documented parameter semantics.
Files:
src/nemo_safe_synthesizer/config/training.py
src/nemo_safe_synthesizer/pii_replacer/**/*.py
⚙️ CodeRabbit configuration file
Treat PII replacement changes as high-risk. Check entity coverage, replacement determinism, leakage of original values, handling of empty or multilingual text, and compatibility with optional dependencies.
Files:
src/nemo_safe_synthesizer/pii_replacer/data_editor/detect.py
docs/**
⚙️ CodeRabbit configuration file
Review documentation as MkDocs Material content. Check Diataxis fit, accurate commands, internal links, code fences, and markdown style from STYLE_GUIDE.md.
Files:
docs/tutorials/safe-synthesizer-101.ipynb
🧠 Learnings (4)
📚 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/training/test_huggingface_backend.pytests/conftest.pytests/pii_replacer/test_detect.py
📚 Learning: 2026-07-27T22:07:22.590Z
Learnt from: binaryaaron
Repo: NVIDIA-NeMo/Safe-Synthesizer PR: 673
File: tests/pii_replacer/test_edit.py:327-327
Timestamp: 2026-07-27T22:07:22.590Z
Learning: When tests read structured logging context from Python `logging.LogRecord` instances, don’t access `record.ctx` directly (it isn’t declared on `LogRecord` and will break static typing). Instead, use `getattr(record, "ctx", default)` (or an appropriate fallback) to safely handle cases where `ctx` may or may not be attached. This applies even if Ruff rule `B009` isn’t enabled in the repo.
Applied to files:
tests/training/test_huggingface_backend.pytests/conftest.pytests/pii_replacer/test_detect.py
📚 Learning: 2026-07-29T17:12:32.642Z
Learnt from: zywind
Repo: NVIDIA-NeMo/Safe-Synthesizer PR: 654
File: tests/config/test_parameters.py:116-118
Timestamp: 2026-07-29T17:12:32.642Z
Learning: In Pydantic validation tests (e.g., models configured with `from_attributes`), when asserting failures from `model_validate(...)`, assert the structured error details (such as `ValidationError.errors()[0]["type"]`, e.g. `"model_attributes_type"`) rather than relying on the human-readable error message text. This keeps tests stable even if wording changes, while still verifying the correct validation rule is triggered.
Applied to files:
tests/training/test_huggingface_backend.pytests/conftest.pytests/pii_replacer/test_detect.py
📚 Learning: 2026-08-05T19:07:15.856Z
Learnt from: binaryaaron
Repo: NVIDIA-NeMo/Safe-Synthesizer PR: 679
File: tests/conftest.py:89-96
Timestamp: 2026-08-05T19:07:15.856Z
Learning: In the Safe Synthesizer test suite, treat CPU and CUDA installation profiles as the supported pytest collection profiles because they install PyTorch. A bare installation without PyTorch is an incomplete, unsupported profile. GPU-marked test modules may import PyTorch before tests/conftest.py::pytest_collection_modifyitems executes, so do not require that collection hook to prevent such imports.
Applied to files:
tests/training/test_huggingface_backend.pytests/conftest.pytests/pii_replacer/test_detect.py
🔇 Additional comments (12)
src/nemo_safe_synthesizer/training/huggingface_backend.py (1)
424-424: LGTM!tests/conftest.py (1)
139-139: LGTM!tests/training/test_huggingface_backend.py (1)
136-141: LGTM!Also applies to: 147-152
script/brev/setup.sh (1)
271-274: LGTM!docs/tutorials/safe-synthesizer-101.ipynb (2)
2-148: LGTM!Also applies to: 149-154, 155-230, 249-251
231-248: 🗄️ Data Integrity & IntegrationNo change required.
setup.shregisters the kernelspec in thepython3directory and setsBNB_CUDA_VERSION=128. The notebook selectspython3.> Likely an incorrect or invalid review comment.src/nemo_safe_synthesizer/pii_replacer/data_editor/detect.py (3)
8-8: LGTM!
629-632: 🎯 Functional CorrectnessNo API compatibility issue
The lockfile selects GLiNER
0.2.24, which providesinference. Older releases withoutinferencealso providepredict_entities, which this fallback supports.> Likely an incorrect or invalid review comment.
589-595: 🎯 Functional CorrectnessDo not change this warning filter for the current dependency set. The project requires
huggingface-hub>=1.3.0and locks version1.17.0;resume_downloadwas removed in version 1.0.0. This deprecation warning does not apply to the pinned dependency.> Likely an incorrect or invalid review comment.tests/pii_replacer/test_detect.py (3)
69-69: LGTM!Also applies to: 87-87
106-113: LGTM!
149-149: LGTM!Also applies to: 166-166
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/nemo_safe_synthesizer/config/training.py (1)
214-225:⚠️ Potential issue | 🟠 MajorEnforce the documented warmup domain for both field names.
warmup_stepsaccepts1.5andfloat("inf"), although the description permits only an integer step count or a ratio in(0, 1).warmup_ratiohas no equivalent constraint, and Line 244 copies it after normal field validation. Invalid legacy values can reach the backend, which forwardswarmup_stepsdirectly to the trainer.Use one finite, range, and integrality validator for both input paths. Add regression tests for both names. This repeats the unresolved validator finding from the previous review.
As per path instructions, configuration changes must check “default values, override merging, validators, aliases, and backwards-compatible user-facing config changes,” and new behavior needs focused tests.
#!/usr/bin/env bash set -euo pipefail rg -n -C 5 'warmup_(ratio|steps)|validate_assignment' src tests pyproject.toml uv.lockAlso applies to: 227-244
Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e68240ed-a2d5-4879-883c-30e6a1918bbb
📒 Files selected for processing (2)
docs/tutorials/safe-synthesizer-101.ipynbsrc/nemo_safe_synthesizer/config/training.py
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/tutorials/safe-synthesizer-101.ipynb
📜 Review details
⏰ Context from checks skipped due to timeout. (8)
- GitHub Check: Analyze (Python)
- GitHub Check: conventional-commit / semantic-pull-request
- GitHub Check: Greptile Review
- GitHub Check: Detect changes
- GitHub Check: Format
- GitHub Check: Typecheck
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{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:
src/nemo_safe_synthesizer/config/training.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Useuvfor Python dependency management and execution; never usepipor rawpython. Target Python 3.11–3.14 and use modern syntax such asX | Y,list[str], andSelf.
Run project tooling throughmisetasks or wrapper scripts intools/; do not invokeruffortydirectly. Useuv runfor Python execution.
Put durable implementation guidance in public function and class docstrings; put local invariants in source comments.
Use Python 3.11–3.14-compatible modern type syntax, includingX | Y,list[str], andSelf.
**/*.py: Use American English spelling in Python code, comments, and documentation; usefrom __future__ import annotationsin every module.
UseBaseSettingsfor environment/CLI settings; preferAliasChoicesfor fields accepting both Python and environment-variable names.
Pydantic model fields must includeField(description=...); prefer assignment-styleField()and useAnnotatedonly for additional metadata or constraints.
Use@dataclass(frozen=True)for immutable value objects and validators, andfield(default_factory=...)for mutable defaults; never use mutable default values directly.
UseStrEnumfor string-valued configuration or serialization enums and plainEnumfor internal-only constants.
Obtain loggers withobservability.get_logger(__name__); do not calllogging.getLogger()orstructlog.get_logger()directly.
Do not useprint()for operational library output; use the approved logger,click.echo()for CLI output, orsys.stdout.write()for raw tool output.
Use loggerextra={}for metrics, counts, durations, and other data intended for machine querying; use f-strings for human-readable context.
Raise known errors through the Safe Synthesizer custom hierarchy, using dual inheritance where callers should also catch a built-in exception.
UseX | Y, built-in generic types,Selffor fluent returns, collection ABCs for arguments,Protocolfor structural boundaries, and avoidAny.
Pre...
Files:
src/nemo_safe_synthesizer/config/training.py
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: ReadAGENTS.local.mdif it exists and give its instructions top priority.
When a task matches a repository-specific skill, read the corresponding skill under.agents/skills/instead of duplicating its workflow instructions.
Do not commit unless the user explicitly asks for a commit or PR work.
When committing, require DCO sign-off and GPG signing usinggit commit --signoff --gpg-sign(or-s -S); never manually addSigned-off-byor use--no-gpg-sign.
Use feature branches based onmain; branch names commonly include an issue-number prefix such as<author>/123-short-name.
For recurring testing, building, syncing, bootstrapping, worktree, and GitHub workflows, use the matching skill under.agents/skills/.
For a full GPU/development environment, useuv sync --frozen --extra cu129 --extra engine --group dev; bareuv sync --frozenis incomplete and can causety, import-check, and GPU-test failures.End files with a newline, contain no trailing whitespace, use one space between sentences, and keep code, comments, and docstrings within 120 characters.
Files:
src/nemo_safe_synthesizer/config/training.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:
src/nemo_safe_synthesizer/config/training.py
**/*.{md,py,sh,toml,yml,yaml,Dockerfile}
📄 CodeRabbit inference engine (AGENTS.md)
Follow the detailed language and file-format conventions defined in
STYLE_GUIDE.md.
Files:
src/nemo_safe_synthesizer/config/training.py
src/**/config/**/*.py
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
Use
NSSBaseModelfor user-facing configuration and parameter models inconfig/; useBaseModelor module-specific bases for DTOs and internal structures.
Files:
src/nemo_safe_synthesizer/config/training.py
src/**/*.py
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
src/**/*.py: Keep shared package code compatible with Python 3.11; do not use Python 3.12-only syntax such as PEP 695 type statements or bracketed generic parameters.
UseTYPE_CHECKINGguards for heavy imports such aspandas,torch, andtransformers; use relative imports insrc/.
Do not add__init__.pyto test directories; every directory undersrc/containing Python files must have one.
Comments should explain why rather than narrate what; do not add redundant docstrings, defensiveexcept Exceptionaround trusted internal calls, unjustified type ignores, or casts/Anyto hide type errors.
Usepathlib.Pathinstead ofos.path, and use explicitif/raisevalidation instead ofassertin library code.
Files:
src/nemo_safe_synthesizer/config/training.py
⚙️ CodeRabbit configuration file
Review library code against STYLE_GUIDE.md. Focus on behavior, API contracts, error handling, resource cleanup, typing, logging, and user-facing failures. Public APIs and nontrivial functions need Google-style docstrings.
Files:
src/nemo_safe_synthesizer/config/training.py
**/*.{py,sh,yaml,yml,md}
📄 CodeRabbit inference engine (STYLE_GUIDE.md)
Include the required SPDX copyright and Apache-2.0 license headers, using comment syntax appropriate to the file format.
Include SPDX copyright headers in all source files, except files explicitly listed in
.copyrightignore.
Files:
src/nemo_safe_synthesizer/config/training.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{py,pyi}: Keep shared Python package code compatible with Python 3.11; do not use Python 3.12-only syntax such as PEP 695 type statements or bracketed generic parameters.
Use the repository's pinned Ruff tasks for Python formatting, import sorting, and linting rather than invoking unpinned tools directly.
Run the repository's pinnedtytype checker and maintain type-correct Python code.
Files:
src/nemo_safe_synthesizer/config/training.py
**/*.{py,sh,yaml,yml,toml,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use
misetasks with the repository's pinned tool versions for formatting, checking, and testing before submitting changes.
Files:
src/nemo_safe_synthesizer/config/training.py
src/nemo_safe_synthesizer/config/**/*.py
⚙️ CodeRabbit configuration file
Treat config changes as user-facing API changes. Check Pydantic field descriptions, defaults, validators, aliases, override behavior, CLI help text impact, YAML compatibility, and documented parameter semantics.
Files:
src/nemo_safe_synthesizer/config/training.py
🔇 Additional comments (1)
src/nemo_safe_synthesizer/config/training.py (1)
7-15: LGTM!
- Replace GLiNER.batch_predict_entities (deprecated) with .inference - Rename warmup_ratio -> warmup_steps in TrainingHyperparams; pass directly to TrainingArguments which accepts floats in (0,1) as ratios, eliminating the manual ratio-to-steps conversion - Suppress huggingface_hub resume_download UserWarning at the GLiNER load site (upstream gliner passes the deprecated arg) - Suppress scipy ks_2samp asymptotic fallback RuntimeWarning in the tutorial notebook via a scoped with catch_warnings() block - Pre-compile torchao and range_regex in brev setup so SyntaxWarnings go to the setup log instead of appearing in notebook output - Pin BNB_CUDA_VERSION=128 in the brev kernelspec; bitsandbytes has no CUDA 12.9 binary (odd releases are skipped) and always falls back to 12.8; update to 130 when upgrading CUDA_EXTRA to cu130+ Closes #328 Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
Mirror transformers behaviour: warmup_ratio is accepted but emits a DeprecationWarning and copies its value to warmup_steps. Also fix notebook JSON indentation (indent=1 -> indent=2). Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
- warmup_ratio migration only copies to warmup_steps when warmup_steps was not explicitly set, so explicit warmup_steps always takes precedence over the deprecated alias - _predict_entities now raises the same explicit AttributeError as _batch_predict_entities when the model has neither predict_entities nor inference Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
warmup_steps accepted 1.5, which transformers truncates to 1, and
float("inf"), which raises OverflowError once converted to an integer.
Validate that the value is finite, positive, and either a ratio below 1
or a whole step count.
Apply the same check to the deprecated warmup_ratio alias: it assigns to
warmup_steps from an "after" model validator, and validate_assignment is
off, so field validation would otherwise be bypassed entirely.
Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
transformers treats warmup_steps=0 as "no warmup", but our validator required a strictly positive value, so that setting was unreachable from config. Relax the lower bound to >= 0; 0 already takes the ratio branch, so no other logic changes. Fractional values at or above 1, non-finite values, and negatives stay rejected. Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
The asymptotic-fallback notice was silenced in the 101 notebook only, so it still surfaced in the other two tutorials that run the pipeline and in every CLI and SDK run. Move the filter next to the two ks_2samp calls in text_semantic_similarity.py, the only place it is raised, and revert the notebook to a plain builder.run(). Also revert an unrelated no-op fixture change in tests/training/test_huggingface_backend.py, left over from a helper that no longer exists. Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
Both branches added in this PR were unexercised, which is what the patch coverage check flagged. Extend the two existing GLiNER fallback tests with a model exposing neither API, and assert _suppress_ks_exact_fallback silences SciPy's asymptotic-fallback notice while letting other RuntimeWarnings through. Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
0fcd4cc to
a671f9e
Compare
Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
Summary
GLiNER.batch_predict_entities(deprecated) with.inferencewarmup_ratio→warmup_stepsinTrainingHyperparamsto match the transformers API;warmup_ratiois kept as a deprecated alias that emits aDeprecationWarningand copies its value towarmup_steps; floats in (0, 1) are still interpreted as ratios byTrainingArguments, so0.05default is unchanged in behaviourhuggingface_hubresume_downloadUserWarningat the GLiNER load site (upstream gliner passes the deprecated arg)scipyks_2sampasymptotic fallbackRuntimeWarningin the tutorial notebook via a scopedwith warnings.catch_warnings()block aroundbuilder.run()torchaoandrange_regexin the brev setup script so their one-timeSyntaxWarnings go to the setup log instead of notebook outputBNB_CUDA_VERSION=128in the brev kernelspec — bitsandbytes skips odd CUDA releases so 12.9 has no native binary and always falls back to 12.8; update to130when upgradingCUDA_EXTRAtocu130+Test plan
mise run test)Summary by CodeRabbit
New Features
warmup_stepsas a fixed count or fraction of total training steps, while maintaining compatibility with the deprecated setting.Bug Fixes
Documentation