From 73e064260f42b8a5734a54547c26f8012dc710fd Mon Sep 17 00:00:00 2001 From: Illia Sapryga Date: Sun, 1 Nov 2026 16:00:32 -0600 Subject: [PATCH 1/2] Geo exclusions and ad schedule on draft_campaign and update_campaign draft_campaign puts them in the campaign's atomic mutate. update_campaign replaces the whole list: omitted leaves it, [] clears it. The preview shows the current values and apply refuses if they changed after it; only the entries that differ are removed or added. Windows use add_ad_schedule's validation. A first schedule and clearing a schedule need a double confirmation, as does dropping a geo exclusion. --- .claude/rules/adloop.md | 5 +- .cursor/rules/adloop.mdc | 5 +- README.md | 4 +- src/adloop/ads/assets.py | 14 +- src/adloop/ads/write.py | 269 ++++++++++++- src/adloop/rules/adloop.md | 5 +- src/adloop/server.py | 33 +- tests/test_campaign_geo_exclude_schedule.py | 403 ++++++++++++++++++++ 8 files changed, 717 insertions(+), 21 deletions(-) create mode 100644 tests/test_campaign_geo_exclude_schedule.py diff --git a/.claude/rules/adloop.md b/.claude/rules/adloop.md index 5ebde5f..8094c46 100644 --- a/.claude/rules/adloop.md +++ b/.claude/rules/adloop.md @@ -371,13 +371,13 @@ Conversions that happen offline (a sales call, a lead that turned into a deal) g | Tool | What It Does | Validation | |------|-------------|------------| -| `draft_campaign` | Create full campaign structure (budget + campaign + ad group + keywords + geo/language targeting) | `campaign_name`, `daily_budget`, `bidding_strategy`, `geo_target_ids` (REQUIRED), `language_ids` (REQUIRED), optional `search_partners_enabled`, `display_network_enabled`, `display_expansion_enabled`, optional `max_cpc` for MANUAL_CPC ad-group bids or TARGET_SPEND CPC caps, optional `location_option` / `negative_location_option`, optional `tracking_url_template` / `final_url_suffix` / `url_custom_parameters` | +| `draft_campaign` | Create full campaign structure (budget + campaign + ad group + keywords + geo/language targeting) | `campaign_name`, `daily_budget`, `bidding_strategy`, `geo_target_ids` (REQUIRED), `language_ids` (REQUIRED), optional `search_partners_enabled`, `display_network_enabled`, `display_expansion_enabled`, optional `max_cpc` for MANUAL_CPC ad-group bids or TARGET_SPEND CPC caps, optional `location_option` / `negative_location_option`, optional `tracking_url_template` / `final_url_suffix` / `url_custom_parameters`, optional `geo_exclude_ids` / `ad_schedule` | | `draft_shopping_campaign` | Create a Standard Shopping campaign for a Merchant Center feed: budget + campaign (PAUSED) + product ad group + Shopping ad + one "All products" product group, atomically | `campaign_name`, `daily_budget`, `merchant_id` (must be linked to the Ads account; checked), `product_bid` (CPC of the product group, required by Google), `geo_target_ids` (REQUIRED), `bidding_strategy` MANUAL_CPC (default) / TARGET_SPEND (Maximize clicks, optional `max_cpc`) / TARGET_ROAS (+ `target_roas`), `campaign_priority` LOW/MEDIUM/HIGH, optional `feed_label`. No keywords, no languages | | `draft_asset_group_assets` | Add and/or remove assets of an existing PMax asset group in one atomic change: text, images, YouTube videos | `asset_group_id`, `headlines` (≤30), `long_headlines` (≤90), `descriptions` (≤90), `business_name` (≤25), `images` (`{"url"}` or `{"upload_id"}`, optional `role` marketing/logo/landscape_logo; marketing images sorted into landscape 1.91:1, square 1:1, portrait 4:5 by ratio), `youtube_video_ids`, `remove_asset_ids` | | `draft_asset_group_signals` | Add search themes or set the audience signal of a PMax asset group, or remove signals | `asset_group_id`, `add_search_themes` (≤80 chars, up to 50 per group), `audience_id` (one per group), `remove_signal_ids` | | `draft_pmax_product_exclusions` | Set the full list of products a retail PMax asset group excludes, on one dimension; everything else stays included | `asset_group_id`, `exclude_values` (the COMPLETE list; `[]` includes everything again), `dimension` item_id (default) / brand / custom_label_0–4 | | `draft_ad_group` | Create a new ad group within an existing campaign (does NOT publish) | `campaign_id` (REQUIRED), `ad_group_name` (REQUIRED), `keywords` (optional list of {text, match_type}), `cpc_bid_micros` (optional) | -| `update_campaign` | Modify existing campaign settings — bid strategy, budget, geo targets, language targets, location options, Search partners, display expansion, URL options, campaign-level broad match | `campaign_id` (REQUIRED), plus any of: `bidding_strategy`, `daily_budget`, `geo_target_ids`, `language_ids`, `location_option`, `negative_location_option`, `search_partners_enabled`, `display_network_enabled`, TARGET_SPEND `max_cpc`, `tracking_url_template`, `final_url_suffix`, `url_custom_parameters`, `keyword_match_type` | +| `update_campaign` | Modify existing campaign settings — bid strategy, budget, geo targets, language targets, location options, Search partners, display expansion, URL options, campaign-level broad match | `campaign_id` (REQUIRED), plus any of: `bidding_strategy`, `daily_budget`, `geo_target_ids`, `language_ids`, `location_option`, `negative_location_option`, `search_partners_enabled`, `display_network_enabled`, TARGET_SPEND `max_cpc`, `tracking_url_template`, `final_url_suffix`, `url_custom_parameters`, `keyword_match_type`, `geo_exclude_ids`, `ad_schedule` | | `update_ad_group` | Update ad group name and/or MANUAL_CPC `max_cpc` | `ad_group_id`, optional `ad_group_name`, optional `max_cpc` | | `draft_responsive_search_ad` | Create RSA preview (does NOT publish) | 3-15 headlines (≤30 chars), 2-4 descriptions (≤90 chars), final_url required, path1/path2 (≤15 chars each). Each headline/description may be a plain string (unpinned) or `{"text": "...", "pinned_field": "HEADLINE_1"}` (pinned). Valid pin slots: `HEADLINE_1/2/3`, `DESCRIPTION_1/2`. Several assets may pin to one slot (they rotate there), but every slot must stay fillable: by an asset pinned to it or by a distinct unpinned asset. | | `draft_callouts` | Create callout assets for an ad group, a campaign or the account (does NOT publish) | `campaign_id` or `ad_group_id` (the scope follows the id), or `scope="account"`; `callouts` list with 1-25 chars each | @@ -464,6 +464,7 @@ Same draft → preview → `confirm_and_apply` gate; the plan carries `platform: - Campaign URL options (`tracking_url_template`, `final_url_suffix`, `url_custom_parameters`): omitted leaves a value unchanged, `""` or `[]` removes it, and `url_custom_parameters` replaces the whole list. The template must contain `{lpurl}` or a variant (`{lpurl+2}`, `{unescapedlpurl}`, `{escapedlpurl+3}`, ...). Custom parameter keys are alphanumeric, at most 8 characters. The `update_campaign` preview shows each current value (`current_tracking_url_template` etc.), warns on a removal, and gives `example_click`: one of the campaign's final URLs with the campaign's options applied. It is approximate: a template on the ad group, ad or keyword overrides the campaign's, and ValueTrack parameters are filled at click time. Check it for typos before approving. Read back with `get_campaign_settings`. - Campaign-level broad match: `update_campaign` `keyword_match_type="BROAD"` CONVERTS every existing phrase and exact keyword in the campaign into broad match. Google shows the originals as Removed, turning the setting off does not restore them, and since September 2026 Google also upgrades such campaigns to AI Max. It is refused unless the campaign (or the `bidding_strategy` in the same call) is Smart Bidding — the same rule as BROAD keywords — checked again at apply, and needs a double confirmation. `keyword_match_type="UNSPECIFIED"` turns it off (single confirmation): no further conversions, but converted keywords stay broad and must be re-added as phrase/exact by hand. Moving a broad-match campaign to a non-Smart `bidding_strategy` is refused unless the same call passes `UNSPECIFIED`. Never turn it on to "get more traffic" on an exact/phrase campaign; read it back with `get_campaign_settings`, which lists it in `widening_settings`. - `update_campaign` replaces POSITIVE geo/language targets entirely (not append). Pass the full desired list. NEGATIVE geo exclusions (criteria with `negative=TRUE`) are **preserved** across a positive-geo replacement — they survive the swap. The preview surfaces preserved negative geo IDs in `preserved_negative_geo_target_ids` so the change is auditable. To ADD negative geo exclusions (e.g. exclude a city inside a targeted state), use `add_negative_locations` — do not try to express exclusions through `geo_target_ids`. To remove a negative geo exclusion explicitly, use `remove_entity` with `entity_type="campaign_criterion"`. +- `geo_exclude_ids` and `ad_schedule` set exclusions and windows when creating a campaign, in the same atomic request. On `update_campaign` they REPLACE the whole list: omitted leaves it unchanged, `[]` clears it. Use them to set the complete state; `add_negative_locations` and `add_ad_schedule` add to what is there. The preview shows the current values (`current_geo_exclude_ids`, `current_ad_schedule`) and apply refuses if they changed after it. Double confirmation is needed for a campaign's first schedule (also on create), for clearing a schedule (the campaign serves at all hours again) and for dropping an exclusion (it can serve there again). - **Asset scope:** callouts and snippets link where the id points (`ad_group_id` → ad group, `campaign_id` → campaign). Account-wide (`scope="account"`) reaches every campaign without assets of its own; use it only when the user asks for account level. **Asset edits** (`update_callout`, `update_sitelink`, `update_structured_snippet`) change the asset everywhere it is linked; check with GAQL (`campaign_asset`, `ad_group_asset`, `customer_asset`) where else it is used before editing a shared asset. If the apply reports the asset changed in Google Ads since the preview, draft again. - **Call assets, schedules, promotions and prices.** Never guess `country_code` or `language_code`: take them from the number and the landing page, and ask when unclear. Money amounts are in the account currency (the preview names it); pass `currency_code` only for a different one. Ad schedule hours are in the **account time zone** (the previews show it), not the searcher's: convert the user's local hours first. A campaign without a schedule serves at all hours, so its first `add_ad_schedule` windows switch off every other hour and day: say so, the preview needs a double confirmation. To change a window, remove the old one with `remove_entity` (`entity_type="campaign_criterion"`, the `remove_id` from the preview), then add the new one. Prices must match the landing page or Google disapproves them. - `remove_entity` is IRREVERSIBLE — always prefer `pause_entity` unless the user explicitly wants permanent removal. Removal triggers double confirmation in the safety layer. diff --git a/.cursor/rules/adloop.mdc b/.cursor/rules/adloop.mdc index 7ae9818..e13d757 100644 --- a/.cursor/rules/adloop.mdc +++ b/.cursor/rules/adloop.mdc @@ -373,13 +373,13 @@ Conversions that happen offline (a sales call, a lead that turned into a deal) g | Tool | What It Does | Validation | |------|-------------|------------| -| `draft_campaign` | Create full campaign structure (budget + campaign + ad group + keywords + geo/language targeting) | `campaign_name`, `daily_budget`, `bidding_strategy`, `geo_target_ids` (REQUIRED), `language_ids` (REQUIRED), optional `search_partners_enabled`, `display_network_enabled`, `display_expansion_enabled`, optional `max_cpc` for MANUAL_CPC ad-group bids or TARGET_SPEND CPC caps, optional `location_option` / `negative_location_option`, optional `tracking_url_template` / `final_url_suffix` / `url_custom_parameters` | +| `draft_campaign` | Create full campaign structure (budget + campaign + ad group + keywords + geo/language targeting) | `campaign_name`, `daily_budget`, `bidding_strategy`, `geo_target_ids` (REQUIRED), `language_ids` (REQUIRED), optional `search_partners_enabled`, `display_network_enabled`, `display_expansion_enabled`, optional `max_cpc` for MANUAL_CPC ad-group bids or TARGET_SPEND CPC caps, optional `location_option` / `negative_location_option`, optional `tracking_url_template` / `final_url_suffix` / `url_custom_parameters`, optional `geo_exclude_ids` / `ad_schedule` | | `draft_shopping_campaign` | Create a Standard Shopping campaign for a Merchant Center feed: budget + campaign (PAUSED) + product ad group + Shopping ad + one "All products" product group, atomically | `campaign_name`, `daily_budget`, `merchant_id` (must be linked to the Ads account; checked), `product_bid` (CPC of the product group, required by Google), `geo_target_ids` (REQUIRED), `bidding_strategy` MANUAL_CPC (default) / TARGET_SPEND (Maximize clicks, optional `max_cpc`) / TARGET_ROAS (+ `target_roas`), `campaign_priority` LOW/MEDIUM/HIGH, optional `feed_label`. No keywords, no languages | | `draft_asset_group_assets` | Add and/or remove assets of an existing PMax asset group in one atomic change: text, images, YouTube videos | `asset_group_id`, `headlines` (≤30), `long_headlines` (≤90), `descriptions` (≤90), `business_name` (≤25), `images` (`{"url"}` or `{"upload_id"}`, optional `role` marketing/logo/landscape_logo; marketing images sorted into landscape 1.91:1, square 1:1, portrait 4:5 by ratio), `youtube_video_ids`, `remove_asset_ids` | | `draft_asset_group_signals` | Add search themes or set the audience signal of a PMax asset group, or remove signals | `asset_group_id`, `add_search_themes` (≤80 chars, up to 50 per group), `audience_id` (one per group), `remove_signal_ids` | | `draft_pmax_product_exclusions` | Set the full list of products a retail PMax asset group excludes, on one dimension; everything else stays included | `asset_group_id`, `exclude_values` (the COMPLETE list; `[]` includes everything again), `dimension` item_id (default) / brand / custom_label_0–4 | | `draft_ad_group` | Create a new ad group within an existing campaign (does NOT publish) | `campaign_id` (REQUIRED), `ad_group_name` (REQUIRED), `keywords` (optional list of {text, match_type}), `cpc_bid_micros` (optional) | -| `update_campaign` | Modify existing campaign settings — bid strategy, budget, geo targets, language targets, location options, Search partners, display expansion, URL options, campaign-level broad match | `campaign_id` (REQUIRED), plus any of: `bidding_strategy`, `daily_budget`, `geo_target_ids`, `language_ids`, `location_option`, `negative_location_option`, `search_partners_enabled`, `display_network_enabled`, TARGET_SPEND `max_cpc`, `tracking_url_template`, `final_url_suffix`, `url_custom_parameters`, `keyword_match_type` | +| `update_campaign` | Modify existing campaign settings — bid strategy, budget, geo targets, language targets, location options, Search partners, display expansion, URL options, campaign-level broad match | `campaign_id` (REQUIRED), plus any of: `bidding_strategy`, `daily_budget`, `geo_target_ids`, `language_ids`, `location_option`, `negative_location_option`, `search_partners_enabled`, `display_network_enabled`, TARGET_SPEND `max_cpc`, `tracking_url_template`, `final_url_suffix`, `url_custom_parameters`, `keyword_match_type`, `geo_exclude_ids`, `ad_schedule` | | `update_ad_group` | Update ad group name and/or MANUAL_CPC `max_cpc` | `ad_group_id`, optional `ad_group_name`, optional `max_cpc` | | `draft_responsive_search_ad` | Create RSA preview (does NOT publish) | 3-15 headlines (≤30 chars), 2-4 descriptions (≤90 chars), final_url required, path1/path2 (≤15 chars each). Each headline/description may be a plain string (unpinned) or `{"text": "...", "pinned_field": "HEADLINE_1"}` (pinned). Valid pin slots: `HEADLINE_1/2/3`, `DESCRIPTION_1/2`. Several assets may pin to one slot (they rotate there), but every slot must stay fillable: by an asset pinned to it or by a distinct unpinned asset. | | `draft_callouts` | Create callout assets for an ad group, a campaign or the account (does NOT publish) | `campaign_id` or `ad_group_id` (the scope follows the id), or `scope="account"`; `callouts` list with 1-25 chars each | @@ -466,6 +466,7 @@ Same draft → preview → `confirm_and_apply` gate; the plan carries `platform: - Campaign URL options (`tracking_url_template`, `final_url_suffix`, `url_custom_parameters`): omitted leaves a value unchanged, `""` or `[]` removes it, and `url_custom_parameters` replaces the whole list. The template must contain `{lpurl}` or a variant (`{lpurl+2}`, `{unescapedlpurl}`, `{escapedlpurl+3}`, ...). Custom parameter keys are alphanumeric, at most 8 characters. The `update_campaign` preview shows each current value (`current_tracking_url_template` etc.), warns on a removal, and gives `example_click`: one of the campaign's final URLs with the campaign's options applied. It is approximate: a template on the ad group, ad or keyword overrides the campaign's, and ValueTrack parameters are filled at click time. Check it for typos before approving. Read back with `get_campaign_settings`. - Campaign-level broad match: `update_campaign` `keyword_match_type="BROAD"` CONVERTS every existing phrase and exact keyword in the campaign into broad match. Google shows the originals as Removed, turning the setting off does not restore them, and since September 2026 Google also upgrades such campaigns to AI Max. It is refused unless the campaign (or the `bidding_strategy` in the same call) is Smart Bidding — the same rule as BROAD keywords — checked again at apply, and needs a double confirmation. `keyword_match_type="UNSPECIFIED"` turns it off (single confirmation): no further conversions, but converted keywords stay broad and must be re-added as phrase/exact by hand. Moving a broad-match campaign to a non-Smart `bidding_strategy` is refused unless the same call passes `UNSPECIFIED`. Never turn it on to "get more traffic" on an exact/phrase campaign; read it back with `get_campaign_settings`, which lists it in `widening_settings`. - `update_campaign` replaces POSITIVE geo/language targets entirely (not append). Pass the full desired list. NEGATIVE geo exclusions (criteria with `negative=TRUE`) are **preserved** across a positive-geo replacement — they survive the swap. The preview surfaces preserved negative geo IDs in `preserved_negative_geo_target_ids` so the change is auditable. To ADD negative geo exclusions (e.g. exclude a city inside a targeted state), use `add_negative_locations` — do not try to express exclusions through `geo_target_ids`. To remove a negative geo exclusion explicitly, use `remove_entity` with `entity_type="campaign_criterion"`. +- `geo_exclude_ids` and `ad_schedule` set exclusions and windows when creating a campaign, in the same atomic request. On `update_campaign` they REPLACE the whole list: omitted leaves it unchanged, `[]` clears it. Use them to set the complete state; `add_negative_locations` and `add_ad_schedule` add to what is there. The preview shows the current values (`current_geo_exclude_ids`, `current_ad_schedule`) and apply refuses if they changed after it. Double confirmation is needed for a campaign's first schedule (also on create), for clearing a schedule (the campaign serves at all hours again) and for dropping an exclusion (it can serve there again). - **Asset scope:** callouts and snippets link where the id points (`ad_group_id` → ad group, `campaign_id` → campaign). Account-wide (`scope="account"`) reaches every campaign without assets of its own; use it only when the user asks for account level. **Asset edits** (`update_callout`, `update_sitelink`, `update_structured_snippet`) change the asset everywhere it is linked; check with GAQL (`campaign_asset`, `ad_group_asset`, `customer_asset`) where else it is used before editing a shared asset. If the apply reports the asset changed in Google Ads since the preview, draft again. - **Call assets, schedules, promotions and prices.** Never guess `country_code` or `language_code`: take them from the number and the landing page, and ask when unclear. Money amounts are in the account currency (the preview names it); pass `currency_code` only for a different one. Ad schedule hours are in the **account time zone** (the previews show it), not the searcher's: convert the user's local hours first. A campaign without a schedule serves at all hours, so its first `add_ad_schedule` windows switch off every other hour and day: say so, the preview needs a double confirmation. To change a window, remove the old one with `remove_entity` (`entity_type="campaign_criterion"`, the `remove_id` from the preview), then add the new one. Prices must match the landing page or Google disapproves them. - `remove_entity` is IRREVERSIBLE — always prefer `pause_entity` unless the user explicitly wants permanent removal. Removal triggers double confirmation in the safety layer. diff --git a/README.md b/README.md index dac65f7..07a6fef 100644 --- a/README.md +++ b/README.md @@ -298,12 +298,12 @@ All write operations follow a **draft → preview → confirm** workflow. Nothin | Tool | What It Does | |------|-------------| -| `draft_campaign` | Create a full campaign structure — budget + campaign (PAUSED) + ad group + optional keywords. Supports Search partners, display expansion, the location option (presence vs presence or interest), campaign URL options (tracking template, final URL suffix, custom parameters), and `max_cpc` for either MANUAL_CPC initial ad-group bids or TARGET_SPEND (Maximize Clicks) CPC caps. | +| `draft_campaign` | Create a full campaign structure — budget + campaign (PAUSED) + ad group + optional keywords. Supports Search partners, display expansion, the location option (presence vs presence or interest), campaign URL options (tracking template, final URL suffix, custom parameters), geo exclusions, an ad schedule, and `max_cpc` for either MANUAL_CPC initial ad-group bids or TARGET_SPEND (Maximize Clicks) CPC caps. | | `draft_shopping_campaign` | Create a Standard Shopping campaign for a linked Merchant Center feed — budget, campaign (PAUSED), product ad group, Shopping ad and an "All products" group in one atomic request. Manual CPC, Maximize clicks or target ROAS. | | `draft_asset_group_assets` | Add and remove Performance Max assets (text, images incl. uploads, YouTube videos) in one change, so replacing an asset never drops a group below Google's minimums | | `draft_asset_group_signals` | Add search themes or set the audience signal of a PMax asset group | | `draft_pmax_product_exclusions` | Exclude products from a retail PMax asset group by item id, brand or custom label | -| `update_campaign` | Modify existing campaign settings — bidding, budget, geo/language targeting, the location option, campaign URL options, campaign-level broad match (Smart Bidding only), Search partners, display expansion, and TARGET_SPEND (Maximize Clicks) `max_cpc` caps. | +| `update_campaign` | Modify existing campaign settings — bidding, budget, geo/language targeting, geo exclusions and the ad schedule (both replace the current list), the location option, campaign URL options, campaign-level broad match (Smart Bidding only), Search partners, display expansion, and TARGET_SPEND (Maximize Clicks) `max_cpc` caps. | | `draft_ad_group` | Create a paused SEARCH_STANDARD ad group inside an existing campaign, with optional MANUAL_CPC `max_cpc`. | | `update_ad_group` | Update an ad group name and/or MANUAL_CPC `max_cpc`. Use `pause_entity` / `enable_entity` for ad-group status changes. | | `draft_responsive_search_ad` | Create RSA preview (3-15 headlines ≤30 chars, 2-4 descriptions ≤90 chars). Warns if headline/description count is below best practice. | diff --git a/src/adloop/ads/assets.py b/src/adloop/ads/assets.py index 5e4d23e..39f2599 100644 --- a/src/adloop/ads/assets.py +++ b/src/adloop/ads/assets.py @@ -1132,6 +1132,14 @@ def update_call_asset( return preview +def _first_schedule_warning(campaign_name: str) -> str: + return ( + f"Campaign '{campaign_name}' has no ad schedule, so it serves at all hours. " + "With these windows it serves only inside them: every other hour, and every " + "day without a window, stops serving." + ) + + def _campaign_schedule(config: AdLoopConfig, customer_id: str, campaign_id: str) -> tuple[dict | None, list[dict]]: """The campaign (name, status, account time zone) and its current windows.""" from adloop.ads.gaql import execute_query @@ -1193,11 +1201,7 @@ def add_ad_schedule( warnings = [_time_zone_note(campaign["time_zone"])] first = not existing if first: - warnings.append( - f"Campaign '{campaign['name']}' has no ad schedule, so it serves at all hours. " - "With these windows it serves only inside them: every other hour, and every " - "day without a window, stops serving." - ) + warnings.append(_first_schedule_warning(campaign["name"])) if campaign["status"] != "ENABLED": warnings.append(f"The campaign is {campaign['status']}; the schedule applies once it is enabled.") plan = ChangePlan( diff --git a/src/adloop/ads/write.py b/src/adloop/ads/write.py index 38379e3..8364781 100644 --- a/src/adloop/ads/write.py +++ b/src/adloop/ads/write.py @@ -2085,11 +2085,14 @@ def draft_campaign( tracking_url_template: str | None = None, final_url_suffix: str | None = None, url_custom_parameters: list[dict] | None = None, + geo_exclude_ids: list[str] | None = None, + ad_schedule: list[dict] | None = None, ) -> dict: """Draft a full campaign structure — returns preview, does NOT execute. Creates: CampaignBudget + Campaign (PAUSED) + AdGroup + optional Keywords - + geo targeting + language targeting. + + geo targeting + language targeting + optional geo exclusions and ad + schedule, all in one atomic mutate. Ads are NOT included — use draft_responsive_search_ad separately. geo_target_ids: list of geo target constant IDs (e.g. ["2276"] for Germany, @@ -2100,7 +2103,17 @@ def draft_campaign( Empty leaves Google's default (PRESENCE_OR_INTEREST for both). tracking_url_template / final_url_suffix / url_custom_parameters: campaign URL options. Omitted or empty leaves them unset. + geo_exclude_ids: geo target constant IDs to exclude; may not overlap + geo_target_ids. + ad_schedule: serving windows, validated like add_ad_schedule. They are the + campaign's first schedule, so the plan needs a double confirmation. """ + from adloop.ads.assets import ( + _account_settings, + _first_schedule_warning, + _time_zone_note, + _validate_schedule, + ) from adloop.safety.guards import ( SafetyViolation, check_blocked_operation, @@ -2151,6 +2164,10 @@ def draft_campaign( display_network_enabled=normalized_display_network_enabled, max_cpc=max_cpc, ) + geo_exclude_ids, exclude_errors = _normalize_geo_exclude_ids(geo_exclude_ids, geo_target_ids) + errors.extend(exclude_errors) + schedule, schedule_errors = _validate_schedule(ad_schedule or []) + errors.extend(schedule_errors) if errors: return {"error": "Validation failed", "details": errors} @@ -2160,6 +2177,15 @@ def draft_campaign( return {"error": str(e)} warnings.extend(_location_option_warnings(location_option, negative_location_option)) + target_changes: dict = {} + if geo_exclude_ids: + target_changes["geo_exclude_ids"] = geo_exclude_ids + if schedule: + time_zone = _account_settings(config, customer_id)["time_zone"] + target_changes["ad_schedule"] = schedule + target_changes["account_time_zone"] = time_zone + warnings.append(_time_zone_note(time_zone)) + warnings.append(_first_schedule_warning(campaign_name)) location_changes = {} if location_option: location_changes["location_option"] = location_option @@ -2186,7 +2212,10 @@ def draft_campaign( "max_cpc": max_cpc if max_cpc else None, **location_changes, **url_options, + **target_changes, }, + # A campaign's first windows switch off every hour outside them. + requires_double_confirm=bool(schedule), ) store_plan(plan) preview = plan.to_preview() @@ -2276,6 +2305,8 @@ def update_campaign( final_url_suffix: str | None = None, url_custom_parameters: list[dict] | None = None, keyword_match_type: str = "", + geo_exclude_ids: list[str] | None = None, + ad_schedule: list[dict] | None = None, ) -> dict: """Draft an update to an existing campaign — returns preview, does NOT execute. @@ -2286,6 +2317,9 @@ def update_campaign( the preview shows the current values and an example click URL. keyword_match_type BROAD (campaign-level broad match on) needs Smart Bidding and a double confirmation; UNSPECIFIED turns it off. + geo_exclude_ids and ad_schedule REPLACE the campaign's exclusions and + windows: None leaves them unchanged, [] clears them. The preview shows the + current values, and apply refuses if those changed after the preview. """ from adloop.safety.guards import ( SafetyViolation, @@ -2357,6 +2391,14 @@ def update_campaign( errors.append("geo_target_ids cannot be empty — provide at least one geo target") if language_ids is not None and len(language_ids) == 0: errors.append("language_ids cannot be empty — provide at least one language") + if geo_exclude_ids is not None: + geo_exclude_ids, exclude_errors = _normalize_geo_exclude_ids(geo_exclude_ids, geo_target_ids) + errors.extend(exclude_errors) + if ad_schedule is not None: + from adloop.ads.assets import _validate_schedule + + ad_schedule, schedule_errors = _validate_schedule(ad_schedule) + errors.extend(schedule_errors) if max_cpc: strategy_for_cap = bs or _campaign_bidding_strategy(config, customer_id, campaign_id) if strategy_for_cap is None: @@ -2384,6 +2426,8 @@ def update_campaign( negative_location_option, url_options, keyword_match_type, + geo_exclude_ids is not None, + ad_schedule is not None, ]) if not has_any_change: errors.append("No changes specified — provide at least one parameter to update") @@ -2413,7 +2457,7 @@ def update_campaign( # this preview note makes the preserved-set explicit so the change is # auditable rather than implicit. preserved_negative_geos: list[str] = [] - if geo_target_ids is not None and campaign_id: + if geo_target_ids is not None and geo_exclude_ids is None and campaign_id: preserved_negative_geos = _existing_negative_geo_exclusions( config, customer_id, campaign_id ) @@ -2484,12 +2528,32 @@ def update_campaign( "phrase or exact match keywords." ) + target_changes: dict = {} + targets_need_double_confirm = False + if geo_exclude_ids is not None or ad_schedule is not None: + from adloop.ads.assets import _campaign_schedule + + campaign, current_windows = _campaign_schedule(config, customer_id, campaign_id) + if campaign is None: + return {"error": f"Campaign {campaign_id} was not found in this account"} + if campaign["status"] == "REMOVED": + return {"error": f"Campaign {campaign_id} is removed"} + replacements = [] + if geo_exclude_ids is not None: + replacements.append(_geo_exclude_replacement(config, customer_id, campaign_id, geo_exclude_ids)) + if ad_schedule is not None: + replacements.append(_ad_schedule_replacement(campaign, current_windows, ad_schedule)) + for replacement_changes, replacement_warnings, double_confirm in replacements: + target_changes.update(replacement_changes) + warnings.extend(replacement_warnings) + targets_need_double_confirm = targets_need_double_confirm or double_confirm + # A budget can be shared by several campaigns, and a portfolio bid # strategy too: say who else a budget change reaches, and that a bidding # change takes the campaign out of its portfolio. Both need a double # confirmation, and apply re-checks the state the preview showed. shared: dict = {} - requires_double_confirm = False + requires_double_confirm = targets_need_double_confirm if daily_budget or bs or max_cpc: shared = _budget_and_bidding_context( config, customer_id, campaign_id, @@ -2560,6 +2624,9 @@ def update_campaign( changes["keyword_match_type"] = keyword_match_type if broad_on: requires_double_confirm = True + changes.update(target_changes) + if set(changes) == {"campaign_id"}: + return {"error": f"Campaign {campaign_id} already has these values; nothing to change"} plan = ChangePlan( operation="update_campaign", @@ -3485,8 +3552,91 @@ def _check_budget_and_bidding_unchanged(service, cid: str, changes: dict) -> Non ) +def _normalize_geo_exclude_ids( + geo_exclude_ids: list[str] | None, geo_target_ids: list[str] | None +) -> tuple[list[str], list[str]]: + """``(ids, errors)``: stripped and deduplicated; a blank entry is an error. + + On update an empty list clears the exclusions, so a list of blanks must not + quietly become one. + """ + ids = [str(geo_id).strip() for geo_id in geo_exclude_ids or []] + errors = [] + if any(not geo_id.isdigit() for geo_id in ids): + errors.append("geo_exclude_ids must be numeric Google geo target constant IDs") + overlap = sorted(set(ids) & {str(geo_id).strip() for geo_id in geo_target_ids or []}) + if overlap: + errors.append(f"geo_exclude_ids overlap geo_target_ids: {overlap}") + return list(dict.fromkeys(ids)), errors + + +def _geo_exclude_replacement( + config: AdLoopConfig, customer_id: str, campaign_id: str, geo_exclude_ids: list[str] +) -> tuple[dict, list[str], bool]: + """``(changes, warnings, double_confirm)`` for replacing a campaign's exclusions.""" + current = _existing_negative_geo_exclusions(config, customer_id, campaign_id, strict=True) + if set(current) == set(geo_exclude_ids): + return {}, [f"Geo exclusions are already {sorted(current)}; left as they are."], False + dropped = sorted(set(current) - set(geo_exclude_ids)) + warnings = [] + if dropped: + warnings.append( + f"Removes the geo exclusions {dropped}: the campaign can serve in those " + "locations again if a geo target covers them." + ) + return ( + {"current_geo_exclude_ids": sorted(current), "geo_exclude_ids": geo_exclude_ids}, + warnings, + bool(dropped), + ) + + +def _ad_schedule_replacement( + campaign: dict, existing: list[dict], windows: list[dict] +) -> tuple[dict, list[str], bool]: + """``(changes, warnings, double_confirm)`` for replacing a campaign's schedule. + + Follows add_ad_schedule: a first schedule switches off every hour outside + its windows, so it needs a double confirmation. Clearing one switches every + hour back on, which gets the same. + """ + from adloop.ads.assets import _first_schedule_warning, _time_zone_note + + if {_window_key(window) for window in existing} == {_window_key(window) for window in windows}: + return {}, ["The ad schedule already has these windows; left as it is."], False + + warnings = [_time_zone_note(campaign["time_zone"])] + double_confirm = False + if not existing: + warnings.append(_first_schedule_warning(campaign["name"])) + double_confirm = True + elif not windows: + warnings.append( + f"Removes all {len(existing)} ad schedule windows of campaign '{campaign['name']}': " + "it serves at all hours and on every day again, including the ones the " + "schedule kept it out of." + ) + double_confirm = True + else: + warnings.append( + f"Replaces the {len(existing)} current ad schedule windows of campaign " + f"'{campaign['name']}': it serves only inside the new ones." + ) + if campaign["status"] != "ENABLED": + warnings.append(f"The campaign is {campaign['status']}; the schedule applies once it is enabled.") + return ( + {"account_time_zone": campaign["time_zone"], "current_ad_schedule": existing, "ad_schedule": windows}, + warnings, + double_confirm, + ) + + +def _window_key(window: dict) -> tuple: + return tuple(window[key] for key in ("day_of_week", "start_hour", "start_minute", "end_hour", "end_minute")) + + def _existing_negative_geo_exclusions( - config: AdLoopConfig, customer_id: str, campaign_id: str + config: AdLoopConfig, customer_id: str, campaign_id: str, *, strict: bool = False ) -> list[str]: """Return geo_target_constant IDs that are currently negative-excluded. @@ -3495,7 +3645,8 @@ def _existing_negative_geo_exclusions( Negative location criteria survive a positive-geo replacement (issue #32) — this helper makes that explicit in the preview so users can see what's staying. Returns an empty list on any query failure; - surfacing exclusions is informational, not safety-critical. + surfacing exclusions is informational, not safety-critical. ``strict`` + raises instead, for when the result decides what gets replaced. """ from adloop.ads.gaql import execute_query @@ -3505,10 +3656,13 @@ def _existing_negative_geo_exclusions( WHERE campaign.id = {campaign_id} AND campaign_criterion.type = 'LOCATION' AND campaign_criterion.negative = TRUE + AND campaign_criterion.status != 'REMOVED' """ try: rows = execute_query(config, customer_id, query) except Exception: + if strict: + raise return [] ids: list[str] = [] @@ -4895,11 +5049,20 @@ def _apply_create_campaign(client: object, cid: str, changes: dict) -> dict: ) operations.append(lang_op) + # 7. Geo exclusions and ad schedule, in the same atomic mutate + campaign_path = campaign_service.campaign_path(cid, "-2") + for geo_id in changes.get("geo_exclude_ids") or []: + operations.append(_geo_exclude_create(client, campaign_path, geo_id)) + for window in changes.get("ad_schedule") or []: + operations.append(_ad_schedule_create(client, campaign_path, window)) + response = service.mutate(customer_id=cid, mutate_operations=operations) results = {} num_keywords = len(kw_list) num_geo = len(changes.get("geo_target_ids") or []) + num_lang = len(changes.get("language_ids") or []) + num_exclude = len(changes.get("geo_exclude_ids") or []) for i, resp in enumerate(response.mutate_operation_responses): rn = _extract_resource_name(resp) if rn: @@ -4913,12 +5076,92 @@ def _apply_create_campaign(client: object, cid: str, changes: dict) -> dict: results.setdefault("keywords", []).append(rn) elif i < 3 + num_keywords + num_geo: results.setdefault("geo_targets", []).append(rn) - else: + elif i < 3 + num_keywords + num_geo + num_lang: results.setdefault("language_targets", []).append(rn) + elif i < 3 + num_keywords + num_geo + num_lang + num_exclude: + results.setdefault("geo_excludes", []).append(rn) + else: + results.setdefault("ad_schedule", []).append(rn) return results +def _geo_exclude_create(client: object, campaign_path: str, geo_id: str) -> object: + op = client.get_type("MutateOperation") + criterion = op.campaign_criterion_operation.create + criterion.campaign = campaign_path + criterion.negative = True + criterion.location.geo_target_constant = f"geoTargetConstants/{geo_id}" + return op + + +def _ad_schedule_create(client: object, campaign_path: str, window: dict) -> object: + from adloop.ads.assets import _populate_schedule + + op = client.get_type("MutateOperation") + criterion = op.campaign_criterion_operation.create + criterion.campaign = campaign_path + _populate_schedule(client, criterion.ad_schedule, window) + return op + + +def _criterion_remove(client: object, resource_name: str) -> object: + op = client.get_type("MutateOperation") + op.campaign_criterion_operation.remove = resource_name + return op + + +def _geo_exclude_replacement_ops( + client: object, service: object, cid: str, changes: dict, campaign_path: str +) -> tuple[list, list]: + """``(removes, creates)`` of what differs; refuse if the exclusions moved after the preview.""" + campaign_id = changes["campaign_id"] + rows = service.search(customer_id=cid, query=( + "SELECT campaign_criterion.resource_name, campaign_criterion.location.geo_target_constant " + f"FROM campaign_criterion WHERE campaign.id = {campaign_id} " + "AND campaign_criterion.type = 'LOCATION' AND campaign_criterion.negative = TRUE " + "AND campaign_criterion.status != 'REMOVED'")) + current = { + row.campaign_criterion.location.geo_target_constant.rsplit("/", 1)[-1]: row.campaign_criterion.resource_name + for row in rows + } + if sorted(current) != sorted(changes.get("current_geo_exclude_ids") or []): + raise RuntimeError( + f"The geo exclusions of campaign {campaign_id} changed after this preview " + f"(now {sorted(current)}); nothing was sent. Draft the change again." + ) + wanted = changes["geo_exclude_ids"] + return ( + [_criterion_remove(client, resource) for geo_id, resource in current.items() if geo_id not in wanted], + [_geo_exclude_create(client, campaign_path, geo_id) for geo_id in wanted if geo_id not in current], + ) + + +def _ad_schedule_replacement_ops(client: object, service: object, cid: str, changes: dict, campaign_path: str) -> list: + """Remove and add only what differs; refuse if the windows moved after the preview.""" + campaign_id = changes["campaign_id"] + rows = service.search(customer_id=cid, query=( + "SELECT campaign_criterion.resource_name, campaign_criterion.criterion_id " + f"FROM campaign_criterion WHERE campaign.id = {campaign_id} " + "AND campaign_criterion.type = 'AD_SCHEDULE' AND campaign_criterion.status != 'REMOVED'")) + current = {str(row.campaign_criterion.criterion_id): row.campaign_criterion.resource_name for row in rows} + pinned = changes.get("current_ad_schedule") or [] + if sorted(current) != sorted(window["remove_id"].split("~")[-1] for window in pinned): + raise RuntimeError( + f"The ad schedule of campaign {campaign_id} changed after this preview; " + "nothing was sent. Draft the change again." + ) + wanted = changes["ad_schedule"] + wanted_keys = {_window_key(window) for window in wanted} + pinned_keys = {_window_key(window) for window in pinned} + return [ + *(_criterion_remove(client, current[window["remove_id"].split("~")[-1]]) + for window in pinned if _window_key(window) not in wanted_keys), + *(_ad_schedule_create(client, campaign_path, window) + for window in wanted if _window_key(window) not in pinned_keys), + ] + + def _apply_create_ad_group(client: object, cid: str, changes: dict) -> dict: """Create ad group + optional keywords in an existing campaign atomically.""" service = client.get_service("GoogleAdsService") @@ -5102,6 +5345,16 @@ def _apply_update_campaign(client: object, cid: str, changes: dict) -> dict: ) operations.append(budget_op) + # Geo exclusions replace what the preview showed. Removals go ahead of the + # positive geo operations and creations after them, so a location can move + # from one list to the other in the same request. + exclude_removes, exclude_creates = [], [] + if "geo_exclude_ids" in changes: + exclude_removes, exclude_creates = _geo_exclude_replacement_ops( + client, service, cid, changes, resource_name + ) + operations.extend(exclude_removes) + # Geo targeting — replace POSITIVE location criteria, preserve NEGATIVE # location exclusions. The previous implementation filtered on # campaign_criterion.type = 'LOCATION' alone, which swept up negative @@ -5161,6 +5414,10 @@ def _apply_update_campaign(client: object, cid: str, changes: dict) -> dict: ) operations.append(add_op) + operations.extend(exclude_creates) + if "ad_schedule" in changes: + operations.extend(_ad_schedule_replacement_ops(client, service, cid, changes, resource_name)) + if not operations: return {"message": "No changes to apply"} diff --git a/src/adloop/rules/adloop.md b/src/adloop/rules/adloop.md index 5ebde5f..8094c46 100644 --- a/src/adloop/rules/adloop.md +++ b/src/adloop/rules/adloop.md @@ -371,13 +371,13 @@ Conversions that happen offline (a sales call, a lead that turned into a deal) g | Tool | What It Does | Validation | |------|-------------|------------| -| `draft_campaign` | Create full campaign structure (budget + campaign + ad group + keywords + geo/language targeting) | `campaign_name`, `daily_budget`, `bidding_strategy`, `geo_target_ids` (REQUIRED), `language_ids` (REQUIRED), optional `search_partners_enabled`, `display_network_enabled`, `display_expansion_enabled`, optional `max_cpc` for MANUAL_CPC ad-group bids or TARGET_SPEND CPC caps, optional `location_option` / `negative_location_option`, optional `tracking_url_template` / `final_url_suffix` / `url_custom_parameters` | +| `draft_campaign` | Create full campaign structure (budget + campaign + ad group + keywords + geo/language targeting) | `campaign_name`, `daily_budget`, `bidding_strategy`, `geo_target_ids` (REQUIRED), `language_ids` (REQUIRED), optional `search_partners_enabled`, `display_network_enabled`, `display_expansion_enabled`, optional `max_cpc` for MANUAL_CPC ad-group bids or TARGET_SPEND CPC caps, optional `location_option` / `negative_location_option`, optional `tracking_url_template` / `final_url_suffix` / `url_custom_parameters`, optional `geo_exclude_ids` / `ad_schedule` | | `draft_shopping_campaign` | Create a Standard Shopping campaign for a Merchant Center feed: budget + campaign (PAUSED) + product ad group + Shopping ad + one "All products" product group, atomically | `campaign_name`, `daily_budget`, `merchant_id` (must be linked to the Ads account; checked), `product_bid` (CPC of the product group, required by Google), `geo_target_ids` (REQUIRED), `bidding_strategy` MANUAL_CPC (default) / TARGET_SPEND (Maximize clicks, optional `max_cpc`) / TARGET_ROAS (+ `target_roas`), `campaign_priority` LOW/MEDIUM/HIGH, optional `feed_label`. No keywords, no languages | | `draft_asset_group_assets` | Add and/or remove assets of an existing PMax asset group in one atomic change: text, images, YouTube videos | `asset_group_id`, `headlines` (≤30), `long_headlines` (≤90), `descriptions` (≤90), `business_name` (≤25), `images` (`{"url"}` or `{"upload_id"}`, optional `role` marketing/logo/landscape_logo; marketing images sorted into landscape 1.91:1, square 1:1, portrait 4:5 by ratio), `youtube_video_ids`, `remove_asset_ids` | | `draft_asset_group_signals` | Add search themes or set the audience signal of a PMax asset group, or remove signals | `asset_group_id`, `add_search_themes` (≤80 chars, up to 50 per group), `audience_id` (one per group), `remove_signal_ids` | | `draft_pmax_product_exclusions` | Set the full list of products a retail PMax asset group excludes, on one dimension; everything else stays included | `asset_group_id`, `exclude_values` (the COMPLETE list; `[]` includes everything again), `dimension` item_id (default) / brand / custom_label_0–4 | | `draft_ad_group` | Create a new ad group within an existing campaign (does NOT publish) | `campaign_id` (REQUIRED), `ad_group_name` (REQUIRED), `keywords` (optional list of {text, match_type}), `cpc_bid_micros` (optional) | -| `update_campaign` | Modify existing campaign settings — bid strategy, budget, geo targets, language targets, location options, Search partners, display expansion, URL options, campaign-level broad match | `campaign_id` (REQUIRED), plus any of: `bidding_strategy`, `daily_budget`, `geo_target_ids`, `language_ids`, `location_option`, `negative_location_option`, `search_partners_enabled`, `display_network_enabled`, TARGET_SPEND `max_cpc`, `tracking_url_template`, `final_url_suffix`, `url_custom_parameters`, `keyword_match_type` | +| `update_campaign` | Modify existing campaign settings — bid strategy, budget, geo targets, language targets, location options, Search partners, display expansion, URL options, campaign-level broad match | `campaign_id` (REQUIRED), plus any of: `bidding_strategy`, `daily_budget`, `geo_target_ids`, `language_ids`, `location_option`, `negative_location_option`, `search_partners_enabled`, `display_network_enabled`, TARGET_SPEND `max_cpc`, `tracking_url_template`, `final_url_suffix`, `url_custom_parameters`, `keyword_match_type`, `geo_exclude_ids`, `ad_schedule` | | `update_ad_group` | Update ad group name and/or MANUAL_CPC `max_cpc` | `ad_group_id`, optional `ad_group_name`, optional `max_cpc` | | `draft_responsive_search_ad` | Create RSA preview (does NOT publish) | 3-15 headlines (≤30 chars), 2-4 descriptions (≤90 chars), final_url required, path1/path2 (≤15 chars each). Each headline/description may be a plain string (unpinned) or `{"text": "...", "pinned_field": "HEADLINE_1"}` (pinned). Valid pin slots: `HEADLINE_1/2/3`, `DESCRIPTION_1/2`. Several assets may pin to one slot (they rotate there), but every slot must stay fillable: by an asset pinned to it or by a distinct unpinned asset. | | `draft_callouts` | Create callout assets for an ad group, a campaign or the account (does NOT publish) | `campaign_id` or `ad_group_id` (the scope follows the id), or `scope="account"`; `callouts` list with 1-25 chars each | @@ -464,6 +464,7 @@ Same draft → preview → `confirm_and_apply` gate; the plan carries `platform: - Campaign URL options (`tracking_url_template`, `final_url_suffix`, `url_custom_parameters`): omitted leaves a value unchanged, `""` or `[]` removes it, and `url_custom_parameters` replaces the whole list. The template must contain `{lpurl}` or a variant (`{lpurl+2}`, `{unescapedlpurl}`, `{escapedlpurl+3}`, ...). Custom parameter keys are alphanumeric, at most 8 characters. The `update_campaign` preview shows each current value (`current_tracking_url_template` etc.), warns on a removal, and gives `example_click`: one of the campaign's final URLs with the campaign's options applied. It is approximate: a template on the ad group, ad or keyword overrides the campaign's, and ValueTrack parameters are filled at click time. Check it for typos before approving. Read back with `get_campaign_settings`. - Campaign-level broad match: `update_campaign` `keyword_match_type="BROAD"` CONVERTS every existing phrase and exact keyword in the campaign into broad match. Google shows the originals as Removed, turning the setting off does not restore them, and since September 2026 Google also upgrades such campaigns to AI Max. It is refused unless the campaign (or the `bidding_strategy` in the same call) is Smart Bidding — the same rule as BROAD keywords — checked again at apply, and needs a double confirmation. `keyword_match_type="UNSPECIFIED"` turns it off (single confirmation): no further conversions, but converted keywords stay broad and must be re-added as phrase/exact by hand. Moving a broad-match campaign to a non-Smart `bidding_strategy` is refused unless the same call passes `UNSPECIFIED`. Never turn it on to "get more traffic" on an exact/phrase campaign; read it back with `get_campaign_settings`, which lists it in `widening_settings`. - `update_campaign` replaces POSITIVE geo/language targets entirely (not append). Pass the full desired list. NEGATIVE geo exclusions (criteria with `negative=TRUE`) are **preserved** across a positive-geo replacement — they survive the swap. The preview surfaces preserved negative geo IDs in `preserved_negative_geo_target_ids` so the change is auditable. To ADD negative geo exclusions (e.g. exclude a city inside a targeted state), use `add_negative_locations` — do not try to express exclusions through `geo_target_ids`. To remove a negative geo exclusion explicitly, use `remove_entity` with `entity_type="campaign_criterion"`. +- `geo_exclude_ids` and `ad_schedule` set exclusions and windows when creating a campaign, in the same atomic request. On `update_campaign` they REPLACE the whole list: omitted leaves it unchanged, `[]` clears it. Use them to set the complete state; `add_negative_locations` and `add_ad_schedule` add to what is there. The preview shows the current values (`current_geo_exclude_ids`, `current_ad_schedule`) and apply refuses if they changed after it. Double confirmation is needed for a campaign's first schedule (also on create), for clearing a schedule (the campaign serves at all hours again) and for dropping an exclusion (it can serve there again). - **Asset scope:** callouts and snippets link where the id points (`ad_group_id` → ad group, `campaign_id` → campaign). Account-wide (`scope="account"`) reaches every campaign without assets of its own; use it only when the user asks for account level. **Asset edits** (`update_callout`, `update_sitelink`, `update_structured_snippet`) change the asset everywhere it is linked; check with GAQL (`campaign_asset`, `ad_group_asset`, `customer_asset`) where else it is used before editing a shared asset. If the apply reports the asset changed in Google Ads since the preview, draft again. - **Call assets, schedules, promotions and prices.** Never guess `country_code` or `language_code`: take them from the number and the landing page, and ask when unclear. Money amounts are in the account currency (the preview names it); pass `currency_code` only for a different one. Ad schedule hours are in the **account time zone** (the previews show it), not the searcher's: convert the user's local hours first. A campaign without a schedule serves at all hours, so its first `add_ad_schedule` windows switch off every other hour and day: say so, the preview needs a double confirmation. To change a window, remove the old one with `remove_entity` (`entity_type="campaign_criterion"`, the `remove_id` from the preview), then add the new one. Prices must match the landing page or Google disapproves them. - `remove_entity` is IRREVERSIBLE — always prefer `pause_entity` unless the user explicitly wants permanent removal. Removal triggers double confirmation in the safety layer. diff --git a/src/adloop/server.py b/src/adloop/server.py index 5c62678..63d69c1 100644 --- a/src/adloop/server.py +++ b/src/adloop/server.py @@ -3859,11 +3859,14 @@ def draft_campaign( tracking_url_template: str | None = None, final_url_suffix: str | None = None, url_custom_parameters: _DictListOpt = None, + geo_exclude_ids: _StrListOpt = None, + ad_schedule: _DictListOpt = None, ) -> dict: """Draft a full campaign structure — returns a PREVIEW, does NOT create anything. Creates: CampaignBudget + Campaign (PAUSED) + AdGroup + optional Keywords - + geo targeting + language targeting. + + geo targeting + language targeting + optional geo exclusions and ad + schedule, all in one atomic request. Ads are NOT included — draft_responsive_search_ad adds them once the campaign exists. Only SEARCH campaigns can be created: Performance Max needs an asset group with all its assets in one request, and DISPLAY, @@ -3930,6 +3933,14 @@ def draft_campaign( url_custom_parameters: List of {"key": "grp", "value": "b2c"}, used as {_grp} in the template. Keys alphanumeric, max 8 characters; values up to 250. + geo_exclude_ids: Geo target constant IDs to exclude (negative location + criteria), e.g. a city inside a targeted state. May not overlap + geo_target_ids. + ad_schedule: Windows the campaign serves in: list of {day_of_week + (MONDAY..SUNDAY), start_hour 0-23, end_hour 0-24, start_minute, + end_minute (0, 15, 30 or 45)}, whole numbers, in the account time + zone. No overlaps, at most six a day. Outside them the campaign does + not serve, so the plan needs a double confirmation. """ from adloop.ads.write import draft_campaign as _impl @@ -3955,6 +3966,8 @@ def draft_campaign( tracking_url_template=tracking_url_template, final_url_suffix=final_url_suffix, url_custom_parameters=url_custom_parameters, + geo_exclude_ids=geo_exclude_ids, + ad_schedule=ad_schedule, ) @@ -4023,12 +4036,15 @@ def update_campaign( final_url_suffix: str | None = None, url_custom_parameters: _DictListOpt = None, keyword_match_type: str = "", + geo_exclude_ids: _StrListOpt = None, + ad_schedule: _DictListOpt = None, ) -> dict: """Draft an update to an existing campaign — returns a PREVIEW, does NOT apply. Only the parameters passed are changed; omitted ones stay as they are, and at least one change is required. Replacing geo targets keeps the campaign's - negative geo exclusions (the preview lists them). A budget change shows the + negative geo exclusions (the preview lists them) unless geo_exclude_ids is + passed too. A budget change shows the current amount; when the budget is shared with other campaigns, the preview names them and needs a double confirmation, because the new amount covers all of them. A bidding change on a campaign in a portfolio bid @@ -4098,6 +4114,17 @@ def update_campaign( non-Smart bidding_strategy is refused unless the same call passes UNSPECIFIED. The preview shows the current value. Empty leaves it unchanged. + geo_exclude_ids: REPLACES the campaign's negative location criteria. + Omitted leaves them unchanged; [] removes them. May not overlap + geo_target_ids. The preview shows the current ones + (current_geo_exclude_ids); dropping any needs a double confirmation, + because the campaign can serve there again. + ad_schedule: REPLACES the campaign's schedule windows (shape as in + draft_campaign). Omitted leaves them unchanged; [] removes them, so + the campaign serves at all hours. The preview shows the current ones + (current_ad_schedule). A first schedule and clearing a schedule both + need a double confirmation. Apply refuses if the exclusions or + windows changed after the preview. """ from adloop.ads.write import update_campaign as _impl @@ -4121,6 +4148,8 @@ def update_campaign( final_url_suffix=final_url_suffix, url_custom_parameters=url_custom_parameters, keyword_match_type=keyword_match_type, + geo_exclude_ids=geo_exclude_ids, + ad_schedule=ad_schedule, ) diff --git a/tests/test_campaign_geo_exclude_schedule.py b/tests/test_campaign_geo_exclude_schedule.py new file mode 100644 index 0000000..d0427f2 --- /dev/null +++ b/tests/test_campaign_geo_exclude_schedule.py @@ -0,0 +1,403 @@ +"""geo_exclude_ids and ad_schedule on draft_campaign and update_campaign. + +On create they go in the campaign's atomic mutate. On update they replace the +whole list: None leaves it, [] clears it, the preview shows the current values +and apply refuses if those moved. Plans run through +``confirm_and_apply(dry_run=True)`` with real protos and the real validate-only +wrapper; only the services are fakes. +""" + +from __future__ import annotations + +from types import SimpleNamespace + +import pytest +from google.ads.googleads.client import GoogleAdsClient + +from adloop.ads import write +from adloop.ads.client import GOOGLE_ADS_API_VERSION +from adloop.config import AdLoopConfig, AdsConfig, SafetyConfig +from adloop.safety import preview as preview_store + +CAMPAIGN = "1001" +MONDAY = {"day_of_week": "MONDAY", "start_hour": 8, "end_hour": 17} +TUESDAY = {"day_of_week": "TUESDAY", "start_hour": 9, "start_minute": 30, "end_hour": 12} + + +@pytest.fixture(autouse=True) +def _plans(): + preview_store.set_plan_store(preview_store.InMemoryPlanStore()) + yield + preview_store.set_plan_store(preview_store.InMemoryPlanStore()) + + +@pytest.fixture +def config(tmp_path) -> AdLoopConfig: + return AdLoopConfig( + ads=AdsConfig(customer_id="1234567890"), + safety=SafetyConfig(log_file=str(tmp_path / "audit.log"), max_daily_budget=500), + ) + + +class _Service: + def __init__(self, client, name): + self._client = client + self._name = name + + def search(self, *, customer_id, query): + if "campaign_criterion.type = 'AD_SCHEDULE'" in query: + return iter(self._client.window_rows) + if "campaign_criterion.negative = TRUE" in query: + return iter(self._client.exclude_rows) + return iter([]) + + def __getattr__(self, attr): + if attr.endswith("_path"): + noun = attr.removesuffix("_path") + plural = {"campaign": "campaigns", "ad_group": "adGroups", "campaign_budget": "campaignBudgets"}[noun] + return lambda cid, entity: f"customers/{cid}/{plural}/{entity}" + if attr.startswith("mutate"): + def send(request=None, **_kwargs): + self._client.sent.append(request) + return SimpleNamespace(results=[], mutate_operation_responses=[], partial_failure_error=None) + return send + raise AttributeError(attr) + + +class _Client: + """Real protos and enums; services record instead of reaching Google.""" + + def __init__(self): + base = GoogleAdsClient(credentials=None, developer_token="t", use_proto_plus=True, + version=GOOGLE_ADS_API_VERSION) + self.enums = base.enums + self.get_type = base.get_type + self.sent = [] + self.window_rows = [] + self.exclude_rows = [] + + def get_service(self, name, *_args, **_kwargs): + return _Service(self, name) + + def operations(self): + assert len(self.sent) == 1 and self.sent[0].validate_only is True + return list(self.sent[0].mutate_operations) + + def removed(self): + return [op.campaign_criterion_operation.remove for op in self.operations() + if op.campaign_criterion_operation.remove] + + def created(self): + return [op.campaign_criterion_operation.create for op in self.operations() + if not op.campaign_criterion_operation.remove and op.campaign_criterion_operation.create.campaign] + + +@pytest.fixture +def client(monkeypatch): + fake = _Client() + monkeypatch.setattr("adloop.ads.client.get_ads_client", lambda _cfg: fake) + # conftest stubs this out; these tests are about the real path. + monkeypatch.setattr( + write, "_validate_with_google", + lambda cfg, plan: write._execute_plan(cfg, plan, validate_only=True), + ) + return fake + + +def _window_row(criterion_id, **window): + return { + "campaign_criterion.criterion_id": criterion_id, + "campaign_criterion.ad_schedule.day_of_week": window["day_of_week"], + "campaign_criterion.ad_schedule.start_hour": window["start_hour"], + "campaign_criterion.ad_schedule.start_minute": window.get("start_minute", "ZERO"), + "campaign_criterion.ad_schedule.end_hour": window["end_hour"], + "campaign_criterion.ad_schedule.end_minute": window.get("end_minute", "ZERO"), + } + + +def _account(monkeypatch, *, windows=(), excludes=(), status="ENABLED", found=True): + """GAQL answers for the time zone, the campaign, its windows and exclusions.""" + def execute_query(_config, _cid, query): + if "FROM customer" in query: + return [{"customer.currency_code": "USD", "customer.time_zone": "America/Chicago"}] + if "campaign_criterion.type = 'AD_SCHEDULE'" in query: + return list(windows) + if "campaign_criterion.negative = TRUE" in query: + return [{"campaign_criterion.location.geo_target_constant": f"geoTargetConstants/{g}"} for g in excludes] + if "FROM campaign WHERE campaign.id" in query: + if not found: + return [] + return [{"campaign.name": "Brand", "campaign.status": status, "customer.time_zone": "America/Chicago"}] + raise AssertionError(f"unexpected query: {query}") + + monkeypatch.setattr("adloop.ads.gaql.execute_query", execute_query) + + +def _draft(config, **kwargs): + return write.draft_campaign( + config, customer_id="1234567890", campaign_name="Search", daily_budget=20, + bidding_strategy="MAXIMIZE_CONVERSIONS", geo_target_ids=["21137"], language_ids=["1000"], **kwargs, + ) + + +def _update(config, **kwargs): + return write.update_campaign(config, customer_id="1234567890", campaign_id=CAMPAIGN, **kwargs) + + +def _dry_run(config, preview): + assert "plan_id" in preview, preview + return write.confirm_and_apply(config, plan_id=preview["plan_id"], dry_run=True) + + +def _apply_check(config, preview): + result = _dry_run(config, preview) + assert result["status"] == "DRY_RUN_SUCCESS", result + return result + + +class TestDraftCampaign: + def test_both_go_in_the_one_atomic_mutate(self, config, client, monkeypatch): + _account(monkeypatch) + preview = _draft(config, geo_exclude_ids=["1014962"], ad_schedule=[MONDAY]) + assert preview["changes"]["geo_exclude_ids"] == ["1014962"] + _apply_check(config, preview) + + criteria = client.created() + negative = [c for c in criteria if c.negative] + assert [c.location.geo_target_constant for c in negative] == ["geoTargetConstants/1014962"] + assert negative[0].campaign == "customers/1234567890/campaigns/-2" + windows = [c for c in criteria if c.ad_schedule.start_hour] + assert windows[0].campaign == "customers/1234567890/campaigns/-2" + assert (windows[0].ad_schedule.start_hour, windows[0].ad_schedule.end_hour) == (8, 17) + assert windows[0].ad_schedule.day_of_week == client.enums.DayOfWeekEnum.MONDAY + + def test_a_schedule_is_a_first_schedule(self, config, client, monkeypatch): + _account(monkeypatch) + preview = _draft(config, ad_schedule=[MONDAY]) + assert preview["requires_double_confirm"] is True + assert preview["changes"]["account_time_zone"] == "America/Chicago" + assert any("account time zone (America/Chicago)" in w for w in preview["warnings"]) + assert any("serves only inside them" in w for w in preview["warnings"]) + + def test_exclusions_alone_need_no_double_confirmation_or_read(self, config, client): + # no_live_gaql from conftest fails this test if the draft queries anything. + preview = _draft(config, geo_exclude_ids=["1014962"]) + assert preview["requires_double_confirm"] is False + + def test_without_them_the_plan_is_unchanged(self, config, client): + preview = _draft(config) + assert "geo_exclude_ids" not in preview["changes"] + assert "ad_schedule" not in preview["changes"] + assert preview["requires_double_confirm"] is False + + def test_schedule_rules_match_add_ad_schedule(self, config): + overlapping = [MONDAY, {"day_of_week": "MONDAY", "start_hour": 16, "end_hour": 20}] + result = _draft(config, ad_schedule=overlapping) + assert any("overlaps" in d for d in result["details"]) + result = _draft(config, ad_schedule=[{"day_of_week": "MONDAY", "start_hour": 8.5, "end_hour": 17}]) + assert any("whole numbers" in d for d in result["details"]) + seven = [{"day_of_week": "MONDAY", "start_hour": h, "end_hour": h + 1} for h in range(7)] + result = _draft(config, ad_schedule=seven) + assert any("at most 6" in d for d in result["details"]) + + def test_exclusions_are_checked(self, config): + result = _draft(config, geo_exclude_ids=["21137"]) + assert any("overlap geo_target_ids" in d for d in result["details"]) + result = _draft(config, geo_exclude_ids=["Los Angeles"]) + assert any("must be numeric" in d for d in result["details"]) + + def test_results_are_sorted_by_kind(self, monkeypatch): + window = {"day_of_week": "MONDAY", "start_hour": 8, "start_minute": 0, "end_hour": 17, "end_minute": 0} + + class _Mutating(_Service): + def mutate(self, *, customer_id, mutate_operations): + return SimpleNamespace(mutate_operation_responses=[f"r{i}" for i in range(len(mutate_operations))]) + + fake = _Client() + fake.get_service = lambda name, *_a, **_k: _Mutating(fake, name) + monkeypatch.setattr(write, "_extract_resource_name", lambda response: response) + results = write._apply_create_campaign(fake, "1", { + "campaign_name": "C", "daily_budget": 1, "bidding_strategy": "MAXIMIZE_CONVERSIONS", + "channel_type": "SEARCH", "ad_group_name": "A", "geo_target_ids": ["1"], "language_ids": ["2"], + "geo_exclude_ids": ["3"], "ad_schedule": [window], + }) + assert results == { + "campaign_budget": "r0", "campaign": "r1", "ad_group": "r2", "geo_targets": ["r3"], + "language_targets": ["r4"], "geo_excludes": ["r5"], "ad_schedule": ["r6"], + } + + +class TestUpdateGeoExcludes: + def test_replaces_and_only_touches_what_differs(self, config, client, monkeypatch): + _account(monkeypatch, excludes=["111", "222"]) + client.exclude_rows = [_exclude_row("111"), _exclude_row("222")] + preview = _update(config, geo_exclude_ids=["222", "333"]) + assert preview["changes"]["current_geo_exclude_ids"] == ["111", "222"] + assert preview["changes"]["geo_exclude_ids"] == ["222", "333"] + assert preview["requires_double_confirm"] is True + assert any("['111']" in w and "serve in those locations again" in w for w in preview["warnings"]) + + _apply_check(config, preview) + assert client.removed() == [f"customers/1/campaignCriteria/{CAMPAIGN}~111"] + added = client.created() + assert [c.location.geo_target_constant for c in added] == ["geoTargetConstants/333"] + assert added[0].negative is True + + def test_empty_list_clears_them(self, config, client, monkeypatch): + _account(monkeypatch, excludes=["111"]) + client.exclude_rows = [_exclude_row("111")] + preview = _update(config, geo_exclude_ids=[]) + assert preview["requires_double_confirm"] is True + _apply_check(config, preview) + assert client.removed() == [f"customers/1/campaignCriteria/{CAMPAIGN}~111"] + assert client.created() == [] + + def test_adding_only_needs_no_double_confirmation(self, config, client, monkeypatch): + _account(monkeypatch) + preview = _update(config, geo_exclude_ids=["333"]) + assert preview["requires_double_confirm"] is False + assert "warnings" not in preview + + def test_blank_entries_are_refused_instead_of_clearing(self, config, client, monkeypatch): + _account(monkeypatch, excludes=["111"]) + result = _update(config, geo_exclude_ids=[" "]) + assert any("must be numeric" in d for d in result["details"]) + + def test_overlap_with_new_targets_is_refused(self, config): + result = _update(config, geo_target_ids=["2840"], geo_exclude_ids=["2840"]) + assert any("overlap geo_target_ids" in d for d in result["details"]) + + def test_same_list_is_nothing_to_change(self, config, monkeypatch): + _account(monkeypatch, excludes=["111"]) + result = _update(config, geo_exclude_ids=["111"]) + assert "nothing to change" in result["error"] + + def test_apply_refuses_when_they_changed_after_the_preview(self, config, client, monkeypatch): + _account(monkeypatch, excludes=["111"]) + client.exclude_rows = [_exclude_row("111"), _exclude_row("999")] + result = _dry_run(config, _update(config, geo_exclude_ids=["333"])) + assert result["status"] == "DRY_RUN_FAILED" + assert "changed after this preview" in str(result) + assert client.sent == [] + + def test_replacing_positive_geos_no_longer_promises_to_preserve_them(self, config, client, monkeypatch): + _account(monkeypatch, excludes=["111"]) + client.exclude_rows = [_exclude_row("111")] + preview = _update(config, geo_target_ids=["2840"], geo_exclude_ids=[]) + assert "preserved_negative_geo_target_ids" not in preview + assert not any("PRESERVED" in w for w in preview["warnings"]) + + def test_moving_a_location_from_excluded_to_targeted_removes_it_first(self, config, client, monkeypatch): + _account(monkeypatch, excludes=["111"]) + client.exclude_rows = [_exclude_row("111")] + _apply_check(config, _update(config, geo_target_ids=["111"], geo_exclude_ids=[])) + ops = client.operations() + assert ops[0].campaign_criterion_operation.remove.endswith("~111") + assert ops[1].campaign_criterion_operation.create.location.geo_target_constant == "geoTargetConstants/111" + + def test_omitted_leaves_them_preserved(self, config, monkeypatch): + _account(monkeypatch, excludes=["111"]) + preview = _update(config, geo_target_ids=["2840"]) + assert preview["preserved_negative_geo_target_ids"] == ["111"] + assert "geo_exclude_ids" not in preview["changes"] + + +def _exclude_row(geo_id): + return SimpleNamespace(campaign_criterion=SimpleNamespace( + resource_name=f"customers/1/campaignCriteria/{CAMPAIGN}~{geo_id}", + location=SimpleNamespace(geo_target_constant=f"geoTargetConstants/{geo_id}"), + )) + + +def _criterion_row(criterion_id): + return SimpleNamespace(campaign_criterion=SimpleNamespace( + resource_name=f"customers/1/campaignCriteria/{CAMPAIGN}~{criterion_id}", + criterion_id=criterion_id, + )) + + +class TestUpdateAdSchedule: + def test_first_schedule_needs_a_double_confirmation_like_add_ad_schedule(self, config, client, monkeypatch): + _account(monkeypatch) + preview = _update(config, ad_schedule=[MONDAY]) + assert preview["requires_double_confirm"] is True + assert any("'Brand' has no ad schedule" in w and "stops serving" in w for w in preview["warnings"]) + assert any("account time zone (America/Chicago)" in w for w in preview["warnings"]) + assert preview["changes"]["current_ad_schedule"] == [] + _apply_check(config, preview) + assert client.removed() == [] + assert client.created()[0].ad_schedule.start_hour == 8 + + def test_clearing_is_flagged_and_needs_a_double_confirmation(self, config, client, monkeypatch): + _account(monkeypatch, windows=[_window_row(5, **MONDAY)]) + client.window_rows = [_criterion_row(5)] + preview = _update(config, ad_schedule=[]) + assert preview["requires_double_confirm"] is True + assert any("serves at all hours" in w for w in preview["warnings"]) + assert preview["changes"]["current_ad_schedule"][0]["day_of_week"] == "MONDAY" + _apply_check(config, preview) + assert client.removed() == [f"customers/1/campaignCriteria/{CAMPAIGN}~5"] + assert client.created() == [] + + def test_replacing_keeps_identical_windows_and_shows_the_current_ones(self, config, client, monkeypatch): + _account(monkeypatch, windows=[_window_row(5, **MONDAY), _window_row(6, **{**TUESDAY, "start_minute": "THIRTY"})]) + client.window_rows = [_criterion_row(5), _criterion_row(6)] + new = {"day_of_week": "FRIDAY", "start_hour": 10, "end_hour": 14} + preview = _update(config, ad_schedule=[MONDAY, new]) + assert preview["requires_double_confirm"] is False + assert any("Replaces the 2 current ad schedule windows" in w for w in preview["warnings"]) + assert len(preview["changes"]["current_ad_schedule"]) == 2 + _apply_check(config, preview) + assert client.removed() == [f"customers/1/campaignCriteria/{CAMPAIGN}~6"] + assert [c.ad_schedule.day_of_week for c in client.created()] == [client.enums.DayOfWeekEnum.FRIDAY] + + def test_a_replacement_uses_the_same_window_rules(self, config): + result = _update(config, ad_schedule=[{"day_of_week": "SOMEDAY", "start_hour": 8, "end_hour": 17}]) + assert any("day_of_week must be one of" in d for d in result["details"]) + + def test_same_windows_are_nothing_to_change(self, config, monkeypatch): + _account(monkeypatch, windows=[_window_row(5, **MONDAY)]) + assert "nothing to change" in _update(config, ad_schedule=[MONDAY])["error"] + + def test_clearing_nothing_is_nothing_to_change(self, config, monkeypatch): + _account(monkeypatch) + assert "nothing to change" in _update(config, ad_schedule=[])["error"] + + def test_a_no_op_next_to_another_change_says_so(self, config, monkeypatch): + _account(monkeypatch) + preview = _update(config, ad_schedule=[], geo_exclude_ids=["333"]) + assert "ad_schedule" not in preview["changes"] + assert any("already has these windows" in w for w in preview["warnings"]) + + def test_apply_refuses_when_the_windows_changed_after_the_preview(self, config, client, monkeypatch): + _account(monkeypatch, windows=[_window_row(5, **MONDAY)]) + client.window_rows = [] # someone removed the window: these would now be a first schedule + result = _dry_run(config, _update(config, ad_schedule=[TUESDAY])) + assert result["status"] == "DRY_RUN_FAILED" + assert "changed after this preview" in str(result) + assert client.sent == [] + + def test_paused_campaign_says_when_it_applies(self, config, monkeypatch): + _account(monkeypatch, status="PAUSED") + preview = _update(config, ad_schedule=[MONDAY]) + assert any("The campaign is PAUSED" in w for w in preview["warnings"]) + + @pytest.mark.parametrize("kwargs", [{"ad_schedule": [MONDAY]}, {"geo_exclude_ids": ["333"]}]) + def test_unknown_and_removed_campaigns_are_refused(self, config, monkeypatch, kwargs): + _account(monkeypatch, found=False) + assert "was not found" in _update(config, **kwargs)["error"] + _account(monkeypatch, status="REMOVED") + assert "is removed" in _update(config, **kwargs)["error"] + + +@pytest.mark.asyncio +async def test_the_mcp_tools_take_both_parameters(): + from adloop.server import mcp + + tools = {tool.name: tool for tool in await mcp.list_tools()} + for name in ("draft_campaign", "update_campaign"): + properties = tools[name].parameters["properties"] + for param in ("geo_exclude_ids", "ad_schedule"): + assert properties[param].get("description"), (name, param) + assert not {"geo_exclude_ids", "ad_schedule"} & set(tools[name].parameters.get("required", [])) + assert "REPLACES" in tools["update_campaign"].parameters["properties"]["ad_schedule"]["description"] From 776754c00dd285b709afe876ade75b93ccd1cf68 Mon Sep 17 00:00:00 2001 From: Illia Sapryga Date: Tue, 10 Nov 2026 15:56:04 -0600 Subject: [PATCH 2/2] draft_campaign: a schedule warns instead of a double confirmation The campaign is created PAUSED, so a schedule stops nothing from serving, and the dialog's "cannot be undone" was wrong there. The preview now says the campaign serves only inside the windows once it is enabled. update_campaign keeps the double confirmation for a first schedule, for clearing a schedule and for dropping an exclusion. Tests pin the order of the one update_campaign request: campaign fields, exclusion removes, positive geo removes and creates, languages, exclusion creates, schedule removes, then schedule creates. A location moving from the exclusions to the targets, or back, is removed before it is created again, and keyword_match_type (#117) can go in the same request. The GA4 annotation suggested after an applied update_campaign now counts exclusions as targeting and names a schedule change. The rules and tool descriptions describe the final behavior. --- .claude/rules/adloop.md | 8 +- .cursor/rules/adloop.mdc | 8 +- src/adloop/ads/write.py | 22 ++-- src/adloop/ga4/suggest.py | 6 +- src/adloop/rules/adloop.md | 8 +- src/adloop/server.py | 24 ++-- tests/test_campaign_geo_exclude_schedule.py | 138 +++++++++++++++++--- tests/test_ga4_suggest.py | 5 + 8 files changed, 168 insertions(+), 51 deletions(-) diff --git a/.claude/rules/adloop.md b/.claude/rules/adloop.md index 8094c46..4325083 100644 --- a/.claude/rules/adloop.md +++ b/.claude/rules/adloop.md @@ -377,7 +377,7 @@ Conversions that happen offline (a sales call, a lead that turned into a deal) g | `draft_asset_group_signals` | Add search themes or set the audience signal of a PMax asset group, or remove signals | `asset_group_id`, `add_search_themes` (≤80 chars, up to 50 per group), `audience_id` (one per group), `remove_signal_ids` | | `draft_pmax_product_exclusions` | Set the full list of products a retail PMax asset group excludes, on one dimension; everything else stays included | `asset_group_id`, `exclude_values` (the COMPLETE list; `[]` includes everything again), `dimension` item_id (default) / brand / custom_label_0–4 | | `draft_ad_group` | Create a new ad group within an existing campaign (does NOT publish) | `campaign_id` (REQUIRED), `ad_group_name` (REQUIRED), `keywords` (optional list of {text, match_type}), `cpc_bid_micros` (optional) | -| `update_campaign` | Modify existing campaign settings — bid strategy, budget, geo targets, language targets, location options, Search partners, display expansion, URL options, campaign-level broad match | `campaign_id` (REQUIRED), plus any of: `bidding_strategy`, `daily_budget`, `geo_target_ids`, `language_ids`, `location_option`, `negative_location_option`, `search_partners_enabled`, `display_network_enabled`, TARGET_SPEND `max_cpc`, `tracking_url_template`, `final_url_suffix`, `url_custom_parameters`, `keyword_match_type`, `geo_exclude_ids`, `ad_schedule` | +| `update_campaign` | Modify existing campaign settings — bid strategy, budget, geo targets, geo exclusions, language targets, ad schedule, location options, Search partners, display expansion, URL options, campaign-level broad match | `campaign_id` (REQUIRED), plus any of: `bidding_strategy`, `daily_budget`, `geo_target_ids`, `language_ids`, `location_option`, `negative_location_option`, `search_partners_enabled`, `display_network_enabled`, TARGET_SPEND `max_cpc`, `tracking_url_template`, `final_url_suffix`, `url_custom_parameters`, `keyword_match_type`, `geo_exclude_ids`, `ad_schedule` | | `update_ad_group` | Update ad group name and/or MANUAL_CPC `max_cpc` | `ad_group_id`, optional `ad_group_name`, optional `max_cpc` | | `draft_responsive_search_ad` | Create RSA preview (does NOT publish) | 3-15 headlines (≤30 chars), 2-4 descriptions (≤90 chars), final_url required, path1/path2 (≤15 chars each). Each headline/description may be a plain string (unpinned) or `{"text": "...", "pinned_field": "HEADLINE_1"}` (pinned). Valid pin slots: `HEADLINE_1/2/3`, `DESCRIPTION_1/2`. Several assets may pin to one slot (they rotate there), but every slot must stay fillable: by an asset pinned to it or by a distinct unpinned asset. | | `draft_callouts` | Create callout assets for an ad group, a campaign or the account (does NOT publish) | `campaign_id` or `ad_group_id` (the scope follows the id), or `scope="account"`; `callouts` list with 1-25 chars each | @@ -463,10 +463,10 @@ Same draft → preview → `confirm_and_apply` gate; the plan carries `platform: - Location options: `location_option` is who the geo targets reach — `PRESENCE` (people in or regularly in the locations), `PRESENCE_OR_INTEREST` (Google's default; also people elsewhere who show interest in them) or `SEARCH_INTEREST`. `negative_location_option` (`PRESENCE` or `PRESENCE_OR_INTEREST`) is who the exclusions exclude. Empty leaves the value as it is. The preview warns when a change widens reach: for `location_option` that is anything but `PRESENCE`, for `negative_location_option` it is `PRESENCE`, because then the exclusions only keep out people physically there, not people interested in the excluded places; `update_campaign` shows the current value as `current_location_option` / `current_negative_location_option`. Read back with `get_campaign_settings`. - Campaign URL options (`tracking_url_template`, `final_url_suffix`, `url_custom_parameters`): omitted leaves a value unchanged, `""` or `[]` removes it, and `url_custom_parameters` replaces the whole list. The template must contain `{lpurl}` or a variant (`{lpurl+2}`, `{unescapedlpurl}`, `{escapedlpurl+3}`, ...). Custom parameter keys are alphanumeric, at most 8 characters. The `update_campaign` preview shows each current value (`current_tracking_url_template` etc.), warns on a removal, and gives `example_click`: one of the campaign's final URLs with the campaign's options applied. It is approximate: a template on the ad group, ad or keyword overrides the campaign's, and ValueTrack parameters are filled at click time. Check it for typos before approving. Read back with `get_campaign_settings`. - Campaign-level broad match: `update_campaign` `keyword_match_type="BROAD"` CONVERTS every existing phrase and exact keyword in the campaign into broad match. Google shows the originals as Removed, turning the setting off does not restore them, and since September 2026 Google also upgrades such campaigns to AI Max. It is refused unless the campaign (or the `bidding_strategy` in the same call) is Smart Bidding — the same rule as BROAD keywords — checked again at apply, and needs a double confirmation. `keyword_match_type="UNSPECIFIED"` turns it off (single confirmation): no further conversions, but converted keywords stay broad and must be re-added as phrase/exact by hand. Moving a broad-match campaign to a non-Smart `bidding_strategy` is refused unless the same call passes `UNSPECIFIED`. Never turn it on to "get more traffic" on an exact/phrase campaign; read it back with `get_campaign_settings`, which lists it in `widening_settings`. -- `update_campaign` replaces POSITIVE geo/language targets entirely (not append). Pass the full desired list. NEGATIVE geo exclusions (criteria with `negative=TRUE`) are **preserved** across a positive-geo replacement — they survive the swap. The preview surfaces preserved negative geo IDs in `preserved_negative_geo_target_ids` so the change is auditable. To ADD negative geo exclusions (e.g. exclude a city inside a targeted state), use `add_negative_locations` — do not try to express exclusions through `geo_target_ids`. To remove a negative geo exclusion explicitly, use `remove_entity` with `entity_type="campaign_criterion"`. -- `geo_exclude_ids` and `ad_schedule` set exclusions and windows when creating a campaign, in the same atomic request. On `update_campaign` they REPLACE the whole list: omitted leaves it unchanged, `[]` clears it. Use them to set the complete state; `add_negative_locations` and `add_ad_schedule` add to what is there. The preview shows the current values (`current_geo_exclude_ids`, `current_ad_schedule`) and apply refuses if they changed after it. Double confirmation is needed for a campaign's first schedule (also on create), for clearing a schedule (the campaign serves at all hours again) and for dropping an exclusion (it can serve there again). +- `update_campaign` replaces POSITIVE geo/language targets entirely (not append). Pass the full desired list. NEGATIVE geo exclusions (criteria with `negative=TRUE`) are **preserved** across a positive-geo replacement — they survive the swap — unless `geo_exclude_ids` is passed in the same call. The preview surfaces preserved negative geo IDs in `preserved_negative_geo_target_ids` so the change is auditable. To ADD negative geo exclusions (e.g. exclude a city inside a targeted state), use `add_negative_locations` — do not try to express exclusions through `geo_target_ids`. To remove a negative geo exclusion explicitly, use `remove_entity` with `entity_type="campaign_criterion"`, or pass `geo_exclude_ids` without it. +- `geo_exclude_ids` and `ad_schedule` set exclusions and windows when creating a campaign, in the same atomic request. A schedule there gets a warning, not a double confirmation: the campaign is created PAUSED, and once it is enabled it serves only inside the windows. On `update_campaign` they REPLACE the whole list: omitted leaves it unchanged, `[]` clears it. Use them to set the complete state; `add_negative_locations` and `add_ad_schedule` add to what is there. The preview shows the current values (`current_geo_exclude_ids`, `current_ad_schedule`), only the entries that differ are removed or added, and apply refuses if the current values changed after the preview. `update_campaign` needs a double confirmation for a campaign's first schedule (every other hour stops serving), for clearing a schedule (the campaign serves at all hours again) and for dropping an exclusion (it can serve there again). A location can move between `geo_target_ids` and `geo_exclude_ids` in one call; a location in both lists is refused. - **Asset scope:** callouts and snippets link where the id points (`ad_group_id` → ad group, `campaign_id` → campaign). Account-wide (`scope="account"`) reaches every campaign without assets of its own; use it only when the user asks for account level. **Asset edits** (`update_callout`, `update_sitelink`, `update_structured_snippet`) change the asset everywhere it is linked; check with GAQL (`campaign_asset`, `ad_group_asset`, `customer_asset`) where else it is used before editing a shared asset. If the apply reports the asset changed in Google Ads since the preview, draft again. -- **Call assets, schedules, promotions and prices.** Never guess `country_code` or `language_code`: take them from the number and the landing page, and ask when unclear. Money amounts are in the account currency (the preview names it); pass `currency_code` only for a different one. Ad schedule hours are in the **account time zone** (the previews show it), not the searcher's: convert the user's local hours first. A campaign without a schedule serves at all hours, so its first `add_ad_schedule` windows switch off every other hour and day: say so, the preview needs a double confirmation. To change a window, remove the old one with `remove_entity` (`entity_type="campaign_criterion"`, the `remove_id` from the preview), then add the new one. Prices must match the landing page or Google disapproves them. +- **Call assets, schedules, promotions and prices.** Never guess `country_code` or `language_code`: take them from the number and the landing page, and ask when unclear. Money amounts are in the account currency (the preview names it); pass `currency_code` only for a different one. Ad schedule hours are in the **account time zone** (the previews show it), not the searcher's: convert the user's local hours first. A campaign without a schedule serves at all hours, so its first `add_ad_schedule` windows switch off every other hour and day: say so, the preview needs a double confirmation. To change a window, pass the complete list as `update_campaign` `ad_schedule`, or remove the old one with `remove_entity` (`entity_type="campaign_criterion"`, the `remove_id` from the preview) and add the new one. Prices must match the landing page or Google disapproves them. - `remove_entity` is IRREVERSIBLE — always prefer `pause_entity` unless the user explicitly wants permanent removal. Removal triggers double confirmation in the safety layer. - `remove_entity` supports `entity_type` values: "campaign", "ad_group", "ad", "keyword", "negative_keyword", "shared_criterion", "ad_group_criterion", "campaign_criterion", "campaign_asset", "ad_group_asset", "asset", "customer_asset". Use "negative_keyword" to remove campaign-level negative keywords. Use "shared_criterion" to remove a keyword from a shared negative keyword list — the `entity_id` format is "sharedSetId~criterionId" (use the `resource_id` field from `get_negative_keyword_list_keywords`). Use "ad_group_criterion" or "campaign_criterion" to remove demographic targeting (age/gender/parental/income) or an attached audience or audience exclusion — pass the `remove_id` returned by `get_demographic_targeting` or `get_audience_targeting`. Use "campaign_asset" to remove sitelinks and other asset links from a campaign, "ad_group_asset" (`adGroupId~assetId~fieldType`) for links on an ad group. Use "asset" to remove a standalone asset. Use "customer_asset" to remove a customer-level asset link. - A Google Ads `dry_run=true` sends the exact change to Google with `validate_only` — Google checks it and executes nothing. `DRY_RUN_FAILED` carries Google's error and means the real apply would fail the same way; fix the cause and draft again. Steps that build on an object an earlier step would create (e.g. keywords for a new negative list) are counted in `checks.skipped_calls` instead of validated. diff --git a/.cursor/rules/adloop.mdc b/.cursor/rules/adloop.mdc index e13d757..022a516 100644 --- a/.cursor/rules/adloop.mdc +++ b/.cursor/rules/adloop.mdc @@ -379,7 +379,7 @@ Conversions that happen offline (a sales call, a lead that turned into a deal) g | `draft_asset_group_signals` | Add search themes or set the audience signal of a PMax asset group, or remove signals | `asset_group_id`, `add_search_themes` (≤80 chars, up to 50 per group), `audience_id` (one per group), `remove_signal_ids` | | `draft_pmax_product_exclusions` | Set the full list of products a retail PMax asset group excludes, on one dimension; everything else stays included | `asset_group_id`, `exclude_values` (the COMPLETE list; `[]` includes everything again), `dimension` item_id (default) / brand / custom_label_0–4 | | `draft_ad_group` | Create a new ad group within an existing campaign (does NOT publish) | `campaign_id` (REQUIRED), `ad_group_name` (REQUIRED), `keywords` (optional list of {text, match_type}), `cpc_bid_micros` (optional) | -| `update_campaign` | Modify existing campaign settings — bid strategy, budget, geo targets, language targets, location options, Search partners, display expansion, URL options, campaign-level broad match | `campaign_id` (REQUIRED), plus any of: `bidding_strategy`, `daily_budget`, `geo_target_ids`, `language_ids`, `location_option`, `negative_location_option`, `search_partners_enabled`, `display_network_enabled`, TARGET_SPEND `max_cpc`, `tracking_url_template`, `final_url_suffix`, `url_custom_parameters`, `keyword_match_type`, `geo_exclude_ids`, `ad_schedule` | +| `update_campaign` | Modify existing campaign settings — bid strategy, budget, geo targets, geo exclusions, language targets, ad schedule, location options, Search partners, display expansion, URL options, campaign-level broad match | `campaign_id` (REQUIRED), plus any of: `bidding_strategy`, `daily_budget`, `geo_target_ids`, `language_ids`, `location_option`, `negative_location_option`, `search_partners_enabled`, `display_network_enabled`, TARGET_SPEND `max_cpc`, `tracking_url_template`, `final_url_suffix`, `url_custom_parameters`, `keyword_match_type`, `geo_exclude_ids`, `ad_schedule` | | `update_ad_group` | Update ad group name and/or MANUAL_CPC `max_cpc` | `ad_group_id`, optional `ad_group_name`, optional `max_cpc` | | `draft_responsive_search_ad` | Create RSA preview (does NOT publish) | 3-15 headlines (≤30 chars), 2-4 descriptions (≤90 chars), final_url required, path1/path2 (≤15 chars each). Each headline/description may be a plain string (unpinned) or `{"text": "...", "pinned_field": "HEADLINE_1"}` (pinned). Valid pin slots: `HEADLINE_1/2/3`, `DESCRIPTION_1/2`. Several assets may pin to one slot (they rotate there), but every slot must stay fillable: by an asset pinned to it or by a distinct unpinned asset. | | `draft_callouts` | Create callout assets for an ad group, a campaign or the account (does NOT publish) | `campaign_id` or `ad_group_id` (the scope follows the id), or `scope="account"`; `callouts` list with 1-25 chars each | @@ -465,10 +465,10 @@ Same draft → preview → `confirm_and_apply` gate; the plan carries `platform: - Location options: `location_option` is who the geo targets reach — `PRESENCE` (people in or regularly in the locations), `PRESENCE_OR_INTEREST` (Google's default; also people elsewhere who show interest in them) or `SEARCH_INTEREST`. `negative_location_option` (`PRESENCE` or `PRESENCE_OR_INTEREST`) is who the exclusions exclude. Empty leaves the value as it is. The preview warns when a change widens reach: for `location_option` that is anything but `PRESENCE`, for `negative_location_option` it is `PRESENCE`, because then the exclusions only keep out people physically there, not people interested in the excluded places; `update_campaign` shows the current value as `current_location_option` / `current_negative_location_option`. Read back with `get_campaign_settings`. - Campaign URL options (`tracking_url_template`, `final_url_suffix`, `url_custom_parameters`): omitted leaves a value unchanged, `""` or `[]` removes it, and `url_custom_parameters` replaces the whole list. The template must contain `{lpurl}` or a variant (`{lpurl+2}`, `{unescapedlpurl}`, `{escapedlpurl+3}`, ...). Custom parameter keys are alphanumeric, at most 8 characters. The `update_campaign` preview shows each current value (`current_tracking_url_template` etc.), warns on a removal, and gives `example_click`: one of the campaign's final URLs with the campaign's options applied. It is approximate: a template on the ad group, ad or keyword overrides the campaign's, and ValueTrack parameters are filled at click time. Check it for typos before approving. Read back with `get_campaign_settings`. - Campaign-level broad match: `update_campaign` `keyword_match_type="BROAD"` CONVERTS every existing phrase and exact keyword in the campaign into broad match. Google shows the originals as Removed, turning the setting off does not restore them, and since September 2026 Google also upgrades such campaigns to AI Max. It is refused unless the campaign (or the `bidding_strategy` in the same call) is Smart Bidding — the same rule as BROAD keywords — checked again at apply, and needs a double confirmation. `keyword_match_type="UNSPECIFIED"` turns it off (single confirmation): no further conversions, but converted keywords stay broad and must be re-added as phrase/exact by hand. Moving a broad-match campaign to a non-Smart `bidding_strategy` is refused unless the same call passes `UNSPECIFIED`. Never turn it on to "get more traffic" on an exact/phrase campaign; read it back with `get_campaign_settings`, which lists it in `widening_settings`. -- `update_campaign` replaces POSITIVE geo/language targets entirely (not append). Pass the full desired list. NEGATIVE geo exclusions (criteria with `negative=TRUE`) are **preserved** across a positive-geo replacement — they survive the swap. The preview surfaces preserved negative geo IDs in `preserved_negative_geo_target_ids` so the change is auditable. To ADD negative geo exclusions (e.g. exclude a city inside a targeted state), use `add_negative_locations` — do not try to express exclusions through `geo_target_ids`. To remove a negative geo exclusion explicitly, use `remove_entity` with `entity_type="campaign_criterion"`. -- `geo_exclude_ids` and `ad_schedule` set exclusions and windows when creating a campaign, in the same atomic request. On `update_campaign` they REPLACE the whole list: omitted leaves it unchanged, `[]` clears it. Use them to set the complete state; `add_negative_locations` and `add_ad_schedule` add to what is there. The preview shows the current values (`current_geo_exclude_ids`, `current_ad_schedule`) and apply refuses if they changed after it. Double confirmation is needed for a campaign's first schedule (also on create), for clearing a schedule (the campaign serves at all hours again) and for dropping an exclusion (it can serve there again). +- `update_campaign` replaces POSITIVE geo/language targets entirely (not append). Pass the full desired list. NEGATIVE geo exclusions (criteria with `negative=TRUE`) are **preserved** across a positive-geo replacement — they survive the swap — unless `geo_exclude_ids` is passed in the same call. The preview surfaces preserved negative geo IDs in `preserved_negative_geo_target_ids` so the change is auditable. To ADD negative geo exclusions (e.g. exclude a city inside a targeted state), use `add_negative_locations` — do not try to express exclusions through `geo_target_ids`. To remove a negative geo exclusion explicitly, use `remove_entity` with `entity_type="campaign_criterion"`, or pass `geo_exclude_ids` without it. +- `geo_exclude_ids` and `ad_schedule` set exclusions and windows when creating a campaign, in the same atomic request. A schedule there gets a warning, not a double confirmation: the campaign is created PAUSED, and once it is enabled it serves only inside the windows. On `update_campaign` they REPLACE the whole list: omitted leaves it unchanged, `[]` clears it. Use them to set the complete state; `add_negative_locations` and `add_ad_schedule` add to what is there. The preview shows the current values (`current_geo_exclude_ids`, `current_ad_schedule`), only the entries that differ are removed or added, and apply refuses if the current values changed after the preview. `update_campaign` needs a double confirmation for a campaign's first schedule (every other hour stops serving), for clearing a schedule (the campaign serves at all hours again) and for dropping an exclusion (it can serve there again). A location can move between `geo_target_ids` and `geo_exclude_ids` in one call; a location in both lists is refused. - **Asset scope:** callouts and snippets link where the id points (`ad_group_id` → ad group, `campaign_id` → campaign). Account-wide (`scope="account"`) reaches every campaign without assets of its own; use it only when the user asks for account level. **Asset edits** (`update_callout`, `update_sitelink`, `update_structured_snippet`) change the asset everywhere it is linked; check with GAQL (`campaign_asset`, `ad_group_asset`, `customer_asset`) where else it is used before editing a shared asset. If the apply reports the asset changed in Google Ads since the preview, draft again. -- **Call assets, schedules, promotions and prices.** Never guess `country_code` or `language_code`: take them from the number and the landing page, and ask when unclear. Money amounts are in the account currency (the preview names it); pass `currency_code` only for a different one. Ad schedule hours are in the **account time zone** (the previews show it), not the searcher's: convert the user's local hours first. A campaign without a schedule serves at all hours, so its first `add_ad_schedule` windows switch off every other hour and day: say so, the preview needs a double confirmation. To change a window, remove the old one with `remove_entity` (`entity_type="campaign_criterion"`, the `remove_id` from the preview), then add the new one. Prices must match the landing page or Google disapproves them. +- **Call assets, schedules, promotions and prices.** Never guess `country_code` or `language_code`: take them from the number and the landing page, and ask when unclear. Money amounts are in the account currency (the preview names it); pass `currency_code` only for a different one. Ad schedule hours are in the **account time zone** (the previews show it), not the searcher's: convert the user's local hours first. A campaign without a schedule serves at all hours, so its first `add_ad_schedule` windows switch off every other hour and day: say so, the preview needs a double confirmation. To change a window, pass the complete list as `update_campaign` `ad_schedule`, or remove the old one with `remove_entity` (`entity_type="campaign_criterion"`, the `remove_id` from the preview) and add the new one. Prices must match the landing page or Google disapproves them. - `remove_entity` is IRREVERSIBLE — always prefer `pause_entity` unless the user explicitly wants permanent removal. Removal triggers double confirmation in the safety layer. - `remove_entity` supports `entity_type` values: "campaign", "ad_group", "ad", "keyword", "negative_keyword", "shared_criterion", "ad_group_criterion", "campaign_criterion", "campaign_asset", "ad_group_asset", "asset", "customer_asset". Use "negative_keyword" to remove campaign-level negative keywords. Use "shared_criterion" to remove a keyword from a shared negative keyword list — the `entity_id` format is "sharedSetId~criterionId" (use the `resource_id` field from `get_negative_keyword_list_keywords`). Use "ad_group_criterion" or "campaign_criterion" to remove demographic targeting (age/gender/parental/income) or an attached audience or audience exclusion — pass the `remove_id` returned by `get_demographic_targeting` or `get_audience_targeting`. Use "campaign_asset" to remove sitelinks and other asset links from a campaign, "ad_group_asset" (`adGroupId~assetId~fieldType`) for links on an ad group. Use "asset" to remove a standalone asset. Use "customer_asset" to remove a customer-level asset link. - A Google Ads `dry_run=true` sends the exact change to Google with `validate_only` — Google checks it and executes nothing. `DRY_RUN_FAILED` carries Google's error and means the real apply would fail the same way; fix the cause and draft again. Steps that build on an object an earlier step would create (e.g. keywords for a new negative list) are counted in `checks.skipped_calls` instead of validated. diff --git a/src/adloop/ads/write.py b/src/adloop/ads/write.py index 8364781..85c86b3 100644 --- a/src/adloop/ads/write.py +++ b/src/adloop/ads/write.py @@ -2105,15 +2105,12 @@ def draft_campaign( URL options. Omitted or empty leaves them unset. geo_exclude_ids: geo target constant IDs to exclude; may not overlap geo_target_ids. - ad_schedule: serving windows, validated like add_ad_schedule. They are the - campaign's first schedule, so the plan needs a double confirmation. + ad_schedule: serving windows, validated like add_ad_schedule. The preview + warns that the campaign serves only inside them once it is enabled. No + double confirmation: the campaign is created PAUSED, so nothing stops + serving. """ - from adloop.ads.assets import ( - _account_settings, - _first_schedule_warning, - _time_zone_note, - _validate_schedule, - ) + from adloop.ads.assets import _account_settings, _time_zone_note, _validate_schedule from adloop.safety.guards import ( SafetyViolation, check_blocked_operation, @@ -2185,7 +2182,12 @@ def draft_campaign( target_changes["ad_schedule"] = schedule target_changes["account_time_zone"] = time_zone warnings.append(_time_zone_note(time_zone)) - warnings.append(_first_schedule_warning(campaign_name)) + # Only a warning: the new campaign is PAUSED, so nothing stops serving. + # update_campaign double-confirms a live campaign's first schedule. + warnings.append( + f"Campaign '{campaign_name}' is created PAUSED. Once it is enabled it serves " + "only inside these windows: no ads at other hours or on days without a window." + ) location_changes = {} if location_option: location_changes["location_option"] = location_option @@ -2214,8 +2216,6 @@ def draft_campaign( **url_options, **target_changes, }, - # A campaign's first windows switch off every hour outside them. - requires_double_confirm=bool(schedule), ) store_plan(plan) preview = plan.to_preview() diff --git a/src/adloop/ga4/suggest.py b/src/adloop/ga4/suggest.py index 5ddf4f9..5e545d9 100644 --- a/src/adloop/ga4/suggest.py +++ b/src/adloop/ga4/suggest.py @@ -25,7 +25,9 @@ _REDDIT_STATUS = {"PAUSED": "paused", "ACTIVE": "enabled", "ARCHIVED": "archived"} # update_campaign keys that change who sees the ads or how much they cost; # URL options and tracking templates do not move traffic. -_TARGETING_KEYS = ("geo_target_ids", "language_ids", "location_option", "negative_location_option") +_TARGETING_KEYS = ( + "geo_target_ids", "geo_exclude_ids", "language_ids", "location_option", "negative_location_option", +) _NETWORK_KEYS = ("search_partners_enabled", "display_network_enabled") @@ -52,6 +54,8 @@ def _google_parts(plan: ChangePlan) -> list[str]: parts.append(f"max CPC {_amount(changes['max_cpc'])}") if any(k in changes for k in _TARGETING_KEYS): parts.append("targeting") + if "ad_schedule" in changes: + parts.append("ad schedule") if any(k in changes for k in _NETWORK_KEYS): parts.append("networks") if "keyword_match_type" in changes: diff --git a/src/adloop/rules/adloop.md b/src/adloop/rules/adloop.md index 8094c46..4325083 100644 --- a/src/adloop/rules/adloop.md +++ b/src/adloop/rules/adloop.md @@ -377,7 +377,7 @@ Conversions that happen offline (a sales call, a lead that turned into a deal) g | `draft_asset_group_signals` | Add search themes or set the audience signal of a PMax asset group, or remove signals | `asset_group_id`, `add_search_themes` (≤80 chars, up to 50 per group), `audience_id` (one per group), `remove_signal_ids` | | `draft_pmax_product_exclusions` | Set the full list of products a retail PMax asset group excludes, on one dimension; everything else stays included | `asset_group_id`, `exclude_values` (the COMPLETE list; `[]` includes everything again), `dimension` item_id (default) / brand / custom_label_0–4 | | `draft_ad_group` | Create a new ad group within an existing campaign (does NOT publish) | `campaign_id` (REQUIRED), `ad_group_name` (REQUIRED), `keywords` (optional list of {text, match_type}), `cpc_bid_micros` (optional) | -| `update_campaign` | Modify existing campaign settings — bid strategy, budget, geo targets, language targets, location options, Search partners, display expansion, URL options, campaign-level broad match | `campaign_id` (REQUIRED), plus any of: `bidding_strategy`, `daily_budget`, `geo_target_ids`, `language_ids`, `location_option`, `negative_location_option`, `search_partners_enabled`, `display_network_enabled`, TARGET_SPEND `max_cpc`, `tracking_url_template`, `final_url_suffix`, `url_custom_parameters`, `keyword_match_type`, `geo_exclude_ids`, `ad_schedule` | +| `update_campaign` | Modify existing campaign settings — bid strategy, budget, geo targets, geo exclusions, language targets, ad schedule, location options, Search partners, display expansion, URL options, campaign-level broad match | `campaign_id` (REQUIRED), plus any of: `bidding_strategy`, `daily_budget`, `geo_target_ids`, `language_ids`, `location_option`, `negative_location_option`, `search_partners_enabled`, `display_network_enabled`, TARGET_SPEND `max_cpc`, `tracking_url_template`, `final_url_suffix`, `url_custom_parameters`, `keyword_match_type`, `geo_exclude_ids`, `ad_schedule` | | `update_ad_group` | Update ad group name and/or MANUAL_CPC `max_cpc` | `ad_group_id`, optional `ad_group_name`, optional `max_cpc` | | `draft_responsive_search_ad` | Create RSA preview (does NOT publish) | 3-15 headlines (≤30 chars), 2-4 descriptions (≤90 chars), final_url required, path1/path2 (≤15 chars each). Each headline/description may be a plain string (unpinned) or `{"text": "...", "pinned_field": "HEADLINE_1"}` (pinned). Valid pin slots: `HEADLINE_1/2/3`, `DESCRIPTION_1/2`. Several assets may pin to one slot (they rotate there), but every slot must stay fillable: by an asset pinned to it or by a distinct unpinned asset. | | `draft_callouts` | Create callout assets for an ad group, a campaign or the account (does NOT publish) | `campaign_id` or `ad_group_id` (the scope follows the id), or `scope="account"`; `callouts` list with 1-25 chars each | @@ -463,10 +463,10 @@ Same draft → preview → `confirm_and_apply` gate; the plan carries `platform: - Location options: `location_option` is who the geo targets reach — `PRESENCE` (people in or regularly in the locations), `PRESENCE_OR_INTEREST` (Google's default; also people elsewhere who show interest in them) or `SEARCH_INTEREST`. `negative_location_option` (`PRESENCE` or `PRESENCE_OR_INTEREST`) is who the exclusions exclude. Empty leaves the value as it is. The preview warns when a change widens reach: for `location_option` that is anything but `PRESENCE`, for `negative_location_option` it is `PRESENCE`, because then the exclusions only keep out people physically there, not people interested in the excluded places; `update_campaign` shows the current value as `current_location_option` / `current_negative_location_option`. Read back with `get_campaign_settings`. - Campaign URL options (`tracking_url_template`, `final_url_suffix`, `url_custom_parameters`): omitted leaves a value unchanged, `""` or `[]` removes it, and `url_custom_parameters` replaces the whole list. The template must contain `{lpurl}` or a variant (`{lpurl+2}`, `{unescapedlpurl}`, `{escapedlpurl+3}`, ...). Custom parameter keys are alphanumeric, at most 8 characters. The `update_campaign` preview shows each current value (`current_tracking_url_template` etc.), warns on a removal, and gives `example_click`: one of the campaign's final URLs with the campaign's options applied. It is approximate: a template on the ad group, ad or keyword overrides the campaign's, and ValueTrack parameters are filled at click time. Check it for typos before approving. Read back with `get_campaign_settings`. - Campaign-level broad match: `update_campaign` `keyword_match_type="BROAD"` CONVERTS every existing phrase and exact keyword in the campaign into broad match. Google shows the originals as Removed, turning the setting off does not restore them, and since September 2026 Google also upgrades such campaigns to AI Max. It is refused unless the campaign (or the `bidding_strategy` in the same call) is Smart Bidding — the same rule as BROAD keywords — checked again at apply, and needs a double confirmation. `keyword_match_type="UNSPECIFIED"` turns it off (single confirmation): no further conversions, but converted keywords stay broad and must be re-added as phrase/exact by hand. Moving a broad-match campaign to a non-Smart `bidding_strategy` is refused unless the same call passes `UNSPECIFIED`. Never turn it on to "get more traffic" on an exact/phrase campaign; read it back with `get_campaign_settings`, which lists it in `widening_settings`. -- `update_campaign` replaces POSITIVE geo/language targets entirely (not append). Pass the full desired list. NEGATIVE geo exclusions (criteria with `negative=TRUE`) are **preserved** across a positive-geo replacement — they survive the swap. The preview surfaces preserved negative geo IDs in `preserved_negative_geo_target_ids` so the change is auditable. To ADD negative geo exclusions (e.g. exclude a city inside a targeted state), use `add_negative_locations` — do not try to express exclusions through `geo_target_ids`. To remove a negative geo exclusion explicitly, use `remove_entity` with `entity_type="campaign_criterion"`. -- `geo_exclude_ids` and `ad_schedule` set exclusions and windows when creating a campaign, in the same atomic request. On `update_campaign` they REPLACE the whole list: omitted leaves it unchanged, `[]` clears it. Use them to set the complete state; `add_negative_locations` and `add_ad_schedule` add to what is there. The preview shows the current values (`current_geo_exclude_ids`, `current_ad_schedule`) and apply refuses if they changed after it. Double confirmation is needed for a campaign's first schedule (also on create), for clearing a schedule (the campaign serves at all hours again) and for dropping an exclusion (it can serve there again). +- `update_campaign` replaces POSITIVE geo/language targets entirely (not append). Pass the full desired list. NEGATIVE geo exclusions (criteria with `negative=TRUE`) are **preserved** across a positive-geo replacement — they survive the swap — unless `geo_exclude_ids` is passed in the same call. The preview surfaces preserved negative geo IDs in `preserved_negative_geo_target_ids` so the change is auditable. To ADD negative geo exclusions (e.g. exclude a city inside a targeted state), use `add_negative_locations` — do not try to express exclusions through `geo_target_ids`. To remove a negative geo exclusion explicitly, use `remove_entity` with `entity_type="campaign_criterion"`, or pass `geo_exclude_ids` without it. +- `geo_exclude_ids` and `ad_schedule` set exclusions and windows when creating a campaign, in the same atomic request. A schedule there gets a warning, not a double confirmation: the campaign is created PAUSED, and once it is enabled it serves only inside the windows. On `update_campaign` they REPLACE the whole list: omitted leaves it unchanged, `[]` clears it. Use them to set the complete state; `add_negative_locations` and `add_ad_schedule` add to what is there. The preview shows the current values (`current_geo_exclude_ids`, `current_ad_schedule`), only the entries that differ are removed or added, and apply refuses if the current values changed after the preview. `update_campaign` needs a double confirmation for a campaign's first schedule (every other hour stops serving), for clearing a schedule (the campaign serves at all hours again) and for dropping an exclusion (it can serve there again). A location can move between `geo_target_ids` and `geo_exclude_ids` in one call; a location in both lists is refused. - **Asset scope:** callouts and snippets link where the id points (`ad_group_id` → ad group, `campaign_id` → campaign). Account-wide (`scope="account"`) reaches every campaign without assets of its own; use it only when the user asks for account level. **Asset edits** (`update_callout`, `update_sitelink`, `update_structured_snippet`) change the asset everywhere it is linked; check with GAQL (`campaign_asset`, `ad_group_asset`, `customer_asset`) where else it is used before editing a shared asset. If the apply reports the asset changed in Google Ads since the preview, draft again. -- **Call assets, schedules, promotions and prices.** Never guess `country_code` or `language_code`: take them from the number and the landing page, and ask when unclear. Money amounts are in the account currency (the preview names it); pass `currency_code` only for a different one. Ad schedule hours are in the **account time zone** (the previews show it), not the searcher's: convert the user's local hours first. A campaign without a schedule serves at all hours, so its first `add_ad_schedule` windows switch off every other hour and day: say so, the preview needs a double confirmation. To change a window, remove the old one with `remove_entity` (`entity_type="campaign_criterion"`, the `remove_id` from the preview), then add the new one. Prices must match the landing page or Google disapproves them. +- **Call assets, schedules, promotions and prices.** Never guess `country_code` or `language_code`: take them from the number and the landing page, and ask when unclear. Money amounts are in the account currency (the preview names it); pass `currency_code` only for a different one. Ad schedule hours are in the **account time zone** (the previews show it), not the searcher's: convert the user's local hours first. A campaign without a schedule serves at all hours, so its first `add_ad_schedule` windows switch off every other hour and day: say so, the preview needs a double confirmation. To change a window, pass the complete list as `update_campaign` `ad_schedule`, or remove the old one with `remove_entity` (`entity_type="campaign_criterion"`, the `remove_id` from the preview) and add the new one. Prices must match the landing page or Google disapproves them. - `remove_entity` is IRREVERSIBLE — always prefer `pause_entity` unless the user explicitly wants permanent removal. Removal triggers double confirmation in the safety layer. - `remove_entity` supports `entity_type` values: "campaign", "ad_group", "ad", "keyword", "negative_keyword", "shared_criterion", "ad_group_criterion", "campaign_criterion", "campaign_asset", "ad_group_asset", "asset", "customer_asset". Use "negative_keyword" to remove campaign-level negative keywords. Use "shared_criterion" to remove a keyword from a shared negative keyword list — the `entity_id` format is "sharedSetId~criterionId" (use the `resource_id` field from `get_negative_keyword_list_keywords`). Use "ad_group_criterion" or "campaign_criterion" to remove demographic targeting (age/gender/parental/income) or an attached audience or audience exclusion — pass the `remove_id` returned by `get_demographic_targeting` or `get_audience_targeting`. Use "campaign_asset" to remove sitelinks and other asset links from a campaign, "ad_group_asset" (`adGroupId~assetId~fieldType`) for links on an ad group. Use "asset" to remove a standalone asset. Use "customer_asset" to remove a customer-level asset link. - A Google Ads `dry_run=true` sends the exact change to Google with `validate_only` — Google checks it and executes nothing. `DRY_RUN_FAILED` carries Google's error and means the real apply would fail the same way; fix the cause and draft again. Steps that build on an object an earlier step would create (e.g. keywords for a new negative list) are counted in `checks.skipped_calls` instead of validated. diff --git a/src/adloop/server.py b/src/adloop/server.py index 63d69c1..11a4f3a 100644 --- a/src/adloop/server.py +++ b/src/adloop/server.py @@ -3939,8 +3939,9 @@ def draft_campaign( ad_schedule: Windows the campaign serves in: list of {day_of_week (MONDAY..SUNDAY), start_hour 0-23, end_hour 0-24, start_minute, end_minute (0, 15, 30 or 45)}, whole numbers, in the account time - zone. No overlaps, at most six a day. Outside them the campaign does - not serve, so the plan needs a double confirmation. + zone. No overlaps, at most six a day. Once the campaign is enabled it + serves only inside them; the preview warns about that. No double + confirmation, since the campaign is created PAUSED. """ from adloop.ads.write import draft_campaign as _impl @@ -4044,11 +4045,11 @@ def update_campaign( Only the parameters passed are changed; omitted ones stay as they are, and at least one change is required. Replacing geo targets keeps the campaign's negative geo exclusions (the preview lists them) unless geo_exclude_ids is - passed too. A budget change shows the - current amount; when the budget is shared with other campaigns, the - preview names them and needs a double confirmation, because the new amount - covers all of them. A bidding change on a campaign in a portfolio bid - strategy takes it out of the portfolio, also with a double confirmation. + passed too. A budget change shows the current amount; when the budget is + shared with other campaigns, the preview names them and needs a double + confirmation, because the new amount covers all of them. A bidding change + on a campaign in a portfolio bid strategy takes it out of the portfolio, + also with a double confirmation. Returns a preview with a plan_id plus any warnings; nothing changes until confirm_and_apply applies it. @@ -4118,13 +4119,16 @@ def update_campaign( Omitted leaves them unchanged; [] removes them. May not overlap geo_target_ids. The preview shows the current ones (current_geo_exclude_ids); dropping any needs a double confirmation, - because the campaign can serve there again. + because the campaign can serve there again. A location can move + between geo_target_ids and geo_exclude_ids in one call: its old + criterion is removed before the new one is created. ad_schedule: REPLACES the campaign's schedule windows (shape as in draft_campaign). Omitted leaves them unchanged; [] removes them, so the campaign serves at all hours. The preview shows the current ones (current_ad_schedule). A first schedule and clearing a schedule both - need a double confirmation. Apply refuses if the exclusions or - windows changed after the preview. + need a double confirmation. Only windows that differ are removed or + added, removals first. Apply refuses if the exclusions or windows + changed after the preview. """ from adloop.ads.write import update_campaign as _impl diff --git a/tests/test_campaign_geo_exclude_schedule.py b/tests/test_campaign_geo_exclude_schedule.py index d0427f2..739acd6 100644 --- a/tests/test_campaign_geo_exclude_schedule.py +++ b/tests/test_campaign_geo_exclude_schedule.py @@ -18,6 +18,7 @@ from adloop.ads.client import GOOGLE_ADS_API_VERSION from adloop.config import AdLoopConfig, AdsConfig, SafetyConfig from adloop.safety import preview as preview_store +from adloop.safety.approval import confirmation_message CAMPAIGN = "1001" MONDAY = {"day_of_week": "MONDAY", "start_hour": 8, "end_hour": 17} @@ -49,6 +50,10 @@ def search(self, *, customer_id, query): return iter(self._client.window_rows) if "campaign_criterion.negative = TRUE" in query: return iter(self._client.exclude_rows) + if "campaign_criterion.negative = FALSE" in query: + return iter(self._client.target_rows) + if "FROM campaign WHERE" in query: + return iter(self._client.campaign_rows) return iter([]) def __getattr__(self, attr): @@ -75,6 +80,8 @@ def __init__(self): self.sent = [] self.window_rows = [] self.exclude_rows = [] + self.target_rows = [] + self.campaign_rows = [] def get_service(self, name, *_args, **_kwargs): return _Service(self, name) @@ -91,6 +98,26 @@ def created(self): return [op.campaign_criterion_operation.create for op in self.operations() if not op.campaign_criterion_operation.remove and op.campaign_criterion_operation.create.campaign] + def order(self): + """The request's operations in the order sent, as short tuples.""" + summary = [] + for op in self.operations(): + criterion_op = op.campaign_criterion_operation + created = criterion_op.create + if op.campaign_operation.update.resource_name: + summary.append(("campaign", *op.campaign_operation.update_mask.paths)) + elif criterion_op.remove: + summary.append(("remove", criterion_op.remove.rsplit("~", 1)[-1])) + elif created.location.geo_target_constant: + geo_id = created.location.geo_target_constant.rsplit("/", 1)[-1] + summary.append(("exclude" if created.negative else "target", geo_id)) + elif created.language.language_constant: + summary.append(("language", created.language.language_constant.rsplit("/", 1)[-1])) + else: + window = created.ad_schedule + summary.append(("window", window.day_of_week.name, window.start_hour, window.end_hour)) + return summary + @pytest.fixture def client(monkeypatch): @@ -127,7 +154,10 @@ def execute_query(_config, _cid, query): if "FROM campaign WHERE campaign.id" in query: if not found: return [] - return [{"campaign.name": "Brand", "campaign.status": status, "customer.time_zone": "America/Chicago"}] + # Also answers update_campaign's keyword match type read (#117). + return [{"campaign.name": "Brand", "campaign.status": status, "customer.time_zone": "America/Chicago", + "campaign.keyword_match_type": "UNSPECIFIED", + "campaign.bidding_strategy_type": "MAXIMIZE_CONVERSIONS"}] raise AssertionError(f"unexpected query: {query}") monkeypatch.setattr("adloop.ads.gaql.execute_query", execute_query) @@ -171,13 +201,17 @@ def test_both_go_in_the_one_atomic_mutate(self, config, client, monkeypatch): assert (windows[0].ad_schedule.start_hour, windows[0].ad_schedule.end_hour) == (8, 17) assert windows[0].ad_schedule.day_of_week == client.enums.DayOfWeekEnum.MONDAY - def test_a_schedule_is_a_first_schedule(self, config, client, monkeypatch): + def test_a_schedule_only_warns_because_the_campaign_is_created_paused(self, config, client, monkeypatch): _account(monkeypatch) preview = _draft(config, ad_schedule=[MONDAY]) - assert preview["requires_double_confirm"] is True + assert preview["requires_double_confirm"] is False assert preview["changes"]["account_time_zone"] == "America/Chicago" assert any("account time zone (America/Chicago)" in w for w in preview["warnings"]) - assert any("serves only inside them" in w for w in preview["warnings"]) + assert any("created PAUSED" in w and "Once it is enabled it serves only inside these windows" in w + for w in preview["warnings"]) + assert not any("stops serving" in w for w in preview["warnings"]) + dialog = confirmation_message(preview_store.get_plan(preview["plan_id"])) + assert "cannot be undone" not in dialog def test_exclusions_alone_need_no_double_confirmation_or_read(self, config, client): # no_live_gaql from conftest fails this test if the draft queries anything. @@ -230,7 +264,7 @@ def mutate(self, *, customer_id, mutate_operations): class TestUpdateGeoExcludes: def test_replaces_and_only_touches_what_differs(self, config, client, monkeypatch): _account(monkeypatch, excludes=["111", "222"]) - client.exclude_rows = [_exclude_row("111"), _exclude_row("222")] + client.exclude_rows = [_location_row("111"), _location_row("222")] preview = _update(config, geo_exclude_ids=["222", "333"]) assert preview["changes"]["current_geo_exclude_ids"] == ["111", "222"] assert preview["changes"]["geo_exclude_ids"] == ["222", "333"] @@ -245,7 +279,7 @@ def test_replaces_and_only_touches_what_differs(self, config, client, monkeypatc def test_empty_list_clears_them(self, config, client, monkeypatch): _account(monkeypatch, excludes=["111"]) - client.exclude_rows = [_exclude_row("111")] + client.exclude_rows = [_location_row("111")] preview = _update(config, geo_exclude_ids=[]) assert preview["requires_double_confirm"] is True _apply_check(config, preview) @@ -274,7 +308,7 @@ def test_same_list_is_nothing_to_change(self, config, monkeypatch): def test_apply_refuses_when_they_changed_after_the_preview(self, config, client, monkeypatch): _account(monkeypatch, excludes=["111"]) - client.exclude_rows = [_exclude_row("111"), _exclude_row("999")] + client.exclude_rows = [_location_row("111"), _location_row("999")] result = _dry_run(config, _update(config, geo_exclude_ids=["333"])) assert result["status"] == "DRY_RUN_FAILED" assert "changed after this preview" in str(result) @@ -282,19 +316,11 @@ def test_apply_refuses_when_they_changed_after_the_preview(self, config, client, def test_replacing_positive_geos_no_longer_promises_to_preserve_them(self, config, client, monkeypatch): _account(monkeypatch, excludes=["111"]) - client.exclude_rows = [_exclude_row("111")] + client.exclude_rows = [_location_row("111")] preview = _update(config, geo_target_ids=["2840"], geo_exclude_ids=[]) assert "preserved_negative_geo_target_ids" not in preview assert not any("PRESERVED" in w for w in preview["warnings"]) - def test_moving_a_location_from_excluded_to_targeted_removes_it_first(self, config, client, monkeypatch): - _account(monkeypatch, excludes=["111"]) - client.exclude_rows = [_exclude_row("111")] - _apply_check(config, _update(config, geo_target_ids=["111"], geo_exclude_ids=[])) - ops = client.operations() - assert ops[0].campaign_criterion_operation.remove.endswith("~111") - assert ops[1].campaign_criterion_operation.create.location.geo_target_constant == "geoTargetConstants/111" - def test_omitted_leaves_them_preserved(self, config, monkeypatch): _account(monkeypatch, excludes=["111"]) preview = _update(config, geo_target_ids=["2840"]) @@ -302,7 +328,7 @@ def test_omitted_leaves_them_preserved(self, config, monkeypatch): assert "geo_exclude_ids" not in preview["changes"] -def _exclude_row(geo_id): +def _location_row(geo_id): return SimpleNamespace(campaign_criterion=SimpleNamespace( resource_name=f"customers/1/campaignCriteria/{CAMPAIGN}~{geo_id}", location=SimpleNamespace(geo_target_constant=f"geoTargetConstants/{geo_id}"), @@ -324,6 +350,7 @@ def test_first_schedule_needs_a_double_confirmation_like_add_ad_schedule(self, c assert any("'Brand' has no ad schedule" in w and "stops serving" in w for w in preview["warnings"]) assert any("account time zone (America/Chicago)" in w for w in preview["warnings"]) assert preview["changes"]["current_ad_schedule"] == [] + assert "cannot be undone" in confirmation_message(preview_store.get_plan(preview["plan_id"])) _apply_check(config, preview) assert client.removed() == [] assert client.created()[0].ad_schedule.start_hour == 8 @@ -390,6 +417,83 @@ def test_unknown_and_removed_campaigns_are_refused(self, config, monkeypatch, kw assert "is removed" in _update(config, **kwargs)["error"] +class TestOneRequestOrder: + """update_campaign sends one mutate, in this order: the campaign's own + fields, exclusion removes, positive geo removes, positive geo creates, + languages, exclusion creates, then schedule removes before schedule creates. + + A location criterion's id is its geo target constant, so a location moving + between the targets and the exclusions is the same criterion removed and + created again: the remove has to come first in the request. + """ + + def test_a_location_moving_from_excluded_to_targeted_is_removed_first(self, config, client, monkeypatch): + _account(monkeypatch, excludes=["111", "222"]) + client.exclude_rows = [_location_row("111"), _location_row("222")] + client.target_rows = [_location_row("2840")] + preview = _update(config, geo_target_ids=["2840", "111"], geo_exclude_ids=["222"]) + assert preview["requires_double_confirm"] is True # 111 is no longer excluded + _apply_check(config, preview) + assert client.order() == [ + ("remove", "111"), # the exclusion + ("remove", "2840"), # positive targets are replaced as a whole + ("target", "2840"), + ("target", "111"), + ] + + def test_a_location_moving_from_targeted_to_excluded_is_removed_first(self, config, client, monkeypatch): + _account(monkeypatch) + client.target_rows = [_location_row("111"), _location_row("2840")] + _apply_check(config, _update(config, geo_target_ids=["2840"], geo_exclude_ids=["111"])) + assert client.order() == [ + ("remove", "111"), # the positive target + ("remove", "2840"), + ("target", "2840"), + ("exclude", "111"), + ] + + def test_a_shifted_window_is_removed_before_its_replacement_is_added(self, config, client, monkeypatch): + _account(monkeypatch, windows=[_window_row(5, **MONDAY), _window_row(6, **{**TUESDAY, "start_minute": "THIRTY"})]) + client.window_rows = [_criterion_row(5), _criterion_row(6)] + later = {"day_of_week": "MONDAY", "start_hour": 9, "end_hour": 18} # overlaps the 8-17 window + _apply_check(config, _update(config, ad_schedule=[later, TUESDAY])) + assert client.order() == [("remove", "5"), ("window", "MONDAY", 9, 18)] + + def test_a_full_day_of_windows_is_swapped_removals_first(self, config, client, monkeypatch): + # Six windows is Google's limit for a day: adding before removing would exceed it. + old = [{"day_of_week": "MONDAY", "start_hour": 2 * i, "end_hour": 2 * i + 1} for i in range(6)] + new = [{"day_of_week": "MONDAY", "start_hour": 2 * i + 1, "end_hour": 2 * i + 2} for i in range(6)] + _account(monkeypatch, windows=[_window_row(i + 1, **window) for i, window in enumerate(old)]) + client.window_rows = [_criterion_row(i + 1) for i in range(6)] + _apply_check(config, _update(config, ad_schedule=new)) + assert [step[0] for step in client.order()] == ["remove"] * 6 + ["window"] * 6 + + def test_broad_match_and_both_replacements_go_in_one_request(self, config, client, monkeypatch): + _account(monkeypatch, excludes=["111"], windows=[_window_row(5, **MONDAY)]) + client.exclude_rows = [_location_row("111")] + client.target_rows = [_location_row("2840")] + client.window_rows = [_criterion_row(5)] + client.campaign_rows = [SimpleNamespace(campaign=SimpleNamespace( + bidding_strategy_type="MAXIMIZE_CONVERSIONS", keyword_match_type="UNSPECIFIED"))] + preview = _update(config, keyword_match_type="BROAD", geo_target_ids=["2840", "111"], + geo_exclude_ids=["222"], ad_schedule=[TUESDAY]) + assert preview["changes"]["keyword_match_type"] == "BROAD" + assert preview["changes"]["geo_exclude_ids"] == ["222"] + assert preview["changes"]["ad_schedule"][0]["day_of_week"] == "TUESDAY" + assert preview["requires_double_confirm"] is True + _apply_check(config, preview) + assert client.order() == [ + ("campaign", "keyword_match_type"), + ("remove", "111"), + ("remove", "2840"), + ("target", "2840"), + ("target", "111"), + ("exclude", "222"), + ("remove", "5"), + ("window", "TUESDAY", 9, 12), + ] + + @pytest.mark.asyncio async def test_the_mcp_tools_take_both_parameters(): from adloop.server import mcp diff --git a/tests/test_ga4_suggest.py b/tests/test_ga4_suggest.py index e2ce8a8..84d947a 100644 --- a/tests/test_ga4_suggest.py +++ b/tests/test_ga4_suggest.py @@ -35,6 +35,11 @@ def _plan(operation, entity_type="campaign", entity_id="111", **changes): "Ads: Brand DE: budget 50 → 80"), (_plan("update_campaign", bidding_strategy="MAXIMIZE_CONVERSIONS", geo_target_ids=["2276"]), "Ads: 111: bidding maximize conversions, targeting"), + (_plan("update_campaign", current_geo_exclude_ids=[], geo_exclude_ids=["2040"]), + "Ads: 111: targeting"), + (_plan("update_campaign", current_ad_schedule=[], + ad_schedule=[{"day_of_week": "MONDAY", "start_hour": 8, "end_hour": 17}]), + "Ads: 111: ad schedule"), (_plan("reddit_set_status", entity_type="ad_group", name="Founders", target_status="PAUSED"), "Reddit: ad group Founders paused"), (_plan("reddit_update_campaign", campaign_name="Launch",