Repository navigation
Conversation
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Comment |
github-actions
Bot
added
area:dev-ex
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
nina-xu
force-pushed
the
feat/replace-pii-v3
branch
from
September 14, 2026 21:12
66fff22 to
6e59404
Compare
nina-xu
force-pushed
the
feat/replace-pii-v3
branch
2 times, most recently
from
October 1, 2026 15:39
316e375 to
17824d3
Compare
zywind
force-pushed
the
feat/replace-pii-v3
branch
2 times, most recently
from
October 6, 2026 13:30
ca908f4 to
c0299d6
Compare
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| def _signal_group(process_group: int, signum: signal.Signals) -> None: | ||
| try: | ||
| os.killpg(process_group, signum) | ||
| except ProcessLookupError: |
<!-- SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. --> <!-- SPDX-License-Identifier: Apache-2.0 --> <!-- Thank you for contributing to Safe Synthesizer! --> <!-- Brief description of changes --> - **Transform v2 is gone**. The NER/GLiNER engine, transform actions, and their tests/deps are removed. What remains is a placeholder `ReplacePiiConfig` (PII still requested by default) and evaluation types like ColumnStatistics. - **Runs with PII on will fail**. process_data() raises unless you set replace_pii: null, --no-replace-pii, or .with_replace_pii(enable=False). --validate still skips replacement. - **Old YAML gets rejected no matter what**. globals / steps raise a dedicated “PII v2 removed” error and point at the config docs, including when unknown_fields: ignore. - **Docs/CLI**. User guides are stubbed. NSS_INFERENCE_* is kept but marked unused. NER/column-classification/GLiNER wording is stripped (notebooks left for later). Unused entity_metadata.py is deleted. <!-- These checks should be completed before a PR is reviewed, --> <!-- but you can submit a draft early to indicate that the issue is being worked on. --> Ensure that the following pass: - [x] `mise run format && mise run check` or via prek validation. - [x] `mise run test` passes locally - [ ] `mise run test:e2e` passes locally - [ ] `mise run 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) <!-- These checks need to be completed before a PR is merged, --> <!-- but as PRs often change significantly during review, --> <!-- it's OK for them to be incomplete when review is first requested. --> - [ ] New or updated tests for any fix or new behavior - [ ] Updated documentation for new features and behaviors, including docstrings for API docs. <!-- Please add the issue number that should be closed when this PR is merged. --> - Closes #<issue> --------- Signed-off-by: nina-xu <19981858+nina-xu@users.noreply.github.com>
<!-- SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. --> <!-- SPDX-License-Identifier: Apache-2.0 --> <!-- Thank you for contributing to Safe Synthesizer! --> <!-- Brief description of changes --> Added the pydantic classes for the agreed upon config shape. This is the first of many PRs that will become the PII replacement V3 feature. For context, the order of execution (and the way these PRs are structured) is ``` Config Contract ├──> Config Parsing │ ├──> LLM-Free PII Replacement Execution │ └──> LLM-Based PII Replacement Execution ├──> Rule-Based Auto-Configuration └──> LLM-Based Auto-Configuration ``` See [pii_replacement_plan_spec.md](https://github.com/user-attachments/files/31711774/pii_replacement_plan_spec.md) for behavior details. This file also includes what to add in the upstream (discovery) and downstream (replacement) PRs so they all work together. See [pii_plan_alexa_max_complexity.yaml](https://github.com/user-attachments/files/31320805/pii_plan_alexa_max_complexity.yaml) for the expected yaml shape for [this dataset](https://docs.google.com/spreadsheets/d/1aimvPT9sXbs0yu5WIAyqzuEbqcQXwuk9jikdNGDcqaM/edit?gid=1179702036#gid=1179702036). <!-- These checks should be completed before a PR is reviewed, --> <!-- but you can submit a draft early to indicate that the issue is being worked on. --> Ensure that the following pass: - [x] `mise run format && mise run check` or via prek validation. - [x] `mise run test` passes locally - [ ] `mise run test:e2e` passes locally - [ ] `mise run 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) <!-- These checks need to be completed before a PR is merged, --> <!-- but as PRs often change significantly during review, --> <!-- it's OK for them to be incomplete when review is first requested. --> - [ ] New or updated tests for any fix or new behavior - [ ] Updated documentation for new features and behaviors, including docstrings for API docs. <!-- Please add the issue number that should be closed when this PR is merged. --> - Closes #<issue> --------- Signed-off-by: nina-xu <19981858+nina-xu@users.noreply.github.com>
# Summary - Add plan resolution interfaces that always run the heuristic baseline before optional LLM enhancement. - Support inline and standalone YAML plans with context-free model validation followed by one dataframe-aware validation gate. - Add deterministic bounded column profiling, canonical sparse plan serialization, tests, and configuration documentation. ## Human review Select exactly one. The default is no human review; uncheck it when choosing another level. - [ ] ⚪ No human review; automated review only - [x] 🟡 Partially reviewed or spot-checked by a human - [ ] 🔵 Complete diff reviewed by a human - [ ] 🟢 Complete diff reviewed and verified by a human Verification performed: - `mise run check` - Focused config/planning/SDK/CLI suite (183 passed) ## Pre-Review Checklist Ensure that the following pass: - [x] `mise run format && mise run check` or via prek validation. - [ ] `mise run test` passes locally - [ ] `mise run test:e2e` passes locally - [ ] `mise run 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 - [x] New or updated tests for any fix or new behavior - [x] Updated documentation for new features and behaviors, including docstrings for API docs. ## Other Notes - No linked issue. - The full unit suite reaches an unrelated existing telemetry context-manager failure; the changed-area suites pass. --------- Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
# Summary - Add OpenAI-compatible LLM enhancement for automatically discovered PII replacement plans. - Classify entity types and optional grammar-constrained patterns in pass one, while NSS determines replacement membership and permitted dependency candidates. - Limit pass two to selecting contextually useful candidate IDs, with structured-output validation, retries, and focused pattern repair. - Share one retry loop across all requests: transient failures back off (numeric `Retry-After`, otherwise capped exponential backoff with jitter), and invalid responses are retried with feedback naming the offending columns, IDs, or dependency conflicts. - Send an OpenAI strict-mode JSON schema that lists every property as required. - Resolve the model as CLI `--inference-model-id`, then YAML `replace_pii.llm.model_id`, then `NSS_INFERENCE_MODEL`, then the default. - Require HTTPS for inference endpoints unless the host is loopback (`localhost`, `127.0.0.0/8`, `::1`), because requests carry raw cell samples and the API key. ## Human review Select exactly one. The default is no human review; uncheck it when choosing another level. - [ ] ⚪ No human review; automated review only - [x] 🟡 Partially reviewed or spot-checked by a human - [ ] 🔵 Complete diff reviewed by a human - [ ] 🟢 Complete diff reviewed and verified by a human Verification performed: - `mise run check` (format, lint, type, license headers, lock) passed locally at d04a91b. - `mise run test` at d04a91b: 1896 passed; 8 `tests/evaluation` modules failed to import because `pkg_resources` is missing from the local environment, unrelated to this change. ## Pre-Review Checklist - [x] `mise run format && mise run check` or via prek validation. - [ ] `mise run test` passes locally - [ ] `mise run test:e2e` passes locally - [ ] `mise run 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 - [x] New or updated tests for any fix or new behavior - [x] Updated documentation for new features and behaviors, including docstrings for API docs. ## Other Notes - Targets `feat/replace-pii-v3`, which contains the shared plan-discovery workflow. - Plain HTTP to a non-loopback host (for example `http://vllm:8000` across containers) is now rejected; such endpoints need HTTPS. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added LLM-assisted PII plan discovery with column classification, dependency selection, and optional replacement-pattern repair. - Added support for hosted and local OpenAI-compatible inference endpoints, including keyless local operation. - Added CLI and environment configuration for the endpoint, credentials, and model. Explicit CLI model selections take precedence over saved settings. - **Bug Fixes** - Pre-flight checks now report invalid inference endpoints and missing credentials for the default hosted endpoint. - **Documentation** - Updated setup, usage, planning, and troubleshooting guidance for PII inference, including endpoint security and configuration precedence. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
# Summary - Define the public `TabularPiiReplacer` seam and private managed/Faker replacement-generator adapters with immutable, PII-safe requests. - Add validated GLiNER2-plus-regex configuration and explicit record, group, canonical-value, free-text reuse, provenance, and safe drift contracts. - Extend `TransformResult` with the resolved plan, total elapsed time, and replacement-generation timing/count statistics. - Make the LLM planning-only and add a dated next-session handoff selecting `fastino/gliner2-privacy-filter-PII-multi` as the implementation default. ## Human review Select exactly one. The default is no human review; uncheck it when choosing another level. - [ ] ⚪ No human review; automated review only - [x] 🟡 Partially reviewed or spot-checked by a human - [ ] 🔵 Complete diff reviewed by a human - [ ] 🟢 Complete diff reviewed and verified by a human Verification performed: Automated checks only; no human review is claimed. ## Pre-Review Checklist - [ ] `mise run format && mise run check` or via prek validation. Formatting, lint, and copyright pass; typecheck reports nine inherited LLM-planning diagnostics from the older stacked base. - [ ] `mise run test` passes locally. The earlier full run reached 1,699 passed and 7 skipped; inherited LLM-planning import errors and CPU-host vLLM/CUDA loading prevent a clean result on this checkout. - [ ] `mise run test:e2e` passes locally - [ ] `mise run 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 - [x] New or updated tests for any fix or new behavior - [x] Updated documentation for new features and behaviors, including docstrings for API docs. ## Other Notes - Stacked on `yunfeng/pii-v3-llm-enhancement`; implementation started from the specified `dc954ef` handoff commit. - Focused config/interface/contract suite: 90 passed. `mise run lock-check` also passes. - Next-session plan: `docs/plans/2026-09-10-pii-v3-replacement-engine-handoff.md`. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added configurable free-text PII detection settings, including model, confidence threshold, batching, chunking, and overlap. - Added public interfaces and result models for tabular PII replacement; replacement execution is not yet available. - Added validated contracts for detection spans, mappings, provenance, and replacement generation, with safeguards for sensitive values. - **Documentation** - Clarified that LLM inference supports plan discovery, while free-text detection uses configurable GLiNER2 settings. - Added a detailed handoff specification for replacement execution. - **Tests** - Added coverage for configuration, validation, privacy safeguards, mappings, generation contracts, and result models. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
<!-- SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. --> <!-- SPDX-License-Identifier: Apache-2.0 --> # Summary Builds on #738. LLM-assisted PII plan discovery now runs `nvidia/NVIDIA-Nemotron-3.5-Lightning-30B-A3B-BF16` in a vLLM server that NSS starts on the local GPU, modeled on NVIDIA-NeMo/Anonymizer#212 but scoped to one process lifetime. - Without `NSS_INFERENCE_ENDPOINT`, the model ID (`replace_pii.llm.model_id`, `--inference-model-id`, `NSS_INFERENCE_MODEL`) selects a bundled pinned profile, defaulting to Nemotron 3.5 Lightning; `--inference-local-profile` takes a custom profile YAML (or a bundled profile name); with a profile, a loopback `NSS_INFERENCE_ENDPOINT` sets the managed server's listening address. Setting an endpoint (hosted or your own server) opts out and then requires an explicit model ID; there is no hosted default model any more. Missing vLLM or GPU is an error, never a silent fallback to a remote service. - `resolve_plan` stops the server's process group before validation, saving, or any later replacement or training, including on failure. Each launch uses a fresh key via `VLLM_API_KEY`, request logging stays off, the server gets SIGTERM if NSS dies without cleanup, and its environment gets the same Outlines diskcache protections as in-process generation (CVE-2025-69872; the helper moved to `outlines_cache.py`). Readiness requires `/v1/models` to list the model and one strict-JSON-schema request to succeed. Loopback inference requests ignore environment proxies, so a proxy never receives the key or cell samples. - Bundled profiles: `nvidia/NVIDIA-Nemotron-3.5-Lightning-30B-A3B-BF16` (default), `openai/gpt-oss-120b`, and `Qwen/Qwen3.8-27B`, all with reasoning on and verified on an A100 80GB. Nemotron and GPT-OSS-120B both performed well in our PII planning evaluation. Each sets a per-request timeout, overridable with `NSS_INFERENCE_TIMEOUT` / `--inference-timeout-seconds`. - Profiles can set per-request options (sampling, `thinking_token_budget`) and server environment; `NSS_INFERENCE_REQUEST_OPTIONS` / `--inference-request-options` override them field by field (`null` drops a field). NSS no longer forces `temperature: 0`; fields left out use the server's defaults (for vLLM, the model's `generation_config.json`). Qwen runs at low effort with a 1,000-token thinking budget and Nemotron with a 2,000-token budget; all three profiles, including GPT-OSS-120B, use model-card sampling (values chosen in our PII planning evaluation). - Preflight validates the local server instead of the remote endpoint and key, and the run log states whether planning data stays on this machine (documented under "Where planning requests go"); an autouse test fixture blocks real vLLM launches outside `requires_gpu` tests. Test plan (automated, run by the agent): `mise run check` passes, and the affected unit suites pass (PII replacer, preflight, CLI, config, vLLM backend, tools; one known-flaky `test_vllm_shutdown` case has failed under xdist in full runs); `mise run test:smoke:gpu:local-inference` passes on an A100 80GB; `run replace-pii --plan-only` with `replace_pii.llm: {}` and no endpoint produced a plan on the A100 with GPU memory back to 0 MiB afterward (run while GPT-OSS-120B was the default; not rerun since the Nemotron default). ## Human review Select exactly one. The default is no human review; uncheck it when choosing another level. - [x] ⚪ No human review; automated review only - [ ] 🟡 Partially reviewed or spot-checked by a human - [ ] 🔵 Complete diff reviewed by a human - [ ] 🟢 Complete diff reviewed and verified by a human Verification performed: ## Pre-Review Checklist Ensure that the following pass: - [x] `mise run format` leaves only intended formatting changes. - [x] `mise run check ::: test` passes locally. - [ ] `mise run test:e2e` passes locally - [ ] `mise run 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 - [x] New or updated tests for any fix or new behavior - [x] Updated documentation for new features and behaviors, including docstrings for API docs. ## Other Notes - The GPU smoke test is marked `smollm2`, so the existing `test:smoke:gpu:smollm2` stage runs it. --------- Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
zywind
force-pushed
the
feat/replace-pii-v3
branch
from
October 28, 2026 19:05
80e9372 to
80a43a9
Compare
# Summary - Implement deterministic, plan-driven structured PII replacement with Faker and managed Nemotron Personas backends. - Add dataset-specific dependency label mappings, pattern validation, collision handling, and managed-to-Faker fallback behavior. - Accelerate managed sampling with projected Arrow-backed columns, precomputed label indexes, and cached candidate intersections. ## Human review Select exactly one. The default is no human review; uncheck it when choosing another level. - [ ] ⚪ No human review; automated review only - [x] 🟡 Partially reviewed or spot-checked by a human - [ ] 🔵 Complete diff reviewed by a human - [ ] 🟢 Complete diff reviewed and verified by a human Verification performed: Automated formatting, static checks, focused tests, and full-dataset performance comparison. ## Pre-Review Checklist - [x] `mise run format && mise run check` or via prek validation. - [ ] `mise run test` passes locally - [ ] `mise run test:e2e` passes locally - [ ] `mise run 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 - [x] New or updated tests for any fix or new behavior - [x] Updated documentation for new features and behaviors, including docstrings for API docs. ## Other Notes - `uv run --frozen pytest tests/pii_replacer tests/config/test_replace_pii.py -q -n0`: 280 passed. - The full unit run reached 99% but two unrelated telemetry lifecycle tests failed and reproduce in isolation. - Full `patient_events.csv` managed replacement improved from 24.05 seconds to 1.35 seconds after caching resolved candidate sets. - Behavior change: identifier replacements start at a position drawn uniformly over the whole value space, so long masks such as `ADM-` plus 32 characters no longer begin with a run of zeros. With a fixed seed, identifier replacements now differ from before. - No linked issue. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Structured PII replacement now runs during synthesis with deterministic, pattern-preserving values. - Supports managed data assets with Faker fallback, dependency-aware mappings, and automatic label discovery. - Plan-only workflows resolve and save a reusable PII replacement configuration. - SDK results now include the resolved replacement configuration. - Email patterns support organization-based domains. - **Bug Fixes** - Improved validation for dependency mappings, replacement dependencies, cycles, collisions, and unsupported free-text targets. - **Documentation** - Updated configuration, planning, execution, and sampler guidance for the new workflows. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Yunfeng Zhang <yunzhang@nvidia.com>
This branch has not been deployed
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
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
ReplacePiiConfig(PII still requested by default) and evaluation types like ColumnStatistics.Pre-Review Checklist
Ensure that the following pass:
mise run format && mise run checkor via prek validation.mise run testpasses locallymise run test:e2epasses locallymise run test:ci-containerpasses locally (recommended)/syncon this PR to trigger a run (auto-triggers on ready-for-review)Pre-Merge Checklist
Other Notes
Summary
Human review
Select exactly one. The default is no human review; uncheck it when choosing another level.
Verification performed:
Pre-Review Checklist
Ensure that the following pass:
mise run format && mise run checkor via prek validation.mise run testpasses locallymise run test:e2epasses locallymise run test:ci-containerpasses locally (recommended)/syncon this PR to trigger a run (auto-triggers on ready-for-review)Pre-Merge Checklist
Other Notes