Skip to content

Conversation

@jokester
Copy link
Member

@jokester jokester commented Sep 17, 2025

Summary by CodeRabbit

  • New Features

    • AI Translate: new button in file list to configure an LLM, test connectivity, and run batch image translations with per-file progress, status messages, and cancel support.
  • Refactor

    • Unified AI translation UI and workflow; replaced older companion integrations with a single LLM-driven flow and updated localization keys for the new dialogs and messages.
  • Chores

    • Added AI-related dependencies and a dev-only tweak to skip sign-out cleanup when token is empty.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 17, 2025

Walkthrough

Adds an LLM-driven image translation flow: new llm_preprocess service, UI hook and modals for config + batch translation, FileList wiring, package dependency updates, removal of legacy Moeflow/MIT/multimodal/packager modules, locale restructuring, and a dev-only saga early-return tweak.

Changes

Cohort / File(s) Summary
Package deps
package.json
Added dependencies: @xsai-ext/providers-cloud, @xsai/generate-object, xsai, zod, zod-to-json-schema.
LLM preprocessing service (new)
src/services/ai/llm_preprocess.ts
New LLMConf type, llmPresets, zod FilePreprocessResult schema/type, and exported testModel and llmTranslateImage (includes img->dataURL helper and provider-specific workarounds).
AI UI: hook & modals (new)
src/components/ai/index.tsx, src/components/ai/ModelConfigForm.tsx, src/components/ai/BatchTranslateModal.tsx
New useAiTranslate hook returning a TranslatorApi and ModalHandle type; ModelConfigForm component for LLM config; BatchTranslateModalContent for concurrent batch translation and per-file state and persistence.
FileList integration
src/components/project/FileList.tsx
Wired AI translate button to useAiTranslate, passes selected files and target, mounts modal holder, and invokes aiTranslateApi.start on click; i18n key updates for label/tooltip.
Removed legacy AI / packaging modules
src/components/project/FileListAiTranslate.tsx, src/services/ai/use_moeflow_companion.ts, src/services/ai/mit_preprocess.ts, src/services/ai/multimodal_recognize.ts, src/services/ai/TranslateCompanion.tsx, src/services/labelplus_packager.ts
Deleted Moeflow companion hook/UI, MIT preprocess, multimodal stub, OCR/translate demo, and LabelPlus packager along with their exported types/functions.
Locale keys & generator
src/locales/messages.yaml, src/locales/en.json, src/locales/zh-cn.json, scripts/generate-locale-json.ts
Replaced flat AI translate keys with structured fileList.aiTranslate subtree (button, config modal, working modal, per-file messages); updated messages extractor to produce pathed keys and count occurrences; removed old flat keys.
Saga dev tweak
src/store/user/sagas.ts
getUserInfoAsync adds a development-only early return when token is empty to skip cleanup in dev mode.
Minor doc comment
src/components/icon/index.ts
Inserted an empty JSDoc-style comment above export (no API change).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant FileList
  participant Hook as useAiTranslate
  participant Config as ModelConfigForm
  participant Batch as BatchTranslateModal
  participant LLM as llm_preprocess
  participant API

  User->>FileList: Click "AI Translate"
  FileList->>Hook: start(onFileSaved)
  Hook->>Config: open config modal
  User->>Config: provide LLMConf + confirm
  Hook->>Batch: open batch modal(files, target, LLMConf)
  loop per file (concurrency-limited)
    Batch->>API: getFile metadata
    API-->>Batch: metadata
    alt needs translation
      Batch->>API: fetch image blob
      API-->>Batch: blob
      Batch->>LLM: llmTranslateImage(conf, target, blob)
      LLM-->>Batch: FilePreprocessResult (text blocks)
      alt texts found
        Batch->>API: create Source(s) & Translation(s)
        API-->>Batch: saved
      else
        Batch-->>Batch: mark noTextDetected
      end
    else
      Batch-->>Batch: mark skip
    end
  end
  Batch-->>Hook: done
  Hook-->>FileList: invoke callbacks
Loading
sequenceDiagram
  participant Batch
  participant FileLimiter
  participant ApiLimiter
  participant LLM
  participant API

  Batch->>FileLimiter: enqueue(file)
  FileLimiter->>ApiLimiter: api.getFile()
  API-->>ApiLimiter: metadata
  FileLimiter->>ApiLimiter: fetch image blob
  API-->>FileLimiter: blob
  FileLimiter->>LLM: llmTranslateImage(blob, conf)
  LLM-->>FileLimiter: textBlocks
  alt textBlocks.length > 0
    loop per textBlock (limited via ApiLimiter)
      FileLimiter->>ApiLimiter: createSource -> createTranslation
      API-->>FileLimiter: ok
    end
  else
    FileLimiter-->>Batch: mark noTextDetected
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

I twitch my ears at modal lights,
I nibble presets through the nights;
I hop through files in tidy queues,
With zod and blobs and JSON news.
Old crates closed—new carrots gleam, translate and dream. 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Add multimodal auto translate" succinctly and accurately summarizes the primary change set: introducing multimodal LLM-based auto-translation features (new AI components, services, hooks, and locales) and related removals of older companion/preprocess modules. It is concise, focused on the main feature, and clear enough for a reviewer scanning PR history.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch add-multimodal-auto-translate

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4df517e and f6e1139.

📒 Files selected for processing (1)
  • src/store/user/sagas.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/store/user/sagas.ts

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

Comment @coderabbitai help to get the list of available commands and usage tips.

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/store/user/sagas.ts (1)

