Skip to content

feat: 优化翻译显示和设置界面#13

Merged
hubo1989 merged 3 commits into
mainfrom
featadd-translate-menu
Feb 12, 2026
Merged

feat: 优化翻译显示和设置界面#13
hubo1989 merged 3 commits into
mainfrom
featadd-translate-menu

Conversation

@hubo1989
Copy link
Copy Markdown
Owner

@hubo1989 hubo1989 commented Feb 12, 2026

概述

优化翻译结果显示和设置界面,提升用户体验。

主要改进

翻译显示优化

  • 翻译显示字号增大到 24pt,提升可读性
  • 翻译区域宽度与原图一致,布局更紧凑
  • 新增 10px 内边距,文字不贴边
  • 翻译显示支持深色/浅色主题自适应

设置界面改进

  • "翻译工作流"更名为"翻译引擎"
  • 增加 MTranServer 测试连接按钮
  • VLM 测试结果支持本地化(中/英文)
  • 测试连接按钮布局与 VLM 配置保持一致

功能新增

  • 增加"复制文本"按钮,支持复制翻译后的文字
  • 原"复制"按钮更名为"复制图片"

引导流程优化

  • 配置步骤增加滚动支持
  • 底部按钮固定显示,不会被截断
  • 窗口高度从 500px 增加到 620px

代码清理

  • 删除未使用的翻译覆盖层代码
  • UserDefaults 前缀从 ScreenCapture. 改为 ScreenTranslate.

修复

  • MTranServer localhost 连接问题(IPv6 解析)
  • 引导窗口底部显示不全
  • 翻译引擎缓存导致的测试失败

测试清单

  • 翻译结果显示正常,字号合适
  • 深色/浅色主题切换后翻译区域颜色正确
  • MTranServer 测试连接功能正常
  • 复制图片和复制文本功能正常
  • 引导流程各步骤显示完整

🤖 Generated with Claude Code

Summary by CodeRabbit

发布说明

  • 新功能

    • 分离复制操作:可分别复制翻译文本或源图像,复制按钮显示相应状态
    • 添加翻译引擎(MTran)连接测试按钮与连通性反馈
  • 改进

    • 在翻译渲染中引入主题支持(明/暗),改善配色与可读性
    • 优化入门界面为可滚动布局并调整窗口尺寸
    • 提高导出覆盖文字最小字号以提升可读性
    • 增强翻译服务器可用性检测与日志记录
  • 移除

    • 删除屏幕覆盖式翻译叠加、沉浸式预览与相关控制器/视图组件

主要改进:
- 翻译显示字号增大到 24pt,翻译区域宽度与原图一致
- 翻译显示支持深色/浅色主题自适应
- 设置界面增加 MTranServer 测试连接按钮
- 增加"复制文本"功能,支持复制翻译后的文字
- 引导流程增加滚动支持,底部按钮固定显示
- 删除未使用的翻译覆盖层代码

修复:
- MTranServer localhost 连接问题(IPv6 解析)
- 引导窗口底部显示不全
- VLM 测试结果本地化

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Feb 12, 2026

📝 Walkthrough

Walkthrough

将双语结果的复制操作拆分为“复制图像”与“复制文本”,在结果窗口与视图模型间引入可选的 translatedText 传递;新增 MTran 服务器连接测试与若干状态;移除多项覆盖层与预览组件;统一 UserDefaults 键前缀并引入主题化渲染。

Changes

Cohort / File(s) Summary
双语结果 (View / ViewModel / WindowController)
ScreenTranslate/Features/BilingualResult/BilingualResultView.swift, ScreenTranslate/Features/BilingualResult/BilingualResultViewModel.swift, ScreenTranslate/Features/BilingualResult/BilingualResultWindowController.swift
将单一复制动作拆分为 copyImageToClipboard()copyTextToClipboard();新增 private(set) var translatedTextshowResult(..., translatedText:) 签名及传递;View 改为两个独立按钮;WindowController 调整窗口尺寸/缩放逻辑。
覆盖层与预览移除
ScreenTranslate/Features/Overlay/BelowModeOverlayWindow.swift, ScreenTranslate/Features/Overlay/TranslationOverlayWindow.swift, ScreenTranslate/Features/Preview/ImmersiveTranslationView.swift, ScreenTranslate/Features/Preview/TranslationOverlayCanvas.swift
完全删除若干 overlay/preview 的窗口、视图、控制器、协议与绘制/交互/生命周期实现。
翻译流程与渲染主题
ScreenTranslate/Features/TranslationFlow/TranslationFlowController.swift, ScreenTranslate/Services/OverlayRenderer.swift
render 链路开始获取并传入 OverlayTheme;render API 增加 theme 参数并用主题色替代硬编码颜色;showResult 现在计算并传递 translatedText 给结果窗口。
MTran 与设置相关
ScreenTranslate/Services/MTranServerEngine.swift, ScreenTranslate/Models/TranslationEngineType.swift, ScreenTranslate/Features/Settings/EngineSettingsTab.swift, ScreenTranslate/Features/Settings/SettingsViewModel.swift, ScreenTranslate/Features/Onboarding/OnboardingConfigurationStepView.swift, ScreenTranslate/Features/Onboarding/OnboardingView.swift, ScreenTranslate/Features/Onboarding/OnboardingWindowController.swift, ScreenTranslate/Features/Onboarding/OnboardingViewModel.swift
增加 MTranServer 可配置 host/port(从 UserDefaults 读取)、多端点健康探测与丰富日志;新增 MTran 测试 UI、状态与 testMTranServerConnection();Onboarding 布局改为可滚动并将窗口高度 500→620。
本地化与 UserDefaults 键
ScreenTranslate/Resources/en.lproj/Localizable.strings, ScreenTranslate/Resources/zh-Hans.lproj/Localizable.strings, ScreenTranslate/Models/AppLanguage.swift, ScreenTranslate/Models/AppSettings.swift, ScreenTranslate/Services/HistoryStore.swift
新增/调整本地化键(如 bilingualResult.copyImage/copyText、MTran 测试消息、翻译工作流→翻译引擎);将 UserDefaults 键前缀由 ScreenCapture 统一更名为 ScreenTranslate
导出与字体微调
ScreenTranslate/Services/ImageExporter+TranslationOverlay.swift
将导出覆盖层最小字体从 10pt 提高到 14pt,影响渲染最小字号表现。

