Repository navigation
Split the Lead agreements by customer kind and drop the consent-count cap - #1061
Merged
bsiaotickchong merged 2 commits intoSep 23, 2026
Merged
Conversation
… cap A consumer and a commercial customer accept different Lead cardholder and accountholder documents, so each is its own agreement type rather than one type covering both. The array cap went with them. It restated a bound the type list already implies, and any new agreement silently invalidated it. The per-type uniqueness the description states is the real constraint.
ls-bolt
Bot
added
bolt
breaking-change
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Contributor
|
Contributor
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+14
to
+18
| - LEAD_ACCOUNTHOLDER_CONSUMER_AGREEMENT | ||
| - LEAD_ACCOUNTHOLDER_COMMERCIAL_AGREEMENT | ||
| - LEAD_PRIVACY_POLICY | ||
| - LEAD_CARDHOLDER_AGREEMENT | ||
| - LEAD_CARDHOLDER_CONSUMER_AGREEMENT | ||
| - LEAD_CARDHOLDER_COMMERCIAL_AGREEMENT |
Contributor
There was a problem hiding this comment.
Removing LEAD_ACCOUNTHOLDER_AGREEMENT and LEAD_CARDHOLDER_AGREEMENT makes previously valid requests and generated enum constants invalid in the existing 2025-10-13 API contract. This violates the repository directive that breaking OpenAPI changes must bump info.version and use a matching new servers.url. Preserve the old values or release the replacements under a new API version.
Context Used: CLAUDE.md (source)
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: openapi/components/schemas/customers/AgreementType.yaml
Line: 14-18
Comment:
**Enum Removal Breaks Compatibility**
Removing `LEAD_ACCOUNTHOLDER_AGREEMENT` and `LEAD_CARDHOLDER_AGREEMENT` makes previously valid requests and generated enum constants invalid in the existing `2025-10-13` API contract. This violates the repository directive that breaking OpenAPI changes must bump `info.version` and use a matching new `servers.url`. Preserve the old values or release the replacements under a new API version.
**Context Used:** CLAUDE.md ([source](https://github.com/lightsparkdev/grid-api/blob/main/CLAUDE.md))
**Knowledge Base Used:**
- [Grid API contract](https://app.greptile.com/lightspark/-/custom-context/knowledge-base/lightsparkdev/grid-api/-/docs/api-contract.md)
- [Contract validation workflows](https://app.greptile.com/lightspark/-/custom-context/knowledge-base/lightsparkdev/grid-api/-/docs/contract-validation-workflows.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.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!
Contributor
There was a problem hiding this comment.
Not treating this as breaking: the two values shipped 8 days ago in #990 and no backend ever accepted them - sparkcore implements neither agreementConsents nor GET /customers/agreements, so no request using them was ever valid. The repo's breaking-change CI agrees (Detect breaking changes passes). Keeping dead aliases would instead ship four spellings for two documents. Happy to bump info.version and servers.url if you'd rather play it safe.
Contributor
There was a problem hiding this comment.
That context is sufficient to resolve my concern. PR #990 explicitly shipped these as spec/docs-only changes, with backend support deferred, and documented that no consent evidence could yet exist. If Sparkcore implements neither the consent field nor the agreements endpoint, the removed enum values could not have been accepted by the service.
With the breaking-change check passing, this is a correction to an unimplemented contract rather than a live wire-compatibility break. I withdraw the version-bump finding; keeping the obsolete aliases would indeed leave four spellings for two documents.
Comment on lines
+5
to
+8
| type does not. Accepting one agreement never implies acceptance of another, | ||
| even when two agreements share a hosted page. | ||
| even when two agreements share a hosted page. A consumer and a commercial | ||
| customer accept different cardholder and accountholder documents, so each is | ||
| its own type. |
Contributor
There was a problem hiding this comment.
The contract represents customer kind as INDIVIDUAL or BUSINESS, but this description introduces consumer and commercial without defining how they map. Because callers must choose an agreement type themselves, this ambiguity can cause them to submit and record the wrong consent. Explicitly state which customerType corresponds to each agreement variant.
Knowledge Base Used: Customers and accounts API
Prompt To Fix With AI
This is a comment left during a code review.
Path: openapi/components/schemas/customers/AgreementType.yaml
Line: 5-8
Comment:
**Customer Kind Mapping Missing**
The contract represents customer kind as `INDIVIDUAL` or `BUSINESS`, but this description introduces `consumer` and `commercial` without defining how they map. Because callers must choose an agreement type themselves, this ambiguity can cause them to submit and record the wrong consent. Explicitly state which `customerType` corresponds to each agreement variant.
**Knowledge Base Used:** [Customers and accounts API](https://app.greptile.com/lightspark/-/custom-context/knowledge-base/lightsparkdev/grid-api/-/docs/customers-and-accounts-api.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Contributor
There was a problem hiding this comment.
Good catch - the description now says an INDIVIDUAL customer accepts the _CONSUMER_ variant and a BUSINESS customer the _COMMERCIAL_ one, using the contract's own customerType spellings.
Co-Authored-By: bsiaotickchong <bsiaotickchong@users.noreply.github.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
Contributor
|
Preview deployment for your docs. Learn more about Mintlify Previews.
|
Contributor
|
⚡ Review ledger Round 1
|
shreyav
approved these changes
Sep 23, 2026
JasonCWang
approved these changes
Sep 23, 2026
bsiaotickchong
deleted the
09-23-split-lead-agreements-by-customer-kind
branch
September 23, 2026 21:22
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
LEAD_CARDHOLDER_AGREEMENTandLEAD_ACCOUNTHOLDER_AGREEMENTinto consumer and commercial variants. A consumer and a commercial customer accept different documents, so a single type cannot identify which one was accepted.maxItems: 7cap onagreementConsentsin the customer create and update requests.Why the cap goes
It restated a bound the type list already implies, and it silently became wrong the moment a type was added — as this change demonstrates, since the list is now nine. A caller reading
maxItems: 7learns nothing they could act on.The constraint that matters is already stated in the field description and enforced server-side: at most one entry per type. That one does not drift when the enum grows.
Compatibility
The two replaced values are not yet recorded against any customer, so no existing consent evidence refers to them.
Test plan
npm run build:openapiregenerates both bundles; verified the new values are present and the replaced ones are gone fromopenapi.yamlandmintlify/openapi.yaml.npm run lint:openapiexits 0. The only agreement-related output is a pre-existing informational note about a missing example onAgreementConsentRequest.acceptedAt, untouched by this change.Original PR: #1053