feat: export strip_control_chars and strip_zero_width_chars from Python (#616) - #623
Conversation
…on (#616) They already existed in the Rust core (disarm::api) and in the C ABI, Java/Kotlin, Node and Ruby bindings. Python was the only surface without them, so control-stripping there meant constructing a TextPipeline instead of calling a function — unlike the ten sibling strip_* operations, four of which (strip_tags, strip_pua, strip_noncharacters, strip_variation_selectors) are narrower and are plain functions. Both are now exported, with matching Text methods. The parity matrix recorded the gap as deliberate and named the substitute as collapse_whitespace(strip_control=True) — a signature that has never existed; collapse_whitespace takes only `text`. That record lived in PROVIDED_VIA in scripts/parity.py, so anyone consulting the matrix for the Python equivalent was sent to a TypeError. Both entries are removed and generated/parity.yaml regenerated; it now names a real function on all six surfaces. Two gates did their job and are worth recording. test_api_stability caught the missing __all__ entries. test_form_invariance_audit auto-discovered both new entrypoints and failed them, because a new str->str name is in scope for boundary normalization unless someone deliberately classifies it — these are targeted strips in the same category as strip_bidi/strip_tags/strip_pua, so they join the reviewed FORM_PRESERVING allowlist rather than being silently exempted. Also closes a coverage gap in collapse_whitespace's property tests. The existing no_leading_trailing_whitespace property draws from \PC*, which excludes controls, so the trim invariant was never tested against them. Added no_edge_whitespace_even_with_controls, which draws from a whitespace-plus-controls alphabet. The invariant holds. #612 reported a trim bug here; that report was wrong and is retracted on the issue. Measured exhaustively over the cross product of whitespace, controls and letters for lengths 1-4, plus 200,000 random strings: zero cases where the output starts or ends with whitespace. What looked like a defeated trim is the space BETWEEN a leading control and the word, which is interior by exactly the rule that makes "a \0 b" keep both of its spaces. No behaviour change here — only the test that makes the answer checkable. Signed-off-by: Richard Quinn <quinn.richard@gmail.com> Assisted-by: Claude Code:claude-opus-5
|
📄 Docs preview: https://fb32c05c.disarm-docs.pages.dev |
There was a problem hiding this comment.
🟡 Changes recommended
The new public Python entrypoints lack direct behavioral pytest coverage (beyond surface/audit gates), and the strip_zero_width_chars docstring currently omits at least one stripped code point (U+180E).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR closes #616 by bringing Python up to parity with the Rust core and other bindings for control/zero-width stripping, while also correcting a misleading parity record and tightening whitespace trimming coverage around control characters.
Changes:
- Export
strip_control_charsandstrip_zero_width_charson the Python surface (both free functions andTextfluent methods), wiring them through the PyO3 shim. - Remove the incorrect Python
provided_viaentries from parity tooling and regenerategenerated/parity.yamlto reflect the real callable API. - Add/adjust test gates: extend the Python form-invariance audit allowlist and add a Rust property test ensuring
collapse_whitespacenever leaves edge spaces even with controls present.
File summaries
| File | Description |
|---|---|
| tests/test_form_invariance_audit.py | Allowlists the new Python entrypoints as form-preserving for the invariance audit. |
| tests/test_api_stability.py | Adds the new names/signatures to the Python public API stability gate. |
| src/whitespace.rs | Adds tests clarifying control preservation in collapse_whitespace and a new proptest for edge-trim invariants with controls. |
| src/py/whitespace.rs | Exposes new PyO3 functions _strip_control_chars / _strip_zero_width_chars. |
| src/lib.rs | Registers the new PyO3 functions in the Python extension module. |
| scripts/parity.py | Removes the incorrect provided_via guidance and documents the constraint for future entries. |
| python/disarm/_text.pyi | Adds Text.strip_control_chars / Text.strip_zero_width_chars to type stubs. |
| python/disarm/_text.py | Implements the new fluent Text methods calling the new public functions. |
| python/disarm/_core.pyi | Adds stub declarations for the new extension functions. |
| python/disarm/_boundary.pyi | Re-exports the new _core functions through the boundary-guarded surface stubs. |
| python/disarm/_api.py | Adds the new public Python functions and updates collapse_whitespace docs to reference them. |
| python/disarm/init.py | Re-exports the new functions and adds them to __all__. |
| generated/parity.yaml | Regenerates parity matrix entries to show real Python callables instead of non-existent signatures. |
| CHANGELOG.md | Documents the new Python exports and the new whitespace test coverage. |
Review details
- Files reviewed: 13/14 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…180E (#616) Both from review on #623. The docstring listed the zero-width set as prose and omitted U+180E, which whitespace::is_zero_width does remove — the Mongolian vowel separator, reclassified Zs -> Cf in Unicode 6.3, so a format character despite the name. The set is now enumerated exactly. The two entrypoints were covered only by the API-surface audits, which assert a name and a signature and nothing about what the function does. 29 cases now pin the character sets, ordinary text passing through untouched, the documented composition matching canonicalize, and the Text methods. The one worth keeping is test_the_two_functions_do_not_overlap: each leaves the other's set alone, which is the reason there are two functions rather than one and the thing a regression in the binding glue would break first. Signed-off-by: Richard Quinn <quinn.richard@gmail.com> Assisted-by: Claude Code:claude-opus-5
Closes #616. Partially addresses #612 — see the retraction below.
The gap
strip_control_charsandstrip_zero_width_charsalready existed in the Rust core (disarm::api,src/api/text.rs:52-60) and in the C ABI, Java/Kotlin, Node and Ruby bindings. Python was the only surface without them.So control-stripping in Python meant constructing a
TextPipelinerather than calling a function — unlike the ten siblingstrip_*operations, four of which (strip_tags,strip_pua,strip_noncharacters,strip_variation_selectors) are narrower and are plain functions. Both are now exported, with matchingTextmethods.The parity record pointed at a signature that never existed
generated/parity.yamlrecorded the gap as deliberate and named the substitute:collapse_whitespacetakes onlytext; that keyword raisesTypeError. The string lived inPROVIDED_VIAinscripts/parity.py, so anyone consulting the matrix for the Python equivalent was sent somewhere that does not work. Both entries are removed and the matrix regenerated — it now names a real function on all six surfaces.I also added a note to
parity.pythatprovided_viais only for a route that is real and callable, since that is the failure this hid.Two gates did their job
Worth recording, because both caught something rather than rubber-stamping it.
test_api_stabilitycaught the missing__all__entries immediately.test_form_invariance_auditauto-discovered both new entrypoints and failed them. That audit enumerates publicstr -> strnames from__all__and assertsf(NFC(x)) == f(NFD(x)), so a new name is in scope for boundary normalization unless someone deliberately classifies it. These are targeted strips in the same category asstrip_bidi/strip_tags/strip_pua, so they join the reviewedFORM_PRESERVINGallowlist with the reasoning written down, rather than being silently exempted.Retraction: #612's trim bug is not real
I filed #612 claiming a leading control defeats
collapse_whitespace's trim. That was wrong, and I have retracted it on the issue.What I read as a defeated trim is the space between the control and the word, which is interior by exactly the rule that makes
"a \x00 b"keep both of its spaces. The leading run was trimmed correctly:The invariant a caller depends on — output never starts or ends with whitespace — holds in every case. Measured exhaustively over the cross product of whitespace, controls and letters for lengths 1–4, plus 200,000 random strings of length 5–12: zero violations.
Why it was not already settled: the existing
no_leading_trailing_whitespaceproperty draws from\PC*, which excludes controls, so it never covered the case either way. This PR addsno_edge_whitespace_even_with_controls, drawing from a whitespace-plus-controls alphabet, turning the measurement into a standing gate.No behaviour change to
collapse_whitespace. Only the test that makes the answer checkable.#612's item 2 stands — nothing detects edge whitespace, and
split_tokensnever emits it, so a check has to be a whole-text pass. That is queued as its own change.Verification
Full local gate: 4,715 pytest, 24 Rust test targets,
cargo fmt --check, clippy clean on both feature sets,scripts/perf_lint.shclean, mypy clean, ruff clean, language-consistency audit clean,scripts/parity.pyregenerated.