12-31: Restore Authorization on new Axios instances or preserve the singleton across HMR

instanceP in src/apis/index.ts is created via lazyThenable(axios.create(...)) and does not restore Authorization on creation; the dev early-return in src/store/user/sagas.ts (token === '' → return) can leave a hot-reloaded axios instance without Authorization while cookie/store still indicate logged-in, causing transient 401s.

  • Fix option 1 (recommended): set the header when creating the instance in src/apis/index.ts by reading the token (src/utils/cookie.ts:getToken) and applying instance.defaults.headers.common['Authorization'] if present.
  • Fix option 2: persist the axios instance across HMR (import.meta.hot.accept or attach instanceP to a global) so headers survive reloads.
  • Also remove/rework the dev early-return in src/store/user/sagas.ts — don’t rely on it to mask HMR; ensure new instances always re-read and apply the current token.
🧹 Nitpick comments (21)
src/store/user/sagas.ts (1)

13-21: Broaden the “empty token” check.

Guard null/undefined/whitespace to avoid accidental cleanup on odd payloads.

-  if (token === '') {
+  const isEmptyToken = token == null || token.trim?.() === '';
+  if (isEmptyToken) {
package.json (3)

13-15: Pin pre-release versions to exact to reduce churn.

Caret on pre-release (^0.4.0-beta.2) can float to future betas unexpectedly. Prefer exact pins during rollout, then relax later.

-    "@xsai-ext/providers-cloud": "^0.4.0-beta.2",
-    "xsai": "^0.4.0-beta.2",
+    "@xsai-ext/providers-cloud": "0.4.0-beta.2",
+    "xsai": "0.4.0-beta.2",

Also applies to: 49-51


13-15: Drop unused @xsai/generate-object from package.json

No imports of '@xsai/generate-object' found; code imports generateObject/UserMessage from 'xsai' (src/services/ai/llm_preprocess.ts, src/components/ai/BatchTranslateModal.tsx). Remove the "@xsai/generate-object" entry in package.json (package.json:14).


48-48: Confirm uuid upgrade compatibility

package.json pins "uuid": "^7.0.3"; code uses named import import { v4 as uuidv4 } from 'uuid' in src/store/source/sagas.ts (import at line 13, call at line 85). If you plan to upgrade to uuid v9+ adjust imports/usage or pin to v7 to avoid runtime breakage.

src/services/ai/llm_preprocess.ts (3)

31-32: zod: use .describe() not { message } on number schema.

z.number({ message: ... }) doesn’t set a description. Use .describe(...).

-  imageW: z.number({ message: 'the width of the image in PX' }),
-  imageH: z.number({ message: 'the height of the image in PX' }),
+  imageW: z.number().describe('the width of the image in PX'),
+  imageH: z.number().describe('the height of the image in PX'),

55-59: testModel is a stub; implement a real connectivity check.

Return value is always “worked: true”, which can mislead the UI.

 export async function testModel(
   modelConf: LLMConf,
 ): Promise<{ worked: boolean; message: string }> {
-  return { worked: true, message: 'test model worked' };
+  try {
+    const schema = z.object({ ok: z.literal(true) });
+    const res = await generateObject({
+      messages: [{ role: 'user', content: [{ type: 'text', text: 'ping' }] }],
+      schema,
+      baseURL: modelConf.baseUrl,
+      model: modelConf.model,
+      apiKey: modelConf.apiKey,
+    });
+    const ok = res.object?.ok === true;
+    return { worked: ok, message: ok ? 'ok' : 'unexpected response' };
+  } catch (e: any) {
+    return { worked: false, message: e?.message || 'request failed' };
+  }
 }

2-2: Import path: consider using the sub‑package to reduce bundle size.

If you keep @xsai/generate-object, import from it directly.

-import { generateObject, GenerateObjectOptions, UserMessage } from 'xsai';
+import { generateObject, type GenerateObjectOptions, type UserMessage } from '@xsai/generate-object';
src/components/project/FileList.tsx (1)

387-401: Minor: aiEnabled is always true with current hook; condition is redundant.

If you plan to gate by capability later, keep it; otherwise simplify the render check.

Also applies to: 593-593

src/components/ai/ModelConfigForm.tsx (2)

103-108: Fix typos in user‑facing copy.

“authencation” → “authentication”; “saved inside in your browser” → “saved in your browser”.

-        The LLM API should use the OpenAI-compatible format and API key
-        authencation. The model should support image input and structured
+        The LLM API should use the OpenAI‑compatible format and API key
+        authentication. The model should support image input and structured
         output.
       </p>
-      <p>This configuration is only used and saved inside in your browser.</p>
+      <p>This configuration is only used and saved in your browser.</p>

81-95: Provider empty for Custom; consider defaulting to 'Custom' to avoid ''.

Downstream code may expect a non‑empty provider.

-    let provider = '';
+    let provider = 'Custom';
     if (values.preset >= 0 && values.preset < LlmService.llmPresets.length) {
       provider = LlmService.llmPresets[values.preset].provider;
     }
src/components/ai/index.tsx (2)

11-11: Namespace nit: logger label mismatches file.

Use a component‑appropriate namespace.

-const debugLogger = createDebugLogger('components:project:FileListAiTranslate');
+const debugLogger = createDebugLogger('components:ai:index');

58-78: Remove unused variable.

finished is assigned but not used.

-    const finished = await new Promise<boolean>((resolve, reject) => {
+    await new Promise<boolean>((resolve, reject) => {
src/components/ai/BatchTranslateModal.tsx (9)

183-199: Conflicting statuses when no text is found.

You set “done: no text blocks” then immediately switch to “saving” and finally to “success: recognized 0…”. Early‑return to keep UX consistent.

Apply this diff:

   async function saveTranslations(f: MFile, r: FilePreprocessResult) {
     if (r.texts.length === 0) {
       setFileState(f, 'done: no text blocks', stateIcons.skip);
-    }
-    setFileState(f, 'saving', stateIcons.working);
+      return;
+    }
+    setFileState(f, 'saving', stateIcons.working);

223-229: img2dataurl never rejects; add error/abort handlers (prevents silent failures).

As written, onloadend resolves even on errors, potentially passing null URLs downstream.

Apply this diff:

 async function img2dataurl(img: Blob) {
   return new Promise<string>((resolve, reject) => {
     const reader = new FileReader();
-    reader.onloadend = () => resolve(reader.result as string);
+    reader.onload = () => resolve(reader.result as string);
+    reader.onerror = () =>
+      reject(reader.error ?? new Error('FileReader failed'));
+    reader.onabort = () => reject(new Error('FileReader aborted'));
     reader.readAsDataURL(img);
   });
 }

72-74: Remove unused parameter.

idx isn’t used; drop it to reduce noise.

Apply this diff:

-    const tasksEnded = Promise.allSettled(
-      files.map((f, idx) => fileLimiter.use(() => translateFile(f, idx))),
-    );
+    const tasksEnded = Promise.allSettled(
+      files.map((f) => fileLimiter.use(() => translateFile(f))),
+    );

95-96: Keep signature tight: drop unused idx.

Apply this diff:

-    async function translateFile(f: MFile, idx: number) {
+    async function translateFile(f: MFile) {

86-93: Match by stable ID, not object identity.

Future renders may pass new file object instances; matching by id is safer.

Apply this diff:

-      setFileStates((prev) =>
-        prev.map((state) =>
-          state.file === f ? { ...state, message, icon } : state,
-        ),
-      );
+      setFileStates((prev) =>
+        prev.map((state) =>
+          state.file.id === f.id ? { ...state, message, icon } : state,
+        ),
+      );

208-210: UI copy is slightly misleading.

Saving translations continues by design (no cancel token) even after closing. Consider clarifying: “Closing will stop new work and cancel in‑flight requests where possible; already-started saves may continue.”


52-55: Consider localizing user‑visible strings.

Statuses like “waiting/working/skip/error/saving/translated…” are plain strings. Prefer intl.formatMessage for i18n consistency.


81-83: Optionally disable OK until finished.

You enable OK on completion, but it may not be disabled initially. If parent didn’t, do it here at start of the effect.

Apply this diff near the start of the effect:

     if (!running.current) {
       debugLogger('canceled');
       return;
     }
+    // ensure OK disabled until batch ends
+    try { getHandle().update({ okButtonProps: { disabled: true } }); } catch {}

122-136: Operational: large data URLs can bloat prompts.

Inlining full‑size images as data URLs may exceed provider limits or spike latency/cost. Consider downscaling/compressing before encoding, or use signed URLs if supported by your provider.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6f2fe3b and 34fc7ad.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (13)
  • package.json (2 hunks)
  • src/components/ai/BatchTranslateModal.tsx (1 hunks)
  • src/components/ai/ModelConfigForm.tsx (1 hunks)
  • src/components/ai/index.tsx (1 hunks)
  • src/components/project/FileList.tsx (4 hunks)
  • src/components/project/FileListAiTranslate.tsx (0 hunks)
  • src/services/ai/TranslateCompanion.tsx (0 hunks)
  • src/services/ai/llm_preprocess.ts (1 hunks)
  • src/services/ai/mit_preprocess.ts (0 hunks)
  • src/services/ai/multimodal_recognize.ts (0 hunks)
  • src/services/ai/use_moeflow_companion.ts (0 hunks)
  • src/services/labelplus_packager.ts (0 hunks)
  • src/store/user/sagas.ts (1 hunks)
💤 Files with no reviewable changes (6)
  • src/services/ai/multimodal_recognize.ts
  • src/services/labelplus_packager.ts
  • src/services/ai/TranslateCompanion.tsx
  • src/services/ai/use_moeflow_companion.ts
  • src/components/project/FileListAiTranslate.tsx
  • src/services/ai/mit_preprocess.ts
🧰 Additional context used
🧬 Code graph analysis (5)
src/components/ai/ModelConfigForm.tsx (1)
src/services/ai/llm_preprocess.ts (1)
  • LLMConf (7-13)
src/components/ai/BatchTranslateModal.tsx (5)
src/utils/debug-logger.ts (1)
  • createDebugLogger (3-5)
src/services/ai/llm_preprocess.ts (3)
  • LLMConf (7-13)
  • llmPreprocessFile (61-96)
  • FilePreprocessResult (53-53)
src/interfaces/target.ts (1)
  • Target (4-12)
src/components/ai/index.tsx (1)
  • ModalHandle (13-13)
src/apis/index.ts (2)
  • api (289-313)
  • resultTypes (88-95)
src/services/ai/llm_preprocess.ts (1)
src/utils/debug-logger.ts (1)
  • createDebugLogger (3-5)
src/components/project/FileList.tsx (1)
src/components/ai/index.tsx (1)
  • useAiTranslate (81-94)
src/components/ai/index.tsx (5)
src/utils/debug-logger.ts (1)
  • createDebugLogger (3-5)
src/services/ai/llm_preprocess.ts (1)
  • LLMConf (7-13)
src/interfaces/target.ts (1)
  • Target (4-12)
src/components/ai/ModelConfigForm.tsx (1)
  • ModelConfigForm (10-163)
src/components/ai/BatchTranslateModal.tsx (1)
  • BatchTranslateModalContent (40-221)
🔇 Additional comments (4)
src/components/project/FileList.tsx (2)

86-91: OK: dedup + map selection for AI flow.

The Set+map+filter(Boolean) pattern is clear and safe.


387-401: aiTranslateApi.start callback currently unused — remove or wire it through

File src/components/project/FileList.tsx passes a callback to aiTranslateApi.start((file) => debugLogger('file saved', file)); I couldn't find aiTranslateApi's implementation in the repo — confirm whether start should accept/use a callback. Either remove the unused argument or make start invoke the callback on successful saves.

src/components/ai/BatchTranslateModal.tsx (2)

113-116: Verify fetch auth context for file URLs.

If resData.url requires cookies/credentials (common for signed or same‑origin URLs), add credentials: 'include'. Otherwise downloads may fail sporadically outside dev.

Would you confirm whether file URLs are public or cookie‑gated? If gated, update to:

-      const imgBlob = await fetch(resData.url!, { signal: abort.signal }).then(
+      const imgBlob = await fetch(resData.url!, { signal: abort.signal, credentials: 'include' }).then(

166-169: Heads‑up: upstream Gemini workaround may not apply.

In src/services/ai/llm_preprocess.ts, the Gemini 1000‑scale workaround assigns to ret but the function returns res.object, discarding the fix. If so, tf.imageW/H could be incorrect here.

Do you want me to open a separate patch to return ret from llmPreprocessFile when applying the Gemini fix?

Comment on lines +163 to +181
const src = await api.source.createSource({
fileID: f.id,
data: {
x: clipTo01((tb.left + tb.width / 2) / tf.imageW),
y: clipTo01((tb.top + tb.height / 2) / tf.imageH),
content: tb.text,
},
configs: { cancelToken },
});
await api.translation.createTranslation({
sourceID: src.data.id,
data: {
content: tb.translated,
targetID: target.id,
},
// not using the cancel token, to make the saving operation closer to atomic
// configs: { cancelToken },
});
}
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

Unchecked API results can throw and leave inconsistent data.

Both createSource and createTranslation results are used without checking their type. If either fails, src.data may be undefined, causing a runtime crash; also partial writes won’t be surfaced.

Apply this diff to validate responses and fail fast:

-      const src = await api.source.createSource({
+      const srcRes = await api.source.createSource({
         fileID: f.id,
         data: {
-          x: clipTo01((tb.left + tb.width / 2) / tf.imageW),
-          y: clipTo01((tb.top + tb.height / 2) / tf.imageH),
+          // normalized center (see safe divisors below)
+          x: clipTo01((tb.left + tb.width / 2) / Math.max(1, tf.imageW ?? 0)),
+          y: clipTo01((tb.top + tb.height / 2) / Math.max(1, tf.imageH ?? 0)),
           content: tb.text,
         },
         configs: { cancelToken },
       });
-      await api.translation.createTranslation({
-        sourceID: src.data.id,
+      if (srcRes.type !== resultTypes.SUCCESS) {
+        throw new Error('createSource failed');
+      }
+      const trRes = await api.translation.createTranslation({
+        sourceID: srcRes.data.id,
         data: {
           content: tb.translated,
           targetID: target.id,
         },
         // not using the cancel token, to make the saving operation closer to atomic
         // configs: { cancelToken },
       });
+      if (trRes.type !== resultTypes.SUCCESS) {
+        throw new Error('createTranslation failed');
+      }
📝 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
const src = await api.source.createSource({
fileID: f.id,
data: {
x: clipTo01((tb.left + tb.width / 2) / tf.imageW),
y: clipTo01((tb.top + tb.height / 2) / tf.imageH),
content: tb.text,
},
configs: { cancelToken },
});
await api.translation.createTranslation({
sourceID: src.data.id,
data: {
content: tb.translated,
targetID: target.id,
},
// not using the cancel token, to make the saving operation closer to atomic
// configs: { cancelToken },
});
}
const srcRes = await api.source.createSource({
fileID: f.id,
data: {
// normalized center (see safe divisors below)
x: clipTo01((tb.left + tb.width / 2) / Math.max(1, tf.imageW ?? 0)),
y: clipTo01((tb.top + tb.height / 2) / Math.max(1, tf.imageH ?? 0)),
content: tb.text,
},
configs: { cancelToken },
});
if (srcRes.type !== resultTypes.SUCCESS) {
throw new Error('createSource failed');
}
const trRes = await api.translation.createTranslation({
sourceID: srcRes.data.id,
data: {
content: tb.translated,
targetID: target.id,
},
// not using the cancel token, to make the saving operation closer to atomic
// configs: { cancelToken },
});
if (trRes.type !== resultTypes.SUCCESS) {
throw new Error('createTranslation failed');
}
}
🤖 Prompt for AI Agents
In src/components/ai/BatchTranslateModal.tsx around lines 163 to 181, the code
assumes api responses are successful and accesses src.data and translation
response fields directly; update the flow to validate each API response type
immediately after the await (e.g., check response.type === 'success' or
equivalent), throw or return a descriptive error when a response indicates
failure so the function fails fast and avoids using undefined data, and wrap the
overall sequence in try/catch to log/propagate errors so partial writes are
surfaced and handled consistently.

Comment on lines +166 to +169
x: clipTo01((tb.left + tb.width / 2) / tf.imageW),
y: clipTo01((tb.top + tb.height / 2) / tf.imageH),
content: tb.text,
},
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

Guard against divide-by-zero when normalizing coordinates.

tf.imageW/H can be 0/undefined (LLM outputs vary). Current math would yield Infinity and then clipTo01 won’t help.

Minimal fix (if you prefer not to touch the rest of the function):

-          x: clipTo01((tb.left + tb.width / 2) / tf.imageW),
-          y: clipTo01((tb.top + tb.height / 2) / tf.imageH),
+          x: clipTo01((tb.left + tb.width / 2) / Math.max(1, tf.imageW ?? 0)),
+          y: clipTo01((tb.top + tb.height / 2) / Math.max(1, tf.imageH ?? 0)),
📝 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
x: clipTo01((tb.left + tb.width / 2) / tf.imageW),
y: clipTo01((tb.top + tb.height / 2) / tf.imageH),
content: tb.text,
},
x: clipTo01((tb.left + tb.width / 2) / Math.max(1, tf.imageW ?? 0)),
y: clipTo01((tb.top + tb.height / 2) / Math.max(1, tf.imageH ?? 0)),
content: tb.text,
},
🤖 Prompt for AI Agents
In src/components/ai/BatchTranslateModal.tsx around lines 166 to 169, the
normalization uses tf.imageW and tf.imageH directly which can be 0 or undefined
causing divide-by-zero/Infinity; update the code to guard the denominators by
using safe values (e.g., const w = tf.imageW || 1 and const h = tf.imageH || 1
or check for zero/undefined and fall back to a sensible default like 1 or center
coordinate) and compute x and y using those safe w/h so the result never becomes
Infinity before calling clipTo01.

Comment on lines 33 to 39
let confValue: LLMConf | null = null;
const onChange = (conf: LLMConf) => {
debugLogger('model configured', conf);
confValue = conf;
if (confValue.model && confValue.baseUrl && confValue.apiKey) {
handle.update({ okButtonProps: {} });
}
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

Do not log API keys. Redact secrets in diagnostics.

conf includes apiKey. Avoid printing it.

-      const onChange = (conf: LLMConf) => {
-        debugLogger('model configured', conf);
+      const onChange = (conf: LLMConf) => {
+        const { apiKey, ...rest } = conf;
+        debugLogger('model configured', { ...rest, apiKey: apiKey ? '***redacted***' : '' });
         confValue = conf;
         if (confValue.model && confValue.baseUrl && confValue.apiKey) {
           handle.update({ okButtonProps: {} });
         }
       };
📝 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
let confValue: LLMConf | null = null;
const onChange = (conf: LLMConf) => {
debugLogger('model configured', conf);
confValue = conf;
if (confValue.model && confValue.baseUrl && confValue.apiKey) {
handle.update({ okButtonProps: {} });
}
let confValue: LLMConf | null = null;
const onChange = (conf: LLMConf) => {
const { apiKey, ...rest } = conf;
debugLogger('model configured', { ...rest, apiKey: apiKey ? '***redacted***' : '' });
confValue = conf;
if (confValue.model && confValue.baseUrl && confValue.apiKey) {
handle.update({ okButtonProps: {} });
}
};
🤖 Prompt for AI Agents
In src/components/ai/index.tsx around lines 33 to 39, the onChange handler calls
debugLogger('model configured', conf) but conf contains an apiKey; remove or
redact secrets before logging. Modify the handler to create a sanitized copy of
conf (omit or replace conf.apiKey with a masked value like '***REDACTED***') and
pass that sanitized object to debugLogger, while keeping confValue unchanged and
preserving the existing logic that enables the OK button when
model/baseUrl/apiKey are set.

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

♻️ Duplicate comments (1)
src/services/ai/llm_preprocess.ts (1)

105-116: Fix from prior review is present — returning patched ret.

The Gemini workaround now takes effect. Good catch.

🧹 Nitpick comments (10)
src/services/ai/llm_preprocess.ts (6)

43-64: Tighten Zod schema (invalid message option; add numeric constraints).

z.number({ message: ... }) is not a valid constructor option. Also, coords/sizes should be ints with proper bounds; allow optional comment.

Apply:

-const filePreprocessResultSchema = z.object({
-  imageW: z.number({ message: 'the width of the image in PX' }),
-  imageH: z.number({ message: 'the height of the image in PX' }),
+const filePreprocessResultSchema = z.object({
+  imageW: z.number().int().positive().describe('image width in px'),
+  imageH: z.number().int().positive().describe('image height in px'),
   texts: z.array(
     z.object({
-      left: z
-        .number()
-        .describe('left coordinate of the text in PX, in the whole image'),
-      top: z
-        .number()
-        .describe('top coordinate of the text in PX, in the whole image'),
-      width: z.number().describe('width of the text in PX'),
-      height: z.number().describe('height of the text in PX'),
-      textLines: z.array(z.string()).describe('the text lines'),
-      text: z.string().describe('concatenated text'),
-      translated: z.string().describe('translated text'),
-      comment: z
-        .string()
-        .describe('additional comment of the text, or the translation'),
+      left: z.number().int().nonnegative().describe('left px from image origin'),
+      top: z.number().int().nonnegative().describe('top px from image origin'),
+      width: z.number().int().positive().describe('box width px'),
+      height: z.number().int().positive().describe('box height px'),
+      textLines: z.array(z.string().min(1)).min(1).describe('text lines'),
+      text: z.string().min(1).describe('concatenated text'),
+      translated: z.string().min(1).describe('translated text'),
+      comment: z.string().optional().describe('additional comment or notes'),
     }),
   ),
 });

21-41: Preset models/URLs may drift; confirm correctness.

Please verify these model IDs and base URLs are current and actually supported in your target tenants.

Happy to adjust presets or gate them behind a feature flag if needed.


84-85: Avoid stray space when extraPrompt is empty.

Inline-trim and conditionally append to prevent “double space” prompts.

Apply:

-        text: `Please translate the image to ${targetLang}. ${llmConf.extraPrompt || ''}`,
+        text: `Please translate the image to ${targetLang}.${llmConf.extraPrompt ? ' ' + llmConf.extraPrompt.trim() : ''}`,

121-127: Type the tool payload in execute for stronger guarantees.

This documents intent and catches drift if the schema changes.

Apply:

-      execute: (_result) => {
+      execute: (_result: FilePreprocessResult) => {
         submittedResult = _result;
         return 'saved';
       },

135-139: Gate Anthropic browser header by provider/baseUrl, not model string.

Prevents leaking Anthropic-only header when a “claude-*” model is proxied elsewhere.

Apply:

-      headers: {
-        // Anthropic-only workaround, to call API from browser (otherwise it rejects with CORS error).
-        ...(llmConf.model.toLowerCase().includes('claude-') && {
-          'anthropic-dangerous-direct-browser-access': 'true',
-        }),
-      },
+      headers: {
+        // Anthropic-only workaround to avoid CORS rejection when calling from browser.
+        ...((llmConf.provider === 'Anthropic' || /anthropic\.com\/v1\/?/i.test(llmConf.baseUrl)) && {
+          'anthropic-dangerous-direct-browser-access': 'true',
+        }),
+      },

68-72: testModel is a stub; consider flagging dev-only or wiring a real smoke test.

At least check presence of apiKey and reachability, or guard behind a dev flag.

scripts/generate-locale-json.ts (3)

11-21: Harden extractor: tighten typing and guard against arrays/non-plain objects

Prevents surprising crashes if arrays or odd YAML nodes appear; also clarifies intent via a simple type guard.

-/**
- * yield [path, message] pairs
- */
-function* extractPathedMessages(obj: object, locale: string, pathPrefix: readonly string[] = []): Generator<[string, string]> {
-  for (const [key, value] of Object.entries(obj)) {
-    if (typeof value === 'object' && value) {
-      yield* extractPathedMessages(value, locale, [...pathPrefix, key]);
-    } else if (typeof value === 'string') {
-      if (key === locale) yield [pathPrefix.join('.'), value];
-    } else {
-      throw new Error(`unexpected value type at ${[...pathPrefix, key].join('.')}: ${typeof value}`);
-    }
-  }
-}
+/**
+ * yield [path, message] pairs
+ */
+const isPlainObject = (v: unknown): v is Record<string, unknown> =>
+  !!v && typeof v === 'object' && !Array.isArray(v);
+
+function* extractPathedMessages(
+  obj: unknown,
+  locale: string,
+  pathPrefix: readonly string[] = [],
+): Generator<[string, string]> {
+  if (!isPlainObject(obj)) return;
+  for (const [key, value] of Object.entries(obj)) {
+    if (isPlainObject(value)) {
+      yield* extractPathedMessages(value, locale, [...pathPrefix, key]);
+    } else if (typeof value === 'string') {
+      if (key === locale) yield [pathPrefix.join('.'), value];
+    } else if (Array.isArray(value)) {
+      throw new Error(`unexpected array at ${[...pathPrefix, key].join('.')}`);
+    } else if (value != null) {
+      throw new Error(`unexpected value type at ${[...pathPrefix, key].join('.')}: ${typeof value}`);
+    }
+  }
+}

23-27: Make locales explicit and typed; iterate the constant

Slight readability/typing improvement and avoids the extra Object.entries allocation.

-const lang2Basename = Object.entries({
-    zhCn: 'zh-cn.json',
-    en: 'en.json',
-})
+const locales = [
+  ['zhCn', 'zh-cn.json'],
+  ['en', 'en.json'],
+] as const;
-for (const [locale, basename] of lang2Basename ) {
+for (const [locale, basename] of locales) {

Also applies to: 37-37


36-45: Remove dead code or put it to work

path2count is accumulated but never used; drop it or emit a warning for missing translations.

Option A (remove):

-  const path2count: Record<string, number> = {};
   for (const [locale, basename] of locales) {
@@
-    for(const [path, msg] of extractPathedMessages(messages, locale)) {
-      path2count[path] = (path2count[path] ?? 0) + 1;
-      value[path] = msg;
-    }
+    for (const [path, msg] of extractPathedMessages(messages, locale)) {
+      value[path] = msg;
+    }

Option B (warn after loop):

Add after the for-loop (below Line 49):

// warn if some paths are missing in one or more locales
const expected = locales.length;
const missing = Object.entries(path2count).filter(([, c]) => c !== expected);
if (missing.length) {
  console.warn(`Missing translations for ${missing.length} keys:`);
  for (const [p, c] of missing) console.warn(`  ${p} (${c}/${expected})`);
}
src/locales/messages.yaml (1)

808-852: Minor copy tweaks for clarity/consistency (EN); source of truth here

Two small improvements:

  • buttonTip.en: “Detect text and Translate with LLM” → “Detect text and translate with LLM”
  • workingModal.content.en: “stop the work” → “stop the translation”

Apply in this YAML block so generated JSONs follow.

-    en: Detect text and Translate with LLM
+    en: Detect text and translate with LLM
@@
-      en: Translating {fileCount} files with LLM. Closing this dialog will stop the work.
+      en: Translating {fileCount} files with LLM. Closing this dialog will stop the translation.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 34fc7ad and fc01360.

📒 Files selected for processing (10)
  • scripts/generate-locale-json.ts (2 hunks)
  • src/components/ai/BatchTranslateModal.tsx (1 hunks)
  • src/components/ai/ModelConfigForm.tsx (1 hunks)
  • src/components/ai/index.tsx (1 hunks)
  • src/components/icon/index.ts (1 hunks)
  • src/components/project/FileList.tsx (4 hunks)
  • src/locales/en.json (1 hunks)
  • src/locales/messages.yaml (1 hunks)
  • src/locales/zh-cn.json (1 hunks)
  • src/services/ai/llm_preprocess.ts (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • src/components/icon/index.ts
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/components/project/FileList.tsx
  • src/components/ai/BatchTranslateModal.tsx
  • src/components/ai/ModelConfigForm.tsx
  • src/components/ai/index.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/services/ai/llm_preprocess.ts (1)
src/utils/debug-logger.ts (1)
  • createDebugLogger (3-5)
🔇 Additional comments (4)
src/locales/zh-cn.json (2)

266-279: LGTM: generated keys match the new nested schema

The new aiTranslate keys are present and consistent with messages.yaml.


260-271: Sanity check — ensure removed locale keys have no runtime references

Sandbox rg attempts failed (PCRE error / no files searched); verification inconclusive.

Location: src/locales/zh-cn.json (lines 260–271)

Run from repo root:
grep -R -n --exclude-dir=src/locales -E 'fileList.aiTranslateTip|fileList.aiTranslate' . || true

Expected: no matches.

src/locales/en.json (1)

266-279: LGTM: new aiTranslate entries exist and align with YAML

Note: copy tweaks (lowercasing “Translate”; “stop the translation”) should be made in messages.yaml, not here.

src/locales/messages.yaml (1)

850-851: ```shell
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo
echo "Top-level entries:"
ls -1 || true
echo
if [ -f src/locales/messages.yaml ]; then
echo "Showing src/locales/messages.yaml (lines ~820-860):"
sed -n '820,860p' src/locales/messages.yaml || true
else
echo "src/locales/messages.yaml not found"
fi
echo
echo "Searching repo for common i18n libs/usages (react-intl/formatjs/i18next/etc.):"
rg -n --hidden --no-ignore -S -C2 'formatMessage|IntlProvider|react-intl|@formatjs|i18next|react-i18next|next-intl|@lingui|IntlMessageFormat|MessageFormat' || true
echo
echo "Searching package.json / lockfiles for i18n deps:"
rg -n --hidden -S '"react-intl"|"@formatjs"|"i18next"|"react-i18next"|"next-intl"|"@lingui"|"formatjs"' || true


</blockquote></details>

</blockquote></details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

Comment on lines 5 to 6
const assetDir = path.join(__dirname, '../src/locales');
const messageYaml = path.join(assetDir, 'messages.yaml');
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

🧩 Analysis chain

Confirm module system for __dirname usage

This script relies on CommonJS (__dirname). If the repo uses ESM for scripts, this will throw at runtime.

Run:


🏁 Script executed:

#!/bin/bash
# Verify TS module target and any Node ESM flags
rg -nP '"module"\s*:\s*"(commonjs|esnext|nodenext)"' tsconfig*.json -n -C2
rg -nP 'type\s*:\s*"module"' package.json -n -C2

Length of output: 166


Fix ESM __dirname usage (scripts/generate-locale-json.ts)

tsconfig*.json sets "module": "esnext", so __dirname (CommonJS) will be undefined when this script runs. Update the script or build config.

  • Location: scripts/generate-locale-json.ts (lines 5–6)
const assetDir = path.join(__dirname, '../src/locales');
const messageYaml = path.join(assetDir, 'messages.yaml');
  • Fix options (choose one):
    • Use ESM-safe code: import { fileURLToPath } from 'url'; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename);
    • Or ensure the script is executed/compiled as CommonJS (e.g., set TS module to "commonjs" for scripts or run with a CommonJS runtime).
🤖 Prompt for AI Agents
In scripts/generate-locale-json.ts around lines 5 to 6, the script uses
__dirname which is undefined under ES modules; replace the CommonJS-specific
__dirname usage with an ESM-safe approach: derive __filename from
import.meta.url via fileURLToPath and then compute __dirname with
path.dirname(__filename), or alternatively ensure this script is run/compiled as
CommonJS (e.g., set module to "commonjs" for scripts or run with a CommonJS node
runtime); update the path.join calls to use the computed ESM __dirname
equivalent so messageYaml resolves correctly.

Comment on lines +155 to +161
async function img2dataurl(img: Blob) {
return new Promise<string>((resolve, reject) => {
const reader = new FileReader();
reader.onloadend = () => resolve(reader.result as string);
reader.readAsDataURL(img);
});
}
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

Harden FileReader: handle errors/abort and ensure string result.

Current impl resolves on onloadend (incl. error) and may return null.

Apply:

 async function img2dataurl(img: Blob) {
-  return new Promise<string>((resolve, reject) => {
-    const reader = new FileReader();
-    reader.onloadend = () => resolve(reader.result as string);
-    reader.readAsDataURL(img);
-  });
+  return new Promise<string>((resolve, reject) => {
+    const reader = new FileReader();
+    reader.onload = () => {
+      const res = reader.result;
+      if (typeof res === 'string') return resolve(res);
+      reject(new Error('Failed to convert image to data URL.'));
+    };
+    reader.onerror = () => reject(reader.error ?? new Error('FileReader error'));
+    reader.onabort = () => reject(new Error('FileReader aborted'));
+    reader.readAsDataURL(img);
+  });
 }
📝 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 function img2dataurl(img: Blob) {
return new Promise<string>((resolve, reject) => {
const reader = new FileReader();
reader.onloadend = () => resolve(reader.result as string);
reader.readAsDataURL(img);
});
}
async function img2dataurl(img: Blob) {
return new Promise<string>((resolve, reject) => {
const reader = new FileReader();
reader.onload = () => {
const res = reader.result;
if (typeof res === 'string') return resolve(res);
reject(new Error('Failed to convert image to data URL.'));
};
reader.onerror = () => reject(reader.error ?? new Error('FileReader error'));
reader.onabort = () => reject(new Error('FileReader aborted'));
reader.readAsDataURL(img);
});
}
🤖 Prompt for AI Agents
In src/services/ai/llm_preprocess.ts around lines 155-161, the img2dataurl
FileReader currently resolves on onloadend (which fires for success and failure)
and may return null; update it to attach onload, onerror, and onabort handlers,
resolve only from reader.result when it's a non-null string, and reject with an
Error on error/abort or if result is null; also ensure you remove/clear handlers
after completion to avoid memory leaks.

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: 0

🧹 Nitpick comments (1)
src/locales/messages.yaml (1)

808-899: Polish phrasing + zh spacing around acronyms (LLM/API/URL).

Minor copy/consistency improvements. Apply in YAML (source of truth), then re-generate JSON.

 fileList.aiTranslate:
   buttonText:
     zhCn: 自动翻译
     en: Auto Translate
   buttonTip:
-    zhCn: 用LLM自动识别文字并翻译
-    en: Detect text and Translate with LLM
+    zhCn: 用 LLM 自动识别文字并翻译
+    en: Detect text and translate with LLM
   configModal:
     title:
-      zhCn: 配置LLM
+      zhCn: 配置 LLM
       en: Configure LLM
     modelDesc:
-      zhCn: 请提供用于翻译图片的LLM API配置。
+      zhCn: 请提供用于翻译图片的 LLM API 配置。
       en: Please provide the LLM API configuration to translate the images.
     modelRequirements:
-      zhCn: LLM API 需使用OpenAI兼容格式。模型需支持图像输入和工具调用。预设配置中的模型经过测试。
+      zhCn: LLM API 需使用 OpenAI 兼容格式。模型需支持图像输入和工具调用。预设配置中的模型经过测试。
       en: The LLM API should use the OpenAI-compatible format. Multimodal (image input) and tool calling capabilities are required. The models in presets are tested to work.
     configsAreLocal:
       zhCn: LLM 配置仅在您的浏览器中使用和保存。
       en: This LLM configuration is only used and saved inside your browser.
     presets:
       label:
         zhCn: 预设
         en: Presets
       placeholder:
         zhCn: 请选择预设
         en: Please pick a preset
       custom:
         zhCn: 自定义
         en: Custom
     model:
       label:
         zhCn: 模型
         en: Model
       required:
         zhCn: 请输入模型名称
         en: Please enter the model name
     baseUrl:
       label:
-        zhCn: API基础URL
+        zhCn: API 基础 URL
         en: API Base URL
       required:
-        zhCn: 请输入基础URL
+        zhCn: 请输入基础 URL
         en: Please enter the base URL
       invalidUrl:
-        zhCn: 请输入有效的URL
+        zhCn: 请输入有效的 URL
         en: Please enter a valid URL
     apiKey:
       label:
-        zhCn: API密钥
+        zhCn: API 密钥
         en: API Key
       required:
         zhCn: 请输入您的API密钥
         en: Please enter your API key
   workingModal:
     content:
-      en: Translating {fileCount} files with LLM. Closing this dialog will stop the work.
-      zhCn: 正在用LLM翻译 {fileCount} 个文件,关闭此对话框将中断翻译。
+      en: Translating {fileCount} files with LLM. Closing this dialog will stop the work.
+      zhCn: 正在用 LLM 翻译 {fileCount} 个文件,关闭此对话框将中断翻译。
   fileMessage:
     sendingImage:
       zhCn: 发送图片中...
       en: Sending image...

Notes:

  • Keep placeholders {fileCount}/{count} unchanged.
  • “tool calling” is stated as required; verify it matches actual implementation needs. If optional, consider softening to “may be required”.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fc01360 and b87524f.

📒 Files selected for processing (4)
  • src/components/ai/ModelConfigForm.tsx (1 hunks)
  • src/locales/en.json (1 hunks)
  • src/locales/messages.yaml (1 hunks)
  • src/locales/zh-cn.json (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/ai/ModelConfigForm.tsx
🔇 Additional comments (2)
src/locales/zh-cn.json (1)

266-293: LGTM, mirror of messages.yaml. Please regen JSON after YAML tweak.

The texts look correct and match the new UI flow. After applying the minor wording/spacing tweaks suggested in messages.yaml, please regenerate this JSON from the source to keep them in sync.

src/locales/en.json (1)

266-293: Mirror fix: lowercase “translate”; then regen from YAML.

Change “Detect text and Translate with LLM” → “Detect text and translate with LLM” via messages.yaml and regenerate this file.

jokester and others added 3 commits September 17, 2025 19:26
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@sonarqubecloud
Copy link

@jokester jokester merged commit 1706201 into main Sep 18, 2025
3 checks passed
@jokester jokester deleted the add-multimodal-auto-translate branch September 18, 2025 16:38
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