refactor(commands): shared register_lifecycle_commands; drop 8 local wrappers (#589 #11)

[axisrow]

Part of #589 (finding #11) / follow-up of #582 / эпик #584.

Что сделано

Восемь модулей (dynamicads, audiencetargets, smartadtargets, dynamicfeedadtargets, ads, campaigns, keywords, strategies) определяли почти идентичную локальную обёртку _<resource>_lifecycle(method, help_text) — только чтобы повторить make_lifecycle_command(group, ..., id_param, id_help, create_client) для delete/suspend/resume (+ archive/unarchive/moderate у ads/campaigns/strategies).

Восемь обёрток сведены в один общий register_lifecycle_commands(group, id_param, id_help, create_client, specs) в _lifecycle.py; каждый модуль передаёт свой список (method, help_text).

Байт-идентичность

• Все lifecycle-команды по-прежнему регистрируются через @group.command внутри фабрики; удалённые module-level биндинги (delete=/suspend=/…) нигде не импортировались (проверено grep).
• Примеры --help (ads delete, campaigns archive, strategies unarchive, dynamicads suspend) — на месте.
• Полный офлайн-прогон: 2536 passed; ruff чисто.
• /simplify: 3 ревьюера; по сходящейся рекомендации убран преждевременный **kwargs из сигнатуры хелпера (немигрированные модули с non-default id-путями имеют по одной команде — batch им не нужен; вернуть тривиально при появлении реального кейса). Остальное — без изменений (module-level биндинги не используются, имя/import-time/spec-валидация — приемлемо).

Diff: +115 / −84 (структурный дедуп: 8 функций-обёрток → 1 общий хелпер; multi-line спеки от black дают нетто +31).

Остаётся в #589

• fix+test: stop sending implicit Type field in add commands; add dry-run coverage #12 — make_set_bids_command (4 модуля).
• feat(ads get): add --text-ad-fields for type-specific field retrieval #13 — execute_add (~17 add-команд).

🤖 Generated with Claude Code

[axisrow]

@claude @codex review. Focus on critical issues only. This is a mechanical dedup (#589 #11): 8 near-identical local lifecycle wrappers replaced by one shared register_lifecycle_commands in _lifecycle.py. CLI surface is byte-identical — every delete/suspend/resume/archive/unarchive/moderate command still registers via the factory's @group.command (the discarded module-level bindings were unreferenced). Please confirm (a) no lifecycle command is dropped or renamed across the 8 modules, (b) the dropped delete=/suspend= module-level bindings are truly unreferenced. Do NOT nitpick style/naming/formatting.

[claude]

Claude finished @axisrow's task in 1m 47s —— View job

---

Review — #589 #11: shared register_lifecycle_commands

• Read the new register_lifecycle_commands helper in _lifecycle.py
• Confirm (a) no lifecycle command dropped/renamed across the 8 modules
• Confirm (b) dropped delete=/suspend= module-level bindings are unreferenced
• Post review

Verdict: LGTM — clean, behavior-preserving dedup. No critical issues. Both requested invariants hold.

(a) No command dropped or renamed ✓

The new helper forwards positionally to make_lifecycle_command, and the order matches the old wrappers exactly:

# helper: register_lifecycle_commands(group, id_param, id_help, create_client, specs)
make_lifecycle_command(group, method, help_text, id_param, id_help, create_client)
# old wrapper: make_lifecycle_command(group, method, help_text, id_param, id_help, create_client)

Per-module the (method, help_text) lists, id_param, and id_help are byte-for-byte the same as the deleted wrappers:

Module	id_param / id_help	Commands (before == after)
ads	ad_id / "Ad ID"	delete, archive, unarchive, suspend, resume, moderate
campaigns	campaign_id / "Campaign ID"	delete, archive, unarchive, suspend, resume
strategies	strategy_id / "Strategy ID"	archive, unarchive
audiencetargets	target_id / "Target ID"	delete, suspend, resume
dynamicads	target_id / "Target ID"	delete, suspend, resume
dynamicfeedadtargets	target_id / "Target ID"	delete, suspend, resume
keywords	keyword_id / "Keyword ID"	delete, suspend, resume
smartadtargets	target_id / "Target ID"	delete, suspend, resume

None of the migrated modules used the non-default kwargs (service, id_option, id_type, criteria_key), so the helper not exposing them is correct — the modules that need --hash/AdImageHash (e.g. adimages) were intentionally left on the old pattern. Registration is a decorator side-effect inside make_lifecycle_command (@group.command), so discarding the return value changes nothing.

(b) Dropped module-level bindings truly unreferenced ✓

• commands/__init__.py imports only the group symbols (from .ads import ads, etc.) — never delete/suspend/resume/archive/unarchive/moderate.
• No from .<module> import <lifecycle> or commands.<module>.<lifecycle> reference anywhere in direct_cli/ or tests/. (The one test_v4_contracts.py:312 delete.example_param hit is an unrelated v4-contract object, not a command binding.)
• A grep for the old module-level binding pattern matches only the non-migrated modules still using it — confirming the 8 targets no longer define those names and nothing depended on them.

Notes (non-blocking, not nitpicks)

• The factory verification I attempted (importing each group and asserting .commands contains the expected names) couldn't run here — the sandbox blocked python execution — so the table above is from static diff analysis. The PR's reported full run (2536 passed) plus tests/test_comprehensive.py::TestCommandsRegistered covers the live registration assertion.
  • refactor/589-lifecycle-wrappers

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. 👍

Reviewed commit: b7bc0c08fc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[axisrow]

Thanks @claude @codex — both clean, no FIX items. Confirmed: all 8 modules' (method, help_text) lists + id_param/id_help are byte-for-byte preserved; dropped module-level bindings unreferenced (only group symbols are imported in commands/init.py); kwargs-less helper is correct (single-command non-default-id modules like adimages stay on the direct make_lifecycle_command path). #12/#13 remain in #589. Merging (CI green).

[chatgpt-codex-connector]

To use Codex here, create an environment for this repo.

[claude]

Claude finished @axisrow's task in 36s —— View job

---

Confirmed — merge looks correct ✅

Verified the merged register_lifecycle_commands on origin/main (4ff263c) — it matches the review exactly:

def register_lifecycle_commands(group, id_param, id_help, create_client, specs):
    for method, help_text in specs:
        make_lifecycle_command(
            group, method, help_text, id_param, id_help, create_client
        )

• (a) No command dropped/renamed — the helper forwards positionally to make_lifecycle_command(group, method, help_text, id_param, id_help, create_client), identical arg order to the 8 deleted wrappers; each module's (method, help_text) list + id_param/id_help are byte-for-byte preserved.
• (b) Dropped module-level bindings unreferenced — only group symbols are imported in commands/__init__.py; nothing depended on the discarded delete=/suspend=/… names. Registration is a @group.command decorator side-effect, so discarding the return value changes nothing.
• kwargs-less helper is correct — non-default-id / single-command modules (e.g. adimages with --hash/AdImageHashes) intentionally stay on the direct make_lifecycle_command path; the batch helper doesn't need to expose service/id_option/id_type/criteria_key.

Both Claude and Codex reviews were clean, no FIX items, CI green. #12 (make_set_bids_command) / #13 (execute_add) remain in #589. Nothing to action here. 👍
• claude/pr-593-20260620-0526