diff --git a/examples/github_actions/README.md b/examples/github_actions/README.md index 272f788..dccf654 100644 --- a/examples/github_actions/README.md +++ b/examples/github_actions/README.md @@ -91,6 +91,22 @@ Use the `--rule` flag to pass a custom rules JSON file: run: ocr review --rule ./my-rules.json --from origin/${{ github.base_ref }} --to origin/${{ github.head_ref }} ``` +### Adjust retry and delay settings + +When posting review comments individually (fallback mode), the workflow includes rate-limit handling with exponential backoff. The retry strategy follows GitHub's documented guidance for REST API rate limits — see [Rate limits for the REST API](https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api?apiVersion=2026-03-10) for details on primary/secondary rate limits and recommended retry behavior. You can configure the retry and delay behavior via **repository variables** (Settings → Secrets and variables → Actions → Variables): + +| Variable | Default | Description | +|----------|---------|-------------| +| `OCR_RETRY_BASE_DELAY` | `60000` | Base delay (ms) for exponential backoff when no retry header is present (per GitHub's "at least one minute" recommendation for secondary limits) | +| `OCR_RETRY_MAX_DELAY` | `300000` | Maximum delay (ms) cap applied to every computed wait, including retry-after and x-ratelimit-reset, so a far-future reset cannot stall the job past its timeout | +| `OCR_MAX_RETRIES` | `3` | Maximum retry attempts per comment when rate-limited | +| `OCR_SUCCESS_DELAY` | `2000` | Delay (ms) after a successful comment post to pace subsequent requests | +| `OCR_FAILURE_DELAY` | `1000` | Delay (ms) after a non-rate-limit failure to pace subsequent requests | +| `OCR_LOW_REMAINING_THRESHOLD` | `3` | When x-ratelimit-remaining is at or below this value, proactively increase request spacing to avoid hitting the limit | +| `OCR_LOW_REMAINING_SPACING` | `10000` | Request spacing (ms) used when remaining quota is low | + +These variables are optional — if not configured, sensible defaults are used. Consider increasing delays for repositories with many concurrent workflows or large PRs that generate numerous review comments. + ### Limit concurrency Adjust the `--concurrency` flag for large PRs to control the number of concurrent LLM requests: diff --git a/examples/github_actions/ocr-review.yml b/examples/github_actions/ocr-review.yml index ddc50c8..e39cc85 100644 --- a/examples/github_actions/ocr-review.yml +++ b/examples/github_actions/ocr-review.yml @@ -13,8 +13,34 @@ # # Optional secrets: # OCR_LLM_MODEL - Model name (default: gpt-4o) +# OCR_LLM_USE_ANTHROPIC - Set to 'true' if using Anthropic Claude models +# +# Optional variables (for retry/delay tuning): +# The retry strategy follows GitHub's documented guidance for REST API rate limits: +# https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api +# - Primary rate limit exhausted (x-ratelimit-remaining=0): wait until x-ratelimit-reset. +# - Secondary rate limit with a retry-after header: wait exactly that long. +# - Secondary rate limit with NO header: wait at least one minute, then use +# exponential backoff on continued failures. +# +# OCR_RETRY_BASE_DELAY - Base delay (ms) for exponential backoff when no retry +# header is present (default: 60000, per GitHub's +# "at least one minute" recommendation for secondary limits). +# OCR_RETRY_MAX_DELAY - Maximum delay (ms) cap applied to EVERY computed wait, +# including retry-after and x-ratelimit-reset, so a far-future +# reset cannot stall the job past its timeout (default: 300000 = 5 min). +# OCR_MAX_RETRIES - Max retry attempts per comment when rate-limited (default: 3). +# OCR_SUCCESS_DELAY - Delay (ms) between successful comment posts to pace requests (default: 2000). +# OCR_FAILURE_DELAY - Delay (ms) after a non-retryable failure to pace subsequent requests (default: 1000). +# OCR_LOW_REMAINING_THRESHOLD - When x-ratelimit-remaining is at or below this value, +# proactively increase request spacing to avoid hitting the limit +# (default: 3; GitHub best practice is to watch the header and slow down). +# OCR_LOW_REMAINING_SPACING - Request spacing (ms) used when remaining quota is low +# (default: 10000 = 10s). # # Note: GITHUB_TOKEN is automatically provided by GitHub Actions. +# Note: The workflow also configures llm.extra_body to '{"thinking": {"type": "disabled"}}' +# to disable thinking mode for compatibility with various LLM providers. name: OpenCodeReview PR Review @@ -76,7 +102,10 @@ jobs: node-version: '20' - name: Install OpenCodeReview - run: npm install -g @alibaba-group/open-code-review + run: | + npm install -g @alibaba-group/open-code-review + echo "OpenCodeReview installed with version:" + ocr version || true - name: Configure OCR run: | @@ -226,7 +255,7 @@ jobs: const failedComments = []; try { - await github.rest.pulls.createReview({ + const batchRes = await github.rest.pulls.createReview({ owner: context.repo.owner, repo: context.repo.repo, pull_number: prNumber, @@ -237,28 +266,85 @@ jobs: }); successCount = reviewComments.length; console.log(`Successfully posted review with ${successCount} inline comments (${commentsWithoutLine.length} in summary)`); + logRateLimitQuota(batchRes, 'after batch createReview'); } catch (e) { console.log('Failed to post review with inline comments:', e.message); - console.log('Falling back to posting comments individually...'); + console.log('Falling back to posting comments individually with rate-limit-aware retry...'); - // Fallback: post comments one by one + // Fallback: post comments one by one with delay to avoid secondary rate limits. + // GitHub enforces ~80 content-generating requests per minute; spacing calls + // helps stay under that threshold. Retry/wait durations are derived from the + // rate-limit response headers per GitHub's documented strategy. + const MAX_RETRIES = parseInt(process.env.OCR_MAX_RETRIES, 10) || 3; + const SUCCESS_DELAY = parseInt(process.env.OCR_SUCCESS_DELAY, 10) || 2000; // delay after successful post + const FAILURE_DELAY = parseInt(process.env.OCR_FAILURE_DELAY, 10) || 1000; // delay after non-retryable failure + const LOW_REMAINING_THRESHOLD = parseInt(process.env.OCR_LOW_REMAINING_THRESHOLD, 10) || 3; + const LOW_REMAINING_SPACING = parseInt(process.env.OCR_LOW_REMAINING_SPACING, 10) || 10000; + + // If the batch itself was rate-limited, honor its rate-limit headers + // (retry-after / x-ratelimit-reset) before retrying per-comment, + // otherwise the first per-comment call re-hits the same wall immediately. + const batchRetry = computeRetryDelayMs(e, 0); + if (batchRetry != null) { + const secs = (batchRetry.delayMs / 1000).toFixed(1); + console.log( + `Batch createReview was rate-limited (HTTP ${e.status}). ` + + `Cooling down ${secs}s via '${batchRetry.source}' (${batchRetry.detail}) before per-comment retry.` + ); + await sleep(batchRetry.delayMs); + } + for (const { comment, reviewComment } of reviewComments) { - try { - await github.rest.pulls.createReview({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: prNumber, - commit_id: commitSha, - body: '', - event: 'COMMENT', - comments: [reviewComment] - }); - successCount++; - console.log(`Successfully posted comment for ${reviewComment.path}`); - } catch (innerE) { - failedCount++; - failedComments.push({ comment, error: innerE.message }); - console.log(`Failed to post comment for ${reviewComment.path}: ${innerE.message}`); + let posted = false; + for (let attempt = 0; attempt <= MAX_RETRIES && !posted; attempt++) { + try { + const res = await github.rest.pulls.createReview({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: prNumber, + commit_id: commitSha, + body: '', + event: 'COMMENT', + comments: [reviewComment] + }); + successCount++; + posted = true; + console.log(`Successfully posted comment for ${reviewComment.path}`); + // Proactive throttle: if remaining quota is low, slow down to + // avoid hitting the limit (GitHub best practice: watch the header). + const remaining = logRateLimitQuota(res, `after ${reviewComment.path}`); + const lowQuota = remaining != null && remaining <= LOW_REMAINING_THRESHOLD; + if (lowQuota) { + console.log(`[rate-limit] quota low (remaining=${remaining} <= ${LOW_REMAINING_THRESHOLD}); increasing spacing to ${LOW_REMAINING_SPACING}ms.`); + await sleep(LOW_REMAINING_SPACING); + } else { + await sleep(SUCCESS_DELAY); + } + } catch (innerE) { + // Decide whether to retry and how long to wait, based on GitHub's + // rate-limit documentation (retry-after / x-ratelimit-* headers). + const retryInfo = computeRetryDelayMs(innerE, attempt); + const willRetry = retryInfo != null && attempt < MAX_RETRIES; + if (willRetry) { + const secs = (retryInfo.delayMs / 1000).toFixed(1); + console.log( + `Rate-limited/transient error on ${reviewComment.path} ` + + `(HTTP ${innerE.status}, attempt ${attempt + 1}/${MAX_RETRIES}). ` + + `Waiting ${secs}s via '${retryInfo.source}' (${retryInfo.detail}). ` + + `Error: ${innerE.message}` + ); + await sleep(retryInfo.delayMs); + } else { + failedCount++; + failedComments.push({ comment, error: innerE.message }); + const reason = retryInfo == null ? 'non-retryable error' : 'rate-limit retries exhausted'; + console.log(`Failed to post comment for ${reviewComment.path} (${reason}, HTTP ${innerE.status || 'n/a'}): ${innerE.message}`); + // After exhausting retries use the success-style pace delay; + // for other errors use the shorter failure pace delay. + await sleep(retryInfo == null ? FAILURE_DELAY : SUCCESS_DELAY); + break; + } + } } } @@ -288,6 +374,115 @@ jobs: }); } + function sleep(ms) { + return new Promise(resolve => setTimeout(resolve, ms)); + } + + // Case-insensitive header lookup. Octokit normalizes response headers to + // lowercase, but this defensive check also handles original casing so that + // quota logging and retry delay computation never silently miss a header. + function getHeader(headers, name) { + const v = headers[name] != null ? headers[name] : headers[name.toLowerCase()]; + return v != null ? String(v).trim() : undefined; + } + + // Decide whether an error is worth retrying and, if so, how long to wait. + // Implements GitHub's documented rate-limit retry strategy using the + // response headers (retry-after, x-ratelimit-remaining, x-ratelimit-reset). + // Returns { delayMs, source, detail } when retryable, or null otherwise. + // See: https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api + function computeRetryDelayMs(error, attempt) { + if (!error) return null; + const status = error.status; + const message = String(error.message || ''); + const isRateLimit = status === 429 || (status === 403 && /rate limit|abuse|secondary/i.test(message)); + const isTransient = (status >= 500 && status < 600) || status === 408; + if (!isRateLimit && !isTransient) return null; + + const headers = ((error.response || {}).headers) || {}; + const header = (name) => getHeader(headers, name); + const nowSec = Math.floor(Date.now() / 1000); + + // The absolute maximum wait for any single retry. Header-derived waits + // (retry-after / x-ratelimit-reset) are GitHub's recommended durations, + // but capping them prevents a far-future reset from stalling the CI job + // past its timeout. When we cap, the next retry may re-hit the limit. + const cap = parseInt(process.env.OCR_RETRY_MAX_DELAY, 10) || 300000; + const base = parseInt(process.env.OCR_RETRY_BASE_DELAY, 10) || 60000; + + // { rawMs, source, detail } describing the recommended wait before cap. + let info = null; + + if (isRateLimit) { + // (1) Honor "retry-after" when present (seconds, or an HTTP-date). + const retryAfter = header('retry-after'); + if (retryAfter) { + const secs = Number(retryAfter); + if (!isNaN(secs) && secs >= 0) { + info = { rawMs: secs * 1000, source: 'retry-after', detail: `${secs}s (from header)` }; + } else { + const dateMs = Date.parse(retryAfter); + if (!isNaN(dateMs)) { + info = { rawMs: Math.max(0, dateMs - Date.now()), source: 'retry-after (HTTP-date)', detail: retryAfter }; + } + } + } + + // (2) Primary limit exhausted (x-ratelimit-remaining=0): wait until reset. + if (!info) { + const remaining = header('x-ratelimit-remaining'); + const reset = header('x-ratelimit-reset'); + if (reset != null && Number(remaining) === 0) { + const rawMs = Math.max(0, Number(reset) - nowSec) * 1000; + info = { rawMs, source: 'x-ratelimit-reset', detail: `remaining=0, reset epoch=${reset} (in ${Math.ceil(rawMs / 1000)}s)` }; + } + } + + // (3) Secondary limit with no retry hint: docs say wait at least one + // minute, then increase exponentially between retries. + if (!info) { + const backoff = Math.min(base * Math.pow(2, attempt), cap); + const jitter = Math.floor(Math.random() * 1000); + info = { rawMs: backoff + jitter, source: 'exponential-backoff', detail: `base=${base}ms*2^${attempt} (cap ${cap}ms) +${jitter}ms jitter` }; + } + } else { + // Transient server error (5xx / 408): back off without the 60s floor. + // Use a shorter base than the rate-limit path: server hiccups are + // typically short-lived, so a 2s initial wait (doubling per retry) + // is sufficient and avoids stalling the CI job unnecessarily. + const transientBase = 2000; + const backoff = Math.min(transientBase * Math.pow(2, attempt), cap); + const jitter = Math.floor(Math.random() * 1000); + info = { rawMs: backoff + jitter, source: 'transient-backoff', detail: `base=${transientBase}ms*2^${attempt} (cap ${cap}ms) +${jitter}ms jitter (HTTP ${status})` }; + } + + // Apply the universal cap to header-derived waits too. + const delayMs = Math.min(info.rawMs, cap); + if (delayMs < info.rawMs) { + info.detail += ` [CAPPED to ${cap}ms; GitHub recommended ${Math.ceil(info.rawMs / 1000)}s]`; + } + return { delayMs, source: info.source, detail: info.detail }; + } + + // Best-effort logging of remaining rate-limit quota from a successful response. + // Returns the parsed x-ratelimit-remaining value (or null) for proactive throttling. + function logRateLimitQuota(response, tag) { + try { + const h = (response && response.headers) || {}; + const header = (name) => getHeader(h, name); + const remaining = header('x-ratelimit-remaining'); + const limit = header('x-ratelimit-limit'); + const reset = header('x-ratelimit-reset'); + if (remaining != null) { + console.log( + `[rate-limit] ${tag}: remaining=${remaining}/${limit != null ? limit : '?'}` + + (reset != null ? `, reset epoch=${reset}` : '') + ); + } + return remaining != null ? Number(remaining) : null; + } catch (_) { return null; } + } + function formatComment(comment) { let body = comment.content || ''; diff --git a/examples/gitlab_ci/.gitlab-ci.yml b/examples/gitlab_ci/.gitlab-ci.yml index 2cff059..e8c3f78 100644 --- a/examples/gitlab_ci/.gitlab-ci.yml +++ b/examples/gitlab_ci/.gitlab-ci.yml @@ -13,6 +13,17 @@ # GITLAB_API_TOKEN - GitLab Personal/Project Access Token with "api" scope # (falls back to CI_JOB_TOKEN if not set) # +# Optional CI/CD Variables (for retry/delay tuning): +# OCR_RETRY_BASE_DELAY - Base delay (ms) for exponential backoff on rate-limit retries (default: 2000) +# OCR_MAX_RETRIES - Max retry attempts per comment when rate-limited or transient error (default: 3) +# OCR_MAX_RETRY_DELAY - Maximum delay (ms) per single retry, caps Retry-After/backoff (default: 60000) +# OCR_SUCCESS_DELAY - Delay (ms) after a successful comment post to pace requests (default: 2000) +# OCR_FAILURE_DELAY - Delay (ms) after a non-rate-limit failure to pace subsequent requests (default: 1000) +# OCR_RATE_LIMIT_THRESHOLD - Proactively slow down when GitLab RateLimit-Remaining is at/below this value (default: 10, set 0 to disable) +# +# Note: The pipeline also configures llm.extra_body to '{"thinking": {"type": "disabled"}}' +# to disable thinking mode for compatibility with various LLM providers. +# # Fork MR Support: # The script uses CI_COMMIT_SHA as the diff target to correctly resolve the # source commit from forked repos. For some GitLab versions, you may need to enable: @@ -34,6 +45,7 @@ code-review: script: # Install OpenCodeReview - npm install -g @alibaba-group/open-code-review + - echo "OpenCodeReview installed with version:" && ocr version || true # Configure OCR - mkdir -p ~/.open-code-review @@ -62,7 +74,9 @@ code-review: python3 << 'PYTHON_SCRIPT' import json import os + import random import sys + import time import urllib.request import urllib.error @@ -75,6 +89,20 @@ code-review: TARGET_BRANCH = os.environ["CI_MERGE_REQUEST_TARGET_BRANCH_NAME"] COMMIT_SHA = os.environ["CI_COMMIT_SHA"] + # Configurable retry/delay settings (via CI/CD variables, with sensible defaults) + RETRY_BASE_DELAY = int(os.environ.get("OCR_RETRY_BASE_DELAY", "2000")) / 1000 # ms → seconds + MAX_RETRIES = int(os.environ.get("OCR_MAX_RETRIES", "3")) + MAX_RETRY_DELAY = int(os.environ.get("OCR_MAX_RETRY_DELAY", "60000")) / 1000 # ms → seconds, cap per-retry wait + SUCCESS_DELAY = int(os.environ.get("OCR_SUCCESS_DELAY", "2000")) / 1000 + FAILURE_DELAY = int(os.environ.get("OCR_FAILURE_DELAY", "1000")) / 1000 + # Base delay (seconds) for exponential backoff on transient server errors (5xx / 408). + # Server hiccups are typically short-lived, so a 2s initial wait (doubling per retry) + # is sufficient and avoids stalling the CI job unnecessarily. + TRANSIENT_BASE_DELAY = 2 + # Proactive throttling: when RateLimit-Remaining drops to/below this threshold, + # the script doubles the pacing delay to avoid hitting 429. Set to 0 to disable. + RATE_LIMIT_THRESHOLD = int(os.environ.get("OCR_RATE_LIMIT_THRESHOLD", "10")) + if not API_TOKEN: print("ERROR: No API token available (GITLAB_API_TOKEN or CI_JOB_TOKEN). Cannot post comments.", file=sys.stderr) sys.exit(1) @@ -85,25 +113,87 @@ code-review: USE_JOB_TOKEN = not os.environ.get("GITLAB_API_TOKEN") AUTH_HEADER = "JOB-TOKEN" if USE_JOB_TOKEN else "PRIVATE-TOKEN" - def api_request(endpoint, data=None, method="POST"): - """Make a GitLab API request.""" - url = f"{API_BASE}{endpoint}" - headers = { - AUTH_HEADER: API_TOKEN, - "Content-Type": "application/json" - } - body = json.dumps(data).encode("utf-8") if data else None - req = urllib.request.Request(url, data=body, headers=headers, method=method) + def _get_header(headers, name): + """Case-insensitive header lookup. + + urllib normalizes response header keys to title-case (e.g. 'Retry-After'), + but this defensive check also handles original casing so that retry delay + computation and quota logging never silently miss a header. + """ + if name in headers: + val = headers[name] + elif name.lower() in headers: + val = headers[name.lower()] + else: + return None + return str(val).strip() if val is not None else None + + def _parse_rate_limit_header(headers, name): + """Safely parse a rate-limit response header (e.g. RateLimit-Remaining) as an int.""" + val = _get_header(headers, name) + if val is None: + return None try: - with urllib.request.urlopen(req) as resp: - return json.loads(resp.read().decode("utf-8")) - except urllib.error.HTTPError as e: - print(f"API error {e.code}: {e.read().decode('utf-8')}", file=sys.stderr) + return int(val) + except (ValueError, TypeError): return None + def api_request_with_retry(endpoint, data=None, method="POST"): + """Make a GitLab API request with retry on rate-limit errors. + Returns a dict: {'success': bool, 'data': response or None, + 'is_rate_limit_exhausted': bool, 'rate_limit_remaining': int or None} + """ + for attempt in range(MAX_RETRIES + 1): + url = f"{API_BASE}{endpoint}" + headers = { + AUTH_HEADER: API_TOKEN, + "Content-Type": "application/json" + } + body = json.dumps(data).encode("utf-8") if data else None + req = urllib.request.Request(url, data=body, headers=headers, method=method) + try: + with urllib.request.urlopen(req) as resp: + resp_data = json.loads(resp.read().decode("utf-8")) + remaining = _parse_rate_limit_header(resp.headers, 'RateLimit-Remaining') + limit = _parse_rate_limit_header(resp.headers, 'RateLimit-Limit') + if remaining is not None and limit is not None: + print(f"RateLimit: {remaining}/{limit} remaining for {endpoint}", file=sys.stderr) + return {'success': True, 'data': resp_data, 'is_rate_limit_exhausted': False, 'rate_limit_remaining': remaining} + except urllib.error.HTTPError as e: + error_body = e.read().decode('utf-8') + is_rate_limit = e.code == 429 or (e.code == 403 and any(kw in error_body.lower() for kw in ['retry later', 'rate limit', 'too many requests', 'abuse'])) + # Transient server errors (5xx) and request timeouts (408) are worth retrying + # with a short exponential backoff, since they are typically short-lived. + is_transient = (500 <= e.code < 600) or e.code == 408 + rl_remaining = _parse_rate_limit_header(e.headers, 'RateLimit-Remaining') + if (is_rate_limit or is_transient) and attempt < MAX_RETRIES: + retry_after = _get_header(e.headers, 'Retry-After') + if retry_after: + try: + delay = float(retry_after) + except ValueError: + delay = RETRY_BASE_DELAY * (2 ** attempt) # fallback if Retry-After is not numeric + elif is_transient: + # Transient server error: use a shorter base than the rate-limit path. + # Server hiccups are typically short-lived, so a 2s initial wait + # (doubling per retry) is sufficient. + delay = TRANSIENT_BASE_DELAY * (2 ** attempt) + else: + delay = RETRY_BASE_DELAY * (2 ** attempt) + delay = min(delay, MAX_RETRY_DELAY) # cap to prevent long stalls + delay = delay * (0.75 + random.random() * 0.5) # ±25% jitter to avoid thundering herd + rl_info = f" (RateLimit-Remaining: {rl_remaining})" if rl_remaining is not None else "" + reason = "rate limit" if is_rate_limit else f"transient error (HTTP {e.code})" + print(f"{reason} hit for {endpoint}, retrying in {delay:.1f}s (attempt {attempt + 1}/{MAX_RETRIES}){rl_info}", file=sys.stderr) + time.sleep(delay) + else: + print(f"API error {e.code}: {error_body}", file=sys.stderr) + return {'success': False, 'data': None, 'is_rate_limit_exhausted': is_rate_limit, 'rate_limit_remaining': rl_remaining} + return {'success': False, 'data': None, 'is_rate_limit_exhausted': False, 'rate_limit_remaining': None} + def post_note(body): - """Post a general note/comment on the MR.""" - return api_request("/notes", {"body": body}) + """Post a general note/comment on the MR (with rate-limit retry).""" + return api_request_with_retry("/notes", {"body": body}) def post_discussion(path, line, body, base_sha, start_sha, head_sha): """Post an inline discussion on a specific file/line in the MR diff.""" @@ -120,7 +210,7 @@ code-review: "body": body, "position": position } - return api_request("/discussions", data) + return api_request_with_retry("/discussions", data) def format_comment(comment): """Format a single review comment as markdown.""" @@ -184,13 +274,11 @@ code-review: print("No review comments to post.") sys.exit(0) - # Get MR diff metadata for position calculation + # Get MR diff metadata for position calculation (uses retry to avoid single-point-of-failure) diff_refs = None - try: - versions_url = f"{API_BASE}/versions" - req = urllib.request.Request(versions_url, headers={AUTH_HEADER: API_TOKEN}) - with urllib.request.urlopen(req) as resp: - versions = json.loads(resp.read().decode("utf-8")) + versions_resp = api_request_with_retry("/versions", method="GET") + if versions_resp and versions_resp.get('success'): + versions = versions_resp.get('data', []) if versions: latest = versions[0] diff_refs = { @@ -198,8 +286,8 @@ code-review: "start_sha": latest.get("start_commit_sha", ""), "head_sha": latest.get("head_commit_sha", ""), } - except Exception as e: - print(f"Warning: Could not fetch MR versions: {e}", file=sys.stderr) + if not diff_refs: + print("Warning: Could not fetch MR versions. Inline comments will use fallback.", file=sys.stderr) # Post inline discussions for each comment success_count = 0 @@ -216,10 +304,22 @@ code-review: continue result_resp = post_discussion(path, end_line, body, **diff_refs) - if result_resp: + if result_resp and result_resp.get('success'): success_count += 1 + # Proactive throttling: slow down when GitLab reports low remaining quota + remaining = result_resp.get('rate_limit_remaining') + if RATE_LIMIT_THRESHOLD > 0 and remaining is not None and remaining <= RATE_LIMIT_THRESHOLD: + pace_delay = SUCCESS_DELAY * 2 + print(f"Rate limit quota low ({remaining} remaining), increasing pacing delay to {pace_delay:.1f}s", file=sys.stderr) + time.sleep(pace_delay) + else: + time.sleep(SUCCESS_DELAY) # Pace requests to avoid rate limits else: failed_comments.append(comment) + # Apply longer delay when rate-limit retries are exhausted, shorter delay for other errors + is_rate_limit_exhausted = result_resp.get('is_rate_limit_exhausted', False) if result_resp else False + post_fail_delay = SUCCESS_DELAY if is_rate_limit_exhausted else FAILURE_DELAY + time.sleep(post_fail_delay) print(f"Successfully posted {success_count}/{len(comments)} inline comments.") diff --git a/examples/gitlab_ci/README.md b/examples/gitlab_ci/README.md index 6ac630b..ab9461f 100644 --- a/examples/gitlab_ci/README.md +++ b/examples/gitlab_ci/README.md @@ -98,6 +98,21 @@ script: - ocr review --rule ./my-rules.json --from origin/$CI_MERGE_REQUEST_TARGET_BRANCH_NAME --to $CI_COMMIT_SHA ``` +### Adjust retry and delay settings + +When posting review discussions, the script includes rate-limit handling with exponential backoff (with jitter), `Retry-After` header support, and proactive throttling based on GitLab's `RateLimit-Remaining` response header. All API requests — including summary notes and MR version fetches — use the same retry logic. See [GitLab Rate Limits](https://docs.gitlab.com/security/rate_limits/) for details on GitLab's rate-limiting policies and recommended handling. You can configure the retry and delay behavior via **CI/CD Variables** (Settings → CI/CD → Variables): + +| Variable | Default | Description | +|----------|---------|-------------| +| `OCR_RETRY_BASE_DELAY` | `2000` | Base delay (ms) for exponential backoff when a rate-limit error is hit | +| `OCR_MAX_RETRIES` | `3` | Maximum retry attempts per discussion when rate-limited | +| `OCR_MAX_RETRY_DELAY` | `60000` | Maximum delay (ms) per single retry, caps both `Retry-After` and backoff | +| `OCR_SUCCESS_DELAY` | `2000` | Delay (ms) after a successful discussion post to pace subsequent requests | +| `OCR_FAILURE_DELAY` | `1000` | Delay (ms) after a non-rate-limit failure to pace subsequent requests | +| `OCR_RATE_LIMIT_THRESHOLD` | `10` | Proactively slow down when GitLab `RateLimit-Remaining` is at/below this value (set `0` to disable) | + +These variables are optional — if not configured, sensible defaults are used. Consider increasing delays for self-hosted GitLab instances with aggressive rate-limit configurations or for large MRs that generate numerous review comments. The `OCR_RATE_LIMIT_THRESHOLD` variable enables proactive throttling: when GitLab reports low remaining quota in the `RateLimit-Remaining` response header, the script automatically doubles the pacing delay to avoid hitting 429 errors. + ### Limit concurrency Adjust the `--concurrency` flag for large MRs to control the number of concurrent LLM requests: