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>
121 lines
3.2 KiB
Ruby
121 lines
3.2 KiB
Ruby
module Reports::ReportMetricRegistry
|
|
# Describes one public report metric.
|
|
# name: API-facing metric name requested by reports.
|
|
# aggregate: whether the metric is a count or average.
|
|
# raw_event_name: source reporting_events name for raw queries.
|
|
# rollup_metric: source reporting_events_rollups metric for rollup queries.
|
|
# summary_key: key used when this metric appears in grouped summary responses.
|
|
# raw_count_strategy: optional raw-query counting rule, such as distinct conversations.
|
|
Metric = Data.define(
|
|
:name,
|
|
:aggregate,
|
|
:raw_event_name,
|
|
:rollup_metric,
|
|
:summary_key,
|
|
:raw_count_strategy
|
|
) do
|
|
def initialize(name:, aggregate:, raw_event_name: nil, rollup_metric: nil, summary_key: nil, raw_count_strategy: nil) # rubocop:disable Metrics/ParameterLists
|
|
super
|
|
end
|
|
|
|
def average?
|
|
aggregate == :average
|
|
end
|
|
|
|
def count?
|
|
aggregate == :count
|
|
end
|
|
|
|
def rollup_supported?
|
|
rollup_metric.present?
|
|
end
|
|
|
|
def summary?
|
|
summary_key.present?
|
|
end
|
|
end
|
|
|
|
METRICS = {
|
|
conversations_count: Metric.new(
|
|
name: :conversations_count,
|
|
aggregate: :count
|
|
),
|
|
incoming_messages_count: Metric.new(
|
|
name: :incoming_messages_count,
|
|
aggregate: :count
|
|
),
|
|
outgoing_messages_count: Metric.new(
|
|
name: :outgoing_messages_count,
|
|
aggregate: :count
|
|
),
|
|
avg_first_response_time: Metric.new(
|
|
name: :avg_first_response_time,
|
|
aggregate: :average,
|
|
raw_event_name: :first_response,
|
|
rollup_metric: :first_response,
|
|
summary_key: :avg_first_response_time
|
|
),
|
|
avg_resolution_time: Metric.new(
|
|
name: :avg_resolution_time,
|
|
aggregate: :average,
|
|
raw_event_name: :conversation_resolved,
|
|
rollup_metric: :resolution_time,
|
|
summary_key: :avg_resolution_time
|
|
),
|
|
reply_time: Metric.new(
|
|
name: :reply_time,
|
|
aggregate: :average,
|
|
raw_event_name: :reply_time,
|
|
rollup_metric: :reply_time,
|
|
summary_key: :avg_reply_time
|
|
),
|
|
resolutions_count: Metric.new(
|
|
name: :resolutions_count,
|
|
aggregate: :count,
|
|
raw_event_name: :conversation_resolved,
|
|
rollup_metric: :resolutions_count,
|
|
summary_key: :resolved_conversations_count
|
|
),
|
|
bot_resolutions_count: Metric.new(
|
|
name: :bot_resolutions_count,
|
|
aggregate: :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,
|
|
aggregate: :count,
|
|
raw_event_name: :conversation_bot_handoff,
|
|
rollup_metric: :bot_handoffs_count,
|
|
raw_count_strategy: :distinct_conversation
|
|
)
|
|
}.freeze
|
|
|
|
SUMMARY_METRIC_NAMES = %i[
|
|
resolutions_count
|
|
avg_resolution_time
|
|
avg_first_response_time
|
|
reply_time
|
|
].freeze
|
|
|
|
module_function
|
|
|
|
def fetch(name)
|
|
return if name.blank?
|
|
|
|
METRICS[name.to_sym]
|
|
end
|
|
|
|
def supported?(name)
|
|
fetch(name).present?
|
|
end
|
|
|
|
def rollup_supported?(name)
|
|
fetch(name)&.rollup_supported? || false
|
|
end
|
|
|
|
def summary_metrics
|
|
SUMMARY_METRIC_NAMES.map { |metric_name| METRICS.fetch(metric_name) }
|
|
end
|
|
end
|