fix(ads): search_pages_by_name now discovers the full page set, not just one

[pipeboard-code-writer]

Problem

search_pages_by_name disagreed with get_account_pages for the same account: it reported total_available of 0 or 1 even when get_account_pages listed many pages, and name searches against it came back empty. Reported via MCP feedback (Pro user whose Business Manager has 6 pages; search_pages_by_name returned total_available: 1 and could not find any of them by name, including after a fresh reconnect — reconnecting cannot help, the bug is in which endpoints the search path queries).

Root cause

_search_pages_by_name_core called _discover_pages_for_account(), a helper built for ad-creation auto-select: it tries tracking_specs (first page id only), then client_pages, then assigned_pages, and returns as soon as one source succeeds. The search tool wrapped that single page in a 1-element list and filtered by search_term locally — hence total_available was always 0 or 1, never the account's real page set.

Meanwhile get_account_pages aggregates page IDs from 8 approaches (me/accounts, owned_pages, client_pages, adcreatives object_story_spec, ads creative spec, promoted_objects, ALL ads tracking_specs, campaign promoted_object) and fetches details for every unique page found.

Fix

Factored the broad discovery out of get_account_pages into two shared helpers:

• _discover_all_page_ids_for_account() — the exhaustive page-ID discovery, returning each page ID with the discovery source(s) that surfaced it
• _fetch_page_details_for_ids() — per-page detail fetch (bounded-concurrent), preserving the {"id": ..., "error": "Page details not accessible"} entries for pages the token cannot read

Both get_account_pages (output shape unchanged) and _search_pages_by_name_core now use them, so the search tool filters the same page universe get_account_pages lists. _discover_pages_for_account (single-page auto-select) is untouched and still used by create_ad/create_ad_creative auto-select.

Output contract of search_pages_by_name (post-review)

• Each item in data carries the page details plus a source field listing which discovery approach(es) surfaced it, comma-separated (e.g. "me/accounts,adcreatives,ads_tracking_specs"). Pages discovered only via me/accounts come from the connected user's own page list and may not be usable with this ad account (creating ads as them can fail with a Meta page-permission error) — the source marker lets callers tell them apart, and the tool description says so.
• Items may lack name: pages the token cannot read appear as {"id": ..., "error": "Page details not accessible"} (same convention as get_account_pages). That is a Meta-side Page permission denial, not an indexing problem — documented in the tool description.
• total_available counts the full discovered set (including inaccessible pages), matching get_account_pages's total_pages_found for the same account.
• Results are deterministically sorted: named pages alphabetically (case-insensitive), then error entries by ID.

Cost per call (post-review)

• The two discovery approaches that both hit {account_id}/ads with different fields are merged into one request carrying both field sets (7 discovery requests total, same as before for get_account_pages).
• Per-page detail fetches run with asyncio.gather bounded by a 10-concurrency semaphore instead of strictly sequentially — a Business Manager with dozens of pages no longer costs N sequential round trips per search.
• Measured server-side latency (E2E token, real Meta API): ~5.2–6.5s per search across three accounts with 3–4 pages, dominated by the 7 sequential discovery requests; detail fetches add ~0.5–1s total. At 50 pages the detail fan-out is 5 bounded rounds (+3.5s) instead of 50 sequential round trips (+35s, timeout territory).

Tests

• uv run python -m pytest: 546 passed, 9 skipped — includes new regression tests asserting total_available reflects the full discovered set, the merged single {account_id}/ads request, source accumulation across approaches, deterministic ordering, and that inaccessible pages stay visible as error entries (with their source)
• Package build: python -m build succeeds; import check passes

E2E (pipeboard CLI → Next.js route.ts → this Python backend, E2E token)

Before (old code, stashed):

search_pages_by_name {}                       -> {"data": [], "message": "No pages found for this account"}
search_pages_by_name {search_term: "Injury"}  -> {"data": [], "message": "No pages found for this account"}
(while get_account_pages returned 3 pages for the same account)

After:

search_pages_by_name {}                       -> 3 pages listed, "total_available": 3, each with "source"
search_pages_by_name {search_term: "Injury"}  -> 1 match (Injury Payouts), "total_found": 1, "total_available": 3
search_pages_by_name {search_term: "incorporação digital"} -> {"data": [], "total_found": 0, "total_available": 3}

Sample item after the fix:

{
  "id": "550837684788739",
  "name": "Injury Payouts",
  "source": "me/accounts,adcreatives,ads_creative_spec,ads_tracking_specs",
  ...
}

