Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Components] langbase #14085 #14466

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

[Components] langbase #14085 #14466

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
7189016
Added actions
lcaresia Oct 30, 2024
d453fda
Merge branch 'master' into issue-14085
lcaresia Oct 30, 2024
f76e21a
Added actions
lcaresia Oct 30, 2024
3c5c02d
Merge branch 'master' into issue-14085
lcaresia Nov 5, 2024
096fe07
package-lock
lcaresia Nov 5, 2024
8a2bb55
package-lock
lcaresia Nov 5, 2024
9616f4f
Merge branch 'issue-14085' of github.com:PipedreamHQ/pipedream into i…
lcaresia Nov 5, 2024
079bc48
Fixing action name
lcaresia Nov 5, 2024
70ca4d6
Merge branch 'master' into issue-14085
lcaresia Nov 7, 2024
9d052cc
Merge branch 'master' into issue-14085
lcaresia Nov 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions components/langbase/actions/create-memory/create-memory.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import app from "../../langbase.app.mjs";

export default {
key: "langbase-create-memory",
name: "Create Memory",
description: "Create a new organization memory by sending the memory data. [See the documentation](https://langbase.com/docs/api-reference/memory/create)",
version: "0.0.1",
type: "action",
props: {
app,
name: {
propDefinition: [
app,
"name",
],
},
description: {
propDefinition: [
app,
"description",
],
},
},

async run({ $ }) {
const response = await this.app.createMemory({
$,
data: {
name: this.name,
description: this.description,
},
});

$.export("$summary", `Successfully created memory ${this.name}`);

return response;
},
};
29 changes: 29 additions & 0 deletions components/langbase/actions/delete-memory/delete-memory.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import app from "../../langbase.app.mjs";

export default {
key: "langbase-delete-memory",
name: "Delete Memory",
description: "Delete an existing memory on Langbase. [See the documentation](https://langbase.com/docs/api-reference/memory/delete)",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Documentation URL needs to be updated to match the API domain

The documentation URL in the component uses langbase.com/docs, but the actual API domain is api.langbase.com. The documentation URL should be consistent with the API domain being used in the application code.

  • Update the documentation URL in components/langbase/actions/delete-memory/delete-memory.mjs to use the correct domain
  • Also update similar documentation URLs in other action files:
    • components/langbase/actions/list-memories/list-memories.mjs
    • components/langbase/actions/create-memory/create-memory.mjs
🔗 Analysis chain

Verify the documentation URL

The documentation URL appears to use a placeholder domain (langbase.com). Please ensure this points to the correct documentation URL.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if the documentation URL is referenced elsewhere in the codebase
rg -l "langbase\.com/docs" --type-add 'docs:*.{md,mdx}' -tdocs

Length of output: 2271

Script:

#!/bin/bash
# Check for the base URL pattern in all JavaScript/TypeScript files
rg "langbase\.com" --type=js

# Also check the app file for API configuration
cat components/langbase/langbase.app.mjs

Length of output: 1851

version: "0.0.1",
type: "action",
props: {
app,
memoryName: {
propDefinition: [
app,
"memoryName",
],
},
},

async run({ $ }) {
const response = await this.app.deleteMemory({
$,
memoryName: this.memoryName,
});

$.export("$summary", `Successfully deleted memory named ${this.memoryName}`);

return response;
},
Comment on lines +19 to +28
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling and response validation

The current implementation lacks error handling and response validation. Consider adding try-catch block and response validation.

Here's a suggested improvement:

  async run({ $ }) {
+   if (!this.memoryName) {
+     throw new Error("Memory name is required");
+   }
+
+   try {
      const response = await this.app.deleteMemory({
        $,
        memoryName: this.memoryName,
      });
+
+     if (!response) {
+       throw new Error("Received invalid response from deleteMemory");
+     }

      $.export("$summary", `Successfully deleted memory named ${this.memoryName}`);

      return response;
+   } catch (error) {
+     throw new Error(`Failed to delete memory: ${error.message}`);
+   }
  },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async run({ $ }) {
const response = await this.app.deleteMemory({
$,
memoryName: this.memoryName,
});
$.export("$summary", `Successfully deleted memory named ${this.memoryName}`);
return response;
},
async run({ $ }) {
if (!this.memoryName) {
throw new Error("Memory name is required");
}
try {
const response = await this.app.deleteMemory({
$,
memoryName: this.memoryName,
});
if (!response) {
throw new Error("Received invalid response from deleteMemory");
}
$.export("$summary", `Successfully deleted memory named ${this.memoryName}`);
return response;
} catch (error) {
throw new Error(`Failed to delete memory: ${error.message}`);
}
},

};
22 changes: 22 additions & 0 deletions components/langbase/actions/list-memories/list-memories.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import app from "../../langbase.app.mjs";

export default {
key: "langbase-list-memories",
name: "List Memories",
description: "Get a list of memory sets on Langbase. [See the documentation](https://langbase.com/docs/api-reference/memory/list)",
version: "0.0.1",
type: "action",
props: {
app,
},

async run({ $ }) {
const response = await this.app.listMemories({
$,
});

$.export("$summary", `Successfully retrieved ${response.memorySets.length} memorysets`);

return response;
},
Comment on lines +13 to +21
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling and response validation.

The current implementation has several areas that could be improved for better reliability and user experience:

  1. Add try-catch block for API error handling
  2. Validate response structure before accessing memorySets
  3. Enhance summary message with more details

Consider applying these improvements:

 async run({ $ }) {
+  try {
     const response = await this.app.listMemories({
       $,
     });
 
+    if (!response?.memorySets) {
+      throw new Error('Invalid response structure: memorySets not found');
+    }
+
+    const count = response.memorySets.length;
     $.export("$summary", `Successfully retrieved ${response.memorySets.length} memorysets`);
 
     return response;
+  } catch (error) {
+    throw new Error(`Failed to list memories: ${error.message}`);
+  }
 },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async run({ $ }) {
const response = await this.app.listMemories({
$,
});
$.export("$summary", `Successfully retrieved ${response.memorySets.length} memorysets`);
return response;
},
async run({ $ }) {
try {
const response = await this.app.listMemories({
$,
});
if (!response?.memorySets) {
throw new Error('Invalid response structure: memorySets not found');
}
const count = response.memorySets.length;
$.export("$summary", `Successfully retrieved ${response.memorySets.length} memorysets`);
return response;
} catch (error) {
throw new Error(`Failed to list memories: ${error.message}`);
}
},

};
74 changes: 70 additions & 4 deletions components/langbase/langbase.app.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,77 @@
import { axios } from "@pipedream/platform";

