06/23 Fix High-impact issues

JanitorialQC/API/APIClient.swift

@@ -277,13 +277,21 @@ actor APIClient {
 
     // ── Notification polling (Phase C) ────────────────────────────────────
 
+    // Shared formatter for the ?since= query parameter.
+    // DateFormatter init is expensive — creating one per fetchNotifications()
+    // call (every 60 seconds) adds unnecessary allocations on the sync cycle.
+    private static let notifSinceFmt: DateFormatter = {
+        let f = DateFormatter()
+        f.locale     = Locale(identifier: "en_US_POSIX")
+        f.dateFormat = "yyyy-MM-dd'T'HH:mm:ss"
+        return f
+    }()
+
     /// Fetch notifications, optionally scoped to those created after `since`.
     func fetchNotifications(since: Date? = nil) async throws -> [APINotification] {
         var ep = "/api/v1/notifications"
         if let since {
-            let fmt = DateFormatter()
+            ep += "?since=\(Self.notifSinceFmt.string(from: since))"
-            fmt.dateFormat = "yyyy-MM-dd'T'HH:mm:ss"
-            ep += "?since=\(fmt.string(from: since))"
         }
         let result: APINotificationsResponseData = try await request(ep)
         return result.notifications

JanitorialQC/CLAUDE.md

@@ -652,7 +652,7 @@ Deletes `LocalIssue` where `serverId != nil`. Preserves `serverId == nil` record
 | 58 | **`ReadOnlyGridFormView` uses `rowView` (GeometryReader + ZStack), NOT a ZStack canvas or LazyVGrid** | ZStack canvas: gaps from unanswered rows because y-offsets are absolute. LazyVGrid: ignores `col` position, flows items sequentially. Correct approach: group fields by original `row` into `RowGroup`s, render each group as a `GeometryReader` that divides width by 12 to get `colW`, positions each field with `.offset(x: colW * (col-1))` and `.frame(width: colW * colSpan)`. `VStack(spacing: 3)` between rows. Row height fixed at 36pt (section headers 28pt). |
 | 59 | **Read-only inspection detail: only answered fields are shown — filtering is 5-pass** | Pass 1: collect `answeredIds` (rating > 0, or non-empty value). Pass 2: collect `visibleLabelIds` (labels immediately before an answered field). Pass 3: collect `visibleSectionIds` (sections with at least one answered field after them). Pass 4: group all schema fields by original `row`. Pass 5: for each row group, emit only visible fields; skip rows with no visible content. |
 | 60 | **`ReadOnlyGridFormView` rows advance by 1 regardless of original `rowSpan`** | The web renders every field with `grid-row: N / span 1`. The read-only view collapses all rowSpans to 1 — no field occupies more than one row of vertical space. |
-| 61 | **`PhotoThumbnailView` owns `@State private var showLightbox`** | `ReadOnlyCellView.valueView` is a computed `@ViewBuilder` — it cannot hold `@State`. The `image` case delegates to `PhotoThumbnailView` (a separate struct) which holds its own sheet state. Thumbnail is 32×32pt; lightbox is a full-screen black sheet dismissed by tap. |
+| 62 | **Sort `schema` by `(row, col)` before grouping fields into row buckets** | The form editor stores fields in creation/drag order, NOT row-numeric order. Section fields have their own row numbers but may appear anywhere in the JSON array. Any code that groups fields by `row` and attaches section headers must first sort by `(f["row"], f["col"])` — exactly like the web's `sorted(key=lambda f: (f['row'], f['col']))`. Without this, sections attach to the wrong rows and appear displaced or missing. Sites that use absolute `(col, row)` pixel offsets (e.g. `GridFormView` ZStack, `canvasHeight()`) are unaffected — sort order only matters when grouping by row for sequential rendering. |
 
 ---
 

JanitorialQC/Models/LocalIssue.swift

@@ -24,16 +24,24 @@ final class LocalIssue {
     /// JSON-encoded array of server paths after upload, e.g. ["uploads/issue_photos/abc.jpg", ...]
     var photoServerPathsJSON: String = "[]"
 
+    // Shared coders — JSONDecoder/Encoder init is expensive (parses locale and
+    // calendar info). Allocating them inside computed property getters means
+    // a new instance per access; on a list showing 50 issues each with two
+    // JSON-backed arrays that's 200 allocations per render pass. Static
+    // instances are created once and reused for the lifetime of the app.
+    private static let jsonDecoder = JSONDecoder()
+    private static let jsonEncoder = JSONEncoder()
+
     /// Decoded local photo paths (up to 5)
     var photoLocalPaths: [String] {
-        get { (try? JSONDecoder().decode([String].self, from: Data(photoLocalPathsJSON.utf8))) ?? [] }
+        get { (try? Self.jsonDecoder.decode([String].self, from: Data(photoLocalPathsJSON.utf8))) ?? [] }
-        set { photoLocalPathsJSON = (try? String(data: JSONEncoder().encode(newValue), encoding: .utf8)) ?? "[]" }
+        set { photoLocalPathsJSON = (try? String(data: Self.jsonEncoder.encode(newValue), encoding: .utf8)) ?? "[]" }
     }
 
     /// Decoded server photo paths
     var photoServerPaths: [String] {
-        get { (try? JSONDecoder().decode([String].self, from: Data(photoServerPathsJSON.utf8))) ?? [] }
+        get { (try? Self.jsonDecoder.decode([String].self, from: Data(photoServerPathsJSON.utf8))) ?? [] }
-        set { photoServerPathsJSON = (try? String(data: JSONEncoder().encode(newValue), encoding: .utf8)) ?? "[]" }
+        set { photoServerPathsJSON = (try? String(data: Self.jsonEncoder.encode(newValue), encoding: .utf8)) ?? "[]" }
     }
 
     var createdAt: Date

JanitorialQC/Sync/SyncManager.swift

@@ -268,6 +268,16 @@ class SyncManager: ObservableObject {
             dup.uploadStatus = "uploaded"
         }
 
+        // Pre-fetch parent records ONCE before the loop.
+        // Without this, every successful photo upload fetched ALL LocalInspection
+        // and ALL LocalIssue records from SwiftData to find the parent —
+        // N photos → 2N full-table fetches. Pre-fetching here reduces that
+        // to 2 fetches regardless of how many photos are in the queue.
+        // Fetch-all + filter in Swift — #Predicate with a captured String variable
+        // causes "LocalInspection is ambiguous" under Xcode 26 (CLAUDE.md rule 3).
+        let allInspections = (try? context.fetch(FetchDescriptor<LocalInspection>())) ?? []
+        let allIssues      = (try? context.fetch(FetchDescriptor<LocalIssue>()))      ?? []
+
         for photo in pending {
             do {
                 let serverPath = try await APIClient.shared.uploadPhoto(
@@ -277,20 +287,18 @@ class SyncManager: ObservableObject {
                 photo.serverPath   = serverPath
                 photo.uploadStatus = "uploaded"
 
-                // Update parent inspection form field value
+                // Update parent inspection form field value.
+                // Pre-fetched before the loop — not repeated per photo.
                 if photo.entityType == "inspection", let fieldId = photo.fieldId {
                     let entityId = photo.entityLocalId
-                    // Fetch-all + filter in Swift — #Predicate with a captured String
+                    allInspections.first(where: { $0.localId == entityId })?
-                    // variable causes "LocalInspection is ambiguous" under Xcode 26
+                        .setValue(serverPath, forFieldId: fieldId)
-                    // SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor (CLAUDE.md rule 3).
-                    let allInspections = (try? context.fetch(FetchDescriptor<LocalInspection>())) ?? []
-                    allInspections.first(where: { $0.localId == entityId })?.setValue(serverPath, forFieldId: fieldId)
                 }
 
-                // Update parent issue photo paths array
+                // Update parent issue photo paths array.
+                // Pre-fetched before the loop — not repeated per photo.
                 if photo.entityType == "issue" {
                     let entityId = photo.entityLocalId
-                    let allIssues = (try? context.fetch(FetchDescriptor<LocalIssue>())) ?? []
                     if let issue = allIssues.first(where: { $0.localId == entityId }) {
                         var paths = issue.photoServerPaths
                         if !paths.contains(serverPath) { paths.append(serverPath) }
@@ -328,12 +336,12 @@ class SyncManager: ObservableObject {
                 inspection.status     = "synced"
 
                 // Clear follow-up flag on parent.
-                if let parentLocalId = inspection.parentLocalId {
+                // Reuses the `all` array already fetched at the top of this
-                    let allInspections = try? context.fetch(FetchDescriptor<LocalInspection>())
+                // function — avoids a redundant full-table fetch per inspection.
-                    if let parent = allInspections?.first(where: { $0.localId == parentLocalId }) {
+                if let parentLocalId = inspection.parentLocalId,
-                        parent.followUpRequired = false
+                   let parent = all.first(where: { $0.localId == parentLocalId }) {
-                        parent.followUpNote     = nil
+                    parent.followUpRequired = false
-                    }
+                    parent.followUpNote     = nil
                 }
 
                 try? context.save()
@@ -583,6 +591,10 @@ class SyncManager: ObservableObject {
     }
 
     func updatePendingCount(context: ModelContext) {
+        // Fetch-all + filter in Swift — #Predicate with string literals is
+        // banned under Xcode 26 SWIFT_DEFAULT_ACTOR_ISOLATION (CLAUDE.md rule 3).
+        // These fetches are lightweight (no relationships loaded) and run once
+        // per sync cycle at the very end, not in a hot loop.
         let inspCount = (try? context.fetch(FetchDescriptor<LocalInspection>()))?
             .filter { $0.syncStatus == "pending" }.count ?? 0
         let issueCount = (try? context.fetch(FetchDescriptor<LocalIssue>()))?