Skip to content

Commit a279d69

Browse files
committed
Add a switch for client_variants check
1 parent ab1a988 commit a279d69

6 files changed

Lines changed: 73 additions & 10 deletions

File tree

merino/configs/default.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -607,6 +607,10 @@ dummy_engaged_count = 1
607607
# The dummy candidate for Thompson sampling - attempted count
608608
dummy_attempted_count = 1000
609609

610+
# MERINO_PROVIDERS__ADM__THOMPSON__CHECK_CLIENT_VARIANTS
611+
# Whether to check `client_variants` for Thompson sampling
612+
check_client_variants = true
613+
610614
[default.amo.dynamic]
611615
# MERINO_AMO__DYNAMIC__API_URL
612616
# This is the URL for the Addons API to get more information for particular addons.

merino/jobs/cli.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,7 @@
1818
# Include your new jobs module here.
1919

2020
# NOTE: `pretty_exceptions_show_locals` argument is set to False to avoid api_key and secrets exposure.
21-
cli = typer.Typer(
22-
no_args_is_help=True, add_completion=False, pretty_exceptions_show_locals=False
23-
)
21+
cli = typer.Typer(no_args_is_help=True, add_completion=False, pretty_exceptions_show_locals=False)
2422

2523
# Add the wikipedia-indexer subcommands
2624
cli.add_typer(indexer_cmd, no_args_is_help=True)

merino/providers/suggest/adm/provider.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ class Provider(BaseProvider):
8383
backend: AdmBackend
8484
resync_interval_sec: float
8585
min_attempted_count: int
86+
should_check_client_variants: bool
8687
thompson: ThompsonSampler | None = None
8788