export default {
type: "app",
app: "langbase",
propDefinitions: {},
propDefinitions: {
memoryName: {
type: "string",
label: "Memory Name",
description: "The name of the memory",
async options() {
const response = await this.listMemories();
const memoryNames = response.memorySets;
return memoryNames.map(({
name, description,
}) => ({
label: description,
value: name,
}));
},
Comment on lines +11 to +20
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add error handling and null checks in options() method.

The options() method could fail if the API call fails or returns unexpected data structure.

Apply this diff to add proper error handling:

       async options() {
+        try {
           const response = await this.listMemories();
+          if (!response?.memorySets) {
+            console.log("No memory sets found or unexpected response structure");
+            return [];
+          }
           const memoryNames = response.memorySets;
           return memoryNames.map(({
             name, description,
           }) => ({
             label: description,
             value: name,
           }));
+        } catch (err) {
+          console.log("Error fetching memory options:", err);
+          return [];
+        }
       },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async options() {
const response = await this.listMemories();
const memoryNames = response.memorySets;
return memoryNames.map(({
name, description,
}) => ({
label: description,
value: name,
}));
},
async options() {
try {
const response = await this.listMemories();
if (!response?.memorySets) {
console.log("No memory sets found or unexpected response structure");
return [];
}
const memoryNames = response.memorySets;
return memoryNames.map(({
name, description,
}) => ({
label: description,
value: name,
}));
} catch (err) {
console.log("Error fetching memory options:", err);
return [];
}
},

},
name: {
type: "string",
label: "Name",
description: "Name of the memory",
},
description: {
type: "string",
label: "Description",
description: "Short description of the memory",
},
},
methods: {
// this.$auth contains connected account data
authKeys() {
console.log(Object.keys(this.$auth));
_baseUrl() {
return "https://api.langbase.com/beta";
},
async _makeRequest(opts = {}) {
const {
$ = this,
path,
headers,
...otherOpts
} = opts;
return axios($, {
...otherOpts,
url: this._baseUrl() + path,
headers: {
...headers,
"Authorization": `Bearer ${this.$auth.org_api_key}`,
"Accept": "application/json",
},
});
},
async createMemory(args = {}) {
return this._makeRequest({
path: `/org/${this.$auth.org}/memorysets`,
method: "post",
...args,
});
},
async deleteMemory({
memoryName, ...args
}) {
return this._makeRequest({
path: `/memorysets/sergio19733/${memoryName}`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix hard-coded organization ID in deleteMemory method

The deleteMemory method uses a hard-coded organization ID ('sergio19733'), which should be replaced with the dynamic value from this.$auth.org to ensure it works correctly for different organizations.

Apply this diff to fix the issue:

           return this._makeRequest({
-            path: `/memorysets/sergio19733/${memoryName}`,
+            path: `/org/${this.$auth.org}/memorysets/${memoryName}`,
             method: "delete",
             ...args,
           });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
path: `/memorysets/sergio19733/${memoryName}`,
path: `/org/${this.$auth.org}/memorysets/${memoryName}`,

method: "delete",
...args,
});
},
async listMemories(args = {}) {
return this._makeRequest({
path: `/org/${this.$auth.org}/memorysets`,
...args,
});
Comment on lines +54 to +74
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Standardize API path patterns across methods.

The API path patterns are inconsistent between methods. Some use /org/${this.$auth.org}/memorysets while others use a different pattern.

Apply this diff to standardize the paths:

     async createMemory(args = {}) {
       return this._makeRequest({
         path: `/org/${this.$auth.org}/memorysets`,
         method: "post",
         ...args,
       });
     },
     async deleteMemory({
       memoryName, ...args
     }) {
       return this._makeRequest({
-        path: `/memorysets/sergio19733/${memoryName}`,
+        path: `/org/${this.$auth.org}/memorysets/${memoryName}`,
         method: "delete",
         ...args,
       });
     },
     async listMemories(args = {}) {
       return this._makeRequest({
         path: `/org/${this.$auth.org}/memorysets`,
         ...args,
       });
     },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async createMemory(args = {}) {
return this._makeRequest({
path: `/org/${this.$auth.org}/memorysets`,
method: "post",
...args,
});
},
async deleteMemory({
memoryName, ...args
}) {
return this._makeRequest({
path: `/memorysets/sergio19733/${memoryName}`,
method: "delete",
...args,
});
},
async listMemories(args = {}) {
return this._makeRequest({
path: `/org/${this.$auth.org}/memorysets`,
...args,
});
async createMemory(args = {}) {
return this._makeRequest({
path: `/org/${this.$auth.org}/memorysets`,
method: "post",
...args,
});
},
async deleteMemory({
memoryName, ...args
}) {
return this._makeRequest({
path: `/org/${this.$auth.org}/memorysets/${memoryName}`,
method: "delete",
...args,
});
},
async listMemories(args = {}) {
return this._makeRequest({
path: `/org/${this.$auth.org}/memorysets`,
...args,
});

Copy link
Collaborator

@GTFalcao GTFalcao Nov 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this intended? (coderabbit's comment @ hard-coded organization)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GTFalcao, the suggestion aims to standardize the API path patterns across the methods. Currently, the deleteMemory method uses a hardcoded username sergio19733 in its path (/memorysets/sergio19733/${memoryName}), while the other methods (createMemory and listMemories) use the organization variable from this.$auth.org (e.g., /org/${this.$auth.org}/memorysets). Is the use of sergio19733 intentional, or should it be replaced with this.$auth.org to ensure consistency and proper functionality across different organizations?

},
},
};
7 changes: 5 additions & 2 deletions components/langbase/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@pipedream/langbase",
"version": "0.0.1",
"version": "0.1.0",
"description": "Pipedream Langbase Components",
"main": "langbase.app.mjs",
"keywords": [
Expand All @@ -11,5 +11,8 @@
"author": "Pipedream <[email protected]> (https://pipedream.com/)",
"publishConfig": {
"access": "public"
},
"dependencies": {
"@pipedream/platform": "^3.0.3"
}
}
}
Loading
Loading