[2.16.0] Consolidate DHH styles and add /feature-video command

- Merge dhh-ruby-style into dhh-rails-style for comprehensive Rails conventions - Add testing.md reference covering Rails testing patterns - Add /feature-video command for recording PR demo videos - Update docs and component counts 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-01-05 09:36:56 -08:00
parent 6d8c8675f3
commit 141bbb42cb
12 changed files with 845 additions and 1274 deletions
--- a/plugins/compound-engineering/skills/dhh-rails-style/SKILL.md
+++ b/plugins/compound-engineering/skills/dhh-rails-style/SKILL.md
@@ -4,7 +4,7 @@ description: This skill should be used when writing Ruby and Rails code in DHH's
 ---

 <objective>
-Apply 37signals/DHH Rails conventions to Ruby and Rails code. This skill provides domain expertise extracted from analyzing production 37signals codebases (Fizzy/Campfire).
+Apply 37signals/DHH Rails conventions to Ruby and Rails code. This skill provides comprehensive domain expertise extracted from analyzing production 37signals codebases (Fizzy/Campfire) and DHH's code review patterns.
 </objective>

 <essential_principles>
@@ -27,17 +27,28 @@ Apply 37signals/DHH Rails conventions to Ruby and Rails code. This skill provide
 - redis (database for everything)
 - view_component (partials work fine)
 - GraphQL (REST with Turbo sufficient)
+- factory_bot (fixtures are simpler)
+- rspec (Minitest ships with Rails)
+- Tailwind (native CSS with layers)
+
+**Development Philosophy:**
+- Ship, Validate, Refine - prototype-quality code to production to learn
+- Fix root causes, not symptoms
+- Write-time operations over read-time computations
+- Database constraints over ActiveRecord validations
 </essential_principles>

 <intake>
 What are you working on?

-1. **Controllers** - REST mapping, concerns, Turbo responses
-2. **Models** - Concerns, state records, callbacks, scopes
+1. **Controllers** - REST mapping, concerns, Turbo responses, API patterns
+2. **Models** - Concerns, state records, callbacks, scopes, POROs
 3. **Views & Frontend** - Turbo, Stimulus, CSS, partials
-4. **Architecture** - Routing, multi-tenancy, authentication, jobs
-5. **Code Review** - Review code against DHH style
-6. **General Guidance** - Philosophy and conventions
+4. **Architecture** - Routing, multi-tenancy, authentication, jobs, caching
+5. **Testing** - Minitest, fixtures, integration tests
+6. **Gems & Dependencies** - What to use vs avoid
+7. **Code Review** - Review code against DHH style
+8. **General Guidance** - Philosophy and conventions

 **Specify a number or describe your task.**
 </intake>
@@ -48,9 +59,11 @@ What are you working on?
 | 1, "controller" | [controllers.md](./references/controllers.md) |
 | 2, "model" | [models.md](./references/models.md) |
 | 3, "view", "frontend", "turbo", "stimulus", "css" | [frontend.md](./references/frontend.md) |
-| 4, "architecture", "routing", "auth", "job" | [architecture.md](./references/architecture.md) |
-| 5, "review" | Read all references, then review code |
-| 6, general task | Read relevant references based on context |
+| 4, "architecture", "routing", "auth", "job", "cache" | [architecture.md](./references/architecture.md) |
+| 5, "test", "testing", "minitest", "fixture" | [testing.md](./references/testing.md) |
+| 6, "gem", "dependency", "library" | [gems.md](./references/gems.md) |
+| 7, "review" | Read all references, then review code |
+| 8, general task | Read relevant references based on context |

 **After reading relevant references, apply patterns to the user's code.**
 </routing>
@@ -70,6 +83,7 @@ What are you working on?
 - `chronologically`, `reverse_chronologically`, `alphabetically`, `latest`
 - `preloaded` (standard eager loading name)
 - `indexed_by`, `sorted_by` (parameterized)
+- `active`, `unassigned` (business terms, not SQL-ish)

 ## REST Mapping

