Add automated review bots (Cloud Personas)#14
Conversation
- Replace minitest gem with rspec-rails and factory_bot_rails - Convert all 175 tests from test/ to spec/ (models, requests, services, helpers) - Convert YAML fixtures to FactoryBot factories with proper association chains - Factories derive org from parent objects to keep data consistent - Add sign_in_as helper and ActiveSupport::Testing::TimeHelpers to rails_helper - Update AGENTS.md to document new testing conventions - Remove old test/ directory Amp-Thread-ID: https://ampcode.com/threads/T-019c8c51-0ef2-723d-bab8-a95e7ecbf4e0 Co-authored-by: Amp <amp@ampcode.com>
Implements Phase 7 from the plan: - AiProviders::OpenAi service wrapping ruby-openai gem - AiProviders::Anthropic stub for future implementation - AutomatedReviewJob (SolidQueue) that calls AI and creates review comments - Plans::TriggerAutomatedReviews service for auto-triggering on status change - AutomatedReviewsController for manual 'Run Reviewer' from the UI - Dropdown Stimulus controller + CSS for reviewer selection - 20 new specs covering all components (all 195 specs pass) Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-019c8c64-60ae-7133-b471-efb8f56a18a9
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 119ed28a68
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| def perform(plan_id:, reviewer_id:, triggered_by: nil) | ||
| plan = Plan.find(plan_id) | ||
| reviewer = AutomatedPlanReviewer.find(reviewer_id) | ||
| version = plan.current_plan_version |
There was a problem hiding this comment.
perform reads plan.current_plan_version when the job runs, so status-triggered reviews can analyze a newer revision than the one that caused the enqueue if someone edits the plan before workers execute. In that case the generated comment is attached to the wrong revision and review provenance becomes inaccurate for the status transition. Pass a plan_version_id (or content snapshot) at enqueue time and use that in the job.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0263f1d — the job now accepts plan_version_id: at enqueue time and uses that pinned version throughout.
| new_status = params[:status] | ||
| if Plan::STATUSES.include?(new_status) && @plan.update(status: new_status) | ||
| broadcast_plan_update(@plan) | ||
| Plans::TriggerAutomatedReviews.call(plan: @plan, new_status: new_status, triggered_by: current_user) |
There was a problem hiding this comment.
This call runs whenever update_status succeeds, but @plan.update(status: new_status) returns true even if the status is unchanged, so repeated requests with the current status will enqueue duplicate reviewer jobs and produce duplicate AI comments/cost. Guard this enqueue behind an actual status change (for example, checking saved_change_to_status? after update).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0263f1d — update_status now checks saved_change_to_status? before enqueuing reviewer jobs.
Instead of one big comment, the AI reviewer now returns structured JSON with individual feedback items. Each item becomes its own CommentThread, anchored to the specific plan text it references. - ReviewPromptFormatter: wraps reviewer prompt with JSON output instructions - ReviewResponseParser: parses JSON response, validates anchor_text against plan content (demotes non-matching anchors to blockquotes), gracefully falls back to single unanchored comment for non-JSON responses - Updated AutomatedReviewJob to create one thread per feedback item - 10 new specs for parser + formatter, updated job specs (205 total, 0 failures) Amp-Thread-ID: https://ampcode.com/threads/T-019c8c64-60ae-7133-b471-efb8f56a18a9 Co-authored-by: Amp <amp@ampcode.com>
…anges - Pass plan_version_id to AutomatedReviewJob at enqueue time so the job reviews the version that triggered the review, not whatever is current when the worker picks it up - Guard TriggerAutomatedReviews behind saved_change_to_status? so setting the same status twice doesn't enqueue duplicate review jobs - Added spec for pinned version behavior Amp-Thread-ID: https://ampcode.com/threads/T-019c8c64-60ae-7133-b471-efb8f56a18a9 Co-authored-by: Amp <amp@ampcode.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0263f1d8b3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| end | ||
|
|
||
| def normalize_item(item) | ||
| anchor = item["anchor_text"].presence |
There was a problem hiding this comment.
parse_json accepts any JSON array, but normalize_item immediately indexes each element as a hash via item["anchor_text"]. If the model returns a syntactically valid array containing non-object entries (for example "[\"Looks good\"]"), this raises NoMethodError, the job fails, and no reviewer comments are produced. Since AI output is only best-effort schema-compliant, the parser should guard element types and gracefully fall back instead of crashing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 09bde02 — normalize_item now returns nil (and is skipped via filter_map) unless the item is a Hash. Added spec coverage for ["Looks good"]-style AI responses.
|
|
||
| def normalize_item(item) | ||
| anchor = item["anchor_text"].presence | ||
| comment = item["comment"].to_s.strip |
There was a problem hiding this comment.
item["comment"].to_s.strip converts missing or null comments into an empty string, but the parser still returns that item as valid feedback. Downstream, AutomatedReviewJob creates the thread first and then fails Comment validation (body_markdown presence), which can leave orphaned threads and cause duplicate partial output on retries. Filter or default blank comments during parsing so invalid items never reach persistence.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 09bde02 — blank/null comments are now filtered out during parsing so they never reach persistence. No more orphaned threads. Added spec coverage for both blank and null comment cases.
- Return nil from normalize_item for non-Hash array elements (e.g. plain strings) - Return nil for items with blank/null comments to prevent orphaned threads - Use filter_map to drop nil results - Add specs for all three edge cases Amp-Thread-ID: https://ampcode.com/threads/T-019c9093-c658-747b-b2a7-255d089449ed Co-authored-by: Amp <amp@ampcode.com>
1 participant
What
Implements Phase 7 (Cloud Personas) - automated AI reviewers that analyze plans and leave comments.
Components
AiProviders::OpenAi- Service wrappingruby-openaigem (credentials viaOPENAI_API_KEYenv var or Rails credentials)AiProviders::Anthropic- Stub for future implementationAutomatedReviewJob- SolidQueue job that calls AI provider and creates a CommentThread + Comment ascloud_personaactor typePlans::TriggerAutomatedReviews- Service that enqueues review jobs for enabled reviewers matching a status triggerAutomatedReviewsController- Manual "Run Reviewer" trigger from the web UIHow it works
considering),TriggerAutomatedReviewsfinds enabled reviewers with matchingtrigger_statusesand enqueues jobsTests
20 new specs covering all components. Full suite: 195 specs, 0 failures.
Depends on