Skip to content

FIX: validate BinaryConverter bit width per converted word - #2591

Open
Umerkhan Golandaz (UmerkhanGolandaz) wants to merge 1 commit into
microsoft:mainfrom
UmerkhanGolandaz:fix/binary-converter-validates-unselected-words
Open

FIX: validate BinaryConverter bit width per converted word#2591
Umerkhan Golandaz (UmerkhanGolandaz) wants to merge 1 commit into
microsoft:mainfrom
UmerkhanGolandaz:fix/binary-converter-validates-unselected-words

Conversation

@UmerkhanGolandaz

Copy link
Copy Markdown

Description

Fixes #2590.

BinaryConverter.validate_input inspected the entire prompt, but WordLevelConverter.convert_async validates before applying the word selection strategy (word_level_converter.py:114 runs ahead of the selection at :118). A character in an unselected word therefore failed the whole conversion, even though that word is passed through unencoded and cannot overflow bits_per_char:

BinaryConverter(
    bits_per_char=BinaryConverter.BitsPerChar.BITS_16,
    word_selection_strategy=WordIndexSelectionStrategy(indices=[0]),
).convert_async(prompt="hello 👋")
# ValueError: bits_per_char=16 is too small for the characters in the prompt. Minimum required bits: 17.

Only hello is ever encoded here. The emoji is passed through as-is.

Changes

  • Validate the bit width against each word that is actually converted, in convert_word_async, instead of against the whole prompt.
  • Rename the helper to _validate_word, matching the style guide's rule that internal validation helpers are private (.github/instructions/style-guide.instructions.md).
  • Add two regression tests: the error still raises when the offending word is selected, and conversion succeeds when it is not.

The ValueError message is unchanged. The default AllWordsSelectionStrategy path behaves identically, since whole-prompt and per-word validation agree when every word is converted — only the selective path changes.

Alternative considered

The same bug could be fixed generically in WordLevelConverter.convert_async by calling validate_input after word selection, which would cover any future subclass. I kept the change local to BinaryConverter because it is the only subclass that currently overrides validate_input, and that avoids changing the base class hook's contract. Happy to switch to the generic fix if you prefer it.

Tests

  • pytest tests/unit/converter/ — 1440 passed, 34 skipped, including the 2 new tests
  • ruff format --check and ruff check clean on both changed files

@UmerkhanGolandaz

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

BinaryConverter.validate_input inspected the entire prompt, but
WordLevelConverter.convert_async validates before applying the word
selection strategy. A character in an unselected word therefore failed
the conversion even though that word is passed through unencoded and
cannot overflow bits_per_char.

Move the check to the words that are actually converted. The default
all-words path is unchanged, so this only affects prompts where a
selection strategy leaves the offending word untouched.

Rename the helper to _validate_word to match the style guide, which
marks internal validation helpers as private.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@UmerkhanGolandaz
Umerkhan Golandaz (UmerkhanGolandaz) force-pushed the fix/binary-converter-validates-unselected-words branch from c77a886 to b2e61ab Compare September 7, 2026 19:21
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: BinaryConverter rejects characters in words the selection strategy never converts

1 participant