Hide AI provider behind CoPlan::Ai facade (COPLAN-24)#120
Merged
HamptonMakes merged 1 commit intoJun 1, 2026
Merged
Conversation
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 <amp@ampcode.com>
HamptonMakes
added a commit
that referenced
this pull request
Jun 1, 2026
* Add AI-generated summaries for plans (COPLAN-24) Every plan now gets a 1-2 sentence AI summary that regenerates whenever the plan content changes. Schema: - coplan_plans gains summary (text), summary_generated_at (datetime), and summary_content_sha256 (string) columns. The sha column is the debounce key: jobs no-op when the plan's current content sha already matches it. Job: - SummarizePlanJob is enqueued from PlanVersion#after_create_commit with a 10s wait. Multiple jobs collapsing onto the same plan all sha-check and only one calls the AI provider. - The persist step reloads under a row lock and re-verifies the sha so a slow job started against revision N cannot overwrite a fresher summary produced against revision N+1. - Plan.find_by + early return so a deleted-mid-flight plan no-ops instead of crashing. Prompt: - engine/prompts/summarize.md — baked into the engine; uses the existing AiProviders::OpenAi hook. No new configuration knob. Frontend rendering is out of scope here — handled in the cards workstream. Amp-Thread-ID: https://ampcode.com/threads/T-019e84ca-1d09-777c-baf1-b1f380ec013c Co-authored-by: Amp <amp@ampcode.com> * Hide AI provider behind CoPlan::Ai facade (COPLAN-24) (#120) 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 <amp@ampcode.com> * Atomically claim sha before AI call to prevent duplicate requests Addresses Codex review on PR #118: two SolidQueue workers waking up against the same plan-and-sha could both pass the pre-AI sha check and both call OpenAI before either persisted, burning a duplicate AI request per burst of edits. Replace the with_lock pre-check with a single conditional UPDATE: UPDATE coplan_plans SET summary_content_sha256 = current_sha WHERE id = plan.id AND (summary_content_sha256 IS NULL OR summary_content_sha256 != current_sha) The DB serializes the UPDATE, so exactly one worker wins the claim per sha — the rest get 0 rows updated and no-op. No row locks held during the AI call. persist_summary is rewritten as a conditional update too: it only writes summary/generated_at when our claimed sha is still current, so a slow job that started against revision N can't stomp a fresher summary produced against revision N+1. Trade-off: if the AI call fails (CoPlan::Ai::Error → discarded), the claim is not released. We won't retry until the next PlanVersion lands. That's intentional — better than retry storms on a broken prompt. Amp-Thread-ID: https://ampcode.com/threads/T-019e84ca-1d09-777c-baf1-b1f380ec013c 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.
Stacked on top of #118.
Addresses Hampton's review comment on
SummarizePlanJob: provider info shouldn't leak into a job that just wants "an AI". The job shouldn't have to know about OpenAi vs Anthropic.What
New
CoPlan::Aifacade that wrapsAiProviders::OpenAiand exposesCoPlan::Ai::Error. Callers candiscard_on CoPlan::Ai::Errorwithout knowing the underlying provider.What this is NOT
configuration.ai_providersetting is speculative — there's no second supported provider in production yet.AutomatedReviewJob. That job legitimately needs to dispatch between providers because eachAutomatedPlanReviewerrow carries its ownai_provider+ai_model. The facade is for the common case where a caller doesn't care.Tests
CoPlan::Aiinstead of the provider🤖 Drafted with Amp.
Co-authored-by: Amp amp@ampcode.com