mirror of
https://github.com/simstudioai/sim.git
synced 2026-10-30 19:32:46 +08:00
refactor(search): one definition of what a search occurrence is (#6905)
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>
This commit is contained in:
co-authored by
Claude Opus 5
parent
2e111f615c
commit
85290693c4
@@ -12,6 +12,7 @@ import {
|
||||
SEARCH_REPLACE_BLOCK_CONFIGS,
|
||||
} from '@/lib/workflows/search-replace/search-replace.fixtures'
|
||||
import { WORKFLOW_SEARCH_SUBFLOW_FIELD_IDS } from '@/lib/workflows/search-replace/subflow-fields'
|
||||
import { NoteBlock } from '@/blocks/blocks/note'
|
||||
|
||||
/**
|
||||
* Uses the real tool registry. Nothing here imports it directly — the dependency
|
||||
@@ -167,6 +168,19 @@ describe('indexWorkflowSearchMatches', () => {
|
||||
expect(matches.some((match) => match.target.kind === 'block-name')).toBe(false)
|
||||
})
|
||||
|
||||
describe('the Note body declares the markdown format the card assumes', () => {
|
||||
/*
|
||||
* The canvas card projects markdown escapes unconditionally — it renders from a package that
|
||||
* cannot read the block registry. The indexer projects only when the field says so. Dropping
|
||||
* the declaration would leave the two disagreeing about what an occurrence is, and the failure
|
||||
* is silent: the panel counts a hit the card marks somewhere else.
|
||||
*/
|
||||
it('keeps searchTextFormat on the Note content field', () => {
|
||||
const content = NoteBlock.subBlocks.find((subBlock) => subBlock.id === 'content')
|
||||
expect(content?.searchTextFormat).toBe('markdown')
|
||||
})
|
||||
})
|
||||
|
||||
describe('a markdown field is searched as it renders', () => {
|
||||
/*
|
||||
* The rich-text editor backslash-escapes every markdown-significant character in prose, so a
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { isRecordLike } from '@sim/utils/object'
|
||||
import { foldSearchWhitespace, projectEscapedMarkdownForSearch } from '@sim/utils/string'
|
||||
import { forEachSearchOccurrence, 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'
|
||||
@@ -57,16 +57,6 @@ import {
|
||||
type ToolParameterConfig,
|
||||
} from '@/tools/params'
|
||||
|
||||
/**
|
||||
* Whitespace is folded before comparison (see {@link foldSearchWhitespace}):
|
||||
* the fold is one-to-one, so ranges found in the normalized string index the
|
||||
* original text correctly.
|
||||
*/
|
||||
function normalizeForSearch(value: string, caseSensitive: boolean): string {
|
||||
const folded = foldSearchWhitespace(value)
|
||||
return caseSensitive ? folded : folded.toLowerCase()
|
||||
}
|
||||
|
||||
/**
|
||||
* Ranges of `query` in `value`, always in `value`'s own coordinates.
|
||||
*
|
||||
@@ -83,23 +73,21 @@ function findTextRanges(
|
||||
caseSensitive: boolean,
|
||||
searchTextFormat?: SubBlockConfig['searchTextFormat']
|
||||
) {
|
||||
if (!query) return []
|
||||
|
||||
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) {
|
||||
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))
|
||||
}
|
||||
forEachSearchOccurrence(
|
||||
projection ? projection.text : value,
|
||||
query,
|
||||
(start, end) => {
|
||||
ranges.push(
|
||||
projection
|
||||
? { start: projection.starts[start], end: projection.starts[end] }
|
||||
: { start, end }
|
||||
)
|
||||
},
|
||||
caseSensitive
|
||||
)
|
||||
|
||||
return ranges
|
||||
}
|
||||
|
||||
@@ -182,6 +182,42 @@ export function foldSearchWhitespace(value: string): string {
|
||||
return value.replace(/\s/g, ' ')
|
||||
}
|
||||
|
||||
/**
|
||||
* Visits every occurrence of `query` in `text`, without overlaps.
|
||||
*
|
||||
* The single definition of what "an occurrence" means for search, shared by the
|
||||
* workflow search index and by the Note card that has to mark the same hits on
|
||||
* the canvas. They live in different packages and cannot see each other, so a
|
||||
* second copy of this loop is a silent disagreement waiting to happen: the
|
||||
* panel counts a match the card never paints, which is exactly the bug that
|
||||
* arrived when only the whitespace fold was shared and the scan was not.
|
||||
*
|
||||
* Whitespace is folded first (see {@link foldSearchWhitespace}) and the fold is
|
||||
* one-to-one, so both bounds index the caller's own unfolded string.
|
||||
*/
|
||||
export function forEachSearchOccurrence(
|
||||
text: string,
|
||||
query: string,
|
||||
visit: (start: number, end: number) => void,
|
||||
caseSensitive = false
|
||||
): void {
|
||||
if (!query) return
|
||||
|
||||
const normalize = (value: string) => {
|
||||
const folded = foldSearchWhitespace(value)
|
||||
return caseSensitive ? folded : folded.toLowerCase()
|
||||
}
|
||||
const haystack = normalize(text)
|
||||
const needle = normalize(query)
|
||||
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)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* ASCII punctuation a backslash may escape in markdown, per CommonMark. A
|
||||
* backslash before anything else is a literal backslash.
|
||||
|
||||
@@ -13,6 +13,7 @@
|
||||
*/
|
||||
|
||||
import { act } from 'react'
|
||||
import { forEachSearchOccurrence } from '@sim/utils/string'
|
||||
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'
|
||||
@@ -24,7 +25,6 @@ import {
|
||||
} from '../index'
|
||||
import {
|
||||
countNoteSearchOccurrencesBefore,
|
||||
forEachNoteSearchOccurrence,
|
||||
noteSearchHighlightPlugin,
|
||||
} from './note-search-highlight'
|
||||
|
||||
@@ -125,13 +125,13 @@ function paragraphTree(...values: string[]): Root {
|
||||
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))
|
||||
forEachSearchOccurrence('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))
|
||||
forEachSearchOccurrence('aaaa', 'aa', (start) => starts.push(start))
|
||||
expect(starts).toEqual([0, 2])
|
||||
})
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { foldSearchWhitespace, projectEscapedMarkdownForSearch } from '@sim/utils/string'
|
||||
import { forEachSearchOccurrence, projectEscapedMarkdownForSearch } from '@sim/utils/string'
|
||||
import type { Element, Root, Text } from 'hast'
|
||||
|
||||
/**
|
||||
@@ -41,46 +41,6 @@ export interface NoteSearchRange {
|
||||
*/
|
||||
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`), 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,
|
||||
query: string,
|
||||
visit: (start: number, end: number) => void
|
||||
): void {
|
||||
if (!query) return
|
||||
|
||||
/* 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)
|
||||
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.
|
||||
*/
|
||||
/**
|
||||
* Visits every occurrence of `query` in a note's markdown SOURCE, reporting each
|
||||
* start in source coordinates.
|
||||
@@ -96,7 +56,7 @@ export function forEachNoteSourceOccurrence(
|
||||
visit: (sourceStart: number) => void
|
||||
): void {
|
||||
const projection = projectEscapedMarkdownForSearch(content)
|
||||
forEachNoteSearchOccurrence(projection.text, query, (start) => {
|
||||
forEachSearchOccurrence(projection.text, query, (start) => {
|
||||
visit(projection.starts[start])
|
||||
})
|
||||
}
|
||||
@@ -279,7 +239,7 @@ export function noteSearchHighlightPlugin({ query }: NoteSearchHighlightOptions)
|
||||
|
||||
for (const run of builder.runs) {
|
||||
const text = run.map((entry) => entry.node.value).join('')
|
||||
forEachNoteSearchOccurrence(text, query, (start, end) => {
|
||||
forEachSearchOccurrence(text, query, (start, end) => {
|
||||
const current = ordinal
|
||||
ordinal += 1
|
||||
for (const entry of run) {
|
||||
|
||||
Reference in New Issue
Block a user