comply_test_controller's simulate_delivery dispatcher drops viewability/reach/frequency/reach_window params

[austinwiltshire]

Summary

register_test_controller's dispatch for the simulate_delivery scenario hardcodes exactly five forwarded kwargs (media_buy_id, impressions, clicks, conversions, reported_spend) to the TestControllerStore method — viewability, reach, frequency, and reach_window are silently dropped before the method is ever called, even though this same repo's own bundled schema (schemas/cache/3.1/compliance/comply-test-controller-request.json) documents all four as real simulate_delivery params (viewability's own description: "sellers MUST surface these values inside viewability on the next get_media_buy_delivery response").

Confirmed present in v6.4.0 (pinned) and still present in the latest v8.0.0-beta.10 pre-release — not yet fixed.

Impact

A TestControllerStore subclass that correctly implements simulate_delivery(..., viewability=...) per the documented schema can never actually receive that value over real MCP/A2A calls — only when called directly in-process (e.g. a unit test bypassing this dispatcher). This makes any storyboard scenario exercising viewability/reach/frequency permanently unwinnable regardless of how correctly the seller implements the feature.

Evidence

• Dispatch code, identical in both versions checked:

  adcp-client-python/src/adcp/server/test_controller.py

  Lines 889 to 897 in 7c81056

  	elif scenario == "simulate_delivery":
  	result = await method(
  	media_buy_id=scenario_params["media_buy_id"],
  	impressions=scenario_params.get("impressions"),
  	clicks=scenario_params.get("clicks"),
  	conversions=scenario_params.get("conversions"),
  	reported_spend=scenario_params.get("reported_spend"),
  	**extra,
  	)

• This repo's own schema documenting viewability as a simulate_delivery param:

  adcp-client-python/schemas/cache/3.1/compliance/comply-test-controller-request.json

  Lines 769 to 799 in 7c81056

  	"viewability": {
  	"type": "object",
  	"description": "Viewability metrics to inject into the simulated delivery row. Used by simulate_delivery. When present, sellers MUST surface these values inside `viewability` on the next get_media_buy_delivery response. Mirrors the `viewability` block in delivery-metrics.json: measurable_impressions is the shared denominator for viewable_rate and viewed_seconds. Sellers SHOULD include standard whenever measured values are present.",
  	"properties": {
  	"measurable_impressions": {
  	"type": "number",
  	"minimum": 0,
  	"description": "Impressions where viewability could be measured. Coverage denominator for viewable_rate and viewed_seconds."
  	},
  	"viewable_impressions": {
  	"type": "number",
  	"minimum": 0,
  	"description": "Impressions that met the viewability threshold."
  	},
  	"viewable_rate": {
  	"type": "number",
  	"minimum": 0,
  	"maximum": 1,
  	"description": "Viewable impression rate (viewable_impressions / measurable_impressions)."
  	},
  	"viewed_seconds": {
  	"type": "number",
  	"minimum": 0,
  	"description": "Average in-view duration per measurable impression in seconds. Reporting counterpart to the viewed_seconds optimization metric."
  	},
  	"standard": {
  	"$ref": "../enums/viewability-standard.json",
  	"description": "Viewability measurement standard governing the threshold for viewable_rate and viewed_seconds."
  	}
  	},
  	"additionalProperties": true

Ask

• Forward viewability, reach, frequency, reach_window the same way the other simulate_delivery params are forwarded — or better, forward whatever the store method's own signature actually accepts (via inspect.signature) so new documented params don't silently require a matching dispatcher update every time.

[bokelley]

Triage

Classification: Bug
Bucket(s): handlers (label gap: handlers label not in repo — noted in run summary)
Status: ready-to-implement
Milestone: (omitted — no target version named)

What the experts said:

• code-reviewer: Bug confirmed; explicit forwarding preferred over inspect-driven; reach/frequency must be float | None, viewability/reach_window must be dict[str, Any] | None; all four params are optional with None defaults so adding them is non-breaking.
• dx-expert: High-impact silent failure — values arrive on the wire and are discarded with no error; storyboard scenarios exercising viewability/reach/frequency are unwinnable regardless of how correct the subclass is; explicit forwarding matches the rest of the dispatcher's uniform pattern.

My take: Both experts converge on the same fix. The four missing params are documented in the bundled schema, silently dropped at the dispatch layer, and confirmed absent from the TestControllerStore base class signature — a clean, non-breaking two-site fix.

Implementation brief:

• File: src/adcp/server/test_controller.py
• Site 1 — base class signature (line 313): Add four keyword-only params (after existing positional params, before *):

  reach: float | None = None,
  frequency: float | None = None,
  reach_window: dict[str, Any] | None = None,
  viewability: dict[str, Any] | None = None,

• Site 2 — dispatcher branch (line 897): Add the four params to the simulate_delivery await method(...) call using .get():

  reach=scenario_params.get("reach"),
  frequency=scenario_params.get("frequency"),
  reach_window=scenario_params.get("reach_window"),
  viewability=scenario_params.get("viewability"),

• Tests: Add a unit test that passes all four new params through the dispatcher and asserts the store method receives them (the existing test file pattern for simulate_delivery is the right model).
• Non-breaking rationale: All four params are optional with None defaults; existing subclasses that don't declare them keep working; no public symbols removed or renamed; no required-param additions.
• Checks required: pytest on the test_controller test subset, mypy src/, ruff check ., black --check .

---

Triaged by Claude Code. Session: https://claude.ai/code/session_01PUiiWfagNXU1V2w8obj9iQ

---

Generated by Claude Code