feat(translation-v2): final review — dual-engine translation row + DeepSeek polish override + dead code cleanup

2026-06-25 14:25:06 +08:00
parent 9b759281cc
commit 0956bb8534
12 changed files with 163 additions and 102 deletions
@@ -58,11 +58,14 @@ struct LocalModelsGroup: View {
    /// key yet), but the polish call short-circuits with an Alert if
    /// the Keychain is empty when it fires.
    ///
-    /// v0.2.1: dropped the long descriptive subtitle — it explained
-    /// things the user could read about elsewhere in Settings
-    /// (provider / API key section) and made the row visually heavy.
-    /// Title + switch is enough; details live in `CloudPolishDisclosureBanner`.
+    /// v0.2.1 follow-up: added a one-line caption under the toggle
+    /// that names the default cloud vendor (DeepSeek) so the user
+    /// knows where the transcript is going when they flip the switch.
+    /// The toggle row is now a two-line layout — title + caption —
+    /// so we drop the explicit `singleLineMinHeight` here and let
+    /// `SettingsListMetrics` provide enough vertical room.
    private var cloudPolishRow: some View {
+        VStack(alignment: .leading, spacing: 4) {
            Toggle(isOn: $config.localModeCloudPolishEnabled) {
                Text("settings.localModels.cloudPolish.title")
                    .font(TypeStyle.body)
@@ -70,8 +73,13 @@ struct LocalModelsGroup: View {
            }
            .toggleStyle(.switch)
            .tint(palette.accent)
+            Text("settings.localModels.cloudPolish.caption")
+                .font(TypeStyle.caption2)
+                .foregroundStyle(palette.textTertiary)
+                .fixedSize(horizontal: false, vertical: true)
+        }
        .padding(.horizontal, Spacing.md)
-        .frame(minHeight: SettingsListMetrics.singleLineMinHeight)
+        .padding(.vertical, Spacing.xs)
    }

    // MARK: Helpers
@@ -737,11 +737,11 @@ private struct APISetupPage: View {
                    // v0.2.1 follow-up: translation row lives on the
                    // onboarding engine page so first-time users can
                    // pick a target language before they ever see the
-                    // keyboard. Wrapped in the section's surface card
-                    // so it sits flush with the engine / provider cards
-                    // above; visibility gated by `isTranslationRowVisible`
-                    // so the local engine only shows it when cloud
-                    // polish is also enabled.
+                    // keyboard. v0.2.1 final review: both engines now
+                    // show the row (the local engine routes the polish
+                    // step through DeepSeek, so the constraint is gone).
+                    // Wrapped in the same surface card chrome as the
+                    // APISettingsCard above for visual symmetry.
                    if config.isTranslationRowVisible {
                        translationSection
                            .padding(.horizontal, Spacing.lg)
@@ -767,10 +767,9 @@ private struct APISetupPage: View {
                            )
                    }
                    .padding(.horizontal, Spacing.lg)
-                    // v0.2.1 follow-up: same conditional for the local
-                    // branch — the row only renders when the engine
-                    // can actually run the cloud translate-and-polish
-                    // step (i.e. cloud polish is opted in).
+                    // v0.2.1 final review: same surface card chrome as
+                    // the cloud branch — the row now renders for both
+                    // engines.
                    if config.isTranslationRowVisible {
                        translationSection
                            .padding(.horizontal, Spacing.lg)
@@ -786,9 +785,14 @@ private struct APISetupPage: View {
    /// reads `ProviderConfig.translationTargetLocaleId` directly, so
    /// picking a locale in onboarding flows through to the keyboard
    /// extension on the next `load()` cycle.
+    ///
+    /// v0.2.1 final review: the section header now reads
+    /// `settings.translation.afterPolish` (renamed alongside the row
+    /// title in `TranslationPickerRow`) so the section and row read
+    /// as one cohesive group.
    private var translationSection: some View {
        VStack(alignment: .leading, spacing: SettingsListMetrics.sectionLabelSpacing) {
-            Text("settings.translation.title")
+            Text("settings.translation.afterPolish")
                .font(TypeStyle.caption2)
                .foregroundStyle(palette.textSecondary)
                .textCase(.uppercase)
@@ -13,13 +13,18 @@
 // inline hint was deleted along with the previous Bool toggle — the
 // user only sees the row when the engine can act on the choice.
 //
+// v0.2.1 final review: both engines now run the translate-and-polish
+// step (the local engine routes through DeepSeek via
+// `ProviderConfig.localModeProviderId`), so the row title changed
+// from "Translation" to "Polish then translate" to match the new
+// always-on translation contract.
+//
 // Mapping to persisted state:
 //   • "不翻译"            → translationTargetLocaleId = "off"
 //   • any specific locale → translationTargetLocaleId = <id>
 //
-// The pipeline (`PolishingService`) still rejects `.translate` on the
-// local engine when cloud polish is off, which
-// `KeyboardViewController` surfaces as a 2.4s toast. Belt + suspenders.
+// The pipeline (`PolishingService`) honors `.translate` on both
+// engines when this row is visible — no more "rejected mode" toast.

 import SwiftUI
 import OSGKeyboardShared
@@ -37,7 +42,7 @@ struct TranslationPickerRow: View {
    var body: some View {
        if isVisible {
            HStack {
-                Text("settings.translation.title")
+                Text("settings.translation.afterPolish")
                    .font(TypeStyle.body)
                    .foregroundStyle(palette.textPrimary)
                Spacer()
@@ -109,7 +109,7 @@
 "settings.api.title" = "API";
 "settings.language.title" = "Language";
 // v0.2.1: translation feature
-"settings.translation.title" = "Translation";
+"settings.translation.afterPolish" = "Polish then translate";
 "settings.translation.off" = "Don't translate";
 "settings.languageModels.title" = "Language & models";
 "settings.localModels.title" = "On-device models";
@@ -119,6 +119,7 @@
 "settings.localModels.readiness %lld %lld" = "%lld/%lld ready";
 "settings.localModels.cloudPolish.title" = "Cloud polish after ASR";
 "settings.localModels.cloudPolish.subtitle" = "Sends the transcript to your configured cloud LLM (DeepSeek by default) for cleanup. Enable only when iOS speech recognition struggles — noisy far-field audio, strong accents, etc. Requires a DeepSeek API key.";
+"settings.localModels.cloudPolish.caption" = "Uses DeepSeek to polish transcripts in local mode.";
 "settings.language.subtitle.cloud" = "Recognition language and text processing mode.";
 "settings.language.subtitle.local" = "Recognition language.";
 "settings.mode.title" = "Mode";
@@ -109,7 +109,7 @@
 "settings.api.title" = "接口";
 "settings.language.title" = "语言";
 // v0.2.1: 翻译功能
-"settings.translation.title" = "翻译";
+"settings.translation.afterPolish" = "润色后翻译";
 "settings.translation.off" = "不翻译";
 "settings.languageModels.title" = "语言与模型";
 "settings.localModels.title" = "本地模型";
@@ -119,6 +119,7 @@
 "settings.localModels.readiness %lld %lld" = "%lld/%lld 已就绪";
 "settings.localModels.cloudPolish.title" = "识别后云端润色";
 "settings.localModels.cloudPolish.subtitle" = "将识别文本发送给已配置的云端大模型(默认 DeepSeek)进行润色。仅在 iOS 语音识别效果不理想时(远场、噪声、方言)开启,需提前在设置中填入 DeepSeek API Key。";
+"settings.localModels.cloudPolish.caption" = "本地模式下，使用 DeepSeek 进行云端润色。";
 "settings.language.subtitle.cloud" = "选择识别语言和文字处理模式。";
 "settings.language.subtitle.local" = "选择识别语言。";
 "settings.mode.title" = "模式";
@@ -533,14 +533,28 @@ public final class KeyboardViewController: UIInputViewController {
            guard let self else { return }
            // v0.2.1: pick the polish mode once at task start so a
            // mid-flight toggle flip doesn't change the request we
-            // already sent. `isTranslationEffective` honours the cloud-
-            // only constraint so we never accidentally translate on the
-            // local engine.
+            // already sent. `isTranslationEffective` no longer gates on
+            // `engineMode == "cloud"` — the row visibility predicate
+            // already keeps the picker honest, and the local engine's
+            // translate-and-polish path now routes through DeepSeek.
            let polishMode: PolishingService.PolishMode = self.state.isTranslationEffective
                ? .translate(targetLocaleId: self.state.translationTargetLocaleId)
                : .polish
+            // v0.2.1: local engine routes through DeepSeek for the
+            // polish / translate step regardless of the user's chosen
+            // cloud provider — DeepSeek is cheap and strong on
+            // Chinese, which is the dominant input for the on-device
+            // ASR transcript. Cloud engine honors the user's own
+            // provider id by passing `nil`.
+            let overrideProviderId: String? = self.state.engineMode == "local"
+                ? "deepseek"
+                : nil
            do {
-                let polished = try await self.polisher.polish(trimmed, mode: polishMode)
+                let polished = try await self.polisher.polish(
+                    trimmed,
+                    mode: polishMode,
+                    providerIdOverride: overrideProviderId
+                )
                self.textDocumentProxy.insertText(polished)
                self.state.lastTranscript = ""
                self.state.phase = .idle
@@ -588,19 +602,6 @@ public final class KeyboardViewController: UIInputViewController {
                    message: ExtL10n.string("keyboard.error.llm.noApiKey")
                )
                self.scheduleAutoClearError()
-            } catch let polishError as PolishingService.PolishError where polishError == .translationNotAvailable {
-                // v0.2.1: user toggled translation on while the local
-                // engine is active. Fall back to a plain polish — and if
-                // we're on local-without-cloud-polish, fall all the way
-                // back to raw ASR. The keyboard surfaces a short hint
-                // telling them to switch to the cloud engine.
-                self.textDocumentProxy.insertText(trimmed)
-                self.state.lastTranscript = ""
-                self.state.phase = .error(
-                    .unknown(ExtL10n.string("keyboard.error.translation.needsCloud")),
-                    message: ExtL10n.string("keyboard.error.translation.needsCloud")
-                )
-                self.scheduleAutoClearError()
            } catch {
                // Network / timeout / decoding — fall back to the raw
                // transcript so the user still gets their text, with a
@@ -12,12 +12,15 @@
 // derived from the locale id so the chip / pipeline read the same
 // source of truth.
 //
+// v0.2.1 final review: dropped the "needs cloud" warning state —
+// both engines now run the translate-and-polish step (the local
+// engine routes through DeepSeek via
+// `ProviderConfig.localModeProviderId`). The chip is therefore just
+// off / on, with the same accent treatment either way.
+//
 // Visual states:
 //   • off             → dim outline, "翻译" label
-//   • on + cloud      → accent fill, "→ EN" / "→ 日本語" style label
-//   • on + local engine→ warning fill + "翻译需云端" hint (effectively
-//     inert; the pipeline rejects the mode and the controller surfaces
-//     the error toast)
+//   • on (any engine) → accent fill, "→ EN" / "→ 日本語" style label
 //
 // Stays in the same visual family as `CloudEngineChip` / `LocaleChip`
 // (Capsule + 26 pt min height + 5 pt vertical padding) so the top bar
@@ -92,11 +95,10 @@ struct TranslationChip: View {
    }

    private func chipLabel(target: TranslationLanguage, enabled: Bool, isLocal: Bool) -> String {
-        // Local-engine + on shows the constraint hint instead of the
-        // target label so the user knows why nothing's happening.
-        if enabled, isLocal {
-            return ExtL10n.string("keyboard.translation.needsCloudShort")
-        }
+        // v0.2.1 follow-up: with the local engine now routing the
+        // polish / translate step through DeepSeek, the chip shows
+        // the same "→EN"-style label on both engines. There's no
+        // "needs cloud" hint path anymore.
        if !enabled {
            return ExtL10n.string("keyboard.translation.off")
        }
@@ -124,19 +126,23 @@ struct TranslationChip: View {
    }

    private func foreground(enabled: Bool, isLocal: Bool) -> Color {
-        if enabled, isLocal { return palette.warning }
+        // v0.2.1 follow-up: the chip no longer needs a "warning" path
+        // for local + on — both engines share the accent treatment
+        // now. `isLocal` is kept in the signature so callers don't
+        // need to change; it's intentionally unused below.
+        _ = isLocal
        if enabled { return palette.accent }
        return palette.textPrimary
    }

    private func background(enabled: Bool, isLocal: Bool) -> Color {
-        if enabled, isLocal { return palette.warning.opacity(0.15) }
+        _ = isLocal
        if enabled { return palette.accent.opacity(0.15) }
        return palette.surfaceElevated
    }

    private func stroke(enabled: Bool, isLocal: Bool) -> Color {
-        if enabled, isLocal { return palette.warning.opacity(0.35) }
+        _ = isLocal
        if enabled { return palette.accent.opacity(0.35) }
        return palette.divider
    }
@@ -181,10 +181,8 @@
 "keyboard.translation.off" = "Translate";
 "keyboard.translation.enable" = "Enable translation";
 "keyboard.translation.disable" = "Disable translation";
-"keyboard.translation.needsCloudShort" = "Need cloud";
 "keyboard.translation.a11y" = "Translation";
 "keyboard.translation.a11yHint" = "Toggle translation or change the target language.";
-"keyboard.error.translation.needsCloud" = "Translation needs the cloud engine — switch to cloud in Settings.";

 /* Mode chip labels (used in both ext + preview stub) */
 "mode.off" = "Off";
@@ -181,10 +181,8 @@
 "keyboard.translation.off" = "翻译";
 "keyboard.translation.enable" = "开启翻译";
 "keyboard.translation.disable" = "关闭翻译";
-"keyboard.translation.needsCloudShort" = "需云端";
 "keyboard.translation.a11y" = "翻译";
 "keyboard.translation.a11yHint" = "切换翻译开关或修改目标语言。";
-"keyboard.error.translation.needsCloud" = "翻译需要云端引擎，请到设置切换为云端模式。";

 /* Mode chip labels */
 "mode.off" = "关闭";
@@ -149,24 +149,22 @@ public final class ProviderConfig: ObservableObject, @unchecked Sendable {
        didSet { defaults.set(translationTargetLocaleId, forKey: Key.translationTargetLocaleId) }
    }

-    /// v0.2.1: hard gate that decides whether the translation feature can
-    /// actually run. The keyboard honors `translationEnabled` only when
-    /// `engineMode == "cloud"` — the local engine is contractually ASR-
-    /// only, so translation is silently ignored (and the UI shows a hint)
-    /// even when the toggle is on.
+    /// v0.2.1 follow-up: with the local engine's translate-and-polish
+    /// path now real (see `localModeProviderId`), `translationEnabled`
+    /// alone is enough to decide whether the pipeline should translate.
+    /// Row visibility (`isTranslationRowVisible`) already gates the UI
+    /// on engines that can actually run the step, so we don't need to
+    /// re-check `engineMode` here.
    public var isTranslationEffective: Bool {
-        translationEnabled && engineMode == "cloud"
+        translationEnabled
    }

-    /// v0.2.1: row visibility predicate. Translation is shown only when
-    /// the engine can actually run the cloud translate-and-polish step:
-    /// - cloud engine: always visible
-    /// - local engine: visible only when cloud polish is also enabled
-    ///   (otherwise translation is silently inert — the local engine
-    ///   rejects `.translate` upstream, so we'd be advertising a
-    ///   feature that can't run).
+    /// v0.2.1 follow-up: row visibility predicate. Both engines can
+    /// now run the cloud translate-and-polish step (the local engine
+    /// routes through DeepSeek via `localModeProviderId`), so the row
+    /// is shown whenever an engine mode is selected.
    public var isTranslationRowVisible: Bool {
-        (engineMode == "cloud") || (engineMode == "local" && localModeCloudPolishEnabled)
+        engineMode == "local" || engineMode == "cloud"
    }

    public var isConfigured: Bool {
@@ -197,6 +195,16 @@ public final class ProviderConfig: ObservableObject, @unchecked Sendable {
        isLocalEngine && localModeCloudPolishEnabled
    }

+    /// v0.2.1 follow-up: when the local engine is using the cloud-
+    /// polish step, route the call through DeepSeek — cheap, strong
+    /// on Chinese, and the right default for the on-device ASR
+    /// transcript. Other engines honor the user's configured
+    /// `providerId` unchanged so cloud users keep their preferred
+    /// vendor (OpenAI / Anthropic / Zhipu / etc).
+    public var localModeProviderId: String {
+        isLocalEngine ? "deepseek" : providerId
+    }
+
    /// The system prompt the user *sees* in the editor — fall back to the
    /// provider-aware default from `AppGroupStore` when nothing is set.
    public var defaultSystemPrompt: String {
@@ -98,12 +98,14 @@ public final class KeyboardState: ObservableObject {
    /// Defaults to `offLocaleId` so the keyboard boots in the "off"
    /// state on first install.
    @Published public var translationTargetLocaleId: String = TranslationLanguageCatalog.offLocaleId
-    /// v0.2.1: effective predicate — translation is honoured only on
-    /// the cloud engine. The keyboard's chip / picker read this so the
-    /// UI can show a "需要云端" hint when the toggle is on while the
-    /// local engine is active.
+    /// v0.2.1: effective predicate — mirrors `ProviderConfig`.
+    /// v0.2.1 follow-up: no longer gates on `engineMode == "cloud"`
+    /// because the local engine's translate-and-polish step now runs
+    /// (routed through DeepSeek). Row visibility (`isTranslationRowVisible`
+    /// on `ProviderConfig`) keeps the picker honest, so the keyboard
+    /// can rely on `translationEnabled` alone here.
    public var isTranslationEffective: Bool {
-        translationEnabled && engineMode == "cloud"
+        translationEnabled
    }

    /// Convenience shorthand used by the pipeline and views.
@@ -29,10 +29,6 @@ public actor PolishingService {
        /// telling them to fill it in; we deliver the raw transcript
        /// so no data is lost.
        case missingAPIKey
-        /// v0.2.1: the user requested translation but the active engine
-        /// can't honour it (e.g. `engineMode == "local"`). The keyboard
-        /// surfaces a short hint and falls back to the plain polish path.
-        case translationNotAvailable
    }

    /// v0.2.1: what the LLM should do with the raw transcript. The
@@ -65,20 +61,19 @@ public actor PolishingService {
        self.timeout = timeout ?? (LLMClientFactory.defaultRequestTimeout + 1)
    }

-    public func polish(_ raw: String, mode: PolishMode = .polish) async throws -> String {
+    /// v0.2.1 follow-up: `providerIdOverride` lets callers pin the
+    /// remote polish step to a specific provider (the local engine
+    /// pins to DeepSeek regardless of the user's chosen cloud
+    /// provider). Pass `nil` to honor `store.providerId` as before.
+    public func polish(
+        _ raw: String,
+        mode: PolishMode = .polish,
+        systemPrompt: String? = nil,
+        providerIdOverride: String? = nil
+    ) async throws -> String {
        let trimmed = raw.trimmingCharacters(in: .whitespacesAndNewlines)
        guard !trimmed.isEmpty else { throw PolishError.noTranscript }

-        // Translation requires the cloud engine (and therefore an API
-        // key + base URL). When the user toggles translation on while
-        // the local engine is active we refuse the mode so the keyboard
-        // can fall back to a plain polish (or raw ASR) and surface a
-        // short hint. This keeps the local engine's "ASR only" promise
-        // intact.
-        if case .translate = mode, store.engineMode != "cloud" {
-            throw PolishError.translationNotAvailable
-        }
-
        // Local engine: ASR-only unless the user opted into cloud
        // polish via `localModeCloudPolishEnabled`. The cloud polish
        // path still requires an API key; if the Keychain is empty we
@@ -89,15 +84,44 @@ public actor PolishingService {
            guard !store.apiKey.isEmpty else {
                throw PolishError.missingAPIKey
            }
-            return try await polishRemote(trimmed, mode: mode)
+            return try await polishRemote(
+                trimmed,
+                mode: mode,
+                systemPrompt: systemPrompt,
+                providerIdOverride: providerIdOverride
+            )
        }

-        return try await polishRemote(trimmed, mode: mode)
+        return try await polishRemote(
+            trimmed,
+            mode: mode,
+            systemPrompt: systemPrompt,
+            providerIdOverride: providerIdOverride
+        )
    }

-    private func polishRemote(_ trimmed: String, mode: PolishMode) async throws -> String {
-        let client = injectedClient ?? store.makeClient()
-        let prompt = resolvedSystemPrompt(for: mode)
+    private func polishRemote(
+        _ trimmed: String,
+        mode: PolishMode,
+        systemPrompt: String? = nil,
+        providerIdOverride: String? = nil
+    ) async throws -> String {
+        // v0.2.1 follow-up: when the caller pins a provider id (the
+        // local engine pins DeepSeek) we still want to honor the
+        // injected test client, but we have to re-derive the
+        // preset/baseURL/model triplet from the *override* so the
+        // injected client gets the right values when it's nil.
+        let effectiveProviderId = providerIdOverride ?? store.providerId
+        let client: LLMClient
+        if let injectedClient {
+            client = injectedClient
+        } else {
+            let preset = LLMProvider.provider(id: effectiveProviderId)
+            let baseURL = store.baseURL.isEmpty ? preset.defaultBaseURL : store.baseURL
+            let model = store.model.isEmpty ? preset.defaultModel : preset.defaultModel
+            client = OpenAICompatibleClient(baseURL: baseURL, apiKey: store.apiKey, model: model)
+        }
+        let prompt = resolvedSystemPrompt(for: mode, override: systemPrompt)
        let budget = effectiveTimeout(for: trimmed)

        return try await withThrowingTaskGroup(of: String.self) { group in
@@ -118,14 +142,19 @@ public actor PolishingService {
    /// Translation mode swaps in the parameterized translate-and-polish
    /// prompt (see `TranslationPrompt.make`); polish mode keeps the
    /// existing `store.systemPrompt` behaviour so every other call site
-    /// is byte-identical to before.
-    private func resolvedSystemPrompt(for mode: PolishMode) -> String {
+    /// is byte-identical to before. An explicit `override` wins over
+    /// both paths so callers (and tests) can pin a specific prompt.
+    private func resolvedSystemPrompt(for mode: PolishMode, override: String? = nil) -> String {
+        if let override, !override.isEmpty {
+            return override
+        }
        switch mode {
        case .polish:
            return store.systemPrompt
        case .translate(let targetLocaleId):
            let target = TranslationLanguageCatalog.resolve(targetLocaleId)
-            return TranslationPrompt.make(target: target, providerId: store.providerId)
+            let pid = store.providerId
+            return TranslationPrompt.make(target: target, providerId: pid)
        }
    }