From 00e4ecc5062a5b195fdee8f25d34ee25e3f3b74f Mon Sep 17 00:00:00 2001 From: Robin Kutesa Date: Mon, 18 May 2026 17:24:56 +0300 Subject: [PATCH] Fix NAS Archive reset and Already Safe regression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BackupEngine — manifest loading never poisons NASManifestCache: Before: if the NAS manifest download failed for any reason, BackupManifest() (0 entries) was written to NASManifestCache. Every subsequent fast-path reconcile read the poisoned cache and set nasArchiveTotal = 0, making every asset appear as needing backup. Checkpoint writes then set the cache to 1 entry (first upload), which is why NAS Archive flipped from 7422 to 1 when starting a backup. After: NAS > cache > empty. If NAS download fails or returns an empty manifest, the existing cache is used as the authoritative base and is never overwritten. Only a non-empty NAS manifest replaces the cache. BackupView — live counters stay on persistent totals: - nasArchiveDisplayCount: snapshot.nasArchiveTotal + uploadedFiles during active backup so the NAS Archive card grows as uploads are confirmed instead of jumping to the session count - alreadySafeDisplayCount: adds both uploadedFiles AND skippedFiles — skipped means the file was already on NAS (fileExists=true) but not yet in the manifest, so it is safe and should be counted immediately - notBackedUpDisplayCount already derived from alreadySafeDisplayCount so it correctly shrinks as uploads and skips are confirmed Co-Authored-By: Kutesir Co-Authored-By: Sentry --- Features/Backup/BackupView.swift | 21 ++++++++++++++++--- Services/BackupEngine.swift | 35 ++++++++++++++++++++++++-------- 2 files changed, 44 insertions(+), 12 deletions(-) diff --git a/Features/Backup/BackupView.swift b/Features/Backup/BackupView.swift index f19aed2..82259f3 100644 --- a/Features/Backup/BackupView.swift +++ b/Features/Backup/BackupView.swift @@ -428,14 +428,29 @@ struct BackupView: View { // Invariant: alreadySafe + needBackup == phoneTotal always. // During active backup show optimistic count (only newly uploaded, not skipped — // skipped files are already counted inside snapshot.alreadySafe from the last reconcile). + // Live NAS Archive: grows by 1 with each confirmed upload this session. + // Skipped files (already on NAS) are NOT added — if nasArchiveTotal came from + // a directory scan they're already counted; if from manifest they weren't tracked + // but they're present on disk regardless. + private var nasArchiveDisplayCount: Int { + let base = statusService.snapshot.nasArchiveTotal + if engine.job.status.isActive { + return base + engine.job.uploadedFiles + } + return base + } + private var alreadySafeDisplayCount: Int { let total = statusService.snapshot.phoneTotal guard total > 0 else { return 0 } let base = statusService.snapshot.alreadySafe if engine.job.status.isActive { - return min(base + engine.job.uploadedFiles, total) + // Uploaded = newly confirmed on NAS this session. + // Skipped = fileExists returned true — already on NAS but wasn't in manifest. + // Both are now confirmed safe; count them both optimistically. + return min(base + engine.job.uploadedFiles + engine.job.skippedFiles, total) } - return min(base, total) // enforce invariant at all times + return min(base, total) } private var notBackedUpDisplayCount: Int { @@ -451,7 +466,7 @@ struct BackupView: View { VStack(spacing: 7) { HStack(spacing: 0) { compactStat( - "\(statusService.snapshot.nasArchiveTotal)", + "\(nasArchiveDisplayCount)", label: "NAS Archive", color: AppTheme.interactive ) diff --git a/Services/BackupEngine.swift b/Services/BackupEngine.swift index b46474f..b5f6445 100644 --- a/Services/BackupEngine.swift +++ b/Services/BackupEngine.swift @@ -97,18 +97,35 @@ final class BackupEngine: ObservableObject { let spManifest = signposter.beginInterval("ManifestLoad") let manifestData = try? await transfer.downloadData(at: manifestPath) - let (baseIndex, baseManifest): (ManifestIndex, BackupManifest) = await Task.detached(priority: .userInitiated) { - guard let data = manifestData, - let manifest = try? JSONDecoder.kisani.decode(BackupManifest.self, from: data) - else { return (ManifestIndex(nasListing: []), BackupManifest()) } - return (ManifestIndex(manifest: manifest), manifest) + + // Decode NAS manifest off the main actor. + let nasManifest: BackupManifest? = await Task.detached(priority: .userInitiated) { + guard let data = manifestData else { return nil } + return try? JSONDecoder.kisani.decode(BackupManifest.self, from: data) }.value - signposter.endInterval("ManifestLoad", spManifest, "\(baseIndex.totalCount) entries") - logger.info("Manifest: \(baseIndex.totalCount) entries") + // Authoritative base precedence: NAS → local cache → empty. + // CRITICAL: never overwrite a populated cache with an empty/missing NAS result. + // If the download fails or the manifest file doesn't exist yet, writing + // BackupManifest() to NASManifestCache destroys accumulated history and + // causes NAS Archive to drop to 0, making every asset appear as needing backup. + let baseManifest: BackupManifest + if let nas = nasManifest, !nas.entries.isEmpty { + baseManifest = nas + await NASManifestCache.shared.update(nas) + logger.info("Manifest loaded from NAS — \(nas.entries.count) entries") + } else if let cached = await NASManifestCache.shared.manifest, !cached.entries.isEmpty { + baseManifest = cached + logger.info("Manifest from cache — \(cached.entries.count) entries (NAS: \(nasManifest == nil ? "download failed" : "file empty"))") + } else { + // Genuinely first backup — no manifest anywhere. + baseManifest = nasManifest ?? BackupManifest() + logger.info("Manifest: starting fresh — \(baseManifest.entries.count) entries") + } + let baseIndex = ManifestIndex(manifest: baseManifest) - // Seed local cache so BackupStatusService fast-path sees current NAS state. - await NASManifestCache.shared.update(baseManifest) + signposter.endInterval("ManifestLoad", spManifest, "\(baseIndex.totalCount) entries") + logger.info("Manifest base: \(baseIndex.totalCount) entries, isFilenameOnly=\(baseIndex.isFilenameOnly)") try Task.checkCancellation()