Skip to content

feat(types): disambiguated aliases for colliding type names (#911 Step 2)#941

Merged
bokelley merged 1 commit into
mainfrom
claude/issue-911-step2-aliases
Jun 7, 2026
Merged

feat(types): disambiguated aliases for colliding type names (#911 Step 2)#941
bokelley merged 1 commit into
mainfrom
claude/issue-911-step2-aliases

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented Jun 7, 2026

Summary

Advances #911 (Step 2). Step 1 (#919, merged) made cross-module generated-type name collisions loud via a build guard; this PR provides disambiguated, adopter-facing aliases for the high-traffic colliding names #911 calls out, so adopters can import each per-module variant unambiguously from adcp.types instead of getting whichever module wins the consolidate sort order.

This does not implement the full set of ~210 collisions — it covers the high-traffic, adopter-facing names named in the issue, so it advances Step 2 rather than closing #911.

Naming convention (the API decision)

<Context><BaseName>, where <Context> is derived from the defining module/verb. This matches existing precedent in aliases.py (SyncCreativeResult, MediaBuyDeliveryStatus, SyncAudiencesAudience).

33 aliases added, each pointing at the correct per-module class:

Base Aliases
Creative DeliveryCreative, ListCreativesCreative, SyncCreativesCreative, BuildCreativeCreative, CapabilitiesCreative
Account CoreAccount, SyncAccountsAccount, SyncGovernanceAccount, CapabilitiesAccount
Authentication PushNotificationAuthentication, NotificationAuthentication, ReportingWebhookAuthentication, GovernanceAuthentication, CreateMediaBuyAuthentication
MediaBuy CoreMediaBuy, GetMediaBuysMediaBuy, CapabilitiesMediaBuy
GovernanceAgent CoreGovernanceAgent, SyncGovernanceGovernanceAgent
CreditLimit CoreCreditLimit, SyncAccountsCreditLimit
Setup CoreSetup, SyncAccountsSetup, SyncEventSourcesSetup
Sort ListCreativesSort, TasksListSort, ListTasksSort
Signal GetSignalsSignal, WholesaleFeedSignal
Unit DurationUnit, OverlayUnit, RealEstateUnit, VehicleUnit

Each alias imports its variant directly from the source module, so it resolves to the correct class regardless of consolidate sort order — verified by is-identity / __module__ checks in the tests.

Notes / scope decisions

  • Brand (in the issue's table) is no longer a collision on current main — only one module defines it, so it already imports unambiguously and is not in the allowlist. Skipped.
  • Unit: the issue table referenced enums.dimension_unit, but that class is actually named DimensionUnit (already unambiguous). The real Unit collision is across core.duration / overlay / real_estate_item / vehicle_item; aliased those by the dimension they measure.
  • Account: the compliance.comply_test_controller_request variant is a trivial {sandbox} test stub; intentionally not aliased.
  • Concrete shape differences that make the silent swap unsafe are asserted in tests (e.g. notification_config.Authentication makes credentials optional while push_notification_config requires it; list_creatives_response.Creative is the rich record vs the lean delivery-totals winner).

Interaction with Step 1

Aliasing in aliases.py does not remove the underlying generated_poc collisions, so per the Step 1 design the collision_allowlist.json + guard stay unchanged. The guard and tests/test_collision_guard.py remain green. No generated_poc/, _generated.py, consolidate_exports.py, or KNOWN_COLLISIONS edits.

Tests / verification

  • New tests/test_collision_aliases.py: each alias resolves to the class defined in its named module (is/__module__), is importable from adcp.types + in __all__, alias set is internally consistent, distinct variants are distinct classes, plus shape-marker assertions.
  • tests/fixtures/public_api_snapshot.json regenerated (purely additive, only the adcp.types section).
  • Green locally: test_collision_aliases, test_collision_guard, test_import_layering, test_public_api, test_type_aliases; make lint, make typecheck, make typecheck-all, make validate-generated; full make test (5727 passed).

No ADCP_VERSION bump, no schema re-download.

🤖 Generated with Claude Code

Implements #911 (Step 2). Step 1 (#919) made cross-module generated-type name
collisions loud via a build guard; this adds disambiguated, adopter-facing
aliases for the high-traffic colliding names #911 calls out so adopters can
import each per-module variant unambiguously from adcp.types.

Convention: <Context><BaseName>, where <Context> is derived from the defining
module/verb (e.g. SyncAccountsAccount, DeliveryCreative, CapabilitiesMediaBuy),
matching existing precedent in aliases.py (SyncCreativeResult,
MediaBuyDeliveryStatus, SyncAudiencesAudience).

Covers 33 aliases across Creative, Account, Authentication, MediaBuy,
GovernanceAgent, CreditLimit, Setup, Sort, Signal, and Unit. Each alias imports
its variant directly from the source module, so it resolves to the correct
class regardless of consolidate sort order.

These aliases do not remove the underlying generated_poc collisions, so the
Step 1 guard and scripts/collision_allowlist.json are intentionally unchanged.
No generated_poc or _generated.py edits.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@bokelley bokelley marked this pull request as ready for review June 7, 2026 13:00
@bokelley bokelley enabled auto-merge (squash) June 7, 2026 13:00
Copy link
Copy Markdown
Contributor

@aao-ipr-bot aao-ipr-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean additive change. Right shape: aliases import each variant directly from its source module, so the binding survives consolidate sort-order shifts and the renumber churn that _generated.py is prone to.

Things I checked

  • Purely additive. None of the 33 names exist at HEAD — the diff only inserts into import blocks, both __all__ blocks, and the snapshot. No public export removed, renamed, or repointed. feat(types): is the correct semver signal; no ! warranted.
  • Alias mapping is faithful. All 24 generated_poc source modules exist and every aliased base class (Account, Creative, Authentication, MediaBuy, Setup, CreditLimit, GovernanceAgent, Sort, Signal, Unit) is defined in the module the alias names. The shape claims that justify the disambiguation are true at source: core/notification_config.py makes credentials optional while core/push_notification_config requires it; get_creative_delivery_response.Creative is the lean totals view (totals/variant_count) vs list_creatives_response.Creative's rich record (status/assets/assignments). ad-tech-protocol-expert: sound.
  • No shadowing. The four *Unit aliases are distinct enums and don't touch the pre-existing Unit = DimensionUnit binding at __init__.py:838 — correctly called out in the aliases.py comment. code-reviewer: clean.
  • Import layering intact. Only aliases.py and __init__.py gained generated_poc imports — both on the test_import_layering.py allowlist. No other module touched. No generated_poc/, _generated.py, or consolidate_exports.py edits, so the Step 1 guard and collision_allowlist.json stay green as designed.
  • Stability contract. tests/test_collision_aliases.py pins each alias to its __module__ by is-identity, so a future regen that repointed a binding fails loudly instead of silently swapping the wire shape. Aliases target named classes at stable module paths, not numbered anonymous variants — the renumber footgun doesn't apply here.

Follow-ups (non-blocking — file as issues)

  • Scope is 33 of ~210 collisions by design — this advances #911 Step 2, doesn't close it. The skipped cases (Brand no longer colliding, DimensionUnit already unambiguous, the {sandbox} compliance Account stub) are documented and reasonable.

Minor nits (non-blocking)

  1. Pre-existing snapshot duplicates. tests/fixtures/public_api_snapshot.json carries duplicate WholesaleFeedEvent / WholesaleFeedWebhook entries adjacent to the new WholesaleFeedSignal insertion. They're on HEAD, stem from _generated.py, and aren't introduced here — worth a separate cleanup, not this PR's problem.

LGTM. Follow-ups noted below.

@bokelley bokelley merged commit 6c058f0 into main Jun 7, 2026
27 checks passed
@bokelley bokelley deleted the claude/issue-911-step2-aliases branch June 7, 2026 13:05
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.

1 participant