Aug 27 - Fixed inspection lost photos recovery

JanitorialQC.xcodeproj/project.pbxproj

@@ -431,7 +431,7 @@
 					"$(inherited)",
 					"@executable_path/Frameworks",
 				);
-				MARKETING_VERSION = 1.9;
+				MARKETING_VERSION = 1.12;
 				PRODUCT_BUNDLE_IDENTIFIER = com.ltservicesinc.JanitorialQC;
 				PRODUCT_NAME = "$(TARGET_NAME)";
 				STRING_CATALOG_GENERATE_SYMBOLS = YES;
@@ -474,7 +474,7 @@
 					"$(inherited)",
 					"@executable_path/Frameworks",
 				);
-				MARKETING_VERSION = 1.9;
+				MARKETING_VERSION = 1.12;
 				PRODUCT_BUNDLE_IDENTIFIER = com.ltservicesinc.JanitorialQC;
 				PRODUCT_NAME = "$(TARGET_NAME)";
 				STRING_CATALOG_GENERATE_SYMBOLS = YES;

JanitorialQC/API/APIClient.swift

@@ -38,8 +38,35 @@ private struct _Envelope<T: Decodable & Sendable>: Decodable, Sendable {
     private enum CodingKeys: String, CodingKey { case ok, data, error }
 }
 
-// Free function removed — see refreshAccessToken() which decodes using a
-// local JSONDecoder to avoid Swift 6 actor-isolation errors.
+// Envelope header only — `ok` and `error`, never the payload.
+//
+// Split from _Envelope so `decode()` can tell "the server reported a failure"
+// apart from "the server succeeded but we could not read the payload". Those
+// were indistinguishable while `data` was decoded with `try?`: any schema
+// mismatch produced data == nil and surfaced as serverError("Unknown server
+// error"), pointing every investigation at the backend.
+private struct _EnvelopeMeta: Decodable, Sendable {
+    let ok: Bool
+    let error: String?
+
+    nonisolated init(from decoder: any Decoder) throws {
+        let c = try decoder.container(keyedBy: CodingKeys.self)
+        ok    = try  c.decode(Bool.self,   forKey: .ok)
+        error = try? c.decode(String.self, forKey: .error)
+    }
+    private enum CodingKeys: String, CodingKey { case ok, error }
+}
+
+// Payload only, decoded STRICTLY so the failure reason propagates.
+private struct _EnvelopePayload<T: Decodable & Sendable>: Decodable, Sendable {
+    let data: T
+
+    nonisolated init(from decoder: any Decoder) throws {
+        let c = try decoder.container(keyedBy: CodingKeys.self)
+        data  = try c.decode(T.self, forKey: .data)   // deliberately not `try?`
+    }
+    private enum CodingKeys: String, CodingKey { case data }
+}
 
 // Refresh-only envelope — Sendable so it can cross actor boundaries in Swift 6.
 // nonisolated init(from:) required on both types: without it the Swift 6 compiler
@@ -150,7 +177,11 @@ actor APIClient {
                      retrying: Bool = false) async throws -> String {
         let url = try buildURL("/api/v1/photos/upload")
 
-        guard let imageData = FileManager.default.contents(atPath: localPath) else {
+        // Read via PhotoStore, not the raw path. Stored paths embed the app
+        // container UUID, which iOS reassigns on every app update — the file
+        // survives, the path does not, and a raw read then fails forever on a
+        // photo that is sitting right there on disk (rule 91).
+        guard let imageData = PhotoStore.contents(at: localPath) else {
             throw APIError.networkError("Could not read photo: \(localPath)")
         }
 
@@ -279,20 +310,41 @@ actor APIClient {
         // inspection synced in the same triggerSync() pass.
         if let id = inspectionServerId { body["inspection_id"] = id }
         if let areaId = issue.areaServerId { body["area_id"] = areaId }
-        // photo_path = primary photo.  Additional photos are sent via a
-        // separate PATCH call in processIssueQueue after the issue is created,
-        // because the server create endpoint only stores a single photo_path.
+        // All evidence photos go in this ONE request.
+        //
+        // `photo_path` is the primary; `result_photos` carries the rest and is
+        // stored server-side in `mobile_photo_paths`, so they display under
+        // "Photo Evidence" rather than "Resolution Details"
+        // (app/api/issues.py, create_issue).
+        //
+        // These used to be split: create sent photo_path only, then
+        // processIssueQueue fired a follow-up PATCH for the extras. The extras
+        // were always known before create — processPhotoQueue fully populates
+        // photoServerPaths first — so the second call bought nothing and cost a
+        // window in which the issue was already `synced` while its photos were
+        // not attached. Sending them together makes attachment atomic with
+        // creation, and the endpoint's mobile_local_id idempotency covers a
+        // retry of the whole thing (rule 85).
         if let first = issue.photoServerPaths.first { body["photo_path"] = first }
+        let extraPhotos = Array(issue.photoServerPaths.dropFirst())
+        if !extraPhotos.isEmpty { body["result_photos"] = extraPhotos }
 
         struct R: Decodable, Sendable { let issueId: Int; let duplicate: Bool }
         let r: R = try await post("/api/v1/issues", body: body)
         return r.issueId
     }
 
-    // ── Attach additional photos to an existing issue ─────────────────────
-    // Called after submitIssue when the issue has more than one photo.
-    // PATCHes /api/v1/issues/{id}/photos with result_photos = [server paths beyond the first].
-    // The create endpoint only stores photo_path (single); extras go here.
+    // ── Attach a LATE evidence photo to an already-created issue ──────────
+    //
+    // NOT part of the normal path: submitIssue() sends every evidence photo in
+    // the create request, and reintroducing a routine post-create call is
+    // exactly what rule 85 forbids. This exists only for recovery — a photo
+    // that exhausted its upload attempts, was submitted without, and later
+    // succeeded via Pending Sync → Retry Failed Items. By then the create
+    // request is long gone and this is the only way across.
+    //
+    // Server-side (app/api/issues.py, update_issue_photos) this merges
+    // idempotently into mobile_photo_paths, so repeating a path is a no-op.
     func updateIssuePhotos(issueId: Int, resultPhotos: [String]) async throws {
         struct R: Decodable, Sendable { let issueId: Int; let resultPhotosCount: Int }
         let _: R = try await request(
@@ -302,6 +354,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.
@@ -312,7 +389,11 @@ actor APIClient {
                            retrying: Bool = false) async throws -> String {
         let url = try buildURL("/api/v1/photos/upload")
 
-        guard let imageData = FileManager.default.contents(atPath: localPath) else {
+        // Read via PhotoStore, not the raw path. Stored paths embed the app
+        // container UUID, which iOS reassigns on every app update — the file
+        // survives, the path does not, and a raw read then fails forever on a
+        // photo that is sitting right there on disk (rule 91).
+        guard let imageData = PhotoStore.contents(at: localPath) else {
             throw APIError.networkError("Could not read photo: \(localPath)")
         }
 
@@ -592,7 +673,32 @@ actor APIClient {
 
     // ── Token Refresh ─────────────────────────────────────────────────────
 
+    /// The refresh currently in flight, if any.
+    ///
+    /// `APIClient` being an actor is NOT enough on its own: `refreshAccessToken`
+    /// suspends at `await`, which releases the actor and lets a second caller
+    /// enter. Two requests 401-ing at once would then each POST /auth/refresh
+    /// with the SAME refresh token — the server rotates it on the first, so the
+    /// second presents an already-spent token, fails, and the user is signed
+    /// out mid-sync. Easy to hit: pollNotifications and registerDevice both run
+    /// alongside triggerSync.
+    ///
+    /// Coalescing here means concurrent callers await one shared result. The
+    /// check-and-store below spans no `await`, so it is atomic within the actor.
+    private var refreshTask: Task<Bool, Never>?
+
     private func refreshAccessToken() async -> Bool {
+        if let inFlight = refreshTask {
+            return await inFlight.value
+        }
+        let task = Task { await self.performTokenRefresh() }
+        refreshTask = task
+        let result = await task.value
+        refreshTask = nil
+        return result
+    }
+
+    private func performTokenRefresh() async -> Bool {
         guard let token = KeychainHelper.get(Constants.Keychain.refreshToken),
               let url   = URL(string: ServerConfig.current + "/api/v1/auth/refresh")
         else { return false }
@@ -657,14 +763,50 @@ actor APIClient {
     }
 
     private func decode<T: Decodable & Sendable>(_ data: Data) throws -> T {
-        if let env = try? decoder.decode(_Envelope<T>.self, from: data) {
-            if env.ok, let result = env.data { return result }
-            throw APIError.serverError(env.error ?? "Unknown server error")
+        // Read the envelope header first, so a server-reported failure and an
+        // unreadable payload cannot be confused for one another.
+        if let meta = try? decoder.decode(_EnvelopeMeta.self, from: data) {
+            guard meta.ok else {
+                throw APIError.serverError(meta.error ?? "Unknown server error")
+            }
+            do {
+                return try decoder.decode(_EnvelopePayload<T>.self, from: data).data
+            } catch {
+                // ok == true, so this is OUR problem, not the server's — a
+                // contract drift between this build and the deployment.
+                throw APIError.decodingError(Self.describe(error, as: T.self))
+            }
         }
+        // Not an envelope — a few endpoints return the object bare.
         do {
             return try decoder.decode(T.self, from: data)
         } catch {
-            throw APIError.decodingError(error.localizedDescription)
+            throw APIError.decodingError(Self.describe(error, as: T.self))
+        }
+    }
+
+    /// Turn a `DecodingError` into something that names the offending field.
+    ///
+    /// `error.localizedDescription` on a DecodingError is always the useless
+    /// "The data couldn't be read because it isn't in the correct format",
+    /// which is what the old path surfaced — so a renamed or retyped API field
+    /// gave no clue which one it was.
+    private static func describe<T>(_ error: Error, as type: T.Type) -> String {
+        func path(_ context: DecodingError.Context) -> String {
+            let keys = context.codingPath.map(\.stringValue).filter { !$0.isEmpty }
+            return keys.isEmpty ? "\(type)" : "\(type).\(keys.joined(separator: "."))"
+        }
+        switch error as? DecodingError {
+        case .keyNotFound(let key, let ctx):
+            return "missing field '\(key.stringValue)' in \(path(ctx))"
+        case .typeMismatch(let expected, let ctx):
+            return "\(path(ctx)) has the wrong type (expected \(expected))"
+        case .valueNotFound(let expected, let ctx):
+            return "\(path(ctx)) was null (expected \(expected))"
+        case .dataCorrupted(let ctx):
+            return "\(path(ctx)) is malformed: \(ctx.debugDescription)"
+        default:
+            return "could not read \(type): \(error.localizedDescription)"
         }
     }
 }

JanitorialQC/API/APIModels.swift

@@ -302,6 +302,12 @@ struct APIInspectionSummary: Decodable, Identifiable, Hashable, Sendable {
     let followUpRequired: Bool
     let followUpNote: String?
     let parentInspectionId: Int?
+    /// phase53 — who was asked to perform the follow-up. nil means it belongs
+    /// to the inspection's own inspector, which is what it always meant.
+    /// The list endpoint already returns only follow-ups this user OWNS, so
+    /// these are for display, not filtering.
+    let followUpAssignedTo: Int?
+    let followUpAssignedToName: String?
 
     /// Form field values as [fieldId: stringValue] for the grid renderer.
     var formValues: [String: String] {
@@ -368,6 +374,8 @@ struct APIInspectionSummary: Decodable, Identifiable, Hashable, Sendable {
         followUpRequired = (try? c.decode(Bool.self,   forKey: .followUpRequired)) ?? false
         followUpNote     = try? c.decode(String.self,  forKey: .followUpNote)
         parentInspectionId = try? c.decode(Int.self,   forKey: .parentInspectionId)
+        followUpAssignedTo     = try? c.decode(Int.self,    forKey: .followUpAssignedTo)
+        followUpAssignedToName = try? c.decode(String.self, forKey: .followUpAssignedToName)
     }
     private enum CodingKeys: String, CodingKey {
         case id, templateId, templateName, facilityId, facilityName
@@ -376,6 +384,7 @@ struct APIInspectionSummary: Decodable, Identifiable, Hashable, Sendable {
         case formData, formSchema
         case formMedia
         case followUpRequired, followUpNote, parentInspectionId
+        case followUpAssignedTo, followUpAssignedToName
     }
 
     // Explicit Hashable — formDataRaw/formSchemaRaw contain JSONValue which

JanitorialQC/Auth/AuthManager.swift

@@ -34,6 +34,7 @@ class AuthManager: ObservableObject {
         do {
             let response: MeResponseData = try await APIClient.shared.request("/api/v1/auth/me")
             applyUser(response.user)
+            reconcileSessionScope(userId: response.user.id)
             isAuthenticated = true
         } catch APIError.notAuthenticated {
             KeychainHelper.clearAll()
@@ -57,6 +58,9 @@ class AuthManager: ObservableObject {
             KeychainHelper.set(response.accessToken,  forKey: Constants.Keychain.accessToken)
             KeychainHelper.set(response.refreshToken, forKey: Constants.Keychain.refreshToken)
             applyUser(response.user)
+            // BEFORE isAuthenticated flips, so DashboardView is never rendered
+            // holding the previous inspector's data.
+            reconcileSessionScope(userId: response.user.id)
             isAuthenticated = true
             // Register device immediately after login — the .task {} and
             // .onChange(scenePhase) paths both miss this case because they run
@@ -97,6 +101,39 @@ class AuthManager: ObservableObject {
         KeychainHelper.set(user.displayName, forKey: Constants.Keychain.displayName)
     }
 
+    /// Purge local data when this session is scoped to a different
+    /// `(server, user)` pair than the database currently holds.
+    ///
+    /// This is the check that was missing: logout deliberately keeps the cache
+    /// so the same inspector can work offline after signing back in, but
+    /// nothing verified that the next sign-in *was* the same inspector. A
+    /// different one inherited their issues; see `SessionScope` and rule 88.
+    ///
+    /// Runs on both `login()` and `restoreSession()` — a session can also be
+    /// restored into a changed scope after a server switch.
+    private func reconcileSessionScope(userId: Int) {
+        let server = ServerConfig.current
+        let stored = SessionScope.stored
+
+        // No marker: either a genuinely fresh install, or an upgrade from a
+        // build that predates this. Those are indistinguishable, and purging on
+        // the guess would delete an in-progress draft belonging to the user
+        // signing in right now — so adopt the existing data and start tracking
+        // from here. Every subsequent identity change is then covered.
+        guard let stored else {
+            SessionScope.record(userId: userId)
+            return
+        }
+
+        guard stored.server != server || stored.userId != userId else { return }
+
+        SyncManager.shared.purgeSessionScopedData(
+            keepingUserId: userId,
+            sameServer:    stored.server == server
+        )
+        SessionScope.record(userId: userId)
+    }
+
     private func restoreUserFromKeychain() {
         currentUserId      = Int(KeychainHelper.get(Constants.Keychain.userId) ?? "0") ?? 0
         currentUserRole    = KeychainHelper.get(Constants.Keychain.userRole)    ?? ""

JanitorialQC/CLAUDE.md

@@ -89,7 +89,7 @@ JanitorialQC/
 │
 ├── API/
 │   ├── APIClient.swift          # actor — URLSession, JWT inject, 401-retry, photo upload
-│   │                            # updateIssuePhotos() — PATCH /issues/<id>/photos
+│   │                            # submitIssue() sends photo_path + result_photos (rule 85)
 │   └── APIModels.swift          # All Codable/Sendable response DTOs
 │                                # APIAssignedIssue has photoPath + mobilePhotoPaths + resultPhotos
 │
@@ -181,6 +181,7 @@ Stored keys (all prefixed `com.jqc.`): `accessToken`, `refreshToken`, `userId`,
 ```swift
 LocalFacility.self, LocalArea.self, LocalTemplate.self,
 LocalInspection.self, LocalIssue.self, LocalScheduledInspection.self,
+LocalFollowUpRequest.self, LocalNotification.self,
 PendingPhoto.self, SyncQueueEntry.self
 ```
 
@@ -197,7 +198,8 @@ PendingPhoto.self, SyncQueueEntry.self
 | `LocalIssue` | Issue record | `localId` (UUID, unique), `serverId`, `inspectionLocalId` (`""` for standalone/server-pulled), `facilityServerId`, `severity`, `syncStatus`, `photoLocalPathsJSON`, `photoServerPathsJSON`, **handler fields** (`handlerType`, `handlerLabel`, `facilityHandler*`, `vendor*` — all optional, synced from server, inspector-editable) |
 | `LocalScheduledInspection` | Read-only cached scheduled/recurring assignment (phase36) | `serverId` (`@Attribute(.unique)`, **no default** — rule 63), `facilityServerId`, `facilityName`, `templateServerId`, `templateName`, `inspectorId`, `frequency`, `frequencyLabel`, `dueDateString` (sort key), `isOverdue`, `nextDue` (computed), `parentInspectionServerId` (`Int?`, phase45 — set when the schedule is a planned follow-up; becomes the run's `parentServerId`). Pulled by `pullScheduledInspections()`; `init(from:)`/`update(from:)` like `LocalFacility` |
 | `LocalFollowUpRequest` | Read-only cached follow-up request raised on the web (July 2026) | `serverId` (`@Attribute(.unique)`, **no default** — rule 63; this is the *flagged parent* inspection's id and the `parentServerId` the re-inspection links to), `facilityServerId`, `facilityName`, `templateServerId`, `templateName`, `overallScore`, `inspectionDateString` (sort key), `followUpNote`, `note` (computed, trimmed/nil-ed), `inspectedOn` (computed, parses the `yyyy-MM-dd` prefix only — see the file comment), `fulfilledLocally` (`= false`, rule 71), `parentFormDataJSON` (`= "{}"`, the parent's answers cached for re-inspection prefill — rule 79), `parentFormData` (computed). Pulled by `pullFollowUpRequests()` |
-| `PendingPhoto` | Photo awaiting upload | `localId`, `localFilePath`, `serverPath`, `uploadStatus`, `entityType` (`"issue"` or `"inspection"`), `fieldId` |
+| `LocalNotification` | In-app notification inbox (Aug 2026) | `serverId` (`@Attribute(.unique)`, **no default** — rule 63), `title`, `body`, `eventType` (nil pre-phase17), `issueId`, `createdAt`, `receivedAt`, `isRead` (`= false`), `readAt`, `readSyncPending` (`= false`). Upserted by `pollNotifications()`; the local store IS the inbox, because the API only returns unread — rule 92 |
+| `PendingPhoto` | Photo awaiting upload | `localId`, `localFilePath`, `serverPath`, `uploadStatus`, `uploadRetryCount` (`Int = 0`, rule 83 — the row stays `"pending"` until it hits 5), `entityType` (`"issue"` or `"inspection"`), `fieldId` |
 | `SyncQueueEntry` | Outbox entry (informational) | `entityType`, `localId`, `syncStatus`, `payloadJSON` |
 
 ### LocalInspection Status Flow
@@ -247,27 +249,125 @@ guard isOnline, let context = modelContext, AuthManager.shared.isAuthenticated e
 
 2. **`processInspectionQueue`** — submits completed inspections when all `pendingPhotos` are settled.
 
-3. **`processIssueQueue`** — guards against submitting when parent inspection `syncStatus == "failed"`. After successful submit: sets `syncStatus = "synced"`, **clears `photoLocalPaths = []`** (prevents duplicate photo sections in `IssueDetailView`), then calls `updateIssuePhotos(issueId:resultPhotos:)` for any extra photos beyond the first (`Array(photoServerPaths.dropFirst())`).
+3. **`processIssueQueue`** — guards against submitting when parent inspection `syncStatus == "failed"`, **and waits for the issue's own photos to settle** (same rule as inspections — see rule 83). `submitIssue()` sends every evidence photo in the one create request (rule 85). After successful submit: sets `syncStatus = "synced"` and clears `photoLocalPaths = []` **only when every photo uploaded** (rule 84).
 
 4. **`pullReferenceData`** — fetches facilities, areas, templates. **Deduplicates facility response by `id` using `seenFacilityIds = Set<Int>()`** before upserting — prevents duplicate buildings in pickers when server returns same facility ID multiple times.
 
+   **Prunes facilities the server no longer returns (Aug 2026).** `/api/v1/facilities` is already scoped server-side, but cached rows were never removed, so a facility survived locally after the inspector's contract was unassigned, after it was deactivated, or after a **different user signed in on the same iPad**. Every picker derives its **contract** list from these rows (`StartInspectionView.contracts`, `IssuesView.contracts` both map over `LocalFacility`), so one stale facility kept a whole contract in the Start Inspection picker forever — which is how this surfaced. Templates already had this prune; facilities were the gap.
+
+   Two rules in the prune:
+   - Deletion only runs after **both** requests succeeded, so a failed sync can never empty the cache (it throws first).
+   - A facility still referenced by **unsynced** local work (a `LocalInspection` or `LocalIssue` with `syncStatus != "synced"`) is **kept but marked `isActive = false`** instead of deleted. `ExecuteInspectionView`/`MyInspectionsView` resolve the facility name by `serverId` and the issue *detail* view has no `facilityNameCache` fallback, so deleting it would turn an in-progress draft into "Unknown Facility". The row is pruned on a later sync once that work has been submitted, and `update(from:)` flips `isActive` back to true if the facility returns to scope.
+
+   **Every picker must therefore filter on `isActive`** — both views expose `availableFacilities` for this and derive `contracts` / `filteredFacilities` from it, never from the raw `@Query`. A retained out-of-scope row is for *display only*; offering it would let an inspector start work the server then rejects.
+
 5. **`pullAssignedIssues`** — fetches `GET /api/v1/issues`. Merges `api.photoPath` + `api.mobilePhotoPaths` into `photoServerPaths`. **Does NOT include `api.resultPhotos`** — resolution photos are web-only. Deletion pass runs always (not short-circuited on empty response).
 
 6. **`pullFollowUpRequests`** — fetches `GET /api/v1/inspections?follow_up_required=true`. Upserts `LocalFollowUpRequest` by `serverId`, deletes rows the server no longer returns, and mirrors `followUpRequired`/`followUpNote` onto the matching `LocalInspection` so the history badge agrees with the card. Best-effort — never blocks the pipeline. Runs after `processInspectionQueue`, so the section clears on the same sync that submits the re-inspection.
 
 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.
 
-### clearServerPulledData() — on logout / server switch
+### purgeSessionScopedData() — on identity change
 
-Deletes every `LocalIssue` where `serverId != nil`. This covers:
-- Server-pulled assigned issues (`inspectionLocalId == ""`, `syncStatus == "synced"`)
-- Inspector-created issues that already synced (`inspectionLocalId != ""`, `serverId != nil`)
+Replaces `clearServerPulledData()`, which deleted `LocalIssue` only and was called
+from the wrong place. See rule 88.
 
-Preserves only truly pending device-created issues (`serverId == nil`, `syncStatus == "pending"`).
+**Trigger is a change of `SessionScope` — the `(server, userId)` pair the database is
+scoped to — not logout.** `AuthManager.reconcileSessionScope()` compares the incoming
+session against the recorded scope on every `login()` and `restoreSession()`, and purges
+only when they differ.
+
+| Model | Kept |
+|---|---|
+| `LocalFacility` / `LocalArea` / `LocalTemplate` / `LocalScheduledInspection` / `LocalFollowUpRequest` | Nothing — pure caches, re-pulled on the next sync |
+| `LocalIssue` | Nothing — the model has no author field, so an unsent issue cannot be attributed and must not be submitted under a different inspector's name |
+| `LocalInspection` | Only rows whose `inspectorUserId` matches the incoming user, **and** only when the server is unchanged |
+| `PendingPhoto` | Only rows belonging to a kept inspection; the JPEGs of the rest are deleted from disk too |
+
+A plain logout still purges nothing: the same inspector signing back into the same server
+keeps their cache and stays usable offline. That was always the right call — the defect was
+that nothing checked whether the next sign-in was the same person.
+
+**`SessionScope.stored == nil` adopts the existing data rather than purging.** A fresh
+install and an upgrade from a build without the marker are indistinguishable, and guessing
+"purge" would delete an in-progress draft belonging to the person signing in right then.
+Every identity change after that first login is covered.
+
+### Notification inbox (Aug 2026)
+
+Notifications are persisted as `LocalNotification` and the inbox reads that store,
+not the poll response. **Why:** `GET /api/v1/notifications` is a poller, not an inbox —
+it filters to `is_read = False` and never sends the flag, so the list was all-unread by
+construction (every row looked identical, which is the defect this fixed) and a
+notification became invisible the moment it was read.
+
+- `pollNotifications()` **upserts** by `serverId` and never deletes. A row missing from a
+  response means nothing: it may have been read on the web, or just predate the cursor.
+- A local banner fires **only for newly-inserted rows**. Previously every polled item was
+  delivered, so a cold launch (cursor nil → server returns the whole unread backlog)
+  re-banner'd all of it on every app start.
+- `unreadNotificationCount` is **derived** by `refreshUnreadNotificationCount()`, not
+  tallied as items arrive — read state changes from both ends now.
+- `markNotificationsViewed()` is **gone**. It zeroed the badge because the screen had been
+  opened, which cannot coexist with real read state (badge 0, every row still unread).
+- Reading is explicit: tap a row, swipe, or **Mark All Read**. Local first
+  (`LocalNotification.markRead()` sets `readSyncPending`), then pushed by
+  `pushNotificationReadState()` — so it works offline and drains on reconnect.
+- `pruneReadNotifications()` drops **read** rows older than 30 days. Unread rows are never
+  pruned at any age; nothing else deletes a row, so without this the store grows forever.
+
+`NotificationDetailView` shows the full text and links to the referenced issue when that
+issue is cached locally, and says so plainly when it is not — the issue may belong to
+another inspector or simply not be pulled yet.
 
 ### Notification polling
 
@@ -287,6 +387,25 @@ let x   = all.filter { ... }
 
 ---
 
+### Role gates — `Constants.Roles` (Aug 2026)
+
+**Never write `role == "inspector"` in a view.** `external_inspector` ("Customer Inspector" — an inspector employed by the customer) has the same powers as our own `inspector` and the API scopes it identically, so a literal equality check locks that account out of actions the server would happily accept. It fails **silently**: no error, no 403 to debug — the control simply is not drawn.
+
+That is exactly what happened to Update Status, Handled By and Start Follow-up, which were three separate hand-written lists in two files:
+
+| Site | Was | Now |
+|---|---|---|
+| `IssuesView.canUpdateStatus` | `admin \| director \| inspector` | `Constants.Roles.issueActors` |
+| `IssuesView.canEditHandler` | `admin \| director \| inspector \| project_manager` | same |
+| `InspectionHistoryView.canStartFollowUp` | `admin \| director \| inspector \| project_manager` | same |
+
+`Constants.Roles` in `Utils/Constants.swift` is the single definition, mirroring `User.INSPECTOR_ROLES` / `User.is_inspector` on the server (server rule 87):
+
+- `inspectorRoles` = `{inspector, external_inspector}` — test membership, never `==`.
+- `issueActors` = `{admin, director, project_manager} ∪ inspectorRoles` — a **subset** of the API's `_ALLOWED_ROLES` for these endpoints, so every role it admits is one the server accepts. `auditor` is deliberately excluded (read-only in the app).
+
+The server stays the authority and additionally enforces facility scope; these gates only decide whether to draw the control.
+
 ## 9. API Client (APIClient)
 
 `actor APIClient` — singleton via `APIClient.shared`. All methods are `async throws`.
@@ -297,16 +416,20 @@ All server URLs built as: `ServerConfig.current + endpoint` — **`Constants.bas
 
 `decoder.keyDecodingStrategy = .convertFromSnakeCase` — snake_case server fields map to camelCase automatically. Server POST body keys are snake_case (`photo_path`, `result_photos`, `facility_id`, etc.).
 
+**`decode()` reads the envelope header before the payload.** `_EnvelopeMeta` (`ok` + `error`) is decoded first; only if `ok` is true is the payload decoded **strictly** via `_EnvelopePayload<T>`. This separates "the server reported a failure" from "the server succeeded and we could not read it" — previously `data` was decoded with `try?`, so *any* schema drift produced `data == nil` and surfaced as `serverError("Unknown server error")`, sending every investigation to the backend for what was a client-side contract mismatch. Failures now report the offending field (`missing field 'x' in APIFoo.bar`) via `describe(_:as:)`, because `DecodingError.localizedDescription` is always the useless "data couldn't be read" string.
+
+**Token refresh is coalesced (`refreshTask`).** Being an `actor` is not sufficient: `refreshAccessToken()` suspends at `await`, releasing the actor, so two requests 401-ing at once each POSTed `/auth/refresh` with the *same* refresh token. The server rotates on the first, so the second presented a spent token, failed, and signed the user out mid-sync — reachable because `pollNotifications` and `registerDevice` run alongside `triggerSync`. Concurrent callers now await one shared `Task`.
+
 ### Key methods
 
 | Method | Endpoint | Notes |
 |---|---|---|
-| `request<T>` | Any | Generic; 401 auto-refresh once |
+| `request<T>` | Any | Generic; 401 auto-refresh once (refresh is coalesced — see above) |
 | `post<T>` | Any | POST convenience |
 | `uploadPhoto` | `POST /api/v1/photos/upload` | Multipart form-data; `entity_type="issue"` → `uploads/issue_photos/` |
 | `submitInspection` | `POST /api/v1/inspections` | Sanitises `local://` paths |
 | `submitIssue` | `POST /api/v1/issues` | Sends `photo_path` = first server photo only |
-| `updateIssuePhotos` | `PATCH /api/v1/issues/<id>/photos` | Sends `{ "result_photos": [extra paths] }`; stored server-side in `mobile_photo_paths` |
+| `updateIssuePhotos` | `PATCH /api/v1/issues/<id>/photos` | **Recovery only** — called from `pushLateIssuePhotoIfNeeded()` for a photo that succeeded *after* its issue was already created. The normal path sends every evidence photo inside `submitIssue`'s create request (rule 85); do not call this from it. Merges idempotently into `mobile_photo_paths`. |
 | `fetchAssignedIssues` | `GET /api/v1/issues` | Returns issues assigned to OR reported by current user |
 | `fetchIssueDetail` | `GET /api/v1/issues/<id>` | Fetches current status |
 | `updateIssueStatus` | `PATCH /api/v1/issues/<id>/status` | Inspector updates status |
@@ -408,6 +531,8 @@ Flagging that row at submit time exposed a second problem: `ScheduledInspections
 
 All start flows are full-screen for consistency (rule 66): draft-resume (dashboard) → `.fullScreenCover` → `ExecuteInspectionView(isModallyPresented: true)` with a leading `Close`; scheduled / new (`+`) / re-inspection → `.fullScreenCover` → `StartInspectionView` (its own Cancel). Pushed presentations (My Inspections row → `ExecuteInspectionView`) keep `isModallyPresented = false` and rely on the nav back button.
 
+**Leaving after submit — `ExecuteInspectionView.onFinished`.** `StartInspectionView` *pushes* the form onto the NavigationStack inside its own cover, so `dismiss()` there only pops: the inspector finished an inspection and landed back on the "New Inspection" form that started it, with Cancel as the only way out. `StartInspectionView` passes its own dismiss as `onFinished` so the whole cover closes. Left nil everywhere popping is correct — the My Inspections row (pushed onto the list's stack) and the dashboard Resume banner (this view *is* the cover root).
+
 ### "Handled By" (issue handler, phase35 → mobile July 2026)
 
 `IssueDetailView` (`IssuesView.swift`) shows a "Handled By" section: current handler label + detail, and — for admin/director/PM **and the assigned inspector** — an inline editor (segmented internal/facility/vendor + name/contact/notes) that PATCHes via `updateIssueHandler` and mirrors the result onto `LocalIssue`. The inspector-writable path is a deliberate divergence from the web form (web CLAUDE.md rule 78).
@@ -512,24 +637,38 @@ The **Follow-up Requested** card / section (July 2026) is the second trigger, an
 ### PendingPhoto lifecycle
 
 ```
-Created (uploadStatus="pending")
+Created (uploadStatus="pending", uploadRetryCount=0)
     ↓ SyncManager.processPhotoQueue()
-Uploaded (uploadStatus="uploaded", serverPath set)
-    ↓ Parent record updated
-    Inspection image fields: LocalInspection.formData[fieldId] = serverPath
-    Issues: LocalIssue.photoServerPaths.append(serverPath)
+    ├─ success → uploadStatus="uploaded", serverPath set
+    │            ↓ Parent record updated (attachServerPath)
+    │            Inspection image fields: LocalInspection.formData[fieldId] = serverPath
+    │            Issues: LocalIssue.photoServerPaths.append(serverPath)
+    └─ error   → uploadRetryCount += 1, STAYS "pending" (retried next sync)
+                 └─ only at maxPhotoUploadAttempts (5) → uploadStatus="failed"
 ```
 
+**`"failed"` is terminal and means "every attempt was used", not "one error happened"** — see rule 83. `SyncStatusView` → Retry Failed Items resets these back to `"pending"`; it is the only thing that does.
+
 ### Multi-photo issue submission sequence
 
 ```
 1. processPhotoQueue:  uploads all N photos → appends each serverPath to issue.photoServerPaths
-2. processIssueQueue:  submitIssue(issue)   → sends photo_path = photoServerPaths[0]
-                       issue.photoLocalPaths = []   (clear local paths — prevents duplicate sections)
-                       updateIssuePhotos(issueId, photoServerPaths.dropFirst())
-                                              → PATCH /issues/<id>/photos with extras
+                       (a shared local file is uploaded ONCE; every PendingPhoto row
+                        pointing at it gets the same serverPath — rule 86)
+2. processIssueQueue:  waits until all N photos are "uploaded" or "failed"   ← rule 83
+                       submitIssue(issue)   → photo_path    = photoServerPaths[0]
+                                              result_photos = the rest        ← rule 85
+                                              (server stores these in mobile_photo_paths)
+                       issue.photoLocalPaths = []   ONLY if all N uploaded   ← rule 84
 ```
 
+**One request, not two.** There is no post-create PATCH on the normal path — see rule 85.
+Two related calls are NOT exceptions to that:
+- `pushLateIssuePhotoIfNeeded()` fires only when `issue.serverId` is already set, i.e. a
+  photo recovered after the issue was created (rule 83's residual case).
+- `PATCH /issues/<id>/result_photos` is `IssueDetailView` attaching *resolution* photos,
+  which genuinely are added after the fact.
+
 ### Photo display in IssueDetailView
 
 Gated on `syncStatus`:
@@ -573,13 +712,15 @@ While a photo is pending upload, the inspection form field value is `"local://<p
 
 | Type | Rule |
 |---|---|
-| `rating` | `0` = unanswered → excluded. Each answered rating: `value / 5` of 1.0 |
+| `rating` | `0` = unanswered → excluded. Each answered rating: `value / 5` of 1.0. **The denominator is a flat 5, never the field's `max`** — `_compute_score_from_form()` hardcodes it, so anything reading `max` disagrees with the score the server stores |
 | `checkbox` | `"true"` = pass |
 | `radio` | Pass: `pass`, `yes`, `ok`, `good`, `acceptable`, `compliant` (case-insensitive) |
 | `pass_fail` | Same keywords. Empty = unanswered → excluded |
 
 Returns `nil` if no scoreable fields or all unanswered.
 
+**Two implementations must agree.** `ExecuteInspectionView.liveScore` recomputes the same thing from in-memory `formValues` to drive the toolbar badge as the inspector fills the form. It read `field["max"] ?? 5` for the rating denominator while `computeScore` hardcoded 5, so any template with `max != 5` showed one percentage in the toolbar and submitted another. Change both together, and check `_compute_score_from_form()` in `app/routes/inspections.py` — it is the authority.
+
 ---
 
 ## 19. Background Sync
@@ -597,7 +738,7 @@ Requirements: `requiresNetworkConnectivity = true`, `requiresExternalPower = fal
 - **Sync Now** — triggers `triggerSync()`; disabled when offline or syncing.
 - **Clear Reference Cache** — deletes `LocalFacility`, `LocalArea`, `LocalTemplate` only. Never touches `LocalInspection`, `LocalIssue`, `PendingPhoto`. Triggers `pullReferenceData()` if online.
 - **Server picker** — see §21.
-- **Log Out** — calls `clearServerPulledData()` + `resetNotificationPoller()` + `auth.logout()`.
+- **Log Out** — calls `resetNotificationPoller()` + `auth.logout()`. Purges **nothing**: the cache stays so the same inspector can work offline after signing back in. A *different* inspector signing in is handled at login by `reconcileSessionScope()` (rule 88).
 - App version + current server URL (from `ServerConfig.current`).
 
 ---
@@ -632,14 +773,19 @@ Segmented picker above the credential fields. `onChange` calls `ServerConfig.sel
 Segmented picker in a "Server" section. `onChange` snaps the picker back to the current saved server, stores intent in `pendingServer`, and shows `Alert("Switch Server?")`.
 
 **Alert actions:**
-- **Switch & Log Out (destructive):** `ServerConfig.select(chosen)` → `clearServerPulledData()` → `resetNotificationPoller()` → `auth.logout()`.
+- **Switch & Log Out (destructive):** `ServerConfig.select(chosen)` → `purgeSessionScopedData(keepingUserId: nil, sameServer: false)` → `SessionScope.clear()` → `resetNotificationPoller()` → `auth.logout()`. Erases **everything**, unsynced work included — the alert says so. Previously this cleared `LocalIssue` alone and left `LocalInspection` rows holding the other server's facility/template ids.
 - **Cancel:** clears `pendingServer`, picker stays on original.
 
 **Why logout is required on server switch:** `serverId` values are server-specific. A `LocalIssue` with `serverId = 48` from `jqc.ltservicesinc.com` has no meaning on `jqc1.ltservicesinc.com`. Keeping stale records causes "Issue not found" errors on every status fetch/update.
 
-### clearServerPulledData() boundary
+### Scope boundary
 
-Deletes `LocalIssue` where `serverId != nil`. Preserves `serverId == nil` records (pending, never synced). This is the correct boundary — not `syncStatus == "synced" && inspectionLocalId == ""` (the old incorrect filter that missed inspector-created synced issues).
+There is no partial boundary any more. `serverId` is not the only server-specific value —
+`facilityServerId`, `templateServerId`, `areaServerId` and `parentServerId` all name rows in
+one particular database, and inspector facility scope differs per user on top of that. So a
+scope change purges wholesale rather than filtering (rule 88); the only thing carried across
+is the incoming user's own unsent `LocalInspection` rows, and only when the server is
+unchanged. See §8, `purgeSessionScopedData()`.
 
 ---
 
@@ -688,7 +834,7 @@ Deletes `LocalIssue` where `serverId != nil`. Preserves `serverId == nil` record
 | 39 | **Server photo URLs include `/static/` prefix** | Server stores at `app/static/uploads/`; Flask serves at `/static/uploads/`. URL = `ServerConfig.current + "/static/" + relativePath`. Missing `/static/` returns 404. |
 | 40 | **Use `RetryablePhotoView` for all server photo loads** | `AsyncImage` has no retry — once in `.failure` it stays there for the view's lifetime. `RetryablePhotoView` allows tap-to-retry by toggling `.id(reloadToken)`. |
 | 41 | **Only use SF Symbols available on iOS 17** | `photo.slash` and `photo.badge.exclamationmark` are absent on some devices. Use `exclamationmark.triangle` for all photo-error states. |
-| 42 | **`clearServerPulledData()` boundary is `serverId != nil`** | Old boundary `syncStatus == "synced" && inspectionLocalId == ""` missed inspector-created synced issues, leaving stale serverIds that caused "Issue not found" after server switch. |
+| 42 | ~~**`clearServerPulledData()` boundary is `serverId != nil`**~~ | **Superseded by rule 88.** The function is gone; `purgeSessionScopedData()` replaces it and no longer filters by `serverId` at all. The history is still worth knowing: the boundary was widened twice (from `syncStatus == "synced" && inspectionLocalId == ""` to `serverId != nil`) and was wrong both times, because the problem was never which *issues* to delete — it was that issues are not the only server-scoped model, and logout is not the moment that matters. |
 | 43 | **`processIssueQueue` clears `photoLocalPaths` after successful submit** | Prevents `IssueDetailView` from rendering a duplicate "local photos" section alongside the server photos section for synced issues. |
 | 44 | **`StandaloneIssueView` uses `inspectionLocalId = ""`** | Same pattern as server-pulled issues. `processIssueQueue`'s parent-inspection guard evaluates `parent?.syncStatus == "failed"` → `false` for `""`, so standalone issues submit normally. |
 | 45 | **Facility lists deduplicate by `serverId` at both storage and display layers** | Storage: `pullReferenceData()` deduplicates server response before upsert. Display: `filteredFacilities` in both `StartInspectionView` and `StandaloneIssueView` uses `filter { seen.insert($0.serverId).inserted }`. |
@@ -729,6 +875,16 @@ Deletes `LocalIssue` where `serverId != nil`. Preserves `serverId == nil` record
 | 80 | **"Schedule Follow-up" is a server-side plan — it is the one action in the app that cannot work offline, and its link must survive the client forgetting it** | History detail (`HistoryDetailView`) carries three toolbar actions: **Re-inspect Now** (immediate, opens the linked re-inspection), **Schedule Follow-up** (deferred), and the existing email button. Starting an inspection writes locally and syncs later, but scheduling writes a `ScheduledInspection` row that only the server can create — there is no local record to queue, so the button is `.disabled(!sync.isOnline)` and failures report inline instead of dismissing as though they worked. Do not "fix" this by faking a local schedule: `pullScheduledInspections()` deletes any row the server doesn't return, so it would vanish on the next sync. The link itself is `scheduled_inspections.parent_inspection_id` (phase45): both start paths inherit it onto the inspection (`ScheduledStartTarget.parentServerId` on iPad, the web's `scheduled_inspections.start`), **and** the API's create-inspection endpoint re-derives it from the schedule when the client sends none — a belt-and-braces step that matters because an older build or a resumed draft would otherwise submit a plain inspection and leave the parent flagged forever. Creation is inspector-writable, a deliberate divergence from the web's `@project_manager_required`, and the endpoint is deliberately narrow: it takes only a parent + date and derives facility/template/assignee, so a follow-up can only ever target the thing it follows up on. |
 | 81 | **Never commit a local-dev override — repointing `ServerOption.primary` at localhost took production login down** | July 2026: `primary` was changed from `https://jqc.ltservicesinc.com` to `http://127.0.0.1:5055` for local API work, with `NSAllowsArbitraryLoads=true` added to `Info.plist` for cleartext. Both shipped in `dac7e6c`, so the "Primary" entry in the server picker dialled a developer laptop and **every inspector failed to log in** — only the untouched secondary worked. Symptom in the device log is unmistakable and is *not* an auth problem: `NSErrorFailingURLStringKey=http://127.0.0.1:5055/...` with `Connection refused [61]`. Revert a dev override in the same session that adds it; being aware of it is not a safeguard. Prefer an override that *cannot* be committed — a debug-only scheme argument, an xcconfig, or `#if DEBUG` — over editing this shared production constant. One thing that saved us: `ServerConfig.current` validates the stored UserDefaults string through `ServerOption(rawValue:)` and falls back to `.primary`, so a stale localhost selection self-heals on update — keep that round-trip validation. ATS exceptions must be scoped to the host (`NSExceptionDomains` for `127.0.0.1`/`localhost`); `NSAllowsArbitraryLoads` disables TLS validation for the *production* servers too and is an App Store review trigger. |
 | 82 | **A shared `static` read from `APIClient` (or any nonisolated context) must be declared `nonisolated`** | `SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor` makes **every** type implicitly `@MainActor`, including a bare constants-holder `enum`. `APIClient` is an `actor`, so reading such a static from it warns *"Main actor-isolated static property 'X' can not be referenced from a nonisolated context"* — and that becomes a **hard error** under the Swift 6 language mode, so it will block a toolchain move. `PhotoCaptureFormat.iso8601` hit this from the two `captured_at` multipart call sites. Mark the enclosing enum `nonisolated`, matching `Constants`, `ServerConfig`, `PhotoCaptureFormat` and `SyncManager.isoFormatter` (rule 35). Do **not** reach for `nonisolated(unsafe)` (used nowhere here — it hides the problem) or allocate a formatter per call (rule 35 exists because that cost is real on the upload/sync paths). Foundation formatters are thread-safe for formatting, so one shared instance is correct. |
+| 83 | **A photo upload error is TRANSIENT — leave the row `"pending"`. `"failed"` means every attempt was used, and nothing else may set it** | Aug 2026, the lost-photo defect. `processPhotoQueue` marked `uploadStatus = "failed"` on the *first* error; `processInspectionQueue`'s `photosReady` accepted `"failed"` as settled and submitted anyway; `APIClient.submitInspection` rewrote the surviving `local://` value to `""`; the inspection was then marked `synced` forever. **And nothing anywhere ever moved a row off `"failed"`** — `SyncStatusView.retryAllFailed()` reset `LocalInspection`/`LocalIssue` only. One dropped connection therefore destroyed an evidence photo permanently and silently, with the sync reported as successful. Now: `uploadRetryCount` increments and the row stays `"pending"` until `SyncManager.maxPhotoUploadAttempts` (5), so the next sync retries it **and** the parent keeps waiting. The cost is that a completed inspection can sit in the outbox for a few sync cycles while a photo retries — that is the correct trade; submitting first is what caused the loss. `retryAllFailed()` now resets photos too, and is the only escape hatch from terminal `"failed"`. `PhotoDiagnosticView` exists to size the damage already done and must stay read-only. |
+| 84 | **Clear `LocalIssue.photoLocalPaths` only when EVERY photo reached the server** | The clear was unconditional after a successful submit, so a partial upload left the JPEGs on disk with nothing referencing them — invisible to `IssueDetailView` and to `PhotoDiagnosticView` alike. Keeping them costs a duplicate photo section in the detail view at worst (rule 34's cosmetic concern); dropping them costs the evidence. `processIssueQueue` now guards on `issuePhotos.allSatisfy { $0.uploadStatus == "uploaded" }`. |
+| 85 | **Send an issue's evidence photos IN the create request — never in a follow-up call after it is marked `"synced"`** | `submitIssue()` sent `photo_path` only, then `processIssueQueue` fired `PATCH /issues/<id>/photos` for the rest with `try? await`. By then `syncStatus == "synced"`, so `processIssueQueue` never revisited the issue: one failed PATCH silently cost every photo after the first, and the loss became invisible on device too once `pullAssignedIssues` overwrote `photoServerPaths` with the server's copy. The split was never necessary — `POST /api/v1/issues` already accepts `result_photos` and stores it in `mobile_photo_paths` (`app/api/issues.py`, `create_issue`), and `processPhotoQueue` fully populates `photoServerPaths` *before* `processIssueQueue` runs, so the extras were always known at create time. `submitIssue()` now sends `photo_path` + `result_photos` together: attachment is atomic with creation, there is no `synced`-but-unattached window to reconcile, and `mobile_local_id` idempotency covers retrying the whole request. The generalisation holds beyond photos — if a second call is needed after a record is marked synced, either fold it into the first or persist the debt; `try?` there means silent permanent loss. |
+| 86 | **Two `PendingPhoto` rows sharing a local file must both receive the uploaded `serverPath`** | The de-dup pass marked the duplicates `"uploaded"` without ever setting `serverPath`, so the same image attached to two form fields submitted the second field blank. `processPhotoQueue` now uploads once and settles every row from a `localFilePath -> serverPath` map (a row whose twin failed stays `"pending"` so both retry together). Uploading once still matters independently: two uploads of one file yield two server filenames and duplicate the photo in the evidence and the PDF. |
+| 87 | **`cleanupOrphanedPhotos()` sweeps `JQC/Photos` only, references EVERY surviving `local://` path, and never deletes a file younger than 7 days** | It pointed at `Documents/JQCPhotos`, which no writer has ever used — `contentsOfDirectory` failed, the `guard` returned, and it silently deleted nothing for its entire life while photos accumulated. Correcting the path is only safe alongside rule 83, and only with the reference set widened: a `local://` sentinel surviving on a *submitted* inspection means that photo never reached the server, so the file is the only copy left and is exactly what `PhotoDiagnosticView` reports as recoverable — the old draft-only filter would have deleted it. `JQC/ResultPhotos` is deliberately **not** swept: those files are staged in `IssueDetailView`'s `@State` with no database row, so nothing can prove one is unused. The 7-day age floor covers that flow and the window between writing a JPEG and saving the record that points at it. |
+| 88 | **Local data is scoped to a `(server, userId)` pair — purge on an identity CHANGE, never on logout** | Two defects, one cause. (a) Logout deliberately kept the cache so the same inspector could work offline after signing back in — correct — but nothing checked that the next sign-in *was* the same inspector. `pullAssignedIssues`' reconciliation only deletes rows with `inspectionLocalId == ""`, so device-authored synced issues survived indefinitely and a different inspector on the same iPad simply inherited them. (b) The server switch cleared `LocalIssue` alone, leaving `LocalInspection` rows carrying `facilityServerId`/`templateServerId` values that name different rows on the server being switched to — ready to be submitted against it. `SessionScope` (UserDefaults, **not** Keychain — it must outlive `KeychainHelper.clearAll()`) records the pair; `AuthManager.reconcileSessionScope()` compares on every `login()`/`restoreSession()` and calls `SyncManager.purgeSessionScopedData()` only on a mismatch, before `isAuthenticated` flips so no view ever renders the previous user's data. `LocalInspection` is the only model with an author (`inspectorUserId`), so it is the only one whose unsent rows can be handed back; `LocalIssue` has none, and submitting one under a different inspector's credentials would put a false name on a QC record. A nil marker adopts the existing data rather than purging — fresh install and pre-marker upgrade are indistinguishable, and guessing wrong would delete the signing-in user's own draft. |
+| 89 | **Never raise a second alert from inside the first one's button action** | Both alerts hang off the same view, so the new presentation is discarded while the first is still tearing down. `ExecuteInspectionView`'s Submit set `showNoGPSAlert = true` from inside the confirm alert's action, and the warning simply never appeared — tapping Submit without a GPS fix did *nothing at all*: no alert, no submission, no feedback. Park the intent in a `@State` flag and act on it from `onChange(of:)` when the first alert's binding flips false, with a short hop so the dismissal animation has finished. Applies to `.sheet`/`.confirmationDialog` chained onto one view too. |
+| 90 | **`date` form fields are `"yyyy-MM-dd"`, and an unanswered one must render as unanswered** | Two defects in one widget, both in `CellDatePicker` and `DateFieldView`. (a) They stored `ISO8601DateFormatter().string(...)` — a full `2026-08-18T14:30:00Z` timestamp — into a field the web writes with `<input type="date">` and both the read-only grid and the PDF print verbatim. `FormDateFormat` (UTC + POSIX, `yyyy-MM-dd`) is now the single definition, and parses a leading date out of legacy timestamp values. (b) A `DatePicker` bound to an empty value still displays TODAY, so the field looked answered — but the setter only fires on a *change*, so selecting the already-shown date wrote nothing and `missingRequiredFields()` reported it missing with a date visible on screen. An explicit "Set date" affordance replaces the picker while the value is empty, plus an × to return to unanswered. |
+| 91 | **A stored photo path is only valid inside the container that wrote it — resolve by FILENAME, never trust the absolute path** | Photo paths are absolute and embed the app-container UUID (`/var/mobile/Containers/Data/Application/<UUID>/Documents/JQC/Photos/<file>.jpg`). iOS assigns a NEW container UUID on every app update, reinstall and restore: `Documents/` survives, every stored path dies. Nothing accounted for that, so `uploadPhoto`'s `FileManager.contents(atPath:)` returned nil and the upload could **never** succeed no matter how often it retried — the path named a container that no longer existed. Any photo still awaiting upload when the app updated was stranded permanently and its inspection submitted with the field blank. Confirmed in the field on inspection #887 (Aug 2026): nine photos, all `upload: pending`, **0 rows failed**, every file present on disk under the current container. `PhotoStore.resolve()` re-resolves by basename (filenames are per-save UUIDs, so a basename is unambiguous) and is now used by every reader, writer and deleter of a stored path. `processPhotoQueue` heals paths BEFORE the pending filter so a row already given up on is revived, and resets `uploadRetryCount` — earlier failures were about a path that no longer applies. **The retry budget added in rule 83 does not help here and never could: retrying an unresolvable path is futile.** Two corollaries: an unresolvable path now fails fast instead of burning five sync cycles, and `cleanupOrphanedPhotos` compares FILENAMES — comparing full paths meant that after an update every reference missed its own file and the sweep would have deleted exactly the photos that were still recoverable. |
+| 92 | **The notification API is a POLLER, not an inbox — `LocalNotification` is the inbox** | `GET /api/v1/notifications` filters to `is_read = False` and never sends the flag (`app/api/notifications.py`), so the iPad list was all-unread by construction — every row rendered identically, with nothing to read or dismiss — and marking one read made it disappear rather than grey out. The fix is a local store the poll upserts into and never deletes from; a row's absence from a response carries no information. Read state is shared with the web: a tap or Mark All is a **deliberate user action**, so it pushes via `PATCH /notifications/mark-read` (previously dead code) and clears the web badge too. That does not contradict the long-standing rule against marking read on POLL — auto-marking would zero the user's web badge just because the iPad was switched on, and `pollNotifications()` still never does it. Writes are local-first with `readSyncPending`, drained by `pushNotificationReadState()`, so reading works in airplane mode. If a mobile inbox endpoint mirroring `routes/notifications.py::index` is ever added, this model becomes a cache of it rather than the source of truth. |
 
 ---
 

JanitorialQC/JanitorialQCApp.swift

@@ -102,6 +102,7 @@ struct JanitorialQCApp: App {
             LocalIssue.self,
             LocalScheduledInspection.self,
             LocalFollowUpRequest.self,
+            LocalNotification.self,
             PendingPhoto.self,
             SyncQueueEntry.self,
         ], isUndoEnabled: false) { result in

JanitorialQC/Models/LocalFollowUpRequest.swift

@@ -49,6 +49,15 @@ final class LocalFollowUpRequest {
     /// Stored raw; read through `note` for the normalised form.
     var followUpNote: String?
 
+    /// Display name of the inspector this follow-up was handed to, when it was
+    /// assigned to somebody other than whoever performed the original
+    /// inspection (phase53). nil = it belongs to the original inspector.
+    ///
+    /// Purely for display — the server only ever returns follow-ups this user
+    /// owns, so nothing here decides what is shown. Optional so existing
+    /// SwiftData stores migrate lightweight (rule 8).
+    var assignedToName: String?
+
     /// Set on device the moment a re-inspection of this request is submitted, so
     /// the FOLLOW-UP REQUESTED lists hide the row immediately — online or
     /// offline — without waiting for the round trip.
@@ -128,6 +137,7 @@ final class LocalFollowUpRequest {
         self.overallScore         = api.overallScore
         self.inspectionDateString = api.inspectionDate ?? ""
         self.followUpNote         = api.followUpNote
+        self.assignedToName       = api.followUpAssignedToName
         self.fulfilledLocally     = false
         self.parentFormDataJSON   = Self.encode(api)
         self.updatedAt            = Date()
@@ -170,6 +180,7 @@ final class LocalFollowUpRequest {
         self.overallScore         = api.overallScore
         self.inspectionDateString = api.inspectionDate ?? ""
         self.followUpNote         = api.followUpNote
+        self.assignedToName       = api.followUpAssignedToName
         self.parentFormDataJSON   = Self.encode(api)
         // The server is authoritative. Being returned by the pull at all means
         // the follow-up is still outstanding, so any local "just did it" flag is

JanitorialQC/Models/LocalNotification.swift

@@ -0,0 +1,102 @@
+// Models/LocalNotification.swift
+// ------------------------------
+// SwiftData model backing the in-app notification inbox.
+//
+// WHY THIS IS A LOCAL STORE, unlike every other server-backed list in the app.
+// `GET /api/v1/notifications` is a POLLER, not an inbox: it filters to
+// `is_read = False` and never sends the flag at all
+// (app/api/notifications.py :: list_notifications). So the instant a
+// notification is marked read the server stops returning it — there is no
+// response the iPad could render as "read", and before this every row in the
+// list was unread by definition, which is why they all looked identical.
+// Keeping our own copy is the only way to show read and unread side by side.
+//
+// The web already has a real inbox (routes/notifications.py :: index, with an
+// all / unread / read filter and paging). If a mobile equivalent is ever added,
+// this model should become a cache of it rather than the source of truth — see
+// rule 92.
+//
+// Scope: notifications are per-user, so this is purged on an identity change
+// like everything else (rule 88).
+
+import Foundation
+import SwiftData
+
+@Model
+final class LocalNotification {
+
+    /// Server notification id — stable identity, and the value
+    /// `PATCH /api/v1/notifications/mark-read` takes.
+    /// No inline default: a `.unique` key must not carry one (rule 63).
+    @Attribute(.unique) var serverId: Int
+
+    var title: String
+    var body: String
+
+    /// e.g. `issue_assigned`, `sla_alert`, `scheduled_inspection`. Nil for rows
+    /// created before the server's phase17 migration added the column.
+    var eventType: String?
+
+    /// Set when the notification refers to an issue — drives "View Issue".
+    var issueId: Int?
+
+    /// Server `created_at`, parsed. Falls back to receipt time when the string
+    /// cannot be parsed so ordering never collapses to a single instant.
+    var createdAt: Date
+
+    /// When THIS device first saw it. Distinct from `createdAt`: a notification
+    /// raised while the iPad was offline arrives late but keeps its real time.
+    var receivedAt: Date
+
+    // ── Read state ────────────────────────────────────────────────────────
+    // Set optimistically on tap / Mark All so the UI responds offline, then
+    // pushed to the server. Read state is shared with the web (rule 92).
+
+    var isRead: Bool = false
+    var readAt: Date?
+
+    /// True while this row's read state has not yet reached the server.
+    /// Drained by `SyncManager.pushNotificationReadState()`.
+    ///
+    /// Non-optional with an inline default so SwiftData migrates lightweight
+    /// (rule 8).
+    var readSyncPending: Bool = false
+
+    init(from api: APINotification) {
+        self.serverId        = api.id
+        self.title           = api.title
+        self.body            = api.body
+        self.eventType       = api.eventType
+        self.issueId         = api.issueId
+        self.createdAt       = SyncManager.isoFormatter.date(from: api.createdAt) ?? Date()
+        self.receivedAt      = Date()
+        self.isRead          = false
+        self.readAt          = nil
+        self.readSyncPending = false
+    }
+
+    /// Refresh the mutable text from a later poll.
+    ///
+    /// Deliberately does NOT touch `isRead`. The endpoint only ever returns
+    /// UNREAD rows, so being returned again carries no information about read
+    /// state — it usually just means our mark-read has not been pushed yet.
+    /// Clobbering it here would make a notification the inspector just opened
+    /// pop straight back to unread.
+    func update(from api: APINotification) {
+        self.title     = api.title
+        self.body      = api.body
+        self.eventType = api.eventType
+        self.issueId   = api.issueId
+        if let parsed = SyncManager.isoFormatter.date(from: api.createdAt) {
+            self.createdAt = parsed
+        }
+    }
+
+    /// Mark read locally and queue the server push. Idempotent.
+    func markRead() {
+        guard !isRead else { return }
+        isRead          = true
+        readAt          = Date()
+        readSyncPending = true
+    }
+}

JanitorialQC/Models/PendingPhoto.swift

@@ -22,9 +22,32 @@ final class PendingPhoto {
     /// Populated after successful upload
     var serverPath: String?
     /// "pending" | "uploaded" | "failed"
+    ///
+    /// "failed" is TERMINAL: it means every upload attempt was used up, and it
+    /// is what lets processInspectionQueue stop waiting and submit without this
+    /// photo. A *transient* error must therefore leave the row "pending" — see
+    /// `uploadRetryCount`. Marking "failed" on the first error is what turned a
+    /// single dropped connection into a permanently lost evidence photo.
     var uploadStatus: String
+    /// Consecutive failed upload attempts. The row stays "pending" — and so
+    /// keeps blocking its parent's submission — until this reaches
+    /// `SyncManager.maxPhotoUploadAttempts`.
+    ///
+    /// Non-optional with an inline default so SwiftData migrates lightweight
+    /// (CLAUDE.md rule 8): rows in existing stores read as 0.
+    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
@@ -53,6 +76,7 @@ final class PendingPhoto {
         self.fieldId          = fieldId
         self.serverPath       = nil
         self.uploadStatus     = "pending"
+        self.uploadRetryCount = 0
         self.createdAt        = Date()
         // Fall back to now when the caller has no recorded capture moment —
         // still far better than the server's upload-time default.

JanitorialQC/Sync/SyncManager.swift

@@ -24,12 +24,14 @@ class SyncManager: ObservableObject {
     @Published var pendingCount = 0
     /// Dashboard KPI stats fetched from the server. Nil until first successful fetch.
     @Published var dashboardStats: APIDashboardStats?
-    /// Count of notifications received since last resetNotificationPoller().
-    /// Incremented on each poll that returns new items; reset to 0 on logout.
+    /// Number of `LocalNotification` rows with `isRead == false`.
+    ///
+    /// Derived from the store rather than counted as items arrive: read state
+    /// is now real and can change from either end (a tap here, Mark All, or the
+    /// same account reading on the web), so an incrementing tally would drift.
+    /// Refreshed by `refreshUnreadNotificationCount()` after every poll and
+    /// every read action.
     @Published var unreadNotificationCount = 0
-    /// The most recent batch of notifications (up to 50) for the in-app inbox.
-    /// Replaced entirely on each successful poll; empty until first fetch.
-    @Published var recentNotifications: [APINotification] = []
 
     // ── Dependencies ──────────────────────────────────────────────────────
 
@@ -142,11 +144,17 @@ class SyncManager: ObservableObject {
     }
 
     /// Called on logout so the next login starts a clean fetch.
+    ///
+    /// Resets the CURSOR and stops the task; it does not touch the stored
+    /// inbox. The same inspector signing back in should still find their
+    /// notifications and their read state, exactly as they find their cached
+    /// facilities and issues — a different inspector is handled by the identity
+    /// purge instead (rule 88). `unreadNotificationCount` is recomputed from
+    /// the store rather than zeroed, so the badge stays truthful.
     func resetNotificationPoller() {
-        lastNotificationFetch   = nil
-        unreadNotificationCount = 0
-        recentNotifications     = []
+        lastNotificationFetch = nil
         stopPollTask()
+        refreshUnreadNotificationCount()
     }
 
     /// Called when the app enters the background (scenePhase == .background).
@@ -164,10 +172,10 @@ class SyncManager: ObservableObject {
         Task { await triggerSync() }
     }
 
-    /// Call when the user opens the NotificationsView to clear the badge.
-    func markNotificationsViewed() {
-        unreadNotificationCount = 0
-    }
+    // NOTE: `markNotificationsViewed()` is gone. It zeroed the badge merely
+    // because the inbox had been OPENED, which is incompatible with showing
+    // real read state — the badge would read 0 while every row still rendered
+    // as unread. Reading is now an explicit act: tap a row, or Mark All Read.
 
     // ── Notification polling ──────────────────────────────────────────────
 
@@ -177,25 +185,39 @@ class SyncManager: ObservableObject {
             let notifications = try await APIClient.shared.fetchNotifications(since: lastNotificationFetch)
             guard !notifications.isEmpty else { return }
 
-            // Deliver a local notification for each new item
-            for n in notifications {
-                deliverLocalNotification(n)
+            // Upsert into the local inbox. The endpoint only ever returns
+            // UNREAD rows and never sends the flag, so a row disappearing from
+            // the response says nothing — it may have been read on the web, or
+            // simply be older than the cursor. Rows are therefore never deleted
+            // here; `pruneReadNotifications()` handles retention instead.
+            if let context = modelContext {
+                let existing = (try? context.fetch(FetchDescriptor<LocalNotification>())) ?? []
+                var byServerId: [Int: LocalNotification] = [:]
+                for row in existing { byServerId[row.serverId] = row }
+
+                for api in notifications {
+                    if let row = byServerId[api.id] {
+                        row.update(from: api)
+                    } else {
+                        context.insert(LocalNotification(from: api))
+                        // Banner ONLY for genuinely new rows. Previously every
+                        // polled item was delivered, so a cold launch (cursor
+                        // nil → the server returns all unread) re-banner'd the
+                        // inspector's whole backlog on every app start.
+                        deliverLocalNotification(api)
+                    }
+                }
+                try? context.save()
+                refreshUnreadNotificationCount()
             }
 
-            // 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
-            // next poll only fetches newer items — do NOT mark notifications as
-            // read on the server. Read state is a deliberate user action managed
-            // via the web app; marking read here would cause the web badge count
-            // to always show zero when the iPad has polled before the user checks.
+            // Advance the cursor so the next poll only fetches newer items.
+            //
+            // Still no implicit mark-read: the server's read state changes only
+            // on a deliberate user action — a tap or Mark All, which route
+            // through markNotificationRead/markAllNotificationsRead. Marking on
+            // poll would zero the user's WEB badge simply because the iPad was
+            // switched on (rule 92).
             let dates = notifications.compactMap { Self.isoFormatter.date(from: $0.createdAt) }
             if let newest = dates.max() {
                 lastNotificationFetch = newest
@@ -208,6 +230,86 @@ class SyncManager: ObservableObject {
         }
     }
 
+    // ── Notification read state ───────────────────────────────────────────
+
+    /// Recount unread rows and publish. Cheap: one fetch, no relationships.
+    func refreshUnreadNotificationCount() {
+        guard let context = modelContext else { return }
+        let all = (try? context.fetch(FetchDescriptor<LocalNotification>())) ?? []
+        unreadNotificationCount = all.filter { !$0.isRead }.count
+    }
+
+    /// Mark one notification read — optimistically local, then pushed.
+    ///
+    /// Local first so the inbox responds instantly and works offline; the
+    /// server call is best-effort and `readSyncPending` keeps the debt until it
+    /// lands (the same shape as every other write in this app).
+    func markNotificationRead(_ notification: LocalNotification) async {
+        guard !notification.isRead else { return }
+        notification.markRead()
+        try? modelContext?.save()
+        refreshUnreadNotificationCount()
+        await pushNotificationReadState()
+    }
+
+    /// Mark every unread notification read.
+    func markAllNotificationsRead() async {
+        guard let context = modelContext else { return }
+        let all = (try? context.fetch(FetchDescriptor<LocalNotification>())) ?? []
+        let unread = all.filter { !$0.isRead }
+        guard !unread.isEmpty else { return }
+        for row in unread { row.markRead() }
+        try? context.save()
+        refreshUnreadNotificationCount()
+        await pushNotificationReadState()
+    }
+
+    /// Push any locally-read notifications the server does not know about yet.
+    ///
+    /// Runs on every sync as well as immediately after a read action, so a
+    /// notification opened in airplane mode still clears the web badge once the
+    /// iPad reconnects. `markNotificationsRead` was dead code before this.
+    func pushNotificationReadState() async {
+        guard isOnline, AuthManager.shared.isAuthenticated,
+              let context = modelContext
+        else { return }
+
+        let all = (try? context.fetch(FetchDescriptor<LocalNotification>())) ?? []
+        let pending = all.filter { $0.readSyncPending }
+        guard !pending.isEmpty else { return }
+
+        do {
+            try await APIClient.shared.markNotificationsRead(ids: pending.map { $0.serverId })
+            for row in pending { row.readSyncPending = false }
+            try? context.save()
+        } catch {
+            // Left pending — retried on the next sync. The row already reads as
+            // read locally, which is what the inspector asked for.
+        }
+    }
+
+    /// Drop read notifications older than the retention window.
+    ///
+    /// Needed because nothing else ever deletes a row: the poll endpoint cannot
+    /// tell us a notification is gone (it only returns unread), so without this
+    /// the inbox would grow without bound. Unread rows are never pruned however
+    /// old — an unread alert is outstanding work.
+    private static let notificationRetention: TimeInterval = 30 * 24 * 3600   // 30 days
+
+    private func pruneReadNotifications(context: ModelContext) {
+        let cutoff = Date().addingTimeInterval(-Self.notificationRetention)
+        let all = (try? context.fetch(FetchDescriptor<LocalNotification>())) ?? []
+        var removed = 0
+        for row in all where row.isRead && !row.readSyncPending && row.createdAt < cutoff {
+            context.delete(row)
+            removed += 1
+        }
+        if removed > 0 {
+            try? context.save()
+            print("[JQC] Sync | pruneReadNotifications | removed \(removed) row(s)")
+        }
+    }
+
     // ── Local notification delivery ───────────────────────────────────────
 
     private func deliverLocalNotification(_ n: APINotification) {
@@ -284,6 +386,9 @@ class SyncManager: ObservableObject {
         // for the 60-second timer — ensures the inspector sees assignments
         // and follow-up requests as soon as the app goes online.
         await pollNotifications()
+        // Drain read state marked while offline, then trim the inbox.
+        await pushNotificationReadState()
+        pruneReadNotifications(context: context)
 
         // Fetch dashboard KPIs — best-effort, non-fatal on failure.
         await fetchDashboardStats()
@@ -299,35 +404,78 @@ class SyncManager: ObservableObject {
 
     // ── Outbox: Photos ────────────────────────────────────────────────────
 
+    /// Upload attempts before a PendingPhoto is given up on.
+    ///
+    /// Until this is reached the row stays "pending", which does two things:
+    /// the next sync retries it, AND processInspectionQueue keeps waiting
+    /// rather than submitting the inspection with a blank photo field.
+    static let maxPhotoUploadAttempts = 5
+
     private func processPhotoQueue(context: ModelContext) async {
         // Fetch all then filter in Swift — #Predicate cannot reference
         // string literals against PendingPhoto.uploadStatus reliably
         // when the predicate type is inferred across model boundaries.
         guard let allPhotos = try? context.fetch(FetchDescriptor<PendingPhoto>()) else { return }
-        var pending = allPhotos
+
+        // ── Heal stale container paths FIRST ──────────────────────────────
+        //
+        // Stored paths are absolute and embed the app-container UUID, which iOS
+        // reassigns on every app update, reinstall and restore. The files
+        // survive in Documents/; the paths do not. `uploadPhoto` then fails on
+        // `FileManager.contents(atPath:)` and can NEVER succeed however often it
+        // is retried, because the path names a container that no longer exists.
+        //
+        // This is what stranded inspection #887's nine photos: reported as
+        // `upload: pending`, 0 rows failed, every file present on disk under the
+        // CURRENT container. Retrying was futile; re-resolving is all that was
+        // ever needed. See Utils/PhotoStore.swift and rule 91.
+        //
+        // Runs before the "pending" filter on purpose, so a row already given up
+        // on is REVIVED rather than left stranded — otherwise a single app
+        // update permanently costs an inspection its evidence.
+        var healed = 0
+        for photo in allPhotos where photo.uploadStatus != "uploaded" {
+            guard let live = PhotoStore.resolve(photo.localFilePath),
+                  live != photo.localFilePath
+            else { continue }
+            photo.localFilePath = live
+            // Previous failures were about a path that no longer applies, so
+            // they are not evidence about this one — reset the retry budget.
+            photo.uploadRetryCount = 0
+            photo.lastUploadError  = nil
+            if photo.uploadStatus == "failed" { photo.uploadStatus = "pending" }
+            healed += 1
+        }
+        if healed > 0 {
+            try? context.save()
+            print("[JQC] Sync | processPhotoQueue | re-resolved \(healed) stale photo path(s)")
+        }
+
+        let pending = allPhotos
             .filter { $0.uploadStatus == "pending" }
             .sorted { $0.createdAt < $1.createdAt }
 
-        // Defense-in-depth: if two PendingPhoto rows somehow reference the
-        // exact same local file (e.g. a future call site re-submitting the
-        // same photo array), only upload it once. The primary fix for photo
-        // duplication is the re-entrancy guard in triggerSync(), but this
-        // keeps processPhotoQueue itself safe even if it's ever invoked
-        // outside that guard.
-        var seenPaths = Set<String>()
+        // Two rows can legitimately point at the same file — the same image
+        // attached to two form fields, or a call site re-submitting a photo
+        // array. Upload it once, then give EVERY row that references it the
+        // same server path.
+        //
+        // The previous version marked the duplicates "uploaded" up front and
+        // never set their serverPath, so the second field was submitted blank.
+        // Settling them from the upload result instead keeps every field
+        // pointing at real evidence. (Uploading once still matters: two uploads
+        // of one file produce two different server filenames and duplicate the
+        // photo in the issue's evidence and its PDF.)
+        var firstByPath: [String: PendingPhoto] = [:]
+        var toUpload:   [PendingPhoto] = []
         var duplicates: [PendingPhoto] = []
-        pending = pending.filter { photo in
-            if seenPaths.contains(photo.localFilePath) {
+        for photo in pending {
+            if firstByPath[photo.localFilePath] == nil {
+                firstByPath[photo.localFilePath] = photo
+                toUpload.append(photo)
+            } else {
                 duplicates.append(photo)
-                return false
             }
-            seenPaths.insert(photo.localFilePath)
-            return true
-        }
-        for dup in duplicates {
-            // Mark the duplicate row as uploaded without re-uploading — the
-            // first row for this file will populate serverPath/photoServerPaths.
-            dup.uploadStatus = "uploaded"
         }
 
         // Pre-fetch parent records ONCE before the loop.
@@ -340,7 +488,21 @@ class SyncManager: ObservableObject {
         let allInspections = (try? context.fetch(FetchDescriptor<LocalInspection>())) ?? []
         let allIssues      = (try? context.fetch(FetchDescriptor<LocalIssue>()))      ?? []
 
-        for photo in pending {
+        var uploadedPaths: [String: String] = [:]   // localFilePath -> serverPath
+
+        for photo in toUpload {
+            // Paths were healed above, so an unresolvable one here means the
+            // file is genuinely gone from every container. Fail fast rather
+            // than burning five attempts and five sync cycles on it.
+            guard PhotoStore.resolve(photo.localFilePath) != nil else {
+                photo.uploadStatus     = "failed"
+                photo.uploadRetryCount = Self.maxPhotoUploadAttempts
+                photo.lastUploadError  = "File no longer on this device: "
+                    + PhotoStore.filename(of: photo.localFilePath)
+                try? context.save()
+                continue
+            }
+
             do {
                 // Capture metadata was recorded at the shutter, not now — the
                 // sync may run hours after an offline capture, and the server
@@ -352,35 +514,180 @@ class SyncManager: ObservableObject {
                     latitude:   photo.captureLatitude,
                     longitude:  photo.captureLongitude
                 )
-                photo.serverPath   = serverPath
-                photo.uploadStatus = "uploaded"
-
-                // 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
-                    allInspections.first(where: { $0.localId == entityId })?
-                        .setValue(serverPath, forFieldId: fieldId)
-                }
-
-                // Update parent issue photo paths array.
-                // Pre-fetched before the loop — not repeated per photo.
-                if photo.entityType == "issue" {
-                    let entityId = photo.entityLocalId
-                    if let issue = allIssues.first(where: { $0.localId == entityId }) {
-                        var paths = issue.photoServerPaths
-                        if !paths.contains(serverPath) { paths.append(serverPath) }
-                        issue.photoServerPaths = paths
-                    }
-                }
+                photo.serverPath       = serverPath
+                photo.uploadStatus     = "uploaded"
+                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 {
-                photo.uploadStatus = "failed"
+                // Treat the error as TRANSIENT by default. Leaving the row
+                // "pending" means the next sync retries it and — critically —
+                // processInspectionQueue keeps waiting instead of submitting
+                // the inspection with this field blanked to "" by
+                // APIClient.submitInspection.
+                //
+                // The old code marked "failed" on the very first error, and
+                // nothing anywhere ever moved a row back off "failed". One
+                // dropped connection therefore cost the inspection its evidence
+                // 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 "
+                        + "\(Self.maxPhotoUploadAttempts) attempts. "
+                        + "Open Pending Sync → Retry Failed Items to try again."
+                }
                 try? context.save()
             }
         }
+
+        // Settle rows that shared a file with one just uploaded. A row whose
+        // twin failed is deliberately left "pending" so both retry together.
+        for dup in duplicates {
+            guard let serverPath = uploadedPaths[dup.localFilePath] else { continue }
+            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,
+                                             issues: allIssues, context: context)
+        }
+        try? context.save()
+    }
+
+    /// Carry a photo across to an issue that has ALREADY been created on the
+    /// server, which the create request therefore could not have included.
+    ///
+    /// Only reachable via recovery: the photo exhausted
+    /// `maxPhotoUploadAttempts`, the issue was submitted without it (rule 83
+    /// deliberately lets that happen rather than blocking forever), and the
+    /// inspector later hit Pending Sync → Retry Failed Items and the upload
+    /// succeeded. In the normal path processPhotoQueue runs BEFORE
+    /// processIssueQueue, so `issue.serverId` is still nil here and this does
+    /// nothing — which is the discriminator, and why this is not a rule 85
+    /// violation.
+    ///
+    /// Must happen now, in this same pass: `pullAssignedIssues` later overwrites
+    /// `photoServerPaths` with the server's copy, so a photo left only in local
+    /// state would be erased before anything else could notice it.
+    ///
+    /// Best-effort. A failure here leaves the photo attached locally but not
+    /// server-side until the next pull overwrites it — the residual limitation
+    /// noted in rule 83. Reaching this at all takes five consecutive upload
+    /// failures, and the PATCH merges idempotently, so a repeat is harmless.
+    private func pushLateIssuePhotoIfNeeded(
+        _ serverPath: String,
+        for photo: PendingPhoto,
+        issues: [LocalIssue],
+        context: ModelContext
+    ) async {
+        guard photo.entityType == "issue" else { return }
+        let entityId = photo.entityLocalId
+        guard let issue = issues.first(where: { $0.localId == entityId }),
+              let issueServerId = issue.serverId
+        else { return }
+
+        do {
+            try await APIClient.shared.updateIssuePhotos(
+                issueId: issueServerId, resultPhotos: [serverPath]
+            )
+            issue.syncErrorMessage = nil
+        } catch {
+            issue.syncErrorMessage =
+                "A recovered photo could not be attached: \(error.localizedDescription)"
+        }
+        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(
+        _ serverPath: String,
+        for photo: PendingPhoto,
+        inspections: [LocalInspection],
+        issues: [LocalIssue]
+    ) {
+        let entityId = photo.entityLocalId
+
+        // Inspection form image field.
+        if photo.entityType == "inspection", let fieldId = photo.fieldId {
+            inspections.first(where: { $0.localId == entityId })?
+                .setValue(serverPath, forFieldId: fieldId)
+        }
+
+        // Issue evidence photo array.
+        if photo.entityType == "issue",
+           let issue = issues.first(where: { $0.localId == entityId }) {
+            var paths = issue.photoServerPaths
+            if !paths.contains(serverPath) { paths.append(serverPath) }
+            issue.photoServerPaths = paths
+        }
     }
 
     // ── Outbox: Inspections ───────────────────────────────────────────────
@@ -391,8 +698,31 @@ class SyncManager: ObservableObject {
             .filter { $0.status == "completed" && $0.syncStatus == "pending" }
             .sorted { $0.createdAt < $1.createdAt }
 
+        // Photos are matched by entityLocalId rather than navigated to via the
+        // relationship — see the photosReady guard below for why.
+        let allPhotos = (try? context.fetch(FetchDescriptor<PendingPhoto>())) ?? []
+
         for inspection in pending {
-            let photosReady = inspection.pendingPhotos.allSatisfy {
+            // "failed" is only reachable after maxPhotoUploadAttempts, so this
+            // means "uploaded, or genuinely unrecoverable" rather than
+            // "uploaded, or hit one network error". A still-retrying photo
+            // keeps its row "pending" and holds the inspection back — which is
+            // the point: submitting first is what blanked the field for good,
+            // since APIClient.submitInspection rewrites a surviving local://
+            // value to "" and the inspection is then marked synced forever.
+            //
+            // Matched by entityLocalId, NOT via `inspection.pendingPhotos`.
+            // That relationship declares no explicit inverse (rule 37, which
+            // LocalIssue follows and PendingPhoto does not), so it is not a
+            // trustworthy source of truth here — and an EMPTY array makes
+            // `allSatisfy` vacuously true, which silently converts this guard
+            // into no guard at all and submits the inspection with every photo
+            // still pending. processPhotoQueue and processIssueQueue already
+            // query globally; this is now consistent with them.
+            let ownPhotos = allPhotos.filter {
+                $0.entityType == "inspection" && $0.entityLocalId == inspection.localId
+            }
+            let photosReady = ownPhotos.allSatisfy {
                 $0.uploadStatus == "uploaded" || $0.uploadStatus == "failed"
             }
             guard photosReady else { continue }
@@ -443,11 +773,30 @@ class SyncManager: ObservableObject {
         // processInspectionQueue wrote the serverId back, leaving it nil even
         // when the parent inspection already synced successfully this same pass.
         let allInspections = (try? context.fetch(FetchDescriptor<LocalInspection>())) ?? []
+        // Issue photos are inserted standalone (FlagIssueView / StandaloneIssueView
+        // do not append them to any relationship), so they have to be matched by
+        // entityType + entityLocalId rather than navigated to.
+        let allPhotos      = (try? context.fetch(FetchDescriptor<PendingPhoto>()))    ?? []
 
         for issue in pending {
             let parentLocalId = issue.inspectionLocalId
             let parent        = allInspections.first(where: { $0.localId == parentLocalId })
 
+            // ── Photo readiness ───────────────────────────────────────────
+            // Wait for this issue's photos exactly as processInspectionQueue
+            // waits for an inspection's. There was no guard here at all: the
+            // issue submitted with photo_path = photoServerPaths.first (nil
+            // while uploads were still in flight or being retried), and the
+            // unconditional clear below then dropped the only local reference
+            // to the files.
+            let issuePhotos = allPhotos.filter {
+                $0.entityType == "issue" && $0.entityLocalId == issue.localId
+            }
+            let photosSettled = issuePhotos.allSatisfy {
+                $0.uploadStatus == "uploaded" || $0.uploadStatus == "failed"
+            }
+            guard photosSettled else { continue }
+
             // ── Parent inspection status guards ───────────────────────────
             if let parent {
                 switch parent.syncStatus {
@@ -486,23 +835,25 @@ class SyncManager: ObservableObject {
                 let issueId = try await APIClient.shared.submitIssue(issue, inspectionServerId: inspectionServerId)
                 issue.serverId   = issueId
                 issue.syncStatus = "synced"
-                // Photos are now represented by photoServerPaths on the server.
-                // Clear the local file paths so IssueDetailView doesn't render
-                // a duplicate "local photos" section alongside the server section.
-                issue.photoLocalPaths = []
-                try? context.save()
 
-                // If there are additional photos beyond the first (which was sent
-                // as photo_path on create), PATCH them to result_photos now.
-                // The server create endpoint only stores photo_path; result_photos
-                // must be set via a separate PATCH call.
-                let extras = Array(issue.photoServerPaths.dropFirst())
-                if !extras.isEmpty {
-                    try? await APIClient.shared.updateIssuePhotos(
-                        issueId: issueId, resultPhotos: extras
-                    )
+                // Clear the local file paths ONLY when every photo actually
+                // reached the server. Clearing unconditionally is what made a
+                // partial upload unrecoverable: the files stayed on disk but
+                // nothing referenced them any more, so neither the UI nor
+                // PhotoDiagnosticView could find them again. Keeping them costs
+                // a duplicate photo section in IssueDetailView at worst; losing
+                // them costs the evidence itself.
+                let allUploaded = issuePhotos.allSatisfy { $0.uploadStatus == "uploaded" }
+                if allUploaded {
+                    issue.photoLocalPaths = []
                 }
 
+                // No follow-up call: submitIssue() sends every photo in the
+                // create request (photo_path + result_photos), so attachment is
+                // atomic with creation — there is no window in which the issue
+                // is `synced` but its photos are not attached. See rule 85.
+                try? context.save()
+
             } catch {
                 issue.syncRetryCount  += 1
                 issue.syncErrorMessage = error.localizedDescription
@@ -554,6 +905,53 @@ class SyncManager: ObservableObject {
                 try await upsertAreas(for: apiFacility.id, facility: localFacility, context: context)
             }
 
+            // ── Prune facilities the server no longer returns ────────────
+            // /api/v1/facilities is already scoped to what this user may see,
+            // but cached rows were never removed — so a facility survived
+            // locally after the inspector's contract was unassigned, after it
+            // was deactivated, or after a different user signed in on the same
+            // iPad. Every picker derives its CONTRACT list from these rows, so
+            // one stale facility keeps a whole contract in the Start
+            // Inspection picker forever. (Templates were already pruned this
+            // way below; facilities were the gap.)
+            //
+            // Safe because we only reach here after BOTH requests succeeded —
+            // a failed sync throws before this point and deletes nothing.
+            let returnedFacilityIds = Set(uniqueFacilities.map { $0.id })
+
+            // Work that has not reached the server yet still needs its
+            // facility row: ExecuteInspectionView and MyInspectionsView resolve
+            // the name by serverId and would otherwise show "Unknown Facility"
+            // on a draft the inspector is midway through. Keep those rows but
+            // mark them unavailable so no NEW work can be started against them;
+            // they are pruned on a later sync once the work has been submitted.
+            let localInspections = try context.fetch(FetchDescriptor<LocalInspection>())
+            let localIssues      = try context.fetch(FetchDescriptor<LocalIssue>())
+            var inUseFacilityIds = Set(
+                localInspections
+                    .filter { $0.syncStatus != "synced" }
+                    .map { $0.facilityServerId }
+            )
+            // Device-created issues too: their facilityNameCache is nil until
+            // the server round-trips, and the issue DETAIL view has no cache
+            // fallback — it would read "Unknown Facility" outright.
+            inUseFacilityIds.formUnion(
+                localIssues
+                    .filter { $0.syncStatus != "synced" }
+                    .map { $0.facilityServerId }
+            )
+
+            for existing in existingFacilities {
+                guard !returnedFacilityIds.contains(existing.serverId) else { continue }
+                if inUseFacilityIds.contains(existing.serverId) {
+                    // Retained for display only. The pickers filter on
+                    // isActive, so it cannot be chosen for new work.
+                    existing.isActive = false
+                } else {
+                    context.delete(existing)   // cascades to its areas
+                }
+            }
+
             let existingTemplates = try context.fetch(FetchDescriptor<LocalTemplate>())
             let templateMap = Dictionary(
                 existingTemplates.map { ($0.serverId, $0) },
@@ -610,58 +1008,105 @@ class SyncManager: ObservableObject {
 
     private static let lastCleanupKey   = "jqc.photoCleanup.lastRunAt"
     private static let cleanupInterval: TimeInterval = 3600   // 1 hour
+    /// A file must be at least this old before cleanup will consider it.
+    ///
+    /// Resolution photos staged in IssueDetailView live purely in `@State`
+    /// until they upload — no database row references them — so for that flow
+    /// age is the only available evidence that a file is not in active use.
+    /// The floor also covers the window between writing a JPEG and saving the
+    /// record that points at it.
+    private static let cleanupMinFileAge: TimeInterval = 7 * 24 * 3600   // 7 days
 
     private func cleanupOrphanedPhotos(context: ModelContext) {
         let last = UserDefaults.standard.object(forKey: Self.lastCleanupKey) as? Date
         guard last == nil || Date().timeIntervalSince(last!) >= Self.cleanupInterval else { return }
         UserDefaults.standard.set(Date(), forKey: Self.lastCleanupKey)
 
-        // ── 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>()
+        // ── Phase 1: collect referenced FILENAMES on @MainActor ───────────────
+        // Filenames, not paths. Stored paths are absolute and embed the app
+        // container UUID, which iOS changes on every app update — so after an
+        // update every reference would fail to match its own file on disk and
+        // this sweep would delete the lot, including photos still awaiting
+        // upload. Filenames are per-save UUIDs and survive the move (PhotoStore).
+        var referencedNames = Set<String>()
+        func reference(_ storedPath: String) {
+            let name = PhotoStore.filename(of: storedPath)
+            if !name.isEmpty { referencedNames.insert(name) }
+        }
 
         // PendingPhoto — not yet uploaded
         if let pendingPhotos = try? context.fetch(FetchDescriptor<PendingPhoto>()) {
             for p in pendingPhotos where p.uploadStatus != "uploaded" {
-                referencedPaths.insert(p.localFilePath)
+                reference(p.localFilePath)
             }
         }
-        // LocalInspection — draft photos (formData values starting with "local://")
+        // LocalInspection — EVERY field still holding the local:// sentinel,
+        // whatever the inspection's status.
+        //
+        // Deliberately not limited to drafts. A sentinel surviving on a
+        // submitted inspection means that photo never reached the server, so
+        // the file on disk is the only copy in existence — it is exactly the
+        // material PhotoDiagnosticView reports as "recoverable". Restricting
+        // this to drafts would have made correcting the directory below destroy
+        // the evidence this whole fix exists to preserve.
         if let inspections = try? context.fetch(FetchDescriptor<LocalInspection>()) {
-            for insp in inspections where insp.status == "draft" {
+            for insp in inspections {
                 for val in insp.formData.values {
                     if let s = val as? String, s.hasPrefix("local://") {
-                        referencedPaths.insert(String(s.dropFirst("local://".count)))
+                        reference(String(s.dropFirst("local://".count)))
                     }
                 }
             }
         }
-        // LocalIssue — unsync'd issue photos
+        // LocalIssue — every retained local path, synced or not, for the same
+        // reason. processIssueQueue now clears these only once all photos have
+        // actually uploaded, so a path still present means evidence the server
+        // does not have.
         if let issues = try? context.fetch(FetchDescriptor<LocalIssue>()) {
-            for issue in issues where issue.syncStatus != "synced" {
-                for path in issue.photoLocalPaths { referencedPaths.insert(path) }
+            for issue in issues {
+                for path in issue.photoLocalPaths { reference(path) }
             }
         }
 
         // ── 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>)
+        // here is safe because `referencedNames` is a value type (Set<String>)
         // captured by copy — no shared mutable state crosses the boundary.
+        let minAge = Self.cleanupMinFileAge
         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")
+
+            // "JQC/Photos" — the directory every writer actually uses
+            // (FlagIssueView, StandaloneIssueView, CompactImageFieldView,
+            // ImageFieldView). This previously read "JQCPhotos", which no code
+            // path has ever written to: contentsOfDirectory failed, the guard
+            // returned, and this function silently deleted nothing for its
+            // entire life while photos accumulated indefinitely.
+            //
+            // "JQC/ResultPhotos" is deliberately NOT swept. Those files are
+            // staged in IssueDetailView's @State with no database row, so
+            // nothing here can prove one is unused; they are cleaned up by
+            // uploadAndAttachResultPhotos() and removeResultPhoto() instead.
+            let photosDir = docsDir.appendingPathComponent("JQC/Photos", isDirectory: true)
             guard let diskFiles = try? fm.contentsOfDirectory(
-                at: photosDir, includingPropertiesForKeys: nil
+                at: photosDir, includingPropertiesForKeys: [.contentModificationDateKey]
             ) else { return }
 
+            let cutoff = Date().addingTimeInterval(-minAge)
             var deletedCount = 0
             for fileURL in diskFiles {
-                if !referencedPaths.contains(fileURL.path) {
-                    try? fm.removeItem(at: fileURL)
-                    deletedCount += 1
-                }
+                // Matched by filename — see the Phase 1 comment.
+                if referencedNames.contains(fileURL.lastPathComponent) { continue }
+                // Age floor — never touch a file young enough to belong to a
+                // capture flow that has not yet written its record.
+                let modified = (try? fileURL.resourceValues(
+                    forKeys: [.contentModificationDateKey]
+                ))?.contentModificationDate
+                guard let modified, modified < cutoff else { continue }
+                try? fm.removeItem(at: fileURL)
+                deletedCount += 1
             }
             if deletedCount > 0 {
                 print("[JQC] Sync | cleanupOrphanedPhotos | removed \(deletedCount) file(s)")
@@ -978,6 +1423,109 @@ class SyncManager: ObservableObject {
         }
     }
 
+    // ── Session-scope purge ───────────────────────────────────────────────
+
+    /// Delete local data belonging to a previous session.
+    ///
+    /// Called when the `(server, user)` pair the database is scoped to changes
+    /// — a different inspector signing in on this iPad, or a server switch. See
+    /// `SessionScope` for why that combination is the boundary, and rule 88.
+    ///
+    /// - Parameters:
+    ///   - keepingUserId: the incoming user. Their own unsent inspections are
+    ///     preserved when `sameServer` is true; pass nil to keep nothing.
+    ///   - sameServer: false when the server changed, in which case NOTHING can
+    ///     be kept — every `serverId`, `facilityServerId` and `templateServerId`
+    ///     names a row in a different database.
+    ///
+    /// Deliberately NOT called on a plain logout: the common case is the same
+    /// inspector signing back into the same server, and wiping the cache there
+    /// would leave them with an empty app until a full sync succeeds, breaking
+    /// offline use. That was the correct instinct in the original code — the
+    /// bug was only that nothing ever checked whether the next login was
+    /// actually the same person.
+    func purgeSessionScopedData(keepingUserId: Int?, sameServer: Bool) {
+        guard let context = modelContext else { return }
+
+        // ── Reference caches ──────────────────────────────────────────────
+        // Pure server-scoped copies with no local authorship. Cheap to re-pull,
+        // so there is never a reason to keep one across an identity change.
+        deleteAll(LocalFacility.self,            from: context)   // cascades areas
+        deleteAll(LocalArea.self,                from: context)
+        deleteAll(LocalTemplate.self,            from: context)
+        deleteAll(LocalScheduledInspection.self, from: context)
+        deleteAll(LocalFollowUpRequest.self,     from: context)
+        // Notifications are addressed to one user — the most personal thing in
+        // the store, and the clearest thing another inspector must never see.
+        deleteAll(LocalNotification.self,        from: context)
+
+        // ── Issues ────────────────────────────────────────────────────────
+        // All of them, unconditionally. LocalIssue carries no author field, so
+        // an unsent one cannot be attributed — and submitting the previous
+        // inspector's issue under the new inspector's credentials would put a
+        // false name on a QC record. Server-pulled and already-synced rows are
+        // the previous user's assignments and stale serverIds respectively.
+        deleteAll(LocalIssue.self, from: context)
+
+        // ── Inspections ───────────────────────────────────────────────────
+        // These DO carry an author (`inspectorUserId`), so the incoming user's
+        // own unsent work can be handed back to them intact — but only when the
+        // server is unchanged, since otherwise its facility/template ids point
+        // into the wrong database.
+        let inspections = (try? context.fetch(FetchDescriptor<LocalInspection>())) ?? []
+        var keptInspectionIds = Set<String>()
+        for insp in inspections {
+            let ownedByIncomingUser = keepingUserId.map { $0 == insp.inspectorUserId } ?? false
+            if sameServer && ownedByIncomingUser {
+                keptInspectionIds.insert(insp.localId)
+            } else {
+                // Remove the JPEGs too — cleanupOrphanedPhotos would otherwise
+                // wait out its 7-day age floor holding another user's evidence.
+                for photo in insp.pendingPhotos {
+                    PhotoStore.remove(at: photo.localFilePath)
+                }
+                context.delete(insp)   // cascades pendingPhotos + localIssues
+            }
+        }
+
+        // ── Photos orphaned by the above ──────────────────────────────────
+        // Issue photos are inserted standalone rather than through a
+        // relationship, so no cascade reaches them — and every LocalIssue has
+        // just been deleted, which is why only kept inspections can retain one.
+        let photos = (try? context.fetch(FetchDescriptor<PendingPhoto>())) ?? []
+        for photo in photos {
+            let stillOwned = photo.entityType == "inspection"
+                          && keptInspectionIds.contains(photo.entityLocalId)
+            if !stillOwned {
+                PhotoStore.remove(at: photo.localFilePath)
+                context.delete(photo)
+            }
+        }
+
+        try? context.save()
+
+        // ── In-memory state from the old session ──────────────────────────
+        // The poll TASK is left running; only its data is dropped. Stopping it
+        // here would leave polling dead until the next connectivity change or
+        // foreground, because nothing on the login path restarts it.
+        dashboardStats          = nil
+        lastNotificationFetch   = nil
+        unreadNotificationCount = 0
+        syncError               = nil
+        lastSyncAt              = nil
+        updatePendingCount(context: context)
+
+        print("[JQC] Sync | purgeSessionScopedData | kept \(keptInspectionIds.count) "
+              + "inspection(s) for user \(keepingUserId.map(String.init) ?? "-")")
+    }
+
+    /// Fetch-all + delete. No #Predicate (CLAUDE.md rule 3), and `try?`
+    /// parenthesised before `??` (rule 25).
+    private func deleteAll<T: PersistentModel>(_ type: T.Type, from context: ModelContext) {
+        let rows = (try? context.fetch(FetchDescriptor<T>())) ?? []
+        for row in rows { context.delete(row) }
+    }
+
     // ── Dashboard Stats ───────────────────────────────────────────────────
     // Best-effort fetch — a network failure silently leaves dashboardStats nil
     // so the UI falls back to a placeholder card.  Never blocks the sync pipeline.

JanitorialQC/Utils/Constants.swift

@@ -59,6 +59,59 @@ nonisolated enum ServerConfig {
     }
 }
 
+// MARK: - Session scope
+
+/// The (server, user) pair the local database is currently scoped to.
+///
+/// Almost every local id is meaningful only within one such pair. `serverId`
+/// values differ between jqc and jqc1; facility/template ids and inspector
+/// facility scope differ per user. So when either half changes, the cached data
+/// is not merely stale — it is *wrong*, and silently belongs to someone else.
+///
+/// Two concrete failures this exists to close:
+///   • A different inspector signing in on the same iPad inherited the previous
+///     one's issues. `pullAssignedIssues`' reconciliation only deletes rows with
+///     `inspectionLocalId == ""`, so device-authored synced issues survived
+///     indefinitely and were simply visible to whoever logged in next.
+///   • A server switch cleared `LocalIssue` alone, leaving `LocalInspection`
+///     rows carrying facility/template ids from the other server, ready to be
+///     submitted against it.
+///
+/// Deliberately in UserDefaults, NOT the Keychain: `KeychainHelper.clearAll()`
+/// runs on logout, and this marker has to OUTLIVE a logout to be able to notice
+/// that the next login is a different person. It is not a secret.
+nonisolated enum SessionScope {
+
+    struct Scope: Equatable, Sendable {
+        let server: String
+        let userId: Int
+    }
+
+    private static let userKey   = "com.jqc.sessionScope.userId"
+    private static let serverKey = "com.jqc.sessionScope.server"
+
+    /// The recorded scope, or nil when none has ever been written.
+    static var stored: Scope? {
+        let d = UserDefaults.standard
+        guard let server = d.string(forKey: serverKey),
+              d.object(forKey: userKey) != nil
+        else { return nil }
+        return Scope(server: server, userId: d.integer(forKey: userKey))
+    }
+
+    static func record(userId: Int, server: String = ServerConfig.current) {
+        UserDefaults.standard.set(userId, forKey: userKey)
+        UserDefaults.standard.set(server, forKey: serverKey)
+    }
+
+    /// Forget the scope entirely — used on a server switch, where the next
+    /// login is guaranteed to be against a different data set.
+    static func clear() {
+        UserDefaults.standard.removeObject(forKey: userKey)
+        UserDefaults.standard.removeObject(forKey: serverKey)
+    }
+}
+
 // MARK: - App-wide constants
 
 // Explicitly not @MainActor — these constants must be readable from
@@ -76,4 +129,50 @@ nonisolated enum Constants {
     }
 
     static let tokenRefreshBufferMinutes: Double = 5
+
+    // MARK: - Roles
+    //
+    // The server's role strings, and the ONE definition of which of them the
+    // app treats as an inspector. This mirrors `User.INSPECTOR_ROLES` /
+    // `User.is_inspector` in the Flask app (see server rule 87).
+    //
+    // Why this exists: `external_inspector` (displayed as "Customer Inspector"
+    // — an inspector employed by the customer) has exactly the same powers as
+    // our own `inspector`, and the API scopes it identically. The views here
+    // were hand-written as `role == "admin" || role == "director" || role ==
+    // "inspector"`, so every one of them silently locked customer inspectors
+    // out of actions the SERVER was perfectly willing to accept — Update
+    // Status, Handled By, Start Follow-up. The failure is invisible: no error,
+    // the control simply isn't drawn.
+    //
+    // Add a role in ONE place here; never re-write the literals in a view.
+    nonisolated enum Roles {
+        static let admin             = "admin"
+        static let director          = "director"
+        static let projectManager    = "project_manager"
+        static let auditor           = "auditor"
+        static let inspector         = "inspector"
+        /// "Customer Inspector" — employed by the customer, same powers as
+        /// `inspector`, scoped to their assigned contracts.
+        static let externalInspector = "external_inspector"
+
+        /// Both inspector roles. Test membership of this, never `== inspector`.
+        static let inspectorRoles: Set<String> = [inspector, externalInspector]
+
+        /// May change an issue's status / handler, and start a follow-up.
+        /// Matches what the API actually accepts for these actions; the server
+        /// remains the authority and additionally enforces facility scope.
+        ///
+        /// Built from `inspectorRoles` (already a Set) rather than from an
+        /// array literal: `[a, b, c].union(...)` does not compile, because the
+        /// literal is typed as Array before `.union` is looked up, and Array
+        /// has no such member — the annotation on the left does not reach back
+        /// into the receiver.
+        static let issueActors: Set<String> =
+            inspectorRoles.union([admin, director, projectManager])
+
+        static func isInspector(_ role: String) -> Bool {
+            inspectorRoles.contains(role)
+        }
+    }
 }

JanitorialQC/Utils/InspectionPDFGenerator.swift

@@ -184,7 +184,8 @@ enum InspectionPDFGenerator {
                 group.addTask {
                     if val.hasPrefix("local://") {
                         let path = String(val.dropFirst("local://".count))
-                        guard let raw = UIImage(contentsOfFile: path) else { return (fid, nil) }
+                        guard let live = PhotoStore.resolve(path),
+                              let raw = UIImage(contentsOfFile: live) else { return (fid, nil) }
                         return (fid, compress(raw))
                     } else if val.hasPrefix("uploads/") {
                         guard let url = URL(string: "\(ServerConfig.current)/static/\(val)")

JanitorialQC/Utils/IssuePDFGenerator.swift

@@ -157,7 +157,7 @@ enum IssuePDFGenerator {
                 return results
             }
         } else if !localPaths.isEmpty {
-            raw = localPaths.compactMap { UIImage(contentsOfFile: $0) }
+            raw = localPaths.compactMap { PhotoStore.resolve($0).flatMap(UIImage.init(contentsOfFile:)) }
         }
 
         return raw.compactMap { compress($0) }

JanitorialQC/Utils/PhotoStore.swift

@@ -0,0 +1,91 @@
+// Utils/PhotoStore.swift
+// ----------------------
+// Resolves a stored photo path against the CURRENT app container.
+//
+// ── The bug this exists to fix ───────────────────────────────────────────────
+// Every photo path in the database is ABSOLUTE and embeds the app-container
+// UUID:
+//
+//   /var/mobile/Containers/Data/Application/<CONTAINER-UUID>/Documents/JQC/Photos/<file>.jpg
+//
+// iOS assigns a NEW container UUID on every app update, reinstall and restore.
+// The Documents directory survives — the files are all still there — but every
+// stored path is instantly dead.
+//
+// Nothing accounted for that. `uploadPhoto` does
+// `FileManager.default.contents(atPath:)`, which returns nil, so the upload
+// throws "Could not read photo" and can NEVER succeed no matter how often it is
+// retried: the path names a container that no longer exists. Any photo still
+// awaiting upload when the app updates is therefore stranded permanently, and
+// its inspection is submitted with the field blank.
+//
+// That is what happened to inspection #887 (9 photos, Aug 2026): the diagnostic
+// reported all nine as `upload: pending` with 0 failed rows and
+// "Stored path stale (container changed) — file found by name, recoverable".
+//
+// ── Why basename lookup is safe ──────────────────────────────────────────────
+// Filenames are `UUID().uuidString + ".jpg"`, generated per save at every call
+// site, so a basename identifies a file unambiguously. This is the same
+// resolution PhotoDiagnosticView already performs to report recoverability —
+// it just was not wired into the code paths that actually read the files.
+
+import Foundation
+
+nonisolated enum PhotoStore {
+
+    /// Sub-directories of Documents/ the app has ever written photos to.
+    /// `JQCPhotos` is not written by any current code path but is checked so a
+    /// file left by an older build is still found.
+    private static let subdirectories = ["JQC/Photos", "JQC/ResultPhotos", "JQCPhotos"]
+
+    /// Documents/ in the CURRENT container. Recomputed per call — caching it
+    /// across an app update would reintroduce the very staleness this fixes.
+    private static var documentsDirectory: URL? {
+        FileManager.default.urls(for: .documentDirectory, in: .userDomainMask).first
+    }
+
+    /// Absolute URL for a photo directory in the current container.
+    static func directory(_ subdirectory: String = "JQC/Photos") -> URL? {
+        documentsDirectory?.appendingPathComponent(subdirectory, isDirectory: true)
+    }
+
+    /// The live absolute path for a stored photo path, or nil if the file is
+    /// genuinely gone.
+    ///
+    /// Returns `storedPath` unchanged when it still resolves (the common case,
+    /// costing one `fileExists` check). Otherwise re-resolves by filename under
+    /// the current container.
+    static func resolve(_ storedPath: String) -> String? {
+        guard !storedPath.isEmpty else { return nil }
+        let fm = FileManager.default
+        if fm.fileExists(atPath: storedPath) { return storedPath }
+
+        let name = URL(fileURLWithPath: storedPath).lastPathComponent
+        guard !name.isEmpty else { return nil }
+        for sub in subdirectories {
+            guard let candidate = directory(sub)?.appendingPathComponent(name) else { continue }
+            if fm.fileExists(atPath: candidate.path) { return candidate.path }
+        }
+        return nil
+    }
+
+    /// Read a photo's bytes, healing a stale container path first.
+    static func contents(at storedPath: String) -> Data? {
+        guard let live = resolve(storedPath) else { return nil }
+        return FileManager.default.contents(atPath: live)
+    }
+
+    /// Delete a photo, whichever container its path was written in.
+    @discardableResult
+    static func remove(at storedPath: String) -> Bool {
+        guard let live = resolve(storedPath) else { return false }
+        return (try? FileManager.default.removeItem(atPath: live)) != nil
+    }
+
+    /// Filename component, which is the only stable part of a stored path.
+    /// Use this — never the full path — to compare a database reference against
+    /// a file on disk (see `SyncManager.cleanupOrphanedPhotos`).
+    static func filename(of storedPath: String) -> String {
+        URL(fileURLWithPath: storedPath).lastPathComponent
+    }
+}

JanitorialQC/Views/Dashboard/DashboardView.swift

@@ -130,7 +130,9 @@ struct DashboardView: View {
                 ForEach(sidebarTabs, id: \.self) { tab in
                     Button {
                         selectTab(tab)
-                        if tab == .notifications { sync.markNotificationsViewed() }
+                        // No longer zeroes the badge on tap: opening the inbox
+                        // is not reading it. The count now tracks genuinely
+                        // unread rows and clears as they are read (rule 92).
                     } label: {
                         sidebarRowLabel(tab, tinted: selectedTab == tab)
                     }

JanitorialQC/Views/Dashboard/ExecuteInspectionView.swift

@@ -29,10 +29,27 @@ struct ExecuteInspectionView: View {
     /// Work is preserved either way — .onDisappear calls saveDraft().
     var isModallyPresented: Bool = false
 
+    /// What to do when the inspection has been submitted, instead of the
+    /// default `dismiss()`.
+    ///
+    /// `StartInspectionView` PUSHES this view onto the NavigationStack inside
+    /// its own `.fullScreenCover`, so a plain `dismiss()` only pops — landing
+    /// the inspector back on the "New Inspection" form they just started from,
+    /// with Cancel as the only way out. It passes its own dismiss here so the
+    /// whole cover closes and they return to the dashboard.
+    ///
+    /// Left nil everywhere else, where popping IS correct: the My Inspections
+    /// row pushes onto the list's stack, and the dashboard's Resume banner
+    /// presents this view as the cover root.
+    var onFinished: (() -> Void)? = nil
+
     @State private var formValues:      [String: String] = [:]
     @State private var showFlagIssue       = false
     @State private var showSubmitAlert     = false
     @State private var showNoGPSAlert      = false
+    /// Set when Submit is tapped with no GPS fix; consumed by
+    /// `onChange(of: showSubmitAlert)` once the confirm alert has dismissed.
+    @State private var pendingNoGPSPrompt   = false
     @State private var showValidationAlert = false
     @State private var missingFields: [String] = []
     @State private var isSaving            = false
@@ -101,7 +118,13 @@ struct ExecuteInspectionView: View {
 
             switch ftype {
             case "rating":
-                if let v = Int(val), v > 0 { earned += v; total += field["max"] as? Int ?? 5 }
+                // Denominator is a FLAT 5, never the field's `max`.
+                // `_compute_score_from_form()` in the Flask app (routes/
+                // inspections.py) hardcodes `total += 5`, and LocalInspection
+                // .computeScore() mirrors it — this was the only site reading
+                // `max`, so a template with max != 5 showed one percentage in
+                // the toolbar and submitted a different one.
+                if let v = Int(val), v > 0 { earned += v; total += 5 }
             case "checkbox":
                 total += 1; if val == "true" { earned += 1 }
             case "radio":
@@ -194,7 +217,16 @@ struct ExecuteInspectionView: View {
                 if locationManager.lastLocation == nil {
                     // No GPS fix yet — warn before proceeding rather than
                     // silently submitting without a location.
-                    showNoGPSAlert = true
+                    //
+                    // Deferred, NOT set here: raising a second alert from
+                    // inside the first one's action, with both attached to the
+                    // same view, is dropped by SwiftUI — the confirm alert is
+                    // still tearing down, so the new presentation is discarded.
+                    // The visible effect was that tapping Submit without a fix
+                    // did nothing at all: no warning, no submission. Handing it
+                    // to onChange(of: showSubmitAlert) below presents it only
+                    // once the first alert has actually gone.
+                    pendingNoGPSPrompt = true
                 } else {
                     Task { await submitInspection() }
                 }
@@ -223,7 +255,26 @@ struct ExecuteInspectionView: View {
         .onChange(of: showSubmitAlert) { _, showing in
             // Begin acquiring a GPS fix the moment the confirm dialog appears
             // so a location is likely ready by the time the inspector taps Submit.
-            if showing { locationManager.requestLocation() }
+            if showing {
+                locationManager.requestLocation()
+                return
+            }
+            // Confirm alert has closed. If Submit was tapped without a fix,
+            // raise the warning now that the presentation slot is free.
+            guard pendingNoGPSPrompt else { return }
+            pendingNoGPSPrompt = false
+            Task {
+                // One runloop hop. `showing == false` means the binding flipped,
+                // not that the dismissal animation has finished, and presenting
+                // into the tail of that animation is unreliable.
+                try? await Task.sleep(for: .milliseconds(350))
+                // Re-check: the fix may have landed while the dialog was up.
+                if locationManager.lastLocation == nil {
+                    showNoGPSAlert = true
+                } else {
+                    await submitInspection()
+                }
+            }
         }
         // Result overlay
         .overlay(alignment: .top) {
@@ -678,9 +729,9 @@ struct ExecuteInspectionView: View {
             Task { await sync.triggerSync() }
         }
 
-        // Wait 2.5 seconds so inspector reads the result, then dismiss
+        // Wait 2.5 seconds so inspector reads the result, then leave.
         try? await Task.sleep(for: .seconds(2.5))
-        dismiss()
+        if let onFinished { onFinished() } else { dismiss() }
     }
 
     /// Find the parent LocalInspection and clear its followUpRequired flag.
@@ -1409,15 +1460,83 @@ struct CellDatePicker: View {
 
     private var dateBinding: Binding<Date> {
         Binding(
-            get: { ISO8601DateFormatter().date(from: value) ?? Date() },
-            set: { value = ISO8601DateFormatter().string(from: $0) }
+            get: { FormDateFormat.date(from: value) ?? Date() },
+            set: { value = FormDateFormat.string(from: $0) }
         )
     }
 
     var body: some View {
-        DatePicker("", selection: dateBinding, displayedComponents: .date)
-            .labelsHidden()
+        // An empty value must LOOK empty until the inspector acts.
+        //
+        // The old version bound the DatePicker straight to the value: when it
+        // was empty the picker still displayed TODAY, so the field looked
+        // answered — but the setter only fires on a CHANGE, so selecting the
+        // already-displayed date wrote nothing and missingRequiredFields()
+        // reported the field missing with a date plainly visible on screen.
+        // The inspector had to pick a different day and navigate back. This
+        // explicit step makes unanswered look unanswered and makes today
+        // selectable in one tap.
+        if value.isEmpty {
+            Button {
+                value = FormDateFormat.string(from: Date())
+            } label: {
+                HStack(spacing: 4) {
+                    Image(systemName: "calendar").font(.system(size: 11))
+                    Text("Set date").font(.system(size: 12))
+                }
+                .foregroundStyle(Color(.placeholderText))
+                .padding(.horizontal, 6).padding(.vertical, 3)
+                .frame(maxWidth: .infinity, alignment: .leading)
+                .background(Color(.systemBackground))
+                .clipShape(RoundedRectangle(cornerRadius: 5))
+                .overlay(RoundedRectangle(cornerRadius: 5)
+                    .stroke(Color(.systemGray4), lineWidth: 1))
+            }
+            .buttonStyle(.plain)
+        } else {
+            HStack(spacing: 2) {
+                DatePicker("", selection: dateBinding, displayedComponents: .date)
+                    .labelsHidden()
+                Button { value = "" } label: {          // back to unanswered
+                    Image(systemName: "xmark.circle.fill")
+                        .font(.system(size: 12))
+                        .foregroundStyle(.secondary)
+                }
+                .buttonStyle(.plain)
+            }
             .frame(maxWidth: .infinity, alignment: .leading)
+        }
+    }
+}
+
+// MARK: - FormDateFormat
+// Wire format for `date` form fields: "yyyy-MM-dd", matching the web's
+// <input type="date"> (templates/inspections/execute.html), so a value entered
+// on the iPad and one entered in a browser are the same string in form_data.
+//
+// Both date widgets previously used ISO8601DateFormatter, which round-tripped a
+// full timestamp ("2026-08-18T14:30:00Z") into a field that the web renders and
+// the PDF prints verbatim.
+//
+// UTC + POSIX locale so the day cannot shift with device timezone or calendar.
+// nonisolated for the same reason as PhotoCaptureFormat (rule 82).
+nonisolated enum FormDateFormat {
+    static let formatter: DateFormatter = {
+        let f = DateFormatter()
+        f.locale     = Locale(identifier: "en_US_POSIX")
+        f.timeZone   = TimeZone(identifier: "UTC")
+        f.dateFormat = "yyyy-MM-dd"
+        return f
+    }()
+
+    static func string(from date: Date) -> String { formatter.string(from: date) }
+
+    /// Parses the canonical form, and tolerates a leading `yyyy-MM-dd` inside a
+    /// longer timestamp so values written by earlier builds still display.
+    static func date(from value: String) -> Date? {
+        if let d = formatter.date(from: value) { return d }
+        guard value.count >= 10 else { return nil }
+        return formatter.date(from: String(value.prefix(10)))
     }
 }
 

JanitorialQC/Views/Dashboard/FollowUpRequestsView.swift

@@ -111,6 +111,22 @@ struct FollowUpRow: View {
                     }
                 }
 
+                // Assigned to somebody other than whoever ran the original
+                // inspection (phase53). The endpoint only returns follow-ups
+                // this user owns, so seeing this badge means "you were given
+                // this one" — worth calling out, because it is NOT the usual
+                // case of re-inspecting your own work.
+                if request.assignedToName != nil {
+                    HStack(spacing: 4) {
+                        Image(systemName: "person.crop.circle.badge.checkmark")
+                            .font(.caption2)
+                        Text("Assigned to you")
+                            .font(.caption2.weight(.semibold))
+                    }
+                    .foregroundStyle(.orange)
+                    .padding(.top, 1)
+                }
+
                 // Note preview — so the inspector can see there is something to
                 // read before committing to the tap. Truncated to one line; the
                 // full text is shown on the start screen. Mirrors the

JanitorialQC/Views/Dashboard/FormFieldView.swift

@@ -183,19 +183,45 @@ struct DateFieldView: View {
 
     private var dateBinding: Binding<Date> {
         Binding(
-            get: {
-                ISO8601DateFormatter().date(from: value) ?? Date()
-            },
-            set: {
-                value = ISO8601DateFormatter().string(from: $0)
-            }
+            get: { FormDateFormat.date(from: value) ?? Date() },
+            set: { value = FormDateFormat.string(from: $0) }
         )
     }
 
     var body: some View {
-        DatePicker("", selection: dateBinding, displayedComponents: .date)
-            .labelsHidden()
+        // Same two problems as CellDatePicker, same fix — see the comments
+        // there. Empty must look empty, and the stored format is "yyyy-MM-dd"
+        // to match the web's <input type="date">, not an ISO 8601 timestamp.
+        if value.isEmpty {
+            Button {
+                value = FormDateFormat.string(from: Date())
+            } label: {
+                HStack(spacing: 6) {
+                    Image(systemName: "calendar")
+                    Text("Set date")
+                }
+                .font(.callout)
+                .foregroundStyle(Color(.placeholderText))
+                .padding(10)
+                .frame(maxWidth: .infinity, alignment: .leading)
+                .background(Color(.systemBackground))
+                .clipShape(RoundedRectangle(cornerRadius: 8))
+                .overlay(RoundedRectangle(cornerRadius: 8)
+                    .stroke(Color(.systemGray4), lineWidth: 1))
+            }
+            .buttonStyle(.plain)
+        } else {
+            HStack(spacing: 6) {
+                DatePicker("", selection: dateBinding, displayedComponents: .date)
+                    .labelsHidden()
+                Button { value = "" } label: {
+                    Image(systemName: "xmark.circle.fill")
+                        .foregroundStyle(.secondary)
+                }
+                .buttonStyle(.plain)
+            }
             .frame(maxWidth: .infinity, alignment: .leading)
+        }
     }
 }
 

JanitorialQC/Views/Dashboard/IssuesView.swift

@@ -290,10 +290,15 @@ struct IssueDetailView: View {
 
     /// Inspector can update status only if the issue has synced (has a serverId)
     /// and we are online. Admins/directors can always update when online.
+    ///
+    /// Uses `Constants.Roles.issueActors` rather than a hand-written list: the
+    /// old `role == "inspector"` check silently excluded Customer Inspectors
+    /// (`external_inspector`), who the API has always accepted here — the
+    /// Update Status control simply never appeared for them, with no error to
+    /// explain why.
     private var canUpdateStatus: Bool {
         guard sync.isOnline, issue.serverId != nil else { return false }
-        let role = AuthManager.shared.currentUserRole
-        return role == "admin" || role == "director" || role == "inspector"
+        return Constants.Roles.issueActors.contains(AuthManager.shared.currentUserRole)
     }
 
     private let allStatuses: [(value: String, label: String, color: Color)] = [
@@ -308,9 +313,7 @@ struct IssueDetailView: View {
     /// admin/director/PM); the server enforces facility scope for inspectors.
     private var canEditHandler: Bool {
         guard sync.isOnline, issue.serverId != nil else { return false }
-        let role = AuthManager.shared.currentUserRole
-        return role == "admin" || role == "director"
-            || role == "inspector" || role == "project_manager"
+        return Constants.Roles.issueActors.contains(AuthManager.shared.currentUserRole)
     }
 
     private func handlerTypeLabel(_ type: String) -> String {
@@ -564,7 +567,7 @@ struct IssueDetailView: View {
                 if !issue.photoLocalPaths.isEmpty {
                     Section("Photos (\(issue.photoLocalPaths.count))") {
                         ForEach(issue.photoLocalPaths, id: \.self) { path in
-                            if let img = UIImage(contentsOfFile: path) {
+                            if let live = PhotoStore.resolve(path), let img = UIImage(contentsOfFile: live) {
                                 Image(uiImage: img)
                                     .resizable()
                                     .scaledToFit()
@@ -1093,11 +1096,19 @@ struct StandaloneIssueView: View {
     }
     private var remainingSlots: Int { maxPhotos - photos.count }
 
+    /// Facilities this user may file a new issue against.
+    /// Excludes rows SyncManager retained purely so an unsynced draft could
+    /// still show its facility name — see StartInspectionView for the full
+    /// explanation. Out-of-scope facilities must not be offered for new work.
+    private var availableFacilities: [LocalFacility] {
+        facilities.filter { $0.isActive }
+    }
+
     /// Unique contracts derived from cached facilities, sorted by name.
     private var contracts: [(id: Int, name: String)] {
         var seen = Set<Int>()
         var result: [(id: Int, name: String)] = []
-        for f in facilities {
+        for f in availableFacilities {
             if seen.insert(f.projectId).inserted {
                 result.append((id: f.projectId, name: f.projectName))
             }
@@ -1110,7 +1121,7 @@ struct StandaloneIssueView: View {
     private var filteredFacilities: [LocalFacility] {
         guard let pid = selectedProjectId else { return [] }
         var seen = Set<Int>()
-        return facilities
+        return availableFacilities
             .filter { $0.projectId == pid }
             .filter { seen.insert($0.serverId).inserted }
     }

JanitorialQC/Views/Dashboard/MyInspectionsView.swift

@@ -173,13 +173,13 @@ struct MyInspectionsView: View {
     private func deleteDraft(_ inspection: LocalInspection) {
         // Delete associated pending photos from disk and SwiftData
         for photo in inspection.pendingPhotos {
-            try? FileManager.default.removeItem(atPath: photo.localFilePath)
+            PhotoStore.remove(at: photo.localFilePath)
             context.delete(photo)
         }
         // Delete associated local issues
         for issue in inspection.localIssues {
             for path in issue.photoLocalPaths {
-                try? FileManager.default.removeItem(atPath: path)
+                PhotoStore.remove(at: path)
             }
             context.delete(issue)
         }

JanitorialQC/Views/Dashboard/NotificationsView.swift

@@ -1,90 +1,355 @@
 // Views/Dashboard/NotificationsView.swift
+// ---------------------------------------
+// In-app notification inbox, backed by LocalNotification (see that file for why
+// the inbox is stored locally rather than re-read from the server each time).
+//
+// Every row used to look identical because the poll endpoint only returns
+// UNREAD notifications — the list was, by construction, all-unread with nothing
+// to distinguish. Now read state is real: unread rows carry a dot and a bold
+// title, read rows are muted, and reading is an explicit act (tap a row, or
+// Mark All Read) rather than a side effect of opening the screen.
+
 import SwiftUI
 import SwiftData
 import MessageUI
 
-// MARK: - Notifications Inbox
-// Shows the most recent notifications fetched during polling.
-// Notifications are already marked read on the server by pollNotifications().
+// MARK: - Inbox
 
 struct NotificationsView: View {
 
     @EnvironmentObject private var sync: SyncManager
+    @Environment(\.modelContext) private var context
+
+    // Sorted newest-first. Filtered in Swift, not in the @Query predicate
+    // (CLAUDE.md rule 3).
+    @Query(sort: \LocalNotification.createdAt, order: .reverse)
+    private var allNotifications: [LocalNotification]
+
+    enum Filter: String, CaseIterable, Identifiable {
+        case all, unread, read
+        var id: String { rawValue }
+        var label: String {
+            switch self {
+            case .all:    return "All"
+            case .unread: return "Unread"
+            case .read:   return "Read"
+            }
+        }
+    }
+
+    @State private var filter: Filter = .all
+    @State private var isMarkingAll = false
+
+    private var unreadCount: Int { allNotifications.filter { !$0.isRead }.count }
+
+    private var visible: [LocalNotification] {
+        switch filter {
+        case .all:    return allNotifications
+        case .unread: return allNotifications.filter { !$0.isRead }
+        case .read:   return allNotifications.filter { $0.isRead }
+        }
+    }
 
     var body: some View {
         Group {
-            if sync.recentNotifications.isEmpty {
-                if !sync.isOnline {
-                    ContentUnavailableView(
-                        "Offline",
-                        systemImage: "wifi.slash",
-                        description: Text("Notifications are delivered when you go online.")
-                    )
-                } else {
-                    ContentUnavailableView(
-                        "No Notifications",
-                        systemImage: "bell.slash",
-                        description: Text("You\'re all caught up.")
-                    )
-                }
+            if allNotifications.isEmpty {
+                emptyState
             } else {
-                List(sync.recentNotifications) { notif in
-                    VStack(alignment: .leading, spacing: 6) {
-                        HStack(alignment: .top) {
-                            Image(systemName: iconName(for: notif.eventType))
-                                .foregroundStyle(iconColor(for: notif.eventType))
-                                .frame(width: 24)
-                            VStack(alignment: .leading, spacing: 2) {
-                                Text(notif.title)
-                                    .font(.callout.bold())
-                                    .lineLimit(2)
-                                Text(notif.body)
-                                    .font(.caption)
-                                    .foregroundStyle(.secondary)
-                                    .lineLimit(3)
-                            }
-                        }
-                        if let date = SyncManager.isoFormatter.date(from: notif.createdAt) {
-                            Text(date.formatted(.relative(presentation: .named)))
-                                .font(.caption2)
-                                .foregroundStyle(.tertiary)
+                VStack(spacing: 0) {
+                    // OUTSIDE the List, so it survives a filter that matches
+                    // nothing. As a list row it vanished with the rows —
+                    // selecting "Read" with nothing read left no way back.
+                    Picker("Show", selection: $filter) {
+                        ForEach(Filter.allCases) { f in
+                            Text(f.label).tag(f)
+                        }
+                    }
+                    .pickerStyle(.segmented)
+                    .padding(.horizontal, 16)
+                    .padding(.vertical, 8)
+
+                    if visible.isEmpty {
+                        ContentUnavailableView(
+                            filter == .unread ? "All Caught Up" : "Nothing Read Yet",
+                            systemImage: filter == .unread ? "checkmark.circle" : "envelope.open",
+                            description: Text(filter == .unread
+                                              ? "You have no unread notifications."
+                                              : "Notifications you open will appear here.")
+                        )
+                        Spacer(minLength: 0)
+                    } else {
+                        List {
+                            ForEach(visible) { notif in
+                                NavigationLink(value: notif) {
+                                    NotificationRow(notification: notif)
+                                }
+                                .swipeActions(edge: .leading, allowsFullSwipe: true) {
+                                    if !notif.isRead {
+                                        Button {
+                                            Task { await sync.markNotificationRead(notif) }
+                                        } label: {
+                                            Label("Read", systemImage: "envelope.open")
+                                        }
+                                        .tint(.blue)
+                                    }
+                                }
+                            }
                         }
                     }
-                    .padding(.vertical, 4)
                 }
             }
         }
-        .navigationTitle("Notifications")
+        // On the ALWAYS-PRESENT Group, never inside the List — same hazard as
+        // rule 68. Opening a notification marks it read, which removes it from
+        // the "Unread" filter; if that was the last row, the List is replaced by
+        // an empty state and a destination declared inside it would be torn
+        // down, popping the detail view out from under the inspector as they
+        // read it.
+        .navigationDestination(for: LocalNotification.self) { notif in
+            NotificationDetailView(notification: notif)
+        }
+        .navigationTitle(unreadCount > 0 ? "Notifications (\(unreadCount))" : "Notifications")
         .navigationBarTitleDisplayMode(.large)
-        .onAppear {
-            sync.markNotificationsViewed()
+        .toolbar {
+            if unreadCount > 0 {
+                ToolbarItem(placement: .primaryAction) {
+                    Button {
+                        Task {
+                            isMarkingAll = true
+                            await sync.markAllNotificationsRead()
+                            isMarkingAll = false
+                        }
+                    } label: {
+                        if isMarkingAll {
+                            ProgressView()
+                        } else {
+                            Label("Mark All Read", systemImage: "envelope.open")
+                        }
+                    }
+                    .labelStyle(.titleAndIcon)   // rule 72
+                    .disabled(isMarkingAll)
+                }
+            }
         }
         .refreshable {
             await sync.pollNotifications()
-            sync.markNotificationsViewed()
         }
+        // Keep the sidebar badge honest if read state changed elsewhere (a
+        // swipe, the detail view, or a push that landed while this was open).
+        .onAppear { sync.refreshUnreadNotificationCount() }
     }
 
-    private func iconName(for eventType: String?) -> String {
-        switch eventType {
-        case "inspection_completed": return "checkmark.circle.fill"
-        case "issue_flagged":        return "exclamationmark.triangle.fill"
-        case "issue_resolved":       return "checkmark.seal.fill"
-        case "sla_alert":            return "clock.badge.exclamationmark"
-        case "follow_up_required":   return "exclamationmark.arrow.circlepath"
-        default:                     return "bell.fill"
-        }
-    }
-
-    private func iconColor(for eventType: String?) -> Color {
-        switch eventType {
-        case "inspection_completed": return .green
-        case "issue_flagged":        return .orange
-        case "issue_resolved":       return .green
-        case "sla_alert":            return .red
-        case "follow_up_required":   return .orange
-        default:                     return .blue
+    @ViewBuilder
+    private var emptyState: some View {
+        if !sync.isOnline {
+            ContentUnavailableView(
+                "Offline",
+                systemImage: "wifi.slash",
+                description: Text("Notifications are delivered when you go online.")
+            )
+        } else {
+            ContentUnavailableView(
+                "No Notifications",
+                systemImage: "bell.slash",
+                description: Text("You're all caught up.")
+            )
         }
     }
 }
 
+// MARK: - Row
+
+struct NotificationRow: View {
+    let notification: LocalNotification
+
+    var body: some View {
+        HStack(alignment: .top, spacing: 10) {
+            // Unread marker. A filled dot rather than colour alone, so the
+            // distinction survives greyscale and colour-blind vision.
+            Circle()
+                .fill(notification.isRead ? Color.clear : Color.blue)
+                .frame(width: 8, height: 8)
+                .padding(.top, 6)
+
+            Image(systemName: NotificationStyle.icon(for: notification.eventType))
+                .foregroundStyle(notification.isRead
+                                 ? Color.secondary
+                                 : NotificationStyle.color(for: notification.eventType))
+                .frame(width: 24)
+                .padding(.top, 2)
+
+            VStack(alignment: .leading, spacing: 3) {
+                Text(notification.title)
+                    .font(notification.isRead ? .callout : .callout.bold())
+                    .foregroundStyle(notification.isRead ? .secondary : .primary)
+                    .lineLimit(2)
+                Text(notification.body)
+                    .font(.caption)
+                    .foregroundStyle(notification.isRead ? Color(.tertiaryLabel) : .secondary)
+                    .lineLimit(2)
+                Text(notification.createdAt.formatted(.relative(presentation: .named)))
+                    .font(.caption2)
+                    .foregroundStyle(.tertiary)
+            }
+        }
+        .padding(.vertical, 4)
+    }
+}
+
+// MARK: - Detail
+
+/// Full text of one notification, plus a route to whatever it refers to.
+///
+/// Opening this marks the notification read — the standard inbox contract, and
+/// the reason a tap is treated as a deliberate read action that also clears the
+/// user's web badge (rule 92).
+struct NotificationDetailView: View {
+
+    let notification: LocalNotification
+
+    @EnvironmentObject private var sync: SyncManager
+    @Environment(\.modelContext) private var context
+
+    /// The issue this notification refers to, when it refers to one AND that
+    /// issue is cached on this device. Absent is normal, not an error: the
+    /// issue may belong to another inspector, or simply not be pulled yet.
+    private var linkedIssue: LocalIssue? {
+        guard let issueId = notification.issueId else { return nil }
+        // Fetch-all + filter in Swift (rule 3), `try?` parenthesised (rule 25).
+        let all = (try? context.fetch(FetchDescriptor<LocalIssue>())) ?? []
+        return all.first { $0.serverId == issueId }
+    }
+
+    var body: some View {
+        List {
+            Section {
+                VStack(alignment: .leading, spacing: 10) {
+                    HStack(spacing: 8) {
+                        Image(systemName: NotificationStyle.icon(for: notification.eventType))
+                            .foregroundStyle(NotificationStyle.color(for: notification.eventType))
+                        Text(NotificationStyle.label(for: notification.eventType))
+                            .font(.caption.bold())
+                            .foregroundStyle(.secondary)
+                    }
+                    Text(notification.title)
+                        .font(.headline)
+                    Text(notification.body)
+                        .font(.callout)
+                        .fixedSize(horizontal: false, vertical: true)
+                }
+                .padding(.vertical, 4)
+            }
+
+            Section("Received") {
+                LabeledContent("Sent",
+                               value: notification.createdAt.formatted(date: .long, time: .shortened))
+                if notification.isRead, let readAt = notification.readAt {
+                    LabeledContent("Read",
+                                   value: readAt.formatted(date: .long, time: .shortened))
+                }
+            }
+
+            if let issueId = notification.issueId {
+                Section("Related") {
+                    if let issue = linkedIssue {
+                        NavigationLink(value: issue) {
+                            Label("View Issue #\(issueId)", systemImage: "exclamationmark.triangle")
+                        }
+                    } else {
+                        // Honest dead end rather than a link that goes nowhere.
+                        Label("Issue #\(issueId) is not on this device yet.",
+                              systemImage: "arrow.down.circle")
+                            .font(.callout)
+                            .foregroundStyle(.secondary)
+                        Text("It will appear under Issues after the next sync, "
+                             + "if it is assigned to you.")
+                            .font(.caption)
+                            .foregroundStyle(.tertiary)
+                    }
+                }
+            }
+
+            if !notification.isRead {
+                Section {
+                    Button {
+                        Task { await sync.markNotificationRead(notification) }
+                    } label: {
+                        Label("Mark as Read", systemImage: "envelope.open")
+                    }
+                }
+            }
+        }
+        .navigationTitle("Notification")
+        .navigationBarTitleDisplayMode(.inline)
+        .navigationDestination(for: LocalIssue.self) { issue in
+            IssueDetailView(issue: issue)
+        }
+        .task {
+            // Opening IS reading — the standard inbox contract.
+            // Re-fires when returning from the issue detail, which is harmless:
+            // markNotificationRead() no-ops once isRead is true.
+            await sync.markNotificationRead(notification)
+        }
+    }
+}
+
+// MARK: - Event styling
+
+/// Icon, colour and human label per server `event_type`.
+///
+/// Keys mirror the constants in `app/models/notification.py`; an unknown or nil
+/// type (rows predating the server's phase17 migration) falls back to a
+/// neutral bell rather than being hidden.
+nonisolated enum NotificationStyle {
+
+    static func icon(for eventType: String?) -> String {
+        switch eventType {
+        case "issue_assigned":        return "person.crop.circle.badge.exclamationmark"
+        case "issue_status":          return "arrow.triangle.2.circlepath"
+        case "issue_comment":         return "text.bubble"
+        case "issue_flagged":         return "exclamationmark.triangle.fill"
+        case "issue_follow_update":   return "bell.badge"
+        case "inspection_completed":  return "checkmark.circle.fill"
+        case "sla_alert":             return "clock.badge.exclamationmark"
+        case "score_alert":           return "chart.line.downtrend.xyaxis"
+        case "scheduled_inspection":  return "calendar.badge.clock"
+        case "followup_requested":    return "exclamationmark.arrow.circlepath"
+        case "admin_broadcast":       return "megaphone"
+        default:                      return "bell.fill"
+        }
+    }
+
+    static func color(for eventType: String?) -> Color {
+        switch eventType {
+        case "issue_assigned":        return .blue
+        case "issue_status":          return .blue
+        case "issue_comment":         return .teal
+        case "issue_flagged":         return .orange
+        case "issue_follow_update":   return .blue
+        case "inspection_completed":  return .green
+        case "sla_alert":             return .red
+        case "score_alert":           return .red
+        case "scheduled_inspection":  return .indigo
+        case "followup_requested":    return .orange
+        case "admin_broadcast":       return .purple
+        default:                      return .blue
+        }
+    }
+
+    static func label(for eventType: String?) -> String {
+        switch eventType {
+        case "issue_assigned":        return "ISSUE ASSIGNED"
+        case "issue_status":          return "ISSUE STATUS"
+        case "issue_comment":         return "NEW COMMENT"
+        case "issue_flagged":         return "ISSUE FLAGGED"
+        case "issue_follow_update":   return "FOLLOWED ISSUE"
+        case "inspection_completed":  return "INSPECTION COMPLETED"
+        case "sla_alert":             return "SLA ALERT"
+        case "score_alert":           return "SCORE ALERT"
+        case "scheduled_inspection":  return "SCHEDULED INSPECTION"
+        case "followup_requested":    return "FOLLOW-UP REQUESTED"
+        case "admin_broadcast":       return "ANNOUNCEMENT"
+        default:                      return "NOTIFICATION"
+        }
+    }
+}

JanitorialQC/Views/Dashboard/PhotoDiagnosticView.swift

@@ -0,0 +1,483 @@
+// Views/Dashboard/PhotoDiagnosticView.swift
+// ------------------------------------------
+// READ-ONLY diagnostic for the lost-photo defect (August 2026).
+//
+// Reports, per record, which photo fields never received a server path and
+// whether the underlying JPEG is still recoverable on this device. It exists to
+// size the problem BEFORE any fix ships, because the fix sequence is
+// destructive if run in the wrong order: correcting cleanupOrphanedPhotos()
+// deletes exactly the files this report is looking for.
+//
+// THIS VIEW MUST STAY READ-ONLY. It never calls context.save(), context.delete(),
+// FileManager write/remove, or any APIClient method. It only fetches, reads
+// files' existence, and formats text. Anything that repairs data belongs in a
+// separate, explicitly-named screen — a diagnostic the user cannot trust to be
+// safe is a diagnostic they will not run.
+//
+// ── What it looks for ────────────────────────────────────────────────────────
+// A photo is "lost" when a form field still holds the local sentinel
+// ("local://<path>") instead of a server path ("uploads/..."). APIClient
+// .submitInspection() rewrites that sentinel to "" in the request body only —
+// inspection.formData is left intact — so the device still knows which field
+// the photo belonged to. That is what makes recovery possible, and what this
+// report enumerates.
+//
+// ── File resolution ──────────────────────────────────────────────────────────
+// Stored paths are ABSOLUTE and include the app container UUID, which iOS
+// changes on update / reinstall / restore. So a stored path that no longer
+// exists does NOT mean the file is gone: the same filename usually still exists
+// under the current container. Filenames are UUIDs generated per save, so a
+// basename match is unambiguous and safe to rely on.
+
+import SwiftUI
+import SwiftData
+import UIKit
+
+// MARK: - Model
+
+/// Where the JPEG actually is, independent of what the database claims.
+enum PhotoFileState {
+    /// The stored absolute path resolves — nothing has moved.
+    case foundAtStoredPath
+    /// The stored path is stale (container UUID changed) but a file with the
+    /// same unique filename exists now. Recoverable; carries the live path.
+    case foundByFilename(String)
+    /// No file with that name anywhere under the photo directories.
+    case missing
+
+    var isRecoverable: Bool {
+        if case .missing = self { return false }
+        return true
+    }
+}
+
+/// How much damage has already been done for one missing photo.
+enum PhotoLossSeverity: Int {
+    /// Submitted to the server with the field blank — the loss is live, and
+    /// only a PATCH can repair it.
+    case lostOnServer = 0
+    /// Completed but still in the outbox: it will be submitted blank on the
+    /// next sync unless the fix lands first.
+    case willBeLostOnNextSync = 1
+    /// Still a draft — nothing lost yet.
+    case draftNotYetSubmitted = 2
+
+    var label: String {
+        switch self {
+        case .lostOnServer:         return "LOST ON SERVER"
+        case .willBeLostOnNextSync: return "WILL BE LOST"
+        case .draftNotYetSubmitted: return "DRAFT (safe)"
+        }
+    }
+
+    var color: Color {
+        switch self {
+        case .lostOnServer:         return .red
+        case .willBeLostOnNextSync: return .orange
+        case .draftNotYetSubmitted: return .secondary
+        }
+    }
+}
+
+struct PhotoDiagnosticEntry: Identifiable {
+    let id = UUID()
+
+    let kind: String            // "Inspection" | "Issue"
+    let localId: String
+    let serverId: Int?          // the row to PATCH, when already submitted
+    let title: String           // template / issue description
+    let facilityName: String
+    let date: Date
+
+    let fieldId: String?        // nil for issue photos (no form field)
+    let storedPath: String
+    let fileState: PhotoFileState
+    /// PendingPhoto.uploadStatus, or nil when no row survives for this photo.
+    let uploadStatus: String?
+    let severity: PhotoLossSeverity
+}
+
+// MARK: - View
+
+struct PhotoDiagnosticView: View {
+
+    @Environment(\.modelContext) private var context
+
+    @State private var entries: [PhotoDiagnosticEntry] = []
+    @State private var failedPhotoCount   = 0
+    @State private var totalPhotoRows     = 0
+    @State private var scanned            = false
+    @State private var copied             = false
+
+    private var lostOnServer: [PhotoDiagnosticEntry] {
+        entries.filter { $0.severity == .lostOnServer }
+    }
+    private var willBeLost: [PhotoDiagnosticEntry] {
+        entries.filter { $0.severity == .willBeLostOnNextSync }
+    }
+    private var drafts: [PhotoDiagnosticEntry] {
+        entries.filter { $0.severity == .draftNotYetSubmitted }
+    }
+    private var recoverable: [PhotoDiagnosticEntry] {
+        entries.filter { $0.fileState.isRecoverable }
+    }
+    private var unrecoverable: [PhotoDiagnosticEntry] {
+        entries.filter { !$0.fileState.isRecoverable }
+    }
+
+    var body: some View {
+        List {
+            Section {
+                Label(
+                    "This screen only reads. It does not upload, delete, "
+                    + "repair, or modify anything.",
+                    systemImage: "lock.shield"
+                )
+                .font(.caption)
+                .foregroundStyle(.secondary)
+            }
+
+            if !scanned {
+                Section {
+                    HStack {
+                        ProgressView()
+                        Text("Scanning…").padding(.leading, 8)
+                    }
+                }
+            } else {
+                summarySection
+                if !entries.isEmpty {
+                    entrySection("Lost on Server", lostOnServer,
+                                 footer: "Submitted with the photo field blank. "
+                                 + "Repairing these needs a re-upload plus a PATCH.")
+                    entrySection("Will Be Lost on Next Sync", willBeLost,
+                                 footer: "Still in the outbox. These are submitted "
+                                 + "blank unless the fix lands first.")
+                    entrySection("Drafts", drafts,
+                                 footer: "Not submitted yet — nothing lost.")
+                }
+                copySection
+            }
+        }
+        .navigationTitle("Photo Diagnostic")
+        .navigationBarTitleDisplayMode(.inline)
+        .task { runScan() }
+        .toolbar {
+            ToolbarItem(placement: .primaryAction) {
+                Button {
+                    scanned = false
+                    runScan()
+                } label: {
+                    Label("Rescan", systemImage: "arrow.clockwise")
+                }
+            }
+        }
+    }
+
+    // ── Sections ──────────────────────────────────────────────────────────
+
+    private var summarySection: some View {
+        Section("Summary") {
+            row("Photos affected", "\(entries.count)",
+                tint: entries.isEmpty ? .green : .red)
+            row("Already lost on server", "\(lostOnServer.count)",
+                tint: lostOnServer.isEmpty ? .secondary : .red)
+            row("Will be lost on next sync", "\(willBeLost.count)",
+                tint: willBeLost.isEmpty ? .secondary : .orange)
+            row("Still recoverable (file on device)", "\(recoverable.count)",
+                tint: recoverable.isEmpty ? .secondary : .green)
+            row("File gone — unrecoverable", "\(unrecoverable.count)",
+                tint: unrecoverable.isEmpty ? .secondary : .red)
+
+            Divider()
+            row("Photo upload rows total", "\(totalPhotoRows)", tint: .secondary)
+            row("Rows marked failed", "\(failedPhotoCount)",
+                tint: failedPhotoCount == 0 ? .secondary : .orange)
+
+            if entries.isEmpty {
+                Label("No missing photos found on this device.",
+                      systemImage: "checkmark.circle.fill")
+                    .foregroundStyle(.green)
+                    .font(.callout)
+            }
+        }
+    }
+
+    @ViewBuilder
+    private func entrySection(_ title: String,
+                              _ list: [PhotoDiagnosticEntry],
+                              footer: String) -> some View {
+        if !list.isEmpty {
+            Section {
+                ForEach(list) { e in entryRow(e) }
+            } header: {
+                Text("\(title) (\(list.count))")
+            } footer: {
+                Text(footer)
+            }
+        }
+    }
+
+    private func entryRow(_ e: PhotoDiagnosticEntry) -> some View {
+        VStack(alignment: .leading, spacing: 4) {
+            HStack {
+                Text(e.severity.label)
+                    .font(.caption2.bold())
+                    .padding(.horizontal, 6).padding(.vertical, 2)
+                    .background(e.severity.color.opacity(0.15))
+                    .foregroundStyle(e.severity.color)
+                    .clipShape(Capsule())
+                Spacer()
+                if let sid = e.serverId {
+                    Text("server #\(sid)")
+                        .font(.caption2).foregroundStyle(.secondary)
+                } else {
+                    Text("not on server")
+                        .font(.caption2).foregroundStyle(.secondary)
+                }
+            }
+
+            Text("\(e.kind): \(e.title)").font(.callout.bold())
+            Text(e.facilityName).font(.caption).foregroundStyle(.secondary)
+            Text(e.date.formatted(date: .abbreviated, time: .shortened))
+                .font(.caption2).foregroundStyle(.secondary)
+
+            if let fid = e.fieldId {
+                Text("Field ID: \(fid)").font(.caption2).foregroundStyle(.secondary)
+            }
+            Text("Upload status: \(e.uploadStatus ?? "no record")")
+                .font(.caption2).foregroundStyle(.secondary)
+
+            switch e.fileState {
+            case .foundAtStoredPath:
+                Label("File present at stored path — recoverable",
+                      systemImage: "checkmark.circle")
+                    .font(.caption2).foregroundStyle(.green)
+            case .foundByFilename:
+                Label("Stored path stale (container changed) — file found by "
+                      + "name, recoverable", systemImage: "arrow.triangle.2.circlepath")
+                    .font(.caption2).foregroundStyle(.orange)
+            case .missing:
+                Label("File not on this device — unrecoverable",
+                      systemImage: "exclamationmark.triangle")
+                    .font(.caption2).foregroundStyle(.red)
+            }
+        }
+        .padding(.vertical, 2)
+    }
+
+    private var copySection: some View {
+        Section {
+            Button {
+                UIPasteboard.general.string = textReport()
+                copied = true
+                DispatchQueue.main.asyncAfter(deadline: .now() + 2) { copied = false }
+            } label: {
+                Label(copied ? "Copied" : "Copy Full Report",
+                      systemImage: copied ? "checkmark" : "doc.on.doc")
+            }
+        } footer: {
+            Text("Copies a plain-text version, including full file paths, for "
+                 + "sharing or for driving a recovery pass.")
+        }
+    }
+
+    private func row(_ label: String, _ value: String, tint: Color) -> some View {
+        HStack {
+            Text(label)
+            Spacer()
+            Text(value).bold().foregroundStyle(tint)
+        }
+        .font(.callout)
+    }
+
+    // ── Scan (read-only) ──────────────────────────────────────────────────
+
+    private func runScan() {
+        // Fetch-all + filter in Swift — no #Predicate (CLAUDE.md rule 3),
+        // and every `try?` parenthesised before `??` (rule 25).
+        let inspections = (try? context.fetch(FetchDescriptor<LocalInspection>())) ?? []
+        let issues      = (try? context.fetch(FetchDescriptor<LocalIssue>()))      ?? []
+        let photos      = (try? context.fetch(FetchDescriptor<PendingPhoto>()))    ?? []
+        let facilities  = (try? context.fetch(FetchDescriptor<LocalFacility>()))   ?? []
+        let templates   = (try? context.fetch(FetchDescriptor<LocalTemplate>()))   ?? []
+
+        totalPhotoRows   = photos.count
+        failedPhotoCount = photos.filter { $0.uploadStatus == "failed" }.count
+
+        var facilityName: [Int: String] = [:]
+        for f in facilities { facilityName[f.serverId] = f.name }
+        var templateName: [Int: String] = [:]
+        for t in templates { templateName[t.serverId] = t.name }
+
+        // basename -> live path, for re-resolving stale container paths.
+        let diskIndex = buildDiskIndex()
+
+        // Photo rows keyed by the file they point at, so a form field can be
+        // matched to its upload record.
+        var photoByPath: [String: PendingPhoto] = [:]
+        for p in photos { photoByPath[p.localFilePath] = p }
+
+        var found: [PhotoDiagnosticEntry] = []
+
+        // ── Inspections: form fields still holding the local:// sentinel ──
+        for insp in inspections {
+            for (fieldId, value) in insp.formData {
+                guard let s = value as? String, s.hasPrefix("local://") else { continue }
+                let path = String(s.dropFirst("local://".count))
+
+                let severity: PhotoLossSeverity
+                if insp.status == "draft" {
+                    severity = .draftNotYetSubmitted
+                } else if insp.serverId != nil || insp.syncStatus == "synced" {
+                    severity = .lostOnServer
+                } else {
+                    severity = .willBeLostOnNextSync
+                }
+
+                found.append(PhotoDiagnosticEntry(
+                    kind:         "Inspection",
+                    localId:      insp.localId,
+                    serverId:     insp.serverId,
+                    title:        templateName[insp.templateServerId]
+                                  ?? "Template #\(insp.templateServerId)",
+                    facilityName: facilityName[insp.facilityServerId]
+                                  ?? "Facility #\(insp.facilityServerId)",
+                    date:         insp.inspectionDate,
+                    fieldId:      fieldId,
+                    storedPath:   path,
+                    fileState:    resolve(path, diskIndex),
+                    uploadStatus: photoByPath[path]?.uploadStatus,
+                    severity:     severity
+                ))
+            }
+        }
+
+        // ── Issues: local photos that never produced a server path ────────
+        // Same defect, different surface. processIssueQueue clears
+        // photoLocalPaths only after a successful submit, so a synced issue
+        // still holding local paths with no server paths lost its evidence.
+        for issue in issues {
+            guard !issue.photoLocalPaths.isEmpty else { continue }
+            let missingServerSide = issue.photoServerPaths.count < issue.photoLocalPaths.count
+            guard missingServerSide else { continue }
+
+            let severity: PhotoLossSeverity
+            if issue.syncStatus == "synced" || issue.serverId != nil {
+                severity = .lostOnServer
+            } else if issue.syncStatus == "failed" {
+                severity = .willBeLostOnNextSync
+            } else {
+                severity = .willBeLostOnNextSync
+            }
+
+            for path in issue.photoLocalPaths {
+                // A path already mirrored server-side is fine — skip it.
+                if let p = photoByPath[path], p.uploadStatus == "uploaded" { continue }
+
+                found.append(PhotoDiagnosticEntry(
+                    kind:         "Issue",
+                    localId:      issue.localId,
+                    serverId:     issue.serverId,
+                    title:        issue.issueDescription.isEmpty
+                                  ? "(no description)"
+                                  : String(issue.issueDescription.prefix(60)),
+                    facilityName: facilityName[issue.facilityServerId]
+                                  ?? "Facility #\(issue.facilityServerId)",
+                    date:         issue.createdAt,
+                    fieldId:      nil,
+                    storedPath:   path,
+                    fileState:    resolve(path, diskIndex),
+                    uploadStatus: photoByPath[path]?.uploadStatus,
+                    severity:     severity
+                ))
+            }
+        }
+
+        entries = found.sorted {
+            if $0.severity.rawValue != $1.severity.rawValue {
+                return $0.severity.rawValue < $1.severity.rawValue
+            }
+            return $0.date > $1.date
+        }
+        scanned = true
+    }
+
+    /// Map of filename -> current absolute path for every file under the photo
+    /// directories. Filenames are per-save UUIDs, so collisions are not a
+    /// practical concern and a basename match identifies a file uniquely.
+    private func buildDiskIndex() -> [String: String] {
+        let fm = FileManager.default
+        guard let docs = fm.urls(for: .documentDirectory, in: .userDomainMask).first
+        else { return [:] }
+
+        var index: [String: String] = [:]
+        // Both directories the app writes to today. Enumerating rather than
+        // assuming, so a file left by an older build is still found.
+        for sub in ["JQC/Photos", "JQC/ResultPhotos", "JQCPhotos"] {
+            let dir = docs.appendingPathComponent(sub, isDirectory: true)
+            guard let files = try? fm.contentsOfDirectory(
+                at: dir, includingPropertiesForKeys: nil
+            ) else { continue }
+            for f in files { index[f.lastPathComponent] = f.path }
+        }
+        return index
+    }
+
+    private func resolve(_ storedPath: String,
+                         _ diskIndex: [String: String]) -> PhotoFileState {
+        if FileManager.default.fileExists(atPath: storedPath) {
+            return .foundAtStoredPath
+        }
+        let name = URL(fileURLWithPath: storedPath).lastPathComponent
+        if let live = diskIndex[name] {
+            return .foundByFilename(live)
+        }
+        return .missing
+    }
+
+    // ── Text report ───────────────────────────────────────────────────────
+
+    private func textReport() -> String {
+        var out = """
+        JQC PHOTO DIAGNOSTIC (read-only)
+        Generated: \(Date().formatted(date: .abbreviated, time: .standard))
+        Server: \(ServerConfig.current)
+
+        SUMMARY
+          Photos affected .................. \(entries.count)
+          Already lost on server ......... \(lostOnServer.count)
+          Will be lost on next sync ...... \(willBeLost.count)
+          Drafts (safe) .................. \(drafts.count)
+          Recoverable (file present) ..... \(recoverable.count)
+          Unrecoverable (file gone) ...... \(unrecoverable.count)
+          Photo rows total ............... \(totalPhotoRows)
+          Rows marked failed ............. \(failedPhotoCount)
+
+        DETAIL
+
+        """
+        for e in entries {
+            let state: String
+            switch e.fileState {
+            case .foundAtStoredPath:      state = "file OK at stored path"
+            case .foundByFilename(let p): state = "file found by name -> \(p)"
+            case .missing:                state = "FILE MISSING"
+            }
+            out += """
+            [\(e.severity.label)] \(e.kind) \(e.serverId.map { "server #\($0)" } ?? "(unsent)")
+              title      : \(e.title)
+              facility   : \(e.facilityName)
+              date       : \(e.date.formatted(date: .abbreviated, time: .shortened))
+              localId    : \(e.localId)
+              fieldId    : \(e.fieldId ?? "-")
+              storedPath : \(e.storedPath)
+              upload     : \(e.uploadStatus ?? "no record")
+              fileState  : \(state)
+
+            """
+        }
+        if entries.isEmpty { out += "(none)\n" }
+        return out
+    }
+}

JanitorialQC/Views/Dashboard/SettingsView.swift

@@ -89,6 +89,25 @@ struct SettingsView: View {
                 }
             }
             
+            // Read-only investigation aid for the lost-photo defect. Placed
+            // above Cache deliberately: "Clear Reference Cache" sits next to it
+            // and the diagnostic must be run BEFORE anything that touches
+            // stored data, while the evidence is still intact.
+            Section("Diagnostics") {
+                NavigationLink {
+                    PhotoDiagnosticView()
+                } label: {
+                    // Not a photo.badge.* symbol — those are not universally
+                    // available on iOS 17 (CLAUDE.md rule 41).
+                    Label("Photo Diagnostic", systemImage: "doc.text.magnifyingglass")
+                }
+                Text("Reports inspection and issue photos that never reached the "
+                     + "server, and whether the original file is still on this "
+                     + "device. Read-only — changes nothing.")
+                    .font(.caption)
+                    .foregroundStyle(.secondary)
+            }
+
             Section("Cache") {
                 Button {
                     showClearCacheAlert = true
@@ -110,13 +129,17 @@ struct SettingsView: View {
             Section {
                 Button(role: .destructive) {
                     Task {
-                        // Do NOT clear server-pulled data on a plain logout —
-                        // the user is logging out of the same server, so cached
-                        // facilities, issues, and templates are still valid on
-                        // their next login.  Clearing here leaves the issues list
-                        // empty until a full sync succeeds, which breaks offline use.
-                        // Server-pulled data is only cleared when switching servers
-                        // (see the Switch & Log Out alert below).
+                        // Do NOT purge on a plain logout — the same inspector
+                        // signing back into the same server must still find
+                        // their facilities, templates and issues there, or the
+                        // app is unusable offline until a full sync succeeds.
+                        //
+                        // What was missing is not a purge here: it is the check
+                        // that the next sign-in is the SAME person.
+                        // AuthManager.reconcileSessionScope() now does that at
+                        // login and purges only on an identity change, so a
+                        // different inspector no longer inherits this one's
+                        // issues (rule 88).
                         sync.resetNotificationPoller()
                         await auth.logout()
                     }
@@ -188,7 +211,14 @@ struct SettingsView: View {
                     settingsServer = chosen
                     pendingServer = nil
                     Task {
-                        clearServerPulledData()
+                        // Purge EVERYTHING, not just issues. The old
+                        // clearServerPulledData() deleted LocalIssue alone,
+                        // leaving LocalInspection rows carrying facility and
+                        // template ids that name different rows on the server
+                        // being switched to — ready to be submitted against it.
+                        // Nothing local survives a server change (rule 88).
+                        sync.purgeSessionScopedData(keepingUserId: nil, sameServer: false)
+                        SessionScope.clear()
                         sync.resetNotificationPoller()
                         await auth.logout()
                     }
@@ -199,7 +229,7 @@ struct SettingsView: View {
             }
         } message: {
             if let chosen = pendingServer {
-                Text("Switching to \(chosen.displayName) will log you out. All cached server data will be cleared. You will need to log in again.")
+                Text("Switching to \(chosen.displayName) will log you out and erase all local data for this server — including any inspections or issues that have not synced yet. You will need to log in again.")
             }
         }
     }
@@ -223,18 +253,4 @@ struct SettingsView: View {
         }
     }
     
-    /// Delete every LocalIssue that has ever been assigned a serverId.
-    /// This covers two categories:
-    ///   1. Server-pulled assigned issues  (inspectionLocalId == "", syncStatus == "synced")
-    ///   2. Inspector-created issues that already synced (inspectionLocalId != "", serverId != nil)
-    ///      — their serverIds are meaningless on a different server, so they must go too.
-    /// The only records preserved are truly pending device-created issues
-    /// (serverId == nil, syncStatus == "pending") that have never reached any server.
-    private func clearServerPulledData() {
-        let allIssues = (try? context.fetch(FetchDescriptor<LocalIssue>())) ?? []
-        allIssues
-            .filter { $0.serverId != nil }
-            .forEach { context.delete($0) }
-        try? context.save()
-    }
 }

JanitorialQC/Views/Dashboard/StartInspectionView.swift

@@ -74,12 +74,23 @@ struct StartInspectionView: View {
 
     // ── Derived lists ─────────────────────────────────────────────────────
 
+    /// Facilities this inspector may actually start work at.
+    ///
+    /// The cache can hold a facility that is no longer in scope — SyncManager
+    /// keeps such a row (marked inactive) when an unsynced draft still needs
+    /// its name, rather than deleting it and showing "Unknown Facility". It
+    /// must not be offered for NEW work, and neither must its contract, so
+    /// every derived list below starts here rather than from `facilities`.
+    private var availableFacilities: [LocalFacility] {
+        facilities.filter { $0.isActive }
+    }
+
     /// Unique contracts (projectId, projectName) sorted by name.
     /// Facilities with projectId == 0 are grouped under "No Contract".
     private var contracts: [(id: Int, name: String)] {
         var seen = Set<Int>()
         var result: [(id: Int, name: String)] = []
-        for f in facilities {
+        for f in availableFacilities {
             if seen.insert(f.projectId).inserted {
                 result.append((id: f.projectId, name: f.projectName))
             }
@@ -93,7 +104,7 @@ struct StartInspectionView: View {
     private var filteredFacilities: [LocalFacility] {
         guard let pid = selectedProjectId else { return [] }
         var seen = Set<Int>()
-        return facilities
+        return availableFacilities
             .filter { $0.projectId == pid }
             .filter { seen.insert($0.serverId).inserted }
     }
@@ -287,7 +298,11 @@ struct StartInspectionView: View {
             }
             .navigationDestination(isPresented: $navigateToExecution) {
                 if let inspection = createdInspection {
-                    ExecuteInspectionView(inspection: inspection)
+                    // onFinished closes THIS cover rather than just popping back
+                    // to the form the inspector already finished with — see the
+                    // property's doc comment on ExecuteInspectionView.
+                    ExecuteInspectionView(inspection: inspection,
+                                          onFinished: { dismiss() })
                 }
             }
             .onAppear {

JanitorialQC/Views/Dashboard/SyncStatusView.swift

@@ -19,9 +19,20 @@ struct SyncStatusView: View {
         sort: \LocalIssue.createdAt
     ) private var pendingIssues: [LocalIssue]
 
+    /// Unfiltered — `uploadStatus` is matched in Swift rather than in a
+    /// #Predicate, per CLAUDE.md rules 3/48.
+    @Query private var allPendingPhotos: [PendingPhoto]
+
     private var failedInspections: [LocalInspection] { pendingInspections.filter { $0.syncStatus == "failed" } }
     private var failedIssues: [LocalIssue]            { pendingIssues.filter      { $0.syncStatus == "failed" } }
-    private var hasFailedItems: Bool { !failedInspections.isEmpty || !failedIssues.isEmpty }
+    /// Photos that exhausted `SyncManager.maxPhotoUploadAttempts`. These are the
+    /// reason an inspection can be submitted with a blank photo field, and
+    /// nothing else in the app ever moves one off "failed" — so they belong in
+    /// the retry action too.
+    private var failedPhotos: [PendingPhoto]          { allPendingPhotos.filter   { $0.uploadStatus == "failed" } }
+    private var hasFailedItems: Bool {
+        !failedInspections.isEmpty || !failedIssues.isEmpty || !failedPhotos.isEmpty
+    }
 
     var body: some View {
         List {
@@ -60,7 +71,7 @@ struct SyncStatusView: View {
                     Button {
                         retryAllFailed()
                     } label: {
-                        Label("Retry Failed Items (\(failedInspections.count + failedIssues.count))",
+                        Label("Retry Failed Items (\(failedInspections.count + failedIssues.count + failedPhotos.count))",
                               systemImage: "exclamationmark.arrow.circlepath")
                             .foregroundStyle(.orange)
                     }
@@ -113,6 +124,14 @@ struct SyncStatusView: View {
             issue.syncRetryCount   = 0
             issue.syncErrorMessage = nil
         }
+        // Photos too. A PendingPhoto only reaches "failed" after every upload
+        // attempt was used, and that is exactly the state that lets an
+        // inspection be submitted with its photo field blank — so without this
+        // the retry button could never actually recover a lost photo.
+        for photo in failedPhotos {
+            photo.uploadStatus     = "pending"
+            photo.uploadRetryCount = 0
+        }
         try? context.save()
         Task { await sync.triggerSync() }
     }

JanitorialQC/Views/Inspection/InspectionHistoryView.swift

@@ -162,6 +162,14 @@ struct InspectionHistoryView: View {
                 filterToDate     = nil
                 showFilterSheet  = false
                 Task { await load(reset: true) }
+            } onCancel: {
+                // Discard the draft edits, keep whatever is currently applied,
+                // and do NOT reload — backing out must change nothing.
+                draftFromEnabled = filterFromDate != nil
+                draftToEnabled   = filterToDate   != nil
+                if let f = filterFromDate { draftFromDate = f }
+                if let t = filterToDate   { draftToDate   = t }
+                showFilterSheet = false
             }
         }
         .task {
@@ -215,6 +223,10 @@ struct DateFilterSheet: View {
     @Binding var toDate:      Date
     let onApply: () -> Void
     let onClear: () -> Void
+    /// Dismiss without touching the active filter. Distinct from `onClear`:
+    /// Cancel used to call that, so backing out of the sheet silently wiped
+    /// whatever date range was already applied and reloaded the list.
+    let onCancel: () -> Void
 
     var body: some View {
         NavigationStack {
@@ -243,7 +255,7 @@ struct DateFilterSheet: View {
             .navigationBarTitleDisplayMode(.inline)
             .toolbar {
                 ToolbarItem(placement: .cancellationAction) {
-                    Button("Cancel") { onClear() }
+                    Button("Cancel") { onCancel() }
                 }
                 ToolbarItem(placement: .confirmationAction) {
                     Button("Apply", action: onApply)
@@ -371,9 +383,12 @@ struct HistoryDetailView: View {
 
     /// Auditors are read-only everywhere else and the API rejects them (403),
     /// so the two action buttons are hidden rather than shown failing.
+    ///
+    /// `issueActors` is the same set minus auditor, and — unlike the literal
+    /// list this replaced — it includes Customer Inspectors, who perform
+    /// inspections exactly as our own do.
     private var canStartFollowUp: Bool {
-        ["admin", "director", "inspector", "project_manager"]
-            .contains(auth.currentUserRole)
+        Constants.Roles.issueActors.contains(auth.currentUserRole)
     }
 
     // Local SwiftData copy — used only for follow-up sync-back.
@@ -1164,7 +1179,7 @@ struct PhotoThumbnailView: View {
         Group {
             if value.hasPrefix("local://") {
                 let path = String(value.dropFirst("local://".count))
-                if let img = UIImage(contentsOfFile: path) {
+                if let live = PhotoStore.resolve(path), let img = UIImage(contentsOfFile: live) {
                     thumbnailButton {
                         Image(uiImage: img)
                             .resizable().scaledToFill()