lekss361/gendesign

Fork 0

tradein/geocode: хвосты после #2601 — непрошитый sibling-caller в geocode_deals_nominatim.py + наблюдаемость гейта #2603

New issue

Closed

opened 2026-07-31 21:41:15 +00:00 by lekss361 · 2 comments

lekss361 commented

2026-07-31 21:41:15 +00:00

Owner

Два хвоста из глубокого ревью PR #2601 (замыкание петли city_hint). Оба не блокировали мерж, но оставлять их не стоит.

1. Непрошитый sibling-caller (функциональный)

tradein-mvp/backend/scripts/geocode_deals_nominatim.py:313 — await geocode(address, db) без city_hint, плюс группировка GROUP BY address (строки 176-189). Это ровно та форма, которую #2601 починил в app/tasks/geocode_missing.py: один текст адреса на несколько городов схлопывается в одну группу и геокодируется без города.

Причём здесь эффект сильнее, чем был в listings: скрипт работает по deals, где колонка city заполнена на 100% (0 NULL, 370 различных значений) — то есть подсказка доступна для каждой строки и просто выбрасывается.

Скрипт ручной (не в scheduler'е), поэтому в scope #2601 не входил. Лечится так же: группировка по паре (address, city), city_hint=city, UPDATE через IS NOT DISTINCT FROM.

⚠️ Нюанс, который выяснился при ревью: deals.city — росреестровое, и в хвосте распределения попадаются сомнительные значения (Бессонова, Бердюгина, Билейский рыбопитомник). Любой не-ЕКБ хинт жёстко закрывает EKB-локальные тиры, поэтому мусорное значение города сделает геокодирование строки хуже, а не лучше. Перед прошивкой стоит либо валидировать город по словарю, либо передавать хинт только для значений из известного набора.

2. Наблюдаемость городского гейта

app/tasks/backfill_listings_coords_geoportal.py:229 — новый column-гейт переиспользует счётчик skipped_non_ekb. Счётчик уходит не только в лог, но и в scrape_runs.counters через to_counters().

Проблема в порядке: column-гейт стоит перед парсером адреса, поэтому после раскатки областных развёрток строки, которые сейчас попадают в no_address (13734 из 13742 кандидатов — 99.94%), начнут перетекать в skipped_non_ekb. Счётчик поменяет смысл ровно в тот момент, когда по нему хочется валидировать раскатку #2598.

Лечится тремя строками, обратно совместимо:

skipped_non_ekb_by_column: int = 0   # + в to_counters()

Мелочи (по желанию, одной строкой каждая)

tests/tasks/test_geocode_missing.py — test_admin_geocode_missing_passes_city_hint покрывает только target="listings"; параметризация на "deals".
app/tasks/geocode_missing.py:194 — dry-run лог не печатает city, хотя это теперь часть ключа группы.

Связано: #2594, #2601, #2576.

Два хвоста из глубокого ревью PR #2601 (замыкание петли `city_hint`). Оба не блокировали мерж, но оставлять их не стоит. ## 1. Непрошитый sibling-caller (функциональный) `tradein-mvp/backend/scripts/geocode_deals_nominatim.py:313` — `await geocode(address, db)` без `city_hint`, плюс группировка `GROUP BY address` (строки 176-189). Это **ровно та форма**, которую #2601 починил в `app/tasks/geocode_missing.py`: один текст адреса на несколько городов схлопывается в одну группу и геокодируется без города. Причём здесь эффект сильнее, чем был в `listings`: скрипт работает по `deals`, где колонка `city` заполнена **на 100%** (0 NULL, 370 различных значений) — то есть подсказка доступна для каждой строки и просто выбрасывается. Скрипт ручной (не в scheduler'е), поэтому в scope #2601 не входил. Лечится так же: группировка по паре `(address, city)`, `city_hint=city`, UPDATE через `IS NOT DISTINCT FROM`. ⚠️ Нюанс, который выяснился при ревью: `deals.city` — росреестровое, и в хвосте распределения попадаются сомнительные значения (`Бессонова`, `Бердюгина`, `Билейский рыбопитомник`). Любой не-ЕКБ хинт жёстко закрывает EKB-локальные тиры, поэтому мусорное значение города сделает геокодирование строки хуже, а не лучше. Перед прошивкой стоит либо валидировать город по словарю, либо передавать хинт только для значений из известного набора. ## 2. Наблюдаемость городского гейта `app/tasks/backfill_listings_coords_geoportal.py:229` — новый column-гейт переиспользует счётчик `skipped_non_ekb`. Счётчик уходит не только в лог, но и в `scrape_runs.counters` через `to_counters()`. Проблема в порядке: column-гейт стоит **перед** парсером адреса, поэтому после раскатки областных развёрток строки, которые сейчас попадают в `no_address` (13734 из 13742 кандидатов — 99.94%), начнут перетекать в `skipped_non_ekb`. Счётчик поменяет смысл ровно в тот момент, когда по нему хочется валидировать раскатку #2598. Лечится тремя строками, обратно совместимо: ```python skipped_non_ekb_by_column: int = 0 # + в to_counters() ``` ## Мелочи (по желанию, одной строкой каждая) - `tests/tasks/test_geocode_missing.py` — `test_admin_geocode_missing_passes_city_hint` покрывает только `target="listings"`; параметризация на `"deals"`. - `app/tasks/geocode_missing.py:194` — dry-run лог не печатает `city`, хотя это теперь часть ключа группы. Связано: #2594, #2601, #2576.

lekss361 referenced this issue

2026-08-01 18:30:42 +00:00

[CRITICAL] tradein: скрапер не пишет город в адрес объявления — данные области уходят в Екатеринбург #2594

Light1YT referenced this issue from a commit

2026-08-05 12:36:16 +00:00

fix(tradein/geocode): прошить city_hint в deals-скрипт + развести счётчики гейта (#2603)

bot-backend referenced this issue

2026-08-05 12:37:05 +00:00

fix(tradein/geocode): прошить city_hint в deals-скрипт + развести счётчики гейта (#2603) #2655

bot-backend commented

2026-08-05 12:37:12 +00:00

Collaborator

Working on this in PR #2655

Light1YT referenced this issue from a commit

2026-08-05 15:20:45 +00:00

fix(tradein/geocode): гейт мусорного города вынести в общий хелпер и прошить в admin-путь (#2603)

bot-backend referenced this issue from a commit

2026-08-05 15:29:22 +00:00

Merge pull request 'fix(tradein/geocode): прошить city_hint в deals-скрипт + развести счётчики гейта (#2603)' (#2655) from fix/2603-geocode-city-hint-tails into main

bot-backend commented

2026-08-05 15:29:45 +00:00

Collaborator

Закрыто — PR #2655 смержен.

Сиблингов оказалось не один, а три

Issue называла непрошитым scripts/geocode_deals_nominatim.py. Грепом по всем потребителям city_hint нашлись ещё два, и более важных:

POST /admin/geocode-missing?target=deals (api/v1/admin.py) — живой путь, работает постоянно;
задача geocode_missing.

Скрипт запускают руками, admin-ручка работает всегда — чинить только скрипт значило бы починить редкую копию при сломанной основной. Поэтому гейт вынесен в общий geocoder.known_city_hint и переиспользуется всеми тремя: копий функции нет, четвёртый потребитель получит проверку сам.

Набор городов — существующий, не выдуманный

SVERDLOVSK_OBLAST_CITIES — тот же frozenset, который уже питает _names_non_ekb_city, _ekb_local_tiers_allowed и _has_oblast_marker, и который переиспользует estimator._resolve_target_city для скоупа ДКП-коридора. То есть подсказка проверяется ровно тем словарём, в который она потом и приходит. Отдельного справочника городов в конфигах и data/sql/ нет — там ссылки на этот же набор.

Нужно это ровно из-за нюанса, отмеченного в issue: deals.city росреестровое, заполнено на 100%, но в хвосте лежат не-города («Бессонова», «Билейский рыбопитомник»). Любой не-ЕКБ хинт жёстко закрывает EKB-локальные тиры и уезжает префиксом в запрос провайдеру — мусорный хинт хуже отсутствия хинта.

Остальное из issue

Группировка в скрипте — по паре (address, city), UPDATE через IS NOT DISTINCT FROM.
Счётчик гейта разведён: skipped_non_ekb_by_column добавлен в to_counters(). Общий skipped_non_ekb намеренно остался суммой обоих гейтов, новый — вклад колоночного; разложение вычитанием, старые строки без ключа читаются как «весь вклад текстового гейта». Обратно совместимо.
test_admin_geocode_missing_passes_city_hint параметризован на deals. Честная оговорка: этот тест проходит и на старом коде — это добор покрытия, а не фикс поведения.
city добавлен в dry-run лог geocode_missing.

Проверка

108 тестов затронутых модулей + 139 после вливания main (PR конфликтовал с #2654 по admin.py, слил и перепроверил обе стороны). Фальсификация: с застэшенной реализацией все три новых теста краснеют — то есть они действительно стерегут поведение, а не совпадают с подстрокой.

Закрыто — PR #2655 смержен. ## Сиблингов оказалось не один, а три Issue называла непрошитым `scripts/geocode_deals_nominatim.py`. Грепом по всем потребителям `city_hint` нашлись ещё два, и **более важных**: - `POST /admin/geocode-missing?target=deals` (`api/v1/admin.py`) — живой путь, работает постоянно; - задача `geocode_missing`. Скрипт запускают руками, admin-ручка работает всегда — чинить только скрипт значило бы починить редкую копию при сломанной основной. Поэтому гейт вынесен в общий `geocoder.known_city_hint` и переиспользуется всеми тремя: копий функции нет, четвёртый потребитель получит проверку сам. ## Набор городов — существующий, не выдуманный `SVERDLOVSK_OBLAST_CITIES` — тот же frozenset, который уже питает `_names_non_ekb_city`, `_ekb_local_tiers_allowed` и `_has_oblast_marker`, и который переиспользует `estimator._resolve_target_city` для скоупа ДКП-коридора. То есть подсказка проверяется ровно тем словарём, в который она потом и приходит. Отдельного справочника городов в конфигах и `data/sql/` нет — там ссылки на этот же набор. Нужно это ровно из-за нюанса, отмеченного в issue: `deals.city` росреестровое, заполнено на 100%, но в хвосте лежат не-города («Бессонова», «Билейский рыбопитомник»). Любой не-ЕКБ хинт жёстко закрывает EKB-локальные тиры и уезжает префиксом в запрос провайдеру — мусорный хинт **хуже отсутствия хинта**. ## Остальное из issue - Группировка в скрипте — по паре `(address, city)`, UPDATE через `IS NOT DISTINCT FROM`. - Счётчик гейта разведён: `skipped_non_ekb_by_column` добавлен в `to_counters()`. Общий `skipped_non_ekb` намеренно остался суммой обоих гейтов, новый — вклад колоночного; разложение вычитанием, старые строки без ключа читаются как «весь вклад текстового гейта». Обратно совместимо. - `test_admin_geocode_missing_passes_city_hint` параметризован на `deals`. Честная оговорка: этот тест проходит и на старом коде — это добор покрытия, а не фикс поведения. - `city` добавлен в dry-run лог `geocode_missing`. ## Проверка 108 тестов затронутых модулей + 139 после вливания main (PR конфликтовал с #2654 по `admin.py`, слил и перепроверил обе стороны). Фальсификация: с застэшенной реализацией все три новых теста краснеют — то есть они действительно стерегут поведение, а не совпадают с подстрокой.

bot-backend closed this issue

2026-08-05 15:29:52 +00:00