refactor: optimize cache and retry handling

raydocs · factory-droid[bot] · raydocs · commit 5e9528be53a5 · 2026-01-30T05:29:05.000-07:00
Co-authored-by: factory-droid[bot] &lt;138933559+factory-droid[bot]@users.noreply.github.com&gt;
diff --git a/.flow/epics/fn-17-vfs.json b/.flow/epics/fn-17-vfs.json
@@ -0,0 +1,13 @@
+{
+  "branch_name": "fn-17-optimization",
+  "created_at": "2026-01-30T12:12:35.947254Z",
+  "depends_on_epics": [],
+  "id": "fn-17-vfs",
+  "next_task": 1,
+  "plan_review_status": "unknown",
+  "plan_reviewed_at": null,
+  "spec_path": ".flow/specs/fn-17-vfs.md",
+  "status": "open",
+  "title": "Optimization cleanup from full scan",
+  "updated_at": "2026-01-30T12:12:35.947723Z"
+}
diff --git a/.flow/specs/fn-17-vfs.md b/.flow/specs/fn-17-vfs.md
@@ -0,0 +1,34 @@
+# fn-17-vfs Optimization cleanup from full scan
+
+## Overview
+Implement prioritized improvements from the full scan (cache correctness, retry consistency, cache hygiene, and OCR efficiency).
+
+## Scope
+- WeatherService cache invalidation tied to user location changes
+- GeminiService retry unification with RetryPolicy/withRetry
+- MatchCache expiration cleanup and cacheExpiration usage
+- IngredientOCR CIContext reuse
+
+## Approach
+1) Track last weather location + invalidate cache on significant location change.
+2) Route GeminiService network calls through withRetry to align with RetryPolicy and Retry-After.
+3) Align MatchCache expiration checks with cacheExpiration and clean expired entries consistently.
+4) Reuse CIContext for OCR preprocessing.
+
+## Quick commands
+<!-- Required: at least one smoke command for the repo -->
+- `make test`
+
+## Acceptance
+- [ ] Weather cache invalidation respects location changes
+- [ ] GeminiService retry uses RetryPolicy/withRetry
+- [ ] MatchCache expiration cleanup is consistent
+- [ ] OCR preprocessing reuses CIContext
+- [ ] Tests/linters pass
+
+## References
+- SkinLab/Core/Network/WeatherService.swift
+- SkinLab/Core/Network/GeminiService.swift
+- SkinLab/Core/Network/RetryPolicy.swift
+- SkinLab/Features/Community/Services/MatchCache.swift
+- SkinLab/Core/Utils/IngredientOCR.swift
diff --git a/.flow/tasks/fn-17-vfs.1.json b/.flow/tasks/fn-17-vfs.1.json
@@ -0,0 +1,14 @@
+{
+  "assignee": null,
+  "claim_note": "",
+  "claimed_at": null,
+  "created_at": "2026-01-30T12:14:28.042624Z",
+  "depends_on": [],
+  "epic": "fn-17-vfs",
+  "id": "fn-17-vfs.1",
+  "priority": null,
+  "spec_path": ".flow/tasks/fn-17-vfs.1.md",
+  "status": "todo",
+  "title": "Weather cache invalidation by location",
+  "updated_at": "2026-01-30T12:14:52.218140Z"
+}
diff --git a/.flow/tasks/fn-17-vfs.1.md b/.flow/tasks/fn-17-vfs.1.md
@@ -0,0 +1,15 @@
+# fn-17-vfs.1 Weather cache invalidation by location
+
+## Description
+Update WeatherService caching so location changes invalidate cached weather. Track last fetch location and clear cache when distance exceeds threshold or location is unavailable.
+## Acceptance
+- Cache invalidates on significant location change
+- Existing weather cache still used when location stable and cache fresh
+- Tests/linters pass
+## Done summary
+- add location-aware cache invalidation in WeatherService
+- fetch location before serving cache and clear when distance exceeds threshold
+## Evidence
+- Commits:
+- Tests:
+- PRs:
diff --git a/.flow/tasks/fn-17-vfs.2.json b/.flow/tasks/fn-17-vfs.2.json
@@ -0,0 +1,14 @@
+{
+  "assignee": null,
+  "claim_note": "",
+  "claimed_at": null,
+  "created_at": "2026-01-30T12:14:30.898057Z",
+  "depends_on": [],
+  "epic": "fn-17-vfs",
+  "id": "fn-17-vfs.2",
+  "priority": null,
+  "spec_path": ".flow/tasks/fn-17-vfs.2.md",
+  "status": "todo",
+  "title": "Unify GeminiService retry policy",
+  "updated_at": "2026-01-30T12:15:01.584732Z"
+}
diff --git a/.flow/tasks/fn-17-vfs.2.md b/.flow/tasks/fn-17-vfs.2.md
@@ -0,0 +1,15 @@
+# fn-17-vfs.2 Unify GeminiService retry policy
+
+## Description
+Refactor GeminiService to use the shared RetryPolicy/withRetry utilities, including honoring Retry-After when present, to align with global retry limits.
+## Acceptance
+- GeminiService uses withRetry/RetryPolicy for network calls
+- Retry-After is respected when available
+- Tests/linters pass
+## Done summary
+- refactor GeminiService analysis and ingredient requests to use withRetry
+- centralize response validation and error mapping with Retry-After support
+## Evidence
+- Commits:
+- Tests:
+- PRs:
diff --git a/.flow/tasks/fn-17-vfs.3.json b/.flow/tasks/fn-17-vfs.3.json
@@ -0,0 +1,14 @@
+{
+  "assignee": null,
+  "claim_note": "",
+  "claimed_at": null,
+  "created_at": "2026-01-30T12:14:33.495635Z",
+  "depends_on": [],
+  "epic": "fn-17-vfs",
+  "id": "fn-17-vfs.3",
+  "priority": null,
+  "spec_path": ".flow/tasks/fn-17-vfs.3.md",
+  "status": "todo",
+  "title": "MatchCache expiration cleanup",
+  "updated_at": "2026-01-30T12:15:09.264002Z"
+}
diff --git a/.flow/tasks/fn-17-vfs.3.md b/.flow/tasks/fn-17-vfs.3.md
@@ -0,0 +1,15 @@
+# fn-17-vfs.3 MatchCache expiration cleanup
+
+## Description
+Make MatchCache expiration behavior consistent: use cacheExpiration constant in isExpired checks and remove expired entries when accessed.
+## Acceptance
+- cacheExpiration constant is used consistently
+- Expired entries are invalidated on read paths
+- Tests/linters pass
+## Done summary
+- align MatchCache expiration checks with cacheExpiration constant
+- invalidate expired entries in recommendation and entry accessors
+## Evidence
+- Commits:
+- Tests:
+- PRs:
diff --git a/.flow/tasks/fn-17-vfs.4.json b/.flow/tasks/fn-17-vfs.4.json
@@ -0,0 +1,14 @@
+{
+  "assignee": null,
+  "claim_note": "",
+  "claimed_at": null,
+  "created_at": "2026-01-30T12:14:36.516188Z",
+  "depends_on": [],
+  "epic": "fn-17-vfs",
+  "id": "fn-17-vfs.4",
+  "priority": null,
+  "spec_path": ".flow/tasks/fn-17-vfs.4.md",
+  "status": "todo",
+  "title": "Reuse CIContext in IngredientOCR",
+  "updated_at": "2026-01-30T12:15:15.463784Z"
+}
diff --git a/.flow/tasks/fn-17-vfs.4.md b/.flow/tasks/fn-17-vfs.4.md
@@ -0,0 +1,14 @@
+# fn-17-vfs.4 Reuse CIContext in IngredientOCR
+
+## Description
+Reuse a shared CIContext in IngredientOCR preprocessing to avoid repeated context creation overhead.
+## Acceptance
+- IngredientOCR uses shared CIContext
+- Behavior unchanged for preprocessing output
+- Tests/linters pass
+## Done summary
+- reuse a shared CIContext inside IngredientOCRService preprocessing
+## Evidence
+- Commits:
+- Tests:
+- PRs:
diff --git a/SkinLab/Core/Network/GeminiService.swift b/SkinLab/Core/Network/GeminiService.swift
@@ -91,14 +91,13 @@ actor GeminiService: SkinAnalysisServiceProtocol {
 
     /// Protocol conformance
     func analyzeSkin(image: UIImage) async throws -> SkinAnalysis {
-        try await analyzeSkin(image: image, previousAnalysis: nil, retryCount: 0)
+        try await analyzeSkin(image: image, previousAnalysis: nil)
     }
 
     /// Enhanced version with context
     func analyzeSkin(
         image: UIImage,
-        previousAnalysis: SkinAnalysis? = nil,
-        retryCount: Int = 0
+        previousAnalysis: SkinAnalysis? = nil
     ) async throws -> SkinAnalysis {
         guard !apiKey.isEmpty else {
             throw GeminiError.invalidAPIKey
@@ -118,47 +117,12 @@ actor GeminiService: SkinAnalysisServiceProtocol {
         )
 
         do {
-            let (data, response) = try await session.data(for: request)
-
-            guard let httpResponse = response as? HTTPURLResponse else {
-                throw GeminiError.networkError(URLError(.badServerResponse))
-            }
-
-            switch httpResponse.statusCode {
-            case 200:
-                return try parseAnalysisResponse(data)
-            case 401:
-                throw GeminiError.unauthorized
-            case 429:
-                // Rate limited - retry with exponential backoff
-                if retryCount < AppConfiguration.API.maxRetryAttempts {
-                    let delay = pow(2.0, Double(retryCount)) // 1s, 2s, 4s
-                    try await Task.sleep(nanoseconds: UInt64(delay * 1_000_000_000))
-                    return try await analyzeSkin(
-                        image: image,
-                        previousAnalysis: previousAnalysis,
-                        retryCount: retryCount + 1
-                    )
-                }
-                throw GeminiError.rateLimited
-            default:
-                let errorMessage = String(data: data, encoding: .utf8) ?? "Unknown error"
-                throw GeminiError.apiError(errorMessage)
-            }
-        } catch let error as GeminiError {
-            throw error
+            let data = try await requestDataWithRetry(for: request)
+            return try parseAnalysisResponse(data)
+        } catch is CancellationError {
+            throw CancellationError()
         } catch {
-            // Network error - retry
-            if retryCount < AppConfiguration.API.maxNetworkRetryAttempts {
-                let delay = pow(2.0, Double(retryCount))
-                try await Task.sleep(nanoseconds: UInt64(delay * 1_000_000_000))
-                return try await analyzeSkin(
-                    image: image,
-                    previousAnalysis: previousAnalysis,
-                    retryCount: retryCount + 1
-                )
-            }
-            throw GeminiError.networkError(error)
+            throw mapRequestError(error)
         }
     }
 
@@ -237,6 +201,50 @@ actor GeminiService: SkinAnalysisServiceProtocol {
         return request
     }
 
+    private func requestDataWithRetry(for request: URLRequest) async throws -> Data {
+        try await withRetry {
+            let (data, response) = try await session.data(for: request)
+            return try validateResponseData(data, response: response)
+        }
+    }
+
+    private func validateResponseData(_ data: Data, response: URLResponse) throws -> Data {
+        guard let httpResponse = response as? HTTPURLResponse else {
+            throw GeminiError.networkError(URLError(.badServerResponse))
+        }
+
+        switch httpResponse.statusCode {
+        case 200:
+            return data
+        case 401:
+            throw GeminiError.unauthorized
+        case 408, 429:
+            throw HTTPError(statusCode: httpResponse.statusCode, data: data, response: httpResponse)
+        case 500 ... 599:
+            throw HTTPError(statusCode: httpResponse.statusCode, data: data, response: httpResponse)
+        default:
+            let errorMessage = String(data: data, encoding: .utf8) ?? "Unknown error"
+            throw GeminiError.apiError(errorMessage)
+        }
+    }
+
+    private func mapRequestError(_ error: Error) -> GeminiError {
+        if let geminiError = error as? GeminiError {
+            return geminiError
+        }
+
+        if let httpError = error as? HTTPError {
+            if httpError.statusCode == 429 {
+                return GeminiError.rateLimited
+            }
+            let errorMessage = String(data: httpError.data ?? Data(), encoding: .utf8)
+                ?? "HTTP \(httpError.statusCode)"
+            return GeminiError.apiError(errorMessage)
+        }
+
+        return GeminiError.networkError(error)
+    }
+
     // MARK: - Response Parsing (OpenRouter/OpenAI Format)
 
     private func parseAnalysisResponse(_ data: Data) throws -> SkinAnalysis {
@@ -495,79 +503,15 @@ extension GeminiService: IngredientAIServiceProtocol {
         guard !apiKey.isEmpty else {
             throw GeminiError.invalidAPIKey
         }
-
-        // Retry logic with exponential backoff
-        let maxRetries = AppConfiguration.API.maxRetryAttempts
-        var retryDelay: TimeInterval = 1.0
-
-        for attempt in 0 ..< maxRetries {
-            // Check if task is cancelled
-            if Task.isCancelled {
-                throw CancellationError()
-            }
-
-            do {
-                let apiRequest = try buildIngredientAnalysisRequest(request: request)
-                let (data, response) = try await session.data(for: apiRequest)
-
-                guard let httpResponse = response as? HTTPURLResponse else {
-                    throw GeminiError.networkError(URLError(.badServerResponse))
-                }
-
-                switch httpResponse.statusCode {
-                case 200:
-                    return try parseIngredientAnalysisResponse(data)
-
-                case 401:
-                    throw GeminiError.unauthorized
-
-                case 429:
-                    // Rate limited - respect Retry-After header if present
-                    if let retryAfter = httpResponse.value(forHTTPHeaderField: "Retry-After"),
-                       let seconds = TimeInterval(retryAfter) {
-                        retryDelay = seconds
-                    }
-
-                    // Only retry if we have attempts left
-                    if attempt < maxRetries - 1 {
-                        try await Task.sleep(nanoseconds: UInt64(retryDelay * 1_000_000_000))
-                        retryDelay *= 2 // Exponential backoff
-                        continue
-                    }
-                    throw GeminiError.rateLimited
-
-                case 500 ... 599:
-                    // Server error - retry with backoff
-                    if attempt < maxRetries - 1 {
-                        try await Task.sleep(nanoseconds: UInt64(retryDelay * 1_000_000_000))
-                        retryDelay *= 2
-                        continue
-                    }
-                    let errorMessage = String(data: data, encoding: .utf8) ?? "Server error"
-                    throw GeminiError.apiError(errorMessage)
-
-                default:
-                    let errorMessage = String(data: data, encoding: .utf8) ?? "Unknown error"
-                    throw GeminiError.apiError(errorMessage)
-                }
-            } catch let error as GeminiError {
-                // Don't retry on client errors (401, invalid key, parse errors)
-                throw error
-            } catch is CancellationError {
-                throw CancellationError()
-            } catch {
-                // Network errors - retry
-                if attempt < maxRetries - 1 {
-                    try await Task.sleep(nanoseconds: UInt64(retryDelay * 1_000_000_000))
-                    retryDelay *= 2
-                    continue
-                }
-                throw GeminiError.networkError(error)
-            }
+        do {
+            let apiRequest = try buildIngredientAnalysisRequest(request: request)
+            let data = try await requestDataWithRetry(for: apiRequest)
+            return try parseIngredientAnalysisResponse(data)
+        } catch is CancellationError {
+            throw CancellationError()
+        } catch {
+            throw mapRequestError(error)
         }
-
-        // Should never reach here
-        throw GeminiError.networkError(URLError(.unknown))
     }
 
     private func buildIngredientAnalysisRequest(request: IngredientAIRequest) throws -> URLRequest {
diff --git a/SkinLab/Core/Network/WeatherService.swift b/SkinLab/Core/Network/WeatherService.swift
diff --git a/SkinLab/Core/Utils/IngredientOCR.swift b/SkinLab/Core/Utils/IngredientOCR.swift
diff --git a/SkinLab/Features/Community/Services/MatchCache.swift b/SkinLab/Features/Community/Services/MatchCache.swift
