fix(captain): add v1 handoff classifier [AI-137] (#14337)
# Pull Request Template ## Description Captain (v1) makes false promises by saying it will handoff but doesn't. This happens due to an exact string match comparison and the prompt gives the model a lot of responsibilities: - identity - what to respond - obey custom instructions - decide on tool calls This PR decouples responsibility, the core prompt responds, and an additional llm call evaluates if handoff was needed or not after that message. ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration. Locally ## Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [x] I have commented on my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] Any dependent changes have been merged and published in downstream modules
This commit is contained in:
Aakash Bakhle
@@ -10,6 +10,7 @@ RSpec.describe Captain::Conversation::ResponseBuilderJob, type: :job do
|
||||
let(:conversation) { create(:conversation, inbox: inbox, account: account, status: :pending) }
|
||||
let(:mock_llm_chat_service) { instance_double(Captain::Llm::AssistantChatService) }
|
||||
let(:mock_agent_runner_service) { instance_double(Captain::Assistant::AgentRunnerService) }
|
||||
let(:mock_action_classifier_service) { instance_double(Captain::Llm::AssistantActionClassifierService) }
|
||||
|
||||
before do
|
||||
create(:message, conversation: conversation, content: 'Hello', message_type: :incoming)
|
||||
@@ -19,6 +20,8 @@ RSpec.describe Captain::Conversation::ResponseBuilderJob, type: :job do
|
||||
allow(mock_llm_chat_service).to receive(:generate_response).and_return({ 'response' => 'Hey, welcome to Captain Specs' })
|
||||
allow(Captain::Assistant::AgentRunnerService).to receive(:new).and_return(mock_agent_runner_service)
|
||||
allow(mock_agent_runner_service).to receive(:generate_response).and_return({ 'response' => 'Hey, welcome to Captain V2' })
|
||||
allow(Captain::Llm::AssistantActionClassifierService).to receive(:new).and_return(mock_action_classifier_service)
|
||||
allow(mock_action_classifier_service).to receive(:classify).and_return({ 'action' => 'continue' })
|
||||
end
|
||||
|
||||
context 'when captain_v2 is disabled' do
|
||||
@@ -48,6 +51,107 @@ RSpec.describe Captain::Conversation::ResponseBuilderJob, type: :job do
|
||||
expect(account.usage_limits[:captain][:responses][:consumed]).to eq(1)
|
||||
end
|
||||
|
||||
it 'does not run the action classifier when the classifier feature is disabled' do
|
||||
expect(Captain::Llm::AssistantActionClassifierService).not_to receive(:new)
|
||||
|
||||
described_class.perform_now(conversation, assistant)
|
||||
|
||||
expect(conversation.messages.last.content).to eq('Hey, welcome to Captain Specs')
|
||||
end
|
||||
|
||||
context 'when V1 action classifier is enabled' do
|
||||
before do
|
||||
allow(account).to receive(:feature_enabled?).and_return(false)
|
||||
allow(account).to receive(:feature_enabled?).with('captain_integration_v2').and_return(false)
|
||||
allow(account).to receive(:feature_enabled?).with('captain_v1_action_classifier').and_return(true)
|
||||
end
|
||||
|
||||
it 'keeps the conversation pending when the classifier returns continue' do
|
||||
expect(Captain::Llm::AssistantActionClassifierService).to receive(:new).with(
|
||||
assistant: assistant,
|
||||
conversation: conversation
|
||||
).and_return(mock_action_classifier_service)
|
||||
expect(mock_action_classifier_service).to receive(:classify).with(
|
||||
message_history: [{ content: 'Hello', role: 'user' }],
|
||||
assistant_response: 'Hey, welcome to Captain Specs'
|
||||
).and_return({
|
||||
'action' => 'continue',
|
||||
'action_reason' => 'general_product_question',
|
||||
'model' => 'gpt-4.1'
|
||||
})
|
||||
|
||||
described_class.perform_now(conversation, assistant)
|
||||
|
||||
expect(conversation.reload.status).to eq('pending')
|
||||
expect(conversation.messages.outgoing.last.content).to eq('Hey, welcome to Captain Specs')
|
||||
expect(account.reload.usage_limits[:captain][:responses][:consumed]).to eq(1)
|
||||
end
|
||||
|
||||
it 'hands off without incrementing response usage when the classifier returns handoff' do
|
||||
allow(mock_action_classifier_service).to receive(:classify).and_return({
|
||||
'action' => 'handoff',
|
||||
'action_reason' => 'explicit_human_request',
|
||||
'model' => 'gpt-4.1'
|
||||
})
|
||||
|
||||
described_class.perform_now(conversation, assistant)
|
||||
|
||||
expect(conversation.reload.status).to eq('open')
|
||||
expect(conversation.messages.outgoing.last.content).to eq(I18n.t('conversations.captain.handoff'))
|
||||
expect(account.reload.usage_limits[:captain][:responses][:consumed]).to eq(0)
|
||||
end
|
||||
|
||||
it 'skips the classifier when the legacy handoff token is returned' do
|
||||
allow(mock_llm_chat_service).to receive(:generate_response).and_return({ 'response' => 'conversation_handoff' })
|
||||
expect(Captain::Llm::AssistantActionClassifierService).not_to receive(:new)
|
||||
|
||||
described_class.perform_now(conversation, assistant)
|
||||
|
||||
expect(conversation.reload.status).to eq('open')
|
||||
expect(conversation.messages.outgoing.last.content).to eq(I18n.t('conversations.captain.handoff'))
|
||||
end
|
||||
|
||||
it 'falls back to the assistant response when the classifier fails' do
|
||||
error = StandardError.new('classifier unavailable')
|
||||
allow(mock_action_classifier_service).to receive(:classify).and_raise(error)
|
||||
allow(ChatwootExceptionTracker).to receive(:new).and_call_original
|
||||
|
||||
described_class.perform_now(conversation, assistant)
|
||||
|
||||
expect(ChatwootExceptionTracker).to have_received(:new).with(error, account: account)
|
||||
expect(conversation.reload.status).to eq('pending')
|
||||
expect(conversation.messages.outgoing.last.content).to eq('Hey, welcome to Captain Specs')
|
||||
expect(account.reload.usage_limits[:captain][:responses][:consumed]).to eq(1)
|
||||
end
|
||||
|
||||
it 'falls back to the assistant response when the classifier returns an invalid action' do
|
||||
allow(mock_action_classifier_service).to receive(:classify).and_return({
|
||||
'action' => nil,
|
||||
'error' => 'invalid_classifier_response'
|
||||
})
|
||||
|
||||
described_class.perform_now(conversation, assistant)
|
||||
|
||||
expect(conversation.reload.status).to eq('pending')
|
||||
expect(conversation.messages.outgoing.last.content).to eq('Hey, welcome to Captain Specs')
|
||||
expect(account.reload.usage_limits[:captain][:responses][:consumed]).to eq(1)
|
||||
end
|
||||
|
||||
it 'skips the classifier when the conversation is no longer pending after response generation' do
|
||||
allow(mock_llm_chat_service).to receive(:generate_response) do
|
||||
conversation.open!
|
||||
{ 'response' => 'Hey, welcome to Captain Specs' }
|
||||
end
|
||||
|
||||
expect(Captain::Llm::AssistantActionClassifierService).not_to receive(:new)
|
||||
|
||||
described_class.perform_now(conversation, assistant)
|
||||
|
||||
expect(conversation.messages.outgoing.count).to eq(0)
|
||||
expect(account.reload.usage_limits[:captain][:responses][:consumed]).to eq(0)
|
||||
end
|
||||
end
|
||||
|
||||
it 'does not send a response when the conversation is no longer pending' do
|
||||
conversation.open!
|
||||
|
||||
@@ -292,9 +396,11 @@ RSpec.describe Captain::Conversation::ResponseBuilderJob, type: :job do
|
||||
it 'handles API errors and triggers handoff' do
|
||||
allow(mock_llm_chat_service).to receive(:generate_response)
|
||||
.and_raise(Faraday::BadRequestError, 'Bad request to image service')
|
||||
allow(Rails.logger).to receive(:info).and_call_original
|
||||
|
||||
described_class.perform_now(conversation, assistant)
|
||||
expect(conversation.reload.status).to eq('open')
|
||||
expect(Rails.logger).to have_received(:info).with(include('source=error reason=faraday_bad_request_error'))
|
||||
end
|
||||
|
||||
it 'succeeds when no error occurs' do
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
require 'rails_helper'
|
||||
|
||||
RSpec.describe Captain::Llm::AssistantActionClassifierService do
|
||||
let(:account) { create(:account) }
|
||||
let(:assistant) do
|
||||
create(
|
||||
:captain_assistant,
|
||||
account: account,
|
||||
config: {
|
||||
'instructions' => 'Only transfer to a manager after the user explicitly confirms.'
|
||||
}
|
||||
)
|
||||
end
|
||||
let(:conversation) { create(:conversation, account: account) }
|
||||
let(:service) { described_class.new(assistant: assistant, conversation: conversation) }
|
||||
let(:mock_chat) { instance_double(RubyLLM::Chat) }
|
||||
let(:mock_response) do
|
||||
instance_double(
|
||||
RubyLLM::Message,
|
||||
content: { 'action' => 'handoff', 'action_reason' => 'human_offer_accepted' }
|
||||
)
|
||||
end
|
||||
|
||||
before do
|
||||
allow(RubyLLM).to receive(:chat).and_return(mock_chat)
|
||||
allow(mock_chat).to receive(:with_temperature).and_return(mock_chat)
|
||||
allow(mock_chat).to receive(:with_schema).and_return(mock_chat)
|
||||
allow(mock_chat).to receive(:with_instructions).and_return(mock_chat)
|
||||
end
|
||||
|
||||
describe '#classify' do
|
||||
let(:message_history) do
|
||||
[
|
||||
{ role: 'user', content: 'I cannot log in' },
|
||||
{ role: 'assistant', content: 'Did you check your inbox?' },
|
||||
{ role: 'user', content: 'Yes, still no reset email' }
|
||||
]
|
||||
end
|
||||
|
||||
it 'passes delimited custom instructions and classifier context to the LLM' do
|
||||
expect(mock_chat).to receive(:with_schema).with(Captain::AssistantActionSchema).and_return(mock_chat)
|
||||
expect(mock_chat).to receive(:with_instructions).with(
|
||||
a_string_including('Account custom instructions are provided inside <account_custom_instructions> tags.')
|
||||
).and_return(mock_chat)
|
||||
expect(mock_chat).to receive(:ask) do |prompt|
|
||||
expect(prompt).to include(
|
||||
'<account_custom_instructions>',
|
||||
'Only transfer to a manager after the user explicitly confirms.',
|
||||
'<conversation_context>',
|
||||
'User: I cannot log in',
|
||||
'Assistant: Did you check your inbox?',
|
||||
'User: Yes, still no reset email',
|
||||
'<assistant_response_to_classify>',
|
||||
'Would you like to talk to support?'
|
||||
)
|
||||
expect(prompt).not_to include('"role"', '"content"', '<current_user_message>')
|
||||
|
||||
mock_response
|
||||
end
|
||||
|
||||
result = service.classify(message_history: message_history, assistant_response: 'Would you like to talk to support?')
|
||||
|
||||
expect(result).to include(
|
||||
'action' => 'handoff',
|
||||
'action_reason' => 'human_offer_accepted'
|
||||
)
|
||||
end
|
||||
|
||||
it 'uses the configured Captain model' do
|
||||
create(:installation_config, name: 'CAPTAIN_OPEN_AI_MODEL', value: 'gpt-4.1-nano')
|
||||
|
||||
expect(RubyLLM).to receive(:chat).with(model: 'gpt-4.1-nano').and_return(mock_chat)
|
||||
allow(mock_chat).to receive(:ask).and_return(mock_response)
|
||||
|
||||
result = service.classify(message_history: message_history, assistant_response: 'Would you like to talk to support?')
|
||||
|
||||
expect(result).to include('model' => 'gpt-4.1-nano')
|
||||
end
|
||||
|
||||
context 'when the assistant has no custom instructions' do
|
||||
before do
|
||||
assistant.update!(config: assistant.config.except('instructions'))
|
||||
end
|
||||
|
||||
it 'does not add custom-instruction policy to the system prompt' do
|
||||
expect(mock_chat).to receive(:with_instructions).with(
|
||||
satisfy { |prompt| prompt.exclude?('Account custom instructions are provided') }
|
||||
).and_return(mock_chat)
|
||||
allow(mock_chat).to receive(:ask).and_return(mock_response)
|
||||
|
||||
service.classify(message_history: message_history, assistant_response: 'Would you like to talk to support?')
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -188,4 +188,29 @@ RSpec.describe Captain::Llm::AssistantChatService do
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe 'account custom instructions in system prompt' do
|
||||
before do
|
||||
assistant.update!(config: assistant.config.merge('instructions' => 'if user enters 1112234 suggest handoff'))
|
||||
end
|
||||
|
||||
it 'adds custom instructions in a separate delimited section' do
|
||||
allow(mock_chat).to receive(:ask).and_return(mock_response)
|
||||
|
||||
expect(mock_chat).to receive(:with_instructions).with(
|
||||
a_string_including(
|
||||
'<account_custom_instructions>',
|
||||
'if user enters 1112234 suggest handoff',
|
||||
'</account_custom_instructions>'
|
||||
)
|
||||
) do |instructions|
|
||||
expect(instructions).not_to include('<custom-instructions>')
|
||||
expect(instructions.index('<account_custom_instructions>')).to be < instructions.index('```json')
|
||||
mock_chat
|
||||
end
|
||||
|
||||
service = described_class.new(assistant: assistant, conversation: conversation)
|
||||
service.generate_response(message_history: [{ role: 'user', content: 'Hello' }])
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user