- CLAUDE.md: 97->63 lines, drop dev-commands and Don't duplicates already covered by .claude/rules/* - git-pr.md, deploy.md: replace gh CLI with Forgejo MCP/curl - code-reviewer.md: fix mcp__postgres__ -> mcp__postgres-gendesign__ - tech-analyst.md: sonnet -> haiku - deep-code-reviewer.md: 391->135 lines, phase details split into .claude/agents/deep-review-phases/ (loaded on demand) - qa-tester, code-reviewer, deep-code-reviewer: add memory: project - ui-ux.md: split into ui-tokens/ui-conventions/ui-microcopy - scripts/claude-hooks/check-no-print.py + PostToolUse hook - .gitignore: explicit list (git whitelist limitation)
7.1 KiB
7.1 KiB
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?
- Celery worker fork: refs
- Transaction symmetry: каждый
BEGIN→COMMITилиROLLBACK? Nestedwith db.begin()корректны? - Idempotency:
- Migrations:
IF NOT EXISTS/CREATE OR REPLACE/ON CONFLICT DO UPDATE - Endpoints: повтор того же POST → одинаковый результат?
- Scrapers: rerun → нет дубликатов в БД?
- Migrations:
- Backward compat:
- API response: новые поля только
Optional, не required - Removal: 2-stage (deprecate → wait → remove), не сразу
- DB column drop: сначала перестать читать в code → отдельным PR drop
- API response: новые поля только
- 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 на больших таблицах
- Run
- Lock duration в миграции:
ALTER TABLE ... ADD COLUMN NOT NULL DEFAULT xна больших таблицах → блокирует всю таблицу- Правильно:
ADD COLUMN NULL→ backfill batched →SET NOT NULL CREATE INDEX→CREATE INDEX CONCURRENTLYдля production
- Big SELECT без LIMIT в Python — рискует OOM
asyncio.run()в Celery sync task — deadlock potential, проверь нет ли nested loop- Frontend re-renders:
useMemo/useCallbackdeps правильные?keyprops на 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 v3only —import psycopg2= BLOCKCAST(:x AS type)в SQL — НЕ:x::type(CAST trap)httpxтолько, неrequestsasync defдля FastAPI handlerslogger.*не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, sequentialBEGIN; ... COMMIT;обёртка для DDLIF 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
- HTTP / serialization →
- DRY: дубль логики в parcels.py vs analytics_queries.py vs services/?
- Dead code: TODO / FIXME без issue link, commented-out blocks, unused imports
- Naming:
_check_tokenvscheck_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.