mirror of
https://github.com/chatwoot/chatwoot.git
synced 2026-06-04 21:02:35 +08:00
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>
166 lines
4.8 KiB
Ruby
166 lines
4.8 KiB
Ruby
class Reports::RawDataSource < Reports::DataSource
|
|
def timeseries
|
|
average_metric? ? average_timeseries : count_timeseries
|
|
end
|
|
|
|
def aggregate
|
|
average_metric? ? average_scope.average(average_value_key) : count_scope.count
|
|
end
|
|
|
|
def summary
|
|
metric_results = summary_scope
|
|
.select(*summary_select_fields)
|
|
.group(summary_group_by_key)
|
|
.index_by { |record| record.public_send(summary_index_key) }
|
|
|
|
merge_summary_results(metric_results, summary_conversation_counts)
|
|
end
|
|
|
|
private
|
|
|
|
def count_timeseries
|
|
grouped_count.map do |event_date, event_count|
|
|
{ value: event_count, timestamp: event_date.in_time_zone(timezone).to_i }
|
|
end
|
|
end
|
|
|
|
def average_timeseries
|
|
grouped_average_time = grouped_average_scope.average(average_value_key)
|
|
grouped_event_count = grouped_average_scope.count
|
|
|
|
grouped_average_time.each_with_object([]) do |(event_date, average_time), results|
|
|
results << {
|
|
value: average_time,
|
|
timestamp: event_date.in_time_zone(timezone).to_i,
|
|
count: grouped_event_count[event_date]
|
|
}
|
|
end
|
|
end
|
|
|
|
def grouped_average_scope
|
|
average_scope.group_by_period(
|
|
group_by,
|
|
:created_at,
|
|
default_value: 0,
|
|
range: range,
|
|
permit: %w[day week month year hour],
|
|
time_zone: timezone
|
|
)
|
|
end
|
|
|
|
def grouped_count
|
|
count_scope.group_by_period(
|
|
group_by,
|
|
:created_at,
|
|
default_value: 0,
|
|
range: range,
|
|
permit: %w[day week month year hour],
|
|
time_zone: timezone
|
|
).count
|
|
end
|
|
|
|
def average_scope
|
|
scope.reporting_events.where(name: raw_event_name, created_at: range, account_id: account.id)
|
|
end
|
|
|
|
def count_scope
|
|
case metric.to_s
|
|
when 'conversations_count'
|
|
scope.conversations.where(account_id: account.id, created_at: range)
|
|
when 'incoming_messages_count'
|
|
scope.messages.where(account_id: account.id, created_at: range).incoming.unscope(:order)
|
|
when 'outgoing_messages_count'
|
|
scope.messages.where(account_id: account.id, created_at: range).outgoing.unscope(:order)
|
|
else
|
|
reporting_event_count_scope
|
|
end
|
|
end
|
|
|
|
def reporting_event_count_scope
|
|
events = scope.reporting_events.where(
|
|
name: raw_event_name,
|
|
account_id: account.id,
|
|
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'
|
|
|
|
scope
|
|
end
|
|
|
|
def summary_conversation_counts
|
|
account.conversations
|
|
.where(created_at: range)
|
|
.group(summary_conversation_group_by_key)
|
|
.count
|
|
end
|
|
|
|
def merge_summary_results(metric_results, conversation_counts)
|
|
(metric_results.keys | conversation_counts.keys).each_with_object({}) do |dimension_id, results|
|
|
record = metric_results[dimension_id]
|
|
results[dimension_id] = summary_attributes_for(record, conversation_counts[dimension_id])
|
|
end
|
|
end
|
|
|
|
def summary_select_fields
|
|
["#{summary_group_by_key} as #{summary_index_key}"] + summary_metrics.map { |definition| summary_select_field(definition) }
|
|
end
|
|
|
|
def summary_select_field(definition)
|
|
if definition.count?
|
|
"COUNT(CASE WHEN name = '#{definition.raw_event_name}' THEN 1 END) as #{definition.summary_key}"
|
|
else
|
|
"AVG(CASE WHEN name = '#{definition.raw_event_name}' THEN #{average_value_key} END) as #{definition.summary_key}"
|
|
end
|
|
end
|
|
|
|
def summary_attributes_for(record, conversations_count = 0)
|
|
summary_metrics.each_with_object({ conversations_count: conversations_count.to_i }) do |definition, attributes|
|
|
value = record&.public_send(definition.summary_key)
|
|
attributes[definition.summary_key] = definition.count? ? value.to_i : value
|
|
end
|
|
end
|
|
|
|
def summary_group_by_key
|
|
{
|
|
'account' => :account_id,
|
|
'agent' => :user_id,
|
|
'inbox' => :inbox_id,
|
|
'team' => 'conversations.team_id'
|
|
}[dimension_type]
|
|
end
|
|
|
|
def summary_conversation_group_by_key
|
|
{
|
|
'account' => :account_id,
|
|
'agent' => :assignee_id,
|
|
'inbox' => :inbox_id,
|
|
'team' => :team_id
|
|
}[dimension_type]
|
|
end
|
|
|
|
def summary_index_key
|
|
summary_group_by_key.to_s.split('.').last
|
|
end
|
|
|
|
def average_value_key
|
|
use_business_hours? ? :value_in_business_hours : :value
|
|
end
|
|
end
|