Skip to content

fix(lyrics): prevent Bengali romanizer crash on mixed-script text - #426

Merged
sozercan merged 1 commit into
sozercan:mainfrom
YuriNachos:YuriNachos/w5-kaset-bengali
Aug 10, 2026
Merged

fix(lyrics): prevent Bengali romanizer crash on mixed-script text#426
sozercan merged 1 commit into
sozercan:mainfrom
YuriNachos:YuriNachos/w5-kaset-bengali

Conversation

@YuriNachos

Copy link
Copy Markdown
Contributor

Description

BengaliRomanizer.romanize(_:) could trap with Fatal error: String index is out of bounds when a line mixed Bengali with another non-Latin script the Bengali-locale tokenizer cannot transcribe (e.g. a Bengali lyric line containing a Thai/CJK/Hangul/Arabic word). The fallback branch indexed the Swift String by grapheme cluster (text.index(_:offsetBy:)), but CFStringTokenizer token ranges are measured in UTF-16 code units. Bengali matras (combining marks) make the grapheme count smaller than the UTF-16 count, so the UTF-16 location overshoots endIndex and the process traps. This harmonizes the romanizer with its ThaiRomanizer / JapaneseRomanizer siblings, which already index via NSString.

AI Prompt (Optional)

🤖 AI Prompt Used
Investigate and fix an indexing crash/mis-romanization in BengaliRomanizer.
Verify the bug at HEAD before fixing (instrument the real CFStringTokenizer
loop; do not infer from source). Mirror the correct Thai/Japanese romanizers:
add `let nsText = text as NSString` and extract fallback tokens with
`nsText.substring(with: NSRange(...))` so the unit matches the CFRange. Do not
change transliteration tables or the sibling romanizers. Add a pure-logic
Swift Testing case that is RED at HEAD and GREEN after the fix, plus a
pure-Bengali control and an emoji passthrough guard. Run the project gate.

AI Tool: Claude Code

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Related Issues

Changes Made

  • BengaliRomanizer.swift: capture let nsText = text as NSString and extract the fallback (non-transliterable) token via nsText.substring(with: NSRange(location:length:)) instead of grapheme-cluster String indexing, matching ThaiRomanizer/JapaneseRomanizer.
  • RomanizationTests.swift: add BengaliRomanizerTests — a cross-script crash regression (বাংলা ก, RED→GREEN), a pure-Bengali control, and an emoji passthrough guard.

Testing

  • Unit tests pass (swift test --skip KasetUITests)
  • Manual testing performed (instrumented tokenizer to confirm RED crash + GREEN output on macOS 26)
  • UI tested on macOS 26+ (n/a — pure-logic romanization change)

RED (HEAD): romanize("বাংলা ก") traps — Swift/StringCharacterView.swift:158: Fatal error: String index is out of bounds (signal 5). GREEN (fix): returns bānlāก; full suite green.

Checklist

  • My code follows the project's style guidelines
  • I have run swiftlint --strict && swiftformat .
  • I have added tests that prove my fix/feature works
  • New and existing unit tests pass locally
  • I have updated documentation if needed
  • I have checked for any performance implications
  • My changes generate no new warnings

Screenshots

n/a — no UI change.

Additional Notes

  • Crash trigger is cross-script non-Latin text, not emoji/symbols. An emoji like ❤️/👋 is emitted by CFStringTokenizer with a LatinTranscription equal to itself, so it takes the transcription branch (not the buggy fallback) and already worked. The crash requires a token that is emitted without a Latin transcription — i.e. another non-Latin script the bn locale can't transcribe (Thai , CJK, Hangul, Arabic). Verified empirically by instrumenting the tokenizer.
  • The regression test's assertions are deliberately limited to the crash contract (no trap + intact Bengali run) rather than asserting the raw foreign character: LatinTranscription availability can vary across macOS versions and CI spans macOS 15 + 26, so asserting the raw glyph would be OS-fragile.
  • A separate, intentionally out-of-scope behavior: word-unit tokenization drops non-word symbols like // (they aren't emitted as tokens). That is a tokenizer-emission characteristic, not the indexing bug fixed here, and is left unchanged.

BengaliRomanizer.romanize(_:) trapped with "String index is out of bounds"
when a line mixed Bengali with another non-Latin script the bn-locale
tokenizer cannot transcribe (e.g. Bengali + Thai). The fallback branch
indexed the Swift String by grapheme cluster (text.index(_:offsetBy:)),
but CFStringTokenizer token ranges are measured in UTF-16 code units.
Bengali matras (combining marks) make the grapheme count smaller than the
UTF-16 count, so the UTF-16 location overshoots endIndex and the process
traps.

Index via NSString (UTF-16 code units) so the unit matches the CFRange,
mirroring the ThaiRomanizer / JapaneseRomanizer siblings. No
transliteration-table or sibling-romanizer changes.

Add BengaliRomanizerTests: a cross-script crash regression
("বাংলা ก" traps at HEAD, returns "bānlāก" after the fix), a pure-Bengali
control, and an emoji passthrough guard. Assertions are kept as weak as
the sibling romanizer tests so they do not depend on per-OS tokenizer
transcription data across the macOS 15 + 26 CI matrix.

Co-Authored-By: Claude <noreply@anthropic.com>
@YuriNachos

Copy link
Copy Markdown
Contributor Author

Heads-up on the red macOS Unit Tests (macos-26) check, so it does not read as a problem with this branch: the only failing test is WebKitCookieRestoreTests … Persisted archive replaces stale live authentication cookies, which is in the cookie/auth layer — unrelated to the Bengali romanizer change in this PR.

It fails only on macos-26; on the same commit macOS Unit Tests (macos-15) passes, and macOS UI Tests passes on both macos-15 and macos-26, as does build/SwiftLint/SwiftFormat. Nothing in the romanizer path can affect cookie restoration, so this looks like a macos-26 runner/WebKit behavior difference rather than a regression here. Happy to rebase/re-trigger once the workflow is healthy.

@sozercan
sozercan merged commit be0eae7 into sozercan:main Aug 10, 2026
7 of 8 checks passed
@YuriNachos
YuriNachos deleted the YuriNachos/w5-kaset-bengali branch August 10, 2026 13:44
Yoddikko pushed a commit to Yoddikko/kasetPlus that referenced this pull request Aug 27, 2026
Yoddikko added a commit to Yoddikko/kasetPlus that referenced this pull request Aug 27, 2026
…zercan#448

Cherry-picked from sozercan/kaset:
- sozercan#426 Bengali romanizer mixed-script crash fix
- sozercan#425 refresh Home suggestions on demand (adapted: kept fork shorts aggregation + chip-bar callers pass forceRefresh:false)
- sozercan#447 stop page URL stored as track artwork
- sozercan#448 eliminate cross-suite unit test flakes (WebKit cookie handling)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012LaRrBbQsc5W5agXFkYc7u
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