refactor(commands): shared execute_request for single-item mutation tails (#589 #13)

[axisrow]

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

Что сделано

В ~22 модулях одиночные add/update/set-команды заканчивались одинаковым хвостом из 4 шагов — печать body под --dry-run, иначе построить клиент, отправить и отформатировать result().extract() как JSON; различался только сервис (RPC-метод уже лежит в body). Этот хвост вынесён в один execute_request(ctx, service, body, dry_run, create_client) в direct_cli/commands/_execute.py; построение body (сборка item) остаётся в каждой команде.

Хелпер назван метод-агностично (по итогам /simplify-ревью): покрывает add, update и set-bids одинаково.

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

• create_client передаётся из module global вызывающей команды (резолвится в момент вызова), поэтому patch.object(<module>, "create_client") продолжает перехватывать live-путь (в отличие от фабрик; обычное тело команды перечитывает глобал каждый вызов). Покрыто test_cli (патч ads/campaigns create_client + invoke mutation).
• get-команды не затронуты (у них format_output(..., output_format, output), не hardcoded JSON — regex не совпадает).
• Полный офлайн-прогон: 2536 passed (PAYLOAD_CASES для add/update dry-run + cassettes для post-пути); ruff чисто.
• /simplify: 2 ревьюера. Applied: метод-агностичное имя + покрытие update (altitude). Отклонено: слияние с _batch.send_batch (разные задачи — chunking/partial-success vs простой single-item хвост); «patchability-риск» — намеренный корректный дизайн, доказан suite.

Diff: +97 / −297 (нетто −200), 23 файла.

Остаётся в #589

• fix+test: stop sending implicit Type field in add commands; add dry-run coverage #12 — make_set_bids_command (дедуп тела set-bids: сборка Bids[] + валидация селектор/bid-полей в 4 модулях).

🤖 Generated with Claude Code

[axisrow]

@claude @codex review. Focus on critical issues only. Mechanical dedup (#589 #13): the identical single-item add/update/set tail (dry-run print → client_from_ctx → client.().post(data=body) → format extract as JSON) across ~22 modules is hoisted into one method-agnostic execute_request in _execute.py. Per-command body construction is unchanged. Please confirm (a) byte-identity of every migrated tail — no request method/service/payload change, dry-run output identical; (b) patchability preserved — create_client is passed from each caller's module global (resolved at call time), so patch.object(, create_client) still intercepts; (c) the regex did not migrate any get tail (those format with --format, not hardcoded JSON) or any command with extra processing in the tail. Do NOT nitpick style/naming/formatting.

[claude]

Claude finished @axisrow's task in 2m 16s —— View job

---

Review: execute_request dedup (#589 #13)

Reviewed the full diff against origin/main, the new helper, and statically verified import hygiene and tail coverage. Focused on critical issues only, per the ask.

Verdict: LGTM — all three invariants hold.

(a) Byte-identity of migrated tails ✅

The helper reproduces the four-step tail exactly:

if dry_run: format_output(body, "json", None); return
client = client_from_ctx(ctx, create_client)
result = getattr(client, service)().post(data=body)
format_output(result().extract(), "json", None)

getattr(client, service)() is equivalent to the inline client.<svc>() for every migrated call (verified service strings match the original method names: ads, campaigns, agencyclients, smartadtargets, vcards, advideos, creatives, clients, keywords, negativekeywordsharedsets, sitelinks, etc.). Every diff I inspected is a pure tail replacement with no payload/method/service change and no extra processing dropped. dry-run path is character-for-character identical ("json", None).

(b) Patchability preserved ✅

create_client is passed positionally from each caller's module global, resolved in the command function's globals at call time. So patch.object("direct_cli.commands.<module>", "create_client", …) still intercepts the live path — the helper only forwards the already-resolved reference into client_from_ctx. client_from_ctx itself is unpatched and unchanged. Correct by construction.

(c) No get tail or extra-processing tail migrated ✅

Confirmed the 35 remaining inline result().extract() sites are legitimately not byte-identical and were correctly skipped:

• get commands format with format_output(data, output_format, output) (not hardcoded "json", None) and branch on fetch_all/iter_items (e.g. bids.py, clients.py, retargeting.py).
• reports.py, changes.py, dictionaries.py, keywordsresearch.py, leads.py and the _get.py/_lifecycle.py/_batch.py helpers are untouched.

The regex was conservative in the right direction — modules like bids/retargeting/smartadtargets correctly have both a migrated mutation tail and a preserved inline get tail.

Import hygiene ✅

No dangling imports: advideos, vcards, negativekeywordsharedsets dropped both client_from_ctx and format_output (their only user was the migrated tail); modules that kept those imports still use them in a get/other command. Verified via occurrence counts (every retained import has ≥1 use).

Note (non-blocking)

I could not execute pytest/ruff here — Bash invocations were blocked by the sandbox permission layer (not a code problem). The verification above is static. The PR's reported 2536 passed + clean ruff is consistent with what I see; the test_cli patch-and-invoke coverage for ads/campaigns create_client is exactly the right guard for invariant (b).

No critical issues found. 👍
· branch refactor/589-execute-add

[axisrow]

@claude reviewed clean (LGTM, all three invariants verified — byte-identity, patchability, no get/extra-tail migration), CI green. Codex didn't post within the window this round; merging on Claude's comprehensive clean review per the finalize gate. #12 (set-bids body dedup) remains in #589.

[claude]

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

---

Acknowledged — merge confirmed ✅

Noted: PR #594 merged on the clean review (all three invariants verified — byte-identity of the migrated single-item add/update/set tails, patchability via caller-module-global create_client, and no get/extra-processing tail migrated), with CI green and Codex absent within this round's finalize window.

Nothing further to do here. Remaining in #589:

• fix+test: stop sending implicit Type field in add commands; add dry-run coverage #12 — make_set_bids_command (dedup of the set-bids body: Bids[] assembly + selector/bid-field validation across the 4 modules).
  · branch claude/pr-594-20260620-0706