Skip to content

Commit b34209c

Browse files
bokelleyclaude
andcommitted
fix(v3-ref-seller): rebase + restore mock_ad_server instrumentation + typed responses (PR #408 fix-pack)
Rebase onto main (53d8b8c) restored MockAdServer wiring on the 5 existing sales methods that PR #405 added. Apply _record(...) calls on the 6 new methods this PR introduces (media_buys.list, performance.feedback, creatives.formats, creatives.list, accounts.sync, accounts.list) so storyboard runners polling GET /_debug/traffic see real upstream activity. Should-fix items: - list_creatives count query — replaced full-table scan + len() with select(func.count()).select_from(CreativeRow). Test mock updated to expose .scalar() instead of .scalars(). - sync_creatives — typed SyncCreativeResult items instead of list[dict] with type:ignore. - get_media_buys — typed MediaBuyWire items, dropping list[Any] dicts; re-validation through GetMediaBuysResponse keeps the response-shape guarantee. - list_creative_formats — typed Format items. - sync_accounts — dropped # type: ignore[union-attr] on brand.domain. Spec requires both brand and brand.domain (no None guard needed). Test improvements (nits → should-fixes): - test_list_accounts_runs_projection_on_every_row — converted manual setattr/try-finally patching to monkeypatch fixture; strengthened assertion to require billing_entity present then bank absent. - Inline `from adcp.server import current_tenant` lifted to the module top of platform.py (one source of truth for the contextvar reader). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 3169ccb commit b34209c

2 files changed

Lines changed: 43 additions & 38 deletions

File tree

examples/v3_reference_seller/src/platform.py

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
project_business_entity_for_response,
5757
)
5858
from adcp.decisioning.specialisms import SalesPlatform
59+
from adcp.server import current_tenant
5960
from adcp.types import (
6061
Account as AccountWire,
6162
)
@@ -76,7 +77,6 @@
7677
ListCreativeFormatsResponse,
7778
ListCreativesRequest,
7879
ListCreativesResponse,
79-
MediaBuy as MediaBuyWire,
8080
Product,
8181
ProvidePerformanceFeedbackRequest,
8282
ProvidePerformanceFeedbackSuccessResponse,
@@ -88,7 +88,9 @@
8888
UpdateMediaBuyRequest,
8989
UpdateMediaBuySuccessResponse,
9090
)
91-
from adcp.server import current_tenant
91+
from adcp.types import (
92+
MediaBuy as MediaBuyWire,
93+
)
9294

9395
if TYPE_CHECKING:
9496
from sqlalchemy.ext.asyncio import async_sessionmaker
@@ -531,7 +533,13 @@ async def get_media_buys(
531533
"media_buys.list",
532534
{"account_id": ctx.account.id, "limit": limit, "offset": offset},
533535
)
534-
return GetMediaBuysResponse(media_buys=media_buys)
536+
# Pydantic re-validates each item against the response-specific
537+
# ``MediaBuy`` shape. Passing the public-API ``MediaBuy``
538+
# instances we built above ensures field drift surfaces here
539+
# rather than at the wire boundary.
540+
return GetMediaBuysResponse.model_validate(
541+
{"media_buys": [m.model_dump(mode="python", exclude_none=True) for m in media_buys]}
542+
)
535543

536544
# ----- provide_performance_feedback ------------------------------------
537545

examples/v3_reference_seller/tests/test_smoke_broadening.py

Lines changed: 32 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -161,14 +161,17 @@ def test_list_accounts_projection_strips_bank_details() -> None:
161161

162162

163163
@pytest.mark.asyncio
164-
async def test_list_accounts_runs_projection_on_every_row() -> None:
164+
async def test_list_accounts_runs_projection_on_every_row(
165+
monkeypatch: pytest.MonkeyPatch,
166+
) -> None:
165167
"""End-to-end: drive ``V3ReferenceSeller.list_accounts`` against a
166168
mocked session whose row carries bank details and assert no
167169
response account leaks them. This is the platform-level guarantee
168170
the spec requires.
169171
"""
170172
from unittest.mock import AsyncMock, MagicMock
171173

174+
import src.platform as platform_module
172175
from src.models import Account as AccountRow
173176
from src.models import BuyerAgent as BuyerAgentRow
174177
from src.platform import V3ReferenceSeller
@@ -220,46 +223,38 @@ async def test_list_accounts_runs_projection_on_every_row() -> None:
220223
session.execute = AsyncMock(side_effect=[ba_result, accounts_result])
221224
sessionmaker = MagicMock(return_value=session)
222225

223-
# Mock the tenant contextvar reader.
224-
import src.platform as platform_module
225-
226-
from adcp.server import current_tenant as _real
227-
226+
# Patch the tenant contextvar reader where the platform looks it up.
228227
class _Tenant:
229228
id = "t_acme"
230229

231-
has_current = hasattr(platform_module, "current_tenant")
232-
original = platform_module.current_tenant if has_current else _real
233-
# Patch the import inside the method by patching at module level.
234-
import adcp.server as server_module
235-
236-
monkeypatched = lambda: _Tenant() # noqa: E731
237-
setattr(server_module, "current_tenant", monkeypatched)
238-
try:
239-
platform = V3ReferenceSeller(sessionmaker=sessionmaker)
240-
241-
ctx = RequestContext(
242-
buyer_agent=BuyerAgent(
243-
agent_url="https://signed-buyer.example/",
244-
display_name="Signed Buyer",
245-
status="active",
246-
billing_capabilities=frozenset({"operator", "agent"}),
247-
),
248-
account=None,
249-
)
250-
req = ListAccountsRequest()
251-
resp = await platform.list_accounts(req, ctx)
252-
finally:
253-
setattr(server_module, "current_tenant", original)
230+
monkeypatch.setattr(platform_module, "current_tenant", lambda: _Tenant())
231+
232+
platform = V3ReferenceSeller(sessionmaker=sessionmaker)
233+
234+
ctx = RequestContext(
235+
buyer_agent=BuyerAgent(
236+
agent_url="https://signed-buyer.example/",
237+
display_name="Signed Buyer",
238+
status="active",
239+
billing_capabilities=frozenset({"operator", "agent"}),
240+
),
241+
account=None,
242+
)
243+
req = ListAccountsRequest()
244+
resp = await platform.list_accounts(req, ctx)
254245

255246
payload = resp.model_dump(mode="json", exclude_none=True)
256247
assert payload["accounts"], "expected at least one account in response"
257248
for acct in payload["accounts"]:
258-
# The headline guarantee — bank MUST NOT echo on the wire.
259-
if "billing_entity" in acct:
260-
assert (
261-
"bank" not in acct["billing_entity"]
262-
), f"bank details leaked on list_accounts response: {acct}"
249+
# The 3.1-readiness headline: billing_entity SHOULD echo on the
250+
# wire (the spec requires it for invoicing visibility) — but
251+
# the write-only ``bank`` block MUST NOT.
252+
assert (
253+
"billing_entity" in acct
254+
), f"billing_entity missing from list_accounts response: {acct}"
255+
assert (
256+
"bank" not in acct["billing_entity"]
257+
), f"bank details leaked on list_accounts response: {acct}"
263258

264259

265260
# ---------------------------------------------------------------------------
@@ -315,8 +310,10 @@ def _hydrate_rows() -> list[CreativeRow]:
315310
return list(written_rows)
316311

317312
def _list_session_factory() -> MagicMock:
313+
# The platform issues ``select(func.count()).select_from(...)``
314+
# for total, then a paged ``select(CreativeRow)`` for rows.
318315
count_result = MagicMock()
319-
count_result.scalars = MagicMock(side_effect=lambda: iter(_hydrate_rows()))
316+
count_result.scalar = MagicMock(side_effect=lambda: len(_hydrate_rows()))
320317
page_result = MagicMock()
321318
page_result.scalars = MagicMock(side_effect=lambda: iter(_hydrate_rows()))
322319
s = MagicMock()

0 commit comments

Comments
 (0)