From 6ae567d008525873b2a9b6e41be435aaa0b08788 Mon Sep 17 00:00:00 2001 From: Robin Kutesa Date: Tue, 19 May 2026 00:32:03 +0300 Subject: [PATCH] Fix Already Safe = 0: hybrid ManifestIndex preserves localIdentifiers + NAS filenames MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The root cause: buildManifestIndex was discarding manifest localIdentifiers and switching to pure filename-only matching whenever dirCount > manifest.totalCount (which is always true when the NAS has files from other tools). If filenames didn't match exactly, Already Safe dropped to 0 on every stale-cache reconcile. Fix: introduce ManifestIndex(manifest:nasFilenames:totalCount:) — a hybrid that keeps Kisani's localIdentifiers as primary keys AND adds all NAS directory filenames for filename-fallback. Used in: - buildManifestIndex (full reconcile) - fast path (when dirCount > cached.entries.count, previously gated at <= 1) - BackupEngine step 3b (so "Back up again" only uploads genuinely missing files) Also simplify countSafe / pendingIDs / step-4 filter to always try filename fallback (removes the isFilenameOnly gate that blocked matches when the index had IDs). Co-Authored-By: Kutesir Co-Authored-By: Sentry --- Core/Models/BackupManifest.swift | 23 +++++++++++++++++++++++ Services/BackupEngine.swift | 15 ++++++++------- Services/BackupStatusService.swift | 30 ++++++++++++++++-------------- Services/LocalPhotoIndex.swift | 14 ++------------ 4 files changed, 49 insertions(+), 33 deletions(-) diff --git a/Core/Models/BackupManifest.swift b/Core/Models/BackupManifest.swift index fb5d56c..f5e16cd 100644 --- a/Core/Models/BackupManifest.swift +++ b/Core/Models/BackupManifest.swift @@ -85,6 +85,29 @@ struct ManifestIndex: Sendable { self.totalCount = names.count } + // Hybrid: manifest localIdentifiers (Kisani primary key) + all NAS filenames + // for filename-fallback matching when the NAS directory has more files than the + // manifest tracks (uploaded by other tools). totalCount = actual NAS file count. + init(manifest: BackupManifest, nasFilenames: [String], totalCount: Int) { + var ids = Set(minimumCapacity: manifest.entries.count) + var names = Set(minimumCapacity: manifest.entries.count + nasFilenames.count) + var unclaimed = Set() + for entry in manifest.entries { + let lower = entry.filename.lowercased() + names.insert(lower) + if !entry.localIdentifier.isEmpty { + ids.insert(entry.localIdentifier) + } else { + unclaimed.insert(lower) + } + } + for name in nasFilenames { names.insert(name.lowercased()) } + self.localIdentifiers = ids + self.lowercasedFilenames = names + self.unclaimedFilenames = unclaimed + self.totalCount = totalCount + } + // Build from NAS directory listing (bootstrap — no localIdentifiers known) init(nasListing items: [NASItem]) { var names = Set() diff --git a/Services/BackupEngine.swift b/Services/BackupEngine.swift index 26900b0..48babba 100644 --- a/Services/BackupEngine.swift +++ b/Services/BackupEngine.swift @@ -135,17 +135,19 @@ final class BackupEngine: ObservableObject { // uploaded by other tools), a manifest-only index will misidentify those files // as "not backed up" and re-upload them on every "Back up again" tap. // Use the broader NAS directory listing as the filter when the manifest is sparse. + // Build a hybrid filter index: manifest localIdentifiers (primary) + all NAS + // filenames (fallback). This ensures files uploaded by other tools are recognised + // as "already on NAS" by filename and are not re-uploaded on every backup run. let filterIndex: ManifestIndex let cachedDirCount = await NASManifestCache.shared.directoryCount ?? 0 if baseIndex.totalCount < cachedDirCount { if let cachedFilenames = await NASManifestCache.shared.directoryFilenames, cachedFilenames.count > baseIndex.totalCount { filterIndex = await Task.detached(priority: .utility) { - ManifestIndex(nasFilenames: cachedFilenames) + ManifestIndex(manifest: baseManifest, nasFilenames: cachedFilenames, totalCount: cachedDirCount) }.value - logger.info("Filter index: \(cachedFilenames.count) cached NAS filenames (manifest=\(baseIndex.totalCount))") + logger.info("Filter index: hybrid — \(baseIndex.totalCount) manifest + \(cachedFilenames.count) cached NAS filenames") } else { - // No cached filenames — do a fresh listing (connection is already open) let nasItems = (try? await transfer.listDirectory(at: connection.remotePath)) ?? [] let names = nasItems.compactMap { item -> String? in guard !item.isDirectory, !item.name.hasPrefix("."), @@ -154,9 +156,9 @@ final class BackupEngine: ObservableObject { } await NASManifestCache.shared.setDirectoryFilenames(names) filterIndex = await Task.detached(priority: .utility) { - ManifestIndex(nasFilenames: names) + ManifestIndex(manifest: baseManifest, nasFilenames: names, totalCount: names.count) }.value - logger.info("Filter index: \(names.count) fresh NAS filenames (manifest=\(baseIndex.totalCount))") + logger.info("Filter index: hybrid — \(baseIndex.totalCount) manifest + \(names.count) fresh NAS filenames") } } else { filterIndex = baseIndex @@ -169,8 +171,7 @@ final class BackupEngine: ObservableObject { let pendingAssets: [PhotoAsset] = await Task.detached(priority: .userInitiated) { assets.filter { asset in !filterIndex.matches(localIdentifier: asset.localIdentifier) && - !(filterIndex.isFilenameOnly && filterIndex.matches(filename: asset.filename)) && - !filterIndex.matchesUnclaimed(filename: asset.filename) + !filterIndex.matches(filename: asset.filename) } }.value signposter.endInterval("BuildPendingQueue", spFilter, "\(pendingAssets.count) pending") diff --git a/Services/BackupStatusService.swift b/Services/BackupStatusService.swift index 84bb80b..43f05db 100644 --- a/Services/BackupStatusService.swift +++ b/Services/BackupStatusService.swift @@ -162,16 +162,17 @@ final class BackupStatusService: NSObject, ObservableObject { fastPath: if !manifestCacheIsStale, let cached = await NASManifestCache.shared.manifest, localIndexCount > 0 { let dirCount = await NASManifestCache.shared.directoryCount ?? 0 let index: ManifestIndex - if cached.entries.count <= 1 && dirCount > 1 { - // Manifest is sparse vs. actual NAS directory. Use cached filenames for - // filename-based matching so countSafe reflects real NAS coverage. - // If filenames aren't cached yet, fall through to a full NAS reconcile. + if dirCount > cached.entries.count { + // NAS has more files than the manifest tracks. Build a hybrid index that keeps + // manifest localIdentifiers (for Kisani uploads) and adds all cached NAS + // filenames as a filename-fallback. If filenames aren't cached yet, fall + // through to a full NAS reconcile so they get populated. guard let filenames = await NASManifestCache.shared.directoryFilenames else { - log.info("Reconcile (cached): sparse manifest, no cached filenames — falling to full reconcile") + log.info("Reconcile (cached): NAS dir(\(dirCount)) > manifest(\(cached.entries.count)) — no cached filenames, falling to full reconcile") break fastPath } index = await Task.detached(priority: .utility) { - ManifestIndex(nasFilenames: filenames) + ManifestIndex(manifest: cached, nasFilenames: filenames, totalCount: dirCount) }.value } else { index = await Task.detached(priority: .utility) { @@ -320,9 +321,13 @@ final class BackupStatusService: NSObject, ObservableObject { let dirCount = filteredItems.count log.info("buildManifestIndex: dir=\(dirCount) manifest=\(index.totalCount)") if dirCount > index.totalCount { - await NASManifestCache.shared.setDirectoryFilenames(filteredItems.map { $0.name }) + // NAS has more files than manifest: keep manifest localIdentifiers + // for Kisani-tracked uploads and add all NAS filenames as fallback. + // This prevents discarding accumulated localIdentifier history. + let nasNames = filteredItems.map { $0.name } + await NASManifestCache.shared.setDirectoryFilenames(nasNames) return await Task.detached(priority: .utility) { - ManifestIndex(nasListing: items) + ManifestIndex(manifest: manifest, nasFilenames: nasNames, totalCount: dirCount) }.value } else { await NASManifestCache.shared.setDirectoryCount(dirCount) @@ -371,12 +376,9 @@ final class BackupStatusService: NSObject, ObservableObject { count += 1 } else { let resources = PHAssetResource.assetResources(for: asset) - if let name = resources.first?.originalFilename { - if index.isFilenameOnly && index.matches(filename: name) { - count += 1 - } else if index.matchesUnclaimed(filename: name) { - count += 1 - } + if let name = resources.first?.originalFilename, + index.matches(filename: name) { + count += 1 } } } diff --git a/Services/LocalPhotoIndex.swift b/Services/LocalPhotoIndex.swift index 375f957..a34902f 100644 --- a/Services/LocalPhotoIndex.swift +++ b/Services/LocalPhotoIndex.swift @@ -61,14 +61,7 @@ actor LocalPhotoIndex { var safe = 0 for record in records.values { guard passes(record, filter: filter) else { continue } - if index.matches(localIdentifier: record.localIdentifier) { - safe += 1 - } else if index.isFilenameOnly && index.matches(filename: record.filename) { - // Pure bootstrap mode: every entry is filename-only, match by name. - safe += 1 - } else if index.matchesUnclaimed(filename: record.filename) { - // Mixed mode: this specific entry was uploaded without a localIdentifier - // (interrupted session / pre-ID backup). Filename match prevents re-upload. + if index.matches(localIdentifier: record.localIdentifier) || index.matches(filename: record.filename) { safe += 1 } } @@ -79,10 +72,7 @@ actor LocalPhotoIndex { func pendingIDs(against index: ManifestIndex, filter: BackupFilter) -> [String] { records.values.compactMap { record in guard passes(record, filter: filter) else { return nil } - let alreadyBacked = - index.matches(localIdentifier: record.localIdentifier) || - (index.isFilenameOnly && index.matches(filename: record.filename)) || - index.matchesUnclaimed(filename: record.filename) + let alreadyBacked = index.matches(localIdentifier: record.localIdentifier) || index.matches(filename: record.filename) return alreadyBacked ? nil : record.localIdentifier } }