Ревью кода #8

Closed
opened 2026-09-06 19:58:53 +03:00 by NotBigGhost · 3 comments
Owner

Проход по коду скилами /code-review и /simplify. После проведение необходимых работ и мёрж на основную ветку.

Зависит от: #1, #2, #4, #17

Проход по коду скилами /code-review и /simplify. После проведение необходимых работ и мёрж на основную ветку. Зависит от: #1, #2, #4, #17
NotBigGhost added this to the v1.35 - промежуточная полировка версии 1.3 milestone 2026-09-06 19:58:53 +03:00
NotBigGhost added the Kind/BugKind/Enhancement labels 2026-09-06 21:35:46 +03:00
NotBigGhost removed the Kind/BugKind/Enhancement labels 2026-09-06 21:41:36 +03:00
NotBigGhost added the Kind/Code Review label 2026-09-06 21:51:48 +03:00
Author
Owner

Последняя задача в этапе.

Последняя задача в этапе.
Agent added the
Status
Blocked
1
label 2026-09-07 15:13:11 +03:00
NotBigGhost removed the
Status
Blocked
1
label 2026-09-09 01:15:23 +03:00
Agent self-assigned this 2026-09-09 15:04:01 +03:00
Agent added the
Reviewed
Confirmed
1
label 2026-09-09 15:04:03 +03:00
Collaborator

План выполнения

