Skip to content

Commit 75767a3

Browse files
committed
fix(attribution): keep owner backfill tenant-scoped
1 parent 2609e95 commit 75767a3

2 files changed

Lines changed: 110 additions & 5 deletions

File tree

backend/alembic/versions/expownercreate001_backfill_creation_owner.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,15 @@
1919
def upgrade() -> None:
2020
# Include deleted and superseded trial history deliberately. Only the exact
2121
# first trial can identify an experiment's creator; selecting the earliest
22-
# later trial with billing data could assign it to an appender.
22+
# later trial with billing data could assign it to an appender. Validate the
23+
# first trial's payer only after ranking because billed_user_id is not an FK.
24+
# Deleted users remain eligible: creator provenance survives offboarding.
2325
op.execute(
2426
"""
2527
WITH earliest_trial AS MATERIALIZED (
2628
SELECT DISTINCT ON (t.experiment_id)
2729
t.experiment_id,
30+
t.org_id,
2831
t.billed_user_id
2932
FROM trials AS t
3033
WHERE t.experiment_id IS NOT NULL
@@ -33,8 +36,11 @@ def upgrade() -> None:
3336
UPDATE experiments AS e
3437
SET owner_user_id = earliest_trial.billed_user_id
3538
FROM earliest_trial
39+
JOIN users AS billed_user
40+
ON billed_user.id = earliest_trial.billed_user_id
3641
WHERE e.id = earliest_trial.experiment_id
37-
AND earliest_trial.billed_user_id IS NOT NULL
42+
AND earliest_trial.org_id = e.org_id
43+
AND billed_user.org_id = e.org_id
3844
AND (
3945
e.owner_user_id IS NULL
4046
OR e.owner_user_id = '__unattributed__'

backend/tests/test_experiment_owner_creation_backfill.py

Lines changed: 102 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,19 @@
11
from __future__ import annotations
22

33
import importlib.util
4+
import os
45
from pathlib import Path
56

7+
import pytest
8+
from sqlalchemy import text
9+
from sqlalchemy.ext.asyncio import create_async_engine
610

7-
def test_backfill_uses_the_exact_first_trial_including_history() -> None:
11+
12+
DB_URL = os.environ.get("ODDISH_DATABASE_URL")
13+
requires_db = pytest.mark.skipif(not DB_URL, reason="ODDISH_DATABASE_URL not set")
14+
15+
16+
def _upgrade_sql() -> str:
817
migration_path = (
918
Path(__file__).resolve().parents[1]
1019
/ "alembic/versions/expownercreate001_backfill_creation_owner.py"
@@ -24,10 +33,100 @@ def test_backfill_uses_the_exact_first_trial_including_history() -> None:
2433
finally:
2534
module.op.execute = original_execute
2635

27-
sql = statements[0]
36+
assert len(statements) == 1
37+
return statements[0]
38+
39+
40+
def test_backfill_uses_the_exact_first_trial_including_history() -> None:
41+
sql = _upgrade_sql()
2842
assert "DISTINCT ON (t.experiment_id)" in sql
2943
assert "ORDER BY t.experiment_id, t.created_at ASC, t.id ASC" in sql
30-
assert "earliest_trial.billed_user_id IS NOT NULL" in sql
3144
assert "owner_user_id IS NULL" in sql
3245
assert "t.deleted_at" not in sql
3346
assert "superseded_by_trial_id" not in sql
47+
48+
49+
def test_backfill_only_accepts_a_first_trial_payer_from_the_experiment_org() -> None:
50+
sql = _upgrade_sql()
51+
52+
# Validate only after selecting the exact first trial. Joining users inside
53+
# the CTE would silently fall forward to a later trial when the first trial
54+
# has stale or cross-tenant billing attribution.
55+
first_trial_query, update_query = sql.split("UPDATE experiments AS e", maxsplit=1)
56+
assert "users" not in first_trial_query
57+
assert "JOIN users AS billed_user" in update_query
58+
assert "billed_user.id = earliest_trial.billed_user_id" in update_query
59+
assert "earliest_trial.org_id = e.org_id" in update_query
60+
assert "billed_user.org_id = e.org_id" in update_query
61+
62+
63+
@requires_db
64+
@pytest.mark.asyncio
65+
async def test_backfill_leaves_cross_org_attribution_unowned() -> None:
66+
assert DB_URL is not None
67+
engine = create_async_engine(DB_URL)
68+
try:
69+
async with engine.begin() as conn:
70+
# Temporary tables shadow the real schema for this connection, so
71+
# this runs the migration SQL without mutating shared test data.
72+
statements = (
73+
"""
74+
CREATE TEMP TABLE experiments (
75+
id TEXT PRIMARY KEY,
76+
org_id TEXT NOT NULL,
77+
owner_user_id TEXT
78+
)
79+
""",
80+
"""
81+
CREATE TEMP TABLE trials (
82+
id TEXT PRIMARY KEY,
83+
experiment_id TEXT,
84+
org_id TEXT NOT NULL,
85+
billed_user_id TEXT,
86+
created_at TIMESTAMPTZ NOT NULL
87+
)
88+
""",
89+
"""
90+
CREATE TEMP TABLE users (
91+
id TEXT PRIMARY KEY,
92+
org_id TEXT NOT NULL
93+
)
94+
""",
95+
"""
96+
INSERT INTO users (id, org_id) VALUES
97+
('user-a', 'org-a'),
98+
('user-b', 'org-b')
99+
""",
100+
"""
101+
INSERT INTO experiments (id, org_id) VALUES
102+
('valid', 'org-a'),
103+
('foreign-payer', 'org-a'),
104+
('foreign-trial', 'org-a'),
105+
('no-fall-forward', 'org-a')
106+
""",
107+
"""
108+
INSERT INTO trials (
109+
id, experiment_id, org_id, billed_user_id, created_at
110+
) VALUES
111+
('t-valid', 'valid', 'org-a', 'user-a', '2026-01-01'),
112+
('t-payer', 'foreign-payer', 'org-a', 'user-b', '2026-01-01'),
113+
('t-trial', 'foreign-trial', 'org-b', 'user-a', '2026-01-01'),
114+
('t-first', 'no-fall-forward', 'org-a', 'user-b', '2026-01-01'),
115+
('t-later', 'no-fall-forward', 'org-a', 'user-a', '2026-01-02')
116+
""",
117+
)
118+
for statement in statements:
119+
await conn.execute(text(statement))
120+
await conn.execute(text(_upgrade_sql()))
121+
result = await conn.execute(
122+
text("SELECT id, owner_user_id FROM experiments ORDER BY id")
123+
)
124+
125+
assert dict(result.all()) == {
126+
"foreign-payer": None,
127+
"foreign-trial": None,
128+
"no-fall-forward": None,
129+
"valid": "user-a",
130+
}
131+
finally:
132+
await engine.dispose()

0 commit comments

Comments
 (0)