feat(tradein): cian price-change badges + split chart Avito/Yandex #547

Merged

lekss361 merged 1 commit from feat/tradein-multi-source-v2 into main

2026-05-24 18:24:01 +00:00

lekss361 commented

2026-05-24 18:16:34 +00:00

Owner

Контекст

Продолжение #546 (уже смержён). Добавляет 2 блока аналитики из live данных.

A. Cian price-change badges

Использует offer_price_history (268+ rows, активно льёт cian playwright_history).

Backend: GET /api/v1/trade-in/estimate/{id}/cian-price-changes — per-analog stats (n_changes, last_diff_pct, total_change_pct).
Frontend: ListingsCard рендерит colored badge ↓N ±X.X% рядом с ценой cian-аналога (зелёный = снижение, красный = рост).

B. Split PriceHistoryChart

Использует house_placement_history.source column (avito_imv + yandex_valuation, 15k+ rows).

Backend (breaking schema): PriceHistoryYearPoint получает поле source, SQL GROUP BY year, source.
Frontend: pivot long→wide client-side, 2 Line (синий Avito #2563eb + оранжевый Яндекс #ea580c) + Legend.

НЕ сделано из плана

Yandex chart_change_pct — NULL у всех 61 row в external_valuations. Реальная история уже в house_placement_history через raw_payload.history_items.
Source-comparison live — требует Phase 0 matching pipeline (listing_sources=0). Отложено в backlog.

Test plan

backend: ruff/mypy/pre-commit pass
frontend: tsc/lint/build pass
smoke /trade-in?id=d1570e00 после deploy

## Контекст Продолжение #546 (уже смержён). Добавляет 2 блока аналитики из live данных. ## A. Cian price-change badges Использует **offer_price_history** (268+ rows, активно льёт cian playwright_history). - Backend: `GET /api/v1/trade-in/estimate/{id}/cian-price-changes` — per-analog stats (n_changes, last_diff_pct, total_change_pct). - Frontend: ListingsCard рендерит colored badge ↓N ±X.X% рядом с ценой cian-аналога (зелёный = снижение, красный = рост). ## B. Split PriceHistoryChart Использует **house_placement_history.source** column (avito_imv + yandex_valuation, 15k+ rows). - Backend (breaking schema): PriceHistoryYearPoint получает поле `source`, SQL `GROUP BY year, source`. - Frontend: pivot long→wide client-side, 2 Line (синий Avito #2563eb + оранжевый Яндекс #ea580c) + Legend. ## НЕ сделано из плана - Yandex chart_change_pct — NULL у всех 61 row в external_valuations. Реальная история уже в house_placement_history через raw_payload.history_items. - Source-comparison live — требует Phase 0 matching pipeline (listing_sources=0). Отложено в backlog. ## Test plan - [x] backend: ruff/mypy/pre-commit pass - [x] frontend: tsc/lint/build pass - [ ] smoke /trade-in?id=d1570e00 после deploy

lekss361 added 1 commit 2026-05-24 18:16:34 +00:00

feat(tradein): cian price-change badges + split price-history chart 07cbcd235b

Backend:
- New GET /api/v1/trade-in/estimate/{id}/cian-price-changes — per-analog
  price change stats (n_changes, last_diff_pct, total_change_pct) из
  offer_price_history (cian playwright_history backfill, 268+ rows).
- house-analytics: PriceHistoryYearPoint gains source field; SQL теперь
  GROUP BY year, source → 2 серии для chart (avito_imv / yandex_valuation).

Frontend:
- ListingsCard показывает colored badge ↓N ±X.X% рядом с ценой cian-аналога
  если есть price changes (зелёный = снижение, красный = рост).
- PriceHistoryChart: pivot long→wide format client-side, 2 Line components
  (синий Avito #2563eb, оранжевый Яндекс #ea580c) + Legend, connectNulls.

Использует данные:
- offer_price_history (live cian price changes)
- house_placement_history.source разделение avito_imv vs yandex_valuation

lekss361 commented

2026-05-24 18:23:54 +00:00

Author

Owner

Deep Code Review — verdict

Summary

Status: APPROVE
Files: 7 (P0:2 backend SQL endpoint + Pydantic schema, P1:3 UI components + hook, P3:2 types/page wiring)
Lines: +248 / -17 · PR: #547 · head SHA: 07cbcd2

Strengths

New endpoint GET /estimate/{id}/cian-price-changes is properly parameterized: CAST(:eid AS uuid) + named bindings — no injection vector.
Regex escaping in SQL (Python literal becomes SQL POSIX \d+) and JS literal — both correct.
Types match across SQL → Pydantic → TS (listings.source_id is text, Cian ID kept as str/string throughout — no coercion traps).
XSS-safe: badge content is only int/float from DB, no user-supplied strings. safeUrl still wraps external links.
Indexes leveraged: oph_listing_time_idx (listing_id, change_time DESC) covers MAX + ORDER BY LIMIT 1 + GROUP BY.
enabled: estimate_id !== null && estimate_id.length > 0 on TanStack query prevents premature fetch.
HouseAnalyticsKpiRow double-minus bug from #546 NOT reproduced here: last_diff_percent > 0 ? '+' : '' + .toFixed(1) produces single -5.0% for negatives.
Recharts pattern matches #530 conventions: connectNulls bridges sparse year coverage; pivoted data keyed by year is stable.
Backend SQL grouped by (year, source) with ORDER BY year ASC, source ASC — deterministic.

Cross-file impact

PriceHistoryYearPoint adds required source field — only callers are this endpoint + PriceHistoryChart; pivot absorbs new shape; no other grepped consumers.
ListingsCard prop estimateId?: string is optional — backward compatible.
HouseAnalyticsSection rendering unchanged.
No DB migration needed — reads existing tables (offer_price_history, listings, trade_in_estimates).

Nit-level (non-blocking)

L4 — Pivot silently drops non-avito_imv/yandex_valuation sources. house_placement_history.source also allows avito / avito_widget (sql/017 + sql/032 + scrapers). If those rows exist for a house, chart lines won't show them, but totalLots = points.reduce(...) (PriceHistoryChart.tsx:43) still sums their n_lots in the caption — slight count/visual mismatch. In practice avito_widget data is rare per current session state; consider an else branch or filtering totalLots to known sources.
L4 — Zero-delta colouring. last_diff_percent < 0 ? green : red → exactly 0 falls into red. Realistically unobservable; <= 0 or neutral branch would be defensible.
L4 — Correlated subquery in changes_agg for first_seen_price could be first_value(price_rub) OVER (PARTITION BY listing_id ORDER BY change_time ASC). At small N (~5-20 Cian analogs per estimate) cost is negligible.

Tests

No unit tests added. Endpoint is read-only and behaviour is smoke-testable via UI badge. Consistent with peer endpoints (/imv-benchmark, /house-analytics). Not a blocker.

Verdict

Approve and merge. Clean cross-domain change, no regressions in the existing chart/listings card.

## Deep Code Review — verdict ### Summary - Status: APPROVE - Files: 7 (P0:2 backend SQL endpoint + Pydantic schema, P1:3 UI components + hook, P3:2 types/page wiring) - Lines: +248 / -17 · PR: #547 · head SHA: `07cbcd2` ### Strengths - New endpoint `GET /estimate/{id}/cian-price-changes` is properly parameterized: `CAST(:eid AS uuid)` + named bindings — no injection vector. - Regex escaping in SQL (Python literal becomes SQL POSIX `\d+`) and JS literal — both correct. - Types match across SQL → Pydantic → TS (`listings.source_id` is `text`, Cian ID kept as `str`/`string` throughout — no coercion traps). - XSS-safe: badge content is only int/float from DB, no user-supplied strings. `safeUrl` still wraps external links. - Indexes leveraged: `oph_listing_time_idx (listing_id, change_time DESC)` covers MAX + ORDER BY LIMIT 1 + GROUP BY. - `enabled: estimate_id !== null && estimate_id.length > 0` on TanStack query prevents premature fetch. - HouseAnalyticsKpiRow double-minus bug from #546 NOT reproduced here: `last_diff_percent > 0 ? '+' : ''` + `.toFixed(1)` produces single `-5.0%` for negatives. - Recharts pattern matches #530 conventions: `connectNulls` bridges sparse year coverage; pivoted data keyed by year is stable. - Backend SQL grouped by `(year, source)` with `ORDER BY year ASC, source ASC` — deterministic. ### Cross-file impact - `PriceHistoryYearPoint` adds required `source` field — only callers are this endpoint + `PriceHistoryChart`; pivot absorbs new shape; no other grepped consumers. - `ListingsCard` prop `estimateId?: string` is optional — backward compatible. - `HouseAnalyticsSection` rendering unchanged. - No DB migration needed — reads existing tables (`offer_price_history`, `listings`, `trade_in_estimates`). ### Nit-level (non-blocking) 1. **L4 — Pivot silently drops non-`avito_imv`/`yandex_valuation` sources.** `house_placement_history.source` also allows `avito` / `avito_widget` (sql/017 + sql/032 + scrapers). If those rows exist for a house, chart lines won't show them, but `totalLots = points.reduce(...)` (PriceHistoryChart.tsx:43) still sums their `n_lots` in the caption — slight count/visual mismatch. In practice avito_widget data is rare per current session state; consider an `else` branch or filtering totalLots to known sources. 2. **L4 — Zero-delta colouring.** `last_diff_percent < 0 ? green : red` → exactly `0` falls into red. Realistically unobservable; `<= 0` or neutral branch would be defensible. 3. **L4 — Correlated subquery in `changes_agg`** for `first_seen_price` could be `first_value(price_rub) OVER (PARTITION BY listing_id ORDER BY change_time ASC)`. At small N (~5-20 Cian analogs per estimate) cost is negligible. ### Tests No unit tests added. Endpoint is read-only and behaviour is smoke-testable via UI badge. Consistent with peer endpoints (`/imv-benchmark`, `/house-analytics`). Not a blocker. ### Verdict Approve and merge. Clean cross-domain change, no regressions in the existing chart/listings card.