feat: target_script="arabic" and "hebrew" (#792) by raeq · Pull Request #849 · raeq/disarm

raeq · 2026-09-01T06:04:30Z

Generation keeps the members of an equivalence class that belong to the target
script and drops the class entirely when no member does. So a class whose members
are all Arabic survived into neither shipped table, and 948 of TR39's 1,007
strong-RTL sources folded to nothing (#791). These two targets give those classes
somewhere to land:

arabic   373 rows    ⸮ -> ؟   ⸲ -> ،   𞣉 -> ٣
hebrew   261 rows    ℵ -> א   ℶ -> ב   ∸ -> ﬩

Opt-in, exactly like cyrillic. No preset consumes a non-Latin target — it is
reachable only through normalize_confusables(target_script=…) and
is_confusable — so this adds a view rather than changing any existing answer.
That is what resolved §1's blocking question: an Arabic target introduces no
second answer on a surface a caller is already using, and so does not collide with
#735, which is about the Latin path inside the key builders.

THEY DO NOT REACH AN INTRA-ARABIC PAIR, and that is the half worth stating.

#792 was filed believing an Arabic target would fold Persian keheh onto Arabic
kaf. I prototyped it before writing any of it, and all four code points in the
issue's motivating table are ABSENT from the generated Arabic table:
filter_direct skips a source already in the target script, and
filter_via_classes maps non-target members onto the target-script member, so an
all-Arabic class has nothing to map from. TR39 does put keheh and kaf in one class
with kaf as prototype — the data is not the problem, the cross-script model is.

Split out as #848, with the class measured: 548 intra-Arabic letter pairs and 2
intra-Hebrew, presentation forms excluded because NFKC removes those first. It is
#831's machinery one script over — equivalence classes with a representative from
the class, and a build assert that a non-ASCII target is in-script and is not
itself a source — so the mechanism exists and the two decisions it needs are
already settled. The dependency runs the other way from how #792 assumed: #848
probably needs these targets, because Arabic folding to Arabic is not a to-Latin
mapping under any reading.

is_suspicious_hostname is unaffected, per §5, and the docs say so rather than
implying otherwise: src/hostname.rs computes whole-script-confusable as
["Latin"] and calls the fold with "latin" hardcoded.

tests/test_rtl_targets.py asserts both halves. The boundary assertions are the
ones that matter — they stop a reader concluding from the CHANGELOG that intra-RTL
is now covered, and they will fail loudly when #848 lands, which is the right time
to revisit them.

Six surfaces per AGENTS.md: the bindings pass the target as a string so they need
no code change, and their doc comments are updated. docs/provenance.md names all
four tables, and test_doc_table_counts.py gains the two new row counts —
anchored on the script name rather than on position, because the ordinal patterns
there already broke once when a new "(N mappings)" appeared between them.

Closes #792
Refs #791, #831, #848, #735, #563

Verification

cargo test / clippy --all-targets -D warnings   clean
cargo doc --no-deps                             clean
pytest -m 'not formal'                          6531 passed
tests/test_parity_floor.py                      45 passed
python3 scripts/run_doc_tests.py                39 doc pages
mkdocs build --strict                           clean
ruff==0.16.4 (CI's pin) check + format          clean
cabi header regenerated (#580 gate)             passing

Split per your call: this is the cross-script half, #848 is the intra-script half.

🤖 Generated with Claude Code

Generation keeps the members of an equivalence class that belong to the target script and drops the class entirely when no member does. So a class whose members are all Arabic survived into neither shipped table, and 948 of TR39's 1,007 strong-RTL sources folded to nothing (#791). These two targets give those classes somewhere to land: arabic 373 rows ⸮ -> ؟ ⸲ -> ، 𞣉 -> ٣ hebrew 261 rows ℵ -> א ℶ -> ב ∸ -> ﬩ Opt-in, exactly like `cyrillic`. No preset consumes a non-Latin target — it is reachable only through `normalize_confusables(target_script=…)` and `is_confusable` — so this adds a view rather than changing any existing answer. That is what resolved §1's blocking question: an Arabic target introduces no second answer on a surface a caller is already using, and so does not collide with #735, which is about the Latin path inside the key builders. THEY DO NOT REACH AN INTRA-ARABIC PAIR, and that is the half worth stating. #792 was filed believing an Arabic target would fold Persian keheh onto Arabic kaf. I prototyped it before writing any of it, and all four code points in the issue's motivating table are ABSENT from the generated Arabic table: `filter_direct` skips a source already in the target script, and `filter_via_classes` maps non-target members onto the target-script member, so an all-Arabic class has nothing to map from. TR39 does put keheh and kaf in one class with kaf as prototype — the data is not the problem, the cross-script model is. Split out as #848, with the class measured: 548 intra-Arabic letter pairs and 2 intra-Hebrew, presentation forms excluded because NFKC removes those first. It is #831's machinery one script over — equivalence classes with a representative from the class, and a build assert that a non-ASCII target is in-script and is not itself a source — so the mechanism exists and the two decisions it needs are already settled. The dependency runs the other way from how #792 assumed: #848 probably needs these targets, because Arabic folding to Arabic is not a to-Latin mapping under any reading. `is_suspicious_hostname` is unaffected, per §5, and the docs say so rather than implying otherwise: `src/hostname.rs` computes whole-script-confusable as `["Latin"]` and calls the fold with `"latin"` hardcoded. `tests/test_rtl_targets.py` asserts both halves. The boundary assertions are the ones that matter — they stop a reader concluding from the CHANGELOG that intra-RTL is now covered, and they will fail loudly when #848 lands, which is the right time to revisit them. Six surfaces per AGENTS.md: the bindings pass the target as a string so they need no code change, and their doc comments are updated. `docs/provenance.md` names all four tables, and `test_doc_table_counts.py` gains the two new row counts — anchored on the script name rather than on position, because the ordinal patterns there already broke once when a new "(N mappings)" appeared between them. Closes #792 Refs #791, #831, #848, #735, #563 Signed-off-by: Richard Quinn <quinn.richard@gmail.com> Assisted-by: Claude Code:claude-opus-5

github-actions · 2026-09-01T06:05:13Z

📄 Docs preview: https://0542693e.disarm-docs.pages.dev

Copilot

🟡 Changes recommended

There are correctness/maintenance gaps in the new/updated validation tests and build/version invariants (plus a Ruff-unused-variable issue) that should be fixed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds opt-in target_script="arabic" and "hebrew" confusable-fold targets across the Rust core, generated tables, bindings, and documentation, extending TR39 cross-script normalization beyond Latin/Cyrillic without changing existing preset behavior.

Changes:

Generate and ship new confusables target tables: confusables_to_arabic.tsv and confusables_to_hebrew.tsv, and wire them into table resolution.
Extend the public API/docs to accept "arabic" / "hebrew" (Rust TargetScript, Python docstrings/docs, bindings doc comments, CHANGELOG).
Add regression tests covering acceptance, a few representative folds, and explicitly asserting the “intra-script boundary” behavior.

File summaries

File	Description
tests/test_rtl_targets.py	Adds tests for the new RTL targets and asserts the intended boundary of what they do/don’t cover.
tests/test_doc_table_counts.py	Updates doc row-count assertions to include Arabic/Hebrew target counts.
src/tables/data/confusables_to_hebrew.tsv	New generated confusables-to-Hebrew table (data).
src/tables/data/confusables_to_arabic.tsv	New generated confusables-to-Arabic table (data).
src/tables/confusables_data.rs	Includes the new PHF tables and exposes them via `resolve_map`.
src/confusables.rs	Extends target validation to accept `"arabic"` / `"hebrew"`.
src/api/safety.rs	Extends `TargetScript` enum and parsing/stringification to include Arabic/Hebrew.
src/api/metadata.rs	Updates metadata tests/comments around confusables table versioning.
scripts/gen_confusables.py	Adds script detectors and registers `"arabic"` / `"hebrew"` as generator targets.
python/disarm/_api.py	Updates Python API docstrings for `normalize_confusables` / `unmapped_confusables`.
docs/user-guide/confusables.md	Documents the new targets and clarifies their boundary/hostname impact.
docs/provenance.md	Adds the new tables to the provenance table.
CHANGELOG.md	Documents the feature and its explicit non-goals/boundaries.
build.rs	Generates PHF maps for the new tables.
bindings/ruby/ext/disarm/src/lib.rs	Updates Ruby binding doc comments for accepted targets.
bindings/node/src/lib.rs	Updates Node binding doc comments for accepted targets.
bindings/java/rust/src/lib.rs	Updates Java binding doc comments for accepted targets.
bindings/cabi/src/lib.rs	Updates C ABI binding doc comments for accepted targets.
bindings/cabi/disarm.h	Updates the committed C header doc comment for accepted targets.

Review details

Suppressed comments (1)

src/api/metadata.rs:246

This test claims (and the comment states) that all four confusables tables share CONFUSABLES_VERSION, but it currently only checks the Cyrillic table header. That can let the new Arabic/Hebrew tables drift without failing tests.

    /// All four bundled tables are folded from one upstream release, which is what lets a
    /// single const cover them. build.rs asserts it at build time; assert it here too so
    /// the reason for the single const is visible in the test suite.
    #[test]
    fn both_confusable_tables_name_the_same_version() {

Files reviewed: 19/19 changed files
Comments generated: 3
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Three findings on #849, all correct. validate_target_script's doc comment still said only latin/cyrillic. The list is written twice -- once as the match arms, once as TargetScript's variants -- so it is now checked rather than kept in step by hand. TargetScript::ALL enumerates the variants, every_target_script_variant_validates asserts each one is accepted, and the_validator_accepts_nothing_outside_the_enum asserts the reverse so the enum stays the definition of what is supported. The build-time CONFUSABLES_VERSION agreement check compared Latin against Cyrillic and nothing else, so #792 added two tables the check did not see -- a mixed upstream release would have shipped one version constant describing four tables it no longer described. There were three separate lists of the tables in build.rs; there is now one, CONFUSABLE_TABLES, and the check derives from it. Adding a target reaches the version check for free. Mutation-checked: setting the Hebrew header to 16.0.0 now fails the build, naming latin and hebrew. was bound and never used, which Ruff's F rules would have flagged. Asserting it turned out to be worth more than renaming it: the label IS suspicious, for a bidi conflict and a cross-label script change that both predate #792, and the test now says so alongside whole_script_confusable staying False. That is the binding that makes it a regression test -- if a target ever starts driving the hostname screen, the flag moves and this fails. Also unfused a doc comment in the same PR: TargetScript::as_str carried two first lines, the second having been inserted above it. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Richard Quinn <quinn.richard@gmail.com>

Three findings on #849, all correct. validate_target_script's doc comment still said only latin/cyrillic. The list is written twice -- once as the match arms, once as TargetScript's variants -- so it is now checked rather than kept in step by hand. TargetScript::ALL enumerates the variants, every_target_script_variant_validates asserts each one is accepted, and the_validator_accepts_nothing_outside_the_enum asserts the reverse so the enum stays the definition of what is supported. The build-time CONFUSABLES_VERSION agreement check compared Latin against Cyrillic and nothing else, so #792 added two tables the check did not see -- a mixed upstream release would have shipped one version constant describing four tables it no longer described. There were three separate lists of the tables in build.rs; there is now one, CONFUSABLE_TABLES, and the check derives from it. Adding a target reaches the version check for free. Mutation-checked: setting the Hebrew header to 16.0.0 now fails the build, naming latin and hebrew. The `suspicious` variable was bound and never used, which Ruff's F rules would have flagged. Asserting it turned out to be worth more than renaming it: the label IS suspicious, for a bidi conflict and a cross-label script change that both predate #792, and the test now says so alongside whole_script_confusable staying False. That is the binding that makes it a regression test -- if a target ever starts driving the hostname screen, the flag moves and this fails. Also unfused a doc comment in the same PR: TargetScript::as_str carried two first lines, the second having been inserted above it. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Richard Quinn <quinn.richard@gmail.com>

The docs conflict was semantic rather than textual. #791's section on main says the two targets are "the only two views that exist" and that there is "no third view to select"; this branch adds two more. Keeping both halves would have left the page contradicting itself, so the heading and opening paragraph are rewritten to state the fact that survives -- generation drops a class entirely when no member belongs to the target script, so adding a target does not open a view onto rows the others hide -- and the rest of #791's section is kept verbatim. Two gates needed repair, both from #845 and both anchored to the wrong thing: - test_the_page_states_the_residue asserted the literal phrase "only two views" was present. That is the drifting thing, not the fact. It now anchors on "drops the class entirely when no member does", which is what the section exists to say and what an edit improving the page keeps. - _documented scanned the whole page for any line holding the label and a pipe. Once this branch added a section that also mentions Arabic it matched the wrong table and returned 373 for a row that says 961 -- a gate reading the wrong row is worse than no gate. It now takes a table header and reads only the rows under it. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Richard Quinn <quinn.richard@gmail.com>

The gate caught its own page: #847's 17 LGR rows and #849's Arabic and Hebrew tables moved the count from 227 of 313 to 226 of 312 between writing the entry and merging main. The detector's share is unchanged at 97.8%. That is the gate working rather than a problem with it -- a figure that moves when the tables move is exactly what test_doc_table_counts.py and test_confusable_residue_docs.py exist to hold, and this one now has the same treatment. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Richard Quinn <quinn.richard@gmail.com>

) It read `target_script must be 'latin' or 'cyrillic'` and stayed that way when `unmapped_confusables` and `find_confusables`. A caller who trusted the message could not discover the two targets that cycle existed to add. The doc comment on `TargetScript::ALL` already warns that these lists drift, and two tests hold the validator and the enum together. The message was a third copy nobody had wired in. It is derived from `TargetScript::ALL` now, so a fifth target updates it by construction, and a test asserts it names every accepted token and none of the unsupported ones. The accepted set is worth measuring against the table it comes from. Counting `data/confusables.txt` by the script its target resolves to, 2,694 of 6,565 pairs — 41% — point at a target that cannot be asked for. Han is the second-largest target at 1,471 pairs and is rejected; Greek carries 161 and is rejected where Cyrillic (37) and Hebrew (24) are both accepted. So the criterion for adding a target is not pair count, and it is undocumented. Whether `target_script` should accept a script with no table is #884, deferred to 0.16.x. This is only the part that was wrong rather than incomplete. Refs #792, #849, #884, #762 Closes #888 Assisted-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Richard Quinn <quinn.richard@gmail.com>

) (#891) * fix: the target_script error named two of the four accepted values (#888) It read `target_script must be 'latin' or 'cyrillic'` and stayed that way when `unmapped_confusables` and `find_confusables`. A caller who trusted the message could not discover the two targets that cycle existed to add. The doc comment on `TargetScript::ALL` already warns that these lists drift, and two tests hold the validator and the enum together. The message was a third copy nobody had wired in. It is derived from `TargetScript::ALL` now, so a fifth target updates it by construction, and a test asserts it names every accepted token and none of the unsupported ones. The accepted set is worth measuring against the table it comes from. Counting `data/confusables.txt` by the script its target resolves to, 2,694 of 6,565 pairs — 41% — point at a target that cannot be asked for. Han is the second-largest target at 1,471 pairs and is rejected; Greek carries 161 and is rejected where Cyrillic (37) and Hebrew (24) are both accepted. So the criterion for adding a target is not pair count, and it is undocumented. Whether `target_script` should accept a script with no table is #884, deferred to 0.16.x. This is only the part that was wrong rather than incomplete. Refs #792, #849, #884, #762 Closes #888 Assisted-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Richard Quinn <quinn.richard@gmail.com> * refactor: accepted_tokens is pub(crate), not pub #891 review. Its only caller is the `InvalidTargetScript` message in `src/error.rs`. A formatting helper for an internal error does not need to be carried under semver forever, and `crate::api` is the semver-governed surface. Assisted-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Richard Quinn <quinn.richard@gmail.com> --------- Signed-off-by: Richard Quinn <quinn.richard@gmail.com>

Copilot AI lite review requested due to automatic review settings September 1, 2026 06:04

Copilot started reviewing on behalf of raeq September 1, 2026 06:05 View session

Copilot AI reviewed Sep 1, 2026

View reviewed changes

Comment thread tests/test_rtl_targets.py

Comment thread src/confusables.rs Outdated

Comment thread build.rs

raeq force-pushed the feat/arabic-hebrew-targets branch from 66f61f6 to 87d6b9f Compare September 1, 2026 06:57

raeq enabled auto-merge (squash) September 1, 2026 07:03

raeq added 3 commits September 1, 2026 09:03

Merge branch 'main' into feat/arabic-hebrew-targets

c107fc9

Merge branch 'main' into feat/arabic-hebrew-targets

5527650

raeq merged commit a902532 into main Sep 1, 2026
35 checks passed

raeq deleted the feat/arabic-hebrew-targets branch September 1, 2026 07:47

raeq mentioned this pull request Sep 2, 2026

Plan 0.16.0: 32 issues, eleven clusters, and the twelve orderings that stop the rework #902

Open

Navigation Menu

feat: target_script="arabic" and "hebrew" (#792) - #849

feat: target_script="arabic" and "hebrew" (#792)#849
raeq merged 5 commits into
mainfrom
feat/arabic-hebrew-targets

raeq commented Sep 1, 2026

Uh oh!

github-actions Bot commented Sep 1, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

raeq commented Sep 1, 2026

Verification

Uh oh!

github-actions Bot commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Sep 1, 2026 •

edited

Loading