fix!: incorrect and inconsistent yield semantics in append_record_batches

[quettabit]

reverts #49

closes #82

[quettabit]

@greptileai review pls

[greptile-apps]

Greptile Summary

This PR removes the except Exception flush-on-error block from append_record_batches, fixing an inconsistency where a partial (not-yet-yielded) accumulator batch was emitted to the caller on a regular exception but silently discarded on CancelledError (which inherits from BaseException, not Exception). The docstring is updated to document the new contract.

• Behavior change: Records consumed but not yet yielded as a complete batch are now always discarded if iteration raises or is cancelled, instead of being flushed only on Exception.
• Test removal: The parametrized test_batch_accumulator_flushes_when_source_iter_raises test is removed; no replacement test for the new discard-on-error semantics is added.

Confidence Score: 5/5

The four-line removal is straightforward and the surviving finally-block correctly handles all cleanup paths.

The change fixes a real behavioral inconsistency (flush on Exception but not on CancelledError) and the updated docstring accurately reflects the new semantics. The surrounding logic — the inner accumulator loop, linger timer, and finally-block task cancellation — is unchanged and correct. The only gap is a missing replacement test for the new contract, which is a quality concern and not a functional defect.

tests/test_batching.py — the new discard-on-error contract has no test coverage.

Important Files Changed

Filename	Overview
src/s2_sdk/_batching.py	Removes the except-Exception flush block; the finally block for pending-task cleanup is preserved. Logic is sound and consistent with the documented contract.
tests/test_batching.py	Removes the test that verified the old flush-on-error behavior; no new test pins the discard-on-error contract, leaving a gap in behavioral coverage.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[outer loop: await next record] -->|record is None| B[break — no yield]
    A -->|record received| C[acc.add record]
    C --> D{linger > 0?}
    D -->|yes| E[inner loop: await task with timeout]
    D -->|no| F[inner loop: await anext]
    E -->|timeout / acc full| G[yield acc.take]
    F -->|record is None or acc full| G
    G --> A

    A -->|exception raised| H[finally: cancel pending task]
    H --> I[partial batch discarded]

    B --> J[finally: cancel pending task]
    J --> K[done]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[outer loop: await next record] -->|record is None| B[break — no yield]
    A -->|record received| C[acc.add record]
    C --> D{linger > 0?}
    D -->|yes| E[inner loop: await task with timeout]
    D -->|no| F[inner loop: await anext]
    E -->|timeout / acc full| G[yield acc.take]
    F -->|record is None or acc full| G
    G --> A

    A -->|exception raised| H[finally: cancel pending task]
    H --> I[partial batch discarded]

    B --> J[finally: cancel pending task]
    J --> K[done]

Loading

_{Reviews (2): Last reviewed commit: "as note" | Re-trigger Greptile}