Add RTL text direction support for 0.8.1#155
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces reading direction post-processing (LTR, RTL, and Auto) for recognized text in the OCR pipeline, utilizing the unicode-bidi crate to convert visual RTL text into logical order. The feedback highlights two issues in the post-processing logic: first, is_ltr_token_char should include BidiClass::NSM to prevent scrambling LTR words with combining marks; second, normalize_leading_combining_marks contains a bug where combining marks can be incorrectly attached to the wrong character, which can be resolved with a more robust implementation that immediately attaches them to the preceding character.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e67062080c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces reading-direction post-processing (LTR, RTL, and Auto) for recognized OCR text, utilizing the unicode-bidi crate to convert visual RTL text (such as Arabic) into logical string order while preserving embedded LTR phrases, numbers, and combining marks. The configuration is exposed across the CLI example, adapters, predictors, and builders. A correctness bug was identified in normalize_leading_combining_marks where multiple combining marks on subsequent characters in reversed RTL text are incorrectly handled, leading to garbled diacritics. A simplified implementation was suggested to collect all combining marks into pending_marks until a base character is encountered.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20231e0264
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2662bf548b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9a7f89378
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22f366dba8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| fn reorder_bidi_line(line: &str) -> String { | ||
| let bidi_info = BidiInfo::new(line, None); |
There was a problem hiding this comment.
The fresh evidence after the earlier auto-direction fixes is this new None paragraph level: for visual-order RTL CTC output whose logical line ends with an LTR token, e.g. abc 123 ابحرم for مرحبا abc 123, the first strong character is the visual-left a, so unicode-bidi resolves the paragraph as LTR and reorder_line only flips the Arabic run, leaving abc 123 مرحبا instead of moving the LTR token after the RTL base. Determine the base from the visual right edge, or force RTL when appropriate, before reordering.
Useful? React with 👍 / 👎.
| { | ||
| if score >= effective_config.score_threshold { | ||
| result_texts.push(text); | ||
| let (text, changed_order) = postprocess_text_direction_with_order_change(text); |
There was a problem hiding this comment.
In environments where the recognition model already returns Unicode logical RTL strings, such as مرحبا rather than the visual CTC order ابحرم, this unconditional post-processing still runs every above-threshold result through reorder_line; that converts logical storage to display order, so pure RTL text is reversed back to visual order and changed_order also clears word-box metadata. Please keep a no-reorder/default path or apply this only to recognizers configured as emitting visual RTL output.
Useful? React with 👍 / 👎.
|
Closing this PR because we decided bidi/RTL postprocessing should stay outside the OCR core and be handled downstream. |
1 participant
Summary
0.8.0to0.8.1.Root cause
The RTL text recognition path needed logical-order post-processing for visual-order OCR output, and the new Clippy gate flagged one vector-drain pattern during preflight.
Validation
cargo fmt --all -- --checkcargo clippy --all-targets --all -- -D warningscargo clippy --all-targets -p oar-ocr-vl -- -D warningsRUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps