diff --git a/enterprise/app/models/captain/document.rb b/enterprise/app/models/captain/document.rb index c278eb879..de048fac2 100644 --- a/enterprise/app/models/captain/document.rb +++ b/enterprise/app/models/captain/document.rb @@ -47,7 +47,7 @@ class Captain::Document < ApplicationRecord after_create_commit :enqueue_crawl_job after_create_commit :update_document_usage after_destroy :update_document_usage - after_commit :enqueue_response_builder_job, on: :update, if: :should_enqueue_response_builder? + after_commit :enqueue_response_builder_job scope :ordered, -> { order(created_at: :desc) } scope :for_account, ->(account_id) { where(account_id: account_id) } @@ -94,15 +94,21 @@ class Captain::Document < ApplicationRecord end def enqueue_response_builder_job - return if status != 'available' + return if destroyed? + return unless status == 'available' - Captain::Documents::ResponseBuilderJob.perform_later(self) - end + status_became_available = saved_change_to_status? + content_now_present = content.present? + content_became_present = saved_change_to_content? && content_now_present - def should_enqueue_response_builder? - # Only enqueue when status changes to available - # Avoid re-enqueueing when metadata is updated by the job itself - saved_change_to_status? && status == 'available' + should_enqueue = + if pdf_document? + status_became_available + else + (status_became_available && content_now_present) || content_became_present + end + + Captain::Documents::ResponseBuilderJob.perform_later(self) if should_enqueue end def update_document_usage diff --git a/spec/enterprise/models/captain/document_spec.rb b/spec/enterprise/models/captain/document_spec.rb index 56dc1727c..4eb0964ee 100644 --- a/spec/enterprise/models/captain/document_spec.rb +++ b/spec/enterprise/models/captain/document_spec.rb @@ -82,4 +82,161 @@ RSpec.describe Captain::Document, type: :model do end end end + + describe 'response builder job callback' do + before { clear_enqueued_jobs } + + describe 'non-PDF documents' do + it 'enqueues when created with available status and content' do + expect do + create(:captain_document, assistant: assistant, account: account, status: :available) + end.to have_enqueued_job(Captain::Documents::ResponseBuilderJob) + end + + it 'does not enqueue when created available without content' do + expect do + create(:captain_document, assistant: assistant, account: account, status: :available, content: nil) + end.not_to have_enqueued_job(Captain::Documents::ResponseBuilderJob) + end + + it 'enqueues when status transitions to available with existing content' do + document = create(:captain_document, assistant: assistant, account: account, status: :in_progress) + + expect do + document.update!(status: :available) + end.to have_enqueued_job(Captain::Documents::ResponseBuilderJob) + end + + it 'does not enqueue when status transitions to available without content' do + document = create( + :captain_document, + assistant: assistant, + account: account, + status: :in_progress, + content: nil + ) + + expect do + document.update!(status: :available) + end.not_to have_enqueued_job(Captain::Documents::ResponseBuilderJob) + end + + it 'enqueues when content is populated on an available document' do + document = create( + :captain_document, + assistant: assistant, + account: account, + status: :available, + content: nil + ) + clear_enqueued_jobs + + expect do + document.update!(content: 'Fresh content from crawl') + end.to have_enqueued_job(Captain::Documents::ResponseBuilderJob) + end + + it 'enqueues when content changes on an available document' do + document = create( + :captain_document, + assistant: assistant, + account: account, + status: :available, + content: 'Initial content' + ) + clear_enqueued_jobs + + expect do + document.update!(content: 'Updated crawl content') + end.to have_enqueued_job(Captain::Documents::ResponseBuilderJob) + end + + it 'does not enqueue when content is cleared on an available document' do + document = create( + :captain_document, + assistant: assistant, + account: account, + status: :available, + content: 'Initial content' + ) + clear_enqueued_jobs + + expect do + document.update!(content: nil) + end.not_to have_enqueued_job(Captain::Documents::ResponseBuilderJob) + end + + it 'does not enqueue for metadata-only updates' do + document = create(:captain_document, assistant: assistant, account: account, status: :available) + clear_enqueued_jobs + + expect do + document.update!(metadata: { 'title' => 'Updated Again' }) + end.not_to have_enqueued_job(Captain::Documents::ResponseBuilderJob) + end + + it 'does not enqueue while document remains in progress' do + document = create(:captain_document, assistant: assistant, account: account, status: :in_progress) + + expect do + document.update!(metadata: { 'title' => 'Updated' }) + end.not_to have_enqueued_job(Captain::Documents::ResponseBuilderJob) + end + end + + describe 'PDF documents' do + def build_pdf_document(status:, content:) + build( + :captain_document, + assistant: assistant, + account: account, + status: status, + content: content + ).tap do |doc| + doc.pdf_file.attach( + io: StringIO.new('PDF content'), + filename: 'sample.pdf', + content_type: 'application/pdf' + ) + end + end + + it 'enqueues when created available without content' do + document = build_pdf_document(status: :available, content: nil) + + expect do + document.save! + end.to have_enqueued_job(Captain::Documents::ResponseBuilderJob) + end + + it 'enqueues when status transitions to available' do + document = build_pdf_document(status: :in_progress, content: nil) + document.save! + clear_enqueued_jobs + + expect do + document.update!(status: :available) + end.to have_enqueued_job(Captain::Documents::ResponseBuilderJob) + end + + it 'does not enqueue when content updates without status change' do + document = build_pdf_document(status: :available, content: nil) + document.save! + clear_enqueued_jobs + + expect do + document.update!(content: 'Extracted PDF text') + end.not_to have_enqueued_job(Captain::Documents::ResponseBuilderJob) + end + end + + it 'does not enqueue when the document is destroyed' do + document = create(:captain_document, assistant: assistant, account: account, status: :available) + clear_enqueued_jobs + + expect do + document.destroy! + end.not_to have_enqueued_job(Captain::Documents::ResponseBuilderJob) + end + end end