diff --git a/MIGRATION_v3_to_v4.md b/MIGRATION_v3_to_v4.md index 7b406e56c..42b5c9cc7 100644 --- a/MIGRATION_v3_to_v4.md +++ b/MIGRATION_v3_to_v4.md @@ -211,6 +211,40 @@ if package.status == PackageStatus.active: ... if media_buy.status == MediaBuyStatus.active: ... ``` +### `MediaBuyStatus.pending_activation` → split + +The single `pending_activation` enum value was split into two +distinct states based on cause. The codemod flags every reference; the +correct replacement is per-call-site. + +| Cause | Replacement | +| --- | --- | +| Buy is scheduled and waiting for its start time | `MediaBuyStatus.pending_start` | +| Buy is waiting on creative approval / asset processing | `MediaBuyStatus.pending_creatives` | + +**Before (v3.x):** +```python +if media_buy.status == MediaBuyStatus.pending_activation: + notify_trafficker(media_buy) +``` + +**After (v4.0):** +```python +if media_buy.status in ( + MediaBuyStatus.pending_start, + MediaBuyStatus.pending_creatives, +): + notify_trafficker(media_buy) +``` + +When the original branch only fired for one cause, narrow to the right +state (e.g. only `pending_creatives` for creative-review notifications). +A blanket replacement to either single value is almost always wrong — +the spec split was driven by adopters needing distinct behaviour for +the two cases. The wire enum still accepts `pending` as a legacy alias +for `pending_start` (per the schema description), so existing buyer +clients reading older payloads keep working without code changes. + ### `ResolvedBrand.brand_manifest` field removed `RegistryClient.lookup_brand()` returns a `ResolvedBrand` whose diff --git a/src/adcp/migrate/v3_to_v4.py b/src/adcp/migrate/v3_to_v4.py index 0607f141c..c0eeb70c6 100644 --- a/src/adcp/migrate/v3_to_v4.py +++ b/src/adcp/migrate/v3_to_v4.py @@ -101,8 +101,23 @@ # Attribute accesses that moved / were removed. Flagged not rewritten # because context determines the right replacement. -REMOVED_ATTRIBUTE_ACCESSES: dict[str, str] = { - ".brand_manifest": ("ResolvedBrand.brand_manifest removed — use .brand instead"), +# +# Each entry is keyed on the dotted attribute including the leading dot +# (the regex below requires it for word-boundary safety) and maps to a +# ``(hint, migration_anchor)`` tuple. ``migration_anchor`` may be +# ``None`` for entries without a dedicated migration-guide section. +REMOVED_ATTRIBUTE_ACCESSES: dict[str, tuple[str, str | None]] = { + ".brand_manifest": ( + "ResolvedBrand.brand_manifest removed — use .brand instead", + None, + ), + ".pending_activation": ( + "MediaBuyStatus.pending_activation was split into " + "`pending_start` (schedule-future buys) and `pending_creatives` " + "(buys waiting on creative approval). The correct replacement " + "is per-call-site — review each usage manually.", + "mediabuystatuspending_activation--split", + ), } @@ -386,7 +401,7 @@ def scan_file(path: Path, *, apply_changes: bool) -> tuple[list[Finding], str | # Removed attribute accesses (.brand_manifest etc.). Regex with # trailing word boundary prevents false-positives on # ``.brand_manifest_v2``, ``.brand_manifest_override``, etc. - for attr, hint in REMOVED_ATTRIBUTE_ACCESSES.items(): + for attr, (attr_hint, attr_anchor) in REMOVED_ATTRIBUTE_ACCESSES.items(): for match in _REMOVED_ATTRIBUTE_PATTERNS[attr].finditer(line): findings.append( Finding( @@ -395,7 +410,8 @@ def scan_file(path: Path, *, apply_changes: bool) -> tuple[list[Finding], str | line=lineno, column=match.start() + 1, before=attr, - hint=hint, + hint=attr_hint, + migration_anchor=attr_anchor, ) ) diff --git a/tests/test_migrate_v3_to_v4.py b/tests/test_migrate_v3_to_v4.py index 941973ef8..1d90863af 100644 --- a/tests/test_migrate_v3_to_v4.py +++ b/tests/test_migrate_v3_to_v4.py @@ -271,6 +271,49 @@ def test_flags_removed_attribute_accesses(tmp_path: Path) -> None: assert attr[0].before == ".brand_manifest" +def test_flags_pending_activation_split(tmp_path: Path) -> None: + """``MediaBuyStatus.pending_activation`` was split into + ``pending_start`` and ``pending_creatives``. Flag every reference + with the per-call-site decision hint and the migration anchor.""" + _write( + tmp_path, + "code.py", + "if media_buy.status == MediaBuyStatus.pending_activation:\n" " notify(media_buy)\n", + ) + + report = v3_to_v4.run(tmp_path, apply_changes=False) + + attr = [ + f + for f in report.flagged + if f.kind == "flag_attribute" and f.before == ".pending_activation" + ] + assert len(attr) == 1 + assert "pending_start" in (attr[0].hint or "") + assert "pending_creatives" in (attr[0].hint or "") + assert attr[0].migration_anchor == "mediabuystatuspending_activation--split" + + +def test_pending_activation_word_boundary_no_false_positive(tmp_path: Path) -> None: + """``.pending_activation_v2`` / ``.pending_activation_count`` are + adopter extensions that share the prefix; the trailing word + boundary keeps them out of the flagged set.""" + _write( + tmp_path, + "code.py", + "x = stats.pending_activation_count\n" "y = obj.pending_activation_v2 = True\n", + ) + + report = v3_to_v4.run(tmp_path, apply_changes=False) + + flagged = [ + f + for f in report.flagged + if f.kind == "flag_attribute" and f.before == ".pending_activation" + ] + assert flagged == [], f"false-positive on pending_activation_* suffixes: {flagged}" + + def test_brand_manifest_word_boundary_no_false_positive(tmp_path: Path) -> None: """``.brand_manifest_v2`` / ``.brand_manifest_override`` are seller-specific extensions that happen to share a prefix. They