Feature for batch #165
Open
Feature for batch #165
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideThis PR adds a batch translation feature that groups multiple text nodes to maximize model throughput, extends the translation queue to track character-level progress and expose speed and ETA, introduces a persistent statistics manager to compute average speed and estimate remaining time, revamps the floating UI to display detailed real-time progress and stats, and updates configuration and UI controls to enable and tune batch translation. Sequence diagram for batch translation processsequenceDiagram
actor User
participant MainVue
participant BatchTranslationManager
participant TranslateQueue
participant TranslationStatsManager
participant TranslationStatusUI
User->>MainVue: Enable batch translation & set batch size
User->>MainVue: Initiate translation
MainVue->>BatchTranslationManager: Collect text nodes
BatchTranslationManager->>BatchTranslationManager: Group texts by batch size
BatchTranslationManager->>TranslateQueue: Enqueue batch translation (with char count)
TranslateQueue->>TranslationStatsManager: Start translation, track stats
TranslateQueue->>TranslateQueue: Process translation
TranslateQueue->>TranslationStatsManager: Complete translation, update stats
TranslateQueue->>BatchTranslationManager: Return translated batch
BatchTranslationManager->>MainVue: Apply translation to DOM nodes
TranslationStatsManager->>TranslationStatusUI: Update progress, speed, ETA
TranslationStatusUI->>User: Show real-time progress
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
Blocking issues:
- User controlled data in methods like
innerHTML,outerHTMLordocument.writeis an anti-pattern that can lead to XSS vulnerabilities (link) - User controlled data in a
node.innerHTMLis an anti-pattern that can lead to XSS vulnerabilities (link)
General comments:
- TranslationStatus.vue has gotten quite complex with countdown and formatting logic—consider extracting the timer and formatRemainingTime/formatNumber into a reusable composable or utility for clarity and testability.
- The setInterval in onMounted may fire overlapping updateStatus calls if one takes longer than 500ms—switch to a recursive setTimeout or guard to avoid concurrent async updates.
- The MutationObserver used for batch translations isn't explicitly disconnected when toggling the feature off—add cleanup logic to disconnect observers and prevent memory leaks.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- TranslationStatus.vue has gotten quite complex with countdown and formatting logic—consider extracting the timer and formatRemainingTime/formatNumber into a reusable composable or utility for clarity and testability.
- The setInterval in onMounted may fire overlapping updateStatus calls if one takes longer than 500ms—switch to a recursive setTimeout or guard to avoid concurrent async updates.
- The MutationObserver used for batch translations isn't explicitly disconnected when toggling the feature off—add cleanup logic to disconnect observers and prevent memory leaks.
## Individual Comments
### Comment 1
<location> `entrypoints/utils/batchTranslation.ts:132-141` </location>
<code_context>
+ const characterCount = origin.length;
+
+ // 使用队列处理翻译请求（支持字符计数）
return enqueueTranslation(async () => {
// 创建翻译任务
const translationTask = async (retryCount: number = 0): Promise<string> => {
@@ -94,7 +97,7 @@ export async function translateText(origin: string, context: string = document.t
// 开始执行翻译任务
return translationTask();
- });
+ }, characterCount);
}
/**
diff --git a/entrypoints/utils/translateQueue.ts b/entrypoints/utils/translateQueue.ts
index 7e792bd..de59db6 100644
--- a/entrypoints/utils/translateQueue.ts
+++ b/entrypoints/utils/translateQueue.ts
@@ -1,14 +1,38 @@
/**
* 翻译队列管理模块
* 控制并发翻译任务的数量，避免同时进行过多翻译请求
+ * 扩展支持字符计数和进度监控
*/
import { config } from './config';
+import { translationStatsManager } from './translationStats';
// 队列状态
let activeTranslations = 0; // 当前活跃的翻译任务数量
</code_context>
<issue_to_address>
**issue (bug_risk):** Error handling in batch translation may leave some items untranslated if a group fails.
Consider adding fallback logic to update nodes with the original text or an error message when a translation group fails, to prevent inconsistent states.
</issue_to_address>
### Comment 2
<location> `entrypoints/utils/batchTranslation.ts:189-191` </location>
<code_context>
+ const originalSegments = originalText.split(this.separator);
+ const translatedSegments = translatedText.split(this.separator);
+
+ // 如果分割后的数量不匹配，返回整个翻译文本
+ if (originalSegments.length !== translatedSegments.length) {
+ return [translatedText];
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Segment mismatch fallback may result in incorrect node updates.
When segment counts differ, all nodes receive the full translated text, which may confuse users. Consider skipping the update or showing a warning to prevent misleading output.
</issue_to_address>
### Comment 3
<location> `entrypoints/utils/translateQueue.ts:91-92` </location>
<code_context>
}
};
+ // 更新总字符数
+ totalCharacters += characterCount;
+
+ // 记录当前任务
</code_context>
<issue_to_address>
**issue (bug_risk):** Accumulating totalCharacters may overcount if tasks are retried or fail.
Increment totalCharacters only after a task completes successfully, or separately track retries to avoid double-counting.
</issue_to_address>
### Comment 4
<location> `entrypoints/utils/batchTranslation.ts:225` </location>
<code_context>
node.innerHTML = translatedText;
</code_context>
<issue_to_address>
**security (javascript.browser.security.insecure-document-method):** User controlled data in methods like `innerHTML`, `outerHTML` or `document.write` is an anti-pattern that can lead to XSS vulnerabilities
*Source: opengrep*
</issue_to_address>
### Comment 5
<location> `entrypoints/utils/batchTranslation.ts:225` </location>
<code_context>
node.innerHTML = translatedText;
</code_context>
<issue_to_address>
**security (javascript.browser.security.insecure-innerhtml):** User controlled data in a `node.innerHTML` is an anti-pattern that can lead to XSS vulnerabilities
*Source: opengrep*
</issue_to_address>
### Comment 6
<location> `entrypoints/utils/batchTranslation.ts:120` </location>
<code_context>
const combinedText = group.combinedText;
</code_context>
<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/use-object-destructuring))
```suggestion
const {combinedText} = group;
```
<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.
From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>
### Comment 7
<location> `entrypoints/utils/batchTranslation.ts:205` </location>
<code_context>
const node = item.node;
</code_context>
<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/use-object-destructuring))
```suggestion
const {node} = item;
```
<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.
From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+132
to
+141
| return enqueueTranslation(async () => { | ||
| try { | ||
| const result = await Promise.race([ | ||
| browser.runtime.sendMessage({ context: document.title, origin: combinedText }), | ||
|
|
||
| new Promise<never>((_, reject) => | ||
| setTimeout(() => reject(new Error('翻译请求超时')), 60000) | ||
| ) | ||
| ]) as string; | ||
|
|
There was a problem hiding this comment.
issue (bug_risk): Error handling in batch translation may leave some items untranslated if a group fails.
Consider adding fallback logic to update nodes with the original text or an error message when a translation group fails, to prevent inconsistent states.
Comment on lines
+189
to
+191
| // 如果分割后的数量不匹配，返回整个翻译文本 | ||
| if (originalSegments.length !== translatedSegments.length) { | ||
| return [translatedText]; |
There was a problem hiding this comment.
issue (bug_risk): Segment mismatch fallback may result in incorrect node updates.
When segment counts differ, all nodes receive the full translated text, which may confuse users. Consider skipping the update or showing a warning to prevent misleading output.
Comment on lines
+91
to
+92
| // 更新总字符数 | ||
| totalCharacters += characterCount; |
There was a problem hiding this comment.
issue (bug_risk): Accumulating totalCharacters may overcount if tasks are retried or fail.
Increment totalCharacters only after a task completes successfully, or separately track retries to avoid double-counting.
| node.appendChild(translationNode); | ||
| } else { | ||
| // 单语模式 | ||
| node.innerHTML = translatedText; |
There was a problem hiding this comment.
security (javascript.browser.security.insecure-document-method): User controlled data in methods like innerHTML, outerHTML or document.write is an anti-pattern that can lead to XSS vulnerabilities
Source: opengrep
| node.appendChild(translationNode); | ||
| } else { | ||
| // 单语模式 | ||
| node.innerHTML = translatedText; |
There was a problem hiding this comment.
security (javascript.browser.security.insecure-innerhtml): User controlled data in a node.innerHTML is an anti-pattern that can lead to XSS vulnerabilities
Source: opengrep
| return; | ||
| } | ||
|
|
||
| const combinedText = group.combinedText; |
There was a problem hiding this comment.
suggestion (code-quality): Prefer object destructuring when accessing and using properties. (use-object-destructuring)
Suggested change
| const combinedText = group.combinedText; | |
| const {combinedText} = group; |
Explanation
Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.From the Airbnb Javascript Style Guide
| return; | ||
| } | ||
|
|
||
| const node = item.node; |
There was a problem hiding this comment.
suggestion (code-quality): Prefer object destructuring when accessing and using properties. (use-object-destructuring)
Suggested change
| const node = item.node; | |
| const {node} = item; |
Explanation
Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.From the Airbnb Javascript Style Guide
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
1.添加批量翻译功能
开启批量翻译后点击翻译为全文翻译。将段落用\n\n=====\n\n拼接，翻译完成后再分割。
这样可以尽可能吃满模型的翻译速度和减少模型的头token时间长的问题。
2.优化了翻译状态显示栏。批量翻译时可以看到剩余时间。
Summary by Sourcery
Enable batch translation by aggregating multiple text items into configurable groups to optimize API usage and reduce latency, and enhance the translation status panel with detailed progress metrics including remaining time and speed estimates.
New Features:
Enhancements: