Skip to content

feat(demographics): add age/gender/parental/income targeting tools - #38

Merged
kLOsk merged 4 commits into
kLOsk:mainfrom
chrmoller:feat/demographic-targeting
Jul 10, 2026
Merged

feat(demographics): add age/gender/parental/income targeting tools#38
kLOsk merged 4 commits into
kLOsk:mainfrom
chrmoller:feat/demographic-targeting

Conversation

@chrmoller

Copy link
Copy Markdown
Contributor
  • get_demographic_targeting (read): list existing demographic criteria on an ad group or campaign, with a composite remove_id ready for remove_entity
  • draft_demographic_targeting (write): propose exclusion (default) or positive demographic criteria. Accepts canonical enums and human-readable aliases (e.g. "25-34", "female", "top-10"). Surfaces warnings for near-total-dimension exclusions and positive narrowing.
  • _apply_add_demographic_criteria: applies AGE_RANGE / GENDER / PARENTAL_STATUS / INCOME_RANGE criteria at ad-group or campaign level
  • remove_entity: accepts ad_group_criterion and campaign_criterion as semantic aliases so removals read sensibly in plans and audit logs
  • Orchestration rules document the bucket mismatch case (e.g. "23-35" has no exact mapping) and the audience-vs-demographic distinction

chrmoller and others added 4 commits May 22, 2026 10:27
- get_demographic_targeting (read): list existing demographic criteria on
  an ad group or campaign, with a composite remove_id ready for remove_entity
- draft_demographic_targeting (write): propose exclusion (default) or
  positive demographic criteria. Accepts canonical enums and human-readable
  aliases (e.g. "25-34", "female", "top-10"). Surfaces warnings for
  near-total-dimension exclusions and positive narrowing.
- _apply_add_demographic_criteria: applies AGE_RANGE / GENDER /
  PARENTAL_STATUS / INCOME_RANGE criteria at ad-group or campaign level
- remove_entity: accepts ad_group_criterion and campaign_criterion as
  semantic aliases so removals read sensibly in plans and audit logs
- Orchestration rules document the bucket mismatch case (e.g. "23-35"
  has no exact mapping) and the audience-vs-demographic distinction

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The draft_demographic_targeting tool used raw list[str] | None for its
four demographic-list parameters, which skipped the project's
JSON-string-to-list coercion (the _StrListOpt validator introduced in
kLOsk#28). MCP clients that pre-serialize list params as JSON strings —
e.g. age_ranges="[\"25-34\"]" — hit Pydantic's list_type error.

Switch all four to _StrListOpt to match the convention used by every
other list-accepting tool in server.py (geo_target_ids, language_ids,
keywords, callouts, image_paths, campaign_ids, etc.). Bare-scalar
inputs like "65+" still fail by design — clients must send a real
array or a JSON-encoded list.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Same root cause as the draft_demographic_targeting fix
(642d870): the headlines and descriptions params used raw
list[str | dict] instead of going through the project's
JSON-string-to-list coercion validator.

Add a new _StrOrDictList alias (no existing one fits because the
RSA params accept mixed string-or-dict entries for unpinned vs
pinned assets) and apply it. Add a regression test that submits
JSON-encoded list strings — the same failure mode users would
hit with pre-serializing MCP clients.

Audit complete: every other list-accepting MCP tool in server.py
already uses _StrList/_StrListOpt/_DictList/_DictListOpt, so this
is the last remaining instance.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The four demographic list params (age_ranges, genders,
parental_statuses, income_ranges) were declared as
`_StrListOpt = None`, which Pydantic encodes as
`{"anyOf": [{"type": "array"}, {"type": "null"}], "default": null}`.

That's spec-correct but some MCP clients only inspect the top-level
`type` field and never traverse `anyOf`, so they see no `type: array`
and refuse to serialize a list. Practical symptom: clients send the
list as a bare string and the Pydantic validator rejects it.

Switch to `_StrList = []` (with the same noqa: B006 comment used by
discover_keywords' seed_keywords). The resulting schema is flat:
`{"type": "array", "items": {...}, "default": []}` — every client
sees `type: array` and serializes correctly. The "at least one
dimension must be non-empty" validation already works with `[]` as
the empty sentinel (no change needed).

This is a targeted fix; the same pattern exists on other tools
(`get_recommendations.recommendation_types`,
`attribution_check.conversion_events`, etc.) and would benefit
from the same cleanup — left for a follow-up.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@kLOsk

kLOsk commented Jul 7, 2026

Copy link
Copy Markdown
Owner

This is exactly how contributions to this repo should look: full preview/confirm compliance, real tests, and both rules files updated. Three asks before merge: (1) rebase onto v0.10.0, the module-level _config is gone, use adloop.runtime.current_config() (mechanical, 4 spots); (2) add a numeric guard on ad_group_id/campaign_id before GAQL interpolation in get_demographic_targeting; (3) campaign-level positive demographic criteria may be rejected by the API (the UI only offers exclusions there), so either verify it works or force negative=True when campaign_id is set. Happy to merge right after.

kLOsk added a commit that referenced this pull request Jul 10, 2026
PR #38 predated the multi-tenant runtime refactor; both new tools
referenced the removed module-level _config global (same class of bug
as 2bdd92e — caught by the static regression guard this time). Also
drop the duplicate _FakeCampaignCriterionService the merge brought in.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EvgwTbTqEinTJXNriN5ga1
@kLOsk
kLOsk merged commit 98831d2 into kLOsk:main Jul 10, 2026
3 checks passed
@kLOsk

kLOsk commented Jul 10, 2026

Copy link
Copy Markdown
Owner

Merged to main — thank you @chrmoller! This is the reference-quality contribution for this repo: full preview/confirm compliance, real MCP-schema-level tests (a pattern we'll reuse), expert rules-file guidance, and it even fixed main's docs-vs-code gap on campaign_criterion removal and the RSA JSON-coercion hole.

Rather than another review round-trip, I applied the three asks from my last comment maintainer-side during the merge, plus one extra guard:

  1. Ported both tools to current_config() (v0.10 runtime).
  2. Numeric validation on ad_group_id/campaign_id before GAQL interpolation.
  3. Campaign-level positive criteria now rejected at preview time with a pointer to ad groups (the API only supports exclusions at campaign level).
  4. New: excluding UNDETERMINED now warns — it silently blocks every user Google can't classify, often 30%+ of impressions.

Ships in the next release as tools #48–49. Hope to see more from you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants