lekss361/gendesign

Fork 0

fix(tradein/geocoder): gate EKB-local tiers for non-EKB oblast addresses (#11) #2504

Merged

lekss361 merged 3 commits from fix/geocoder-non-ekb-city-leak into main

2026-07-12 21:08:50 +00:00

lekss361 commented

2026-07-12 20:46:34 +00:00

Owner

Summary

Root cause (#11): geocode() ran EKB-only local tiers (2a _geoportal_house_match
on ekb_geoportal_buildings, 2c _cadastral_house_match, 2d
_cadastral_forward_sync) BEFORE the external providers. _parse_street_house
drops the city, so any non-EKB Sverdlovsk-oblast street+house that collides
with an EKB building (e.g. "проспект Ленина, 1" exists in both Nizhny Tagil and EKB)
short-circuited to EKB coordinates before ever reaching Yandex/Nominatim.

Note: as of this branch's base, forgejo/main already ships oblast-wide
awareness for the EXTERNAL providers (OBLAST66_BBOX, OBLAST66_VIEWBOX,
_has_oblast_marker, Yandex/Nominatim region cross-check) from a same-day
prior commit (c0cbdc2f). That fix solved "valid oblast geocodes get
rejected" but did NOT gate the EKB-only LOCAL DB tiers, which is the bug
this PR closes.

Change

Add _names_non_ekb_city(address) -> bool — reuses the existing
SVERDLOVSK_OBLAST_CITIES gazetteer (minus Екатеринбург) and the
word-boundary matching + district-prefix exclusion already used by
_has_oblast_marker (no new gazetteer/normalization scheme introduced).
geocode(): compute use_local_ekb = not _names_non_ekb_city(address)
right after parsed = _parse_street_house(...); gate tiers 2a/2c/2d behind
it. Non-EKB-city addresses skip straight to the already oblast-aware
Yandex/Nominatim tiers (3/4) below.
EKB path unchanged: for any address where _names_non_ekb_city is
False (EKB address or bare/ambiguous address), use_local_ekb is True
and the code path is byte-identical in ordering/logic to before this PR
— only additive gating.
Verified via DB query: gendesign_cad_buildings is 99.85% EKB-scoped
(47043/47111 rows; only 1 real non-EKB row found), ekb_geoportal_buildings
is 100% EKB by construction — gating loses no meaningful non-EKB local
coverage.

Tests

tests/test_geocoder_bbox.py: unit tests for _names_non_ekb_city
(Nizhny Tagil → True, EKB-prefixed → False, bare address → False,
"Серова 27" word-boundary → False, district-prefix → False,
"Каменск-Уральский" → True).
tests/test_geocoder_cadastral_matcher.py: geocode() wiring test — for a
Nizhny Tagil address, _geoportal_house_match / _cadastral_house_match /
_cadastral_forward_sync are NOT called; result comes from a stubbed
_nominatim_lookup instead.

ruff check / ruff format --check / pytest tests -k geocod -q all green
(160 passed).

Follow-up (NOT in this PR)

geocode_cache has rows poisoned by the old behavior — non-EKB addresses
cached with wrong EKB coordinates from a prior lookup. A cache purge of those
rows is a required follow-up; main session will handle it (out of scope for
this PR per task instructions — no SQL/migrations touched here).

## Summary Root cause (#11): `geocode()` ran EKB-only local tiers (2a `_geoportal_house_match` on `ekb_geoportal_buildings`, 2c `_cadastral_house_match`, 2d `_cadastral_forward_sync`) BEFORE the external providers. `_parse_street_house` drops the city, so any non-EKB Sverdlovsk-oblast street+house that collides with an EKB building (e.g. "проспект Ленина, 1" exists in both Nizhny Tagil and EKB) short-circuited to EKB coordinates before ever reaching Yandex/Nominatim. Note: as of this branch's base, `forgejo/main` already ships oblast-wide awareness for the EXTERNAL providers (`OBLAST66_BBOX`, `OBLAST66_VIEWBOX`, `_has_oblast_marker`, Yandex/Nominatim region cross-check) from a same-day prior commit (`c0cbdc2f`). That fix solved "valid oblast geocodes get rejected" but did NOT gate the EKB-only LOCAL DB tiers, which is the bug this PR closes. ## Change - Add `_names_non_ekb_city(address) -> bool` — reuses the existing `SVERDLOVSK_OBLAST_CITIES` gazetteer (minus Екатеринбург) and the word-boundary matching + district-prefix exclusion already used by `_has_oblast_marker` (no new gazetteer/normalization scheme introduced). - `geocode()`: compute `use_local_ekb = not _names_non_ekb_city(address)` right after `parsed = _parse_street_house(...)`; gate tiers 2a/2c/2d behind it. Non-EKB-city addresses skip straight to the already oblast-aware Yandex/Nominatim tiers (3/4) below. - **EKB path unchanged**: for any address where `_names_non_ekb_city` is `False` (EKB address or bare/ambiguous address), `use_local_ekb` is `True` and the code path is byte-identical in ordering/logic to before this PR — only additive gating. - Verified via DB query: `gendesign_cad_buildings` is 99.85% EKB-scoped (47043/47111 rows; only 1 real non-EKB row found), `ekb_geoportal_buildings` is 100% EKB by construction — gating loses no meaningful non-EKB local coverage. ## Tests - `tests/test_geocoder_bbox.py`: unit tests for `_names_non_ekb_city` (Nizhny Tagil → True, EKB-prefixed → False, bare address → False, "Серова 27" word-boundary → False, district-prefix → False, "Каменск-Уральский" → True). - `tests/test_geocoder_cadastral_matcher.py`: `geocode()` wiring test — for a Nizhny Tagil address, `_geoportal_house_match` / `_cadastral_house_match` / `_cadastral_forward_sync` are NOT called; result comes from a stubbed `_nominatim_lookup` instead. `ruff check` / `ruff format --check` / `pytest tests -k geocod -q` all green (160 passed). ## Follow-up (NOT in this PR) `geocode_cache` has rows poisoned by the old behavior — non-EKB addresses cached with wrong EKB coordinates from a prior lookup. A cache purge of those rows is a required follow-up; main session will handle it (out of scope for this PR per task instructions — no SQL/migrations touched here).

lekss361 added 1 commit 2026-07-12 20:46:34 +00:00

fix(tradein/geocoder): gate EKB-local tiers for non-EKB oblast addresses (#11 )

CI Trade-In / frontend-checks (pull_request) Has been skipped

Details

CI / backend-tests (pull_request) Has been skipped

Details

CI / frontend-tests (pull_request) Has been skipped

Details

CI / openapi-codegen-check (pull_request) Has been skipped

Details

CI Trade-In / backend-tests (pull_request) Successful in 53s

Details

CI / changes (pull_request) Successful in 8s

Details

CI Trade-In / changes (pull_request) Successful in 9s

Details

720cee2c8e

geocode() ran EKB-only local tiers (2a _geoportal_house_match on
ekb_geoportal_buildings, 2c _cadastral_house_match, 2d
_cadastral_forward_sync — all strictly EKB or EKB-dominated) BEFORE the
already oblast-aware external providers (Yandex/Nominatim via OBLAST66_BBOX
+ region cross-check, shipped in c0cbdc2f). _parse_street_house drops the
city, so any non-EKB oblast street+house that collides with an EKB building
(e.g. "проспект Ленина 1" exists in both Nizhny Tagil and EKB) short-circuited
to EKB coordinates.

Add _names_non_ekb_city() — reuses the existing SVERDLOVSK_OBLAST_CITIES
gazetteer (minus Ekaterinburg) and word-boundary matching from
_has_oblast_marker/_DISTRICT_PREFIXES — to detect when an address explicitly
names a different oblast city. Gate tiers 2a/2c/2d behind
`use_local_ekb = not _names_non_ekb_city(address)`; EKB/bare addresses keep
the exact same code path (byte-identical ordering/logic, only additive
gating). Non-EKB addresses fall straight through to Yandex/Nominatim, which
already handle oblast-wide geocoding correctly.

Verified gendesign_cad_buildings is 99.85% EKB-scoped (47043/47111 rows);
ekb_geoportal_buildings is 100% EKB by construction — gating loses no
meaningful local coverage for non-EKB cities.

Follow-up (not in this PR): geocode_cache has rows poisoned by the old
behavior (non-EKB addresses cached with EKB coordinates) — needs a purge.

lekss361 added 1 commit 2026-07-12 20:50:59 +00:00

fix(tradein/geocoder): explicit Екатеринбург wins over oblast-city homonym (#11 )

CI Trade-In / changes (pull_request) Successful in 8s

Details

CI / changes (pull_request) Successful in 8s

Details

CI / backend-tests (pull_request) Has been skipped

Details

CI / frontend-tests (pull_request) Has been skipped

Details

CI Trade-In / backend-tests (pull_request) Successful in 51s

Details

CI Trade-In / frontend-checks (pull_request) Has been skipped

Details

CI / openapi-codegen-check (pull_request) Has been skipped

Details

2ef3e42da1

Prod-cache verification (deep-review) found 3 false-positives: EKB addresses
that literally contain "екатеринбург" but also happen to contain a homonym
of another oblast city inside a neighborhood/posyolok name — ЖК "Заречный"
and пос. Сысерть, both administratively inside Ekaterinburg, not the ZATO
Zarechny / town Sysert they share a name with. These were gated off the
EKB-only local tiers; if Nominatim were momentarily down, a real EKB address
would return None — a latent regression not present before this PR.

Add an explicit-city short-circuit at the top of _names_non_ekb_city: if the
normalized address contains "екатеринбург", return False immediately (EKB
tiers stay on). Cannot affect the true-positive (non-EKB) cases, which never
contain "екатеринбург".

lekss361 added 1 commit 2026-07-12 21:06:29 +00:00

fix(tradein/geocoder): word-boundary EKB short-circuit + gate suggest() + gazetteer (#11 )

CI / changes (pull_request) Successful in 7s

Details

CI Trade-In / changes (pull_request) Successful in 8s

Details

CI Trade-In / frontend-checks (pull_request) Has been skipped

Details

CI / backend-tests (pull_request) Has been skipped

Details

CI / frontend-tests (pull_request) Has been skipped

Details

CI / openapi-codegen-check (pull_request) Has been skipped

Details

CI Trade-In / backend-tests (pull_request) Successful in 56s

Details

8fdcd8df62

Deep-review fast-follows on the non-EKB city gate:

1. Word-boundary the "екатеринбург" short-circuit in _names_non_ekb_city
   (new _EKATERINBURG_RE = \bекатеринбург\b, .search() instead of substring
   `in`). A bare-substring check matched "Екатеринбургское шоссе" (a real
   street in satellite towns like Pervouralsk) as if it named EKB, wrongly
   keeping EKB-only local tiers on for a non-EKB address. Still returns False
   for the existing homonym false-positive test case (word boundary is
   satisfied there — "екатеринбург" appears as a standalone token).

2. Gate suggest()'s Tier 1 (same root cause as geocode()): the EKB-only
   cadastral matchers _cadastral_house_match / _cadastral_forward_sync ran
   unconditionally, so a non-EKB oblast autocomplete query could surface an
   EKB building via street+house collision. Gated behind
   `not _names_non_ekb_city(query)`, mirroring geocode()'s use_local_ekb.
   External suggest tiers (DaData/Yandex/Nominatim) untouched — still the
   path for non-EKB autocomplete.

3. Extended SVERDLOVSK_OBLAST_CITIES with unambiguous, deal-heavy oblast
   cities: алапаевск, сухой лог, кушва, красноуральск, карпинск, нижняя
   тура, верхний тагил, нижние серги. Deliberately did NOT add "лесной"
   (reviewer flagged as reviewer-optional, left to judgment): DB check
   (ekb_geoportal_buildings) confirms a real EKB street named exactly
   "Лесной" (3 buildings) — an exact whole-word collision indistinguishable
   from ЗАТО Лесной by word-boundary matching alone (unlike "Серова"/"Серов",
   which are different word forms). Adding it would misclassify a bare
   EKB address ("Лесной, 5", no "Екатеринбург" mention) as non-EKB.

Tests: word-boundary EKB regression test, suggest() gating test, two new
gazetteer entries (Верхняя Пышма multi-word, Сухой Лог).