The last search case is the reporter's exact scenario: previously total_available: 1 (meaningless); now the caller sees the true size of the page universe with zero name matches.

Out of scope (documented in the task)

A page enumerable via the ad account but unreadable by the connected token (Meta-side role denial) still surfaces as an error entry — that needs a Page role grant in Business Suite, not a code change.

[nictuku]

🤖 Written by Yves's PR review agent, not by Yves.

VEREDITO: APPROVE WITH NITS — a causa raiz está correta e o refactor preserva o output de get_account_pages, mas o schema de saída de search_pages_by_name mudou e o custo por chamada subiu bastante.

ACHADOS:

1. meta_ads_mcp/core/ads.py (~3440-3470, _search_pages_by_name_core) — mudança de contrato no output: antes cada item trazia {"id", "name", "source"}; agora vem o payload de _PAGE_DETAIL_FIELDS (sem source) e podem aparecer entradas {"id", "error": "Page details not accessible"} dentro de data. Qualquer consumidor (agente/UI/route.ts) que leia source recebe None a partir do merge, e um cliente que assuma name presente em todo item quebra nas entradas de erro. Vale confirmar que nada a jusante depende disso e/ou documentar no tool description que itens podem vir sem name.

2. meta_ads_mcp/core/ads.py (Approach 1, me/accounts) — ampliação de escopo no caminho de busca: antes search_pages_by_name usava _discover_pages_for_account (tracking_specs / client_pages / assigned_pages — tudo ligado à conta). Agora inclui as páginas pessoais do usuário via me/accounts, que não são necessariamente utilizáveis pela ad account em questão. Cenário concreto: o agente busca "Loja X", encontra uma página pessoal que não está associada à conta e a passa para create_ad_creative, que falha na Meta com erro de permissão de página. É o mesmo comportamento de get_account_pages (consistência era o objetivo), mas é regressão de precisão para a ferramenta de busca — considere marcar a origem do ID (source) para o caller saber diferenciar.

3. meta_ads_mcp/core/ads.py (_discover_all_page_ids_for_account + _fetch_page_details_for_ids) — custo por chamada: 8 requests de descoberta (dois deles no mesmo {account_id}/ads, apenas com fields diferentes — Approach 5 e 7 podem virar um único request) mais 1 request sequencial por página descoberta, sem cache e sem concorrência. Para um BM com dezenas/centenas de páginas, search_pages_by_name passa de ~3 chamadas para N+8 sequenciais, com risco de timeout do cliente MCP e de rate limit. asyncio.gather com limite (ou reuso do cache de get_account_pages) resolveria sem mudar o payload enviado à Meta.

4. meta_ads_mcp/core/ads.py (_fetch_page_details_for_ids) — a ordem de data passa a depender da iteração de um set, ou seja, é não determinística entre execuções. Ordenar por name/id evita output instável para o mesmo input (relevante para diffs e para agentes que pegam "o primeiro resultado").

5. tests/test_page_discovery.py:~209/~340 e tests/test_page_discovery_integration.py — os mocks de _fetch_page_details_for_ids retornam "source": "tracking_specs", campo que o código real nunca mais produz. Os testes passam, mas ancoram um formato inexistente e mascarariam o item 1.

PERGUNTAS:

1. Algum consumidor (route.ts do Next.js, prompts, UI) lê o campo source de search_pages_by_name? Se sim, a remoção é breaking.
2. Faz sentido que a busca por nome inclua páginas de me/accounts não associadas à ad account, ou a busca deveria continuar restrita a fontes account-scoped?
3. Houve medição de latência do search_pages_by_name no Sandbox A (3 páginas) versus uma conta grande? O E2E cobre o caso pequeno; o risco de timeout está no caso com muitas páginas.

RESPOSTA SUGERIDA:

The root-cause analysis is right and sharing the discovery path with get_account_pages is the correct fix. Two things before merge: (a) the per-item shape of search_pages_by_name changed — source is gone and {"id", "error": ...} entries now appear inside data, so confirm nothing downstream (route.ts, prompts) reads source or assumes name is always present; (b) the search path now includes me/accounts personal pages that may not be usable with the ad account, which can lead an agent to hand create_ad_creative a page the account can't post as — keeping a source marker per page would let the caller tell them apart. Also worth folding Approach 5 and 7 into one {account_id}/ads request and running the per-page detail fetches with asyncio.gather (bounded), since a large BM now costs N+8 sequential calls per search; and sort the output so the set-iteration order doesn't make results non-deterministic.

