fix(source-stripe): break same-second event cursor ties in favor of update events - #84355
fix(source-stripe): break same-second event cursor ties in favor of update events#84355devin-ai-integration[bot] wants to merge 7 commits into
Conversation
Co-Authored-By: bot_apk <apk@cognition.ai>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksPR Slash CommandsAirbyte Maintainers (that's you!) can execute the following slash commands on your PR:
📚 Show Repo GuidanceHelpful Resources
|
Co-Authored-By: bot_apk <apk@cognition.ai>
|
Deploy preview for airbyte-docs ready!
Deployed with vercel-action |
|
|
/ai-prove-fix
|
🟢 Fix ProvenThe 1-second cursor adjustment for Connector: Evidence summaryAttempt 1 — sandbox credentials, cold read (run): SPEC/CHECK/DISCOVER/READ all succeeded on both versions, 1,787 records on each side, no stream lower on target. The Attempt 2 — warm/stateful read against an eligible existing connection (run, comparison mode, all streams, real incoming state): SPEC/CHECK/DISCOVER/READ succeeded on both versions (both processes exited 0). Incoming state confirmed the event-based path executed. Records: 23,668 control vs 23,713 target (+45); no stream lower on target, the extras being streams whose upstream data changed between the two sequential reads. The designed behavior shows up directly in the cursor sequences for the three affected streams: In each stream exactly one cursor is 1 second lower on the target and every other cursor is byte-identical: the creation-event record moves earlier, so a same-second update payload can no longer tie with it and is the one that wins destination dedup. State safety: final cursors are identical on both versions for all three affected streams, and across every stream the target never ended more than 1 second behind control (a single stream by exactly 1s). Worst case on upgrade is a 1-second re-read overlap; nothing is skipped. LimitationThe regression harness flattens records, so the Stripe event envelope (and its Pre-flight checklist
Assessment: non-breaking and reversible. The one behavioral caveat — cursor values for Next stepsReady to merge from a validation standpoint; the existing progressive rollout config (autopilot, fast) can carry 6.0.14 out. No live version pin was applied and no customer sync was triggered — the regression runs only read connection config/catalog/state. Evidence plan used (posted before running the tests)Proving evidence: (1) SPEC/CHECK/DISCOVER identical between target and control; (2) READ succeeds on both with no stream losing records on target; (3) on event-derived incremental reads, Disproving evidence: (1) the cursor tie persists on target; (2) target fails on a read path that succeeds on control; (3) target emits fewer records, or unexpected schema/catalog/state differences appear. Anticipated limitation (confirmed in attempt 1): the events-based read path is only exercised on a warm, stateful incremental read; a cold read hits entity endpoints and leaves the changed expressions unevaluated, which is why the run was escalated to a stateful comparison against an existing connection. |
Airbyte Support Bot (airbyte-support-bot)
added
the
hyd-prove
|
|
↪️ Triggering Reason: |
Reviewing PR for connector safety and quality.
|
🛡️ AI PR Review Report🟢 Review Action: APPROVEDAll 12 gates passed.
|
| Gate | Status | Enforced? | Details |
|---|---|---|---|
| PR Hygiene | PASS | Yes | Description 5066 raw chars with What/How/Test Coverage/User Impact filled in; changelog row added to docs/integrations/sources/stripe.md for 6.0.14; no unresolved human review comments (0 inline comments; lazebnyi APPROVED at this SHA). |
| Code Hygiene | PASS | WARNING | Source change (manifest.yaml) is accompanied by test changes in unit_tests/integration/test_external_account_cards.py; coverage evidence found. |
| Test Coverage | PASS | Yes | Behavioral change (title fix(...), links airbytehq/oncall#13191) ships two new mock-server tests with real assertions on cursor values and state non-regression. |
| Code Security | PASS | Yes | No changed file matches the auth/credential/secret path patterns; manifest.yaml hunks contain no authenticator/token/key keywords; the only metadata.yaml hunk is dockerImageTag: 6.0.13 → 6.0.14 (not dockerRepository, allowedHosts, or connectorBuildOptions). |
| Per-Record Performance | PASS | WARNING | The added per-record work is a single record.get('type','').endswith('.created') string check inside existing AddFields transformations — negligible, no new objects, allocations, or I/O per record. |
| Breaking Dependencies | PASS | WARNING | No dependency or build files touched (pyproject.toml, lockfile, gradle all unchanged); only the connector version tag moved. |
| Backwards Compatibility | PASS | Yes (enforced) | No spec, schema, stream, primary-key, or cursor-field change (no spec*.json, schemas/*.json, or code type mapping in the diff), and no data-scope reduction — records emitted are identical in set and shape. See the cursor-value analysis below for why this is not a re-keying breaking change. |
| Forwards Compatibility | PASS | Warning (elevates Risk Level) | Cursor semantics change but state format is unchanged (plain int epoch seconds), so rollback to 6.0.13 reads and writes the same state shape; test_given_created_event_at_state_boundary_when_read_then_emit_record_without_regressing_state proves a decremented cursor at the state boundary does not regress emitted state. No partition-router or partition_field change. |
| Behavioral Changes | PASS | Warning (elevates Risk Level) | No operational keywords in functional hunks: no rate-limit, retry/backoff, timeout, or error_handler changes. The only limit match is with_limit(100) inside a new test fixture, which is not an operational change. |
| Out-of-Scope Changes | PASS | Skip | All 5 changed files are in scope: airbyte-integrations/connectors/source-stripe/** (3) and docs/integrations/sources/stripe.md (1), plus the connector's AGENTS.md. |
| CI Checks | PASS | Yes | Core checks green: Test source-stripe Connector ✅, Lint source-stripe Connector ✅, Connector CI Checks Summary ✅, Format Check ✅, Build and Verify Artifacts (source-stripe) ✅, Check Changelog Updated ✅; connector test results 209 passed / 2 skipped / 0 failed. No core check failed, cancelled, or pending. |
| Live / E2E Tests | PASS | Yes | Validation is required (bug fix + sync-behavior change: incremental_sync cursor derivation). /ai-prove-fix posted 🟢 Fix Proven on this branch — the 1-second adjustment was observed in a live warm incremental read against pre-release airbyte/source-stripe:6.0.14-preview.4e0517d with no record loss and no state regression. |
Cursor-value analysis (Backwards Compatibility)
The change alters the value written to the cursor field updated (and invoice_updated / subscription_updated) for records derived from *.created events, subtracting one second. The playbook treats a PK/cursor value change as breaking when it re-keys records so existing destination rows no longer match. That does not apply here:
- Primary keys are unchanged (
idand friends are untouched) — existing destination rows still match on the same key, so dedup/upsert targets the same rows. - The cursor field name, type, and state format are unchanged; the cursor only participates as the dedup ordering tiebreaker, which is exactly the bug being fixed.
- No user action is required after upgrade (no re-sync, reconfiguration, or schema refresh), and no records are dropped from scope.
Worst case is that a *.created event landing exactly at the state high-watermark second is re-emitted once — a duplicate at append destinations, not data loss — and the new boundary test covers that path. So: PASS, with the user-visible ordering change reflected in Risk Level 4 rather than a breaking-change requirement.
Files that triggered gate evaluation
airbyte-integrations/connectors/source-stripe/manifest.yaml— three cursor expressions (definitions.events_based_stream→data.object.updated,invoice_line_items→invoice_updated,subscription_items→subscription_updated). Drove Backwards/Forwards Compatibility and Live / E2E.airbyte-integrations/connectors/source-stripe/unit_tests/integration/test_external_account_cards.py— two newdef test_...functions with new assertions. Satisfied Test Coverage and Code Hygiene.airbyte-integrations/connectors/source-stripe/metadata.yaml—dockerImageTag6.0.13 → 6.0.14 only; noreleases.breakingChangesentry, consistent with the non-breaking classification.docs/integrations/sources/stripe.md— 6.0.14 changelog row. Satisfied PR Hygiene changelog check.airbyte-integrations/connectors/source-stripe/AGENTS.md— documents the new same-second tie-break behavior for future maintainers.
📚 Evidence Consulted
Evidence
- Changed files: 5 files (+71 / −4)
- CI checks: 47 check-runs on
e5b5253; core build/lint/test allsuccess;source-stripe Pre-Release Checks,source-stripe Progressive Rollout Gate,Connector Active Progressive Rollout Checks Summary,Validate PR Titleallsuccess(and excluded from the CI Checks gate by policy);CodeQLneutral, JVM/Bulk-CDK jobs skipped (no JVM connector modified) - PR labels:
connectors/source/stripe,hyd-fix,hyd-prove,hyd-review(applied by this run; also applied to the originating issue airbytehq/oncall#13191). No validation-override labels present or needed. - PR description: present (5066 chars)
/ai-prove-fix: 🟢 Fix Proven — fix(source-stripe): break same-second event cursor ties in favor of update events #84355 (comment) (detailed customer-data results in airbytehq/oncall#13191)- Existing bot reviews: none for this SHA (human APPROVE from
lazebnyiat this SHA)
|
↪️ Triggering Reason: the |
Auto-merge evaluation: FAILPreconditions (all must pass)
Change scope (at least one must pass)❌ No matching change scope detected. |
|
↪️ Triggering Reason: the |
Reviewing PR for connector safety and quality.
|
🛡️ AI PR Review Report🟢 Review Action: APPROVEDAll 12 gates passed.
|
| Gate | Status | Enforced? | Details |
|---|---|---|---|
| PR Hygiene | PASS | Yes | Description ~5.1k raw chars with What/How/Test Coverage/User Impact filled in; changelog row 6.0.14 added in docs/integrations/sources/stripe.md; no unresolved human review comments (one human APPROVED review by lazebnyi at the current head, zero inline comments). |
| Code Hygiene | PASS | WARNING | Source change (manifest.yaml) accompanied by test change (unit_tests/integration/test_external_account_cards.py, +64 lines). Coverage evidence: Found. |
| Test Coverage | PASS | Yes | Behavioral change (title fix(...), links airbytehq/oncall#13191). Two new def test_... functions with new assertions on cursor values and state non-regression. |
| Code Security | PASS | Yes | No changed path matches auth/oauth/token/credential/secret/encrypt patterns. manifest.yaml diff hunks contain no authenticator/api_token/client_secret-class keywords; metadata.yaml diff is only dockerImageTag: 6.0.13 → 6.0.14 (no allowedHosts, connectorBuildOptions, or dockerRepository line changed). |
| Per-Record Performance | PASS | WARNING | The change adds one str.endswith test and an integer subtraction inside an existing AddFields Jinja expression that already ran per record. No new object creation, network call, or loop. |
| Breaking Dependencies | PASS | WARNING | No pyproject.toml, poetry.lock, build.gradle, or base-image change in the diff. |
| Backwards Compatibility | PASS | Yes (enforced) | No spec*.json/spec*.yaml, schemas/*.json, stream, primary-key, or cursor-field change; no format/type change on any existing field. The cursor value on *.created-derived records shifts 1s earlier, which was evaluated against the PK/cursor re-keying rule: the dedup key (id) is unchanged, so existing destination rows still match, and the cursor is used only for ordering/tie-breaking. metadata.yaml is a patch bump (6.0.13 → 6.0.14) with no breaking-change semantics requiring a releases.breakingChanges entry. |
| Forwards Compatibility | PASS | Warning (elevates Risk Level) | Diff touches a cursor-deriving AddFields transformation, so the gate's keywords match. State format and partition-router shape are unchanged, so state written by either version is readable by the other; test_given_created_event_at_state_boundary_when_read_then_emit_record_without_regressing_state proves a .created event at the state high-watermark is still emitted and the emitted state does not regress. Rollback re-emits the same records with a 1s-higher cursor — no state incompatibility. |
| Behavioral Changes | PASS | Yes → Warning (elevates Risk Level) | No rate-limit, retry, backoff, timeout, error-handler, or resource-config change in the diff hunks. The only match against the keyword list is with_limit(100) in the new mock-server test request builder (test-only mock setup, not connector configuration). |
| Out-of-Scope Changes | PASS | Skip | All 5 changed files are in scope: 4 under airbyte-integrations/connectors/source-stripe/, 1 under docs/integrations/sources/. No platform, CDK-core, CI-workflow, or infrastructure files. |
| CI Checks | PASS | Yes | 39 passed / 0 failed / 0 pending at 1edcf522, including Test source-stripe Connector, Lint source-stripe Connector, Build and Verify Artifacts (source-stripe), Connector CI Checks Summary, Check Changelog Updated, Format Check. source-stripe connector test results: 211 tests, 209 ✅, 0 ❌. |
| Live / E2E Tests | PASS | Yes | Validation is required (bug fix + sync-behavior change: incremental_sync/cursor/transformations). /ai-prove-fix posted 🟢 Fix Proven (comment 5283482563) on pre-release airbyte/source-stripe:6.0.14-preview.4e0517d, from a warm/stateful comparison read against an eligible existing connection with real incoming state: the 1-second adjustment is observable, with no record loss and no state regression. The connector directory is byte-identical between 4e0517d and the current head, so that conclusion still describes this branch. |
Files changed (5): source-stripe/manifest.yaml (3 cursor expressions), source-stripe/metadata.yaml (version bump), source-stripe/AGENTS.md (behavior note), source-stripe/unit_tests/integration/test_external_account_cards.py (2 new tests), docs/integrations/sources/stripe.md (changelog row).
📚 Evidence Consulted
Evidence
- Changed files: 5 files (
git diff --merge-base origin/master) - CI checks: 39 passed, 0 failed, 0 pending, 10 skipped at
1edcf522 - PR labels:
connectors/source/stripe,hyd-review,hyd-fix,hyd-prove,hyd-ready— no validation-override labels relied on - PR description: present (~5.1k chars)
- Existing bot reviews: prior
APPROVEmarker ate5b52530(comment5292951049); humanAPPROVEDreview bylazebnyiat the current head /ai-prove-fixconclusion: 🟢 Fix Proven (comment5283482563), detailed results inairbytehq/oncall#13191
|
↪️ Triggering Reason: the re-run |
Auto-merge evaluation: FAILPreconditions (all must pass)
Change scope (at least one must pass)❌ No matching change scope detected. |
|
💬 Readiness outcome for the record, per Hands-Free AI Triage Project: with the review refreshed at the current head, all four |
|
↪️ Triggering |
Reviewing PR for connector safety and quality.
|
🛡️ AI PR Review Report🟢 Review Action: APPROVEDAll 12 gates passed.
|
| Gate | Status | Enforced? | Details |
|---|---|---|---|
| PR Hygiene | PASS | Yes | Description is 5712 raw chars with What/How/Test Coverage/User Impact filled in; changelog row for 6.0.14 added to docs/integrations/sources/stripe.md; no unresolved human review comments (0 inline comments; lazebnyi APPROVED at the current head). |
| Code Hygiene | PASS | WARNING | Source (manifest.yaml) changed and tests changed in the same PR (unit_tests/integration/test_external_account_cards.py, +64 lines) — coverage evidence Found. |
| Test Coverage | PASS | Yes | Behavioral change (title fix(...), linked oncall issue). Two new mock-server tests with real assertions: test_given_created_and_updated_events_in_same_second_when_read_then_updated_event_wins_cursor_tie and test_given_created_event_at_state_boundary_when_read_then_emit_record_without_regressing_state. |
| Code Security | PASS | Yes | No changed file matches auth/credential/secret path patterns. manifest.yaml diff hunks contain no security keywords (no authenticator, *_token, client_id/secret, api_key); the metadata.yaml hunk is only dockerImageTag: 6.0.13 → 6.0.14 (not dockerRepository/allowedHosts/connectorBuildOptions). |
| Per-Record Performance | PASS | WARNING | The three changed expressions add one constant-time str.endswith per record inside existing AddFields transformations — no new allocation, no per-record I/O, no added complexity class. |
| Breaking Dependencies | PASS | WARNING | No dependency files changed (pyproject.toml, poetry.lock, gradle files untouched); base image unchanged in this PR. |
| Backwards Compatibility | PASS | Yes (enforced) | No spec file changed (no spec*.json/spec*.yaml, no spec section in manifest.yaml), so required is unchanged; no stream added/removed/renamed, no schema property or type/format change, no primary-key change (PK remains id), no state-format change, and no stream loses records. The cursor field (updated) is unchanged; the derived cursor value for *.created-sourced records is 1s lower. This is not the re-keying case the gate guards: destination dedup keys on the unchanged PK, so no existing row is re-keyed, and a 1s-lower cursor can at worst re-read one extra second (dedup'd), never skip records — /ai-prove-fix confirmed no record loss and no state regression on a warm live read. metadata.yaml is a patch bump with no breaking-change semantics requiring a releases.breakingChanges entry. |
| Forwards Compatibility | PASS | Warning (elevates Risk Level) | Cursor-value expressions changed, so the gate's keywords are present — but state format, cursor field, and partition routing are all untouched (no partition_router/parent_stream_configs/state_migrations change), so rollback simply restores the prior +1s value with the same state shape. test_given_created_event_at_state_boundary_when_read_then_emit_record_without_regressing_state proves a .created event at the state high-watermark is still emitted and emitted state does not regress. |
| Behavioral Changes | PASS | Yes* (warning) | Keyword scan over all +/- lines in the diff found no operational keywords (rate_limit, retry, backoff, Retry-After, timeout, sleep, page_size, error_handler, response_filters, http_codes, memory_*). The dedup-ordering effect of the change is documented in the PR description, the changelog row, and the connector's AGENTS.md. |
| Out-of-Scope Changes | PASS | Skip | All 5 changed files are in scope: 4 under airbyte-integrations/connectors/source-stripe/ and 1 under docs/integrations/sources/. |
| CI Checks | PASS | Yes | All core check-runs on 3a7b8322 are green, including Test source-stripe Connector, Lint source-stripe Connector, Build and Verify Artifacts (source-stripe), Connector CI Checks Summary, Format Check, Check Changelog Updated, and the docs checks. 0 failing, 0 pending. |
| Live / E2E Tests | PASS | Yes | Validation is required (bug fix + sync-behavior change). /ai-prove-fix posted 🟢 Fix Proven (comment 5283482563) at 4e0517d — pre-release airbyte/source-stripe:6.0.14-preview.4e0517d, warm/stateful comparison read against an eligible existing connection vs. control 6.0.11, with the 1s cursor adjustment observable, no record loss and no state regression. The connector directory is byte-identical between 4e0517d and the current head, so that conclusion still describes this HEAD. |
Changed files driving the evaluation
airbyte-integrations/connectors/source-stripe/manifest.yaml— three cursor expressions (definitions.events_based_stream→data.object.updated;invoice_line_items→invoice_updated;subscription_items→subscription_updated) each gain- (1 if <event>.get('type','').endswith('.created') else 0).airbyte-integrations/connectors/source-stripe/unit_tests/integration/test_external_account_cards.py— two newHttpMockertests.airbyte-integrations/connectors/source-stripe/metadata.yaml—6.0.13 → 6.0.14.airbyte-integrations/connectors/source-stripe/AGENTS.md— one documentation line recording the tie-break rule.docs/integrations/sources/stripe.md—6.0.14changelog row linking this PR.
📚 Evidence Consulted
Evidence
- Changed files: 5 files (
git diff --merge-base origin/master), +71 / -4. - CI checks: 49 check-runs on
3a7b8322— 38 success, 10 skipped, 1 neutral (CodeQL), 0 failed, 0 pending. Core connector checks (Test source-stripe Connector,Lint source-stripe Connector,Build and Verify Artifacts (source-stripe)) all success;source-stripe Pre-Release Checksandsource-stripe Progressive Rollout Gateexcluded from this gate by design and also green. - PR labels:
connectors/source/stripe,hyd-review,hyd-fix,hyd-prove,hyd-ready. No validation-override labels relied on. - PR description: present (5712 chars).
- Reviews:
lazebnyiAPPROVED at3a7b8322. Prior bot markers:APPROVEate5b52530and at1edcf522; no REQUEST CHANGES review has ever been submitted on this PR. /ai-prove-fix: 🟢 Fix Proven (comment 5283482563), detailed customer-data results at airbytehq/oncall#13191.- Hydra stage label
hyd-reviewapplied to this PR and to the originating issue airbytehq/oncall#13191.
|
↪️ Triggering |
Auto-merge evaluation: FAILPreconditions (all must pass)
Change scope (at least one must pass)❌ No matching change scope detected. |
4 participants
Requested by Maddie Kashyap (Maddie Kashyap (@maddiekashyapAB)) via
/ai-fixon the oncall issue.Resolves https://github.com/airbytehq/oncall/issues/13191:
What
On incremental syncs,
source-stripereconstructs entity records from/v1/eventsand derives the record cursor (updated) from the event'screatedtimestamp, which has 1-second granularity (cursor_granularity: PT1S). When a*.createdand a*.updatedevent for the same object fall in the same second, both records carry the same cursor value, so the destination's dedup falls through to its secondary key:ORDER BY <cursor> DESC, _airbyte_extracted_at DESC. Stripe pages/v1/eventsnewest-first, so the older.createdpayload is emitted last, gets the larger_airbyte_extracted_at, and wins — the destination keeps the creation-time payload and any field first set by the update event (in the reported case,metadata) reads as empty.A user reported this on
subscriptions, with both event payloads for the same subscription in the same second as evidence: thecustomer.subscription.updatedevent carried the fullmetadata, thecustomer.subscription.createdevent carried{}, and the empty one landed in the warehouse. The mechanism applies to every events-based stream in this connector, not justsubscriptions.How
Creation events now derive a cursor one second earlier than their event timestamp, so an update event in the same second sorts strictly higher and wins dedup deterministically — independent of emission order and
_airbyte_extracted_at.Applied at the three places that derive a cursor from an event payload:
The
entity_stream/entity_restricted_stream/full_refresh_streamtransformations are deliberately untouched: those read real entity objects (notypefield, no collision).Declarative-First Evaluation
Declarative only — a one-line Jinja change to the existing
AddFieldstransformations. No custom Python component was added or needed;components.pystill does not exist for this connector.Review guide
airbyte-integrations/connectors/source-stripe/manifest.yaml— the three cursor expressions.unit_tests/integration/test_external_account_cards.py— the two new mock-server tests.metadata.yaml/docs/integrations/sources/stripe.md— version bump and changelog.Test Coverage
Two new mock-server tests in
unit_tests/integration/test_external_account_cards.py(the existing mock harness coversexternal_account_cards' events path; it has nosubscriptionsfixture, and the expanded-record streams would need disproportionately larger fixtures):test_given_created_and_updated_events_in_same_second_when_read_then_updated_event_wins_cursor_tie— a.createdand an.updatedevent for the same object id in the same second; asserts the.created-derived record's cursor is exactly one lower. Fails onmaster(both records get the same cursor), passes with this change.test_given_created_event_at_state_boundary_when_read_then_emit_record_without_regressing_state— guards the one real risk of decrementing a cursor: a.createdevent exactly at the state high-watermark second is still emitted, and the emitted state does not regress below the incoming state.Full connector suite:
poetry run pytest -xinunit_tests/→206 passed.Not verified: no live sync against a real Stripe account, so this is verified at the record/cursor level rather than end-to-end into a warehouse.
User Impact
Non-breaking. No schema, primary key, cursor field, state format, or spec change, and no records are excluded. The only observable difference is that the synthetic
updatedvalue on records derived from*.createdevents is one second lower than before; going forward, a same-second create/update pair keeps the update payload at the destination.Rows already stale in a destination are not repaired by this change — they self-heal when Stripe next emits an event for that object, or immediately via a stream refresh (which reads the entity endpoint rather than
/v1/events).Two same-second collisions remain unaddressed, both out of scope here and not fixable by the same trick:
*.updatedevents for one object in the same second (the older still wins);*.deletedevent tying with an*.updatedevent (the update still wins, sois_deletedcan be lost).Can this PR be safely reverted and rolled back?
Devin session
Important
Autopilot Progressive Rollout Enabled
Autopilot progressive rollouts are enabled for one or more connector(s) modified in this PR. Check the box below if you need to bypass normal rollout safety processes and release to all users immediately upon merge:
Note: