Skip to content

[codex] Fix RTL chat message rendering #3674

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
ekrako wants to merge 1 commit into
base: main
Choose a base branch
from
+368 −11
Draft

[codex] Fix RTL chat message rendering #3674

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions apps/web/src/components/ChatMarkdown.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import { EnvironmentId } from "@t3tools/contracts";
import { renderToStaticMarkup } from "react-dom/server";
import { describe, expect, it, vi } from "vite-plus/test";

import ChatMarkdown from "./ChatMarkdown";

vi.mock("@effect/atom-react", () => ({
useAtomValue: () => ({ availableEditors: [] }),
}));

vi.mock("../hooks/useTheme", () => ({
useTheme: () => ({ resolvedTheme: "light" }),
}));

vi.mock("../editorPreferences", () => ({
useOpenInPreferredEditor: () => () => {},
}));

vi.mock("../state/assets", () => ({
assetEnvironment: { createUrl: {} },
}));

vi.mock("../state/entities", () => ({
useActiveEnvironmentId: () => EnvironmentId.make("environment-local"),
}));

vi.mock("../state/preview", () => ({
previewEnvironment: { open: {} },
}));

vi.mock("../state/server", () => ({
serverEnvironment: { configValueAtom: () => ({}) },
}));

vi.mock("../state/session", () => ({
usePreparedConnection: () => ({ _tag: "None" }),
}));

vi.mock("../state/use-atom-command", () => ({
useAtomCommand: () => () => {},
}));

vi.mock("../state/use-atom-query-runner", () => ({
useAtomQueryRunner: () => () => "",
}));

