Skip to content

Commit 55dafa0

Browse files
authored
[DISCO-4020] engagement data check (#1355)
1 parent a18eb74 commit 55dafa0

File tree

3 files changed

+48
-1
lines changed

3 files changed

+48
-1
lines changed

merino/providers/suggest/adm/provider.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,8 @@ def _is_thompson_eligible(self, client_variants: list[str]) -> bool:
221221
"""Return True if Thompson sampling should be applied to this request."""
222222
if not self.thompson:
223223
return False
224+
if not self.engagement_data.amp:
225+
return False
224226
if self.should_check_client_variants:
225227
return any(cv in CLIENT_VARIANTS_ALLOW_LIST for cv in client_variants)
226228
return True

tests/unit/providers/suggest/adm/conftest.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,12 +317,20 @@ def fixture_adm_with_thompson_single_candidate_below_threshold(
317317
adm_parameters: dict[str, Any],
318318
thompson_sampler_with_dummy: ThompsonSampler,
319319
statsd_mock: Any,
320+
engagement_data: EngagementData,
320321
) -> Provider:
321322
"""Create an AdM Provider with Thompson sampling and a min_attempted_count threshold."""
322-
return Provider(
323+
provider = Provider(
323324
backend=backend_mock,
324325
metrics_client=statsd_mock,
325326
min_attempted_count=1000,
326327
thompson=thompson_sampler_with_dummy,
327328
**adm_parameters,
328329
)
330+
provider.engagement_data = EngagementData(
331+
amp={
332+
"Example.org": {"click": 10, "impression": 100},
333+
},
334+
amp_aggregated={},
335+
)
336+
return provider

tests/unit/providers/suggest/adm/test_provider_thompson.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
from merino.middleware.geolocation import Location
1313
from merino.middleware.user_agent import UserAgent
1414
from merino.optimizers.thompson import ThompsonSampler
15+
from merino.providers.suggest.adm.backends.protocol import EngagementData
1516
from merino.providers.suggest.adm.provider import NonsponsoredSuggestion, Provider
1617

1718
from tests.unit.types import SuggestionRequestFixture
@@ -75,6 +76,9 @@ async def test_query_with_thompson_dummy_suppresses_suggestion(
7576
) -> None:
7677
"""Provider with a dominant dummy should suppress the suggestion (return empty list)."""
7778
await adm_with_thompson_dummy.initialize()
79+
adm_with_thompson_dummy.engagement_data = EngagementData(
80+
amp={"something": {}}, amp_aggregated={}
81+
)
7882

7983
res = await adm_with_thompson_dummy.query(
8084
srequest("firefox", GEOLOCATION, USER_AGENT, CLIENT_VARIANTS)
@@ -216,3 +220,36 @@ async def test_query_with_thompson_single_candidate_below_threshold_returns_sugg
216220
statsd_mock.increment.assert_called_once_with(
217221
"providers.adm.thompson.select", tags={"outcome": "skipped"}
218222
)
223+
224+
225+
@pytest.mark.asyncio
226+
async def test_query_with_thompson_without_engagement_data_skips_sampling(
227+
srequest: SuggestionRequestFixture,
228+
adm_with_thompson_dummy: Provider,
229+
adm_parameters: dict[str, Any],
230+
statsd_mock: Any,
231+
) -> None:
232+
"""Provider should skip Thompson sampling when engagement data is empty."""
233+
await adm_with_thompson_dummy.initialize()
234+
235+
res = await adm_with_thompson_dummy.query(
236+
srequest("firefox", GEOLOCATION, USER_AGENT, CLIENT_VARIANTS)
237+
)
238+
239+
assert res == [
240+
NonsponsoredSuggestion(
241+
block_id=2,
242+
full_keyword="firefox accounts",
243+
title="Mozilla Firefox Accounts",
244+
url=HttpUrl("https://example.org/target/mozfirefoxaccounts"),
245+
categories=[],
246+
impression_url=HttpUrl("https://example.org/impression/mozilla"),
247+
click_url=HttpUrl("https://example.org/click/mozilla"),
248+
provider="adm",
249+
advertiser="Example.org",
250+
is_sponsored=False,
251+
icon="attachment-host/main-workspace/quicksuggest/icon-01",
252+
score=adm_parameters["score"],
253+
)
254+
]
255+
statsd_mock.increment.assert_not_called()

0 commit comments

Comments
 (0)