From 2ce2de9c0fd6b5820d99f6133545595408d643f9 Mon Sep 17 00:00:00 2001 From: xgp <244253+xgp@users.noreply.github.com> Date: Mon, 26 Oct 2026 21:39:33 +0000 Subject: [PATCH] Add draft_dismiss_recommendations Dismiss Google recommendations the user has decided not to take, through the usual draft, preview and confirm_and_apply flow. get_recommendations now returns each recommendation's ID and resource name, which the new tool takes. DismissRecommendation has no validate_only field, so the dry run re-reads the account and checks every recommendation is still active, the way the Reddit, Tag Manager and GA4 writes do. Closes #108. --- .claude/rules/adloop.md | 4 +- .cursor/rules/adloop.mdc | 4 +- README.md | 1 + src/adloop/ads/read.py | 4 + src/adloop/ads/recommendations.py | 210 +++++++++++++++++ src/adloop/ads/write.py | 17 +- src/adloop/rules/adloop.md | 4 +- src/adloop/server.py | 48 +++- tests/test_recommendations_dismiss.py | 314 ++++++++++++++++++++++++++ 9 files changed, 597 insertions(+), 9 deletions(-) create mode 100644 src/adloop/ads/recommendations.py create mode 100644 tests/test_recommendations_dismiss.py diff --git a/.claude/rules/adloop.md b/.claude/rules/adloop.md index c14935f..5f0cda5 100644 --- a/.claude/rules/adloop.md +++ b/.claude/rules/adloop.md @@ -79,7 +79,7 @@ You have access to AdLoop MCP tools that connect Google Ads, Reddit Ads and Goog - `get_ad_performance` returns full `headlines` and `descriptions` lists for RSAs, plus `ad_group_ad.policy_summary.approval_status`, `review_status` and `policy_topics` (topic names only) per ad. Ads that are `DISAPPROVED` (cannot serve) or `APPROVED_LIMITED` (serving restricted) are listed in `policy_issues` with their spend and flagged in `insights[]`; compact mode adds `by_approval_status` counts. - `get_campaign_performance` returns `metrics.search_impression_share`, `metrics.search_budget_lost_impression_share` and `metrics.search_rank_lost_impression_share` as fractions (0.25 = 25%) with matching `*_pct` percentages. Google caps the reported values: 0.0999 means below 10% share, 0.9001 means above 90% lost. They are `null` for campaigns that do not serve on Search (Display, Video, PMax, ...) or had no impressions, which means "does not apply", not zero. `budget_limited_converters` and `insights[]` flag converting campaigns losing 20% or more of impressions to budget: those can absorb more budget, rank-lost share instead points at bids, ad relevance and quality. - `get_change_history` returns changes newest first with counts `by_resource_type`, `by_client`, `by_user` and `by_day`. Google Ads keeps 30 days of change history, so an older start date is moved forward (see `notes`). `insights[]` flag auto-applied recommendations and budget or bidding changes. `truncated: true` means the limit was hit; narrow the range or filters to see older changes. -- `get_recommendations` returns `estimated_improvement` per recommendation (potential minus base metrics) and `insights[]` that flag budget-increase recommendations, whose projected gain comes from spending more. +- `get_recommendations` returns each recommendation's `recommendation.id` (what `draft_dismiss_recommendations` takes) and `estimated_improvement` per recommendation (potential minus base metrics) and `insights[]` that flag budget-increase recommendations, whose projected gain comes from spending more. - PMax tools: `get_pmax_performance` returns `insights[]` flagging weak ad strength and zero-conversion asset groups. `segments.ad_network_type` includes MIXED — a Google catch-all for most PMax traffic. Full channel splits (Search vs YouTube vs Display vs Discover) are not available via the API. - `get_pmax_assets` returns `by_status` and `by_field_type` summaries. Note: per-asset performance labels (BEST/GOOD/LOW) are not available for PMax assets in the Google Ads API. Use `get_detailed_asset_performance` for quality signals via top combinations. - `get_audience_performance` works for campaigns with explicit audience targeting. PMax audience targeting is automatic and may not appear in this report. When the results include SEARCH campaigns, `insights[]` reminds you that custom segments cannot be attached to them (see the compatibility matrix below) — relay that constraint instead of proposing impossible pairings. @@ -317,6 +317,7 @@ Conversions that happen offline (a sales call, a lead that turned into a deal) g | `add_to_negative_keyword_list` | Append keywords to an EXISTING shared negative keyword list (does NOT add) | `shared_set_id` (from `get_negative_keyword_lists`), keyword list, `match_type` | | `attach_shared_set_to_campaigns` | Attach an EXISTING shared set (e.g. shared negative keyword list) to one or more campaigns. Use after creating a campaign to inherit pre-built negatives. | `shared_set_id` (from `get_negative_keyword_lists`), `campaign_ids` list | | `detach_shared_set_from_campaigns` | Detach a shared set from one or more campaigns. Removes only the linkage; the shared set and its keywords stay intact. | `shared_set_id`, `campaign_ids` list | +| `draft_dismiss_recommendations` | Dismiss Google recommendations the user has decided not to take. Nothing in the account changes and Google can show them again later; unknown or already dismissed IDs are refused. No validate-only mode: the dry run re-reads the account and checks each one is still active | `recommendation_ids` (`recommendation.id` from `get_recommendations`) | | `draft_demographic_targeting` | Propose demographic criteria (age, gender, parental status, income range) at ad group or campaign level. Defaults to EXCLUSION (`negative=True`). | exactly one of `ad_group_id` or `campaign_id`, at least one of `age_ranges`/`genders`/`parental_statuses`/`income_ranges`, optional `negative` (default True) | | `draft_audience_attachment` | Attach audiences to a campaign or ad group as OBSERVATION (default: bids and reporting, reach unchanged), TARGETING (ads only reach the audiences; double confirmation) or exclusion. Refuses incompatible pairings, Search lists too small to target, audiences at both campaign and ad group level, and Performance Max | exactly one of `campaign_id` or `ad_group_id`; any of `user_list_ids`, `segment_ids`, `custom_audience_ids`, `combined_audience_ids`; `mode` observation/targeting; `exclude` | | `draft_ai_max_settings` | Draft AI Max controls for one Search campaign: `enable_ai_max`, per-ad-group `disable_search_term_matching`, text and final-URL asset automation. Refuses a plan that would switch AI Max on while an automation it was never told about keeps running; an explicitly requested automation asks for a double confirmation. | `campaign_id`, optional `enable_ai_max`, optional `disable_search_term_matching`, optional `ad_group_ids`, optional `include_paused_ad_groups`, optional `text_asset_automation`, optional `final_url_expansion` | @@ -657,6 +658,7 @@ Reddit's structure is campaign → ad group (budget, bid, pixel, targeting) → 4. Use `estimated_improvement` to prioritize: recommendations with >1 estimated additional conversion are worth investigating 5. For keyword recommendations, call `get_keyword_performance` to check existing keyword overlap before acting 6. Present recommendations grouped by type with your assessment of each — the user should understand which ones are genuinely helpful vs which ones just increase Google's revenue +7. For the ones the user decides not to take, offer `draft_dismiss_recommendations` with their IDs, so they stop reappearing. Dismiss only what the user turned down: a dismissal is not a verdict you make on their behalf ### When user asks about audience performance or targeting diff --git a/.cursor/rules/adloop.mdc b/.cursor/rules/adloop.mdc index cd519aa..33cfad6 100644 --- a/.cursor/rules/adloop.mdc +++ b/.cursor/rules/adloop.mdc @@ -81,7 +81,7 @@ You have access to AdLoop MCP tools that connect Google Ads, Reddit Ads and Goog - `get_ad_performance` returns full `headlines` and `descriptions` lists for RSAs, plus `ad_group_ad.policy_summary.approval_status`, `review_status` and `policy_topics` (topic names only) per ad. Ads that are `DISAPPROVED` (cannot serve) or `APPROVED_LIMITED` (serving restricted) are listed in `policy_issues` with their spend and flagged in `insights[]`; compact mode adds `by_approval_status` counts. - `get_campaign_performance` returns `metrics.search_impression_share`, `metrics.search_budget_lost_impression_share` and `metrics.search_rank_lost_impression_share` as fractions (0.25 = 25%) with matching `*_pct` percentages. Google caps the reported values: 0.0999 means below 10% share, 0.9001 means above 90% lost. They are `null` for campaigns that do not serve on Search (Display, Video, PMax, ...) or had no impressions, which means "does not apply", not zero. `budget_limited_converters` and `insights[]` flag converting campaigns losing 20% or more of impressions to budget: those can absorb more budget, rank-lost share instead points at bids, ad relevance and quality. - `get_change_history` returns changes newest first with counts `by_resource_type`, `by_client`, `by_user` and `by_day`. Google Ads keeps 30 days of change history, so an older start date is moved forward (see `notes`). `insights[]` flag auto-applied recommendations and budget or bidding changes. `truncated: true` means the limit was hit; narrow the range or filters to see older changes. -- `get_recommendations` returns `estimated_improvement` per recommendation (potential minus base metrics) and `insights[]` that flag budget-increase recommendations, whose projected gain comes from spending more. +- `get_recommendations` returns each recommendation's `recommendation.id` (what `draft_dismiss_recommendations` takes) and `estimated_improvement` per recommendation (potential minus base metrics) and `insights[]` that flag budget-increase recommendations, whose projected gain comes from spending more. - PMax tools: `get_pmax_performance` returns `insights[]` flagging weak ad strength and zero-conversion asset groups. `segments.ad_network_type` includes MIXED — a Google catch-all for most PMax traffic. Full channel splits (Search vs YouTube vs Display vs Discover) are not available via the API. - `get_pmax_assets` returns `by_status` and `by_field_type` summaries. Note: per-asset performance labels (BEST/GOOD/LOW) are not available for PMax assets in the Google Ads API. Use `get_detailed_asset_performance` for quality signals via top combinations. - `get_audience_performance` works for campaigns with explicit audience targeting. PMax audience targeting is automatic and may not appear in this report. When the results include SEARCH campaigns, `insights[]` reminds you that custom segments cannot be attached to them (see the compatibility matrix below) — relay that constraint instead of proposing impossible pairings. @@ -319,6 +319,7 @@ Conversions that happen offline (a sales call, a lead that turned into a deal) g | `add_to_negative_keyword_list` | Append keywords to an EXISTING shared negative keyword list (does NOT add) | `shared_set_id` (from `get_negative_keyword_lists`), keyword list, `match_type` | | `attach_shared_set_to_campaigns` | Attach an EXISTING shared set (e.g. shared negative keyword list) to one or more campaigns. Use after creating a campaign to inherit pre-built negatives. | `shared_set_id` (from `get_negative_keyword_lists`), `campaign_ids` list | | `detach_shared_set_from_campaigns` | Detach a shared set from one or more campaigns. Removes only the linkage; the shared set and its keywords stay intact. | `shared_set_id`, `campaign_ids` list | +| `draft_dismiss_recommendations` | Dismiss Google recommendations the user has decided not to take. Nothing in the account changes and Google can show them again later; unknown or already dismissed IDs are refused. No validate-only mode: the dry run re-reads the account and checks each one is still active | `recommendation_ids` (`recommendation.id` from `get_recommendations`) | | `draft_demographic_targeting` | Propose demographic criteria (age, gender, parental status, income range) at ad group or campaign level. Defaults to EXCLUSION (`negative=True`). | exactly one of `ad_group_id` or `campaign_id`, at least one of `age_ranges`/`genders`/`parental_statuses`/`income_ranges`, optional `negative` (default True) | | `draft_audience_attachment` | Attach audiences to a campaign or ad group as OBSERVATION (default: bids and reporting, reach unchanged), TARGETING (ads only reach the audiences; double confirmation) or exclusion. Refuses incompatible pairings, Search lists too small to target, audiences at both campaign and ad group level, and Performance Max | exactly one of `campaign_id` or `ad_group_id`; any of `user_list_ids`, `segment_ids`, `custom_audience_ids`, `combined_audience_ids`; `mode` observation/targeting; `exclude` | | `draft_ai_max_settings` | Draft AI Max controls for one Search campaign: `enable_ai_max`, per-ad-group `disable_search_term_matching`, text and final-URL asset automation. Refuses a plan that would switch AI Max on while an automation it was never told about keeps running; an explicitly requested automation asks for a double confirmation. | `campaign_id`, optional `enable_ai_max`, optional `disable_search_term_matching`, optional `ad_group_ids`, optional `include_paused_ad_groups`, optional `text_asset_automation`, optional `final_url_expansion` | @@ -659,6 +660,7 @@ Reddit's structure is campaign → ad group (budget, bid, pixel, targeting) → 4. Use `estimated_improvement` to prioritize: recommendations with >1 estimated additional conversion are worth investigating 5. For keyword recommendations, call `get_keyword_performance` to check existing keyword overlap before acting 6. Present recommendations grouped by type with your assessment of each — the user should understand which ones are genuinely helpful vs which ones just increase Google's revenue +7. For the ones the user decides not to take, offer `draft_dismiss_recommendations` with their IDs, so they stop reappearing. Dismiss only what the user turned down: a dismissal is not a verdict you make on their behalf ### When user asks about audience performance or targeting diff --git a/README.md b/README.md index 4d3c02b..1e70ff7 100644 --- a/README.md +++ b/README.md @@ -306,6 +306,7 @@ All write operations follow a **draft → preview → confirm** workflow. Nothin | `draft_update_custom_conversion_goal` | Rename a custom conversion goal and/or replace its action list (list replace, not append) | | `draft_assign_custom_conversion_goal` | Point a campaign at a custom conversion goal (`goal_config_level = CAMPAIGN`) | | `draft_clear_custom_conversion_goal` | Put a campaign back on the account-level goals (rollback) | +| `draft_dismiss_recommendations` | Dismiss Google recommendations you won't take, so they stop reappearing (Google can show them again later) | | `pause_entity` | Pause a campaign, ad group, ad, keyword or PMax asset group | | `enable_entity` | Re-enable a paused entity | | `remove_entity` | Permanently remove an entity (irreversible — prefers pause). Supports keywords, negative keywords, ads, ad groups, campaigns. | diff --git a/src/adloop/ads/read.py b/src/adloop/ads/read.py index 021c728..8132039 100644 --- a/src/adloop/ads/read.py +++ b/src/adloop/ads/read.py @@ -843,6 +843,10 @@ def get_recommendations( pot_conversions = _round_metric(getattr(pot, "conversions", 0)) entry = { + # The ID (last segment of the resource name) is what + # draft_dismiss_recommendations takes. + "recommendation.id": rec.resource_name.rsplit("/", 1)[-1], + "recommendation.resource_name": rec.resource_name, "recommendation.type": rec_type, "recommendation.campaign": rec.campaign, "recommendation.ad_group": rec.ad_group or "", diff --git a/src/adloop/ads/recommendations.py b/src/adloop/ads/recommendations.py new file mode 100644 index 0000000..df741fd --- /dev/null +++ b/src/adloop/ads/recommendations.py @@ -0,0 +1,210 @@ +"""Dismiss Google Ads recommendations. + +``get_recommendations`` (in ``ads/read.py``) lists the active ones; this module +turns a chosen subset down. The v25 call (verified against the SDK): + + RecommendationService.DismissRecommendation + DismissRecommendationRequest{customer_id, operations[], partial_failure} + DismissRecommendationOperation{resource_name} + +The request has **no** ``validate_only`` field, so a dry run cannot hand the +exact call to Google the way other Google Ads writes do. Instead the dry run +re-reads the account and checks that every planned recommendation is still +active (``preflight``), the same approach the Reddit, Tag Manager and GA4 +writes use. The validate-only client still refuses the call outright, so this +path can never dismiss anything during a dry run. + +Dismissing is mild: nothing in the account changes, and Google can show the +recommendation again later. A single confirmation is enough. +""" + +from __future__ import annotations + +import re +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from adloop.config import AdLoopConfig + +OPERATION = "dismiss_recommendations" + +# A recommendation ID is the last segment of its resource name. Google does not +# document the alphabet, so accept what resource names can carry and nothing +# that could break out of one. +_ID_RE = re.compile(r"^[A-Za-z0-9_~=+-]+$") +_RESOURCE_RE = re.compile(r"^customers/(\d+)/recommendations/([A-Za-z0-9_~=+-]+)$") + + +def recommendation_id(resource_name: str) -> str: + """``customers/1/recommendations/abc`` -> ``abc``.""" + return str(resource_name or "").rsplit("/", 1)[-1] + + +def _clean_ids(values: list[str] | None, cid: str) -> tuple[list[str], list[str]]: + """Accept bare IDs or full resource names. Returns (errors, deduped IDs).""" + errors: list[str] = [] + ids: list[str] = [] + for raw in values or []: + value = str(raw or "").strip() + if not value: + continue + match = _RESOURCE_RE.match(value) + if match: + if match.group(1) != cid: + errors.append( + f"{value} belongs to account {match.group(1)}, not {cid}" + ) + continue + value = match.group(2) + elif not _ID_RE.match(value): + errors.append(f"'{value}' is not a recommendation ID or resource name") + continue + if value not in ids: + ids.append(value) + return errors, ids + + +def _active_by_id(config: AdLoopConfig, customer_id: str) -> dict[str, dict]: + """Every active (not dismissed) recommendation, keyed by ID.""" + from adloop.ads.read import get_recommendations + + rows = get_recommendations(config, customer_id=customer_id).get( + "recommendations", [] + ) + return { + recommendation_id(row.get("recommendation.resource_name", "")): row + for row in rows + if row.get("recommendation.resource_name") + } + + +def draft_dismiss_recommendations( + config: AdLoopConfig, + *, + customer_id: str = "", + recommendation_ids: list[str] | None = None, +) -> dict: + """Draft dismissing recommendations — returns PREVIEW. + + Reads the active recommendations first: an ID that is unknown or already + dismissed is refused by name, so the preview only ever lists what the + apply will turn down. Each entry carries its type, campaign and Google's + estimated improvement, the same numbers ``get_recommendations`` shows. + + Call ``confirm_and_apply`` with the returned plan_id to execute. + """ + from adloop.ads.client import normalize_customer_id + from adloop.safety.guards import SafetyViolation, check_blocked_operation + from adloop.safety.preview import ChangePlan, store_plan + + try: + check_blocked_operation(OPERATION, config.safety) + except SafetyViolation as e: + return {"error": str(e)} + + cid = normalize_customer_id(customer_id or config.ads.customer_id) + errors, ids = _clean_ids(recommendation_ids, cid) + if not ids and not errors: + errors.append("At least one recommendation_id is required") + if errors: + return {"error": "Validation failed", "details": errors} + + active = _active_by_id(config, customer_id) + missing = [rec_id for rec_id in ids if rec_id not in active] + if missing: + return { + "error": "Validation failed", + "details": [ + "Not an active recommendation in this account (unknown, or " + "already dismissed): " + ", ".join(missing) + + ". get_recommendations lists the active ones with their IDs." + ], + } + + recommendations = [ + { + "recommendation_id": rec_id, + "resource_name": active[rec_id]["recommendation.resource_name"], + "type": active[rec_id].get("recommendation.type", ""), + "campaign": active[rec_id].get("recommendation.campaign", ""), + "ad_group": active[rec_id].get("recommendation.ad_group", ""), + "estimated_improvement": active[rec_id].get("estimated_improvement", {}), + } + for rec_id in ids + ] + plan = ChangePlan( + operation=OPERATION, + entity_type="recommendation", + entity_id=",".join(ids), + customer_id=customer_id, + changes={ + "recommendations": recommendations, + "count": len(recommendations), + "note": ( + "Dismissing changes nothing in the account. Google can show a " + "dismissed recommendation again later." + ), + }, + ) + store_plan(plan) + return plan.to_preview() + + +def preflight(config: AdLoopConfig, plan: object) -> dict: + """Dry run: every planned recommendation must still be active. + + DismissRecommendation has no validate-only mode, so this re-read is the + whole check. Raises when any recommendation has gone (dismissed in the UI, + applied, or expired), so the dry run fails instead of reporting success. + """ + planned = [r["recommendation_id"] for r in plan.changes["recommendations"]] + active = _active_by_id(config, plan.customer_id) + gone = [rec_id for rec_id in planned if rec_id not in active] + if gone: + raise ValueError( + "No longer active (dismissed, applied or expired since the " + "preview): " + ", ".join(gone) + ". Draft again." + ) + return {"recommendations_checked": len(planned), "still_active": len(planned)} + + +def _apply_dismiss_recommendations(client: object, cid: str, changes: dict) -> dict: + """Dismiss the planned recommendations; per-recommendation errors are reported.""" + from adloop.ads.write import _parse_partial_failure_per_op + + names = [r["resource_name"] for r in changes["recommendations"]] + service = client.get_service("RecommendationService") + request = client.get_type("DismissRecommendationRequest") + request.customer_id = cid + request.partial_failure = True + operation_type = type(request).DismissRecommendationOperation + for name in names: + request.operations.append(operation_type(resource_name=name)) + + response = service.dismiss_recommendation(request=request) + + pf_error = getattr(response, "partial_failure_error", None) + per_op_errors = _parse_partial_failure_per_op(client, pf_error) + dismissed: list[str] = [] + failed: list[dict] = [] + for index, result in enumerate(response.results): + if result.resource_name: + dismissed.append(result.resource_name) + else: + failed.append( + { + "recommendation_id": recommendation_id(names[index]), + "error": per_op_errors.get( + index, "Unknown error (see partial_failure_message)" + ), + } + ) + + out: dict = {"dismissed": dismissed, "dismissed_count": len(dismissed)} + if failed: + out["partial_failure"] = True + out["failed"] = failed + message = getattr(pf_error, "message", "") if pf_error is not None else "" + if message: + out["partial_failure_message"] = message + return out diff --git a/src/adloop/ads/write.py b/src/adloop/ads/write.py index 9cc6650..90acf56 100644 --- a/src/adloop/ads/write.py +++ b/src/adloop/ads/write.py @@ -2717,6 +2717,9 @@ def confirm_and_apply( is_reddit = plan.operation.startswith("reddit_") is_gtm = plan.operation.startswith("gtm_") is_ga4 = plan.operation in GA4_OPERATIONS + # Google Ads calls without a validate_only field: the dry run re-reads the + # target instead, like the platforms above. + is_ads_reread = plan.operation == "dismiss_recommendations" platform_label = ( "Reddit Ads" if is_reddit else "Google Tag Manager" if is_gtm @@ -2749,6 +2752,12 @@ def confirm_and_apply( from adloop.ga4.write import preflight as ga4_preflight preflight_checks = ga4_preflight(config, plan) + elif is_ads_reread: + # DismissRecommendation has no validate_only field, and the + # validate-only client refuses it; re-read the targets instead. + from adloop.ads.recommendations import preflight as rec_preflight + + preflight_checks = rec_preflight(config, plan) else: # Google Ads checks the exact mutates with validate_only=True # and executes nothing. @@ -2784,7 +2793,7 @@ def confirm_and_apply( ) checked_against = ( f"re-checked the target against {platform_label}" - if is_reddit or is_gtm or is_ga4 + if is_reddit or is_gtm or is_ga4 or is_ads_reread else "sent the change to Google Ads in validate-only mode" ) return { @@ -2835,7 +2844,9 @@ def confirm_and_apply( if preflight_checks is not None: response["checks"] = preflight_checks response["note"] = ( - f"{platform_label} has no validate-only mode: the dry run " + f"{platform_label} has no validate-only mode" + + (" for dismissing recommendations" if is_ads_reread else "") + + ": the dry run " "re-read the target and re-checked the safety gates; nothing " "was changed live." + ( @@ -4089,6 +4100,7 @@ def _dispatch_ads_plan(client: object, cid: str, plan: object) -> dict: from adloop.ads.pmax import apply_asset_group_assets as _apply_asset_group_assets from adloop.ads.pmax import apply_asset_group_signals as _apply_asset_group_signals from adloop.ads.pmax import apply_pmax_product_filter as _apply_pmax_product_filter + from adloop.ads.recommendations import _apply_dismiss_recommendations # Custom conversion goals live in their own module for the same reason. from adloop.ads.custom_conversion_goals import ( _apply_assign_custom_conversion_goal, @@ -4123,6 +4135,7 @@ def _dispatch_ads_plan(client: object, cid: str, plan: object) -> dict: "attach_brand_list_to_campaigns": _apply_attach_brand_list_to_campaigns, "detach_brand_list_from_campaigns": _apply_detach_brand_list_from_campaigns, "update_ai_max_settings": _apply_ai_max_settings, + "dismiss_recommendations": _apply_dismiss_recommendations, "create_custom_conversion_goal": _apply_create_custom_conversion_goal, "update_custom_conversion_goal": _apply_update_custom_conversion_goal, "assign_custom_conversion_goal": _apply_assign_custom_conversion_goal, diff --git a/src/adloop/rules/adloop.md b/src/adloop/rules/adloop.md index c14935f..5f0cda5 100644 --- a/src/adloop/rules/adloop.md +++ b/src/adloop/rules/adloop.md @@ -79,7 +79,7 @@ You have access to AdLoop MCP tools that connect Google Ads, Reddit Ads and Goog - `get_ad_performance` returns full `headlines` and `descriptions` lists for RSAs, plus `ad_group_ad.policy_summary.approval_status`, `review_status` and `policy_topics` (topic names only) per ad. Ads that are `DISAPPROVED` (cannot serve) or `APPROVED_LIMITED` (serving restricted) are listed in `policy_issues` with their spend and flagged in `insights[]`; compact mode adds `by_approval_status` counts. - `get_campaign_performance` returns `metrics.search_impression_share`, `metrics.search_budget_lost_impression_share` and `metrics.search_rank_lost_impression_share` as fractions (0.25 = 25%) with matching `*_pct` percentages. Google caps the reported values: 0.0999 means below 10% share, 0.9001 means above 90% lost. They are `null` for campaigns that do not serve on Search (Display, Video, PMax, ...) or had no impressions, which means "does not apply", not zero. `budget_limited_converters` and `insights[]` flag converting campaigns losing 20% or more of impressions to budget: those can absorb more budget, rank-lost share instead points at bids, ad relevance and quality. - `get_change_history` returns changes newest first with counts `by_resource_type`, `by_client`, `by_user` and `by_day`. Google Ads keeps 30 days of change history, so an older start date is moved forward (see `notes`). `insights[]` flag auto-applied recommendations and budget or bidding changes. `truncated: true` means the limit was hit; narrow the range or filters to see older changes. -- `get_recommendations` returns `estimated_improvement` per recommendation (potential minus base metrics) and `insights[]` that flag budget-increase recommendations, whose projected gain comes from spending more. +- `get_recommendations` returns each recommendation's `recommendation.id` (what `draft_dismiss_recommendations` takes) and `estimated_improvement` per recommendation (potential minus base metrics) and `insights[]` that flag budget-increase recommendations, whose projected gain comes from spending more. - PMax tools: `get_pmax_performance` returns `insights[]` flagging weak ad strength and zero-conversion asset groups. `segments.ad_network_type` includes MIXED — a Google catch-all for most PMax traffic. Full channel splits (Search vs YouTube vs Display vs Discover) are not available via the API. - `get_pmax_assets` returns `by_status` and `by_field_type` summaries. Note: per-asset performance labels (BEST/GOOD/LOW) are not available for PMax assets in the Google Ads API. Use `get_detailed_asset_performance` for quality signals via top combinations. - `get_audience_performance` works for campaigns with explicit audience targeting. PMax audience targeting is automatic and may not appear in this report. When the results include SEARCH campaigns, `insights[]` reminds you that custom segments cannot be attached to them (see the compatibility matrix below) — relay that constraint instead of proposing impossible pairings. @@ -317,6 +317,7 @@ Conversions that happen offline (a sales call, a lead that turned into a deal) g | `add_to_negative_keyword_list` | Append keywords to an EXISTING shared negative keyword list (does NOT add) | `shared_set_id` (from `get_negative_keyword_lists`), keyword list, `match_type` | | `attach_shared_set_to_campaigns` | Attach an EXISTING shared set (e.g. shared negative keyword list) to one or more campaigns. Use after creating a campaign to inherit pre-built negatives. | `shared_set_id` (from `get_negative_keyword_lists`), `campaign_ids` list | | `detach_shared_set_from_campaigns` | Detach a shared set from one or more campaigns. Removes only the linkage; the shared set and its keywords stay intact. | `shared_set_id`, `campaign_ids` list | +| `draft_dismiss_recommendations` | Dismiss Google recommendations the user has decided not to take. Nothing in the account changes and Google can show them again later; unknown or already dismissed IDs are refused. No validate-only mode: the dry run re-reads the account and checks each one is still active | `recommendation_ids` (`recommendation.id` from `get_recommendations`) | | `draft_demographic_targeting` | Propose demographic criteria (age, gender, parental status, income range) at ad group or campaign level. Defaults to EXCLUSION (`negative=True`). | exactly one of `ad_group_id` or `campaign_id`, at least one of `age_ranges`/`genders`/`parental_statuses`/`income_ranges`, optional `negative` (default True) | | `draft_audience_attachment` | Attach audiences to a campaign or ad group as OBSERVATION (default: bids and reporting, reach unchanged), TARGETING (ads only reach the audiences; double confirmation) or exclusion. Refuses incompatible pairings, Search lists too small to target, audiences at both campaign and ad group level, and Performance Max | exactly one of `campaign_id` or `ad_group_id`; any of `user_list_ids`, `segment_ids`, `custom_audience_ids`, `combined_audience_ids`; `mode` observation/targeting; `exclude` | | `draft_ai_max_settings` | Draft AI Max controls for one Search campaign: `enable_ai_max`, per-ad-group `disable_search_term_matching`, text and final-URL asset automation. Refuses a plan that would switch AI Max on while an automation it was never told about keeps running; an explicitly requested automation asks for a double confirmation. | `campaign_id`, optional `enable_ai_max`, optional `disable_search_term_matching`, optional `ad_group_ids`, optional `include_paused_ad_groups`, optional `text_asset_automation`, optional `final_url_expansion` | @@ -657,6 +658,7 @@ Reddit's structure is campaign → ad group (budget, bid, pixel, targeting) → 4. Use `estimated_improvement` to prioritize: recommendations with >1 estimated additional conversion are worth investigating 5. For keyword recommendations, call `get_keyword_performance` to check existing keyword overlap before acting 6. Present recommendations grouped by type with your assessment of each — the user should understand which ones are genuinely helpful vs which ones just increase Google's revenue +7. For the ones the user decides not to take, offer `draft_dismiss_recommendations` with their IDs, so they stop reappearing. Dismiss only what the user turned down: a dismissal is not a verdict you make on their behalf ### When user asks about audience performance or targeting diff --git a/src/adloop/server.py b/src/adloop/server.py index 76f29ef..b069489 100644 --- a/src/adloop/server.py +++ b/src/adloop/server.py @@ -1546,10 +1546,12 @@ def get_recommendations( ) -> dict: """Retrieve Google's auto-generated recommendations with estimated impact. - Returns each recommendation's type, associated campaign/ad group, current - (base) and projected (potential) metrics, and the estimated improvement. - Includes insights that flag budget-increase recommendations (their projected - gain comes from spending more) and highlight high-impact suggestions. + Returns each recommendation's ID, type, associated campaign/ad group, + current (base) and projected (potential) metrics, and the estimated + improvement. Includes insights that flag budget-increase recommendations + (their projected gain comes from spending more) and highlight high-impact + suggestions. The `recommendation.id` is what draft_dismiss_recommendations + takes. Args: customer_id: Google Ads customer ID, digits with or without dashes @@ -1570,6 +1572,44 @@ def get_recommendations( ) +@_tool(title="Draft dismissing recommendations", annotations=_WRITE, tags={"ads"}) +@_safe +def draft_dismiss_recommendations( + recommendation_ids: _StrList, + customer_id: str = "", +) -> dict: + """Dismiss Google Ads recommendations — returns a PREVIEW. + + Turns down recommendations you will not take (e.g. "add broad match" on a + deliberately narrow account) so they stop reappearing in + get_recommendations. Nothing in the account changes, and Google can show a + dismissed recommendation again later, so a single confirmation is enough. + + Each ID must be an active recommendation: unknown or already dismissed + IDs are refused by name. The preview lists each one's type, campaign and + estimated improvement, so the user sees what they are turning down. + + DismissRecommendation has no validate-only mode: the dry run re-reads the + account and checks every recommendation is still active instead of + sending the call to Google. The returned plan_id is applied with + confirm_and_apply. + + Args: + recommendation_ids: `recommendation.id` values from + get_recommendations, or full resource names + (customers/{customer_id}/recommendations/{id}). Duplicates are + collapsed. + customer_id: Ads account ID. Defaults to the configured account. + """ + from adloop.ads.recommendations import draft_dismiss_recommendations as _impl + + return _impl( + current_config(), + customer_id=customer_id or current_config().ads.customer_id, + recommendation_ids=recommendation_ids, + ) + + @_tool(title="Performance Max performance", annotations=_READONLY, tags={"ads"}) @_safe def get_pmax_performance( diff --git a/tests/test_recommendations_dismiss.py b/tests/test_recommendations_dismiss.py new file mode 100644 index 0000000..cadd4cb --- /dev/null +++ b/tests/test_recommendations_dismiss.py @@ -0,0 +1,314 @@ +"""Tests for draft_dismiss_recommendations and its dry run / apply.""" + +from __future__ import annotations + +from types import SimpleNamespace + +import pytest +from google.ads.googleads.client import GoogleAdsClient + +from adloop.ads import recommendations as rec +from adloop.ads import write +from adloop.ads.client import GOOGLE_ADS_API_VERSION +from adloop.config import AdLoopConfig, AdsConfig, SafetyConfig + +CUSTOMER = "1234567890" +BROAD = "3401876612" +BUDGET = "3401876613" +KEYWORD = "3401876614" + + +def _resource(rec_id: str, customer: str = CUSTOMER) -> str: + return f"customers/{customer}/recommendations/{rec_id}" + + +def _row(rec_id: str, rec_type: str, campaign: str = "11") -> dict: + """One row as get_recommendations returns it.""" + return { + "recommendation.id": rec_id, + "recommendation.resource_name": _resource(rec_id), + "recommendation.type": rec_type, + "recommendation.campaign": f"customers/{CUSTOMER}/campaigns/{campaign}", + "recommendation.ad_group": "", + "recommendation.dismissed": False, + "estimated_improvement": {"clicks": 40.0, "cost": 120.5, "conversions": 0.4}, + } + + +ACTIVE = [ + _row(BROAD, "USE_BROAD_MATCH_KEYWORD"), + _row(BUDGET, "CAMPAIGN_BUDGET"), + _row(KEYWORD, "KEYWORD", campaign="12"), +] + + +@pytest.fixture +def config() -> AdLoopConfig: + return AdLoopConfig(ads=AdsConfig(customer_id=CUSTOMER)) + + +@pytest.fixture +def active(monkeypatch): + """Patch the live read; tests edit the list to simulate changes.""" + rows = list(ACTIVE) + calls: list[str] = [] + + def fake_get_recommendations(_config, *, customer_id="", **_kwargs): + calls.append(customer_id) + return {"recommendations": list(rows)} + + monkeypatch.setattr("adloop.ads.read.get_recommendations", fake_get_recommendations) + return SimpleNamespace(rows=rows, calls=calls) + + +# --------------------------------------------------------------------------- +# Draft +# --------------------------------------------------------------------------- + + +class TestDraft: + def test_preview_names_each_recommendation_with_its_impact(self, config, active): + result = rec.draft_dismiss_recommendations( + config, recommendation_ids=[BROAD, BUDGET] + ) + + assert result["operation"] == "dismiss_recommendations" + assert result["requires_double_confirm"] is False + planned = result["changes"]["recommendations"] + assert [r["recommendation_id"] for r in planned] == [BROAD, BUDGET] + assert planned[0]["type"] == "USE_BROAD_MATCH_KEYWORD" + assert planned[0]["resource_name"] == _resource(BROAD) + assert planned[0]["campaign"].endswith("/campaigns/11") + assert planned[0]["estimated_improvement"]["clicks"] == 40.0 + assert result["changes"]["count"] == 2 + assert "show a dismissed recommendation again" in result["changes"]["note"] + + def test_accepts_resource_names_and_collapses_duplicates(self, config, active): + result = rec.draft_dismiss_recommendations( + config, recommendation_ids=[_resource(KEYWORD), KEYWORD, " ", KEYWORD] + ) + + assert [r["recommendation_id"] for r in result["changes"]["recommendations"]] == [ + KEYWORD + ] + + def test_unknown_or_already_dismissed_ids_are_refused_by_name(self, config, active): + result = rec.draft_dismiss_recommendations( + config, recommendation_ids=[BROAD, "999"] + ) + + assert result["error"] == "Validation failed" + assert "999" in result["details"][0] + assert "already dismissed" in result["details"][0] + + def test_a_resource_name_from_another_account_is_refused(self, config, active): + result = rec.draft_dismiss_recommendations( + config, recommendation_ids=[_resource(BROAD, customer="5555555555")] + ) + + assert result["error"] == "Validation failed" + assert "5555555555" in result["details"][0] + assert active.calls == [], "nothing may be read for invalid input" + + @pytest.mark.parametrize("bad", ["abc def", "1/2", "x' OR 1=1", "customers/1/campaigns/2"]) + def test_malformed_ids_are_refused_before_any_read(self, config, active, bad): + result = rec.draft_dismiss_recommendations(config, recommendation_ids=[bad]) + + assert result["error"] == "Validation failed" + assert active.calls == [] + + def test_an_empty_list_is_refused(self, config, active): + result = rec.draft_dismiss_recommendations(config, recommendation_ids=[]) + + assert result == { + "error": "Validation failed", + "details": ["At least one recommendation_id is required"], + } + + def test_blocked_operation_is_refused(self, active): + config = AdLoopConfig( + ads=AdsConfig(customer_id=CUSTOMER), + safety=SafetyConfig(blocked_operations=["dismiss_recommendations"]), + ) + + result = rec.draft_dismiss_recommendations(config, recommendation_ids=[BROAD]) + + assert "blocked" in result["error"] + + +# --------------------------------------------------------------------------- +# Dry run +# --------------------------------------------------------------------------- + + +class TestDryRun: + def test_dry_run_rereads_instead_of_sending_to_google(self, config, active, monkeypatch): + def never(*_args, **_kwargs): + raise AssertionError("DismissRecommendation has no validate-only mode") + + monkeypatch.setattr(write, "_validate_with_google", never) + plan_id = rec.draft_dismiss_recommendations( + config, recommendation_ids=[BROAD, BUDGET] + )["plan_id"] + + result = write.confirm_and_apply(config, plan_id=plan_id, dry_run=True) + + assert result["status"] == "DRY_RUN_SUCCESS" + assert result["checks"] == {"recommendations_checked": 2, "still_active": 2} + assert "no validate-only mode for dismissing recommendations" in result["note"] + assert len(active.calls) == 2 # draft + dry run + + def test_dry_run_fails_when_a_recommendation_went_away(self, config, active): + plan_id = rec.draft_dismiss_recommendations( + config, recommendation_ids=[BROAD, BUDGET] + )["plan_id"] + active.rows[:] = [r for r in active.rows if r["recommendation.id"] != BUDGET] + + result = write.confirm_and_apply(config, plan_id=plan_id, dry_run=True) + + assert result["status"] == "DRY_RUN_FAILED" + assert BUDGET in result["error"] + assert BROAD not in result["error"] + assert "re-checked the target against Google Ads" in result["message"] + + def test_the_validate_only_client_still_refuses_the_call(self): + """Fail closed: even if a dry run reached the applier, nothing is dismissed.""" + from adloop.ads.validate_only import ValidateOnlyClient, ValidateOnlyFailure + + dismissed: list = [] + inner = SimpleNamespace( + get_service=lambda _name: SimpleNamespace( + dismiss_recommendation=lambda **kw: dismissed.append(kw) + ) + ) + service = ValidateOnlyClient(inner).get_service("RecommendationService") + + with pytest.raises(ValidateOnlyFailure, match="no validate-only mode"): + service.dismiss_recommendation(request=None) + assert dismissed == [] + + +# --------------------------------------------------------------------------- +# Apply +# --------------------------------------------------------------------------- + + +class _DismissClient: + """Real proto types, a fake RecommendationService.""" + + def __init__(self, *, fail_index: int | None = None): + base = GoogleAdsClient( + credentials=None, developer_token="test-token", + use_proto_plus=True, version=GOOGLE_ADS_API_VERSION, + ) + self.enums = base.enums + self.get_type = base.get_type + self.request = None + self._fail_index = fail_index + + def get_service(self, name): + assert name == "RecommendationService" + return SimpleNamespace(dismiss_recommendation=self._dismiss) + + def _dismiss(self, request=None): + self.request = request + results = [ + SimpleNamespace(resource_name="" if i == self._fail_index else op.resource_name) + for i, op in enumerate(request.operations) + ] + error = ( + SimpleNamespace(code=3, message="Recommendation not found.", details=[]) + if self._fail_index is not None + else None + ) + return SimpleNamespace(results=results, partial_failure_error=error) + + +def _changes(*ids: str) -> dict: + return { + "recommendations": [ + {"recommendation_id": i, "resource_name": _resource(i)} for i in ids + ] + } + + +class TestApply: + def test_sends_one_partial_failure_request_with_every_resource_name(self): + client = _DismissClient() + + result = rec._apply_dismiss_recommendations(client, CUSTOMER, _changes(BROAD, BUDGET)) + + assert client.request.customer_id == CUSTOMER + assert client.request.partial_failure is True + assert [op.resource_name for op in client.request.operations] == [ + _resource(BROAD), _resource(BUDGET) + ] + assert result == { + "dismissed": [_resource(BROAD), _resource(BUDGET)], + "dismissed_count": 2, + } + + def test_a_failed_operation_is_reported_by_recommendation_id(self): + client = _DismissClient(fail_index=1) + + result = rec._apply_dismiss_recommendations(client, CUSTOMER, _changes(BROAD, BUDGET)) + + assert result["dismissed"] == [_resource(BROAD)] + assert result["partial_failure"] is True + assert result["failed"][0]["recommendation_id"] == BUDGET + assert result["partial_failure_message"] == "Recommendation not found." + + def test_the_operation_is_dispatched(self): + client = _DismissClient() + plan = SimpleNamespace( + operation="dismiss_recommendations", + apply_payload=lambda: _changes(KEYWORD), + ) + + result = write._dispatch_ads_plan(client, CUSTOMER, plan) + + assert result["dismissed_count"] == 1 + + +# --------------------------------------------------------------------------- +# get_recommendations exposes the ID +# --------------------------------------------------------------------------- + + +class TestReadExposesId: + def test_rows_carry_id_and_resource_name(self, config, monkeypatch): + from adloop.ads import read + + metrics = SimpleNamespace(impressions=0, clicks=0, cost_micros=0, conversions=0) + proto = SimpleNamespace( + recommendation=SimpleNamespace( + resource_name=_resource(BROAD), + type_=SimpleNamespace(name="USE_BROAD_MATCH_KEYWORD"), + campaign=f"customers/{CUSTOMER}/campaigns/11", + ad_group="", + dismissed=False, + impact=SimpleNamespace(base_metrics=metrics, potential_metrics=metrics), + ) + ) + client = SimpleNamespace( + get_service=lambda _name: SimpleNamespace(search=lambda **_kw: [proto]) + ) + monkeypatch.setattr("adloop.ads.client.get_ads_client", lambda _config: client) + + rows = read.get_recommendations(config, customer_id=CUSTOMER)["recommendations"] + + assert rows[0]["recommendation.id"] == BROAD + assert rows[0]["recommendation.resource_name"] == _resource(BROAD) + + +class TestToolRegistration: + @pytest.mark.asyncio + async def test_write_annotations_and_documented_parameters(self): + from adloop.server import mcp + + tools = {t.name: t for t in await mcp.list_tools()} + tool = tools["draft_dismiss_recommendations"] + assert tool.annotations.read_only_hint is False + assert tool.annotations.destructive_hint is False + assert tool.parameters["properties"]["recommendation_ids"].get("description") + assert "recommendation.id" in tools["get_recommendations"].description