Skip to content

fix: Fix inconsistent schema on documentation examples & discounts extension schema#371

Merged
jingyli merged 3 commits intoUniversal-Commerce-Protocol:mainfrom
jingyli:spec-minor-fix
Apr 28, 2026
Merged

fix: Fix inconsistent schema on documentation examples & discounts extension schema#371
jingyli merged 3 commits intoUniversal-Commerce-Protocol:mainfrom
jingyli:spec-minor-fix

Conversation

@jingyli
Copy link
Copy Markdown
Contributor

@jingyli jingyli commented Apr 21, 2026

Description

This PR aims to address some minor schema inconsistencies that are present in the current UCP spec:

  1. In discount.md: Update all the examples to follow the correct UCP schema - item.id is the only field populated in requests, quantity pulled out to the line_item level, etc.
  2. In checkout-rest.md and checkout-mcp.md: Remove unsupported available_quantity field and add in other required fields like item in business error example.
  3. Fix incorrect usage of readOnly annotation in discount.json.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • Documentation update

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@jingyli jingyli requested review from a team as code owners April 21, 2026 13:49
@jingyli jingyli added this to the Working Draft milestone Apr 21, 2026
Copy link
Copy Markdown

@cusell-google cusell-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some automated doc checks were skipped, could you make sure they run before merging?

"item": {
"id": "item_123",
"title": "Blue Jeans",
"price": 5000
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thinking is that the current model is more explicit: we already have currency at the checkout object level - https://github.com/Universal-Commerce-Protocol/ucp/blob/main/source/schemas/shopping/checkout.json#L74 - and our expectation is that all amounts in the checkout object should follow that currency field.

@yanheChen yanheChen self-requested a review April 21, 2026 18:10
@jingyli
Copy link
Copy Markdown
Contributor Author

jingyli commented Apr 21, 2026

@cusell-google Thanks for the review! The skipped checks are expected - those checks should only be executed as part of the release workflow.

@jingyli jingyli added the TC review Ready for TC review label Apr 21, 2026
Copy link
Copy Markdown
Contributor

@raginpirate raginpirate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically this is a breaking change, because the readOnly was not our expected control over the output schemas and validation here. But we should be fine accepting this given our belief that consumers of the spec will not be impacted and would have interpreted this the same way.

Copy link
Copy Markdown

@mmohades mmohades left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this inconsistency in the docs!

Copy link
Copy Markdown
Contributor

@igrigorik igrigorik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Copy Markdown
Contributor

@westeezy westeezy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm ty.

@jingyli jingyli merged commit f217f08 into Universal-Commerce-Protocol:main Apr 28, 2026
11 checks passed
@jingyli jingyli deleted the spec-minor-fix branch April 28, 2026 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

TC review Ready for TC review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants