Complete #198 mutating WSDL audit#214
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR completes the mutating-command WSDL parity audit (#198) by tightening subtype-aware flag validation across several write commands, updating dry-run/coverage gates to catch silent-loss and invalid payload shapes, and adding a static audit mode for the WRITE_SANDBOX surface to run in CI without credentials.
Changes:
- Enforce subtype-specific flag compatibility (reject incompatible flags instead of silently dropping them) for
campaigns add,adgroups add,ads add,bidmodifiers add, andstrategies add/update. - Fix remaining WSDL/CLI drift (e.g.,
TextAd.AdImageHash,IncomeGradeAdjustments[].Grade,SmartTvAdjustment, and rejecting WSDL-invalidbidmodifiers setlegacy payload shape). - Add static WRITE_SANDBOX audit tooling (
scripts/sandbox_write_audit.py,scripts/test_sandbox_write.sh --audit) and expand/align tests and docs to match the new contract.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_wsdl_parity_gate.py | Extends the parity gate probes and fixes smartadtargets WSDL container mapping. |
| tests/test_sandbox_write_audit.py | Adds CI tests for the static WRITE_SANDBOX audit (markdown + JSON output; no-credentials mode). |
| tests/test_low_coverage_payloads.py | Updates strategy flag expectations and adds rejection tests for invalid typed fields. |
| tests/test_integration_write.py | Updates integration regression guard messaging for bidmodifiers set legacy-shape rejection. |
| tests/test_dry_run.py | Adds helpers and new dry-run tests for subtype-flag rejection + WSDL field mapping fixes. |
| tests/test_api_coverage.py | Strengthens exclusion rationale policing and validates referenced focused tests exist. |
| scripts/test_sandbox_write.sh | Adds --audit mode to run the static coverage audit without credential resolution. |
| scripts/sandbox_write_audit.py | New static audit script that inspects live-runner handler coverage and emits JSON/markdown artifacts. |
| README.md | Updates strategy examples to use --weekly-spend-limit / --bid-ceiling (WSDL-aligned). |
| docs/audits/issue-198-mutating-wsdl-audit.md | Adds the committed audit artifact documenting the regenerated 83-row surface audit. |
| direct_cli/commands/strategies.py | Implements WSDL-aligned subtype field allow-lists and rejects invalid typed flags (incl. --spend-limit). |
| direct_cli/commands/campaigns.py | Rejects smart-only flags on non-smart types and guards --filter-average-cpc strategy compatibility. |
| direct_cli/commands/bidmodifiers.py | Adds subtype extra-flag validation, fixes WSDL field names/subtypes, and rejects legacy set payload shape. |
| direct_cli/commands/ads.py | Adds subtype flag allow-lists, preserves TextAd.AdImageHash, and rejects incompatible subtype flags. |
| direct_cli/commands/adgroups.py | Adds subtype flag compatibility checks for dynamic vs smart adgroup-specific flags. |
Comment on lines
358
to
+362
| raise click.UsageError( | ||
| "Provide either --id (preferred) or both --campaign-id " | ||
| "and --type (legacy)." | ||
| "Provide --id with --value for bidmodifiers set. " | ||
| "The legacy --campaign-id/--type shape is not supported by " | ||
| "WSDL BidModifierSetItem; use bidmodifiers add to create a " | ||
| "new modifier." |
Comment on lines
+252
to
+254
| _reject_incompatible_flags( | ||
| ad_type_norm, type_fields[ad_type_norm], provided, flag_for | ||
| ) |
Comment on lines
+36
to
+51
| def _reject_incompatible_flags( | ||
| command_type: str, | ||
| allowed_flags: set[str], | ||
| provided_flags: dict[str, object], | ||
| ) -> None: | ||
| """Reject typed flags that do not belong to the chosen subtype.""" | ||
| incompatible = [ | ||
| flag | ||
| for flag, value in provided_flags.items() | ||
| if value is not None and flag not in allowed_flags | ||
| ] | ||
| if incompatible: | ||
| raise click.UsageError( | ||
| f"{', '.join(sorted(incompatible))} is not compatible with --type " | ||
| f"{command_type}." | ||
| ) |
This was referenced May 20, 2026
axisrow
added a commit
that referenced
this pull request
May 21, 2026
… (#217) * fix(0.3.9): remove feed cassette workaround + Copilot follow-ups Closes #206. - tests/conftest.py: drop `_FEED_REGRESSION_PATTERNS` skip workaround; sandbox_feed now passes `--business-type RETAIL` (matching what `TestWriteFeeds.test_add_update_delete` already does). A cassette body mismatch (until refresh) is now reported as an explicit `pytest.skip` pointing at the rewrite command, instead of a stale `Missing option '--business-type'` skip. - tests/test_wsdl_parity_gate.py: assert that COMMAND_WSDL_MAP's container exists in the WSDL request schema before the no-required- fields skip. Misnamed containers can no longer be silently masked (Copilot follow-up from PR #205). - tests/test_wsdl_parity_gate.py: clean `WSDL_FIELD_TO_CLI_OPTION` — `SourceType` → `{--url}`, `ImageData` → `{--image-data, --image-file}`. Remove the non-existent generic `--file` flag (Copilot follow-up from PR #205). - direct_cli/commands/bidmodifiers.py: hide the rejected legacy `--campaign-id`/`--type` options on `bidmodifiers set` via the existing `keywords update`-style eager-callback pattern (hidden=True, expose_value=False, is_eager=True). `--help` no longer advertises the rejected path while the UsageError text is preserved for regression coverage (Copilot follow-up from PR #214). - tests/test_dry_run.py: adapt `test_bidmodifiers_set_id_and_legacy_flags_are_mutex` to assert on the canonical "legacy --campaign-id/--type shape is not supported" message — legacy flags now fail before the mutex check via eager callback semantics. - CHANGELOG.md: add 0.3.9 (Unreleased) Fixed bullets covering all four sub-fixes. Note: VCR cassettes `TestWriteFeeds.test_add_update_delete.yaml` and `TestWriteSmartAdTargets.test_add_update_delete.yaml` need response-side refresh against a valid sandbox login (current response bodies contain a stale `nonexistent username` error). Until then `TestWriteSmartAdTargets` skips with an actionable "cassette needs refresh" message instead of the previous misleading `Missing option '--business-type'` skip. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * fix(tests): refresh feed cassettes against live sandbox Completes issue #206 cassette refresh path. - tests/conftest.py: _invoke now also passes --login; without it the CLI resolved login from the active `direct auth` profile, which rarely matches the sandbox token's account. Also invert the credentials chain for tests (env vars > profile) per CLAUDE.md, so a plain `pytest` never silently hits production via an active profile. - tests/conftest.py: sandbox_smart_adgroup now passes --counter-id 12345678 — SmartCampaignAddItem.CounterId is WSDL minOccurs=1 (this requirement was previously masked by the feed workaround). - tests/test_integration_write.py: TestWriteFeeds wraps the `feeds update` failure path in `_is_sandbox_error` + skip, mirroring the add path. The sandbox occasionally returns 8800 on the just-created feed; this is a sandbox quirk, not a CLI regression. - tests/cassettes/test_integration_write/TestWriteFeeds.test_add_update_delete.yaml and TestWriteSmartAdTargets.test_add_update_delete.yaml: re-recorded via `pytest -m integration_write --record-mode=rewrite -k "TestWriteFeeds or TestWriteSmartAdTargets"`. Body now contains `"BusinessType":"RETAIL"` for `feeds.add`. Both tests skip on authentic sandbox limitations instead of the missing-option proxy. - Removed the transitional "Can't overwrite existing cassette" skip branch in `sandbox_feed`: bodies now match. - CHANGELOG.md: past-tense wording, no PENDING follow-up note. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> --------- Co-authored-by: axisrow <[email protected]> Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
This was referenced May 21, 2026
axisrow
added a commit
that referenced
this pull request
May 21, 2026
Reword 0.3.10 entry from "fix per-type rejection" to "cover per-type rejection" to avoid implying a behavioral fix shipped in this release — the corrected rejection behavior was already shipped in 0.3.9 via the #198 audit follow-up PRs (#205/#208/#214/#202/#204). This PR only adds regression coverage. Addresses Copilot review comment on PR #226. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
axisrow
added a commit
that referenced
this pull request
May 21, 2026
…226) * test: lock down subtype validation invariants from #210 repro matrix Add 9 new SILENT_LOSS_PROBES in tests/test_wsdl_parity_gate.py covering campaigns add, adgroups add, ads add, bidmodifiers add and strategies add incompatible flag/--type combinations from the #210 umbrella repro matrix. Add 3 non-regression tests in tests/test_dry_run.py for strategies update field aliases (AverageCpcPerFilter→FilterAverageCpc, PayForConversion→Cpa) and confirm AverageCpa update without --goal-id stays WSDL-valid. Refs: Closes #210. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * docs(changelog): clarify PR is test-only, not behavioral fix Reword 0.3.10 entry from "fix per-type rejection" to "cover per-type rejection" to avoid implying a behavioral fix shipped in this release — the corrected rejection behavior was already shipped in 0.3.9 via the #198 audit follow-up PRs (#205/#208/#214/#202/#204). This PR only adds regression coverage. Addresses Copilot review comment on PR #226. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> --------- Co-authored-by: axisrow <[email protected]> Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #198
Refs #210
Refs #213
Summary
WRITE_SANDBOXsurface with a committed 83-row artifact indocs/audits/issue-198-mutating-wsdl-audit.mdcampaigns addsmart-only flag rejection and--filter-average-cpcstrategy guardadgroups addsubtype flag compatibilityads addTextAd.AdImageHashplus incompatible subtype flag rejectionbidmodifiers addincompatible extra flag rejection, WSDLGrade, andSmartTvAdjustmentbidmodifiers setlocal rejection for the legacy WSDL-invalidCampaignId + Typeshapestrategies add/updateWSDL subtype field allow-list and explicit rejection for raw--spend-limitsmartadtargetsgate container fromWebpagestoSmartAdTargetsWRITE_SANDBOXlive-runner audit mode (scripts/test_sandbox_write.sh --audit) with follow-up-only live-runner gaps tracked in Follow up: classify clients.update and v4tags update live sandbox gaps #213Verification
python3 -m pytest tests/test_wsdl_parity_gate.py -q-> 46 passed, 6 skippedpython3 -m pytest tests/test_dry_run.py tests/test_low_coverage_payloads.py tests/test_api_coverage.py tests/test_smoke_matrix.py -q-> 355 passedpython3 -m pytest -m "not integration"-> 760 passed, 44 skipped, 32 deselectedpython3 scripts/build_api_coverage_report.py > /tmp/api_coverage_report.jsonsummary.schema_parity_ok == trueschema.nested_schema_violations == []python3 scripts/sandbox_write_audit.py --json-output /tmp/sandbox_audit.json-> 83 rows audited; 80 PASS, 3 NOT_COVERED tracked in Follow up: classify clients.update and v4tags update live sandbox gaps #213python3 -m ruff check .-> passmypy .-> passgit diff --check-> passpython3 -m black --checkon changed Python files -> passNotes
The original "7 Med + 4 Low" table was not recoverable from the repo, issue text, or targeted local transcript search available in this workspace, so the artifact records a fresh regenerated WSDL/code/dry-run audit instead. #198 should remain open until this PR has passed CI/review and the final issue comment is posted.