fix(agent): add explicit provider retry lifecycle

[yyhhyyyyyy]

Summary

Make DeepChat the explicit owner of provider retries and add durable attempt provenance across the agent loop, Tape, and message traces.

Transient failures are retried only before semantic output is committed. Context recovery remains a separate payload-changing flow and does not consume the logical-round or transient-retry budget.

Changes

• Model provider execution with three explicit identities:
  • logicalRound: one model response and tool-settlement cycle
  • requestSeq: one immutable provider payload and ViewManifest
  • physicalAttempt: one actual send of that request
• Add an internal transient retry policy:
  • at most two retries per logical round
  • abortable exponential backoff with downward jitter
  • bounded Retry-After handling
  • rate-gate admission before every physical attempt
• Classify provider failures as aborted, context overflow, permanent, transient, or unknown.
• Buffer usage, stop, and error control events until the retry decision is final.
• Prevent transparent replay after text, reasoning, tool, permission, image, plan, or rate-limit output.
• Treat premature EOF before semantic output as retryable; preserve partial output and fail without replay after output is committed.
• Aggregate checked usage across physical attempts while retaining attempt-local usage in Tape.
• Propagate the run AbortSignal through AI SDK, Ollama, GitHub Copilot, Voice, and ACP streams.
• Disable hidden AI SDK retries for chat streams and non-idempotent media paths; retain explicit retries for one-shot text and embeddings.
• Add structured retry lifecycle diagnostics without introducing renderer state or another durable source of truth.
• Write provider/attempt_completed Tape records using schema v2 while retaining v1 compatibility.
• Add migration v45 for nullable logical-round and physical-attempt trace identity.
• Select the newest physical attempt during trace replay.
• Correct logical-round limit and resume accounting without changing public metadata field names.

Compatibility

• Existing providerRounds and maxProviderRounds names are unchanged.
• Tape schema v1 remains readable.
• Migration v45 is additive; existing trace rows remain valid with nullable identity.
• ACP traces remain compatible.
• No Session or UI retry configuration is introduced.
• ACP, image, video, and TTS requests are never transparently replayed.

Summary by CodeRabbit

• New Features

  • Improved provider retry handling with bounded backoff and clearer recovery behavior.
  • Added structured error details for retryable failures, rate limits, cancellations, and content filtering.
  • Improved cancellation support across streaming text, image, video, audio, and voice operations.
  • Enhanced replay and diagnostics to consistently identify individual generation attempts.

• Bug Fixes

  • Prevented retries after meaningful output has already been delivered.
  • Improved usage and cache metrics when requests require multiple attempts.

• Documentation

  • Updated architecture and retry lifecycle documentation to reflect current behavior.

[coderabbitai]

📝 Walkthrough

Walkthrough

DeepChat now models logical rounds separately from provider request sequences and physical attempts. Coordinator-owned retries use bounded backoff, structured failure metadata, abort propagation, per-attempt Tape provenance, and deterministic trace replay selection, with updated runtime contracts, migrations, documentation, and tests.

Changes

Provider retry lifecycle and attempt provenance

