Review Code

review-code

Use this skill when reviewing Hanami 2.x code for quality and convention adherence.

Core principle: Code review catches architectural violations, not just bugs. Focus on structure, boundaries, and testability.

Quick Reference

Review Workflow

Follow this sequence and report violations in priority order:

1. Check Action responsibility — Is the Action ≤ ~10 lines? Does it delegate business logic to services or repositories? Flag any SQL, filtering, or domain logic inline in the Action.
2. Verify DI usage — Are all dependencies declared via include Deps[...]? Flag any Hanami.app['key'] direct access.
3. Audit query locations — Do all DB queries live in Repositories or Relations? Flag SQL in Actions or Views immediately.
4. Inspect Repositories — Do they return Entities (not raw hashes)? Is auto_struct true / struct_namespace configured?
5. Review Views — Do expose blocks receive pre-fetched data only? Flag any DB calls or business logic.
6. Check error handling — Are exceptions logged and generic messages returned? Flag any e.message or e.backtrace in responses.
7. Assess test coverage — Are there request specs for each endpoint? Are 400, 404, 422, and 500 paths tested?

For each violation found, report: location, rule broken, concrete fix (with code where helpful), and priority (blocker if it exposes internals or bypasses DI; suggestion otherwise).

Core Rules with Examples

1. Actions — single-responsibility, delegate logic:

# GOOD
class Index < MyApp::Action
  include Deps["repos.user_repo"]
  def handle(request, response)
    response.render(view, users: user_repo.all)
  end
end

# BAD — business logic and direct ROM access in Action
users = Hanami.app["db.rom"].gateways[:default].dataset(:users).where(status: "active").to_a
active_users = users.select { |u| u.status == "active" }

2. Dependencies — always via Deps, never direct container:

# GOOD
include Deps["repos.user_repo"]

# BAD
repo = Hanami.app["repos.user_repo"]

3. Repositories — return Entities, not raw hashes:

# GOOD
class UserRepo < Hanami::DB::Repo[:users]
  struct_namespace MyApp::Entities
  auto_struct true
end

# BAD
def find(id) = users.where(id: id).one.to_h

4. Views — no DB queries or business logic in expose:

# GOOD
expose :user  # receives pre-fetched entity from Action

# BAD
expose :user do |request|
  Hanami.app["repos.user_repo"].by_id(request.params[:id]).one
end

5. Error handling — log details, return generic messages:

# GOOD
rescue StandardError => e
  Hanami.app[:logger].error(e.message)
  halt 500, { error: "Internal server error" }.to_json
end

# BAD — leaks internals
halt 500, { error: e.message, backtrace: e.backtrace }.to_json

6. Tests — cover behavior and all error paths:

# GOOD
it "returns 404 for unknown user" do
  get "/users/9999"
  expect(last_response.status).to eq(404)
end

# BAD — tests implementation, not behavior
expect(user_repo).to receive(:active)

Integration