diff --git a/CLAUDE.md b/CLAUDE.md index 960aa89..5b31478 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -19,7 +19,9 @@ PackageRequest = dict[str, Any] **NEVER Modify Generated Files Directly** -Files in `src/adcp/types/generated_poc/` are auto-generated by `scripts/generate_types.py`. Any manual edits will be lost on regeneration. +Files in `src/adcp/types/generated_poc/` and `src/adcp/types/generated.py` are auto-generated by `scripts/generate_types.py`. Any manual edits will be lost on regeneration. + +**CRITICAL**: Do not add code to `generated.py` or any files in `generated_poc/` directory. These are regenerated from schemas. **Post-Generation Fix System:** @@ -64,6 +66,100 @@ Edit `scripts/post_generate_fixes.py` and add a new function. The script: - Validates fixes were successfully applied - Fails loudly if schema changes break the fix patterns +## Semantic Type Aliases for Discriminated Unions + +**Problem**: The code generator (`datamodel-code-generator`) creates numbered type names for discriminated union variants (e.g., `PreviewRender1`, `PreviewRender2`, `SubAsset1`, `SubAsset2`). While functionally correct, these don't convey semantic meaning. + +**Solution**: Add semantic type aliases in `src/adcp/types/aliases.py` that provide clear, descriptive names based on the discriminator field value. + +**Process for Adding New Semantic Aliases:** + +1. **Identify the discriminated union types** - Look for numbered types (e.g., `TypeName1`, `TypeName2`) in generated files +2. **Determine the discriminator field** - Check the schema/generated code for the `Literal` field that distinguishes variants +3. **Create semantic aliases in `aliases.py`**: + ```python + # Import the generated types + from adcp.types.generated import PreviewRender1, PreviewRender2, PreviewRender3 + + # Create semantic aliases based on discriminator values + UrlPreviewRender = PreviewRender1 # output_format='url' + HtmlPreviewRender = PreviewRender2 # output_format='html' + BothPreviewRender = PreviewRender3 # output_format='both' + ``` + +4. **Add to exports** in `aliases.py`: + ```python + __all__ = [ + ..., + "UrlPreviewRender", + "HtmlPreviewRender", + "BothPreviewRender", + ] + ``` + +5. **Re-export from main package** in `src/adcp/__init__.py`: + ```python + from adcp.types.aliases import ( + ..., + UrlPreviewRender, + HtmlPreviewRender, + BothPreviewRender, + ) + + __all__ = [ + ..., + "UrlPreviewRender", + "HtmlPreviewRender", + "BothPreviewRender", + ] + ``` + +6. **Add comprehensive tests** in `tests/test_type_aliases.py`: + - Test that aliases can be imported + - Test that aliases point to correct generated types + - Test that aliases are exported from main package + +**Current Semantic Aliases:** + +- **Preview Renders** (discriminated by `output_format`): + - `UrlPreviewRender` = `PreviewRender1` (output_format='url') + - `HtmlPreviewRender` = `PreviewRender2` (output_format='html') + - `BothPreviewRender` = `PreviewRender3` (output_format='both') + +- **VAST Assets** (discriminated by `delivery_type`): + - `UrlVastAsset` = `VastAsset1` (delivery_type='url') + - `InlineVastAsset` = `VastAsset2` (delivery_type='inline') + +- **DAAST Assets** (discriminated by `delivery_type`): + - `UrlDaastAsset` = `DaastAsset1` (delivery_type='url') + - `InlineDaastAsset` = `DaastAsset2` (delivery_type='inline') + +- **SubAssets** (discriminated by `asset_kind`): + - `MediaSubAsset` = `SubAsset1` (asset_kind='media') + - `TextSubAsset` = `SubAsset2` (asset_kind='text') + +- **Response Types** (discriminated by success/error): + - Success/Error variants for: ActivateSignal, BuildCreative, CreateMediaBuy, ProvidePerformanceFeedback, SyncCreatives, UpdateMediaBuy + +- **Request Types** (discriminated by operation variant): + - `PreviewCreativeFormatRequest`/`PreviewCreativeManifestRequest` + - `UpdateMediaBuyPackagesRequest`/`UpdateMediaBuyPropertiesRequest` + +- **Activation Keys** (discriminated by identifier type): + - `PropertyIdActivationKey`/`PropertyTagActivationKey` + +**Guidelines for Choosing What to Alias:** + +✅ **DO create aliases for:** +- User-facing discriminated unions used in API calls +- Types where the discriminator conveys important semantic meaning +- Types where numbered suffixes cause confusion + +❌ **DON'T create aliases for:** +- Internal helper types not commonly used directly +- Types where parent context makes the meaning clear +- Generic helper types (Input1, Parameters2, etc.) + **Type Checking Best Practices** - Use `TYPE_CHECKING` for optional dependencies to avoid runtime import errors - Use `cast()` for JSON deserialization to satisfy mypy's `no-any-return` checks diff --git a/src/adcp/__init__.py b/src/adcp/__init__.py index 4e5cc10..2a95e3d 100644 --- a/src/adcp/__init__.py +++ b/src/adcp/__init__.py @@ -56,27 +56,33 @@ from adcp.types.aliases import ( ActivateSignalErrorResponse, ActivateSignalSuccessResponse, + BothPreviewRender, BuildCreativeErrorResponse, BuildCreativeSuccessResponse, CreateMediaBuyErrorResponse, CreateMediaBuySuccessResponse, + HtmlPreviewRender, + InlineDaastAsset, + InlineVastAsset, + MediaSubAsset, PreviewCreativeFormatRequest, PreviewCreativeInteractiveResponse, PreviewCreativeManifestRequest, PreviewCreativeStaticResponse, - PreviewRenderHtml, - PreviewRenderIframe, - PreviewRenderImage, PropertyIdActivationKey, PropertyTagActivationKey, ProvidePerformanceFeedbackErrorResponse, ProvidePerformanceFeedbackSuccessResponse, SyncCreativesErrorResponse, SyncCreativesSuccessResponse, + TextSubAsset, UpdateMediaBuyErrorResponse, UpdateMediaBuyPackagesRequest, UpdateMediaBuyPropertiesRequest, UpdateMediaBuySuccessResponse, + UrlDaastAsset, + UrlPreviewRender, + UrlVastAsset, ) from adcp.types.core import AgentConfig, Protocol, TaskResult, TaskStatus, WebhookMetadata @@ -219,25 +225,31 @@ # Semantic type aliases (for better API ergonomics) "ActivateSignalSuccessResponse", "ActivateSignalErrorResponse", + "BothPreviewRender", "BuildCreativeSuccessResponse", "BuildCreativeErrorResponse", "CreateMediaBuySuccessResponse", "CreateMediaBuyErrorResponse", - "ProvidePerformanceFeedbackSuccessResponse", - "ProvidePerformanceFeedbackErrorResponse", - "SyncCreativesSuccessResponse", - "SyncCreativesErrorResponse", - "UpdateMediaBuySuccessResponse", - "UpdateMediaBuyErrorResponse", + "HtmlPreviewRender", + "InlineDaastAsset", + "InlineVastAsset", + "MediaSubAsset", "PreviewCreativeFormatRequest", "PreviewCreativeManifestRequest", "PreviewCreativeStaticResponse", "PreviewCreativeInteractiveResponse", - "PreviewRenderImage", - "PreviewRenderHtml", - "PreviewRenderIframe", "PropertyIdActivationKey", "PropertyTagActivationKey", + "ProvidePerformanceFeedbackSuccessResponse", + "ProvidePerformanceFeedbackErrorResponse", + "SyncCreativesSuccessResponse", + "SyncCreativesErrorResponse", + "TextSubAsset", + "UpdateMediaBuySuccessResponse", + "UpdateMediaBuyErrorResponse", "UpdateMediaBuyPackagesRequest", "UpdateMediaBuyPropertiesRequest", + "UrlDaastAsset", + "UrlPreviewRender", + "UrlVastAsset", ] diff --git a/src/adcp/types/__init__.py b/src/adcp/types/__init__.py index 30ff4ac..f57eae6 100644 --- a/src/adcp/types/__init__.py +++ b/src/adcp/types/__init__.py @@ -2,6 +2,17 @@ """Type definitions for AdCP client.""" +from adcp.types.aliases import ( + BothPreviewRender, + HtmlPreviewRender, + InlineDaastAsset, + InlineVastAsset, + MediaSubAsset, + TextSubAsset, + UrlDaastAsset, + UrlPreviewRender, + UrlVastAsset, +) from adcp.types.base import AdCPBaseModel from adcp.types.core import ( Activity, @@ -24,4 +35,14 @@ "Activity", "ActivityType", "DebugInfo", + # Semantic aliases for discriminated unions + "BothPreviewRender", + "HtmlPreviewRender", + "InlineDaastAsset", + "InlineVastAsset", + "MediaSubAsset", + "TextSubAsset", + "UrlDaastAsset", + "UrlPreviewRender", + "UrlVastAsset", ] diff --git a/src/adcp/types/aliases.py b/src/adcp/types/aliases.py index 11feaf9..bd391b0 100644 --- a/src/adcp/types/aliases.py +++ b/src/adcp/types/aliases.py @@ -45,6 +45,9 @@ # Create media buy responses CreateMediaBuyResponse1, CreateMediaBuyResponse2, + # DAAST assets + DaastAsset1, + DaastAsset2, # Preview creative requests PreviewCreativeRequest1, PreviewCreativeRequest2, @@ -58,6 +61,9 @@ # Performance feedback responses ProvidePerformanceFeedbackResponse1, ProvidePerformanceFeedbackResponse2, + # SubAssets + SubAsset1, + SubAsset2, # Sync creatives responses SyncCreativesResponse1, SyncCreativesResponse2, @@ -67,6 +73,9 @@ # Update media buy responses UpdateMediaBuyResponse1, UpdateMediaBuyResponse2, + # VAST assets + VastAsset1, + VastAsset2, ) # ============================================================================ @@ -157,15 +166,40 @@ PreviewCreativeInteractiveResponse = PreviewCreativeResponse2 """Preview response with interactive renders (iframe embedding).""" -# Preview Render Variants -PreviewRenderImage = PreviewRender1 -"""Image-based preview render (PNG/JPEG).""" +# Preview Render Variants (discriminated by output_format) +UrlPreviewRender = PreviewRender1 +"""Preview render with output_format='url' - provides preview_url for iframe embedding.""" -PreviewRenderHtml = PreviewRender2 -"""HTML-based preview render (static markup).""" +HtmlPreviewRender = PreviewRender2 +"""Preview render with output_format='html' - provides preview_html for direct embedding.""" -PreviewRenderIframe = PreviewRender3 -"""Interactive iframe-based preview render.""" +BothPreviewRender = PreviewRender3 +"""Preview render with output_format='both' - provides both preview_url and preview_html.""" + +# ============================================================================ +# ASSET TYPE ALIASES - Delivery & Kind Discriminated Unions +# ============================================================================ + +# VAST Asset Variants (discriminated by delivery_type) +UrlVastAsset = VastAsset1 +"""VAST asset delivered via URL endpoint - delivery_type='url'.""" + +InlineVastAsset = VastAsset2 +"""VAST asset with inline XML content - delivery_type='inline'.""" + +# DAAST Asset Variants (discriminated by delivery_type) +UrlDaastAsset = DaastAsset1 +"""DAAST asset delivered via URL endpoint - delivery_type='url'.""" + +InlineDaastAsset = DaastAsset2 +"""DAAST asset with inline XML content - delivery_type='inline'.""" + +# SubAsset Variants (discriminated by asset_kind) +MediaSubAsset = SubAsset1 +"""SubAsset for media content (images, videos) - asset_kind='media', provides content_uri.""" + +TextSubAsset = SubAsset2 +"""SubAsset for text content (headlines, body text) - asset_kind='text', provides content.""" # ============================================================================ # EXPORTS @@ -178,6 +212,16 @@ # Activation keys "PropertyIdActivationKey", "PropertyTagActivationKey", + # Asset type aliases + "BothPreviewRender", + "HtmlPreviewRender", + "InlineDaastAsset", + "InlineVastAsset", + "MediaSubAsset", + "TextSubAsset", + "UrlDaastAsset", + "UrlPreviewRender", + "UrlVastAsset", # Build creative responses "BuildCreativeSuccessResponse", "BuildCreativeErrorResponse", @@ -193,10 +237,6 @@ # Preview creative responses "PreviewCreativeStaticResponse", "PreviewCreativeInteractiveResponse", - # Preview renders - "PreviewRenderImage", - "PreviewRenderHtml", - "PreviewRenderIframe", # Sync creatives responses "SyncCreativesSuccessResponse", "SyncCreativesErrorResponse", diff --git a/tests/test_discriminated_unions.py b/tests/test_discriminated_unions.py index 42d49d4..0b3edd4 100644 --- a/tests/test_discriminated_unions.py +++ b/tests/test_discriminated_unions.py @@ -9,8 +9,17 @@ from adcp import ( ActivateSignalErrorResponse, ActivateSignalSuccessResponse, + BothPreviewRender, CreateMediaBuyErrorResponse, CreateMediaBuySuccessResponse, + HtmlPreviewRender, + InlineDaastAsset, + InlineVastAsset, + MediaSubAsset, + TextSubAsset, + UrlDaastAsset, + UrlPreviewRender, + UrlVastAsset, ) # Keep using generated names for authorization/deployment/destination variants @@ -448,3 +457,254 @@ def test_product_accepts_mixed_publisher_properties(self): assert len(mixed_props) == 2 assert mixed_props[0].selection_type == "by_id" assert mixed_props[1].selection_type == "by_tag" + + +class TestPreviewRenderDiscriminators: + """Test PreviewRender discriminator field values match semantic aliases.""" + + def test_url_preview_render_has_url_discriminator(self): + """UrlPreviewRender has output_format='url'.""" + render = UrlPreviewRender( + render_id="render_1", + role="primary", + output_format="url", + preview_url="https://preview.example.com/creative", + ) + assert render.output_format == "url" + assert hasattr(render, "preview_url") + assert not hasattr(render, "preview_html") + + def test_html_preview_render_has_html_discriminator(self): + """HtmlPreviewRender has output_format='html'.""" + render = HtmlPreviewRender( + render_id="render_1", + role="primary", + output_format="html", + preview_html="