review-ruby-code — Skillopedia

Ruby Code Review Review Ruby/Rails code changes against Sandi Metz rules, SOLID principles, Rails best practices, and security standards. Generate a structured REVIEW.md with clickable VSCode links. Workflow 1. Detect scope If not on a feature branch, review files specified by the user. 2. Run analysis tools Parse rubycritic JSON for complexity/smells/duplication. Read for per-file coverage and uncovered lines. If tools aren't configured, invoke their respective skills for setup guidance. Optionally run the bundled static analyzer: 3. Analyze each changed file Review in this order for each fi…

\n```\n\nIf not on a feature branch, review files specified by the user.\n\n### 2. Run analysis tools\n\n```bash\n# RubyCritic on changed files\nrubycritic --format json --no-browser $(git diff --name-only base...HEAD | grep '\\.rb

Important: agents should read /llm.txt, /llms.txt, or /.well-known/skills.json to discover the public Skillopedia API.

)\n\n# SimpleCov coverage run\nCOVERAGE=true bundle exec rspec\n```\n\nParse rubycritic JSON for complexity/smells/duplication. Read `coverage/.resultset.json` for per-file coverage and uncovered lines. If tools aren't configured, invoke their respective skills for setup guidance.\n\nOptionally run the bundled static analyzer:\n```bash\nruby scripts/code_reviewer.rb \u003cfile.rb>\n```\n\n### 3. Analyze each changed file\n\nReview in this order for each file:\n\n**OOP Design** — Apply Sandi Metz rules and SOLID principles:\n- Classes ≤ 100 lines, methods ≤ 5 lines, parameters ≤ 4, instance variables ≤ 4\n- Controllers instantiate ≤ 1 object, views reference ≤ 1 instance variable\n- SRP, Open/Closed, Liskov, Interface Segregation, Dependency Inversion\n- Law of Demeter (\"only talk to immediate friends\")\n- Tell, Don't Ask (objects make their own decisions)\n- See [references/sandi-metz-rules.md](references/sandi-metz-rules.md) and [references/solid-principles.md](references/solid-principles.md)\n\n**Code Smells** — Check for the 18 canonical smells:\n- Structural: Long Method, Large Class, Long Parameter List, Data Clump\n- Coupling: Feature Envy, Message Chains, Inappropriate Intimacy\n- Conditional: Complex conditionals, case statements (polymorphism candidates), speculative generality\n- Naming: Vague names (Manager, Handler, Processor), methods with \"and\", flag parameters\n- See [references/sandi-metz-rules.md](references/sandi-metz-rules.md) (Code Smells section)\n\n**Rails Patterns** — Detect anti-patterns:\n- N+1 queries (missing `includes`/`preload`/`eager_load`)\n- Callback overuse (prefer service objects for side effects)\n- Fat models (extract to services, queries, presenters, concerns)\n- Business logic in controllers\n- Missing database indexes\n- See [references/rails-patterns.md](references/rails-patterns.md)\n\n**Security** — Flag vulnerabilities:\n- SQL injection (string interpolation in queries)\n- XSS (`html_safe`/`raw` on user input)\n- Mass assignment (missing strong parameters, `permit!`)\n- Authorization gaps (missing checks, inconsistent patterns)\n- See [references/security-checklist.md](references/security-checklist.md)\n\n**Test Coverage** — Cross-reference with simplecov:\n- Untested methods and uncovered lines\n- Missing edge case and error path coverage\n- Test quality (implementation vs behavior testing, excessive mocking)\n\n### 4. Check codebase patterns\n\nBefore making suggestions, understand existing patterns:\n\n```bash\nls app/services/ app/queries/ app/decorators/ app/presenters/ app/policies/ 2>/dev/null\n```\n\nEnsure recommendations align with established patterns (naming conventions, abstraction layers, test framework usage). Don't suggest decorators if the codebase uses presenters.\n\n### 5. Generate REVIEW.md\n\nEvery code reference MUST use VSCode-compatible links:\n```markdown\n[description](file:///absolute/path/to/file.rb#L42)\n```\nSee [references/vscode-links.md](references/vscode-links.md) for format details.\n\nUse severity levels for findings:\n- **Error**: Serious violations (security, accessing internals, tight coupling)\n- **Warning**: Rule violations that should be fixed\n- **Info**: Suggestions and best practices\n- **Pass**: Correctly following principles\n\n```markdown\n# Code Review - [Branch Name]\n\n**Base Branch**: [detected-branch]\n**Changed Files**: [count]\n**Review Date**: [date]\n\n---\n\n## Summary\n\n[High-level overview of changes and main findings]\n\n## Critical Issues\n\n[Security vulnerabilities, major bugs requiring immediate attention]\n\n## Design & Architecture\n\n### OOP Violations\n[Sandi Metz rule and SOLID violations with VSCode links and severity]\n\n### Code Smells\n[Detected smells with specific refactoring suggestions]\n\n### Rails Patterns\n[N+1 queries, callback issues, anti-patterns with VSCode links]\n\n## Security Concerns\n\n[Vulnerabilities with VSCode links]\n\n## Test Coverage\n\n[Coverage gaps, missing tests, quality issues with VSCode links]\n\n## Tool Reports\n\n### RubyCritic Summary\n- **Complexity**: [score]\n- **Duplication**: [score]\n- **Code Smells**: [count]\n\n### SimpleCov Summary\n- **Total Coverage**: [percentage]\n- **Files with \u003c 90% coverage**: [list]\n\n---\n\n## Recommendations\n\n[Prioritized improvements aligned with codebase patterns]\n\n## Positive Observations\n\n[Well-designed code, good patterns, improvements from previous reviews]\n```\n\n### 6. Validate\n\nBefore finalizing:\n- [ ] Every code reference has a clickable VSCode link with absolute path\n- [ ] All changed files reviewed\n- [ ] RubyCritic and SimpleCov findings incorporated\n- [ ] Suggestions match existing codebase patterns\n- [ ] Positive observations included\n\n## Reference Guides\n\n- [references/sandi-metz-rules.md](references/sandi-metz-rules.md) — Five rules, Law of Demeter, Tell Don't Ask, code smells, Shameless Green philosophy\n- [references/solid-principles.md](references/solid-principles.md) — SOLID principles with Ruby examples\n- [references/rails-patterns.md](references/rails-patterns.md) — Rails anti-patterns, N+1 queries, callbacks, service objects\n- [references/security-checklist.md](references/security-checklist.md) — SQL injection, XSS, mass assignment, auth vulnerabilities\n- [references/vscode-links.md](references/vscode-links.md) — VSCode link format and examples\n---","attachment_filenames":["references/rails-patterns.md","references/sandi-metz-rules.md","references/security-checklist.md","references/solid-principles.md","references/vscode-links.md"],"attachments":[{"filename":"references/rails-patterns.md","content":"# Rails Patterns and Anti-Patterns Reference\n\nThis guide covers Rails-specific patterns, anti-patterns, and performance issues to check during code reviews.\n\n## N+1 Query Detection and Prevention\n\n**What it is**: Making one query to get a collection, then making additional queries for each item in the collection.\n\n### Detection Strategies\n\n**In code review, look for**:\n```ruby\n# RED FLAG: Accessing associations in loops\[email protected] do |post|\n post.author.name # Queries author for EACH post\n post.comments.count # Queries comments for EACH post\nend\n\n# RED FLAG: Iteration with nested associations\nusers.map { |user| user.profile.avatar_url }\n\n# RED FLAG: In views without eager loading\n\u003c% @articles.each do |article| %>\n \u003c%= article.user.name %> \u003c!-- N+1 here -->\n\u003c% end %>\n```\n\n**Runtime detection**:\n```ruby\n# Add to development.rb to get warnings\nconfig.after_initialize do\n Bullet.enable = true\n Bullet.alert = true\n Bullet.bullet_logger = true\n Bullet.console = true\nend\n```\n\n### Solutions\n\n**Use `includes` for eager loading**:\n```ruby\n# Bad - N+1 query\n@posts = Post.all\[email protected] { |post| post.author.name } # 1 + N queries\n\n# Good - eager load with includes\n@posts = Post.includes(:author)\[email protected] { |post| post.author.name } # 2 queries total\n```\n\n**Use `preload` for separate queries**:\n```ruby\n# Loads associations in separate queries (better for large datasets)\nPost.preload(:author, :comments)\n```\n\n**Use `eager_load` for LEFT OUTER JOIN**:\n```ruby\n# Loads everything in one query with LEFT OUTER JOIN\nPost.eager_load(:author).where(authors: { active: true })\n```\n\n**Use `joins` for filtering only** (doesn't load association):\n```ruby\n# Only for WHERE clauses, doesn't prevent N+1\nPost.joins(:author).where(authors: { country: 'US' })\n\n# Still need includes if accessing association\nPost.joins(:author).includes(:author).where(authors: { country: 'US' })\n```\n\n**Nested associations**:\n```ruby\n# Bad - N+1 on nested associations\nPost.includes(:comments)\n# Accessing comment.author still causes N+1\n\n# Good - eager load nested associations\nPost.includes(comments: :author)\n```\n\n**Counter caches for counts**:\n```ruby\n# Bad - counts cause N+1\[email protected] { |post| post.comments.count }\n\n# Good - use counter cache\nclass Comment \u003c ApplicationRecord\n belongs_to :post, counter_cache: true\nend\n\n# Migration to add counter cache\nadd_column :posts, :comments_count, :integer, default: 0\nPost.find_each { |post| Post.reset_counters(post.id, :comments) }\n\n# Now this is free\[email protected] { |post| post.comments_count } # No queries!\n```\n\n---\n\n## Callback Anti-Patterns\n\n**Problem**: Callbacks make code hard to understand, test, and maintain. They create hidden dependencies and side effects.\n\n### Common Callback Issues\n\n**1. Side effects in callbacks**:\n```ruby\n# Bad - email sending in callback\nclass User \u003c ApplicationRecord\n after_create :send_welcome_email\n\n private\n\n def send_welcome_email\n UserMailer.welcome_email(self).deliver_now\n # What if we're creating users in a seed script?\n # What if we're importing bulk users?\n # What if mail server is down?\n end\nend\n```\n\n**2. Callbacks calling external services**:\n```ruby\n# Bad - external API calls in callbacks\nclass Order \u003c ApplicationRecord\n after_save :sync_to_crm\n\n private\n\n def sync_to_crm\n CrmApi.update_order(self) # Slows down every save\n end\nend\n```\n\n**3. Callbacks updating other models**:\n```ruby\n# Bad - touching other models in callbacks\nclass Comment \u003c ApplicationRecord\n after_create :update_post_stats\n\n private\n\n def update_post_stats\n post.increment!(:comment_count)\n post.touch(:last_commented_at)\n # Creates complex dependencies\n end\nend\n```\n\n### Better Alternatives\n\n**Use service objects for complex operations**:\n```ruby\n# Good - explicit service object\nclass UserCreationService\n def self.call(params)\n user = User.create!(params)\n UserMailer.welcome_email(user).deliver_later\n AnalyticsService.track_signup(user)\n user\n end\nend\n\n# Clear when side effects happen\nUserCreationService.call(user_params)\n```\n\n**Use background jobs for async operations**:\n```ruby\n# Good - background job\nclass User \u003c ApplicationRecord\n # No callbacks for side effects\nend\n\nclass UserCreationService\n def self.call(params)\n user = User.create!(params)\n WelcomeEmailJob.perform_later(user.id)\n CrmSyncJob.perform_later(user.id)\n user\n end\nend\n```\n\n**When callbacks are acceptable**:\n```ruby\n# OK - setting default values\nclass User \u003c ApplicationRecord\n before_validation :normalize_email\n\n private\n\n def normalize_email\n self.email = email.downcase.strip if email.present?\n end\nend\n\n# OK - maintaining data consistency within same record\nclass Post \u003c ApplicationRecord\n before_save :generate_slug\n\n private\n\n def generate_slug\n self.slug = title.parameterize if slug.blank?\n end\nend\n```\n\n**Use ActiveRecord callbacks sparingly**:\n- `before_validation`: Normalize/sanitize data\n- `before_save`: Set calculated fields\n- `after_commit`: Safer for side effects (transaction-aware)\n\n**Avoid**:\n- `after_create`, `after_save` with side effects\n- Callbacks that call other models\n- Callbacks with external API calls\n- Callbacks with email sending\n\n---\n\n## Fat Model Anti-Pattern\n\n**Problem**: Models with too many responsibilities become hard to maintain, test, and understand.\n\n### Signs of Fat Models\n\n```ruby\n# Bad - model doing everything\nclass User \u003c ApplicationRecord\n # 500+ lines\n\n # Validations (OK)\n validates :email, presence: true\n\n # Associations (OK)\n has_many :posts\n\n # Business logic (should be extracted)\n def calculate_reputation_score\n # 50 lines of complex logic\n end\n\n # Presentation logic (should be in presenter)\n def display_name\n \"#{first_name} #{last_name}\"\n end\n\n # External API integration (should be in service)\n def sync_to_mailchimp\n # Mailchimp API calls\n end\n\n # Complex queries (should be in query object)\n def self.active_premium_users_in_region(region)\n # Complex query logic\n end\n\n # Email sending (should be in service)\n def send_welcome_sequence\n # Email logic\n end\n\n # Report generation (should be in service)\n def generate_activity_report\n # Report logic\n end\nend\n```\n\n### Refactoring Strategies\n\n**1. Extract to Service Objects**:\n```ruby\n# app/services/user_reputation_calculator.rb\nclass UserReputationCalculator\n def initialize(user)\n @user = user\n end\n\n def calculate\n # Complex reputation logic\n end\nend\n\n# app/services/user_mailchimp_sync.rb\nclass UserMailchimpSync\n def initialize(user)\n @user = user\n end\n\n def sync\n # Mailchimp API logic\n end\nend\n```\n\n**2. Extract to Query Objects**:\n```ruby\n# app/queries/user_query.rb\nclass UserQuery\n def initialize(relation = User.all)\n @relation = relation\n end\n\n def active_premium_in_region(region)\n @relation\n .where(status: 'active')\n .where(premium: true)\n .where(region: region)\n end\nend\n\n# Usage\nUserQuery.new.active_premium_in_region('US')\n```\n\n**3. Extract to Presenters/Decorators**:\n```ruby\n# app/presenters/user_presenter.rb\nclass UserPresenter\n def initialize(user)\n @user = user\n end\n\n def display_name\n \"#{@user.first_name} #{@user.last_name}\"\n end\n\n def avatar_url\n @user.avatar.present? ? @user.avatar.url : default_avatar_url\n end\nend\n```\n\n**4. Extract to Concerns** (for shared behavior):\n```ruby\n# app/models/concerns/sluggable.rb\nmodule Sluggable\n extend ActiveSupport::Concern\n\n included do\n before_save :generate_slug\n end\n\n private\n\n def generate_slug\n self.slug = send(slug_source).parameterize if slug.blank?\n end\n\n def slug_source\n :title # Override in including class if needed\n end\nend\n\nclass Post \u003c ApplicationRecord\n include Sluggable\nend\n```\n\n---\n\n## Service Object Patterns\n\nService objects extract business logic from models and controllers.\n\n### When to Use Service Objects\n\n- Complex multi-step operations\n- Operations involving multiple models\n- External API integrations\n- Business logic that doesn't fit in a model\n- Operations with side effects (emails, jobs, etc.)\n\n### Service Object Structure\n\n```ruby\n# Simple structure\nclass UserRegistrationService\n def self.call(params)\n new(params).call\n end\n\n def initialize(params)\n @params = params\n end\n\n def call\n create_user\n send_welcome_email\n track_analytics\n user\n end\n\n private\n\n attr_reader :params, :user\n\n def create_user\n @user = User.create!(user_params)\n end\n\n def send_welcome_email\n UserMailer.welcome_email(user).deliver_later\n end\n\n def track_analytics\n AnalyticsService.track('user_registered', user_id: user.id)\n end\n\n def user_params\n params.require(:user).permit(:email, :name)\n end\nend\n```\n\n### Result Object Pattern\n\n```ruby\n# app/services/result.rb\nclass Result\n attr_reader :value, :error\n\n def initialize(success:, value: nil, error: nil)\n @success = success\n @value = value\n @error = error\n end\n\n def success?\n @success\n end\n\n def failure?\n !@success\n end\n\n def self.success(value = nil)\n new(success: true, value: value)\n end\n\n def self.failure(error)\n new(success: false, error: error)\n end\nend\n\n# Service using Result\nclass OrderProcessingService\n def self.call(order_params)\n new(order_params).call\n end\n\n def initialize(order_params)\n @order_params = order_params\n end\n\n def call\n order = create_order\n return Result.failure(order.errors) unless order.persisted?\n\n charge_result = charge_payment(order)\n return Result.failure(charge_result.error) if charge_result.failure?\n\n Result.success(order)\n end\n\n private\n\n def create_order\n Order.create(@order_params)\n end\n\n def charge_payment(order)\n PaymentService.charge(order)\n end\nend\n\n# Usage in controller\ndef create\n result = OrderProcessingService.call(order_params)\n\n if result.success?\n redirect_to result.value\n else\n @errors = result.error\n render :new\n end\nend\n```\n\n---\n\n## Concerns: Proper Usage\n\n**When to use concerns**:\n- Shared behavior across multiple models\n- Extracting cohesive functionality\n- Domain-specific mixins\n\n**When NOT to use concerns**:\n- Single-use code (just keep it in the model)\n- Hiding complexity (doesn't solve fat model problem)\n- As a dumping ground for \"misc\" methods\n\n### Good Concern Example\n\n```ruby\n# app/models/concerns/publishable.rb\nmodule Publishable\n extend ActiveSupport::Concern\n\n included do\n scope :published, -> { where(published: true) }\n scope :draft, -> { where(published: false) }\n\n validates :published_at, presence: true, if: :published?\n end\n\n def publish!\n update!(published: true, published_at: Time.current)\n end\n\n def unpublish!\n update!(published: false)\n end\n\n def published?\n published == true\n end\nend\n\n# Used by multiple models\nclass Post \u003c ApplicationRecord\n include Publishable\nend\n\nclass Article \u003c ApplicationRecord\n include Publishable\nend\n```\n\n### Bad Concern Example\n\n```ruby\n# app/models/concerns/user_stuff.rb\nmodule UserStuff # Vague name is red flag\n extend ActiveSupport::Concern\n\n # Unrelated methods dumped together\n def full_name\n \"#{first_name} #{last_name}\"\n end\n\n def calculate_reputation\n # Complex logic\n end\n\n def sync_to_crm\n # External API\n end\n\n # Used by only User model - should just be in User\nend\n```\n\n---\n\n## Query Objects\n\nExtract complex queries into dedicated classes.\n\n```ruby\n# app/queries/post_query.rb\nclass PostQuery\n def initialize(relation = Post.all)\n @relation = relation\n end\n\n def recent\n @relation.where('created_at > ?', 1.week.ago)\n end\n\n def popular\n @relation.where('views_count > ?', 1000)\n end\n\n def by_author(author)\n @relation.where(author: author)\n end\n\n def published\n @relation.where(published: true)\n end\n\n # Chainable complex queries\n def trending\n recent.popular.published.order(views_count: :desc)\n end\nend\n\n# Usage\nPostQuery.new.trending\nPostQuery.new.by_author(current_user).recent\n```\n\n---\n\n## Performance Patterns\n\n### Database Indexes\n\n**Check for missing indexes**:\n```ruby\n# Bad - queries without indexes\nUser.where(email: params[:email]) # Needs index on email\nPost.where(published: true) # Needs index on published\nComment.where(post_id: 123) # Foreign keys need indexes!\n```\n\n**Add indexes in migrations**:\n```ruby\nclass AddIndexes \u003c ActiveRecord::Migration[7.0]\n def change\n add_index :users, :email, unique: true\n add_index :posts, :published\n add_index :comments, :post_id\n add_index :posts, [:author_id, :published] # Composite index\n end\nend\n```\n\n### Caching Strategies\n\n**Fragment caching**:\n```erb\n\u003c!-- app/views/posts/show.html.erb -->\n\u003c% cache @post do %>\n \u003c%= render @post %>\n\u003c% end %>\n```\n\n**Russian doll caching**:\n```erb\n\u003c% cache @post do %>\n \u003ch1>\u003c%= @post.title %>\u003c/h1>\n\n \u003c% cache @post.comments do %>\n \u003c% @post.comments.each do |comment| %>\n \u003c% cache comment do %>\n \u003c%= render comment %>\n \u003c% end %>\n \u003c% end %>\n \u003c% end %>\n\u003c% end %>\n```\n\n**Low-level caching**:\n```ruby\ndef expensive_calculation\n Rails.cache.fetch(\"calculation-#{id}\", expires_in: 12.hours) do\n # Expensive operation\n complex_computation\n end\nend\n```\n\n### Background Jobs\n\n**Move slow operations to background**:\n```ruby\n# Bad - slow operation in request\ndef create\n @user = User.create!(user_params)\n UserMailer.welcome_email(@user).deliver_now # Blocks request\n redirect_to @user\nend\n\n# Good - async with background job\ndef create\n @user = User.create!(user_params)\n WelcomeEmailJob.perform_later(@user.id)\n redirect_to @user\nend\n```\n\n---\n\n## Controller Patterns\n\n### Skinny Controllers\n\n**Bad - fat controller**:\n```ruby\nclass OrdersController \u003c ApplicationController\n def create\n @user = User.find(params[:user_id])\n @product = Product.find(params[:product_id])\n @order = Order.new(user: @user, product: @product)\n\n if @order.save\n charge = Stripe::Charge.create(\n amount: @order.total,\n currency: 'usd',\n customer: @user.stripe_customer_id\n )\n\n @order.update!(payment_id: charge.id)\n OrderMailer.confirmation(@order).deliver_later\n AnalyticsService.track('order_created', @order.id)\n\n redirect_to @order\n else\n render :new\n end\n end\nend\n```\n\n**Good - skinny controller with service**:\n```ruby\nclass OrdersController \u003c ApplicationController\n def create\n result = OrderCreationService.call(order_params)\n\n if result.success?\n redirect_to result.order\n else\n @order = result.order\n flash.now[:error] = result.error\n render :new\n end\n end\nend\n```\n\n---\n\n## Checking Rails Patterns in Reviews\n\n**Quick checklist**:\n\n**N+1 Queries**:\n- [ ] Check for association access in loops\n- [ ] Ensure `includes`/`preload` for eager loading\n- [ ] Use counter caches for count operations\n- [ ] Run Bullet gem to detect N+1s\n\n**Callbacks**:\n- [ ] No external API calls in callbacks\n- [ ] No email sending in callbacks\n- [ ] Use `after_commit` over `after_save` for side effects\n- [ ] Consider service objects instead of callbacks\n\n**Fat Models**:\n- [ ] Models under 200 lines\n- [ ] Business logic in service objects\n- [ ] Complex queries in query objects\n- [ ] Presentation logic in presenters\n\n**Performance**:\n- [ ] Indexes on foreign keys\n- [ ] Indexes on frequently queried columns\n- [ ] Caching for expensive operations\n- [ ] Background jobs for slow tasks\n\n**Controllers**:\n- [ ] Controllers under 100 lines\n- [ ] No business logic in controllers\n- [ ] Service objects for complex operations\n- [ ] Strong parameters properly used\n","content_type":"text/markdown; charset=utf-8","language":"markdown","size":15569,"content_sha256":"a8926239caa242d319946f3dd0b22b6cbdd9550bfc5cb53dffc2c7961ad16353"},{"filename":"references/sandi-metz-rules.md","content":"# Sandi Metz Rules Reference\n\nSandi Metz's rules from \"Practical Object-Oriented Design in Ruby\" (POODR) and \"99 Bottles of OOP\" provide practical constraints that encourage better object-oriented design.\n\n## The Five Rules\n\n### Rule 1: Classes ≤ 100 Lines\n\n**Principle**: Classes should be small and focused on a single responsibility.\n\n**Detection**:\n```bash\n# Count lines in a class (excluding blank lines and comments)\ngrep -v '^\\s*#' file.rb | grep -v '^\\s*

