Skip to content

Commit 120ae3b

Browse files
authored
fix(types): widen canceled Literal[True]=True to Literal[True]|None=None on request types (#643)
* fix(types): widen canceled Literal[True]=True to Literal[True]|None=None on request types (#641) Omitting canceled from UpdateMediaBuyRequest or PackageUpdate previously defaulted to True (datamodel-codegen const:true handling), silently canceling the media buy on every routine update. Adds fix_canceled_literal_defaults() to post_generate_fixes.py, widens 4 request-type emissions to Literal[True] | None = None, and adds 11 behavioral tests. https://claude.ai/code/session_013iJiiNZi9aBGFawGKUNiGw * fix(tests): remove unused import re in test_canceled_literal_default (nit from pre-PR review) https://claude.ai/code/session_013iJiiNZi9aBGFawGKUNiGw
1 parent 96ccfd4 commit 120ae3b

5 files changed

Lines changed: 218 additions & 8 deletions

File tree

scripts/post_generate_fixes.py

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
3. Fixes BrandManifest forward references
1111
4. Adds deprecated=True to fields marked deprecated in JSON schema
1212
5. Unwraps specified RootModel unions to plain Union type aliases (#155)
13+
6. Widens canceled: Literal[True] = True on request types to | None = None (#641)
1314
"""
1415

1516
from __future__ import annotations
@@ -1053,6 +1054,68 @@ def _ensure_sequence_import(content: str) -> str:
10531054
return "from collections.abc import Sequence\n\n" + content
10541055

10551056

1057+
# Matches the four request-type 'canceled: Literal[True] = True' emissions.
1058+
# datamodel-codegen emits '= True' directly from "const": true boolean
1059+
# schema properties — it is NOT produced by inject_literal_discriminator_defaults()
1060+
# (which already skips bool-valued Literals). Each match rewrites only the
1061+
# annotation and the default; the Field description and the rest of the class
1062+
# are untouched. The regex is inherently idempotent: 'Literal[True] | None,'
1063+
# does not match 'Literal[True],' so a second pass is a no-op.
1064+
_CANCELED_FIELD_RE = re.compile(
1065+
r"( canceled: Annotated\[\n )"
1066+
r"Literal\[True\]"
1067+
r"(,\n Field\(\n description='Cancel[^']*'\n \),\n \])"
1068+
r" = True"
1069+
)
1070+
1071+
1072+
def fix_canceled_literal_defaults() -> None:
1073+
"""Widen ``canceled: Literal[True] = True`` on request types to ``Literal[True] | None = None``.
1074+
1075+
Issue #641: the generated ``= True`` default silently cancels media buys /
1076+
packages when a buyer omits the field from an update request. Changing to
1077+
``Literal[True] | None = None`` preserves wire semantics (the field still
1078+
only accepts ``true`` when present) while making omission non-destructive.
1079+
1080+
Response-side ``canceled: bool | None = False`` fields (status indicators
1081+
like ``core/package.py``) are out of scope — their default is already safe.
1082+
1083+
Root cause: ``datamodel-codegen`` emits ``= True`` from the schema's
1084+
``"const": true`` boolean property. This function corrects that misfire for
1085+
the four request-type emissions listed below.
1086+
"""
1087+
targets = [
1088+
OUTPUT_DIR / "media_buy/update_media_buy_request.py",
1089+
OUTPUT_DIR / "media_buy/package_update.py",
1090+
OUTPUT_DIR / "bundled/media_buy/update_media_buy_request.py",
1091+
]
1092+
1093+
total_fixed = 0
1094+
for py_file in targets:
1095+
if not py_file.exists():
1096+
print(f" {py_file.relative_to(OUTPUT_DIR)}: not found (skipping)")
1097+
continue
1098+
1099+
source = py_file.read_text()
1100+
new_source, count = _CANCELED_FIELD_RE.subn(
1101+
r"\1Literal[True] | None\2 = None",
1102+
source,
1103+
)
1104+
1105+
if count == 0:
1106+
print(f" {py_file.relative_to(OUTPUT_DIR)}: no destructive canceled defaults found")
1107+
continue
1108+
1109+
py_file.write_text(new_source)
1110+
total_fixed += count
1111+
print(f" {py_file.relative_to(OUTPUT_DIR)}: fixed {count} canceled field(s)")
1112+
1113+
if total_fixed > 0:
1114+
print(f" ✓ Widened {total_fixed} canceled Literal[True] default(s) to None")
1115+
else:
1116+
print(" No canceled field defaults needed fixing")
1117+
1118+
10561119
def main():
10571120
"""Apply all post-generation fixes."""
10581121
print("Applying post-generation fixes...")
@@ -1071,6 +1134,7 @@ def main():
10711134
restore_format_category_deprecation_shim()
10721135
inject_literal_discriminator_defaults()
10731136
widen_extension_point_lists_to_sequence()
1137+
fix_canceled_literal_defaults()
10741138

10751139
print("\n✓ Post-generation fixes complete\n")
10761140

src/adcp/types/generated_poc/bundled/media_buy/update_media_buy_request.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4775,11 +4775,11 @@ class Package(AdCPBaseModel):
47754775
Field(description='Pause/resume specific package (true = paused, false = active)'),
47764776
] = None
47774777
canceled: Annotated[
4778-
Literal[True],
4778+
Literal[True] | None,
47794779
Field(
47804780
description='Cancel this specific package. Cancellation is irreversible — canceled packages stop delivery and cannot be reactivated. Sellers MAY reject with NOT_CANCELLABLE.'
47814781
),
4782-
] = True
4782+
] = None
47834783
cancellation_reason: Annotated[
47844784
str | None, Field(description='Reason for canceling this package.', max_length=500)
47854785
] = None
@@ -7575,11 +7575,11 @@ class UpdateMediaBuyRequest(AdCPBaseModel):
75757575
Field(description='Pause/resume the entire media buy (true = paused, false = active)'),
75767576
] = None
75777577
canceled: Annotated[
7578-
Literal[True],
7578+
Literal[True] | None,
75797579
Field(
75807580
description='Cancel the entire media buy. Cancellation is irreversible — canceled media buys cannot be reactivated. Sellers MAY reject with NOT_CANCELLABLE if the media buy cannot be canceled in its current state.'
75817581
),
7582-
] = True
7582+
] = None
75837583
cancellation_reason: Annotated[
75847584
str | None,
75857585
Field(

src/adcp/types/generated_poc/media_buy/package_update.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,11 @@ class PackageUpdate(AdCPBaseModel):
9797
Field(description='Pause/resume specific package (true = paused, false = active)'),
9898
] = None
9999
canceled: Annotated[
100-
Literal[True],
100+
Literal[True] | None,
101101
Field(
102102
description='Cancel this specific package. Cancellation is irreversible — canceled packages stop delivery and cannot be reactivated. Sellers MAY reject with NOT_CANCELLABLE.'
103103
),
104-
] = True
104+
] = None
105105
cancellation_reason: Annotated[
106106
str | None, Field(description='Reason for canceling this package.', max_length=500)
107107
] = None

src/adcp/types/generated_poc/media_buy/update_media_buy_request.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,11 @@ class UpdateMediaBuyRequest(AdCPBaseModel):
5050
Field(description='Pause/resume the entire media buy (true = paused, false = active)'),
5151
] = None
5252
canceled: Annotated[
53-
Literal[True],
53+
Literal[True] | None,
5454
Field(
5555
description='Cancel the entire media buy. Cancellation is irreversible — canceled media buys cannot be reactivated. Sellers MAY reject with NOT_CANCELLABLE if the media buy cannot be canceled in its current state.'
5656
),
57-
] = True
57+
] = None
5858
cancellation_reason: Annotated[
5959
str | None,
6060
Field(
Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
"""Tests for issue #641 — canceled: Literal[True] = True destructive default.
2+
3+
UpdateMediaBuyRequest and PackageUpdate previously had ``canceled: Literal[True] = True``,
4+
meaning any call that omitted the field silently triggered irreversible cancellation.
5+
The fix widens the type to ``Literal[True] | None = None`` so omission is non-destructive.
6+
"""
7+
8+
from __future__ import annotations
9+
10+
import pytest
11+
12+
13+
class TestUpdateMediaBuyRequestCanceledDefault:
14+
"""UpdateMediaBuyRequest.canceled must default to None (non-destructive)."""
15+
16+
_ACCOUNT_KWARGS = {
17+
"account": {"account_id": "acc_test_001"},
18+
"media_buy_id": "mbuy-123",
19+
"idempotency_key": "a" * 16,
20+
}
21+
22+
def test_canceled_defaults_to_none_when_omitted(self) -> None:
23+
from adcp.types.generated_poc.media_buy.update_media_buy_request import (
24+
UpdateMediaBuyRequest,
25+
)
26+
27+
req = UpdateMediaBuyRequest(**self._ACCOUNT_KWARGS)
28+
assert req.canceled is None
29+
30+
def test_canceled_true_still_accepted(self) -> None:
31+
from adcp.types.generated_poc.media_buy.update_media_buy_request import (
32+
UpdateMediaBuyRequest,
33+
)
34+
35+
req = UpdateMediaBuyRequest(**self._ACCOUNT_KWARGS, canceled=True)
36+
assert req.canceled is True
37+
38+
def test_canceled_none_excluded_from_wire_payload(self) -> None:
39+
"""When canceled is None, model_dump(exclude_none=True) omits it — no cancellation on wire."""
40+
from adcp.types.generated_poc.media_buy.update_media_buy_request import (
41+
UpdateMediaBuyRequest,
42+
)
43+
44+
req = UpdateMediaBuyRequest(**self._ACCOUNT_KWARGS)
45+
payload = req.model_dump(mode="json", exclude_none=True)
46+
assert "canceled" not in payload
47+
48+
def test_canceled_true_present_in_wire_payload(self) -> None:
49+
"""Explicit canceled=True must appear in the wire payload to trigger cancellation."""
50+
from adcp.types.generated_poc.media_buy.update_media_buy_request import (
51+
UpdateMediaBuyRequest,
52+
)
53+
54+
req = UpdateMediaBuyRequest(**self._ACCOUNT_KWARGS, canceled=True)
55+
payload = req.model_dump(mode="json", exclude_none=True)
56+
assert payload["canceled"] is True
57+
58+
def test_canceled_false_rejected(self) -> None:
59+
"""Literal[True] still rejects False — the field is a one-way commit signal."""
60+
from pydantic import ValidationError
61+
62+
from adcp.types.generated_poc.media_buy.update_media_buy_request import (
63+
UpdateMediaBuyRequest,
64+
)
65+
66+
with pytest.raises(ValidationError):
67+
UpdateMediaBuyRequest(**self._ACCOUNT_KWARGS, canceled=False) # type: ignore[arg-type]
68+
69+
70+
class TestPackageUpdateCanceledDefault:
71+
"""PackageUpdate.canceled must default to None (non-destructive)."""
72+
73+
def test_canceled_defaults_to_none_when_omitted(self) -> None:
74+
from adcp.types.generated_poc.media_buy.package_update import PackageUpdate
75+
76+
pkg = PackageUpdate(package_id="pkg-1")
77+
assert pkg.canceled is None
78+
79+
def test_canceled_true_still_accepted(self) -> None:
80+
from adcp.types.generated_poc.media_buy.package_update import PackageUpdate
81+
82+
pkg = PackageUpdate(package_id="pkg-1", canceled=True)
83+
assert pkg.canceled is True
84+
85+
def test_canceled_none_excluded_from_wire_payload(self) -> None:
86+
from adcp.types.generated_poc.media_buy.package_update import PackageUpdate
87+
88+
pkg = PackageUpdate(package_id="pkg-1")
89+
payload = pkg.model_dump(mode="json", exclude_none=True)
90+
assert "canceled" not in payload
91+
92+
def test_canceled_true_present_in_wire_payload(self) -> None:
93+
from adcp.types.generated_poc.media_buy.package_update import PackageUpdate
94+
95+
pkg = PackageUpdate(package_id="pkg-1", canceled=True)
96+
payload = pkg.model_dump(mode="json", exclude_none=True)
97+
assert payload["canceled"] is True
98+
99+
100+
class TestFixIdempotency:
101+
"""The post-gen fix must be idempotent — running it twice produces the same output."""
102+
103+
def test_fix_is_idempotent_on_synthesized_source(self) -> None:
104+
from scripts.post_generate_fixes import _CANCELED_FIELD_RE
105+
106+
already_fixed = (
107+
" canceled: Annotated[\n"
108+
" Literal[True] | None,\n"
109+
" Field(\n"
110+
" description='Cancel this specific package. Cancellation is irreversible"
111+
" — canceled packages stop delivery and cannot be reactivated."
112+
" Sellers MAY reject with NOT_CANCELLABLE.'\n"
113+
" ),\n"
114+
" ] = None\n"
115+
)
116+
117+
result, count = _CANCELED_FIELD_RE.subn(
118+
r"\1Literal[True] | None\2 = None",
119+
already_fixed,
120+
)
121+
assert count == 0, "fix must not re-apply to already-widened source"
122+
assert result == already_fixed
123+
124+
def test_fix_matches_destructive_source(self) -> None:
125+
from scripts.post_generate_fixes import _CANCELED_FIELD_RE
126+
127+
destructive = (
128+
" canceled: Annotated[\n"
129+
" Literal[True],\n"
130+
" Field(\n"
131+
" description='Cancel this specific package. Cancellation is irreversible"
132+
" — canceled packages stop delivery and cannot be reactivated."
133+
" Sellers MAY reject with NOT_CANCELLABLE.'\n"
134+
" ),\n"
135+
" ] = True\n"
136+
)
137+
138+
result, count = _CANCELED_FIELD_RE.subn(
139+
r"\1Literal[True] | None\2 = None",
140+
destructive,
141+
)
142+
assert count == 1
143+
assert "Literal[True] | None," in result
144+
assert "] = None" in result
145+
assert "Literal[True]," not in result
146+
assert "] = True" not in result

0 commit comments

Comments
 (0)