From 7c071e244b0e2923ba2c9fbaa51efcac60731c55 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 14 Jun 2026 16:59:12 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A2=20fix:=20Prevent=20"approximately"?= =?UTF-8?q?=20tildes=20from=20rendering=20as=20markdown=20subscript=20(#13?= =?UTF-8?q?743)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🔢 fix: Prevent "approximately" tildes from rendering as markdown subscript `remark-supersub` splits text nodes on every `~`; an even number of tildes wraps the in-between text in ``. "Approximately" usage like `~50% ... ~10%` pairs up and subscripts everything between the two tildes. A backslash escape cannot fix this: micromark resolves `\~` back to a bare `~` before supersub runs. Instead, `preprocessTilde` swaps approximation tildes (a `~` prefixing a number, not attached to a word) for the Unicode tilde operator `∼` (U+223C), which renders as a tilde but is not split by supersub. Mirrors `preprocessLaTeX`: early return, single regex pass, code-region skipping. Genuine subscripts (`H~2~O`, `a ~2~ b`), strikethrough, escaped tildes, and home paths are preserved. * 🔢 fix: Harden tilde preprocessing — escaped tildes, URLs, math, MarkdownLite Addresses Codex review findings: - Convert escaped approximation tildes too (`\~50%`): markdown decodes `\~` to `~` before supersub, so the escape still pairs into a subscript. - Anchor matches to a prose boundary (start / whitespace / open bracket) and exclude `\(`/`\[`/`\{`, so URL path tildes (`/~50`) and math delimiters (`$~10$`, `$$~10$$`, `\(~10\)`) are left untouched. - Apply preprocessTilde in MarkdownLite (user messages + search/subagent/ code-analysis displays), which also enables remark-supersub. * 🔧 refactor: Neutralize approximation tildes via remark plugin, not raw-text Replaces the string-level preprocessTilde with `remarkApproxTilde`, a remark plugin that rewrites "approximately" tildes (`~50%` → `∼`) on parsed text nodes before remark-supersub runs. Because it operates on the AST, code spans, fenced code blocks (backtick *and* ~~~), inline code with any backtick count, link destinations, and math spans are structurally excluded — none are `text` nodes — resolving every raw-text edge case Codex flagged without region-scanning heuristics. Escaped `\~` is covered for free (markdown decodes it before the plugin runs). - New client/src/utils/tilde.ts: `normalizeApproxTildes` (pure, per-text-node) + `remarkApproxTilde` plugin. - Wired into both renderers (markdownConfig + MarkdownLite), before supersub. - latex.ts / Markdown.tsx reverted to original; preprocessTilde removed. - tilde.spec.ts: pure-function cases + a hand-built-tree test proving code, math, and link URLs are untouched while text (incl. link text) is converted. * 🔧 fix: Cover quoted approximations and the markdown error-boundary fallback - Broaden the boundary to any non-word, non-tilde char (`(?` segments is still subscripted by remark-supersub itself — that's pre-existing behavior (reproduces with no plugin) and out of scope here. * 🔧 refactor: Drop unist-util-visit runtime dep from tilde plugin Replace the `unist-util-visit` import with a small self-contained recursive walk over text nodes (tracking the parent for the autolink-label check). This removes reliance on a transitively-hoisted runtime package — addressing the dependency- hygiene concern without adding a dependency or churning the lockfile. The type-only `unist` import remains (erased at build, no runtime resolution). Behavior is unchanged; verified against nested emphasis and list/paragraph trees. --- .../Content/MarkdownErrorBoundary.tsx | 3 +- .../Chat/Messages/Content/MarkdownLite.tsx | 3 +- .../Chat/Messages/Content/markdownConfig.ts | 3 +- client/src/utils/index.ts | 1 + client/src/utils/tilde.spec.ts | 143 ++++++++++++++++++ client/src/utils/tilde.ts | 77 ++++++++++ 6 files changed, 227 insertions(+), 3 deletions(-) create mode 100644 client/src/utils/tilde.spec.ts create mode 100644 client/src/utils/tilde.ts diff --git a/client/src/components/Chat/Messages/Content/MarkdownErrorBoundary.tsx b/client/src/components/Chat/Messages/Content/MarkdownErrorBoundary.tsx index 9862da2102..08d91a43b5 100644 --- a/client/src/components/Chat/Messages/Content/MarkdownErrorBoundary.tsx +++ b/client/src/components/Chat/Messages/Content/MarkdownErrorBoundary.tsx @@ -5,8 +5,8 @@ import ReactMarkdown from 'react-markdown'; import rehypeHighlight from 'rehype-highlight'; import type { PluggableList } from 'unified'; import { code, codeNoExecution, a, p, table } from './MarkdownComponents'; +import { langSubset, remarkApproxTilde } from '~/utils'; import { CodeBlockProvider } from '~/Providers'; -import { langSubset } from '~/utils'; interface ErrorBoundaryState { hasError: boolean; @@ -61,6 +61,7 @@ class MarkdownErrorBoundary extends React.Component< { @@ -31,6 +31,7 @@ const MarkdownLite = memo( { if (remarkPluginsCache === null) { remarkPluginsCache = [ + remarkApproxTilde, supersub, remarkGfm, remarkDirective, diff --git a/client/src/utils/index.ts b/client/src/utils/index.ts index 0beb890c0b..ea322b047f 100644 --- a/client/src/utils/index.ts +++ b/client/src/utils/index.ts @@ -10,6 +10,7 @@ export * from './email'; export * from './share'; export * from './files'; export * from './latex'; +export * from './tilde'; export * from './forms'; export * from './roles'; export * from './errors'; diff --git a/client/src/utils/tilde.spec.ts b/client/src/utils/tilde.spec.ts new file mode 100644 index 0000000000..fbda8d8258 --- /dev/null +++ b/client/src/utils/tilde.spec.ts @@ -0,0 +1,143 @@ +import type { Node } from 'unist'; +import { normalizeApproxTildes, remarkApproxTilde } from './tilde'; + +const T = '∼'; // U+223C TILDE OPERATOR + +describe('normalizeApproxTildes', () => { + test('returns the same string when no tilde is present', () => { + const content = 'a plain string without any tilde'; + expect(normalizeApproxTildes(content)).toBe(content); + }); + + test('neutralizes a paired approximation that would become subscript', () => { + expect(normalizeApproxTildes('down. ~50% of IPOs ... to ~10%).')).toBe( + `down. ${T}50% of IPOs ... to ${T}10%).`, + ); + }); + + test('neutralizes a single approximation tilde', () => { + expect(normalizeApproxTildes('about ~50 people')).toBe(`about ${T}50 people`); + }); + + test('neutralizes approximation before currency and after an open bracket', () => { + expect(normalizeApproxTildes('range (~$5 to ~10%)')).toBe(`range (${T}$5 to ${T}10%)`); + }); + + test('neutralizes a spaced negative approximation', () => { + expect(normalizeApproxTildes('is ~ -3% from')).toBe(`is ${T} -3% from`); + }); + + test('neutralizes quoted approximations', () => { + expect(normalizeApproxTildes('about "~50%" and "~10%"')).toBe(`about "${T}50%" and "${T}10%"`); + }); + + test('preserves word-attached subscripts', () => { + expect(normalizeApproxTildes('H~2~O and x~1~')).toBe('H~2~O and x~1~'); + }); + + test('preserves a space-delimited closed-number subscript', () => { + expect(normalizeApproxTildes('the term a ~2~ here')).toBe('the term a ~2~ here'); + }); + + test('does not match a tilde attached to a preceding number', () => { + expect(normalizeApproxTildes('range 5~10 items')).toBe('range 5~10 items'); + }); +}); + +interface TextNode extends Node { + type: 'text'; + value: string; +} +interface CodeNode extends Node { + type: 'inlineCode' | 'code'; + value: string; +} +interface MathNode extends Node { + type: 'inlineMath' | 'math'; + value: string; +} +interface LinkNode extends Node { + type: 'link'; + url: string; + children: Node[]; +} + +describe('remarkApproxTilde', () => { + /** + * supersub only splits `text` nodes, so the plugin's contract is to rewrite + * approximation tildes in text nodes while leaving every other node type — code, + * math, and link destinations — byte-for-byte untouched. A hand-built tree proves + * that contract without depending on the parser. + */ + test('rewrites text nodes but leaves code, math, and link URLs untouched', () => { + const text: TextNode = { type: 'text', value: 'about ~50% and ~10% here' }; + const inlineCode: CodeNode = { type: 'inlineCode', value: '~50%' }; + const fenced: CodeNode = { type: 'code', value: '~50% in code\n~/path' }; + const inlineMath: MathNode = { type: 'inlineMath', value: 'x ~ 10' }; + const displayMath: MathNode = { type: 'math', value: '~10' }; + const linkText: TextNode = { type: 'text', value: '~50% off' }; + const link: LinkNode = { + type: 'link', + url: 'https://example.com/~50/file', + children: [linkText], + }; + const tree: Node = { + type: 'root', + children: [text, inlineCode, fenced, inlineMath, displayMath, link], + } as Node; + + remarkApproxTilde()(tree); + + expect(text.value).toBe(`about ${T}50% and ${T}10% here`); + expect(inlineCode.value).toBe('~50%'); + expect(fenced.value).toBe('~50% in code\n~/path'); + expect(inlineMath.value).toBe('x ~ 10'); + expect(displayMath.value).toBe('~10'); + expect(link.url).toBe('https://example.com/~50/file'); + expect(linkText.value).toBe(`${T}50% off`); + }); + + test('preserves an autolink label that displays the URL', () => { + const label: TextNode = { type: 'text', value: 'https://example.com/~50/file' }; + const link: LinkNode = { + type: 'link', + url: 'https://example.com/~50/file', + children: [label], + }; + remarkApproxTilde()({ type: 'root', children: [link] } as Node); + expect(label.value).toBe('https://example.com/~50/file'); + }); + + test('preserves a www autolink label whose href has an implied scheme', () => { + const label: TextNode = { type: 'text', value: 'www.example.com/~50/x' }; + const link: LinkNode = { + type: 'link', + url: 'http://www.example.com/~50/x', + children: [label], + }; + remarkApproxTilde()({ type: 'root', children: [link] } as Node); + expect(label.value).toBe('www.example.com/~50/x'); + }); + + test('still converts a regular link label that is prose', () => { + const label: TextNode = { type: 'text', value: '~50% to ~10% off' }; + const link: LinkNode = { type: 'link', url: 'https://example.com', children: [label] }; + remarkApproxTilde()({ type: 'root', children: [link] } as Node); + expect(label.value).toBe(`${T}50% to ${T}10% off`); + }); + + test('removes both tildes of an approximation pair so no `~` remains to subscript', () => { + const node: TextNode = { type: 'text', value: 'first ~50% then ~10% drop' }; + const tree: Node = { type: 'root', children: [node] } as Node; + remarkApproxTilde()(tree); + expect(node.value).not.toContain('~'); + expect(node.value).toBe(`first ${T}50% then ${T}10% drop`); + }); + + test('leaves a genuine subscript text node intact for supersub to handle', () => { + const node: TextNode = { type: 'text', value: 'water H~2~O molecule' }; + const tree: Node = { type: 'root', children: [node] } as Node; + remarkApproxTilde()(tree); + expect(node.value).toBe('water H~2~O molecule'); + }); +}); diff --git a/client/src/utils/tilde.ts b/client/src/utils/tilde.ts new file mode 100644 index 0000000000..e2b4384ae5 --- /dev/null +++ b/client/src/utils/tilde.ts @@ -0,0 +1,77 @@ +import type { Node } from 'unist'; + +interface TextNode extends Node { + type: 'text'; + value: string; +} +interface LinkNode extends Node { + type: 'link'; + url: string; +} +interface ParentNode extends Node { + children?: Node[]; +} + +/** + * A GFM autolink renders the URL itself as its label, so the label text node equals the + * destination (with an implied scheme for `www.`/email links). Rewriting it would change the + * URL the user sees while leaving the href intact, so these are left untouched. + */ +function isAutolinkLabel(value: string, parent?: Node): boolean { + if (!parent || parent.type !== 'link') { + return false; + } + const url = (parent as LinkNode).url; + return ( + url === value || + url === `http://${value}` || + url === `https://${value}` || + url === `mailto:${value}` + ); +} + +// "Approximately" tilde: a `~` prefixing a number that is not attached to a preceding word — +// e.g. `~50%`, `~ -3%`, `~$5`, `"~50%"`, `(~10%)`. Running on parsed text nodes (never code, +// links, math, or URL destinations) means only real prose is touched, so the boundary can be +// permissive: anything except a word character or another tilde. Word-attached and +// closed-number subscripts (`H~2~O`, `a ~2~ b`) are excluded so genuine subscripts survive. +// Markdown decodes `\~` to `~` before this runs, so escaped tildes are covered too. +const APPROX_TILDE_REGEX = /(?` ranges. + * A backslash escape cannot help — markdown decodes `\~` back to `~` before supersub runs — + * so the glyph itself must change. + */ +export function normalizeApproxTildes(value: string): string { + if (value.indexOf('~') === -1) return value; + return value.replace(APPROX_TILDE_REGEX, TILDE_OPERATOR); +} + +function normalizeTextNodes(node: Node, parent?: Node): void { + if (node.type === 'text') { + const textNode = node as TextNode; + if (typeof textNode.value === 'string' && !isAutolinkLabel(textNode.value, parent)) { + textNode.value = normalizeApproxTildes(textNode.value); + } + return; + } + const children = (node as ParentNode).children; + if (!children) return; + for (const child of children) { + normalizeTextNodes(child, node); + } +} + +/** + * remark plugin that neutralizes "approximately" tildes in text nodes before + * `remark-supersub` runs. Walking the parsed tree means code, links, math, and URL + * destinations are structurally excluded — none of them are `text` nodes. Autolink labels + * (which display the URL itself) are skipped so the visible URL is preserved verbatim. + */ +export function remarkApproxTilde() { + return (tree: Node) => normalizeTextNodes(tree); +}