Skip to content

Conversation

@jztmanyl
Copy link

No description provided.

@jztmanyl jztmanyl requested a review from kibertoad as a code owner August 20, 2025 08:28
@coderabbitai
Copy link

coderabbitai bot commented Aug 20, 2025

📝 Walkthrough

Summary by CodeRabbit

  • New Features
    • Added LanguageWire as a supported translation engine.
    • New settings to configure LanguageWire:
    • Extended engine selection to include “languagewire” in i18n-ally.translate.engines.
    • Supports automatic source language detection when available from the service.
    • Returns translated text and preserves the full response for inspection.

Walkthrough

Adds LanguageWire as a new translation engine. Updates package.json to include the engine enum value and new config keys for API key and API root. Extends Config with getters for languageWireApiKey and languageWireApiRoot. Introduces src/translators/engines/languagewire.ts implementing translate() via axios to POST to {apiRoot}/translations/text and transform responses. Registers the engine in src/translators/index.ts and exports LanguageWireTranslate.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as Caller
  participant T as Translator
  participant LW as LanguageWireTranslate
  participant API as LanguageWire API

  U->>T: translate({ engine: "languagewire", text, from, to })
  T->>LW: translate(options)
  Note over LW: Resolve apiRoot (config/override), read apiKey
  LW->>API: POST /translations/text<br/>headers: Bearer apiKey<br/>body: sourceText, targetLanguage,<br/>[sourceLanguage]
  API-->>LW: 200 OK<br/>{ translation, detectedSourceLanguage }
  LW->>LW: transform(response, options)
  LW-->>T: TranslateResult
  T-->>U: TranslateResult

  alt Error
    API-->>LW: 4xx/5xx error
    LW-->>T: throw error
    T-->>U: propagate error
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.


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.

❤️ Share
🪧 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.
    • 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.
  • 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 the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

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

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • 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

@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: 2

🧹 Nitpick comments (5)
src/core/Config.ts (1)

575-581: New LanguageWire config getters are correctly wired.

The getters align with package.json keys (i18n-ally.translate.languagewire.apiKey/apiRoot). Consider normalizing apiRoot here (strip trailing slash) to keep callers simpler, but this is optional as the engine already handles it.

-  static get languageWireApiRoot() {
-    return this.getConfig<string | null | undefined>('translate.languagewire.apiRoot')
-  }
+  static get languageWireApiRoot() {
+    const root = this.getConfig<string | null | undefined>('translate.languagewire.apiRoot')
+    return root ? root.replace(/\/$/, '') : root
+  }
package.json (1)

1045-1047: Engine enum updated — verify documentation/UI strings show “LanguageWire”.

The new engine value is added. Ensure any user-visible docs/help that list engines also mention languagewire.

src/translators/engines/languagewire.ts (3)

13-24: Only include sourceLanguage when explicitly provided (and not "auto").

As written, undefined !== 'auto' evaluates true and spreads { sourceLanguage: undefined }, which is then serialized away but is brittle. Be explicit.

       {
         sourceText: options.text,
         reference: {
           source: 'USER',
         },
-        targetLanguage: options.to,
-        // If source language is specified, include it
-        ...(options.from !== 'auto' && { sourceLanguage: options.from }),
+        targetLanguage: options.to,
+        // If source language is specified, include it
+        ...(options.from && options.from !== 'auto' ? { sourceLanguage: options.from } : {}),
       },

25-31: Add a request timeout and optionally set baseURL.

Without a timeout, requests can hang indefinitely. A modest timeout improves resilience.

       {
         headers: {
           'Content-Type': 'application/json',
           'Accept': 'application/json',
           'Authorization': `Bearer ${apiKey}`,
         },
+        timeout: 15000,
       },

5-7: Optional: centralize apiRoot normalization.

