fix: use country-level resolution for low-precision geohashes (#1044)

* fix: use country-level resolution for low-precision geohashes (#887) * fix: skip admin fallback when country exists but is duplicate Prevents mixed labels like "United Kingdom and Scotland" for single-country geohashes. Admin fallback now only triggers when no country is available from the placemark. * fix: migrate stale low-precision bookmark names so country-first logic applies Users who bookmarked a <=2-char geohash before the country-first resolver kept the old administrativeArea cache (e.g. "England" for `gc`) because resolveBookmarkNameIfNeeded bails when bookmarkNames[gh] is non-nil. Add a one-shot, versioned migration that drops cached names for <=2-char geohashes on load; the next LocationChannelsSheet .onAppear re-resolves them via the fixed logic. Higher-precision entries are untouched. * Fix low-precision geohash country names --------- Co-authored-by: jack <212554440+jackjackbits@users.noreply.github.com> Co-authored-by: jack <jackjackbits@users.noreply.github.com>
2026-07-24 20:45:18 +00:00 · 2026-07-10 15:16:02 +02:00
parent 820c933958
commit 733098bb63
2 changed files with 124 additions and 13 deletions
@@ -109,6 +109,8 @@ final class LocationStateManager: NSObject, CLLocationManagerDelegate, Observabl
    private let teleportedStoreKey = "locationChannel.teleportedSet"
    private let bookmarksKey = "locationChannel.bookmarks"
    private let bookmarkNamesKey = "locationChannel.bookmarkNames"
+    private let bookmarkNamesSchemaVersionKey = "locationChannel.bookmarkNamesSchemaVersion"
+    private let bookmarkNamesCurrentSchemaVersion = 1

    // MARK: - Published State (Channel)

@@ -222,6 +224,26 @@ final class LocationStateManager: NSObject, CLLocationManagerDelegate, Observabl
           let dict = try? JSONDecoder().decode([String: String].self, from: data) {
            bookmarkNames = dict
        }
+
+        migrateBookmarkNamesIfNeeded()
+    }
+
+    /// Drops names cached before low-precision geohashes became country-first.
+    /// Correctly resolved names written after this migration must survive
+    /// subsequent launches, so the migration is explicitly versioned.
+    private func migrateBookmarkNamesIfNeeded() {
+        guard storage.integer(forKey: bookmarkNamesSchemaVersionKey) < bookmarkNamesCurrentSchemaVersion else {
+            return
+        }
+
+        let retainedNames = bookmarkNames.filter {
+            Self.normalizeGeohash($0.key).count > 2
+        }
+        if retainedNames.count != bookmarkNames.count {
+            bookmarkNames = retainedNames
+            persistBookmarkNames()
+        }
+        storage.set(bookmarkNamesCurrentSchemaVersion, forKey: bookmarkNamesSchemaVersionKey)
    }

    private func initializePermissionState() {
@@ -525,15 +547,15 @@ final class LocationStateManager: NSObject, CLLocationManagerDelegate, Observabl
    }

    private func resolveCompositeAdminName(geohash gh: String, points: [CLLocation]) {
-        var uniqueAdmins: [String] = []
-        var seenAdmins = Set<String>()
+        var uniqueRegions: [String] = []
+        var seenRegions = Set<String>()
        var idx = 0

        func step() {
            if idx >= points.count {
                let finalName: String? = {
-                    if uniqueAdmins.count >= 2 { return uniqueAdmins[0] + " and " + uniqueAdmins[1] }
-                    return uniqueAdmins.first
+                    if uniqueRegions.count >= 2 { return uniqueRegions[0] + " and " + uniqueRegions[1] }
+                    return uniqueRegions.first
                }()
                if let finalName = finalName, !finalName.isEmpty {
                    DispatchQueue.main.async {
@@ -549,12 +571,16 @@ final class LocationStateManager: NSObject, CLLocationManagerDelegate, Observabl
            geocoder.reverseGeocodeLocation(loc) { [weak self] placemarks, _ in
                guard self != nil else { return }
                if let pm = placemarks?.first {
-                    if let admin = pm.administrativeArea, !admin.isEmpty, !seenAdmins.contains(admin) {
-                        seenAdmins.insert(admin)
-                        uniqueAdmins.append(admin)
-                    } else if let country = pm.country, !country.isEmpty, !seenAdmins.contains(country) {
-                        seenAdmins.insert(country)
-                        uniqueAdmins.append(country)
+                    if let country = pm.country, !country.isEmpty {
+                        if !seenRegions.contains(country) {
+                            seenRegions.insert(country)
+                            uniqueRegions.append(country)
+                        }
+                    } else if let admin = pm.administrativeArea,
+                              !admin.isEmpty,
+                              !seenRegions.contains(admin) {
+                        seenRegions.insert(admin)
+                        uniqueRegions.append(admin)
                    }
                }
                step()
@@ -566,7 +592,7 @@ final class LocationStateManager: NSObject, CLLocationManagerDelegate, Observabl
    private static func nameForGeohashLength(_ len: Int, from pm: CLPlacemark) -> String? {
        switch len {
        case 0...2:
-            return pm.administrativeArea ?? pm.country
+            return pm.country ?? pm.administrativeArea
        case 3...4:
            return pm.administrativeArea ?? pm.subAdministrativeArea ?? pm.country
        case 5:
@@ -207,7 +207,54 @@ final class LocationStateManagerTests: XCTestCase {
        XCTAssertFalse(reloaded.teleported)
    }

-    func test_addBookmark_lowPrecisionResolvesCompositeAdminName() async {
+    func test_loadPersistedState_migratesLowPrecisionBookmarkNamesOnce() throws {
+        let storage = makeStorage()
+        let staleNames = [
+            "gc": "England",
+            "u3": "Île-de-France",
+            "u4pr": "Paris",
+            "u4pruy": "Le Marais"
+        ]
+        storage.set(try JSONEncoder().encode(staleNames), forKey: "locationChannel.bookmarkNames")
+
+        let migrated = LocationStateManager(
+            storage: storage,
+            locationManager: MockLocationManager(authorizationStatus: .denied),
+            geocoder: MockLocationGeocoder(),
+            shouldInitializeCoreLocation: false
+        )
+
+        XCTAssertNil(migrated.bookmarkNames["gc"])
+        XCTAssertNil(migrated.bookmarkNames["u3"])
+        XCTAssertEqual(migrated.bookmarkNames["u4pr"], "Paris")
+        XCTAssertEqual(migrated.bookmarkNames["u4pruy"], "Le Marais")
+        XCTAssertEqual(storage.integer(forKey: "locationChannel.bookmarkNamesSchemaVersion"), 1)
+
+        let persistedData = try XCTUnwrap(storage.data(forKey: "locationChannel.bookmarkNames"))
+        let persistedNames = try JSONDecoder().decode([String: String].self, from: persistedData)
+        XCTAssertEqual(persistedNames, [
+            "u4pr": "Paris",
+            "u4pruy": "Le Marais"
+        ])
+
+        let correctedNames = [
+            "gc": "United Kingdom",
+            "u4pr": "Paris"
+        ]
+        storage.set(try JSONEncoder().encode(correctedNames), forKey: "locationChannel.bookmarkNames")
+
+        let reloaded = LocationStateManager(
+            storage: storage,
+            locationManager: MockLocationManager(authorizationStatus: .denied),
+            geocoder: MockLocationGeocoder(),
+            shouldInitializeCoreLocation: false
+        )
+
+        XCTAssertEqual(reloaded.bookmarkNames["gc"], "United Kingdom")
+        XCTAssertEqual(reloaded.bookmarkNames["u4pr"], "Paris")
+    }
+
+    func test_addBookmark_lowPrecisionPrefersCountryOverAdministrativeAreas() async {
        let geocoder = MockLocationGeocoder()
        geocoder.enqueue(placemarks: [makePlacemark(country: "United States", administrativeArea: "California")])
        geocoder.enqueue(placemarks: [makePlacemark(country: "United States", administrativeArea: "Nevada")])
@@ -223,12 +270,50 @@ final class LocationStateManagerTests: XCTestCase {

        manager.addBookmark("9q")

-        let bookmarkResolved = await waitUntil { manager.bookmarkNames["9q"] == "California and Nevada" }
+        let bookmarkResolved = await waitUntil { manager.bookmarkNames["9q"] == "United States" }
        XCTAssertTrue(bookmarkResolved)
        XCTAssertEqual(geocoder.reverseRequests.count, 5)
        XCTAssertEqual(manager.bookmarks, ["9q"])
    }

+    func test_addBookmark_lowPrecisionFallsBackToDistinctAdministrativeAreas() async {
+        let geocoder = MockLocationGeocoder()
+        geocoder.enqueue(placemarks: [makePlacemark(administrativeArea: "California")])
+        geocoder.enqueue(placemarks: [makePlacemark(administrativeArea: "Nevada")])
+        geocoder.enqueue(placemarks: [makePlacemark(administrativeArea: "California")])
+        geocoder.enqueue(placemarks: [makePlacemark(administrativeArea: "Arizona")])
+        geocoder.enqueue(placemarks: [makePlacemark(administrativeArea: "Nevada")])
+        let manager = LocationStateManager(
+            storage: makeStorage(),
+            locationManager: MockLocationManager(authorizationStatus: .denied),
+            geocoder: geocoder,
+            shouldInitializeCoreLocation: false
+        )
+
+        manager.addBookmark("9q")
+
+        let bookmarkResolved = await waitUntil { manager.bookmarkNames["9q"] == "California and Nevada" }
+        XCTAssertTrue(bookmarkResolved)
+        XCTAssertEqual(geocoder.reverseRequests.count, 5)
+    }
+
+    func test_addBookmark_higherPrecisionStillPrefersAdministrativeArea() async {
+        let geocoder = MockLocationGeocoder()
+        geocoder.enqueue(placemarks: [makePlacemark(country: "United States", administrativeArea: "California")])
+        let manager = LocationStateManager(
+            storage: makeStorage(),
+            locationManager: MockLocationManager(authorizationStatus: .denied),
+            geocoder: geocoder,
+            shouldInitializeCoreLocation: false
+        )
+
+        manager.addBookmark("9q8")
+
+        let bookmarkResolved = await waitUntil { manager.bookmarkNames["9q8"] == "California" }
+        XCTAssertTrue(bookmarkResolved)
+        XCTAssertEqual(geocoder.reverseRequests.count, 1)
+    }
+
    private func makeStorage() -> UserDefaults {
        let suiteName = "LocationStateManagerTests-\(UUID().uuidString)"
        let storage = UserDefaults(suiteName: suiteName)!