-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
thoughtbot agents rules #783
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
437f52c
cddf211
78827e4
e86908e
dcb5e3c
bd35016
54e43e0
f9d5974
96c7461
da229c7
6052e75
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| # thoughtbot project architecture and coding standards for Rails development using agents | ||
|
|
||
| See the folder `rules` for language-specific guidelines, testing conventions, | ||
| and other standards. | ||
|
|
||
| > **Usage:** | ||
| > | ||
| > 1. Copy the content of this file. | ||
| > 2. Create a new file in the root of your project called `.claude/CLAUDE.md`. | ||
| > 3. Update the information in the new file to match your project. | ||
| > 4. Paste the content of this file into the new file. | ||
| > 5. Copy the rules folder into `.claude/` | ||
|
|
||
| ## Project: [APP_NAME] | ||
|
|
||
| [One sentence: what the app does and who it serves.] | ||
|
|
||
| ## Commands | ||
|
|
||
| ```bash | ||
| bin/rails server # Start dev server | ||
| bin/rails spec # Full test suite (Suspenders rake task) | ||
| bundle exec rspec spec/models # Model specs only | ||
| bundle exec rspec spec/requests # Request specs only | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have you found it knows how to run tests just for one file or how to run just one test? This can be useful for speed on a big project eg.
^ it's a bit wordy...
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh! I haven't tried that. It tends to run the whole file for the affected areas.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm testing with
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Claude running in cursor tends to run examples or groups with https://rspec.info/features/3-12/rspec-core/command-line/example-matches-name-option/ |
||
| bundle exec rspec spec/path/to/file_spec.rb # Run all tests in file | ||
| bundle exec rspec spec/path/to/file_spec.rb:72 # Run just the test at line 72 | ||
| rake standard # Lint | ||
| rake standard:fix # Auto-fix lint issues | ||
| bin/rails db:migrate # Run migrations | ||
| bin/rails suspenders:db:migrate # Migrate + annotate | ||
| bin/rails suspenders:cleanup:organize_gemfile # Sort Gemfile | ||
| bundle audit # Check gem vulnerabilities | ||
| bin/rails routes # View routes | ||
| ``` | ||
|
|
||
| ## Rules | ||
|
|
||
| Short constraints in .claude/rules/: | ||
|
|
||
| rules/models.md — models conventions | ||
| rules/controllers.md — controller conventions | ||
| rules/testing.md - **MUST write tests first** TDD guidelines | ||
| rules/security.md — security guidelines | ||
| rules/views.md — No logic in views, presenter usage, Turbo conventions | ||
| rules/database.md — Indexes, N+1, migration rules, query guidelines | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| # Controllers | ||
|
|
||
| - Controllers handle HTTP only: receive request, delegate to model, return response. | ||
| - Actions should not exceed 10 lines (excluding strong params). Longer actions often signal business logic that belongs in a model or PORO. | ||
| - Maximum one instance variable per action. | ||
| - No business logic, calculations, email sending, or multi-object operations in controllers. | ||
| - Always use strong parameters. Never `params.permit!`. | ||
| - Return `status: :unprocessable_entity` on failed form renders (required by Turbo). | ||
| - Prefer RESTful routes. Custom verb actions (e.g., post "activate") usually mean a missing noun/resource (e.g., resource :trial, only: [:create]). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| # Database & Migrations | ||
|
laicuRoot marked this conversation as resolved.
|
||
|
|
||
| - Always use the `rails generate migration` command to create migration files. | ||
| - Migrations must be reversible. | ||
| - Add `null: false` and database-level defaults where appropriate. | ||
| - Use `text` over `string` if length varies significantly. | ||
| - Wrap multi-record operations in transactions. Use `save!` (bang) inside transactions. | ||
| - Keep scopes as one-liners. Complex queries belong in search/query objects. | ||
| - Never use `Post.all` without pagination. | ||
| - Avoid `.count` in loops. | ||
| - Use `counter_cache`. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| # Models & Domain Objects | ||
|
|
||
| - No service objects. All domain classes live in `app/models/` with namespaces, never `app/services/`. | ||
| - Name classes after domain nouns, not actions. No `*Service`, `*Manager`, `*Handler` suffixes. | ||
| - Use `ActiveModel::Model` for POROs that need validation or form integration. | ||
| - Replace `.call` / `.perform` with domain verbs: `#save`, `#complete`, `#submit`, `#deliver`. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this just service objects by another name?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess it can be, if you're not thoughtful about it. Perhaps these rules oversimplify that what really matters is whether the class represents a genuine concept in the domain. That said, they've been useful for me when I'm working with Claude. With these rules Claude stop suggesting yet another service object when there are already plenty in the codebase. |
||
| - Look to identify domain models that can be extracted when an existing | ||
| model exceeds: 200 lines, 15 public methods, or 7 private methods. | ||
| - Callbacks only for data integrity (normalise fields, set defaults). Never for emails, payments, or external systems. | ||
| - Prefer composition over inheritance. Extract behaviour into small, focused objects. | ||
| - Avoid feature envy, long parameter lists (max 3 args), case statements on type, and mixin abuse. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| # Security | ||
|
|
||
| - Never interpolate user input into SQL. Use parameterised queries or `where(key: value)`. | ||
| - Always use strong parameters. Never `params.permit!`. | ||
| - Scope all queries to the current user or use Pundit authorisation. | ||
| - Every controller must have authentication unless explicitly public. | ||
| - Never use `raw`, `html_safe`, or `<%==` with user-supplied data. | ||
| - Never skip CSRF verification for browser-facing controllers. | ||
| - Filter sensitive params in logs: passwords, tokens, secrets, API keys. | ||
| - Never `render json: model` without explicit `only:` — whitelist attributes. | ||
| - Never redirect to `params[:return_to]` without validation. | ||
| - Use array form for system commands: `system("cmd", arg)`, never `system("cmd #{arg}")`. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| # Testing | ||
|
laicuRoot marked this conversation as resolved.
|
||
|
|
||
| - Must use TDD. Write tests first and follow red, green, refactor | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is your experience with this red, green, refactor process? How does your iterations look like if you have this process? I mentioned my TDD workflow in the Hub message as well.
Maybe this is more a general TDD question about how folks are doing it, so it might be out of scope for this PR. I do wonder how the agents work with this 3-step TDD process though.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From my own experience of having this line in my CLAUDE.md, certainly the red > green process works well. I don't know if I've observed or thought too much about the refactor step
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So, does the agent stop its editing process twice to go from red -> green -> refactor? Then you review it? Or it just uses this as a coding process for itself, without you reviewing it?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It does it automatically. You normally step in after the refactor.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've found that forcing red -> green causes agents to write higher quality tests. It's much harder to accidentally introduce a false positive. |
||
| - Must not use let or before in specs (avoid mystery guests). Do test setup | ||
| within each test. | ||
| - Test behaviour, not implementation. Four Phase Test: setup, exercise, verify, teardown. | ||
| - Test pyramid: many model/PORO unit specs, some request specs, few system specs. | ||
| - Every public method on every model and PORO must have at least one spec. | ||
|
laicuRoot marked this conversation as resolved.
|
||
| - Every branch in a conditional must have at least one spec. | ||
| - Use `build` / `build_stubbed` over `create` unless persistence is needed. | ||
| - Factories: only required attributes with sensible defaults. Start in `spec/factories.rb`. | ||
| - Shoulda Matchers for validations and associations. | ||
| - WebMock blocks all external HTTP in tests — always stub external requests. | ||
| - One `expect` per `it` block. Max 2 levels of context nesting. | ||
| - Never test private methods directly. Never stub the system under test. | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,7 @@ | ||||||
| # Views & Presenters | ||||||
|
|
||||||
| - Views render data. No calculations, queries, or complex conditionals. | ||||||
| - Use presenters to display logic. Instantiate in controller, use in view. | ||||||
| - Extract repeated markup into partials. Pass data via `locals:`, not instance variables. | ||||||
| - Helpers for simple formatting only (dates, currencies). If longer than 5 lines, use a presenter. | ||||||
| - Turbo: return `status: :unprocessable_entity` on failed forms. Keep Stimulus controllers small. | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's probably a Hotwire / Stimulus rule file to come in the future :)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rubocop's Rails/HttpStatusNameConsistency recommends usage of
Suggested change
|
||||||
Uh oh!
There was an error while loading. Please reload this page.