Architecture and scalability review: current limits and proposed solutions
Companion to #33/#34/#35: a review of the codebase from the angle of scalability, structure, and performance. To be clear upfront: the current architecture is sound for its stated scope (typed core, zero deps, immutable models, sanitized failures), and after #29 the hot path is near-linear. The points below are the limits I can identify beyond that scope, each with a concrete proposal and a rough size (S/M/L). Numbered for easy discussion; none of this is urgent, and some may be explicit non-goals for you.
A. Large inputs and memory
1. Everything is materialized in memory (S to document, L to change).
BuiltinFileAdapter.extract does source.read_bytes() and every adapter builds the full Document before processing; CSV rendering additionally builds the full transformed matrix. A multi-GB log/JSONL file means multi-GB (or more) of RSS. Proposal in two steps: (a) document the memory model and practical file-size expectations in docs/limitations.md (S); (b) if large files become a real use case, add a streaming path for the line-oriented formats only (LOG, JSONL, CSV), e.g. process_file_streaming() or an iterator-based process_lines(), which processes record by record and writes through the existing atomic-output mechanism. Text/Markdown/JSON stay whole-document by nature. (L)
2. Deep nested payloads crash with RecursionError (S).
_process_data, _string_blocks, and _replace_json_strings are recursive; a payload nested ~1000 levels deep raises a raw RecursionError (verified). An adversarial LLM payload can trigger this. Proposal: enforce an explicit maximum depth with a clean UnsupportedDataError (cheap, keeps recursion), or convert to iterative traversal with an explicit stack (slightly more code, no limit). I would do the explicit depth cap first: it also protects the JSON adapter path.
B. Long-lived and distributed processing
3. AliasContext grows without bound in long-lived scopes (S/M).
Every distinct normalized value adds one entry to context.aliases forever. Fine for request-scoped use (the intended design), but a long-running gateway holding one scope for days leaks memory proportionally to distinct values seen. Proposal: (a) document the intended scope lifecycle explicitly ("a scope is a bounded processing unit, not a process lifetime"); (b) state that deterministic mode is the correct choice for long-lived or distributed processing, because it is stateless (HMAC), so alias stability costs zero memory. I would not add LRU eviction: evicting silently breaks the shared-alias guarantee, which is worse than the leak.
4. Numbered mode does not scale horizontally, and that is fine, but say it (S).
Counters live in-process, so two workers assign <EMAIL_1> to different people. Deterministic mode already solves cross-process consistency by design. Proposal: a short "scaling model" section in docs/architecture.md: numbered/generic = single scope, deterministic = safe across processes, machines, and time. This turns an implicit property into a documented architectural contract.
5. LocalONNXPIIBackend lazy init is not thread-safe (S).
Two threads calling detect concurrently can both run _load_model and race on _session/_tokenizer assignment. Proposal: a threading.Lock around initialization, or eager loading in __init__ behind a flag. Related but separate from the #33 chunking work.
C. Hot-path leftovers
6. Placeholder-protection check is O(detections x placeholders) (S).
In _detect_block, each candidate detection is compared against every pre-existing placeholder span with a linear any(...). On placeholder-heavy inputs (e.g. re-processing already pseudonymized logs) this is the last quadratic-ish spot in the hot path. Proposal: the same sorted-intervals + bisect technique that #29 applied to resolve_overlaps; placeholders are non-overlapping and already found in text order, so this is a ~15-line change.
7. ONNX inference runs one block at a time (M).
A JSONL payload with thousands of small strings triggers thousands of single-sequence inference calls. ONNX Runtime handles batched input well, and padding logic is straightforward with the tokenizer in hand. Proposal: batch block inference inside the backend (transparent to the engine contract, which stays per-block), with a configurable batch size. Best done after the #33 chunking lands, since the two touch the same code.
D. Structural seams
8. ProcessingWarning exists but nothing ever emits one (S).
The dataclass, the warnings tuple, and the CLI serialization are all wired, but no code path constructs a warning (verified by grep). Either it is dead weight or an unused asset. Proposal: make the ML backend the first real emitter: "text exceeded model context, processed in N segments" (once chunking exists) and "id2label missing, ML detections disabled" are exactly warning-shaped conditions today, and the second one currently fails silently.
9. Closed EntityType enum vs locale/custom detectors (design discussion, ties into #34).
EntityType is a closed StrEnum; _ENTITY_PRIORITY and the placeholder-protection regex are built from it at import time, and backends must declare capabilities within it. That is a good safety property, but it means every new category (e.g. NATIONAL_ID from #34) is a core release, and third-party detectors can never introduce types. I am not proposing open registration (it would ripple through priorities, placeholder parsing, and the compatibility contract); the cheap hardening is _ENTITY_PRIORITY.get(entity_type, default) instead of a raw KeyError path, plus a deliberate decision in #34 about generic categories. Flagging it here so the enum's closedness is a documented choice rather than an accident.
10. Sanitized errors are great for privacy, hard for operations (S/M).
invoke_backend and the adapter paths deliberately collapse failures into generic messages with from None (no PII in tracebacks: good). At scale, though, "backend failed during detection" with no exception class, no block id, and no hook makes production debugging blind. Proposal: an opt-in diagnostics hook (e.g. on_backend_error: Callable[[BackendErrorInfo], None] carrying exception type, backend name, block id, but never text content), preserving the no-matched-values invariant while giving operators something to log.
Non-goals I would explicitly not pursue
An async API (CPU-bound work belongs in executors; a doc recipe is enough), a plugin/entry-point system, and native/Rust rewrites: all overengineering relative to current needs, and the zero-dependency property is worth more than any of them.
Happy to implement any subset in the usual one-PR-per-item style; 2, 5, 6, and 8 are small and independent, so they are natural first picks if you agree.
Architecture and scalability review: current limits and proposed solutions
Companion to #33/#34/#35: a review of the codebase from the angle of scalability, structure, and performance. To be clear upfront: the current architecture is sound for its stated scope (typed core, zero deps, immutable models, sanitized failures), and after #29 the hot path is near-linear. The points below are the limits I can identify beyond that scope, each with a concrete proposal and a rough size (S/M/L). Numbered for easy discussion; none of this is urgent, and some may be explicit non-goals for you.
A. Large inputs and memory
1. Everything is materialized in memory (S to document, L to change).
BuiltinFileAdapter.extractdoessource.read_bytes()and every adapter builds the fullDocumentbefore processing; CSV rendering additionally builds the full transformed matrix. A multi-GB log/JSONL file means multi-GB (or more) of RSS. Proposal in two steps: (a) document the memory model and practical file-size expectations indocs/limitations.md(S); (b) if large files become a real use case, add a streaming path for the line-oriented formats only (LOG, JSONL, CSV), e.g.process_file_streaming()or an iterator-basedprocess_lines(), which processes record by record and writes through the existing atomic-output mechanism. Text/Markdown/JSON stay whole-document by nature. (L)2. Deep nested payloads crash with
RecursionError(S)._process_data,_string_blocks, and_replace_json_stringsare recursive; a payload nested ~1000 levels deep raises a rawRecursionError(verified). An adversarial LLM payload can trigger this. Proposal: enforce an explicitmaximum depthwith a cleanUnsupportedDataError(cheap, keeps recursion), or convert to iterative traversal with an explicit stack (slightly more code, no limit). I would do the explicit depth cap first: it also protects the JSON adapter path.B. Long-lived and distributed processing
3.
AliasContextgrows without bound in long-lived scopes (S/M).Every distinct normalized value adds one entry to
context.aliasesforever. Fine for request-scoped use (the intended design), but a long-running gateway holding one scope for days leaks memory proportionally to distinct values seen. Proposal: (a) document the intended scope lifecycle explicitly ("a scope is a bounded processing unit, not a process lifetime"); (b) state that deterministic mode is the correct choice for long-lived or distributed processing, because it is stateless (HMAC), so alias stability costs zero memory. I would not add LRU eviction: evicting silently breaks the shared-alias guarantee, which is worse than the leak.4. Numbered mode does not scale horizontally, and that is fine, but say it (S).
Counters live in-process, so two workers assign
<EMAIL_1>to different people. Deterministic mode already solves cross-process consistency by design. Proposal: a short "scaling model" section indocs/architecture.md: numbered/generic = single scope, deterministic = safe across processes, machines, and time. This turns an implicit property into a documented architectural contract.5.
LocalONNXPIIBackendlazy init is not thread-safe (S).Two threads calling
detectconcurrently can both run_load_modeland race on_session/_tokenizerassignment. Proposal: athreading.Lockaround initialization, or eager loading in__init__behind a flag. Related but separate from the #33 chunking work.C. Hot-path leftovers
6. Placeholder-protection check is O(detections x placeholders) (S).
In
_detect_block, each candidate detection is compared against every pre-existing placeholder span with a linearany(...). On placeholder-heavy inputs (e.g. re-processing already pseudonymized logs) this is the last quadratic-ish spot in the hot path. Proposal: the same sorted-intervals + bisect technique that #29 applied toresolve_overlaps; placeholders are non-overlapping and already found in text order, so this is a ~15-line change.7. ONNX inference runs one block at a time (M).
A JSONL payload with thousands of small strings triggers thousands of single-sequence inference calls. ONNX Runtime handles batched input well, and padding logic is straightforward with the tokenizer in hand. Proposal: batch block inference inside the backend (transparent to the engine contract, which stays per-block), with a configurable batch size. Best done after the #33 chunking lands, since the two touch the same code.
D. Structural seams
8.
ProcessingWarningexists but nothing ever emits one (S).The dataclass, the
warningstuple, and the CLI serialization are all wired, but no code path constructs a warning (verified by grep). Either it is dead weight or an unused asset. Proposal: make the ML backend the first real emitter: "text exceeded model context, processed in N segments" (once chunking exists) and "id2label missing, ML detections disabled" are exactly warning-shaped conditions today, and the second one currently fails silently.9. Closed
EntityTypeenum vs locale/custom detectors (design discussion, ties into #34).EntityTypeis a closed StrEnum;_ENTITY_PRIORITYand the placeholder-protection regex are built from it at import time, and backends must declare capabilities within it. That is a good safety property, but it means every new category (e.g.NATIONAL_IDfrom #34) is a core release, and third-party detectors can never introduce types. I am not proposing open registration (it would ripple through priorities, placeholder parsing, and the compatibility contract); the cheap hardening is_ENTITY_PRIORITY.get(entity_type, default)instead of a rawKeyErrorpath, plus a deliberate decision in #34 about generic categories. Flagging it here so the enum's closedness is a documented choice rather than an accident.10. Sanitized errors are great for privacy, hard for operations (S/M).
invoke_backendand the adapter paths deliberately collapse failures into generic messages withfrom None(no PII in tracebacks: good). At scale, though, "backend failed during detection" with no exception class, no block id, and no hook makes production debugging blind. Proposal: an opt-in diagnostics hook (e.g.on_backend_error: Callable[[BackendErrorInfo], None]carrying exception type, backend name, block id, but never text content), preserving the no-matched-values invariant while giving operators something to log.Non-goals I would explicitly not pursue
An async API (CPU-bound work belongs in executors; a doc recipe is enough), a plugin/entry-point system, and native/Rust rewrites: all overengineering relative to current needs, and the zero-dependency property is worth more than any of them.
Happy to implement any subset in the usual one-PR-per-item style; 2, 5, 6, and 8 are small and independent, so they are natural first picks if you agree.