Uh oh!
There was an error while loading. Please reload this page.
chore: Sync account schemas - #374
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✱ Stainless preview buildsThis PR will update the kotlin openapi python typescript Edit this comment to update them. They will appear in their respective SDK's changelogs. ✅ grid-openapistudio · code · diff
✅ grid-kotlinstudio · code · diff
✅ grid-pythonstudio · code · diff
✅ grid-typescriptstudio · code · diff
This comment is auto-generated by GitHub Actions and is automatically kept up to date as you push. |
Greptile SummaryThis auto-synced PR flattens all
Confidence Score: 4/5Safe to merge only after confirming the backend already enforces the new required fields and all API consumers are prepared for the breaking changes. Two P1 concerns: new required fields on
|
| Filename | Overview |
|---|---|
| openapi/components/schemas/common/UsdAccountInfo.yaml | Flattened from allOf+Base to a standalone schema; adds paymentRails, bankName, phoneNumber as required, and adds MOBILE_MONEY as a payment rail — notable for a USD schema. |
| openapi/components/schemas/common/BwpBeneficiary.yaml | Promotes nationality to required — a breaking change for callers submitting BWP beneficiaries without this field. Same pattern applies to 9 other beneficiary schemas. |
| openapi/components/schemas/common/CopBeneficiary.yaml | Reorders address before documentType/documentNumber — cosmetic property ordering change, no functional impact. |
| openapi.yaml | Bundled spec updated in sync with component YAML changes; removes all *AccountInfoBase named schemas and flattens them into the *AccountInfo schemas. Mirrors mintlify/openapi.yaml exactly. |
| mintlify/openapi.yaml | Kept in exact sync with openapi.yaml — no divergence detected. |
| openapi/components/schemas/common/BdtAccountInfo.yaml | Flattened to standalone schema; adds bankName, accountNumber, branchCode (required), swiftCode (optional), phoneNumber (required), with correct regex patterns. |
| openapi/components/schemas/common/EgpAccountInfo.yaml | Flattened to standalone schema; adds bankName, accountNumber (required), iban and swiftCode (optional) with correct IBAN and SWIFT regex patterns. |
| openapi/components/schemas/common/ZarBeneficiary.yaml | Promotes nationality to required — breaking change for ZAR beneficiary submissions missing this field. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[sparkcore VASP adapter definitions] -->|auto-sync| B[common/*AccountInfo.yaml]
A -->|auto-sync| C[common/*Beneficiary.yaml]
B -->|used by| D[PaymentInstructions oneOf]
C -->|used by| E[*ExternalAccountCreateInfo.yaml]
F[common/*AccountInfoBase.yaml\nunchanged] -->|$ref| E
E -->|bundled into| G[openapi.yaml]
G -->|copied to| H[mintlify/openapi.yaml]
B -->|flattened allOf removed| I["Before: allOf[*Base + paymentRails]"]
B -->|now standalone| J["After: flat type:object with all fields"]
style I fill:#f9a,stroke:#f00
style J fill:#afa,stroke:#090
Prompt To Fix All With AI
This is a comment left during a code review.
Path: openapi/components/schemas/common/UsdAccountInfo.yaml
Line: 19-26
Comment:
**`MOBILE_MONEY` added to USD payment rails**`MOBILE_MONEY` has been added as a valid payment rail for `UsdAccountInfo`, alongside the existing US-specific rails (`ACH`, `WIRE`, `RTP`, `FEDNOW`, `BANK_TRANSFER`). USD + mobile money is an unusual pairing — traditional US dollar payments don't go over mobile money networks. If this is intended to cover specific corridors (e.g., El Salvador or other dollarized economies), it's worth confirming that the backend already supports this rail for USD and that the discriminator logic downstream handles it correctly.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: openapi/components/schemas/common/UsdAccountInfo.yaml
Line: 2-8
Comment:
**New required fields may be a breaking schema change**`phoneNumber` and `bankName` are newly required in `UsdAccountInfo`. Previously, `UsdAccountInfo` (via `UsdAccountInfoBase` + `paymentRails`) did not mandate these fields. Any existing API consumer that reads or validates against `UsdAccountInfo` (e.g., in `PaymentInstructions`) without these fields will now fail schema validation. Please confirm the backend always returns both fields for USD account info payloads before merging, or consider making them optional for backward compatibility.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: openapi/components/schemas/common/BwpBeneficiary.yaml
Line: 4-6
Comment:
**`nationality` promoted to required — potential breaking change**`nationality` is now required on `BwpBeneficiary` (and similarly on `ZarBeneficiary`, `KesBeneficiary`, `MwkBeneficiary`, `NgnBeneficiary`, `RwfBeneficiary`, `TzsBeneficiary`, `UgxBeneficiary`, `XafBeneficiary`, `XofBeneficiary`, `ZmwBeneficiary`). This is a breaking change for callers creating or submitting beneficiaries for these currencies who don't currently send `nationality`. Please confirm the backend enforces this field and that all existing stored beneficiaries for these currencies will continue to pass validation.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "chore: Sync account schemas" | Re-trigger Greptile
| - ACH | ||
| - WIRE | ||
| - RTP | ||
| - FEDNOW | ||
| - BANK_TRANSFER | ||
| - MOBILE_MONEY | ||
| accountNumber: | ||
| type: string |
There was a problem hiding this comment.
MOBILE_MONEY added to USD payment rails
MOBILE_MONEY has been added as a valid payment rail for UsdAccountInfo, alongside the existing US-specific rails (ACH, WIRE, RTP, FEDNOW, BANK_TRANSFER). USD + mobile money is an unusual pairing — traditional US dollar payments don't go over mobile money networks. If this is intended to cover specific corridors (e.g., El Salvador or other dollarized economies), it's worth confirming that the backend already supports this rail for USD and that the discriminator logic downstream handles it correctly.
Prompt To Fix With AI
This is a comment left during a code review.
Path: openapi/components/schemas/common/UsdAccountInfo.yaml
Line: 19-26
Comment:
**`MOBILE_MONEY` added to USD payment rails**`MOBILE_MONEY` has been added as a valid payment rail for `UsdAccountInfo`, alongside the existing US-specific rails (`ACH`, `WIRE`, `RTP`, `FEDNOW`, `BANK_TRANSFER`). USD + mobile money is an unusual pairing — traditional US dollar payments don't go over mobile money networks. If this is intended to cover specific corridors (e.g., El Salvador or other dollarized economies), it's worth confirming that the backend already supports this rail for USD and that the discriminator logic downstream handles it correctly.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| required: | ||
| - accountType | ||
| - paymentRails | ||
| - accountNumber | ||
| - routingNumber | ||
| - bankName | ||
| - phoneNumber |
There was a problem hiding this comment.
phoneNumber and bankName are newly required in UsdAccountInfo. Previously, UsdAccountInfo (via UsdAccountInfoBase + paymentRails) did not mandate these fields. Any existing API consumer that reads or validates against UsdAccountInfo (e.g., in PaymentInstructions) without these fields will now fail schema validation. Please confirm the backend always returns both fields for USD account info payloads before merging, or consider making them optional for backward compatibility.
Prompt To Fix With AI
This is a comment left during a code review.
Path: openapi/components/schemas/common/UsdAccountInfo.yaml
Line: 2-8
Comment:
**New required fields may be a breaking schema change**`phoneNumber` and `bankName` are newly required in `UsdAccountInfo`. Previously, `UsdAccountInfo` (via `UsdAccountInfoBase` + `paymentRails`) did not mandate these fields. Any existing API consumer that reads or validates against `UsdAccountInfo` (e.g., in `PaymentInstructions`) without these fields will now fail schema validation. Please confirm the backend always returns both fields for USD account info payloads before merging, or consider making them optional for backward compatibility.
How can I resolve this? If you propose a fix, please make it concise.| - beneficiaryType | ||
| - fullName | ||
| - nationality |
There was a problem hiding this comment.
nationality promoted to required — potential breaking change
nationality is now required on BwpBeneficiary (and similarly on ZarBeneficiary, KesBeneficiary, MwkBeneficiary, NgnBeneficiary, RwfBeneficiary, TzsBeneficiary, UgxBeneficiary, XafBeneficiary, XofBeneficiary, ZmwBeneficiary). This is a breaking change for callers creating or submitting beneficiaries for these currencies who don't currently send nationality. Please confirm the backend enforces this field and that all existing stored beneficiaries for these currencies will continue to pass validation.
Prompt To Fix With AI
This is a comment left during a code review.
Path: openapi/components/schemas/common/BwpBeneficiary.yaml
Line: 4-6
Comment:
**`nationality` promoted to required — potential breaking change**`nationality` is now required on `BwpBeneficiary` (and similarly on `ZarBeneficiary`, `KesBeneficiary`, `MwkBeneficiary`, `NgnBeneficiary`, `RwfBeneficiary`, `TzsBeneficiary`, `UgxBeneficiary`, `XafBeneficiary`, `XofBeneficiary`, `ZmwBeneficiary`). This is a breaking change for callers creating or submitting beneficiaries for these currencies who don't currently send `nationality`. Please confirm the backend enforces this field and that all existing stored beneficiaries for these currencies will continue to pass validation.
How can I resolve this? If you propose a fix, please make it concise.| - FEDNOW | ||
| - BANK_TRANSFER | ||
| type: object | ||
| required: |
There was a problem hiding this comment.
@AaryamanBhute this looks fishy. I had to add some fields for colombia and other thunes corridors, but the el salvador ones (which transact in USD) are adding some things here. Im concerned these fields will be a breaking change, what do you think?
There was a problem hiding this comment.
as long as these are optional, I don't think they're a breaking chage. Looks like these are optional right?
There was a problem hiding this comment.
https://github.com/lightsparkdev/webdev/blob/9374a2634a00091b752dd12c06884a1aea633673/sparkcore/sparkcore/bridge/extend_integration/thunes_fields_provider.py#L43 yeah I think it's baed on these? I have to update it anyway though, el salvador is just going to use mobile money
There was a problem hiding this comment.
something wierd is going on with the generator, it shouldn't be inlining these objects - investigating
AaryamanBhute
left a comment
There was a problem hiding this comment.
wait for generator investigation
Superseded by #383 |
Auto-synced account schemas.
These schemas are generated from VASP adapter field definitions in sparkcore.
Synced schemas:
common/— per-currency account info, beneficiary, and payment account schemascommon/PaymentInstructions.yaml— payment instructions oneOf (new currencies added)external_accounts/— per-currency external account schemas (reference common/)Please review the changes before merging.