From 95131daf15eb53a8664a07fecdde6f5d496537d7 Mon Sep 17 00:00:00 2001 From: Saverio Mazza Date: Fri, 2 Oct 2026 08:18:54 +0200 Subject: [PATCH 1/2] fix(bench): restore one-to-one matching and value-safe diagnostics --- CHANGELOG.md | 6 +++ HANDOVER.md | 23 ++++++++++ src/pseudonymize/bench.py | 19 ++++++--- tests/unit/test_bench.py | 90 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 132 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8600206..f4e3057 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes follow Keep a Changelog and Semantic Versioning. ## [Unreleased] +### Fixed + +- Correct one-to-one match accounting in the local benchmark, including empty and + disjoint inputs. Explanations now report offsets without copying source values. +- Keep optional ML imports and root logging configuration out of benchmark imports. + ## [1.34.0] - 2026-09-30 ### Added diff --git a/HANDOVER.md b/HANDOVER.md index 091c165..c589bcd 100644 --- a/HANDOVER.md +++ b/HANDOVER.md @@ -399,3 +399,26 @@ verification unless they are intentionally retained as a user-approved release a - Push only when explicitly requested. - A version is published only after its matching tag, successful release workflow, PyPI artifact, and GitHub release exist. Until then, keep changes under `[Unreleased]`. + + +## Local benchmark accounting review (2026-10-02) + +`pseudonymize.bench._match` updated `matched_truth` outside the acceptance loop. +This produced duplicate detection matches, undercounted true positives, and an +UnboundLocalError for empty/disjoint inputs. Six new cases failed before the fix. +The correction restores one-to-one matching; explanation logs now contain row, +type and offsets instead of source values. Optional ML imports are lazy and the +root logger is configured only by the CLI entry point. + +Observed verification using the frozen lockfile: + +- Local benchmark and evaluator tests: 13 passed, including exact expected + aggregate metrics for a corpus with true positives, false positives and misses. +- Suite excluding ONNX/BIO modules with `--no-cov`: 496 passed; 1 Tesseract skip. +- Pre-commit, mypy, strict docs build, package build and release verifier passed. +- Full coverage and supported-platform verification are pending CI. + +This repairs the local benchmark harness; it does not improve detection accuracy +or revise the separate published evaluation baseline. Reusable lesson assessment: +assert match cardinality and zero-match cases instead of only exercising the +benchmark command. Those invariants are now covered by regression tests. diff --git a/src/pseudonymize/bench.py b/src/pseudonymize/bench.py index 047c745..24a7b97 100644 --- a/src/pseudonymize/bench.py +++ b/src/pseudonymize/bench.py @@ -4,14 +4,12 @@ from pathlib import Path from typing import Any -from pseudonymize.backends.ml.onnx import LocalONNXPIIBackend from pseudonymize.engine import Pseudonymizer from pseudonymize.result import EntityType # We duplicate the minimum necessary parts from evaluate_quality.py # to make it completely stand-alone and zero-dependency (other than what's already in core). -logging.basicConfig(level=logging.INFO, format="%(asctime)s - %(levelname)s - %(message)s") logger = logging.getLogger("pseudonymize.bench") SUPPORTED_LABELS = { @@ -50,7 +48,7 @@ def _match( if detection_index in matched_detections or truth_index in matched_truth: continue matched_detections.add(detection_index) - matched_truth.add(truth_index) + matched_truth.add(truth_index) return matched_detections, matched_truth @@ -78,6 +76,8 @@ def evaluate( engine = Pseudonymizer() if use_ml: + from pseudonymize.backends.ml.onnx import LocalONNXPIIBackend + CACHE_DIR = Path(".cache/pseudonymize-tests/models/multilang-pii-ner-ml") onnx_model_path = CACHE_DIR / "model_int8.onnx" tokenizer_path = CACHE_DIR / "tokenizer.json" @@ -135,7 +135,7 @@ def evaluate( entity_type = EntityType[label] fn_per_type[entity_type] += 1 if explain: - logger.info(f"FN: '{text[g_start:g_end]}' should be {label}") + logger.info("FN: row=%d type=%s start=%d end=%d", count, label, g_start, g_end) # Unmatched Detections (False Positives) for index, (d_start, d_end, entity_type) in enumerate(detections): @@ -145,7 +145,13 @@ def evaluate( fp_per_type[entity_type] += 1 false_positives += 1 if explain: - logger.info(f"FP: '{text[d_start:d_end]}' as {entity_type.value}") + logger.info( + "FP: row=%d type=%s start=%d end=%d", + count, + entity_type.value, + d_start, + d_end, + ) count += 1 @@ -195,6 +201,7 @@ def evaluate( def main() -> None: + logging.basicConfig(level=logging.INFO, format="%(asctime)s - %(levelname)s - %(message)s") parser = argparse.ArgumentParser( description="Local evaluation harness for pseudonymize quality testing." ) @@ -210,7 +217,7 @@ def main() -> None: parser.add_argument( "--explain", action="store_true", - help="Log details of all False Positives and False Negatives.", + help="Log row numbers, entity types and offsets for errors without source values.", ) args = parser.parse_args() diff --git a/tests/unit/test_bench.py b/tests/unit/test_bench.py index bdcf468..a7fca49 100644 --- a/tests/unit/test_bench.py +++ b/tests/unit/test_bench.py @@ -27,3 +27,93 @@ def test_bench_main(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: ) monkeypatch.setattr("sys.argv", ["pseudonymize.bench", str(sample_file), "--ml"]) main() + + +@pytest.mark.parametrize( + "detections,truth", + [ + ([], []), + ([(0, 3, "EMAIL")], []), + ([], [(0, 3, "EMAIL")]), + ([(0, 3, "EMAIL")], [(4, 7, "EMAIL")]), + ], +) +def test_match_empty_and_disjoint_spans( + detections: list[tuple[int, int, str]], truth: list[tuple[int, int, str]] +) -> None: + from pseudonymize.bench import _match + from pseudonymize.result import EntityType + + assert _match( + [(start, end, EntityType(kind)) for start, end, kind in detections], truth, True + ) == (set(), set()) + + +def test_match_is_one_to_one() -> None: + from pseudonymize.bench import _match + from pseudonymize.result import EntityType + + assert _match( + [(0, 8, EntityType.EMAIL), (1, 7, EntityType.EMAIL)], [(0, 8, "EMAIL")], True + ) == ({0}, {0}) + assert _match([(0, 8, EntityType.EMAIL)], [(0, 8, "EMAIL"), (1, 7, "EMAIL")], True) == ( + {0}, + {0}, + ) + assert _match( + [(0, 3, EntityType.EMAIL), (5, 8, EntityType.PHONE)], + [(0, 3, "EMAIL"), (5, 8, "PHONE")], + True, + ) == ({0, 1}, {0, 1}) + assert _match([(0, 3, EntityType.EMAIL)], [(0, 3, "PHONE")], True) == (set(), set()) + assert _match([(0, 3, EntityType.EMAIL)], [(0, 3, "PHONE")], False) == ({0}, {0}) + + +def test_benchmark_metrics_and_explanations( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + import json + import logging + + email = "synthetic@example.com" + missing = "Syntheticname" + rows = [ + {"source_text": email, "privacy_mask": [{"start": 0, "end": len(email), "label": "EMAIL"}]}, + {"source_text": email, "privacy_mask": []}, + { + "source_text": missing, + "privacy_mask": [{"start": 0, "end": len(missing), "label": "PERSON"}], + }, + {"source_text": "nothing sensitive here", "privacy_mask": []}, + ] + sample = tmp_path / "corpus.jsonl" + sample.write_text("\n".join(json.dumps(row) for row in rows), encoding="utf-8") + with caplog.at_level(logging.INFO, logger="pseudonymize.bench"): + evaluate(sample, explain=True) + assert "True Positives: 1" in caplog.text + assert "False Positives: 1" in caplog.text + assert "False Negatives: 1" in caplog.text + assert "Precision: 0.5000" in caplog.text + assert "Recall: 0.5000" in caplog.text + assert "F1 Score: 0.5000" in caplog.text + assert email not in caplog.text + assert missing not in caplog.text + + +def test_import_does_not_load_optional_ml_or_configure_logging() -> None: + import subprocess + import sys + + subprocess.run( + [ + sys.executable, + "-c", + "import logging, sys; before = list(logging.getLogger().handlers); " + "import pseudonymize.bench; " + "assert logging.getLogger().handlers == before; " + "assert not {'onnxruntime', 'tokenizers', 'numpy'} & sys.modules.keys()", + ], + check=True, + capture_output=True, + timeout=30, + ) From c9d43edf95dc174567ed16c45c916b8f7396bea7 Mon Sep 17 00:00:00 2001 From: Paolo Mazza Date: Fri, 9 Oct 2026 11:58:06 +0000 Subject: [PATCH 2/2] fix: verify and audit the optional service extra --- CHANGELOG.md | 4 ++++ scripts/audit_extras.py | 2 +- scripts/audit_install.py | 4 ++++ scripts/verify_release.py | 4 +++- tests/integration/test_release_verifier.py | 13 +++++++++++++ 5 files changed, 25 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 93ec177..979c70b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ All notable changes follow Keep a Changelog and Semantic Versioning. - Optional bounded ONNX case recovery for lowercase PERSON candidates using high-confidence re-inference and constrained BIO decoding. Disabled by default; no name dictionaries or fabricated spans. - Python-owned Lambda HTTP service with verified model artifacts, required gateway authentication, ONNX-only person detection, and explicit inference failure. Hosted processing disables coreference expansion so each name occurrence requires model confirmation. +### Fixed +- Include the hosted-service extra in release verification and isolated installation + audits, while keeping service frameworks out of the dependency-free base import. + ## [1.36.0] - 2026-10-09 ### Added diff --git a/scripts/audit_extras.py b/scripts/audit_extras.py index 4c2cf20..7f8d028 100644 --- a/scripts/audit_extras.py +++ b/scripts/audit_extras.py @@ -6,7 +6,7 @@ import tempfile from pathlib import Path -DOCUMENTED_EXTRAS = ("html", "ml", "ocr", "office", "pdf", "remote") +DOCUMENTED_EXTRAS = ("html", "ml", "ocr", "office", "pdf", "remote", "service") def audit_extras(wheel_path: Path, python_executable: str | None = None) -> None: diff --git a/scripts/audit_install.py b/scripts/audit_install.py index c51b2c7..41fe382 100644 --- a/scripts/audit_install.py +++ b/scripts/audit_install.py @@ -11,6 +11,9 @@ "aiohttp", "docling", "docx", + "fastapi", + "pydantic", + "uvicorn", "httpx", "onnxruntime", "openpyxl", @@ -19,6 +22,7 @@ } EXPECTED_BASE_REQUIREMENTS: frozenset[str] = frozenset() EXTRA_CHECKS: dict[str, tuple[str, frozenset[str]]] = { + "service": ("pseudonymize.service", frozenset({"fastapi", "pydantic", "uvicorn"})), "remote": ("pseudonymize.backends.remote", frozenset({"httpx"})), "html": ("pseudonymize.html_xml", frozenset()), "office": ("pseudonymize.inspection.office", frozenset({"docx", "openpyxl"})), diff --git a/scripts/verify_release.py b/scripts/verify_release.py index 8b21d44..847f1f8 100644 --- a/scripts/verify_release.py +++ b/scripts/verify_release.py @@ -23,7 +23,9 @@ } EXPECTED_DEVELOPMENT_CLASSIFIER = "Development Status :: 5 - Production/Stable" EXPECTED_BASE_REQUIREMENTS: frozenset[str] = frozenset() -EXPECTED_EXTRAS: frozenset[str] = frozenset({"ml", "office", "pdf", "ocr", "remote", "html"}) +EXPECTED_EXTRAS: frozenset[str] = frozenset( + {"ml", "office", "pdf", "ocr", "remote", "html", "service"} +) REQUIRED_SDIST_FILES = frozenset( { "CHANGELOG.md", diff --git a/tests/integration/test_release_verifier.py b/tests/integration/test_release_verifier.py index 19aa4c3..5647aba 100644 --- a/tests/integration/test_release_verifier.py +++ b/tests/integration/test_release_verifier.py @@ -1,6 +1,7 @@ import email.message import io import tarfile +import tomllib import zipfile from pathlib import Path @@ -171,3 +172,15 @@ def test_verify_quality_gate_report_missing_file(tmp_path: Path) -> None: missing = tmp_path / "nonexistent.json" with pytest.raises(ValueError, match="quality gate report not found"): verify_quality_gate_report(missing) + + +def test_declared_extras_have_release_and_install_audits() -> None: + from scripts.audit_extras import DOCUMENTED_EXTRAS + from scripts.audit_install import EXTRA_CHECKS + + project_file = Path(__file__).resolve().parents[2] / "pyproject.toml" + with project_file.open("rb") as stream: + declared = set(tomllib.load(stream)["project"]["optional-dependencies"]) + assert declared == EXPECTED_EXTRAS + assert declared == set(DOCUMENTED_EXTRAS) + assert declared == set(EXTRA_CHECKS)