[bokelley]

Slice P13 of the reporting-subscriptions DX plan (.context/dx-reporting-subscriptions.md §2.9, §6.2). Stacked on #1768 (P10), which is stacked on #1766 (P11).

API

async for event in client.reporting.events(poll_every=timedelta(minutes=5)):   # acked per batch
    ...
batch = await client.reporting.poll(); await handle(batch.events); await batch.ack()   # serverless
await client.reporting.handle_webhook(VerifiedWebhook.from_outcome(outcome))    # doorbell

Contract

• Derived from durable state. poll() runs reconcile and reads the subscription readback and each account's subscription_coverage[]. It compares that snapshot with the one saved at the last ack() and emits one ReportingEvent per difference. A webhook body is never event content.
• At least once, with deterministic ids. batch.ack() saves the snapshot under compare-and-set (ReportingEventStore.save(..., expected_revision=)). A crash before ack redelivers the same events. event.id is rse_ plus sha256(agent, type, natural key, version). A second worker acking the same snapshot gets ReportingEventConflictError.
• Partial reads never fabricate changes. An account the run could not read keeps its previous state, and a failed roster keeps everything.
• poll(baseline=True) records the present without emitting it, for cold start.
• Webhooks. handle_webhook takes a VerifiedWebhook (built only through VerifiedWebhook.from_outcome from a verified, non-rejected WebhookOutcome), or such an outcome directly. Anything else is refused at runtime. With an events() loop running it rings that loop; otherwise it polls inline and returns the batch.

Event types:

• revision.available, revision.adjusted;
• period.late, period.missing_obligation, period.withheld;
• coverage.exception_opened, coverage.exception_closed;
• account.covered, account.unauthorized, account.regranted;
• subscription.state_changed, timezone.cutover.

Every event has id, type, agent_id, occurred_at, observed_at and data. Fields in data read as attributes, for example event.reporting_revision_id.

Not emitted yet: escalation. It needs the seller issue and operations_contact view, which a later slice can add to the snapshot.

Tests

tests/test_reporting_subscription_events.py runs against the SDK managed seller in process:

• The first poll reports the present. An un-acked repoll repeats the same ids. After ack nothing remains, and the stale batch's ack conflicts.
• period.missing_obligation, with occurred_at at period end plus grace, followed by revision.available once the seller delivers.
• account.unauthorized, then account.regranted.
• handle_webhook refuses unverified and rejected deliveries and polls inline.
• The events() loop delivers, acks, and wakes on the doorbell.

The reconcile, facade and public-API suites are green. ruff and mypy are clean. The buyer guide's Events section and the README are updated.

🤖 Generated with Claude Code

[aao-secretariat]

Ladon verdict: Approve

Approve — buyer reporting subscriptions events (poll/events/handle_webhook, AdCP 3.3).

Decision path: No critical/high findings (row 1 N/A). gated_paths false (row 2 N/A). high_risk false, no deletions/modifications triggers (rows 3–5 N/A). No prior decision (row 6 N/A). No no-auto-approve team match (row 7 N/A). Only 1 medium finding, fewer than 3 (row 8 N/A). Falls through to row 9 → approve.

What was checked: snapshot-diff derivation, deterministic rse_ ids, CAS ack (ReportingEventConflictError), and VerifiedWebhook fail-closed construction are the right shape. Semver signal correct — feat(reporting): for additive new exports, no breaking surface. Partial-read merge (_merge_partial) carries periods/exceptions/cutovers/covered/authorizations forward; account.regranted guarded by None check so per-account/roster failure does not fabricate authority events; principal read failure raises rather than emitting an empty report. Forward-compat unions and type-import layering untouched; no generated-code hand-edits; no credentials in ctx_metadata.

Medium findings (non-blocking):

• src/adcp/reporting/subscriptions/events.py:548 — events() clears the doorbell after poll+ack, swallowing mid-cycle webhook rings (latency only, falls back to poll_every; not data loss).

Note: poll() lacks an explicit timeout around reconcile I/O — flagged as a follow-up (belongs to the client/transport layer), not blocking.

Medium findings

• src/adcp/reporting/subscriptions/events.py:548 — events() clears the doorbell after poll+ack, swallowing mid-cycle webhook rings

[aao-secretariat]

Ladon verdict: Approve

Approve — subsequent review; prior blocking concerns cleared, 2 medium findings fall through to row 9.

What I checked

• reconcile.py new load(strict=) principal-read guard, events.py _merge_partial subscriptions/unauthorized carry-forward, and the two new tests. _merge_partial is sound: unioning previous+current for the everything-case keeps the snapshot continuous (no fabricated events on partial/failed cycle), and current wins on real reads so legitimate state_changed/unauthorized still emit.
• No semver/wire-shape, forward-compat, import-layering, generated-code, credential, or CI-gate concerns in this delta.

