Skip to content

.git-pr-summary.md

Latest commit

 

History

History
183 lines (141 loc) · 6.34 KB

.git-pr-summary.md

File metadata and controls

183 lines (141 loc) · 6.34 KB

Fix adapter schema validation errors and add automated compliance testing

Summary

Fixed 22 pre-commit adapter schema validation errors and implemented comprehensive automated testing to prevent future schema drift between schema_adapters.py and the official AdCP specification.

Changes

1. Fixed Schema Mismatches (22 errors → 0 errors)

Field corrections:

  • ✅ ListCreativesResponse: Removed invalid message field
  • ✅ GetSignalsResponse: Added required message and context_id fields
  • ✅ ActivateSignalResponse: Fixed to use task_id/status instead of signal_id/activation_details
  • ✅ ListAuthorizedPropertiesResponse: Restored advertising_policies field (confirmed in AdCP spec)
  • ✅ UpdateMediaBuyResponse: Removed invalid status/task_id, made media_buy_id required, added missing implementation_date
  • ✅ GetMediaBuyDeliveryResponse: Added missing aggregated_totals field

Files changed:

  • src/core/main.py - Fixed 22 response constructor calls
  • src/core/schema_adapters.py - Updated 6 adapter schemas to match spec

2. Automated Compliance Testing (NEW)

Created: tests/unit/test_adapter_schema_compliance.py

Features:

  • Uses Pydantic's model_fields introspection to extract runtime field definitions
  • Parses official cached JSON schemas from tests/e2e/schemas/v1/
  • Single parametrized test covers all 10 adapter responses
  • 4-layer validation:
    1. ✅ Missing fields (spec → adapter)
    2. ✅ Extra fields (adapter → spec) - warns about internal fields
    3. ✅ Required/optional status matching
    4. ✅ Type compatibility checking

Coverage: 100% of adapter responses (10/10)

@pytest.mark.parametrize("adapter_class,schema_name", [
    (ListAuthorizedPropertiesResponse, "ListAuthorizedPropertiesResponse"),
    (GetSignalsResponse, "GetSignalsResponse"),
    (ActivateSignalResponse, "ActivateSignalResponse"),
    (UpdateMediaBuyResponse, "UpdateMediaBuyResponse"),
    (ListCreativesResponse, "ListCreativesResponse"),
    (CreateMediaBuyResponse, "CreateMediaBuyResponse"),
    (GetProductsResponse, "GetProductsResponse"),
    (GetMediaBuyDeliveryResponse, "GetMediaBuyDeliveryResponse"),
    (ListCreativeFormatsResponse, "ListCreativeFormatsResponse"),
    (SyncCreativesResponse, "SyncCreativesResponse"),
])
def test_response_adapter_matches_spec(self, adapter_class, schema_name):
    # Automated validation

Test results:

======================== 10 passed, 9 warnings in 0.64s ========================

Warnings: 9 adapters have extra protocol fields (documented as TODO for cleanup per AdCP PR #113)

3. Pre-Commit Hook Integration

Added: adapter-schema-compliance hook in .pre-commit-config.yaml

- id: adapter-schema-compliance
  name: Validate adapter schemas match AdCP spec
  entry: uv run pytest tests/unit/test_adapter_schema_compliance.py -v --tb=short
  files: '^(src/core/schema_adapters\.py|tests/e2e/schemas/v1/.*\.json)$'
  always_run: true

When it runs:

  • Every commit when schema_adapters.py changes
  • When official JSON schemas are updated
  • Fast execution (~0.6s)

What it catches:

  • Missing fields from official spec
  • Extra fields not in spec
  • Wrong required/optional status
  • Type mismatches

4. Documentation

Created: docs/testing/adapter-schema-compliance.md

Comprehensive guide covering:

  • Problem statement and three-layer schema architecture
  • How Pydantic introspection works
  • How mypy + Pydantic + compliance tests work together
  • Best practices for adding new fields/adapters
  • Integration with existing validation tools
  • Debugging guide
  • Future enhancements

Why This Matters

Problem: schema_adapters.py can drift from the official AdCP specification, causing:

  • Response construction errors (wrong field names)
  • Client validation failures (missing spec-required fields)
  • Protocol incompatibility (extra fields sent to clients)

Solution: Automated testing catches drift at commit time using:

  1. Pydantic introspection for runtime field metadata
  2. JSON schema parsing for official spec requirements
  3. Pre-commit hooks for immediate feedback

Benefits:

  • ✅ Prevents schema drift before it reaches CI/CD
  • ✅ Clear error messages with field-level details
  • ✅ Found 2 real bugs immediately (missing fields)
  • ✅ 100% coverage with minimal code (single parametrized test)
  • ✅ Easy to maintain (add new adapter = 1 line)

How Pydantic + mypy + Tests Work Together

Pydantic provides:

  • Runtime field introspection via model_fields
  • Validation at instantiation time
  • Type coercion and conversion

mypy provides:

  • Static type checking (str | None enforcement)
  • Field existence checking (catches typos)
  • SQLAlchemy 2.0 Mapped[] validation

Compliance tests provide:

  • Validation against external JSON schemas
  • Detection of missing spec fields
  • Detection of extra non-spec fields
  • Type compatibility checking

Why all three are needed:

  • mypy can't validate against external JSON schemas
  • Pydantic can't tell you if you're missing spec fields
  • Tests catch schema drift at commit time

Code Quality Improvements

Reduced duplication:

  • Before: 5 separate test methods × ~50 lines = 250+ lines
  • After: 1 parametrized test = ~50 lines (80% reduction)

Improved maintainability:

  • Adding new adapter = 1 line in parameter list
  • Validation logic centralized
  • Consistent error messages

Enhanced validation:

  • Missing fields ✅
  • Extra fields ✅ (with warnings)
  • Required/optional status ✅
  • Type compatibility ✅

Testing

All pre-commit hooks pass:

✅ pre-commit run validate-adapter-usage --all-files
✅ pre-commit run adapter-schema-compliance --all-files
✅ pytest tests/unit/test_adapter_schema_compliance.py -v
   10 passed, 9 warnings in 0.64s

No breaking changes - all existing code continues to work.

Related Issues

  • Addresses pre-commit failures in validate-adapter-usage hook
  • Implements AdCP PR #113 guidance (protocol fields excluded from domain responses)
  • Prevents schema drift bugs (e.g., issue #460 missing advertising_policies field)

Future Work

Documented as TODOs in code/tests:

  1. Clean up extra protocol fields in 9 adapters (per AdCP PR #113)
  2. Consider adding request adapter validation (lower priority)
  3. Optional: Add nested field validation for complex objects
  4. Optional: Schema auto-generation from JSON schemas