From 870fdcc47c34054d94fdea5680e0fafa4aadf2f6 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Fri, 26 Dec 2025 16:14:23 -0400 Subject: [PATCH 1/4] Add parameterized display_js and video_vast formats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds two new template formats that accept parameters, enabling the sales agent's template picker to validate user-configured templates: - display_js: JavaScript-based display ads accepting dimensions parameter - video_vast: VAST video ads accepting duration parameter (parameterized version of the existing fixed video_vast_30s format) Fixes #40 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- src/creative_agent/data/standard_formats.py | 47 +++++++++++++++++++++ tests/integration/test_template_formats.py | 38 +++++++++-------- tests/unit/test_filter_formats.py | 5 +-- 3 files changed, 70 insertions(+), 20 deletions(-) diff --git a/src/creative_agent/data/standard_formats.py b/src/creative_agent/data/standard_formats.py index d138d5a..beb3203 100644 --- a/src/creative_agent/data/standard_formats.py +++ b/src/creative_agent/data/standard_formats.py @@ -349,6 +349,26 @@ def create_responsive_render( ), ], ), + # Template format - VAST tag with any duration + CreativeFormat( + format_id=create_format_id("video_vast"), + name="VAST Video", + type=FormatCategory.video, + description="Video ad via VAST tag (supports any duration)", + accepts_parameters=[FormatIdParameter.duration], + supported_macros=[*COMMON_MACROS, "VIDEO_ID", "POD_POSITION", "CONTENT_GENRE"], + assets_required=[ + create_asset_required( + asset_id="vast_tag", + asset_type=AssetType.text, + required=True, + requirements={ + "parameters_from_format_id": True, + "description": "VAST 4.x compatible tag matching format_id duration", + }, + ), + ], + ), # Concrete formats for backward compatibility CreativeFormat( format_id=create_format_id("video_standard_30s"), @@ -897,6 +917,32 @@ def create_responsive_render( ), ] +# Display Formats - JavaScript +# Template format for JavaScript-based display ads +DISPLAY_JS_FORMATS = [ + # Template format - supports any dimensions + CreativeFormat( + format_id=create_format_id("display_js"), + name="Display Banner - JavaScript", + type=FormatCategory.display, + description="JavaScript-based display ad (supports any dimensions)", + accepts_parameters=[FormatIdParameter.dimensions], + supported_macros=COMMON_MACROS, + assets_required=[ + create_asset_required( + asset_id="js_creative", + asset_type=AssetType.javascript, + required=True, + requirements={ + "parameters_from_format_id": True, + "max_file_size_mb": 0.5, + "description": "JavaScript creative code matching format_id dimensions", + }, + ), + ], + ), +] + # Native Formats NATIVE_FORMATS = [ CreativeFormat( @@ -1348,6 +1394,7 @@ def create_responsive_render( + VIDEO_FORMATS + DISPLAY_IMAGE_FORMATS + DISPLAY_HTML_FORMATS + + DISPLAY_JS_FORMATS + NATIVE_FORMATS + AUDIO_FORMATS + DOOH_FORMATS diff --git a/tests/integration/test_template_formats.py b/tests/integration/test_template_formats.py index 0fffbf2..3260566 100644 --- a/tests/integration/test_template_formats.py +++ b/tests/integration/test_template_formats.py @@ -29,7 +29,7 @@ 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)}" + assert len(template_formats) == 7, f"Expected 7 template formats, found {len(template_formats)}" # Verify template format IDs template_ids = {f.format_id.id for f in template_formats} @@ -37,8 +37,10 @@ def test_standard_formats_include_templates(self): "display_generative", "display_image", "display_html", + "display_js", "video_standard", "video_dimensions", + "video_vast", } assert template_ids == expected_ids, f"Template IDs mismatch: {template_ids} != {expected_ids}" @@ -85,7 +87,7 @@ def test_list_creative_formats_returns_templates(self): # 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)}" + assert len(template_formats) == 7, f"Expected 7 template formats in response, found {len(template_formats)}" class TestTemplateFormatFiltering: @@ -100,11 +102,11 @@ def test_filter_by_type_includes_templates(self): # 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)}" + assert len(template_displays) == 4, f"Expected 4 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"} + expected_ids = {"display_generative", "display_image", "display_html", "display_js"} assert template_ids == expected_ids def test_filter_by_type_video_includes_templates(self): @@ -113,13 +115,13 @@ def test_filter_by_type_video_includes_templates(self): video_formats = filter_formats(type=FormatCategory.video) - # Should include 2 video templates + 9 concrete + # Should include 3 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)}" + assert len(template_videos) == 3, f"Expected 3 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"} + expected_ids = {"video_standard", "video_dimensions", "video_vast"} assert template_ids == expected_ids def test_dimension_filter_includes_templates(self): @@ -133,9 +135,9 @@ def test_dimension_filter_includes_templates(self): 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)}" + # Should have display_generative, display_image, display_html, display_js, video_dimensions + assert len(dimension_templates) >= 4, ( + f"Expected at least 4 dimension templates for 468x60, found {len(dimension_templates)}" ) # All should accept dimensions parameter @@ -153,7 +155,7 @@ def test_max_width_filter_includes_templates(self): ] # Template formats can satisfy any dimension requirement - assert len(dimension_templates) >= 3, "Should include dimension-accepting templates" + assert len(dimension_templates) >= 4, "Should include dimension-accepting templates" class TestTemplateFormatLookup: @@ -373,10 +375,11 @@ def test_have_display_dimension_templates(self): template_ids = {f.format_id.id for f in display_templates} - # Should have templates for: generative, image, html + # Should have templates for: generative, image, html, js assert "display_generative" in template_ids assert "display_image" in template_ids assert "display_html" in template_ids + assert "display_js" in template_ids def test_have_video_templates(self): """Should have template formats for video with both duration and dimensions.""" @@ -388,7 +391,8 @@ def test_have_video_templates(self): 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" + # video_standard and video_vast accept duration, video_dimensions accepts dimensions + assert len(duration_templates) >= 2, "Should have video duration templates (video_standard, video_vast)" assert len(dimension_templates) >= 1, "Should have video dimension template" def test_template_to_concrete_ratio(self): @@ -397,11 +401,11 @@ def test_template_to_concrete_ratio(self): 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)}" + # Expected: 7 templates + 42 concrete = 49 total + assert len(templates) == 7, f"Expected 7 templates, found {len(templates)}" assert len(concrete) == 42, f"Expected 42 concrete formats, found {len(concrete)}" - assert len(STANDARD_FORMATS) == 47 + assert len(STANDARD_FORMATS) == 49 - # Ratio should be roughly 1:8 (5 templates replace ~40 potential concrete formats) + # Ratio should be roughly 1:6 (7 templates replace ~42 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/unit/test_filter_formats.py b/tests/unit/test_filter_formats.py index 30b3763..4f21887 100644 --- a/tests/unit/test_filter_formats.py +++ b/tests/unit/test_filter_formats.py @@ -250,9 +250,8 @@ def test_no_filters_returns_all_formats(self): """No filters returns all standard formats.""" results = filter_formats() # Should return all formats including audio, video, display, native, dooh, info card - assert ( - 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 + # 8 generative (1 template + 7 concrete) + 12 video (3 templates + 9 concrete) + 8 display_image (1 template + 7 concrete) + 7 display_html (1 template + 6 concrete) + 1 display_js template + 2 native + 3 audio + 4 dooh + 4 info card = 49 + assert len(results) == 49 # Verify we have multiple types types = {fmt.type for fmt in results} assert FormatCategory.audio in types From 398f596f1b985c2ca5ce56ae86bafb1c54ceaca9 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Fri, 26 Dec 2025 16:29:34 -0400 Subject: [PATCH 2/4] Fix video_vast asset type to use AssetType.vast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the dedicated `vast` asset type instead of `text` for VAST tag assets. Also fixes the existing video_vast_30s format which had the same issue. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- src/creative_agent/data/standard_formats.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/creative_agent/data/standard_formats.py b/src/creative_agent/data/standard_formats.py index beb3203..0087d69 100644 --- a/src/creative_agent/data/standard_formats.py +++ b/src/creative_agent/data/standard_formats.py @@ -360,7 +360,7 @@ def create_responsive_render( assets_required=[ create_asset_required( asset_id="vast_tag", - asset_type=AssetType.text, + asset_type=AssetType.vast, required=True, requirements={ "parameters_from_format_id": True, @@ -417,7 +417,7 @@ def create_responsive_render( assets_required=[ create_asset_required( asset_id="vast_tag", - asset_type=AssetType.text, + asset_type=AssetType.vast, required=True, requirements={ "description": "VAST 4.x compatible tag", From 17374715e5fd9ae80d6f9ece27e1dc90a37bc996 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Fri, 26 Dec 2025 16:35:40 -0400 Subject: [PATCH 3/4] Simplify requirements to only parameters_from_format_id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove duplicative description and max_file_size_mb fields from asset requirements - they don't add value beyond the format-level description. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- src/creative_agent/data/standard_formats.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/creative_agent/data/standard_formats.py b/src/creative_agent/data/standard_formats.py index 0087d69..5cb6cff 100644 --- a/src/creative_agent/data/standard_formats.py +++ b/src/creative_agent/data/standard_formats.py @@ -364,7 +364,6 @@ def create_responsive_render( required=True, requirements={ "parameters_from_format_id": True, - "description": "VAST 4.x compatible tag matching format_id duration", }, ), ], @@ -935,8 +934,6 @@ def create_responsive_render( required=True, requirements={ "parameters_from_format_id": True, - "max_file_size_mb": 0.5, - "description": "JavaScript creative code matching format_id dimensions", }, ), ], From 6b922986a73489b017836c47c28933387d729d0d Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Fri, 26 Dec 2025 16:40:25 -0400 Subject: [PATCH 4/4] Remove redundant parameters_from_format_id from all formats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The accepts_parameters field on the format already indicates which parameters are accepted from FormatId. Having parameters_from_format_id on each asset was redundant and not actually used by any code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- src/creative_agent/data/standard_formats.py | 14 ----- tests/integration/test_template_formats.py | 38 ++---------- .../test_template_parameter_validation.py | 58 ++++++------------- 3 files changed, 22 insertions(+), 88 deletions(-) diff --git a/src/creative_agent/data/standard_formats.py b/src/creative_agent/data/standard_formats.py index 5cb6cff..46f3ef8 100644 --- a/src/creative_agent/data/standard_formats.py +++ b/src/creative_agent/data/standard_formats.py @@ -321,9 +321,7 @@ def create_responsive_render( asset_type=AssetType.video, required=True, requirements={ - "parameters_from_format_id": True, "acceptable_formats": ["mp4", "mov", "webm"], - "description": "Video file matching format_id duration", }, ), ], @@ -342,9 +340,7 @@ def create_responsive_render( asset_type=AssetType.video, required=True, requirements={ - "parameters_from_format_id": True, "acceptable_formats": ["mp4", "mov", "webm"], - "description": "Video file matching format_id dimensions", }, ), ], @@ -362,9 +358,6 @@ def create_responsive_render( asset_id="vast_tag", asset_type=AssetType.vast, required=True, - requirements={ - "parameters_from_format_id": True, - }, ), ], ), @@ -565,9 +558,7 @@ def create_responsive_render( 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( @@ -785,9 +776,7 @@ def create_responsive_render( 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", }, ), ], @@ -932,9 +921,6 @@ def create_responsive_render( asset_id="js_creative", asset_type=AssetType.javascript, required=True, - requirements={ - "parameters_from_format_id": True, - }, ), ], ), diff --git a/tests/integration/test_template_formats.py b/tests/integration/test_template_formats.py index 3260566..fb07d26 100644 --- a/tests/integration/test_template_formats.py +++ b/tests/integration/test_template_formats.py @@ -213,10 +213,10 @@ def test_get_video_template_by_base_id(self): class TestTemplateAssetRequirements: - """Test asset requirements with parameters_from_format_id.""" + """Test asset requirements for template vs concrete formats.""" - def test_template_assets_have_parameters_from_format_id(self): - """Template format assets should use parameters_from_format_id.""" + def test_template_formats_have_assets_required(self): + """Template formats should have assets_required defined.""" # Get display_image template format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="display_image") fmt = get_format_by_id(format_id) @@ -234,35 +234,8 @@ def test_template_assets_have_parameters_from_format_id(self): 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.""" + """Concrete formats should have explicit dimension requirements.""" # Get concrete format format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="display_300x250_image") fmt = get_format_by_id(format_id) @@ -288,9 +261,6 @@ def test_concrete_formats_have_explicit_requirements(self): 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.""" diff --git a/tests/validation/test_template_parameter_validation.py b/tests/validation/test_template_parameter_validation.py index d464761..48270ae 100644 --- a/tests/validation/test_template_parameter_validation.py +++ b/tests/validation/test_template_parameter_validation.py @@ -9,6 +9,7 @@ import pytest from adcp import FormatId +from adcp.types.generated_poc.enums.format_id_parameter import FormatIdParameter from pydantic import AnyUrl, ValidationError from creative_agent.data.standard_formats import AGENT_URL, get_format_by_id @@ -105,28 +106,18 @@ 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 + """Dimensions can be extracted from format_id for template formats.""" + # Get template format with dimensions 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" + # Template format accepts dimensions parameter + assert FormatIdParameter.dimensions in getattr(fmt, "accepts_parameters", []) - # In actual validation, dimensions would be extracted from template_format_id + # Dimensions can be extracted from format_id extracted_width = getattr(template_format_id, "width", None) extracted_height = getattr(template_format_id, "height", None) @@ -134,7 +125,7 @@ def test_extract_dimensions_from_format_id(self): 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.""" + """Duration can be extracted from format_id for template formats.""" # 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) @@ -142,12 +133,10 @@ def test_extract_duration_from_format_id(self): 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 + # Template format accepts duration parameter + assert FormatIdParameter.duration in getattr(fmt, "accepts_parameters", []) - # Extract duration + # Duration can be extracted from format_id extracted_duration = getattr(template_format_id, "duration_ms", None) assert extracted_duration == 15000 @@ -156,27 +145,19 @@ 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 - + """Template formats accept dimensions from format_id.""" format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="display_image", width=728, height=90) - # Get format and check asset requirements + # Get format - template should accept dimensions 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 + assert FormatIdParameter.dimensions in getattr(fmt, "accepts_parameters", []) - # In validation, we would check: - # - Asset width == format_id.width (728) - # - Asset height == format_id.height (90) + # Format_id carries the dimensions + assert format_id.width == 728 + assert format_id.height == 90 def test_concrete_format_has_explicit_dimensions(self): - """Concrete formats should have explicit dimension requirements, not from format_id.""" + """Concrete formats should have explicit dimension requirements.""" format_id = FormatId(agent_url=AnyUrl(str(AGENT_URL)), id="display_300x250_image") fmt = get_format_by_id(format_id) @@ -184,10 +165,7 @@ def test_concrete_format_has_explicit_dimensions(self): 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 + # Concrete format has explicit values in requirements assert requirements.get("width") == 300 assert requirements.get("height") == 250