lekss361/gendesign

Fork 0

feat(tradein): integrate Avito IMV as 5th evaluation source (on-demand cached) #452

Merged

lekss361 merged 1 commit from feat/tradein-estimator-imv-integration into main

2026-05-23 13:08:22 +00:00

lekss361 commented

2026-05-23 12:50:14 +00:00

Owner

Summary

Stage 3 of Avito Scraper v2 — интеграция Avito IMV (Index of Market Value) как 5-й source в estimator.py. On-demand only (per estimator call), с 24h cache по sha256 hash address+params. IMV НЕ в cron (per план update — bulk IMV → rate-limit risk + stale cache).

LOC: estimator.py +190/-3 (net +187), новый test file 102 lines.

Changes

`tradein-mvp/backend/app/services/estimator.py`

NEW _get_or_fetch_imv_cached(db, **params) -> IMVEvaluation | None
- TTL 24h по cache_key (sha256 of address + IMV input params)
- Cache HIT: SELECT из avito_imv_evaluations WHERE cache_key AND fetched_at > NOW() - 24h → reconstruct IMVEvaluation
- Cache MISS: evaluate_via_imv (2 HTTP requests to Avito) → save_imv_evaluation → return
- Graceful: любой error (network, address not found, etc) → log warning + return None; estimator продолжает без IMV
NEW mapping dicts: _IMV_HOUSE_TYPE_MAP, _IMV_REPAIR_MAP — переводят наши enum-значения в Avito IMV format
MODIFY estimate_quality():
- После aggregation (median/range computed): вызвать _get_or_fetch_imv_cached если payload has required fields
- Append 'avito_imv' в sources_used (saved row + returned AggregatedEstimate)
- После INSERT estimate: link estimate_id в avito_imv_evaluations row для analytics joining
- Logging: imv=<recommended_price> в final estimate log line

`tradein-mvp/backend/tests/test_estimator_imv_integration.py`

4 tests: house_type_map / repair_map / cache_miss_fetch_flow / fetch_failure_graceful
Использует anyio.run() (pytest-asyncio не в venv tradein-mvp — anyio есть)

Smoke

✅ ruff clean (новые строки; 5 pre-existing ruff ошибок в estimator.py НЕ тронуты — out of scope)
✅ pytest 4/4 passed (0.52s, offline)
⏸ Network smoke deferred to deploy (Avito IMV требует валидный browser session)

Behavioral changes

Aspect	Before	After
5th evaluation source	none	`avito_imv` (если params valid + IMV доступен)
IMV cache	none	`avito_imv_evaluations` table, TTL 24h по sha256 cache_key
Estimator graceful degradation	без IMV	если IMV fail → log warning, continue без IMV
`sources_used`	`[avito, cian, rosreestr, yandex, n1]` (whatever есть)	+ `avito_imv` опционально
`avito_imv_evaluations.estimate_id`	NULL	linked to created `trade_in_estimates.id` для analytics

Out-of-scope

UI exposure avito_imv benchmark в frontend — Stage 4b (EstimateResult badge)
API endpoint для standalone IMV trigger (debug) — Stage 4a
from_imv_suggest flag для save_imv_suggestions в listings — отложено

Test plan

ruff (new code)
pytest (4 unit tests: maps + cache flows)
Deploy → POST /api/v1/trade-in/estimate для real ЕКБ address → verify sources_used содержит avito_imv И avito_imv_evaluations row created
Re-test тот же address в течение 24h → cache HIT (no Avito network call)
Stage 4a/4b unblocked после merge

Refs

Plan: AvitoScraper_v2_Implementation_Plan Stage 3 + IMV on-demand update
Depends on: Stage 1 migrations (cache_key column) + Stage 2d (avito_imv.py)

## Summary **Stage 3 of Avito Scraper v2** — интеграция Avito IMV (Index of Market Value) как 5-й source в `estimator.py`. **On-demand only** (per estimator call), с 24h cache по sha256 hash address+params. IMV НЕ в cron (per план update — bulk IMV → rate-limit risk + stale cache). LOC: estimator.py `+190/-3` (net +187), новый test file 102 lines. ## Changes ### `tradein-mvp/backend/app/services/estimator.py` - **NEW** `_get_or_fetch_imv_cached(db, **params) -> IMVEvaluation | None` - TTL 24h по `cache_key` (sha256 of address + IMV input params) - **Cache HIT**: SELECT из `avito_imv_evaluations WHERE cache_key AND fetched_at > NOW() - 24h` → reconstruct IMVEvaluation - **Cache MISS**: `evaluate_via_imv` (2 HTTP requests to Avito) → `save_imv_evaluation` → return - **Graceful**: любой error (network, address not found, etc) → log warning + return None; estimator продолжает без IMV - **NEW** mapping dicts: `_IMV_HOUSE_TYPE_MAP`, `_IMV_REPAIR_MAP` — переводят наши enum-значения в Avito IMV format - **MODIFY** `estimate_quality()`: - После aggregation (median/range computed): вызвать `_get_or_fetch_imv_cached` если payload has required fields - Append `'avito_imv'` в `sources_used` (saved row + returned `AggregatedEstimate`) - После INSERT estimate: link `estimate_id` в `avito_imv_evaluations` row для analytics joining - Logging: `imv=<recommended_price>` в final estimate log line ### `tradein-mvp/backend/tests/test_estimator_imv_integration.py` - 4 tests: house_type_map / repair_map / cache_miss_fetch_flow / fetch_failure_graceful - Использует `anyio.run()` (pytest-asyncio не в venv tradein-mvp — anyio есть) ## Smoke - ✅ ruff clean (новые строки; 5 pre-existing ruff ошибок в estimator.py НЕ тронуты — out of scope) - ✅ pytest 4/4 passed (0.52s, offline) - ⏸ Network smoke deferred to deploy (Avito IMV требует валидный browser session) ## Behavioral changes | Aspect | Before | After | |---|---|---| | 5th evaluation source | none | `avito_imv` (если params valid + IMV доступен) | | IMV cache | none | `avito_imv_evaluations` table, TTL 24h по sha256 cache_key | | Estimator graceful degradation | без IMV | если IMV fail → log warning, continue без IMV | | `sources_used` | `[avito, cian, rosreestr, yandex, n1]` (whatever есть) | + `avito_imv` опционально | | `avito_imv_evaluations.estimate_id` | NULL | linked to created `trade_in_estimates.id` для analytics | ## Out-of-scope - **UI exposure** `avito_imv` benchmark в frontend — Stage 4b (EstimateResult badge) - **API endpoint** для standalone IMV trigger (debug) — Stage 4a - `from_imv_suggest` flag для save_imv_suggestions в listings — отложено ## Test plan - [x] ruff (new code) - [x] pytest (4 unit tests: maps + cache flows) - [ ] Deploy → POST /api/v1/trade-in/estimate для real ЕКБ address → verify `sources_used` содержит `avito_imv` И `avito_imv_evaluations` row created - [ ] Re-test тот же address в течение 24h → cache HIT (no Avito network call) - [ ] Stage 4a/4b unblocked после merge ## Refs - Plan: AvitoScraper_v2_Implementation_Plan Stage 3 + IMV on-demand update - Depends on: Stage 1 migrations (cache_key column) + Stage 2d (`avito_imv.py`)

lekss361 added 1 commit 2026-05-23 12:50:15 +00:00

feat(tradein): integrate Avito IMV as 5th evaluation source (on-demand cached) 7af97c2c59

Stage 3 of AvitoScraper_v2.

- _get_or_fetch_imv_cached(db, **params) — TTL 24h по cache_key (sha256 of address+params)
  * Cache HIT: SELECT avito_imv_evaluations WHERE cache_key AND fetched_at > NOW() - 24h
  * Cache MISS: evaluate_via_imv + save_imv_evaluation (2 HTTP requests to Avito)
  * Graceful: на любой error → return None, estimator продолжает без IMV
- В estimate_quality после aggregation:
  * Map house_type/repair_state → Avito IMV формат (_IMV_HOUSE_TYPE_MAP, _IMV_REPAIR_MAP)
  * Skip IMV если payload без required fields (rooms/area/floor/total_floors/house_type/repair)
  * Append 'avito_imv' в sources_used (saved + returned)
- После INSERT estimate: link estimate_id в avito_imv_evaluations row (для analytics joining)
- Logging: imv=<price> в final estimate log line
- pytest offline (4 tests): house_type_map / repair_map / cache_miss_fetch / fetch_failure_graceful
  * anyio.run() вместо pytest-asyncio (не установлен в tradein-mvp venv)
  * DATABASE_URL dummy через os.environ.setdefault (Settings fail-fast без него)

IMV — ON-DEMAND only (per estimator call), НЕ в cron-scrape.
Refs: AvitoScraper_v2_Implementation_Plan Stage 3.

lekss361 referenced this pull request

2026-05-23 12:55:49 +00:00

feat(tradein): scrape_pipeline.py — Avito full orchestrator (search → houses → detail) #451

lekss361 commented

2026-05-23 12:56:34 +00:00

Author

Owner

Deep Code Review — 🟠 HIGH (blocker is in dependency, not this PR)

Summary

Code in this PR is correct and well-tested for its scope.
cache_key save↔lookup consistency: PASS — both sides call compute_imv_cache_key with same 9 args in same order; produces matching sha256.
Graceful degradation: PASS — broad except Exception returns None; estimator continues without IMV.
sources_used consistency: PASS — both saved JSON column and returned AggregatedEstimate.sources_used get "avito_imv" appended.
estimate_id linking strategy: PASS — first-call wins (UPDATE clause WHERE estimate_id IS NULL OR estimate_id = :id), matches recommended COALESCE-equivalent semantics.

🔴 Blocker (NOT introduced by this PR — discovered by it)

tradein-mvp/backend/data/sql/018_avito_imv_evaluations.sql:54 — cache_key column has no UNIQUE constraint (only non-unique imv_cache_key_idx).

But save_imv_evaluation in avito_imv.py:481 uses ON CONFLICT (cache_key) DO UPDATE. PostgreSQL will raise at runtime:

ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification

Live impact: first POST /api/v1/trade-in/estimate with IMV-compatible payload → save fails → caught by except Exception in _get_or_fetch_imv_cached → logged as warning, returns None. Cache never populates. Every subsequent call re-fetches from Avito (rate-limit risk + 2-5s latency per request).

Why not caught earlier: PR #444 (Stage 2d) tests likely mocked save_imv_evaluation. This PR's tests also mock save. Bug only manifests on first live POST.

Fix required (separate PR, merge BEFORE #452)

-- tradein-mvp/backend/data/sql/030_avito_imv_cache_key_unique.sql
BEGIN;
-- Удалить дубликаты по cache_key (если уже есть в БД)
DELETE FROM avito_imv_evaluations a USING avito_imv_evaluations b
  WHERE a.id < b.id AND a.cache_key = b.cache_key;
CREATE UNIQUE INDEX IF NOT EXISTS imv_cache_key_uniq_idx
  ON avito_imv_evaluations (cache_key);
COMMIT;

(No CONCURRENTLY needed if table empty / fresh.)

🟡 Medium / nits (non-blocking)

Cache HIT reconstruction incomplete (estimator.py:128-150) — placement_history=[], suggestions=[], search_similar_url=None set by dataclass defaults. Estimator current scope only reads recommended_price, but Stage 4a/4b consumers will see empty lists on cache hit. Document or persist+restore.
has_loggia=False hardcoded (estimator.py:328) — adding has_loggia to payload later → mass cache invalidation. Note in decisions/ or module doc.
house_type == "other" silently skips IMV without log. Add logger.info("imv: skipped — house_type=%s not in IMV map") for observability.
Test gap: no cache HIT path test (would cover reconstruction logic). Low priority — graceful path + miss path covered.
os.environ.setdefault("DATABASE_URL", ...) in test file — should be in shared conftest.py. Follow-up cleanup.

Recommended next steps

Land DDL fix first (new PR with 030_avito_imv_cache_key_unique.sql).
Re-trigger CI on #452 (no rebase needed — independent file).
Merge #452.
Live smoke: POST /api/v1/trade-in/estimate ЕКБ address → verify in logs imv: fresh recommended=... on first call, then imv: cache HIT key=... on second call within 24h. If permanent cache MISS → DDL fix not applied.

Vault updates needed

fixes/Avito_IMV_Cache_Key_Missing_Unique.md — document the DDL bug + fix
code/modules/tradein/Estimator_IMV_Integration.md — describe 24h cache flow, graceful degradation, first-call-wins estimate_id link, has_loggia=False limitation

Verdict

Not merging this PR yet — blocker is in DDL dependency. Once 030_avito_imv_cache_key_unique.sql migration lands, this PR is ready for merge (squash).

## Deep Code Review — 🟠 HIGH (blocker is in dependency, not this PR) ### Summary - Code in this PR is correct and well-tested for its scope. - **`cache_key` save↔lookup consistency: PASS** — both sides call `compute_imv_cache_key` with same 9 args in same order; produces matching sha256. - **Graceful degradation: PASS** — broad `except Exception` returns `None`; estimator continues without IMV. - **`sources_used` consistency: PASS** — both saved JSON column and returned `AggregatedEstimate.sources_used` get `"avito_imv"` appended. - **`estimate_id` linking strategy: PASS** — first-call wins (UPDATE clause `WHERE estimate_id IS NULL OR estimate_id = :id`), matches recommended COALESCE-equivalent semantics. ### 🔴 Blocker (NOT introduced by this PR — discovered by it) **`tradein-mvp/backend/data/sql/018_avito_imv_evaluations.sql:54`** — `cache_key` column has **no `UNIQUE` constraint** (only non-unique `imv_cache_key_idx`). But `save_imv_evaluation` in `avito_imv.py:481` uses `ON CONFLICT (cache_key) DO UPDATE`. PostgreSQL will raise at runtime: ``` ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification ``` **Live impact**: first `POST /api/v1/trade-in/estimate` with IMV-compatible payload → save fails → caught by `except Exception` in `_get_or_fetch_imv_cached` → logged as warning, returns `None`. Cache never populates. Every subsequent call re-fetches from Avito (rate-limit risk + 2-5s latency per request). **Why not caught earlier**: PR #444 (Stage 2d) tests likely mocked `save_imv_evaluation`. This PR's tests also mock save. Bug only manifests on first live POST. ### Fix required (separate PR, merge BEFORE #452) ```sql -- tradein-mvp/backend/data/sql/030_avito_imv_cache_key_unique.sql BEGIN; -- Удалить дубликаты по cache_key (если уже есть в БД) DELETE FROM avito_imv_evaluations a USING avito_imv_evaluations b WHERE a.id < b.id AND a.cache_key = b.cache_key; CREATE UNIQUE INDEX IF NOT EXISTS imv_cache_key_uniq_idx ON avito_imv_evaluations (cache_key); COMMIT; ``` (No `CONCURRENTLY` needed if table empty / fresh.) ### 🟡 Medium / nits (non-blocking) 1. **Cache HIT reconstruction incomplete** (`estimator.py:128-150`) — `placement_history=[]`, `suggestions=[]`, `search_similar_url=None` set by dataclass defaults. Estimator current scope only reads `recommended_price`, but Stage 4a/4b consumers will see empty lists on cache hit. Document or persist+restore. 2. **`has_loggia=False` hardcoded** (`estimator.py:328`) — adding `has_loggia` to payload later → mass cache invalidation. Note in `decisions/` or module doc. 3. **`house_type == "other"`** silently skips IMV without log. Add `logger.info("imv: skipped — house_type=%s not in IMV map")` for observability. 4. **Test gap**: no cache HIT path test (would cover reconstruction logic). Low priority — graceful path + miss path covered. 5. **`os.environ.setdefault("DATABASE_URL", ...)`** in test file — should be in shared `conftest.py`. Follow-up cleanup. ### Recommended next steps 1. **Land DDL fix first** (new PR with `030_avito_imv_cache_key_unique.sql`). 2. Re-trigger CI on #452 (no rebase needed — independent file). 3. Merge #452. 4. Live smoke: `POST /api/v1/trade-in/estimate` ЕКБ address → verify in logs `imv: fresh recommended=...` on first call, then `imv: cache HIT key=...` on second call within 24h. If permanent `cache MISS` → DDL fix not applied. ### Vault updates needed - `fixes/Avito_IMV_Cache_Key_Missing_Unique.md` — document the DDL bug + fix - `code/modules/tradein/Estimator_IMV_Integration.md` — describe 24h cache flow, graceful degradation, first-call-wins estimate_id link, `has_loggia=False` limitation ### Verdict **Not merging this PR yet** — blocker is in DDL dependency. Once `030_avito_imv_cache_key_unique.sql` migration lands, this PR is ready for merge (squash).

lekss361 referenced this pull request from a commit

2026-05-23 12:59:04 +00:00

fix(tradein): 030 — UNIQUE constraint on avito_imv_evaluations.cache_key

lekss361 referenced this pull request

2026-05-23 12:59:28 +00:00

fix(tradein): 030 — UNIQUE constraint on avito_imv_evaluations.cache_key (unblocks PR #452) #454

lekss361 referenced this pull request

2026-05-23 13:01:20 +00:00

feat(tradein): SQL migrations 030-032 — Yandex schema (listings/houses/history) #453

lekss361 referenced this pull request from a commit

2026-05-23 13:04:23 +00:00

fix(tradein): 030 — UNIQUE constraint on avito_imv_evaluations.cache_key (unblocks PR #452) (#454)

lekss361 referenced this pull request

2026-05-23 13:04:42 +00:00

fix(tradein): 030 — UNIQUE constraint on avito_imv_evaluations.cache_key (unblocks PR #452) #454

lekss361 merged commit 5051ebde2e into main

2026-05-23 13:08:22 +00:00

lekss361 referenced this pull request from a commit

2026-05-23 13:08:24 +00:00

feat(tradein): integrate Avito IMV as 5th evaluation source (on-demand cached) (#452)

lekss361 commented

2026-05-23 13:08:48 +00:00

Author

Owner

Merged via deep-code-reviewer — verdict ✅ APPROVE (re-review after DDL blocker resolved).

Blocker resolution confirmed:

PR #454 merged (commit 0db71033) at 13:04:22Z — adds 030_avito_imv_cache_key_unique.sql creating imv_cache_key_uniq_idx UNIQUE INDEX on avito_imv_evaluations.cache_key
ON CONFLICT (cache_key) DO UPDATE в save_imv_evaluation теперь имеет требуемый UNIQUE constraint
Cache flow (HIT path) теперь работоспособен — больше не perpetual MISS из-за silently-swallowed exception

Pre-merge verification:

Head SHA unchanged (7af97c2c594db169ab293966654d62709061ded3) с момента previous review — no unauthorized changes
All 4 verified items still hold: cache_key consistency (9-arg sequence), graceful degradation, sources_used consistency, estimate_id linking
mergeable: true, state: open, not draft
Base auto-bumped to 1b3edff6 (post-PR #455 merge, lineage includes #454)

Squash merge: 5051ebde2e8037cc8abcbf001c01f6f0d95b2148

Post-merge verify (для пользователя, ~3 min after deploy):

deploy-tradein.yml auto-trigger на tradein-mvp/backend/** paths
POST /api/v1/trade-in/estimate для ЕКБ address → response sources_used содержит avito_imv
SELECT count(*) FROM avito_imv_evaluations WHERE cache_key IS NOT NULL → > 0
SELECT estimate_id FROM avito_imv_evaluations ORDER BY fetched_at DESC LIMIT 1 → не NULL
Re-POST тот же address в течение 24h → log imv: cache HIT key=... (не fresh fetch)

Unblocked:

Stage 4a: API endpoint для standalone IMV trigger (debug)
Stage 4b: UI exposure avito_imv benchmark в EstimateResult badge

Merged via deep-code-reviewer — verdict ✅ APPROVE (re-review after DDL blocker resolved). **Blocker resolution confirmed:** - PR #454 merged (commit `0db71033`) at 13:04:22Z — adds `030_avito_imv_cache_key_unique.sql` creating `imv_cache_key_uniq_idx` UNIQUE INDEX on `avito_imv_evaluations.cache_key` - `ON CONFLICT (cache_key) DO UPDATE` в `save_imv_evaluation` теперь имеет требуемый UNIQUE constraint - Cache flow (HIT path) теперь работоспособен — больше не perpetual MISS из-за silently-swallowed exception **Pre-merge verification:** - Head SHA unchanged (`7af97c2c594db169ab293966654d62709061ded3`) с момента previous review — no unauthorized changes - All 4 verified items still hold: cache_key consistency (9-arg sequence), graceful degradation, sources_used consistency, estimate_id linking - mergeable: true, state: open, not draft - Base auto-bumped to `1b3edff6` (post-PR #455 merge, lineage includes #454) **Squash merge**: `5051ebde2e8037cc8abcbf001c01f6f0d95b2148` **Post-merge verify** (для пользователя, ~3 min after deploy): 1. `deploy-tradein.yml` auto-trigger на `tradein-mvp/backend/**` paths 2. `POST /api/v1/trade-in/estimate` для ЕКБ address → response `sources_used` содержит `avito_imv` 3. `SELECT count(*) FROM avito_imv_evaluations WHERE cache_key IS NOT NULL` → > 0 4. `SELECT estimate_id FROM avito_imv_evaluations ORDER BY fetched_at DESC LIMIT 1` → не NULL 5. Re-POST тот же address в течение 24h → log `imv: cache HIT key=...` (не fresh fetch) **Unblocked**: - Stage 4a: API endpoint для standalone IMV trigger (debug) - Stage 4b: UI exposure `avito_imv` benchmark в EstimateResult badge