From 82d8db9f425bdfe7326c8bfb18fa5bd29b45746f Mon Sep 17 00:00:00 2001 From: lekss361 Date: Tue, 11 Aug 2026 04:15:40 +0000 Subject: [PATCH] =?UTF-8?q?fix(tradein/estimate):=20=D0=BE=D0=B6=D0=B8?= =?UTF-8?q?=D0=B2=D0=BB=D1=8F=D1=82=D1=8C=20=D0=BC=D1=91=D1=80=D1=82=D0=B2?= =?UTF-8?q?=D1=8B=D0=B5=20=D1=81=D0=BE=D1=85=D1=80=D0=B0=D0=BD=D1=91=D0=BD?= =?UTF-8?q?=D0=BD=D1=8B=D0=B5=20=D0=BE=D1=86=D0=B5=D0=BD=D0=BA=D0=B8=20?= =?UTF-8?q?=D0=BF=D1=80=D0=B8=20=D0=BE=D1=82=D0=BA=D1=80=D1=8B=D1=82=D0=B8?= =?UTF-8?q?=D0=B8=20(#2826)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tradein-mvp/backend/app/api/v1/trade_in.py | 258 ++++++++- tradein-mvp/backend/app/core/config.py | 13 + tradein-mvp/backend/app/services/estimator.py | 14 + ...trade_in_estimates_revival_relaxations.sql | 77 +++ .../backend/tests/test_estimate_idor.py | 8 + .../backend/tests/test_estimate_revival.py | 542 ++++++++++++++++++ .../components/trade-in/v2/ParamsPanel.tsx | 26 +- 7 files changed, 935 insertions(+), 3 deletions(-) create mode 100644 tradein-mvp/backend/data/sql/255_trade_in_estimates_revival_relaxations.sql create mode 100644 tradein-mvp/backend/tests/test_estimate_revival.py diff --git a/tradein-mvp/backend/app/api/v1/trade_in.py b/tradein-mvp/backend/app/api/v1/trade_in.py index 4aa9934c..7c806bf0 100644 --- a/tradein-mvp/backend/app/api/v1/trade_in.py +++ b/tradein-mvp/backend/app/api/v1/trade_in.py @@ -5,7 +5,9 @@ from __future__ import annotations +import asyncio import calendar +import json import logging from datetime import UTC, date, datetime, timedelta from typing import Annotated, Any @@ -167,6 +169,228 @@ def _resolve_target_house_id( return None +# ── Revival на GET /estimate/{id} (incident 2026-08-10) ───────────────────── +# Заказчик открыл сохранённую ссылку (?id=...) и увидел «НЕДОСТАТОЧНО ДАННЫХ»: +# запись создана ДО фикса оценщика (#oblast-E/#oblast-F, PR #2823/#2825) и +# лежит в БД мёртвой (median_price<=0/NULL), хотя тот же адрес/параметры +# сейчас честно считаются. get_estimate() ниже пытается пересчитать такую +# строку ОДИН раз (throttled) через тот же estimate_quality(), что и POST +# /estimate, и пишет результат В ТУ ЖЕ строку (id/ссылка не меняются). Живую +# строку (median_price>0) этот путь не трогает вообще — сохранённая клиенту +# цена неприкосновенна. +def _precision_to_qc_geo(precision: str | None) -> int | None: + """Best-effort обратное отображение к estimator._qc_geo_to_precision. + + AggregatedEstimate наружу отдаёт только бакетированный address_precision + (house/street/approximate), не сырой dadata.qc_geo (0..5) — тот остаётся + приватным для estimate_quality(). При revival нам нужно записать ЧТО-ТО в + колонку dadata_qc_geo, чтобы будущие (уже НЕ revival, обычные) GET той же + теперь-живой строки не откатили address_precision в None. Бакеты 2..5 + (settlement/city/region/unknown) неразличимы ПОСЛЕ _qc_geo_to_precision — + 2 репрезентативно для всех: тот же helper на чтении схлопывает их обратно + в тот же "approximate", наблюдаемое поведение не меняется. + """ + if precision == "house": + return 0 + if precision == "street": + return 1 + if precision == "approximate": + return 2 + return None + + +def _payload_from_dead_row(row: Any) -> TradeInEstimateInput: + """Восстанавливает вход оценки из мёртвой сохранённой строки для revival. + + Только поля, реально персистящиеся в trade_in_estimates при создании + (address/lat/lon/area_m2/rooms/floor/total_floors/year_built/house_type/ + repair_state/has_balcony) — CRM-only поля (ownership_type/has_mortgage) + на расчёт не влияют и не нужны здесь. radius_m НИКОГДА не персистится + (payload.radius_m живёт только в рамках одного POST-запроса, ни главный + INSERT, ни _empty_estimate его не пишут) — None здесь даёт тот же + default-каскад (DEFAULT_RADIUS_M/FALLBACK_RADIUS_M), что у подавляющего + большинства сохранённых строк (явный радиус выбирает меньшинство). + consent=None + require_consent=False у вызывающего — revival не новое + согласие физлица, а служебный recompute уже существующей записи. + """ + return TradeInEstimateInput( + address=row.address, + area_m2=float(row.area_m2), + rooms=row.rooms, + floor=row.floor, + total_floors=row.total_floors, + year_built=row.year_built, + house_type=row.house_type, + repair_state=row.repair_state, + has_balcony=row.has_balcony, + lat=row.lat, + lon=row.lon, + radius_m=None, + consent=None, + ) + + +async def _try_revive_dead_estimate( + db: Session, estimate_id: UUID, row: Any +) -> AggregatedEstimate | None: + """Пытается пересчитать «мёртвую» (median_price<=0/NULL) строку на месте. + + Возвращает свежий AggregatedEstimate (estimate_id ПОДМЕНЁН на исходный — + id/ссылка не меняются) при успехе; None если: (а) throttle ещё не истёк / + заявку уже забрал параллельный запрос — anti-storm через атомарный + conditional `UPDATE ... RETURNING` ниже (тот же паттерн, что + account_quota.increment, #747): WHERE перепроверяет и «мертва ли строка + сейчас», и «давно ли последняя попытка» НЕПОСРЕДСТВЕННО в БД, а не по + значению, прочитанному раньше в Python — TOCTOU-гонка между двумя + параллельными GET невозможна, проигравший просто не дублирует работу; + (б) пересчёт сам дал 0 (по-прежнему недостаточно данных); (в) пересчёт + упал с исключением (сеть/геокод/что угодно). Во всех трёх случаях caller + обязан отдать сохранённую (по-прежнему мёртвую) строку как раньше — НЕ 500. + """ + claim = db.execute( + text( + """ + UPDATE trade_in_estimates + SET revival_attempted_at = NOW() + WHERE id = CAST(:id AS uuid) + AND (median_price <= 0 OR median_price IS NULL) + AND ( + revival_attempted_at IS NULL + OR revival_attempted_at + < NOW() - make_interval(mins => CAST(:throttle AS integer)) + ) + RETURNING id + """ + ), + {"id": str(estimate_id), "throttle": settings.trade_in_revival_throttle_minutes}, + ).fetchone() + db.commit() + if claim is None: + logger.info("estimate revival throttled/lost race: id=%s", estimate_id) + return None + + from app.services.estimator import estimate_quality + + try: + payload = _payload_from_dead_row(row) + result = await estimate_quality( + payload, + db, + created_by=row.created_by, + client_ip=None, + require_consent=False, + ) + except Exception: + logger.exception("estimate revival failed: id=%s address=%r", estimate_id, row.address) + return None + + temp_id = result.estimate_id + if result.median_price_rub <= 0: + logger.info("estimate revival still insufficient data: id=%s", estimate_id) + db.execute( + text("DELETE FROM trade_in_estimates WHERE id = CAST(:id AS uuid)"), + {"id": str(temp_id)}, + ) + db.commit() + return None + + # estimate_quality() persists under a BRAND NEW uuid (temp_id) — it has no + # notion of "recompute this existing row". Copy the computed OUTPUT fields + # into the ORIGINAL row (id/link contract), then drop the throwaway one. + # INPUT snapshot (address/area/rooms/...) is untouched — it did not change, + # only the outputs were recomputed. + db.execute( + text( + """ + UPDATE avito_imv_evaluations + SET estimate_id = CAST(:orig AS uuid) + WHERE estimate_id = CAST(:temp AS uuid) + """ + ), + {"orig": str(estimate_id), "temp": str(temp_id)}, + ) + db.execute( + text( + """ + UPDATE trade_in_estimates SET + median_price = :median_price, + range_low = :range_low, + range_high = :range_high, + median_price_per_m2 = :median_ppm2, + confidence = :confidence, + confidence_explanation = :explanation, + n_analogs = :n_analogs, + analogs = CAST(:analogs_json AS jsonb), + actual_deals = CAST(:deals_json AS jsonb), + sources_used = CAST(:sources_json AS jsonb), + data_freshness_minutes = :freshness, + canonical_address = :canonical_address, + house_cadnum = :house_cadnum, + house_fias_id = :house_fias_id, + dadata_qc_geo = :dadata_qc_geo, + dadata_metro = CAST(:dadata_metro_json AS jsonb), + expected_sold_price = :expected_sold_price, + expected_sold_range_low = :expected_sold_range_low, + expected_sold_range_high = :expected_sold_range_high, + expected_sold_per_m2 = :expected_sold_per_m2, + asking_to_sold_ratio = :asking_to_sold_ratio, + ratio_basis = :ratio_basis, + relaxations = CAST(:relaxations_json AS jsonb), + reliability = :reliability, + created_at = :created_at + WHERE id = CAST(:id AS uuid) + """ + ), + { + "id": str(estimate_id), + "median_price": result.median_price_rub, + "range_low": result.range_low_rub, + "range_high": result.range_high_rub, + "median_ppm2": result.median_price_per_m2, + "confidence": result.confidence, + "explanation": result.confidence_explanation, + "n_analogs": result.n_analogs, + "analogs_json": json.dumps( + [a.model_dump(mode="json") for a in result.analogs], ensure_ascii=False + ), + "deals_json": json.dumps( + [a.model_dump(mode="json") for a in result.actual_deals], ensure_ascii=False + ), + "sources_json": json.dumps(result.sources_used, ensure_ascii=False), + "freshness": result.data_freshness_minutes, + "canonical_address": result.canonical_address, + "house_cadnum": result.house_cadnum, + "house_fias_id": result.house_fias_id, + "dadata_qc_geo": _precision_to_qc_geo(result.address_precision), + "dadata_metro_json": json.dumps(result.metro_nearest, ensure_ascii=False), + "expected_sold_price": result.expected_sold_price_rub, + "expected_sold_range_low": result.expected_sold_range_low_rub, + "expected_sold_range_high": result.expected_sold_range_high_rub, + "expected_sold_per_m2": result.expected_sold_per_m2, + "asking_to_sold_ratio": result.asking_to_sold_ratio, + "ratio_basis": result.ratio_basis, + "relaxations_json": json.dumps(result.relaxations, ensure_ascii=False), + "reliability": result.reliability, + "created_at": result.created_at, + }, + ) + db.execute( + text("DELETE FROM trade_in_estimates WHERE id = CAST(:id AS uuid)"), + {"id": str(temp_id)}, + ) + db.commit() + + logger.info( + "estimate revived: id=%s median=%d n=%d confidence=%s reliability=%s", + estimate_id, + result.median_price_rub, + result.n_analogs, + result.confidence, + result.reliability, + ) + return result.model_copy(update={"estimate_id": estimate_id}) + + @router.post("/estimate", response_model=AggregatedEstimate) async def estimate( payload: TradeInEstimateInput, @@ -281,7 +505,8 @@ def get_estimate( dadata_qc_geo, dadata_metro, expected_sold_price, expected_sold_range_low, expected_sold_range_high, expected_sold_per_m2, - asking_to_sold_ratio, ratio_basis, created_by, created_at + asking_to_sold_ratio, ratio_basis, created_by, created_at, + relaxations, reliability FROM trade_in_estimates WHERE id = CAST(:id AS uuid) AND {ESTIMATE_READABLE_SQL} @@ -295,6 +520,22 @@ def get_estimate( _assert_estimate_access(row.created_by, x_authenticated_user) + # #incident-2026-08-10: строка «мертва» (median_price<=0/NULL) — посчитана + # ДО фикса оценщика (#oblast-E/#oblast-F, PR #2823/#2825). Пробуем + # пересчитать её на месте (throttled, race-safe — см. докстринг + # _try_revive_dead_estimate) через тот же путь, что и POST /estimate. + # Живую строку (median_price>0) не трогаем вообще. asyncio.run() — sync↔ + # async мост (тот же паттерн, что app/scheduler_main.py): get_estimate + # остаётся `def` (Starlette гоняет его в threadpool, как сейчас), поэтому + # ОСТАЛЬНЫЕ синхронные db.execute() ниже по функции не переезжают на event + # loop — только сама попытка revival временно занимает свой поток на время + # await estimate_quality(). Любая ошибка расчёта — не 500: revived is None, + # и функция просто продолжает как раньше, отдавая сохранённую строку. + if row.median_price is None or row.median_price <= 0: + revived = asyncio.run(_try_revive_dead_estimate(db, estimate_id, row)) + if revived is not None: + return revived + from app.services.estimator import ( _canonical_sources, _cv_from_ppm2, @@ -431,6 +672,14 @@ def get_estimate( cv=cv, source_counts=source_counts, created_at=row.created_at, + # PR #2823 open follow-up (fixed incident 2026-08-10, migration 255): + # relaxations/reliability теперь персистятся — GET-rehydrate больше не + # теряет красный баннер «точность снижена» при открытии по ссылке. + # getattr defensive: старые in-memory test doubles / любая строка без + # этих колонок (не должно случаться после миграции) деградируют в + # дефолт схемы (ok / []), а не падают AttributeError. + relaxations=list(getattr(row, "relaxations", None) or []), + reliability=getattr(row, "reliability", None) or "ok", ) @@ -462,7 +711,8 @@ def estimate_pdf( dadata_qc_geo, dadata_metro, expected_sold_price, expected_sold_range_low, expected_sold_range_high, expected_sold_per_m2, - asking_to_sold_ratio, ratio_basis, created_by + asking_to_sold_ratio, ratio_basis, created_by, + relaxations, reliability FROM trade_in_estimates WHERE id = CAST(:id AS uuid) """ @@ -523,6 +773,10 @@ def estimate_pdf( house_fias_id=row.house_fias_id, address_precision=_qc_geo_to_precision(row.dadata_qc_geo), metro_nearest=(row.dadata_metro or []), + # migration 255 — та же сноска «точность снижена», что и на JSON GET, + # теперь и в PDF-регенерации сохранённой оценки (см. get_estimate). + relaxations=list(getattr(row, "relaxations", None) or []), + reliability=getattr(row, "reliability", None) or "ok", ) input_snapshot = { "address": row.address, diff --git a/tradein-mvp/backend/app/core/config.py b/tradein-mvp/backend/app/core/config.py index b0f5b791..8f0e2b31 100644 --- a/tradein-mvp/backend/app/core/config.py +++ b/tradein-mvp/backend/app/core/config.py @@ -852,6 +852,19 @@ class Settings(BaseSettings): # «год»". ENV: TRADE_IN_PAID_RETENTION_DAYS. trade_in_paid_retention_days: int = 365 + # ── Revival на GET /estimate/{id} (incident 2026-08-10) ───────────────── + # Throttle повторных попыток пересчёта «мёртвой» (median_price<=0/NULL) + # сохранённой строки — записи, посчитанные ДО фикса оценщика (#oblast-E/F, + # PR #2823/#2825) и навсегда застрявшие с median_price=0. GET пытается + # пересчитать такую строку через тот же estimate_quality(), что и POST + # (app/api/v1/trade_in.py::_try_revive_dead_estimate), не чаще одного раза + # в это число минут на строку — иначе каждый refresh страницы бил бы по + # геокодеру/DaData для объективно мёртвого адреса. 10 минут — компромисс: + # достаточно редко, чтобы не спамить внешние сервисы, достаточно быстро, + # чтобы повторный визит клиента после нашего фикса увидел живую цену. ENV: + # TRADE_IN_REVIVAL_THROTTLE_MINUTES. + trade_in_revival_throttle_minutes: int = 10 + # Батч-размер физического DELETE в purge_expired_trade_in_data (нельзя одним # DELETE по всей таблице — долгая блокировка на большом бэклоге). Задача сама # крутит цикл батчей за один прогон (см. _DEFAULT_MAX_BATCHES в таске) — diff --git a/tradein-mvp/backend/app/services/estimator.py b/tradein-mvp/backend/app/services/estimator.py index 8f809e37..5e3ef0a3 100644 --- a/tradein-mvp/backend/app/services/estimator.py +++ b/tradein-mvp/backend/app/services/estimator.py @@ -4370,6 +4370,7 @@ async def estimate_quality( expected_sold_price, expected_sold_range_low, expected_sold_range_high, expected_sold_per_m2, asking_to_sold_ratio, ratio_basis, + relaxations, reliability, created_by, expires_at, consent, client_ip, consent_policy_version, consent_text_snapshot @@ -4391,6 +4392,7 @@ async def estimate_quality( :expected_sold_price, :expected_sold_range_low, :expected_sold_range_high, :expected_sold_per_m2, :asking_to_sold_ratio, :ratio_basis, + CAST(:relaxations_json AS jsonb), :reliability, :created_by, :expires_at, :consent, CAST(:client_ip AS inet), :consent_policy_version, @@ -4444,6 +4446,12 @@ async def estimate_quality( "expected_sold_per_m2": expected_sold_per_m2, "asking_to_sold_ratio": asking_to_sold_ratio, "ratio_basis": ratio_basis, + # #oblast-F / GET-rehydrate (PR #2823 open follow-up): персистим + # relaxations/reliability вместе с median — раньше эти поля жили + # только в POST-ответе, и красный баннер «точность снижена» + # пропадал при открытии оценки по сохранённой ссылке (?id=). + "relaxations_json": json.dumps(relaxations, ensure_ascii=False), + "reliability": reliability, "created_by": created_by, "expires_at": expires_at, **_estimate_consent_persist_fields(require_consent, client_ip), @@ -6885,6 +6893,7 @@ def _empty_estimate( confidence, confidence_explanation, n_analogs, analogs, actual_deals, sources_used, + relaxations, reliability, created_by, expires_at, consent, client_ip, consent_policy_version, consent_text_snapshot @@ -6897,6 +6906,11 @@ def _empty_estimate( 'low', :explanation, 0, '[]'::jsonb, '[]'::jsonb, '[]'::jsonb, + -- #oblast-F: поиск аналогов вообще не выполнялся (geocode failed / + -- no coords) — каскад послаблений не запускался, relaxations честно + -- пуст; reliability='very_low' зеркалит то, что возвращает Python + -- ниже (см. AggregatedEstimate(..., reliability="very_low")). + '[]'::jsonb, 'very_low', :created_by, :expires_at, :consent, CAST(:client_ip AS inet), :consent_policy_version, diff --git a/tradein-mvp/backend/data/sql/255_trade_in_estimates_revival_relaxations.sql b/tradein-mvp/backend/data/sql/255_trade_in_estimates_revival_relaxations.sql new file mode 100644 index 00000000..de19a1f4 --- /dev/null +++ b/tradein-mvp/backend/data/sql/255_trade_in_estimates_revival_relaxations.sql @@ -0,0 +1,77 @@ +-- 255_trade_in_estimates_revival_relaxations.sql +-- Номер сверен по `ls data/sql | sort` (max applied = 254) непосредственно +-- перед коммитом — см. sql.md § file naming + tradein.md § collision trap +-- (108_*/084_* уже дублировались в прошлом). +-- +-- ── Incident 2026-08-10: «мёртвые» сохранённые оценки ─────────────────────── +-- Заказчик открыл сохранённую ссылку /trade-in/v2?id=... и увидел «НЕДОСТАТОЧНО +-- ДАННЫХ»: запись создана ДО фикса оценщика (#oblast-E/#oblast-F, PR +-- #2823/#2825) и лежит в БД с median_price=0/NULL, хотя тот же адрес и +-- параметры сейчас честно считаются (4 031 157 ₽ / 39 аналогов). 117 из 1071 +-- строк trade_in_estimates находятся в этом состоянии (29 за последние 30 +-- дней). GET /api/v1/trade-in/estimate/{id} (app/api/v1/trade_in.py:: +-- _try_revive_dead_estimate) теперь пересчитывает такую строку на месте и +-- пишет результат В ТУ ЖЕ строку (id/ссылка не меняются) — этому нужны две +-- новые колонки: +-- +-- revival_attempted_at — throttle: не пересчитывать чаще одного раза в N +-- минут (settings.trade_in_revival_throttle_minutes, default 10) на одну +-- строку. Заявка на пересчёт — атомарный conditional +-- `UPDATE ... WHERE revival_attempted_at IS NULL OR ... < NOW() - N min +-- RETURNING id` (тот же паттерн, что account_quota.increment, #747) — +-- защищает и от шторма повторных попыток на мёртвый адрес, и от гонки +-- двух параллельных GET (второй просто теряет заявку и отдаёт то, что +-- есть, без 500). +-- +-- ── relaxations / reliability (открытый хвост PR #2823, найден post-deploy +-- 2026-08-10 — см. fixes/Fix_Mera_Studio_Not_Estimated_Never_Block_Aug10) ── +-- Обе колонки УЖЕ вычисляются в estimator.estimate_quality() и уходят в POST- +-- ответ (AggregatedEstimate.relaxations/reliability), но раньше НЕ +-- персистились — на GET-rehydrate (открытие сохранённой ссылки) красный +-- баннер «точность снижена» пропадал, хотя цена по-прежнему построена на +-- расширенной/тонкой выборке. Теперь пишутся при каждом (re)compute (основной +-- INSERT в estimate_quality() + этот revival-путь) и читаются на GET. +-- +-- ── IDEMPOTENCY ─────────────────────────────────────────────────────────── +-- ADD COLUMN IF NOT EXISTS — безопасный re-run. Бэкфилла нет: все существующие +-- строки получают DEFAULT (reliability='ok', relaxations='[]', revival_ +-- attempted_at=NULL) — честно отражает то, что для них каскад послаблений +-- никогда не считался (записи ДО #oblast-F) и revival ещё не запускался. +-- +-- Dependencies: 001_trade_in_estimates.sql. Apply after: 254_*. + +BEGIN; + +SET LOCAL lock_timeout = '5s'; + +ALTER TABLE trade_in_estimates + ADD COLUMN IF NOT EXISTS relaxations jsonb NOT NULL DEFAULT '[]'::jsonb; + +ALTER TABLE trade_in_estimates + ADD COLUMN IF NOT EXISTS reliability text NOT NULL DEFAULT 'ok' + CHECK (reliability IN ('ok', 'low', 'very_low')); + +ALTER TABLE trade_in_estimates + ADD COLUMN IF NOT EXISTS revival_attempted_at timestamptz; + +COMMENT ON COLUMN trade_in_estimates.relaxations IS + 'RU-подписи применённых послаблений подбора (estimator.py #oblast-F cascade) ' + '— персистится, чтобы GET-rehydrate (?id=) мог восстановить дисклеймер ' + '«точность снижена». [] = базовой выборки хватило / запись создана до ' + '#oblast-F (без бэкфилла).'; + +COMMENT ON COLUMN trade_in_estimates.reliability IS + 'Надёжность итоговой выборки (ok|low|very_low), производная от n_analogs + ' + 'relaxations (estimator.py::estimate_quality) — персистится для GET-rehydrate ' + 'красного баннера. Default ok = запись создана до #oblast-F (без бэкфилла).'; + +COMMENT ON COLUMN trade_in_estimates.revival_attempted_at IS + 'Момент последней попытки пересчитать «мёртвую» (median_price<=0/NULL) ' + 'строку на GET /estimate/{id} (incident 2026-08-10, app/api/v1/trade_in.py::' + '_try_revive_dead_estimate). Throttle: не пересчитывать чаще одного раза в ' + 'settings.trade_in_revival_throttle_minutes на одну строку — атомарный ' + 'conditional UPDATE...RETURNING (см. модульный докстринг). NULL = либо ' + 'строка живая (median_price>0) и revival никогда не запускался, либо ' + 'запись создана до этой фичи.'; + +COMMIT; diff --git a/tradein-mvp/backend/tests/test_estimate_idor.py b/tradein-mvp/backend/tests/test_estimate_idor.py index a9ed8d31..790fc31f 100644 --- a/tradein-mvp/backend/tests/test_estimate_idor.py +++ b/tradein-mvp/backend/tests/test_estimate_idor.py @@ -57,6 +57,12 @@ def _make_estimate_row(created_by: str | None, retain_until: object = None) -> S retain_until defaults to None (PR-D1, migration 240) -- unpaid, matches every row that existed before that migration; explicit param lets retention-gate tests (see test_estimate_retention_gate.py) construct a paid row. + + relaxations/reliability (migration 255) default to the schema defaults + ('[]' / 'ok') -- matches every pre-migration row (no backfill). Revival + scenarios (a "dead" median_price<=0/NULL row) are covered separately in + test_estimate_revival.py with their own dedicated row builders, since this + fixture's downstream tests here all assume a "live" estimate. """ from datetime import UTC, datetime, timedelta @@ -99,6 +105,8 @@ def _make_estimate_row(created_by: str | None, retain_until: object = None) -> S ratio_basis="per_rooms", created_by=created_by, created_at=datetime.now(tz=UTC), + relaxations=[], + reliability="ok", ) diff --git a/tradein-mvp/backend/tests/test_estimate_revival.py b/tradein-mvp/backend/tests/test_estimate_revival.py new file mode 100644 index 00000000..42a0b439 --- /dev/null +++ b/tradein-mvp/backend/tests/test_estimate_revival.py @@ -0,0 +1,542 @@ +"""Tests for GET /estimate/{id} revival of "dead" (median_price<=0/NULL) rows. + +Incident 2026-08-10: a customer opened a saved estimate link and saw +«НЕДОСТАТОЧНО ДАННЫХ» — the row was created BEFORE the estimator fix +(#oblast-E/#oblast-F, PR #2823/#2825) and is permanently stuck at +median_price=0, even though the same address/params now compute a real +price. app.api.v1.trade_in::_try_revive_dead_estimate recomputes such a row +in place (same id/link) via the same estimate_quality() path as POST +/estimate. Also covers migration 255 (relaxations/reliability persistence). + +Реальная БД не нужна: DB + get_role + estimate_quality мокируются, mirroring +test_estimate_idor.py's approach (self-contained, no cross-file fixture +imports — this repo has no precedent for importing fixtures across +tests/test_*.py modules, only from tests/support/). + +DB mock dispatches by SQL substring (not call-position): get_estimate's +existing rehydrate path calls the real (non-stubbed) _resolve_target_house_id +helper, which itself fires 1-2 incidental `SELECT id FROM houses` queries +whenever the revival attempt does NOT short-circuit with an early return — +hand-counting positional side_effect entries around that would be brittle. +""" + +from __future__ import annotations + +import inspect +import os +import sys +from datetime import UTC, datetime, timedelta +from types import SimpleNamespace +from unittest.mock import MagicMock + +# psycopg v3 driver required; stub DATABASE_URL before any app import +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") + +# WeasyPrint requires GTK — not present in CI/Windows. Stub before any app import. +_wp_mock = MagicMock() +sys.modules.setdefault("weasyprint", _wp_mock) +sys.modules.setdefault("weasyprint.CSS", _wp_mock) +sys.modules.setdefault("weasyprint.HTML", _wp_mock) + +import pytest # noqa: E402 +from fastapi import FastAPI # noqa: E402 +from fastapi.testclient import TestClient # noqa: E402 + +_ESTIMATE_ID = "22222222-2222-2222-2222-222222222222" +_TEMP_ID = "33333333-3333-3333-3333-333333333333" + + +@pytest.fixture(autouse=True) +def _restore_get_role(): + """Restore app.core.auth.get_role after each test (mirror test_estimate_idor).""" + from app.core import auth as auth_mod + + original = auth_mod.get_role + yield + auth_mod.get_role = original + + +@pytest.fixture() +def trade_in_app() -> FastAPI: + """Minimal FastAPI app mounting only the trade-in router.""" + from app.api.v1 import trade_in as trade_in_module + + application = FastAPI() + application.include_router(trade_in_module.router, prefix="/api/v1/trade-in") + return application + + +def _make_dead_row( + *, + median_price: int | None = 0, + house_type: str | None = None, + repair_state: str | None = None, + relaxations: list[str] | None = None, + reliability: str = "ok", +) -> SimpleNamespace: + """A trade_in_estimates row in the "dead" state (incident 2026-08-10). + + house_type/repair_state default to None (valid TradeInEstimateInput input — + most revival tests need a row that round-trips through pydantic cleanly); + a dedicated test passes legacy Russian literal values to exercise the + "invalid persisted value" graceful-degrade path. + """ + return SimpleNamespace( + id=_ESTIMATE_ID, + median_price=median_price, + range_low=0, + range_high=0, + median_price_per_m2=0, + confidence="low", + confidence_explanation="Рядом найдено недостаточно объявлений (4 шт.)", + n_analogs=0, + analogs=[], + actual_deals=[], + sources_used=[], + data_freshness_minutes=None, + expires_at=datetime.now(tz=UTC) + timedelta(hours=12), + retain_until=None, + address="г Екатеринбург, ул Академика Парина, д 46, к 5", + lat=56.8519, + lon=60.6122, + area_m2=23.1, + rooms=1, + floor=5, + total_floors=16, + year_built=2018, + house_type=house_type, + repair_state=repair_state, + has_balcony=None, + canonical_address=None, + house_cadnum=None, + house_fias_id=None, + dadata_qc_geo=None, + dadata_metro=[], + expected_sold_price=None, + expected_sold_range_low=None, + expected_sold_range_high=None, + expected_sold_per_m2=None, + asking_to_sold_ratio=None, + ratio_basis=None, + created_by="kopylov", + created_at=datetime(2026, 5, 29, tzinfo=UTC), + relaxations=relaxations or [], + reliability=reliability, + ) + + +def _make_live_row( + *, relaxations: list[str] | None = None, reliability: str = "ok" +) -> SimpleNamespace: + """A "live" row (median_price>0) — revival must never touch it.""" + return SimpleNamespace( + id=_ESTIMATE_ID, + median_price=4_031_157, + range_low=3_700_000, + range_high=4_300_000, + median_price_per_m2=174_000, + confidence="medium", + confidence_explanation="Найдено 39 аналогов", + n_analogs=39, + analogs=[], + actual_deals=[], + sources_used=["avito", "rosreestr"], + data_freshness_minutes=15, + expires_at=datetime.now(tz=UTC) + timedelta(hours=12), + retain_until=None, + address="г Екатеринбург, ул Академика Парина, д 46, к 5", + lat=56.8519, + lon=60.6122, + area_m2=23.1, + rooms=1, + floor=5, + total_floors=16, + year_built=2018, + house_type=None, + repair_state=None, + has_balcony=None, + canonical_address="г Екатеринбург, ул Академика Парина, д 46, к 5", + house_cadnum=None, + house_fias_id=None, + dadata_qc_geo=0, + dadata_metro=[], + expected_sold_price=None, + expected_sold_range_low=None, + expected_sold_range_high=None, + expected_sold_per_m2=None, + asking_to_sold_ratio=None, + ratio_basis=None, + created_by="kopylov", + created_at=datetime.now(tz=UTC), + relaxations=relaxations or [], + reliability=reliability, + ) + + +def _fake_revived_result(**overrides): + """A canned AggregatedEstimate mimicking a successful estimate_quality() call.""" + from app.schemas.trade_in import AggregatedEstimate + + defaults = dict( + estimate_id=_TEMP_ID, + median_price_rub=4_031_157, + range_low_rub=3_700_000, + range_high_rub=4_300_000, + median_price_per_m2=174_000, + confidence="medium", + confidence_explanation="Найдено 39 аналогов", + n_analogs=39, + period_months=12, + analogs=[], + actual_deals=[], + expires_at=datetime.now(tz=UTC) + timedelta(hours=24), + target_address="г Екатеринбург, ул Академика Парина, д 46, к 5", + target_lat=56.8519, + target_lon=60.6122, + sources_used=["avito", "rosreestr"], + data_freshness_minutes=15, + canonical_address="г Екатеринбург, ул Академика Парина, д 46, к 5", + relaxations=["снят фильтр по году постройки", "учтены студии"], + reliability="low", + created_at=datetime.now(tz=UTC), + ) + defaults.update(overrides) + return AggregatedEstimate(**defaults) + + +def _dispatch_db(row: object, claim_result: object = None) -> MagicMock: + """DB session mock dispatching fetchone() results by SQL substring. + + - initial GET SELECT ("SELECT id, median_price ...") -> row + - revival claim UPDATE ("SET revival_attempted_at") -> claim_result + - everything else (houses lookup, persist UPDATE, DELETE, + avito_imv UPDATE — none of which .fetchone() in real code + except the two above, but MagicMock tolerates the unused + call either way) -> None + + call_args_list still records every call in order regardless of dispatch, + so tests can assert on it directly (grep by substring) without needing to + hand-count incidental queries fired by _resolve_target_house_id. + """ + db = MagicMock() + + def _execute(clause, params=None, *_a, **_k): + sql = getattr(clause, "text", str(clause)) + result = MagicMock() + if "SET revival_attempted_at" in sql: + result.fetchone.return_value = claim_result + elif "SELECT id, median_price" in sql: + result.fetchone.return_value = row + else: + result.fetchone.return_value = None + return result + + db.execute.side_effect = _execute + return db + + +def _calls_containing(db: MagicMock, needle: str) -> list: + return [c for c in db.execute.call_args_list if needle in getattr(c.args[0], "text", "")] + + +def _client_with(app: FastAPI, db_mock: MagicMock, role: str = "pilot") -> TestClient: + from app.core.db import get_db + + def _override_db(): + yield db_mock + + app.dependency_overrides[get_db] = _override_db + auth_mod = sys.modules["app.core.auth"] + auth_mod.get_role = lambda _u: role # type: ignore[assignment] + return TestClient(app) + + +@pytest.fixture() +def _estimator_stub(): + """Replaces app.services.estimator with a SimpleNamespace stub. + + Mirrors test_estimate_idor.py::_stub_precision_and_pdf, plus an + `estimate_quality` async attribute (revival's own lazy import target). + Individual tests overwrite `estimate_quality` per-scenario. + """ + real_estimator = sys.modules.get("app.services.estimator") + + async def _default_estimate_quality(*_a, **_k): # pragma: no cover — overridden per test + raise AssertionError("estimate_quality stub not configured for this test") + + stub = SimpleNamespace( + _qc_geo_to_precision=lambda _qc: None, + _fetch_price_trend=lambda *a, **k: None, + _fetch_dkp_corridor=lambda *a, **k: None, + _fetch_house_imv_anchor=lambda *a, **k: None, + _resolve_target_city=lambda *a, **k: None, + _cv_from_ppm2=lambda *a, **k: None, + _source_counts=lambda *a, **k: {}, + _canonical_sources=lambda *a, **k: [], + estimate_quality=_default_estimate_quality, + ) + sys.modules["app.services.estimator"] = stub # type: ignore[assignment] + yield stub + if real_estimator is not None: + sys.modules["app.services.estimator"] = real_estimator + else: + sys.modules.pop("app.services.estimator", None) + + +# ── Revival success ─────────────────────────────────────────────────────── + + +def test_dead_row_revives_and_updates_db( + trade_in_app: FastAPI, _estimator_stub: SimpleNamespace +) -> None: + """Dead row (median_price=0) → recomputed, written to the SAME id, returned fresh. + + Success path returns early (before the pre-existing rehydrate block), so + the call count is exactly the 5 documented in _try_revive_dead_estimate's + docstring: claim, avito_imv relink, persist UPDATE, DELETE temp — plus the + initial SELECT. + """ + + async def _fake_estimate_quality(payload, db, **kwargs): + assert payload.address.startswith("г Екатеринбург") + assert payload.rooms == 1 + assert payload.radius_m is None # never persisted — default cascade + return _fake_revived_result() + + _estimator_stub.estimate_quality = _fake_estimate_quality + + row = _make_dead_row() + db = _dispatch_db(row, claim_result=SimpleNamespace(id=_ESTIMATE_ID)) + client = _client_with(trade_in_app, db) + resp = client.get( + f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}", + headers={"X-Authenticated-User": "kopylov"}, + ) + assert resp.status_code == 200 + body = resp.json() + # id/link contract: response carries the ORIGINAL id, not the temp uuid + # estimate_quality() minted internally. + assert body["estimate_id"] == _ESTIMATE_ID + assert body["median_price_rub"] == 4_031_157 + assert body["n_analogs"] == 39 + assert body["insufficient_data"] is False + assert body["reliability"] == "low" + assert "снят фильтр по году постройки" in body["relaxations"] + + assert len(db.execute.call_args_list) == 5 + persist_calls = _calls_containing(db, "UPDATE trade_in_estimates SET") + assert len(persist_calls) == 1 + persist_params = persist_calls[0].args[1] + assert persist_params["id"] == _ESTIMATE_ID + assert persist_params["median_price"] == 4_031_157 + assert persist_params["reliability"] == "low" + delete_calls = _calls_containing(db, "DELETE FROM trade_in_estimates") + assert len(delete_calls) == 1 + assert delete_calls[0].args[1]["id"] == _TEMP_ID + + +# ── Live row is never touched ──────────────────────────────────────────── + + +def test_live_row_never_triggers_revival( + trade_in_app: FastAPI, _estimator_stub: SimpleNamespace +) -> None: + """median_price>0 → revival branch skipped entirely: estimate_quality is + never called and no claim UPDATE fires — the saved price is untouched.""" + + async def _must_not_be_called(*_a, **_k): + raise AssertionError("estimate_quality must not be called for a live row") + + _estimator_stub.estimate_quality = _must_not_be_called + + row = _make_live_row() + db = _dispatch_db(row) + client = _client_with(trade_in_app, db) + resp = client.get( + f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}", + headers={"X-Authenticated-User": "kopylov"}, + ) + assert resp.status_code == 200 + body = resp.json() + assert body["median_price_rub"] == 4_031_157 + assert _calls_containing(db, "SET revival_attempted_at") == [] + + +# ── Throttle / anti-storm ──────────────────────────────────────────────── + + +def test_dead_row_throttled_does_not_recompute( + trade_in_app: FastAPI, _estimator_stub: SimpleNamespace +) -> None: + """Claim UPDATE returns no row (recent attempt / lost race) → no recompute, + honest still-dead response, no 500, exactly one claim attempt (no retry + loop within the same request).""" + + async def _must_not_be_called(*_a, **_k): + raise AssertionError("estimate_quality must not be called when throttled") + + _estimator_stub.estimate_quality = _must_not_be_called + + row = _make_dead_row() + db = _dispatch_db(row, claim_result=None) # throttled: WHERE matched nothing + client = _client_with(trade_in_app, db) + resp = client.get( + f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}", + headers={"X-Authenticated-User": "kopylov"}, + ) + assert resp.status_code == 200 + body = resp.json() + assert body["insufficient_data"] is True + assert body["median_price_rub"] == 0 + assert len(_calls_containing(db, "SET revival_attempted_at")) == 1 + + +# ── Recompute error degrades gracefully (no 500) ───────────────────────── + + +def test_revival_recompute_exception_falls_back_without_500( + trade_in_app: FastAPI, _estimator_stub: SimpleNamespace +) -> None: + async def _raises(*_a, **_k): + raise RuntimeError("geocode timeout") + + _estimator_stub.estimate_quality = _raises + + row = _make_dead_row() + db = _dispatch_db(row, claim_result=SimpleNamespace(id=_ESTIMATE_ID)) + client = _client_with(trade_in_app, db) + resp = client.get( + f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}", + headers={"X-Authenticated-User": "kopylov"}, + ) + assert resp.status_code == 200 + assert resp.json()["insufficient_data"] is True + # exception happened AFTER the claim — throttle was still recorded once. + assert len(_calls_containing(db, "SET revival_attempted_at")) == 1 + # ...but nothing was written back to the row (no persist UPDATE fired). + assert _calls_containing(db, "UPDATE trade_in_estimates SET") == [] + + +def test_revival_still_zero_falls_back_without_500( + trade_in_app: FastAPI, _estimator_stub: SimpleNamespace +) -> None: + """Recompute runs but still finds nothing (median_price_rub=0) — honest + insufficient_data, temp throwaway row cleaned up, no crash.""" + + async def _still_empty(*_a, **_k): + return _fake_revived_result( + median_price_rub=0, + range_low_rub=0, + range_high_rub=0, + median_price_per_m2=0, + n_analogs=0, + confidence="low", + ) + + _estimator_stub.estimate_quality = _still_empty + + row = _make_dead_row() + db = _dispatch_db(row, claim_result=SimpleNamespace(id=_ESTIMATE_ID)) + client = _client_with(trade_in_app, db) + resp = client.get( + f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}", + headers={"X-Authenticated-User": "kopylov"}, + ) + assert resp.status_code == 200 + assert resp.json()["insufficient_data"] is True + delete_calls = _calls_containing(db, "DELETE FROM trade_in_estimates") + assert len(delete_calls) == 1 + assert delete_calls[0].args[1]["id"] == _TEMP_ID + # the still-dead ORIGINAL row was never overwritten with (fresh) zeros. + assert _calls_containing(db, "UPDATE trade_in_estimates SET") == [] + + +def test_revival_invalid_persisted_house_type_falls_back_gracefully( + trade_in_app: FastAPI, _estimator_stub: SimpleNamespace +) -> None: + """Legacy row with a house_type outside the current Literal set — payload + reconstruction itself raises (pydantic ValidationError), caught the same + way as any other recompute failure. No 500.""" + + async def _must_not_be_called(*_a, **_k): + raise AssertionError("estimate_quality must not be reached — payload build fails first") + + _estimator_stub.estimate_quality = _must_not_be_called + + row = _make_dead_row(house_type="монолит", repair_state="хороший") + db = _dispatch_db(row, claim_result=SimpleNamespace(id=_ESTIMATE_ID)) + client = _client_with(trade_in_app, db) + resp = client.get( + f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}", + headers={"X-Authenticated-User": "kopylov"}, + ) + assert resp.status_code == 200 + assert resp.json()["insufficient_data"] is True + + +# ── relaxations/reliability round-trip (migration 255) ─────────────────── + + +def test_relaxations_reliability_roundtrip_on_get( + trade_in_app: FastAPI, _estimator_stub: SimpleNamespace +) -> None: + """A live row with persisted relaxations/reliability surfaces them + byte-for-byte on GET — the red "точность снижена" banner survives + reopening a saved link (previously always reset to ok/[]).""" + row = _make_live_row( + relaxations=["радиус расширен до 2000 м", "площадь ±25%"], reliability="low" + ) + db = _dispatch_db(row) + client = _client_with(trade_in_app, db) + resp = client.get( + f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}", + headers={"X-Authenticated-User": "kopylov"}, + ) + assert resp.status_code == 200 + body = resp.json() + assert body["reliability"] == "low" + assert body["relaxations"] == ["радиус расширен до 2000 м", "площадь ±25%"] + + +def test_get_estimate_defaults_relaxations_reliability_when_row_lacks_columns( + trade_in_app: FastAPI, _estimator_stub: SimpleNamespace +) -> None: + """Defensive getattr fallback: a row/mock without relaxations/reliability + attrs (e.g. a stale test double) degrades to schema defaults, not a crash.""" + row = _make_live_row() + del row.relaxations + del row.reliability + db = _dispatch_db(row) + client = _client_with(trade_in_app, db) + resp = client.get( + f"/api/v1/trade-in/estimate/{_ESTIMATE_ID}", + headers={"X-Authenticated-User": "kopylov"}, + ) + assert resp.status_code == 200 + body = resp.json() + assert body["reliability"] == "ok" + assert body["relaxations"] == [] + + +# ── estimator.py persists relaxations/reliability on (re)compute ───────── + + +def test_estimate_quality_insert_persists_relaxations_reliability() -> None: + """Source guard: the main POST-path INSERT must write relaxations/ + reliability, not just return them in the response — regression guard + against the exact gap this migration closes (PR #2823 open follow-up).""" + from app.services import estimator + + src = inspect.getsource(estimator.estimate_quality) + assert "relaxations_json" in src + assert '"reliability": reliability' in src + + +def test_empty_estimate_persists_relaxations_reliability() -> None: + """_empty_estimate's INSERT must also set reliability='very_low' (mirrors + the Python object it returns) rather than silently defaulting to 'ok'.""" + from app.services import estimator + + src = inspect.getsource(estimator._empty_estimate) + assert "relaxations, reliability" in src + assert "'very_low'" in src diff --git a/tradein-mvp/frontend/src/components/trade-in/v2/ParamsPanel.tsx b/tradein-mvp/frontend/src/components/trade-in/v2/ParamsPanel.tsx index f6196e41..3a1fa2d2 100644 --- a/tradein-mvp/frontend/src/components/trade-in/v2/ParamsPanel.tsx +++ b/tradein-mvp/frontend/src/components/trade-in/v2/ParamsPanel.tsx @@ -629,7 +629,30 @@ function initCityConfirmed(address: string | undefined): boolean { // РАДИУС options. "Авто" (default) sends no radius_m → the backend keeps its // two-tier default (1000 m primary / 2000 m fallback). A fixed value overrides // both ("ищем строго в пределах X м"). Design dropdown was values-only. -const RADIUS_OPTIONS = ["Авто", "300 м", "500 м", "1000 м", "2000 м"]; +// 3000/5000 added (support ticket "выбран мах радиус 2 000") — the dropdown +// used to cap at 2000 m while the backend accepts up to 5000 +// (schemas/trade_in.py Field(ge=100, le=5000)), so a user in a sparse district +// picking "2000 м" thinking it was the widest option actually CUT OFF the +// cascade's own 3/5 km fallback steps (contract #2044 — an explicit radius +// pick must never be silently widened past itself). See radiusHint below for +// the disclosure that "Авто" is the option that widens automatically. +const RADIUS_OPTIONS = [ + "Авто", + "300 м", + "500 м", + "1000 м", + "2000 м", + "3000 м", + "5000 м", +]; + +// #support — "Авто" delegates radius selection to the backend's own two-tier +// (and, since #2044, cascade) widening; any explicit pick is a hard ceiling +// the cascade may not cross. Same zero-layout-cost title= pattern as +// ResultPanel's confidence tooltip / HeroBar's location-index disclaimer — +// a native hover tooltip, no modal, no panel re-layout. +const radiusHint = + "Авто — расширяем радиус сами, если аналогов мало. Явный радиус ограничивает поиск."; // "Авто" sends no radius_m → the backend applies its two-tier default (1000 m // primary / 2000 m fallback, see RADIUS_OPTIONS comment above). The map circle @@ -1408,6 +1431,7 @@ export default function ParamsPanel({ : undefined } aria-label="Радиус анализа" + title={radiusHint} style={{ display: "flex", alignItems: "center",