feat(tradein): Stage 4a — API endpoints for Avito enrichment + IMV benchmark UI #460
No reviewers
Labels
No labels
admin
analytics
auth
automation
bug
business
chore
ci
compliance
data
data-moat
docs
duplicate
dx
enhancement
Fable 5 ревью
feedback/max
generative
GG-форсайт
needs-discussion
needs-human
observability
pause-bots
performance
priority/p0
priority/p1
priority/p2
priority/p3
scope/backend
scope/db
scope/devops
scope/frontend
scope/qa
scrapers
security
site-finder
stage/1
stage/2
status/blocked
status/done
status/needs-analysis
status/needs-fix
status/qa
status/ready
status/review
status/wip
tech-debt
tradein
ux
week ревью 1
wontfix
вторичка
ИРД
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: lekss361/gendesign#460
Loading…
Add table
Reference in a new issue
No description provided.
Delete branch "feat/tradein-api-avito-stage4a"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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)POST /api/v1/admin/scrape/avito-house?house_url=...POST /api/v1/admin/scrape/avito-detail?item_url=...POST /api/v1/admin/scrape/avito-imv?address=...&rooms=...Все защищены через
Depends(require_admin)(X-Admin-Token).trade_in.py— 2 new GET endpoints (UI)GET /api/v1/trade-in/estimate/{id}/houseslist[HouseInfoForEstimate]GET /api/v1/trade-in/estimate/{id}/imv-benchmarkIMVBenchmarkResponseour_median_price+diff_pctschemas/trade_in.py— 2 new Pydantic schemasHouseInfoForEstimate(18 fields: identity, geo, year_built, total_floors, lifts, concierge, rating, etc.)IMVBenchmarkResponse(available flag + IMV price triplet + market_count + comparison metadata)Tests
Deviations from spec
# noqa: BLE001убраны — BLE001 не в ruffselectпроекта; unused noqa → RUF100raise HTTPException(...) from e(B904 chaining)sys.modulesstub дляweasyprintв тесте — на Windows без GTK/Pango import failsDATABASE_URLtest:postgresql+psycopg://...(psycopg v3 dialect, не default psycopg2)Test plan
curl -X POST .../admin/scrape/avito-house?house_url=...→ verify houses row + counterscurl .../trade-in/estimate/{id}/imv-benchmarkпосле успешного estimate → verify available=true с populated fieldsRefs
Deep Code Review — verdict
Status: BLOCK (rebase required) — code itself is APPROVE-quality, but
mergeable=falseand 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:avito_detail / avito_houses / avito_imvimports, PR #457 (merged) добавилcian_session as cian_session_svc— обе вставки в один блок betweengeocoderиavitolines.geocode_missing, PR #457 добавил 2 cian-cookie endpoint'а ровно туда же.Conflict trivially resolvable (оба добавляют новый, не перекрывающийся код), но требует ручной правки:
После 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:Если
item_urlначинается сhttp://— endpoint выполнит HTTP GET на любой URL. Admin token может быть скомпрометирован, илиsettings.admin_token is None→ endpoints open (perrequire_adminline 47-49: dev mode bypass).POST /admin/scrape/avito-house?house_url=...→avito_houses.py:620:Если
house_url = "@attacker.com/x"→ URL становитсяhttps://www.avito.ru@attacker.com/x→ goes to attacker.com (userinfo trick).Mitigation: добавить URL validation в endpoint (или в scraper):
Severity: medium — мitigated admin-token, но открыт в dev/staging без token. Не блокирует merge для pilot (внутренняя сеть), но создать follow-up issue.
MINOR — edge cases / quality
schemas/trade_in.py:131—IMVBenchmarkResponse.diff_pctcomputation в endpoint:(our - imv) / imv * 100. Защитаif our_median and row.recommended_priceкорректна (0falsy → пропускаем). Но negative diff (наша оценка ниже) — отображается негативной → frontend должен это handle. Документирован в comment "(our - imv) / imv * 100" — OK.trade_in.py:670(linked IMV SELECT):ORDER BY fetched_at DESC LIMIT 1— берёт latest. Если за день несколько IMV вычислений для one estimate (refetch) — берёт свежее. OK.trade_in.py:702(fallback by address):WHERE address = :address— точное совпадение строки. Если address normalize по-разному при первом IMV и при estimate save — fallback miss. Risk низкий (single source), accept.trade_in.py:619(/houses): 500m radius hard-coded. Не критично, но magic number — лучшеRADIUS_M = 500константа сверху. Nit.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 покрывает это.admin.py(3 endpoints):except Exception as ecatchesIMVAddressNotFoundErrorfirst, потом genericException— корректно (specific before generic). Chained viafrom eper B904 — ✅.Test
test_get_imv_benchmark_unavailable:assert r.status_code in (200, 404)— тест слабый (accept'ит и 200 и 404). Должен mock'ать estimate row + verifyavailable=False. Nit.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 mitigationON CONFLICT (cache_key) DO UPDATEкорректно вsave_imv_evaluation(verified)psycopg v3dialect в test DATABASE_URL (postgresql+psycopg://)from echaining во всехraise HTTPException(B904)logger.exception()для error context,logger.info()для success metricsAnnotated[Session, Depends(get_db)]— consistenthouses_geom_idx(009_houses.sql:58) — performantevaluate_via_imvsignature точно совпадает (verified)trade_in_estimates.median_pricecolumn verified (001_trade_in_estimates.sql:20)imv_cache_key_idxavailable=Falsegraceful fallback вместо 4xx когда нет IMV — UX-friendlyCross-PR dependency (#461 frontend)
HouseInfoForEstimate(18 fields) +IMVBenchmarkResponse(10 fields) — stable, все Optional кромеavailable: bool. PR #461 безопасно import'ит types черезnpm run codegen.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_historyattribute existing ✅housescolumns referenced в/housesendpoint (009 + 010 schemas) ✅trade_in_estimates.address / lat / lon / median_priceexisting ✅Risk assessment
Recommended next steps
--force-with-leasecode/modules/tradein/Admin_API_Stage4a.md(Avito enrichment trigger surface)Не merged — ожидаю rebase от worker.
92e584a613toa425f4eef0Merged via deep-code-reviewer (squash) — verdict APPROVE.
Re-review summary (post-rebase)
Rebase quality: clean
92e584a6→a425f4ee(1 squashed commit, parent = merge_base5e90797a)29f94963(after PR #462 Yandex SERP merge)mergeable: true, no conflictsConflict resolution verification (admin.py):
cian_session as cian_session_svc+AvitoScraperpreserved, NEWavito_detail/avito_houses/avito_imvimports added without duplicates/scrape/avito-house,/scrape/avito-detail,/scrape/avito-imv) appended AFTERtest_cian_auth(line 313+) — no overlap with PR #457 cian endpointsAPPROVE-quality items confirmed:
Depends(require_admin)on all 3 admin endpoints (tradein-mvp own dep)HouseInfoForEstimate18 fields all Optional;IMVBenchmarkResponseonlyavailable: boolrequired, rest OptionalCAST(:id AS uuid)(no::uuid), named params, no f-strings in SQLraise ... from e(B904),geom IS NOT NULLguard beforeST_DWithinMEDIUM follow-up (non-blocking, pre-existing in scraper modules — NOT touched by this PR):
avito_detail.py:252(startswith("http")) иavito_houses.py:620(f-string concat). Admin-gated, для pilot acceptable. Рекомендую отдельный issue с allowlistavito.ru.Post-merge
deploy-tradein.ymlauto-triggers (pathstradein-mvp/backend/**)