feat(#126): Sub-PR A — NSPDClient.get_features_in_bbox_grid + engineering classifier #247

Merged

lekss361 merged 1 commit from fix/126-nspd-grid-walk-scraper into main

2026-05-17 05:54:31 +00:00

lekss361 commented

2026-05-17 05:48:57 +00:00

Owner

Summary

Foundation для #126 → #231/#234 fix. Не меняет prod behaviour — старый get_features_in_bbox (single-pixel, возвращает 0-3 features) депрекейтнут только в docstring; всё ещё вызывается из search_by_quarter. Switch произойдёт в Sub-PR B.

Что добавлено

NSPDClient.get_features_in_bbox_grid(layer_id, bbox, *, grid_n=7, step_m=50) в backend/app/services/scrapers/nspd_client.py:
- Делегирует grid_n×grid_n WMS GetFeatureInfo calls через NSPDBulkClient.wms_feature_info
- Async httpx + Semaphore(3) для concurrency cap
- Dedup cascade: feature_id → cad_num → reg_numb_border
classify_engineering_kind(properties) в nspd_denorm.py — regex classifier для layer 36328 structures: water | sewage | gas | heat | electric | other. Cyrillic patterns из post-mortem (gas — газопровод|ГРП|ГК, electric — ВЛ|КЛ|ТП|ЛЭП|подстанц, и т.д.).
31 unit-test (mock-based, no live HTTP), 1 integration skip placeholder.

Pre-push review

deep-code-reviewer ✅ APPROVE с 1 minor yellow:

🟡 Dedup gap при feature_id=None / cad_num=None / reg_numb_border=None — features без всех key полей могут проходить N раз через соседние ячейки. Не блокер для foundation PR; flag для Sub-PR B (геометрический hash fallback).
✅ asyncio.run() safety verified — NSPDClient импортируется только из Celery sync tasks
✅ Module-level Semaphore(3) thread-safe (Celery prefork = отдельный event loop per worker)
✅ ruff line ≤100, no print, no requests, httpx only, psycopg patterns OK
✅ No live HTTP в тестах (verified all AsyncMock)

Plan context (4 sub-PRs)

A (this PR) — Foundation: grid-walk method + classifier
C — UX mitigation для #234: harvest ETA badge + SETNX dedupe (можно параллельно)
B — Integration: switch search_by_quarter на grid-walk (depends A)
D — Operations: re-enable beat schedule + ЕКБ back-fill + live contract test (depends B)

Test plan

cd backend && uv run pytest tests/scrapers/test_nspd_grid_walk.py -v → 31 passed, 1 skipped
uv run ruff check + ruff format --check → All checks passed
Pre-commit hooks pass
No regression: cd backend && uv run pytest tests/scrapers/ -k "not grid_walk" 2>&1 | tail -5 (legacy tests still green)

Bug post-mortem: fixes/Bug_NSPD_WMS_NotBulk_2026_May14.md (vault)
Foundation: vault code/modules/NSPD_Bulk_Client.md updated
Blocker для: #231 / #234 (без grid-walk dumps пусты, fallback ZOUIT в PR #240 не активируется)

Refs #126, #231, #234

## Summary **Foundation для #126 → #231/#234 fix.** Не меняет prod behaviour — старый `get_features_in_bbox` (single-pixel, возвращает 0-3 features) депрекейтнут только в docstring; всё ещё вызывается из `search_by_quarter`. Switch произойдёт в Sub-PR B. ### Что добавлено 1. **`NSPDClient.get_features_in_bbox_grid(layer_id, bbox, *, grid_n=7, step_m=50)`** в `backend/app/services/scrapers/nspd_client.py`: - Делегирует `grid_n×grid_n` WMS `GetFeatureInfo` calls через `NSPDBulkClient.wms_feature_info` - Async httpx + `Semaphore(3)` для concurrency cap - Dedup cascade: `feature_id` → `cad_num` → `reg_numb_border` 2. **`classify_engineering_kind(properties)` в `nspd_denorm.py`** — regex classifier для layer 36328 structures: `water | sewage | gas | heat | electric | other`. Cyrillic patterns из post-mortem (gas — `газопровод|ГРП|ГК`, electric — `ВЛ|КЛ|ТП|ЛЭП|подстанц`, и т.д.). 3. **31 unit-test** (mock-based, no live HTTP), 1 integration skip placeholder. ### Pre-push review deep-code-reviewer ✅ APPROVE с 1 minor yellow: - 🟡 Dedup gap при `feature_id=None / cad_num=None / reg_numb_border=None` — features без всех key полей могут проходить N раз через соседние ячейки. Не блокер для foundation PR; flag для Sub-PR B (геометрический hash fallback). - ✅ `asyncio.run()` safety verified — NSPDClient импортируется только из Celery sync tasks - ✅ Module-level `Semaphore(3)` thread-safe (Celery prefork = отдельный event loop per worker) - ✅ ruff line ≤100, no print, no requests, httpx only, psycopg patterns OK - ✅ No live HTTP в тестах (verified all AsyncMock) ### Plan context (4 sub-PRs) - **A (this PR)** — Foundation: grid-walk method + classifier - C — UX mitigation для #234: harvest ETA badge + SETNX dedupe (можно параллельно) - B — Integration: switch `search_by_quarter` на grid-walk (depends A) - D — Operations: re-enable beat schedule + ЕКБ back-fill + live contract test (depends B) ### Test plan - [x] `cd backend && uv run pytest tests/scrapers/test_nspd_grid_walk.py -v` → 31 passed, 1 skipped - [x] `uv run ruff check + ruff format --check` → All checks passed - [x] Pre-commit hooks pass - [ ] No regression: `cd backend && uv run pytest tests/scrapers/ -k "not grid_walk" 2>&1 | tail -5` (legacy tests still green) ### Related - Bug post-mortem: `fixes/Bug_NSPD_WMS_NotBulk_2026_May14.md` (vault) - Foundation: vault `code/modules/NSPD_Bulk_Client.md` updated - Blocker для: #231 / #234 (без grid-walk dumps пусты, fallback ZOUIT в PR #240 не активируется) Refs #126, #231, #234

lekss361 added 1 commit 2026-05-17 05:48:57 +00:00

feat(#126 ): Sub-PR A — NSPDClient.get_features_in_bbox_grid + engineering classifier 7338693500

Add get_features_in_bbox_grid(layer_id, bbox, *, grid_n=7, step_m=50) to NSPDClient:
delegates grid_n×grid_n WMS GetFeatureInfo calls via NSPDBulkClient.wms_feature_info
(async httpx + Semaphore(3)), deduplicates by feature_id/cad_num/reg_numb_border.

Deprecate get_features_in_bbox (single-pixel, 0-3 results) in docstring — not removed,
used by search_by_quarter (Sub-PR B).

Add classify_engineering_kind(properties) in nspd_denorm.py — regex classifier for
layer 36328 structures: water/sewage/gas/heat/electric/other.

Tests: 31 unit tests (mock-based, no live HTTP), 1 integration skip placeholder.
Foundation only — no behaviour change in harvest_quarter (Sub-PR B).

lekss361 commented

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

Author

Owner

Deep review — verdict ✅ APPROVE

Scope: foundation only, dead code до Sub-PR B. Bar для merge снижен — главное no regression в существующих тестах. Подтверждено.

Cross-file impact

from app.scrapers.nspd_bulk_client import NSPDBulkClient — путь существует (backend/app/scrapers/nspd_bulk_client.py:106), wms_feature_info() есть (line 321), _SEMAPHORE module-level (line 61).
Callers NSPDClient: только backend/app/workers/tasks/nspd_sync.py:289 (Celery sync def harvest_quarter) + tests. Async caller отсутствует → asyncio.run() safe.
Старый get_features_in_bbox сохранён (используется в search_by_quarter:578). Legacy test test_get_features_in_bbox_url_builds_correctly зелёный.

Correctness

✅ asyncio.run() pattern OK (sync caller, fresh event loop per Celery worker process).
✅ Module-level Semaphore(3) per-event-loop — Celery prefork = process isolation.
✅ return_exceptions=True + warning log per cell error — partial failures не валят весь grid.
✅ Auto-reduce effective_n для маленьких bbox (избегает деления на ноль).
🟡 Dedup gap (self-acknowledged): если feature_id/cad_num/cad_number/reg_numb_border/label все None — feature попадает в deduped без записи в seen, следующая такая же тоже попадёт. В практике NSPD WMS features почти всегда имеют id, но для Sub-PR B стоит добавить geometric hash fallback (e.g. hash(json.dumps(feat.geometry, sort_keys=True))).
🟡 Test test_field_priority_params_name_over_purpose docstring говорит «params_name проверяется раньше purpose», но фактически логика склеивает все поля в одну строку и порядок решает _ENGINEERING_PATTERNS. Тест проходит, но название/docstring вводят в заблуждение. Nit, fix в Sub-PR B.

Performance

7×7=49 calls × Semaphore(3) ≈ ~17 batches × ~150ms = ~2.5s per quarter (приемлемо для monthly harvest).
⚠️ Note для Sub-PR B: 8000 кварталов × 49 = 392k requests, при _BACKOFF_BASE_S=2.0 (на 429) — нужен budget-monitor. Не блокер для A.

Conventions

✅ ruff line ≤100, no print, no requests, httpx only, no psycopg patterns (PR не трогает БД).
✅ Cyrillic regex с re.IGNORECASE. ВЛ[\s\-]|ВЛ-?\d достаточно safe — false positive на «вложение» исключён (требуется пробел/дефис/цифра после).
✅ Mock-only tests, integration marker skip placeholder.

Vault

✅ fixes/Bug_NSPD_WMS_NotBulk_2026_May14.md — post-mortem существует, упомянут в docstring и nspd_denorm.py.
✅ code/modules/NSPD_Bulk_Client.md — Update section 2026-05-17 для Sub-PR A добавлена.
✅ Decision tracked в research/Bulk_Cadastre_Ingest_Plan_May15.md.

Risk / Reversibility

Risk: Low — dead code, не вызывается из prod paths.
Reversibility: trivial revert (1 commit, 3 files, нет миграций / data changes).

Мержу via squash. Yellow dedup flag оставляю как follow-up для Sub-PR B.

## Deep review — verdict ✅ APPROVE **Scope:** foundation only, dead code до Sub-PR B. Bar для merge снижен — главное no regression в существующих тестах. Подтверждено. ### Cross-file impact - `from app.scrapers.nspd_bulk_client import NSPDBulkClient` — путь существует (`backend/app/scrapers/nspd_bulk_client.py:106`), `wms_feature_info()` есть (line 321), `_SEMAPHORE` module-level (line 61). - Callers `NSPDClient`: только `backend/app/workers/tasks/nspd_sync.py:289` (Celery sync `def harvest_quarter`) + tests. Async caller отсутствует → `asyncio.run()` safe. - Старый `get_features_in_bbox` сохранён (используется в `search_by_quarter:578`). Legacy test `test_get_features_in_bbox_url_builds_correctly` зелёный. ### Correctness - ✅ `asyncio.run()` pattern OK (sync caller, fresh event loop per Celery worker process). - ✅ Module-level `Semaphore(3)` per-event-loop — Celery prefork = process isolation. - ✅ `return_exceptions=True` + warning log per cell error — partial failures не валят весь grid. - ✅ Auto-reduce `effective_n` для маленьких bbox (избегает деления на ноль). - 🟡 **Dedup gap (self-acknowledged):** если `feature_id`/`cad_num`/`cad_number`/`reg_numb_border`/`label` все None — feature попадает в `deduped` без записи в `seen`, следующая такая же тоже попадёт. В практике NSPD WMS features почти всегда имеют `id`, но для Sub-PR B стоит добавить geometric hash fallback (e.g. `hash(json.dumps(feat.geometry, sort_keys=True))`). - 🟡 **Test `test_field_priority_params_name_over_purpose`** docstring говорит «params_name проверяется раньше purpose», но фактически логика склеивает все поля в одну строку и порядок решает `_ENGINEERING_PATTERNS`. Тест проходит, но название/docstring вводят в заблуждение. Nit, fix в Sub-PR B. ### Performance - 7×7=49 calls × Semaphore(3) ≈ ~17 batches × ~150ms = ~2.5s per quarter (приемлемо для monthly harvest). - ⚠️ Note для Sub-PR B: 8000 кварталов × 49 = 392k requests, при `_BACKOFF_BASE_S=2.0` (на 429) — нужен budget-monitor. Не блокер для A. ### Conventions - ✅ ruff line ≤100, no `print`, no `requests`, `httpx` only, no psycopg patterns (PR не трогает БД). - ✅ Cyrillic regex с `re.IGNORECASE`. `ВЛ[\s\-]|ВЛ-?\d` достаточно safe — false positive на «вложение» исключён (требуется пробел/дефис/цифра после). - ✅ Mock-only tests, integration marker skip placeholder. ### Vault - ✅ `fixes/Bug_NSPD_WMS_NotBulk_2026_May14.md` — post-mortem существует, упомянут в docstring и `nspd_denorm.py`. - ✅ `code/modules/NSPD_Bulk_Client.md` — Update section 2026-05-17 для Sub-PR A добавлена. - ✅ Decision tracked в `research/Bulk_Cadastre_Ingest_Plan_May15.md`. ### Risk / Reversibility - **Risk:** Low — dead code, не вызывается из prod paths. - **Reversibility:** trivial revert (1 commit, 3 files, нет миграций / data changes). Мержу via squash. Yellow dedup flag оставляю как follow-up для Sub-PR B.