You could pre-normalize apiRoot in the class property to avoid repeating replace(//$/, '') elsewhere. Not a blocker.

 export default class LanguageWireTranslate extends TranslateEngine {
-  apiRoot = 'https://lwt.languagewire.com/p/api/v1'
+  apiRoot = 'https://lwt.languagewire.com/p/api/v1' // no trailing slash
📜 Review details

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

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 4c504c9 and 2eb14c4.

📒 Files selected for processing (4)
  • package.json (2 hunks)
  • src/core/Config.ts (2 hunks)
  • src/translators/engines/languagewire.ts (1 hunks)
  • src/translators/index.ts (3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/translators/index.ts (1)
src/translators/engines/languagewire.ts (1)
  • LanguageWireTranslate (5-56)
src/translators/engines/languagewire.ts (2)
src/translators/engines/base.ts (2)
  • TranslateOptions (6-10)
  • TranslateResult (12-21)
src/core/Config.ts (1)
  • Config (13-598)
🔇 Additional comments (4)
src/core/Config.ts (1)

183-185: LGTM on sortLocale getter.

Non-functional formatting change only. No concerns.

package.json (1)

1109-1118: Verify i18n translation entries for new config descriptions

I wasn’t able to locate any VS Code NLS or locale JSON files containing the keys referenced by your new descriptions. Please confirm where your extension’s translation files live (e.g. package.nls.json, package.nls.<lang>.json, or under a locales/ folder) and ensure they include entries for:

config.languagewire_api_key
config.languagewire_api_root

For example, in package.nls.json:

{
  "config.languagewire_api_key": "LanguageWire API Key",
  "config.languagewire_api_root": "LanguageWire API Root URL"
}
src/translators/index.ts (1)

8-8: Engine registration and exports look correct.

Import, engine map registration under 'languagewire', and re-export are consistent with other engines.

Also applies to: 18-19, 35-36

src/translators/engines/languagewire.ts (1)

8-34: Optional: map VS Code locales to LanguageWire expected codes.

If LanguageWire expects MMT codes differing from BCP47, introduce a mapping (similar to other engines) to avoid 400s on uncommon locales. If LanguageWire accepts BCP47 as-is, ignore this.

Would you like me to add a minimal mapper (BCP47 -> MMT codes) after confirming LanguageWire’s expected codes?

Comment on lines +8 to +12
async translate(options: TranslateOptions) {
const apiKey = Config.languageWireApiKey
let apiRoot = this.apiRoot
if (Config.languageWireApiRoot) apiRoot = Config.languageWireApiRoot.replace(/\/$/, '')

Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Guard against missing API key and undefined target language.

Calling the API without a key or target language will fail with a 401/validation error and yields a poor UX. Throw early with a clear message.

Apply:

   async translate(options: TranslateOptions) {
     const apiKey = Config.languageWireApiKey
     let apiRoot = this.apiRoot
-    if (Config.languageWireApiRoot) apiRoot = Config.languageWireApiRoot.replace(/\/$/, '')
+    if (Config.languageWireApiRoot) apiRoot = Config.languageWireApiRoot.replace(/\/$/, '')
+
+    if (!apiKey) {
+      throw new Error('LanguageWire: missing API key. Configure i18n-ally.translate.languagewire.apiKey in settings.')
+    }
+    if (!options.to) {
+      throw new Error('LanguageWire: target language "to" is required.')
+    }
📝 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 translate(options: TranslateOptions) {
const apiKey = Config.languageWireApiKey
let apiRoot = this.apiRoot
if (Config.languageWireApiRoot) apiRoot = Config.languageWireApiRoot.replace(/\/$/, '')
async translate(options: TranslateOptions) {
const apiKey = Config.languageWireApiKey
let apiRoot = this.apiRoot
if (Config.languageWireApiRoot) apiRoot = Config.languageWireApiRoot.replace(/\/$/, '')
if (!apiKey) {
throw new Error('LanguageWire: missing API key. Configure i18n-ally.translate.languagewire.apiKey in settings.')
}
if (!options.to) {
throw new Error('LanguageWire: target language "to" is required.')
}
// …rest of implementation…
}
🤖 Prompt for AI Agents
In src/translators/engines/languagewire.ts around lines 8 to 12, add early
guards to validate Config.languageWireApiKey and the requested target language
before making the API call: if the API key is missing/empty throw a clear Error
like "LanguageWire API key is missing: set Config.languageWireApiKey", and if
the TranslateOptions does not include a target language (e.g.,
options.targetLanguage or options.target) throw "Missing target language in
translate options"; do these checks immediately after reading apiKey and
options, so the function fails fast with descriptive messages rather than
letting the request go to the API and return a 401/validation error.

Comment on lines 36 to 55
transform(response: any, options: TranslateOptions): TranslateResult {
const { text, from = 'auto', to = 'auto' } = options

// Get the translated text from the response
const translatedText = response.data.translation?.trim()

// Determine the detected source language
const detectedSource = response.data.detectedSourceLanguage?.mmtCode || from

const r: TranslateResult = {
text,
to,
from: detectedSource,
response,
result: translatedText ? [translatedText] : undefined,
linkToResult: '',
}

return r
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Harden transform against non-string translations and missing fields.

If the API returns a non-string translation, translation?.trim() will throw. Also, keep result consistently typed.

   transform(response: any, options: TranslateOptions): TranslateResult {
     const { text, from = 'auto', to = 'auto' } = options
 
-    // Get the translated text from the response
-    const translatedText = response.data.translation?.trim()
+    // Get the translated text from the response
+    const raw = response?.data?.translation
+    const translatedText = typeof raw === 'string' ? raw.trim() : undefined
 
-    // Determine the detected source language
-    const detectedSource = response.data.detectedSourceLanguage?.mmtCode || from
+    // Determine the detected source language
+    const detectedSource = response?.data?.detectedSourceLanguage?.mmtCode || from
 
     const r: TranslateResult = {
       text,
       to,
       from: detectedSource,
       response,
       result: translatedText ? [translatedText] : undefined,
       linkToResult: '',
     }
 
     return r
   }
📝 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
transform(response: any, options: TranslateOptions): TranslateResult {
const { text, from = 'auto', to = 'auto' } = options
// Get the translated text from the response
const translatedText = response.data.translation?.trim()
// Determine the detected source language
const detectedSource = response.data.detectedSourceLanguage?.mmtCode || from
const r: TranslateResult = {
text,
to,
from: detectedSource,
response,
result: translatedText ? [translatedText] : undefined,
linkToResult: '',
}
return r
}
transform(response: any, options: TranslateOptions): TranslateResult {
const { text, from = 'auto', to = 'auto' } = options
// Get the translated text from the response
const raw = response?.data?.translation
const translatedText = typeof raw === 'string' ? raw.trim() : undefined
// Determine the detected source language
const detectedSource = response?.data?.detectedSourceLanguage?.mmtCode || from
const r: TranslateResult = {
text,
to,
from: detectedSource,
response,
result: translatedText ? [translatedText] : undefined,
linkToResult: '',
}
return r
}
🤖 Prompt for AI Agents
src/translators/engines/languagewire.ts around lines 36 to 55, the transform()
assumes response.data.translation is a string and calls .trim(), which will
throw for non-strings or when fields are missing; also response.data may be
undefined and result should remain a string[] | undefined. Fix by defensively
reading nested fields (guard response && response.data), check typeof
translation === 'string' before trimming, otherwise coerce to
String(response.data.translation) only if not null/undefined or skip; set
translatedText to the trimmed string or undefined; ensure result is either an
array of strings when a valid translation exists or undefined; similarly guard
detectedSourceLanguage access and default to options.from.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant