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.

Sign up for GitHub

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

828 streamline network call for toggle defence #884

Merged
merged 4 commits into from
Mar 28, 2024
Merged

828 streamline network call for toggle defence #884

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
50bf3da
adds info message in backend
pmarsh-scottlogic Mar 28, 2024
06879d4
tests now check for adding info message
pmarsh-scottlogic Mar 28, 2024
654f557
frontend now uses generated info message
pmarsh-scottlogic Mar 28, 2024
503559f
Merge branch 'dev' into 828-streamline-network-call-for-toggle-defence
pmarsh-scottlogic Mar 28, 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
32 changes: 30 additions & 2 deletions backend/src/controller/defenceController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,21 @@ function handleDefenceActivation(req: DefenceActivateRequest, res: Response) {
defenceId,
currentDefences
);
res.status(200).send();

const displayedDefenceId = defenceId.replace(/_/g, ' ').toLowerCase();
const chatInfoMessage = {
infoMessage: `${displayedDefenceId} defence activated`,
chatMessageType: 'GENERIC_INFO',
} as ChatInfoMessage;

req.session.levelState[level].chatHistory = pushMessageToHistory(
req.session.levelState[level].chatHistory,
chatInfoMessage
);

res.send({
chatInfoMessage,
});
}

function handleDefenceDeactivation(req: DefenceActivateRequest, res: Response) {
Expand Down Expand Up @@ -96,7 +110,21 @@ function handleDefenceDeactivation(req: DefenceActivateRequest, res: Response) {
defenceId,
currentDefences
);
res.status(200).send();

const displayedDefenceId = defenceId.replace(/_/g, ' ').toLowerCase();
const chatInfoMessage = {
infoMessage: `${displayedDefenceId} defence deactivated`,
chatMessageType: 'GENERIC_INFO',
} as ChatInfoMessage;

req.session.levelState[level].chatHistory = pushMessageToHistory(
req.session.levelState[level].chatHistory,
chatInfoMessage
);

res.send({
chatInfoMessage,
});
}

function handleConfigureDefence(req: DefenceConfigureRequest, res: Response) {
Expand Down
10 changes: 6 additions & 4 deletions backend/test/integration/defenceController.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,10 @@ describe('The correct levels can have their defences changed', () => {
: levelState
);

expect(res.status).toHaveBeenCalledWith(200);
expect(res.send).toHaveBeenCalled();
expect(req.session.levelState).toEqual(newLevelState);
expect(req.session.levelState[level].defences).toEqual(
newLevelState[level].defences
);
}
);
});
Expand Down Expand Up @@ -139,9 +140,10 @@ describe('The correct levels can have their defences changed', () => {

handleDefenceDeactivation(req, res);

expect(res.status).toHaveBeenCalledWith(200);
expect(res.send).toHaveBeenCalled();
expect(req.session.levelState).toEqual(getInitialLevelStates());
expect(req.session.levelState[level].defences).toEqual(
getInitialLevelStates()[level].defences
);
}
);
});
Expand Down
34 changes: 30 additions & 4 deletions backend/test/unit/controller/defenceController.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ function responseMock() {
}

