test: cover captain reopen rate, range whitelist, and summary caching

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)

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