Feat/translation service #2615
Feat/translation service #2615
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hi @gregslag! 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!
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request introduces translation capabilities to the platform by adding a new Changes
Possibly related PRs
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
packages/plugin-node/src/services/translation.ts (2)
88-88: Make the OpenAI model configurableThe OpenAI model is hardcoded as
"gpt-4o-mini". Making the model configurable enhances flexibility and ease of updates.Proposed change:
- model: "gpt-4o-mini", + model: this.runtime.getSetting("OPENAI_MODEL") || "gpt-4",
18-62: Simplify provider initialization logicThe
initializemethod contains nested conditionals and repeated code for initializing the translation provider. Refactoring this logic can improve readability and reduce duplication.Consider consolidating the provider selection logic into a more streamlined structure.
packages/core/src/types.ts (1)
1627-1629: Well-structured enum for future extensibility!Consider adding JSDoc comments to document supported providers and their requirements.
+/** + * Available translation providers + * @property OpenAI - Uses OpenAI's API for translation + */ export enum TranslationProvider { OpenAI = "openai", }packages/plugin-node/__tests__/translation.test.ts (2)
42-58: Consider improving type safety of mock runtime.The
as unknown as IAgentRuntimetype assertion could be replaced with a proper mock implementation.- } as unknown as IAgentRuntime; + mockRuntime = { + character: { + settings: { + translation: TranslationProvider.OpenAI, + }, + }, + getSetting: vi.fn().mockImplementation((key: string) => ({ + OPENAI_API_KEY: "test-openai-key", + })[key]), + } satisfies Partial<IAgentRuntime> as IAgentRuntime;
60-129: Add tests for error handling scenarios.Consider adding test cases for:
- Rate limiting errors from OpenAI
- Network timeouts
- Invalid API responses
Example test case:
it("should handle rate limiting errors from OpenAI", async () => { const rateLimitError = new Error("Rate limit exceeded"); rateLimitError.name = "RateLimitError"; vi.mocked(OpenAI).mockImplementationOnce(() => ({ chat: { completions: { create: vi.fn().mockRejectedValue(rateLimitError) } } })); await service.initialize(mockRuntime); const result = await service.translate("Hello, world!", "French"); expect(result).toBeNull(); expect(elizaLogger.error).toHaveBeenCalledWith( expect.stringContaining("Rate limit") ); });packages/client-discord/src/index.ts (1)
150-167: Enhance command descriptions for better user experience.The command descriptions could be more informative about expected inputs.
{ name: "translate", - description: "Translate text", + description: "Translate text to one or more languages", options: [ { name: "text", type: 3, // STRING type - description: "Text for translation", + description: "The text you want to translate", required: true, }, { name: "to", type: 3, // STRING type - description: "Target language(s) for translation", + description: "Target language(s) (e.g., 'French' or 'Spanish, German')", required: true, }, ], }.env.example (1)
176-178: Consider enhancing the configuration documentation.While the default OpenAI provider is specified, consider adding:
- Required API configuration for OpenAI
- Future supported providers (if planned)
docs/docs/packages/plugins.md (1)
97-97: LGTM! Added TranslationService documentation.The addition of TranslationService to the Node Plugin services list is appropriate.
Consider adding a brief description of the translation service's capabilities and supported languages.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
.env.example(1 hunks)docs/docs/packages/clients.md(1 hunks)docs/docs/packages/plugins.md(3 hunks)packages/client-discord/src/index.ts(2 hunks)packages/client-discord/src/messages.ts(52 hunks)packages/core/src/types.ts(4 hunks)packages/plugin-node/__tests__/translation.test.ts(1 hunks)packages/plugin-node/package.json(2 hunks)packages/plugin-node/src/index.ts(2 hunks)packages/plugin-node/src/services/index.ts(2 hunks)packages/plugin-node/src/services/translation.ts(1 hunks)
🔇 Additional comments (12)
packages/plugin-node/src/services/index.ts (2)
9-9: ImportingTranslationServiceAdding
TranslationServiceto the imports correctly integrates the new service.
20-20: ExportingTranslationServiceExporting
TranslationServicemakes it accessible for use in other modules.packages/plugin-node/src/index.ts (2)
15-15: ImportingTranslationServiceIncluding
TranslationServicein the imports enhances the plugin's functionality.
33-33: RegisteringTranslationServicein servicesAdding
TranslationServiceto theservicesarray registers it properly with the plugin.packages/core/src/types.ts (2)
1354-1356: Clean interface design!The
ITranslationServiceinterface is well-defined with clear parameters and proper error handling through nullable return type.
1492-1492: LGTM! Maintains enum consistency.The TRANSLATION service type is properly placed and follows the established naming convention.
packages/plugin-node/__tests__/translation.test.ts (1)
6-36: Well-structured mock setup!The mocks are properly isolated and provide consistent test data.
packages/client-discord/src/index.ts (1)
405-407: LGTM! Clean integration of translate command.The command handling follows the established pattern in the switch statement.
packages/plugin-node/package.json (2)
82-83: LGTM! Adding vitest for testing.The addition of vitest as a dev dependency is appropriate for the new translation service implementation.
88-90: LGTM! Added lint and test scripts.The new scripts enhance the development workflow:
lintfor code qualitytestfor running vitest testsdocs/docs/packages/clients.md (1)
91-91: LGTM! Added translation feature documentation.The addition of "Message translations" to the Discord client features section appropriately documents the new capability.
docs/docs/packages/plugins.md (1)
671-674: LGTM! Added TEE_MARLIN_ATTESTATION_ENDPOINT configuration.The documentation clearly specifies the default endpoint and its optional nature.
🧰 Tools
🪛 Markdownlint (0.37.0)
672-672: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
| this.openai = new OpenAI({ apiKey: openaiKey }); | ||
| chosenProvider = TranslationProvider.OpenAI; | ||
| } else { | ||
| elizaLogger.error("TranslationService unable"); |
There was a problem hiding this comment.
Incomplete error message in logger
The error message at line 57 is incomplete and does not provide sufficient information. It simply states "TranslationService unable" without specifying the issue.
Proposed fix:
- elizaLogger.error("TranslationService unable");
+ elizaLogger.error("TranslationService unable to initialize: No translation provider available");📝 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.
| elizaLogger.error("TranslationService unable"); | |
| elizaLogger.error("TranslationService unable to initialize: No translation provider available"); |
| const completion = await this.openai!.chat.completions.create({ | ||
| model: "gpt-4o-mini", | ||
| messages: [ | ||
| { | ||
| role: "system", | ||
| content: | ||
| 'You are a language translator and you will be provided with a text to translate and one or more target languages. Return the result of the translation in the target language(s) organized in a list with the target language and the translation separated by a colon. Example: "English: Hello, world!"', | ||
| }, | ||
| { | ||
| role: "user", | ||
| content: `Translate the following text into ${targetLanguages}: ${text}`, | ||
| }, | ||
| ], | ||
| temperature: 1, | ||
| max_tokens: 256, | ||
| top_p: 1, | ||
| frequency_penalty: 0, | ||
| presence_penalty: 0, | ||
| }); | ||
|
|
||
| return completion.choices[0].message.content; |
There was a problem hiding this comment.
Add error handling for OpenAI API call
The translateWithAI method lacks error handling for the OpenAI API call. If the API request fails, it could result in an unhandled exception.
Proposed fix:
Wrap the API call in a try-catch block to handle exceptions gracefully:
+ try {
const completion = await this.openai!.chat.completions.create({
model: "gpt-4o-mini",
messages: [
{
role: "system",
content:
'You are a language translator and you will be provided with a text to translate and one or more target languages. Return the result of the translation in the target language(s) organized in a list with the target language and the translation separated by a colon. Example: "English: Hello, world!"',
},
{
role: "user",
content: `Translate the following text into ${targetLanguages}: ${text}`,
},
],
temperature: 1,
max_tokens: 256,
top_p: 1,
frequency_penalty: 0,
presence_penalty: 0,
});
return completion.choices[0].message.content;
+ } catch (error) {
+ elizaLogger.error('Error during translation with OpenAI:', error);
+ return null;
+ }Committable suggestion skipped: line range outside the PR's diff.
| async handleTranslateCommand(interaction: any) { | ||
| try { | ||
| // Defer the reply immediately to prevent interaction timeout | ||
| await interaction.deferReply(); | ||
|
|
||
| const text = interaction.options.get("text")?.value as string; | ||
| if (!text) { | ||
| await interaction.editReply( | ||
| "Please provide a text to translate.", | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| const targetLanguages = interaction.options.get("to") | ||
| ?.value as string; | ||
| if (!targetLanguages) { | ||
| await interaction.editReply( | ||
| "Please provide at least one target language to translate.", | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| const translatedText = await this.runtime | ||
| .getService<ITranslationService>(ServiceType.TRANSLATION) | ||
| .translate(text, targetLanguages); | ||
|
|
||
| await interaction.editReply(translatedText); | ||
| } catch (error) { | ||
| console.error("Error on text translation:", error); | ||
| await interaction | ||
| .editReply("Failed to translate text.") | ||
| .catch(console.error); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling and input validation.
The implementation needs more robust error handling and input validation.
async handleTranslateCommand(interaction: any) {
try {
await interaction.deferReply();
const text = interaction.options.get("text")?.value as string;
- if (!text) {
+ if (!text?.trim()) {
await interaction.editReply(
- "Please provide a text to translate."
+ "Please provide non-empty text to translate."
);
return;
}
const targetLanguages = interaction.options.get("to")?.value as string;
- if (!targetLanguages) {
+ if (!targetLanguages?.trim()) {
await interaction.editReply(
- "Please provide at least one target language to translate."
+ "Please provide at least one valid target language (e.g., 'French' or 'Spanish, German')."
);
return;
}
+ // Sanitize inputs
+ const sanitizedText = text.trim();
+ const sanitizedLanguages = targetLanguages
+ .split(',')
+ .map(lang => lang.trim())
+ .filter(Boolean)
+ .join(', ');
+
const translatedText = await this.runtime
.getService<ITranslationService>(ServiceType.TRANSLATION)
- .translate(text, targetLanguages);
+ .translate(sanitizedText, sanitizedLanguages);
+
+ if (!translatedText) {
+ await interaction.editReply("Translation service is currently unavailable. Please try again later.");
+ return;
+ }
await interaction.editReply(translatedText);
} catch (error) {
console.error("Error on text translation:", error);
await interaction
- .editReply("Failed to translate text.")
+ .editReply("An error occurred during translation. Please try again later.")
.catch(console.error);
}
}📝 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.
| async handleTranslateCommand(interaction: any) { | |
| try { | |
| // Defer the reply immediately to prevent interaction timeout | |
| await interaction.deferReply(); | |
| const text = interaction.options.get("text")?.value as string; | |
| if (!text) { | |
| await interaction.editReply( | |
| "Please provide a text to translate.", | |
| ); | |
| return; | |
| } | |
| const targetLanguages = interaction.options.get("to") | |
| ?.value as string; | |
| if (!targetLanguages) { | |
| await interaction.editReply( | |
| "Please provide at least one target language to translate.", | |
| ); | |
| return; | |
| } | |
| const translatedText = await this.runtime | |
| .getService<ITranslationService>(ServiceType.TRANSLATION) | |
| .translate(text, targetLanguages); | |
| await interaction.editReply(translatedText); | |
| } catch (error) { | |
| console.error("Error on text translation:", error); | |
| await interaction | |
| .editReply("Failed to translate text.") | |
| .catch(console.error); | |
| } | |
| } | |
| async handleTranslateCommand(interaction: any) { | |
| try { | |
| // Defer the reply immediately to prevent interaction timeout | |
| await interaction.deferReply(); | |
| const text = interaction.options.get("text")?.value as string; | |
| if (!text?.trim()) { | |
| await interaction.editReply( | |
| "Please provide non-empty text to translate." | |
| ); | |
| return; | |
| } | |
| const targetLanguages = interaction.options.get("to")?.value as string; | |
| if (!targetLanguages?.trim()) { | |
| await interaction.editReply( | |
| "Please provide at least one valid target language (e.g., 'French' or 'Spanish, German')." | |
| ); | |
| return; | |
| } | |
| // Sanitize inputs | |
| const sanitizedText = text.trim(); | |
| const sanitizedLanguages = targetLanguages | |
| .split(',') | |
| .map(lang => lang.trim()) | |
| .filter(Boolean) | |
| .join(', '); | |
| const translatedText = await this.runtime | |
| .getService<ITranslationService>(ServiceType.TRANSLATION) | |
| .translate(sanitizedText, sanitizedLanguages); | |
| if (!translatedText) { | |
| await interaction.editReply("Translation service is currently unavailable. Please try again later."); | |
| return; | |
| } | |
| await interaction.editReply(translatedText); | |
| } catch (error) { | |
| console.error("Error on text translation:", error); | |
| await interaction | |
| .editReply("An error occurred during translation. Please try again later.") | |
| .catch(console.error); | |
| } | |
| } |
Relates to
N/A
Risks
Low
Background
What does this PR do?
plugin-nodeclient-discordand use the translation serviceWhat kind of change is this?
Features:
plugin-nodewith one provider (OpenAI) implementedclient-discordfor text translationsThe idea is to make available a text translation service using IA or other providers (e.g.
DeepL,Google Cloud Translation). At this moment I only implement a slash command for Discord, but it can be used for other clients integrations.Documentation changes needed?
Testing
Where should a reviewer start?
TRANSLATION_PROVIDERENV variable in .env (for now, justopenaiis accepted)client-discordDetailed testing steps
/translateSummary by CodeRabbit
Release Notes: Translation Service and Discord Integration
New Features
/translatefor translating messagesConfiguration Updates
Documentation
Testing
These release notes highlight the key improvements in translation services and Discord bot functionality, focusing on the end-user experience and new capabilities.