gendesign/.claude/agents/deep-review-phases/phase-3-dimensions.md

7.1 KiB
Raw Blame History

Deep Review — Phase 3: Семь измерений

Supporting reference. Применяй к файлам P0/P1 минимум, к P2/P3 — выборочно.

A. Security 🔒 (BLOCK при любой находке)

  • Hardcoded secrets / API keys / passwords (grep -E "(api[_-]?key|password|token|secret).*=.*['\"][a-zA-Z0-9]{16,}" по diff)
  • SQL injection: f-string / .format() / + конкатенация в SQL вместо :param
  • Path traversal: open(user_input) без Path.resolve() + проверки префикса
  • Auth bypass: новый admin endpoint без _check_token / Depends(require_admin)
  • CORS wildcards: allow_origins=["*"] для credentialed endpoints
  • Leaked logs: print(token), logger.info(f"...{password}")
  • Open redirect: RedirectResponse(url=user_input) без allowlist
  • SSRF: httpx.get(user_input) без allowlist домена
  • XXE / unsafe YAML / unsafe pickle deserialization
  • TOCTOU в file operations / DB checks

B. Correctness 🎯

  • Edge cases: None / [] / empty string / 0 / float('nan') обработка
  • Off-by-one: range bounds, slicing, pagination (limit/offset)
  • Race conditions:
    • Celery worker fork: refs Bug_Worker_Ready_EarlyReturn (vault)
    • Multiple instances одного task — есть acks_late=True + idempotency?
    • DB: SELECT ... FOR UPDATE где нужен row lock?
  • Transaction symmetry: каждый BEGINCOMMIT или ROLLBACK? Nested with db.begin() корректны?
  • Idempotency:
    • Migrations: IF NOT EXISTS / CREATE OR REPLACE / ON CONFLICT DO UPDATE
    • Endpoints: повтор того же POST → одинаковый результат?
    • Scrapers: rerun → нет дубликатов в БД?
  • Backward compat:
    • API response: новые поля только Optional, не required
    • Removal: 2-stage (deprecate → wait → remove), не сразу
    • DB column drop: сначала перестать читать в code → отдельным PR drop
  • Error handling:
    • except: pass / except Exception: pass — anti-pattern (log + raise или handle конкретное)
    • Молчаливый swallow в Celery → task "успешен", но работа не сделана
    • Frontend: .catch() без error toast → user не видит fail
  • Type safety: TS any / as unknown as Foo, Python missing type hints на public API

C. Performance

  • N+1 queries: цикл с db.execute(...) или await session.execute(...) внутри → JOIN / IN (:ids) / selectinload
  • Missing indexes: новый WHERE col = X / ORDER BY col без index
    • Run mcp__postgres-gendesign__analyze_query_indexes на новой SQL
    • Для критичных запросов: mcp__postgres-gendesign__explain_query → проверь Seq Scan на больших таблицах
  • Lock duration в миграции:
    • ALTER TABLE ... ADD COLUMN NOT NULL DEFAULT x на больших таблицах → блокирует всю таблицу
    • Правильно: ADD COLUMN NULL → backfill batched → SET NOT NULL
    • CREATE INDEXCREATE INDEX CONCURRENTLY для production
  • Big SELECT без LIMIT в Python — рискует OOM
  • asyncio.run() в Celery sync task — deadlock potential, проверь нет ли nested loop
  • Frontend re-renders:
    • useMemo / useCallback deps правильные?
    • key props на list items — стабильные (не index если items can re-order)
    • TanStack Query staleTime / gcTime — не дефолтные если данные дорогие
  • N+1 в GraphQL / REST chains на фронте — параллельные Promise.all или batch endpoint

D. Project conventions 📋

Python (.claude/rules/backend.md):

  • psycopg v3 only — import psycopg2 = BLOCK
  • CAST(:x AS type) в SQL — НЕ :x::type (CAST trap)
  • httpx только, не requests
  • async def для FastAPI handlers
  • logger.* не print() в prod code
  • ruff line ≤100
  • SAVEPOINT pattern в циклах с per-item commit

TS / React (.claude/rules/frontend.md + ui-tokens.md + ui-conventions.md):

  • strict, no any
  • TanStack Query для HTTP, не bare useEffect + fetch
  • safeUrl() для user-controlled hrefs (XSS prevention)
  • Next.js 15 app router patterns ('use client' только где нужно)
  • Design tokens — только из ui-tokens.md списка (нет inline #hex)

SQL (.claude/rules/sql.md):

  • data/sql/NN_topic.sql — numbered, sequential
  • BEGIN; ... COMMIT; обёртка для DDL
  • IF EXISTS / IF NOT EXISTS — idempotent
  • VIEW dependencies — DROP VIEW IF EXISTS X CASCADE если refactor

Deploy / infra (.claude/rules/deploy.md):

  • .env.runtime не коммитится
  • caddy reload после Caddyfile change
  • Forgejo Actions secrets через UI, не в коде

E. Architecture & maintenance 🏗

  • Right placement:
    • HTTP / serialization → api/v1/
    • Business logic → services/
    • Long-running / scheduled → tasks.py (Celery) / services/<area>/
    • Не запихивай business logic в FastAPI handler
  • DRY: дубль логики в parcels.py vs analytics_queries.py vs services/?
  • Dead code: TODO / FIXME без issue link, commented-out blocks, unused imports
  • Naming: _check_token vs check_admin_token — consistency с остальным codebase
  • Module boundaries: scraper не должен импортить из api/v1/; frontend hook не зовёт другой hook напрямую если можно через service

F. Tests coverage 🧪

  • Если PR меняет бизнес-логику в services/ — есть ли тест в backend/tests/?
  • Если added endpoint — есть ли smoke test?
  • Если fix bug — есть ли regression test покрывающий именно тот сценарий?
  • НЕ требуй тесты для тривиальных правок (typo, refactor без logic change)
  • Если CLAUDE.md / project явно говорит "не пиши тесты пока не попросят" — не настаивай

G. Vault & decisions cross-check 📚

Обязательный шаг для P0-P1 PR.

mcp__obsidian__obsidian_simple_search "<keyword из diff>"
mcp__obsidian__obsidian_simple_search "<имя функции / эндпоинта>"

Проверь:

  • fixes/ — была ли уже эта же ошибка? Новый код не повторяет старый bug?
  • decisions/ — есть ли архитектурный decision который этот PR нарушает?
  • limitations/ — known issue который этот код должен учитывать?
  • code/patterns/ — есть ли established pattern для этого типа изменения?

Если PR fix bug — должна быть entry в fixes/<bug>.md (per CLAUDE.md rule #6). Если PR architectural change — должна быть entry в decisions/<decision>.md.