refactor(commands): split ads.py into modular components

[axisrow]

Summary

Splits the 3 156-line direct_cli/commands/ads.py (79 functions/classes) into a command package with type-specific modules, mirroring the campaigns.py refactoring (#602). No CLI surface changes — every command, flag, and emitted payload is byte-identical.

Closes #603

Layout

Module	Responsibility
ads/__init__.py	Package entry; re-exports ads, build_ad_object, build_ad_update_object, create_client
ads/_cli.py	Thin Click router: group + get/add/update options + lifecycle registration
ads/objects.py	build_ad_object / build_ad_update_object — pure item builders shared by single-item + batch paths
ads/batch.py	--from-file / --ads-json row normalization + chunked send
ads/base.py	Shared validation/parsing helpers (incompatible-flag guard, callouts, price-extension, CSV/ID parsers)
ads/text.py	TEXT_AD / TEXT_IMAGE_AD / DYNAMIC_TEXT_AD
ads/responsive.py	RESPONSIVE_AD
ads/shopping.py	SHOPPING_AD / LISTING_AD (feed-based)
ads/mobile_app.py	MOBILE_APP_AD / MOBILE_APP_IMAGE_AD
ads/builder.py	*_AD_BUILDER_AD + SMART_AD_BUILDER_AD

Design notes

• Public surface unchanged. from direct_cli.commands.ads import ads, build_ad_object, build_ad_update_object works as before. The parity gate (COMMAND_WSDL_MAP, STRATEGY_TYPES, nillable handling) is untouched.
• create_client patchability preserved. The package re-exports create_client (aliased from _api in _cli.py); scripts/build_api_coverage_report.py now patches the _cli submodule when a group is a package, so the wire-payload capture keeps intercepting the live path.
• Lazy imports break the cycle. batch.py imports add/update from _cli inside the param-type helpers (the CLI module imports batch at top level), so no circular import at load time.

Verification

• pytest tests/test_wsdl_parity_gate.py tests/test_api_coverage.py tests/test_cli.py tests/test_comprehensive.py tests/test_ads_build_ad_object.py tests/test_ads_build_ad_update_object.py → 371 passed, 6 skipped
• Full offline tier (-m "not integration...", ignoring the pre-existing aiohttp/vcr cassette env breakage) → 2507 passed
• ruff check . → clean
• mypy direct_cli/commands/ads → no issues
• scripts/build_api_coverage_report.py → strict_parity_ok, live_model_parity_ok, schema_parity_ok all true

Note: tests/test_read_cassettes.py / test_integration_write.py error out in this worktree on an unrelated aiohttp.streams/vcr version mismatch — pre-existing, not introduced here (they touch no ads code).

🤖 Generated with Claude Code

[axisrow]

📋 Review summary — all cycles

Reviewed locally (/review + Codex companion), no bots pinged. Cycle 1/3 clean on the first pass.

Cycle	Reviewer	Finding	Verdict	Resolution
1	codex	(none)	APPROVE	No critical payload/security/data-loss regression; builders preserve payload assembly + key ordering
1	claude	hasattr(module, "_cli") package-patch heuristic is an implicit contract	SKIP	Working as designed + documented for ads package; covered by WSDL parity gate. Hypothetical risk for future campaigns split (#602), not a current defect — left as-is
1	claude	Manual flags_local dict + flags.get() unpacker risks silent None on new flag	SKIP	Inherited 1:1 from old ads.py (byte-identity refactor); long-term maintainability concern, out of scope for this pure move

Totals: 0 FIX, 2 SKIP, 0 UNVERIFIED. No blocking issues. CI green (test-and-report 3.9/3.11/3.13 + quality = SUCCESS).

Byte-identical-payload invariance holds: single-item and --from-file/--ads-json batch paths both route through the same ctx-free build_ad_object / build_ad_update_object, with batch rows coerced through the single-flag Click ParamType.

Ready to merge.

[axisrow]

📋 Review summary — all cycles

Local review (cycle 1/3) — reviewed locally via /review + Codex companion (adversarial). No bots pinged.

PR #611: pure refactor splitting the 3156-line ads.py monolith into an ads/ package (_cli router + objects/batch/base + per-subtype text/responsive/shopping/mobile_app/builder). Issue #603.

Cycle	Reviewer	Finding	Verdict	Resolution
1	codex	No critical behavioral regression; 49 ads/CLI unit tests pass	APPROVE	—
1	claude	FeedFilterConditions wrapped in {"Items":...} in update but not add (shopping.py:66 vs :133)	non-blocking	Preserved historical behavior — byte-identical old-vs-new payloads confirm not a regression. Changing it would violate issue #603. Left as-is.

Verification beyond the reviewers:

• CLI surface 1:1 — subcommand set identical (add/archive/delete/get/moderate/resume/suspend/unarchive/update).
• Byte-identical payloads proven empirically across 11 ad-type/operation cases (TEXT_AD add+update, RESPONSIVE/SHOPPING/MOBILE_APP add, DYNAMIC_TEXT_AD add+feed, TEXT_IMAGE_AD, CPC_VIDEO_AD, RESPONSIVE/SHOPPING update) vs the pre-refactor monolith.
• test_wsdl_parity_gate + test_api_coverage + test_dry_run_ads + test_dry_run_common — all green.
• Merge conflict with refactor(tests): split test_dry_run.py into thematic modules (closes #604) #609 (test_dry_run.py split) resolved: the PR's one test edit moved into tests/test_dry_run_common.py; old monolith not restored. Diff-stat matches the original PR file-for-file (3439+/3154−).

Totals: 0 FIX, 0 UNVERIFIED, 1 non-blocking observation (left as-is with rationale). Review complete.

Review mode: local — no auto-merge. PR is mergeable/clean and CI is green; merge is yours to trigger.

[axisrow]

📋 Review summary — rebase verification (cycle 1/3, clean)

Re-review after rebase + force-push to resolve the main conflict (rebased #611 onto current main incl. merged #609 + WSDL fix). Reviewed locally (/review + Codex companion) at head 952bd6e vs base main@eb88810. No bots pinged.

Reviewer	Finding	Verdict	Resolution
codex	(none)	APPROVE	"Against 952bd6e, all ads definitions preserved; payload builders structurally identical to eb88810; add/update/batch-add/batch-update smoke probes produced identical item payloads"
claude	rebase resolution of test_dry_run_common.py (reads ads/ package instead of deleted ads.py)	OK	Correct adaptation after merged #609 — test reads all .py in package except __init__.py
claude	ads/__init__.py re-exports intact after rebase	OK	ads, create_client, build_ad_object, build_ad_update_object re-exported as before
claude	hasattr(module,"_cli") package-patch heuristic	SKIP	Working + documented; covered by WSDL parity gate (carried forward, non-blocking)
claude	manual flags_local dict + flags.get() unpacker	SKIP	Inherited 1:1 from old code (byte-identity invariant); applying = scope creep

Totals: 0 FIX, 0 UNVERIFIED, 2 non-blocking SKIP. CI green on 952bd6e (test-and-report 3.9/3.11/3.13 + quality = SUCCESS). PR MERGEABLE/CLEAN.

Byte-identical-payload invariance confirmed post-rebase by Codex smoke probes. Ready to merge.