Swe Code Review

Code Review — Python SWE Agent

Protocolo de Review

Execute sempre nesta ordem. Cada categoria tem peso diferente:

PRIORIDADE 1 — BLOQUEANTE (deve corrigir antes de merge)
  🔴 Segurança
  🔴 Corretude (bugs, edge cases)
  🔴 Violações arquiteturais graves

PRIORIDADE 2 — IMPORTANTE (deve corrigir em follow-up)
  🟡 Ausência de testes para caminho crítico
  🟡 Acoplamento desnecessário
  🟡 Complexidade excessiva

PRIORIDADE 3 — SUGESTÃO (melhoria opcional)
  🟢 Nomenclatura
  🟢 Performance não-crítica
  🟢 Estilo/idioms do Python

Template de Review

Para cada arquivo/função revisada, use este formato:

📁 src/application/use_cases/create_order.py

🔴 [BLOQUEANTE] Linha 45: SQL injection
  - Problema: cursor.execute(f"SELECT... {order_id}")
  - Risco: Permite acesso não autorizado ao banco
  - Correção: Use ORM ou cursor.execute("SELECT... %s", (order_id,))

🟡 [IMPORTANTE] Linha 12-67: Falta teste para cenário de estoque zerado
  - Problema: Nenhum teste cobre `InsufficientStockError`
  - Impacto: Bug silencioso em produção possível
  - Correção: Adicionar test_create_order_with_zero_stock_raises_error

🟢 [SUGESTÃO] Linha 23: Nomenclatura confusa
  - Problema: variável `data` não é descritiva
  - Sugestão: renomear para `order_dto` ou `create_order_request`

Checklist Completo de Code Review

🔴 Segurança (Use skill swe-security para detalhes)

• Nenhum secret/senha/token hardcoded no código
• Todos os inputs de usuário são validados com Pydantic
• Queries ao banco usam ORM ou parameterized (sem f-strings em SQL)
• Verificação de ownership em todos os endpoints que acessam recursos
• Nenhum eval(), exec(), pickle.loads() com dados não confiáveis
• Senhas hashadas com bcrypt/argon2
• Nenhum PII em logs
• Rate limiting em endpoints sensíveis (login, reset de senha)

🔴 Corretude

• Edge cases cobertos: None, lista vazia, string vazia, valores negativos, zero
• Concorrência: race conditions em operações de escrita?
• Transações de banco: operações que devem ser atômicas estão em transaction?
• Erros de rede/IO têm retry ou graceful degradation?
• Overflow em operações numéricas?

🔴 Arquitetura (Use skill swe-architecture para detalhes)

• Domínio não importa de infra (sem Django ORM, SQLAlchemy dentro de entities/)
• Use Cases não têm lógica de apresentação (sem request/response objects de HTTP)
• Inversão de dependência: Use Cases dependem de interfaces, não de implementações
• Nenhum God Object (classe com >10 métodos públicos não relacionados)
• Nenhum Anemic Domain Model (entidades sem comportamento, só getters/setters)

🟡 Testes (Use skill swe-testing para detalhes)

• Cobertura mínima de 80% para o código novo
• Caminho feliz testado
• Ao menos 1 caminho de erro testado para cada Use Case
• Fixtures limpas (sem estado compartilhado entre testes)
• Nomes de testes descritivos (test_<cenário>)

🟡 Observabilidade (Use skill swe-observability para detalhes)

• Erros inesperados logam com exc_info=True e contexto suficiente
• Operações críticas de negócio têm log estruturado
• Nenhum print() de debug esquecido
• Logs usam nível correto (DEBUG/INFO/WARNING/ERROR/CRITICAL)

🟡 Qualidade Geral

• Funções com >20 linhas: justificável ou precisa ser decomposta?
• Complexidade ciclomática: nenhuma função com >5 branches aninhados
• Duplicação de código: DRY sem over-engenharia
• Type hints em todas as funções públicas
• Docstrings em classes/funções públicas complexas

🟢 Python Idioms

• List comprehensions em vez de loops simples onde legível
• Context managers (with) para recursos (arquivos, conexões)
• dataclasses ou pydantic para DTOs (sem dicts sem tipo)
• pathlib em vez de os.path
• f-strings em vez de .format() ou %
• Enum para constantes relacionadas (não strings mágicas)

Padrões Problemáticos — Reconhecer de Imediato

# 🔴 Fat View / Fat Route — lógica de negócio na camada HTTP
@router.post("/orders")
async def create_order(data: dict):
    # 40 linhas de lógica de negócio aqui = PROBLEMA
    user = await db.get(User, data["user_id"])
    if user.balance < data["total"]:
        raise ...
    order = Order(**data)
    await db.save(order)
    await email_service.send(...)
    # Isso tudo deveria estar num Use Case

# 🔴 Service como Namespace (procedural disfarçado de OO)
class OrderService:
    @staticmethod
    def create(data): ...
    @staticmethod
    def cancel(order_id): ...
    @staticmethod
    def calculate_tax(order): ...  # não tem coesão!

# 🟡 Exceção genérica capturada (swallow errors)
try:
    result = await external_api.call()
except Exception:
    pass  # NUNCA silenciar exceções

# 🟡 Mutable default argument
def process(items=[]):  # bug clássico Python!
    items.append(...)   # corrigir: def process(items=None): items = items or []

# 🟡 N+1 query
for order in orders:
    customer = await db.get(Customer, order.customer_id)  # 1 query por loop!
# Corrigir: eager loading com joinedload / prefetch_related

Como Dar Feedback Construtivo

❌ "Isso está errado"
✅ "Linha 45: Esta query é vulnerável a SQL injection porque usa
   interpolação direta. Substitua por: cursor.execute('SELECT ... WHERE id = %s', (order_id,))
   Referência: OWASP A03"

❌ "Adicione mais testes"
✅ "Falta cobertura para o caminho de erro: quando o estoque é insuficiente.
   Sugestão:
   def test_create_order_with_insufficient_stock_raises_error():
       product = build_product(stock=0)
       with pytest.raises(InsufficientStockError):
           use_case.execute(CreateOrderDTO(items=[...]))"

Métricas de Qualidade do PR

Ao final do review, sumarize:

📊 Resumo do Review
═══════════════════════════════
Arquivos revisados: N
Bloqueantes:        X 🔴
Importantes:        Y 🟡
Sugestões:          Z 🟢

Veredicto:
  🚫 Bloqueado  — corrigir itens 🔴 antes do merge
  ⚠️  Aprovado com ressalvas — itens 🟡 em follow-up
  ✅ Aprovado  — qualidade adequada para produção