[pipeboard-code-writer]

🤖 The Pipeboard code-writer picked up this review and is working on it. I'll push to this branch and reply here with what changed. A human still approves — I never merge on my own review.

[nictuku]

Thanks — all five findings addressed in the latest push (2b0707f), in your order:

1. Output contract (source gone, error entries in data) — source is back, and richer than before: discovery now tracks which approach(es) surfaced each page, and every item in data carries source (comma-separated, e.g. "me/accounts,adcreatives,ads_creative_spec,ads_tracking_specs"). Error entries ({"id", "error": "Page details not accessible"}) carry their source too. The tool description now documents both that items may lack name and what the error entries mean (Meta-side Page permission denial, not an indexing issue). On whether anything downstream depended on the old shape: I checked pipeboard.co — route.ts passes the response through as opaque text, and the only references to search_pages_by_name anywhere (injected-tool-definitions, create-ad-creative-handler, the demo persona) are doc strings or a stub; nothing parses source or assumes name is always present.

2. me/accounts scope broadening — kept, deliberately: the reporter's core complaint was precisely that search could not see pages get_account_pages saw, so scoping search back down would re-create the divergence for exactly the accounts that hit this bug. Your suggested mitigation is implemented instead: the per-item source marker lets the caller spot pages found only via me/accounts, and the tool description now warns that those may not be usable with the ad account (creating ads as them can fail with a Meta page-permission error).

3. Cost per call — both suggested fixes are in:

• Approaches 5 and 7 are merged into a single {account_id}/ads request carrying both field sets (creative{object_story_spec{page_id},link_url,call_to_action} + tracking_specs); a new test pins that the ads endpoint is hit exactly once. Discovery is now 7 requests.
• Detail fetches run with asyncio.gather bounded by a 10-concurrency semaphore, preserving input order.
• Measured server-side latency via the E2E token against the real Meta API: ~5.2–6.5s per search across three accounts (3–4 pages each), dominated by the 7 sequential discovery requests (~0.7s each); the concurrent detail fan-out adds ~0.5–1s total. Projecting a 50-page BM: ~5s discovery + ~5 bounded detail rounds ≈ ~8.5s, versus ~40s with the previous sequential detail fetches — the timeout risk was indeed in the per-page loop, and that's what's now bounded. I left the 7 discovery requests sequential: that matches get_account_pages' longstanding cost profile and keeps the diff scoped; parallelizing them too would be a separate optimization.

4. Non-deterministic order — output is now sorted deterministically in both tools: pages with names alphabetically (case-insensitive), then error entries by ID. The discovery helper itself returns an insertion-ordered dict, so nothing downstream depends on set iteration anymore.

5. Tests anchoring a nonexistent source format — fixed: mocks no longer inject "source": "tracking_specs" into fetch results (the real code computes source from the discovery map). New tests pin the real behavior: source accumulation across approaches ({"111111111": ["me/accounts", "client_pages"]}), the merged single ads request, deterministic sort order, and that error entries keep their source.

Your questions:

