diff --git a/MIGRATION_v3_to_v4.md b/MIGRATION_v3_to_v4.md index 7b406e56c..850e1c9ef 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`, 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..8fba2b3ec 100644 --- a/src/adcp/migrate/v3_to_v4.py +++ b/src/adcp/migrate/v3_to_v4.py @@ -106,6 +106,18 @@ } +# Enum values removed or split between v3 and v4. Flagged (not rewritten) +# because the correct replacement depends on call-site semantics. +REMOVED_ENUM_VALUES: dict[str, tuple[str, str]] = { + "MediaBuyStatus.pending_activation": ( + "`pending_activation` split in v4: use `pending_start` if the buy hasn't reached " + "its scheduled start date, or `pending_creatives` if creatives haven't been " + "submitted. Check `valid_actions` on the MediaBuy response to confirm which applies.", + "mediabuystatuspending_activation--split", + ), +} + + # Private-module imports that shouldn't appear in downstream code. PRIVATE_IMPORT_PATHS: dict[str, str] = { "adcp.types.generated_poc": ( @@ -168,7 +180,9 @@ class Finding: """One migration finding — either an applied rename or a manual TODO.""" - kind: str # "rename" | "flag_removed" | "flag_private" | "flag_numbered" | "flag_attribute" + # Valid kind values: "rename" | "flag_removed" | "flag_private" | + # "flag_numbered" | "flag_attribute" | "flag_enum_value" + kind: str path: str line: int column: int @@ -253,6 +267,12 @@ def _iter_python_files(root: Path) -> list[Path]: attr: re.compile(rf"{re.escape(attr)}\b") for attr in REMOVED_ATTRIBUTE_ACCESSES } +# Enum value patterns — re.escape handles the dot so the pattern matches +# the literal ``MediaBuyStatus.pending_activation``, not a regex wildcard. +_REMOVED_ENUM_VALUE_PATTERNS = { + val: re.compile(rf"{re.escape(val)}\b") for val in REMOVED_ENUM_VALUES +} + def scan_file(path: Path, *, apply_changes: bool) -> tuple[list[Finding], str | None]: """Scan one file. Returns (findings, new_contents_or_None). @@ -399,6 +419,24 @@ def scan_file(path: Path, *, apply_changes: bool) -> tuple[list[Finding], str | ) ) + # Removed enum values (e.g. MediaBuyStatus.pending_activation). The + # class-qualified form is anchored tightly enough that false positives + # are unlikely; trailing word boundary prevents suffix matches like + # ``MediaBuyStatus.pending_activation_v2``. + for enum_val, (enum_hint, enum_anchor) in REMOVED_ENUM_VALUES.items(): + for match in _REMOVED_ENUM_VALUE_PATTERNS[enum_val].finditer(line): + findings.append( + Finding( + kind="flag_enum_value", + path=str(path), + line=lineno, + column=match.start() + 1, + before=enum_val, + hint=enum_hint, + migration_anchor=enum_anchor, + ) + ) + if apply_changes and rename_hits: for old, new in ASSET_CONTENT_RENAMES.items(): updated = _RENAME_PATTERNS[old].sub(new, updated) @@ -513,7 +551,8 @@ def _format_text_report(report: Report, *, apply_changes: bool) -> str: "before": str, "after": str, "hint": null, "migration_anchor": null} ], "flagged": [ - {"kind": "flag_removed" | "flag_numbered" | "flag_private" | "flag_attribute", + {"kind": "flag_removed" | "flag_numbered" | "flag_private" + | "flag_attribute" | "flag_enum_value", "path": str, "line": int, "column": int, "before": str, "after": null, "hint": str | null, "migration_anchor": str | null} ] diff --git a/tests/test_migrate_v3_to_v4.py b/tests/test_migrate_v3_to_v4.py index 941973ef8..609f87d1c 100644 --- a/tests/test_migrate_v3_to_v4.py +++ b/tests/test_migrate_v3_to_v4.py @@ -271,6 +271,47 @@ def test_flags_removed_attribute_accesses(tmp_path: Path) -> None: assert attr[0].before == ".brand_manifest" +def test_flags_removed_enum_values(tmp_path: Path) -> None: + """`MediaBuyStatus.pending_activation` references are flagged with + a hint describing both replacement values and a runtime check.""" + _write( + tmp_path, + "code.py", + "if status == MediaBuyStatus.pending_activation:\n" + " handle_pending()\n" + "# also in a comparison\n" + "is_pending = mb.status is MediaBuyStatus.pending_activation\n", + ) + + report = v3_to_v4.run(tmp_path, apply_changes=False) + + enum_flags = [f for f in report.flagged if f.kind == "flag_enum_value"] + assert len(enum_flags) == 2 + for finding in enum_flags: + assert finding.before == "MediaBuyStatus.pending_activation" + assert finding.hint is not None + assert "pending_start" in finding.hint + assert "pending_creatives" in finding.hint + assert "valid_actions" in finding.hint + + +def test_enum_value_word_boundary_no_false_positive(tmp_path: Path) -> None: + """`MediaBuyStatus.pending_activation_v2` must NOT be flagged — + the trailing `_v2` is a word character so the word boundary fires + before the suffix, not after.""" + _write( + tmp_path, + "code.py", + "x = MediaBuyStatus.pending_activation_v2\n" + "y = MediaBuyStatus.pending_activation_custom()\n", + ) + + report = v3_to_v4.run(tmp_path, apply_changes=False) + + enum_flags = [f for f in report.flagged if f.kind == "flag_enum_value"] + assert enum_flags == [], f"false-positive on pending_activation_* suffixes: {enum_flags}" + + 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