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: move calls to LLM into CdGpt services #196

Draft
wants to merge 1 commit into
base: 139-ts-migration
Choose a base branch
from

Conversation

davidzlu
Copy link
Collaborator

@davidzlu davidzlu commented Nov 19, 2024

Note: entry services needs to be refactored to use CdGpt services before this PR can be merged.

Moved the functions getAnalysisContent and getChatContent from the models into a service layer above the model. The reasoning for this is to have a call to an external API encapsulated in a module that's separate from the EntryAnalysis and EntryConversation models so that they don't depend on it.

Unit tests for these functions were also moved into a new test file, models/services/CdGpt.test.ts

Closes #173
Closes #174

Moved the functions getAnalysisContent and getChatContent from the models
into a service layer above the model. The reasoning for this is to
have a call to an external API encapsulated in a module that's separate
from the EntryAnalysis and EntryConversation models so that they
don't depend on it.

Unit tests for these functions were also moved into a new test file,
models/services/CdGpt.test.ts
@davidzlu davidzlu added the refactor Code refactoring label Nov 19, 2024
@davidzlu davidzlu requested a review from hiyaryan November 19, 2024 01:51
@davidzlu davidzlu self-assigned this Nov 19, 2024
@hiyaryan
Copy link
Member

hiyaryan commented Nov 23, 2024

For the record, this refactor will be made after the TS migration as discussed in our previous CDJ Standup #12. Note that this might open a larger PR addressing SSE in Issue #57.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Code refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants