fix: Update specs, add background response job implementation for copilot threads (#11600)
- Enable jobs by default when a copilot thread or a message is created. - Rename thread_id to copilot_thread_id to keep it consistent with the model name - Add a spec for search_linear_issues service
This commit is contained in:
@@ -12,9 +12,10 @@ class Api::V1::Accounts::Captain::CopilotMessagesController < Api::V1::Accounts:
|
||||
|
||||
def create
|
||||
@copilot_message = @copilot_thread.copilot_messages.create!(
|
||||
message: params[:message],
|
||||
message: { content: params[:message] },
|
||||
message_type: :user
|
||||
)
|
||||
@copilot_message.enqueue_response_job(params[:conversation_id], Current.user.id)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
@@ -18,7 +18,12 @@ class Api::V1::Accounts::Captain::CopilotThreadsController < Api::V1::Accounts::
|
||||
assistant: assistant
|
||||
)
|
||||
|
||||
@copilot_thread.copilot_messages.create!(message_type: :user, message: copilot_thread_params[:message])
|
||||
copilot_message = @copilot_thread.copilot_messages.create!(
|
||||
message_type: :user,
|
||||
message: { content: copilot_thread_params[:message] }
|
||||
)
|
||||
|
||||
copilot_message.enqueue_response_job(copilot_thread_params[:conversation_id], Current.user.id)
|
||||
end
|
||||
end
|
||||
|
||||
@@ -33,7 +38,7 @@ class Api::V1::Accounts::Captain::CopilotThreadsController < Api::V1::Accounts::
|
||||
end
|
||||
|
||||
def copilot_thread_params
|
||||
params.permit(:message, :assistant_id)
|
||||
params.permit(:message, :assistant_id, :conversation_id)
|
||||
end
|
||||
|
||||
def permitted_params
|
||||
|
||||
@@ -13,6 +13,9 @@ module Captain::ChatHelper
|
||||
)
|
||||
|
||||
handle_response(response)
|
||||
rescue StandardError => e
|
||||
Rails.logger.error "#{self.class.name} Assistant: #{@assistant.id}, Error in chat completion: #{e}"
|
||||
raise e
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
25
enterprise/app/jobs/captain/copilot/response_job.rb
Normal file
25
enterprise/app/jobs/captain/copilot/response_job.rb
Normal file
@@ -0,0 +1,25 @@
|
||||
class Captain::Copilot::ResponseJob < ApplicationJob
|
||||
queue_as :default
|
||||
|
||||
def perform(assistant:, conversation_id:, user_id:, copilot_thread_id:, message:)
|
||||
Rails.logger.info("#{self.class.name} Copilot response job for assistant_id=#{assistant.id} user_id=#{user_id}")
|
||||
generate_chat_response(
|
||||
assistant: assistant,
|
||||
conversation_id: conversation_id,
|
||||
user_id: user_id,
|
||||
copilot_thread_id: copilot_thread_id,
|
||||
message: message
|
||||
)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def generate_chat_response(assistant:, conversation_id:, user_id:, copilot_thread_id:, message:)
|
||||
Captain::Copilot::ChatService.new(
|
||||
assistant,
|
||||
user_id: user_id,
|
||||
copilot_thread_id: copilot_thread_id,
|
||||
conversation_id: conversation_id
|
||||
).generate_response(message)
|
||||
end
|
||||
end
|
||||
@@ -19,14 +19,12 @@ class CopilotMessage < ApplicationRecord
|
||||
belongs_to :copilot_thread
|
||||
belongs_to :account
|
||||
|
||||
before_validation :ensure_account
|
||||
|
||||
enum message_type: { user: 0, assistant: 1, assistant_thinking: 2 }
|
||||
|
||||
validates :message_type, presence: true, inclusion: { in: message_types.keys }
|
||||
validates :message_type, presence: true
|
||||
validates :message, presence: true
|
||||
before_validation :ensure_account
|
||||
validate :validate_message_attributes
|
||||
|
||||
after_create_commit :broadcast_message
|
||||
|
||||
def push_event_data
|
||||
@@ -39,10 +37,20 @@ class CopilotMessage < ApplicationRecord
|
||||
}
|
||||
end
|
||||
|
||||
def enqueue_response_job(conversation_id, user_id)
|
||||
Captain::Copilot::ResponseJob.perform_later(
|
||||
assistant: copilot_thread.assistant,
|
||||
conversation_id: conversation_id,
|
||||
user_id: user_id,
|
||||
copilot_thread_id: copilot_thread.id,
|
||||
message: message['content']
|
||||
)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def ensure_account
|
||||
self.account = copilot_thread.account
|
||||
self.account_id = copilot_thread&.account_id
|
||||
end
|
||||
|
||||
def broadcast_message
|
||||
|
||||
@@ -51,7 +51,7 @@ class Captain::Copilot::ChatService < Llm::BaseOpenAiService
|
||||
"#{self.class.name} Assistant: #{@assistant.id}, Previous History: #{config[:previous_history]&.length || 0}, Language: #{config[:language]}"
|
||||
)
|
||||
|
||||
@copilot_thread = @account.copilot_threads.find_by(id: config[:thread_id]) if config[:thread_id].present?
|
||||
@copilot_thread = @account.copilot_threads.find_by(id: config[:copilot_thread_id]) if config[:copilot_thread_id].present?
|
||||
@previous_history = if @copilot_thread.present?
|
||||
@copilot_thread.previous_history
|
||||
else
|
||||
|
||||
@@ -46,7 +46,7 @@ class Captain::Tools::Copilot::SearchLinearIssuesService < Captain::Tools::BaseS
|
||||
end
|
||||
|
||||
def active?
|
||||
@user.present? && @assistant.account.hooks.find_by(app_id: 'linear').present?
|
||||
@user.present? && @assistant.account.hooks.exists?(app_id: 'linear')
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
@@ -44,7 +44,7 @@ RSpec.describe 'Api::V1::Accounts::Captain::CopilotMessagesController', type: :r
|
||||
end.to change(CopilotMessage, :count).by(1)
|
||||
|
||||
expect(response).to have_http_status(:success)
|
||||
expect(CopilotMessage.last.message).to eq(message_content)
|
||||
expect(CopilotMessage.last.message).to eq({ 'content' => message_content })
|
||||
expect(CopilotMessage.last.message_type).to eq('user')
|
||||
expect(CopilotMessage.last.copilot_thread_id).to eq(copilot_thread.id)
|
||||
end
|
||||
|
||||
@@ -4,6 +4,7 @@ RSpec.describe 'Api::V1::Accounts::Captain::CopilotThreads', type: :request do
|
||||
let(:account) { create(:account) }
|
||||
let(:admin) { create(:user, account: account, role: :administrator) }
|
||||
let(:agent) { create(:user, account: account, role: :agent) }
|
||||
let(:conversation) { create(:conversation, account: account) }
|
||||
|
||||
def json_response
|
||||
JSON.parse(response.body, symbolize_names: true)
|
||||
@@ -50,7 +51,7 @@ RSpec.describe 'Api::V1::Accounts::Captain::CopilotThreads', type: :request do
|
||||
|
||||
describe 'POST /api/v1/accounts/{account.id}/captain/copilot_threads' do
|
||||
let(:assistant) { create(:captain_assistant, account: account) }
|
||||
let(:valid_params) { { message: 'Hello, how can you help me?', assistant_id: assistant.id } }
|
||||
let(:valid_params) { { message: 'Hello, how can you help me?', assistant_id: assistant.id, conversation_id: conversation.display_id } }
|
||||
|
||||
context 'when it is an un-authenticated user' do
|
||||
it 'returns unauthorized' do
|
||||
@@ -103,7 +104,7 @@ RSpec.describe 'Api::V1::Accounts::Captain::CopilotThreads', type: :request do
|
||||
|
||||
message = thread.copilot_messages.last
|
||||
expect(message.message_type).to eq('user')
|
||||
expect(message.message).to eq(valid_params[:message])
|
||||
expect(message.message).to eq({ 'content' => valid_params[:message] })
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
41
spec/enterprise/jobs/captain/copilot/response_job_spec.rb
Normal file
41
spec/enterprise/jobs/captain/copilot/response_job_spec.rb
Normal file
@@ -0,0 +1,41 @@
|
||||
require 'rails_helper'
|
||||
|
||||
RSpec.describe Captain::Copilot::ResponseJob, type: :job do
|
||||
let(:account) { create(:account) }
|
||||
let(:user) { create(:user, account: account) }
|
||||
let(:assistant) { create(:captain_assistant, account: account) }
|
||||
let(:copilot_thread) { create(:captain_copilot_thread, account: account, user: user, assistant: assistant) }
|
||||
let(:conversation_id) { 123 }
|
||||
let(:message) { { 'content' => 'Test message' } }
|
||||
|
||||
describe '#perform' do
|
||||
let(:chat_service) { instance_double(Captain::Copilot::ChatService) }
|
||||
|
||||
before do
|
||||
allow(Captain::Copilot::ChatService).to receive(:new).with(
|
||||
assistant,
|
||||
user_id: user.id,
|
||||
copilot_thread_id: copilot_thread.id,
|
||||
conversation_id: conversation_id
|
||||
).and_return(chat_service)
|
||||
allow(chat_service).to receive(:generate_response).with(message)
|
||||
end
|
||||
|
||||
it 'initializes ChatService with correct parameters and calls generate_response' do
|
||||
expect(Captain::Copilot::ChatService).to receive(:new).with(
|
||||
assistant,
|
||||
user_id: user.id,
|
||||
copilot_thread_id: copilot_thread.id,
|
||||
conversation_id: conversation_id
|
||||
)
|
||||
expect(chat_service).to receive(:generate_response).with(message)
|
||||
described_class.perform_now(
|
||||
assistant: assistant,
|
||||
conversation_id: conversation_id,
|
||||
user_id: user.id,
|
||||
copilot_thread_id: copilot_thread.id,
|
||||
message: message
|
||||
)
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -1,15 +1,14 @@
|
||||
require 'rails_helper'
|
||||
|
||||
RSpec.describe CopilotMessage, type: :model do
|
||||
describe 'associations' do
|
||||
it { is_expected.to belong_to(:copilot_thread) }
|
||||
it { is_expected.to belong_to(:account) }
|
||||
end
|
||||
let(:account) { create(:account) }
|
||||
let(:user) { create(:user, account: account) }
|
||||
let(:assistant) { create(:captain_assistant, account: account) }
|
||||
let(:copilot_thread) { create(:captain_copilot_thread, account: account, user: user, assistant: assistant) }
|
||||
|
||||
describe 'validations' do
|
||||
it { is_expected.to validate_presence_of(:message_type) }
|
||||
it { is_expected.to validate_presence_of(:message) }
|
||||
it { is_expected.to validate_inclusion_of(:message_type).in_array(described_class.message_types.keys) }
|
||||
end
|
||||
|
||||
describe 'callbacks' do
|
||||
@@ -31,7 +30,7 @@ RSpec.describe CopilotMessage, type: :model do
|
||||
message = build(:captain_copilot_message, copilot_thread: copilot_thread)
|
||||
|
||||
expect(Rails.configuration.dispatcher).to receive(:dispatch)
|
||||
.with(COPILOT_MESSAGE_CREATED, anything, copilot_message: message)
|
||||
.with('copilot.message.created', anything, copilot_message: message)
|
||||
|
||||
message.save!
|
||||
end
|
||||
|
||||
@@ -17,7 +17,7 @@ RSpec.describe Captain::Copilot::ChatService do
|
||||
let(:previous_history) { [{ role: copilot_message.message_type, content: copilot_message.message['content'] }] }
|
||||
|
||||
let(:config) do
|
||||
{ user_id: user.id, thread_id: copilot_thread.id, conversation_id: conversation.display_id }
|
||||
{ user_id: user.id, copilot_thread_id: copilot_thread.id, conversation_id: conversation.display_id }
|
||||
end
|
||||
|
||||
before do
|
||||
@@ -157,16 +157,16 @@ RSpec.describe Captain::Copilot::ChatService do
|
||||
end
|
||||
|
||||
describe '#setup_message_history' do
|
||||
context 'when thread_id is present' do
|
||||
context 'when copilot_thread_id is present' do
|
||||
it 'finds the copilot thread and sets previous history from it' do
|
||||
service = described_class.new(assistant, { thread_id: copilot_thread.id })
|
||||
service = described_class.new(assistant, { copilot_thread_id: copilot_thread.id })
|
||||
|
||||
expect(service.copilot_thread).to eq(copilot_thread)
|
||||
expect(service.previous_history).to eq previous_history
|
||||
end
|
||||
end
|
||||
|
||||
context 'when thread_id is not present' do
|
||||
context 'when copilot_thread_id is not present' do
|
||||
it 'uses previous_history from config if present' do
|
||||
custom_history = [{ role: 'user', content: 'Custom message' }]
|
||||
service = described_class.new(assistant, { previous_history: custom_history })
|
||||
@@ -222,7 +222,7 @@ RSpec.describe Captain::Copilot::ChatService do
|
||||
}.with_indifferent_access)
|
||||
|
||||
expect do
|
||||
described_class.new(assistant, { thread_id: copilot_thread.id }).generate_response('Hello')
|
||||
described_class.new(assistant, { copilot_thread_id: copilot_thread.id }).generate_response('Hello')
|
||||
end.to change(CopilotMessage, :count).by(1)
|
||||
|
||||
last_message = CopilotMessage.last
|
||||
|
||||
@@ -102,10 +102,6 @@ RSpec.describe Captain::Tools::Copilot::SearchArticlesService do
|
||||
end
|
||||
|
||||
context 'when no articles are found' do
|
||||
before do
|
||||
allow(Article).to receive(:where).and_return(Article.none)
|
||||
end
|
||||
|
||||
it 'returns no articles found message' do
|
||||
expect(service.execute({ 'query' => 'test' })).to eq('No articles found')
|
||||
end
|
||||
@@ -113,13 +109,8 @@ RSpec.describe Captain::Tools::Copilot::SearchArticlesService do
|
||||
|
||||
context 'when articles are found' do
|
||||
let(:portal) { create(:portal, account: account) }
|
||||
let(:article1) { create(:article, account: account, portal: portal, author: user, title: 'Test Article 1', content: 'Content 1') }
|
||||
let(:article2) { create(:article, account: account, portal: portal, author: user, title: 'Test Article 2', content: 'Content 2') }
|
||||
|
||||
before do
|
||||
article1
|
||||
article2
|
||||
end
|
||||
let!(:article1) { create(:article, account: account, portal: portal, author: user, title: 'Test Article 1', content: 'Content 1') }
|
||||
let!(:article2) { create(:article, account: account, portal: portal, author: user, title: 'Test Article 2', content: 'Content 2') }
|
||||
|
||||
it 'returns formatted articles with count' do
|
||||
result = service.execute({ 'query' => 'Test' })
|
||||
@@ -130,11 +121,7 @@ RSpec.describe Captain::Tools::Copilot::SearchArticlesService do
|
||||
|
||||
context 'when filtered by category' do
|
||||
let(:category) { create(:category, slug: 'test-category', portal: portal, account: account) }
|
||||
let(:article3) { create(:article, account: account, portal: portal, author: user, category: category, title: 'Test Article 3') }
|
||||
|
||||
before do
|
||||
article3
|
||||
end
|
||||
let!(:article3) { create(:article, account: account, portal: portal, author: user, category: category, title: 'Test Article 3') }
|
||||
|
||||
it 'returns only articles from the specified category' do
|
||||
result = service.execute({ 'query' => 'Test', 'category_id' => category.id })
|
||||
@@ -146,15 +133,8 @@ RSpec.describe Captain::Tools::Copilot::SearchArticlesService do
|
||||
end
|
||||
|
||||
context 'when filtered by status' do
|
||||
let(:article3) do
|
||||
create(:article, account: account, portal: portal, author: user, title: 'Test Article 3', status: 'published')
|
||||
end
|
||||
let(:article4) { create(:article, account: account, portal: portal, author: user, title: 'Test Article 4', status: 'draft') }
|
||||
|
||||
before do
|
||||
article3
|
||||
article4
|
||||
end
|
||||
let!(:article3) { create(:article, account: account, portal: portal, author: user, title: 'Test Article 3', status: 'published') }
|
||||
let!(:article4) { create(:article, account: account, portal: portal, author: user, title: 'Test Article 4', status: 'draft') }
|
||||
|
||||
it 'returns only articles with the specified status' do
|
||||
result = service.execute({ 'query' => 'Test', 'status' => 'published' })
|
||||
|
||||
@@ -3,7 +3,8 @@ require 'rails_helper'
|
||||
RSpec.describe Captain::Tools::Copilot::SearchLinearIssuesService do
|
||||
let(:account) { create(:account) }
|
||||
let(:assistant) { create(:captain_assistant, account: account) }
|
||||
let(:service) { described_class.new(assistant) }
|
||||
let(:user) { create(:user, account: account) }
|
||||
let(:service) { described_class.new(assistant, user: user) }
|
||||
|
||||
describe '#name' do
|
||||
it 'returns the correct service name' do
|
||||
@@ -40,14 +41,34 @@ RSpec.describe Captain::Tools::Copilot::SearchLinearIssuesService do
|
||||
create(:integrations_hook, :linear, account: account)
|
||||
end
|
||||
|
||||
it 'returns true' do
|
||||
expect(service.active?).to be true
|
||||
context 'when user is present' do
|
||||
it 'returns true' do
|
||||
expect(service.active?).to be true
|
||||
end
|
||||
end
|
||||
|
||||
context 'when user is not present' do
|
||||
let(:service) { described_class.new(assistant) }
|
||||
|
||||
it 'returns false' do
|
||||
expect(service.active?).to be false
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when Linear integration is not enabled' do
|
||||
it 'returns false' do
|
||||
expect(service.active?).to be false
|
||||
context 'when user is present' do
|
||||
it 'returns false' do
|
||||
expect(service.active?).to be false
|
||||
end
|
||||
end
|
||||
|
||||
context 'when user is not present' do
|
||||
let(:service) { described_class.new(assistant) }
|
||||
|
||||
it 'returns false' do
|
||||
expect(service.active?).to be false
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -1,13 +1,14 @@
|
||||
FactoryBot.define do
|
||||
factory :article, class: 'Article' do
|
||||
account_id { 1 }
|
||||
category_id { 1 }
|
||||
account
|
||||
category { nil }
|
||||
portal
|
||||
locale { 'en' }
|
||||
author_id { 1 }
|
||||
association :author, factory: :user
|
||||
title { "#{Faker::Movie.title} #{SecureRandom.hex}" }
|
||||
content { 'MyText' }
|
||||
description { 'MyDescrption' }
|
||||
status { 1 }
|
||||
status { :published }
|
||||
views { 0 }
|
||||
end
|
||||
end
|
||||
|
||||
@@ -1,9 +1,10 @@
|
||||
FactoryBot.define do
|
||||
factory :category, class: 'Category' do
|
||||
portal { portal }
|
||||
portal
|
||||
name { 'MyString' }
|
||||
description { 'MyText' }
|
||||
position { 1 }
|
||||
slug { name.parameterize }
|
||||
|
||||
after(:build) do |category|
|
||||
category.account ||= category.portal.account
|
||||
|
||||
Reference in New Issue
Block a user