fix(migrate): don't corrupt mixed numbered+unknown imports under --auto-apply

Per review on #540 — a generated_poc import mixing a known numbered
asset (Assets81) with an unknown one (Assets149) was getting
half-rewritten:

  Input : from adcp.types.generated_poc.core.format import Assets81, Assets149
  Output: from adcp.types.generated_poc.core.format import VideoFormatAsset, Assets149

leaving VideoFormatAsset imported from a private module path that
doesn't export it — guaranteed ImportError in adopter source.

Root cause: step 1 (file-wide Assets<N> substitution) ran before step 2
(import-path safety check) had a chance to veto the line.

Fix: line-by-line rewrite. For each generated_poc single-line import,
compute the post-numbered-substitution symbol set up-front; only run
both substitutions when every symbol on the line is in
``_AUTO_APPLY_PUBLIC_SYMBOLS``. Mixed-unsafe lines are left entirely
alone. Non-import lines still get free numbered substitution.

Scan-time also fixed: numbered references on a mixed-unsafe import line
now report ``flag_numbered`` instead of ``auto_applied`` so the report
matches the file content.

Adds regression test
``test_auto_apply_mixed_numbered_known_unknown_does_not_corrupt_import``.

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

Author: bokelley

src/adcp/migrate/v3_to_v4.py

@@ -331,9 +331,7 @@ def _iter_python_files(root: Path) -> list[Path]:
 # Regex used in the ``--auto-apply`` import-path fix pass: matches the
 # ``from adcp.types.generated_poc...`` prefix so it can be replaced with
 # ``from adcp.types``.
-_GENERATED_POC_MODULE_RE = re.compile(
-    r"from\s+adcp\.types\.generated_poc(?:\.[\w.]+)?\s+import"
-)
+_GENERATED_POC_MODULE_RE = re.compile(r"from\s+adcp\.types\.generated_poc(?:\.[\w.]+)?\s+import")
 
 # Union of symbol names that ``--auto-apply`` can safely reroute to
 # ``adcp.types``: the explicit flag_private symbol map plus every public
@@ -378,6 +376,28 @@ def scan_file(
     auto_apply_hits = False  # any numbered or private-import rewrites queued
 
     for lineno, line in enumerate(original.splitlines(), start=1):
+        # Pre-pass: when this line is a single-line ``generated_poc``
+        # import, decide whether the line as a whole is auto-apply-safe.
+        # An import is *unsafe* when at least one of its symbols (after
+        # the hypothetical numbered substitution) isn't in
+        # ``_AUTO_APPLY_PUBLIC_SYMBOLS``; rewriting one symbol while
+        # leaving another behind would leave the line importing a
+        # public name from a private module — guaranteed ImportError.
+        # The rewrite block (`updated.splitlines()` later) skips
+        # unsafe-mixed lines; the per-symbol Finding emission below
+        # also treats numbered references on those lines as
+        # ``flag_numbered`` rather than ``auto_applied`` so the report
+        # matches the file content.
+        line_is_mixed_unsafe_import = False
+        if "adcp.types.generated_poc" in line:
+            from_match = _GENERATED_POC_FROM_IMPORT.search(line)
+            if from_match:
+                raw_syms = [s.strip() for s in from_match.group(1).split(",")]
+                pre_syms = [r.split(" as ")[0].strip() for r in raw_syms if r.strip()]
+                post_syms = [NUMBERED_ASSETS_RENAMES.get(s, s) for s in pre_syms]
+                if pre_syms and not all(s in _AUTO_APPLY_PUBLIC_SYMBOLS for s in post_syms):
+                    line_is_mixed_unsafe_import = True
+
         for old, new in ASSET_CONTENT_RENAMES.items():
             for match in _RENAME_PATTERNS[old].finditer(line):
                 findings.append(
@@ -411,7 +431,7 @@ def scan_file(
         for match in NUMBERED_ASSETS_PATTERN.finditer(line):
             symbol = match.group(0)
             alias = NUMBERED_ASSETS_RENAMES.get(symbol)
-            if auto_apply and alias is not None:
+            if auto_apply and alias is not None and not line_is_mixed_unsafe_import:
                 findings.append(
                     Finding(
                         kind="auto_applied",
@@ -489,8 +509,7 @@ def scan_file(
                     continue
 
                 all_known = all(
-                    repl is not None
-                    or (auto_apply and symbol in NUMBERED_ASSETS_RENAMES)
+                    repl is not None or (auto_apply and symbol in NUMBERED_ASSETS_RENAMES)
                     for symbol, repl in parsed
                 )
 
@@ -588,38 +607,45 @@ def scan_file(
         needs_write = True
 
     if apply_changes and auto_apply and auto_apply_hits:
-        # Step 1: substitute Assets<N> → SemanticAlias everywhere
-        # (handles both usage sites and import symbols).
-        for old, new in NUMBERED_ASSETS_RENAMES.items():
-            updated = _NUMBERED_RENAME_PATTERNS[old].sub(new, updated)
-
-        # Step 2: fix any generated_poc import whose symbols are now all
-        # resolvable to adcp.types.  This covers two cases:
-        #
-        #   a. ``from generated_poc.core.format import Assets81`` became
-        #      ``from generated_poc.core.format import VideoFormatAsset``
-        #      after step 1 — rewrite the module path.
-        #
-        #   b. ``from generated_poc.core.x import ContextObject`` whose
-        #      all-known flag_private findings were promoted to auto_applied
-        #      — rewrite the module path here too.
-        #
-        # The check uses ``_AUTO_APPLY_PUBLIC_SYMBOLS`` (a frozen set of all
-        # known-safe names) so we never fix an import that still references
-        # an unknown symbol.
+        # Process the file line-by-line so generated_poc imports get a
+        # safety check against the post-numbered-substitution symbol set
+        # before any rewrite happens. The earlier "Step 1: substitute
+        # Assets<N> file-wide; Step 2: fix import paths only when safe"
+        # ordering corrupted mixed lines like
+        # ``from generated_poc.core.format import Assets81, Assets149``
+        # — Assets81 became VideoFormatAsset while Assets149 stayed,
+        # leaving VideoFormatAsset imported from a private module.
         new_lines: list[str] = []
         for text_line in updated.splitlines(keepends=True):
-            if "adcp.types.generated_poc" not in text_line:
+            is_generated_poc_import = (
+                "adcp.types.generated_poc" in text_line
+                and _GENERATED_POC_FROM_IMPORT.search(text_line) is not None
+            )
+            if is_generated_poc_import:
+                m = _GENERATED_POC_FROM_IMPORT.search(text_line)
+                assert m is not None  # narrowed above
+                raw_syms = [s.strip() for s in m.group(1).split(",")]
+                pre_syms = [r.split(" as ")[0].strip() for r in raw_syms if r.strip()]
+                # Apply the hypothetical numbered rename to each symbol
+                # so we can check if the *post-rename* symbol set is
+                # safe.
+                post_syms = [NUMBERED_ASSETS_RENAMES.get(s, s) for s in pre_syms]
+                if post_syms and all(s in _AUTO_APPLY_PUBLIC_SYMBOLS for s in post_syms):
+                    # Whole import is safe — substitute numbered names
+                    # AND fix the module path.
+                    for old, new in NUMBERED_ASSETS_RENAMES.items():
+                        text_line = _NUMBERED_RENAME_PATTERNS[old].sub(new, text_line)
+                    text_line = _GENERATED_POC_MODULE_RE.sub("from adcp.types import", text_line)
+                # Mixed line — leave it alone. The findings list still
+                # carries the per-symbol flag_private and flag_numbered
+                # entries so the adopter sees the work to do.
                 new_lines.append(text_line)
                 continue
-            m = _GENERATED_POC_FROM_IMPORT.search(text_line)
-            if m:
-                raw_syms = [s.strip() for s in m.group(1).split(",")]
-                syms = [r.split(" as ")[0].strip() for r in raw_syms if r.strip()]
-                if syms and all(sym in _AUTO_APPLY_PUBLIC_SYMBOLS for sym in syms):
-                    text_line = _GENERATED_POC_MODULE_RE.sub(
-                        "from adcp.types import", text_line
-                    )
+            # Non-import lines: substitute numbered names freely (the
+            # semantic alias is already importable via adcp.types and
+            # any local reference the line carries is a usage site).
+            for old, new in NUMBERED_ASSETS_RENAMES.items():
+                text_line = _NUMBERED_RENAME_PATTERNS[old].sub(new, text_line)
             new_lines.append(text_line)
         updated = "".join(new_lines)
         needs_write = True
@@ -634,9 +660,7 @@ def run(root: Path, *, apply_changes: bool = False, auto_apply: bool = False) ->
     report = Report()
     for path in _iter_python_files(root):
         report.scanned_files += 1
-        findings, new_contents = scan_file(
-            path, apply_changes=apply_changes, auto_apply=auto_apply
-        )
+        findings, new_contents = scan_file(path, apply_changes=apply_changes, auto_apply=auto_apply)
         for f in findings:
             report.add(f)
         if new_contents is not None:
@@ -728,9 +752,7 @@ def _format_text_report(report: Report, *, apply_changes: bool, auto_apply: bool
         lines.append(f"Rewrote {report.rewritten_files} files in place.")
         lines.append("Review with `git diff` before committing.")
 
-    if not auto_apply and any(
-        f.kind in ("flag_private", "flag_numbered") for f in report.flagged
-    ):
+    if not auto_apply and any(f.kind in ("flag_private", "flag_numbered") for f in report.flagged):
         lines.append("")
         lines.append(
             "Tip: rerun with --auto-apply to mechanically fix the "

tests/test_migrate_v3_to_v4.py

@@ -659,8 +659,7 @@ def test_auto_apply_rewrites_all_known_private_import(tmp_path: Path) -> None:
     path = _write(
         tmp_path,
         "code.py",
-        "from adcp.types.generated_poc.core.context import ContextObject\n"
-        "x = ContextObject()\n",
+        "from adcp.types.generated_poc.core.context import ContextObject\n" "x = ContextObject()\n",
     )
     report = v3_to_v4.run(tmp_path, apply_changes=True, auto_apply=True)
 
@@ -723,7 +722,8 @@ def test_auto_apply_mixed_line_not_rewritten(tmp_path: Path) -> None:
 
     # Unknown symbol gets a generic flag too (silent-drop bug is fixed).
     generic_flags = [
-        f for f in report.flagged
+        f
+        for f in report.flagged
         if f.kind == "flag_private" and f.before == "adcp.types.generated_poc"
     ]
     assert len(generic_flags) >= 1
@@ -759,16 +759,15 @@ def test_auto_apply_rewrites_numbered_asset_and_fixes_import_path(tmp_path: Path
     path = _write(
         tmp_path,
         "code.py",
-        "from adcp.types.generated_poc.core.format import Assets81\n"
-        "slot: Assets81\n",
+        "from adcp.types.generated_poc.core.format import Assets81\n" "slot: Assets81\n",
     )
     v3_to_v4.run(tmp_path, apply_changes=True, auto_apply=True)
 
     rewritten = path.read_text()
     assert "Assets81" not in rewritten
-    assert "adcp.types.generated_poc" not in rewritten, (
-        "generated_poc import path must be fixed after numbered rename"
-    )
+    assert (
+        "adcp.types.generated_poc" not in rewritten
+    ), "generated_poc import path must be fixed after numbered rename"
     assert "from adcp.types import VideoFormatAsset" in rewritten
     assert "slot: VideoFormatAsset" in rewritten
 
@@ -797,8 +796,7 @@ def test_auto_apply_numbered_plus_known_symbol_same_line(tmp_path: Path) -> None
     path = _write(
         tmp_path,
         "code.py",
-        "from adcp.types.generated_poc.core.x import Assets81, ContextObject\n"
-        "slot: Assets81\n",
+        "from adcp.types.generated_poc.core.x import Assets81, ContextObject\n" "slot: Assets81\n",
     )
     report = v3_to_v4.run(tmp_path, apply_changes=True, auto_apply=True)
 
@@ -814,6 +812,37 @@ def test_auto_apply_numbered_plus_known_symbol_same_line(tmp_path: Path) -> None
     assert not any(f.kind == "flag_private" for f in report.flagged)
 
 
+def test_auto_apply_mixed_numbered_known_unknown_does_not_corrupt_import(
+    tmp_path: Path,
+) -> None:
+    """Regression: a generated_poc import mixing a known numbered asset
+    (Assets81) with an unknown numbered asset (Assets149) MUST NOT
+    silently rewrite Assets81 alone — that would leave VideoFormatAsset
+    imported from a private module path that doesn't export it
+    (guaranteed ImportError in adopter source)."""
+    path = _write(
+        tmp_path,
+        "code.py",
+        "from adcp.types.generated_poc.core.format import Assets81, Assets149\n",
+    )
+    report = v3_to_v4.run(tmp_path, apply_changes=True, auto_apply=True)
+
+    # File content must be untouched on the unsafe-mixed line.
+    rewritten = path.read_text()
+    assert "from adcp.types.generated_poc.core.format import Assets81, Assets149" in rewritten
+    assert "VideoFormatAsset" not in rewritten
+
+    # Findings: Assets81 and Assets149 are both flagged for review (the
+    # adopter has to split the import or wait for Assets149 to land in
+    # the rename table). Neither is reported as auto_applied.
+    auto_applied_names = {f.before for f in report.auto_applied}
+    assert "Assets81" not in auto_applied_names
+    assert "Assets149" not in auto_applied_names
+    flagged_names = {f.before for f in report.flagged if f.kind == "flag_numbered"}
+    assert "Assets81" in flagged_names
+    assert "Assets149" in flagged_names
+
+
 # ---------------------------------------------------------------------------
 # --auto-apply: combined behaviour + idempotency
 # ---------------------------------------------------------------------------