705 refactor handlechattogpt to remove mutation

[heatherlogan-scottlogic]

Description

Refactor of functions inside chatController.ts and openAI.ts to reduce mutation. ChatResponse, chatHistroy, sentEmails, should not be mutated across different functions - instead changes should be returned by the using functions either as

Notes

openai.ts

• chatGptCallFunction()

• • Before - mutates sentEmails, declares then mutates reply: ChatCompletionMessageParam
• • Changed - Remove init declaration for reply and construct at the end. Split up function calls into handleSendEmailFunction and handleAskQuestionFunction. Create a copy of sentEmails and updates this. Returns chat completion (with tool call ids), wonLevel and updated sentEmails obj

• performToolCalls()

• • Before - mutates chatResponse (sets wonLevel and defenceReport), calls pushCompletionToHistory()
• • Changed - Removed chatResponse from input as unused. Return wonLevel and defenceReport instead of setting chatResponse parameters. Clones history and updates tht If no toolCalls then returns original chatResponse.

• getFinalReplyAfterToolCalls()

• • Before - Created blank chatRespond and mutated, calls pushCompletionToHistory and performToolCalls (which previously mutated wonLevel/sentEmails/chatHistory)
• • Changed - Removed blank chatResponse at start (as not needed in chatGptChatCompletion). Clones to create sentEmails and updatedChatHistory. Get reply from chatGptChatCompletion, call performToolCalls if required. Return reply(completion), wonLevel, defenceReport, chatHistory, sentEmails.

• pushCompletionToHistory()

• • Before - mutates chatHisttory
• • Changed - Clones chatHistory and returns updated version

• chatGptChatCompletion()

• • Before - mutates chatHistory and chatResponse (sets openAIErrorMessage)
• • Changed - eemove chatResponse input parameter. Clone chatHistory to apply changes to that depending on system role. Returns the completion, updated chat history and error message (possibly null)

• chatGptSendMessage()

• • Before - mutates chatHistory and chatResponse (sets openAIErrorMessage)
• • Changed - Remove initial blank declaration of ChatResponse that will be mutated Clones chatHistory and edits that. Calls getFinalReplyAfterAllToolCalls which returns new chatResponse, chatHistory, sentEmails

chatController.ts

• handleLowLevelChat()

• • Before - mutates chatResponse
• • Changed - instead returns chatResponse, chatHistory, sentEmails that have been copied/updated by funcs. in openai.ts

• handleHigherLevelChat()

• • Before - mutates chatReponse (lots), mutates req.session.levelState[currentLevel].chatHistory in several places, calls chatGptSendMessage()
• • Changed - Clones the chatReponse and chatHistory objects and applies updates to these, returns chatResponse, chatHistory, sentEmails

• handleChatError()

