FIX: validate BinaryConverter bit width per converted word - #2591
Open
Umerkhan Golandaz (UmerkhanGolandaz) wants to merge 1 commit into
Open
Conversation
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>
Umerkhan Golandaz (UmerkhanGolandaz)
force-pushed
the
fix/binary-converter-validates-unselected-words
branch
from
September 7, 2026 19:21
c77a886 to
b2e61ab
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #2590.
BinaryConverter.validate_inputinspected the entire prompt, butWordLevelConverter.convert_asyncvalidates before applying the word selection strategy (word_level_converter.py:114runs 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 overflowbits_per_char:Only
hellois ever encoded here. The emoji is passed through as-is.Changes
convert_word_async, instead of against the whole prompt._validate_word, matching the style guide's rule that internal validation helpers are private (.github/instructions/style-guide.instructions.md).The
ValueErrormessage is unchanged. The defaultAllWordsSelectionStrategypath 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_asyncby callingvalidate_inputafter word selection, which would cover any future subclass. I kept the change local toBinaryConverterbecause it is the only subclass that currently overridesvalidate_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 testsruff format --checkandruff checkclean on both changed files