-
Notifications
You must be signed in to change notification settings - Fork 15
Add multimodal auto translate #41
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
Conversation
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
WalkthroughAdds 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
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
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
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. Comment |
There was a problem hiding this 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 HMRinstanceP 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.jsonNo 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 compatibilitypackage.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:testModelis 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:aiEnabledis 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.
finishedis 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,
onloadendresolves even on errors, potentially passingnullURLs 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.
idxisn’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
fileobject instances; matching byidis 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.formatMessagefor 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
⛔ Files ignored due to path filters (1)
package-lock.jsonis 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 throughFile 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.urlrequires cookies/credentials (common for signed or same‑origin URLs), addcredentials: '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 toretbut the function returnsres.object, discarding the fix. If so,tf.imageW/Hcould be incorrect here.Do you want me to open a separate patch to return
retfromllmPreprocessFilewhen applying the Gemini fix?
| 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 }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| 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.
| x: clipTo01((tb.left + tb.width / 2) / tf.imageW), | ||
| y: clipTo01((tb.top + tb.height / 2) / tf.imageH), | ||
| content: tb.text, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| 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.
src/components/ai/index.tsx
Outdated
| 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: {} }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| 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.
There was a problem hiding this 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 patchedret.The Gemini workaround now takes effect. Good catch.
🧹 Nitpick comments (10)
src/services/ai/llm_preprocess.ts (6)
43-64: Tighten Zod schema (invalidmessageoption; add numeric constraints).
z.number({ message: ... })is not a valid constructor option. Also, coords/sizes should be ints with proper bounds; allow optionalcomment.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 whenextraPromptis 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 inexecutefor 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:testModelis a stub; consider flagging dev-only or wiring a real smoke test.At least check presence of
apiKeyand 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 objectsPrevents 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 constantSlight 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
path2countis 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 hereTwo 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
📒 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 schemaThe new aiTranslate keys are present and consistent with messages.yaml.
260-271: Sanity check — ensure removed locale keys have no runtime referencesSandbox 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' . || trueExpected: no matches.
src/locales/en.json (1)
266-279: LGTM: new aiTranslate entries exist and align with YAMLNote: 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 -->
| const assetDir = path.join(__dirname, '../src/locales'); | ||
| const messageYaml = path.join(assetDir, 'messages.yaml'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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 -C2Length 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).
- Use ESM-safe code:
🤖 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.
| 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); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| 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.
There was a problem hiding this 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
📒 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.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|



Summary by CodeRabbit
New Features
Refactor
Chores