Skip to content

fix(compat): warn on non-standard brand_manifest path in inline-object branch#688

Merged
bokelley merged 1 commit into
mainfrom
claude/issue-687-warn-inline-object-brand-manifest
May 12, 2026
Merged

fix(compat): warn on non-standard brand_manifest path in inline-object branch#688
bokelley merged 1 commit into
mainfrom
claude/issue-687-warn-inline-object-brand-manifest

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Summary

Fixes #687. After #685 (inline-object branch) and #686 (non-standard-path warning) landed, the warning was applied only to the URL-string branch. The v2.5 brand-manifest-ref schema allows brand_manifest to be either a URL string or an inline BrandManifest object — the inline object's url field carries the same path semantics, and the same information loss applies (v3 sellers fetch the canonical {domain}/.well-known/brand.json and will likely 404).

Changes

  • src/adcp/compat/legacy/v2_5/_url.py: new warn_brand_manifest_path_lossy(manifest_url, domain) function plus the _STANDARD_BRAND_MANIFEST_PATHS constant and _brand_manifest_path_warned dedup set (moved from per-adapter modules). Logger is adcp.compat.legacy.v2_5._url.
  • src/adcp/compat/legacy/v2_5/get_products.py and _media_buy_helpers.py: remove the per-module helpers; call warn_brand_manifest_path_lossy from both the string and inline-object branches.
  • Tests updated for the new monkeypatch target and logger name. Four new regression tests cover the inline-object warning behaviour (CDN url warns, canonical url silent) on both adapters.

Test plan

  • pytest tests/test_legacy_adapter_v2_5_*.py — 64 passed (was 60; +4 new dict-branch warning tests)
  • Verified the two new tests fail against pre-fix source with assert 0 == 1 (no warning emitted)
  • ruff check and mypy src/adcp/compat/legacy/v2_5/ clean
  • Black formatted (pre-commit re-ran)

Logger-name change

Operators previously alerting on adcp.compat.legacy.v2_5.get_products / adcp.compat.legacy.v2_5._media_buy_helpers warnings will need to update the logger name to adcp.compat.legacy.v2_5._url. The warning message text is unchanged, so grep-based alerting on "non-standard path" is unaffected. Acceptable shift since #686 landed only an hour ago and adoption of log-based alerting on this specific warning is unlikely.

…t branch

The non-standard-path warning was applied only on the v2.5 brand_manifest
URL-string branch. The inline BrandManifest object's url field can carry
the same path (v2.5 spec example points at https://cdn.example.com/brand-manifest.json),
and the same information loss applies — v3 sellers will fetch the canonical
{domain}/.well-known/brand.json path and likely 404.

Centralize the warning helper + dedup state in _url.py alongside
extract_brand_domain so get_products and _media_buy_helpers share one
implementation. Both adapters now call warn_brand_manifest_path_lossy
from both the string and the inline-object branches.

Fixes #687

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley
Copy link
Copy Markdown
Contributor Author

Expert review

code-reviewer: approved — no blockers. Confirmed:

Non-blocking nits filed for future work:

  • Pytest autouse fixture for _brand_manifest_path_warned reset would prevent future test pollution (currently every test monkeypatches; one missed reset would flake)
  • Optional cross-adapter dedup test would document the now-shared-set semantic across get_products and create_media_buy

CI: 15/15 green. Merging.

@bokelley bokelley merged commit 840e6b3 into main May 12, 2026
16 checks passed
@bokelley bokelley deleted the claude/issue-687-warn-inline-object-brand-manifest branch May 12, 2026 01:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extend brand_manifest non-standard-path warning to inline-object branch

1 participant