fix(ptica): три места, где код делал не то, что говорил (#2464) #3003

Merged
bot-backend merged 1 commit from fix/2464-three-small into main 2026-08-20 19:18:01 +00:00
4 changed files with 161 additions and 6 deletions

View file

@ -295,13 +295,17 @@ def place_program(
)
placed_for_item += 1
if placed_for_item < item.count:
# Печатаем ФАКТИЧЕСКИ использованные размеры fp_w/fp_d, а не каталожные
# house.footprint_* (#2464): если элемент программы переопределил габарит,
# прежнее сообщение называло размер, которым никто не пытался ставить, —
# диагностика уводила от причины «участок мал».
logger.warning(
"program: type=%s placed %d of %d sections (%.0fx%.0f m) — участок мал",
item.section_type,
placed_for_item,
item.count,
house.footprint_w_m,
house.footprint_d_m,
fp_w,
fp_d,
)
result = PlacedProgram(

View file

@ -463,7 +463,15 @@ def get_sqlite_info(sqlite_path: str | Path) -> dict[str, Any]:
}
if not p.exists():
return info
st = p.stat()
# stat() под защитой (#2464): между exists() и stat() файл может исчезнуть —
# его переписывает выгрузка Объектива. Раньше try/except покрывал только
# sqlite3.connect ниже, и OSError отсюда улетал наружу, превращая
# диагностическую функцию в источник отказа. Отдаём то, что успели узнать.
try:
st = p.stat()
except OSError as e:
info["stat_error"] = f"{type(e).__name__}: {e}"
return info
info["size_bytes"] = st.st_size
info["modified_at"] = st.st_mtime # epoch seconds
try:

View file

@ -458,11 +458,15 @@ def load_water_reserves_from_docx(
system_kind: str,
docx_bytes: bytes,
source_url: str = "",
) -> dict[str, int]:
) -> dict[str, object]:
"""Парсит docx-байты → UPSERT ЦСВ/ЦСК в water_supply_reserves.
Выделено из load_water_reserves для юнит-теста на синтетическом docx.
Читает word/document.xml из zip, forward-fill vMerge, извлечение записей.
Возвращает счётчики (`records`, `inserted`, `updated`, ) И `period` строку
вида «III кв. 2025» либо None. Раньше тип был `dict[str, int]`, и ради него
период выбрасывался фильтром на выходе, хотя в лог печатался (#2464).
"""
with zipfile.ZipFile(io.BytesIO(docx_bytes)) as zf:
document_xml = zf.read("word/document.xml")
@ -485,9 +489,19 @@ def load_water_reserves_from_docx(
logger.exception("load_water_reserves_from_docx: outer tx rolled back: %s", e)
raise
result = {"records": len(records), **counts, "period": period} # type: ignore[dict-item]
# `period` возвращаем вместе с остальным (#2464). Раньше стоял фильтр
# `isinstance(v, int)`, который выбрасывал его ВСЕГДА — период это строка или
# None. В лог при этом печатался полный словарь, поэтому по логам казалось, что
# период отдаётся, а вызывающий его не получал никогда.
#
# Фильтр ничего не защищал: соседняя ветка `load_water_reserves` кладёт в тот же
# словарь `{"error": str(...)}`, то есть «только int» контрактом не было, а
# единственный потребитель (задача sync_water_reserves) результат логирует и
# возвращает как есть. Период при этом полезен: он говорит, за какой квартал
# данные, — без него «загружено 42 записи» не отличить от прошлогодних.
result: dict[str, object] = {"records": len(records), **counts, "period": period}
logger.info("water_reserves[%s] done: %s", system_kind, result)
return {k: v for k, v in result.items() if isinstance(v, int)}
return result
def load_water_reserves(db: Session | None = None) -> dict[str, dict]:

View file

@ -0,0 +1,129 @@
"""Три места, где код делал не то, что говорил (#2464).
1. `load_water_reserves_from_docx` (vodokanal): собирал `result` с ключом `period`,
печатал полный словарь в лог, а возвращал `{k: v for if isinstance(v, int)}`
период это строка или None, поэтому он выбрасывался ВСЕГДА. По логам казалось,
что период отдаётся; вызывающий не получал его ни разу.
2. `get_sqlite_info` (objective_etl): TOCTOU `p.exists()`, затем незащищённый
`p.stat()`. try/except покрывал только `sqlite3.connect` ниже, поэтому OSError
из stat улетал наружу и превращал диагностическую функцию в источник отказа.
3. `place_program` (placement): предупреждение «участок мал» печатало КАТАЛОЖНЫЕ
`house.footprint_*`, хотя ставили по `fp_w`/`fp_d`. Если элемент программы
переопределил габарит, сообщение называло размер, которым никто не пытался
ставить, диагностика уводила от причины.
"""
from __future__ import annotations
import inspect
from pathlib import Path
from unittest.mock import MagicMock, patch
import pytest
# ── 1. period больше не выбрасывается ────────────────────────────────────────
def test_water_result_keeps_period() -> None:
"""Головной: период обязан доехать до вызывающего, раз он попал в лог.
На origin/main фильтр `isinstance(v, int)` выбрасывает его всегда.
"""
from app.services.site_finder import vodokanal_reserve_loader as mod
with (
patch.object(mod, "parse_docx_table_rows", lambda _x: [["шапка"]]),
patch.object(mod, "extract_water_rows", lambda _m: [{"name": "ЦСВ-1"}]),
patch.object(mod, "_dedupe_names", lambda r: r),
patch.object(mod, "infer_period", lambda _u: "III кв. 2025"),
patch.object(mod, "_upsert_water_rows", lambda *a, **k: {"inserted": 1, "updated": 0}),
patch.object(mod.zipfile, "ZipFile", MagicMock()),
):
res = mod.load_water_reserves_from_docx(MagicMock(), "supply", b"", "http://x")
assert (
res.get("period") == "III кв. 2025"
), f"период выброшен из ответа: {res} — по логам он есть, у вызывающего нет"
assert res.get("records") == 1 and res.get("inserted") == 1, res
def test_water_return_annotation_allows_non_int() -> None:
"""Контроль: фильтр стоял ради аннотации `dict[str, int]` — она тоже исправлена.
Иначе следующий проход mypy вернул бы фильтр обратно.
"""
from app.services.site_finder.vodokanal_reserve_loader import load_water_reserves_from_docx
ann = inspect.signature(load_water_reserves_from_docx).return_annotation
assert "int]" not in str(ann), f"аннотация всё ещё требует только int: {ann}"
# ── 2. TOCTOU в get_sqlite_info ──────────────────────────────────────────────
def test_sqlite_info_survives_file_vanishing_between_exists_and_stat() -> None:
"""Головной: файл исчез между exists() и stat() — функция не падает.
На origin/main OSError улетает наружу: try/except покрывает только connect.
"""
from app.services import objective_etl as mod
настоящий_stat = Path.stat
def _stat(self, *a, **k):
if str(self).endswith("исчезающий.sqlite"):
raise FileNotFoundError(2, "No such file or directory")
return настоящий_stat(self, *a, **k)
with (
patch.object(Path, "exists", lambda self: True),
patch.object(Path, "stat", _stat),
):
info = mod.get_sqlite_info("/tmp/исчезающий.sqlite")
assert info["exists"] is True
assert "stat_error" in info, f"ошибка stat не отражена в ответе: {info}"
assert "size_bytes" not in info, "размер выдуман при отсутствующем файле"
def test_sqlite_info_missing_file_unchanged() -> None:
"""Контроль: отсутствующий файл по-прежнему даёт exists=False без ошибок."""
from app.services.objective_etl import get_sqlite_info
info = get_sqlite_info("/tmp/такого-файла-нет-2464.sqlite")
assert info["exists"] is False
assert "stat_error" not in info, info
# ── 3. предупреждение placement называет фактические габариты ────────────────
def test_placement_warning_uses_actual_footprint_not_catalog() -> None:
"""Головной: в сообщении должны стоять fp_w/fp_d, а не house.footprint_*.
На origin/main при переопределённом габарите печатается каталожный размер.
"""
from app.services.generative import placement as mod
src = inspect.getsource(mod.place_program)
хвост = src[src.index("участок мал") :]
assert (
"fp_w," in хвост and "fp_d," in хвост
), f"в предупреждении не фактические габариты:\n{хвост[:320]}"
assert (
"house.footprint_w_m," not in хвост and "house.footprint_d_m," not in хвост
), f"в предупреждении остался каталожный размер:\n{хвост[:320]}"
@pytest.mark.parametrize("имя", ["fp_w", "fp_d"])
def test_placement_uses_those_names_for_placement(имя: str) -> None:
"""Контроль: fp_w/fp_d — это действительно те размеры, которыми ставят.
Без него тест выше проверял бы совпадение имён, а не смысл.
"""
from app.services.generative import placement as mod
src = inspect.getsource(mod.place_program)
assert "_centered_footprint(cell.cx, cell.cy, fp_w, fp_d)" in src, src[:200]
assert имя in src