From df97d2df9bc389f91bd05ae0c97ce1d86e9ae608 Mon Sep 17 00:00:00 2001 From: Vibe Mapper Date: Mon, 24 Aug 2026 09:27:12 -0700 Subject: [PATCH 1/2] =?UTF-8?q?feat:=20result-language=20preference=20?= =?UTF-8?q?=E2=80=94=20return=20Overture=20name=20variants=20(#410)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a `lang` field to the stored preferences() document (2-3 lowercase letters, validated like mode) plus a per-call `lang` override on geocode/geocode_detailed, resolve_place, and place_details — the four name-heavy answer tools this round, matching find_places' own documented scope line (its rows, and resolve_place's place-kind candidates sourced from it, keep primary names). When Overture's names.common carries a language-tagged variant for the matched row, `name` becomes that variant and `name_primary` is added only when it differs from the primary. No variant, or no lang requested, keeps the answer byte-identical to before — never invented or transliterated, only what's actually in the data. Query-cost design: divisions get a new small local lang_names.parquet table, materialized alongside #214's alt-name table from the same names.common scan (one more one-time build pass, not a per-request cost). A lang lookup at request time is one indexed join keyed by the page of result ids, not a scan per row. place_details piggybacks the variant onto its existing single-row places query as one extra selected column, with the same try/except-and-fall-back-to-no-variant convention #373's alt-name tier already uses for the same optional names.common field. Fixture work: tests/fixtures/divisions.parquet already carried real names.common data from #214's exonym corpus, so no change was needed there beyond the new query-layer plumbing. tests/fixtures/places.parquet had no names.common column at all; scripts/build_fixture.py now writes one (empty on every existing row, so no prior test's counts move) plus one new, deliberately isolated fixture row carrying real variants for the place_details tests. Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 16 ++ docs/REFERENCE.md | 8 +- docs/benchmarks-vs.md | 8 +- docs/benchmarks.md | 18 +- scripts/build_fixture.py | 24 ++- src/placeroot/geocode.py | 189 +++++++++++++++++++-- src/placeroot/overture.py | 88 ++++++++-- src/placeroot/preferences.py | 48 ++++++ src/placeroot/resources.py | 3 +- src/placeroot/server.py | 67 ++++++-- tests/fixtures/places.parquet | Bin 21597 -> 22288 bytes tests/test_gers_lookup.py | 4 +- tests/test_lang_preference.py | 305 ++++++++++++++++++++++++++++++++++ tests/test_name_path.py | 8 +- tests/test_poi_name_tiers.py | 10 +- tests/test_preferences.py | 2 + tests/test_progressive.py | 6 +- tests/test_resolve_place.py | 2 +- 18 files changed, 738 insertions(+), 68 deletions(-) create mode 100644 tests/test_lang_preference.py diff --git a/CHANGELOG.md b/CHANGELOG.md index e31b6f9..047bf4b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,22 @@ fixing behavior is patch. ## [Unreleased] ### Added +- Result-language preference (#410, north-star: pelias/pelias#979 and #967; + competitive: Nominatim's `accept-language`). A `lang` field (2-3 lowercase + letters) on the stored `preferences()` document, plus a per-call `lang` + override on `geocode`, `resolve_place`, and `place_details` (per-call + wins). When Overture's `names.common` carries a language-tagged variant + for the matched row, `name` becomes that variant and `name_primary` is + added only when it differs from the primary — never invented or + transliterated, and never a payload change when no variant exists or no + `lang` is given. Divisions get a new small local `lang_names.parquet` + table (materialized alongside #214's alt-name table, from the same + `names.common` scan) so a lang lookup is one indexed join by the page of + result ids, not a second scan; `place_details` piggybacks the variant + onto its existing single-row places query as one extra column. Scoped to + the four name-heavy answer tools this round — `find_places` rows (and, + through it, `resolve_place`'s place-kind candidates) keep primary names, + matching `find_places`' own documented scope line. - Declared MCP `outputSchema` on all 42 tools (docs/ROADMAP.md §4 feature 3 / §5.3) — the one MCP-conformance gap docs/benchmarks-vs.md conceded to Mapbox is closed. Hand-authored, not derived: every tool returns a bare, diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index f75fabe..5a54c7e 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -51,7 +51,7 @@ Every tool returns a compact, budgeted answer. Several single-item tools have a | `geometry_op` | Offline geometry math and predicates behind one `op` catalog: point distance/bearing/destination/midpoint, area/length/bbox/centroid, buffer/convex hull (returns geometry), point-in-polygon, nearest point, nearest point on a line | | `warmup_city` | Pre-cache a city's places and transportation tiles so later place searches over it read locally — does not build the street graph or cache buildings. A first tile COPY without `confirm` returns `needs_confirm` | | `data_version` | Which Overture release the answers are drawn from | -| `preferences` | Read or update local travel/household defaults (mode, pace, dog, …). Nothing leaves the machine | +| `preferences` | Read or update local travel/household defaults (mode, pace, dog, result language, …). Nothing leaves the machine | ## Confirming a slow hop @@ -123,7 +123,7 @@ context, so you can pin them into a conversation without spending a tool call: | Resource | Contents | |---|---| | `placeroot://data-version` | The resolved Overture release, its date, how it was resolved (discovery, env override, the pinned fallback, or held at the artifact release), its age, and whether the bundled acceleration applies to it. Same values the `data_version` tool returns — one shared code path, so they cannot drift. | -| `placeroot://preferences` | Local travel and household preferences (mode, pace, household). Same document the `preferences` tool reads and updates — one shared code path. Nothing in this file leaves the machine. | +| `placeroot://preferences` | Local travel and household preferences (mode, pace, household, result-language `lang`). Same document the `preferences` tool reads and updates — one shared code path. Nothing in this file leaves the machine. | | `placeroot://categories` | Summary of the place-category taxonomy: all 22 top-level categories with how many slugs sit under each, plus how to get an exact slug. ~530 tokens — a summary, not the 2,117-slug CSV, which stays behind `search_categories`. | In Claude Code they auto-complete as @-mentions: @@ -200,7 +200,9 @@ union of everything named: `data_version` and `preferences` are registered under every profile. `data_version` is ~230 tokens and the only way an agent can tell which Overture release backs its answers. `preferences` is the local defaults -document; routing tools read its stored mode when theirs is omitted. +document; routing tools read its stored mode when theirs is omitted, and +`geocode`, `resolve_place`, and `place_details` read its stored `lang` +(#410) the same way. Profiles may overlap, and a list may mix them with bare tool names — `PLACEROOT_TOOLS=routing,find_places` or diff --git a/docs/benchmarks-vs.md b/docs/benchmarks-vs.md index bf138e4..ba27d08 100644 --- a/docs/benchmarks-vs.md +++ b/docs/benchmarks-vs.md @@ -141,9 +141,9 @@ Regenerate with `uv run python benchmarks/competitor_comparison.py --write`. Eve - Snapshots captured: **2026-08-08** - PlaceRoot's own answers were captured on **macOS-26.3.1-arm64-arm-64bit** (Python 3.11.15, Overture `2026-08-19.0`) and are snapshotted rather than recomputed here: floating-point differences in routing and geometry change digit counts between platforms, so a live rerun costs a few tokens more or less on Linux than on macOS. A tolerance test reruns them for real and fails if this snapshot drifts from what the code now answers. - Schema figures are counted twice: over the **common fields** every server here publishes, and **verbatim** over everything it sends. `outputSchema` is one of the common fields now that both sides declare one (roadmap §4.3) — the ratios below are the common-field ones; see the note under the table for the size gap that remains. -- Schema surface, whole install (common fields): PlaceRoot **31878** tokens (42 tools) · Mapbox **28958** (29 tools) · Google Maps **655** (7 tools) -- Schema surface, the six tools each server needs for the scenarios below (common fields): PlaceRoot **7396** · Mapbox **13069** (1.8x ours) · Google Maps **500** (5 tools — no isochrone tool exists) -- Whole-install surface: Mapbox is **0.9x** PlaceRoot's on common fields (both sides' output schemas included), on 29 tools against 42. Verbatim — counting the rest of what each side sends beyond the common fields — it is 0.9x (29295 against 31878), and 1.8x on the six-tool subset. +- Schema surface, whole install (common fields): PlaceRoot **32503** tokens (42 tools) · Mapbox **28958** (29 tools) · Google Maps **655** (7 tools) +- Schema surface, the six tools each server needs for the scenarios below (common fields): PlaceRoot **7576** · Mapbox **13069** (1.7x ours) · Google Maps **500** (5 tools — no isochrone tool exists) +- Whole-install surface: Mapbox is **0.9x** PlaceRoot's on common fields (both sides' output schemas included), on 29 tools against 42. Verbatim — counting the rest of what each side sends beyond the common fields — it is 0.9x (29295 against 32503), and 1.7x on the six-tool subset. - Answers, over the 6 scenarios both PlaceRoot and Mapbox answer: PlaceRoot **1685** tokens total, Mapbox **1337** (1219 with pretty-print whitespace removed) ### Where the competitor numbers come from @@ -157,7 +157,7 @@ Regenerate with `uv run python benchmarks/competitor_comparison.py --write`. Eve | server | tools registered | whole install, verbatim | whole install, common fields | the 6-scenario subset | subset verbatim | subset common fields | |---|---:|---:|---:|---:|---:|---:| -| PlaceRoot | 42 | 31878 | **31878** | 6 | 7396 | **7396** | +| PlaceRoot | 42 | 32503 | **32503** | 6 | 7576 | **7576** | | Mapbox MCP | 29 | 29295 | **28958** | 6 | 13129 | **13069** | | Google Maps MCP (archived) | 7 | 655 | **655** | 5 | 500 | **500** | diff --git a/docs/benchmarks.md b/docs/benchmarks.md index 03e7392..1515ead 100644 --- a/docs/benchmarks.md +++ b/docs/benchmarks.md @@ -77,11 +77,11 @@ Generated 2026-08-24 by `uv run python benchmarks/token_efficiency.py --write`. - Token counting method: **chars/4 heuristic (no tokenizer installed; same estimator as placeroot.budget.estimate_tokens)** - Overture release pinned for the fixture run: `2026-08-19.0` - Tools registered: **42** -- Total schema surface: **31878 tokens** (127575 chars, 127915 bytes) -- Of which inputSchema: **5101 tokens**, outputSchema: **3891 tokens** (roadmap §4.3, #403) — the rest is names/descriptions/annotations -- Schema cost per tool: min 261, median 651, max 2971 tokens +- Total schema surface: **32503 tokens** (130079 chars, 130434 bytes) +- Of which inputSchema: **5381 tokens**, outputSchema: **3891 tokens** (roadmap §4.3, #403) — the rest is names/descriptions/annotations +- Schema cost per tool: min 261, median 680, max 2971 tokens - Median scenario response: **436 tokens** (range 87-540) -- Break-even: the schema surface costs about as much as **73 median answers** +- Break-even: the schema surface costs about as much as **75 median answers** ### Schema surface (paid once per conversation) @@ -92,14 +92,14 @@ Generated 2026-08-24 by `uv run python benchmarks/token_efficiency.py --write`. | `changes_in_area` | 1161 | 212 | 52 | **1515** | 6060 | | `optimize_route` | 799 | 132 | 324 | **1342** | 5371 | | `meeting_point` | 1018 | 106 | 52 | **1265** | 5063 | +| `resolve_place` | 682 | 182 | 206 | **1151** | 4606 | | `compare_areas` | 885 | 105 | 52 | **1122** | 4489 | | `travel_time_matrix` | 707 | 139 | 173 | **1099** | 4399 | +| `geocode` | 667 | 114 | 195 | **1048** | 4193 | | `suggest_areas` | 803 | 100 | 52 | **1039** | 4156 | -| `resolve_place` | 601 | 112 | 206 | **997** | 3989 | | `verify_claims` | 781 | 68 | 52 | **989** | 3957 | | `geometry_op` | 559 | 290 | 52 | **972** | 3891 | | `from_to` | 450 | 213 | 227 | **957** | 3830 | -| `geocode` | 559 | 44 | 195 | **868** | 3473 | | `render_map` | 597 | 136 | 52 | **857** | 3430 | | `isochrone` | 458 | 195 | 127 | **848** | 3393 | | `places_along_route` | 513 | 198 | 52 | **829** | 3316 | @@ -107,12 +107,13 @@ Generated 2026-08-24 by `uv run python benchmarks/token_efficiency.py --write`. | `infrastructure_at` | 494 | 118 | 52 | **738** | 2953 | | `ground_location` | 463 | 150 | 52 | **734** | 2937 | | `distance_matrix` | 401 | 93 | 136 | **698** | 2795 | +| `place_details` | 342 | 228 | 52 | **680** | 2723 | | `find_near` | 242 | 126 | 222 | **651** | 2606 | | `within_distance` | 361 | 156 | 52 | **637** | 2549 | | `geocode_address` | 407 | 108 | 52 | **630** | 2522 | | `address_at` | 427 | 54 | 52 | **597** | 2388 | | `gers_lookup` | 385 | 74 | 52 | **575** | 2303 | -| `place_details` | 265 | 158 | 52 | **532** | 2130 | +| `preferences` | 206 | 216 | 52 | **532** | 2131 | | `neighborhood_verdict` | 200 | 152 | 52 | **462** | 1849 | | `geocode_batch` | 211 | 58 | 98 | **430** | 1722 | | `land_use_at` | 274 | 41 | 52 | **429** | 1717 | @@ -120,7 +121,6 @@ Generated 2026-08-24 by `uv run python benchmarks/token_efficiency.py --write`. | `resolve_place_batch` | 248 | 42 | 52 | **406** | 1626 | | `elevation_at` | 242 | 41 | 52 | **394** | 1579 | | `warmup_city` | 175 | 109 | 52 | **391** | 1565 | -| `preferences` | 135 | 146 | 52 | **389** | 1557 | | `summarize_area` | 148 | 105 | 52 | **365** | 1460 | | `data_version` | 166 | 16 | 119 | **355** | 1421 | | `buildings_at` | 151 | 91 | 52 | **349** | 1399 | @@ -129,7 +129,7 @@ Generated 2026-08-24 by `uv run python benchmarks/token_efficiency.py --write`. | `simplify_geometry` | 129 | 58 | 52 | **297** | 1188 | | `admin_lookup` | 145 | 41 | 52 | **296** | 1185 | | `reverse_geocode` | 111 | 42 | 52 | **261** | 1044 | -| **all 42 tools** | 19998 | 5101 | 3891 | **31878** | 127575 | +| **all 42 tools** | 20335 | 5381 | 3891 | **32503** | 130079 | ### Response cost (paid per tool call, measured on committed fixtures) diff --git a/scripts/build_fixture.py b/scripts/build_fixture.py index df22218..58cc472 100644 --- a/scripts/build_fixture.py +++ b/scripts/build_fixture.py @@ -91,12 +91,19 @@ def build_place_rows() -> list[tuple]: def add( name, lat, lon, category, basic_category, status, confidence, alternates=None, addresses=None, websites=None, phones=None, socials=None, brand=None, sources=None, + common=None, ): + # #410: `common` mirrors Overture's names.common on a places row — + # written for every row (empty when there are no language-tagged + # variants) so the fixture's `names` struct has the same shape the + # real places theme does and place_details'/resolve_place's lang + # lookup runs against it unchanged, the same convention + # build_geocode_fixture.py's divisions `common` column already uses. index = len(rows) rows.append(( gers_id(index), {"xmin": lon, "ymin": lat, "xmax": lon, "ymax": lat}, - {"primary": name}, + {"primary": name, "common": common or {}}, {"primary": category, "alternates": alternates or []}, basic_category, status, @@ -190,6 +197,19 @@ def add( add("Dateline East", 10.0, -179.98, "restaurant", "restaurant", "open", 0.7) add("Dateline East Bank", 9.999, -179.985, "bank", "bank", "open", 0.65) + # #410: a place carrying names.common language variants, so place_details' + # and resolve_place's lang lookup has a real one to swap in. Appended at + # the end (not spliced into the dense cluster, so no earlier row's GERS + # id — derived from its insertion index — shifts) AND at an isolated + # coordinate well clear of CENTER_LAT/CENTER_LON, HIGH_LAT_CENTER, the + # antimeridian cluster and (0.0, 0.0): several tests assert exact place + # counts within a radius of those points (e.g. radius_m=100 at CENTER), + # and a new row inside any of those windows would silently change them. + add( + "Kaffeehaus Wien", 5.0, 100.0, "coffee_shop", "coffee_shop", "open", 0.8, + common={"en": "Vienna Coffee House", "fr": "Maison du Cafe Viennois"}, + ) + return rows @@ -414,7 +434,7 @@ def build_places(con: duckdb.DuckDBPyConnection) -> None: CREATE TABLE places ( id VARCHAR, bbox STRUCT(xmin DOUBLE, ymin DOUBLE, xmax DOUBLE, ymax DOUBLE), - names STRUCT("primary" VARCHAR), + names STRUCT("primary" VARCHAR, common MAP(VARCHAR, VARCHAR)), taxonomy STRUCT("primary" VARCHAR, alternates VARCHAR[]), basic_category VARCHAR, operating_status VARCHAR, diff --git a/src/placeroot/geocode.py b/src/placeroot/geocode.py index 7b08d4a..50a0a1d 100644 --- a/src/placeroot/geocode.py +++ b/src/placeroot/geocode.py @@ -513,6 +513,14 @@ def _kick_autowarm(hit: dict | None) -> None: _DIVISIONS_TABLE_FILENAME = "table.parquet" # #214: the alternate-name table, written alongside the primary one. _ALT_NAMES_TABLE_FILENAME = "alt_names.parquet" +# #410: the language-tagged name table, written alongside the primary one. +# Distinct from the alt-name table above: that one folds/dedupes +# names.common down to spellings for *searching* and discards which +# language each came from; this one keeps (id, lang) intact for *serving* +# the caller's requested language back — the two answer different +# questions off the same source column, so keeping them separate tables +# means neither has to carry columns the other's query pattern doesn't use. +_LANG_NAMES_TABLE_FILENAME = "lang_names.parquet" # #224: the bbox columns carried by the materialized divisions table. Named # with a bbox_ prefix rather than reusing the struct so the stale-cache check @@ -899,6 +907,41 @@ def _materialize_alt_names_table(path: Path, glob: str) -> None: _publish_copied_parquet(con, tmp_path, path) +def _materialize_lang_names_table(path: Path, glob: str) -> None: + """COPY the #410 language-tagged name table — one row per (division id, + names.common language key) — into a local parquet at `path`. + + Reads the same `names.common` map the #214 alt-name table (above) reads, + but keeps the map key (the language code) instead of discarding it, and + does not fold/dedupe by spelling — a lang lookup wants "the row for this + exact id and this exact language", not "some spelling near this string". + One extra pass over names.common at materialization time (a one-time + cost, alongside the alt-name table's own pass); the per-request cost + this buys is a single indexed join by (id, lang) against a small local + parquet — see _lang_variants_for, not a second scan of Overture data. + + Raises duckdb.Error if names.common isn't there or isn't a map — same + convention as _materialize_alt_names_table; the caller treats that as + "no lang table" and geocode answers with primary names only. + """ + path.parent.mkdir(parents=True, exist_ok=True) + tmp_path = path.with_suffix(".parquet.tmp") + con = overture._new_connection() + sql = f""" + COPY ( + SELECT id, lower(entry.key) AS lang, entry.value AS name + FROM ( + SELECT id, unnest(map_entries(names.common)) AS entry + FROM read_parquet('{glob}', hive_partitioning=1) + WHERE names.common IS NOT NULL + ) + WHERE entry.value IS NOT NULL AND entry.value <> '' + ) TO '{tmp_path}' (FORMAT PARQUET, COMPRESSION ZSTD) + """ + con.execute(sql) + _publish_copied_parquet(con, tmp_path, path) + + def _is_remote_glob(glob: str) -> bool: return glob.startswith(("s3://", "http://", "https://")) @@ -948,6 +991,7 @@ def _materialize_divisions_table(path: Path, glob: str) -> None: else: _materialize_divisions_pass(path, glob, with_hierarchies=True) _try_materialize_alt_names_table(path.with_name(_ALT_NAMES_TABLE_FILENAME), glob) + _try_materialize_lang_names_table(path.with_name(_LANG_NAMES_TABLE_FILENAME), glob) def _materialize_divisions_pass(path: Path, glob: str, with_hierarchies: bool) -> None: @@ -1066,6 +1110,7 @@ def _upgrade_divisions_table(path: Path, glob: str) -> None: t0 = time.time() _materialize_divisions_pass(path, glob, with_hierarchies=True) _try_materialize_alt_names_table(path.with_name(_ALT_NAMES_TABLE_FILENAME), glob) + _try_materialize_lang_names_table(path.with_name(_LANG_NAMES_TABLE_FILENAME), glob) _stage1_sentinel(path).unlink(missing_ok=True) logger.info( "divisions table upgraded with admin chains in %.1fs -> %s", @@ -1108,6 +1153,79 @@ def _try_materialize_alt_names_table(alt_path: Path, glob: str) -> None: ) +_LANG_BUILD_ATTEMPTED: set[str] = set() + + +def _try_materialize_lang_names_table(lang_path: Path, glob: str) -> None: + """Build the #410 lang table, logging and swallowing any failure — same + reasoning as _try_materialize_alt_names_table: losing this table means + geocode/resolve_place/place_details answer with primary names only, + which is a much smaller loss than failing the call that triggered the + build. + """ + t0 = time.time() + try: + _materialize_lang_names_table(lang_path, glob) + except (duckdb.Error, overture.UpstreamUnavailable, OSError) as e: + logger.warning( + "language-tagged name table materialization failed, lang lookups " + "will find no variant: %s", e, + ) + return + logger.info( + "language-tagged name table materialized in %.1fs -> %s", time.time() - t0, lang_path + ) + + +def _local_lang_names_table(local_table: str | None) -> str | None: + """Path to the #410 language-tagged name table sitting beside + `local_table`, or None if there isn't one — same "no lang variants + available" convention as _local_alt_names_table's None (cache off, a + cache directory predating #410, a dataset without names.common, or a + failed best-effort build). + """ + if local_table is None: + return None + path = Path(local_table).with_name(_LANG_NAMES_TABLE_FILENAME) + if path.exists(): + return str(path) + key = str(path) + if key not in _LANG_BUILD_ATTEMPTED: + _LANG_BUILD_ATTEMPTED.add(key) + logger.info("no language-tagged name table at %s (cache predates #410); building it", path) + _try_materialize_lang_names_table( + path, overture.upstream_glob(theme="divisions", type_="division") + ) + if path.exists(): + return str(path) + return None + + +def _lang_variants_for(lang_table: str | None, ids: list[str], lang: str) -> dict[str, str]: + """{id: variant name} for every id in `ids` that has a names.common entry + under `lang` in `lang_table`. One indexed lookup for the whole batch of + result rows — not one query per row — so applying #410's lang preference + to a page of results costs one extra small local-parquet scan, not N. + + Empty dict (never an error) when there's no lang table, no ids, or the + id/lang combination just isn't present — all of which mean the same + thing to the caller: no variant, primary name stands. + """ + if not lang_table or not ids: + return {} + sql = f""" + SELECT id, name + FROM read_parquet('{lang_table}') + WHERE lang = $lang AND id IN (SELECT unnest($ids)) + """ + try: + with overture._conn_lock: + rows = overture.conn().execute(sql, {"lang": lang, "ids": ids}).fetchall() + except duckdb.Error: + return {} + return dict(rows) + + def _local_alt_names_table(local_table: str | None) -> str | None: """Path to the #214 alternate-name table sitting beside `local_table`, or None if there isn't one. @@ -1620,19 +1738,23 @@ def _well_known_city_near(row: dict, query: str) -> int: def _resolve_cache_key( query: str, city: str | None, near_lat: float | None, near_lon: float | None, + lang: str | None = None, ) -> tuple: near = ( (round(near_lat, 3), round(near_lon, 3)) if near_lat is not None and near_lon is not None else (None, None) ) - return (_fold_query_key(query), _fold_query_key(city) if city else "", *near) + # #410: lang is part of the key — otherwise a lang="de" call would replay + # a cache entry another lang (or no lang at all) already populated. + return (_fold_query_key(query), _fold_query_key(city) if city else "", *near, lang or "") def _resolve_cache_get( query: str, city: str | None, near_lat: float | None, near_lon: float | None, + lang: str | None = None, ) -> list[dict] | None: - key = _resolve_cache_key(query, city, near_lat, near_lon) + key = _resolve_cache_key(query, city, near_lat, near_lon, lang) with _resolve_lru_lock: rows = _resolve_lru.get(key) if rows is None: @@ -1647,8 +1769,9 @@ def _resolve_cache_put( near_lat: float | None, near_lon: float | None, rows: list[dict], + lang: str | None = None, ) -> None: - key = _resolve_cache_key(query, city, near_lat, near_lon) + key = _resolve_cache_key(query, city, near_lat, near_lon, lang) stored = [dict(r) for r in rows] with _resolve_lru_lock: _resolve_lru[key] = stored @@ -3103,9 +3226,9 @@ def _postcode_empty_note(display: str) -> str: ) -def geocode(query: str, limit: int = DEFAULT_LIMIT) -> list[dict]: +def geocode(query: str, limit: int = DEFAULT_LIMIT, lang: str | None = None) -> list[dict]: """Free-text place name -> ranked candidates. See geocode_detailed.""" - return geocode_detailed(query, limit)["results"] + return geocode_detailed(query, limit, lang=lang)["results"] def geocode_batch(queries: list[str], limit_per_query: int = 3) -> list[dict]: @@ -3152,6 +3275,7 @@ def geocode_detailed( *, local_table: str | None = None, alt_table: str | None = None, + lang: str | None = None, ) -> dict: """Free-text place name -> ranked candidates, from Overture divisions (and places fallback). @@ -3193,6 +3317,16 @@ def geocode_detailed( runner-up anchor sitting in a different country than the top candidate ("London" -> London, Ontario for a UK query) and cannot do that from admin_context alone once a row's chain is empty. + + lang (#410, a 2-3 letter code) requests Overture's language-tagged + names.common variant for each division-kind row, when one exists for + that id and language: `name` becomes the variant and the primary is + added back as `name_primary` only when it differs — piggybacked as one + extra lookup keyed by the batch of result ids, not a scan per row (see + _lang_variants_for). Places-fallback rows (a row's `_category` is set) + are unaffected — same scope line find_places itself draws this round. + No lang given, or no variant found for a row, leaves it byte-identical + to the no-lang answer. """ query = query.strip() limit = max(1, min(limit, MAX_LIMIT)) @@ -3558,6 +3692,18 @@ def _rank_place(r): recalled.sort(key=lambda r: _rank_key(r, search_query, region_population)) candidates = recalled + # #410: one lookup for the whole page of division-kind rows about to be + # returned, keyed by the ids actually surviving [:limit] — not a scan per + # row, and not run at all unless a lang was actually requested. Places- + # fallback rows (row["_category"] is set) are excluded: they come from a + # different theme/id-space than the divisions lang table indexes, same + # scope line find_places draws this round (see the docstring above). + lang_variants: dict[str, str] = {} + if lang: + page = candidates[:limit] + division_ids = [r["id"] for r in page if r.get("id") and r.get("_category") is None] + lang_variants = _lang_variants_for(_local_lang_names_table(local_table), division_ids, lang) + out = [] for row in candidates[:limit]: entry = { @@ -3592,6 +3738,14 @@ def _rank_place(r): # from a name that doesn't contain the query, and call a # correction a "substring" match. entry["matched_by"] = "fuzzy" + variant = lang_variants.get(row["id"]) if row.get("id") else None + if variant and variant != entry["name"]: + # #410: never invent or transliterate — variant only ever comes + # from Overture's own names.common map, looked up above. + # name_primary appears only when it actually differs, so a + # no-variant answer stays byte-identical to the no-lang one. + entry["name_primary"] = entry["name"] + entry["name"] = variant out.append(entry) if out: _kick_autowarm(out[0]) @@ -3916,6 +4070,7 @@ def resolve_place( near_lon: float | None = None, limit: int = 3, city: str | None = None, + lang: str | None = None, ) -> list[dict]: """Free-text place reference -> ranked, typed GERS ids an agent can hold onto. @@ -3952,6 +4107,15 @@ def resolve_place( fails after retries, or overture.SchemaDegraded if the places dataset is missing bbox — the caller (server.py) turns either into a structured error like every other tool. + + lang (#410) requests Overture's language-tagged names.common variant, + the same as geocode() — but only for "kind": "division" candidates + (threaded through the internal geocode() call this function already + makes). "kind": "place" candidates come from find_places, which is out + of scope for #410 this round (the same scope line find_places' own + docstring draws), so they always carry their primary name. A division + candidate's `name_primary` is present under the same rule as + geocode()'s: only when the variant actually differs from the primary. """ query = query.strip() limit = max(1, min(limit, MAX_LIMIT)) @@ -3983,7 +4147,7 @@ def resolve_place( city_bounded = True cache_city, cache_lat, cache_lon = city, near_lat, near_lon - cached = _resolve_cache_get(query, cache_city, cache_lat, cache_lon) + cached = _resolve_cache_get(query, cache_city, cache_lat, cache_lon, lang) if cached is not None: return cached[:limit] @@ -4023,7 +4187,7 @@ def resolve_place( # Search the place half when we stripped a trailing city, so # "Colosseo Roma" does not return Rome the city as the pin. search_query = place_query if city_bounded and place_query != query else query - geocode_hits = geocode(search_query, limit=_RESOLVE_OVERFETCH) + geocode_hits = geocode(search_query, limit=_RESOLVE_OVERFETCH, lang=lang) if city_bounded and near_lat is not None and near_lon is not None: geocode_hits = [ r for r in geocode_hits @@ -4128,14 +4292,19 @@ def resolve_place( if not r["id"] or r["id"] in seen_ids: continue seen_ids.add(r["id"]) - candidates.append({ + candidate = { "id": r["id"], "kind": "division", "name": r["name"], "lat": r["lat"], "lon": r["lon"], "type": r.get("type"), "admin_context": r["admin_context"], "match": _division_match_label(r, query, search_query), "_prominence": r["rank_score"], - }) + } + if r.get("name_primary"): + # #410: geocode()'s own lang enrichment already applied — just + # carry it through the merge rather than re-deriving it. + candidate["name_primary"] = r["name_primary"] + candidates.append(candidate) for r in place_rows: if not r["id"] or r["id"] in seen_ids or not r["name"]: continue @@ -4225,7 +4394,7 @@ def _rank_candidate(c): for c in candidates: del c["_prominence"] out = candidates[:limit] - _resolve_cache_put(query, cache_city, cache_lat, cache_lon, out) + _resolve_cache_put(query, cache_city, cache_lat, cache_lon, out, lang) if out: _remember_last_city(city, out[0]) _kick_autowarm(out[0]) diff --git a/src/placeroot/overture.py b/src/placeroot/overture.py index e775ae3..0d558cb 100644 --- a/src/placeroot/overture.py +++ b/src/placeroot/overture.py @@ -2073,6 +2073,7 @@ def summarize_area(lat: float, lon: float, radius_m: float = 1000) -> dict: def _place_details_sql( from_source: str, filters: list[str], order_by: str, missing: set[str], has_recreation: bool = False, with_clause: str = "", + lang: str | None = None, ) -> str: """The shared place_details SELECT, sourced from from_source. @@ -2083,19 +2084,29 @@ def _place_details_sql( caller can label a recreation-layer row's provenance (source_theme: base) — and so gers.py can name the right theme for its id; it reads FALSE whenever the union isn't active and the column doesn't exist. + + #410: when `lang` is given, one extra trailing column + (names.common[$lang]) is appended to this SAME SELECT — no second scan, + just one more field read off the row already being fetched. names.common + is an optional nested field probe_schema can't see (same caveat as + #373's tier 2), so a dataset/theme row without it fails the query at + bind time; the caller (_run_place_details_query) catches that and + re-issues the query without this column, same convention as the #373 + alt-name fallback. """ select_list = ",\n ".join( f'{"NULL" if col in missing else expr} AS {alias}' for col, expr, alias in _PLACE_DETAIL_COLUMNS ) marker_expr = recreation.MARKER_COLUMN if has_recreation else "FALSE" + lang_col = ",\n names.common[$lang] AS _lang_variant" if lang is not None else "" return f""" {with_clause} SELECT {select_list}, round(bbox.ymin, 6) AS lat, round(bbox.xmin, 6) AS lon, - {marker_expr} AS {recreation.MARKER_COLUMN} + {marker_expr} AS {recreation.MARKER_COLUMN}{lang_col} FROM {from_source} WHERE {" AND ".join(filters)} ORDER BY {order_by} @@ -2106,19 +2117,45 @@ def _place_details_sql( def _run_place_details_query(from_source: str, filters: list[str], order_by: str, params: dict, missing: set[str], has_recreation: bool = False, - with_clause: str = "") -> tuple | None: + with_clause: str = "", + lang: str | None = None) -> tuple[tuple | None, str | None]: + """Runs the place_details SELECT, returning (row, lang_variant). + + #410: lang_variant is the requested-language name.common variant for + this row, piggybacked onto the same query lang triggers — not a second + scan. When lang is given, the lang-carrying query is tried first; a + duckdb.Error there (names.common absent from this dataset/theme row + shape) falls back to the plain query with lang_variant left None, the + same "best-effort, never fails the call" convention #373's alt-name + tier and #214's alt-name table use for the same optional field. A + successful lang query that simply finds no row returns (None, None) + without a second query — that emptiness is real, not a schema miss. + """ + if lang is not None: + lang_sql = _place_details_sql(from_source, filters, order_by, missing, + has_recreation, with_clause, lang=lang) + try: + with _conn_lock: + row = _conn().execute(lang_sql, {**params, "lang": lang}).fetchone() + except duckdb.Error: + row = None + else: + if row is None: + return None, None + return row[:-1], row[-1] sql = _place_details_sql(from_source, filters, order_by, missing, has_recreation, with_clause) try: with _conn_lock: - return _conn().execute(sql, params).fetchone() + return _conn().execute(sql, params).fetchone(), None except duckdb.Error as e: raise UpstreamUnavailable(str(e)) from e def _place_details_by_id(id: str, near_lat: float | None, near_lon: float | None, upstream: str, missing: set[str], - bound_to_hint: bool = False) -> tuple | None: + bound_to_hint: bool = False, + lang: str | None = None) -> tuple[tuple | None, str | None]: """Resolve a GERS id, cheapest source first (issue #41). 1. Whatever tiles the local cache already has on disk — no upstream @@ -2151,34 +2188,36 @@ def _place_details_by_id(id: str, near_lat: float | None, near_lon: float | None if tile_paths: joined = ", ".join(f"'{p}'" for p in tile_paths) source, active = _with_recreation(f"read_parquet([{joined}])", None) - row = _run_place_details_query( - source, ["id = $id"], "1", {"id": id}, missing, active, + row, lang_variant = _run_place_details_query( + source, ["id = $id"], "1", {"id": id}, missing, active, lang=lang, ) if row is not None: - return row + return row, lang_variant if near_lat is not None and near_lon is not None: xmin, ymin, xmax, ymax = _bbox_around(near_lat, near_lon, ID_HINT_RADIUS_M) bbox_filter, bbox_params = _bbox_filter_sql(xmin, ymin, xmax, ymax) params = {"id": id, **bbox_params} from_source, active = _places_source((xmin, ymin, xmax, ymax)) - row = _run_place_details_query(from_source, ["id = $id", bbox_filter], "1", - params, missing, active) + row, lang_variant = _run_place_details_query(from_source, ["id = $id", bbox_filter], "1", + params, missing, active, lang=lang) if row is not None: - return row + return row, lang_variant if bound_to_hint: logger.info( "place_details(id=%s): near-hint missed and the lookup is hint-bounded; " "not falling back to a full-dataset scan", id, ) - return None + return None, None logger.warning( "place_details(id=%s) fell back to a full-dataset scan (no cache hit, " "no near_lat/near_lon hint given, or the hint missed) — issue #41", id, ) source, active = _with_recreation(f"read_parquet('{upstream}', hive_partitioning=1)", None) - return _run_place_details_query(source, ["id = $id"], "1", {"id": id}, missing, active) + return _run_place_details_query( + source, ["id = $id"], "1", {"id": id}, missing, active, lang=lang, + ) def place_details( @@ -2190,6 +2229,7 @@ def place_details( near_lat: float | None = None, near_lon: float | None = None, bound_to_hint: bool = False, + lang: str | None = None, ) -> dict | None: """One place, in full: resolved by GERS id, or by name + a nearby point. @@ -2208,6 +2248,15 @@ def place_details( hint that misses returns None rather than scanning the dataset — see _place_details_by_id. + lang (#410, a 2-3 letter code) requests Overture's language-tagged + name.common variant for this row instead of its primary name, when one + exists — piggybacked onto the same query already reading `names`, not a + second scan. `name` becomes the variant; the primary is added back as + `name_primary` only when it differs. No variant for the requested + language (or a dataset/theme row with no names.common at all) leaves + `name` as the primary with no `name_primary` and no note — see + _run_place_details_query. + Raises SchemaDegraded if bbox is missing (needed for the name+point path; an id lookup doesn't strictly need it, but the schema probe doesn't distinguish the two calls) or UpstreamUnavailable if the remote @@ -2222,7 +2271,9 @@ def place_details( missing = set(_check_schema(upstream)) if id: - row = _place_details_by_id(id, near_lat, near_lon, upstream, missing, bound_to_hint) + row, lang_variant = _place_details_by_id( + id, near_lat, near_lon, upstream, missing, bound_to_hint, lang=lang, + ) else: bbox_filter, distance_filter, params, bbox, _radius_m = area_geometry( lat, lon, radius_m @@ -2236,8 +2287,8 @@ def place_details( # both themes, details should come from the richer places row, not # from whichever polygon centroid happened to sit closer. with_clause, from_clause, filters = _dedup_rows_sql(from_source, filters, has_recreation) - row = _run_place_details_query(from_clause, filters, _DISTANCE_EXPR, params, - missing, has_recreation, with_clause) + row, lang_variant = _run_place_details_query(from_clause, filters, _DISTANCE_EXPR, params, + missing, has_recreation, with_clause, lang=lang) if row is None: return None @@ -2246,6 +2297,13 @@ def place_details( # Truthful provenance for a recreation-layer row (docs/RECREATION.md); # absent for ordinary places rows, whose theme is the tool's default. result["source_theme"] = "base" + if lang_variant and lang_variant != result.get("name"): + # #410: never invent or transliterate — lang_variant only ever comes + # from Overture's own names.common map. name_primary appears only + # when it actually differs, so a no-variant / same-spelling answer + # stays byte-identical to the no-lang call. + result["name_primary"] = result["name"] + result["name"] = lang_variant _annotate_place(result) for field in _PLACE_DETAIL_LIST_FIELDS: kept, omitted = budget.truncate_list(result[field]) diff --git a/src/placeroot/preferences.py b/src/placeroot/preferences.py index d7b0d9a..d1b2241 100644 --- a/src/placeroot/preferences.py +++ b/src/placeroot/preferences.py @@ -6,6 +6,12 @@ when theirs is omitted. An explicit argument always wins. pace and household are stored for later features; they do not change answers yet. +#410: `lang` is a stored result-language preference (a 2-3 letter code, +e.g. "de", "fra") that geocode/geocode_detailed, resolve_place, and +place_details consult when their own per-call `lang` is omitted — see +resolve_lang. Unlike pace/household it does change answers, the same way +mode does for routing. + Nothing leaves the machine: the file lives under the user's config directory (or PLACEROOT_PREFERENCES_PATH) and is never sent upstream. """ @@ -14,6 +20,7 @@ import json import os +import re import threading from collections.abc import Iterator from contextlib import contextmanager @@ -28,8 +35,24 @@ DEFAULT_MODE_ISOCHRONE = "walk" DEFAULT_MODE_ROUTE = "drive" +# #410: a language code stored/passed for the result-language preference. +# 2-3 lowercase letters covers both ISO 639-1 ("de", "en") and 639-2/3 +# ("fra", "yue") — Overture's names.common keys are themselves BCP-47-ish +# short codes, not validated against a fixed registry, so this is a shape +# check (reject obvious junk) rather than a membership check against a +# closed list of "supported" languages the way MODES is. +_LANG_PATTERN = re.compile(r"^[a-z]{2,3}$") + ENV_PATH = "PLACEROOT_PREFERENCES_PATH" + +def is_valid_lang(value: str) -> bool: + """Shape check for a #410 language code — 2-3 lowercase letters after + stripping/lowercasing. Public so server.py's preferences() tool can + reject junk before it ever reaches update()/_normalize, the same way + it checks mode against MODES.""" + return bool(_LANG_PATTERN.match(str(value).strip().lower())) + _THREAD_LOCK = threading.Lock() @@ -61,6 +84,7 @@ def empty() -> dict[str, Any]: "pace": None, "household": [], "note": None, + "lang": None, } @@ -135,6 +159,7 @@ def update( pace: str | None = None, household: list[str] | None = None, note: str | None = None, + lang: str | None = None, ) -> dict[str, Any]: """Merge the given fields into the document. Omitted fields stay as-is. @@ -151,6 +176,8 @@ def update( current["household"] = household if note is not None: current["note"] = note + if lang is not None: + current["lang"] = lang return save(current) @@ -172,6 +199,21 @@ def resolve_mode(explicit: str | None, fallback: str) -> str: return fallback +def resolve_lang(explicit: str | None) -> str | None: + """Pick a result-language code (#410). `explicit` is the caller's + per-call `lang` argument; None means omitted, and then the stored + preference is used if there is one. Neither given returns None — no + lang override at all, byte-identical to pre-#410 behavior. A corrupt + or unreadable preferences file behaves the same as no stored lang. + """ + if explicit is not None: + return explicit + try: + return load().get("lang") + except PreferencesError: + return None + + @contextmanager def _exclusive() -> Iterator[None]: """Same-process lock plus a POSIX file lock for two MCP processes.""" @@ -218,11 +260,17 @@ def _normalize(data: dict[str, Any]) -> dict[str, Any]: note = data.get("note") if note is not None: note = str(note).strip() or None + lang = data.get("lang") + if lang is not None: + lang = str(lang).strip().lower() or None + if lang is not None and not _LANG_PATTERN.match(lang): + lang = None return { "mode": mode, "pace": pace, "household": _household(data.get("household")), "note": note, + "lang": lang, } diff --git a/src/placeroot/resources.py b/src/placeroot/resources.py index 773c7c7..466d24e 100644 --- a/src/placeroot/resources.py +++ b/src/placeroot/resources.py @@ -54,7 +54,8 @@ ) PREFERENCES_DESCRIPTION = ( - "Local travel and household preferences (mode, pace, household). " + "Local travel and household preferences (mode, pace, household, " + "result-language lang). " "The same document the preferences tool reads and updates. " "Nothing in this file leaves the machine." ) diff --git a/src/placeroot/server.py b/src/placeroot/server.py index 838ce1f..27476c9 100644 --- a/src/placeroot/server.py +++ b/src/placeroot/server.py @@ -240,6 +240,17 @@ json_schema_extra={"enum": _MODE_ENUM}, ), ] +# #410: no fixed enum — a language code is validated by shape (2-3 lowercase +# letters), not membership in a closed list the way mode is, since Overture's +# names.common keys are not a small fixed set. +_LangArg = Annotated[ + str | None, + Field( + description="Result-language code (2-3 lowercase letters, e.g. \"de\"). " + "Overture-tagged name variants only — never transliterated or invented. " + "Default: stored preference, else the primary name.", + ), +] _OperatingStatusArg = Annotated[ str | None, Field( @@ -1366,6 +1377,7 @@ def place_details( radius_m: float = overture.DEFAULT_DETAILS_RADIUS_M, near_lat: float | None = None, near_lon: float | None = None, + lang: _LangArg = None, ) -> dict: """One place, in full: addresses, websites, phones, socials, brand, source attribution, GERS id, confidence, operating status, and a @@ -1385,6 +1397,12 @@ def place_details( can be narrowed to a ~50km box instead of scanning the whole dataset. Ignored when resolving by name. Omitting it still works, just slower on a cold, uncached id. + + lang (#410) requests Overture's language-tagged name variant for this + place, when the data has one: `name` becomes the variant and + `name_primary` is added only when it differs. Default: the stored + `preferences()` lang, else the primary name unchanged. Never invented + or transliterated. """ if lat is not None and lon is not None: coord_error = _invalid_coord(lat, lon) @@ -1394,8 +1412,11 @@ def place_details( coord_error = _invalid_coord(near_lat, near_lon) if coord_error is not None: return coord_error + lang = preference_store.resolve_lang(lang) try: - result = overture.place_details(id, name, lat, lon, radius_m, near_lat, near_lon) + result = overture.place_details( + id, name, lat, lon, radius_m, near_lat, near_lon, lang=lang, + ) except ValueError as e: return {"error": "bad_request", "detail": str(e)} except overture.UpstreamUnavailable as e: @@ -3071,7 +3092,7 @@ def water_near( @_tool("Geocode a place name") -def geocode(query: str, limit: int = 5) -> dict: +def geocode(query: str, limit: int = 5, lang: _LangArg = None) -> dict: """Free-text place name -> ranked candidate locations, from Overture divisions and places. No Nominatim, no third-party geocoding API. Matches localities, @@ -3109,9 +3130,17 @@ def geocode(query: str, limit: int = 5) -> dict: München and "Tokyo" answers 東京都. `name` is always the canonical spelling; such rows carry an extra "matched_name" naming the alternate that matched. + + lang (#410) requests Overture's language-tagged name variant instead + of a division row's primary name, when the data has one for that row + and language: `name` becomes the variant and `name_primary` is added + only when it differs. Default: the stored `preferences()` lang, else + the primary name unchanged. Never invented or transliterated — only a + variant actually present in Overture's data is ever returned. """ + lang = preference_store.resolve_lang(lang) try: - result = geocoding.geocode_detailed(query, limit) + result = geocoding.geocode_detailed(query, limit, lang=lang) except overture.UpstreamUnavailable as e: return _upstream_error(e) payload = budget.apply_budget({"results": result["results"]}, "results") @@ -3180,6 +3209,7 @@ def resolve_place( near_lon: float | None = None, limit: int = 3, city: str | None = None, + lang: _LangArg = None, ) -> dict: """Free-text place reference -> ranked, typed GERS ids to hold onto. @@ -3220,14 +3250,21 @@ def resolve_place( An unresolvable query returns {"results": []} — not an error. Returns a structured {"error": ...} instead of raising if the remote scan fails or the places dataset is missing columns this tool depends on. + + lang (#410) requests Overture's language-tagged name variant, same as + geocode() — but only for "kind": "division" rows; "kind": "place" rows + (from find_places, out of scope for #410 this round) always carry + their primary name. Default: the stored `preferences()` lang, else the + primary name unchanged. """ if near_lat is not None and near_lon is not None: coord_error = _invalid_coord(near_lat, near_lon) if coord_error is not None: return coord_error + lang = preference_store.resolve_lang(lang) try: rows = geocoding.resolve_place( - query, near_lat, near_lon, limit, city=city, + query, near_lat, near_lon, limit, city=city, lang=lang, ) except overture.UpstreamUnavailable as e: return _upstream_error(e) @@ -4720,6 +4757,7 @@ def preferences( pace: str | None = None, household: list[str] | None = None, note: str | None = None, + lang: _LangArg = None, clear: bool = False, ) -> dict: """Travel defaults. @@ -4727,14 +4765,18 @@ def preferences( State "I bike everywhere, I have a dog" once. Routing tools use the stored mode when you omit theirs; an explicit argument always wins. pace and household are stored for later features and do not change - answers yet. The same document is the placeroot://preferences resource. - - Call with no arguments to read. Pass mode, pace, household tags, or a - free-text note to merge those fields. + answers yet. lang (#410) is the stored result-language preference: the + name-lookup tools that accept their own `lang` use this one when + theirs is omitted, returning an Overture-tagged name variant (e.g. + "Munich" for "München" with lang="en") — a per-call `lang` always + wins. The same document is the placeroot://preferences resource. + + Call with no arguments to read. Pass mode, pace, household tags, a + free-text note, or lang to merge those fields. clear=true deletes the file and cannot be combined with other fields. Nothing is sent off this machine. """ - fields = (mode, pace, household, note) + fields = (mode, pace, household, note, lang) if clear and any(value is not None for value in fields): return { "error": "bad_request", @@ -4750,8 +4792,13 @@ def preferences( "detail": f"mode={mode!r} is not supported", "supported": sorted(preference_store.MODES), } + if lang is not None and not preference_store.is_valid_lang(lang): + return { + "error": "bad_request", + "detail": f"lang={lang!r} must be 2-3 lowercase letters", + } return preference_store.update( - mode=mode, pace=pace, household=household, note=note + mode=mode, pace=pace, household=household, note=note, lang=lang, ) return preference_store.payload() except preference_store.PreferencesError as exc: diff --git a/tests/fixtures/places.parquet b/tests/fixtures/places.parquet index 67535a4e2711416cdfb27b6f58851a064702424e..5b6a269f0ea113875b09f0ef991a1430be43557e 100644 GIT binary patch delta 2607 zcmcImdu&rx7(eISyV226_Tb*#v1%@_1BMOSvUW2JX!i&My0KS5jG~wJRu))`d*Db+ zwtqxG-UsBNjzZYt`b6t7sqceUtT{vU$f^0ok6EPsM{QQuE4&JY)kSS4*wE|=5z*ZOTguo z(Nfpwc=ty}d>VD%aWgA35x20Hr#4dv2V;@j!1oTq-}wR?jGbe9Fuu^SZ2UQKyg?Zi zqx|0?&X!20)8ebRF?P9;Q@5){Y{981YM~ZdhCk1rrv-z$zEo>!P#4zfAxV@J$?Am% zB%GX0;I4=GJy;LLwqq$bTRJ4ba#Z>nH360)0wNeEsqF$01>zA=A4Y%(NK0dW99`)E zTLA7unpzYNRzgIzG~CpnXMnv-t8EB})IgJ3qy=>~3JcdZ3@Mx#3&-Y9-<}375zA{W z&W%>f`leH*zf&xRRY1mKZo%sr{hSTBco<}Jli*2cH0vpt@9kfk%Ha z`6_+B{brQy!@!9RqZ`hc<8?nWBHz%xZpN1+$+~+NA3w}uh3=(deYSL*Z7Cjwx3Tjc zSrSF$!blN4DUOMbK{Yf1%rUTZjADorKn~|xYb27 zaD;Ug&B|OFugft!P?!`$&lJ98*p-e>Fi){}kmfB9)f~FlZM7nF|3V_-UqA|6YKjR# zO{xi*5SS6FKyG9j1P26mBj6BZQd<#%pQuLBtpp}ZPY#YHPJyfnxa><(AQ;hgez5JSn}oPHQ&28y+tn!0hnc!s&5VaNKq3F&wk= z^Tv1}r@j)oH;rES3aBFoomYy*Q}1uA^{W-jHNURfAi=>9X7@hXQkoFb>hy+~mi-!X z?I7zzg4{~v&6xJx!t~TFY*k3GULkr&ZWm2h7sAA@$92*;9xIRN# zihla+ZhQbEbjM0uS+yMDv;&@b4^ zY-OX^TCbI?*~Q-TP8$ILw_Hp+@8AMggx&C}oN`o@dkOuu57R&H#3?v#Uqaka&Mx1nb4c734Ql9 zR#`p{A7oAC6UqL!SX;S+1G-;`&G2L+k1Z_AZLUQ_GafTOO&di|wZ z{z1sV=p^z%Os}pbv~w+iZ7W~IFPZc1PdN1or%q&h=9}|@bP{$P)AerBmoRPHM(B48k`%%Xb&6Q0H)_0iF>cP& zxXnG-=YwE93G!a3>rFxrZRf%REhkSgabgZ^0Vk*Z$I4xt-5+gd`RYA>fKij&wS#?M XJq0UlOwBZAtC5nB!ve=K{73v3MLm5a delta 1882 zcmcJQZA?>F7{|{!w_@c@A>89FR8wAdh6`@RAs`l|1&j`qmiDqO+03q`3uL@3(hW?^ zrp%Y2Sda%DZp06SXk!+#4bm8AT;_*`n8i5f*v2-)mcQA^*2QxtB7^t6`cx^8ri zZt`2KE5A&;jPb?6?u<(Y^Z!#aSSt4=Ms|56txoq|nD?$B+ES`D`7I8u;SH#=9ZBR- z81yLGa86hT4{S`CO(_NfS;*tNPH-kt;K#y>^4YUZEk5OB~W zgbFZH4E80fJZ~EdD4N1(0H*9uDV|8|t;Z?uPyka76ErxC^9zHHjQK@Vj@0=@uHu*2 z3*l*Ja__`NdQhmCPZf>rFTSdt!^j}R#bRqld8E{3o1;|EV`P&dwZ!x9ggqsNI0z#p z>rJmmMl&=g(Z%6JmxznQ1xY9bp3?Gvz61KCJHTkLWW!N{pMjq?z6P0P@?hJh2RJ<_ znarm9lp(VmYm$*kA+^;qp+Sj`G}F#TX0$1AtE?Dqlx>UQn|G7x&&jN(5vI!>-r5FT z$v0b5*kl{FDaM4+6}gsO?jpt5RCD;|>|biZY+Dn~=cs6|T|3HPf(dTVso%rCw`! zRFeDL?T4xIjk&rnx48Vpf3Sd^n^V_w^inCkkjSo#ax{K}w%uZH)^=E&b(+_w#M`%f z{k2|wWwG*3=P2On=H+mIi@ccExnQ6NBo?}b+3}lTsaQq&0X!AY$MK^gTEc=8*p?fF z?h1<#e`cWLg#CUSvl|m!yc9+&Qb=nz{8f=Ijst{#joBxYn2p?l70z{JtQ%^b%VrY( zEr#{|35T6q7l$M|9z{n98*U|R&u*AKB0Jb^Z1h8p)e-ZS{zrKI~9eC%c-&|QQMU^aY^u=a!S zt9uzf0n_eOa{Cyhd7c(U&J#L;+37=sooVMqKKHyr=m|LO@sdj)!ZMW!4KLD-*<^sQ zQvt5ot!4_%JL+>};3HU4l~K5`TJvj7KL1S25*^0uUI$_4I`|+`V^h6isQ39bF^eH$ zEyii@ {en: Munich, ...}, Wien -> + {en: Vienna, ...}, ...) — no fixture change was needed on the divisions + side, only the query-layer plumbing (geocode.py's new lang_names table). +- tests/fixtures/places.parquet had no names.common column at all before + this issue (see test_poi_name_tiers.py's module docstring). scripts/ + build_fixture.py now writes one, empty on every row except one isolated + far-away place ("Kaffeehaus Wien", 5.0/100.0 — chosen away from every + other fixture cluster and test radius so it cannot change any existing + count-based assertion), which is what the place_details tests below use. + +Divisions-side lookups need the #43 local table (and the #214 alt-name +table beside it, and now the lang table beside that), hence `geocode_cache` +throughout the geocode()/resolve_place() tests, mirroring +test_geocode.py's #214 section. +""" + +import duckdb + +from placeroot import geocode, overture, preferences, server + +# --- preferences: lang field ------------------------------------------------- + + +def test_missing_lang_is_none_by_default(): + assert preferences.load()["lang"] is None + + +def test_lang_round_trips_through_the_tool(): + updated = server.preferences(lang="de") + assert updated["lang"] == "de" + assert server.preferences()["lang"] == "de" + + +def test_lang_is_lowercased_and_stripped(): + server.preferences(lang=" DE ") + assert server.preferences()["lang"] == "de" + + +def test_lang_accepts_a_three_letter_code(): + server.preferences(lang="fra") + assert server.preferences()["lang"] == "fra" + + +def test_junk_lang_is_rejected_like_an_unsupported_mode(): + result = server.preferences(lang="english") + assert result["error"] == "bad_request" + # Rejected before it ever reaches the stored document. + assert server.preferences()["lang"] is None + + +def test_single_letter_lang_is_rejected(): + result = server.preferences(lang="e") + assert result["error"] == "bad_request" + + +def test_lang_with_digits_is_rejected(): + result = server.preferences(lang="d3") + assert result["error"] == "bad_request" + + +def test_clear_resets_lang_too(): + server.preferences(lang="de") + cleared = server.preferences(clear=True) + assert cleared["lang"] is None + + +def test_resolve_lang_explicit_wins_over_stored(): + preferences.update(lang="de") + assert preferences.resolve_lang("en") == "en" + + +def test_resolve_lang_falls_back_to_stored_when_omitted(): + preferences.update(lang="de") + assert preferences.resolve_lang(None) == "de" + + +def test_resolve_lang_is_none_with_nothing_stored(): + assert preferences.resolve_lang(None) is None + + +# --- geocode()/geocode_detailed(): division-side variant swap --------------- + + +def test_no_lang_is_byte_identical(geocode_cache): + """Guardrail: omitting lang must not change a single byte of the answer.""" + with_no_lang = geocode.geocode("München", limit=1) + assert with_no_lang[0]["name"] == "München" + assert "name_primary" not in with_no_lang[0] + + +def test_lang_swaps_in_the_requested_variant(geocode_cache): + results = geocode.geocode("München", limit=1, lang="en") + assert results[0]["name"] == "Munich" + assert results[0]["name_primary"] == "München" + + +def test_lang_with_no_variant_leaves_primary_untouched(geocode_cache): + """München's names.common has no "zz" entry — primary stands, no note, + no name_primary noise.""" + results = geocode.geocode("München", limit=1, lang="zz") + assert results[0]["name"] == "München" + assert "name_primary" not in results[0] + + +def test_lang_and_alt_name_match_compose_cleanly(geocode_cache): + """Querying the exonym itself ("Munich", #214's alt-name tier) and also + asking for the "en" variant both fire on the same row without + conflicting: matched_name still names what was searched, name_primary + still names the (different) canonical spelling.""" + top = geocode.geocode("Munich", limit=5, lang="en")[0] + assert top["name"] == "Munich" + assert top["matched_name"] == "Munich" + assert top["name_primary"] == "München" + + +def test_lang_applies_across_the_live_verified_exonym_corpus(geocode_cache): + for query, lang, variant in [ + ("München", "en", "Munich"), + ("東京都", "en", "Tokyo"), + ("Москва", "en", "Moscow"), + ("Wien", "en", "Vienna"), + ("Bratislava", "de", "Preßburg"), + ]: + results = geocode.geocode(query, limit=5, lang=lang) + assert results, f"{query!r} found nothing" + assert results[0]["name"] == variant, query + assert results[0]["name_primary"] == query, query + + +def test_geocode_detailed_results_key_carries_the_swap(geocode_cache): + result = geocode.geocode_detailed("München", limit=1, lang="en") + assert result["results"][0]["name"] == "Munich" + + +def test_places_fallback_rows_are_unaffected_by_lang(geocode_cache): + """#410 deliberately scopes out places-fallback rows this round (the + same line find_places' own docstring draws) — a places-fallback row + (row["_category"] set) must keep its primary name even when lang is + requested.""" + result = geocode.geocode_detailed("Blue Bottle Roastery", limit=5, lang="en") + place_rows = [r for r in result["results"] if r.get("type") == "place"] + for row in place_rows: + assert "name_primary" not in row + + +# --- server.geocode(): preference wiring ------------------------------------ + + +def test_server_geocode_no_lang_configured_is_byte_identical(geocode_cache): + with_lang_call = server.geocode(query="München", limit=1) + assert with_lang_call["results"][0]["name"] == "München" + assert "name_primary" not in with_lang_call["results"][0] + + +def test_server_geocode_uses_stored_lang_when_omitted(geocode_cache): + preferences.update(lang="en") + result = server.geocode(query="München", limit=1) + assert result["results"][0]["name"] == "Munich" + assert result["results"][0]["name_primary"] == "München" + + +def test_server_geocode_per_call_lang_beats_stored_preference(geocode_cache): + preferences.update(lang="de") + result = server.geocode(query="München", limit=1, lang="en") + assert result["results"][0]["name"] == "Munich" + assert result["results"][0]["name_primary"] == "München" + + +# --- resolve_place(): division-kind rows only ------------------------------- + + +def test_resolve_place_lang_swaps_division_candidate(geocode_cache): + rows = geocode.resolve_place( + "München", near_lat=48.14, near_lon=11.58, limit=3, lang="en", + ) + division_rows = [r for r in rows if r["kind"] == "division"] + assert division_rows, "expected at least one division candidate" + top = division_rows[0] + assert top["name"] == "Munich" + assert top["name_primary"] == "München" + + +def test_resolve_place_lang_cache_key_does_not_leak_across_langs(geocode_cache): + """The #410 resolve cache is keyed on lang (see geocode._resolve_cache_key) + — a lang="en" call must not be replayed for a later call with no lang.""" + lang_rows = geocode.resolve_place( + "München", near_lat=48.14, near_lon=11.58, limit=3, lang="en", + ) + plain_rows = geocode.resolve_place( + "München", near_lat=48.14, near_lon=11.58, limit=3, + ) + lang_top = next(r for r in lang_rows if r["kind"] == "division") + plain_top = next(r for r in plain_rows if r["kind"] == "division") + assert lang_top["name"] == "Munich" + assert plain_top["name"] == "München" + assert "name_primary" not in plain_top + + +def test_server_resolve_place_no_lang_is_byte_identical(geocode_cache): + result = server.resolve_place(query="München", near_lat=48.14, near_lon=11.58, limit=3) + top = next(r for r in result["results"] if r["kind"] == "division") + assert top["name"] == "München" + assert "name_primary" not in top + + +def test_server_resolve_place_uses_stored_lang(geocode_cache): + preferences.update(lang="en") + result = server.resolve_place(query="München", near_lat=48.14, near_lon=11.58, limit=3) + top = next(r for r in result["results"] if r["kind"] == "division") + assert top["name"] == "Munich" + assert top["name_primary"] == "München" + + +# --- place_details(): places-side variant swap ------------------------------ + +_WIEN_LAT, _WIEN_LON = 5.0, 100.0 + + +def test_place_details_no_lang_is_byte_identical(): + result = overture.place_details(name="Kaffeehaus Wien", lat=_WIEN_LAT, lon=_WIEN_LON) + assert result["name"] == "Kaffeehaus Wien" + assert "name_primary" not in result + + +def test_place_details_lang_swaps_in_the_variant(): + result = overture.place_details( + name="Kaffeehaus Wien", lat=_WIEN_LAT, lon=_WIEN_LON, lang="en", + ) + assert result["name"] == "Vienna Coffee House" + assert result["name_primary"] == "Kaffeehaus Wien" + + +def test_place_details_lang_with_no_variant_leaves_primary_untouched(): + result = overture.place_details( + name="Kaffeehaus Wien", lat=_WIEN_LAT, lon=_WIEN_LON, lang="de", + ) + assert result["name"] == "Kaffeehaus Wien" + assert "name_primary" not in result + + +def test_place_details_lang_on_a_row_with_no_names_common_at_all(): + """Every other fixture row's names.common is an empty map, not absent — + but the query-layer fallback (a duckdb.Error caught in + _run_place_details_query) also has to hold for a genuinely missing + names.common. Exercised together with an ordinary place here rather + than a second fixture: an empty map already answers "no variant" + through the same code path as a missing column would.""" + result = overture.place_details( + name="Blue Bottle Roastery", lat=40.700000, lon=-73.900000, lang="en", + ) + assert result["name"] == "Blue Bottle Roastery" + assert "name_primary" not in result + + +def test_server_place_details_uses_stored_lang(geocode_cache): + preferences.update(lang="en") + result = server.place_details(name="Kaffeehaus Wien", lat=_WIEN_LAT, lon=_WIEN_LON) + assert result["name"] == "Vienna Coffee House" + assert result["name_primary"] == "Kaffeehaus Wien" + + +def test_server_place_details_per_call_lang_beats_preference(geocode_cache): + preferences.update(lang="de") + result = server.place_details( + name="Kaffeehaus Wien", lat=_WIEN_LAT, lon=_WIEN_LON, lang="en", + ) + assert result["name"] == "Vienna Coffee House" + assert result["name_primary"] == "Kaffeehaus Wien" + + +def test_server_place_details_no_lang_configured_is_byte_identical(): + result = server.place_details(name="Kaffeehaus Wien", lat=_WIEN_LAT, lon=_WIEN_LON) + assert result["name"] == "Kaffeehaus Wien" + assert "name_primary" not in result + + +# --- lang_names table machinery --------------------------------------------- + + +def test_lang_names_table_is_materialized_beside_the_divisions_table(geocode_cache): + table = geocode._local_divisions_table() + lang_table = geocode._local_lang_names_table(table) + assert lang_table is not None + rows = duckdb.connect().execute( + f"SELECT id, lang, name FROM read_parquet('{lang_table}') " + "WHERE id = 'gers-div-munchen' ORDER BY lang" + ).fetchall() + langs = {r[1]: r[2] for r in rows} + assert langs["en"] == "Munich" + assert langs["fr"] == "Munich" + assert langs["it"] == "Monaco di Baviera" + assert langs["es"] == "Múnich" + + +def test_lang_variants_for_empty_without_a_table_or_ids(): + assert geocode._lang_variants_for(None, ["gers-div-munchen"], "en") == {} + assert geocode._lang_variants_for("/does/not/matter.parquet", [], "en") == {} diff --git a/tests/test_name_path.py b/tests/test_name_path.py index 43590ab..6f269fb 100644 --- a/tests/test_name_path.py +++ b/tests/test_name_path.py @@ -33,7 +33,7 @@ def test_a_bare_city_is_not_a_poi_alias(): def test_famous_poi_does_not_lose_to_an_obscure_exact_division(monkeypatch): """Colosseum used to exact-match a Queensland locality and win.""" - def fake_geocode(query, limit=5): + def fake_geocode(query, limit=5, lang=None): q = query.lower() if q in {"rome", "roma"}: return [{ @@ -64,7 +64,7 @@ def fake_find_places(lat, lon, radius_m=1000, category=None, name=None, limit=10 def test_ebisu_alias_does_not_aim_at_shikoku(monkeypatch): - def fake_geocode(query, limit=5): + def fake_geocode(query, limit=5, lang=None): if query.lower() == "tokyo": return [{ "name": "Tokyo", "type": "locality", "lat": 35.68, "lon": 139.69, @@ -117,7 +117,7 @@ def test_last_city_is_reused_for_the_next_poi(monkeypatch): seen = {} - def fake_geocode(query, limit=5): + def fake_geocode(query, limit=5, lang=None): seen.setdefault("q", []).append(query) return [{ "name": query, "type": "locality", "lat": CENTER_LAT, "lon": CENTER_LON, @@ -152,7 +152,7 @@ def test_observation_tower_does_not_replay_brooklyn_after_paris(monkeypatch): geocode.resolve_place("Brooklyn") assert geocode._last_good_city - def fake_geocode(query, limit=5): + def fake_geocode(query, limit=5, lang=None): q = query.lower() if q == "paris": return [{ diff --git a/tests/test_poi_name_tiers.py b/tests/test_poi_name_tiers.py index 3c3c705..0b86953 100644 --- a/tests/test_poi_name_tiers.py +++ b/tests/test_poi_name_tiers.py @@ -13,10 +13,12 @@ Tier-2/3 rows carry "matched_by": "alt_name" | "fuzzy"; tier-1 rows never do. The fixture (tests/fixtures/places.parquet) has a real name to fuzz — -"Blue Bottle Roastery", near CENTER_LAT/CENTER_LON — but no names.common -data at all (confirmed live: the `names` struct there is -STRUCT(primary VARCHAR), no "common" key), so the alt-name tier is tested -against a small tmp_path fixture built for that one purpose. +"Blue Bottle Roastery", near CENTER_LAT/CENTER_LON — but every row's +names.common there is empty (#410 added the `common` MAP column itself, +carried on one deliberately isolated far-away row for #410's own lang +tests; nothing near CENTER_LAT/CENTER_LON has an alternate spelling), so +the alt-name tier is tested against a small tmp_path fixture built for +that one purpose. """ import duckdb diff --git a/tests/test_preferences.py b/tests/test_preferences.py index 45a7065..fadd479 100644 --- a/tests/test_preferences.py +++ b/tests/test_preferences.py @@ -12,6 +12,7 @@ def test_missing_file_is_an_empty_document(): "pace": None, "household": [], "note": None, + "lang": None, } @@ -64,6 +65,7 @@ def test_tool_merge_then_clear(): "pace": None, "household": [], "note": None, + "lang": None, } assert server.preferences() == cleared diff --git a/tests/test_progressive.py b/tests/test_progressive.py index 7934f24..97aa983 100644 --- a/tests/test_progressive.py +++ b/tests/test_progressive.py @@ -205,7 +205,7 @@ def test_catalog_summaries_match_the_tools_own_descriptions(): def test_required_and_optional_args_are_distinguished(): - assert server._arg_summary(server._TOOL_FUNCS["geocode"]) == "query,limit?" + assert server._arg_summary(server._TOOL_FUNCS["geocode"]) == "query,limit?,lang?" assert server._arg_summary(server._TOOL_FUNCS["data_version"]) == "" @@ -290,7 +290,7 @@ def test_unknown_tool_is_a_structured_error_listing_the_valid_names(): def test_unknown_argument_is_a_bad_request_naming_what_the_tool_accepts(): result = server.placeroot_call("geocode", {"querry": "Brooklyn"}) assert result["error"] == "bad_request" - assert result["accepts"] == "query,limit?" + assert result["accepts"] == "query,limit?,lang?" def test_missing_required_argument_is_a_bad_request(): @@ -370,7 +370,7 @@ def test_an_uncoercible_value_is_rejected_by_the_same_model_the_sdk_uses(): result = server.placeroot_call("geocode", {"query": "x", "limit": "many"}) assert result["error"] == "bad_request" assert "limit" in result["detail"] - assert result["accepts"] == "query,limit?" + assert result["accepts"] == "query,limit?,lang?" def test_the_validation_detail_is_advice_not_a_pydantic_dump(): diff --git a/tests/test_resolve_place.py b/tests/test_resolve_place.py index 225ab12..01ee262 100644 --- a/tests/test_resolve_place.py +++ b/tests/test_resolve_place.py @@ -78,7 +78,7 @@ def test_merged_ordering_is_kind_agnostic_by_match_tier(monkeypatch): # White-box: verify the merge/sort ranks candidates by match tier # regardless of kind, using controlled inputs so the test doesn't # depend on incidental overlap between fixture division/place names. - def fake_geocode(query, limit=5): + def fake_geocode(query, limit=5, lang=None): return [ { "name": "Example Town", "type": "locality", "lat": 1.0, "lon": 2.0, From 2b603fa0672bcdfd32690d681ef9799aadf2a509 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 03:59:57 +0000 Subject: [PATCH 2/2] lang preference: rank and prune from primary names, normalize per-call lang MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - resolve_place's division match labels (the merged ranking's primary sort key) and its city-word token pruning now read the primary name (name_primary when a variant was applied): grading 'München' against a lang-swapped 'Munich' demoted the exactly-matched division below coincidentally-named places and stopped the city-hint pruning from firing. - preferences.resolve_lang now strips/lowercases an explicit per-call lang exactly like the stored write path, and a value failing the same shape check disables lang for that call instead of silently passing junk that can never match a variant. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01YCuTkstNj5KscUQ9u8cLMK --- src/placeroot/geocode.py | 16 ++++++++++++++-- src/placeroot/preferences.py | 11 +++++++++-- tests/test_lang_preference.py | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 4 deletions(-) diff --git a/src/placeroot/geocode.py b/src/placeroot/geocode.py index 50a0a1d..8fb5b24 100644 --- a/src/placeroot/geocode.py +++ b/src/placeroot/geocode.py @@ -4264,7 +4264,12 @@ def resolve_place( if len(tokens) > 1: folded_city = {t.lower() for t in tokens} for div in division_hits[:1]: - folded_city &= {w.lower() for w in (div.get("name") or "").split()} + # #410: prune against the *primary* name — under a lang + # override div["name"] may be the localized variant + # ("Munich"), and the caller's own city word ("München") + # must still be recognized and pruned. + div_name = div.get("name_primary") or div.get("name") or "" + folded_city &= {w.lower() for w in div_name.split()} tokens = [t for t in tokens if t.lower() not in folded_city] or tokens # #374: the query words a fallback-matched row must account for — # the single distinctive tokens that survived the generic/city @@ -4297,7 +4302,14 @@ def resolve_place( "lat": r["lat"], "lon": r["lon"], "type": r.get("type"), "admin_context": r["admin_context"], - "match": _division_match_label(r, query, search_query), + # #410: label (and therefore rank) off the primary name — the + # caller's query was written against it, and grading "München" + # against a lang-swapped "Munich" would demote the correct + # division to a substring match below coincidentally-named + # places. + "match": _division_match_label( + {**r, "name": r.get("name_primary") or r["name"]}, query, search_query + ), "_prominence": r["rank_score"], } if r.get("name_primary"): diff --git a/src/placeroot/preferences.py b/src/placeroot/preferences.py index d1b2241..8ad6cb6 100644 --- a/src/placeroot/preferences.py +++ b/src/placeroot/preferences.py @@ -204,10 +204,17 @@ def resolve_lang(explicit: str | None) -> str | None: per-call `lang` argument; None means omitted, and then the stored preference is used if there is one. Neither given returns None — no lang override at all, byte-identical to pre-#410 behavior. A corrupt - or unreadable preferences file behaves the same as no stored lang. + or unreadable preferences file behaves the same as no stored lang. The + explicit value is normalized (strip/lower) the same way the stored one + was at write time, and a value failing the same is_valid_lang shape + check the write path enforces disables lang for that call entirely + (the stored preference is not silently substituted for a value the + caller explicitly sent) — the lang table's keys are lowercased 2-3 + letter codes, so junk could never match a variant anyway. """ if explicit is not None: - return explicit + normalized = str(explicit).strip().lower() + return normalized if is_valid_lang(normalized) else None try: return load().get("lang") except PreferencesError: diff --git a/tests/test_lang_preference.py b/tests/test_lang_preference.py index a413a63..da1e84d 100644 --- a/tests/test_lang_preference.py +++ b/tests/test_lang_preference.py @@ -303,3 +303,37 @@ def test_lang_names_table_is_materialized_beside_the_divisions_table(geocode_cac def test_lang_variants_for_empty_without_a_table_or_ids(): assert geocode._lang_variants_for(None, ["gers-div-munchen"], "en") == {} assert geocode._lang_variants_for("/does/not/matter.parquet", [], "en") == {} + + +def test_per_call_lang_is_normalized_at_the_tool_layer(geocode_cache): + """resolve_lang strips/lowercases an explicit lang the same way the + preferences write path does — " EN " and "en" must behave alike.""" + result = server.geocode("München", limit=1, lang=" EN ") + assert result["results"][0]["name"] == "Munich" + + +def test_junk_per_call_lang_disables_lang_for_that_call(geocode_cache): + """An explicit lang that fails the shape check disables lang for the + call outright — the stored preference is not silently substituted for a + value the caller actually sent.""" + server.preferences(lang="en") + result = server.geocode("München", limit=1, lang="english") + top = result["results"][0] + assert top["name"] == "München" + assert "name_primary" not in top + + +def test_lang_does_not_change_division_match_labels_in_resolve_place(geocode_cache): + """#410: match labels (and therefore ranking) grade the caller's query + against the *primary* name — a lang-swapped "Munich" must not demote the + division the query "München" matched exactly.""" + base = geocode.resolve_place("München", near_lat=48.14, near_lon=11.58, limit=3) + swapped = geocode.resolve_place( + "München", near_lat=48.14, near_lon=11.58, limit=3, lang="en", + ) + base_div = next(r for r in base if r["kind"] == "division") + swapped_div = next( + r for r in swapped + if r["kind"] == "division" and r.get("name_primary") == "München" + ) + assert swapped_div["match"] == base_div["match"]