Important: agents should read /llm.txt, /llms.txt, or /.well-known/skills.json to discover the public Skillopedia API.

| wc -l\n```\n\n**Why it matters**: Large classes often violate Single Responsibility Principle and become difficult to understand, test, and maintain.\n\n**Violations indicate**:\n- Class doing too many things\n- Missing extraction of collaborators\n- Business logic mixed with coordination logic\n\n**Refactoring strategies**:\n- Extract related methods into new classes\n- Identify cohesive groups of methods and data\n- Create service objects for complex operations\n- Use composition over large inheritance hierarchies\n\n**Example violation**:\n```ruby\nclass UserManager\n # 150 lines handling:\n # - User creation\n # - Authentication\n # - Password reset\n # - Email notifications\n # - Profile updates\n # - Permission checks\nend\n```\n\n**Refactored**:\n```ruby\nclass User \u003c ApplicationRecord\n # 30 lines - just the model\nend\n\nclass UserAuthenticator\n # 40 lines - authentication logic\nend\n\nclass UserNotifier\n # 35 lines - notification logic\nend\n\nclass UserProfileUpdater\n # 45 lines - profile update logic\nend\n```\n\n---\n\n### Rule 2: Methods ≤ 5 Lines\n\n**Principle**: Methods should do one thing and do it well.\n\n**Detection**: Count lines between `def` and `end` (excluding the definition line itself).\n\n**Why it matters**: Short methods are easier to understand, test, and reuse. They encourage proper naming and reveal intention.\n\n**Violations indicate**:\n- Method doing multiple things\n- Missing intermediate abstractions\n- Low-level details mixed with high-level logic\n\n**Refactoring strategies**:\n- Extract logical groups into private methods\n- Use composed method pattern\n- Create intention-revealing method names\n- Move complex conditions into query methods\n\n**Example violation**:\n```ruby\ndef process_order(order)\n if order.items.any? && order.user.active? && order.payment_valid?\n order.items.each do |item|\n item.reduce_inventory\n item.update(status: 'processed')\n end\n order.calculate_total\n order.send_confirmation_email\n order.update(status: 'completed')\n true\n else\n false\n end\nend\n```\n\n**Refactored**:\n```ruby\ndef process_order(order)\n return false unless processable?(order)\n\n process_items(order)\n finalize_order(order)\n true\nend\n\nprivate\n\ndef processable?(order)\n order.items.any? && order.user.active? && order.payment_valid?\nend\n\ndef process_items(order)\n order.items.each { |item| process_item(item) }\nend\n\ndef process_item(item)\n item.reduce_inventory\n item.update(status: 'processed')\nend\n\ndef finalize_order(order)\n order.calculate_total\n order.send_confirmation_email\n order.update(status: 'completed')\nend\n```\n\n---\n\n### Rule 3: Methods ≤ 4 Parameters\n\n**Principle**: Too many parameters indicate the method is doing too much or should be a class.\n\n**Detection**: Count parameters in method definition (including keyword arguments).\n\n**Why it matters**: Methods with many parameters are hard to understand, hard to test, and suggest missing abstractions.\n\n**Violations indicate**:\n- Method needs too much context\n- Missing object to encapsulate related data\n- Primitive obsession (using primitives instead of objects)\n\n**Refactoring strategies**:\n- Introduce parameter object\n- Extract class to encapsulate related parameters\n- Use builder pattern for complex construction\n- Consider if method belongs in different class\n\n**Example violation**:\n```ruby\ndef send_notification(user, title, body, type, priority, scheduled_at, metadata)\n # ...\nend\n```\n\n**Refactored with parameter object**:\n```ruby\nclass Notification\n attr_reader :user, :title, :body, :type, :priority, :scheduled_at, :metadata\n\n def initialize(user:, title:, body:, type: :email, priority: :normal,\n scheduled_at: Time.current, metadata: {})\n @user = user\n @title = title\n @body = body\n @type = type\n @priority = priority\n @scheduled_at = scheduled_at\n @metadata = metadata\n end\nend\n\ndef send_notification(notification)\n # Now works with a single, well-defined object\nend\n```\n\n---\n\n### Rule 4: Controllers Instantiate ≤ 1 Object\n\n**Principle**: Controllers should be thin and coordinate, not create multiple objects directly.\n\n**Detection**: Count `new` calls or model queries in controller actions.\n\n**Why it matters**: Controllers should orchestrate, not contain business logic. Multiple instantiations suggest missing service layer.\n\n**Violations indicate**:\n- Business logic in controller\n- Missing service object or command pattern\n- Poor separation of concerns\n\n**Refactoring strategies**:\n- Extract to service objects\n- Use command pattern\n- Create facade objects\n- Move queries to model scopes or query objects\n\n**Example violation**:\n```ruby\nclass OrdersController \u003c ApplicationController\n def create\n @user = User.find(params[:user_id])\n @product = Product.find(params[:product_id])\n @payment = Payment.new(payment_params)\n @shipping = ShippingAddress.new(shipping_params)\n @order = Order.new(user: @user, product: @product,\n payment: @payment, shipping: @shipping)\n\n if @order.save\n @notification = Notification.new(order: @order)\n @notification.send\n redirect_to @order\n else\n render :new\n end\n end\nend\n```\n\n**Refactored**:\n```ruby\nclass OrdersController \u003c ApplicationController\n def create\n result = OrderCreationService.call(order_params)\n\n if result.success?\n redirect_to result.order\n else\n @order = result.order\n render :new\n end\n end\nend\n\nclass OrderCreationService\n def self.call(params)\n new(params).call\n end\n\n def initialize(params)\n @params = params\n end\n\n def call\n build_order\n save_order\n send_notification if order.persisted?\n Result.new(order: order, success: order.persisted?)\n end\n\n private\n\n attr_reader :params, :order\n\n def build_order\n @order = Order.new(\n user: find_user,\n product: find_product,\n payment: build_payment,\n shipping: build_shipping\n )\n end\n\n # ... rest of implementation\nend\n```\n\n---\n\n### Rule 5: Views Reference ≤ 1 Instance Variable\n\n**Principle**: Views should only know about one thing, reducing coupling between controller and view.\n\n**Detection**: Count `@` symbols in view files (excluding partials with explicit locals).\n\n**Why it matters**: Multiple instance variables create tight coupling and make views hard to reuse and test.\n\n**Violations indicate**:\n- Missing presenter or decorator\n- Business logic in views\n- Controller exposing too much\n\n**Refactoring strategies**:\n- Use presenter pattern\n- Use decorator pattern (Draper gem)\n- Pass locals to partials explicitly\n- Create view objects\n\n**Example violation**:\n```ruby\n# app/controllers/users_controller.rb\ndef show\n @user = User.find(params[:id])\n @posts = @user.posts.recent\n @comments = @user.comments.recent\n @followers = @user.followers\nend\n\n# app/views/users/show.html.erb\n\u003ch1>\u003c%= @user.name %>\u003c/h1>\n\u003c%= render @posts %>\n\u003c%= render @comments %>\n\u003cp>Followers: \u003c%= @followers.count %>\u003c/p>\n```\n\n**Refactored with presenter**:\n```ruby\n# app/controllers/users_controller.rb\ndef show\n @presenter = UserProfilePresenter.new(User.find(params[:id]))\nend\n\n# app/presenters/user_profile_presenter.rb\nclass UserProfilePresenter\n def initialize(user)\n @user = user\n end\n\n def name\n @user.name\n end\n\n def recent_posts\n @user.posts.recent\n end\n\n def recent_comments\n @user.comments.recent\n end\n\n def follower_count\n @user.followers.count\n end\nend\n\n# app/views/users/show.html.erb\n\u003ch1>\u003c%= @presenter.name %>\u003c/h1>\n\u003c%= render @presenter.recent_posts %>\n\u003c%= render @presenter.recent_comments %>\n\u003cp>Followers: \u003c%= @presenter.follower_count %>\u003c/p>\n```\n\n---\n\n## Law of Demeter\n\n**\"Only talk to your immediate friends\"**\n\nAlso known as the principle of least knowledge, this rule states that an object should only call methods on:\n1. Itself\n2. Objects passed as parameters\n3. Objects it creates\n4. Its direct component objects (instance variables)\n\n**Violations look like**: `user.address.city.name` (train wrecks)\n\n**Why it matters**: Reduces coupling and makes code more maintainable.\n\n**Detection patterns**:\n```ruby\n# Violation - multiple dots (train wreck)\norder.user.shipping_address.city.tax_rate\n\n# Violation - reaching through objects\[email protected]_url\n```\n\n**Refactoring strategy**:\n```ruby\n# Hide delegation behind methods\nclass Order\n def tax_rate\n user.tax_rate # User knows how to get tax rate\n end\nend\n\nclass User\n def tax_rate\n shipping_address.tax_rate # Address knows its tax rate\n end\nend\n\n# Or use Rails delegate\nclass Order\n delegate :tax_rate, to: :user\nend\n```\n\n---\n\n## Tell, Don't Ask\n\n**Principle**: Objects should make their own decisions, not have their state queried and acted upon externally.\n\n**Bad** (asking):\n```ruby\nclass Controller\n def handle_request(user)\n if user.admin?\n admin_dashboard\n elsif user.premium?\n premium_dashboard\n else\n standard_dashboard\n end\n end\nend\n```\n\n**Good** (telling):\n```ruby\nclass Controller\n def handle_request(user)\n user.show_dashboard\n end\nend\n\nclass User\n def show_dashboard\n dashboard_class.new(self).render\n end\n\n private\n\n def dashboard_class\n return AdminDashboard if admin?\n return PremiumDashboard if premium?\n StandardDashboard\n end\nend\n```\n\n---\n\n## Code Smells (18 types)\n\n**Structural**:\n- Long Method (>5 lines)\n- Large Class (>100 lines)\n- Long Parameter List (>4 params)\n- Data Clump (same params together repeatedly)\n\n**Coupling**:\n- Feature Envy (method uses data from another class more than own)\n- Message Chains (Law of Demeter violations)\n- Inappropriate Intimacy (classes too tightly coupled)\n\n**Conditional Logic**:\n- Conditional Complexity (nested if-elsif)\n- Case Statements (candidate for polymorphism)\n- Speculative Generality (code added \"just in case\")\n\n**Naming**:\n- Vague names (Manager, Handler, Processor, Data)\n- Methods with \"and\" (doing multiple things)\n- Flag parameters (boolean params that change behavior)\n\n**Comments**:\n- Comments explaining \"what\" code does → Code should be self-documenting\n- Keep comments that explain \"why\" decisions were made\n\n---\n\n## Shameless Green Philosophy\n\nFrom 99 Bottles of OOP - encourage this refactoring approach:\n\n1. **Start with Shameless Green** - Write simplest code that works\n2. **Wait for duplication** - Don't abstract too early\n3. **Follow Flocking Rules**:\n - Find things that are most alike\n - Find smallest difference between them\n - Make simplest change to remove difference\n4. **Converge on abstractions** - Let patterns emerge\n\n**Key wisdom**: \"Make the change easy, then make the easy change\"\n\n**Example journey**:\n\nStep 1 - Shameless Green:\n```ruby\ndef verse(n)\n if n == 0\n \"No more bottles of beer on the wall\"\n elsif n == 1\n \"1 bottle of beer on the wall\"\n else\n \"#{n} bottles of beer on the wall\"\n end\nend\n```\n\nStep 2 - Remove Duplication:\n```ruby\ndef verse(n)\n \"#{quantity(n)} #{container(n)} of beer on the wall\"\nend\n```\n\nStep 3 - Polymorphism (Open/Closed):\n```ruby\nclass BottleNumber\n def self.for(number)\n case number\n when 0 then BottleNumber0\n when 1 then BottleNumber1\n else BottleNumber\n end.new(number)\n end\n\n def quantity = number.to_s\n def container = 'bottles'\nend\n\nclass BottleNumber0 \u003c BottleNumber\n def quantity = 'no more'\nend\n\nclass BottleNumber1 \u003c BottleNumber\n def container = 'bottle'\nend\n```\n\n---\n\n## Breaking The Rules\n\n**When to break these rules:**\n\n1. **Rule 1 (100 lines)**: Complex algorithms that are easier to understand together\n2. **Rule 2 (5 lines)**: Simple assignment methods, DSL configuration blocks\n3. **Rule 3 (4 parameters)**: Framework requirements (e.g., Rails callbacks), public APIs\n4. **Rule 4 (1 instantiation)**: Simple CRUD operations, scaffold-generated actions\n5. **Rule 5 (1 instance variable)**: Form objects with multiple models, complex forms\n\n**Guideline**: If you break a rule, document why with a comment explaining the trade-off.\n\nSandi Metz: \"Break them only if you have a good reason and you've tried not to.\"\n","content_type":"text/markdown; charset=utf-8","language":"markdown","size":12480,"content_sha256":"465084c2621bcecbf402e99bdf179538a57b88ac093b0cdf481ee5802fb1065a"},{"filename":"references/security-checklist.md","content":"# Security Checklist Reference\n\nThis guide covers common security vulnerabilities to check in Ruby on Rails code reviews.\n\n## SQL Injection\n\n**Risk**: Attackers can execute arbitrary SQL queries, potentially reading, modifying, or deleting data.\n\n### Vulnerable Patterns\n\n**1. String interpolation in queries**:\n```ruby\n# DANGEROUS - SQL injection vulnerability\nUser.where(\"email = '#{params[:email]}'\")\nPost.where(\"title LIKE '%#{params[:search]}%'\")\n\n# Attacker input: ' OR '1'='1\n# Results in: SELECT * FROM users WHERE email = '' OR '1'='1'\n```\n\n**2. Raw SQL without parameterization**:\n```ruby\n# DANGEROUS\nUser.find_by_sql(\"SELECT * FROM users WHERE id = #{params[:id]}\")\nActiveRecord::Base.connection.execute(\"DELETE FROM posts WHERE id = #{params[:id]}\")\n```\n\n**3. Unsafe sanitize usage**:\n```ruby\n# DANGEROUS - still vulnerable\nUser.where(\"name = #{sanitize(params[:name])}\")\n```\n\n### Safe Alternatives\n\n**1. Use parameterized queries**:\n```ruby\n# SAFE - uses placeholders\nUser.where(\"email = ?\", params[:email])\nPost.where(\"title LIKE ?\", \"%#{params[:search]}%\")\nUser.where(\"created_at > ? AND status = ?\", date, status)\n```\n\n**2. Use hash conditions**:\n```ruby\n# SAFE - ActiveRecord handles escaping\nUser.where(email: params[:email])\nPost.where(status: 'published', category: params[:category])\n```\n\n**3. Use named placeholders**:\n```ruby\n# SAFE - named placeholders\nUser.where(\"email = :email AND status = :status\",\n email: params[:email], status: params[:status])\n```\n\n**4. Use Arel for complex queries**:\n```ruby\n# SAFE - using Arel\nusers = User.arel_table\nUser.where(users[:email].eq(params[:email]))\n```\n\n**5. Safe raw SQL**:\n```ruby\n# SAFE - with bind parameters\nUser.find_by_sql([\"SELECT * FROM users WHERE email = ?\", params[:email]])\n```\n\n### Detection Strategy\n\n**In code review, search for**:\n- String interpolation (`#{}`) in SQL strings\n- `where()` with strings containing user input\n- `find_by_sql` without bind parameters\n- `execute` or `exec_query` with user input\n- `delete_all` or `update_all` with unsafe conditions\n\n---\n\n## Cross-Site Scripting (XSS)\n\n**Risk**: Attackers can inject malicious JavaScript that executes in other users' browsers, potentially stealing data or performing actions on their behalf.\n\n### Vulnerable Patterns\n\n**1. Using `html_safe` on user input**:\n```ruby\n# DANGEROUS\n\u003c%= params[:message].html_safe %>\n\u003c%= @user.bio.html_safe %> # If bio contains user input\n\n# Attacker input: \u003cscript>alert('XSS')\u003c/script>\n```\n\n**2. Using `raw()` on user input**:\n```ruby\n# DANGEROUS\n\u003c%= raw @comment.body %>\n\u003c%= raw params[:content] %>\n```\n\n**3. Rendering unescaped HTML**:\n```ruby\n# DANGEROUS in Rails 2\n\u003c%== @user.description %>\n```\n\n**4. Direct JavaScript embedding**:\n```ruby\n# DANGEROUS\n\u003cscript>\n var message = \"\u003c%= @message %>\"; // Not escaped in JS context\n\u003c/script>\n```\n\n**5. Unsafe content_tag usage**:\n```ruby\n# DANGEROUS\ncontent_tag :div, params[:content].html_safe\n```\n\n### Safe Alternatives\n\n**1. Let Rails auto-escape** (default behavior):\n```ruby\n# SAFE - Rails escapes by default\n\u003c%= @user.bio %>\n\u003c%= params[:message] %>\n```\n\n**2. Use sanitize for limited HTML**:\n```ruby\n# SAFE - allows safe HTML tags only\n\u003c%= sanitize @comment.body %>\n\n# With custom allowed tags\n\u003c%= sanitize @post.content, tags: %w(strong em a), attributes: %w(href) %>\n```\n\n**3. Use JSON for JavaScript data**:\n```ruby\n# SAFE - properly escaped for JavaScript\n\u003cscript>\n var message = \u003c%= @message.to_json %>;\n var user = \u003c%= @user.to_json %>;\n\u003c/script>\n```\n\n**4. Use data attributes**:\n```ruby\n# SAFE - let Rails handle escaping\n\u003cdiv data-message=\"\u003c%= @message %>\">\n\u003c/div>\n\n\u003cscript>\n const message = document.querySelector('[data-message]').dataset.message;\n\u003c/script>\n```\n\n**5. Use content_security_policy**:\n```ruby\n# config/initializers/content_security_policy.rb\nRails.application.config.content_security_policy do |policy|\n policy.default_src :self\n policy.script_src :self, :https\n policy.style_src :self, :https\nend\n```\n\n### Detection Strategy\n\n**In code review, search for**:\n- `.html_safe` on user-generated content\n- `raw()` helper usage\n- User input in `\u003cscript>` tags\n- `sanitize()` without restricted tags\n- Direct rendering of params in views\n\n---\n\n## Mass Assignment\n\n**Risk**: Attackers can modify fields they shouldn't have access to (e.g., admin flags, prices).\n\n### Vulnerable Patterns\n\n**1. No strong parameters**:\n```ruby\n# DANGEROUS - allows any parameter\ndef create\n @user = User.create(params[:user])\nend\n```\n\n**2. Using permit!**:\n```ruby\n# DANGEROUS - permits everything\ndef create\n @user = User.create(user_params)\nend\n\nprivate\n\ndef user_params\n params.require(:user).permit! # Allows ALL attributes\nend\n```\n\n**3. Direct assignment of params**:\n```ruby\n# DANGEROUS\ndef update\n current_user.attributes = params[:user]\n current_user.save\nend\n```\n\n**4. Bypassing strong parameters**:\n```ruby\n# DANGEROUS\ndef create\n @product = Product.new\n params[:product].each do |key, value|\n @product.send(\"#{key}=\", value) # Allows any attribute\n end\n @product.save\nend\n```\n\n### Safe Alternatives\n\n**1. Use strong parameters**:\n```ruby\n# SAFE - explicitly permit attributes\ndef create\n @user = User.create(user_params)\nend\n\nprivate\n\ndef user_params\n params.require(:user).permit(:name, :email, :bio)\nend\n```\n\n**2. Conditional permissions**:\n```ruby\n# SAFE - different permissions for admins\ndef user_params\n if current_user.admin?\n params.require(:user).permit(:name, :email, :role, :status)\n else\n params.require(:user).permit(:name, :email)\n end\nend\n```\n\n**3. Nested attributes**:\n```ruby\n# SAFE - permit nested attributes\ndef post_params\n params.require(:post).permit(\n :title,\n :body,\n comments_attributes: [:id, :body, :_destroy]\n )\nend\n```\n\n**4. Use form objects for complex scenarios**:\n```ruby\n# SAFE - explicit attribute handling\nclass UserRegistrationForm\n include ActiveModel::Model\n\n attr_accessor :name, :email, :password\n\n validates :name, :email, :password, presence: true\n\n def save\n return false unless valid?\n\n User.create!(\n name: name,\n email: email,\n password: password,\n role: 'user' # Hardcoded, not from params\n )\n end\nend\n```\n\n### Detection Strategy\n\n**In code review, search for**:\n- `params[:model]` without strong parameters\n- `.permit!` usage\n- Direct assignment: `model.attributes = params`\n- Dynamic attribute assignment with `send()`\n- Missing authorization checks on sensitive fields\n\n---\n\n## Authentication & Authorization\n\n### Authentication Issues\n\n**1. Weak password requirements**:\n```ruby\n# WEAK\nvalidates :password, length: { minimum: 4 }\n\n# BETTER\nvalidates :password,\n length: { minimum: 12 },\n format: {\n with: /(?=.*[a-z])(?=.*[A-Z])(?=.*\\d)/,\n message: \"must include lowercase, uppercase, and number\"\n }\n```\n\n**2. Storing passwords in plain text**:\n```ruby\n# DANGEROUS\ndef password=(value)\n self[:password] = value # Never store plain text!\nend\n\n# SAFE - use has_secure_password\nclass User \u003c ApplicationRecord\n has_secure_password\n\n validates :password, length: { minimum: 12 }, if: :password_digest_changed?\nend\n```\n\n**3. Predictable password reset tokens**:\n```ruby\n# DANGEROUS\ndef generate_reset_token\n self.reset_token = id.to_s + Time.now.to_i.to_s # Predictable!\nend\n\n# SAFE\ndef generate_reset_token\n self.reset_token = SecureRandom.urlsafe_base64(32)\n self.reset_token_expires_at = 2.hours.from_now\nend\n```\n\n### Authorization Issues\n\n**1. Missing authorization checks**:\n```ruby\n# DANGEROUS - no authorization\ndef destroy\n @post = Post.find(params[:id])\n @post.destroy\n redirect_to posts_path\nend\n\n# SAFE - verify ownership\ndef destroy\n @post = current_user.posts.find(params[:id]) # Scoped to user\n @post.destroy\n redirect_to posts_path\nend\n```\n\n**2. Inconsistent authorization**:\n```ruby\n# DANGEROUS - authorization only on some actions\nclass PostsController \u003c ApplicationController\n before_action :authorize_user, only: [:edit, :update]\n # Missing authorization on destroy!\n\n def destroy\n @post = Post.find(params[:id])\n @post.destroy\n end\nend\n```\n\n**3. Client-side authorization only**:\n```ruby\n# DANGEROUS - client can bypass this\n\u003c% if current_user.admin? %>\n \u003c%= link_to \"Delete\", post_path(@post), method: :delete %>\n\u003c% end %>\n\n# Controller MUST also check\ndef destroy\n return head :forbidden unless current_user.admin?\n @post = Post.find(params[:id])\n @post.destroy\nend\n```\n\n**4. Use authorization gems**:\n```ruby\n# Use Pundit for consistent authorization\nclass PostPolicy\n attr_reader :user, :post\n\n def initialize(user, post)\n @user = user\n @post = post\n end\n\n def destroy?\n user.admin? || post.user == user\n end\nend\n\nclass PostsController \u003c ApplicationController\n def destroy\n @post = Post.find(params[:id])\n authorize @post # Pundit checks PostPolicy#destroy?\n @post.destroy\n redirect_to posts_path\n end\nend\n```\n\n### Detection Strategy\n\n**In code review, check for**:\n- Actions modifying data without authorization\n- Password validations (length, complexity)\n- Plain text password storage\n- Missing scoping to current user\n- Authorization logic inconsistency\n\n---\n\n## CSRF (Cross-Site Request Forgery)\n\n**Risk**: Attackers can trick users into performing unwanted actions.\n\n### Protection\n\n**1. CSRF tokens (enabled by default)**:\n```ruby\n# app/controllers/application_controller.rb\nclass ApplicationController \u003c ActionController::Base\n protect_from_forgery with: :exception # Default in Rails\nend\n```\n\n**2. Don't disable CSRF protection**:\n```ruby\n# DANGEROUS - disables CSRF protection\nclass ApiController \u003c ApplicationController\n skip_before_action :verify_authenticity_token # Only for APIs with other auth\nend\n```\n\n**3. Use proper HTTP verbs**:\n```ruby\n# BAD - state-changing action with GET\nget '/users/:id/delete', to: 'users#destroy'\n\n# GOOD - use DELETE\ndelete '/users/:id', to: 'users#destroy'\n```\n\n---\n\n## Session Management\n\n**1. Secure session cookies**:\n```ruby\n# config/initializers/session_store.rb\nRails.application.config.session_store :cookie_store,\n key: '_app_session',\n secure: Rails.env.production?, # HTTPS only in production\n httponly: true, # Not accessible via JavaScript\n same_site: :lax # CSRF protection\n```\n\n**2. Session fixation protection**:\n```ruby\n# SAFE - Rails resets session on login by default\ndef create\n user = User.find_by(email: params[:email])\n if user&.authenticate(params[:password])\n reset_session # Important: prevent session fixation\n session[:user_id] = user.id\n redirect_to root_path\n end\nend\n```\n\n**3. Session timeout**:\n```ruby\n# Add session expiration\nclass ApplicationController \u003c ActionController::Base\n before_action :check_session_expiry\n\n private\n\n def check_session_expiry\n if session[:expires_at] && session[:expires_at] \u003c Time.current\n reset_session\n redirect_to login_path, alert: 'Session expired'\n else\n session[:expires_at] = 30.minutes.from_now\n end\n end\nend\n```\n\n---\n\n## File Upload Security\n\n**1. Validate file types**:\n```ruby\n# DANGEROUS - trusts client-provided content type\ndef create\n if params[:file].content_type == 'image/jpeg'\n # Client can lie about content type!\n end\nend\n\n# BETTER - validate actual file content\nclass ImageUploader \u003c CarrierWave::Uploader::Base\n def content_type_whitelist\n /image\\//\n end\n\n def extension_whitelist\n %w(jpg jpeg gif png)\n end\nend\n```\n\n**2. Scan uploaded files**:\n```ruby\n# Use ClamAV or similar to scan for malware\nclass DocumentUploader \u003c CarrierWave::Uploader::Base\n before :cache, :scan_for_viruses\n\n def scan_for_viruses(file)\n result = `clamscan #{file.path}`\n raise SecurityError, 'Virus detected' if $?.exitstatus != 0\n end\nend\n```\n\n**3. Store uploads outside web root**:\n```ruby\n# Don't store in public/ - use cloud storage\nclass AvatarUploader \u003c CarrierWave::Uploader::Base\n storage :fog # AWS S3, etc.\n\n def store_dir\n \"uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}\"\n end\nend\n```\n\n**4. Limit file sizes**:\n```ruby\nclass AvatarUploader \u003c CarrierWave::Uploader::Base\n def size_range\n 1..5.megabytes\n end\nend\n```\n\n---\n\n## API Security\n\n**1. Use API authentication**:\n```ruby\n# Use tokens, not session cookies for APIs\nclass Api::BaseController \u003c ActionController::API\n before_action :authenticate_api_user\n\n private\n\n def authenticate_api_user\n token = request.headers['Authorization']&.split(' ')&.last\n @current_user = User.find_by(api_token: token)\n head :unauthorized unless @current_user\n end\nend\n```\n\n**2. Rate limiting**:\n```ruby\n# Use Rack::Attack for rate limiting\nclass Rack::Attack\n throttle('api/ip', limit: 300, period: 5.minutes) do |req|\n req.ip if req.path.start_with?('/api/')\n end\n\n throttle('api/token', limit: 100, period: 1.minute) do |req|\n req.env['HTTP_AUTHORIZATION'] if req.path.start_with?('/api/')\n end\nend\n```\n\n**3. Validate content types**:\n```ruby\nclass Api::PostsController \u003c Api::BaseController\n before_action :verify_content_type\n\n private\n\n def verify_content_type\n unless request.content_type == 'application/json'\n head :unsupported_media_type\n end\n end\nend\n```\n\n---\n\n## Input Validation\n\n**1. Validate all inputs**:\n```ruby\nclass User \u003c ApplicationRecord\n validates :email, format: { with: URI::MailTo::EMAIL_REGEXP }\n validates :age, numericality: { only_integer: true, greater_than: 0 }\n validates :website, format: { with: /\\Ahttps?:\\/\\// }, allow_blank: true\nend\n```\n\n**2. Sanitize inputs**:\n```ruby\n# Remove dangerous characters\nbefore_validation :sanitize_inputs\n\ndef sanitize_inputs\n self.name = name.strip if name.present?\n self.bio = ActionController::Base.helpers.sanitize(bio) if bio.present?\nend\n```\n\n---\n\n## Security Review Checklist\n\n**For every code review, check**:\n\n**SQL Injection**:\n- [ ] No string interpolation in SQL\n- [ ] Parameterized queries used\n- [ ] Hash conditions for simple queries\n\n**XSS**:\n- [ ] No `.html_safe` on user input\n- [ ] No `raw()` on user input\n- [ ] Proper escaping in JavaScript contexts\n- [ ] Content Security Policy configured\n\n**Mass Assignment**:\n- [ ] Strong parameters used\n- [ ] No `.permit!` usage\n- [ ] Sensitive fields not accessible\n\n**Authentication**:\n- [ ] Strong password requirements\n- [ ] Passwords hashed (has_secure_password)\n- [ ] Secure token generation\n- [ ] Session reset on login\n\n**Authorization**:\n- [ ] Authorization checks on all actions\n- [ ] Consistent authorization approach\n- [ ] Scoped queries to current user\n- [ ] Server-side authorization (not just client-side)\n\n**CSRF**:\n- [ ] CSRF protection enabled\n- [ ] Proper HTTP verbs used\n- [ ] State changes require POST/PUT/DELETE\n\n**Sessions**:\n- [ ] Secure cookie settings\n- [ ] Session timeout implemented\n- [ ] Session fixation prevention\n\n**File Uploads**:\n- [ ] File type validation\n- [ ] File size limits\n- [ ] Stored outside web root\n- [ ] Virus scanning for user uploads\n\n**API Security**:\n- [ ] Token-based authentication\n- [ ] Rate limiting implemented\n- [ ] Content type validation\n\n**General**:\n- [ ] Input validation on all user data\n- [ ] Proper error handling (don't leak info)\n- [ ] Dependencies up to date (check `bundle audit`)\n- [ ] Secrets in credentials, not code\n","content_type":"text/markdown; charset=utf-8","language":"markdown","size":15261,"content_sha256":"0d38b1da3bab53238e0e1ca698b7bda4af1178dc9444e0049717aa995481e0ab"},{"filename":"references/solid-principles.md","content":"# SOLID Principles Reference\n\nThe SOLID principles are five design principles that make software designs more understandable, flexible, and maintainable.\n\n## S - Single Responsibility Principle (SRP)\n\n**Definition**: A class should have one, and only one, reason to change.\n\n**Core idea**: Each class should have a single, well-defined responsibility. If a class has multiple responsibilities, changes to one responsibility can affect the others.\n\n**How to identify violations**:\n- Class name contains \"And\" or \"Manager\" or \"Handler\"\n- Difficult to give the class a concise name\n- Class has methods that operate on different data\n- Changes in different parts of the application require modifying this class\n\n**Example violation**:\n```ruby\nclass User \u003c ApplicationRecord\n # Responsibility 1: User data\n validates :email, presence: true\n\n # Responsibility 2: Authentication\n def authenticate(password)\n BCrypt::Password.new(password_digest) == password\n end\n\n # Responsibility 3: Email notifications\n def send_welcome_email\n UserMailer.welcome_email(self).deliver_now\n end\n\n # Responsibility 4: Report generation\n def generate_activity_report\n activities.map { |a| \"#{a.created_at}: #{a.description}\" }.join(\"\\n\")\n end\n\n # Responsibility 5: External API integration\n def sync_to_crm\n CrmApi.create_contact(\n email: email,\n name: name\n )\n end\nend\n```\n\n**Refactored**:\n```ruby\n# Responsibility 1: User data only\nclass User \u003c ApplicationRecord\n validates :email, presence: true\nend\n\n# Responsibility 2: Authentication\nclass UserAuthenticator\n def initialize(user)\n @user = user\n end\n\n def authenticate(password)\n BCrypt::Password.new(@user.password_digest) == password\n end\nend\n\n# Responsibility 3: Email notifications\nclass UserNotifier\n def initialize(user)\n @user = user\n end\n\n def send_welcome_email\n UserMailer.welcome_email(@user).deliver_now\n end\nend\n\n# Responsibility 4: Report generation\nclass UserActivityReportGenerator\n def initialize(user)\n @user = user\n end\n\n def generate\n @user.activities.map { |a| format_activity(a) }.join(\"\\n\")\n end\n\n private\n\n def format_activity(activity)\n \"#{activity.created_at}: #{activity.description}\"\n end\nend\n\n# Responsibility 5: External API integration\nclass UserCrmSync\n def initialize(user)\n @user = user\n end\n\n def sync\n CrmApi.create_contact(\n email: @user.email,\n name: @user.name\n )\n end\nend\n```\n\n**Benefits of SRP**:\n- Easier to test (each class has focused tests)\n- Easier to understand (clear purpose)\n- Less likely to break when changing\n- Better reusability\n\n---\n\n## O - Open/Closed Principle (OCP)\n\n**Definition**: Software entities should be open for extension but closed for modification.\n\n**Core idea**: You should be able to add new functionality without changing existing code. Achieve this through abstraction and polymorphism.\n\n**How to identify violations**:\n- Long if/elsif/case statements that grow with new types\n- Modifying existing methods to handle new cases\n- Lack of abstraction or inheritance where behavior varies\n\n**Example violation**:\n```ruby\nclass ReportGenerator\n def generate(report_type, data)\n case report_type\n when :pdf\n generate_pdf(data)\n when :csv\n generate_csv(data)\n when :json\n generate_json(data)\n # Adding new format requires modifying this method\n end\n end\n\n private\n\n def generate_pdf(data)\n # PDF generation logic\n end\n\n def generate_csv(data)\n # CSV generation logic\n end\n\n def generate_json(data)\n # JSON generation logic\n end\nend\n```\n\n**Refactored with polymorphism**:\n```ruby\n# Base abstraction\nclass ReportGenerator\n def generate(data)\n raise NotImplementedError, \"Subclasses must implement generate\"\n end\nend\n\n# Concrete implementations\nclass PdfReportGenerator \u003c ReportGenerator\n def generate(data)\n # PDF generation logic\n end\nend\n\nclass CsvReportGenerator \u003c ReportGenerator\n def generate(data)\n # CSV generation logic\n end\nend\n\nclass JsonReportGenerator \u003c ReportGenerator\n def generate(data)\n # JSON generation logic\n end\nend\n\n# New format can be added without modifying existing code\nclass XmlReportGenerator \u003c ReportGenerator\n def generate(data)\n # XML generation logic\n end\nend\n\n# Usage with strategy pattern\nclass ReportService\n def initialize(generator)\n @generator = generator\n end\n\n def create_report(data)\n @generator.generate(data)\n end\nend\n\n# Usage\nservice = ReportService.new(PdfReportGenerator.new)\nservice.create_report(data)\n```\n\n**Rails example with concerns**:\n```ruby\n# Instead of modifying existing Payment class for new payment types\nmodule Payable\n extend ActiveSupport::Concern\n\n def process_payment\n raise NotImplementedError\n end\nend\n\nclass CreditCardPayment \u003c Payment\n include Payable\n\n def process_payment\n # Credit card logic\n end\nend\n\nclass PaypalPayment \u003c Payment\n include Payable\n\n def process_payment\n # PayPal logic\n end\nend\n\n# Adding new payment type doesn't modify existing classes\nclass CryptoPayment \u003c Payment\n include Payable\n\n def process_payment\n # Cryptocurrency logic\n end\nend\n```\n\n---\n\n## L - Liskov Substitution Principle (LSP)\n\n**Definition**: Objects of a superclass should be replaceable with objects of a subclass without breaking the application.\n\n**Core idea**: Subtypes must be substitutable for their base types. If S is a subtype of T, then objects of type T can be replaced with objects of type S without altering program correctness.\n\n**How to identify violations**:\n- Subclass removes functionality from parent\n- Subclass throws exceptions parent doesn't throw\n- Subclass strengthens preconditions (requires more)\n- Subclass weakens postconditions (guarantees less)\n- Type checking (`is_a?`, `kind_of?`) before method calls\n\n**Example violation**:\n```ruby\nclass Bird\n def fly\n \"Flying high!\"\n end\nend\n\nclass Penguin \u003c Bird\n def fly\n raise \"Penguins can't fly!\" # Violates LSP\n end\nend\n\n# This breaks when we substitute Penguin for Bird\ndef make_bird_fly(bird)\n bird.fly # Will raise error for Penguin\nend\n\nbird = Bird.new\nmake_bird_fly(bird) # Works\n\npenguin = Penguin.new\nmake_bird_fly(penguin) # Raises error - LSP violation!\n```\n\n**Refactored**:\n```ruby\n# Better abstraction\nclass Bird\n def move\n raise NotImplementedError\n end\nend\n\nclass FlyingBird \u003c Bird\n def move\n fly\n end\n\n def fly\n \"Flying high!\"\n end\nend\n\nclass Penguin \u003c Bird\n def move\n swim\n end\n\n def swim\n \"Swimming fast!\"\n end\nend\n\n# Now substitution works correctly\ndef make_bird_move(bird)\n bird.move # Works for all birds\nend\n\neagle = FlyingBird.new\nmake_bird_move(eagle) # \"Flying high!\"\n\npenguin = Penguin.new\nmake_bird_move(penguin) # \"Swimming fast!\"\n```\n\n**Rails example**:\n```ruby\n# Violation\nclass User\n def save\n # Normal save behavior\n super\n end\nend\n\nclass AdminUser \u003c User\n def save\n raise \"Admins can't be saved directly\" # LSP violation\n end\nend\n\n# Refactored\nclass User\n def save\n return false unless can_be_saved?\n super\n end\n\n def can_be_saved?\n true\n end\nend\n\nclass AdminUser \u003c User\n def can_be_saved?\n false # Doesn't violate LSP, just returns false\n end\nend\n```\n\n**Key rule**: If you need to check the type before calling a method, LSP is probably violated.\n\n---\n\n## I - Interface Segregation Principle (ISP)\n\n**Definition**: No client should be forced to depend on methods it does not use.\n\n**Core idea**: Many specific interfaces are better than one general-purpose interface. Classes shouldn't be forced to implement methods they don't need.\n\n**How to identify violations**:\n- Classes implementing interfaces with empty/no-op methods\n- Modules forcing unrelated behavior\n- Fat interfaces with many unrelated methods\n- Clients that use only a small subset of an interface\n\n**Example violation**:\n```ruby\n# Fat interface forcing too many responsibilities\nmodule Workable\n def work\n raise NotImplementedError\n end\n\n def eat_lunch\n raise NotImplementedError\n end\n\n def take_break\n raise NotImplementedError\n end\n\n def attend_meeting\n raise NotImplementedError\n end\nend\n\nclass HumanWorker\n include Workable\n\n def work\n \"Working on tasks\"\n end\n\n def eat_lunch\n \"Eating lunch\"\n end\n\n def take_break\n \"Taking a break\"\n end\n\n def attend_meeting\n \"Attending meeting\"\n end\nend\n\nclass RobotWorker\n include Workable\n\n def work\n \"Processing tasks\"\n end\n\n def eat_lunch\n # Robots don't eat - forced to implement anyway\n nil\n end\n\n def take_break\n # Robots don't need breaks - forced to implement\n nil\n end\n\n def attend_meeting\n # Robots don't attend meetings - forced to implement\n nil\n end\nend\n```\n\n**Refactored with segregated interfaces**:\n```ruby\n# Split into focused interfaces\nmodule Workable\n def work\n raise NotImplementedError\n end\nend\n\nmodule Feedable\n def eat_lunch\n raise NotImplementedError\n end\nend\n\nmodule Breakable\n def take_break\n raise NotImplementedError\n end\nend\n\nmodule Attendable\n def attend_meeting\n raise NotImplementedError\n end\nend\n\nclass HumanWorker\n include Workable\n include Feedable\n include Breakable\n include Attendable\n\n def work\n \"Working on tasks\"\n end\n\n def eat_lunch\n \"Eating lunch\"\n end\n\n def take_break\n \"Taking a break\"\n end\n\n def attend_meeting\n \"Attending meeting\"\n end\nend\n\nclass RobotWorker\n include Workable # Only includes what it needs\n\n def work\n \"Processing tasks\"\n end\nend\n```\n\n**Rails example**:\n```ruby\n# Violation - fat concern\nmodule Publishable\n extend ActiveSupport::Concern\n\n def publish\n update(published: true, published_at: Time.current)\n end\n\n def unpublish\n update(published: false)\n end\n\n def schedule_publication(time)\n update(scheduled_for: time)\n end\n\n def send_publication_notification\n NotificationMailer.published(self).deliver_later\n end\n\n def generate_social_media_post\n # Not all publishable things need this\n end\nend\n\n# Refactored - segregated concerns\nmodule Publishable\n extend ActiveSupport::Concern\n\n def publish\n update(published: true, published_at: Time.current)\n end\n\n def unpublish\n update(published: false)\n end\nend\n\nmodule Schedulable\n extend ActiveSupport::Concern\n\n def schedule_publication(time)\n update(scheduled_for: time)\n end\nend\n\nmodule Notifiable\n extend ActiveSupport::Concern\n\n def send_publication_notification\n NotificationMailer.published(self).deliver_later\n end\nend\n\nmodule SocialMediaIntegrated\n extend ActiveSupport::Concern\n\n def generate_social_media_post\n SocialMediaService.create_post(self)\n end\nend\n\n# Models include only what they need\nclass BlogPost \u003c ApplicationRecord\n include Publishable\n include Schedulable\n include Notifiable\n include SocialMediaIntegrated\nend\n\nclass Comment \u003c ApplicationRecord\n include Publishable\n # Comments don't need scheduling or social media\nend\n```\n\n---\n\n## D - Dependency Inversion Principle (DIP)\n\n**Definition**: High-level modules should not depend on low-level modules. Both should depend on abstractions. Abstractions should not depend on details. Details should depend on abstractions.\n\n**Core idea**: Depend on interfaces/abstractions rather than concrete implementations. This decouples code and makes it more flexible.\n\n**How to identify violations**:\n- Classes instantiating dependencies directly\n- Hard-coded dependencies\n- Tight coupling to specific implementations\n- Difficult to test due to concrete dependencies\n\n**Example violation**:\n```ruby\n# High-level class depends on low-level concrete implementation\nclass OrderProcessor\n def process(order)\n # Tightly coupled to MySqlDatabase\n database = MySqlDatabase.new\n database.save(order)\n\n # Tightly coupled to SmtpEmailService\n email_service = SmtpEmailService.new\n email_service.send_confirmation(order.user.email)\n\n # Tightly coupled to StripePaymentGateway\n payment = StripePaymentGateway.new\n payment.charge(order.amount)\n end\nend\n```\n\n**Refactored with dependency injection**:\n```ruby\n# Define abstractions (interfaces)\nclass DatabaseAdapter\n def save(record)\n raise NotImplementedError\n end\nend\n\nclass EmailService\n def send_confirmation(email)\n raise NotImplementedError\n end\nend\n\nclass PaymentGateway\n def charge(amount)\n raise NotImplementedError\n end\nend\n\n# Concrete implementations depend on abstractions\nclass MySqlDatabase \u003c DatabaseAdapter\n def save(record)\n # MySQL-specific logic\n end\nend\n\nclass PostgresDatabase \u003c DatabaseAdapter\n def save(record)\n # Postgres-specific logic\n end\nend\n\nclass SmtpEmailService \u003c EmailService\n def send_confirmation(email)\n # SMTP logic\n end\nend\n\nclass SendgridEmailService \u003c EmailService\n def send_confirmation(email)\n # Sendgrid API logic\n end\nend\n\nclass StripePaymentGateway \u003c PaymentGateway\n def charge(amount)\n # Stripe API logic\n end\nend\n\nclass PaypalPaymentGateway \u003c PaymentGateway\n def charge(amount)\n # PayPal API logic\n end\nend\n\n# High-level class now depends on abstractions\nclass OrderProcessor\n def initialize(database:, email_service:, payment_gateway:)\n @database = database\n @email_service = email_service\n @payment_gateway = payment_gateway\n end\n\n def process(order)\n @database.save(order)\n @email_service.send_confirmation(order.user.email)\n @payment_gateway.charge(order.amount)\n end\nend\n\n# Easy to swap implementations\nprocessor = OrderProcessor.new(\n database: PostgresDatabase.new,\n email_service: SendgridEmailService.new,\n payment_gateway: StripePaymentGateway.new\n)\n\n# Easy to test with mocks\ntest_processor = OrderProcessor.new(\n database: MockDatabase.new,\n email_service: MockEmailService.new,\n payment_gateway: MockPaymentGateway.new\n)\n```\n\n**Rails example with service objects**:\n```ruby\n# Violation - controller tightly coupled to implementation\nclass OrdersController \u003c ApplicationController\n def create\n payment = Stripe::Charge.create(\n amount: order_params[:amount],\n currency: 'usd',\n source: order_params[:token]\n )\n\n order = Order.create!(order_params.merge(payment_id: payment.id))\n\n Twilio::REST::Client.new.messages.create(\n from: ENV['TWILIO_NUMBER'],\n to: order.user.phone,\n body: \"Order confirmed\"\n )\n\n redirect_to order\n end\nend\n\n# Refactored - depend on abstractions\nclass OrdersController \u003c ApplicationController\n def create\n result = OrderCreationService.call(\n params: order_params,\n payment_gateway: payment_gateway,\n notification_service: notification_service\n )\n\n if result.success?\n redirect_to result.order\n else\n render :new, status: :unprocessable_entity\n end\n end\n\n private\n\n def payment_gateway\n # Can be configured or swapped easily\n @payment_gateway ||= PaymentGateways::Stripe.new\n end\n\n def notification_service\n @notification_service ||= NotificationServices::Sms.new\n end\nend\n\nclass OrderCreationService\n def self.call(params:, payment_gateway:, notification_service:)\n new(params, payment_gateway, notification_service).call\n end\n\n def initialize(params, payment_gateway, notification_service)\n @params = params\n @payment_gateway = payment_gateway\n @notification_service = notification_service\n end\n\n def call\n payment = process_payment\n return failure(payment.error) unless payment.success?\n\n order = create_order(payment)\n send_notification(order)\n\n success(order)\n end\n\n private\n\n def process_payment\n @payment_gateway.charge(\n amount: @params[:amount],\n token: @params[:token]\n )\n end\n\n def create_order(payment)\n Order.create!(@params.merge(payment_id: payment.id))\n end\n\n def send_notification(order)\n @notification_service.send(\n to: order.user.phone,\n message: \"Order confirmed\"\n )\n end\nend\n```\n\n**Benefits of DIP**:\n- Easy to swap implementations (e.g., test vs. production)\n- Decoupled code\n- Better testability\n- More flexible and maintainable\n\n---\n\n## Checking SOLID in Code Reviews\n\n**Quick checklist**:\n\n**SRP violations**:\n- [ ] Class has methods operating on different data\n- [ ] Class name contains \"And\", \"Manager\", \"Handler\"\n- [ ] Class would need to change for multiple reasons\n\n**OCP violations**:\n- [ ] Long case statements based on type\n- [ ] Modifying existing methods to add new behavior\n- [ ] Lack of polymorphism where behavior varies\n\n**LSP violations**:\n- [ ] Type checking before method calls\n- [ ] Subclass removes or restricts parent functionality\n- [ ] Unexpected exceptions in subclasses\n\n**ISP violations**:\n- [ ] Empty or no-op method implementations\n- [ ] Modules/concerns forcing unrelated behavior\n- [ ] Classes implementing large interfaces partially\n\n**DIP violations**:\n- [ ] Direct instantiation of dependencies\n- [ ] Hard-coded class names for dependencies\n- [ ] Difficult to test due to concrete dependencies\n\n**Code review comments should**:\n- Identify which principle is violated\n- Explain why it matters\n- Suggest specific refactoring approach\n- Provide code example if helpful\n","content_type":"text/markdown; charset=utf-8","language":"markdown","size":17029,"content_sha256":"1c59e1116c7a0169fbec2d4a2e561cf8a2800244abecfbf64af87c83840b66b1"},{"filename":"references/vscode-links.md","content":"# VSCode Links Reference\n\nThis guide explains how to create VSCode-compatible file links in REVIEW.md that allow reviewers to click and jump directly to the relevant line of code.\n\n## Link Format\n\n### Basic Structure\n\n```markdown\n[descriptive text](file:///absolute/path/to/file.rb#L123)\n```\n\n**Components**:\n- `file://` - URI scheme for local files (note: three slashes total)\n- `/absolute/path/to/file.rb` - Complete absolute path to the file\n- `#L123` - Line number anchor (capital L followed by line number)\n\n### Important Rules\n\n1. **Must use absolute paths** - Relative paths won't work\n2. **Three slashes after file:** - `file:///` not `file://`\n3. **Capital L in line anchor** - `#L42` not `#l42` or `#42`\n4. **No spaces in path** - URL encode spaces as `%20` if necessary\n5. **Forward slashes** - Even on Windows, use `/` not `\\`\n\n## Examples\n\n### Basic File Reference\n\n```markdown\n[UserService#create violates SRP](file:///Users/dev/myapp/app/services/user_service.rb#L23)\n```\n\nClicking this opens `app/services/user_service.rb` at line 23 in VSCode.\n\n### Controller Action\n\n```markdown\n[Missing authorization check in destroy action](file:///Users/dev/myapp/app/controllers/posts_controller.rb#L45)\n```\n\n### Model Method\n\n```markdown\n[N+1 query in Post#recent_comments](file:///Users/dev/myapp/app/models/post.rb#L67)\n```\n\n### View Template\n\n```markdown\n[XSS vulnerability using html_safe](file:///Users/dev/myapp/app/views/posts/show.html.erb#L12)\n```\n\n### Test File\n\n```markdown\n[Missing test for edge case](file:///Users/dev/myapp/spec/models/user_spec.rb#L89)\n```\n\n## Constructing Links Programmatically\n\n### Ruby Code to Generate Links\n\n```ruby\ndef vscode_link(description, file_path, line_number)\n absolute_path = File.expand_path(file_path)\n \"[#{description}](file://#{absolute_path}#L#{line_number})\"\nend\n\n# Usage\nvscode_link(\n \"UserService#create violates SRP\",\n \"app/services/user_service.rb\",\n 23\n)\n# => \"[UserService#create violates SRP](file:///Users/dev/myapp/app/services/user_service.rb#L23)\"\n```\n\n### Get Absolute Path\n\n```ruby\n# From relative path\nabsolute_path = File.expand_path(\"app/models/user.rb\")\n# => \"/Users/dev/myapp/app/models/user.rb\"\n\n# From git root\ngit_root = `git rev-parse --show-toplevel`.strip\nfile_path = File.join(git_root, \"app/models/user.rb\")\n# => \"/Users/dev/myapp/app/models/user.rb\"\n```\n\n### Finding Line Numbers\n\nWhen analyzing code, capture the line number from grep output:\n\n```bash\n# Grep with line numbers\ngrep -n \"def create\" app/services/user_service.rb\n# => 23: def create\n\n# In Ruby\nline_number = File.readlines(file_path).find_index { |line| line.include?(\"def create\") } + 1\n```\n\n## Integration with Code Review\n\n### REVIEW.md Structure\n\n```markdown\n## Design & Architecture\n\n### OOP Violations\n\n#### Single Responsibility Principle\n\n**UserService class has too many responsibilities** ([app/services/user_service.rb#L1](file:///Users/dev/myapp/app/services/user_service.rb#L1))\n\nThe `UserService` class handles:\n- User creation ([line 23](file:///Users/dev/myapp/app/services/user_service.rb#L23))\n- Email sending ([line 45](file:///Users/dev/myapp/app/services/user_service.rb#L45))\n- Payment processing ([line 67](file:///Users/dev/myapp/app/services/user_service.rb#L67))\n- Analytics tracking ([line 89](file:///Users/dev/myapp/app/services/user_service.rb#L89))\n\n**Recommendation**: Extract each responsibility into separate service objects.\n```\n\n### Inline Code References\n\nEvery code reference should be clickable:\n\n```markdown\nThe method [`Post#recent_comments`](file:///Users/dev/myapp/app/models/post.rb#L34)\ncauses an N+1 query. Each iteration at\n[line 36](file:///Users/dev/myapp/app/models/post.rb#L36) loads comments\nindividually instead of eager loading them.\n```\n\n## Testing Links\n\n### Manual Testing\n\nAfter generating REVIEW.md:\n\n1. Open REVIEW.md in VSCode\n2. Click each link\n3. Verify it opens the correct file at the correct line\n\n### Automated Verification\n\n```ruby\n# Verify all links in REVIEW.md point to existing files\nreview_content = File.read('REVIEW.md')\n\n# Extract all file:// links\nlinks = review_content.scan(/file:\\/\\/(.*?)#L(\\d+)/)\n\nlinks.each do |file_path, line_number|\n unless File.exist?(file_path)\n puts \"ERROR: File not found: #{file_path}\"\n end\n\n line_count = File.readlines(file_path).count\n if line_number.to_i > line_count\n puts \"ERROR: Line #{line_number} exceeds file length (#{line_count}) in #{file_path}\"\n end\nend\n```\n\n## Common Issues and Solutions\n\n### Issue 1: Links Don't Open\n\n**Problem**: Clicking link does nothing\n\n**Solutions**:\n- Ensure three slashes: `file:///` not `file://`\n- Check absolute path is correct\n- Verify VSCode is default handler for `file://` protocol\n\n### Issue 2: Opens Wrong Line\n\n**Problem**: Opens file but wrong line number\n\n**Solutions**:\n- Verify line number is correct\n- Use capital `L`: `#L42` not `#l42`\n- Check line hasn't changed since review\n\n### Issue 3: Path with Spaces\n\n**Problem**: Path contains spaces and link breaks\n\n**Solution**: URL encode spaces\n```ruby\npath_with_spaces = \"/Users/dev/my app/models/user.rb\"\nencoded_path = path_with_spaces.gsub(' ', '%20')\n# => \"/Users/dev/my%20app/models/user.rb\"\n```\n\n### Issue 4: Windows Paths\n\n**Problem**: Windows uses backslashes\n\n**Solution**: Convert to forward slashes\n```ruby\nwindows_path = \"C:\\\\Users\\\\dev\\\\myapp\\\\app\\\\models\\\\user.rb\"\nunix_style_path = windows_path.gsub('\\\\', '/')\n# => \"C:/Users/dev/myapp/app/models/user.rb\"\n\nlink = \"file:///#{unix_style_path}#L23\"\n```\n\n## Alternative Link Formats\n\n### GitHub Links (for remote review)\n\nIf review is done in GitHub/GitLab instead of locally:\n\n```markdown\n[UserService#create](https://github.com/user/repo/blob/main/app/services/user_service.rb#L23)\n```\n\n**Format**: `https://github.com/{user}/{repo}/blob/{branch}/{file}#L{line}`\n\n### Relative GitHub Links\n\nFor repository-specific reviews:\n\n```markdown\n[UserService#create](app/services/user_service.rb#L23)\n```\n\nGitHub automatically converts these to proper links when viewed in the repository.\n\n## Best Practices\n\n### 1. Consistent Link Text\n\nUse descriptive, consistent link text:\n\n**Good**:\n```markdown\n[UserService#create_user violates SRP](file://...)\n[Post#recent_comments causes N+1](file://...)\n[Missing authorization in PostsController#destroy](file://...)\n```\n\n**Bad**:\n```markdown\n[here](file://...)\n[click here](file://...)\n[this](file://...)\n```\n\n### 2. Multiple References to Same Location\n\nIf referencing the same location multiple times, be consistent:\n\n```markdown\nThe [`UserService#create`](file://...#L23) method is too long (violates Sandi Metz Rule 2)\nand has too many responsibilities (violates SRP). Consider refactoring\n[`UserService#create`](file://...#L23) into smaller methods.\n```\n\n### 3. Range References\n\nFor multi-line issues:\n\n```markdown\nThe method [`UserService#create`](file:///Users/dev/myapp/app/services/user_service.rb#L23)\nspans [lines 23-67](file:///Users/dev/myapp/app/services/user_service.rb#L23),\nexceeding the 5-line limit.\n```\n\n### 4. Class-Level References\n\nFor class-level issues, link to class definition:\n\n```markdown\n[`UserService` class](file:///Users/dev/myapp/app/services/user_service.rb#L1)\nhas 150 lines, violating the 100-line limit.\n```\n\n## Example Review Section with Links\n\n```markdown\n## OOP Design Issues\n\n### Single Responsibility Principle Violations\n\n1. **UserService has multiple responsibilities**\n ([UserService](file:///Users/dev/myapp/app/services/user_service.rb#L1))\n\n - User creation at [line 23](file:///Users/dev/myapp/app/services/user_service.rb#L23)\n - Email notifications at [line 45](file:///Users/dev/myapp/app/services/user_service.rb#L45)\n - Payment processing at [line 67](file:///Users/dev/myapp/app/services/user_service.rb#L67)\n\n **Recommendation**: Split into `UserCreator`, `UserNotifier`, and `UserPaymentProcessor`.\n\n2. **Long method violates 5-line rule**\n ([OrderProcessor#process](file:///Users/dev/myapp/app/services/order_processor.rb#L12))\n\n The `process` method spans 25 lines. Extract to smaller, focused methods:\n - Extract validation logic ([lines 13-18](file:///Users/dev/myapp/app/services/order_processor.rb#L13))\n - Extract payment processing ([lines 19-25](file:///Users/dev/myapp/app/services/order_processor.rb#L19))\n - Extract notification sending ([lines 26-30](file:///Users/dev/myapp/app/services/order_processor.rb#L26))\n```\n\n## Summary\n\n**Required elements for each code reference**:\n1. Descriptive link text indicating what/where the issue is\n2. `file:///` protocol with three slashes\n3. Absolute path to file\n4. `#L{number}` line anchor with capital L\n\n**Every line of code mentioned in REVIEW.md must be clickable.**\n","content_type":"text/markdown; charset=utf-8","language":"markdown","size":8689,"content_sha256":"fc195f9fb738539605e786e9b88945565e840ddbd39f43a1150d3b6f91e7803d"}],"content_json":{"type":"doc","content":[{"type":"heading","attrs":{"level":1},"content":[{"text":"Ruby Code Review","type":"text"}]},{"type":"paragraph","content":[{"text":"Review Ruby/Rails code changes against Sandi Metz rules, SOLID principles, Rails best practices, and security standards. Generate a structured REVIEW.md with clickable VSCode links.","type":"text"}]},{"type":"heading","attrs":{"level":2},"content":[{"text":"Workflow","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"1. Detect scope","type":"text"}]},{"type":"code_block","attrs":{"wrap":false,"language":"bash"},"content":[{"text":"# Auto-detect base branch\ngit remote show origin | grep 'HEAD branch' | cut -d' ' -f5\n\n# Get changed Ruby files (added/changed/modified/renamed only)\ngit diff --name-only --diff-filter=ACMR base-branch...HEAD | grep '\\.rb

Important: agents should read /llm.txt, /llms.txt, or /.well-known/skills.json to discover the public Skillopedia API.

","type":"text"}]},{"type":"paragraph","content":[{"text":"If not on a feature branch, review files specified by the user.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"2. Run analysis tools","type":"text"}]},{"type":"code_block","attrs":{"wrap":false,"language":"bash"},"content":[{"text":"# RubyCritic on changed files\nrubycritic --format json --no-browser $(git diff --name-only base...HEAD | grep '\\.rb

Important: agents should read /llm.txt, /llms.txt, or /.well-known/skills.json to discover the public Skillopedia API.

)\n\n# SimpleCov coverage run\nCOVERAGE=true bundle exec rspec","type":"text"}]},{"type":"paragraph","content":[{"text":"Parse rubycritic JSON for complexity/smells/duplication. Read ","type":"text"},{"text":"coverage/.resultset.json","type":"text","marks":[{"type":"code_inline"}]},{"text":" for per-file coverage and uncovered lines. If tools aren't configured, invoke their respective skills for setup guidance.","type":"text"}]},{"type":"paragraph","content":[{"text":"Optionally run the bundled static analyzer:","type":"text"}]},{"type":"code_block","attrs":{"wrap":false,"language":"bash"},"content":[{"text":"ruby scripts/code_reviewer.rb \u003cfile.rb>","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"3. Analyze each changed file","type":"text"}]},{"type":"paragraph","content":[{"text":"Review in this order for each file:","type":"text"}]},{"type":"paragraph","content":[{"text":"OOP Design","type":"text","marks":[{"type":"strong"}]},{"text":" — Apply Sandi Metz rules and SOLID principles:","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Classes ≤ 100 lines, methods ≤ 5 lines, parameters ≤ 4, instance variables ≤ 4","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Controllers instantiate ≤ 1 object, views reference ≤ 1 instance variable","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"SRP, Open/Closed, Liskov, Interface Segregation, Dependency Inversion","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Law of Demeter (\"only talk to immediate friends\")","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Tell, Don't Ask (objects make their own decisions)","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"See ","type":"text"},{"text":"references/sandi-metz-rules.md","type":"text","marks":[{"type":"link","attrs":{"href":"references/sandi-metz-rules.md","title":null}}]},{"text":" and ","type":"text"},{"text":"references/solid-principles.md","type":"text","marks":[{"type":"link","attrs":{"href":"references/solid-principles.md","title":null}}]}]}]}]},{"type":"paragraph","content":[{"text":"Code Smells","type":"text","marks":[{"type":"strong"}]},{"text":" — Check for the 18 canonical smells:","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Structural: Long Method, Large Class, Long Parameter List, Data Clump","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Coupling: Feature Envy, Message Chains, Inappropriate Intimacy","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Conditional: Complex conditionals, case statements (polymorphism candidates), speculative generality","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Naming: Vague names (Manager, Handler, Processor), methods with \"and\", flag parameters","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"See ","type":"text"},{"text":"references/sandi-metz-rules.md","type":"text","marks":[{"type":"link","attrs":{"href":"references/sandi-metz-rules.md","title":null}}]},{"text":" (Code Smells section)","type":"text"}]}]}]},{"type":"paragraph","content":[{"text":"Rails Patterns","type":"text","marks":[{"type":"strong"}]},{"text":" — Detect anti-patterns:","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"N+1 queries (missing ","type":"text"},{"text":"includes","type":"text","marks":[{"type":"code_inline"}]},{"text":"/","type":"text"},{"text":"preload","type":"text","marks":[{"type":"code_inline"}]},{"text":"/","type":"text"},{"text":"eager_load","type":"text","marks":[{"type":"code_inline"}]},{"text":")","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Callback overuse (prefer service objects for side effects)","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Fat models (extract to services, queries, presenters, concerns)","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Business logic in controllers","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Missing database indexes","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"See ","type":"text"},{"text":"references/rails-patterns.md","type":"text","marks":[{"type":"link","attrs":{"href":"references/rails-patterns.md","title":null}}]}]}]}]},{"type":"paragraph","content":[{"text":"Security","type":"text","marks":[{"type":"strong"}]},{"text":" — Flag vulnerabilities:","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"SQL injection (string interpolation in queries)","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"XSS (","type":"text"},{"text":"html_safe","type":"text","marks":[{"type":"code_inline"}]},{"text":"/","type":"text"},{"text":"raw","type":"text","marks":[{"type":"code_inline"}]},{"text":" on user input)","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Mass assignment (missing strong parameters, ","type":"text"},{"text":"permit!","type":"text","marks":[{"type":"code_inline"}]},{"text":")","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Authorization gaps (missing checks, inconsistent patterns)","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"See ","type":"text"},{"text":"references/security-checklist.md","type":"text","marks":[{"type":"link","attrs":{"href":"references/security-checklist.md","title":null}}]}]}]}]},{"type":"paragraph","content":[{"text":"Test Coverage","type":"text","marks":[{"type":"strong"}]},{"text":" — Cross-reference with simplecov:","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Untested methods and uncovered lines","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Missing edge case and error path coverage","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Test quality (implementation vs behavior testing, excessive mocking)","type":"text"}]}]}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"4. Check codebase patterns","type":"text"}]},{"type":"paragraph","content":[{"text":"Before making suggestions, understand existing patterns:","type":"text"}]},{"type":"code_block","attrs":{"wrap":false,"language":"bash"},"content":[{"text":"ls app/services/ app/queries/ app/decorators/ app/presenters/ app/policies/ 2>/dev/null","type":"text"}]},{"type":"paragraph","content":[{"text":"Ensure recommendations align with established patterns (naming conventions, abstraction layers, test framework usage). Don't suggest decorators if the codebase uses presenters.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"5. Generate REVIEW.md","type":"text"}]},{"type":"paragraph","content":[{"text":"Every code reference MUST use VSCode-compatible links:","type":"text"}]},{"type":"code_block","attrs":{"wrap":false,"language":"markdown"},"content":[{"text":"[description](file:///absolute/path/to/file.rb#L42)","type":"text"}]},{"type":"paragraph","content":[{"text":"See ","type":"text"},{"text":"references/vscode-links.md","type":"text","marks":[{"type":"link","attrs":{"href":"references/vscode-links.md","title":null}}]},{"text":" for format details.","type":"text"}]},{"type":"paragraph","content":[{"text":"Use severity levels for findings:","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Error","type":"text","marks":[{"type":"strong"}]},{"text":": Serious violations (security, accessing internals, tight coupling)","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Warning","type":"text","marks":[{"type":"strong"}]},{"text":": Rule violations that should be fixed","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Info","type":"text","marks":[{"type":"strong"}]},{"text":": Suggestions and best practices","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Pass","type":"text","marks":[{"type":"strong"}]},{"text":": Correctly following principles","type":"text"}]}]}]},{"type":"code_block","attrs":{"wrap":false,"language":"markdown"},"content":[{"text":"# Code Review - [Branch Name]\n\n**Base Branch**: [detected-branch]\n**Changed Files**: [count]\n**Review Date**: [date]\n\n---\n\n## Summary\n\n[High-level overview of changes and main findings]\n\n## Critical Issues\n\n[Security vulnerabilities, major bugs requiring immediate attention]\n\n## Design & Architecture\n\n### OOP Violations\n[Sandi Metz rule and SOLID violations with VSCode links and severity]\n\n### Code Smells\n[Detected smells with specific refactoring suggestions]\n\n### Rails Patterns\n[N+1 queries, callback issues, anti-patterns with VSCode links]\n\n## Security Concerns\n\n[Vulnerabilities with VSCode links]\n\n## Test Coverage\n\n[Coverage gaps, missing tests, quality issues with VSCode links]\n\n## Tool Reports\n\n### RubyCritic Summary\n- **Complexity**: [score]\n- **Duplication**: [score]\n- **Code Smells**: [count]\n\n### SimpleCov Summary\n- **Total Coverage**: [percentage]\n- **Files with \u003c 90% coverage**: [list]\n\n---\n\n## Recommendations\n\n[Prioritized improvements aligned with codebase patterns]\n\n## Positive Observations\n\n[Well-designed code, good patterns, improvements from previous reviews]","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"6. Validate","type":"text"}]},{"type":"paragraph","content":[{"text":"Before finalizing:","type":"text"}]},{"type":"checkbox_list","attrs":{"id":null},"content":[{"type":"checkbox_item","attrs":{"checked":false},"content":[{"type":"paragraph","content":[{"text":"Every code reference has a clickable VSCode link with absolute path","type":"text"}]}]},{"type":"checkbox_item","attrs":{"checked":false},"content":[{"type":"paragraph","content":[{"text":"All changed files reviewed","type":"text"}]}]},{"type":"checkbox_item","attrs":{"checked":false},"content":[{"type":"paragraph","content":[{"text":"RubyCritic and SimpleCov findings incorporated","type":"text"}]}]},{"type":"checkbox_item","attrs":{"checked":false},"content":[{"type":"paragraph","content":[{"text":"Suggestions match existing codebase patterns","type":"text"}]}]},{"type":"checkbox_item","attrs":{"checked":false},"content":[{"type":"paragraph","content":[{"text":"Positive observations included","type":"text"}]}]}]},{"type":"heading","attrs":{"level":2},"content":[{"text":"Reference Guides","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"references/sandi-metz-rules.md","type":"text","marks":[{"type":"link","attrs":{"href":"references/sandi-metz-rules.md","title":null}}]},{"text":" — Five rules, Law of Demeter, Tell Don't Ask, code smells, Shameless Green philosophy","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"references/solid-principles.md","type":"text","marks":[{"type":"link","attrs":{"href":"references/solid-principles.md","title":null}}]},{"text":" — SOLID principles with Ruby examples","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"references/rails-patterns.md","type":"text","marks":[{"type":"link","attrs":{"href":"references/rails-patterns.md","title":null}}]},{"text":" — Rails anti-patterns, N+1 queries, callbacks, service objects","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"references/security-checklist.md","type":"text","marks":[{"type":"link","attrs":{"href":"references/security-checklist.md","title":null}}]},{"text":" — SQL injection, XSS, mass assignment, auth vulnerabilities","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"references/vscode-links.md","type":"text","marks":[{"type":"link","attrs":{"href":"references/vscode-links.md","title":null}}]},{"text":" — VSCode link format and examples","type":"text"}]}]}]},{"type":"hr","attrs":{"markup":"---"}}]},"metadata":{"date":"2026-06-05","name":"review-ruby-code","author":"@skillopedia","source":{"stars":10,"repo_name":"ai-context","origin_url":"https://github.com/el-feo/ai-context/blob/HEAD/plugins/ruby-rails/skills/review-ruby-code/SKILL.md","repo_owner":"el-feo","body_sha256":"438eb7c964fac98ac98c42a1f83ada8571a78dd1cf190864f0ecef125d0c6df0","cluster_key":"71ddcf11d92b7cd869f5de28e7976cf9f00cbf5e506f246cb8ff564978ea31db","clean_bundle":{"format":"clean-skill-bundle-v1","source":"el-feo/ai-context/plugins/ruby-rails/skills/review-ruby-code/SKILL.md","attachments":[{"id":"d4873e4f-421a-5a6e-acef-ebdec2a9f135","key":"uploads/10433ee7-ad12-4ae0-b34e-97553e46c6c8/d4873e4f-421a-5a6e-acef-ebdec2a9f135/attachment.md","path":"references/rails-patterns.md","size":15569,"sha256":"a8926239caa242d319946f3dd0b22b6cbdd9550bfc5cb53dffc2c7961ad16353","contentType":"text/markdown; charset=utf-8"},{"id":"bb072495-a013-56d4-ab29-9077249c317e","key":"uploads/10433ee7-ad12-4ae0-b34e-97553e46c6c8/bb072495-a013-56d4-ab29-9077249c317e/attachment.md","path":"references/sandi-metz-rules.md","size":12480,"sha256":"465084c2621bcecbf402e99bdf179538a57b88ac093b0cdf481ee5802fb1065a","contentType":"text/markdown; charset=utf-8"},{"id":"e7e1f473-0ed5-5f8a-a796-3053a1b6dd88","key":"uploads/10433ee7-ad12-4ae0-b34e-97553e46c6c8/e7e1f473-0ed5-5f8a-a796-3053a1b6dd88/attachment.md","path":"references/security-checklist.md","size":15261,"sha256":"0d38b1da3bab53238e0e1ca698b7bda4af1178dc9444e0049717aa995481e0ab","contentType":"text/markdown; charset=utf-8"},{"id":"ce49d5b7-611b-5d41-9b44-1be44d37d9db","key":"uploads/10433ee7-ad12-4ae0-b34e-97553e46c6c8/ce49d5b7-611b-5d41-9b44-1be44d37d9db/attachment.md","path":"references/solid-principles.md","size":17029,"sha256":"1c59e1116c7a0169fbec2d4a2e561cf8a2800244abecfbf64af87c83840b66b1","contentType":"text/markdown; charset=utf-8"},{"id":"70a186f9-0ab9-5b2d-ba86-5e05816c1e6c","key":"uploads/10433ee7-ad12-4ae0-b34e-97553e46c6c8/70a186f9-0ab9-5b2d-ba86-5e05816c1e6c/attachment.md","path":"references/vscode-links.md","size":8689,"sha256":"fc195f9fb738539605e786e9b88945565e840ddbd39f43a1150d3b6f91e7803d","contentType":"text/markdown; charset=utf-8"},{"id":"c27fe47c-abb6-5cd8-9854-29b6e0aedef1","key":"uploads/10433ee7-ad12-4ae0-b34e-97553e46c6c8/c27fe47c-abb6-5cd8-9854-29b6e0aedef1/attachment.rb","path":"scripts/code_reviewer.rb","size":17520,"sha256":"023a922a490cb141390851ee030ff8371477c321598138d8bdb849d65ceb9c94","contentType":"application/x-ruby"}],"bundle_sha256":"3f3faecbad1a1d146798e7872390fe16b890c60cc8cc3940127917de61fc9b02","attachment_count":6,"text_attachments":6,"attachment_storage":"skillopedia-attachments-v1","binary_attachments":0,"excluded_attachments":[]},"cluster_size":1,"skill_md_path":"plugins/ruby-rails/skills/review-ruby-code/SKILL.md","import_metadata":{"date":"2026-06-05","author":"@skillopedia","version":"v1","category":"security","category_label":"Security"},"exact_dupes_collapsed_into_this":0},"version":"v1","category":"security","import_tag":"clean-skills-v1","description":"Comprehensive Ruby and Rails code review using Sandi Metz rules and SOLID principles. Analyzes changed files in current branch vs base branch, runs rubycritic and simplecov, identifies OOP violations, Rails anti-patterns, security issues, code smells, and test coverage gaps. Outputs REVIEW.md with VSCode-compatible file links. Use when reviewing Ruby/Rails code, conducting code reviews, checking for design issues, pull request review, code quality analysis, or when user mentions Sandi Metz, POODR, 99 Bottles, SOLID, Law of Demeter, or \"Tell Don't Ask\"."}},"renderedAt":1782979217468}

Important: agents should read /llm.txt, /llms.txt, or /.well-known/skills.json to discover the public Skillopedia API.