Skip to content

FIX: Preserve word-selection parameters in converter identifiers - #3000

Merged
Roman Lutz (romanlutz) merged 8 commits into
microsoft:mainfrom
LE0-Lin:fix/bin-ascii-selection-identity
Oct 12, 2026
Merged

Roman Lutz (romanlutz) merged 8 commits into
microsoft:mainfrom
LE0-Lin:fix/bin-ascii-selection-identity

Conversation

@LE0-Lin

@LE0-Lin Zhibo Lin (LE0-Lin) commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Description

Fixes #2995. Different word selections can produce different text but the same converter identifier, causing the second default registry registration to fail.

I extended the BinAscii fix to CharSwap, FirstLetter, Leetspeak, UnicodeReplacement, and Zalgo. They now include the parent word-selection parameters. Existing default identities are preserved, including CharSwap's default 20% selection and the other converters' all-words configurations. Non-default selections intentionally receive corrected hashes; previously saved hashes for those configurations can differ.

WordIndexSelectionStrategy also retains the supplied index order in its identity. Reordering indices can change which random values are used for each word. The fix preserves existing conversion order and outputs rather than sorting execution.

This follows the maintainer invitation to continue in this PR. Constructor APIs and conversion methods are unchanged.

Tests and Documentation

  • Original BinAscii regressions: 5 failed / 7 passed before its fix. The other five converter overrides: 25 failed / 15 passed before the expansion.
  • Updated index-order regressions: 9 failed against the previous production code. New seeded CharSwap, Zalgo, and nondeterministic Leetspeak cases check different but repeatable outputs, distinct identities, and successful registration of both instances. Independent before/after runs confirm conversion outputs are unchanged.
  • Related converter, word-selection, seeded-determinism, and registry tests across 12 modules: 389 passed. Python 3.12.0, frozen lockfile, full-extras environment.
  • uv run --frozen --all-extras ty check on the five files changed for review: passed, including the test files. The fixture now narrows request.param before returning it.
  • Normal repository pre-commit run --files on those five files: passed, including full-tree ty check pyrit. git diff --check passed.

The WordIndex selection docstrings now describe conversion order. Full repository tests and real-model integrations were not run locally.

AI assistance was used.

Signed-off-by: Zhibo Lin <147509942+LE0-Lin@users.noreply.github.com>
Roman Lutz (romanlutz) and others added 2 commits October 9, 2026 03:37
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Zhibo Lin <147509942+LE0-Lin@users.noreply.github.com>
@LE0-Lin Zhibo Lin (LE0-Lin) changed the title [DRAFT] FIX: Include word-selection parameters in BinAsciiConverter identifiers FIX: Preserve word-selection parameters in converter identifiers Oct 10, 2026
@LE0-Lin
Zhibo Lin (LE0-Lin) marked this pull request as ready for review October 10, 2026 05:43
@LE0-Lin

Copy link
Copy Markdown
Contributor Author

I included the other five converter overrides and kept the legacy default hashes. The related 321 tests and pre-commit checks pass locally. This is now ready for review, Roman Lutz (@romanlutz). Thanks!

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Comment thread tests/unit/converter/test_word_selection_identifiers.py
Comment thread tests/unit/converter/test_word_selection_identifiers.py Outdated
@romanlutz
Roman Lutz (romanlutz) added this pull request to the merge queue Oct 12, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 12, 2026
@romanlutz
Roman Lutz (romanlutz) added this pull request to the merge queue Oct 12, 2026
Merged via the queue into microsoft:main with commit c3c8dca Oct 12, 2026
55 checks passed
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.

BUG: BinAsciiConverter identifiers collide for different word-selection indices

2 participants