Medium findings (non-blocking)

• src/adcp/reporting/subscriptions/reconcile.py:463 — load() narrows the principal-read catch to ReportingSubscriptionError while sibling reads catch broad Exception; a transport/auth/budget/parse error could escape and crash reconcile. Note: the reviewer flags this is in the PR base, not base→head.
• src/adcp/reporting/subscriptions/events.py:548 — doorbell cleared after poll+ack can swallow mid-cycle webhook rings (carried from prior pass, posted inline).

Decision path

• No critical/high (row 1 n/a). gated_paths: false and review_decision: APPROVED (rows 2 n/a). high_risk: false (rows 3/5 n/a). No data-loss/schema/infra medium category (row 4 n/a). Prior decision was approve, not escalate (row 6 n/a). No no-auto-approve team (row 7 n/a). Only 2 medium findings (<3, row 8 n/a). → Row 9 approve.

A new medium on a second pass is normal reviewer coverage, not regression; the one carried-forward medium plus one new medium (in base code) stay below the comment threshold.

Medium findings

• src/adcp/reporting/subscriptions/reconcile.py:463 — load() narrows principal-read catch to ReportingSubscriptionError; other raised errors crash reconcile
• src/adcp/reporting/subscriptions/events.py:548 — events() clears the doorbell after poll+ack, swallowing mid-cycle webhook rings

[aao-secretariat]

Ladon verdict: Approve

Approve — no blocking or medium findings in this run.

This is a subsequent review (prior outcome: approve). The developer resolved prior Medium #2: events() now calls self._doorbell.clear() at the top of the loop (events.py:645) before poll+yield+ack, so a webhook ring landing mid-cycle survives and forces an immediate re-poll rather than blocking for the full poll_every interval. At-least-once delivery is preserved, and test_a_ring_during_a_cycle_is_not_swallowed pins the behavior. Prior Medium #1 (reconcile.py:463) is out of scope this pass (file absent from the delta).

The reviewer surfaced no current findings. Walking the decision table top-down: no critical/high (row 1 n/a); gated_paths is false and review_decision is APPROVED (rows 2 n/a); high_risk false (rows 3/5 n/a); no medium findings (rows 4/8 n/a); prior decision was approve, not escalate (row 6 n/a); no no-auto-approve team match (row 7 n/a). Falls through to row 9 — approve.

[aao-secretariat]

Ladon verdict: Approve

Approve — additive buyer-events feature (AdCP 3.3) with no blocking findings.

What I checked:

• Purely additive feature (poll/events/handle_webhook) under feat: — no public-surface removal, rename, or signature change; semver signal matches.
• Forward-compat / discriminated-union files and type-import layering untouched.
• reconcile degrades wire-read failures to report.issues (never raises), so the long-running events() loop survives transient reads.
• VerifiedWebhook fails closed; at-least-once ack semantics and the doorbell race are sound and tested.
• New feature is well-tested (tests/test_reporting_subscription_events.py).

Medium findings (1 — non-blocking):

• src/adcp/reporting/subscriptions/events.py:550 — events() loop can die permanently on a transient ReportingEventConflictError; the unguarded await batch.ack() should catch-and-re-poll rather than terminate the loop.

Decision: no critical/high findings (rows 1,4-6 N/A); gated_paths false and review_decision APPROVED (row 2 N/A); high_risk false (rows 3,5 N/A); no no-auto-approve team match (row 7 N/A); only 1 medium finding (row 8 requires ≥3). Falls through to row 9 — approve. Consistent with the prior clean approve.

Medium findings

• src/adcp/reporting/subscriptions/events.py:550 — events() loop dies permanently on transient ReportingEventConflictError

[aao-secretariat]

Ladon verdict: Approve

Approve — prior Medium resolved, no current findings.

This is a subsequent review. The prior run (approve) flagged one Medium: src/adcp/reporting/subscriptions/events.py:550 — the events() loop dying permanently on a transient ReportingEventConflictError. The delta addresses it directly: events() now catches ReportingEventConflictError on batch.ack() and continues (events.py:649-654), re-polling immediately so the winning worker's acked events are not redelivered; derive_events picks up the new revision on re-poll and the next ack settles, so there's no persistent spin. A new test (test_the_events_loop_survives_an_acknowledgement_conflict) exercises the branch with two clients sharing one store.

