Remove AutomatedPlanReviewer and review-bot machinery#121
Merged
Conversation
![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: 7aff15a552
ℹ️ 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".
Comment on lines
+2
to
+3
| def up | ||
| drop_table :coplan_automated_plan_reviewers |
There was a problem hiding this comment.
The deployable host app keeps copied CoPlan engine migrations under db/migrate (the existing engine migrations all have .co_plan.rb copies there), but this commit only adds the drop under engine/db/migrate while also updating db/schema.rb. In existing environments that run the host app’s db:migrate, this migration will not be picked up, so coplan_automated_plan_reviewers remains in production even though the model/admin code and schema entry are gone. Please add the corresponding copied migration under db/migrate or explicitly include the engine migration path.
Useful? React with 👍 / 👎.
The scheduled-review-bot feature ("AutomatedPlanReviewer") never produced
useful enough feedback to justify its surface area, and its presence is now
blocking a cleaner refactor of the AI-call abstraction (\`CoPlan::Ai\` →
\`config.simple_ai_call\`, coming in a follow-up).
Removes:
- Model + table (\`coplan_automated_plan_reviewers\`) — drop migration included
- Job (\`AutomatedReviewJob\`) and trigger service (\`Plans::TriggerAutomatedReviews\`)
- Helpers (\`Plans::ReviewPromptFormatter\`, \`Plans::ReviewResponseParser\`)
- Web controller + route (\`POST /plans/:id/automated_reviews\`)
- Plan-header "Run Reviewer" dropdown
- ActiveAdmin page
- Seeded default reviewers (security/scalability/routing) and their prompt files
- \`CoPlan::AiProviders::Anthropic\` (was a stub; only ever called from the
removed job)
Kept intentionally:
- The \`cloud_persona\` value in
\`PlanEvent::ACTOR_TYPES\` / \`PlanVersion::ACTOR_TYPES\` /
\`EditSession::ACTOR_TYPES\` / \`EditLease::HOLDER_TYPES\` /
\`Comment::AUTHOR_TYPES\`. These represent *external* AI agents that
interact with CoPlan over the API (remote agent creates an EditSession or
posts a comment as cloud_persona); only the internal scheduled review-bot
machinery is gone.
- \`CoPlan::Ai\` and \`CoPlan::AiProviders::OpenAi\` (still used by
SummarizePlanJob).
- \`CommentsHelper#comment_author_name\` — falls back to
\`comment.author_type\` for non-user authors, matching the previous
behavior when no reviewer record matched.
Tests: 916 → 863 (all passing). 53 removed examples == the deleted specs.
🤖 Generated with [Amp](https://ampcode.com)
Amp-Thread-ID: https://ampcode.com/threads/T-019e84ca-1bd1-7435-acbc-73b00095d896
Co-authored-by: Amp <amp@ampcode.com>
HamptonMakes
force-pushed
the
hampton/coplan-23/remove-review-bots
branch
from
7aff15a to
c7e2124
Compare
HamptonMakes
added a commit
that referenced
this pull request
Jun 2, 2026
Replaces the engine's hard-coded \`CoPlan::AiProviders::OpenAi.call\`
plumbing with a host-supplied callable so deployments can swap in any
backend (raw OpenAI, an internal LLM gateway like Square's Gondola, a
test stub, etc.) without touching engine code.
CoPlan::Ai.call(messages: [
{ role: :system, content: "You are concise." },
{ role: :user, content: "Hi" },
])
# …with sugar for the 90% case:
CoPlan::Ai.call(system: "You are concise.", user: "Hi")
The messages-array shape is the canonical wire format every chat-style
LLM API speaks (OpenAI chat completions, Anthropic Messages, Gondola,
Bedrock), so the host's lambda passes it straight through with zero
translation.
# config/initializers/coplan.rb
config.ai_call = ->(messages:) {
GondolaProvider.call(messages: messages, model: "gpt-4o")
}
Model choice and deployment policy (which provider, project, rate
limits, observability) live inside the callable — not on the engine's
API surface. The engine never sees an API key or a model name.
When \`ENV["OPENAI_API_KEY"]\` is present, Configuration auto-wires
\`CoPlan::AiProviders::OpenAi\` so a standalone deployment works with
zero config. The OpenAI plugin now accepts a messages array and honors
\`ENV["OPENAI_MODEL"]\` (default gpt-4o) for per-deployment model
tuning without code changes.
- \`Configuration#ai_api_key\`, \`#ai_base_url\`, \`#ai_model\` accessors
(deployment concerns now live inside the provider lambda)
- \`coplan-square\`-style \`config.ai_api_key = ...\` lines in the host
initializer (auto-wire handles it)
- \`Configuration#ai_call\` (with docstring) + \`#ai_call_configured?\`
predicate
- \`CoPlan::Ai::NoProviderError\` (subclass of \`Ai::Error\`) for jobs
that want to distinguish "not configured" from "call failed"
Any exception raised inside the host's lambda is wrapped in
\`CoPlan::Ai::Error\`, so existing call sites
(\`SummarizePlanJob.discard_on CoPlan::Ai::Error\`) continue to work
unchanged without coupling to the provider.
Tests: 868 examples, 0 failures.
Stacked on #121 (review-bot removal).
🤖 Generated with [Amp](https://ampcode.com)
Amp-Thread-ID: https://ampcode.com/threads/T-019e84ca-1bd1-7435-acbc-73b00095d896
Co-authored-by: Amp <amp@ampcode.com>
HamptonMakes
added a commit
that referenced
this pull request
Jun 2, 2026
* Introduce config.ai_call: pluggable AI surface (messages-array)
Replaces the engine's hard-coded \`CoPlan::AiProviders::OpenAi.call\`
plumbing with a host-supplied callable so deployments can swap in any
backend (raw OpenAI, an internal LLM gateway like Square's Gondola, a
test stub, etc.) without touching engine code.
CoPlan::Ai.call(messages: [
{ role: :system, content: "You are concise." },
{ role: :user, content: "Hi" },
])
# …with sugar for the 90% case:
CoPlan::Ai.call(system: "You are concise.", user: "Hi")
The messages-array shape is the canonical wire format every chat-style
LLM API speaks (OpenAI chat completions, Anthropic Messages, Gondola,
Bedrock), so the host's lambda passes it straight through with zero
translation.
# config/initializers/coplan.rb
config.ai_call = ->(messages:) {
GondolaProvider.call(messages: messages, model: "gpt-4o")
}
Model choice and deployment policy (which provider, project, rate
limits, observability) live inside the callable — not on the engine's
API surface. The engine never sees an API key or a model name.
When \`ENV["OPENAI_API_KEY"]\` is present, Configuration auto-wires
\`CoPlan::AiProviders::OpenAi\` so a standalone deployment works with
zero config. The OpenAI plugin now accepts a messages array and honors
\`ENV["OPENAI_MODEL"]\` (default gpt-4o) for per-deployment model
tuning without code changes.
- \`Configuration#ai_api_key\`, \`#ai_base_url\`, \`#ai_model\` accessors
(deployment concerns now live inside the provider lambda)
- \`coplan-square\`-style \`config.ai_api_key = ...\` lines in the host
initializer (auto-wire handles it)
- \`Configuration#ai_call\` (with docstring) + \`#ai_call_configured?\`
predicate
- \`CoPlan::Ai::NoProviderError\` (subclass of \`Ai::Error\`) for jobs
that want to distinguish "not configured" from "call failed"
Any exception raised inside the host's lambda is wrapped in
\`CoPlan::Ai::Error\`, so existing call sites
(\`SummarizePlanJob.discard_on CoPlan::Ai::Error\`) continue to work
unchanged without coupling to the provider.
Tests: 868 examples, 0 failures.
Stacked on #121 (review-bot removal).
🤖 Generated with [Amp](https://ampcode.com)
Amp-Thread-ID: https://ampcode.com/threads/T-019e84ca-1bd1-7435-acbc-73b00095d896
Co-authored-by: Amp <amp@ampcode.com>
* Address Codex feedback on #122
- Auto-wire OpenAI default unconditionally (lazy). The provider already
resolves its key from Rails credentials -> ENV and raises
CoPlan::Ai::Error at call time when neither is set, so credential-only
deployments now wire up correctly instead of silently leaving ai_call
nil. AI jobs still discard cleanly.
- Drop unused ai_call_configured? helper (no longer meaningful now that
the default is always wired; nothing called it).
- Update docs/HOST_APP_GUIDE.md to remove the obsolete ai_api_key /
ai_base_url / ai_model setters and document config.ai_call with the
built-in OpenAI auto-wire behavior, so hosts following the guide
don't NoMethodError on boot.
Amp-Thread-ID: https://ampcode.com/threads/T-019e84ca-1bd1-7435-acbc-73b00095d896
Co-authored-by: Amp <amp@ampcode.com>
---------
Co-authored-by: Amp <amp@ampcode.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The scheduled-review-bot feature (
AutomatedPlanReviewer) never produced useful enough feedback to justify its surface area, and its presence is now blocking a cleaner refactor of the AI-call abstraction (CoPlan::Ai→config.simple_ai_call, coming in a follow-up PR).What's removed
engine/app/models/coplan/automated_plan_reviewer.rb; new migration20260602170000_drop_coplan_automated_plan_reviewers.rbengine/app/jobs/coplan/automated_review_job.rbengine/app/services/coplan/plans/trigger_automated_reviews.rb(called fromPlansController#update_statusand the API v1 equivalent)engine/app/services/coplan/plans/{review_prompt_formatter,review_response_parser}.rbPOST /plans/:id/automated_reviewsapp/admin/automated_plan_reviewers.rbengine/app/services/coplan/ai_providers/anthropic.rb— was a "not yet implemented" stub; its only caller wasAutomatedReviewJob, so it became dead codeWhat's kept intentionally
cloud_personavalue inPlanEvent::ACTOR_TYPES/PlanVersion::ACTOR_TYPES/EditSession::ACTOR_TYPES/EditLease::HOLDER_TYPES/Comment::AUTHOR_TYPES. These represent external AI agents that interact with CoPlan over the API (a remote agent creates an EditSession or posts a comment ascloud_persona). Only the internal scheduled review-bot machinery is removed.CoPlan::AiandCoPlan::AiProviders::OpenAi(still used bySummarizePlanJob).CommentsHelper#comment_author_name— now falls back tocomment.author_typefor non-user authors. Matches the previous fallback ("Reviewer") for any orphaned cloud_persona comments.Verification
bundle exec rspecbin/rails routesautomated_reviewsroute remainsplans/PLAN.md,docs/PLAN.mdnarrative docs, and the originalcreate_tablemigration — all intentional)Follow-ups (separate PRs)
config.simple_ai_call(renameCoPlan::Aito read from a host-supplied callable; auto-wire the built-in OpenAI plugin whenENV["OPENAI_API_KEY"]is set). Once that lands, coplan-square will wire a Gondola-backed lambda — no raw OpenAI key in Keywhiz.🤖 Generated with Amp