8889
def __init__(
@@ -95,6 +96,7 @@ def __init__(
9596
enabled_by_default: bool = True,
9697
min_attempted_count: int = 0,
9798
thompson: ThompsonSampler | None = None,
99+
should_check_client_variants=True,
98100
**kwargs: Any,
99101
) -> None:
100102
"""Store the given Remote Settings backend on the provider."""
@@ -107,6 +109,7 @@ def __init__(
107109
self._enabled_by_default = enabled_by_default
108110
self.min_attempted_count = min_attempted_count
109111
self.thompson = thompson
112+
self.should_check_client_variants = should_check_client_variants
110113
super().__init__(**kwargs)
111114

112115
async def initialize(self) -> None:
@@ -167,7 +170,10 @@ def _select(
167170
Either a winner `PyAmpResult` or None if the optimizer (e.g. Thompson sampler)
168171
determines so. Return the first candidate when the optimizer is disabled.
169172
"""
170-
if self.thompson and any([cv in CLIENT_VARIANTS_ALLOW_LIST for cv in client_variants]):
173+
if self.thompson and (
174+
not self.should_check_client_variants
175+
or any([cv in CLIENT_VARIANTS_ALLOW_LIST for cv in client_variants])
176+
):
171177
candidates = [
172178
ThompsonCandidate(id=i, metrics=self._fetch_engagement_metrics(suggestion))
173179
for i, suggestion in enumerate(suggestions)

merino/providers/suggest/manager.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,7 @@ def _create_provider(provider_id: str, setting: Settings) -> BaseProvider:
202202
enabled_by_default=setting.enabled_by_default,
203203
min_attempted_count=settings.providers.adm.thompson.min_attempted_count,
204204
thompson=thompson,
205+
should_check_client_variants=settings.providers.adm.thompson.check_client_variants,
205206
)
206207
case ProviderType.GEOLOCATION:
207208
return GeolocationProvider(

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,3 +167,18 @@ def fixture_adm_with_thompson_dummy_min_attempted_count(
167167
thompson=thompson_sampler_with_dummy,
168168
**adm_parameters,
169169
)
170+
171+
172+
@pytest.fixture(name="adm_with_thompson_skip_client_variants_check")
173+
def fixture_adm_with_thompson_skip_client_variants_check(
174+
backend_mock: Any,
175+
adm_parameters: dict[str, Any],
176+
thompson_sampler: ThompsonSampler,
177+
) -> Provider:
178+
"""Create an AdM Provider with Thompson sampling enabled for testing."""
179+
return Provider(
180+
backend=backend_mock,
181+
thompson=thompson_sampler,
182+
should_check_client_variants=False,
183+
**adm_parameters,
184+
)

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

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717
from tests.unit.types import SuggestionRequestFixture
1818

1919

20+
CLIENT_VARIANTS: list[str] = ["engagement_guided_suggestions"]
21+
22+
2023
def test_thompson_attribute_is_none_by_default(adm: Provider) -> None:
2124
"""Provider created without a thompson argument should have thompson=None."""
2225
assert adm.thompson is None
@@ -69,10 +72,10 @@ async def test_query_with_thompson_dummy_suppresses_suggestion(
6972
await adm_with_thompson_dummy.initialize()
7073
geolocation = Location(country="US")
7174
user_agent = UserAgent(form_factor="desktop", browser="firefox", os_family="macos")
72-
client_variant = ["engagement_guided_suggestions"]
75+
client_variants = CLIENT_VARIANTS
7376

7477
res = await adm_with_thompson_dummy.query(
75-
srequest("firefox", geolocation, user_agent, client_variant)
78+
srequest("firefox", geolocation, user_agent, client_variants)
7679
)
7780

7881
assert res == []
@@ -85,7 +88,7 @@ async def test_query_with_thompson_no_match_returns_empty(
8588
) -> None:
8689
"""Thompson-enabled provider should return empty list when the query matches nothing."""
8790
await adm_with_thompson.initialize()
88-
client_variants = ["engagement_guided_suggestions"]
91+
client_variants = CLIENT_VARIANTS
8992

9093
res = await adm_with_thompson.query(srequest("zzznomatch", None, None, client_variants))
9194

@@ -100,8 +103,8 @@ async def test_query_with_thompson_uses_fallback_country_and_form_factor(
100103
) -> None:
101104
"""Thompson-enabled provider should apply country/form-factor fallbacks when absent."""
102105
await adm_with_thompson.initialize()
103-
client_variant = ["engagement_guided_suggestions"]
104-
res = await adm_with_thompson.query(srequest("firefox", None, None, client_variant))
106+
client_variants = CLIENT_VARIANTS
107+
res = await adm_with_thompson.query(srequest("firefox", None, None, client_variants))
105108

106109
assert len(res) == 1
107110
assert res[0].score == adm_parameters["score"]
@@ -119,7 +122,7 @@ async def test_query_with_thompson_min_attempted_count_returns_suggestion(
119122
await adm_with_thompson_dummy_min_attempted_count.initialize()
120123
geolocation = Location(country="US")
121124
user_agent = UserAgent(form_factor="desktop", browser="firefox", os_family="macos")
122-
client_variants = ["engagement_guided_suggestions"]
125+
client_variants = CLIENT_VARIANTS
123126

124127
res = await adm_with_thompson_dummy_min_attempted_count.query(
125128
srequest("firefox", geolocation, user_agent, client_variants)
@@ -141,3 +144,39 @@ async def test_query_with_thompson_min_attempted_count_returns_suggestion(
141144
score=adm_parameters["score"],
142145
)
143146
]
147+
148+
149+
@pytest.mark.asyncio
150+
async def test_query_with_thompson_without_client_variants_check(
151+
srequest: SuggestionRequestFixture,
152+
adm_with_thompson_skip_client_variants_check: Provider,
153+
adm_parameters: dict[str, Any],
154+
) -> None:
155+
"""Thompson-enabled provider without the client_variants check should return
156+
a suggestion when the sampler picks a winner even if client_variants does
157+
not match.
158+
"""
159+
await adm_with_thompson_skip_client_variants_check.initialize()
160+
geolocation = Location(country="US")
161+
user_agent = UserAgent(form_factor="desktop", browser="firefox", os_family="macos")
162+
client_variants: list[str] = []
163+
res = await adm_with_thompson_skip_client_variants_check.query(
164+
srequest("firefox", geolocation, user_agent, client_variants)
165+
)
166+
167+
assert res == [
168+
NonsponsoredSuggestion(
169+
block_id=2,
170+
full_keyword="firefox accounts",
171+
title="Mozilla Firefox Accounts",
172+
url=HttpUrl("https://example.org/target/mozfirefoxaccounts"),
173+
categories=[],
174+
impression_url=HttpUrl("https://example.org/impression/mozilla"),
175+
click_url=HttpUrl("https://example.org/click/mozilla"),
176+
provider="adm",
177+
advertiser="Example.org",
178+
is_sponsored=False,
179+
icon="attachment-host/main-workspace/quicksuggest/icon-01",
180+
score=adm_parameters["score"],
181+
)
182+
]

0 commit comments

Comments
 (0)