379e28df1f
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>
109 lines
4.9 KiB
Ruby
109 lines
4.9 KiB
Ruby
require 'rails_helper'
|
|
|
|
RSpec.describe V2::Reports::BotMetricsBuilder do
|
|
subject(:bot_metrics_builder) { described_class.new(inbox.account, params) }
|
|
|
|
let(:inbox) { create(:inbox) }
|
|
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)
|
|
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) { {} }
|
|
|
|
it 'handles missing since and until params gracefully' do
|
|
expect { bot_metrics_builder.metrics }.not_to raise_error
|
|
end
|
|
end
|
|
end
|
|
end
|