feat(tradein-imv): retry with cleaned address + categorize errors #505

Merged

lekss361 merged 1 commit from feat/tradein-imv-retry-cleaned-address into main

2026-05-24 11:04:25 +00:00

lekss361 commented

2026-05-24 10:59:08 +00:00

Owner

Problem

Estimate a0a0b820-e8a8-4eee-aa73-0ab3b98ac233 → IMV available:false because address contained prefix Склад, which Avito geocoder rejected.

Fix

IMVAuthError / IMVTransientError exceptions for proper categorization (was silent warning)
Retry once with cleaned address: re.sub(r"(Склад|Гараж|Подсобка|Нежилое|Помещение|Цех),\s*", "", addr)
Probe script scripts/probe-imv-by-address.py for VPS-side diagnosis

Roadmap

PR 5 of 8 — [[Decision_TradeIn_DataQuality_8PR_Roadmap]].

## Problem Estimate `a0a0b820-e8a8-4eee-aa73-0ab3b98ac233` → IMV `available:false` because address contained prefix `Склад,` which Avito geocoder rejected. ## Fix - `IMVAuthError` / `IMVTransientError` exceptions for proper categorization (was silent warning) - Retry once with cleaned address: `re.sub(r"(Склад|Гараж|Подсобка|Нежилое|Помещение|Цех),\s*", "", addr)` - Probe script `scripts/probe-imv-by-address.py` for VPS-side diagnosis ## Roadmap PR 5 of 8 — `[[Decision_TradeIn_DataQuality_8PR_Roadmap]]`.

lekss361 added 1 commit 2026-05-24 10:59:08 +00:00

feat(tradein-imv): retry with cleaned address + categorize errors b78dfae02d

IMV returned available:false for estimate a0a0b820-... because target address
contained junk prefix 'Склад,'. Add:
- IMVAuthError / IMVTransientError для категоризации (вместо silent warning)
- Retry once с stripped prefix (Склад|Гараж|Подсобка|Нежилое|Помещение|Цех)
- probe-imv-by-address.py для диагностики на VPS

Source: estimate a0a0b820-e8a8-4eee-aa73-0ab3b98ac233 (imv available:false).

lekss361 commented

2026-05-24 11:04:14 +00:00

Author

Owner

Deep Code Review — verdict APPROVE

Summary

Status: APPROVE
Files: 3 (P1: estimator.py + avito_imv.py · P3: probe script)
Lines: +168 / -4 · Scope: PR 5/8 of Decision_TradeIn_DataQuality_8PR_Roadmap
Goal: фикс IMV available:false для адресов с префиксами Склад, / Гараж, / etc + типизация error categorization

Retry logic correctness — OK

_get_or_fetch_imv_cached (estimator.py:220-266): catch IMVAddressNotFoundError → cleaned = _NOISE_PREFIX_RE.sub("", address) → guard if cleaned != address (исключает бесконечную попытку на already-clean адресах) → ровно один retry через evaluate_via_imv(cleaned, ...).
Внутренний except IMVAddressNotFoundError падает в return None без рекурсии — повторная очистка уже очищенного адреса невозможна, т.к. retry зовёт evaluate_via_imv напрямую, а не _get_or_fetch_imv_cached.
**Нет infinite loop. ** Bounded by single retry attempt.

Error categorization mapping — OK

_raise_for_status_categorized: 401/403 → IMVAuthError · ≥500 → IMVTransientError · network/timeout (через try/except вокруг session.get/post) → IMVTransientError.
IMVAuthError (estimator.py:256) → logger.error + None — корректный сигнал «требует human attention».
IMVTransientError (estimator.py:261) → logger.warning + None — комментарий «skipping retry in estimator context» явно объясняет policy decision (estimator не блокирующий).
Order ловли исключений (IMVAddressNotFoundError → IMVAuthError → IMVTransientError → Exception) — корректный, нет shadow capture.

Cross-file impact

admin.py:511-527 (POST /scrape/avito-imv debug endpoint) импортирует evaluate_via_imv. После PR может бросить IMVAuthError / IMVTransientError — оба попадают в существующий except Exception → HTTP 502. Behavior preserved, но категория ошибки в логе теперь информативнее (logger.exception сохранит traceback с типом). Не блокер.
tests/test_estimator_imv_integration.py, tests/test_avito_imv_parse.py — ничего не сломано (новые exceptions optional, evaluate_via_imv API не меняется).
IMVCityMismatchError определён но нигде не raise/catch — pre-existing dead code, не вводится этим PR.

Edge cases

_NOISE_PREFIX_RE требует запятую после префикса → «ул. Складская» НЕ ложно-срабатывает (no false-positive clip).
re.IGNORECASE корректно ловит «склад,» / «Склад,» / «СКЛАД,».
Module-level re.compile — без warm-up cost на каждый estimate.

Vault cross-ref

Полностью соответствует roadmap PR 5 (steps 1-3): probe script (#1), categorized logging (#2), retry с clean (#3).
Gap (Low): step 4 imv_lookup_success_rate метрика в /cache-stats не реализована. Можно отдельным follow-up PR; не блокер для merge т.к. logging уже categorized.
Gap (Low): acceptance criterion «imv-benchmark для целевого UUID возвращает available:true» можно проверить только post-deploy через probe-script. План OK.

Minor observations (non-blocking)

🟢 429 (rate limit) попадает в «other 4xx» → resp.raise_for_status() → standard HTTPStatusError → caught by except Exception → silent None. Pre-existing behavior, не введено этим PR; в Avito-контексте может быть полезно отдельно типизировать как IMVRateLimitError (future enhancement).
🟢 Probe script (scripts/probe-imv-by-address.py): дублирует regex pattern из estimator.py. Для diagnostic утилиты приемлемо (no shared module overhead), но при будущем расширении списка префиксов придётся синхронизировать оба места.
🟢 Логи через address[:60] — корректно обрезают для compactness без leak полного адреса.

Positive

Защитный guard if cleaned != address — defensive correctness против infinite loop.
Network error → from exc сохраняет original cause в traceback.
Inner exception handling в retry block: separate ловит IMVAddressNotFoundError (известный outcome) vs Exception (unexpected) — корректное разделение «expected miss» / «error to log».
Probe script — отличный pre-deploy diagnostic tool, соответствует workflow «test before claim done».
Comment над _NOISE_PREFIX_RE объясняет почему именно эти префиксы (Avito-геокодер не распознаёт нежилые назначения).

Verdict

Merge ready. Per [Auto-merge any scope] (2026-05-16): squash + delete branch.

## Deep Code Review — verdict APPROVE  ### Summary - Status: APPROVE - Files: 3 (P1: estimator.py + avito_imv.py · P3: probe script) - Lines: +168 / -4 · Scope: PR 5/8 of [[Decision_TradeIn_DataQuality_8PR_Roadmap]] - Goal: фикс IMV `available:false` для адресов с префиксами `Склад,` / `Гараж,` / etc + типизация error categorization ### Retry logic correctness — OK - `_get_or_fetch_imv_cached` (estimator.py:220-266): catch `IMVAddressNotFoundError` → `cleaned = _NOISE_PREFIX_RE.sub("", address)` → guard `if cleaned != address` (исключает бесконечную попытку на already-clean адресах) → ровно **один** retry через `evaluate_via_imv(cleaned, ...)`. - Внутренний `except IMVAddressNotFoundError` падает в `return None` без рекурсии — повторная очистка уже очищенного адреса невозможна, т.к. retry зовёт `evaluate_via_imv` напрямую, а не `_get_or_fetch_imv_cached`. - **Нет infinite loop. ** Bounded by single retry attempt. ### Error categorization mapping — OK - `_raise_for_status_categorized`: 401/403 → `IMVAuthError` · ≥500 → `IMVTransientError` · network/timeout (через `try/except` вокруг `session.get/post`) → `IMVTransientError`. - `IMVAuthError` (estimator.py:256) → `logger.error` + None — корректный сигнал «требует human attention». - `IMVTransientError` (estimator.py:261) → `logger.warning` + None — комментарий «skipping retry in estimator context» явно объясняет policy decision (estimator не блокирующий). - Order ловли исключений (`IMVAddressNotFoundError` → `IMVAuthError` → `IMVTransientError` → `Exception`) — корректный, нет shadow capture. ### Cross-file impact - `admin.py:511-527` (`POST /scrape/avito-imv` debug endpoint) импортирует `evaluate_via_imv`. После PR может бросить `IMVAuthError` / `IMVTransientError` — оба попадают в существующий `except Exception` → HTTP 502. **Behavior preserved**, но категория ошибки в логе теперь информативнее (`logger.exception` сохранит traceback с типом). Не блокер. - `tests/test_estimator_imv_integration.py`, `tests/test_avito_imv_parse.py` — ничего не сломано (новые exceptions optional, `evaluate_via_imv` API не меняется). - `IMVCityMismatchError` определён но нигде не raise/catch — pre-existing dead code, не вводится этим PR. ### Edge cases - `_NOISE_PREFIX_RE` требует запятую после префикса → «ул. Складская» НЕ ложно-срабатывает (no false-positive clip). - `re.IGNORECASE` корректно ловит «склад,» / «Склад,» / «СКЛАД,». - Module-level `re.compile` — без warm-up cost на каждый estimate. ### Vault cross-ref - Полностью соответствует roadmap PR 5 (steps 1-3): probe script (#1), categorized logging (#2), retry с clean (#3). - **Gap (Low):** step 4 `imv_lookup_success_rate` метрика в `/cache-stats` не реализована. Можно отдельным follow-up PR; не блокер для merge т.к. logging уже categorized. - **Gap (Low):** acceptance criterion «`imv-benchmark` для целевого UUID возвращает `available:true`» можно проверить только post-deploy через probe-script. План OK. ### Minor observations (non-blocking) - 🟢 429 (rate limit) попадает в «other 4xx» → `resp.raise_for_status()` → standard `HTTPStatusError` → caught by `except Exception` → silent None. Pre-existing behavior, не введено этим PR; в Avito-контексте может быть полезно отдельно типизировать как `IMVRateLimitError` (future enhancement). - 🟢 Probe script (`scripts/probe-imv-by-address.py`): дублирует regex pattern из estimator.py. Для diagnostic утилиты приемлемо (no shared module overhead), но при будущем расширении списка префиксов придётся синхронизировать оба места. - 🟢 Логи через `address[:60]` — корректно обрезают для compactness без leak полного адреса. ### Positive - Защитный guard `if cleaned != address` — defensive correctness против infinite loop. - Network error → `from exc` сохраняет original cause в traceback. - Inner exception handling в retry block: separate ловит `IMVAddressNotFoundError` (известный outcome) vs `Exception` (unexpected) — корректное разделение «expected miss» / «error to log». - Probe script — отличный pre-deploy diagnostic tool, соответствует workflow «test before claim done». - Comment над `_NOISE_PREFIX_RE` объясняет почему именно эти префиксы (Avito-геокодер не распознаёт нежилые назначения). ### Verdict Merge ready. Per [Auto-merge any scope] (2026-05-16): squash + delete branch.