Skip to content

Phase 25: Advanced De-identification - #47

Merged
jas88 merged 20 commits into
mainfrom
phase-25-advanced-deidentification
Feb 6, 2026
Merged

jas88 merged 20 commits into
mainfrom
phase-25-advanced-deidentification

Conversation

@jas88

@jas88 jas88 commented Feb 6, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Add UidReferenceWalker for recursive VR=UI traversal across all sequence depths, with multi-valued UID support and consistent remapping
  • Add native Tesseract C wrapper with stub compilation mode, Zig build integration, and managed P/Invoke layer (LibraryImport + DllImport dual support)
  • Add OcrScanner for burned-in PHI detection in DICOM pixel data (8-bit, 16-bit windowing, RGB, MONOCHROME1 inversion, compressed data decompression via CodecRegistry)
  • Integrate both features into DicomDeidentifier pipeline via WithUidReferenceWalking() and WithOcrScanner() builder methods
  • Pipeline order: OCR scan -> primary de-identification -> date shifting -> UID reference walking -> markers
  • 50 new tests across 4 test files (total: 2263 tests, 0 failures)

Test plan

  • All 18 UidReferenceWalker unit tests pass (core remapping, 10-level deep nesting, multi-valued UIDs, RT/SR patterns, consistency)
  • All 14 OcrScannerOptions tests pass (defaults, allowlist content)
  • All 9 OcrScanner tests pass (fail-fast without Tesseract, result conversion, filtering, edge regions)
  • All 9 integration tests pass (pipeline, backward compatibility, cross-file consistency)
  • Full suite: 2263 total (2209 pass, 54 skipped, 0 failed)
  • Zero build warnings
  • Phase verification: 14/14 must-haves confirmed against codebase

High-level PR Summary

This PR delivers Phase 25: Advanced De-identification, adding OCR-based burned-in PHI detection and comprehensive UID reference walking to the DICOM de-identification pipeline. The UidReferenceWalker recursively remaps all VR=UI elements across sequences at unlimited depth, handling multi-valued UIDs per-component while preserving standard DICOM UIDs. A native Tesseract C wrapper with stub compilation mode integrates via P/Invoke (LibraryImport for NET7+, DllImport for netstandard2.0), exposing OCR text detection capabilities. The OcrScanner class detects burned-in text in 8-bit, 16-bit windowed, RGB, and MONOCHROME1-inverted pixel data, decompresses encapsulated frames via CodecRegistry, and applies dual-threshold confidence filtering with allow/deny lists for non-PHI text (orientation markers, units, abbreviations). Both features integrate into DicomDeidentifier as opt-in builder methods (WithUidReferenceWalking(), WithOcrScanner()), with pipeline order: OCR scan → primary de-identification → date shifting → UID reference walking → markers. 50 new tests across 4 test files bring the total to 2263 tests (2209 pass, 54 skipped, 0 failures), with comprehensive coverage of recursive traversal, OCR configuration, pixel format handling, and cross-file UID consistency.

⏱️ Estimated Review Time: 3+ hours

💡 Review Order Suggestion
Order File Path
1 .planning/ROADMAP.md
2 .planning/STATE.md
3 .planning/phases/25-advanced-deidentification/25-01-PLAN.md
4 .planning/phases/25-advanced-deidentification/25-02-PLAN.md
5 .planning/phases/25-advanced-deidentification/25-03-PLAN.md
6 .planning/phases/25-advanced-deidentification/25-04-PLAN.md
7 .planning/phases/25-advanced-deidentification/25-01-SUMMARY.md
8 .planning/phases/25-advanced-deidentification/25-02-SUMMARY.md
9 .planning/phases/25-advanced-deidentification/25-03-SUMMARY.md
10 .planning/phases/25-advanced-deidentification/25-04-SUMMARY.md
11 .planning/phases/25-advanced-deidentification/25-CONTEXT.md
12 .planning/phases/25-advanced-deidentification/25-RESEARCH.md
13 .planning/phases/25-advanced-deidentification/25-VERIFICATION.md
14 native/src/tesseract_wrapper.h
15 native/src/tesseract_wrapper.c
16 native/build.zig
17 native/src/sharpdicom_codecs.c
18 native/src/sharpdicom_codecs.h
19 src/SharpDicom.Codecs/Interop/TesseractNativeMethods.cs
20 src/SharpDicom.Codecs/Interop/TesseractHandle.cs
21 src/SharpDicom.Codecs/Interop/NativeMethods.cs
22 src/SharpDicom/Deidentification/UidRemapResult.cs
23 src/SharpDicom/Deidentification/UidReferenceWalker.cs
24 src/SharpDicom/Deidentification/OcrScannerOptions.cs
25 src/SharpDicom/Deidentification/OcrScanResult.cs
26 src/SharpDicom/Deidentification/OcrScanner.cs
27 src/SharpDicom/Deidentification/DeidentificationResult.cs
28 src/SharpDicom/Deidentification/DicomDeidentifierBuilder.cs
29 src/SharpDicom/Deidentification/DicomDeidentifier.cs
30 tests/SharpDicom.Tests/Deidentification/UidReferenceWalkerTests.cs
31 tests/SharpDicom.Tests/Deidentification/OcrScannerOptionsTests.cs
32 tests/SharpDicom.Tests/Deidentification/OcrScannerTests.cs
33 tests/SharpDicom.Tests/Deidentification/AdvancedDeidentificationIntegrationTests.cs

