diff --git a/.agent_rules.md b/.agent_rules.md new file mode 100644 index 0000000..3554971 --- /dev/null +++ b/.agent_rules.md @@ -0,0 +1,332 @@ +# Rules for AI + +## 1. Code Quality + +- **Follow Rubocop**: Always consider our Rubocop rules when writing code. If uncertain about a style or convention, check `.rubocop.yml` or existing code patterns. + +## 2. Techniques & Standards + +### Ruby Style + +**Use Hash Value Omission**: For Ruby 3.1+ code, use hash value omission syntax when the key and value are the same. + +- **BAD**: `{ instance: instance, duration: duration }` +- **GOOD**: `{ instance:, duration: }` +- **BAD**: `create(:account, instance: instance, status: status)` +- **GOOD**: `create(:account, instance:, status:)` + +This applies to: + +- Method arguments +- Hash literals +- Factory definitions +- Any hash where keys match variable names + +**Avoid Ugly Multi-line Formatting**: When array or hash elements span multiple lines, put each element on its own line. Never create partial multi-line elements that result in awkward `,],` or `,),` patterns on the same line. + +- **BAD** - partial multi-line creates ugly `,],`: + + ```ruby + rows: [ + ['Label', format_number(value), + format_change(value, previous),], + ] + ``` + +- **GOOD** - each element on its own line: + + ```ruby + rows: [ + [ + 'Label', + format_number(value), + format_change(value, previous), + ], + ] + ``` + +- **ALSO GOOD** - single line if short enough: + ```ruby + rows: [ + ['Label', format_number(value), format_change(value, previous)], + ] + ``` + +Note: Trailing commas are fine (and enforced by Rubocop). The issue is the _formatting_ that creates visual clutter. + +### ActiveRecord Queries + +**Use Ruby Ranges**: Prefer Ruby range syntax over string comparisons for date/time and numeric ranges. + +- **BAD**: `.where('created_at > ?', 1.day.ago)` +- **GOOD**: `.where(created_at: 1.day.ago..)` (Use endless range for `>=`) +- **GOOD**: `.where(created_at: ..1.day.ago)` (Use beginless range for `<=`) +- **GOOD**: `.where(created_at: ...1.day.ago)` (Use range with 3 dots for `<`) + +### Exception Handling + +Always specify the exception class in rescue clauses: + +- **BAD**: `rescue => e` +- **GOOD**: `rescue StandardError => e` +- **BETTER**: `rescue SpecificError => e` + +Let errors propagate naturally unless you have a specific reason to catch them. + +### Database migrations + +Always use `rails generate migration` to create new migrations. Avoid creating migration files manually to ensure proper timestamps and naming conventions. + +## 3. Architecture + +**No Side Effects in Models**: Avoid putting side-effects (like sending notifications or enqueuing jobs) in models. These belong in the Service Layer / Interactors. + +## 4. Testing + +### Validation Testing + +**DO NOT** write specs that test basic ActiveRecord validations like: + +- `validates :field, presence: true` +- `validates :field, uniqueness: true` +- `validates :field, numericality: true` +- Other standard Rails validations + +#### Why? + +- These validations are tested by Rails itself +- They add maintenance overhead without adding value +- They make specs brittle to simple model changes + +#### What to test instead: + +- Complex custom validations with business logic +- Validation behavior in integration/request specs +- The actual user-facing behavior that relies on validations + +#### Examples: + +**BAD** - Testing simple validations: + +```ruby +describe Favorite do + it { is_expected.to validate_presence_of(:user_id) } + it { is_expected.to validate_uniqueness_of(:topic_id).scoped_to(:user_id) } +end +``` + +**GOOD** - Testing through behavior: + +```ruby +describe "POST /api/v1/favorites" do + subject(:request) do + post "/api/v1/favorites", params: { topic_id: } + end + + let(:user) { create(:user) } + let(:topic) { create(:topic) } + let(:topic_id) { topic.id } + + it "creates a favorite" do + expect { request }.to change(Favorite, :count).by(1) + + expect(response).to have_http_status(:created) + end +end +``` + +**GOOD** - Testing complex custom validations: + +```ruby +describe 'custom validation' do + subject(:request) { described_class.new(start_date:, end_date:) } + + context 'when both start_date and end_date are present' do + let(:start_date) { Date.today } + let(:end_date) { Date.yesterday } + + it 'requires end_date to be after start_date' do + expect(request).not_to be_valid + expect(request.errors[:end_date]).to include('must be after start date') + end + end +end +``` + +### Mocks and Spies + +Prefer using `instance_spy` and the `expect(...).to receive` style for verification over `have_received`. This mocking style is generally more readable and separates setup from verification effectively. + +Use `instance_spy` primarily when you stub the initializer to return the spy. This allows you to avoid defining the interface of a double explicitly, as spies allow any message. + +**BAD**: + +```ruby +let(:client) { instance_double(ClientClass, call: nil) } + +before do + allow(ClientClass).to receive(:new).and_return(client) +end + +it 'calls the client' do + subject.perform + expect(client).to have_received(:call) +end +``` + +**GOOD**: + +```ruby +let(:client) { instance_spy(ClientClass) } + +before do + allow(ClientClass).to receive(:new).and_return(client) +end + +it 'calls the client' do + expect(client).to receive(:call) + subject.perform +end +``` + +#### For Class Methods + +For class methods, set expectations directly before the action - do not use `allow` + `have_received`. + +**BAD**: + +```ruby +before do + allow(UserActivityLog).to receive(:create!) +end + +it "logs user activity" do + action.call + + expect(UserActivityLog).to have_received(:create!).with( + user:, topic:, action_type: :view, + ) +end +``` + +**GOOD**: + +```ruby +it "logs user activity" do + expect(UserActivityLog).to receive(:create!).with( + user:, topic:, action_type: :view, + ) + + action.call +end +``` + +### Use Contexts for Conditional Scenarios + +Always use `context` blocks to organize specs by conditions or states. This makes specs more readable and clearly separates different scenarios. + +**Use `context` for:** + +- Different states: `context 'when user is authenticated'` +- Different inputs: `context 'with valid params'` +- Different configurations: `context 'with feature flag enabled'` +- Error conditions: `context 'when API fails'` +- Variations: `context 'with force: true'` + +**BAD**: + +```ruby +describe "#call" do + it "returns error for missing topic" do + # ... + end + + it "creates favorite for authenticated user" do + user.update!(login_count: 1) + # ... + end + + it "logs activity when user is present" do + # ... + end +end +``` + +**GOOD**: + +```ruby +describe "#call" do + context "when topic does not exist" do + it "returns an error" do + # ... + end + end + + context "with authenticated user" do + let(:user) { create(:user, login_count: 1) } + + it "creates a favorite" do + # ... + end + + it "logs the favorite action" do + # ... + end + end + + context "when database error occurs" do + before do + allow(Favorite).to receive(:create!).and_raise(ActiveRecord::RecordInvalid) + end + + it "captures exception and returns error" do + # ... + end + end +end +``` + +**Benefits:** + +- Clear separation of different scenarios +- Easier to locate specific test cases +- Better test organization and readability +- `let` blocks can be scoped to specific contexts + +### Side-Effect Records in Before Blocks + +When creating records that exist only for their side effects (not directly referenced in tests), use `before` blocks instead of `let!`. This keeps `let` blocks clean and focused on records that are actually referenced. + +**BAD** - Creating side-effect records in `let!`: + +```ruby +let!(:topic_with_activity) do + topic = create(:topic, content_provider:) + create(:user_activity_log, topic:, user:, action_type: :view) + topic +end +``` + +**GOOD** - Separate the side-effect into a `before` block: + +```ruby +let!(:topic_with_activity) do + create(:topic, content_provider:) +end + +before do + create(:user_activity_log, topic: topic_with_activity, user:, action_type: :view) +end +``` + +**Why?** + +- `let` blocks should return the record they're named after +- Side effects hidden inside `let` blocks are harder to understand +- `before` blocks clearly signal "setup that happens before tests" +- Makes test data setup more explicit and readable + +## 5. Workflow + +- **Run DB migrations**: Make sure to always run any pending database migrations with `rails db:migrate RAILS_ENV=test` command before running tests. +- **Run Verification**: Once a chunk of work is completed, always run both Rubocop checks and relevant tests to ensure no regressions or style violations. diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 9e27b13..902f224 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -1,3 +1,4 @@ # Claude AI Coding Rules Follow the coding standards defined in `.agent_rules.md` for all code changes. +Also follow developer specific standards defined in `.agent_rules.local.md` for all code changes. diff --git a/.gitignore b/.gitignore index 4ad4ee8..7931b77 100644 --- a/.gitignore +++ b/.gitignore @@ -37,5 +37,3 @@ /app/assets/builds/* !/app/assets/builds/.keep - -.agent_rules.md