perf: bound rich Markdown comment detection and preservation scans (#20287)

config/scripts/rich-markdown-comment-scan-benchmark.mjs

@@ -0,0 +1,104 @@
+import assert from 'node:assert/strict'
+import { execFileSync } from 'node:child_process'
+import { readFileSync } from 'node:fs'
+import { dirname, resolve } from 'node:path'
+import { performance } from 'node:perf_hooks'
+import { build } from 'esbuild'
+import { buildCounterbalancedSchedule } from './counterbalanced-benchmark-schedule.mjs'
+
+const baseline = process.argv[2] ?? '20ab9950654'
+const file = 'src/renderer/src/components/editor/markdown-rich-mode.ts'
+async function load(contents) {
+  const result = await build({
+    stdin: { contents, loader: 'ts', resolveDir: dirname(resolve(file)) },
+    bundle: true,
+    platform: 'node',
+    format: 'esm',
+    write: false,
+    plugins: [
+      {
+        name: 'cached-round-trip',
+        setup(bundler) {
+          bundler.onResolve({ filter: /markdown-round-trip$|^@\/i18n\/i18n$/ }, (args) => ({
+            path: args.path,
+            namespace: 'bench'
+          }))
+          bundler.onLoad({ filter: /.*/, namespace: 'bench' }, (args) => ({
+            contents: args.path.endsWith('markdown-round-trip')
+              ? 'export const getRichMarkdownRoundTripOutput = (content) => content'
+              : 'export const translate = (_key, fallback) => fallback',
+            loader: 'js'
+          }))
+        }
+      }
+    ]
+  })
+  return import(
+    `data:text/javascript;base64,${Buffer.from(result.outputFiles[0].text).toString('base64')}`
+  )
+}
+const arms = {
+  before: await load(execFileSync('git', ['show', `${baseline}:${file}`], { encoding: 'utf8' })),
+  after: await load(readFileSync(file, 'utf8'))
+}
+const results = []
+for (const [name, content] of [
+  ['plain', '# Heading\nOrdinary prose with <placeholder>.'],
+  ['complete', '<!-- metadata --><span>text</span>'],
+  ['unclosed-1000', '<!--x'.repeat(1000)],
+  ['unclosed-8000', '<!--x'.repeat(8000)],
+  ['preserved-html-and-unclosed-8000', `<span>text</span>${'<!--x'.repeat(8000)}<b>tail</b>`]
+]) {
+  assert.equal(
+    arms.after.getMarkdownRichModeUnsupportedReason(content),
+    arms.before.getMarkdownRichModeUnsupportedReason(content)
+  )
+  const iterations = name.includes('unclosed') ? 2 : 100
+  function run(arm) {
+    global.gc?.()
+    const start = performance.now()
+    const cpuStart = process.cpuUsage()
+    for (let i = 0; i < iterations; i++) {
+      arms[arm].getMarkdownRichModeUnsupportedReason(content)
+    }
+    const cpu = process.cpuUsage(cpuStart)
+    return {
+      ms: (performance.now() - start) / iterations,
+      cpuMs: (cpu.user + cpu.system) / 1000 / iterations
+    }
+  }
+  run('before')
+  run('after')
+  const samples = { before: [], after: [] }
+  for (const pair of buildCounterbalancedSchedule(10, 'before', 'after')) {
+    for (const arm of pair) {
+      samples[arm].push(run(arm))
+    }
+  }
+  const median = (values) => {
+    const sorted = [...values].sort((a, b) => a - b)
+    return (sorted[4] + sorted[5]) / 2
+  }
+  results.push({
+    name,
+    beforeCpuMs: median(samples.before.map((s) => s.cpuMs)),
+    afterCpuMs: median(samples.after.map((s) => s.cpuMs)),
+    beforeMs: median(samples.before.map((s) => s.ms)),
+    afterMs: median(samples.after.map((s) => s.ms)),
+    samples
+  })
+}
+console.log(
+  JSON.stringify(
+    {
+      baseline,
+      node: process.version,
+      platform: process.platform,
+      roundTrip:
+        'identity stub, modeling an already-cached lossless round trip; editor parsing excluded',
+      results
+    },
+    null,
+    2
+  )
+)

src/renderer/src/components/editor/markdown-rich-comment-scan.test.ts

@@ -0,0 +1,61 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import { getMarkdownRichModeUnsupportedReason } from './markdown-rich-mode'
+import { getRichMarkdownRoundTripOutput } from './markdown-round-trip'
+
+vi.mock('./markdown-round-trip', () => ({
+  getRichMarkdownRoundTripOutput: vi.fn((content: string) => content)
+}))
+beforeEach(() =>
+  vi
+    .mocked(getRichMarkdownRoundTripOutput)
+    .mockReset()
+    .mockImplementation((text) => text)
+)
+afterEach(() => vi.restoreAllMocks())
+
+describe('rich Markdown comment scanning', () => {
+  it.each([
+    ['plain <placeholder>', null],
+    ['<!--unclosed<!--again', null],
+    ['<!--->', null],
+    ['<!---->', 'html-or-jsx'],
+    ['<!--outer<!--inner-->tail', 'html-or-jsx'],
+    ['<!--unclosed<span>text</span>', 'html-or-jsx'],
+    ['<custom x="<!--complete-->">', 'html-or-jsx'],
+    ['`<!--complete-->`', null],
+    ['```html\n<!--complete-->\n```', null],
+    ['<!--complete-->\n[a]: https://example.com', 'reference-links'],
+    ['<!--complete-->\n[^a]: footnote', 'reference-links']
+  ] as const)('preserves the decision for %j', (content, expected) => {
+    vi.mocked(getRichMarkdownRoundTripOutput).mockReturnValue(null)
+    expect(getMarkdownRichModeUnsupportedReason(content)).toBe(expected)
+  })
+
+  it('does not pass unclosed comment openers through a comment regex', () => {
+    const input = '<!--x'.repeat(8000)
+    const matchAll = vi.spyOn(String.prototype, 'matchAll')
+    const result = getMarkdownRichModeUnsupportedReason(input)
+    const commentScans = matchAll.mock.calls.filter(
+      ([pattern]) => pattern instanceof RegExp && pattern.source.includes('<!--')
+    ).length
+    expect(result).toBeNull()
+    expect(commentScans).toBe(0)
+    expect(getRichMarkdownRoundTripOutput).not.toHaveBeenCalled()
+  })
+
+  it('continues preserving tags after unmatched comments without repeated closer searches', () => {
+    const input = `<span>before</span>${'<!--x'.repeat(8000)}<b>after</b>`
+    const indexOf = vi.spyOn(String.prototype, 'indexOf')
+    const includes = vi.spyOn(String.prototype, 'includes')
+    const result = getMarkdownRichModeUnsupportedReason(input)
+    const closerSearches = [...indexOf.mock.calls, ...includes.mock.calls].filter(
+      ([needle]) => needle === '-->'
+    ).length
+    expect(result).toBeNull()
+    expect(closerSearches).toBeLessThanOrEqual(1)
+    vi.mocked(getRichMarkdownRoundTripOutput).mockReturnValue(
+      input.replace('<b>after</b>', 'after')
+    )
+    expect(getMarkdownRichModeUnsupportedReason(input)).toBe('html-or-jsx')
+  })
+})

src/renderer/src/components/editor/markdown-rich-mode.ts

@@ -48,7 +48,7 @@ const UNSUPPORTED_PATTERNS: UnsupportedMatch[] = [
     // Why: the rich editor preserves common embedded markup via placeholder
     // tokens before parsing, but any HTML shape that still fails round-trip
     // must fall back instead of risking silent source corruption.
-    pattern: /<\/?[A-Za-z][\w.:-]*(?:\s[^<>]*)?\/?>|<!--[\s\S]*?-->/
+    pattern: /<\/?[A-Za-z][\w.:-]*(?:\s[^<>]*)?\/?>/
   },
   {
     reason: 'reference-links',
@@ -157,6 +157,11 @@ export function getMarkdownRichModeEligibility(params: {
 }
 
 function hasHtmlOrJsx(content: string, pattern: RegExp): boolean {
+  // A missing closer after the first opener rules out every later opener.
+  const commentStart = content.indexOf('<!--')
+  if (commentStart !== -1 && content.includes('-->', commentStart + 4)) {
+    return true
+  }
   for (const match of content.matchAll(new RegExp(pattern, 'g'))) {
     if (isHtmlOrJsxFragment(match[0])) {
       return true
@@ -166,7 +171,7 @@ function hasHtmlOrJsx(content: string, pattern: RegExp): boolean {
 }
 
 function isHtmlOrJsxFragment(fragment: string): boolean {
-  if (fragment.startsWith('<!--') || fragment.startsWith('</')) {
+  if (fragment.startsWith('</')) {
     return true
   }
 
@@ -224,6 +229,7 @@ function forEachEmbeddedHtmlFragment(
   content: string,
   visit: (fragment: string) => boolean
 ): boolean {
+  const lastCommentClose = content.lastIndexOf('-->')
   for (let index = 0; index < content.length; index++) {
     if (content.charCodeAt(index) !== 60) {
       continue
@@ -231,7 +237,7 @@ function forEachEmbeddedHtmlFragment(
 
     let fragmentEnd: number | null = null
     if (content.startsWith('<!--', index)) {
-      const commentEnd = content.indexOf('-->', index + 4)
+      const commentEnd = index + 4 <= lastCommentClose ? content.indexOf('-->', index + 4) : -1
       fragmentEnd = commentEnd === -1 ? null : commentEnd + 3
     } else {
       fragmentEnd = getHtmlTagEnd(content, index)