fix(migrate): per-symbol replacement for generated_poc reach-ins (#329)

Round-5 adopter feedback (salesagent v3→v4 experiment, 270 files /
161 collection failures): 82 of 156 codemod findings were
``adcp.types.generated_poc`` flag-only output that forced adopters
to hand-grep the public-API module to find each replacement.

Fixes:

- ``GENERATED_POC_SYMBOL_MAP`` covers the 8 unambiguous reach-ins
  with explicit "Symbol → adcp.types.Symbol" replacements:
  AccountReference, BrandReference, ContextObject, CreativeAsset,
  Error, MediaBuyStatus, ProductFilters, ReportingWebhook. The
  ``test_generated_poc_symbol_map_covers_publicly_exported_names``
  test guards drift between the codemod and the SDK's public surface.
- Multi-symbol single-line imports (``from … import A, B, C``) emit
  one Finding per known symbol; unknown symbols and multiline imports
  fall back to the generic "private module" flag.
- Text report shows per-symbol replacement on the header line so
  adopters fix without leaving the codemod output.
- ``import X as Y`` aliases are handled — the codemod keys off the
  canonical LHS name.

Intentionally NOT mapped: ``CreditLimit``, ``Setup``,
``GovernanceAgent``. These names appear in 8+ generated files
(per-schema variants like ``core/account.Setup`` vs.
``account/sync_accounts_response.Setup``); a blanket
``adcp.types.Setup`` hint would be ambiguous. Adopters reaching for
these get the generic flag; landing them in the public API is a
separate design decision.

Migration guide additions:

- "Fix removed-type imports first — they cascade" section documents
  salesagent's pattern of centralizing SDK imports in a barrel
  module (``_base.py`` re-export) where one broken import crashes
  ~140 tests during pytest collect-only. Calls out the priority
  order: barrel modules first, then generated_poc reach-ins, then
  numbered Assets, then per-call-site shape changes.
- "a2a-sdk transitive bump" section flags the ``a2a-sdk>=1.0.0``
  surprise migration for adopters with direct ``a2a.utils.errors`` /
  ``a2a.types`` imports.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: bokelley

MIGRATION_v3_to_v4.md

@@ -46,6 +46,58 @@ dependencies = [
 ]
 ```
 
+## Fix removed-type imports first — they cascade
+
+Real-adopter feedback (salesagent v3→v4 experiment, 270 files scanned, 161
+test-collection failures): consumers tend to centralize SDK imports in one
+schema module, so a single broken import there crashes test collection across
+the whole codebase. salesagent re-exported through `src/core/schemas/_base.py`
+— **one missing `FormatCategory` import there cascaded into ~140 test failures
+during pytest collect-only**, and stubbing it revealed the next ~140-test
+cascade from `BrandManifest`, then the next from the `generated_poc` reach-ins.
+
+The codemod's static finding count understates this by 100x+. To minimize the
+felt blast radius, work in this order:
+
+1. **Removed types in central re-export modules first.** Find every
+   `BrandManifest`, `FormatCategory`, `DeliverTo`, `PromotedProducts`,
+   `PromotedOfferings`, `Pricing`, `PackageStatus` import in modules that
+   re-export to the rest of your codebase (e.g. a `_base.py` schema barrel).
+   Fix those before anything else — most of your test-collection failures
+   disappear.
+2. **`adcp.types.generated_poc` reach-ins.** The codemod's per-symbol mapping
+   tells you the public alias for each (`ContextObject` →
+   `adcp.types.ContextObject`, etc.). Mechanical lookup once you know the
+   pattern.
+3. **Numbered `Assets<N>` imports.** Switch to the semantic alias from
+   `adcp.types`. See [Numbered discriminated-union classes
+   shifted](#numbered-discriminated-union-classes-shifted) below.
+4. **Per-call-site shape changes** (e.g. `BrandManifest(name=..., logo_url=...)`
+   → `BrandReference(domain=...)`). The codemod can't auto-rewrite these —
+   the data is different.
+
+## a2a-sdk transitive bump (only matters if you have direct `a2a` imports)
+
+v4 of this SDK requires `a2a-sdk>=1.0.0`. If your codebase has any direct
+imports from the `a2a` package — typically `a2a.utils.errors.ServerError`,
+`a2a.types.DataPart`, or other surface — those are a separate migration that
+this SDK forces on you transitively.
+
+Symptoms during pytest collection after upgrading:
+
+- `cannot import name 'ServerError' from 'a2a.utils.errors'`
+- `cannot import name 'DataPart' from 'a2a.types'`
+
+If you don't import from `a2a` directly (only via `adcp.server` /
+`adcp.protocols.a2a`), this section doesn't apply — the SDK's wrapper layer
+absorbs the change.
+
+If you do, see the [a2a-sdk
+1.0 release notes](https://github.com/a2aproject/a2a-python/releases) for the
+import-path moves. The most common pattern is `a2a.utils.errors.ServerError`
+→ `a2a.types.A2AError` and `a2a.types.DataPart` → `a2a.types.MessagePart`,
+but verify against your specific usage.
+
 ## Removed types
 
 The following types were removed from the AdCP spec and have no replacement

src/adcp/migrate/v3_to_v4.py

@@ -114,6 +114,50 @@
 }
 
 
+# Per-symbol mapping for the most common ``generated_poc`` reach-ins
+# salesagent surfaced during their v3→v4 experiment (and any other
+# adopter would hit). The codemod scans for ``from
+# adcp.types.generated_poc.<path> import <Symbol>`` lines and emits an
+# explicit "before → after" hint per symbol so adopters don't have to
+# hand-grep the public-API module to find the canonical alias.
+#
+# Mapping shape: ``<symbol-name> → adcp.types.<symbol-name>``. Every
+# symbol listed here is already exported from ``adcp.types``; the
+# ``test_generated_poc_symbol_map_covers_publicly_exported_names`` test
+# guards drift between this map and the SDK's public surface.
+#
+# Intentionally NOT in the map (yet): ``CreditLimit``, ``Setup``,
+# ``GovernanceAgent``. These names appear in 8+ generated files
+# (``core/account.py``, ``account/sync_accounts_response.py``,
+# ``media_buy/sync_event_sources_response.py``, ``bundled/...``) — the
+# codegen emits one independent class per containing schema, so a
+# blanket "import from adcp.types" hint would be ambiguous about
+# which variant. Adopters reaching for these get the generic
+# private-module flag; landing them in the public API is a separate
+# design decision (which canonical variant to expose / whether to
+# expose schema-namespaced aliases like ``AccountSetup``).
+GENERATED_POC_SYMBOL_MAP: dict[str, str] = {
+    "AccountReference": "adcp.types.AccountReference",
+    "BrandReference": "adcp.types.BrandReference",
+    "ContextObject": "adcp.types.ContextObject",
+    "CreativeAsset": "adcp.types.CreativeAsset",
+    "Error": "adcp.types.Error",
+    "MediaBuyStatus": "adcp.types.MediaBuyStatus",
+    "ProductFilters": "adcp.types.ProductFilters",
+    "ReportingWebhook": "adcp.types.ReportingWebhook",
+}
+
+
+# ``from adcp.types.generated_poc.<...> import <Symbol[, ...]>`` —
+# captures the symbol list so we can emit per-symbol replacement hints.
+# Multiline imports (parenthesized) aren't covered by this regex; they
+# fall through to the generic "private module" flag, which still
+# surfaces the issue and prints the migration anchor.
+_GENERATED_POC_FROM_IMPORT = re.compile(
+    r"from\s+adcp\.types\.generated_poc(?:\.[\w.]+)?\s+import\s+([\w\s,]+)"
+)
+
+
 # Regex for numbered Assets direct imports (``Assets5``, ``Assets14``, etc).
 # Bare ``Assets`` (no digits) is a legitimate base class alias; the
 # regex requires at least one digit to avoid false positives.
@@ -283,10 +327,51 @@ def scan_file(path: Path, *, apply_changes: bool) -> tuple[list[Finding], str |
                 )
             )
 
-        # adcp.types.generated_poc imports.
+        # adcp.types.generated_poc imports. When the line is a
+        # single-line ``from adcp.types.generated_poc.<path> import
+        # <symbols>`` and any of the imported symbols are in
+        # GENERATED_POC_SYMBOL_MAP, emit one per-symbol Finding with the
+        # public-API replacement (e.g. "ContextObject → adcp.types.ContextObject").
+        # Otherwise fall back to the generic "private module" flag so
+        # multiline / star imports still surface.
         for private_path, hint in PRIVATE_IMPORT_PATHS.items():
-            if private_path in line:
-                col = line.index(private_path) + 1
+            if private_path not in line:
+                continue
+            col = line.index(private_path) + 1
+            from_match = _GENERATED_POC_FROM_IMPORT.search(line)
+            mapped_any = False
+            if from_match:
+                # Symbols list — handles ``A``, ``A, B``, ``A as X``.
+                # ``as`` aliases are rare in practice for these reach-ins
+                # but treat the LHS as the canonical symbol when present.
+                raw_symbols = [s.strip() for s in from_match.group(1).split(",")]
+                for raw in raw_symbols:
+                    if not raw:
+                        continue
+                    symbol = raw.split(" as ")[0].strip()
+                    replacement = GENERATED_POC_SYMBOL_MAP.get(symbol)
+                    if replacement is None:
+                        continue
+                    sym_col = line.find(symbol, from_match.start(1)) + 1
+                    findings.append(
+                        Finding(
+                            kind="flag_private",
+                            path=str(path),
+                            line=lineno,
+                            column=sym_col,
+                            before=symbol,
+                            after=replacement,
+                            hint=(
+                                f"private module — import {symbol} from "
+                                "adcp.types (stable public API) instead"
+                            ),
+                        )
+                    )
+                    mapped_any = True
+            if not mapped_any:
+                # Generic flag — multiline imports, star imports, or
+                # symbols without a known public alias. Adopter does the
+                # lookup; codemod still surfaces the issue.
                 findings.append(
                     Finding(
                         kind="flag_private",
@@ -373,7 +458,17 @@ def _format_text_report(report: Report, *, apply_changes: bool) -> str:
         for f in report.flagged:
             by_name.setdefault(f.before, []).append(f)
         for name, hits in sorted(by_name.items()):
-            lines.append(f"  {name}  ({len(hits)} hit{'s' if len(hits) != 1 else ''})")
+            # Per-symbol mapping ("ContextObject → adcp.types.ContextObject")
+            # — print the explicit replacement on the header line so
+            # adopters fix without leaving the report. Falls back to
+            # bare name when no replacement is mapped.
+            replacement = hits[0].after
+            header = (
+                f"  {name} → {replacement}  ({len(hits)} hit{'s' if len(hits) != 1 else ''})"
+                if replacement
+                else f"  {name}  ({len(hits)} hit{'s' if len(hits) != 1 else ''})"
+            )
+            lines.append(header)
             hint = hits[0].hint
             if hint:
                 lines.append(f"    → {hint}")

tests/test_migrate_v3_to_v4.py

@@ -160,13 +160,16 @@ def test_bare_assets_is_not_flagged_as_numbered(tmp_path: Path) -> None:
     assert numbered == []
 
 
-def test_flags_generated_poc_imports(tmp_path: Path) -> None:
-    """``adcp.types.generated_poc`` is a private module — flag imports
-    from it and point at the public alias path."""
+def test_flags_generated_poc_imports_unknown_symbol_falls_back_to_generic_hint(
+    tmp_path: Path,
+) -> None:
+    """A ``generated_poc`` import for a symbol not in the per-symbol map
+    falls back to the generic 'private module' flag — still surfaces the
+    issue, adopter does the lookup."""
     _write(
         tmp_path,
         "code.py",
-        "from adcp.types.generated_poc.core.account import Account\n",
+        "from adcp.types.generated_poc.core.something import Unknown\n",
     )
 
     report = v3_to_v4.run(tmp_path, apply_changes=False)
@@ -176,6 +179,82 @@ def test_flags_generated_poc_imports(tmp_path: Path) -> None:
     assert private[0].before == "adcp.types.generated_poc"
 
 
+def test_flags_generated_poc_imports_per_symbol_mapping(tmp_path: Path) -> None:
+    """Round-5 adopter feedback (salesagent v3→v4 experiment): the
+    ``generated_poc`` flag-only output forced 82 of 156 findings into
+    hand-grep territory. Each known reach-in now emits an explicit
+    "Symbol → adcp.types.Symbol" replacement so adopters apply the fix
+    without leaving the codemod report."""
+    _write(
+        tmp_path,
+        "code.py",
+        "from adcp.types.generated_poc.core.context import ContextObject\n"
+        "from adcp.types.generated_poc.core.brand_ref import BrandReference\n"
+        "from adcp.types.generated_poc.enums.media_buy_status import MediaBuyStatus\n"
+        "from adcp.types.generated_poc.core.error import Error as AdCPResponseError\n",
+    )
+
+    report = v3_to_v4.run(tmp_path, apply_changes=False)
+
+    private = [f for f in report.flagged if f.kind == "flag_private"]
+    by_symbol = {f.before: f for f in private}
+
+    # Each known symbol gets a per-symbol replacement, NOT the generic
+    # "adcp.types.generated_poc" flag.
+    assert by_symbol["ContextObject"].after == "adcp.types.ContextObject"
+    assert by_symbol["BrandReference"].after == "adcp.types.BrandReference"
+    assert by_symbol["MediaBuyStatus"].after == "adcp.types.MediaBuyStatus"
+    # ``import Error as AdCPResponseError`` — codemod keys off the LHS
+    # canonical name, ignoring the local alias.
+    assert by_symbol["Error"].after == "adcp.types.Error"
+    # The generic private-module flag MUST NOT also fire when the
+    # per-symbol mapping handled the line — would double-count and
+    # confuse the report.
+    assert "adcp.types.generated_poc" not in by_symbol
+
+
+def test_flags_generated_poc_multiple_symbols_one_line(tmp_path: Path) -> None:
+    """``from adcp.types.generated_poc.core.x import A, B, C`` emits
+    one Finding per symbol so the report surfaces every replacement."""
+    _write(
+        tmp_path,
+        "code.py",
+        "from adcp.types.generated_poc.core.x import "
+        "BrandReference, ContextObject, MediaBuyStatus\n",
+    )
+
+    report = v3_to_v4.run(tmp_path, apply_changes=False)
+
+    private = [f for f in report.flagged if f.kind == "flag_private"]
+    by_symbol = {f.before: f.after for f in private}
+    assert by_symbol == {
+        "BrandReference": "adcp.types.BrandReference",
+        "ContextObject": "adcp.types.ContextObject",
+        "MediaBuyStatus": "adcp.types.MediaBuyStatus",
+    }
+
+
+def test_generated_poc_symbol_map_covers_publicly_exported_names() -> None:
+    """Every entry in ``GENERATED_POC_SYMBOL_MAP`` MUST point at a real
+    public-API symbol on ``adcp.types`` — otherwise the hint sends
+    adopters to a NameError. Guards against drift between the codemod's
+    map and the SDK's __all__."""
+    import importlib
+
+    types_module = importlib.import_module("adcp.types")
+    for symbol, replacement in v3_to_v4.GENERATED_POC_SYMBOL_MAP.items():
+        # Every replacement is exactly ``adcp.types.<symbol>``.
+        assert replacement == f"adcp.types.{symbol}", (
+            f"GENERATED_POC_SYMBOL_MAP[{symbol!r}] = {replacement!r} but "
+            f"the convention is adcp.types.{symbol}"
+        )
+        assert hasattr(types_module, symbol), (
+            f"GENERATED_POC_SYMBOL_MAP claims adcp.types.{symbol} exists "
+            "but it's not on the public types module — drop the entry "
+            "or add the public alias."
+        )
+
+
 def test_flags_removed_attribute_accesses(tmp_path: Path) -> None:
     """``.brand_manifest`` on ResolvedBrand was removed — flag the
     attribute access with a hint."""