fix(tradein-imv): adapt to Avito 3-step API contract (2026-05+) #524

Merged
lekss361 merged 1 commit from feat/tradein-imv-fix-3step-api into main 2026-05-24 13:48:53 +00:00
Owner

Problem

Avito changed their IMV flow. Our _geocode() expected geoHash directly from /web/1/coords/by_address, but that endpoint no longer returns it — only components/normalizedAddress/point/postalCode. Effect: IMVAddressNotFoundError for EVERY estimate since contract change → imv-benchmark available:false everywhere. PR #505 retry-with-cleaned-address never helped because both attempts went through the same broken flow.

Discovery (via playwright in production browser)

Current contract has 3 steps:

Step Endpoint Purpose
A GET /web/1/coords/by_address?address=… normalize + lat/lon
B POST /js/v2/geo/position rich JWT (geoFieldsHash) with addressId/locationId/metroId/districtIdwe were missing this
C POST /web/1/realty-imv/get-data recommendedPrice/lowerPrice/higherPrice/count/suggestions/placementHistory

geoFieldsHash from step B is what /realty-imv/get-data expects as geoHash field.

Step B request body

{
  "address": "<normalizedAddress from step A>",
  "addressId": "",
  "categoryId": 24,
  "isRadius": false,
  "latitude": 56.828153,
  "longitude": 60.548462
}

Step B response

{
  "address": "Свердловская область, Екатеринбург, Заводская улица, 44А",
  "addressId": 22292600,
  "districtId": 787,
  "geoFieldsHash": "eyJhbG…JWT…",
  "latitude": 56.828153, "longitude": 60.548462,
  "locationId": 654070, "metroId": 2045
}

Verified live

Full chain works for production estimate a0a0b820-e8a8-4eee-aa73-0ab3b98ac233 (Заводская 44-а):

IMV geocode A: address=… → lat=56.828153, lon=60.548462
IMV geocode B: addressId=22292600, locationId=654070, metroId=2045
imv: fresh recommended=7,520,000 range=(7,294,400, 7,896,000) count=800

Avito IMV = 7.52 М ₽ vs our estimator 6.58 М ₽ (standard repair) — разница +14% это разумный benchmark gap (Avito count=800 включая стройку, у нас 13 secondary-only analogs).

Fix

  • _geocode() rewritten as two-step (A + B) with proper exception handling (IMVAddressNotFoundError / IMVTransientError / _raise_for_status_categorized)
  • New const POSITION_ENDPOINT = "/js/v2/geo/position"
  • _imv_evaluate() (step C) unchanged — already reads data.price.recommendedPrice/lowerPrice/higherPrice/count
  • Probe script unchanged — uses public evaluate_via_imv() entrypoint

Verify after deploy

ssh gendesign "docker exec tradein-backend python /app/scripts/probe-imv-by-address.py \"Свердловская область, г. Екатеринбург, ул. Заводская, д. 44-а\""
# expected: recommended=7520000 (or similar live value)

Roadmap follow-up

Follow-up to [[Decision_TradeIn_DataQuality_8PR_Roadmap]] PR 5 (#505) and [[Event_TradeIn_DataQuality_8PR_Done_May24]] open item «Avito IMV fallback». Discovered via playwright when investigating why imv-benchmark still available:false after roadmap completion.

## Problem Avito changed their IMV flow. Our `_geocode()` expected `geoHash` directly from `/web/1/coords/by_address`, but that endpoint **no longer returns it** — only `components/normalizedAddress/point/postalCode`. Effect: `IMVAddressNotFoundError` for EVERY estimate since contract change → `imv-benchmark` `available:false` everywhere. PR #505 retry-with-cleaned-address never helped because both attempts went through the same broken flow. ## Discovery (via playwright in production browser) Current contract has **3 steps**: | Step | Endpoint | Purpose | |------|----------|---------| | A | `GET /web/1/coords/by_address?address=…` | normalize + lat/lon | | B | `POST /js/v2/geo/position` | rich JWT (`geoFieldsHash`) with `addressId/locationId/metroId/districtId` ← **we were missing this** | | C | `POST /web/1/realty-imv/get-data` | `recommendedPrice/lowerPrice/higherPrice/count/suggestions/placementHistory` | `geoFieldsHash` from step B is what `/realty-imv/get-data` expects as `geoHash` field. ### Step B request body ```json { "address": "<normalizedAddress from step A>", "addressId": "", "categoryId": 24, "isRadius": false, "latitude": 56.828153, "longitude": 60.548462 } ``` ### Step B response ```json { "address": "Свердловская область, Екатеринбург, Заводская улица, 44А", "addressId": 22292600, "districtId": 787, "geoFieldsHash": "eyJhbG…JWT…", "latitude": 56.828153, "longitude": 60.548462, "locationId": 654070, "metroId": 2045 } ``` ## Verified live Full chain works for production estimate `a0a0b820-e8a8-4eee-aa73-0ab3b98ac233` (Заводская 44-а): ``` IMV geocode A: address=… → lat=56.828153, lon=60.548462 IMV geocode B: addressId=22292600, locationId=654070, metroId=2045 imv: fresh recommended=7,520,000 range=(7,294,400, 7,896,000) count=800 ``` **Avito IMV = 7.52 М ₽** vs our estimator **6.58 М ₽** (standard repair) — разница +14% это разумный benchmark gap (Avito count=800 включая стройку, у нас 13 secondary-only analogs). ## Fix - `_geocode()` rewritten as two-step (A + B) with proper exception handling (`IMVAddressNotFoundError` / `IMVTransientError` / `_raise_for_status_categorized`) - New const `POSITION_ENDPOINT = "/js/v2/geo/position"` - `_imv_evaluate()` (step C) unchanged — already reads `data.price.recommendedPrice/lowerPrice/higherPrice/count` - Probe script unchanged — uses public `evaluate_via_imv()` entrypoint ## Verify after deploy ```bash ssh gendesign "docker exec tradein-backend python /app/scripts/probe-imv-by-address.py \"Свердловская область, г. Екатеринбург, ул. Заводская, д. 44-а\"" # expected: recommended=7520000 (or similar live value) ``` ## Roadmap follow-up Follow-up to `[[Decision_TradeIn_DataQuality_8PR_Roadmap]]` PR 5 (#505) and `[[Event_TradeIn_DataQuality_8PR_Done_May24]]` open item «Avito IMV fallback». Discovered via playwright when investigating why `imv-benchmark` still `available:false` after roadmap completion.
lekss361 added 1 commit 2026-05-24 13:40:43 +00:00
Avito changed IMV flow — /web/1/coords/by_address no longer returns
geoHash. New flow has 3 endpoints:

1. /web/1/coords/by_address       -> normalize + lat/lon
2. /js/v2/geo/position             -> rich JWT (geoFieldsHash) with addressId/
                                       locationId/metroId/districtId
3. /web/1/realty-imv/get-data      -> recommendedPrice/lowerPrice/higherPrice

Old code skipped step 2 -> IMVAddressNotFoundError for every address since
contract change. PR #505 retry-with-cleaned-address never had a chance.

Verified via playwright browser for production estimate
a0a0b820-e8a8-4eee-aa73-0ab3b98ac233 (Zavodskaya 44-a): Avito IMV
recommendedPrice=7,520,000 RUB (range 7,294,400-7,896,000).
Author
Owner

Deep Code Review — PR #524 verdict: APPROVE

Status: APPROVE
Files: 1 (P1: tradein-mvp/backend/app/services/scrapers/avito_imv.py)
Lines: +79 / -31
SHA: c897d57 (verified head match)

Summary

Surgical fix adapting _geocode() to Avito's current 3-step contract (2026-05+). Replaces the broken 1-call geocode (which expected geoHash directly from /coords/by_address — endpoint no longer returns it) with a two-step A→B sequence: A normalizes address + returns coords, B issues POST /js/v2/geo/position and returns geoFieldsHash (JWT) used as geoHash in step C. Step C (_imv_evaluate) unchanged.

Cross-file impact

  • Public API: evaluate_via_imv(...) signature unchanged. IMVEvaluation / IMVGeo dataclasses unchanged. Caller services/estimator.py::_get_or_fetch_imv_cached catches IMVAddressNotFoundError + generic Exception → returns None (graceful). New IMVAuthError/IMVTransientError raises are subsumed by except Exception (intended).
  • DB schema: avito_imv_evaluations.geo_hash column unchanged — still stores the value used as geoHash in step C body (now a JWT instead of a hash, but the column is text). No migration needed.
  • Cache safety: 24h cache hit returns reconstructed IMVEvaluation without re-calling step C → JWT expiry inside cached geo_hash is irrelevant on hit.
  • Tests: tests/test_avito_imv_parse.py is offline-only (cache_key, parse_title, parse_placement_item, dataclasses) — unaffected.
  • PR #505 regression check: grep'нул cleaned|retry|Склад|Гараж|strip_prefix в app/services/{estimator.py,scrapers/avito_imv.py} — no IMV-retry logic exists in current code. PR diff doesn't touch retry; description's PR #505 reference is context, not a code removal.

Seven-dimension scan

  • A. Security 🔒 — no secrets/credentials/SQL injection touched. Step B body fields (address, categoryId, lat/lon) are derived from step A response or fixed constants. No user input to JSON injection vector.
  • B. Correctness 🎯 — state machine is linear: A fail → no B; B fail → no C. Each step has its own typed try/except. _own_session is always defined when finally executes (ImportError raises before try).
  • C. Performance — adds 1 HTTP RTT per cache miss (Step B). 24h cache fronts it. No new locks/N+1.
  • D. Conventions 📋 — type hints, _raise_for_status_categorized reused for both A+B (consistent), _COMMON_HEADERS reused. logger.info for step boundaries (observable).
  • E. Architecture 🏗 — DRY (categorized status helper covers both new calls), no dead code added.
  • F. Tests 🧪 — no new unit tests for step B (acceptable: HTTP-shim, hard to unit-test without VCR; PR provides live probe command). 🟢 low
  • G. Vault cross-check 📚 — implementation matches Bug_Avito_Imv_Houses_Catalog_Broken_OPEN.md plan verbatim: point.lat/lon extraction, POST /js/v2/geo/position, return geoFieldsHash as geo_hash, _imv_evaluate() unchanged, DB schema unchanged.

🟡 Minor (non-blocking)

  • avito_imv.py:355if not lat or not lon or not normalized: trips on lat=0.0/lon=0.0 (Gulf of Guinea edge case). For real RU addresses negligible, but is None would be precise: if lat is None or lon is None or not normalized:.
  • avito_imv.py:367categoryId: 24 hardcoded — current caller is apartments-only, fine. Worth a constant CATEGORY_ID_APARTMENT = 24 for future expansion.
  • No asyncio.sleep between A→B→C. Browser does the same so likely OK, but if Avito starts 429-ing keep it on the radar.
  • IMVCityMismatchError defined but never raised (pre-existing dead exception, not introduced here).

🟢 Positive

  • Typed exception categorization (IMVAddressNotFoundError / IMVAuthError / IMVTransientError) applied symmetrically across step A and step B network/HTTP failures.
  • Error message for B includes errorText= from response — debuggable.
  • logger.info lines log addressId/locationId/metroId after B — observability for future contract drift detection.
  • Docstring updated with full 3-step flow and updated Raises: list.
  • Diff is tightly scoped — only _geocode() rewritten, no churn elsewhere.

Live verification

PR includes concrete production trace a0a0b820-e8a8-4eee-aa73-0ab3b98ac233 (Заводская 44-а) → recommended=7,520,000 range=(7,294,400, 7,896,000) count=800. Step B confirmed returning addressId=22292600 locationId=654070 metroId=2045. Probe command in PR body for post-deploy revalidation.

  1. Merge as-is.
  2. Post-deploy: run probe command from PR body to confirm recommended non-zero on a fresh address.
  3. (Optional follow-up, not blocker) Monitor whether any address that previously succeeded with 1-step now fails at step B — Avito may return errorText we want to alarm on.

Complexity / blast radius

  • Risk: low — fix targets a currently-broken code path; worst case = unchanged behavior (still returns None gracefully).
  • Reversibility: trivial revert (single-file diff).
  • Merge window: anytime.
<!-- gendesign-review-bot: sha=c897d57 verdict=approve --> ## Deep Code Review — PR #524 verdict: APPROVE **Status**: ✅ APPROVE **Files**: 1 (P1: tradein-mvp/backend/app/services/scrapers/avito_imv.py) **Lines**: +79 / -31 **SHA**: c897d57 (verified head match) ### Summary Surgical fix adapting `_geocode()` to Avito's current 3-step contract (2026-05+). Replaces the broken 1-call geocode (which expected `geoHash` directly from `/coords/by_address` — endpoint no longer returns it) with a two-step A→B sequence: A normalizes address + returns coords, B issues `POST /js/v2/geo/position` and returns `geoFieldsHash` (JWT) used as `geoHash` in step C. Step C (`_imv_evaluate`) unchanged. ### Cross-file impact - **Public API**: `evaluate_via_imv(...)` signature unchanged. `IMVEvaluation` / `IMVGeo` dataclasses unchanged. Caller `services/estimator.py::_get_or_fetch_imv_cached` catches `IMVAddressNotFoundError` + generic `Exception` → returns `None` (graceful). New `IMVAuthError`/`IMVTransientError` raises are subsumed by `except Exception` (intended). - **DB schema**: `avito_imv_evaluations.geo_hash` column unchanged — still stores the value used as `geoHash` in step C body (now a JWT instead of a hash, but the column is text). No migration needed. - **Cache safety**: 24h cache hit returns reconstructed `IMVEvaluation` without re-calling step C → JWT expiry inside cached `geo_hash` is irrelevant on hit. - **Tests**: `tests/test_avito_imv_parse.py` is offline-only (cache_key, parse_title, parse_placement_item, dataclasses) — unaffected. - **PR #505 regression check**: grep'нул `cleaned|retry|Склад|Гараж|strip_prefix` в `app/services/{estimator.py,scrapers/avito_imv.py}` — no `IMV`-retry logic exists in current code. PR diff doesn't touch retry; description's PR #505 reference is context, not a code removal. ### Seven-dimension scan - **A. Security 🔒** — no secrets/credentials/SQL injection touched. Step B body fields (`address`, `categoryId`, lat/lon) are derived from step A response or fixed constants. No user input to JSON injection vector. ✅ - **B. Correctness 🎯** — state machine is linear: A fail → no B; B fail → no C. Each step has its own typed `try/except`. `_own_session` is always defined when `finally` executes (ImportError raises before `try`). ✅ - **C. Performance ⚡** — adds 1 HTTP RTT per cache miss (Step B). 24h cache fronts it. No new locks/N+1. ✅ - **D. Conventions 📋** — type hints, `_raise_for_status_categorized` reused for both A+B (consistent), `_COMMON_HEADERS` reused. `logger.info` for step boundaries (observable). ✅ - **E. Architecture 🏗** — DRY (categorized status helper covers both new calls), no dead code added. ✅ - **F. Tests 🧪** — no new unit tests for step B (acceptable: HTTP-shim, hard to unit-test without VCR; PR provides live probe command). 🟢 low - **G. Vault cross-check 📚** — implementation matches `Bug_Avito_Imv_Houses_Catalog_Broken_OPEN.md` plan verbatim: `point.lat/lon` extraction, POST `/js/v2/geo/position`, return `geoFieldsHash` as `geo_hash`, `_imv_evaluate()` unchanged, DB schema unchanged. ✅ ### 🟡 Minor (non-blocking) - `avito_imv.py:355` — `if not lat or not lon or not normalized:` trips on `lat=0.0`/`lon=0.0` (Gulf of Guinea edge case). For real RU addresses negligible, but `is None` would be precise: `if lat is None or lon is None or not normalized:`. - `avito_imv.py:367` — `categoryId: 24` hardcoded — current caller is apartments-only, fine. Worth a constant `CATEGORY_ID_APARTMENT = 24` for future expansion. - No `asyncio.sleep` between A→B→C. Browser does the same so likely OK, but if Avito starts 429-ing keep it on the radar. - `IMVCityMismatchError` defined but never raised (pre-existing dead exception, not introduced here). ### 🟢 Positive - Typed exception categorization (`IMVAddressNotFoundError` / `IMVAuthError` / `IMVTransientError`) applied symmetrically across step A and step B network/HTTP failures. - Error message for B includes `errorText=` from response — debuggable. - `logger.info` lines log `addressId/locationId/metroId` after B — observability for future contract drift detection. - Docstring updated with full 3-step flow and updated `Raises:` list. - Diff is tightly scoped — only `_geocode()` rewritten, no churn elsewhere. ### Live verification PR includes concrete production trace `a0a0b820-e8a8-4eee-aa73-0ab3b98ac233` (Заводская 44-а) → `recommended=7,520,000 range=(7,294,400, 7,896,000) count=800`. Step B confirmed returning `addressId=22292600 locationId=654070 metroId=2045`. Probe command in PR body for post-deploy revalidation. ### Recommended next steps 1. Merge as-is. 2. Post-deploy: run probe command from PR body to confirm `recommended` non-zero on a fresh address. 3. (Optional follow-up, not blocker) Monitor whether any address that previously succeeded with 1-step now fails at step B — Avito may return `errorText` we want to alarm on. ### Complexity / blast radius - **Risk**: low — fix targets a currently-broken code path; worst case = unchanged behavior (still returns None gracefully). - **Reversibility**: trivial revert (single-file diff). - **Merge window**: anytime.
lekss361 merged commit 0f7d6c4996 into main 2026-05-24 13:48:53 +00:00
lekss361 deleted branch feat/tradein-imv-fix-3step-api 2026-05-24 13:48:53 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lekss361/gendesign#524
No description provided.