fix: prevent bot metrics double-counting when handoff and resolution coexist [CW-6210] (#14032)

The bot metrics dashboard can show `handoff_rate + resolution_rate >
100%`. A single conversation can accumulate both
`conversation_bot_handoff` and `conversation_bot_resolved` events, and
the rate queries count them independently against a shared denominator.

## How it happens

```
Customer messages bot inbox
        │
        ▼
   ┌──────────┐
   │ pending  │ (bot handling)
   └────┬─────┘
        │ bot can't help
        ▼
   ┌──────────┐
   │   open   │ (handed off → conversation_bot_handoff event created)
   └────┬─────┘
        │ agent clicks "Resolve" WITHOUT sending a message
        ▼
   ┌──────────┐
   │ resolved │ conversation_resolved fires
   └──────────┘
        │
        ▼
   create_bot_resolved_event guard checks:
     ✅ inbox.active_bot?
     ✅ no outgoing messages with sender_type: 'User'  ← agent never messaged!
        │
        ▼
   conversation_bot_resolved event ALSO created ← BUG
        │
        ▼
   Same conversation counted in BOTH rates → sum exceeds 100%
```

## Why fix at the read path, not the write path

An earlier attempt added guards in the listener to make the two events
mutually exclusive per conversation — deleting `bot_resolved` when a
handoff fires, suppressing resolutions when a handoff exists. This was
rejected because conversations can be reopened across multiple cycles
(bot resolves on day 1, customer returns on day 5, bot hands off).
Deleting the day-1 resolution corrupts historical reports, and the async
event dispatcher makes listener-level guards vulnerable to race
conditions.

## What this PR does

Within a reporting window, if a conversation has both events, **handoff
wins** — the conversation is excluded from the resolution count. This is
applied via SQL subquery across all three read paths:

```
                    ┌─────────────────────────┐
                    │   Reporting Events DB    │
                    │                          │
                    │  conv_bot_handoff: [A,B] │
                    │  conv_bot_resolved: [A,C]│
                    └────────┬────────────────┘
                             │
              ┌──────────────┼──────────────┐
              ▼              ▼              ▼
       BotMetricsBuilder  ReportHelper  CountReportBuilder
       (rate cards)       (bot_summary)  (timeseries charts)
              │              │              │
              ▼              ▼              ▼
       resolutions:        resolutions:   resolutions:
       [A,C] minus [A,B]  same logic     same logic
       = [C] only          = [C] only     = [C] only

       Result: Conversation A → handoff only
               Conversation B → handoff only
               Conversation C → resolution only
```

For wide date ranges spanning multiple lifecycles, a conversation
bot-resolved in one cycle and handed off in a later cycle will only show
as a handoff. This is an acceptable tradeoff — the alternative (>100%
rates) is clearly worse, and narrow ranges handle this correctly since
the events fall into different windows. No reporting events are
modified, so historical data stays intact.

## Diagnostic tool

`rake bot_metrics:diagnose` — read-only task that prompts for account ID
and date range, shows a before/after rate comparison without modifying
data.

---------

Co-authored-by: aakashb95 <aakashbakhle@gmail.com>
Co-authored-by: Aakash Bakhle <48802744+aakashb95@users.noreply.github.com>

app/builders/v2/reports/bot_metrics_builder.rb

@@ -31,13 +31,24 @@ class V2::Reports::BotMetricsBuilder
   end
 
   def bot_resolutions_count
-    account.reporting_events.joins(:conversation).select(:conversation_id).where(account_id: account.id, name: :conversation_bot_resolved,
-                                                                                 created_at: range).distinct.count
+    # Exclude conversations that also had a handoff in the same range — handoff wins
+    account.reporting_events.joins(:conversation).select(:conversation_id)
+           .where(account_id: account.id, name: :conversation_bot_resolved, created_at: range)
+           .where.not(conversation_id: bot_handoff_conversation_ids_subquery)
+           .distinct.count
   end
 
   def bot_handoffs_count
-    account.reporting_events.joins(:conversation).select(:conversation_id).where(account_id: account.id, name: :conversation_bot_handoff,
-                                                                                 created_at: range).distinct.count
+    account.reporting_events.joins(:conversation).select(:conversation_id)
+           .where(account_id: account.id, name: :conversation_bot_handoff, created_at: range)
+           .distinct.count
+  end
+
+  def bot_handoff_conversation_ids_subquery
+    account.reporting_events
+           .where(name: :conversation_bot_handoff, created_at: range)
+           .where.not(conversation_id: nil)
+           .select(:conversation_id)
   end
 
   def bot_resolution_rate

app/helpers/report_helper.rb

@@ -53,13 +53,12 @@ module ReportHelper
   end
 
   def resolutions
-    scope.reporting_events.where(account_id: account.id, name: :conversation_resolved,
-                                 created_at: range)
+    scope.reporting_events.where(account_id: account.id, name: :conversation_resolved, created_at: range)
   end
 
   def bot_resolutions
-    scope.reporting_events.where(account_id: account.id, name: :conversation_bot_resolved,
-                                 created_at: range)
+    scope.reporting_events.where(account_id: account.id, name: :conversation_bot_resolved, created_at: range)
+         .where.not(conversation_id: bot_handoff_conversation_ids_subquery)
   end
 
   def bot_handoffs
@@ -67,6 +66,10 @@ module ReportHelper
                                                                                created_at: range).distinct
   end
 
