lekss361/gendesign

Fork 0

feat(#263 sub-A): cad_* overlay schemas + cadastre territorial_zones (ПЗЗ) #265

Merged

lekss361 merged 2 commits from feat/263-sub-a-cad-nspd-overlay-schemas-and-pzz into main

2026-05-17 07:50:09 +00:00

lekss361 commented

2026-05-17 07:29:54 +00:00

Owner

Summary

NSPD unify Вариант B — Sub-PR A (foundation + ПЗЗ writer). Epic: #263.

Phase 1 (migration 101): создаёт 5 новых таблиц — cad_territorial_zones, cad_red_lines, cad_engineering_structures, cad_risk_zones, cad_opportunity_parcels. Idempotent (IF NOT EXISTS), GIST-индексы.
Phase 2 partial (ПЗЗ layer 875838 only): NSPDBulkClient.get_features_in_bbox_grid (async grid-walk) + get_territorial_zones_in_bbox wrapper + harvest_quarter Phase 2.5 + _save_territorial_zones UPSERT helper.
9 unit tests (mock DB): insert 3 rows, empty list, no-geom, no-id fallback, error isolation, savepoint-per-row, quarter_cad param, raw_props JSON.

Что НЕ в этом PR

red_lines, engineering, risks, opportunity harvest — Sub-PR B/C
Refactor quarter_dump_lookup._get_nspd_zoning чтобы читать из cad_territorial_zones — Sub-PR D
Cleanup nspd_quarter_dumps / nspd_sync — Sub-PR F

Resolves (после полного epic)

#259 — pzz_sync deprecated (после Sub-PR D quarter_dump_lookup переключится на cad_territorial_zones)
Часть #263 (Phase 1 complete + Phase 2 partial — ПЗЗ)

Test plan

9/9 unit tests passed
Migration 101 applies без ошибок (auto-apply через deploy.yml)
Verify 5 tables: SELECT tablename FROM pg_tables WHERE tablename IN ('cad_territorial_zones','cad_red_lines','cad_engineering_structures','cad_risk_zones','cad_opportunity_parcels');

Smoke post-merge:

curl -X POST https://gendsgn.ru/api/v1/admin/cadastre/jobs \
  -H "X-Admin-Token: $TOKEN" -H "Content-Type: application/json" \
  -d '{"scope":"manual_list","quarters":["66:41:0204016"]}'
# → expect job_id

# Через ~30-60 сек:
SELECT count(*), max(fetched_at) FROM cad_territorial_zones WHERE quarter_cad='66:41:0204016';
# → expect > 0 rows

## Summary NSPD unify Вариант B — **Sub-PR A** (foundation + ПЗЗ writer). Epic: #263. - **Phase 1 (migration 101):** создаёт 5 новых таблиц — `cad_territorial_zones`, `cad_red_lines`, `cad_engineering_structures`, `cad_risk_zones`, `cad_opportunity_parcels`. Idempotent (`IF NOT EXISTS`), GIST-индексы. - **Phase 2 partial (ПЗЗ layer 875838 only):** `NSPDBulkClient.get_features_in_bbox_grid` (async grid-walk) + `get_territorial_zones_in_bbox` wrapper + `harvest_quarter` Phase 2.5 + `_save_territorial_zones` UPSERT helper. - **9 unit tests** (mock DB): insert 3 rows, empty list, no-geom, no-id fallback, error isolation, savepoint-per-row, quarter_cad param, raw_props JSON. ## Что НЕ в этом PR - red_lines, engineering, risks, opportunity harvest — **Sub-PR B/C** - Refactor `quarter_dump_lookup._get_nspd_zoning` чтобы читать из `cad_territorial_zones` — **Sub-PR D** - Cleanup `nspd_quarter_dumps` / `nspd_sync` — **Sub-PR F** ## Resolves (после полного epic) - #259 — pzz_sync deprecated (после Sub-PR D quarter_dump_lookup переключится на cad_territorial_zones) - Часть #263 (Phase 1 complete + Phase 2 partial — ПЗЗ) ## Test plan - [x] 9/9 unit tests passed - [ ] Migration 101 applies без ошибок (auto-apply через deploy.yml) - [ ] Verify 5 tables: `SELECT tablename FROM pg_tables WHERE tablename IN ('cad_territorial_zones','cad_red_lines','cad_engineering_structures','cad_risk_zones','cad_opportunity_parcels');` - [ ] Smoke post-merge: ```bash curl -X POST https://gendsgn.ru/api/v1/admin/cadastre/jobs \ -H "X-Admin-Token: $TOKEN" -H "Content-Type: application/json" \ -d '{"scope":"manual_list","quarters":["66:41:0204016"]}' # → expect job_id # Через ~30-60 сек: SELECT count(*), max(fetched_at) FROM cad_territorial_zones WHERE quarter_cad='66:41:0204016'; # → expect > 0 rows ```

lekss361 added 1 commit 2026-05-17 07:29:55 +00:00

feat(#263 sub-A): cad_* overlay schemas + cadastre territorial_zones (ПЗЗ) f08db43145

Phase 1 (migration 101): создаёт 5 новых таблиц cad_territorial_zones,
cad_red_lines, cad_engineering_structures, cad_risk_zones,
cad_opportunity_parcels. Idempotent (IF NOT EXISTS), GIST-индексы.

Phase 2 partial (ПЗЗ only):
- NSPDBulkClient.get_features_in_bbox_grid — async grid-walk WMS GetFeatureInfo
- NSPDBulkClient.get_territorial_zones_in_bbox — convenience wrapper layer 875838
- harvest_quarter Phase 2.5 — вызов get_territorial_zones_in_bbox после grid-walk
- _save_territorial_zones — UPSERT loop с SAVEPOINT per-row, ON CONFLICT zone_id

9 unit tests (mock DB): insert/upsert/no-geom/no-id/error-isolation/savepoint pattern.

lekss361 commented

2026-05-17 07:34:41 +00:00

Author

Owner

Deep Code Review — verdict: 🔴 BLOCK

Не мержу. Один корректностный блокер + одно SQL-замечание.

🔴 Critical — CRS mismatch (broken at runtime)

backend/app/services/cadastre/bulk_harvest.py:1218 + data/sql/101_cad_nspd_overlay_tables.sql:18

harvest_quarter Phase 2.5 берёт quarter_bbox_3857(db, quarter) (EPSG:3857, метры) → грид-walk возвращает features в EPSG:3857. Дальше _save_territorial_zones пишет:

ST_GeogFromGeoJSON(CAST(:geom AS text))   -- ожидает WGS84 lon/lat

В колонку geom GEOGRAPHY(POLYGON, 4326).

GeoJSON приходит с координатами вида 6090000.0, 7590000.0 (тест-фикстура это подтверждает) → ST_GeogFromGeoJSON либо упадёт, либо положит "longitude=6090000" в geography. SAVEPOINT проглотит exception → inserted останется 0 на всех тайлах. Получится тихий no-op.

Сравни с другими апсертами в этом же файле (line 477, 591, 714, 810, …):

ST_SetSRID(ST_GeomFromGeoJSON(CAST(:geom AS text)), 3857)
-- + ST_Transform(..., 4326) когда нужно WGS84

Fix: либо переключить колонку на GEOMETRY(POLYGON, 3857), либо в апсерте:

ST_Transform(ST_SetSRID(ST_GeomFromGeoJSON(CAST(:geom AS text)), 3857), 4326)::geography

и убрать ST_GeogFromGeoJSON.

🟠 High — UPSERT ON CONFLICT таргет не покрывает no-geom path

zone_id fallback f"{quarter_cad}_{inserted}" использует счётчик inserted который инкрементится только при успехе. При повторном запуске на тех же фичах с пустыми id — fallback id будут совпадать друг с другом в рамках одного прогона, но между прогонами счётчик начнётся с 0 заново → ON CONFLICT обновит "не ту" фичу. Не критично пока NSPD стабильно отдаёт id, но как fallback ненадёжен. Лучше f"{quarter_cad}_{hashlib.md5(json.dumps(props, sort_keys=True).encode()).hexdigest()[:12]}".

🟡 Medium — naming collision миграции 101

В main уже есть data/sql/101_user_custom_pois.sql (PR #257). Этот PR добавляет 101_cad_nspd_overlay_tables.sql. Функционально OK (deploy.yml: ls | sort, оба идемпотентны, разные таблицы, трекаются по filename в _schema_migrations независимо), но конвенция NN_ требует уникальный номер. Переименовать в 102_cad_nspd_overlay_tables.sql — 30 секунд работы.

🟡 Medium — `_save_territorial_zones`: одиночный `db.commit()` в конце

with db.begin_nested(): создаёт savepoint, но если outer transaction не была начата (autocommit-style session) — commit() в конце ничего не сделает / savepoint не сработает. У других апсертов в файле обычно outer tx уже открыт caller'ом. Проверить, что harvest_quarter гарантирует transaction context до этого вызова. Если нет — обернуть with db.begin():.

✅ Positive

Semaphore pattern корректный — новый get_features_in_bbox_grid использует self.wms_feature_info который через _get_json берёт self._sem (per-instance, инициализирован в __aenter__). После #260 module-level _SEMAPHORE удалён — здесь не используется. 👍
psycopg v3 + CAST(:x AS type) соблюдён везде.
9 unit тестов покрывают edge-cases (no-geom, no-id, error-isolation, savepoint-per-row).
BEGIN; … COMMIT; обёртка, IF NOT EXISTS, GIST-индексы — sql.md ✓.
Vault entry code/modules/Cad_NSPD_Overlay_Tables.md есть.

Action

После фикса CRS (🔴) + rename миграции в 102 (🟡) + желательно стабильный fallback zone_id (🟠) — повторный re-review, апрув, мерж.

Не мержу.

## Deep Code Review — verdict: 🔴 BLOCK Не мержу. Один корректностный блокер + одно SQL-замечание. ### 🔴 Critical — CRS mismatch (broken at runtime) **`backend/app/services/cadastre/bulk_harvest.py:1218`** + **`data/sql/101_cad_nspd_overlay_tables.sql:18`** `harvest_quarter` Phase 2.5 берёт `quarter_bbox_3857(db, quarter)` (EPSG:3857, метры) → грид-walk возвращает features в **EPSG:3857**. Дальше `_save_territorial_zones` пишет: ```sql ST_GeogFromGeoJSON(CAST(:geom AS text)) -- ожидает WGS84 lon/lat ``` В колонку `geom GEOGRAPHY(POLYGON, 4326)`. GeoJSON приходит с координатами вида `6090000.0, 7590000.0` (тест-фикстура это подтверждает) → `ST_GeogFromGeoJSON` либо упадёт, либо положит "longitude=6090000" в `geography`. SAVEPOINT проглотит exception → `inserted` останется 0 на всех тайлах. Получится тихий no-op. Сравни с другими апсертами в этом же файле (line 477, 591, 714, 810, …): ```sql ST_SetSRID(ST_GeomFromGeoJSON(CAST(:geom AS text)), 3857) -- + ST_Transform(..., 4326) когда нужно WGS84 ``` **Fix:** либо переключить колонку на `GEOMETRY(POLYGON, 3857)`, либо в апсерте: ```sql ST_Transform(ST_SetSRID(ST_GeomFromGeoJSON(CAST(:geom AS text)), 3857), 4326)::geography ``` и убрать `ST_GeogFromGeoJSON`. ### 🟠 High — UPSERT ON CONFLICT таргет не покрывает no-geom path `zone_id` fallback `f"{quarter_cad}_{inserted}"` использует счётчик `inserted` который инкрементится **только при успехе**. При повторном запуске на тех же фичах с пустыми id — fallback id будут совпадать друг с другом в рамках одного прогона, но **между прогонами** счётчик начнётся с 0 заново → ON CONFLICT обновит "не ту" фичу. Не критично пока NSPD стабильно отдаёт `id`, но как fallback ненадёжен. Лучше `f"{quarter_cad}_{hashlib.md5(json.dumps(props, sort_keys=True).encode()).hexdigest()[:12]}"`. ### 🟡 Medium — naming collision миграции 101 В main уже есть `data/sql/101_user_custom_pois.sql` (PR #257). Этот PR добавляет `101_cad_nspd_overlay_tables.sql`. Функционально OK (deploy.yml: `ls | sort`, оба идемпотентны, разные таблицы, трекаются по filename в `_schema_migrations` независимо), но конвенция NN_ требует уникальный номер. Переименовать в `102_cad_nspd_overlay_tables.sql` — 30 секунд работы. ### 🟡 Medium — `_save_territorial_zones`: одиночный `db.commit()` в конце `with db.begin_nested():` создаёт savepoint, но если outer transaction не была начата (autocommit-style session) — `commit()` в конце ничего не сделает / savepoint не сработает. У других апсертов в файле обычно outer tx уже открыт caller'ом. Проверить, что `harvest_quarter` гарантирует transaction context до этого вызова. Если нет — обернуть `with db.begin():`. ### ✅ Positive - Semaphore pattern **корректный** — новый `get_features_in_bbox_grid` использует `self.wms_feature_info` который через `_get_json` берёт `self._sem` (per-instance, инициализирован в `__aenter__`). После #260 module-level `_SEMAPHORE` удалён — здесь не используется. 👍 - `psycopg v3` + `CAST(:x AS type)` соблюдён везде. - 9 unit тестов покрывают edge-cases (no-geom, no-id, error-isolation, savepoint-per-row). - `BEGIN; … COMMIT;` обёртка, `IF NOT EXISTS`, GIST-индексы — sql.md ✓. - Vault entry `code/modules/Cad_NSPD_Overlay_Tables.md` есть. ### Action После фикса CRS (🔴) + rename миграции в 102 (🟡) + желательно стабильный fallback zone_id (🟠) — повторный re-review, апрув, мерж. Не мержу.

lekss361 referenced this pull request from a commit

2026-05-17 07:46:08 +00:00

fix(#265 review): CRS 3857→4326 + stable zone_id hash + rename 102 + tx comment

lekss361 added 1 commit 2026-05-17 07:46:09 +00:00

fix(#265 review): CRS 3857→4326 + stable zone_id hash + rename 102 + tx comment 8e377059a4

- #1 (BLOCK): ST_GeogFromGeoJSON → ST_Transform(ST_SetSRID(..., 3857), 4326)::geography
  so EPSG:3857 features from get_territorial_zones_in_bbox are correctly
  stored in GEOGRAPHY(POLYGON, 4326) column (matches cad_parcels/cad_buildings pattern)
- #2: zone_id fallback changed from counter-based f"{quarter_cad}_{inserted}"
  to md5(json(props))[:12] — stable between runs, prevents wrong-row ON CONFLICT
- #3: rename 101_cad_nspd_overlay_tables.sql → 102_* (101_user_custom_pois.sql exists)
- #4: add comment clarifying SQLAlchemy autobegin guarantees outer tx before begin_nested()
- Tests: updated test_feature_without_zone_id_uses_fallback to assert md5 format + idempotency

lekss361 commented

2026-05-17 07:47:49 +00:00

Author

Owner

Все 4 findings зафиксил в коммите 8e37705. Re-review please.

🔴 #1 CRS: ST_GeogFromGeoJSON(...) → ST_Transform(ST_SetSRID(ST_GeomFromGeoJSON(CAST(:geom AS text)), 3857), 4326)::geography
🟠 #2 zone_id fallback: f"{quarter_cad}_{inserted}" → f"{quarter_cad}_{md5(props_sorted)[:12]}" (stable между runs)
🟡 #3 migration rename: 101_cad_nspd_overlay_tables.sql → 102_* (избежать collision с PR #257 migration 101)
🟡 #4 outer tx: добавлен comment про SQLAlchemy autobegin гарантию (begin_nested работает корректно)

Files: bulk_harvest.py (+hashlib import, 2 SQL fixes, comment), test_bulk_harvest_territorial.py (test обновлён на md5 + idempotency check), 102_cad_nspd_overlay_tables.sql (rename), vault Cad_NSPD_Overlay_Tables.md (filename + CRS note).

Tests: 9/9 pass. Pre-commit hooks: all pass.

Все 4 findings зафиксил в коммите `8e37705`. Re-review please. - **🔴 #1 CRS:** `ST_GeogFromGeoJSON(...)` → `ST_Transform(ST_SetSRID(ST_GeomFromGeoJSON(CAST(:geom AS text)), 3857), 4326)::geography` - **🟠 #2 zone_id fallback:** `f"{quarter_cad}_{inserted}"` → `f"{quarter_cad}_{md5(props_sorted)[:12]}"` (stable между runs) - **🟡 #3 migration rename:** `101_cad_nspd_overlay_tables.sql` → `102_*` (избежать collision с PR #257 migration 101) - **🟡 #4 outer tx:** добавлен comment про SQLAlchemy autobegin гарантию (begin_nested работает корректно) **Files:** `bulk_harvest.py` (+hashlib import, 2 SQL fixes, comment), `test_bulk_harvest_territorial.py` (test обновлён на md5 + idempotency check), `102_cad_nspd_overlay_tables.sql` (rename), vault `Cad_NSPD_Overlay_Tables.md` (filename + CRS note). **Tests:** 9/9 pass. **Pre-commit hooks:** all pass.

lekss361 commented

2026-05-17 07:50:01 +00:00

Author

Owner

Deep Code Review (round 2) — verdict APPROVE

Head SHA 8e377059 — все 4 finding'а из первого review закрыты.

Что изменилось в followup (+31/-8)

#	Was	Now	Status
1 BLOCK	`ST_GeogFromGeoJSON(geom)` → 3857 coords в GEOGRAPHY(4326)	`ST_Transform(ST_SetSRID(ST_GeomFromGeoJSON(...), 3857), 4326)::geography`	fixed
2 HIGH	`f"{quarter_cad}_{inserted}"` (counter resets)	`f"{quarter_cad}_{md5(json(props, sort_keys=True))[:12]}"` — стабильный между runs	fixed
3 MED	`101_cad_nspd_overlay_tables.sql` (collision с 101_user_custom_pois)	renamed → `102_cad_nspd_overlay_tables.sql`	fixed
4 MED	`db.begin_nested()` без явного outer tx comment	добавлен comment про SQLAlchemy autobegin	fixed
Tests	counter-based fallback assertion	`test_feature_without_zone_id_uses_fallback` — md5 формат (12 hex) + idempotency check (вызов 2 раза → тот же zone_id)	updated

Verified

CRS fix матчит pattern в bulk_harvest.py lines 477/591/714/810 (cad_parcels/cad_buildings/etc) — consistency OK
md5 deterministic: json.dumps(props, sort_keys=True) → одинаковый input → одинаковый zone_id → ON CONFLICT (zone_id) DO UPDATE сработает корректно при rerun
CASE WHEN CAST(:geom AS text) IS NOT NULL сохраняет no-geom path (test_feature_without_geometry_no_crash проходит)
CAST(:geom AS text) psycopg v3 convention сохранён
Vault code/modules/Cad_NSPD_Overlay_Tables.md обновлён с CRS pattern + md5 fallback + outer tx note

Smoke test после deploy

curl -X POST https://gendsgn.ru/api/v1/admin/cadastre/jobs \
  -H "X-Admin-Token: $TOKEN" -H "Content-Type: application/json" \
  -d '{"scope":"manual_list","quarters":["66:41:0204016"]}'

# ~30-60 сек:
SELECT count(*) FROM cad_territorial_zones WHERE quarter_cad='66:41:0204016';
-- expect > 0

-- Critical: verify CRS — ST_AsText должен вернуть lon/lat (60.x, 56.x), НЕ метры 3857 (6090000)
SELECT zone_id, ST_AsText(geom::geometry) FROM cad_territorial_zones
WHERE quarter_cad='66:41:0204016' LIMIT 1;

Merging via deep-code-reviewer.

## Deep Code Review (round 2) — verdict APPROVE Head SHA `8e377059` — все 4 finding'а из первого review закрыты. ### Что изменилось в followup (+31/-8) | # | Was | Now | Status | |---|---|---|---| | 1 BLOCK | `ST_GeogFromGeoJSON(geom)` → 3857 coords в GEOGRAPHY(4326) | `ST_Transform(ST_SetSRID(ST_GeomFromGeoJSON(...), 3857), 4326)::geography` | fixed | | 2 HIGH | `f"{quarter_cad}_{inserted}"` (counter resets) | `f"{quarter_cad}_{md5(json(props, sort_keys=True))[:12]}"` — стабильный между runs | fixed | | 3 MED | `101_cad_nspd_overlay_tables.sql` (collision с 101_user_custom_pois) | renamed → `102_cad_nspd_overlay_tables.sql` | fixed | | 4 MED | `db.begin_nested()` без явного outer tx comment | добавлен comment про SQLAlchemy autobegin | fixed | | Tests | counter-based fallback assertion | `test_feature_without_zone_id_uses_fallback` — md5 формат (12 hex) + idempotency check (вызов 2 раза → тот же zone_id) | updated | ### Verified - CRS fix матчит pattern в `bulk_harvest.py` lines 477/591/714/810 (cad_parcels/cad_buildings/etc) — consistency OK - md5 deterministic: `json.dumps(props, sort_keys=True)` → одинаковый input → одинаковый zone_id → `ON CONFLICT (zone_id) DO UPDATE` сработает корректно при rerun - `CASE WHEN CAST(:geom AS text) IS NOT NULL` сохраняет no-geom path (test_feature_without_geometry_no_crash проходит) - `CAST(:geom AS text)` psycopg v3 convention сохранён - Vault `code/modules/Cad_NSPD_Overlay_Tables.md` обновлён с CRS pattern + md5 fallback + outer tx note ### Smoke test после deploy ```bash curl -X POST https://gendsgn.ru/api/v1/admin/cadastre/jobs \ -H "X-Admin-Token: $TOKEN" -H "Content-Type: application/json" \ -d '{"scope":"manual_list","quarters":["66:41:0204016"]}' # ~30-60 сек: SELECT count(*) FROM cad_territorial_zones WHERE quarter_cad='66:41:0204016'; -- expect > 0 -- Critical: verify CRS — ST_AsText должен вернуть lon/lat (60.x, 56.x), НЕ метры 3857 (6090000) SELECT zone_id, ST_AsText(geom::geometry) FROM cad_territorial_zones WHERE quarter_cad='66:41:0204016' LIMIT 1; ``` Merging via deep-code-reviewer.

lekss361 merged commit f160c5efaf into main

2026-05-17 07:50:09 +00:00

lekss361 referenced this pull request from a commit

2026-05-17 07:50:09 +00:00

Merge pull request 'feat(#263 sub-A): cad_* overlay schemas + cadastre territorial_zones (ПЗЗ)' (#265) from feat/263-sub-a-cad-nspd-overlay-schemas-and-pzz into main