describe("ChatMarkdown bidi list rendering", () => {
it("renders code-prefixed Hebrew unordered lists as RTL", () => {
const markup = renderToStaticMarkup(
<ChatMarkdown
text={[
"- **`apps/web`**: אפליקציית React/Vite שמציגה צ'אט",
"- **`apps/server`**: שרת Node.js שמריץ CLI",
].join("\n")}
cwd={undefined}
/>,
);

expect(markup).toContain('<ul dir="rtl">');
expect(markup).toContain('<li dir="rtl">');
});

it("keeps English unordered lists LTR", () => {
const markup = renderToStaticMarkup(
<ChatMarkdown
text={["- README.md", "- apps/server/package.json"].join("\n")}
cwd={undefined}
/>,
);

expect(markup).toContain('<ul dir="ltr">');
expect(markup).toContain('<li dir="ltr">');
});
});
110 changes: 107 additions & 3 deletions apps/web/src/components/ChatMarkdown.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import { useOpenInPreferredEditor } from "../editorPreferences";
import { resolveDiffThemeName, type DiffThemeName } from "../lib/diffRendering";
import { fnv1a32 } from "../lib/diffRendering";
import { LRUCache } from "../lib/lruCache";
import { dirFor, dirForMarkdown } from "../lib/rtl";
import { useTheme } from "../hooks/useTheme";
import { getClientSettings } from "../hooks/useSettings";
import {
Expand Down Expand Up @@ -242,6 +243,38 @@ function nodeToPlainText(node: ReactNode): string {
return "";
}

function blockDir(children: ReactNode): "rtl" | "ltr" {
return dirFor(nodeToPlainText(children));
}

type MarkdownNodeWithPosition = {
position?:
| {
start?: { offset?: number | undefined } | undefined;
end?: { offset?: number | undefined } | undefined;
}
| undefined;
};

function blockDirForMarkdownNode(
markdown: string,
node: MarkdownNodeWithPosition | undefined,
children: ReactNode,
): "rtl" | "ltr" {
const start = node?.position?.start?.offset;
const end = node?.position?.end?.offset;
if (
typeof start === "number" &&
typeof end === "number" &&
start >= 0 &&
end > start &&
end <= markdown.length
) {
return dirForMarkdown(markdown.slice(start, end));
}
return blockDir(children);
}

function extractCodeBlock(
children: ReactNode,
): { className: string | undefined; code: string } | null {
Expand Down Expand Up @@ -1328,19 +1361,90 @@ function ChatMarkdown({
);
const markdownComponents = useMemo<Components>(
() => ({
p({ node: _node, children, ...props }) {
return <p {...props}>{renderSkillInlineMarkdownChildren(children, skills)}</p>;
p({ node, children, ...props }) {
return (
<p {...props} dir={blockDirForMarkdownNode(text, node, children)}>
{renderSkillInlineMarkdownChildren(children, skills)}
</p>
);
},
li({ node, children, ...props }) {
const listItemStart = node?.position?.start.offset;
const markerOffset =
typeof listItemStart === "number" ? findTaskListMarkerOffset(text, listItemStart) : null;
return (
<li {...props} data-task-marker-offset={markerOffset ?? undefined}>
<li
{...props}
dir={blockDirForMarkdownNode(text, node, children)}
data-task-marker-offset={markerOffset ?? undefined}
>
{renderSkillInlineMarkdownChildren(children, skills)}
</li>
);
},
h1({ node, children, ...props }) {
return (
<h1 {...props} dir={blockDirForMarkdownNode(text, node, children)}>
{children}
</h1>
);
},
h2({ node, children, ...props }) {
return (
<h2 {...props} dir={blockDirForMarkdownNode(text, node, children)}>
{children}
</h2>
);
},
h3({ node, children, ...props }) {
return (
<h3 {...props} dir={blockDirForMarkdownNode(text, node, children)}>
{children}
</h3>
);
},
h4({ node, children, ...props }) {
return (
<h4 {...props} dir={blockDirForMarkdownNode(text, node, children)}>
{children}
</h4>
);
},
h5({ node, children, ...props }) {
return (
<h5 {...props} dir={blockDirForMarkdownNode(text, node, children)}>
{children}
</h5>
);
},
h6({ node, children, ...props }) {
return (
<h6 {...props} dir={blockDirForMarkdownNode(text, node, children)}>
{children}
</h6>
);
},
ul({ node, children, ...props }) {

@macroscopeapp macroscopeapp Bot Jul 3, 2026

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.

🟡 Medium components/ChatMarkdown.tsx:1427

The ul, ol, and li component overrides compute dir by calling blockDirForMarkdownNode(text, node, children), but a list node's position spans the entire subtree including nested lists. A top-level English list that contains a long nested Hebrew/Arabic sublist will resolve the outer <ul>/<ol> and parent <li> as dir="rtl", placing the top-level bullets/numbers on the wrong side even though the parent items are LTR. Consider computing the direction from only the node's own direct text rather than the full subtree range.

🤖 Copy this AI Prompt to have your agent fix this: 
In file @apps/web/src/components/ChatMarkdown.tsx around line 1427:

The `ul`, `ol`, and `li` component overrides compute `dir` by calling `blockDirForMarkdownNode(text, node, children)`, but a list node's `position` spans the entire subtree including nested lists. A top-level English list that contains a long nested Hebrew/Arabic sublist will resolve the outer `<ul>`/`<ol>` and parent `<li>` as `dir="rtl"`, placing the top-level bullets/numbers on the wrong side even though the parent items are LTR. Consider computing the direction from only the node's own direct text rather than the full subtree range.

return (
<ul {...props} dir={blockDirForMarkdownNode(text, node, children)}>
{children}
</ul>
);
},
ol({ node, children, ...props }) {
return (
<ol {...props} dir={blockDirForMarkdownNode(text, node, children)}>
{children}
</ol>
);
},
blockquote({ node, children, ...props }) {
return (
<blockquote {...props} dir={blockDirForMarkdownNode(text, node, children)}>
{children}
</blockquote>
);
},
input({ node: _node, type, checked, disabled: _disabled, ...props }) {
if (type !== "checkbox" || !onTaskListChange) {
return (
Expand Down
23 changes: 22 additions & 1 deletion apps/web/src/components/ComposerPromptEditor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ import {
INLINE_TERMINAL_CONTEXT_PLACEHOLDER,
type TerminalContextDraft,
} from "~/lib/terminalContext";
import { dirFor } from "~/lib/rtl";
import { cn } from "~/lib/utils";
import { basenameOfPath } from "~/pierre-icons";
import {
Expand Down Expand Up @@ -1383,6 +1384,25 @@ function ComposerSurroundSelectionPlugin(props: {
return null;
}

function ComposerBidiDirectionPlugin() {
const [editor] = useLexicalComposerContext();
useEffect(() => {
const syncRootDirection = (text: string) => {
const root = editor.getRootElement();
if (!root) return;
const dir = dirFor(text);
if (root.getAttribute("dir") !== dir) root.setAttribute("dir", dir);
};

editor.getEditorState().read(() => {
syncRootDirection($getRoot().getTextContent());
});

return editor.registerTextContentListener(syncRootDirection);
}, [editor]);
return null;
}

function ComposerPromptEditorInner({
value,
cursor,
Expand Down Expand Up @@ -1610,7 +1630,7 @@ function ComposerPromptEditorInner({
contentEditable={
<ContentEditable
className={cn(
"block max-h-50 min-h-17.5 w-full overflow-y-auto whitespace-pre-wrap wrap-break-word bg-transparent text-[16px] leading-relaxed text-foreground focus:outline-none sm:text-[14px]",
"bidi-plaintext block max-h-50 min-h-17.5 w-full overflow-y-auto whitespace-pre-wrap wrap-break-word bg-transparent text-[16px] leading-relaxed text-foreground focus:outline-none sm:text-[14px]",
className,
)}
data-testid="composer-editor"
Expand All @@ -1634,6 +1654,7 @@ function ComposerPromptEditorInner({
<ComposerInlineTokenArrowPlugin />
<ComposerInlineTokenSelectionNormalizePlugin />
<ComposerInlineTokenBackspacePlugin />
<ComposerBidiDirectionPlugin />
<HistoryPlugin />
</div>
</ComposerTerminalContextActionsContext>
Expand Down
11 changes: 9 additions & 2 deletions apps/web/src/components/chat/MessagesTimeline.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
workLogEntryIsToolLike,
} from "../../session-logic";
import { type TurnDiffSummary } from "../../types";
import { dirFor } from "../../lib/rtl";
import { summarizeTurnDiffStats } from "../../lib/turnDiffTree";
import {
getRenderablePatch,
Expand Down Expand Up @@ -1561,7 +1562,10 @@ const UserMessageBody = memo(function UserMessageBody(props: {
}

return (
<div className="whitespace-pre-wrap wrap-break-word text-sm leading-relaxed text-foreground">
<div
dir={dirFor(props.text)}

@macroscopeapp macroscopeapp Bot Jul 3, 2026

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.

🟡 Medium chat/MessagesTimeline.tsx:1566

UserMessageBody computes the wrapper dir from dirFor(props.text) alone, so when the message body is empty or neutral/LTR but the terminalContexts chip labels are RTL, the wrapper stays ltr and the inline chips are ordered and aligned incorrectly. Consider deriving dir from the combined text of props.text and the terminal context headers (e.g. dirFor(props.text + inlinePrefix)) so RTL chip content is respected.

Also found in 1 other location(s)

apps/web/src/components/ComposerPromptEditor.tsx:1398

ComposerBidiDirectionPlugin derives the editor direction from $getRoot().getTextContent(), but inline token nodes do not expose their rendered chip labels there. For example, ComposerTerminalContextNode.getTextContent() returns only INLINE_TERMINAL_CONTEXT_PLACEHOLDER, while the visible chip label comes from formatTerminalContextLabel(context). A composer containing only RTL terminal-context chips (or RTL display labels on other chips) will therefore keep dir=&#34;ltr&#34;, so the new bidi fix fails for those prompts.

🤖 Copy this AI Prompt to have your agent fix this: 
In file @apps/web/src/components/chat/MessagesTimeline.tsx around line 1566:

`UserMessageBody` computes the wrapper `dir` from `dirFor(props.text)` alone, so when the message body is empty or neutral/LTR but the `terminalContexts` chip labels are RTL, the wrapper stays `ltr` and the inline chips are ordered and aligned incorrectly. Consider deriving `dir` from the combined text of `props.text` and the terminal context headers (e.g. `dirFor(props.text + inlinePrefix)`) so RTL chip content is respected.

Also found in 1 other location(s):
- apps/web/src/components/ComposerPromptEditor.tsx:1398 -- `ComposerBidiDirectionPlugin` derives the editor direction from `$getRoot().getTextContent()`, but inline token nodes do not expose their rendered chip labels there. For example, `ComposerTerminalContextNode.getTextContent()` returns only `INLINE_TERMINAL_CONTEXT_PLACEHOLDER`, while the visible chip label comes from `formatTerminalContextLabel(context)`. A composer containing only RTL terminal-context chips (or RTL display labels on other chips) will therefore keep `dir="ltr"`, so the new bidi fix fails for those prompts.

className="bidi-plaintext whitespace-pre-wrap wrap-break-word text-sm leading-relaxed text-foreground"
>
{inlineNodes}
</div>
);
Expand Down Expand Up @@ -1599,7 +1603,10 @@ const UserMessageBody = memo(function UserMessageBody(props: {
}

return (
<div className="whitespace-pre-wrap wrap-break-word text-sm leading-relaxed text-foreground">
<div
dir={dirFor(props.text)}
className="bidi-plaintext whitespace-pre-wrap wrap-break-word text-sm leading-relaxed text-foreground"
>
{inlineNodes}
</div>
);
Expand Down
39 changes: 34 additions & 5 deletions apps/web/src/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,12 +472,12 @@ label:has(> select#reasoning-effort) select {
}

.chat-markdown ul {
padding-left: 1.25rem;
padding-inline-start: 1.25rem;
list-style-type: disc;
}

.chat-markdown ol {
padding-left: 1.25rem;
padding-inline-start: 1.25rem;
list-style-type: decimal;
}

Expand Down Expand Up @@ -507,7 +507,8 @@ label:has(> select#reasoning-effort) select {
}

.chat-markdown li.task-list-item input[type="checkbox"] {
margin: 0 0.35em 0.15em -1.25rem;
margin-block: 0 0.15em;
margin-inline: -1.25rem 0.35em;
vertical-align: middle;
}

Expand Down Expand Up @@ -537,11 +538,39 @@ label:has(> select#reasoning-effort) select {
}

.chat-markdown blockquote {
border-left: 2px solid var(--border);
padding-left: 0.8rem;
border-inline-start: 2px solid var(--border);
padding-inline-start: 0.8rem;
color: var(--muted-foreground);
}

/* Bidi: each line resolves its own base direction (UAX#9 plaintext). */
.bidi-plaintext {

@macroscopeapp macroscopeapp Bot Jul 3, 2026

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.

🟡 Medium src/index.css:547

unicode-bidi: plaintext on .bidi-plaintext and the markdown block selectors makes the browser infer direction from the text's first strong character, ignoring the explicit dir attribute set on those elements. As a result, RTL messages whose content begins with LTR characters (e.g. code-prefixed Hebrew) are rendered LTR regardless of the dir value. Consider using unicode-bidi: isolate instead, which still isolates the element's direction from surrounding text but honors the explicit dir attribute.

🤖 Copy this AI Prompt to have your agent fix this: 
In file @apps/web/src/index.css around line 547:

`unicode-bidi: plaintext` on `.bidi-plaintext` and the markdown block selectors makes the browser infer direction from the text's first strong character, ignoring the explicit `dir` attribute set on those elements. As a result, RTL messages whose content begins with LTR characters (e.g. code-prefixed Hebrew) are rendered LTR regardless of the `dir` value. Consider using `unicode-bidi: isolate` instead, which still isolates the element's direction from surrounding text but honors the explicit `dir` attribute.

unicode-bidi: plaintext;
}

/* Per-line direction inside markdown text blocks. */
.chat-markdown p,
.chat-markdown li,
.chat-markdown h1,
.chat-markdown h2,
.chat-markdown h3,
.chat-markdown h4,
.chat-markdown h5,
.chat-markdown h6,
.chat-markdown blockquote {
unicode-bidi: plaintext;
}

/* Source code stays LTR even inside RTL messages. */
.chat-markdown pre,

@macroscopeapp macroscopeapp Bot Jul 3, 2026

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.

🟡 Medium src/index.css:565

Setting direction: ltr on .chat-markdown .chat-markdown-table-container forces every markdown table to render left-to-right, so an RTL table (e.g. Hebrew/Arabic content) has its column order and cell direction reversed instead of following the surrounding message direction. Unlike pre/code, tables are general markdown content, so forcing LTR is wrong here. Consider removing .chat-markdown-table-container from the direction: ltr rule and instead letting it inherit the bidi context, keeping only unicode-bidi: isolate if needed.

🤖 Copy this AI Prompt to have your agent fix this: 
In file @apps/web/src/index.css around line 565:

Setting `direction: ltr` on `.chat-markdown .chat-markdown-table-container` forces every markdown table to render left-to-right, so an RTL table (e.g. Hebrew/Arabic content) has its column order and cell direction reversed instead of following the surrounding message direction. Unlike `pre`/`code`, tables are general markdown content, so forcing LTR is wrong here. Consider removing `.chat-markdown-table-container` from the `direction: ltr` rule and instead letting it inherit the bidi context, keeping only `unicode-bidi: isolate` if needed.

.chat-markdown code,
.chat-markdown .chat-markdown-codeblock,
.chat-markdown .chat-markdown-shiki,
.chat-markdown .chat-markdown-table-container {
direction: ltr;
unicode-bidi: isolate;
}

.chat-markdown section[data-footnotes] {
margin-top: 1.25rem;
border-top: 1px solid var(--border);
Expand Down
Loading
Loading