lekss361/gendesign

Fork 0

feat(site-finder): integrate nspd_quarter_dumps cache в analyze_parcel (#94 Sprint 1.1 FINAL) #116

Merged

lekss361 merged 3 commits from feat/analyze-uses-quarter-dump into main

2026-05-13 06:14:19 +00:00

lekss361 commented

2026-05-12 16:00:56 +00:00

(Migrated from github.com)

Summary

Sprint 1.1 item #4 FINAL из #94 part 2 plan. Замыкает foundation: analyze_parcel теперь читает quarter dump cache и возвращает 4 новых поля — G1 ПЗЗ, G3 ЗОУИТ overlaps, I3 engineering proximity, freshness metadata.

Sprint 1.1 progress

#	PR	Status
1	#109 (search_by_quarter + QuarterDump)	✅ merged
2	#110 (schema nspd_quarter_dumps)	✅ merged
3	#111 (harvest_quarter Celery + beat)	✅ merged
4	этот PR (analyze_parcel integration)	🔄

После merge — Sprint 1.1 закрыт, foundation готов. Frontend rendering этих 4 новых полей в UI — отдельный PR (next: #112 / #115 / #114).

Backend

`backend/app/services/site_finder/quarter_dump_lookup.py` (new)

Public	Purpose
`derive_quarter_cad(cad_num)`	3/4/5-сегмент → quarter (3-segment). None если invalid.
`get_quarter_dump_data(db, cad_num, parcel_wkt)`	Main entrypoint — reads dump, returns 4-keys dict
`EMPTY_DUMP_RESULT`	Module const для DRY no-dump fallback

Flow:

Lookup row в nspd_quarter_dumps для derived quarter
Freshness check: fetched_at_utc > NOW() - INTERVAL '180 days' AND harvest_error IS NULL
Missing/stale/error → harvest_quarter.apply_async([quarter]) fire-and-forget (lazy import против circular), return EMPTY_DUMP_RESULT
Fresh + parcel_wkt=None → metadata only (1 DB call)
Fresh + geometry → 3 spatial queries:
- _get_zoning: point-in-polygon parcel centroid vs territorial_zones, LIMIT 1
- _get_zouit_overlaps: все zouit_% layers пересекающиеся с parcel
- _get_engineering_nearby: engineering_structures в 200m, sorted by distance

Все используют ST_Transform(ST_SetSRID(ST_GeomFromGeoJSON(feature->'geometry'), 3857), 4326) для inline 3857 raw NSPD в features_json → 4326 для intersection.

`backend/app/api/v1/parcels.py` (modified)

Import EMPTY_DUMP_RESULT, get_quarter_dump_data
Call wrapped в try/except Exception с logger.warning + EMPTY_DUMP_RESULT fallback. Защита от DB unavailability (table missing после deploy / timeout) — consistent с resilience pattern других optional fetches в endpoint
Response gains 4 new fields:

{
  ...,
  "nspd_zoning": {"zone_code": "Ж-1", "zone_name": "...", "source": "nspd-quarter-dump"} | null,
  "nspd_zouit_overlaps": [{"group_key": "zouit_engineering", "subcategory": "...", "name": "..."}, ...],
  "nspd_engineering_nearby": [{"name": "...", "distance_m": 45, "type": "..."}, ...],
  "nspd_dump": {
    "available": true,
    "fetched_at_utc": "2026-05-12T12:00:00+00:00",
    "stale": false,
    "harvest_triggered": false,
    "total_features": 117
  }
}

Tests: `backend/tests/test_quarter_dump_lookup.py` (13 new)

Mock-based, no real DB. Coverage:

derive_quarter_cad: 5 edge cases (3seg/4seg/5seg/invalid/whitespace)
get_quarter_dump: no_row, stale, harvest_error, no_geometry, fresh_zoning, fresh_zouit, no_match, includes_all_4_fields

13/13 pass + 47 pre-existing still pass.

Code review (pre-push, code-reviewer agent)

Verdict: MINOR, 0 blocking. Applied 2 of 4 рекомендаций:

✅ #1: try/except wrap around get_quarter_dump_data в analyze_parcel + DRY EMPTY_DUMP_RESULT module constant. Без этого если nspd_quarter_dumps недоступна → 500 instead of degraded response.
✅ #2: removed redundant nspd_zoning.fetched_at_utc (DRY — freshness живёт в nspd_dump.fetched_at_utc, не дублируется в zoning sub-object).
⏭ #3 deferred: ad-hoc harvest_quarter retry cooldown для harvest_error rows (только при high traffic + persistent NSPD errors). Acceptable defer — Celery apply_async идемпотентен, лишний WAF traffic не страшен на текущем scale.
⏭ #4 deferred: raw_props в response — tech debt, убрать вместе с frontend PR.

Performance note

3 spatial queries per analyze adds ~10-50ms на typical ~100-feature quarter. analyze_parcel cumulative budget OK. Mitigation если quarters grow dense (>500 features): materialized per-layer sub-table — отдельная database-expert issue.

Acceptance (Sprint 1.1 #4 from #94 plan)

analyze_parcel reads nspd_quarter_dumps через helper
G1 ПЗЗ zone extracted (nspd_zoning)
G3 ЗОУИТ overlaps extracted (nspd_zouit_overlaps)
I3 engineering proximity extracted (nspd_engineering_nearby)
Freshness metadata (nspd_dump)
Missing/stale → harvest trigger fire-and-forget
Resilience: DB error → degraded response (no 500)
13 new tests + 47 pre-existing pass
Code-reviewer APPROVE (MINOR)

Test plan

ruff check + format — clean
pytest tests/test_quarter_dump_lookup.py -v — 13/13 passed
All 47 pre-existing tests still pass

After deploy — smoke:

# Trigger harvest manually first для test quarter
curl -X POST .../admin/scrape/nspd/harvest-quarter -H "X-Admin-Token: $TOKEN" \
  -d '{"quarter_cad":"66:41:0204016"}'
# 30s later — analyze
curl -X POST .../parcels/66:41:0204016:10/analyze | jq '.nspd_dump, .nspd_zoning, .nspd_zouit_overlaps | length'
# Expected: nspd_dump.available=true, nspd_zoning не null если parcel в territorial_zone, nspd_zouit_overlaps возможно [].

What's next

После merge #94 part 2 Sprint 1.1 closed. Очередь по user-выбранному порядку:

#112 Активные конкуренты (UI slider + add/exclude + per-комнатность)
#115 Phase 1 Connection Points (visual layer на SiteMap)
#114 Custom weights (user POI weights override)
Sprint 1.2 ekburg.rf RNS (#105)
Sprint 1.3 G5 Gate Verdict UI (#32)

Sequential rule — следующий PR создаётся ТОЛЬКО после merge этого.

Part of #94. Closes Sprint 1.1 of part 2 plan.

## Summary **Sprint 1.1 item #4 FINAL** из #94 part 2 plan. Замыкает foundation: `analyze_parcel` теперь читает quarter dump cache и возвращает 4 новых поля — G1 ПЗЗ, G3 ЗОУИТ overlaps, I3 engineering proximity, freshness metadata. ### Sprint 1.1 progress | # | PR | Status | |---|---|---| | 1 | #109 (search_by_quarter + QuarterDump) | ✅ merged | | 2 | #110 (schema nspd_quarter_dumps) | ✅ merged | | 3 | #111 (harvest_quarter Celery + beat) | ✅ merged | | **4** | **этот PR (analyze_parcel integration)** | 🔄 | После merge — Sprint 1.1 закрыт, foundation готов. Frontend rendering этих 4 новых полей в UI — отдельный PR (next: #112 / #115 / #114). ## Backend ### `backend/app/services/site_finder/quarter_dump_lookup.py` (new) | Public | Purpose | |---|---| | `derive_quarter_cad(cad_num)` | 3/4/5-сегмент → quarter (3-segment). None если invalid. | | `get_quarter_dump_data(db, cad_num, parcel_wkt)` | Main entrypoint — reads dump, returns 4-keys dict | | `EMPTY_DUMP_RESULT` | Module const для DRY no-dump fallback | **Flow:** 1. Lookup row в `nspd_quarter_dumps` для derived quarter 2. Freshness check: `fetched_at_utc > NOW() - INTERVAL '180 days'` AND `harvest_error IS NULL` 3. Missing/stale/error → `harvest_quarter.apply_async([quarter])` fire-and-forget (lazy import против circular), return `EMPTY_DUMP_RESULT` 4. Fresh + `parcel_wkt=None` → metadata only (1 DB call) 5. Fresh + geometry → 3 spatial queries: - `_get_zoning`: point-in-polygon parcel centroid vs `territorial_zones`, LIMIT 1 - `_get_zouit_overlaps`: все `zouit_%` layers пересекающиеся с parcel - `_get_engineering_nearby`: `engineering_structures` в 200m, sorted by distance Все используют `ST_Transform(ST_SetSRID(ST_GeomFromGeoJSON(feature->'geometry'), 3857), 4326)` для inline 3857 raw NSPD в features_json → 4326 для intersection. ### `backend/app/api/v1/parcels.py` (modified) - Import `EMPTY_DUMP_RESULT, get_quarter_dump_data` - Call wrapped в `try/except Exception` с `logger.warning` + `EMPTY_DUMP_RESULT` fallback. Защита от DB unavailability (table missing после deploy / timeout) — consistent с resilience pattern других optional fetches в endpoint - Response gains 4 new fields: ```json { ..., "nspd_zoning": {"zone_code": "Ж-1", "zone_name": "...", "source": "nspd-quarter-dump"} | null, "nspd_zouit_overlaps": [{"group_key": "zouit_engineering", "subcategory": "...", "name": "..."}, ...], "nspd_engineering_nearby": [{"name": "...", "distance_m": 45, "type": "..."}, ...], "nspd_dump": { "available": true, "fetched_at_utc": "2026-05-12T12:00:00+00:00", "stale": false, "harvest_triggered": false, "total_features": 117 } } ``` ## Tests: `backend/tests/test_quarter_dump_lookup.py` (13 new) Mock-based, no real DB. Coverage: - `derive_quarter_cad`: 5 edge cases (3seg/4seg/5seg/invalid/whitespace) - `get_quarter_dump`: no_row, stale, harvest_error, no_geometry, fresh_zoning, fresh_zouit, no_match, includes_all_4_fields **13/13 pass + 47 pre-existing still pass.** ## Code review (pre-push, code-reviewer agent) Verdict: **MINOR**, 0 blocking. Applied 2 of 4 рекомендаций: - ✅ **#1**: `try/except` wrap around `get_quarter_dump_data` в `analyze_parcel` + DRY `EMPTY_DUMP_RESULT` module constant. Без этого если `nspd_quarter_dumps` недоступна → 500 instead of degraded response. - ✅ **#2**: removed redundant `nspd_zoning.fetched_at_utc` (DRY — freshness живёт в `nspd_dump.fetched_at_utc`, не дублируется в zoning sub-object). - ⏭ **#3 deferred**: ad-hoc `harvest_quarter` retry cooldown для harvest_error rows (только при high traffic + persistent NSPD errors). Acceptable defer — Celery `apply_async` идемпотентен, лишний WAF traffic не страшен на текущем scale. - ⏭ **#4 deferred**: `raw_props` в response — tech debt, убрать вместе с frontend PR. ## Performance note 3 spatial queries per analyze adds ~10-50ms на typical ~100-feature quarter. analyze_parcel cumulative budget OK. Mitigation если quarters grow dense (>500 features): materialized per-layer sub-table — отдельная database-expert issue. ## Acceptance (Sprint 1.1 #4 from #94 plan) - [x] `analyze_parcel` reads `nspd_quarter_dumps` через helper - [x] G1 ПЗЗ zone extracted (`nspd_zoning`) - [x] G3 ЗОУИТ overlaps extracted (`nspd_zouit_overlaps`) - [x] I3 engineering proximity extracted (`nspd_engineering_nearby`) - [x] Freshness metadata (`nspd_dump`) - [x] Missing/stale → harvest trigger fire-and-forget - [x] Resilience: DB error → degraded response (no 500) - [x] 13 new tests + 47 pre-existing pass - [x] Code-reviewer APPROVE (MINOR) ## Test plan - [x] `ruff check + format` — clean - [x] `pytest tests/test_quarter_dump_lookup.py -v` — 13/13 passed - [x] All 47 pre-existing tests still pass - [ ] After deploy — smoke: ```bash # Trigger harvest manually first для test quarter curl -X POST .../admin/scrape/nspd/harvest-quarter -H "X-Admin-Token: $TOKEN" \ -d '{"quarter_cad":"66:41:0204016"}' # 30s later — analyze curl -X POST .../parcels/66:41:0204016:10/analyze | jq '.nspd_dump, .nspd_zoning, .nspd_zouit_overlaps | length' # Expected: nspd_dump.available=true, nspd_zoning не null если parcel в territorial_zone, nspd_zouit_overlaps возможно []. ``` ## What's next После merge #94 part 2 Sprint 1.1 closed. Очередь по user-выбранному порядку: 1. **#112 Активные конкуренты** (UI slider + add/exclude + per-комнатность) 2. **#115 Phase 1 Connection Points** (visual layer на SiteMap) 3. **#114 Custom weights** (user POI weights override) 4. Sprint 1.2 ekburg.rf RNS (#105) 5. Sprint 1.3 G5 Gate Verdict UI (#32) Sequential rule — следующий PR создаётся ТОЛЬКО после merge этого. Part of #94. Closes Sprint 1.1 of part 2 plan.

lekss361 commented

2026-05-13 05:56:13 +00:00

(Migrated from github.com)

⚠️ Auto-review: minor (non-blocking, M1 — real mutation risk)

Backend — backend/app/services/site_finder/quarter_dump_lookup.py:

M1 (real bug, fix first): :131,136 — EMPTY_DUMP_RESULT возвращается напрямую как singleton. empty_result = EMPTY_DUMP_RESULT (ref, not copy) → return empty_result на :136. Любая downstream-мутация испортит модульный sentinel для всех последующих запросов. Дополнительно dict(empty_result) на :159/:183 — это shallow copy, вложенный nspd_dump dict shared by reference. Fix: factory function _make_empty_result() или dict(EMPTY_DUMP_RESULT) | {"nspd_dump": {...}} везде.
M2 (architectural debt): :236-394 JSONB lateral expansion без GIN индекса. Все 3 spatial helper'а делают jsonb_array_elements(d.features_json) + ST_Transform + ST_Intersects/ST_DWithin per element. Для квартала с 2000+ фичами — O(N) PostGIS-ops × 3 запроса per analyze_parcel. Митигейт прямо сейчас: pre-filter через денормализованные счётчики (territorial_zones_count > 0, zouit_count > 0, engineering_count > 0) перед отправкой тяжёлого запроса. Долгосрочно — отдельная PostGIS таблица для features. Создать GitHub issue для Sprint 1.2.
M4: :400-413 нет дедупликации harvest triggers. Burst (10 concurrent requests один незакешированный квартал) → 10 harvest задач в очередь. Митигейт: Redis SETNX с TTL перед apply_async. Документировать как known limitation.

Tests — backend/tests/test_quarter_dump_lookup.py:

M5: :580-600 _make_db_mock_with_spatial positionally fragile — завязан на порядок db.execute вызовов (0=dump, 1=zoning, 2=zouit, 3=engineering). Если реализация изменит порядок — тесты пройдут с неверными данными. Использовать call_args_list matching по SQL или задокументировать contractual order.

Positive:

SQL injection: все 3 spatial запроса через named bind params (:q, :wkt, :radius_m). Нет f-string SQL.
CRS handling корректен: ST_Transform(ST_SetSRID(..., 3857), 4326) всюду где features_json→4326. End-to-end CRS chain верна.
Fallback chain intact: except Exception → logger.warning + EMPTY_DUMP_RESULT. analyze_parcel не ломается.
Stale/error правильно триггерят harvest + возвращают available=False. Не тихо отдаёт протухшие данные.
derive_quarter_cad: isdigit() per segment validation.
Lazy import в _trigger_harvest корректно решает circular import.
7 mock-based test scenarios: stale, harvest_error, no-geometry, fresh-with-data, и др.
No print(), no hardcoded creds, no requests/psycopg2. Sync handler без async/await mixing.

Fix and push — bot will re-review automatically.

⚠️ Auto-review: minor (non-blocking, M1 — real mutation risk) **Backend — `backend/app/services/site_finder/quarter_dump_lookup.py`:** - **M1 (real bug, fix first): `:131,136` — `EMPTY_DUMP_RESULT` возвращается напрямую как singleton.** `empty_result = EMPTY_DUMP_RESULT` (ref, not copy) → `return empty_result` на :136. Любая downstream-мутация испортит модульный sentinel для всех последующих запросов. Дополнительно `dict(empty_result)` на :159/:183 — это shallow copy, вложенный `nspd_dump` dict shared by reference. Fix: factory function `_make_empty_result()` или `dict(EMPTY_DUMP_RESULT) | {"nspd_dump": {...}}` везде. - **M2 (architectural debt): `:236-394` JSONB lateral expansion без GIN индекса.** Все 3 spatial helper'а делают `jsonb_array_elements(d.features_json)` + `ST_Transform` + `ST_Intersects/ST_DWithin` per element. Для квартала с 2000+ фичами — O(N) PostGIS-ops × 3 запроса per analyze_parcel. Митигейт прямо сейчас: pre-filter через денормализованные счётчики (`territorial_zones_count > 0`, `zouit_count > 0`, `engineering_count > 0`) перед отправкой тяжёлого запроса. Долгосрочно — отдельная PostGIS таблица для features. Создать GitHub issue для Sprint 1.2. - **M4: `:400-413` нет дедупликации harvest triggers.** Burst (10 concurrent requests один незакешированный квартал) → 10 harvest задач в очередь. Митигейт: Redis SETNX с TTL перед `apply_async`. Документировать как known limitation. **Tests — `backend/tests/test_quarter_dump_lookup.py`:** - **M5: `:580-600` `_make_db_mock_with_spatial` positionally fragile** — завязан на порядок `db.execute` вызовов (0=dump, 1=zoning, 2=zouit, 3=engineering). Если реализация изменит порядок — тесты пройдут с неверными данными. Использовать `call_args_list` matching по SQL или задокументировать contractual order. **Positive:** - SQL injection: все 3 spatial запроса через named bind params (`:q`, `:wkt`, `:radius_m`). Нет f-string SQL. - CRS handling корректен: `ST_Transform(ST_SetSRID(..., 3857), 4326)` всюду где features_json→4326. End-to-end CRS chain верна. - Fallback chain intact: `except Exception → logger.warning + EMPTY_DUMP_RESULT`. `analyze_parcel` не ломается. - Stale/error правильно триггерят harvest + возвращают `available=False`. Не тихо отдаёт протухшие данные. - `derive_quarter_cad`: `isdigit()` per segment validation. - Lazy import в `_trigger_harvest` корректно решает circular import. - 7 mock-based test scenarios: stale, harvest_error, no-geometry, fresh-with-data, и др. - No `print()`, no hardcoded creds, no `requests`/`psycopg2`. Sync handler без async/await mixing. Fix and push — bot will re-review automatically.

lekss361 commented

2026-05-13 06:03:54 +00:00

(Migrated from github.com)

Все 4 findings применены в `aef8308`:

M1 (mutation risk) ✅ — введён `_make_empty_result(**overrides)` factory. Каждый caller теперь получает независимый dict — никаких shared references на nested `nspd_dump`. `EMPTY_DUMP_RESULT` сохранён только для read-only fallback в parcels.py exception handler (там не мутируется).

M2 (O(N) spatial scan) ✅ — все 3 spatial helpers принимают `layer_counts` и делают early return при count=0. SELECT расширен `territorial_zones_count` / `zouit_count` / `engineering_count` (колонки уже в schema PR #110). Тяжёлый `jsonb_array_elements` пропускается полностью когда не нужен.

M4 (documentation) ✅ — `_trigger_harvest` docstring описывает burst/no-dedup limitation + TODO Redis SETNX для следующего PR.

M5 (test fragility) ✅ — `_make_db_mock_with_spatial` docstring describes positional-call contract.

+4 новых tests (17 total, all pass):

mutation-safety on _make_empty_result
overrides correctness
early-exit при all counts=0 (только 1 DB call)
early-exit при partial counts (только 2 DB calls)

ruff + format clean. Ждём re-review.

Все 4 findings применены в \`aef8308\`: **M1 (mutation risk) ✅** — введён \`_make_empty_result(**overrides)\` factory. Каждый caller теперь получает независимый dict — никаких shared references на nested \`nspd_dump\`. \`EMPTY_DUMP_RESULT\` сохранён только для read-only fallback в parcels.py exception handler (там не мутируется). **M2 (O(N) spatial scan) ✅** — все 3 spatial helpers принимают \`layer_counts\` и делают early return при count=0. SELECT расширен \`territorial_zones_count\` / \`zouit_count\` / \`engineering_count\` (колонки уже в schema PR #110). Тяжёлый \`jsonb_array_elements\` пропускается полностью когда не нужен. **M4 (documentation) ✅** — \`_trigger_harvest\` docstring описывает burst/no-dedup limitation + TODO Redis SETNX для следующего PR. **M5 (test fragility) ✅** — \`_make_db_mock_with_spatial\` docstring describes positional-call contract. **+4 новых tests (17 total, all pass):** - mutation-safety on `_make_empty_result` - overrides correctness - early-exit при all counts=0 (только 1 DB call) - early-exit при partial counts (только 2 DB calls) ruff + format clean. Ждём re-review.

lekss361 commented

2026-05-13 06:06:48 +00:00

(Migrated from github.com)

⚠️ Auto-review: minor (non-blocking) — fixup частично:

Fixed ✅:

M1 в quarter_dump_lookup.py: _make_empty_result() factory создаёт fresh nested dicts на каждый вызов. Все return sites внутри get_quarter_dump_data используют factory. Singleton safe внутри модуля.
M2: Early bail-out по territorial_zones_count/zouit_count/engineering_count (правильные имена per schema PR#110). Positional row[4]/[5]/[6] matches SELECT order. Default-1 guards корректны.

Not fixed / deferred ⚠️:

M1 residual: parcels.py:22 exception path — nspd_dump_data = EMPTY_DUMP_RESULT (singleton) вместо _make_empty_result(). Сейчас readonly access (["nspd_zoning"] etc.) — mutation не происходит, но нарушает документированный invariant модуля. Fix: импортировать _make_empty_result в parcels.py:9 и использовать в except block.
M4 deferred: _trigger_harvest имеет TODO comment, Redis SETNX не реализован. Reasonable trade-off (UPSERT idempotency делает data-safe), но создаёт burst-duplicate task'и в очереди. Document как known limitation в vault или создать follow-up issue для Sprint 1.2.
M5 deferred: _make_db_mock_with_spatial всё ещё positional call_count dispatch. Fragility задокументирована в docstring но не устранена. test_get_quarter_dump_fresh_zouit_overlaps:716 явно устанавливает territorial_zones_count=1 искусственно чтобы сохранить slot alignment — это test smell, подтверждает что M5 fragility реальна. Fix at next test touch: match по SQL content через call_args_list.

Fix and push — bot will re-review automatically. (Или подтвердите явно "approved" / "merge it" если хотите смерджить с deferred items как-есть.)

⚠️ Auto-review: minor (non-blocking) — fixup частично: **Fixed ✅:** - **M1 в `quarter_dump_lookup.py`:** `_make_empty_result()` factory создаёт fresh nested dicts на каждый вызов. Все return sites внутри `get_quarter_dump_data` используют factory. Singleton safe внутри модуля. - **M2:** Early bail-out по `territorial_zones_count`/`zouit_count`/`engineering_count` (правильные имена per schema PR#110). Positional `row[4]/[5]/[6]` matches SELECT order. Default-1 guards корректны. **Not fixed / deferred ⚠️:** - **M1 residual: `parcels.py:22` exception path** — `nspd_dump_data = EMPTY_DUMP_RESULT` (singleton) вместо `_make_empty_result()`. Сейчас readonly access (`["nspd_zoning"]` etc.) — mutation не происходит, но нарушает документированный invariant модуля. Fix: импортировать `_make_empty_result` в `parcels.py:9` и использовать в except block. - **M4 deferred:** `_trigger_harvest` имеет TODO comment, Redis SETNX не реализован. Reasonable trade-off (UPSERT idempotency делает data-safe), но создаёт burst-duplicate task'и в очереди. Document как known limitation в vault или создать follow-up issue для Sprint 1.2. - **M5 deferred:** `_make_db_mock_with_spatial` всё ещё positional `call_count` dispatch. Fragility задокументирована в docstring но не устранена. `test_get_quarter_dump_fresh_zouit_overlaps:716` явно устанавливает `territorial_zones_count=1` искусственно чтобы сохранить slot alignment — это test smell, подтверждает что M5 fragility реальна. Fix at next test touch: match по SQL content через `call_args_list`. Fix and push — bot will re-review automatically. (Или подтвердите явно "approved" / "merge it" если хотите смерджить с deferred items как-есть.)

lekss361 commented

2026-05-13 06:12:00 +00:00

(Migrated from github.com)

M1 residual применён в `7ca4d35`:

✅ `_make_empty_result` → `make_empty_result` (public API). Импортирован в `parcels.py` exception path вместо `EMPTY_DUMP_RESULT` singleton. Каждый request получает независимый dict.

M4 (Redis SETNX dedup) и M5 (test fragility positional order) — deferred per review как acceptable trade-off:

M4: UPSERT идемпотентен, burst-duplicate tasks тратят WAF traffic впустую но данные safe. Создам follow-up issue для Sprint 1.2.
M5: docstring contractually описывает positional-call invariant. Refactor matching по SQL pattern — при следующем touch test'ов.

17/17 tests pass. ruff/format clean. Ждём re-review.

M1 residual применён в \`7ca4d35\`: ✅ \`_make_empty_result\` → \`make_empty_result\` (public API). Импортирован в \`parcels.py\` exception path вместо \`EMPTY_DUMP_RESULT\` singleton. Каждый request получает независимый dict. M4 (Redis SETNX dedup) и M5 (test fragility positional order) — **deferred per review как acceptable trade-off**: - M4: UPSERT идемпотентен, burst-duplicate tasks тратят WAF traffic впустую но данные safe. Создам follow-up issue для Sprint 1.2. - M5: docstring contractually описывает positional-call invariant. Refactor matching по SQL pattern — при следующем touch test'ов. 17/17 tests pass. ruff/format clean. Ждём re-review.

lekss361 commented

2026-05-13 06:14:08 +00:00

(Migrated from github.com)

✅ Auto-review passed (LGTM) — все residual findings корректно resolved в 7ca4d35.

Fixes verified:

M1 fully fixed: parcels.py:26 теперь вызывает make_empty_result() в exception path (не EMPTY_DUMP_RESULT singleton). Import :9-12 включает оба: get_quarter_dump_data + make_empty_result. Inline comment "Independent dict per request — never mutate module singleton" документирует invariant. Mutation-safety test :784 verifies fresh nspd_dump sub-dict + nspd_zouit_overlaps list per call.
M4 documented deferral (acceptable): docstring _trigger_harvest :459 записывает burst-duplicate limitation + TODO Redis SETNX для следующего PR. UPSERT idempotency safety net acknowledged. No data correctness risk.
M5 documented (acceptable): _make_db_mock_with_spatial :635-643 docstring explicitly explains positional contract, 7-element tuple schema, reorder instruction. Test self-aware, не silent.

Positive:

SQL все через named bind params (:q/:wkt/:radius_m) — no injection.
No hardcoded secrets, no print().
make_empty_result() создаёт fresh nested dict + sub-dict per call — verified test'ом.
EMPTY_DUMP_RESULT kept как sentinel для isinstance check'ов и mutation-test assertions.
_trigger_harvest lazy-imports harvest_quarter — correct pattern для circular import avoidance.
derive_quarter_cad covers 3/4/5-segment, whitespace, non-digit, short inputs — edge cases handled.

Cosmetic nit (не блокер): test functions testmake_empty_result_* :784/:807 без underscore после test — pytest discovers их по test-prefix, но inconsistent с остальным файлом (test_* стиль). Можно поправить при следующем касании.

Merging.

✅ Auto-review passed (LGTM) — все residual findings корректно resolved в `7ca4d35`. **Fixes verified:** - **M1 fully fixed:** `parcels.py:26` теперь вызывает `make_empty_result()` в exception path (не `EMPTY_DUMP_RESULT` singleton). Import :9-12 включает оба: `get_quarter_dump_data` + `make_empty_result`. Inline comment "Independent dict per request — never mutate module singleton" документирует invariant. Mutation-safety test :784 verifies fresh `nspd_dump` sub-dict + `nspd_zouit_overlaps` list per call. - **M4 documented deferral (acceptable):** docstring `_trigger_harvest :459` записывает burst-duplicate limitation + TODO Redis SETNX для следующего PR. UPSERT idempotency safety net acknowledged. No data correctness risk. - **M5 documented (acceptable):** `_make_db_mock_with_spatial :635-643` docstring explicitly explains positional contract, 7-element tuple schema, reorder instruction. Test self-aware, не silent. **Positive:** - SQL все через named bind params (`:q`/`:wkt`/`:radius_m`) — no injection. - No hardcoded secrets, no `print()`. - `make_empty_result()` создаёт fresh nested dict + sub-dict per call — verified test'ом. - `EMPTY_DUMP_RESULT` kept как sentinel для isinstance check'ов и mutation-test assertions. - `_trigger_harvest` lazy-imports `harvest_quarter` — correct pattern для circular import avoidance. - `derive_quarter_cad` covers 3/4/5-segment, whitespace, non-digit, short inputs — edge cases handled. **Cosmetic nit (не блокер):** test functions `testmake_empty_result_*` :784/:807 без underscore после `test` — pytest discovers их по `test`-prefix, но inconsistent с остальным файлом (`test_*` стиль). Можно поправить при следующем касании. Merging.