Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions MIGRATION_v3_to_v4.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 41 additions & 2 deletions src/adcp/migrate/v3_to_v4.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": (
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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}
]
Expand Down
41 changes: 41 additions & 0 deletions tests/test_migrate_v3_to_v4.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading