Skip to content

Fix SidebarSearch debounce reset and stale-response race - #97

Merged
danibsheehan merged 1 commit into
mainfrom
fix/sidebar-search-debounce-race
Aug 30, 2026
Merged

Fix SidebarSearch debounce reset and stale-response race#97
danibsheehan merged 1 commit into
mainfrom
fix/sidebar-search-debounce-race

Conversation

@danibsheehan

Copy link
Copy Markdown
Owner

Summary

Found via `/code-review` while working on the docs PR (#96) — it scanned the whole repo since that branch had no code diff of its own, and surfaced two real issues in already-merged `SidebarSearch.tsx` (from #94), independent of the review's incorrect line-number citations. Verified both against the actual file before fixing.

  • Debounce reset on unrelated edits: `getPage`'s identity changes on every commit anywhere in the workspace (`WorkspaceContext` replaces the `pages` array on each `updatePageBlocks`/`updatePageTitle`), not just when search-relevant state changes. It was in the debounce effect's dependency array, so typing in the main editor while a search was pending kept resetting or double-firing the debounce — burning calls against the budgeted search endpoint for no new input. Fixed via the standard "latest ref" pattern so the effect only depends on `query`.
  • Stale-response race: `searchNotes` results had no staleness guard, unlike the sibling `RelatedPagesSection`'s `cancelled` flag. A slower in-flight request for an old query could resolve after a newer, faster one and silently overwrite its correct results. Added the same guard.

Both new regression tests fail against the pre-fix code (verified via `git stash`) and pass against the fix — not just asserting the code does what it does.

Test plan

  • `npx tsc -b --noEmit`, `npm run lint`, `npm run format:check` — all pass
  • `npx vitest run` — 163/163 passing (2 new)
  • Both new tests independently verified to fail against the old code
  • `npm run build` — succeeds
  • `/code-review` — clean

🤖 Generated with Claude Code

Found via /code-review while working on an unrelated docs PR (it
scanned the whole repo since that branch had no code diff of its
own) — both issues checked out as real against the actual merged
code, independent of the review's wrong line numbers.

- getPage's identity changes on every commit anywhere in the
  workspace (WorkspaceContext replaces the `pages` array on each
  updatePageBlocks/updatePageTitle), not just when search-relevant
  state changes. It was in the debounce effect's deps, so typing in
  the main editor while a search was pending kept resetting or
  double-firing the debounce — burning calls against the budgeted
  search endpoint for no new input. Fixed by reading it through a
  ref (the standard "latest ref" pattern) instead, so the effect
  only depends on `query`.
- searchNotes results had no staleness guard, unlike the sibling
  RelatedPagesSection's `cancelled` flag. A slower in-flight request
  for an old query could resolve after a newer, faster one and
  silently overwrite its correct results. Added the same guard.

Both regression tests fail against the pre-fix code (verified via
git stash) and pass against the fix.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

PR guide

Auto-generated from changed paths. Fill in Summary and How to verify in the PR description above.

Touches: app shell / routes, tests

Diff: 2 files changed, 159 insertions(+), 2 deletions(-)

Suggested verify

  • npm run lint
  • npm run format:check
  • npm run test:run or targeted Vitest files for the changed area
  • npm run build

Checklist (applies to this PR)

  • No unintended secrets or local-only config committed
  • Routes work with import.meta.env.BASE_URL for GitHub Pages subpath hosting

Reviewer focus

  • Routing, layout state, and Pages BASE_URL behavior
  • Test assertions cover the behavior under review rather than only implementation details

Commits

  • dcc3575 Fix SidebarSearch debounce reset and stale-response race

Files by area

Area Files
app shell / routes 2
tests 1

CI

Primary check: CI runs lint, Prettier format check, Vitest coverage, and build for every PR. Coverage tables are posted separately.


Template: pull_request_template.md

@github-actions github-actions Bot added area: app App shell, layout, or routing area: tests Test files or test tooling labels Aug 30, 2026
@github-actions

Copy link
Copy Markdown

Unit tests (Vitest)

File Coverage Lines Branches
All files 55% 63% 48%
src/components/DatabaseEmbedNodeView.tsx 0% 0% 0%
src/components/DatabaseTableView.tsx 0% 0% 0%
src/components/SidebarSearch.tsx 63% 76% 51%
src/components/SlashMenu.tsx 72% 85% 60%
src/context/WorkspaceContext.tsx 25% 35% 14%
src/extensions/blockIdOnBlocks.ts 75% 100% 50%
src/extensions/musingDatabaseEmbed.ts 70% 90% 50%
src/extensions/singleTopLevelBlock.ts 80% 100% 60%
src/hooks/usePageIndexing.ts 95% 90% 100%
src/lib/aiClient.ts 81% 86% 76%
src/lib/blockEditorCommands.ts 42% 50% 35%
src/lib/blockPlainText.ts 55% 60% 50%
src/lib/editorBlockText.ts 13% 13% 14%
src/lib/emojiSuggestionItems.ts 93% 100% 87%
src/lib/htmlToDocx.ts 24% 31% 18%
src/lib/pageToExportHtml.ts 97% 100% 94%
src/lib/resolveWikiPage.ts 91% 100% 83%
src/lib/slashMenuOptions.ts 57% 57% 0%
src/lib/themePreference.ts 32% 25% 40%
src/lib/workspaceStorage.ts 80% 81% 80%
src/lib/pageDocument/blockGutterOps.ts 92% 100% 84%
src/lib/pageDocument/blockIdAtSelection.ts 83% 100% 66%
src/lib/pageDocument/lastBlockNeedsBelowHit.ts 91% 100% 83%
src/lib/pageDocument/serializeDocToBlocks.ts 58% 69% 47%
src/lib/pageDocument/slashMenuDeleteRange.ts 82% 95% 70%
src/lib/pageDocument/tryDeleteEmptyTopLevelBlock.ts 87% 94% 81%

Minimum allowed coverage is 0%

Generated by 🐒 cobertura-action against dcc3575

@danibsheehan
danibsheehan merged commit 21a5a28 into main Aug 30, 2026
8 checks passed
@danibsheehan
danibsheehan deleted the fix/sidebar-search-debounce-race branch August 30, 2026 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: app App shell, layout, or routing area: tests Test files or test tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant