feat(nspd): TIER 4 opportunity + red lines (#94 PR2 of 4) #220
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#220
Loading…
Add table
Reference in a new issue
No description provided.
Delete branch "feat/94-pr2-opportunity-redlines"
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 2 of 4 для #94 (NSPD extended layers).
/analyzeчерез existing core harvest path (dump.red_lines); НЕТ duplicate harvest / NEТ separate column (fix после code-reviewer feedback)nspd_quarter_dumps:has_auction_parcels,opportunity_count+ partial index для быстрого filter "найти кварталы с аукционом"NspdOpportunityBlock(color-coded by type) +NspdRedLinesBlock(ALERT при intersect)Roadmap (#94)
nspd_parcels/nspd_buildings(~380 LOC)Files (12, +966/-9)
data/sql/89_nspd_quarter_dumps_opportunity_flag.sql(NEW, +43) — 2 cols + 1 partial indexbackend/app/services/scrapers/nspd_client.py(+33) — QUARTER_OPPORTUNITY_LAYERS + opportunity fieldbackend/app/workers/tasks/nspd_sync.py(+39) —_build_*helpers + extended UPSERTbackend/app/services/site_finder/quarter_dump_lookup.py(+265) —_get_opportunity_parcels+_get_red_linesbackend/app/schemas/parcel.py(+39) — OpportunityParcel + RedLinebackend/app/api/v1/parcels.py(+11) — analyze fieldsbackend/tests/services/test_quarter_dump_lookup.py(+200) — 17 new tests (28 total)NspdOpportunityBlock.tsx(+162),NspdRedLinesBlock.tsx(+133),LandTab.tsx(+30),types/nspd.ts(+17),types/site-finder.ts(+3)Test plan
/analyzeна cad_num в квартале с auction-парцелом → response.nspd_opportunity_parcels not emptycd backend && uv run pytest tests/services/test_quarter_dump_lookup.py -v→ 28/28 passPre-push checks
red_lines_count_v2+QUARTER_RED_LINES_LAYERSgroup +include_red_linesparam +_build_red_lines_count_v2ВСЁ удалено; layer 879243 → существующийdump.red_linescore path)has_auction_parcels,opportunity_countсуществуют ТОЛЬКО после миграции 89_*.sql apply. Migration runs ПЕРЕД containers up → safe в проде.Known follow-ups
has_auction_parcelspartial indextest_phantom_columns.pyfollow-up issueNspdRiskZonesBlock(4/11 layers) — расширить если нужноPart of #94
🟠 HIGH — изменения нужны перед merge
Deep review PR #220 (sha
ed3c128, +966/−9, 12 files). Сборка корректная, контракт API/типы/тесты согласованы, но есть два содержательных issue + один cosmetic. Auto-merge запрещён (см. блок в конце).🔴 BLOCK 1 — Filename collision на
data/sql/89_*(sql.md violation)В репозитории уже есть
data/sql/89_drop_dead_brin_rosreestr_deals.sql(applied в prod_schema_migrations). Новый файл назван тоже89_*—89_nspd_quarter_dumps_opportunity_flag.sql. Последняя применённая миграция в prod —97_objective_backfill_indexes.sql.Что сейчас произойдёт на deploy (per
.forgejo/workflows/deploy.ymlline 208 —ls -1 data/sql/*.sql | sort):89_drop_dead...sqlуже applied → skip89_nspd_quarter_dumps_opportunity_flag.sql→ НЕ в_schema_migrations→ apply. Применится после уже-применённого 97_ — нарушает «NN order».Технически работает (DDL аддитивный), но нарушает convention из
.claude/rules/sql.md: «NN_topic.sqlгде NN — следующий sequential номер. Проверка:ls data/sql/ | sort | tail -5».Fix:
git mv data/sql/89_nspd_quarter_dumps_opportunity_flag.sql data/sql/98_nspd_quarter_dumps_opportunity_flag.sql+ update комментариев header.🟠 HIGH 2 —
_get_red_lines:ST_Area(ST_Intersection(LINESTRING::geography, POLYGON::geography))неверная семантика + runtime errorLayer 879243 (
red_lines) — этоLINESTRING/MULTILINESTRING. В SQL_get_red_linesрассчитываетсяST_Area(ST_Intersection(geom::geography, parcel::geography)). Проблемы:ST_Intersection(line, polygon)→ line/point.ST_Area(line)= 0. Полеintersection_area_sqmдля пересекающейся красной линии будет постоянно ~0 (или NaN) — мисс metric. Правильная метрика —ST_Length(...)в метрах (длина пересечения).ST_Intersection(::geography, ::geography)для line × polygon у PostGIS 3.4 регулярно даётtransform: tolerance condition error (-20)— я воспроизвёл локально на live DB (см. ниже). Try/except в Python проглотит исключение и вернёт[]для всего квартала, даже если интерсект есть. Tests мокают rows и runtime error не ловят.Доказательство:
Fix: заменить
intersection_area_sqm→intersection_length_m, использоватьST_Length(ST_Intersection(geom_4326, parcel_4326)::geography)(cast результата, не операндов). Обновить:backend/app/schemas/parcel.py :: RedLine(поле + docstring)backend/app/services/site_finder/quarter_dump_lookup.py :: _get_red_lines(SQL + key)frontend/src/types/nspd.ts :: RedLinefrontend/src/components/site-finder/NspdRedLinesBlock.tsx :: formatArea → formatLengthbackend/tests/services/test_quarter_dump_lookup.py— тесты переписать на новое поле + добавить тест на DB exception (как в opportunity).🟡 MEDIUM —
_OPPORTUNITY_TYPE_LABELSбесполезенquarter_dump_lookup.py:571-577— словарь маппит ключи в самих себя:Метод
.get(short_name, short_name)всегда возвращаетshort_name. Либо удалить dict + вернутьraw_layer.removeprefix("opportunity_")напрямую, либо положить туда настоящие human-labels (если планировались — UI labels уже в frontendLAYER_CONFIG).✅ Что хорошо
ADD COLUMN IF NOT EXISTS+BEGIN/COMMIT+ partial indexIF NOT EXISTS) — правильно.WHERE has_auction_parcels = TRUE— корректный паттерн, не путает с UI-фильтром.CAST(:wkt AS ...)стиль (не:wkt::type) — соблюдён psycopg v3 trap.harvest_quarterиharvest_stale_quartersпробрасываютinclude_opportunity=Trueconsistently — нет regression в TIER 3 fanout._upsert_dumppartial-fail branch (dump=None) корректно ставитhas_auction_parcels=False, opportunity_count=0.LAYERSdict вnspd_client.py:71-105содержит все ID.Optional[list], default[]).buildNspdViewerUrlиспользуетencodeURIComponent— XSS-safe. Текстовый рендер cad_num внутри<a>тоже safe (React escaping).Резюме
Verdict: 🟠 HIGH (changes requested). Fix необходим перед merge:
89_→98_.intersection_area_sqm→intersection_length_m+ST_Length(схема + SQL + frontend + tests)._OPPORTUNITY_TYPE_LABELSили наполнить реальными labels.❌ Blocked for auto-merge per
.claude/rules/git-pr.mdscope policy (touchesdata/sql/**,backend/app/api/v1/**,backend/app/services/**,backend/app/services/scrapers/**). Human merge required.🟢 APPROVE — fixup verified (re-review on
fdb5483)Все 3 issue из предыдущего review (
ed3c128, comment #566) адресованы. Контракт, тесты и live DB sanity сходятся.✅ Fix verification
🔴 #1 — Migration rename — DONE
data/sql/89_nspd_quarter_dumps_opportunity_flag.sql→98_nspd_quarter_dumps_opportunity_flag.sql_schema_migrationsподтверждает: applied89..97, новый98_*встаёт sequential без conflicts (verified viamcp__postgres-gendesign).ADD COLUMN IF NOT EXISTS+CREATE INDEX IF NOT EXISTS— idempotent ✅.🟠 #2 — Red lines SQL semantic + runtime bug — DONE
intersection_area_sqm→intersection_length_m(parcel.py:117, nspd.ts:63)._get_red_lines): Cast в::geographyпосле ST_Intersection — обходит PostGIS 3.4 tolerance bug. Подтверждено на live DB: Старый broken patternST_Area(ST_Intersection(::geography, ::geography))возвращал0.0(wrong metric on lines).ST_Intersectspre-check + раздельная семантикаintersecting(distance=null) vsnearby(length=null) — UI ALERT logic корректный.NspdRedLinesBlock:formatLength(км/м), ALERT banner для intersect — XSS-safe (encodeURIComponent + React text escaping).test_get_red_lines_intersects/_nearby_only/_empty/_early_exit/_db_exception— все на новое поле.🟡 #3 —
_OPPORTUNITY_TYPE_LABELSdead code — NOT FIXED (downgrading to nit){"auction_parcels": "auction_parcels", ...}→.get(short_name, short_name)всегда возвращаетshort_name. 5 LOC dead code (quarter_dump_lookup.py:571-577).LAYER_CONFIG. Можно убрать в PR3 cleanup или оставить как extension point если планируется backend-side label override.✅ Non-regression checks
WHERE has_auction_parcels = TRUE— корректный low-cardinality pattern:parambindings; нет:x::type(psycopg v3 trap OK);CAST(:features_json AS jsonb)правильноimport psycopg2/print()/import requestsOptional[list],.get(..., [])defaults)_upsert_dumppartial-fail branch (dump=None) ставитhas_auction_parcels=False, opportunity_count=0— нет partial nullable stateharvest_quarter+harvest_stale_quartersconsistently пробрасываютinclude_opportunity=TrueencodeURIComponentдля NSPD viewer URL, React text rendering — XSS-safeРезюме
Verdict: 🟢 APPROVE. All 🔴 / 🟠 issues fixed. 🟡 dead-code dict — оставлен, не блокирует.
❌ Blocked for auto-merge per
.claude/rules/git-pr.mdscope policy (touchesdata/sql/**,backend/app/api/v1/**,backend/app/services/**,backend/app/services/scrapers/**). Human merge required.