Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor: Shift the logic for checking win condition #780

Closed
pmarsh-scottlogic opened this issue Jan 18, 2024 · 1 comment · Fixed by #876
Closed

Refactor: Shift the logic for checking win condition #780

pmarsh-scottlogic opened this issue Jan 18, 2024 · 1 comment · Fixed by #876
Assignees
Labels
backend Requires work on the backend question Further information is requested refactor Improve code quality triage New tickets to be checked by the maintainers

Comments

@pmarsh-scottlogic
Copy link
Contributor

pmarsh-scottlogic commented Jan 18, 2024

Question: do we want this? I think it will make things easier to follow.

this goes along with #761. Do it before or after, but not at the same time.

Description

summary: move the win condition logic out from deep within the call stack, to encourage a more flat structure, and be better for single responsibility.

Right now, a user sends a message, and to see if that message has caused the level to be completed, we have to look right at the bottom of the call stack: handleChatToGPT(...) => handleLow[orHigher]LevelChat(...) => chatGptSendMessage(...) => getFinalReplyAfterAllToolCalls(...) => performToolCalls(...) => chatGptCallFunction(...) => sendEmail(...) => checkLevelWinCondition(...).

I think we should separate the logic for checking the win condition from the logic that deals with processing the user's message and getting a reply. Something like

function handleChatToGPT(...) {
  const reply = handleLow[orHigher]LevelChat(...); // reply object includes a list of sent emails
  chatReponse.wonLevel = checkLevelWinCondition(reply.sentEmails);
  res.send(chatResponse);

Which will leave chatGptSendMessage(...) responsible for one fewer thing. Solid.

Acceptance Criteria

Regressions on winning the level:

GIVEN each level
WHEN you send an email that would win the level
THEN the level is won

GIVEN each level
WHEN you try to send an email that would win the level, but the message is blocked by any defence (input or output)
THEN the level is not won

GIVEN each level
WHEN you try to send an email that would win the level, but there is a problem in the openAI API when getting a reply following a tool call*
THEN the level is won

*for example:

Image

(to do this, you will have to mock the openai library throwing an error. See below)

Mocking an OpenAI API error directly after tool call

Go to backend/src/openai.ts. Go to the chatGptChatCompletion method and go to the try/catch statement. At the top of the try block, paste this code:

const triggerString = '!!';
const mostRecentUserMessage = chatHistory
	.filter((chatMessage) => {
		return chatMessage.chatMessageType === 'USER';
	})
	.at(-1);
const mostRecentUserMessageContainsTrigger =
	mostRecentUserMessage &&
	'completion' in mostRecentUserMessage &&
	mostRecentUserMessage.completion?.content
		?.toString()
		.includes(triggerString);
const mostRecentMessageIsToolCall =
	chatHistory.at(-1)?.chatMessageType === 'FUNCTION_CALL';
if (mostRecentUserMessageContainsTrigger && mostRecentMessageIsToolCall) {
	throw new Error('Mock openai error');
}

Now if you want to mock an OpenAI error directly after a tool call, just include "!!" somewhere in your message

@pmarsh-scottlogic pmarsh-scottlogic added question Further information is requested backend Requires work on the backend refactor Improve code quality triage New tickets to be checked by the maintainers labels Jan 18, 2024
@pmarsh-scottlogic pmarsh-scottlogic changed the title Refactor: Shift the code for checking win condition Refactor: Shift the logic for checking win condition Jan 18, 2024
@pmarsh-scottlogic pmarsh-scottlogic self-assigned this Mar 22, 2024
@pmarsh-scottlogic pmarsh-scottlogic linked a pull request Mar 25, 2024 that will close this issue
3 tasks
@pmarsh-scottlogic
Copy link
Contributor Author

this might be able to skip testing? Or maybe just regressions on winning a level. Test also winning a level when the user's message is blocked

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Requires work on the backend question Further information is requested refactor Improve code quality triage New tickets to be checked by the maintainers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant