Skip to content

fix(addon-search): fix PR #5904 integration test failures#5908

Closed
Tyriar wants to merge 49 commits into
masterfrom
cursor/fix-pr-5904-integration-tests-e9c4
Closed

fix(addon-search): fix PR #5904 integration test failures#5908
Tyriar wants to merge 49 commits into
masterfrom
cursor/fix-pr-5904-integration-tests-e9c4

Conversation

@Tyriar

@Tyriar Tyriar commented May 27, 2026

Copy link
Copy Markdown
Member

Fixes integration test failures in the re-implemented search addon from #5904.

Problem

The rewritten SearchAddon in #5904 failed several addon-search Playwright integration tests related to:

  • Viewport centering after findNext
  • onDidChangeResults resultIndex when stepping, changing terms, or exceeding the 1000-match highlight limit
  • findPrevious when the active match is outside the highlighted match list

Solution

  • Track navigated index separately from reported index so stepping works even when the active match is not in the capped highlight list (resultIndex: -1)
  • Restore buffer-backed findPrevious / findNext navigation from the saved selection position (matching pre-rewrite behavior)
  • Preserve result index when changing search terms with decorations (including incremental flows)
  • Fix the viewport centering test page.evaluate syntax (IIFE string; avoid template-literal interpolation)

Verification

npm run build && npm run esbuild && npm run esbuild-demo-client
npm run test-integration-chromium -- --suite=addon-search

All 28 Chromium addon-search integration tests pass.

Related: #5904

Open in Web Open in Cursor 

Tyriar and others added 30 commits May 25, 2026 16:34
Evaluate shouldUpdateHighlighting before updating lastSearchOptions so
option-only changes trigger highlight refresh for the same term.

Co-authored-by: Cursor <cursoragent@cursor.com>
Compare decoration style fields in didOptionsChange so same-term searches
with updated colors or rulers trigger highlight recomputation.

Co-authored-by: Cursor <cursoragent@cursor.com>
Pass isActiveResult through to _applyStyles in onRender so active matches
receive xterm-find-active-result-decoration.

Co-authored-by: Cursor <cursoragent@cursor.com>
Scan for additional matches when the first candidate fails whole-word
checks instead of returning early.

Co-authored-by: Cursor <cursoragent@cursor.com>
Skip empty regex matches in the reverse path to match forward behavior and
avoid unstable result selection.

Co-authored-by: Cursor <cursoragent@cursor.com>
Hoist searchStringLine.slice(0, offset) outside the reverse regex exec loop.

Co-authored-by: Cursor <cursoragent@cursor.com>
Compile the line search RegExp once per find/findNext/findPrevious call
and pass it into _findInLine to avoid per-line recompilation.

Co-authored-by: Cursor <cursoragent@cursor.com>
Lazily store a lowercased line variant in the line cache entry to avoid
repeated toLowerCase calls during case-insensitive searches.

Co-authored-by: Cursor <cursoragent@cursor.com>
Skip slicing when the caller already respects the highlight limit.

Co-authored-by: Cursor <cursoragent@cursor.com>
Clear the debounced highlight timeout when decorations are cleared to
avoid stale delayed searches.

Co-authored-by: Cursor <cursoragent@cursor.com>
Clamp highlightLimit to a positive finite integer, falling back to the
default when the option is missing or invalid.

Co-authored-by: Cursor <cursoragent@cursor.com>
Add no-match and case-insensitive benchmark cases, validate found counts,
tighten SearchAddon runtime tolerances, and use 10-run baseline/eval passes.

Co-authored-by: Cursor <cursoragent@cursor.com>
Advance regex lastIndex when skipping zero-length matches during reverse
search to avoid hanging on patterns like .*?

Co-authored-by: Cursor <cursoragent@cursor.com>
Support optional overview ruler colors from decoration options without
passing undefined into terminal decoration registration.

Co-authored-by: Cursor <cursoragent@cursor.com>
Add SearchState and SearchResultTracker unit tests plus whole-word,
reverse-regex, and lowercase cache coverage.

Co-authored-by: Cursor <cursoragent@cursor.com>
Clarify incremental search, result event semantics, highlightLimit, and
optional decoration overview ruler fields.

Co-authored-by: Cursor <cursoragent@cursor.com>
Drop accidental debugger statement and unnecessary delay in
onDidChangeResults coverage.

Co-authored-by: Cursor <cursoragent@cursor.com>
Unify regex and whole-word matching into a single scan pass and avoid
per-line substring allocations by searching the full logical line with
lastIndex. Forward non-whole-word regex keeps a single-exec fast path.

Co-authored-by: Cursor <cursoragent@cursor.com>
Add SearchEngine.collectMatches to initialize cache and regex once while
gathering matches, and use it from _highlightAllMatches instead of
repeated find() calls.

