lekss361/gendesign

Fork 0

fix(parcels): _parse_floors handle int (post-migration #169) #176

Merged

lekss361 merged 4 commits from fix/parse-floors-int into main

2026-05-15 12:08:17 +00:00

lekss361 commented

2026-05-15 11:50:27 +00:00

(Migrated from github.com)

Summary

Hotfix — analyze endpoint 500 после deploy bulk schema migration
cad_buildings.floors теперь INT (был TEXT), but _parse_floors() ожидал str → 'int' object has no attribute 'strip'

Error from prod logs

File "/app/app/api/v1/parcels.py", line 1883, in analyze_parcel
  "neighbors_summary": _neighbors_summary(db, geom_wkt, cad_num),
File "/app/app/api/v1/parcels.py", line 715, in _neighbors_summary
  f = _parse_floors(r.get("floors"))
File "/app/app/api/v1/parcels.py", line 651, in _parse_floors
  raw = raw.strip()
AttributeError: 'int' object has no attribute 'strip'

Fix

Type union str | int | None. If int → return directly. Else strip + parse как раньше.

Test plan

After merge + deploy → POST /api/v1/parcels/66:41:0204016:10/analyze returns 200
neighbors_summary рендерится в UI

Closes 500 regression from #169.

## Summary - **Hotfix** — analyze endpoint 500 после deploy bulk schema migration - `cad_buildings.floors` теперь INT (был TEXT), but `_parse_floors()` ожидал str → `'int' object has no attribute 'strip'` ## Error from prod logs ``` File "/app/app/api/v1/parcels.py", line 1883, in analyze_parcel "neighbors_summary": _neighbors_summary(db, geom_wkt, cad_num), File "/app/app/api/v1/parcels.py", line 715, in _neighbors_summary f = _parse_floors(r.get("floors")) File "/app/app/api/v1/parcels.py", line 651, in _parse_floors raw = raw.strip() AttributeError: 'int' object has no attribute 'strip' ``` ## Fix Type union `str | int | None`. If `int` → return directly. Else strip + parse как раньше. ## Test plan - [ ] After merge + deploy → `POST /api/v1/parcels/66:41:0204016:10/analyze` returns 200 - [ ] `neighbors_summary` рендерится в UI Closes 500 regression from #169.

lekss361 commented

2026-05-15 11:56:20 +00:00

(Migrated from github.com)

Review

CI ещё pending; сам hotfix _parse_floors корректный и тонкий — но в этом же PR прилетают smoke-тесты tests/smoke/test_prod_smoke.py, которые сломают CI policy. Блокер до того как смерджу.

✅ Что хорошо

_parse_floors fix корректен. Подтвердил по data/sql/92_cad_bulk_layers.sql — cad_buildings.floors теперь INT. Оба caller'а (parcels.py:715 и :789) читают r.get("floors") из row dict, никаких bool не попадёт. Сохранилась эквивалентность для 0/""/None: пустая строка → None через явный check, 0 → isinstance branch → 0 > 0 == False → None.
Type contract drift отсутствует для floors — нет упоминаний в app/schemas/parcel.py и frontend обращается к neighbors_summary как opaque object.
Smoke-тесты по содержанию хорошо покрывают регресс-поверхность (#149, #152, #160, #169, #163, #165, #171, #172).

🔴 Блокер: prod_smoke marker не зарегистрирован → CI любого PR будет стрелять в prod

.github/workflows/ci.yml:60 — uv run pytest -q (без -m фильтра).
backend/pyproject.toml [tool.pytest.ini_options].markers — только slow, prod_smoke отсутствует.

pytestmark = pytest.mark.prod_smoke НЕ skip'нет тесты, потому что у marker'а нет deselect-механизма без -m 'not prod_smoke' в addopts. Pytest выкинет PytestUnknownMarkWarning, но соберёт и выполнит все тесты.

Эффект: каждый push/PR от любого fork'а будет в CI делать:

POST https://gendsgn.ru/api/v1/parcels/66:41:0204016:10/analyze (60s timeout × 3 теста)
GET https://gendsgn.ru/health, /site-finder, /admin/scrape/*

То есть hammer prod из всех CI-раннеров + утечка prod URL в логи forked PRs.

В docstring файла прямо написано "skip by default in CI, run only on-demand" — но реализация это не обеспечивает.

Требуемый фикс (в этом же PR)

В backend/pyproject.toml [tool.pytest.ini_options]:

markers = [
    "slow: marks tests as slow (need real network, deselect with -m 'not slow')",
    "prod_smoke: real HTTP against prod — opt-in only (run with -m prod_smoke)",
]
addopts = "-m 'not prod_smoke'"

Тогда дефолтные pytest -q будут исключать smoke по умолчанию, а post-deploy workflow сможет явно запустить pytest -m prod_smoke override.

Минор (некритичный — можно follow-up)

Ассерт в test_parcels_analyze_returns_200:91-93 логически верный (фейлится только при data_available=False AND note startswith "neighbors query failed"), но фраза is not False or (... is False) нечитаема — лучше assert not (data_available is False and note.startswith(...)).

Жду нового SHA с addopts фильтром.

## Review CI ещё pending; сам hotfix `_parse_floors` корректный и тонкий — но в этом же PR прилетают smoke-тесты `tests/smoke/test_prod_smoke.py`, которые **сломают CI policy**. Блокер до того как смерджу. ### ✅ Что хорошо - **`_parse_floors` fix корректен.** Подтвердил по `data/sql/92_cad_bulk_layers.sql` — `cad_buildings.floors` теперь `INT`. Оба caller'а (`parcels.py:715` и `:789`) читают `r.get("floors")` из row dict, никаких `bool` не попадёт. Сохранилась эквивалентность для `0`/`""`/None: пустая строка → None через явный check, `0` → isinstance branch → `0 > 0 == False` → None. - **Type contract drift отсутствует** для `floors` — нет упоминаний в `app/schemas/parcel.py` и `frontend` обращается к `neighbors_summary` как opaque object. - **Smoke-тесты по содержанию** хорошо покрывают регресс-поверхность (#149, #152, #160, #169, #163, #165, #171, #172). ### 🔴 Блокер: prod_smoke marker не зарегистрирован → CI любого PR будет стрелять в prod `.github/workflows/ci.yml:60` — `uv run pytest -q` (без `-m` фильтра). `backend/pyproject.toml [tool.pytest.ini_options].markers` — только `slow`, `prod_smoke` отсутствует. `pytestmark = pytest.mark.prod_smoke` НЕ skip'нет тесты, потому что у marker'а нет deselect-механизма без `-m 'not prod_smoke'` в addopts. Pytest выкинет `PytestUnknownMarkWarning`, но **соберёт и выполнит** все тесты. Эффект: каждый push/PR от любого fork'а будет в CI делать: - `POST https://gendsgn.ru/api/v1/parcels/66:41:0204016:10/analyze` (60s timeout × 3 теста) - `GET https://gendsgn.ru/health`, `/site-finder`, `/admin/scrape/*` То есть hammer prod из всех CI-раннеров + утечка prod URL в логи forked PRs. В docstring файла прямо написано "skip by default in CI, run only on-demand" — но реализация это не обеспечивает. ### Требуемый фикс (в этом же PR) В `backend/pyproject.toml [tool.pytest.ini_options]`: ```toml markers = [ "slow: marks tests as slow (need real network, deselect with -m 'not slow')", "prod_smoke: real HTTP against prod — opt-in only (run with -m prod_smoke)", ] addopts = "-m 'not prod_smoke'" ``` Тогда дефолтные `pytest -q` будут исключать smoke по умолчанию, а post-deploy workflow сможет явно запустить `pytest -m prod_smoke` override. ### Минор (некритичный — можно follow-up) - Ассерт в `test_parcels_analyze_returns_200:91-93` логически верный (фейлится только при `data_available=False AND note startswith "neighbors query failed"`), но фраза `is not False or (... is False)` нечитаема — лучше `assert not (data_available is False and note.startswith(...))`. Жду нового SHA с `addopts` фильтром.

lekss361 commented

2026-05-15 12:04:07 +00:00

(Migrated from github.com)

Re-review (SHA `4ace87a`)

CI fail в backend job — именно тот сценарий что я и предупреждал в прошлом ревью:

tests/smoke/test_prod_smoke.py:36: PytestUnknownMarkWarning: Unknown pytest.mark.prod_smoke
3 failed: test_parcels_analyze_returns_200, ...velocity_block_present, ...pipeline_by_class_enriched

CI runner GitHub Actions сделал POST на https://gendsgn.ru/api/v1/parcels/.../analyze (потому что fix ещё не задеплоен!) → prod вернул 500 → тесты упали → CI red. И prod URL утёк в публичные CI логи.

🔴 Что НЕ исправлено

backend/pyproject.toml всё тот же (проверил branch fix/parse-floors-int):

markers = [
    "slow: ...",
]

То что я просил в предыдущем комменте:

markers = [
    "slow: ...",
    "prod_smoke: real HTTP against prod — opt-in only (run with -m prod_smoke)",
]
addopts = "-m 'not prod_smoke'"

Без этого smoke-тесты будут стрелять в prod из CI каждого push'а + PR'а. Это обязательный фикс в этом же PR, без него мерджить нельзя.

🟡 Scope creep — NSPD headers

В этом же SHA прилетели изменения в backend/app/scrapers/nspd_bulk_client.py (rewrite DEFAULT_HEADERS под legacy nspd_lite.py headers — Chrome 144, lowercase keys, cache-control). Это не из PR title "fix(parcels): _parse_floors". Это самостоятельный fix WAF blocks на pilot v2, заслуживает отдельного PR или хотя бы upgraded title + body.

Так как изменение headers — это connection-config diff и риск низкий (legacy precedent уже работает), могу пропустить если будет:

упомянуто в PR body (обновить summary), или
вынесено в #177.

Что нужно для approve+merge

Обязательно: добавить prod_smoke marker + addopts = "-m 'not prod_smoke'" в backend/pyproject.toml [tool.pytest.ini_options].
Желательно: либо разнести NSPD headers в отдельный PR, либо обновить PR title/body.
CI должен стать зелёным после фикса (smoke тесты будут пропускаться по умолчанию).

Жду нового SHA.

## Re-review (SHA 4ace87a) CI **fail** в backend job — именно тот сценарий что я и предупреждал в прошлом ревью: ``` tests/smoke/test_prod_smoke.py:36: PytestUnknownMarkWarning: Unknown pytest.mark.prod_smoke 3 failed: test_parcels_analyze_returns_200, ...velocity_block_present, ...pipeline_by_class_enriched ``` CI runner GitHub Actions сделал POST на `https://gendsgn.ru/api/v1/parcels/.../analyze` (потому что fix ещё не задеплоен!) → prod вернул 500 → тесты упали → CI red. И prod URL утёк в публичные CI логи. ### 🔴 Что НЕ исправлено **`backend/pyproject.toml` всё тот же** (проверил branch `fix/parse-floors-int`): ``` markers = [ "slow: ...", ] ``` То что я просил в [предыдущем комменте](https://github.com/lekss361/-gendesign/pull/176#issuecomment-4459531065): ```toml markers = [ "slow: ...", "prod_smoke: real HTTP against prod — opt-in only (run with -m prod_smoke)", ] addopts = "-m 'not prod_smoke'" ``` Без этого smoke-тесты будут стрелять в prod из CI каждого push'а + PR'а. **Это обязательный фикс в этом же PR**, без него мерджить нельзя. ### 🟡 Scope creep — NSPD headers В этом же SHA прилетели изменения в `backend/app/scrapers/nspd_bulk_client.py` (rewrite `DEFAULT_HEADERS` под legacy `nspd_lite.py` headers — Chrome 144, lowercase keys, cache-control). Это не из PR title "fix(parcels): _parse_floors". Это самостоятельный fix WAF blocks на pilot v2, заслуживает отдельного PR или хотя бы upgraded title + body. Так как изменение `headers` — это connection-config diff и риск низкий (legacy precedent уже работает), могу пропустить если будет: - упомянуто в PR body (обновить summary), или - вынесено в #177. ### Что нужно для approve+merge 1. **Обязательно**: добавить `prod_smoke` marker + `addopts = "-m 'not prod_smoke'"` в `backend/pyproject.toml [tool.pytest.ini_options]`. 2. **Желательно**: либо разнести NSPD headers в отдельный PR, либо обновить PR title/body. 3. CI должен стать зелёным после фикса (smoke тесты будут пропускаться по умолчанию). Жду нового SHA.

lekss361 commented

2026-05-15 12:08:08 +00:00

(Migrated from github.com)

Review (SHA `d83fb9c`)

Все блокеры из прошлых ревью закрыты + cross-check (agent):

✅ Закрыто

Marker fix: backend/pyproject.toml — addopts = ["-m", "not prod_smoke"] + marker prod_smoke зарегистрирован. List-form addopts валиден, pytest парсит как pytest -m "not prod_smoke".
CI зелёный: backend 1m15s + frontend 1m32s. Smoke-тесты по умолчанию исключаются, остальные 167 прошли + 7 skip.
_parse_floors: type-union str | int | None, isinstance fast-path для INT, сохранён range-parser для backwards-compat. Оба caller'а (parcels.py:715, :789) читают row dict.

✅ NSPD headers — низкий риск

Изменение в nspd_bulk_client.py DEFAULT_HEADERS (Chrome 148→144, lowercase keys, +cache-control/pragma/origin) — outbound-only, exact match nspd_lite.py. Проверил backend/tests/scrapers/ — нет assertions на header keys/values, mock'и stuck на httpx response side. Inline-комментарий "blocked 50/50 в pilot run v2" justifies change. Принимаю как WAF hotfix-продолжение, в идеале PR body стоило бы обновить — оставлю как минор.

✅ Contract drift отсутствует

backend/app/schemas/ не содержит поля floors; frontend относится к neighbors_summary как opaque. NSPD headers — outbound only.

approve merge

## Review (SHA d83fb9c) Все блокеры из прошлых ревью закрыты + cross-check (agent): ### ✅ Закрыто - **Marker fix**: `backend/pyproject.toml` — `addopts = ["-m", "not prod_smoke"]` + marker `prod_smoke` зарегистрирован. List-form `addopts` валиден, pytest парсит как `pytest -m "not prod_smoke"`. - **CI зелёный**: backend 1m15s + frontend 1m32s. Smoke-тесты по умолчанию исключаются, остальные 167 прошли + 7 skip. - **`_parse_floors`**: type-union `str | int | None`, isinstance fast-path для INT, сохранён range-parser для backwards-compat. Оба caller'а (`parcels.py:715`, `:789`) читают row dict. ### ✅ NSPD headers — низкий риск Изменение в `nspd_bulk_client.py` DEFAULT_HEADERS (Chrome 148→144, lowercase keys, +cache-control/pragma/origin) — outbound-only, exact match `nspd_lite.py`. Проверил `backend/tests/scrapers/` — нет assertions на header keys/values, mock'и stuck на httpx response side. Inline-комментарий "blocked 50/50 в pilot run v2" justifies change. Принимаю как WAF hotfix-продолжение, в идеале PR body стоило бы обновить — оставлю как минор. ### ✅ Contract drift отсутствует `backend/app/schemas/` не содержит поля `floors`; frontend относится к `neighbors_summary` как opaque. NSPD headers — outbound only. approve merge