[Detail Bug] Read sessions under-deliver non-command records when `ignore_command_records=True` and limits are set

[detail-app]

Detail Bug Report

https://app.detail.dev/org_89d327b3-b883-4365-b6a3-46b6701342a9/bugs/bug_45e1544a-79ea-4822-a2cf-1669525a3fba

Introduced in #34 by @quettabit on Apr 14, 2026

Summary

• Context: File with bug: src/s2_sdk/_s2s/_read_session.py (lines 97-115)
• Bug: When ignore_command_records=True, command records count against the user's limit even though they are filtered out and never returned to the caller.
• Actual vs. expected: With limit.count=10 and ignore_command_records=True, the user receives fewer than 10 records when command records are present. The user expects to receive up to 10 non-command records as documented ("Maximum number of records to return").
• Impact: Users cannot reliably limit the number of records they receive when filtering command records. The session terminates early, delivering fewer records than documented.

Code with Bug

In src/s2_sdk/_s2s/_read_session.py:

# Limit tracking BEFORE filtering (BUG)
if remaining_count is not None:
    remaining_count = max(remaining_count - len(batch.records), 0)  # <-- BUG 🔴 counts records that may be filtered out
    params["count"] = remaining_count
if remaining_bytes is not None:
    remaining_bytes = max(
        remaining_bytes - metered_bytes(batch.records), 0  # <-- BUG 🔴 counts bytes for records that may be filtered out
    )
    params["bytes"] = remaining_bytes

# Filtering AFTER limit tracking
if ignore_command_records:
    batch = ReadBatch(
        records=[r for r in batch.records if not r.is_command_record()],
        tail=batch.tail,
    )

# Yield filtered batch (may be empty)
if batch.records:
    yield batch

Explanation

read_session() decrements the remaining count/bytes based on the unfiltered server response, then filters out command records before yielding. If command records are present, the session reaches remaining_count == 0 sooner than it should and stops, even though fewer non-command records were actually returned.

This contradicts the SDK’s documented semantics that limit.count is the “Maximum number of records to return,” since some records counted toward the limit are never returned to the caller.

Concrete reproduction (simulated): with batches containing command records, requesting 10 records and ignoring command records can deliver only 6 non-command records.

Recommended Fix

Filter the batch first, then decrement remaining_count/remaining_bytes based on the filtered records (while keeping seq_num tracking based on the last server record):

• Keep seq_num advancement based on batch.records[-1] before filtering.
• Apply ignore_command_records filtering.
• Update remaining_count/remaining_bytes using the filtered batch.records.

History

This bug was introduced in commit d69e4f0. The commit intended to fix seq_num advancement after client-side filtering but moved filtering to occur after limit tracking, causing filtered-out command records to count against the user’s limits.

[quettabit]

this is the expected behavior, when client-side post filtering is enabled. can be good to call it out in docs though.