feat(nspd): denorm nspd_parcels + nspd_buildings (#94 PR3 of 4) #222
No reviewers
Labels
No labels
admin
analytics
auth
automation
bug
business
chore
ci
compliance
data
data-moat
docs
duplicate
dx
enhancement
Fable 5 ревью
feedback/max
generative
GG-форсайт
needs-discussion
needs-human
observability
pause-bots
performance
priority/p0
priority/p1
priority/p2
priority/p3
scope/backend
scope/db
scope/devops
scope/frontend
scope/qa
scrapers
security
site-finder
stage/1
stage/2
status/blocked
status/done
status/needs-analysis
status/needs-fix
status/qa
status/ready
status/review
status/wip
tech-debt
tradein
ux
week ревью 1
wontfix
вторичка
ИРД
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: lekss361/gendesign#222
Loading…
Add table
Reference in a new issue
No description provided.
Delete branch "feat/94-pr3-denorm-entities"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Sub-PR 3 of 4 для #94. Денормализует NSPD features из
nspd_quarter_dumps.features_jsonJSONB в отдельные entity таблицы для быстрого lookup.nspd_parcels(PKcad_num) — permitted_use, land_category, cost_value, cost_per_m2, area_sqm, geom (4326)nspd_buildings(PKcad_num) — purpose, floors, floors_underground, year_built, cost_value, build_record_area, geom (4326). Partial indexWHERE purpose ILIKE '%многокв%'для МКД-фильтра.nspd_denorm.py: SAVEPOINT per row, ST_Transform 3857→4326, json.dumps geom bindharvest_quarter(non-fatal — denorm failure не блокирует harvest)POST /api/v1/admin/etl/nspd-denorm-backfill?limit=NRoadmap (#94)
Files (6, +932 LOC)
data/sql/99_nspd_entities_denorm.sql(NEW, +171) — 2 tables + 6 indexes (idempotent)backend/app/services/scrapers/nspd_denorm.py(NEW, +290)backend/app/workers/tasks/nspd_denorm_backfill.py(NEW, +98)backend/app/workers/tasks/nspd_sync.py(+27 EDIT)backend/app/api/v1/admin_etl.py(+26 EDIT)backend/tests/services/test_nspd_denorm.py(NEW, +322, 19 tests)Test plan
SELECT * FROM information_schema.tables WHERE table_name IN ('nspd_parcels','nspd_buildings');→ 2 rowspermitted_use/cost_valueсильно NULL — property names в features_json могут бытьcn_30/cn_36(NSPD PKK REST) вместоpermitted_use/cost_value(WMS). Тогда нужно adjust mapping.cd backend && uv run pytest tests/services/test_nspd_denorm.py -v→ 19/19 passPre-push checks
CAST(:x AS type), нет:x::typetext()ST_Transform(ST_SetSRID(..., 3857), 4326)правильноwith db.begin_nested()per rowjson.dumps(geom, ensure_ascii=False)(не Pythonstr(dict))AdminTokenAuthdep99_*после98_*— sequential orderKnown follow-ups
test_phantom_columns.pyпосле deploy.cn_*keysPart of #94
Deep Code Review — verdict
Summary
db1ff5b🔴 Critical (BLOCK)
None.
🟠 High
None.
🟡 Medium (worth addressing — non-blocking)
backend/app/services/scrapers/nspd_denorm.py:262-280—denorm_dumpconflates expected skips (nocad_num) with real DB errors into singleerrors_ncounter. Docstring acknowledges это ("пропусков (нет cad_num, DB ошибки)"), но в проде telemetry будет показывать высокий errors_n даже на healthy NSPD data (часть feature'ов в NSPD приходит без cad_num — например, общие fragments quartal'а). Suggested: spliterrors_n→skipped_n(no cad_num) +errors_n(DB failure). Сразу видно если real ETL начнёт ломаться.backend/app/workers/tasks/nspd_denorm_backfill.py:26—soft_time_limit=3600(1h) недостаточно для full backfill at scale. На Свердловской обл. ожидается ~50k+ кварталов × ~100 features = ~5M UPSERTов × ~5-10ms (SAVEPOINT overhead) ≈ 7-14 часов. Сейчас прод-data пустая (0 dumps) — реального impact нет, но при первом full harvest backfill будет фейлиться по soft_time_limit. Suggest либо: (a) поднять до 8h при known full-region run, либо (b) batch-resume — track lastquarter_cad, продолжать с него при retry. Можно отдельным PR.backend/app/api/v1/admin_etl.py:80-82—limit: int | None = Noneбез upper bound. СлучайныйPOST /api/v1/admin/etl/nspd-denorm-backfillбез?limit=запустит all-quarters scan. ЗащищеноAdminTokenAuth, но defense-in-depth: добавить max cap (например,Query(..., le=100_000)).🟢 Low / nits
data/sql/99_nspd_entities_denorm.sql:160— partial indexWHERE purpose = 'Многоквартирный дом'exact-match. Если НСПД когда-нибудь нормализует строку (whitespace, casing, синонимы "МКД" / "Многоквартирные дома") — индекс станет неэффективен. Сейчас OK, но добавить test/runbook check на values DISTINCT purpose после первого harvest'а.backend/app/services/scrapers/nspd_denorm.py:140-145, 220-226—except Exception as e: logger.warning(...)— generic catch безlogger.exception. Stack trace потерян, debug по NSPD-edge-case будет сложнее. Suggestlogger.warning("...", exc_info=True)илиlogger.exception(...).Cross-file impact analysis
nspd_sync.py:305-329— inline denorm правильно не-fatal: outertry/except Exceptionловит любую ошибку (включаяSessionLocal()failure), innertry/finallyгарантируетdenorm_db.close(). Harvest продолжается даже если denorm полностью сломан. Good._schema_migrationslast =98_*(verified via MCP), file =99_*— correct sequential slot, auto-apply черезdeploy.ymlподнимет таблицы перед deploy backend.features_jsoncolumn вnspd_quarter_dumpsсуществует (verified via MCP:data_type=jsonb,default='[]'::jsonb) — denorm read-path не сломается.cad_num,permitted_use,cost_value,area,floors_above_ground,year_built,build_record_area) совпадает с WMS GetFeatureInfo raw response (NSPDFeature.from_raw → pass-through черезprops = raw.get("properties") or {}). PKK RESTcn_30/cn_36naming здесь не применим — это WMS pipeline.nspd_quarter_dumpsсейчас пустая (SELECT COUNT(*) = 0в prod via MCP), поэтому property-key mapping unverifiable end-to-end до первого harvest run. Post-merge: первыйharvest_quarter→ проверить чтоnspd_parcels.permitted_use IS NOT NULLдля свежей строки.Conventions / standards
CAST(:x AS type), никаких:x::type(CAST trap избежан)text()+ dictwith db.begin_nested():per-row (matches.claude/rules/backend.md)json.dumps(geom, ensure_ascii=False)для geom вместоstr(dict)(per CLAUDE.md rule)CREATE TABLE IF NOT EXISTS,CREATE INDEX IF NOT EXISTS,ON CONFLICT DO UPDATEBEGIN; ... COMMIT;wrap (per.claude/rules/sql.md)logger.*everywhere, noprint()dict | None)Tests coverage
ST_Transform/ST_SetSRID/ST_GeomFromGeoJSONповедение не верифицировано в unit (мок не выполняет реальный SQL). Acceptable для unit scope; рекомендовано добавить один integration test с docker-compose postgres в future PR (отдельным chore/, не блок).Positive observations
harvest_quarterправильно non-fatal — bug в denorm не положит harvest pipelineON CONFLICT DO UPDATEдля re-harvest idempotencyGEOMETRY(Geometry, 4326)(не Polygon) — корректно для NSPD который может вернуть MultiPolygon/PointCOMMENT ON TABLE/COLUMN/INDEX— самодокументирующаяся схема_PARCEL_UPSERT_SQL/_BUILDING_UPSERT_SQLкак module-leveltext()— переиспользуется compiled statement (perf win)Complexity / blast radius
DROP TABLE nspd_parcels, nspd_buildings CASCADE+ удалить запись из_schema_migrations. Никаких изменений existing schema.Recommended next steps
harvest_quarterна реальном квартале → verifySELECT * FROM nspd_parcels WHERE permitted_use IS NOT NULL LIMIT 5;(property-mapping smoke check)?limit=10→ измерить avg time per quarter → решить про soft_time_limit❌ Blocked for auto-merge per
.claude/rules/git-pr.mdscope policy (touchesdata/sql/**,backend/app/api/v1/**,backend/app/services/**,backend/app/workers/**). Human merge required.Verdict: code-quality APPROVE — implementation clean, conventions соблюдены, мелкие nit'ы не блокирующие. Все три medium-замечания — improvements, не bugs. PR3/4 готов к человеческому merge.