Skip to content

Commit 0ccad3f

Browse files
icecrasher321claude
andcommitted
fix(markdown): skip quoted fences, and stop joining runs across inline tags
Two review findings, both real, both the same shape: a rule that looked at the rendered form and forgot what the source actually says. QUOTED FENCES. The fence walk only recognised a bare delimiter run, but the serializer writes a fence inside a blockquote or a `[!NOTE]` callout with a `>` on every line. Code state was therefore never entered there and the block's interior was cleaned up as prose: `> x = a\_b` round-tripped to `> x = a_b`, losing the author's backslash. Unlike the fence-length cases this one is reachable today — verified against the real serializer before and after. Both fence walks now unquote the line first. INLINE JOINS. Runs concatenated the visible text of every inline tag, so `a<strong>b</strong>c` read as `abc` — a hit that cannot exist in the markdown the indexer scans, where `**` sits between the words. That is worse than a spurious mark: `occurrenceIndex` counts SOURCE occurrences, so a fabricated hit earlier in the document steals the current ordinal and paints the mark on text the search never matched. Only `<br>` continues a run now, because it alone stands for a character the source really has (a `\n`, folded to a space). Everything else stands for syntax the render drops. Nothing real is lost: a match spanning `a**b**c` would have to contain the asterisks to exist at all, and a match wholly inside an element is still found — the element simply starts its own run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 93ceb84 commit 0ccad3f

4 files changed

Lines changed: 129 additions & 46 deletions

File tree

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ function unescapeIntrawordUnderscores(markdown: string): string {
6060
continue
6161
}
6262

63-
const delimiter = lines[i].match(FENCE_DELIMITER)?.[1]
63+
const delimiter = opensFence(lines[i])
6464
if (delimiter) {
6565
fence = delimiter
6666
continue
@@ -177,10 +177,26 @@ const FENCE_DELIMITER = /^[ \t]*(`{3,}|~{3,})/
177177
* as prose.
178178
*/
179179
const CLOSING_FENCE = /^[ \t]*(`{3,}|~{3,})[ \t]*$/
180+
/**
181+
* Leading blockquote markers, which every line of a fence inside a quote or a `[!NOTE]` callout
182+
* carries. The serializer writes those (` > ```js `), so a walk that only recognises a bare fence
183+
* never enters code state there and rewrites the block's interior as prose.
184+
*/
185+
const BLOCKQUOTE_PREFIX = /^[ \t]*(?:>[ \t]?)*/
186+
187+
/** The line with any blockquote markers removed, so a quoted fence reads like a bare one. */
188+
function unquote(line: string): string {
189+
return line.replace(BLOCKQUOTE_PREFIX, '')
190+
}
191+
192+
/** The delimiter run that OPENS a fence on this line, quoted or not, or undefined. */
193+
function opensFence(line: string): string | undefined {
194+
return unquote(line).match(FENCE_DELIMITER)?.[1]
195+
}
180196

181197
/** Whether `line` closes a fence opened with `fence`: same character, and at least as long. */
182198
function closesFence(line: string, fence: string): boolean {
183-
const delimiter = line.match(CLOSING_FENCE)?.[1]
199+
const delimiter = unquote(line).match(CLOSING_FENCE)?.[1]
184200
return Boolean(delimiter && delimiter[0] === fence[0] && delimiter.length >= fence.length)
185201
}
186202
/** Leading indentation of a line, used to detect whether an empty list item has indented children. */
@@ -213,7 +229,7 @@ function stripEmptyListItemLines(markdown: string): string {
213229
if (closesFence(line, fence)) fence = null
214230
continue
215231
}
216-
const delimiter = line.match(FENCE_DELIMITER)?.[1]
232+
const delimiter = opensFence(line)
217233
if (delimiter) {
218234
fence = delimiter
219235
kept.push(line)

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,21 @@ describe('editor markdown round-trip', () => {
279279
expect(roundTrip('call ``a\\_b`c`` here')).toContain('a\\_b')
280280
})
281281

282+
/* Unlike the fence-length cases, this one is reachable: the serializer really does write a
283+
quoted fence as ` > ```js `, so a walk that only recognises a bare fence never enters code
284+
state and rewrites the block's interior as prose. */
285+
it('leaves code alone inside a blockquoted fence', () => {
286+
expect(roundTrip('> ```js\n> x = a\\_b\n> ```')).toContain('a\\_b')
287+
})
288+
289+
it('leaves code alone inside a callout fence', () => {
290+
expect(roundTrip('> [!NOTE]\n> ```js\n> x = a\\_b\n> ```')).toContain('a\\_b')
291+
})
292+
293+
it('still unescapes prose inside a blockquote', () => {
294+
expect(roundTrip('> SB_ACTION_ROUTER_SECRET')).toContain('SB_ACTION_ROUTER_SECRET')
295+
})
296+
282297
it('preserves an image url (does not drop the src)', () => {
283298
const out = roundTrip('![alt](https://example.com/i.png)')
284299
expect(out).toContain('![alt](https://example.com/i.png)')

packages/workflow-renderer/src/note/note-search-highlight.test.tsx

Lines changed: 80 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,10 @@ describe('note search across inline boundaries', () => {
274274
expect(ordinals).toEqual(['0', '0'])
275275
})
276276

277-
it('marks a phrase spanning a bold word', () => {
277+
/* A match spanning `a**b**c` cannot exist in the source the indexer scans — the asterisks are
278+
between the words there. Joining across the element would invent one, and because the ordinal
279+
counts source occurrences, an invented hit appearing earlier steals the current mark. */
280+
it('does not join text across a bold word', () => {
278281
const tree: Root = {
279282
type: 'root',
280283
children: [
@@ -296,7 +299,7 @@ describe('note search across inline boundaries', () => {
296299
],
297300
}
298301
noteSearchHighlightPlugin({ query: 'a bold word' })(tree)
299-
expect(markedTextsOf(tree)).toEqual(['a ', 'bold', ' word'])
302+
expect(markedTextsOf(tree)).toEqual([])
300303
})
301304

302305
/* Two paragraphs are not one phrase on screen. Joining them would invent a hit
@@ -308,6 +311,81 @@ describe('note search across inline boundaries', () => {
308311
expect(markedTextsOf(tree)).toEqual([])
309312
})
310313

314+
it('still marks a match wholly inside an inline element', () => {
315+
const tree: Root = {
316+
type: 'root',
317+
children: [
318+
{
319+
type: 'element',
320+
tagName: 'p',
321+
properties: {},
322+
children: [
323+
{
324+
type: 'element',
325+
tagName: 'strong',
326+
properties: {},
327+
children: [{ type: 'text', value: 'SB_ACTION' }],
328+
},
329+
],
330+
},
331+
],
332+
}
333+
noteSearchHighlightPlugin({ query: 'SB_ACTION' })(tree)
334+
expect(markedTextsOf(tree)).toEqual(['SB_ACTION'])
335+
})
336+
337+
/* The ordinal counts SOURCE occurrences. A hit that only exists once formatting is stripped
338+
would take ordinal 0 here while the real one — the one the panel is pointing at — became 1,
339+
so the card would paint the current mark on text the search never matched. */
340+
it('does not let a formatted concatenation steal the current ordinal', () => {
341+
const tree: Root = {
342+
type: 'root',
343+
children: [
344+
{
345+
type: 'element',
346+
tagName: 'p',
347+
properties: {},
348+
children: [
349+
{ type: 'text', value: 'a' },
350+
{
351+
type: 'element',
352+
tagName: 'strong',
353+
properties: {},
354+
children: [{ type: 'text', value: 'b' }],
355+
},
356+
{ type: 'text', value: 'c' },
357+
],
358+
},
359+
{
360+
type: 'element',
361+
tagName: 'p',
362+
properties: {},
363+
children: [{ type: 'text', value: 'abc' }],
364+
},
365+
],
366+
}
367+
noteSearchHighlightPlugin({ query: 'abc' })(tree)
368+
369+
const marks: Array<[string, unknown]> = []
370+
const walk = (node: Root | Element) => {
371+
const children: Array<RootContent | ElementContent> = node.children
372+
for (const child of children) {
373+
if (child.type !== 'element') continue
374+
if (child.tagName === 'mark') {
375+
const [first] = child.children
376+
marks.push([
377+
first?.type === 'text' ? first.value : '',
378+
child.properties.dataNoteSearchIndex,
379+
])
380+
continue
381+
}
382+
walk(child)
383+
}
384+
}
385+
walk(tree)
386+
expect(marks).toEqual([['abc', '0']])
387+
})
388+
311389
it('folds a non-breaking space the way the indexer does', () => {
312390
const tree = paragraphTree('a b')
313391
noteSearchHighlightPlugin({ query: 'a b' })(tree)

packages/workflow-renderer/src/note/note-search-highlight.ts

Lines changed: 15 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -41,40 +41,6 @@ export interface NoteSearchRange {
4141
*/
4242
export const NOTE_SEARCH_MARK_INDEX_PROPERTY = 'dataNoteSearchIndex'
4343

44-
/**
45-
* Elements that do not interrupt a run of text. Text either side of one reads
46-
* as a single phrase, so the scan below joins across them — the same thing the
47-
* browser's own find does in `a<strong>b</strong>c`.
48-
*
49-
* An allowlist rather than a block-level denylist: raw HTML can put any tag in
50-
* this tree, and treating something unrecognised as a break can only ever miss
51-
* a match, while treating it as inline could invent one that is not on screen.
52-
*/
53-
const INLINE_TAG_NAMES: ReadonlySet<string> = new Set([
54-
'a',
55-
'abbr',
56-
'b',
57-
'br',
58-
'cite',
59-
'code',
60-
'del',
61-
'em',
62-
'i',
63-
'ins',
64-
'kbd',
65-
'mark',
66-
'q',
67-
's',
68-
'samp',
69-
'small',
70-
'span',
71-
'strong',
72-
'sub',
73-
'sup',
74-
'u',
75-
'var',
76-
])
77-
7844
/**
7945
* Visits every occurrence of `query` in `text`, case-insensitively and without
8046
* overlaps.
@@ -187,13 +153,21 @@ function collectTextRuns(node: Root | Element, builder: RunBuilder): void {
187153
continue
188154
}
189155

190-
/* An inline element continues the run — the builder state is shared, so
191-
descending is all it takes. Anything else breaks it either side. */
192-
if (INLINE_TAG_NAMES.has(child.tagName)) {
193-
collectTextRuns(child, builder)
194-
continue
195-
}
196-
156+
/*
157+
* Every other element ends the run, `<strong>` and `<a>` included.
158+
*
159+
* `<br>` is the one boundary that stands for a character the source really
160+
* has. Every other inline element stands for syntax the render DROPS —
161+
* `**`, `_`, a backtick, a link's `](url)` — so joining across one invents
162+
* an adjacency that exists on screen but not in the markdown the indexer
163+
* scans. That is not merely a spurious extra mark: `occurrenceIndex` counts
164+
* source occurrences, so a fabricated hit appearing earlier in the document
165+
* steals the current ordinal and paints the wrong one.
166+
*
167+
* Nothing real is lost. A match spanning `a**b**c` would have to contain
168+
* the asterisks to exist in the source at all, and a match wholly inside
169+
* the element is still found — the element simply starts its own run.
170+
*/
197171
endRun(builder)
198172
collectTextRuns(child, builder)
199173
endRun(builder)

0 commit comments

Comments
 (0)