From b8c59f9bf4c2f5bcb20f899252bb8817bd2d413b Mon Sep 17 00:00:00 2001 From: Hampton Lintorn-Catlin Date: Mon, 1 Jun 2026 15:45:12 -0500 Subject: [PATCH] Hide AI provider behind CoPlan::Ai facade (COPLAN-24) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review feedback on PR #118: provider information was leaking into SummarizePlanJob, which should be transparent about which provider actually runs the prompt. Adds CoPlan::Ai — a thin facade that wraps AiProviders::OpenAi today and exposes CoPlan::Ai::Error so callers can discard_on without knowing the underlying provider. SummarizePlanJob now calls CoPlan::Ai.call and discards CoPlan::Ai::Error. AutomatedReviewJob is intentionally left alone — its reviewers configure their own provider+model per row, so it legitimately needs to dispatch between providers. The facade is for the common case where a caller just wants 'an AI'. Amp-Thread-ID: https://ampcode.com/threads/T-019e84ca-1d09-777c-baf1-b1f380ec013c Co-authored-by: Amp --- engine/app/jobs/coplan/summarize_plan_job.rb | 4 +-- engine/app/services/coplan/ai.rb | 23 +++++++++++++++++ spec/jobs/summarize_plan_job_spec.rb | 23 +++++++++-------- spec/services/ai_spec.rb | 26 ++++++++++++++++++++ 4 files changed, 62 insertions(+), 14 deletions(-) create mode 100644 engine/app/services/coplan/ai.rb create mode 100644 spec/services/ai_spec.rb diff --git a/engine/app/jobs/coplan/summarize_plan_job.rb b/engine/app/jobs/coplan/summarize_plan_job.rb index 4eed62e..a2955dc 100644 --- a/engine/app/jobs/coplan/summarize_plan_job.rb +++ b/engine/app/jobs/coplan/summarize_plan_job.rb @@ -15,7 +15,7 @@ class SummarizePlanJob < ApplicationJob queue_as :default - discard_on AiProviders::OpenAi::Error + discard_on CoPlan::Ai::Error def perform(plan_id:) plan = Plan.find_by(id: plan_id) @@ -37,7 +37,7 @@ def generate_summary(plan) content = plan.current_content return nil if content.blank? - AiProviders::OpenAi.call( + CoPlan::Ai.call( system_prompt: File.read(PROMPT_PATH), user_content: content ).to_s.strip.presence diff --git a/engine/app/services/coplan/ai.rb b/engine/app/services/coplan/ai.rb new file mode 100644 index 0000000..4ce9d6a --- /dev/null +++ b/engine/app/services/coplan/ai.rb @@ -0,0 +1,23 @@ +module CoPlan + # Provider-agnostic facade for AI calls where the caller doesn't care + # which underlying provider runs the prompt. Use this from any place + # that just wants "an AI" (e.g. SummarizePlanJob). + # + # Provider-specific jobs that need to pin a model or provider per call + # (e.g. AutomatedReviewJob, where each reviewer is configured with its + # own provider+model) should keep calling AiProviders::OpenAi / + # AiProviders::Anthropic directly. + # + # The provider chosen here is an implementation detail; swap it without + # touching callers. Raises CoPlan::Ai::Error on provider failure so + # callers can `discard_on` without knowing which provider is in use. + module Ai + class Error < StandardError; end + + def self.call(system_prompt:, user_content:) + AiProviders::OpenAi.call(system_prompt: system_prompt, user_content: user_content) + rescue AiProviders::OpenAi::Error => e + raise Error, e.message + end + end +end diff --git a/spec/jobs/summarize_plan_job_spec.rb b/spec/jobs/summarize_plan_job_spec.rb index bd4c271..bdfb538 100644 --- a/spec/jobs/summarize_plan_job_spec.rb +++ b/spec/jobs/summarize_plan_job_spec.rb @@ -7,14 +7,14 @@ let(:current_sha) { plan.current_plan_version.content_sha256 } before do - allow(CoPlan::AiProviders::OpenAi).to receive(:call).and_return("Fresh summary.") + allow(CoPlan::Ai).to receive(:call).and_return("Fresh summary.") end describe "#perform" do - it "passes the summarize prompt and plan content to the AI provider" do + it "passes the summarize prompt and plan content to CoPlan::Ai" do described_class.perform_now(plan_id: plan.id) - expect(CoPlan::AiProviders::OpenAi).to have_received(:call).with( + expect(CoPlan::Ai).to have_received(:call).with( system_prompt: File.read(CoPlan::SummarizePlanJob::PROMPT_PATH), user_content: plan.current_content ) @@ -32,7 +32,7 @@ end it "strips whitespace from the AI response before persisting" do - allow(CoPlan::AiProviders::OpenAi).to receive(:call).and_return(" trimmed\n\n") + allow(CoPlan::Ai).to receive(:call).and_return(" trimmed\n\n") described_class.perform_now(plan_id: plan.id) @@ -48,7 +48,7 @@ described_class.perform_now(plan_id: plan.id) - expect(CoPlan::AiProviders::OpenAi).not_to have_received(:call) + expect(CoPlan::Ai).not_to have_received(:call) expect(plan.reload.summary).to eq("Existing.") end @@ -65,7 +65,7 @@ end it "does not update when the AI returns blank" do - allow(CoPlan::AiProviders::OpenAi).to receive(:call).and_return(" \n") + allow(CoPlan::Ai).to receive(:call).and_return(" \n") expect { described_class.perform_now(plan_id: plan.id) @@ -77,7 +77,7 @@ described_class.perform_now(plan_id: plan.id) - expect(CoPlan::AiProviders::OpenAi).not_to have_received(:call) + expect(CoPlan::Ai).not_to have_received(:call) end it "no-ops when the plan has been deleted" do @@ -86,7 +86,7 @@ expect { described_class.perform_now(plan_id: missing_id) }.not_to raise_error - expect(CoPlan::AiProviders::OpenAi).not_to have_received(:call) + expect(CoPlan::Ai).not_to have_received(:call) end # Race-condition guard: a slow job started against revision N must @@ -97,7 +97,7 @@ # Simulate "a newer version landed while the AI was thinking" by # mutating the plan's current_plan_version between the AI call and # the persist step. - allow(CoPlan::AiProviders::OpenAi).to receive(:call) do + allow(CoPlan::Ai).to receive(:call) do newer = create(:plan_version, plan: plan, revision: plan.current_revision + 1, content_markdown: "# Fresher\n\nNewer body.") plan.update!(current_plan_version: newer, current_revision: newer.revision, @@ -113,9 +113,8 @@ expect(plan.summary_content_sha256).not_to eq(stale_sha) end - it "discards on AI provider errors instead of retrying" do - allow(CoPlan::AiProviders::OpenAi).to receive(:call) - .and_raise(CoPlan::AiProviders::OpenAi::Error, "boom") + it "discards on AI errors instead of retrying" do + allow(CoPlan::Ai).to receive(:call).and_raise(CoPlan::Ai::Error, "boom") expect { perform_enqueued_jobs { described_class.perform_later(plan_id: plan.id) } diff --git a/spec/services/ai_spec.rb b/spec/services/ai_spec.rb new file mode 100644 index 0000000..2d6b257 --- /dev/null +++ b/spec/services/ai_spec.rb @@ -0,0 +1,26 @@ +require "rails_helper" + +RSpec.describe CoPlan::Ai do + describe ".call" do + it "delegates to AiProviders::OpenAi and returns its response" do + allow(CoPlan::AiProviders::OpenAi).to receive(:call).and_return("ai output") + + result = described_class.call(system_prompt: "sys", user_content: "body") + + expect(result).to eq("ai output") + expect(CoPlan::AiProviders::OpenAi).to have_received(:call).with( + system_prompt: "sys", + user_content: "body" + ) + end + + it "wraps provider errors in CoPlan::Ai::Error so callers don't know the provider" do + allow(CoPlan::AiProviders::OpenAi).to receive(:call) + .and_raise(CoPlan::AiProviders::OpenAi::Error, "rate limited") + + expect { + described_class.call(system_prompt: "sys", user_content: "body") + }.to raise_error(CoPlan::Ai::Error, "rate limited") + end + end +end