Skip to content

Commit 86dbd0a

Browse files
improvement(search): make overlap dedupe linear in match count (#6640)
* perf(search): make overlap dedupe linear in match count 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. * docs(search): describe maxEnd as a bound, not the exact maximum 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. * fix(search): preserve infinite range overlap semantics * test(search): group dedupe equivalence coverage --------- Co-authored-by: Vikhyath Mondreti <vikhyath@simstudio.ai>
1 parent 2da8855 commit 86dbd0a

2 files changed

Lines changed: 339 additions & 11 deletions

File tree

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

Lines changed: 258 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,13 @@
44
import { describe, expect, it } from 'vitest'
55
import {
66
dedupeOverlappingWorkflowSearchMatches,
7+
OVERLAPPING_MATCH_KIND_PRIORITY,
78
workflowSearchMatchMatchesQuery,
89
} from '@/lib/workflows/search-replace/resources/resolvers'
9-
import type { WorkflowSearchMatch } from '@/lib/workflows/search-replace/types'
10+
import type {
11+
WorkflowSearchMatch,
12+
WorkflowSearchMatchKind,
13+
} from '@/lib/workflows/search-replace/types'
1014

1115
function createMatch(overrides: Partial<WorkflowSearchMatch>): WorkflowSearchMatch {
1216
return {
@@ -115,6 +119,259 @@ describe('dedupeOverlappingWorkflowSearchMatches', () => {
115119
secondMatch,
116120
])
117121
})
122+
123+
/**
124+
* The bucketed dedupe replaced an O(n^2) linear rescan. This pins it to a
125+
* transcription of the original algorithm over randomized inputs, so any
126+
* divergence in which overlapping match wins shows up as a diff rather than
127+
* as a subtly wrong result the fixed examples above would miss.
128+
*/
129+
describe('bucketed dedupe matches the original linear scan', () => {
130+
function scopeKey(match: WorkflowSearchMatch): string | null {
131+
if (!match.range) return null
132+
if (match.target.kind !== 'subblock') return null
133+
const path = match.valuePath.map((s) => `${typeof s}:${String(s)}`).join('/')
134+
return [match.blockId, match.subBlockId, path].join(':')
135+
}
136+
137+
function rangeLength(match: WorkflowSearchMatch): number {
138+
return match.range ? match.range.end - match.range.start : Number.POSITIVE_INFINITY
139+
}
140+
141+
function prefers(candidate: WorkflowSearchMatch, current: WorkflowSearchMatch): boolean {
142+
const a = rangeLength(candidate)
143+
const b = rangeLength(current)
144+
if (a !== b) return a < b
145+
const pa = OVERLAPPING_MATCH_KIND_PRIORITY[candidate.kind]
146+
const pb = OVERLAPPING_MATCH_KIND_PRIORITY[current.kind]
147+
if (pa !== pb) return pa > pb
148+
return false
149+
}
150+
151+
/** Straight transcription of the pre-optimization implementation. */
152+
function referenceDedupe(matches: WorkflowSearchMatch[]): WorkflowSearchMatch[] {
153+
const deduped: WorkflowSearchMatch[] = []
154+
for (const match of matches) {
155+
const key = scopeKey(match)
156+
const range = match.range
157+
const existingIndex =
158+
key && range
159+
? deduped.findIndex(
160+
(candidate) =>
161+
scopeKey(candidate) === key &&
162+
candidate.range &&
163+
candidate.range.start < range.end &&
164+
range.start < candidate.range.end
165+
)
166+
: -1
167+
if (existingIndex === -1) {
168+
deduped.push(match)
169+
continue
170+
}
171+
if (prefers(match, deduped[existingIndex])) deduped[existingIndex] = match
172+
}
173+
return deduped
174+
}
175+
176+
/** Deterministic PRNG so a failure is reproducible from the seed alone. */
177+
function makeRandom(seed: number) {
178+
let state = seed
179+
return () => {
180+
state = (state * 1103515245 + 12345) & 0x7fffffff
181+
return state / 0x7fffffff
182+
}
183+
}
184+
185+
const KINDS = Object.keys(OVERLAPPING_MATCH_KIND_PRIORITY) as WorkflowSearchMatchKind[]
186+
187+
function randomMatches(seed: number, count: number): WorkflowSearchMatch[] {
188+
const random = makeRandom(seed)
189+
const pick = <T>(xs: T[]) => xs[Math.floor(random() * xs.length)]
190+
return Array.from({ length: count }, (_, index) => {
191+
const start = Math.floor(random() * 30)
192+
const hasRange = random() > 0.15
193+
// Degenerate spans too: the bucketed scan keys off range arithmetic, so
194+
// the oracle has to defend inverted, empty and non-finite ends as well.
195+
const degenerate = random()
196+
const end =
197+
degenerate > 0.98
198+
? Number.NaN
199+
: degenerate > 0.96
200+
? Number.POSITIVE_INFINITY
201+
: degenerate > 0.94
202+
? Number.NEGATIVE_INFINITY
203+
: degenerate > 0.91
204+
? start - 1 - Math.floor(random() * 3)
205+
: degenerate > 0.88
206+
? start
207+
: start + 1 + Math.floor(random() * 8)
208+
const isSubBlockTarget = random() > 0.15
209+
return createMatch({
210+
id: `m-${index}`,
211+
blockId: pick(['b1', 'b2', 'b3']),
212+
subBlockId: pick(['s1', 's2']),
213+
valuePath: pick([[], ['content'], [0], ['rows', 1]]),
214+
kind: pick(KINDS),
215+
target: isSubBlockTarget ? { kind: 'subblock' } : { kind: 'block-name' },
216+
range: hasRange ? { start, end } : undefined,
217+
})
218+
})
219+
}
220+
221+
/**
222+
* A sequential sweep, not a handful of hand-picked seeds. An earlier version
223+
* pinned 8 seeds that happened to be clean while 7.7% of the space diverged,
224+
* so the count is what gives this test its power - keep it wide.
225+
*/
226+
it('agrees with the original scan across 400 seeded inputs', () => {
227+
const diverged: number[] = []
228+
229+
for (let seed = 1; seed <= 400; seed++) {
230+
const matches = randomMatches(seed, 120)
231+
const actual = dedupeOverlappingWorkflowSearchMatches(matches).map((m) => m.id)
232+
const expected = referenceDedupe(matches).map((m) => m.id)
233+
if (actual.join('|') !== expected.join('|')) diverged.push(seed)
234+
}
235+
236+
expect(diverged).toEqual([])
237+
})
238+
239+
/**
240+
* The exact shape that broke the `maxEnd` short-circuit: a shorter range
241+
* evicts a longer one but ends further right, so a stale `maxEnd` let the
242+
* next match skip the overlap scan and leak through as a duplicate.
243+
*/
244+
it.each([
245+
{
246+
name: 'shorter replacement ends further right',
247+
spans: [
248+
{ kind: 'workflow-reference' as const, start: 0, end: 13 },
249+
{ kind: 'environment' as const, start: 10, end: 17 },
250+
{ kind: 'text' as const, start: 13, end: 16 },
251+
],
252+
},
253+
{
254+
name: 'replacement extends past the evicted range',
255+
spans: [
256+
{ kind: 'text' as const, start: 0, end: 10 },
257+
{ kind: 'environment' as const, start: 5, end: 15 },
258+
{ kind: 'text' as const, start: 10, end: 14 },
259+
],
260+
},
261+
])('collapses overlaps when a $name', ({ spans }) => {
262+
const matches = spans.map((span, index) =>
263+
createMatch({
264+
id: `span-${index}`,
265+
blockId: 'b1',
266+
subBlockId: 's1',
267+
valuePath: [],
268+
kind: span.kind,
269+
range: { start: span.start, end: span.end },
270+
})
271+
)
272+
273+
expect(dedupeOverlappingWorkflowSearchMatches(matches).map((m) => m.id)).toEqual(
274+
referenceDedupe(matches).map((m) => m.id)
275+
)
276+
expect(dedupeOverlappingWorkflowSearchMatches(matches)).toHaveLength(1)
277+
})
278+
279+
it('agrees when a positive-infinity range contains a later range', () => {
280+
const matches = [
281+
createMatch({
282+
id: 'unbounded',
283+
kind: 'workflow-reference',
284+
range: { start: 0, end: Number.POSITIVE_INFINITY },
285+
}),
286+
createMatch({
287+
id: 'inside',
288+
kind: 'text',
289+
range: { start: 1, end: 2 },
290+
}),
291+
]
292+
293+
expect(dedupeOverlappingWorkflowSearchMatches(matches).map((match) => match.id)).toEqual(
294+
referenceDedupe(matches).map((match) => match.id)
295+
)
296+
expect(dedupeOverlappingWorkflowSearchMatches(matches).map((match) => match.id)).toEqual([
297+
'inside',
298+
])
299+
})
300+
301+
it.each([0, 1])('agrees on a %i-element input', (count) => {
302+
const matches = randomMatches(5, count)
303+
304+
expect(dedupeOverlappingWorkflowSearchMatches(matches).map((m) => m.id)).toEqual(
305+
referenceDedupe(matches).map((m) => m.id)
306+
)
307+
})
308+
309+
/**
310+
* The bucketed scan is still linear *within* one scope, so a single field
311+
* holding many non-overlapping hits is the residual worst case. It stays
312+
* cheap because the inner loop is two integer comparisons - the old code
313+
* rebuilt a scope-key string per candidate, which is where the 100x went.
314+
*/
315+
it('agrees when one scope holds many non-overlapping ranges', () => {
316+
const matches = Array.from({ length: 300 }, (_, index) =>
317+
createMatch({
318+
id: `disjoint-${index}`,
319+
blockId: 'b1',
320+
subBlockId: 'code',
321+
valuePath: [],
322+
kind: 'text',
323+
range: { start: index * 4, end: index * 4 + 1 },
324+
})
325+
)
326+
327+
expect(dedupeOverlappingWorkflowSearchMatches(matches)).toHaveLength(300)
328+
expect(dedupeOverlappingWorkflowSearchMatches(matches).map((m) => m.id)).toEqual(
329+
referenceDedupe(matches).map((m) => m.id)
330+
)
331+
})
332+
333+
/**
334+
* Pins the asymptotics, not a stopwatch. 20k disjoint hits in one scope run
335+
* in single-digit ms bucketed; the O(n^2) rescan this replaced took ~30s on
336+
* the same input, so the bound has roughly three orders of magnitude of
337+
* headroom and only trips on a genuine complexity regression.
338+
*/
339+
it('stays sub-quadratic on a single scope full of disjoint ranges', () => {
340+
const matches = Array.from({ length: 20_000 }, (_, index) =>
341+
createMatch({
342+
id: `wide-${index}`,
343+
blockId: 'b1',
344+
subBlockId: 'code',
345+
valuePath: [],
346+
kind: 'text',
347+
range: { start: index * 4, end: index * 4 + 1 },
348+
})
349+
)
350+
351+
const startedAt = performance.now()
352+
const deduped = dedupeOverlappingWorkflowSearchMatches(matches)
353+
354+
expect(deduped).toHaveLength(20_000)
355+
expect(performance.now() - startedAt).toBeLessThan(2_000)
356+
})
357+
358+
it('agrees when every match shares one scope and range', () => {
359+
const matches = Array.from({ length: 40 }, (_, index) =>
360+
createMatch({
361+
id: `same-${index}`,
362+
blockId: 'b1',
363+
subBlockId: 's1',
364+
valuePath: [],
365+
kind: index % 2 === 0 ? 'text' : 'table',
366+
range: { start: 0, end: 5 },
367+
})
368+
)
369+
370+
expect(dedupeOverlappingWorkflowSearchMatches(matches).map((m) => m.id)).toEqual(
371+
referenceDedupe(matches).map((m) => m.id)
372+
)
373+
})
374+
})
118375
})
119376

120377
describe('workflowSearchMatchMatchesQuery', () => {

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

Lines changed: 81 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,12 @@ import type {
77
} from '@/lib/workflows/search-replace/types'
88
import type { SelectorContext } from '@/hooks/selectors/types'
99

10-
const OVERLAPPING_MATCH_KIND_PRIORITY: Record<WorkflowSearchMatchKind, number> = {
10+
/**
11+
* Which kind wins when two matches cover the same span. Exported so the
12+
* equivalence tests can share it instead of hand-copying the values, which
13+
* silently drifted once already.
14+
*/
15+
export const OVERLAPPING_MATCH_KIND_PRIORITY: Record<WorkflowSearchMatchKind, number> = {
1116
text: 0,
1217
environment: 1,
1318
'workflow-reference': 2,
@@ -141,31 +146,97 @@ function shouldPreferOverlappingMatch(
141146
return false
142147
}
143148

149+
/** Kept indices for one overlap scope, plus an upper bound on their `range.end`. */
150+
interface RangeMatchScopeBucket {
151+
indices: number[]
152+
maxEnd: number
153+
}
154+
155+
function widenScopeBucket(bucket: RangeMatchScopeBucket, end: number): void {
156+
if (!Number.isNaN(end)) bucket.maxEnd = Math.max(bucket.maxEnd, end)
157+
}
158+
159+
/**
160+
* Overlap resolution is scoped to one value inside one subblock, so candidates
161+
* are bucketed by that scope rather than rescanned. The previous `findIndex`
162+
* over the whole accumulated list recomputed every candidate's scope key on
163+
* every iteration - O(n^2) string builds, which cost ~1.4s on a workflow
164+
* producing ~500 matches and froze the search field while typing.
165+
*
166+
* A bucket only ever holds entries that have both a scope key and a range, and
167+
* those are exactly the entries the old predicate could match. Buckets keep
168+
* insertion order and the scan stops at the first overlap, so this picks the
169+
* same candidate the linear scan did.
170+
*
171+
* `maxEnd` is a monotonic high-water mark, not the exact current maximum: a
172+
* replacement can swap in a range that ends earlier without lowering it. Only
173+
* the upper bound is load-bearing. A match starting at or after it cannot
174+
* overlap anything in the bucket, so the scan is skipped; a bound left too high
175+
* only costs a scan that would have been skipped, never a wrong answer.
176+
*
177+
* Recomputing the exact maximum on every shrinking replacement is a net loss -
178+
* it walks the bucket, which is the cost this is here to avoid, and staleness
179+
* is capped at one token length because every range spans a matched token
180+
* (`query.length`, or a reference's `rawValue.length`) rather than the field.
181+
* Measured on the realistic overlap shape at 10k matches: 23ms as written,
182+
* 45ms with the recompute, against 1010ms for the scan this replaced.
183+
*
184+
* The bound keeps a field full of disjoint hits linear instead of quadratic
185+
* within its own bucket, to the extent its matches arrive in ascending offset
186+
* order; out-of-order producers just fall back to scanning.
187+
*
188+
* It must be refreshed on the replacement path too, not only on append:
189+
* `shouldPreferOverlappingMatch` prefers the SHORTER range, and a shorter range
190+
* can still end further right than the one it evicts. Leaving `maxEnd` stale
191+
* there let the short-circuit skip genuine overlaps and leak duplicates.
192+
*
193+
* Only `NaN` ends are ignored. `Math.max` with `NaN` would pin `maxEnd` at
194+
* `NaN`, and since every comparison against `NaN` is false that would silently
195+
* switch dedupe off for the rest of the scope. A `NaN`-ended range cannot
196+
* overlap anything anyway, while positive infinity is an unbounded end that
197+
* can overlap later ranges and therefore must widen the high-water mark.
198+
*/
144199
export function dedupeOverlappingWorkflowSearchMatches<T extends WorkflowSearchMatch>(
145200
matches: T[]
146201
): T[] {
147202
const deduped: T[] = []
203+
const bucketsByScopeKey = new Map<string, RangeMatchScopeBucket>()
148204

149205
for (const match of matches) {
150206
const scopeKey = getRangeMatchScopeKey(match)
151207
const matchRange = match.range
152-
const existingIndex =
153-
scopeKey && matchRange
154-
? deduped.findIndex(
155-
(candidate) =>
156-
getRangeMatchScopeKey(candidate) === scopeKey &&
157-
candidate.range &&
158-
rangesOverlap(candidate.range, matchRange)
159-
)
160-
: -1
208+
const bucket = scopeKey && matchRange ? bucketsByScopeKey.get(scopeKey) : undefined
209+
210+
let existingIndex = -1
211+
if (bucket && matchRange && matchRange.start < bucket.maxEnd) {
212+
for (const index of bucket.indices) {
213+
const candidate = deduped[index]
214+
if (candidate.range && rangesOverlap(candidate.range, matchRange)) {
215+
existingIndex = index
216+
break
217+
}
218+
}
219+
}
161220

162221
if (existingIndex === -1) {
222+
if (scopeKey && matchRange) {
223+
if (bucket) {
224+
bucket.indices.push(deduped.length)
225+
widenScopeBucket(bucket, matchRange.end)
226+
} else {
227+
bucketsByScopeKey.set(scopeKey, {
228+
indices: [deduped.length],
229+
maxEnd: Number.isNaN(matchRange.end) ? Number.NEGATIVE_INFINITY : matchRange.end,
230+
})
231+
}
232+
}
163233
deduped.push(match)
164234
continue
165235
}
166236

167237
if (shouldPreferOverlappingMatch(match, deduped[existingIndex])) {
168238
deduped[existingIndex] = match
239+
if (bucket && matchRange) widenScopeBucket(bucket, matchRange.end)
169240
}
170241
}
171242

0 commit comments

Comments
 (0)