feat: add excluded labels for entities in Anonymizer - #243
Conversation
|
|
/nvskills-ci |
6a153c2 to
e3b99da
Compare
|
/nvskills-ci |
fd66bbd to
7f87728
Compare
|
The param name |
|
|
/nvskills-ci |
|
/nvskills-ci |
1 similar comment
|
/nvskills-ci |
Adds entity_label_denylist: list[str] | None to the Detect model, with the same normalisation (strip, lowercase, deduplicate) as entity_labels. A model_validator warns at config construction time when entity_labels and entity_label_denylist share labels that would never be detected. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Applies the denylist at two points: - _resolve_detection_labels: subtracts denied labels before they reach GLiNER and the LLM augmenter/validator prompts - _materialize_final_entities: safety-net filter that drops any entity whose label is in the denylist from COL_FINAL_ENTITIES Threads entity_label_denylist through detect_and_validate_entities, _build_detection_spec, identify_latent_entities, and run on EntityDetectionWorkflow, and wires it from Anonymizer._run_internal via config.detect.entity_label_denylist. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…ion paths Adds tests for build_detection_config and build_detection_builder_for_seed confirming that denied labels are subtracted from the GLiNER label list in the serialized workflow config, so external runtimes see the same effective label set as the in-process path. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…abel set When the denylist subtracts all labels from the effective detection set, _resolve_detection_labels now emits a warning instead of silently passing an empty list to GLiNER (which returns no detections, not the default set). Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…peline Stores entity_label_denylist on AnonymizerResult and PreviewResult so evaluate() can scope its judges to the same label set that was active during detection. Changes: - AnonymizerResult / PreviewResult: add entity_label_denylist field - Anonymizer.evaluate(): extract entity_label_denylist from the result and pass it to EntityCoverageWorkflow (rewrite path) and ReplacementWorkflow.evaluate() (replace path) - EntityCoverageWorkflow: accept entity_label_denylist, pass to _filter_out_of_scope_entities in postprocess - _filter_out_of_scope_entities: exclude entities whose label is in the denylist so the judge does not penalise the output for not anonymizing denied labels - ReplacementWorkflow.evaluate(): thread entity_label_denylist through to EntityCoverageWorkflow Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…filter Signed-off-by: memadi <memadi@nvidia.com>
Signed-off-by: memadi <memadi@nvidia.com>
Signed-off-by: memadi <memadi@nvidia.com>
Signed-off-by: memadi <memadi@nvidia.com>
Signed-off-by: memadi <memadi@nvidia.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Signed-off-by: nvskills-svc-account <svc-nvskills-signing@nvidia.com>
Signed-off-by: memadi <memadi@nvidia.com>
Signed-off-by: memadi <memadi@nvidia.com>
NVSkills rewrites these files without SPDX preambles, so exclude them until the generator preserves existing headers. Upstream: NVIDIA/nvskills-ci#66 Signed-off-by: memadi <memadi@nvidia.com>
Signed-off-by: memadi <memadi@nvidia.com>
Signed-off-by: nvskills-svc-account <svc-nvskills-signing@nvidia.com>
1d9580f to
ca63978
Compare
|
/nvskills-ci |
Signed-off-by: memadi <memadi@nvidia.com>
Signed-off-by: memadi <memadi@nvidia.com>
_filter_excluded_latent_entities only handled schema/dict/list shapes, so a JSON-string payload (the shape every other schema's from_raw() already anticipates via _parse_raw_wrapper) fell through unfiltered, letting excluded latent labels leak into the rewrite prompt. Also fixes a missing blank line that broke the MkDocs warning admonition in docs/concepts/detection.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The strip+casefold idiom for comparing/filtering entity labels was independently reimplemented in ~8 places across detection_workflow.py, postprocess.py, and entity_coverage_judge.py, with at least one site (entity_coverage_judge._effective_entity_labels) silently missing the .strip() step. Consolidate into normalize_label()/normalize_labels() in postprocess.py so the normalization rule only needs to change once. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…_labels Previously, entity_labels being entirely canceled out by excluded_entity_labels only logged a warning and silently produced a config that detects nothing at runtime. Fail early instead: raise a ValueError at config-construction time when excluded_entity_labels entirely overlaps an explicit entity_labels, leaving an empty effective detection set. Partial overlaps still only warn, and entity_labels=None (the default set) is unaffected by this check. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Extend the ValueError-on-full-overlap documentation added in 52975e8 to the two other places that described this field: the field's own description= (feeds the mkdocstrings API reference) and the duplicate excluded_entity_labels section in choosing-a-strategy.md, which never had the overlap warning at all. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Matches the phrasing already used in detection.md and choosing-a-strategy.md for the same behavior. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
/nvskills-ci |
Signed-off-by: nvskills-svc-account <svc-nvskills-signing@nvidia.com>
binaryaaron
left a comment
There was a problem hiding this comment.
agent (
review-pr): Blocking empty effective-label path remains
[P1] Reject exclusions that remove every default label
The explicit-overlap validator does not run when entity_labels=None. On the current head (f3cffb5), this remains valid:
Detect(
entity_labels=None,
excluded_entity_labels=list(DEFAULT_ENTITY_LABELS),
)Both the standard and latent detection paths call _resolve_detection_labels(). It returns [], logs a warning, and the workflow injects that empty list into the detector configuration.
A local regression test confirms the missing guard:
Failed: DID NOT RAISE <class 'ValueError'>
With the bundled GLiNER server, this silently disables detection. Other detector endpoints may interpret an empty label list differently. Either outcome violates the intended fail-early behavior for an empty effective detection set.
Please treat this as blocking and ensure no workflow can send labels=[] to the detector.
Prompt for an implementation agent
Fix the remaining empty-effective-label defect in PR #243.
Requirements:
- Reproduce the defect with
entity_labels=Noneandexcluded_entity_labelscontaining every value inDEFAULT_ENTITY_LABELS. - Reject that configuration with a clear
ValueErrorbefore a detector workflow executes. - Ensure both callers of
_resolve_detection_labels()—the standard detection specification and latent-entity detection—cannot injectlabels=[]. - Add a regression test for the default-label case. Update the existing warning test if the new invariant makes it obsolete.
- Preserve partial default exclusions, label normalization, deduplication, and explicit-label overlap behavior.
- Keep the invariant in the narrowest owning layer; do not duplicate the effective-label calculation across callers.
- Use
astnav refs _resolve_detection_labelsto verify the complete caller set before editing. - Run the focused config and detection-workflow tests, Ruff format/lint, and
git diff --check.
Report the changed files, the ownership decision, and the exact verification results.
…usion The earlier full-overlap guard only checked an explicit entity_labels against excluded_entity_labels. Reviewer feedback: entity_labels=None falls back to DEFAULT_ENTITY_LABELS, and excluded_entity_labels could still cancel that out entirely, leaving _resolve_detection_labels to silently return [] at runtime with only a warning. Detect now raises in that case too, and docs/SKILL.md are reworded to state the rule once against "the effective allowlist" instead of covering the two cases with separate sentences. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
/nvskills-ci |
lipikaramaswamy
left a comment
There was a problem hiding this comment.
Small nit, but looks great overall!
| ``` | ||
|
|
||
| !!! warning | ||
| `excluded_entity_labels` is always checked against the effective allowlist — `entity_labels` if set, otherwise `DEFAULT_ENTITY_LABELS`. A partial overlap just drops the shared labels and logs a warning. If the overlap is total, leaving an empty effective detection set, `Detect` raises a `ValueError` at config time instead of silently detecting nothing. |
There was a problem hiding this comment.
Small mismatch: this says a partial overlap always logs a warning, but the validator only warns when entity_labels is explicitly set. Partial exclusions from DEFAULT_ENTITY_LABELS are accepted without a warning, which is also the common use case. Could we clarify that distinction here and in the matching warning in docs/concepts/choosing-a-strategy.md?
Summary
Adds
excluded_entity_labelsto theDetectconfiguration, allowing users to explicitly exclude specific entity label types from detection. Denied labels are removed from detector scope, filtered from final explicit and latent entity outputs, and excluded from entity-coverage scoring.Changes
Config
excluded_entity_labels: list[str] | Nonefield toDetectwith the same normalisation asentity_labels(strip, lowercase, deduplicate, reject empty)entity_labelsandexcluded_entity_labelsshare labels that would never be detected.Detection Engine
_resolve_detection_labels)_materialize_final_entitiesto drop denied labels from final entity output regardless of LLM outputexcluded_entity_labelsthrough all detection pipeline methods:run,detect_and_validate_entities,_build_detection_spec,identify_latent_entitiesbuild_detection_configandbuild_detection_builder_for_seedEvaluation
excluded_entity_labelsfield toAnonymizerResultandPreviewResultsoevaluate()has access to the full detection scopeexcluded_entity_labelsthroughevaluate()toEntityCoverageWorkflowandReplacementWorkflow.evaluate()Telemetry
excluded_entity_labelsto_detect_config_metadataso it is captured in measurement records alongsideentity_labelsTests
_effective_entity_labelshelper, coverage judge postprocess filter, coverage prompt scopingexcluded_entity_labelscaptured correctly when set and absentDocs
Detectfields reference table and added "Excluding labels with a deny list" section indetection.mdexcluded_entity_labelsto the judge scoping list inevaluation.mdexcluded_entity_labelssection and corrected thedata_summarysoft-exclusion claim inchoosing-a-strategy.mdexcluded_entity_labelsas the recommended fix for systematically noisy label types introubleshooting.mdskills/anonymizer/SKILL.mdwith denylist behavior and precedence ruleType of Change
Contributor Checklist
fix: handle empty entity listskills/anonymizer/SKILL.mdupdated if neededValidation
Documentation and Artifacts
make docs-buildpasses locallymake convert-notebookscloses issue #164