Sequence Diagram(s)

(未生成时序图)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

诗篇

🐰 我在键盘边蹦跳唱:
图像与文本各自成行,
译文静候窗中光,
旧层褪去新声响,
小胡萝卜,代码欢畅。

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 38.71% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PR标题用中文简洁地总结了主要变更:优化翻译显示和设置界面,与PR的核心目标(翻译显示优化、设置界面改进、功能新增等)高度相关。

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch featadd-translate-menu

No actionable comments were generated in the recent review. 🎉


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

Copy link
Copy Markdown

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

Caution

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

⚠️ Outside diff range comments (5)
ScreenTranslate/Models/AppSettings.swift (1)

6-48: ⚠️ Potential issue | 🟠 Major

需要设置项迁移,否则老用户配置会全部丢失。

仅变更前缀会导致读取不到旧配置。建议在初始化时批量迁移旧前缀的值到新前缀(或在读取不到新值时回退旧值并写回)。

✅ 建议迁移思路(示例)
     private enum Keys {
-        static let prefix = "ScreenTranslate."
+        static let prefix = "ScreenTranslate."
+        static let legacyPrefix = "ScreenCapture."
         static let saveLocation = prefix + "saveLocation"
         static let defaultFormat = prefix + "defaultFormat"
         ...
     }
     private init() {
         let defaults = UserDefaults.standard
+        Self.migrateIfNeeded(using: defaults)
         ...
     }
private static func migrateIfNeeded(using defaults: UserDefaults) {
    let keyPairs: [(new: String, old: String)] = [
        (Keys.saveLocation, Keys.legacyPrefix + "saveLocation"),
        (Keys.defaultFormat, Keys.legacyPrefix + "defaultFormat"),
        (Keys.jpegQuality, Keys.legacyPrefix + "jpegQuality"),
        // 其余 key 同样处理
    ]
    for pair in keyPairs {
        if defaults.object(forKey: pair.new) == nil,
           let oldValue = defaults.object(forKey: pair.old) {
            defaults.set(oldValue, forKey: pair.new)
        }
    }
}
ScreenTranslate/Services/OverlayRenderer.swift (1)

70-132: ⚠️ Potential issue | 🟠 Major

高度计算与实际渲染宽度不一致,可能导致文本被截断

总高度计算用的是全宽,而渲染时使用了减去 padding 的宽度,容易低估高度,造成底部行被裁切。建议统一使用同一宽度。

🧩 建议修复
-        // Calculate required height for translations
-        let maxTextWidth = originalWidth  // Full width, no padding
-        var totalTranslationHeight: CGFloat = 40  // Top padding
+        // Calculate required height for translations
+        let padding: CGFloat = 10
+        let maxTextWidth = originalWidth - padding * 2
+        var totalTranslationHeight: CGFloat = 40  // Top padding

         for row in rows {
             let rowText = row.segments.map { $0.translated }.joined(separator: " ")
-            let textHeight = calculateTextHeight(rowText, font: translationFont, maxWidth: maxTextWidth)
+            let textHeight = calculateTextHeight(rowText, font: translationFont, maxWidth: maxTextWidth)
             totalTranslationHeight += textHeight + lineHeight * 0.5
         }
         totalTranslationHeight += 40  // Bottom padding
@@
-        // Draw translations below with 10px padding
-        let padding: CGFloat = 10
+        // Draw translations below with 10px padding
         var currentY: CGFloat = separatorY - padding  // Start below separator
@@
-            let textHeight = calculateTextHeight(rowText, font: translationFont, maxWidth: maxTextWidth - padding * 2)
+            let textHeight = calculateTextHeight(rowText, font: translationFont, maxWidth: maxTextWidth)
@@
-                at: CGRect(x: padding, y: currentY - textHeight, width: maxTextWidth - padding * 2, height: textHeight),
+                at: CGRect(x: padding, y: currentY - textHeight, width: maxTextWidth, height: textHeight),
ScreenTranslate/Services/MTranServerEngine.swift (1)

73-114: ⚠️ Potential issue | 🟠 Major

避免记录明文翻译内容,存在隐私/合规风险。
当前日志包含用户原文与译文,可能泄露敏感信息。建议仅记录长度/摘要或使用隐私标记。

🛡️ 建议修改
-        logger.info("Starting translation: '\(text)' to \(targetLanguage)")
+        logger.info("Starting translation to \(targetLanguage), chars=\(text.count)")
...
-            logger.info("Translation successful: \(result.translatedText)")
+            logger.info("Translation successful, chars=\(result.translatedText.count)")
ScreenTranslate/Features/BilingualResult/BilingualResultViewModel.swift (2)

16-50: ⚠️ Potential issue | 🟠 Major

避免译文残留导致复制旧内容。
showResult 仅在 translatedText 非 nil 时赋值,若新结果为空或异常,会保留旧译文并允许复制,易误导用户。建议在加载/结果阶段显式清空。

🔧 建议修改
     func showLoading(originalImage: CGImage, message: String? = nil) {
         self.image = originalImage
         self.isLoading = true
         self.loadingMessage = message ?? String(localized: "bilingualResult.loading")
         self.errorMessage = nil
+        self.translatedText = ""
     }
@@
     func showResult(image: CGImage, displayScaleFactor: CGFloat? = nil, translatedText: String? = nil) {
         self.image = image
         if let sf = displayScaleFactor { self.displayScaleFactor = sf }
-        if let text = translatedText { self.translatedText = text }
+        self.translatedText = translatedText ?? ""
         self.isLoading = false
         self.loadingMessage = ""
         self.errorMessage = nil
         self.scale = 1.0
     }

86-138: ⚠️ Potential issue | 🟡 Minor

处理复制文本失败的粘贴板返回值。
NSPasteboard.setString 可能失败,当前无错误处理会误报成功。建议检查返回值并给出失败提示。

🧩 建议修改
-        pasteboard.setString(translatedText, forType: .string)
-        showCopyTextSuccess()
+        guard pasteboard.setString(translatedText, forType: .string) else {
+            errorMessage = String(localized: "error.clipboard.write.failed")
+            return
+        }
+        showCopyTextSuccess()
🤖 Fix all issues with AI agents
In `@ScreenTranslate/Features/Settings/SettingsViewModel.swift`:
- Around line 749-784: Save originalHost and originalPort then use a defer (or
equivalent finally) immediately after changing settings to ensure
settings.mtranServerHost and settings.mtranServerPort are restored regardless of
errors from MTranServerChecker.resetCache() or MTranServerChecker.isAvailable;
move the current restoration logic out of the try body into that defer so the
catch only updates mtranTestSuccess/mtranTestResult (using
error.localizedDescription) and the MainActor.run UI update still happens after
the restore.

In `@ScreenTranslate/Models/AppLanguage.swift`:
- Around line 71-74: The UserDefaults key change must preserve existing user
settings by migrating the old key; update the AppLanguage initialization to
first attempt reading the new key (languageKey = "ScreenTranslate.appLanguage"),
and if it is nil/absent read the legacy key "ScreenCapture.appLanguage", set
that value into the new key and save it back to UserDefaults so older users
don't lose their preference; reference the languageKey constant and the
AppLanguage init/loader method when implementing this fallback-and-writeback
migration logic.

In `@ScreenTranslate/Models/TranslationEngineType.swift`:
- Around line 72-129: checkMTranServer() currently blocks the caller with
DispatchSemaphore in a loop (up to 3×2.5s) causing UI/actor stalls when called
via MTranServerChecker.isAvailable (and from testMTranServerConnection() and
MTranServerEngine.translate()); change the health check to run asynchronously
and use a cached availability state to avoid repeated probes: make
checkMTranServer() async (or provide an async variant) and perform the URL
probes with async/await URLSession APIs and per-request timeouts or run the
existing probe logic on a background Task/Task.detached so it never blocks the
main actor, update MTranServerChecker.isAvailable to return quickly (either an
async call or a cached Bool) and ensure resetCache() clears the cached value so
probes only occur when truly needed.

In `@ScreenTranslate/Services/HistoryStore.swift`:
- Around line 23-24: 在 HistoryStore 中补充历史记录键的迁移:保留现有私有常量 historyKey =
"ScreenTranslate.translationHistory" 并在加载方法(例如 HistoryStore 的 load/getHistory
方法)先尝试从新 key 读取,若为空则回退尝试读取旧 key "ScreenCapture.translationHistory";如果从旧 key
成功读取,则将该数据写入新 key(使用相同的序列化/保存逻辑,例如 save/persist 方法),然后删除旧
key(UserDefaults.removeObject(forKey:)),确保后续都只使用 historyKey 并处理可能的反序列化错误。

In `@ScreenTranslate/Services/OverlayRenderer.swift`:
- Around line 26-33: OverlayTheme.current reads NSApp.effectiveAppearance which
must be accessed on the main thread; make this thread-safe by annotating the
static var current with `@MainActor` so it always runs on the main thread, or
alternatively change the OverlayRenderer.render signature to require an explicit
theme parameter (removing the implicit default use of OverlayTheme.current) and
update callers such as TranslationFlowController.performTranslation to pass the
theme from their `@MainActor` context; reference OverlayTheme.current,
NSApp.effectiveAppearance, OverlayRenderer.render, and
TranslationFlowController.performTranslation when applying the change.
- Around line 6-10: OverlayTheme currently declares conformance to Sendable but
its CGColor properties (backgroundColor, textColor, separatorColor) are not
Sendable, causing compile errors; fix by either removing ": Sendable" from
struct OverlayTheme if it does not need cross-concurrency use, or mark it as
"@unchecked Sendable" and ensure you safely wrap/validate CGColor usage (e.g.,
document invariants) if you truly must send across concurrency domains, or as a
temporary migration add "@preconcurrency import CoreGraphics" to the file;
locate the struct OverlayTheme declaration and apply one of these three fixes
consistently.
🧹 Nitpick comments (2)
ScreenTranslate/Features/Onboarding/OnboardingConfigurationStepView.swift (1)

