lekss361/gendesign

Fork 0

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

Merged

lekss361 merged 2 commits from feat/113-pdf-tz-and-ui-v2 into main

2026-05-16 09:53:05 +00:00

lekss361 commented

2026-05-16 09:40:16 +00:00

Owner

Summary

Final PR (D из 4) для Issue #113 Layout Analysis. Builds on PR #193 (A) + PR #194 (B) + PR #196 (C).

(PR #198 closed — stale branch from PR C base SHA; this is replacement, identical content cherry-picked onto current main)

Backend (~330 LOC)

POST /api/v1/parcels/{cad_num}/best-layouts/pdf — WeasyPrint PDF "ТЗ на проектирование (data-driven)"
backend/app/services/exporters/layout_tz_pdf.py (NEW) — render_layout_tz_pdf() с XSS-escape (html.escape) для cad_num/parcel_address/rationale_text/time_window
5 unit tests (skip on Windows если GTK/Pango missing — OSError|ImportError guard)

Frontend (~800 LOC)

BestLayoutsBlock.tsx (NEW) — DataQualityCard + TopLayoutsTable + RecommendationCard + кнопка «Скачать ТЗ (PDF)» (Blob + revokeObjectURL)
useBestLayouts.ts (NEW) — TanStack useMutation hook
types/best-layouts.ts (NEW) — manual TS types
MarketTab.tsx (+4 LOC) — integration

Pre-push review fixes

🟠 XSS escape для PDF HTML template
🟠 sold_pct_of_supply — убрать ×100 (backend уже 0..100 percent)
alert() → pdfError state
isNaN() → Number.isNaN()
7 inline styles → Tailwind utilities (~40 LOC)
BytesIO unused import removed
Test guard OSError|ImportError

Phase 2.1 complete

Завершает #113. Phase 2.2 (layout_type + balcony_count via B2B Объектив #52) — отдельный issue.

Test plan

backend: pytest tests/test_layout_tz_pdf.py -v — 5 tests (skip Windows, run Docker CI)
backend: pytest tests/api/v1/test_parcel_best_layouts.py tests/test_layout_signature.py -q — 53/53 pass
backend: ruff + format + mypy clean
frontend: tsc --noEmit, npm run lint, npm run build — all clean (17/17 pages)
code-reviewer pre-push: ✅ HIGH→APPROVE после fixup
qa-tester post-deploy:
- curl POST /best-layouts/pdf → 200 + valid PDF
- UI smoke chrome-devtools: /site-finder/{cad} → MarketTab → BestLayoutsBlock visible + PDF download works

Closes #113.

Related: #197 (test-infra integration gate follow-up).

## Summary **Final PR (D из 4)** для Issue #113 Layout Analysis. Builds on PR #193 (A) + PR #194 (B) + PR #196 (C). *(PR #198 closed — stale branch from PR C base SHA; this is replacement, identical content cherry-picked onto current main)* ### Backend (~330 LOC) - **`POST /api/v1/parcels/{cad_num}/best-layouts/pdf`** — WeasyPrint PDF "ТЗ на проектирование (data-driven)" - **`backend/app/services/exporters/layout_tz_pdf.py`** (NEW) — `render_layout_tz_pdf()` с XSS-escape (`html.escape`) для `cad_num`/`parcel_address`/`rationale_text`/`time_window` - **5 unit tests** (skip on Windows если GTK/Pango missing — `OSError|ImportError` guard) ### Frontend (~800 LOC) - **`BestLayoutsBlock.tsx`** (NEW) — DataQualityCard + TopLayoutsTable + RecommendationCard + кнопка «Скачать ТЗ (PDF)» (Blob + `revokeObjectURL`) - **`useBestLayouts.ts`** (NEW) — TanStack `useMutation` hook - **`types/best-layouts.ts`** (NEW) — manual TS types - **`MarketTab.tsx`** (+4 LOC) — integration ### Pre-push review fixes - 🟠 XSS escape для PDF HTML template - 🟠 `sold_pct_of_supply` — убрать `×100` (backend уже 0..100 percent) - `alert()` → `pdfError` state - `isNaN()` → `Number.isNaN()` - 7 inline styles → Tailwind utilities (~40 LOC) - `BytesIO` unused import removed - Test guard `OSError|ImportError` ### Phase 2.1 complete Завершает #113. Phase 2.2 (`layout_type` + `balcony_count` via B2B Объектив #52) — отдельный issue. ## Test plan - [x] backend: `pytest tests/test_layout_tz_pdf.py -v` — 5 tests (skip Windows, run Docker CI) - [x] backend: `pytest tests/api/v1/test_parcel_best_layouts.py tests/test_layout_signature.py -q` — 53/53 pass - [x] backend: ruff + format + mypy clean - [x] frontend: `tsc --noEmit`, `npm run lint`, `npm run build` — all clean (17/17 pages) - [x] code-reviewer pre-push: ✅ HIGH→APPROVE после fixup - [ ] **qa-tester post-deploy**: - `curl POST /best-layouts/pdf` → 200 + valid PDF - UI smoke chrome-devtools: `/site-finder/{cad}` → MarketTab → BestLayoutsBlock visible + PDF download works Closes #113. Related: #197 (test-infra integration gate follow-up).

lekss361 added 1 commit 2026-05-16 09:40:17 +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.

lekss361 commented

2026-05-16 09:45:38 +00:00

Author

Owner

Deep Code Review — verdict

Summary

Status: ⚠️ MINOR — APPROVE eligible (no 🔴/🟠 blockers, 4 🟡 medium / 2 🟢 low)
Files reviewed: 7 (P1: 2, P2: 1, P3: 4)
Lines: +1184 / -0
PR: #199 · Head SHA 5878596 · base ad16fc0e · mergeable: true
Time spent: ~12 min

🔴 Critical

none

🟠 High

none

🟡 Medium (желательно fix, но не блокирующее)

backend/app/api/v1/parcels.py:2147 — inconsistent 404 handling
/best-layouts JSON endpoint converts ValueError("Геометрия для … не найдена") → 404, но новый /best-layouts/pdf endpoint только ловит HTTPException + generic Exception → возвращает 500 вместо 404 для несуществующего участка. Fix:
```
except HTTPException:
    raise
except ValueError as exc:
    raise HTTPException(status_code=404, detail=str(exc)) from exc
except Exception as exc:
    logger.error(...)
    raise HTTPException(status_code=500, detail="Internal server error") from exc
```
backend/app/services/exporters/layout_tz_pdf.py:51-58, 61-67 — XSS escape incomplete (defense-in-depth)
Escaped: cad_num, parcel_address, time_window, rationale_text ✅
НЕ escaped: r.room_bucket, r.area_bin, r.signature, m.room_bucket (внутри <td>…</td>). Pydantic schema TopLayoutRow.room_bucket — просто str (не Literal), значения сейчас идут из доверенного MV mv_layout_velocity, но без escape любой будущий шаг (например prompt → ML генерит buckets) станет XSS-vector. Рекомендую обернуть _html.escape(r.room_bucket) etc для consistency.
frontend/src/components/site-finder/BestLayoutsBlock.tsx:329-331 — даты выводятся как raw ISO
```
период {rec.data_window_start} — {rec.data_window_end}
```
Pydantic сериализует dt.date как "2026-02-01". UI получит 2026-02-01 — 2026-05-01 вместо 01.02.2026 — 01.05.2026. Не критично, но не соответствует ru-локали (которая консистентно используется выше для чисел через toLocaleString("ru-RU")).
backend/app/api/v1/parcels.py:2137 — sync get_best_layouts() в async handler блокирует event loop
async def get_parcel_best_layouts_pdf(...) → вызывает sync get_best_layouts(db, ...) + sync render_layout_tz_pdf(...) (WeasyPrint CPU-bound, ~100-500ms). Для PDF это особенно ощутимо (CPU expensive). Это established pattern в codebase (тот же /best-layouts JSON endpoint так делает) — не блокирующее, но в идеале PDF gen уехала бы в asyncio.to_thread() или Celery task. Отметить как follow-up.

🟢 Low / nits

backend/app/api/v1/parcels.py:2160 — Content-Disposition filename без RFC 5987 fallback
f'attachment; filename="{filename}"' — cad_num в проекте всегда ASCII (\d+:\d+:…), но если в будущем filename получит Cyrillic (например через parcel_address), нужен filename*=UTF-8''…. Сейчас OK.
frontend/src/components/site-finder/BestLayoutsBlock.tsx:467 — клиентский input допускает > backend max
target_total_flats backend Pydantic le=10000, frontend <input type="number" max={10000}> — max это hint, paste может проскочить. На invalid backend вернёт 422 → попадёт в error handler. Можно clamp на клиенте до Math.min(parsed, 10000).

Cross-file impact analysis

✅ BestLayoutsRequest shape: parcel.py:151-160 (backend) vs types/best-layouts.ts:10-18 (frontend) — match 1:1 (все 7 полей с правильной optional/required семантикой).
✅ BestLayoutsResponse / TopLayoutRow / LayoutTzMixRow / LayoutTzRecommendation / LayoutDataQuality — все 5 schemas match между Pydantic и TS.
✅ apiFetch (lib/api.ts:5-20) уже sets Content-Type: application/json — useBestLayouts.ts использует корректно. PDF endpoint в BestLayoutsBlock.tsx:451-459 использует raw fetch (apiFetch не подходит т.к. возвращает binary blob, не JSON) — это правильное решение.
✅ Endpoint URL /api/v1/parcels/{encoded}/best-layouts/pdf в frontend (BestLayoutsBlock.tsx:454) match backend route (parcels.py:2136).
✅ Auth: новый /best-layouts/pdf НЕ требует _check_token — consistent с /best-layouts и /competitors (весь /parcels/* namespace public-read в этом проекте, не auth issue).
✅ get_best_layouts() (PR #196) reused, не дублирован. Renderer изолирован в новом модуле services/exporters/layout_tz_pdf.py.
✅ MarketTab.tsx integration — +4 LOC, минимальная, без breaking changes для других tabs.
✅ Note: existing services/exporters/pdf.py это 4-строчная заглушка ("Stage 1c") — layout_tz_pdf.py это первая реальная WeasyPrint реализация в prod. Pattern reference в docstring корректен (заглушку как pattern не возьмёшь, но архитектурно правильно).

Vault cross-check

✅ domains/api/endpoints/Endpoint_Parcel_BestLayouts.md уже создан и описывает PDF endpoint + renderer + tests (соответствует CLAUDE.md rule #6: "vault обновляется вместе с кодом").
✅ domains/api/endpoints/api-endpoints-MOC.md — entry "Endpoint_Parcel_BestLayouts — JSON + PDF ТЗ (#113 PR D)" присутствует.
✅ Поиск "WeasyPrint" в vault: только meta/Stages + TechStack mentions, нет prior bugs / fixes — это первая WeasyPrint feature в prod, нет regression risks.
✅ fixes/ — нет похожих PDF / XSS bugs которые могли бы быть повторены.
✅ decisions/ — нет архитектурных решений, нарушаемых этим PR.

Conventions

✅ Backend: psycopg v3 (нет SQL в новом коде), httpx (нет HTTP), async def for handlers, logger.* not print()
✅ Ruff clean: uv run ruff check backend/app/services/exporters/layout_tz_pdf.py backend/app/api/v1/parcels.py backend/tests/test_layout_tz_pdf.py → All checks passed
✅ TS strict: no any, manual types match Pydantic (codegen может быть позже)
✅ TanStack useMutation для POST (правильно — это mutation, не query)
✅ "use client" в каждом client component
✅ Test guard pattern try/except (OSError, ImportError) → pytest.skip(allow_module_level=True) — корректный workaround для Windows-без-GTK

Tests coverage

5 тестов покрывают:

✅ Non-empty bytes
✅ PDF magic bytes (%PDF)
✅ Cyrillic rationale_text (no UnicodeEncodeError)
✅ Empty top_layouts edge case
✅ Null avg_price_per_m2_rub edge case

⚠️ Не покрыто: 404 path (если get_best_layouts raises ValueError), но это можно покрыть через integration test endpoint'а отдельным PR.

Positive observations

✅ XSS escape html.escape() на critical user-data inputs (cad_num/parcel_address/rationale_text)
✅ URL.revokeObjectURL() после Blob download — нет memory leak
✅ Number.isNaN() вместо isNaN() (per ESLint best practice)
✅ alert() → pdfError state (per UX best practice)
✅ Loading skeleton + error state + idle state — все 4 состояния UI явно обработаны
✅ Tailwind utilities используются для большинства layout (inline styles только для динамических color/width values — допустимо per frontend.md)
✅ DataQualityCard color-coded confidence badge — consistent с PDF rendering
✅ Filename sanitization для PDF (cad_num.replace(":", "-")) — нет path traversal

Pre-flight

✅ Feature branch: feat/113-pdf-tz-and-ui-v2
✅ Single squashed commit, no force/amend/no-verify в истории
✅ Mergeable: true, no conflicts
✅ Base ad16fc0e = main HEAD (includes merged #193/#194/#196)

Complexity / blast radius

Risk: Low — new endpoint (additive), new file, +4 LOC frontend integration, no schema changes, no DB changes
Reversibility: Easy revert — single commit, no migrations, no data side-effects
Recommended merge window: anytime — completes Issue #113 Phase 2.1

Recommended next steps

(opt) Medium fix: parcels.py:2147 — catch ValueError → 404 for consistency с JSON endpoint (10 LOC change, безопасно). Можно отдельным follow-up PR.
(opt) Medium fix: layout_tz_pdf.py — _html.escape() на room_bucket/area_bin/signature в loop generators (defense-in-depth).
(opt) Low fix: Format dates ru-locale в BestLayoutsBlock.tsx recommendation card meta.
Post-deploy QA (per PR test plan): qa-tester → curl /best-layouts/pdf + chrome-devtools UI smoke. Не повторяю — внешний qa-tester subagent сделает.

Verdict: ⚠️ MINOR — eligible for human merge. Все 🟡 medium issues — non-blocking, можно адресовать follow-up. Scope (backend/app/api/v1/** + backend/app/services/**) blocked для auto-merge per git-pr.md — нужен human approval. Per user policy ("MINOR не мержим, только ✅ APPROVE") — main session должна спросить user перед merge.

cc: main session — verdict готов, scope blocked, не мержу.

## Deep Code Review — verdict ### Summary - **Status**: ⚠️ MINOR — APPROVE eligible (no 🔴/🟠 blockers, 4 🟡 medium / 2 🟢 low) - **Files reviewed**: 7 (P1: 2, P2: 1, P3: 4) - **Lines**: +1184 / -0 - **PR**: #199 · Head SHA `5878596` · base `ad16fc0e` · mergeable: true - **Time spent**: ~12 min ### 🔴 Critical _none_ ### 🟠 High _none_ ### 🟡 Medium (желательно fix, но не блокирующее) 1. **`backend/app/api/v1/parcels.py:2147` — inconsistent 404 handling** `/best-layouts` JSON endpoint converts `ValueError("Геометрия для … не найдена")` → 404, но новый `/best-layouts/pdf` endpoint только ловит `HTTPException` + generic `Exception` → возвращает **500 вместо 404** для несуществующего участка. Fix: ```python except HTTPException: raise except ValueError as exc: raise HTTPException(status_code=404, detail=str(exc)) from exc except Exception as exc: logger.error(...) raise HTTPException(status_code=500, detail="Internal server error") from exc ``` 2. **`backend/app/services/exporters/layout_tz_pdf.py:51-58, 61-67` — XSS escape incomplete (defense-in-depth)** Escaped: `cad_num`, `parcel_address`, `time_window`, `rationale_text` ✅ НЕ escaped: `r.room_bucket`, `r.area_bin`, `r.signature`, `m.room_bucket` (внутри `<td>…</td>`). Pydantic schema `TopLayoutRow.room_bucket` — просто `str` (не `Literal`), значения сейчас идут из доверенного MV `mv_layout_velocity`, но без escape любой будущий шаг (например prompt → ML генерит buckets) станет XSS-vector. Рекомендую обернуть `_html.escape(r.room_bucket)` etc для consistency. 3. **`frontend/src/components/site-finder/BestLayoutsBlock.tsx:329-331` — даты выводятся как raw ISO** ```tsx период {rec.data_window_start} — {rec.data_window_end} ``` Pydantic сериализует `dt.date` как `"2026-02-01"`. UI получит `2026-02-01 — 2026-05-01` вместо `01.02.2026 — 01.05.2026`. Не критично, но не соответствует ru-локали (которая консистентно используется выше для чисел через `toLocaleString("ru-RU")`). 4. **`backend/app/api/v1/parcels.py:2137` — sync `get_best_layouts()` в async handler блокирует event loop** `async def get_parcel_best_layouts_pdf(...)` → вызывает sync `get_best_layouts(db, ...)` + sync `render_layout_tz_pdf(...)` (WeasyPrint CPU-bound, ~100-500ms). Для PDF это особенно ощутимо (CPU expensive). Это **established pattern** в codebase (тот же `/best-layouts` JSON endpoint так делает) — не блокирующее, но в идеале PDF gen уехала бы в `asyncio.to_thread()` или Celery task. Отметить как follow-up. ### 🟢 Low / nits 1. **`backend/app/api/v1/parcels.py:2160` — `Content-Disposition` filename без RFC 5987 fallback** `f'attachment; filename="{filename}"'` — cad_num в проекте всегда ASCII (`\d+:\d+:…`), но если в будущем filename получит Cyrillic (например через `parcel_address`), нужен `filename*=UTF-8''…`. Сейчас OK. 2. **`frontend/src/components/site-finder/BestLayoutsBlock.tsx:467` — клиентский input допускает > backend max** `target_total_flats` backend Pydantic `le=10000`, frontend `<input type="number" max={10000}>` — `max` это hint, paste может проскочить. На invalid backend вернёт 422 → попадёт в error handler. Можно clamp на клиенте до `Math.min(parsed, 10000)`. ### Cross-file impact analysis ✅ `BestLayoutsRequest` shape: `parcel.py:151-160` (backend) vs `types/best-layouts.ts:10-18` (frontend) — match 1:1 (все 7 полей с правильной optional/required семантикой). ✅ `BestLayoutsResponse` / `TopLayoutRow` / `LayoutTzMixRow` / `LayoutTzRecommendation` / `LayoutDataQuality` — все 5 schemas match между Pydantic и TS. ✅ `apiFetch` (`lib/api.ts:5-20`) уже sets `Content-Type: application/json` — `useBestLayouts.ts` использует корректно. PDF endpoint в `BestLayoutsBlock.tsx:451-459` использует raw `fetch` (apiFetch не подходит т.к. возвращает binary blob, не JSON) — это правильное решение. ✅ Endpoint URL `/api/v1/parcels/{encoded}/best-layouts/pdf` в frontend (`BestLayoutsBlock.tsx:454`) match backend route (`parcels.py:2136`). ✅ Auth: новый `/best-layouts/pdf` НЕ требует `_check_token` — consistent с `/best-layouts` и `/competitors` (весь `/parcels/*` namespace public-read в этом проекте, не auth issue). ✅ `get_best_layouts()` (PR #196) reused, не дублирован. Renderer изолирован в новом модуле `services/exporters/layout_tz_pdf.py`. ✅ `MarketTab.tsx` integration — +4 LOC, минимальная, без breaking changes для других tabs. ✅ Note: existing `services/exporters/pdf.py` это 4-строчная заглушка ("Stage 1c") — `layout_tz_pdf.py` это **первая реальная** WeasyPrint реализация в prod. Pattern reference в docstring корректен (заглушку как pattern не возьмёшь, но архитектурно правильно). ### Vault cross-check ✅ `domains/api/endpoints/Endpoint_Parcel_BestLayouts.md` уже создан и описывает PDF endpoint + renderer + tests (соответствует CLAUDE.md rule #6: "vault обновляется вместе с кодом"). ✅ `domains/api/endpoints/api-endpoints-MOC.md` — entry "Endpoint_Parcel_BestLayouts — JSON + PDF ТЗ (#113 PR D)" присутствует. ✅ Поиск "WeasyPrint" в vault: только meta/Stages + TechStack mentions, нет prior bugs / fixes — это первая WeasyPrint feature в prod, нет regression risks. ✅ `fixes/` — нет похожих PDF / XSS bugs которые могли бы быть повторены. ✅ `decisions/` — нет архитектурных решений, нарушаемых этим PR. ### Conventions - ✅ Backend: psycopg v3 (нет SQL в новом коде), httpx (нет HTTP), async def for handlers, `logger.*` not `print()` - ✅ Ruff clean: `uv run ruff check backend/app/services/exporters/layout_tz_pdf.py backend/app/api/v1/parcels.py backend/tests/test_layout_tz_pdf.py` → All checks passed - ✅ TS strict: no `any`, manual types match Pydantic (codegen может быть позже) - ✅ TanStack `useMutation` для POST (правильно — это mutation, не query) - ✅ `"use client"` в каждом client component - ✅ Test guard pattern `try/except (OSError, ImportError) → pytest.skip(allow_module_level=True)` — корректный workaround для Windows-без-GTK ### Tests coverage 5 тестов покрывают: - ✅ Non-empty bytes - ✅ PDF magic bytes (`%PDF`) - ✅ Cyrillic rationale_text (no UnicodeEncodeError) - ✅ Empty top_layouts edge case - ✅ Null avg_price_per_m2_rub edge case ⚠️ Не покрыто: 404 path (если `get_best_layouts` raises ValueError), но это можно покрыть через integration test endpoint'а отдельным PR. ### Positive observations - ✅ XSS escape `html.escape()` на critical user-data inputs (cad_num/parcel_address/rationale_text) - ✅ `URL.revokeObjectURL()` после Blob download — нет memory leak - ✅ `Number.isNaN()` вместо `isNaN()` (per ESLint best practice) - ✅ `alert()` → `pdfError` state (per UX best practice) - ✅ Loading skeleton + error state + idle state — все 4 состояния UI явно обработаны - ✅ Tailwind utilities используются для большинства layout (inline styles только для динамических color/width values — допустимо per `frontend.md`) - ✅ DataQualityCard color-coded confidence badge — consistent с PDF rendering - ✅ Filename sanitization для PDF (`cad_num.replace(":", "-")`) — нет path traversal ### Pre-flight - ✅ Feature branch: `feat/113-pdf-tz-and-ui-v2` - ✅ Single squashed commit, no force/amend/no-verify в истории - ✅ Mergeable: true, no conflicts - ✅ Base `ad16fc0e` = main HEAD (includes merged #193/#194/#196) ### Complexity / blast radius - **Risk**: Low — new endpoint (additive), new file, +4 LOC frontend integration, no schema changes, no DB changes - **Reversibility**: Easy revert — single commit, no migrations, no data side-effects - **Recommended merge window**: anytime — completes Issue #113 Phase 2.1 ### Recommended next steps 1. **(opt) Medium fix**: `parcels.py:2147` — catch `ValueError → 404` for consistency с JSON endpoint (10 LOC change, безопасно). Можно отдельным follow-up PR. 2. **(opt) Medium fix**: `layout_tz_pdf.py` — `_html.escape()` на `room_bucket`/`area_bin`/`signature` в loop generators (defense-in-depth). 3. **(opt) Low fix**: Format dates ru-locale в `BestLayoutsBlock.tsx` recommendation card meta. 4. **Post-deploy QA** (per PR test plan): qa-tester → curl `/best-layouts/pdf` + chrome-devtools UI smoke. Не повторяю — внешний qa-tester subagent сделает. --- **Verdict: ⚠️ MINOR — eligible for human merge.** Все 🟡 medium issues — non-blocking, можно адресовать follow-up. Scope (`backend/app/api/v1/**` + `backend/app/services/**`) **blocked для auto-merge** per `git-pr.md` — нужен human approval. Per user policy ("MINOR не мержим, только ✅ APPROVE") — main session должна спросить user перед merge. cc: main session — verdict готов, scope blocked, не мержу.

lekss361 added 1 commit 2026-05-16 09:50:19 +00:00

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.

lekss361 commented

2026-05-16 09:52:36 +00:00

Author

Owner

Deep Code Review — re-verdict (after fixup)

Summary

Status: ✅ APPROVE
SHA: 434341e (was 5878596)
Fixup: +11 / -5 across 3 files
Counts: 0🔴 / 0🟠 / 0🟡 / 1🟢 (informational)

Previous 🟡 items — all addressed

#	Item	Status	Evidence
1	PDF endpoint 404 vs 500 для missing parcel	✅ FIXED	`parcels.py` теперь ловит `ValueError → 404` перед catch-all `Exception → 500`. Consistent с JSON `/best-layouts` endpoint.
2	XSS escape на table cells (`room_bucket`/`area_bin`)	✅ FIXED	`layout_tz_pdf.py` — `_html.escape()` добавлен в top_rows на `r.room_bucket` + `r.area_bin`, и в mix_rows на `m.room_bucket`. `signature` в HTML не рендерится (только в TS types) → escape не требуется.
3	ru-locale date formatting (`dd.MM.yyyy`)	✅ FIXED	`BestLayoutsBlock.tsx` — `new Date(rec.data_window_start).toLocaleDateString("ru-RU")` для start + end.
4	`target_total_flats` client clamp	✅ FIXED	`buildRequest()` — `Math.min(Math.max(parsed, 1), 10000)`. Backend Pydantic `le=10000` остаётся как defense-in-depth.

New issues introduced — none 🔴/🟠/🟡

Reviewed +11 lines line-by-line:

_html.escape(r.room_bucket/area_bin, m.room_bucket) — values typed как str enum-ish в Pydantic schemas, escape safe ✅
except ValueError → 404 ordering правильный: HTTPException first (re-raise 4xx from inner), затем ValueError, затем catch-all ✅
Math.min(Math.max(parsed, 1), 10000) — корректно clamps paste-attacks (99999999 → 10000, negative → 1). NaN guarded upstream через Number.isNaN(parsed) ✅
toLocaleDateString("ru-RU") — runs корректно в Node SSR + browser, рендерит формат dd.MM.yyyy ✅

🟢 Low / nits (informational, не блокирующее)

_price_cell() в layout_tz_pdf.py использует :,.0f форматирование + .replace(",", " ") — стандартный Python idiom для thousand separator. Альтернатива через babel.numbers.format_decimal(locale='ru') была бы cleaner, но требует extra dep. Существующий подход OK для PDF гена.

Cross-file impact

parcels.py import добавлен (from app.services.exporters.layout_tz_pdf import render_layout_tz_pdf) — service module exists, signature совпадает ✅
Frontend types в types/best-layouts.ts совпадают с Pydantic schemas в schemas/parcel.py ✅
MarketTab.tsx integration — <BestLayoutsBlock cadNum={data.cad_num} /> без props mismatch ✅

Pre-flight

Branch: feat/113-pdf-tz-and-ui-v2 ✅ (не main)
Mergeable: true, no conflicts ✅
Fixup commit clean — no --amend / --no-verify traces ✅
Verification gpg.error.not_signed_commit — это default для repo (предыдущие коммиты тоже unsigned), не блокер

Scope / merge action

Auto-merge BLOCKED per policy: PR touches backend/app/api/v1/parcels.py + backend/app/services/exporters/** — оба в blocked-list.
Decision передан main session / human reviewer.

Risk / blast radius

Risk: Low — нет миграций, нет breaking API changes, PDF endpoint additive
Reversibility: Easy revert (single PR, no DB state)
Recommended merge window: anytime

Approved for merge by human / main session.

## Deep Code Review — re-verdict (after fixup) ### Summary - **Status**: ✅ APPROVE - **SHA**: `434341e` (was `5878596`) - **Fixup**: +11 / -5 across 3 files - **Counts**: 0🔴 / 0🟠 / 0🟡 / 1🟢 (informational) ### Previous 🟡 items — all addressed | # | Item | Status | Evidence | |---|---|---|---| | 1 | PDF endpoint 404 vs 500 для missing parcel | ✅ FIXED | `parcels.py` теперь ловит `ValueError → 404` перед catch-all `Exception → 500`. Consistent с JSON `/best-layouts` endpoint. | | 2 | XSS escape на table cells (`room_bucket`/`area_bin`) | ✅ FIXED | `layout_tz_pdf.py` — `_html.escape()` добавлен в top_rows на `r.room_bucket` + `r.area_bin`, и в mix_rows на `m.room_bucket`. `signature` в HTML не рендерится (только в TS types) → escape не требуется. | | 3 | ru-locale date formatting (`dd.MM.yyyy`) | ✅ FIXED | `BestLayoutsBlock.tsx` — `new Date(rec.data_window_start).toLocaleDateString("ru-RU")` для start + end. | | 4 | `target_total_flats` client clamp | ✅ FIXED | `buildRequest()` — `Math.min(Math.max(parsed, 1), 10000)`. Backend Pydantic `le=10000` остаётся как defense-in-depth. | ### New issues introduced — none 🔴/🟠/🟡 Reviewed +11 lines line-by-line: - `_html.escape(r.room_bucket/area_bin, m.room_bucket)` — values typed как str enum-ish в Pydantic schemas, escape safe ✅ - `except ValueError → 404` ordering правильный: `HTTPException` first (re-raise 4xx from inner), затем `ValueError`, затем catch-all ✅ - `Math.min(Math.max(parsed, 1), 10000)` — корректно clamps paste-attacks (`99999999` → 10000, negative → 1). NaN guarded upstream через `Number.isNaN(parsed)` ✅ - `toLocaleDateString("ru-RU")` — runs корректно в Node SSR + browser, рендерит формат `dd.MM.yyyy` ✅ ### 🟢 Low / nits (informational, не блокирующее) - `_price_cell()` в `layout_tz_pdf.py` использует `:,.0f` форматирование + `.replace(",", " ")` — стандартный Python idiom для thousand separator. Альтернатива через `babel.numbers.format_decimal(locale='ru')` была бы cleaner, но требует extra dep. Существующий подход OK для PDF гена. ### Cross-file impact - `parcels.py` import добавлен (`from app.services.exporters.layout_tz_pdf import render_layout_tz_pdf`) — service module exists, signature совпадает ✅ - Frontend types в `types/best-layouts.ts` совпадают с Pydantic schemas в `schemas/parcel.py` ✅ - `MarketTab.tsx` integration — `<BestLayoutsBlock cadNum={data.cad_num} />` без props mismatch ✅ ### Pre-flight - Branch: `feat/113-pdf-tz-and-ui-v2` ✅ (не main) - Mergeable: `true`, no conflicts ✅ - Fixup commit clean — no `--amend` / `--no-verify` traces ✅ - Verification `gpg.error.not_signed_commit` — это default для repo (предыдущие коммиты тоже unsigned), не блокер ### Scope / merge action - **Auto-merge BLOCKED per policy**: PR touches `backend/app/api/v1/parcels.py` + `backend/app/services/exporters/**` — оба в blocked-list. - Decision передан main session / human reviewer. ### Risk / blast radius - **Risk**: Low — нет миграций, нет breaking API changes, PDF endpoint additive - **Reversibility**: Easy revert (single PR, no DB state) - **Recommended merge window**: anytime Approved for merge by human / main session.