06/23 Fix Medium//Low-impact issues

JanitorialQC/ContentView.swift

@@ -7,6 +7,7 @@ struct ContentView: View {
     @EnvironmentObject private var auth: AuthManager
     @EnvironmentObject private var sync: SyncManager
     @StateObject private var updateChecker = UpdateChecker.shared
+    @Environment(\.scenePhase) private var scenePhase
 
     var body: some View {
         Group {
@@ -42,6 +43,27 @@ struct ContentView: View {
             //    normal app use.
             await updateChecker.checkForUpdate()
         }
+        // Stop the 60s notification poll when the app goes to background and
+        // restart it when it returns to the foreground. iOS suspends Tasks
+        // automatically in the background anyway, but explicitly managing the
+        // poll task here:
+        //   • Prevents the Task object from accumulating sleep-resume cycles
+        //     that never fired while suspended.
+        //   • Triggers an immediate sync+poll when the inspector returns to the
+        //     app after being away, rather than waiting up to 60s for the next
+        //     scheduled poll tick.
+        //   • Makes the lifecycle intent explicit and avoids relying on implicit
+        //     iOS suspension behaviour.
+        .onChange(of: scenePhase) { _, newPhase in
+            switch newPhase {
+            case .background:
+                SyncManager.shared.suspendPolling()
+            case .active:
+                SyncManager.shared.resumePolling()
+            default:
+                break
+            }
+        }
         .alert("Update Available", isPresented: $updateChecker.updateAvailable) {
             Button("Update") {
                 if let url = updateChecker.appStoreURL {

JanitorialQC/Models/LocalIssue.swift

@@ -32,16 +32,60 @@ final class LocalIssue {
     private static let jsonDecoder = JSONDecoder()
     private static let jsonEncoder = JSONEncoder()
 
+    // Lightweight decode cache — avoids re-parsing identical JSON strings.
+    // SwiftData may call the getter multiple times per render pass (once for
+    // isEmpty, once for count, once for ForEach). Caching the last-decoded
+    // value by JSON string identity means the JSON parse only happens when
+    // the underlying data actually changes.
+    // @Transient tells SwiftData not to persist these — they're in-memory only.
+    // Lightweight decode cache — split into key+value pairs because SwiftData's
+    // @Transient macro does not support tuple types. Two separate @Transient
+    // properties per cache entry achieve the same result with no schema impact.
+    @Transient private var _cachedLocalKey:    String   = ""
+    @Transient private var _cachedLocalValue:  [String] = []
+    @Transient private var _cachedServerKey:   String   = ""
+    @Transient private var _cachedServerValue: [String] = []
+
     /// Decoded local photo paths (up to 5)
     var photoLocalPaths: [String] {
-        get { (try? Self.jsonDecoder.decode([String].self, from: Data(photoLocalPathsJSON.utf8))) ?? [] }
-        set { photoLocalPathsJSON = (try? String(data: Self.jsonEncoder.encode(newValue), encoding: .utf8)) ?? "[]" }
+        get {
+            if _cachedLocalKey == photoLocalPathsJSON, !_cachedLocalKey.isEmpty {
+                return _cachedLocalValue
+            }
+            let decoded = (try? Self.jsonDecoder.decode([String].self,
+                           from: Data(photoLocalPathsJSON.utf8))) ?? []
+            _cachedLocalKey   = photoLocalPathsJSON
+            _cachedLocalValue = decoded
+            return decoded
+        }
+        set {
+            let encoded = (try? String(data: Self.jsonEncoder.encode(newValue),
+                                       encoding: .utf8)) ?? "[]"
+            photoLocalPathsJSON = encoded
+            _cachedLocalKey     = encoded
+            _cachedLocalValue   = newValue
+        }
     }
 
     /// Decoded server photo paths
     var photoServerPaths: [String] {
-        get { (try? Self.jsonDecoder.decode([String].self, from: Data(photoServerPathsJSON.utf8))) ?? [] }
-        set { photoServerPathsJSON = (try? String(data: Self.jsonEncoder.encode(newValue), encoding: .utf8)) ?? "[]" }
+        get {
+            if _cachedServerKey == photoServerPathsJSON, !_cachedServerKey.isEmpty {
+                return _cachedServerValue
+            }
+            let decoded = (try? Self.jsonDecoder.decode([String].self,
+                           from: Data(photoServerPathsJSON.utf8))) ?? []
+            _cachedServerKey   = photoServerPathsJSON
+            _cachedServerValue = decoded
+            return decoded
+        }
+        set {
+            let encoded = (try? String(data: Self.jsonEncoder.encode(newValue),
+                                       encoding: .utf8)) ?? "[]"
+            photoServerPathsJSON = encoded
+            _cachedServerKey     = encoded
+            _cachedServerValue   = newValue
+        }
     }
 
     var createdAt: Date

JanitorialQC/Models/LocalTemplate.swift

@@ -15,6 +15,10 @@ final class LocalTemplate {
     var formSchemaJSON: String
     var lastSyncedAt: Date
     var isActive: Bool = true   // phase21 — false templates excluded from picker
+    /// Timestamp of the last successful schema fetch (GET /api/v1/templates/{id}).
+    /// Nil when the schema has never been fetched (e.g. template just inserted).
+    /// Used to skip redundant detail calls when summary fields are unchanged.
+    var schemaFetchedAt: Date? = nil
 
     init(from summary: APITemplateSummary) {
         self.serverId            = summary.id
@@ -26,12 +30,20 @@ final class LocalTemplate {
         self.isActive            = summary.isActive
     }
 
-    func updateSummary(from summary: APITemplateSummary) {
+    /// Update summary fields and return whether any field changed.
+    /// Used by pullReferenceData to skip schema re-fetching when nothing changed.
+    @discardableResult
+    func updateSummary(from summary: APITemplateSummary) -> Bool {
+        let changed = name != summary.name
+                   || templateDescription != summary.description
+                   || frequency           != summary.frequency
+                   || isActive            != summary.isActive
         self.name                = summary.name
         self.templateDescription = summary.description
         self.frequency           = summary.frequency
         self.isActive            = summary.isActive
         self.lastSyncedAt        = Date()
+        return changed
     }
 
     func updateSchema(from template: APITemplate) {
@@ -43,7 +55,8 @@ final class LocalTemplate {
            let str  = String(data: data, encoding: .utf8) {
             self.formSchemaJSON = str
         }
-        self.lastSyncedAt = Date()
+        self.schemaFetchedAt = Date()
+        self.lastSyncedAt    = Date()
     }
 
     /// Decode stored JSON back into [[String: Any]] for the form renderer

JanitorialQC/Models/SyncQueueEntry.swift

@@ -1,8 +1,17 @@
 // Models/SyncQueueEntry.swift
 // ---------------------------
-// SwiftData model for the outbox sync queue.
-// Every offline write (inspection, issue, photo) enqueues an entry here.
-// SyncManager processes entries in FIFO order when connectivity is restored.
+// LEGACY — This model is no longer used. The original design enqueued every
+// offline write here and processed in FIFO order; the current architecture uses
+// LocalInspection.syncStatus / LocalIssue.syncStatus / PendingPhoto.uploadStatus
+// directly (simpler, fewer moving parts, no double-bookkeeping).
+//
+// The model is kept registered in the SwiftData container solely to maintain
+// schema compatibility with existing installs — removing it from modelContainer
+// would trigger a migration failure on devices that already have the table.
+// A future dedicated migration (phase N) can drop the table explicitly using
+// op.execute("DROP TABLE IF EXISTS SyncQueueEntry") once it's safe to do so.
+//
+// DO NOT add new code that reads or writes this model.
 
 import Foundation
 import SwiftData

JanitorialQC/Sync/SyncManager.swift

@@ -118,6 +118,21 @@ class SyncManager: ObservableObject {
         stopPollTask()
     }
 
+    /// Called when the app enters the background (scenePhase == .background).
+    /// Stops the poll loop so it doesn't accumulate suspended sleep cycles.
+    func suspendPolling() {
+        stopPollTask()
+    }
+
+    /// Called when the app returns to the foreground (scenePhase == .active).
+    /// Restarts the poll loop and immediately syncs so stale data is refreshed
+    /// without waiting up to 60s for the next scheduled tick.
+    func resumePolling() {
+        guard isOnline, AuthManager.shared.isAuthenticated else { return }
+        startPollTask()
+        Task { await triggerSync() }
+    }
+
     /// Call when the user opens the NotificationsView to clear the badge.
     func markNotificationsViewed() {
         unreadNotificationCount = 0
@@ -136,8 +151,13 @@ class SyncManager: ObservableObject {
                 deliverLocalNotification(n)
             }
 
-            // Update in-app inbox state
-            recentNotifications = notifications + recentNotifications.prefix(50 - notifications.count)
+            // Update in-app inbox state.
+            // Prepend new notifications and cap at 50 — avoids allocating two
+            // arrays and concatenating them on every poll (the old pattern
+            // `notifications + recentNotifications.prefix(50 - count)` always
+            // created a new array even when notifications.count >= 50).
+            recentNotifications.insert(contentsOf: notifications, at: 0)
+            if recentNotifications.count > 50 { recentNotifications = Array(recentNotifications.prefix(50)) }
             unreadNotificationCount += notifications.count
 
             // Update the cursor to the newest notification's timestamp so the
@@ -493,13 +513,18 @@ class SyncManager: ObservableObject {
             )
 
             for apiSummary in templatesData.templates {
+                let summaryChanged: Bool
                 if let existing = templateMap[apiSummary.id] {
-                    existing.updateSummary(from: apiSummary)
+                    summaryChanged = existing.updateSummary(from: apiSummary)
                 } else {
                     context.insert(LocalTemplate(from: apiSummary))
+                    summaryChanged = true   // new template — must fetch schema
                 }
                 try await upsertTemplateSchema(
-                    id: apiSummary.id, context: context, templateMap: templateMap
+                    id: apiSummary.id,
+                    context: context,
+                    templateMap: templateMap,
+                    summaryChanged: summaryChanged
                 )
             }
 
@@ -543,14 +568,8 @@ class SyncManager: ObservableObject {
         guard last == nil || Date().timeIntervalSince(last!) >= Self.cleanupInterval else { return }
         UserDefaults.standard.set(Date(), forKey: Self.lastCleanupKey)
 
-        let fm = FileManager.default
-        guard let docsDir = fm.urls(for: .documentDirectory, in: .userDomainMask).first else { return }
-        let photosDir = docsDir.appendingPathComponent("JQCPhotos")
-        guard let diskFiles = try? fm.contentsOfDirectory(
-            at: photosDir, includingPropertiesForKeys: nil
-        ) else { return }
-
-        // Collect all local paths that are still in use.
+        // ── Phase 1: collect referenced paths on @MainActor (SwiftData fetches) ──
+        // These are fast in-memory operations — always runs on the main actor.
         var referencedPaths = Set<String>()
 
         // PendingPhoto — not yet uploaded
@@ -576,17 +595,29 @@ class SyncManager: ObservableObject {
             }
         }
 
-        // Delete any disk file not in referencedPaths
-        var deletedCount = 0
-        for fileURL in diskFiles {
-            let path = fileURL.path
-            if !referencedPaths.contains(path) {
-                try? fm.removeItem(at: fileURL)
-                deletedCount += 1
+        // ── Phase 2: FileManager enumeration + deletion on a background thread ──
+        // Directory enumeration and file removal are I/O-bound and can stutter
+        // the main thread when JQCPhotos/ contains hundreds of files. Dispatching
+        // here is safe because `referencedPaths` is a value type (Set<String>)
+        // captured by copy — no shared mutable state crosses the boundary.
+        Task.detached(priority: .utility) {
+            let fm = FileManager.default
+            guard let docsDir = fm.urls(for: .documentDirectory, in: .userDomainMask).first else { return }
+            let photosDir = docsDir.appendingPathComponent("JQCPhotos")
+            guard let diskFiles = try? fm.contentsOfDirectory(
+                at: photosDir, includingPropertiesForKeys: nil
+            ) else { return }
+
+            var deletedCount = 0
+            for fileURL in diskFiles {
+                if !referencedPaths.contains(fileURL.path) {
+                    try? fm.removeItem(at: fileURL)
+                    deletedCount += 1
+                }
+            }
+            if deletedCount > 0 {
+                print("[JQC] Sync | cleanupOrphanedPhotos | removed \(deletedCount) file(s)")
             }
-        }
-        if deletedCount > 0 {
-            print("[JQC] Sync | cleanupOrphanedPhotos | removed \(deletedCount) file(s)")
         }
     }
 
@@ -631,18 +662,33 @@ class SyncManager: ObservableObject {
     private func upsertTemplateSchema(
         id: Int,
         context: ModelContext,
-        templateMap: [Int: LocalTemplate]
+        templateMap: [Int: LocalTemplate],
+        summaryChanged: Bool
     ) async throws {
+        let existing = templateMap[id] ?? (try? context.fetch(FetchDescriptor<LocalTemplate>()))?
+            .first { $0.serverId == id }
+
+        // Skip the detail API call if:
+        //   • The schema was already fetched (schemaFetchedAt is non-nil)
+        //   • No summary fields changed this sync pass (name, frequency, isActive)
+        //   • The local schema is non-empty (not a first-run blank)
+        //
+        // This reduces N sequential GET /api/v1/templates/{id} calls to zero
+        // on a typical sync where templates haven't changed — the common case.
+        // The schema is always re-fetched when any summary field changes,
+        // when schemaFetchedAt is nil (new template or first launch), or
+        // when the local schema is empty ("[]").
+        if let existing,
+           existing.schemaFetchedAt != nil,
+           existing.formSchemaJSON != "[]",
+           !summaryChanged {
+            return   // schema is current — no network call needed
+        }
+
         let detailData: TemplateDetailResponseData = try await APIClient.shared.request(
             "/api/v1/templates/\(id)"
         )
-        if let existing = templateMap[id] {
-            existing.updateSchema(from: detailData.template)
-        } else {
-            (try? context.fetch(FetchDescriptor<LocalTemplate>()))?
-                .first { $0.serverId == id }?
-                .updateSchema(from: detailData.template)
-        }
+        existing?.updateSchema(from: detailData.template)
     }
 
     // ── Pull server-assigned issues ───────────────────────────────────────