44-72: 考虑在 URL 变更时清除测试结果

当前实现中,用户修改 mtranServerURL 后,旧的测试结果仍会显示。这可能导致用户误认为新输入的地址已通过测试。

建议在 OnboardingViewModel 中监听 mtranServerURL 的变化,自动清除 translationTestResult

♻️ 可选的改进方案(在 ViewModel 中)
var mtranServerURL: String = "" {
    didSet {
        if mtranServerURL != oldValue {
            translationTestResult = nil
            translationTestSuccess = false
        }
    }
}
ScreenTranslate/Features/Settings/SettingsViewModel.swift (1)

807-814: 端口解析可能遗漏路径部分

如果用户输入 localhost:8989/translateportString 会包含 /translate,导致 Int(portString) 解析失败并静默使用默认端口 8989。建议在提取端口前先移除路径部分。

♻️ 建议的改进
         // Split by colon for port
         if let colonIndex = hostPart.firstIndex(of: ":") {
             let host = String(hostPart[..<colonIndex])
-            let portString = String(hostPart[hostPart.index(after: colonIndex)...])
+            var portString = String(hostPart[hostPart.index(after: colonIndex)...])
+            // Remove path component if present
+            if let slashIndex = portString.firstIndex(of: "/") {
+                portString = String(portString[..<slashIndex])
+            }
             let port = Int(portString) ?? 8989
             return (host.isEmpty ? "localhost" : host, port)

Comment on lines +749 to +784
let originalHost = settings.mtranServerHost
let originalPort = settings.mtranServerPort

// Update settings for test
settings.mtranServerHost = host
settings.mtranServerPort = port

// Reset cache to use new settings
MTranServerChecker.resetCache()

// Check availability
let isAvailable = MTranServerChecker.isAvailable

// Restore original settings if test is just for checking
settings.mtranServerHost = originalHost
settings.mtranServerPort = originalPort

await MainActor.run {
mtranTestSuccess = isAvailable
if isAvailable {
mtranTestResult = NSLocalizedString("settings.translation.mtran.test.success", comment: "")
} else {
mtranTestResult = String(
format: NSLocalizedString("settings.translation.mtran.test.failed", comment: ""),
"Server not responding"
)
}
}
} catch {
await MainActor.run {
mtranTestSuccess = false
mtranTestResult = String(
format: NSLocalizedString("settings.translation.mtran.test.failed", comment: ""),
error.localizedDescription
)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

错误路径未恢复原始设置

MTranServerChecker.isAvailableresetCache() 过程中抛出异常时,catch 块不会恢复原始的 mtranServerHostmtranServerPort 设置,导致设置被意外修改。

🐛 建议的修复方案
             do {
                 // Parse URL and update settings temporarily for test
                 guard let (host, port) = parseMTranServerURL(mtranServerURL), !host.isEmpty else {
                     throw MTranServerError.invalidURL
                 }

                 // Save current settings
                 let originalHost = settings.mtranServerHost
                 let originalPort = settings.mtranServerPort

-                // Update settings for test
-                settings.mtranServerHost = host
-                settings.mtranServerPort = port
-
-                // Reset cache to use new settings
-                MTranServerChecker.resetCache()
-
-                // Check availability
-                let isAvailable = MTranServerChecker.isAvailable
-
-                // Restore original settings if test is just for checking
-                settings.mtranServerHost = originalHost
-                settings.mtranServerPort = originalPort
+                defer {
+                    // Always restore original settings
+                    settings.mtranServerHost = originalHost
+                    settings.mtranServerPort = originalPort
+                }
+
+                // Update settings for test
+                settings.mtranServerHost = host
+                settings.mtranServerPort = port
+
+                // Reset cache to use new settings
+                MTranServerChecker.resetCache()
+
+                // Check availability
+                let isAvailable = MTranServerChecker.isAvailable

                 await MainActor.run {
🤖 Prompt for AI Agents
In `@ScreenTranslate/Features/Settings/SettingsViewModel.swift` around lines 749 -
784, Save originalHost and originalPort then use a defer (or equivalent finally)
immediately after changing settings to ensure settings.mtranServerHost and
settings.mtranServerPort are restored regardless of errors from
MTranServerChecker.resetCache() or MTranServerChecker.isAvailable; move the
current restoration logic out of the try body into that defer so the catch only
updates mtranTestSuccess/mtranTestResult (using error.localizedDescription) and
the MainActor.run UI update still happens after the restore.

Comment on lines 71 to 74
// MARK: - UserDefaults Key

private let languageKey = "ScreenCapture.appLanguage"
private let languageKey = "ScreenTranslate.appLanguage"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

建议语言偏好做旧 key 迁移,避免用户语言设置被重置。

目前仅改了 key,会导致老用户回到系统语言。建议初始化时优先读取新 key,缺失时回退旧 key(ScreenCapture.appLanguage)并写回新 key。

✅ 迁移示例
-    private let languageKey = "ScreenTranslate.appLanguage"
+    private let languageKey = "ScreenTranslate.appLanguage"
+    private let legacyLanguageKey = "ScreenCapture.appLanguage"
     private init() {
-        if let savedLanguage = UserDefaults.standard.string(forKey: languageKey),
+        let defaults = UserDefaults.standard
+        let savedLanguage = defaults.string(forKey: languageKey)
+            ?? defaults.string(forKey: legacyLanguageKey)
+        if let savedLanguage,
            let language = AppLanguage(rawValue: savedLanguage) {
             currentLanguage = language
+            if defaults.string(forKey: languageKey) == nil {
+                defaults.set(savedLanguage, forKey: languageKey)
+                defaults.removeObject(forKey: legacyLanguageKey)
+            }
         } else {
             currentLanguage = .system
         }
🤖 Prompt for AI Agents
In `@ScreenTranslate/Models/AppLanguage.swift` around lines 71 - 74, The
UserDefaults key change must preserve existing user settings by migrating the
old key; update the AppLanguage initialization to first attempt reading the new
key (languageKey = "ScreenTranslate.appLanguage"), and if it is nil/absent read
the legacy key "ScreenCapture.appLanguage", set that value into the new key and
save it back to UserDefaults so older users don't lose their preference;
reference the languageKey constant and the AppLanguage init/loader method when
implementing this fallback-and-writeback migration logic.

Comment on lines 72 to +129
private static func checkMTranServer() -> Bool {
var components = URLComponents()
components.scheme = "http"
components.host = "localhost"
components.port = 8989
components.path = "/health"

guard let url = components.url else {
return false
// Read settings directly from UserDefaults to avoid MainActor isolation issues
let defaults = UserDefaults.standard
let prefix = "ScreenTranslate."
var host = defaults.string(forKey: prefix + "mtranServerHost") ?? "localhost"
let port = defaults.object(forKey: prefix + "mtranServerPort") as? Int ?? 8989

// Normalize localhost to 127.0.0.1 to avoid IPv6 resolution issues
if host == "localhost" {
host = "127.0.0.1"
}

var request = URLRequest(url: url)
request.timeoutInterval = 2.0
request.httpMethod = "GET"

let semaphore = DispatchSemaphore(value: 0)
let resultBox = ResultBox()

let task = URLSession.shared.dataTask(with: request) { _, response, _ in
resultBox.value = (response as? HTTPURLResponse)?.statusCode == 200
semaphore.signal()
let logger = Logger(subsystem: Bundle.main.bundleIdentifier ?? "ScreenTranslate", category: "MTranServerChecker")
logger.info("Checking MTranServer at \(host):\(port)")

// Try multiple endpoints for health check
let endpoints = ["/health", "/", "/translate"]
var isAvailable = false

for endpoint in endpoints {
var components = URLComponents()
components.scheme = "http"
components.host = host
components.port = port
components.path = endpoint

guard let url = components.url else { continue }

var request = URLRequest(url: url)
request.timeoutInterval = 2.0
request.httpMethod = "GET"

let semaphore = DispatchSemaphore(value: 0)
let resultBox = ResultBox()

let task = URLSession.shared.dataTask(with: request) { _, response, error in
if let error = error {
logger.debug("MTranServer check \(endpoint) failed: \(error.localizedDescription)")
}
let statusCode = (response as? HTTPURLResponse)?.statusCode ?? -1
logger.debug("MTranServer check \(endpoint): status \(statusCode)")
// Accept any response that indicates server is running (not connection refused)
resultBox.value = statusCode > 0
semaphore.signal()
}

task.resume()
_ = semaphore.wait(timeout: .now() + 2.5)

if resultBox.value {
isAvailable = true
logger.info("MTranServer available via \(endpoint)")
break
}
}

task.resume()
_ = semaphore.wait(timeout: .now() + 2.5)

return resultBox.value
logger.info("MTranServer final availability: \(isAvailable)")
return isAvailable
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# Search for references to checkMTranServer or isAvailable in Swift files
rg -n --type=swift 'checkMTranServer|\.isAvailable' -A3 -B3

Repository: hubo1989/ScreenTranslate

Length of output: 10090


🏁 Script executed:

# Also search for the property/method definition and all references to TranslationEngineType
rg -n --type=swift 'mtranServer|MTranServer' -A2 -B2

Repository: hubo1989/ScreenTranslate

Length of output: 37119


🏁 Script executed:

# Look for the property definition in TranslationEngineType
rg -n --type=swift 'var.*mtranServer|static.*mtranServer' -A5

Repository: hubo1989/ScreenTranslate

Length of output: 4181


🏁 Script executed:

# Check how MTranServerEngine.translate() is called and from where
rg -n --type=swift 'await.*mtranServerProvider\.translate|await.*primary\.translate|await.*fallback\.translate' -B5 -A2

Repository: hubo1989/ScreenTranslate

Length of output: 50


🏁 Script executed:

# Check if testMTranServerConnection is called from main thread context
rg -n --type=swift 'testMTranServerConnection' -B3 -A3

Repository: hubo1989/ScreenTranslate

Length of output: 1510


🏁 Script executed:

# Get the full testMTranServerConnection function
sed -n '736,791p' ScreenTranslate/Features/Settings/SettingsViewModel.swift

Repository: hubo1989/ScreenTranslate

Length of output: 2195


主线程同步阻塞确认,最长可导致 UI 冻结 ~7.5 秒

已验证 testMTranServerConnection()(设置界面"Test Connection"按钮)在主线程 Task 中直接调用 MTranServerChecker.isAvailable,触发 checkMTranServer() 的同步阻塞:

  • 串行探测 3 个端点,每个 2.5s 超时 = 最长 7.5s 的主线程 DispatchSemaphore.wait() 阻塞
  • MTranServerEngine.translate() 异步函数(actor 隔离)也在第 78 行同步调用该属性,阻塞 actor 执行

建议改为异步健康检查或后台线程探测,并优化缓存策略(当前 resetCache() 被多处调用会重复检测)。

🤖 Prompt for AI Agents
In `@ScreenTranslate/Models/TranslationEngineType.swift` around lines 72 - 129,
checkMTranServer() currently blocks the caller with DispatchSemaphore in a loop
(up to 3×2.5s) causing UI/actor stalls when called via
MTranServerChecker.isAvailable (and from testMTranServerConnection() and
MTranServerEngine.translate()); change the health check to run asynchronously
and use a cached availability state to avoid repeated probes: make
checkMTranServer() async (or provide an async variant) and perform the URL
probes with async/await URLSession APIs and per-request timeouts or run the
existing probe logic on a background Task/Task.detached so it never blocks the
main actor, update MTranServerChecker.isAvailable to return quickly (either an
async call or a cached Bool) and ensure resetCache() clears the cached value so
probes only occur when truly needed.

Comment on lines 23 to +24
/// UserDefaults key for history data
private static let historyKey = "ScreenCapture.translationHistory"
private static let historyKey = "ScreenTranslate.translationHistory"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

补充历史记录键迁移,避免用户历史丢失。

仅切换到新 key 会导致老用户历史记录读取不到。建议在加载时回退读取旧 key(如 ScreenCapture.translationHistory),并成功读取后迁移写回新 key,同时清理旧 key。

✅ 建议迁移方案(示例)
+    private static let legacyHistoryKey = "ScreenCapture.translationHistory"
     private static let historyKey = "ScreenTranslate.translationHistory"
     private func loadHistory() {
-        guard let data = UserDefaults.standard.data(forKey: Self.historyKey) else {
-            entries = []
-            return
-        }
+        let defaults = UserDefaults.standard
+        let data = defaults.data(forKey: Self.historyKey)
+            ?? defaults.data(forKey: Self.legacyHistoryKey)
+        guard let data else {
+            entries = []
+            return
+        }
 
         if let decoded = try? JSONDecoder().decode([TranslationHistory].self, from: data) {
             entries = decoded
+            // 若来自旧 key,迁移到新 key
+            if defaults.data(forKey: Self.historyKey) == nil,
+               defaults.data(forKey: Self.legacyHistoryKey) != nil {
+                saveHistory()
+                defaults.removeObject(forKey: Self.legacyHistoryKey)
+            }
         } else {
             entries = []
         }
     }
📝 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
/// UserDefaults key for history data
private static let historyKey = "ScreenCapture.translationHistory"
private static let historyKey = "ScreenTranslate.translationHistory"
/// UserDefaults key for history data
private static let legacyHistoryKey = "ScreenCapture.translationHistory"
private static let historyKey = "ScreenTranslate.translationHistory"
🤖 Prompt for AI Agents
In `@ScreenTranslate/Services/HistoryStore.swift` around lines 23 - 24, 在
HistoryStore 中补充历史记录键的迁移:保留现有私有常量 historyKey =
"ScreenTranslate.translationHistory" 并在加载方法(例如 HistoryStore 的 load/getHistory
方法)先尝试从新 key 读取,若为空则回退尝试读取旧 key "ScreenCapture.translationHistory";如果从旧 key
成功读取,则将该数据写入新 key(使用相同的序列化/保存逻辑,例如 save/persist 方法),然后删除旧
key(UserDefaults.removeObject(forKey:)),确保后续都只使用 historyKey 并处理可能的反序列化错误。

Comment on lines +6 to +10
/// Color theme for overlay rendering
struct OverlayTheme: Sendable {
let backgroundColor: CGColor
let textColor: CGColor
let separatorColor: CGColor
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's find and examine the OverlayRenderer.swift file
find . -name "OverlayRenderer.swift" -type f

Repository: hubo1989/ScreenTranslate

Length of output: 116


🌐 Web query:

CGColor Swift Sendable protocol conformance strict concurrency

💡 Result:

CGColor (like many CoreFoundation/CoreGraphics reference types) is not declared Sendable, so with Strict Concurrency (targeted/complete) you’ll get warnings/errors when you store it in a Sendable type, capture it in @Sendable closures, or move it across actor/task boundaries. This is the same class of issue people have hit with CGImage under Swift 6 strict checking. [1]

Fix options (pick based on what you’re actually guaranteeing)

  1. Keep CGColor actor-isolated (preferred when it’s UI state)
  • Put the CGColor behind @MainActor (or inside an actor) and don’t send it across concurrency domains.
  1. Wrap it as @unchecked Sendable (you take responsibility)
    If you know your usage is safe (e.g., you treat it as immutable and you don’t share mutable state), wrap it:
import CoreGraphics

struct SendableCGColor: `@unchecked` Sendable {
    let cgColor: CGColor
}

This is the standard “escape hatch” pattern to silence Sendable failures when the compiler can’t prove safety. [2]

  1. Suppress warnings from imported modules (migration aid)
    If the warning is primarily fallout from CoreGraphics types not being annotated the way strict checking wants, you can suppress those diagnostics using @preconcurrency on the import:
`@preconcurrency` import CoreGraphics

This is meant for incremental migration / when you can’t fix the upstream module’s annotations. [3]


Sources: Swift Forums discussion showing strict-concurrency issues even for CGImage (a similar CG type) [1]; example pattern using an @unchecked Sendable wrapper to silence strict-concurrency captures [2]; explanation of @preconcurrency import ... to suppress Sendable-related warnings from a module [3].

[1] (forums.swift.org)
[2] (jessesquires.com)
[3] (donnywals.com)

Citations:


🏁 Script executed:

# Let's read the OverlayRenderer.swift file to see the actual implementation
cat -n ./ScreenTranslate/Services/OverlayRenderer.swift | head -50

Repository: hubo1989/ScreenTranslate

Length of output: 1922


🏁 Script executed:

# Check for Swift version and concurrency settings
find . -name "Package.swift" -o -name "project.pbxproj" -o -name ".swiftpm" -type f | head -5

Repository: hubo1989/ScreenTranslate

Length of output: 111


🏁 Script executed:

# Check if there are any build settings or swiftSettings related to concurrency
rg -i "strict.*concurrency|concurrency.*strict" --max-count=10

Repository: hubo1989/ScreenTranslate

Length of output: 669


OverlayTheme 中移除 Sendable 或使用 @unchecked Sendable

CGColor 在 Swift 并发模型中不符合 Sendable 协议。由于项目使用 Swift 6.2.3 且启用了 SWIFT_STRICT_CONCURRENCY = complete,当前的 struct OverlayTheme: Sendable 声明会产生编译错误。建议采用以下任一方案:

  1. 移除 Sendable 符合性(如果主题不需要跨并发域传递)
  2. 使用 @unchecked Sendable 并包装 CGColor(如果确实需要跨域传递)
  3. 使用 @preconcurrency import CoreGraphics(临时迁移方案)
🔧 方案 1:移除 Sendable
-struct OverlayTheme: Sendable {
+struct OverlayTheme {
📝 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
/// Color theme for overlay rendering
struct OverlayTheme: Sendable {
let backgroundColor: CGColor
let textColor: CGColor
let separatorColor: CGColor
/// Color theme for overlay rendering
struct OverlayTheme {
let backgroundColor: CGColor
let textColor: CGColor
let separatorColor: CGColor
🤖 Prompt for AI Agents
In `@ScreenTranslate/Services/OverlayRenderer.swift` around lines 6 - 10,
OverlayTheme currently declares conformance to Sendable but its CGColor
properties (backgroundColor, textColor, separatorColor) are not Sendable,
causing compile errors; fix by either removing ": Sendable" from struct
OverlayTheme if it does not need cross-concurrency use, or mark it as
"@unchecked Sendable" and ensure you safely wrap/validate CGColor usage (e.g.,
document invariants) if you truly must send across concurrency domains, or as a
temporary migration add "@preconcurrency import CoreGraphics" to the file;
locate the struct OverlayTheme declaration and apply one of these three fixes
consistently.

Comment thread ScreenTranslate/Services/OverlayRenderer.swift
- OverlayRenderer: fix height calculation to use padded width consistently
- OverlayRenderer: add @mainactor to OverlayTheme.current for thread safety
- MTranServerEngine: log character count instead of plaintext for privacy
- BilingualResultViewModel: clear translatedText when loading new content
- BilingualResultViewModel: check pasteboard.setString return value
- OnboardingViewModel: clear test result when URL changes
- OnboardingViewModel: fix port parsing to handle path component
- SettingsViewModel: clear test result when URL changes
- SettingsViewModel: fix port parsing to handle path component
- TranslationFlowController: pass theme explicitly to render method

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

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

Caution

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

⚠️ Outside diff range comments (3)
ScreenTranslate/Features/Onboarding/OnboardingViewModel.swift (1)

184-194: ⚠️ Potential issue | 🟡 Minor

测试连接时临时修改设置的模式存在风险。

testTranslation() 中,临时修改 settings.mtranServerHostsettings.mtranServerPort,然后在翻译后恢复。如果 MTranServerEngine.shared.translate 抛出异常,原始设置将不会被恢复。建议使用 defer 来确保设置始终被恢复。

🛡️ 建议使用 defer 确保恢复设置
             if let (host, port) = parseServerURL(mtranServerURL), !host.isEmpty {
                 let originalHost = settings.mtranServerHost
                 let originalPort = settings.mtranServerPort
+                defer {
+                    settings.mtranServerHost = originalHost
+                    settings.mtranServerPort = originalPort
+                }
                 settings.mtranServerHost = host
                 settings.mtranServerPort = port

                 let result = try await MTranServerEngine.shared.translate(testText, to: "zh")

-                settings.mtranServerHost = originalHost
-                settings.mtranServerPort = originalPort
-
                 translationTestResult = String(
ScreenTranslate/Services/MTranServerEngine.swift (1)

169-174: ⚠️ Potential issue | 🟡 Minor

resolveSourceLanguage 方法的两个分支返回相同值。

无论 autoDetect 参数为 true 还是 false,方法都返回 "auto"。如果 autoDetectfalse 时应该有不同的行为,请修正此逻辑;否则可以简化此方法。

🐛 建议的修复(如果两种情况应不同)
 private func resolveSourceLanguage(_ source: String?, autoDetect: Bool) -> String {
     if let source = source, !source.isEmpty {
         return source
     }
-    return autoDetect ? "auto" : "auto"
+    return autoDetect ? "auto" : "en"  // 或其他合理的默认语言
 }

或者简化为:

 private func resolveSourceLanguage(_ source: String?, autoDetect: Bool) -> String {
     if let source = source, !source.isEmpty {
         return source
     }
-    return autoDetect ? "auto" : "auto"
+    return "auto"
 }
ScreenTranslate/Services/OverlayRenderer.swift (1)

226-231: ⚠️ Potential issue | 🟡 Minor

避免强制解包 CFAttributedStringCreate 返回值

CFAttributedStringCreate 在极端情况下(如内存不足)可能返回 nil,强制解包会导致崩溃。建议使用可选绑定并提前返回。

🔧 建议修复
-        let attrString = CFAttributedStringCreate(
+        guard let attrString = CFAttributedStringCreate(
             nil,
             text as CFString,
             attributes as CFDictionary
-        )!
+        ) else { return }

         let framesetter = CTFramesetterCreateWithAttributedString(attrString)
🤖 Fix all issues with AI agents
In `@ScreenTranslate/Services/OverlayRenderer.swift`:
- Around line 125-132: The code double-subtracts padding: maxTextWidth is
already originalWidth - padding * 2 but the calls to calculateTextHeight and to
CGRect(width:) subtract padding * 2 again; update the calls in
OverlayRenderer.swift (the calculateTextHeight(...) invocation and the
renderTranslationBlock(...) CGRect width parameter) to use maxTextWidth directly
(remove the extra "- padding * 2") while keeping the x origin as padding,
matching the approach used in renderSideBySideHorizontal; ensure you still pass
translationFont and theme.textColor to renderTranslationBlock.
🧹 Nitpick comments (3)
ScreenTranslate/Features/Onboarding/OnboardingViewModel.swift (1)

241-264: 与 SettingsViewModel 存在重复的 URL 解析逻辑。

parseServerURL 方法在 OnboardingViewModel (Lines 241-264) 和 SettingsViewModel (Lines 799-822) 中几乎完全相同。建议将此逻辑提取到一个共享的工具函数或扩展中,以避免代码重复和潜在的维护问题。

♻️ 建议提取共享工具函数

创建一个共享的解析函数,例如在一个 URLParsingUtility 中:

enum URLParsingUtility {
    /// Parses server URL to extract host and port
    /// - Parameter url: URL string (e.g., "localhost:8989", "http://192.168.1.1:8989/path")
    /// - Returns: Tuple of (host, port) or nil if invalid
    static func parseServerURL(_ url: String, defaultPort: Int = 8989) -> (host: String, port: Int)? {
        let trimmed = url.trimmingCharacters(in: .whitespacesAndNewlines)
        guard !trimmed.isEmpty else { return nil }

        var hostPart = trimmed
        if hostPart.hasPrefix("http://") {
            hostPart = String(hostPart.dropFirst(7))
        } else if hostPart.hasPrefix("https://") {
            hostPart = String(hostPart.dropFirst(8))
        }

        if let colonIndex = hostPart.firstIndex(of: ":") {
            let host = String(hostPart[..<colonIndex])
            let portAndPath = String(hostPart[hostPart.index(after: colonIndex)...])
            let portString = portAndPath.prefix { $0.isNumber }
            let port = Int(portString) ?? defaultPort
            return (host.isEmpty ? "localhost" : host, port)
        } else {
            return (hostPart.isEmpty ? "localhost" : hostPart, defaultPort)
        }
    }
}
ScreenTranslate/Services/MTranServerEngine.swift (1)

76-81: 每次翻译前重置缓存可能影响性能。

在每次翻译请求前调用 MTranServerChecker.resetCache() 会导致每次都重新检查服务器可用性。如果翻译请求频繁(如批量翻译),这可能会增加不必要的网络开销。

建议考虑:

  1. 仅在设置变更时重置缓存(已在 didSet 中实现)
  2. 或者引入缓存过期时间机制
ScreenTranslate/Features/Settings/SettingsViewModel.swift (1)

798-822: 与 OnboardingViewModel 存在重复代码。

parseMTranServerURL 方法与 OnboardingViewModel.parseServerURL 几乎完全相同。如前所述,建议提取为共享工具函数。

Comment thread ScreenTranslate/Services/OverlayRenderer.swift Outdated
maxTextWidth is already calculated as originalWidth - padding * 2,
so the calls to calculateTextHeight and renderTranslationBlock should
use maxTextWidth directly instead of subtracting padding again.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@hubo1989 hubo1989 merged commit 660677e into main Feb 12, 2026
1 check passed
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.

1 participant