Уровень ревью — high, охват — весь код проекта (не только диапазон main..dev), как выбрал владелец.

  1. /code-review high тремя прогонами по слоям, чтобы находки оставались предметными: backend/app (роутеры → сервисы → модели, аутентификация, статистика на сыром SQL, SSE); frontend/src (компоненты, хуки, роутинг; сгенерированный api/schema.d.ts исключён); backend/tests + инфраструктура (alembic/, scripts/, deploy/, compose-файлы, лаунчеры).
  2. Отсев находок: уровень high даёт и неуверенные гипотезы — каждую проверяю по коду до правки.
  3. Правки в ветке issue-8-code-review логическими коммитами: сначала корректность, затем упрощения.
  4. /simplify по коду, которого коснулось ревью.
  5. Инварианты из CLAUDE.md не трогаю: изоляция dev-стаба от прод-артефакта, слои routers → services → models, отсутствие Relationship, ветвление окружений только через core/config.py, UTC + APP_TZ_OFFSET_HOURS. Формула League Points — предмет вехи v1.5 (#22/#23), в ревью не переписываю.
  6. Находки, требующие продуктового решения, смены API или крупного рефакторинга, завожу отдельными задачами со ссылкой на #8, а не чиню молча.

Критерии готовности

  • Оба скила прогнаны по заявленному охвату.
  • Каждая находка закрыта одним из трёх способов: исправлена в ветке, вынесена в отдельную задачу, отклонена с обоснованием в отчёте.
  • Поведение не изменилось: pytest — 74 passed, npm run build — зелёный.
  • PR в dev открыт. Мёрж dev→main (релиз с версией на merge-коммите) остаётся за вами.

Ветка: issue-8-code-review от dev

## План выполнения Уровень ревью — `high`, охват — **весь код проекта** (не только диапазон `main..dev`), как выбрал владелец. 1. `/code-review high` тремя прогонами по слоям, чтобы находки оставались предметными: `backend/app` (роутеры → сервисы → модели, аутентификация, статистика на сыром SQL, SSE); `frontend/src` (компоненты, хуки, роутинг; сгенерированный `api/schema.d.ts` исключён); `backend/tests` + инфраструктура (`alembic/`, `scripts/`, `deploy/`, compose-файлы, лаунчеры). 2. Отсев находок: уровень `high` даёт и неуверенные гипотезы — каждую проверяю по коду до правки. 3. Правки в ветке `issue-8-code-review` логическими коммитами: сначала корректность, затем упрощения. 4. `/simplify` по коду, которого коснулось ревью. 5. Инварианты из `CLAUDE.md` не трогаю: изоляция dev-стаба от прод-артефакта, слои `routers → services → models`, отсутствие `Relationship`, ветвление окружений только через `core/config.py`, UTC + `APP_TZ_OFFSET_HOURS`. Формула League Points — предмет вехи v1.5 (#22/#23), в ревью не переписываю. 6. Находки, требующие продуктового решения, смены API или крупного рефакторинга, завожу отдельными задачами со ссылкой на #8, а не чиню молча. **Критерии готовности** - Оба скила прогнаны по заявленному охвату. - Каждая находка закрыта одним из трёх способов: исправлена в ветке, вынесена в отдельную задачу, отклонена с обоснованием в отчёте. - Поведение не изменилось: `pytest` — 74 passed, `npm run build` — зелёный. - PR в `dev` открыт. Мёрж `dev→main` (релиз с версией на merge-коммите) остаётся за вами. **Ветка:** `issue-8-code-review` от `dev`
Agent added the
Status
In Review
label 2026-09-09 18:53:39 +03:00
Collaborator

Работа выполнена, открыт PR: #34

Итог: /code-review high тремя прогонами (бэкенд, фронт, тесты и инфраструктура) и /simplify четырьмя углами по всему проекту. Исправлено 20 находок — среди них обход каталога ачивок, из-за которого админский запрос сносил /data целиком; неработавшая оптимистичная блокировка при правке участников и вложений; гонка индексов в создании партии; разлогин при обрыве связи. Убраны N+1 и дубли: общий загрузчик файлов, единый источник времени, client_ip, read_capped_image, общие ключи инвалидации.

Отдельно: прогон ревью по тестам поймал, что моя же первая проверка обхода каталога была вакуумной — httpx нормализует .. до отправки, и тест прошёл бы и без защиты. Переписал на percent-кодированную форму и убедился, что он краснеет при снятой защите.

Проверки: pytest — 81 passed (было 74, добавлено 7 тестов); npm run build — успешно.

Вынесено в отдельные задачи (требуют вашего решения, молча чинить не стал): #28, #29, #30, #32, #33.

Статус: Status/In Review
Осталось за вами: ревью и мёрж PR — задача закроется автоматически. Затем релизный мёрж dev→main с версией на merge-коммите: этап v1.35 после закрытия #8 остаётся только с задачами, которые я завёл сегодня, — решите, брать ли их в этот релиз или перенести.

Работа выполнена, открыт PR: https://gitea.arseniev.info/NotBigGhost/ForbiddenStarsApp/pulls/34 **Итог:** `/code-review high` тремя прогонами (бэкенд, фронт, тесты и инфраструктура) и `/simplify` четырьмя углами по всему проекту. Исправлено 20 находок — среди них обход каталога ачивок, из-за которого админский запрос сносил `/data` целиком; неработавшая оптимистичная блокировка при правке участников и вложений; гонка индексов в создании партии; разлогин при обрыве связи. Убраны N+1 и дубли: общий загрузчик файлов, единый источник времени, `client_ip`, `read_capped_image`, общие ключи инвалидации. Отдельно: прогон ревью по тестам поймал, что моя же первая проверка обхода каталога была вакуумной — httpx нормализует `..` до отправки, и тест прошёл бы и без защиты. Переписал на percent-кодированную форму и убедился, что он краснеет при снятой защите. **Проверки:** `pytest` — 81 passed (было 74, добавлено 7 тестов); `npm run build` — успешно. **Вынесено в отдельные задачи** (требуют вашего решения, молча чинить не стал): #28, #29, #30, #32, #33. **Статус:** `Status/In Review` **Осталось за вами:** ревью и мёрж PR — задача закроется автоматически. Затем релизный мёрж `dev→main` с версией на merge-коммите: этап v1.35 после закрытия #8 остаётся только с задачами, которые я завёл сегодня, — решите, брать ли их в этот релиз или перенести.
Sign in to join this conversation.
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: NotBigGhost/ForbiddenStarsApp#8