lekss361/gendesign

Fork 0

fix(cadastre): NSPD response parse — properties.category + top-level meta (#168) #177

Merged

lekss361 merged 2 commits from fix/nspd-response-parse into main

2026-05-15 12:40:01 +00:00

lekss361 commented

2026-05-15 12:23:57 +00:00

(Migrated from github.com)

Summary

Hotfix для #168 — pilot v3 ran successfully (no WAF/SSL) but 50/50 quarters returned parcels=0
2 parsing bugs in NSPD response shape

Bugs

1. `NSPDBulkFeature.category_id` ищет `categoryId` — реальное поле `category`

NSPD response: properties.category (без Id suffix). Initial HAR parse misinterpreted.
Fix: try both for backwards compat.

2. `meta` лежит на top-level, не в `data`

Real shape: {"data": {"features": [...]}, "meta": [...]}. Code искал в data.meta.
Fix: payload.get("meta") or data_block.get("meta") or [].

Live verification on prod

>>> s = await client.search_by_quarter('66:41:0303161')
>>> len(s.features)
101
>>> s.features[0].properties['category']
37159  # Помещения
>>> # Top-level meta:
>>> [{"totalCount": 170, "categoryId": 36368}, ...]

Без этих фиксов upsert_features routes по None category_id → no-op для всех 50/50 quarters в pilot v3.

Test plan

After merge + deploy → retry pilot (already cancelled #2, #3 done with 0 data)
Expect cad_parcels grows for ЕКБ

## Summary - **Hotfix** для #168 — pilot v3 ran successfully (no WAF/SSL) but 50/50 quarters returned parcels=0 - 2 parsing bugs in NSPD response shape ## Bugs ### 1. `NSPDBulkFeature.category_id` ищет `categoryId` — реальное поле `category` NSPD response: `properties.category` (без Id suffix). Initial HAR parse misinterpreted. Fix: try both for backwards compat. ### 2. `meta` лежит на top-level, не в `data` Real shape: `{"data": {"features": [...]}, "meta": [...]}`. Code искал в `data.meta`. Fix: `payload.get("meta") or data_block.get("meta") or []`. ## Live verification on prod ```python >>> s = await client.search_by_quarter('66:41:0303161') >>> len(s.features) 101 >>> s.features[0].properties['category'] 37159 # Помещения >>> # Top-level meta: >>> [{"totalCount": 170, "categoryId": 36368}, ...] ``` Без этих фиксов upsert_features routes по None category_id → no-op для всех 50/50 quarters в pilot v3. ## Test plan - [ ] After merge + deploy → retry pilot (already cancelled #2, #3 done with 0 data) - [ ] Expect cad_parcels grows for ЕКБ

lekss361 commented

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

(Migrated from github.com)

Review

Cross-check (agent) выявил реальный test-coverage gap. CI зелёный + diff минимальный, но существующие фикстуры тестируют только legacy fallback, не новый primary path. Это критично — именно потому что не было unit-теста, парс-баг прошёл аж до prod и положил все 50/50 quarters в pilot v3.

🔴 Блокер: новые ветки кода не покрыты тестами

backend/tests/scrapers/test_nspd_bulk_client.py:51,71 и backend/tests/services/test_cadastre_bulk.py:45,79,115,240,254,461,472 — все фикстуры всё ещё используют "categoryId": N в properties. После фикса CI ходит по fallback branch (properties.get("categoryId")), а primary branch (properties.get("category")) не выполняется ни в одном тесте.

То же самое для meta — фикстура в test_nspd_bulk_client.py:75-83 всё ещё держит meta внутри data.meta (legacy shape), top-level meta path не тестируется.

Author заявляет live verification на quarter 66:41:0303161 → 101 features, но recorded payload в repo не положен. Это уже третий "live-only" парс-фикс подряд (#175 verify=False, #176 headers + smoke, #177 parse) — паттерн пора прервать unit-тестом.

Минимально нужно:

Обновить один fixture с properties.categoryId → properties.category (или добавить новый кейс)
Перенести meta в одной фикстуре на top-level
Тогда обе новые ветки парсера действительно проверяются в CI

🟡 `int(val)` без try/except — crash на non-numeric string

backend/app/schemas/nspd_bulk.py:75 — если NSPD когда-нибудь вернёт category: "ЗУ" (а они известны type-inconsistency'ю), int(val) рейзит ValueError посреди loop'а в bulk_harvest.upsert_features:295 и убивает harvest остальных features в quarter. Используй уже существующий _safe_int из bulk_harvest.py или try/except → None.

🟡 Truthiness trap (низкий риск, но real)

properties.get("category") or properties.get("categoryId") — если NSPD вернёт category: 0 (наблюдаемые ID 36368/36369/37159 — все truthy, поэтому в practice safe), fallback сработает неожиданно. Правильнее:

val = self.properties.get("category")
if val is None:
    val = self.properties.get("categoryId")

То же для meta payload.get("meta") or data_block.get("meta") — но для meta=[] поведение совпадает (оба → пустой meta_counts), оставлю как минор.

✅ Что хорошо

CI зелёный (backend 1m17s + frontend 1m35s).
Downstream routing intact: bulk_harvest.py:296-318 маппит cat == CAT_PARCEL (36368) — category_id всё ещё возвращает тот же int, источник просто сменился. Подтверждает диагноз "all 50/50 routed to None".
Meta entries shape consistent: внутренний loop meta_counts всё ещё читает categoryId (с Id) — соответствует author'скому live payload [{"totalCount":170,"categoryId":36368}]. Только location переехала, parser unchanged.
TS contract drift отсутствует — frontend (admin/scrape/cadastre/page.tsx) парсит admin job-state, не NSPD feature shape.

Что нужно

Обязательно: обновить хотя бы одну фикстуру в test_nspd_bulk_client.py или test_cadastre_bulk.py чтобы ветка properties.category + top-level meta реально выполнилась в CI.
Желательно: _safe_int(val) вместо голого int(val) в category_id property.
Минор: труcинесс → is None check.

Жду новый SHA.

## Review Cross-check (agent) выявил реальный test-coverage gap. CI зелёный + diff минимальный, **но** существующие фикстуры тестируют **только legacy fallback**, не новый primary path. Это критично — именно потому что не было unit-теста, парс-баг прошёл аж до prod и положил все 50/50 quarters в pilot v3. ### 🔴 Блокер: новые ветки кода не покрыты тестами `backend/tests/scrapers/test_nspd_bulk_client.py:51,71` и `backend/tests/services/test_cadastre_bulk.py:45,79,115,240,254,461,472` — все фикстуры всё ещё используют `"categoryId": N` в `properties`. После фикса CI ходит по **fallback branch** (`properties.get("categoryId")`), а **primary branch** (`properties.get("category")`) не выполняется ни в одном тесте. То же самое для meta — фикстура в `test_nspd_bulk_client.py:75-83` всё ещё держит `meta` внутри `data.meta` (legacy shape), top-level meta path не тестируется. Author заявляет live verification на quarter `66:41:0303161` → 101 features, но recorded payload в repo не положен. Это уже третий "live-only" парс-фикс подряд (#175 verify=False, #176 headers + smoke, #177 parse) — паттерн пора прервать unit-тестом. **Минимально нужно**: - Обновить один fixture с `properties.categoryId` → `properties.category` (или добавить новый кейс) - Перенести `meta` в одной фикстуре на top-level - Тогда обе новые ветки парсера действительно проверяются в CI ### 🟡 `int(val)` без try/except — crash на non-numeric string `backend/app/schemas/nspd_bulk.py:75` — если NSPD когда-нибудь вернёт `category: "ЗУ"` (а они известны type-inconsistency'ю), `int(val)` рейзит ValueError посреди loop'а в `bulk_harvest.upsert_features:295` и убивает harvest остальных features в quarter. Используй уже существующий `_safe_int` из `bulk_harvest.py` или try/except → None. ### 🟡 Truthiness trap (низкий риск, но real) `properties.get("category") or properties.get("categoryId")` — если NSPD вернёт `category: 0` (наблюдаемые ID 36368/36369/37159 — все truthy, поэтому в practice safe), fallback сработает неожиданно. Правильнее: ```python val = self.properties.get("category") if val is None: val = self.properties.get("categoryId") ``` То же для meta `payload.get("meta") or data_block.get("meta")` — но для `meta=[]` поведение совпадает (оба → пустой meta_counts), оставлю как минор. ### ✅ Что хорошо - **CI зелёный** (backend 1m17s + frontend 1m35s). - **Downstream routing intact**: `bulk_harvest.py:296-318` маппит `cat == CAT_PARCEL (36368)` — `category_id` всё ещё возвращает тот же int, источник просто сменился. Подтверждает диагноз "all 50/50 routed to None". - **Meta entries shape consistent**: внутренний loop `meta_counts` всё ещё читает `categoryId` (с Id) — соответствует author'скому live payload `[{"totalCount":170,"categoryId":36368}]`. Только location переехала, parser unchanged. - **TS contract drift отсутствует** — frontend (`admin/scrape/cadastre/page.tsx`) парсит admin job-state, не NSPD feature shape. ### Что нужно 1. **Обязательно**: обновить хотя бы одну фикстуру в `test_nspd_bulk_client.py` или `test_cadastre_bulk.py` чтобы ветка `properties.category` + top-level `meta` реально выполнилась в CI. 2. **Желательно**: `_safe_int(val)` вместо голого `int(val)` в `category_id` property. 3. Минор: труcинесс → `is None` check. Жду новый SHA.

lekss361 commented

2026-05-15 12:39:53 +00:00

(Migrated from github.com)

Re-review (SHA `4f48684`)

Все блокеры из прошлого комента закрыты + cross-check (agent):

✅ Test coverage gap — closed

test_nspd_bulk_client.py:146-158 — test_nspd_bulk_feature_parse_basic теперь валидирует feature с "category": 36368 и assert'ит feat.category_id == 36368. Primary path реально выполняется.
test_cadastre_bulk.py — все 3 фикстуры (_make_parcel_feature, _make_building_feature, _make_zouit_feature) и inline-cases (37, 79, 115, 240, 254, 461, 472) теперь используют properties.category. upsert_features тестируется по primary path.
sample_quarter_response — meta переехал на top-level (выпрыгнул из data блока). test_search_by_quarter_parses_features validates meta_counts → top-level meta path exercised.

✅ Truthiness trap + int crash — closed

val = self.properties.get("category")
if val is None:
    val = self.properties.get("categoryId")
if val is None:
    return None
try:
    return int(val)
except (TypeError, ValueError):
    return None

category=0 — is not None корректно возвращает 0 (а не падает к fallback).
category="ЗУ" — ValueError caught → None (не убивает upsert_features loop).
Legacy categoryId=N fallback работает когда category отсутствует.

✅ CI зелёный

backend 1m16s + frontend 1m35s — оба новых code path реально выполняются + не сломали legacy fallback.

Минор (не блокер)

Нет отдельных юнит-тестов на int("ЗУ") и category=0 paranoia ветки — fixture edits покрыли rename, но defensive code (try/except) explicitly не тестируется. Логика прямолинейная, low risk, оставлю как минор. После pilot v4 ничто не мешает добавить параметризованный тест на error paths.

approve merge

## Re-review (SHA 4f48684) Все блокеры из прошлого комента закрыты + cross-check (agent): ### ✅ Test coverage gap — closed - `test_nspd_bulk_client.py:146-158` — `test_nspd_bulk_feature_parse_basic` теперь валидирует feature с `"category": 36368` и assert'ит `feat.category_id == 36368`. Primary path реально выполняется. - `test_cadastre_bulk.py` — все 3 фикстуры (`_make_parcel_feature`, `_make_building_feature`, `_make_zouit_feature`) и inline-cases (37, 79, 115, 240, 254, 461, 472) теперь используют `properties.category`. `upsert_features` тестируется по primary path. - `sample_quarter_response` — `meta` переехал на top-level (выпрыгнул из `data` блока). `test_search_by_quarter_parses_features` validates `meta_counts` → top-level meta path exercised. ### ✅ Truthiness trap + int crash — closed ```python val = self.properties.get("category") if val is None: val = self.properties.get("categoryId") if val is None: return None try: return int(val) except (TypeError, ValueError): return None ``` - `category=0` — `is not None` корректно возвращает 0 (а не падает к fallback). - `category="ЗУ"` — ValueError caught → None (не убивает upsert_features loop). - Legacy `categoryId=N` fallback работает когда `category` отсутствует. ### ✅ CI зелёный backend 1m16s + frontend 1m35s — оба новых code path реально выполняются + не сломали legacy fallback. ### Минор (не блокер) Нет отдельных юнит-тестов на `int("ЗУ")` и `category=0` paranoia ветки — fixture edits покрыли rename, но defensive code (try/except) explicitly не тестируется. Логика прямолинейная, low risk, оставлю как минор. После pilot v4 ничто не мешает добавить параметризованный тест на error paths. approve merge