Uh oh!
There was an error while loading. Please reload this page.
feat: upgrade Python SDK to UCP v2026-04-08 - #48
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the UCP Python SDK schemas to support new shopping cart, catalog lookup, catalog search, and order lifecycle capabilities, while transitioning monetary fields to use typed Amount and SignedAmount models. The code review feedback suggests enhancing several of these generated Pydantic models with runtime validators to enforce documented constraints, such as ensuring at least one format is provided in Description, exactly one subtotal and total entry exist in Totals, valid price ranges in PriceFilter, proper rating bounds in Rating, and cursor presence when pagination has a next page.
| from pydantic import BaseModel, ConfigDict | ||
| class Description(BaseModel): | ||
| """ | ||
| Description content in one or more formats. At least one format must be provided. | ||
| """ | ||
| model_config = ConfigDict( | ||
| extra="allow", | ||
| ) | ||
| plain: str | None = None | ||
| """ | ||
| Plain text content. | ||
| """ | ||
| html: str | None = None | ||
| """ | ||
| HTML-formatted content. Security: Platforms MUST sanitize before rendering—strip scripts, event handlers, and untrusted elements. Treat all rich text as untrusted input. | ||
| """ | ||
| markdown: str | None = None | ||
| """ | ||
| Markdown-formatted content. | ||
| """ |
There was a problem hiding this comment.
The docstring specifies that 'At least one format must be provided' for the Description model. However, all fields (plain, html, markdown) are currently optional with no validation enforcing this constraint. Adding a model validator ensures compliance with the schema definition.
frompydanticimportBaseModel, ConfigDict, model_validatorclassDescription(BaseModel):
""" Description content in one or more formats. At least one format must be provided. """model_config=ConfigDict(
extra="allow",
)
plain: str|None=None""" Plain text content. """html: str|None=None""" HTML-formatted content. Security: Platforms MUST sanitize before rendering—strip scripts, event handlers, and untrusted elements. Treat all rich text as untrusted input. """markdown: str|None=None""" Markdown-formatted content. """@model_validator(mode="after")defvalidate_at_least_one_format(self) ->Description:
ifself.plainisNoneandself.htmlisNoneandself.markdownisNone:
raiseValueError("At least one description format (plain, html, markdown) must be provided.")
returnselfThere was a problem hiding this comment.
+1. This can be filed as an issue for fast-follow in a separate PR, we'll need to consider this for other classes too IMO.
| from __future__ import annotations | ||
| from pydantic import BaseModel, ConfigDict, Field, RootModel |
| class Totals(RootModel[list[Total]]): | ||
| """ | ||
| Pricing breakdown provided by the business. MUST contain exactly one subtotal and one total entry. Detail types (tax, fee, discount, fulfillment) may appear multiple times for itemization. Platforms MUST render all entries in order using display_text and amount. | ||
| """ | ||
| model_config = ConfigDict( | ||
| frozen=True, | ||
| ) | ||
| root: list[Total] = Field(..., title="Totals") | ||
| """ | ||
| Pricing breakdown provided by the business. MUST contain exactly one subtotal and one total entry. Detail types (tax, fee, discount, fulfillment) may appear multiple times for itemization. Platforms MUST render all entries in order using display_text and amount. | ||
| """ |
There was a problem hiding this comment.
The docstring specifies that the Totals model 'MUST contain exactly one subtotal and one total entry.' Adding a model validator enforces this structural constraint at runtime.
| classTotals(RootModel[list[Total]]): | |
| """ | |
| Pricingbreakdownprovidedbythebusiness. MUSTcontainexactlyonesubtotalandonetotalentry. Detailtypes (tax, fee, discount, fulfillment) mayappearmultipletimesforitemization. PlatformsMUSTrenderallentriesinorderusingdisplay_textandamount. | |
| """ | |
| model_config=ConfigDict( | |
| frozen=True, | |
| ) | |
| root: list[Total] =Field(..., title="Totals") | |
| """ | |
| Pricingbreakdownprovidedbythebusiness. MUSTcontainexactlyonesubtotalandonetotalentry. Detailtypes (tax, fee, discount, fulfillment) mayappearmultipletimesforitemization. PlatformsMUSTrenderallentriesinorderusingdisplay_textandamount. | |
| """ | |
| classTotals(RootModel[list[Total]]): | |
| """ | |
| Pricingbreakdownprovidedbythebusiness. MUSTcontainexactlyonesubtotalandonetotalentry. Detailtypes (tax, fee, discount, fulfillment) mayappearmultipletimesforitemization. PlatformsMUSTrenderallentriesinorderusingdisplay_textandamount. | |
| """ | |
| model_config=ConfigDict( | |
| frozen=True, | |
| ) | |
| root: list[Total] =Field(..., title="Totals") | |
| """ | |
| Pricingbreakdownprovidedbythebusiness. MUSTcontainexactlyonesubtotalandonetotalentry. Detailtypes (tax, fee, discount, fulfillment) mayappearmultipletimesforitemization. PlatformsMUSTrenderallentriesinorderusingdisplay_textandamount. | |
| """ | |
| @model_validator(mode="after") | |
| defvalidate_totals_entries(self) ->Totals: | |
| subtotal_count=sum(1fortinself.rootift.type=="subtotal") | |
| total_count=sum(1fortinself.rootift.type=="total") | |
| ifsubtotal_count!=1ortotal_count!=1: | |
| raiseValueError("Totals must contain exactly one subtotal and one total entry.") | |
| returnself |
| from pydantic import BaseModel, ConfigDict | ||
| from . import amount | ||
| class PriceFilter(BaseModel): | ||
| """ | ||
| Price range filter denominated in context.currency. When context.currency matches the presentment currency, businesses apply the filter directly. When it differs, businesses SHOULD convert filter values to the presentment currency before applying; if conversion is not supported, businesses MAY ignore the filter and SHOULD indicate this via a message. When context.currency is absent, filter denomination is ambiguous and businesses MAY ignore it. | ||
| """ | ||
| model_config = ConfigDict( | ||
| extra="allow", | ||
| ) | ||
| min: amount.Amount | None = None | ||
| """ | ||
| Minimum price in ISO 4217 minor units. | ||
| """ | ||
| max: amount.Amount | None = None | ||
| """ | ||
| Maximum price in ISO 4217 minor units. |
There was a problem hiding this comment.
Add a model validator to ensure that the minimum price (min) is not greater than the maximum price (max) when both are provided.
frompydanticimportBaseModel, ConfigDict, model_validatorfrom . importamountclassPriceFilter(BaseModel):
""" Price range filter denominated in context.currency. When context.currency matches the presentment currency, businesses apply the filter directly. When it differs, businesses SHOULD convert filter values to the presentment currency before applying; if conversion is not supported, businesses MAY ignore the filter and SHOULD indicate this via a message. When context.currency is absent, filter denomination is ambiguous and businesses MAY ignore it. """model_config=ConfigDict(
extra="allow",
)
min: amount.Amount|None=None""" Minimum price in ISO 4217 minor units. """max: amount.Amount|None=None""" Maximum price in ISO 4217 minor units. """@model_validator(mode="after")defvalidate_price_range(self) ->PriceFilter:
ifself.minisnotNoneandself.maxisnotNone:
ifself.min.root>self.max.root:
raiseValueError("Minimum price (min) cannot be greater than maximum price (max).")
returnself| from pydantic import BaseModel, ConfigDict, Field | ||
| class Rating(BaseModel): | ||
| """ | ||
| Product rating aggregate. | ||
| """ | ||
| model_config = ConfigDict( | ||
| extra="allow", | ||
| ) | ||
| value: float = Field(..., ge=0.0) | ||
| """ | ||
| Average rating value. | ||
| """ | ||
| scale_min: float | None = Field(1, ge=0.0) | ||
| """ | ||
| Minimum value on the rating scale (e.g., 1 for 1-5 stars). | ||
| """ | ||
| scale_max: float = Field(..., ge=1.0) | ||
| """ | ||
| Maximum value on the rating scale (e.g., 5 for 5-star). | ||
| """ | ||
| count: int | None = Field(None, ge=0) | ||
| """ | ||
| Number of reviews contributing to the rating. | ||
| """ |
There was a problem hiding this comment.
Add a model validator to ensure that scale_min is strictly less than scale_max, and that the rating value lies within the [scale_min, scale_max] range.
frompydanticimportBaseModel, ConfigDict, Field, model_validatorclassRating(BaseModel):
""" Product rating aggregate. """model_config=ConfigDict(
extra="allow",
)
value: float=Field(..., ge=0.0)
""" Average rating value. """scale_min: float|None=Field(1, ge=0.0)
""" Minimum value on the rating scale (e.g., 1 for 1-5 stars). """scale_max: float=Field(..., ge=1.0)
""" Maximum value on the rating scale (e.g., 5 for 5-star). """count: int|None=Field(None, ge=0)
""" Number of reviews contributing to the rating. """@model_validator(mode="after")defvalidate_rating_bounds(self) ->Rating:
min_val=self.scale_minifself.scale_minisnotNoneelse1.0ifmin_val>=self.scale_max:
raiseValueError("scale_min must be less than scale_max.")
ifnot (min_val<=self.value<=self.scale_max):
raiseValueError(f"Rating value {self.value} must be between {min_val} and {self.scale_max}.")
returnself| # pyformat: disable | ||
| from __future__ import annotations | ||
| class Response(BaseModel): | ||
| """ | ||
| Pagination information in responses. | ||
| """ | ||
| model_config = ConfigDict( | ||
| extra="allow", | ||
| ) | ||
| cursor: str | None = None | ||
| """ | ||
| Cursor to fetch the next page of results. MUST be present when has_next_page is true. | ||
| """ | ||
| has_next_page: bool | ||
| """ | ||
| Whether more results are available. | ||
| """ | ||
| total_count: int | None = Field(None, ge=0) | ||
| """ | ||
| Total number of matching items, if available. | ||
| """ |
There was a problem hiding this comment.
The docstring for cursor specifies that it 'MUST be present when has_next_page is true.' Adding a model validator enforces this requirement at runtime.
classResponse(BaseModel):
""" Pagination information in responses. """model_config=ConfigDict(
extra="allow",
)
cursor: str|None=None""" Cursor to fetch the next page of results. MUST be present when has_next_page is true. """has_next_page: bool""" Whether more results are available. """total_count: int|None=Field(None, ge=0)
""" Total number of matching items, if available. """@model_validator(mode="after")defvalidate_cursor_presence(self) ->Response:
ifself.has_next_pageandself.cursorisNone:
raiseValueError("cursor must be present when has_next_page is True.")
returnselfUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
26f5f31 to
71c1f65Compare71c1f65 to
5e4495cCompare
ptiper
left a comment
There was a problem hiding this comment.
Hi @segiodongo, thanks for the PR!
It looks like the Lint check is failing because some of the generated init.py files have extra trailing newlines that violate end-of-file-fixer. Running pre-commit run --all-files locally should fix these formatting issues. Could you please run it and push the updates?
Uh oh!
There was an error while loading. Please reload this page.
| from pydantic import BaseModel, ConfigDict | ||
| class Description(BaseModel): | ||
| """ | ||
| Description content in one or more formats. At least one format must be provided. | ||
| """ | ||
| model_config = ConfigDict( | ||
| extra="allow", | ||
| ) | ||
| plain: str | None = None | ||
| """ | ||
| Plain text content. | ||
| """ | ||
| html: str | None = None | ||
| """ | ||
| HTML-formatted content. Security: Platforms MUST sanitize before rendering—strip scripts, event handlers, and untrusted elements. Treat all rich text as untrusted input. | ||
| """ | ||
| markdown: str | None = None | ||
| """ | ||
| Markdown-formatted content. | ||
| """ |
There was a problem hiding this comment.
+1. This can be filed as an issue for fast-follow in a separate PR, we'll need to consider this for other classes too IMO.
298c295
into
Universal-Commerce-Protocol:mainUh oh!
There was an error while loading. Please reload this page.
Regenerates Pydantic models against UCP April 8th (v48) schemas, updates package metadata to 0.4.0, and configures per-file-ignores for generated docstrings.