Skip to content

feat(migrate): --auto-apply mode for mechanically-rewritable codemod findings#540

Merged
bokelley merged 5 commits into
mainfrom
claude/issue-512-codemod-auto-apply
May 4, 2026
Merged

feat(migrate): --auto-apply mode for mechanically-rewritable codemod findings#540
bokelley merged 5 commits into
mainfrom
claude/issue-512-codemod-auto-apply

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented May 4, 2026

Closes #512

Summary

  • Extends python -m adcp.migrate v3-to-v4 with --auto-apply: rewrites the ~78% of findings that are mechanically safe while leaving flag_removed findings for human review.
  • flag_private — from adcp.types.generated_poc.X import Symbol lines are rewritten to from adcp.types import Symbol when every symbol on the line has a verified public alias in GENERATED_POC_SYMBOL_MAP. Mixed lines (any unknown symbol) remain flagged.
  • flag_numbered — Assets<N> symbols with entries in the new NUMBERED_ASSETS_RENAMES table (Assets81–Assets106) are substituted with their semantic aliases (VideoFormatAsset, etc.). Unknown numbered assets remain flagged.
  • Two-pass rewrite: (1) full-file numbered-asset substitution, (2) line-by-line generated_poc import-path fix where all post-rename symbols are in _AUTO_APPLY_PUBLIC_SYMBOLS.
  • --auto-apply implies --apply; enforced before the dirty-tree guard.
  • JSON report gains auto_applied array (additive, no version bump).
  • Text report gains "Safe rewrites" section and a discoverability tip for --apply users.
  • MIGRATION_v3_to_v4.md updated with --auto-apply example and description.
  • Bug fix (pre-existing): unknown symbols on mixed generated_poc import lines were silently dropped from the report; they now always produce a flag_private finding.

What was tested

  • 53 tests, all pass (pytest tests/test_migrate_v3_to_v4.py)
  • ruff check src/adcp/migrate/ tests/test_migrate_v3_to_v4.py — clean
  • mypy src/adcp/migrate/ — zero errors in migrate/ files

Key test coverage:

  • test_dry_run_with_auto_apply_does_not_write_files — apply_changes=False, auto_apply=True must not write
  • test_auto_apply_numbered_plus_known_symbol_same_line — mixed numbered + known symbol line is fully auto-applied
  • test_auto_apply_mixed_line_not_rewritten — unknown symbol prevents rewrite of that line
  • test_auto_apply_idempotent — second pass is a no-op
  • test_auto_apply_exits_nonzero_when_flag_removed_remain — CI gate is preserved

Pre-PR expert review

DX expert — LGTM with fixes applied:

  • ✅ Discoverability: text report hints at --auto-apply when flag_private/flag_numbered findings remain
  • ✅ --apply help cross-links --auto-apply
  • ✅ Dirty-tree error shows --auto-apply when that flag was used
  • ✅ Exit codes (0/1/2) documented in argparse description
  • ✅ MIGRATION_v3_to_v4.md updated

Code reviewer — found 2 bugs, both fixed:

  • ✅ apply_changes=False, auto_apply=True was silently writing files — guarded with apply_changes and auto_apply and auto_apply_hits
  • ✅ all_known check excluded numbered-asset symbols on mixed lines (Assets81, ContextObject) — fixed to treat NUMBERED_ASSETS_RENAMES keys as resolvable when auto_apply=True

https://claude.ai/code/session_01MiPRbdJUEFBes5zzUGd5Vb

Generated by Claude Code

bokelley added a commit that referenced this pull request May 4, 2026
@bokelley @claude
fixup(migrate): don't corrupt mixed numbered+unknown imports under --…
392db89 
…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>
@bokelley bokelley marked this pull request as ready for review May 4, 2026 02:07
@bokelley bokelley force-pushed the claude/issue-512-codemod-auto-apply branch from 392db89 to 17df4b9 Compare May 4, 2026 02:13
bokelley added a commit that referenced this pull request May 4, 2026
@bokelley @claude
fix(migrate): don't corrupt mixed numbered+unknown imports under --au…
17df4b9 
…to-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>
claude and others added 5 commits May 3, 2026 22:18
@claude @bokelley
feat(migrate): --auto-apply mode for mechanically-rewritable codemod …
1ba551b 
…findings

