lekss361/gendesign

Fork 0

fix(tradein): extract_street_name handles Nominatim reverse format #557

Merged

lekss361 merged 1 commit from fix/tradein-extract-street-name-reverse into main

2026-05-24 21:03:43 +00:00

lekss361 commented

2026-05-24 20:55:18 +00:00

Owner

Live bug

На проде после deploy PR-C: секция «По вашей улице» не показывалась — endpoint возвращал count=0 для любого реального адреса.

Real browser request (DevTools captured):

GET /api/v1/trade-in/street-deals?address=80, улица 8 Марта, Артек, Форум-Сити, Ленинский район, Екатеринбург, ..., 640144, Россия&area_m2=35&rooms=1
→ {"street": "8 Марта, Артек, Форум-Сити, Ленинский район, Екатеринбург, городской округ Екатеринбург, Свердловская область, Уральский федеральный округ", "count": 0, ...}

Root cause

extract_street_name() была спроектирована под FORWARD-формат ("ул. Космонавтов, 50"), но geo.full_address от geocoder Nominatim — REVERSE-формат:

"80, улица 8 Марта, Артек, ..., 640144, Россия"

Старая логика:

_extract_short_addr находит keyword "улица" → возвращает остаток до конца строки
_TRAILING_HOUSE_RE = r',\s*\d+.*$' → matches последний postal-index (, 640144, Россия) — но .*$ greedy → strip последние 2 элемента, оставив всю админ-кишку
_STREET_PREFIX_RE отрезает "улица "
Result: "8 Марта, Артек, Форум-Сити, Ленинский район, Екатеринбург, ..." ⛔
SQL ILIKE '%<мусор>%' → 0 matches в tradein.deals

Verified локально на production-адресе перед fix.

Fix

Новая логика без _extract_short_addr промежуточного шага:

_STREET_KW_RE = re.compile(r"(?<![А-Яа-яёЁa-zA-Z])(?:ул\.|улица|пр\.|пр-т|проспект|...)\s+", IGNORECASE)

_STREET_NAME_RE = re.compile(
    r"^([0-9]+\s+[А-Яа-яёЁ][А-Яа-яёЁ-]+"           # "8 Марта"
    r"|[А-Яа-яёЁ][А-Яа-яёЁ-]+(?:\s+[А-Яа-яёЁ][А-Яа-яёЁ-]+){0,2})"  # "Космонавтов" / "Большая Конюшенная"
    r"(?=,|\s+(?:д\.?\s*)?\d|\s*$)"               # stop at comma / house number / EOL
)

Алгоритм:

Найти street-keyword (ул./улица/пр./...) — IGNORECASE, anywhere в строке
После keyword: захватить 1-3 слова до первой запятой / номера дома
Fallback: первый capitalized токен без keyword (для "Большая Конюшенная, 25"); фильтр admin-слов (Россия/область/район/...)

Работает с обоими форматами:

Forward: "Екатеринбург, ул. Космонавтов, 50" → "Космонавтов"
Reverse: "80, улица 8 Марта, Артек, ..." → "8 Марта"

Changes

tradein-mvp/backend/app/services/estimator.py — replaced _TRAILING_HOUSE_RE + _STREET_PREFIX_RE + old extract_street_name with new _STREET_KW_RE + _STREET_NAME_RE + rewritten function
tradein-mvp/backend/tests/test_street_deals_endpoint.py — test_extract_street_name_parametrized (10 cases: 4 forward, 2 reverse Nominatim, 4 edge cases)

Verification

ruff check — pass
pytest — 15/15 pass (5 pre-existing + 10 new parametrized)
После deploy ожидание: live запрос на ул. 8 Марта → street="8 Марта", count > 30 → секция в UI рендерится

Refs: финальный hotfix 4-PR roadmap. После deploy: hard reload в браузере (Ctrl+Shift+R) для очистки JS-кеша.

## Live bug На проде после deploy PR-C: секция «По вашей улице» не показывалась — endpoint возвращал `count=0` для любого реального адреса. Real browser request (DevTools captured): ``` GET /api/v1/trade-in/street-deals?address=80, улица 8 Марта, Артек, Форум-Сити, Ленинский район, Екатеринбург, ..., 640144, Россия&area_m2=35&rooms=1 → {"street": "8 Марта, Артек, Форум-Сити, Ленинский район, Екатеринбург, городской округ Екатеринбург, Свердловская область, Уральский федеральный округ", "count": 0, ...} ``` ## Root cause `extract_street_name()` была спроектирована под FORWARD-формат (`"ул. Космонавтов, 50"`), но `geo.full_address` от geocoder Nominatim — REVERSE-формат: ``` "80, улица 8 Марта, Артек, ..., 640144, Россия" ``` Старая логика: 1. `_extract_short_addr` находит keyword `"улица"` → возвращает остаток до конца строки 2. `_TRAILING_HOUSE_RE = r',\s*\d+.*$'` → matches последний postal-index (`, 640144, Россия`) — но `.*$` greedy → strip последние 2 элемента, оставив всю админ-кишку 3. `_STREET_PREFIX_RE` отрезает `"улица "` 4. Result: `"8 Марта, Артек, Форум-Сити, Ленинский район, Екатеринбург, ..."` ⛔ 5. SQL `ILIKE '%<мусор>%'` → 0 matches в `tradein.deals` Verified локально на production-адресе перед fix. ## Fix Новая логика без `_extract_short_addr` промежуточного шага: ```python _STREET_KW_RE = re.compile(r"(?<![А-Яа-яёЁa-zA-Z])(?:ул\.|улица|пр\.|пр-т|проспект|...)\s+", IGNORECASE) _STREET_NAME_RE = re.compile( r"^([0-9]+\s+[А-Яа-яёЁ][А-Яа-яёЁ-]+" # "8 Марта" r"|[А-Яа-яёЁ][А-Яа-яёЁ-]+(?:\s+[А-Яа-яёЁ][А-Яа-яёЁ-]+){0,2})" # "Космонавтов" / "Большая Конюшенная" r"(?=,|\s+(?:д\.?\s*)?\d|\s*$)" # stop at comma / house number / EOL ) ``` Алгоритм: 1. Найти street-keyword (ул./улица/пр./...) — IGNORECASE, anywhere в строке 2. После keyword: захватить 1-3 слова до первой запятой / номера дома 3. Fallback: первый capitalized токен без keyword (для `"Большая Конюшенная, 25"`); фильтр admin-слов (Россия/область/район/...) Работает с **обоими** форматами: - Forward: `"Екатеринбург, ул. Космонавтов, 50"` → `"Космонавтов"` - Reverse: `"80, улица 8 Марта, Артек, ..."` → `"8 Марта"` ## Changes - `tradein-mvp/backend/app/services/estimator.py` — replaced `_TRAILING_HOUSE_RE` + `_STREET_PREFIX_RE` + old `extract_street_name` with new `_STREET_KW_RE` + `_STREET_NAME_RE` + rewritten function - `tradein-mvp/backend/tests/test_street_deals_endpoint.py` — `test_extract_street_name_parametrized` (10 cases: 4 forward, 2 reverse Nominatim, 4 edge cases) ## Verification - `ruff check` — pass - `pytest` — **15/15 pass** (5 pre-existing + 10 new parametrized) - После deploy ожидание: live запрос на `ул. 8 Марта` → `street="8 Марта"`, `count > 30` → секция в UI рендерится Refs: финальный hotfix 4-PR roadmap. После deploy: hard reload в браузере (Ctrl+Shift+R) для очистки JS-кеша.

lekss361 added 1 commit 2026-05-24 20:55:19 +00:00

fix(tradein): extract_street_name handles Nominatim reverse format 8de8daef01

Live bug: geocoder Nominatim возвращает адрес в REVERSE формате
"80, улица 8 Марта, район, ..., Россия" — старый regex брал всё после
"8 Марта" вместо stop'а перед первой запятой → ILIKE '%<huge mess>%'
возвращал 0 matches на проде.

Fix: новый _STREET_KW_RE ищет keyword (ул./улица/пр./...) затем
_STREET_NAME_RE капчит 1-3 слова имени улицы со stop'ом на comma или
house number. Работает с обоими форматами:
  forward:  "Екатеринбург, ул. Космонавтов, 50" → "Космонавтов"
  reverse:  "80, улица 8 Марта, район, ..." → "8 Марта"

Bonus fallback: первый capitalized токен (для "Большая Конюшенная, 25"
без keyword).

Tests: 10 parametrized cases в test_street_deals_endpoint.py.

lekss361 commented

2026-05-24 21:03:33 +00:00

Author

Owner

Deep Code Review #557 — verdict: APPROVE

Files: 2 (P1: services/estimator.py; P2: tests/test_street_deals_endpoint.py) · +83 / -29
Checked SHA: 8de8daef01 (matches prompt).

Live bug to fix mapping

Verified: PR body's count=0 symptom traces to _extract_short_addr returning the entire admin tail of REVERSE-format Nominatim string. New _STREET_KW_RE + _STREET_NAME_RE pair extracts the street name directly without the broken _TRAILING_HOUSE_RE greedy strip. Root-cause analysis is accurate.

Correctness — regex traced on real data

Locally executed both regexes against 23 cases (5 pre-existing forward, 6 new parametrized incl. real 80, улица 8 Марта, ... from prod). All pass. Also probed:

ул. Малышева, д. 1 → Малышева — resolves PR #555 review concern (д.N form now stripped via the trailing-house lookahead).
Космонавтов 12 (no comma) → Космонавтов — resolves second PR #555 concern.
ул. Тимофеева-Ресовского, 12 → Тимофеева-Ресовского (hyphenated names OK).
Leading whitespace, 2-word names (Большая Конюшенная), digit-prefix (8 Марта) — all OK.

Security

No new SQL touched; output still fed into parameterized street_pattern = "%" + street + "%" ILIKE bind (psycopg, not string-concat). No injection surface.
_STREET_KW_RE and _STREET_NAME_RE are anchored / bounded — no catastrophic backtracking risk (each alternative has fixed {0,2} quantifier).
re.IGNORECASE only on keyword regex — name regex relies on [А-Яа-яёЁ] character class which already accepts both cases. Fine.

Cross-file impact

_extract_short_addr is still used at estimator.py:1303 (different code path) — removal of its call from extract_street_name does NOT orphan it. Verified.
Single caller trade_in.py:1109 (/street-deals endpoint, PR #555). Output type/shape unchanged: str | None. Backward-compatible.
No DB / migration / API contract change.

Tests

All 5 cases from existing test_extract_street_name_basic still pass with new logic (traced manually).
New test_extract_street_name_parametrized adds 11 cases incl. the live-bug reproduction string and edge-case None inputs. Good regression coverage.
DB-mock tests untouched — no fixture rewrite needed.

Minor remaining edge cases (non-blocking, leave as follow-up)

Reverse format without keyword: 50, Космонавтов, Екатеринбург, Россия (Nominatim sometimes omits улица) → returns None. Real Nominatim usually includes улица/пр./etc., so blast is small.
bad substring filter false positives: Россиянская, 5 (fallback path, no keyword) — Россия in Россиянская blocks it. Use == or word-boundary match if this surfaces. Fallback path is rare.
Sets-of-words names: ул. Сакко и Ванцетти, 30 → None (lowercase и connector breaks 3-word constraint requiring capital starts). Real corner case; PR scope is the live bug, not exhaustive RU street naming.
Slash-separated: ул. Чкалова/Большакова, 5 → None. Out of scope.
Performance bounds on period_months/area_tolerance (raised in PR #555 review) remain unaddressed but PR is not the place for that.

Project conventions

Module-level compiled regex (good — no per-call compile).
Ruff line-100 respected. Type annotation str | None preserved.
Russian docstring + English fallback comments — consistent with surrounding code.

Complexity / blast radius

Risk: Low — single helper, behavior strictly widens (more inputs now resolve to non-None).
Reversibility: Trivial — revert one commit; extract_street_name is a leaf helper.
Merge window: OK now — production blocker for /street-deals endpoint.

Auto-merging (squash + branch delete).

## Deep Code Review #557 — verdict: APPROVE **Files**: 2 (P1: services/estimator.py; P2: tests/test_street_deals_endpoint.py) · +83 / -29 Checked SHA: 8de8daef0115948c99dba3c600dec01410236707 (matches prompt). ### Live bug to fix mapping Verified: PR body's `count=0` symptom traces to `_extract_short_addr` returning the entire admin tail of REVERSE-format Nominatim string. New `_STREET_KW_RE` + `_STREET_NAME_RE` pair extracts the street name directly without the broken `_TRAILING_HOUSE_RE` greedy strip. Root-cause analysis is accurate. ### Correctness — regex traced on real data Locally executed both regexes against 23 cases (5 pre-existing forward, 6 new parametrized incl. real `80, улица 8 Марта, ...` from prod). All pass. Also probed: - `ул. Малышева, д. 1` → `Малышева` — **resolves PR #555 review concern** (д.N form now stripped via the trailing-house lookahead). - `Космонавтов 12` (no comma) → `Космонавтов` — **resolves second PR #555 concern**. - `ул. Тимофеева-Ресовского, 12` → `Тимофеева-Ресовского` (hyphenated names OK). - Leading whitespace, 2-word names (`Большая Конюшенная`), digit-prefix (`8 Марта`) — all OK. ### Security - No new SQL touched; output still fed into parameterized `street_pattern = "%" + street + "%"` ILIKE bind (psycopg, not string-concat). No injection surface. - `_STREET_KW_RE` and `_STREET_NAME_RE` are anchored / bounded — no catastrophic backtracking risk (each alternative has fixed `{0,2}` quantifier). - `re.IGNORECASE` only on keyword regex — name regex relies on `[А-Яа-яёЁ]` character class which already accepts both cases. Fine. ### Cross-file impact - `_extract_short_addr` is **still used at estimator.py:1303** (different code path) — removal of its call from `extract_street_name` does NOT orphan it. Verified. - Single caller `trade_in.py:1109` (`/street-deals` endpoint, PR #555). Output type/shape unchanged: `str | None`. Backward-compatible. - No DB / migration / API contract change. ### Tests - All 5 cases from existing `test_extract_street_name_basic` still pass with new logic (traced manually). - New `test_extract_street_name_parametrized` adds 11 cases incl. the live-bug reproduction string and edge-case None inputs. Good regression coverage. - DB-mock tests untouched — no fixture rewrite needed. ### Minor remaining edge cases (non-blocking, leave as follow-up) 1. **Reverse format without keyword**: `50, Космонавтов, Екатеринбург, Россия` (Nominatim sometimes omits `улица`) → returns `None`. Real Nominatim usually includes `улица`/`пр.`/etc., so blast is small. 2. **`bad` substring filter false positives**: `Россиянская, 5` (fallback path, no keyword) — `Россия in Россиянская` blocks it. Use `==` or word-boundary match if this surfaces. Fallback path is rare. 3. **Sets-of-words names**: `ул. Сакко и Ванцетти, 30` → `None` (lowercase `и` connector breaks 3-word constraint requiring capital starts). Real corner case; PR scope is the live bug, not exhaustive RU street naming. 4. **Slash-separated**: `ул. Чкалова/Большакова, 5` → `None`. Out of scope. 5. Performance bounds on `period_months`/`area_tolerance` (raised in PR #555 review) remain unaddressed but PR is not the place for that. ### Project conventions - Module-level compiled regex (good — no per-call compile). - Ruff line-100 respected. Type annotation `str | None` preserved. - Russian docstring + English fallback comments — consistent with surrounding code. ### Complexity / blast radius - Risk: **Low** — single helper, behavior strictly widens (more inputs now resolve to non-None). - Reversibility: **Trivial** — revert one commit; `extract_street_name` is a leaf helper. - Merge window: **OK now** — production blocker for /street-deals endpoint. Auto-merging (squash + branch delete).