+  def bot_handoff_conversation_ids_subquery
+    bot_handoffs
+  end
+
   def avg_first_response_time
     grouped_reporting_events = (get_grouped_values scope.reporting_events.where(name: 'first_response', account_id: account.id))
     return grouped_reporting_events.average(:value_in_business_hours) if params[:business_hours]

app/services/reports/raw_data_source.rb

@@ -83,11 +83,20 @@ class Reports::RawDataSource < Reports::DataSource
       created_at: range
     )
 
+    return events.where.not(conversation_id: bot_handoff_conversation_ids_subquery) if raw_count_strategy == :exclude_bot_handoffs
     return events unless raw_count_strategy == :distinct_conversation
 
     events.joins(:conversation).select(:conversation_id).distinct
   end
 
+  def bot_handoff_conversation_ids_subquery
+    scope.reporting_events.where(
+      name: :conversation_bot_handoff,
+      account_id: account.id,
+      created_at: range
+    ).where.not(conversation_id: nil).select(:conversation_id)
+  end
+
   def summary_scope
     scope = account.reporting_events.where(created_at: range)
     return scope.joins(:conversation) if dimension_type == 'team'

app/services/reports/report_metric_registry.rb

@@ -79,8 +79,8 @@ module Reports::ReportMetricRegistry
     bot_resolutions_count: Metric.new(
       name: :bot_resolutions_count,
       aggregate: :count,
-      raw_event_name: :conversation_bot_resolved,
-      rollup_metric: :bot_resolutions_count
+      raw_event_name: :conversation_bot_resolved, rollup_metric: :bot_resolutions_count,
+      raw_count_strategy: :exclude_bot_handoffs
     ),
     bot_handoffs_count: Metric.new(
       name: :bot_handoffs_count,

spec/builders/v2/report_builder_spec.rb

@@ -120,6 +120,8 @@ describe V2::ReportBuilder do
             # Reopen 1 conversation
             conversations.first.open!
           end
+          create(:reporting_event, account: account, inbox: account.inboxes.first, conversation: nil, conversation_id: nil,
+                                   name: 'conversation_bot_handoff', created_at: Time.zone.today)
 
           builder = described_class.new(account, params)
           metrics = builder.timeseries

spec/builders/v2/reports/bot_metrics_builder_spec.rb

@@ -4,35 +4,99 @@ RSpec.describe V2::Reports::BotMetricsBuilder do
   subject(:bot_metrics_builder) { described_class.new(inbox.account, params) }
 
   let(:inbox) { create(:inbox) }
-  let!(:resolved_conversation) { create(:conversation, account: inbox.account, inbox: inbox, created_at: 2.days.ago) }
-  let!(:unresolved_conversation) { create(:conversation, account: inbox.account, inbox: inbox, created_at: 2.days.ago) }
   let(:since) { 1.week.ago.to_i.to_s }
   let(:until_time) { Time.now.to_i.to_s }
   let(:params) { { since: since, until: until_time } }
 
   before do
     create(:agent_bot_inbox, inbox: inbox)
-    create(:message, account: inbox.account, conversation: resolved_conversation, created_at: 2.days.ago, message_type: 'outgoing')
-    create(:reporting_event, account_id: inbox.account.id, name: 'conversation_bot_resolved', conversation_id: resolved_conversation.id,
-                             created_at: 2.days.ago)
-    create(:reporting_event, account_id: inbox.account.id, name: 'conversation_bot_handoff',
-                             conversation_id: resolved_conversation.id, created_at: 2.days.ago)
-    create(:reporting_event, account_id: inbox.account.id, name: 'conversation_bot_handoff',
-                             conversation_id: unresolved_conversation.id, created_at: 2.days.ago)
   end
 
   describe '#metrics' do
     context 'with valid params' do
+      let!(:resolved_conversation) { create(:conversation, account: inbox.account, inbox: inbox, created_at: 2.days.ago) }
+      let!(:handoff_conversation) { create(:conversation, account: inbox.account, inbox: inbox, created_at: 2.days.ago) }
+
+      before do
+        create(:message, account: inbox.account, conversation: resolved_conversation, created_at: 2.days.ago, message_type: 'outgoing')
+        create(:reporting_event, account_id: inbox.account.id, name: 'conversation_bot_resolved',
+                                 conversation_id: resolved_conversation.id, created_at: 2.days.ago)
+        create(:reporting_event, account_id: inbox.account.id, name: 'conversation_bot_handoff',
+                                 conversation_id: handoff_conversation.id, created_at: 2.days.ago)
+      end
+
       it 'returns correct metrics' do
         metrics = bot_metrics_builder.metrics
 
         expect(metrics[:conversation_count]).to eq(2)
         expect(metrics[:message_count]).to eq(1)
         expect(metrics[:resolution_rate]).to eq(50)
+        expect(metrics[:handoff_rate]).to eq(50)
+      end
+    end
+
+    context 'when a conversation has both bot_resolved and bot_handoff events in the same range' do
+      let!(:double_counted_conversation) { create(:conversation, account: inbox.account, inbox: inbox, created_at: 2.days.ago) }
+      let!(:handoff_only_conversation) { create(:conversation, account: inbox.account, inbox: inbox, created_at: 2.days.ago) }
+
+      before do
+        create(:reporting_event, account_id: inbox.account.id, name: 'conversation_bot_resolved',
+                                 conversation_id: double_counted_conversation.id, created_at: 2.days.ago)
+        create(:reporting_event, account_id: inbox.account.id, name: 'conversation_bot_handoff',
+                                 conversation_id: double_counted_conversation.id, created_at: 2.days.ago)
+        create(:reporting_event, account_id: inbox.account.id, name: 'conversation_bot_handoff',
+                                 conversation_id: handoff_only_conversation.id, created_at: 2.days.ago)
+      end
+
+      it 'excludes the conversation from resolution count — handoff wins' do
+        metrics = bot_metrics_builder.metrics
+
+        expect(metrics[:conversation_count]).to eq(2)
+        expect(metrics[:resolution_rate]).to eq(0)
         expect(metrics[:handoff_rate]).to eq(100)
       end
     end
 
+    context 'when bot_resolved and bot_handoff are in different date ranges' do
+      let!(:multi_cycle_conversation) { create(:conversation, account: inbox.account, inbox: inbox, created_at: 2.days.ago) }
+
+      before do
+        # Bot resolved in current range
+        create(:reporting_event, account_id: inbox.account.id, name: 'conversation_bot_resolved',
+                                 conversation_id: multi_cycle_conversation.id, created_at: 2.days.ago)
+        # Handoff happened before the range (in a previous cycle)
+        create(:reporting_event, account_id: inbox.account.id, name: 'conversation_bot_handoff',
+                                 conversation_id: multi_cycle_conversation.id, created_at: 2.weeks.ago)
+      end
+
+      it 'counts the resolution since the handoff is outside the range' do
+        metrics = bot_metrics_builder.metrics
+
+        expect(metrics[:conversation_count]).to eq(1)
+        expect(metrics[:resolution_rate]).to eq(100)
+        expect(metrics[:handoff_rate]).to eq(0)
+      end
+    end
+
+    context 'when a bot_handoff event has no conversation' do
+      let!(:resolved_conversation) { create(:conversation, account: inbox.account, inbox: inbox, created_at: 2.days.ago) }
+
+      before do
+        create(:reporting_event, account_id: inbox.account.id, name: 'conversation_bot_resolved',
+                                 conversation_id: resolved_conversation.id, created_at: 2.days.ago)
+        create(:reporting_event, account: inbox.account, inbox: inbox, conversation: nil, conversation_id: nil,
+                                 name: 'conversation_bot_handoff', created_at: 2.days.ago)
+      end
+
+      it 'does not exclude all bot resolutions' do
+        metrics = bot_metrics_builder.metrics
+
+        expect(metrics[:conversation_count]).to eq(1)
+        expect(metrics[:resolution_rate]).to eq(100)
+        expect(metrics[:handoff_rate]).to eq(0)
+      end
+    end
+
     context 'with missing params' do
       let(:params) { {} }
 

spec/builders/v2/reports/timeseries/report_builder_spec.rb

@@ -310,4 +310,43 @@ describe V2::Reports::Timeseries::ReportBuilder do
       end
     end
   end
+
+  describe 'bot resolution counts' do
+    subject(:builder) { described_class.new(account, params) }
+
+    let(:account) { create(:account) }
+    let(:inbox) { create(:inbox, account: account) }
+    let(:current_time) { Time.current }
+    let(:params) do
+      {
+        type: 'account',
+        metric: 'bot_resolutions_count',
+        since: (current_time - 1.day).beginning_of_day.to_i.to_s,
+        until: current_time.end_of_day.to_i.to_s,
+        timezone_offset: nil,
+        group_by: 'day'
+      }
+    end
+
+    before do
+      travel_to current_time
+
+      resolved_conversation = create(:conversation, account: account, inbox: inbox)
+      double_counted_conversation = create(:conversation, account: account, inbox: inbox)
+
+      create(:reporting_event, name: 'conversation_bot_resolved', account: account, conversation: resolved_conversation,
+                               created_at: current_time)
+      create(:reporting_event, name: 'conversation_bot_resolved', account: account, conversation: double_counted_conversation,
+                               created_at: current_time)
+      create(:reporting_event, name: 'conversation_bot_handoff', account: account, conversation: double_counted_conversation,
+                               created_at: current_time)
+      create(:reporting_event, name: 'conversation_bot_handoff', account: account, inbox: inbox, conversation: nil, conversation_id: nil,
+                               created_at: current_time)
+    end
+
+    it 'excludes conversations that also had a bot handoff in the range' do
+      expect(builder.aggregate_value).to eq(1)
+      expect(builder.timeseries.sum { |row| row[:value] }).to eq(1)
+    end
+  end
 end

spec/services/reports/report_metric_registry_spec.rb

@@ -30,6 +30,15 @@ RSpec.describe Reports::ReportMetricRegistry do
       expect(metric.raw_count_strategy).to eq(:distinct_conversation)
     end
 
+    it 'locks the handoff exclusion strategy for bot_resolutions_count' do
+      metric = described_class.fetch(:bot_resolutions_count)
+
+      expect(metric.count?).to be(true)
+      expect(metric.raw_event_name).to eq(:conversation_bot_resolved)
+      expect(metric.rollup_metric).to eq(:bot_resolutions_count)
+      expect(metric.raw_count_strategy).to eq(:exclude_bot_handoffs)
+    end
+
     it 'returns nil for unsupported metrics' do
       expect(described_class.fetch(:unknown_metric)).to be_nil
     end