No new findings in this run. Decision table: no critical/high (row 1 n/a); gated_paths false and review_decision APPROVED (row 2 n/a); high_risk false (rows 3,5 n/a); no medium findings (rows 4,8 n/a); prior decision was approve, not escalate (row 6 n/a); no no-auto-approve team match (row 7 n/a). Falls through to row 9 → approve. Net improvement over the prior pass.

[aao-secretariat]

Ladon verdict: Approve

Approve — prior Medium resolved, no current findings.

This is a subsequent review. The prior run (approve) flagged one medium:

• events.py:550 — events() loop dies permanently on transient ReportingEventConflictError

That finding is now resolved: events.py:649-654 catches the transient ReportingEventConflictError around batch.ack() and re-polls via continue, so a lost compare-and-set race no longer kills the loop. The winner's snapshot loads as the next poll's baseline, deterministic event ids dedup overlap, and poll()'s reconcile I/O paces the re-poll (no busy-spin). Two new tests cover conflict survival and account_resolved source-zone exclusion from the explain digest; explain.py source unchanged.

Decision table: no critical/high findings (row 1 n/a), gated_paths false (row 2 n/a), high_risk false (rows 3/5 n/a), no medium findings (rows 4/8 n/a), no no-auto-approve team match (row 7 n/a), prior decision was approve so row 6 n/a. Falls through to row 9 — approve.

[aao-secretariat]

Ladon verdict: Approve

Approve — buyer reporting subscription events (poll/events/handle_webhook).

Reviewed against the decision table top-down. No critical/high findings (row 1 clear). gated_paths is false (row 2 n/a; review_decision is also APPROVED). high_risk is false (rows 3, 5 n/a). No no-auto-approve team match (row 7 n/a). Only one medium finding, below the three-medium threshold (row 8 n/a). Prior decision was approve, so sticky escalation (row 6) does not apply. Falls through to row 9.

What was checked: the deterministic-id / derive-from-durable-state design is sound — ack is compare-and-set (events.py:258-263), partial reads merge the prior view via _merge_partial (L693-723) so an unread account never fabricates a change, and VerifiedWebhook fail-closes on unverified/rejected deliveries (L285-287). Poll-path timeouts live at the SDK client layer, consistent with reconcile/explain. Semver signal correct: feat(reporting): for additive public exports.

Medium findings

• src/adcp/reporting/subscriptions/events.py:370 — account.covered id hashes a constant version, so a leave-then-rejoin re-coverage collides with the original event.id and is deduped away. Worth a follow-up but non-blocking.

Medium findings

• src/adcp/reporting/subscriptions/events.py:370 — account.covered id uses constant version, dedup drops re-coverage

[aao-secretariat]

Ladon verdict: Approve

Approve — subsequent pass, prior Medium resolved and no new blocking or Medium findings.

What I checked:

• The prior Medium (events.py:370 — account.covered id used a constant version, so a rejoin deduplicated against the first join) is resolved. The delta threads a per-(subscription, account) generation counter through the snapshot and increments it on each new join (events.py:465-478), giving a rejoin a distinct event id. The thread holds across the real poll/ack path and replay, and the dict is roster-bounded (not per-rejoin), so no unbounded growth.
• Regression test pins first[0].id != again[0].id, covering the branch the PR added.
• No new findings in the delta.

Gates: gated_paths: false; review_decision: APPROVED; no no-auto-approve team match; high_risk: false.

Decision table: No critical/high findings (row 1 n/a). Gated paths false (row 2 n/a). No deletions (row 3 n/a). No medium findings (rows 4, 5, 8 n/a). Prior decision was approve, not escalate (row 6 n/a). No team gate (row 7 n/a). Falls through to row 9 → approve. Net improvement over the prior run: the previously-flagged Medium is addressed with a test.

[bokelley]

TS parity note from adcp-client#3581 (T14, the TS port of this slice):

• Ids are equal across SDKs. TS builds the natural keys in json.dumps form (", " separators, non-ASCII escaped) and hashes [agent, type, key, version] with canonical JSON. A TS test pins rse_2434b1bd… (subscription.state_changed) and rse_a0c00d32… (account.covered), both computed by _event_id here.
• One difference to consider: account.regranted.chain. While an account is unauthorized it is missing from list_accounts, so the snapshot acked during the loss has no authorizations entry for it. The re-grant then compares against nothing and always reads chain: "new", even for a re-grant within status_retention_days that keeps authorized_at. TS copies the previous snapshot's authorization forward for accounts in unauthorized that aren't listed now (rememberAuthorizations), and its test asserts continued after revoke → regrant inside retention. Python's test_losing_and_regaining_authority_are_events only checks that the type is present, so it wouldn't catch this. That holds unless BuyerReportingState keeps listed stale, which I didn't verify.

🤖 Generated with Claude Code