lekss361/gendesign

Fork 0

feat(site-finder): inline POI weights pass-through в /analyze (#201 Phase 1) #206

Merged

lekss361 merged 4 commits from feat/201-inline-weights into main

2026-05-16 11:00:42 +00:00

lekss361 commented

2026-05-16 10:39:42 +00:00

Owner

Summary

Critical UX fix для #114 — user-drag слайдеры в WeightProfilePanel теперь применяются immediately к scoring, без обязательного profile save. Issue #201 Phase 1.

Backend

POST /api/v1/parcels/{cad_num}/analyze теперь принимает optional body: AnalyzeRequest = { weights: dict[str, float] | None }
Priority order: inline → profile_id → user_default → system defaults
Validation: keys ∈ ALLOWED_CATEGORIES, values ∈ [MIN_WEIGHT, MAX_WEIGHT] → 422 при violation
Partial override: {**_POI_WEIGHTS, **inline_weights} — только переданные categories override, остальные keep defaults
Response.weights_profile.inline_weights для traceability
5 mock tests (no body / inline applied / bad cat / out-of-range / inline beats profile_id)

Frontend

useSiteAnalysis.ts — weights?: Record<string, number> | null param в analyze mutation, body JSON conditional
page.tsx — handleAnalyze всегда передаёт currentWeights если activeProfileId=null (раньше draft игнорировался); handleWeightsChange re-trigger analyze immediately если parcel loaded

Что НЕ в этом PR (sequenced sub-PRs)

Phase 2 — debounce live preview (debounced 300-500ms re-fetch при drag)
Phase 3 — Update/Delete UI для existing profiles (hooks готовы в lib/api/weightProfiles.ts, не wired)
Phase 4 (опц) — 25 categories extension (currently 11)
Phase 5 (опц) — public CRUD без admin token

Test plan

backend: pytest tests/api/v1/test_analyze_inline_weights.py -v (5 tests)
backend: ruff check + format, no :x::type, no f-string SQL
backend: regression best_layouts/competitors/connection_points tests — no break
frontend: tsc --noEmit clean, npm run lint clean, npm run build green
pre-push code-reviewer: ✅ APPROVE (2 MINOR — test fixture brittle + slider drag без debounce — оба Phase 2 follow-up)
qa-tester post-deploy: curl POST /analyze с body.weights → verify score changes; slider drag → immediate re-fetch (network panel)

Carryover bugs (НЕ в скоупе)

competitors.py:143 WHERE f.status='sold' — отдельный follow-up issue

Closes part of #201. Phase 2/3 — отдельные PR'ы после merge этого.

## Summary **Critical UX fix для #114** — user-drag слайдеры в `WeightProfilePanel` теперь применяются immediately к scoring, без обязательного profile save. Issue #201 Phase 1. ### Backend - `POST /api/v1/parcels/{cad_num}/analyze` теперь принимает optional `body: AnalyzeRequest = { weights: dict[str, float] | None }` - **Priority order**: inline → profile_id → user_default → system defaults - **Validation**: keys ∈ `ALLOWED_CATEGORIES`, values ∈ `[MIN_WEIGHT, MAX_WEIGHT]` → 422 при violation - **Partial override**: `{**_POI_WEIGHTS, **inline_weights}` — только переданные categories override, остальные keep defaults - Response.weights_profile.inline_weights для traceability - 5 mock tests (no body / inline applied / bad cat / out-of-range / inline beats profile_id) ### Frontend - `useSiteAnalysis.ts` — `weights?: Record<string, number> | null` param в analyze mutation, body JSON conditional - `page.tsx` — `handleAnalyze` всегда передаёт `currentWeights` если `activeProfileId=null` (раньше draft игнорировался); `handleWeightsChange` re-trigger analyze immediately если parcel loaded ### Что НЕ в этом PR (sequenced sub-PRs) - **Phase 2** — debounce live preview (debounced 300-500ms re-fetch при drag) - **Phase 3** — Update/Delete UI для existing profiles (hooks готовы в `lib/api/weightProfiles.ts`, не wired) - **Phase 4** (опц) — 25 categories extension (currently 11) - **Phase 5** (опц) — public CRUD без admin token ## Test plan - [x] backend: `pytest tests/api/v1/test_analyze_inline_weights.py -v` (5 tests) - [x] backend: `ruff check + format`, no `:x::type`, no f-string SQL - [x] backend: regression best_layouts/competitors/connection_points tests — no break - [x] frontend: `tsc --noEmit` clean, `npm run lint` clean, `npm run build` green - [x] pre-push code-reviewer: ✅ APPROVE (2 MINOR — test fixture brittle + slider drag без debounce — оба Phase 2 follow-up) - [ ] qa-tester post-deploy: curl POST /analyze с body.weights → verify score changes; slider drag → immediate re-fetch (network panel) ### Carryover bugs (НЕ в скоупе) - `competitors.py:143` `WHERE f.status='sold'` — отдельный follow-up issue Closes part of #201. Phase 2/3 — отдельные PR'ы после merge этого.

lekss361 added 3 commits 2026-05-16 10:39:42 +00:00

feat(layouts): PDF ТЗ endpoint + BestLayoutsBlock UI (#113 PR D) 5878596bc5

Backend (~330 LOC):
- POST /api/v1/parcels/{cad_num}/best-layouts/pdf via WeasyPrint
- services/exporters/layout_tz_pdf.py — HTML template с XSS-escape (html.escape)
  для cad_num/parcel_address/rationale_text/time_window
- 5 unit tests (skip on Windows если WeasyPrint deps missing)

Frontend (~800 LOC):
- BestLayoutsBlock.tsx — DataQualityCard + TopLayoutsTable + RecommendationCard
  + PDF download button (Blob → revokeObjectURL)
- useBestLayouts.ts — TanStack useMutation hook
- types/best-layouts.ts — manual TS types
- MarketTab.tsx +4 LOC integration после competitors

Pre-push fixes per review:
- XSS escape user-data в PDF HTML
- sold_pct_of_supply — убрать double ×100 (backend уже 0..100)
- alert() → pdfError state
- isNaN → Number.isNaN
- 7 inline-styles → Tailwind utilities

Phase 2.1 complete. Phase 2.2 (layout_type/balcony_count via B2B Объектив
#52) — follow-up issue.

fix(layouts): address review-bot minor items (#113 PR D) 434341e98f

Backend (parcels.py + layout_tz_pdf.py):
- 🟡 /best-layouts/pdf 404 handling: catch ValueError → 404
  (consistent с JSON endpoint handling геометрия не найдена)
- 🟡 XSS defense-in-depth: html.escape() для r.room_bucket,
  r.area_bin, m.room_bucket в HTML row generators

Frontend (BestLayoutsBlock.tsx):
- 🟡 data_window dates → ru-RU locale (toLocaleDateString)
- 🟢 target_total_flats clamped к [1, 10000] перед submit
  (paste-bypass на client защищён, backend Pydantic le=10000)

Tests: 5+27 backend pass, frontend tsc/lint/build clean.

feat(site-finder): inline POI weights pass-through в /analyze (#201 Phase 1) 038e39e2ec

Critical UX fix для #114 — user-drag слайдеры в WeightProfilePanel
теперь применяются immediately к scoring, без обязательного profile save.

Backend (parcels.py + schemas/parcel.py + tests):
- POST /analyze принимает optional AnalyzeRequest { weights: dict[str,float] | None }
- Priority: inline → profile_id → user_default → system defaults
- Validate against ALLOWED_CATEGORIES + [MIN_WEIGHT, MAX_WEIGHT] → 422 на violation
- Partial override semantics
- 5 mock tests

Frontend (useSiteAnalysis.ts + page.tsx):
- weights param в analyze mutation
- handleAnalyze всегда передаёт currentWeights когда activeProfileId=null
- handleWeightsChange re-trigger analyze immediately если parcel loaded

Phase 2 (debounce) + Phase 3 (Edit/Delete UI) — follow-up.

lekss361 commented

2026-05-16 10:48:21 +00:00

Author

Owner

Deep Code Review — verdict

Summary

Status: 🟡 MEDIUM (1 medium + 4 low — non-blocking, can be follow-up)
Files reviewed: 11 (P1: 3 backend + 1 schema, P2: 2 frontend hooks + 2 pages, P3: 2 components + 1 type, tests: 2)
Lines: +1609 / −15
Head SHA: 038e39e · Base: 256909d · Mergeable: true, no conflicts
Pre-flight: ✅ feature branch, no force/no-verify/amend in 10 last commits, all 3 commits clean Conventional + plain GPG-unsigned (project норма).
PR scope note: PR #206 actually merges two logical bundles — #113 PR D (PDF endpoint + BestLayoutsBlock UI, +1184 LOC) и #201 Phase 1 (inline weights, +419 LOC). Both verified.

🔴 Critical

(none)

🟠 High

(none)

🟡 Medium (желательно fix, можно follow-up)

backend/app/api/v1/parcels.py:1257-1259 — NaN bypass в inline weights validation.
Pydantic v2 with default allow_inf_nan=True принимает {"weights": {"school": NaN}} (Python json.loads тоже парсит NaN literal). Текущая проверка v < _MIN_WEIGHT or v > _MAX_WEIGHT возвращает False для NaN (любое сравнение с NaN → False), поэтому NaN проходит. Дальше score += w * decay → score становится NaN → response.score: NaN → frontend JSON.parse принимает (Node) или ломается (strict JSON). Не эксплуатируется через UI (слайдеры дают finite floats), но тривиально через curl.

Fix (минимально):
```
import math
out_of_range = {
    k: v for k, v in _inline_weights.items()
    if not math.isfinite(v) or v < _MIN_WEIGHT or v > _MAX_WEIGHT
}
```
Или на уровне Pydantic: weights: dict[str, Annotated[float, Field(allow_inf_nan=False)]]. Тот же fix желательно применить в weight_profiles.py:_validate_weights_dict (та же дыра в saved-profile path — pre-existing, не введено этим PR, но логически связано).

🟢 Low / nits (отдельным PR)

backend/app/api/v1/parcels.py:302 и backend/app/services/site_finder/weight_profiles.py:48 — duplicated _POI_WEIGHTS / _SYSTEM_POI_WEIGHTS. Сейчас идентичны (11 категорий), помечены "keep in sync" комментарием. Drift-risk при добавлении 12-й категории. Single source of truth: re-export _SYSTEM_POI_WEIGHTS из weight_profiles в parcels.py. Не блокирующее — drift пока не случился.
backend/tests/api/v1/test_analyze_inline_weights.py:445-478 — мок 16 db.execute calls по индексу — крайне fragile. Любая перестановка SQL-запросов в analyze_parcel сломает тесты. Уже отмечено в pre-push review. Long-term refactor: вместо positional mock — patch per-helper-функции (_compute_score, _load_competitors) и оставить только integration test с реальной БД. Follow-up.
frontend/src/components/site-finder/BestLayoutsBlock.tsx (#113 PR D) — 7 sub-components в одном 761-LOC файле с inline style={...} (40+ блоков). Pre-push review уже флагал. Cleanup до Tailwind utilities — follow-up.
frontend/src/types/best-layouts.ts — manual TS types, не сгенерены через npm run codegen. Комментарий в файле признаёт это. При следующем codegen run эти типы должны быть удалены и заменены OpenAPI-сгенеренными. Не блок, но не забыть.

Cross-file impact analysis

✅ useSiteAnalysis API contract (AnalyzeOptions.weights? added) — единственный consumer frontend/src/app/site-finder/page.tsx:95. Updated. Безопасно.
✅ POST /analyze body added — backward compat сохранён: AnalyzeRequest | None = None. Старые clients (без body) → body is None → fallback на старую _resolve_weights ветку. Test test_analyze_no_body_uses_system_defaults покрывает.
✅ Response shape: новое поле weights_profile.inline_weights — additive, Optional. Frontend ParcelAnalysis types обновлять опционально (если поле читается). Не ломает.
✅ _POI_WEIGHTS vs ALLOWED_CATEGORIES (drift check): 11 keys в обоих, идентичны. ОК.
✅ handleWeightsChange trigger: Я опасался, что каждый slider drag → re-analyze (full pipeline = ~5-15 db.execute + httpx Weather + Velocity Celery). На деле — WeightProfilePanel.tsx:124 handleApply() вызывает onWeightsChange ТОЛЬКО по кнопке "Применить" (или при load profile). Внутренний 300ms debounce в handleSliderChange — для visual update draft state, не для callback. Performance concern из brief'а — mitigated by UX design. Phase 2 debounce будет нужен только если изменят семантику apply-on-drag.
✅ /best-layouts/pdf endpoint — pdf renderer применяет html.escape() на user-controllable: cad_num, parcel_address, time_window, room_bucket, area_bin, rationale_text. dq.confidence (используется как CSS class) защищён Pydantic Literal["high","medium","low"]. XSS surface clean.

Vault cross-check

✅ fixes/Fix_InlineWeights_BodyIgnored_201.md уже создан для этого fix — соответствует CLAUDE.md rule #6.
✅ code/modules/Module_Weight_Profiles_Frontend.md / Module_Weight_Profiles_Service.md — есть. Желательно обновить после merge: добавить inline-weights priority order в Module_Weight_Profiles_Service.md.
✅ Никаких decisions в decisions/, нарушающих этот PR, не найдено.

Conventions check

✅ psycopg v3: ничего нового в SQL, CAST(:x AS type) соблюдён (verified в schema файле).
✅ httpx: нет новых requests imports.
✅ print(): нет.
✅ TS strict, no any: новые TS types все строго типизированы (Record<string, number>, generic Confidence).
✅ TanStack Query: useBestLayouts использует useMutation корректно.
✅ Pydantic v2: Field(default=None, description=...) — canonical pattern.
⚠️ Inline styles в BestLayoutsBlock.tsx — отступление от .claude/rules/frontend.md ("Tailwind 4 утилитарные классы; inline styles только для динамических значений"). Большая часть — статические. Не блок, но nit.

Security findings

✅ Auth: /analyze — read-only, без auth (existing pattern). /best-layouts/pdf тоже без auth — consistent с другими /best-layouts endpoints, безопасно (read-only, no PII).
✅ SQL injection: нет f-string SQL, нет :x::type concat.
✅ XSS: _html.escape() применён в PDF renderer для всех user-controllable fields.
⚠️ NaN bypass — см. 🟡 Medium выше.
✅ Path traversal: filename tz-layout-{cad_safe}-{today}.pdf — cad_num.replace(":", "-"), не пропускает / или ... Но если бы cad_num содержал /, ушло бы в Content-Disposition. Сейчас формат kadastr 66:41:0204016:10 — только digits + colons → safe.

Performance findings

⚠️ Re-trigger analyze в handleWeightsChange — task brief opasался: "каждый weight change → re-trigger". Реальность: trigger только по handleApply ("Применить" button). Each apply → full analyze_parcel pipeline (~15 SQL queries + Weather httpx 5s timeout × 2 + Celery enqueue check). На 100ms RTT БД + 200ms Weather → ~3-7s per re-analyze. Acceptable для on-demand UX, но Phase 2 debounce + cancel-in-flight нужен ЕСЛИ позже изменят на apply-on-drag.
✅ Нет новых N+1 (inline weights — pure dict merge, scoring loop остался один).
ℹ️ /best-layouts/pdf synchronously вызывает WeasyPrint (CPU-bound, ~1-3s). Должно быть в FastAPI thread pool. Используется async def + Response(content=...) без run_in_threadpool — потенциальный event loop block. Pre-existing pattern, follow-up.

Positive observations

✅ Validation messages бережно бессеблегу в detail 422 (помогает диагностике: указывают конкретные категории + диапазон).
✅ Test coverage Phase 1: 5 mock tests + 5 PDF tests — добротно. Покрывают: no body / inline applied / bad cat / out-of-range / inline beats profile_id.
✅ Partial override semantics {**_POI_WEIGHTS, **inline_weights} — корректно: если фронт послал {kindergarten: 2.5}, остальные 10 категорий keep defaults. Test test_analyze_inline_weights_applied проверяет.
✅ Traceability: response.weights_profile.inline_weights echo'нет обратно user input для debugging.
✅ Vault entry создана до PR — fixes/Fix_InlineWeights_BodyIgnored_201.md.
✅ Pre-push fixup commit 434341e уже устранил замечания review-bot (404 handling + XSS escape + locale dates + clamp).
✅ Carryover bug competitors.py:143 WHERE f.status='sold' — упомянут в PR body как НЕ в скоупе, отдельный issue. Подтверждено что не трогает (verified в diff). Не блокирует.

Recommended next steps

(Medium, желательно до merge) math.isfinite() guard в inline weights validation (+ доп. unit-test с {"school": "NaN"} payload через raw httpx).
(Low, follow-up issue) Single-source-of-truth для _POI_WEIGHTS (re-export из weight_profiles).
(Low, follow-up) PDF endpoint run_in_threadpool обёртка для WeasyPrint.
После merge — обновить code/modules/Module_Weight_Profiles_Service.md (priority order inline→profile→user→system).

Complexity / blast radius

Risk: Medium (новый optional body на read-only endpoint, no DB change, no migration)
Reversibility: Easy revert (single commit 038e39e — обратимо без data loss; #113 PR D commits 5878596+434341e — также чисто обратимы)
Recommended merge window: anytime (read-only endpoints, нет миграции, нет breaking change)

Auto-merge decision

❌ BLOCKED for auto-merge. Scope = backend/app/api/v1/parcels.py + backend/app/services/exporters/** + backend/app/schemas/parcel.py — все в .claude/rules/git-pr.md blocked-list (backend/app/api/v1/**, backend/app/services/**). Even при ✅ verdict — нужен human merge per scope policy.

Verdict сам по себе 🟡 MEDIUM (NaN issue). Recommend: либо human fix NaN guard + re-push, либо human merge как есть с отдельным follow-up issue (impact низкий — exploitable только через ручной curl).

Ping @lekss361 для merge decision.

## Deep Code Review — verdict ### Summary - **Status**: 🟡 MEDIUM (1 medium + 4 low — non-blocking, can be follow-up) - **Files reviewed**: 11 (P1: 3 backend + 1 schema, P2: 2 frontend hooks + 2 pages, P3: 2 components + 1 type, tests: 2) - **Lines**: +1609 / −15 - **Head SHA**: `038e39e` · **Base**: `256909d` · **Mergeable: true**, no conflicts - **Pre-flight**: ✅ feature branch, no force/no-verify/amend in 10 last commits, all 3 commits clean Conventional + plain GPG-unsigned (project норма). - **PR scope note**: PR #206 actually merges **two logical bundles** — `#113 PR D` (PDF endpoint + BestLayoutsBlock UI, +1184 LOC) и `#201 Phase 1` (inline weights, +419 LOC). Both verified. ### 🔴 Critical *(none)* ### 🟠 High *(none)* ### 🟡 Medium (желательно fix, можно follow-up) - [ ] **`backend/app/api/v1/parcels.py:1257-1259` — NaN bypass в inline weights validation.** Pydantic v2 with default `allow_inf_nan=True` принимает `{"weights": {"school": NaN}}` (Python `json.loads` тоже парсит `NaN` literal). Текущая проверка `v < _MIN_WEIGHT or v > _MAX_WEIGHT` возвращает `False` для NaN (любое сравнение с NaN → False), поэтому NaN проходит. Дальше `score += w * decay` → score становится NaN → response.score: NaN → frontend `JSON.parse` принимает (Node) или ломается (strict JSON). Не эксплуатируется через UI (слайдеры дают finite floats), но тривиально через `curl`. Fix (минимально): ```python import math out_of_range = { k: v for k, v in _inline_weights.items() if not math.isfinite(v) or v < _MIN_WEIGHT or v > _MAX_WEIGHT } ``` Или на уровне Pydantic: `weights: dict[str, Annotated[float, Field(allow_inf_nan=False)]]`. Тот же fix желательно применить в `weight_profiles.py:_validate_weights_dict` (та же дыра в saved-profile path — pre-existing, не введено этим PR, но логически связано). ### 🟢 Low / nits (отдельным PR) - [ ] **`backend/app/api/v1/parcels.py:302` и `backend/app/services/site_finder/weight_profiles.py:48` — duplicated `_POI_WEIGHTS` / `_SYSTEM_POI_WEIGHTS`.** Сейчас идентичны (11 категорий), помечены "keep in sync" комментарием. Drift-risk при добавлении 12-й категории. Single source of truth: re-export `_SYSTEM_POI_WEIGHTS` из `weight_profiles` в `parcels.py`. Не блокирующее — drift пока не случился. - [ ] **`backend/tests/api/v1/test_analyze_inline_weights.py:445-478` — мок 16 `db.execute` calls по индексу — крайне fragile.** Любая перестановка SQL-запросов в `analyze_parcel` сломает тесты. Уже отмечено в pre-push review. Long-term refactor: вместо positional mock — patch per-helper-функции (`_compute_score`, `_load_competitors`) и оставить только integration test с реальной БД. Follow-up. - [ ] **`frontend/src/components/site-finder/BestLayoutsBlock.tsx` (#113 PR D)** — 7 sub-components в одном 761-LOC файле с inline `style={...}` (40+ блоков). Pre-push review уже флагал. Cleanup до Tailwind utilities — follow-up. - [ ] **`frontend/src/types/best-layouts.ts` — manual TS types, не сгенерены через `npm run codegen`.** Комментарий в файле признаёт это. При следующем `codegen` run эти типы должны быть удалены и заменены OpenAPI-сгенеренными. Не блок, но не забыть. ### Cross-file impact analysis - ✅ **`useSiteAnalysis` API contract** (`AnalyzeOptions.weights?` added) — единственный consumer `frontend/src/app/site-finder/page.tsx:95`. Updated. Безопасно. - ✅ **`POST /analyze` body added** — backward compat сохранён: `AnalyzeRequest | None = None`. Старые clients (без body) → `body is None` → fallback на старую `_resolve_weights` ветку. Test `test_analyze_no_body_uses_system_defaults` покрывает. - ✅ **Response shape**: новое поле `weights_profile.inline_weights` — additive, Optional. Frontend `ParcelAnalysis` types обновлять опционально (если поле читается). Не ломает. - ✅ **`_POI_WEIGHTS` vs `ALLOWED_CATEGORIES` (drift check)**: 11 keys в обоих, идентичны. ОК. - ✅ **`handleWeightsChange` trigger**: Я опасался, что каждый slider drag → re-analyze (full pipeline = ~5-15 db.execute + httpx Weather + Velocity Celery). На деле — `WeightProfilePanel.tsx:124 handleApply()` вызывает `onWeightsChange` ТОЛЬКО по кнопке "Применить" (или при load profile). Внутренний 300ms debounce в `handleSliderChange` — для visual update draft state, не для callback. **Performance concern из brief'а — mitigated by UX design.** Phase 2 debounce будет нужен только если изменят семантику apply-on-drag. - ✅ **`/best-layouts/pdf` endpoint** — pdf renderer применяет `html.escape()` на user-controllable: `cad_num`, `parcel_address`, `time_window`, `room_bucket`, `area_bin`, `rationale_text`. `dq.confidence` (используется как CSS class) защищён Pydantic `Literal["high","medium","low"]`. XSS surface clean. ### Vault cross-check - ✅ **`fixes/Fix_InlineWeights_BodyIgnored_201.md`** уже создан для этого fix — соответствует CLAUDE.md rule #6. - ✅ **`code/modules/Module_Weight_Profiles_Frontend.md`** / **`Module_Weight_Profiles_Service.md`** — есть. Желательно обновить после merge: добавить inline-weights priority order в `Module_Weight_Profiles_Service.md`. - ✅ Никаких decisions в `decisions/`, нарушающих этот PR, не найдено. ### Conventions check - ✅ **psycopg v3**: ничего нового в SQL, `CAST(:x AS type)` соблюдён (verified в schema файле). - ✅ **httpx**: нет новых `requests` imports. - ✅ **`print()`**: нет. - ✅ **TS strict, no `any`**: новые TS types все строго типизированы (`Record<string, number>`, generic Confidence). - ✅ **TanStack Query**: `useBestLayouts` использует `useMutation` корректно. - ✅ **Pydantic v2**: `Field(default=None, description=...)` — canonical pattern. - ⚠️ **Inline styles** в BestLayoutsBlock.tsx — отступление от `.claude/rules/frontend.md` ("Tailwind 4 утилитарные классы; inline styles только для динамических значений"). Большая часть — статические. Не блок, но nit. ### Security findings - ✅ Auth: `/analyze` — read-only, без auth (existing pattern). `/best-layouts/pdf` тоже без auth — consistent с другими `/best-layouts` endpoints, безопасно (read-only, no PII). - ✅ SQL injection: нет f-string SQL, нет `:x::type` concat. - ✅ XSS: `_html.escape()` применён в PDF renderer для всех user-controllable fields. - ⚠️ NaN bypass — см. 🟡 Medium выше. - ✅ Path traversal: filename `tz-layout-{cad_safe}-{today}.pdf` — `cad_num.replace(":", "-")`, не пропускает `/` или `..`. Но если бы cad_num содержал `/`, ушло бы в Content-Disposition. Сейчас формат kadastr `66:41:0204016:10` — только digits + colons → safe. ### Performance findings - ⚠️ **Re-trigger analyze в `handleWeightsChange`** — task brief opasался: "каждый weight change → re-trigger". Реальность: trigger только по `handleApply` ("Применить" button). Each apply → full `analyze_parcel` pipeline (~15 SQL queries + Weather httpx 5s timeout × 2 + Celery enqueue check). На 100ms RTT БД + 200ms Weather → ~3-7s per re-analyze. Acceptable для on-demand UX, но Phase 2 debounce + cancel-in-flight нужен ЕСЛИ позже изменят на apply-on-drag. - ✅ Нет новых N+1 (inline weights — pure dict merge, scoring loop остался один). - ℹ️ `/best-layouts/pdf` synchronously вызывает WeasyPrint (CPU-bound, ~1-3s). Должно быть в FastAPI thread pool. Используется `async def` + `Response(content=...)` без `run_in_threadpool` — потенциальный event loop block. Pre-existing pattern, follow-up. ### Positive observations - ✅ **Validation messages бережно бессеблегу** в `detail` 422 (помогает диагностике: указывают конкретные категории + диапазон). - ✅ **Test coverage Phase 1**: 5 mock tests + 5 PDF tests — добротно. Покрывают: no body / inline applied / bad cat / out-of-range / inline beats profile_id. - ✅ **Partial override semantics** `{**_POI_WEIGHTS, **inline_weights}` — корректно: если фронт послал `{kindergarten: 2.5}`, остальные 10 категорий keep defaults. Test `test_analyze_inline_weights_applied` проверяет. - ✅ **Traceability**: `response.weights_profile.inline_weights` echo'нет обратно user input для debugging. - ✅ **Vault entry создана** до PR — `fixes/Fix_InlineWeights_BodyIgnored_201.md`. - ✅ Pre-push fixup commit `434341e` уже устранил замечания review-bot (404 handling + XSS escape + locale dates + clamp). - ✅ **Carryover bug `competitors.py:143 WHERE f.status='sold'`** — упомянут в PR body как НЕ в скоупе, отдельный issue. Подтверждено что не трогает (verified в diff). Не блокирует. ### Recommended next steps 1. **(Medium, желательно до merge)** `math.isfinite()` guard в inline weights validation (+ доп. unit-test с `{"school": "NaN"}` payload через raw httpx). 2. **(Low, follow-up issue)** Single-source-of-truth для `_POI_WEIGHTS` (re-export из `weight_profiles`). 3. **(Low, follow-up)** PDF endpoint `run_in_threadpool` обёртка для WeasyPrint. 4. После merge — обновить `code/modules/Module_Weight_Profiles_Service.md` (priority order inline→profile→user→system). ### Complexity / blast radius - **Risk**: Medium (новый optional body на read-only endpoint, no DB change, no migration) - **Reversibility**: Easy revert (single commit `038e39e` — обратимо без data loss; #113 PR D commits 5878596+434341e — также чисто обратимы) - **Recommended merge window**: anytime (read-only endpoints, нет миграции, нет breaking change) ### Auto-merge decision **❌ BLOCKED for auto-merge.** Scope = `backend/app/api/v1/parcels.py` + `backend/app/services/exporters/**` + `backend/app/schemas/parcel.py` — все в `.claude/rules/git-pr.md` blocked-list (`backend/app/api/v1/**`, `backend/app/services/**`). Even при ✅ verdict — нужен human merge per scope policy. Verdict сам по себе 🟡 MEDIUM (NaN issue). Recommend: либо human fix NaN guard + re-push, либо human merge как есть с отдельным follow-up issue (impact низкий — exploitable только через ручной curl). Ping @lekss361 для merge decision.

lekss361 referenced this pull request from a commit

2026-05-16 10:53:50 +00:00

fix(parcels): NaN/Inf bypass in inline weights validation, dedupe _POI_WEIGHTS (#201 #206 fixup)

lekss361 added 1 commit 2026-05-16 10:53:50 +00:00

fix(parcels): NaN/Inf bypass in inline weights validation, dedupe _POI_WEIGHTS (#201 #206 fixup) cada96e874

- math.isfinite() guard в out_of_range check (NaN bypass: NaN<MIN/MAX → False)
- Same guard в weight_profiles._validate_weights_dict (pre-existing dup bug)
- _POI_WEIGHTS теперь импорт из weight_profiles._SYSTEM_POI_WEIGHTS (SSOT)
- Test test_inline_weights_rejects_nan (raw NaN payload → 422)

lekss361 commented

2026-05-16 10:59:48 +00:00

Author

Owner

Deep Code Review — re-verdict (after fixup `cada96e`)

Summary

Status: ✅ APPROVE
Previous SHA: 038e39e (🟡 MEDIUM — 1 NaN bypass, 4 🟢 nits)
New SHA: cada96e (fixup +35/-19, 3 files touched)
Counts: 0 🔴 / 0 🟠 / 0 🟡 / 4 🟢 (informational only, same as before)

🟡 → ✅ NaN bypass resolved

Confirmed fix in 3 layers:

backend/app/api/v1/parcels.py:1257 (inline validation):
```
if not math.isfinite(v) or v < _MIN_WEIGHT or v > _MAX_WEIGHT
```
Plain NaN < MIN evaluates False → bypass теперь невозможен. ✅
backend/app/services/site_finder/weight_profiles.py:118 — defense-in-depth fix для _validate_weights_dict (pre-existing dup bug на CRUD path для сохранённых профилей). Bonus. ✅
backend/tests/api/v1/test_analyze_inline_weights.py::test_inline_weights_rejects_nan — regression test: отправляет raw bytes b'{"weights": {"school": NaN}}' (т.к. httpx не encode float('nan')), expect 422. ✅

🟢 → ✅ `_POI_WEIGHTS` duplication resolved

_POI_WEIGHTS теперь не локальная константа в parcels.py, а импорт из weight_profiles._SYSTEM_POI_WEIGHTS (SSOT). Docstring обновлён ("single source of truth; imported by api/v1/parcels.py"). Drift risk устранён. ✅

🟢 Остающиеся nits (informational, не блокирующие)

test_analyze_inline_weights.py — 16-call positional mock в _make_db_for_analyze остался brittle (см. комментарии 0..15). Если в analyze_parcel поменяется порядок execute() — тесты молча сломаются. Recommendation: дальше через side_effect dispatch on SQL pattern, но это рефакторинг не для этого PR.
BestLayoutsBlock.tsx 761 LOC inline-style heavy — Tailwind migration отдельным PR.
frontend/src/types/best-layouts.ts — manual TS types, расхождение с Pydantic возможно. Run npm run codegen если/когда codegen wired для этого endpoint.
_inline_weights хранится в response.weights_profile.inline_weights как None или объект — frontend должен handle оба case'а (видно в useSiteAnalysis.ts корректно).

Cross-file impact analysis

_POI_WEIGHTS импорт в parcels.py — проверил: используется в score accumulation (line ~1267), в weights_applied response field (line ~1965). Все consumers получают тот же dict object — OK.
math.isfinite — import math уже был в weight_profiles.py (added строка 19), не сломал ничего ещё импортирующего модуль.
New test file test_analyze_inline_weights.py — изолированный, использует dependency_overrides + patch() для тяжёлых вызовов — не зависит от env / реальной DB.

Pre-merge checks

✅ mergeable=true
✅ State open
✅ Head SHA cada96e matches scan
⚠️ CI status — не проверил (рекомендуется убедиться что backend tests + ruff green перед merge)

Auto-merge policy

❌ BLOCKED for auto-merge per .claude/agents/deep-code-reviewer.md:
PR touches backend/app/api/v1/**, backend/app/services/**, backend/app/schemas/** → human approval required даже при ✅ APPROVE.

Recommended action

Human merge (squash + delete branch). Все 🟡 findings resolved, no new issues introduced fixup'ом.

Complexity / blast radius

Risk: Low (validation logic isolated, no schema changes affecting clients)
Reversibility: Easy revert (no DB migration, no breaking API change — weights field optional)
Merge window: anytime

## Deep Code Review — re-verdict (after fixup `cada96e`)  ### Summary - **Status**: ✅ **APPROVE** - **Previous SHA**: `038e39e` (🟡 MEDIUM — 1 NaN bypass, 4 🟢 nits) - **New SHA**: `cada96e` (fixup +35/-19, 3 files touched) - **Counts**: 0 🔴 / 0 🟠 / 0 🟡 / 4 🟢 (informational only, same as before) ### 🟡 → ✅ NaN bypass resolved **Confirmed fix in 3 layers:** 1. `backend/app/api/v1/parcels.py:1257` (inline validation): ```python if not math.isfinite(v) or v < _MIN_WEIGHT or v > _MAX_WEIGHT ``` Plain `NaN < MIN` evaluates `False` → bypass теперь невозможен. ✅ 2. `backend/app/services/site_finder/weight_profiles.py:118` — defense-in-depth fix для `_validate_weights_dict` (pre-existing dup bug на CRUD path для сохранённых профилей). Bonus. ✅ 3. `backend/tests/api/v1/test_analyze_inline_weights.py::test_inline_weights_rejects_nan` — regression test: отправляет raw bytes `b'{"weights": {"school": NaN}}'` (т.к. httpx не encode `float('nan')`), expect 422. ✅ ### 🟢 → ✅ `_POI_WEIGHTS` duplication resolved `_POI_WEIGHTS` теперь не локальная константа в `parcels.py`, а импорт из `weight_profiles._SYSTEM_POI_WEIGHTS` (SSOT). Docstring обновлён ("single source of truth; imported by api/v1/parcels.py"). Drift risk устранён. ✅ ### 🟢 Остающиеся nits (informational, не блокирующие) - `test_analyze_inline_weights.py` — 16-call positional mock в `_make_db_for_analyze` остался brittle (см. комментарии 0..15). Если в `analyze_parcel` поменяется порядок execute() — тесты молча сломаются. Recommendation: дальше через side_effect dispatch on SQL pattern, но это рефакторинг не для этого PR. - `BestLayoutsBlock.tsx` 761 LOC inline-style heavy — Tailwind migration отдельным PR. - `frontend/src/types/best-layouts.ts` — manual TS types, расхождение с Pydantic возможно. Run `npm run codegen` если/когда codegen wired для этого endpoint. - `_inline_weights` хранится в response.weights_profile.inline_weights как `None` или объект — frontend должен handle оба case'а (видно в `useSiteAnalysis.ts` корректно). ### Cross-file impact analysis - `_POI_WEIGHTS` импорт в `parcels.py` — проверил: используется в `score` accumulation (line ~1267), в `weights_applied` response field (line ~1965). Все consumers получают тот же dict object — OK. - `math.isfinite` — `import math` уже был в `weight_profiles.py` (added строка 19), не сломал ничего ещё импортирующего модуль. - New test file `test_analyze_inline_weights.py` — изолированный, использует `dependency_overrides` + `patch()` для тяжёлых вызовов — не зависит от env / реальной DB. ### Pre-merge checks - ✅ `mergeable=true` - ✅ State `open` - ✅ Head SHA `cada96e` matches scan - ⚠️ CI status — не проверил (рекомендуется убедиться что backend tests + ruff green перед merge) ### Auto-merge policy **❌ BLOCKED for auto-merge** per `.claude/agents/deep-code-reviewer.md`: PR touches `backend/app/api/v1/**`, `backend/app/services/**`, `backend/app/schemas/**` → human approval required даже при ✅ APPROVE. ### Recommended action Human merge (squash + delete branch). Все 🟡 findings resolved, no new issues introduced fixup'ом. ### Complexity / blast radius - **Risk**: Low (validation logic isolated, no schema changes affecting clients) - **Reversibility**: Easy revert (no DB migration, no breaking API change — `weights` field optional) - **Merge window**: anytime

lekss361 merged commit 092976f656 into main

2026-05-16 11:00:42 +00:00

lekss361 referenced this pull request from a commit

2026-05-16 11:00:43 +00:00

feat(site-finder): inline POI weights pass-through в /analyze (#201 Phase 1) (#206)