refactor(campaigns)!: rename --*-fields → --*-field-names (BREAKING) by axisrow · Pull Request #421 · axisrow/direct-cli

axisrow · 2026-05-28T04:15:01Z

Summary

BREAKING CHANGE. The ten --*-fields options on direct campaigns get are renamed to their WSDL-canonical --*-field-names form (one per *FieldNames parameter declared by CampaignsGetRequest). The previous names are no longer accepted by Click.

Renames:

Old (removed)	New (canonical)
`--text-campaign-fields`	`--text-campaign-field-names`
`--mobile-app-campaign-fields`	`--mobile-app-campaign-field-names`
`--dynamic-text-campaign-fields`	`--dynamic-text-campaign-field-names`
`--cpm-banner-campaign-fields`	`--cpm-banner-campaign-field-names`
`--smart-campaign-fields`	`--smart-campaign-field-names`
`--unified-campaign-fields`	`--unified-campaign-field-names`
`--text-campaign-search-strategy-placement-types-fields`	`--text-campaign-search-strategy-placement-types-field-names`
`--dynamic-text-campaign-search-strategy-placement-types-fields`	`--dynamic-text-campaign-search-strategy-placement-types-field-names`
`--unified-campaign-search-strategy-placement-types-fields`	`--unified-campaign-search-strategy-placement-types-field-names`
`--unified-campaign-package-bidding-strategy-platforms-fields`	`--unified-campaign-package-bidding-strategy-platforms-field-names`

Per the new "No legacy CLI flag aliases" rule in CLAUDE.md (added in this branch's follow-up): when the WSDL canonical name differs from an existing CLI flag, rename it as a breaking change rather than keeping the old name as a second Click alias.

Closes #409. Seventh PR in Wave 2 milestone 0.3.14.

Test plan

`pytest tests/test_api_coverage.py::test_every_nested_fieldnames_param_has_cli_option` — passes after row removal.
`pytest tests/test_dry_run.py -k campaigns_get` — all updated tests pass; new rejection test `test_campaigns_get_legacy_field_aliases_removed` confirms the old form is no longer accepted.
`pytest --ignore=tests/test_integration.py --ignore=tests/test_v5_live_write.py --ignore=tests/test_v4_live_contracts.py` — 1851 passed, 21 skipped.
`direct campaigns get --help | grep field-names` — all 10 canonical flags visible; `--text-campaign-fields` etc. are not.

… on get Register the canonical kebab-case form `--xxx-field-names` for every existing `--xxx-fields` legacy flag on `direct campaigns get`. Click accepts both names on the same Python variable, so the `CampaignsGetRequest` WSDL parameter name maps 1:1 to the CLI: - `--text-campaign-field-names` (legacy: `--text-campaign-fields`) - `--mobile-app-campaign-field-names` - `--dynamic-text-campaign-field-names` - `--cpm-banner-campaign-field-names` - `--smart-campaign-field-names` - `--unified-campaign-field-names` - `--text-campaign-search-strategy-placement-types-field-names` - `--dynamic-text-campaign-search-strategy-placement-types-field-names` - `--unified-campaign-search-strategy-placement-types-field-names` - `--unified-campaign-package-bidding-strategy-platforms-field-names` The legacy `-fields` aliases remain accepted unchanged for backward compatibility. The internal `selector_options` dict is untouched — only the `@click.option` decorators above `get` change. Help strings gain sample values (drawn from the corresponding `*FieldEnum` in `tests/wsdl_cache/campaigns.xml`) and call out the alias. Removes the ten corresponding rows from `NESTED_FIELDNAMES_EXCLUSIONS` so `test_every_nested_fieldnames_param_has_cli_option` enforces all ten flags. Adds dry-run tests for: canonical-flag payload coverage (all 10), legacy-alias regression guard (subset), and `--help` visibility. Existing legacy-form tests (`test_campaigns_get_text_campaign_fields_dry_run` etc.) continue passing unchanged. Closes #409. Co-Authored-By: Claude Opus 4.7 <[email protected]>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

axisrow · 2026-05-28T05:09:18Z

@claude review. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences — only flag issues that could break functionality or cause real harm in production.

claude · 2026-05-28T05:09:33Z

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

Claude Code Review — complete

Fetch PR diff vs origin/main
Read direct_cli/commands/campaigns.py changes
Read tests/test_dry_run.py & tests/test_api_coverage.py changes
Look for bugs / regressions / data-loss / perf issues
Post final review

Verdict: No critical issues found

Per the trigger scope (bugs / security / logic errors / data loss / performance only — no style nits), this PR is clean. Summary of why:

Click multi-decl pattern is correct

direct_cli/commands/campaigns.py:531-642 — every changed option uses

@click.option("--canonical-form", "--legacy-form", "snake_case_var", help="...")

Click resolves the first non-- string as the destination variable, so both flags bind to the same Python parameter. The function signature (direct_cli/commands/campaigns.py:658-667) and selector_options dict (:702-743) still reference the unchanged snake_case names — no rewiring needed, and the WSDL request keys (TextCampaignFieldNames, etc.) are unchanged, so the wire payload is bit-for-bit identical for legacy invocations.

Backward compatibility is preserved end-to-end

Existing dry-run tests (test_campaigns_get_text_campaign_fields_dry_run and siblings) still target the legacy flag names and continue to pass.
The new test_campaigns_get_legacy_field_aliases_still_accepted test (tests/test_dry_run.py:22099-22116) explicitly guards two legacy aliases.
No removal of any flag, alias, env var, or environment variable.

Coverage tightening is safe

Removing 10 rows from NESTED_FIELDNAMES_EXCLUSIONS (tests/test_api_coverage.py:2518-2527) makes test_every_nested_fieldnames_param_has_cli_option strictly enforce the canonical CLI flags. The new help-output test (tests/test_dry_run.py:22115) further pins the flag names. If a future change drops one of them, both tests fail loudly — a net safety win.

No data-loss / security / perf concerns

Read-only get path; no mutation.
No new auth, network, parsing, or trust-boundary logic.
Click option registration cost is constant per option at import time — irrelevant.

Non-blocking observation (informational, not a bug)

direct_cli/commands/campaigns.py:702-743 still passes the legacy flag string (e.g. "--text-campaign-fields") as the option_name argument to _parse_csv_option. That string only surfaces in the UsageError raised when a user passes an empty value (_parse_csv_option, :91). So a user who runs --text-campaign-field-names "" will see an error referencing --text-campaign-fields instead of the flag they actually typed. This is purely a UX message issue — no functional impact, no data loss, no regression. Mentioning only for completeness; the trigger explicitly excluded UX/style nits, so I am not flagging this as a required change.

Ship it

Diff is small, surgical, well-tested, and adds an explicit regression guard for the backward-compat surface. No blockers.
· Branch: feat/409-campaigns-nested-field-names

…ield-names BREAKING CHANGE: The ten `--*-fields` options on `direct campaigns get` (`--text-campaign-fields`, `--mobile-app-campaign-fields`, etc.) are removed in favour of the WSDL-canonical `--*-field-names` form. The parameter name now maps 1:1 to the kebab-case CLI flag. Per CLAUDE.md "No legacy CLI flag aliases" rule: when the WSDL canonical name differs from an existing CLI flag, rename it as a breaking change rather than keeping the old name as a second Click alias. Also enforces the rule by a rejection test for the old form.

Resolve conflicts with #415 #416 #417 #418 #419 #420: keep all six previously-merged CHANGELOG entries (feeds/keywords/creatives/clients/ agencyclients/adgroups) and place the campaigns BREAKING CHANGES block at the end of Unreleased. Drop the already-removed NESTED_FIELDNAMES_EXCLUSIONS rows. Update the surviving legacy `-fields` test fixtures to the new `-field-names` form, while keeping one explicit rejection test for `--text-campaign-fields` (the old name).

axisrow · 2026-05-28T05:22:30Z

@claude review. PR scope changed since the previous review: aliases were removed per the new no-legacy-aliases rule in CLAUDE.md — this is now a breaking rename. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences — only flag issues that could break functionality or cause real harm in production.

claude · 2026-05-28T05:22:41Z

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

Claude Code Review — complete

Fetch PR diff vs origin/main
Read direct_cli/commands/campaigns.py changes (rename, no aliases)
Read tests/test_dry_run.py & tests/test_api_coverage.py changes
Look for bugs / regressions / data-loss / perf issues introduced by removing the aliases
Post final review

Verdict: No critical issues found

Re-reviewed after the scope flip from "alias both names" → "rename, drop the legacy form." Per the trigger (bugs / security / logic / data-loss / perf only — no style nits), this PR is clean.

Rename is wired end-to-end with no stragglers

direct_cli/commands/campaigns.py:529-628 — all 10 options replaced with their --*-field-names form; help strings updated and now reference the WSDL parameter names with concrete examples.
direct_cli/commands/campaigns.py:637-666 — function signature renamed in lockstep (text_campaign_field_names, …). Click derives the Python parameter name from the first non-dash option string, so the new flag names map cleanly to the new snake_case variables.
direct_cli/commands/campaigns.py:680-721 — selector_options dict references both the renamed option strings (used only in _parse_csv_option's UsageError) and the renamed variables. The legacy-error-message pitfall I flagged in the previous review is fixed here: a user typing --text-campaign-field-names "" now sees an error citing the canonical flag, not the old one.
WSDL request keys (TextCampaignFieldNames, …) are unchanged → wire payload is bit-for-bit identical to the previous canonical-alias invocation.

No silent data loss from the breakage

Click rejects unknown options with a hard UsageError (exit code 2), so any caller still passing --text-campaign-fields will fail loudly rather than have the value silently dropped. This is the only safe form a breaking rename can take, and it's what test_campaigns_get_legacy_field_aliases_removed (tests/test_dry_run.py:22593-22606) explicitly pins.

Test coverage tightens rather than weakens

test_campaigns_get_canonical_field_names_flags_payload (tests/test_dry_run.py:22552-22577) — drives all 10 new flags in a single invocation and asserts the dry-run payload carries each value under the right WSDL key. Strong regression guard against future name drift.
test_campaigns_get_help_exposes_canonical_field_names_flags (tests/test_dry_run.py:22580-22585) — pins the --help surface, catching accidental removal.
test_campaigns_get_legacy_field_aliases_removed — locks the breaking-change behaviour.
tests/test_api_coverage.py:2505-2514 — 10 rows removed from NESTED_FIELDNAMES_EXCLUSIONS so test_every_nested_fieldnames_param_has_cli_option now strictly enforces canonical CLI flags for every Campaigns*FieldNames WSDL parameter. Net coverage win.
Pre-existing dry-run tests (test_campaigns_get_text_campaign_fields_dry_run, test_campaigns_get_campaign_specific_fields_dry_run, test_campaigns_get_strategy_placement_fields_dry_run, test_campaigns_get_rejects_empty_campaign_specific_fields_csv) all updated to the new flag names and the empty-CSV error message assertion now matches the canonical flag.

No leftover references in the repo

grep across source, tests, docs, and scripts finds no stale --*-fields references outside (a) the CHANGELOG entry announcing the rename and (b) the test that explicitly verifies rejection. The README has no campaign-fields references to update.

Breaking-change is correctly advertised

CHANGELOG.md:107-122 lists all 10 renames under a **BREAKING CHANGES:** heading with the explicit "no longer accepted — update scripts and automation accordingly" callout.
Conventional-commit footer (! in refactor(campaigns)!:) is present on d1b261b.

No data-loss / security / perf concerns

Read-only get path; no mutation, no new auth/network/parsing/trust-boundary logic.
Click option registration cost is constant at import time — irrelevant.
Removing aliases reduces Click's option table; no perf regression possible.

Non-blocking observation (informational only — not a bug, not flagging per trigger scope)

The test function name test_campaigns_get_text_campaign_fields_dry_run (tests/test_dry_run.py:342) still carries the old _fields suffix in its identifier. The test body itself was correctly updated to drive --text-campaign-field-names. Purely cosmetic; no functional impact. The trigger excluded naming/style nits, so I'm not flagging this as a required change.

Ship it

Breaking rename is surgical, fully wired, well-tested, and announced. No blockers.
· Branch: feat/409-campaigns-nested-field-names

When the WSDL canonical name differs from an existing CLI flag, rename it as a breaking change rather than keeping the old name as a second Click alias. Reference implementation: PR #421 (campaigns). This rule was articulated during Wave 2 (#402) review and applies project-wide. New CLI options must be exactly the kebab-case form of the WSDL request parameter (e.g. `SitelinkFieldNames` → `--sitelink-field-names`). Co-authored-by: axisrow <[email protected]>

Resolve conflicts with #415 #416 #417 #418 #419 #420 #421: keep all six previously-merged CHANGELOG entries (feeds/keywords/creatives/clients/ agencyclients/adgroups), the campaigns BREAKING CHANGES block, and add the bidmodifiers entry as a separate 'Additional features' block. Drop already-removed NESTED_FIELDNAMES_EXCLUSIONS rows. Append bidmodifiers tests after the canonical campaigns block in test_dry_run.py.

Copilot AI review requested due to automatic review settings May 28, 2026 04:15

Copilot AI reviewed May 28, 2026

axisrow added 2 commits May 28, 2026 12:17

axisrow changed the title ~~feat(campaigns): canonical --*-field-names aliases for legacy -fields on get~~ refactor(campaigns)!: rename --*-fields → --*-field-names (BREAKING) May 28, 2026

axisrow merged commit 97215b6 into main May 28, 2026
6 checks passed

axisrow deleted the feat/409-campaigns-nested-field-names branch May 28, 2026 05:27

axisrow mentioned this pull request May 28, 2026

docs(claude): add 'no legacy CLI flag aliases' rule #423

Merged

2 tasks

This was referenced May 28, 2026

Wave 2: expose remaining nested *FieldNames request parameters as CLI options #402

Closed

audit: снимок покрытия Yandex Direct API после волны #415–#425 (v5 ✅, v4 23/33) #428

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(campaigns)!: rename ---fields → ---field-names (BREAKING)#421

refactor(campaigns)!: rename ---fields → ---field-names (BREAKING)#421
axisrow merged 3 commits into
mainfrom
feat/409-campaigns-nested-field-names

axisrow commented May 28, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

axisrow commented May 28, 2026

Uh oh!

claude Bot commented May 28, 2026 •

edited

Loading

Uh oh!

axisrow commented May 28, 2026

Uh oh!

claude Bot commented May 28, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

axisrow commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

axisrow commented May 28, 2026

Uh oh!

claude Bot commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Claude Code Review — complete

Verdict: No critical issues found

Click multi-decl pattern is correct

Backward compatibility is preserved end-to-end

Coverage tightening is safe

No data-loss / security / perf concerns

Non-blocking observation (informational, not a bug)

Ship it

Uh oh!

axisrow commented May 28, 2026

Uh oh!

claude Bot commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Claude Code Review — complete

Verdict: No critical issues found

Rename is wired end-to-end with no stragglers

No silent data loss from the breakage

Test coverage tightens rather than weakens

No leftover references in the repo

Breaking-change is correctly advertised

No data-loss / security / perf concerns

Non-blocking observation (informational only — not a bug, not flagging per trigger scope)

Ship it

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

axisrow commented May 28, 2026 •

edited

Loading

claude Bot commented May 28, 2026 •

edited

Loading

claude Bot commented May 28, 2026 •

edited

Loading