docs(ptica): две докстроки обещали то, чего в коде нет (#2464)
All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
CI / changes (pull_request) Successful in 9s
CI Trade-In / backend-tests (pull_request) Has been skipped
CI Trade-In / browser-tests (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Successful in 2m4s
CI / backend-tests (pull_request) Successful in 17m21s

1. `QuarterDump` (nspd_client): «Default = только core, чтобы не сжигать
   rate-limit на 17 запросов». Фактический дефолт `search_by_quarter` —
   `include_zouit=True`, то есть 5 ЗОУИТ-слоёв входят в дефолтный вызов.
   Числа 17 тоже нет: territorial_zones/red_lines/engineering и все ЗОУИТ
   идут через grid-walk при grid_n=7, по 49 запросов КАЖДЫЙ — дефолтный
   дамп это сотни запросов. Экономит rate-limit только include_risks=False.
   Докстрока самого метода 640 строками ниже говорит верно («Default
   True») — правильный образец лежал рядом с дефектом.

2. `find_active_on_demand_job` (cadastre_fetch): «Если в БД есть FAILED
   on-demand за последние 60 секунд — тоже None». В SQL нет ни слова
   'failed', ни какого-либо временного фильтра. Обещание вдвойне вредно:
   подразумевало, что неуспешная джоба СТАРШЕ минуты вернётся как
   активная (не вернётся), и отправляло отлаживающего искать окно,
   которого нет.

Гейты сверяют утверждение докстроки с кодом, а не читаемость текста:
обещание «только core» требует `include_zouit=False` в сигнатуре;
обещание минутного окна требует временного фильтра в теле.

Двусторонне: против origin/main три гейта красные с конкретными
сообщениями. Контроли зелёные с обеих сторон — характеризующий фиксирует
фактические три статуса в SQL, а test_docstrings_state_the_actual_behaviour
ловит «починку» через вычёркивание неудобной фразы.

Два подводных камня, на которые наступил и оставил защиту:
- гейт ищет обещание по тексту, поэтому старые формулировки в докстроках
  ПЕРЕСКАЗАНЫ, а не процитированы — иначе он не отличает цитату от
  утверждения (оговорено прямо в тексте докстроки);
- тело функции нельзя брать как последний кусок разбиения по тройным
  кавычкам: SQL сам в них обёрнут, и проверка шла бы по огрызку после
  запроса. Из-за этого один гейт проходил по случайности. Вынесен
  хелпер `_body`.

pytest backend/tests/services/ — 3199 passed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
bot-backend 2026-08-20 23:43:40 +05:00
parent 9810a946a6
commit c3c8674b3c
3 changed files with 134 additions and 5 deletions

View file

@ -260,7 +260,19 @@ class QuarterDump:
- core: parcels + buildings + territorial_zones + red_lines + engineering
- zouit: 5 ЗОУИТ layers (G3)
- risks: 11 risk-zone layers (TIER 3)
Default = только core, чтобы не сжигать rate-limit на 17 запросов.
По умолчанию берутся core + zouit: `search_by_quarter(include_zouit=True,
include_risks=False)`. Прежняя редакция утверждала обратное будто по умолчанию
берётся один core ради экономии полутора десятков запросов (#2464). Неверно
вдвойне. Во-первых, `include_zouit` по умолчанию True, и 5 ЗОУИТ-слоёв входят в
дефолтный вызов; докстрока самого метода это говорит правильно. Во-вторых, порядок
величины не тот: territorial_zones/red_lines/engineering и все ЗОУИТ идут через
grid-walk при grid_n=7, то есть по 49 запросов КАЖДЫЙ дефолтный дамп это сотни
запросов. Экономит rate-limit только `include_risks=False`.
(Старая формулировка здесь пересказана, а не процитирована: гейт
test_2464_docstring_matches_code ищет обещание по тексту и не отличил бы
цитату от утверждения.)
"""
quarter_cad: str

View file

@ -98,10 +98,17 @@ def cad_exists_in_db(db: Session, cad_num: str) -> bool:
def find_active_on_demand_job(db: Session, cad_num: str) -> int | None:
"""Найти существующий on-demand job (queued/running/paused) для этого cad.
Возвращает job_id или None. Если в БД есть FAILED on-demand за последние 60
секунд тоже None (чтобы повторно пробовать). Если есть DONE job, но cad
отсутствует в БД (на NSPD не нашлось) тоже None, но caller через
`fetch_status` отличит этот случай как `not_in_nspd`.
Возвращает job_id или None.
Неуспешные джобы не возвращаются НИКОГДА, независимо от давности: запрос отбирает
только `status IN ('queued','running','paused')`, и других статусов в нём нет.
Прежняя редакция обещала минутное окно давности для неуспешных (#2464) — такой
логики здесь никогда не было, временного фильтра в SQL нет вовсе. Обещание было
вдвойне вредным: оно подразумевало, что неуспешная джоба ПОСТАРШЕ вернётся как
активная (не вернётся), и отправляло отлаживающего искать окно, которого нет.
Если есть DONE job, но cad отсутствует в БД (на NSPD не нашлось) тоже None,
но caller через `fetch_status` отличит этот случай как `not_in_nspd`.
NB (issue #1356): 'paused' тоже считается active. Job переходит в 'paused'
при WAF (consecutive>=8) или Celery soft_time_limit (6h) нетронутые targets

View file

@ -0,0 +1,110 @@
"""Две докстроки обещали то, чего в коде нет (#2464).
1. `QuarterDump` (nspd_client): «Default = только core, чтобы не сжигать rate-limit
на 17 запросов». Фактический дефолт `search_by_quarter` `include_zouit=True`,
то есть 5 ЗОУИТ-слоёв входят в дефолтный вызов. Числа 17 тоже нет: grid-walk при
grid_n=7 даёт по 49 запросов на слой. Причём докстрока самого метода 640 строками
ниже пишет верно «Default True»: правильный образец лежал рядом с дефектом.
2. `find_active_on_demand_job` (cadastre_fetch): «Если в БД есть FAILED on-demand за
последние 60 секунд тоже None (чтобы повторно пробовать)». В SQL нет ни слова
'failed', ни какого-либо временного фильтра. Обещание вдвойне вредно: оно
подразумевало, что failed СТАРШЕ 60 секунд вернётся как активный (не вернётся),
и отправляло отлаживающего искать окно, которого нет.
Гейты сверяют утверждение докстроки с кодом, а не читаемость текста.
"""
from __future__ import annotations
import inspect
def _body(fn) -> str:
"""Тело функции без её докстроки.
НЕ `split('\"\"\"')[-1]`: SQL внутри сам обёрнут в тройные кавычки, поэтому
последний кусок это хвост ПОСЛЕ запроса, и проверка шла бы по огрызку.
Отбрасываем ровно первую докстроку и склеиваем остальное обратно.
"""
части = inspect.getsource(fn).split('"""')
return '"""'.join(части[2:]) if len(части) > 2 else части[-1]
def test_quarter_dump_docstring_matches_the_real_default() -> None:
"""Головной 1: если докстрока обещает «только core» — дефолт обязан быть False.
На origin/main она это обещает, а `include_zouit` по умолчанию True.
"""
from app.services.scrapers.nspd_client import NSPDClient, QuarterDump
doc = inspect.getdoc(QuarterDump) or ""
default = inspect.signature(NSPDClient.search_by_quarter).parameters["include_zouit"].default
обещает_только_core = "Default = только core" in doc
assert not (обещает_только_core and default is True), (
f"докстрока QuarterDump обещает «Default = только core», "
f"а include_zouit по умолчанию {default}"
)
def test_quarter_dump_docstring_does_not_claim_17_requests() -> None:
"""Контроль числа: «17 запросов» противоречит grid-walk (49 запросов на слой).
Просто убрать слово «core» было бы недостаточно довод про rate-limit
держался на выдуманном числе.
"""
from app.services.scrapers.nspd_client import QuarterDump
doc = inspect.getdoc(QuarterDump) or ""
assert (
"не сжигать rate-limit на 17 запросов" not in doc
), "в докстроке осталось число 17, противоречащее grid-walk"
def test_on_demand_docstring_does_not_promise_a_60s_window() -> None:
"""Головной 2: обещание окна «60 секунд» должно подтверждаться SQL.
На origin/main докстрока его обещает, а в запросе нет ни 'failed',
ни временного фильтра.
"""
from app.services.site_finder.cadastre_fetch import find_active_on_demand_job
doc = inspect.getdoc(find_active_on_demand_job) or ""
тело = _body(find_active_on_demand_job)
обещает_окно = "за последние 60" in doc
есть_фильтр = any(kw in тело.lower() for kw in ("interval", "now()", "failed"))
assert not (обещает_окно and not есть_фильтр), (
f"докстрока обещает окно «за последние 60 секунд», а в SQL нет ни "
f"временного фильтра, ни статуса failed:\n{тело.strip()[:400]}"
)
def test_on_demand_sql_really_ignores_failed() -> None:
"""Характеризующий: запрос отбирает ровно три статуса, failed среди них нет.
Зелёный с обеих сторон фиксирует фактическое поведение, о котором теперь
говорит докстрока. Если кто-то добавит окно, тест покраснеет и заставит
обновить и текст.
"""
from app.services.site_finder.cadastre_fetch import find_active_on_demand_job
тело = _body(find_active_on_demand_job)
assert "'queued', 'running', 'paused'" in тело, тело[:300]
assert "failed" not in тело.lower(), "в запросе появился failed — обнови докстроку"
def test_docstrings_state_the_actual_behaviour() -> None:
"""Контроль от вычёркивания: обе докстроки обязаны НАЗЫВАТЬ фактическое поведение.
Молча убрать неверную фразу не починка: молчание читается как «всё хорошо».
"""
from app.services.scrapers.nspd_client import QuarterDump
from app.services.site_finder.cadastre_fetch import find_active_on_demand_job
qd = inspect.getdoc(QuarterDump) or ""
assert "include_zouit" in qd, "не назван фактический дефолт дампа"
od = inspect.getdoc(find_active_on_demand_job) or ""
assert (
"НИКОГДА" in od or "никогда" in od
), "не сказано, что failed не возвращается независимо от давности"