feat(customers): split BusinessInfo response schema (taxId/incorporatedOn optional)#524
Conversation
…edOn optional) Adds BusinessInfoResponse with taxId and incorporatedOn optional so legacy customers with null values deserialize cleanly. The request-side BusinessInfo schema keeps both fields required so KYB creation continues to enforce them. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
✱ Stainless preview builds for gridThis PR will update the cli csharp go kotlin openapi php python ruby typescript
|
Greptile SummaryThis PR introduces a
Confidence Score: 3/5Safe to merge after verifying whether the server sends null or omits the fields entirely for legacy records — the answer determines whether the nullable annotations are required. The read/write schema split is the correct approach and BusinessCustomer.yaml is wired up cleanly. The uncertainty is in BusinessInfoResponse.yaml: the DB has null values for taxId/incorporatedOn, and if the API serialises those rows with explicit null values (rather than omitting the keys), OAS 3.1 clients will still reject the response because null is not a valid string. The downstream removal of none_throws() calls suggests the server does return null today. openapi/components/schemas/customers/BusinessInfoResponse.yaml — specifically the taxId and incorporatedOn field definitions.
|
| Filename | Overview |
|---|---|
| openapi/components/schemas/customers/BusinessInfoResponse.yaml | New read-model schema making taxId and incorporatedOn optional — needs nullable annotations for both fields since legacy DB rows carry null (not absent) values. |
| openapi/components/schemas/customers/BusinessCustomer.yaml | One-line ref change from BusinessInfo to BusinessInfoResponse — correct and clean. |
| openapi.yaml | Regenerated bundle reflecting source changes; both BusinessInfoResponse and original BusinessInfo schemas are present with correct required fields. |
| mintlify/openapi.yaml | Mintlify bundle regenerated in sync with openapi.yaml — no independent issues. |
Class Diagram
%%{init: {'theme': 'neutral'}}%%
classDiagram
class BusinessInfo {
+string legalName [required]
+string taxId [required]
+string incorporatedOn [required]
+string doingBusinessAs
+string country
}
class BusinessInfoResponse {
+string legalName [required]
+string taxId [optional]
+string incorporatedOn [optional]
+string doingBusinessAs
+string country
}
class BusinessCustomer {
+BusinessInfoResponse businessInfo
+BeneficialOwner[] beneficialOwners
}
class BusinessCustomerCreateRequest {
+BusinessInfo businessInfo
}
BusinessCustomer --> BusinessInfoResponse : read (response)
BusinessCustomerCreateRequest --> BusinessInfo : write (request)
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
openapi/components/schemas/customers/BusinessInfoResponse.yaml:32-35
**Nullable vs. absent — `taxId` may need `type: 'null'` in OAS 3.1**
The PR description notes that legacy DB rows have `null` values for `tax_id` and `incorporated_on`. Removing these fields from `required` allows them to be *absent*, but in OpenAPI 3.1.0 (which this spec uses) an absent field is not the same as a `null` value — `null` is only valid when the schema explicitly permits it. If the API serialises legacy rows by writing `taxId: null` rather than omitting the key, any strictly-conforming OAS 3.1 client or validator will still reject the response. The existing codebase pattern (see `Card.yaml` `stateReason`) uses `oneOf: [{...}, {type: 'null'}]` for exactly this case.
```suggestion
taxId:
oneOf:
- type: string
description: Tax identification number
example: 47-1234567
- type: 'null'
```
### Issue 2 of 2
openapi/components/schemas/customers/BusinessInfoResponse.yaml:25-29
**`incorporatedOn` also needs nullable annotation**
Same issue as `taxId`: if legacy records serialise this field as `null` rather than omitting it, OAS 3.1 validators and generated clients will reject the response because `null` is not a valid `date`-formatted string.
```suggestion
incorporatedOn:
oneOf:
- type: string
format: date
description: Date of incorporation in ISO 8601 format (YYYY-MM-DD)
example: '2018-03-14'
- type: 'null'
```
Reviews (1): Last reviewed commit: "feat(customers): split BusinessInfo resp..." | Re-trigger Greptile
| taxId: | ||
| type: string | ||
| description: Tax identification number | ||
| example: 47-1234567 |
There was a problem hiding this comment.
Nullable vs. absent —
taxId may need type: 'null' in OAS 3.1
The PR description notes that legacy DB rows have null values for tax_id and incorporated_on. Removing these fields from required allows them to be absent, but in OpenAPI 3.1.0 (which this spec uses) an absent field is not the same as a null value — null is only valid when the schema explicitly permits it. If the API serialises legacy rows by writing taxId: null rather than omitting the key, any strictly-conforming OAS 3.1 client or validator will still reject the response. The existing codebase pattern (see Card.yaml stateReason) uses oneOf: [{...}, {type: 'null'}] for exactly this case.
| taxId: | |
| type: string | |
| description: Tax identification number | |
| example: 47-1234567 | |
| taxId: | |
| oneOf: | |
| - type: string | |
| description: Tax identification number | |
| example: 47-1234567 | |
| - type: 'null' |
Prompt To Fix With AI
This is a comment left during a code review.
Path: openapi/components/schemas/customers/BusinessInfoResponse.yaml
Line: 32-35
Comment:
**Nullable vs. absent — `taxId` may need `type: 'null'` in OAS 3.1**
The PR description notes that legacy DB rows have `null` values for `tax_id` and `incorporated_on`. Removing these fields from `required` allows them to be *absent*, but in OpenAPI 3.1.0 (which this spec uses) an absent field is not the same as a `null` value — `null` is only valid when the schema explicitly permits it. If the API serialises legacy rows by writing `taxId: null` rather than omitting the key, any strictly-conforming OAS 3.1 client or validator will still reject the response. The existing codebase pattern (see `Card.yaml` `stateReason`) uses `oneOf: [{...}, {type: 'null'}]` for exactly this case.
```suggestion
taxId:
oneOf:
- type: string
description: Tax identification number
example: 47-1234567
- type: 'null'
```
How can I resolve this? If you propose a fix, please make it concise.| incorporatedOn: | ||
| type: string | ||
| format: date | ||
| description: Date of incorporation in ISO 8601 format (YYYY-MM-DD) | ||
| example: '2018-03-14' |
There was a problem hiding this comment.
incorporatedOn also needs nullable annotation
Same issue as taxId: if legacy records serialise this field as null rather than omitting it, OAS 3.1 validators and generated clients will reject the response because null is not a valid date-formatted string.
| incorporatedOn: | |
| type: string | |
| format: date | |
| description: Date of incorporation in ISO 8601 format (YYYY-MM-DD) | |
| example: '2018-03-14' | |
| incorporatedOn: | |
| oneOf: | |
| - type: string | |
| format: date | |
| description: Date of incorporation in ISO 8601 format (YYYY-MM-DD) | |
| example: '2018-03-14' | |
| - type: 'null' |
Prompt To Fix With AI
This is a comment left during a code review.
Path: openapi/components/schemas/customers/BusinessInfoResponse.yaml
Line: 25-29
Comment:
**`incorporatedOn` also needs nullable annotation**
Same issue as `taxId`: if legacy records serialise this field as `null` rather than omitting it, OAS 3.1 validators and generated clients will reject the response because `null` is not a valid `date`-formatted string.
```suggestion
incorporatedOn:
oneOf:
- type: string
format: date
description: Date of incorporation in ISO 8601 format (YYYY-MM-DD)
example: '2018-03-14'
- type: 'null'
```
How can I resolve this? If you propose a fix, please make it concise.
Summary
BusinessInfoResponseschema wheretaxIdandincorporatedOnare optional, and pointsBusinessCustomer(response model) at it.BusinessInfoschema unchanged for the create request, so KYB creation still enforcestaxId+incorporatedOn.openapi.yaml/mintlify/openapi.yaml.Why
Pre-existing business customers in our DB have null
tax_id/incorporated_on(data predates the requirement). The current spec marks both required on every read, which causes generated clients to fail when constructing aBusinessCustomerfrom legacy rows for the grid dashboard. Splitting the read and write models lets us keep the write contract strict without lying about legacy reads.Test plan
make buildproduces a valid bundlemake lint-openapipasses (Redocly validation)none_throws(...)calls