Co-authored-by: Cursor <cursoragent@cursor.com>
Track buffer cache generation and skip full-buffer scans when the same
term, options, and start position previously produced no match until the
line cache is invalidated.

Co-authored-by: Cursor <cursoragent@cursor.com>
Hoist buffer bounds, reuse prepared plain terms, fast-path non-wholeWord
indexOf, and use match size for highlight collection wrap checks.

Co-authored-by: Cursor <cursoragent@cursor.com>
Create highlight decorations in a single pass, dispose markers when
registration fails, and reduce per-decoration allocation overhead.

Co-authored-by: Cursor <cursoragent@cursor.com>
Rebuild a result key map on updateResults and manage selected decoration
lifecycle with MutableDisposable.

Co-authored-by: Cursor <cursoragent@cursor.com>
Normalize and snapshot search options, clear stale decorations when
decorations are disabled, use try/finally for search events, and refresh
highlights without re-entering public navigation APIs.

Co-authored-by: Cursor <cursoragent@cursor.com>
Add benchmark coverage for whole-word navigation and reverse regex
whole-word searches to guard optimization regressions.

Co-authored-by: Cursor <cursoragent@cursor.com>
Clarify decorations usage, result event semantics, whole-word boundaries,
and export ISearchDecorationOptions.

Co-authored-by: Cursor <cursoragent@cursor.com>
Exclude out-benchmark from version control to avoid accidental commits
of compiled benchmark artifacts.

Co-authored-by: Cursor <cursoragent@cursor.com>
Tyriar and others added 19 commits May 25, 2026 20:18
…l-line caches.

Avoid recomputing full-buffer scans on repeated queries and defer expensive offset mapping work until matches are found, reducing navigation and refresh benchmark runtimes.

Co-authored-by: Cursor <cursoragent@cursor.com>
Reduce repeated search overhead with faster cache/index paths and avoid creating decorations when no terminal DOM is attached, significantly cutting benchmark runtime in decoration-heavy scenarios.

Co-authored-by: Cursor <cursoragent@cursor.com>
…next result.

This keeps findNext/findPrevious on the cached navigation path even when no selection is available, reducing repeated selection-index resolution overhead in benchmarked search loops.

Co-authored-by: Cursor <cursoragent@cursor.com>
This avoids tearing down and recreating all match decorations when users revisit recent search terms, which significantly reduces decoration refresh overhead in alternating-term search flows.

Co-authored-by: Cursor <cursoragent@cursor.com>
This avoids redundant match/decorations recomputation during bursts of parsed writes while preserving the latest refresh result.

Co-authored-by: Cursor <cursoragent@cursor.com>
Consolidate duplicated search-state and index-stepping logic into small private helpers, and centralize decoration width/render setup to reduce drift between active/inactive paths. Replace match-flag magic numbers with a typed enum and shared direction alias so internal APIs are clearer while preserving behavior and performance.

Co-authored-by: Cursor <cursoragent@cursor.com>
Replace static readonly class limit fields with an internal const enum and add JSDoc for each limit so cache and highlight bounds are centralized and self-describing.

Co-authored-by: Cursor <cursoragent@cursor.com>
Drop the unused `_lastDirection` field and remove unused method parameters introduced by prior refactors so SearchAddon internals stay minimal and easier to follow without behavior changes.

Co-authored-by: Cursor <cursoragent@cursor.com>
Remove inline decoration background styling and dispose inactive cached decoration sets so stale overlays do not render over glyphs.

Co-authored-by: Cursor <cursoragent@cursor.com>
This reduces per-cell work in cold-cache searches by using the optimized line translation path while preserving match behavior.

Co-authored-by: Cursor <cursoragent@cursor.com>
This enables BufferLine internal full-line string caching on cold-cache searches and substantially reduces findNext cold-cache runtime.

Co-authored-by: Cursor <cursoragent@cursor.com>
This adds a cold-path fast search for non-decorated findNext so cache-cleared scans can stop after finding the next selection target instead of materializing all matches.

Co-authored-by: Cursor <cursoragent@cursor.com>
This limits the cold fast path to repeated successful non-decorated searches and scans logical lines lazily, preserving general behavior while driving findNext/cold cache well past the 10x target.

Co-authored-by: Cursor <cursoragent@cursor.com>
…n tests

- Track navigated index separately from reported highlight index
- Report resultIndex -1 when the active match is outside the highlight list
- Fix findPrevious buffer search from a saved selection position
- Preserve result index when changing search terms with decorations
- Fix viewport centering integration test page.evaluate syntax

Co-authored-by: Daniel Imms <Tyriar@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants