feat(tradein): Stage 4a — API endpoints for Avito enrichment + IMV benchmark UI #460

Merged
lekss361 merged 1 commit from feat/tradein-api-avito-stage4a into main 2026-05-23 13:34:39 +00:00
Owner

Summary

Stage 4a of Avito Scraper v2 — 3 admin debug endpoints (manual triggers для Stages 2b/2c/2d) + 2 trade-in UI endpoints (house info + IMV benchmark для Stage 4b frontend).

LOC: admin.py +135, trade_in.py +155, schemas/trade_in.py +42, new test 93 lines.

Changes

admin.py — 3 new POST endpoints (debug)

Endpoint Body Purpose
POST /api/v1/admin/scrape/avito-house?house_url=... query Single house Catalog enrichment (Stage 2c)
POST /api/v1/admin/scrape/avito-detail?item_url=... query Single listing detail enrichment (Stage 2b)
POST /api/v1/admin/scrape/avito-imv?address=...&rooms=... query IMV debug (Stage 2d, no cache — always fresh fetch)

Все защищены через Depends(require_admin) (X-Admin-Token).

trade_in.py — 2 new GET endpoints (UI)

Endpoint Response Purpose
GET /api/v1/trade-in/estimate/{id}/houses list[HouseInfoForEstimate] Top-5 ближайших houses к target (PostGIS 500m radius) — Stage 2c data
GET /api/v1/trade-in/estimate/{id}/imv-benchmark IMVBenchmarkResponse Avito IMV для UI badge — lookup linked → fallback by address (24h TTL) — includes our_median_price + diff_pct

schemas/trade_in.py — 2 new Pydantic schemas

  • HouseInfoForEstimate (18 fields: identity, geo, year_built, total_floors, lifts, concierge, rating, etc.)
  • IMVBenchmarkResponse (available flag + IMV price triplet + market_count + comparison metadata)

Tests

  • pytest 5/5 passed (1.39s, offline FastAPI TestClient + mocked DB)
  • ruff clean (новые строки)
  • 5 pre-existing E501 в admin.py:57-60 и trade_in.py:228 — НЕ тронуты (out of scope)

Deviations from spec

  1. # noqa: BLE001 убраны — BLE001 не в ruff select проекта; unused noqa → RUF100
  2. raise HTTPException(...) from e (B904 chaining)
  3. sys.modules stub для weasyprint в тесте — на Windows без GTK/Pango import fails
  4. DATABASE_URL test: postgresql+psycopg://... (psycopg v3 dialect, не default psycopg2)

Test plan

  • ruff clean
  • pytest 5/5
  • Deploy → curl -X POST .../admin/scrape/avito-house?house_url=... → verify houses row + counters
  • curl .../trade-in/estimate/{id}/imv-benchmark после успешного estimate → verify available=true с populated fields
  • Frontend (Stage 4b PR паралельный) wires эти endpoints

Refs

  • Plan: AvitoScraper_v2_Implementation_Plan Stage 4a
  • Frontend contract: PR Stage 4b (паралельный, использует те же types)
## Summary **Stage 4a of Avito Scraper v2** — 3 admin debug endpoints (manual triggers для Stages 2b/2c/2d) + 2 trade-in UI endpoints (house info + IMV benchmark для Stage 4b frontend). LOC: admin.py `+135`, trade_in.py `+155`, schemas/trade_in.py `+42`, new test 93 lines. ## Changes ### `admin.py` — 3 new POST endpoints (debug) | Endpoint | Body | Purpose | |---|---|---| | `POST /api/v1/admin/scrape/avito-house?house_url=...` | query | Single house Catalog enrichment (Stage 2c) | | `POST /api/v1/admin/scrape/avito-detail?item_url=...` | query | Single listing detail enrichment (Stage 2b) | | `POST /api/v1/admin/scrape/avito-imv?address=...&rooms=...` | query | IMV debug (Stage 2d, **no cache** — always fresh fetch) | Все защищены через `Depends(require_admin)` (X-Admin-Token). ### `trade_in.py` — 2 new GET endpoints (UI) | Endpoint | Response | Purpose | |---|---|---| | `GET /api/v1/trade-in/estimate/{id}/houses` | `list[HouseInfoForEstimate]` | Top-5 ближайших houses к target (PostGIS 500m radius) — Stage 2c data | | `GET /api/v1/trade-in/estimate/{id}/imv-benchmark` | `IMVBenchmarkResponse` | Avito IMV для UI badge — lookup linked → fallback by address (24h TTL) — includes `our_median_price` + `diff_pct` | ### `schemas/trade_in.py` — 2 new Pydantic schemas - `HouseInfoForEstimate` (18 fields: identity, geo, year_built, total_floors, lifts, concierge, rating, etc.) - `IMVBenchmarkResponse` (available flag + IMV price triplet + market_count + comparison metadata) ## Tests - ✅ pytest 5/5 passed (1.39s, offline FastAPI TestClient + mocked DB) - ✅ ruff clean (новые строки) - 5 pre-existing E501 в admin.py:57-60 и trade_in.py:228 — НЕ тронуты (out of scope) ## Deviations from spec 1. `# noqa: BLE001` убраны — BLE001 не в ruff `select` проекта; unused noqa → RUF100 2. `raise HTTPException(...) from e` (B904 chaining) 3. `sys.modules` stub для `weasyprint` в тесте — на Windows без GTK/Pango import fails 4. `DATABASE_URL` test: `postgresql+psycopg://...` (psycopg v3 dialect, не default psycopg2) ## Test plan - [x] ruff clean - [x] pytest 5/5 - [ ] Deploy → `curl -X POST .../admin/scrape/avito-house?house_url=...` → verify houses row + counters - [ ] `curl .../trade-in/estimate/{id}/imv-benchmark` после успешного estimate → verify available=true с populated fields - [ ] Frontend (Stage 4b PR паралельный) wires эти endpoints ## Refs - Plan: AvitoScraper_v2_Implementation_Plan Stage 4a - Frontend contract: PR Stage 4b (паралельный, использует те же types)
Author
Owner

Deep Code Review — verdict

Status: BLOCK (rebase required) — code itself is APPROVE-quality, but mergeable=false and update_pr_branch tool unavailable.

Files reviewed: 4 (P1: admin.py, trade_in.py; P2: schemas/trade_in.py, test_api_avito_stage4a.py)
Lines: +431 / -1
Reviews state: 0 prior reviews


BLOCKER — manual rebase required

git merge-tree forgejo/main forgejo/feat/tradein-api-avito-stage4a подтверждает реальный conflict в tradein-mvp/backend/app/api/v1/admin.py:

  1. Imports block (line 18-30): PR #460 добавляет avito_detail / avito_houses / avito_imv imports, PR #457 (merged) добавил cian_session as cian_session_svc — обе вставки в один блок between geocoder и avito lines.
  2. End-of-file: PR #460 добавляет 3 endpoint'а после geocode_missing, PR #457 добавил 2 cian-cookie endpoint'а ровно туда же.

Conflict trivially resolvable (оба добавляют новый, не перекрывающийся код), но требует ручной правки:

git checkout feat/tradein-api-avito-stage4a
git fetch forgejo
git rebase forgejo/main
# В admin.py: оставить ВСЕ imports (cian_session_svc + avito_detail/houses/imv)
#             оставить ВСЕ endpoints (cian/upload-cookies, cian/test-auth + scrape/avito-house, scrape/avito-detail, scrape/avito-imv)
git add tradein-mvp/backend/app/api/v1/admin.py
git rebase --continue
git push forgejo feat/tradein-api-avito-stage4a --force-with-lease

После rebase — re-request review (или dropping этот comment в новом review thread).


MEDIUM — SSRF risk (admin-gated, recommend follow-up hardening)

POST /admin/scrape/avito-detail?item_url=...fetch_detailavito_detail.py:252:

full_url = item_url if item_url.startswith("http") else urljoin(AVITO_BASE, item_url)

Если item_url начинается с http:// — endpoint выполнит HTTP GET на любой URL. Admin token может быть скомпрометирован, или settings.admin_token is None → endpoints open (per require_admin line 47-49: dev mode bypass).

POST /admin/scrape/avito-house?house_url=...avito_houses.py:620:

url = f"{AVITO_BASE}{house_url}"

Если house_url = "@attacker.com/x" → URL становится https://www.avito.ru@attacker.com/x → goes to attacker.com (userinfo trick).

Mitigation: добавить URL validation в endpoint (или в scraper):

from urllib.parse import urlparse
parsed = urlparse(item_url if item_url.startswith("http") else f"https://www.avito.ru{item_url}")
if parsed.netloc not in ("www.avito.ru", "avito.ru", ""):
    raise HTTPException(400, "URL must be on avito.ru domain")

Severity: medium — мitigated admin-token, но открыт в dev/staging без token. Не блокирует merge для pilot (внутренняя сеть), но создать follow-up issue.


MINOR — edge cases / quality

  1. schemas/trade_in.py:131IMVBenchmarkResponse.diff_pct computation в endpoint: (our - imv) / imv * 100. Защита if our_median and row.recommended_price корректна (0 falsy → пропускаем). Но negative diff (наша оценка ниже) — отображается негативной → frontend должен это handle. Документирован в comment "(our - imv) / imv * 100" — OK.

  2. trade_in.py:670 (linked IMV SELECT): ORDER BY fetched_at DESC LIMIT 1 — берёт latest. Если за день несколько IMV вычислений для one estimate (refetch) — берёт свежее. OK.

  3. trade_in.py:702 (fallback by address): WHERE address = :address — точное совпадение строки. Если address normalize по-разному при первом IMV и при estimate save — fallback miss. Risk низкий (single source), accept.

  4. trade_in.py:619 (/houses): 500m radius hard-coded. Не критично, но magic number — лучше RADIUS_M = 500 константа сверху. Nit.

  5. admin.py:323 (avito-imv): 9 required query params (address, rooms, area_m2, floor, floor_at_home, house_type, renovation_type, has_balcony, has_loggia — последние два с defaults). Документировать в docstring пример вызова. Worker pre-deploy test plan покрывает это.

  6. admin.py (3 endpoints): except Exception as e catches IMVAddressNotFoundError first, потом generic Exception — корректно (specific before generic). Chained via from e per B904 — .

  7. Test test_get_imv_benchmark_unavailable: assert r.status_code in (200, 404) — тест слабый (accept'ит и 200 и 404). Должен mock'ать estimate row + verify available=False. Nit.

  8. WeasyPrint stub в тесте (sys.modules.setdefault("weasyprint", ...)): isolated в test file, не leak'ит в production. acceptable workaround для Windows dev.


POSITIVE observations

  • CAST(:id AS uuid) — следует sql-rules CAST trap mitigation
  • ON CONFLICT (cache_key) DO UPDATE корректно в save_imv_evaluation (verified)
  • psycopg v3 dialect в test DATABASE_URL (postgresql+psycopg://)
  • from e chaining во всех raise HTTPException (B904)
  • logger.exception() для error context, logger.info() для success metrics
  • Imports / type hints / type alias Annotated[Session, Depends(get_db)] — consistent
  • Houses query использует существующий GIST index houses_geom_idx (009_houses.sql:58) — performant
  • evaluate_via_imv signature точно совпадает (verified)
  • trade_in_estimates.median_price column verified (001_trade_in_estimates.sql:20)
  • Fallback by address имеет 24h TTL via existing index imv_cache_key_idx
  • available=False graceful fallback вместо 4xx когда нет IMV — UX-friendly

Cross-PR dependency (#461 frontend)

  • Contract HouseInfoForEstimate (18 fields) + IMVBenchmarkResponse (10 fields) — stable, все Optional кроме available: bool. PR #461 безопасно import'ит types через npm run codegen.
  • Этот PR должен merge первым, иначе #461 frontend ссылается на несуществующие endpoints.

Cross-file impact analysis

  • avito_houses.fetch_house_catalog / save_house_catalog_enrichment — existing functions, не модифицируются
  • avito_detail.fetch_detail / save_detail_enrichment — existing, не модифицируются
  • avito_imv.evaluate_via_imv (kwargs match: address, rooms, area_m2, floor, floor_at_home, house_type, renovation_type, has_balcony, has_loggia)
  • IMVEvaluation.placement_history attribute existing
  • houses columns referenced в /houses endpoint (009 + 010 schemas)
  • trade_in_estimates.address / lat / lon / median_price existing

Risk assessment

  • Risk: Low (read-only endpoints + admin-gated triggers)
  • Reversibility: Easy (revert single commit, no DB migrations)
  • Merge window: anytime после rebase

  1. Worker: rebase onto current main (resolve admin.py conflict — оба добавляют новый код, take both)
  2. Worker: push --force-with-lease
  3. Reviewer (next pass): после re-push — verify mergeable=true → squash merge
  4. Follow-up issue (separate PR, не блокирующее): SSRF URL validation в avito-house / avito-detail admin endpoints
  5. После merge: разблокируется PR #461 (frontend Stage 4b)
  6. Vault entry: worker создать code/modules/tradein/Admin_API_Stage4a.md (Avito enrichment trigger surface)

Не merged — ожидаю rebase от worker.

## Deep Code Review — verdict **Status**: BLOCK (rebase required) — code itself is APPROVE-quality, but `mergeable=false` and update_pr_branch tool unavailable. **Files reviewed**: 4 (P1: admin.py, trade_in.py; P2: schemas/trade_in.py, test_api_avito_stage4a.py) **Lines**: +431 / -1 **Reviews state**: 0 prior reviews --- ### BLOCKER — manual rebase required `git merge-tree forgejo/main forgejo/feat/tradein-api-avito-stage4a` подтверждает **реальный conflict** в `tradein-mvp/backend/app/api/v1/admin.py`: 1. **Imports block** (line 18-30): PR #460 добавляет `avito_detail / avito_houses / avito_imv` imports, PR #457 (merged) добавил `cian_session as cian_session_svc` — обе вставки в один блок between `geocoder` и `avito` lines. 2. **End-of-file**: PR #460 добавляет 3 endpoint'а после `geocode_missing`, PR #457 добавил 2 cian-cookie endpoint'а ровно туда же. Conflict trivially resolvable (оба добавляют новый, не перекрывающийся код), но требует ручной правки: ```bash git checkout feat/tradein-api-avito-stage4a git fetch forgejo git rebase forgejo/main # В admin.py: оставить ВСЕ imports (cian_session_svc + avito_detail/houses/imv) # оставить ВСЕ endpoints (cian/upload-cookies, cian/test-auth + scrape/avito-house, scrape/avito-detail, scrape/avito-imv) git add tradein-mvp/backend/app/api/v1/admin.py git rebase --continue git push forgejo feat/tradein-api-avito-stage4a --force-with-lease ``` После rebase — re-request review (или dropping этот comment в новом review thread). --- ### MEDIUM — SSRF risk (admin-gated, recommend follow-up hardening) `POST /admin/scrape/avito-detail?item_url=...` → `fetch_detail` → `avito_detail.py:252`: ```python full_url = item_url if item_url.startswith("http") else urljoin(AVITO_BASE, item_url) ``` **Если `item_url` начинается с `http://` — endpoint выполнит HTTP GET на любой URL.** Admin token может быть скомпрометирован, или `settings.admin_token is None` → endpoints open (per `require_admin` line 47-49: dev mode bypass). `POST /admin/scrape/avito-house?house_url=...` → `avito_houses.py:620`: ```python url = f"{AVITO_BASE}{house_url}" ``` Если `house_url = "@attacker.com/x"` → URL становится `https://www.avito.ru@attacker.com/x` → goes to attacker.com (userinfo trick). **Mitigation**: добавить URL validation в endpoint (или в scraper): ```python from urllib.parse import urlparse parsed = urlparse(item_url if item_url.startswith("http") else f"https://www.avito.ru{item_url}") if parsed.netloc not in ("www.avito.ru", "avito.ru", ""): raise HTTPException(400, "URL must be on avito.ru domain") ``` **Severity**: medium — мitigated admin-token, но открыт в dev/staging без token. Не блокирует merge для pilot (внутренняя сеть), но создать follow-up issue. --- ### MINOR — edge cases / quality 1. **`schemas/trade_in.py:131` — `IMVBenchmarkResponse.diff_pct` computation в endpoint**: `(our - imv) / imv * 100`. Защита `if our_median and row.recommended_price` корректна (`0` falsy → пропускаем). Но negative diff (наша оценка ниже) — отображается негативной → frontend должен это handle. Документирован в comment "(our - imv) / imv * 100" — OK. 2. **`trade_in.py:670` (linked IMV SELECT)**: `ORDER BY fetched_at DESC LIMIT 1` — берёт latest. Если за день несколько IMV вычислений для one estimate (refetch) — берёт свежее. OK. 3. **`trade_in.py:702` (fallback by address)**: `WHERE address = :address` — точное совпадение строки. Если address normalize по-разному при первом IMV и при estimate save — fallback miss. Risk низкий (single source), accept. 4. **`trade_in.py:619` (`/houses`)**: 500m radius hard-coded. Не критично, но **magic number** — лучше `RADIUS_M = 500` константа сверху. Nit. 5. **`admin.py:323` (`avito-imv`)**: 9 required query params (`address, rooms, area_m2, floor, floor_at_home, house_type, renovation_type, has_balcony, has_loggia` — последние два с defaults). Документировать в docstring пример вызова. Worker pre-deploy test plan покрывает это. 6. **`admin.py` (3 endpoints)**: `except Exception as e` catches `IMVAddressNotFoundError` first, потом generic `Exception` — корректно (specific before generic). Chained via `from e` per B904 — ✅. 7. **Test `test_get_imv_benchmark_unavailable`**: `assert r.status_code in (200, 404)` — тест слабый (accept'ит и 200 и 404). Должен mock'ать estimate row + verify `available=False`. Nit. 8. **WeasyPrint stub в тесте** (`sys.modules.setdefault("weasyprint", ...)`): isolated в test file, не leak'ит в production. ✅ acceptable workaround для Windows dev. --- ### POSITIVE observations - ✅ `CAST(:id AS uuid)` — следует sql-rules CAST trap mitigation - ✅ `ON CONFLICT (cache_key) DO UPDATE` корректно в `save_imv_evaluation` (verified) - ✅ `psycopg v3` dialect в test DATABASE_URL (`postgresql+psycopg://`) - ✅ `from e` chaining во всех `raise HTTPException` (B904) - ✅ `logger.exception()` для error context, `logger.info()` для success metrics - ✅ Imports / type hints / type alias `Annotated[Session, Depends(get_db)]` — consistent - ✅ Houses query использует **существующий GIST index** `houses_geom_idx` (009_houses.sql:58) — performant - ✅ `evaluate_via_imv` signature точно совпадает (verified) - ✅ `trade_in_estimates.median_price` column verified (001_trade_in_estimates.sql:20) - ✅ Fallback by address имеет 24h TTL via existing index `imv_cache_key_idx` - ✅ `available=False` graceful fallback вместо 4xx когда нет IMV — UX-friendly --- ### Cross-PR dependency (#461 frontend) - Contract `HouseInfoForEstimate` (18 fields) + `IMVBenchmarkResponse` (10 fields) — stable, все Optional кроме `available: bool`. PR #461 безопасно import'ит types через `npm run codegen`. - **Этот PR должен merge первым**, иначе #461 frontend ссылается на несуществующие endpoints. ### Cross-file impact analysis - `avito_houses.fetch_house_catalog` / `save_house_catalog_enrichment` — existing functions, не модифицируются ✅ - `avito_detail.fetch_detail` / `save_detail_enrichment` — existing, не модифицируются ✅ - `avito_imv.evaluate_via_imv` (kwargs match: `address, rooms, area_m2, floor, floor_at_home, house_type, renovation_type, has_balcony, has_loggia`) ✅ - `IMVEvaluation.placement_history` attribute existing ✅ - `houses` columns referenced в `/houses` endpoint (009 + 010 schemas) ✅ - `trade_in_estimates.address / lat / lon / median_price` existing ✅ ### Risk assessment - **Risk**: Low (read-only endpoints + admin-gated triggers) - **Reversibility**: Easy (revert single commit, no DB migrations) - **Merge window**: anytime после rebase --- ### Recommended next steps 1. **Worker**: rebase onto current main (resolve admin.py conflict — оба добавляют новый код, take both) 2. **Worker**: push `--force-with-lease` 3. **Reviewer (next pass)**: после re-push — verify mergeable=true → squash merge 4. **Follow-up issue** (separate PR, не блокирующее): SSRF URL validation в avito-house / avito-detail admin endpoints 5. **После merge**: разблокируется PR #461 (frontend Stage 4b) 6. **Vault entry**: worker создать `code/modules/tradein/Admin_API_Stage4a.md` (Avito enrichment trigger surface) --- **Не merged** — ожидаю rebase от worker.
lekss361 force-pushed feat/tradein-api-avito-stage4a from 92e584a613 to a425f4eef0 2026-05-23 13:32:32 +00:00 Compare
lekss361 merged commit b5d206bfa8 into main 2026-05-23 13:34:39 +00:00
Author
Owner

Merged via deep-code-reviewer (squash) — verdict APPROVE.

Re-review summary (post-rebase)

Rebase quality: clean

  • Head SHA 92e584a6a425f4ee (1 squashed commit, parent = merge_base 5e90797a)
  • Base auto-bumped to 29f94963 (after PR #462 Yandex SERP merge)
  • mergeable: true, no conflicts

Conflict resolution verification (admin.py):

  • Imports merged cleanly — existing cian_session as cian_session_svc + AvitoScraper preserved, NEW avito_detail / avito_houses / avito_imv imports added without duplicates
  • New endpoints (/scrape/avito-house, /scrape/avito-detail, /scrape/avito-imv) appended AFTER test_cian_auth (line 313+) — no overlap with PR #457 cian endpoints
  • No deleted/missing functions

APPROVE-quality items confirmed:

  • Depends(require_admin) on all 3 admin endpoints (tradein-mvp own dep)
  • Contract for PR #461: HouseInfoForEstimate 18 fields all Optional; IMVBenchmarkResponse only available: bool required, rest Optional
  • psycopg v3 patterns: CAST(:id AS uuid) (no ::uuid), named params, no f-strings in SQL
  • raise ... from e (B904), geom IS NOT NULL guard before ST_DWithin

MEDIUM follow-up (non-blocking, pre-existing in scraper modules — NOT touched by this PR):

  • SSRF в avito_detail.py:252 (startswith("http")) и avito_houses.py:620 (f-string concat). Admin-gated, для pilot acceptable. Рекомендую отдельный issue с allowlist avito.ru.

Post-merge

  • deploy-tradein.yml auto-triggers (paths tradein-mvp/backend/**)
  • PR #461 frontend (IMVBenchmark + HouseInfoCard) теперь имеет live backend contract
Merged via deep-code-reviewer (squash) — verdict APPROVE. ## Re-review summary (post-rebase) **Rebase quality**: clean - Head SHA `92e584a6` → `a425f4ee` (1 squashed commit, parent = merge_base `5e90797a`) - Base auto-bumped to `29f94963` (after PR #462 Yandex SERP merge) - `mergeable: true`, no conflicts **Conflict resolution verification (admin.py)**: - Imports merged cleanly — existing `cian_session as cian_session_svc` + `AvitoScraper` preserved, NEW `avito_detail` / `avito_houses` / `avito_imv` imports added without duplicates - New endpoints (`/scrape/avito-house`, `/scrape/avito-detail`, `/scrape/avito-imv`) appended AFTER `test_cian_auth` (line 313+) — no overlap with PR #457 cian endpoints - No deleted/missing functions **APPROVE-quality items confirmed**: - `Depends(require_admin)` on all 3 admin endpoints (tradein-mvp own dep) - Contract for PR #461: `HouseInfoForEstimate` 18 fields all Optional; `IMVBenchmarkResponse` only `available: bool` required, rest Optional - psycopg v3 patterns: `CAST(:id AS uuid)` (no `::uuid`), named params, no f-strings in SQL - `raise ... from e` (B904), `geom IS NOT NULL` guard before `ST_DWithin` **MEDIUM follow-up** (non-blocking, pre-existing in scraper modules — NOT touched by this PR): - SSRF в `avito_detail.py:252` (`startswith("http")`) и `avito_houses.py:620` (f-string concat). Admin-gated, для pilot acceptable. Рекомендую отдельный issue с allowlist `avito.ru`. ## Post-merge - `deploy-tradein.yml` auto-triggers (paths `tradein-mvp/backend/**`) - PR #461 frontend (IMVBenchmark + HouseInfoCard) теперь имеет live backend contract
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#460
No description provided.