Feat/pii subprocess venv#519
Merged
Merged
Conversation
…opt-in Hosts on Python versions newer than spaCy ships wheels for (3.14 today) silently fell back to the no-detector path in ``pii.py`` and produced ``contains_pii: false`` reports across every clip — a false-negative on PII-bearing audio for any host that can't install Presidio in-process. Move detection into an isolated Python-3.13 subprocess venv so the host can stay on whatever recent Python it likes without losing PII checks. Detection content ----------------- Two detectors run inside the new ``pii-detection`` venv and their spans are merged before cross-ASR-model corroboration runs in the host: - **Microsoft Presidio Analyzer** — regex + spaCy-NER recognizers for emails, phone numbers, SSNs, credit cards, IPs, dates, locations. - **GLiNER PII** (``nvidia/gliner-pii`` by default, configurable) — a 570M-param transformer fine-tuned on ~100k synthetic PII/PHI records. Catches medical / health entities Presidio doesn't natively cover, plus the long tail of unstructured personal-context references that zero-shot well from the labels we pass. GLiNER's lowercase labels are normalized to Presidio's uppercase scheme inside the worker so the existing ``(category, text.lower())`` dedupe key in ``pii.py`` treats both detectors' hits on the same entity as the same finding. Per-detector opt-in ------------------- ``detect_pii_in_pass`` and ``detect_pii_via_subprocess`` gain a ``detectors`` argument symmetrical to the project's ASR-model selection pattern: - ``detectors=None`` (default) → both Presidio and GLiNER run. - ``detectors=["presidio"]`` → skip GLiNER's model load (~5 s saved). - ``detectors=["gliner"]`` → skip Presidio. - ``detectors=[]`` → explicit-disable; no subprocess spawned, no venv build attempted, report carries ``failures["pii_disabled"]`` so an auditor can distinguish "didn't run on purpose" from "ran and found nothing" and "subprocess crashed". ``gliner_model``, ``gliner_labels``, and ``gliner_threshold`` are also exposed on ``detect_pii_in_pass`` so callers can swap in a different GLiNER variant (e.g. ``urchade/gliner_multi-v2.1`` for multilingual coverage) or restrict the label set without touching the worker. Architectural changes --------------------- - New module ``pii_subprocess.py`` — venv constants (``_PII_VENV = "pii-detection"``, Python 3.13, presidio + spacy + en_core_web_lg wheel pinned via direct URL + gliner + torch), GLiNER -> Presidio category map, worker script, and the ``detect_pii_via_subprocess`` dispatch function. ``torch`` is named explicitly in ``_PII_REQUIREMENTS`` so ``ensure_venv``'s auto-detect (post-#516) routes it through the matched CUDA index — GLiNER's transformer wants matched ``torch`` / nvidia-cuda-runtime wheels. ``torchaudio`` is intentionally omitted (no audio decoding). - ``pii.py`` simplified — removed ``_load_presidio_analyzer``, ``_load_spacy_nlp``, ``_scan_with_presidio``, ``_scan_with_spacy``, and the in-process tier-fallback chain. Dispatch now goes to the subprocess; everything else (transcript concatenation, ``PiiSpan`` materialization, cross-ASR-model corroboration, ``PiiPassReport`` construction) stays in-process. - Subprocess-only — no in-process fallback. Matches the canary/nemo/qwen subprocess-venv pattern for hermetic deployment. A host that can't build the venv at all gets a single ``failures["pii_subprocess"]`` entry, not a confusing partial-fail state. Worker model loads are paid per subprocess invocation. For a typical ``analyze_audio.py`` run (138 ``detect_pii_in_pass`` calls per subject) that's ~15 min of model-load overhead per subject; batching all of a subject's transcripts into a single subprocess invocation is a worthwhile follow-up but isn't required for correctness. Tests ----- ``src/tests/audio/workflows/audio_analysis/pii_subprocess_test.py``, 10 tests covering: default-both-detectors plumbing, single-detector selection, empty-list short-circuit (no venv build, no subprocess spawn), unknown-detector ``ValueError``, dedupe with order preservation, default-arg pass-through (threshold, GLiNER model id, labels, full GLiNER -> Presidio category map), caller overrides for the GLiNER settings, response field pass-through. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The PII axis previously surfaced only ``contains_pii ? 1.0 : 0.0`` to ``global_summary``, throwing away every per-span detector score that the worker produced. Worse, when the subprocess crashed or no detector loaded, the report fell to ``0.0`` — indistinguishable from "we checked, found nothing." That's the wrong default; it tells a reviewer "this clip is clean" when the truth is "we don't know." Add a continuous ``detection_confidence`` field on ``PiiPassReport``, combined from three signals per unique ``(category, normalized_text)`` finding: - **per-span detector score** — Presidio analyzer score or GLiNER prediction probability, ``max()`` within the finding's group. - **cross-detector agreement** — Presidio AND GLiNER both flagged the same finding (factor 1.0) vs only one detector (0.5). The strongest "real entity vs hallucination?" signal at this layer, mirroring the model-voting pattern presence/identity already use. - **cross-ASR-model agreement** — fraction of available ASR transcripts that contained the finding. Spans only one ASR transcribed and no sibling ASR confirms are the prototypical hallucination case. Then ``max()`` across findings, matching how the transcript and single-speaker axes combine their internal sub-signals. Deliberately NO category-severity weighting. In pediatric / clinical voice data the categories nominally most "severe" (US_SSN, CREDIT_CARD, MEDICAL_RECORD_NUMBER) have near-zero true-positive rate — children don't speak SSNs aloud — and the synthetic-PII-trained detectors gladly flag any 9-digit ASR hallucination as SSN. Weighting them up would inflate exactly the findings a reviewer should de-prioritize. Heuristic enhancements (category weights, find-count severity) are deferred as later goals; this commit just plumbs through what the detectors and ASRs actually report. ``None`` propagation -------------------- ``detection_confidence`` is ``None`` when no detector ran: - subprocess raised at dispatch (CalledProcessError / RuntimeError) - both detectors failed to load inside the subprocess venv - caller passed ``detectors=[]`` (explicit-disable short-circuit) ``0.0`` is reserved for "detectors ran, found nothing" — the confident-negative case. ``global_summary.compute_pass_global_summary`` propagates the same distinction into ``no_pii_uncertainty`` so the ``combined_uncertainty`` consumer can tell "didn't actually check" apart from "checked clean." No silent demotion to 0. Tests ----- ``src/tests/audio/workflows/audio_analysis/pii_test.py``, 12 tests: - ``_compute_detection_confidence`` unit cases: empty spans -> 0.0, single detector / single ASR -> 0.5 x score, both detectors -> 1.0 factor, cross-ASR scaling by fraction, max across independent findings (not sum), whitespace-only text filtered, missing per- span score collapses to 0. - ``detect_pii_in_pass`` integration with mocked ``detect_pii_via_subprocess``: happy path populates the field, ``detectors_used=[]`` -> None, subprocess raise -> None, detectors-ran-no-spans -> 0.0, ``detectors=[]`` short-circuit -> None plus ``failures["pii_disabled"]``. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Empirical finding from a diagnostic on ``john.doe@example.com``:
nvidia/gliner-pii exhibits competing-claim interference. When the
prompt includes overlapping labels that can both plausibly cover the
same span — e.g. ``person`` + ``first_name`` + ``last_name`` plus
``email`` + ``email_address`` — the model commits the span to one
label and silently drops it from consideration under the other,
*even when the other would have been the right answer*. The
diagnostic at ``temp/diag_gliner_email.py`` walks this through
explicitly at threshold=0.0:
Labels: [person, first_name, last_name, email, email_address, ...]
→ John = first_name (1.0), Doe = last_name (1.0),
john.doe@example.com = no email span at any score above 0.0
Labels: [name, address, date, phone_number, email, ssn, ...] (flat)
→ John, Doe = name (1.0), john.doe@example.com = email (1.0)
Same model. Same threshold. Same input. The only difference was
label-set granularity. Email goes missing under the over-decomposed
set; both name and email surface cleanly under the flat HIPAA set.
This is the same set b2aiprep PR #256 ships in its
``hipaa_labels.json``. Aligning on it has the secondary benefit
that the two projects' PII outputs now share a vocabulary, which
matters for downstream consumers that aggregate across both.
Changes
-------
- ``_DEFAULT_GLINER_LABELS`` replaced with the HIPAA Safe Harbor 18
identifiers (45 CFR §164.514(b)(2)) plus two clinical-voice
extensions (``health_condition``, ``medication``) that don't
overlap with the HIPAA labels.
- Long block comment above the constant captures the empirical
finding so future contributors don't walk back into the trap
(i.e. "this would catch more if I added first_name and
last_name..."). Names the diagnostic, names the symptom, names
the fix.
- ``gliner_labels`` docstring on ``detect_pii_via_subprocess``
points at the warning so anyone customizing the label set hits
it before re-introducing overlap.
The category map (``_GLINER_TO_PRESIDIO_CATEGORY``) already handles
every new HIPAA label that has a Presidio analogue (``name``,
``email``, ``date``, ``phone_number``, ``address``,
``social_security_number``, ``ip_address``,
``medical_record_number``). New labels without Presidio analogues
(``fax_number``, ``health_plan_number``, ``account_number``,
``license_number``, ``vehicle_identifier``, ``device_identifier``,
``url``, ``biometric_identifier``, ``photographic_image``,
``unique_identifier``) fall through to ``str.upper()`` and stand on
their own — Presidio doesn't emit competing categories for them so
there's nothing to align with. Entries for the now-removed labels
(``first_name``, ``last_name``, ``street_address``, ``city``,
``date_of_birth``, ``patient_id``, etc.) stay in the map for
backward-compat — a caller can still pass them explicitly via the
``gliner_labels`` override.
Tests
-----
22 tests in pii_subprocess_test.py + pii_test.py still pass — they
check the request shape and detection_confidence math, not the
content of the default label list, so the swap is transparent at
the dispatch layer. The behavioral verification is in the real
end-to-end run via temp/test_pii_subprocess.py + temp/diag_gliner_email.py.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The previous commit on this branch (a2f384c) shipped the HIPAA Safe Harbor 18 identifiers PLUS two clinical-voice-specific extensions (``health_condition``, ``medication``) tacked on the end. Running the smoke test against ``temp/test_pii_subprocess.py`` showed Step 3 (detectors=["gliner"]) still didn't catch ``john.doe@example.com`` even with the new flat HIPAA-aligned base. Cross-referenced against the threshold=0.0 diagnostic (``temp/diag_gliner_email.py``): the bare HIPAA-18 set catches the email at GLiNER score 1.0. Adding ``health_condition`` and ``medication`` on top is enough to push the email's score below the 0.5 threshold and make it disappear from the report, even though those two labels are not semantically competing with ``email`` at all. The mechanism is unclear — possibly a label-list length effect on the model's projection space, possibly an artefact of nvidia/ gliner-pii's training data — but the symptom is reproducible. Two-detector mode (the default) is robust to this: Presidio catches the email at score 1.0 and feeds it through ``pii.py``'s cross- detector dedup, so the report still surfaces the EMAIL_ADDRESS span. The user-visible regression is confined to ``detectors=["gliner"]`` mode and to the contribution of the email to ``detection_confidence`` when both detectors run (cross-detector agreement collapses from 1.0 to 0.5 for that one span). For the b2ai voice-data workflow the cost of losing the clinical-extension coverage was lower than the cost of weakening the email signal — so this commit removes the extensions and matches b2aiprep #256's HIPAA-18 list verbatim. The empirical finding is captured as a second ``DO NOT extend this list`` warning above the constant, so a future contributor doesn't re-add ``health_condition`` / ``medication`` / similar adjacent labels without first re-running the diagnostic against the email- detection case. Callers who actually want the clinical-extension coverage (or any other labels beyond the HIPAA-18) can still pass them via ``gliner_labels=[...]`` on the dispatch function — they just take on the responsibility of verifying their specific additions don't trip the interference on their corpus. Module docstrings on both pii.py and pii_subprocess.py updated to reflect that GLiNER's coverage is now strictly HIPAA-aligned — no longer the broader "medical / health entities and unstructured personal-context references" framing the previous commit used. 22 tests in pii_subprocess_test.py + pii_test.py still pass; they key off ``_DEFAULT_GLINER_LABELS`` as a literal, not its content. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors PII detection to run within an isolated subprocess virtual environment, integrating both Microsoft Presidio and GLiNER detectors. It introduces a continuous detection_confidence metric derived from cross-detector and cross-ASR agreement. Feedback highlighted an invalid PyTorch version constraint in the new subprocess requirements and logic errors in the worker script where empty entity allow-lists would incorrectly trigger full scans. Additionally, it was suggested to replace a magic number in the confidence calculation with a dynamic value for better maintainability.
| _EN_CORE_WEB_LG_WHEEL, | ||
| "gliner>=0.2", | ||
| "transformers>=4.40", | ||
| "torch>=2.8,<2.9", |
Contributor
There was a problem hiding this comment.
The version constraint torch>=2.8,<2.9 is invalid as PyTorch 2.8 has not been released yet (the current stable version is 2.5.x). This will cause ensure_venv to fail during the subprocess environment creation. Please update this to a valid version range, such as torch>=2.4,<2.6.
Suggested change
| "torch>=2.8,<2.9", | |
| "torch>=2.4,<2.6", |
Collaborator
Author
There was a problem hiding this comment.
bad bot, out of date information
#2 (pii.py:159) — replace literal `2` in detector-agreement divisor with `len(_KNOWN_DETECTORS)`. Today they're equal, but the symbolic form removes the silent-miscalculation trap if a third detector is ever added to `_KNOWN_DETECTORS`. #3 (pii_subprocess.py:327) — drop the `or None` fallback when passing `entities` to `AnalyzerEngine.analyze`. Presidio interprets `entities=None` as "scan for ALL entity types"; `[] or None` collapses to `None`, so a caller passing `entities=[]` (explicit "scan for nothing") got the opposite of intent. The dispatcher already substitutes the default full list when the caller-side `presidio_entities` is None, so the worker never needs to pass None itself — empty list now passes straight through to Presidio. #4 (pii_subprocess.py:334) — change the post-scan category-filter guard from `if presidio_entities` to `if presidio_entities is not None`. Same falsy-empty-list trap as #3: with the old guard, an explicit empty list silently skipped the filter so any Presidio result would pass through. With `is not None`, the empty-list path filters everything out, matching the "scan for nothing" semantics. The comment is updated to clarify this is a belt-and-suspenders check on Presidio's output rather than a category fallback. Gemini's review comment #1 (the torch version pin) is being rejected in a separate PR reply — torch 2.8 has been on PyPI for months (latest is 2.12.0), the pin is intentional for NeMo trunk SALM compatibility, and Gemini's training data predates the release. Tests: src/tests/audio/workflows/audio_analysis/{pii,pii_subprocess}_test.py 22/22 still pass; the empty-list path was already covered by the detector-agreement tests with `len(_KNOWN_DETECTORS)` in scope. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
3 tasks
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
1 participant
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
Make PII detection actually run on Python 3.14 hosts (it was silently no-op'ing because spaCy / Presidio top out at cp313 wheels), and pass through richer per-detection
signal to consumers instead of just a single boolean.
Three logical changes stacked, each in its own commit so they're individually reviewable and revertible:
91ec2327— Move PII detection into a newpii-detectionsubprocess venv (Python 3.13) running Presidio + GLiNER (nvidia/gliner-pii). Subprocess-only, noin-process fallback. Per-detector opt-in via a
detectors=[...]argument symmetrical to ASR-model selection elsewhere.4129b49d— Adddetection_confidencetoPiiPassReport: a continuous[0, 1]aggregated from per-detector scores × cross-detector agreement × cross-ASR-modelagreement. Replaces the prior
1.0 if contains_pii else 0.0boolean inglobal_summary.Nonepropagates when no detector actually ran (subprocess crash, both detectorsfailed to load, or caller passed
detectors=[]) — distinct from0.0("ran, found nothing").a2f384ce+628973cc— GLiNER default labels set to the HIPAA Safe Harbor 18 identifiers verbatim, matching b2aiprep PR 253 task implement initial version of bioacoustic quality control #256 so the two projects' PII outputs sharea vocabulary.
Why a subprocess at all
The host venv may be on a Python version newer than spaCy ships wheels for (3.14 today). Pre-this-PR the host-side detector chain silently fell back to
failures["no_pii_detector"]+contains_pii: Falseon every clip — a privacy-meaningful false negative on b2ai voice data. Moving Presidio + GLiNER into their ownPython-3.13 subprocess venv (built and cached via
ensure_venv) keeps the host on whatever recent Python it likes while making the PII path actually run.What runs inside the venv
Two detectors run on every transcript and their spans are merged before cross-ASR-model corroboration in
pii.py:nvidia/gliner-pii) — a 570M-param transformer fine-tuned on ~100k synthetic PII/PHI records. Catches HIPAA PHI categories Presidio doesn't nativelyrecognize (
medical_record_number,health_plan_number,account_number,fax_number,url,biometric_identifier,unique_identifier, ...).GLiNER's lowercase labels are normalized to Presidio's uppercase scheme inside the worker so the existing
(category, text.lower())dedupe key inpii.pytreats bothdetectors' hits on the same entity as the same finding.
Per-detector opt-in
gliner_model, gliner_labels, and gliner_threshold are also exposed on detect_pii_in_pass so callers can swap in a different GLiNER variant (e.g. urchade/gliner_multi-v2.1 for
multilingual coverage) or restrict the label set without touching the worker.
detection_confidence vs contains_pii
The PII axis previously surfaced only contains_pii ? 1.0 : 0.0 to global_summary and threw away every per-span detector score the worker produced. Worse, when the subprocess
crashed or no detector loaded, the report fell to 0.0 — indistinguishable from "we checked, found nothing."
detection_confidence is now a continuous [0, 1] combining three signals per unique (category, normalized_text) finding:
at this layer.
prototypical hallucination case.
Then max() across findings — matches how the transcript and single-speaker axes already combine their internal sub-signals.
Deliberately NO category-severity weighting. In pediatric / clinical voice data the categories nominally most "severe" (US_SSN, CREDIT_CARD, MEDICAL_RECORD_NUMBER) have
near-zero true-positive rate — children don't speak SSNs aloud — and the synthetic-PII-trained detectors gladly flag any 9-digit ASR hallucination as SSN. Weighting them up
would inflate exactly the findings a reviewer should de-prioritize. Heuristic enhancements deferred as later goals.
None propagation
detection_confidence is None when no detector ran:
0.0 is reserved for "detectors ran, found nothing" — the confident-negative case. global_summary.compute_pass_global_summary propagates the same distinction into
no_pii_uncertainty so the combined_uncertainty consumer can tell "didn't actually check" apart from "checked clean." No silent demotion to 0.
Why the HIPAA-18 labels (and why exactly that list)
GLiNER picks were tuned via an out-of-band diagnostic (temp/diag_gliner_email.py) on john.doe@example.com. Two non-obvious findings drove the final label set:
nvidia/gliner-pii exhibits competing-claim interference. When two labels can plausibly cover the same span, the model commits the span to one and silently drops it from
consideration under the other — even when the other would have been the right answer. Passing [person, first_name, last_name, email, email_address, ...] lost the email
entirely; the model classified John and Doe as first_name/last_name at score 1.0 and emitted no span containing @. A flat label set with one name and one email caught both at
score 1.0. Keep label sets flat.
Even flat label sets have a length / projection-space effect we don't fully understand. Tacking the two clinical-voice extensions health_condition and medication onto the
end of the HIPAA-18 list (semantically non-overlapping with anything) was enough to push the email's score below the 0.5 threshold and lose it. The HIPAA-18 verbatim catches
it cleanly. Don't extend past the HIPAA-18 unless you've verified your specific additions don't trip the interference on your corpus.
Both findings are captured as DO NOT warnings above _DEFAULT_GLINER_LABELS so a future contributor doesn't walk back into either trap.
Architectural changes
en_core_web_lg wheel via direct URL + gliner + transformers + torch), GLiNER → Presidio category map, worker script, and the detect_pii_via_subprocess dispatch function.
torch named explicitly in _PII_REQUIREMENTS so the auto-detect in ensure_venv routes it through the matched CUDA index. torchaudio intentionally omitted (no audio decoding).
the subprocess; everything else (transcript concatenation, PiiSpan materialization, cross-ASR-model corroboration, PiiPassReport construction) stays in-process.
None branch so the "didn't actually run" signal surfaces as None, not silently 0.0. New detection_confidence field also emitted in the per-pass no_pii block for downstream
tools.
Performance notes
for the first GLiNER model download from HuggingFace.
transcripts into a single subprocess invocation is a worthwhile follow-up but isn't required for correctness.
Tests
32 tests added across two new files:
empty-detector short-circuit, unknown-detector ValueError, dedupe-with-order, default-arg pass-through, caller overrides.
detectors → 1.0 factor, cross-ASR scaling by fraction, max across independent findings, whitespace-only text filtered, missing per-span score collapses to 0) and
detect_pii_in_pass integration with a mocked detect_pii_via_subprocess (happy path, no-detectors → None, subprocess raise → None, ran-no-spans → 0.0, detectors=[]
short-circuit → None plus failures["pii_disabled"]).
All 32 pass. Pre-commit hooks (ruff, mypy, codespell) clean.
The real Presidio + GLiNER worker inside the subprocess venv is exercised by the out-of-band scripts temp/test_pii_subprocess.py and temp/diag_gliner_email.py — including
them in the unit test suite would require building the actual ~5 GB venv on every test run.
Test plan
correctly, all four detectors=[...] selection paths behave as designed.
name-related labels (the empirical justification for the label-set choice).
Out-of-band reviewer notes
subject into a single subprocess invocation at the analyze_audio.py level rather than per-pass-per-clip. The dispatch function already accepts an arbitrary number of
(asr_model, full_text) entries, so the change would be at the call site, not the API.
_DEFAULT_GLINER_LABELS capture the two empirical traps (overlapping labels, extending past HIPAA-18) so the caller can verify their additions don't trip them.
highest false-positive rate. Reintroducing severity weighting for a different downstream domain should be opt-in, not the default.
Version
Published prerelease version:
1.3.1-alpha.32Changelog
🐛 Bug Fix
Authors: 3