Skip to content

feat: reliability hardening for Groq client (P1) - #10

Merged
PG1204 merged 2 commits into
mainfrom
p1-reliability
Jun 15, 2026
Merged

feat: reliability hardening for Groq client (P1)#10
PG1204 merged 2 commits into
mainfrom
p1-reliability

Conversation

@PG1204

@PG1204 PG1204 commented Jun 15, 2026

Copy link
Copy Markdown
Owner

Summary

P1 reliability hardening for the Groq integration, stacked on the P0 security work already on main.

  • Backoff + jitter between retriesGroqClient previously retried in a tight loop with no delay. Now uses exponential backoff with equal jitter ([cap/2, cap], cap doubling per attempt up to 8s) so concurrent callers don't stampede a recovering upstream. Interrupt-safe sleeps.
  • Retry on HTTP 429 — a rate-limit 429 previously hit the 4xx "don't retry" path and failed immediately. It's now treated as retryable and honors Retry-After / retry-after-ms headers (capped), falling back to backoff when absent.
  • modelName single source of truth — removed the hardcoded MODEL_NAME constant in AnalysisService that duplicated (and could drift from) groq.api.model. The reported model now comes from GroqClient.getModel().

Testing

  • Made GroqClient testable via a package-private constructor (injectable HttpClient + backoff bounds).
  • New GroqClientTest (8 cases): success, 5xx/429/IOException retry, 4xx no-retry, Retry-After parsing, attempt exhaustion.
  • New AnalysisServiceTest case asserting modelName is sourced from config.
  • Full suite: 47 passing locally (live GoldenDiffTest excluded — requires a real GROQ_API_KEY + network).

PG1204 and others added 2 commits June 15, 2026 12:11
- Add exponential backoff with equal jitter between retries (was tight-loop, no delay).
- Retry on HTTP 429 (rate limit), honoring Retry-After / retry-after-ms headers.
- Source response modelName from GroqClient.getModel() as the single source of
  truth, instead of a hardcoded AnalysisService constant that could drift from
  the configured groq.api.model.
- Add GroqClientTest (mocked HttpClient, tiny backoff) covering 2xx, 5xx/429/
  IOException retry, 4xx no-retry, and attempt exhaustion; add AnalysisService
  test asserting modelName comes from config.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…able

A non-2xx response (e.g. an offline ngrok tunnel returning ERR_NGROK_3200)
previously failed the PR check with exit 1. Now it logs a warning, posts an
advisory comment (overallRisk=UNKNOWN) and exits 0, so an ephemeral dev
service can never block a PR. Pair with an ngrok reserved domain to keep
BLAST_RADIUS_URL stable across restarts.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

💥 Blast Radius Analysis

Overall Risk: ❓ UNKNOWN

🔍 Impact Areas

  • Analysis service was unreachable — this check is advisory and does not block the PR.

🧪 Suggested Tests

  • None suggested

Analyzed by Blast Radius (Spring Boot + Groq LLM) • 2026-06-15

@PG1204
PG1204 merged commit d08d6fe into main Jun 15, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant