feat(tradein): Phase C — Avito IMV per-house backfill (SQL + script) #534

Merged
lekss361 merged 1 commit from feat/tradein-phase-c-house-imv-backfill into main 2026-05-24 14:50:32 +00:00
Owner

Context

Phase C of [[Decision_TradeIn_DataQuality_8PR_Roadmap]] — наполнить house_placement_history + 2 новые таблицы данными от Avito IMV для каждого из 6,507 houses. Survives restart, runs локально пишет в продовую БД.

Depends on:

  • PR #524 (IMV 3-step API fix) — evaluate_via_imv() working
  • PR #527 (houses bootstrap) — есть кого scrape'ить

SQL migration 064

  • houses.imv_status text ('pending' | 'ok' | 'not_found' | 'transient_error' | 'error' | 'no_params' | 'no_address')
  • houses.last_imv_attempt_at timestamptz
  • houses.imv_error_reason text
  • Partial index houses_imv_status_idx для быстрой выборки pending/transient
  • New table house_imv_evaluations (1:1 c houses, UPSERT по house_id) — recommended/lower/higher_price + market_count + raw_response + lot-params used
  • New table house_suggestions (8 Avito-аналогов per house, UNIQUE (house_id, ext_item_id)) — title/address/price/exposure/publish_date/item_url/image/metro

Script scripts/backfill-house-imv.py

