Skip to content

FIX: escape literal apostrophes in DigitBijectionConverter to avoid case-marker collision - #2696

Open
Mallika Kalangi (bhargavikvmpl-2001) wants to merge 1 commit into
microsoft:mainfrom
bhargavikvmpl-2001:fix/digit-bijection-apostrophe-marker-collision
Open

Mallika Kalangi (bhargavikvmpl-2001) wants to merge 1 commit into
microsoft:mainfrom
bhargavikvmpl-2001:fix/digit-bijection-apostrophe-marker-collision

Conversation

@bhargavikvmpl-2001

Copy link
Copy Markdown
Contributor

Description

DigitBijectionConverter uses ' as _CASE_MARKER, written before an encoded letter to record that the letter was uppercase. Because every letter always encodes to a digit token, decode recognises a marker by the fact that a digit token follows it.

A literal apostrophe in the plaintext — contractions and possessives such as it's, don't, I'm — is not in the mapping, so it is passed through unchanged. When the next character is a letter, that letter encodes to a digit token, and the passed-through apostrophe becomes indistinguishable from a real case marker. decode consumes it as one: the apostrophe is dropped and the following letter is wrongly uppercased.

On main (mapping a10z35):

plaintext encode decode
it's 1829'28 itS
I'm '18'22 IM
don't 132423'29 donT
O'Brien's '24''1127181423'28 O'BrienS

Round-tripping is therefore lossy for any input containing an apostrophe followed by a letter, which covers most naturally occurring English prose. Since the bijection technique sends the encoded prompt to the target and decodes the response, a corrupted round trip silently alters text on both legs.

Changes

  • encode: double a literal _CASE_MARKER when it is not being emitted as a case marker.
  • decode: collapse a doubled marker back to a single literal marker before the case-marker check, so the escape is removed and the following token is not misread as an uppercase letter.
  • Extend the comment above _CASE_MARKER to record why the escape is needed.

All four examples above round-trip exactly with this change.

Tests

  • tests/unit/converter/test_bijection_converter.py: 2 new tests — a lowercase letter after a literal apostrophe (it's) and an uppercase letter adjacent to one (I'm). Both were verified to fail against the pre-fix logic (assert "'18'22" == "'18''22") and to pass with this change.
  • tests/unit/converter/test_bijection_converter.py → 42 passed.
  • Full converter suite → 1564 passed, 52 failed, 34 skipped. All 52 failures are in test_seeded_converter_determinism.py (pinyin seeding) and reproduce identically on a clean upstream/main checkout (1562 passed, 52 failed, 34 skipped), so they are pre-existing and unrelated to this change.
  • pre-commit run --files pyrit/converter/bijection_converter.py tests/unit/converter/test_bijection_converter.py → all hooks pass (ruff format, ruff check, ty).
  • git diff --check → clean.

No documentation change: this is an internal encoding correctness fix with no API surface change.

I did not find an existing issue covering this — happy to open one if you would prefer it tracked separately.

🤖 Generated with Claude Code

…ase-marker collision

decode() treats any literal "'" as the case marker that precedes an uppercase
letter's digit token. Since the apostrophe is passed through unchanged at encode
time and every letter always encodes to a digit token, a plaintext apostrophe
immediately followed by a letter (e.g. "it's", "don't") is indistinguishable
from a real case marker: decode() swallowed the apostrophe and force-uppercased
the next letter instead of round-tripping it. Escape a literal case-marker
character by doubling it on encode, and collapse the doubled marker back to one
character on decode before checking for a real case marker.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Comment on lines +294 to +295
elif char == self._CASE_MARKER:
encoded += self._CASE_MARKER * 2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we update the digit converter's get_teaching_instructions() along with this change? It still tells the target to "preserve spaces/punctuation", without explaining that '' represents a literal apostrophe and ' before a digit token marks uppercase.

With the mapping in these tests, it's encodes to 1829''28. Following the current instructions literally would read that as it''s. In the other direction, a target encoding it's with unchanged punctuation would produce 1829'28, which decode() turns into itS.

Please teach both marker rules, include a contraction example, and add regression coverage for the generated instructions. Am I missing somewhere else these rules are already explained?

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.

2 participants