[BUG] Didn't retry if meet LLM call failed

[manhnt9]

Quick Summary

Environment & Tools

• PicoClaw Version: v0.1.2
• AI Model & Provider: (OpenRouter)
• Operating System: (Ubuntu 22.04)
• Channels: (Discord)

📸 Steps to Reproduce

1. Run a long task
2. Server may return HTTP 500 sometimes
3. Task hang without retry, logs output LLM call failed

❌ Actual Behavior

Task dropped

✅ Expected Behavior

Retry and continue

[nikolasdehor]

This is a significant reliability gap. LLM APIs are inherently unreliable — transient 500s, rate limits (429), and network timeouts are expected in production. Dropping the task silently on first failure is not acceptable.

Suggested retry strategy:

1. Exponential backoff with jitter for retryable errors (500, 502, 503, 429, network timeout):

func (a *Agent) callLLMWithRetry(ctx context.Context, messages []Message, maxRetries int) (*LLMResponse, error) {
    for attempt := 0; attempt <= maxRetries; attempt++ {
        resp, err := a.provider.ChatCompletion(ctx, messages)
        if err == nil {
            return resp, nil
        }
        if !isRetryable(err) {
            return nil, err // 401, 403, 400 — don't retry
        }
        backoff := time.Duration(math.Pow(2, float64(attempt))) * time.Second
        jitter := time.Duration(rand.Int63n(int64(backoff / 2)))
        select {
        case <-ctx.Done():
            return nil, ctx.Err()
        case <-time.After(backoff + jitter):
        }
    }
    return nil, fmt.Errorf("LLM call failed after %d retries", maxRetries)
}

2. Error classification — use the existing pkg/providers/error_classifier.go (I see it exists in the codebase) to distinguish retryable vs. terminal errors. 4xx client errors (except 429) should not be retried.

3. Config — make max_retries configurable (default: 3). Some users may want 0 retries for fast-fail during development.

4. 429 special handling — if the API returns a Retry-After header, respect it instead of using the calculated backoff.

5. User feedback — on retry, send a "thinking..." or typing indicator to the channel so the user knows the bot hasn't crashed.

This pairs well with #564 (user-friendly error messages) — if retries are exhausted, show a friendly message instead of raw Go errors.

[SirBrenton]

You’re right to treat this as a reliability gap. One add: retries should be bounded by max elapsed time (not just attempts) to prevent “infinite hang” under persistent 5xx/429. Also, 429 should honor Retry-After when present, and you want minimal retry telemetry (attempt, backoff, classification) so you can see where pain concentrates later.

If helpful: I maintain a tiny deterministic tool that takes LLM/tool-call “exhaust” as JSONL and produces a ranked hazard pack (report + CSVs) showing which call signatures drive failures/budget breaches. If you can share ~50–200 redacted events (status/error_class/latency_ms/budget_ms/retries/provider), I’ll run it and reply with the pack + the top 2 guardrails to implement first.

[nikolasdehor]

@SirBrenton Good call on bounding by max elapsed time rather than just attempt count — that prevents the edge case where persistent 5xx with long Retry-After headers accumulate into minutes of waiting. A config like max_retry_elapsed: 60s alongside max_retries: 3 gives operators both knobs.

Worth noting that PR #594 is open and implements retry with exponential backoff for LLM calls. It would be good to check if it already handles the elapsed time bound and Retry-After header, or if those should be added before merge.

The telemetry suggestion (attempt/backoff/classification JSONL) is interesting for observability. PicoClaw currently has minimal structured metrics — this could be a good starting point for a lightweight retry dashboard.