From 0a114a5d6015a76fb2e1d49f703e665712de616e Mon Sep 17 00:00:00 2001 From: Illia Sapryga Date: Sun, 1 Nov 2026 16:00:32 -0600 Subject: [PATCH] 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 273364c..70a4601 100644 --- a/.claude/rules/adloop.md +++ b/.claude/rules/adloop.md @@ -279,13 +279,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_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` | +| `update_campaign` | Modify existing campaign settings — bid strategy, budget, geo targets, language targets, location options, Search partners, display expansion, URL options | `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`, `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 | @@ -371,6 +371,7 @@ 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`. - `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 2263af9..85519dc 100644 --- a/.cursor/rules/adloop.mdc +++ b/.cursor/rules/adloop.mdc @@ -281,13 +281,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_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` | +| `update_campaign` | Modify existing campaign settings — bid strategy, budget, geo targets, language targets, location options, Search partners, display expansion, URL options | `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`, `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 | @@ -373,6 +373,7 @@ 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`. - `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 abe7801..653b43d 100644 --- a/README.md +++ b/README.md @@ -270,12 +270,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, 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, 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 bd08e5c..3b1eb1a 100644 --- a/src/adloop/ads/assets.py +++ b/src/adloop/ads/assets.py @@ -1095,6 +1095,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 @@ -1156,11 +1164,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 54c61ba..7b40baf 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() @@ -2275,6 +2304,8 @@ def update_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 an update to an existing campaign — returns preview, does NOT execute. @@ -2283,6 +2314,9 @@ def update_campaign( A location option change shows the current value next to the new one. URL options: None leaves one unchanged, an empty string or list clears it; the preview shows the current values and an example click URL. + 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, @@ -2347,6 +2381,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: @@ -2373,6 +2415,8 @@ def update_campaign( location_option, negative_location_option, url_options, + 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") @@ -2402,7 +2446,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 ) @@ -2446,12 +2490,32 @@ def update_campaign( } url_example = _example_click_urls(current_urls["final_url"], effective) + 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, @@ -2516,6 +2580,9 @@ def update_campaign( if field in url_options: changes[f"current_{field}"] = current_urls[field] changes[field] = url_options[field] + 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", @@ -3333,8 +3400,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. @@ -3343,7 +3493,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 @@ -3353,10 +3504,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] = [] @@ -4609,11 +4763,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: @@ -4627,12 +4790,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") @@ -4807,6 +5050,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 @@ -4866,6 +5119,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 273364c..70a4601 100644 --- a/src/adloop/rules/adloop.md +++ b/src/adloop/rules/adloop.md @@ -279,13 +279,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_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` | +| `update_campaign` | Modify existing campaign settings — bid strategy, budget, geo targets, language targets, location options, Search partners, display expansion, URL options | `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`, `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 | @@ -371,6 +371,7 @@ 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`. - `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 9d759aa..6bb32f3 100644 --- a/src/adloop/server.py +++ b/src/adloop/server.py @@ -3305,11 +3305,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, @@ -3376,6 +3379,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 @@ -3401,6 +3412,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, ) @@ -3468,12 +3481,15 @@ def update_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 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 @@ -3531,6 +3547,17 @@ def update_campaign( 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. + 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 @@ -3553,6 +3580,8 @@ def update_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, ) 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"]