feat(nspd): TIER 3 risk zones (#94 PR1 of 4) #217

Merged
lekss361 merged 1 commit from feat/94-pr1-risk-zones into main 2026-05-16 15:03:19 +00:00
Owner

Summary

Sub-PR 1 of 4 для #94 (NSPD extended layers).

  • harvest_stale_quarters fanout теперь передаёт include_risks=True → 11 TIER 3 layers (подтопление, оползни, гари, эрозии и т.п.) попадают в nspd_quarter_dumps.features_json
  • /analyze извлекает risk zones пересекающие участок: ST_Intersection(...::geography) для metric area
  • Frontend block NspdRiskZonesBlock — severity color-coded badges (🔴 затопление/оползни, 🟡 подтопление/гари, 🟠 эрозии)
  • Helper _extract_features_by_layer — generic для re-use в PR2 (opportunity + red lines)
  • 11 unit tests (extract / intersect / null-area / db-exception / label fallback)

Roadmap (#94 decomposition)

  • PR1: TIER 3 risk zones (этот PR)
  • PR2: TIER 4 opportunity + red lines (~400 LOC)
  • PR3: Denormalized entity tables nspd_parcels + nspd_buildings (~380 LOC)
  • PR4: TIER 5 ARN квартирография heatmap (~450 LOC)

Files (9, +545 LOC)

  • backend/app/workers/tasks/nspd_sync.py (+5) — fanout flag
  • backend/app/services/site_finder/quarter_dump_lookup.py (+156) — _RISK_SUBTYPE_LABELS + _extract_features_by_layer + _get_risk_zones
  • backend/app/schemas/parcel.py (+12) — RiskZone
  • backend/app/api/v1/parcels.py (+5) — analyze response field
  • backend/tests/services/test_quarter_dump_lookup.py (+202) — 11 tests
  • frontend/src/components/site-finder/NspdRiskZonesBlock.tsx (+147)
  • frontend/src/components/site-finder/LandTab.tsx (+14)
  • frontend/src/types/nspd.ts (+9)
  • frontend/src/types/site-finder.ts (+1)

Test plan

  • Deploy + curl /api/v1/parcels/analyze на cad_num с известной risk zone (например quarter под подтоплением) → response содержит nspd_risk_zones[]
  • UI: LandTab → NspdRiskZonesBlock виден с badges если intersect, иначе зелёный "Риски не обнаружены"
  • Celery: harvest_stale_quarters.delay() → проверить логи что include_risks=True передан
  • Pytest: cd backend && uv run pytest tests/services/test_quarter_dump_lookup.py -v → 11/11 pass

Pre-push checks

  • ruff + ruff-format: pass
  • code-reviewer subagent: APPROVE (схема columns risks_count/features_json verified против live DB, SRID consistency через ST_SetSRID + ST_Transform, no f-string SQL, no XSS)
  • phantom column gate (PR #216): SQL queries в PR1 используют real columns — clean

Known follow-ups

  • Severity mapping (NspdRiskZonesBlock.getSeverity) explicitly covers 4 из 11 layers; остальные default → "low". Расширить в PR2 если нужно тонкое разделение.
  • _get_risk_zones использует bare except Exception (consistent с sibling helpers); narrow to (OperationalError, ProgrammingError) — отдельный refactor PR.

Part of #94

## Summary **Sub-PR 1 of 4** для #94 (NSPD extended layers). - `harvest_stale_quarters` fanout теперь передаёт `include_risks=True` → 11 TIER 3 layers (подтопление, оползни, гари, эрозии и т.п.) попадают в `nspd_quarter_dumps.features_json` - `/analyze` извлекает risk zones пересекающие участок: `ST_Intersection(...::geography)` для metric area - Frontend block `NspdRiskZonesBlock` — severity color-coded badges (🔴 затопление/оползни, 🟡 подтопление/гари, 🟠 эрозии) - Helper `_extract_features_by_layer` — generic для re-use в PR2 (opportunity + red lines) - 11 unit tests (extract / intersect / null-area / db-exception / label fallback) ## Roadmap (#94 decomposition) - ✅ **PR1**: TIER 3 risk zones (этот PR) - ⏳ **PR2**: TIER 4 opportunity + red lines (~400 LOC) - ⏳ **PR3**: Denormalized entity tables `nspd_parcels` + `nspd_buildings` (~380 LOC) - ⏳ **PR4**: TIER 5 ARN квартирография heatmap (~450 LOC) ## Files (9, +545 LOC) - `backend/app/workers/tasks/nspd_sync.py` (+5) — fanout flag - `backend/app/services/site_finder/quarter_dump_lookup.py` (+156) — `_RISK_SUBTYPE_LABELS` + `_extract_features_by_layer` + `_get_risk_zones` - `backend/app/schemas/parcel.py` (+12) — `RiskZone` - `backend/app/api/v1/parcels.py` (+5) — analyze response field - `backend/tests/services/test_quarter_dump_lookup.py` (+202) — 11 tests - `frontend/src/components/site-finder/NspdRiskZonesBlock.tsx` (+147) - `frontend/src/components/site-finder/LandTab.tsx` (+14) - `frontend/src/types/nspd.ts` (+9) - `frontend/src/types/site-finder.ts` (+1) ## Test plan - [ ] Deploy + curl `/api/v1/parcels/analyze` на cad_num с известной risk zone (например quarter под подтоплением) → response содержит `nspd_risk_zones[]` - [ ] UI: LandTab → NspdRiskZonesBlock виден с badges если intersect, иначе зелёный "Риски не обнаружены" - [ ] Celery: `harvest_stale_quarters.delay()` → проверить логи что `include_risks=True` передан - [ ] Pytest: `cd backend && uv run pytest tests/services/test_quarter_dump_lookup.py -v` → 11/11 pass ## Pre-push checks - ruff + ruff-format: pass - code-reviewer subagent: APPROVE (схема columns `risks_count`/`features_json` verified против live DB, SRID consistency через `ST_SetSRID` + `ST_Transform`, no f-string SQL, no XSS) - phantom column gate (PR #216): SQL queries в PR1 используют real columns — clean ## Known follow-ups - Severity mapping (`NspdRiskZonesBlock.getSeverity`) explicitly covers 4 из 11 layers; остальные default → "low". Расширить в PR2 если нужно тонкое разделение. - `_get_risk_zones` использует bare `except Exception` (consistent с sibling helpers); narrow to `(OperationalError, ProgrammingError)` — отдельный refactor PR. Part of #94
lekss361 added 1 commit 2026-05-16 13:12:37 +00:00
- harvest_stale_quarters fanout passes include_risks=True to all queued tasks
- _get_risk_zones() with ST_Intersects + ST_Area intersection calc
- _extract_features_by_layer() generic helper for future PR2+ reuse
- RiskZone Pydantic schema; /analyze returns nspd_risk_zones list
- NspdRiskZonesBlock component with severity color-coding (red/yellow/orange)
- 11 unit tests for extract/intersect/null-area/db-exception paths

Part of #94 (TIER 3 risk layers — flooding, landslide, burns, erosion etc)
Author
Owner

Code Review — verdict

Summary

  • Status: APPROVE (with low-severity follow-ups)
  • Head SHA: 1595157 · Mergeable: true · Auto-merge scope: BLOCKED (api/v1, services, workers)
  • Files reviewed: 9 · Lines: +545 / -3

Critical

None.

High

None.

Medium

None.

Low

  • quarter_dump_lookup._get_risk_zones uses bare except Exception — acknowledged in PR body as consistent with sibling helpers (_get_zoning, _get_zouit_overlaps, _get_engineering_nearby). Acceptable for now; tracked as follow-up to narrow to (OperationalError, ProgrammingError).
  • NspdRiskZonesBlock.getSeverity explicitly maps only 4 of 11 risk layers; the other 7 fall through to "low". Per PR body this is intentional — расширить в PR2. UI still renders subtype label correctly via _RISK_SUBTYPE_LABELS fallback in backend.
  • NspdRiskZonesBlock uses key={idx} for the sorted list. Stable enough for a static analyze response, but layer+geom_wkt hash would be more correct if list ever updates client-side.
  • intersection_area_sqm rounded to 1 decimal (round(val, 1)); the UI then re-rounds via Math.round / toFixed(2) for hectares. No data loss but two rounding stages is slightly redundant.

Schema verification

Verified against live DB (public.nspd_quarter_dumps):

  • risks_count (integer NOT NULL default 0) — exists
  • features_json (jsonb NOT NULL default '[]'::jsonb) — exists
  • SQL in _get_risk_zones references only real columns (quarter_cad, features_json) + jsonb_array_elements — no phantom columns.
  • SRID handling ST_SetSRID(ST_GeomFromGeoJSON(...), 3857) → ST_Transform(..., 4326) matches the convention used by all sibling helpers (zouit, engineering) in the same file — consistent .
  • Layer key contract verified: NSPDClient.QUARTER_RISK_LAYERS stores features with layer = "risk_<short_name>"; PR1's _RISK_SUBTYPE_LABELS keys are short_names and looked up via layer.removeprefix("risk_") → mapping aligns .
  • Pydantic RiskZone fields (layer, subtype, geom_wkt, intersection_area_sqm) exactly match the dict shape returned by _get_risk_zones .

Security audit

  • All SQL uses parameter binding (:q, :wkt) — no f-string / format / % interpolation.
  • No hardcoded secrets / tokens / keys introduced.
  • Frontend: zone.subtype and zone.layer are plain strings rendered inside React text nodes (no dangerouslySetInnerHTML, no URL construction) — auto-escaped by React. No XSS surface.
  • No safeUrl() needed (no URL fields in this PR).

Correctness

  • ST_Intersection(...::geography) returns metric m² → correct for area .
  • _extract_features_by_layer guards non-string layer via isinstance(...,str); empty list returns [] — edge cases covered.
  • _get_risk_zones early-exits when risks_count == 0 (skips heavy jsonb_array_elements scan) — good perf guard.
  • DB exception → empty list + warning log (no propagate) — consistent with siblings, won't break /analyze .
  • Tests (backend/tests/services/test_quarter_dump_lookup.py): 11 tests covering extract / intersect / no-intersect / no-risks-in-dump / db-exception / null-area / non-string layer / label fallback (landslide → "Обвально-осыпные процессы") / make_empty_result includes new key / derive_quarter_cad smoke (parametrized 5 cases). Coverage matches PR claim .
  • LandTab.tsx: block conditionally rendered when data.nspd_risk_zones !== undefined; component itself shows green "Риски не обнаружены" fallback when array empty .
  • harvest_quarter already accepts include_risks: bool = False kwarg (line 241 of nspd_sync.py) — fanout change in harvest_stale_quarters is signature-compatible. Other call-sites (e.g. harvest_quarter.delay(cad) without kwarg) still work, just default to False — no breakage .

Conventions

  • Python ruff line ≤100 , no psycopg2 imports, no requests, uses text(...) + :param binding (no CAST trap encountered since no type casts on params).
  • TS strict: RiskZone interface explicit, no any, no useEffect for fetch (data comes via parent prop drilling).
  • Inline styles used (consistent with sibling Nspd*Block components in the project — not Tailwind, but follows existing pattern).

Cross-file impact

  • Parallel PRs (#205 infra, #207 backend-sentry, #208 frontend-sentry) touch entirely different files. No conflict with parcels.py, LandTab.tsx, quarter_dump_lookup.py, or site-finder.ts.
  • parcels.py analyze response gains one new field nspd_risk_zones: list[RiskZone] (always present, defaults to [] via nspd_dump_data.get(...)). Frontend type marks it ?: optional. Non-breaking for existing consumers — codegen will add an optional array property.
  • harvest_stale_quarters fanout change is backward-compatible (see Correctness section).
  • make_empty_result returns new key nspd_risk_zones: [] — anything destructuring this dict (e.g. analyze handler nspd_dump_data["nspd_risk_zones"]) will Just Work; older callers ignoring it are unaffected.

Auto-merge policy

BLOCKED for auto-merge per .claude/rules/git-pr.md scope policy: PR touches backend/app/api/v1/parcels.py, backend/app/services/site_finder/*, and backend/app/workers/tasks/* — all three are in the blocked-list (api/v1, services, workers). Human merge required despite APPROVE verdict.

<!-- gendesign-review-bot: sha=1595157 verdict=approve --> ## Code Review — verdict ### Summary - Status: APPROVE (with low-severity follow-ups) - Head SHA: `1595157` · Mergeable: true · Auto-merge scope: BLOCKED (api/v1, services, workers) - Files reviewed: 9 · Lines: +545 / -3 ### Critical _None._ ### High _None._ ### Medium _None._ ### Low - `quarter_dump_lookup._get_risk_zones` uses bare `except Exception` — acknowledged in PR body as consistent with sibling helpers (`_get_zoning`, `_get_zouit_overlaps`, `_get_engineering_nearby`). Acceptable for now; tracked as follow-up to narrow to `(OperationalError, ProgrammingError)`. - `NspdRiskZonesBlock.getSeverity` explicitly maps only 4 of 11 risk layers; the other 7 fall through to `"low"`. Per PR body this is intentional — расширить в PR2. UI still renders subtype label correctly via `_RISK_SUBTYPE_LABELS` fallback in backend. - `NspdRiskZonesBlock` uses `key={idx}` for the sorted list. Stable enough for a static analyze response, but layer+geom_wkt hash would be more correct if list ever updates client-side. - `intersection_area_sqm` rounded to 1 decimal (`round(val, 1)`); the UI then re-rounds via `Math.round` / `toFixed(2)` for hectares. No data loss but two rounding stages is slightly redundant. ### Schema verification Verified against live DB (`public.nspd_quarter_dumps`): - `risks_count` (integer NOT NULL default 0) — exists ✅ - `features_json` (jsonb NOT NULL default `'[]'::jsonb`) — exists ✅ - SQL in `_get_risk_zones` references only real columns (`quarter_cad`, `features_json`) + `jsonb_array_elements` — no phantom columns. - SRID handling `ST_SetSRID(ST_GeomFromGeoJSON(...), 3857) → ST_Transform(..., 4326)` matches the convention used by all sibling helpers (zouit, engineering) in the same file — consistent ✅. - Layer key contract verified: `NSPDClient.QUARTER_RISK_LAYERS` stores features with `layer = "risk_<short_name>"`; PR1's `_RISK_SUBTYPE_LABELS` keys are short_names and looked up via `layer.removeprefix("risk_")` → mapping aligns ✅. - Pydantic `RiskZone` fields (`layer`, `subtype`, `geom_wkt`, `intersection_area_sqm`) exactly match the dict shape returned by `_get_risk_zones` ✅. ### Security audit - All SQL uses parameter binding (`:q`, `:wkt`) — no f-string / format / % interpolation. ✅ - No hardcoded secrets / tokens / keys introduced. - Frontend: `zone.subtype` and `zone.layer` are plain strings rendered inside React text nodes (no `dangerouslySetInnerHTML`, no URL construction) — auto-escaped by React. No XSS surface. ✅ - No `safeUrl()` needed (no URL fields in this PR). ### Correctness - `ST_Intersection(...::geography)` returns metric m² → correct for area ✅. - `_extract_features_by_layer` guards non-string `layer` via `isinstance(...,str)`; empty list returns `[]` — edge cases covered. - `_get_risk_zones` early-exits when `risks_count == 0` (skips heavy `jsonb_array_elements` scan) — good perf guard. - DB exception → empty list + warning log (no propagate) — consistent with siblings, won't break `/analyze` ✅. - Tests (`backend/tests/services/test_quarter_dump_lookup.py`): 11 tests covering extract / intersect / no-intersect / no-risks-in-dump / db-exception / null-area / non-string layer / label fallback (`landslide` → "Обвально-осыпные процессы") / `make_empty_result` includes new key / `derive_quarter_cad` smoke (parametrized 5 cases). Coverage matches PR claim ✅. - `LandTab.tsx`: block conditionally rendered when `data.nspd_risk_zones !== undefined`; component itself shows green "Риски не обнаружены" fallback when array empty ✅. - `harvest_quarter` already accepts `include_risks: bool = False` kwarg (line 241 of `nspd_sync.py`) — fanout change in `harvest_stale_quarters` is signature-compatible. Other call-sites (e.g. `harvest_quarter.delay(cad)` without kwarg) still work, just default to `False` — no breakage ✅. ### Conventions - Python ruff line ≤100 ✅, no `psycopg2` imports, no `requests`, uses `text(...) + :param` binding (no CAST trap encountered since no type casts on params). - TS strict: `RiskZone` interface explicit, no `any`, no `useEffect` for fetch (data comes via parent prop drilling). ✅ - Inline styles used (consistent with sibling `Nspd*Block` components in the project — not Tailwind, but follows existing pattern). ### Cross-file impact - Parallel PRs (#205 infra, #207 backend-sentry, #208 frontend-sentry) touch entirely different files. **No conflict** with `parcels.py`, `LandTab.tsx`, `quarter_dump_lookup.py`, or `site-finder.ts`. - `parcels.py` analyze response gains one new field `nspd_risk_zones: list[RiskZone]` (always present, defaults to `[]` via `nspd_dump_data.get(...)`). Frontend type marks it `?:` optional. **Non-breaking** for existing consumers — codegen will add an optional array property. - `harvest_stale_quarters` fanout change is backward-compatible (see Correctness section). - `make_empty_result` returns new key `nspd_risk_zones: []` — anything destructuring this dict (e.g. analyze handler `nspd_dump_data["nspd_risk_zones"]`) will Just Work; older callers ignoring it are unaffected. ### Auto-merge policy **BLOCKED for auto-merge** per `.claude/rules/git-pr.md` scope policy: PR touches `backend/app/api/v1/parcels.py`, `backend/app/services/site_finder/*`, and `backend/app/workers/tasks/*` — all three are in the blocked-list (api/v1, services, workers). **Human merge required** despite APPROVE verdict.
lekss361 merged commit e0055d299c into main 2026-05-16 15:03:19 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lekss361/gendesign#217
No description provided.