Resumable iterator:

  1. SELECT houses WHERE imv_status='pending' AND lat/lon/address NOT NULL ORDER BY last_imv_attempt_at NULLS FIRST, id LIMIT batch
  2. Pick median lot-params (rooms/area/floor/total_floors via percentile_cont, house_type via mode()) из linked listings
  3. Call evaluate_via_imv() (PR #524 3-step chain)
  4. Save: house_imv_evaluations + house_placement_history (source='avito_imv') + house_suggestions
  5. Mark imv_status='ok' / appropriate error status

Args: --batch N --delay SEC --limit-total N --only-status pending|transient_error

Recovery:

  • IMVAddressNotFoundError → status='not_found' (won't retry)
  • IMVAuthError → 60-sec backoff + status='transient_error' (can retry с --only-status transient_error)
  • IMVTransientError → status='transient_error' (retry)
  • Other → status='error' + repr in error_reason

Workflow для запуска (после merge+deploy)

# SSH tunnel: LocalForward 35432:tradein-postgres:5432 в ~/.ssh/config
DATABASE_URL=postgresql+psycopg://tradein:<pwd>@localhost:35432/tradein \
    python tradein-mvp/scripts/backfill-house-imv.py --batch 50 --delay 5

# или внутри контейнера на VPS (use same backend session/cookies):
docker exec tradein-backend python /app/scripts/backfill-house-imv.py --batch 100 --delay 3

Агрессивный темп ~3-5 sec → 6,507 houses ≈ 5-9 часов. Прерываемо в любой момент: следующий запуск продолжит с imv_status='pending'.

After backfill complete

Follow-up PR (отдельно): surface house_imv_evaluations + house_suggestions на UI (расширить HouseInfoCard, добавить новую SuggestionsCard).

## Context Phase C of `[[Decision_TradeIn_DataQuality_8PR_Roadmap]]` — наполнить `house_placement_history` + 2 новые таблицы данными от Avito IMV для каждого из 6,507 houses. Survives restart, runs локально пишет в продовую БД. Depends on: - PR #524 (IMV 3-step API fix) — `evaluate_via_imv()` working - PR #527 (houses bootstrap) — есть кого scrape'ить ## SQL migration 064 - `houses.imv_status text` ('pending' | 'ok' | 'not_found' | 'transient_error' | 'error' | 'no_params' | 'no_address') - `houses.last_imv_attempt_at timestamptz` - `houses.imv_error_reason text` - Partial index `houses_imv_status_idx` для быстрой выборки pending/transient - New table `house_imv_evaluations` (1:1 c houses, UPSERT по `house_id`) — recommended/lower/higher_price + market_count + raw_response + lot-params used - New table `house_suggestions` (8 Avito-аналогов per house, UNIQUE `(house_id, ext_item_id)`) — title/address/price/exposure/publish_date/item_url/image/metro ## Script `scripts/backfill-house-imv.py` Resumable iterator: 1. SELECT houses WHERE imv_status='pending' AND lat/lon/address NOT NULL ORDER BY last_imv_attempt_at NULLS FIRST, id LIMIT batch 2. Pick **median** lot-params (rooms/area/floor/total_floors via `percentile_cont`, house_type via `mode()`) из linked listings 3. Call `evaluate_via_imv()` (PR #524 3-step chain) 4. Save: house_imv_evaluations + house_placement_history (source='avito_imv') + house_suggestions 5. Mark `imv_status='ok'` / appropriate error status **Args:** `--batch N --delay SEC --limit-total N --only-status pending|transient_error` **Recovery:** - `IMVAddressNotFoundError` → status='not_found' (won't retry) - `IMVAuthError` → 60-sec backoff + status='transient_error' (can retry с --only-status transient_error) - `IMVTransientError` → status='transient_error' (retry) - Other → status='error' + repr in error_reason ## Workflow для запуска (после merge+deploy) ```bash # SSH tunnel: LocalForward 35432:tradein-postgres:5432 в ~/.ssh/config DATABASE_URL=postgresql+psycopg://tradein:<pwd>@localhost:35432/tradein \ python tradein-mvp/scripts/backfill-house-imv.py --batch 50 --delay 5 # или внутри контейнера на VPS (use same backend session/cookies): docker exec tradein-backend python /app/scripts/backfill-house-imv.py --batch 100 --delay 3 ``` Агрессивный темп ~3-5 sec → 6,507 houses ≈ 5-9 часов. Прерываемо в любой момент: следующий запуск продолжит с `imv_status='pending'`. ## After backfill complete Follow-up PR (отдельно): surface `house_imv_evaluations` + `house_suggestions` на UI (расширить `HouseInfoCard`, добавить новую `SuggestionsCard`).
lekss361 added 1 commit 2026-05-24 14:41:32 +00:00
SQL migration 064: adds houses.imv_status + last_imv_attempt_at + imv_error_reason
columns, creates house_imv_evaluations (1:1 with houses, latest IMV result)
and house_suggestions (8 Avito similar lots per house, UNIQUE on (house_id, ext_item_id)).

Script scripts/backfill-house-imv.py: resumable iterator that picks houses with
imv_status='pending' AND lat/lon NOT NULL, runs evaluate_via_imv() per house with
median lot-params from linked listings, saves house_imv_evaluations +
house_placement_history (from result.placement_history items) + house_suggestions
(from result.suggestions items). Configurable --batch, --delay, --limit-total.

Survives restart via imv_status state machine ('pending' -> 'ok'/'not_found'/
'transient_error'/'error'/'no_params'/'no_address'). Long backoff on
IMVAuthError (anti-captcha).
Author
Owner

Deep Code Review — verdict

Summary

  • Status: APPROVE
  • Files: 2 (P0 SQL migration: 1, P1 script: 1)
  • Lines: +470 / -0 · Phase C of [[Decision_TradeIn_DataQuality_8PR_Roadmap]]
  • Head SHA: e868b01278e3b53ae3243f2618ebce3b80a81c30 (matches)

Verified strengths

  • Migration 064 fully idempotent: IF NOT EXISTS everywhere, single BEGIN/COMMIT, lex-sort 063→064 correct for deploy-tradein.yml SQL applier loop.
  • All ON CONFLICT targets backed by existing UNIQUE: house_imv_eval_house_uniq_idx(house_id), house_suggestions_uniq_idx(house_id, ext_item_id), and pre-existing house_placement_history (source, ext_item_id) from 017.
  • Resumable backfill: partial index houses_imv_status_idx WHERE imv_status IN ('pending','transient_error') makes per-batch SELECT ... ORDER BY last_imv_attempt_at NULLS FIRST cheap.
  • Per-house transaction boundary correct: db.commit() after each process_one; pre-commit failure in save_imv_result leaves house at 'pending' → re-run picks up cleanly.
  • evaluate_via_imv(address=, rooms=, area_m2=, floor=, floor_at_home=, house_type=, renovation_type=, has_balcony=, has_loggia=) call matches the 3-step adapter signature from PR #524 exactly.
  • Error mapping aligns with adapter exception hierarchy: IMVAddressNotFoundError → not_found (terminal), IMVAuthError → transient_error + 60s sleep, IMVTransientError → transient_error, generic Exception → error + repr.
  • psycopg v3 driver (postgresql+psycopg://) + CAST(:raw AS jsonb) everywhere, no :x::type traps.
  • pick_lot_params uses correct FK listings.house_id_fk + percentile_cont(0.5) WITHIN GROUP (ORDER BY ...) median + mode() for house_type + reasonable fallbacks (floor=1, total_floors=9, panel/cosmetic).
  • raw_payload filters out heavy itemImage/imageLink keys inside adapter dataclass parser (already done upstream).
  • No literal secrets in diff; DATABASE_URL read from env with clear guard.

Non-blocking suggestions (post-merge follow-ups)

  • LOW · jitter on delayscripts/backfill-house-imv.py:474. PR #533 (sweep-yandex-valuation-ekb.py) uses await asyncio.sleep(delay * random.uniform(0.85, 1.3)) for Avito-captcha resilience. Phase C uses deterministic args.delay; recommend matching the jitter pattern.
  • LOW · consecutive-AuthError early breakprocess_one on IMVAuthError marks one house + sleeps 60s then returns. If Avito blocks auth, all 50 houses in a --batch get marked transient_error over ~50 min. Suggest a counter in main loop: 3+ consecutive auth_error → break with logger.error('Avito auth blocked, aborting batch').
  • LOW · save_imv_result not in try/except — a SQL-side failure (e.g. concurrent house delete → FK violation) kills the whole batch with no mark_status='error' audit trail. Wrap in try/except Exception and mark_status before re-raising or returning 'error'.
  • LOW · --only-status arg validation — no whitelist; typo 'pendng' silently returns 0 rows; passing 'ok' reprocesses healthy houses (unintended). Recommend choices=['pending','transient_error'].
  • LOW · house_suggestions over-specified columns — DDL has area_m2, rooms, floor, total_floors, image_link but IMVSuggestion dataclass doesn't expose these so INSERT leaves them NULL. Either drop the columns or populate from raw_payload now to avoid future schema drift confusion.
  • LOW · no advisory_lock / FOR UPDATE SKIP LOCKED — two concurrent runs against the same DB pick overlapping rows. UPSERT prevents corruption; Avito side gets 2× rate. Docstring states "locally with SSH tunnel" so single-instance is the intended mode; document --only-status transient_error retry pattern as the canonical resumption path.
  • INFO · raw_response jsonb growth — full IMV response × 6,507 houses ≈ 0.5–1.0 GB jsonb. Acceptable for backfill audit; long-term consider pruning to parsed fields once Phase D consumes them.
  • INFO · --skip-recent-days spec drift — body promised this flag; implementation provides only --only-status. To re-scan stale ok houses, operator must manually UPDATE houses SET imv_status='pending' WHERE imv_status='ok' AND last_imv_attempt_at < now() - interval '30 days'. Worth noting in script docstring.

Cross-file impact

  • house_placement_history 1.4× write amplification per backfilled house (8 suggestions + ~5–20 placement items each). Existing indexes hph_house_idx, hph_source_item_idx, hph_house_source_extid_uniq_idx cover the UPSERT cost.
  • houses UPDATE per row writes imv_status, last_imv_attempt_at, imv_error_reason. Partial index houses_imv_status_idx will see ~6,500 deletes (rows leaving pending) over the full sweep — cheap.
  • New house_imv_evaluations is 1:1 with houses (UNIQUE house_id); house_suggestions ≤ 8 per house. Storage cap ~52k rows + jsonb.

Vault cross-check

  • [[Decision_TradeIn_DataQuality_8PR_Roadmap]] covered the original 8/8 estimator-quality PRs (Event_TradeIn_DataQuality_8PR_Done_May24). Phase C (this PR) is a separate per-house data buildup roadmap not in the original 8 — recommend a brief inbox decision note when Phase D lands so the chain is traceable.
  • Adapter contract from [[avito_scraper]]: evaluate_via_imv() 3-request flow + IMVAddressNotFoundError/IMVAuthError/IMVTransientError exception types — script consumes the API surface correctly.

Risk / blast radius

  • Risk: LOW. Migration is pure additive DDL. Script writes to new tables + 3 existing houses columns + UPSERTs into house_placement_history with documented constraint.
  • Reversibility: easy. Migration is IF NOT EXISTS; rollback = DROP TABLE house_imv_evaluations, house_suggestions; ALTER TABLE houses DROP COLUMN imv_status, last_imv_attempt_at, imv_error_reason.
  • Merge window: any. Backfill is operator-initiated; no automatic prod load on merge.

Approving for any-scope auto-merge.

<!-- gendesign-review-bot: sha=e868b01 verdict=approve --> ## Deep Code Review — verdict ### Summary - Status: APPROVE - Files: 2 (P0 SQL migration: 1, P1 script: 1) - Lines: +470 / -0 · Phase C of `[[Decision_TradeIn_DataQuality_8PR_Roadmap]]` - Head SHA: `e868b01278e3b53ae3243f2618ebce3b80a81c30` (matches) ### Verified strengths - Migration `064` fully idempotent: `IF NOT EXISTS` everywhere, single `BEGIN/COMMIT`, lex-sort 063→064 correct for `deploy-tradein.yml` SQL applier loop. - All `ON CONFLICT` targets backed by existing UNIQUE: `house_imv_eval_house_uniq_idx(house_id)`, `house_suggestions_uniq_idx(house_id, ext_item_id)`, and pre-existing `house_placement_history (source, ext_item_id)` from 017. - Resumable backfill: partial index `houses_imv_status_idx WHERE imv_status IN ('pending','transient_error')` makes per-batch `SELECT ... ORDER BY last_imv_attempt_at NULLS FIRST` cheap. - Per-house transaction boundary correct: `db.commit()` after each `process_one`; pre-commit failure in `save_imv_result` leaves house at `'pending'` → re-run picks up cleanly. - `evaluate_via_imv(address=, rooms=, area_m2=, floor=, floor_at_home=, house_type=, renovation_type=, has_balcony=, has_loggia=)` call matches the 3-step adapter signature from PR #524 exactly. - Error mapping aligns with adapter exception hierarchy: `IMVAddressNotFoundError → not_found` (terminal), `IMVAuthError → transient_error + 60s sleep`, `IMVTransientError → transient_error`, generic `Exception → error + repr`. - psycopg v3 driver (`postgresql+psycopg://`) + `CAST(:raw AS jsonb)` everywhere, no `:x::type` traps. - `pick_lot_params` uses correct FK `listings.house_id_fk` + `percentile_cont(0.5) WITHIN GROUP (ORDER BY ...)` median + `mode()` for house_type + reasonable fallbacks (floor=1, total_floors=9, panel/cosmetic). - `raw_payload` filters out heavy `itemImage`/`imageLink` keys inside adapter dataclass parser (already done upstream). - No literal secrets in diff; DATABASE_URL read from env with clear guard. ### Non-blocking suggestions (post-merge follow-ups) - **LOW · jitter on delay** — `scripts/backfill-house-imv.py:474`. PR #533 (`sweep-yandex-valuation-ekb.py`) uses `await asyncio.sleep(delay * random.uniform(0.85, 1.3))` for Avito-captcha resilience. Phase C uses deterministic `args.delay`; recommend matching the jitter pattern. - **LOW · consecutive-AuthError early break** — `process_one` on `IMVAuthError` marks one house + sleeps 60s then returns. If Avito blocks auth, all 50 houses in a `--batch` get marked `transient_error` over ~50 min. Suggest a counter in main loop: 3+ consecutive `auth_error` → break with `logger.error('Avito auth blocked, aborting batch')`. - **LOW · `save_imv_result` not in try/except** — a SQL-side failure (e.g. concurrent house delete → FK violation) kills the whole batch with no `mark_status='error'` audit trail. Wrap in `try/except Exception` and `mark_status` before re-raising or returning `'error'`. - **LOW · `--only-status` arg validation** — no whitelist; typo `'pendng'` silently returns 0 rows; passing `'ok'` reprocesses healthy houses (unintended). Recommend `choices=['pending','transient_error']`. - **LOW · `house_suggestions` over-specified columns** — DDL has `area_m2, rooms, floor, total_floors, image_link` but `IMVSuggestion` dataclass doesn't expose these so INSERT leaves them NULL. Either drop the columns or populate from raw_payload now to avoid future schema drift confusion. - **LOW · no advisory_lock / `FOR UPDATE SKIP LOCKED`** — two concurrent runs against the same DB pick overlapping rows. UPSERT prevents corruption; Avito side gets 2× rate. Docstring states "locally with SSH tunnel" so single-instance is the intended mode; document `--only-status transient_error` retry pattern as the canonical resumption path. - **INFO · `raw_response` jsonb growth** — full IMV response × 6,507 houses ≈ 0.5–1.0 GB jsonb. Acceptable for backfill audit; long-term consider pruning to parsed fields once Phase D consumes them. - **INFO · `--skip-recent-days` spec drift** — body promised this flag; implementation provides only `--only-status`. To re-scan stale `ok` houses, operator must manually `UPDATE houses SET imv_status='pending' WHERE imv_status='ok' AND last_imv_attempt_at < now() - interval '30 days'`. Worth noting in script docstring. ### Cross-file impact - `house_placement_history` 1.4× write amplification per backfilled house (8 suggestions + ~5–20 placement items each). Existing indexes `hph_house_idx`, `hph_source_item_idx`, `hph_house_source_extid_uniq_idx` cover the UPSERT cost. - `houses` UPDATE per row writes `imv_status, last_imv_attempt_at, imv_error_reason`. Partial index `houses_imv_status_idx` will see ~6,500 deletes (rows leaving `pending`) over the full sweep — cheap. - New `house_imv_evaluations` is 1:1 with houses (UNIQUE house_id); `house_suggestions` ≤ 8 per house. Storage cap ~52k rows + jsonb. ### Vault cross-check - `[[Decision_TradeIn_DataQuality_8PR_Roadmap]]` covered the original 8/8 estimator-quality PRs (`Event_TradeIn_DataQuality_8PR_Done_May24`). Phase C (this PR) is a separate per-house data buildup roadmap not in the original 8 — recommend a brief inbox decision note when Phase D lands so the chain is traceable. - Adapter contract from `[[avito_scraper]]`: `evaluate_via_imv()` 3-request flow + `IMVAddressNotFoundError`/`IMVAuthError`/`IMVTransientError` exception types — script consumes the API surface correctly. ### Risk / blast radius - Risk: LOW. Migration is pure additive DDL. Script writes to new tables + 3 existing `houses` columns + UPSERTs into `house_placement_history` with documented constraint. - Reversibility: easy. Migration is `IF NOT EXISTS`; rollback = `DROP TABLE house_imv_evaluations, house_suggestions; ALTER TABLE houses DROP COLUMN imv_status, last_imv_attempt_at, imv_error_reason`. - Merge window: any. Backfill is operator-initiated; no automatic prod load on merge. Approving for any-scope auto-merge.
lekss361 merged commit b0d377c984 into main 2026-05-24 14:50:32 +00:00
lekss361 deleted branch feat/tradein-phase-c-house-imv-backfill 2026-05-24 14:50:32 +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#534
No description provided.