feat: FeedMirror — client-side wholesale feed mirror for products/signals#940
Merged
Merged
Conversation
Add a client-side helper that maintains an in-memory mirror of an agent's wholesale product and signal feeds per the AdCP 3.1 wholesale-feed sync pattern (closes #900): - bootstrap: full load via get_products buying_mode=wholesale and get_signals discovery_mode=wholesale, walking pagination cursors and recording wholesale_feed_version / pricing_version / cache_scope per feed - refresh: conditional re-read presenting if_wholesale_feed_version (and if_pricing_version); unchanged: true short-circuits without mutating the replica (build-then-swap, never wipes on unchanged) - apply_webhook: applies denormalized product.* / signal.* events in place; wholesale_feed.bulk_change re-bootstraps only the feed named by affected_entity_type - optional FeedStateStore persistence hook and on_event callback Mirrors the JS adcp-client wholesale-feed-sync shape for the core bootstrap/refresh/webhook/bulk-change semantics; the JS background-poll and EventEmitter surface is deliberately deferred (out of #900's stated scope). Exported from the top-level adcp package, matching the established pattern for the RegistrySync sync helper. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Clean port of the buyer side of the AdCP 3.1 wholesale-feed sync pattern. The build-then-swap discipline is the right shape — the replica is the load-bearing invariant, and an unchanged probe must never wipe it.
Things I checked
- Atomic swap holds.
_commit(src/adcp/feed_mirror.py:442-453) only replaces the live indexif not meta.unchanged. Anunchanged: trueshort-circuit updates only the cached version tokens and leavesself._products/self._signalsin place. Confirmed bytest_conditional_refresh_unchanged_is_noop. - Conditional-fetch token scoping is correct on the wire.
_build_products_request/_build_signals_request(feed_mirror.py:396-399,411-414) gateif_wholesale_feed_versiononfirst_pageonly, and nestif_pricing_versioninside the feed-version guard so pricing-version is never sent alone. Both match the normative descriptions ingenerated_poc/media_buy/get_products_request.py:236,242("pagination.cursor is NOT part of the scoping tuple", "MUST only be sent together with if_wholesale_feed_version").ad-tech-protocol-expert: sound on points 1–6. - Happy-path webhook apply does not spuriously raise. The
notification_type != event.event_typeintegrity check atfeed_mirror.py:492compares aNotificationTypeStrEnumagainst aLiteralvalue, bothstr-derived, so a valid envelope evaluates equal and never tripsFeedMirrorError.code-reviewerverified against_str_enum.py. *.pricedreplaces, not merges._apply_event(feed_mirror.py:535-537,547-549) doesmodel_copy(update={"pricing_options": ...})with the full post-change array — matches the schema's "NOT a delta — consumers replace" contract.- Pagination terminates.
_next_cursorreturns a cursor only whenhas_moreis true;has_more: truewithcursor: nullexits via theif cursor is Noneguard. - Public surface is additive. 7 new exports, all new symbols, registered in
tests/fixtures/public_api_snapshot.json. No removals, no signature changes, no required↔optional flips —feat:is the correct semver signal. No generated files touched; noADCP_VERSIONbump, as claimed.
Follow-ups (non-blocking — file as issues)
- State persistence is asymmetric.
apply_webhook's incremental path persists via_persist_state(feed_mirror.py:506), butrefresh,bootstrap, and thebulk_changere-bootstrap do not. After a refresh or bulk-change repair, the bumpedwholesale_feed_versionlives only in memory; a restart restores the stale persisted token. Self-healing (the stale token forces a full re-read on the next conditional fetch), so not corruption — but it silently defeats the restart optimization thestate_storeexists to provide. Persist on a successful swap, or document the asymmetry. (code-reviewer: Medium.) - Webhook envelope check is incomplete.
apply_webhookvalidatesnotification_type/notification_idagainst the embedded event, but not the third schema-mandated invariant:cache_scopeMUST equalevent.payload.applies_to.scope(schemas/cache/3.1.0-rc.9/core/wholesale-feed-webhook.jsonallOf). The generated Pydantic model doesn't encode that cross-fieldallOf, somodel_validatewon't catch a mismatched envelope either. Blast radius here is limited — aFeedMirroris single-account-scoped and doesn't multiplex scopes internally — but the cross-check is cheap and belongs next to the two it already does. (ad-tech-protocol-expert: caveat, not unsound.) - New public exports aren't in the human-facing docs. Check
README.md/AGENTS.mdforFeedMirrordrift, the same wayRegistrySyncis documented. - No infinite-loop guard if a non-conformant seller returns a constant non-null cursor with
has_more: true. Amax_pagescap would harden the walk.
Minor nits (non-blocking)
_remember_webhook_versionskipspricing_versionby design (feed_mirror.py:553-557) — the webhook envelope carries no pricing token, so the retained-stale value correctly forces a full payload on the next refresh. One line of comment would save the next reader the trip to the schema.- Silent no-op on
*.pricedfor an unmirrored id (feed_mirror.py:533,545). Defensible — apricedpayload carries onlypricing_options, nothing to insert — but the spec's reconciliation SHOULD means a missedcreatedstays missing until the next bulk_change. A debug-log would make the gap observable.
Stale-deferral-gate note in the PR body is appreciated — citing a JS issue that doesn't exist is a notable way to defer a feature for two quarters.
LGTM. Follow-ups noted below. Ship it once CI is green.
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.
Closes #900
What
Adds
FeedMirror, a client-side helper that maintains an in-memory mirror of an AdCP agent's wholesale product and signal feeds, implementing the AdCP 3.1 wholesale-feed sync pattern.get_products(buying_mode="wholesale")andget_signals(discovery_mode="wholesale"), walking pagination cursors and recordingwholesale_feed_version/pricing_version/cache_scopeper feed.if_wholesale_feed_version(andif_pricing_versionwhen known).unchanged: trueshort-circuits and leaves the replica untouched (build-then-swap; an unchanged probe never wipes the mirror).apply_webhook()applies denormalizedproduct.*/signal.*events to the index in place (created/updated replace, priced re-prices, removed deletes) and updates the cached feed version.wholesale_feed.bulk_changewebhook re-bootstraps only the feed named byaffected_entity_type.FeedStateStorepersistence hook (survive restart without a cold re-bootstrap) andon_eventcallback.Lives in
src/adcp/feed_mirror.py, exported from the top-leveladcppackage — matching the established pattern for theRegistrySyncsync helper. Built entirely on the generatedwholesale_feed/ product / signal types; no schema re-download, noADCP_VERSIONbump.Context on the deferral gate
#900 was deferred pending JS
adcp-client#2094"stabilizing the TS shape". That issue does not exist, so the gate is stale. The JS SDK does ship a reference (src/lib/wholesale-feed-sync/, classWholesaleFeedSync). This port mirrors its core bootstrap / refresh / webhook / bulk-change semantics. The JS background-poll loop, capability auto-resolution, and EventEmitter surface are deliberately not ported — they are beyond #900's stated "Suggested scope" and add a large speculative API; deferred for a follow-up if demand appears.Tests
tests/test_feed_mirror.py(14 tests, real types via.model_validate, in-memory seller stub — no HTTP mocking): bootstrap + pagination walk + wholesale-mode/account request assertions; incremental product & signal events; conditional-refresh no-op preserves the replica and re-presents cached version tokens; changed-feed refresh swaps the index; bulk_change re-bootstraps only the affected feed; state-store restore/persist; envelope-mismatch and failed-read error paths.Verification
make lint,make typecheck,make typecheck-all, and the feed-mirror / import-layering / public-API tests all pass. The public-API snapshot was regenerated to register the 7 new intentional exports.🤖 Generated with Claude Code