Repository navigation
Conversation
@tiptap/markdown's escapeMarkdownSyntax backslash-escapes every `_` character unconditionally when re-serializing an edited block. This turns any identifier containing underscores (e.g. `user_name_field`) into `user\_name\_field` on save, which then renders literally with visible backslashes. Patch @tiptap/markdown@3.31.3 (via pnpm patch) so an underscore is only escaped when it is NOT flanked by word characters on both sides, matching CommonMark's intraword-underscore-is-literal rule. Emphasis- style underscores (leading/trailing/whitespace-flanked) keep escaping exactly as before. Scope: this only fixes underscores. Backslash, backtick, asterisk, and brackets are unchanged. Tilde is also left unchanged: GFM strikethrough delimiters in the actual parser (marked) can pair up across arbitrary distance within a text run based on flanking rules, not just adjacent `~~`, so a safe per-character fix for tilde isn't possible without a full delimiter-matching pass - out of scope here. Fixes stablyai#22014 (partially; see scope note above).
Contributor
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughThe TipTap markdown patch now preserves underscores between word characters and escapes underscores in other positions. The patched dependency is registered in the pnpm workspace. New tests cover serialization, markdown round trips, boundary cases, punctuation, and preservation of existing escaping for other markdown syntax characters. Priority: ⬇️ Low Merge Risk: 🔵 Low · up to Identifiers containing non-ASCII letters can still be rewritten with escaped underscores when rich Markdown is serialized. Extend the intraword check before merging to preserve the intended behavior consistently. 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
Full details: Description checkExplanation The description explains the bug, solution, scope, linked issue, implementation, and testing. However, it omits the required Visual Proof section and does not complete the required template sections and checklist items. Resolution Add the required template sections, including Visual Proof with screenshots or an explicit N/A and reason, Agent skill upstream boundary, Notes, and completed Checklist items. Keep the linked issue in the Linked Issue section and document platform testing and command results. Full details: Linked Issues checkExplanation Issue Resolution Extend the markdown serialization fix for the remaining syntax characters and the document-wide corruption behavior described in Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files. (2 skipped: 2 unsupported.)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: e53afed9-4d76-48b4-9434-0d41afe0b92e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
config/patches/@tiptap__markdown@3.31.3.patchpnpm-workspace.yamlsrc/renderer/src/components/editor/rich-markdown-escape-syntax.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.
Comment on lines
+12
to
+15
| + const before = str[offset - 1]; | ||
| + const after = str[offset + 1]; | ||
| + const intraword = before !== void 0 && after !== void 0 && /\w/.test(before) && /\w/.test(after); | ||
| + return intraword ? match : `\\${match}`; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '1,90p' config/patches/@tiptap__markdown@3.31.3.patch
sed -n '1,120p' src/renderer/src/components/editor/rich-markdown-escape-syntax.test.ts
rg -n "escapeMarkdownSyntax|intraword|Unicode|non-ASCII|user_name" config/patches src/renderer/src/components/editorRepository: stablyai/orca
Length of output: 45546
🌐 Web query:
CommonMark specification underscores intraword Unicode alphanumeric characters delimiter run
💡 Result:
<source_evidence>
Citations:
- 1: GitHub issue 496 in commonmark/CommonMark (link omitted to avoid creating a cross-reference)
- 2: GitHub issue 590 in commonmark/commonmark-spec (link omitted to avoid creating a cross-reference)
- 3: https://spec.commonmark.org/spec
- 4: https://spec.commonmark.org/0.31.2/
🏁 Script executed:
#!/bin/bash
set -o pipefail
printf '%s\n' '--- patch matches ---'
rg -n -C 10 'const intraword|before !== void 0|str\[offset' config/patches/@tiptap__markdown@3.31.3.patch
printf '%s\n' '--- patch header and nearby changed hunks ---'
rg -n -C 4 'escapeMarkdownSyntax|intraword|function .*escape|Markdown' config/patches/@tiptap__markdown@3.31.3.patch | head -n 220
printf '%s\n' '--- focused test matches ---'
rg -n -C 8 'escapeMarkdownSyntax|intraword|underscore|non-ASCII|Unicode|user_name|markdown' src/renderer/src/components/editor/rich-markdown-escape-syntax.test.ts
printf '%s\n' '--- test file size ---'
wc -l src/renderer/src/components/editor/rich-markdown-escape-syntax.test.tsRepository: stablyai/orca
Length of output: 8426
🤖 get_repo_knowledge executed:
get_repo_knowledge stablyai/orca /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/learnings /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/conventions
Length of output: 10797
Make the intraword check Unicode- and code-point-aware.
All three patched artifacts use UTF-16 indexing and ASCII-only /\w/. Therefore пристаням_стремятся becomes пристаням\_стремятся, and astral letters also fail because their surrogate code units are not word characters. CommonMark treats an underscore between Unicode letters as literal. Use code-point-aware neighbor lookup and Unicode property checks, such as /[\p{L}\p{N}_]/u, in all three artifacts. Add regression coverage for Cyrillic and astral-letter identifiers.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
The intraword rule uses \w, which matches _. Underscores adjacent to another underscore are left unescaped even though CommonMark treats _ as punctuation, so a text node like __init__ now round-trips into an added italic mark. Details inline.
Reviewed changes — vendor patch to @tiptap/markdown so intraword underscores survive re-serialization, plus its registration, lockfile hash, and regression tests.
- Escaping rule —
escapeMarkdownSyntaxkeeps the old character set and now only leaves_literal when both neighbours match\w. - Packaging — patch registered in
pnpm-workspace.yaml;pnpm-lock.yamlrecords its patch hash and a patched snapshot. The patch applies cleanly on install. - Tests — new
rich-markdown-escape-syntax.test.tsdrives a realEditorand a serialize → reparse round trip; I confirmed it fails (3 tests) when the dist function is reverted, so it is real coverage.
ℹ️ Nitpicks
pnpm-lock.yamlcarries unrelated churn:express-rate-limit: 8.5.2(express@5.2.1)→8.5.2(express@5.2.1(supports-color@7.2.0)). Re-running install from a clean base would drop it.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| + if (match !== "_") return `\\${match}`; | ||
| + const before = str[offset - 1]; | ||
| + const after = str[offset + 1]; | ||
| + const intraword = before !== void 0 && after !== void 0 && /\w/.test(before) && /\w/.test(after); |
Contributor
There was a problem hiding this comment.
\w includes _, so an underscore next to another underscore is classified as intraword and left unescaped — but CommonMark treats _ as punctuation, so such a run can still open or close emphasis. A text node __init__ serializes to \__init_\_, which reparses as literal _ + italic init + _, adding an italic mark the original text did not have. Reverting this patch makes __init__ round-trip literally, so this is a new regression; use a neighbour class that excludes _ (e.g. [A-Za-z0-9]) in all three copies of the function.
Technical details
# intraword check treats `_` as a word character
## Affected sites
- `config/patches/@tiptap__markdown@3.31.3.patch` line 14 (`dist/index.cjs`), line 33 (`dist/index.js`), line 54 (`src/MarkdownManager.ts`) — `\w` is used for both the `before` and `after` neighbour tests.
- `src/renderer/src/components/editor/rich-markdown-escape-syntax.test.ts` — no case covers a doubled underscore at a text boundary, so the suite stays green over the regression.
## Evidence
- `__init__` → `editor.getMarkdown()` = `\__init_\_`; reopening yields `text "_"`, `italic "init"`, `text "_"` (an added mark).
- `__a__`, `__1__`, `a. __b__c` behave the same; the pre-patch regex round-trips all of them (verified against a real `Editor`).
- `\w` matches ASCII letters, digits and `_`. CommonMark classifies `_` as punctuation (Pc), so an underscore run is not necessarily intraword.
## Required outcome
- Underscores adjacent to `_` stay escaped, while `user_name_field` / `a_2_b` / `_word` / `word_` keep their current behaviour.
## Suggested approach
- Use `/[A-Za-z0-9]/` (or a Unicode letter/number class) for the neighbour tests in all three copies. Verified locally: `__init__` → `\_\_init_\_` (round-trips) and `user_name_field` stays literal.
- Add a doubled-underscore regression test (`__init__`, `__a__`).
This was referenced Sep 23, 2026
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ELI5
When you edit a rich-markdown block that contains a word with an underscore in it (like
user_name_field), saving turns it intouser\_name\_field. The visible backslashes then just sit there in the text.Problem
escapeMarkdownSyntaxin@tiptap/markdownbackslash-escapes every_character unconditionally whenever a block gets re-serialized after an edit:Any identifier, filename, or variable name containing an underscore gets corrupted with visible backslashes the moment the containing block is edited and saved.
Solution
Patched
@tiptap/markdown@3.31.3(viapnpm patch, following the project's existingconfig/patches/*.patchconvention) so an underscore is only escaped when it is not flanked by word characters on both sides — matching CommonMark's rule that intraword underscores are literal, not emphasis delimiters:Leading/trailing/whitespace-flanked underscores (the ones that actually could be emphasis delimiters, e.g.
_word,word_,a _ b) still get escaped exactly as before.Why this scope only
The linked issue reports five broken characters:
[,],_,~, and backtick/*. This PR fixes only_.marked, GFM mode) pairs up strikethrough delimiters using left/right-flanking rules that can span arbitrary distance within a text run, not just adjacent~~. A round-trip test caught this —~5 dollars, see v2~betagot silently reparsed as strikethrough even though neither tilde was doubled. Getting this right requires reimplementingmarked's delimiter-matching pass, which is too risky to hand-roll in a vendor patch. Left unchanged (still always escaped, same as before — no regression, no fix).escapeMarkdownSyntaxin ways I didn't fully trace, and touching it without full understanding risks new corruption.Testing
New test file
src/renderer/src/components/editor/rich-markdown-escape-syntax.test.ts, run through a realEditorinstance (same pattern as the existingrich-markdown-literal-serialization.test.ts), including a round-trip check (serialize → reparse → compare JSON) proving no corruption on save:Also re-ran the existing
rich-markdown-literal-serialization.test.tsandrich-markdown-normalize.test.ts(8 tests) to confirm no regression from the patch.Review
config/patches/@tiptap__markdown@3.31.3.patch— the actual fix, applied tosrc/MarkdownManager.ts,dist/index.js, anddist/index.cjsidentically (all three are part of the published package and get patched in sync).pnpm-workspace.yaml— registers the new patch underpatchedDependencies, same convention as the existing@xterm/*/lint-staged/etc. patches.src/renderer/src/components/editor/rich-markdown-escape-syntax.test.ts— new, focused test file.Checklist
pnpm patchFixes #22014 (partial — see "Why this scope only" above).