From b3b8c04f610c358a8074f645cd8cc2833fe0205b Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Thu, 20 Aug 2026 14:10:44 -0700 Subject: [PATCH 1/5] fix(search): answer a Note match on the canvas card MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A workflow search match inside a Note counted towards the result total and then highlighted nowhere: the editor panel renders nothing for a Note, and `clearCurrentBlock` — which the panel calls to refuse one — also cleared the shared `activeSearchTarget`, destroying the very target the card was about to paint. Searching a 15k-character note reported "1 of 6" and moved nothing. The card's read view is now the surface that answers: - A rehype plugin marks every rendered occurrence; the current one is picked by an ordinal counted with the same scan the indexer uses, and travels by context so cycling matches does not re-parse the document. - `` is keyed on the query. Its memo comparator ignores `rehypePlugins`/`components`, so a plugin change alone cannot re-render it — marks appeared only when something else remounted the card, and then outlived the query that produced them. - The canvas selects, centres and expands the Note, because a compact card resets its scroll region to the top and cannot hold a position deep in its own body. Scrolling to the mark is `scrollTop` arithmetic, never `scrollIntoView`, which would drag ReactFlow's transformed viewport off-frame. - Title matches mark the name too. `activeSearchTarget` is re-published with a fresh identity on most of the search panel's renders, so subscribers take primitives. Holding the object in `WorkflowContent` — the panel's own ancestor — closed an unbounded update loop. Separately, the serializer escaped every underscore, writing `SB\_ACTION\_ROUTER\_SECRET` into the document. CommonMark's intraword rule means that backslash carries no meaning, and search matches the stored markdown, so it made anything with an underscore unfindable in a note that plainly showed it. Dropped outside code regions, where the serializer emits verbatim and a backslash is the author's own character. Co-Authored-By: Claude Opus 5 (1M context) --- .../rich-markdown-editor/markdown-fidelity.ts | 56 ++- .../rich-markdown-editor/round-trip.test.ts | 33 ++ .../components/note-block/note-block.tsx | 135 +++++++- .../[workspaceId]/w/[workflowId]/workflow.tsx | 69 +++- apps/sim/stores/panel/editor/store.test.ts | 59 ++++ apps/sim/stores/panel/editor/store.ts | 22 +- bun.lock | 1 + packages/workflow-renderer/package.json | 1 + packages/workflow-renderer/src/index.ts | 5 + .../src/lib/overflow-span.tsx | 12 +- .../src/note/note-block-view.tsx | 209 ++++++++++- .../src/note/note-search-highlight.test.tsx | 325 ++++++++++++++++++ .../src/note/note-search-highlight.ts | 160 +++++++++ 13 files changed, 1063 insertions(+), 24 deletions(-) create mode 100644 apps/sim/stores/panel/editor/store.test.ts create mode 100644 packages/workflow-renderer/src/note/note-search-highlight.test.tsx create mode 100644 packages/workflow-renderer/src/note/note-search-highlight.ts diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts index 4470187fefa..f6b661e8f02 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts @@ -9,15 +9,52 @@ const FRONTMATTER_REGEX = /^---\r?\n(?:[\s\S]*?\r?\n)?---[ \t]*(?:\r?\n)*/ const ESCAPED_CALLOUT_REGEX = /^(\s*>(?:\s*>)*\s*)\\\[!([A-Za-z]+)\\\]/gm /** - * Alternates a code region (fenced block or inline span \u2014 never rewritten) with an inline link whose - * destination has no title and isn't angle-bracketed. The code branch is listed first so a link inside - * code is consumed as code and left untouched. The destination stops at `)` / whitespace, so a link - * carrying a title (`[x](url "t")`) never matches and is preserved verbatim. + * A code region \u2014 fenced block or inline span. Never rewritten by the cleanups below, and always + * the FIRST branch of the alternations that use it so a candidate sitting inside code is consumed + * as code and left verbatim. */ -const CODE_OR_PLAIN_LINK_REGEX = - /(```[\s\S]*?```|~~~[\s\S]*?~~~|`[^`\n]+`)|\[([^\]]+)]\(([^)\s<>]+)\)/g +const CODE_REGION_SOURCE = '(```[\\s\\S]*?```|~~~[\\s\\S]*?~~~|`[^`\\n]+`)' + +/** + * Alternates a code region with an inline link whose destination has no title and isn't + * angle-bracketed. The destination stops at `)` / whitespace, so a link carrying a title + * (`[x](url "t")`) never matches and is preserved verbatim. + */ +const CODE_OR_PLAIN_LINK_REGEX = new RegExp( + `${CODE_REGION_SOURCE}|\\[([^\\]]+)]\\(([^)\\s<>]+)\\)`, + 'g' +) + +/** + * Alternates a code region with a single underscore that has a letter or digit on both sides. + * + * CommonMark's intraword rule means such an underscore can neither open nor close emphasis, so the + * serializer's backslash before it carries no meaning \u2014 it just writes `SB\_ACTION\_ROUTER\_SECRET` + * into the document. That is ugly in the file, and it silently breaks workflow search, which matches + * against the stored markdown rather than the rendered text: searching `SB_ACTION` finds nothing in + * a note whose stored form has a backslash the reader never sees. + * + * Code is excluded because the serializer emits it verbatim: a `\_` inside a fence is the author's + * own backslash, not an escape this may drop. + * + * Written with a capture group rather than a lookbehind: lookbehind only landed in Safari 16.4, and + * an unsupported one throws when the pattern is constructed \u2014 taking the whole editor module with + * it. The group is consumed and put back, and the lookahead is not, so runs like `A\_B\_C` still + * match on every pair. + */ +const CODE_OR_INTRAWORD_ESCAPED_UNDERSCORE = new RegExp( + `${CODE_REGION_SOURCE}|([\\p{L}\\p{N}])\\\\_(?=[\\p{L}\\p{N}])`, + 'gu' +) const HTTP_URL_REGEX = /^https?:\/\/\S+$/i +/** Drops the meaningless backslash before an intraword underscore, outside code. */ +function unescapeIntrawordUnderscores(markdown: string): string { + return markdown.replace(CODE_OR_INTRAWORD_ESCAPED_UNDERSCORE, (match, code, flank) => + code ? code : `${flank}_` + ) +} + /** * Collapses an autolinked destination back to its bare form: our normalizing serializer rewrites a bare * URL or `` autolink to `[url](url)` and a bare email to `[a@b.com](mailto:a@b.com)`, which churns @@ -171,7 +208,8 @@ function stripEmptyListItemLines(markdown: string): string { /** * Cleans up serializer output: drops empty list-item marker lines that would otherwise corrupt on * round-trip ({@link stripEmptyListItemLines}), restores callout markers the serializer - * backslash-escapes (`> \[!NOTE\]` → `> [!NOTE]`), and collapses trailing blank lines to a single + * backslash-escapes (`> \[!NOTE\]` → `> [!NOTE]`), drops the equally unnecessary escape on an + * intraword underscore ({@link unescapeIntrawordUnderscores}), and collapses trailing blank lines to a single * newline. Interior blank runs are NOT collapsed here — blank lines inside a fenced code block (or a * verbatim raw-markdown-snippet) are significant, and a global collapse would corrupt them. An interior * run between top-level blocks is significant too: it is how an empty paragraph is written, and @@ -184,6 +222,8 @@ function stripEmptyListItemLines(markdown: string): string { */ export function postProcessSerializedMarkdown(markdown: string): string { return collapseAutolinkedUrls( - stripEmptyListItemLines(markdown).replace(ESCAPED_CALLOUT_REGEX, '$1[!$2]') + unescapeIntrawordUnderscores( + stripEmptyListItemLines(markdown).replace(ESCAPED_CALLOUT_REGEX, '$1[!$2]') + ) ).replace(/\n+$/, '\n') } diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip.test.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip.test.ts index c8745e5e861..4a903af1ca8 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip.test.ts @@ -175,6 +175,13 @@ describe('editor markdown round-trip', () => { 'highlight nested in bold': '**bold ==mark== here**', 'highlight in list': '- ==a== item', 'highlight with interior equals': 'x ==a=b== y', + 'intraword underscores': 'SB_ACTION_ROUTER_SECRET', + 'env token with underscores': '{{TE_SERET}} and {{OPENAI_API_KEY}}', + 'underscore emphasis': 'an _italic_ word', + 'underscore bold': 'a __bold__ word', + 'mixed underscores': '_em_ then SNAKE_CASE_NAME then _em again_', + 'escaped underscore in code': '```py\nx = a\\_b\n```', + 'escaped underscore in inline code': 'call `a\\_b` here', } for (const [name, input] of Object.entries(cases)) { @@ -204,6 +211,32 @@ describe('editor markdown round-trip', () => { expect(roundTrip('> [!NOTE]\n> Heads up')).toContain('[!NOTE]') }) + /* + * The serializer escapes every underscore, but CommonMark's intraword rule means one flanked by + * letters or digits can neither open nor close emphasis. The escape is therefore invisible to a + * reader and load-bearing for nobody — while workflow search matches the STORED markdown, so a + * stray backslash made `SB_ACTION` unfindable in a note that plainly showed it. + */ + it('writes an intraword underscore without a backslash', () => { + expect(roundTrip('SB_ACTION_ROUTER_SECRET')).toBe('SB_ACTION_ROUTER_SECRET') + expect(roundTrip('{{TE_SERET}}')).toBe('{{TE_SERET}}') + }) + + /* Emphasis itself normalises to asterisks, which is pre-existing and fine. What must survive + is the distinction: a literal underscore pair keeps its escape, so re-parsing cannot turn + the user's text into emphasis. */ + it('still escapes an underscore that would open or close emphasis', () => { + expect(roundTrip('an _italic_ word')).toBe('an *italic* word') + expect(roundTrip('literal \\_not emphasis\\_ here')).toContain('\\_') + }) + + /* Code is emitted verbatim, so a backslash inside it is the author's own character and not an + escape to drop. Unescaping blind would silently rewrite people's code. */ + it('leaves a backslash-underscore inside code alone', () => { + expect(roundTrip('```py\nx = a\\_b\n```')).toContain('a\\_b') + expect(roundTrip('call `a\\_b` here')).toContain('a\\_b') + }) + it('preserves an image url (does not drop the src)', () => { const out = roundTrip('![alt](https://example.com/i.png)') expect(out).toContain('![alt](https://example.com/i.png)') diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/note-block/note-block.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/note-block/note-block.tsx index ef57b42424c..f5aa127f4e8 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/note-block/note-block.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/note-block/note-block.tsx @@ -1,6 +1,7 @@ import { memo, useCallback, useEffect, useMemo, useRef, useState } from 'react' import { BLOCK_DIMENSIONS, + countNoteSearchOccurrencesBefore, DEFAULT_NOTE_COLOR, estimateNoteBlockHeight, getNoteStringValue, @@ -8,9 +9,12 @@ import { NoteBlockView, type NoteColor, type NoteContentEditorProps, + type NoteSearchHighlight, + type NoteSearchRange, } from '@sim/workflow-renderer' import dynamic from 'next/dynamic' import { type NodeProps, useReactFlow } from 'reactflow' +import { useShallow } from 'zustand/react/shallow' import { appendNoteImageMarkdown } from '@/lib/workflows/notes/add-image' import { NOTE_ADD_IMAGE_EVENT, @@ -26,7 +30,7 @@ import { useBlockDimensions } from '@/app/workspace/[workspaceId]/w/[workflowId] import { isBlockProtected } from '@/app/workspace/[workspaceId]/w/[workflowId]/utils' import { useCollaborativeWorkflow } from '@/hooks/use-collaborative-workflow' import { useIsCurrentWorkflowExecuting } from '@/stores/execution' -import { usePanelEditorStore } from '@/stores/panel' +import { usePanelEditorSearchStore, usePanelEditorStore } from '@/stores/panel' import { useSubBlockStore } from '@/stores/workflows/subblock/store' import { useWorkflowStore } from '@/stores/workflows/workflow/store' @@ -48,6 +52,9 @@ const NoteMarkdownEditor = dynamic( const NOTE_EXPAND_FOCUS_DURATION_MS = 300 +/** The markdown body's sub-block id, as declared by the Note block config. */ +const NOTE_CONTENT_SUBBLOCK_ID = 'content' + function renderNoteContentEditor(props: NoteContentEditorProps) { return } @@ -106,7 +113,86 @@ export const NoteBlock = memo(function NoteBlock({ useCallback((state) => isBlockProtected(id, state.blocks), [id]) ) const clearCurrentBlock = usePanelEditorStore((state) => state.clearCurrentBlock) + /* Flattened to primitives under a shallow compare, never held as the target + object: the search panel re-publishes an equal target on most of its own + renders, and a card that subscribes to the object rebuilds its highlight on + every one of them. */ + const searchTarget = usePanelEditorSearchStore( + useShallow((state) => { + const target = state.activeSearchTarget + return { + blockId: target?.blockId ?? null, + subBlockId: target?.subBlockId ?? null, + targetKind: target?.targetKind ?? null, + query: target?.query ?? null, + rawValue: target?.rawValue ?? null, + rangeStart: target?.range?.start ?? null, + rangeEnd: target?.range?.end ?? null, + } + }) + ) const canEditNote = canEditWorkflow && !data.isPreview && !isProtected + + /** Whether the active search match belongs to this card at all. */ + const isSearchTargetBlock = + !data.isPreview && + !data.isEmbedded && + searchTarget.blockId === id && + Boolean(searchTarget.query) + + /** + * The workflow search match this card should paint in its body. + * + * The panel editor renders nothing for a note, so the card is the only + * surface that can answer a search — without this a note match counts towards + * "1 of 6" and then highlights nowhere. + * + * The stored range is re-checked against the live content before it is + * trusted: the index runs over a snapshot, and a collaborator's edit between + * indexing and painting would shift the ordinal onto the wrong words. When it + * no longer holds — or the match never carried one — this falls back to the + * first rendered occurrence, which is the same fallback the editor panel + * makes for a label whose stored range does not fit. + */ + const searchHighlight = useMemo(() => { + if (!isSearchTargetBlock) return null + if (searchTarget.targetKind !== 'subblock') return null + if (searchTarget.subBlockId !== NOTE_CONTENT_SUBBLOCK_ID) return null + + const { query, rawValue, rangeStart, rangeEnd } = searchTarget + if (!query) return null + + const rangeHolds = + rangeStart !== null && rangeEnd !== null && content.slice(rangeStart, rangeEnd) === rawValue + if (rangeHolds) { + return { + query, + occurrenceIndex: countNoteSearchOccurrencesBefore(content, query, rangeStart), + } + } + + return content.toLowerCase().includes(query.toLowerCase()) + ? { query, occurrenceIndex: 0 } + : null + }, [content, isSearchTargetBlock, searchTarget]) + + /** + * The match to paint in the title, for a search that hit the note's name. + * + * A name match carries an exact range over `block.name`, so unlike the body + * there is no occurrence to reconstruct — it is used directly, once it still + * describes the live name. + */ + const nameSearchRange = useMemo(() => { + if (!isSearchTargetBlock) return null + if (searchTarget.targetKind !== 'block-name') return null + + const { rawValue, rangeStart, rangeEnd } = searchTarget + if (rangeStart === null || rangeEnd === null) return null + if ((name ?? '').slice(rangeStart, rangeEnd) !== rawValue) return null + + return { start: rangeStart, end: rangeEnd } + }, [isSearchTargetBlock, name, searchTarget]) const uploadNoteImage = useNoteImageUpload() const imageInputRef = useRef(null) const [blockHeight, setBlockHeight] = useState(() => estimateNoteBlockHeight(content)) @@ -132,6 +218,40 @@ export const NoteBlock = memo(function NoteBlock({ [] ) + const isExpandedRef = useRef(isExpanded) + useEffect(() => { + isExpandedRef.current = isExpanded + }, [isExpanded]) + + /** + * Opens the card while it holds the current search match, and closes it again + * when the match moves on. + * + * A compact note shows a few lines of what is often a long document, so a + * match found deep inside it lands in a body the user cannot read. Expanding + * is the same gesture a click makes, and it gives the mark somewhere to be. + * + * Two things this deliberately does not do. It does not call + * {@link handleExpandedChange}, whose own `setCenter` would race the camera + * the canvas already moved for this match. And it only closes a card it + * opened — a note the user expanded by hand, or collapsed by hand while the + * match still points here, is left exactly as they left it. + */ + const searchExpandedRef = useRef(false) + const hasSearchMatch = searchHighlight !== null || nameSearchRange !== null + useEffect(() => { + if (hasSearchMatch) { + if (searchExpandedRef.current || isExpandedRef.current || !canEditNote) return + searchExpandedRef.current = true + setIsExpanded(true) + return + } + + if (!searchExpandedRef.current) return + searchExpandedRef.current = false + setIsExpanded(false) + }, [canEditNote, hasSearchMatch]) + const handleNameChange = (nextName: string) => { if (!canEditNote) return false return collaborativeUpdateBlockName(id, nextName).success @@ -139,7 +259,7 @@ export const NoteBlock = memo(function NoteBlock({ const handleContentChange = (nextContent: string) => { if (!canEditNote) return - collaborativeSetSubblockValue(id, 'content', nextContent) + collaborativeSetSubblockValue(id, NOTE_CONTENT_SUBBLOCK_ID, nextContent) } /** @@ -175,9 +295,14 @@ export const NoteBlock = memo(function NoteBlock({ if (!image) continue /* Re-read per image: each append has to build on the previous one, and on anything a collaborator wrote while the upload was in flight. */ - const current = getNoteStringValue(useSubBlockStore.getState().getValue(id, 'content')) ?? '' + const current = + getNoteStringValue(useSubBlockStore.getState().getValue(id, NOTE_CONTENT_SUBBLOCK_ID)) ?? '' setExternalContentWrites((count) => count + 1) - collaborativeSetSubblockValue(id, 'content', appendNoteImageMarkdown(current, image)) + collaborativeSetSubblockValue( + id, + NOTE_CONTENT_SUBBLOCK_ID, + appendNoteImageMarkdown(current, image) + ) } } @@ -292,6 +417,8 @@ export const NoteBlock = memo(function NoteBlock({ onExpandedChange={handleExpandedChange} onImageFilesDrop={(files) => void insertImages(files)} renderContentEditor={renderNoteContentEditor} + searchHighlight={searchHighlight} + nameSearchRange={nameSearchRange} actionBar={ window.removeEventListener('keydown', handleArrowNavigation, true) }, [embedded, getNodes, blocks, focusBlockInView]) + /** + * Brings a Note holding the current search match onto the canvas. + * + * Every other block answers a search through the editor panel, which scrolls + * the matching field into view for free. A Note renders nothing there — the + * card itself is the surface — so the camera has to do that job here, and + * the card has to be selected before it will hold a scroll position deep in + * its own body rather than snapping back to the top. + * + * Keyed on the match rather than the block, so cycling between two matches + * in one Note re-asserts a camera that is already where it needs to be + * (visually inert) instead of stranding the second match off-screen after + * the user has panned away. Matches on every other kind of block are marked + * handled and otherwise left alone — without that, walking away to a block + * match and back to a Note one would read as the same match twice and skip + * the camera the second time. + * + * Also covers a match on the Note's *name*, which the card cannot underline + * but which at least lands the user on the right card. + * + * Subscribed as two ids and NEVER as the target object. The search panel + * renders inside this component and re-publishes an equal target on most of + * its own renders (its hydration hooks hand back fresh arrays), so holding + * the object here re-renders the panel, whose effect re-publishes, which + * re-renders it again — an unbounded update loop the moment a search opens. + * Two string selectors are compared by value, so a re-publish of the same + * match is inert. + */ + const searchMatchId = usePanelEditorSearchStore( + (state) => state.activeSearchTarget?.matchId ?? null + ) + const searchMatchBlockId = usePanelEditorSearchStore( + (state) => state.activeSearchTarget?.blockId ?? null + ) + const focusedSearchMatchIdRef = useRef(null) + useEffect(() => { + if (embedded) return + if (!searchMatchId || !searchMatchBlockId) { + focusedSearchMatchIdRef.current = null + return + } + if (searchMatchId === focusedSearchMatchIdRef.current) return + + if (blocks[searchMatchBlockId]?.type !== 'note') { + focusedSearchMatchIdRef.current = searchMatchId + return + } + + /* Read from `displayNodes` rather than `getNodes()` so a match that + arrives before its node has mounted is retried on the commit that + mounts it, instead of being dropped. */ + const node = displayNodes.find((candidate) => candidate.id === searchMatchBlockId) + if (!node) return + + focusedSearchMatchIdRef.current = searchMatchId + setDisplayNodes((currentNodes) => + resolveSelectionConflicts( + currentNodes.map((currentNode) => ({ + ...currentNode, + selected: currentNode.id === node.id, + })), + blocks + ) + ) + focusBlockInView(node) + }, [blocks, displayNodes, embedded, focusBlockInView, searchMatchBlockId, searchMatchId]) + /** Handles edge selection with container context tracking and Shift-click multi-selection. */ const onEdgeClick = useCallback( (event: React.MouseEvent, edge: any) => { diff --git a/apps/sim/stores/panel/editor/store.test.ts b/apps/sim/stores/panel/editor/store.test.ts new file mode 100644 index 00000000000..91050a52a22 --- /dev/null +++ b/apps/sim/stores/panel/editor/store.test.ts @@ -0,0 +1,59 @@ +/** + * @vitest-environment jsdom + * + * Deselecting a block must not end a workflow search. + * + * `clearCurrentBlock` used to clear `activeSearchTarget` too, which made a Note + * match unrenderable: the editor answers a Note by deselecting it, destroying + * the target the Note card was about to paint. The search panel owns that + * target's lifetime — it re-asserts it on every match change and clears it on + * close — so nothing else may reach in and drop it. + */ +import { beforeEach, describe, expect, it } from 'vitest' +import type { ActiveSearchTarget } from '@/stores/panel/editor/store' +import { usePanelEditorSearchStore, usePanelEditorStore } from '@/stores/panel/editor/store' + +const NOTE_SEARCH_TARGET: ActiveSearchTarget = { + matchId: 'text:note-1:content:0', + blockId: 'note-1', + subBlockId: 'content', + canonicalSubBlockId: 'content', + valuePath: [], + kind: 'text', + targetKind: 'subblock', + subBlockType: 'long-input', + rawValue: 'SB_ACTION_ROUTER_SECRET', + searchText: 'uses SB_ACTION_ROUTER_SECRET here', + query: 'SB_ACTION_ROUTER_SECRET', + range: { start: 5, end: 28 }, +} + +beforeEach(() => { + usePanelEditorSearchStore.setState({ activeSearchTarget: null }) + usePanelEditorStore.setState({ currentBlockId: null }) +}) + +describe('clearCurrentBlock', () => { + it('clears the selected block', () => { + usePanelEditorStore.getState().setCurrentBlockId('block-1') + usePanelEditorStore.getState().clearCurrentBlock() + expect(usePanelEditorStore.getState().currentBlockId).toBeNull() + }) + + it('leaves the active search target in place', () => { + usePanelEditorSearchStore.getState().setActiveSearchTarget(NOTE_SEARCH_TARGET) + usePanelEditorStore.getState().setCurrentBlockId('note-1') + + usePanelEditorStore.getState().clearCurrentBlock() + + expect(usePanelEditorSearchStore.getState().activeSearchTarget).toEqual(NOTE_SEARCH_TARGET) + }) +}) + +describe('setActiveSearchTarget', () => { + it('clears the target when the search panel asks it to', () => { + usePanelEditorSearchStore.getState().setActiveSearchTarget(NOTE_SEARCH_TARGET) + usePanelEditorSearchStore.getState().setActiveSearchTarget(null) + expect(usePanelEditorSearchStore.getState().activeSearchTarget).toBeNull() + }) +}) diff --git a/apps/sim/stores/panel/editor/store.ts b/apps/sim/stores/panel/editor/store.ts index e29116be452..7474f35c60c 100644 --- a/apps/sim/stores/panel/editor/store.ts +++ b/apps/sim/stores/panel/editor/store.ts @@ -41,7 +41,15 @@ interface PanelEditorState { currentBlockId: string | null /** Sets the current selected block identifier (use null to clear) */ setCurrentBlockId: (blockId: string | null) => void - /** Clears the current selection */ + /** + * Clears the current selection. + * + * Leaves {@link PanelEditorSearchState.activeSearchTarget} alone: the search + * panel owns that target's lifetime, and deselecting a block is not the end + * of a search. Clearing it here made a Note match impossible to render — the + * editor answers a Note by deselecting it, which destroyed the very target + * the Note card was about to paint. + */ clearCurrentBlock: () => void /** Height of the connections section in pixels */ connectionsHeight: number @@ -56,7 +64,16 @@ interface PanelEditorState { } interface PanelEditorSearchState { - /** Ephemeral workflow search target used for scrolling/highlighting editor fields */ + /** + * Ephemeral workflow search target used for scrolling/highlighting editor fields. + * + * Re-published with a fresh object identity on most of the search panel's own + * renders — its match-hydration hooks hand back new arrays, so the effect that + * publishes this re-runs constantly with an equal target. Subscribe to the + * fields you need as primitives (or under `useShallow`), never to the object. + * Any component that both holds the object and can re-render the search panel + * closes an unbounded update loop the moment a search opens. + */ activeSearchTarget: ActiveSearchTarget | null /** Sets an active search target to highlight in the editor */ setActiveSearchTarget: (target: ActiveSearchTarget | null) => void @@ -95,7 +112,6 @@ export const usePanelEditorStore = create()( }, clearCurrentBlock: () => { set({ currentBlockId: null }) - usePanelEditorSearchStore.getState().setActiveSearchTarget(null) }, setConnectionsHeight: (height) => { const clampedHeight = Math.max( diff --git a/bun.lock b/bun.lock index 85505faa987..aac7a11547c 100644 --- a/bun.lock +++ b/bun.lock @@ -721,6 +721,7 @@ "@sim/utils": "workspace:*", "@sim/workflow-types": "workspace:*", "@testing-library/jest-dom": "^6.6.3", + "@types/hast": "^3.0.4", "@types/react": "^19", "jsdom": "^26.0.0", "react": "19.2.4", diff --git a/packages/workflow-renderer/package.json b/packages/workflow-renderer/package.json index 78070fec26f..1039d73f200 100644 --- a/packages/workflow-renderer/package.json +++ b/packages/workflow-renderer/package.json @@ -46,6 +46,7 @@ "@sim/utils": "workspace:*", "@sim/workflow-types": "workspace:*", "@testing-library/jest-dom": "^6.6.3", + "@types/hast": "^3.0.4", "@types/react": "^19", "jsdom": "^26.0.0", "react": "19.2.4", diff --git a/packages/workflow-renderer/src/index.ts b/packages/workflow-renderer/src/index.ts index d8d315cddf5..63961c81fdf 100644 --- a/packages/workflow-renderer/src/index.ts +++ b/packages/workflow-renderer/src/index.ts @@ -25,6 +25,11 @@ export { type NoteColorOption, } from './note/note-colors' export { getNoteStringValue, isNoteContentEmpty } from './note/note-content' +export { + countNoteSearchOccurrencesBefore, + type NoteSearchHighlight, + type NoteSearchRange, +} from './note/note-search-highlight' export { type SubflowNodeData, SubflowNodeView, diff --git a/packages/workflow-renderer/src/lib/overflow-span.tsx b/packages/workflow-renderer/src/lib/overflow-span.tsx index 65437f0d8a5..b86caeff67d 100644 --- a/packages/workflow-renderer/src/lib/overflow-span.tsx +++ b/packages/workflow-renderer/src/lib/overflow-span.tsx @@ -1,8 +1,16 @@ +import type { ReactNode } from 'react' import { FloatingTooltip, isTextClipped, useFloatingTooltip } from '@sim/emcn' interface OverflowSpanProps { value: string className: string + /** + * Decorated rendering of `value` — the same characters, wrapped. Used to mark + * a search hit inside a name without letting the decoration reach the + * tooltip, which stays plain `value` so it can never leak markup or drift + * from the text being truncated. + */ + children?: ReactNode } /** @@ -11,13 +19,13 @@ interface OverflowSpanProps { * attribute here: on the canvas it pops the browser's raw, unstyled tooltip * with the full untruncated value (including raw code/JSON) over the graph. */ -export function OverflowSpan({ value, className }: OverflowSpanProps) { +export function OverflowSpan({ value, className, children }: OverflowSpanProps) { const { state, handlers } = useFloatingTooltip(isTextClipped) return ( <> - {value} + {children ?? value} diff --git a/packages/workflow-renderer/src/note/note-block-view.tsx b/packages/workflow-renderer/src/note/note-block-view.tsx index 3c608b218e2..f5a7cb0926b 100644 --- a/packages/workflow-renderer/src/note/note-block-view.tsx +++ b/packages/workflow-renderer/src/note/note-block-view.tsx @@ -1,10 +1,12 @@ import { type ComponentProps, + createContext, memo, type MouseEvent as ReactMouseEvent, type ReactNode, type PointerEvent as ReactPointerEvent, useCallback, + useContext, useEffect, useLayoutEffect, useMemo, @@ -14,7 +16,7 @@ import { import { ChevronsDownUp, Expand } from '@sim/emcn/icons' import remarkBreaks from 'remark-breaks' import remarkGfm from 'remark-gfm' -import { Streamdown } from 'streamdown' +import { defaultRehypePlugins, Streamdown, type StreamdownProps } from 'streamdown' import 'streamdown/styles.css' import { Button, cn, handleKeyboardActivation, Tooltip } from '@sim/emcn' import { getEmbedInfo } from '@sim/utils/media-embed' @@ -26,6 +28,11 @@ import { type WorkflowBorderPort, } from '../workflow-block/workflow-block-border' import { DEFAULT_NOTE_COLOR, getNoteColorOption, type NoteColor } from './note-colors' +import { + type NoteSearchHighlight, + type NoteSearchRange, + noteSearchHighlightPlugin, +} from './note-search-highlight' const EMBED_SCALE = 0.78 const EMBED_INVERSE_SCALE = `${(1 / EMBED_SCALE) * 100}%` @@ -107,6 +114,68 @@ const NOTE_TASK_CHECKBOX_CLASS = [ 'checked:after:[clip-path:polygon(14%_44%,0_65%,50%_100%,100%_16%,80%_0%,43%_62%)]', ].join(' ') +/** + * The ordinal of the mark to paint as current, or null when no workflow search + * points at this note. + * + * Carried by context rather than by prop because the rehype plugin below is + * keyed on the query alone: cycling between two matches inside one note then + * re-renders the marks instead of re-parsing the whole document on every press + * of Enter. + */ +const NoteSearchActiveIndexContext = createContext(null) + +/* + * A note paints its own card fill, so the editor panel's fixed orange cannot + * simply be reused: it disappears against a light card and fights the white + * text on a dark one. Other matches wash the card's own colour, and the current + * one paints both fill and text so it reads on every colour in the palette. + */ +const NOTE_SEARCH_MARK_CLASS = 'rounded-sm bg-current/20 text-inherit' +const NOTE_SEARCH_ACTIVE_MARK_CLASS = 'rounded-sm bg-orange-400 text-black' + +interface NoteSearchMarkProps { + children?: ReactNode + 'data-note-search-index'?: string +} + +function NoteSearchMark({ + children, + 'data-note-search-index': indexAttribute, +}: NoteSearchMarkProps) { + const activeIndex = useContext(NoteSearchActiveIndexContext) + const index = Number.parseInt(indexAttribute ?? '', 10) + const isActive = Number.isInteger(index) && index === activeIndex + + return ( + + {children} + + ) +} + +/** + * The title with its search hit marked, or undefined to render it plain. + * + * Takes a range rather than a query: a name match carries an exact one, so + * there is no occurrence to guess at the way there is in the markdown body. + */ +function renderMarkedName(name: string, range: NoteSearchRange | null): ReactNode | undefined { + if (!range) return undefined + return ( + <> + {name.slice(0, range.start)} + + {name.slice(range.start, range.end)} + + {name.slice(range.end)} + + ) +} + const NOTE_COMPONENTS = { p: ({ children }: { children?: ReactNode }) => (

{children}

@@ -267,6 +336,7 @@ const NOTE_COMPONENTS = { em: ({ children }: { children?: ReactNode }) => ( {children} ), + mark: NoteSearchMark, blockquote: ({ children }: { children?: ReactNode }) => (
{children} @@ -305,12 +375,52 @@ const NOTE_COMPONENTS = { */ export const NOTE_MARKDOWN_FLOW = 'space-y-4 [&>*:first-child]:mt-0 [&>*:last-child]:mb-0' -const NoteMarkdown = memo(function NoteMarkdown({ content }: { content: string }) { +interface NoteMarkdownProps { + content: string + /** Omitted when no search points here, so the pipeline stays the default one. */ + searchQuery?: string +} + +const NoteMarkdown = memo(function NoteMarkdown({ content, searchQuery }: NoteMarkdownProps) { + /* + * `defaultRehypePlugins` is a record keyed by role, not a list — the array + * Streamdown actually defaults to is `Object.values` of it, in that order. + * Appending rather than replacing is what keeps this note's sanitization and + * link hardening intact; the marks are added afterwards precisely because + * sanitization would otherwise strip them as an unknown tag. + */ + const rehypePlugins = useMemo( + () => + searchQuery + ? [ + ...Object.values(defaultRehypePlugins), + [noteSearchHighlightPlugin, { query: searchQuery }], + ] + : undefined, + [searchQuery] + ) + return ( + /* + * Keyed on the query so a change to it remounts. + * + * Streamdown is memoised behind a hand-written comparator that checks + * `children`, `mode`, `className`, `dir` and friends — but NOT + * `rehypePlugins`, `remarkPlugins` or `components`. Starting or ending a + * search changes only the plugin list, so without a key Streamdown bails + * out and keeps its previous render: marks appear only if something else + * happens to remount the card, and once painted they survive the query + * being cleared. The key is the query rather than a counter because the + * pipeline genuinely has to re-parse when the plugin changes; the current + * occurrence still travels by context, so cycling matches inside one note + * re-renders the marks without remounting the document. + */ {content} @@ -375,6 +485,21 @@ export interface NoteBlockViewProps { renderContentEditor: (props: NoteContentEditorProps) => ReactNode /** Editor-only action bar; omit in read-only / preview contexts. */ actionBar?: ReactNode + /** + * The workflow search match to paint, or null when no search points here. + * + * Applies to the read view only. A note the user has clicked into is a live + * markdown editor holding its own document, and repainting a match inside it + * would put decorations on text the user is editing; the highlight comes back + * when they click out. + */ + searchHighlight?: NoteSearchHighlight | null + /** + * Range of a workflow search hit inside `name`, or null. Separate from + * {@link searchHighlight} because a title and a body are different surfaces + * with different match shapes, and only one of the two can be current. + */ + nameSearchRange?: NoteSearchRange | null } /** @@ -402,6 +527,8 @@ export function NoteBlockView({ onImageFilesDrop, renderContentEditor, actionBar, + searchHighlight = null, + nameSearchRange = null, }: NoteBlockViewProps) { const colorOption = getNoteColorOption(noteColor) const showActionMenu = Boolean(actionBar) @@ -423,6 +550,12 @@ export function NoteBlockView({ const [isFileDropTarget, setIsFileDropTarget] = useState(false) const [canScrollUp, setCanScrollUp] = useState(false) const [canScrollDown, setCanScrollDown] = useState(false) + /* Unpacked to primitives so the scroll below re-runs when the match moves and + not merely when the search panel hands over an equal target under a new + identity — which it does often, and which would yank a note the user has + scrolled by hand back onto the mark. */ + const searchQuery = searchHighlight?.query + const searchOccurrenceIndex = searchHighlight?.occurrenceIndex const activeContent = editingField === 'content' ? draftContent : content const isEmpty = activeContent.trim().length === 0 const hasVisualFocus = isFocused || isExpanded @@ -680,6 +813,58 @@ export function NoteBlockView({ updateScrollFades() }, [content, editingField, hasVisualFocus, updateScrollFades]) + /** + * Scrolls the current search match into the card's own scroll region. + * + * `scrollTop` arithmetic, never `scrollIntoView`: the card sits inside + * ReactFlow's transformed viewport, and `scrollIntoView` keeps walking past + * this region to scroll every scrollable ancestor — which drags the canvas + * itself off-frame. Summing `offsetTop` up the offset-parent chain stays in + * layout space, so the canvas zoom never enters the arithmetic. + * + * Called again when the card finishes resizing, because the host expands a + * note that holds a match: the region's height animates for 280ms, so the + * position computed on arrival is measured against a card that is still + * growing. + */ + const scrollActiveMatchIntoView = useCallback(() => { + const scrollRegion = scrollRegionRef.current + const activeMark = scrollRegion?.querySelector('[data-note-search-active]') + if (!scrollRegion || !activeMark) return + + let markTop = 0 + let ancestor: HTMLElement | null = activeMark + while (ancestor && ancestor !== scrollRegion) { + markTop += ancestor.offsetTop + const nextAncestor: Element | null = ancestor.offsetParent + ancestor = nextAncestor instanceof HTMLElement ? nextAncestor : null + } + /* The walk left the region without passing through it — the offsets just + summed are measured against something else entirely. */ + if (!ancestor) return + + const maxScrollTop = Math.max(0, scrollRegion.scrollHeight - scrollRegion.clientHeight) + const centeredTop = markTop - (scrollRegion.clientHeight - activeMark.offsetHeight) / 2 + scrollRegion.scrollTop = Math.max(0, Math.min(centeredTop, maxScrollTop)) + updateScrollFades() + }, [updateScrollFades]) + + /** + * Declared after the reset above so it wins the commit where a note gains + * both focus and a match. + */ + useEffect(() => { + if (searchQuery === undefined || editingField === 'content') return + scrollActiveMatchIntoView() + }, [ + content, + editingField, + isExpanded, + scrollActiveMatchIntoView, + searchOccurrenceIndex, + searchQuery, + ]) + const { rootRef: actionMenuRootRef, hostRef: actionMenuHostRef, @@ -772,7 +957,13 @@ export function NoteBlockView({ icons), so an unguarded handler forces a sync layout read on every hover. */ onTransitionEnd={(event) => { - if (event.target === event.currentTarget) updateScrollFades() + if (event.target !== event.currentTarget) return + updateScrollFades() + /* Only the size transitions, never the card's own colour one: a + re-scroll on every hover tint would yank a note the user has + scrolled by hand back onto the mark. */ + const isResize = event.propertyName === 'height' || event.propertyName === 'width' + if (isResize && searchQuery !== undefined) scrollActiveMatchIntoView() }} onKeyDown={(event) => { if (event.target === event.currentTarget) { @@ -842,13 +1033,17 @@ export function NoteBlockView({ !isEnabled && 'opacity-50' )} > - + + {renderMarkedName(name ?? '', nameSearchRange)} + ) : ( + > + {renderMarkedName(name ?? '', nameSearchRange)} + )} {canEdit && onExpandedChange && ( @@ -998,7 +1193,9 @@ export function NoteBlockView({ {isEmpty ? (

Add note…

) : ( - + + + )} diff --git a/packages/workflow-renderer/src/note/note-search-highlight.test.tsx b/packages/workflow-renderer/src/note/note-search-highlight.test.tsx new file mode 100644 index 00000000000..5a9bef3925f --- /dev/null +++ b/packages/workflow-renderer/src/note/note-search-highlight.test.tsx @@ -0,0 +1,325 @@ +/** + * @vitest-environment jsdom + * + * A workflow search match inside a Note has to be visible. + * + * The editor panel renders nothing for a Note, so a match in one counted + * towards the result total and then highlighted nowhere — the first two of six + * hits landed on a note and looked like a broken search. The card's read view + * is the only surface that can answer, so these cover the two halves of that: + * which occurrence the card is told to paint, and that the paint survives the + * markdown pipeline (sanitization strips unknown tags, so a mark added in the + * wrong place vanishes without a word). + */ + +import { act } from 'react' +import type { Root } from 'hast' +import { createRoot, type Root as ReactRoot } from 'react-dom/client' +import { afterEach, beforeAll, describe, expect, it } from 'vitest' +import { + NoteBlockView, + type NoteContentEditorProps, + type NoteSearchHighlight, + type NoteSearchRange, +} from '../index' +import { + countNoteSearchOccurrencesBefore, + forEachNoteSearchOccurrence, + noteSearchHighlightPlugin, +} from './note-search-highlight' + +/* Assigned rather than `vi.stubGlobal`ed: the suite runs with `unstubGlobals`, which restores stubs + before every test and would strip these back out after the first one. */ +beforeAll(() => { + globalThis.ResizeObserver = class { + observe() {} + unobserve() {} + disconnect() {} + } as unknown as typeof ResizeObserver + window.matchMedia = ((query: string) => ({ + matches: false, + media: query, + addEventListener: () => {}, + removeEventListener: () => {}, + addListener: () => {}, + removeListener: () => {}, + onchange: null, + dispatchEvent: () => false, + })) as unknown as typeof window.matchMedia +}) + +function renderTestContentEditor(_props: NoteContentEditorProps) { + return null +} + +let host: HTMLDivElement | null = null +let root: ReactRoot | null = null + +afterEach(() => { + if (root) act(() => root?.unmount()) + host?.remove() + host = null + root = null +}) + +interface RenderNoteOptions { + name?: string + nameSearchRange?: NoteSearchRange | null +} + +function noteElement( + content: string, + searchHighlight: NoteSearchHighlight | null, + options: RenderNoteOptions +) { + return ( + undefined} + renderContentEditor={renderTestContentEditor} + searchHighlight={searchHighlight} + nameSearchRange={options.nameSearchRange ?? null} + /> + ) +} + +function renderNote( + content: string, + searchHighlight: NoteSearchHighlight | null, + options: RenderNoteOptions = {} +): HTMLDivElement { + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + host = document.createElement('div') + document.body.appendChild(host) + root = createRoot(host) + act(() => root?.render(noteElement(content, searchHighlight, options))) + return host +} + +/** Re-renders the mounted card, the way a live search changing does. */ +function rerenderNote( + content: string, + searchHighlight: NoteSearchHighlight | null, + options: RenderNoteOptions = {} +): void { + act(() => root?.render(noteElement(content, searchHighlight, options))) +} + +function paragraphTree(...values: string[]): Root { + return { + type: 'root', + children: values.map((value) => ({ + type: 'element', + tagName: 'p', + properties: {}, + children: [{ type: 'text', value }], + })), + } +} + +describe('note search occurrence scanning', () => { + it('matches case-insensitively, like the workflow search index', () => { + const starts: number[] = [] + forEachNoteSearchOccurrence('Secret and secret', 'SECRET', (start) => starts.push(start)) + expect(starts).toEqual([0, 11]) + }) + + it('does not overlap a self-overlapping query', () => { + const starts: number[] = [] + forEachNoteSearchOccurrence('aaaa', 'aa', (start) => starts.push(start)) + expect(starts).toEqual([0, 2]) + }) + + it('counts the occurrences that start before an offset', () => { + const content = 'KEY one KEY two KEY' + expect(countNoteSearchOccurrencesBefore(content, 'KEY', 0)).toBe(0) + expect(countNoteSearchOccurrencesBefore(content, 'KEY', 8)).toBe(1) + expect(countNoteSearchOccurrencesBefore(content, 'KEY', 16)).toBe(2) + }) + + it('reports no occurrences for an empty query', () => { + expect(countNoteSearchOccurrencesBefore('anything', '', 4)).toBe(0) + }) +}) + +describe('note search rehype plugin', () => { + it('numbers marks in document order across elements', () => { + const tree = paragraphTree('one KEY here', 'and KEY again') + noteSearchHighlightPlugin({ query: 'KEY' })(tree) + + const marks = tree.children.flatMap((paragraph) => + paragraph.type === 'element' + ? paragraph.children.filter((child) => child.type === 'element' && child.tagName === 'mark') + : [] + ) + expect(marks).toHaveLength(2) + expect( + marks.map((mark) => mark.type === 'element' && mark.properties.dataNoteSearchIndex) + ).toEqual(['0', '1']) + }) + + it('keeps the text either side of a match', () => { + const tree = paragraphTree('one KEY here') + noteSearchHighlightPlugin({ query: 'KEY' })(tree) + + const paragraph = tree.children[0] + const values = + paragraph.type === 'element' + ? paragraph.children.map((child) => + child.type === 'text' + ? child.value + : child.type === 'element' && child.children[0]?.type === 'text' + ? child.children[0].value + : '' + ) + : [] + expect(values).toEqual(['one ', 'KEY', ' here']) + }) + + it('does not re-scan the text it just wrapped', () => { + const tree = paragraphTree('KEYKEY') + noteSearchHighlightPlugin({ query: 'KEY' })(tree) + + const paragraph = tree.children[0] + expect(paragraph.type === 'element' && paragraph.children).toHaveLength(2) + }) + + it('leaves a tree without an occurrence untouched', () => { + const tree = paragraphTree('nothing to see') + const before = structuredClone(tree) + noteSearchHighlightPlugin({ query: 'KEY' })(tree) + expect(tree).toEqual(before) + }) + + it('does nothing for an empty query', () => { + const tree = paragraphTree('KEY') + const before = structuredClone(tree) + noteSearchHighlightPlugin({ query: '' })(tree) + expect(tree).toEqual(before) + }) +}) + +describe('note search highlight rendering', () => { + it('marks every occurrence in the read view', () => { + const container = renderNote('first SB_SECRET line\n\nsecond SB_SECRET line', { + query: 'SB_SECRET', + occurrenceIndex: 0, + }) + + const marks = container.querySelectorAll('mark') + expect(marks).toHaveLength(2) + expect(Array.from(marks, (mark) => mark.textContent)).toEqual(['SB_SECRET', 'SB_SECRET']) + }) + + /* The ordinal reaches the mark component as a `data-*` prop only because hast + spells the property `dataNoteSearchIndex` and the JSX runtime converts it + back. Get that spelling wrong and every mark renders as a non-current one, + silently. */ + it('paints only the current occurrence as active', () => { + const container = renderNote('first SB_SECRET line\n\nsecond SB_SECRET line', { + query: 'SB_SECRET', + occurrenceIndex: 1, + }) + + const marks = Array.from(container.querySelectorAll('mark')) + expect(marks.map((mark) => mark.hasAttribute('data-note-search-active'))).toEqual([false, true]) + }) + + it('survives sanitization inside formatted markdown', () => { + const container = renderNote('## Heading SB_SECRET\n\n- item **SB_SECRET**', { + query: 'SB_SECRET', + occurrenceIndex: 0, + }) + + expect(container.querySelectorAll('mark')).toHaveLength(2) + expect(container.querySelector('h2 mark')).not.toBeNull() + expect(container.querySelector('strong mark')).not.toBeNull() + }) + + it('renders no marks when no search points at the note', () => { + const container = renderNote('first SB_SECRET line', null) + expect(container.querySelectorAll('mark')).toHaveLength(0) + }) + + /* An env-var token is indexed as one `environment` match spanning the whole + `{{…}}`, not as the plain text the user typed into the search box. The card + marks what is on screen, so a partial query still has to land. */ + it('marks a partial query inside an environment token', () => { + const container = renderNote('{{TE_SECRET}}', { query: '{{TE', occurrenceIndex: 0 }) + + const mark = container.querySelector('mark') + expect(mark?.textContent).toBe('{{TE') + expect(mark?.hasAttribute('data-note-search-active')).toBe(true) + }) +}) + +/* + * Every case above mounts the card fresh, which is the one situation that + * cannot catch this: Streamdown is memoised behind a comparator that ignores + * `rehypePlugins`, so on an already-mounted card a plugin change alone does not + * re-render it. Marks then outlived the query that produced them — cleared the + * search box and the note stayed highlighted. + */ +describe('note search highlight on an already-mounted card', () => { + it('clears the marks when the query is cleared', () => { + const container = renderNote('first SB_SECRET line', { query: 'SB_SECRET', occurrenceIndex: 0 }) + expect(container.querySelectorAll('mark')).toHaveLength(1) + + rerenderNote('first SB_SECRET line', null) + + expect(container.querySelectorAll('mark')).toHaveLength(0) + }) + + it('re-marks as the query is edited down', () => { + const container = renderNote('{{TE_SECRET}}', { query: '{{TE', occurrenceIndex: 0 }) + expect(container.querySelector('mark')?.textContent).toBe('{{TE') + + rerenderNote('{{TE_SECRET}}', { query: '{{T', occurrenceIndex: 0 }) + + expect(container.querySelector('mark')?.textContent).toBe('{{T') + }) + + it('moves the current mark without dropping the others', () => { + const container = renderNote('one KEY two KEY', { query: 'KEY', occurrenceIndex: 0 }) + const activeOf = () => + Array.from(container.querySelectorAll('mark'), (mark) => + mark.hasAttribute('data-note-search-active') + ) + expect(activeOf()).toEqual([true, false]) + + rerenderNote('one KEY two KEY', { query: 'KEY', occurrenceIndex: 1 }) + + expect(activeOf()).toEqual([false, true]) + }) +}) + +describe('note title search highlight', () => { + it('marks the range inside the title', () => { + const container = renderNote('body', null, { + name: 'Handler Notes', + nameSearchRange: { start: 8, end: 13 }, + }) + + const mark = container.querySelector('mark') + expect(mark?.textContent).toBe('Notes') + }) + + it('keeps the rest of the title intact', () => { + const container = renderNote('body', null, { + name: 'Handler Notes', + nameSearchRange: { start: 8, end: 13 }, + }) + + expect(container.querySelector('mark')?.parentElement?.textContent).toBe('Handler Notes') + }) + + it('leaves the title plain without a range', () => { + const container = renderNote('body', null, { name: 'Handler Notes' }) + expect(container.querySelectorAll('mark')).toHaveLength(0) + }) +}) diff --git a/packages/workflow-renderer/src/note/note-search-highlight.ts b/packages/workflow-renderer/src/note/note-search-highlight.ts new file mode 100644 index 00000000000..c49154e27e6 --- /dev/null +++ b/packages/workflow-renderer/src/note/note-search-highlight.ts @@ -0,0 +1,160 @@ +import type { Element, Root, Text } from 'hast' + +/** + * Which occurrence of a workflow search query a note should paint as current. + * + * `occurrenceIndex` counts occurrences in the note's **markdown source**, in + * document order, because that is the only thing the search index and the card + * share: a match carries a character range into the raw value, and the read + * view renders a tree that has thrown those offsets away. + * + * The two agree whenever the query occurs in text markdown also renders — the + * ordinary case, and every case for the plain prose notes are usually made of. + * They diverge when an occurrence lives somewhere the read view does not print + * (a link's URL, an image's `src`, an HTML attribute), which shifts every later + * occurrence's rendered position by one. The mark then lands on a neighbouring + * occurrence rather than nowhere, so search still leads to the right region of + * the note. Closing that gap means carrying source offsets through the markdown + * pipeline, which is a much larger change than the miss is worth. + */ +export interface NoteSearchHighlight { + query: string + occurrenceIndex: number +} + +export interface NoteSearchHighlightOptions { + query: string +} + +/** Half-open character range of a search hit inside a note's title. */ +export interface NoteSearchRange { + start: number + end: number +} + +/** + * Hast property name for a mark's ordinal. Hast spells `data-*` attributes in + * camelCase and the JSX runtime converts them back, so the DOM attribute — and + * the prop the `mark` component receives — is `data-note-search-index`. + */ +export const NOTE_SEARCH_MARK_INDEX_PROPERTY = 'dataNoteSearchIndex' + +/** + * Visits every occurrence of `query` in `text`, case-insensitively and without + * overlaps. + * + * Deliberately the same scan the workflow search indexer runs over the raw + * value (`findTextRanges`): counting and marking have to agree on what "the + * third occurrence" means, and an overlapping scan here against a + * non-overlapping one there would silently offset every mark in a note whose + * query self-overlaps (`aa` in `aaaa`). + */ +export function forEachNoteSearchOccurrence( + text: string, + query: string, + visit: (start: number, end: number) => void +): void { + if (!query) return + + const haystack = text.toLowerCase() + const needle = query.toLowerCase() + const step = Math.max(needle.length, 1) + + let index = haystack.indexOf(needle) + while (index !== -1) { + visit(index, index + needle.length) + index = haystack.indexOf(needle, index + step) + } +} + +/** + * How many occurrences of `query` start before `offset` in `content` — the + * ordinal of the occurrence that starts there. + */ +export function countNoteSearchOccurrencesBefore( + content: string, + query: string, + offset: number +): number { + let count = 0 + forEachNoteSearchOccurrence(content, query, (start) => { + if (start < offset) count += 1 + }) + return count +} + +interface MarkCounter { + value: number +} + +/** + * Splits a text node around every occurrence, wrapping each in a `mark` that + * carries its document-order ordinal. Returns an empty list when the node holds + * no occurrence, so an untouched node keeps its identity in the tree. + */ +function splitTextNode(node: Text, query: string, counter: MarkCounter): Array { + const { value } = node + const pieces: Array = [] + let cursor = 0 + + forEachNoteSearchOccurrence(value, query, (start, end) => { + if (start > cursor) { + pieces.push({ type: 'text', value: value.slice(cursor, start) }) + } + pieces.push({ + type: 'element', + tagName: 'mark', + properties: { [NOTE_SEARCH_MARK_INDEX_PROPERTY]: String(counter.value) }, + children: [{ type: 'text', value: value.slice(start, end) }], + }) + counter.value += 1 + cursor = end + }) + + if (pieces.length === 0) return [] + if (cursor < value.length) { + pieces.push({ type: 'text', value: value.slice(cursor) }) + } + return pieces +} + +function markNode(node: Root | Element, query: string, counter: MarkCounter): void { + for (let index = 0; index < node.children.length; index += 1) { + const child = node.children[index] + + if (child.type === 'element') { + markNode(child, query, counter) + continue + } + if (child.type !== 'text') continue + + const pieces = splitTextNode(child, query, counter) + if (pieces.length === 0) continue + + node.children.splice(index, 1, ...pieces) + /* Past the pieces just spliced in: their own text carries the match, and + re-scanning it would mark the inside of a mark. */ + index += pieces.length - 1 + } +} + +/** + * Rehype plugin that wraps every rendered occurrence of `query` in a `mark`. + * + * Runs **after** Streamdown's own defaults rather than replacing them, so + * sanitization and hardening have already had the tree: these marks are + * generated from text that survived both, carry no user-supplied markup, and + * would otherwise be stripped as unknown tags. + * + * Must be used as a plugin **tuple** — `[noteSearchHighlightPlugin, { query }]`. + * Streamdown caches one processor per plugin list and keys it on each plugin's + * function name plus its serialized options, so a closure-per-query would key + * every query to the same empty name and paint the second query's note with the + * first query's marks. + */ +export function noteSearchHighlightPlugin({ query }: NoteSearchHighlightOptions) { + return (tree: Root): void => { + if (!query) return + markNode(tree, query, { value: 0 }) + } +} From 7d75e145e5dcb0fb3e3b33c65f37d6070db2343c Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Thu, 20 Aug 2026 14:35:49 -0700 Subject: [PATCH 2/5] fix(search): honour fences and folded whitespace in Note highlighting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 `
`, 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) --- .../rich-markdown-editor/markdown-fidelity.ts | 80 ++++--- .../rich-markdown-editor/round-trip.test.ts | 16 ++ .../lib/workflows/search-replace/indexer.ts | 2 +- .../search-replace/resources/references.ts | 13 +- .../search-replace/resources/resolvers.ts | 2 +- packages/utils/src/string.ts | 19 ++ .../src/note/note-search-highlight.test.tsx | 113 +++++++++- .../src/note/note-search-highlight.ts | 202 +++++++++++++++--- 8 files changed, 373 insertions(+), 74 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts index f6b661e8f02..1a0d3d56153 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts @@ -9,24 +9,17 @@ const FRONTMATTER_REGEX = /^---\r?\n(?:[\s\S]*?\r?\n)?---[ \t]*(?:\r?\n)*/ const ESCAPED_CALLOUT_REGEX = /^(\s*>(?:\s*>)*\s*)\\\[!([A-Za-z]+)\\\]/gm /** - * A code region \u2014 fenced block or inline span. Never rewritten by the cleanups below, and always - * the FIRST branch of the alternations that use it so a candidate sitting inside code is consumed - * as code and left verbatim. + * Alternates a code region (fenced block or inline span \u2014 never rewritten) with an inline link whose + * destination has no title and isn't angle-bracketed. The code branch is listed first so a link inside + * code is consumed as code and left untouched. The destination stops at `)` / whitespace, so a link + * carrying a title (`[x](url "t")`) never matches and is preserved verbatim. */ -const CODE_REGION_SOURCE = '(```[\\s\\S]*?```|~~~[\\s\\S]*?~~~|`[^`\\n]+`)' - -/** - * Alternates a code region with an inline link whose destination has no title and isn't - * angle-bracketed. The destination stops at `)` / whitespace, so a link carrying a title - * (`[x](url "t")`) never matches and is preserved verbatim. - */ -const CODE_OR_PLAIN_LINK_REGEX = new RegExp( - `${CODE_REGION_SOURCE}|\\[([^\\]]+)]\\(([^)\\s<>]+)\\)`, - 'g' -) +const CODE_OR_PLAIN_LINK_REGEX = + /(```[\s\S]*?```|~~~[\s\S]*?~~~|`[^`\n]+`)|\[([^\]]+)]\(([^)\s<>]+)\)/g +const HTTP_URL_REGEX = /^https?:\/\/\S+$/i /** - * Alternates a code region with a single underscore that has a letter or digit on both sides. + * Alternates an inline code span with a single underscore that has a letter or digit on both sides. * * CommonMark's intraword rule means such an underscore can neither open nor close emphasis, so the * serializer's backslash before it carries no meaning \u2014 it just writes `SB\_ACTION\_ROUTER\_SECRET` @@ -34,25 +27,52 @@ const CODE_OR_PLAIN_LINK_REGEX = new RegExp( * against the stored markdown rather than the rendered text: searching `SB_ACTION` finds nothing in * a note whose stored form has a backslash the reader never sees. * - * Code is excluded because the serializer emits it verbatim: a `\_` inside a fence is the author's - * own backslash, not an escape this may drop. + * Code is excluded because the serializer emits it verbatim: a `\_` inside a span is the author's own + * backslash, not an escape this may drop. The span branch matches a backtick RUN and requires a run of + * the same length to close it, per CommonMark \u2014 a fixed single-backtick pattern would read ``` ``a`b`` ``` + * as `` `a` `` plus loose text and rewrite the interior. Fenced blocks are handled a line at a time by + * {@link unescapeIntrawordUnderscores}, which is the only way to honour a fence of any length. * - * Written with a capture group rather than a lookbehind: lookbehind only landed in Safari 16.4, and - * an unsupported one throws when the pattern is constructed \u2014 taking the whole editor module with - * it. The group is consumed and put back, and the lookahead is not, so runs like `A\_B\_C` still - * match on every pair. + * The flanking character is a capture group rather than a lookbehind: lookbehind only landed in + * Safari 16.4, and an unsupported one throws when the pattern is constructed \u2014 taking the whole editor + * module with it. The group is consumed and put back, and the lookahead is not, so runs like `A\_B\_C` + * still match on every pair. */ -const CODE_OR_INTRAWORD_ESCAPED_UNDERSCORE = new RegExp( - `${CODE_REGION_SOURCE}|([\\p{L}\\p{N}])\\\\_(?=[\\p{L}\\p{N}])`, - 'gu' -) -const HTTP_URL_REGEX = /^https?:\/\/\S+$/i +const CODE_SPAN_OR_INTRAWORD_ESCAPED_UNDERSCORE = + /(`+)((?:[^`]|(?!\1)`)*?)\1(?!`)|([\p{L}\p{N}])\\_(?=[\p{L}\p{N}])/gu -/** Drops the meaningless backslash before an intraword underscore, outside code. */ +/** + * Drops the meaningless backslash before an intraword underscore, outside code. + * + * Fenced blocks are skipped a line at a time, tracking the opening delimiter the same way + * {@link stripEmptyListItemLines} does: a fence is three OR MORE backticks or tildes and is closed + * only by a run of the same character at least as long, so a `````` ```` `````-fenced block wrapping + * ``` ``` ``` stays code throughout. Matching a fixed ``` pair instead would end the region early and + * hand the rest of the author's code to the rewrite. + */ function unescapeIntrawordUnderscores(markdown: string): string { - return markdown.replace(CODE_OR_INTRAWORD_ESCAPED_UNDERSCORE, (match, code, flank) => - code ? code : `${flank}_` - ) + const lines = markdown.split('\n') + let fence: string | null = null + + for (let i = 0; i < lines.length; i++) { + const delimiter = lines[i].match(FENCE_DELIMITER)?.[1] + + if (fence) { + if (delimiter && delimiter[0] === fence[0] && delimiter.length >= fence.length) fence = null + continue + } + if (delimiter) { + fence = delimiter + continue + } + + lines[i] = lines[i].replace( + CODE_SPAN_OR_INTRAWORD_ESCAPED_UNDERSCORE, + (match, ticks, _span, flank) => (ticks === undefined ? `${flank}_` : match) + ) + } + + return lines.join('\n') } /** diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip.test.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip.test.ts index 4a903af1ca8..bdced976c8f 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip.test.ts @@ -237,6 +237,22 @@ describe('editor markdown round-trip', () => { expect(roundTrip('call `a\\_b` here')).toContain('a\\_b') }) + /* A fence is three OR MORE delimiters, closed only by a run at least as long, and an inline span + opens and closes on backtick runs of equal length. Recognising just the shortest form ends the + code region early and hands the rest of the author's code to the rewrite. */ + it('leaves code alone in a longer fence', () => { + expect(roundTrip('````\nx = a\\_b\n```\nstill code y = c\\_d\n````')).toContain('a\\_b') + expect(roundTrip('````\nx = a\\_b\n```\nstill code y = c\\_d\n````')).toContain('c\\_d') + }) + + it('leaves code alone in a tilde fence', () => { + expect(roundTrip('~~~~\nx = a\\_b\n~~~~')).toContain('a\\_b') + }) + + it('leaves code alone in a multi-backtick inline span', () => { + expect(roundTrip('call ``a\\_b`c`` here')).toContain('a\\_b') + }) + it('preserves an image url (does not drop the src)', () => { const out = roundTrip('![alt](https://example.com/i.png)') expect(out).toContain('![alt](https://example.com/i.png)') diff --git a/apps/sim/lib/workflows/search-replace/indexer.ts b/apps/sim/lib/workflows/search-replace/indexer.ts index 18de77eca65..a2d022753f8 100644 --- a/apps/sim/lib/workflows/search-replace/indexer.ts +++ b/apps/sim/lib/workflows/search-replace/indexer.ts @@ -1,4 +1,5 @@ import { isRecordLike } from '@sim/utils/object' +import { foldSearchWhitespace } from '@sim/utils/string' import { DEFAULT_SUBBLOCK_TYPE } from '@sim/workflow-persistence/subblocks' import type { SubBlockType } from '@sim/workflow-types/blocks' import { isWorkflowBlockProtected } from '@sim/workflow-types/workflow' @@ -10,7 +11,6 @@ import { } from '@/lib/workflows/search-replace/json-value-fields' import { buildBlockNamesByReferencePrefix, - foldSearchWhitespace, getResourceKindForSubBlock, matchesSearchText, parseInlineReferences, diff --git a/apps/sim/lib/workflows/search-replace/resources/references.ts b/apps/sim/lib/workflows/search-replace/resources/references.ts index 1adabcf9fa1..cfd718c833d 100644 --- a/apps/sim/lib/workflows/search-replace/resources/references.ts +++ b/apps/sim/lib/workflows/search-replace/resources/references.ts @@ -1,3 +1,4 @@ +import { foldSearchWhitespace } from '@sim/utils/string' import { getWorkflowSearchSubBlockResourceKind, parseWorkflowSearchSubBlockResources, @@ -140,18 +141,6 @@ export function parseStructuredResourceReferences( return parseWorkflowSearchSubBlockResources(value, subBlockConfig, selectorContext) } -/** - * Maps every Unicode whitespace character to a plain space, one-to-one. - * Agent-authored block names and values routinely carry non-breaking or - * narrow spaces that render identically to " " but never equal a typed - * space, silently hiding matches. The replacement is length-preserving - * (every `\s` character is a single UTF-16 unit), so indexes into the - * folded string remain valid ranges into the original. - */ -export function foldSearchWhitespace(value: string): string { - return value.replace(/\s/g, ' ') -} - export function matchesSearchText( candidate: string, query: string | undefined, diff --git a/apps/sim/lib/workflows/search-replace/resources/resolvers.ts b/apps/sim/lib/workflows/search-replace/resources/resolvers.ts index 29368d41859..94afb29389c 100644 --- a/apps/sim/lib/workflows/search-replace/resources/resolvers.ts +++ b/apps/sim/lib/workflows/search-replace/resources/resolvers.ts @@ -1,4 +1,4 @@ -import { foldSearchWhitespace } from '@/lib/workflows/search-replace/resources/references' +import { foldSearchWhitespace } from '@sim/utils/string' import type { WorkflowSearchMatch, WorkflowSearchMatchKind, diff --git a/packages/utils/src/string.ts b/packages/utils/src/string.ts index 38a7388cfbd..7f6c542cf2c 100644 --- a/packages/utils/src/string.ts +++ b/packages/utils/src/string.ts @@ -162,3 +162,22 @@ export function formatQuotedNameList(names: string[], maxListed: number): string const overflow = names.length - maxListed return overflow > 0 ? `${listed} and ${overflow} more` : listed } + +/** + * Maps every Unicode whitespace character to a plain space, one-to-one. + * + * Agent-authored block names and values routinely carry non-breaking or narrow + * spaces that render identically to " " but never equal a typed space, silently + * hiding matches. The replacement is length-preserving (every `\s` character is + * a single UTF-16 unit), so indexes into the folded string remain valid ranges + * into the original. + * + * Lives here rather than beside the workflow search index because the Note card + * on the canvas has to fold identically to find the same occurrences, and it + * renders from `@sim/workflow-renderer` — a package, which cannot import from + * `apps/*`. Two copies of this rule silently disagreeing is precisely the bug + * that made a match count in the panel and highlight nowhere on the card. + */ +export function foldSearchWhitespace(value: string): string { + return value.replace(/\s/g, ' ') +} diff --git a/packages/workflow-renderer/src/note/note-search-highlight.test.tsx b/packages/workflow-renderer/src/note/note-search-highlight.test.tsx index 5a9bef3925f..8733905ab67 100644 --- a/packages/workflow-renderer/src/note/note-search-highlight.test.tsx +++ b/packages/workflow-renderer/src/note/note-search-highlight.test.tsx @@ -13,7 +13,7 @@ */ import { act } from 'react' -import type { Root } from 'hast' +import type { Element, ElementContent, Root, RootContent } from 'hast' import { createRoot, type Root as ReactRoot } from 'react-dom/client' import { afterEach, beforeAll, describe, expect, it } from 'vitest' import { @@ -204,6 +204,117 @@ describe('note search rehype plugin', () => { }) }) +/* + * The indexer folds every `\s` to a space before matching, so a phrase can match + * across a soft line break — which `remark-breaks` renders as a `
` splitting + * the phrase over two text nodes. A per-node scan saw neither half, leaving the + * hit counted in the panel and highlighted nowhere on the card. + */ +describe('note search across inline boundaries', () => { + function markedTextsOf(tree: Root): string[] { + const texts: string[] = [] + const walk = (node: Root | Element) => { + /* `Root['children']` and `Element['children']` are different unions, so iterating the + parameter directly widens each child to their intersection and drops narrowing. */ + const children: Array = node.children + for (const child of children) { + if (child.type !== 'element') continue + if (child.tagName === 'mark') { + const [first] = child.children + texts.push(first?.type === 'text' ? first.value : '') + continue + } + walk(child) + } + } + walk(tree) + return texts + } + + function paragraphWithBreak(before: string, after: string): Root { + return { + type: 'root', + children: [ + { + type: 'element', + tagName: 'p', + properties: {}, + children: [ + { type: 'text', value: before }, + { type: 'element', tagName: 'br', properties: {}, children: [] }, + { type: 'text', value: after }, + ], + }, + ], + } + } + + it('marks a phrase spanning a soft line break', () => { + const tree = paragraphWithBreak('the quick', 'brown fox') + noteSearchHighlightPlugin({ query: 'quick brown' })(tree) + expect(markedTextsOf(tree)).toEqual(['quick', 'brown']) + }) + + it('gives both halves of one hit the same ordinal', () => { + const tree = paragraphWithBreak('the quick', 'brown fox') + noteSearchHighlightPlugin({ query: 'quick brown' })(tree) + + const ordinals: unknown[] = [] + const walk = (node: Root | Element) => { + /* `Root['children']` and `Element['children']` are different unions, so iterating the + parameter directly widens each child to their intersection and drops narrowing. */ + const children: Array = node.children + for (const child of children) { + if (child.type !== 'element') continue + if (child.tagName === 'mark') ordinals.push(child.properties.dataNoteSearchIndex) + else walk(child) + } + } + walk(tree) + expect(ordinals).toEqual(['0', '0']) + }) + + it('marks a phrase spanning a bold word', () => { + const tree: Root = { + type: 'root', + children: [ + { + type: 'element', + tagName: 'p', + properties: {}, + children: [ + { type: 'text', value: 'a ' }, + { + type: 'element', + tagName: 'strong', + properties: {}, + children: [{ type: 'text', value: 'bold' }], + }, + { type: 'text', value: ' word' }, + ], + }, + ], + } + noteSearchHighlightPlugin({ query: 'a bold word' })(tree) + expect(markedTextsOf(tree)).toEqual(['a ', 'bold', ' word']) + }) + + /* Two paragraphs are not one phrase on screen. Joining them would invent a hit + the reader cannot see — and one the indexer never counted, since the source + carries a blank line there, not a single space. */ + it('does not join text across a block boundary', () => { + const tree = paragraphTree('the quick', 'brown fox') + noteSearchHighlightPlugin({ query: 'quick brown' })(tree) + expect(markedTextsOf(tree)).toEqual([]) + }) + + it('folds a non-breaking space the way the indexer does', () => { + const tree = paragraphTree('a b') + noteSearchHighlightPlugin({ query: 'a b' })(tree) + expect(markedTextsOf(tree)).toEqual(['a b']) + }) +}) + describe('note search highlight rendering', () => { it('marks every occurrence in the read view', () => { const container = renderNote('first SB_SECRET line\n\nsecond SB_SECRET line', { diff --git a/packages/workflow-renderer/src/note/note-search-highlight.ts b/packages/workflow-renderer/src/note/note-search-highlight.ts index c49154e27e6..9b0a11f5382 100644 --- a/packages/workflow-renderer/src/note/note-search-highlight.ts +++ b/packages/workflow-renderer/src/note/note-search-highlight.ts @@ -1,3 +1,4 @@ +import { foldSearchWhitespace } from '@sim/utils/string' import type { Element, Root, Text } from 'hast' /** @@ -11,11 +12,12 @@ import type { Element, Root, Text } from 'hast' * The two agree whenever the query occurs in text markdown also renders — the * ordinary case, and every case for the plain prose notes are usually made of. * They diverge when an occurrence lives somewhere the read view does not print - * (a link's URL, an image's `src`, an HTML attribute), which shifts every later - * occurrence's rendered position by one. The mark then lands on a neighbouring - * occurrence rather than nowhere, so search still leads to the right region of - * the note. Closing that gap means carrying source offsets through the markdown - * pipeline, which is a much larger change than the miss is worth. + * (a link's URL, an image's `src`, an HTML attribute) or spans two blocks, + * which shifts every later occurrence's rendered position by one. The mark then + * lands on a neighbouring occurrence rather than nowhere, so search still leads + * to the right region of the note. Closing that gap means carrying source + * offsets through the markdown pipeline, which is a much larger change than the + * miss is worth. */ export interface NoteSearchHighlight { query: string @@ -39,15 +41,55 @@ export interface NoteSearchRange { */ export const NOTE_SEARCH_MARK_INDEX_PROPERTY = 'dataNoteSearchIndex' +/** + * Elements that do not interrupt a run of text. Text either side of one reads + * as a single phrase, so the scan below joins across them — the same thing the + * browser's own find does in `abc`. + * + * An allowlist rather than a block-level denylist: raw HTML can put any tag in + * this tree, and treating something unrecognised as a break can only ever miss + * a match, while treating it as inline could invent one that is not on screen. + */ +const INLINE_TAG_NAMES: ReadonlySet = new Set([ + 'a', + 'abbr', + 'b', + 'br', + 'cite', + 'code', + 'del', + 'em', + 'i', + 'ins', + 'kbd', + 'mark', + 'q', + 's', + 'samp', + 'small', + 'span', + 'strong', + 'sub', + 'sup', + 'u', + 'var', +]) + /** * Visits every occurrence of `query` in `text`, case-insensitively and without * overlaps. * * Deliberately the same scan the workflow search indexer runs over the raw - * value (`findTextRanges`): counting and marking have to agree on what "the - * third occurrence" means, and an overlapping scan here against a - * non-overlapping one there would silently offset every mark in a note whose - * query self-overlaps (`aa` in `aaaa`). + * value (`findTextRanges`), down to folding whitespace with the shared + * {@link foldSearchWhitespace}: counting and marking have to agree on what "the + * third occurrence" means. An overlapping scan here against a non-overlapping + * one there would silently offset every mark in a note whose query + * self-overlaps (`aa` in `aaaa`), and an unfolded one would miss a phrase the + * indexer matched across a line break. + * + * Case sensitivity is not plumbed through: the search panel is the only caller + * of the indexer and never enables it. If it ever does, this is the second + * place that has to change. */ export function forEachNoteSearchOccurrence( text: string, @@ -56,8 +98,10 @@ export function forEachNoteSearchOccurrence( ): void { if (!query) return - const haystack = text.toLowerCase() - const needle = query.toLowerCase() + /* Folding is length-preserving, so every index below is also a valid index + into the caller's unfolded string. */ + const haystack = foldSearchWhitespace(text).toLowerCase() + const needle = foldSearchWhitespace(query).toLowerCase() const step = Math.max(needle.length, 1) let index = haystack.indexOf(needle) @@ -83,33 +127,97 @@ export function countNoteSearchOccurrencesBefore( return count } -interface MarkCounter { - value: number +/** A text node and where its own text begins within its run. */ +interface TextRun { + node: Text + start: number +} + +/** A slice of one text node that a mark has to wrap, and which match it belongs to. */ +interface NodeMark { + start: number + end: number + ordinal: number } /** - * Splits a text node around every occurrence, wrapping each in a `mark` that - * carries its document-order ordinal. Returns an empty list when the node holds - * no occurrence, so an untouched node keeps its identity in the tree. + * Collects the runs of text that read continuously on screen. + * + * A run ends at any non-inline element, so text in two paragraphs is never + * joined into a phrase the reader cannot see. Within a run every inline + * boundary is crossed, including the `
` that `remark-breaks` puts at a soft + * line break — that break is a single `\n` in the source, which the fold turns + * into a single space, so the run reproduces it as one. */ -function splitTextNode(node: Text, query: string, counter: MarkCounter): Array { +interface RunBuilder { + runs: TextRun[][] + current: TextRun[] | null + length: number +} + +/** Appends text to the open run, opening one if none is. */ +function appendText(builder: RunBuilder, node: Text): void { + if (!builder.current) { + builder.current = [] + builder.runs.push(builder.current) + } + builder.current.push({ node, start: builder.length }) + builder.length += node.value.length +} + +function endRun(builder: RunBuilder): void { + builder.current = null + builder.length = 0 +} + +function collectTextRuns(node: Root | Element, builder: RunBuilder): void { + for (const child of node.children) { + if (child.type === 'text') { + appendText(builder, child) + continue + } + if (child.type !== 'element') continue + + if (child.tagName === 'br') { + /* The newline this stands for is a single `\n` in the source, which the + fold turns into a single space — reproduce it so a phrase the indexer + matched across a soft break also matches here. The node is synthetic + and never reaches the tree; it only carries the offset. */ + if (builder.current) appendText(builder, { type: 'text', value: ' ' }) + continue + } + + /* An inline element continues the run — the builder state is shared, so + descending is all it takes. Anything else breaks it either side. */ + if (INLINE_TAG_NAMES.has(child.tagName)) { + collectTextRuns(child, builder) + continue + } + + endRun(builder) + collectTextRuns(child, builder) + endRun(builder) + } +} + +/** Splits a text node at its marked slices, wrapping each in a `mark`. */ +function splitTextNode(node: Text, marks: NodeMark[]): Array { const { value } = node const pieces: Array = [] let cursor = 0 - forEachNoteSearchOccurrence(value, query, (start, end) => { - if (start > cursor) { - pieces.push({ type: 'text', value: value.slice(cursor, start) }) + for (const mark of marks) { + if (mark.start > cursor) { + pieces.push({ type: 'text', value: value.slice(cursor, mark.start) }) } pieces.push({ type: 'element', tagName: 'mark', - properties: { [NOTE_SEARCH_MARK_INDEX_PROPERTY]: String(counter.value) }, - children: [{ type: 'text', value: value.slice(start, end) }], + properties: { [NOTE_SEARCH_MARK_INDEX_PROPERTY]: String(mark.ordinal) }, + children: [{ type: 'text', value: value.slice(mark.start, mark.end) }], }) - counter.value += 1 - cursor = end - }) + cursor = mark.end + } if (pieces.length === 0) return [] if (cursor < value.length) { @@ -118,17 +226,20 @@ function splitTextNode(node: Text, query: string, counter: MarkCounter): Array): void { for (let index = 0; index < node.children.length; index += 1) { const child = node.children[index] if (child.type === 'element') { - markNode(child, query, counter) + applyMarks(child, marksByNode) continue } if (child.type !== 'text') continue - const pieces = splitTextNode(child, query, counter) + const marks = marksByNode.get(child) + if (!marks) continue + + const pieces = splitTextNode(child, marks) if (pieces.length === 0) continue node.children.splice(index, 1, ...pieces) @@ -141,6 +252,12 @@ function markNode(node: Root | Element, query: string, counter: MarkCounter): vo /** * Rehype plugin that wraps every rendered occurrence of `query` in a `mark`. * + * A match spanning an inline boundary — a soft line break, a bold word — is + * wrapped as several marks sharing one ordinal, so it paints as one hit. A + * per-text-node scan could not see those at all: `remark-breaks` alone was + * enough to hide any phrase the indexer matched across a newline, leaving it + * counted in the panel and highlighted nowhere on the card. + * * Runs **after** Streamdown's own defaults rather than replacing them, so * sanitization and hardening have already had the tree: these marks are * generated from text that survived both, carry no user-supplied markup, and @@ -155,6 +272,33 @@ function markNode(node: Root | Element, query: string, counter: MarkCounter): vo export function noteSearchHighlightPlugin({ query }: NoteSearchHighlightOptions) { return (tree: Root): void => { if (!query) return - markNode(tree, query, { value: 0 }) + + const builder: RunBuilder = { runs: [], current: null, length: 0 } + collectTextRuns(tree, builder) + + const marksByNode = new Map() + let ordinal = 0 + + for (const run of builder.runs) { + const text = run.map((entry) => entry.node.value).join('') + forEachNoteSearchOccurrence(text, query, (start, end) => { + const current = ordinal + ordinal += 1 + for (const entry of run) { + const nodeStart = entry.start + const nodeEnd = nodeStart + entry.node.value.length + const from = Math.max(start, nodeStart) + const to = Math.min(end, nodeEnd) + if (from >= to) continue + const marks = marksByNode.get(entry.node) + const mark = { start: from - nodeStart, end: to - nodeStart, ordinal: current } + if (marks) marks.push(mark) + else marksByNode.set(entry.node, [mark]) + } + }) + } + + if (marksByNode.size === 0) return + applyMarks(tree, marksByNode) } } From 93ceb84a36b7b0a45ff80040ca992b4e0d8b0060 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Thu, 20 Aug 2026 14:48:35 -0700 Subject: [PATCH 3/5] fix(markdown): close a fence only on a bare delimiter run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A closing fence carries nothing but its delimiter run; a line that merely starts with one is content. The guard matched the prefix alone, so an interior line like ` ````example ` inside a same-length fence ended the block, and every cleanup below then processed the author's remaining code as prose — dropping the backslashes from their `a\_b`. Both fence walks in this file shared that flaw, so both now go through one `closesFence`, which requires the run to be followed by nothing but whitespace. Strictly more conservative: a fence stays open longer, so more content is left verbatim. Scope, stated plainly: the serializer always opens a block with one more delimiter than the longest run inside it, so its own output cannot reach this shape today, and `postProcessSerializedMarkdown` only ever sees serializer output. This is a correctness fix that removes an unstated coupling to that choice, not a live corruption path. The tests therefore exercise `postProcessSerializedMarkdown` directly — a round-trip test of the same input would pass either way, which is exactly the vacuous check worth avoiding. Co-Authored-By: Claude Opus 5 (1M context) --- .../rich-markdown-editor/markdown-fidelity.ts | 26 +++++++++++++++---- .../rich-markdown-editor/round-trip.test.ts | 26 +++++++++++++++++++ 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts index 1a0d3d56153..2727454aad7 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts @@ -55,12 +55,12 @@ function unescapeIntrawordUnderscores(markdown: string): string { let fence: string | null = null for (let i = 0; i < lines.length; i++) { - const delimiter = lines[i].match(FENCE_DELIMITER)?.[1] - if (fence) { - if (delimiter && delimiter[0] === fence[0] && delimiter.length >= fence.length) fence = null + if (closesFence(lines[i], fence)) fence = null continue } + + const delimiter = lines[i].match(FENCE_DELIMITER)?.[1] if (delimiter) { fence = delimiter continue @@ -167,6 +167,22 @@ export function normalizeLinkHref(href: string): string { const EMPTY_LIST_ITEM_LINE = /^([ \t]*)(?:[-*+]|\d+[.)])[ \t]*$/ /** A fenced code-block delimiter (``` or ~~~), used to leave code interiors untouched. */ const FENCE_DELIMITER = /^[ \t]*(`{3,}|~{3,})/ +/** + * A line carrying nothing but a delimiter run — the only thing that CLOSES a fence. + * + * An OPENING fence may be followed by an info string (` ```python `), so opening is matched with + * {@link FENCE_DELIMITER}; a closing one may be followed only by whitespace. Treating any + * delimiter-prefixed line as a close ends the block at an interior line like ` ```example ` inside + * a `````` ```` ``````-fence, and every cleanup below then processes the rest of the author's code + * as prose. + */ +const CLOSING_FENCE = /^[ \t]*(`{3,}|~{3,})[ \t]*$/ + +/** Whether `line` closes a fence opened with `fence`: same character, and at least as long. */ +function closesFence(line: string, fence: string): boolean { + const delimiter = line.match(CLOSING_FENCE)?.[1] + return Boolean(delimiter && delimiter[0] === fence[0] && delimiter.length >= fence.length) +} /** Leading indentation of a line, used to detect whether an empty list item has indented children. */ const LEADING_INDENT = /^[ \t]*/ @@ -192,12 +208,12 @@ function stripEmptyListItemLines(markdown: string): string { let fence: string | null = null for (let i = 0; i < lines.length; i++) { const line = lines[i] - const delimiter = line.match(FENCE_DELIMITER)?.[1] if (fence) { kept.push(line) - if (delimiter && delimiter[0] === fence[0] && delimiter.length >= fence.length) fence = null + if (closesFence(line, fence)) fence = null continue } + const delimiter = line.match(FENCE_DELIMITER)?.[1] if (delimiter) { fence = delimiter kept.push(line) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip.test.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip.test.ts index bdced976c8f..d07527fa649 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip.test.ts @@ -98,6 +98,32 @@ describe('markdown-fidelity utils', () => { expect(postProcessSerializedMarkdown('> \\[!NOTE\\]\n> hi')).toBe('> [!NOTE]\n> hi') }) + /* + * A closing fence carries nothing but its delimiter run; a line that merely STARTS with one is + * content. Matching the prefix alone ends the block at an interior line like ` ````example `, + * after which the rest of the author's code is cleaned up as prose and its backslashes vanish. + * + * Exercised directly rather than through `roundTrip` on purpose: the serializer always opens a + * block with one more delimiter than the longest run inside it, so its own output cannot reach + * this shape and a round-trip test of it would pass either way. Relying on that choice staying + * true is an unstated coupling — this keeps the guard honest on any input. + */ + it('does not close a fence on a delimiter-prefixed content line', () => { + const input = '````\nx = a\\_b\n````example\ny = c\\_d\n````\n' + expect(postProcessSerializedMarkdown(input)).toBe(input) + }) + + it('does not close a tilde fence on a delimiter-prefixed content line', () => { + const input = '~~~~\n~~~~note\ny = c\\_d\n~~~~\n' + expect(postProcessSerializedMarkdown(input)).toBe(input) + }) + + it('still closes a fence on a bare delimiter run with trailing spaces', () => { + expect(postProcessSerializedMarkdown('````\nx = a\\_b\n```` \ny = c\\_d\n')).toBe( + '````\nx = a\\_b\n```` \ny = c_d\n' + ) + }) + it('restores escaped callout markers in nested blockquotes', () => { expect(postProcessSerializedMarkdown('> > \\[!WARNING\\]\n> > hi')).toBe( '> > [!WARNING]\n> > hi' From 0ccad3fe7ae60d4aabd6f5f8adb865776196008a Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Thu, 20 Aug 2026 14:58:37 -0700 Subject: [PATCH 4/5] fix(markdown): skip quoted fences, and stop joining runs across inline tags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 `abc` 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 `
` 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) --- .../rich-markdown-editor/markdown-fidelity.ts | 22 ++++- .../rich-markdown-editor/round-trip.test.ts | 15 ++++ .../src/note/note-search-highlight.test.tsx | 82 ++++++++++++++++++- .../src/note/note-search-highlight.ts | 56 ++++--------- 4 files changed, 129 insertions(+), 46 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts index 2727454aad7..1cc78e3c536 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts @@ -60,7 +60,7 @@ function unescapeIntrawordUnderscores(markdown: string): string { continue } - const delimiter = lines[i].match(FENCE_DELIMITER)?.[1] + const delimiter = opensFence(lines[i]) if (delimiter) { fence = delimiter continue @@ -177,10 +177,26 @@ const FENCE_DELIMITER = /^[ \t]*(`{3,}|~{3,})/ * as prose. */ const CLOSING_FENCE = /^[ \t]*(`{3,}|~{3,})[ \t]*$/ +/** + * Leading blockquote markers, which every line of a fence inside a quote or a `[!NOTE]` callout + * carries. The serializer writes those (` > ```js `), so a walk that only recognises a bare fence + * never enters code state there and rewrites the block's interior as prose. + */ +const BLOCKQUOTE_PREFIX = /^[ \t]*(?:>[ \t]?)*/ + +/** The line with any blockquote markers removed, so a quoted fence reads like a bare one. */ +function unquote(line: string): string { + return line.replace(BLOCKQUOTE_PREFIX, '') +} + +/** The delimiter run that OPENS a fence on this line, quoted or not, or undefined. */ +function opensFence(line: string): string | undefined { + return unquote(line).match(FENCE_DELIMITER)?.[1] +} /** Whether `line` closes a fence opened with `fence`: same character, and at least as long. */ function closesFence(line: string, fence: string): boolean { - const delimiter = line.match(CLOSING_FENCE)?.[1] + const delimiter = unquote(line).match(CLOSING_FENCE)?.[1] return Boolean(delimiter && delimiter[0] === fence[0] && delimiter.length >= fence.length) } /** 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 { if (closesFence(line, fence)) fence = null continue } - const delimiter = line.match(FENCE_DELIMITER)?.[1] + const delimiter = opensFence(line) if (delimiter) { fence = delimiter kept.push(line) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip.test.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip.test.ts index d07527fa649..c50629f2851 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip.test.ts @@ -279,6 +279,21 @@ describe('editor markdown round-trip', () => { expect(roundTrip('call ``a\\_b`c`` here')).toContain('a\\_b') }) + /* Unlike the fence-length cases, this one is reachable: the serializer really does write a + quoted fence as ` > ```js `, so a walk that only recognises a bare fence never enters code + state and rewrites the block's interior as prose. */ + it('leaves code alone inside a blockquoted fence', () => { + expect(roundTrip('> ```js\n> x = a\\_b\n> ```')).toContain('a\\_b') + }) + + it('leaves code alone inside a callout fence', () => { + expect(roundTrip('> [!NOTE]\n> ```js\n> x = a\\_b\n> ```')).toContain('a\\_b') + }) + + it('still unescapes prose inside a blockquote', () => { + expect(roundTrip('> SB_ACTION_ROUTER_SECRET')).toContain('SB_ACTION_ROUTER_SECRET') + }) + it('preserves an image url (does not drop the src)', () => { const out = roundTrip('![alt](https://example.com/i.png)') expect(out).toContain('![alt](https://example.com/i.png)') diff --git a/packages/workflow-renderer/src/note/note-search-highlight.test.tsx b/packages/workflow-renderer/src/note/note-search-highlight.test.tsx index 8733905ab67..54dcac2eb34 100644 --- a/packages/workflow-renderer/src/note/note-search-highlight.test.tsx +++ b/packages/workflow-renderer/src/note/note-search-highlight.test.tsx @@ -274,7 +274,10 @@ describe('note search across inline boundaries', () => { expect(ordinals).toEqual(['0', '0']) }) - it('marks a phrase spanning a bold word', () => { + /* A match spanning `a**b**c` cannot exist in the source the indexer scans — the asterisks are + between the words there. Joining across the element would invent one, and because the ordinal + counts source occurrences, an invented hit appearing earlier steals the current mark. */ + it('does not join text across a bold word', () => { const tree: Root = { type: 'root', children: [ @@ -296,7 +299,7 @@ describe('note search across inline boundaries', () => { ], } noteSearchHighlightPlugin({ query: 'a bold word' })(tree) - expect(markedTextsOf(tree)).toEqual(['a ', 'bold', ' word']) + expect(markedTextsOf(tree)).toEqual([]) }) /* Two paragraphs are not one phrase on screen. Joining them would invent a hit @@ -308,6 +311,81 @@ describe('note search across inline boundaries', () => { expect(markedTextsOf(tree)).toEqual([]) }) + it('still marks a match wholly inside an inline element', () => { + const tree: Root = { + type: 'root', + children: [ + { + type: 'element', + tagName: 'p', + properties: {}, + children: [ + { + type: 'element', + tagName: 'strong', + properties: {}, + children: [{ type: 'text', value: 'SB_ACTION' }], + }, + ], + }, + ], + } + noteSearchHighlightPlugin({ query: 'SB_ACTION' })(tree) + expect(markedTextsOf(tree)).toEqual(['SB_ACTION']) + }) + + /* The ordinal counts SOURCE occurrences. A hit that only exists once formatting is stripped + would take ordinal 0 here while the real one — the one the panel is pointing at — became 1, + so the card would paint the current mark on text the search never matched. */ + it('does not let a formatted concatenation steal the current ordinal', () => { + const tree: Root = { + type: 'root', + children: [ + { + type: 'element', + tagName: 'p', + properties: {}, + children: [ + { type: 'text', value: 'a' }, + { + type: 'element', + tagName: 'strong', + properties: {}, + children: [{ type: 'text', value: 'b' }], + }, + { type: 'text', value: 'c' }, + ], + }, + { + type: 'element', + tagName: 'p', + properties: {}, + children: [{ type: 'text', value: 'abc' }], + }, + ], + } + noteSearchHighlightPlugin({ query: 'abc' })(tree) + + const marks: Array<[string, unknown]> = [] + const walk = (node: Root | Element) => { + const children: Array = node.children + for (const child of children) { + if (child.type !== 'element') continue + if (child.tagName === 'mark') { + const [first] = child.children + marks.push([ + first?.type === 'text' ? first.value : '', + child.properties.dataNoteSearchIndex, + ]) + continue + } + walk(child) + } + } + walk(tree) + expect(marks).toEqual([['abc', '0']]) + }) + it('folds a non-breaking space the way the indexer does', () => { const tree = paragraphTree('a b') noteSearchHighlightPlugin({ query: 'a b' })(tree) diff --git a/packages/workflow-renderer/src/note/note-search-highlight.ts b/packages/workflow-renderer/src/note/note-search-highlight.ts index 9b0a11f5382..2ef12d0f6a5 100644 --- a/packages/workflow-renderer/src/note/note-search-highlight.ts +++ b/packages/workflow-renderer/src/note/note-search-highlight.ts @@ -41,40 +41,6 @@ export interface NoteSearchRange { */ export const NOTE_SEARCH_MARK_INDEX_PROPERTY = 'dataNoteSearchIndex' -/** - * Elements that do not interrupt a run of text. Text either side of one reads - * as a single phrase, so the scan below joins across them — the same thing the - * browser's own find does in `abc`. - * - * An allowlist rather than a block-level denylist: raw HTML can put any tag in - * this tree, and treating something unrecognised as a break can only ever miss - * a match, while treating it as inline could invent one that is not on screen. - */ -const INLINE_TAG_NAMES: ReadonlySet = new Set([ - 'a', - 'abbr', - 'b', - 'br', - 'cite', - 'code', - 'del', - 'em', - 'i', - 'ins', - 'kbd', - 'mark', - 'q', - 's', - 'samp', - 'small', - 'span', - 'strong', - 'sub', - 'sup', - 'u', - 'var', -]) - /** * Visits every occurrence of `query` in `text`, case-insensitively and without * overlaps. @@ -187,13 +153,21 @@ function collectTextRuns(node: Root | Element, builder: RunBuilder): void { continue } - /* An inline element continues the run — the builder state is shared, so - descending is all it takes. Anything else breaks it either side. */ - if (INLINE_TAG_NAMES.has(child.tagName)) { - collectTextRuns(child, builder) - continue - } - + /* + * Every other element ends the run, `` and `` included. + * + * `
` is the one boundary that stands for a character the source really + * has. Every other inline element stands for syntax the render DROPS — + * `**`, `_`, a backtick, a link's `](url)` — so joining across one invents + * an adjacency that exists on screen but not in the markdown the indexer + * scans. That is not merely a spurious extra mark: `occurrenceIndex` counts + * source occurrences, so a fabricated hit appearing earlier in the document + * steals the current ordinal and paints the wrong one. + * + * Nothing real is lost. A match spanning `a**b**c` would have to contain + * the asterisks to exist in the source at all, and a match wholly inside + * the element is still found — the element simply starts its own run. + */ endRun(builder) collectTextRuns(child, builder) endRun(builder) From 371a3de09cbe64b058fa9f54f43b11544b83a4c1 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Thu, 20 Aug 2026 15:12:23 -0700 Subject: [PATCH 5/5] fix(search): match a Note body as it renders, instead of rewriting the file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the serializer change with one that writes nothing. The editor backslash-escapes every markdown-significant character in prose, so a Note the reader sees as `{{TE_SERET}}` is stored as `{{TE\_SERET}}` and search — which matches the stored value — could not find it. The previous approach undid that escape in `postProcessSerializedMarkdown`, which meant re-deriving markdown structure from the serialized string with regexes so it knew what was code. That is a losing game: three review rounds, each finding another construct it did not model (longer fences, then delimiter-prefixed lines, then quoted fences), and each miss REWROTE somebody's code. `markdown-fidelity.ts` is back to staging, byte for byte. The escape is now undone on the matching side only. A field declares `searchTextFormat: 'markdown'` (the Note body is the only one), and the indexer matches it against `projectEscapedMarkdownForSearch(value)` — a total, structure-free function that returns the rendered text plus an index back into the source. Ranges stay in source coordinates, so replace still rewrites the whole `\_` and never strands a backslash. The asymmetry is the whole point: a matcher that de-escapes something a fence would have kept literal changes only which text highlights, and no caller writes it back. A rewriter making the identical mistake corrupts the file. So there is nothing here that needs to know about fences at all. Two consequences worth having: existing notes are searchable immediately rather than after their next edit, and no stored byte changes, so no document the editor has ever written can be affected. Co-Authored-By: Claude Opus 5 (1M context) --- .../rich-markdown-editor/markdown-fidelity.ts | 100 +----------------- .../rich-markdown-editor/round-trip.test.ts | 90 ---------------- .../components/note-block/note-block.tsx | 23 +++- apps/sim/blocks/blocks/note.ts | 1 + apps/sim/blocks/types.ts | 13 +++ .../workflows/search-replace/indexer.test.ts | 73 +++++++++++++ .../lib/workflows/search-replace/indexer.ts | 36 ++++++- packages/utils/src/string.test.ts | 39 +++++++ packages/utils/src/string.ts | 58 ++++++++++ packages/workflow-renderer/src/index.ts | 1 + .../src/note/note-search-highlight.ts | 28 ++++- 11 files changed, 265 insertions(+), 197 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts index 1cc78e3c536..4470187fefa 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-fidelity.ts @@ -18,63 +18,6 @@ const CODE_OR_PLAIN_LINK_REGEX = /(```[\s\S]*?```|~~~[\s\S]*?~~~|`[^`\n]+`)|\[([^\]]+)]\(([^)\s<>]+)\)/g const HTTP_URL_REGEX = /^https?:\/\/\S+$/i -/** - * Alternates an inline code span with a single underscore that has a letter or digit on both sides. - * - * CommonMark's intraword rule means such an underscore can neither open nor close emphasis, so the - * serializer's backslash before it carries no meaning \u2014 it just writes `SB\_ACTION\_ROUTER\_SECRET` - * into the document. That is ugly in the file, and it silently breaks workflow search, which matches - * against the stored markdown rather than the rendered text: searching `SB_ACTION` finds nothing in - * a note whose stored form has a backslash the reader never sees. - * - * Code is excluded because the serializer emits it verbatim: a `\_` inside a span is the author's own - * backslash, not an escape this may drop. The span branch matches a backtick RUN and requires a run of - * the same length to close it, per CommonMark \u2014 a fixed single-backtick pattern would read ``` ``a`b`` ``` - * as `` `a` `` plus loose text and rewrite the interior. Fenced blocks are handled a line at a time by - * {@link unescapeIntrawordUnderscores}, which is the only way to honour a fence of any length. - * - * The flanking character is a capture group rather than a lookbehind: lookbehind only landed in - * Safari 16.4, and an unsupported one throws when the pattern is constructed \u2014 taking the whole editor - * module with it. The group is consumed and put back, and the lookahead is not, so runs like `A\_B\_C` - * still match on every pair. - */ -const CODE_SPAN_OR_INTRAWORD_ESCAPED_UNDERSCORE = - /(`+)((?:[^`]|(?!\1)`)*?)\1(?!`)|([\p{L}\p{N}])\\_(?=[\p{L}\p{N}])/gu - -/** - * Drops the meaningless backslash before an intraword underscore, outside code. - * - * Fenced blocks are skipped a line at a time, tracking the opening delimiter the same way - * {@link stripEmptyListItemLines} does: a fence is three OR MORE backticks or tildes and is closed - * only by a run of the same character at least as long, so a `````` ```` `````-fenced block wrapping - * ``` ``` ``` stays code throughout. Matching a fixed ``` pair instead would end the region early and - * hand the rest of the author's code to the rewrite. - */ -function unescapeIntrawordUnderscores(markdown: string): string { - const lines = markdown.split('\n') - let fence: string | null = null - - for (let i = 0; i < lines.length; i++) { - if (fence) { - if (closesFence(lines[i], fence)) fence = null - continue - } - - const delimiter = opensFence(lines[i]) - if (delimiter) { - fence = delimiter - continue - } - - lines[i] = lines[i].replace( - CODE_SPAN_OR_INTRAWORD_ESCAPED_UNDERSCORE, - (match, ticks, _span, flank) => (ticks === undefined ? `${flank}_` : match) - ) - } - - return lines.join('\n') -} - /** * Collapses an autolinked destination back to its bare form: our normalizing serializer rewrites a bare * URL or `` autolink to `[url](url)` and a bare email to `[a@b.com](mailto:a@b.com)`, which churns @@ -167,38 +110,6 @@ export function normalizeLinkHref(href: string): string { const EMPTY_LIST_ITEM_LINE = /^([ \t]*)(?:[-*+]|\d+[.)])[ \t]*$/ /** A fenced code-block delimiter (``` or ~~~), used to leave code interiors untouched. */ const FENCE_DELIMITER = /^[ \t]*(`{3,}|~{3,})/ -/** - * A line carrying nothing but a delimiter run — the only thing that CLOSES a fence. - * - * An OPENING fence may be followed by an info string (` ```python `), so opening is matched with - * {@link FENCE_DELIMITER}; a closing one may be followed only by whitespace. Treating any - * delimiter-prefixed line as a close ends the block at an interior line like ` ```example ` inside - * a `````` ```` ``````-fence, and every cleanup below then processes the rest of the author's code - * as prose. - */ -const CLOSING_FENCE = /^[ \t]*(`{3,}|~{3,})[ \t]*$/ -/** - * Leading blockquote markers, which every line of a fence inside a quote or a `[!NOTE]` callout - * carries. The serializer writes those (` > ```js `), so a walk that only recognises a bare fence - * never enters code state there and rewrites the block's interior as prose. - */ -const BLOCKQUOTE_PREFIX = /^[ \t]*(?:>[ \t]?)*/ - -/** The line with any blockquote markers removed, so a quoted fence reads like a bare one. */ -function unquote(line: string): string { - return line.replace(BLOCKQUOTE_PREFIX, '') -} - -/** The delimiter run that OPENS a fence on this line, quoted or not, or undefined. */ -function opensFence(line: string): string | undefined { - return unquote(line).match(FENCE_DELIMITER)?.[1] -} - -/** Whether `line` closes a fence opened with `fence`: same character, and at least as long. */ -function closesFence(line: string, fence: string): boolean { - const delimiter = unquote(line).match(CLOSING_FENCE)?.[1] - return Boolean(delimiter && delimiter[0] === fence[0] && delimiter.length >= fence.length) -} /** Leading indentation of a line, used to detect whether an empty list item has indented children. */ const LEADING_INDENT = /^[ \t]*/ @@ -224,12 +135,12 @@ function stripEmptyListItemLines(markdown: string): string { let fence: string | null = null for (let i = 0; i < lines.length; i++) { const line = lines[i] + const delimiter = line.match(FENCE_DELIMITER)?.[1] if (fence) { kept.push(line) - if (closesFence(line, fence)) fence = null + if (delimiter && delimiter[0] === fence[0] && delimiter.length >= fence.length) fence = null continue } - const delimiter = opensFence(line) if (delimiter) { fence = delimiter kept.push(line) @@ -260,8 +171,7 @@ function stripEmptyListItemLines(markdown: string): string { /** * Cleans up serializer output: drops empty list-item marker lines that would otherwise corrupt on * round-trip ({@link stripEmptyListItemLines}), restores callout markers the serializer - * backslash-escapes (`> \[!NOTE\]` → `> [!NOTE]`), drops the equally unnecessary escape on an - * intraword underscore ({@link unescapeIntrawordUnderscores}), and collapses trailing blank lines to a single + * backslash-escapes (`> \[!NOTE\]` → `> [!NOTE]`), and collapses trailing blank lines to a single * newline. Interior blank runs are NOT collapsed here — blank lines inside a fenced code block (or a * verbatim raw-markdown-snippet) are significant, and a global collapse would corrupt them. An interior * run between top-level blocks is significant too: it is how an empty paragraph is written, and @@ -274,8 +184,6 @@ function stripEmptyListItemLines(markdown: string): string { */ export function postProcessSerializedMarkdown(markdown: string): string { return collapseAutolinkedUrls( - unescapeIntrawordUnderscores( - stripEmptyListItemLines(markdown).replace(ESCAPED_CALLOUT_REGEX, '$1[!$2]') - ) + stripEmptyListItemLines(markdown).replace(ESCAPED_CALLOUT_REGEX, '$1[!$2]') ).replace(/\n+$/, '\n') } diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip.test.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip.test.ts index c50629f2851..c8745e5e861 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip.test.ts @@ -98,32 +98,6 @@ describe('markdown-fidelity utils', () => { expect(postProcessSerializedMarkdown('> \\[!NOTE\\]\n> hi')).toBe('> [!NOTE]\n> hi') }) - /* - * A closing fence carries nothing but its delimiter run; a line that merely STARTS with one is - * content. Matching the prefix alone ends the block at an interior line like ` ````example `, - * after which the rest of the author's code is cleaned up as prose and its backslashes vanish. - * - * Exercised directly rather than through `roundTrip` on purpose: the serializer always opens a - * block with one more delimiter than the longest run inside it, so its own output cannot reach - * this shape and a round-trip test of it would pass either way. Relying on that choice staying - * true is an unstated coupling — this keeps the guard honest on any input. - */ - it('does not close a fence on a delimiter-prefixed content line', () => { - const input = '````\nx = a\\_b\n````example\ny = c\\_d\n````\n' - expect(postProcessSerializedMarkdown(input)).toBe(input) - }) - - it('does not close a tilde fence on a delimiter-prefixed content line', () => { - const input = '~~~~\n~~~~note\ny = c\\_d\n~~~~\n' - expect(postProcessSerializedMarkdown(input)).toBe(input) - }) - - it('still closes a fence on a bare delimiter run with trailing spaces', () => { - expect(postProcessSerializedMarkdown('````\nx = a\\_b\n```` \ny = c\\_d\n')).toBe( - '````\nx = a\\_b\n```` \ny = c_d\n' - ) - }) - it('restores escaped callout markers in nested blockquotes', () => { expect(postProcessSerializedMarkdown('> > \\[!WARNING\\]\n> > hi')).toBe( '> > [!WARNING]\n> > hi' @@ -201,13 +175,6 @@ describe('editor markdown round-trip', () => { 'highlight nested in bold': '**bold ==mark== here**', 'highlight in list': '- ==a== item', 'highlight with interior equals': 'x ==a=b== y', - 'intraword underscores': 'SB_ACTION_ROUTER_SECRET', - 'env token with underscores': '{{TE_SERET}} and {{OPENAI_API_KEY}}', - 'underscore emphasis': 'an _italic_ word', - 'underscore bold': 'a __bold__ word', - 'mixed underscores': '_em_ then SNAKE_CASE_NAME then _em again_', - 'escaped underscore in code': '```py\nx = a\\_b\n```', - 'escaped underscore in inline code': 'call `a\\_b` here', } for (const [name, input] of Object.entries(cases)) { @@ -237,63 +204,6 @@ describe('editor markdown round-trip', () => { expect(roundTrip('> [!NOTE]\n> Heads up')).toContain('[!NOTE]') }) - /* - * The serializer escapes every underscore, but CommonMark's intraword rule means one flanked by - * letters or digits can neither open nor close emphasis. The escape is therefore invisible to a - * reader and load-bearing for nobody — while workflow search matches the STORED markdown, so a - * stray backslash made `SB_ACTION` unfindable in a note that plainly showed it. - */ - it('writes an intraword underscore without a backslash', () => { - expect(roundTrip('SB_ACTION_ROUTER_SECRET')).toBe('SB_ACTION_ROUTER_SECRET') - expect(roundTrip('{{TE_SERET}}')).toBe('{{TE_SERET}}') - }) - - /* Emphasis itself normalises to asterisks, which is pre-existing and fine. What must survive - is the distinction: a literal underscore pair keeps its escape, so re-parsing cannot turn - the user's text into emphasis. */ - it('still escapes an underscore that would open or close emphasis', () => { - expect(roundTrip('an _italic_ word')).toBe('an *italic* word') - expect(roundTrip('literal \\_not emphasis\\_ here')).toContain('\\_') - }) - - /* Code is emitted verbatim, so a backslash inside it is the author's own character and not an - escape to drop. Unescaping blind would silently rewrite people's code. */ - it('leaves a backslash-underscore inside code alone', () => { - expect(roundTrip('```py\nx = a\\_b\n```')).toContain('a\\_b') - expect(roundTrip('call `a\\_b` here')).toContain('a\\_b') - }) - - /* A fence is three OR MORE delimiters, closed only by a run at least as long, and an inline span - opens and closes on backtick runs of equal length. Recognising just the shortest form ends the - code region early and hands the rest of the author's code to the rewrite. */ - it('leaves code alone in a longer fence', () => { - expect(roundTrip('````\nx = a\\_b\n```\nstill code y = c\\_d\n````')).toContain('a\\_b') - expect(roundTrip('````\nx = a\\_b\n```\nstill code y = c\\_d\n````')).toContain('c\\_d') - }) - - it('leaves code alone in a tilde fence', () => { - expect(roundTrip('~~~~\nx = a\\_b\n~~~~')).toContain('a\\_b') - }) - - it('leaves code alone in a multi-backtick inline span', () => { - expect(roundTrip('call ``a\\_b`c`` here')).toContain('a\\_b') - }) - - /* Unlike the fence-length cases, this one is reachable: the serializer really does write a - quoted fence as ` > ```js `, so a walk that only recognises a bare fence never enters code - state and rewrites the block's interior as prose. */ - it('leaves code alone inside a blockquoted fence', () => { - expect(roundTrip('> ```js\n> x = a\\_b\n> ```')).toContain('a\\_b') - }) - - it('leaves code alone inside a callout fence', () => { - expect(roundTrip('> [!NOTE]\n> ```js\n> x = a\\_b\n> ```')).toContain('a\\_b') - }) - - it('still unescapes prose inside a blockquote', () => { - expect(roundTrip('> SB_ACTION_ROUTER_SECRET')).toContain('SB_ACTION_ROUTER_SECRET') - }) - it('preserves an image url (does not drop the src)', () => { const out = roundTrip('![alt](https://example.com/i.png)') expect(out).toContain('![alt](https://example.com/i.png)') diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/note-block/note-block.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/note-block/note-block.tsx index f5aa127f4e8..65976fb4cbd 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/note-block/note-block.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/note-block/note-block.tsx @@ -4,6 +4,7 @@ import { countNoteSearchOccurrencesBefore, DEFAULT_NOTE_COLOR, estimateNoteBlockHeight, + forEachNoteSourceOccurrence, getNoteStringValue, isNoteColor, NoteBlockView, @@ -171,9 +172,15 @@ export const NoteBlock = memo(function NoteBlock({ } } - return content.toLowerCase().includes(query.toLowerCase()) - ? { query, occurrenceIndex: 0 } - : null + /* Asked through the same scan that will do the marking, rather than a local + `includes`: a bare comparison skips the whitespace fold, so a query the + indexer matched across a newline or a non-breaking space would read as + absent here and the card would paint nothing while the panel counted it. */ + let occurs = false + forEachNoteSourceOccurrence(content, query, () => { + occurs = true + }) + return occurs ? { query, occurrenceIndex: 0 } : null }, [content, isSearchTargetBlock, searchTarget]) /** @@ -240,8 +247,16 @@ export const NoteBlock = memo(function NoteBlock({ const searchExpandedRef = useRef(false) const hasSearchMatch = searchHighlight !== null || nameSearchRange !== null useEffect(() => { + /* Losing edit rights already force-collapses the card during render. Drop the latch with it, + or regaining them would hit the early return below and leave a deep match clipped in the + compact body until the active match moved. */ + if (!canEditNote) { + searchExpandedRef.current = false + return + } + if (hasSearchMatch) { - if (searchExpandedRef.current || isExpandedRef.current || !canEditNote) return + if (searchExpandedRef.current || isExpandedRef.current) return searchExpandedRef.current = true setIsExpanded(true) return diff --git a/apps/sim/blocks/blocks/note.ts b/apps/sim/blocks/blocks/note.ts index 477cb888ce5..7c38358cea9 100644 --- a/apps/sim/blocks/blocks/note.ts +++ b/apps/sim/blocks/blocks/note.ts @@ -15,6 +15,7 @@ export const NoteBlock: BlockConfig = { { id: 'content', type: 'long-input', + searchTextFormat: 'markdown', rows: 8, placeholder: 'Add context or instructions for collaborators...', description: 'Write your note using Markdown. YouTube links will display as embedded videos.', diff --git a/apps/sim/blocks/types.ts b/apps/sim/blocks/types.ts index a322213781e..4f792272ffe 100644 --- a/apps/sim/blocks/types.ts +++ b/apps/sim/blocks/types.ts @@ -269,6 +269,19 @@ export interface SubBlockConfig { type: SubBlockType mode?: 'basic' | 'advanced' | 'both' | 'trigger' | 'trigger-advanced' // Default is 'both' if not specified. 'trigger' means only shown in trigger mode. 'trigger-advanced' is the advanced side of a trigger field — either a canonical pair member or a standalone field shown under the block-level advanced toggle canonicalParamId?: string + /** + * Declares that the stored value is markdown, so workflow search matches it + * against the text it RENDERS as rather than its source. + * + * The rich-text editor backslash-escapes every markdown-significant character + * in prose, so a Note body the reader sees as `SB_ACTION` is stored as + * `SB\_ACTION` and would otherwise be unfindable by what is on screen. Ranges + * stay in source coordinates, so replace still rewrites the escaped span. + * + * Omit for every ordinary field: a code or plain-text value is searched as + * stored, where a backslash is the author's own character. + */ + searchTextFormat?: 'markdown' /** Controls parameter visibility in agent/tool-input context */ paramVisibility?: 'user-or-llm' | 'user-only' | 'llm-only' | 'hidden' /** diff --git a/apps/sim/lib/workflows/search-replace/indexer.test.ts b/apps/sim/lib/workflows/search-replace/indexer.test.ts index 7a6c3fe3975..73628576916 100644 --- a/apps/sim/lib/workflows/search-replace/indexer.test.ts +++ b/apps/sim/lib/workflows/search-replace/indexer.test.ts @@ -167,6 +167,79 @@ describe('indexWorkflowSearchMatches', () => { expect(matches.some((match) => match.target.kind === 'block-name')).toBe(false) }) + describe('a markdown field is searched as it renders', () => { + /* + * The rich-text editor backslash-escapes every markdown-significant character in prose, so a + * Note the reader sees as `{{TE_SERET}}` is stored as `{{TE\_SERET}}`. Searching what is on + * screen has to see through that, and the range has to keep spanning the escaped source so + * replace rewrites the whole `\_` instead of stranding the backslash. + */ + const NOTE_CONFIGS = { + note: { subBlocks: [{ id: 'content', type: 'long-input', searchTextFormat: 'markdown' }] }, + function: { subBlocks: [{ id: 'code', type: 'code' }] }, + } as unknown as typeof SEARCH_REPLACE_BLOCK_CONFIGS + + function workflowWith(noteContent: string, code: string) { + return { + blocks: { + 'note-1': { + id: 'note-1', + type: 'note', + name: 'Note', + position: { x: 0, y: 0 }, + enabled: true, + horizontalHandles: true, + subBlocks: { content: { id: 'content', type: 'long-input', value: noteContent } }, + outputs: {}, + }, + 'fn-1': { + id: 'fn-1', + type: 'function', + name: 'Fn', + position: { x: 0, y: 0 }, + enabled: true, + horizontalHandles: true, + subBlocks: { code: { id: 'code', type: 'code', value: code } }, + outputs: {}, + }, + }, + } as unknown as Parameters[0]['workflow'] + } + + it('finds an escaped underscore by what the reader sees', () => { + const matches = indexWorkflowSearchMatches({ + workflow: workflowWith('{{TE\\_SERET}}', ''), + query: '{{TE_', + mode: 'text', + blockConfigs: NOTE_CONFIGS, + }) + expect(matches.map((match) => match.blockId)).toContain('note-1') + }) + + it('keeps the range over the escape, so replace cannot strand a backslash', () => { + const content = 'uses SB\\_ACTION here' + const [match] = indexWorkflowSearchMatches({ + workflow: workflowWith(content, ''), + query: 'SB_ACTION', + mode: 'text', + blockConfigs: NOTE_CONFIGS, + }) + expect(content.slice(match.range!.start, match.range!.end)).toBe('SB\\_ACTION') + expect(match.rawValue).toBe('SB\\_ACTION') + }) + + /* A code field stores what the author typed: a backslash there is theirs, not an escape. */ + it('leaves a field that is not markdown searched as stored', () => { + const matches = indexWorkflowSearchMatches({ + workflow: workflowWith('', 'const s = "a\\_b"'), + query: 'a_b', + mode: 'text', + blockConfigs: NOTE_CONFIGS, + }) + expect(matches.map((match) => match.blockId)).not.toContain('fn-1') + }) + }) + describe('block references search under the name the canvas shows', () => { /** * The panel's own pipeline: index everything, then keep what the query diff --git a/apps/sim/lib/workflows/search-replace/indexer.ts b/apps/sim/lib/workflows/search-replace/indexer.ts index a2d022753f8..012e9d48b83 100644 --- a/apps/sim/lib/workflows/search-replace/indexer.ts +++ b/apps/sim/lib/workflows/search-replace/indexer.ts @@ -1,5 +1,5 @@ import { isRecordLike } from '@sim/utils/object' -import { foldSearchWhitespace } from '@sim/utils/string' +import { foldSearchWhitespace, projectEscapedMarkdownForSearch } from '@sim/utils/string' import { DEFAULT_SUBBLOCK_TYPE } from '@sim/workflow-persistence/subblocks' import type { SubBlockType } from '@sim/workflow-types/blocks' import { isWorkflowBlockProtected } from '@sim/workflow-types/workflow' @@ -67,15 +67,37 @@ function normalizeForSearch(value: string, caseSensitive: boolean): string { return caseSensitive ? folded : folded.toLowerCase() } -function findTextRanges(value: string, query: string, caseSensitive: boolean) { +/** + * Ranges of `query` in `value`, always in `value`'s own coordinates. + * + * A field declaring `searchTextFormat: 'markdown'` is matched against the text + * it RENDERS as: the rich-text editor backslash-escapes every + * markdown-significant character in prose, so a Note body reading `SB_ACTION` + * on screen is stored as `SB\_ACTION`. The escape is undone only to match — the + * returned range still spans the escaped source, so replace rewrites the whole + * `\_` and never strands a backslash. + */ +function findTextRanges( + value: string, + query: string, + caseSensitive: boolean, + searchTextFormat?: SubBlockConfig['searchTextFormat'] +) { if (!query) return [] - const source = normalizeForSearch(value, caseSensitive) + + const projection = searchTextFormat === 'markdown' ? projectEscapedMarkdownForSearch(value) : null + const source = normalizeForSearch(projection ? projection.text : value, caseSensitive) const target = normalizeForSearch(query, caseSensitive) const ranges: Array<{ start: number; end: number }> = [] let index = source.indexOf(target) while (index !== -1) { - ranges.push({ start: index, end: index + target.length }) + const end = index + target.length + ranges.push( + projection + ? { start: projection.starts[index], end: projection.starts[end] } + : { start: index, end } + ) index = source.indexOf(target, index + Math.max(target.length, 1)) } @@ -578,6 +600,8 @@ interface AddTextMatchesOptions { protectedByLock: boolean isSnapshotView: boolean readonlyReason?: string + /** Declared by the field's config; see {@link findTextRanges}. */ + searchTextFormat?: SubBlockConfig['searchTextFormat'] } function getReadonlyReason({ @@ -610,8 +634,9 @@ function addTextMatches({ protectedByLock, isSnapshotView, readonlyReason, + searchTextFormat, }: AddTextMatchesOptions) { - const ranges = query ? findTextRanges(value, query, caseSensitive) : [] + const ranges = query ? findTextRanges(value, query, caseSensitive, searchTextFormat) : [] ranges.forEach((range, occurrenceIndex) => { matches.push({ id: createMatchId([ @@ -1474,6 +1499,7 @@ export function indexWorkflowSearchMatches( target: { kind: 'subblock' }, query, caseSensitive, + searchTextFormat: subBlockConfig?.searchTextFormat, editable: leafEditable, protectedByLock, isSnapshotView, diff --git a/packages/utils/src/string.test.ts b/packages/utils/src/string.test.ts index e96bfb932dd..387f222b177 100644 --- a/packages/utils/src/string.test.ts +++ b/packages/utils/src/string.test.ts @@ -6,6 +6,7 @@ import { formatQuotedNameList, isVersionedType, normalizeEmail, + projectEscapedMarkdownForSearch, sanitizeForJsonb, sanitizeValueForJsonb, stripVersionSuffix, @@ -136,3 +137,41 @@ describe('formatQuotedNameList', () => { expect(formatQuotedNameList([], 3)).toBe('') }) }) + +describe('projectEscapedMarkdownForSearch', () => { + it('leaves text with no escapes alone', () => { + const { text, starts } = projectEscapedMarkdownForSearch('plain text') + expect(text).toBe('plain text') + expect(starts[0]).toBe(0) + expect(starts[text.length]).toBe(10) + }) + + it('drops the backslash a markdown escape carries', () => { + expect(projectEscapedMarkdownForSearch('SB\\_ACTION\\_ROUTER').text).toBe('SB_ACTION_ROUTER') + expect(projectEscapedMarkdownForSearch('{{TE\\_SERET}}').text).toBe('{{TE_SERET}}') + }) + + it('keeps a backslash that escapes nothing markdown cares about', () => { + expect(projectEscapedMarkdownForSearch('a\\nb').text).toBe('a\\nb') + expect(projectEscapedMarkdownForSearch('trailing\\').text).toBe('trailing\\') + }) + + /* The span must cover the backslash, or replacing a match would leave it stranded. */ + it('maps a projected range back over the escape it consumed', () => { + const source = 'x SB\\_ACTION y' + const { text, starts } = projectEscapedMarkdownForSearch(source) + const start = text.indexOf('SB_ACTION') + const end = start + 'SB_ACTION'.length + expect(source.slice(starts[start], starts[end])).toBe('SB\\_ACTION') + }) + + it('maps every position when escapes repeat', () => { + const source = 'a\\_b\\_c' + const { text, starts } = projectEscapedMarkdownForSearch(source) + expect(text).toBe('a_b_c') + for (let i = 0; i < text.length; i += 1) { + expect(source.slice(starts[i], starts[i + 1]).endsWith(text[i])).toBe(true) + } + expect(starts[text.length]).toBe(source.length) + }) +}) diff --git a/packages/utils/src/string.ts b/packages/utils/src/string.ts index 7f6c542cf2c..be20494b068 100644 --- a/packages/utils/src/string.ts +++ b/packages/utils/src/string.ts @@ -181,3 +181,61 @@ export function formatQuotedNameList(names: string[], maxListed: number): string export function foldSearchWhitespace(value: string): string { return value.replace(/\s/g, ' ') } + +/** + * ASCII punctuation a backslash may escape in markdown, per CommonMark. A + * backslash before anything else is a literal backslash. + */ +const MARKDOWN_ESCAPABLE = new Set('!"#$%&\'()*+,-./:;<=>?@[\\]^_`{|}~') + +/** + * `text` with an index back into the string it came from. + * + * `starts` has one more entry than `text` has characters: `starts[i]` is where + * projected character `i` begins in the source, and `starts[text.length]` is + * the source length. A projected range `[s, e)` therefore maps to the source + * range `[starts[s], starts[e])` — including any backslash the projection + * consumed, so a caller rewriting that span never leaves one stranded. + */ +export interface SearchTextProjection { + text: string + starts: number[] +} + +/** + * Projects markdown onto the text it renders as, for MATCHING only. + * + * The rich-text editor's serializer backslash-escapes every markdown-significant + * character in prose, so a note the reader sees as `SB_ACTION` is stored as + * `SB\_ACTION`. Searching what is on screen has to see through that. + * + * Deliberately a total, structure-free function: it does not try to know which + * spans are code. Undoing an escape that a code fence would have kept literal + * only ever changes which text a search highlights — no caller writes this + * back — whereas a rewriter making the same mistake would corrupt the file. + * That asymmetry is why the escape is undone here rather than at serialization. + */ +export function projectEscapedMarkdownForSearch(value: string): SearchTextProjection { + if (!value.includes('\\')) { + return { text: value, starts: identityStarts(value.length) } + } + + let text = '' + const starts: number[] = [] + + for (let index = 0; index < value.length; index += 1) { + const isEscape = value[index] === '\\' && MARKDOWN_ESCAPABLE.has(value[index + 1] ?? '') + starts.push(index) + if (isEscape) index += 1 + text += value[index] + } + starts.push(value.length) + + return { text, starts } +} + +function identityStarts(length: number): number[] { + const starts: number[] = new Array(length + 1) + for (let index = 0; index <= length; index += 1) starts[index] = index + return starts +} diff --git a/packages/workflow-renderer/src/index.ts b/packages/workflow-renderer/src/index.ts index 63961c81fdf..4f39addccc7 100644 --- a/packages/workflow-renderer/src/index.ts +++ b/packages/workflow-renderer/src/index.ts @@ -27,6 +27,7 @@ export { export { getNoteStringValue, isNoteContentEmpty } from './note/note-content' export { countNoteSearchOccurrencesBefore, + forEachNoteSourceOccurrence, type NoteSearchHighlight, type NoteSearchRange, } from './note/note-search-highlight' diff --git a/packages/workflow-renderer/src/note/note-search-highlight.ts b/packages/workflow-renderer/src/note/note-search-highlight.ts index 2ef12d0f6a5..570806c8050 100644 --- a/packages/workflow-renderer/src/note/note-search-highlight.ts +++ b/packages/workflow-renderer/src/note/note-search-highlight.ts @@ -1,4 +1,4 @@ -import { foldSearchWhitespace } from '@sim/utils/string' +import { foldSearchWhitespace, projectEscapedMarkdownForSearch } from '@sim/utils/string' import type { Element, Root, Text } from 'hast' /** @@ -81,13 +81,37 @@ export function forEachNoteSearchOccurrence( * How many occurrences of `query` start before `offset` in `content` — the * ordinal of the occurrence that starts there. */ +/** + * Visits every occurrence of `query` in a note's markdown SOURCE, reporting each + * start in source coordinates. + * + * Matches against the text the markdown renders as, exactly as the search index + * does for a field declaring `searchTextFormat: 'markdown'` — the card has to + * agree with the panel about what counts as an occurrence, or the ordinal it is + * handed points at a different hit than the one the user is on. + */ +export function forEachNoteSourceOccurrence( + content: string, + query: string, + visit: (sourceStart: number) => void +): void { + const projection = projectEscapedMarkdownForSearch(content) + forEachNoteSearchOccurrence(projection.text, query, (start) => { + visit(projection.starts[start]) + }) +} + +/** + * How many occurrences of `query` start before `offset` in `content` — the + * ordinal of the occurrence that starts there. Both are source coordinates. + */ export function countNoteSearchOccurrencesBefore( content: string, query: string, offset: number ): number { let count = 0 - forEachNoteSearchOccurrence(content, query, (start) => { + forEachNoteSourceOccurrence(content, query, (start) => { if (start < offset) count += 1 }) return count