feat(isl): GitHub API optimization — caching, dedup, rate limit monitoring#1240
Closed
sontiO wants to merge 134 commits intofacebook:mainfrom
Closed
feat(isl): GitHub API optimization — caching, dedup, rate limit monitoring#1240sontiO wants to merge 134 commits intofacebook:mainfrom
sontiO wants to merge 134 commits intofacebook:mainfrom
Conversation
…ntation - Add Graphite stack comment parser to detect and parse PR stacks managed by Graphite, extracting PR numbers, current position, and base branch - Replace Netlify auth provider with custom Cloudflare Pages Functions for GitHub OAuth flow, reading token from URL hash after redirect - Add support for reviews.qlax.dev hostname in login dialog - Update GraphQL query to include comment body for Graphite parsing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Replace GitHub Pages deployment with Cloudflare Pages using wrangler-action - Trigger on push to main when reviewstack files change (instead of schedule) - Keep existing build process with codegen, eslint, and tests 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove custom build container that doesn't exist in fork - Use actions/setup-node@v4 with Node 20 and yarn caching - Update actions/checkout to v4 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Detect generated files via .gitattributes linguist-generated patterns, @generated/@partially-generated tags in file content, and default patterns (lock files, .min.js, etc.) - Show "X sig" counter next to +/- counts with explanatory tooltip - Label generated files in diff header with "Generated" badge - Auto-collapse generated files in the diff view - Fix: Move fileGeneratedStatus into waitForAll preload block to prevent re-rendering issues (per TODO T122242329) - Fix: Calculate SLOC using per-file diff lines instead of entire file content for accurate significant line counts Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary: Test Plan:
Add `sl pr get` CLI command to pull entire PR stacks from GitHub by parsing the Sapling stack footer in PR descriptions. The command supports --goto to checkout the target commit and --downstack to only fetch from trunk to the target PR. Add ISL PR Dashboard in the left drawer showing all open PRs from the last 30 days, grouped by stacks. Each stack shows PRs in order with one-click "Pull" buttons to fetch stacks via `sl pr get`. Features: - sl pr get <PR> - Import full stack based on PR body footer - sl pr get <PR> --goto - Import and checkout target - sl pr get <PR> --downstack - Only fetch trunk→target - PR Dashboard shows all open PRs grouped by stacks - Keyboard shortcut Cmd+, to toggle left drawer Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add a new config option `github.submit-to-upstream` (default: true) that controls whether `sl pr submit` creates PRs against the upstream repository when working from a fork, or against the fork itself. When set to false, PRs will be created on your fork instead of the upstream repository. This is useful when you want to create PRs within your own fork for testing or other purposes. Usage: [github] submit-to-upstream = false Or via command line: sl --config github.submit-to-upstream=false pr submit Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add author { login, avatarUrl } to YourPullRequestsQuery and YourPullRequestsWithoutMergeQueueQuery
- Add viewer { login } to get current authenticated user
- Pass author info through githubCodeReviewProvider to DiffSummary
- Update CodeReviewProvider interface to pass currentUser in onChangeDiffSummaries callback
- Add currentGitHubUser atom to track the authenticated user on the client
- Add mainAuthor and mainAuthorAvatarUrl to PRStack type
- Add isExternalCommitByDiffId atom to detect commits from other authors
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
… file click Collapsible stacks: - Add collapse/expand button to StackActions for stacks with children - Add CollapseAllStacksButton to TopBar for bulk collapse/expand - Filter collapsed commits from CommitTreeList view - Store collapsed state in localStorage for persistence External commit styling: - Add useLineColor prop to RenderDag for custom line colors - Show external commits (from other authors) with gray line color PR Dashboard enhancements: - Add hide/show functionality for stacks with localStorage persistence - Add custom labels with double-click to edit - Display author avatar on stack cards - Add "View changes" button on PR rows to open ComparisonView - Style external stacks with distinct border and background ComparisonView file click: - Add scrollToFile support to ComparisonView - Click on changed file opens ComparisonView and scrolls to that file - Falls back to platform.openFile when no comparison available Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Fork of Sapling ISL focused on improving stacked PR workflow UX for a small team.
- Add drawerPadding (16px) for drawer internal padding - Add sectionGap (12px) for content section spacing - Add itemPadding (8px) for compact list items - Add prominentPadding (20px) for middle column emphasis - Add columnGap (1px) for subtle column separation
- Add graphiteColors StyleX vars in tokens.stylex.ts - Add Graphite CSS custom properties in themeDarkVariables.css - Navy background: #1a1f36 - Soft blue accent: #4a90e2 - Text colors: primary #e8eaed, secondary #9aa0a6 - Border colors: rgba white at 10% and 5% opacity - Hover/selected states with subtle backgrounds
- Add DETAILS_PANEL_BREAKPOINT (1200px) and STACK_PANEL_BREAKPOINT (800px) - Add shouldAutoCollapseDrawers derived atom - Add autoCollapsedState atom to track auto vs manual collapse - Remove legacy window.resize-based auto-close logic Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add CSS custom properties for layout spacing - --drawer-padding (16px) for side drawer content - --section-gap (12px) for content sections - --item-padding (8px) for compact items - --prominent-padding (20px) for middle column emphasis - Apply prominent padding to .drawer-main-content (middle column) - Apply drawer padding to side drawer content areas - Update drawer label padding to use item-padding
- Set deep navy background (var(--graphite-bg)) on main container - Apply unified background across all three columns - Add subtle borders for section separation (rgba white at 10%) - Update resize handle to use graphite border and accent colors - Add hover glow effect to drawer labels - Use graphite text color for primary text
- Add useAutoCollapseDrawers hook with responsive collapse logic - Auto-collapse right drawer below 1200px, left below 800px - Auto-expand drawers when window widens (if auto-collapsed) - Manual collapse/expand clears auto-collapsed flag - Integrate hook into Drawers component Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add isOriginMain() helper function to detect main branch commits - Pass isOriginMain flag to Commit component - Detects origin/main, origin/master, remote/main, remote/master bookmarks
- Create MainBranchSection with go-to-main functionality - Add imports for PullOperation, GotoOperation, dagWithPreviews - Integrate MainBranchSection between header and content - Display sync status (updates available / you are here / up to date) - Show loading state during operation
- Add isOriginMain prop to Commit component - Apply origin-main-commit CSS class when commit is main branch - Render origin-main-badge with git-branch icon and 'main' text - Update memo comparison to include isOriginMain for proper re-rendering
- Add origin-main-commit class with subtle left border using Graphite accent color - Add subtle background gradient for visual prominence - Style origin-main-badge with Graphite color scheme - Maintain border color when commit is both origin/main and head commit
- Add sticky positioning to main-branch-section (stays at top on scroll) - Style main-branch-info, main-branch-name, main-branch-status - Add disabled state styling for go-to-main button - Change pr-dashboard overflow from hidden to auto (enables sticky) - Remove overflow from pr-dashboard-content (parent handles scrolling) - Fix eslint issues (add curly braces, suppress unused var warning)
- Import GotoOperation and required atoms - Add checkout handler using GotoOperation with PR head hash - Track current commit and inline progress state - Add loading indicator during checkout - Prevent checkout when already on commit - Add stopPropagation to view changes button - Apply CSS classes for visual feedback
- Get top PR head hash from stack - Add handleStackCheckout with smart child click filtering - Track current stack state and inline progress - Add CSS classes for clickable header and current state - Header becomes clickable except when on current stack - Child elements (buttons, title) don't trigger checkout
- Add pr-row-clickable with pointer cursor and hover darken - Add pr-row-current with accent border and background - Add pr-row-loading with opacity and disabled pointer events - Add stack-card-header-clickable with hover state - Add stack-card-current with accent border on left - Add stack-card-loading for checkout in progress - Use Graphite accent color for current indicators
- Add curly braces around early returns in handleCheckout - Add curly braces around early returns in handleStackCheckout - Fixes eslint curly rule violations
- Hook watches selectedCommits atom for changes - When exactly one commit is selected, scrolls to center it in view - Uses 100ms timeout to ensure DOM has rendered before scrolling - Smooth scroll animation with behavior: 'smooth' - Cleanup timer on effect teardown Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Export avatarUrl atom for use in CommitAvatar - Add AVATAR_COLORS palette with 12 distinct colors - Implement hashStringToColor for consistent author colors - Create InitialsAvatar with colored circle fallback - Create CommitAvatar that shows photo or initials - Avatar size defaults to 20px
- 3px left border in graphite-accent color (#4a90e2) - Negative margin prevents layout shift when selecting/deselecting - Subtle hover state (rgba(255, 255, 255, 0.03)) on non-selected commits - Consistent with VS Code file tree selection pattern Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Import CommitAvatar in Commit.tsx - Render avatar before commit title for non-public commits - Add CSS styling for commit-author-avatar and avatar-initials - Avatar sized at 20px, circular, and doesn't shrink
- Add lineCount style with highlight foreground color - Use tabular-nums for consistent number spacing - Add TODO for future +/- format when backend supports it
…b 502s The combined PR query was triggering HTTP 502 Bad Gateway errors from GitHub's GraphQL API because mergeable, mergeStateStatus, and viewerCanMergeAsAdmin require per-PR merge computation — too expensive for 100-200 PRs in bulk. These fields are now lazy-loaded per-PR when the review modal opens. Also removes merge queue support and CI check context details from the bulk query. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When creating a worktree with `sl wt add`, large directories like node_modules were copied synchronously, blocking the command for 30+ seconds. This made ISL's UI unresponsive during worktree creation. Now directory copies (copydirs) spawn a background `cp` process via subprocess.Popen instead of waiting. Small config files (.env, etc.) still copy synchronously since they're fast. The worktree is fully usable immediately — node_modules appears once the background copy finishes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…view mode Add a comment sidebar panel that auto-opens when a PR has existing review comments. Comments are grouped by file with collapsible sections, reply and resolve/unresolve support. Also adds per-file comment sections below each file diff showing relevant comments. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add click-and-drag on diff line numbers to select a range of lines for commenting. Visual highlighting during drag, range display in comment input and pending comments, and proper startLine/line mapping for GitHub's GraphQL API. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
fix(worktree): copy directories in background to avoid blocking UI
…-6.14.0 chore(deps): bump ajv from 6.12.6 to 6.14.0 in /addons
…ib/hono-4.12.0 chore(deps): bump hono from 4.11.7 to 4.12.0 in /eden/contrib
…-6.14.2 chore(deps): bump qs from 6.14.0 to 6.14.2 in /website
…bpack-5.105.0 chore(deps): bump webpack from 5.96.1 to 5.105.0 in /website
…ib/webpack-5.105.0 chore(deps): bump webpack from 5.76.0 to 5.105.0 in /eden/contrib
feat(isl): add file list overview to PR review mode
feat(isl): add comment sidebar and per-file comment sections to PR review mode
feat(isl): add multi-line range comment selection to PR review mode
Add comprehensive PR review enhancements to ISL: - Comment sidebar with tabbed Files/Comments views, collapsible file tree navigation, comment posting, editing, threaded replies, and thread resolution - File tree view replacing flat file list, with collapsible directories and click-to-navigate - GraphQL mutations for adding issue comments and editing comments (both issue and review) - Merge status cards with distinct visual styles for conflicts, draft, and behind states - Sidebar positioned on left with localStorage persistence for open/close state Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
When submitting PR reviews (approve/request changes), optional parameters like 'body' and 'threads' are set to undefined when not provided. The queryGraphQL function iterates all variable entries including undefined ones, but the type switch only handles boolean/number/string, causing: 'unexpected type: undefined for body: undefined' Skip undefined values since they represent optional GraphQL variables that should simply be omitted from the gh CLI args.
The queryGraphQL function only handled boolean, number, and string variable types. Submitting a PR review with inline comment threads passes an array of DraftPullRequestReviewThread objects, which has typeof 'object' and hit the default throw. Serialize objects/arrays as JSON via the gh CLI's -F flag, which parses non-string values including JSON arrays and objects.
…ails, and toast - Fix PR stack column to show approved/changes_requested status by falling back to latestReviews when reviewDecision is null (repos without branch protection) - Add publishedAt field to GitHub GraphQL latestReviews query - Show reviewer names and relative timestamps in review status banner (e.g. "Approved by user1, user2 · 3 min ago") - Make review submission toast more prominent with colored accents (green gradient for approvals, amber for changes requested) - Add className support to toast system for styled variants - Update AGENTS.md with complete ISL rebuild + deploy instructions
…haustion Increase focused poll interval from 30 seconds to 2 minutes. The previous 30-second interval caused ~120 API requests/hour, exhausting GitHub's 5000/hour GraphQL rate limit in ~40 minutes of active use. At 2 minutes, polling uses ~30 requests/hour, making rate limit exhaustion practically impossible. User-triggered refreshes (after review submission, merge, etc.) still fire immediately.
When PRs are stacked via branch targeting (baseRefName = "prNNNN") but lack Sapling footer metadata, ISL previously showed them as orphan singles. This adds branch-chain detection that: - Passes baseRefName through from GitHub GraphQL to GitHubDiffSummary - After footer-based grouping, walks parent→child relationships via baseRefName matching to extend existing stacks or form new ones - Prioritizes footer-based stacks to prevent duplicate PR entries - Collects all unique authors per stack for multi-author avatar display - Updates "show only mine" filter to match any author in multi-author stacks Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
fix(isl): detect branch-chained PR stacks without Sapling footers
Use allDiffs.value instead of allDiffs in useMemo dependency arrays. The Loadable wrapper object changes identity on every render, causing the memoized values to recompute infinitely. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Wraps PR summaries (5min TTL), comments (10min TTL), and merge state (2min TTL) with cache. User-triggered refresh bypasses cache. Expected ~60-70% reduction in API calls. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reset lastComments state when diffId changes so stale comments from the previous PR don't briefly render while the new PR's comments are loading. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Clears all cached comments after reply, add, edit, resolve, and unresolve mutations so the next fetch returns fresh data. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Hi @sontiO! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
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
fetchAllPRDatarequestsrateLimitfield to CombinedPRQuery GraphQL for monitoring, with low-rate warningsTest Plan
🤖 Generated with Claude Code