fix(desktop): implement server-side PR search in new workspace modal#2909
Merged
Conversation
Replace client-side PR search with server-side GitHub search to fix performance and completeness issues in the new workspace modal. Previously: - Fetched all PRs upfront (slow 5-10s initial load) - Limited to 30 PRs total (hardcoded backend limit) - Client-side Fuse.js search only searched fetched PRs - Pasting PR URLs didn't work Now: - Fast initial load: shows 30 recent PRs for browsing (~1-2s) - Unlimited search: server-side GitHub search via gh CLI - Searches ALL PRs in the repo (no limits) - Debounced search (300ms) for better UX - URL detection: extracts PR number from pasted GitHub URLs Changes: - Add searchPullRequests tRPC endpoint using gh pr list --search - Update PRLinkCommand to use conditional queries - Add URL parsing to extract PR numbers from GitHub URLs - Show result count and appropriate loading states Fixes the regression from v1.1.7 where PR search could access all PRs via synced database. This approach works without requiring ElectricSQL sync and provides better performance.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded server-side PR search via a new tRPC query that runs Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as PRLinkCommand UI
participant Debounce as Debounce Logic
participant tRPC as tRPC Router
participant CLI as GitHub CLI
User->>UI: Type or paste search query
UI->>Debounce: Raw input
Debounce-->>Debounce: Wait (debounce)
alt GitHub URL detected (same-repo)
Debounce->>Debounce: Extract PR number
Debounce-->>UI: Effective query = PR#
else Normal text query
Debounce-->>UI: Effective query = trimmed query
end
UI->>tRPC: searchPullRequests(projectId, query)
tRPC->>CLI: gh pr list --state all --search "<query>" --limit 100 --json number,title,url,state,isDraft
CLI-->>tRPC: JSON stdout
tRPC->>tRPC: parsePullRequests(stdout)
tRPC-->>UI: [{ prNumber, title, url, state }, ...] or []
UI->>UI: Render results / handle selection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
…arch-server-side-search
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/lib/trpc/routers/projects/projects.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/projects/projects.ts:395">
P3: The PR parsing/filtering logic is duplicated from listPullRequests. Extract a shared helper to keep browse/search results consistent and avoid future drift when the PR shape or state mapping changes.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/lib/trpc/routers/projects/projects.ts`:
- Around line 382-390: The gh PR list invocation currently builds args including
"--search", input.query and "--json" but omits the state flag so it defaults to
open PRs; update the argument array used when calling the GitHub CLI (the args
list that contains "pr","list","--search", input.query,
"--limit","100","--json","number,title,url,state,isDraft") to include the state
flag by inserting the two entries "--state" and "all" so closed and merged PRs
are returned as well.
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx`:
- Around line 47-48: The current code trims the debounced value (debouncedQuery
-> trimmedQuery), which delays the UI's trimmed input and causes stale browse
results/Enter behavior; change this so trimmedQuery is derived directly from the
immediate searchQuery (e.g., const trimmedQuery = searchQuery.trim()) and keep
useDebouncedValue(searchQuery, 300) only for the RPC trigger that fetches
results (the effect or function that uses debouncedQuery); update any places
referencing trimmedQuery for display/Enter handling to use the immediate
trimmedQuery and keep debouncedQuery for calling the search/URL RPC.
- Around line 50-59: The code extracts only the PR number (prNumberFromUrl) and
sets effectiveQuery to that number, which loses the original repo context and
can make a pasted cross-repo URL resolve against the current project; change the
logic in PRLinkCommand so that when a GitHub URL is pasted you parse and retain
owner/repo plus PR number (extract owner and repo from trimmedQuery using the
same regex or a new one), compare the parsed owner/repo to the currently
selected project's repo identifier (e.g. the prop or state that holds the
current project's repo) and if they differ either use the full URL (not just the
number) for lookup or surface a mismatch/reject; ensure prNumberFromUrl remains
usable but set effectiveQuery to include repo context (owner/repo#number or full
URL) only when the parsed repo matches or when you intend to resolve cross-repo
PRs explicitly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5c2cb0ad-313a-4f60-b9d3-4c729c056672
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
apps/desktop/src/lib/trpc/routers/projects/projects.tsapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx
Fix stale UI behavior caused by debouncing the trimmed query value. Previously: - trimmedQuery was derived from debouncedQuery.trim() - This caused 300ms delay before UI decisions (show browse/search mode) - Pressing Enter had stale behavior due to delayed trim Now: - trimmedQuery = searchQuery.trim() (immediate, for UI decisions) - debouncedTrimmed = debouncedQuery.trim() (debounced, for RPC calls) - URL detection and search RPC use debounced value - UI state (enabled flags, display logic) use immediate trim - No more stale behavior on Enter or mode switching
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx:48">
P2: Search mode switches immediately, but data fetching is still debounced, which causes a 300ms empty-results gap after typing. Align the mode toggle with the debounced value (or add an explicit pending-debounce state) to avoid clearing results prematurely.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx (1)
76-83:⚠️ Potential issue | 🟡 MinorBrief "No pull requests found" flash during debounce window.
When the user types,
trimmedQuerybecomes truthy immediately, buteffectiveQuery(derived fromdebouncedTrimmed) remains empty for ~300ms. During this window:
searchPullRequestsquery is disabled (!!effectiveQueryis false)isSearchingisfalse,searchResultsisundefinedpullRequests=[],isLoading=false- UI shows "No pull requests found." instead of "Searching..."
Consider including the debounce-in-progress state in the loading check:
🔧 Proposed fix to show loading during debounce
- const isLoading = trimmedQuery ? isSearching : isLoadingRecent; + const isLoading = trimmedQuery + ? isSearching || trimmedQuery !== debouncedTrimmed + : isLoadingRecent;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx` around lines 76 - 83, The flicker happens because pullRequests and isLoading use trimmedQuery (immediate user input) while the search actually runs against effectiveQuery (debounced), so during the debounce window the UI shows empty results and not-loading; update the logic to base which list to show on effectiveQuery (use effectiveQuery ? searchResults ?? [] : recentPRs ?? [] for pullRequests) and update isLoading to treat the debounce-in-progress as loading (e.g. if trimmedQuery && !effectiveQuery then true, else if effectiveQuery use isSearching, otherwise use isLoadingRecent) so the UI shows "Searching..." during debounce; adjust references to trimmedQuery, debouncedTrimmed/effectiveQuery, searchResults, recentPRs, isSearching and isLoadingRecent accordingly.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx (1)
32-33: UnusedgithubOwnerandrepoNameprops.These props were added to the interface and function signature but are immediately aliased to unused variables (
_githubOwner,_repoName). This appears to be scaffolding for cross-repo URL validation that was never implemented.Either remove these props if the cross-repo validation is out of scope, or implement the validation logic they were intended to support.
🧹 If validation is out of scope, remove the unused props
interface PRLinkCommandProps { open: boolean; onOpenChange: (open: boolean) => void; onSelect: (pr: SelectedPR) => void; projectId: string | null; - githubOwner: string | null; - repoName: string | null; anchorRef: RefObject<HTMLElement | null>; } export function PRLinkCommand({ open, onOpenChange, onSelect, projectId, - githubOwner: _githubOwner, - repoName: _repoName, anchorRef, }: PRLinkCommandProps) {Also applies to: 42-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx` around lines 32 - 33, PRLinkCommand currently declares props githubOwner and repoName which are immediately aliased to unused variables (_githubOwner, _repoName); either remove them or implement the intended cross-repo validation. To fix quickly, remove githubOwner and repoName from the component prop type and from the PRLinkCommand function signature (and delete the unused _githubOwner/_repoName aliases) so the interface and component only include actually used props; alternatively, if you want validation, implement the cross-repo URL check inside PRLinkCommand using githubOwner and repoName and remove the underscore aliases after using them in the validation logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx`:
- Around line 76-83: The flicker happens because pullRequests and isLoading use
trimmedQuery (immediate user input) while the search actually runs against
effectiveQuery (debounced), so during the debounce window the UI shows empty
results and not-loading; update the logic to base which list to show on
effectiveQuery (use effectiveQuery ? searchResults ?? [] : recentPRs ?? [] for
pullRequests) and update isLoading to treat the debounce-in-progress as loading
(e.g. if trimmedQuery && !effectiveQuery then true, else if effectiveQuery use
isSearching, otherwise use isLoadingRecent) so the UI shows "Searching..."
during debounce; adjust references to trimmedQuery,
debouncedTrimmed/effectiveQuery, searchResults, recentPRs, isSearching and
isLoadingRecent accordingly.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx`:
- Around line 32-33: PRLinkCommand currently declares props githubOwner and
repoName which are immediately aliased to unused variables (_githubOwner,
_repoName); either remove them or implement the intended cross-repo validation.
To fix quickly, remove githubOwner and repoName from the component prop type and
from the PRLinkCommand function signature (and delete the unused
_githubOwner/_repoName aliases) so the interface and component only include
actually used props; alternatively, if you want validation, implement the
cross-repo URL check inside PRLinkCommand using githubOwner and repoName and
remove the underscore aliases after using them in the validation logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8e471057-12a7-4604-82ac-53370ba5a254
📒 Files selected for processing (1)
apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/components/PRLinkCommand/PRLinkCommand.tsx
Previously, search mode toggled immediately on trimmedQuery while data fetching was debounced on debouncedTrimmed, causing a 300ms window where the UI showed "No pull requests found" even though results were still loading. Changes: - Add isPendingDebounce state detection (trimmedQuery !== debouncedTrimmed) - Use debouncedTrimmed consistently for all mode decisions and UI display - Include isPendingDebounce in isLoading calculation - Keep trimmedQuery only for pending state detection This ensures the mode doesn't switch until debounced data is ready, eliminating the empty results gap and providing consistent UI behavior.
Extract parsePullRequests() helper to eliminate duplication between listPullRequests and searchPullRequests endpoints. Both now use the same filter/map logic for consistent PR shape and state mapping. Benefits: - Single source of truth for PR parsing - Future changes only need updating in one place - Consistent behavior across browse and search modes - Reduced code by 18 lines Resolves cubic.dev P3 refactoring suggestion
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/desktop/src/lib/trpc/routers/projects/projects.ts (1)
391-399:⚠️ Potential issue | 🟠 MajorSearch should include closed/merged PRs too.
gh pr listsearch is currently called without an explicit state, so closed/merged PRs may be excluded. That can break URL/number lookups for historical PRs.Does `gh pr list --search "<query>"` default to only open pull requests unless `--state` is provided, and should `--state all` be used to include closed and merged PRs?Suggested fix
[ "pr", "list", + "--state", + "all", "--search", input.query, "--limit", "100",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/projects/projects.ts` around lines 391 - 399, The gh CLI invocation building the PR args in projects.ts currently omits an explicit state and therefore may ignore closed/merged PRs; update the args array used for the "gh pr list" command (the array containing "pr","list","--search",input.query,"--limit","100","--json",...) to include the explicit flags "--state","all" so the search returns open, closed and merged PRs (add the two elements in the same args array where the command is constructed).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/lib/trpc/routers/projects/projects.ts`:
- Around line 388-401: The gh CLI call via execWithShellEnv can hang during
debounced typing; add a timeout to the call around the "gh pr list" invocation
so slow/hung calls get aborted. Update the execWithShellEnv call in projects.ts
(the invocation that runs "gh pr list" with args "--search", input.query, ...)
to include a timeout (e.g., 3–5s) or wrap it with a Promise.race/AbortController
that kills the child process on timeout, ensure you propagate a clear timeout
error so the caller can ignore stale results and the modal remains responsive.
- Around line 61-95: The parsePullRequests function currently only checks for
key presence and may pass objects where pr.state and pr.isDraft have wrong
types; tighten the type guard in parsePullRequests to verify typeof pr.number
=== "number", typeof pr.title === "string", typeof pr.url === "string", typeof
pr.state === "string" and typeof pr.isDraft === "boolean" (or accept undefined
and handle it), then map using those assured types; additionally guard the
mapping logic for pr.state.toLowerCase() by providing a safe fallback (e.g.
const state = typeof pr.state === "string" ? pr.state.toLowerCase() : "unknown")
and treat missing/invalid isDraft as false so the computed state expression
(pr.isDraft ? "draft" : ...) cannot throw.
---
Duplicate comments:
In `@apps/desktop/src/lib/trpc/routers/projects/projects.ts`:
- Around line 391-399: The gh CLI invocation building the PR args in projects.ts
currently omits an explicit state and therefore may ignore closed/merged PRs;
update the args array used for the "gh pr list" command (the array containing
"pr","list","--search",input.query,"--limit","100","--json",...) to include the
explicit flags "--state","all" so the search returns open, closed and merged PRs
(add the two elements in the same args array where the command is constructed).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c419a6ab-92b6-48d6-89bf-5ae65fb740d8
📒 Files selected for processing (1)
apps/desktop/src/lib/trpc/routers/projects/projects.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes PR search in the new workspace modal by implementing server-side GitHub search instead of fetching all PRs upfront.
Problem
The new workspace modal had significant PR search issues:
This was a regression from v1.1.7 which used synced database to access all PRs instantly.
Solution
Implemented server-side search using GitHub's search API:
Backend (
projects.ts)searchPullRequeststRPC endpointgh pr list --search "query" --limit 100Frontend (
PRLinkCommand.tsx)Benefits
✅ Fast initial load - Only fetches 30 PRs for browsing
✅ Unlimited search - Can find ANY PR by typing
✅ Better UX - No long delays, instant feedback
✅ URL support - Paste PR URLs to find specific PRs
✅ Smart - Debounced search, only triggers when needed
Testing
Example:
https://github.com/superset-sh/superset/pull/815Summary by cubic
Switch PR search in the new workspace modal to server-side GitHub search for faster loads and full results. Browsing shows recent PRs; typing or pasting a PR URL searches across all PRs with responsive, accurate results.
Bug Fixes
tRPCsearchPullRequestsusinggh pr list --state all --search(up to 100 results).PRLinkCommandto browse recent PRs by default and use debounced (300ms) server-side search; parses PR numbers from GitHub URLs, blocks cross-repo URLs with a clear message, shows result counts, and keeps loading states consistent (no 300ms empty gap; no stale Enter).Refactors
parsePullRequestsfor shared PR parsing acrosslistPullRequestsandsearchPullRequests.PromptGroupnow passesgithubOwnerandrepoNametoPRLinkCommandfor repository validation.Written for commit ae0ceec. Summary will update on new commits.
Summary by CodeRabbit
New Features
Refactor