diff --git a/tradein-mvp/backend/data/sql/259_data_quality_drop_pct_cadastr.sql b/tradein-mvp/backend/data/sql/259_data_quality_drop_pct_cadastr.sql new file mode 100644 index 00000000..16312e53 --- /dev/null +++ b/tradein-mvp/backend/data/sql/259_data_quality_drop_pct_cadastr.sql @@ -0,0 +1,145 @@ +-- 259_data_quality_drop_pct_cadastr.sql +-- Purpose (#2674, третий показатель того же класса): убрать v_data_quality.pct_cadastr. +-- +-- 214 убрала outliers_flagged, 216 — price_disagreements_count по одному доводу: ноль, +-- гарантированный устройством системы, читается как «проверили — чисто», хотя честно он +-- означает «мы это не считаем». pct_cadastr — третий такой же, поэтому и действие то же: +-- не переключать источник, а снять показатель. +-- +-- ── ЧИСЛА С ПРОДА (2026-08-13, точный count) ──────────────────────────────── +-- v_data_quality.pct_cadastr .................... 0.000000000000000000000000 +-- знаменатель витрины (listings_active) ......... 45 198 (в listings всего 99 304) +-- listings.cadastral_number IS NOT NULL ......... 0 из 99 304 (и 0 из 45 198 активных) +-- deals.cadastral_number ........................ 0 из 96 974 +-- houses.cadastral_number (DaData) .............. 2 648 из 9 468 ← ДРУГОЙ объект +-- listings.building_cadastral_number ............ 30 970 из 99 304 ← ДРУГОЙ объект +-- +-- ── ЭТО НЕ ДЕФЕКТ ИЗМЕРИТЕЛЯ (контроль на здоровом образце в тех же данных) ── +-- Тот же CTE active_listings и тот же шаблон `count(*) WHERE IS NOT NULL * 100.0 +-- / NULLIF(count(*), 0)` в соседних строках витрины даёт 95.61% (pct_geocoded), 39.82% +-- (pct_description), 65.10% (pct_year_built). Ровно 0% — про колонку, а не про арифметику. +-- +-- ── ПОЧЕМУ НОЛЬ СТРУКТУРНЫЙ ───────────────────────────────────────────────── +-- listings.cadastral_number — кадастр КВАРТИРЫ. Единственное место в коде, которое его +-- вообще читает, — providers/cian/serp.py:886 (`offer.get("cadastralNumber")`); в парсерах +-- avito/yandex/domclick/n1 слов cadastr/kadastr нет ни разу, то есть для ЧЕТЫРЁХ площадок +-- из пяти ноль гарантирован НАШИМ кодом и о предметной области не говорит ничего. Пусто +-- при этом везде, где мы этот номер храним (три таблицы выше) — то же уже записано в +-- app/services/matching/houses.py: «площадки кадастр не отдают». +-- +-- ── ПОЧЕМУ НЕЛЬЗЯ «ПОЧИНИТЬ ОДНОЙ СТРОКОЙ», ПЕРЕКЛЮЧИВ НА СОСЕДНЮЮ КОЛОНКУ ── +-- Напрашивается считать по listings.building_cadastral_number (31.19% всего, 29.47% у +-- активных). Под подписью «доля объявлений с кадастром» это НОВАЯ ложь вместо старой: +-- * это кадастр ЗДАНИЯ, и в listings у него РОВНО ОДИН писатель — наш ночной KNN ≤50 м +-- по локальному зеркалу ЕГРН (tasks/cadastral_geo_match.py:161; проверено `git grep` +-- по origin/main: других INSERT/UPDATE этой колонки нет). Он не «тот же кадастр из +-- другого места», а наша производная; +-- * #2674 замерил ключ как неинъективный (656 из 3 260 значений накрывают >1 здание ГАР, +-- 20.1%; 751 из 2 864 зданий получают >1 значение, 26.2%) и прямо запретил считать его +-- идентичностью здания; +-- * разброс по площадкам среди активных геокодированных (cian 33.8%, yandex 20.3%, +-- avito 42.0%, domclick 49.7%) — про точность НАШИХ координат и охват зеркала по ЕКБ, +-- а не про качество объявления. +-- Переименовать подпись мало: честное имя было бы «доля объявлений, которым ночной KNN +-- подобрал здание в 50 м» — это другой показатель, и заводить его надо отдельно и +-- осознанно, а не под видом починки этого. Авторитетный кадастр здания у нас есть — +-- houses.cadastral_number из DaData (2 648/9 468 домов), но он про ДОМА, а витрина считает +-- ОБЪЯВЛЕНИЯ; подставить его в эту строку — снова назвать одно другим. +-- +-- ── ЦЕНА ПРАВКИ ──────────────────────────────────────────────────────────── +-- Читателей у витрины в коде нет (grep по /app/app в живом backend-контейнере пуст; +-- /api/v1/admin/scraper/data-quality считает свои метрики сам и кадастр не показывает +-- вовсе) — это ручной psql-снимок. Зависимых объектов у view тоже нет (pg_depend по +-- 'v_data_quality'::regclass, прод 13.08: 0 строк), поэтому CASCADE не нужен и не должен +-- появиться: в этом продукте `DROP ... CASCADE` уже терял гранты FDW-пользователю (C3). +-- +-- ── ПОРЯДОК И БЛОКИРОВКА ─────────────────────────────────────────────────── +-- CREATE OR REPLACE VIEW колонку УДАЛИТЬ не может → DROP VIEW → CREATE VIEW (тот же +-- порядок, что 214/216). DROP VIEW берёт ACCESS EXCLUSIVE, поэтому `SET LOCAL +-- lock_timeout` (см. scripts/check-migration-lock-timeout.py). В отличие от 222, которая +-- обошлась CREATE OR REPLACE, здесь COMMENT ON VIEW надо выставить ЗАНОВО: DROP уносит +-- комментарий вместе с объектом. +-- +-- Тело SELECT скопировано из 222_db_audit_cleanup.sql (последний DDL; сверено с живым +-- pg_get_viewdef на проде 13.08 — совпадает) минус строка pct_cadastr. Из CTE убран +-- ставший ненужным cadastral_number: 222 завела явный список колонок ровно затем, чтобы +-- view не держал column-level зависимость на то, чего не показывает. +-- +-- Dependencies: 216_dead_code_sweep.sql (текст COMMENT ON VIEW), 222_db_audit_cleanup.sql +-- (последний DDL v_data_quality). +-- Apply after: 258_houses_imv_transient_attempts.sql +-- Идемпотентно: DROP VIEW IF EXISTS + CREATE VIEW + COMMENT — повторный прогон даёт тот +-- же результат. + +BEGIN; + +-- Ждём лок не дольше 5 s: сам DROP мгновенный, но ждущий ACCESS EXCLUSIVE встаёт в +-- очередь ПЕРЕД новыми запросами (#2791/#2792). +SET LOCAL lock_timeout = '5s'; + +DROP VIEW IF EXISTS v_data_quality; + +-- DDL идентичен 222, минус строка pct_cadastr и минус cadastral_number в CTE. +CREATE VIEW v_data_quality AS +WITH active_listings AS ( + SELECT id, lat, description, house_id_fk, is_active + FROM listings + WHERE is_active = true +) +SELECT + (SELECT count(*) FROM houses) AS houses_total, + (SELECT count(*) FROM houses h + WHERE EXISTS (SELECT 1 FROM house_sources hs WHERE hs.house_id = h.id)) AS houses_with_source, + (SELECT count(*) FROM houses h + WHERE EXISTS (SELECT 1 FROM house_sources hs + WHERE hs.house_id = h.id AND hs.ext_source = 'avito')) AS houses_with_avito, + (SELECT count(*) FROM houses h + WHERE EXISTS (SELECT 1 FROM house_sources hs + WHERE hs.house_id = h.id AND hs.ext_source LIKE 'cian%')) AS houses_with_cian, + (SELECT count(*) FROM houses h + WHERE EXISTS (SELECT 1 FROM house_sources hs + WHERE hs.house_id = h.id AND hs.ext_source = 'yandex')) AS houses_with_yandex, + (SELECT count(*) FROM ( + SELECT house_id FROM house_sources GROUP BY house_id HAVING count(*) >= 2 + ) sub) AS houses_2plus_sources, + (SELECT count(*) FROM ( + SELECT house_id FROM house_sources GROUP BY house_id HAVING count(*) >= 3 + ) sub) AS houses_3plus_sources, + (SELECT count(*) FROM active_listings) AS listings_active, + (SELECT count(*) FROM ( + SELECT listing_id FROM listing_sources + WHERE listing_id IN (SELECT id FROM active_listings) + GROUP BY listing_id HAVING count(*) >= 2 + ) sub) AS listings_dedup_2sources, + (SELECT count(*) FROM active_listings WHERE lat IS NOT NULL) * 100.0 + / NULLIF((SELECT count(*) FROM active_listings), 0) AS pct_geocoded, + (SELECT count(*) FROM active_listings WHERE description IS NOT NULL) * 100.0 + / NULLIF((SELECT count(*) FROM active_listings), 0) AS pct_description, + (SELECT count(*) FROM active_listings l + JOIN houses h ON h.id = l.house_id_fk + WHERE h.year_built IS NOT NULL) * 100.0 + / NULLIF((SELECT count(*) FROM active_listings), 0) AS pct_year_built, + NOW() - (SELECT max(scraped_at) FROM listings WHERE source = 'avito') AS avito_last_scrape_ago, + NOW() - (SELECT max(scraped_at) FROM listings WHERE source = 'cian') AS cian_last_scrape_ago, + NOW() - (SELECT max(scraped_at) FROM listings WHERE source = 'yandex') AS yandex_last_scrape_ago; + +-- Текст 216 + абзац про pct_cadastr. Выставляем заново, потому что DROP VIEW выше унёс +-- прежний комментарий вместе с объектом. +COMMENT ON VIEW v_data_quality IS + 'KPI-снимок для РУЧНЫХ psql-запросов. Читателей в коде нет (проверено #2674): ' + '/api/v1/admin/scraper/data-quality считает свои метрики сам и этот view не трогает. ' + '#2674: price_disagreements_count убран — у всех 89 699 объявлений ровно один ' + 'источник, поэтому показатель структурно не мог быть ненулевым и ноль читался как ' + '«расхождений нет» вместо «мы не сравниваем». listings_dedup_2sources оставлен ' + 'намеренно: он ту же пустоту называет своим именем («объявлений с 2+ источниками»), ' + 'ноль в нём — честный ответ, а не мнимое благополучие. ' + '#2674 (мигр. 259): pct_cadastr убран по тому же доводу — считал ' + 'listings.cadastral_number (кадастр КВАРТИРЫ), а его не отдаёт ни одна площадка: ' + '0 из 99 304 объявлений, 0 из 96 974 deals, единственный читающий его парсер — ' + 'cian/serp.py. Показатель НЕ переведён на listings.building_cadastral_number: та ' + 'колонка — кадастр ЗДАНИЯ и на 100% производная нашего ночного KNN ≤50 м ' + '(tasks/cadastral_geo_match.py), неинъективного как ключ здания (#2674: 20.1% ' + 'значений накрывают >1 здание ГАР); под подписью «доля объявлений с кадастром» она ' + 'мерила бы покрытие нашего геокодера, а не качество объявлений.'; + +COMMIT; diff --git a/tradein-mvp/backend/data/sql/_manifest_applied.txt b/tradein-mvp/backend/data/sql/_manifest_applied.txt index 119f1fb0..61f36e02 100644 --- a/tradein-mvp/backend/data/sql/_manifest_applied.txt +++ b/tradein-mvp/backend/data/sql/_manifest_applied.txt @@ -247,3 +247,4 @@ 254_listings_backfill_avito_rating_glued_address.sql 257_listings_backfill_yandex_source_url.sql 258_houses_imv_transient_attempts.sql +259_data_quality_drop_pct_cadastr.sql diff --git a/tradein-mvp/backend/tests/test_2674_dead_admin_metrics.py b/tradein-mvp/backend/tests/test_2674_dead_admin_metrics.py index ced39ac9..37d2df85 100644 --- a/tradein-mvp/backend/tests/test_2674_dead_admin_metrics.py +++ b/tradein-mvp/backend/tests/test_2674_dead_admin_metrics.py @@ -275,25 +275,62 @@ def test_migration_drops_every_dead_column() -> None: assert "DROP COLUMN IF EXISTS is_outlier" in sql +_VIEW_MARKER = re.compile(r"CREATE\s+(?:OR\s+REPLACE\s+)?VIEW\s+v_data_quality\b") + + +def _latest_v_data_quality() -> tuple[str, str]: + """(текст последней миграции, создающей v_data_quality; тело её SELECT). + + Ищем обе формы DDL (`CREATE VIEW` и `CREATE OR REPLACE VIEW`): миграция с парой + DROP+CREATE иначе оказалась бы невидимой, и тест продолжил бы проверять старую + миграцию, пока показатель уже вернулся в прод. Порядок = лексикографический: + деплой применяет файлы отсортированными, последний по имени — последний в проде. + """ + creators = sorted( + p for p in _SQL_DIR.glob("*.sql") if _VIEW_MARKER.search(p.read_text("utf-8")) + ) + assert creators, "не найдено ни одной миграции, создающей v_data_quality" + sql = creators[-1].read_text(encoding="utf-8") + hit = _VIEW_MARKER.search(sql) + assert hit is not None + return sql, sql[hit.end() :].split(";")[0] + + def test_latest_v_data_quality_no_longer_reports_outliers() -> None: - """Действующее определение v_data_quality (последняя миграция, которая его - создаёт) не упоминает is_outlier. + """Действующее определение v_data_quality не упоминает is_outlier. Red на origin/main: там последним был 095_dead_schema.sql со строкой `(SELECT count(*) FROM listings WHERE is_outlier = true) AS outliers_flagged` — показатель, который не мог быть ненулевым, потому что колонку не писал никто. - - Ищем обе формы DDL (`CREATE VIEW` и `CREATE OR REPLACE VIEW`): миграция с парой - DROP+CREATE иначе оказалась бы невидимой, и тест продолжил бы проверять эту - миграцию, пока показатель уже вернулся в прод. Порядок = лексикографический: - деплой применяет файлы отсортированными, последний по имени — последний в проде. """ - marker = re.compile(r"CREATE\s+(?:OR\s+REPLACE\s+)?VIEW\s+v_data_quality\b") - creators = sorted(p for p in _SQL_DIR.glob("*.sql") if marker.search(p.read_text("utf-8"))) - assert creators, "не найдено ни одной миграции, создающей v_data_quality" - latest = creators[-1].read_text(encoding="utf-8") - hit = marker.search(latest) - assert hit is not None - body = latest[hit.end() :].split(";")[0] + _, body = _latest_v_data_quality() assert "outliers_flagged" not in body assert "is_outlier" not in body + + +def test_latest_v_data_quality_no_longer_reports_flat_cadastre() -> None: + """Тот же класс, третий случай: pct_cadastr (мигр. 259). + + Считался по listings.cadastral_number — кадастру КВАРТИРЫ, которого не отдаёт ни + одна площадка (прод 13.08: 0 из 99 304 объявлений, 0 из 96 974 deals), поэтому + показатель не мог быть ненулевым, а «0.000000» рядом с pct_geocoded 95.61% + читался как измеренное качество данных. + + Red на origin/main: последний DDL там — 222_db_audit_cleanup.sql, в нём строка + `... WHERE cadastral_number IS NOT NULL ... AS pct_cadastr` на месте. + + Замена источника на listings.building_cadastral_number — НЕ починка: та колонка + про ЗДАНИЕ и целиком производная нашего ночного KNN ≤50 м, который #2674 + замерил как неинъективный ключ здания. Поэтому тест запрещает и её появление + в этой витрине. + """ + sql, body = _latest_v_data_quality() + assert "pct_cadastr" not in body, "показатель вернулся в v_data_quality" + assert "cadastral_number" not in body, ( + "в витрину подставили другой кадастр — под подписью «доля объявлений с " + "кадастром» это новая ложь вместо старой (см. шапку 259)" + ) + # DROP VIEW уносит COMMENT вместе с объектом — миграция, которая дропает, обязана + # выставить его заново, иначе объяснение «почему показателя нет» молча теряется. + if re.search(r"DROP\s+VIEW\s+(?:IF\s+EXISTS\s+)?v_data_quality\b", sql): + assert "COMMENT ON VIEW v_data_quality" in sql