Skip to content

feat(types): fix pagination cursor, billing fee, account type and wallet-create types - #64

Merged
ericviana merged 1 commit into
mainfrom
eric/fix-audit-defects
Aug 4, 2026
Merged

ericviana merged 1 commit into
mainfrom
eric/fix-audit-defects

Conversation

@ericviana

Copy link
Copy Markdown
Member

Summary

Fixes four type-surface defects surfaced by .api-sync/sync.py --audit-types (added in #58) and a follow-up manual review. These are type-surface changes for existing consumers, not new API surface -- in every case the wire contract already behaved this way; the SDK's declared type was simply wrong, and this PR makes the type match the (unchanged) reality. Anyone relying on the old, incorrect type is the one currently at risk (a str-typed field that was always secretly a number, an int-typed field that was always secretly a nullable id string, an enum value the API has never accepted), not the other way around.

The four fixes, quoting the contract for each

1. PaginationMetadata.next_page / .prev_page: int -> Optional[str]

Contract (packages/api-contract/src/generic.schema.ts): next_page: z.string().nullish(), example "pi_123". It's a pagination cursor id, not a page number. This is the same defect that makes the go/php/swift SDKs throw at runtime on any paginated list call; TypedDicts aren't runtime-enforced, so python silently accepted whatever the wire sent -- the declared type was just misleading every typed consumer (autocomplete, mypy/pyright checks on their own code) into treating it as an integer.

2. Payin.billing_fee_amount: Optional[str] -> Optional[float]

Spec: {"type": ["number", "null"], "description": "Billing fee in cents...", "example": 50}. Every other monetary _amount/_fee field in this SDK is float -- including Payout.billing_fee_amount, already correct. Payin.billing_fee_amount was the one outlier.

Chose Optional[float], matching the repo's own established, universal convention for monetary fields (verified: every _amount/_fee response field across quotes.py, payins/quotes.py, payins.py, transfers.py, payouts.py is float; none use int or str for money). No competing convention exists to override this.

Also added billing_fee_amount: NotRequired[Optional[float]] to GetPayinTrackResponse and CreateEvmPayinResponse. Correction to my own prior work: these map to the same wire schemas as Payin (PayinOut/CreatePayinOut respectively, both of which have this field), but did not model it at all before this PR -- not "modeled with the wrong type," simply absent. The .api-sync/unmodeled.json entry I wrote in #58 for CreatePayinOut.billing_fee_amount incorrectly asserted "Payin/GetPayinTrackResponse do model it"; only Payin did. Fixed now, correctly typed from the start, on all three.

3. BankAccountType: Literal["checking", "savings"] -> Literal["checking", "saving"]

Contract: accountType = ['checking', 'saving'] (singular). The API's own payout code branches on the literal string 'saving'. The plural "savings" is a value the API has never accepted -- not an alternate spelling choice, a value that fails.

4. CreateCustodialWalletInput: add name: str (required) and external_id: NotRequired[Optional[str]]

CreateWalletIn.required on the wire is ['network', 'name']; external_id is present and optional (["string","null"]). Before this PR, CreateCustodialWalletInput modeled only customer_id and network -- every custodial wallet creation through this SDK was rejected by the API for a missing required field. name is a bare required key here (not NotRequired) because the API genuinely requires it; unlike a normal "new optional field on an already-published TypedDict" case, there is no previously-working caller to protect, since every prior call was already failing server-side. node/go/swift already send name; python and php did not.

No method code change was needed: create() already forwards every non-customer_id key verbatim (payload = {k: v for k, v in data.items() if k != "customer_id"}), so name/external_id now flow through automatically once the type carries them -- proven by the updated mock_request.assert_called_once_with(...) body assertions in test_custodial_wallets.py.

unmodeled.json cleanup

Removed the four now-fixed entries (stale records misdirect the next reader):

  • {"kind": "enum", "enum": "BankAccountType", "missing_values": ["saving"]}
  • {"kind": "property", "schema": "CreateWalletIn", "field": "name"}
  • {"kind": "property", "schema": "CreateWalletIn", "field": "external_id"}
  • {"kind": "property", "schema": "CreatePayinOut", "field": "billing_fee_amount"}

KycStatus's divergence (2 of 8 members modeled) is untouched -- separate, still undecided, not part of this PR.

142 -> 138 entries.

--audit-types delta

BEFORE (316 findings):
  PayinOut.billing_fee_amount (Payin in .../payins/payins.py): spec type `number` (expected `float`) but SDK annotation is `str` [annotation: Optional[str]]
  PaginationMetadata.next_page (PaginationMetadata in .../types.py): spec is nullable but SDK annotation `int` has no Optional[...] [annotation: int]
  PaginationMetadata.prev_page (PaginationMetadata in .../types.py): spec is nullable but SDK annotation `int` has no Optional[...] [annotation: int]

AFTER (313 findings):
  (zero matches for billing_fee_amount / next_page / prev_page)

Drop of exactly 3 -- exactly the type-mismatch findings this PR fixes. BankAccountType and the wallet-create fields are presence/enum-divergence issues tracked in unmodeled.json, not --audit-types findings (that tool only flags type mismatches on properties the SDK already models); those are the 4 entries removed from unmodeled.json above.

sync.py --check stays exit 0 (silent) before and after -- these are hand-authored type corrections, not spec-drift the patcher would have applied differently.

Tests

  • New tests/test_types.py: explicit TypedDict-annotated literals for all four fixes, so pyright/mypy (both run over tests/) enforce the shape at the assignment, not just a runtime equality check -- a string next_page, a numeric billing_fee_amount (plus proof the field is optional on GetPayinTrackResponse/CreateEvmPayinResponse), "saving" as a valid BankAccountType, and name/external_id on CreateCustodialWalletInput.
  • Updated test_payouts.py/test_payins.py: pagination fixtures now use string cursors ("pi_123") instead of integers.
  • Updated test_custodial_wallets.py: both test_create_custodial_wallet (async + sync) now pass name and assert it's in the actual POST body sent; new test_create_custodial_wallet_with_external_id proves external_id flows through when supplied.

Proof

$ uv sync --group dev --group test
Resolved 28 packages ... Checked 27 packages

$ uv run ruff format --check .
72 files already formatted

$ uv run ruff check .
All checks passed!

$ uv run pyright
0 errors, 0 warnings, 0 informations

$ uv run mypy .
Success: no issues found in 69 source files

$ uv run pytest --tb=short -q
...
215 passed in 1.00s

$ python3 .api-sync/check_contract.py
Checked 1347 declared TypedDict fields across 175 classes against 527 known wire keys.
Direction B (fields) -- warning only: 232 spec property name(s) are not declared by any SDK TypedDict.
Direction A and B: PASSED.

$ python3 .api-sync/sync.py --validate-map
Map validity: OK

$ python3 .api-sync/sync.py --check
(silent, exit 0)

Determinism (two independent copies of this branch, sync.py --apply --spec .api-sync/spec-snapshot.json re-run in each -- unaffected by these hand-authored fixes, confirming the patcher itself is untouched and still behaves correctly):

No changes.
No changes.
$ diff -rq /tmp/fix-det-a /tmp/fix-det-b
(no output -- byte-identical)

Version bump

Title uses feat: to produce a minor bump (these change public TypedDict/Literal shapes, not just internal behavior). Not fix: (patch) because the task explicitly calls for minor given the type-surface change, and not feat!:/breaking (these correct wrong types to match an unchanged wire contract; nothing that previously worked correctly stops working).

Per the working rules: fresh clone, branch off current main (post #58/#59/#60/#61/#62), not merged, opening for review and stopping here.

https://claude.ai/code/session_01F1stiNzuNtJXoXtiW9ZCbs

…let-create types

Four type-surface defects surfaced by .api-sync/sync.py --audit-types and a
manual review, all fixed here. These are corrections to match the existing,
unchanged API contract, not new API surface -- the wire behavior was never
what the old types claimed.

1. PaginationMetadata.next_page/prev_page: int -> Optional[str]. The contract
   (packages/api-contract/src/generic.schema.ts) declares
   `next_page: z.string().nullish()` (example "pi_123"), a cursor id, not a
   page number. This is the same defect that makes the go/php/swift SDKs
   throw at runtime; TypedDicts aren't enforced so python silently returned
   whatever the wire sent, mistyped.

2. Payin.billing_fee_amount: Optional[str] -> Optional[float]. The spec types
   it `["number","null"]` (cents, example 50). Every other monetary
   `_amount`/`_fee` field in this SDK (Payout.billing_fee_amount included) is
   already `float`; Payin's was the one outlier. Also added
   `billing_fee_amount: NotRequired[Optional[float]]` to GetPayinTrackResponse
   and CreateEvmPayinResponse, which map to the same wire schemas
   (PayinOut/CreatePayinOut) and did not model the field at all before this --
   my own Phase A unmodeled.json entry incorrectly claimed
   "Payin/GetPayinTrackResponse do model it"; only Payin did. Corrected now
   that the field exists everywhere the wire sends it.

3. BankAccountType: Literal["checking","savings"] -> Literal["checking","saving"].
   The contract's accountType enum is ['checking','saving'] (singular); the
   API's own payout code branches on the literal string 'saving'. The old
   plural value is a value the API rejects outright, not an alternate
   spelling.

4. CreateCustodialWalletInput: added `name: str` (required -- CreateWalletIn.
   required includes it on the wire, so this is the one place a bare
   required key is correct) and `external_id: NotRequired[Optional[str]]`
   (optional on the wire). Before this, the input modeled only customer_id
   and network, so every custodial wallet creation through this SDK was
   rejected by the API. No method code change needed: create() already
   forwards every non-customer_id key verbatim, so name/external_id now flow
   through once the type carries them. node/go/swift already send name;
   python and php did not.

Removed the four now-fixed entries from .api-sync/unmodeled.json (the
BankAccountType enum divergence, CreateWalletIn.name, CreateWalletIn.
external_id, CreatePayinOut.billing_fee_amount) -- keeping a stale record
after the underlying field is fixed would misdirect the next person who
reads it. KycStatus's divergence is untouched; it is a separate, undecided
gap.

.api-sync/sync.py --audit-types drops from 316 to 313 findings (exactly the
3 type-mismatch findings this fixes: next_page, prev_page,
Payin.billing_fee_amount -- BankAccountType and the wallet-create fields are
presence/enum-divergence issues tracked in unmodeled.json, not audit-types
findings, and both drop out of that file's 142 -> 138 entries). sync.py
--check stays exit 0.

tests/test_types.py is new: explicit TypedDict-annotated literals for each
fix, so pyright/mypy (both run over tests/) enforce the shape at the
assignment itself. Updated test_payouts.py/test_payins.py's pagination
fixtures from integer to string cursors, and test_custodial_wallets.py's
create() calls to send name (required) with a new test proving external_id
flows through when supplied.

Claude-Session: https://claude.ai/code/session_01F1stiNzuNtJXoXtiW9ZCbs
@BernardoSM

Copy link
Copy Markdown
Collaborator

✅ Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
✅ Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@ericviana
ericviana merged commit 767dd62 into main Aug 4, 2026
8 checks passed
@ericviana
ericviana deleted the eric/fix-audit-defects branch August 4, 2026 00:40
ericviana added a commit that referenced this pull request Aug 4, 2026
Literal ended with "2500000000_plus" (2.5 billion). The spec's enum on
CreateCustomerIn, UpdateCustomerIn, CustomerOut and both customer webhook
schemas is:
["0_99999", "100000_999999", "1000000_9999999", "10000000_49999999",
 "50000000_249999999", "250000000_plus"]

250000000_plus (250 million) is the only coherent reading: the band directly
below it tops out at 249999999, so the top band has to start at 250000000.
A business customer selecting the top revenue band sends a value the API
rejects -- the same class of live defect as account_type/BankAccountType
(#64).

Why .api-sync/sync.py's enum reconciliation did not catch this on its own:
it did not compare at all. EstimatedAnnualRevenue was never added to
spec-map.json's `enums` list during Phase A (#58) -- that list covers 17
shared, broadly-reused Literals, not the long tail of customer-domain-
specific ones customers.py declares (CustomerBusinessType, BusinessIndustry,
SourceOfWealth, TaxType, AmlStatus, ProofOfAddressDocType,
PurposeOfTransactions, SourceOfFundsDocType, and others -- roughly 15 more
Literals with no map entry at all). This is a map-coverage gap, not a
comparison-logic flaw: reconcile_enums only ever inspects what
spec-map.json lists, so an unmapped Literal is invisible to it regardless of
whether it has one extra member, one missing member, or both. Not
redesigning or expanding map coverage in this PR -- noting it as a real,
separate gap for a future pass.

Claude-Session: https://claude.ai/code/session_01F1stiNzuNtJXoXtiW9ZCbs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants