fix(search): answer a Note match on the canvas card - #6901
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview
Markdown fields ( Reviewed by Cursor Bugbot for commit 371a3de. Configure here. |
Greptile SummaryThe PR makes workflow-search matches inside Note blocks visible and navigable on the canvas while searching markdown as rendered text.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/note-block/note-block.tsx | Subscribes to primitive search-target fields and coordinates Note expansion, title/body highlighting, and active-match presentation. |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx | Selects and focuses Note cards as workflow search moves between active matches. |
| apps/sim/lib/workflows/search-replace/indexer.ts | Adds markdown-aware search projection while preserving source-coordinate ranges. |
| apps/sim/stores/panel/editor/store.ts | Separates search-target state from block-editor clearing so Note cards can continue rendering active matches. |
| packages/workflow-renderer/src/note/note-search-highlight.ts | Implements source occurrence counting and rendered Note-mark decoration used by canvas highlighting. |
| packages/workflow-renderer/src/note/note-block-view.tsx | Integrates Note title/body marks and internal scrolling into the shared Note renderer. |
Reviews (4): Last reviewed commit: "fix(search): match a Note body as it ren..." | Re-trigger Greptile
|
@cursor review |
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. - `<Streamdown>` 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) <noreply@anthropic.com>
Two review findings, both real. The intraword-underscore cleanup guarded code with a pattern that recognised only the shortest delimiter forms — a bare ``` pair and a single-backtick span. A ````-fenced block, a tilde fence, or a ``multi-backtick`` span ended the region early and handed the rest of the author's code to the rewrite, turning `a\_b` into `a_b` inside their code sample. Fenced blocks are now walked a line at a time, tracking the opening delimiter exactly the way stripEmptyListItemLines already does (three or more, closed only by a run at least as long), and the inline branch matches a backtick RUN closed by one of equal length. The note scanner claimed to be the same scan as the indexer's `findTextRanges` but did not fold whitespace, which staging added since this branch was written. The indexer folds every `\s` to a space, so a phrase matches across a soft line break — which `remark-breaks` renders as a `<br>`, splitting the phrase over two text nodes that a per-node scan could never see. The hit counted in the panel and highlighted nowhere, the exact bug this branch exists to fix. The plugin now scans runs of continuously-readable text rather than single nodes, so a match spanning an inline boundary (a soft break, a bold word) is wrapped as several marks sharing one ordinal. Runs end at any non-inline element, so two paragraphs are never joined into a phrase the reader cannot see. `foldSearchWhitespace` moved to `@sim/utils/string`: the canvas card renders from a package, which cannot import from `apps/*`, and two copies of that rule silently disagreeing is precisely what produced the second finding. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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) <noreply@anthropic.com>
3027fee to
93ceb84
Compare
|
@cursor review |
…e tags Two review findings, both real, both the same shape: a rule that looked at the rendered form and forgot what the source actually says. QUOTED FENCES. The fence walk only recognised a bare delimiter run, but the serializer writes a fence inside a blockquote or a `[!NOTE]` callout with a `>` on every line. Code state was therefore never entered there and the block's interior was cleaned up as prose: `> x = a\_b` round-tripped to `> x = a_b`, losing the author's backslash. Unlike the fence-length cases this one is reachable today — verified against the real serializer before and after. Both fence walks now unquote the line first. INLINE JOINS. Runs concatenated the visible text of every inline tag, so `a<strong>b</strong>c` read as `abc` — a hit that cannot exist in the markdown the indexer scans, where `**` sits between the words. That is worse than a spurious mark: `occurrenceIndex` counts SOURCE occurrences, so a fabricated hit earlier in the document steals the current ordinal and paints the mark on text the search never matched. Only `<br>` continues a run now, because it alone stands for a character the source really has (a `\n`, folded to a space). Everything else stands for syntax the render drops. Nothing real is lost: a match spanning `a**b**c` would have to contain the asterisks to exist at all, and a match wholly inside an element is still found — the element simply starts its own run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e file
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) <noreply@anthropic.com>
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 371a3de. Configure here.
Follow-up to #6901, closing two places where the workflow search index and the Note card that mirrors it could drift apart. Neither is a live bug; both are the shape that produced one — the card silently disagreeing with the panel about which hit is which, counted in one place and painted in another. THE SCAN. #6901 shared `foldSearchWhitespace` but left the scan around it duplicated: normalize, then non-overlapping `indexOf` stepping by `max(len, 1)`, written out once in the indexer and once in the renderer package. They agree today. They would stop agreeing the moment either grew whole-word matching, diacritic folding, or a regex mode, and the failure is silent. Both now call one `forEachSearchOccurrence` in `@sim/utils/string` — the only place either package can share, since the card renders from a package that cannot import from `apps/*`. THE DECLARATION. The indexer projects markdown escapes only for a field declaring `searchTextFormat: 'markdown'`; the card projects unconditionally, because it cannot read the block registry. Dropping that one line from the Note config would leave them disagreeing with nothing to catch it, so a test now pins it and explains why. Net negative in lines: this deletes a duplicated loop rather than adding a layer. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The bug
Searching a workflow counted matches inside Note blocks and then highlighted nowhere. In the reported case a 15,628-character note held 2 of 6 hits for
SB_ACTION_ROUTER_SECRET; the panel read "1 of 6" and pressing Enter through the first two moved nothing on screen.The panel editor deliberately renders nothing for a Note (
editor.tsx:if (isNoteBlock) return null) — the card is its own editing surface. But it refuses one by callingclearCurrentBlock(), which also cleared the sharedactiveSearchTarget, destroying the target the card was about to paint. Note matches were alsoeditable: true, so Replace All would have silently rewritten prose with nothing on screen to show it.What changed
The card's read view now answers the search.
<Streamdown>is keyed on the query. Its memo comparator checkschildren,mode,className,dirand friends but notrehypePlugins/remarkPlugins/components— so a plugin change alone cannot re-render it. Marks appeared only when something else happened to remount the card, and then survived the query being cleared.scrollToparithmetic, neverscrollIntoView— the card sits inside ReactFlow's transformed viewport, andscrollIntoViewwalks past the region to scroll the pane itself.OverflowSpangained an optional decorated-children slot; the tooltip stays plainvalue).activeSearchTargetsubscribers now take primitives. The search panel re-publishes an equal target on most of its own renders — its hydration hooks hand back fresh arrays. Holding the object inWorkflowContent, which is the panel's own ancestor, closed an unbounded update loop (Maximum update depth exceededthe instant Cmd-F ran). Documented on the store field.The markdown serializer stopped over-escaping underscores. It wrote
SB\_ACTION\_ROUTER\_SECRETinto the document. CommonMark's intraword rule means that backslash can never open or close emphasis, and workflow search matches the stored markdown — so anything with an underscore was unfindable in a note that plainly displayed it. Dropped outside code regions, where the serializer emits verbatim and a\_is the author's own character.Notes for review
<mark>in a note now picks up the card's tint instead of browser-default yellow. Every other element in the note view is already restyled;markwas the last one falling through.@types/hastadded topackages/workflow-renderer— types-only, already present transitively, made explicit rather than relied on as a phantom dep.src) shifts the current mark to a neighbouring occurrence rather than nowhere. Recorded in the module's TSDoc.Testing
packages/workflow-renderer: 97 pass (20 new). Includes rerender cases against an already-mounted card — the only shape that catches the Streamdown memo bug; verified by deleting thekeyand watching exactly those two fail.rich-markdown-editor: 571 pass (10 new), covering intraword unescape, emphasis still escaped, and\_preserved inside fenced and inline code. The code-region guard was likewise verified by removing it and confirming the test fails.stores/panel+lib/workflows/search-replace: 699 pass across 44 files (3 new).check:api-validation, monorepo boundaries, and native-typecheck pass; biome clean.staging(which touchedsearch-replace/indexer.tsandresources/*) and fully re-verified after.Not covered by tests: the scroll and camera behaviour — jsdom reports every offset as
0, so that path is exercised by hand only.🤖 Generated with Claude Code