fix(backend): add retry logic and backoff for secondary rate limits in commit_files - #100
Conversation
…per-file commit delays
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughGitHub file operations now retry matching rate-limit 403 responses and pace multi-file commits. The direct-commit route tracks successful file IDs, updates staged-file state, persists commit logs, sends monitoring notifications, and reports the committed count. ChangesCommit flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@backend/github_service.py`:
- Around line 180-184: The async commit_direct route synchronously invokes
commit_files, whose time.sleep and retry delays block the FastAPI event loop.
Update commit_direct to run the synchronous service invocation through
fastapi.concurrency.run_in_threadpool, preserving the existing commit_files
behavior and route response handling.
🪄 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: CHILL
Plan: Pro
Run ID: 1d3622c7-33c4-4981-8bdb-a1a2548bef15
📒 Files selected for processing (2)
backend/github_service.pybackend/routes/staged_files.py
| for idx, staged in enumerate(staged_files): | ||
| if idx > 0: | ||
| # Proactively sleep 1s between file commits per GitHub's integrator best practices | ||
| time.sleep(1) | ||
|
|
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Blocking call inside an async route.
The addition of time.sleep(1) (and the potential 60s sleep in _execute_with_retry) will block the thread. Since commit_files is called synchronously from the async def commit_direct route (in backend/routes/staged_files.py), this will block the FastAPI event loop, preventing the server from handling other concurrent requests on this worker.
Consider wrapping this synchronous service invocation in a thread pool using fastapi.concurrency.run_in_threadpool from the route, or changing the route definition to a synchronous def so FastAPI automatically runs it in a thread pool.
🤖 Prompt for 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.
In `@backend/github_service.py` around lines 180 - 184, The async commit_direct
route synchronously invokes commit_files, whose time.sleep and retry delays
block the FastAPI event loop. Update commit_direct to run the synchronous
service invocation through fastapi.concurrency.run_in_threadpool, preserving the
existing commit_files behavior and route response handling.
…ommit_direct Avoid blocking the FastAPI async event loop during sequential commit sleeps and retry delays.
📋 Pull Request Description
To support the vision of maximizing GitHub contributions ("green dots"), GitPhone commits files sequentially (1 commit per file). However, executing rapid sequential mutating requests (POST/PUT/DELETE) can trigger GitHub's Secondary Rate Limit (Abuse) mechanisms during high-volume commits, returning
403 Forbiddenerrors and dropping remaining staged files.This PR adds resilience to
commit_filesinbackend/github_service.py:time.sleep(1)) between processing sequential file commits, adhering strictly to GitHub's integrator best practices._execute_with_retryhelper method to catchGithubException(status 403), parse theretry-afterheader if present, and retry up to 3 times before failing.repo.create_file,repo.update_file, andrepo.delete_filecalls with_execute_with_retry.🔗 Related Issue
Closes #92
🏷️ PR Type
✅ Checklist
mainECSoC26label to this PRruff check backend/locally and it passesnpx tsc --noEmitlocally and it passes.envfiles, secrets, or tokens🧪 How to Test
ruff check backend/github_service.pypasses cleanly.📸 Screenshots (if UI changes)
N/A — backend resilience fix only.
Summary by CodeRabbit
Bug Fixes
New Features