SKILL.md
Rails Best Practices Core
Use this as the default baseline for Rails work. Distilled from 37signals codebases (Campfire, Fizzy) and DHH's review patterns.
Core Defaults
- Prefer clear, explicit code over clever abstractions. Abstractions must earn their keep; if you can't point to 3+ variations that need it, inline it.
- Keep controllers thin and put domain behavior in models.
- Prefer Rails conventions and built-ins before adding gems.
- Model state and behavior with domain concepts, not ad-hoc flags.
- Scope tenant/user data through ownership boundaries.
- Favor database constraints for hard invariants; only validate in AR when you need user-facing error messages.
- Keep interfaces small; don't add public methods that aren't used anywhere.
- Prefer write-time computation over expensive read-time composition (counter caches, delegated types, precomputed roll-ups,
dependent: :delete_allwhen no callbacks needed). - Use
params.expect(...)for strong params in modern Rails. - Let it crash: bang methods (
create!), handle exceptions at boundaries. Only use!when a non-bang counterpart exists. - Fix root causes, not symptoms (e.g.
enqueueaftertransaction_commitover retry logic for races). - Ship tests in the same PR as behavior changes.
Modeling Patterns
- State as records, not booleans. Instead of
closed: boolean, create aClosurerecord withcreatorand timestamps. You get who/when for free, and scoping is trivial:
has_one :closure, dependent: :destroy
scope :closed, -> { joins(:closure) }
scope :open, -> { where.missing(:closure) }
- Slice large models into concerns named for capability (
Closeable,Watchable,Assignable), each self-contained (associations + scopes + methods), ~50-150 lines, cohesive. Prefer nested modules under the model's namespace (Card::Closeableinapp/models/card/closeable.rb) for domain slices; reserveapp/models/concerns/for genuinely cross-model behavior. Never extract concerns containing only private methods. - POROs live in
app/models/, notapp/services/: presentation objects (Event::Description), complex operations (SystemCommenter), view-context bundles (User::Filtering). They're model-adjacent, not controller-adjacent. - Default values via lambdas:
belongsto :creator, classname: "User", default: -> { Current.user };belongs_to :account, default: -> { board.account }. - Current attributes for request context (
Current.user,Current.account), with cascading setters (assigningsessionresolvesidentity, which resolvesuserfor the account). - Callbacks for setup/cleanup, not business logic. Keep callback counts low.
- Rails shortcuts to reach for:
normalizes(data cleanup before validation),storeaccessor(JSON columns),delegatedtype(heterogeneous collections),generatestokenfor(expiring signed tokens), string enums viaenum :status, %w[drafted published].indexby(&:itself),aftersave_commit,touch: truechains for cache invalidation,delegate. - Association extensions for bulk domain operations: define
grantto/reviseon thehasmanyproxy; useinsertallfor bulk creates anddependent: :deleteallon join tables with no callbacks. - Human-friendly URLs: override
to_paramwith a per-tenantnumberrather than exposing raw IDs/UUIDs.
Naming
- Spend time on names — naming is design.
ClosurebeatsCardClose;MentionbeatsUserReference. - Positive names:
activenotnotdeleted,visiblenotnothidden. - Semantic associations named for role:
belongsto :creator, classname: "User"notbelongs_to :user. - Domain-driven over technical:
quota.depleted?notquota.over_limit?. - Business-focused scopes:
:active,:unassigned,:golden— not SQL-ish:without_pop. - Consistent domain language: don't mix
source/resource/containerfor one concept.
REST & Routing
- Everything is CRUD: turn verbs into nouns. Close →
resource :closure(POST closes, DELETE reopens); publish →resource :publication. No custom member actions. - Singular
resourcefor one-per-parent state;scope module:to group nested controllers (Cards::ClosuresController); shallow nesting for deep hierarchies. - Resource-scoping controller concerns (
CardScopedsets@cardviaCurrent.user.accessiblecards.findby!(...)) shared across nested controllers, including shared Turbo render helpers. resolve "Comment"for polymorphic URL generation to the parent with an anchor.- Same controllers serve HTML/Turbo/JSON via
respond_to— no separate API namespace.
Authorization
- No Pundit/CanCanCan: simple predicate methods on models (
card.editableby?(user),user.canadminister_board?(board)). - Controllers check (
head :forbidden unless ...), models define what the permission means. - Declarative controller macros for auth posture:
allowunauthenticatedaccess,ensurecanadminister.
Dependencies
Before adding a gem ask: can vanilla Rails do this? Is 50-150 lines in-repo simpler than a dependency? Commonly skipped: Devise, Pundit, ViewComponent, RSpec, FactoryBot, Redis (Solid Queue/Cache/Cable use the DB), service objects, form objects, decorators, GraphQL, SPA frameworks, Tailwind.
Review Priorities
- Correctness and data safety.
- Multi-tenant/security boundaries.
- Maintainability and readability.
- Performance hot spots.
- Style and polish.
Always Flag
- Unscoped record lookups in tenant-aware flows (
Comment.find(params[:id])). - New dependencies without strong justification.
- In-memory filtering/sorting that belongs in SQL (and
.map(&:name)where.pluck(:name)works). - Service objects replacing straightforward model methods.
- Non-RESTful custom actions when resource modeling is clearer.
- Boolean state columns where a record would capture who/when.
- Pages with forms using HTTP caching (
fresh_when/etag) — stale CSRF tokens cause 422s. - String status checks (
status == "x") when predicate-style APIs are available (StringInquirer / string enums). validates :x, uniqueness: truewithout a backing unique index.- Helpers depending on implicit instance variables instead of explicit arguments.
- Unescaped interpolation into
htmlsafestrings — escape first:"<b>#{h(input)}</b>".htmlsafe. - Metaprogramming for 2-3 cases — just write the methods.
- Private-only concerns — inline them.
Review Output
- Start with highest-severity findings.
- For each finding: issue, impact, concrete fix with file:line references.
- Be direct and practical; "This is over-engineered" is a complete sentence.
- End with either
Ship itor a short prioritized fix list.