fix: CommandRecord.trim raises OverflowError instead of S2ClientError

[quettabit]

closes #8

[greptile-apps]

Greptile Summary

Adds @fallible to CommandRecord.trim so that OverflowError (raised by int.to_bytes when given a negative or oversized integer) is caught and re-raised as S2ClientError instead of leaking a raw Python exception — consistent with the pattern already used by Endpoints.from_env.

Confidence Score: 5/5

Safe to merge; the one-line fix is correct and consistent with the existing @Fallible pattern in the codebase.

The change is minimal and correct — @Fallible is already used elsewhere (e.g. Endpoints.from_env) and is purpose-built to convert non-S2Error exceptions into S2ClientError. The only finding is a P2 suggestion to add a regression test, which does not block merge.

No files require special attention.

Important Files Changed

Filename	Overview
src/s2_sdk/_types.py	Adds @fallible decorator to CommandRecord.trim so OverflowError from int.to_bytes is wrapped in S2ClientError; fix is correct and consistent with existing codebase patterns.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["CommandRecord.trim(desired_first_seq_num)"] --> B["@fallible wrapper"]
    B --> C["desired_first_seq_num.to_bytes(8, 'big')"]
    C -- "valid int (0 .. 2^64-1)" --> D["Return Record with TRIM header"]
    C -- "OverflowError\n(negative or too large)" --> E["fallible catches Exception"]
    E -- "not S2Error" --> F["raise S2ClientError(e)"]
    D --> G["Caller receives Record"]
    F --> H["Caller receives S2ClientError"]

Loading

Comments Outside Diff (1)

1. src/s2_sdk/_types.py, line 275-284 (link)

   Missing test for the fixed behaviour

   tests/test_validators.py already has test_fencing_token_rejects_too_long as a unit-level regression guard for CommandRecord.fence. A parallel test for CommandRecord.trim with an out-of-range value (e.g. -1) would lock in the fix and prevent regressions without requiring an integration environment.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: src/s2_sdk/_types.py
   Line: 275-284
   
   Comment:
   **Missing test for the fixed behaviour**
   
   `tests/test_validators.py` already has `test_fencing_token_rejects_too_long` as a unit-level regression guard for `CommandRecord.fence`. A parallel test for `CommandRecord.trim` with an out-of-range value (e.g. `-1`) would lock in the fix and prevent regressions without requiring an integration environment.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/s2_sdk/_types.py
Line: 275-284

Comment:
**Missing test for the fixed behaviour**

`tests/test_validators.py` already has `test_fencing_token_rejects_too_long` as a unit-level regression guard for `CommandRecord.fence`. A parallel test for `CommandRecord.trim` with an out-of-range value (e.g. `-1`) would lock in the fix and prevent regressions without requiring an integration environment.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "initial commit" | Re-trigger Greptile}