Need help? Join our Discord


Summary by cubic

Adds OCR-based burned-in PHI detection and a recursive UID reference walker to the de-identification pipeline to improve PHI coverage and keep cross-object UID references consistent.

  • New Features

    • UidReferenceWalker: remaps all VR=UI elements at any depth (handles multi-valued UIDs; preserves 1.2.840.10008.*); enable with WithUidReferenceWalking().
    • OcrScanner: Tesseract-based text detection for 8-bit, 16-bit (windowed), RGB (grayscale), MONOCHROME1 (inversion), with optional decompression and allow/deny filtering; scans all frames; enable with WithOcrScanner().
    • Native layer: Tesseract C wrapper with stub mode, Zig build wiring, SafeHandle and P/Invoke; feature bit exposed via NativeFeatures.
    • Pipeline: when enabled runs OCR -> primary de-id -> date shift -> UID walking -> markers; DeidentificationResult now tracks OCR and UID-walk stats.
    • Tests: 50 new tests for UID walking, OCR options, OCR behavior, and pipeline integration.
  • Migration

    • No breaking changes; both features are opt-in via DicomDeidentifierBuilder.
    • To use OCR, deploy a native build compiled with Tesseract; without it, OcrScanner is unavailable and should not be enabled.

Written for commit 5836366. Summary will update on new commits.

jas88 added 18 commits February 6, 2026 09:13
Phase 25: Advanced De-identification
- Implementation decisions documented
- Phase boundary established
Phase 25: Advanced De-identification
- Tesseract C API surface documented (capi.h functions, PSM modes)
- Leptonica dependency chain verified (libpng + zlib required)
- Zig cross-compilation risk flagged for macOS targets
- VR-based UID reference walking pattern documented
- Existing Phase 14 integration points catalogued
- Common pitfalls identified (16-bit conversion, MONOCHROME1 inversion, memory management)
Phase 25: Advanced De-identification
- 4 plans in 3 waves
- 2 parallel (Wave 1), 1 sequential (Wave 2), 1 sequential (Wave 3)
- Ready for execution
Address 5 blockers and 3 warnings:
- Clarify ROADMAP should-haves as deferred per CONTEXT.md
- Add zig build verification command to Plan 25-02 Task 2
- Wire tesseract_wrapper.h into sharpdicom_codecs.c
- Implement compressed pixel data decompression via CodecRegistry
- Add 25-02 to Plan 25-04 depends_on
- Split Plan 25-03 Task 2 into 2a (OcrScanner) and 2b (pipeline)
- Add multi-valued UID backslash splitting to Plan 25-01
- Establish definitive pipeline order across Plans 25-01 and 25-03
- UidReferenceWalker recursively walks all VR=UI elements at unlimited depth
- Handles multi-valued UIDs (backslash-separated) per component
- Preserves standard DICOM UIDs (1.2.840.10008.*)
- UidRemapResult provides traversal statistics and remapped tag list
- tesseract_wrapper.h: API declarations for 10 wrapper functions
- tesseract_wrapper.c: full Tesseract C API wrapper and stub mode
- TessDetectionResult struct for word-level OCR results
- Stub mode when SHARPDICOM_WITH_TESSERACT not defined
- Follows existing wrapper patterns (jpeg_wrapper, jls_wrapper)
…line

- Add walkAllUidReferences parameter to DicomDeidentifier constructor
- Pipeline order: ProcessDataset -> date shift -> UID reference walk -> markers
- Add WithUidReferenceWalking() to DicomDeidentifierBuilder for opt-in
- Add UidReferencesRemapped to DeidentificationSummary for tracking
- Fully backward-compatible: existing callers unaffected
Tasks completed: 2/2
- UidReferenceWalker and UidRemapResult types
- DicomDeidentifier and Builder pipeline integration

