feat: add per-attempt observability logging

[dtsong]

Summary

• Add AttemptRecord and RetryResult to capture per-attempt observability data (model, latency, failure type, strategy applied)
• RetryExhaustedError carries attempt records even on failure for debugging
• SupabaseLogger.log_attempts() bulk-inserts attempt data to review_attempts table
• Updated degradation pipeline to unwrap RetryResult

Test plan

• 12 new tests covering attempt recording, retry scenarios, and Supabase logging
• Updated existing retry handler and integration tests for new return type
• Full suite passes (366 tests, 95.72% coverage)

Closes #38

🤖 Generated with Claude Code

[github-actions]

AI Code Review

Summary

This PR adds comprehensive observability to the retry system, tracking per-attempt metrics and failures. The implementation is well-structured with good separation of concerns, though there are some minor issues around error handling and missing edge cases.

Confidence: 0.00 (low)
This PR may require human review

Strengths

• Comprehensive observability with detailed attempt tracking
• Clean separation between retry logic and observability concerns
• Proper error handling with contextlib.suppress for non-critical logging
• Good test coverage with realistic failure scenarios
• Backwards compatibility maintained through result unwrapping in degradation pipeline
• Proper time measurement with monotonic clock

Issues Found

Major (1)

src/pr_review_agent/execution/retry_handler.py:201 - logic

The failure variable is not initialized before the exception handling blocks, which could cause UnboundLocalError if an unexpected exception type is raised
Suggestion: Initialize failure to a default value before the exception handling blocks

Minor (3)

src/pr_review_agent/execution/retry_handler.py:196 - logic

Non-anthropic exceptions (like network errors, timeouts) are re-raised without being recorded in attempt_records, which could lead to incomplete observability data
Suggestion: Consider catching and recording other types of exceptions before re-raising them to ensure complete observability

tests/test_retry_observability.py:157 - testing

The test catches a generic Exception but expects it to have an 'attempts' attribute. This should specifically catch RetryExhaustedError to be more precise
Suggestion: Import and catch the specific RetryExhaustedError exception type instead of generic Exception

src/pr_review_agent/execution/retry_handler.py:46 - logic

The _describe_strategy function could return an empty string when parts is empty instead of None, which might be more consistent for logging
Suggestion: Consider returning empty string instead of None when no strategy parts are present

Suggestion (2)

src/pr_review_agent/metrics/supabase_logger.py:86 - performance

The log_attempts method creates a list comprehension for all attempts even if the Supabase call fails. Consider creating the rows inside the try block to avoid unnecessary work
Suggestion: Move the rows list comprehension inside the contextlib.suppress block to avoid creating data structures when logging is disabled or fails

src/pr_review_agent/execution/retry_handler.py:130 - documentation

The docstring for retry_with_adaptation should be updated to reflect the new return type and mention that failure records are included in the exception
Suggestion: Update the docstring to document the RetryResult return type and RetryExhaustedError behavior

Concerns

• Uninitialized variable could cause runtime errors in edge cases
• Non-anthropic exceptions might not be properly recorded for observability
• The API surface has changed significantly which could affect other consumers of retry_with_adaptation

Questions

• Are there other parts of the codebase that call retry_with_adaptation directly that would need similar unwrapping?
• Should the retry system handle and record non-anthropic exceptions (network timeouts, etc.) for complete observability?
• Is there a plan to eventually use the attempt records for adaptive improvements to the retry strategy?

---

_{Model: claude-sonnet-4-20250514 | Cost: $0.0416 |
Tokens: 7534 in / 1265 out}