perf: O(n log n) entity merge + single-pass pseudonymize - #8
Draft
comppaz wants to merge 20 commits into
Draft
Conversation
…ehavior Pin the observable behavior of the two masking hot loops before touching them, so the upcoming rewrites cannot change semantics unnoticed: - `_merge_entities`: the (start, -length) sort order, dual-type overlaps kept as-is, in-place replacement of the accepted slot (the returned list is therefore not sorted by start), chains resolving against whatever currently occupies the slot rather than against the union of all losers, and zero-length spans never satisfying the overlap test. - `pseudonymize`: pseudonyms are assigned back-to-front, so the counter numbering depends on the descending replacement order. A recording mapper pins the exact get_or_create call sequence. - The latent corruption bug: two overlapping entities of different types splice a half-eaten placeholder into the output and mint a phantom mapping. The test asserts the mangled string and says so in its docstring — it documents a bug, not desired behavior. No implementation changes.
`_merge_entities` rescanned the whole accepted list for every candidate, which is O(n²) — noticeable on documents where three detectors each return hundreds of spans, and it runs on the caller's event loop. Candidates are already processed in start order, so the accepted spans of any one type are pairwise disjoint and ordered: once a second span of a type is accepted, the earlier ones end at or before its start and can never overlap a later candidate again. Only the most recently accepted span per type can match, so the rescan collapses into a per-type "open slot" lookup and the sort dominates. Zero-length spans are excluded from the open slot: they satisfy neither half of the overlap test, so they neither get replaced nor shadow the preceding span of their type. Without that exclusion the sweep diverges from the original on inputs containing empty spans (4 of the 5 equivalence tests catch it). No semantic change. `tests/test_merge_equivalence.py` keeps the pre-0.1.3 implementation as an oracle and asserts exact list equality across 10 000 randomized entity sets — mixed types, single-type overlap chains, dual-type duplicates, zero-length spans, and identical spans from several sources.
Substitution replaced entities back to front, rebuilding the whole string
once per entity: O(entities × len(text)), and it corrupted overlapping
entities.
Overlaps are not hypothetical — `_merge_entities` deliberately keeps
dual-type annotations (rule 4), so a PERSON and a LOCATION span can cover
the same characters. Replacing the later one first left the earlier one's
offsets pointing into already-substituted text, which spliced a pseudonym
into the middle of another pseudonym ("<<PERSON_1>>ATION_1>>") and minted a
mapping for a value that never reached the output.
The builder now walks the entities in start order once, emitting
text[cursor:start] + pseudonym and joining at the end, and clips: an entity
starting inside the consumed span is skipped whole. Skipped means skipped —
get_or_create is not called for it, so no phantom mappings.
This commit is deliberately self-contained, CHANGELOG entry included, so it
can be dropped on its own if the clip semantics are not wanted; the two
performance commits do not depend on it.
Behavior for non-overlapping entities is byte-identical, pseudonym numbering
included: pseudonyms are still minted back-to-front because the mapper's
counters are order-sensitive and callers persist the mapping.
`tests/test_pseudonymize_equivalence.py` keeps the pre-0.1.3 implementation
as an oracle and compares output plus full mapper state over 2 500
randomized non-overlapping cases.
The library is used in-process by an async gateway, so every millisecond `detect` spends merging is a millisecond that gateway's event loop is not serving other requests. Above 64 entities the overlap resolution and the PERSON false-positive filtering move to a worker thread via asyncio.to_thread; below it they stay inline, because the thread hop costs more than the work it offloads. The two steps are folded into `_merge_and_validate` so the offload is a single hop. `pseudonymize` stays synchronous — it is O(n) after the single-pass rewrite.
There is no perf harness in the repo, so this is a plain script rather than a CI gate: it times the shipped `_merge_entities` and `pseudonymize` against the pre-0.1.3 implementations on synthetic input and prints a markdown table for PR descriptions. The legacy code is copied in, mirroring the oracles the equivalence tests use. Measured on the development machine (Python 3.12, best of 5): | loop | entities | text | old (ms) | new (ms) | speedup | | --- | ---: | ---: | ---: | ---: | ---: | | _merge_entities | 10 | - | 0.0021 | 0.0015 | 1.4x | | _merge_entities | 100 | - | 0.0790 | 0.0163 | 4.8x | | _merge_entities | 1000 | - | 9.8572 | 0.1875 | 52.6x | | pseudonymize | 10 | 128 | 0.0040 | 0.0046 | 0.9x | | pseudonymize | 100 | 1,208 | 0.0492 | 0.0404 | 1.2x | | pseudonymize | 1000 | 12,008 | 0.8649 | 0.4057 | 2.1x | | pseudonymize | 1000 | 200,000 | 7.1434 | 0.4077 | 17.5x | The pseudonymize rows carry a text size because the old cost was entities × text length: at ten entities on a 128-character string the new builder is 0.6 µs slower (two sorts instead of ten splices of a tiny string), and the gap only ever moves in its favour from there.
The clip's caveat was described as a lost annotation. It is more than that: skipping the overlapping entity means the characters of the losing span that reach beyond the winner's end are emitted verbatim. For PERSON [10, 20) overlapping LOCATION [15, 25) the output now contains text[20:25] unmasked — the old back-to-front splice did consume those characters, at the price of corrupting the placeholder around them. So the change trades "corrupt but fully masked" for "clean but partially unmasked" on overlapping spans, which for a redaction engine is the decision-relevant direction. It is now spelled out in the CHANGELOG, in the `pseudonymize` docstring, and in the characterization test that asserts the "KLMNO" residue, so the drop-or-keep call on the clip commit can be made on the real trade-off. The CHANGELOG also names the better variant that is deliberately NOT shipped here: keep the skip for the mapping decision but pseudonymize the remainder slice text[consumed_to:entity.end] under its own mapping — full coverage, reversible, still one pass — left out because it exceeds this change's reviewed scope. Documentation only: no executable line changed, the engine diff is twelve added docstring lines.
The docstring claimed zero-length spans "satisfy neither half of the overlap test". That is false in the candidate direction: a zero-length span strictly inside the open slot's span — [6, 6) against [5, 10) — satisfies both halves (6 < 10 and 5 < 6), reaches `_pick_winner`, and with the higher score replaces the real span. The nested loop did the same, so the sweep is still equivalent (the 50 %-zero-length property test covers this shape), but the written safety argument was wrong and the "last non-empty span" comment on `open_slot` is violated on exactly that path. The true invariant is one-directional: a zero-length span can never be overlapped ONCE ACCEPTED, because the overlap test needs a candidate starting strictly before its point and every later candidate starts at or after it. That is why it must not become the open slot — otherwise it would shadow the real span in front of it. As a candidate it is fully live. `test_merge_char_zero_length_entity_never_overlaps` overclaimed the same thing while only exercising the equal-start case; renamed to `..._zero_length_span_at_an_accepted_start_shadows_nothing` and narrowed to what its fixture proves, with a new test pinning the candidate direction and the fact that nothing can overlap the zero-length span left in the slot. Documentation and tests only; no executable line changed in the merge.
The clip tests covered a partial overlap and an enclosed span, but not the shape that actually dominates in production: two detectors reporting the IDENTICAL span under different labels. It is the reassuring case and the maintainer's drop-or-keep call needs it stated, because the residual cleartext of a clip is text[winner.end:loser.end] — empty when the spans end together, so an identical-span overlap is fully masked. The test also pins the tie-break: equal starts mean the stable sort keeps the caller's list order, so whichever entity was listed first supplies the label. The test sits in the clip commit's drop radius and says so in its docstring. Also drops "Say the word and it will be added." from the CHANGELOG's not-implemented-alternative note; an offer to the reviewer belongs in the PR description, not in a released changelog. The factual part stays.
comppaz
force-pushed
the
perf/merge-and-pseudonymize
branch
from
August 14, 2026 06:46
254d824 to
23419b1
Compare
comppaz
changed the title
Contributor
Author
Verification: recognition A/B on real model outputsTo confirm the merge rewrite changes nothing about entity recognition beyond the synthetic property proof, an A/B was run on real detector outputs: Setup. All three production detectors constructed as the noirdoc-cloud gateway does (Presidio + spaCy Corpus. 100 real German PII samples (ai4privacy/Faker-derived + LLM-generated contract/letter texts). Result: 709 final entities compared, 0 mismatches. A concatenated 17,346-char composite document (565 filtered entities, Overlap census on the same corpus (relevant to the overlap-handling discussion above): 96/100 samples contained overlapping (dual-type) entities — 170 overlapping pairs total — but only 1 pair was leak-shaped (loser extending beyond the winner's end): PERSON |
Clipping an overlap dropped the losing entity whole, which kept the output clean but emitted the part of that entity reaching past the winner in cleartext: with PERSON [10, 20) overlapping LOCATION [15, 25), text[20:25] survived verbatim. The single forward pass now masks overlaps in pieces. The span that starts first is replaced whole; an entity reaching past it contributes its remainder, text[consumed_to:entity.end], replaced by its own pseudonym under its own entity type. An entity lying entirely inside the consumed span still has no remainder and is skipped whole, mapping included, so identical and enclosed spans mint exactly one pseudonym as before. Chains compose: A [0, 10), B [5, 15), C [8, 20) mask as A whole plus the remainders [10, 15) and [15, 20). A remainder's mapping stores the slice it replaced, not the span it came from, so reidentification restores the input character for character. Remainders are minted in the slot their entity would have had, keeping the back-to-front numbering callers persist. Non-overlapping entities take the same path as before, byte for byte. Also renames the mint loop's shadowing lambda parameter to idx.
Replaces the residual-cleartext caveat: overlapping spans are now masked whole plus remainder, each piece reversible under its own mapping, which supersedes the interim clip behaviour described in the same entry.
…d spans
Masking overlap remainders added a placeholder that nothing outside the
builder could predict, and DOCX/XLSX reconstruction was predicting.
_build_replacements paired entities with placeholders by walking the
entity list and accumulating an offset shift for each entity whose
shifted start happened to land on a "<<". The remainder's placeholder is
invisible to that arithmetic, so from the first overlap onwards every
later entity was read at the wrong offset: its replacement was dropped
(the original stayed in the reconstructed file) or paired with another
entity's placeholder (a reveal would then restore the wrong value). On a
mixed document with one overlapping pair, "Hamburg" was dropped; a
3 000-case randomized sweep found 4 217 missing and 53 wrong pairings,
and 1 556 documents whose replacement map did not reproduce the
pseudonymized text.
The pairing now comes from the substitution that created it.
PseudonymizationEngine.pseudonymize_detailed returns the text plus one
EmittedSpan per placeholder — original text, placeholder, position in
the output; pseudonymize() is unchanged and delegates to it.
pseudonymize_block records both on the FileBlock, and the sdk and
pipeline call sites go through it. Replacements are applied longest
original first, so a short original cannot eat a longer one containing
it before that one is replaced ("Weber" inside "Anna Weber"). A block
carrying entities but no record of the substitution is refused instead
of rewritten from a guess; callers already fall back to converted text.
Remainders are also trimmed before minting: leading and trailing
whitespace is emitted as literal text and only the core is
pseudonymized, and a whitespace-only remainder mints nothing.
Remainders cut at arbitrary offsets, so a slice like " GmbH" would key
the mapper at "gmbh" while storing the leading space, colliding with a
real "GmbH" elsewhere and revealing with the wrong spacing. Trimmed
remainders are shaped like detector spans, which never carry edge
whitespace, so that collision class is unreachable. The mapper's
designed case-insensitive dedup remains and is now pinned by a test.
Adds the DOCX/XLSX replacement regression under Fixed, the additive pseudonymize_detailed / EmittedSpan entry under Added, and the remainder trimming. Qualifies the reversibility claim: exact, up to the mapper's designed case-insensitive dedup, which predates overlap handling and applies to every entity.
…ce the masked text Reconstruction rewrites a DOCX or XLSX by text, not by offset, so an original drawn from the placeholder charset can land inside a placeholder that was already inserted. An ID "12" in a document with twelve people turns <<PERSON_12>> into <<PERSON_<<ID_1>>>>: still masked, but no reverse mapping can undo it, and nothing noticed. The map is now checked before it is used. An original that is a substring of any placeholder is rejected outright — that is the direct signature of the hazard and gets its own log line — and applying the map to the extracted text must reproduce the pseudonymized text. On failure _build_replacements returns None, so the caller takes the fallback it already has and converts the file to text: masking and reveal both survive, only the formatting is lost. Neither log line carries any of the values involved. The randomized sweep cannot see this class: its alphabet is deliberately disjoint from the placeholder charset so that substring assertions stay decidable. The two cases here cover it instead — the repro, and an ordinary document pinning that the guard does not fire.
The pre-check refused any original that occurred inside any placeholder, which includes an original inside its OWN placeholder: an ID "1" mints <<ID_1>>, "1" is in it, and an otherwise ordinary document lost its formatting to the text fallback for nothing. Self-pairing cannot corrupt anything — str.replace makes one left-to-right pass and never rescans what it inserted, and a shorter original's placeholder is not in the text yet while the longer originals are applied. The self-check that follows it is necessary and sufficient on its own: if replacement lands anywhere it should not, the rebuilt text stops matching what the pseudonymizer produced. That covers the "12" inside <<PERSON_12>> case and equally an original replaced at an occurrence the detector never flagged. One check now, one event.
Reconstruction rewrites paragraph runs and string cells. A document has
text that lives in neither, and the extracted text does not say where in
the file its characters came from, so a replacement map can be perfect
and the write still incomplete. Three shapes shipped the original next
to its own placeholder, with every existing check passing:
hyperlink run masked 'Kontakt: <<PERSON_1>>'
shipped 'Kontakt: <<PERSON_1>>Anna Beispiel'
paragraph span masked 'Rechnung von <<ORGANIZATION_1>> in Hamburg'
shipped 'Rechnung von Muster\nGmbH & Co in Hamburg'
numeric cell masked '<<ID_1>>\n<<PERSON_1>>'
shipped '480815\n<<PERSON_1>>'
python-docx counts hyperlink text in Paragraph.text but leaves it out of
Paragraph.runs; the extractor joins paragraphs with a newline, so a
detected span can match the flat text and no paragraph; the XLSX writer
only touches string cells while the extractor stringifies numbers too.
The bytes that were written are now extracted again with the same
extractor and must yield the masked text. On a mismatch the file is
refused and the caller converts it to text, as it already does for the
other refusals: formatting is lost, masking is not. The pre-apply check
on the map stays as a cheap early-out — it catches a map that would
over-reach before any file is parsed — but it no longer claims to cover
what the writers can reach.
A span with an empty original is left out of the map: it replaces no
characters, and str.replace("", …) would splice its placeholder between
every character in the document. A placeholder that replaces nothing has
nothing to anchor it in the file either, so the comparison ignores it —
no PII rides on a mapping whose original is the empty string.
Bytes that come out of reconstruct() are kept on the block, and the
pipeline now counts a file as reconstructed only if there are bytes. A
refused rebuild goes out as text and is counted as a conversion, which
is what the caller actually shipped.
emitted_spans was inserted between pseudonymized_text and reconstructed_bytes, which rebinds the arguments of anyone constructing a FileBlock positionally. Moved to the end, with a note that new fields belong there.
Two claims in the CHANGELOG and the engine docstring said colliding spellings reveal as the one "minted first", glossed as "GMBH after GmbH reads back as GmbH". Minting runs back to front, so the occurrence that mints first is the LAST one in the document — which is what the pinned test asserts. Both surfaces now say that, add the same for the type label (the mapper's key ignores the entity type), and state the guarantee as character-exact reveal for identical spellings. The substitution row of the perf table was measured before remainder masking and the emitted-span record added per-entity work, and it only advertised the win. Re-measured at HEAD with benchmark/bench_hot_loops.py and given as a table: the 200 KB case is 8.7x rather than 17.5x, and below roughly a thousand entities the new path is slower than 0.1.2 — about 2x at ten entities, single-digit microseconds in absolute terms. Also qualifies "every character any entity covers is replaced", which the whitespace trimming two paragraphs above already contradicted.
Only the nested sdd/.gitignore kept review artifacts out of this public repo; a file added anywhere else under .superpowers/ would be swept up by `git add -A`.
Four places said more than the code does. The README promised DOCX and XLSX "round-trip fully". They round-trip their formatting only when the redacted file verifies; a mailto: in a signature block is enough to send a document out as masked plain text instead. The format table and the caveat now say that, in one line: formatting is best-effort, masking is not. The CHANGELOG called the small-input slowdown "single-digit microseconds", which is the ten-entity row only — the hundred-entity row in the same table is about +30 µs. Both are named now, with the crossover. The reconstruction guarantee was stated unconditionally, but it covers what this module writes: the column-aware XLSX path produces its own bytes and reconstruct() hands them back through the short-circuit without verifying them. That is now said in the module docstring and carried into the CHANGELOG as a named follow-up, along with the fact that the generic writer's numeric-cell gap was reachable by calling that writer directly rather than through the SDK or the pipeline. Plus two notes: the early-out runs before the rewrite and the re-parse, not before any parse, and reconstruct() caches its bytes on the block, so editing the masked text afterwards does not rebuild anything.
Contributor
Author
|
Correction to the overlap census in the A/B comment above. The census's replica of the sequential overlap walk did not advance |
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
EnsembleDetector._merge_entitieswas an O(n²) nested loop over all entities from all three detectors; it is now an O(n log n) sort+sweep (ddc5cbe). Detection semantics are unchanged: the sweep is property-tested against the old implementation as an in-test oracle (10,000 seeded randomized cases including same-type overlap chains, dual-type overlaps, zero-length spans, and forced score ties), and the individual detectors (presidio/spaCy, Flair, GLiNER) have zero changes on this branch. Additionally verified end-to-end on real model outputs — see the A/B comment below (709 entities, 100 samples, 0 mismatches).PseudonymizationEngine.pseudonymizerebuilt the full string once per entity — O(entities × text length); it is now a single-pass builder, O(n) (0d771a7). Non-overlapping behavior is byte-identical to the old implementation (property-tested against the oracle including full mapper state; pseudonym mint order preserved under the stable sort).5be58d8, superseding an interim clip): see the next section.EmittedSpan+pseudonymize_detailedAPI (c576f7f) —pseudonymize()'s signature is untouched. Two independent guards fail safe to text conversion (masking and reversibility preserved, formatting dropped, warning logged): a pre-apply self-check that the replacement map reproduces the masked text (5e82b7c,1021ccf), and post-write verification (12d8ff9) — the produced file is re-extracted and its text must equal the masked text, so writer blind spots (hyperlink runs, paragraph-spanning entities, non-string cells) can never ship cleartext with formatting. Formatting is best-effort; masking is not.asyncio.to_threadabove 64 entities (23a8c68). The 64 threshold was calibrated against the old O(n²) merge; post-rewrite a 1000-entity merge is ~0.18 ms, so it is worth retuning once real traffic numbers exist.benchmark/bench_hot_loops.py, re-measured at HEAD): merge ~53× at 1000 entities; pseudonymize ~8.7× on 200 KB text. Honest caveat: at small inputs the new builder is slower — ~+5 µs per call at 10 entities, ~+30 µs at 100, crossover around 1000 entities / large texts. Absolute costs are microseconds against model inference.f90114d) and carry the full before/after trail.Overlap handling
The old splice corrupted output when dual-type spans overlapped (mangled tokens like
<<PERSON_1>>ATION_1>>, characterized inf90114d). Overlaps are now handled in one pass:Reversibility qualifier: the mapper deliberately dedups case-insensitively across all entities (pre-existing behavior). Because minting runs back-to-front, colliding spellings (
GMBH/GmbH) reveal as the last document occurrence's spelling, and the placeholder's type label is likewise the last occurrence's — stated in the docs and pinned by test.Test plan
make check— ruff clean, strict mypy clean, 302 passed (2 skipped, 28 slow deselected)tests/test_merge_equivalence.py)tests/test_pseudonymize_equivalence.py)"1"inside its own<<ID_1>>) reconstructs byte-exact with zero eventsto_threadpath: equal-results test above/below the 64-entity threshold; 32-way concurrent shared-instance run matches serial results