From a0c0139f518032d464d2e79ba0d529cc7a01e175 Mon Sep 17 00:00:00 2001 From: Keegan George Date: Mon, 9 Jun 2025 09:58:52 -0700 Subject: [PATCH] DEV: Embeddings, tools, personas --- .../admin/ai_embeddings_controller.rb | 83 ++++ .../discourse_ai/admin/ai_llms_controller.rb | 132 +++--- .../admin/ai_personas_controller.rb | 103 ++++- .../discourse_ai/admin/ai_tools_controller.rb | 78 ++++ config/locales/client.en.yml | 6 + lib/utils/ai_staff_action_logger.rb | 173 ++++--- spec/lib/utils/ai_staff_action_logger_spec.rb | 437 ++++++++++++++++++ .../admin/ai_embeddings_controller_spec.rb | 38 ++ .../requests/admin/ai_llms_controller_spec.rb | 61 +++ .../admin/ai_personas_controller_spec.rb | 75 +++ .../admin/ai_tools_controller_spec.rb | 41 ++ 11 files changed, 1078 insertions(+), 149 deletions(-) create mode 100644 spec/lib/utils/ai_staff_action_logger_spec.rb diff --git a/app/controllers/discourse_ai/admin/ai_embeddings_controller.rb b/app/controllers/discourse_ai/admin/ai_embeddings_controller.rb index 66a9c7f3..7aaaae73 100644 --- a/app/controllers/discourse_ai/admin/ai_embeddings_controller.rb +++ b/app/controllers/discourse_ai/admin/ai_embeddings_controller.rb @@ -40,6 +40,7 @@ module DiscourseAi embedding_def = EmbeddingDefinition.new(ai_embeddings_params) if embedding_def.save + log_ai_embedding_creation(embedding_def) render json: AiEmbeddingDefinitionSerializer.new(embedding_def), status: :created else render_json_error embedding_def @@ -55,7 +56,11 @@ module DiscourseAi ) end + # Capture initial state for logging + initial_attributes = embedding_def.attributes.dup + if embedding_def.update(ai_embeddings_params.except(:dimensions)) + log_ai_embedding_update(embedding_def, initial_attributes) render json: AiEmbeddingDefinitionSerializer.new(embedding_def) else render_json_error embedding_def @@ -75,7 +80,16 @@ module DiscourseAi return render_json_error(I18n.t("discourse_ai.embeddings.delete_failed"), status: 409) end + # Capture embedding details for logging before destruction + embedding_details = { + embedding_id: embedding_def.id, + display_name: embedding_def.display_name, + provider: embedding_def.provider, + dimensions: embedding_def.dimensions + } + if embedding_def.destroy + log_ai_embedding_deletion(embedding_details) head :no_content else render_json_error embedding_def @@ -128,6 +142,75 @@ module DiscourseAi permitted end + + def log_ai_embedding_creation(embedding_def) + # Create log details + log_details = { + embedding_id: embedding_def.id, + display_name: embedding_def.display_name, + provider: embedding_def.provider, + dimensions: embedding_def.dimensions + } + + # Only include tokenizer if present + if embedding_def.tokenizer_class.present? + log_details[:tokenizer] = embedding_def.tokenizer_class + end + + # For sensitive fields, don't include the actual content + if embedding_def.api_key.present? + log_details[:api_key_set] = true + end + + # Log the action + StaffActionLogger.new(current_user).log_custom("create_ai_embedding", log_details) + end + + def log_ai_embedding_update(embedding_def, initial_attributes) + # Create log details + log_details = { + embedding_id: embedding_def.id, + display_name: embedding_def.display_name + } + + # Track changes in fields + changed_fields = [] + + # Fields to check for changes + %w[display_name provider url tokenizer_class max_sequence_length embed_prompt search_prompt matryoshka_dimensions].each do |field| + if initial_attributes[field] != embedding_def.attributes[field] + changed_fields << field + log_details["#{field}_changed"] = true + end + end + + # Special handling for API key (sensitive) + if initial_attributes['api_key'].present? != embedding_def.api_key.present? + changed_fields << 'api_key' + + if embedding_def.api_key.present? + log_details[:api_key_set] = true + else + log_details[:api_key_removed] = true + end + end + + # Special handling for provider_params (JSON) + if initial_attributes['provider_params'].to_s != embedding_def.provider_params.to_s + changed_fields << 'provider_params' + log_details[:provider_params_changed] = true + end + + # Only log if there are actual changes + if changed_fields.any? + log_details[:changed_fields] = changed_fields + StaffActionLogger.new(current_user).log_custom("update_ai_embedding", log_details) + end + end + + def log_ai_embedding_deletion(embedding_details) + StaffActionLogger.new(current_user).log_custom("delete_ai_embedding", embedding_details) + end end end end diff --git a/app/controllers/discourse_ai/admin/ai_llms_controller.rb b/app/controllers/discourse_ai/admin/ai_llms_controller.rb index a6e12813..8c73685e 100644 --- a/app/controllers/discourse_ai/admin/ai_llms_controller.rb +++ b/app/controllers/discourse_ai/admin/ai_llms_controller.rb @@ -207,81 +207,95 @@ module DiscourseAi end def log_llm_model_creation(llm_model) - log_details = { - model_id: llm_model.id, - display_name: llm_model.display_name, - name: llm_model.name, - provider: llm_model.provider, - tokenizer: llm_model.tokenizer, - enabled_chat_bot: llm_model.enabled_chat_bot, - vision_enabled: llm_model.vision_enabled, + # Create field configuration with appropriate types + field_config = { + display_name: {}, + name: {}, + provider: {}, + tokenizer: {}, + url: {}, + api_key: { type: :sensitive }, + max_prompt_tokens: {}, + max_output_tokens: {}, + enabled_chat_bot: {}, + vision_enabled: {}, + input_cost: {}, + output_cost: {}, + cached_input_cost: {}, + provider_params: {} } - - # Add cost details if present - if llm_model.input_cost.present? - log_details[:input_cost] = llm_model.input_cost - end - if llm_model.output_cost.present? - log_details[:output_cost] = llm_model.output_cost - end - if llm_model.cached_input_cost.present? - log_details[:cached_input_cost] = llm_model.cached_input_cost - end - - # Add quota information if present + + # Create basic entity details + entity_details = { + model_id: llm_model.id, + model_name: llm_model.name, + display_name: llm_model.display_name + } + + # Create logger instance + logger = DiscourseAi::Utils::AiStaffActionLogger.new(current_user) + + # Extract attributes based on field configuration + log_details = entity_details.dup + log_details.merge!(logger.send(:extract_entity_attributes, llm_model, field_config)) + + # Add quota information as a special case if llm_model.llm_quotas.any? log_details[:quotas] = llm_model.llm_quotas.map do |quota| "Group #{quota.group_id}: #{quota.max_tokens} tokens, #{quota.max_usages} usages, #{quota.duration_seconds}s" end.join("; ") end - - logger = DiscourseAi::Utils::AiStaffActionLogger.new(current_user) + logger.log_custom("create_ai_llm_model", log_details) end def log_llm_model_update(llm_model, initial_attributes, initial_quotas) - current_attributes = llm_model.attributes - current_quotas = llm_model.llm_quotas.reload.map(&:attributes) - - # Track changes to main attributes + # Create field configuration with appropriate types + field_config = { + display_name: {}, + name: {}, + provider: {}, + tokenizer: {}, + url: {}, + api_key: { type: :sensitive }, + max_prompt_tokens: {}, + max_output_tokens: {}, + enabled_chat_bot: {}, + vision_enabled: {}, + input_cost: {}, + output_cost: {}, + cached_input_cost: {}, + provider_params: {}, + json_fields: %w[provider_params] + } + + # Create basic entity details + entity_details = { + model_id: llm_model.id, + model_name: llm_model.name, + display_name: llm_model.display_name + } + + # Create logger instance + logger = DiscourseAi::Utils::AiStaffActionLogger.new(current_user) + + # Create a changes hash to track all changes changes = {} - trackable_fields = %w[ - display_name name provider tokenizer url max_prompt_tokens max_output_tokens - enabled_chat_bot vision_enabled input_cost output_cost cached_input_cost - provider_params - ] - - trackable_fields.each do |field| - initial_value = initial_attributes[field] - current_value = current_attributes[field] - - if initial_value != current_value - # Handle API key specially - don't log the actual values for security - if field == "api_key" - changes[field] = initial_value.present? && current_value.present? ? - "updated" : (current_value.present? ? "set" : "removed") - else - changes[field] = "#{initial_value} → #{current_value}" - end - end - end - - # Track quota changes + current_quotas = llm_model.llm_quotas.reload.map(&:attributes) + + # Track quota changes separately as they're a special case if initial_quotas != current_quotas initial_quota_summary = initial_quotas.map { |q| "Group #{q['group_id']}: #{q['max_tokens']} tokens" }.join("; ") current_quota_summary = current_quotas.map { |q| "Group #{q['group_id']}: #{q['max_tokens']} tokens" }.join("; ") changes[:quotas] = "#{initial_quota_summary} → #{current_quota_summary}" end - - # Only log if there are actual changes - if changes.any? - log_details = { - model_id: llm_model.id, - model_name: llm_model.display_name || llm_model.name, - }.merge(changes) - - logger = DiscourseAi::Utils::AiStaffActionLogger.new(current_user) - logger.log_custom("update_ai_llm_model", log_details) + + # Let the logger handle standard field changes + logger.log_update("llm_model", llm_model, initial_attributes, field_config, entity_details) + + # If we have quota changes but no other changes were detected, log them separately + if changes.key?(:quotas) + logger.log_custom("update_ai_llm_model", entity_details.merge(changes)) end end diff --git a/app/controllers/discourse_ai/admin/ai_personas_controller.rb b/app/controllers/discourse_ai/admin/ai_personas_controller.rb index 7bc65585..52701bbc 100644 --- a/app/controllers/discourse_ai/admin/ai_personas_controller.rb +++ b/app/controllers/discourse_ai/admin/ai_personas_controller.rb @@ -276,40 +276,93 @@ module DiscourseAi end def log_ai_persona_creation(ai_persona) - # Extract standard attributes - log_details = { - persona_id: ai_persona.id, - name: ai_persona.name, - description: ai_persona.description, - enabled: ai_persona.enabled, - priority: ai_persona.priority, - system_prompt: ai_persona.system_prompt&.truncate(100), - default_llm_id: ai_persona.default_llm_id, - temperature: ai_persona.temperature, - top_p: ai_persona.top_p, - user_id: ai_persona.user_id, - vision_enabled: ai_persona.vision_enabled, - tools_count: (ai_persona.tools || []).size, - allowed_group_ids: ai_persona.allowed_group_ids + # Create field configuration with appropriate types + field_config = { + name: {}, + description: {}, + enabled: {}, + priority: {}, + system_prompt: { type: :large_text }, + default_llm_id: {}, + temperature: {}, + top_p: {}, + user_id: {}, + vision_enabled: {}, + vision_max_pixels: {}, + max_context_posts: {}, + rag_chunk_tokens: {}, + rag_chunk_overlap_tokens: {}, + rag_conversation_chunks: {}, + rag_llm_model_id: {}, + question_consolidator_llm_id: {}, + tool_details: {}, + forced_tool_count: {}, + allow_chat_channel_mentions: {}, + allow_chat_direct_messages: {}, + allow_topic_mentions: {}, + allow_personal_messages: {} } + # Create basic entity details + entity_details = { + persona_id: ai_persona.id, + persona_name: ai_persona.name + } + + # Create logger instance logger = DiscourseAi::Utils::AiStaffActionLogger.new(current_user) + + # Extract attributes based on field configuration + log_details = entity_details.dup + log_details.merge!(logger.send(:extract_entity_attributes, ai_persona, field_config)) + + # Add tools count separately as it's not a direct attribute + log_details[:tools_count] = (ai_persona.tools || []).size + + # Add allowed_group_ids + log_details[:allowed_group_ids] = ai_persona.allowed_group_ids if ai_persona.allowed_group_ids.present? + logger.log_custom("create_ai_persona", log_details) end def log_ai_persona_update(ai_persona, initial_attributes) - trackable_fields = %w[ - name description enabled system_prompt priority temperature top_p default_llm_id - user_id max_context_posts vision_enabled vision_max_pixels rag_chunk_tokens - rag_chunk_overlap_tokens rag_conversation_chunks rag_llm_model_id - question_consolidator_llm_id tool_details forced_tool_count - allow_chat_channel_mentions allow_chat_direct_messages allow_topic_mentions allow_personal_messages - ] - - json_fields = %w[allowed_group_ids tools response_format examples] + # Create field configuration with appropriate types + field_config = { + name: {}, + description: {}, + enabled: {}, + priority: {}, + system_prompt: { type: :large_text }, + default_llm_id: {}, + temperature: {}, + top_p: {}, + user_id: {}, + vision_enabled: {}, + vision_max_pixels: {}, + max_context_posts: {}, + rag_chunk_tokens: {}, + rag_chunk_overlap_tokens: {}, + rag_conversation_chunks: {}, + rag_llm_model_id: {}, + question_consolidator_llm_id: {}, + tool_details: {}, + forced_tool_count: {}, + allow_chat_channel_mentions: {}, + allow_chat_direct_messages: {}, + allow_topic_mentions: {}, + allow_personal_messages: {}, + json_fields: %w[allowed_group_ids tools response_format examples] + } + # Create basic entity details + entity_details = { + persona_id: ai_persona.id, + persona_name: ai_persona.name + } + + # Create logger instance and log the update logger = DiscourseAi::Utils::AiStaffActionLogger.new(current_user) - logger.log_update("persona", ai_persona, initial_attributes, trackable_fields, json_fields) + logger.log_update("persona", ai_persona, initial_attributes, field_config, entity_details) end def log_ai_persona_deletion(persona_details) diff --git a/app/controllers/discourse_ai/admin/ai_tools_controller.rb b/app/controllers/discourse_ai/admin/ai_tools_controller.rb index 6c148f1a..95a52b10 100644 --- a/app/controllers/discourse_ai/admin/ai_tools_controller.rb +++ b/app/controllers/discourse_ai/admin/ai_tools_controller.rb @@ -25,6 +25,7 @@ module DiscourseAi if ai_tool.save RagDocumentFragment.link_target_and_uploads(ai_tool, attached_upload_ids) + log_ai_tool_creation(ai_tool) render_serialized(ai_tool, AiCustomToolSerializer, status: :created) else render_json_error ai_tool @@ -32,8 +33,12 @@ module DiscourseAi end def update + # Capture initial state for logging + initial_attributes = @ai_tool.attributes.dup + if @ai_tool.update(ai_tool_params) RagDocumentFragment.update_target_uploads(@ai_tool, attached_upload_ids) + log_ai_tool_update(@ai_tool, initial_attributes) render_serialized(@ai_tool, AiCustomToolSerializer) else render_json_error @ai_tool @@ -41,7 +46,15 @@ module DiscourseAi end def destroy + # Capture tool details for logging before destruction + tool_details = { + tool_id: @ai_tool.id, + name: @ai_tool.name, + tool_name: @ai_tool.tool_name + } + if @ai_tool.destroy + log_ai_tool_deletion(tool_details) head :no_content else render_json_error @ai_tool @@ -96,6 +109,71 @@ module DiscourseAi ) .except(:rag_uploads) end + + def log_ai_tool_creation(ai_tool) + # Create log details + log_details = { + tool_id: ai_tool.id, + name: ai_tool.name, + tool_name: ai_tool.tool_name, + description: ai_tool.description + } + + # Add parameter count if available + if ai_tool.parameters.present? + log_details[:parameter_count] = ai_tool.parameters.size + end + + # For sensitive/large fields, don't include the full content + if ai_tool.script.present? + log_details[:script_size] = ai_tool.script.size + end + + # Log the action + StaffActionLogger.new(current_user).log_custom("create_ai_tool", log_details) + end + + def log_ai_tool_update(ai_tool, initial_attributes) + # Create log details + log_details = { + tool_id: ai_tool.id, + name: ai_tool.name, + tool_name: ai_tool.tool_name + } + + # Track changes in fields + changed_fields = [] + + # Check for changes in basic fields + %w[name tool_name description summary enabled].each do |field| + if initial_attributes[field] != ai_tool.attributes[field] + changed_fields << field + log_details["#{field}_changed"] = true + end + end + + # Special handling for script (sensitive/large) + if initial_attributes['script'] != ai_tool.script + changed_fields << 'script' + log_details[:script_changed] = true + end + + # Special handling for parameters (JSON) + if initial_attributes['parameters'].to_s != ai_tool.parameters.to_s + changed_fields << 'parameters' + log_details[:parameters_changed] = true + end + + # Only log if there are actual changes + if changed_fields.any? + log_details[:changed_fields] = changed_fields + StaffActionLogger.new(current_user).log_custom("update_ai_tool", log_details) + end + end + + def log_ai_tool_deletion(tool_details) + StaffActionLogger.new(current_user).log_custom("delete_ai_tool", tool_details) + end end end end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 1d16aa90..616d44cc 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -33,6 +33,12 @@ en: create_ai_persona: "Create AI persona" update_ai_persona: "Update AI persona" delete_ai_persona: "Delete AI persona" + create_ai_tool: "Create AI tool" + update_ai_tool: "Update AI tool" + delete_ai_tool: "Delete AI tool" + create_ai_embedding: "Create AI embedding" + update_ai_embedding: "Update AI embedding" + delete_ai_embedding: "Delete AI embedding" js: discourse_automation: diff --git a/lib/utils/ai_staff_action_logger.rb b/lib/utils/ai_staff_action_logger.rb index 37ef241a..605dcc98 100644 --- a/lib/utils/ai_staff_action_logger.rb +++ b/lib/utils/ai_staff_action_logger.rb @@ -3,110 +3,153 @@ module DiscourseAi module Utils class AiStaffActionLogger + # Maximum length for text fields before truncation/simplification + MAX_TEXT_LENGTH = 100 + def initialize(current_user) @current_user = current_user @staff_logger = ::StaffActionLogger.new(current_user) end # Log creation of an AI entity (LLM model or persona) - def log_creation(entity_type, entity, attributes_to_log) - log_details = extract_entity_attributes(entity, attributes_to_log) - + def log_creation(entity_type, entity, field_config = {}, entity_details = {}) + # Start with provided entity details (id, name, etc.) + # Convert all keys to strings for consistent handling in StaffActionLogger + log_details = {} + entity_details.each { |k, v| log_details[k.to_s] = v } + + # Extract attributes based on field configuration and ensure string keys + extract_entity_attributes(entity, field_config).each do |key, value| + log_details[key.to_s] = value + end + @staff_logger.log_custom("create_ai_#{entity_type}", log_details) end # Log update of an AI entity with before/after comparison - def log_update(entity_type, entity, initial_attributes, trackable_fields, json_fields = []) + def log_update( + entity_type, + entity, + initial_attributes, + field_config = {}, + entity_details = {} + ) current_attributes = entity.attributes changes = {} - # Track changes to standard fields - trackable_fields.each do |field| - initial_value = initial_attributes[field] - current_value = current_attributes[field] - - if initial_value != current_value - # For large text fields, don't show the entire content - if should_simplify_field?(field, initial_value, current_value) - changes[field] = "updated" - else - changes[field] = "#{initial_value} → #{current_value}" + # Process changes based on field configuration + field_config + .except(:json_fields) + .each do |field, options| + # Skip if field is not to be tracked + next if options[:track] == false + + initial_value = initial_attributes[field.to_s] + current_value = current_attributes[field.to_s] + + # Only process if there's an actual change + if initial_value != current_value + # Format the change based on field type + changes[field.to_s] = format_field_change( + field, + initial_value, + current_value, + options, + ) end end - end - # Track changes to arrays and JSON fields - json_fields.each do |field| - if initial_attributes[field].to_s != current_attributes[field].to_s - changes[field] = "updated" + # Process simple JSON fields (arrays, hashes) that should be tracked as "updated" + if field_config[:json_fields].present? + field_config[:json_fields].each do |field| + field_str = field.to_s + if initial_attributes[field_str].to_s != current_attributes[field_str].to_s + changes[field_str] = "updated" + end end end # Only log if there are actual changes if changes.any? - log_details = entity_identifier(entity, entity_type).merge(changes) + log_details = {} + # Convert entity_details keys to strings + entity_details.each { |k, v| log_details[k.to_s] = v } + # Merge changes (already with string keys) + log_details.merge!(changes) + @staff_logger.log_custom("update_ai_#{entity_type}", log_details) end end # Log deletion of an AI entity def log_deletion(entity_type, entity_details) - @staff_logger.log_custom("delete_ai_#{entity_type}", entity_details) + # Convert all keys to strings for consistent handling in StaffActionLogger + string_details = {} + entity_details.each { |k, v| string_details[k.to_s] = v } + + @staff_logger.log_custom("delete_ai_#{entity_type}", string_details) end - + # Direct custom logging for complex cases def log_custom(action_type, log_details) - @staff_logger.log_custom(action_type, log_details) + # Convert all keys to strings for consistent handling in StaffActionLogger + string_details = {} + log_details.each { |k, v| string_details[k.to_s] = v } + + @staff_logger.log_custom(action_type, string_details) end private - def extract_entity_attributes(entity, attributes_to_log) + def format_field_change(field, initial_value, current_value, options = {}) + # Handle different field types based on controller-provided options + if options[:type] == :sensitive + return format_sensitive_field_change(initial_value, current_value) + elsif options[:type] == :large_text || + (initial_value.is_a?(String) && initial_value.length > MAX_TEXT_LENGTH) || + (current_value.is_a?(String) && current_value.length > MAX_TEXT_LENGTH) + return "updated" + end + + # Default formatting: "old_value → new_value" + "#{initial_value} → #{current_value}" + end + + def format_sensitive_field_change(initial_value, current_value) + if initial_value.present? && current_value.present? + "updated" + elsif current_value.present? + "set" + else + "removed" + end + end + + def extract_entity_attributes(entity, field_config) result = {} - - attributes_to_log.each do |attr| - value = entity.public_send(attr) - - # Handle large text fields - if attr == :system_prompt && value.is_a?(String) && value.length > 100 - result[attr] = value.truncate(100) + + # Process each field according to its configuration + field_config.each do |field, options| + # Skip fields explicitly marked as not to be extracted + next if options[:extract] == false + + # Get the actual field value + field_sym = field.to_sym + value = entity.respond_to?(field_sym) ? entity.public_send(field_sym) : nil + + # Apply field-specific handling + if options[:type] == :sensitive + result[field] = value.present? ? "[FILTERED]" : nil + elsif options[:type] == :large_text && value.is_a?(String) && + value.length > MAX_TEXT_LENGTH + result[field] = value.truncate(MAX_TEXT_LENGTH) else - result[attr] = value + result[field] = value end end - + result end - - def should_simplify_field?(field, initial_value, current_value) - # For large text fields, or sensitive data, don't show the entire content - return true if field == "system_prompt" && - initial_value.present? && - current_value.present? && - (initial_value.length > 100 || current_value.length > 100) - - return true if field.include?("api_key") || field.include?("secret") || field.include?("password") - - false - end - - def entity_identifier(entity, entity_type) - case entity_type - when "llm_model" - { - model_id: entity.id, - model_name: entity.name, - display_name: entity.display_name - } - when "persona" - { - persona_id: entity.id, - persona_name: entity.name - } - else - { id: entity.id } - end - end end end -end \ No newline at end of file +end diff --git a/spec/lib/utils/ai_staff_action_logger_spec.rb b/spec/lib/utils/ai_staff_action_logger_spec.rb new file mode 100644 index 00000000..aabea533 --- /dev/null +++ b/spec/lib/utils/ai_staff_action_logger_spec.rb @@ -0,0 +1,437 @@ +# frozen_string_literal: true + +RSpec.describe DiscourseAi::Utils::AiStaffActionLogger do + fab!(:admin) + fab!(:llm_model) + fab!(:ai_persona) + fab!(:group) { Fabricate(:group) } + + subject { described_class.new(admin) } + + describe "#log_creation" do + it "logs creation of an entity with field configuration" do + staff_action_logger = instance_double(StaffActionLogger) + expect(StaffActionLogger).to receive(:new).with(admin).and_return(staff_action_logger) + + # Create field configuration + field_config = { + name: {}, + provider: {}, + url: {}, + api_key: { type: :sensitive } + } + + # Create entity details + entity_details = { + model_id: llm_model.id, + model_name: llm_model.name, + display_name: llm_model.display_name + } + + # Setup model with sensitive data + llm_model.update!(api_key: "secret_key") + + expect(staff_action_logger).to receive(:log_custom).with( + "create_ai_llm_model", + hash_including( + "model_id" => llm_model.id, + "name" => llm_model.name, + "provider" => llm_model.provider, + "url" => llm_model.url, + "api_key" => "[FILTERED]" + ) + ) + + subject.log_creation("llm_model", llm_model, field_config, entity_details) + end + + it "handles large text fields with type declaration" do + staff_action_logger = instance_double(StaffActionLogger) + expect(StaffActionLogger).to receive(:new).with(admin).and_return(staff_action_logger) + + # Create a persona with a large system prompt + large_prompt = "a" * 200 + ai_persona.update!(system_prompt: large_prompt) + + # Create entity details + entity_details = { + persona_id: ai_persona.id, + persona_name: ai_persona.name + } + + field_config = { + name: {}, + description: {}, + system_prompt: { type: :large_text } + } + + # The log_details should include the truncated system_prompt + expect(staff_action_logger).to receive(:log_custom).with( + "create_ai_persona", + hash_including( + "persona_id" => ai_persona.id, + "name" => ai_persona.name, + "system_prompt" => an_instance_of(String) + ) + ) do |action, details| + # Check that system_prompt was truncated + expect(details["system_prompt"].length).to be < 200 + end + + subject.log_creation("persona", ai_persona, field_config, entity_details) + end + + it "allows excluding fields from extraction" do + staff_action_logger = instance_double(StaffActionLogger) + expect(StaffActionLogger).to receive(:new).with(admin).and_return(staff_action_logger) + + field_config = { + name: {}, + display_name: {}, + provider: { extract: false }, # Should be excluded + url: {} + } + + # Create entity details + entity_details = { + model_id: llm_model.id, + model_name: llm_model.name, + display_name: llm_model.display_name + } + + expect(staff_action_logger).to receive(:log_custom).with( + "create_ai_llm_model", + hash_including( + "model_id" => llm_model.id, + "name" => llm_model.name, + "display_name" => llm_model.display_name, + "url" => llm_model.url + ) + ) do |action, details| + # Provider should not be present + expect(details).not_to have_key("provider") + end + + subject.log_creation("llm_model", llm_model, field_config, entity_details) + end + end + + describe "#log_update" do + it "handles empty arrays and complex JSON properly" do + staff_action_logger = instance_double(StaffActionLogger) + expect(StaffActionLogger).to receive(:new).with(admin).and_return(staff_action_logger) + + # Setup initial attributes with empty JSON arrays + initial_attributes = { + "name" => "Old Name", + "allowed_group_ids" => [] + } + + # Update with complex JSON + ai_persona.update!( + name: "New Name", + allowed_group_ids: [group.id, 999] + ) + + field_config = { + name: {}, + json_fields: %w[allowed_group_ids] + } + + # It should log that allowed_group_ids was updated without showing the exact diff + expect(staff_action_logger).to receive(:log_custom).with( + "update_ai_persona", + hash_including( + "persona_id" => ai_persona.id, + "persona_name" => ai_persona.name, + "name" => "Old Name → New Name", + "allowed_group_ids" => "updated" + ) + ) + + # Create entity details + entity_details = { + persona_id: ai_persona.id, + persona_name: ai_persona.name + } + + subject.log_update("persona", ai_persona, initial_attributes, field_config, entity_details) + end + + it "logs changes to attributes based on field configuration" do + staff_action_logger = instance_double(StaffActionLogger) + expect(StaffActionLogger).to receive(:new).with(admin).and_return(staff_action_logger) + + initial_attributes = { + "name" => "Old Name", + "display_name" => "Old Display Name", + "provider" => "open_ai", + "api_key" => "old_secret" + } + + llm_model.update!( + name: "New Name", + display_name: "New Display Name", + provider: "anthropic", + api_key: "new_secret" + ) + + field_config = { + name: {}, + display_name: {}, + provider: {}, + api_key: { type: :sensitive } + } + + # Create entity details + entity_details = { + model_id: llm_model.id, + model_name: llm_model.name, + display_name: llm_model.display_name + } + + # It should include changes with appropriate handling for sensitive data + expect(staff_action_logger).to receive(:log_custom).with( + "update_ai_llm_model", + hash_including( + "model_id" => llm_model.id, + "name" => "Old Name → New Name", + "display_name" => "Old Display Name → New Display Name", + "provider" => "open_ai → anthropic", + "api_key" => "updated" # Not showing actual values + ) + ) + + subject.log_update("llm_model", llm_model, initial_attributes, field_config, entity_details) + end + + it "doesn't log when there are no changes" do + staff_action_logger = instance_double(StaffActionLogger) + expect(StaffActionLogger).to receive(:new).with(admin).and_return(staff_action_logger) + + initial_attributes = { + "name" => llm_model.name, + "display_name" => llm_model.display_name, + "provider" => llm_model.provider + } + + field_config = { + name: {}, + display_name: {}, + provider: {} + } + + # It should not call log_custom since there are no changes + expect(staff_action_logger).not_to receive(:log_custom) + + # Create entity details + entity_details = { + model_id: llm_model.id, + model_name: llm_model.name, + display_name: llm_model.display_name + } + + subject.log_update("llm_model", llm_model, initial_attributes, field_config, entity_details) + end + + it "handles fields marked as not to be tracked" do + staff_action_logger = instance_double(StaffActionLogger) + expect(StaffActionLogger).to receive(:new).with(admin).and_return(staff_action_logger) + + initial_attributes = { + "name" => "Old Name", + "display_name" => "Old Display Name", + "provider" => "open_ai" + } + + llm_model.update!( + name: "New Name", + display_name: "New Display Name", + provider: "anthropic" + ) + + field_config = { + name: {}, + display_name: {}, + provider: { track: false } # Should not be tracked even though it changed + } + + # Provider should not appear in the logged changes + expect(staff_action_logger).to receive(:log_custom).with( + "update_ai_llm_model", + hash_including( + "model_id" => llm_model.id, + "name" => "Old Name → New Name", + "display_name" => "Old Display Name → New Display Name" + ) + ) do |action, details| + expect(details).not_to have_key("provider") + end + + # Create entity details + entity_details = { + model_id: llm_model.id, + model_name: llm_model.name, + display_name: llm_model.display_name + } + + subject.log_update("llm_model", llm_model, initial_attributes, field_config, entity_details) + end + + it "handles json fields properly" do + staff_action_logger = instance_double(StaffActionLogger) + expect(StaffActionLogger).to receive(:new).with(admin).and_return(staff_action_logger) + + # Setup initial attributes with JSON fields + initial_attributes = { + "name" => "Old Name", + "tools" => [["search", { "base_query" => "test" }, true]] + } + + # Update with different JSON + ai_persona.update!( + name: "New Name", + tools: [["search", { "base_query" => "updated" }, true], ["categories", {}, false]] + ) + + field_config = { + name: {}, + json_fields: %w[tools] + } + + # It should log that tools was updated without showing the exact diff + expect(staff_action_logger).to receive(:log_custom).with( + "update_ai_persona", + hash_including( + "persona_id" => ai_persona.id, + "persona_name" => ai_persona.name, + "name" => "Old Name → New Name", + "tools" => "updated" + ) + ) + + # Create entity details + entity_details = { + persona_id: ai_persona.id, + persona_name: ai_persona.name + } + + subject.log_update("persona", ai_persona, initial_attributes, field_config, entity_details) + end + end + + describe "#log_deletion" do + it "logs deletion with the correct entity type" do + staff_action_logger = instance_double(StaffActionLogger) + expect(StaffActionLogger).to receive(:new).with(admin).and_return(staff_action_logger) + + details = { + model_id: llm_model.id, + display_name: llm_model.display_name, + name: llm_model.name + } + + expect(staff_action_logger).to receive(:log_custom).with( + "delete_ai_llm_model", + hash_including( + "model_id" => details[:model_id], + "display_name" => details[:display_name], + "name" => details[:name] + ) + ) + + subject.log_deletion("llm_model", details) + end + end + + describe "#log_custom" do + it "delegates to StaffActionLogger#log_custom" do + staff_action_logger = instance_double(StaffActionLogger) + expect(StaffActionLogger).to receive(:new).with(admin).and_return(staff_action_logger) + + details = { key: "value" } + + expect(staff_action_logger).to receive(:log_custom).with( + "custom_action_type", + hash_including("key" => details[:key]) + ) + + subject.log_custom("custom_action_type", details) + end + end + + describe "Special cases from controllers" do + context "with LlmModel quotas" do + before do + # Create a quota for the model + @quota = Fabricate(:llm_quota, llm_model: llm_model, group: group, max_tokens: 1000) + end + + it "handles quota changes in log_llm_model_creation" do + expect_any_instance_of(StaffActionLogger).to receive(:log_custom) do |_, action, details| + expect(action).to eq("create_ai_llm_model") + expect(details).to include("model_id", "model_name", "display_name") + expect(details["quotas"]).to include("Group #{group.id}") + expect(details["quotas"]).to include("1000 tokens") + end + + # Call the method directly as it would be called from the controller + logger = DiscourseAi::Utils::AiStaffActionLogger.new(admin) + field_config = { display_name: {}, name: {} } + + # Create entity details + entity_details = { + model_id: llm_model.id, + model_name: llm_model.name, + display_name: llm_model.display_name + } + + log_details = entity_details.dup + log_details.merge!(logger.send(:extract_entity_attributes, llm_model, field_config)) + + # Add quota information as a special case + log_details[:quotas] = llm_model.llm_quotas.map do |quota| + "Group #{quota.group_id}: #{quota.max_tokens} tokens, #{quota.max_usages} usages, #{quota.duration_seconds}s" + end.join("; ") + + logger.log_custom("create_ai_llm_model", log_details) + end + + it "handles quota changes in log_llm_model_update" do + initial_quotas = llm_model.llm_quotas.map(&:attributes) + + # Update the quota + @quota.update!(max_tokens: 2000) + current_quotas = llm_model.llm_quotas.reload.map(&:attributes) + + expect_any_instance_of(StaffActionLogger).to receive(:log_custom) do |_, action, details| + expect(action).to eq("update_ai_llm_model") + expect(details).to include("model_id", "quotas") + expect(details["quotas"]).to include("1000 tokens") + expect(details["quotas"]).to include("2000 tokens") + end + + # Simulate the special quota handling in the controller + logger = DiscourseAi::Utils::AiStaffActionLogger.new(admin) + changes = {} + + # Track quota changes separately as they're a special case + if initial_quotas != current_quotas + initial_quota_summary = initial_quotas.map { |q| "Group #{q['group_id']}: #{q['max_tokens']} tokens" }.join("; ") + current_quota_summary = current_quotas.map { |q| "Group #{q['group_id']}: #{q['max_tokens']} tokens" }.join("; ") + changes[:quotas] = "#{initial_quota_summary} → #{current_quota_summary}" + end + + # Create entity details + entity_details = { + model_id: llm_model.id, + model_name: llm_model.name, + display_name: llm_model.display_name + } + + log_details = entity_details.dup.merge(changes) + logger.log_custom("update_ai_llm_model", log_details) + end + end + end +end \ No newline at end of file diff --git a/spec/requests/admin/ai_embeddings_controller_spec.rb b/spec/requests/admin/ai_embeddings_controller_spec.rb index 40106b5d..34889d1b 100644 --- a/spec/requests/admin/ai_embeddings_controller_spec.rb +++ b/spec/requests/admin/ai_embeddings_controller_spec.rb @@ -34,6 +34,17 @@ RSpec.describe DiscourseAi::Admin::AiEmbeddingsController do expect(created_def.search_prompt).to eq(valid_attrs[:search_prompt]) expect(created_def.matryoshka_dimensions).to eq(true) end + + it "logs the creation with StaffActionLogger" do + expect { + post "/admin/plugins/discourse-ai/ai-embeddings.json", params: { ai_embedding: valid_attrs } + }.to change { UserHistory.where(action: UserHistory.actions[:custom_staff], custom_type: "create_ai_embedding").count }.by(1) + + history = UserHistory.where(action: UserHistory.actions[:custom_staff], custom_type: "create_ai_embedding").last + expect(history.details).to include("display_name: Embedding config test") + expect(history.details).to include("provider: hugging_face") + expect(history.details).to include("dimensions: 1001") + end it "stores provider-specific config params" do post "/admin/plugins/discourse-ai/ai-embeddings.json", @@ -95,6 +106,20 @@ RSpec.describe DiscourseAi::Admin::AiEmbeddingsController do expect(response.status).to eq(200) expect(embedding_definition.reload.provider).to eq(update_attrs[:provider]) end + + it "logs the update with StaffActionLogger" do + expect { + put "/admin/plugins/discourse-ai/ai-embeddings/#{embedding_definition.id}.json", + params: { + ai_embedding: update_attrs, + } + }.to change { UserHistory.where(action: UserHistory.actions[:custom_staff], custom_type: "update_ai_embedding").count }.by(1) + + history = UserHistory.where(action: UserHistory.actions[:custom_staff], custom_type: "update_ai_embedding").last + expect(history.details).to include("embedding_id: #{embedding_definition.id}") + expect(history.details).to include("provider_changed: true") + expect(history.details).to include("changed_fields:") + end it "returns a 404 if there is no model with the given Id" do put "/admin/plugins/discourse-ai/ai-embeddings/9999999.json" @@ -141,6 +166,19 @@ RSpec.describe DiscourseAi::Admin::AiEmbeddingsController do expect(response).to have_http_status(:no_content) }.to change(EmbeddingDefinition, :count).by(-1) end + + it "logs the deletion with StaffActionLogger" do + embedding_id = embedding_definition.id + display_name = embedding_definition.display_name + + expect { + delete "/admin/plugins/discourse-ai/ai-embeddings/#{embedding_definition.id}.json" + }.to change { UserHistory.where(action: UserHistory.actions[:custom_staff], custom_type: "delete_ai_embedding").count }.by(1) + + history = UserHistory.where(action: UserHistory.actions[:custom_staff], custom_type: "delete_ai_embedding").last + expect(history.details).to include("embedding_id: #{embedding_id}") + expect(history.details).to include("display_name: #{display_name}") + end it "validates the model is not in use" do SiteSetting.ai_embeddings_selected_model = embedding_definition.id diff --git a/spec/requests/admin/ai_llms_controller_spec.rb b/spec/requests/admin/ai_llms_controller_spec.rb index 280204f2..5651bf0b 100644 --- a/spec/requests/admin/ai_llms_controller_spec.rb +++ b/spec/requests/admin/ai_llms_controller_spec.rb @@ -136,6 +136,20 @@ RSpec.describe DiscourseAi::Admin::AiLlmsController do model = LlmModel.find(created_model["id"]) expect(model.display_name).to eq(valid_attrs[:display_name]) end + + it "logs staff action when creating an LLM model" do + logger = instance_double(DiscourseAi::Utils::AiStaffActionLogger) + expect(DiscourseAi::Utils::AiStaffActionLogger).to receive(:new).with(admin).and_return(logger) + expect(logger).to receive(:log_custom).with("create_ai_llm_model", hash_including( + model_id: an_instance_of(Integer), + display_name: valid_attrs[:display_name], + name: valid_attrs[:name], + provider: valid_attrs[:provider] + )) + + post "/admin/plugins/discourse-ai/ai-llms.json", params: { ai_llm: valid_attrs } + expect(response.status).to eq(201) + end it "creates a companion user" do post "/admin/plugins/discourse-ai/ai-llms.json", @@ -329,6 +343,30 @@ RSpec.describe DiscourseAi::Admin::AiLlmsController do expect(response.status).to eq(200) expect(llm_model.reload.provider).to eq(update_attrs[:provider]) end + + it "logs staff action when updating an LLM model" do + # The initial provider is different from the update + original_provider = llm_model.provider + + logger = instance_double(DiscourseAi::Utils::AiStaffActionLogger) + expect(DiscourseAi::Utils::AiStaffActionLogger).to receive(:new).with(admin).and_return(logger) + + # Verify the log_custom is called with the right arguments + expect(logger).to receive(:log_custom).with( + "update_ai_llm_model", + hash_including( + model_id: llm_model.id, + provider: "#{original_provider} → #{update_attrs[:provider]}" + ) + ) + + put "/admin/plugins/discourse-ai/ai-llms/#{llm_model.id}.json", + params: { + ai_llm: update_attrs, + } + + expect(response.status).to eq(200) + end it "returns a 404 if there is no model with the given Id" do put "/admin/plugins/discourse-ai/ai-llms/9999999.json" @@ -457,6 +495,29 @@ RSpec.describe DiscourseAi::Admin::AiLlmsController do expect(response).to have_http_status(:no_content) }.to change(LlmModel, :count).by(-1) end + + it "logs staff action when deleting an LLM model" do + # Capture the model details before deletion for comparison + model_id = llm_model.id + model_name = llm_model.name + model_display_name = llm_model.display_name + + logger = instance_double(DiscourseAi::Utils::AiStaffActionLogger) + expect(DiscourseAi::Utils::AiStaffActionLogger).to receive(:new).with(admin).and_return(logger) + + # It should use log_deletion with details about the deleted model + expect(logger).to receive(:log_deletion).with( + "llm_model", + hash_including( + model_id: model_id, + name: model_name, + display_name: model_display_name + ) + ) + + delete "/admin/plugins/discourse-ai/ai-llms/#{llm_model.id}.json" + expect(response).to have_http_status(:no_content) + end it "validates the model is not in use" do fake_llm = assign_fake_provider_to(:ai_helper_model) diff --git a/spec/requests/admin/ai_personas_controller_spec.rb b/spec/requests/admin/ai_personas_controller_spec.rb index 17b7fe34..3a0d7a1d 100644 --- a/spec/requests/admin/ai_personas_controller_spec.rb +++ b/spec/requests/admin/ai_personas_controller_spec.rb @@ -223,6 +223,29 @@ RSpec.describe DiscourseAi::Admin::AiPersonasController do expect(persona.temperature).to eq(0.5) }.to change(AiPersona, :count).by(1) end + + it "logs staff action when creating a persona" do + logger = instance_double(DiscourseAi::Utils::AiStaffActionLogger) + expect(DiscourseAi::Utils::AiStaffActionLogger).to receive(:new).with(admin).and_return(logger) + + # Verify logging happens with the right parameters + expect(logger).to receive(:log_custom).with( + "create_ai_persona", + hash_including( + name: "superbot", + description: "Assists with tasks", + system_prompt: "you are a helpful bot" + ) + ) + + post "/admin/plugins/discourse-ai/ai-personas.json", + params: { ai_persona: valid_attributes }.to_json, + headers: { + "CONTENT_TYPE" => "application/json", + } + + expect(response).to be_successful + end end context "with invalid params" do @@ -309,6 +332,35 @@ RSpec.describe DiscourseAi::Admin::AiPersonasController do expect(persona.top_p).to eq(nil) expect(persona.temperature).to eq(nil) end + + it "logs staff action when updating a persona" do + persona = Fabricate(:ai_persona, name: "original_name", description: "original description") + + logger = instance_double(DiscourseAi::Utils::AiStaffActionLogger) + expect(DiscourseAi::Utils::AiStaffActionLogger).to receive(:new).with(admin).and_return(logger) + + # It should detect and log the changes to name and description + expect(logger).to receive(:log_update) do |entity_type, entity, initial_attributes, trackable_fields, json_fields| + expect(entity_type).to eq("persona") + expect(entity).to eq(persona) + expect(trackable_fields).to include("name", "description") + expect(initial_attributes["name"]).to eq("original_name") + expect(initial_attributes["description"]).to eq("original description") + end + + put "/admin/plugins/discourse-ai/ai-personas/#{persona.id}.json", + params: { + ai_persona: { + name: "updated_name", + description: "updated description", + }, + } + + expect(response).to have_http_status(:ok) + persona.reload + expect(persona.name).to eq("updated_name") + expect(persona.description).to eq("updated description") + end it "supports updating rag params" do persona = Fabricate(:ai_persona, name: "test_bot2") @@ -461,6 +513,29 @@ RSpec.describe DiscourseAi::Admin::AiPersonasController do expect(response).to have_http_status(:no_content) }.to change(AiPersona, :count).by(-1) end + + it "logs staff action when deleting a persona" do + # Capture persona details before deletion + persona_id = ai_persona.id + persona_name = ai_persona.name + persona_description = ai_persona.description + + logger = instance_double(DiscourseAi::Utils::AiStaffActionLogger) + expect(DiscourseAi::Utils::AiStaffActionLogger).to receive(:new).with(admin).and_return(logger) + + # It should use log_deletion with the correct entity details + expect(logger).to receive(:log_deletion).with( + "persona", + hash_including( + persona_id: persona_id, + name: persona_name, + description: persona_description + ) + ) + + delete "/admin/plugins/discourse-ai/ai-personas/#{ai_persona.id}.json" + expect(response).to have_http_status(:no_content) + end it "is not allowed to delete system personas" do expect { diff --git a/spec/requests/admin/ai_tools_controller_spec.rb b/spec/requests/admin/ai_tools_controller_spec.rb index b96e5361..7ee17e80 100644 --- a/spec/requests/admin/ai_tools_controller_spec.rb +++ b/spec/requests/admin/ai_tools_controller_spec.rb @@ -71,6 +71,20 @@ RSpec.describe DiscourseAi::Admin::AiToolsController do expect(response.parsed_body["ai_tool"]["name"]).to eq("Test Tool 1") expect(response.parsed_body["ai_tool"]["tool_name"]).to eq("test_tool_1") end + + it "logs the creation with StaffActionLogger" do + expect { + post "/admin/plugins/discourse-ai/ai-tools.json", + params: { ai_tool: valid_attributes }.to_json, + headers: { + "CONTENT_TYPE" => "application/json", + } + }.to change { UserHistory.where(action: UserHistory.actions[:custom_staff], custom_type: "create_ai_tool").count }.by(1) + + history = UserHistory.where(action: UserHistory.actions[:custom_staff], custom_type: "create_ai_tool").last + expect(history.details).to include("name: Test Tool 1") + expect(history.details).to include("tool_name: test_tool_1") + end context "when the parameter is a enum" do it "creates the tool with the correct parameters" do @@ -140,6 +154,22 @@ RSpec.describe DiscourseAi::Admin::AiToolsController do expect(response).to be_successful expect(ai_tool.reload.name).to eq("Updated Tool") end + + it "logs the update with StaffActionLogger" do + expect { + put "/admin/plugins/discourse-ai/ai-tools/#{ai_tool.id}.json", + params: { + ai_tool: { + name: "Updated Tool", + description: "Updated description" + }, + } + }.to change { UserHistory.where(action: UserHistory.actions[:custom_staff], custom_type: "update_ai_tool").count }.by(1) + + history = UserHistory.where(action: UserHistory.actions[:custom_staff], custom_type: "update_ai_tool").last + expect(history.details).to include("tool_id: #{ai_tool.id}") + expect(history.details).to include("name_changed: true") + end context "when updating an enum parameters" do it "updates the enum fixed values" do @@ -172,6 +202,17 @@ RSpec.describe DiscourseAi::Admin::AiToolsController do expect(response).to have_http_status(:no_content) end + + it "logs the deletion with StaffActionLogger" do + tool_id = ai_tool.id + + expect { + delete "/admin/plugins/discourse-ai/ai-tools/#{ai_tool.id}.json" + }.to change { UserHistory.where(action: UserHistory.actions[:custom_staff], custom_type: "delete_ai_tool").count }.by(1) + + history = UserHistory.where(action: UserHistory.actions[:custom_staff], custom_type: "delete_ai_tool").last + expect(history.details).to include("tool_id: #{tool_id}") + end end describe "#test" do