diff --git a/apps/src/aiTutor/chatApi.ts b/apps/src/aiTutor/chatApi.ts index 82fe41513eb18..120bc1878515f 100644 --- a/apps/src/aiTutor/chatApi.ts +++ b/apps/src/aiTutor/chatApi.ts @@ -40,11 +40,17 @@ const logViolationDetails = (response: OpenaiChatCompletionMessage) => { export async function postOpenaiChatCompletion( messagesToSend: OpenaiChatCompletionMessage[], levelId?: number, - tutorType?: AITutorTypesValue + tutorType?: AITutorTypesValue, + levelInstructions?: string ): Promise { const payload = levelId - ? {levelId: levelId, messages: messagesToSend, type: tutorType} - : {messages: messagesToSend, type: tutorType}; + ? { + levelId: levelId, + messages: messagesToSend, + type: tutorType, + levelInstructions, + } + : {messages: messagesToSend, type: tutorType, levelInstructions}; const response = await HttpClient.post( CHAT_COMPLETION_URL, @@ -57,7 +63,7 @@ export async function postOpenaiChatCompletion( if (response.ok) { return await response.json(); } else { - return null; + throw new Error('Error getting chat completion response'); } } @@ -74,14 +80,13 @@ const formatForChatCompletion = ( * to `postOpenaiChatCompletion`, then returns the status of the response and assistant message if successful. */ export async function getChatCompletionMessage( - systemPrompt: string, formattedQuestion: string, chatMessages: ChatCompletionMessage[], levelId?: number, - tutorType?: AITutorTypesValue + tutorType?: AITutorTypesValue, + levelInstructions?: string ): Promise { const messagesToSend = [ - {role: Role.SYSTEM, content: systemPrompt}, ...formatForChatCompletion(chatMessages), {role: Role.USER, content: formattedQuestion}, ]; @@ -90,7 +95,8 @@ export async function getChatCompletionMessage( response = await postOpenaiChatCompletion( messagesToSend, levelId, - tutorType + tutorType, + levelInstructions ); } catch (error) { MetricsReporter.logError({ @@ -100,7 +106,12 @@ export async function getChatCompletionMessage( }); } - if (!response) return {status: Status.ERROR}; + if (!response) + return { + status: Status.ERROR, + assistantResponse: + 'There was an error processing your request. Please try again.', + }; switch (response.status) { case ShareFilterStatus.Profanity: diff --git a/apps/src/aiTutor/constants.ts b/apps/src/aiTutor/constants.ts index 9558594e25a37..9e36c19c2e90a 100644 --- a/apps/src/aiTutor/constants.ts +++ b/apps/src/aiTutor/constants.ts @@ -3,9 +3,6 @@ import { AITutorInteractionStatus as Status, } from '@cdo/apps/aiTutor/types'; -export const systemPrompt = - 'You are a tutor in a high school classroom where the students are learning Java using the Code.org curriculum. Answer their questions in plain, easy-to-understand English. Do not write any code. Do not answer the question if it is not about Java or computer programming. Please format all responses in Markdown where appropriate. Use four spaces for indentation instead of tabs.'; - // Initial messages we set when the user selects a tutor type. // General Chat export const generalChatMessage = { diff --git a/apps/src/aiTutor/redux/aiTutorRedux.ts b/apps/src/aiTutor/redux/aiTutorRedux.ts index a39ac89abf995..886aa13229628 100644 --- a/apps/src/aiTutor/redux/aiTutorRedux.ts +++ b/apps/src/aiTutor/redux/aiTutorRedux.ts @@ -1,7 +1,6 @@ import _ from 'lodash'; import {getChatCompletionMessage} from '@cdo/apps/aiTutor/chatApi'; import {createSlice, PayloadAction, createAsyncThunk} from '@reduxjs/toolkit'; -import {systemPrompt as baseSystemPrompt} from '@cdo/apps/aiTutor/constants'; import {savePromptAndResponse} from '../interactionsApi'; import { Role, @@ -20,7 +19,6 @@ export interface AITutorState { aiResponse: string | undefined; chatMessages: ChatCompletionMessage[]; isWaitingForChatResponse: boolean; - chatMessageError: boolean; isChatOpen: boolean; } @@ -42,7 +40,6 @@ const initialState: AITutorState = { aiResponse: '', chatMessages: initialChatMessages, isWaitingForChatResponse: false, - chatMessageError: false, isChatOpen: false, }; @@ -72,15 +69,8 @@ export const askAITutor = createAsyncThunk( scriptId: aiTutorState.aiTutor.scriptId, }; - let systemPrompt = baseSystemPrompt; const levelInstructions = instructionsState.instructions.longInstructions; - if (levelInstructions.length > 0) { - systemPrompt += - '\n Here are the student instructions for this level: ' + - levelInstructions; - } - const storedMessages = aiTutorState.aiTutor.chatMessages; const newMessage: ChatCompletionMessage = { role: Role.USER, @@ -91,11 +81,11 @@ export const askAITutor = createAsyncThunk( const formattedQuestion = formatQuestionForAITutor(chatContext); const chatApiResponse = await getChatCompletionMessage( - systemPrompt, formattedQuestion, storedMessages, levelContext.levelId, - chatContext.actionType + chatContext.actionType, + levelInstructions ); thunkAPI.dispatch( updateLastChatMessage({ diff --git a/apps/src/aiTutor/views/AssistantMessage.tsx b/apps/src/aiTutor/views/AssistantMessage.tsx index e88a120148258..4c889ae98c538 100644 --- a/apps/src/aiTutor/views/AssistantMessage.tsx +++ b/apps/src/aiTutor/views/AssistantMessage.tsx @@ -47,6 +47,7 @@ const AssistantMessage: React.FC = ({message}) => { const shouldRenderFeedbackButtons = message.id && + message.status !== Status.ERROR && message.status !== Status.PROFANITY_VIOLATION && message.status !== Status.PII_VIOLATION; diff --git a/apps/src/aiTutor/views/UserMessage.tsx b/apps/src/aiTutor/views/UserMessage.tsx index 78adeb2949b17..21163e00bef63 100644 --- a/apps/src/aiTutor/views/UserMessage.tsx +++ b/apps/src/aiTutor/views/UserMessage.tsx @@ -16,15 +16,13 @@ const PROFANITY_VIOLATION_USER_MESSAGE = 'This chat has been hidden because it is inappropriate.'; const PII_VIOLATION_USER_MESSAGE = 'This chat has been hidden because it contains personal information.'; -const ERROR_USER_MESSAGE = - 'There was an error getting a response. Please try again.'; const statusToStyleMap = { [Status.OK]: style.userMessage, [Status.UNKNOWN]: style.userMessage, [Status.PROFANITY_VIOLATION]: style.profaneMessage, [Status.PII_VIOLATION]: style.piiMessage, - [Status.ERROR]: style.errorMessage, + [Status.ERROR]: style.userMessage, }; const getMessageText = (status: string, chatMessageText: string) => { @@ -34,7 +32,6 @@ const getMessageText = (status: string, chatMessageText: string) => { case Status.PII_VIOLATION: return PII_VIOLATION_USER_MESSAGE; case Status.ERROR: - return ERROR_USER_MESSAGE; default: return chatMessageText; } diff --git a/dashboard/app/controllers/openai_chat_controller.rb b/dashboard/app/controllers/openai_chat_controller.rb index 2af9e1185b170..5ae78cc5aa703 100644 --- a/dashboard/app/controllers/openai_chat_controller.rb +++ b/dashboard/app/controllers/openai_chat_controller.rb @@ -1,7 +1,14 @@ class OpenaiChatController < ApplicationController + S3_AI_BUCKET = 'cdo-ai'.freeze + S3_TUTOR_SYSTEM_PROMPT_PATH = 'tutor/system_prompt.txt'.freeze + include OpenaiChatHelper authorize_resource class: false + def s3_client + @s3_client ||= AWS::S3.create_client + end + # POST /openai/chat_completion def chat_completion unless has_required_messages_param? @@ -16,16 +23,19 @@ def chat_completion # If the content is inappropriate, we skip sending to OpenAI and instead hardcode a warning response on the front-end. return render(status: :ok, json: {status: filter_result.type, flagged_content: filter_result.content}) if filter_result + # The system prompt is stored server-side so we need to prepend it to the student's messages + system_prompt = read_file_from_s3(S3_TUTOR_SYSTEM_PROMPT_PATH) + + # Determine if the level is validated and fetch test file contents if it is + test_file_contents = "" if validated_level? level_id = params[:levelId] test_file_contents = get_validated_level_test_file_contents(level_id) - messages = params[:messages] - messages.first["content"] = messages.first["content"] + " The contents of the test file are: #{test_file_contents}" - messages.second["content"] = "The student's code is: " + messages.second["content"] - else - messages = params[:messages] end + updated_system_prompt = add_content_to_system_prompt(system_prompt, params[:levelInstructions], test_file_contents) + messages = prepend_system_prompt(updated_system_prompt, params[:messages]) + response = OpenaiChatHelper.request_chat_completion(messages) chat_completion_return_message = OpenaiChatHelper.get_chat_completion_response_message(response) return render(status: chat_completion_return_message[:status], json: chat_completion_return_message[:json]) @@ -39,7 +49,55 @@ def validated_level? params[:type].present? && params[:type] == 'validation' end - def get_validated_level_test_file_contents(level_id) + def add_content_to_system_prompt(system_prompt, level_instructions, test_file_contents) + if level_instructions.present? + system_prompt += "\n Here are the student instructions for this level: #{level_instructions}" + end + + if test_file_contents.present? + system_prompt += "\n The contents of the test file are: #{test_file_contents}" + end + + system_prompt + end + + private def prepend_system_prompt(system_prompt, messages) + system_prompt_message = { + content: system_prompt, + role: "system" + } + + messages.unshift(system_prompt_message) + messages + end + + private def read_file_from_s3(key_path) + full_s3_path = "#{S3_AI_BUCKET}/#{key_path}" + cache_key = "s3_file:#{full_s3_path}" + unless Rails.env.development? + cached_content = CDO.shared_cache.read(cache_key) + return cached_content if cached_content.present? + end + + if Rails.env.development? + local_path = File.join("local-aws", S3_AI_BUCKET, key_path) + if File.exist?(local_path) + puts "Note: Reading AI prompt from local file: #{key_path}" + return File.read(local_path) + end + end + + # Note: We will hit this codepath in dev if the file is not found locally + content = s3_client.get_object(bucket: S3_AI_BUCKET, key: key_path).body.read + + # In production and test, cache the content after fetching it from S3 + unless Rails.env.development? + CDO.shared_cache.write(cache_key, content, expires_in: 1.hour) + end + return content + end + + private def get_validated_level_test_file_contents(level_id) level = Level.find(level_id) unless level diff --git a/dashboard/test/controllers/openai_chat_controller_test.rb b/dashboard/test/controllers/openai_chat_controller_test.rb index 0340553364437..8aa3413666859 100644 --- a/dashboard/test/controllers/openai_chat_controller_test.rb +++ b/dashboard/test/controllers/openai_chat_controller_test.rb @@ -1,3 +1,4 @@ +require 'aws-sdk-s3' require 'test_helper' class OpenaiChatControllerTest < ActionController::TestCase @@ -6,6 +7,14 @@ class OpenaiChatControllerTest < ActionController::TestCase OpenaiChatHelper.stubs(:request_chat_completion).returns(response) OpenaiChatHelper.stubs(:get_chat_completion_response_message).returns({status: 200, json: {}}) ShareFiltering.stubs(:find_failure).returns(nil) + + @mock_s3_client = mock + OpenaiChatController.any_instance.stubs(:s3_client).returns(@mock_s3_client) + stubbed_response = stub + stubbed_response.stubs(:body).returns(stub(read: "Content of the system prompt file")) + @mock_s3_client.stubs(:get_object).with(bucket: OpenaiChatController::S3_AI_BUCKET, key: 'tutor/system_prompt.txt').returns(stubbed_response) + + @controller = OpenaiChatController.new end # Student without ai tutor access is unable to access the chat completion endpoint @@ -48,6 +57,14 @@ class OpenaiChatControllerTest < ActionController::TestCase params: {messages: [{role: "user", content: "this is also a test!"}]}, response: :success + # Post request without a messages param returns a bad request + test_user_gets_response_for :chat_completion, + name: "no_messages_test", + user: :ai_tutor_access, + method: :post, + params: {}, + response: :bad_request + # Post request with a profane messages param returns a failure test 'returns failure when chat message contains profanity' do student = create(:student_with_ai_tutor_access) @@ -68,11 +85,97 @@ class OpenaiChatControllerTest < ActionController::TestCase assert_equal json_response["flagged_content"], "l.lovegood@hogwarts.edu" end - # Post request without a messages param returns a bad request - test_user_gets_response_for :chat_completion, - name: "no_messages_test", - user: :ai_tutor_access, - method: :post, - params: {}, - response: :bad_request + test 'add_content_to_system_prompt appends both level instructions and test file contents' do + system_prompt = "Initial prompt" + level_instructions = "Complete these steps" + test_file_contents = "Test file data" + + expected_result = "Initial prompt\n Here are the student instructions for this level: Complete these steps\n The contents of the test file are: Test file data" + actual_result = @controller.add_content_to_system_prompt(system_prompt, level_instructions, test_file_contents) + + assert_equal expected_result, actual_result + end + + test 'add_content_to_system_prompt handles nil inputs without appending' do + system_prompt = "Initial prompt" + level_instructions = nil + test_file_contents = nil + + expected_result = "Initial prompt" + actual_result = @controller.add_content_to_system_prompt(system_prompt, level_instructions, test_file_contents) + + assert_equal expected_result, actual_result + end + + test 'prepend_system_prompt correctly prepends system prompt to messages' do + system_prompt = "Initial system prompt" + messages = [{role: "user", content: "First message"}, {role: "user", content: "Second message"}] + + result = @controller.send(:prepend_system_prompt, system_prompt, messages) + + assert_equal 3, result.size + assert_equal "Initial system prompt", result.first[:content] + assert_equal "system", result.first[:role] + assert_equal "First message", result[1][:content] + assert_equal "Second message", result[2][:content] + end + + test 'prepend_system_prompt handles empty message array' do + system_prompt = "Initial system prompt" + messages = [] + + result = @controller.send(:prepend_system_prompt, system_prompt, messages) + + assert_equal 1, result.size + assert_equal "Initial system prompt", result.first[:content] + assert_equal "system", result.first[:role] + end + + test 'read_file_from_s3 retrieves the correct system prompt' do + assert_equal "Content of the system prompt file", @controller.send(:read_file_from_s3, 'tutor/system_prompt.txt') + end + + test 'read_file_from_s3 reads from local file system in development' do + key_path = OpenaiChatController::S3_TUTOR_SYSTEM_PROMPT_PATH + expected_content = "Local system prompt" + Rails.stubs(:env).returns(ActiveSupport::StringInquirer.new("development")) + + local_path = File.join("local-aws", OpenaiChatController::S3_AI_BUCKET, key_path) + File.stubs(:exist?).with(local_path).returns(true) + File.stubs(:read).with(local_path).returns(expected_content) + + assert_equal expected_content, @controller.send(:read_file_from_s3, key_path) + end + + test 'read_file_from_s3 handles errors when S3 file is not found' do + key_path = 'nonexistent/file.txt' + @mock_s3_client.stubs(:get_object).with(bucket: OpenaiChatController::S3_AI_BUCKET, key: key_path). + raises(Aws::S3::Errors::NoSuchKey.new(nil, "The specified key does not exist.")) + + exception = assert_raises(Aws::S3::Errors::NoSuchKey) do + @controller.send(:read_file_from_s3, key_path) + end + + assert_equal "The specified key does not exist.", exception.message + end + + test 'read_file_from_s3 caches content in production' do + key_path = OpenaiChatController::S3_TUTOR_SYSTEM_PROMPT_PATH + expected_content = "Content of the system prompt file" + Rails.stubs(:env).returns(ActiveSupport::StringInquirer.new("production")) + + CDO.shared_cache.stubs(:read).with("s3_file:cdo-ai/#{key_path}").returns(nil) + CDO.shared_cache.expects(:write).with("s3_file:cdo-ai/#{key_path}", expected_content, expires_in: 1.hour) + + assert_equal expected_content, @controller.send(:read_file_from_s3, key_path) + end + + test 'read_file_from_s3 reads from cache if available in production' do + key_path = OpenaiChatController::S3_TUTOR_SYSTEM_PROMPT_PATH + cached_content = "Cached system prompt" + Rails.stubs(:env).returns(ActiveSupport::StringInquirer.new("production")) + CDO.shared_cache.stubs(:read).with("s3_file:cdo-ai/#{key_path}").returns(cached_content) + + assert_equal cached_content, @controller.send(:read_file_from_s3, key_path) + end end