-
Notifications
You must be signed in to change notification settings - Fork 13
Description
AdCP Schema Compliance Issues - Comprehensive Audit
Overview
Multiple adapters are constructing AdCP response objects with invalid fields that don't exist in the AdCP v2.4 spec. Per AdCP PR #113, response schemas contain ONLY domain data. Protocol fields like status, message, reason, detail belong in the protocol envelope wrapper, not the domain response.
Impact
- Severity: HIGH - Causes Pydantic validation errors in production
- Scope: 4 adapters (GAM, Kevel, Triton, Mock) + main.py orchestration code
- Status: GAM adapter partially fixed, others remain broken
- Production Evidence: See Fly.io logs 2025-10-17T01:15:18Z - CreateMediaBuyResponse validation errors
Affected Response Schemas
1. CreateMediaBuyResponse
Valid Fields (per src/core/schemas.py:2378-2393):
buyer_ref(required)media_buy_id(optional)creative_deadline(optional)packages(optional)errors(optional) - Use this for error reporting
Invalid Fields Being Used:
- ❌
status- not in spec - ❌
message- not in spec - ❌
detail- not in spec
Affected Files:
- ✅
src/adapters/google_ad_manager.py- FIXED (7 instances corrected) - ❌
src/adapters/kevel.py:209- Invalid:status="failed", detail=error_msg - ❌
src/adapters/triton_digital.py:139- Invalid:status="failed", detail=error_msg
2. UpdateMediaBuyResponse
Valid Fields (per src/core/schemas.py:2623-2638):
media_buy_id(required)buyer_ref(required)implementation_date(optional)affected_packages(optional)errors(optional) - Use this for error reporting
Invalid Fields Being Used:
- ❌
status- not in spec - ❌
reason- not in spec - ❌
detail- not in spec
Affected Files:
- ❌
src/adapters/kevel.py:603- Invalid:status="failed", reason=... - ❌
src/adapters/kevel.py:626- Invalid:status="failed", reason=... - ❌
src/adapters/kevel.py:643-644- Invalid:status="accepted", detail=... - ❌
src/adapters/kevel.py:649- Invalid:status="failed", reason=... - ❌
src/adapters/triton_digital.py:541- Invalid:status="failed", reason=... - ❌
src/adapters/triton_digital.py:564- Invalid:status="failed", reason=... - ❌
src/adapters/triton_digital.py:588- Invalid:status="failed", reason=... - ❌
src/adapters/mock_ad_server.py:1105- Invalid:status="accepted"
CRITICAL - Orchestration Code Broken:
- ❌
src/core/main.py:5619- Checksresult.status(doesn't exist!) - ❌
src/core/main.py:5635- Checksresult.status - ❌
src/core/main.py:5652- Checksresult.status - ❌
src/core/main.py:5667- Checksresult.status
Root Cause Analysis
Issue 1: Missing buyer_ref Parameter
The adapter interface signature doesn't include buyer_ref:
# src/adapters/base.py:150-153
@abstractmethod
def update_media_buy(
self, media_buy_id: str, action: str, package_id: str | None, budget: int | None, today: datetime
) -> UpdateMediaBuyResponse:But UpdateMediaBuyResponse requires it:
# src/core/schemas.py:2632-2633
media_buy_id: str = Field(..., description="Publisher's unique identifier")
buyer_ref: str = Field(..., description="Buyer's reference identifier") # REQUIRED!Solution: Either:
- Add
buyer_refto adapter interface signature, OR - Have adapters fetch it from database via MediaBuy model lookup
Issue 2: Status/Message Pattern Instead of Errors Array
Adapters are using protocol-level fields instead of the domain-level errors array:
❌ Current Pattern (Invalid):
return UpdateMediaBuyResponse(status="failed", reason="Flight not found")✅ Correct Pattern:
return UpdateMediaBuyResponse(
media_buy_id=media_buy_id,
buyer_ref=buyer_ref, # Fetch from DB or parameter
errors=[Error(code="flight_not_found", message="Flight not found")]
)Issue 3: Orchestration Code Depends on Non-Existent Fields
src/core/main.py checks result.status which doesn't exist:
# Line 5619 - BROKEN!
result = adapter.update_media_buy(...)
if result.status == "failed": # ❌ UpdateMediaBuyResponse has no 'status' field!
return resultSolution: Check result.errors instead:
result = adapter.update_media_buy(...)
if result.errors:
return result # Protocol layer will wrap with appropriate statusFix Strategy
Phase 1: Update Adapter Interface (Breaking Change)
Update base adapter signature to include buyer_ref:
# src/adapters/base.py
@abstractmethod
def update_media_buy(
self,
media_buy_id: str,
buyer_ref: str, # NEW: Required for response construction
action: str,
package_id: str | None,
budget: int | None,
today: datetime
) -> UpdateMediaBuyResponse:Phase 2: Update All Adapter Implementations
For each adapter (GAM, Kevel, Triton, Mock):
- Add buyer_ref parameter to method signature
- Remove all
status,message,reason,detailfields from response constructions - Use errors array for error reporting:
errors=[Error(code="operation_failed", message="Error details")]
- Ensure required fields are always included:
- CreateMediaBuyResponse:
buyer_ref(required) - UpdateMediaBuyResponse:
media_buy_id,buyer_ref(both required)
- CreateMediaBuyResponse:
Phase 3: Update Orchestration Code
In src/core/main.py _update_media_buy_impl():
Change from:
result = adapter.update_media_buy(...)
if result.status == "failed":
return resultTo:
result = adapter.update_media_buy(
media_buy_id=req.media_buy_id,
buyer_ref=req.buyer_ref, # Pass through from request
action=action,
...
)
if result.errors:
# Protocol layer will wrap this with appropriate status/message
return resultPhase 4: Testing
- Unit Tests: Update all adapter unit tests to pass buyer_ref
- Integration Tests: Verify error handling uses errors array
- E2E Tests: Confirm protocol envelope wraps responses correctly
- Manual Testing: Test against production-like scenarios
Files Requiring Changes
High Priority (Blocking Production):
-
src/adapters/base.py- Update interface signature -
src/adapters/google_ad_manager.py- Update update_media_buy signature (CreateMediaBuyResponse already fixed) -
src/adapters/kevel.py- Fix CreateMediaBuyResponse (line 209) + UpdateMediaBuyResponse (lines 603, 626, 643-644, 649) -
src/adapters/triton_digital.py- Fix CreateMediaBuyResponse (line 139) + UpdateMediaBuyResponse (lines 541, 564, 588) -
src/adapters/mock_ad_server.py- Fix UpdateMediaBuyResponse (line 1105) -
src/core/main.py- Fix orchestration code checking result.status (lines 5619, 5635, 5652, 5667)
Medium Priority (Testing):
- All adapter unit tests - Update to pass buyer_ref parameter
- Integration tests - Verify error handling patterns
- E2E tests - Confirm protocol envelope behavior
Documentation:
- Update adapter development guide with correct response patterns
- Add schema compliance checklist for new adapters
- Document protocol vs domain field separation
Related Issues
- AdCP PR Fix PostgreSQL connection issues with proper pooling and retry logic #113: Protocol/domain field separation
- Production bug: "Extra inputs are not permitted" validation errors
- FormatId field extraction:
idvsformat_id(partially fixed)
Notes
- ✅ GAM adapter CreateMediaBuyResponse issues fixed in commit adcce5a
- ✅ FormatId extraction fixed to use
idfield (commit 70903dd) ⚠️ Those commits are incomplete without fixing update_media_buy - may need to revert and redo comprehensively- 📋 This issue documents all remaining schema compliance problems for a coordinated fix
Estimated Effort
- Interface changes: 2-3 hours
- Adapter implementations: 4-6 hours (4 adapters × ~1.5h each)
- Orchestration code: 1-2 hours
- Testing updates: 3-4 hours
- Documentation: 1 hour
- Total: ~12-16 hours of focused work
Success Criteria
- All response constructions use only AdCP spec-compliant fields
- No Pydantic validation errors in production
- All adapter tests pass
- Integration/E2E tests confirm proper error handling
- Protocol envelope correctly wraps domain responses