feat(preview-utils): adopt self-rendering product_card (5/5 of #1902 sweep — CI green!)#1908
Merged
bokelley merged 3 commits intoMay 21, 2026
Conversation
AdCP 3.1.0-beta.2 changed product_card from creative-agent-rendered
({format_id, manifest}) to a self-contained visual card ({image, title,
description, price_label, cta_label}). The card IS the preview now;
no creative-agent round-trip is required.
batchPreviewProducts rewritten to extract product_card.image?.url
directly. The creativeAgentClient + options parameters are retained
for signature compatibility (renamed to _-prefixed unused args)
and marked @deprecated; planned removal in 8.0 final or 9.0.
format_card / batchPreviewFormats are unchanged — only product_card
had this spec migration in 3.1.0-beta.2.
Adopter migration: existing calls keep working (PreviewResult[] still
populated with previewUrl from the new inline image.url). Direct
product.product_card?.image?.url access is the new recommended path.
Part of the #1902 8.0-beta sweep (5/5 structural breaks closed — CI
should now be green on the foundation stack).
There was a problem hiding this comment.
LGTM. Right shape for the 3.1.0-beta.2 migration — the spec note says "Receivers render the card directly from these fields," so a direct product_card?.image?.url read is the witness move; the creative-agent round-trip was translation the new spec doesn't ask for.
Things I checked
- Generated type matches the new card shape —
src/lib/types/tools.generated.ts:1337-1359is{ image, title, description, price_label, cta_label }withImageAsset.url: stringrequired. - New body is correct — optional chain handles
product_cardabsent,imageabsent, andurlabsent (the last can't happen given the generated type, but the guard is cheap). No fabrication, no fallback URL. asyncover a sync body is the right deprecation shim — call sites keep awaiting, return type is byte-identical.- Signature compatibility holds —
_creativeAgentClient,_optionsunderscore-prefixed; adopters' calls keep compiling. - Changeset is
major(.changeset/product-card-self-rendering.md). Correct:previewIdno longer populated on the hot path anderrorstrings disappear. Both are observable, both warrant the bump. - Orphaned helpers (
getCacheKey,getCachedPreview,setCachedPreview,previewCache,clearPreviewCache) — all still load-bearing forbatchPreviewFormatsatsrc/lib/utils/preview-utils.ts:173-308, andclearPreviewCacheis re-exported. Leave them. - No tests reference
batchPreviewProductsdirectly (grep clean). Example callers atexamples/batch-preview-test.ts:127,151,253andexamples/debug-preview-with-logging.ts:44keep compiling against the preserved signature.
Follow-ups (non-blocking — file as issues)
- Examples still call
batchPreviewProductsand demonstrate cache reuse (examples/batch-preview-test.ts:151). The cache claim is no longer meaningful for products. Either point the examples atproduct.product_card?.image?.urldirectly or add a banner noting the deprecation. Out of scope for this PR. - Loss of
error: 'Preview generation failed'for product rows is the right call under the new spec — "noimage.url" is a data state, not a failure — but worth a sentence in release notes for adopters who were branching onresult.error. The changeset prose covers the happy path; the silent-error case is what catches people.
Minor nits (non-blocking)
- Unused import.
src/lib/utils/preview-utils.ts:5importsPreviewCreativeResponsewhich is unused (pre-existing, not introduced here —PreviewCreativeRequeston the same line is still used bybatchPreviewFormatsat L248). Trivial split-and-drop while you're here. - Redundant eslint pragma.
src/lib/utils/preview-utils.ts:130—// eslint-disable-next-line @typescript-eslint/no-unused-varsaboveexport async functiononly suppresses the next line; the unused args are two and three lines down. The_-prefix on L133-134 is what's actually doing the work in most configs. Pragma can be dropped.
Safe to merge. Closes 5/5 of the #1902 sweep.
bokelley
added a commit
that referenced
this pull request
May 22, 2026
…ion) (#1902) * feat!: cut 8.0-beta line (#1580) — flip ADCP_VERSION to 3.1.0-beta.2 Foundation for the 8.0-beta line per docs/development/v8.0-beta-plan.md. Enters changesets pre-mode with tag 'beta'; subsequent releases publish as 8.0.0-beta.N under the @beta npm dist-tag until upstream AdCP 3.1 goes GA. Changes: - ADCP_VERSION file: 3.0.12 → 3.1.0-beta.2 - src/lib/version.ts regenerated via sync-version --force - scripts/sync-version.ts: extended buildCompatibleVersions() to accept 3.1.0-beta.x pins; retains 3.0.x GA compat through LAST_3_0_GA_PATCH=12 so an 8.0-beta SDK still talks to a 3.0-pinned seller per spec's open-enum wire compat. - schemas/cache/3.1.0-beta.2 synced as primary (cosign verified) - Regenerated types: tools, schemas, core, manifest (80 codes, 61 tools, 20 specialisms), enums, entity-hydration map - FORWARD_COMPAT_ERROR_CODES emptied — AUTH_MISSING/AUTH_INVALID/ AGENT_SUSPENDED/AGENT_BLOCKED are now in primary manifest-driven ErrorCodeValues. Compile-time disjointness assertion would fail if a code returned to the overlay after manifest absorption. - BuyerRetryPolicy.DEFAULT_CODE_POLICY: added 31 entries for new 3.1 codes (provenance, billing, format-projection, pixel-tracker, scope/permission, configuration, brand, idempotency-in-flight, credential-in-args, retention, etc.) - Mechanical undefined-guards in responses.ts, signals.ts, test-controller-bridge.ts, test-controller.ts What's red (separate follow-up PRs, listed in the changeset): - OutcomeMeasurement export removed (replaced by ?) - Brand object lost categories field (spec migration) - AssetVariant became a union with array - creative-asset shape changed (format_id/manifest) - ControllerScenario gained new enum values; exhaustiveness guard fails - Envelope status REQUIRED on auto-registered get_adcp_capabilities These are all per-feature spec migrations that map to needs:adcp-3.1 queue items; will land as separate PRs against this branch line. * fix(retry-policy): add 'capability' escalate reason + enumerate divergence Addresses non-blocking review feedback on #1902: - code-reviewer: 'capability' missing from RetryDecision.escalate.reason union; format-projection + pixel-tracker codes were routed to 'commercial' with a TODO comment. - ad-tech-protocol-expert: divergence comment on the escalate union should enumerate the billing- and provenance-family escalations explicitly so future readers don't "fix" the apparent drift from spec recovery values. Changes: - Add 'capability' to the escalate.reason union with a comment listing the 7 format-projection and pixel-tracker codes that route to it. - Route the 7 codes from 'commercial' to 'capability'. Dashboards can now filter implementation-choice tickets from commercial-policy ones. - Extend the JSDoc on the existing reasons to enumerate billing + provenance escalations and the new SCOPE_INSUFFICIENT / READ_ONLY_SCOPE / etc. routes. - Note that FORMAT_DECLARATION_* codes are spec-advisory (emitted in errors[] on a 200-success), so the retry policy primarily fires when an adopter explicitly routes them; separate follow-up to revisit advisory-vs-error policy dispatch. * fix(types): handle OutcomeMeasurement rename to OutcomeMeasurementDeprecated (#1903) AdCP 3.1.0-beta.2 renamed the OutcomeMeasurement interface to OutcomeMeasurementDeprecated to signal the surface is on the 4.0 removal track. Update compat.ts and the index.ts re-export to point at the new name, with the original name preserved as a re-export alias so existing adopter imports keep working. Part of the #1902 8.0-beta sweep (1/5 structural breaks). * feat(test-controller): promote query_upstream_traffic to CONTROLLER_SCENARIOS (#1905) AdCP 3.1.0-beta.2 added query_upstream_traffic to ListScenariosSuccess['scenarios'] (spec PR adcp#3816 landed). Promote it from the open-extension literal-string path to a first-class CONTROLLER_SCENARIOS member. Resolves the test-controller.ts:210 exhaustiveness guard from the #1902 foundation sweep. Changes: - CONTROLLER_SCENARIOS.QUERY_UPSTREAM_TRAFFIC added as first-class. - SCENARIO_MAP extended with the queryUpstreamTraffic → QUERY_UPSTREAM_TRAFFIC mapping (auto-advertised via scenariosFromStore, the canonical typed path). - Removed local QUERY_UPSTREAM_TRAFFIC_SCENARIO literal and the `as unknown as ComplyTestControllerResponse` cast at the dispatcher call site — UpstreamTrafficSuccess is now in the generated union. - Exhaustive-scenario test extended with queryUpstreamTraffic store method so the CONTROLLER_SCENARIOS / SCENARIO_MAP coverage invariant holds. 114/114 affected tests pass. Part of the #1902 8.0-beta sweep (2/5 structural breaks closed). * fix(governance): drop categories from governance_agents[] wire emission (#1906) AdCP 3.1.0-beta.2 narrowed the governance_agents[] wire shape from {url, categories?} to {url} only. Per-agent category signaling moved out of band; the wire schema no longer carries categories. Changes: - src/lib/server/decisioning/account.ts: projectGovernanceAgent emits {url} only; syncGovernanceRowToWire's inline projection mirrors. - src/lib/server/responses.ts: stripGovernanceAgentSecrets drops the categories preservation branch. - test/lib/sync-governance-credential-strip.test.js: now asserts categories is stripped (defense-in-depth with the existing authentication.credentials strip). All 82 governance tests pass. - src/lib/server/wire-spec-fields.generated.ts: regenerated against 3.1.0-beta.2 schemas (autogenerated). Adopter migration: the SDK no longer emits categories on governance_agents[]. Reading the field off the wire returns undefined. Switch to whatever out-of-band channel the seller now uses for per-agent category metadata. Part of the #1902 8.0-beta sweep (3/5 structural breaks closed). * feat(manifest-helpers): handle AssetVariant array slots (3.1.0-beta.2 widening) (#1907) AdCP 3.1.0-beta.2 widened each creative_manifest.assets[asset_id] slot from AssetVariant to AssetVariant | AssetVariant[] so carousel cards, responsive_creative headlines, etc. can carry multiple assets per slot. Changes: - getAsset / requireAsset: when the slot is an array, return the first element. Preserves pre-3.1 behavior for single-asset callers. - New getAssetSlot(manifest, assetId, assetType): returns the full array (or single-element array if scalar), filtered by asset_type. Use for carousel / responsive_creative platforms. - 7 new tests pinning array-unwrap behavior on getAsset, the new getAssetSlot helper, and asset_type filtering. Resolves 3 of 3 AssetVariant compile errors in src/lib/server/decisioning/manifest-helpers.ts. Part of the #1902 8.0-beta sweep (4/5 structural breaks closed). * feat(preview-utils): adopt self-rendering product_card (5/5 of #1902 sweep — CI green!) (#1908) * feat(preview-utils): adopt 3.1.0-beta.2 self-rendering product_card AdCP 3.1.0-beta.2 changed product_card from creative-agent-rendered ({format_id, manifest}) to a self-contained visual card ({image, title, description, price_label, cta_label}). The card IS the preview now; no creative-agent round-trip is required. batchPreviewProducts rewritten to extract product_card.image?.url directly. The creativeAgentClient + options parameters are retained for signature compatibility (renamed to _-prefixed unused args) and marked @deprecated; planned removal in 8.0 final or 9.0. format_card / batchPreviewFormats are unchanged — only product_card had this spec migration in 3.1.0-beta.2. Adopter migration: existing calls keep working (PreviewResult[] still populated with previewUrl from the new inline image.url). Direct product.product_card?.image?.url access is the new recommended path. Part of the #1902 8.0-beta sweep (5/5 structural breaks closed — CI should now be green on the foundation stack). * Merge bokelley/cut-8-0-beta: resolve wire-spec-fields timestamp conflict * fix(beta.3): envelope status + governance/rights field renames sweep Closes the remaining 45+ compile errors from the beta.2 → beta.3 jump. The spec broadened the envelope-required-status change AND renamed several decision-shape fields to free `status` for `TaskStatus` at the envelope level. Envelope status added to: - src/lib/adapters/content-standards-adapter.ts - src/lib/adapters/governance-adapter.ts - src/lib/adapters/property-list-adapter.ts - src/lib/adapters/si-session-manager.ts - src/lib/core/GovernanceMiddleware.ts - src/lib/server/create-adcp-server.ts - src/lib/server/decisioning/account.ts - src/lib/server/decisioning/list-helpers.ts - src/lib/server/decisioning/runtime/from-platform.ts - src/lib/server/governance.ts - src/lib/server/responses.ts - src/lib/server/test-controller-bridge.ts - src/lib/core/GovernanceTypes.ts Field renames absorbed: - Governance decision `status: 'approved' | 'denied'` → `verdict` - ReportPlanOutcomeResponse `status` → `outcome_state` - Rights response `status: 'acquired'` → `rights_status` - NotificationConfig[] intersection collision resolved at the two account.ts call sites Subagent-executed mechanical sweep. Build:lib clean (0 errors). * fix(beta.3): complete governance/outcome/rights rename + STALE_RESPONSE policy Closes the BLOCK items flagged by code-reviewer + ad-tech-protocol-expert on the post-rebase foundation: Governance verdict rename (`CheckGovernanceResponse.status` → `verdict`): - src/lib/testing/scenarios/governance.ts: 19 sites across 5 scenarios (initial check, over-budget, geo, conditions+recheck, delivery+drift). Renamed local `const status = data.status` → `const verdict = data.verdict` and threaded through step.details log strings ("verdict=approved"). Plan-sync status sites (867/872/883) left untouched — different field. - src/lib/testing/storyboard/context.ts:261: extractor `d?.status` → `d?.verdict`. - src/lib/testing/stubs/governance-agent-stub.ts:288: stub emits `verdict: 'approved'`. Outcome rename (`ReportPlanOutcomeResponse.status` → `outcome_state`): - src/lib/testing/storyboard/context.ts:269: extractor → `d?.outcome_state`. - src/lib/testing/stubs/governance-agent-stub.ts:315: stub emits `outcome_state: 'accepted'`. Rights rename (`AcquireRights*.status: 'acquired'` → `rights_status`): - src/lib/server/responses.ts:616 JSDoc autocomplete hint. - src/lib/server/decisioning/specialisms/brand-rights.ts:85/91/99 JSDoc. STALE_RESPONSE retry-policy correction: - src/lib/utils/buyer-retry-policy.ts: was {action: 'mutate-and-retry', reason: 'state'} (wrong — implies the payload should be re-fetched). Now {action: 'escalate', escalateReason: 'terminal'}. Spec semantics: STALE_RESPONSE is a non-fatal advisory paired with a populated success payload — consume the payload, surface the advisory, do not retry. - RetryDecision.escalate.reason JSDoc updated to enumerate STALE_RESPONSE alongside CREDENTIAL_IN_ARGS in the 'terminal' branch. Symmetric pattern with the witness-not-translator memory: stub and consumer must speak the same wire vocabulary or scenarios silently pass by reading undefined on both sides. Build:lib clean (0 errors). * feat(compat): 3.0.x receiver-side leniency for envelope `status` Addresses ad-tech-protocol-expert's must-fix on PR #1902. AdCP 3.1.0-beta.2 made envelope `status` REQUIRED. A 3.1-pinned SDK acting as a receiver against a 3.0.12 seller that omits envelope `status` would fail strict validation on every response — COMPATIBLE_ADCP_VERSIONS enumeration alone doesn't relax the validators. New helper: src/lib/utils/envelope-status-compat.ts - `injectLegacyEnvelopeStatus(response)` synthesizes envelope `status` ONLY for responses that declare themselves 3.0.x (adcp_version starting with "3.0", or adcp_major_version: 3 with no adcp_version, or no version fields at all). - `completed` when no top-level errors[] present, `failed` otherwise. - Never overwrites an existing truthy `status`. - 3.1+ responses that omit `status` still fail strict validation — the leniency is back-compat affordance, not permanent loosening. Wired into 5 validation paths: - response-unwrapper.ts:165 — unwrapProtocolResponse main path - response-unwrapper.ts:566 — isAdcpSuccess symmetric path - validation/schema-validator.ts:693 — Ajv validateResponse (applied BEFORE selectResponseVariant so synthesized `completed` doesn't misroute to the async-variant schema) - core/ResponseValidator.ts:327 — runtime ResponseValidator - testing/client.ts:672 + testing/storyboard/validations.ts:476 — storyboard/test-kit direct Zod paths Tests: test/lib/envelope-status-compat.test.js — 17 cases covering all required pins (3.0 → completed, 3.0+errors → failed, 3.1 unchanged, 3.1 with status unchanged, 3.0 with explicit status unchanged, no version → legacy) plus adcp_major_version: 3 legacy, foreign majors (4.0, 2.5), mutation-safety, and null/non-object guard. All pass. Subagent-executed sweep. Build:lib clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #1902. Final structural-break follow-up.
AdCP 3.1.0-beta.2 changed `product_card` from a creative-agent-rendered shape (`{ format_id, manifest }`) to a self-contained visual card (`{ image, title, description, price_label, cta_label }`). The card IS the preview — no creative-agent round-trip required. (Schema note: "Receivers render the card directly from these fields.")
Changes
Adopter migration
Verification
Stack status — CI ready
This closes the 5/5 structural breaks listed in #1902:
Once all five are merged into `bokelley/cut-8-0-beta`, #1902 can be marked ready and the whole stack admin-merged.
Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com