Rails 8 PR Review

Input: github_pr_url
Output: pr_context
Gate: none
On error: abort

• Start by reading the PR description thoroughly to understand the "Why" behind the changes.
• Locate and read mandatory repository instruction files: AGENTS.md, CONTRIBUTING.md, README.md, or any /docs/architecture/ files.
• Requirement: Treat any rules found in these files as mandatory constraints that override general defaults.

Input: pr_context
Output: intent_analysis
Gate: none
On error: abort

• Before judging the code, identify the specific problem the PR solves.
• Map out the affected user flows.
• Check: Does the implementation actually match the stated intent? Is there "scope creep" (unrelated changes) or missing edge cases for the core feature?

Input: pr_context
Output: file_analysis
Gate: none
On error: continue

• Read each changed file within its surrounding context, not just the diff chunks.
• Use github_read_file to see the full content of related files:
  • If a Controller changes, check the associated Model, Routes, and Policy/Ability.
  • If a Model changes, check Migrations, Factories/Fixtures, and Serializers/Views.
  • If a View changes, check the Controller's @variables and any Stimulus controllers or Turbo Frames involved.

Input: file_analysis
Output: rails_convention_check
Gate: none
On error: continue

Enforce the following "Basecamp-style" conventions:
- Turbo 8 Morphing: Prefer Page Refresh Morphing (<meta name="turbo-refresh-method" content="morph">) for standard UI updates. If you see manual turbo_stream responses for simple state changes (like updating a counter or a list item), suggest a redirect/morph instead.
- Turbo Frames: Ensure frames are used for isolated interactivity (e.g., inline editing, lazy loading) and use dom_id(model) for stable IDs.
- Turbo Streams: Only use for multi-user real-time updates (via broadcasts_refreshes) or specific animations.
- Stimulus: Verify JS is used only as "sprinkles" for UI behavior. No business logic in Stimulus. Avoid keeping server-side state in the client.

Input: file_analysis
Output: arch_analysis
Gate: none
On error: continue

• Fat Models, Skinny Controllers: Business logic should live in models or scoped concerns. Controllers should handle request flow only.
• Delegated Types: For complex polymorphic-like content (e.g., a "Post" that can be a "Video", "Article", or "Link"), check if Delegated Type is used to keep the DB and models clean.
• Naming & Reuse: Verify consistent naming that follows the existing project patterns. Favor Rails-native solutions (e.g., Enum, GlobalID) over custom reinventions.

Input: file_analysis
Output: security_audit
Gate: none
On error: continue

• Scoping: Verify every query is scoped to the current tenant (e.g., Current.account.projects vs Project.all).
• Identifiers: If the app uses UUIDs publicly, ensure integer IDs are never leaked in URLs or APIs.
• Strong Params: Check for permit vulnerabilities or missing required fields.
• AuthZ: Ensure every action is wrapped in a permission check (Pundit, CanCanCan, or custom policy).

Input: file_analysis
Output: db_safety_audit
Gate: none
On error: continue

• N+1 Queries: Look for associations called in loops without .includes or .preload.
• Migrations: Check for safety and reversibility. Ensure indexes exist for foreign keys and frequently searched columns.
• Constraints: Verify that model-level validates calls have matching database-level NOT NULL or UNIQUE constraints where applicable.

Input: file_analysis
Output: async_report
Gate: none
On error: continue

• Jobs/Mailers: Ensure jobs are idempotent (safe to retry). Avoid passing complex objects; pass GlobalIDs or IDs instead.
• Callbacks: Watch for "callback hell"—side effects in after_save that trigger slow external API calls or deep state changes that should be in a Service or Job.
• Transactions: Ensure multi-record updates are wrapped in ActiveRecord::Base.transaction.

Input: file_analysis
Output: test_coverage_report
Gate: none
On error: continue

• Verify the PR adds or updates Specs (System, Request, Model) for the changed behavior.
• Check for Regression Tests: If this PR fixes a bug, is there a test that would have caught it?
• Quality: Avoid "happy path only" tests. Look for coverage of nil cases, invalid inputs, and unauthorized access.

Input: file_analysis
Output: ops_impact_report
Gate: none
On error: continue

• Feedback: Check for appropriate flash messages or "toast" notifications after actions.
• States: Are loading and error states handled in the UI (especially within Turbo Frames)?
• Rollout: Are there breaking changes to APIs, config files, or env variables that require specific deployment steps?

Input: all_steps_output
Output: review_report
Gate: none
On error: abort

Prioritize findings over style comments. Output findings ordered by severity:

Format:
[severity] short title
- Why: Why this is a problem
- Where: File path and context
- Impact: What behavior could break
- Fix: Suggested fix or direction

Severity Levels:
- P0: Critical (Security vulnerability, data loss, immediate crash).
- P1: High (Major bug, broken flow, significant regression risk).
- P2: Medium (Correctness issue, N+1 query, maintainability problem).
- P3: Minor (Style nit, worthwhile follow-up, minor optimization).

If no issues are found, state "No issues found" and briefly list any assumptions or testing gaps.