fix: retry HTTP 408 timeouts 🤖🤖🤖

[rlissi-nv]

What does this PR do?

LiteLLM timeouts stop after the first attempt, even though Nooa's default client is configured to retry timeouts. LiteLLM reports these errors as status 408, which the default retry policy currently skips.

Add 408 to the existing default retry set. The current retry loop, backoff, and retry budget handle it. Callers that explicitly exclude 408 still get no retry.

Related issues

This surfaced through Trace Analyst's CompletionClient.acall() path. It is separate from the application-owned admission-control work in #349; no concurrency policy changes are included.

Checklist

• Code follows the project style. Full lint and formatting checks pass with the locked Ruff version, 0.15.20.
• Tests added/updated and passing. 93 focused tests and 1,391 non-live client tests pass.
• Documentation checked. The existing client docstrings already promise timeout retries; this restores that behavior without adding an API.
• SPDX headers preserved. No new source files are added, and the repository-wide header check passes.

Validation

Before the fix, 11 timeout regressions fail. After the fix, the default CompletionClient recovers from an injected LiteLLM timeout followed by a successful response. Tests also cover synchronous/asynchronous recovery, backoff, exhaustion, explicit exclusions, permanent 4xx errors, and cancellation.

The existing budget remains three retries after the first attempt. A persistent 600-second timeout can therefore take about 40 minutes plus backoff. This fixes skipped retries, not the cause of the endpoint delay.

Test scope and commands

Tested against b814ca1db9e7377f9aa3a3471b9396da2f4ff3da using Python 3.13 and LiteLLM 1.98.0, the version from the failing client environment. Provider calls were mocked. Five live-provider cases were excluded. The full repository suite remains for CI.

uv run pytest tests/unifiedllm -m 'not integration and not stress and not sandbox' -q
uv run ruff check .
uv run ruff format --check .
uv run python scripts/check_license_headers.py
git diff --check

Summary by CodeRabbit

• Bug Fixes
  • Requests that time out or return HTTP 408 are now retried automatically, following the usual retry schedule. If retries are exhausted, the original error is returned.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: NVIDIA-NeMo/labs-OO-Agents/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 7790873c-7983-4fbc-b91a-ec1d72de36fd

📥 Commits

Reviewing files that changed from the base of the PR and between b814ca1 and e587786.

📒 Files selected for processing (4)

• src/nooa/unifiedllm/retry_config.py
• tests/unifiedllm/test_empty_content_retry.py
• tests/unifiedllm/test_retry.py
• tests/unifiedllm/test_retry_config.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 8 remain after this review.

---

📝 Walkthrough

Walkthrough

The default retryable status-code set now includes HTTP 408. Tests cover classification and async and sync retries, including backoff delays and exhausted retry attempts.

Changes

HTTP 408 retries

Layer / File(s)	Summary
Add HTTP 408 to the retry policy src/nooa/unifiedllm/retry_config.py, tests/unifiedllm/test_retry.py, tests/unifiedllm/test_retry_config.py	The default retryable status-code set and classification tests now include HTTP 408.
Verify async and sync retry behavior tests/unifiedllm/test_retry.py, tests/unifiedllm/test_empty_content_retry.py	Tests cover retries for status-408 errors and an Anthropic timeout, including backoff delays, successful retries, and retry exhaustion.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: furgalep

Merge Risk: ⚪ Minimal · up to e5877

Timeouts reported as HTTP 408 now retry under the default policy, using the existing backoff and retry limits. No merge-blocking risk was found.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 35.71% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the main change: adding retry support for HTTP 408 timeouts. The emojis are unnecessary but do not make the title unclear or unrelated.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

_{Comment @coderabbitai help to get the list of available commands.}

[furgalep]

LGTM. Thank you!