epic-2/story-4: Expose Tracking Lifecycle Events and Delivery Outcomes - #32
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a lifecycle event model and an in-process synchronous event bus to the Convert Python SDK, allowing consumers to subscribe to SDK events like conversions and queue releases. It integrates event emission into the tracking queue release path, updates the delivery failure contract to drop events rather than re-queue them, and adds extensive tests. Feedback on the changes suggests moving payload serialization and transport resolution inside the try-except block in the release path to prevent event loss on non-delivery faults, and tightening the EventHandler type signature to enable static type checking for registered handlers.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| payload = self._build_batch_payload(items) | ||
| transport = self._ensure_transport() | ||
| try: | ||
| transport.send_tracking(payload, sdk_key=str(self._config.sdk_key)) |
There was a problem hiding this comment.
The _build_batch_payload(items) and _ensure_transport() calls are executed outside of the try/except block. If a serialization bug or transport-provisioning error occurs (which are non-delivery faults), the exception will propagate immediately without executing the except Exception: block on line 292. As a result, the drained events will be permanently lost instead of being restored to the queue as intended by the design contract.
Moving these calls inside the try block ensures that any non-delivery faults during payload construction or transport resolution will correctly trigger the queue restoration before propagating the error.
| payload = self._build_batch_payload(items) | |
| transport = self._ensure_transport() | |
| try: | |
| transport.send_tracking(payload, sdk_key=str(self._config.sdk_key)) | |
| try: | |
| payload = self._build_batch_payload(items) | |
| transport = self._ensure_transport() | |
| transport.send_tracking(payload, sdk_key=str(self._config.sdk_key)) |
| #: A lifecycle-event handler. Invoked as ``handler(payload, error)`` — mirroring | ||
| #: the JS ``EventManager`` ``fn(args, err)`` contract — so a handler can observe | ||
| #: both the success payload and an optional delivery error. | ||
| EventHandler = Callable[..., None] |
There was a problem hiding this comment.
Using Callable[..., None] as the type alias for EventHandler disables static type checking for the handler's signature. Since the SDK strictly invokes handlers with two arguments (payload and error), registering a handler with a different signature (e.g., a single-argument lambda like lambda payload: ...) will pass static analysis but raise a TypeError at runtime, which is then silently swallowed and logged as an error by the event bus.
Changing the type definition to Callable[[Any, Optional[BaseException]], None] allows static type checkers (like mypy or pyright) and IDEs to enforce and surface the correct signature to integrators.
| EventHandler = Callable[..., None] | |
| EventHandler = Callable[[Any, Optional[BaseException]], None] |
Beads: ai-driven-product-dev-vqmn ai-driven-product-dev-d3l7 ai-driven-product-dev-37ep ai-driven-product-dev-w34z L0 events.py: LifecycleEvent enum (PRD-frozen identifiers incl API_QUEUE_RELEASED) + typed ConversionEventPayload / QueueReleasedPayload (privacy-safe, ReleaseReason reused). L1 ports/event_bus.py: EventBus Protocol (on/emit). L3 adapters/events/in_process.py: synchronous bus, per-handler try/except isolation, no-op when no subscribers. logging.py: privacy-safe queue-release / delivery-failure / handler-error log sites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Beads: ai-driven-product-dev-orob ai-driven-product-dev-9y97 ai-driven-product-dev-a6iv - Core.on(event,handler): one EventBus per Core, injected into Tracker; export LifecycleEvent. - Tracker emits CONVERSION only after a tracked (non-suppressed) enqueue; never on dedup/goal-not-found. - Single shared release path emits API_QUEUE_RELEASED once per actual release with reason+batch_size+counts; empty flush emits nothing. Per-trigger reason (explicit/size/timeout/atexit) via flush_timeout/flush_atexit. - F-010: on TrackingDeliveryError emit failure outcome (status_code+retry only), log privacy-safe, DROP events, do NOT raise (flush() stays non-raising). Updated stale Story 2.3 preserve-and-raise tests to F-010 contract. - New: tests/test_lifecycle_events.py; extended conversion/delivery/queue-lifecycle suites. 366 passed (was 340). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fb7d7f5 to
627d3ca
Compare
a4d2036 to
919e549
Compare
|
Superseded — all commits already in main (bc76b64). Closing without merge as part of post-sprint cleanup. |
Story 2.4 — Expose Tracking Lifecycle Events and Delivery Outcomes
Adds the observability surface on top of Story 2.3's queue/dedup/flush/transport layer: lifecycle-event emission and delivery-outcome reporting. Integrates with — does not re-implement — the existing tracking pipeline.
What was built
events.py(L0) —LifecycleEventenum (READY/CONFIG_UPDATED/BUCKETING/CONVERSION/API_QUEUE_RELEASED/DATA_STORE_QUEUE_RELEASED, JS-parity dot-separated values) + typed event payload structures. Runtime-clean L0 (referencesReleaseReasononly underTYPE_CHECKING).ports/event_bus.py(L1) —EventBustyping.Protocol(on+emit).adapters/events/in_process.py(L3) — in-process synchronous EventBus; per-listener try/except isolates, logs, and swallows handler errors (JSEventManager.fireparity); no-subscriber emit is a zero-cost no-op.Core.on(event, handler)(L4) — public subscription surface; oneEventBusperCore, injected into the tracker.LifecycleEventexported from__init__.py.CONVERSIONfrom the tracker only on a tracked (non-suppressed) enqueue;API_QUEUE_RELEASEDonce per release on the single shared release path with success/failure outcome + privacy-safe logging.Acceptance criteria & precedence rulings
All 5 ACs and 12 Critical Warnings satisfied. Sprint-driver precedence rulings honored (resolved against current PRD ground truth):
API_QUEUE_RELEASED = "api.queue.released",CONFIG_UPDATED = "config.updated"(per audit-corrected prd.md:398-404; staleQUEUE_RELEASED/config_updatedprose ignored).API_QUEUE_RELEASEDwith error, does NOT restart timer/re-queue;flush()stays non-raising (intentional divergence from JS).ReleaseReasonenum; no second release-reason representation.Tests
340 → 366 (+26; all passing offline via the qs-06 RESPX harness). Two stale Story 2.3 tests asserting preserve-and-raise on delivery failure were updated to the authoritative drop-and-don't-raise contract (F-010).
Traceability
Beads epic
ai-driven-product-dev-v7zj; tasks-vqmn,-d3l7,-37ep,-w34z,-orob,-9y97,-a6iv(all closed).Readiness gate: PASS 9.1/10, no auto-delegated questions. Code review: clean, round 1, no warnings.
Part of sprint
sprint/2026-04-06-convert-python-sdk. Stacked on #31 (story 2-3) → #30 → #29 → #24 chain.