@@ -80,6 +94,55 @@ POST /cards/:id/close    → POST /cards/:id/closure
 DELETE /cards/:id/close  → DELETE /cards/:id/closure
 POST /cards/:id/archive  → POST /cards/:id/archival
 ```
+
+## Ruby Syntax Preferences
+
+```ruby
+# Symbol arrays with spaces inside brackets
+before_action :set_message, only: %i[ show edit update destroy ]
+
+# Private method indentation
+  private
+    def set_message
+      @message = Message.find(params[:id])
+    end
+
+# Expression-less case for conditionals
+case
+when params[:before].present?
+  messages.page_before(params[:before])
+else
+  messages.last_page
+end
+
+# Bang methods for fail-fast
+@message = Message.create!(params)
+
+# Ternaries for simple conditionals
+@room.direct? ? @room.users : @message.mentionees
+```
+
+## Key Patterns
+
+**State as Records:**
+```ruby
+Card.joins(:closure)         # closed cards
+Card.where.missing(:closure) # open cards
+```
+
+**Current Attributes:**
+```ruby
+belongs_to :creator, default: -> { Current.user }
+```
+
+**Authorization on Models:**
+```ruby
+class User < ApplicationRecord
+  def can_administer?(message)
+    message.creator == self || admin?
+  end
+end
+```
 </quick_reference>

 <reference_index>
@@ -89,11 +152,12 @@ All detailed patterns in `references/`:

 | File | Topics |
 |------|--------|
-| [controllers.md](./references/controllers.md) | REST mapping, concerns, Turbo responses, API patterns |
-| [models.md](./references/models.md) | Concerns, state records, callbacks, scopes, POROs |
-| [frontend.md](./references/frontend.md) | Turbo, Stimulus, CSS architecture, view patterns |
-| [architecture.md](./references/architecture.md) | Routing, auth, jobs, caching, multi-tenancy, config |
-| [gems.md](./references/gems.md) | What they use vs avoid, and why |
+| [controllers.md](./references/controllers.md) | REST mapping, concerns, Turbo responses, API patterns, HTTP caching |
+| [models.md](./references/models.md) | Concerns, state records, callbacks, scopes, POROs, authorization, broadcasting |
+| [frontend.md](./references/frontend.md) | Turbo Streams, Stimulus controllers, CSS layers, OKLCH colors, partials |
+| [architecture.md](./references/architecture.md) | Routing, authentication, jobs, Current attributes, caching, database patterns |
+| [testing.md](./references/testing.md) | Minitest, fixtures, unit/integration/system tests, testing patterns |
+| [gems.md](./references/gems.md) | What they use vs avoid, decision framework, Gemfile examples |
 </reference_index>

 <success_criteria>
@@ -105,8 +169,16 @@ Code follows DHH style when:
 - Database-backed solutions preferred over external services
 - Tests use Minitest with fixtures
 - Turbo/Stimulus for interactivity (no heavy JS frameworks)
+- Native CSS with modern features (layers, OKLCH, nesting)
+- Authorization logic lives on User model
+- Jobs are shallow wrappers calling model methods
 </success_criteria>

 <credits>
-Based on [The Unofficial 37signals/DHH Rails Style Guide](https://gist.github.com/marckohlbrugge/d363fb90c89f71bd0c816d24d7642aca) by [Marc Köhlbrugge](https://x.com/marckohlbrugge), generated through deep analysis of the Fizzy codebase.
+Based on [The Unofficial 37signals/DHH Rails Style Guide](https://github.com/marckohlbrugge/unofficial-37signals-coding-style-guide) by [Marc Köhlbrugge](https://x.com/marckohlbrugge), generated through deep analysis of 265 pull requests from the Fizzy codebase.
+
+**Important Disclaimers:**
+- LLM-generated guide - may contain inaccuracies
+- Code examples from Fizzy are licensed under the O'Saasy License
+- Not affiliated with or endorsed by 37signals
 </credits>
--- a/plugins/compound-engineering/skills/dhh-rails-style/references/testing.md
+++ b/plugins/compound-engineering/skills/dhh-rails-style/references/testing.md
@@ -0,0 +1,338 @@
+# Testing - DHH Rails Style
+
+## Core Philosophy
+
+"Minitest with fixtures - simple, fast, deterministic." The approach prioritizes pragmatism over convention.
+
+## Why Minitest Over RSpec
+
+- **Simpler**: Less DSL magic, plain Ruby assertions
+- **Ships with Rails**: No additional dependencies
+- **Faster boot times**: Less overhead
+- **Plain Ruby**: No specialized syntax to learn
+
+## Fixtures as Test Data
+
+Rather than factories, fixtures provide preloaded data:
+- Loaded once, reused across tests
+- No runtime object creation overhead
+- Explicit relationship visibility
+- Deterministic IDs for easier debugging
+
+### Fixture Structure
+```yaml
+# test/fixtures/users.yml
+david:
+  identity: david
+  account: basecamp
+  role: admin
+
+jason:
+  identity: jason
+  account: basecamp
+  role: member
+
+# test/fixtures/rooms.yml
+watercooler:
+  name: Water Cooler
+  creator: david
+  direct: false
+
+# test/fixtures/messages.yml
+greeting:
+  body: Hello everyone!
+  room: watercooler
+  creator: david
+```
+
+### Using Fixtures in Tests
+```ruby
+test "sending a message" do
+  user = users(:david)
+  room = rooms(:watercooler)
+
+  # Test with fixture data
+end
+```
+
+### Dynamic Fixture Values
+ERB enables time-sensitive data:
+```yaml
+recent_card:
+  title: Recent Card
+  created_at: <%= 1.hour.ago %>
+
+old_card:
+  title: Old Card
+  created_at: <%= 1.month.ago %>
+```
+
+## Test Organization
+
+### Unit Tests
+Verify business logic using setup blocks and standard assertions:
+
+```ruby
+class CardTest < ActiveSupport::TestCase
+  setup do
+    @card = cards(:one)
+    @user = users(:david)
+  end
+
+  test "closing a card creates a closure" do
+    assert_difference -> { Card::Closure.count } do
+      @card.close(creator: @user)
+    end
+
+    assert @card.closed?
+    assert_equal @user, @card.closure.creator
+  end
+
+  test "reopening a card destroys the closure" do
+    @card.close(creator: @user)
+
+    assert_difference -> { Card::Closure.count }, -1 do
+      @card.reopen
+    end
+
+    refute @card.closed?
+  end
+end
+```
+
+### Integration Tests
+Test full request/response cycles:
+
+```ruby
+class CardsControllerTest < ActionDispatch::IntegrationTest
+  setup do
+    @user = users(:david)
+    sign_in @user
+  end
+
+  test "closing a card" do
+    card = cards(:one)
+
+    post card_closure_path(card)
+
+    assert_response :success
+    assert card.reload.closed?
+  end
+
+  test "unauthorized user cannot close card" do
+    sign_in users(:guest)
+    card = cards(:one)
+
+    post card_closure_path(card)
+
+    assert_response :forbidden
+    refute card.reload.closed?
+  end
+end
+```
+
+### System Tests
+Browser-based tests using Capybara:
+
+```ruby
+class MessagesTest < ApplicationSystemTestCase
+  test "sending a message" do
+    sign_in users(:david)
+    visit room_path(rooms(:watercooler))
+
+    fill_in "Message", with: "Hello, world!"
+    click_button "Send"
+
+    assert_text "Hello, world!"
+  end
+
+  test "editing own message" do
+    sign_in users(:david)
+    visit room_path(rooms(:watercooler))
+
+    within "#message_#{messages(:greeting).id}" do
+      click_on "Edit"
+    end
+
+    fill_in "Message", with: "Updated message"
+    click_button "Save"
+
+    assert_text "Updated message"
+  end
+
+  test "drag and drop card to new column" do
+    sign_in users(:david)
+    visit board_path(boards(:main))
+
+    card = find("#card_#{cards(:one).id}")
+    target = find("#column_#{columns(:done).id}")
+
+    card.drag_to target
+
+    assert_selector "#column_#{columns(:done).id} #card_#{cards(:one).id}"
+  end
+end
+```
+
+## Advanced Patterns
+
+### Time Testing
+Use `travel_to` for deterministic time-dependent assertions:
+
+```ruby
+test "card expires after 30 days" do
+  card = cards(:one)
+
+  travel_to 31.days.from_now do
+    assert card.expired?
+  end
+end
+```
+
+### External API Testing with VCR
+Record and replay HTTP interactions:
+
+```ruby
+test "fetches user data from API" do
+  VCR.use_cassette("user_api") do
+    user_data = ExternalApi.fetch_user(123)
+
+    assert_equal "John", user_data[:name]
+  end
+end
+```
+
+### Background Job Testing
+Assert job enqueueing and email delivery:
+
+```ruby
+test "closing card enqueues notification job" do
+  card = cards(:one)
+
+  assert_enqueued_with(job: NotifyWatchersJob, args: [card]) do
+    card.close
+  end
+end
+
+test "welcome email is sent on signup" do
+  assert_emails 1 do
+    Identity.create!(email: "new@example.com")
+  end
+end
+```
+
+### Testing Turbo Streams
+```ruby
+test "message creation broadcasts to room" do
+  room = rooms(:watercooler)
+
+  assert_turbo_stream_broadcasts [room, :messages] do
+    room.messages.create!(body: "Test", creator: users(:david))
+  end
+end
+```
+
+## Testing Principles
+
+### 1. Test Observable Behavior
+Focus on what the code does, not how it does it:
+
+```ruby
+# ❌ Testing implementation
+test "calls notify method on each watcher" do
+  card.expects(:notify).times(3)
+  card.close
+end
+
+# ✅ Testing behavior
+test "watchers receive notifications when card closes" do
+  assert_difference -> { Notification.count }, 3 do
+    card.close
+  end
+end
+```
+
+### 2. Don't Mock Everything
+
+```ruby
+# ❌ Over-mocked test
+test "sending message" do
+  room = mock("room")
+  user = mock("user")
+  message = mock("message")
+
+  room.expects(:messages).returns(stub(create!: message))
+  message.expects(:broadcast_create)
+
+  MessagesController.new.create
+end
+
+# ✅ Test the real thing
+test "sending message" do
+  sign_in users(:david)
+  post room_messages_url(rooms(:watercooler)),
+    params: { message: { body: "Hello" } }
+
+  assert_response :success
+  assert Message.exists?(body: "Hello")
+end
+```
+
+### 3. Tests Ship with Features
+Same commit, not TDD-first but together. Neither before (strict TDD) nor after (deferred testing).
+
+### 4. Security Fixes Always Include Regression Tests
+Every security fix must include a test that would have caught the vulnerability.
+
+### 5. Integration Tests Validate Complete Workflows
+Don't just test individual pieces - test that they work together.
+
+## File Organization
+
+```
+test/
+├── controllers/         # Integration tests for controllers
+├── fixtures/           # YAML fixtures for all models
+├── helpers/            # Helper method tests
+├── integration/        # API integration tests
+├── jobs/               # Background job tests
+├── mailers/            # Mailer tests
+├── models/             # Unit tests for models
+├── system/             # Browser-based system tests
+└── test_helper.rb      # Test configuration
+```
+
+## Test Helper Setup
+
+```ruby
+# test/test_helper.rb
+ENV["RAILS_ENV"] ||= "test"
+require_relative "../config/environment"
+require "rails/test_help"
+
+class ActiveSupport::TestCase
+  fixtures :all
+
+  parallelize(workers: :number_of_processors)
+end
+
+class ActionDispatch::IntegrationTest
+  include SignInHelper
+end
+
+class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
+  driven_by :selenium, using: :headless_chrome
+end
+```
+
+## Sign In Helper
+
+```ruby
+# test/support/sign_in_helper.rb
+module SignInHelper
+  def sign_in(user)
+    session = user.identity.sessions.create!
+    cookies.signed[:session_id] = session.id
+  end
+end
+```