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.

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

add media creation engine #2452

Closed
wants to merge 5 commits into from

Conversation

beatsfoundation
Copy link

Risks

Low/medium risk - adding a new plugin for the Beats Foundation media creation APIs to be accessible for AI agents via ai16z.

Background

What does this PR do?

adds beatsfoundation apis as described here: docs.beatsfoundation.com and here: beatsfoundation.com for agentic media creation.

What kind of change is this?

Features (non-breaking change which adds functionality)

Enabling ai16z agents to have much more powerful user impact through multimodal media generation in addition to text-based outputs and interactions. Example of our in-house agentic framework for song generation can be found at https://x.com/0xbeatscat.

Documentation changes needed?

README is included

Testing

Where should a reviewer start?

Detailed testing steps

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hi @beatsfoundation! Welcome to the elizaOS community. Thanks for submitting your first pull request; your efforts are helping us accelerate towards AGI. We'll review it shortly. You are now an elizaOS contributor!

Copy link
Contributor

coderabbitai bot commented Jan 17, 2025

📝 Walkthrough

Walkthrough

The pull request introduces the Beats Foundation plugin for ElizaOS, enabling AI-powered music generation. The plugin adds functionality to create, retrieve, and manage songs through a new package @elizaos/plugin-beatsfoundation. It includes actions for creating songs, fetching songs by ID, and listing songs, with comprehensive validation, error handling, and configuration management.

Changes

File Change Summary
agent/src/index.ts Added import and conditional inclusion of beatsfoundationPlugin
packages/plugin-beatsfoundation/... New plugin package with actions, services, types, and configuration files
packages/plugin-beatsfoundation/src/actions/CreateSong/... Implemented song creation action with validation and service
packages/plugin-beatsfoundation/src/actions/GetSongById/... Added action to retrieve a song by its ID
packages/plugin-beatsfoundation/src/actions/GetSongs/... Implemented action to list and paginate songs
packages/plugin-beatsfoundation/src/environment.ts Added environment configuration validation
packages/plugin-beatsfoundation/src/types.ts Defined interfaces for song generation and song data

Finishing Touches

  • 📝 Generate Docstrings (Beta)

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 9

🧹 Nitpick comments (15)
packages/plugin-beatsfoundation/src/actions/GetSongs/index.ts (2)

21-21: Correct the property name 'similes'

The property 'similes' may be incorrect. Did you mean 'aliases' to specify alternative names for the action?

Apply this diff:

-    similes: ["LIST_SONGS", "FETCH_SONGS", "SHOW_SONGS"],
+    aliases: ["LIST_SONGS", "FETCH_SONGS", "SHOW_SONGS"],

65-94: Simplify nested try-catch blocks

The nested try-catch blocks in your handler can be consolidated to avoid code duplication and improve readability.

Refactor the code as follows:

             // Existing code up to line 64

-            try {
-                const songs = await songsService.getSongs(content.limit, content.offset);
-                elizaLogger.success(
-                    `Songs retrieved successfully! Count: ${songs.length}`
-                );
-                if (callback) {
-                    callback({
-                        text: `Retrieved ${songs.length} songs`,
-                        content: {
-                            songs,
-                            pagination: {
-                                limit: content.limit,
-                                offset: content.offset
-                            }
-                        },
-                    });
-                }
-                return true;
-            } catch (error: any) {
-                elizaLogger.error("Error in GET_SONGS handler:", error);
-                if (callback) {
-                    callback({
-                        text: `Error fetching songs: ${error.message}`,
-                        content: { error: error.message },
-                    });
-                }
-                return false;
-            }
+            const songs = await songsService.getSongs(content.limit, content.offset);
+            elizaLogger.success(
+                `Songs retrieved successfully! Count: ${songs.length}`
+            );
+            if (callback) {
+                callback({
+                    text: `Retrieved ${songs.length} songs`,
+                    content: {
+                        songs,
+                        pagination: {
+                            limit: content.limit,
+                            offset: content.offset
+                        }
+                    },
+                });
+            }
+            return true;
         } catch (error: any) {
             elizaLogger.error("Error in GET_SONGS handler:", error);
             if (callback) {
                 callback({
                     text: `Error fetching songs: ${error.message}`,
                     content: { error: error.message },
                 });
             }
             return false;
         }
packages/plugin-beatsfoundation/src/actions/GetSongs/types.ts (1)

4-5: Provide default values for limit and offset