SUMMARY: .planning/phases/25-advanced-deidentification/25-01-SUMMARY.md
…aged P/Invoke layer

- build.zig: add tesseract_wrapper.c to all build targets (stub mode)
- sharpdicom_codecs.h: add SHARPDICOM_HAS_TESSERACT feature constant
- sharpdicom_codecs.c: include tesseract_wrapper.h, add Tesseract feature flag
- TesseractNativeMethods.cs: P/Invoke for 10 functions (LibraryImport + DllImport)
- TesseractHandle.cs: SafeHandle for TessBaseAPI lifecycle management
- NativeMethods.cs: add Tesseract flag to NativeFeatures enum
- TessDetection struct for managed detection results
Tasks completed: 2/2
- Task 1: Native Tesseract C wrapper (tesseract_wrapper.h/c)
- Task 2: Zig build integration, sharpdicom_codecs wiring, managed P/Invoke layer

SUMMARY: .planning/phases/25-advanced-deidentification/25-02-SUMMARY.md
- OcrScanModality flags enum for risk-based modality filtering
- OcrScannerOptions with confidence thresholds, edge zones, allow/deny lists
- DefaultNonPhiAllowlist with orientation markers, units, medical abbreviations
- OcrDetection readonly record struct with text, confidence, bounding box
- OcrScanResult with filtered detections and ToRedactionRegions() converter
- OcrScanner sealed class with Tesseract P/Invoke interop
- Handles 8-bit, 16-bit, and RGB pixel data with MONOCHROME1 inversion
- 16-bit windowed to 8-bit using Window Center/Width or full-range normalisation
- RGB converted to grayscale via ITU-R BT.601 luminance formula
- Compressed pixel data decompressed via CodecRegistry when DecompressForOcr is true
- Edge/center dual-threshold confidence filtering
- Allow/deny list filtering for non-PHI text (orientation markers, units)
- Fail-fast with clear exception when Tesseract native library unavailable
- Multi-frame scanning processes every frame
- DicomDeidentifierBuilder.WithOcrScanner() enables OCR in the pipeline
- DicomDeidentifier accepts OcrScannerOptions, lazy-creates OcrScanner
- Pipeline order: OCR scan -> ProcessDataset -> date shifting -> UID walking -> markers
- OCR runs BEFORE primary de-id to preserve metadata needed for scanning
- DeidentificationSummary extended with OCR statistics
- OcrScanner disposed in DicomDeidentifier.Dispose()
- Full backward compatibility: callers not using WithOcrScanner() are unaffected
Tasks completed: 3/3
- OcrScannerOptions and OcrScanResult types
- OcrScanner standalone implementation
- DicomDeidentifier pipeline integration for OCR scanning

SUMMARY: .planning/phases/25-advanced-deidentification/25-03-SUMMARY.md
- 18 tests covering core remapping, sequence traversal, consistency,
  RT/SR reference patterns, and result type verification
- Tests verify recursive VR=UI traversal at arbitrary depth (10 levels)
- Multi-valued UID per-component remapping validated
- Standard DICOM UIDs (1.2.840.10008.*) proven never remapped
- 14 OcrScannerOptions tests: defaults, allowlist content, case sensitivity
- 10 OcrScanner tests: constructor fail-fast, result conversion, filtering
- 9 integration tests: pipeline with/without UID walking, OCR unavailability,
  multi-valued UIDs, standard UID preservation, cross-file consistency
- All tests work without Tesseract native library installed
Tasks completed: 2/2
- UidReferenceWalker unit tests (18 tests)
- OCR scanner, options, and integration tests (32 tests)

SUMMARY: .planning/phases/25-advanced-deidentification/25-04-SUMMARY.md

@recurseml recurseml Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review by RecurseML

🔍 Review performed on 17d7aa8..76a9b09

✨ No bugs found, your code is sparkling clean

✅ Files analyzed, no issues (33)

• .planning/ROADMAP.md
• .planning/STATE.md
• .planning/phases/25-advanced-deidentification/25-01-PLAN.md
• .planning/phases/25-advanced-deidentification/25-01-SUMMARY.md
• .planning/phases/25-advanced-deidentification/25-02-PLAN.md
• .planning/phases/25-advanced-deidentification/25-02-SUMMARY.md
• .planning/phases/25-advanced-deidentification/25-03-PLAN.md
• .planning/phases/25-advanced-deidentification/25-03-SUMMARY.md
• .planning/phases/25-advanced-deidentification/25-04-PLAN.md
• .planning/phases/25-advanced-deidentification/25-04-SUMMARY.md
• .planning/phases/25-advanced-deidentification/25-CONTEXT.md
• .planning/phases/25-advanced-deidentification/25-RESEARCH.md
• .planning/phases/25-advanced-deidentification/25-VERIFICATION.md
• native/build.zig
• native/src/sharpdicom_codecs.c
• native/src/sharpdicom_codecs.h
• native/src/tesseract_wrapper.c
• native/src/tesseract_wrapper.h
• src/SharpDicom.Codecs/Interop/NativeMethods.cs
• src/SharpDicom.Codecs/Interop/TesseractHandle.cs
• src/SharpDicom.Codecs/Interop/TesseractNativeMethods.cs
• src/SharpDicom/Deidentification/DeidentificationResult.cs
• src/SharpDicom/Deidentification/DicomDeidentifier.cs
• src/SharpDicom/Deidentification/DicomDeidentifierBuilder.cs
• src/SharpDicom/Deidentification/OcrScanResult.cs
• src/SharpDicom/Deidentification/OcrScanner.cs
• src/SharpDicom/Deidentification/OcrScannerOptions.cs
• src/SharpDicom/Deidentification/UidReferenceWalker.cs
• src/SharpDicom/Deidentification/UidRemapResult.cs
• tests/SharpDicom.Tests/Deidentification/AdvancedDeidentificationIntegrationTests.cs
• tests/SharpDicom.Tests/Deidentification/OcrScannerOptionsTests.cs
• tests/SharpDicom.Tests/Deidentification/OcrScannerTests.cs
• tests/SharpDicom.Tests/Deidentification/UidReferenceWalkerTests.cs

@codecov

codecov Bot commented Feb 6, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 25.75188% with 395 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.72%. Comparing base (17d7aa8) to head (5836366).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/SharpDicom/Deidentification/OcrScanner.cs 0.56% 355 Missing ⚠️
...c/SharpDicom/Deidentification/DicomDeidentifier.cs 29.72% 24 Missing and 2 partials ⚠️
.../SharpDicom/Deidentification/UidReferenceWalker.cs 86.44% 4 Missing and 4 partials ⚠️
...rpDicom/Deidentification/DeidentificationResult.cs 50.00% 3 Missing ⚠️
src/SharpDicom/Deidentification/OcrScanResult.cs 94.28% 2 Missing ⚠️
...c/SharpDicom/Deidentification/OcrScannerOptions.cs 96.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #47      +/-   ##
==========================================
- Coverage   65.76%   64.72%   -1.04%     
==========================================
  Files         231      236       +5     
  Lines       18658    19187     +529     
  Branches     3494     3602     +108     
==========================================
+ Hits        12270    12419     +149     
- Misses       5438     5815     +377     
- Partials      950      953       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 33 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="src/SharpDicom/Deidentification/OcrScanner.cs">

<violation number="1" location="src/SharpDicom/Deidentification/OcrScanner.cs:519">
P1: `double.TryParse` without `CultureInfo.InvariantCulture` will misparse DICOM Window Center/Width values on non-US locales, causing incorrect 16-bit to 8-bit windowing. DICOM DS values always use `.` as the decimal separator regardless of locale.</violation>
</file>

<file name="src/SharpDicom.Codecs/Interop/TesseractNativeMethods.cs">

<violation number="1" location="src/SharpDicom.Codecs/Interop/TesseractNativeMethods.cs:171">
P2: The netstandard/legacy P/Invoke for `tess_init` marshals strings as ANSI (`LPStr`), which will corrupt non‑ASCII tessdata paths on Windows and is inconsistent with the NET7 UTF‑8 marshalling. Use UTF‑8 marshalling (e.g., `LPUTF8Str` or manual UTF‑8 byte conversion) to avoid initialization failures.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread src/SharpDicom/Deidentification/OcrScanner.cs Outdated
Comment thread src/SharpDicom.Codecs/Interop/TesseractNativeMethods.cs Outdated
jas88 added 2 commits February 6, 2026 10:51
Phase 26: Migration Tooling
- Implementation decisions documented
- Phase boundary established
- Use CultureInfo.InvariantCulture for double.TryParse of Window
  Center/Width DS values (DICOM always uses '.' decimal separator)
- Change tess_init P/Invoke from LPStr (ANSI) to LPUTF8Str (value 48)
  for non-ASCII tessdata paths, matching the NET7+ LibraryImport
@jas88
jas88 merged commit bfcb687 into main Feb 6, 2026
4 of 6 checks passed
@jas88
jas88 deleted the phase-25-advanced-deidentification branch February 6, 2026 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant