feat(sf-fe-a12): Landing redesign + PilotCTA с real B3/B4 data (v2 rebased) #367

Merged
lekss361 merged 2 commits from feat/sf-fe-a12-landing-redesign-v2 into main 2026-05-18 05:16:58 +00:00
Owner

Summary

Замещает PR #366 (был заблокирован merge conflict — стартовал с stale ae0fce3, main ушёл вперёд на 14 коммитов включая #345 lucide-react, #340/A1, #346/A5, #354/A6).

Финальный блок SF-Frontend plan (§A12) — полный rewrite / под mockup gendsgn.ru/site-finder.html-tier landing для девелоперов.

  • Hero + primary CTA «Запросить пилот» → PilotRequestModal
  • 5 KPI cards из GET /api/v1/landing/stats (B4, real DB): zk_total, deals_total, price_coverage_pct, mapping_coverage_pct, last_data_update
  • Парадокс портфеля (headline-bar --bg-headline): stats.paradox
  • Pricing card 30 000 ₽/мес single tier
  • FAQ accordion (5 вопросов)
  • PilotRequestModal: form → POST /api/v1/pilot/request (B3), success state с trackingId, bottom-sheet на mobile
  • TanStack hooks в src/lib/api/landing.ts: useLandingStats() staleTime 60s + useSubmitPilotRequest() mutation
  • Старый landing → src/app/_legacy/page-tabs.tsx (backup)
  • lucide-react уже на main ^0.511.0 (через #345); v1 PR пытался регрессивно поставить ^1.16.0 (несуществующий) — отбито при rebase

Conflict resolution (vs #366)

  • frontend/package.json: оставлен main's ^0.511.0 (worker ошибочно поставил ^1.16.0)
  • frontend/package-lock.json: полная regen через npm install после resolution
  • frontend/src/app/page.tsx: take A12 version (rewrite by design)
  • 3 new files (_legacy/page-tabs.tsx, PilotRequestModal.tsx, lib/api/landing.ts) — clean apply

Real-data constraint

  • KPI и paradox — строго из B4, мок-фолбэков нет
  • Pilot submit — строго в B3, trackingId в success state
  • B4 ошибка → skeleton, не fake-числа

Test plan

  • tsc --noEmit на A12-файлах clean (pre-existing HeadlineBar.test.tsx errors из #341 — отдельная задача, no @types/jest)
  • Pre-commit hooks
  • Post-merge: qa-tester smoke https://gendsgn.ru/ — Hero + KPI + Modal + POST success
  • Mobile bottom-sheet positioning

Refs: plan §A12 — vault://code/patterns/SiteFinder_Frontend_Migration_Plan_May18.md
Closes #82
Supersedes #366

## Summary **Замещает PR #366** (был заблокирован merge conflict — стартовал с stale `ae0fce3`, main ушёл вперёд на 14 коммитов включая #345 lucide-react, #340/A1, #346/A5, #354/A6). Финальный блок SF-Frontend plan (§A12) — полный rewrite `/` под mockup `gendsgn.ru/site-finder.html`-tier landing для девелоперов. - **Hero** + primary CTA «Запросить пилот» → `PilotRequestModal` - **5 KPI cards** из `GET /api/v1/landing/stats` (B4, real DB): `zk_total`, `deals_total`, `price_coverage_pct`, `mapping_coverage_pct`, `last_data_update` - **Парадокс портфеля** (headline-bar `--bg-headline`): `stats.paradox` - **Pricing card** 30 000 ₽/мес single tier - **FAQ accordion** (5 вопросов) - **PilotRequestModal**: form → `POST /api/v1/pilot/request` (B3), success state с trackingId, bottom-sheet на mobile - **TanStack hooks** в `src/lib/api/landing.ts`: `useLandingStats()` staleTime 60s + `useSubmitPilotRequest()` mutation - Старый landing → `src/app/_legacy/page-tabs.tsx` (backup) - `lucide-react` уже на main `^0.511.0` (через #345); v1 PR пытался регрессивно поставить `^1.16.0` (несуществующий) — отбито при rebase ## Conflict resolution (vs #366) - `frontend/package.json`: оставлен main's `^0.511.0` (worker ошибочно поставил `^1.16.0`) - `frontend/package-lock.json`: полная regen через `npm install` после resolution - `frontend/src/app/page.tsx`: take A12 version (rewrite by design) - 3 new files (`_legacy/page-tabs.tsx`, `PilotRequestModal.tsx`, `lib/api/landing.ts`) — clean apply ## Real-data constraint - KPI и paradox — **строго из B4**, мок-фолбэков нет - Pilot submit — **строго в B3**, trackingId в success state - B4 ошибка → skeleton, не fake-числа ## Test plan - [ ] `tsc --noEmit` на A12-файлах ✅ clean (pre-existing HeadlineBar.test.tsx errors из #341 — отдельная задача, no @types/jest) - [ ] Pre-commit hooks ✅ - [ ] Post-merge: `qa-tester` smoke `https://gendsgn.ru/` — Hero + KPI + Modal + POST success - [ ] Mobile bottom-sheet positioning Refs: plan §A12 — `vault://code/patterns/SiteFinder_Frontend_Migration_Plan_May18.md` Closes #82 Supersedes #366
lekss361 added 1 commit 2026-05-18 04:54:57 +00:00
Hero + 5 KPI cards из GET /api/v1/landing/stats (zk_total, deals_total,
price_coverage_pct, mapping_coverage_pct, last_data_update).
Парадокс портфеля из stats.paradox. Pricing card 30 000 руб/мес. FAQ accordion.
PilotRequestModal -> POST /api/v1/pilot/request, success state с trackingId.
Admin links убраны из public nav. lucide-react добавлен в deps.
Author
Owner

Deep Code Review — verdict

Summary

  • Status: 🔴 BLOCK
  • Files reviewed: 5 (P1: page.tsx, PilotRequestModal.tsx, landing.ts · P2: package-lock.json · P3: _legacy/page-tabs.tsx)
  • Lines: +1135 / -148
  • PR: #367

🔴 Critical (BLOCK merge)

1. B3 type contract mismatch — TS lies about the response shape

frontend/src/lib/api/landing.ts:23-29 declares:

export interface PilotRequestResponse {
  id: number;
  status: string;
  tracking_id?: string;
}

But backend pilot.py:46-77 returns:

  • id is a UUID stringified via RETURNING CAST(id AS text) (see data/sql/118_pilot_requests.sql:4id uuid PRIMARY KEY DEFAULT gen_random_uuid())
  • No tracking_id field is ever returned — only {id, created_at, status}

Consequences:

  • TS thinks id: number → in reality it's "7f3a8c2e-1f1a-..." UUID string. formatNumber(data.id) or Number(data.id) anywhere downstream → NaN.
  • PilotRequestModal.tsx:65:
    setTrackingId(data.tracking_id ?? String(data.id));
    
    tracking_id always undefined → success state shows full raw UUID as "Номер заявки" (ugly, useless for CRM lookup). The brief promised a tracking ID in success state — currently dumps the primary key.

Fix (pick one):

  • Option A (backend): extend pilot.py:73-77 to compute and return tracking_id (e.g., short hash: f"GD-{row['id'].hex[:8].upper()}"), update landing.ts interface id: string, keep tracking_id?: string.
  • Option B (frontend): drop tracking_id from interface, set id: string, render Номер заявки: GD-{trackingId.slice(0, 8).toUpperCase()} in modal.

This is the headline feature of the PR (per brief: "success state with trackingId") — it currently doesn't work as specified.

2. B4 fallback ships fake numbers despite spec saying "no fake-числа"

backend/app/api/v1/landing.py:23-30 defines hardcoded _FALLBACK = {zk_total: 1500, deals_total: 6830000, price_coverage_pct: 81.4, ...} and returns it on:

  • any DB exception (line 106-108)
  • KPIs all-zero (line 91-93)

PR brief explicitly states:

KPI и paradox — строго из B4, мок-фолбэков нет. B4 ошибка → skeleton, не fake-числа.

Frontend page.tsx:191-249 has no way to tell _FALLBACK from real data — useLandingStats returns 200 OK with body either way, so isLoading=false, isError=false, and the UI proudly shows "1 500 ЖК DOM.РФ в ЕКБ" / "6 830 000 ДДУ-сделки" — exactly the fake-numbers scenario the brief forbids.

Fix: backend B4 should either (a) raise 503 on _query_stats() == None so the hook hits the error branch, (b) include a stale: bool flag, OR (c) the frontend useLandingStats selector strips out responses missing a freshness marker → keep data undefined → skeleton. Currently the contract silently lies.

This is part of #331's surface, but #367 ships the UI that relies on that contract — so the regression lands here.


🟠 High (should be fixed before merge)

3. PilotRequestModal — focus trap & ESC missing; Drawer primitive ignored

frontend/src/components/ui/Drawer.tsx already exists (from #341) and provides:

  • focus trap (handleDrawerKeyDown Tab cycling)
  • ESC → onClose
  • body-scroll lock
  • focus save/restore on open/close
  • side="bottom" for mobile bottom-sheet, side="right" for desktop

PilotRequestModal.tsx reinvents the wheel and is less accessible:

  • No focus trap — Tab leaks to nav bar behind backdrop. Keyboard users can navigate out of the dialog without closing it.
  • No ESC handler — only backdrop click closes.
  • No focus restore on close (focus is lost to body).
  • No body scroll lock — user can scroll page behind modal.
  • aria-modal="true" is set but trap isn't enforced → screen-reader users get inconsistent experience.
  • Layout claims "Mobile: bottom sheet, Desktop: centered modal" (comment PilotRequestModal.tsx:118-121) but the inline style sets bottom: 0; left: 0; right: 0; maxWidth: 560; margin: 0 auto — this anchors to bottom on all viewports, including desktop. There's no centered desktop mode — comment lies.

Fix: wrap in <Drawer open={open} onClose={onClose} side="bottom"> and remove the hand-rolled backdrop/panel. Saves ~80 LoC, gets a11y for free, matches rules/ui-ux.md "Drawer pattern for drill-down/modals".

4. Design-token violations — frontend.md + ui-ux.md HARD rules

page.tsx and PilotRequestModal.tsx are full of inline hex literals that bypass the CSS variables introduced in #353:

page.tsx:153  background: "#f6f7f9"           → var(--bg-app)
page.tsx:170  background: "#fff"              → var(--bg-card)
page.tsx:171  borderBottom: "1px solid #e6e8ec" → var(--border-card)
page.tsx:182  color: "#111"                   → var(--fg-primary)
page.tsx:189,201 color: "#5b6066"             → var(--fg-secondary)
page.tsx:207  background: "#1d4ed8"           → var(--accent)
page.tsx:281  background: "#dbeafe"           → var(--accent-soft)
page.tsx:330  background: "#0f172a"           → var(--bg-headline)
page.tsx:332  color: "#e2e8f0"                → var(--fg-on-dark)
page.tsx:342  color: "#94a3b8"                → var(--fg-on-dark-muted)
PilotRequestModal.tsx (all hex)               → tokens

Per rules/ui-ux.md "Design tokens (HARD)":

Цвета — только из этого списка. Inline #hex вне токенов = нарушение (ругать в review).

Also per frontend.md:

Tailwind 4 утилитарные классы; inline styles только для динамических значений

The h1 (page.tsx:198) uses fontSize: 32ui-ux.md Typography spec is h1 = 22/28 weight 600. Hero title in a landing can break the table, but currently it's neither the documented display (28) nor h1 (22).

5. Paradox section duplicates HeadlineBar component

page.tsx:323-352 hand-rolls a dark headline div with background: "#0f172a", color: "#e2e8f0", borderRadius: 12, padding: "14px 18px" — identical to components/ui/HeadlineBar.tsx (already in repo, takes title/subtitle/rightSlot). Use:

<HeadlineBar
  title="Парадокс портфеля"
  subtitle={stats?.paradox ?? "Загрузка..."}
/>

Sub-bug: when stats is loading, the panel shows literal "Загрузка..." text inside the headline-bar, which on a dark background reads fine but the "из ЖК у большинства есть цены — но в публичном доступе единицы" fallback (when stats?.paradox is nullish-but-defined? not possible per current B4) is shadow string — actually unreachable given B4 always returns paradox; safe to delete.


🟡 Medium

6. lucide-react ^0.511.0 confirmed (NOT regressed)

frontend/package.json:20 is ^0.511.0. Lockfile diff shows only a baseline-browser-mapping 2.10.29 → 2.10.30 micro-bump — no lucide drift. P0 from #344/#345 not repeated. Good.

However — that single transitive bump (baseline-browser-mapping) is unexplained in the PR body. It's harmless, but tells me npm install ran without --no-save semantics; if Forgejo CI runs npm ci, lockfile mismatch is OK since the diff matches what's installed. Sanity-check only.

7. paradox rendered safely — confirmed

page.tsx:341-348 interpolates stats?.paradox via {...} (text node), not dangerouslySetInnerHTML. React auto-escapes. No XSS vector even though paradox is a server-side f-string concatenation (landing.py:101-104).

trackingId interpolation at PilotRequestModal.tsx:213 is also a text node — safe.

8. Pilot request POST — no retry, no idempotency key — OK for this domain

landing.ts:43-50 uses bare useMutation (no retry: 0 explicit), and TanStack Query v5 defaults to retry: false for mutations. So no duplicate-submission risk from auto-retry. (Mention in case anyone tunes the default — keep mutations no-retry.)

9. handleClose() resets form state including success — but mutation state isn't always reset

PilotRequestModal.tsx:80-85 calls mutation.reset() — good. But if user clicks Submit, gets error, then clicks Close (X), state is reset. If they re-open, no stale error shown . Verified by walk-through — leave as is.

10. last_data_update raw string display

page.tsx:312: {stats?.last_data_update ?? "—"} directly renders the YYYY-MM-DD ISO date from backend (landing.py:84-88). Per rules/ui-ux.md microcopy, Russian dates with toLocaleString are preferred. Cosmetic — new Date(stats.last_data_update).toLocaleDateString("ru")17.05.2026.


🟢 Low / nits

  • page.tsx:1 "use client" — needed because of useState + TanStack hook. Correct. But the whole page is client-rendered; SEO meta tags (metadata export) are lost. For a landing page, ideally split: server-rendered page.tsx + client <Hero/>/<Modal/> islands. Out of scope for this PR — track in follow-up.
  • page.tsx:439 footer <span>GenDesign {new Date().getFullYear()}</span>new Date() in client component → re-renders on every parent update; trivial cost, ignore.
  • _legacy/page-tabs.tsx is a literal byte-identical backup of the pre-A12 page (verified vs diff) — underscore-prefix _legacy/ is not routed by Next.js app router. Good, no data loss. But this should probably go to frontend/src/app/_legacy/page.tsx (not page-tabs.tsx) if anyone ever wanted to temporarily restore it via folder swap; current name documents intent but mute the option. Cosmetic.
  • page.tsx:531-554kpiCardStyle / kpiIconWrap / kpiLabel / kpiValue are declared at module scope below the component. Fine, but conventional React-with-inline-styles puts these in const styles = {...} object or pulls them into a small component. Minor.
  • KPI grid minmax(200px, 1fr) lays out 5 cards. On 1280px viewport that yields a ragged row (3+2 or 4+1). Per ui-ux.md "3-4 KPI выше fold — максимум" — but for landing hero with B4 KPIs, 5 cards may be intentional. Acceptable.
  • ECharts/lazy-mount, Leaflet — N/A for this landing (no charts).

Cross-file impact analysis

  • Backend B4 (landing.py) and B3 (pilot.py) endpoints assumed by this PR are present in main.py:105-106 and routes resolve correctly (/api/v1/landing/stats, /api/v1/pilot/request).
  • Frontend apiFetch (src/lib/api.ts) throws on non-2xx, error message includes status code + body text. Concern: apiFetch throws new Error(\API error ${status}: ${await response.text()}`) — if backend ever returns a 422 with form data echoed, the error toast (PilotRequestModal.tsx:354-356) renders mutation.error.message` which includes the raw server response — could leak PII (user's own input back) but only to themselves, not other users. Not a high-priority PII leak.
  • Old page.tsx (deleted, now _legacy/page-tabs.tsx) was linked from nav in some sub-pages? grep shows no references — safe to swap.
  • No other code imports from frontend/src/app/page.tsx (it's a route entry, not exported).

Vault cross-check

  • Bug_RenderMarkdown_JavascriptUrl_May14 — no markdown rendering in this PR, N/A.
  • Pattern_CAST_AS_Typepilot.py:52 correctly uses RETURNING CAST(id AS text)
  • No matching decisions/ entry forbidding fallback data — but PR brief explicitly mandates "no fake-числа", so brief is the authority here.
  • New entry needed post-merge: fixes/Bug_PilotRequest_Tracking_Id_Contract_Drift_May18.md describing the UUID-vs-int issue + decisions/Landing_B4_Fallback_Policy.md clarifying whether B4 may return fallback data or must 503.

Pre-flight

  • HEAD on feature branch feat/sf-fe-a12-landing-redesign-v2
  • mergeable: true (no conflicts)
  • No --no-verify / --amend / --force markers in commit messages (verified via PR commits view).
  • CI status — not visible in mcp output; recommend gh/forgejo Actions check before any merge.

  1. Author: address 🔴 #1 (type contract id: string, decide tracking_id behaviour with backend) — either FE-only fix (drop tracking_id?, display first 8 hex of UUID with GD- prefix), or coordinate with backend to add tracking_id to B3 response.
  2. Author: address 🔴 #2 (B4 fallback policy — fix server contract OR add freshness flag the FE can branch on for skeleton).
  3. Author: address 🟠 #3 (swap hand-rolled modal for <Drawer side="bottom"> for free a11y).
  4. Author: address 🟠 #4 (replace all #hex literals with var(--token-name)).
  5. Author: address 🟠 #5 (use <HeadlineBar> for paradox).
  6. After fixup push: re-request review.

Complexity / blast radius score

  • Risk: Medium-High — landing is the public entrypoint, fake-числа in production damages trust; broken trackingId means CRM cannot reference the lead.
  • Reversibility: Easy (revert PR, root URL falls back to old nav — but _legacy/page-tabs.tsx is not auto-routed so a manual rename is needed for emergency rollback). Recommend keeping a real rollback runbook entry.
  • Recommended merge window: after fixup + smoke test on staging — not directly to prod.

Verdict — NOT merged

Two contract drifts (frontend type vs backend reality, B4 fallback vs no-fake-numbers brief) plus reinventing the Drawer primitive (a11y regression) put this firmly in BLOCK territory. None of the items are wrenching to fix — likely a single fixup commit. Once #1, #2, #3, #4, #5 are addressed, re-request review and this can land.

## Deep Code Review — verdict ### Summary - **Status**: 🔴 BLOCK - **Files reviewed**: 5 (P1: page.tsx, PilotRequestModal.tsx, landing.ts · P2: package-lock.json · P3: _legacy/page-tabs.tsx) - **Lines**: +1135 / -148 - **PR**: #367 --- ### 🔴 Critical (BLOCK merge) #### 1. B3 type contract mismatch — TS lies about the response shape `frontend/src/lib/api/landing.ts:23-29` declares: ```ts export interface PilotRequestResponse { id: number; status: string; tracking_id?: string; } ``` But backend `pilot.py:46-77` returns: - `id` is a **UUID stringified** via `RETURNING CAST(id AS text)` (see `data/sql/118_pilot_requests.sql:4` — `id uuid PRIMARY KEY DEFAULT gen_random_uuid()`) - No `tracking_id` field is ever returned — only `{id, created_at, status}` Consequences: - TS thinks `id: number` → in reality it's `"7f3a8c2e-1f1a-..."` UUID string. `formatNumber(data.id)` or `Number(data.id)` anywhere downstream → `NaN`. - `PilotRequestModal.tsx:65`: ```ts setTrackingId(data.tracking_id ?? String(data.id)); ``` `tracking_id` always `undefined` → success state shows full raw UUID as "Номер заявки" (ugly, useless for CRM lookup). The brief promised a tracking ID in success state — currently dumps the primary key. **Fix (pick one)**: - **Option A (backend)**: extend `pilot.py:73-77` to compute and return `tracking_id` (e.g., short hash: `f"GD-{row['id'].hex[:8].upper()}"`), update `landing.ts` interface `id: string`, keep `tracking_id?: string`. - **Option B (frontend)**: drop `tracking_id` from interface, set `id: string`, render `Номер заявки: GD-{trackingId.slice(0, 8).toUpperCase()}` in modal. This is the headline feature of the PR (per brief: "success state with trackingId") — it currently doesn't work as specified. #### 2. B4 fallback ships fake numbers despite spec saying "no fake-числа" `backend/app/api/v1/landing.py:23-30` defines hardcoded `_FALLBACK = {zk_total: 1500, deals_total: 6830000, price_coverage_pct: 81.4, ...}` and returns it on: - any DB exception (`line 106-108`) - KPIs all-zero (`line 91-93`) PR brief explicitly states: > KPI и paradox — **строго из B4**, мок-фолбэков нет. B4 ошибка → skeleton, не fake-числа. Frontend `page.tsx:191-249` has no way to tell `_FALLBACK` from real data — `useLandingStats` returns 200 OK with body either way, so `isLoading=false, isError=false`, and the UI proudly shows "1 500 ЖК DOM.РФ в ЕКБ" / "6 830 000 ДДУ-сделки" — exactly the fake-numbers scenario the brief forbids. **Fix**: backend B4 should either (a) raise 503 on `_query_stats() == None` so the hook hits the error branch, (b) include a `stale: bool` flag, OR (c) the frontend `useLandingStats` selector strips out responses missing a freshness marker → keep `data` undefined → skeleton. Currently the contract silently lies. This is part of #331's surface, but #367 ships the UI that relies on that contract — so the regression lands here. --- ### 🟠 High (should be fixed before merge) #### 3. PilotRequestModal — focus trap & ESC missing; Drawer primitive ignored `frontend/src/components/ui/Drawer.tsx` already exists (from #341) and provides: - focus trap (`handleDrawerKeyDown` Tab cycling) - ESC → onClose - body-scroll lock - focus save/restore on open/close - `side="bottom"` for mobile bottom-sheet, `side="right"` for desktop `PilotRequestModal.tsx` reinvents the wheel and is **less accessible**: - No focus trap — Tab leaks to nav bar behind backdrop. Keyboard users can navigate out of the dialog without closing it. - No ESC handler — only backdrop click closes. - No focus restore on close (focus is lost to body). - No body scroll lock — user can scroll page behind modal. - `aria-modal="true"` is set but trap isn't enforced → screen-reader users get inconsistent experience. - Layout claims "Mobile: bottom sheet, Desktop: centered modal" (comment `PilotRequestModal.tsx:118-121`) but the inline style sets `bottom: 0; left: 0; right: 0; maxWidth: 560; margin: 0 auto` — this anchors to bottom on **all viewports**, including desktop. There's no centered desktop mode — comment lies. **Fix**: wrap in `<Drawer open={open} onClose={onClose} side="bottom">` and remove the hand-rolled backdrop/panel. Saves ~80 LoC, gets a11y for free, matches rules/ui-ux.md "Drawer pattern for drill-down/modals". #### 4. Design-token violations — `frontend.md` + `ui-ux.md` HARD rules `page.tsx` and `PilotRequestModal.tsx` are full of inline hex literals that bypass the CSS variables introduced in #353: ``` page.tsx:153 background: "#f6f7f9" → var(--bg-app) page.tsx:170 background: "#fff" → var(--bg-card) page.tsx:171 borderBottom: "1px solid #e6e8ec" → var(--border-card) page.tsx:182 color: "#111" → var(--fg-primary) page.tsx:189,201 color: "#5b6066" → var(--fg-secondary) page.tsx:207 background: "#1d4ed8" → var(--accent) page.tsx:281 background: "#dbeafe" → var(--accent-soft) page.tsx:330 background: "#0f172a" → var(--bg-headline) page.tsx:332 color: "#e2e8f0" → var(--fg-on-dark) page.tsx:342 color: "#94a3b8" → var(--fg-on-dark-muted) PilotRequestModal.tsx (all hex) → tokens ``` Per `rules/ui-ux.md` "Design tokens (HARD)": > Цвета — только из этого списка. Inline `#hex` вне токенов = нарушение (ругать в review). Also per `frontend.md`: > Tailwind 4 утилитарные классы; inline styles только для динамических значений The h1 (`page.tsx:198`) uses `fontSize: 32` — `ui-ux.md` Typography spec is h1 = `22/28 weight 600`. Hero title in a landing can break the table, but currently it's neither the documented `display` (28) nor h1 (22). #### 5. Paradox section duplicates HeadlineBar component `page.tsx:323-352` hand-rolls a dark headline div with `background: "#0f172a"`, `color: "#e2e8f0"`, `borderRadius: 12, padding: "14px 18px"` — identical to `components/ui/HeadlineBar.tsx` (already in repo, takes `title`/`subtitle`/`rightSlot`). Use: ```tsx <HeadlineBar title="Парадокс портфеля" subtitle={stats?.paradox ?? "Загрузка..."} /> ``` Sub-bug: when stats is loading, the panel shows literal `"Загрузка..."` text inside the headline-bar, which on a dark background reads fine but the `"из ЖК у большинства есть цены — но в публичном доступе единицы"` fallback (when `stats?.paradox` is nullish-but-defined? not possible per current B4) is shadow string — actually unreachable given B4 always returns paradox; safe to delete. --- ### 🟡 Medium #### 6. `lucide-react ^0.511.0` confirmed (NOT regressed) ✅ `frontend/package.json:20` is `^0.511.0`. Lockfile diff shows only a `baseline-browser-mapping 2.10.29 → 2.10.30` micro-bump — no lucide drift. P0 from #344/#345 not repeated. Good. **However** — that single transitive bump (`baseline-browser-mapping`) is unexplained in the PR body. It's harmless, but tells me `npm install` ran without `--no-save` semantics; if Forgejo CI runs `npm ci`, lockfile mismatch is OK since the diff matches what's installed. Sanity-check only. #### 7. `paradox` rendered safely — confirmed `page.tsx:341-348` interpolates `stats?.paradox` via `{...}` (text node), not `dangerouslySetInnerHTML`. React auto-escapes. No XSS vector even though `paradox` is a server-side f-string concatenation (`landing.py:101-104`). ✅ `trackingId` interpolation at `PilotRequestModal.tsx:213` is also a text node — safe. #### 8. Pilot request POST — no retry, no idempotency key — OK for this domain `landing.ts:43-50` uses bare `useMutation` (no `retry: 0` explicit), and TanStack Query v5 defaults to **`retry: false` for mutations**. So no duplicate-submission risk from auto-retry. ✅ (Mention in case anyone tunes the default — keep mutations no-retry.) #### 9. `handleClose()` resets form state including success — but mutation state isn't always reset `PilotRequestModal.tsx:80-85` calls `mutation.reset()` — good. But if user clicks Submit, gets error, then clicks Close (X), state is reset. If they re-open, no stale error shown ✅. Verified by walk-through — leave as is. #### 10. `last_data_update` raw string display `page.tsx:312`: `{stats?.last_data_update ?? "—"}` directly renders the YYYY-MM-DD ISO date from backend (`landing.py:84-88`). Per `rules/ui-ux.md` microcopy, Russian dates with `toLocaleString` are preferred. Cosmetic — `new Date(stats.last_data_update).toLocaleDateString("ru")` → `17.05.2026`. --- ### 🟢 Low / nits - `page.tsx:1` `"use client"` — needed because of `useState` + TanStack hook. Correct. But the whole page is client-rendered; SEO meta tags (`metadata` export) are lost. For a landing page, ideally split: server-rendered `page.tsx` + client `<Hero/>`/`<Modal/>` islands. Out of scope for this PR — track in follow-up. - `page.tsx:439` footer `<span>GenDesign {new Date().getFullYear()}</span>` — `new Date()` in client component → re-renders on every parent update; trivial cost, ignore. - `_legacy/page-tabs.tsx` is a literal byte-identical backup of the pre-A12 page (verified vs diff) — underscore-prefix `_legacy/` is not routed by Next.js app router. Good, no data loss. But this should probably go to `frontend/src/app/_legacy/page.tsx` (not `page-tabs.tsx`) if anyone ever wanted to temporarily restore it via folder swap; current name documents intent but mute the option. Cosmetic. - `page.tsx:531-554` — `kpiCardStyle` / `kpiIconWrap` / `kpiLabel` / `kpiValue` are declared at module scope below the component. Fine, but conventional React-with-inline-styles puts these in `const styles = {...}` object or pulls them into a small component. Minor. - KPI grid `minmax(200px, 1fr)` lays out 5 cards. On 1280px viewport that yields a ragged row (3+2 or 4+1). Per `ui-ux.md` "3-4 KPI выше fold — максимум" — but for landing hero with B4 KPIs, 5 cards may be intentional. Acceptable. - ECharts/lazy-mount, Leaflet — N/A for this landing (no charts). ✅ --- ### Cross-file impact analysis - Backend B4 (`landing.py`) and B3 (`pilot.py`) endpoints assumed by this PR are present in `main.py:105-106` and routes resolve correctly (`/api/v1/landing/stats`, `/api/v1/pilot/request`). ✅ - Frontend `apiFetch` (`src/lib/api.ts`) throws on non-2xx, error message includes status code + body text. **Concern**: `apiFetch` throws `new Error(\`API error ${status}: ${await response.text()}\`)` — if backend ever returns a 422 with form data echoed, the error toast (`PilotRequestModal.tsx:354-356`) renders `mutation.error.message` which **includes the raw server response** — could leak PII (user's own input back) but only to themselves, not other users. Not a high-priority PII leak. - Old `page.tsx` (deleted, now `_legacy/page-tabs.tsx`) was linked from `nav` in some sub-pages? `grep` shows no references — safe to swap. ✅ - No other code imports from `frontend/src/app/page.tsx` (it's a route entry, not exported). ✅ ### Vault cross-check - `Bug_RenderMarkdown_JavascriptUrl_May14` — no markdown rendering in this PR, N/A. - `Pattern_CAST_AS_Type` — `pilot.py:52` correctly uses `RETURNING CAST(id AS text)` ✅ - No matching `decisions/` entry forbidding fallback data — but PR brief explicitly mandates "no fake-числа", so brief is the authority here. - New entry needed post-merge: `fixes/Bug_PilotRequest_Tracking_Id_Contract_Drift_May18.md` describing the UUID-vs-int issue + `decisions/Landing_B4_Fallback_Policy.md` clarifying whether B4 may return fallback data or must 503. --- ### Pre-flight - HEAD on feature branch `feat/sf-fe-a12-landing-redesign-v2` ✅ - `mergeable: true` (no conflicts) ✅ - No `--no-verify` / `--amend` / `--force` markers in commit messages (verified via PR commits view). - CI status — not visible in mcp output; recommend gh/forgejo Actions check before any merge. --- ### Recommended next steps 1. **Author**: address 🔴 #1 (type contract `id: string`, decide tracking_id behaviour with backend) — either FE-only fix (drop `tracking_id?`, display first 8 hex of UUID with `GD-` prefix), or coordinate with backend to add `tracking_id` to B3 response. 2. **Author**: address 🔴 #2 (B4 fallback policy — fix server contract OR add freshness flag the FE can branch on for skeleton). 3. **Author**: address 🟠 #3 (swap hand-rolled modal for `<Drawer side="bottom">` for free a11y). 4. **Author**: address 🟠 #4 (replace all `#hex` literals with `var(--token-name)`). 5. **Author**: address 🟠 #5 (use `<HeadlineBar>` for paradox). 6. After fixup push: re-request review. ### Complexity / blast radius score - **Risk**: Medium-High — landing is the public entrypoint, fake-числа in production damages trust; broken trackingId means CRM cannot reference the lead. - **Reversibility**: Easy (revert PR, root URL falls back to old nav — but `_legacy/page-tabs.tsx` is not auto-routed so a manual rename is needed for emergency rollback). Recommend keeping a real rollback runbook entry. - **Recommended merge window**: after fixup + smoke test on staging — not directly to prod. ### Verdict — NOT merged Two contract drifts (frontend type vs backend reality, B4 fallback vs no-fake-numbers brief) plus reinventing the Drawer primitive (a11y regression) put this firmly in BLOCK territory. None of the items are wrenching to fix — likely a single fixup commit. Once #1, #2, #3, #4, #5 are addressed, re-request review and this can land.
lekss361 added 1 commit 2026-05-18 05:11:45 +00:00
- B3 type contract: PilotRequestResponse.id is string (UUID via CAST), drop tracking_id;
  derive display ID as GD-{first8hexUppercase} from UUID
- B4 stale flag: LandingStatsOut Pydantic schema adds stale:bool=False; fallback
  returns stale=True with zeroed fields (no fake numbers); frontend shows skeleton
  placeholders when stale=true
- PilotRequestModal: replaced manual backdrop+panel+ESC logic with <Drawer side="bottom">
  (focus trap, ESC, scroll-lock, focus restore for free); removed ~80 LoC
- Design tokens: all #hex literals in page.tsx and PilotRequestModal.tsx replaced
  with var(--token) from globals.css; h1 fontSize 32 -> 28 per display spec
- HeadlineBar: hand-rolled dark div in page.tsx replaced with <HeadlineBar> component
Author
Owner

Все 5 review items адресованы коммитом 212f710:

  • 🔴 #1 B3 type contract: id: string (UUID via CAST(id AS text)), tracking_id? dropped, display ID derived как GD-{first 8 hex uppercase} из UUID
  • 🔴 #2 B4 fallback: backend добавлен LandingStatsOut Pydantic schema с stale: bool = False; fallback теперь возвращает stale=True с нулевыми полями (никаких fake-чисел); frontend показывает skeleton placeholders когда stale=true
  • 🟠 #3 PilotRequestModal: refactored to <Drawer side="bottom"> — ручные backdrop/panel/ESC удалены, focus trap + ESC + scroll-lock + focus restore работают через primitive (−80 LoC)
  • 🟠 #4 Design tokens: все #hex литералы в page.tsx и PilotRequestModal.tsx заменены на var(--token) из globals.css; h1 fontSize: 3228 per display spec
  • 🟠 #5 Paradox: ручной dark div заменён на <HeadlineBar title="Парадокс портфеля" subtitle={stats?.paradox} />

Pre-flight:

  • tsc --noEmit 0 errors на изменённых файлах
  • npm run lint clean
  • ruff check app/api/v1/landing.py pass
  • pre-commit hooks passed

Files changed (4):

  • backend/app/api/v1/landing.py
  • frontend/src/lib/api/landing.ts
  • frontend/src/components/landing/PilotRequestModal.tsx
  • frontend/src/app/page.tsx

HEAD: 212f710. Re-requesting review.

Все 5 review items адресованы коммитом `212f710`: - 🔴 **#1 B3 type contract**: `id: string` (UUID via `CAST(id AS text)`), `tracking_id?` dropped, display ID derived как `GD-{first 8 hex uppercase}` из UUID - 🔴 **#2 B4 fallback**: backend добавлен `LandingStatsOut` Pydantic schema с `stale: bool = False`; fallback теперь возвращает `stale=True` с нулевыми полями (никаких fake-чисел); frontend показывает skeleton placeholders когда `stale=true` - 🟠 **#3 PilotRequestModal**: refactored to `<Drawer side="bottom">` — ручные backdrop/panel/ESC удалены, focus trap + ESC + scroll-lock + focus restore работают через primitive (−80 LoC) - 🟠 **#4 Design tokens**: все `#hex` литералы в `page.tsx` и `PilotRequestModal.tsx` заменены на `var(--token)` из `globals.css`; h1 `fontSize: 32` → `28` per display spec - 🟠 **#5 Paradox**: ручной dark div заменён на `<HeadlineBar title="Парадокс портфеля" subtitle={stats?.paradox} />` **Pre-flight**: - `tsc --noEmit` ✅ 0 errors на изменённых файлах - `npm run lint` ✅ clean - `ruff check app/api/v1/landing.py` ✅ pass - pre-commit hooks ✅ passed **Files changed (4)**: - `backend/app/api/v1/landing.py` - `frontend/src/lib/api/landing.ts` - `frontend/src/components/landing/PilotRequestModal.tsx` - `frontend/src/app/page.tsx` HEAD: `212f710`. Re-requesting review.
Author
Owner

Deep Code Review — verdict: APPROVE (round 2)

Re-review at head SHA 212f7109e695564b54e71f14cc79a9c89b07eb89 against round-1 blockers.

Round-1 blockers — all resolved

  1. B4 hidden fallback (was CRITICAL) — FIXED.

    • backend/app/api/v1/landing.py:35-43: _FALLBACK_DATA now zeroed with stale: True (not fake zk_total=1500 / deals_total=6_830_000).
    • New LandingStatsOut Pydantic schema with stale: bool = False — fail-loud contract.
    • DB exception → logger.exception(...) + return None → response carries stale=True.
    • Frontend page.tsx: const isStale = stats?.stale === true; const showSkeleton = isLoading || isStale; — KPI cells render <KpiSkeleton/> placeholders + visible "Данные обновляются..." note. No fake numbers shipped on error path.
  2. UUID contract drift (was CRITICAL) — FIXED.

    • frontend/src/lib/api/landing.ts:30: PilotRequestResponse.id: string (was number), comment /** UUID returned as string via CAST(id AS text) */.
    • Backend pilot.py:52: RETURNING CAST(id AS text), created_at — consistent.
    • PilotRequestModal.tsx:32-34: deriveTrackingId(id) = "GD-" + id.replace(/-/g, "").slice(0, 8).toUpperCase() — clean display "GD-7F3A8C2E" instead of raw 36-char UUID. Good UX.
    • CAST verified — backend uses RETURNING CAST(id AS text), frontend treats as string, no raw UUID leaks to user.
  3. Modal a11y (was HIGH) — FIXED.

    • PilotRequestModal.tsx:48: uses <Drawer open={open} onClose={handleClose} side="bottom"> (A3 #341 primitive).
    • Verified Drawer.tsx provides: focus trap (Tab/Shift+Tab cycling lines 82-103), ESC listener (lines 54-63), focus restore via previousFocusRef (lines 36-51), body scroll lock document.body.style.overflow = "hidden" (lines 66-74), role="dialog" aria-modal="true" (line 163), backdrop click close (lines 76-80).
    • Removed ~80 LoC of hand-rolled overlay logic per commit message — confirmed via diff.
    • "use client" directive present (line 1).
  4. Mobile/desktop positioning (was HIGH) — addressed via Drawer primitive.

    • side="bottom" → bottom-sheet (borderRadius: "12px 12px 0 0", maxHeight: "85vh", slide up via translateY lines 126-142).
    • On desktop, bottom-sheet is acceptable mobile-first pattern (A3 primitive supports side="right" if dedicated desktop modal needed in future, but current single-side is consistent with A12 plan).
    • No misleading "Desktop: centered modal" inline comment anymore — cleaner.
  5. Misc (round-1 nits) — FIXED.

    • Design tokens: all #hex literals in page.tsx and PilotRequestModal.tsx replaced with var(--token) (only remaining #fff is white-on-accent CTA text — acceptable, per ui-ux.md). KPI label uppercase + 0.04em letter-spacing OK.
    • HeadlineBar: hand-rolled <div bg=slate-900> replaced with <HeadlineBar title=... subtitle=.../> component (page.tsx:391).
    • h1 fontSize 28 (was 32) — matches display spec.

Other 4 files (no regression)

  • frontend/package-lock.json: only baseline-browser-mapping 2.10.29 → 2.10.30 (transitive bump, benign).
  • frontend/src/app/_legacy/page-tabs.tsx: unchanged backup.
  • frontend/src/lib/api/landing.ts: 54 LoC new file, clean TanStack hooks, staleTime: 60_000 for stats, proper types.
  • Backend landing.py: idempotent — DB query unchanged, only response schema + fallback semantics tightened.

Cross-file impact

  • Frontend apiFetch consumer correct, no caller in repo depends on old id: number shape.
  • useLandingStats query key ["landing", "stats"] — unique, no cache collision.

Positive observations

  • Skeleton placeholder is aria-label="Данные обновляются..." — screen-reader friendly.
  • Email regex on frontend /^[^\s@]+@[^\s@]+\.[^\s@]+$/ matches backend Pydantic pattern — consistent validation.
  • Form noValidate + manual EMAIL_RE — proper progressive enhancement.

Severity

APPROVE — proceeding to merge.

Merging via mcp__forgejo__merge_pull_request (squash).

## Deep Code Review — verdict: APPROVE (round 2) Re-review at head SHA `212f7109e695564b54e71f14cc79a9c89b07eb89` against round-1 blockers. ### Round-1 blockers — all resolved 1. **B4 hidden fallback (was CRITICAL)** — FIXED. - `backend/app/api/v1/landing.py:35-43`: `_FALLBACK_DATA` now zeroed with `stale: True` (not fake `zk_total=1500` / `deals_total=6_830_000`). - New `LandingStatsOut` Pydantic schema with `stale: bool = False` — fail-loud contract. - DB exception → `logger.exception(...)` + `return None` → response carries `stale=True`. - Frontend `page.tsx`: `const isStale = stats?.stale === true; const showSkeleton = isLoading || isStale;` — KPI cells render `<KpiSkeleton/>` placeholders + visible "Данные обновляются..." note. No fake numbers shipped on error path. 2. **UUID contract drift (was CRITICAL)** — FIXED. - `frontend/src/lib/api/landing.ts:30`: `PilotRequestResponse.id: string` (was `number`), comment `/** UUID returned as string via CAST(id AS text) */`. - Backend `pilot.py:52`: `RETURNING CAST(id AS text), created_at` — consistent. - `PilotRequestModal.tsx:32-34`: `deriveTrackingId(id) = "GD-" + id.replace(/-/g, "").slice(0, 8).toUpperCase()` — clean display "GD-7F3A8C2E" instead of raw 36-char UUID. Good UX. - CAST verified — backend uses `RETURNING CAST(id AS text)`, frontend treats as string, no raw UUID leaks to user. 3. **Modal a11y (was HIGH)** — FIXED. - `PilotRequestModal.tsx:48`: uses `<Drawer open={open} onClose={handleClose} side="bottom">` (A3 #341 primitive). - Verified `Drawer.tsx` provides: focus trap (Tab/Shift+Tab cycling lines 82-103), ESC listener (lines 54-63), focus restore via `previousFocusRef` (lines 36-51), body scroll lock `document.body.style.overflow = "hidden"` (lines 66-74), `role="dialog" aria-modal="true"` (line 163), backdrop click close (lines 76-80). - Removed ~80 LoC of hand-rolled overlay logic per commit message — confirmed via diff. - `"use client"` directive present (line 1). 4. **Mobile/desktop positioning (was HIGH)** — addressed via Drawer primitive. - `side="bottom"` → bottom-sheet (`borderRadius: "12px 12px 0 0"`, `maxHeight: "85vh"`, slide up via `translateY` lines 126-142). - On desktop, bottom-sheet is acceptable mobile-first pattern (A3 primitive supports `side="right"` if dedicated desktop modal needed in future, but current single-side is consistent with A12 plan). - No misleading "Desktop: centered modal" inline comment anymore — cleaner. 5. **Misc (round-1 nits)** — FIXED. - Design tokens: all `#hex` literals in `page.tsx` and `PilotRequestModal.tsx` replaced with `var(--token)` (only remaining `#fff` is white-on-accent CTA text — acceptable, per ui-ux.md). KPI label uppercase + 0.04em letter-spacing OK. - HeadlineBar: hand-rolled `<div bg=slate-900>` replaced with `<HeadlineBar title=... subtitle=.../>` component (page.tsx:391). - h1 fontSize 28 (was 32) — matches display spec. ### Other 4 files (no regression) - `frontend/package-lock.json`: only `baseline-browser-mapping 2.10.29 → 2.10.30` (transitive bump, benign). - `frontend/src/app/_legacy/page-tabs.tsx`: unchanged backup. - `frontend/src/lib/api/landing.ts`: 54 LoC new file, clean TanStack hooks, `staleTime: 60_000` for stats, proper types. - Backend `landing.py`: idempotent — DB query unchanged, only response schema + fallback semantics tightened. ### Cross-file impact - Frontend `apiFetch` consumer correct, no caller in repo depends on old `id: number` shape. - `useLandingStats` query key `["landing", "stats"]` — unique, no cache collision. ### Positive observations - Skeleton placeholder is `aria-label="Данные обновляются..."` — screen-reader friendly. - Email regex on frontend `/^[^\s@]+@[^\s@]+\.[^\s@]+$/` matches backend Pydantic pattern — consistent validation. - Form noValidate + manual EMAIL_RE — proper progressive enhancement. ### Severity **APPROVE** — proceeding to merge. Merging via `mcp__forgejo__merge_pull_request` (squash).
lekss361 merged commit f242d9bc44 into main 2026-05-18 05:16:58 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lekss361/gendesign#367
No description provided.