feat: integrate model discovery and fix keychain authorization popups#74
Merged
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 41 minutes and 23 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
📝 WalkthroughWalkthrough本PR为ScreenTranslate新增模型发现服务与权限授权流程，增强翻译管道语言检测与输出清洁化，重构Keychain凭证存储以支持DEBUG调试模式，并改进VLM供应商的请求/响应处理与localhost本地开发支持。 Changes模型发现与引擎配置
权限与安全
翻译管道与供应商增强
测试与本地化
构建配置
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ScreenTranslate/Features/Settings/EngineConfigSheet.swift (1)
393-416:⚠️ Potential issue | 🟠 Major | ⚡ Quick win连接测试未覆盖当前未保存的表单配置
Line 415 仅按
engine调用verifyConnection，但当前界面里的baseURL/modelName还在本地状态（Line 348-349），未写回config。这会出现“测试通过但保存后失败”或反向情况。建议测试前构建临时配置并显式传入测试逻辑。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ScreenTranslate/Features/Settings/EngineConfigSheet.swift` around lines 393 - 416, testConnection() currently calls TranslationService.shared.verifyConnection(for: engine) which only uses saved config, ignoring the unsaved local state for baseURL/modelName/apiKey/secretKey/appID in the sheet; construct a temporary EngineConfig (or a dictionary) from the local state fields (baseURL, modelName, apiKey, secretKey, appID) and pass it into the verification logic instead of relying on persisted config—either call an existing verifyConnection overload that accepts a config or add one (e.g., verifyConnection(for:engine, config: tempConfig)); ensure you use temporary credentials for the test (do not persist them) and only save to Keychain when the user explicitly saves.
🟡 Minor comments (15)
ScreenTranslate/Resources/es.lproj/Localizable.strings-825-826 (1)
825-826:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win请改为西班牙语文案，避免语言回退策略失效。
Line 825 和 Line 826 在西语资源里仍是英文，这会让西语用户看到混合语言文本。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ScreenTranslate/Resources/es.lproj/Localizable.strings` around lines 825 - 826, 两条本地化键 engine.config.fetchModels 和 engine.config.fetchModels.failed 目前值仍为英文，导致西班牙语界面出现混合语言；将这两项替换为对应的西班牙语翻译（例如 "engine.config.fetchModels" = "Obtener"; "engine.config.fetchModels.failed" = "Error al obtener los modelos"; 或根据产品术语表使用合适译文），并确保保存到 es.lproj/Localizable.strings，避免回退到英文。ScreenTranslate/Resources/de.lproj/Localizable.strings-825-826 (1)
825-826:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win请将新增键值本地化为德语，避免界面语言混杂。
Line 825 和 Line 826 目前仍是英文，德语 UI 下会直接显示英文按钮/错误提示，影响一致性。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ScreenTranslate/Resources/de.lproj/Localizable.strings` around lines 825 - 826, 文件中两条本地化键 "engine.config.fetchModels" 和 "engine.config.fetchModels.failed" 仍为英文，需将它们翻译为德语以避免界面语言混杂；请在 ScreenTranslate/Resources/de.lproj/Localizable.strings 中替换这两个键的值为德语对应文本（例如 "Abrufen" 或更合适的上下文词，以及 "Fehler beim Abrufen der Modelle" 或同义的德语错误提示），确保键名不变只修改右侧字符串并遵循现有本地化风格和语气。ScreenTranslate/Resources/ko.lproj/Localizable.strings-825-826 (1)
825-826:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win韩语资源中的新增键需要本地化。
Line 825 和 Line 826 当前是英文，会在韩语界面直接显示英文文案。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ScreenTranslate/Resources/ko.lproj/Localizable.strings` around lines 825 - 826, 这两条资源键 "engine.config.fetchModels" 和 "engine.config.fetchModels.failed" 在韩语词条中仍为英文，请在 ko.lproj 的本地化文件中为这两个键提供对应的韩文翻译（替换当前英文值），确保字符串值使用正确的韩语文本并保持与其他条目相同的格式和编码，以便韩语界面显示本地化文案。ScreenTranslate/Resources/ja.lproj/Localizable.strings-825-826 (1)
825-826:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win请补齐日语翻译，避免新增按钮/错误提示显示英文。
Line 825 和 Line 826 在日语资源中仍为英文，建议改为日语文本。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ScreenTranslate/Resources/ja.lproj/Localizable.strings` around lines 825 - 826, 资源文件中的两个键 "engine.config.fetchModels" 和 "engine.config.fetchModels.failed" 在日语本地化仍为英文；请在 ScreenTranslate/Resources/ja.lproj/Localizable.strings 中找到这两个键并将其值替换为日语翻译（例如 "モデルを取得" 或更自然的 "モデルを取得する" 用于按钮，"モデルの取得に失敗しました" 用于错误提示），确保按钮和错误提示在日语环境下不再显示英文。ScreenTranslate/Resources/fr.lproj/Localizable.strings-825-826 (1)
825-826:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win新增键请使用法语翻译，当前会破坏法语界面一致性。
Line 825 和 Line 826 为英文文案，建议替换为法语对应文本。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ScreenTranslate/Resources/fr.lproj/Localizable.strings` around lines 825 - 826, Replace the English values for the localization keys "engine.config.fetchModels" and "engine.config.fetchModels.failed" with proper French translations so the French locale remains consistent: change "Fetch" to a French equivalent (e.g., "Récupérer") and "Failed to fetch models" to a French sentence (e.g., "Échec de la récupération des modèles" or "Impossible de récupérer les modèles") for the keys "engine.config.fetchModels" and "engine.config.fetchModels.failed" respectively.ScreenTranslate/Resources/it.lproj/Localizable.strings-826-827 (1)
826-827:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win请将新增字符串本地化为意大利语。
Line 826 和 Line 827 目前是英文，意大利语环境下会显示不一致的语言。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ScreenTranslate/Resources/it.lproj/Localizable.strings` around lines 826 - 827, 替换这两个键的英文字符串为意大利语：将 "engine.config.fetchModels" 的值从 "Fetch" 修改为意大利语（例如 "Recupera" 或 "Ottieni"），并将 "engine.config.fetchModels.failed" 的值从 "Failed to fetch models" 修改为意大利语（建议 "Impossibile recuperare i modelli" 或相近表述），确保只更新这两个键的右侧字符串以保持文件其它条目不变。ScreenTranslate/Resources/pt.lproj/Localizable.strings-840-841 (1)
840-841:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win请将新增键值翻译为葡萄牙语。
Line 840 和 Line 841 仍是英文，会导致葡语 UI 的文案不统一。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ScreenTranslate/Resources/pt.lproj/Localizable.strings` around lines 840 - 841, 两个新添加的本地化键 engine.config.fetchModels 和 engine.config.fetchModels.failed 仍为英文，按葡萄牙语本地化替换即可：将 "engine.config.fetchModels" 的值改为 "Buscar modelos"（或 "Buscar" 视上下文为按钮简短文案），将 "engine.config.fetchModels.failed" 的值改为 "Falha ao buscar modelos"（或 "Falha ao obter modelos"），确保用双引号包裹并保持与现有 Localizable.strings 格式一致。ScreenTranslate/Resources/ru.lproj/Localizable.strings-825-826 (1)
825-826:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win俄语本地化键值仍是英文。
这会导致俄语界面出现英文文案，建议改为俄语翻译。
建议修复
-"engine.config.fetchModels" = "Fetch"; -"engine.config.fetchModels.failed" = "Failed to fetch models"; +"engine.config.fetchModels" = "Получить"; +"engine.config.fetchModels.failed" = "Не удалось получить модели";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ScreenTranslate/Resources/ru.lproj/Localizable.strings` around lines 825 - 826, The Russian localization strings for the keys "engine.config.fetchModels" and "engine.config.fetchModels.failed" are still in English; update their values to proper Russian translations (e.g., set "engine.config.fetchModels" to a Russian word for "Fetch" like "Загрузить" and "engine.config.fetchModels.failed" to a Russian phrase for "Failed to fetch models" like "Не удалось загрузить модели") so the ru.lproj Localizable.strings file shows Russian text for these keys.ScreenTranslate/Features/Settings/CompatibleEngineConfigSheet.swift-320-324 (1)
320-324:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win拉取失败后应清空旧模型列表
Line 320-324 失败分支仅弹错，不会清空
availableModels。这会让 UI 继续展示旧数据，用户可能误选过期模型。建议在catch中将availableModels = []。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ScreenTranslate/Features/Settings/CompatibleEngineConfigSheet.swift` around lines 320 - 324, The catch block in CompatibleEngineConfigSheet.swift currently only sets errorMessage, showErrorAlert and isFetchingModels but does not clear the old model list; update the catch handler to also set availableModels = [] (reset the property on the view) so the UI won't display stale models when the fetch fails, keeping the existing errorMessage, showErrorAlert and isFetchingModels updates intact.ScreenTranslate/Features/Settings/EngineConfigSheet.swift-446-450 (1)
446-450:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win拉取失败后应清空旧模型列表
Line 446-450 失败分支未清空
availableModels，会遗留旧结果。建议在catch中重置为空，避免配置误导。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ScreenTranslate/Features/Settings/EngineConfigSheet.swift` around lines 446 - 450, 在对可用模型拉取的异常分支（当前 catch 块，已设置 self.errorMessage / self.showErrorAlert / self.isFetchingModels）中清空旧结果：将 self.availableModels 重置为一个空数组或合适的初始值，以避免在拉取失败后保留过时的模型列表并误导配置界面；在 EngineConfigSheet 的该 catch 分支中加入这一步骤即可。ScreenTranslate/Features/Settings/EngineSettingsTab.swift-120-133 (1)
120-133:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win模型菜单标签为空，交互可发现性偏弱
Line 128-130 与 Line 333-335 的
Menu使用空文本标签，用户不易理解其用途。建议改成可见图标（如下拉箭头）并补充可访问性描述。Also applies to: 325-338
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ScreenTranslate/Features/Settings/EngineSettingsTab.swift` around lines 120 - 133, The Menu currently uses an empty Text label which makes the control invisible and hard to discover; replace the empty label with a visible affordance such as a Label or Image (e.g. Image(systemName: "chevron.down") or Label("Model", systemImage: "chevron.down")) and set accessibility modifiers so VoiceOver reads its purpose; update the Menu instances that iterate availableVLMModels (the Menu that sets viewModel.vlmModelName) to use the visible icon/label and add .accessibilityLabel("Select VLM model") or a localized string and .accessibilityHint(...) to improve discoverability.ScreenTranslate/Features/Settings/EngineConfigSheet.swift-274-287 (1)
274-287:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win模型下拉入口可访问性不足
Line 282-284 的
Menu标签为空文本且区域很小，建议改为可见图标并添加可访问性标签，避免用户难以发现模型选择入口。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ScreenTranslate/Features/Settings/EngineConfigSheet.swift` around lines 274 - 287, The model selection Menu uses an invisible tiny label (Text("") with a small frame), making the entry hard to discover; replace the empty Text label with a visible tappable icon (e.g., an Image system icon like a chevron or gear) and increase its tappable frame, update the Menu label in the same block that wraps ForEach(availableModels, id: \.self) and Button(model) so users can see and hit it, and add accessibility modifiers (e.g., accessibilityLabel / accessibilityIdentifier and accessibilityAddTraits(.isButton)) targeting the Menu label so VoiceOver/screen readers announce it while preserving the existing .menuStyle(.borderlessButton) and modelName assignment logic.ScreenTranslate/Features/Settings/CompatibleEngineConfigSheet.swift-84-97 (1)
84-97:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win模型下拉入口可访问性不足
Line 92-94 的
Menu标签是空文本且点击区域极小，用户很难发现该入口。建议使用明确图标（如chevron.down）并补充可访问性标签。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ScreenTranslate/Features/Settings/CompatibleEngineConfigSheet.swift` around lines 84 - 97, The Menu's label is an empty Text with a tiny hit area, which hurts discoverability and accessibility; replace the empty label with a clear tappable icon (e.g., Image(systemName: "chevron.down")) and increase its touch target (padding or larger frame), then add accessibility modifiers (e.g., .accessibilityLabel("Select model") and an optional .accessibilityHint) so VoiceOver users can find and understand the control; keep the existing Menu, ForEach(availableModels, id: \.self) and modelName assignment logic unchanged.ScreenTranslate/Features/Settings/EngineSettingsTab.swift-224-228 (1)
224-228:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win模型拉取失败后建议清空旧列表
Line 224-228 和 Line 449-452 的失败分支没有清空已有模型，可能继续显示上一次成功结果。建议在两处
catch中重置对应available...Models = []。Also applies to: 449-452
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ScreenTranslate/Features/Settings/EngineSettingsTab.swift` around lines 224 - 228, In each catch block where you set self.errorMessage, self.showErrorAlert, and self.isFetchingVLMModels (e.g., the catch at EngineSettingsTab.swift lines around the isFetchingVLMModels flow), also reset the model list to an empty array by assigning self.availableVLMModels = []; likewise, in the other failure catch (the block around lines 449-452) clear the corresponding list (e.g., self.availableLLMModels = []) so stale models aren’t shown after a failed fetch.ScreenTranslate/Services/OpenAIVLMProvider.swift-349-383 (1)
349-383:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win手工字段提取的括号深度逻辑会误伤字符串边界
在 Line 364 到 Line 372，
extractField对{/[维护depth，会让字符串里的普通字符影响终止条件。若内容中出现未配对花括号，Line 371 的结束引号不会被及时识别，可能提取过长内容并导致后续解析失败。建议修复
private func extractField(from json: String, pattern: String) -> String? { guard let range = json.range(of: pattern) else { return nil } let start = range.upperBound var end = start var escaped = false - var depth = 0 for char in json[start...] { if escaped { escaped = false end = json.index(after: end) } else if char == "\\" { escaped = true end = json.index(after: end) - } else if char == "{" || char == "[" { - depth += 1 - end = json.index(after: end) - } else if char == "}" || char == "]" { - depth -= 1 - end = json.index(after: end) - if depth < 0 { break } - } else if char == "\"" && depth == 0 { + } else if char == "\"" { break } else { end = json.index(after: end) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ScreenTranslate/Services/OpenAIVLMProvider.swift` around lines 349 - 383, The loop in extractField(from:pattern:) misinterprets braces inside string literals because it only tracks depth and not whether we're inside a quoted string; update the loop to track an inString Bool (alongside escaped) so you only increment/decrement depth when inString == false, and toggle inString when encountering an unescaped quote; then treat the end condition as "encounter an unescaped quote that closes the top-level string (inString becomes false and depth == 0)" so the break happens correctly. Ensure you reference the existing variables (start, end, escaped, depth) and add inString to control when '{', '}', '[' and ']' affect depth and when '"' toggles string state.
🧹 Nitpick comments (5)
create_dmg.sh (1)
38-38: ⚡ Quick win确认：
set -e不会在该写法下因find非零而提前退出，但仍会产生 stderr 噪音本仓库中
APP_PATH=$(find ... | head -n 1)属于“命令替换 + 赋值”的场景；在 bash 下该写法不会因为find在部分目录不存在时返回非零而导致脚本提前退出（只会继续执行）。不过当目录不存在时仍会打印find: ... No such file or directory到 stderr，建议静音该find的错误输出（如重定向到/dev/null或忽略错误），以避免日志干扰。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@create_dmg.sh` at line 38, The APP_PATH assignment uses command substitution with find (the APP_PATH=$(find "$PROJECT_PATH/build" ... | head -n 1)) which can emit noisy "No such file or directory" messages to stderr when some searched paths are absent; update the find invocation to suppress its error output (e.g., redirect find's stderr to /dev/null or otherwise ignore errors) so the command substitution still returns the first match but does not print spurious errors to stderr while preserving existing behavior when set -e is enabled.ScreenTranslateTests/ModelDiscoveryServiceTests.swift (2)
99-105: ⚡ Quick win避免用错误文案子串做断言。
localizedDescription.contains("empty")对文案变更很敏感，建议断言具体错误类型/枚举 case（或稳定错误码）。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ScreenTranslateTests/ModelDiscoveryServiceTests.swift` around lines 99 - 105, The test testEmptyURLThrowsError relies on string matching of error.localizedDescription; change it to assert the concrete error type/enum case returned by ModelDiscoveryService.fetchModels instead. Update the catch to cast the thrown error to the specific error type (e.g., ModelDiscoveryError) and assert the exact case (for example .emptyURL or the appropriate case defined on ModelDiscoveryError), or compare against a stable error value/errno if your service exposes one, so the test checks ModelDiscoveryService.fetchModels for the expected error case rather than a substring of localizedDescription.
8-95: 🏗️ Heavy lift这两条解析测试未真正覆盖
ModelDiscoveryService的解析逻辑。目前只验证了测试内临时
Codable结构体能解码，生产代码改动时这两条用例可能仍会误通过。建议改为直接调用ModelDiscoveryService的可测试入口（或通过可注入网络层构造响应），断言最终模型列表。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ScreenTranslateTests/ModelDiscoveryServiceTests.swift` around lines 8 - 95, The tests currently only decode local temporary Codable structs instead of exercising the real parsing in ModelDiscoveryService; update testParseOpenAIModelsResponse and testParseOllamaTagsResponse to call the ModelDiscoveryService parsing entry point (or instantiate ModelDiscoveryService with a mock/injectable network layer that returns the JSON data) and assert the returned model list contents and order; locate parsing logic on the ModelDiscoveryService type (e.g., a parseModels/fromData or fetchAvailableModels method) and use that API so the tests validate production parsing behavior rather than the local test-only Codable structs.ScreenTranslateTests/TranslationServicePipelineTests.swift (1)
770-783: ⚡ Quick win
testPrintRealUserSettings不是有效测试用例。该方法只有打印没有断言，建议删除，或改成明确可重复的断言测试。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ScreenTranslateTests/TranslationServicePipelineTests.swift` around lines 770 - 783, The testPrintRealUserSettings method is not a valid test because it only prints values; either remove this method or replace it with a deterministic, repeatable test: obtain AppSettings.shared inside MainActor.run (keep the existing reference to AppSettings.shared and the same properties ocrEngine, translationEngine, translationTargetLanguage, translationSourceLanguage, translationFallbackEnabled, engineSelectionMode, vlmProvider), set or reset AppSettings to a known state before reading, and add XCTAssert assertions for each expected property value (instead of printing) so the test is deterministic and repeatable; if you prefer deletion, simply remove the testPrintRealUserSettings function.ScreenTranslate/Models/AppSettings.swift (1)
743-783: ⚡ Quick win避免在
AppSettings里复制KeychainService的存储协议。这两段把 debug key 前缀、
StoredCredentials编码格式和 Keychain 查询细节又实现了一遍。现在凭证读取规则已经散落在两个文件里，后面只要改一次存储 key 或序列化格式，DEBUG/Release 就很容易出现一边能写一边读不出来的静默漂移。更稳妥的做法是把共享常量或读取 helper 收敛回KeychainService，让这里走单一入口。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ScreenTranslate/Models/AppSettings.swift` around lines 743 - 783, The duplicated Keychain read logic in loadPaddleOCRAPIKeyFromKeychain and loadVLMAPIKeyFromKeychain should be removed and replaced with a single call into KeychainService (or a new helper on KeychainService) so storage keys, JSON encoding for StoredCredentials and query parameters are centralized; update these two functions to call KeychainService.getStoredCredentials(account:)/KeychainService.apiKey(forAccount:) (or similar) and return credentials.apiKey, keeping DEBUG behavior delegated to KeychainService as well so key prefixes and serialization aren’t duplicated in AppSettings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ScreenTranslate/Features/Settings/CompatibleEngineConfigSheet.swift`:
- Line 289: 当前连通性测试直接调用
provider.translate(...)（provider.translate），会发起真实推理请求产生费用与延迟；请替换为轻量的连通性检查：在
Provider 协议/类中新增一个无副作用的方法（例如
verifyConnection()/ping()/healthCheck()），让各实现返回仅校验认证与可达性的结果，并在
CompatibleEngineConfigSheet.swift 中把原来调用 provider.translate("Hello", from: "en",
to: "zh") 的逻辑改为调用该新方法；如果某些 provider 目前无法实现完整 ping，可实现为调用一个内部 metadata/health
路由或模拟快速认证请求，确保不触发真实推理调用。
In `@ScreenTranslate/Services/ModelDiscoveryService.swift`:
- Around line 47-49: The code currently always prefixes cleanURL with "https://"
in the ModelDiscoveryService code path (the if block that checks
cleanURL.lowercased().hasPrefix("http://") / "https://"), which breaks local
Ollama connections that use plain HTTP; update that logic to detect local hosts
(e.g., hostnames/hosts like "localhost", "127.0.0.1", "::1", or host:port forms)
and prefix "http://" for those, otherwise continue to default to "https://".
Locate the cleanURL handling in ModelDiscoveryService (the cleanURL variable and
its protocol-prefixing if-block) and implement host detection (or use
URLComponents parsing) to decide which scheme to prepend. Ensure existing
.lowercased() checks are preserved for non-local URLs and that ported localhost
forms (e.g., "localhost:11434") are recognized as local.
In `@ScreenTranslate/Services/PermissionManager.swift`:
- Around line 31-35: 当前实现重用单个 flowController 和 单一 flowPollingTask
导致不同权限流程互相覆盖；修改为为每次发起的权限流程创建独立的 PermissionFlowController 实例并且为该实例维护各自的
Task（不要复用类级别的 flowController/flowPollingTask），或者使用基于权限类型的字典（keyed by permission
identifier）来存储 PermissionFlowController 和对应的 polling Task，启动新流程时只取消与该权限类型关联的旧
Task/Controller；在代码中调整使用到的符号包括 flowController, flowPollingTask,
PermissionFlowController（以及发起/取消轮询的相关方法）以确保取消/刷新只影响对应权限流程。
In `@ScreenTranslate/Services/Security/KeychainService.swift`:
- Around line 268-284: The existsRaw(account:) helper currently treats any
non-errSecSuccess as "not configured"; change it to distinguish
errSecItemNotFound from other Keychain errors: in existsRaw(account:) (the
function with the SecItemCopyMatching call) return false only for
errSecItemNotFound, and for any other non-success status either log the status
(e.g., via your app logger) with context including the account and service or
propagate the error to the caller (throw or return a Result/optional with error)
so callers like MultiEngineSettingsSection can handle auth/interaction failures
correctly. Ensure the `#if` DEBUG branch remains unchanged and only modify the
SecItemCopyMatching handling to check for errSecItemNotFound versus other status
codes.
In
`@ScreenTranslate/Services/Translation/Providers/CompatibleTranslationProvider.swift`:
- Around line 321-323: The logger calls in CompatibleTranslationProvider
(references: displayString, trimmedResponse, jsonString, streamText, and
logger.error) currently write raw response fragments to logs; change those error
logs to omit any response body or truncated content and instead log only safe
metadata (e.g., HTTP status code, response byte count/length, and a request or
correlation ID if available). Locate the three error sites (around the JSON
serialization error using displayString, and the other two sites that reference
jsonString and streamText) and replace the message payload with a concise,
non-sensitive diagnostic message plus metadata; ensure the thrown error is still
propagated but never concatenate or include
trimmedResponse/jsonString/streamText in the logged text.
In `@ScreenTranslate/Services/Translation/Providers/LLMTranslationProvider.swift`:
- Around line 341-343: The code currently logs raw response text (via
displayString/trimmedResponse) in logger.error; remove any raw-response content
from logs and instead log only non-sensitive metadata: include
engineType.rawValue, the response byte/character count (e.g.,
trimmedResponse.utf8.count or .count), any HTTP/status code or SSE status
available, and the structured error.localizedDescription; apply this change to
the logger.error sites referenced (the block using displayString/trimmedResponse
around logger.error, and the similar occurrences at the other spots). Keep
throwing the original error, but ensure no full response body is written to logs
(optionally include a stable checksum/fingerprint if you need to correlate
without exposing content).
- Around line 295-301: The log currently only masks the "Authorization" header
and therefore may print the Claude API key sent as "x-api-key"; update the
header-sanitizing logic in LLMTranslationProvider.swift (the block that reads
request.allHTTPHeaderFields and constructs safeHeaders) to also detect
"x-api-key" (handle common casing variants) and replace its value with a masked
version (e.g., show the first 4 chars then "..."), then pass that masked
safeHeaders to logger.info for the endpoint log; ensure you reference and modify
the same safeHeaders variable used before calling logger.info so no real API
keys are logged.
In `@ScreenTranslate/Services/TranslationEngine.swift`:
- Around line 236-249: The code currently falls back to `.english` when
detection fails and then short-circuits translation if that fallback equals the
target; change this so the short-circuit only happens when an explicit source
was provided (config.sourceLanguage) or detection actually returned a non-nil
result: compute an optional detectedSourceOptional = config.sourceLanguage ??
(config.autoDetectSourceLanguage ? Self.detectLanguage(for: text) : nil), then
only compare/evaluate short-circuit when detectedSourceOptional is non-nil (let
detectedSource = detectedSourceOptional!), preserving the existing
signpost/logging and return behavior; apply the same conditional change to the
similar block that was referenced (the lines around the second short-circuit at
361-364).
In `@ScreenTranslate/Services/TranslationService.swift`:
- Around line 421-429: The current Han-character shortcut wrongly treats any
text with Han characters as Chinese by setting resolvedSourceLang = targetLang,
which causes Japanese (or other languages with Kanji) to be skipped by the
self-translation check; change the logic so you only take this shortcut when the
detected/assigned resolvedSourceLang is already a Chinese variant (i.e.,
resolvedSourceLang == .chineseSimplified || resolvedSourceLang ==
.chineseTraditional) OR when no explicit sourceLang was provided and detection
yielded a Chinese variant; do NOT overwrite an explicit non-Chinese
sourceLang—use the existing containsHanCharacters check only as a tie-breaker
when detection/result already indicates Chinese, and keep the self-translation
bypass using resolvedSourceLang and targetLang as before.
In `@ScreenTranslateTests/TranslationServicePipelineTests.swift`:
- Line 844: 当前测试文件过长导致 file_length lint 阻断；将 MockTranslationProvider 和
MockTranslationServicing 以及与它们相关的辅助类型从 TranslationServicePipelineTests
中抽离到一个新的测试文件（例如 MockTranslationProvidersTests.swift），并在新文件顶部添加相同的 test
target/imports，确保类/struct 的访问级别（public/internal）能被测试目标访问；在原测试文件中移除这些 mock
定义并更新任何引用（如
枚举/方法名：MockTranslationProvider、MockTranslationServicing、TranslationServicePipelineTests）以导入或使用新文件中的实现，运行测试并修复编译或命名冲突。
- Around line 195-207: The setUp modifies and persists
AppSettings.shared.engineConfigs via saveEngineConfigs(), which can pollute
other tests; capture a snapshot of the original settings (e.g., copy
AppSettings.shared or its engineConfigs) before enabling engines in setUp, and
implement tearDown to restore that snapshot and call saveEngineConfigs() to
revert persistent state; reference the setUp override, AppSettings.shared,
engineConfigs, and saveEngineConfigs(), and add an override func tearDown()
async throws that restores the previous config and persists it.
- Line 755: The conditional using appleRequests.count > 0 triggers SwiftLint
empty_count; change the check to use the collection API (replace if
appleRequests.count > 0 with if !appleRequests.isEmpty) or remove the if
entirely because the test already asserts appleRequests.count == 1 earlier in
TranslationServicePipelineTests, so the branch can be simplified accordingly.
---
Outside diff comments:
In `@ScreenTranslate/Features/Settings/EngineConfigSheet.swift`:
- Around line 393-416: testConnection() currently calls
TranslationService.shared.verifyConnection(for: engine) which only uses saved
config, ignoring the unsaved local state for
baseURL/modelName/apiKey/secretKey/appID in the sheet; construct a temporary
EngineConfig (or a dictionary) from the local state fields (baseURL, modelName,
apiKey, secretKey, appID) and pass it into the verification logic instead of
relying on persisted config—either call an existing verifyConnection overload
that accepts a config or add one (e.g., verifyConnection(for:engine, config:
tempConfig)); ensure you use temporary credentials for the test (do not persist
them) and only save to Keychain when the user explicitly saves.
---
Minor comments:
In `@ScreenTranslate/Features/Settings/CompatibleEngineConfigSheet.swift`:
- Around line 320-324: The catch block in CompatibleEngineConfigSheet.swift
currently only sets errorMessage, showErrorAlert and isFetchingModels but does
not clear the old model list; update the catch handler to also set
availableModels = [] (reset the property on the view) so the UI won't display
stale models when the fetch fails, keeping the existing errorMessage,
showErrorAlert and isFetchingModels updates intact.
- Around line 84-97: The Menu's label is an empty Text with a tiny hit area,
which hurts discoverability and accessibility; replace the empty label with a
clear tappable icon (e.g., Image(systemName: "chevron.down")) and increase its
touch target (padding or larger frame), then add accessibility modifiers (e.g.,
.accessibilityLabel("Select model") and an optional .accessibilityHint) so
VoiceOver users can find and understand the control; keep the existing Menu,
ForEach(availableModels, id: \.self) and modelName assignment logic unchanged.
In `@ScreenTranslate/Features/Settings/EngineConfigSheet.swift`:
- Around line 446-450: 在对可用模型拉取的异常分支（当前 catch 块，已设置 self.errorMessage /
self.showErrorAlert / self.isFetchingModels）中清空旧结果：将 self.availableModels
重置为一个空数组或合适的初始值，以避免在拉取失败后保留过时的模型列表并误导配置界面；在 EngineConfigSheet 的该 catch
分支中加入这一步骤即可。
- Around line 274-287: The model selection Menu uses an invisible tiny label
(Text("") with a small frame), making the entry hard to discover; replace the
empty Text label with a visible tappable icon (e.g., an Image system icon like a
chevron or gear) and increase its tappable frame, update the Menu label in the
same block that wraps ForEach(availableModels, id: \.self) and Button(model) so
users can see and hit it, and add accessibility modifiers (e.g.,
accessibilityLabel / accessibilityIdentifier and
accessibilityAddTraits(.isButton)) targeting the Menu label so VoiceOver/screen
readers announce it while preserving the existing .menuStyle(.borderlessButton)
and modelName assignment logic.
In `@ScreenTranslate/Features/Settings/EngineSettingsTab.swift`:
- Around line 120-133: The Menu currently uses an empty Text label which makes
the control invisible and hard to discover; replace the empty label with a
visible affordance such as a Label or Image (e.g. Image(systemName:
"chevron.down") or Label("Model", systemImage: "chevron.down")) and set
accessibility modifiers so VoiceOver reads its purpose; update the Menu
instances that iterate availableVLMModels (the Menu that sets
viewModel.vlmModelName) to use the visible icon/label and add
.accessibilityLabel("Select VLM model") or a localized string and
.accessibilityHint(...) to improve discoverability.
- Around line 224-228: In each catch block where you set self.errorMessage,
self.showErrorAlert, and self.isFetchingVLMModels (e.g., the catch at
EngineSettingsTab.swift lines around the isFetchingVLMModels flow), also reset
the model list to an empty array by assigning self.availableVLMModels = [];
likewise, in the other failure catch (the block around lines 449-452) clear the
corresponding list (e.g., self.availableLLMModels = []) so stale models aren’t
shown after a failed fetch.
In `@ScreenTranslate/Resources/de.lproj/Localizable.strings`:
- Around line 825-826: 文件中两条本地化键 "engine.config.fetchModels" 和
"engine.config.fetchModels.failed" 仍为英文，需将它们翻译为德语以避免界面语言混杂；请在
ScreenTranslate/Resources/de.lproj/Localizable.strings 中替换这两个键的值为德语对应文本（例如
"Abrufen" 或更合适的上下文词，以及 "Fehler beim Abrufen der Modelle"
或同义的德语错误提示），确保键名不变只修改右侧字符串并遵循现有本地化风格和语气。
In `@ScreenTranslate/Resources/es.lproj/Localizable.strings`:
- Around line 825-826: 两条本地化键 engine.config.fetchModels 和
engine.config.fetchModels.failed 目前值仍为英文，导致西班牙语界面出现混合语言；将这两项替换为对应的西班牙语翻译（例如
"engine.config.fetchModels" = "Obtener"; "engine.config.fetchModels.failed" =
"Error al obtener los modelos"; 或根据产品术语表使用合适译文），并确保保存到
es.lproj/Localizable.strings，避免回退到英文。
In `@ScreenTranslate/Resources/fr.lproj/Localizable.strings`:
- Around line 825-826: Replace the English values for the localization keys
"engine.config.fetchModels" and "engine.config.fetchModels.failed" with proper
French translations so the French locale remains consistent: change "Fetch" to a
French equivalent (e.g., "Récupérer") and "Failed to fetch models" to a French
sentence (e.g., "Échec de la récupération des modèles" or "Impossible de
récupérer les modèles") for the keys "engine.config.fetchModels" and
"engine.config.fetchModels.failed" respectively.
In `@ScreenTranslate/Resources/it.lproj/Localizable.strings`:
- Around line 826-827: 替换这两个键的英文字符串为意大利语：将 "engine.config.fetchModels" 的值从
"Fetch" 修改为意大利语（例如 "Recupera" 或 "Ottieni"），并将 "engine.config.fetchModels.failed"
的值从 "Failed to fetch models" 修改为意大利语（建议 "Impossibile recuperare i modelli"
或相近表述），确保只更新这两个键的右侧字符串以保持文件其它条目不变。
In `@ScreenTranslate/Resources/ja.lproj/Localizable.strings`:
- Around line 825-826: 资源文件中的两个键 "engine.config.fetchModels" 和
"engine.config.fetchModels.failed" 在日语本地化仍为英文；请在
ScreenTranslate/Resources/ja.lproj/Localizable.strings 中找到这两个键并将其值替换为日语翻译（例如
"モデルを取得" 或更自然的 "モデルを取得する" 用于按钮，"モデルの取得に失敗しました" 用于错误提示），确保按钮和错误提示在日语环境下不再显示英文。
In `@ScreenTranslate/Resources/ko.lproj/Localizable.strings`:
- Around line 825-826: 这两条资源键 "engine.config.fetchModels" 和
"engine.config.fetchModels.failed" 在韩语词条中仍为英文，请在 ko.lproj
的本地化文件中为这两个键提供对应的韩文翻译（替换当前英文值），确保字符串值使用正确的韩语文本并保持与其他条目相同的格式和编码，以便韩语界面显示本地化文案。
In `@ScreenTranslate/Resources/pt.lproj/Localizable.strings`:
- Around line 840-841: 两个新添加的本地化键 engine.config.fetchModels 和
engine.config.fetchModels.failed 仍为英文，按葡萄牙语本地化替换即可：将 "engine.config.fetchModels"
的值改为 "Buscar modelos"（或 "Buscar" 视上下文为按钮简短文案），将
"engine.config.fetchModels.failed" 的值改为 "Falha ao buscar modelos"（或 "Falha ao
obter modelos"），确保用双引号包裹并保持与现有 Localizable.strings 格式一致。
In `@ScreenTranslate/Resources/ru.lproj/Localizable.strings`:
- Around line 825-826: The Russian localization strings for the keys
"engine.config.fetchModels" and "engine.config.fetchModels.failed" are still in
English; update their values to proper Russian translations (e.g., set
"engine.config.fetchModels" to a Russian word for "Fetch" like "Загрузить" and
"engine.config.fetchModels.failed" to a Russian phrase for "Failed to fetch
models" like "Не удалось загрузить модели") so the ru.lproj Localizable.strings
file shows Russian text for these keys.
In `@ScreenTranslate/Services/OpenAIVLMProvider.swift`:
- Around line 349-383: The loop in extractField(from:pattern:) misinterprets
braces inside string literals because it only tracks depth and not whether we're
inside a quoted string; update the loop to track an inString Bool (alongside
escaped) so you only increment/decrement depth when inString == false, and
toggle inString when encountering an unescaped quote; then treat the end
condition as "encounter an unescaped quote that closes the top-level string
(inString becomes false and depth == 0)" so the break happens correctly. Ensure
you reference the existing variables (start, end, escaped, depth) and add
inString to control when '{', '}', '[' and ']' affect depth and when '"' toggles
string state.
---
Nitpick comments:
In `@create_dmg.sh`:
- Line 38: The APP_PATH assignment uses command substitution with find (the
APP_PATH=$(find "$PROJECT_PATH/build" ... | head -n 1)) which can emit noisy "No
such file or directory" messages to stderr when some searched paths are absent;
update the find invocation to suppress its error output (e.g., redirect find's
stderr to /dev/null or otherwise ignore errors) so the command substitution
still returns the first match but does not print spurious errors to stderr while
preserving existing behavior when set -e is enabled.
In `@ScreenTranslate/Models/AppSettings.swift`:
- Around line 743-783: The duplicated Keychain read logic in
loadPaddleOCRAPIKeyFromKeychain and loadVLMAPIKeyFromKeychain should be removed
and replaced with a single call into KeychainService (or a new helper on
KeychainService) so storage keys, JSON encoding for StoredCredentials and query
parameters are centralized; update these two functions to call
KeychainService.getStoredCredentials(account:)/KeychainService.apiKey(forAccount:)
(or similar) and return credentials.apiKey, keeping DEBUG behavior delegated to
KeychainService as well so key prefixes and serialization aren’t duplicated in
AppSettings.
In `@ScreenTranslateTests/ModelDiscoveryServiceTests.swift`:
- Around line 99-105: The test testEmptyURLThrowsError relies on string matching
of error.localizedDescription; change it to assert the concrete error type/enum
case returned by ModelDiscoveryService.fetchModels instead. Update the catch to
cast the thrown error to the specific error type (e.g., ModelDiscoveryError) and
assert the exact case (for example .emptyURL or the appropriate case defined on
ModelDiscoveryError), or compare against a stable error value/errno if your
service exposes one, so the test checks ModelDiscoveryService.fetchModels for
the expected error case rather than a substring of localizedDescription.
- Around line 8-95: The tests currently only decode local temporary Codable
structs instead of exercising the real parsing in ModelDiscoveryService; update
testParseOpenAIModelsResponse and testParseOllamaTagsResponse to call the
ModelDiscoveryService parsing entry point (or instantiate ModelDiscoveryService
with a mock/injectable network layer that returns the JSON data) and assert the
returned model list contents and order; locate parsing logic on the
ModelDiscoveryService type (e.g., a parseModels/fromData or fetchAvailableModels
method) and use that API so the tests validate production parsing behavior
rather than the local test-only Codable structs.
In `@ScreenTranslateTests/TranslationServicePipelineTests.swift`:
- Around line 770-783: The testPrintRealUserSettings method is not a valid test
because it only prints values; either remove this method or replace it with a
deterministic, repeatable test: obtain AppSettings.shared inside MainActor.run
(keep the existing reference to AppSettings.shared and the same properties
ocrEngine, translationEngine, translationTargetLanguage,
translationSourceLanguage, translationFallbackEnabled, engineSelectionMode,
vlmProvider), set or reset AppSettings to a known state before reading, and add
XCTAssert assertions for each expected property value (instead of printing) so
the test is deterministic and repeatable; if you prefer deletion, simply remove
the testPrintRealUserSettings function.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0061bf8c-75a6-404a-8d4d-406010b5007a
📒 Files selected for processing (35)
ScreenTranslate.xcodeproj/project.pbxprojScreenTranslate/App/Coordinators/CaptureCoordinator.swiftScreenTranslate/App/Coordinators/TextTranslationCoordinator.swiftScreenTranslate/Extensions/URLExtensions.swiftScreenTranslate/Features/Settings/CompatibleEngineConfigSheet.swiftScreenTranslate/Features/Settings/EngineConfigSheet.swiftScreenTranslate/Features/Settings/EngineSettingsTab.swiftScreenTranslate/Features/Settings/MultiEngineSettingsSection.swiftScreenTranslate/Features/Settings/SettingsViewModel.swiftScreenTranslate/Models/AppSettings.swiftScreenTranslate/Resources/de.lproj/Localizable.stringsScreenTranslate/Resources/en.lproj/Localizable.stringsScreenTranslate/Resources/es.lproj/Localizable.stringsScreenTranslate/Resources/fr.lproj/Localizable.stringsScreenTranslate/Resources/it.lproj/Localizable.stringsScreenTranslate/Resources/ja.lproj/Localizable.stringsScreenTranslate/Resources/ko.lproj/Localizable.stringsScreenTranslate/Resources/pt.lproj/Localizable.stringsScreenTranslate/Resources/ru.lproj/Localizable.stringsScreenTranslate/Resources/zh-Hans.lproj/Localizable.stringsScreenTranslate/Services/ClaudeVLMProvider.swiftScreenTranslate/Services/GLMOCRVLMProvider.swiftScreenTranslate/Services/ModelDiscoveryService.swiftScreenTranslate/Services/OllamaVLMProvider.swiftScreenTranslate/Services/OpenAIVLMProvider.swiftScreenTranslate/Services/PermissionManager.swiftScreenTranslate/Services/Security/KeychainService.swiftScreenTranslate/Services/Translation/Providers/CompatibleTranslationProvider.swiftScreenTranslate/Services/Translation/Providers/LLMTranslationProvider.swiftScreenTranslate/Services/TranslationEngine.swiftScreenTranslate/Services/TranslationService.swiftScreenTranslateTests/GLMOCRVLMProviderTests.swiftScreenTranslateTests/ModelDiscoveryServiceTests.swiftScreenTranslateTests/TranslationServicePipelineTests.swiftcreate_dmg.sh
| let appleRequests = await apple.requests | ||
| print("DEBUG_APPLE_REQUESTS: \(appleRequests)") | ||
| XCTAssertEqual(appleRequests.count, 1) | ||
| if appleRequests.count > 0 { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP 'count\s*>\s*0' ScreenTranslateTests/TranslationServicePipelineTests.swiftRepository: hubo1989/ScreenTranslate
Length of output: 109
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="ScreenTranslateTests/TranslationServicePipelineTests.swift"
# 打印 740-770 行上下文，确认 appleRequests 的类型与该 if 的语义
nl -ba "$FILE" | sed -n '740,770p'Repository: hubo1989/ScreenTranslate
Length of output: 111
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="ScreenTranslateTests/TranslationServicePipelineTests.swift"
# 740-770 行上下文（使用 awk 自带行号）
awk 'NR>=740 && NR<=770 {printf "%d:%s\n", NR, $0}' "$FILE"
# 查找 appleRequests 的声明位置与类型线索
rg -n --context 2 'appleRequests' "$FILE"Repository: hubo1989/ScreenTranslate
Length of output: 2175
修复 SwiftLint empty_count 规则触发点
ScreenTranslateTests/TranslationServicePipelineTests.swift:755 处的 if appleRequests.count > 0 { 会触发 empty_count；改为 if !appleRequests.isEmpty {。另外前面已断言 appleRequests.count == 1，该 if 也可考虑直接移除。
🧰 Tools
🪛 SwiftLint (0.63.2)
[Error] 755-755: Prefer checking isEmpty over comparing count to zero
(empty_count)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ScreenTranslateTests/TranslationServicePipelineTests.swift` at line 755, The
conditional using appleRequests.count > 0 triggers SwiftLint empty_count; change
the check to use the collection API (replace if appleRequests.count > 0 with if
!appleRequests.isEmpty) or remove the if entirely because the test already
asserts appleRequests.count == 1 earlier in TranslationServicePipelineTests, so
the branch can be simplified accordingly.
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
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.
Summary
Brief description of the changes in this PR.
Type of Change
Related Issues
Fixes #(issue number)
Changes Made
Testing
Describe the tests you ran to verify your changes:
Screenshots
If applicable, add screenshots showing the changes.
Checklist
Summary by CodeRabbit
发布说明
新功能
改进
测试