Extends `python -m adcp.migrate v3-to-v4` with `--auto-apply`: rewrites
the ~78% of findings that are mechanically safe (flag_private + flag_numbered
with known aliases) while leaving flag_removed findings for human review.

- Add NUMBERED_ASSETS_RENAMES dict mapping Assets<N> to semantic aliases
- Add _AUTO_APPLY_PUBLIC_SYMBOLS frozenset for safe-rewrite eligibility
- Add auto_applied field to Report; route kind==auto_applied there
- Two-pass rewrite: (1) numbered asset substitution, (2) generated_poc
  import path fix for lines where all symbols have public equivalents
- Fix pre-existing silent-drop bug: unknown symbols on mixed import lines
  were silently discarded; now always emit flag_private
- --auto-apply implies --apply; enforced before dirty-tree guard
- JSON report gains auto_applied array (additive, no version bump)
- 16 new tests (total 49, all pass)

Closes #512

https://claude.ai/code/session_01MiPRbdJUEFBes5zzUGd5Vb
@claude @bokelley
fix(migrate): apply dx-expert review feedback on --auto-apply
23563b3 
- Text report hints at --auto-apply when flag_private/flag_numbered
  findings remain and the flag was not used (discoverability)
- --apply help text cross-links --auto-apply
- Dirty-tree error shows --auto-apply when that flag was used
- Exit code semantics (0/1/2) documented in argparse description
- MIGRATION_v3_to_v4.md: add --auto-apply example + description
- 2 new tests for tip-line presence/absence (51 total, all pass)

https://claude.ai/code/session_01MiPRbdJUEFBes5zzUGd5Vb
@claude @bokelley
fix(migrate): address code-reviewer findings on --auto-apply
53a401c 
Two bugs found in pre-PR review:

1. apply_changes=False guard missing on auto-apply rewrite block.
   run(apply_changes=False, auto_apply=True) was silently writing files
   because the auto_apply block only checked `auto_apply and auto_apply_hits`
   with no apply_changes guard. Fixed by requiring apply_changes too.

2. all_known check excluded numbered-asset symbols on mixed lines.
   A line like `from ...generated_poc import Assets81, ContextObject`
   got all_known=False (Assets81 not in GENERATED_POC_SYMBOL_MAP), so
   ContextObject was flagged even though both symbols are auto-applicable.
   Fixed: all_known now treats numbered assets as resolvable when auto_apply=True.

New tests:
- test_dry_run_with_auto_apply_does_not_write_files (regression guard)
- test_auto_apply_numbered_plus_known_symbol_same_line (mismatch case)

53 tests total, all pass.

https://claude.ai/code/session_01MiPRbdJUEFBes5zzUGd5Vb
@claude @bokelley
chore(migrate): fix orphaned test section header; document scan_file …
67c07d2 
…auto_apply contract

- Remove misplaced "Pre-existing silent-drop bug fix" section header
  that sat above the discoverability tests; reattach it immediately
  before test_mixed_line_unknown_symbol_not_silently_dropped
- scan_file docstring now documents that auto_apply=True without
  apply_changes=True produces findings but never writes files

https://claude.ai/code/session_01MiPRbdJUEFBes5zzUGd5Vb
@bokelley @claude
fix(migrate): don't corrupt mixed numbered+unknown imports under --au…
ce10f70 
…to-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>
@bokelley bokelley force-pushed the claude/issue-512-codemod-auto-apply branch from 17df4b9 to ce10f70 Compare May 4, 2026 02:18
@bokelley bokelley merged commit d19e71e into main May 4, 2026
15 checks passed
@github-actions github-actions Bot mentioned this pull request May 4, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

codemod: auto-apply mode for mechanically-rewritable findings

2 participants

@bokelley @claude