-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
New Components - zep #15616
base: master
Are you sure you want to change the base?
New Components - zep #15616
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
Warning Rate limit exceeded@michelle0927 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 53 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request extends the Zep component by introducing several new modules for session management and event handling. New action modules for adding memory, creating sessions, updating sessions, and adding users have been added alongside new source modules to manage new messages, new sessions, and session updates. Additionally, utility functions for JSON parsing were introduced, and the Zep app now includes extra API request methods and properties. The package configuration was updated with a new version and dependency details. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant M as Module (Action/Source)
participant Z as Zep App
participant API as External API
C->>M: Invoke run() with parameters
M->>Z: Call respective method (createSession, addMemoryToSession, updateSession, etc.)
Z->>API: _makeRequest(payload)
API-->>Z: Return API response
Z-->>M: Return summary response
M-->>C: Output summary message
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 3
🧹 Nitpick comments (11)
components/zep/zep.app.mjs (3)
6-21
: Return labeled options forsessionId
Currently, the code returns just an array of session IDs. Consider returning an array of{ value, label }
objects for improved user clarity in the UI.
40-50
: Consider adding validation or defaults
While it’s valid to keepfactRatingInstructions
andmetadata
free-form, adding optional validation or default values can help handle edge cases.
53-68
: Add more robust error handling
Consider wrapping theaxios
call in a try/catch block or leveraging Pipedream’s error-handling features. Also, to follow conventional headers, use “Authorization” instead of “authorization” for clarity.components/zep/common/utils.mjs (2)
1-9
: Handle potential JSON parsing errors inparseObject
Ifobj
is not valid JSON when it's a string,JSON.parse(obj)
will throw an exception. Consider a try/catch or a fallback to avoid runtime errors.
11-25
: Handle potential JSON parsing errors inparseArray
Ifarr
is an invalid JSON string,JSON.parse(arr)
will throw. Adding error handling or defaults can make the function more robust.components/zep/actions/update-session/update-session.mjs (1)
41-41
: Fix typo in success message.There's a typo in the word "session".
Apply this diff to fix the typo:
- $.export("$summary", `Successfully updated ssession with ID ${this.sessionId}`); + $.export("$summary", `Successfully updated session with ID ${this.sessionId}`);components/zep/actions/create-session/create-session.mjs (1)
7-7
: Update documentation link.The documentation link uses "add-session" instead of "create-session" which might be confusing.
Apply this diff to update the link:
- description: "Creates a new session in Zep. [See the documentation](https://help.getzep.com/api-reference/memory/add-session)", + description: "Creates a new session in Zep. [See the documentation](https://help.getzep.com/api-reference/memory/create-session)",components/zep/actions/add-memory/add-memory.mjs (2)
18-22
: Enhance messages prop description with a complete example.The current example shows a single message object. Consider providing a more comprehensive example that demonstrates multiple messages with different role types.
- description: "An array of message objects, where each message contains a role (`norole`, `system`, `assistant`, `user`, `function`, or `tool`) and content. Example: `[{ \"content\": \"content\", \"role_type\": \"norole\" }]` [See the documentation](https://help.getzep.com/api-reference/memory/add) for more information", + description: "An array of message objects, where each message contains a role (`norole`, `system`, `assistant`, `user`, `function`, or `tool`) and content. Example: `[{ \"content\": \"Hello\", \"role_type\": \"user\" }, { \"content\": \"Hi there!\", \"role_type\": \"assistant\" }]` [See the documentation](https://help.getzep.com/api-reference/memory/add) for more information",
42-54
: Add error handling for message parsing.The
parseArray
function could throw an error if the messages are not properly formatted. Consider adding try-catch block to handle potential parsing errors gracefully.async run({ $ }) { + let parsedMessages; + try { + parsedMessages = utils.parseArray(this.messages); + } catch (error) { + throw new Error(`Failed to parse messages: ${error.message}`); + } const response = await this.zep.addMemoryToSession({ sessionId: this.sessionId, data: { - messages: utils.parseArray(this.messages), + messages: parsedMessages, factInstruction: this.factInstruction, returnContext: this.returnContext, summaryInstruction: this.summaryInstruction, }, }); $.export("$summary", `Added memory to session ${this.sessionId}`); return response; },components/zep/sources/common/base.mjs (2)
31-31
: Extract magic number into a named constant.The page size limit of 1000 should be extracted into a named constant for better maintainability.
+const MAX_PAGE_SIZE = 1000; + export default { // ... methods: { async getSessions({ lastTs, orderBy, max, }) { const params = { - page_size: max || 1000, + page_size: max || MAX_PAGE_SIZE,
70-77
: Extract magic number into a named constant.The event limit of 25 in the deploy hook should be extracted into a named constant for better maintainability.
+const INITIAL_EVENT_LIMIT = 25; + export default { // ... hooks: { async deploy() { - await this.processEvent(25); + await this.processEvent(INITIAL_EVENT_LIMIT); }, },
📜 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 (10)
components/zep/actions/add-memory/add-memory.mjs
(1 hunks)components/zep/actions/create-session/create-session.mjs
(1 hunks)components/zep/actions/update-session/update-session.mjs
(1 hunks)components/zep/common/utils.mjs
(1 hunks)components/zep/package.json
(2 hunks)components/zep/sources/common/base.mjs
(1 hunks)components/zep/sources/new-message/new-message.mjs
(1 hunks)components/zep/sources/new-session/new-session.mjs
(1 hunks)components/zep/sources/session-updated/session-updated.mjs
(1 hunks)components/zep/zep.app.mjs
(1 hunks)
🧰 Additional context used
🪛 GitHub Check: Lint Code Base
components/zep/sources/session-updated/session-updated.mjs
[warning] 6-6:
Source names should start with "New". See https://pipedream.com/docs/components/guidelines/#source-name
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (13)
components/zep/zep.app.mjs (3)
1-2
: Validate environment for@pipedream/platform
usage
This import appears correct for Pipedream, but please ensure that the environment supports the@pipedream/platform
package.
22-39
: LGTM
This approach foruserId
provides a user-friendly label combining the first and last names.
69-113
:✅ Verification successful
Verify endpoints and param consistency
All these methods look appropriate, but confirm the naming and parameters match the actual API docs.
🏁 Script executed:
#!/bin/bash # Searching for references to Zep endpoints rg -A3 '"sessions-ordered"' rg -A3 '"users-ordered"' rg -A3 '"messages"' rg -A3 '"sessions"'Length of output: 7199
API Endpoints and Parameter Naming Verified
I've reviewed the code along with repository references and found that the endpoints and parameter destructuring in the Zep implementation conform to the patterns used elsewhere in our codebase. In particular:
/sessions-ordered
&/users-ordered
These endpoints appear to be unique to this Zep integration. Their naming is consistent with their purpose in listing sessions and users.
listMessages
,addMemoryToSession
, andupdateSession
The use of a destructuredsessionId
and the template literal for forming the endpoint paths match the established patterns for session-specific endpoints. The HTTP methods (POST, PATCH) correctly align with their respective actions.No inconsistency was observed relative to how endpoints are defined in similar modules. Still, please ensure these match the current API documentation for Zep as a final confirmation.
components/zep/sources/new-session/new-session.mjs (2)
3-10
: Overall source definition looks good
Providing a clear key, name, and description is beneficial for discoverability and manageability.
13-26
: Check validity of session timestamps
Date.parse(session.created_at)
can yieldNaN
ifcreated_at
is missing or malformed. Ensurecreated_at
is always present and valid.components/zep/sources/session-updated/session-updated.mjs (1)
13-19
: LGTM! Well-structured implementation.The implementation looks good:
getNewResults
correctly orders byupdated_at
for chronological processinggenerateMeta
generates unique IDs by combining session ID with timestampAlso applies to: 20-27
components/zep/actions/update-session/update-session.mjs (1)
32-43
: LGTM! Well-structured implementation.The implementation looks good:
- Correctly uses utility functions for parsing metadata and fact rating instructions
- Properly handles response and exports summary
components/zep/actions/create-session/create-session.mjs (1)
37-49
: LGTM! Well-structured implementation.The implementation looks good:
- Correctly uses utility functions for parsing metadata and fact rating instructions
- Properly handles response and exports summary
components/zep/sources/new-message/new-message.mjs (1)
22-42
: LGTM! Well-structured implementation.The implementation looks good:
- Correctly handles timestamp-based filtering
- Properly implements message slicing for max results
- Generates unique metadata for each message
Also applies to: 62-68
components/zep/actions/add-memory/add-memory.mjs (1)
8-8
: Version number is inconsistent with package.json.The version number "0.0.1" is inconsistent with the package.json version "0.1.0". Consider updating to match the package version.
components/zep/sources/common/base.mjs (2)
1-14
: LGTM!The implementation correctly uses the platform's default polling interval and properly defines the required props.
16-21
: LGTM!The timestamp management methods are well-implemented with proper persistence and default value handling.
components/zep/package.json (1)
3-17
: LGTM!The version bump and dependency addition are appropriate for the new features introduced in this PR.
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 @michelle0927, LGTM! Ready for QA!
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: 1
🧹 Nitpick comments (4)
components/zep/sources/new-message/new-message.mjs (2)
44-52
: Consider making the session limit configurable.The method currently has a hard-coded limit of 100 sessions. For high-traffic systems, this might lead to missed messages. Consider making this limit configurable through component props or environment variables.
+ props: { + maxSessions: { + type: "integer", + label: "Maximum Sessions to Process", + description: "The maximum number of sessions to process in each run. Increase this for high-traffic systems.", + default: 100, + optional: true, + }, + }, methods: { async getRecentlyUpdatedSessionIds(lastTs) { const sessions = await this.getSessions({ lastTs, orderBy: "updated_at", updateLastTs: false, - max: 100, + max: this.maxSessions || 100, });
72-78
: Consider truncating the message content in the summary.Including the full message content in the summary might make the logs difficult to read for long messages. Consider truncating the content to a reasonable length.
generateMeta(message) { + const MAX_SUMMARY_LENGTH = 100; + const truncatedContent = message.content.length > MAX_SUMMARY_LENGTH + ? `${message.content.slice(0, MAX_SUMMARY_LENGTH)}...` + : message.content; return { id: message.uuid, - summary: `New Message: ${message.content}`, + summary: `New Message: ${truncatedContent}`, ts: Date.parse(message.created_at), }; },components/zep/zep.app.mjs (1)
42-52
: Consider adding schema validation for factRatingInstructions.While the documentation and example are helpful, consider adding schema validation to ensure the object structure matches the expected format.
factRatingInstructions: { type: "object", label: "Fact Rating Instructions", description: "Instructions to use for the fact rating consisting of examples and instruction. Example: `{ \"examples\": { \"high\": \"high\", \"low\": \"low\", \"medium\": \"medium\" }, \"instruction\": \"instruction\" }`. [See the documentation](https://help.getzep.com/api-reference/memory/add-session) for more info.", optional: true, + additionalProperties: false, + properties: { + examples: { + type: "object", + properties: { + high: { type: "string" }, + medium: { type: "string" }, + low: { type: "string" } + }, + required: ["high", "medium", "low"] + }, + instruction: { type: "string" } + }, + required: ["examples", "instruction"] },components/zep/actions/add-user/add-user.mjs (1)
10-49
: Consider reordering properties for better UX.The properties are well-defined, but consider reordering them to improve user experience by:
- Grouping required fields first (factRatingInstructions)
- Following with core optional fields (userId, email)
- Ending with supplementary optional fields (firstName, lastName, metadata)
props: { zep, + factRatingInstructions: { + propDefinition: [ + zep, + "factRatingInstructions", + ], + }, + userId: { + type: "string", + label: "User ID", + description: "The unique identifier of the new user", + optional: true, + }, email: { type: "string", label: "Email", description: "Email address of the user", optional: true, }, firstName: { type: "string", label: "First Name", description: "First name of the new user", optional: true, }, lastName: { type: "string", label: "Last Name", description: "Last name of the new user", optional: true, }, - factRatingInstructions: { - propDefinition: [ - zep, - "factRatingInstructions", - ], - }, metadata: { propDefinition: [ zep, "metadata", ], optional: true, }, - userId: { - type: "string", - label: "User ID", - description: "The unique identifier of the new user", - optional: true, - }, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
components/zep/actions/add-user/add-user.mjs
(1 hunks)components/zep/sources/common/base.mjs
(1 hunks)components/zep/sources/new-message/new-message.mjs
(1 hunks)components/zep/zep.app.mjs
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/zep/sources/common/base.mjs
🔇 Additional comments (11)
components/zep/sources/new-message/new-message.mjs (3)
1-11
: LGTM!The component configuration is well-structured with appropriate documentation, versioning, and deduplication strategy.
13-43
: LGTM!The method effectively retrieves and processes new messages with proper timestamp handling, sorting, and result limiting.
53-71
: Improve pagination implementation.The current pagination implementation has potential issues:
- The cursor starts at 1 which might skip the first page
- The loop condition relies on array length which might be misleading if the last page has exactly the limit size
Apply this diff to improve the pagination:
async *paginateMessages() { const params = { limit: 1000, - cursor: 1, + cursor: 0, }; - let total; + let hasMore = true; - do { + while (hasMore) { const { messages } = await this.zep.listMessages({ sessionId: this.sessionId, params, }); for (const message of messages) { yield message; } - total = messages?.length; + hasMore = messages?.length === params.limit; params.cursor++; - } while (total); + } },components/zep/zep.app.mjs (6)
1-1
: LGTM! Using platform's axios instance.Using the axios instance from @pipedream/platform is the recommended approach for making HTTP requests in Pipedream components.
7-21
: LGTM! Well-structured session ID prop definition.The sessionId prop is well-implemented with:
- Proper type and descriptive label
- Async options for dynamic loading
- Pagination support
- Sorting by updated_at in descending order
22-41
: LGTM! User-friendly user ID prop implementation.The userId prop implementation is well thought out with:
- Dynamic loading of user options
- User-friendly label construction using name or email fallback
- Proper pagination support
55-70
: LGTM! Well-structured API request handling.The utility methods are well-implemented:
- Clean base URL management
- Centralized request handling with proper authentication
- Good parameter flexibility
71-90
: LGTM! Consistent list operation implementations.The list methods follow a consistent pattern and properly handle parameters.
91-122
:✅ Verification successful
LGTM! RESTful implementation of session and user operations.
The create/update methods:
- Follow REST conventions
- Handle parameters consistently
- Maintain good separation of concerns
However, verify that the API endpoint is correct and accessible.
🏁 Script executed:
#!/bin/bash # Verify the API endpoint is accessible curl -I https://api.getzep.com/api/v2/sessionsLength of output: 664
API Endpoint Verified & Accessible
LGTM! The RESTful operations incomponents/zep/zep.app.mjs
(lines 91-122) demonstrate proper REST conventions, consistent parameter handling, and clear separation of concerns. We verified that the API endpoint (https://api.getzep.com/api/v2/sessions
) is reachable. The 401 response indicates that authentication credentials are required—which is expected behavior when none are provided. Please ensure that valid authentication is configured in production if necessary.components/zep/actions/add-user/add-user.mjs (2)
1-2
: LGTM!The imports are appropriate for the module's requirements.
4-9
:✅ Verification successful
Verify the documentation link.
The metadata is well-structured. Let's verify that the documentation link is accessible and points to the correct API reference.
🏁 Script executed:
#!/bin/bash # Description: Verify that the Zep API documentation link is accessible curl -I "https://help.getzep.com/api-reference/user/add"Length of output: 796
Documentation Link Verified
The metadata in
components/zep/actions/add-user/add-user.mjs
is well-organized, and the documentation link (https://help.getzep.com/api-reference/user/add
) has been verified to return a successful HTTP 200 response. No further action is required on this part.
/approve |
Resolves #15601.
Summary by CodeRabbit
New Features
Chores