Aug 25 - Fix inspection lost photos

JanitorialQC/API/APIClient.swift

@@ -350,6 +350,31 @@ actor APIClient {
         )
     }
 
+    // ── Attach a late photo to an ALREADY-SUBMITTED inspection ────────────
+    // The inspection twin of updateIssuePhotos above, and the piece that was
+    // missing: an inspection photo that exhausted its upload attempts was
+    // submitted with its field blanked, and nothing could ever put it back.
+    // attachServerPath() wrote the recovered path into LOCAL form data only,
+    // so the server copy stayed empty forever even after a successful retry.
+    //
+    // Server-side (app/api/inspections.py, update_inspection) form_data is
+    // merged field-by-field via _merge_form_data: a non-empty incoming value
+    // wins, and existing 'uploads/...' paths are never blanked. Sending only
+    // the recovered fields is therefore safe and idempotent.
+    //
+    // `status` is deliberately NOT sent: including it would re-run the
+    // draft→completed transition server-side, which is what fulfils a linked
+    // schedule. Omitting it leaves status, score and schedule untouched.
+    func updateInspectionFormData(inspectionId: Int,
+                                  fields: [String: String]) async throws {
+        struct R: Decodable, Sendable { let id: Int }
+        let _: R = try await request(
+            "/api/v1/inspections/\(inspectionId)",
+            method: "PATCH",
+            body: ["form_data": fields]
+        )
+    }
+
     // ── Upload a resolution photo (entity_type = issue_result) ────────────
     // Saves to issue_result_photos subfolder on the server — same bucket
     // as photos uploaded via the web update form.

JanitorialQC/CLAUDE.md

@@ -265,6 +265,51 @@ guard isOnline, let context = modelContext, AuthManager.shared.isAuthenticated e
 
 7. **`pollNotifications`** — fetches new notifications since `lastNotificationFetch` cursor.
 
+### Photo loss on inspections — the recovery loop (Aug 2026)
+
+An inspection can reach the server with its photo fields BLANK while the files
+sit safely on the device. The chain:
+
+1. `processPhotoQueue` upload fails → `uploadRetryCount++`; after
+   `maxPhotoUploadAttempts` (5) the row goes `uploadStatus = "failed"`, which is
+   **terminal**.
+2. `processInspectionQueue` treats `"failed"` as ready — deliberate, so a dead
+   photo cannot block a submission forever — and submits.
+3. `APIClient.submitInspection` rewrites any surviving `local://` value to `""`,
+   so the field lands **blank on the server**.
+4. The inspection is marked `synced`. Nothing revisits it.
+
+**Step 4 was a dead end until this fix.** "Retry Failed Items" resets the photo
+to `pending` and the re-upload can succeed, but `attachServerPath()` writes the
+recovered path into **local** form data only — and the inspection is already
+synced, so nothing carried it across. The photo was recoverable in principle and
+unreachable in practice, which is what the Photo Diagnostic screen reports as
+*"LOST ON SERVER … File present at stored path — recoverable"*.
+
+`pushLateInspectionPhotoIfNeeded()` closes it, mirroring
+`pushLateIssuePhotoIfNeeded()`:
+
+| | Issue | Inspection |
+|---|---|---|
+| late-attach call | `PATCH /api/v1/issues/<id>/photos` | `PATCH /api/v1/inspections/<id>` with `form_data` |
+| helper | `pushLateIssuePhotoIfNeeded` | `pushLateInspectionPhotoIfNeeded` |
+
+Server-side `_merge_form_data` makes this safe: a non-empty incoming value wins,
+and an existing `uploads/...` path is never blanked by an empty one — so the
+PATCH is idempotent and cannot erase a good path. **`status` is deliberately not
+sent**: including it would re-run the draft→completed transition, which is what
+fulfils a linked schedule.
+
+Normal path is unaffected — `processPhotoQueue` runs before
+`processInspectionQueue`, so a first-time inspection has no `serverId` yet and
+the helper no-ops; only a recovery reaches it.
+
+`PendingPhoto.lastUploadError` records **why** the last attempt failed. Nothing
+recorded it before: a photo could burn all five attempts with the reason visible
+nowhere — the device showed only "failed", and the server logged only
+*successful* uploads (now fixed: `app/api/photos.py` logs every rejection and any
+storage-write failure at WARNING/ERROR with the username).
+
 ### Server-pulled issue identification
 
 Records inserted by `pullAssignedIssues` are identified by: `syncStatus == "synced"` AND `inspectionLocalId == ""`. These are the only records safe to delete during reconciliation.

JanitorialQC/Models/PendingPhoto.swift

@@ -38,6 +38,16 @@ final class PendingPhoto {
     var uploadRetryCount: Int = 0
     var createdAt: Date
 
+    /// Why the last upload attempt failed, for diagnosis.
+    ///
+    /// Nothing recorded this before: a photo could burn all five attempts and
+    /// cost an inspection its evidence with no trace anywhere of the reason —
+    /// the device showed only "failed" and the server logs only SUCCESSFUL
+    /// uploads. Cleared on a successful upload.
+    ///
+    /// Optional so existing SwiftData stores migrate lightweight (rule 8).
+    var lastUploadError: String?
+
     // ── Capture metadata (sent to the server, burned into the photo) ───────
     // Recorded when the shutter fires, NOT when the upload runs — the app is
     // offline-first, so a photo taken at 09:14 may not sync until 16:00 and

JanitorialQC/Sync/SyncManager.swift

@@ -367,10 +367,14 @@ class SyncManager: ObservableObject {
                 photo.uploadRetryCount = 0
                 uploadedPaths[photo.localFilePath] = serverPath
 
+                photo.lastUploadError  = nil
                 attachServerPath(serverPath, for: photo,
                                  inspections: allInspections, issues: allIssues)
                 await pushLateIssuePhotoIfNeeded(serverPath, for: photo,
                                                  issues: allIssues, context: context)
+                await pushLateInspectionPhotoIfNeeded(serverPath, for: photo,
+                                                      inspections: allInspections,
+                                                      context: context)
                 try? context.save()
 
             } catch {
@@ -386,6 +390,9 @@ class SyncManager: ObservableObject {
                 // photo permanently, silently, with the inspection still
                 // reported as successfully synced.
                 photo.uploadRetryCount += 1
+                // Keep the reason. Without it a lost evidence photo is
+                // undiagnosable after the fact — see PendingPhoto.lastUploadError.
+                photo.lastUploadError = error.localizedDescription
                 if photo.uploadRetryCount >= Self.maxPhotoUploadAttempts {
                     photo.uploadStatus = "failed"
                     syncError = "A photo failed to upload after "
@@ -403,6 +410,7 @@ class SyncManager: ObservableObject {
             dup.serverPath       = serverPath
             dup.uploadStatus     = "uploaded"
             dup.uploadRetryCount = 0
+            dup.lastUploadError  = nil
             attachServerPath(serverPath, for: dup,
                              inspections: allInspections, issues: allIssues)
             await pushLateIssuePhotoIfNeeded(serverPath, for: dup,
@@ -455,6 +463,56 @@ class SyncManager: ObservableObject {
         try? context.save()
     }
 
+    /// Push a recovered photo onto an inspection that has ALREADY been submitted.
+    ///
+    /// The inspection counterpart of pushLateIssuePhotoIfNeeded, and the gap
+    /// that made inspection photo loss permanent:
+    ///
+    ///   1. the upload fails `maxPhotoUploadAttempts` times -> row goes "failed"
+    ///   2. "failed" counts as ready, so processInspectionQueue submits anyway
+    ///      and APIClient.submitInspection rewrites the surviving local:// value
+    ///      to "" — the field lands BLANK on the server
+    ///   3. the inspection is marked synced; nothing ever revisits it
+    ///
+    /// Step 3 was the dead end. "Retry Failed Items" could re-upload the file
+    /// successfully, but attachServerPath only wrote the path into LOCAL form
+    /// data, which no longer goes anywhere — the inspection was already synced.
+    /// The photo sat on the device, recoverable in principle and unreachable in
+    /// practice. This closes the loop by PATCHing the server copy.
+    ///
+    /// Normal path: processPhotoQueue runs BEFORE processInspectionQueue, so a
+    /// first-time inspection has no serverId yet and this does nothing — the
+    /// path travels in the submit body as usual. Only a recovery reaches here.
+    ///
+    /// Best-effort: a failure leaves the photo attached locally and retried on
+    /// the next pass, exactly like the issue version.
+    private func pushLateInspectionPhotoIfNeeded(
+        _ serverPath: String,
+        for photo: PendingPhoto,
+        inspections: [LocalInspection],
+        context: ModelContext
+    ) async {
+        guard photo.entityType == "inspection",
+              let fieldId = photo.fieldId
+        else { return }
+        let entityId = photo.entityLocalId
+        guard let inspection = inspections.first(where: { $0.localId == entityId }),
+              let inspectionServerId = inspection.serverId
+        else { return }
+
+        do {
+            try await APIClient.shared.updateInspectionFormData(
+                inspectionId: inspectionServerId,
+                fields: [fieldId: serverPath]
+            )
+            inspection.syncErrorMessage = nil
+        } catch {
+            inspection.syncErrorMessage =
+                "A recovered photo could not be attached: \(error.localizedDescription)"
+        }
+        try? context.save()
+    }
+
     /// Write a freshly uploaded server path onto whichever record owns the photo.
     /// Both collections are pre-fetched by the caller — see processPhotoQueue.
     private func attachServerPath(