From 566de02385ce216ae25e7831411db105ad81ca8c Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Wed, 7 Jan 2026 13:57:43 +0530 Subject: [PATCH] feat: allow agent bot and captain responses to reset waiting since (#13181) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When AgentBot responds to customer messages, the `waiting_since` timestamp is not reset, causing inflated reply time metrics when a human agent eventually responds. This results in inaccurate reporting that incorrectly includes periods when customers were satisfied with bot responses. ### Timeline from Production Data ``` Dec 12, 16:20:14 - Customer sends message (ID: 368451924) ↓ waiting_since = Dec 12, 16:20:14 Dec 12, 16:20:17 - AgentBot replies (ID: 368451960) ↓ waiting_since STILL = Dec 12, 16:20:14 ❌ ↓ (Bot response doesn't clear it) 14-day gap - Customer satisfied, no messages ↓ waiting_since STILL = Dec 12, 16:20:14 ❌ Dec 26, 22:25:45 - Customer sends new message (ID: 383522275) ↓ waiting_since STILL = Dec 12, 16:20:14 ❌ ↓ (New message doesn't reset it) Dec 26-27 - More AgentBot interactions ↓ waiting_since STILL = Dec 12, 16:20:14 ❌ Dec 27, 07:36:53 - Human agent finally replies (ID: 383799517) ↓ Reply time calculated: 1,268,404 seconds ↓ = 14.7 DAYS ❌ ``` ## Root Cause The core issues is in `app/models/message.rb`, where **AgentBot messages does not clear `waiting_since`** - The `human_response?` method only returns true for `User` senders, so bot replies never trigger the clearing logic. This means once `waiting_since` is set, it stays set even when customers send new messages after receiving bot responses. The solution is to simply reset `waiting_since` **after a bot has responded**. This ensures reply time metrics reflect actual human agent response times, not bot-handled periods. ### What triggers the rest This is an intentional "gotcha", that only `AgentBot` and `Captain::Assistant` messages trigger the waiting time reset. Automation and campaign messages maintain current behavior (no reset). This is because interactive bot assistants provide conversational help that might satisfy customers. Automation and campaigns are one-way communications and shouldn't affect waiting time calculations. ## Related Work Extends PR #11787 which fixed `waiting_since` clearing on conversation resolution. This PR addresses the bot interaction scenario which was not covered by that fix. Scripts to clean data: https://gist.github.com/scmmishra/bd133208e219d0ab52fbfdf03036c48a --- app/models/message.rb | 24 ++++++++-- spec/enterprise/models/message_spec.rb | 3 +- spec/models/conversation_spec.rb | 65 ++++++++++++++++++++++++++ spec/models/message_spec.rb | 57 ++++++++++++++++++++++ 4 files changed, 143 insertions(+), 6 deletions(-) diff --git a/app/models/message.rb b/app/models/message.rb index 0bf7a176b..3be04c54f 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -322,12 +322,21 @@ class Message < ApplicationRecord end def update_waiting_since - if human_response? && !private && conversation.waiting_since.present? - Rails.configuration.dispatcher.dispatch( - REPLY_CREATED, Time.zone.now, waiting_since: conversation.waiting_since, message: self - ) - conversation.update(waiting_since: nil) + waiting_present = conversation.waiting_since.present? + + if waiting_present && !private + if human_response? + Rails.configuration.dispatcher.dispatch( + REPLY_CREATED, Time.zone.now, waiting_since: conversation.waiting_since, message: self + ) + conversation.update(waiting_since: nil) + elsif bot_response? + # Bot responses also clear waiting_since (simpler than checking on next customer message) + conversation.update(waiting_since: nil) + end end + + # Set waiting_since when customer sends a message (if currently blank) conversation.update(waiting_since: created_at) if incoming? && conversation.waiting_since.blank? end @@ -341,6 +350,11 @@ class Message < ApplicationRecord sender.is_a?(User) end + def bot_response? + # Check if this is a response from AgentBot or Captain::Assistant + outgoing? && sender_type.in?(['AgentBot', 'Captain::Assistant']) + end + def dispatch_create_events Rails.configuration.dispatcher.dispatch(MESSAGE_CREATED, Time.zone.now, message: self, performed_by: Current.executed_by) diff --git a/spec/enterprise/models/message_spec.rb b/spec/enterprise/models/message_spec.rb index 5a9dc4e27..aa1537e65 100644 --- a/spec/enterprise/models/message_spec.rb +++ b/spec/enterprise/models/message_spec.rb @@ -14,8 +14,9 @@ RSpec.describe Message do create(:message, message_type: :outgoing, conversation: conversation, sender: captain_assistant) + # Captain::Assistant responses clear waiting_since (like AgentBot) expect(conversation.first_reply_created_at).to be_nil - expect(conversation.waiting_since).to be_within(0.000001.seconds).of(conversation.created_at) + expect(conversation.waiting_since).to be_nil create(:message, message_type: :outgoing, conversation: conversation) diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index 7a5398456..5a4acf329 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -978,5 +978,70 @@ RSpec.describe Conversation do reply_events = account.reporting_events.where(name: 'reply_time', conversation_id: conversation.id) expect(reply_events.count).to eq(0) end + + context 'when AgentBot responds between customer messages' do + let(:agent_bot) { create(:agent_bot, account: account) } + + def create_bot_message(conversation, created_at: Time.current) + message = nil + perform_enqueued_jobs do + message = create(:message, + message_type: 'outgoing', + account: conversation.account, + inbox: conversation.inbox, + conversation: conversation, + sender: agent_bot, + created_at: created_at) + end + message + end + + it 'calculates reply time from the most recent customer message after bot response' do + # Initial conversation: customer message -> agent first reply (to establish first_reply_created_at) + create_customer_message(conversation, created_at: 10.hours.ago) + create_agent_message(conversation, created_at: 9.hours.ago) + + # Customer message 1 + create_customer_message(conversation, created_at: 5.hours.ago) + + # Bot responds + create_bot_message(conversation, created_at: 4.hours.ago) + + # Customer message 2 (after bot response) - should reset waiting_since + create_customer_message(conversation, created_at: 2.hours.ago) + + # Human agent replies - should create reply_time event from customer message 2 + create_agent_message(conversation, created_at: 1.hour.ago) + + reply_events = account.reporting_events.where(name: 'reply_time', conversation_id: conversation.id) + expect(reply_events.count).to eq(1) # Only the second agent reply creates a reply_time event + # Reply time should be 1 hour (from customer message 2 to agent reply) + expect(reply_events.first.value).to be_within(60).of(3600) + end + + it 'handles multiple bot responses before customer messages again' do + # Initial conversation: customer message -> agent first reply + create_customer_message(conversation, created_at: 10.hours.ago) + create_agent_message(conversation, created_at: 9.hours.ago) + + # Customer message 1 + create_customer_message(conversation, created_at: 6.hours.ago) + + # Bot responds multiple times + create_bot_message(conversation, created_at: 5.hours.ago) + create_bot_message(conversation, created_at: 4.hours.ago) + + # Customer message 2 (after multiple bot responses) - should reset waiting_since + create_customer_message(conversation, created_at: 2.hours.ago) + + # Human agent replies + create_agent_message(conversation, created_at: 1.hour.ago) + + reply_events = account.reporting_events.where(name: 'reply_time', conversation_id: conversation.id) + expect(reply_events.count).to eq(1) # Only the second agent reply creates a reply_time event + # Reply time should be 1 hour (from customer message 2 to agent reply) + expect(reply_events.first.value).to be_within(60).of(3600) + end + end end end diff --git a/spec/models/message_spec.rb b/spec/models/message_spec.rb index 8905f5103..d606c266d 100644 --- a/spec/models/message_spec.rb +++ b/spec/models/message_spec.rb @@ -300,6 +300,63 @@ RSpec.describe Message do expect(conversation.waiting_since).to eq old_waiting_since end + + context 'when bot has responded to the conversation' do + let(:agent_bot) { create(:agent_bot, account: conversation.account) } + + before do + # Create initial customer message + create(:message, conversation: conversation, message_type: :incoming, + created_at: 2.hours.ago) + conversation.update(waiting_since: 2.hours.ago) + + # Bot responds + create(:message, conversation: conversation, message_type: :outgoing, + sender: agent_bot, created_at: 1.hour.ago) + end + + it 'resets waiting_since when customer sends a new message after bot response' do + new_message = build(:message, conversation: conversation, message_type: :incoming) + new_message.save! + + conversation.reload + expect(conversation.waiting_since).to be_within(1.second).of(new_message.created_at) + end + + it 'does not reset waiting_since if last response was from human agent' do + # Human agent responds (clears waiting_since) + create(:message, conversation: conversation, message_type: :outgoing, + sender: agent) + conversation.reload + expect(conversation.waiting_since).to be_nil + + # Customer sends new message + new_message = build(:message, conversation: conversation, message_type: :incoming) + new_message.save! + + conversation.reload + expect(conversation.waiting_since).to be_within(1.second).of(new_message.created_at) + end + + it 'clears waiting_since when bot responds' do + # After the bot response in before block, waiting_since should already be cleared + conversation.reload + expect(conversation.waiting_since).to be_nil + + # Customer sends another message + create(:message, conversation: conversation, message_type: :incoming, + created_at: 30.minutes.ago) + conversation.reload + expect(conversation.waiting_since).to be_within(1.second).of(30.minutes.ago) + + # Another bot response should clear it again + create(:message, conversation: conversation, message_type: :outgoing, + sender: agent_bot, created_at: 15.minutes.ago) + + conversation.reload + expect(conversation.waiting_since).to be_nil + end + end end context 'with webhook_data' do