Merge PR #2273: #2242: reverseGeocode maxDistance, and the 15 000-people limit is documented

crates/engine/src/cmd/geocode.rs

@@ -115,6 +115,12 @@ fn fill(meta: &mut lightcraft_catalog::Meta, place: &lightcraft_geocode::Place,
 fn reverse(s: &mut Session, p: &Value) -> Result<Value> {
     const C: &str = "metadata.reverseGeocode";
     let overwrite = bool_or(p, "overwrite", false);
+    // the place list holds towns of 15 000 people or more: a photo in a smaller one finds the nearest
+    // larger town, which may be another municipality; past this many metres City is left empty (#2242)
+    let max_distance = match p.get("maxDistance") {
+        None | Some(Value::Null) => None,
+        Some(v) => Some(v.as_f64().filter(|m| m.is_finite() && *m > 0.0).ok_or_else(|| bad(C, "`maxDistance` must be a positive number of metres"))?),
+    };
     let ids = s.targets(p)?;
     let todo: Vec<(PhotoId, LatLon)> = ids
         .iter()
@@ -140,14 +146,24 @@ fn reverse(s: &mut Session, p: &Value) -> Result<Value> {
             }
             t.progress(n as u64, todo.len() as u64);
         }
-        let Some(place) = names.reverse(*at) else { continue };
+        let Some(mut place) = names.reverse(*at) else { continue };
+        // too far from the town found: State and Country still hold, City (and below) doesn't
+        let too_far = max_distance.is_some_and(|m| place.distance > m);
+        let nearest_city = too_far.then(|| std::mem::take(&mut place.city));
+        if too_far {
+            place.sublocation.clear();
+        }
         let Some(ph) = s.catalog.photo(*id) else { continue };
         let mut meta = ph.meta.clone();
         fill(&mut meta, &place, overwrite);
         if meta != ph.meta {
             ops.push(Op::SetMeta { id: *id, meta: Box::new(meta) });
         }
-        found.push(json!({"id": id.0, "place": place}));
+        let mut entry = json!({"id": id.0, "place": place});
+        if let Some(city) = nearest_city {
+            entry["nearestCity"] = json!(city);
+        }
+        found.push(entry);
     }
     drop(task);
     let changed = ops.len();
@@ -271,7 +287,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Fill Location from GPS",
             ["Photo"],
             None,
-            "{ids?, overwrite?: bool (also replace place fields that already have text; default false: only empty ones are filled), dryRun?, dataset?: folder} → {looked, found, changed, photos: [{id, place: {sublocation, city, state, country, iso, distance}}], attribution} — fills City / State / Country (and Sublocation when known) from GPS with the offline GeoNames place names (download them first); one undo step",
+            "{ids?, overwrite?: bool (also replace place fields that already have text; default false: only empty ones are filled), dryRun?, dataset?: folder, maxDistance?: metres (past this distance from the town found, City is left empty and the town is given as nearestCity)} → {looked, found, changed, photos: [{id, place: {sublocation, city, state, country, iso, distance}, nearestCity?}], attribution} — fills City / State / Country (and Sublocation when known) from GPS with the offline GeoNames place names (download them first); one undo step. The place names list towns of 15 000 people or more (GeoNames cities15000): a photo taken in a smaller town gets the nearest larger one, up to 50 km away (`distance` says how far); `maxDistance` keeps such a town out of City",
             has_selection,
             reverse
         ),
@@ -343,6 +359,36 @@ mod tests {
         let _ = std::fs::remove_dir_all(&dir);
     }
 
+    /// Issue #2242: the place names hold only towns of 15 000 people or more, so a photo in a
+    /// village finds the nearest larger town. `maxDistance` leaves City empty past that distance
+    /// (State and Country still fill) and says which town it was.
+    #[test]
+    fn max_distance_leaves_a_far_town_out_of_city() {
+        let mut s = Session::with_demo();
+        let ids = s.visible_cloned();
+        let dir = dataset("maxdist");
+        // about 5 km north of Paris (like Portofino and Rapallo in the issue), and right in Paris
+        set_gps(&mut s, ids[0], Some((48.90, 2.35)), "");
+        set_gps(&mut s, ids[1], Some((48.855, 2.35)), "");
+        let r = s.execute("metadata.reverseGeocode", &json!({"ids": [ids[0].0, ids[1].0], "dataset": dir, "maxDistance": 2000})).unwrap();
+        let m0 = s.catalog.photo(ids[0]).unwrap().meta.clone();
+        assert_eq!((m0.city.as_str(), m0.state.as_str(), m0.country.as_str()), ("", "Île-de-France", "France"));
+        assert_eq!(s.catalog.photo(ids[1]).unwrap().meta.city, "Paris", "close enough");
+        let far = r["photos"].as_array().unwrap().iter().find(|p| p["id"] == json!(ids[0].0)).unwrap();
+        assert_eq!((far["place"]["city"].as_str(), far["nearestCity"].as_str()), (Some(""), Some("Paris")), "{far}");
+        assert!(far["place"]["distance"].as_f64().unwrap() > 2000.0);
+        // without it, the nearest town fills City as before
+        s.undo_step().unwrap();
+        s.execute("metadata.reverseGeocode", &json!({"ids": [ids[0].0], "dataset": dir})).unwrap();
+        assert_eq!(s.catalog.photo(ids[0]).unwrap().meta.city, "Paris");
+        // hostile values are refused, not taken as "no limit"
+        for bad in [json!(-1), json!(0), json!("far"), json!(true)] {
+            let e = s.execute("metadata.reverseGeocode", &json!({"ids": [ids[0].0], "dataset": dir, "maxDistance": bad})).unwrap_err();
+            assert!(e.to_string().contains("maxDistance"), "{e}");
+        }
+        let _ = std::fs::remove_dir_all(&dir);
+    }
+
     #[test]
     fn explains_missing_data_and_gps() {
         let mut s = Session::with_demo();

crates/ui-egui/src/panels/right.rs

@@ -771,7 +771,11 @@ fn info(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) {
                 }
             });
             if text_button(ui, "fillLocation", crate::i18n::tr("Fill Location"), false)
-                .on_hover_text(crate::i18n::tr("Fill empty City, State and Country from GPS (offline GeoNames place names)"))
+                .on_hover_text(format!(
+                    "{}\n{}",
+                    crate::i18n::tr("Fill empty City, State and Country from GPS (offline GeoNames place names)"),
+                    crate::i18n::tr("The place names list towns of 15,000 people or more: in a smaller one, City gets the nearest larger town.")
+                ))
                 .clicked()
                 && let Err(e) = app.run("metadata.reverseGeocode", json!({}))
             {

docs/parity-checklist.md

@@ -737,7 +737,7 @@ From `08-lightroom-classic-extras.md` (Classic-only features) and part 2 of `06-
 | LRC-MAP-LOCATIONS | Saved locations | P2 | ⬜ | | |
 | LRC-MAP-TRACKLOG | GPS track logs | P2 | ✅ | `cmd:photo.autoTagTracklog`, `cmd:photo.tagFromTracklog`, `crates/meta/src/gpx.rs` | Photo ▸ Auto-Tag from Tracklog…: a GPX 1.0 / 1.1 track log sets the GPS of the selected photos by capture time — interpolated between the points of a track segment, else the nearest point within `maxGap` (10 min); never across segment breaks. The camera's time zone comes from the photo (Exif offset) or is asked for; photos that already have a location keep it unless `replace`; `dryRun` previews; one undo step. No track drawn on a map (no Map module) |
 | LRC-MAP-FILTER | Location filter bar | P2 | ⬜ | | |
-| LRC-MAP-REVGEO | Reverse geocoding | P2 | 🟡 | `cmd:metadata.reverseGeocode` | offline from GeoNames (City / State / Country, no sublocation); no online provider |
+| LRC-MAP-REVGEO | Reverse geocoding | P2 | 🟡 | `cmd:metadata.reverseGeocode` | offline from GeoNames (City / State / Country, no sublocation); the list holds towns of 15 000 people or more, so a smaller town gets the nearest larger one (the tooltip and command help say so); `maxDistance` leaves City empty past a distance and names the town as `nearestCity` (issue #2242); no online provider |
 | LRC-BOOK-SETTINGS | Book settings | P2 | ⬜ | | |
 | LRC-BOOK-AUTOLAYOUT | Book auto layout | P2 | ⬜ | | |
 | LRC-BOOK-PAGE | Book pages & templates | P2 | ⬜ | | |

docs/whats-new.md

@@ -1,5 +1,13 @@
 # What's new in LightCraft
 
+## 2026-10-18
+
+### Fill Location from GPS: no more neighbouring town in City
+- The offline place names list towns of 15,000 people or more, so a photo taken in a smaller one got the nearest larger
+  town in City, sometimes another municipality (issue #2242). The Fill Location tooltip and the command help now say so,
+  and `metadata.reverseGeocode` takes `maxDistance` (metres): past it, City stays empty (State and Country still fill)
+  and the reply names the town it found as `nearestCity`.
+
 ## 2026-10-14
 
 ### Model downloads time out on a name that doesn't resolve