Feat/llm retry report:SDK retry-attempt observability for review (#785)

[Gongyl01]

Summary

Closes #785.

Part of #368.

Adds a versioned, immutable RetryReport to ocr review that records the real HTTP attempts made inside the official Anthropic and OpenAI SDK retry loops. OCR currently sees only the final return value of a logical LLM request, so a request that receives 429, then 529, and finally succeeds is indistinguishable from a first-attempt success.

The report is frozen once after the review run and its background work have finished. Text output renders a compact attempt chain, while --format json exposes the same frozen value under the optional top-level retry_report field. A clean first-attempt-success run emits no report, so the existing output remains unchanged.

This is the observability slice of #368. It explains what the existing SDK retry loops did; it does not take ownership of retry policy.

• internal/llm/retry_report.go: adds the ocr.llm-retry-report/v1 value model and a per-run, concurrency-safe RetryCollector. The collector derives attempt numbers and timings, decides request outcomes exactly once, validates aggregate invariants, and freezes requests in deterministic logical_request_id order.
• internal/llm/retry_observer.go: mounts one shared Middleware implementation on Anthropic, OpenAI Chat Completions, and OpenAI Responses clients. It observes each real HTTP attempt without reading response bodies or overriding SDK decisions, recording status, provider request ID, server retry hints, x-should-retry, time to response headers, and the measured gap between attempts.
• internal/llm/retry_boundary.go + client boundaries: correct attempts whose failure becomes visible only after HTTP 200, including unexpected EOF or malformed decoding, interrupted/incomplete SSE streams, and unsuccessful Responses object statuses. Every logical request finalizes on success, error, cancellation, or panic.
• internal/llm/retry_meta.go + review call sites: stamps stable, non-secret identity on plan, main_task, memory compression, re-location, and review filter requests. The identity joins the report to existing session task records; scan and llm test remain outside the report.
• cmd/opencodereview: freezes the collector at the same run boundary as the manifest and publishes the report exactly once across normal and failure exits. Terminal and JSON output consume the same immutable snapshot.

Commits (5 layered slices)

Commit	Slice
6c7525c	feat(llm): add retry report data layer
5b9509b	feat(llm): observe retry attempts via SDK middleware
4145d13	feat(llm): correct attempts and finalize requests at the client boundary
8fec2a3	feat(llm): stamp request identity on review LLM requests
e41741e	feat(cmd): publish the frozen retry report at the run boundary

Design (key invariants)

• Observation only. The official SDK remains the retry owner. This PR does not change WithMaxRetries(5), SDK backoff or jitter, Retry-After behavior, or retry admission.
• One logical request, one final outcome. Request outcome is decided from the complete attempt sequence, the logical call's return value, and the parent context: succeeded, recovered, failed, or cancelled. It is never inferred from the last attempt alone.
• Observed facts, not guessed text. Attempt classification reads HTTP status and Go error types only. Raw provider error text is never parsed. A non-2xx status is authoritative; failures discovered after HTTP 200 are revised only when the boundary has typed evidence about the phase.
• Exactly-once publication. The collector freezes only after Agent.Run has joined background work. Normal result and failure-usage paths cannot publish the same report twice, and an invariant violation suppresses the self-contradictory report instead of emitting partial data.
• Deterministic under concurrency. A per-run collector prevents state leakage between runs. Stable request identity, contiguous attempt numbering, and sorted logical_request_id output make concurrent runs reproducible.
• Additive output contract. retry_report is optional and uses omitempty; a run with only clean first attempts keeps its previous terminal and JSON output.
• Allowlisted diagnostics. The report may contain provider/model labels, file/task identity, status codes, provider request IDs, retry hints, and timings. It never contains credentials, authorization headers, prompts, request or response bodies, complete endpoint URLs, or raw provider error strings.

Attempt classification

error_class	Evidence
rate_limited	HTTP 429
overloaded	HTTP 529
authentication	HTTP 401/403
timeout	HTTP 408/504 or an error matching context.DeadlineExceeded
network	Transport failure or unexpected EOF
provider	Other explicit non-2xx provider status, or a typed provider stream/status failure
cancelled	An error matching context.Canceled, including explicit parent-context cancellation
unknown	Stable fallback when a post-200 failure cannot be classified more specifically without parsing message text

Output contract

Example terminal output:

LLM retry report: 1/2 requests retried, 2 retries, 1 recovered, 1 failed
- internal/agent/agent.go / main_task #1: rate_limited(429) -> overloaded(529) -> success
- internal/llm/client.go / plan #1: authentication(401) -> failed

The JSON path is additive:

.retry_report.schema_version == "ocr.llm-retry-report/v1"

The terminal summary and retry_report are generated from the same frozen RetryReport; JSON mode emits one JSON document and never mixes in the terminal summary.

Upstream integration

This branch is synchronized through upstream/main@62e2b99. The retry report is published alongside #367's frozen run manifest without changing its coverage or terminal-state contract. Existing scan output, llm test, Resume/checkpoint behavior, budget reporting, and session persistence semantics remain unchanged.

How to test

make test
make vet

# A clean run keeps the existing contract: retry_report is absent.
ocr review --from main --to feature --format json | jq '.retry_report'

# On a run that encounters a retryable or terminal LLM failure,
# inspect the frozen report and its attempt chains.
ocr review --from main --to feature --format json | jq '.retry_report'

Checklist

• make test passes locally (full non-extension package suite with -race -count=1)
• make vet passes
• git diff --check upstream/main...HEAD is clean
• gofmt -l reports no changed Go files
• Tests cover 429 recovery, 529 exhaustion, authentication behavior, timeout and cancellation semantics, transport and unexpected EOF recovery, post-200 decode/stream/status correction, panic finalization, concurrent collection, deterministic ordering, and exactly-once terminal/JSON publication
• Clean first-attempt-success output is regression-locked with no retry_report
• The emitted schema and terminal rendering are allowlist-tested against secret or raw provider content

Out of scope (deliberately)

• Provider-specific concurrency limits, retry budgets, retry admission, custom backoff, or a shared coordinator; tracked by Add provider-specific retry budgets and a shared retry coordinator #787.
• Trusted Resume, changed-input rejection, or provider/model transition lineage; tracked by Add trusted resume with explicit provider and model transition lineage #786.
• Automatic provider selection, provider ranking, or implicit cross-provider failover.
• CLI reference updates are deferred until the remaining Add provider-aware partial retry with preserved lineage #368 contracts are finalized, so retry output, provider policy, and Resume lineage can be documented together.

[wu21-web]

Thank you, but this is big and hard to review.

[github-actions]

✅ OpenCodeReview: Review partially complete: 0 finding(s); 2 of 16 selected item(s) failed.

[Gongyl01]

Thank you, but this is big and hard to review.

Hi @wu21-web, Thanks for the feedback! To help with the review, here's a breakdown — about 4,670 of the 6,197 added lines (~75%) are tests. The core implementation is only ~1,500 lines:

New core files (suggested reading order):

1. internal/llm/retry_meta.go (+154) — retry metadata carried per request
2. internal/llm/retry_boundary.go (+141) — retry boundary decisions
3. internal/llm/retry_observer.go (+132) — observes retry attempts from the SDK
4. internal/llm/retry_report.go (+658) — aggregates observations into the report

Integration points:

• internal/llm/client.go (+97) and internal/llm/responses_client.go (+27) — hook observers into the SDK clients
• internal/llmloop/loop.go (+70) — thread retry metadata through the loop
• internal/agent/agent.go (+40) — propagate to agents
• cmd/opencodereview/output.go (+79) and cmd/opencodereview/review_cmd.go (+32) — render the report