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

Reorganise backend modules used for chat #761

Open
pmarsh-scottlogic opened this issue Jan 12, 2024 · 0 comments
Open

Reorganise backend modules used for chat #761

pmarsh-scottlogic opened this issue Jan 12, 2024 · 0 comments
Labels
backend Requires work on the backend refactor Improve code quality triage New tickets to be checked by the maintainers

Comments

@pmarsh-scottlogic
Copy link
Contributor

pmarsh-scottlogic commented Jan 12, 2024

Feature Request

Completing this will greatly ease unit testing and documentation, which are two priorities for the end of the project.

Description

Lil' refactor to separate concerns and hopefully ease testing.

At the moment we have chatController.ts and openai.ts modules which contain the bulk of the logic for chatting with the chatbot. Both files are a bit bloated and seperation of concerns is wish-washy.

I propose that we introduce a new module chatService.ts, so that the concerns are separated into 3 layers (plus utils, introduced in #740 ):

  • chatController.ts deals with the API side of things. It validates the request parameters and constructs a sensible response based on how the chat goes (including unhappy paths).
  • chatService.ts contains the business logic for most of the interaction. Its job it to take an (already validated) message from the user, and get a response, including dealing with the tool calls faff.
  • openai.ts is only concerned with directly interfacing with the openAI API.

In concrete terms, I reckon we should:

  • Move handleLowLevelChat, handleHigherLevelChat and about half of the logic in handleChatToGPT to a fresh module chatService.ts.
  • Move performToolCalls, getFinalReplyAfterAllToolCalls, chatGptSendMessage also to chatService.ts.
  • Move getChatCompletionsFromHistory, pushCompletionToHistory, getBlankChatResponse to utils/chat.ts (introduced in 708 move logic for detecting output defence bot filtering #740 )

After all this it should be easier to unit test the exported methods from each layer.

NB: AFter a bunch of backend refactoring (#740, #743, #758) some of the above details will change, but the general gist is the same

Acceptance criteria

Refactor, so only regression testing.

Question: Do we want this, or does the benefit not outweigh the effort?

@pmarsh-scottlogic pmarsh-scottlogic added enhancement New feature or request triage New tickets to be checked by the maintainers backend Requires work on the backend refactor Improve code quality and removed enhancement New feature or request labels Jan 12, 2024
@pmarsh-scottlogic pmarsh-scottlogic changed the title Add service for chatController Reorganise backend modules used for chat Jan 22, 2024
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 refactor Improve code quality triage New tickets to be checked by the maintainers
Projects
None yet
Development

No branches or pull requests

1 participant