From d3f8df49e566838b6a6df7ca546bddc49da3614a Mon Sep 17 00:00:00 2001 From: Aleksander Sekowski Date: Sat, 3 Oct 2026 20:31:07 -0700 Subject: [PATCH 1/3] fix: reject inventory filters without a query --- src/ad_buyer/clients/unified_client.py | 12 +++- .../unit/test_buyer_deal_discovery_pricing.py | 68 ++++++++++++++++++- 2 files changed, 76 insertions(+), 4 deletions(-) diff --git a/src/ad_buyer/clients/unified_client.py b/src/ad_buyer/clients/unified_client.py index 3f6347b2..a5eafcc8 100644 --- a/src/ad_buyer/clients/unified_client.py +++ b/src/ad_buyer/clients/unified_client.py @@ -502,11 +502,17 @@ async def discover_inventory( if query: args["query"] = query - # Try search_products first, fall back to list_products + has_product_filters = any( + key in filters + for key in ("channel", "maxPrice", "minImpressions", "targeting", "publisher") + ) + if has_product_filters and not query: + raise ValueError( + "Product filters require a query; list_products does not support filters" + ) if query: return await self.call_tool("search_products", args, protocol=protocol) - else: - return await self.call_tool("list_products", protocol=protocol) + return await self.call_tool("list_products", protocol=protocol) async def get_pricing( self, diff --git a/tests/unit/test_buyer_deal_discovery_pricing.py b/tests/unit/test_buyer_deal_discovery_pricing.py index b5a83d85..abd75675 100644 --- a/tests/unit/test_buyer_deal_discovery_pricing.py +++ b/tests/unit/test_buyer_deal_discovery_pricing.py @@ -16,7 +16,7 @@ import pytest -from ad_buyer.clients.unified_client import UnifiedClient +from ad_buyer.clients.unified_client import Protocol, UnifiedClient, UnifiedResult from ad_buyer.models.buyer_identity import ( AccessTier, BuyerContext, @@ -886,6 +886,72 @@ def test_get_activation_case_insensitive(self): class TestUnifiedClientBuyerDealMethods: """Test buyer deal-specific methods on UnifiedClient.""" + @pytest.mark.asyncio + async def test_discover_inventory_searches_with_query_and_filters(self, agency_identity): + client = UnifiedClient(base_url="http://localhost:8080", buyer_identity=agency_identity) + expected_result = UnifiedResult(data=[{"id": "ctv-1"}]) + client.call_tool = AsyncMock(return_value=expected_result) + + result = await client.discover_inventory( + query="CTV inventory", + channel="ctv", + max_cpm=30, + min_impressions=1000, + targeting=["household"], + publisher="example.test", + protocol=Protocol.A2A, + ) + + assert result is expected_result + client.call_tool.assert_awaited_once_with( + "search_products", + { + "query": "CTV inventory", + "filters": { + **agency_identity.to_context_dict(), + "channel": "ctv", + "maxPrice": 30, + "minImpressions": 1000, + "targeting": ["household"], + "publisher": "example.test", + }, + }, + protocol=Protocol.A2A, + ) + + @pytest.mark.asyncio + @pytest.mark.parametrize( + "filter_kwargs", + [ + {"channel": "ctv"}, + {"max_cpm": 30}, + {"min_impressions": 1000}, + {"targeting": ["household"]}, + {"publisher": "example.test"}, + ], + ) + async def test_discover_inventory_rejects_filters_without_query( + self, agency_identity, filter_kwargs + ): + client = UnifiedClient(base_url="http://localhost:8080", buyer_identity=agency_identity) + client.call_tool = AsyncMock() + + with pytest.raises(ValueError, match="Product filters require a query"): + await client.discover_inventory(**filter_kwargs) + + client.call_tool.assert_not_awaited() + + @pytest.mark.asyncio + async def test_discover_inventory_without_query_or_product_filters_lists_products( + self, agency_identity + ): + client = UnifiedClient(base_url="http://localhost:8080", buyer_identity=agency_identity) + client.call_tool = AsyncMock(return_value=UnifiedResult(data=[])) + + await client.discover_inventory() + + client.call_tool.assert_awaited_once_with("list_products", protocol=None) + def test_set_buyer_identity(self, advertiser_identity): """set_buyer_identity should store the identity.""" client = UnifiedClient(base_url="http://localhost:8080") From 286c5ad6ddd03b45b0d61e3d02746a5069e8b5db Mon Sep 17 00:00:00 2001 From: Aleksander Sekowski Date: Tue, 27 Oct 2026 22:15:25 -0700 Subject: [PATCH 2/3] fix: reject inventory filters without a query in the discovery tool DiscoverInventoryTool returns an error before calling list_products when product filters are set and query is absent. The client checks a separate product filter dict, and the example passes a query with its filters. --- examples/dsp_deal_discovery.py | 1 + src/ad_buyer/clients/unified_client.py | 28 +++++++-------- .../tools/buyer_deals/discover_inventory.py | 22 +++++++----- .../unit/test_buyer_deal_discovery_pricing.py | 34 +++++++++++++++++++ 4 files changed, 61 insertions(+), 24 deletions(-) diff --git a/examples/dsp_deal_discovery.py b/examples/dsp_deal_discovery.py index 42761d84..650ce222 100644 --- a/examples/dsp_deal_discovery.py +++ b/examples/dsp_deal_discovery.py @@ -198,6 +198,7 @@ async def demo_unified_client_dsp_methods(): # Discover inventory print("\n--- Discovering Inventory ---") result = await client.discover_inventory( + query="display inventory", channel="display", max_cpm=25.0, ) diff --git a/src/ad_buyer/clients/unified_client.py b/src/ad_buyer/clients/unified_client.py index a5eafcc8..14b94e74 100644 --- a/src/ad_buyer/clients/unified_client.py +++ b/src/ad_buyer/clients/unified_client.py @@ -485,31 +485,27 @@ async def discover_inventory( Returns: UnifiedResult with discovered inventory """ - # Build filters including identity context - filters = self._get_identity_context() + product_filters = {} if channel: - filters["channel"] = channel + product_filters["channel"] = channel if max_cpm is not None: - filters["maxPrice"] = max_cpm + product_filters["maxPrice"] = max_cpm if min_impressions is not None: - filters["minImpressions"] = min_impressions + product_filters["minImpressions"] = min_impressions if targeting: - filters["targeting"] = targeting + product_filters["targeting"] = targeting if publisher: - filters["publisher"] = publisher + product_filters["publisher"] = publisher - args = {"filters": filters} - if query: - args["query"] = query - - has_product_filters = any( - key in filters - for key in ("channel", "maxPrice", "minImpressions", "targeting", "publisher") - ) - if has_product_filters and not query: + if product_filters and not query: raise ValueError( "Product filters require a query; list_products does not support filters" ) + + filters = {**self._get_identity_context(), **product_filters} + args = {"filters": filters} + if query: + args["query"] = query if query: return await self.call_tool("search_products", args, protocol=protocol) return await self.call_tool("list_products", protocol=protocol) diff --git a/src/ad_buyer/tools/buyer_deals/discover_inventory.py b/src/ad_buyer/tools/buyer_deals/discover_inventory.py index db7b5735..1209f4fa 100644 --- a/src/ad_buyer/tools/buyer_deals/discover_inventory.py +++ b/src/ad_buyer/tools/buyer_deals/discover_inventory.py @@ -153,21 +153,27 @@ async def _arun( ) -> str: """Discover inventory with buyer identity context.""" try: - # Build filters - filters = {} + # Product filters are separate from identity. list_products cannot apply them. + product_filters = {} if channel: - filters["channel"] = channel + product_filters["channel"] = channel if max_cpm is not None: - filters["maxPrice"] = max_cpm + product_filters["maxPrice"] = max_cpm if min_impressions is not None: - filters["minImpressions"] = min_impressions + product_filters["minImpressions"] = min_impressions if targeting: - filters["targeting"] = targeting + product_filters["targeting"] = targeting if publisher: - filters["publisher"] = publisher + product_filters["publisher"] = publisher + + if product_filters and not query: + return ( + "Error discovering inventory: Product filters require a query; " + "list_products does not support filters" + ) - # Add identity context to filters identity_context = self._buyer_context.identity.to_context_dict() + filters = dict(product_filters) filters["buyer_context"] = identity_context # Execute search diff --git a/tests/unit/test_buyer_deal_discovery_pricing.py b/tests/unit/test_buyer_deal_discovery_pricing.py index abd75675..4c34922a 100644 --- a/tests/unit/test_buyer_deal_discovery_pricing.py +++ b/tests/unit/test_buyer_deal_discovery_pricing.py @@ -197,6 +197,27 @@ async def test_discover_includes_identity_context(self, mock_client, advertiser_ assert "buyer_context" in filters assert filters["buyer_context"]["access_tier"] == "advertiser" + @pytest.mark.asyncio + @pytest.mark.parametrize( + "filter_kwargs", + [ + {"channel": "ctv"}, + {"max_cpm": 20.0}, + {"min_impressions": 5_000_000}, + {"targeting": ["household"]}, + {"publisher": "hulu"}, + ], + ) + async def test_discover_rejects_product_filters_without_query( + self, mock_client, agency_context, filter_kwargs + ): + """Agents call this tool, so a filter without a query must fail here too.""" + tool = DiscoverInventoryTool(client=mock_client, buyer_context=agency_context) + result = await tool._arun(**filter_kwargs) + assert "Product filters require a query" in result + mock_client.search_products.assert_not_called() + mock_client.list_products.assert_not_called() + class TestDiscoverInventoryFormatting: """Test output formatting in DiscoverInventoryTool.""" @@ -941,6 +962,19 @@ async def test_discover_inventory_rejects_filters_without_query( client.call_tool.assert_not_awaited() + @pytest.mark.asyncio + async def test_discover_inventory_query_without_product_filters_searches(self, agency_identity): + client = UnifiedClient(base_url="http://localhost:8080", buyer_identity=agency_identity) + client.call_tool = AsyncMock(return_value=UnifiedResult(data=[])) + + await client.discover_inventory(query="CTV inventory") + + client.call_tool.assert_awaited_once_with( + "search_products", + {"query": "CTV inventory", "filters": agency_identity.to_context_dict()}, + protocol=None, + ) + @pytest.mark.asyncio async def test_discover_inventory_without_query_or_product_filters_lists_products( self, agency_identity From 76f646ba8d805d5adebf0543f5d59f02882f14aa Mon Sep 17 00:00:00 2001 From: Aleksander Sekowski Date: Wed, 4 Nov 2026 21:59:16 -0800 Subject: [PATCH 3/3] fix: apply inventory filters to the products sellers return Sellers may ignore discovery filters. The hosted agentic-direct search_products accepts only a query, and seller-agent has no search_products and a list_products without filters. UnifiedClient and DiscoverInventoryTool now apply channel, max_cpm, min_impressions, targeting and publisher to the returned products, with or without a query, and still forward them to search_products. This replaces the guard that rejected filters without a query. A product passes only if it lists a matching value, so a product missing the field is excluded. min_impressions=0 means no minimum, max_cpm=0 stays a cap, and a blank query lists products. seller-agent's {"products": [...]} listing with inventory_type and base_cpm is read. The example goes back to its filter-only call. --- docs/architecture/tools.md | 16 +- examples/dsp_deal_discovery.py | 1 - src/ad_buyer/clients/unified_client.py | 183 +++++++++-- .../tools/buyer_deals/discover_inventory.py | 65 ++-- .../unit/test_buyer_deal_discovery_pricing.py | 290 ++++++++++++++++-- 5 files changed, 460 insertions(+), 95 deletions(-) diff --git a/docs/architecture/tools.md b/docs/architecture/tools.md index 45ea1aad..8299ae91 100644 --- a/docs/architecture/tools.md +++ b/docs/architecture/tools.md @@ -199,13 +199,25 @@ Queries sellers for available inventory with identity-based access and tiered pr |-----------|------|----------|-------------| | `query` | `str` | no | Natural language query (e.g., "CTV inventory under $25 CPM") | | `channel` | `str` | no | Channel filter: `ctv`, `display`, `video`, `mobile` | -| `max_cpm` | `float` | no | Maximum CPM price | -| `min_impressions` | `int` | no | Minimum available impressions | +| `max_cpm` | `float` | no | Maximum listed CPM price | +| `min_impressions` | `int` | no | Minimum available impressions; `0` means no minimum | | `targeting` | `list[str]` | no | Required targeting capabilities | | `publisher` | `str` | no | Specific publisher to search | **Returns:** List of products with tiered pricing based on buyer identity, availability, and targeting capabilities. +A nonblank `query` calls the seller's `search_products`; otherwise the tool calls `list_products`. Sellers may ignore product filters, so the buyer applies them to the returned products in both cases (they are still forwarded to `search_products`). `UnifiedClient.discover_inventory()` does the same. A product passes a filter only if it lists a matching value, and a product that omits the field is excluded: + +| Filter | Product field read (first present) | Match | +|--------|------------------------------------|-------| +| `channel` | `channel`, `inventory_type` | equal, ignoring case | +| `max_cpm` | `basePrice`, `price`, `base_cpm` | positive and at most `max_cpm`; listed price before tier discounts | +| `min_impressions` | `availableImpressions`, `available_impressions` | at least `min_impressions` | +| `targeting` | `targeting`, `availableTargeting` (a list, or a dict with `capabilities`) | lists every requested capability, ignoring case | +| `publisher` | `publisherId`, `publisher` | equal, ignoring case | + +Blank strings and an empty `targeting` list are not filters. `max_cpm=0` is a cap, so it matches no priced product. The `{"products": [...]}` envelope that seller-agent's MCP `list_products` returns is read as a product list. + ```python from ad_buyer.tools.buyer_deals import DiscoverInventoryTool from ad_buyer.clients.unified_client import UnifiedClient diff --git a/examples/dsp_deal_discovery.py b/examples/dsp_deal_discovery.py index 650ce222..42761d84 100644 --- a/examples/dsp_deal_discovery.py +++ b/examples/dsp_deal_discovery.py @@ -198,7 +198,6 @@ async def demo_unified_client_dsp_methods(): # Discover inventory print("\n--- Discovering Inventory ---") result = await client.discover_inventory( - query="display inventory", channel="display", max_cpm=25.0, ) diff --git a/src/ad_buyer/clients/unified_client.py b/src/ad_buyer/clients/unified_client.py index 14b94e74..214d689b 100644 --- a/src/ad_buyer/clients/unified_client.py +++ b/src/ad_buyer/clients/unified_client.py @@ -3,7 +3,7 @@ """Unified client for IAB agentic-direct server supporting both MCP and A2A protocols.""" -from dataclasses import dataclass +from dataclasses import dataclass, replace from enum import Enum from typing import TYPE_CHECKING, Any @@ -60,6 +60,131 @@ def from_a2a(cls, response: A2AResponse) -> "UnifiedResult": ) +# Product fields the discovery filters read; the first key present wins, as in +# DiscoverInventoryTool's display. inventory_type and base_cpm are what +# seller-agent's MCP list_products returns. +_CHANNEL_KEYS = ("channel", "inventory_type") +_PRICE_KEYS = ("basePrice", "price", "base_cpm") +_IMPRESSIONS_KEYS = ("availableImpressions", "available_impressions") +_TARGETING_KEYS = ("targeting", "availableTargeting") +_PUBLISHER_KEYS = ("publisherId", "publisher") + + +def build_product_filters( + channel: str | None = None, + max_cpm: float | None = None, + min_impressions: int | None = None, + targeting: list[str] | None = None, + publisher: str | None = None, +) -> dict[str, Any]: + """Collect the product filters that are set, keyed as sent to search_products. + + Blank strings, an empty targeting list and min_impressions=0 (no minimum) + are not filters. max_cpm=0 is a filter. + """ + if isinstance(channel, str): + channel = channel.strip() + if isinstance(publisher, str): + publisher = publisher.strip() + + filters: dict[str, Any] = {} + if channel: + filters["channel"] = channel + if max_cpm is not None: + filters["maxPrice"] = max_cpm + if min_impressions: + filters["minImpressions"] = min_impressions + if targeting: + filters["targeting"] = targeting + if publisher: + filters["publisher"] = publisher + return filters + + +def apply_product_filters(data: Any, filters: dict[str, Any]) -> tuple[list[Any], int]: + """Keep the products that satisfy every filter. + + ``data`` is a product list, a single product dict, or seller-agent's + ``{"products": [...]}`` envelope. A product passes a filter only if it lists + a value that satisfies it, so products missing the field and entries that + are not dicts are excluded. + + Returns: + The matching products and the number excluded. + """ + if isinstance(data, dict) and isinstance(data.get("products"), list): + products = data["products"] + elif isinstance(data, list): + products = data + elif data: + products = [data] + else: + products = [] + + checks = [(_PRODUCT_FILTER_CHECKS[key], value) for key, value in filters.items()] + kept = [ + product + for product in products + if isinstance(product, dict) and all(check(product, value) for check, value in checks) + ] + return kept, len(products) - len(kept) + + +def _product_value(product: dict, keys: tuple[str, ...]) -> Any: + for key in keys: + if key in product: + return product[key] + return None + + +def _is_number(value: Any) -> bool: + return isinstance(value, (int, float)) and not isinstance(value, bool) + + +def _same_text(value: Any, wanted: Any) -> bool: + return isinstance(value, str) and value.strip().lower() == str(wanted).strip().lower() + + +def _matches_channel(product: dict, channel: str) -> bool: + return _same_text(_product_value(product, _CHANNEL_KEYS), channel) + + +def _within_max_cpm(product: dict, max_cpm: float) -> bool: + # Listed price, before tier discounts. A missing or non-positive price is + # "pricing unavailable", as DiscoverInventoryTool displays it. + price = _product_value(product, _PRICE_KEYS) + return _is_number(price) and 0 < price <= max_cpm + + +def _meets_min_impressions(product: dict, min_impressions: int) -> bool: + available = _product_value(product, _IMPRESSIONS_KEYS) + return _is_number(available) and available >= min_impressions + + +def _offers_targeting(product: dict, required: list[str]) -> bool: + offered = _product_value(product, _TARGETING_KEYS) + if isinstance(offered, dict): + # OpenDirect Product.targeting lists them under "capabilities". + offered = offered.get("capabilities") + if not isinstance(offered, list): + return False + available = {str(item).strip().lower() for item in offered} + return all(str(item).strip().lower() in available for item in required) + + +def _matches_publisher(product: dict, publisher: str) -> bool: + return _same_text(_product_value(product, _PUBLISHER_KEYS), publisher) + + +_PRODUCT_FILTER_CHECKS = { + "channel": _matches_channel, + "maxPrice": _within_max_cpm, + "minImpressions": _meets_min_impressions, + "targeting": _offers_targeting, + "publisher": _matches_publisher, +} + + class UnifiedClient: """Unified client that supports both MCP and A2A protocols. @@ -473,42 +598,44 @@ async def discover_inventory( Queries sellers for available inventory, presenting the buyer's identity to unlock tiered pricing and premium inventory access. + A nonblank query calls search_products; otherwise list_products. Product + filters are forwarded to search_products and also applied here to the + returned products, because sellers may ignore them. A product passes + only if it lists a matching value; products missing a filtered field + are excluded. + Args: query: Natural language query (e.g., 'CTV inventory under $25 CPM') channel: Channel filter ('ctv', 'display', 'video', 'mobile') - max_cpm: Maximum CPM price filter - min_impressions: Minimum available impressions filter - targeting: Required targeting capabilities + max_cpm: Maximum listed CPM; unpriced products are excluded. 0 is a cap. + min_impressions: Minimum available impressions; 0 means no minimum + targeting: Required targeting capabilities; the product must list all publisher: Specific publisher to search protocol: Protocol to use for the request Returns: - UnifiedResult with discovered inventory + UnifiedResult with discovered inventory. With product filters set and + a successful call, ``data`` is the list of matching product dicts. """ - product_filters = {} - if channel: - product_filters["channel"] = channel - if max_cpm is not None: - product_filters["maxPrice"] = max_cpm - if min_impressions is not None: - product_filters["minImpressions"] = min_impressions - if targeting: - product_filters["targeting"] = targeting - if publisher: - product_filters["publisher"] = publisher - - if product_filters and not query: - raise ValueError( - "Product filters require a query; list_products does not support filters" - ) + product_filters = build_product_filters( + channel=channel, + max_cpm=max_cpm, + min_impressions=min_impressions, + targeting=targeting, + publisher=publisher, + ) - filters = {**self._get_identity_context(), **product_filters} - args = {"filters": filters} - if query: - args["query"] = query - if query: - return await self.call_tool("search_products", args, protocol=protocol) - return await self.call_tool("list_products", protocol=protocol) + if query and query.strip(): + filters = {**self._get_identity_context(), **product_filters} + args = {"query": query, "filters": filters} + result = await self.call_tool("search_products", args, protocol=protocol) + else: + result = await self.call_tool("list_products", protocol=protocol) + + if not product_filters or not result.success: + return result + products, _ = apply_product_filters(result.data, product_filters) + return replace(result, data=products) async def get_pricing( self, diff --git a/src/ad_buyer/tools/buyer_deals/discover_inventory.py b/src/ad_buyer/tools/buyer_deals/discover_inventory.py index 1209f4fa..cf6bce87 100644 --- a/src/ad_buyer/tools/buyer_deals/discover_inventory.py +++ b/src/ad_buyer/tools/buyer_deals/discover_inventory.py @@ -12,7 +12,11 @@ from ...async_utils import run_async from ...booking.pricing import PricingCalculator from ...clients.sgp_client import SGPClient, SGPClientError, extract_product_domain -from ...clients.unified_client import UnifiedClient +from ...clients.unified_client import ( + UnifiedClient, + apply_product_filters, + build_product_filters, +) from ...models.buyer_identity import BuyerContext from ...models.sgp import ApprovalRecord, normalize_unknown_policy from ...security.prompt_sanitizer import sanitize_untrusted_text @@ -33,17 +37,17 @@ class DiscoverInventoryInput(BaseModel): ) max_cpm: float | None = Field( default=None, - description="Maximum CPM price filter", + description="Maximum listed CPM; products without a listed price are excluded", ge=0, ) min_impressions: int | None = Field( default=None, - description="Minimum available impressions filter", + description="Minimum available impressions; 0 means no minimum", ge=0, ) targeting: list[str] | None = Field( default=None, - description="Required targeting capabilities (e.g., ['household', 'geo', 'demographic'])", + description="Targeting capabilities a product must all list (e.g., ['household', 'geo'])", ) publisher: str | None = Field( default=None, @@ -67,13 +71,15 @@ class DiscoverInventoryTool(BaseTool): name: str = "discover_inventory" description: str = """Discover available advertising inventory from sellers. Presents buyer identity to unlock tiered pricing and premium access. +Filters work with or without a query. They are applied to the products the seller +returns, and a product that does not list a filtered field is excluded. Args: query: Natural language query (e.g., 'CTV inventory under $25 CPM') channel: Channel filter ('ctv', 'display', 'video', 'mobile') - max_cpm: Maximum CPM price - min_impressions: Minimum available impressions - targeting: Required targeting capabilities + max_cpm: Maximum listed CPM; unpriced products are excluded + min_impressions: Minimum available impressions; 0 means no minimum + targeting: Required targeting capabilities; a product must list all of them publisher: Specific publisher to search Returns: @@ -153,31 +159,19 @@ async def _arun( ) -> str: """Discover inventory with buyer identity context.""" try: - # Product filters are separate from identity. list_products cannot apply them. - product_filters = {} - if channel: - product_filters["channel"] = channel - if max_cpm is not None: - product_filters["maxPrice"] = max_cpm - if min_impressions is not None: - product_filters["minImpressions"] = min_impressions - if targeting: - product_filters["targeting"] = targeting - if publisher: - product_filters["publisher"] = publisher - - if product_filters and not query: - return ( - "Error discovering inventory: Product filters require a query; " - "list_products does not support filters" - ) - + product_filters = build_product_filters( + channel=channel, + max_cpm=max_cpm, + min_impressions=min_impressions, + targeting=targeting, + publisher=publisher, + ) identity_context = self._buyer_context.identity.to_context_dict() filters = dict(product_filters) filters["buyer_context"] = identity_context # Execute search - if query: + if query and query.strip(): result = await self._client.search_products( query=query, filters=filters if filters else None, @@ -188,9 +182,20 @@ async def _arun( if not result.success: return f"Error discovering inventory: {result.error}" - approvals = await self._fetch_approvals(result.data) - filtered, filter_summary = self._apply_enforcement(result.data, approvals) - return self._format_results(filtered, identity_context, approvals, filter_summary) + products, excluded = result.data, 0 + if product_filters: + # Sellers may ignore the filters, so apply them to what came back. + products, excluded = apply_product_filters(result.data, product_filters) + + approvals = await self._fetch_approvals(products) + filtered, filter_summary = self._apply_enforcement(products, approvals) + output = self._format_results(filtered, identity_context, approvals, filter_summary) + if excluded: + output += ( + f"\nProduct filters excluded {excluded} product(s), " + "including any that do not list a filtered field." + ) + return output except SGPClientError as e: # Reached only when enforcement is on; _fetch_approvals swallows diff --git a/tests/unit/test_buyer_deal_discovery_pricing.py b/tests/unit/test_buyer_deal_discovery_pricing.py index 4c34922a..34c2a3dc 100644 --- a/tests/unit/test_buyer_deal_discovery_pricing.py +++ b/tests/unit/test_buyer_deal_discovery_pricing.py @@ -119,6 +119,70 @@ def _product(product_id="prod_001", name="Test Product", base_price=20.00, **kwa return data +# Each case: filter kwargs, fields of a product that matches, fields of one that +# does not. _filter_case_products adds a third product with no filterable fields, +# which every filter must exclude. +PRODUCT_FILTER_CASES = [ + pytest.param({"channel": "ctv"}, {"channel": "CTV"}, {"channel": "display"}, id="channel"), + pytest.param({"max_cpm": 20.0}, {"basePrice": 15.0}, {"basePrice": 25.0}, id="max_cpm"), + pytest.param( + {"min_impressions": 5_000_000}, + {"availableImpressions": 6_000_000}, + {"availableImpressions": 1_000_000}, + id="min_impressions", + ), + pytest.param( + {"targeting": ["household", "geo"]}, + {"targeting": ["Household", "geo", "demographic"]}, + {"targeting": ["household"]}, + id="targeting", + ), + pytest.param( + {"publisher": "hulu"}, {"publisherId": "hulu"}, {"publisherId": "espn"}, id="publisher" + ), +] + + +def _filter_case_products(match_fields, miss_fields): + return [ + {"id": "match", "name": "Matching Product", **match_fields}, + {"id": "miss", "name": "Other Product", **miss_fields}, + {"id": "bare", "name": "Bare Product"}, + ] + + +# Shape of seller-agent's MCP list_products response (src/ad_seller/interfaces/mcp_server.py). +SELLER_AGENT_LISTING = { + "products": [ + { + "product_id": "ctv-premium", + "name": "CTV Premium", + "inventory_type": "ctv", + "base_cpm": 35.0, + "floor_cpm": 28.0, + "deal_types": ["preferreddeal"], + }, + { + "product_id": "ctv-on-request", + "name": "CTV On Request", + "inventory_type": "ctv", + "base_cpm": None, + "floor_cpm": None, + "deal_types": ["programmaticguaranteed"], + }, + { + "product_id": "display-run", + "name": "Display Run", + "inventory_type": "display", + "base_cpm": 12.0, + "floor_cpm": 8.0, + "deal_types": ["preferreddeal"], + }, + ], + "count": 3, +} + + # ============================================================================= # DiscoverInventoryTool - Extended Tests # ============================================================================= @@ -198,25 +262,90 @@ async def test_discover_includes_identity_context(self, mock_client, advertiser_ assert filters["buyer_context"]["access_tier"] == "advertiser" @pytest.mark.asyncio - @pytest.mark.parametrize( - "filter_kwargs", - [ - {"channel": "ctv"}, - {"max_cpm": 20.0}, - {"min_impressions": 5_000_000}, - {"targeting": ["household"]}, - {"publisher": "hulu"}, - ], - ) - async def test_discover_rejects_product_filters_without_query( - self, mock_client, agency_context, filter_kwargs + @pytest.mark.parametrize(("filter_kwargs", "match", "miss"), PRODUCT_FILTER_CASES) + async def test_discover_applies_filters_to_listed_products( + self, mock_client, agency_context, filter_kwargs, match, miss ): - """Agents call this tool, so a filter without a query must fail here too.""" + """Without a query, the tool lists products and applies the filter to them.""" + mock_client.list_products.return_value = MagicMock( + success=True, data=_filter_case_products(match, miss) + ) tool = DiscoverInventoryTool(client=mock_client, buyer_context=agency_context) result = await tool._arun(**filter_kwargs) - assert "Product filters require a query" in result + + mock_client.list_products.assert_awaited_once_with() mock_client.search_products.assert_not_called() - mock_client.list_products.assert_not_called() + assert "Matching Product" in result + assert "Other Product" not in result + assert "Bare Product" not in result + assert "Total products found: 1" in result + assert "Product filters excluded 2 product(s)" in result + + @pytest.mark.asyncio + @pytest.mark.parametrize(("filter_kwargs", "match", "miss"), PRODUCT_FILTER_CASES) + async def test_discover_applies_filters_the_search_ignored( + self, mock_client, agency_context, filter_kwargs, match, miss + ): + """A seller's search_products may accept only the query (#140).""" + mock_client.search_products.return_value = MagicMock( + success=True, data=_filter_case_products(match, miss) + ) + tool = DiscoverInventoryTool(client=mock_client, buyer_context=agency_context) + result = await tool._arun(query="inventory", **filter_kwargs) + + assert "Matching Product" in result + assert "Other Product" not in result + assert "Bare Product" not in result + assert "Total products found: 1" in result + + @pytest.mark.asyncio + @pytest.mark.parametrize("query", [None, "", " "]) + async def test_discover_blank_query_lists_products(self, mock_client, agency_context, query): + mock_client.list_products.return_value = MagicMock( + success=True, data=[_product(name="CTV Product", channel="ctv")] + ) + tool = DiscoverInventoryTool(client=mock_client, buyer_context=agency_context) + result = await tool._arun(query=query, channel="ctv") + + mock_client.list_products.assert_awaited_once_with() + mock_client.search_products.assert_not_called() + assert "CTV Product" in result + + @pytest.mark.asyncio + async def test_discover_zero_min_impressions_is_no_minimum(self, mock_client, agency_context): + mock_client.list_products.return_value = MagicMock( + success=True, data=[{"id": "bare", "name": "Bare Product"}] + ) + tool = DiscoverInventoryTool(client=mock_client, buyer_context=agency_context) + result = await tool._arun(min_impressions=0) + + assert "Bare Product" in result + assert "Product filters excluded" not in result + + @pytest.mark.asyncio + async def test_discover_zero_max_cpm_is_a_cap(self, mock_client, agency_context): + mock_client.search_products.return_value = MagicMock( + success=True, data=[_product(name="Ten Dollar Product", base_price=10.0)] + ) + tool = DiscoverInventoryTool(client=mock_client, buyer_context=agency_context) + result = await tool._arun(query="inventory", max_cpm=0) + + filters = mock_client.search_products.call_args.kwargs["filters"] + assert filters["maxPrice"] == 0 + assert "Ten Dollar Product" not in result + assert "No inventory found" in result + assert "Product filters excluded 1 product(s)" in result + + @pytest.mark.asyncio + async def test_discover_filters_seller_agent_listing(self, mock_client, agency_context): + mock_client.list_products.return_value = MagicMock(success=True, data=SELLER_AGENT_LISTING) + tool = DiscoverInventoryTool(client=mock_client, buyer_context=agency_context) + result = await tool._arun(channel="ctv", max_cpm=40.0) + + assert "CTV Premium" in result + assert "CTV On Request" not in result + assert "Display Run" not in result + assert "Total products found: 1" in result class TestDiscoverInventoryFormatting: @@ -910,8 +1039,18 @@ class TestUnifiedClientBuyerDealMethods: @pytest.mark.asyncio async def test_discover_inventory_searches_with_query_and_filters(self, agency_identity): client = UnifiedClient(base_url="http://localhost:8080", buyer_identity=agency_identity) - expected_result = UnifiedResult(data=[{"id": "ctv-1"}]) - client.call_tool = AsyncMock(return_value=expected_result) + matching = { + "id": "ctv-1", + "channel": "ctv", + "basePrice": 25.0, + "availableImpressions": 5000, + "targeting": ["household", "geo"], + "publisherId": "example.test", + } + too_expensive = {**matching, "id": "ctv-2", "basePrice": 45.0} + client.call_tool = AsyncMock( + return_value=UnifiedResult(data=[matching, too_expensive], protocol=Protocol.A2A) + ) result = await client.discover_inventory( query="CTV inventory", @@ -923,7 +1062,9 @@ async def test_discover_inventory_searches_with_query_and_filters(self, agency_i protocol=Protocol.A2A, ) - assert result is expected_result + assert result.success + assert result.protocol is Protocol.A2A + assert result.data == [matching] client.call_tool.assert_awaited_once_with( "search_products", { @@ -941,26 +1082,107 @@ async def test_discover_inventory_searches_with_query_and_filters(self, agency_i ) @pytest.mark.asyncio - @pytest.mark.parametrize( - "filter_kwargs", - [ - {"channel": "ctv"}, - {"max_cpm": 30}, - {"min_impressions": 1000}, - {"targeting": ["household"]}, - {"publisher": "example.test"}, - ], - ) - async def test_discover_inventory_rejects_filters_without_query( - self, agency_identity, filter_kwargs + @pytest.mark.parametrize(("filter_kwargs", "match", "miss"), PRODUCT_FILTER_CASES) + async def test_discover_inventory_lists_and_filters_without_query( + self, agency_identity, filter_kwargs, match, miss + ): + client = UnifiedClient(base_url="http://localhost:8080", buyer_identity=agency_identity) + products = _filter_case_products(match, miss) + client.call_tool = AsyncMock(return_value=UnifiedResult(data=products)) + + result = await client.discover_inventory(**filter_kwargs) + + client.call_tool.assert_awaited_once_with("list_products", protocol=None) + assert result.data == [products[0]] + + @pytest.mark.asyncio + @pytest.mark.parametrize(("filter_kwargs", "match", "miss"), PRODUCT_FILTER_CASES) + async def test_discover_inventory_filters_what_search_returns( + self, agency_identity, filter_kwargs, match, miss ): + """A seller's search_products may accept only the query (#140).""" + client = UnifiedClient(base_url="http://localhost:8080", buyer_identity=agency_identity) + products = _filter_case_products(match, miss) + client.call_tool = AsyncMock(return_value=UnifiedResult(data=products)) + + result = await client.discover_inventory(query="inventory", **filter_kwargs) + + assert client.call_tool.await_args.args[0] == "search_products" + assert result.data == [products[0]] + + @pytest.mark.asyncio + async def test_discover_inventory_filters_a_single_product_response(self, agency_identity): + """The hosted agentic-direct server answers search_products with one product.""" + client = UnifiedClient(base_url="http://localhost:8080", buyer_identity=agency_identity) + product = {"id": "mock-1", "name": "Mock Product", "basePrice": 10.0} + client.call_tool = AsyncMock(return_value=UnifiedResult(data=product)) + + over_cap = await client.discover_inventory(query="inventory", max_cpm=1.0) + under_cap = await client.discover_inventory(query="inventory", max_cpm=20.0) + + assert over_cap.data == [] + assert under_cap.data == [product] + + @pytest.mark.asyncio + async def test_discover_inventory_filters_seller_agent_listing(self, agency_identity): + client = UnifiedClient(base_url="http://localhost:8080", buyer_identity=agency_identity) + client.call_tool = AsyncMock(return_value=UnifiedResult(data=SELLER_AGENT_LISTING)) + + result = await client.discover_inventory(channel="ctv", max_cpm=40.0) + + client.call_tool.assert_awaited_once_with("list_products", protocol=None) + assert [p["product_id"] for p in result.data] == ["ctv-premium"] + + @pytest.mark.asyncio + @pytest.mark.parametrize("query", [None, "", " "]) + async def test_discover_inventory_blank_query_lists_products(self, agency_identity, query): + client = UnifiedClient(base_url="http://localhost:8080", buyer_identity=agency_identity) + product = {"id": "ctv-1", "channel": "ctv"} + client.call_tool = AsyncMock(return_value=UnifiedResult(data=[product])) + + result = await client.discover_inventory(query=query, channel="ctv") + + client.call_tool.assert_awaited_once_with("list_products", protocol=None) + assert result.data == [product] + + @pytest.mark.asyncio + async def test_discover_inventory_zero_min_impressions_is_no_minimum(self, agency_identity): + client = UnifiedClient(base_url="http://localhost:8080", buyer_identity=agency_identity) + bare = {"id": "bare"} + client.call_tool = AsyncMock(return_value=UnifiedResult(data=[bare])) + + listed = await client.discover_inventory(min_impressions=0) + await client.discover_inventory(query="inventory", min_impressions=0) + + assert listed.data == [bare] + assert client.call_tool.await_args.args == ( + "search_products", + {"query": "inventory", "filters": agency_identity.to_context_dict()}, + ) + + @pytest.mark.asyncio + async def test_discover_inventory_zero_max_cpm_is_a_cap(self, agency_identity): + client = UnifiedClient(base_url="http://localhost:8080", buyer_identity=agency_identity) + client.call_tool = AsyncMock( + return_value=UnifiedResult(data=[{"id": "ten", "basePrice": 10.0}]) + ) + + listed = await client.discover_inventory(max_cpm=0) + searched = await client.discover_inventory(query="inventory", max_cpm=0) + + assert listed.data == [] + assert searched.data == [] + assert client.call_tool.await_args.args[1]["filters"]["maxPrice"] == 0 + + @pytest.mark.asyncio + async def test_discover_inventory_returns_a_failed_call_unchanged(self, agency_identity): client = UnifiedClient(base_url="http://localhost:8080", buyer_identity=agency_identity) - client.call_tool = AsyncMock() + failed = UnifiedResult(success=False, data="Unknown tool: search_products") + client.call_tool = AsyncMock(return_value=failed) - with pytest.raises(ValueError, match="Product filters require a query"): - await client.discover_inventory(**filter_kwargs) + result = await client.discover_inventory(query="inventory", channel="ctv") - client.call_tool.assert_not_awaited() + assert result is failed @pytest.mark.asyncio async def test_discover_inventory_query_without_product_filters_searches(self, agency_identity):