Skip to content

refactor(search): one definition of what a search occurrence is - #6905

Merged
icecrasher321 merged 1 commit into
stagingfrom
fix/search-occurrence-parity
Aug 20, 2026
Merged

refactor(search): one definition of what a search occurrence is#6905
icecrasher321 merged 1 commit into
stagingfrom
fix/search-occurrence-parity

Conversation

@icecrasher321

Copy link
Copy Markdown
Collaborator

Follow-up to #6901. 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 during that PR's review, where the card silently disagreed with the panel about which hit was which.

1. The scan was duplicated

#6901 shared foldSearchWhitespace but left the loop around it written out twice — normalize, then non-overlapping indexOf stepping by max(len, 1) — once in findTextRanges and once in the renderer package.

They agree today. They stop agreeing the moment either grows whole-word matching, diacritic folding, or a regex mode, and the failure mode is silent: the panel counts a hit the card marks somewhere else. That is exactly the bug Bugbot caught in #6901, in its whitespace-folding form; sharing only the fold fixed the instance and left the mechanism.

Both now call one forEachSearchOccurrence in @sim/utils/string. That is the only place they can share it: the card renders from @sim/workflow-renderer, and packages cannot import from apps/*.

2. The markdown declaration was unpinned

The indexer projects markdown escapes only when a field declares searchTextFormat: 'markdown'. The card projects unconditionally — it has no access to the block registry. Removing that one line from the Note config would leave the two disagreeing about what an occurrence is, with nothing to catch it.

A test now pins the declaration and explains the coupling. Verified it fails when the line is removed.

Notes for review

  • Net −2 lines. This deletes a duplicated loop; it does not add a layer.
  • forEachNoteSearchOccurrence is gone from the renderer package — it was a wrapper around the duplicate. Callers use the shared primitive directly.
  • Case sensitivity stays where it was: the shared scanner takes the flag, the indexer passes it, the card does not (the search panel is the only caller and never enables it). Now that the algorithm is in one place, anyone plumbing it through will see both callers at once rather than having to know a second copy exists.

Testing

@sim/utils 160, @sim/workflow-renderer 104, apps/sim search-replace + panel 132 — all pass. Both type-checks, check:api-validation, monorepo boundaries and biome clean.

The new parity test was checked for vacuousness: deleting searchTextFormat from blocks/blocks/note.ts makes it fail.

🤖 Generated with Claude Code

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>
@vercel

vercel Bot commented Aug 20, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview Aug 20, 2026 10:38pm

Request Review

@cursor

cursor Bot commented Aug 20, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Internal search-matching refactor with no auth or data-path changes. Behavior is intended to stay the same; the main risk is a mismatch in highlight ordinals if the shared scanner is used incorrectly.

Overview
Unifies what counts as a search hit so the workflow search panel and the Note canvas card cannot silently drift.

The non-overlapping indexOf loop (plus whitespace fold and case folding) now lives once as forEachSearchOccurrence in @sim/utils/string. The indexer and the Note highlighter both call it; forEachNoteSearchOccurrence is removed. Case sensitivity is optional on the shared helper — the indexer still passes it through; the card still does not.

A test pins searchTextFormat: 'markdown' on the Note content field, because the card always projects markdown escapes while the indexer only does so when that flag is set.

Reviewed by Cursor Bugbot for commit 44fe5f4. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR centralizes workflow-search occurrence scanning in @sim/utils/string so the search indexer and Note renderer share one algorithm.

  • Replaces duplicated normalization and non-overlapping scan loops with forEachSearchOccurrence.
  • Preserves case-sensitivity handling, whitespace folding, markdown projection, and source-range mapping.
  • Adds a test pinning the Note content field’s markdown search declaration.

Confidence Score: 5/5

The PR appears safe to merge with no actionable correctness, security, or quality issues identified.

The extracted scanner is behaviorally equivalent for all current callers, remains reachable through existing package exports and dependencies, and preserves the indexer-to-renderer occurrence and coordinate contracts.

Important Files Changed

Filename Overview
packages/utils/src/string.ts Adds the shared occurrence scanner while preserving the duplicated algorithms’ whitespace folding, case handling, and non-overlapping iteration.
apps/sim/lib/workflows/search-replace/indexer.ts Delegates range scanning to the shared utility without changing markdown projection or projected-to-source coordinate mapping.
packages/workflow-renderer/src/note/note-search-highlight.ts Removes the renderer-specific scanner and uses the shared primitive for source occurrence counting and rendered highlights.
apps/sim/lib/workflows/search-replace/indexer.test.ts Pins the Note content configuration to markdown search projection using the same NoteBlock object registered in production.
packages/workflow-renderer/src/note/note-search-highlight.test.tsx Updates scanner tests to exercise the shared utility directly while retaining case-insensitive and non-overlapping behavior coverage.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Query[Search query] --> Scanner[forEachSearchOccurrence]
  Indexer[Workflow indexer] --> Projection[Optional markdown projection]
  Projection --> Scanner
  Note[Note renderer] --> NoteProjection[Markdown projection]
  NoteProjection --> Scanner
  Scanner --> Ranges[Index ranges]
  Scanner --> Highlights[Note highlights]
Loading

Reviews (1): Last reviewed commit: "refactor(search): one definition of what..." | Re-trigger Greptile

@icecrasher321
icecrasher321 merged commit 8529069 into staging Aug 20, 2026
30 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/search-occurrence-parity branch August 21, 2026 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant