fix(source-stripe): Fix invoice_line_items and subscription_items incremental streams#72845
fix(source-stripe): Fix invoice_line_items and subscription_items incremental streams#72845devin-ai-integration[bot] wants to merge 6 commits into
Conversation
…remental streams Uses RecordExpander component to extract nested items from parent objects: - invoice_line_items: extracts line items from invoice.lines.data - subscription_items: extracts subscription items from subscription.items.data Both streams now emit individual child items instead of parent objects, with parent metadata (id, created, updated) preserved on each child record. Co-Authored-By: sophie.cui@airbyte.io <sophie.cui@airbyte.io>
🤖 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
|
|
/ai-prove-fix
|
|
|
Fix Validation EvidenceOutcome: Could not Run Tests (Live connection testing blocked - non-reversible change requires human approval) Evidence SummaryPre-flight checks completed. The fix uses Next StepsBefore this PR can be validated on live connections:
To proceed with live testing after approval:
Regression Tests:
Connector & PR DetailsConnector: Evidence PlanProving Criteria
Disproving Criteria
Potential Test Cases (Internal Airbyte Org)Found 10 internal connections with
Note: Live testing blocked pending human approval due to non-reversible nature of change. Pre-flight Checks
Reversibility Concern: Per the playbook, non-reversible changes require:
Design Intent CheckWARNING: Before merging, please verify this is actually a bug and not intentional behavior. The current behavior emits parent objects (invoices/subscriptions) from the events endpoint. This may have been intentional to provide full context. The fix changes this to emit individual child items, which is a significant data structure change. Questions for reviewers:
Session: https://app.devin.ai/sessions/5c6a54c91f93497b819be1cd28d13f52 |
|
/ai-review
Reviewing PR for connector safety and quality.
|
AI PR Review ReportReview Action: REQUEST CHANGES
🔧 Remediation RequiredRequired ActionsGate 3: Safety/Reversibility - FAIL The PR is explicitly marked as NOT reversible in the description:
This is a breaking change that modifies the data structure of
Gate 5: Pre-Release Validation - UNKNOWN The
📋 PR Details & EligibilityConnector & PR InfoConnector(s): Auto-Approve EligibilityEligible: No Review Action DetailsREQUEST CHANGES - Gate 3 (Safety/Reversibility) has definitively FAILED because the PR explicitly acknowledges this is a breaking, non-reversible change. The workflow automation will submit a REQUEST_CHANGES review via octavia-bot.
🔍 Gate Evaluation DetailsGate-by-Gate Analysis
Gate 1: CI/Tests - PASSCore CI Checks:
Non-blocking failure: Coverage Evidence: Exempt - manifest.yaml changes are covered by connector unit tests which passed. Gate 2: Security - PASSNo security-sensitive patterns detected in diff:
Gate 3: Safety/Reversibility - FAILTrigger: PR description explicitly states:
Breaking Change Analysis:
Gate 4: Operational Risk - PASSNo operational risk patterns detected:
Gate 5: Pre-Release Validation - UNKNOWNEvidence Search:
Context: The
Pre-release version Gate 6: PR Quality - PASS
📚 Evidence ConsultedEvidence
Files AnalyzedChanges Summary:
❓ How to RespondProviding Context or JustificationYou can add explanations that the bot will see on the next review: Option 1: PR Description (recommended) ## AI PR Review Justification
### Gate 3 (Safety/Reversibility)
[Your explanation here - e.g., why this breaking change is necessary and how users will be supported]
### Gate 5 (Pre-Release Validation)
[Your explanation here - e.g., why validation is not required or how it will be obtained]Option 2: PR Comment After adding your response, re-run Note: For Gate 3 (Safety), justifications provide context but cannot override the FAIL status for breaking changes. The PR requires:
Once these requirements are met, re-run |
![octavia-bot[bot]](https://avatars.githubusercontent.com/in/217399?s=60&v=4){kind=small}
|
follow remediations for gate 3 |
…for v6.0.0 Co-Authored-By: sophie.cui@airbyte.io <sophie.cui@airbyte.io>
|
Deploy preview for airbyte-docs ready! ✅ Preview Built with commit ccf737a. |
|
can you find a live connection first and tell me which one before actually testing it? |
Live Connections Found for TestingI found the following internal Airbyte connections that have the affected streams enabled: Connections with
|
| Connection Name | Workspace | Connection URL |
|---|---|---|
stripe_config_dev_null |
integration-test-sandboxes | Link |
Stripe -> ab-analytics |
_airbyte-internal-team | Link |
Stripe → End-to-End Testing (/dev/null) |
aldo.gonzalez.testing.replicate_integration | Link |
Connections with subscription_items stream:
| Connection Name | Workspace | Connection URL |
|---|---|---|
stripe_config_dev_null |
integration-test-sandboxes | Link |
Stripe -> ab-analytics |
_airbyte-internal-team | Link |
Stripe (descript oc but running master) → e2etest |
alexandre-default | Link |
Recommended for Testing
The stripe_config_dev_null connection in the integration-test-sandboxes workspace (0f3c8a84-53e2-4a33-89a9-26bf36a2367c) has both streams enabled and outputs to /dev/null, making it ideal for safe testing.
Let me know which connection you'd like me to use for testing.
|
yes use stripe_config_dev_null for live testing. make sure to do this in tangent with this pr airbytehq/airbyte-python-cdk#859 |
Live Testing TriggeredI've triggered a regression test using the Workflow: https://github.com/airbytehq/airbyte-ops-mcp/actions/runs/21725597380 This will build the connector from this PR and run tests against the live connection. I'll report back with the results. Note: This test depends on the CDK PR airbytehq/airbyte-python-cdk#859 being available. If the RecordExpander component isn't available in the CDK version used, the test may fail with a component not found error. |
|
/ai-review
Reviewing PR for connector safety and quality.
|
|
Reviewing PR now. Evaluating all 6 gates against HEAD SHA Session: https://app.devin.ai/sessions/95c20eb82b1c480199b8e0888b971842 |
AI PR Review ReportReview Action: NO ACTION (INCONCLUSIVE)
All gates now pass except Gate 5 (Pre-Release Validation), which remains UNKNOWN due to incomplete live testing evidence. Prior REQUEST CHANGES review (for Gate 3) may be dismissed by a maintainer since breaking change documentation is now complete. 📋 PR Details & EligibilityConnector & PR InfoConnector(s): Auto-Approve EligibilityEligible: No Review Action DetailsNO ACTION (INCONCLUSIVE) - No gates have definitively FAILED, but Gate 5 (Pre-Release Validation) is UNKNOWN because live testing evidence is not yet available. No PR review submitted. Human review required. Re-run context: The previous
Gate 3 now PASSES. The prior REQUEST CHANGES review from octavia-bot may be dismissed by a maintainer.
🔍 Gate Evaluation DetailsGate-by-Gate Analysis
Gate 1: CI/Tests - PASSCore CI Checks:
Excluded from Gate 1 (pre-release checks): Coverage Evidence:
Gate 2: Security - PASSNo security-sensitive patterns detected in diff:
Gate 3: Safety/Reversibility - PASSPrevious status: FAIL (at SHA Current status: PASS - all breaking change requirements now met:
Breaking change analysis:
Gate 4: Operational Risk - PASSNo operational risk patterns detected in diff:
Gate 5: Pre-Release Validation - UNKNOWNEvidence search (Priority 1 - MCP):
Evidence search (Priority 2 - CI labels):
Evidence search (Priority 3 - Author justification):
Context:
To resolve Gate 5, either:
Gate 6: PR Quality - PASS
📚 Evidence ConsultedEvidence
❓ How to RespondResolving Gate 5 (Pre-Release Validation)The only remaining inconclusive gate is Gate 5. To resolve it: Option 1: Wait for live test results Option 2: Add justification in PR description ## AI PR Review Justification
### Gate 5 (Pre-Release Validation)
[Explain why validation is sufficient - e.g., regression test results, manual testing evidence]Option 3: Post a comment After adding your response, re-run Dismissing Prior REQUEST CHANGESThe prior REQUEST CHANGES review from octavia-bot (for Gate 3) can be dismissed by a maintainer since breaking change documentation is now complete. |
|
the live test finished running and all tests passed as seen in the workflow |
|
Great news! With the live test passing, the fix is validated. Summary:
Next steps:
Let me know if you need anything else! |
|
add the updated review report |
Updated Review ReportReview Status: Ready for Human Review
Live Testing EvidenceConnection tested: Breaking Change Documentation
Next Steps
Requested by sophiecuiy |
Ian Alton (ian-at-airbyte)
left a comment
Ian Alton (ian-at-airbyte)
left a comment
There was a problem hiding this comment.
The migration guide looks good. A couple of things for you Devin:
- You should apply the linting suggestions on the docs you've modified.
- You need to update the main Stripe doc page with a new changelog entry and, if applicable, specifics about the affected streams to ensure their current description is still correct. This is mandatory to pass CI.
|
/ai-docs-review
|
…v6.0.0 Co-Authored-By: sophie.cui@airbyte.io <sophie.cui@airbyte.io>
|
Ian Alton (@ian-at-airbyte) I've addressed your feedback:
Commit: a7fda3a |
Documentation ReviewOverall Assessment: The documentation changes are solid. The migration guide is clear, well-structured, and accurately describes the breaking change. The changelog entry and stream sync mode corrections in High Priority (Correctness Issues)
Completeness Gaps
Readability/OrganizationDocumentation is well-organized and clear. The migration guide follows a logical structure: summary of changes, migration steps with conditional paths for different destination capabilities, and downstream impact. No issues found. Style NitsNo significant style issues. The linting fixes from reviewdog suggestions appear to have been applied in the latest commit (heading whitespace fix on "Upgrading to 5.6.0", blank line before list). Summary of Recommended Changes
This review was generated by Devin. View session |
|
aside - sophiecuiy some of the recommendations Devin found in docs review feel like they might be connector issues to me. FYI. |
…notes - Add subscription_id and subscription_created fields to subscription_items full refresh stream for schema consistency - Add MigrationGuide component import and usage in migration guide - Add migration guide link to breakingChanges message in metadata.yaml Co-Authored-By: sophie.cui@airbyte.io <sophie.cui@airbyte.io>
|
Addressed the correctness and completeness notes from the docs review: Correctness fix:
Completeness fixes:
Commit: 77d67ea |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| The `invoice_line_items` stream now emits individual line item records with IDs starting with `il_` instead of invoice records with IDs starting with `in_`. Similarly, the `subscription_items` stream now emits individual subscription item records with IDs starting with `si_` instead of subscription records with IDs starting with `sub_`. | ||
|
|
||
| Each child record includes parent metadata fields for context: | ||
| - `invoice_line_items`: Includes `invoice_id`, `invoice_created`, and `invoice_updated` fields |
There was a problem hiding this comment.
[markdownlint] reported by reviewdog 🐶
MD032/blanks-around-lists Lists should be surrounded by blank lines [Context: "- invoice_line_items: Includ..."]
| The `invoice_line_items` stream now emits individual line item records with IDs starting with `il_` instead of invoice records with IDs starting with `in_`. Similarly, the `subscription_items` stream now emits individual subscription item records with IDs starting with `si_` instead of subscription records with IDs starting with `sub_`. | ||
|
|
||
| Each child record includes parent metadata fields for context: | ||
| - `invoice_line_items`: Includes `invoice_id`, `invoice_created`, and `invoice_updated` fields |
There was a problem hiding this comment.
[markdownlint-fix] reported by reviewdog 🐶
| - `invoice_line_items`: Includes `invoice_id`, `invoice_created`, and `invoice_updated` fields | |
| - `invoice_line_items`: Includes `invoice_id`, `invoice_created`, and `invoice_updated` fields |
The CI check requires the migration guide to start with the header '# Stripe Migration Guide' as the first line. Co-Authored-By: sophie.cui@airbyte.io <sophie.cui@airbyte.io>
The 5.6.0 section is not a breaking change, so it should use ### (h3) instead of ## (h2). The CI check validates that ## headings match the breakingChanges entries in metadata.yaml. Co-Authored-By: sophie.cui@airbyte.io <sophie.cui@airbyte.io>
3 participants
Link to Devin run: https://app.devin.ai/sessions/756ea1d8dac544ac899ca58aa3b71a4e
Requested by: sophiecuiy
Recreates: #70294
Related issues:
What
Fixes both
invoice_line_itemsandsubscription_itemsincremental streams which were incorrectly emitting parent objects instead of child items.Root cause (same for both streams): The incremental streams use the Stripe events endpoint which returns parent objects (invoices/subscriptions) with nested child arrays. The streams were emitting the entire parent object instead of extracting and emitting each child item separately.
invoice_line_itemsinvoice.*in_xxx)il_xxx) fromlines.data[]subscription_itemscustomer.subscription.*sub_xxx)si_xxx) fromitems.data[]How
Uses
RecordExpandercomponent withexpand_records_from_fieldandremain_original_recordparameters to apply the same fix pattern to both streams:RecordExpanderto expand the nested array and emit each item as a separate recordremain_original_record: trueto embed the parent object temporarilyrecord_filterto only emit records withoriginal_recordfieldoriginal_recordfield from final outputReview guide
invoice_line_items (manifest.yaml ~lines 1435-1487):
expand_records_from_field: ["lines", "data"]invoice_id,invoice_created,invoice_updatedsubscription_items (manifest.yaml ~lines 1576-1670):
expand_records_from_field: ["items", "data"]subscription_id,subscription_created,subscription_updatedBreaking change metadata (metadata.yaml):
6.0.0breakingChangesentry with upgrade deadline2026-02-19invoice_line_itemsandsubscription_itemsstreamsMigration guide (docs/integrations/sources/stripe-migrations.md):
MigrationGuidecomponent (import placed after header per CI requirements)Connector docs (docs/integrations/sources/stripe.md):
(Incremental)RecordExpanderCDK component is available in the current CDK version (original PR was blocked on CDK PR Basic Normalization doesn't Normalize #859)User Impact
Positive:
Negative:
Can this PR be safely reverted and rolled back?
Reason: This changes the fundamental data structure of both streams. Reverting would cause inconsistency between historical and new data.
Updates since last revision
Latest (d16603c): Fixed migration guide header order:
import MigrationGuidestatement after the# Stripe Migration Guideheader (CI requires header as first line)Previous (77d67ea): Addressed docs review correctness and completeness notes:
subscription_itemsfull refresh stream to includesubscription_idandsubscription_createdfields (was only addingsubscription_updated, causing schema inconsistency between sync modes)MigrationGuidecomponent import and usage in migration guidebreakingChangesmessage in metadata.yamlPrevious (a7fda3a): Documentation updates per reviewer feedback:
Invoice Line ItemsandSubscription Itemsas(Incremental)Initial: Added breaking change documentation per Gate 3 remediation requirements:
6.0.0(major version bump for breaking change)breakingChangesentry inmetadata.yamlwith scoped impactdocs/integrations/sources/stripe-migrations.md