perf(search): make overlap dedupe linear in match count - #6640
perf(search): make overlap dedupe linear in match count#6640mzxchandra wants to merge 3 commits into
Conversation
Typing in workflow Cmd+F froze the editor for over a second on a large workflow, with the typed characters landing late in one burst. `dedupeOverlappingWorkflowSearchMatches` ran `deduped.findIndex(...)` over the whole accumulated list for every match, recomputing each candidate's scope key inside the predicate - O(n^2) string builds. The memo re-runs on every keystroke (the query is not debounced), and a single character is the worst case because it matches the most. Reproduced on an 81-block workflow (4530 subblocks, real knowledge-base and OAuth references). Stage timing during one typing burst: searchBlocks merge 13ms index 35ms hydration 10ms filter + dedupe 1458ms <- resource options 6ms Overlap is only ever resolved within one value of one subblock, so bucket candidate indices by scope key and scan the bucket. A bucket holds exactly the entries the old predicate could match (scope key and range both present), buckets keep insertion order, and the scan stops at the first overlap, so the same candidate wins. A per-bucket `maxEnd` skips the scan entirely when a match starts at or after every kept range's end, which keeps a single long field full of disjoint hits linear too. Measured on that workflow, dedupe alone, by query: query matches before after email 558 6.4ms 0.56ms r 1698 53.3ms 0.71ms e 3373 232.9ms 1.10ms End to end in the browser the longest task while typing went from 1534ms to 247ms, with the same 521 matches either way. Two traps `maxEnd` sets, both found by adversarial review and both now pinned by tests: - `shouldPreferOverlappingMatch` prefers the SHORTER range, and a shorter range can end further right than the one it evicts. `maxEnd` has to be refreshed on the replacement path, not only on append, or the short-circuit skips real overlaps and leaks duplicates into replace-all. - Widening with a non-finite end would pin `maxEnd` at NaN, and since every comparison against NaN is false that silently switches dedupe off for the rest of the scope. Only finite ends widen it, which matches how the unbucketed scan treated such a range. `resolvers.test.ts` gains a reference implementation - a transcription of the original linear scan - checked against the bucketed one over 400 sequential seeds whose generator also emits inverted, empty and non-finite ranges, plus the two concrete replacement shapes above and a 20k-element single-scope case that pins the asymptotics. An earlier revision of this test pinned 8 hand-picked seeds and passed while 7.7% of the seed space diverged, so the sweep width is the point.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview Instead of Exports Reviewed by Cursor Bugbot for commit cdeeb43. Bugbot is set up for automated code reviews on this repo. Configure here. |
Greptile SummaryThe PR replaces repeated global overlap scans with scope-local buckets while preserving first-overlap and match-priority behavior.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/workflows/search-replace/resources/resolvers.ts | Replaces the global accumulated-list scan with insertion-ordered per-scope buckets and a high-water-mark shortcut. |
| apps/sim/lib/workflows/search-replace/resources/resolvers.test.ts | Adds a reference implementation, broad deterministic equivalence coverage, targeted replacement cases, and a complexity regression check. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Next search match] --> B{Has subblock scope and range?}
B -- No --> G[Append match]
B -- Yes --> C[Look up scope bucket]
C --> D{Start below bucket maxEnd?}
D -- No --> G
D -- Yes --> E[Scan bucket indices for first overlap]
E --> F{Overlap found?}
F -- No --> G
F -- Yes --> H{New match preferred?}
H -- Yes --> I[Replace existing match and widen maxEnd]
H -- No --> J[Keep existing match]
G --> K[Add index to bucket and widen maxEnd]
Reviews (2): Last reviewed commit: "Merge remote-tracking branch 'origin/sta..." | Re-trigger Greptile
There was a problem hiding this comment.
Pull request overview
Improves workflow editor Cmd+F responsiveness by optimizing overlap deduplication for workflow search matches, reducing per-keystroke main-thread work on large workflows.
Changes:
- Exported
OVERLAPPING_MATCH_KIND_PRIORITYso tests can share the canonical tie-break priority ordering. - Reworked
dedupeOverlappingWorkflowSearchMatchesto bucket candidates by overlap scope and avoid globalfindIndexrescans, using a per-bucketmaxEndshort-circuit. - Added equivalence + regression tests, including a seeded sweep against a reference implementation and an asymptotic guard.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/sim/lib/workflows/search-replace/resources/resolvers.ts | Buckets overlap dedupe by scope and adds maxEnd to reduce scanning; exports kind-priority map for shared test usage. |
| apps/sim/lib/workflows/search-replace/resources/resolvers.test.ts | Adds a reference dedupe implementation and seeded/property-style tests to assert equivalence and guard performance regressions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Review pointed out the comment claimed `maxEnd` is "the largest range.end currently kept in the bucket", which stops being true the moment a replacement swaps in a range that ends earlier - it is only ever widened. Only the upper bound is load-bearing, so say that. A bound left too high costs a scan that would have been skipped, never a wrong answer, and the staleness is capped at one token length because every range spans a matched token rather than the field. Also records why the exact maximum is deliberately not recomputed: on the realistic overlap shape at 10k matches, recomputing measures 45ms against 23ms as written, and 1010ms for the scan this replaced.
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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 cdeeb43. Configure here.
Summary
Typing in the workflow editor's Cmd+F froze the editor for over a second on a large workflow, with the typed characters landing late in one burst.
dedupeOverlappingWorkflowSearchMatchesrandeduped.findIndex(...)over the whole accumulated list for every match, recomputing each candidate's scope key inside the predicate - O(n^2) string builds. The memo re-runs on every keystroke (the query is not debounced), and a single character is the worst case because it matches the most.Reproduced on an 81-block workflow (4530 subblocks, real knowledge-base and OAuth references). Stage timing across one typing burst:
Overlap is only ever resolved within one value of one subblock, so candidate indices are bucketed by that scope and only the bucket is scanned. A per-bucket
maxEndskips the scan entirely when a match starts at or after every kept range's end, which keeps a single long field full of disjoint hits linear too.Dedupe alone, on that workflow:
emailreEnd to end the longest main-thread task while typing went from 1534ms to 247ms, with the same 521 matches either way.
Type of Change
Testing
Behaviour is unchanged - this is a pure restructuring of one function, and the tests are built to prove that rather than assert it.
resolvers.test.tsgains a reference implementation: a transcription of the original linear scan, checked against the bucketed one over 400 sequential seeds. The generator emits inverted, empty and non-finite ranges as well as normal ones, plus non-subblock targets and rangeless matches.Each test was verified to fail against the corresponding defect, not merely to pass against the current code:
findIndexrescanmaxEndrefresh on replacementmaxEndTwo traps the
maxEndshort-circuit sets, both surfaced by adversarial review and both now pinned:shouldPreferOverlappingMatchprefers the shorter range, and a shorter range can end further right than the one it evicts.maxEndhas to be refreshed on the replacement path, not only on append, or the short-circuit skips real overlaps and leaks duplicates into replace-all.maxEndatNaN, and since every comparison againstNaNis false that silently switches dedupe off for the rest of the scope. Only finite ends widen it, matching how the unbucketed scan treated such a range.An earlier revision of this test pinned 8 hand-picked seeds and passed while 7.7% of the seed space diverged. The sweep width is the point.
Suites: full
apps/simsuite green (23,611 passed, 25 skipped, 0 failed).resolvers.test.ts13 tests; 682 across every suite that touches this module.Reviewers should focus on: the equivalence argument in the TSDoc - specifically that a bucket holds exactly the entries the old predicate could match (scope key and range present), and that buckets keep insertion order so the first-overlap break reproduces
findIndex's lowest-index semantics.Checklist
Screenshots/Videos
No visual change. The observable difference is responsiveness: on an 81-block workflow, Cmd+F then typing
e(3373 matches) hangs the field for ~1.5s before this change and types through cleanly after.