From f39527b1d88140b78ae2bbfe702b59a3b4b20902 Mon Sep 17 00:00:00 2001 From: Rune Olsen Date: Thu, 23 Jul 2026 13:48:41 +0200 Subject: [PATCH] Phase 2 seam: expose pure helpers for testing (no logic changes) Every change here is either dropping `private` (still invisible outside the module, @testable import just needs internal-or-wider) or converting a self-independent instance method to `static func` (ChatViewModel.inferProvider/calculateCost/detectGoodbyePhrase -- none of the three ever touched `self`, and constructing a real ChatViewModel triggers a real network call in init, so static-ifying them sidesteps that entirely rather than fighting it). Call sites updated to `Self.foo(...)` where the static conversion required it. Touches: GitSyncService's URL/secret-scanning helpers, EmbeddingService's embedding (de)serialization, ContextSelectionService's importance scoring, and the request-building/response-parsing helpers on all four providers (OpenRouter, Ollama, OpenAI, Anthropic) -- the core AI request/response layer, previously 100% untested. Verified: full existing test suite (27 tests) still green, no regressions. Tests for these functions land in the next commit. Phase 2 of the test-suite rollout plan (peaceful-baking-kurzweil). --- oAI/Providers/AnthropicProvider.swift | 6 +++--- oAI/Providers/OllamaProvider.swift | 4 ++-- oAI/Providers/OpenAIProvider.swift | 6 +++--- oAI/Providers/OpenRouterProvider.swift | 8 ++++---- oAI/Services/ContextSelectionService.swift | 4 ++-- oAI/Services/EmbeddingService.swift | 4 ++-- oAI/Services/GitSyncService.swift | 8 ++++---- oAI/ViewModels/ChatViewModel.swift | 24 +++++++++++----------- 8 files changed, 32 insertions(+), 32 deletions(-) diff --git a/oAI/Providers/AnthropicProvider.swift b/oAI/Providers/AnthropicProvider.swift index 1f9ea94..dcebe80 100644 --- a/oAI/Providers/AnthropicProvider.swift +++ b/oAI/Providers/AnthropicProvider.swift @@ -571,7 +571,7 @@ class AnthropicProvider: AIProvider { // MARK: - Request Building - private func buildURLRequest(from request: ChatRequest, stream: Bool) throws -> (URLRequest, Data) { + func buildURLRequest(from request: ChatRequest, stream: Bool) throws -> (URLRequest, Data) { let url = messagesURL // Separate system message @@ -685,7 +685,7 @@ class AnthropicProvider: AIProvider { return (urlRequest, bodyData) } - private func parseResponse(data: Data) throws -> ChatResponse { + func parseResponse(data: Data) throws -> ChatResponse { guard let json = try? JSONSerialization.jsonObject(with: data) as? [String: Any] else { throw ProviderError.invalidResponse } @@ -741,7 +741,7 @@ class AnthropicProvider: AIProvider { ) } - private func convertParametersToDict(_ params: Tool.Function.Parameters) -> [String: Any] { + func convertParametersToDict(_ params: Tool.Function.Parameters) -> [String: Any] { var props: [String: Any] = [:] for (key, prop) in params.properties { var propDict: [String: Any] = [ diff --git a/oAI/Providers/OllamaProvider.swift b/oAI/Providers/OllamaProvider.swift index 2a5bd19..8ebcab3 100644 --- a/oAI/Providers/OllamaProvider.swift +++ b/oAI/Providers/OllamaProvider.swift @@ -275,7 +275,7 @@ class OllamaProvider: AIProvider { // MARK: - Helpers - private func buildRequestBody(from request: ChatRequest, stream: Bool) -> [String: Any] { + func buildRequestBody(from request: ChatRequest, stream: Bool) -> [String: Any] { var messages: [[String: Any]] = [] // Add system prompt as a system message @@ -301,7 +301,7 @@ class OllamaProvider: AIProvider { return body } - private func parseOllamaResponse(_ json: [String: Any], model: String) -> ChatResponse { + func parseOllamaResponse(_ json: [String: Any], model: String) -> ChatResponse { let message = json["message"] as? [String: Any] let content = message?["content"] as? String ?? "" let promptTokens = json["prompt_eval_count"] as? Int ?? 0 diff --git a/oAI/Providers/OpenAIProvider.swift b/oAI/Providers/OpenAIProvider.swift index 037ef60..c99d647 100644 --- a/oAI/Providers/OpenAIProvider.swift +++ b/oAI/Providers/OpenAIProvider.swift @@ -117,7 +117,7 @@ class OpenAIProvider: AIProvider { } } - private func fallbackModels() -> [ModelInfo] { + func fallbackModels() -> [ModelInfo] { Self.knownModels.map { id, info in ModelInfo( id: id, @@ -283,7 +283,7 @@ class OpenAIProvider: AIProvider { // MARK: - Helpers - private func buildURLRequest(from request: ChatRequest, stream: Bool) throws -> URLRequest { + func buildURLRequest(from request: ChatRequest, stream: Bool) throws -> URLRequest { let url = URL(string: "\(baseURL)/chat/completions")! var apiMessages: [[String: Any]] = [] @@ -358,7 +358,7 @@ class OpenAIProvider: AIProvider { return urlRequest } - private func convertToChatResponse(_ apiResponse: OpenRouterChatResponse) -> ChatResponse { + func convertToChatResponse(_ apiResponse: OpenRouterChatResponse) -> ChatResponse { guard let choice = apiResponse.choices.first else { return ChatResponse(id: apiResponse.id, model: apiResponse.model, content: "", role: "assistant", finishReason: nil, usage: nil, created: Date()) } diff --git a/oAI/Providers/OpenRouterProvider.swift b/oAI/Providers/OpenRouterProvider.swift index 2e63b87..309e6a9 100644 --- a/oAI/Providers/OpenRouterProvider.swift +++ b/oAI/Providers/OpenRouterProvider.swift @@ -416,7 +416,7 @@ class OpenRouterProvider: AIProvider { // MARK: - Helper Methods - private func buildAPIRequest(from request: ChatRequest) throws -> OpenRouterChatRequest { + func buildAPIRequest(from request: ChatRequest) throws -> OpenRouterChatRequest { let apiMessages = request.messages.map { message -> OpenRouterChatRequest.APIMessage in let hasAttachments = message.attachments?.contains(where: { $0.data != nil }) ?? false @@ -500,7 +500,7 @@ class OpenRouterProvider: AIProvider { ) } - private func convertToChatResponse(_ apiResponse: OpenRouterChatResponse) throws -> ChatResponse { + func convertToChatResponse(_ apiResponse: OpenRouterChatResponse) throws -> ChatResponse { guard let choice = apiResponse.choices.first else { throw ProviderError.invalidResponse } @@ -540,7 +540,7 @@ class OpenRouterProvider: AIProvider { ) } - private func convertToStreamChunk(_ apiChunk: OpenRouterStreamChunk) throws -> StreamChunk { + func convertToStreamChunk(_ apiChunk: OpenRouterStreamChunk) throws -> StreamChunk { guard let choice = apiChunk.choices.first else { throw ProviderError.invalidResponse } @@ -579,7 +579,7 @@ class OpenRouterProvider: AIProvider { } /// Decode base64 data URL images from API response - private func decodeImageOutputs(_ outputs: [OpenRouterChatResponse.ImageOutput]) -> [Data]? { + func decodeImageOutputs(_ outputs: [OpenRouterChatResponse.ImageOutput]) -> [Data]? { let decoded = outputs.compactMap { output -> Data? in let url = output.imageUrl.url // Strip "data:image/...;base64," prefix diff --git a/oAI/Services/ContextSelectionService.swift b/oAI/Services/ContextSelectionService.swift index c77d5ae..310f735 100644 --- a/oAI/Services/ContextSelectionService.swift +++ b/oAI/Services/ContextSelectionService.swift @@ -214,7 +214,7 @@ final class ContextSelectionService { // MARK: - Importance Scoring /// Calculate importance score (0.0 - 1.0) for a message - private func getImportanceScore(_ message: Message) -> Double { + func getImportanceScore(_ message: Message) -> Double { var score = 0.0 // Factor 1: Cost (expensive calls are important) @@ -248,7 +248,7 @@ final class ContextSelectionService { // MARK: - Token Estimation /// Estimate token count for messages (rough approximation) - private func estimateTokens(_ messages: [Message]) -> Int { + func estimateTokens(_ messages: [Message]) -> Int { var total = 0 for message in messages { if let tokens = message.tokens { diff --git a/oAI/Services/EmbeddingService.swift b/oAI/Services/EmbeddingService.swift index 2461c15..5d523e8 100644 --- a/oAI/Services/EmbeddingService.swift +++ b/oAI/Services/EmbeddingService.swift @@ -348,7 +348,7 @@ final class EmbeddingService { // MARK: - Serialization /// Serialize embedding to binary data (4 bytes per float, little-endian) - private func serializeEmbedding(_ embedding: [Float]) -> Data { + func serializeEmbedding(_ embedding: [Float]) -> Data { var data = Data(capacity: embedding.count * 4) for value in embedding { var littleEndian = value.bitPattern.littleEndian @@ -360,7 +360,7 @@ final class EmbeddingService { } /// Deserialize embedding from binary data - private func deserializeEmbedding(_ data: Data) -> [Float] { + func deserializeEmbedding(_ data: Data) -> [Float] { var embedding: [Float] = [] embedding.reserveCapacity(data.count / 4) diff --git a/oAI/Services/GitSyncService.swift b/oAI/Services/GitSyncService.swift index f087afb..d6dde1f 100644 --- a/oAI/Services/GitSyncService.swift +++ b/oAI/Services/GitSyncService.swift @@ -507,7 +507,7 @@ class GitSyncService { } } - private func detectSecretsInText(_ text: String) -> [String] { + func detectSecretsInText(_ text: String) -> [String] { let patterns: [(name: String, pattern: String)] = [ ("OpenAI Key", "sk-[a-zA-Z0-9]{32,}"), ("Anthropic Key", "sk-ant-[a-zA-Z0-9_-]+"), @@ -610,7 +610,7 @@ class GitSyncService { } } - private func convertToSSH(_ url: String) -> String { + func convertToSSH(_ url: String) -> String { // If already SSH format, return as-is if url.hasPrefix("git@") { return url @@ -642,7 +642,7 @@ class GitSyncService { return url } - private func injectCredentials(_ url: String, username: String, password: String) -> String { + func injectCredentials(_ url: String, username: String, password: String) -> String { // Convert https://github.com/user/repo.git // To: https://username:password@github.com/user/repo.git @@ -697,7 +697,7 @@ class GitSyncService { } } - private func sanitizeFilename(_ name: String) -> String { + func sanitizeFilename(_ name: String) -> String { // Remove invalid filename characters let invalid = CharacterSet(charactersIn: "/\\:*?\"<>|") return name.components(separatedBy: invalid).joined(separator: "-") diff --git a/oAI/ViewModels/ChatViewModel.swift b/oAI/ViewModels/ChatViewModel.swift index 7b3f88e..0561643 100644 --- a/oAI/ViewModels/ChatViewModel.swift +++ b/oAI/ViewModels/ChatViewModel.swift @@ -413,15 +413,15 @@ Don't narrate future actions ("Let me...") - just use the tools. /// Update the selected model and keep currentProvider + settings in sync. /// Call this whenever the user picks a model in the model selector. func selectModel(_ model: ModelInfo) { - let newProvider = inferProvider(from: model.id) ?? currentProvider + let newProvider = Self.inferProvider(from: model.id) ?? currentProvider selectedModel = model currentProvider = newProvider MCPService.shared.resetBashSessionApproval() } - func inferProviderPublic(from modelId: String) -> Settings.Provider? { inferProvider(from: modelId) } + func inferProviderPublic(from modelId: String) -> Settings.Provider? { Self.inferProvider(from: modelId) } - private func inferProvider(from modelId: String) -> Settings.Provider? { + static func inferProvider(from modelId: String) -> Settings.Provider? { // OpenRouter models always contain a "/" (e.g. "anthropic/claude-3-5-sonnet") if modelId.contains("/") { return .openrouter } // Anthropic direct (e.g. "claude-sonnet-4-5-20250929") @@ -437,7 +437,7 @@ Don't narrate future actions ("Let me...") - just use the tools. /// Shows a system message only on failure or on a successful switch. @MainActor private func switchToConversationModel(_ modelId: String) async { - guard let targetProvider = inferProvider(from: modelId) else { + guard let targetProvider = Self.inferProvider(from: modelId) else { showSystemMessage("⚠️ Could not determine provider for model '\(modelId)' — keeping current model") return } @@ -942,7 +942,7 @@ Don't narrate future actions ("Let me...") - just use the tools. messages[index].tokens = usage.completionTokens if let model = selectedModel { let hasPricing = model.pricing.prompt > 0 || model.pricing.completion > 0 - let cost: Double? = hasPricing ? calculateCost(usage: usage, pricing: model.pricing) : nil + let cost: Double? = hasPricing ? Self.calculateCost(usage: usage, pricing: model.pricing) : nil messages[index].cost = cost sessionStats.addMessage(inputTokens: usage.promptTokens, outputTokens: usage.completionTokens, cost: cost) } @@ -1006,7 +1006,7 @@ Don't narrate future actions ("Let me...") - just use the tools. messages[index].tokens = usage.completionTokens if let model = selectedModel { let hasPricing = model.pricing.prompt > 0 || model.pricing.completion > 0 - let cost: Double? = hasPricing ? calculateCost(usage: usage, pricing: model.pricing) : nil + let cost: Double? = hasPricing ? Self.calculateCost(usage: usage, pricing: model.pricing) : nil messages[index].cost = cost sessionStats.addMessage(inputTokens: usage.promptTokens, outputTokens: usage.completionTokens, cost: cost) } @@ -1306,7 +1306,7 @@ Don't narrate future actions ("Let me...") - just use the tools. sessionStats.addMessage(inputTokens: usage.promptTokens, outputTokens: usage.completionTokens, cost: cost) } } - _ = detectGoodbyePhrase(in: "") + _ = Self.detectGoodbyePhrase(in: "") } catch { if let index = messages.firstIndex(where: { $0.id == messageId }) { messages[index].content = "❌ Image generation failed: \(error.localizedDescription)" @@ -1582,7 +1582,7 @@ Don't narrate future actions ("Let me...") - just use the tools. // Nothing worth showing yet — still record usage/cost for this turn. if let usage = totalUsage, let model = selectedModel { let hasPricing = model.pricing.prompt > 0 || model.pricing.completion > 0 - let cost: Double? = hasPricing ? calculateCost(usage: usage, pricing: model.pricing) : nil + let cost: Double? = hasPricing ? Self.calculateCost(usage: usage, pricing: model.pricing) : nil sessionStats.addMessage( inputTokens: usage.promptTokens, outputTokens: usage.completionTokens, @@ -1607,7 +1607,7 @@ Don't narrate future actions ("Let me...") - just use the tools. // Calculate cost if let usage = totalUsage, let model = selectedModel { let hasPricing = model.pricing.prompt > 0 || model.pricing.completion > 0 - let cost: Double? = hasPricing ? calculateCost(usage: usage, pricing: model.pricing) : nil + let cost: Double? = hasPricing ? Self.calculateCost(usage: usage, pricing: model.pricing) : nil if let index = messages.lastIndex(where: { $0.id == assistantMessage.id }) { messages[index].cost = cost } @@ -2038,7 +2038,7 @@ Don't narrate future actions ("Let me...") - just use the tools. } /// Detect goodbye phrases in user message - func detectGoodbyePhrase(in text: String) -> Bool { + static func detectGoodbyePhrase(in text: String) -> Bool { let lowercased = text.lowercased() let goodbyePhrases = [ "bye", "goodbye", "bye bye", "good bye", @@ -2078,7 +2078,7 @@ Don't narrate future actions ("Let me...") - just use the tools. updateConversationTracking() // Check for goodbye phrase - if detectGoodbyePhrase(in: text) { + if Self.detectGoodbyePhrase(in: text) { Log.ui.info("Goodbye phrase detected - triggering auto-save") // Wait a bit to see if user continues try? await Task.sleep(for: .seconds(30)) @@ -2259,7 +2259,7 @@ Don't narrate future actions ("Let me...") - just use the tools. /// pricing when present: cache writes cost 1.25x the base input rate, cache /// reads cost 0.1x. `usage.promptTokens` is already the uncached remainder — /// it does not need cache tokens subtracted from it. - private func calculateCost(usage: ChatResponse.Usage, pricing: ModelInfo.Pricing) -> Double { + static func calculateCost(usage: ChatResponse.Usage, pricing: ModelInfo.Pricing) -> Double { let inputCost = Double(usage.promptTokens) * pricing.prompt / 1_000_000 let cacheReadCost = Double(usage.cacheReadInputTokens ?? 0) * pricing.prompt * 0.1 / 1_000_000 let cacheWriteCost = Double(usage.cacheCreationInputTokens ?? 0) * pricing.prompt * 1.25 / 1_000_000