Since limit and offset are optional, ensure default values are set in the handler to prevent issues when they are undefined.

packages/plugin-beatsfoundation/src/actions/GetSongs/template.ts (1)

1-8: Add parameter range constraints to the template.

The template should specify valid ranges for limit and offset to guide the AI in generating appropriate values.

 export const getSongsTemplate = `
 Given the conversation context, extract any pagination parameters for retrieving songs.
 Return a JSON object with the following optional structure:
 {
-    "limit": number (optional),
-    "offset": number (optional)
+    "limit": number (optional, must be between 1 and 100),
+    "offset": number (optional, must be non-negative)
 }
 `;
packages/plugin-beatsfoundation/src/types.ts (2)

1-7: Add JSDoc comments and string literal types for better type safety.

Consider adding documentation and constraining the genre and mood fields to specific values.

+/**
+ * Request parameters for generating a song using the Beats Foundation API
+ */
 export interface GenerateSongRequest {
   prompt: string;
   lyrics?: string;
-  genre?: string;
-  mood?: string;
+  genre?: 'rock' | 'pop' | 'jazz' | 'classical' | 'electronic';
+  mood?: 'happy' | 'sad' | 'energetic' | 'calm' | 'mysterious';
   isInstrumental?: boolean;
 }

9-17: Add URL validation for audio_url and song_url fields.

Consider using a URL type or regex pattern to ensure valid URLs.

 export interface Song {
   id: string;
   title: string;
-  audio_url: string;
+  audio_url: URL | `https://${string}`;
   streams: number;
   upvote_count: number;
-  song_url: string;
+  song_url: URL | `https://${string}`;
   username: string;
 }
packages/plugin-beatsfoundation/src/actions/CreateSong/template.ts (1)

1-11: Align template with GenerateSongRequest type constraints.

Update the template to reflect the allowed values for genre and mood.

 export const createSongTemplate = `
 Given the conversation context, extract the song creation parameters.
 Return a JSON object with the following structure:
 {
     "prompt": string (required),
     "lyrics": string (optional),
-    "genre": string (optional),
-    "mood": string (optional),
+    "genre": string (optional, one of: rock, pop, jazz, classical, electronic),
+    "mood": string (optional, one of: happy, sad, energetic, calm, mysterious),
     "isInstrumental": boolean (optional)
 }
 `;
packages/plugin-beatsfoundation/src/index.ts (1)

9-12: Consider implementing required services for API communication

Empty arrays for clients and services suggest missing implementations. The plugin might need a dedicated service for handling API communication with Beats Foundation.

packages/plugin-beatsfoundation/src/actions/CreateSong/examples.ts (1)

1-27: Enhance example coverage

Add examples for edge cases and error scenarios:

  • Maximum length inputs
  • Special characters in lyrics
  • Invalid genre/mood combinations
packages/plugin-beatsfoundation/src/environment.ts (1)

16-18: Add sensitive data handling

Consider using a secure configuration manager for the API key.
[security]

packages/plugin-beatsfoundation/src/actions/CreateSong/service.ts (2)

10-11: Move API URL to configuration

The API endpoint should be configurable to support different environments and easier updates.

+import { BEATS_FOUNDATION_API_URL } from '../../config';
-                    'https://www.beatsfoundation.com/api/songs',
+                    `${BEATS_FOUNDATION_API_URL}/songs`,

21-26: Enhance error handling specificity

The current error handling could be more specific to help with debugging.

             } catch (error: any) {
                 if (error.response) {
-                    throw new Error(`Beats Foundation API Error: ${error.response.data.error || error.response.status}`);
+                    const errorMessage = error.response.data.error 
+                        ? `API Error: ${error.response.data.error}`
+                        : `HTTP ${error.response.status}: ${error.response.statusText}`;
+                    throw new Error(errorMessage);
+                } else if (error.request) {
+                    throw new Error(`Network Error: ${error.message}`);
                 }
                 throw error;
packages/plugin-beatsfoundation/src/actions/GetSongs/service.ts (1)

8-11: Improve type safety for pagination parameters

Replace Record<string, any> with a properly typed interface.

+            interface PaginationParams {
+                limit?: number;
+                offset?: number;
+            }
-            const params: Record<string, any> = {};
+            const params: PaginationParams = {};
packages/plugin-beatsfoundation/README.md (2)

73-77: Add blank lines around the table.

According to markdown best practices, tables should be surrounded by blank lines for better readability.

 ### Environment Variables
+
 | Variable | Description | Required |
 | -------- | ----------- | -------- |
 | BEATS_FOUNDATION_API_KEY | Your Beats Foundation API key | Yes |
+
🧰 Tools
🪛 Markdownlint (0.37.0)

74-74: null
Tables should be surrounded by blank lines

(MD058, blanks-around-tables)


80-97: Consider adding validation constraints to the interfaces.

The interfaces could benefit from additional JSDoc comments describing validation constraints, such as:

  • Maximum length for the prompt field
  • Supported values for genre and mood
  • Size limits for lyrics
 interface GenerateSongRequest {
+  /** Maximum 200 characters as specified in the parameters section */
   prompt: string;
+  /** Optional lyrics for the song */
   lyrics?: string;
+  /** Supported genres: pop, rock, jazz, etc. */
   genre?: string;
+  /** Supported moods: happy, sad, energetic, etc. */
   mood?: string;
   isInstrumental?: boolean;
 }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 94ee57b and a5cc630.

📒 Files selected for processing (26)
  • agent/src/index.ts (2 hunks)
  • packages/plugin-beatsfoundation/README.md (1 hunks)
  • packages/plugin-beatsfoundation/package.json (1 hunks)
  • packages/plugin-beatsfoundation/src/actions/CreateSong/examples.ts (1 hunks)
  • packages/plugin-beatsfoundation/src/actions/CreateSong/index.ts (1 hunks)
  • packages/plugin-beatsfoundation/src/actions/CreateSong/service.ts (1 hunks)
  • packages/plugin-beatsfoundation/src/actions/CreateSong/template.ts (1 hunks)
  • packages/plugin-beatsfoundation/src/actions/CreateSong/types.ts (1 hunks)
  • packages/plugin-beatsfoundation/src/actions/CreateSong/validation.ts (1 hunks)
  • packages/plugin-beatsfoundation/src/actions/GetSongById/examples.ts (1 hunks)
  • packages/plugin-beatsfoundation/src/actions/GetSongById/index.ts (1 hunks)
  • packages/plugin-beatsfoundation/src/actions/GetSongById/service.ts (1 hunks)
  • packages/plugin-beatsfoundation/src/actions/GetSongById/template.ts (1 hunks)
  • packages/plugin-beatsfoundation/src/actions/GetSongById/types.ts (1 hunks)
  • packages/plugin-beatsfoundation/src/actions/GetSongById/validation.ts (1 hunks)
  • packages/plugin-beatsfoundation/src/actions/GetSongs/examples.ts (1 hunks)
  • packages/plugin-beatsfoundation/src/actions/GetSongs/index.ts (1 hunks)
  • packages/plugin-beatsfoundation/src/actions/GetSongs/service.ts (1 hunks)
  • packages/plugin-beatsfoundation/src/actions/GetSongs/template.ts (1 hunks)
  • packages/plugin-beatsfoundation/src/actions/GetSongs/types.ts (1 hunks)
  • packages/plugin-beatsfoundation/src/actions/GetSongs/validation.ts (1 hunks)
  • packages/plugin-beatsfoundation/src/environment.ts (1 hunks)
  • packages/plugin-beatsfoundation/src/index.ts (1 hunks)
  • packages/plugin-beatsfoundation/src/types.ts (1 hunks)
  • packages/plugin-beatsfoundation/tsconfig.json (1 hunks)
  • packages/plugin-beatsfoundation/tsup.config.ts (1 hunks)
✅ Files skipped from review due to trivial changes (9)
  • packages/plugin-beatsfoundation/src/actions/GetSongById/service.ts
  • packages/plugin-beatsfoundation/src/actions/GetSongById/validation.ts
  • packages/plugin-beatsfoundation/src/actions/GetSongById/types.ts
  • packages/plugin-beatsfoundation/src/actions/GetSongById/examples.ts
  • packages/plugin-beatsfoundation/src/actions/GetSongById/template.ts
  • packages/plugin-beatsfoundation/tsconfig.json
  • packages/plugin-beatsfoundation/tsup.config.ts
  • packages/plugin-beatsfoundation/package.json
  • packages/plugin-beatsfoundation/src/actions/GetSongs/examples.ts
🧰 Additional context used
🪛 Markdownlint (0.37.0)
packages/plugin-beatsfoundation/README.md

74-74: null
Tables should be surrounded by blank lines

(MD058, blanks-around-tables)

🔇 Additional comments (7)
packages/plugin-beatsfoundation/src/actions/CreateSong/types.ts (1)

4-8: Interface definition looks good

The CreateSongContent interface is well-defined with appropriate required and optional fields.

packages/plugin-beatsfoundation/src/actions/CreateSong/validation.ts (1)

7-11: Add length constraints for string fields

Consider adding maximum length constraints for string fields to prevent potential abuse.
[security]

-    if (typeof c.prompt !== "string" || c.prompt.length === 0) return false;
+    if (typeof c.prompt !== "string" || c.prompt.length === 0 || c.prompt.length > 1000) return false;
-    if (c.lyrics !== undefined && typeof c.lyrics !== "string") return false;
+    if (c.lyrics !== undefined && (typeof c.lyrics !== "string" || c.lyrics.length > 5000)) return false;
packages/plugin-beatsfoundation/src/environment.ts (1)

4-8: Strengthen API key validation

Add regex pattern validation for the API key format to catch invalid keys early.
[security]

     BEATSFOUNDATION_API_KEY: z
         .string()
-        .min(1, "BeatsFoundation API key is required"),
+        .min(1, "BeatsFoundation API key is required")
+        .regex(/^bf_[a-zA-Z0-9]{32}$/, "Invalid BeatsFoundation API key format"),
agent/src/index.ts (2)

55-55: LGTM!

The import statement is correctly placed among other plugin imports.


949-949: LGTM!

The plugin initialization follows the established pattern of conditional loading based on API key presence.

packages/plugin-beatsfoundation/README.md (2)

1-72: LGTM! Well-structured documentation.

The documentation is comprehensive and follows best practices with clear sections for installation, configuration, and usage examples.


102-102: Verify rate limit information.

The rate limit mentioned here (2 generations per hour) should be verified as it's a critical piece of information for users.

✅ Verification successful

Rate limit information is correctly documented

The rate limit of 2 generations per hour is consistently documented and enforced by the Beats Foundation API server, not locally in the plugin.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for rate limit information in the codebase
rg -i "rate.?limit" --type ts

Length of output: 9350


Script:

#!/bin/bash
# Search specifically in beatsfoundation plugin directory
fd . packages/plugin-beatsfoundation -t f -x rg -i "rate.?limit|generation.?limit|per.?hour" {}

Length of output: 341


Script:

#!/bin/bash
# Look for implementation files and configuration
fd . packages/plugin-beatsfoundation -e ts -e js -e json -x cat {}

Length of output: 18694

Comment on lines +50 to +54
const content = (await generateObjectDeprecated({
runtime,
context,
modelClass: ModelClass.SMALL,
})) as unknown as GetSongsContent;
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

Replace deprecated generateObjectDeprecated method

You're using a deprecated method generateObjectDeprecated. Update it to generateObject to ensure future compatibility.

Apply this diff:

-    const content = (await generateObjectDeprecated({
+    const content = (await generateObject({
         runtime,
         context,
         modelClass: ModelClass.SMALL,
     })) as unknown as GetSongsContent;

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines +3 to +11
export function isGetSongsContent(content: unknown): content is GetSongsContent {
if (!content || typeof content !== "object") return false;
const c = content as GetSongsContent;

if (c.limit !== undefined && (typeof c.limit !== "number" || c.limit < 0)) return false;
if (c.offset !== undefined && (typeof c.offset !== "number" || c.offset < 0)) return false;

return true;
}
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

Strengthen the validation with upper bounds and type assertions.

The validation should include an upper limit for the limit parameter and use type assertions more safely.

 export function isGetSongsContent(content: unknown): content is GetSongsContent {
     if (!content || typeof content !== "object") return false;
-    const c = content as GetSongsContent;
+    const c = content as Record<string, unknown>;
 
-    if (c.limit !== undefined && (typeof c.limit !== "number" || c.limit < 0)) return false;
-    if (c.offset !== undefined && (typeof c.offset !== "number" || c.offset < 0)) return false;
+    if (c.limit !== undefined && (
+        typeof c.limit !== "number" ||
+        c.limit < 0 ||
+        c.limit > 100
+    )) return false;
+    if (c.offset !== undefined && (
+        typeof c.offset !== "number" ||
+        c.offset < 0 ||
+        !Number.isInteger(c.offset)
+    )) return false;
 
     return true;
 }
📝 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
export function isGetSongsContent(content: unknown): content is GetSongsContent {
if (!content || typeof content !== "object") return false;
const c = content as GetSongsContent;
if (c.limit !== undefined && (typeof c.limit !== "number" || c.limit < 0)) return false;
if (c.offset !== undefined && (typeof c.offset !== "number" || c.offset < 0)) return false;
return true;
}
export function isGetSongsContent(content: unknown): content is GetSongsContent {
if (!content || typeof content !== "object") return false;
const c = content as Record<string, unknown>;
if (c.limit !== undefined && (
typeof c.limit !== "number" ||
c.limit < 0 ||
c.limit > 100
)) return false;
if (c.offset !== undefined && (
typeof c.offset !== "number" ||
c.offset < 0 ||
!Number.isInteger(c.offset)
)) return false;
return true;
}

Comment on lines +4 to +5
if (!content || typeof content !== "object") return false;
const c = content as CreateSongContent;
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

Avoid type casting and strengthen validation

The type casting with as could be unsafe. Consider using type narrowing instead.

-    if (!content || typeof content !== "object") return false;
-    const c = content as CreateSongContent;
+    if (!content || typeof content !== "object" || Array.isArray(content) || content === null) return false;
+    const c = content;
📝 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
if (!content || typeof content !== "object") return false;
const c = content as CreateSongContent;
if (!content || typeof content !== "object" || Array.isArray(content) || content === null) return false;
const c = content;

Comment on lines +8 to +21
try {
const response = await axios.post(
'https://www.beatsfoundation.com/api/songs',
content,
{
headers: {
Authorization: `Bearer ${apiKey}`,
'Content-Type': 'application/json',
},
timeout: 300000, // 5 minutes timeout for song generation
}
);
return response.data.song;
} catch (error: any) {
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 retry mechanism and request cancellation

For long-running song generation requests:

  1. Implement retry logic for transient failures
  2. Add request cancellation support
+import { CancelToken } from 'axios';
+import axiosRetry from 'axios-retry';

         createSong: async (content: CreateSongContent): Promise<Song> => {
+            const source = CancelToken.source();
             try {
+                const client = axios.create();
+                axiosRetry(client, { 
+                    retries: 3,
+                    retryDelay: axiosRetry.exponentialDelay,
+                    retryCondition: (error) => {
+                        return axiosRetry.isNetworkOrIdempotentRequestError(error) 
+                            || error.code === 'ECONNABORTED';
+                    }
+                });
-                const response = await axios.post(
+                const response = await client.post(
                     'https://www.beatsfoundation.com/api/songs',
                     content,
                     {
                         headers: {
                             Authorization: `Bearer ${apiKey}`,
                             'Content-Type': 'application/json',
                         },
                         timeout: 300000,
+                        cancelToken: source.token
                     }
                 );
📝 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
try {
const response = await axios.post(
'https://www.beatsfoundation.com/api/songs',
content,
{
headers: {
Authorization: `Bearer ${apiKey}`,
'Content-Type': 'application/json',
},
timeout: 300000, // 5 minutes timeout for song generation
}
);
return response.data.song;
} catch (error: any) {
import { CancelToken } from 'axios';
import axiosRetry from 'axios-retry';
try {
const source = CancelToken.source();
const client = axios.create();
axiosRetry(client, {
retries: 3,
retryDelay: axiosRetry.exponentialDelay,
retryCondition: (error) => {
return axiosRetry.isNetworkOrIdempotentRequestError(error)
|| error.code === 'ECONNABORTED';
}
});
const response = await client.post(
'https://www.beatsfoundation.com/api/songs',
content,
{
headers: {
Authorization: `Bearer ${apiKey}`,
'Content-Type': 'application/json',
},
timeout: 300000, // 5 minutes timeout for song generation
cancelToken: source.token
}
);
return response.data.song;
} catch (error: any) {

Comment on lines +22 to +23
return response.data.songs;
} catch (error: any) {
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

Include pagination metadata in response

The response should include pagination metadata for proper client-side handling.

+                interface PaginatedResponse {
+                    songs: Song[];
+                    pagination: {
+                        total: number;
+                        limit: number;
+                        offset: number;
+                    };
+                }
-                return response.data.songs;
+                const { songs, pagination } = response.data as PaginatedResponse;
+                return {
+                    data: songs,
+                    pagination
+                };

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines +65 to +98
try {
const song = await songService.getSongById(content.songId);
elizaLogger.success(
`Song retrieved successfully! ID: ${content.songId}, Title: ${song.title}`
);

if (callback) {
callback({
text: `Retrieved song: ${song.title}`,
content: song,
});
}

return true;
} catch (error: any) {
elizaLogger.error("Error in GET_SONG_BY_ID handler:", error);
if (callback) {
callback({
text: `Error fetching song: ${error.message}`,
content: { error: error.message },
});
}
return false;
}
} catch (error: any) {
elizaLogger.error("Error in GET_SONG_BY_ID handler:", error);
if (callback) {
callback({
text: `Error fetching song: ${error.message}`,
content: { error: error.message },
});
}
return false;
}
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

Refactor duplicate error handling

Extract common error handling logic to reduce duplication.

+            const handleError = (error: any, callback?: HandlerCallback) => {
+                elizaLogger.error("Error in GET_SONG_BY_ID handler:", error);
+                if (callback) {
+                    callback({
+                        text: `Error fetching song: ${error.message}`,
+                        content: { error: error.message },
+                    });
+                }
+                return false;
+            };

             try {
                 const song = await songService.getSongById(content.songId);
                 // ... success handling ...
             } catch (error: any) {
-                elizaLogger.error("Error in GET_SONG_BY_ID handler:", error);
-                if (callback) {
-                    callback({
-                        text: `Error fetching song: ${error.message}`,
-                        content: { error: error.message },
-                    });
-                }
-                return false;
+                return handleError(error, callback);
             }
         } catch (error: any) {
-            elizaLogger.error("Error in GET_SONG_BY_ID handler:", error);
-            if (callback) {
-                callback({
-                    text: `Error fetching song: ${error.message}`,
-                    content: { error: error.message },
-                });
-            }
-            return false;
+            return handleError(error, callback);
         }
📝 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
try {
const song = await songService.getSongById(content.songId);
elizaLogger.success(
`Song retrieved successfully! ID: ${content.songId}, Title: ${song.title}`
);
if (callback) {
callback({
text: `Retrieved song: ${song.title}`,
content: song,
});
}
return true;
} catch (error: any) {
elizaLogger.error("Error in GET_SONG_BY_ID handler:", error);
if (callback) {
callback({
text: `Error fetching song: ${error.message}`,
content: { error: error.message },
});
}
return false;
}
} catch (error: any) {
elizaLogger.error("Error in GET_SONG_BY_ID handler:", error);
if (callback) {
callback({
text: `Error fetching song: ${error.message}`,
content: { error: error.message },
});
}
return false;
}
const handleError = (error: any, callback?: HandlerCallback) => {
elizaLogger.error("Error in GET_SONG_BY_ID handler:", error);
if (callback) {
callback({
text: `Error fetching song: ${error.message}`,
content: { error: error.message },
});
}
return false;
};
try {
const song = await songService.getSongById(content.songId);
elizaLogger.success(
`Song retrieved successfully! ID: ${content.songId}, Title: ${song.title}`
);
if (callback) {
callback({
text: `Retrieved song: ${song.title}`,
content: song,
});
}
return true;
} catch (error: any) {
return handleError(error, callback);
}
} catch (error: any) {
return handleError(error, callback);
}

Comment on lines +74 to +77
content: {
song,
request: content
},
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

Sanitize sensitive information in callback response

The callback includes the full request content which might contain sensitive information.

                         content: {
                             song,
-                            request: content
+                            request: {
+                                title: content.title,
+                                // Add other non-sensitive fields as needed
+                            }
                         },
📝 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
content: {
song,
request: content
},
content: {
song,
request: {
title: content.title,
// Add other non-sensitive fields as needed
}
},

Comment on lines +56 to +59
// Validate content
if (!isCreateSongContent(content)) {
throw new Error("Invalid song creation content");
}
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 input sanitization

Validate and sanitize input content before sending to the API.

+            const sanitizeContent = (content: CreateSongContent): CreateSongContent => {
+                return {
+                    ...content,
+                    title: content.title.trim(),
+                    // Add other sanitization as needed
+                };
+            };

             // Validate content
             if (!isCreateSongContent(content)) {
                 throw new Error("Invalid song creation content");
             }
+            const sanitizedContent = sanitizeContent(content);

Committable suggestion skipped: line range outside the PR's diff.

@wtfsayo
Copy link
Member

wtfsayo commented Jan 19, 2025

@beatsfoundation can you implement ai feedback

@wtfsayo
Copy link
Member

wtfsayo commented Jan 20, 2025

closing in favour of #2552

@wtfsayo wtfsayo closed this Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants