feat(marketing-analytics): add a ROAS column driven by revenue conversion goals#73052
Draft
jabahamondes wants to merge 6 commits into
Draft
feat(marketing-analytics): add a ROAS column driven by revenue conversion goals#73052jabahamondes wants to merge 6 commits into
jabahamondes wants to merge 6 commits into
Conversation
… base currency A sum-math conversion goal summed its math_property raw, ignoring math_property_revenue_currency. Goals whose revenue property mixes currencies were added together as if they were one, and the total was in no currency at all. Now each value is converted at its own event-day exchange rate, in both the live and precompute paths so they agree. Goals without a currency configured are unchanged.
…sion goals ROAS per channel is the summed value of every conversion goal marked counts_as_revenue, over that channel's spend. It only appears when at least one goal is flagged as revenue and when spend is available for the level, so it stays hidden until revenue goals exist. Revenue is already in the base currency thanks to the conversion-goal currency conversion.
Contributor
🤖 CI reportℹ️ MCP UI apps size — 31 app(s), 16602.7 KB JSBuilt size of each MCP UI app (
|
2 tasks
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
1 participant
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.
Note
Sits on top of the conversion-goal stack (#73033 → ... → #72301) and also carries the currency fix from #73047 (cherry-picked, not merged), because ROAS is only correct once revenue is converted to the base currency. This branch is the one to check out to try the whole thing end to end: flags, write endpoints, MCP tools, currency conversion, and now ROAS. When #73047 and the stack land on master, a rebase drops the duplicated currency commit.
Problem
Marketing analytics can show you spend per channel and conversions per channel, but not the one ratio a marketer actually optimizes: revenue over spend. The pieces were all there (spend per channel, a per-goal value column, currency conversion from #73047) but nothing tied them together.
Changes
A
ROAScolumn: the summed value of every conversion goal markedcounts_as_revenue, divided by that channel's spend.It reuses the exact pattern the existing "cost per conversion" column already uses (
campaign_costs.total_costover anullif'd conversion value from the unified goals CTE), just inverted and summed across the revenue-flagged goals. Built inconversion_goals_aggregator.get_conversion_goal_columns, so it flows to the table through the sameall_columns.update(...)every other conversion column does.Three deliberate boundaries:
counts_as_revenuemeans no column. The table doesn't grow an empty ROAS column for teams that haven't set one up. The existing table and aggregator snapshots are unchanged, which is the proof it's inert by default.include_cost_perguard the cost-per column uses. At UTM levels where platform cost can't be attributed, both are correctly absent.No new column metadata was needed.
ROASfalls throughto_marketing_analytics_datato the unit / higher-is-good defaults, which is exactly right for a ratio where more is better.How did you test this code?
3 new unit tests in
test_conversion_goals_aggregator.py, run by me (Claude):include_cost_per=False). Guards the denominator boundary at UTM levels.Full
test_conversion_goals_aggregator.py(30),test_marketing_analytics_table_query_runner.pyand_business.py(79) pass, snapshots unchanged.ruffclean;mypyclean on the file (the one printed error is a pre-existing unrelated one inpersonhog_client).I did not add an end-to-end ClickHouse test that asserts a computed ROAS number. The column's construction is covered by the unit tests, and its propagation into the final SELECT is the same generic
update()path already exercised by the cost-per columns. An end-to-end test would have needed a full cost + conversion fixture to cover a path that isn't new.Automatic notifications
Docs update
Holding until the read-side columns settle and the dashboard redesign decides how they surface.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
I directed this, Claude wrote it. Invoked
/writing-tests, which drove the test list toward the one assertion that matters (numerator sums only revenue goals) and away from re-testing the round/nullif plumbing the cost-per column already covers.This is the first of the two read-side metric columns. CAC is deliberately not in this PR. It turned out to be structurally larger than ROAS: "new customers" means each person's first-ever conversion, which needs a historical first-time-per-user scan that the current per-window precompute doesn't provide (
first_time_for_userisn't wired into the marketing processor at all yet). That deserves its own design pass rather than being bolted on here, so ROAS ships on its own.