[Detail Bug] Streaming: Retry limiting and exponential backoff break after receiving an acknowledgment

[detail-app]

Detail Bug Report

https://app.detail.dev/org_89d327b3-b883-4365-b6a3-46b6701342a9/bugs/bug_22bb25e1-6c85-4bd2-af14-5b4d187bb7e7

Introduced in #1 by @quettabit on Apr 7, 2026

Summary

• Context: The attempt.value counter is reset to 0 after receiving any acknowledgment in streaming sessions.
• Bug: The attempt.value counter is reset to 0 after receiving any acknowledgment in streaming sessions, allowing retry attempts to exceed the configured max_attempts limit and breaking exponential backoff.
• Actual vs. expected: With max_attempts=3, actual calls observed: 6; expected max calls: 3. Backoff times do not increase exponentially (ratio ~1.0 instead of ~2.0).
• Impact: The bug allows retry attempts to exceed the configured max_attempts limit and breaks exponential backoff.

Code with Bug

# src/s2_sdk/_s2s/_append_session.py
if attempt.value > 0:
    attempt.value = 0  # <-- BUG 🔴 resets retry counter, defeating max_attempts/backoff
ack = pb.AppendAck()
ack.ParseFromString(msg_body)

# src/s2_sdk/_s2s/_read_session.py
if attempt.value > 0:
    attempt.value = 0  # <-- BUG 🔴 resets retry counter, defeating max_attempts/backoff
ack = pb.ReadAck()
ack.ParseFromString(msg_body)

Explanation

The streaming sessions reset the shared retry attempt counter after any ack, so the counter never increases monotonically across retries. This makes retry-limiting checks effectively repeat the same low attempt number (e.g., oscillating 0 → 1 → 0 → 1 ...), allowing more total attempts than the configured max_attempts and causing backoff to repeatedly use the smallest delay.

The reset also contradicts the meaning of existing logging that assumes a monotonic attempt counter:

logger.debug(
    "retrying append session: error=%s backoff=%.3fs retries_remaining=%d",
    e,
    backoff,
    max_retries - attempt.value - 1,
)

With the reset, retries_remaining does not decrease across retries, which is inconsistent with the intended semantics.

Codebase Inconsistency

Unary retry logic does not reset attempt counters:

# src/s2_sdk/_s2s/_retrier.py
attempt += 1  # Monotonic increase, never reset

This indicates Attempt is intended to be monotonic, and the streaming reset is inconsistent with the shared retrier behavior.

Recommended Fix

Remove the attempt reset in streaming sessions so the counter increases monotonically and both retry limiting and exponential backoff work as documented:

- if attempt.value > 0:
-     attempt.value = 0
  ack = pb.AppendAck()
  ack.ParseFromString(msg_body)

(Apply the same change in src/s2_sdk/_s2s/_read_session.py.)

History

This bug was introduced in commit 3dc9795. The commit added the initial implementation of the S2 SDK, including the streaming session retry logic. The reset of attempt.value to 0 after receiving an acknowledgment was part of the original design, likely intended to handle some edge case in stream resumption, but it inadvertently breaks the retry limiting contract by allowing the counter to oscillate instead of increasing monotonically. The bug slipped in because the massive initial commit (adding 100+ files) lacked tests that verify retry count limits are respected, and the complex interaction between acknowledgment handling and retry logic was not thoroughly validated.

[quettabit]

for append session, the semantics for retries is that its meant for consecutive failures rather than for all failures during the lifetime of the session.