Repository navigation
fix(examples): stop zero-filling impressions and flat multi-standard viewability in the reference seller - #1709
Merged
Conversation
…viewability in the reference seller The reference seller's delivery totals reported impressions as 0 when no delivery measured them, set viewable_rate to 0 with nothing measurable, and combined viewability reported under different standards into one flat total. AdCP 3.3 forbids all three: an absent count means not measured, receivers never derive an impression rate from it, and a metric reported under more than one qualifier omits its flat carrier from buy totals. Totals now carry impressions only when a delivery measured them, omit viewable_rate without measurable impressions, and omit the flat viewability block when deliveries used more than one standard. Toward callers served below 3.3, format breakdown rows without impressions are omitted rather than sent with a zero. Refs #1669 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Ladon verdict: Approve
Approve.
Clean, well-scoped fix to the reference seller example plus tests (2 files, +77/-4). The reviewer traced all three AdCP 3.3 behaviors (impressions omitted when unmeasured, viewable_rate dropped when measurable == 0, flat viewability block gated on a single standard) and confirmed _serves_impressionless_rows parses version robustly (3.3-beta.1 keep, 3.2 drop, None/malformed fail-open). The DR-0032/0033 droppable pathway for pre-3.3 callers is tested.
No critical/high/medium findings. Not a gated path, high_risk is false, no author team gate, no prior decision. None of rows 1–8 fire, so this falls through to row 9 = approve.
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.
Part of #1669, alongside #1707: the reference seller (
examples/seller_agent.py) gaps that #1707's audit found.AdCP 3.3 rules (migration guide §Delivery rows may omit
impressions, §Multi-qualifier metrics omit flat totals):impressionsonly when a delivery measured them; package rows already followed the totals.viewable_rateis omitted when nothing was measurable, where it used to be0.viewabilityblock is omitted when deliveries used more than onestandard.impressions: 0.by_formatrows without impressions are dropped for those callers.Totals and
by_packagedon't requireimpressionsin 3.2 either, so they stay valid for 3.2 callers. Storyboard deliveries always inject impressions, so their responses are unchanged.Tests: three new cases in
tests/test_seller_agent_storyboard.py. That file andtest_seller_agent_dual_emit.py: 43 passed.Refs #1669
🤖 Generated with Claude Code