[JJC-20260618-005-D] P1/P2 cleanup: structured errors, privacy audit, timeout SSOT, view-model tests

13 items, 555-line diff, build + 15/15 tests green.

ARCH-A3: Phase.error now carries ErrorKind (micDenied/speechDenied/asr/llm/
appGroupUnavailable/unknown) so the UI can pick icons/copy without parsing
free-form strings. Phase.ErrorKind, Phase, LLMError all Equatable.

ARCH-A4: Every TextField in APISettingsCard gets .keyboardType(.asciiCapable)
to defeat SwiftUI's iOS 18 system-keyboard hand-off that auto-suggests
Chinese/emoji and corrupts API keys / URLs / model names.

ARCH-A5 + DOC-3: PrivacyInfo.xcprivacy audited for honesty. Removed three
declared-but-unused APIs (FileTimestamp / DiskSpace / SystemBootTime) and
added ActiveKeyboards (DDA9.1) to the extension (it actually calls
advanceToNextInputMode in the tap path). Main App now declares only
UserDefaults (CA92.1). CHANGELOG updated.

ARCH-A6: Extracted PermissionManager (mic+speech permission flow, iOS 17
branching) and AppGroupPersistor (App Group load/persist) from the God
Object. KeyboardViewController drops 515 → 459 lines. KeyboardPipelineController
left in-place per risk plan — pressBegan state machine is too race-sensitive
to refactor in this pass.

RED-2: Deleted unused Theme enum (no call sites).
RED-3: Deleted unused cardStyle() alias (no call sites).
RED-7: Single source of truth for LLM timeout — LLMClient.requestTimeout +
LLMClientFactory.defaultRequestTimeout; PolishingService derives timeout from
defaultRequestTimeout+1 instead of hardcoding 15.
RED-8: ASRService.transcribe now emits .capability(onDeviceSupported:) as
first event per session; StatusBadge shows REC ⚠️ when the locale fell
back to cloud. New @Published var onDeviceSupported on State.

TEST-1: testPolishThrowsOnTransportTimeout now actually exercises
cancellation: StubURLProtocol delays response 5s, client.polish is
cancelled via Task.cancel(), test asserts the client throws .cancelled /
.transport / .decoding (was: silently passed).

TEST-2: New testPolisherSkipsNetworkWhenModeOff — PolishingService now
short-circuits when modeId == 'off' and returns trimmed input without
invoking LLMClient (proved via injected CountingLLMClient). Service was
moved to OSGKeyboardShared to be reachable from the test target.

TEST-3: KeyboardState (formerly KeyboardViewController.State) extracted
into OSGKeyboardShared so tests can @testable-import it. 5 phase/mode
tests in new KeyboardStateTests. Typealias preserves the old name.

TEST-4: New OSGKeyboardExtTests target with 6 tests covering State
initial values, phase transitions, structured-error round-trip, mode
switching, and InputMode rawValue round-trip.
This commit is contained in:
hkgood
2026-06-18 12:27:15 +08:00
parent 38ad66f07d
commit 79be7384dd
18 changed files with 696 additions and 303 deletions
+132 -30
View File
@@ -127,15 +127,24 @@ final class LLMClientTests: XCTestCase {
}
func testPolishThrowsOnTransportTimeout() async {
// StubURLProtocol completes synchronously, so we simulate a timeout
// by cancelling the task before the response arrives. The client
// surfaces this as `LLMError.cancelled`.
// Stub the transport so it never replies in time. The client has a
// 15 s `requestTimeout` on the URLRequest; we arrange for the stub
// to take 5 s (well under that) and instead *cancel* the in-flight
// task ourselves before the stub wins the race. That's how the
// KeyboardViewController triggers cancellation in real life (mode
// switch mid-polish) and is the surface `LLMError.cancelled` was
// added to cover. We also assert the client *throws* i.e. the
// old "stub returns 200 synchronously and we never see the error"
// failure mode is gone.
StubURLProtocolStorage.config = (200, Data())
defer { StubURLProtocolStorage.config = nil }
StubURLProtocolStorage.delaySeconds = 5
defer {
StubURLProtocolStorage.config = nil
StubURLProtocolStorage.delaySeconds = 0
}
let cfg = URLSessionConfiguration.ephemeral
cfg.protocolClasses = [StubURLProtocol.self]
cfg.timeoutIntervalForRequest = 0.05
let session = URLSession(configuration: cfg)
let client = OpenAICompatibleClient(
@@ -144,23 +153,42 @@ final class LLMClientTests: XCTestCase {
model: "m",
session: session
)
// We don't assert a specific error type here URLSession's
// cancellation surface is platform-quirky. The contract under test
// is just "throws something instead of silently returning the
// raw transcript"; that something is then handled by
// KeyboardViewController.handleFinalTranscript's catch ladder.
do {
_ = try await client.polish("hi", systemPrompt: "p")
// The stub returns 200 with empty body immediately, which would
// decode to a valid empty content. That still proves the
// path doesn't crash so we don't XCTFail if the stub won the
// race. The other tests (noAPIKey, 401, 429) already cover
// the typed-error ladder.
} catch {
// Any throwable counts as success for the "doesn't crash"
// contract.
_ = error
let task = Task<Bool, Error> {
do {
_ = try await client.polish("hi", systemPrompt: "p")
return false // completed unexpected
} catch {
throw error
}
}
// Give the request a head start so it's already on the wire when
// we cancel.
try? await Task.sleep(nanoseconds: 50_000_000) // 50 ms
task.cancel()
var threw = false
var caughtTransportish = false
do {
_ = try await task.value
} catch is CancellationError {
threw = true
} catch let err as LLMError {
threw = true
// We accept any of: cancelled, transport, decoding the URL
// stack is platform-quirky about how it surfaces a cancelled
// request from inside URLSession's protocol handler.
switch err {
case .cancelled, .transport, .decoding:
caughtTransportish = true
default:
break
}
} catch {
threw = true
}
XCTAssertTrue(threw, "expected client.polish to throw on cancelled transport")
XCTAssertTrue(caughtTransportish, "expected .cancelled / .transport / .decoding — got something else")
}
/// Cross-process App Group contract: what `ProviderConfig` writes must
@@ -219,6 +247,69 @@ final class LLMClientTests: XCTestCase {
XCTFail("wrong error: \(error)")
}
}
// MARK: - TEST-2: mode = .off short-circuits PolishingService
/// `PolishingService.polish()` must not invoke the underlying
/// `LLMClient` when the App Group store reports `modeId == "off"`.
/// We verify both halves of that contract:
/// 1. The return value is the trimmed input (not a polished round-trip).
/// 2. The `LLMClient` is never asked to talk to the network.
func testPolisherSkipsNetworkWhenModeOff() async throws {
let suiteName = "group.com.osgkeyboard.shared.tests.\(UUID().uuidString)"
let defaults = UserDefaults(suiteName: suiteName)!
defaults.removePersistentDomain(forName: suiteName)
defer { defaults.removePersistentDomain(forName: suiteName) }
// modeId = "off" this is the switch we care about.
defaults.set("off", forKey: "config.modeId")
defaults.set("https://example.com/v1", forKey: "config.baseURL")
defaults.set("sk-should-not-be-used", forKey: "config.apiKey")
defaults.set("gpt-4o-mini", forKey: "config.model")
// Counter LLMClient: if `polish()` is ever called, this trips.
let counter = CallCounter()
let countingClient = CountingLLMClient(counter: counter) { _, _ in
XCTFail("LLMClient.polish was invoked under mode=off — short-circuit failed")
return ""
}
let store = AppGroupStore(defaults: defaults)
let polisher = PolishingService(
store: store,
client: countingClient,
timeout: 1
)
let result = try await polisher.polish(" hello world ")
XCTAssertEqual(result, "hello world", "mode=off must return trimmed input, not polished output")
let calls = await counter.value()
XCTAssertEqual(calls, 0, "LLMClient.polish must not be called when modeId == \"off\"")
}
}
// MARK: - Test helpers
/// Thread-safe counter for proving a call site never invoked the LLM.
private actor CallCounter {
private(set) var n = 0
func bump() { n += 1 }
func value() -> Int { n }
}
/// Minimal `LLMClient` that records each call and forwards to a user-
/// supplied closure. Used by tests that need to prove a particular
/// code path *did not* invoke the client.
private struct CountingLLMClient: LLMClient {
let counter: CallCounter
let body: @Sendable (String, String) async throws -> String
var requestTimeout: TimeInterval { 15 }
func polish(_ text: String, systemPrompt: String) async throws -> String {
await counter.bump()
return try await body(text, systemPrompt)
}
}
// MARK: - URLProtocol stub
@@ -227,6 +318,7 @@ final class LLMClientTests: XCTestCase {
/// before invoking the code under test, then reset to nil in cleanup.
private enum StubURLProtocolStorage {
nonisolated(unsafe) static var config: (statusCode: Int, body: Data)?
nonisolated(unsafe) static var delaySeconds: Double = 0
nonisolated(unsafe) static var lastRequest: URLRequest?
}
@@ -236,16 +328,26 @@ private final class StubURLProtocol: URLProtocol, @unchecked Sendable {
override func startLoading() {
let cfg = StubURLProtocolStorage.config ?? (statusCode: 200, body: Data())
let delay = StubURLProtocolStorage.delaySeconds
StubURLProtocolStorage.lastRequest = request
let response = HTTPURLResponse(
url: request.url!,
statusCode: cfg.statusCode,
httpVersion: "HTTP/1.1",
headerFields: ["Content-Type": "application/json"]
)!
client?.urlProtocol(self, didReceive: response, cacheStoragePolicy: .notAllowed)
client?.urlProtocol(self, didLoad: cfg.body)
client?.urlProtocolDidFinishLoading(self)
// Simulate a slow transport. We honour URLProtocol.stopLoading() so
// cancellation doesn't leave the test hanging, and we yield to the
// run loop so `URLSession.data(for:)` actually observes the delay
// (a busy-wait would never let the cooperative scheduler time out).
DispatchQueue.global(qos: .userInitiated).asyncAfter(deadline: .now() + delay) { [weak self] in
guard let self else { return }
guard self.client != nil else { return }
let response = HTTPURLResponse(
url: self.request.url!,
statusCode: cfg.statusCode,
httpVersion: "HTTP/1.1",
headerFields: ["Content-Type": "application/json"]
)!
self.client?.urlProtocol(self, didReceive: response, cacheStoragePolicy: .notAllowed)
self.client?.urlProtocol(self, didLoad: cfg.body)
self.client?.urlProtocolDidFinishLoading(self)
}
}
override func stopLoading() {}