diff --git a/tradein-mvp/backend/app/services/estimator.py b/tradein-mvp/backend/app/services/estimator.py index c8d0fa46..d885c882 100644 --- a/tradein-mvp/backend/app/services/estimator.py +++ b/tradein-mvp/backend/app/services/estimator.py @@ -827,10 +827,15 @@ def _save_yandex_history_items( (address|publish_date|area|floor|prices) hash. Batch semantics: single try/except; on any failure the batch rolls back. - """ - if not result.history_items: - return 0 + #2674 (ревью): резолв дома и запись houses.has_panorama идут ДО раннего возврата по + пустой истории. Раньше возврат стоял первым, и страница, отрисованная идеально, но + без единого объявления в истории, до записи панорамы не доходила — на проде это + 1519 оценок против 1360 домов с историей, ~10% страниц молча пропускались. Цена + переноса: match_or_create_house теперь вызывается и для таких страниц (может + СОЗДАТЬ дом). Это тот же вызов, с тем же адресом, что уже отрабатывает на + остальных 90% — новых сущностей класс не появляется, появляется недостающая доля. + """ # Resolve house ONCE per page. Synthetic ext_id = sha256(address)[:16] # — stable across re-runs, distinguishes pages for different addresses. address_seed = (result.address or "").strip().lower() @@ -866,6 +871,12 @@ def _save_yandex_history_items( result.address, ) + # Наблюдение о доме не зависит от того, есть ли на странице история объявлений. + _save_yandex_house_panorama(db, house_id, result.house) + + if not result.history_items: + return 0 + rows = [] skipped_area = 0 for item in result.history_items: @@ -936,7 +947,6 @@ def _save_yandex_history_items( if rows: db.execute(sql, rows) db.commit() - _save_yandex_house_panorama(db, house_id, result.house) return len(rows) except Exception as e: logger.warning( diff --git a/tradein-mvp/backend/data/sql/216_dead_code_sweep.sql b/tradein-mvp/backend/data/sql/216_dead_code_sweep.sql index c735a5e8..4bacb319 100644 --- a/tradein-mvp/backend/data/sql/216_dead_code_sweep.sql +++ b/tradein-mvp/backend/data/sql/216_dead_code_sweep.sql @@ -6,7 +6,10 @@ -- 2. МЁРТВОЕ — механизм невыразим, дублирует существующее или потерял смысл. Удаляем. -- 3. ЗАДЕЛ — оставляем, но в схеме должно быть написано, чем он НЕ является сегодня. -- --- Все числа — с прод-БД tradein 2026-08-06. +-- Все числа — с прод-БД tradein 2026-08-06, ТОЧНЫМ count(*). Первая редакция несла +-- сюда reltuples-оценки планировщика (listings «142 569» против реальных 93 408 — +-- раздув мёртвыми кортежами на 53%); в постоянном комментарии к схеме оценке не место, +-- тем более в PR, тезис которого — «каждое утверждение несёт число с прода». -- -- ── 1. ПОДКЛЮЧАЕМ ─────────────────────────────────────────────────────────── -- @@ -39,20 +42,20 @@ -- получить оценку по устаревшему рынку. Методика не потеряна: derivation-CTE -- целиком сохранён в 098, восстановить = переприменить файл. -- --- D. listings.merged_into — 142 569 строк, NULL у всех, ноль упоминаний в коде. +-- D. listings.merged_into — 93 408 строк, NULL у всех, ноль упоминаний в коде. -- Заведена в 028 «under dedup workflow», который так и не построили; 113 уже -- писала прямым текстом «column is dead, no code writer». Дедуп объявлений живёт -- в другом месте и по-другому (estimator._union_find_phys_dedup, во время оценки, -- без записи в БД). Соседнюю listings.canonical НЕ трогаем — она вырождена (t у --- всех 142 569), но её читает WHERE listings_search_mv (050/094), и снос колонки +-- всех 93 408), но её читает WHERE listings_search_mv (050/094), и снос колонки -- потянул бы пересоздание matview с шестью индексами ради нулевого выигрыша. -- --- E. house_sources.raw_payload + GIN-индекс по нему — 46 813 строк, NULL у всех. +-- E. house_sources.raw_payload + GIN-индекс по нему — 49 502 строки, NULL у всех. -- Оба писателя house_sources (matching/houses.py:556, house_dedup_merge.py:493) -- эту колонку в INSERT не включают; читателей нет, из публичного контракта -- market.v_house_sources (154) она намеренно исключена. GIN-индекс по колонке, -- которая всегда NULL, — чистая стоимость на каждой вставке. --- Соседний house_sources.ext_url тоже пуст 46 813/46 813, но он ВХОДИТ в +-- Соседний house_sources.ext_url тоже пуст 49 502/49 502, но он ВХОДИТ в -- market.v_house_sources — удаление сломало бы обещание стабильности контракта. -- Оставляем и подписываем (см. п. 3). -- @@ -97,9 +100,19 @@ BEGIN; -- interval_days = 7: реестр капремонта не меняется ежедневно, а прогон качает два -- zip и парсит ~30 тыс. строк. Недельный такт достаточен и не жжёт трафик впустую. -- Ключ читает compute_next_run_at из default_params (см. 129). --- Окно 03:00-04:00 UTC — до rosreestr_dkp_import (04:00-06:00) и до --- asking_to_sold_ratio_refresh (06:00-07:00): год постройки должен доехать в --- listings раньше, чем по ним считают дневные агрегаты. +-- Окно 01:00-02:00 UTC. Первая редакция ставила 03:00-04:00 — ровно туда, где сидит +-- refresh_search_matview (сверено с прод-таблицей scrape_schedules), то есть именно +-- то задание, которое и переносит year_built в поиск. Планировщик берёт случайный +-- момент внутри окна и гоняет источники ПАРАЛЛЕЛЬНО, порядок он не гарантирует +-- ничем — совпадение окон превращало «сначала загрузка, потом обновление поиска» +-- в подбрасывание монеты. Час до 02:00 разводит их при типовой длительности прогона +-- и остаётся раньше rosreestr_dkp_import (04:00-06:00) и +-- asking_to_sold_ratio_refresh (06:00-07:00). +-- ЧЕСТНАЯ ОГОВОРКА: гарантии всё равно нет — при аномально долгом прогоне (сеть +-- ДОМ.РФ, ретраи) свежий year_built доедет до поиска на цикл позже. Ни блокировок, +-- ни потери данных: следующее обновление matview его подхватит. +-- Соседи в 01:00-02:00 — listing_source_snapshot и avito_city_sweep_kamensk_uralskiy; +-- общих ресурсов нет (ДОМ.РФ ходит своим httpx, мимо прокси-пула). INSERT INTO scrape_schedules ( source, enabled, @@ -112,9 +125,9 @@ VALUES ( 'domrf_kapremont_load', true, - 3, - 4, - ((CURRENT_DATE + INTERVAL '1 day') + make_interval(hours => 3)) AT TIME ZONE 'UTC', + 1, + 2, + ((CURRENT_DATE + INTERVAL '1 day') + make_interval(hours => 1)) AT TIME ZONE 'UTC', '{"interval_days": 7}'::jsonb ) ON CONFLICT (source) DO NOTHING; @@ -209,7 +222,7 @@ COMMENT ON VIEW v_cross_source_health IS 'источников не подключено. Читателей в коде нет.'; COMMENT ON COLUMN house_sources.ext_url IS - '#2674: NULL у всех 46 813 строк — ни один из двух писателей house_sources ' + '#2674: NULL у всех 49 502 строк — ни один из двух писателей house_sources ' '(matching/houses.py, house_dedup_merge.py) эту колонку не заполняет. НЕ удалена ' 'только потому, что входит в публичный контракт market.v_house_sources (мигр. 154), ' 'где удаление колонки объявлено ломающим изменением. Соседний raw_payload из ' diff --git a/tradein-mvp/backend/tests/test_dead_code_sweep_2674.py b/tradein-mvp/backend/tests/test_dead_code_sweep_2674.py index 2fda663c..bdb6f976 100644 --- a/tradein-mvp/backend/tests/test_dead_code_sweep_2674.py +++ b/tradein-mvp/backend/tests/test_dead_code_sweep_2674.py @@ -110,6 +110,28 @@ def test_has_panorama_false_written_when_page_rendered() -> None: assert _panorama_updates(db) == [{"hid": 7, "panorama": False}] +def test_has_panorama_written_when_page_has_no_history() -> None: + """Отрисованная страница БЕЗ истории объявлений — ~10% случаев на проде. + + Ревью #2689: вызов стоял после раннего возврата по пустой истории, поэтому такие + страницы молча пропускались (1519 оценок против 1360 домов с историей). Наблюдение + о доме к наличию объявлений отношения не имеет. + """ + db = MagicMock() + result = _result_with_meta( + ValuationHouseMeta(year_built=2015, total_floors=25, has_panorama=True) + ) + result.history_items = [] + + with patch( + "app.services.estimator.match_or_create_house", + return_value=(42, 0.9, "fp"), + ): + assert _save_yandex_history_items(db, result) == 0 + + assert _panorama_updates(db) == [{"hid": 42, "panorama": True}] + + def test_has_panorama_not_written_when_page_unconfirmed() -> None: """Пустая мета (капча/редизайн) → NULL, а не сфабрикованный false.""" db = MagicMock() @@ -160,6 +182,26 @@ def test_domrf_loader_is_seeded_into_schedules() -> None: assert '"interval_days": 7' in sql +def test_domrf_window_does_not_collide_with_matview_refresh() -> None: + """Окно ДОМ.РФ не должно совпадать с refresh_search_matview (03:00-04:00 UTC). + + Ревью #2689: планировщик берёт случайный момент внутри окна и гоняет источники + параллельно — общее окно с тем заданием, которое переносит year_built в поиск, + это подбрасывание монеты. Тест ловит откат окна обратно на 3. + """ + sql = MIGRATION.read_text(encoding="utf-8") + values = sql.split("'domrf_kapremont_load',", 1)[1].split(")", 1)[0] + tokens = [t.strip().rstrip(",") for t in values.splitlines()] + hours = [int(t) for t in tokens if t.isdigit()] + assert hours, "не нашли window_start_hour/window_end_hour в INSERT" + start, end = hours[0], hours[1] + matview_start, matview_end = 3, 4 # прод-значение scrape_schedules на 2026-08-06 + assert end <= matview_start or start >= matview_end, ( + f"окно {start}-{end} пересекается с refresh_search_matview " + f"{matview_start}-{matview_end}" + ) + + def test_domrf_handler_reuses_loader_functions() -> None: """Дизайн-инвариант product_handlers: job переиспользует боевое тело, не копирует.""" src = (TRADEIN / "backend" / "app" / "services" / "product_handlers.py").read_text( diff --git a/tradein-mvp/backend/tests/test_estimator_yandex_integration.py b/tradein-mvp/backend/tests/test_estimator_yandex_integration.py index cff1264d..b5d6ebbe 100644 --- a/tradein-mvp/backend/tests/test_estimator_yandex_integration.py +++ b/tradein-mvp/backend/tests/test_estimator_yandex_integration.py @@ -24,6 +24,14 @@ from app.services.estimator import ( from app.services.scraper_settings import get_scraper_delay +def _history_rows(db) -> list[dict]: + """Строки батча house_placement_history из мока сессии (фильтр по SQL, не по позиции).""" + for call in db.execute.call_args_list: + if "INSERT INTO house_placement_history" in str(call.args[0]): + return call.args[1] + return [] + + def _sample_result(address: str = "Екатеринбург, ул. Учителей, 18") -> YandexValuationResult: return YandexValuationResult( address=address, @@ -172,17 +180,20 @@ def test_save_history_items_inserts_each(): assert saved == 2 # 1 batch INSERT (executemany). #2674 добавил вторым вызовом UPDATE # houses.has_panorama — считаем именно вставки истории, а не все execute. - inserts = [ - c - for c in db.execute.call_args_list - if "INSERT INTO house_placement_history" in str(c.args[0]) - ] - assert len(inserts) == 1 - rows = inserts[0].args[1] + rows = _history_rows(db) assert isinstance(rows, list) and len(rows) == 2 + # Два коммита: панорама (до истории) + батч истории. Раньше был один. + assert db.commit.call_count == 2 def test_save_history_items_empty_no_commit(): + """Пустая история + НЕподтверждённая страница → дом резолвится, но не пишется ничего. + + #2674 (ревью): ранний возврат по пустой истории раньше стоял ПЕРВЫМ и заодно + отрезал запись houses.has_panorama для отрисованных страниц без объявлений (~10%). + Теперь резолв дома идёт до возврата, поэтому match_or_create_house вызывается — + а вот записей по-прежнему ноль: мета пустая, гейт панорамы не пропускает. + """ db = MagicMock() result = YandexValuationResult( address="x", @@ -193,11 +204,13 @@ def test_save_history_items_empty_no_commit(): house=ValuationHouseMeta(), history_items=[], ) - # match_or_create_house must NOT be called when there are no items (early return) - with patch("app.services.estimator.match_or_create_house") as m: + with patch( + "app.services.estimator.match_or_create_house", + return_value=(1, 0.9, "fingerprint"), + ) as m: saved = _save_yandex_history_items(db, result) assert saved == 0 - m.assert_not_called() + m.assert_called_once() db.execute.assert_not_called() db.commit.assert_not_called() @@ -231,9 +244,20 @@ def test_save_history_items_ext_id_stable_across_calls(): def test_save_history_items_db_error_rolls_back_batch(): - """Any item failing rolls back the whole batch — batch semantics (finding #5).""" + """Any item failing rolls back the whole batch — batch semantics (finding #5). + + #2674: side_effect адресуем по SQL, а не по позиции вызова — иначе исключение + доставалось бы UPDATE houses.has_panorama (он идёт первым и свои ошибки глотает), + а батч истории проходил бы успешно, и тест молча проверял бы не тот путь. + """ db = MagicMock() - db.execute.side_effect = [RuntimeError("first row fails"), None] + + def _fail_history(sql, *args, **kwargs): + if "INSERT INTO house_placement_history" in str(sql): + raise RuntimeError("first row fails") + return MagicMock() + + db.execute.side_effect = _fail_history result = _sample_result() with patch( "app.services.estimator.match_or_create_house", @@ -242,4 +266,5 @@ def test_save_history_items_db_error_rolls_back_batch(): saved = _save_yandex_history_items(db, result) assert saved == 0 # whole batch rolled back db.rollback.assert_called_once() - db.commit.assert_not_called() + # Панорама коммитится отдельно и раньше — её успех не отменяет отката истории. + assert db.commit.call_count == 1 diff --git a/tradein-mvp/backend/tests/test_yandex_history_area_filter.py b/tradein-mvp/backend/tests/test_yandex_history_area_filter.py index 481d6610..44d6ac26 100644 --- a/tradein-mvp/backend/tests/test_yandex_history_area_filter.py +++ b/tradein-mvp/backend/tests/test_yandex_history_area_filter.py @@ -28,6 +28,19 @@ from scraper_kit.providers.yandex.valuation import ( from app.services.estimator import _save_yandex_history_items +def _history_rows(db) -> list[dict]: + """Строки батча house_placement_history из мока сессии. + + #2674: раньше тесты брали `db.execute.call_args_list[0]` — позиционно. Позиция + сломалась, как только у функции появился второй execute (UPDATE houses.has_panorama + перед вставкой истории). Фильтруем по SQL: тест переживёт любой новый вызов. + """ + for call in db.execute.call_args_list: + if "INSERT INTO house_placement_history" in str(call.args[0]): + return call.args[1] + return [] + + def _make_result(items: list[ValuationHistoryItem]) -> YandexValuationResult: return YandexValuationResult( address="Россия, Свердловская область, Екатеринбург, ул. Куйбышева, 106", @@ -84,7 +97,7 @@ def test_item_with_area_none_is_skipped() -> None: saved = _save_yandex_history_items(db, result) assert saved == 1, f"Ожидали 1 сохранённый item, получили {saved}" - rows = db.execute.call_args_list[0].args[1] + rows = _history_rows(db) assert len(rows) == 1 assert rows[0]["area"] == 50.0 @@ -109,7 +122,7 @@ def test_item_with_area_zero_is_skipped() -> None: saved = _save_yandex_history_items(db, result) assert saved == 1 - rows = db.execute.call_args_list[0].args[1] + rows = _history_rows(db) assert rows[0]["area"] == 55.0 @@ -155,12 +168,7 @@ def test_all_invalid_area_returns_zero_no_crash() -> None: assert saved == 0 # db.execute не должен вызываться для пустого rows (нет INSERT) - inserts = [ - c - for c in db.execute.call_args_list - if "INSERT INTO house_placement_history" in str(c.args[0]) - ] - assert inserts == [] + assert _history_rows(db) == [] # Commit вызывается, rollback — нет. Два коммита: пустой батч истории + запись # houses.has_panorama (#2674) — наблюдение о доме не зависит от того, отфильтровалась # ли история по площади. @@ -208,7 +216,7 @@ def test_mixed_items_only_valid_saved() -> None: saved = _save_yandex_history_items(db, result) assert saved == 2 - rows = db.execute.call_args_list[0].args[1] + rows = _history_rows(db) assert len(rows) == 2 areas = {r["area"] for r in rows} assert areas == {40.0, 60.0} diff --git a/tradein-mvp/backend/tests/test_yandex_valuation_save.py b/tradein-mvp/backend/tests/test_yandex_valuation_save.py index 20386d31..308d4cb1 100644 --- a/tradein-mvp/backend/tests/test_yandex_valuation_save.py +++ b/tradein-mvp/backend/tests/test_yandex_valuation_save.py @@ -26,6 +26,19 @@ from scraper_kit.providers.yandex.valuation import ( from app.services.estimator import _save_yandex_history_items +def _history_rows(db) -> list[dict]: + """Строки батча house_placement_history из мока сессии. + + #2674: раньше тесты брали `db.execute.call_args_list[0]` — позиционно. Позиция + сломалась, как только у функции появился второй execute (UPDATE houses.has_panorama + перед вставкой истории). Фильтруем по SQL: тест переживёт любой новый вызов. + """ + for call in db.execute.call_args_list: + if "INSERT INTO house_placement_history" in str(call.args[0]): + return call.args[1] + return [] + + def _make_result(items: list[ValuationHistoryItem]) -> YandexValuationResult: return YandexValuationResult( address="Россия, Свердловская область, Екатеринбург, улица Куйбышева, 106", @@ -91,15 +104,7 @@ def test_save_row_contains_house_id_and_confidence(): _save_yandex_history_items(db, result) # История — один execute со list-of-dicts (executemany, один round-trip). - # #2674 добавил отдельный UPDATE houses.has_panorama — фильтруем по SQL, а не - # по порядковому номеру вызова. - inserts = [ - c - for c in db.execute.call_args_list - if "INSERT INTO house_placement_history" in str(c.args[0]) - ] - assert len(inserts) == 1 - rows = inserts[0].args[1] + rows = _history_rows(db) assert isinstance(rows, list) and len(rows) == 2 for row in rows: assert row["house_id"] == 54321 @@ -132,7 +137,7 @@ def test_save_row_contains_removed_date(): _save_yandex_history_items(db, result) # args[1] is now the list-of-dicts passed to executemany - rows = db.execute.call_args_list[0].args[1] + rows = _history_rows(db) assert rows[0]["removed_date"] == date(2024, 5, 20) @@ -159,7 +164,7 @@ def test_save_row_removed_date_none_when_active(): ): _save_yandex_history_items(db, result) - rows = db.execute.call_args_list[0].args[1] + rows = _history_rows(db) assert rows[0]["removed_date"] is None @@ -187,7 +192,7 @@ def test_save_handles_match_failure_gracefully(): saved = _save_yandex_history_items(db, result) assert saved == 1 - rows = db.execute.call_args_list[0].args[1] + rows = _history_rows(db) assert rows[0]["house_id"] is None assert rows[0]["confidence"] == pytest.approx(0.0) assert rows[0]["notes"] is None