-
Notifications
You must be signed in to change notification settings - Fork 5.4k
17235 source gmail sources return too much text #17236
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
17235 source gmail sources return too much text #17236
Conversation
…ources - Bump package versions for @pipedream/gmail to 1.0.2, actions to their respective new versions, and sources to 0.1.0 or 0.3.0. - Introduce 'withTextPayload' option in new email sources to return payload as plaintext, enhancing usability for LLMs. - Refactor message handling in sources and actions to support the new payload format.
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis update introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SourceComponent
participant Utils
User->>SourceComponent: Enable withTextPayload and trigger event
SourceComponent->>Utils: validateTextPayload(message, withTextPayload)
alt withTextPayload is true and text extracted
Utils-->>SourceComponent: Parsed message with plaintext payload
SourceComponent-->>User: Emit event with simplified plaintext message
else withTextPayload is false or no text extracted
Utils-->>SourceComponent: false
SourceComponent-->>User: Emit event with original message structure
end
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (3)
components/gmail/sources/common/base.mjs (1)
21-48
: Good refactor with fallback logic, but consider adding error handling.The refactored
decodeContent
method effectively centralizes text payload processing while maintaining backward compatibility through fallback logic. The approach of tryingutils.validateTextPayload
first and falling back to the original logic is sound.However, consider adding error handling around the utility call to prevent potential runtime exceptions:
decodeContent(message) { + let parsedMessage; + try { + parsedMessage = utils.validateTextPayload(message, this.withTextPayload); + } catch (error) { + console.warn('Error in validateTextPayload:', error); + parsedMessage = null; + } - let parsedMessage = utils.validateTextPayload(message, this.withTextPayload); if (!parsedMessage) { // ... rest of fallback logicPlease verify that
utils.validateTextPayload
properly handles edge cases and returns falsy values when fallback logic should be used:#!/bin/bash # Search for validateTextPayload implementation and usage rg -A 10 "validateTextPayload" --type jscomponents/gmail/actions/find-email/find-email.mjs (1)
93-103
: Good refactor to centralized utility, but add error handling.The replacement of manual text extraction logic with
utils.validateTextPayload
centralizes the functionality and reduces code duplication. The fallback logic appropriately maintains the original behavior when the utility returns falsy.Consider adding error handling similar to the suggestion for base.mjs:
- const parsedMessage = utils.validateTextPayload(message, this.withTextPayload); + let parsedMessage; + try { + parsedMessage = utils.validateTextPayload(message, this.withTextPayload); + } catch (error) { + console.warn('Error in validateTextPayload:', error); + parsedMessage = null; + }components/gmail/sources/new-attachment-received/new-attachment-received.mjs (1)
62-62
: Clean integration of text payload utility with appropriate fallback.The implementation properly:
- Calls the centralized utility function
- Uses the parsed message when available
- Falls back to the original message when utility returns falsy
- Maintains backward compatibility
Consider adding error handling around the utility call for improved robustness:
- const parsedMessage = utils.validateTextPayload(message, this.withTextPayload); + let parsedMessage; + try { + parsedMessage = utils.validateTextPayload(message, this.withTextPayload); + } catch (error) { + console.warn('Error in validateTextPayload:', error); + parsedMessage = null; + }Also applies to: 66-66
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
components/gmail/actions/add-label-to-email/add-label-to-email.mjs
(1 hunks)components/gmail/actions/approve-workflow/approve-workflow.mjs
(1 hunks)components/gmail/actions/archive-email/archive-email.mjs
(1 hunks)components/gmail/actions/create-draft/create-draft.mjs
(1 hunks)components/gmail/actions/download-attachment/download-attachment.mjs
(1 hunks)components/gmail/actions/find-email/find-email.mjs
(3 hunks)components/gmail/actions/list-labels/list-labels.mjs
(1 hunks)components/gmail/actions/remove-label-from-email/remove-label-from-email.mjs
(1 hunks)components/gmail/actions/send-email/send-email.mjs
(1 hunks)components/gmail/actions/update-org-signature/update-org-signature.mjs
(1 hunks)components/gmail/actions/update-primary-signature/update-primary-signature.mjs
(1 hunks)components/gmail/common/utils.mjs
(1 hunks)components/gmail/package.json
(2 hunks)components/gmail/sources/common/base.mjs
(2 hunks)components/gmail/sources/new-attachment-received/new-attachment-received.mjs
(4 hunks)components/gmail/sources/new-email-matching-search/new-email-matching-search.mjs
(2 hunks)components/gmail/sources/new-email-received/new-email-received.mjs
(2 hunks)components/gmail/sources/new-labeled-email/new-labeled-email.mjs
(2 hunks)components/gmail/sources/new-sent-email/new-sent-email.mjs
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
🔇 Additional comments (25)
components/gmail/actions/download-attachment/download-attachment.mjs (1)
10-10
: Version bump looks correct
Updating the action version to0.0.11
aligns with the coordinated release cycle across Gmail components.components/gmail/actions/add-label-to-email/add-label-to-email.mjs (1)
7-7
: Version bump looks good.The version increment is consistent with the coordinated update across Gmail actions in this PR.
components/gmail/actions/create-draft/create-draft.mjs (1)
9-9
: Version bump looks good.The version increment is consistent with the coordinated update across Gmail actions in this PR.
components/gmail/actions/remove-label-from-email/remove-label-from-email.mjs (1)
7-7
: Version bump looks good.The version increment is consistent with the coordinated update across Gmail actions in this PR.
components/gmail/actions/approve-workflow/approve-workflow.mjs (1)
7-7
: Version bump looks good.The version increment is consistent with the coordinated update across Gmail actions in this PR.
components/gmail/actions/update-org-signature/update-org-signature.mjs (1)
11-11
: Bump version to 0.0.11
Consistent metadata update aligning with the PR’s coordinated version increments.components/gmail/actions/send-email/send-email.mjs (1)
9-9
: Bump version to 0.1.15
No functional changes; version increment matches other Gmail actions in this PR.components/gmail/actions/list-labels/list-labels.mjs (1)
7-7
: Bump version to 0.0.8
Only metadata updated—version aligns with overall Gmail component versioning.components/gmail/actions/update-primary-signature/update-primary-signature.mjs (1)
7-7
: Bump version to 0.0.11
Metadata change only; consistent with the PR’s version bump strategy.components/gmail/actions/archive-email/archive-email.mjs (1)
8-8
: Bump version to 0.0.7
No logic alterations—version update keeps pace with other Gmail actions.components/gmail/package.json (1)
3-3
: Let’s broaden the search to cover all common JS/TS file extensions and drop the unrecognized--type
flags:#!/bin/bash # Broader search for any import/require of @pipedream/platform across JS/TS files rg -n "@pipedream/platform" -g "*.js" -g "*.ts" -g "*.cjs" -g "*.mjs"components/gmail/sources/new-email-received/new-email-received.mjs (2)
18-18
: LGTM! Appropriate version bump for new feature.The version update from 0.2.8 to 0.3.0 correctly uses a minor version bump for the new
withTextPayload
feature addition.
44-49
: LGTM! Well-defined property with clear benefits.The
withTextPayload
property is well-implemented with:
- Clear, descriptive label and description
- Appropriate boolean type
- Backward-compatible default value (
false
)- Clear explanation of LLM processing benefits
components/gmail/sources/new-labeled-email/new-labeled-email.mjs (2)
11-11
: LGTM! Consistent version bump across components.The version update from 0.0.14 to 0.1.0 appropriately uses a minor version bump for the new feature addition, maintaining consistency with other Gmail source components.
24-29
: LGTM! Consistent property implementation.The
withTextPayload
property implementation is identical to other Gmail source components, ensuring consistent behavior and user experience across the component ecosystem.components/gmail/sources/new-sent-email/new-sent-email.mjs (1)
9-9
: LGTM! Consistent implementation across Gmail sources.The version bump and
withTextPayload
property implementation maintain perfect consistency with other Gmail source components, ensuring a unified user experience.Also applies to: 21-26
components/gmail/sources/new-email-matching-search/new-email-matching-search.mjs (1)
9-9
: LGTM! Excellent consistency across all Gmail source components.The version bump and
withTextPayload
property implementation complete the consistent rollout across all Gmail source components. The standardized property definition ensures users will have a consistent experience regardless of which Gmail source they use.Also applies to: 30-35
components/gmail/sources/common/base.mjs (1)
2-2
: LGTM: Clean utility import.The import of the utils module is appropriately placed and follows the existing import structure.
components/gmail/actions/find-email/find-email.mjs (3)
8-8
: Appropriate version bump.The minor version increment from 0.1.4 to 0.1.5 correctly reflects the addition of new functionality.
18-23
: Well-defined property for text payload functionality.The
withTextPayload
property is properly configured with:
- Appropriate boolean type
- Clear, descriptive label and description
- Sensible default value of
false
to maintain backward compatibility- Good explanation of benefits for LLM processing
58-58
: Correct variable declaration change.Changing from
const
tolet
is necessary since themessage
variable is potentially reassigned on line 95. This prevents potential runtime errors.components/gmail/sources/new-attachment-received/new-attachment-received.mjs (4)
1-1
: LGTM: Utility import added consistently.The utils import follows the same pattern established in other Gmail components.
10-10
: Verify the major version bump rationale.The version increased from 0.0.15 to 0.1.0, which is a minor version bump in semantic versioning but represents a more significant change than the other components. Ensure this aligns with your versioning strategy for the new functionality.
31-36
: Excellent property definition with clear benefits.The
withTextPayload
property is well-defined with:
- Clear labeling and description
- Explicit mention of LLM benefits and payload size reduction
- Appropriate default to maintain backward compatibility
61-61
: Good defensive programming practice.The safe default
= []
for the attachments array prevents potential runtime errors ifmessage.payload.parts
is undefined. This improves the robustness of the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @luancazarine lgtm! Ready for QA!
- Simplify the logic for validating and extracting text payloads from messages. - Ensure that the new payload is assigned correctly based on the presence of body data or parts.
/approve |
Resolves #17235
Summary by CodeRabbit