-
Notifications
You must be signed in to change notification settings - Fork 14
fix: AdCP responses now exclude None values in JSON serialization #642
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Problem: - AdCP client validates responses against JSON schemas that don't allow null for optional fields - AdCPBaseModel.model_dump() correctly excluded None, but model_dump_json() did not (Pydantic requires explicit config for JSON) - list_authorized_properties failed client-side validation with: "Expected array, received null" for optional fields Solution: - Override model_dump_json() in AdCPBaseModel to default exclude_none=True - Now both model_dump() and model_dump_json() exclude None by default - Matches AdCP spec requirement: optional fields should be omitted, not null Testing: - Added tests/unit/test_adcp_json_serialization.py with 4 test cases - Verified all existing tests still pass (833 unit tests) - Tested list_authorized_properties with minimal response Related: Fixes schema validation errors in adcp/client@2.5.1 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
bokelley
added a commit
that referenced
this pull request
Oct 27, 2025
Problem: - ListAuthorizedPropertiesResponse had a 'tags' field not in AdCP spec - Official spec at /schemas/v1/media-buy/list-authorized-properties-response.json only defines: publisher_domains, primary_channels, primary_countries, portfolio_description, advertising_policies, last_updated, errors - Client validation failed with "Expected array/string, received null" errors - Previous fix (PR #642) added exclude_none=True but didn't address root cause Root Cause: - The 'tags' field was added to both schemas.py and schema_adapters.py - This field is NOT in the official AdCP v2.2.0 specification - When None-valued optional fields were excluded, other None fields would still cause validation errors in clients Solution: - Remove 'tags' field from ListAuthorizedPropertiesResponse in both files - Now response only includes spec-compliant fields - Combined with PR #642's exclude_none=True, optional fields are properly omitted - Result: No null values in JSON, no non-spec fields Testing: - Updated test_adcp_contract.py to match spec (removed tags field test) - Unit tests pass (test_adcp_json_serialization.py) - AdCP contract tests pass (test_adcp_contract.py) - Response now validates against official schema Related: Fixes client validation errors from adcp/client@2.5.1 Follows up: PR #642 (exclude_none fix)
bokelley
added a commit
that referenced
this pull request
Oct 27, 2025
…#643) * fix: Remove non-spec tags field from ListAuthorizedPropertiesResponse Problem: - ListAuthorizedPropertiesResponse had a 'tags' field not in AdCP spec - Official spec at /schemas/v1/media-buy/list-authorized-properties-response.json only defines: publisher_domains, primary_channels, primary_countries, portfolio_description, advertising_policies, last_updated, errors - Client validation failed with "Expected array/string, received null" errors - Previous fix (PR #642) added exclude_none=True but didn't address root cause Root Cause: - The 'tags' field was added to both schemas.py and schema_adapters.py - This field is NOT in the official AdCP v2.2.0 specification - When None-valued optional fields were excluded, other None fields would still cause validation errors in clients Solution: - Remove 'tags' field from ListAuthorizedPropertiesResponse in both files - Now response only includes spec-compliant fields - Combined with PR #642's exclude_none=True, optional fields are properly omitted - Result: No null values in JSON, no non-spec fields Testing: - Updated test_adcp_contract.py to match spec (removed tags field test) - Unit tests pass (test_adcp_json_serialization.py) - AdCP contract tests pass (test_adcp_contract.py) - Response now validates against official schema Related: Fixes client validation errors from adcp/client@2.5.1 Follows up: PR #642 (exclude_none fix) * test: Remove tags field references from ListAuthorizedPropertiesResponse tests Updates all tests to match the spec-compliant schema which does not include a tags field. Tests now verify only the fields defined in the official AdCP v2.2.0 specification: - publisher_domains (required) - primary_channels, primary_countries, portfolio_description, advertising_policies, last_updated, errors (optional) Related: Follows up commit 633b498 (remove tags field from schema)
danf-newton
pushed a commit
to Newton-Research-Inc/salesagent
that referenced
this pull request
Nov 24, 2025
…contextprotocol#642) Problem: - AdCP client validates responses against JSON schemas that don't allow null for optional fields - AdCPBaseModel.model_dump() correctly excluded None, but model_dump_json() did not (Pydantic requires explicit config for JSON) - list_authorized_properties failed client-side validation with: "Expected array, received null" for optional fields Solution: - Override model_dump_json() in AdCPBaseModel to default exclude_none=True - Now both model_dump() and model_dump_json() exclude None by default - Matches AdCP spec requirement: optional fields should be omitted, not null Testing: - Added tests/unit/test_adcp_json_serialization.py with 4 test cases - Verified all existing tests still pass (833 unit tests) - Tested list_authorized_properties with minimal response Related: Fixes schema validation errors in adcp/client@2.5.1 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com>
danf-newton
pushed a commit
to Newton-Research-Inc/salesagent
that referenced
this pull request
Nov 24, 2025
…adcontextprotocol#643) * fix: Remove non-spec tags field from ListAuthorizedPropertiesResponse Problem: - ListAuthorizedPropertiesResponse had a 'tags' field not in AdCP spec - Official spec at /schemas/v1/media-buy/list-authorized-properties-response.json only defines: publisher_domains, primary_channels, primary_countries, portfolio_description, advertising_policies, last_updated, errors - Client validation failed with "Expected array/string, received null" errors - Previous fix (PR adcontextprotocol#642) added exclude_none=True but didn't address root cause Root Cause: - The 'tags' field was added to both schemas.py and schema_adapters.py - This field is NOT in the official AdCP v2.2.0 specification - When None-valued optional fields were excluded, other None fields would still cause validation errors in clients Solution: - Remove 'tags' field from ListAuthorizedPropertiesResponse in both files - Now response only includes spec-compliant fields - Combined with PR adcontextprotocol#642's exclude_none=True, optional fields are properly omitted - Result: No null values in JSON, no non-spec fields Testing: - Updated test_adcp_contract.py to match spec (removed tags field test) - Unit tests pass (test_adcp_json_serialization.py) - AdCP contract tests pass (test_adcp_contract.py) - Response now validates against official schema Related: Fixes client validation errors from adcp/client@2.5.1 Follows up: PR adcontextprotocol#642 (exclude_none fix) * test: Remove tags field references from ListAuthorizedPropertiesResponse tests Updates all tests to match the spec-compliant schema which does not include a tags field. Tests now verify only the fields defined in the official AdCP v2.2.0 specification: - publisher_domains (required) - primary_channels, primary_countries, portfolio_description, advertising_policies, last_updated, errors (optional) Related: Follows up commit 633b498 (remove tags field from schema)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
The AdCP client (
adcp/client@2.5.1) validates responses against JSON schemas that don't allownullfor optional fields. When callinglist_authorized_propertieswith minimal configuration, the response includednullvalues for optional fields likeprimary_channels,primary_countries, etc., causing validation errors:Root Cause
AdCPBaseModel.model_dump()correctly excluded None values (exclude_none=Trueby default)model_dump_json()did not inherit this behavior (Pydantic requires explicit configuration for JSON serialization)model_dump_json()for serialization"field": nullinstead of omitting the field entirelySolution
Override
model_dump_json()inAdCPBaseModelto defaultexclude_none=True, matching the behavior ofmodel_dump(). This ensures:Testing
tests/unit/test_adcp_json_serialization.pywith 4 comprehensive test caseslist_authorized_properties,get_products, andlist_creative_formatsall correctly exclude None valuesExample
Before:
{ "publisher_domains": ["example.com"], "primary_channels": null, "primary_countries": null, "portfolio_description": null, "advertising_policies": null }After:
{ "publisher_domains": ["example.com"] }Impact
nullanyway)Fixes validation errors in
adcp/client@2.5.1