fix(desktop): simplify GitHub query polling with hover debounce#3125
Merged
Conversation
… of polling Active surfaces (changes-sidebar, workspace-page) still poll every 10s. Hover surfaces (list-item, row, hover-card) now fetch on hover and use staleTime as a debounce — no continuous background polling.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughConsolidates GitHub status stale-time, removes the review-tab flag from query policy options, adds a shared Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Component as WorkspaceListItem/Row
participant Hook as useHoverGitHubStatus
participant TRPC as electronTrpc.workspaces.getGitHubStatus
User->>Component: mouseEnter
Component->>Hook: onMouseEnter()
alt first hover
Hook->>Hook: set hasHovered = true
Hook->>TRPC: start useQuery(workspaceId, policy)
TRPC-->>Hook: githubStatus, dataUpdatedAt
Hook-->>Component: githubStatus, isStale, refetch
else subsequent hover & stale
Hook->>TRPC: refetch()
TRPC-->>Hook: updated githubStatus
Hook-->>Component: githubStatus
else subsequent hover & fresh
Hook->>Hook: schedule timeout to refetch when stale (rgba(0,128,0,0.5))
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/constants.ts (1)
3-3: Consider importing the stale time from the centralized policy module.This constant duplicates
GITHUB_STATUS_STALE_TIME_MSfromgithubQueryPolicy.ts. While co-location is generally preferred, having the same magic number in two places risks drift if one is updated without the other.♻️ Option: Export and reuse the constant
In
githubQueryPolicy.ts:-const GITHUB_STATUS_STALE_TIME_MS = 10_000; +export const GITHUB_STATUS_STALE_TIME_MS = 10_000;In
constants.ts:+import { GITHUB_STATUS_STALE_TIME_MS } from "renderer/lib/githubQueryPolicy"; + export const WORKSPACE_DND_TYPE = "WORKSPACE"; export const MAX_KEYBOARD_SHORTCUT_INDEX = 9; -export const GITHUB_STATUS_STALE_TIME = 10_000; +export const GITHUB_STATUS_STALE_TIME = GITHUB_STATUS_STALE_TIME_MS;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/constants.ts` at line 3, Replace the duplicated magic number by importing and reusing the centralized policy constant: remove or replace GITHUB_STATUS_STALE_TIME in WorkspaceListItem/constants.ts with an import of GITHUB_STATUS_STALE_TIME_MS from githubQueryPolicy (or export the centralized constant from that module if not already exported) and update any usages to reference the imported GITHUB_STATUS_STALE_TIME_MS so both modules share a single source of truth.apps/desktop/src/renderer/lib/githubQueryPolicy/githubQueryPolicy.test.ts (1)
7-59: Consider adding tests for disabled query states.The tests cover happy-path scenarios where queries are enabled. Adding tests for disabled states would improve coverage and document the expected behavior when:
hasWorkspaceId: falseisActive: falsehasActivePullRequest: false🧪 Suggested additional test cases
test("disabled when no workspace id", () => { expect( getGitHubStatusQueryPolicy("changes-sidebar", { hasWorkspaceId: false, isActive: true, }), ).toEqual({ enabled: false, refetchInterval: false, refetchOnWindowFocus: false, staleTime: 10_000, }); }); test("disabled when not active", () => { expect( getGitHubStatusQueryPolicy("workspace-page", { hasWorkspaceId: true, isActive: false, }), ).toEqual({ enabled: false, refetchInterval: false, refetchOnWindowFocus: false, staleTime: 10_000, }); }); test("PR comments disabled without active pull request", () => { expect( getGitHubPRCommentsQueryPolicy({ hasWorkspaceId: true, hasActivePullRequest: false, isActive: true, }), ).toEqual({ enabled: false, refetchInterval: false, refetchOnWindowFocus: false, staleTime: 30_000, }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/lib/githubQueryPolicy/githubQueryPolicy.test.ts` around lines 7 - 59, The tests only cover enabled/happy-path cases; add unit tests for disabled states for getGitHubStatusQueryPolicy and getGitHubPRCommentsQueryPolicy: add tests asserting that getGitHubStatusQueryPolicy returns enabled: false, refetchInterval: false, refetchOnWindowFocus: false, staleTime: 10_000 when hasWorkspaceId: false (e.g., surface "changes-sidebar") and when isActive: false (e.g., surface "workspace-page"), and add a test asserting getGitHubPRCommentsQueryPolicy returns enabled: false, refetchInterval: false, refetchOnWindowFocus: false, staleTime: 30_000 when hasActivePullRequest: false (with hasWorkspaceId: true/isActive: true); use the existing test file and naming patterns to add these three tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/desktop/src/renderer/lib/githubQueryPolicy/githubQueryPolicy.test.ts`:
- Around line 7-59: The tests only cover enabled/happy-path cases; add unit
tests for disabled states for getGitHubStatusQueryPolicy and
getGitHubPRCommentsQueryPolicy: add tests asserting that
getGitHubStatusQueryPolicy returns enabled: false, refetchInterval: false,
refetchOnWindowFocus: false, staleTime: 10_000 when hasWorkspaceId: false (e.g.,
surface "changes-sidebar") and when isActive: false (e.g., surface
"workspace-page"), and add a test asserting getGitHubPRCommentsQueryPolicy
returns enabled: false, refetchInterval: false, refetchOnWindowFocus: false,
staleTime: 30_000 when hasActivePullRequest: false (with hasWorkspaceId:
true/isActive: true); use the existing test file and naming patterns to add
these three tests.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/constants.ts`:
- Line 3: Replace the duplicated magic number by importing and reusing the
centralized policy constant: remove or replace GITHUB_STATUS_STALE_TIME in
WorkspaceListItem/constants.ts with an import of GITHUB_STATUS_STALE_TIME_MS
from githubQueryPolicy (or export the centralized constant from that module if
not already exported) and update any usages to reference the imported
GITHUB_STATUS_STALE_TIME_MS so both modules share a single source of truth.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b383fede-77fe-41ef-b9e9-620eefee36bf
📒 Files selected for processing (6)
apps/desktop/src/renderer/lib/githubQueryPolicy/githubQueryPolicy.test.tsapps/desktop/src/renderer/lib/githubQueryPolicy/githubQueryPolicy.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/constants.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/WorkspaceRow.tsx
💤 Files with no reviewable changes (1)
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsx
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 6 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/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx:235">
P2: Manual `refetchGithubStatus()` is not gated by workspace type, so branch workspaces can still fetch GitHub status on hover and bypass the query policy’s `type === "worktree"` restriction.</violation>
</file>
<file name="apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/WorkspaceRow.tsx">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/WorkspaceRow.tsx:92">
P2: Guard the manual hover refetch with the same worktree/workspaceId conditions as the query policy; otherwise disabled rows can still trigger GitHub status requests on hover.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…red fetching Moves the hover debounce + queued refetch logic into a shared hook so WorkspaceListItem and WorkspaceRow are simple consumers.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/renderer/lib/githubQueryPolicy/useHoverGitHubStatus.ts`:
- Around line 45-58: In useHoverGitHubStatus's onMouseEnter handler, when taking
the isStale branch and calling refetch(), clear any scheduled timeout stored in
pendingRefetchRef.current first to avoid a duplicate refetch later;
specifically, in the isStale branch of onMouseEnter
clearTimeout(pendingRefetchRef.current) (if truthy) and set
pendingRefetchRef.current = null before invoking refetch(), so the pending timer
created in the later branch cannot fire and trigger another refetch.
🪄 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: da1f8fa9-2261-4132-bc7a-14d847d3fd83
📒 Files selected for processing (5)
apps/desktop/src/renderer/lib/githubQueryPolicy/githubQueryPolicy.tsapps/desktop/src/renderer/lib/githubQueryPolicy/index.tsapps/desktop/src/renderer/lib/githubQueryPolicy/useHoverGitHubStatus.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspacesListView/WorkspaceRow/WorkspaceRow.tsx
✅ Files skipped from review due to trivial changes (2)
- apps/desktop/src/renderer/lib/githubQueryPolicy/index.ts
- apps/desktop/src/renderer/lib/githubQueryPolicy/githubQueryPolicy.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx
Contributor
There was a problem hiding this comment.
2 issues found across 5 files (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/lib/githubQueryPolicy/useHoverGitHubStatus.ts">
<violation number="1" location="apps/desktop/src/renderer/lib/githubQueryPolicy/useHoverGitHubStatus.ts:48">
P2: Clear pending timeout before triggering immediate refetch in the `isStale` branch. If a refetch was scheduled during a previous hover (in the `else if (!pendingRefetchRef.current)` branch), it remains active when this branch fires, resulting in a duplicate `refetch()` call when the timeout subsequently executes.</violation>
<violation number="2" location="apps/desktop/src/renderer/lib/githubQueryPolicy/useHoverGitHubStatus.ts:49">
P2: `onMouseEnter` can call `refetch()` for non-worktree items, bypassing the `isWorktree` gate and triggering unintended GitHub status requests.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
MocA-Love
added a commit
to MocA-Love/superset
that referenced
this pull request
Apr 5, 2026
githubQueryPolicy.ts と github-sync-service.ts に、upstream (be22b46, superset-sh#3125) との設計差異と将来のマージ時の判断指針を明記。
MocA-Love
added a commit
to MocA-Love/superset
that referenced
this pull request
Apr 5, 2026
cherry-pick方式で内容を取り込み済みの14コミットをgit履歴上もマージ済みにする。 取り込み済み (cherry-pick / 手動移植): - be22b46 superset-sh#3125 — スキップ (下記参照) - 88bc7fb superset-sh#3127 — Revert DA1 ✓ - 92d0ff9 superset-sh#3054 — DA1 fix ✓ - c48450e superset-sh#3093 — file viewer pane fix ✓ - fffa8db superset-sh#3128 — version 1.4.7 ✓ - 589a7c7 superset-sh#3136 — fuzzy scorer (ハイブリッド方式) ✓ - ceb8c81 superset-sh#3150 — Electron 40.8.5 ✓ - 8922b94 superset-sh#3137 — terminalId分離 ✓ - c7508e5 superset-sh#3152 — GitHub無料化 ✓ - 2b91f11 superset-sh#3155 — v2 terminal theme ✓ - b8b11af superset-sh#3154 — TUI dimension fix ✓ - 7599ace superset-sh#3149 — v2 sidebar file tree (手動統合) ✓ - 4d7c612 superset-sh#3174 — DnD重複削除 ✓ - 864977d superset-sh#3157 — Host Service分離 ✓ 意図的にスキップ: - be22b46 superset-sh#3125 (GitHub polling簡素化) フォーク独自のGitHubSyncService (バックエンド集中ポーリング) と 設計が異なるため不採用。upstreamはフロントエンドhover駆動、フォークは バックエンドキャッシュウォーマー方式。詳細は githubQueryPolicy.ts と github-sync-service.ts のFORK NOTEを参照。 ゴースト・マージ復元 (revert 134cfd5 で消失した内容): - 538f306 superset-sh#3120 — Patch vuln ✓ - 1588d20 superset-sh#3108 — terminal lifecycle分離 ✓ - 59426f6 superset-sh#3122 — file tree + FilePane + Alert refactor (手動統合) ✓ - 10d9a5d superset-sh#3097 — tiptap line-height ✓ - 337a9ae superset-sh#3121 — Codex hooks削除 ✓
3 tasks
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
1 participant
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
isReviewTabActiveisReviewTabActivefrom both policy functionsTest plan
Summary by cubic
Simplifies GitHub polling: active surfaces poll every 10s; hover surfaces fetch on hover with a 10s debounce and no background polling. Removes review-tab gating so PR status and comments stay fresh; comments poll every 30s when a PR is active.
Refactors
useHoverGitHubStatusfor hover-triggered fetching with a 10s debounce; used inWorkspaceListItemandWorkspaceRow.GITHUB_STATUS_STALE_TIME_MS); removed review-tab gating from policies and tests.Bug Fixes
useHoverGitHubStatusaccepts a nullworkspaceIdto avoid invalid queries during initial renders.Written for commit 7a9f1b2. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests