Skip to content

Commit 7d75e14

Browse files
icecrasher321claude
andcommitted
fix(search): honour fences and folded whitespace in Note highlighting
Two review findings, both real. The intraword-underscore cleanup guarded code with a pattern that recognised only the shortest delimiter forms — a bare ``` pair and a single-backtick span. A ````-fenced block, a tilde fence, or a ``multi-backtick`` span ended the region early and handed the rest of the author's code to the rewrite, turning `a\_b` into `a_b` inside their code sample. Fenced blocks are now walked a line at a time, tracking the opening delimiter exactly the way stripEmptyListItemLines already does (three or more, closed only by a run at least as long), and the inline branch matches a backtick RUN closed by one of equal length. The note scanner claimed to be the same scan as the indexer's `findTextRanges` but did not fold whitespace, which staging added since this branch was written. The indexer folds every `\s` to a space, so a phrase matches across a soft line break — which `remark-breaks` renders as a `<br>`, splitting the phrase over two text nodes that a per-node scan could never see. The hit counted in the panel and highlighted nowhere, the exact bug this branch exists to fix. The plugin now scans runs of continuously-readable text rather than single nodes, so a match spanning an inline boundary (a soft break, a bold word) is wrapped as several marks sharing one ordinal. Runs end at any non-inline element, so two paragraphs are never joined into a phrase the reader cannot see. `foldSearchWhitespace` moved to `@sim/utils/string`: the canvas card renders from a package, which cannot import from `apps/*`, and two copies of that rule silently disagreeing is precisely what produced the second finding. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent b3b8c04 commit 7d75e14

8 files changed

Lines changed: 373 additions & 74 deletions

File tree

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

Lines changed: 50 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -9,50 +9,70 @@ const FRONTMATTER_REGEX = /^---\r?\n(?:[\s\S]*?\r?\n)?---[ \t]*(?:\r?\n)*/
99
const ESCAPED_CALLOUT_REGEX = /^(\s*>(?:\s*>)*\s*)\\\[!([A-Za-z]+)\\\]/gm
1010

1111
/**
12-
* A code region \u2014 fenced block or inline span. Never rewritten by the cleanups below, and always
13-
* the FIRST branch of the alternations that use it so a candidate sitting inside code is consumed
14-
* as code and left verbatim.
12+
* Alternates a code region (fenced block or inline span \u2014 never rewritten) with an inline link whose
13+
* destination has no title and isn't angle-bracketed. The code branch is listed first so a link inside
14+
* code is consumed as code and left untouched. The destination stops at `)` / whitespace, so a link
15+
* carrying a title (`[x](url "t")`) never matches and is preserved verbatim.
1516
*/
16-
const CODE_REGION_SOURCE = '(```[\\s\\S]*?```|~~~[\\s\\S]*?~~~|`[^`\\n]+`)'
17-
18-
/**
19-
* Alternates a code region with an inline link whose destination has no title and isn't
20-
* angle-bracketed. The destination stops at `)` / whitespace, so a link carrying a title
21-
* (`[x](url "t")`) never matches and is preserved verbatim.
22-
*/
23-
const CODE_OR_PLAIN_LINK_REGEX = new RegExp(
24-
`${CODE_REGION_SOURCE}|\\[([^\\]]+)]\\(([^)\\s<>]+)\\)`,
25-
'g'
26-
)
17+
const CODE_OR_PLAIN_LINK_REGEX =
18+
/(```[\s\S]*?```|~~~[\s\S]*?~~~|`[^`\n]+`)|\[([^\]]+)]\(([^)\s<>]+)\)/g
19+
const HTTP_URL_REGEX = /^https?:\/\/\S+$/i
2720

2821
/**
29-
* Alternates a code region with a single underscore that has a letter or digit on both sides.
22+
* Alternates an inline code span with a single underscore that has a letter or digit on both sides.
3023
*
3124
* CommonMark's intraword rule means such an underscore can neither open nor close emphasis, so the
3225
* serializer's backslash before it carries no meaning \u2014 it just writes `SB\_ACTION\_ROUTER\_SECRET`
3326
* into the document. That is ugly in the file, and it silently breaks workflow search, which matches
3427
* against the stored markdown rather than the rendered text: searching `SB_ACTION` finds nothing in
3528
* a note whose stored form has a backslash the reader never sees.
3629
*
37-
* Code is excluded because the serializer emits it verbatim: a `\_` inside a fence is the author's
38-
* own backslash, not an escape this may drop.
30+
* Code is excluded because the serializer emits it verbatim: a `\_` inside a span is the author's own
31+
* backslash, not an escape this may drop. The span branch matches a backtick RUN and requires a run of
32+
* the same length to close it, per CommonMark \u2014 a fixed single-backtick pattern would read ``` ``a`b`` ```
33+
* as `` `a` `` plus loose text and rewrite the interior. Fenced blocks are handled a line at a time by
34+
* {@link unescapeIntrawordUnderscores}, which is the only way to honour a fence of any length.
3935
*
40-
* Written with a capture group rather than a lookbehind: lookbehind only landed in Safari 16.4, and
41-
* an unsupported one throws when the pattern is constructed \u2014 taking the whole editor module with
42-
* it. The group is consumed and put back, and the lookahead is not, so runs like `A\_B\_C` still
43-
* match on every pair.
36+
* The flanking character is a capture group rather than a lookbehind: lookbehind only landed in
37+
* Safari 16.4, and an unsupported one throws when the pattern is constructed \u2014 taking the whole editor
38+
* module with it. The group is consumed and put back, and the lookahead is not, so runs like `A\_B\_C`
39+
* still match on every pair.
4440
*/
45-
const CODE_OR_INTRAWORD_ESCAPED_UNDERSCORE = new RegExp(
46-
`${CODE_REGION_SOURCE}|([\\p{L}\\p{N}])\\\\_(?=[\\p{L}\\p{N}])`,
47-
'gu'
48-
)
49-
const HTTP_URL_REGEX = /^https?:\/\/\S+$/i
41+
const CODE_SPAN_OR_INTRAWORD_ESCAPED_UNDERSCORE =
42+
/(`+)((?:[^`]|(?!\1)`)*?)\1(?!`)|([\p{L}\p{N}])\\_(?=[\p{L}\p{N}])/gu
5043

51-
/** Drops the meaningless backslash before an intraword underscore, outside code. */
44+
/**
45+
* Drops the meaningless backslash before an intraword underscore, outside code.
46+
*
47+
* Fenced blocks are skipped a line at a time, tracking the opening delimiter the same way
48+
* {@link stripEmptyListItemLines} does: a fence is three OR MORE backticks or tildes and is closed
49+
* only by a run of the same character at least as long, so a `````` ```` `````-fenced block wrapping
50+
* ``` ``` ``` stays code throughout. Matching a fixed ``` pair instead would end the region early and
51+
* hand the rest of the author's code to the rewrite.
52+
*/
5253
function unescapeIntrawordUnderscores(markdown: string): string {
53-
return markdown.replace(CODE_OR_INTRAWORD_ESCAPED_UNDERSCORE, (match, code, flank) =>
54-
code ? code : `${flank}_`
55-
)
54+
const lines = markdown.split('\n')
55+
let fence: string | null = null
56+
57+
for (let i = 0; i < lines.length; i++) {
58+
const delimiter = lines[i].match(FENCE_DELIMITER)?.[1]
59+
60+
if (fence) {
61+
if (delimiter && delimiter[0] === fence[0] && delimiter.length >= fence.length) fence = null
62+
continue
63+
}
64+
if (delimiter) {
65+
fence = delimiter
66+
continue
67+
}
68+
69+
lines[i] = lines[i].replace(
70+
CODE_SPAN_OR_INTRAWORD_ESCAPED_UNDERSCORE,
71+
(match, ticks, _span, flank) => (ticks === undefined ? `${flank}_` : match)
72+
)
73+
}
74+
75+
return lines.join('\n')
5676
}
5777

5878
/**

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,22 @@ describe('editor markdown round-trip', () => {
237237
expect(roundTrip('call `a\\_b` here')).toContain('a\\_b')
238238
})
239239

240+
/* A fence is three OR MORE delimiters, closed only by a run at least as long, and an inline span
241+
opens and closes on backtick runs of equal length. Recognising just the shortest form ends the
242+
code region early and hands the rest of the author's code to the rewrite. */
243+
it('leaves code alone in a longer fence', () => {
244+
expect(roundTrip('````\nx = a\\_b\n```\nstill code y = c\\_d\n````')).toContain('a\\_b')
245+
expect(roundTrip('````\nx = a\\_b\n```\nstill code y = c\\_d\n````')).toContain('c\\_d')
246+
})
247+
248+
it('leaves code alone in a tilde fence', () => {
249+
expect(roundTrip('~~~~\nx = a\\_b\n~~~~')).toContain('a\\_b')
250+
})
251+
252+
it('leaves code alone in a multi-backtick inline span', () => {
253+
expect(roundTrip('call ``a\\_b`c`` here')).toContain('a\\_b')
254+
})
255+
240256
it('preserves an image url (does not drop the src)', () => {
241257
const out = roundTrip('![alt](https://example.com/i.png)')
242258
expect(out).toContain('![alt](https://example.com/i.png)')

apps/sim/lib/workflows/search-replace/indexer.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { isRecordLike } from '@sim/utils/object'
2+
import { foldSearchWhitespace } from '@sim/utils/string'
23
import { DEFAULT_SUBBLOCK_TYPE } from '@sim/workflow-persistence/subblocks'
34
import type { SubBlockType } from '@sim/workflow-types/blocks'
45
import { isWorkflowBlockProtected } from '@sim/workflow-types/workflow'
@@ -10,7 +11,6 @@ import {
1011
} from '@/lib/workflows/search-replace/json-value-fields'
1112
import {
1213
buildBlockNamesByReferencePrefix,
13-
foldSearchWhitespace,
1414
getResourceKindForSubBlock,
1515
matchesSearchText,
1616
parseInlineReferences,

apps/sim/lib/workflows/search-replace/resources/references.ts

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { foldSearchWhitespace } from '@sim/utils/string'
12
import {
23
getWorkflowSearchSubBlockResourceKind,
34
parseWorkflowSearchSubBlockResources,
@@ -140,18 +141,6 @@ export function parseStructuredResourceReferences(
140141
return parseWorkflowSearchSubBlockResources(value, subBlockConfig, selectorContext)
141142
}
142143

143-
/**
144-
* Maps every Unicode whitespace character to a plain space, one-to-one.
145-
* Agent-authored block names and values routinely carry non-breaking or
146-
* narrow spaces that render identically to " " but never equal a typed
147-
* space, silently hiding matches. The replacement is length-preserving
148-
* (every `\s` character is a single UTF-16 unit), so indexes into the
149-
* folded string remain valid ranges into the original.
150-
*/
151-
export function foldSearchWhitespace(value: string): string {
152-
return value.replace(/\s/g, ' ')
153-
}
154-
155144
export function matchesSearchText(
156145
candidate: string,
157146
query: string | undefined,

apps/sim/lib/workflows/search-replace/resources/resolvers.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { foldSearchWhitespace } from '@/lib/workflows/search-replace/resources/references'
1+
import { foldSearchWhitespace } from '@sim/utils/string'
22
import type {
33
WorkflowSearchMatch,
44
WorkflowSearchMatchKind,

packages/utils/src/string.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,3 +162,22 @@ export function formatQuotedNameList(names: string[], maxListed: number): string
162162
const overflow = names.length - maxListed
163163
return overflow > 0 ? `${listed} and ${overflow} more` : listed
164164
}
165+
166+
/**
167+
* Maps every Unicode whitespace character to a plain space, one-to-one.
168+
*
169+
* Agent-authored block names and values routinely carry non-breaking or narrow
170+
* spaces that render identically to " " but never equal a typed space, silently
171+
* hiding matches. The replacement is length-preserving (every `\s` character is
172+
* a single UTF-16 unit), so indexes into the folded string remain valid ranges
173+
* into the original.
174+
*
175+
* Lives here rather than beside the workflow search index because the Note card
176+
* on the canvas has to fold identically to find the same occurrences, and it
177+
* renders from `@sim/workflow-renderer` — a package, which cannot import from
178+
* `apps/*`. Two copies of this rule silently disagreeing is precisely the bug
179+
* that made a match count in the panel and highlight nowhere on the card.
180+
*/
181+
export function foldSearchWhitespace(value: string): string {
182+
return value.replace(/\s/g, ' ')
183+
}

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

Lines changed: 112 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
*/
1414

1515
import { act } from 'react'
16-
import type { Root } from 'hast'
16+
import type { Element, ElementContent, Root, RootContent } from 'hast'
1717
import { createRoot, type Root as ReactRoot } from 'react-dom/client'
1818
import { afterEach, beforeAll, describe, expect, it } from 'vitest'
1919
import {
@@ -204,6 +204,117 @@ describe('note search rehype plugin', () => {
204204
})
205205
})
206206

207+
/*
208+
* The indexer folds every `\s` to a space before matching, so a phrase can match
209+
* across a soft line break — which `remark-breaks` renders as a `<br>` splitting
210+
* the phrase over two text nodes. A per-node scan saw neither half, leaving the
211+
* hit counted in the panel and highlighted nowhere on the card.
212+
*/
213+
describe('note search across inline boundaries', () => {
214+
function markedTextsOf(tree: Root): string[] {
215+
const texts: string[] = []
216+
const walk = (node: Root | Element) => {
217+
/* `Root['children']` and `Element['children']` are different unions, so iterating the
218+
parameter directly widens each child to their intersection and drops narrowing. */
219+
const children: Array<RootContent | ElementContent> = node.children
220+
for (const child of children) {
221+
if (child.type !== 'element') continue
222+
if (child.tagName === 'mark') {
223+
const [first] = child.children
224+
texts.push(first?.type === 'text' ? first.value : '')
225+
continue
226+
}
227+
walk(child)
228+
}
229+
}
230+
walk(tree)
231+
return texts
232+
}
233+
234+
function paragraphWithBreak(before: string, after: string): Root {
235+
return {
236+
type: 'root',
237+
children: [
238+
{
239+
type: 'element',
240+
tagName: 'p',
241+
properties: {},
242+
children: [
243+
{ type: 'text', value: before },
244+
{ type: 'element', tagName: 'br', properties: {}, children: [] },
245+
{ type: 'text', value: after },
246+
],
247+
},
248+
],
249+
}
250+
}
251+
252+
it('marks a phrase spanning a soft line break', () => {
253+
const tree = paragraphWithBreak('the quick', 'brown fox')
254+
noteSearchHighlightPlugin({ query: 'quick brown' })(tree)
255+
expect(markedTextsOf(tree)).toEqual(['quick', 'brown'])
256+
})
257+
258+
it('gives both halves of one hit the same ordinal', () => {
259+
const tree = paragraphWithBreak('the quick', 'brown fox')
260+
noteSearchHighlightPlugin({ query: 'quick brown' })(tree)
261+
262+
const ordinals: unknown[] = []
263+
const walk = (node: Root | Element) => {
264+
/* `Root['children']` and `Element['children']` are different unions, so iterating the
265+
parameter directly widens each child to their intersection and drops narrowing. */
266+
const children: Array<RootContent | ElementContent> = node.children
267+
for (const child of children) {
268+
if (child.type !== 'element') continue
269+
if (child.tagName === 'mark') ordinals.push(child.properties.dataNoteSearchIndex)
270+
else walk(child)
271+
}
272+
}
273+
walk(tree)
274+
expect(ordinals).toEqual(['0', '0'])
275+
})
276+
277+
it('marks a phrase spanning a bold word', () => {
278+
const tree: Root = {
279+
type: 'root',
280+
children: [
281+
{
282+
type: 'element',
283+
tagName: 'p',
284+
properties: {},
285+
children: [
286+
{ type: 'text', value: 'a ' },
287+
{
288+
type: 'element',
289+
tagName: 'strong',
290+
properties: {},
291+
children: [{ type: 'text', value: 'bold' }],
292+
},
293+
{ type: 'text', value: ' word' },
294+
],
295+
},
296+
],
297+
}
298+
noteSearchHighlightPlugin({ query: 'a bold word' })(tree)
299+
expect(markedTextsOf(tree)).toEqual(['a ', 'bold', ' word'])
300+
})
301+
302+
/* Two paragraphs are not one phrase on screen. Joining them would invent a hit
303+
the reader cannot see — and one the indexer never counted, since the source
304+
carries a blank line there, not a single space. */
305+
it('does not join text across a block boundary', () => {
306+
const tree = paragraphTree('the quick', 'brown fox')
307+
noteSearchHighlightPlugin({ query: 'quick brown' })(tree)
308+
expect(markedTextsOf(tree)).toEqual([])
309+
})
310+
311+
it('folds a non-breaking space the way the indexer does', () => {
312+
const tree = paragraphTree('a b')
313+
noteSearchHighlightPlugin({ query: 'a b' })(tree)
314+
expect(markedTextsOf(tree)).toEqual(['a b'])
315+
})
316+
})
317+
207318
describe('note search highlight rendering', () => {
208319
it('marks every occurrence in the read view', () => {
209320
const container = renderNote('first SB_SECRET line\n\nsecond SB_SECRET line', {

0 commit comments

Comments
 (0)