From 1693e7083e6b5b22992acc3719c4fd946a8619b3 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Sun, 23 Nov 2025 16:26:53 -0500 Subject: [PATCH 1/2] Implement ADCP 2.12.0 template formats to eliminate format explosion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add 5 template formats (display_generative, display_image, display_html, video_standard, video_dimensions) that accept parameterized dimensions or duration, reducing the need for hundreds of concrete formats. Maintains full backward compatibility with all 42 existing concrete formats. Includes comprehensive test coverage with 44 new tests validating template format behavior, parameter extraction, and asset requirements. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- pyproject.toml | 2 +- src/creative_agent/api_server.py | 4 +- src/creative_agent/data/standard_formats.py | 328 ++++++++++---- src/creative_agent/schemas/__init__.py | 11 +- src/creative_agent/schemas/manifest.py | 2 +- src/creative_agent/server.py | 5 +- tests/integration/test_preview_creative.py | 26 +- .../test_preview_html_and_batch.py | 18 +- tests/integration/test_template_formats.py | 407 ++++++++++++++++++ .../integration/test_tool_response_formats.py | 6 +- tests/smoke/test_server_startup.py | 2 +- tests/unit/test_filter_formats.py | 114 +++-- tests/unit/test_format_card_renderer.py | 2 +- tests/unit/test_info_card_formats.py | 14 +- tests/unit/test_preview_generation.py | 10 +- tests/unit/test_product_card_renderer.py | 2 +- tests/unit/test_storage_error_handling.py | 2 +- .../test_template_parameter_validation.py | 322 ++++++++++++++ uv.lock | 9 +- 19 files changed, 1083 insertions(+), 203 deletions(-) create mode 100644 tests/integration/test_template_formats.py create mode 100644 tests/validation/test_template_parameter_validation.py diff --git a/pyproject.toml b/pyproject.toml index 5530368..5c64c07 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -16,7 +16,7 @@ dependencies = [ "boto3>=1.35.0", "markdown>=3.6", "bleach>=6.3.0", - "adcp>=2.2.0", # Official ADCP Python client with semantic type aliases + "adcp>=2.12.0", # Official ADCP Python client with template format support ] [project.scripts] diff --git a/src/creative_agent/api_server.py b/src/creative_agent/api_server.py index b655c27..11f470e 100644 --- a/src/creative_agent/api_server.py +++ b/src/creative_agent/api_server.py @@ -60,7 +60,7 @@ async def list_formats() -> list[dict[str, Any]]: async def get_format(format_id: str) -> dict[str, Any]: """Get a specific format by ID (assumes this agent's formats).""" - from adcp.types.generated import FormatId + from adcp import FormatId from .data.standard_formats import AGENT_URL @@ -77,7 +77,7 @@ async def get_format(format_id: str) -> dict[str, Any]: async def preview_creative(request: PreviewRequest) -> dict[str, Any]: """Generate preview from creative manifest.""" - from adcp.types.generated import FormatId + from adcp import FormatId from .data.standard_formats import AGENT_URL diff --git a/src/creative_agent/data/standard_formats.py b/src/creative_agent/data/standard_formats.py index 7fef544..20f6b79 100644 --- a/src/creative_agent/data/standard_formats.py +++ b/src/creative_agent/data/standard_formats.py @@ -5,16 +5,10 @@ from typing import Any -from adcp.types.generated import FormatId -from adcp.types.generated_poc.format import ( - AssetsRequired as LibAssetsRequired, -) -from adcp.types.generated_poc.format import ( - Render as LibRender, -) -from adcp.types.generated_poc.format import ( - Type as LibType, -) +from adcp import FormatCategory, FormatId +from adcp.types.generated_poc.core.format import AssetsRequired as LibAssetsRequired +from adcp.types.generated_poc.core.format import Renders as LibRender +from adcp.types.generated_poc.enums.format_id_parameter import FormatIdParameter from pydantic import AnyUrl from ..schemas import CreativeFormat @@ -58,8 +52,8 @@ def create_asset_required( The library model automatically handles exclude_none serialization and includes the item_type discriminator for union types. """ - # Convert local AssetType enum to library's AssetType - from adcp.types.generated_poc.format import AssetType as LibAssetType + # Convert local AssetType enum to library's AssetContentType + from adcp import AssetContentType as LibAssetType lib_asset_type = LibAssetType(asset_type.value) @@ -78,51 +72,79 @@ def create_fixed_render(width: int, height: int, role: str = "primary") -> LibRe Returns the library's Pydantic model which automatically handles exclude_none serialization. """ - from adcp.types.generated_poc.format import Dimensions as LibDimensions - from adcp.types.generated_poc.format import Responsive as LibResponsive - from adcp.types.generated_poc.format import Unit as LibUnit + from adcp.types.generated_poc.core.format import Dimensions as LibDimensions + from adcp.types.generated_poc.core.format import Responsive return LibRender( role=role, dimensions=LibDimensions( width=width, height=height, - responsive=LibResponsive(width=False, height=False), - unit=LibUnit.px, + responsive=Responsive(width=False, height=False), ), ) def create_responsive_render( role: str = "primary", + min_width: int | None = None, + max_width: int | None = None, + min_height: int | None = None, + max_height: int | None = None, ) -> LibRender: """Create a render with responsive dimensions. Returns the library's Pydantic model which automatically handles exclude_none serialization. """ - from adcp.types.generated_poc.format import Dimensions as LibDimensions - from adcp.types.generated_poc.format import Responsive as LibResponsive - from adcp.types.generated_poc.format import Unit as LibUnit + from adcp.types.generated_poc.core.format import Dimensions as LibDimensions + from adcp.types.generated_poc.core.format import Responsive return LibRender( role=role, dimensions=LibDimensions( - width=None, - height=None, - responsive=LibResponsive(width=True, height=True), - unit=LibUnit.px, + min_width=min_width, + max_width=max_width, + min_height=min_height, + max_height=max_height, + responsive=Responsive(width=True, height=True), ), ) # Generative Formats - AI-powered creative generation # These use promoted_offerings asset type which provides brand context and product info +# Template format that accepts dimension parameters (for new integrations) +# Plus concrete formats (for backward compatibility) GENERATIVE_FORMATS = [ + # Template format - supports any dimensions + CreativeFormat( + format_id=create_format_id("display_generative"), + name="Display Banner - AI Generated", + type=FormatCategory.display, + description="AI-generated display banner from brand context and prompt (supports any dimensions)", + accepts_parameters=["dimensions"], + supported_macros=COMMON_MACROS, + assets_required=[ + create_asset_required( + asset_id="promoted_offerings", + asset_type=AssetType.promoted_offerings, + required=True, + requirements={"description": "Brand manifest and product offerings for AI generation"}, + ), + create_asset_required( + asset_id="generation_prompt", + asset_type=AssetType.text, + required=True, + requirements={"description": "Text prompt describing the desired creative"}, + ), + ], + ), + # Concrete formats for backward compatibility CreativeFormat( format_id=create_format_id("display_300x250_generative"), name="Medium Rectangle - AI Generated", - type=LibType.display, + type=FormatCategory.display, description="AI-generated 300x250 banner from brand context and prompt", renders=[create_fixed_render(300, 250)], output_format_ids=[create_format_id("display_300x250_image")], @@ -145,7 +167,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("display_728x90_generative"), name="Leaderboard - AI Generated", - type=LibType.display, + type=FormatCategory.display, description="AI-generated 728x90 banner from brand context and prompt", renders=[create_fixed_render(728, 90)], output_format_ids=[create_format_id("display_728x90_image")], @@ -168,7 +190,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("display_320x50_generative"), name="Mobile Banner - AI Generated", - type=LibType.display, + type=FormatCategory.display, description="AI-generated 320x50 mobile banner from brand context and prompt", renders=[create_fixed_render(320, 50)], output_format_ids=[create_format_id("display_320x50_image")], @@ -191,7 +213,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("display_160x600_generative"), name="Wide Skyscraper - AI Generated", - type=LibType.display, + type=FormatCategory.display, description="AI-generated 160x600 wide skyscraper from brand context and prompt", renders=[create_fixed_render(160, 600)], output_format_ids=[create_format_id("display_160x600_image")], @@ -214,7 +236,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("display_336x280_generative"), name="Large Rectangle - AI Generated", - type=LibType.display, + type=FormatCategory.display, description="AI-generated 336x280 large rectangle from brand context and prompt", renders=[create_fixed_render(336, 280)], output_format_ids=[create_format_id("display_336x280_image")], @@ -237,7 +259,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("display_300x600_generative"), name="Half Page - AI Generated", - type=LibType.display, + type=FormatCategory.display, description="AI-generated 300x600 half page from brand context and prompt", renders=[create_fixed_render(300, 600)], output_format_ids=[create_format_id("display_300x600_image")], @@ -260,7 +282,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("display_970x250_generative"), name="Billboard - AI Generated", - type=LibType.display, + type=FormatCategory.display, description="AI-generated 970x250 billboard from brand context and prompt", renders=[create_fixed_render(970, 250)], output_format_ids=[create_format_id("display_970x250_image")], @@ -283,11 +305,55 @@ def create_responsive_render( ] # Video Formats +# Template formats (for new integrations) + concrete formats (for backward compatibility) VIDEO_FORMATS = [ + # Template format - supports any duration + CreativeFormat( + format_id=create_format_id("video_standard"), + name="Standard Video", + type=FormatCategory.video, + description="Video ad in standard aspect ratios (supports any duration)", + accepts_parameters=["duration"], + supported_macros=[*COMMON_MACROS, "VIDEO_ID", "POD_POSITION", "CONTENT_GENRE"], + assets_required=[ + create_asset_required( + asset_id="video_file", + asset_type=AssetType.video, + required=True, + requirements={ + "parameters_from_format_id": True, + "acceptable_formats": ["mp4", "mov", "webm"], + "description": "Video file matching format_id duration", + }, + ), + ], + ), + # Template format - supports any dimensions + CreativeFormat( + format_id=create_format_id("video_dimensions"), + name="Video with Dimensions", + type=FormatCategory.video, + description="Video ad with specific dimensions (supports any size)", + accepts_parameters=["dimensions"], + supported_macros=[*COMMON_MACROS, "VIDEO_ID", "POD_POSITION", "CONTENT_GENRE"], + assets_required=[ + create_asset_required( + asset_id="video_file", + asset_type=AssetType.video, + required=True, + requirements={ + "parameters_from_format_id": True, + "acceptable_formats": ["mp4", "mov", "webm"], + "description": "Video file matching format_id dimensions", + }, + ), + ], + ), + # Concrete formats for backward compatibility CreativeFormat( format_id=create_format_id("video_standard_30s"), name="Standard Video - 30 seconds", - type=LibType.video, + type=FormatCategory.video, description="30-second video ad in standard aspect ratios", supported_macros=[*COMMON_MACROS, "VIDEO_ID", "POD_POSITION", "CONTENT_GENRE"], assets_required=[ @@ -306,7 +372,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("video_standard_15s"), name="Standard Video - 15 seconds", - type=LibType.video, + type=FormatCategory.video, description="15-second video ad in standard aspect ratios", supported_macros=[*COMMON_MACROS, "VIDEO_ID", "POD_POSITION", "CONTENT_GENRE"], assets_required=[ @@ -325,7 +391,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("video_vast_30s"), name="VAST Video - 30 seconds", - type=LibType.video, + type=FormatCategory.video, description="30-second video ad via VAST tag", supported_macros=[*COMMON_MACROS, "VIDEO_ID", "POD_POSITION", "CONTENT_GENRE"], assets_required=[ @@ -342,7 +408,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("video_1920x1080"), name="Full HD Video - 1920x1080", - type=LibType.video, + type=FormatCategory.video, description="1920x1080 Full HD video (16:9)", supported_macros=[*COMMON_MACROS, "VIDEO_ID", "POD_POSITION", "CONTENT_GENRE"], renders=[create_fixed_render(1920, 1080)], @@ -363,7 +429,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("video_1280x720"), name="HD Video - 1280x720", - type=LibType.video, + type=FormatCategory.video, description="1280x720 HD video (16:9)", supported_macros=[*COMMON_MACROS, "VIDEO_ID", "POD_POSITION", "CONTENT_GENRE"], renders=[create_fixed_render(1280, 720)], @@ -384,7 +450,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("video_1080x1920"), name="Vertical Video - 1080x1920", - type=LibType.video, + type=FormatCategory.video, description="1080x1920 vertical video (9:16) for mobile stories", supported_macros=[*COMMON_MACROS, "VIDEO_ID", "POD_POSITION", "CONTENT_GENRE"], renders=[create_fixed_render(1080, 1920)], @@ -405,7 +471,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("video_1080x1080"), name="Square Video - 1080x1080", - type=LibType.video, + type=FormatCategory.video, description="1080x1080 square video (1:1) for social feeds", supported_macros=[*COMMON_MACROS, "VIDEO_ID", "POD_POSITION", "CONTENT_GENRE"], renders=[create_fixed_render(1080, 1080)], @@ -426,7 +492,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("video_ctv_preroll_30s"), name="CTV Pre-Roll - 30 seconds", - type=LibType.video, + type=FormatCategory.video, description="30-second pre-roll ad for Connected TV and streaming platforms", supported_macros=[*COMMON_MACROS, "VIDEO_ID", "POD_POSITION", "CONTENT_GENRE", "PLAYER_SIZE"], assets_required=[ @@ -445,7 +511,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("video_ctv_midroll_30s"), name="CTV Mid-Roll - 30 seconds", - type=LibType.video, + type=FormatCategory.video, description="30-second mid-roll ad for Connected TV and streaming platforms", supported_macros=[*COMMON_MACROS, "VIDEO_ID", "POD_POSITION", "CONTENT_GENRE", "PLAYER_SIZE"], assets_required=[ @@ -464,11 +530,42 @@ def create_responsive_render( ] # Display Formats - Image-based +# Template format (for new integrations) + concrete formats (for backward compatibility) DISPLAY_IMAGE_FORMATS = [ + # Template format - supports any dimensions + CreativeFormat( + format_id=create_format_id("display_image"), + name="Display Banner - Image", + type=FormatCategory.display, + description="Static image banner (supports any dimensions)", + accepts_parameters=["dimensions"], + supported_macros=COMMON_MACROS, + assets_required=[ + create_asset_required( + asset_id="banner_image", + asset_type=AssetType.image, + required=True, + requirements={ + "parameters_from_format_id": True, + "acceptable_formats": ["jpg", "png", "gif", "webp"], + "description": "Banner image matching format_id dimensions", + }, + ), + create_asset_required( + asset_id="click_url", + asset_type=AssetType.url, + required=True, + requirements={ + "description": "Clickthrough destination URL", + }, + ), + ], + ), + # Concrete formats for backward compatibility CreativeFormat( format_id=create_format_id("display_300x250_image"), name="Medium Rectangle - Image", - type=LibType.display, + type=FormatCategory.display, description="300x250 static image banner", supported_macros=COMMON_MACROS, renders=[create_fixed_render(300, 250)], @@ -497,7 +594,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("display_728x90_image"), name="Leaderboard - Image", - type=LibType.display, + type=FormatCategory.display, description="728x90 static image banner", supported_macros=COMMON_MACROS, renders=[create_fixed_render(728, 90)], @@ -523,7 +620,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("display_320x50_image"), name="Mobile Banner - Image", - type=LibType.display, + type=FormatCategory.display, description="320x50 mobile banner", supported_macros=COMMON_MACROS, renders=[create_fixed_render(320, 50)], @@ -549,7 +646,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("display_160x600_image"), name="Wide Skyscraper - Image", - type=LibType.display, + type=FormatCategory.display, description="160x600 wide skyscraper banner", supported_macros=COMMON_MACROS, renders=[create_fixed_render(160, 600)], @@ -575,7 +672,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("display_336x280_image"), name="Large Rectangle - Image", - type=LibType.display, + type=FormatCategory.display, description="336x280 large rectangle banner", supported_macros=COMMON_MACROS, renders=[create_fixed_render(336, 280)], @@ -601,7 +698,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("display_300x600_image"), name="Half Page - Image", - type=LibType.display, + type=FormatCategory.display, description="300x600 half page banner", supported_macros=COMMON_MACROS, renders=[create_fixed_render(300, 600)], @@ -627,7 +724,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("display_970x250_image"), name="Billboard - Image", - type=LibType.display, + type=FormatCategory.display, description="970x250 billboard banner", supported_macros=COMMON_MACROS, renders=[create_fixed_render(970, 250)], @@ -653,11 +750,34 @@ def create_responsive_render( ] # Display Formats - HTML5 +# Template format (for new integrations) + concrete formats (for backward compatibility) DISPLAY_HTML_FORMATS = [ + # Template format - supports any dimensions + CreativeFormat( + format_id=create_format_id("display_html"), + name="Display Banner - HTML5", + type=FormatCategory.display, + description="HTML5 creative (supports any dimensions)", + accepts_parameters=["dimensions"], + supported_macros=COMMON_MACROS, + assets_required=[ + create_asset_required( + asset_id="html_creative", + asset_type=AssetType.html, + required=True, + requirements={ + "parameters_from_format_id": True, + "max_file_size_mb": 0.5, + "description": "HTML5 creative code matching format_id dimensions", + }, + ), + ], + ), + # Concrete formats for backward compatibility CreativeFormat( format_id=create_format_id("display_300x250_html"), name="Medium Rectangle - HTML5", - type=LibType.display, + type=FormatCategory.display, description="300x250 HTML5 creative", supported_macros=COMMON_MACROS, renders=[create_fixed_render(300, 250)], @@ -678,7 +798,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("display_728x90_html"), name="Leaderboard - HTML5", - type=LibType.display, + type=FormatCategory.display, description="728x90 HTML5 creative", supported_macros=COMMON_MACROS, renders=[create_fixed_render(728, 90)], @@ -698,7 +818,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("display_160x600_html"), name="Wide Skyscraper - HTML5", - type=LibType.display, + type=FormatCategory.display, description="160x600 HTML5 creative", supported_macros=COMMON_MACROS, renders=[create_fixed_render(160, 600)], @@ -718,7 +838,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("display_336x280_html"), name="Large Rectangle - HTML5", - type=LibType.display, + type=FormatCategory.display, description="336x280 HTML5 creative", supported_macros=COMMON_MACROS, renders=[create_fixed_render(336, 280)], @@ -738,7 +858,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("display_300x600_html"), name="Half Page - HTML5", - type=LibType.display, + type=FormatCategory.display, description="300x600 HTML5 creative", supported_macros=COMMON_MACROS, renders=[create_fixed_render(300, 600)], @@ -758,7 +878,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("display_970x250_html"), name="Billboard - HTML5", - type=LibType.display, + type=FormatCategory.display, description="970x250 HTML5 creative", supported_macros=COMMON_MACROS, renders=[create_fixed_render(970, 250)], @@ -782,7 +902,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("native_standard"), name="IAB Native Standard", - type=LibType.native, + type=FormatCategory.native, description="Standard native ad with title, description, image, and CTA", supported_macros=COMMON_MACROS, assets_required=[ @@ -839,7 +959,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("native_content"), name="Native Content Placement", - type=LibType.native, + type=FormatCategory.native, description="In-article native ad with editorial styling", supported_macros=COMMON_MACROS, assets_required=[ @@ -900,7 +1020,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("audio_standard_15s"), name="Standard Audio - 15 seconds", - type=LibType.audio, + type=FormatCategory.audio, description="15-second audio ad", supported_macros=[*COMMON_MACROS, "CONTENT_GENRE"], assets_required=[ @@ -918,7 +1038,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("audio_standard_30s"), name="Standard Audio - 30 seconds", - type=LibType.audio, + type=FormatCategory.audio, description="30-second audio ad", supported_macros=[*COMMON_MACROS, "CONTENT_GENRE"], assets_required=[ @@ -936,7 +1056,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("audio_standard_60s"), name="Standard Audio - 60 seconds", - type=LibType.audio, + type=FormatCategory.audio, description="60-second audio ad", supported_macros=[*COMMON_MACROS, "CONTENT_GENRE"], assets_required=[ @@ -958,7 +1078,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("dooh_billboard_1920x1080"), name="Digital Billboard - 1920x1080", - type=LibType.dooh, + type=FormatCategory.dooh, description="Full HD digital billboard", supported_macros=[*COMMON_MACROS, "SCREEN_ID", "VENUE_TYPE", "VENUE_LAT", "VENUE_LONG"], renders=[create_fixed_render(1920, 1080)], @@ -978,7 +1098,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("dooh_billboard_landscape"), name="Digital Billboard - Landscape", - type=LibType.dooh, + type=FormatCategory.dooh, description="Landscape-oriented digital billboard (various sizes)", supported_macros=[*COMMON_MACROS, "SCREEN_ID", "VENUE_TYPE", "VENUE_LAT", "VENUE_LONG"], assets_required=[ @@ -996,7 +1116,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("dooh_billboard_portrait"), name="Digital Billboard - Portrait", - type=LibType.dooh, + type=FormatCategory.dooh, description="Portrait-oriented digital billboard (various sizes)", supported_macros=[*COMMON_MACROS, "SCREEN_ID", "VENUE_TYPE", "VENUE_LAT", "VENUE_LONG"], assets_required=[ @@ -1014,7 +1134,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("dooh_transit_screen"), name="Transit Screen", - type=LibType.dooh, + type=FormatCategory.dooh, description="Transit and subway screen displays", supported_macros=[*COMMON_MACROS, "SCREEN_ID", "VENUE_TYPE", "VENUE_LAT", "VENUE_LONG", "TRANSIT_LINE"], renders=[create_fixed_render(1920, 1080)], @@ -1039,7 +1159,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("product_card_standard"), name="Product Card - Standard", - type=LibType.display, + type=FormatCategory.display, description="Standard visual card (300x400px) for displaying ad inventory products", supported_macros=COMMON_MACROS, renders=[create_fixed_render(300, 400)], @@ -1113,7 +1233,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("product_card_detailed"), name="Product Card - Detailed", - type=LibType.display, + type=FormatCategory.display, description="Detailed card with carousel and full specifications for rich product presentation", supported_macros=COMMON_MACROS, renders=[create_responsive_render()], @@ -1187,7 +1307,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("format_card_standard"), name="Format Card - Standard", - type=LibType.display, + type=FormatCategory.display, description="Standard visual card (300x400px) for displaying creative formats in user interfaces", supported_macros=COMMON_MACROS, renders=[create_fixed_render(300, 400)], @@ -1205,7 +1325,7 @@ def create_responsive_render( CreativeFormat( format_id=create_format_id("format_card_detailed"), name="Format Card - Detailed", - type=LibType.display, + type=FormatCategory.display, description="Detailed card with carousel and full specifications for rich format documentation", supported_macros=COMMON_MACROS, renders=[create_responsive_render()], @@ -1236,17 +1356,35 @@ def create_responsive_render( def get_format_by_id(format_id: FormatId) -> CreativeFormat | None: - """Get format by FormatId object.""" + """Get format by FormatId object. + + For template formats, matches on base ID (agent_url + id) regardless of parameters. + For concrete formats, requires exact match including any dimension parameters. + """ for fmt in STANDARD_FORMATS: - # Compare both ID and agent URL - if fmt.format_id.id == format_id.id and str(fmt.format_id.agent_url) == str(format_id.agent_url): + # Compare base ID and agent URL + if fmt.format_id.id != format_id.id or str(fmt.format_id.agent_url) != str(format_id.agent_url): + continue + + # If format is a template, match on base ID only + if getattr(fmt, "accepts_parameters", None): return fmt + + # For concrete formats, dimensions must match exactly + fmt_width = getattr(fmt.format_id, "width", None) + fmt_height = getattr(fmt.format_id, "height", None) + search_width = getattr(format_id, "width", None) + search_height = getattr(format_id, "height", None) + + if fmt_width == search_width and fmt_height == search_height: + return fmt + return None def filter_formats( format_ids: list[FormatId] | None = None, - type: LibType | str | None = None, + type: FormatCategory | str | None = None, asset_types: list[AssetType | str] | None = None, dimensions: str | None = None, max_width: int | None = None, @@ -1260,9 +1398,26 @@ def filter_formats( results = STANDARD_FORMATS if format_ids: - # Convert to (id, agent_url) tuples for comparison - search_ids = [(fid.id, str(fid.agent_url)) for fid in format_ids] - results = [fmt for fmt in results if (fmt.format_id.id, str(fmt.format_id.agent_url)) in search_ids] + # Convert to (id, agent_url, width, height) tuples for comparison + # For template formats, match on base ID if width/height not specified in search + search_ids = [ + (fid.id, str(fid.agent_url), getattr(fid, "width", None), getattr(fid, "height", None)) + for fid in format_ids + ] + + def matches_format_id(fmt: CreativeFormat, search_id: tuple[str, str, int | None, int | None]) -> bool: + base_id, agent_url, search_width, search_height = search_id + if fmt.format_id.id != base_id or str(fmt.format_id.agent_url) != agent_url: + return False + # If searching with parameters, template must match exactly + # If searching without parameters, match base template ID + if search_width is not None or search_height is not None: + fmt_width = getattr(fmt.format_id, "width", None) + fmt_height = getattr(fmt.format_id, "height", None) + return bool(fmt_width == search_width and fmt_height == search_height) + return True + + results = [fmt for fmt in results if any(matches_format_id(fmt, sid) for sid in search_ids)] if type: # Handle both Type enum and string values @@ -1282,6 +1437,13 @@ def filter_formats( target_width, target_height = int(parts[0]), int(parts[1]) def matches_dimensions(fmt: CreativeFormat) -> bool: + # Template formats accept dimensions but don't have fixed renders + if ( + getattr(fmt, "accepts_parameters", None) + and FormatIdParameter.dimensions in fmt.accepts_parameters + ): + return True + # Concrete formats have fixed renders if not fmt.renders or len(fmt.renders) == 0: return False render = fmt.renders[0] @@ -1309,6 +1471,12 @@ def get_dimensions(fmt: CreativeFormat) -> tuple[float | None, float | None]: filtered = [] for fmt in results: + # Template formats accept any dimensions within the constraints + if getattr(fmt, "accepts_parameters", None) and FormatIdParameter.dimensions in fmt.accepts_parameters: + # Include template formats - they can satisfy any dimension requirements + filtered.append(fmt) + continue + width, height = get_dimensions(fmt) # Exclude formats without dimensions when dimension filtering is requested if width is None or height is None: @@ -1327,17 +1495,19 @@ def get_dimensions(fmt: CreativeFormat) -> tuple[float | None, float | None]: results = filtered if is_responsive is not None: - # Filter based on responsive flag in renders + # Filter based on responsive field in ADCP 2.12.0+ schema def is_format_responsive(fmt: CreativeFormat) -> bool: if not fmt.renders or len(fmt.renders) == 0: return False render = fmt.renders[0] - # renders are always Pydantic models (adcp 2.2.0+) + # renders are always Pydantic models (adcp 2.12.0+) dims = render.dimensions - if hasattr(dims, "responsive"): - resp = dims.responsive - return getattr(resp, "width", False) or getattr(resp, "height", False) - return False + # In ADCP 2.12.0+, responsive field indicates if dimensions adapt to container + responsive = getattr(dims, "responsive", None) + if responsive is None: + return False + # Check if either width or height is responsive + return getattr(responsive, "width", False) or getattr(responsive, "height", False) if is_responsive: results = [fmt for fmt in results if is_format_responsive(fmt)] diff --git a/src/creative_agent/schemas/__init__.py b/src/creative_agent/schemas/__init__.py index c5ce0fa..ba0e37f 100644 --- a/src/creative_agent/schemas/__init__.py +++ b/src/creative_agent/schemas/__init__.py @@ -9,15 +9,8 @@ """ # Core schemas from adcp library -from adcp.types.generated import ( - CreativeAsset as CreativeManifest, -) -from adcp.types.generated import ( - Format as CreativeFormat, -) -from adcp.types.generated import ( - ListCreativeFormatsResponse, -) +from adcp import CreativeManifest, ListCreativeFormatsResponse +from adcp import Format as CreativeFormat # Build schemas (agent-specific, not part of AdCP) from .build import ( diff --git a/src/creative_agent/schemas/manifest.py b/src/creative_agent/schemas/manifest.py index 11f5107..8e8c9e8 100644 --- a/src/creative_agent/schemas/manifest.py +++ b/src/creative_agent/schemas/manifest.py @@ -2,7 +2,7 @@ from typing import Any -from adcp.types.generated import FormatId +from adcp import FormatId from pydantic import BaseModel # CreativeManifest is imported from AdCP schemas via __init__.py diff --git a/src/creative_agent/server.py b/src/creative_agent/server.py index 2263445..48070b1 100644 --- a/src/creative_agent/server.py +++ b/src/creative_agent/server.py @@ -6,8 +6,9 @@ from datetime import UTC, datetime, timedelta from typing import Any -from adcp.types.generated import FormatId -from adcp.types.generated_poc.list_creative_formats_response import Capability, CreativeAgent +from adcp import FormatId +from adcp.types import Capability +from adcp.types.generated_poc.media_buy.list_creative_formats_response import CreativeAgent from fastmcp import FastMCP from fastmcp.tools.tool import ToolResult from mcp.types import TextContent diff --git a/tests/integration/test_preview_creative.py b/tests/integration/test_preview_creative.py index a3fdc2e..dd6e994 100644 --- a/tests/integration/test_preview_creative.py +++ b/tests/integration/test_preview_creative.py @@ -6,7 +6,7 @@ from datetime import UTC, datetime import pytest -from adcp.types.generated import FormatId +from adcp import FormatId from pytest_mock import MockerFixture from creative_agent import server @@ -32,8 +32,6 @@ def test_preview_creative_with_spec_compliant_manifest(self, mock_s3_upload): """Test preview_creative tool with fully spec-compliant manifest.""" # Create spec-compliant Pydantic manifest manifest = CreativeManifest( - creative_id="test-creative", - name="Test Creative", format_id=FormatId(agent_url=AGENT_URL, id="display_300x250_image"), assets={ "banner_image": { @@ -74,8 +72,6 @@ def test_preview_creative_with_spec_compliant_manifest(self, mock_s3_upload): def test_preview_creative_with_custom_inputs(self, mock_s3_upload): """Test preview_creative with custom input variants.""" manifest = CreativeManifest( - creative_id="test-creative", - name="Test Creative", format_id=FormatId(agent_url=AGENT_URL, id="display_300x250_image"), assets={ "banner_image": { @@ -106,8 +102,6 @@ def test_preview_creative_validates_format_id_mismatch(self, mock_s3_upload): """Test that preview_creative rejects manifest with mismatched format_id.""" # Create manifest with DIFFERENT format_id than request manifest = CreativeManifest( - creative_id="test-creative", - name="Test Creative", format_id=FormatId(agent_url=AGENT_URL, id="display_728x90_image"), assets={ "banner_image": { @@ -134,8 +128,6 @@ def test_preview_creative_validates_format_id_mismatch(self, mock_s3_upload): def test_preview_creative_accepts_format_id_as_dict(self, mock_s3_upload): """Test that preview_creative accepts format_id as FormatId object (dict).""" manifest = CreativeManifest( - creative_id="test-creative", - name="Test Creative", format_id=FormatId(agent_url=AGENT_URL, id="display_300x250_image"), assets={ "banner_image": { @@ -160,8 +152,6 @@ def test_preview_creative_accepts_format_id_as_dict(self, mock_s3_upload): def test_preview_creative_validates_malicious_urls(self, mock_s3_upload): """Test that preview_creative validates and sanitizes malicious URLs.""" manifest = CreativeManifest( - creative_id="test-creative", - name="Test Creative", format_id=FormatId(agent_url=AGENT_URL, id="display_300x250_image"), assets={ "banner_image": { @@ -188,8 +178,6 @@ def test_preview_creative_validates_malicious_urls(self, mock_s3_upload): def test_preview_creative_returns_interactive_url(self, mock_s3_upload): """Test that preview response includes interactive_url.""" manifest = CreativeManifest( - creative_id="test-creative", - name="Test Creative", format_id=FormatId(agent_url=AGENT_URL, id="display_300x250_image"), assets={ "banner_image": { @@ -213,8 +201,6 @@ def test_preview_creative_returns_interactive_url(self, mock_s3_upload): def test_preview_creative_returns_expiration(self, mock_s3_upload): """Test that preview response includes expires_at timestamp.""" manifest = CreativeManifest( - creative_id="test-creative", - name="Test Creative", format_id=FormatId(agent_url=AGENT_URL, id="display_300x250_image"), assets={ "banner_image": { @@ -240,8 +226,6 @@ def test_preview_creative_returns_expiration(self, mock_s3_upload): def test_preview_creative_rejects_unknown_format(self, mock_s3_upload): """Test that preview_creative rejects unknown format_id.""" manifest = CreativeManifest( - creative_id="test-creative", - name="Test Creative", format_id=FormatId(agent_url=AGENT_URL, id="display_300x250_image"), assets={ "banner_image": { @@ -264,11 +248,9 @@ def test_preview_creative_rejects_unknown_format(self, mock_s3_upload): def test_preview_creative_returns_spec_compliant_response(self, mock_s3_upload): """Test that response matches ADCP PreviewCreativeResponse spec exactly.""" - from adcp.types.generated import PreviewCreativeResponse + from adcp import PreviewCreativeResponse manifest = CreativeManifest( - creative_id="test-creative", - name="Test Creative", format_id=FormatId(agent_url=AGENT_URL, id="display_300x250_image"), assets={ "banner_image": { @@ -299,8 +281,6 @@ def test_preview_creative_returns_spec_compliant_response(self, mock_s3_upload): def test_preview_expiration_is_valid_iso8601_timestamp(self, mock_s3_upload): """Test that expires_at is a valid ISO 8601 timestamp in the future.""" manifest = CreativeManifest( - creative_id="test-creative", - name="Test Creative", format_id=FormatId(agent_url=AGENT_URL, id="display_300x250_image"), assets={ "banner_image": { @@ -340,8 +320,6 @@ def test_preview_creative_fails_with_missing_required_asset(self, mock_s3_upload """Test that preview_creative returns clear error when required asset is missing.""" # Create manifest missing required click_url manifest = CreativeManifest( - creative_id="test-creative", - name="Test Creative", format_id=FormatId(agent_url=AGENT_URL, id="display_300x250_image"), assets={ "banner_image": { diff --git a/tests/integration/test_preview_html_and_batch.py b/tests/integration/test_preview_html_and_batch.py index 56501eb..264e922 100644 --- a/tests/integration/test_preview_html_and_batch.py +++ b/tests/integration/test_preview_html_and_batch.py @@ -1,7 +1,7 @@ """Integration tests for HTML output and batch preview modes.""" import pytest -from adcp.types.generated import FormatId, PreviewCreativeResponse +from adcp import FormatId, PreviewCreativeResponse from creative_agent import server from creative_agent.data.standard_formats import AGENT_URL @@ -25,8 +25,6 @@ def mock_s3_upload(self, mocker): def test_html_output_returns_preview_html(self): """Test that output_format='html' returns preview_html field.""" manifest = CreativeManifest( - creative_id="test-creative", - name="Test Creative", format_id=FormatId(agent_url=AGENT_URL, id="display_300x250_image"), assets={ "banner_image": { @@ -72,8 +70,6 @@ def test_html_output_returns_preview_html(self): def test_html_output_does_not_upload_to_s3(self, mock_s3_upload): """Test that HTML output mode doesn't upload to S3.""" manifest = CreativeManifest( - creative_id="test-creative", - name="Test Creative", format_id=FormatId(agent_url=AGENT_URL, id="display_300x250_image"), assets={ "banner_image": { @@ -97,8 +93,6 @@ def test_html_output_does_not_upload_to_s3(self, mock_s3_upload): def test_url_output_still_works(self, mock_s3_upload): """Test that default URL output mode still works.""" manifest = CreativeManifest( - creative_id="test-creative", - name="Test Creative", format_id=FormatId(agent_url=AGENT_URL, id="display_300x250_image"), assets={ "banner_image": { @@ -150,8 +144,6 @@ def mock_s3_upload(self, mocker): def test_batch_mode_with_multiple_requests(self): """Test batch mode with multiple preview requests.""" manifest1 = CreativeManifest( - creative_id="test-creative", - name="Test Creative", format_id=FormatId(agent_url=AGENT_URL, id="display_300x250_image"), assets={ "banner_image": { @@ -164,8 +156,6 @@ def test_batch_mode_with_multiple_requests(self): ) manifest2 = CreativeManifest( - creative_id="test-creative", - name="Test Creative", format_id=FormatId(agent_url=AGENT_URL, id="display_728x90_image"), assets={ "banner_image": { @@ -211,8 +201,6 @@ def test_batch_mode_with_multiple_requests(self): def test_batch_mode_with_html_output(self): """Test batch mode with HTML output format.""" manifest = CreativeManifest( - creative_id="test-creative", - name="Test Creative", format_id=FormatId(agent_url=AGENT_URL, id="display_300x250_image"), assets={ "banner_image": { @@ -252,8 +240,6 @@ def test_batch_mode_with_html_output(self): def test_batch_mode_handles_errors_gracefully(self): """Test that batch mode handles individual request errors.""" valid_manifest = CreativeManifest( - creative_id="test-creative", - name="Test Creative", format_id=FormatId(agent_url=AGENT_URL, id="display_300x250_image"), assets={ "banner_image": { @@ -295,8 +281,6 @@ def test_batch_mode_handles_errors_gracefully(self): def test_batch_mode_per_request_output_format_override(self): """Test that individual requests can override batch output_format.""" manifest = CreativeManifest( - creative_id="test-creative", - name="Test Creative", format_id=FormatId(agent_url=AGENT_URL, id="display_300x250_image"), assets={ "banner_image": { diff --git a/tests/integration/test_template_formats.py b/tests/integration/test_template_formats.py new file mode 100644 index 0000000..0fffbf2 --- /dev/null +++ b/tests/integration/test_template_formats.py @@ -0,0 +1,407 @@ +"""Integration tests for ADCP 2.12.0 template format functionality. + +Tests template formats that accept parameters (dimensions, duration) and can be +instantiated with specific values, avoiding the need for hundreds of concrete formats. +""" + +import json + +from adcp import FormatId, ListCreativeFormatsResponse +from adcp.types.generated_poc.enums.format_id_parameter import FormatIdParameter +from pydantic import AnyUrl + +from creative_agent import server +from creative_agent.data.standard_formats import ( + AGENT_URL, + STANDARD_FORMATS, + filter_formats, + get_format_by_id, +) + +# Get actual function from FastMCP wrapper +list_creative_formats = server.list_creative_formats.fn + + +class TestTemplateFormatDiscovery: + """Test discovery and listing of template formats.""" + + def test_standard_formats_include_templates(self): + """STANDARD_FORMATS should include template formats with accepts_parameters.""" + template_formats = [f for f in STANDARD_FORMATS if getattr(f, "accepts_parameters", None)] + + assert len(template_formats) == 5, f"Expected 5 template formats, found {len(template_formats)}" + + # Verify template format IDs + template_ids = {f.format_id.id for f in template_formats} + expected_ids = { + "display_generative", + "display_image", + "display_html", + "video_standard", + "video_dimensions", + } + assert template_ids == expected_ids, f"Template IDs mismatch: {template_ids} != {expected_ids}" + + def test_template_formats_have_valid_accepts_parameters(self): + """Template formats must have valid accepts_parameters field per ADCP 2.12.0.""" + template_formats = [f for f in STANDARD_FORMATS if getattr(f, "accepts_parameters", None)] + + for fmt in template_formats: + # Must be a list of FormatIdParameter enums + assert isinstance(fmt.accepts_parameters, list), f"{fmt.format_id.id}: accepts_parameters must be list" + assert len(fmt.accepts_parameters) > 0, f"{fmt.format_id.id}: accepts_parameters cannot be empty" + assert all(isinstance(p, FormatIdParameter) for p in fmt.accepts_parameters), ( + f"{fmt.format_id.id}: accepts_parameters must contain FormatIdParameter enums" + ) + + # Must be valid parameter types + valid_params = {FormatIdParameter.dimensions, FormatIdParameter.duration} + for param in fmt.accepts_parameters: + assert param in valid_params, ( + f"{fmt.format_id.id}: unknown parameter '{param}', expected one of {valid_params}" + ) + + def test_template_formats_no_renders_field(self): + """Template formats should not have renders field - they accept arbitrary dimensions.""" + template_formats = [f for f in STANDARD_FORMATS if getattr(f, "accepts_parameters", None)] + + for fmt in template_formats: + # Template formats should have None or empty renders + renders = getattr(fmt, "renders", None) + assert renders is None or len(renders) == 0, f"{fmt.format_id.id}: template format should not have renders" + + def test_list_creative_formats_returns_templates(self): + """list_creative_formats tool should return template formats.""" + # Call the tool function directly + result = list_creative_formats() + + # Parse response + assert hasattr(result, "content"), "Result should have content attribute" + assert hasattr(result, "structured_content"), "Result should have structured_content attribute" + assert result.structured_content, "Structured content should not be empty" + + # Validate against ADCP schema + response = ListCreativeFormatsResponse.model_validate(result.structured_content) + + # Check for template formats + template_formats = [f for f in response.formats if getattr(f, "accepts_parameters", None)] + assert len(template_formats) == 5, f"Expected 5 template formats in response, found {len(template_formats)}" + + +class TestTemplateFormatFiltering: + """Test filtering template formats vs concrete formats.""" + + def test_filter_by_type_includes_templates(self): + """Filtering by type should include both template and concrete formats.""" + from adcp import FormatCategory + + # Filter for display formats + display_formats = filter_formats(type=FormatCategory.display) + + # Should include template formats + template_displays = [f for f in display_formats if getattr(f, "accepts_parameters", None)] + assert len(template_displays) == 3, f"Expected 3 display templates, found {len(template_displays)}" + + # Verify IDs + template_ids = {f.format_id.id for f in template_displays} + expected_ids = {"display_generative", "display_image", "display_html"} + assert template_ids == expected_ids + + def test_filter_by_type_video_includes_templates(self): + """Filtering for video should include video template formats.""" + from adcp import FormatCategory + + video_formats = filter_formats(type=FormatCategory.video) + + # Should include 2 video templates + 9 concrete + template_videos = [f for f in video_formats if getattr(f, "accepts_parameters", None)] + assert len(template_videos) == 2, f"Expected 2 video templates, found {len(template_videos)}" + + # Verify IDs + template_ids = {f.format_id.id for f in template_videos} + expected_ids = {"video_standard", "video_dimensions"} + assert template_ids == expected_ids + + def test_dimension_filter_includes_templates(self): + """Dimension filtering should include template formats that accept dimensions.""" + # Filter for specific dimensions + results = filter_formats(dimensions="468x60") + + # Should include dimension-accepting templates + template_results = [f for f in results if getattr(f, "accepts_parameters", None)] + dimension_templates = [ + f for f in template_results if FormatIdParameter.dimensions in getattr(f, "accepts_parameters", []) + ] + + # Should have display_generative, display_image, display_html, video_dimensions + assert len(dimension_templates) >= 3, ( + f"Expected at least 3 dimension templates for 468x60, found {len(dimension_templates)}" + ) + + # All should accept dimensions parameter + for fmt in dimension_templates: + assert FormatIdParameter.dimensions in fmt.accepts_parameters + + def test_max_width_filter_includes_templates(self): + """Max width filtering should include dimension templates.""" + results = filter_formats(max_width=500) + + # Should include templates that accept dimensions + template_results = [f for f in results if getattr(f, "accepts_parameters", None)] + dimension_templates = [ + f for f in template_results if FormatIdParameter.dimensions in getattr(f, "accepts_parameters", []) + ] + + # Template formats can satisfy any dimension requirement + assert len(dimension_templates) >= 3, "Should include dimension-accepting templates" + + +class TestTemplateFormatLookup: + """Test looking up template formats by ID.""" + + def test_get_template_format_by_base_id(self): + """get_format_by_id should match template on base ID only.""" + # Look up template without parameters + format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="display_image") + + result = get_format_by_id(format_id) + + assert result is not None, "Should find template format" + assert result.format_id.id == "display_image" + assert FormatIdParameter.dimensions in getattr(result, "accepts_parameters", []) + + def test_get_template_format_ignores_dimension_params(self): + """Template format lookup should ignore dimension parameters in format_id.""" + # Look up template WITH dimension parameters + format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="display_image", width=468, height=60) + + result = get_format_by_id(format_id) + + # Should still match the template (not look for concrete format) + assert result is not None, "Should find template format" + assert result.format_id.id == "display_image" + assert FormatIdParameter.dimensions in getattr(result, "accepts_parameters", []) + # Template format_id should NOT have dimensions + assert getattr(result.format_id, "width", None) is None + assert getattr(result.format_id, "height", None) is None + + def test_get_concrete_format_requires_exact_match(self): + """Concrete format lookup should require exact dimension match.""" + # Look up concrete format + format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="display_300x250_image") + + result = get_format_by_id(format_id) + + assert result is not None, "Should find concrete format" + assert result.format_id.id == "display_300x250_image" + assert getattr(result, "accepts_parameters", None) is None, "Should not be a template" + # Should have renders with fixed dimensions + assert result.renders is not None + assert len(result.renders) > 0 + + def test_get_video_template_by_base_id(self): + """get_format_by_id should match video template on base ID.""" + format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="video_standard") + + result = get_format_by_id(format_id) + + assert result is not None, "Should find video template" + assert result.format_id.id == "video_standard" + assert FormatIdParameter.duration in getattr(result, "accepts_parameters", []) + + +class TestTemplateAssetRequirements: + """Test asset requirements with parameters_from_format_id.""" + + def test_template_assets_have_parameters_from_format_id(self): + """Template format assets should use parameters_from_format_id.""" + # Get display_image template + format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="display_image") + fmt = get_format_by_id(format_id) + + assert fmt is not None + assert fmt.assets_required is not None + assert len(fmt.assets_required) > 0 + + # Find the image asset + image_asset = None + for asset in fmt.assets_required: + if asset.asset_id == "banner_image": + image_asset = asset + break + + assert image_asset is not None, "Should have banner_image asset" + + # Check for parameters_from_format_id in requirements + requirements = getattr(image_asset, "requirements", None) + assert requirements is not None, "Image asset should have requirements" + assert "parameters_from_format_id" in requirements, "Image asset should specify parameters_from_format_id" + assert requirements["parameters_from_format_id"] is True + + def test_video_template_assets_have_parameters_from_format_id(self): + """Video template assets should use parameters_from_format_id for duration.""" + # Get video_standard template + format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="video_standard") + fmt = get_format_by_id(format_id) + + assert fmt is not None + assert fmt.assets_required is not None + assert len(fmt.assets_required) > 0 + + # Find the video asset + video_asset = fmt.assets_required[0] + assert video_asset.asset_id == "video_file" + + # Check for parameters_from_format_id + requirements = getattr(video_asset, "requirements", None) + assert requirements is not None + assert requirements.get("parameters_from_format_id") is True, ( + "Video asset should use parameters_from_format_id for duration" + ) + + def test_concrete_formats_have_explicit_requirements(self): + """Concrete formats should have explicit dimension requirements, not parameters_from_format_id.""" + # Get concrete format + format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="display_300x250_image") + fmt = get_format_by_id(format_id) + + assert fmt is not None + assert fmt.assets_required is not None + assert len(fmt.assets_required) > 0 + + # Find the image asset + image_asset = None + for asset in fmt.assets_required: + if asset.asset_id == "banner_image": + image_asset = asset + break + + assert image_asset is not None + + # Should have explicit width/height requirements + requirements = getattr(image_asset, "requirements", None) + assert requirements is not None + assert "width" in requirements, "Concrete format should have explicit width" + assert "height" in requirements, "Concrete format should have explicit height" + assert requirements["width"] == 300 + assert requirements["height"] == 250 + + # Should NOT use parameters_from_format_id + assert requirements.get("parameters_from_format_id") is not True + + +class TestTemplateFormatSerialization: + """Test that template formats serialize correctly for ADCP.""" + + def test_template_format_serializes_with_accepts_parameters(self): + """Template format should serialize with accepts_parameters field.""" + format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="display_image") + fmt = get_format_by_id(format_id) + + # Serialize to dict with mode='json' to get enum string values + fmt_dict = fmt.model_dump(mode="json", exclude_none=True) + + assert "accepts_parameters" in fmt_dict, "Serialized format should include accepts_parameters" + assert fmt_dict["accepts_parameters"] == ["dimensions"] + + # Should NOT have renders + assert "renders" not in fmt_dict or fmt_dict.get("renders") is None + + def test_template_format_roundtrip(self): + """Template format should deserialize correctly.""" + from creative_agent.schemas import CreativeFormat + + format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="video_standard") + fmt = get_format_by_id(format_id) + + # Serialize with mode='json' to get enum string values + fmt_dict = fmt.model_dump(mode="json", exclude_none=True) + fmt_json = json.dumps(fmt_dict) + fmt_restored = CreativeFormat.model_validate_json(fmt_json) + + # Should preserve template properties (as enum objects after deserialization) + assert fmt_restored.format_id.id == "video_standard" + assert FormatIdParameter.duration in getattr(fmt_restored, "accepts_parameters", []) + + +class TestTemplateFormatValidation: + """Test validation rules for template formats.""" + + def test_template_cannot_have_both_renders_and_accepts_parameters(self): + """A format should not have both renders and accepts_parameters (logical constraint).""" + # This is more of a schema validation - template formats by definition + # accept arbitrary dimensions, so they shouldn't specify fixed renders + + for fmt in STANDARD_FORMATS: + accepts_params = getattr(fmt, "accepts_parameters", None) + renders = getattr(fmt, "renders", None) + + if accepts_params and FormatIdParameter.dimensions in accepts_params: + # Template format - should not have fixed renders + assert renders is None or len(renders) == 0, ( + f"{fmt.format_id.id}: template format should not specify renders" + ) + + def test_template_with_dimensions_has_no_output_format_ids(self): + """Template formats accepting dimensions should not specify output_format_ids.""" + # Templates are flexible - they don't transform to a specific concrete format + + for fmt in STANDARD_FORMATS: + accepts_params = getattr(fmt, "accepts_parameters", None) + if accepts_params and FormatIdParameter.dimensions in accepts_params: + output_formats = getattr(fmt, "output_format_ids", None) + + # Generative template might have output formats, but dimension templates shouldn't + if fmt.format_id.id not in ["display_generative"]: + assert output_formats is None or len(output_formats) == 0, ( + f"{fmt.format_id.id}: dimension template should not have output_format_ids" + ) + + +class TestTemplateFormatCoverage: + """Test that we have appropriate template format coverage.""" + + def test_have_display_dimension_templates(self): + """Should have template formats for common display types.""" + display_templates = [ + f + for f in STANDARD_FORMATS + if getattr(f, "accepts_parameters", None) + and FormatIdParameter.dimensions in f.accepts_parameters + and f.type.value == "display" + ] + + template_ids = {f.format_id.id for f in display_templates} + + # Should have templates for: generative, image, html + assert "display_generative" in template_ids + assert "display_image" in template_ids + assert "display_html" in template_ids + + def test_have_video_templates(self): + """Should have template formats for video with both duration and dimensions.""" + video_templates = [ + f for f in STANDARD_FORMATS if getattr(f, "accepts_parameters", None) and f.type.value == "video" + ] + + # Should have both duration and dimension templates + duration_templates = [f for f in video_templates if FormatIdParameter.duration in f.accepts_parameters] + dimension_templates = [f for f in video_templates if FormatIdParameter.dimensions in f.accepts_parameters] + + assert len(duration_templates) >= 1, "Should have video duration template" + assert len(dimension_templates) >= 1, "Should have video dimension template" + + def test_template_to_concrete_ratio(self): + """Should have reasonable ratio of templates to concrete formats.""" + templates = [f for f in STANDARD_FORMATS if getattr(f, "accepts_parameters", None)] + concrete = [f for f in STANDARD_FORMATS if not getattr(f, "accepts_parameters", None)] + + # With templates, we should have far fewer total formats than without + # Expected: 5 templates + 42 concrete = 47 total + assert len(templates) == 5, f"Expected 5 templates, found {len(templates)}" + assert len(concrete) == 42, f"Expected 42 concrete formats, found {len(concrete)}" + assert len(STANDARD_FORMATS) == 47 + + # Ratio should be roughly 1:8 (5 templates replace ~40 potential concrete formats) + ratio = len(concrete) / len(templates) + assert ratio > 5, f"Should have healthy template-to-concrete ratio, got {ratio}" diff --git a/tests/integration/test_tool_response_formats.py b/tests/integration/test_tool_response_formats.py index d14ea78..0c0e85f 100644 --- a/tests/integration/test_tool_response_formats.py +++ b/tests/integration/test_tool_response_formats.py @@ -11,7 +11,7 @@ import json import pytest -from adcp.types.generated import ( +from adcp import ( FormatId, ListCreativeFormatsResponse, PreviewCreativeResponse, @@ -174,8 +174,6 @@ class TestPreviewCreativeResponseFormat: def valid_manifest(self): """Create a valid manifest per ADCP spec.""" return CreativeManifest( - creative_id="test-creative-1", - name="Test Creative", format_id=FormatId(agent_url=AGENT_URL, id="display_300x250_image"), assets={ "banner_image": { @@ -306,8 +304,6 @@ def test_structured_content_not_double_encoded(self, mocker): # Test preview_creative manifest = CreativeManifest( - creative_id="test-creative-2", - name="Test Creative 2", format_id=FormatId(agent_url=AGENT_URL, id="display_300x250_image"), assets={ "banner_image": { diff --git a/tests/smoke/test_server_startup.py b/tests/smoke/test_server_startup.py index b661598..b6eb41f 100644 --- a/tests/smoke/test_server_startup.py +++ b/tests/smoke/test_server_startup.py @@ -49,7 +49,7 @@ def test_list_creative_formats(): @pytest.mark.smoke def test_preview_creative_logic(): """Verify preview generation logic works.""" - from adcp.types.generated import FormatId + from adcp import FormatId from src.creative_agent.data.standard_formats import AGENT_URL, get_format_by_id diff --git a/tests/unit/test_filter_formats.py b/tests/unit/test_filter_formats.py index 953f799..30b3763 100644 --- a/tests/unit/test_filter_formats.py +++ b/tests/unit/test_filter_formats.py @@ -1,7 +1,7 @@ """Tests for format filtering logic.""" -from adcp.types.generated import FormatId -from adcp.types.generated_poc.format import Type +from adcp import FormatCategory, FormatId +from adcp.types.generated_poc.enums.format_id_parameter import FormatIdParameter from creative_agent.data.standard_formats import filter_formats @@ -24,78 +24,96 @@ class TestDimensionFiltering: """Test dimension-based filtering.""" def test_exact_dimensions_match(self): - """Filter by exact dimensions string returns only matching formats.""" - results = filter_formats(dimensions="970x250") + """Filter by exact dimensions string returns template formats and matching concrete formats.""" + results = filter_formats(dimensions="300x400") assert len(results) > 0 - # All results should have 970x250 dimensions + # Results should either be templates that accept dimensions OR have 300x400 dimensions for fmt in results: + # Template formats accept any dimensions + if getattr(fmt, "accepts_parameters", None) and FormatIdParameter.dimensions in fmt.accepts_parameters: + continue + # Concrete formats must have exact dimensions assert fmt.renders assert len(fmt.renders) > 0 - assert get_render_dimensions(fmt)[0] == 970 - assert get_render_dimensions(fmt)[1] == 250 + assert get_render_dimensions(fmt)[0] == 300 + assert get_render_dimensions(fmt)[1] == 400 def test_exact_dimensions_excludes_audio(self): """Filter by dimensions excludes audio formats without dimensions.""" - results = filter_formats(dimensions="970x250") + results = filter_formats(dimensions="300x400") # No audio formats should be in results for fmt in results: - assert fmt.type != Type.audio + assert fmt.type != FormatCategory.audio def test_max_width_excludes_larger(self): - """Filter by max_width excludes formats wider than limit.""" + """Filter by max_width includes template formats and excludes concrete formats wider than limit.""" results = filter_formats(max_width=728) assert len(results) > 0 for fmt in results: + # Template formats accept any dimensions + if getattr(fmt, "accepts_parameters", None) and FormatIdParameter.dimensions in fmt.accepts_parameters: + continue + # Concrete formats must be within width limit assert fmt.renders assert get_render_dimensions(fmt)[0] is not None assert get_render_dimensions(fmt)[0] <= 728 def test_max_width_excludes_formats_without_dimensions(self): - """Filter by max_width excludes formats without dimensions (audio).""" + """Filter by max_width excludes audio formats but includes templates.""" results = filter_formats(max_width=1000) - # No audio formats should be in results + # Audio formats without dimensions should not be in results, but templates OK for fmt in results: - assert fmt.type != Type.audio + if getattr(fmt, "accepts_parameters", None) and FormatIdParameter.dimensions in fmt.accepts_parameters: + continue + assert fmt.type != FormatCategory.audio assert fmt.renders assert len(fmt.renders) > 0 assert get_render_dimensions(fmt)[0] is not None def test_max_height_excludes_larger(self): - """Filter by max_height excludes formats taller than limit.""" - results = filter_formats(max_height=250) + """Filter by max_height includes templates and excludes concrete formats taller than limit.""" + results = filter_formats(max_height=500) assert len(results) > 0 for fmt in results: + if getattr(fmt, "accepts_parameters", None) and FormatIdParameter.dimensions in fmt.accepts_parameters: + continue assert fmt.renders assert get_render_dimensions(fmt)[1] is not None - assert get_render_dimensions(fmt)[1] <= 250 + assert get_render_dimensions(fmt)[1] <= 500 def test_min_width_excludes_smaller(self): - """Filter by min_width excludes formats narrower than limit.""" - results = filter_formats(min_width=728) + """Filter by min_width includes templates and excludes concrete formats narrower than limit.""" + results = filter_formats(min_width=1000) assert len(results) > 0 for fmt in results: + if getattr(fmt, "accepts_parameters", None) and FormatIdParameter.dimensions in fmt.accepts_parameters: + continue assert fmt.renders assert get_render_dimensions(fmt)[0] is not None - assert get_render_dimensions(fmt)[0] >= 728 + assert get_render_dimensions(fmt)[0] >= 1000 def test_min_height_excludes_smaller(self): - """Filter by min_height excludes formats shorter than limit.""" - results = filter_formats(min_height=600) + """Filter by min_height includes templates and excludes concrete formats shorter than limit.""" + results = filter_formats(min_height=1000) assert len(results) > 0 for fmt in results: + if getattr(fmt, "accepts_parameters", None) and FormatIdParameter.dimensions in fmt.accepts_parameters: + continue assert fmt.renders assert get_render_dimensions(fmt)[1] is not None - assert get_render_dimensions(fmt)[1] >= 600 + assert get_render_dimensions(fmt)[1] >= 1000 def test_combined_min_max_filters(self): """Combine min and max dimension filters.""" - results = filter_formats(min_width=300, max_width=970, min_height=250, max_height=600) + results = filter_formats(min_width=300, max_width=2000, min_height=400, max_height=2000) assert len(results) > 0 for fmt in results: + if getattr(fmt, "accepts_parameters", None) and FormatIdParameter.dimensions in fmt.accepts_parameters: + continue assert fmt.renders w, h = get_render_dimensions(fmt) - assert 300 <= w <= 970 - assert 250 <= h <= 600 + assert 300 <= w <= 2000 + assert 400 <= h <= 2000 class TestTypeFiltering: @@ -106,35 +124,35 @@ def test_filter_by_display_type(self): results = filter_formats(type="display") assert len(results) > 0 for fmt in results: - assert fmt.type == Type.display + assert fmt.type == FormatCategory.display def test_filter_by_audio_type(self): """Filter by audio type returns only audio formats.""" results = filter_formats(type="audio") assert len(results) > 0 for fmt in results: - assert fmt.type == Type.audio + assert fmt.type == FormatCategory.audio def test_filter_by_video_type(self): """Filter by video type returns only video formats.""" results = filter_formats(type="video") assert len(results) > 0 for fmt in results: - assert fmt.type == Type.video + assert fmt.type == FormatCategory.video def test_filter_by_native_type(self): """Filter by native type returns only native formats.""" results = filter_formats(type="native") assert len(results) > 0 for fmt in results: - assert fmt.type == Type.native + assert fmt.type == FormatCategory.native def test_filter_by_dooh_type(self): """Filter by DOOH type returns only DOOH formats.""" results = filter_formats(type="dooh") assert len(results) > 0 for fmt in results: - assert fmt.type == Type.dooh + assert fmt.type == FormatCategory.dooh class TestCombinedFiltering: @@ -145,7 +163,10 @@ def test_type_and_dimensions(self): results = filter_formats(type="display", dimensions="970x250") assert len(results) > 0 for fmt in results: - assert fmt.type == Type.display + # Template formats accept any dimensions + if getattr(fmt, "accepts_parameters", None) and FormatIdParameter.dimensions in fmt.accepts_parameters: + continue + assert fmt.type == FormatCategory.display assert get_render_dimensions(fmt)[0] == 970 assert get_render_dimensions(fmt)[1] == 250 @@ -154,7 +175,10 @@ def test_type_and_max_width(self): results = filter_formats(type="display", max_width=728) assert len(results) > 0 for fmt in results: - assert fmt.type == Type.display + # Template formats accept any dimensions + if getattr(fmt, "accepts_parameters", None) and FormatIdParameter.dimensions in fmt.accepts_parameters: + continue + assert fmt.type == FormatCategory.display assert get_render_dimensions(fmt)[0] <= 728 def test_dimensions_excludes_audio_even_with_no_type_filter(self): @@ -163,13 +187,16 @@ def test_dimensions_excludes_audio_even_with_no_type_filter(self): # when dimension filters are applied results = filter_formats(max_width=1920, max_height=1080) for fmt in results: + # Template formats accept any dimensions + if getattr(fmt, "accepts_parameters", None) and FormatIdParameter.dimensions in fmt.accepts_parameters: + continue # All results must have dimensions assert fmt.renders assert len(fmt.renders) > 0 assert get_render_dimensions(fmt)[0] is not None assert get_render_dimensions(fmt)[1] is not None # No audio formats should appear - assert fmt.type != Type.audio + assert fmt.type != FormatCategory.audio class TestNameSearch: @@ -224,15 +251,15 @@ def test_no_filters_returns_all_formats(self): results = filter_formats() # Should return all formats including audio, video, display, native, dooh, info card assert ( - len(results) == 42 - ) # 7 generative + 9 video + 7 display_image + 6 display_html + 2 native + 3 audio + 4 dooh + 4 info card + len(results) == 47 + ) # 8 generative (1 template + 7 concrete) + 11 video (2 templates + 9 concrete) + 8 display_image (1 template + 7 concrete) + 7 display_html (1 template + 6 concrete) + 2 native + 3 audio + 4 dooh + 4 info card # Verify we have multiple types types = {fmt.type for fmt in results} - assert Type.audio in types - assert Type.video in types - assert Type.display in types - assert Type.native in types - assert Type.dooh in types + assert FormatCategory.audio in types + assert FormatCategory.video in types + assert FormatCategory.display in types + assert FormatCategory.native in types + assert FormatCategory.dooh in types class TestBugReproduction: @@ -241,7 +268,7 @@ class TestBugReproduction: def test_no_filter_returns_audio_formats(self): """When no filters are applied, audio formats should be returned.""" results = filter_formats() - audio_formats = [fmt for fmt in results if fmt.type == Type.audio] + audio_formats = [fmt for fmt in results if fmt.type == FormatCategory.audio] assert len(audio_formats) > 0 def test_dimension_filter_excludes_audio_formats(self): @@ -254,11 +281,14 @@ def test_dimension_filter_excludes_audio_formats(self): results = filter_formats(dimensions="970x250") # Audio formats should NOT appear in results - audio_formats = [fmt for fmt in results if fmt.type == Type.audio] + audio_formats = [fmt for fmt in results if fmt.type == FormatCategory.audio] assert len(audio_formats) == 0, "Audio formats should not appear when filtering by dimensions" # Only display formats with 970x250 should appear for fmt in results: + # Template formats accept any dimensions + if getattr(fmt, "accepts_parameters", None) and FormatIdParameter.dimensions in fmt.accepts_parameters: + continue assert fmt.renders assert len(fmt.renders) > 0 assert get_render_dimensions(fmt)[0] == 970 diff --git a/tests/unit/test_format_card_renderer.py b/tests/unit/test_format_card_renderer.py index db66eb5..06c9bec 100644 --- a/tests/unit/test_format_card_renderer.py +++ b/tests/unit/test_format_card_renderer.py @@ -2,7 +2,7 @@ import json -from adcp.types.generated import FormatId +from adcp import FormatId from creative_agent.data.standard_formats import AGENT_URL, filter_formats from creative_agent.renderers.format_card_renderer import FormatCardRenderer diff --git a/tests/unit/test_info_card_formats.py b/tests/unit/test_info_card_formats.py index 66bc4bb..19ca5cd 100644 --- a/tests/unit/test_info_card_formats.py +++ b/tests/unit/test_info_card_formats.py @@ -1,6 +1,6 @@ """Tests for info card format definitions.""" -from adcp.types.generated import FormatId +from adcp import FormatId from creative_agent.data.format_types import AssetType, Type from creative_agent.data.standard_formats import AGENT_URL, INFO_CARD_FORMATS, filter_formats @@ -228,10 +228,16 @@ class TestInfoCardFormatsFiltering: """Test filtering behavior with info card formats.""" def test_filter_by_300x400_dimensions(self): - """Filter by 300x400 returns both standard card formats.""" + """Filter by 300x400 returns both standard card formats plus dimension templates.""" results = filter_formats(dimensions="300x400") - assert len(results) == 2 - result_ids = {fmt.format_id.id for fmt in results} + # Should have 2 concrete info card formats + 4 dimension templates + assert len(results) >= 2, f"Expected at least 2 formats, got {len(results)}" + + # Filter out templates to check concrete formats + concrete_results = [fmt for fmt in results if not getattr(fmt, "accepts_parameters", None)] + + assert len(concrete_results) == 2, f"Expected 2 concrete formats, got {len(concrete_results)}" + result_ids = {fmt.format_id.id for fmt in concrete_results} assert "product_card_standard" in result_ids assert "format_card_standard" in result_ids diff --git a/tests/unit/test_preview_generation.py b/tests/unit/test_preview_generation.py index 3b1a66a..308573f 100644 --- a/tests/unit/test_preview_generation.py +++ b/tests/unit/test_preview_generation.py @@ -4,7 +4,7 @@ """ import pytest -from adcp.types.generated import FormatId +from adcp import FormatId from creative_agent.data.standard_formats import AGENT_URL, get_format_by_id from creative_agent.schemas import CreativeManifest @@ -28,8 +28,6 @@ def spec_compliant_manifest_dict(self): """ # First create Pydantic objects to ensure schema compliance manifest_obj = CreativeManifest( - creative_id="test-creative", - name="Test Creative", format_id=FormatId(agent_url=AGENT_URL, id="display_300x250_image"), assets={ "banner_image": { @@ -83,8 +81,6 @@ def test_generate_html_sanitizes_javascript_urls(self, display_format, input_set """Test that javascript: URLs are sanitized for security.""" # Create manifest with malicious URL (still needs to be schema-valid structure) manifest = CreativeManifest( - creative_id="test-creative", - name="Test Creative", format_id=FormatId(agent_url=AGENT_URL, id="display_300x250_image"), assets={ "banner_image": { @@ -125,8 +121,6 @@ def test_generate_html_with_video_format(self, input_set): video_format = get_format_by_id(FormatId(agent_url=AGENT_URL, id="video_standard_15s")) manifest = CreativeManifest( - creative_id="test-creative", - name="Test Creative", format_id=FormatId(agent_url=AGENT_URL, id="video_standard_15s"), assets={ "video_file": { @@ -148,8 +142,6 @@ def test_manifest_can_have_optional_assets_not_required_by_format(self, display_ """Test that manifests can include optional assets beyond format requirements.""" # Include optional headline text asset (not required by format) manifest = CreativeManifest( - creative_id="test-creative", - name="Test Creative", format_id=FormatId(agent_url=AGENT_URL, id="display_300x250_image"), assets={ "banner_image": { diff --git a/tests/unit/test_product_card_renderer.py b/tests/unit/test_product_card_renderer.py index 5aaf93e..70f1158 100644 --- a/tests/unit/test_product_card_renderer.py +++ b/tests/unit/test_product_card_renderer.py @@ -1,6 +1,6 @@ """Tests for product card renderer.""" -from adcp.types.generated import FormatId +from adcp import FormatId from creative_agent.data.standard_formats import AGENT_URL, filter_formats from creative_agent.renderers.product_card_renderer import ProductCardRenderer diff --git a/tests/unit/test_storage_error_handling.py b/tests/unit/test_storage_error_handling.py index 4a8107e..51d597a 100644 --- a/tests/unit/test_storage_error_handling.py +++ b/tests/unit/test_storage_error_handling.py @@ -7,7 +7,7 @@ from unittest.mock import MagicMock, patch import pytest -from adcp.types.generated import FormatId +from adcp import FormatId from botocore.exceptions import ClientError, NoCredentialsError from creative_agent.data.standard_formats import AGENT_URL, get_format_by_id diff --git a/tests/validation/test_template_parameter_validation.py b/tests/validation/test_template_parameter_validation.py new file mode 100644 index 0000000..d464761 --- /dev/null +++ b/tests/validation/test_template_parameter_validation.py @@ -0,0 +1,322 @@ +"""Validation tests for template format parameters. + +Tests parameter validation for template formats including: +- Dimension parameters (width, height) +- Duration parameters (duration_ms) +- Parameter extraction from format_id +- Asset requirement parameter matching +""" + +import pytest +from adcp import FormatId +from pydantic import AnyUrl, ValidationError + +from creative_agent.data.standard_formats import AGENT_URL, get_format_by_id + + +class TestDimensionParameterValidation: + """Test validation of dimension parameters in format_id.""" + + def test_format_id_accepts_width_height_parameters(self): + """FormatId should accept width and height parameters.""" + format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="display_image", width=468, height=60) + + assert format_id.width == 468 + assert format_id.height == 60 + assert format_id.id == "display_image" + + def test_format_id_width_must_be_positive(self): + """FormatId width parameter should reject negative values.""" + # ADCP schema should validate this, but let's verify + # Note: Depending on ADCP library validation, this might pass + # If validation is loose, we need application-level validation + + # Try to create with negative width + try: + format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="display_image", width=-100, height=60) + # If it doesn't raise, check our validation catches it + assert format_id.width >= 0, "Width should be validated as positive" + except ValidationError: + # Good - library validates it + pass + + def test_format_id_height_must_be_positive(self): + """FormatId height parameter should reject negative values.""" + try: + format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="display_image", width=468, height=-60) + assert format_id.height >= 0, "Height should be validated as positive" + except ValidationError: + # Good - library validates it + pass + + def test_format_id_width_must_be_integer(self): + """FormatId width should be integer type.""" + format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="display_image", width=468, height=60) + + # Should be stored as int + assert isinstance(format_id.width, int) + + def test_format_id_dimensions_are_optional(self): + """FormatId should work without dimension parameters.""" + format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="display_image") + + # Should have no dimensions + assert getattr(format_id, "width", None) is None + assert getattr(format_id, "height", None) is None + + +class TestDurationParameterValidation: + """Test validation of duration parameters in format_id.""" + + def test_format_id_accepts_duration_ms_parameter(self): + """FormatId should accept duration_ms parameter.""" + format_id = FormatId( + agent_url=AnyUrl(str(AGENT_URL)), + id="video_standard", + duration_ms=30000, # 30 seconds + ) + + assert format_id.duration_ms == 30000 + assert format_id.id == "video_standard" + + def test_format_id_duration_must_be_positive(self): + """FormatId duration_ms should reject negative values.""" + try: + format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="video_standard", duration_ms=-5000) + assert format_id.duration_ms >= 0, "Duration should be validated as positive" + except ValidationError: + # Good - library validates it + pass + + def test_format_id_duration_is_milliseconds(self): + """FormatId duration_ms should be in milliseconds.""" + # 15 seconds = 15000ms + format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="video_standard", duration_ms=15000) + + assert format_id.duration_ms == 15000 + + # 30 seconds = 30000ms + format_id_30s = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="video_standard", duration_ms=30000) + + assert format_id_30s.duration_ms == 30000 + + +class TestParameterExtraction: + """Test extraction of parameters from format_id for asset validation.""" + + def test_extract_dimensions_from_format_id(self): + """Asset requirements should be able to reference format_id dimensions.""" + # Get template format + template_format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="display_image", width=468, height=60) + fmt = get_format_by_id(template_format_id) + + assert fmt is not None + assert fmt.assets_required is not None + + # Find image asset requirement + image_asset = None + for asset in fmt.assets_required: + if asset.asset_id == "banner_image": + image_asset = asset + break + + assert image_asset is not None + + # Should have parameters_from_format_id flag + requirements = getattr(image_asset, "requirements", {}) + assert requirements.get("parameters_from_format_id") is True, "Asset should specify parameters_from_format_id" + + # In actual validation, dimensions would be extracted from template_format_id + extracted_width = getattr(template_format_id, "width", None) + extracted_height = getattr(template_format_id, "height", None) + + assert extracted_width == 468, "Should extract width from format_id" + assert extracted_height == 60, "Should extract height from format_id" + + def test_extract_duration_from_format_id(self): + """Asset requirements should be able to reference format_id duration.""" + # Get video template with duration + template_format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="video_standard", duration_ms=15000) + fmt = get_format_by_id(template_format_id) + + assert fmt is not None + assert fmt.assets_required is not None + + # Video asset should reference format_id parameters + video_asset = fmt.assets_required[0] + requirements = getattr(video_asset, "requirements", {}) + assert requirements.get("parameters_from_format_id") is True + + # Extract duration + extracted_duration = getattr(template_format_id, "duration_ms", None) + assert extracted_duration == 15000 + + +class TestAssetParameterMatching: + """Test that asset properties match format_id parameters.""" + + def test_asset_dimensions_should_match_format_id_dimensions(self): + """When parameters_from_format_id is true, asset dimensions should match format_id.""" + # This would be validated by the validation logic, not just schema + # Testing the expected behavior + + format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="display_image", width=728, height=90) + + # Get format and check asset requirements + fmt = get_format_by_id(format_id) + image_asset = next(a for a in fmt.assets_required if a.asset_id == "banner_image") + + requirements = getattr(image_asset, "requirements", {}) + + # Should indicate parameters come from format_id + assert requirements.get("parameters_from_format_id") is True + + # In validation, we would check: + # - Asset width == format_id.width (728) + # - Asset height == format_id.height (90) + + def test_concrete_format_has_explicit_dimensions(self): + """Concrete formats should have explicit dimension requirements, not from format_id.""" + format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="display_300x250_image") + + fmt = get_format_by_id(format_id) + image_asset = next(a for a in fmt.assets_required if a.asset_id == "banner_image") + + requirements = getattr(image_asset, "requirements", {}) + + # Should NOT use parameters_from_format_id + assert requirements.get("parameters_from_format_id") is not True + + # Should have explicit values + assert requirements.get("width") == 300 + assert requirements.get("height") == 250 + + +class TestParameterValidationEdgeCases: + """Test edge cases and boundary conditions for parameter validation.""" + + def test_format_id_with_only_width_parameter(self): + """FormatId can have width without height (partial parameters).""" + format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="display_image", width=468) + + assert format_id.width == 468 + assert getattr(format_id, "height", None) is None + + def test_format_id_with_only_height_parameter(self): + """FormatId can have height without width (partial parameters).""" + format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="display_image", height=60) + + assert getattr(format_id, "width", None) is None + assert format_id.height == 60 + + def test_format_id_with_zero_dimensions(self): + """FormatId with zero dimensions should be rejected by ADCP schema.""" + # ADCP 2.12.0 validates dimensions must be >= 1 + from pydantic import ValidationError + + with pytest.raises(ValidationError) as exc_info: + FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="display_image", width=0, height=0) + + # Should raise validation error + assert "greater_than_equal" in str(exc_info.value) + + def test_format_id_with_large_dimensions(self): + """FormatId should accept large dimension values.""" + # DOOH screens can be very large + format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="display_image", width=3840, height=2160) + + assert format_id.width == 3840 + assert format_id.height == 2160 + + def test_format_id_with_very_long_duration(self): + """FormatId should accept long duration values.""" + # 60 second video = 60000ms + format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="video_standard", duration_ms=60000) + + assert format_id.duration_ms == 60000 + + def test_format_id_with_short_duration(self): + """FormatId should accept short duration values.""" + # 5 second video = 5000ms + format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="video_standard", duration_ms=5000) + + assert format_id.duration_ms == 5000 + + +class TestMultipleParameterTypes: + """Test formats that might accept multiple parameter types.""" + + def test_video_format_with_dimensions_and_duration(self): + """Video format could potentially accept both dimensions and duration.""" + # While our current video_dimensions template only accepts dimensions, + # theoretically a video could have both + + format_id = FormatId( + agent_url=AnyUrl(str(AGENT_URL)), id="video_dimensions", width=1920, height=1080, duration_ms=30000 + ) + + # FormatId should accept multiple parameters + assert format_id.width == 1920 + assert format_id.height == 1080 + assert format_id.duration_ms == 30000 + + def test_template_lookup_with_extra_parameters(self): + """Template lookup should work even with extra parameters.""" + from adcp.types.generated_poc.enums.format_id_parameter import FormatIdParameter + + # Look up display_image with duration parameter (which it doesn't use) + format_id = FormatId( + agent_url=AnyUrl(str(AGENT_URL)), + id="display_image", + width=468, + height=60, + duration_ms=15000, # Extra parameter + ) + + # Should still find the template + fmt = get_format_by_id(format_id) + assert fmt is not None + assert fmt.format_id.id == "display_image" + assert FormatIdParameter.dimensions in getattr(fmt, "accepts_parameters", []) + + +class TestParameterSerialization: + """Test that format_id parameters serialize correctly.""" + + def test_format_id_with_dimensions_serializes_correctly(self): + """FormatId with dimensions should serialize to JSON correctly.""" + import json + + format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="display_image", width=300, height=250) + + # Serialize with mode='json' to handle AnyUrl + format_id_dict = format_id.model_dump(mode="json", exclude_none=True) + format_id_json = json.dumps(format_id_dict) + + # Deserialize + format_id_restored = FormatId.model_validate_json(format_id_json) + + # Should preserve parameters + assert format_id_restored.width == 300 + assert format_id_restored.height == 250 + assert format_id_restored.id == "display_image" + + def test_format_id_without_parameters_serializes_correctly(self): + """FormatId without parameters should serialize cleanly (no null fields).""" + import json + + format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="display_image") + + # Serialize with exclude_none and mode='json' to handle AnyUrl + format_id_dict = format_id.model_dump(mode="json", exclude_none=True) + + # Should not have width, height, duration_ms keys + assert "width" not in format_id_dict + assert "height" not in format_id_dict + assert "duration_ms" not in format_id_dict + + # Should be valid JSON + format_id_json = json.dumps(format_id_dict) + format_id_restored = FormatId.model_validate_json(format_id_json) + + assert format_id_restored.id == "display_image" diff --git a/uv.lock b/uv.lock index 951da16..970d205 100644 --- a/uv.lock +++ b/uv.lock @@ -24,18 +24,19 @@ wheels = [ [[package]] name = "adcp" -version = "2.2.0" +version = "2.12.0" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "a2a-sdk" }, + { name = "email-validator" }, { name = "httpx" }, { name = "mcp" }, { name = "pydantic" }, { name = "typing-extensions" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/e9/59/60bb913f3baeb4ece9b99e66f0f7e767bed44d0fc77a5007197037c3ced0/adcp-2.2.0.tar.gz", hash = "sha256:6a69ed44f202df88e7e400a38595f0fe40d6475837205b9b3b6b6ca191422c85", size = 129722, upload-time = "2025-11-16T16:18:54.563Z" } +sdist = { url = "https://files.pythonhosted.org/packages/74/e5/c0582bcc0b0bf0caf30ca32c5124710c7091934efbac4b36d8983869a27b/adcp-2.12.0.tar.gz", hash = "sha256:b2b5035ffc013c4f9e82115f076538a5528a4fc1f6f5435702b4e2562370f8cf", size = 153689, upload-time = "2025-11-22T21:56:24.488Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/fc/ce/ac49498b553f1c480e28b6a915d4aa3f869ef17ffecc6d50d9cea8973cee/adcp-2.2.0-py3-none-any.whl", hash = "sha256:8ae1a9f7a547ffd8749d8dceabb9902b937c761093af2580e68dbc7e75dca0ce", size = 162270, upload-time = "2025-11-16T16:18:53.098Z" }, + { url = "https://files.pythonhosted.org/packages/34/e3/3f4293096d5a8372ccc820f345635d87770e608ee20ac1e966dd246e76f3/adcp-2.12.0-py3-none-any.whl", hash = "sha256:9ed5a14ae52d9150177af676713f0caf4269b08d4f8d2783e2b17c00bb2575ae", size = 190486, upload-time = "2025-11-22T21:56:22.815Z" }, ] [[package]] @@ -72,7 +73,7 @@ dev = [ [package.metadata] requires-dist = [ - { name = "adcp", specifier = ">=2.2.0" }, + { name = "adcp", specifier = ">=2.12.0" }, { name = "bleach", specifier = ">=6.3.0" }, { name = "boto3", specifier = ">=1.35.0" }, { name = "fastapi", specifier = ">=0.100.0" }, From b274604b78ffd2aa89e14fd03f4630fac394a3ca Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Sun, 23 Nov 2025 17:54:07 -0500 Subject: [PATCH 2/2] Fix mypy type errors in template formats - Use FormatIdParameter enum instead of strings for accepts_parameters - Add null checks for fmt.accepts_parameters before using 'in' operator - Add null check for render.dimensions before accessing width/height - Ensures type safety while maintaining all functionality --- src/creative_agent/data/standard_formats.py | 19 +++++++++---------- src/creative_agent/server.py | 2 +- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/creative_agent/data/standard_formats.py b/src/creative_agent/data/standard_formats.py index 20f6b79..d138d5a 100644 --- a/src/creative_agent/data/standard_formats.py +++ b/src/creative_agent/data/standard_formats.py @@ -123,7 +123,7 @@ def create_responsive_render( name="Display Banner - AI Generated", type=FormatCategory.display, description="AI-generated display banner from brand context and prompt (supports any dimensions)", - accepts_parameters=["dimensions"], + accepts_parameters=[FormatIdParameter.dimensions], supported_macros=COMMON_MACROS, assets_required=[ create_asset_required( @@ -313,7 +313,7 @@ def create_responsive_render( name="Standard Video", type=FormatCategory.video, description="Video ad in standard aspect ratios (supports any duration)", - accepts_parameters=["duration"], + accepts_parameters=[FormatIdParameter.duration], supported_macros=[*COMMON_MACROS, "VIDEO_ID", "POD_POSITION", "CONTENT_GENRE"], assets_required=[ create_asset_required( @@ -334,7 +334,7 @@ def create_responsive_render( name="Video with Dimensions", type=FormatCategory.video, description="Video ad with specific dimensions (supports any size)", - accepts_parameters=["dimensions"], + accepts_parameters=[FormatIdParameter.dimensions], supported_macros=[*COMMON_MACROS, "VIDEO_ID", "POD_POSITION", "CONTENT_GENRE"], assets_required=[ create_asset_required( @@ -538,7 +538,7 @@ def create_responsive_render( name="Display Banner - Image", type=FormatCategory.display, description="Static image banner (supports any dimensions)", - accepts_parameters=["dimensions"], + accepts_parameters=[FormatIdParameter.dimensions], supported_macros=COMMON_MACROS, assets_required=[ create_asset_required( @@ -758,7 +758,7 @@ def create_responsive_render( name="Display Banner - HTML5", type=FormatCategory.display, description="HTML5 creative (supports any dimensions)", - accepts_parameters=["dimensions"], + accepts_parameters=[FormatIdParameter.dimensions], supported_macros=COMMON_MACROS, assets_required=[ create_asset_required( @@ -1438,10 +1438,8 @@ def matches_format_id(fmt: CreativeFormat, search_id: tuple[str, str, int | None def matches_dimensions(fmt: CreativeFormat) -> bool: # Template formats accept dimensions but don't have fixed renders - if ( - getattr(fmt, "accepts_parameters", None) - and FormatIdParameter.dimensions in fmt.accepts_parameters - ): + accepts_params = getattr(fmt, "accepts_parameters", None) + if accepts_params is not None and FormatIdParameter.dimensions in accepts_params: return True # Concrete formats have fixed renders if not fmt.renders or len(fmt.renders) == 0: @@ -1472,7 +1470,8 @@ def get_dimensions(fmt: CreativeFormat) -> tuple[float | None, float | None]: filtered = [] for fmt in results: # Template formats accept any dimensions within the constraints - if getattr(fmt, "accepts_parameters", None) and FormatIdParameter.dimensions in fmt.accepts_parameters: + accepts_params = getattr(fmt, "accepts_parameters", None) + if accepts_params is not None and FormatIdParameter.dimensions in accepts_params: # Include template formats - they can satisfy any dimension requirements filtered.append(fmt) continue diff --git a/src/creative_agent/server.py b/src/creative_agent/server.py index 48070b1..5d831d5 100644 --- a/src/creative_agent/server.py +++ b/src/creative_agent/server.py @@ -650,7 +650,7 @@ def build_creative( if output_fmt.renders and len(output_fmt.renders) > 0: render = output_fmt.renders[0] - if render.dimensions.width and render.dimensions.height: + if render.dimensions and render.dimensions.width and render.dimensions.height: format_spec += f"Dimensions: {int(render.dimensions.width)}x{int(render.dimensions.height)}\n" format_spec += "\nRequired Assets:\n"