fix(meta_ads): count conversions via a canonical exact-match counter (#340)#343
Merged
Conversation
…340) `meta_row_conversions` counted Meta conversions with a substring scan (`"lead" in action_type or "purchase" in action_type`) that over-counted for two reasons: 1. Alias double-count. Meta's Insights `actions` array returns one conversion under several overlapping action_type rows — the generic `lead` action_type is, by Meta's own definition, the aggregate ("all offsite + all On-Facebook leads") of its components `offsite_conversion.fb_pixel_lead` + `onsite_conversion.lead_grouped`; `purchase`/`omni_purchase` is the analogous roll-up. Because actions are fetched unfiltered, aggregate + component rows co-occur, so summing every `*lead*`/`*purchase*` row counted the same conversions two or three times. 2. Substring false positives. Operator-named custom conversions (`offsite_conversion.custom.<slug>`) carry free-text slugs that may contain `lead`/`purchase` and were swept in. This deflated the absolute CPA emitted by `diagnose_performance` and the budget-efficiency scorer (the anomaly-detector CPA-spike path is ratio-based and largely shielded). It also diverged from the MCP-analysis path's `_extract_cv`, which already used an exact `{lead, purchase, complete_registration}` set — so there were multiple live counters disagreeing on what counts as a conversion. Fix: introduce a single canonical counter `mureo/meta_ads/_conversion_count.py` (`count_conversions_from_actions` + `CONVERSION_ACTION_TYPES`) that counts ONLY the deduped generic conversion action_types — the aggregate already includes its components, so the components are not added on top, and custom slugs no longer match. Route ALL four live counters through it so they cannot drift again: - `mureo/analytics/builtin/_common.py` `meta_row_conversions` (lazy import to keep adapter registration free of the meta_ads client weight; BYOD branch unchanged) - `mureo/meta_ads/_analysis.py` `_extract_cv` (now delegates) - `mureo/analytics/builtin/_budget_efficiency.py` - `mureo/meta_ads/_insights.py` breakdown The campaign-type classifier (`_classify_result_indicator`) keeps its substring match — it is a boolean "any conversion signal?" check, not a counter, so substring is correct there. Note: `complete_registration` is now counted on the analytics/CPA path (previously only `_extract_cv` counted it) — an intentional consistency change. A known component-only / custom-event under-count is tracked as a separate operator-canonical-event enhancement (#342). Tests: new tests/test_meta_conversion_count.py (dedup, custom-slug false positive, view/engagement exclusion, junk handling, _extract_cv equivalence); updated the two fixtures that encoded the old substring behaviour to realistic aggregate+component arrays that prove the dedup; added a budget-efficiency custom-slug regression test. Closes #340 Claude-Session: https://claude.ai/code/session_011rAu94b1o1xWYZhARk1VmL
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
meta_row_conversions(the Meta conversion counter feeding CPA → anomalydetection /
diagnose_performance/ budget-efficiency) counted via asubstring scan
"lead" in action_type or "purchase" in action_type, whichover-counted for two reasons:
actionsarray returns oneconversion under several overlapping
action_typerows. The genericleadis, by Meta's own definition, the aggregate ("all offsite + all
On-Facebook leads") of its components
offsite_conversion.fb_pixel_lead+onsite_conversion.lead_grouped;purchase/omni_purchaseis theanalogous roll-up. Because
actionsis fetched unfiltered, aggregate +component rows co-occur, so summing every
*lead*/*purchase*row countedthe same conversions two or three times.
(
offsite_conversion.custom.<slug>) carry free-text slugs that may containlead/purchaseand were swept in.This deflated the absolute CPA emitted by
diagnose_performanceand thebudget-efficiency scorer (the anomaly-detector CPA-spike path is ratio-based
and largely shielded). It also diverged from the MCP-analysis counter
_extract_cv, which already used an exact{lead, purchase, complete_registration}set — so multiple live counters disagreed on whatcounts as a conversion.
Fix
A single canonical counter
mureo/meta_ads/_conversion_count.py(
count_conversions_from_actions+CONVERSION_ACTION_TYPES) that countsonly the deduped generic conversion action_types — the aggregate already
includes its components, so components are not added on top, and custom slugs
no longer match. All four live counters now route through it so they cannot
drift again:
mureo/analytics/builtin/_common.pymeta_row_conversions(lazy import tokeep adapter registration free of the
mureo.meta_adsclient weight; BYODbranch unchanged)
mureo/meta_ads/_analysis.py_extract_cv(delegates)mureo/analytics/builtin/_budget_efficiency.pymureo/meta_ads/_insights.pybreakdownThe campaign-type classifier (
_classify_result_indicator) keeps substringmatching — it is a boolean "any conversion signal?" check, not a counter, so
substring is correct there.
Behaviour notes
complete_registrationis now counted on the analytics/CPA path (previouslyonly
_extract_cvcounted it) — an intentional consistency change.as a component row, or as a custom event) is tracked separately as the
operator-canonical-event enhancement (feat(meta_ads): operator-specified canonical conversion event (avoid component-only under-count) #342).
Test plan
tests/test_meta_conversion_count.py— dedup (aggregate+component),custom-slug false positive, view/engagement exclusion, non-list/empty/junk
handling,
_extract_cvbyte-for-byte equivalence.(
test_live_clients.py,test_budget_efficiency.py) to realisticaggregate+component arrays that prove the dedup; added a budget-efficiency
custom-slug regression test.
ruff+blackclean onmureo/;mypy --strictclean on changedmodules; 1125 passed in the meta/analytics sweep (only the known
installed-plugin env-gating noise excluded).
substring counters (
_budget_efficiency.py,_insights.py) that this PR nowalso migrates.
Closes #340
https://claude.ai/code/session_011rAu94b1o1xWYZhARk1VmL