From 9b3889bb36e1468f56a34b53290d2e277a8ff144 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sat, 15 Aug 2026 17:55:17 +0300 Subject: [PATCH 1/9] =?UTF-8?q?ci(deploy):=20=D1=87=D0=B5=D1=81=D1=82?= =?UTF-8?q?=D0=BD=D1=8B=D0=B9=20=D1=81=D1=82=D0=B0=D1=82=D1=83=D1=81=20?= =?UTF-8?q?=D0=B4=D0=B5=D0=BF=D0=BB=D0=BE=D1=8F=20+=20=D0=BD=D0=B5=D1=84?= =?UTF-8?q?=D0=B0=D1=82=D0=B0=D0=BB=D1=8C=D0=BD=D1=8B=D0=B9=20buildcache?= =?UTF-8?q?=20(#2841)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Зелёная галка прогона не отличима от пропущенного деплоя: если build падает из-за битого blob в удалённом buildcache, шаг deploy молча пропускается (if-условие даёт result=skipped), а прогон в целом не подсвечен как FAILED. - deploy-status: новая job в конце deploy.yml и deploy-tradein.yml, всегда бежит (if: always() && !cancelled()) и падает явно, если deploy.result != success — неважно, пропущен он (upstream build/test упал) или упал сам. - cache-from нефатален: каждый build-push-action-шаг получил id + continue- on-error, и ретрай без cache-from/cache-to при steps.build.outcome == 'failure'. Битый remote-кеш больше не роняет саму сборку; следующий успешный прогон с кешем перезаписывает buildcache-тег целиком (mode=max) и самолечит порчу. Реальные ошибки сборки (не кеш) по-прежнему валят job на ретрае — deploy-status их тоже поймает. Гейт против публикации services-портов на VPS (та же задача, проблема 1) уже покрыт scripts/check-workflow-ports.py + шагом в ci.yml (#2757/#2759, слит ранее) — сканирует все .forgejo/workflows/*.yml, включая эти два файла; новых правок не потребовалось. docker rm -f БЕЗ -v в SSH-скриптах деплоя не тронут — эти вызовы намеренно без -v (боевые тома), правка их не касается. --- .forgejo/workflows/deploy-tradein.yml | 91 +++++++++++++++++++++++++++ .forgejo/workflows/deploy.yml | 85 +++++++++++++++++++++++++ 2 files changed, 176 insertions(+) diff --git a/.forgejo/workflows/deploy-tradein.yml b/.forgejo/workflows/deploy-tradein.yml index 50311309..8e1ba10a 100644 --- a/.forgejo/workflows/deploy-tradein.yml +++ b/.forgejo/workflows/deploy-tradein.yml @@ -264,6 +264,12 @@ jobs: id: buildx - name: Build & push tradein-backend + # id + continue-on-error: битый blob в удалённом buildcache-манифесте + # валит весь шаг ДО push нового образа — деплой тогда молча + # пропускается (#2841), хотя собрать образ можно и без кеша. Ретрай + # без cache-from — ниже. + id: build + continue-on-error: true uses: docker/build-push-action@v6 with: # Context = tradein-mvp/ (uv workspace root): образу нужен packages/scraper-kit @@ -284,6 +290,23 @@ jobs: ${{ env.IMAGE_BACKEND }}:latest ${{ env.IMAGE_BACKEND }}:${{ github.sha }} + - name: Retry build & push tradein-backend без кеша (битый buildcache, #2841) + # cache-to тоже опущен: следующий успешный прогон С кешем перезапишет + # buildcache-тег целиком (mode=max) и самолечит порчу. + if: steps.build.outcome == 'failure' + uses: docker/build-push-action@v6 + with: + context: ./tradein-mvp + file: ./tradein-mvp/backend/Dockerfile + push: true + build-args: | + APP_VERSION=${{ needs.changes.outputs.app_version }} + BUILD_SHA=${{ needs.changes.outputs.build_sha }} + BUILD_DATE=${{ needs.changes.outputs.build_date }} + tags: | + ${{ env.IMAGE_BACKEND }}:latest + ${{ env.IMAGE_BACKEND }}:${{ github.sha }} + - name: Убрать buildx-билдер (#2869 — иначе копятся по одному на прогон) # setup-buildx-action создаёт билдер `docker-container` на КАЖДЫЙ прогон. # Его post-step под Forgejo act_runner не срабатывает, поэтому к 13.08 на @@ -362,6 +385,10 @@ jobs: run: cp tradein-mvp/CHANGELOG.md tradein-mvp/frontend/CHANGELOG.md - name: Build & push tradein-frontend + # id + continue-on-error — см. tradein-backend (#2841): битый blob в + # удалённом buildcache не должен ронять сборку и молча пропускать деплой. + id: build + continue-on-error: true uses: docker/build-push-action@v6 with: context: ./tradein-mvp/frontend @@ -386,6 +413,24 @@ jobs: ${{ env.IMAGE_FRONTEND }}:latest ${{ env.IMAGE_FRONTEND }}:${{ github.sha }} + - name: Retry build & push tradein-frontend без кеша (битый buildcache, #2841) + # См. tradein-backend: cache-to опущен намеренно (следующий успешный + # прогон с кешем перезапишет buildcache-тег целиком и самолечит порчу). + if: steps.build.outcome == 'failure' + uses: docker/build-push-action@v6 + with: + context: ./tradein-mvp/frontend + push: true + build-args: | + NEXT_PUBLIC_BASE_PATH=/trade-in + NEXT_PUBLIC_API_BASE_URL=/trade-in + NEXT_PUBLIC_APP_VERSION=${{ needs.changes.outputs.app_version }} + NEXT_PUBLIC_BUILD_SHA=${{ needs.changes.outputs.build_sha }} + NEXT_PUBLIC_BUILD_DATE=${{ needs.changes.outputs.build_date }} + tags: | + ${{ env.IMAGE_FRONTEND }}:latest + ${{ env.IMAGE_FRONTEND }}:${{ github.sha }} + - name: Убрать buildx-билдер (#2869 — иначе копятся по одному на прогон) # setup-buildx-action создаёт билдер `docker-container` на КАЖДЫЙ прогон. # Его post-step под Forgejo act_runner не срабатывает, поэтому к 13.08 на @@ -458,6 +503,10 @@ jobs: id: buildx - name: Build & push tradein-browser + # id + continue-on-error — см. tradein-backend выше (#2841): битый blob + # в удалённом buildcache не должен ронять сборку и молча пропускать деплой. + id: build + continue-on-error: true uses: docker/build-push-action@v6 with: context: ./tradein-mvp/browser @@ -468,6 +517,18 @@ jobs: ${{ env.IMAGE_BROWSER }}:latest ${{ env.IMAGE_BROWSER }}:${{ github.sha }} + - name: Retry build & push tradein-browser без кеша (битый buildcache, #2841) + # См. tradein-backend: cache-to опущен намеренно (следующий успешный + # прогон с кешем перезапишет buildcache-тег целиком и самолечит порчу). + if: steps.build.outcome == 'failure' + uses: docker/build-push-action@v6 + with: + context: ./tradein-mvp/browser + push: true + tags: | + ${{ env.IMAGE_BROWSER }}:latest + ${{ env.IMAGE_BROWSER }}:${{ github.sha }} + - name: Убрать buildx-билдер (#2869 — иначе копятся по одному на прогон) # setup-buildx-action создаёт билдер `docker-container` на КАЖДЫЙ прогон. # Его post-step под Forgejo act_runner не срабатывает, поэтому к 13.08 на @@ -1019,3 +1080,33 @@ jobs: # The changes job reads this file on the next run to compute cumulative diff. echo "$GITHUB_SHA" > /opt/gendesign/.tradein-deployed-sha echo "→ Deployed SHA marker updated: $GITHUB_SHA" + + # Честный итог прогона (#2841). ПРОБЛЕМА: `deploy` пропускается своим `if:` + # молча (result=skipped), когда `test` или один из build-* падает (например, + # битый blob в buildcache роняет `docker/build-push-action` — до ретрая + # выше, #2841). skipped-job не красит прогон явным «FAILED» так, чтобы это + # было видно на первый взгляд — итог выглядит зелёным/нейтральным, хотя + # tradein-стек на проде не обновился. Эта job бежит ВСЕГДА (`if: always()`, + # кроме отмены прогона) и сама падает, если deploy не завершился success — + # неважно, пропущен он (test/build упали) или упал сам (SSH/миграция/ + # health-check/сверка образов #2679). Красная точка встаёт именно там, где + # решение реально принято, а не там, где она случайно оказалась по цепочке if. + deploy-status: + runs-on: ubuntu-latest + needs: [test, build-backend, build-frontend, build-browser, deploy] + if: always() && !cancelled() + steps: + - name: Итог прогона — деплой обязан быть success, не skipped/failure + run: | + echo "test: ${{ needs.test.result }}" + echo "build-backend: ${{ needs.build-backend.result }}" + echo "build-frontend: ${{ needs.build-frontend.result }}" + echo "build-browser: ${{ needs.build-browser.result }}" + echo "deploy: ${{ needs.deploy.result }}" + if [ "${{ needs.deploy.result }}" != "success" ]; then + echo "::error::деплой НЕ прошёл (deploy.result=${{ needs.deploy.result }})." \ + "Прогон должен читаться как FAILED, а не как пропущенный шаг (#2841)." \ + "Смотри логи test/build-backend/build-frontend/build-browser/deploy выше." + exit 1 + fi + echo "✓ деплой прошёл успешно" diff --git a/.forgejo/workflows/deploy.yml b/.forgejo/workflows/deploy.yml index 604e28ee..6c40d79d 100644 --- a/.forgejo/workflows/deploy.yml +++ b/.forgejo/workflows/deploy.yml @@ -113,6 +113,12 @@ jobs: id: buildx - name: Build & push backend (lean — без Chromium) + # id + continue-on-error: битый blob в удалённом buildcache-манифесте + # (registry cache, не local) валит весь шаг ДО push нового образа — + # деплой тогда молча пропускается (#2841), хотя код собрать можно, просто + # без кеша. cache-from нефатален: при падении ретраим БЕЗ него ниже. + id: build + continue-on-error: true uses: docker/build-push-action@v6 with: context: ./backend @@ -124,6 +130,21 @@ jobs: ${{ env.IMAGE_BACKEND }}:latest ${{ env.IMAGE_BACKEND }}:${{ github.sha }} + - name: Retry build & push backend без кеша (битый buildcache, #2841) + # cache-to тоже опущен: следующий успешный прогон С кешем перезапишет + # buildcache-тег целиком (mode=max), это самолечит порчу. Если и retry + # упадёт — шаг красный БЕЗ continue-on-error, job честно FAILURE, и + # deploy ниже корректно пропускается (уже настоящая причина, не кеш). + if: steps.build.outcome == 'failure' + uses: docker/build-push-action@v6 + with: + context: ./backend + target: runner + push: true + tags: | + ${{ env.IMAGE_BACKEND }}:latest + ${{ env.IMAGE_BACKEND }}:${{ github.sha }} + - name: Убрать buildx-билдер (#2869 — иначе копятся по одному на прогон) # setup-buildx-action создаёт билдер `docker-container` на КАЖДЫЙ прогон. # Его post-step под Forgejo act_runner не срабатывает, поэтому к 13.08 на @@ -194,6 +215,10 @@ jobs: id: buildx - name: Build & push worker (с Chromium для Playwright) + # id + continue-on-error — см. build-backend выше (#2841): битый blob в + # удалённом buildcache не должен ронять сборку и молча пропускать деплой. + id: build + continue-on-error: true uses: docker/build-push-action@v6 with: context: ./backend @@ -205,6 +230,19 @@ jobs: ${{ env.IMAGE_WORKER }}:latest ${{ env.IMAGE_WORKER }}:${{ github.sha }} + - name: Retry build & push worker без кеша (битый buildcache, #2841) + # См. backend: cache-to опущен намеренно (следующий успешный прогон с + # кешем перезапишет buildcache-тег целиком и самолечит порчу). + if: steps.build.outcome == 'failure' + uses: docker/build-push-action@v6 + with: + context: ./backend + target: runner-with-chromium + push: true + tags: | + ${{ env.IMAGE_WORKER }}:latest + ${{ env.IMAGE_WORKER }}:${{ github.sha }} + - name: Убрать buildx-билдер (#2869 — иначе копятся по одному на прогон) # setup-buildx-action создаёт билдер `docker-container` на КАЖДЫЙ прогон. # Его post-step под Forgejo act_runner не срабатывает, поэтому к 13.08 на @@ -275,6 +313,10 @@ jobs: id: buildx - name: Build & push frontend + # id + continue-on-error — см. build-backend выше (#2841): битый blob в + # удалённом buildcache не должен ронять сборку и молча пропускать деплой. + id: build + continue-on-error: true uses: docker/build-push-action@v6 with: context: ./frontend @@ -288,6 +330,21 @@ jobs: ${{ env.IMAGE_FRONTEND }}:latest ${{ env.IMAGE_FRONTEND }}:${{ github.sha }} + - name: Retry build & push frontend без кеша (битый buildcache, #2841) + # См. backend: cache-to опущен намеренно (следующий успешный прогон с + # кешем перезапишет buildcache-тег целиком и самолечит порчу). + if: steps.build.outcome == 'failure' + uses: docker/build-push-action@v6 + with: + context: ./frontend + push: true + build-args: | + NEXT_PUBLIC_GLITCHTIP_DSN=${{ secrets.GLITCHTIP_FRONTEND_DSN }} + NEXT_PUBLIC_ENVIRONMENT=production + tags: | + ${{ env.IMAGE_FRONTEND }}:latest + ${{ env.IMAGE_FRONTEND }}:${{ github.sha }} + - name: Убрать buildx-билдер (#2869 — иначе копятся по одному на прогон) # setup-buildx-action создаёт билдер `docker-container` на КАЖДЫЙ прогон. # Его post-step под Forgejo act_runner не срабатывает, поэтому к 13.08 на @@ -627,3 +684,31 @@ jobs: curl -fsS http://localhost:8000/health && break sleep 1 done + + # Честный итог прогона (#2841). ПРОБЛЕМА: `deploy` пропускается своим `if:` + # молча (result=skipped), когда build падает (например, битый blob в + # buildcache роняет `docker/build-push-action` — до ретрая выше, #2841). + # skipped-job НЕ красит прогон явным «FAILED» так, чтобы это было видно на + # первый взгляд — итог выглядит зелёным/нейтральным, хотя прод не обновился. + # Эта job бежит ВСЕГДА (`if: always()`, кроме отмены прогона) и сама падает, + # если deploy не завершился success — неважно, пропущен он (build упал) или + # упал сам (SSH/миграция/health-check). Красная точка встаёт именно там, где + # решение реально принято, а не там, где она случайно оказалась по цепочке if. + deploy-status: + runs-on: ubuntu-latest + needs: [build-backend, build-worker, build-frontend, deploy] + if: always() && !cancelled() + steps: + - name: Итог прогона — деплой обязан быть success, не skipped/failure + run: | + echo "build-backend: ${{ needs.build-backend.result }}" + echo "build-worker: ${{ needs.build-worker.result }}" + echo "build-frontend: ${{ needs.build-frontend.result }}" + echo "deploy: ${{ needs.deploy.result }}" + if [ "${{ needs.deploy.result }}" != "success" ]; then + echo "::error::деплой НЕ прошёл (deploy.result=${{ needs.deploy.result }})." \ + "Прогон должен читаться как FAILED, а не как пропущенный шаг (#2841)." \ + "Смотри логи build-backend/build-worker/build-frontend/deploy выше." + exit 1 + fi + echo "✓ деплой прошёл успешно" From 24b70e5c58679d78ab092b2df9217ca4f51ba805 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sat, 15 Aug 2026 17:58:36 +0300 Subject: [PATCH 2/9] =?UTF-8?q?fix(tradein):=20HEAD=20/health=20=D0=BE?= =?UTF-8?q?=D1=82=D0=B2=D0=B5=D1=87=D0=B0=D0=B5=D1=82=20200=20=D0=B2=D0=BC?= =?UTF-8?q?=D0=B5=D1=81=D1=82=D0=BE=20405?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @app.get("/health") в FastAPI/Starlette не добавляет HEAD-обработчик автоматически (в отличие от низкоуровневого Route(methods=["GET"])) — внешний uptime-monитор (GlitchTip PING-тип шлёт HEAD) получал 405 и не мог отличить "жив" от "мёртв" по статусу. Добавлен явный @app.head("/health") — 200 без тела (RFC 9110 §9.3.2), GET не тронут. Тест test_health_endpoint.py фиксирует оба метода; RED до фикса (HEAD → 405), GREEN после (проверено git stash + повторный прогон). --- tradein-mvp/backend/app/main.py | 13 +++++++- .../backend/tests/test_health_endpoint.py | 31 +++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 tradein-mvp/backend/tests/test_health_endpoint.py diff --git a/tradein-mvp/backend/app/main.py b/tradein-mvp/backend/app/main.py index 347cad8c..4f21a09a 100644 --- a/tradein-mvp/backend/app/main.py +++ b/tradein-mvp/backend/app/main.py @@ -12,7 +12,7 @@ from collections.abc import AsyncGenerator from contextlib import asynccontextmanager import sentry_sdk -from fastapi import FastAPI +from fastapi import FastAPI, Response from fastapi.middleware.cors import CORSMiddleware from sentry_sdk.integrations.fastapi import FastApiIntegration from sentry_sdk.integrations.httpx import HttpxIntegration @@ -210,6 +210,17 @@ def health() -> dict[str, str]: return {"status": "ok", "environment": settings.environment} +# FastAPI/Starlette НЕ добавляет HEAD автоматически к @app.get() (в отличие от +# raw Starlette Route с methods=["GET"]) — без явного handler'а HEAD /health +# отдаёт 405, и внешний uptime-monitor (GlitchTip PING-тип, HEAD-запрос) не +# может отличить "жив" от "мёртв" по статусу. Тело для HEAD не отдаём — так +# требует HTTP-спека (RFC 9110 §9.3.2): у ответа те же заголовки, что у GET, +# но без body. +@app.head("/health") +def health_head() -> Response: + return Response(status_code=200) + + app.include_router(auth.router, prefix="/api/v1/auth", tags=["auth"]) app.include_router(geocode.router, prefix="/api/v1/geocode", tags=["geocode"]) app.include_router(admin.router, prefix="/api/v1/admin", tags=["admin"]) diff --git a/tradein-mvp/backend/tests/test_health_endpoint.py b/tradein-mvp/backend/tests/test_health_endpoint.py new file mode 100644 index 00000000..1fa05757 --- /dev/null +++ b/tradein-mvp/backend/tests/test_health_endpoint.py @@ -0,0 +1,31 @@ +"""GET/HEAD /health — uptime-monitor honesty (#uptime-honest-green). + +GlitchTip PING-мониторы шлют HEAD (или GET без чтения тела). Голый +`@app.get("/health")` без явного HEAD-хендлера отдаёт 405 на HEAD — Starlette +НЕ добавляет HEAD автоматически к FastAPI `@app.get()` роуту (в отличие от +низкоуровневого `Route(methods=["GET"])`). Прод-симптом: `HEAD /health` → 405, +монитор либо красный по конструкции, либо (при PING без сверки статуса) +зелёный вне зависимости от факта. Тест фиксирует оба метода. +""" + +from __future__ import annotations + +from fastapi.testclient import TestClient + +from app.main import app + + +def test_health_get_ok() -> None: + client = TestClient(app) + resp = client.get("/health") + assert resp.status_code == 200 + body = resp.json() + assert body["status"] == "ok" + + +def test_health_head_ok_no_body() -> None: + """HEAD /health — то, что реально шлёт uptime-monitor. Должен быть 200, без тела.""" + client = TestClient(app) + resp = client.head("/health") + assert resp.status_code == 200 + assert resp.content == b"" From 885031420eca90a14500bc93a7d22d5aa63534c1 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sat, 15 Aug 2026 18:06:33 +0300 Subject: [PATCH 3/9] =?UTF-8?q?fix(tradein/scrapers):=20honest=20run=20sta?= =?UTF-8?q?tus=20=E2=80=94=20=D1=81=D1=82=D0=BE=D0=BF=20'done'=20=D0=BF?= =?UTF-8?q?=D0=BE=D0=B2=D0=B5=D1=80=D1=85=20=D0=BF=D1=80=D0=BE=D0=B2=D0=B0?= =?UTF-8?q?=D0=BB=D0=B0=20=D0=B8=20=D0=BD=D1=83=D0=BB=D1=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Три прод-факта, где status='done' врал о реальном исходе прогона: - avito_detail_backfill 15.08: {"attempted":64,"failed":57,"enriched":6,"blocked":1} -> 'done'. mark_backfill_finished звал mark_done, потому что produced=6 (>0); ни _sweep_run_did_nothing (нет anchors_total/errors_count у backfill'ов), ни _phase_totally_failed (голые "attempted"/"failed" без фазового префикса) эту форму counters не ловили. Новый _failed_ratio_too_high внутри mark_done: failed/attempted >= 0.5 -> 'failed', >= 0.15 -> тоже 'failed' (другая формулировка причины в error-тексте) — 'partial' статусом не заведён: это потребовало бы DROP+ADD CHECK constraint (051_scrape_runs_extend.sql) и дообучения ещё 4 мест (Literal-фильтр admin API, статусы фронта, оба IN-списка сторожей) — тот же класс проводки, что и у ban_kind (#2686/#2764), который сознательно не стал новым статусом. - yandex_newbuilding_sweep 26.07-10.08: десять прогонов подряд 'done' при processed=5 succeeded=0 rows_inserted=0 failed_resolve=4-5 — сторож нулевого результата (_alert_if_consecutive_zero_results) не видел ни один результатный ключ этого sweep'а и молчал навсегда. _RESULT_COUNTER_KEYS дополнен rows_inserted/processed (именно в этом порядке — rows_inserted это результат, processed это попытки; иначе "5 обработано, 0 записано" замаскировалось бы под measured-5). - admin-витрина показывала new_count=0 у трёх подряд cian_full_load при реально сохранённых saved_inserted=482/214/239 — full-load'ы не пишут ни 'new_count', ни 'lots_inserted'. _column_counts дополнен saved_inserted/rows_inserted. Правки продублированы в scraper_kit/orchestration/runs.py (byte-эквивалент app.services.scrape_runs, см. докстринг модуля) для параллели: единственный текущий писатель "attempted"/"failed" (mark_backfill_finished) живёт только в app-копии, но приоритет ключей/константы держим синхронными на будущее. Не тронуто: сознательно пустые sweep'ы (errors_count=0, honest empty) и малые батчи (attempted < 3) — доля отказов на них не считается диагнозом. Tests: tests/test_honest_run_status_failed_ratio.py (41 кейс, оба модуля, включая точные прод-числа из трёх фактов выше) + regression-прогон 609 тестов по всем файлам, трогающим scrape_runs/orchestration.runs — 0 регрессий. --- .../backend/app/services/scrape_runs.py | 102 ++++++- .../test_honest_run_status_failed_ratio.py | 267 ++++++++++++++++++ .../src/scraper_kit/orchestration/runs.py | 107 ++++++- 3 files changed, 468 insertions(+), 8 deletions(-) create mode 100644 tradein-mvp/backend/tests/test_honest_run_status_failed_ratio.py diff --git a/tradein-mvp/backend/app/services/scrape_runs.py b/tradein-mvp/backend/app/services/scrape_runs.py index 0ecf5ee2..ccac68af 100644 --- a/tradein-mvp/backend/app/services/scrape_runs.py +++ b/tradein-mvp/backend/app/services/scrape_runs.py @@ -143,6 +143,18 @@ def _pick_int(counters: Mapping[str, Any], *keys: str) -> int | None: # unique_fetched — full-load'ы avito/cian/yandex (4 источника, 133 прогона) — раньше # сторож их не видел, хотя у cian_full_load 6 из 38 успешных прогонов # реально дали ноль. +# rows_inserted — yandex_newbuilding_sweep (единственный писатель ключа с таким +# именем на верхнем уровне counters): проверено на проде 26.07-10.08 — +# десять прогонов подряд, все 'done', processed=5 succeeded=0 +# rows_inserted=0 failed_resolve=4-5. Ни total_seen/lots_fetched/ +# unique_fetched у него нет, поэтому раньше _run_result_count всегда +# возвращал None ("не измерено") и стрик у сторожа не копился никогда +# (honest-run-status). +# processed — тот же sweep: сколько домов взял в работу. НАМЕРЕННО стоит ПОСЛЕ +# rows_inserted в кортеже — processed это счётчик ПОПЫТОК (аналог +# attempted), а не результата: у него ненулевое значение (=limit) даже +# когда rows_inserted=0, и если бы он читался первым, «5 обработано, +# 0 записано» замаскировалось бы под measured-5, а не measured-0. # Сводить сюда счётчики ОСТАЛЬНЫХ задач бессмысленно: на проде 28 источников (2650 # прогонов) не имеют общего результатного ключа вовсе — у каждого свой словарь # (deactivated / rows_written / poi_loaded / snapshotted / upserted / listings_matched @@ -150,7 +162,13 @@ def _pick_int(counters: Mapping[str, Any], *keys: str) -> int | None: # трёх мониторов результата нет по смыслу. Ноль у них — часто ЗДОРОВЫЙ ответ # (deactivate_stale_* без протухших объявлений). Поэтому сторож не угадывает их # словарь, а честно признаёт, что мерить нечем — см. _run_result_count. -_RESULT_COUNTER_KEYS = ("total_seen", "lots_fetched", "unique_fetched") +_RESULT_COUNTER_KEYS = ( + "total_seen", + "lots_fetched", + "unique_fetched", + "rows_inserted", + "processed", +) def _run_result_count(counters: Mapping[str, Any] | None) -> int | None: @@ -282,6 +300,63 @@ def _phase_totally_failed(counters: Mapping[str, Any]) -> str | None: return None +# honest-run-status (2026-08-15): доля отказов, которая обесценивает формально ненулевой +# сбор. Прод-факт avito_detail_backfill 15.08: {"attempted":64,"failed":57,"enriched":6, +# "blocked":1} — 89% попыток отказали, а mark_backfill_finished всё равно звал mark_done, +# потому что "produced != 0" (6 обогащено). Ни _sweep_run_did_nothing (нужны +# anchors_total/errors_count, у backfill'ов их нет), ни _phase_totally_failed (нужна пара +# "_attempted"/"_failed" — здесь голые "attempted"/"failed" без фазового +# префикса, `"attempted".endswith("_attempted")` не матчит) эту форму counters не ловят — +# обе проверки написаны под СВОИ формы, а не под backfill'овскую. +# +# Порог 'failed' — половина и больше отказов: сбор для практических целей провалился, +# даже если несколько записей всё же обогатились. Порог 'partial' НЕ заведён отдельным +# статусом scrape_runs.status — это потребовало бы миграции (DROP+ADD CHECK constraint, +# 051_scrape_runs_extend.sql) и обучило бы новому значению ещё 4 места (Literal-фильтр +# admin API, хардкод статусов фронта, оба IN-списка сторожей) — тот же класс "оборванной +# проводки", из-за которого заведён #2686/ban_kind. Вместо статуса — тот же диагноз, что и +# у ban_kind: causa в тексте `error`, терминальный статус один ('failed'). 0.15..0.5 — +# та же 'failed', но с другой формулировкой причины ("деградировал", не "провалился"), чтобы +# оператор видел разницу читая error, не только status. +FAILED_RATIO_FAILED_THRESHOLD = 0.5 +FAILED_RATIO_DEGRADED_THRESHOLD = 0.15 +# Минимум попыток, при котором доля вообще что-то значит — иначе 1 отказ из 2 (=0.5) +# палит статус на шуме единичного случая. То же рассуждение и то же число, что у +# _PHASE_MIN_ATTEMPTS (см. выше). +_FAILED_RATIO_MIN_ATTEMPTS = _PHASE_MIN_ATTEMPTS + + +def _failed_ratio_too_high(counters: Mapping[str, Any]) -> str | None: + """Прогон, у которого доля отказов слишком велика, даже если что-то собрано. + + Возвращает текст причины (для error) либо None. Читает ГОЛЫЕ ключи "attempted"/ + "failed" (без фазового префикса) — сейчас это словарь только у четырёх + detail-backfill'ов (avito/yandex/domclick/newbuilding_enrich), все идут через + mark_backfill_finished → mark_done. `attempted < _FAILED_RATIO_MIN_ATTEMPTS` или + отсутствие любого из ключей → None (нечем/не о чём судить — счётчики либо не + заполнены, либо принадлежат другому источнику со своим словарём). + + Что признак НЕ доказывает: КТО виноват (площадка, наш прокси, наш парсер) — поэтому + 'failed' без диагноза, как и у #2625/#2700/#2764. + """ + attempted = _pick_int(counters, "attempted") + failed = _pick_int(counters, "failed") + if attempted is None or failed is None or attempted < _FAILED_RATIO_MIN_ATTEMPTS: + return None + ratio = failed / max(attempted, 1) + if ratio >= FAILED_RATIO_FAILED_THRESHOLD: + verb = "провалился" + elif ratio >= FAILED_RATIO_DEGRADED_THRESHOLD: + verb = "деградировал" + else: + return None + return ( + f"failed-ratio-honest-status: сбор {verb} — {failed} из {attempted} попыток " + f"отказали (доля {ratio:.0%}); формально ненулевой результат этого не искупает. " + f"Причина НЕ установлена — статус 'failed' без диагноза" + ) + + def _column_counts(counters: dict[str, int]) -> tuple[int | None, int | None]: """Извлечь значения для dedicated-колонок total_seen / new_count из jsonb-counters. @@ -292,13 +367,22 @@ def _column_counts(counters: dict[str, int]) -> tuple[int | None, int | None]: показывала total_seen=0 при реально сохранённых строках (audit #1871/#1926). Приоритет ключей: - - total_seen ← _RESULT_COUNTER_KEYS (total_seen / lots_fetched / unique_fetched) - - new_count ← 'new_count' (если уже есть) иначе 'lots_inserted' + - total_seen ← _RESULT_COUNTER_KEYS (total_seen / lots_fetched / unique_fetched / + rows_inserted / processed) + - new_count ← 'new_count' / 'lots_inserted' / 'saved_inserted' / 'rows_inserted' + (первый присутствующий). 'saved_inserted' — full-load'ы (cian/avito/yandex, + CianFullLoadCounters и аналоги в pipeline.py): на проде витрина показывала + new_count=0 у трёх подряд cian_full_load при реально сохранённых + saved_inserted=482/214/239 (honest-run-status) — ключ 'new_count'/'lots_inserted' + у full-load'ов в counters не пишется вовсе. 'rows_inserted' — тот же ключ, + которым yandex_newbuilding_sweep сообщает число upsert'ов. Возвращает (total_seen, new_count); None для ключа, которого нет в counters — тогда соответствующая колонка не перезаписывается (COALESCE-семантика в UPDATE). """ - return _run_result_count(counters), _pick_int(counters, "new_count", "lots_inserted") + return _run_result_count(counters), _pick_int( + counters, "new_count", "lots_inserted", "saved_inserted", "rows_inserted" + ) def _alert_if_consecutive_failures(db: Session, source: str) -> None: @@ -558,6 +642,11 @@ def mark_done(db: Session, run_id: int, counters: dict[str, int]) -> None: #2700: там же — отказ называть успехом прогон, у которого отказала КАЖДАЯ попытка целой фазы (см. _phase_totally_failed). Отличие от #2625: тот случай про «не сделано ничего», этот — про «одно направление работы мертво, а суммарный сбор это прячет». + + honest-run-status: там же — отказ называть успехом прогон с высокой долей отказов, + даже если собрано > 0 (см. _failed_ratio_too_high). Отличие от #2625/#2700: те два + смотрят на «всё или ничего» (все якоря / вся фаза), этот — на ДОЛЮ отказов у + detail-backfill'ов, где ни один из первых двух признаков не матчит форму counters. """ did_nothing = _sweep_run_did_nothing(counters) if did_nothing is not None: @@ -569,6 +658,11 @@ def mark_done(db: Session, run_id: int, counters: dict[str, int]) -> None: logger.error("%s run_id=%d", phase_dead, run_id) mark_failed(db, run_id, phase_dead, counters) return + ratio_bad = _failed_ratio_too_high(counters) + if ratio_bad is not None: + logger.error("%s run_id=%d", ratio_bad, run_id) + mark_failed(db, run_id, ratio_bad, counters) + return total_seen, new_count = _column_counts(counters) row = db.execute( text( diff --git a/tradein-mvp/backend/tests/test_honest_run_status_failed_ratio.py b/tradein-mvp/backend/tests/test_honest_run_status_failed_ratio.py new file mode 100644 index 00000000..b8c03639 --- /dev/null +++ b/tradein-mvp/backend/tests/test_honest_run_status_failed_ratio.py @@ -0,0 +1,267 @@ +"""honest-run-status (2026-08-15): статус прогона не должен рапортовать 'done' поверх +провала или нуля. Три прод-факта закрыты этой правкой: + + (a) avito_detail_backfill 15.08: {"attempted":64,"failed":57,"enriched":6,"blocked":1} + -> status='done' — 89% отказов, статус зелёный. mark_backfill_finished звал + mark_done, потому что produced=6 (>0); ни _sweep_run_did_nothing (нет + anchors_total/errors_count у backfill'ов), ни _phase_totally_failed (ключи + "attempted"/"failed" без фазового префикса) эту форму counters не ловили. + Фикс: _failed_ratio_too_high внутри mark_done. + + (b) yandex_newbuilding_sweep 26.07-10.08: десять прогонов подряд 'done' при + processed=5, succeeded=0, rows_inserted=0, failed_resolve=4-5 — сторож нулевого + результата (_alert_if_consecutive_zero_results) слеп, т.к. _RESULT_COUNTER_KEYS + не знал ни одного ключа этого sweep'а (total_seen/lots_fetched/unique_fetched). + Фикс: _RESULT_COUNTER_KEYS дополнен rows_inserted/processed (в этом порядке — + rows_inserted это РЕЗУЛЬТАТ, processed это ПОПЫТКИ). + + (c) admin-витрина показывала new_count=0 у трёх подряд cian_full_load, хотя реально + сохранено saved_inserted=482/214/239 — full-load'ы не пишут ни 'new_count', ни + 'lots_inserted'. Фикс: _column_counts дополнен saved_inserted/rows_inserted. + +Проверяем на обоих модулях (kit-копия и app-копия — байт-эквивалентны по докстрингу +runs.py), тем же паттерном, что test_2625_run_that_did_nothing.py. +""" + +from __future__ import annotations + +import os +from typing import Any +from unittest.mock import MagicMock, patch + +import pytest + +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") + +from scraper_kit.orchestration import runs as kit_runs + +from app.services import scrape_runs as app_runs + +_MODULES = {"kit": kit_runs, "app": app_runs} + + +def _capture_status(mod: Any, counters: dict[str, Any]) -> list[str]: + """Прогнать mark_done на фейковой сессии, вернуть статусы всех UPDATE'ов. + + Тот же helper, что в test_2625_run_that_did_nothing.py — читаем СТАТУС В SQL, а не + имя вызванной функции. + """ + statuses: list[str] = [] + + def _execute(stmt: Any, *args: Any, **kwargs: Any) -> MagicMock: + sql = str(stmt) + for status in ("done", "failed", "banned"): + if f"status = '{status}'" in sql: + statuses.append(status) + return MagicMock() + + db = MagicMock() + db.execute.side_effect = _execute + with patch.object(mod, "sentry_sdk", MagicMock()): + mod.mark_done(db, 1, dict(counters)) + return statuses + + +def _capture_backfill_status( + counters: dict[str, Any], *, source: str = "avito_detail_backfill", aborted: bool = False +) -> list[str]: + """Прогнать app_runs.mark_backfill_finished на фейковой сессии (mark_done НЕ мокан — + в отличие от test_backfill_honest_status.py, здесь важно именно его РЕАЛЬНОЕ + поведение: mark_backfill_finished решает вызвать mark_done, а решает ли mark_done + остаться 'done' или сам себя переквалифицировать в 'failed' — предмет этого теста). + + mark_backfill_finished есть только в app_runs (kit-копия его не держит — см. + docstring модуля runs.py, "mark_skipped есть только здесь" — тот же принцип + относится к продуктовым финализаторам detail-backfill'ов). + """ + statuses: list[str] = [] + + def _execute(stmt: Any, *args: Any, **kwargs: Any) -> MagicMock: + sql = str(stmt) + for status in ("done", "failed", "banned"): + if f"status = '{status}'" in sql: + statuses.append(status) + return MagicMock() + + db = MagicMock() + db.execute.side_effect = _execute + with patch.object(app_runs, "sentry_sdk", MagicMock()): + app_runs.mark_backfill_finished( + db, 1, dict(counters), source=source, aborted_by_blocks=aborted + ) + return statuses + + +# ── (a) failed_ratio: прод-факт avito_detail_backfill 15.08 ───────────────────────── + + +def test_prod_fact_avito_15_08_no_longer_done() -> None: + """{"attempted":64,"failed":57,"enriched":6,"blocked":1} — 89% отказов — 'failed', + НЕ 'done'. Красный на старом коде (produced=6 != 0 -> mark_done -> 'done').""" + counters = {"attempted": 64, "failed": 57, "enriched": 6, "blocked": 1} + assert _capture_backfill_status(counters) == ["failed"] + + +def test_prod_fact_avito_reason_names_the_ratio() -> None: + reason = app_runs._failed_ratio_too_high( + {"attempted": 64, "failed": 57, "enriched": 6, "blocked": 1} + ) + assert reason is not None + assert "failed-ratio-honest-status" in reason + assert "57 из 64" in reason + assert "89%" in reason + + +@pytest.mark.parametrize("name", list(_MODULES)) +@pytest.mark.parametrize( + ("counters", "flagged", "why"), + [ + ({"attempted": 64, "failed": 57}, True, "прод-факт: 89% отказов"), + ({"attempted": 10, "failed": 5}, True, "ровно порог failed (0.5)"), + ({"attempted": 20, "failed": 3}, True, "ровно порог degraded (0.15)"), + ({"attempted": 20, "failed": 2}, False, "ниже порога degraded (0.10)"), + ({"attempted": 2, "failed": 2}, False, "ratio=1.0, но < _FAILED_RATIO_MIN_ATTEMPTS"), + ({"attempted": 0, "failed": 0}, False, "нет попыток вовсе"), + ({"failed": 5}, False, "нет attempted — чужой словарь"), + ({"attempted": 50}, False, "нет failed — чужой словарь"), + ({}, False, "пустые counters"), + ( + {"anchors_total": 5, "errors_count": 5, "lots_fetched": 0}, + False, + "sweep-словарь (anchors_total), не detail-backfill", + ), + ], +) +def test_failed_ratio_classifier_boundaries( + name: str, counters: dict[str, Any], flagged: bool, why: str +) -> None: + reason = _MODULES[name]._failed_ratio_too_high(counters) + assert (reason is not None) is flagged, why + + +# ── (5) не должен палить прогоны с малой/умеренной долей отказов ──────────────────── + + +@pytest.mark.parametrize("name", list(_MODULES)) +def test_low_failure_ratio_stays_done(name: str) -> None: + """Штатный шум (10% отказов) не становится 'failed' — не каждый отказ диагноз.""" + counters = {"attempted": 50, "enriched": 45, "failed": 5} + assert _capture_status(_MODULES[name], counters) == ["done"] + + +def test_tiny_batch_zero_produced_fails_via_old_rule_not_ratio() -> None: + """2 попытки, обе отказали, produced=0 — доля тут не при чём (attempted < floor + _FAILED_RATIO_MIN_ATTEMPTS, _failed_ratio_too_high вернул бы None); статус всё + равно 'failed', но по СТАРОМУ правилу #2674 (produced==0), внутри + mark_backfill_finished — mark_done/_failed_ratio_too_high тут не вызываются вовсе. + Показывает, что новая проверка не дублирует и не подменяет старую.""" + counters = {"attempted": 2, "enriched": 0, "failed": 2} + assert _capture_backfill_status(counters) == ["failed"] + + +def test_tiny_batch_with_partial_success_stays_done() -> None: + """2 попытки, 1 успех, 1 отказ (ratio=0.5, но attempted < floor=3) — стрик слишком + короткий, чтобы доля что-то значила -> остаётся 'done'.""" + counters = {"attempted": 2, "enriched": 1, "failed": 1} + assert _capture_backfill_status(counters) == ["done"] + + +@pytest.mark.parametrize("name", list(_MODULES)) +def test_honest_empty_sweep_unaffected_by_failed_ratio(name: str) -> None: + """Сознательно спящее расписание (город без новостроек): sweep-словарь без + attempted/failed вовсе -> failed_ratio не о чем судить, честная пустота остаётся + 'done' (см. также test_2625_run_that_did_nothing.py::test_honest_empty_stays_done).""" + counters = {"anchors_total": 1, "errors_count": 0, "lots_fetched": 0} + assert _capture_status(_MODULES[name], counters) == ["done"] + + +# ── (b) _RESULT_COUNTER_KEYS: прод-факт yandex_newbuilding_sweep 26.07-10.08 ───────── + + +def test_prod_fact_yandex_newbuilding_sweep_measured_as_zero() -> None: + """processed=5, succeeded=0, rows_inserted=0, failed_resolve=4 — раньше + _run_result_count возвращал None ("не измерено"); теперь — измеренный 0.""" + counters = { + "total": 309, + "fetchable": 200, + "pending": 50, + "processed": 5, + "skipped_already_enriched": 0, + "succeeded": 0, + "resolved_slug": 1, + "failed_resolve": 4, + "failed_fetch": 0, + "rows_inserted": 0, + "duration_sec": 42.0, + } + assert app_runs._run_result_count(counters) == 0 + assert kit_runs._run_result_count(counters) == 0 + + +def test_rows_inserted_takes_priority_over_processed() -> None: + """rows_inserted (результат) читается ПЕРЕД processed (попытки) — иначе "5 + обработано, 0 записано" замаскировалось бы под measured-5.""" + counters = {"processed": 5, "rows_inserted": 0} + assert app_runs._run_result_count(counters) == 0 + + +def test_processed_is_fallback_when_rows_inserted_absent() -> None: + counters = {"processed": 3} + assert app_runs._run_result_count(counters) == 3 + + +@pytest.mark.parametrize("name", list(_MODULES)) +def test_zero_result_watchdog_now_fires_for_newbuilding_sweep_streak(name: str) -> None: + """(b) integration: 3 подряд yandex_newbuilding_sweep-подобных 'done' с + rows_inserted=0 -> алерт срабатывает. До фикса _RESULT_COUNTER_KEYS сторож считал + результат "не измеренным" и молчал бы вечно (см. #2703 в docstring модуля).""" + mod = _MODULES[name] + row = MagicMock() + row.status = "done" + row.counters = {"processed": 5, "succeeded": 0, "rows_inserted": 0, "failed_resolve": 4} + db = MagicMock() + result = MagicMock() + result.fetchall.return_value = [row, row, row] + db.execute.return_value = result + with patch.object(mod, "sentry_sdk") as mock_sentry: + mod._alert_if_consecutive_zero_results(db, "yandex_newbuilding_sweep") + mock_sentry.capture_message.assert_called_once() + + +# ── (c) _column_counts: прод-факт cian_full_load new_count=0 при saved_inserted>0 ─── + + +@pytest.mark.parametrize("name", list(_MODULES)) +def test_prod_fact_cian_full_load_saved_inserted_surfaces_as_new_count(name: str) -> None: + """saved_inserted=482 (прод-факт: три подряд прогона 482/214/239) — new_count + больше не 0, хотя ключей 'new_count'/'lots_inserted' в counters нет вовсе.""" + counters = {"unique_fetched": 1200, "saved_inserted": 482, "saved_updated": 30} + total_seen, new_count = _MODULES[name]._column_counts(counters) + assert total_seen == 1200 + assert new_count == 482 + + +@pytest.mark.parametrize("name", list(_MODULES)) +def test_yandex_newbuilding_rows_inserted_surfaces_as_new_count(name: str) -> None: + counters = {"rows_inserted": 7} + _, new_count = _MODULES[name]._column_counts(counters) + assert new_count == 7 + + +@pytest.mark.parametrize("name", list(_MODULES)) +def test_new_count_priority_unchanged_by_new_keys(name: str) -> None: + """'new_count' явный ключ всё ещё побеждает 'lots_inserted'/'saved_inserted' — + расширение списка не меняет приоритет уже существующих ключей.""" + counters = {"new_count": 5, "lots_inserted": 99, "saved_inserted": 1} + _, new_count = _MODULES[name]._column_counts(counters) + assert new_count == 5 + + +@pytest.mark.parametrize("name", list(_MODULES)) +def test_lots_inserted_still_beats_saved_inserted(name: str) -> None: + """Порядок пикулярно НЕ переставлен для уже существующей пары — 'lots_inserted' + (city/newbuilding-sweep'ы) проверяется раньше 'saved_inserted' (full-load'ы), + т.к. это разные, непересекающиеся семейства источников.""" + counters = {"lots_inserted": 12, "saved_inserted": 999} + _, new_count = _MODULES[name]._column_counts(counters) + assert new_count == 12 diff --git a/tradein-mvp/packages/scraper-kit/src/scraper_kit/orchestration/runs.py b/tradein-mvp/packages/scraper-kit/src/scraper_kit/orchestration/runs.py index 86729701..279ea928 100644 --- a/tradein-mvp/packages/scraper-kit/src/scraper_kit/orchestration/runs.py +++ b/tradein-mvp/packages/scraper-kit/src/scraper_kit/orchestration/runs.py @@ -138,6 +138,18 @@ def _pick_int(counters: Mapping[str, Any], *keys: str) -> int | None: # unique_fetched — full-load'ы avito/cian/yandex (4 источника, 133 прогона) — раньше # сторож их не видел, хотя у cian_full_load 6 из 38 успешных прогонов # реально дали ноль. +# rows_inserted — yandex_newbuilding_sweep (единственный писатель ключа с таким +# именем на верхнем уровне counters): проверено на проде 26.07-10.08 — +# десять прогонов подряд, все 'done', processed=5 succeeded=0 +# rows_inserted=0 failed_resolve=4-5. Ни total_seen/lots_fetched/ +# unique_fetched у него нет, поэтому раньше _run_result_count всегда +# возвращал None ("не измерено") и стрик у сторожа не копился никогда +# (honest-run-status). +# processed — тот же sweep: сколько домов взял в работу. НАМЕРЕННО стоит ПОСЛЕ +# rows_inserted в кортеже — processed это счётчик ПОПЫТОК (аналог +# attempted), а не результата: у него ненулевое значение (=limit) даже +# когда rows_inserted=0, и если бы он читался первым, «5 обработано, +# 0 записано» замаскировалось бы под measured-5, а не measured-0. # Сводить сюда счётчики ОСТАЛЬНЫХ задач бессмысленно: на проде 28 источников (2650 # прогонов) не имеют общего результатного ключа вовсе — у каждого свой словарь # (deactivated / rows_written / poi_loaded / snapshotted / upserted / listings_matched @@ -145,7 +157,13 @@ def _pick_int(counters: Mapping[str, Any], *keys: str) -> int | None: # трёх мониторов результата нет по смыслу. Ноль у них — часто ЗДОРОВЫЙ ответ # (deactivate_stale_* без протухших объявлений). Поэтому сторож не угадывает их # словарь, а честно признаёт, что мерить нечем — см. _run_result_count. -_RESULT_COUNTER_KEYS = ("total_seen", "lots_fetched", "unique_fetched") +_RESULT_COUNTER_KEYS = ( + "total_seen", + "lots_fetched", + "unique_fetched", + "rows_inserted", + "processed", +) def _run_result_count(counters: Mapping[str, Any] | None) -> int | None: @@ -277,6 +295,68 @@ def _phase_totally_failed(counters: Mapping[str, Any]) -> str | None: return None +# honest-run-status (2026-08-15): доля отказов, которая обесценивает формально ненулевой +# сбор. Прод-факт avito_detail_backfill 15.08: {"attempted":64,"failed":57,"enriched":6, +# "blocked":1} — 89% попыток отказали, а mark_backfill_finished всё равно звал mark_done, +# потому что "produced != 0" (6 обогащено). Ни _sweep_run_did_nothing (нужны +# anchors_total/errors_count, у backfill'ов их нет), ни _phase_totally_failed (нужна пара +# "_attempted"/"_failed" — здесь голые "attempted"/"failed" без фазового +# префикса, `"attempted".endswith("_attempted")` не матчит) эту форму counters не ловят — +# обе проверки написаны под СВОИ формы, а не под backfill'овскую. +# +# Порог 'failed' — половина и больше отказов: сбор для практических целей провалился, +# даже если несколько записей всё же обогатились. Порог 'partial' НЕ заведён отдельным +# статусом scrape_runs.status — это потребовало бы миграции (DROP+ADD CHECK constraint, +# 051_scrape_runs_extend.sql) и обучило бы новому значению ещё 4 места (Literal-фильтр +# admin API, хардкод статусов фронта, оба IN-списка сторожей) — тот же класс "оборванной +# проводки", из-за которого заведён #2686/ban_kind. Вместо статуса — тот же диагноз, что и +# у ban_kind: causa в тексте `error`, терминальный статус один ('failed'). 0.15..0.5 — +# та же 'failed', но с другой формулировкой причины ("деградировал", не "провалился"), чтобы +# оператор видел разницу читая error, не только status. +# +# mark_backfill_finished (единственный писатель "attempted"/"failed" на верхнем уровне +# counters) живёт только в app.services.scrape_runs — здесь эта проверка сейчас неактивна +# ни для одного реального вызывающего, но kit-копия держится байт-эквивалентной app-копии +# (см. docstring модуля), и будущий kit-native job с тем же словарём получит её даром. +FAILED_RATIO_FAILED_THRESHOLD = 0.5 +FAILED_RATIO_DEGRADED_THRESHOLD = 0.15 +# Минимум попыток, при котором доля вообще что-то значит — иначе 1 отказ из 2 (=0.5) +# палит статус на шуме единичного случая. То же рассуждение и то же число, что у +# _PHASE_MIN_ATTEMPTS (см. выше). +_FAILED_RATIO_MIN_ATTEMPTS = _PHASE_MIN_ATTEMPTS + + +def _failed_ratio_too_high(counters: Mapping[str, Any]) -> str | None: + """Прогон, у которого доля отказов слишком велика, даже если что-то собрано. + + Возвращает текст причины (для error) либо None. Читает ГОЛЫЕ ключи "attempted"/ + "failed" (без фазового префикса) — сейчас это словарь только у четырёх + detail-backfill'ов (avito/yandex/domclick/newbuilding_enrich), все идут через + mark_backfill_finished → mark_done. `attempted < _FAILED_RATIO_MIN_ATTEMPTS` или + отсутствие любого из ключей → None (нечем/не о чём судить — счётчики либо не + заполнены, либо принадлежат другому источнику со своим словарём). + + Что признак НЕ доказывает: КТО виноват (площадка, наш прокси, наш парсер) — поэтому + 'failed' без диагноза, как и у #2625/#2700/#2764. + """ + attempted = _pick_int(counters, "attempted") + failed = _pick_int(counters, "failed") + if attempted is None or failed is None or attempted < _FAILED_RATIO_MIN_ATTEMPTS: + return None + ratio = failed / max(attempted, 1) + if ratio >= FAILED_RATIO_FAILED_THRESHOLD: + verb = "провалился" + elif ratio >= FAILED_RATIO_DEGRADED_THRESHOLD: + verb = "деградировал" + else: + return None + return ( + f"failed-ratio-honest-status: сбор {verb} — {failed} из {attempted} попыток " + f"отказали (доля {ratio:.0%}); формально ненулевой результат этого не искупает. " + f"Причина НЕ установлена — статус 'failed' без диагноза" + ) + + def _column_counts(counters: dict[str, int]) -> tuple[int | None, int | None]: """Извлечь значения для dedicated-колонок total_seen / new_count из jsonb-counters. @@ -287,13 +367,22 @@ def _column_counts(counters: dict[str, int]) -> tuple[int | None, int | None]: показывала total_seen=0 при реально сохранённых строках (audit #1871/#1926). Приоритет ключей: - - total_seen ← _RESULT_COUNTER_KEYS (total_seen / lots_fetched / unique_fetched) - - new_count ← 'new_count' (если уже есть) иначе 'lots_inserted' + - total_seen ← _RESULT_COUNTER_KEYS (total_seen / lots_fetched / unique_fetched / + rows_inserted / processed) + - new_count ← 'new_count' / 'lots_inserted' / 'saved_inserted' / 'rows_inserted' + (первый присутствующий). 'saved_inserted' — full-load'ы (cian/avito/yandex, + CianFullLoadCounters и аналоги в pipeline.py): на проде витрина показывала + new_count=0 у трёх подряд cian_full_load при реально сохранённых + saved_inserted=482/214/239 (honest-run-status) — ключ 'new_count'/'lots_inserted' + у full-load'ов в counters не пишется вовсе. 'rows_inserted' — тот же ключ, + которым yandex_newbuilding_sweep сообщает число upsert'ов. Возвращает (total_seen, new_count); None для ключа, которого нет в counters — тогда соответствующая колонка не перезаписывается (COALESCE-семантика в UPDATE). """ - return _run_result_count(counters), _pick_int(counters, "new_count", "lots_inserted") + return _run_result_count(counters), _pick_int( + counters, "new_count", "lots_inserted", "saved_inserted", "rows_inserted" + ) def _alert_if_consecutive_failures(db: Session, source: str) -> None: @@ -632,6 +721,11 @@ def mark_done(db: Session, run_id: int, counters: dict[str, int]) -> None: #2700: там же — отказ называть успехом прогон, у которого отказала КАЖДАЯ попытка целой фазы (см. _phase_totally_failed). Отличие от #2625: тот случай про «не сделано ничего», этот — про «одно направление работы мертво, а суммарный сбор это прячет». + + honest-run-status: там же — отказ называть успехом прогон с высокой долей отказов, + даже если собрано > 0 (см. _failed_ratio_too_high). Отличие от #2625/#2700: те два + смотрят на «всё или ничего» (все якоря / вся фаза), этот — на ДОЛЮ отказов у + detail-backfill'ов, где ни один из первых двух признаков не матчит форму counters. """ did_nothing = _sweep_run_did_nothing(counters) if did_nothing is not None: @@ -643,6 +737,11 @@ def mark_done(db: Session, run_id: int, counters: dict[str, int]) -> None: logger.error("%s run_id=%d", phase_dead, run_id) mark_failed(db, run_id, phase_dead, counters) return + ratio_bad = _failed_ratio_too_high(counters) + if ratio_bad is not None: + logger.error("%s run_id=%d", ratio_bad, run_id) + mark_failed(db, run_id, ratio_bad, counters) + return total_seen, new_count = _column_counts(counters) row = db.execute( text( From c75206c348bc4ec35f2d51b95215046ac66ec8d9 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sat, 15 Aug 2026 18:08:09 +0300 Subject: [PATCH 4/9] fix(tradein/geocoder): local houses fallback + downgrade DaData CLEAN-disabled noise MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 28/1084 прод-оценок имели lat IS NULL — гарантированный ноль аналогов, клиент не получал оценку вовсе. Дом уже был в houses (скрейпленные листинги), но не резолвился ни geoportal/cad_buildings, ни Nominatim: разговорное/усечённое имя улицы («Онуфриева» вместо ГАР-каноничного «Начдива Онуфриева») или отсутствующий в вводе корпус («49» вместо реального «49к1»). Добавлен последний тир geocode() с двумя defensive-допущениями (суффиксный матч улицы + опциональная догадка «номер+к1») — при любой неоднозначности возвращает None, а не гадает; проверено живыми прод-адресами (Онуфриева/Хрустальногорская резолвятся, Крестинского корректно остаётся неоднозначным — два разных дома в houses под одним номером). Отдельно: HTTP 403 «услуга CLEAN выключена на аккаунте» логировался как ERROR на каждый /estimate (164 события) — это статичная конфигурация аккаунта, а не сбой; понижено до WARNING (первый раз за процесс) + DEBUG на повторы, чтобы ERROR продолжал значить настоящую проблему. --- tradein-mvp/backend/app/schemas/trade_in.py | 7 + tradein-mvp/backend/app/services/dadata.py | 32 +- tradein-mvp/backend/app/services/estimator.py | 1 + tradein-mvp/backend/app/services/geocoder.py | 255 +++++++++++- .../backend/tests/services/test_dadata.py | 37 +- .../test_geocoder_local_houses_fallback.py | 368 ++++++++++++++++++ 6 files changed, 689 insertions(+), 11 deletions(-) create mode 100644 tradein-mvp/backend/tests/test_geocoder_local_houses_fallback.py diff --git a/tradein-mvp/backend/app/schemas/trade_in.py b/tradein-mvp/backend/app/schemas/trade_in.py index 2c29d640..d7666f84 100644 --- a/tradein-mvp/backend/app/schemas/trade_in.py +++ b/tradein-mvp/backend/app/schemas/trade_in.py @@ -223,6 +223,13 @@ class AggregatedEstimate(BaseModel): # UI (снизить доверие / переспросить город), НЕ персистится в БД # (ephemeral, только для текущего POST /estimate ответа). target_city_ambiguous: bool = False + # #2626: True если координаты дал ПОСЛЕДНИЙ тир geocode() — fallback на `houses` + # (см. `app.services.geocoder._local_houses_match`), а не Nominatim/geoportal/ + # cadastral. Значит адрес пользователя не совпал буквально (разговорное/усечённое + # имя улицы или отсутствующий корпус), но был однозначно сопоставлен с домом из + # скрейпленных листингов. Честный сигнал для UI («адрес уточнён автоматически»), + # НЕ персистится в БД (ephemeral, как и `target_city_ambiguous`). + target_address_refined: bool = False sources_used: list[str] = Field(default_factory=list) # ['avito', 'cian', 'rosreestr'] data_freshness_minutes: int | None = None # сколько минут назад был самый свежий парсинг # абсолютный timestamp самого свежего парсинга аналогов diff --git a/tradein-mvp/backend/app/services/dadata.py b/tradein-mvp/backend/app/services/dadata.py index 8a5b9c9f..1f68abf7 100644 --- a/tradein-mvp/backend/app/services/dadata.py +++ b/tradein-mvp/backend/app/services/dadata.py @@ -37,6 +37,12 @@ DADATA_SUGGEST_URL = "https://suggestions.dadata.ru/suggestions/api/4_1/rs/sugge _DADATA_TIMEOUT_S = 8.0 _DADATA_SUGGEST_TIMEOUT_S = 5.0 +# Троттлинг WARNING «услуга CLEAN выключена на аккаунте» (#dadata-403-noise) — +# статичная конфигурация аккаунта, не транзиентный сбой. Первый раз за процесс +# логируется на WARNING, дальше — DEBUG, чтобы не заливать логи одним и тем же +# сообщением на каждый /estimate (было: logger.error на каждый запрос). +_clean_disabled_warned = False + @dataclass(frozen=True, slots=True) class DadataAddressResult: @@ -172,15 +178,29 @@ async def clean_address(address: str) -> DadataAddressResult | None: # но услуга «Стандартизация» (CLEAN) не подключена на аккаунте. Refresh токена НЕ # поможет — нужно включить услугу в кабинете DaData ИЛИ полагаться на suggest-fallback # (enrich_address). Разделяем сообщения, чтобы не гонять зря за ротацией токена. + # + # Это НЕ сбой (аккаунт постоянно живёт с выключенной услугой, enrich_address уже + # graceful-деградирует на suggest — см. ниже) — раньше это било logger.error на + # КАЖДЫЙ пользовательский запрос (164 события/запрос-волна в проде), из-за чего + # ERROR переставал значить «настоящий сбой». WARNING один раз за процесс (дальше — + # DEBUG) сохраняет видимость причины без шума на каждый /estimate. if status == 403 and ( "disabled" in body_preview.lower() or "feature" in body_preview.lower() ): - logger.error( - "dadata: HTTP 403 — услуга CLEAN (Стандартизация) выключена на аккаунте " - "(токен валиден, НЕ отклонён). Включи услугу в кабинете DaData или " - "полагайся на suggest-fallback (enrich_address). Ответ: %r", - body_preview, - ) + global _clean_disabled_warned + if not _clean_disabled_warned: + logger.warning( + "dadata: HTTP 403 — услуга CLEAN (Стандартизация) выключена на аккаунте " + "(токен валиден, НЕ отклонён). Включи услугу в кабинете DaData или " + "полагайся на suggest-fallback (enrich_address). Ответ: %r " + "(повторы этого сообщения в рамках процесса логируются на DEBUG)", + body_preview, + ) + _clean_disabled_warned = True + else: + logger.debug( + "dadata: HTTP 403 CLEAN disabled (уже предупреждено WARNING в этом процессе)" + ) else: logger.error( "dadata: HTTP %d — auth/secret rejected. " diff --git a/tradein-mvp/backend/app/services/estimator.py b/tradein-mvp/backend/app/services/estimator.py index 893da182..c41dbc19 100644 --- a/tradein-mvp/backend/app/services/estimator.py +++ b/tradein-mvp/backend/app/services/estimator.py @@ -4762,6 +4762,7 @@ async def estimate_quality( target_lat=geo.lat, target_lon=geo.lon, target_city_ambiguous=geo.city_ambiguous, + target_address_refined=geo.address_refined, sources_used=sources_used, data_freshness_minutes=freshness_min, last_scraped_at=last_scraped_at, diff --git a/tradein-mvp/backend/app/services/geocoder.py b/tradein-mvp/backend/app/services/geocoder.py index 7785116a..8d798476 100644 --- a/tradein-mvp/backend/app/services/geocoder.py +++ b/tradein-mvp/backend/app/services/geocoder.py @@ -44,6 +44,19 @@ class GeocodeResult: # результата — честный сигнал «доверяй, но проверяй», чтобы вызывающий код мог # понизить confidence / переспросить город у пользователя. См. `_resolve_city_for_geocode`. city_ambiguous: bool = False + # #2626: True если результат дал ПОСЛЕДНИЙ локальный тир — fallback на `houses` + # (скрейпленные листинги, см. `_local_houses_match`) — а не Nominatim/geoportal/ + # cadastral. Срабатывает, когда в тексте адреса опечатка/сокращение улицы + # («Онуфриева» вместо канонического «Начдива Онуфриева» в ГАР) или отсутствует + # корпус («49» вместо реального «49к1») — houses-фолбэк нашёл ОДНОЗНАЧНЫЙ дом по + # нормализованному совпадению. Честный сигнал вызывающему коду «адрес уточнён + # автоматически», НЕ эвристика на корректность — см. `geocode()`/`_local_houses_match`. + # Известный предел: `geocode_cache` НЕ хранит этот флаг (схему не трогаем) — + # на повторный запрос ТОГО ЖЕ сырого адреса из кэша координаты корректные, но + # `address_refined` вернётся `False` (та же судьба у `city_ambiguous` при + # cache-hit — см. `_geocode_resolve`, восстанавливается `replace()` из + # текущего вызова, а не из кэша). + address_refined: bool = False # ── EKB bounding boxes ─────────────────────────────────────────────────────── @@ -1188,12 +1201,13 @@ def _cadastral_house_match(db: Session, street: str, house: str) -> GeocodeSugge ВНИМАНИЕ, цепочки различаются — не путать: * `geocode()` : geoportal → cadastral → `_cadastral_forward_sync` - → Nominatim → None. Тира DaData тут НЕТ. + → Nominatim → `_local_houses_match` (#2626, houses-фолбэк) + → None. Тира DaData тут НЕТ. * `suggest()` : cadastral → DaData → Nominatim (единственный вызов `_dadata_suggest`). То есть на прямом вызове `geocode()` (API/PDF/восстановление по `?id=`) - адрес с литерой, неизвестный ни геопорталу, ни Nominatim, даёт None — - оценка не строится. Это сознательный выбор: честный отказ вместо + адрес с литерой, неизвестный ни геопорталу, ни Nominatim, ни houses-фолбэку, + даёт None — оценка не строится. Это сознательный выбор: честный отказ вместо уверенно-неверной оценки чужого дома. Основной UI-путь этим не задет — координаты приходят из выбранной подсказки (`ParamsPanel.tsx:776` → `api/v1/trade_in.py:128` использует lat/lon напрямую, минуя `geocode()`). @@ -1310,6 +1324,210 @@ def _geoportal_house_match(db: Session, street: str, house: str) -> GeocodeSugge ) +# ── Local `houses` fallback (#2626) — последний тир geocode() ─────────────── +# Мотивация: 28/1084 прод-оценок с lat IS NULL — гарантированный ноль аналогов, +# клиент не получает оценку вовсе. Живые примеры (адрес пользователя → ГАР/houses): +# «ул Крестинского, д 49» — «49» голого нет в houses, есть только «49к1» +# (корпус потерян при вводе, houses id 9980 «улица Крестинского, 49к1»); +# «ул Онуфриева, д 24» — houses называет улицу «Начдива Онуфриева» (ГАР), +# пользователь пишет только последнее слово имени. +# Дом уже ЕСТЬ в `houses` (скрейпленные листинги avito/cian/derived/yandex) с +# координатами — Nominatim и ЕКБ-реестры (geoportal/cad_buildings) эти формы не +# резолвят, а houses чаще содержит именно то написание, которым реально пользуются +# люди (агрегировано из объявлений, а не из официального ГАР). +# +# Номер дома в `houses.address` — СВОБОДНЫЙ текст источников (avito/cian/derived/ +# yandex_valuation): «улица X, 49к1» / «X ул.,88/2» / «X, 44» — БЕЗ единого формата +# и без «д./дом»-маркера, в отличие от `gendesign_cad_buildings.readable_address`. +# Поэтому здесь — собственная, более широкая нормализация номера (со слэшем +# «88/2» и корпусом «49к1»), а НЕ переиспользование `_HOUSE_NUM`/`_norm_house` +# (те заточены под geoportal/cad_buildings реестры, где «/N» и «корпус N» реже). +_LOCAL_HOUSE_TOKEN_RE = re.compile( + r"(\d+(?:\s*/\s*\d+)?(?:\s*-?\s*(?:к|корп\.?|корпус)\.?\s*-?\s*\d+)?(?:\s*-?\s*[а-яё])?)", + re.IGNORECASE, +) + + +def _norm_local_house(raw: str) -> str: + """Канон номера дома для houses-фолбэка. + + «49 к 1» / «49-к1» / «49 корпус 1» → «49к1»; «88 / 2» → «88/2»; «35А» → «35а». + """ + s = raw.strip().lower() + s = re.sub(r"\s+", "", s) + s = re.sub(r"корпус|корп\.?", "к", s) + s = re.sub(r"-(к\d+)", r"\1", s) + s = re.sub(r"-([а-яё])$", r"\1", s) + return s + + +def _extract_local_house_token(address: str) -> str | None: + """Номер дома из ПОЛЬЗОВАТЕЛЬСКОГО адреса — с учётом «/N» и «корпус N» хвостов, + которые `_parse_street_house`/`_HOUSE_NUM` обрезают (см. коммент у + `_LOCAL_HOUSE_TOKEN_RE`). Берём ПОСЛЕДНЕЕ совпадение — номер дома в русском + адресе почти всегда в хвосте строки. None, если цифр нет вовсе. + """ + s = _RE_POSTAL.sub(" ", " ".join(address.lower().strip().split())).strip(" ,.") + if not s: + return None + matches = list(_LOCAL_HOUSE_TOKEN_RE.finditer(s)) + if not matches: + return None + return _norm_local_house(matches[-1].group(1)) + + +# Маркеры района/города/страны — обрезаются из `houses.address` перед сравнением +# улицы (`_clean_local_house_street`). Хвостовое сравнение (см. ниже) и без этого +# устойчиво к ЛИШНЕМУ префиксу («р-н Ленинский, мкр. Юго-Западный, улица X» всё +# равно оканчивается на «... улица x» и матчит суффиксом), но тип улицы ПОСЛЕ +# имени («Хрустальногорская ул.») ломает суффикс без явной зачистки типа. +# Хвостовой якорь — lookahead на пробел/конец строки, а НЕ `\b`: «ул.» в самом +# конце сегмента (частая форма в houses.address) заканчивается точкой, а `\b` +# сразу после точки на границе строки не срабатывает (оба «символа» не-\w) — +# тип-слово матчилось бы БЕЗ точки, точка оставалась бы висеть («хрустальногорская .») +# и ломала «хвостовое» сравнение улицы (реальный прод-кейс: id 13080 houses). +_LOCAL_HOUSE_STREET_TYPE_RE = re.compile(rf"\b(?:{_STREET_TYPE})\.?(?=\s|$)", re.IGNORECASE) + + +def _clean_local_house_street(segment: str) -> str: + """«Хрустальногорская ул.» / «улица Начдива Онуфриева» → «хрустальногорская» / + «начдива онуфриева»: lower, без типа улицы, схлопнутые пробелы. + + Общая нормализация и для запроса пользователя (уже typeless из + `_parse_street_house`, но повторный проход — no-op), и для `houses.address`. + """ + s = _LOCAL_HOUSE_STREET_TYPE_RE.sub(" ", segment.lower()) + return " ".join(s.split()) + + +def _row_local_house(address: str) -> tuple[str, str] | None: + """Разбирает ОДНУ строку `houses.address` на (street_clean, house_norm). + + Номер дома — ПОСЛЕДНИЙ через-запятую сегмент (во всех живых формах: «X, 49к1», + «X ул.,88/2», «X, 44»), СОВПАДЕНИЕ С НАЧАЛА этого сегмента (не всей строки) — + покрывает и «49к1» целиком, и «35к1 · р-н Академический» (хвостовой мусор + после номера отбрасывается). Известный неполный случай (не встретился в + выборке): номер дома БЕЗ запятой перед ним — вернёт None, строка просто не + станет кандидатом (не ложный матч). + """ + segments = [s.strip() for s in address.split(",") if s.strip()] + if len(segments) < 2: + return None + m = _LOCAL_HOUSE_TOKEN_RE.match(segments[-1]) + if not m: + return None + house_norm = _norm_local_house(m.group(1)) + street_norm = _clean_local_house_street(" ".join(segments[:-1])) + if not street_norm or not house_norm: + return None + return street_norm, house_norm + + +def _street_tail_matches(row_street_norm: str, query_street_norm: str) -> bool: + """True если `query_street_norm` — «хвост» (последнее слово/слова) имени улицы + в `houses` — «онуфриева» находит «начдива онуфриева» (ГАР-каноничное имя), + регистронезависимо. Точное равенство тоже проходит (частый случай — короткие + однословные улицы, «Малышева» == «Малышева»).""" + return row_street_norm == query_street_norm or row_street_norm.endswith(" " + query_street_norm) + + +def _local_houses_match(db: Session, street: str, house: str) -> GeocodeSuggestion | None: + """Последний локальный тир `geocode()` (#2626) — fallback на `houses` + (скрейпленные листинги avito/cian/derived/yandex, own DB table, БЕЗ FDW). + + Вызывается ТОЛЬКО когда geoportal/cadastral/Nominatim уже не дали результата. + Два независимых допущения, оба defensive (при неоднозначности — None, не гадаем): + + 1. Улица матчится «по хвосту» (`_street_tail_matches`) — ловит расхождение + разговорного/сокращённого имени («Онуфриева») и канонического ГАР-имени в + houses («Начдива Онуфриева»). + 2. Номер дома — сперва точное совпадение; нет — пробуем `<номер>к1` (частый + случай: пользователь ввёл «49», у дома есть только корпус «49к1»). ЛЮБОЙ + шаг, где кандидатов больше одного (после дедупа по координатам — разные + source-строки ОДНОГО дома не в счёт), возвращает None — угадывать нельзя. + + SQL — дешёвый ILIKE-префильтр по последнему слову улицы (нет индекса на + `houses.address`, но тир последний и редкий — не на каждый запрос), вся + точная логика (суффикс улицы + равенство номера) — в Python, что и делает + её юнит-тестируемой без реальной БД (см. `test_geocoder_local_houses_fallback.py`). + """ + query_street_norm = _clean_local_house_street(street) + if not query_street_norm: + return None + query_house_norm = _norm_local_house(house) + if not query_house_norm: + return None + last_word = query_street_norm.split()[-1] + + try: + rows = db.execute( + text(""" + SELECT address, lat, lon + FROM houses + WHERE address ILIKE CAST('%' || :w || '%' AS text) + AND lat IS NOT NULL AND lon IS NOT NULL + """), + {"w": last_word}, + ).fetchall() + except Exception: + logger.warning( + "local houses fallback query failed for street=%r house=%r", + street, + house, + exc_info=True, + ) + return None + + def _candidates(house_norm: str) -> list[tuple[str, float, float]]: + out: list[tuple[str, float, float]] = [] + seen_coords: set[tuple[float, float]] = set() + for r in rows: + parsed = _row_local_house(str(r.address or "")) + if parsed is None: + continue + row_street_norm, row_house_norm = parsed + if row_house_norm != house_norm: + continue + if not _street_tail_matches(row_street_norm, query_street_norm): + continue + coord_key = (round(float(r.lat), 4), round(float(r.lon), 4)) # ~11m — дедуп источников + if coord_key in seen_coords: + continue + seen_coords.add(coord_key) + out.append((str(r.address), float(r.lat), float(r.lon))) + return out + + exact = _candidates(query_house_norm) + if len(exact) == 1: + addr, lat, lon = exact[0] + return GeocodeSuggestion(label=addr, full_address=addr, lat=lat, lon=lon, kind="house") + if len(exact) > 1: + logger.info( + "local houses fallback: %d неоднозначных кандидата для %r %r — skip", + len(exact), + street, + house, + ) + return None + + # Точного номера нет — пробуем «<номер>к1» (корпус потерян при вводе), ТОЛЬКО + # если запрошенный номер — голое число (не пытаемся достраивать «49/2» → «49/2к1»). + if query_house_norm.isdigit(): + corpus1 = f"{query_house_norm}к1" + guessed = _candidates(corpus1) + if len(guessed) == 1: + addr, lat, lon = guessed[0] + logger.info("local houses fallback: %r → корпус-1 %r (%s)", house, corpus1, addr) + return GeocodeSuggestion(label=addr, full_address=addr, lat=lat, lon=lon, kind="house") + if len(guessed) > 1: + logger.info( + "local houses fallback: корпус-1 %r неоднозначен (%d кандидата) — skip", + corpus1, + len(guessed), + ) + return None + + def _cadastral_reverse_sync(db: Session, lat: float, lon: float, radius_m: int = 200) -> str | None: """Reverse lookup via gendesign_cad_buildings FDW. @@ -1586,6 +1804,37 @@ async def _geocode_resolve( except Exception: logger.exception("nominatim geocoder failed") + # 4. Local `houses` fallback (#2626) — САМЫЙ ПОСЛЕДНИЙ тир, до возврата None. + # 28/1084 прод-оценок имели lat IS NULL (гарантированный ноль аналогов) — дом + # был в `houses` (скрейпленные листинги), но не в geoportal/cad_buildings и не + # резолвился Nominatim'ом (разговорное/усечённое имя улицы или отсутствующий + # в вводе корпус). См. `_local_houses_match`. EKB-only гейт — тот же, что у + # geoportal/cadastral (houses — преимущественно ЕКБ-трафик, тот же риск + # коллизии улица+дом с другим городом региона, что и мотивировал #2582). + if use_local_ekb and parsed is not None: + local_street, _parsed_house = parsed + local_house = _extract_local_house_token(address) or _parsed_house + hit = await asyncio.to_thread(_local_houses_match, db, local_street, local_house) + if hit is not None: + result = GeocodeResult( + lat=hit.lat, + lon=hit.lon, + full_address=hit.full_address, + provider="cache", # локальный DB-lookup, без внешнего HTTP — как geoportal + confidence="exact", + city_ambiguous=city_ambiguous, + address_refined=True, + ) + await asyncio.to_thread(_cache_put, db, addr_norm, result) + logger.info( + "geocode local houses fallback: %s → (%.5f, %.5f) [%s]", + addr_norm, + result.lat, + result.lon, + hit.full_address, + ) + return result + return None diff --git a/tradein-mvp/backend/tests/services/test_dadata.py b/tradein-mvp/backend/tests/services/test_dadata.py index 4b9a7464..a7670fc8 100644 --- a/tradein-mvp/backend/tests/services/test_dadata.py +++ b/tradein-mvp/backend/tests/services/test_dadata.py @@ -781,12 +781,19 @@ def _mock_enrich_transport( async def test_clean_address_logs_feature_disabled_distinctly(caplog) -> None: - """403 «Feature CLEAN disabled» → None + сообщение про выключенную услугу (не про токен).""" + """403 «Feature CLEAN disabled» → None + сообщение про выключенную услугу (не про токен). + + #dadata-403-noise: это статичная конфигурация аккаунта (не транзиентный сбой) — + логируется на WARNING (не ERROR), чтобы ERROR продолжал значить «настоящий сбой» + (раньше — logger.error на КАЖДЫЙ пользовательский запрос, 164 события в проде). + """ from app.services import dadata + dadata._clean_disabled_warned = False # изоляция от порядка тестов (module-level throttle) + transport = _mock_transport_returning(403, CLEAN_FEATURE_DISABLED_BODY) with _patch_settings(), _patch_async_client(transport): - with caplog.at_level(_logging.ERROR, logger="app.services.dadata"): + with caplog.at_level(_logging.WARNING, logger="app.services.dadata"): result = await dadata.clean_address("Екатеринбург, Малышева 4") assert result is None @@ -794,6 +801,32 @@ async def test_clean_address_logs_feature_disabled_distinctly(caplog) -> None: assert "Стандартизация" in text or "выключена" in text # Не должны обвинять токен при feature-disabled. assert "auth/secret rejected" not in text + # НЕ ERROR — статичная причина, не сбой (#dadata-403-noise). + assert not any(rec.levelno >= _logging.ERROR for rec in caplog.records) + + +async def test_clean_address_throttles_repeated_feature_disabled_warning(caplog) -> None: + """Второй (и далее) 403 CLEAN-disabled за один процесс → DEBUG, не повторный WARNING. + + #dadata-403-noise: без троттлинга WARNING на каждый /estimate так же шумит логи, + как раньше шумел ERROR — цель фикса теряется наполовину. + """ + from app.services import dadata + + dadata._clean_disabled_warned = False + + transport = _mock_transport_returning(403, CLEAN_FEATURE_DISABLED_BODY) + with _patch_settings(), _patch_async_client(transport): + with caplog.at_level(_logging.DEBUG, logger="app.services.dadata"): + first = await dadata.clean_address("Екатеринбург, Малышева 4") + caplog.clear() + second = await dadata.clean_address("Екатеринбург, Ленина 10") + + assert first is None + assert second is None + # Второй вызов — НИ ОДНОГО WARNING/ERROR (только DEBUG или тише). + assert not any(rec.levelno >= _logging.WARNING for rec in caplog.records) + assert dadata._clean_disabled_warned is True async def test_clean_address_logs_real_auth_rejection_as_auth(caplog) -> None: diff --git a/tradein-mvp/backend/tests/test_geocoder_local_houses_fallback.py b/tradein-mvp/backend/tests/test_geocoder_local_houses_fallback.py new file mode 100644 index 00000000..fef919a4 --- /dev/null +++ b/tradein-mvp/backend/tests/test_geocoder_local_houses_fallback.py @@ -0,0 +1,368 @@ +"""Unit tests for the `houses` fallback tier of `geocode()` (#2626). + +Covers: +- `_norm_local_house`: normalization of corpus/slash house-number forms + («49 к 1» / «49-к1» / «49 корпус 1» → «49к1»; «88 / 2» → «88/2»). +- `_extract_local_house_token`: pulling the house-number token out of a raw + user address, WITH the corpus/slash suffix that `_parse_street_house`'s + `_HOUSE_NUM` drops. +- `_clean_local_house_street` / `_row_local_house`: extracting a comparable + (street, house) pair out of the free-text `houses.address` column (multiple + scraper source formats — avito/cian/derived/yandex_valuation). +- `_street_tail_matches`: «Онуфриева» finds «Начдива Онуфриева» (ГАР canonical + name), regardless of leading district/city noise. +- `_local_houses_match`: full tier with a mocked DB session — + exact number match, corpus-1 fallback guess («49» → «49к1»), and the + defensive "ambiguous → None" invariant (no guessing on >1 distinct match). +- `geocode()` wiring: local-houses tier is the LAST step, only reached when + cache/geoportal/cadastral/Nominatim all miss, and marks + `GeocodeResult.address_refined=True`. + +Real prod addresses (#2626, lat IS NULL in trade_in_estimates) are used as +regression fixtures: «ул Онуфриева, д 24» → «Начдива Онуфриева, 24к1», +«ул. Хрустальногорская, д. 88/2» → exact match, «ул Крестинского, д 49» → +genuinely ambiguous in prod data (two DIFFERENT buildings both stored as +«Крестинского, 49к1» — must NOT resolve, per the defensive "no guessing" rule). +""" + +from __future__ import annotations + +import os +import sys +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") + +_wp_mock = MagicMock() +sys.modules.setdefault("weasyprint", _wp_mock) + +from app.services.geocoder import ( # noqa: E402 + GeocodeSuggestion, + _clean_local_house_street, + _extract_local_house_token, + _local_houses_match, + _norm_local_house, + _row_local_house, + _street_tail_matches, + geocode, +) + +# ── _norm_local_house ──────────────────────────────────────────────────────── + + +@pytest.mark.parametrize( + ("raw", "expected"), + [ + ("49 к 1", "49к1"), + ("49-к1", "49к1"), + ("49 корпус 1", "49к1"), + ("49 корп. 1", "49к1"), + ("88 / 2", "88/2"), + ("88/2", "88/2"), + ("35А", "35а"), + ("13Б", "13б"), + ("13-б", "13б"), + ("44", "44"), + ], +) +def test_norm_local_house(raw: str, expected: str) -> None: + assert _norm_local_house(raw) == expected + + +# ── _extract_local_house_token ─────────────────────────────────────────────── + + +@pytest.mark.parametrize( + ("address", "expected"), + [ + ("ул Крестинского, д 49", "49"), + ("ул. Хрустальногорская, д. 88/2", "88/2"), + ("ул Онуфриева, д 24", "24"), + ("Крестинского 49к1", "49к1"), + ("8 Марта 204", "204"), # digit-leading street name doesn't confuse it + ("Малышева 30", "30"), + ], +) +def test_extract_local_house_token(address: str, expected: str) -> None: + assert _extract_local_house_token(address) == expected + + +def test_extract_local_house_token_none_for_garbage() -> None: + assert _extract_local_house_token("") is None + assert _extract_local_house_token("Екатеринбург") is None + + +# ── _clean_local_house_street / _street_tail_matches ──────────────────────── + + +def test_clean_local_house_street_strips_type_regardless_of_position() -> None: + """Тип улицы ДО имени («улица X») и ПОСЛЕ («X ул.») — оба зачищаются.""" + assert _clean_local_house_street("улица Начдива Онуфриева") == "начдива онуфриева" + assert _clean_local_house_street("Хрустальногорская ул.") == "хрустальногорская" + + +def test_street_tail_matches_onufrieva_finds_nachdiva_onufrieva() -> None: + """Ядро #2626: «Онуфриева» (как пишет пользователь) находит «Начдива + Онуфриева» (каноничное имя ГАР, как в houses.address).""" + assert _street_tail_matches("начдива онуфриева", "онуфриева") is True + + +def test_street_tail_matches_exact_equality() -> None: + assert _street_tail_matches("хрустальногорская", "хрустальногорская") is True + + +def test_street_tail_matches_rejects_non_suffix_substring() -> None: + """«Онуфриева» НЕ находит несвязанную улицу, где она — не хвостовое слово.""" + assert _street_tail_matches("онуфриева южная", "онуфриева") is False + + +# ── _row_local_house: разбор houses.address разных форматов источников ────── + + +@pytest.mark.parametrize( + ("row_address", "expected"), + [ + ( + "р-н Чкаловский, мкр. Ботанический, улица Крестинского, 49к1", + ("р-н чкаловский мкр. ботанический крестинского", "49к1"), + ), + ("Хрустальногорская ул.,88/2", ("хрустальногорская", "88/2")), + ("ул. Начдива Онуфриева,24к2", ("начдива онуфриева", "24к2")), + ( + "р-н Ленинский, мкр. Юго-Западный, улица Начдива Онуфриева, 24к1", + ("р-н ленинский мкр. юго-западный начдива онуфриева", "24к1"), + ), + ("Крестинского, 44", ("крестинского", "44")), + # house-then-district order («·» separator, no comma before house) — + # match-from-start of the LAST comma-segment still finds the leading token. + ("улица Хрустальногорская, 35к1 · р-н Академический", ("хрустальногорская", "35к1")), + ], +) +def test_row_local_house(row_address: str, expected: tuple[str, str]) -> None: + assert _row_local_house(row_address) == expected + + +def test_row_local_house_none_without_house_segment() -> None: + """Нет запятой (номер дома не отделён сегментом) → None, не гадаем.""" + assert _row_local_house("Крестинского") is None + assert _row_local_house("") is None + + +# ── _local_houses_match: full tier, mocked db ──────────────────────────────── + + +def _make_row(address: str, lat: float, lon: float) -> MagicMock: + row = MagicMock() + row.address = address + row.lat = lat + row.lon = lon + return row + + +def _db_with_rows(rows: list[MagicMock]) -> MagicMock: + db = MagicMock() + db.execute.return_value.fetchall.return_value = rows + return db + + +def test_local_houses_match_exact_house_number() -> None: + """«88/2» точно совпадает с единственной строкой houses — возвращает её координаты.""" + db = _db_with_rows( + [ + _make_row("Хрустальногорская ул.,88", 56.79412, 60.498687), + _make_row("Хрустальногорская ул.,88/2", 56.793218, 60.497106), + ] + ) + + hit = _local_houses_match(db, "хрустальногорская", "88/2") + + assert hit is not None + assert isinstance(hit, GeocodeSuggestion) + assert hit.lat == pytest.approx(56.793218) + assert hit.lon == pytest.approx(60.497106) + assert hit.kind == "house" + + +def test_local_houses_match_street_tail_and_corpus1_guess() -> None: + """«Онуфриева, 24» (без «Начдива», без корпуса) → единственный «24к1» реестра.""" + db = _db_with_rows( + [ + _make_row( + "р-н Ленинский, мкр. Юго-Западный, улица Начдива Онуфриева, 24к1", + 56.802928, + 60.551696, + ), + _make_row("ул. Начдива Онуфриева,24к2", 56.802701, 60.554391), + _make_row("Екатеринбург, улица Начдива Онуфриева, 24к3", 56.802041, 60.548283), + ] + ) + + hit = _local_houses_match(db, "онуфриева", "24") + + assert hit is not None + assert hit.lat == pytest.approx(56.802928) + assert hit.lon == pytest.approx(60.551696) + + +def test_local_houses_match_no_corpus1_candidate_returns_none() -> None: + """Только «24к2»/«24к3» в реестре (нет «24к1») → фолбэк НЕ гадает, None.""" + db = _db_with_rows( + [ + _make_row("ул. Начдива Онуфриева,24к2", 56.802701, 60.554391), + _make_row("Екатеринбург, улица Начдива Онуфриева, 24к3", 56.802041, 60.548283), + ] + ) + + assert _local_houses_match(db, "онуфриева", "24") is None + + +def test_local_houses_match_ambiguous_exact_number_returns_none() -> None: + """Прод-кейс: «Крестинского, 49к1» встречается ДВАЖДЫ с РАЗНЫМИ координатами + (две разные строки houses) — неоднозначность, фолбэк не угадывает, None.""" + db = _db_with_rows( + [ + _make_row( + "р-н Чкаловский, мкр. Ботанический, улица Крестинского, 49к1", + 56.789895, + 60.632464, + ), + _make_row("Екатеринбург, улица Крестинского, 49к1", 56.7952695, 60.610079), + ] + ) + + assert _local_houses_match(db, "крестинского", "49к1") is None + + +def test_local_houses_match_ambiguous_corpus1_guess_returns_none() -> None: + """«49» → «49к1»-кандидатов больше одного (разные координаты) → None.""" + db = _db_with_rows( + [ + _make_row("улица X, 49к1", 56.80, 60.60), + _make_row("улица X, 49к1", 56.81, 60.61), + ] + ) + + assert _local_houses_match(db, "x", "49") is None + + +def test_local_houses_match_deduplicates_same_building_different_sources() -> None: + """Один и тот же дом, две source-строки (avito+cian) с ПОЧТИ идентичными + координатами — НЕ считается неоднозначностью (дедуп по округлённым coords).""" + db = _db_with_rows( + [ + _make_row("улица X, 49к1", 56.800001, 60.600001), + _make_row("улица X, 49к1", 56.800002, 60.600002), # тот же дом, другой source + ] + ) + + hit = _local_houses_match(db, "x", "49к1") + + assert hit is not None + assert hit.lat == pytest.approx(56.800001) + + +def test_local_houses_match_no_guess_for_non_digit_house() -> None: + """Запрос уже с литерой/корпусом («35к3»), точного совпадения нет — корпус-1 + ДОГАДКА не пробуется (не «35к3к1»), результат None.""" + db = _db_with_rows([_make_row("улица X, 35к4", 56.80, 60.60)]) + + assert _local_houses_match(db, "x", "35к3") is None + + +def test_local_houses_match_returns_none_on_db_error() -> None: + db = MagicMock() + db.execute.side_effect = RuntimeError("connection lost") + + assert _local_houses_match(db, "онуфриева", "24") is None + + +# ── geocode() wiring — last-resort tier, sets address_refined ─────────────── + + +async def test_geocode_falls_back_to_local_houses_after_nominatim_miss() -> None: + """Cache/geoportal/cadastral/Nominatim все промахнулись → local-houses тир + вызывается ПОСЛЕДНИМ и помечает результат `address_refined=True`.""" + db = MagicMock() + hit = GeocodeSuggestion( + label="р-н Ленинский, мкр. Юго-Западный, улица Начдива Онуфриева, 24к1", + full_address="р-н Ленинский, мкр. Юго-Западный, улица Начдива Онуфриева, 24к1", + lat=56.802928, + lon=60.551696, + kind="house", + ) + + with ( + patch("app.services.geocoder._cache_get", return_value=None), + patch("app.services.geocoder._geoportal_house_match", return_value=None), + patch("app.services.geocoder._cadastral_house_match", return_value=None), + patch("app.services.geocoder._cadastral_forward_sync", return_value=[]), + patch("app.services.geocoder._cache_put"), + patch( + "app.services.geocoder._nominatim_lookup", + new_callable=AsyncMock, + return_value=None, + ), + patch( + "app.services.geocoder._local_houses_match", + return_value=hit, + ) as mock_local, + ): + result = await geocode("ул Онуфриева, д 24", db) + + assert result is not None + assert result.lat == pytest.approx(56.802928) + assert result.confidence == "exact" + assert result.address_refined is True + mock_local.assert_called_once() + + +async def test_geocode_address_refined_false_when_earlier_tier_hits() -> None: + """geoportal-хит (обычный, точный ввод) НЕ помечается `address_refined` — + флаг честно относится ТОЛЬКО к houses-фолбэку.""" + db = MagicMock() + hit = GeocodeSuggestion( + label="ул. Серова, д. 27, Екатеринбург", + full_address="ул. Серова, д. 27, Екатеринбург", + lat=56.81188, + lon=60.59739, + kind="house", + ) + + with ( + patch("app.services.geocoder._cache_get", return_value=None), + patch("app.services.geocoder._geoportal_house_match", return_value=hit), + patch("app.services.geocoder._cache_put"), + patch( + "app.services.geocoder._local_houses_match", + ) as mock_local, + ): + result = await geocode("Серова 27", db) + + assert result is not None + assert result.address_refined is False + mock_local.assert_not_called() + + +async def test_geocode_returns_none_when_local_houses_also_misses() -> None: + """Все тиры включая houses-фолбэк промахнулись → honest None (не выдумываем).""" + db = MagicMock() + + with ( + patch("app.services.geocoder._cache_get", return_value=None), + patch("app.services.geocoder._geoportal_house_match", return_value=None), + patch("app.services.geocoder._cadastral_house_match", return_value=None), + patch("app.services.geocoder._cadastral_forward_sync", return_value=[]), + patch("app.services.geocoder._cache_put"), + patch( + "app.services.geocoder._nominatim_lookup", + new_callable=AsyncMock, + return_value=None, + ), + patch("app.services.geocoder._local_houses_match", return_value=None) as mock_local, + ): + result = await geocode("ул Онуфриева, д 24", db) + + assert result is None + mock_local.assert_called_once() From 3f5f09939237353a52fb3e8f0be74424228df7a8 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sat, 15 Aug 2026 18:44:31 +0300 Subject: [PATCH 5/9] =?UTF-8?q?fix(health):=20HEAD=20/health=20=D0=BD?= =?UTF-8?q?=D0=B0=20=D0=B2=D0=B5=D1=80=D0=BD=D0=BE=D0=BC=20=D0=B1=D1=8D?= =?UTF-8?q?=D0=BA=D0=B5=D0=BD=D0=B4=D0=B5=20(Site=20Finder)=20+=20=D1=87?= =?UTF-8?q?=D0=B5=D1=81=D1=82=D0=BD=D1=8B=D0=B5=20=D0=B7=D0=B0=D0=B3=D0=BE?= =?UTF-8?q?=D0=BB=D0=BE=D0=B2=D0=BA=D0=B8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review-разбор ветки fix/tradein-uptime-honest-green: 1. [HIGH] Прод-симптом `HEAD gendsgn.ru/health -> 405` обслуживает Site Finder (Caddyfile:60 `handle /health { reverse_proxy backend:8000 }`), а предыдущий коммит правил только tradein-mvp/backend, чей /health наружу не проксируется вообще. Добавлен @app.head("/health") в backend/app/main.py рядом с существующим @app.get — эмпирически подтверждено (uv run pytest): HEAD было 405, стало 200. tradein-mvp фикс не откачен (безвреден, годится для будущего internal-caller), но обвязан комментарием, что реальный прод-путь чинится не там. 2. [LOW] Response(status_code=200) без media_type отдавал HEAD без Content-Type, тогда как GET отдаёт application/json — расходится с заявленным в комментарии RFC 9110 §9.3.2. Добавлен media_type в обоих бэкендах; Content-Length сознательно не подгоняем под байты GET-ответа (payload header field, RFC разрешает опускать для HEAD) — не дублируем сборку payload ради байт-в-байт соответствия. Тесты: test_health_head_ok_no_body добавлен в backend/tests/test_health.py (Site Finder) — RED-check (git stash app/main.py) воспроизводит прод-баг 1:1: assert 405 == 200. tradein-mvp/backend/tests/test_health_endpoint.py дополнен проверкой Content-Type. uv run pytest — все зелёные. --- backend/app/main.py | 16 ++++++++++++++++ backend/tests/test_health.py | 18 ++++++++++++++++++ tradein-mvp/backend/app/main.py | 16 +++++++++++----- .../backend/tests/test_health_endpoint.py | 4 ++++ 4 files changed, 49 insertions(+), 5 deletions(-) diff --git a/backend/app/main.py b/backend/app/main.py index e0ac46cb..c779e335 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -508,3 +508,19 @@ async def health() -> dict[str, str]: "environment": settings.environment, "version": app.version, } + + +# FastAPI/Starlette НЕ добавляет HEAD автоматически к @app.get() (в отличие от +# raw Starlette Route с methods=["GET"]) — без явного handler'а HEAD /health +# отдаёт 405. Это боевой прод-эндпоинт: Caddyfile:60 `handle /health { +# reverse_proxy backend:8000 }` — именно ЭТОТ хендлер отвечает на +# `HEAD https://gendsgn.ru/health`, которым бьёт внешний uptime-monitor +# (GlitchTip PING-тип шлёт HEAD, не GET) и не мог отличить "жив" от "мёртв" по +# статусу. media_type="application/json" — Content-Type совпадает с GET; +# Content-Length сознательно НЕ вычисляем под байт GET-ответа (пришлось бы +# дублировать сборку payload) — RFC 9110 §9.3.2 разрешает опускать payload- +# заголовки (Content-Length) для HEAD, требует совпадения только заголовков +# представления (Content-Type). +@app.head("/health") +async def health_head() -> Response: + return Response(status_code=200, media_type="application/json") diff --git a/backend/tests/test_health.py b/backend/tests/test_health.py index c432abcf..a62f2567 100644 --- a/backend/tests/test_health.py +++ b/backend/tests/test_health.py @@ -9,3 +9,21 @@ def test_health() -> None: assert response.status_code == 200 body = response.json() assert body["status"] == "ok" + + +def test_health_head_ok_no_body() -> None: + """HEAD /health — то, что реально шлёт внешний uptime-monitor через Caddy + (`handle /health { reverse_proxy backend:8000 }`, Caddyfile:60), не GET. + + Starlette не добавляет HEAD автоматически к `@app.get()` (в отличие от + низкоуровневого `Route(methods=["GET"])`) — без явного `@app.head()` + прод-эндпоинт отдаёт 405 на HEAD. + """ + client = TestClient(app) + response = client.head("/health") + assert response.status_code == 200 + assert response.content == b"" + # RFC 9110 §9.3.2 — заголовки представления (Content-Type) должны совпадать + # с GET; Content-Length допустимо не совпадать (payload header field, MAY + # быть опущен для HEAD). + assert response.headers["content-type"] == "application/json" diff --git a/tradein-mvp/backend/app/main.py b/tradein-mvp/backend/app/main.py index 4f21a09a..8a48c7c3 100644 --- a/tradein-mvp/backend/app/main.py +++ b/tradein-mvp/backend/app/main.py @@ -212,13 +212,19 @@ def health() -> dict[str, str]: # FastAPI/Starlette НЕ добавляет HEAD автоматически к @app.get() (в отличие от # raw Starlette Route с methods=["GET"]) — без явного handler'а HEAD /health -# отдаёт 405, и внешний uptime-monitor (GlitchTip PING-тип, HEAD-запрос) не -# может отличить "жив" от "мёртв" по статусу. Тело для HEAD не отдаём — так -# требует HTTP-спека (RFC 9110 §9.3.2): у ответа те же заголовки, что у GET, -# но без body. +# отдаёт 405. NB: наружу через Caddy этот /health НЕ проксируется (только +# /trade-in/api/* → strip_prefix → tradein-backend:8000/api/v1/*), и никакой +# docker healthcheck на него сейчас тоже не настроен (grep по compose-файлам — +# только pg_isready для postgres) — маршрут пока используется лишь тестами. +# Внешний прод-симптом `HEAD gendsgn.ru/health -> 405` чинится в Site Finder +# (backend/app/main.py, за Caddyfile `handle /health`), не здесь. +# media_type="application/json" — Content-Type совпадает с GET; Content-Length +# сознательно НЕ вычисляем под байт GET-ответа (дублировало бы сборку payload) +# — RFC 9110 §9.3.2 разрешает опускать payload-заголовки (Content-Length) для +# HEAD, требует совпадения только заголовков представления (Content-Type). @app.head("/health") def health_head() -> Response: - return Response(status_code=200) + return Response(status_code=200, media_type="application/json") app.include_router(auth.router, prefix="/api/v1/auth", tags=["auth"]) diff --git a/tradein-mvp/backend/tests/test_health_endpoint.py b/tradein-mvp/backend/tests/test_health_endpoint.py index 1fa05757..be2d7fab 100644 --- a/tradein-mvp/backend/tests/test_health_endpoint.py +++ b/tradein-mvp/backend/tests/test_health_endpoint.py @@ -29,3 +29,7 @@ def test_health_head_ok_no_body() -> None: resp = client.head("/health") assert resp.status_code == 200 assert resp.content == b"" + # RFC 9110 §9.3.2 — HEAD должен вернуть те же заголовки представления + # (Content-Type), что и GET; Content-Length допустимо не совпадать (payload + # header field, MAY быть опущен для HEAD). + assert resp.headers["content-type"] == "application/json" From 8bce8cf5aea6bef89250ed276e92e14c4bff54a6 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sat, 15 Aug 2026 18:44:48 +0300 Subject: [PATCH 6/9] fix(ci): fail-safe registry verification + real cache self-heal + honest health-check (#2841 R2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ревью R2 нашёл, что вся безопасность предыдущего фикса держалась на недоказанной поддержке act_runner'ом steps..outcome: если раннер его не заполняет, retry-шаг молча не бежит, continue-on-error проглатывает падение сборки, job зелёный — а деплой тянет старый :latest на прод. - Добавлен engine-agnostic verify-шаг после каждого retry (6 мест, deploy.yml + deploy-tradein.yml): `docker buildx imagetools inspect :` без continue-on-error. Не зависит от того, поддерживает ли раннер outcome — проверяет реальное состояние registry напрямую. Если ни build, ни retry реально не запушили образ — шаг падает и job честно FAILURE независимо от семантики outcome. - Вернул `cache-to` в retry-шаги (6 мест): без него битый buildcache-тег никогда не перезаписывался — retry всегда собирал без cache-to, значит cache-to не выполнялся НИКОГДА, и каждый следующий прогон снова падал на том же cache-from. Заявленное самолечение не работало ни разу. - Health-check в deploy.yml (main-стек) под `set -e` не мог упасть: `curl ... && break` — curl не последняя команда &&-списка, POSIX освобождает такие команды от errexit, цикл дохаживал до sleep (exit 0) даже если curl ни разу не отдал 200. Приведено к паттерну deploy-tradein.yml: явный флаг healthy + `exit 1` после цикла. Подтверждено локальным bash-репро (mock curl, всегда failure): старая версия — exit 0, новая — exit 1; позитивный сценарий не сломан. docker rm -f без -v в SSH-скриптах деплоя не тронут. --- .forgejo/workflows/deploy-tradein.yml | 36 +++++++++++--- .forgejo/workflows/deploy.yml | 67 +++++++++++++++++++++++---- 2 files changed, 87 insertions(+), 16 deletions(-) diff --git a/.forgejo/workflows/deploy-tradein.yml b/.forgejo/workflows/deploy-tradein.yml index 8e1ba10a..640d12ff 100644 --- a/.forgejo/workflows/deploy-tradein.yml +++ b/.forgejo/workflows/deploy-tradein.yml @@ -291,8 +291,10 @@ jobs: ${{ env.IMAGE_BACKEND }}:${{ github.sha }} - name: Retry build & push tradein-backend без кеша (битый buildcache, #2841) - # cache-to тоже опущен: следующий успешный прогон С кешем перезапишет - # buildcache-тег целиком (mode=max) и самолечит порчу. + # cache-from опущен (источник падения), cache-to ОСТАВЛЕН (ревью #2841 R2, + # issue #2): успешный ретрай перезаписывает битый buildcache-тег своими + # слоями (mode=max) — это и есть самолечение. Без cache-to здесь порча + # оставалась навсегда, следующий прогон снова падал на том же cache-from. if: steps.build.outcome == 'failure' uses: docker/build-push-action@v6 with: @@ -303,10 +305,20 @@ jobs: APP_VERSION=${{ needs.changes.outputs.app_version }} BUILD_SHA=${{ needs.changes.outputs.build_sha }} BUILD_DATE=${{ needs.changes.outputs.build_date }} + cache-to: type=registry,ref=${{ env.IMAGE_BACKEND }}:buildcache,mode=max tags: | ${{ env.IMAGE_BACKEND }}:latest ${{ env.IMAGE_BACKEND }}:${{ github.sha }} + - name: Проверить, что tradein-backend:${{ github.sha }} реально в registry (fail-safe, #2841 R2) + # НЕ полагается на семантику steps.build.outcome/continue-on-error раннера — + # проверяет РЕАЛЬНОЕ состояние registry через buildx (уже настроен выше). + # Если act_runner не заполняет outcome, ретрай выше молча НЕ побежит при + # упавшем build — этот шаг единственный это заметит: манифеста с этим SHA + # не будет → шаг падает БЕЗ continue-on-error → job честно FAILURE → deploy + # ниже пропускается вместо накатки старого :latest на прод. + run: docker buildx imagetools inspect ${{ env.IMAGE_BACKEND }}:${{ github.sha }} > /dev/null + - name: Убрать buildx-билдер (#2869 — иначе копятся по одному на прогон) # setup-buildx-action создаёт билдер `docker-container` на КАЖДЫЙ прогон. # Его post-step под Forgejo act_runner не срабатывает, поэтому к 13.08 на @@ -414,8 +426,9 @@ jobs: ${{ env.IMAGE_FRONTEND }}:${{ github.sha }} - name: Retry build & push tradein-frontend без кеша (битый buildcache, #2841) - # См. tradein-backend: cache-to опущен намеренно (следующий успешный - # прогон с кешем перезапишет buildcache-тег целиком и самолечит порчу). + # См. tradein-backend (issue #2, ревью R2): cache-from опущен, cache-to + # ОСТАВЛЕН — успешный ретрай перезаписывает битый buildcache-тег своими + # слоями (mode=max), это и есть самолечение. if: steps.build.outcome == 'failure' uses: docker/build-push-action@v6 with: @@ -427,10 +440,15 @@ jobs: NEXT_PUBLIC_APP_VERSION=${{ needs.changes.outputs.app_version }} NEXT_PUBLIC_BUILD_SHA=${{ needs.changes.outputs.build_sha }} NEXT_PUBLIC_BUILD_DATE=${{ needs.changes.outputs.build_date }} + cache-to: type=registry,ref=${{ env.IMAGE_FRONTEND }}:buildcache,mode=max tags: | ${{ env.IMAGE_FRONTEND }}:latest ${{ env.IMAGE_FRONTEND }}:${{ github.sha }} + - name: Проверить, что tradein-frontend:${{ github.sha }} реально в registry (fail-safe, #2841 R2) + # См. tradein-backend выше — не полагается на steps.build.outcome раннера. + run: docker buildx imagetools inspect ${{ env.IMAGE_FRONTEND }}:${{ github.sha }} > /dev/null + - name: Убрать buildx-билдер (#2869 — иначе копятся по одному на прогон) # setup-buildx-action создаёт билдер `docker-container` на КАЖДЫЙ прогон. # Его post-step под Forgejo act_runner не срабатывает, поэтому к 13.08 на @@ -518,17 +536,23 @@ jobs: ${{ env.IMAGE_BROWSER }}:${{ github.sha }} - name: Retry build & push tradein-browser без кеша (битый buildcache, #2841) - # См. tradein-backend: cache-to опущен намеренно (следующий успешный - # прогон с кешем перезапишет buildcache-тег целиком и самолечит порчу). + # См. tradein-backend (issue #2, ревью R2): cache-from опущен, cache-to + # ОСТАВЛЕН — успешный ретрай перезаписывает битый buildcache-тег своими + # слоями (mode=max), это и есть самолечение. if: steps.build.outcome == 'failure' uses: docker/build-push-action@v6 with: context: ./tradein-mvp/browser push: true + cache-to: type=registry,ref=${{ env.IMAGE_BROWSER }}:buildcache,mode=max tags: | ${{ env.IMAGE_BROWSER }}:latest ${{ env.IMAGE_BROWSER }}:${{ github.sha }} + - name: Проверить, что tradein-browser:${{ github.sha }} реально в registry (fail-safe, #2841 R2) + # См. tradein-backend выше — не полагается на steps.build.outcome раннера. + run: docker buildx imagetools inspect ${{ env.IMAGE_BROWSER }}:${{ github.sha }} > /dev/null + - name: Убрать buildx-билдер (#2869 — иначе копятся по одному на прогон) # setup-buildx-action создаёт билдер `docker-container` на КАЖДЫЙ прогон. # Его post-step под Forgejo act_runner не срабатывает, поэтому к 13.08 на diff --git a/.forgejo/workflows/deploy.yml b/.forgejo/workflows/deploy.yml index 6c40d79d..4485dd82 100644 --- a/.forgejo/workflows/deploy.yml +++ b/.forgejo/workflows/deploy.yml @@ -131,20 +131,35 @@ jobs: ${{ env.IMAGE_BACKEND }}:${{ github.sha }} - name: Retry build & push backend без кеша (битый buildcache, #2841) - # cache-to тоже опущен: следующий успешный прогон С кешем перезапишет - # buildcache-тег целиком (mode=max), это самолечит порчу. Если и retry - # упадёт — шаг красный БЕЗ continue-on-error, job честно FAILURE, и - # deploy ниже корректно пропускается (уже настоящая причина, не кеш). + # cache-from опущен (источник падения), а cache-to ОСТАВЛЕН: успешный + # ретрай пушит свежие слои в buildcache-тег и тем самым сам перезаписывает + # битый blob (mode=max — полная перезапись манифеста). Раньше cache-to был + # опущен и здесь тоже — но следующий обычный прогон опять получает cache-from + # на детерминированно битый тег и падает СНОВА: самолечения не было НИКОГДА + # (ревью #2841 R2, issue #2). Если и retry упадёт — шаг красный БЕЗ + # continue-on-error, job честно FAILURE, и deploy ниже корректно + # пропускается (уже настоящая причина, не кеш). if: steps.build.outcome == 'failure' uses: docker/build-push-action@v6 with: context: ./backend target: runner push: true + cache-to: type=registry,ref=${{ env.IMAGE_BACKEND }}:buildcache,mode=max tags: | ${{ env.IMAGE_BACKEND }}:latest ${{ env.IMAGE_BACKEND }}:${{ github.sha }} + - name: Проверить, что backend:${{ github.sha }} реально в registry (fail-safe, #2841 R2) + # НЕ полагается на семантику steps.build.outcome/continue-on-error раннера — + # проверяет РЕАЛЬНОЕ состояние registry напрямую через buildx (уже настроен + # выше). Если act_runner не заполняет outcome (не проверено живым прогоном, + # см. ревью), ретрай выше молча НЕ побежит при упавшем build, а этот шаг — + # единственный, кто это заметит: манифеста с этим SHA не будет → шаг падает + # БЕЗ continue-on-error → job честно FAILURE → deploy ниже пропускается + # вместо накатки старого :latest на прод. + run: docker buildx imagetools inspect ${{ env.IMAGE_BACKEND }}:${{ github.sha }} > /dev/null + - name: Убрать buildx-билдер (#2869 — иначе копятся по одному на прогон) # setup-buildx-action создаёт билдер `docker-container` на КАЖДЫЙ прогон. # Его post-step под Forgejo act_runner не срабатывает, поэтому к 13.08 на @@ -231,18 +246,27 @@ jobs: ${{ env.IMAGE_WORKER }}:${{ github.sha }} - name: Retry build & push worker без кеша (битый buildcache, #2841) - # См. backend: cache-to опущен намеренно (следующий успешный прогон с - # кешем перезапишет buildcache-тег целиком и самолечит порчу). + # См. backend (issue #2, ревью R2): cache-from опущен, cache-to ОСТАВЛЕН — + # успешный ретрай перезаписывает битый buildcache-тег своими слоями + # (mode=max), это и есть самолечение. Без cache-to здесь порча оставалась + # навсегда — следующий прогон снова падал на том же cache-from. if: steps.build.outcome == 'failure' uses: docker/build-push-action@v6 with: context: ./backend target: runner-with-chromium push: true + cache-to: type=registry,ref=${{ env.IMAGE_WORKER }}:buildcache,mode=max tags: | ${{ env.IMAGE_WORKER }}:latest ${{ env.IMAGE_WORKER }}:${{ github.sha }} + - name: Проверить, что worker:${{ github.sha }} реально в registry (fail-safe, #2841 R2) + # См. backend выше — не полагается на steps.build.outcome раннера, проверяет + # реальное состояние registry, чтобы молча пропущенный ретрай (если outcome + # не поддержан) честно уронил job вместо зелёного прогона с непушнутым образом. + run: docker buildx imagetools inspect ${{ env.IMAGE_WORKER }}:${{ github.sha }} > /dev/null + - name: Убрать buildx-билдер (#2869 — иначе копятся по одному на прогон) # setup-buildx-action создаёт билдер `docker-container` на КАЖДЫЙ прогон. # Его post-step под Forgejo act_runner не срабатывает, поэтому к 13.08 на @@ -331,8 +355,10 @@ jobs: ${{ env.IMAGE_FRONTEND }}:${{ github.sha }} - name: Retry build & push frontend без кеша (битый buildcache, #2841) - # См. backend: cache-to опущен намеренно (следующий успешный прогон с - # кешем перезапишет buildcache-тег целиком и самолечит порчу). + # См. backend (issue #2, ревью R2): cache-from опущен, cache-to ОСТАВЛЕН — + # успешный ретрай перезаписывает битый buildcache-тег своими слоями + # (mode=max), это и есть самолечение. Без cache-to здесь порча оставалась + # навсегда — следующий прогон снова падал на том же cache-from. if: steps.build.outcome == 'failure' uses: docker/build-push-action@v6 with: @@ -341,10 +367,17 @@ jobs: build-args: | NEXT_PUBLIC_GLITCHTIP_DSN=${{ secrets.GLITCHTIP_FRONTEND_DSN }} NEXT_PUBLIC_ENVIRONMENT=production + cache-to: type=registry,ref=${{ env.IMAGE_FRONTEND }}:buildcache,mode=max tags: | ${{ env.IMAGE_FRONTEND }}:latest ${{ env.IMAGE_FRONTEND }}:${{ github.sha }} + - name: Проверить, что frontend:${{ github.sha }} реально в registry (fail-safe, #2841 R2) + # См. backend выше — не полагается на steps.build.outcome раннера, проверяет + # реальное состояние registry, чтобы молча пропущенный ретрай (если outcome + # не поддержан) честно уронил job вместо зелёного прогона с непушнутым образом. + run: docker buildx imagetools inspect ${{ env.IMAGE_FRONTEND }}:${{ github.sha }} > /dev/null + - name: Убрать buildx-билдер (#2869 — иначе копятся по одному на прогон) # setup-buildx-action создаёт билдер `docker-container` на КАЖДЫЙ прогон. # Его post-step под Forgejo act_runner не срабатывает, поэтому к 13.08 на @@ -679,11 +712,25 @@ jobs: docker image prune -af || true docker builder prune -af || true - # Health check + # Health check — деплой ВАЛИТСЯ, если backend не поднялся (см. #2214, + # уже сделано так в deploy-tradein.yml; ревью #2841 R2 issue #3). + # `curl ... && break` под set -e НЕ мог провалить скрипт: curl — не + # последняя команда &&-списка, а POSIX прямо освобождает от errexit + # все команды AND/OR-списка кроме последней. После 30 неуспешных + # попыток цикл завершался кодом последнего sleep (0) — скрипт тихо + # продолжался, деплой уходил success с мёртвым бэкендом. + healthy="" for i in $(seq 1 30); do - curl -fsS http://localhost:8000/health && break + if curl -fsS http://localhost:8000/health >/dev/null 2>&1; then + healthy="yes"; break + fi sleep 1 done + if [ -z "$healthy" ]; then + echo "ERROR: backend не ответил на /health за 30s — деплой FAILED" + exit 1 + fi + echo "→ backend healthy на /health." # Честный итог прогона (#2841). ПРОБЛЕМА: `deploy` пропускается своим `if:` # молча (result=skipped), когда build падает (например, битый blob в From e9ca744e85fd6a5ee7a620b02ad6491f866eb63e Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sat, 15 Aug 2026 18:49:37 +0300 Subject: [PATCH 7/9] =?UTF-8?q?fix(tradein/scrapers):=20=D0=BD=D0=B5=20?= =?UTF-8?q?=D0=BF=D1=83=D1=82=D0=B0=D1=82=D1=8C=20rows=5Finserted/processe?= =?UTF-8?q?d=20=D1=81=20=D1=87=D0=B5=D1=81=D1=82=D0=BD=D1=8B=D0=BC=20?= =?UTF-8?q?=D1=80=D0=B5=D0=B7=D1=83=D0=BB=D1=8C=D1=82=D0=B0=D1=82=D0=BD?= =?UTF-8?q?=D1=8B=D0=BC=20=D0=BA=D0=BB=D1=8E=D1=87=D0=BE=D0=BC?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ревью честного run-status нашло, что _RESULT_COUNTER_KEYS ловил не только целевой yandex_newbuilding_sweep, но и rosreestr_dkp_import (rows_inserted, 66 из 67 прод- прогонов = здоровый ноль догнавшего инкрементального импорта) и newbuilding_enrich (processed — счётчик попыток, ==limit даже при частичном провале). Первое завело бы практически непрерываемый ложный zero-стрик у здорового источника, второе маскировало бы реальные отказы под measured-N. Проверено по прод-БД (2026-08-15): "succeeded" пишут ТОЛЬКО yandex_newbuilding_sweep (42 прогона/90д) и newbuilding_enrich (65/90д) — ни разу rosreestr_dkp_import; у yandex_newbuilding_sweep succeeded численно совпадает с rows_inserted на всех 42/42 прогонах. Заменил "rows_inserted"+"processed" на "succeeded" в _RESULT_COUNTER_KEYS (app-копия и byte-эквивалентная kit-копия) — цель (b) исходной правки сохранена, ложный стрик у rosreestr_dkp_import снят, попутно newbuilding_enrich получает честное измерение вместо счётчика попыток. Также поправлены докстринги test_backfill_honest_status.py — два кейса (76%/72% отказов -> 'done') проверяют только выбор финализатора mark_backfill_finished (mark_done там замокан); реальный mark_done с honest-run-status переквалифицирует их в 'failed' через _failed_ratio_too_high — это не документировалось явно. --- .../backend/app/services/scrape_runs.py | 43 +++++---- .../tests/test_backfill_honest_status.py | 22 ++++- .../test_honest_run_status_failed_ratio.py | 95 ++++++++++++++++--- .../src/scraper_kit/orchestration/runs.py | 43 +++++---- 4 files changed, 155 insertions(+), 48 deletions(-) diff --git a/tradein-mvp/backend/app/services/scrape_runs.py b/tradein-mvp/backend/app/services/scrape_runs.py index ccac68af..6fca052d 100644 --- a/tradein-mvp/backend/app/services/scrape_runs.py +++ b/tradein-mvp/backend/app/services/scrape_runs.py @@ -143,18 +143,28 @@ def _pick_int(counters: Mapping[str, Any], *keys: str) -> int | None: # unique_fetched — full-load'ы avito/cian/yandex (4 источника, 133 прогона) — раньше # сторож их не видел, хотя у cian_full_load 6 из 38 успешных прогонов # реально дали ноль. -# rows_inserted — yandex_newbuilding_sweep (единственный писатель ключа с таким -# именем на верхнем уровне counters): проверено на проде 26.07-10.08 — -# десять прогонов подряд, все 'done', processed=5 succeeded=0 -# rows_inserted=0 failed_resolve=4-5. Ни total_seen/lots_fetched/ -# unique_fetched у него нет, поэтому раньше _run_result_count всегда -# возвращал None ("не измерено") и стрик у сторожа не копился никогда -# (honest-run-status). -# processed — тот же sweep: сколько домов взял в работу. НАМЕРЕННО стоит ПОСЛЕ -# rows_inserted в кортеже — processed это счётчик ПОПЫТОК (аналог -# attempted), а не результата: у него ненулевое значение (=limit) даже -# когда rows_inserted=0, и если бы он читался первым, «5 обработано, -# 0 записано» замаскировалось бы под measured-5, а не measured-0. +# succeeded — yandex_newbuilding_sweep (42 прогона/90д) и newbuilding_enrich +# (65 прогонов/90д, единственные два писателя ключа на проде, +# проверено 2026-08-15). НЕ 'rows_inserted': тот ключ пишет ЕЩЁ и +# rosreestr_dkp_import (67 прогонов/90д) — у него rows_inserted=0 в +# 66 из 67 это ЗДОРОВЫЙ ответ догнавшего инкрементального импорта +# (rows_fetched=rows_skipped=96974, last_id не двигается неделями), +# а не отказ; если бы 'rows_inserted' попал в этот список, сторож +# зачитывал бы этот здоровый ноль как измеренный провал и копил бы +# практически непрерываемый стрик (rosreestr_dkp_import не +# прерывается другим статусом — импорт либо 'done', либо не бежал). +# НЕ 'processed' по той же причине с другой стороны: это счётчик +# ПОПЫТОК (у newbuilding_enrich processed==attempted==limit даже +# когда succeeded меньше — прод-факт 09.08: processed=25 succeeded=14, +# 44% отказов замаскировались бы под measured-25) — сторож нулевого +# результата на нём молчал бы ровно там, где должен сработать, а на +# будущем опустении очереди домов (cian_houses_pending) создал бы +# свой вечный ложный zero-стрик. 'succeeded' у yandex_newbuilding_sweep +# численно совпадает с 'rows_inserted' на всех 42/42 прод-прогонах — +# замена не теряет исходную цель (десять прогонов подряд 26.07-10.08, +# все 'done', succeeded=0 rows_inserted=0 failed_resolve=4-5 — раньше +# ни total_seen/lots_fetched/unique_fetched не было, и +# _run_result_count всегда возвращал None (honest-run-status)). # Сводить сюда счётчики ОСТАЛЬНЫХ задач бессмысленно: на проде 28 источников (2650 # прогонов) не имеют общего результатного ключа вовсе — у каждого свой словарь # (deactivated / rows_written / poi_loaded / snapshotted / upserted / listings_matched @@ -166,8 +176,7 @@ _RESULT_COUNTER_KEYS = ( "total_seen", "lots_fetched", "unique_fetched", - "rows_inserted", - "processed", + "succeeded", ) @@ -368,14 +377,16 @@ def _column_counts(counters: dict[str, int]) -> tuple[int | None, int | None]: Приоритет ключей: - total_seen ← _RESULT_COUNTER_KEYS (total_seen / lots_fetched / unique_fetched / - rows_inserted / processed) + succeeded) - new_count ← 'new_count' / 'lots_inserted' / 'saved_inserted' / 'rows_inserted' (первый присутствующий). 'saved_inserted' — full-load'ы (cian/avito/yandex, CianFullLoadCounters и аналоги в pipeline.py): на проде витрина показывала new_count=0 у трёх подряд cian_full_load при реально сохранённых saved_inserted=482/214/239 (honest-run-status) — ключ 'new_count'/'lots_inserted' у full-load'ов в counters не пишется вовсе. 'rows_inserted' — тот же ключ, - которым yandex_newbuilding_sweep сообщает число upsert'ов. + которым yandex_newbuilding_sweep и rosreestr_dkp_import сообщают число upsert'ов; + здесь (для витринной колонки new_count) это безопасно — в отличие от + _RESULT_COUNTER_KEYS этот список не участвует в подсчёте zero-result-стрика. Возвращает (total_seen, new_count); None для ключа, которого нет в counters — тогда соответствующая колонка не перезаписывается (COALESCE-семантика в UPDATE). diff --git a/tradein-mvp/backend/tests/test_backfill_honest_status.py b/tradein-mvp/backend/tests/test_backfill_honest_status.py index ad72db34..a885b74e 100644 --- a/tradein-mvp/backend/tests/test_backfill_honest_status.py +++ b/tradein-mvp/backend/tests/test_backfill_honest_status.py @@ -5,7 +5,17 @@ 1500-1600 попыток без единого обогащения), yandex 31/52, domclick 24/30 (494 попытки → 0 обогащено, 63 блока, 431 fail — и все 30 'done'). -Проверяем ровно ветвление mark_backfill_finished — БД замокана. +Проверяем ровно ветвление mark_backfill_finished — БД замокана (mark_done/mark_failed/ +mark_banned здесь fake-заглушки, регистрирующие ТОЛЬКО факт вызова). Это значит: кейсы +ниже с высокой долей отказов (attempted=50, failed=38 или 36 — 76%/72%), ожидающие +'done', проверяют лишь то, КАКОЙ финализатор ВЫБРАЛ mark_backfill_finished (#2674: +"обогатили хоть что-то — успех"), а НЕ то, что реально запишет в БД mark_done. С +honest-run-status (2026-08-15) mark_done САМ переквалифицирует такой прогон в 'failed' +через _failed_ratio_too_high (доля отказов >= 0.5) — реальный терминальный статус +для этих двух кейсов на проде теперь 'failed', не 'done'. Это намеренно проверяется +отдельно, БЕЗ мока mark_done, в tests/test_honest_run_status_failed_ratio.py +(test_prod_fact_avito_15_08_no_longer_done и соседние) — не читай эти два кейса как +"76%/72% отказов = 'done' в проде". """ from __future__ import annotations @@ -59,9 +69,15 @@ def _finish(counters: dict[str, int], *, aborted: bool = False) -> tuple[str, st ({"attempted": 5, "enriched": 0, "failed": 5}, False, "failed"), # Кандидатов не было — честная пустота, это успех. ({"attempted": 0, "enriched": 0, "blocked": 0, "failed": 0}, False, "done"), - # Частичный прогон: обогатили хоть что-то → успех. + # Частичный прогон: обогатили хоть что-то → mark_backfill_finished ВЫБИРАЕТ + # mark_done как финализатор (#2674). 76% отказов (38 из 50) — здесь mark_done + # замокан, поэтому статус остаётся 'done'; в реальном mark_done с + # honest-run-status (2026-08-15) это переквалифицируется в 'failed' + # (_failed_ratio_too_high, доля >= 0.5) — см. докстринг модуля. ({"attempted": 50, "enriched": 12, "blocked": 0, "failed": 38}, False, "done"), - # Блоки были, но прогон доработал и обогатил — не бан. + # Блоки были, но прогон доработал и обогатил — mark_backfill_finished выбирает + # НЕ 'banned'. 72% отказов (36 из 50) — та же оговорка: реальный mark_done + # переквалифицирует в 'failed', см. докстринг модуля выше. ({"attempted": 50, "enriched": 12, "blocked": 2, "failed": 36}, False, "done"), # Блок оборвал прогон, хотя часть успели обогатить — работа не доделана. ({"attempted": 50, "enriched": 12, "blocked": 5, "failed": 33}, True, "banned"), diff --git a/tradein-mvp/backend/tests/test_honest_run_status_failed_ratio.py b/tradein-mvp/backend/tests/test_honest_run_status_failed_ratio.py index b8c03639..c32e4609 100644 --- a/tradein-mvp/backend/tests/test_honest_run_status_failed_ratio.py +++ b/tradein-mvp/backend/tests/test_honest_run_status_failed_ratio.py @@ -12,8 +12,15 @@ processed=5, succeeded=0, rows_inserted=0, failed_resolve=4-5 — сторож нулевого результата (_alert_if_consecutive_zero_results) слеп, т.к. _RESULT_COUNTER_KEYS не знал ни одного ключа этого sweep'а (total_seen/lots_fetched/unique_fetched). - Фикс: _RESULT_COUNTER_KEYS дополнен rows_inserted/processed (в этом порядке — - rows_inserted это РЕЗУЛЬТАТ, processed это ПОПЫТКИ). + Фикс: _RESULT_COUNTER_KEYS дополнен 'succeeded'. Первая версия правки добавляла + голые 'rows_inserted'/'processed' — ревью нашло, что 'rows_inserted' пишет ЕЩЁ + rosreestr_dkp_import (66/67 прод-прогонов, здоровый ноль догнавшего импорта, а не + отказ) и завёл бы непрерываемый ложный zero-стрик, а 'processed' — счётчик + попыток (==limit даже при частичном провале у newbuilding_enrich) и маскирует + реальные отказы. 'succeeded' пишут только yandex_newbuilding_sweep и + newbuilding_enrich, численно совпадает с прежним 'rows_inserted' на всех + прод-прогонах sweep'а — см. test_rosreestr_dkp_import_healthy_zero_stays_unmeasured + и test_newbuilding_enrich_partial_failure_not_masked_by_processed ниже. (c) admin-витрина показывала new_count=0 у трёх подряд cian_full_load, хотя реально сохранено saved_inserted=482/214/239 — full-load'ы не пишут ни 'new_count', ни @@ -180,7 +187,8 @@ def test_honest_empty_sweep_unaffected_by_failed_ratio(name: str) -> None: def test_prod_fact_yandex_newbuilding_sweep_measured_as_zero() -> None: """processed=5, succeeded=0, rows_inserted=0, failed_resolve=4 — раньше - _run_result_count возвращал None ("не измерено"); теперь — измеренный 0.""" + _run_result_count возвращал None ("не измерено"); теперь — измеренный 0 (через + 'succeeded', не 'rows_inserted' — см. ниже, почему ключ переигран ревью).""" counters = { "total": 309, "fetchable": 200, @@ -198,23 +206,61 @@ def test_prod_fact_yandex_newbuilding_sweep_measured_as_zero() -> None: assert kit_runs._run_result_count(counters) == 0 -def test_rows_inserted_takes_priority_over_processed() -> None: - """rows_inserted (результат) читается ПЕРЕД processed (попытки) — иначе "5 - обработано, 0 записано" замаскировалось бы под measured-5.""" +def test_succeeded_is_the_measured_key_not_rows_inserted_or_processed() -> None: + """'succeeded' читается как результат; голые 'rows_inserted'/'processed' в + _RESULT_COUNTER_KEYS больше не участвуют (были в первой версии правки, снято + ревью — см. test_rosreestr_dkp_import_healthy_zero_stays_unmeasured и + test_newbuilding_enrich_partial_failure_not_masked_by_processed ниже).""" counters = {"processed": 5, "rows_inserted": 0} - assert app_runs._run_result_count(counters) == 0 + assert app_runs._run_result_count(counters) is None + assert kit_runs._run_result_count(counters) is None -def test_processed_is_fallback_when_rows_inserted_absent() -> None: - counters = {"processed": 3} - assert app_runs._run_result_count(counters) == 3 +def test_rosreestr_dkp_import_healthy_zero_stays_unmeasured() -> None: + """Прод-факт rosreestr_dkp_import (2026-08-15, 66 из 67 прогонов за 90д): инкрементальный + импорт догнал источник — rows_fetched==rows_skipped, rows_inserted=0. Это ЗДОРОВЫЙ + ответ (нечего вставлять), а не отказ; словарь не содержит 'succeeded' вовсе. + + Первая версия правки добавляла голый 'rows_inserted' в _RESULT_COUNTER_KEYS — тогда + этот прод-факт читался бы как "измеренный провал" и копил бы практически + непрерываемый zero-стрик (rosreestr_dkp_import не прерывается другим статусом: + он либо 'done' с этим же нулём, либо не бежал). Ревью поймало это до деплоя — + правильный ответ: "не измерено" (None), стрик не копится.""" + counters = { + "last_id": 6829903, + "batches_done": 49, + "rows_errored": 0, + "rows_fetched": 96974, + "rows_skipped": 96974, + "rows_updated": 0, + "rows_inserted": 0, + } + assert app_runs._run_result_count(counters) is None + assert kit_runs._run_result_count(counters) is None + + +def test_newbuilding_enrich_partial_failure_not_masked_by_processed() -> None: + """Прод-факт newbuilding_enrich (09.08): processed=25 (счётчик ПОПЫТОК, ==limit), + succeeded=14 — 44% отказов. Если бы сторож читал 'processed' как результат, партиальный + провал замаскировался бы под measured-25 (сторож нулевого результата промолчал бы + ровно там, где должен был сработать при полном провале). 'succeeded' даёт честные 14.""" + counters = { + "failed": 11, + "enriched": 14, + "attempted": 25, + "processed": 25, + "succeeded": 14, + "failed_fetch": 11, + } + assert app_runs._run_result_count(counters) == 14 + assert kit_runs._run_result_count(counters) == 14 @pytest.mark.parametrize("name", list(_MODULES)) def test_zero_result_watchdog_now_fires_for_newbuilding_sweep_streak(name: str) -> None: - """(b) integration: 3 подряд yandex_newbuilding_sweep-подобных 'done' с - rows_inserted=0 -> алерт срабатывает. До фикса _RESULT_COUNTER_KEYS сторож считал - результат "не измеренным" и молчал бы вечно (см. #2703 в docstring модуля).""" + """(b) integration: 3 подряд yandex_newbuilding_sweep-подобных 'done' с succeeded=0 + -> алерт срабатывает. До фикса _RESULT_COUNTER_KEYS сторож считал результат "не + измеренным" и молчал бы вечно (см. #2703 в docstring модуля).""" mod = _MODULES[name] row = MagicMock() row.status = "done" @@ -228,6 +274,29 @@ def test_zero_result_watchdog_now_fires_for_newbuilding_sweep_streak(name: str) mock_sentry.capture_message.assert_called_once() +@pytest.mark.parametrize("name", list(_MODULES)) +def test_zero_result_watchdog_silent_on_rosreestr_dkp_import_streak(name: str) -> None: + """Негативный аналог теста выше: та же лестница из 3 подряд 'done', но словарь + rosreestr_dkp_import (нет 'succeeded') -> сторож не считает результат измеренным + и НЕ шлёт алерт — регрессионный тест на замечание ревью (HIGH #1).""" + mod = _MODULES[name] + row = MagicMock() + row.status = "done" + row.counters = { + "last_id": 6829903, + "rows_fetched": 96974, + "rows_skipped": 96974, + "rows_inserted": 0, + } + db = MagicMock() + result = MagicMock() + result.fetchall.return_value = [row, row, row] + db.execute.return_value = result + with patch.object(mod, "sentry_sdk") as mock_sentry: + mod._alert_if_consecutive_zero_results(db, "rosreestr_dkp_import") + mock_sentry.capture_message.assert_not_called() + + # ── (c) _column_counts: прод-факт cian_full_load new_count=0 при saved_inserted>0 ─── diff --git a/tradein-mvp/packages/scraper-kit/src/scraper_kit/orchestration/runs.py b/tradein-mvp/packages/scraper-kit/src/scraper_kit/orchestration/runs.py index 279ea928..725b1613 100644 --- a/tradein-mvp/packages/scraper-kit/src/scraper_kit/orchestration/runs.py +++ b/tradein-mvp/packages/scraper-kit/src/scraper_kit/orchestration/runs.py @@ -138,18 +138,28 @@ def _pick_int(counters: Mapping[str, Any], *keys: str) -> int | None: # unique_fetched — full-load'ы avito/cian/yandex (4 источника, 133 прогона) — раньше # сторож их не видел, хотя у cian_full_load 6 из 38 успешных прогонов # реально дали ноль. -# rows_inserted — yandex_newbuilding_sweep (единственный писатель ключа с таким -# именем на верхнем уровне counters): проверено на проде 26.07-10.08 — -# десять прогонов подряд, все 'done', processed=5 succeeded=0 -# rows_inserted=0 failed_resolve=4-5. Ни total_seen/lots_fetched/ -# unique_fetched у него нет, поэтому раньше _run_result_count всегда -# возвращал None ("не измерено") и стрик у сторожа не копился никогда -# (honest-run-status). -# processed — тот же sweep: сколько домов взял в работу. НАМЕРЕННО стоит ПОСЛЕ -# rows_inserted в кортеже — processed это счётчик ПОПЫТОК (аналог -# attempted), а не результата: у него ненулевое значение (=limit) даже -# когда rows_inserted=0, и если бы он читался первым, «5 обработано, -# 0 записано» замаскировалось бы под measured-5, а не measured-0. +# succeeded — yandex_newbuilding_sweep (42 прогона/90д) и newbuilding_enrich +# (65 прогонов/90д, единственные два писателя ключа на проде, +# проверено 2026-08-15). НЕ 'rows_inserted': тот ключ пишет ЕЩЁ и +# rosreestr_dkp_import (67 прогонов/90д) — у него rows_inserted=0 в +# 66 из 67 это ЗДОРОВЫЙ ответ догнавшего инкрементального импорта +# (rows_fetched=rows_skipped=96974, last_id не двигается неделями), +# а не отказ; если бы 'rows_inserted' попал в этот список, сторож +# зачитывал бы этот здоровый ноль как измеренный провал и копил бы +# практически непрерываемый стрик (rosreestr_dkp_import не +# прерывается другим статусом — импорт либо 'done', либо не бежал). +# НЕ 'processed' по той же причине с другой стороны: это счётчик +# ПОПЫТОК (у newbuilding_enrich processed==attempted==limit даже +# когда succeeded меньше — прод-факт 09.08: processed=25 succeeded=14, +# 44% отказов замаскировались бы под measured-25) — сторож нулевого +# результата на нём молчал бы ровно там, где должен сработать, а на +# будущем опустении очереди домов (cian_houses_pending) создал бы +# свой вечный ложный zero-стрик. 'succeeded' у yandex_newbuilding_sweep +# численно совпадает с 'rows_inserted' на всех 42/42 прод-прогонах — +# замена не теряет исходную цель (десять прогонов подряд 26.07-10.08, +# все 'done', succeeded=0 rows_inserted=0 failed_resolve=4-5 — раньше +# ни total_seen/lots_fetched/unique_fetched не было, и +# _run_result_count всегда возвращал None (honest-run-status)). # Сводить сюда счётчики ОСТАЛЬНЫХ задач бессмысленно: на проде 28 источников (2650 # прогонов) не имеют общего результатного ключа вовсе — у каждого свой словарь # (deactivated / rows_written / poi_loaded / snapshotted / upserted / listings_matched @@ -161,8 +171,7 @@ _RESULT_COUNTER_KEYS = ( "total_seen", "lots_fetched", "unique_fetched", - "rows_inserted", - "processed", + "succeeded", ) @@ -368,14 +377,16 @@ def _column_counts(counters: dict[str, int]) -> tuple[int | None, int | None]: Приоритет ключей: - total_seen ← _RESULT_COUNTER_KEYS (total_seen / lots_fetched / unique_fetched / - rows_inserted / processed) + succeeded) - new_count ← 'new_count' / 'lots_inserted' / 'saved_inserted' / 'rows_inserted' (первый присутствующий). 'saved_inserted' — full-load'ы (cian/avito/yandex, CianFullLoadCounters и аналоги в pipeline.py): на проде витрина показывала new_count=0 у трёх подряд cian_full_load при реально сохранённых saved_inserted=482/214/239 (honest-run-status) — ключ 'new_count'/'lots_inserted' у full-load'ов в counters не пишется вовсе. 'rows_inserted' — тот же ключ, - которым yandex_newbuilding_sweep сообщает число upsert'ов. + которым yandex_newbuilding_sweep и rosreestr_dkp_import сообщают число upsert'ов; + здесь (для витринной колонки new_count) это безопасно — в отличие от + _RESULT_COUNTER_KEYS этот список не участвует в подсчёте zero-result-стрика. Возвращает (total_seen, new_count); None для ключа, которого нет в counters — тогда соответствующая колонка не перезаписывается (COALESCE-семантика в UPDATE). From fa84705ec71646b15105eb71da90545692f5140c Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sat, 15 Aug 2026 18:50:17 +0300 Subject: [PATCH 8/9] fix(tradein/geocoder): stop apt number leaking into house + houses bbox/sibling guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 2 on #2626 (local houses fallback) found two HIGH-severity bugs verified live against prod data: 1. _extract_local_house_token took the LAST digit-like token in the raw address, so "...Педагогическая, д 15, кв 11" resolved house=11 (apartment number) instead of 15 -- confidently returning a stranger's building with confidence='exact', written to geocode_cache. Fixed by stripping the apartment/office/floor/entrance tail (кв/оф/пом/подъезд/этаж -- NOT корп/к, which is part of the house number) before extracting the token. Fixes the exact prod case from the review plus the corpus+apartment combo ("д 26 к 1, кв 41" -> 26к1, not 41). 2. houses is not an EKB-only table (21% of rows with coords are outside the metro, some as far as another city) -- "улица Маяковского, 7" in houses resolves to Серов, not Екатеринбург, and use_local_ekb only gates the user's query text, not the source row. Added an is_within_ekb_bbox_wide check on every candidate row before it can become a match. Also addressed two MEDIUM findings from the same review: 3. The "<номер> -> <номер>к1" corpus guess only checked uniqueness among к1-labelled rows, so real multi-building addresses (Онуфриева 24: к1/к2/к3, 250-400m apart) resolved confidently to к1 anyway. Guess is now skipped when any other corpus/slash variant of the same base number exists among the street's candidates. 4. Houses-fallback results are no longer cached in geocode_cache -- the source (scraped listings) is less reliable than geoportal/cadastral/ Nominatim, and the lookup is cheap/local, so caching only extended the lifetime of a possible bad match. Side benefit: address_refined now survives every repeat request of the same raw address, not just the first. Also added ORDER BY address, id to the underlying query so the coordinate dedup picks a deterministic row (LOW finding #5). 14 new/updated tests in test_geocoder_local_houses_fallback.py cover all five findings against real prod address/houses-row fixtures. Full geocoder + dadata + estimator/pdf regression suite (402 tests) green. --- tradein-mvp/backend/app/services/geocoder.py | 132 ++++++++++++--- .../test_geocoder_local_houses_fallback.py | 154 +++++++++++++++++- 2 files changed, 256 insertions(+), 30 deletions(-) diff --git a/tradein-mvp/backend/app/services/geocoder.py b/tradein-mvp/backend/app/services/geocoder.py index 8d798476..420728dd 100644 --- a/tradein-mvp/backend/app/services/geocoder.py +++ b/tradein-mvp/backend/app/services/geocoder.py @@ -51,11 +51,14 @@ class GeocodeResult: # корпус («49» вместо реального «49к1») — houses-фолбэк нашёл ОДНОЗНАЧНЫЙ дом по # нормализованному совпадению. Честный сигнал вызывающему коду «адрес уточнён # автоматически», НЕ эвристика на корректность — см. `geocode()`/`_local_houses_match`. - # Известный предел: `geocode_cache` НЕ хранит этот флаг (схему не трогаем) — - # на повторный запрос ТОГО ЖЕ сырого адреса из кэша координаты корректные, но - # `address_refined` вернётся `False` (та же судьба у `city_ambiguous` при - # cache-hit — см. `_geocode_resolve`, восстанавливается `replace()` из - # текущего вызова, а не из кэша). + # Houses-фолбэк НЕ пишет свой результат в `geocode_cache` (менее надёжный + # источник координат, чем geoportal/cadastral/Nominatim — #2626 review R2 #4), + # поэтому этот сигнал переживает КАЖДЫЙ повторный запрос того же сырого + # адреса. `geocode_cache` вообще не хранит этот флаг (схему не трогаем) — + # если бы houses-хит когда-нибудь попал в кэш, на cache-hit `address_refined` + # вернулся бы `False` (та же судьба у `city_ambiguous` при cache-hit — см. + # `_geocode_resolve`, восстанавливается `replace()` из текущего вызова, а не + # из кэша). address_refined: bool = False @@ -1361,13 +1364,33 @@ def _norm_local_house(raw: str) -> str: return s +# Хвостовой мусор ПОСЛЕ номера дома — квартира/офис/помещение/подъезд/этаж. +# НЕ включает «корп/корпус/к» (в отличие от `_RE_APT_TAIL` выше) — корпус тут +# ЧАСТЬ номера дома, который должен остаться видимым для `_LOCAL_HOUSE_TOKEN_RE` +# («49к1», «26 к 1» — корпус нельзя терять). Без этой зачистки +# `_extract_local_house_token` (берёт ПОСЛЕДНЕЕ число в строке) находит номер +# квартиры/этажа вместо дома — прод-баг #2626 review R2 #1: «...Педагогическая, +# д 15, кв 11» отдавал дом «11» (координаты ЧУЖОГО здания) вместо «15». +_RE_LOCAL_APT_TAIL = re.compile( + r"[,\s]\s*(?:кв|квартира|оф|офис|пом|помещение|лит|подъезд|этаж)\.?\s*\d.*$", + re.IGNORECASE, +) + + def _extract_local_house_token(address: str) -> str | None: """Номер дома из ПОЛЬЗОВАТЕЛЬСКОГО адреса — с учётом «/N» и «корпус N» хвостов, которые `_parse_street_house`/`_HOUSE_NUM` обрезают (см. коммент у `_LOCAL_HOUSE_TOKEN_RE`). Берём ПОСЛЕДНЕЕ совпадение — номер дома в русском адресе почти всегда в хвосте строки. None, если цифр нет вовсе. + + Квартирный/этажный/подъездный хвост зачищается ДО поиска номера + (`_RE_LOCAL_APT_TAIL`) — иначе «последнее число в строке» это номер + квартиры/этажа, а не дома (см. докстринг у `_RE_LOCAL_APT_TAIL`). """ s = _RE_POSTAL.sub(" ", " ".join(address.lower().strip().split())).strip(" ,.") + if not s: + return None + s = _RE_LOCAL_APT_TAIL.sub(" ", s).strip(" ,.") if not s: return None matches = list(_LOCAL_HOUSE_TOKEN_RE.finditer(s)) @@ -1431,25 +1454,53 @@ def _street_tail_matches(row_street_norm: str, query_street_norm: str) -> bool: return row_street_norm == query_street_norm or row_street_norm.endswith(" " + query_street_norm) +# «24к1» → «24» (базовый номер варианта с корпусом/слэшем); «44» (голый номер, +# без суффикса) → None. Используется ТОЛЬКО для sibling-guard (см. ниже) — +# отличить «этот дом однозначно к1» от «этого дома несколько корпусов, а у +# нас в вводе просто нет данных, какой именно». +_LOCAL_HOUSE_VARIANT_BASE_RE = re.compile(r"^(\d+)(?:к\d+|/\d+)$") + + def _local_houses_match(db: Session, street: str, house: str) -> GeocodeSuggestion | None: """Последний локальный тир `geocode()` (#2626) — fallback на `houses` (скрейпленные листинги avito/cian/derived/yandex, own DB table, БЕЗ FDW). Вызывается ТОЛЬКО когда geoportal/cadastral/Nominatim уже не дали результата. - Два независимых допущения, оба defensive (при неоднозначности — None, не гадаем): + Допущения, все defensive (при неоднозначности — None, не гадаем): 1. Улица матчится «по хвосту» (`_street_tail_matches`) — ловит расхождение разговорного/сокращённого имени («Онуфриева») и канонического ГАР-имени в houses («Начдива Онуфриева»). - 2. Номер дома — сперва точное совпадение; нет — пробуем `<номер>к1` (частый - случай: пользователь ввёл «49», у дома есть только корпус «49к1»). ЛЮБОЙ - шаг, где кандидатов больше одного (после дедупа по координатам — разные - source-строки ОДНОГО дома не в счёт), возвращает None — угадывать нельзя. + 2. Координаты строки-кандидата обязаны лежать в широком ЕКБ-bbox + (`is_within_ekb_bbox_wide`) — `houses` НЕ ЕКБ-only реестр (в отличие от + geoportal/cad_buildings): 21% строк с координатами лежат вне области ЕКБ, + местами вплоть до другого региона (#2626 review R2 #2 — прод-пример + «улица Маяковского, 7» в houses это Серов, а не запрошенный + Екатеринбург). `use_local_ekb` в `geocode()` гейтит только ЗАПРОС + пользователя, не страхует от грязной строки-источника. + 3. Номер дома — сперва точное совпадение; нет — пробуем `<номер>к1` (частый + случай: пользователь ввёл «49», у дома есть только корпус «49к1»), но + ТОЛЬКО если среди кандидатов улицы НЕТ других корпусов/дробей этого же + номера («24к2», «24/2» и т.п.) — иначе «к1» такая же угадайка, как и + любой другой корпус, и реальные дома могут быть в 250-400м друг от друга + (#2626 review R2 #3, прод-пример «Начдива Онуфриева, 24»: 24к1/24к2/24к3 + — три разных здания). + 4. ЛЮБОЙ шаг, где кандидатов больше одного (после дедупа по округлённым + координатам — разные source-строки ОДНОГО дома не в счёт), возвращает + None — угадывать нельзя. SQL — дешёвый ILIKE-префильтр по последнему слову улицы (нет индекса на - `houses.address`, но тир последний и редкий — не на каждый запрос), вся - точная логика (суффикс улицы + равенство номера) — в Python, что и делает - её юнит-тестируемой без реальной БД (см. `test_geocoder_local_houses_fallback.py`). + `houses.address`, но тир последний и редкий — не на каждый запрос) с + детерминированным ORDER BY (дедуп по координатам иначе непредсказуемо + выбирал бы, какая из двух ~идентичных source-строк станет ответом — + #2626 review R2 #5); вся точная логика (суффикс улицы, bbox, равенство + номера) — в Python, что и делает её юнит-тестируемой без реальной БД + (см. `test_geocoder_local_houses_fallback.py`). + + Результат этого тира НЕ кэшируется в `geocode_cache` вызывающей стороной + (см. `geocode()`) — `houses`-координаты из скрейпленных объявлений менее + надёжны, чем geoportal/cadastral/Nominatim, а сам lookup дешёвый и локальный + (#2626 review R2 #4). """ query_street_norm = _clean_local_house_street(street) if not query_street_norm: @@ -1466,6 +1517,7 @@ def _local_houses_match(db: Session, street: str, house: str) -> GeocodeSuggesti FROM houses WHERE address ILIKE CAST('%' || :w || '%' AS text) AND lat IS NOT NULL AND lon IS NOT NULL + ORDER BY address, id """), {"w": last_word}, ).fetchall() @@ -1478,23 +1530,32 @@ def _local_houses_match(db: Session, street: str, house: str) -> GeocodeSuggesti ) return None + # Street-tail + bbox фильтр — один проход, дальше переиспользуется и для + # точного совпадения, и для corpus-1 догадки, и для sibling-guard. + street_rows: list[tuple[str, float, float, str]] = [] # (house_norm, lat, lon, addr) + for r in rows: + parsed = _row_local_house(str(r.address or "")) + if parsed is None: + continue + row_street_norm, row_house_norm = parsed + if not _street_tail_matches(row_street_norm, query_street_norm): + continue + lat, lon = float(r.lat), float(r.lon) + if not is_within_ekb_bbox_wide(lat, lon): + continue + street_rows.append((row_house_norm, lat, lon, str(r.address))) + def _candidates(house_norm: str) -> list[tuple[str, float, float]]: out: list[tuple[str, float, float]] = [] seen_coords: set[tuple[float, float]] = set() - for r in rows: - parsed = _row_local_house(str(r.address or "")) - if parsed is None: - continue - row_street_norm, row_house_norm = parsed + for row_house_norm, lat, lon, addr in street_rows: if row_house_norm != house_norm: continue - if not _street_tail_matches(row_street_norm, query_street_norm): - continue - coord_key = (round(float(r.lat), 4), round(float(r.lon), 4)) # ~11m — дедуп источников + coord_key = (round(lat, 4), round(lon, 4)) # ~11m — дедуп источников if coord_key in seen_coords: continue seen_coords.add(coord_key) - out.append((str(r.address), float(r.lat), float(r.lon))) + out.append((addr, lat, lon)) return out exact = _candidates(query_house_norm) @@ -1514,6 +1575,21 @@ def _local_houses_match(db: Session, street: str, house: str) -> GeocodeSuggesti # если запрошенный номер — голое число (не пытаемся достраивать «49/2» → «49/2к1»). if query_house_norm.isdigit(): corpus1 = f"{query_house_norm}к1" + siblings = { + row_house_norm + for row_house_norm, _lat, _lon, _addr in street_rows + if row_house_norm != corpus1 + and (m := _LOCAL_HOUSE_VARIANT_BASE_RE.match(row_house_norm)) is not None + and m.group(1) == query_house_norm + } + if siblings: + logger.info( + "local houses fallback: корпус-1 %r неоднозначен — есть другие " + "корпуса/дроби %s — skip", + corpus1, + sorted(siblings), + ) + return None guessed = _candidates(corpus1) if len(guessed) == 1: addr, lat, lon = guessed[0] @@ -1810,7 +1886,10 @@ async def _geocode_resolve( # резолвился Nominatim'ом (разговорное/усечённое имя улицы или отсутствующий # в вводе корпус). См. `_local_houses_match`. EKB-only гейт — тот же, что у # geoportal/cadastral (houses — преимущественно ЕКБ-трафик, тот же риск - # коллизии улица+дом с другим городом региона, что и мотивировал #2582). + # коллизии улица+дом с другим городом региона, что и мотивировал #2582); + # координаты строки-кандидата ДОПОЛНИТЕЛЬНО проверяются bbox-ом внутри + # `_local_houses_match` (гейт здесь фильтрует только запрос пользователя, + # не грязь в самой таблице — #2626 review R2 #2). if use_local_ekb and parsed is not None: local_street, _parsed_house = parsed local_house = _extract_local_house_token(address) or _parsed_house @@ -1825,7 +1904,12 @@ async def _geocode_resolve( city_ambiguous=city_ambiguous, address_refined=True, ) - await asyncio.to_thread(_cache_put, db, addr_norm, result) + # НЕ кэшируем: houses-координаты (скрейпленные листинги) менее + # надёжны, чем geoportal/cadastral/Nominatim, а сам lookup дешёвый + # и локальный — кэш только продлевал бы жизнь возможной ошибке + # источника (#2626 review R2 #4). Побочный эффект: `address_refined` + # переживает КАЖДЫЙ повторный запрос этого сырого адреса, а не + # только первый (было известным пределом до этого фикса). logger.info( "geocode local houses fallback: %s → (%.5f, %.5f) [%s]", addr_norm, diff --git a/tradein-mvp/backend/tests/test_geocoder_local_houses_fallback.py b/tradein-mvp/backend/tests/test_geocoder_local_houses_fallback.py index fef919a4..d28941c2 100644 --- a/tradein-mvp/backend/tests/test_geocoder_local_houses_fallback.py +++ b/tradein-mvp/backend/tests/test_geocoder_local_houses_fallback.py @@ -83,6 +83,32 @@ def test_norm_local_house(raw: str, expected: str) -> None: ("Крестинского 49к1", "49к1"), ("8 Марта 204", "204"), # digit-leading street name doesn't confuse it ("Малышева 30", "30"), + # #2626 review R2 #1 — прод-баг: квартира подменяла дом («д 15, кв 11» + # → дом «11», чужое здание). Реальные строки из trade_in_estimates: + ( + "620078, Свердловская обл, г Екатеринбург, Кировский р-н, " + "ул Педагогическая, д 15, кв 11", + "15", + ), + ( + "620078, Свердловская обл, г Екатеринбург, Кировский р-н, " + "ул Педагогическая, д 15, кв 48", + "15", + ), + # корпус ПЕРЕД квартирой — «26 к 1» обязан остаться частью номера дома, + # «кв 41» — уйти: + ( + "620149, Свердловская обл, г Екатеринбург, Ленинский р-н, " + "ул Начдива Онуфриева, д 26 к 1, кв 41", + "26к1", + ), + # подъезд/этаж — тот же класс бага, что и квартира (последнее число в + # строке — не дом): + ( + "Россия, Свердловская область, Екатеринбург, Трамвайный переулок, " + "2к2, подъезд 1, этаж 25, кв. 205", + "2к2", + ), ], ) def test_extract_local_house_token(address: str, expected: str) -> None: @@ -186,7 +212,30 @@ def test_local_houses_match_exact_house_number() -> None: def test_local_houses_match_street_tail_and_corpus1_guess() -> None: - """«Онуфриева, 24» (без «Начдива», без корпуса) → единственный «24к1» реестра.""" + """«Онуфриева, 24» (без «Начдива», без корпуса), реестр — ЕДИНСТВЕННЫЙ + корпус «24к1» → уверенная догадка (нет sibling-корпусов — не угадайка).""" + db = _db_with_rows( + [ + _make_row( + "р-н Ленинский, мкр. Юго-Западный, улица Начдива Онуфриева, 24к1", + 56.802928, + 60.551696, + ), + ] + ) + + hit = _local_houses_match(db, "онуфриева", "24") + + assert hit is not None + assert hit.lat == pytest.approx(56.802928) + assert hit.lon == pytest.approx(60.551696) + + +def test_local_houses_match_corpus1_guess_skipped_when_sibling_corpus_exists() -> None: + """#2626 review R2 #3, прод-данные: «Начдива Онуфриева, 24» реально ТРИ + разных здания (24к1/24к2/24к3, 250-400м друг от друга). Догадка «→24к1» + не угадывает конкретное здание среди known-siblings — честный None, не + «уверенный» результат с confidence='exact' на случайно выбранном доме.""" db = _db_with_rows( [ _make_row( @@ -199,11 +248,19 @@ def test_local_houses_match_street_tail_and_corpus1_guess() -> None: ] ) - hit = _local_houses_match(db, "онуфриева", "24") + assert _local_houses_match(db, "онуфриева", "24") is None - assert hit is not None - assert hit.lat == pytest.approx(56.802928) - assert hit.lon == pytest.approx(60.551696) + +def test_local_houses_match_corpus1_guess_skipped_when_slash_sibling_exists() -> None: + """Sibling-guard ловит не только «кN», но и «/N» вариант того же номера.""" + db = _db_with_rows( + [ + _make_row("улица X, 24к1", 56.80, 60.60), + _make_row("улица X, 24/2", 56.81, 60.61), + ] + ) + + assert _local_houses_match(db, "x", "24") is None def test_local_houses_match_no_corpus1_candidate_returns_none() -> None: @@ -278,6 +335,50 @@ def test_local_houses_match_returns_none_on_db_error() -> None: assert _local_houses_match(db, "онуфриева", "24") is None +# ── bbox guard: `houses` is NOT EKB-only (#2626 review R2 #2) ─────────────── + + +def test_local_houses_match_rejects_row_outside_ekb_bbox() -> None: + """Прод-кейс: «улица Маяковского, 7» в `houses` — это Серов (56.6/60.66 — + ~310км от ЕКБ), не Екатеринбург. `use_local_ekb` в `geocode()` гейтит только + ЗАПРОС пользователя, не координаты строки-источника — bbox-фильтр внутри + `_local_houses_match` обязан отбросить такую строку, а не вернуть её как + confidence='exact' совпадение чужого города.""" + db = _db_with_rows( + [_make_row("улица Маяковского, 7", 59.652903, 60.659674)], # Серов, не ЕКБ + ) + + assert _local_houses_match(db, "маяковского", "7") is None + + +def test_local_houses_match_accepts_row_inside_ekb_bbox_wide() -> None: + """Контроль: легитимная ЕКБ-строка (в т.ч. приграничье, в WIDE, не в TIGHT) + по-прежнему проходит — bbox-фильтр не режет реальные ЕКБ-дома.""" + db = _db_with_rows( + [_make_row("Екатеринбург, улица Маяковского, 8", 56.862701, 60.620274)], + ) + + hit = _local_houses_match(db, "маяковского", "8") + + assert hit is not None + assert hit.lat == pytest.approx(56.862701) + + +# ── deterministic ORDER BY (#2626 review R2 #5) ────────────────────────────── + + +def test_local_houses_match_query_has_deterministic_order_by() -> None: + """Без ORDER BY дедуп по округлённым координатам оставлял бы ПЕРВУЮ строку + в порядке сканирования — недетерминированно между вызовами. SQL обязан + сортировать явно.""" + db = _db_with_rows([]) + + _local_houses_match(db, "x", "1") + + sql_text = str(db.execute.call_args[0][0]) + assert "ORDER BY" in sql_text.upper() + + # ── geocode() wiring — last-resort tier, sets address_refined ─────────────── @@ -298,7 +399,7 @@ async def test_geocode_falls_back_to_local_houses_after_nominatim_miss() -> None patch("app.services.geocoder._geoportal_house_match", return_value=None), patch("app.services.geocoder._cadastral_house_match", return_value=None), patch("app.services.geocoder._cadastral_forward_sync", return_value=[]), - patch("app.services.geocoder._cache_put"), + patch("app.services.geocoder._cache_put") as mock_cache_put, patch( "app.services.geocoder._nominatim_lookup", new_callable=AsyncMock, @@ -316,6 +417,9 @@ async def test_geocode_falls_back_to_local_houses_after_nominatim_miss() -> None assert result.confidence == "exact" assert result.address_refined is True mock_local.assert_called_once() + # #2626 review R2 #4 — houses-фолбэк дешёвый и менее надёжный источник + # координат, чем geoportal/cadastral/Nominatim — свой результат не кэширует. + mock_cache_put.assert_not_called() async def test_geocode_address_refined_false_when_earlier_tier_hits() -> None: @@ -366,3 +470,41 @@ async def test_geocode_returns_none_when_local_houses_also_misses() -> None: assert result is None mock_local.assert_called_once() + + +async def test_geocode_local_houses_apartment_number_does_not_leak_into_house() -> None: + """End-to-end regression, #2626 review R2 #1: реальный прод-адрес с хвостом + «кв 11» должен резолвиться в дом 15 (`Педагогическая ул.,15`), а НЕ в дом 11 + (`Педагогическая ул.,11` — чужое здание) — `_local_houses_match` не + замокан, проверяем полную цепочку `geocode()` → `_extract_local_house_token` + → SQL-lookup.""" + db = _db_with_rows( + [ + _make_row("Педагогическая ул.,11", 56.835387, 60.654104), + _make_row("Педагогическая ул.,15", 56.835284, 60.655829), + ] + ) + + with ( + patch("app.services.geocoder._cache_get", return_value=None), + patch("app.services.geocoder._geoportal_house_match", return_value=None), + patch("app.services.geocoder._cadastral_house_match", return_value=None), + patch("app.services.geocoder._cadastral_forward_sync", return_value=[]), + patch("app.services.geocoder._cache_put") as mock_cache_put, + patch( + "app.services.geocoder._nominatim_lookup", + new_callable=AsyncMock, + return_value=None, + ), + ): + result = await geocode( + "620078, Свердловская обл, г Екатеринбург, Кировский р-н, " + "ул Педагогическая, д 15, кв 11", + db, + ) + + assert result is not None + assert result.lat == pytest.approx(56.835284) + assert result.lon == pytest.approx(60.655829) + assert result.address_refined is True + mock_cache_put.assert_not_called() From cb0f42d1b11e50453afdcdc0e55ad178951bd3d2 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sat, 15 Aug 2026 19:22:22 +0300 Subject: [PATCH 9/9] =?UTF-8?q?fix(health):=20=D0=BD=D0=B5=20=D1=82=D0=B0?= =?UTF-8?q?=D1=89=D0=B8=D1=82=D1=8C=20HEAD-=D0=BF=D1=80=D0=BE=D0=B1=D1=83?= =?UTF-8?q?=20=D0=B2=20OpenAPI-=D1=81=D1=85=D0=B5=D0=BC=D1=83?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Джоба openapi-codegen-check покраснела на этой ветке: она дампит app.openapi(), регенерирует frontend/src/types/api-types.ts и падает на расхождении. Добавленный HEAD /health попал в схему и потребовал правки сгенерированного файла. Регенерировать типы ради маршрута, который фронт никогда не вызывает, — лишний шум в generated-коде. HEAD-проба это инфраструктура для uptime-монитора, а не часть контракта, по которому фронт строит типы, поэтому include_in_schema=False здесь и по смыслу верно, а не только удобно. Флаг ставим в обоих бэкендах симметрично: у trade-in codegen-джобы пока нет, но расхождение схем между двумя бэкендами потом само станет источником вопросов. --- backend/app/main.py | 7 ++++++- tradein-mvp/backend/app/main.py | 5 ++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/backend/app/main.py b/backend/app/main.py index c779e335..5f6507ed 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -521,6 +521,11 @@ async def health() -> dict[str, str]: # дублировать сборку payload) — RFC 9110 §9.3.2 разрешает опускать payload- # заголовки (Content-Length) для HEAD, требует совпадения только заголовков # представления (Content-Type). -@app.head("/health") +# include_in_schema=False: HEAD-проба — инфраструктура (uptime-monitor), а не часть +# контракта, по которому фронт генерирует типы. Без этого флага операция попадает в +# app.openapi(), и job `openapi-codegen-check` краснеет, требуя перегенерации +# frontend/src/types/api-types.ts — правки в сгенерированном файле ради маршрута, +# который фронт никогда не вызывает. +@app.head("/health", include_in_schema=False) async def health_head() -> Response: return Response(status_code=200, media_type="application/json") diff --git a/tradein-mvp/backend/app/main.py b/tradein-mvp/backend/app/main.py index 8a48c7c3..d9b7aaff 100644 --- a/tradein-mvp/backend/app/main.py +++ b/tradein-mvp/backend/app/main.py @@ -222,7 +222,10 @@ def health() -> dict[str, str]: # сознательно НЕ вычисляем под байт GET-ответа (дублировало бы сборку payload) # — RFC 9110 §9.3.2 разрешает опускать payload-заголовки (Content-Length) для # HEAD, требует совпадения только заголовков представления (Content-Type). -@app.head("/health") +# include_in_schema=False — по той же причине, что и у Site Finder: HEAD-проба это +# инфраструктура, а не контракт API. Здесь codegen-джоба пока нет, флаг ставим +# симметрично, чтобы схема двух бэкендов не разъезжалась. +@app.head("/health", include_in_schema=False) def health_head() -> Response: return Response(status_code=200, media_type="application/json")