Skip to content

🐛(search) fix double request and flickering on search#596

Merged
jbpenrath merged 1 commit intomainfrom
fix/search-query
Mar 19, 2026
Merged

🐛(search) fix double request and flickering on search#596
jbpenrath merged 1 commit intomainfrom
fix/search-query

Conversation

@jbpenrath
Copy link
Copy Markdown
Contributor

@jbpenrath jbpenrath commented Mar 19, 2026

Purpose

The search queryKey included searchParams.toString(), creating a new React Query entry per search term (triggering fetch #1). Meanwhile, resetSearchQueryDebounced cleared the active query 500ms later (triggering fetch #2 + flicker).

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Search results now refresh immediately when you change the search term in a mailbox.
    • Exiting search mode reliably clears search-specific results to avoid showing stale threads.
    • Thread selection is cleared when search parameters change, preventing mismatched thread views.

@jbpenrath jbpenrath self-assigned this Mar 19, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 19, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e933d308-e1d3-48ae-8b99-8d5e6c4b06b5

📥 Commits

Reviewing files that changed from the base of the PR and between e620ce7 and a575d4f.

📒 Files selected for processing (1)
  • src/frontend/src/features/providers/mailbox.tsx

📝 Walkthrough

Walkthrough

The mailbox provider's search handling was refactored: debounced global query resets were removed, the threads query key now uses a fixed 'search' identifier when a search is active, and cache invalidation is performed immediately and scoped to that simplified key while preserving thread unselection.

Changes

Cohort / File(s) Summary
Search Query Management
src/frontend/src/features/providers/mailbox.tsx
Removed debounced resetSearchQueryDebounced that targeted all 'search' queries; changed threadQueryKey to ['threads', selectedMailbox?.id, 'search'] when searching; replaced debounced global resets with immediate, scoped queryClient.removeQueries when leaving search and queryClient.resetQueries when search term changes; retained unselectThread() call in the same effect.

Sequence Diagram(s)

sequenceDiagram
    participant UI as Mailbox Component
    participant QC as QueryClient
    participant Sel as ThreadSelection

    UI->>UI: detect search param change
    alt leaving search mode
        UI->>QC: removeQueries(['threads', mailboxId, 'search'], exact:true)
        QC-->>UI: queries removed
        UI->>Sel: unselectThread()
        Sel-->>UI: thread unselected
    else search term changed (still searching)
        UI->>QC: resetQueries(['threads', mailboxId, 'search'])
        QC-->>UI: queries reset/refetched
        UI->>Sel: unselectThread()
        Sel-->>UI: thread unselected
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I nudged the queries, straight and quick,
No bouncy waits, no debounced trick,
Keys trimmed down, the chase is neat,
Threads reset brisk on nimble feet,
A carrot sprint — so swift, so slick! 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly addresses the main issue: fixing double requests and flickering on search, which aligns with the PR's objective to simplify the search query key and remove debounced cache clearing.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/search-query
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/frontend/src/features/providers/mailbox.tsx (1)

140-146: Centralize the thread query-key builder.

src/frontend/src/features/layouts/components/thread-view/components/thread-summary/index.tsx:1-80 now duplicates this same search ? ['threads', mailboxId, 'search'] : ['threads', mailboxId, searchParams.toString()] branching. Pulling it into one helper would keep future cache-key fixes from drifting between the provider and the thread summary view.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/frontend/src/features/providers/mailbox.tsx` around lines 140 - 146,
Duplicate logic for building the threads query key exists in the mailbox
provider (threadQueryKey in the useMemo using selectedMailbox?.id and
searchParams) and in thread-summary; extract that branching into a single helper
(e.g., buildThreadQueryKey or getThreadQueryKey) that accepts mailboxId and
searchParams and returns either ['threads', mailboxId, 'search'] or ['threads',
mailboxId, searchParams.toString()], then replace the inline useMemo logic in
the provider (threadQueryKey) and the component in
src/frontend/src/features/layouts/components/thread-view/components/thread-summary
to call the shared helper so both places use the same canonical cache-key
builder.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/frontend/src/features/providers/mailbox.tsx`:
- Around line 597-603: The effect only resets queries when the search term
changes but doesn't clear the cached search results when exiting search mode;
update the useEffect that references previousSearchParams, searchParams and
queryClient so that when previousSearchParams?.get('search') is truthy and
searchParams.get('search') is falsy (search -> null transition) you call
queryClient.removeQueries({ queryKey: ['threads', selectedMailbox?.id, 'search']
}) to purge the cached search results for the current mailbox ID (use the same
selectedMailbox?.id key used elsewhere).

---

Nitpick comments:
In `@src/frontend/src/features/providers/mailbox.tsx`:
- Around line 140-146: Duplicate logic for building the threads query key exists
in the mailbox provider (threadQueryKey in the useMemo using selectedMailbox?.id
and searchParams) and in thread-summary; extract that branching into a single
helper (e.g., buildThreadQueryKey or getThreadQueryKey) that accepts mailboxId
and searchParams and returns either ['threads', mailboxId, 'search'] or
['threads', mailboxId, searchParams.toString()], then replace the inline useMemo
logic in the provider (threadQueryKey) and the component in
src/frontend/src/features/layouts/components/thread-view/components/thread-summary
to call the shared helper so both places use the same canonical cache-key
builder.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1605adb1-2274-40dd-8cd1-bf6dbf963268

📥 Commits

Reviewing files that changed from the base of the PR and between 7a8cf44 and e620ce7.

📒 Files selected for processing (1)
  • src/frontend/src/features/providers/mailbox.tsx

The search queryKey included searchParams.toString(), creating a new
React Query entry per search term (triggering fetch #1). Meanwhile,
resetSearchQueryDebounced cleared the active query 500ms later
(triggering fetch #2 + flicker).
@jbpenrath jbpenrath merged commit 3461981 into main Mar 19, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant