diff --git a/JanitorialQC/API/APIClient.swift b/JanitorialQC/API/APIClient.swift index 52cab9e..7aad1a9 100644 --- a/JanitorialQC/API/APIClient.swift +++ b/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() - fmt.dateFormat = "yyyy-MM-dd'T'HH:mm:ss" - ep += "?since=\(fmt.string(from: since))" + ep += "?since=\(Self.notifSinceFmt.string(from: since))" } let result: APINotificationsResponseData = try await request(ep) return result.notifications diff --git a/JanitorialQC/CLAUDE.md b/JanitorialQC/CLAUDE.md index 2bf319e..ac6b036 100644 --- a/JanitorialQC/CLAUDE.md +++ b/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. | --- diff --git a/JanitorialQC/Models/LocalIssue.swift b/JanitorialQC/Models/LocalIssue.swift index 46f87ea..249acbe 100644 --- a/JanitorialQC/Models/LocalIssue.swift +++ b/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))) ?? [] } - set { photoLocalPathsJSON = (try? String(data: JSONEncoder().encode(newValue), encoding: .utf8)) ?? "[]" } + get { (try? Self.jsonDecoder.decode([String].self, from: Data(photoLocalPathsJSON.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))) ?? [] } - set { photoServerPathsJSON = (try? String(data: JSONEncoder().encode(newValue), encoding: .utf8)) ?? "[]" } + get { (try? Self.jsonDecoder.decode([String].self, from: Data(photoServerPathsJSON.utf8))) ?? [] } + set { photoServerPathsJSON = (try? String(data: Self.jsonEncoder.encode(newValue), encoding: .utf8)) ?? "[]" } } var createdAt: Date diff --git a/JanitorialQC/Sync/SyncManager.swift b/JanitorialQC/Sync/SyncManager.swift index aaba008..2f831b8 100644 --- a/JanitorialQC/Sync/SyncManager.swift +++ b/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())) ?? [] + let allIssues = (try? context.fetch(FetchDescriptor())) ?? [] + 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 - // variable causes "LocalInspection is ambiguous" under Xcode 26 - // SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor (CLAUDE.md rule 3). - let allInspections = (try? context.fetch(FetchDescriptor())) ?? [] - allInspections.first(where: { $0.localId == entityId })?.setValue(serverPath, forFieldId: fieldId) + 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())) ?? [] 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 { - let allInspections = try? context.fetch(FetchDescriptor()) - if let parent = allInspections?.first(where: { $0.localId == parentLocalId }) { - parent.followUpRequired = false - parent.followUpNote = nil - } + // Reuses the `all` array already fetched at the top of this + // function — avoids a redundant full-table fetch per inspection. + if let parentLocalId = inspection.parentLocalId, + let parent = all.first(where: { $0.localId == parentLocalId }) { + 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()))? .filter { $0.syncStatus == "pending" }.count ?? 0 let issueCount = (try? context.fetch(FetchDescriptor()))?