fix(chat): align messages with their text direction - #7466
ShlomiPorush wants to merge 13 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Warning Your free Security trial is over. An organization admin can activate Security or dismiss this notice. Comment |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
One finding: the new dir="auto" on the .chat-markdown root makes RTL messages render against markdown CSS that is written entirely with physical LTR properties, so lists, task lists, and blockquotes lay out incorrectly for exactly the Hebrew/Arabic content this PR targets. Details inline.
Posted via Macroscope — UI Consistency
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a focused chat-rendering fix that derives message direction while preserving LTR behavior for code, paths, tables, and generated labels across web and mobile. The implementation is scoped to existing Markdown presentation code and includes targeted regression coverage, with no product-default or broader system changes. You can add or adjust custom eligibility rules. Learn more. |
There was a problem hiding this comment.
One remaining RTL layout inconsistency in the markdown renderer; details inline.
Posted via Macroscope — UI Consistency
|
Landing here from #7574, which was closed today pointing at this PR as the review path for RTL chat. Happy to fold my work into yours rather than run a competing PR — the mobile coverage here is something mine never had. One finding worth acting on before this merges. for (const character of text) {
if (!LETTER_CHARACTER.test(character)) continue;
return RTL_SCRIPT_CHARACTER.test(character) ? "rtl" : "ltr";
}In a coding tool, a large share of Hebrew messages open with a Latin technical token — a branch name, a command, a PR reference, an identifier. Those all resolve LTR, and because the direction is applied per message rather than per block, the whole reply flips. I ran your function verbatim against ten realistic assistant replies, next to a dominant-script scorer: The English controls matter as much as the Hebrew ones: "The word שלום means peace" must stay LTR, and does. This is not "any Hebrew character forces RTL", which fails just as badly in the other direction. The change is contained — same signature, same call sites: const RTL_CHAR_G = /[--ۿ܀-ݏיִ-﷿ﹰ-]/g;
const LTR_TOKEN_G = /[A-Za-z][A-Za-z0-9._/\\:-]*/g;
// Identifiers, paths, ALL-CAPS and camelCase are usually code, and shouldn't
// pull a Hebrew sentence to LTR as hard as an ordinary English word does.
function ltrTokenWeight(token: string): number {
if (/[._/\\:]/.test(token)) return 0.25;
if (/^[A-Z0-9-]{2,}$/.test(token)) return 0.5;
if (/^[a-z]+[A-Z]/.test(token)) return 0.5;
return 1;
}
export function resolveTextDirection(text: string): TextDirection {
const rtl = text.match(RTL_CHAR_G) ?? [];
if (rtl.length === 0) return "ltr";
const tokens = text.match(LTR_TOKEN_G) ?? [];
if (tokens.length === 0) return "rtl";
let ltrScore = 0;
for (const token of tokens) ltrScore += ltrTokenWeight(token);
return rtl.length > ltrScore ? "rtl" : "ltr"; // a tie goes to ltr
}Keep Two smaller things from my own attempt, take or leave:
The weights come from motcke/cursor-ext-rtl (Apache-2.0), which solved the same problem for Cursor's chat panel. Glad to open a PR against your branch if that's easier than patching it in. |
|
@nioasoft I have fixed them in my fix that I run locally. |
|
@ShlomiPorush They are — and they said so by name. When they closed my #7574 today they wrote:
So this PR is not one of several open RTL attempts they might get to. It is the one they picked and deliberately kept while clearing the rest of the backlog. That is a stronger signal than silence on the thread suggests, and worth knowing before you decide how much more to invest. The one thing I would change: land it in pieces rather than growing this branch until it covers tables, questions and plans all at once. It is already Happy to help rather than just comment. I have working versions of two of the surfaces you mention:
Both were reviewed on #7574 and carry tests. Say the word and I will open a PR against your branch with whichever is useful, or just paste the diffs here — your call, it is your PR and I would rather add to it than fork the effort. |
|
@ShlomiPorush Would you push the local fixes you mentioned — tables, questions, plans — somewhere I can pull from? A branch on your fork is enough; it does not have to be PR-ready, or even tidy. Two reasons, and the first one is what I can actually give you back. I run a local T3 Code build with RTL patched in and use it in Hebrew all day, for real work. That is the thing your branch does not have and cannot easily get: a reviewer will look at a diff, but nobody is going to live in it for a week and find that a bold heading hugs the wrong edge, or that a status table's label column changes sides row to row, or that the caret drifts away from the glyphs in a mixed-language line. Every one of those took me days to notice, and none of them were visible in a screenshot — I got them wrong repeatedly until I stopped judging by eye and started measuring. Point me at your branch and I will run it against real Hebrew sessions and report back concretely: which case, what I expected, what I got. Second, more bluntly: unmerged work on a laptop tends to stay there. This PR has not moved in nine days and the team said it is the RTL path they kept, so whatever you have locally is currently the most advanced RTL work in this project and it exists in exactly one place. Happy to go the other way too — I said earlier I would open a PR against your branch with the table direction handling and the leaf-block bidi rules from #7574. That offer stands whenever you want it, and it is easier for me to rebase onto yours than the reverse. If you would rather not publish half-finished work, no problem at all — even a description of which surfaces you touched and how would save me from re-deriving it. |
|
@nioasoft Let's work on this together and get it landed step by step for everyone's benefit. I appreciate your willingness to help and the work you've already done. One challenge for me is that it's quite difficult to have an ongoing technical discussion inside a GitHub PR thread, especially as the conversation grows. Would you be open to moving the discussion to another channel, such as Telegram? I'm available at @ShlomiMe. Looking forward to working together on this BTW my fix is on git https://github.com/ShlomiPorush/t3code-rtl-fix |
|
here are some screenshots |
…ction # Conflicts: # apps/mobile/src/features/threads/ThreadFeed.tsx # apps/web/src/components/ChatMarkdown.tsx # apps/web/src/index.css
There was a problem hiding this comment.
One finding: the new hard-coded footnote list gutter overrides the --list-gutter contract for footnote lists.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
One finding on the logical-property migration in apps/web/src/index.css. The rest of the web changes look consistent: the alert chrome now uses border-s-2 ps-3 (matching DiffCommentAnnotation's existing border-s-2 house style), list/blockquote/task-checkbox gutters translate 1:1 to logical properties, and the forced dir="ltr" on code, pre, tables and file chips correctly excludes those subtrees from the root dir="auto" resolution (the HTML dir=auto algorithm skips descendants that carry their own dir), which mirrors the code-stripping the mobile helper does.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
One finding on the GitHub-alert title: forcing dir="ltr" on the title paragraph pins it to the left edge of an otherwise RTL note block, detaching it from the border-s-2 rail this PR just made direction-aware. The rest of the web changes (logical properties in index.css, dir="ltr" on code/pre/table/file chips, dir="auto" on cells and the markdown root) look consistent.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4adb4fe. Configure here.
|
Re-measured against your current engine ( Two of the five I reported earlier now pass, both thanks to your stripping: the fenced The remaining five are the same ones, and they share one shape: a technical token that is not inside a code span. The English controls are the other half of the point: all four pass under both rules. This is not "any RTL character wins", which fails just as badly in the opposite direction. The change is contained — same signature, same call sites, and it composes with your const RTL_CHAR_G = /[--ۿ܀-ݏיִ-﷿ﹰ-]/g;
const LTR_TOKEN_G = /[A-Za-z][A-Za-z0-9._/\\:-]*/g;
// Identifiers, paths, ALL-CAPS and camelCase are usually code, and shouldn't
// pull a Hebrew sentence to LTR as hard as an ordinary English word does.
function ltrTokenWeight(token: string): number {
if (/[._/\\:]/.test(token)) return 0.25;
if (/^[A-Z0-9-]{2,}$/.test(token)) return 0.5;
if (/^[a-z]+[A-Z]/.test(token)) return 0.5;
return 1;
}
export function resolveTextDirection(text: string): TextDirection {
const rtl = text.match(RTL_CHAR_G) ?? [];
if (rtl.length === 0) return "ltr";
const tokens = text.match(LTR_TOKEN_G) ?? [];
if (tokens.length === 0) return "rtl";
let ltrScore = 0;
for (const token of tokens) ltrScore += ltrTokenWeight(token);
return rtl.length > ltrScore ? "rtl" : "ltr"; // a tie goes to ltr
}Weights from motcke/cursor-ext-rtl (Apache-2.0), which solved the same problem for Cursor's chat panel. Their RTL multiplier is 1.5 with ties going to RTL; a 1.0 multiplier with ties to LTR measured better here and never flips English. Say the word and I will open a PR against your branch with this plus the twelve cases as tests — or paste the diff here, whichever you prefer. It is your PR and I would rather add to it. One separate thing worth knowing, since your repo does it too: do not put |
There was a problem hiding this comment.
One remaining physical-direction leftover inside the now dir="auto" markdown root. Everything else in the web direction work (logical list/blockquote/table CSS, dir="ltr" isolation for code, file-link chips and table structure, <bdi> alert label) looks internally consistent.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
One finding: the new dir="auto" message direction still counts a synthesized chip label, so skill-mention messages resolve to the wrong direction. Details inline.
Posted via Macroscope — UI Consistency
|
Note 🤖 GPT-6 Astra (preview) responding on behalf of Theo This note is part of an automated cleanup pass. Preserve the web cases from #7126 at head 9481d7ce. Its outermost-block approach lets Arabic and English paragraphs resolve separately, keeps list markers with their text, and gives alert bodies direction under LTR labels. It adds automatic direction to sidebar titles, rename inputs, the chat header, and command-palette text. Compare these cases with the current design before merge. Keep code and table scroll wrappers LTR. Check both tight and loose lists with a bilingual reviewer. No per-block or title code was transferred by this cleanup. |
|
@ShlomiPorush please fix conflicts |
|
@t3dotgg please fix conflicts |
|
@haithamassoli-plus-connect I don't mind to fix them but to do this for every version with no merge it's just a waste of token. |
4 participants
Problem
Hebrew and Arabic chat messages currently inherit the app's left-to-right direction, so their text starts from the wrong edge. This change lets each message derive its direction from its own content without affecting English messages or reversing code, commands, and file paths.
What Changed
dir="auto".Why
This keeps the change at the message-rendering boundary. English messages continue to render left-to-right, RTL messages begin on the right, and mixed prose relies on the platform bidi algorithm while code remains source-ordered.
Related work exists in #6575 and #7126. This PR is deliberately narrower: it fixes chat message bodies on current
mainand includes the observed Android paragraph-container issue rather than assuming Android's first-strong text behavior also aligns the surrounding flex row.UI Changes
Same seeded thread, theme, viewport, and message. The only difference in the “before” capture is the absence of automatic message direction.
The after capture also includes Hebrew and English in the same response. Both fenced code blocks and inline code remain left-to-right.
Verification
vp test run apps/web/src/components/chat/MessagesTimeline.test.tsx apps/web/src/components/ChatMarkdown.test.tsx apps/mobile/src/lib/textDirection.test.ts— 94 passing tests.vp run --filter @t3tools/web typecheck.vp run --filter @t3tools/mobile typecheck.Android was not run on a device or emulator because this environment does not have an Android SDK. The Android path is covered by the direction helper tests and the mobile typecheck. The native iOS renderer is unchanged.
Checklist
Note
Low Risk
Rendering-only bidi and CSS logical-property changes with targeted tests; mobile renderer shape changes are confined to ThreadFeed’s markdown style setup.
Overview
Chat message bodies now pick LTR vs RTL from message content instead of inheriting the app’s default direction, while code, paths, and tables stay source-ordered LTR.
Mobile: Adds
resolveTextDirection/resolveMarkdownNodeTextDirection(first strong letter in prose; skips code/images and GitHub alert markers). Non-native markdown paths use newNitroMarkdownMessage, which parses once, setsdirectionon document/paragraph/list/blockquote styles (RTL blockquotes flip border/padding), and picksrenderers[direction](lists get mirrored gutters; inline code and fenced code usewritingDirection: "ltr").Web: Root chat markdown uses
dir="auto"; code blocks,prefallbacks, file chips, and table wrappers usedir="ltr"; table cells usedir="auto". List/blockquote/task-list/alert chrome moves to logical CSS (padding-inline-start,border-s,text-start); GitHub alert titles wrap in<bdi>; skill chip labels use<bdi>.Regression tests cover Hebrew/Arabic/English, mixed RTL prose with LTR code, tables, and alerts.
Reviewed by Cursor Bugbot for commit f124b9f. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Align chat messages with their text direction on mobile and web
NitroMarkdownMessagewhich parses the markdown AST, resolves direction, and applies direction-scoped renderers and mirrored blockquote styling; code blocks and inline code are forced LTRdir="auto"on the chat markdown root,dir="ltr"on code blocks, tables, file links, and inline code; switches chat markdown CSS to logical properties (padding-inline-start,border-inline-start,text-align: start); wraps alert labels and skill chip labels in<bdi>MarkdownStyleSet.rendererstype changes fromCustomRendererstoReadonly<Record<TextDirection, CustomRenderers>>; any consumer constructing a style set directly must provide bothltrandrtlrenderer mapsMacroscope summarized f124b9f.