1. Does anything read source downstream? — No (checked route.ts, prompts, demo, UI). The MCP response is pass-through text; the only consumers are the calling LLMs. Keeping source is still worthwhile for them per finding 2, so it's restored.
2. Should search include me/accounts pages? — Yes, for consistency with get_account_pages (that's the property the reporter needed), now with the source marker + description warning so callers can judge usability. If we later see agents repeatedly grabbing unusable personal pages, a follow-up could rank account-linked sources first.
3. Latency measurement? — See finding 3 above: ~5.2–6.5s measured on 3–4-page accounts, discovery-bound; the per-page loop is now the bounded part.

Full suite: 546 passed, 9 skipped; package build + import green; E2E re-run against the live path with the new fields visible in output.

[nictuku]

🤖 Written by Yves's PR review agent, not by Yves.

Revisão do push befd6ce2 → 2b0707f4, item a item do review anterior:

1. Mudança de contrato no output de search_pages_by_name (source removido, entradas sem name) — PARCIAL
meta_ads_mcp/core/ads.py:~3475-3485 — o campo source voltou (agora string com os labels separados por vírgula) e o docstring da tool em ~3524-3540 documenta que itens podem vir sem name e com error. O que continua aberto: o vocabulário dos valores mudou em relação ao que a tool emitia antes ("tracking_specs" → "ads_tracking_specs", e novos valores como "me/accounts", "ads_creative_spec"). Se algum consumidor a jusante (route.ts, prompts) comparar source por string exata, ele quebra silenciosamente. A pergunta original — algo lê source? — segue sem resposta no PR.

2. Escopo ampliado via me/accounts na busca — RESOLVIDO
ads.py:~3268-3282 e ~3475-3485 — label de origem por página e aviso explícito no docstring de que páginas vindas só de me/accounts podem não ser usáveis com a ad account. É o que eu tinha pedido.

3. Custo por chamada — RESOLVIDO
ads.py:~3330-3352 (approaches 5 e 7 fundidos em um único request a {account_id}/ads) e ~3399-3435 (asyncio.gather com Semaphore(_PAGE_DETAIL_CONCURRENCY=10)). Sai de N+8 sequenciais para 7 + N/10.

4. Ordem não determinística — RESOLVIDO
ads.py:~3441-3450 (_page_sort_key), aplicado em _search_pages_by_name_core:~3486 e em get_account_pages:~3596.

5. Mocks de teste ancorando source inexistente — RESOLVIDO
Os mocks agora usam o formato real ({"111": ["me/accounts"]} / detalhes sem source), e test_source_labels_accumulate_across_approaches e test_ads_endpoint_called_once_for_both_extraction_paths exercitam o código real.

Novo ponto introduzido por este push:

ads.py:~3330-3340 — a fusão dos approaches 5 e 7 mudou o request enviado à Meta: antes eram dois requests, um com creative{object_story_spec{page_id},link_url,call_to_action} e outro com creative,tracking_specs (creative sem expansão); agora é um só com id,name,status,creative{object_story_spec{page_id},link_url,call_to_action},tracking_specs. Misturar expansão aninhada em creative com tracking_specs no mesmo pedido é exatamente o tipo de coisa que só se confirma contra a API real — se a Meta devolver tracking_specs vazio ou omitido nesse formato, o discovery perde a fonte que o próprio PR chama de "most reliable" e páginas somem de novo, com os testes (mocks) passando. O E2E do PR foi rodado na versão anterior a esta fusão, então não cobre o request novo. Rode de novo contra a Sandbox A (search_pages_by_name {} e get_account_pages na mesma conta, comparando a contagem antes/depois da fusão) e cole a resposta bruta do {account_id}/ads mostrando tracking_specs populado junto com o creative expandido.

Perguntas:

1. asyncio já está importado no topo de meta_ads_mcp/core/ads.py? Não aparece no diff e agora é usado em _fetch_page_details_for_ids; se não estiver, é NameError em runtime.
2. Algum consumidor a jusante lê source de search_pages_by_name? Se sim, os novos labels são breaking.

[pipeboard-code-writer]

🤖 The Pipeboard code-writer picked up this review and is working on it. I'll push to this branch and reply here with what changed. A human still approves — I never merge on my own review.

[nictuku]

Closing as superseded — the diagnosis here was right, but the fix landed on the wrong side.

get_account_pages has not been served by this package in production for some time: it is intercepted by the Next.js fork (app/meta-ads-mcp/account-read-tools-handler.ts, handleGetAccountPagesParity), and only tools/call for un-intercepted tools falls through to the Python backend. So wiring _discover_all_page_ids_for_account into Python's get_account_pages gave the two tools a shared universe inside this package only — in production get_account_pages would have kept running the TypeScript implementation while search_pages_by_name ran this one. Two independent 8-approach implementations in two languages, free to drift, which is the same class of bug as the one being fixed.

The fix shipped instead as pipeboard-co/pipeboard.co#2651 (squash 6ae36d144, deployed and verified on Vercel + mcp1 + mcp2): search_pages_by_name is now intercepted in Next.js and shares discoverAllPageIdsForAccount() with get_account_pages, so there is one page universe and one code path. It carries over this PR's good ideas — per-page source attribution, the merged {account}/ads request for approaches 5+7, concurrent detail fetches, deterministic ordering — plus concurrent discovery.

Measured before/after on the production path, same account and token: total_available 1 (2 of 3 pages unsearchable) → 3, matching get_account_pages exactly.

Worth noting for the standalone package: search_pages_by_name here still has the original bug for anyone self-hosting meta-ads-mcp without the Pipeboard proxy. If we want to keep the OSS package correct on its own, this branch is a reasonable starting point for a follow-up — but it should not be merged as-is on the assumption that it fixes the hosted product, because it does not.