From ddb0535a93f032ef8ea1b215a7ee1b11763340b0 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Wed, 22 Jul 2026 22:03:25 +0530 Subject: [PATCH] perf: reuse resolved count for reopen rate (#15122) This improves the Captain overview by loading reporting metrics and FAQ stats from separate endpoints. Range changes now refresh only the metrics, while reopen-rate calculation reuses the resolved conversation count to avoid redundant database queries. ## What changed - Split Captain overview metrics and FAQ stats into separate APIs. - Fetch FAQ stats independently from range-based metrics. - Reuse resolved conversation totals when calculating reopen rate. - Skip the reopen query when there are no resolved conversations. --- .../dashboard/api/captain/assistant.js | 11 ++- .../captain/assistants/overview/Index.vue | 90 +++++++++++++------ config/routes.rb | 3 +- .../captain/assistant_stats_builder.rb | 45 +++++----- .../accounts/captain/assistants_controller.rb | 8 +- .../app/policies/captain/assistant_policy.rb | 6 +- .../captain/assistant_stats_builder_spec.rb | 8 +- .../policies/captain/assistant_policy_spec.rb | 2 +- 8 files changed, 112 insertions(+), 61 deletions(-) diff --git a/app/javascript/dashboard/api/captain/assistant.js b/app/javascript/dashboard/api/captain/assistant.js index 5af6110ab..806b45bb2 100644 --- a/app/javascript/dashboard/api/captain/assistant.js +++ b/app/javascript/dashboard/api/captain/assistant.js @@ -26,13 +26,20 @@ class CaptainAssistant extends ApiClient { }); } - getStats({ assistantId, range, signal }) { + getMetrics({ assistantId, range, signal }) { const requestConfig = { params: { range, timezone_offset: getTimezoneOffset() }, }; if (signal) requestConfig.signal = signal; - return axios.get(`${this.url}/${assistantId}/stats`, requestConfig); + return axios.get(`${this.url}/${assistantId}/metrics`, requestConfig); + } + + getFaqStats({ assistantId, signal }) { + const requestConfig = {}; + if (signal) requestConfig.signal = signal; + + return axios.get(`${this.url}/${assistantId}/faq_stats`, requestConfig); } getSummary({ assistantId, range, stats }) { diff --git a/app/javascript/dashboard/routes/dashboard/captain/assistants/overview/Index.vue b/app/javascript/dashboard/routes/dashboard/captain/assistants/overview/Index.vue index ba31bc05d..124b7c924 100644 --- a/app/javascript/dashboard/routes/dashboard/captain/assistants/overview/Index.vue +++ b/app/javascript/dashboard/routes/dashboard/captain/assistants/overview/Index.vue @@ -26,25 +26,28 @@ const canDrilldown = computed(() => checkPermissions(['administrator'])); const selectedRange = ref('this_month'); const assistantId = computed(() => route.params.assistantId); -const stats = ref(null); -const isFetching = ref(false); +const metricStats = ref(null); +const faqStats = ref(null); +const isFetchingMetrics = ref(false); // Increments on every fetch so a response (or retry) from a superseded // range/assistant can't clobber the latest request's state. -let fetchToken = 0; -let abortController = null; +let metricsFetchToken = 0; +let faqStatsFetchToken = 0; +let metricsAbortController = null; +let faqStatsAbortController = null; -const fetchStats = async () => { - fetchToken += 1; - const token = fetchToken; - abortController?.abort(); - abortController = new AbortController(); - const { signal } = abortController; - stats.value = null; - isFetching.value = true; +const fetchMetrics = async () => { + metricsFetchToken += 1; + const token = metricsFetchToken; + metricsAbortController?.abort(); + metricsAbortController = new AbortController(); + const { signal } = metricsAbortController; + metricStats.value = null; + isFetchingMetrics.value = true; - const requestStats = () => - CaptainAssistant.getStats({ + const requestMetrics = () => + CaptainAssistant.getMetrics({ assistantId: assistantId.value, range: selectedRange.value, signal, @@ -52,25 +55,54 @@ const fetchStats = async () => { let data = null; try { - ({ data } = await requestStats()); + ({ data } = await requestMetrics()); } catch { // One silent retry before giving up, unless the request was aborted. try { - if (token === fetchToken && !signal.aborted) - ({ data } = await requestStats()); + if (token === metricsFetchToken && !signal.aborted) + ({ data } = await requestMetrics()); } catch { data = null; } } - if (token !== fetchToken || signal.aborted) return; - stats.value = data; - isFetching.value = false; + if (token !== metricsFetchToken || signal.aborted) return; + metricStats.value = data; + isFetchingMetrics.value = false; }; -onUnmounted(() => abortController?.abort()); +const fetchFaqStats = async () => { + faqStatsFetchToken += 1; + const token = faqStatsFetchToken; + faqStatsAbortController?.abort(); + faqStatsAbortController = new AbortController(); + const { signal } = faqStatsAbortController; + faqStats.value = null; -watch([selectedRange, assistantId], fetchStats, { immediate: true }); + try { + const { data } = await CaptainAssistant.getFaqStats({ + assistantId: assistantId.value, + signal, + }); + if (token === faqStatsFetchToken && !signal.aborted) faqStats.value = data; + } catch { + if (token === faqStatsFetchToken && !signal.aborted) faqStats.value = null; + } +}; + +const summaryStats = computed(() => { + if (!metricStats.value || !faqStats.value) return null; + + return { ...metricStats.value, knowledge: faqStats.value }; +}); + +onUnmounted(() => { + metricsAbortController?.abort(); + faqStatsAbortController?.abort(); +}); + +watch([selectedRange, assistantId], fetchMetrics, { immediate: true }); +watch(assistantId, fetchFaqStats, { immediate: true }); // `direction` says whether a rising trend is good ('up'), bad ('down'), or // neutral, so we can colour the delta independently of its sign. @@ -90,7 +122,7 @@ const formatDuration = hours => hours >= 100 ? `${Math.round(hours / 24)}d` : `${hours}h`; const metricFor = (statKey, formatValue, direction, trendKind = 'percent') => { - const data = stats.value?.[statKey]; + const data = metricStats.value?.[statKey]; if (!data) return { value: '—', trend: '', trendGood: null }; const sign = data.trend > 0 ? '+' : ''; @@ -184,9 +216,9 @@ const closeDrilldown = () => {
- + - +
{ :trend="metric.trend" :hint="metric.hint" :trend-good="metric.trendGood" - :loading="isFetching" - :clickable="canDrilldown && Boolean(metric.metric) && !isFetching" + :loading="isFetchingMetrics" + :clickable=" + canDrilldown && Boolean(metric.metric) && !isFetchingMetrics + " @click="openDrilldown(metric)" />
- +
diff --git a/config/routes.rb b/config/routes.rb index 0bf6b40c8..7314dbda5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -66,7 +66,8 @@ Rails.application.routes.draw do resources :assistants do member do post :playground - get :stats + get :metrics + get :faq_stats get :summary get :drilldown end diff --git a/enterprise/app/builders/captain/assistant_stats_builder.rb b/enterprise/app/builders/captain/assistant_stats_builder.rb index d162406ad..93f51448b 100644 --- a/enterprise/app/builders/captain/assistant_stats_builder.rb +++ b/enterprise/app/builders/captain/assistant_stats_builder.rb @@ -37,6 +37,23 @@ class Captain::AssistantStatsBuilder build_metrics(current, previous) end + # Approved/pending FAQ counts and the document total in a single round trip. + def faq_stats + approved, pending, documents = Captain::AssistantResponse.by_assistant(assistant.id).reorder(nil).pick( + Arel.sql("COUNT(*) FILTER (WHERE status = #{Captain::AssistantResponse.statuses['approved']})"), + Arel.sql("COUNT(*) FILTER (WHERE status = #{Captain::AssistantResponse.statuses['pending']})"), + Arel.sql("(SELECT COUNT(*) FROM captain_documents WHERE assistant_id = #{assistant.id.to_i})") + ) + total = approved + pending + + { + approved: approved, + pending: pending, + documents: documents, + coverage: total.zero? ? 0 : (approved.to_f / total * 100).round + } + end + private attr_reader :window @@ -56,8 +73,7 @@ class Captain::AssistantStatsBuilder handoff_rate: pack(current[:handoff], previous[:handoff], :point), hours_saved: pack(current[:hours_saved], previous[:hours_saved], :percent), reopen_rate: pack(current[:reopen], previous[:reopen], :point), - conversation_depth: pack(current[:depth], previous[:depth], :absolute), - knowledge: knowledge + conversation_depth: pack(current[:depth], previous[:depth], :absolute) } end @@ -73,7 +89,7 @@ class Captain::AssistantStatsBuilder auto_resolution: rate(resolution[:resolved], handled), handoff: rate(resolution[:handoff], handled), hours_saved: (public_count * SECONDS_SAVED_PER_REPLY / 3600.0).round, - reopen: reopen_rate(range), + reopen: reopen_rate(range, resolution[:resolved]), depth: depth_conversations.zero? ? 0 : (public_count.to_f / depth_conversations).round(1) } end @@ -158,7 +174,9 @@ class Captain::AssistantStatsBuilder # derived from the assistant's handled conversations (not current inbox membership) so a later # inbox reassignment doesn't drop historical resolves, and covers both the evaluated (inference) # and time-based (bot) resolve paths so the denominator matches auto_resolution_rate. - def reopen_rate(range) + def reopen_rate(range, resolved_count) + return 0 if resolved_count.zero? + resolved_scope = account.reporting_events .where(name: RESOLVED_EVENT_NAMES, created_at: range, conversation_id: handled_scope(range).select(:conversation_id)) @@ -178,24 +196,7 @@ class Captain::AssistantStatsBuilder 'ON resolves.conversation_id = reporting_events.conversation_id ' \ 'AND reporting_events.event_end_time >= resolves.event_end_time') .distinct.count('reporting_events.conversation_id') - rate(reopened, resolved_scope.distinct.count(:conversation_id)) - end - - # Approved/pending FAQ counts and the document total in a single round trip. - def knowledge - approved, pending, documents = Captain::AssistantResponse.by_assistant(assistant.id).reorder(nil).pick( - Arel.sql("COUNT(*) FILTER (WHERE status = #{Captain::AssistantResponse.statuses['approved']})"), - Arel.sql("COUNT(*) FILTER (WHERE status = #{Captain::AssistantResponse.statuses['pending']})"), - Arel.sql("(SELECT COUNT(*) FROM captain_documents WHERE assistant_id = #{assistant.id.to_i})") - ) - total = approved + pending - - { - approved: approved, - pending: pending, - documents: documents, - coverage: total.zero? ? 0 : (approved.to_f / total * 100).round - } + rate(reopened, resolved_count) end def rate(numerator, denominator) diff --git a/enterprise/app/controllers/api/v1/accounts/captain/assistants_controller.rb b/enterprise/app/controllers/api/v1/accounts/captain/assistants_controller.rb index 1b7d77289..f1509f11c 100644 --- a/enterprise/app/controllers/api/v1/accounts/captain/assistants_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/captain/assistants_controller.rb @@ -1,7 +1,7 @@ class Api::V1::Accounts::Captain::AssistantsController < Api::V1::Accounts::BaseController before_action -> { check_authorization(Captain::Assistant) } - before_action :set_assistant, only: [:show, :update, :destroy, :playground, :stats, :summary, :drilldown] + before_action :set_assistant, only: [:show, :update, :destroy, :playground, :metrics, :faq_stats, :summary, :drilldown] def index @assistants = account_assistants.ordered @@ -42,10 +42,14 @@ class Api::V1::Accounts::Captain::AssistantsController < Api::V1::Accounts::Base @tools = assistant.available_agent_tools end - def stats + def metrics render json: Captain::AssistantStatsBuilder.new(@assistant, params[:range], params[:timezone_offset]).metrics end + def faq_stats + render json: Captain::AssistantStatsBuilder.new(@assistant).faq_stats + end + def summary window = Captain::AssistantStatsWindow.new(params[:range], params[:timezone_offset]) result = cached_or_generated_summary(window, summary_stats) diff --git a/enterprise/app/policies/captain/assistant_policy.rb b/enterprise/app/policies/captain/assistant_policy.rb index 573c0400c..fdcb2db89 100644 --- a/enterprise/app/policies/captain/assistant_policy.rb +++ b/enterprise/app/policies/captain/assistant_policy.rb @@ -7,7 +7,11 @@ class Captain::AssistantPolicy < ApplicationPolicy true end - def stats? + def metrics? + true + end + + def faq_stats? true end diff --git a/spec/enterprise/builders/captain/assistant_stats_builder_spec.rb b/spec/enterprise/builders/captain/assistant_stats_builder_spec.rb index 6575c9856..c495244c4 100644 --- a/spec/enterprise/builders/captain/assistant_stats_builder_spec.rb +++ b/spec/enterprise/builders/captain/assistant_stats_builder_spec.rb @@ -27,7 +27,7 @@ RSpec.describe Captain::AssistantStatsBuilder do expect(metrics.keys).to contain_exactly( :conversations_handled, :auto_resolution_rate, :handoff_rate, - :hours_saved, :reopen_rate, :conversation_depth, :knowledge + :hours_saved, :reopen_rate, :conversation_depth ) expect(metrics[:conversations_handled]).to include(:current, :previous, :trend) end @@ -229,7 +229,7 @@ RSpec.describe Captain::AssistantStatsBuilder do end end - describe '#metrics knowledge' do + describe '#faq_stats' do before do create_list(:captain_assistant_response, 3, assistant: assistant, account: account, status: :approved) create(:captain_assistant_response, assistant: assistant, account: account, status: :pending) @@ -237,7 +237,7 @@ RSpec.describe Captain::AssistantStatsBuilder do end it 'returns approved, pending, document counts and coverage' do - knowledge = described_class.new(assistant, '30').metrics[:knowledge] + knowledge = described_class.new(assistant).faq_stats expect(knowledge).to eq(approved: 3, pending: 1, documents: 2, coverage: 75) end @@ -245,7 +245,7 @@ RSpec.describe Captain::AssistantStatsBuilder do it 'reports zero coverage when there are no responses' do Captain::AssistantResponse.where(assistant: assistant).delete_all - knowledge = described_class.new(assistant, '30').metrics[:knowledge] + knowledge = described_class.new(assistant).faq_stats expect(knowledge[:coverage]).to eq(0) end diff --git a/spec/enterprise/policies/captain/assistant_policy_spec.rb b/spec/enterprise/policies/captain/assistant_policy_spec.rb index e04b680e4..b53d73a5e 100644 --- a/spec/enterprise/policies/captain/assistant_policy_spec.rb +++ b/spec/enterprise/policies/captain/assistant_policy_spec.rb @@ -12,7 +12,7 @@ RSpec.describe Captain::AssistantPolicy, type: :policy do let(:administrator_context) { { user: administrator, account: account, account_user: account.account_users.first } } let(:agent_context) { { user: agent, account: account, account_user: account.account_users.first } } - permissions :index?, :show?, :playground? do + permissions :index?, :show?, :playground?, :metrics?, :faq_stats? do context 'when administrator' do it { expect(assistant_policy).to permit(administrator_context, assistant) } end