From 09455b804d436b445fe0f8a6a26406acba2e5e97 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Mon, 27 Oct 2025 14:00:31 +0530 Subject: [PATCH] feat: clean debug files --- .../concerns/activity_message_handler.rb | 57 --------- app/models/conversation.rb | 58 ---------- .../conversation/response_builder_job.rb | 109 +----------------- .../enterprise/activity_message_handler.rb | 67 +++-------- enterprise/lib/captain/tools/handoff_tool.rb | 36 +----- solution.md | 22 ---- 6 files changed, 23 insertions(+), 326 deletions(-) delete mode 100644 solution.md diff --git a/app/models/concerns/activity_message_handler.rb b/app/models/concerns/activity_message_handler.rb index e21593a78..c25aba47f 100644 --- a/app/models/concerns/activity_message_handler.rb +++ b/app/models/concerns/activity_message_handler.rb @@ -9,36 +9,12 @@ module ActivityMessageHandler private def create_activity - debug_file = "tmp/conversation-#{id}.txt" - - File.open(debug_file, 'a') do |f| - f.puts "\n========== CREATE_ACTIVITY START ==========" - f.puts "Time: #{Time.current}" - f.puts "Conversation ID: #{id}" - f.puts "Current.executed_by: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" - f.puts "Current.user: #{Current.user&.class&.name} - #{Current.user&.id}" - f.puts "saved_change_to_status?: #{saved_change_to_status?}" - f.puts "saved_change_to_priority?: #{saved_change_to_priority?}" - f.puts "saved_change_to_label_list?: #{saved_change_to_label_list?}" - f.puts "saved_change_to_sla_policy_id?: #{saved_change_to_sla_policy_id?}" - f.puts "Status: #{status}" - f.puts "Previous changes: #{previous_changes.inspect}" - end - user_name = determine_user_name - File.open(debug_file, 'a') do |f| - f.puts "\nDetermined user_name: #{user_name.inspect}" - end - handle_status_change(user_name) handle_priority_change(user_name) handle_label_change(user_name) handle_sla_policy_change(user_name) - - File.open(debug_file, 'a') do |f| - f.puts "========== CREATE_ACTIVITY END ==========\n" - end end def determine_user_name @@ -46,22 +22,9 @@ module ActivityMessageHandler end def handle_status_change(user_name) - debug_file = "tmp/conversation-#{id}.txt" - - File.open(debug_file, 'a') do |f| - f.puts "\n----- handle_status_change START -----" - f.puts "saved_change_to_status?: #{saved_change_to_status?}" - f.puts "user_name: #{user_name.inspect}" - f.puts "Current.executed_by: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" - end - return unless saved_change_to_status? status_change_activity(user_name) - - File.open(debug_file, 'a') do |f| - f.puts "----- handle_status_change END -----\n" - end end def handle_priority_change(user_name) @@ -84,32 +47,12 @@ module ActivityMessageHandler end def status_change_activity(user_name) - debug_file = "tmp/conversation-#{id}.txt" - - File.open(debug_file, 'a') do |f| - f.puts "\n----- status_change_activity START -----" - f.puts "Time: #{Time.current}" - f.puts "user_name: #{user_name.inspect}" - f.puts "Current.executed_by: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" - f.puts "Current.executed_by present?: #{Current.executed_by.present?}" - f.puts "Will call automation_status_change_activity_content?: #{Current.executed_by.present?}" - end - content = if Current.executed_by.present? - File.open(debug_file, 'a') { |f| f.puts 'Calling automation_status_change_activity_content' } automation_status_change_activity_content else - File.open(debug_file, 'a') { |f| f.puts 'Calling user_status_change_activity_content' } user_status_change_activity_content(user_name) end - File.open(debug_file, 'a') do |f| - f.puts "Content generated: #{content.inspect}" - f.puts "Will enqueue ActivityMessageJob?: #{content.present?}" - f.puts "Enqueueing ActivityMessageJob with params: #{activity_message_params(content).inspect}" if content - f.puts "----- status_change_activity END -----\n" - end - ::Conversations::ActivityMessageJob.perform_later(self, activity_message_params(content)) if content end diff --git a/app/models/conversation.rb b/app/models/conversation.rb index 06e6faeab..4ec63acc2 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -155,37 +155,8 @@ class Conversation < ApplicationRecord end def bot_handoff! - debug_file = "tmp/conversation-#{id}.txt" - - File.open(debug_file, 'a') do |f| - f.puts "\n========== BOT_HANDOFF! START ==========" - f.puts "Time: #{Time.current}" - f.puts "Conversation ID: #{id}" - f.puts "Status BEFORE open!: #{status}" - f.puts "Current.executed_by: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" - f.puts "Transaction open?: #{self.class.connection.transaction_open?}" - f.puts "Thread ID: #{Thread.current.object_id}" - end - open! - - File.open(debug_file, 'a') do |f| - f.puts "\nAfter open! called:" - f.puts "Status AFTER open!: #{status}" - f.puts "Changed?: #{changed?}" - f.puts "Changes: #{changes.inspect}" - f.puts "Previous changes: #{previous_changes.inspect}" - f.puts "Saved changes to status?: #{saved_change_to_status?}" - f.puts "Will callbacks fire?: #{saved_change_to_status? ? 'YES' : 'NO'}" - f.puts 'About to dispatch CONVERSATION_BOT_HANDOFF' - end - dispatcher_dispatch(CONVERSATION_BOT_HANDOFF) - - File.open(debug_file, 'a') do |f| - f.puts 'Dispatcher dispatch completed' - f.puts "========== BOT_HANDOFF! END ==========\n" - end end def unread_messages @@ -227,39 +198,10 @@ class Conversation < ApplicationRecord private def execute_after_update_commit_callbacks - debug_file = "tmp/conversation-#{id}.txt" - - File.open(debug_file, 'a') do |f| - f.puts "\n========== AFTER_UPDATE_COMMIT CALLBACKS START ==========" - f.puts "Time: #{Time.current}" - f.puts "Conversation ID: #{id}" - f.puts "Current.executed_by: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" - f.puts "Transaction open?: #{self.class.connection.transaction_open?}" - f.puts "Thread ID: #{Thread.current.object_id}" - f.puts "Previous changes: #{previous_changes.inspect}" - f.puts "Saved change to status?: #{saved_change_to_status?}" - f.puts "Status: #{status}" - end - handle_resolved_status_change notify_status_change - - File.open(debug_file, 'a') do |f| - f.puts "\nAbout to call create_activity" - f.puts "Will create_activity run?: #{saved_change_to_status? ? 'YES' : 'NO (no status change)'}" - end - create_activity - - File.open(debug_file, 'a') do |f| - f.puts 'create_activity completed' - end - notify_conversation_updation - - File.open(debug_file, 'a') do |f| - f.puts "========== AFTER_UPDATE_COMMIT CALLBACKS END ==========\n" - end end def handle_resolved_status_change diff --git a/enterprise/app/jobs/captain/conversation/response_builder_job.rb b/enterprise/app/jobs/captain/conversation/response_builder_job.rb index b1936221b..0ef537132 100644 --- a/enterprise/app/jobs/captain/conversation/response_builder_job.rb +++ b/enterprise/app/jobs/captain/conversation/response_builder_job.rb @@ -8,67 +8,22 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob @inbox = conversation.inbox @assistant = assistant - debug_file = "tmp/conversation-#{conversation.id}.txt" - - File.open(debug_file, 'a') do |f| - f.puts "\n\n" + ('='*100) - f.puts '='*100 - f.puts '[RESPONSE BUILDER] ========== JOB PERFORM START ==========' - f.puts "[RESPONSE BUILDER] Time: #{Time.current}" - f.puts "[RESPONSE BUILDER] Conversation ID: #{conversation.id}" - f.puts "[RESPONSE BUILDER] Assistant: #{assistant&.class&.name} - #{assistant&.id}" - f.puts "[RESPONSE BUILDER] Current.executed_by BEFORE setting: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" - end - Current.executed_by = @assistant - File.open(debug_file, 'a') do |f| - f.puts "[RESPONSE BUILDER] Current.executed_by AFTER setting: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" - f.puts '[RESPONSE BUILDER] About to start transaction' - end - ActiveRecord::Base.transaction do - File.open(debug_file, 'a') { |f| f.puts '[RESPONSE BUILDER] Inside transaction' } generate_and_process_response - File.open(debug_file, 'a') { |f| f.puts '[RESPONSE BUILDER] Transaction completed successfully' } - end - - File.open(debug_file, 'a') do |f| - f.puts '[RESPONSE BUILDER] After transaction block' - f.puts "[RESPONSE BUILDER] Checking if handoff was requested: #{@handoff_requested.inspect}" end # Process handoff OUTSIDE the transaction so after_commit callbacks work properly - if @handoff_requested - File.open(debug_file, 'a') do |f| - f.puts '[RESPONSE BUILDER] Processing deferred handoff OUTSIDE transaction' - f.puts "[RESPONSE BUILDER] Transaction open?: #{ActiveRecord::Base.connection.transaction_open?}" - end - process_handoff - end + # This ensures Rails' dirty tracking (previous_changes, saved_change_to_*?) works correctly + process_handoff if @handoff_requested rescue StandardError => e - File.open(debug_file, 'a') do |f| - f.puts "[RESPONSE BUILDER] ERROR occurred: #{e.class} - #{e.message}" - f.puts "[RESPONSE BUILDER] Backtrace (first 5): #{e.backtrace.first(5).join("\n")}" - end raise e if e.is_a?(ActiveStorage::FileNotFoundError) || e.is_a?(Faraday::BadRequestError) handle_error(e) ensure - File.open(debug_file, 'a') do |f| - f.puts '[RESPONSE BUILDER] In ensure block' - f.puts "[RESPONSE BUILDER] Current.executed_by BEFORE clearing: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" - f.puts "[RESPONSE BUILDER] Current.handoff_requested BEFORE clearing: #{Current.handoff_requested.inspect}" - end Current.executed_by = nil Current.handoff_requested = nil - File.open(debug_file, 'a') do |f| - f.puts "[RESPONSE BUILDER] Current.executed_by AFTER clearing: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" - f.puts "[RESPONSE BUILDER] Current.handoff_requested AFTER clearing: #{Current.handoff_requested.inspect}" - f.puts '[RESPONSE BUILDER] ========== JOB PERFORM END ==========' - f.puts '='*100 - f.puts ('='*100) + "\n\n" - end end private @@ -76,37 +31,17 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob delegate :account, :inbox, to: :@conversation def generate_and_process_response - debug_file = "tmp/conversation-#{@conversation.id}.txt" - - File.open(debug_file, 'a') do |f| - f.puts "\n[RESPONSE BUILDER] generate_and_process_response START" - f.puts "[RESPONSE BUILDER] captain_v2_enabled?: #{captain_v2_enabled?}" - end - @response = if captain_v2_enabled? - File.open(debug_file, 'a') { |f| f.puts '[RESPONSE BUILDER] Using AgentRunnerService (V2)' } Captain::Assistant::AgentRunnerService.new(assistant: @assistant, conversation: @conversation).generate_response( message_history: collect_previous_messages ) else - File.open(debug_file, 'a') { |f| f.puts '[RESPONSE BUILDER] Using AssistantChatService (V1)' } Captain::Llm::AssistantChatService.new(assistant: @assistant).generate_response( message_history: collect_previous_messages ) end - File.open(debug_file, 'a') do |f| - f.puts "[RESPONSE BUILDER] Response received: #{@response.inspect}" - f.puts "[RESPONSE BUILDER] handoff_requested? (from response): #{handoff_requested?}" - f.puts "[RESPONSE BUILDER] Current.handoff_requested (from tool): #{Current.handoff_requested.inspect}" - f.puts "[RESPONSE BUILDER] Will process handoff?: #{handoff_requested? || Current.handoff_requested}" - end - if handoff_requested? || Current.handoff_requested - File.open(debug_file, 'a') do |f| - f.puts '[RESPONSE BUILDER] Handoff requested - deferring until AFTER transaction commits' - f.puts '[RESPONSE BUILDER] Setting @handoff_requested = true' - end @handoff_requested = true create_handoff_message return @@ -115,8 +50,6 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob create_messages Rails.logger.info("[CAPTAIN][ResponseBuilderJob] Incrementing response usage for #{account.id}") account.increment_response_usage - - File.open(debug_file, 'a') { |f| f.puts '[RESPONSE BUILDER] generate_and_process_response END' } end def collect_previous_messages @@ -150,30 +83,9 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob end def process_handoff - debug_file = "tmp/conversation-#{@conversation.id}.txt" - - File.open(debug_file, 'a') do |f| - f.puts "\n[RESPONSE BUILDER] process_handoff START (OUTSIDE TRANSACTION)" - f.puts "[RESPONSE BUILDER] Current.executed_by: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" - f.puts "[RESPONSE BUILDER] Current.handoff_requested: #{Current.handoff_requested.inspect}" - f.puts "[RESPONSE BUILDER] Triggered by: #{Current.handoff_requested ? 'HandoffTool (Current flag)' : 'LLM response'}" - f.puts "[RESPONSE BUILDER] Transaction open?: #{ActiveRecord::Base.connection.transaction_open?}" - end - I18n.with_locale(@assistant.account.locale) do - File.open(debug_file, 'a') do |f| - f.puts "[RESPONSE BUILDER] Locale set to: #{@assistant.account.locale}" - f.puts "[RESPONSE BUILDER] Conversation status BEFORE bot_handoff!: #{@conversation.reload.status}" - end - File.open(debug_file, 'a') { |f| f.puts '[RESPONSE BUILDER] Calling bot_handoff!' } @conversation.bot_handoff! - File.open(debug_file, 'a') do |f| - f.puts '[RESPONSE BUILDER] bot_handoff! completed' - f.puts "[RESPONSE BUILDER] Conversation status AFTER bot_handoff!: #{@conversation.reload.status}" - end end - - File.open(debug_file, 'a') { |f| f.puts '[RESPONSE BUILDER] process_handoff END' } end def create_handoff_message @@ -206,24 +118,9 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob end def handle_error(error) - debug_file = "tmp/conversation-#{@conversation.id}.txt" - - File.open(debug_file, 'a') do |f| - f.puts "\n[RESPONSE BUILDER] handle_error called" - f.puts "[RESPONSE BUILDER] Error: #{error.class} - #{error.message}" - f.puts '[RESPONSE BUILDER] Will handoff due to error' - end - log_error(error) - - # Create handoff message and set flag so handoff happens outside transaction @handoff_requested = true - create_handoff_message if @conversation.messages.where(content: @assistant.config['handoff_message'].presence || I18n.t('conversations.captain.handoff')).where( - 'created_at > ?', 1.minute.ago - ).empty? - - File.open(debug_file, 'a') { |f| f.puts '[RESPONSE BUILDER] @handoff_requested set to true due to error' } - + create_handoff_message true end diff --git a/enterprise/app/models/enterprise/activity_message_handler.rb b/enterprise/app/models/enterprise/activity_message_handler.rb index 0b750aa93..e6a93718f 100644 --- a/enterprise/app/models/enterprise/activity_message_handler.rb +++ b/enterprise/app/models/enterprise/activity_message_handler.rb @@ -1,55 +1,22 @@ module Enterprise::ActivityMessageHandler def automation_status_change_activity_content - debug_file = "tmp/conversation-#{id}.txt" - - File.open(debug_file, "a") do |f| - f.puts "\n----- Enterprise automation_status_change_activity_content START -----" - f.puts "Time: #{Time.current}" - f.puts "Current.executed_by: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" - f.puts "Is Captain::Assistant?: #{Current.executed_by.instance_of?(Captain::Assistant)}" - f.puts "Status: #{status}" - f.puts "resolved?: #{resolved?}" - f.puts "open?: #{open?}" + if Current.executed_by.instance_of?(Captain::Assistant) + locale = Current.executed_by.account.locale + if resolved? + I18n.t( + 'conversations.activity.captain.resolved', + user_name: Current.executed_by.name, + locale: locale + ) + elsif open? + I18n.t( + 'conversations.activity.captain.open', + user_name: Current.executed_by.name, + locale: locale + ) + end + else + super end - - result = if Current.executed_by.instance_of?(Captain::Assistant) - locale = Current.executed_by.account.locale - File.open(debug_file, "a") do |f| - f.puts "Captain::Assistant detected!" - f.puts "Locale: #{locale}" - f.puts "Assistant name: #{Current.executed_by.name}" - end - - if resolved? - content = I18n.t( - 'conversations.activity.captain.resolved', - user_name: Current.executed_by.name, - locale: locale - ) - File.open(debug_file, "a") { |f| f.puts "Generated resolved message: #{content}" } - content - elsif open? - content = I18n.t( - 'conversations.activity.captain.open', - user_name: Current.executed_by.name, - locale: locale - ) - File.open(debug_file, "a") { |f| f.puts "Generated open message: #{content}" } - content - else - File.open(debug_file, "a") { |f| f.puts "Neither resolved nor open - returning nil" } - nil - end - else - File.open(debug_file, "a") { |f| f.puts "Not Captain::Assistant, calling super" } - super - end - - File.open(debug_file, "a") do |f| - f.puts "Final result: #{result.inspect}" - f.puts "----- Enterprise automation_status_change_activity_content END -----\n" - end - - result end end diff --git a/enterprise/lib/captain/tools/handoff_tool.rb b/enterprise/lib/captain/tools/handoff_tool.rb index 6d9191703..cba9ce54f 100644 --- a/enterprise/lib/captain/tools/handoff_tool.rb +++ b/enterprise/lib/captain/tools/handoff_tool.rb @@ -23,41 +23,11 @@ class Captain::Tools::HandoffTool < Captain::Tools::BasePublicTool private - def trigger_handoff(conversation, reason) - debug_file = "tmp/conversation-#{conversation.id}.txt" - - File.open(debug_file, "a") do |f| - f.puts "\n\n" + "-"*80 - f.puts "========== HANDOFF TOOL TRIGGER START ==========" - f.puts "Time: #{Time.current}" - f.puts "Conversation ID: #{conversation.id}" - f.puts "Conversation Status BEFORE: #{conversation.status}" - f.puts "Conversation persisted?: #{conversation.persisted?}" - f.puts "Current.executed_by BEFORE: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" - f.puts "Current.executed_by object_id: #{Current.executed_by&.object_id}" - f.puts "Current.handoff_requested BEFORE: #{Current.handoff_requested.inspect}" - f.puts "Reason: #{reason}" - f.puts "Assistant: #{@assistant&.class&.name} - #{@assistant&.id}" - f.puts "ActiveRecord transaction open?: #{conversation.class.connection.transaction_open?}" - f.puts "Thread ID: #{Thread.current.object_id}" - f.puts "Caller stack (first 10):" - caller.first(10).each { |line| f.puts " #{line}" } - end - - File.open(debug_file, "a") do |f| - f.puts "\nSetting Current.handoff_requested = true" - f.puts "NOT calling bot_handoff! - letting ResponseBuilderJob handle it" - end - + def trigger_handoff(_conversation, _reason) # Signal to ResponseBuilderJob that handoff is requested + # The job will handle the actual bot_handoff! call outside the transaction + # to ensure after_commit callbacks work correctly (see PR.md for details) Current.handoff_requested = true - - File.open(debug_file, "a") do |f| - f.puts "Current.handoff_requested set to: #{Current.handoff_requested.inspect}" - f.puts "Current.executed_by AFTER: #{Current.executed_by&.class&.name} - #{Current.executed_by&.id}" - f.puts "Conversation Status (unchanged): #{conversation.status}" - f.puts "========== HANDOFF TOOL TRIGGER END ==========\n\n" - end end # TODO: Future enhancement - Add team assignment capability diff --git a/solution.md b/solution.md deleted file mode 100644 index 1a8b9856c..000000000 --- a/solution.md +++ /dev/null @@ -1,22 +0,0 @@ -# Captain Handoff Activity Message – Proposed Fix - -When the Captain assistant triggers a handoff, we need two things to happen reliably: -1. Persist the conversation status change from `pending` to `open`. -2. Attribute that state change to the assistant so the existing activity callback emits the “Captain opened” message. - -The clean approach is to give `Conversation` a helper (e.g., `handoff_by(executor)`) that wraps the status update in a `with_executed_by(executor)` block: - -```ruby -def handoff_by(executor) - with_executed_by(executor) do - open! unless open? - dispatcher_dispatch(CONVERSATION_BOT_HANDOFF) - end -end -``` - -Within that helper, `with_executed_by` should set `Current.executed_by` just for the duration of the update and then restore the previous value. Both the V1 handoff path and the Captain tool call this helper, ensuring the status change is tracked and the enterprise override sees the assistant. - -## Why Not Store `Current.handoff_requested?` - -Using a thread-local flag would mean the tool sets `Current.handoff_requested = true` and hopes the job checks it later. That requires carefully clearing the flag on every execution path, and it leaves state lingering on `Current` while unrelated callbacks run. It’s easy to forget the cleanup and leak the flag into subsequent jobs or observers. By contrast, a push/pop around the actual update keeps responsibility localized and avoids coordination hazards.