Layer / File(s)	Summary
Logical-round and attempt identity src/main/agent/deepchat/loop/*, src/main/agent/deepchat/runtime/*, src/shared/types/provider-attempt.ts	Execution accounting now uses logicalRound, requestSeq, and physicalAttempt; loop callbacks and persisted provider-round metadata use logical rounds.
Retry policy and orchestration src/main/agent/deepchat/loop/contextCoordinator.ts, src/main/agent/deepchat/loop/providerRetryPolicy.ts	Provider attempts are classified, retried, delayed, settled, and usage-aggregated through centralized retry policy logic with observer events.
Provider streaming and failure boundary src/main/provider/*, src/shared/types/core/llm-events.ts, src/shared/types/provider.ts	Streaming APIs propagate AbortSignal, AI SDK retry settings are explicit, and provider failures emit sanitized structured metadata.
Tape and trace persistence src/main/tape/*, src/main/session/*, src/shared/types/tape-replay.ts	Tape schema v2, physical-attempt provenance keys, SQLite migration v45, nullable trace identity fields, and latest-attempt replay ordering are implemented.
Documentation and validation docs/architecture/*, docs/issues/provider-retry-lifecycle/spec.md, test/*	Architecture contracts and tests cover retry limits, replay boundaries, cancellation, usage aggregation, failure sanitization, migration compatibility, and trace selection.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

• ThinkInAIXYZ/deepchat#847: Related to the structured LLM error event and failure metadata types.
• ThinkInAIXYZ/deepchat#1962: Related to provider-attempt retry handling in contextCoordinator.ts.

Suggested reviewers: zerob13

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 1.89% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the PR’s main change: adding an explicit provider retry lifecycle for the agent.
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.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/provider-retry-lifecycle

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/main/provider/providers/acpProvider.ts (1)

373-431: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Aborting mid-catch can skip ACP session cleanup, leaking the session/permissions.

signal?.throwIfAborted() at Line 426 runs before queue.push/queue.done(). If session was already assigned by sessionController.open(...) and a later synchronous step (e.g. messageFormatter.format) throws while the signal happens to be aborted, this throw propagates out of coreStream immediately — bypassing the second try { while… } finally { … } block entirely. That finally is the only place that cancels the ACP connection, calls clearMappedSession, and resolves pending permissions, so none of that cleanup runs.

🔧 Proposed fix: guarantee cleanup regardless of where the throw originates

-    try {
-      const acpEnabled = await this.agentSettings.getAcpEnabled()
-      ...
-    } catch (error) {
-      signal?.throwIfAborted()
-      const message =
-        error instanceof Error ? error.message : typeof error === 'string' ? error : 'Unknown error'
-      queue.push(createStreamEvent.error(`ACP: ${message}`, extractProviderFailureMetadata(error)))
-      queue.done()
-    }
-
-    try {
-      while (true) {
-        const event = await queue.next()
-        if (event === null) break
-        yield event
-      }
-      signal?.throwIfAborted()
-    } finally {
-      if (session) {
-        ...
-      }
-    }
+    try {
+      const acpEnabled = await this.agentSettings.getAcpEnabled()
+      ...
+    } catch (error) {
+      const message =
+        error instanceof Error ? error.message : typeof error === 'string' ? error : 'Unknown error'
+      queue.push(createStreamEvent.error(`ACP: ${message}`, extractProviderFailureMetadata(error)))
+      queue.done()
+      signal?.throwIfAborted()
+    }
+
+    try {
+      while (true) {
+        const event = await queue.next()
+        if (event === null) break
+        yield event
+      }
+      signal?.throwIfAborted()
+    } finally {
+      if (session) {
+        ...
+      }
+    }

Moving signal?.throwIfAborted() to after the queue is drained/marked done ensures cleanup is always reachable, and abort is still surfaced via the second block's own throwIfAborted() check.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/provider/providers/acpProvider.ts` around lines 373 - 431, In the
catch block around session setup and prompt formatting, move
signal?.throwIfAborted() until after queue.push(createStreamEvent.error(...))
and queue.done(). Preserve the existing error-message and metadata handling,
ensuring aborted errors do not escape before the downstream cleanup finally
block can cancel the ACP session, clear the mapped session, and resolve pending
permissions.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/main/provider/providers/acpProvider.ts`:
- Around line 373-431: In the catch block around session setup and prompt
formatting, move signal?.throwIfAborted() until after
queue.push(createStreamEvent.error(...)) and queue.done(). Preserve the existing
error-message and metadata handling, ensuring aborted errors do not escape
before the downstream cleanup finally block can cancel the ACP session, clear
the mapped session, and resolve pending permissions.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4a824170-52e4-432b-8d60-0caa3e8c2786

📥 Commits

Reviewing files that changed from the base of the PR and between 298bb85 and 5da33b8.

📒 Files selected for processing (57)

• docs/architecture/agent-system.md
• docs/architecture/cache-aware-context-runtime/spec.md
• docs/architecture/tape-system.md
• docs/issues/provider-retry-lifecycle/spec.md
• src/main/agent/deepchat/loop/contextCoordinator.ts
• src/main/agent/deepchat/loop/deepChatLoopEngine.ts
• src/main/agent/deepchat/loop/loopRun.ts
• src/main/agent/deepchat/loop/providerRetryPolicy.ts
• src/main/agent/deepchat/runtime/deepChatLoopRunner.ts
• src/main/agent/deepchat/runtime/process.ts
• src/main/agent/deepchat/runtime/types.ts
• src/main/provider/aiSdk/runtime.ts
• src/main/provider/aiSdk/streamAdapter.ts
• src/main/provider/baseProvider.ts
• src/main/provider/index.ts
• src/main/provider/providerFailure.ts
• src/main/provider/providers/acpProvider.ts
• src/main/provider/providers/aiSdkProvider.ts
• src/main/provider/providers/githubCopilotProvider.ts
• src/main/provider/providers/ollamaProvider.ts
• src/main/provider/providers/voiceAIProvider.ts
• src/main/session/data/tables/deepchatMessageTraces.ts
• src/main/session/data/transcript.ts
• src/main/session/query.ts
• src/main/tape/application/providerAttemptService.ts
• src/main/tape/application/viewReplayService.ts
• src/main/tape/domain/providerAttempt.ts
• src/main/tape/ports/application.ts
• src/shared/types/agent-interface.d.ts
• src/shared/types/core/llm-events.ts
• src/shared/types/provider-attempt.ts
• src/shared/types/provider.ts
• src/shared/types/tape-replay.ts
• test/main/agent/deepchat/loop/contextCoordinator.test.ts
• test/main/agent/deepchat/loop/deepChatLoopEngine.test.ts
• test/main/agent/deepchat/loop/loopRun.test.ts
• test/main/agent/deepchat/loop/providerRetryPolicy.test.ts
• test/main/agent/deepchat/runtime/deepChatRuntimeCoordinator.test.ts
• test/main/agent/deepchat/runtime/process.test.ts
• test/main/provider/acpProvider.test.ts
• test/main/provider/aiSdkRuntime.test.ts
• test/main/provider/aiSdkStreamAdapter.test.ts
• test/main/provider/coreEvents.test.ts
• test/main/provider/githubCopilotProvider.test.ts
• test/main/provider/ollamaProviderCancellation.test.ts
• test/main/provider/openAICompatibleProvider.test.ts
• test/main/provider/providerFailure.test.ts
• test/main/provider/providerRuntime.test.ts
• test/main/provider/voiceAIProvider.test.ts
• test/main/session/data/tables/deepchatMessageTraces.test.ts
• test/main/session/data/tapeRecall.test.ts
• test/main/session/data/tapeTestHarness.ts
• test/main/session/data/tapeViewReplay.test.ts
• test/main/session/data/transcript.test.ts
• test/main/session/query.test.ts
• test/main/session/runtimeIntegration.test.ts
• test/main/session/session.integration.test.ts