From 718e8c53a0df0b2bd3f5a994b54083dd15a28a95 Mon Sep 17 00:00:00 2001 From: Hampton Lintorn-Catlin Date: Mon, 1 Jun 2026 15:36:57 -0500 Subject: [PATCH 1/3] Add AI-generated summaries for plans (COPLAN-24) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- ...711_add_summary_to_coplan_plans.co_plan.rb | 15 ++ db/schema.rb | 5 +- engine/app/jobs/coplan/summarize_plan_job.rb | 64 ++++++++ engine/app/models/coplan/plan_version.rb | 9 ++ ...60601152600_add_summary_to_coplan_plans.rb | 14 ++ engine/prompts/summarize.md | 7 + spec/jobs/summarize_plan_job_spec.rb | 142 ++++++++++++++++++ 7 files changed, 255 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20260601202711_add_summary_to_coplan_plans.co_plan.rb create mode 100644 engine/app/jobs/coplan/summarize_plan_job.rb create mode 100644 engine/db/migrate/20260601152600_add_summary_to_coplan_plans.rb create mode 100644 engine/prompts/summarize.md create mode 100644 spec/jobs/summarize_plan_job_spec.rb diff --git a/db/migrate/20260601202711_add_summary_to_coplan_plans.co_plan.rb b/db/migrate/20260601202711_add_summary_to_coplan_plans.co_plan.rb new file mode 100644 index 00000000..67d28e1f --- /dev/null +++ b/db/migrate/20260601202711_add_summary_to_coplan_plans.co_plan.rb @@ -0,0 +1,15 @@ +# This migration comes from co_plan (originally 20260601152600) +class AddSummaryToCoplanPlans < ActiveRecord::Migration[8.1] + def change + change_table :coplan_plans do |t| + t.text :summary + t.datetime :summary_generated_at + # SHA256 of the PlanVersion content the summary was generated from. + # Used by SummarizePlanJob to debounce regeneration: if the plan's + # current content sha hasn't changed since the last summary, the job + # no-ops. This lets us fire the job from every PlanVersion#after_create_commit + # without re-calling the AI on rapid back-to-back edits. + t.string :summary_content_sha256, limit: 64 + end + end +end diff --git a/db/schema.rb b/db/schema.rb index bf998541..cfc948ad 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_05_19_011926) do +ActiveRecord::Schema[8.1].define(version: 2026_06_01_202711) do create_table "active_admin_comments", id: { type: :string, limit: 36 }, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.bigint "author_id" t.string "author_type" @@ -225,6 +225,9 @@ t.json "metadata" t.string "plan_type_id", limit: 36 t.string "status", default: "brainstorm", null: false + t.text "summary" + t.string "summary_content_sha256", limit: 64 + t.datetime "summary_generated_at" t.string "title", null: false t.datetime "updated_at", null: false t.index ["created_by_user_id"], name: "index_coplan_plans_on_created_by_user_id" diff --git a/engine/app/jobs/coplan/summarize_plan_job.rb b/engine/app/jobs/coplan/summarize_plan_job.rb new file mode 100644 index 00000000..4eed62e7 --- /dev/null +++ b/engine/app/jobs/coplan/summarize_plan_job.rb @@ -0,0 +1,64 @@ +module CoPlan + # Regenerates a plan's AI-generated summary. + # + # Enqueued from PlanVersion#after_create_commit. Debounced via + # `plan.summary_content_sha256`: if the plan's current content sha + # already matches the sha the existing summary was generated from, + # the job no-ops. This is safe under rapid back-to-back edits — each + # PlanVersion enqueues a job, but only one ends up calling the AI. + # + # AI provider errors are discarded rather than retried — a stale + # summary is fine, and the next PlanVersion will trigger another + # attempt. + class SummarizePlanJob < ApplicationJob + PROMPT_PATH = CoPlan::Engine.root.join("prompts", "summarize.md").freeze + + queue_as :default + + discard_on AiProviders::OpenAi::Error + + def perform(plan_id:) + plan = Plan.find_by(id: plan_id) + return unless plan + + current_sha = plan.current_plan_version&.content_sha256 + return if current_sha.blank? + return if plan.summary_content_sha256 == current_sha + + summary = generate_summary(plan) + return if summary.blank? + + persist_summary(plan, summary, current_sha) + end + + private + + def generate_summary(plan) + content = plan.current_content + return nil if content.blank? + + AiProviders::OpenAi.call( + system_prompt: File.read(PROMPT_PATH), + user_content: content + ).to_s.strip.presence + end + + # Persist the summary only if the plan's current content sha still + # matches the sha we generated from. Without this check, a slow job + # that started against revision N could overwrite a fresher summary + # generated against revision N+1 — AI calls take seconds, plenty of + # time for a newer version to land first. + def persist_summary(plan, summary, expected_sha) + plan.with_lock do + plan.reload + return if plan.current_plan_version&.content_sha256 != expected_sha + + plan.update!( + summary: summary, + summary_generated_at: Time.current, + summary_content_sha256: expected_sha + ) + end + end + end +end diff --git a/engine/app/models/coplan/plan_version.rb b/engine/app/models/coplan/plan_version.rb index d84c9116..11215d1f 100644 --- a/engine/app/models/coplan/plan_version.rb +++ b/engine/app/models/coplan/plan_version.rb @@ -23,6 +23,7 @@ def history_kind after_create_commit :extract_references after_create_commit :broadcast_history_update + after_create_commit :enqueue_summary_regeneration private @@ -64,5 +65,13 @@ def broadcast_references_update def compute_sha256 self.content_sha256 = Digest::SHA256.hexdigest(content_markdown) end + + # Fire SummarizePlanJob after every new version. The job is debounced + # against `plan.summary_content_sha256`, so rapid back-to-back versions + # collapse to a single AI call. The `wait` further reduces wasted calls + # during a burst of edits (e.g. a session commit followed by another). + def enqueue_summary_regeneration + SummarizePlanJob.set(wait: 10.seconds).perform_later(plan_id: plan_id) + end end end diff --git a/engine/db/migrate/20260601152600_add_summary_to_coplan_plans.rb b/engine/db/migrate/20260601152600_add_summary_to_coplan_plans.rb new file mode 100644 index 00000000..52477f5d --- /dev/null +++ b/engine/db/migrate/20260601152600_add_summary_to_coplan_plans.rb @@ -0,0 +1,14 @@ +class AddSummaryToCoplanPlans < ActiveRecord::Migration[8.1] + def change + change_table :coplan_plans do |t| + t.text :summary + t.datetime :summary_generated_at + # SHA256 of the PlanVersion content the summary was generated from. + # Used by SummarizePlanJob to debounce regeneration: if the plan's + # current content sha hasn't changed since the last summary, the job + # no-ops. This lets us fire the job from every PlanVersion#after_create_commit + # without re-calling the AI on rapid back-to-back edits. + t.string :summary_content_sha256, limit: 64 + end + end +end diff --git a/engine/prompts/summarize.md b/engine/prompts/summarize.md new file mode 100644 index 00000000..069ef3b8 --- /dev/null +++ b/engine/prompts/summarize.md @@ -0,0 +1,7 @@ +You are summarizing engineering planning documents. + +Write a 1-2 sentence summary of the plan below. Be concrete — what is the +plan proposing, and (if applicable) why. Skip filler like "This plan +discusses..." or "The author proposes...". Start with the substance. + +Plain prose only. No Markdown, no lists, no headings. Maximum ~280 characters. diff --git a/spec/jobs/summarize_plan_job_spec.rb b/spec/jobs/summarize_plan_job_spec.rb new file mode 100644 index 00000000..bd4c271e --- /dev/null +++ b/spec/jobs/summarize_plan_job_spec.rb @@ -0,0 +1,142 @@ +require "rails_helper" + +RSpec.describe CoPlan::SummarizePlanJob, type: :job do + include ActiveJob::TestHelper + + let(:plan) { create(:plan) } + let(:current_sha) { plan.current_plan_version.content_sha256 } + + before do + allow(CoPlan::AiProviders::OpenAi).to receive(:call).and_return("Fresh summary.") + end + + describe "#perform" do + it "passes the summarize prompt and plan content to the AI provider" do + described_class.perform_now(plan_id: plan.id) + + expect(CoPlan::AiProviders::OpenAi).to have_received(:call).with( + system_prompt: File.read(CoPlan::SummarizePlanJob::PROMPT_PATH), + user_content: plan.current_content + ) + end + + it "updates summary, generated_at, and sha when content has changed" do + freeze_time do + expect { + described_class.perform_now(plan_id: plan.id) + }.to change { plan.reload.summary }.from(nil).to("Fresh summary.") + .and change { plan.reload.summary_content_sha256 }.from(nil).to(current_sha) + + expect(plan.reload.summary_generated_at).to eq(Time.current) + end + end + + it "strips whitespace from the AI response before persisting" do + allow(CoPlan::AiProviders::OpenAi).to receive(:call).and_return(" trimmed\n\n") + + described_class.perform_now(plan_id: plan.id) + + expect(plan.reload.summary).to eq("trimmed") + end + + it "no-ops when summary_content_sha256 already matches current content" do + plan.update!( + summary: "Existing.", + summary_generated_at: 1.hour.ago, + summary_content_sha256: current_sha + ) + + described_class.perform_now(plan_id: plan.id) + + expect(CoPlan::AiProviders::OpenAi).not_to have_received(:call) + expect(plan.reload.summary).to eq("Existing.") + end + + it "regenerates when a new PlanVersion changes the content sha" do + plan.update!(summary: "Old.", summary_content_sha256: current_sha, summary_generated_at: 1.hour.ago) + new_version = create(:plan_version, plan: plan, revision: plan.current_revision + 1, + content_markdown: "# New content\n\nDifferent text.") + plan.update!(current_plan_version: new_version, current_revision: new_version.revision) + + described_class.perform_now(plan_id: plan.id) + + expect(plan.reload.summary).to eq("Fresh summary.") + expect(plan.summary_content_sha256).to eq(new_version.content_sha256) + end + + it "does not update when the AI returns blank" do + allow(CoPlan::AiProviders::OpenAi).to receive(:call).and_return(" \n") + + expect { + described_class.perform_now(plan_id: plan.id) + }.not_to change { plan.reload.summary } + end + + it "no-ops when the plan has no current version" do + plan.update_columns(current_plan_version_id: nil, current_revision: 0) + + described_class.perform_now(plan_id: plan.id) + + expect(CoPlan::AiProviders::OpenAi).not_to have_received(:call) + end + + it "no-ops when the plan has been deleted" do + missing_id = SecureRandom.uuid + + expect { + described_class.perform_now(plan_id: missing_id) + }.not_to raise_error + expect(CoPlan::AiProviders::OpenAi).not_to have_received(:call) + end + + # Race-condition guard: a slow job started against revision N must + # NOT overwrite a fresher summary already persisted for revision N+1. + it "does not overwrite a fresher summary when a newer version landed mid-flight" do + stale_sha = current_sha + + # 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 + 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, + summary: "Fresher summary persisted by the newer job.", + summary_content_sha256: newer.content_sha256, + summary_generated_at: Time.current) + "Stale summary from the slow job." + end + + described_class.perform_now(plan_id: plan.id) + + expect(plan.reload.summary).to eq("Fresher summary persisted by the newer job.") + 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") + + expect { + perform_enqueued_jobs { described_class.perform_later(plan_id: plan.id) } + }.not_to raise_error + end + + it "enqueues on the default queue" do + existing_plan_id = plan.id + expect { + described_class.perform_later(plan_id: existing_plan_id) + }.to have_enqueued_job(described_class).on_queue("default").with(plan_id: existing_plan_id) + end + end + + describe "PlanVersion after_create_commit hook" do + it "enqueues SummarizePlanJob when a new version is created" do + existing_plan = create(:plan) + + expect { + create(:plan_version, plan: existing_plan, revision: existing_plan.current_revision + 1) + }.to have_enqueued_job(described_class).with(plan_id: existing_plan.id) + end + end +end From a37242921bc00e9075e6b10eadf94fc6c56a5a9f Mon Sep 17 00:00:00 2001 From: Hampton Lintorn-Catlin Date: Mon, 1 Jun 2026 15:51:48 -0500 Subject: [PATCH 2/3] Hide AI provider behind CoPlan::Ai facade (COPLAN-24) (#120) 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 4eed62e7..a2955dcd 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 00000000..4ce9d6a8 --- /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 bd4c271e..bdfb5381 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 00000000..2d6b257d --- /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 From 2d4884900dd6ca455e22fa917fd2ffbc0dce4983 Mon Sep 17 00:00:00 2001 From: Hampton Lintorn-Catlin Date: Mon, 1 Jun 2026 15:54:15 -0500 Subject: [PATCH 3/3] Atomically claim sha before AI call to prevent duplicate requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- engine/app/jobs/coplan/summarize_plan_job.rb | 52 +++++++++++--------- spec/jobs/summarize_plan_job_spec.rb | 19 +++++++ 2 files changed, 48 insertions(+), 23 deletions(-) diff --git a/engine/app/jobs/coplan/summarize_plan_job.rb b/engine/app/jobs/coplan/summarize_plan_job.rb index a2955dcd..f2d1c68f 100644 --- a/engine/app/jobs/coplan/summarize_plan_job.rb +++ b/engine/app/jobs/coplan/summarize_plan_job.rb @@ -2,14 +2,16 @@ module CoPlan # Regenerates a plan's AI-generated summary. # # Enqueued from PlanVersion#after_create_commit. Debounced via - # `plan.summary_content_sha256`: if the plan's current content sha - # already matches the sha the existing summary was generated from, - # the job no-ops. This is safe under rapid back-to-back edits — each - # PlanVersion enqueues a job, but only one ends up calling the AI. + # `plan.summary_content_sha256`: each job atomically "claims" the + # current content sha before calling the AI. Two workers that wake + # up against the same plan-and-sha will race on the claim — only one + # wins, the other no-ops. Without the atomic claim, both would pass + # a naive pre-check, both call the AI, and waste a full AI request. # - # AI provider errors are discarded rather than retried — a stale - # summary is fine, and the next PlanVersion will trigger another - # attempt. + # AI errors are discarded rather than retried — a stale summary is + # fine, and the next PlanVersion will trigger another attempt. The + # claim survives the failure, which is intentional: we'd rather skip + # this revision than retry a broken prompt in a loop. class SummarizePlanJob < ApplicationJob PROMPT_PATH = CoPlan::Engine.root.join("prompts", "summarize.md").freeze @@ -23,7 +25,8 @@ def perform(plan_id:) current_sha = plan.current_plan_version&.content_sha256 return if current_sha.blank? - return if plan.summary_content_sha256 == current_sha + + return unless claim_sha(plan, current_sha) summary = generate_summary(plan) return if summary.blank? @@ -33,6 +36,19 @@ def perform(plan_id:) private + # Atomic claim: set summary_content_sha256 = current_sha only if it + # isn't already current_sha. Returns true if THIS job won the claim. + # + # Using a single conditional UPDATE (one round-trip, atomic at the + # DB) means concurrent workers can't both pass the check and both + # call the AI — exactly one row update succeeds per sha. + def claim_sha(plan, current_sha) + claimed = Plan.where(id: plan.id) + .where("summary_content_sha256 IS NULL OR summary_content_sha256 != ?", current_sha) + .update_all(summary_content_sha256: current_sha) + claimed.positive? + end + def generate_summary(plan) content = plan.current_content return nil if content.blank? @@ -43,22 +59,12 @@ def generate_summary(plan) ).to_s.strip.presence end - # Persist the summary only if the plan's current content sha still - # matches the sha we generated from. Without this check, a slow job - # that started against revision N could overwrite a fresher summary - # generated against revision N+1 — AI calls take seconds, plenty of - # time for a newer version to land first. + # Write the summary only if our claim is still current — if a newer + # version landed mid-flight, a fresher job has already re-claimed + # the sha and we'd be stomping its work. def persist_summary(plan, summary, expected_sha) - plan.with_lock do - plan.reload - return if plan.current_plan_version&.content_sha256 != expected_sha - - plan.update!( - summary: summary, - summary_generated_at: Time.current, - summary_content_sha256: expected_sha - ) - end + Plan.where(id: plan.id, summary_content_sha256: expected_sha) + .update_all(summary: summary, summary_generated_at: Time.current) end end end diff --git a/spec/jobs/summarize_plan_job_spec.rb b/spec/jobs/summarize_plan_job_spec.rb index bdfb5381..ae296e5d 100644 --- a/spec/jobs/summarize_plan_job_spec.rb +++ b/spec/jobs/summarize_plan_job_spec.rb @@ -89,6 +89,25 @@ expect(CoPlan::Ai).not_to have_received(:call) end + # Concurrency guard: two workers waking up against the same plan-and-sha + # must collapse to a single AI call. Without the atomic claim, both jobs + # would pass a naive pre-check and both burn an AI request. + it "claims the sha before calling AI so a concurrent worker skips" do + call_count = 0 + allow(CoPlan::Ai).to receive(:call) do + call_count += 1 + # Simulate a second worker firing while we're mid-AI-call. With the + # claim already taken, this inner perform should no-op. + described_class.perform_now(plan_id: plan.id) + "Fresh summary." + end + + described_class.perform_now(plan_id: plan.id) + + expect(call_count).to eq(1) + expect(plan.reload.summary).to eq("Fresh summary.") + end + # Race-condition guard: a slow job started against revision N must # NOT overwrite a fresher summary already persisted for revision N+1. it "does not overwrite a fresher summary when a newer version landed mid-flight" do