• • Before - mutates chatResponse, res
• • Changed - Creates updatedChatResponse with blocked/error info and res.sends that. (concern – should we be doing res.send() in chatController?

• handleErrorGettingReply()

• • Before - handleErrorGettingReply() calls handleChatError(), mutates req (adds error to history)
• • Changed - Change to addErrorToChatHistory to prevent req being mutated in this and handleChatError. Instead returns updated chatHistory, then we can call handleChatError after in the outer function.

• handleChatToGPT()

• • Before - mutates chatResponse
• • Changed - creates an initial blank chat repsonse obj to use in error handing. then creates an updated obj with updated values from the handleXlevelChat function. updates the state in res.levelState for chatHistory and sentEmails with the final updated values (defences in levelState shouldn't be altered by handleChatToGPT(). sends the updated response

Concerns

• Removed the filterOutputDefence as thought it would conflict more with https://github.com/ScottLogic/prompt-injection/pull/740 - TBC. (but this defence doesn't work in this branch currently.. )

Checklist

Have you done the following?

• [x] Linked the relevant Issue
• [ ] Added tests
• [x] Ensured the workflow steps are passing

[chriswilty]

One thing that goes hand-in-hand with immutability is avoiding let declarations wherever possible. Sometimes it doesn't seem possible, if we have conditional statements that might or might not overwrite that value, but such cases are often a sign that the function needs to be split. In any case, this is a strong start, and the rest we can tackle as we go.

[pmarsh-scottlogic]

Really appreciate the thorough PR description ⭐

[pmarsh-scottlogic]

Nothing else from me. Aside from Peter's open questions (and the merge conflict) this looks good to go.

@pmarsh-scottlogic Would you like to take this one, and I'll handle George's PR?

@chriswilty I was about to suggest exactly the same thing :)

[pmarsh-scottlogic]

Review notes for myself

• combineChatDefenceReports added to chatController. Did it exist before and is it doing the same thing?
• createNewUserMessages added to chatController. Did it exist before and is it doing the same thing?

What happened before when user sends a message on a low level, say level 1?

• take original history and user message
• add user message to history (type depends on whether it was transformed)
• getFinalReply
  • mutate history to set/unset system role
  • get reply
  • while tool calls
  • mutate history to add reply with type function call
  • mutate chatResponse with new wonLevel value and new defenceReport value
  • if email function call
    • generate sent emails and mutate the emails session variable to add new emails
  • mutate history to add function call completion
• if problems [no reply content or openAiErrorMessage]
  • don't mutate chatResponse further
  • add error message to history
  • send error response
• if no problems
  • add bot response to history (type depends on whether it was blocked by output defence)
  • mutate chatreponse reply, wonLevel and openAIErrorMessage
  • update sentEmails in response and send httpResponse

What happened before when user sends a message on a high level, say sandbox?

• take original history and user message
• transform user message according to defences
• if transformed, mutate history to add original message [user type] and continue onwards with transformed message
• detect input defences, and mutate chatResponse accordingly.
• mutate chat history to add (possibly transformed) message. type depends on whether it was transformed or not
• getFinalReply
  • mutate history to set/unset system role
  • get reply
  • while tool calls
  • mutate history to add reply with type function call
  • mutate chatResponse with new wonLevel value and new defenceReport value
  • if email function call
    • generate sent emails and mutate the emails session variable to add new emails
  • mutate history to add function call completion
• if problems [no reply content or openAiErrorMessage]
  • don't mutate chatResponse further
  • mutate chat history to add error message
  • send error response
• if no problems
  • run output defences
  • add bot response to history (type depends on whether it was blocked by output defence)
  • mutate chatreponse reply, wonLevel and openAIErrorMessage
  • if blocked by input defences
  • undo all mutations to chatHistory (so that it is as it was before any of above processing).
  • mutate chatHistory to add user message as info message rather than completion.
  • else (not blocked, or blocked by output defences)
  • update chatResponse's wonLevel, reply, defenceReport,isBlocked, defenceReport.blockedReason, openAIErrorMessage based on output of chatGptSendMessage(...)
  • update chatReponse's triggeredDefences (combine input and output report)
  • if blocked by either input or output defence
  • mutate history to add block message
  • updateEmails and send response
  • else
  • update emails and send response

What happens now when user sends a message on a low level, say level 1?

• add user message to history
• get finalReply
  • while tool calls
  • update history according system role
  • update history to add completion as function call type.
  • if email function call
    • send email and get reply. see if email won level. Update sent emails
  • update history to add functioncall reply as completion
  • update wonLevel to true according to latest function call
• continue with updated sentEmails, completion, wonLevel, openAIErrorMessage. If problem [no reply content or openAiErrorMessage] then revert chatHistory
• mutate emails to add freshly sent ones
• if problems [no reply content or openAiErrorMessage]
  • mutate chat history to add error message then return error response
• else no problems
  • add bot message to history
  • mutate state according to updated history and emails
  • send response

What happens now when user sends a message on a high level, say sandbox?

• update history to include message and transformed message if there is one
• detect input defences
• get finalReply
  • while tool calls
  • update history according system role
  • update history to add completion as function call type.
  • if email function call
    • send email and get reply. see if email won level. Update sent emails
  • update history to add functioncall reply as completion
  • update wonLevel to true according to latest function call
• detect output defences
• if blocked restore original history and add user message as infoMessage, not completion
• if blocked
  • update history to add blocked message
  • mutate state according to updated history and emails
  • send response
• if problems [no reply content or openAiErrorMessage]
  • mutate chat history to add error message then return error response
• else not blocked and no problems
  • update history to add bot reply
  • mutate state according to updated history and emails
  • send response

[chriswilty]

@pmarsh-scottlogic I'm very happy with this now, good work. The linter rule is small beans, remove it if you wish - doesn't stop me approving.