describe('handleDefenceActivation', () => {
test('WHEN passed sensible parameters THEN activates defence', () => {
test('WHEN passed sensible parameters THEN activates defence AND adds info message to history AND returns info message', () => {
const mockActivateDefence = activateDefence as jest.MockedFunction<
typeof activateDefence
>;
Expand All @@ -59,10 +59,12 @@ describe('handleDefenceActivation', () => {
config: [],
},
] as Defence[],
chatHistory: [] as ChatMessage[],
},
],
},
} as DefenceActivateRequest;
const res = responseMock();

const configuredDefences: Defence[] = [
{
Expand All @@ -73,7 +75,7 @@ describe('handleDefenceActivation', () => {
];
mockActivateDefence.mockReturnValueOnce(configuredDefences);

handleDefenceActivation(req, responseMock());
handleDefenceActivation(req, res);

expect(mockActivateDefence).toHaveBeenCalledTimes(1);
expect(mockActivateDefence).toHaveBeenCalledWith(
Expand All @@ -89,6 +91,17 @@ describe('handleDefenceActivation', () => {
expect(req.session.levelState[LEVEL_NAMES.SANDBOX].defences).toEqual(
configuredDefences
);

const expectedChatInfoMessage = {
infoMessage: 'prompt evaluation llm defence activated',
chatMessageType: 'GENERIC_INFO',
} as ChatInfoMessage;
expect(
req.session.levelState[LEVEL_NAMES.SANDBOX].chatHistory.at(-1)
).toEqual(expectedChatInfoMessage);
expect(res.send).toHaveBeenCalledWith({
chatInfoMessage: expectedChatInfoMessage,
});
});

test('WHEN missing defenceId THEN does not activate defence', () => {
Expand Down Expand Up @@ -200,7 +213,7 @@ describe('handleDefenceActivation', () => {
});

describe('handleDefenceDeactivation', () => {
test('WHEN passed sensible parameters THEN deactivates defence', () => {
test('WHEN passed sensible parameters THEN deactivates defence AND adds info message to history AND returns info message', () => {
const mockDeactivateDefence = deactivateDefence as jest.MockedFunction<
typeof deactivateDefence
>;
Expand All @@ -224,10 +237,12 @@ describe('handleDefenceDeactivation', () => {
config: [],
},
] as Defence[],
chatHistory: [] as ChatMessage[],
},
],
},
} as DefenceActivateRequest;
const res = responseMock();

const configuredDefences: Defence[] = [
{
Expand All @@ -238,7 +253,7 @@ describe('handleDefenceDeactivation', () => {
];
mockDeactivateDefence.mockReturnValueOnce(configuredDefences);

handleDefenceDeactivation(req, responseMock());
handleDefenceDeactivation(req, res);

expect(mockDeactivateDefence).toHaveBeenCalledTimes(1);
expect(mockDeactivateDefence).toHaveBeenCalledWith(
Expand All @@ -254,6 +269,17 @@ describe('handleDefenceDeactivation', () => {
expect(req.session.levelState[LEVEL_NAMES.SANDBOX].defences).toEqual(
configuredDefences
);

const expectedChatInfoMessage = {
infoMessage: 'prompt evaluation llm defence deactivated',
chatMessageType: 'GENERIC_INFO',
} as ChatInfoMessage;
expect(
req.session.levelState[LEVEL_NAMES.SANDBOX].chatHistory.at(-1)
).toEqual(expectedChatInfoMessage);
expect(res.send).toHaveBeenCalledWith({
chatInfoMessage: expectedChatInfoMessage,
});
});

test('WHEN missing defenceId THEN does not deactivate defence', () => {
Expand Down
22 changes: 11 additions & 11 deletions frontend/src/components/MainComponent/MainComponent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,23 +232,23 @@ function MainComponent({
}

async function setDefenceToggle(defence: Defence) {
await defenceService.toggleDefence(
const chatInfoMessage = await defenceService.toggleDefence(
defence.id,
defence.isActive,
currentLevel
);

const newDefenceDetails = defences.map((defenceDetail) => {
if (defenceDetail.id === defence.id) {
defenceDetail.isActive = !defence.isActive;
const action = defenceDetail.isActive ? 'activated' : 'deactivated';
const infoMessage = `${defence.name} defence ${action}`;
addInfoMessage(infoMessage.toLowerCase());
}
return defenceDetail;
});
if (chatInfoMessage) {
addChatMessage(chatInfoMessage);
const newDefenceDetails = defences.map((defenceDetail) => {
if (defenceDetail.id === defence.id) {
defenceDetail.isActive = !defence.isActive;
}
return defenceDetail;
});

setDefences(newDefenceDetails);
setDefences(newDefenceDetails);
}
}

async function setDefenceConfiguration(
Expand Down
9 changes: 7 additions & 2 deletions frontend/src/service/defenceService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ async function toggleDefence(
defenceId: DEFENCE_ID,
isActive: boolean,
level: LEVEL_NAMES
): Promise<boolean> {
): Promise<ChatMessage | null> {
const requestPath = isActive ? 'deactivate' : 'activate';
const response = await sendRequest(`${PATH}${requestPath}`, {
method: 'POST',
Expand All @@ -50,7 +50,12 @@ async function toggleDefence(
},
body: JSON.stringify({ defenceId, level }),
});
return response.status === 200;
if (response.status !== 200) return null;

const { chatInfoMessage } =
(await response.json()) as ChatInfoMessageResponse;

return makeChatMessageFromDTO(chatInfoMessage);
Copy link
Member

@chriswilty chriswilty Mar 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lovely thing about returning a ChatMessage from all these service functions is that it'd be really easy to return an "error" chat message to go into the chat panel, instead of null to be ignored. If only we had time....

}

async function configureDefence(
Expand Down
Loading