Add AI-generated summaries for plans (COPLAN-24)#118
Conversation
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>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 718e8c53a0
ℹ️ 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".
| return if plan.summary_content_sha256 == current_sha | ||
|
|
||
| summary = generate_summary(plan) |
There was a problem hiding this comment.
Recheck the debounce before calling OpenAI
With the production SolidQueue config running multiple worker threads (config/queue.yml has threads: 3), two delayed summary jobs for the same plan can start together after a burst of edits and both pass this pre-AI SHA check before either writes summary_content_sha256. The later row lock only serializes the final update and does not skip once the same SHA has already been summarized, so rapid edits can still trigger duplicate OpenAI calls for the same current version instead of collapsing to one call. Recheck or reserve the SHA under the lock before calling the provider, or skip the locked write when the SHA already matches.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2d48849. Replaced the with_lock pre-check with a single atomic conditional UPDATE that claims the sha before the AI call — only one worker wins per sha, the rest no-op without touching OpenAI. persist_summary is now a conditional update too, so a slow job started against revision N can't stomp a fresher summary produced against N+1. Added a concurrency-guard spec that simulates a second worker firing mid-AI-call and asserts only one AI call is made.
| content = plan.current_content | ||
| return nil if content.blank? | ||
|
|
||
| AiProviders::OpenAi.call( |
There was a problem hiding this comment.
why do we have an openAI thing that's specific? this feels off....
There was a problem hiding this comment.
Addressed in stacked PR #120 — added a CoPlan::Ai facade that hides provider selection. SummarizePlanJob now just calls CoPlan::Ai.call and discards CoPlan::Ai::Error.
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>
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>
…summaries * origin/main: Sitewide search modal with MySQL FULLTEXT (COPLAN-21) (#119) Amp-Thread-ID: https://ampcode.com/threads/T-019e84ca-1d09-777c-baf1-b1f380ec013c Co-authored-by: Amp <amp@ampcode.com> # Conflicts: # db/schema.rb
Resolves COPLAN-24.
What
Every plan now gets a 1-2 sentence AI summary that regenerates whenever the plan content changes.
Backend-only
Card and show-page rendering are intentionally out of scope here — they're being rebuilt in a separate workstream. This PR ships the schema, hook, and job.
How it works
Schema
coplan_plansgains:summary(text)summary_generated_at(datetime)summary_content_sha256(string, 64) — debounce keyNotable design choices
persist_summaryreloads under a row lock and re-checks the sha before writing, so a slow job started against revision N cannot stomp a fresher summary produced against revision N+1.find_by+ early return if the plan was deleted between enqueue and run.engine/prompts/summarize.mdrather than aconfiguration.summarize_planhook — the existingAiProviders::OpenAihook is already pluggable for API key / base URL, and adding another knob for one prompt is over-engineering.Out of scope (for follow-up)
Tests
883 / 883 green on an isolated test DB.
🤖 Drafted with Amp.
Co-authored-by: Amp amp@ampcode.com