From da8ce630dbb8d3a1af2a72abe430b0aca0fe2cec Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Wed, 1 Jul 2026 15:20:28 +0530 Subject: [PATCH] test: cover captain reopen rate, range whitelist, and summary caching --- .../captain/assistant_stats_builder_spec.rb | 79 +++++++++++++++++++ .../captain/assistants_controller_spec.rb | 42 ++++++++++ 2 files changed, 121 insertions(+) diff --git a/spec/enterprise/builders/captain/assistant_stats_builder_spec.rb b/spec/enterprise/builders/captain/assistant_stats_builder_spec.rb index 35052be89..cbbcdc0d7 100644 --- a/spec/enterprise/builders/captain/assistant_stats_builder_spec.rb +++ b/spec/enterprise/builders/captain/assistant_stats_builder_spec.rb @@ -52,6 +52,15 @@ RSpec.describe Captain::AssistantStatsBuilder do expect(metrics[:handoff_rate][:current]).to eq(50.0) end + it 'excludes resolution events that fall outside the current window' do + create(:reporting_event, account: account, conversation: current_convo_a, + name: 'conversation_captain_inference_resolved', created_at: 60.days.ago) + + metrics = described_class.new(assistant, '30').metrics + + expect(metrics[:auto_resolution_rate][:current]).to eq(0.0) + end + it 'computes conversation depth as public replies per handled conversation' do depth = described_class.new(assistant, '30').metrics[:conversation_depth] @@ -69,6 +78,76 @@ RSpec.describe Captain::AssistantStatsBuilder do end end + describe 'range handling' do + it 'accepts the allowed day and named ranges' do + %w[7 30 90 this_month last_month].each do |allowed| + expect(described_class.new(assistant, allowed).range).to eq(allowed) + end + end + + it 'falls back to the default range for values outside the allowed set' do + expect(described_class.new(assistant, '365000').range).to eq('30') + expect(described_class.new(assistant, 'bogus').range).to eq('30') + expect(described_class.new(assistant, nil).range).to eq('30') + end + end + + describe '#metrics reopen_rate' do + # A conversation the assistant handled (messaged) inside the current 30-day window. + let(:conversation) { create(:conversation, account: account, inbox: inbox) } + + before do + create(:message, account: account, inbox: inbox, conversation: conversation, + sender: assistant, message_type: :outgoing, private: false, created_at: 8.days.ago) + end + + it 'counts a reopen that happened after the captain resolve' do + create(:reporting_event, account: account, inbox: inbox, conversation: conversation, + name: 'conversation_bot_resolved', event_start_time: 6.days.ago, event_end_time: 6.days.ago) + create(:reporting_event, account: account, inbox: inbox, conversation: conversation, + name: 'conversation_opened', value: 120, event_start_time: 6.days.ago, event_end_time: 4.days.ago) + + expect(described_class.new(assistant, '30').metrics[:reopen_rate][:current]).to eq(100.0) + end + + it 'ignores a human resolve/reopen that happened before the captain resolve' do + # Earlier resolve/reopen cycle, then Captain resolves later in the same window. + create(:reporting_event, account: account, inbox: inbox, conversation: conversation, + name: 'conversation_opened', value: 120, event_start_time: 20.days.ago, event_end_time: 18.days.ago) + create(:reporting_event, account: account, inbox: inbox, conversation: conversation, + name: 'conversation_bot_resolved', event_start_time: 5.days.ago, event_end_time: 5.days.ago) + + expect(described_class.new(assistant, '30').metrics[:reopen_rate][:current]).to eq(0.0) + end + + it 'counts both inference and time-based bot resolves in the denominator' do + # conversation: inference-resolved and reopened + create(:reporting_event, account: account, inbox: inbox, conversation: conversation, + name: 'conversation_captain_inference_resolved', event_start_time: 6.days.ago, event_end_time: 6.days.ago) + create(:reporting_event, account: account, inbox: inbox, conversation: conversation, + name: 'conversation_opened', value: 120, event_start_time: 6.days.ago, event_end_time: 4.days.ago) + # other: time-based bot-resolved, never reopened + other = create(:conversation, account: account, inbox: inbox) + create(:message, account: account, inbox: inbox, conversation: other, + sender: assistant, message_type: :outgoing, private: false, created_at: 8.days.ago) + create(:reporting_event, account: account, inbox: inbox, conversation: other, + name: 'conversation_bot_resolved', event_start_time: 6.days.ago, event_end_time: 6.days.ago) + + expect(described_class.new(assistant, '30').metrics[:reopen_rate][:current]).to eq(50.0) + end + + it 'derives the cohort from handled conversations, not current inbox membership' do + create(:reporting_event, account: account, inbox: inbox, conversation: conversation, + name: 'conversation_captain_inference_resolved', event_start_time: 6.days.ago, event_end_time: 6.days.ago) + create(:reporting_event, account: account, inbox: inbox, conversation: conversation, + name: 'conversation_opened', value: 120, event_start_time: 6.days.ago, event_end_time: 4.days.ago) + # The assistant is later removed from the inbox; the cohort must still resolve via handled messages. + CaptainInbox.where(captain_assistant: assistant).delete_all + + expect(described_class.new(assistant, '30').metrics[:reopen_rate][:current]).to eq(100.0) + end + end + describe '#metrics knowledge' do before do create_list(:captain_assistant_response, 3, assistant: assistant, account: account, status: :approved) diff --git a/spec/enterprise/controllers/api/v1/accounts/captain/assistants_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/captain/assistants_controller_spec.rb index 4689defaf..afb6aa2de 100644 --- a/spec/enterprise/controllers/api/v1/accounts/captain/assistants_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/captain/assistants_controller_spec.rb @@ -252,6 +252,48 @@ RSpec.describe 'Api::V1::Accounts::Captain::Assistants', type: :request do end end + describe 'GET /api/v1/accounts/{account.id}/captain/assistants/{id}/summary' do + let(:assistant) { create(:captain_assistant, account: account) } + let(:alice) { create(:user, account: account, role: :administrator, name: 'Alice Adams') } + let(:bob) { create(:user, account: account, role: :administrator, name: 'Bob Brown') } + let(:summary_service) { instance_double(Captain::OverviewSummaryService) } + + def get_summary(user) + get "/api/v1/accounts/#{account.id}/captain/assistants/#{assistant.id}/summary", + params: { range: '30' }, + headers: user.create_new_auth_token, + as: :json + end + + before do + # Test env uses a null store; swap in a real store so caching behaviour is observable. + allow(Rails).to receive(:cache).and_return(ActiveSupport::Cache::MemoryStore.new) + allow(Captain::OverviewSummaryService).to receive(:new).and_return(summary_service) + end + + it 'caches the summary per viewer so one user never receives another user\'s greeting' do + allow(summary_service).to receive(:perform).and_return({ message: 'Hi Alice' }) + + get_summary(alice) + get_summary(alice) # served from Alice's cache, no regeneration + get_summary(bob) # distinct cache key, regenerated for Bob + + expect(response).to have_http_status(:success) + expect(Captain::OverviewSummaryService).to have_received(:new).twice + end + + it 'does not cache failures so a transient error is retried' do + allow(summary_service).to receive(:perform).and_return({ error: 'LLM unavailable' }) + + get_summary(alice) + get_summary(alice) + + expect(response).to have_http_status(:unprocessable_content) + expect(json_response[:error]).to eq('LLM unavailable') + expect(Captain::OverviewSummaryService).to have_received(:new).twice + end + end + describe 'POST /api/v1/accounts/{account.id}/captain/assistants/{id}/playground' do let(:assistant) { create(:captain_assistant, account: account) } let(:valid_params) do