Repository navigation
Conversation
|
@mverrilli is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthrough
ChangesFocus-Aware GitHub PR Polling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (15 passed)
✨ Finishing Touches🧪 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 |
Greptile SummaryThis PR addresses a GitHub API hammering issue (#3136) by introducing differentiated poll cadences: no-PR branches back off to 60s, non-GitHub repos to 5min, and open PRs relax from 10s to 15s when focused (60s unfocused). Rescheduling on
Confidence Score: 5/5Safe to merge — the focus-change reschedule is narrowly scoped to the newly-focused key and only shortens existing deadlines, jitter is applied uniformly, and the new outcome dictionary is correctly added to all four eviction paths. The cadence refactor is well-contained: outcome classification is now a single pure function, the focus-change hook avoids the previously-reported thundering-herd and over-reset problems, and all state cleanup sites were updated consistently. The only finding is a one-word stale cadence value in the class-level doc comment — a documentation accuracy issue with no runtime impact. The class-level doc comment in Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant TM as TabManager (selectedTabId.didSet)
participant PPS as PullRequestPollService
participant Host as SidebarGitHosting
participant Timer as Poll Timer
TM->>PPS: rescheduleWorkspacePullRequestPollsForFocusChange()
PPS->>Host: selectedFocusedPanel()
Host-->>PPS: (workspaceId, panelId)
PPS->>PPS: lookup lastOutcomeByKey[key]
PPS->>PPS: workspacePullRequestPollOutcomeIsFocusSensitive(outcome)?
alt outcome is focus-sensitive (openPR / transientFailure w/o terminal)
PPS->>PPS: nextWorkspacePullRequestPollAt(now, outcome, isFocused: true)
PPS->>PPS: "if target < current deadline → update nextPollAt"
PPS->>Timer: updateWorkspacePullRequestPollTimer()
else outcome is focus-insensitive (noPR / terminal / unsupported)
PPS-->>TM: return (no change)
end
Timer->>PPS: refreshTrackedWorkspacePullRequestsIfNeeded(timer)
PPS->>PPS: applyWorkspacePullRequestRefreshResults(...)
PPS->>PPS: classifyWorkspacePullRequestPollOutcome(key, resolution)
PPS->>PPS: nextWorkspacePullRequestPollAt(now, outcome, isFocused)
PPS->>PPS: "lastOutcomeByKey[key] = outcome"
PPS->>Timer: updateWorkspacePullRequestPollTimer()
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant TM as TabManager (selectedTabId.didSet)
participant PPS as PullRequestPollService
participant Host as SidebarGitHosting
participant Timer as Poll Timer
TM->>PPS: rescheduleWorkspacePullRequestPollsForFocusChange()
PPS->>Host: selectedFocusedPanel()
Host-->>PPS: (workspaceId, panelId)
PPS->>PPS: lookup lastOutcomeByKey[key]
PPS->>PPS: workspacePullRequestPollOutcomeIsFocusSensitive(outcome)?
alt outcome is focus-sensitive (openPR / transientFailure w/o terminal)
PPS->>PPS: nextWorkspacePullRequestPollAt(now, outcome, isFocused: true)
PPS->>PPS: "if target < current deadline → update nextPollAt"
PPS->>Timer: updateWorkspacePullRequestPollTimer()
else outcome is focus-insensitive (noPR / terminal / unsupported)
PPS-->>TM: return (no change)
end
Timer->>PPS: refreshTrackedWorkspacePullRequestsIfNeeded(timer)
PPS->>PPS: applyWorkspacePullRequestRefreshResults(...)
PPS->>PPS: classifyWorkspacePullRequestPollOutcome(key, resolution)
PPS->>PPS: nextWorkspacePullRequestPollAt(now, outcome, isFocused)
PPS->>PPS: "lastOutcomeByKey[key] = outcome"
PPS->>Timer: updateWorkspacePullRequestPollTimer()
Reviews (4): Last reviewed commit: "Back off PR refresh cadence for branches..." | Re-trigger Greptile |
0c2ff1d to
dee4bce
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/TabManager.swift`:
- Around line 2276-2283: The loop is recomputing a fresh jittered target for
every key on each tab switch which re-jitters non-focused outcomes; change the
logic in the block iterating workspacePullRequestLastOutcomeByKey (and the
similar block at 2298-2324) to skip recalculating when a non-focused outcome
(e.g., .noPullRequest or .unsupportedRepository) already has an entry in
workspacePullRequestNextPollAtByKey: retrieve the existing target first, and
only call Self.nextWorkspacePullRequestPollAt(from:outcome:isFocused:) to set a
new target when there is no existing target, the outcome has changed, or the
focus state transitioned to focused (isSelectedFocusedPanel returned true);
reference workspacePullRequestNextPollAtByKey,
workspacePullRequestLastOutcomeByKey,
isSelectedFocusedPanel(workspace:panelId:), and
Self.nextWorkspacePullRequestPollAt to locate the code to modify.
- Around line 2310-2315: The transient-failure branch currently sets base to
backgroundPollInterval when unfocused, which removes the dedicated transient
retry cadence; in the case .transientFailure(let hadTerminalState) block, change
the logic so that when hadTerminalState is false you always set base =
workspacePullRequestTransientFailurePollInterval (ignoring isFocused) and only
use workspacePullRequestTerminalStateSweepInterval when hadTerminalState is
true, keeping backgroundPollInterval out of this branch.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 235ef7d4-f3a7-4dad-be77-c4f51ab8049a
📒 Files selected for processing (1)
Sources/TabManager.swift
dee4bce to
7a43b2d
Compare
There was a problem hiding this comment.
1 issue found across 1 file
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
7a43b2d to
a4127a8
Compare
A branch with no PR was being re-polled at the same fast cadence as an open PR (every 10s when focused), hammering the GitHub API. Branches without a PR rarely gain one without a local action (push, gh pr create), and those fire event-driven refreshes that bypass the periodic cadence — so back off to 60s for no-PR branches and 5min for non-GitHub repos. Focused open-PR cadence relaxes from 10s to 15s. Additional cleanup folded in: - Classify refresh outcomes in one place (classifyWorkspacePullRequestPollOutcome) rather than splitting the mapping across two early-return guards plus a tail switch. Adding a new Resolution case now touches one site. - Reschedule pending polls when the host's selection changes. The focused/unfocused cadence asymmetry took effect only on the NEXT refresh; now focus changes recompute target nextPollAt immediately. The hook only shortens deadlines and only targets the newly-focused key (not focus-insensitive outcomes), so no-PR/unsupported/terminal/transient-with-terminal cadences aren't disturbed. - Centralize jitter inside nextWorkspacePullRequestPollAt so all cadence paths spread load, including terminal-sweep transitions (previously un-jittered, prone to thundering-herd on bulk merges). - Keep transient-failure retries on a dedicated 10s constant so they don't ride the (now slower) open-PR cadence. - isSelectedFocusedPanel reads selectedWorkspace once instead of twice. The PR-polling subsystem lives in Packages/CmuxSidebarGit since the recent extraction (manaflow-ai#5907); changes target that package plus the host-side wiring and the SidebarGitHosting/PullRequestProbing protocol seams.
a4127a8 to
ce64373
Compare
Summary
gh pr create), and those fire event-driven refreshes that bypass the periodic cadence — so back off to 60s for no-PR branches and 5min for non-GitHub repos. Focused open-PR cadence relaxes from 10s to 15s.classifyWorkspacePullRequestPollOutcome) rather than splitting the mapping across two early-return guards plus a tail switch. Adding a newResolutioncase now touches one site.selectedTabIdchange. The focused/unfocused cadence asymmetry took effect only on the next refresh; now focus changes recompute targetnextPollAtimmediately, so Cmd-Tab into an open-PR workspace honors the 15s focused cadence without waiting out the prior 60s. (Within-tab panel focus changes are still not hooked; noted in the doc comment.)nextWorkspacePullRequestPollAtso all cadence paths spread load, including terminal-sweep transitions (previously un-jittered, prone to thundering-herd on bulk merges).isSelectedFocusedPanelreadsselectedWorkspaceonce instead of twice.Fixes #3136.
Test plan
./scripts/reload.sh --tag fix-pr-poll-3136builds cleanlygh pr createin the integrated terminal of a no-PR workspace: PR badge appears via the event-driven refresh path (not the 60s periodic)Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Summary by cubic
Backed off PR refresh polling to reduce GitHub API load and make focus changes apply immediately. No-PR branches poll every 60s, non-GitHub repos every 5m, and open PRs every 15s when focused (60s unfocused). Fixes #3136.
Bug Fixes
Refactors
classifyWorkspacePullRequestPollOutcomeandnextWorkspacePullRequestPollAt; addWorkspacePullRequestPollOutcome.selectedFocusedPanel()and invoke reschedule on workspace selection.Written for commit ce64373. Summary will update on new commits.
Summary by CodeRabbit
Performance Improvements
Bug Fixes