diff --git a/tradein-mvp/backend/scripts/backfill_houses_dadata.py b/tradein-mvp/backend/scripts/backfill_houses_dadata.py index c3fe8038..ec7dd2be 100644 --- a/tradein-mvp/backend/scripts/backfill_houses_dadata.py +++ b/tradein-mvp/backend/scripts/backfill_houses_dadata.py @@ -19,6 +19,14 @@ Priority order (--priority both, default): `--priority coords` — только #1, `--priority cadnum` — только #2. +Estimate-relevance ordering (accuracy-roadmap #2): + Внутри каждого priority bucket'а дома с active listings + (`EXISTS listings WHERE house_id_fk = houses.id AND is_active`) сортируются + ПЕРВЫМИ (`ORDER BY has_active_listings DESC, id`). Так дневной 100/day budget + сначала закрывает дома, которые реально появляются в оценках → estimator #6 + `cadastr_exact` tier «загорается» на real estimate targets за дни, а не недели. + `--priority listings` — focused sweep: только NULL cadnum + есть active listing. + Per-row outcome rules (после `clean_address` ответа): - result.qc_geo IN (0, 1) AND result.lat IS NOT NULL → точное / уличное совпадение, UPDATE houses SET lat=, lon=, cadastral_number=, house_fias_id=, @@ -45,7 +53,7 @@ Flags: --limit N max rows to process (default 100) --batch LABEL log label (default `dadata_YYYY-MM-DD`) --dry-run log what would happen, no DB writes - --priority X coords | cadnum | both (default both) + --priority X coords | cadnum | both (default both) | listings """ from __future__ import annotations @@ -143,6 +151,17 @@ class Stats: # Source-row query — приоритет: NULL coords → NULL cadnum # --------------------------------------------------------------------------- +# accuracy-roadmap #2: дом «участвует в оценках» iff у него есть active listing +# (`listings.house_id_fk → houses.id`, `listings.is_active`). Такие дома — +# реальные target'ы estimator'а (#6 `cadastr_exact` tier), поэтому их кадастры +# нужны раньше всего. Используем как ORDER BY-ключ (DESC → впереди) + как фильтр +# для `--priority listings`. Correlated EXISTS бьёт по partial-индексу +# `listings_house_id_fk_idx (house_id_fk) WHERE house_id_fk IS NOT NULL`. +_HAS_ACTIVE_LISTINGS_EXPR = ( + "EXISTS (SELECT 1 FROM listings l " + "WHERE l.house_id_fk = houses.id AND l.is_active)" +) + def _select_candidates( db: Session, *, priority: str, limit: int @@ -150,9 +169,16 @@ def _select_candidates( """Возвращает список (HouseRow, priority_bucket) для enrichment. priority: - - 'coords' → только rows с NULL coords (max бизнес-value) - - 'cadnum' → только rows с NOT NULL coords + NULL cadnum - - 'both' → coords first, потом cadnum (FIFO в пределах buckets) + - 'coords' → только rows с NULL coords (max бизнес-value) + - 'cadnum' → только rows с NOT NULL coords + NULL cadnum + - 'both' → coords first, потом cadnum (FIFO в пределах buckets) + - 'listings' → только NULL cadnum + есть active listing (focused sweep + для #6 same-building матчинга — дома, которые реально в оценках) + + Внутри каждого bucket'а строки с active listings (`has_active_listings`) + идут ПЕРВЫМИ (`ORDER BY has_active_listings DESC, id`), чтобы дневной + 100/day budget сначала закрывал estimate-relevant дома, а уже потом + длинный хвост. Это и есть accuracy-roadmap #2. Фильтр `dadata_enriched_at IS NULL` — resume-safe (не повторяем уже обработанные rows). После full enrichment колонка станет NOT NULL у всех @@ -172,12 +198,13 @@ def _select_candidates( "SELECT id, " " COALESCE(NULLIF(trim(address), ''), NULLIF(trim(full_address), '')) " " AS address, " - " lat, lon, cadastral_number " + " lat, lon, cadastral_number, " + f" {_HAS_ACTIVE_LISTINGS_EXPR} AS has_active_listings " "FROM houses " f"WHERE {where} " " AND dadata_enriched_at IS NULL " f" AND {address_filter} " - "ORDER BY id " + "ORDER BY has_active_listings DESC, id " "LIMIT CAST(:lim AS int)" ) rows = db.execute(sql, {"lim": lim}).mappings().all() @@ -195,6 +222,19 @@ def _select_candidates( for r in rows ] + if priority == "listings": + # Focused sweep: только дома с active listings + NULL cadnum. + # has_active_listings гарантированно TRUE здесь, но ORDER BY оставляем + # консистентным (DESC, id) — деградирует до ORDER BY id. + out.extend( + _fetch( + f"(cadastral_number IS NULL AND {_HAS_ACTIVE_LISTINGS_EXPR})", + "listings", + remaining, + ) + ) + return out[:limit] + if priority in ("coords", "both"): out.extend(_fetch("(lat IS NULL OR lon IS NULL)", "coords", remaining)) remaining = limit - len(out) @@ -490,12 +530,14 @@ def _parse_args(argv: list[str] | None = None) -> argparse.Namespace: ) p.add_argument( "--priority", - choices=("coords", "cadnum", "both"), + choices=("coords", "cadnum", "both", "listings"), default="both", help=( "Какой набор кандидатов обрабатывать. coords = только NULL coords, " "cadnum = NOT NULL coords + NULL cadnum, both (default) = первый " - "приоритет coords." + "приоритет coords, listings = только NULL cadnum + есть active " + "listing (focused sweep для #6 same-building матчинга). Во всех " + "режимах дома с active listings сортируются вперёд." ), ) return p.parse_args(argv) diff --git a/tradein-mvp/backend/tests/scripts/test_backfill_houses_dadata.py b/tradein-mvp/backend/tests/scripts/test_backfill_houses_dadata.py index 87aebe46..ea4711bf 100644 --- a/tradein-mvp/backend/tests/scripts/test_backfill_houses_dadata.py +++ b/tradein-mvp/backend/tests/scripts/test_backfill_houses_dadata.py @@ -31,6 +31,7 @@ from scripts.backfill_houses_dadata import ( HouseRow, Stats, _is_enriched_result, + _parse_args, _result_in_ekb, _run_backfill, _select_candidates, @@ -75,15 +76,24 @@ def _make_dadata_result( def _make_db_mock( coords_sample: list[dict] | None = None, cadnum_sample: list[dict] | None = None, + listings_sample: list[dict] | None = None, ) -> tuple[MagicMock, list[dict]]: """MagicMock Session что: - - возвращает coords_sample для SELECT с `lat IS NULL OR lon IS NULL` - - возвращает cadnum_sample для SELECT с `cadastral_number IS NULL` + - возвращает coords_sample для coords-priority SELECT (`lat IS NULL OR lon IS NULL`) + - возвращает listings_sample для listings-priority SELECT + (WHERE содержит `cadastral_number IS NULL AND EXISTS ... listings`) + - возвращает cadnum_sample для cadnum-priority SELECT (`cadastral_number IS NULL`) - записывает UPDATE houses в `updated` список (полные binds dict'ы) - корректно работает с `db.begin_nested()` контекст-менеджером + + Порядок проверок важен: все три SELECT'а теперь содержат + `EXISTS ... listings` в projection (`AS has_active_listings`), поэтому + listings-priority детектим по EXISTS *в WHERE* (`cadastral_number IS NULL + AND EXISTS`) ДО общего `cadastral_number IS NULL` бранча. """ coords_sample = coords_sample or [] cadnum_sample = cadnum_sample or [] + listings_sample = listings_sample or [] updated: list[dict] = [] db = MagicMock() @@ -96,6 +106,9 @@ def _make_db_mock( if "FROM houses" in sql_str and "lat IS NULL OR lon IS NULL" in sql_str: lim = params.get("lim", len(coords_sample)) if params else len(coords_sample) result.mappings.return_value.all.return_value = coords_sample[:lim] + elif "FROM houses" in sql_str and "cadastral_number IS NULL AND EXISTS" in sql_str: + lim = params.get("lim", len(listings_sample)) if params else len(listings_sample) + result.mappings.return_value.all.return_value = listings_sample[:lim] elif "FROM houses" in sql_str and "cadastral_number IS NULL" in sql_str: lim = params.get("lim", len(cadnum_sample)) if params else len(cadnum_sample) result.mappings.return_value.all.return_value = cadnum_sample[:lim] @@ -477,6 +490,136 @@ def test_select_candidates_both_priority_fetches_coords_first_then_cadnum(): assert out[1][0].id == 100 +# --------------------------------------------------------------------------- +# accuracy-roadmap #2 — active-listings prioritization (EXISTS + ORDER BY) +# --------------------------------------------------------------------------- + + +def _capture_select_sql(db: MagicMock) -> list[str]: + """Извлекает SQL-строки всех SELECT FROM houses из db.execute.call_args_list.""" + out: list[str] = [] + for call in db.execute.call_args_list: + sql_str = str(call.args[0]) + if "FROM houses" in sql_str and "SELECT" in sql_str.upper(): + out.append(sql_str) + return out + + +def test_select_candidates_coords_sql_has_active_listings_exists_and_order(): + """coords-priority SELECT включает has_active_listings EXISTS + ORDER BY DESC.""" + coords_rows = [ + {"id": 1, "address": "ул A 1", "lat": None, "lon": None, "cadastral_number": None}, + ] + db, _updated = _make_db_mock(coords_sample=coords_rows) + + _select_candidates(db, priority="coords", limit=10) + + sqls = _capture_select_sql(db) + assert len(sqls) == 1 + sql = sqls[0] + # EXISTS на listings.house_id_fk → houses.id + is_active, aliased в projection + assert "EXISTS (SELECT 1 FROM listings l" in sql + assert "l.house_id_fk = houses.id" in sql + assert "l.is_active" in sql + assert "AS has_active_listings" in sql + # active-listings дома сортируются первыми, затем стабильно по id + assert "ORDER BY has_active_listings DESC, id" in sql + + +def test_select_candidates_cadnum_sql_has_active_listings_order(): + """cadnum-priority SELECT — тот же EXISTS + ORDER BY (приоритизация во всех режимах).""" + cadnum_rows = [ + {"id": 50, "address": "ул C 3", "lat": 56.8, "lon": 60.6, "cadastral_number": None}, + ] + db, _updated = _make_db_mock(cadnum_sample=cadnum_rows) + + _select_candidates(db, priority="cadnum", limit=10) + + sql = _capture_select_sql(db)[0] + assert "AS has_active_listings" in sql + assert "ORDER BY has_active_listings DESC, id" in sql + + +def test_select_candidates_active_listings_sort_first_within_bucket(): + """ORDER BY DESC: дом с active listings идёт перед домом без — within один bucket. + + Mock возвращает rows уже в order'е, в котором их вернул бы Postgres + (has_active_listings DESC) — проверяем, что _select_candidates сохраняет + порядок и не пересортировывает по id. + """ + # id=2 имеет active listing → Postgres вернёт его первым несмотря на больший id + coords_rows = [ + {"id": 2, "address": "ул B 2", "lat": None, "lon": None, "cadastral_number": None}, + {"id": 1, "address": "ул A 1", "lat": None, "lon": None, "cadastral_number": None}, + ] + db, _updated = _make_db_mock(coords_sample=coords_rows) + + out = _select_candidates(db, priority="coords", limit=10) + + assert [r.id for r, _p in out] == [2, 1] + + +def test_select_candidates_listings_priority_filters_null_cadnum_and_active(): + """priority=listings → WHERE = NULL cadnum AND EXISTS active listing.""" + listings_rows = [ + {"id": 7, "address": "ул D 4", "lat": 56.8, "lon": 60.6, "cadastral_number": None}, + {"id": 8, "address": "ул E 5", "lat": None, "lon": None, "cadastral_number": None}, + ] + db, _updated = _make_db_mock(listings_sample=listings_rows) + + out = _select_candidates(db, priority="listings", limit=10) + + assert len(out) == 2 + assert all(p == "listings" for _row, p in out) + assert [r.id for r, _p in out] == [7, 8] + + sql = _capture_select_sql(db)[0] + # Фильтр в WHERE: NULL cadnum AND active-listing EXISTS + assert "cadastral_number IS NULL AND EXISTS (SELECT 1 FROM listings l" in sql + assert "l.house_id_fk = houses.id" in sql + assert "l.is_active" in sql + assert "ORDER BY has_active_listings DESC, id" in sql + + +def test_select_candidates_listings_priority_skips_coords_and_cadnum_buckets(): + """priority=listings не должен трогать coords/cadnum SELECT'ы — только один fetch.""" + listings_rows = [ + {"id": 7, "address": "ул D 4", "lat": 56.8, "lon": 60.6, "cadastral_number": None}, + ] + coords_rows = [ + {"id": 1, "address": "ул A 1", "lat": None, "lon": None, "cadastral_number": None}, + ] + cadnum_rows = [ + {"id": 99, "address": "ул Z 9", "lat": 56.8, "lon": 60.6, "cadastral_number": None}, + ] + db, _updated = _make_db_mock( + coords_sample=coords_rows, cadnum_sample=cadnum_rows, listings_sample=listings_rows + ) + + out = _select_candidates(db, priority="listings", limit=10) + + # Ровно один SELECT FROM houses (listings bucket), coords/cadnum не запрошены + assert len(_capture_select_sql(db)) == 1 + assert [r.id for r, _p in out] == [7] + + +def test_select_candidates_listings_priority_respects_limit(): + """priority=listings прокидывает limit в LIMIT bind.""" + listings_rows = [ + {"id": 7, "address": "ул D 4", "lat": 56.8, "lon": 60.6, "cadastral_number": None}, + {"id": 8, "address": "ул E 5", "lat": 56.8, "lon": 60.6, "cadastral_number": None}, + {"id": 9, "address": "ул F 6", "lat": 56.8, "lon": 60.6, "cadastral_number": None}, + ] + db, _updated = _make_db_mock(listings_sample=listings_rows) + + out = _select_candidates(db, priority="listings", limit=2) + + assert len(out) == 2 + # bind lim передан в execute + _sql, params = db.execute.call_args.args + assert params["lim"] == 2 + + # --------------------------------------------------------------------------- # --limit caps batch size # --------------------------------------------------------------------------- @@ -509,6 +652,60 @@ async def test_main_respects_limit(monkeypatch): assert len(updated) == 1 +# --------------------------------------------------------------------------- +# --priority listings — CLI choice + end-to-end routing +# --------------------------------------------------------------------------- + + +def test_parse_args_default_priority_is_both(): + """Дефолт priority остаётся 'both' (не ломаем существующий cron).""" + assert _parse_args([]).priority == "both" + + +def test_parse_args_accepts_priority_listings(): + """--priority listings — валидный choice.""" + assert _parse_args(["--priority", "listings"]).priority == "listings" + + +def test_parse_args_still_accepts_legacy_priorities(): + """coords / cadnum / both продолжают работать без изменений.""" + assert _parse_args(["--priority", "coords"]).priority == "coords" + assert _parse_args(["--priority", "cadnum"]).priority == "cadnum" + assert _parse_args(["--priority", "both"]).priority == "both" + + +def test_parse_args_rejects_unknown_priority(): + """Несуществующий priority → argparse SystemExit (choices guard).""" + with pytest.raises(SystemExit): + _parse_args(["--priority", "garbage"]) + + +async def test_main_priority_listings_enriches_active_listing_house(monkeypatch): + """main(--priority listings) гонит listings-bucket rows через enrichment.""" + listings_rows = [ + {"id": 7, "address": "ул D 4", "lat": 56.8, "lon": 60.6, "cadastral_number": None}, + ] + db, updated = _make_db_mock(listings_sample=listings_rows) + monkeypatch.setenv("DADATA_API_TOKEN", "TEST_TOKEN") + monkeypatch.setenv("DADATA_API_SECRET", "TEST_SECRET") + fake = _make_dadata_result(qc_geo=0) + + with ( + patch("scripts.backfill_houses_dadata.SessionLocal", return_value=db), + patch( + "scripts.backfill_houses_dadata.clean_address", + new=AsyncMock(return_value=fake), + ), + patch("scripts.backfill_houses_dadata.asyncio.sleep", new=AsyncMock()), + ): + n = await main(["--priority", "listings", "--batch", "test_listings"]) + + assert n == 1 + assert len(updated) == 1 + assert updated[0]["id"] == 7 + assert updated[0]["cadnum"] == "66:41:0704045:350" + + # --------------------------------------------------------------------------- # Missing creds → SystemExit # ---------------------------------------------------------------------------