lekss361/gendesign

Fork 0

feat(#254): user_custom_pois — backend schema + CRUD + scoring integration #257

Merged

lekss361 merged 1 commit from feat/254-custom-pois-backend into main

2026-05-17 06:42:30 +00:00

lekss361 commented

2026-05-17 06:36:56 +00:00

Owner

Summary

SQL migration 101_user_custom_pois.sql: GEOGRAPHY(POINT) + GIST, weight CHECK [-5,5]
Pydantic schemas: CustomPoiCreate / CustomPoiUpdate / CustomPoiOut
Service custom_pois.py: CRUD + get_overlaps_for_scoring (psycopg v3 CAST)
API POST/GET/PATCH/DELETE /api/v1/custom-pois: X-Session-Id header auth
Scoring in analyze_parcel: custom POI after OSM loop, contribution=weight*decay, custom_poi_score_items in response
Tests (13): CRUD, validation 422, scoring integration (mock-based)
Vault: Schema_User_Custom_Pois.md + v3.9 in Endpoint_Parcel_Analyze.md

Frontend contract (X-Session-Id)

Frontend saves uuid from X-Session-Id response header to localStorage, sends it as X-Session-Id request header in all custom-pois and analyze calls.

Test plan

Apply migration 101_user_custom_pois.sql
POST /api/v1/custom-pois with X-Session-Id header -> 201
POST /api/v1/parcels/.../analyze with same X-Session-Id -> custom_poi_score_items non-empty
weight=10 -> 422, lon=200 -> 422
DELETE -> 204, repeat -> 404

Dependencies

Does NOT require #67 NextAuth (workaround via X-Session-Id)
Frontend PR is separate
Migration applied by database-expert after review

## Summary - SQL migration `101_user_custom_pois.sql`: GEOGRAPHY(POINT) + GIST, weight CHECK [-5,5] - Pydantic schemas: CustomPoiCreate / CustomPoiUpdate / CustomPoiOut - Service `custom_pois.py`: CRUD + `get_overlaps_for_scoring` (psycopg v3 CAST) - API POST/GET/PATCH/DELETE `/api/v1/custom-pois`: X-Session-Id header auth - Scoring in `analyze_parcel`: custom POI after OSM loop, contribution=weight*decay, `custom_poi_score_items` in response - Tests (13): CRUD, validation 422, scoring integration (mock-based) - Vault: Schema_User_Custom_Pois.md + v3.9 in Endpoint_Parcel_Analyze.md ## Frontend contract (X-Session-Id) Frontend saves uuid from `X-Session-Id` response header to localStorage, sends it as `X-Session-Id` request header in all custom-pois and analyze calls. ## Test plan - [ ] Apply migration `101_user_custom_pois.sql` - [ ] POST /api/v1/custom-pois with X-Session-Id header -> 201 - [ ] POST /api/v1/parcels/.../analyze with same X-Session-Id -> custom_poi_score_items non-empty - [ ] weight=10 -> 422, lon=200 -> 422 - [ ] DELETE -> 204, repeat -> 404 ## Dependencies - Does NOT require #67 NextAuth (workaround via X-Session-Id) - Frontend PR is separate - Migration applied by database-expert after review

lekss361 added 1 commit 2026-05-17 06:36:56 +00:00

feat(#254 ): user_custom_pois — backend schema + CRUD + scoring integration 485f489bd2

- SQL migration 101_user_custom_pois.sql: table with GEOGRAPHY(POINT) + GIST index
- Pydantic schemas: CustomPoiCreate / CustomPoiUpdate / CustomPoiOut
- Service custom_pois.py: CRUD + get_overlaps_for_scoring (1km radius, psycopg v3 CAST)
- API /api/v1/custom-pois: POST(201)/GET/PATCH/DELETE, X-Session-Id header auth
- Scoring in analyze_parcel: custom POI block after OSM POI loop,
  decay weight * max(0, 1 - dist/1000), custom_poi_score_items in response
- Tests: CRUD + validation (weight/lon/lat 422) + scoring integration (mock-based)
- Vault: Schema_User_Custom_Pois.md + Endpoint_Parcel_Analyze v3.9 section

lekss361 commented

2026-05-17 06:42:18 +00:00

Author

Owner

Deep Code Review — verdict: APPROVE

Summary

Status: APPROVE
Files reviewed: 7 (P0: 1 SQL migration; P1: 3 backend service/api; P2: 1 main.py routing; P3: 2 schema/tests)
Lines: +1011 / -1
PR: #257, head 485f489b, base 9dd72ec, mergeable=true

Critical / High / Medium

Нет блокирующих находок. Все P0/P1 правила соблюдены.

Конвенции (rules check)

data/sql/101_user_custom_pois.sql — корректное sequential naming (после 100_user_weight_profiles_default_seed.sql), BEGIN; ... COMMIT;, CREATE TABLE IF NOT EXISTS, CREATE INDEX IF NOT EXISTS — идемпотентно.
psycopg v3 + CAST trap: ни одного :name::type в diff (grep -E ':[a-z_]+::[a-z]' → пусто). Только CAST(:x AS type) + PostgreSQL-нативные касты ::geography (это inline cast результата ST_MakePoint(...), к bind-name не относится — OK).
SQL injection: все параметры через bind, нет f-string в SQL, UPDATE строит SET-список из whitelisted полей с bind-параметрами — safe.
logger.* вместо print(): соблюдено (logger.info/debug/warning).
Line length 100: соблюдён, строки разбиты.
SAVEPOINT в loop: bulk-инсертов в loop нет, CRUD по одной записи — паттерн не требуется.

Schema (data/sql/101_user_custom_pois.sql)

geom GEOGRAPHY(POINT, 4326) GENERATED ALWAYS AS (...) STORED — отличный выбор, PostGIS сам пересчитывает на UPDATE lon/lat (видно в update_custom_poi, не пишет в geom явно).
weight REAL NOT NULL CHECK (weight BETWEEN -5 AND 5) — соответствует Pydantic-валидации (ge=-5, le=5).
GIST на geom + B-tree на (user_id, parcel_cad) — хорошо покрывает scoring-запрос (WHERE user_id = ... AND ST_DWithin(geom, ...)).
created_at / updated_at с DEFAULT NOW(), trigger не нужен — updated_at = NOW() ставится в UPDATE-сервисе явно.

Pydantic schemas (custom_poi.py)

CustomPoiCreate: min_length=1, max_length=200 на name; lon/lat в WGS-84; weight ge=-5, le=5. Все границы покрыты.
CustomPoiUpdate: все поля Optional, проверка диапазонов сохранена.
CustomPoiOut: типы соответствуют DDL.

Service (services/site_finder/custom_pois.py)

Подготовленные SQL-константы (_INSERT_SQL, _SELECT_BY_USER_ALL, ...) — DRY, инъекции невозможны.
update_custom_poi: динамическое построение SET — корректно, имена колонок жёстко заданы (whitelist), bind-параметры для значений.
get_overlaps_for_scoring: ST_Centroid(ST_GeomFromText(CAST(:wkt AS text), 4326))::geography + ST_DWithin(..., 1000) — radius hardcoded 1000m matched с analyze_parcel decay 1 - distance / 1000. Согласовано.
try/except Exception в get_overlaps_for_scoring → logger.warning + return [] — graceful, scoring продолжает работать если custom POI таблица недоступна.

API (api/v1/custom_pois.py)

X-Session-Id resolved через _resolve_session_id helper: при отсутствии генерирует UUID и кладёт в response header — frontend-friendly.
POST/GET/PATCH/DELETE handlers — все используют user_id scope (WHERE user_id = :user_id) — изоляция данных пользователей.
PATCH/DELETE: 404 при попытке доступа к чужой POI (через user_id filter в SELECT/DELETE WHERE) — privacy-safe (404 не раскрывает existence чужой записи).

Scoring integration (parcels.py)

Custom POI loop после OSM POI loop, до score_final = score + center_bonus — корректное место, contribution учитывается в score_top_3_positives/negatives, factors_detailed, score_by_group.
_decay = max(0.0, 1.0 - _distance_m / 1000.0) — линейный decay, согласован с radius 1000m в _OVERLAPS_SQL.ST_DWithin.
try/except Exception вокруг custom POI блока → logger.warning — failure не ломает analyze.
custom_poi_score_items добавлен в response — frontend (PR #258) сможет отрисовать.
if not _session_id: skip — gracefully скипает если header отсутствует.

Tests (13 тестов)

CRUD happy paths (POST 201, GET list, PATCH 200, DELETE 204).
Validation 422: weight=10, weight=-6, lon=200, lat=100.
Authorization: PATCH/DELETE для чужой POI → 404 (mock-based, но семантика проверяется).
Auto-generated UUID в response header.
Scoring integration: 3 теста (positive weight, negative weight, no session_id → empty items).
Mock-based — slabee реального DB, но acceptable для unit-уровня; миграция/реальный API должны быть проверены smoke-тестом после deploy.

Cross-file impact

backend/app/main.py: router подключён под /api/v1/custom-pois — OK.
analyze_parcel response shape: добавлено новое optional-by-default поле custom_poi_score_items: list — backward-compatible (старый FE проигнорирует).
Frontend PR #258 должен мержиться после деплоя этого PR.
Migration #100 → #101 → sequential apply через deploy.yml автоматически.

Vault check

code/schemas/Schema_User_Custom_Pois.md — присутствует, корректный frontmatter (status: ready_to_apply, github_issue: 254).
domains/api/endpoints/Endpoint_Parcel_Analyze.md — содержит ссылку на Schema_User_Custom_Pois, описание custom_poi_score_items.
Документация синхронна с кодом.

Security

Privacy через user_id scope — корректная фильтрация во всех CRUD-операциях.
Note (production hardening, не блокирующее): X-Session-Id — это user-controlled value (FE генерирует), любой knowing чужой session_id может читать/редактировать его POI. Это явный trade-off в PR ("workaround до #67 NextAuth"). Acceptable для текущего этапа.
Rate limiting: на POST /custom-pois отсутствует — может быть abused (10k POI за минуту). Создать отдельный issue, не блокирует merge.

Performance

Index (user_id, parcel_cad) покрывает WHERE user_id = ... AND (parcel_cad IS NULL OR parcel_cad = ...).
GIST на geom покрывает ST_DWithin в scoring query.
N=100 POI на пользователя — query вернётся быстро (GIST + центроид рассчитывается один раз).
LIMIT в _OVERLAPS_SQL отсутствует — теоретический риск, если у пользователя 10000 POI и все в радиусе 1км. Realistic risk низкий, но suggest: добавить LIMIT 1000 на будущее (не блокирующее).

Risk score

Risk: Low — изолированная фича, новые таблица/endpoint/scoring блок, не трогает существующие data flows.
Reversibility: Easy revert — DROP TABLE user_custom_pois CASCADE + revert PR.
Recommended merge window: anytime.

Post-merge checklist

Migration 101_user_custom_pois.sql применится автоматически через deploy.yml.
Smoke test: curl -X POST -H 'X-Session-Id: test-uuid' -H 'Content-Type: application/json' -d '{"name":"test","weight":1,"lon":60.6,"lat":56.84}' https://gendsgn.ru/api/v1/custom-pois → 201.
Frontend PR #258 — мержить вторым.
Future hardening: rate-limit + LIMIT 1000 в _OVERLAPS_SQL — отдельным issue.

Merging via deep-code-reviewer.

## Deep Code Review — verdict: APPROVE ### Summary - **Status**: APPROVE - **Files reviewed**: 7 (P0: 1 SQL migration; P1: 3 backend service/api; P2: 1 main.py routing; P3: 2 schema/tests) - **Lines**: +1011 / -1 - **PR**: #257, head `485f489b`, base `9dd72ec`, mergeable=true ### Critical / High / Medium Нет блокирующих находок. Все P0/P1 правила соблюдены. ### Конвенции (rules check) - `data/sql/101_user_custom_pois.sql` — корректное sequential naming (после `100_user_weight_profiles_default_seed.sql`), `BEGIN; ... COMMIT;`, `CREATE TABLE IF NOT EXISTS`, `CREATE INDEX IF NOT EXISTS` — идемпотентно. - **psycopg v3 + CAST trap**: ни одного `:name::type` в diff (`grep -E ':[a-z_]+::[a-z]'` → пусто). Только `CAST(:x AS type)` + PostgreSQL-нативные касты `::geography` (это inline cast результата `ST_MakePoint(...)`, к bind-name не относится — OK). - **SQL injection**: все параметры через bind, нет f-string в SQL, UPDATE строит `SET`-список из whitelisted полей с bind-параметрами — safe. - **`logger.*` вместо `print()`**: соблюдено (`logger.info/debug/warning`). - **Line length 100**: соблюдён, строки разбиты. - **SAVEPOINT в loop**: bulk-инсертов в loop нет, CRUD по одной записи — паттерн не требуется. ### Schema (data/sql/101_user_custom_pois.sql) - `geom GEOGRAPHY(POINT, 4326) GENERATED ALWAYS AS (...) STORED` — отличный выбор, PostGIS сам пересчитывает на UPDATE `lon/lat` (видно в `update_custom_poi`, не пишет в `geom` явно). - `weight REAL NOT NULL CHECK (weight BETWEEN -5 AND 5)` — соответствует Pydantic-валидации (`ge=-5, le=5`). - GIST на geom + B-tree на `(user_id, parcel_cad)` — хорошо покрывает scoring-запрос (`WHERE user_id = ... AND ST_DWithin(geom, ...)`). - `created_at` / `updated_at` с `DEFAULT NOW()`, trigger не нужен — `updated_at = NOW()` ставится в UPDATE-сервисе явно. ### Pydantic schemas (custom_poi.py) - `CustomPoiCreate`: `min_length=1, max_length=200` на name; lon/lat в WGS-84; weight `ge=-5, le=5`. Все границы покрыты. - `CustomPoiUpdate`: все поля Optional, проверка диапазонов сохранена. - `CustomPoiOut`: типы соответствуют DDL. ### Service (services/site_finder/custom_pois.py) - Подготовленные SQL-константы (`_INSERT_SQL`, `_SELECT_BY_USER_ALL`, ...) — DRY, инъекции невозможны. - `update_custom_poi`: динамическое построение `SET` — корректно, имена колонок жёстко заданы (whitelist), bind-параметры для значений. - `get_overlaps_for_scoring`: `ST_Centroid(ST_GeomFromText(CAST(:wkt AS text), 4326))::geography` + `ST_DWithin(..., 1000)` — radius hardcoded 1000m matched с `analyze_parcel` decay `1 - distance / 1000`. **Согласовано**. - `try/except Exception` в `get_overlaps_for_scoring` → `logger.warning` + return `[]` — graceful, scoring продолжает работать если custom POI таблица недоступна. ### API (api/v1/custom_pois.py) - `X-Session-Id` resolved через `_resolve_session_id` helper: при отсутствии генерирует UUID и кладёт в response header — frontend-friendly. - POST/GET/PATCH/DELETE handlers — все используют `user_id` scope (`WHERE user_id = :user_id`) — изоляция данных пользователей. - PATCH/DELETE: 404 при попытке доступа к чужой POI (через `user_id` filter в SELECT/DELETE WHERE) — privacy-safe (404 не раскрывает existence чужой записи). ### Scoring integration (parcels.py) - Custom POI loop **после** OSM POI loop, **до** `score_final = score + center_bonus` — корректное место, contribution учитывается в `score_top_3_positives/negatives`, `factors_detailed`, `score_by_group`. - `_decay = max(0.0, 1.0 - _distance_m / 1000.0)` — линейный decay, согласован с radius 1000m в `_OVERLAPS_SQL.ST_DWithin`. - `try/except Exception` вокруг custom POI блока → `logger.warning` — failure не ломает analyze. - `custom_poi_score_items` добавлен в response — frontend (PR #258) сможет отрисовать. - `if not _session_id: skip` — gracefully скипает если header отсутствует. ### Tests (13 тестов) - CRUD happy paths (POST 201, GET list, PATCH 200, DELETE 204). - Validation 422: weight=10, weight=-6, lon=200, lat=100. - Authorization: PATCH/DELETE для чужой POI → 404 (mock-based, но семантика проверяется). - Auto-generated UUID в response header. - Scoring integration: 3 теста (positive weight, negative weight, no session_id → empty items). - Mock-based — slabee реального DB, но acceptable для unit-уровня; миграция/реальный API должны быть проверены smoke-тестом после deploy. ### Cross-file impact - `backend/app/main.py`: router подключён под `/api/v1/custom-pois` — OK. - `analyze_parcel` response shape: добавлено новое **optional-by-default** поле `custom_poi_score_items: list` — backward-compatible (старый FE проигнорирует). - Frontend PR #258 должен мержиться **после** деплоя этого PR. - Migration #100 → #101 → sequential apply через deploy.yml автоматически. ### Vault check - `code/schemas/Schema_User_Custom_Pois.md` — присутствует, корректный frontmatter (`status: ready_to_apply`, `github_issue: 254`). - `domains/api/endpoints/Endpoint_Parcel_Analyze.md` — содержит ссылку на `Schema_User_Custom_Pois`, описание `custom_poi_score_items`. - Документация синхронна с кодом. ### Security - Privacy через `user_id` scope — корректная фильтрация во всех CRUD-операциях. - **Note** (production hardening, не блокирующее): X-Session-Id — это user-controlled value (FE генерирует), любой knowing чужой `session_id` может читать/редактировать его POI. Это явный trade-off в PR ("workaround до #67 NextAuth"). Acceptable для текущего этапа. - **Rate limiting**: на POST `/custom-pois` отсутствует — может быть abused (10k POI за минуту). Создать отдельный issue, не блокирует merge. ### Performance - Index `(user_id, parcel_cad)` покрывает `WHERE user_id = ... AND (parcel_cad IS NULL OR parcel_cad = ...)`. - GIST на geom покрывает `ST_DWithin` в scoring query. - N=100 POI на пользователя — query вернётся быстро (GIST + центроид рассчитывается один раз). - LIMIT в `_OVERLAPS_SQL` отсутствует — теоретический риск, если у пользователя 10000 POI и все в радиусе 1км. Realistic risk низкий, но **suggest**: добавить `LIMIT 1000` на будущее (не блокирующее). ### Risk score - **Risk**: Low — изолированная фича, новые таблица/endpoint/scoring блок, не трогает существующие data flows. - **Reversibility**: Easy revert — `DROP TABLE user_custom_pois CASCADE` + revert PR. - **Recommended merge window**: anytime. ### Post-merge checklist 1. Migration `101_user_custom_pois.sql` применится автоматически через deploy.yml. 2. Smoke test: `curl -X POST -H 'X-Session-Id: test-uuid' -H 'Content-Type: application/json' -d '{"name":"test","weight":1,"lon":60.6,"lat":56.84}' https://gendsgn.ru/api/v1/custom-pois` → 201. 3. Frontend PR #258 — мержить вторым. 4. Future hardening: rate-limit + LIMIT 1000 в _OVERLAPS_SQL — отдельным issue. **Merging via deep-code-reviewer.**