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

707 refactor of chatgptchatcompletion #758

Merged
merged 41 commits into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
dab6741
removes unneeded eslint disabling
pmarsh-scottlogic Jan 11, 2024
9c04d11
moves setting system role into its own method and improves if statmen…
pmarsh-scottlogic Jan 11, 2024
2540b83
removes mutation from within setSystemRoleInChatHistory
pmarsh-scottlogic Jan 11, 2024
40e2da6
renames variable, plus moves prinst statement back to the main method
pmarsh-scottlogic Jan 11, 2024
c72b33b
sets system role in chat history in chatController
pmarsh-scottlogic Jan 11, 2024
59b6770
updates props in openai that were needed for setting system role
pmarsh-scottlogic Jan 11, 2024
a279310
adds placeholder unit tests for setSystemRoleInChatHistory
pmarsh-scottlogic Jan 11, 2024
c42f55b
tidies test file
pmarsh-scottlogic Jan 11, 2024
8647792
adds skeleton tests with fleshed out names
pmarsh-scottlogic Jan 11, 2024
965a6df
write first unit test
pmarsh-scottlogic Jan 11, 2024
b54ce11
added extra mock and finished another test
pmarsh-scottlogic Jan 11, 2024
a166f8c
finishes adding unit tests for setSystemRoleInChatHistory
pmarsh-scottlogic Jan 11, 2024
97a2eac
removes big comment
pmarsh-scottlogic Jan 11, 2024
57069c9
tests that we set the system role when we chat with gpt
pmarsh-scottlogic Jan 11, 2024
21ecaa5
removes tests for setting system role from integration/openai.test
pmarsh-scottlogic Jan 11, 2024
f8d6819
simplifies bool expression
pmarsh-scottlogic Jan 17, 2024
a34e494
removes unneeded mock
pmarsh-scottlogic Jan 17, 2024
5f352b5
remove some extra fluff
pmarsh-scottlogic Jan 17, 2024
2cad40f
merge dev
pmarsh-scottlogic Jan 17, 2024
8510a5d
moves beforeEach into relevant describe block
pmarsh-scottlogic Jan 17, 2024
b378013
improves test name
pmarsh-scottlogic Jan 17, 2024
8e8f9e2
moves afterEach into relevant describe
pmarsh-scottlogic Jan 17, 2024
171e07b
moves mockOpenAI into describe block and simplifies the mock
pmarsh-scottlogic Jan 17, 2024
cb7a28c
cleans up mocking of @src/defence
pmarsh-scottlogic Jan 17, 2024
ad00a0e
more tidying to mock getSystemRole
pmarsh-scottlogic Jan 17, 2024
6fcf3a8
remove mock from variable names in test
pmarsh-scottlogic Jan 17, 2024
b5a93d7
moves some variables into describe block
pmarsh-scottlogic Jan 17, 2024
c78308f
removes further unnecessary mocks
pmarsh-scottlogic Jan 17, 2024
a896428
removes unit test frofom dwescribe block names
pmarsh-scottlogic Jan 17, 2024
3af68f7
runs linter
pmarsh-scottlogic Jan 17, 2024
9d805b9
merge dev
pmarsh-scottlogic Jan 23, 2024
cde1894
mocks setSystemRoleInChatHistory in chatController unit tests to fix …
pmarsh-scottlogic Jan 23, 2024
1560ea5
moves setSystemRoleInChatHistory to utils file
pmarsh-scottlogic Jan 23, 2024
8c608a4
changes chat utils unit tests, removes descirbe block and renames to …
pmarsh-scottlogic Jan 23, 2024
643f906
fixes imports in chatController unit tests
pmarsh-scottlogic Jan 23, 2024
2ecb735
does way too many things for one commit
pmarsh-scottlogic Jan 23, 2024
436b322
removes mutation to set system role
pmarsh-scottlogic Jan 23, 2024
b0ee499
don't check that system role set method is called
pmarsh-scottlogic Jan 23, 2024
6632976
adds unit test to chatController for dealing with blocked ouput
pmarsh-scottlogic Jan 23, 2024
36bc014
mocks library method in better way
pmarsh-scottlogic Jan 25, 2024
8148127
corrects import
pmarsh-scottlogic Jan 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions backend/src/controller/chatController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
defaultChatModel,
} from '@src/models/chat';
import { LEVEL_NAMES } from '@src/models/level';
import { chatGptSendMessage } from '@src/openai';
import { chatGptSendMessage, setSystemRoleInChatHistory } from '@src/openai';

import { handleChatError } from './handleError';

Expand All @@ -29,7 +29,6 @@ async function handleLowLevelChat(
currentLevel: LEVEL_NAMES,
chatModel: ChatModel
) {
// get the chatGPT reply
const openAiReply = await chatGptSendMessage(
req.session.levelState[currentLevel].chatHistory,
req.session.levelState[currentLevel].defences,
Expand Down Expand Up @@ -177,13 +176,19 @@ async function handleChatToGPT(req: OpenAiChatRequest, res: Response) {
? req.session.chatModel
: defaultChatModel;

req.session.levelState[currentLevel].chatHistory = setSystemRoleInChatHistory(
currentLevel,
req.session.levelState[currentLevel].defences,
req.session.levelState[currentLevel].chatHistory
);

// record the history before chat completion called
const chatHistoryBefore = [
...req.session.levelState[currentLevel].chatHistory,
];
try {
// skip defence detection / blocking for levels 1 and 2 - sets chatResponse obj
if (currentLevel < LEVEL_NAMES.LEVEL_3) {
// skip defence detection / blocking for levels 1 and 2 - sets chatResponse obj
await handleLowLevelChat(
req,
message,
Expand Down
76 changes: 39 additions & 37 deletions backend/src/openai.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,50 +218,55 @@ async function chatGptCallFunction(
};
}

async function chatGptChatCompletion(
chatResponse: ChatResponse,
chatHistory: ChatHistoryMessage[],
function setSystemRoleInChatHistory(
currentLevel: LEVEL_NAMES,
defences: Defence[],
chatModel: ChatModel,
openai: OpenAI,
// default to sandbox
// eslint-disable-next-line @typescript-eslint/no-unused-vars
currentLevel: LEVEL_NAMES = LEVEL_NAMES.SANDBOX
chatHistory: ChatHistoryMessage[]
) {
// check if we need to set a system role
// system role is always active on levels
if (
currentLevel !== LEVEL_NAMES.SANDBOX ||
isDefenceActive(DEFENCE_ID.SYSTEM_ROLE, defences)
) {
const systemRoleNeededInChatHistory =
currentLevel === LEVEL_NAMES.SANDBOX
? isDefenceActive(DEFENCE_ID.SYSTEM_ROLE, defences)
: true;
pmarsh-scottlogic marked this conversation as resolved.
Show resolved Hide resolved

if (systemRoleNeededInChatHistory) {
const completionConfig: ChatCompletionSystemMessageParam = {
role: 'system',
content: getSystemRole(defences, currentLevel),
};

// check to see if there's already a system role
const systemRole = chatHistory.find(
const existingSystemRole = chatHistory.find(
(message) => message.completion?.role === 'system'
);
if (!systemRole) {
// add the system role to the start of the chat history
chatHistory.unshift({
completion: completionConfig,
chatMessageType: CHAT_MESSAGE_TYPE.SYSTEM,
});
if (!existingSystemRole) {
return [
{
completion: completionConfig,
chatMessageType: CHAT_MESSAGE_TYPE.SYSTEM,
},
...chatHistory,
];
} else {
// replace with the latest system role
systemRole.completion = completionConfig;
return chatHistory.map((message) => {
if (message.completion?.role === 'system') {
return { ...existingSystemRole, completion: completionConfig };
} else {
return message;
}
});
}
} else {
// remove the system role from the chat history
while (
chatHistory.length > 0 &&
chatHistory[0].completion?.role === 'system'
) {
chatHistory.shift();
}
return chatHistory.filter(
(message) => message.completion?.role !== 'system'
);
pmarsh-scottlogic marked this conversation as resolved.
Show resolved Hide resolved
}
}

async function chatGptChatCompletion(
chatResponse: ChatResponse,
chatHistory: ChatHistoryMessage[],
chatModel: ChatModel,
openai: OpenAI
) {
console.debug('Talking to model: ', JSON.stringify(chatModel));

// get start time
Expand Down Expand Up @@ -449,10 +454,8 @@ async function getFinalReplyAfterAllToolCalls(
let reply = await chatGptChatCompletion(
chatResponse,
chatHistory,
defences,
chatModel,
openai,
currentLevel
openai
);

// check if GPT wanted to call a tool
Expand All @@ -477,10 +480,8 @@ async function getFinalReplyAfterAllToolCalls(
reply = await chatGptChatCompletion(
chatResponse,
chatHistory,
defences,
chatModel,
openai,
currentLevel
openai
);
}

Expand Down Expand Up @@ -551,4 +552,5 @@ export {
chatGptSendMessage,
getOpenAIKey,
getValidModelsFromOpenAI,
setSystemRoleInChatHistory,
};
6 changes: 6 additions & 0 deletions backend/test/integration/chatController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { ChatHistoryMessage, ChatModel } from '@src/models/chat';
import { Defence } from '@src/models/defence';
import { EmailInfo } from '@src/models/email';
import { LEVEL_NAMES, LevelState } from '@src/models/level';
import * as openaiModule from '@src/openai';

declare module 'express-session' {
interface Session {
Expand Down Expand Up @@ -152,12 +153,17 @@ describe('handleChatToGPT integration tests', () => {
const req = openAiChatRequestMock('Hello chatbot', LEVEL_NAMES.LEVEL_1);
const res = responseMock();

const mockedSetSystemRoleInChatHistory = jest.spyOn(
openaiModule,
'setSystemRoleInChatHistory'
);
mockCreateChatCompletion.mockResolvedValueOnce(
chatResponseAssistant('Howdy human!')
);

await handleChatToGPT(req, res);

expect(mockedSetSystemRoleInChatHistory).toHaveBeenCalled();
pmarsh-scottlogic marked this conversation as resolved.
Show resolved Hide resolved
expect(res.send).toHaveBeenCalledWith({
reply: 'Howdy human!',
defenceReport: {
Expand Down
Loading