Skip to content

refactored LLM calls for maintenance, future additions#60

Open
CCranney wants to merge 1 commit intoSamuelSchmidgall:mainfrom
CCranney:main
Open

refactored LLM calls for maintenance, future additions#60
CCranney wants to merge 1 commit intoSamuelSchmidgall:mainfrom
CCranney:main

Conversation

@CCranney
Copy link

Hi,

Thank you so much for putting together this package, it has been a delight to use. I've been using the 'deepseek-chat' LLM, and noticed a couple of bugs, as it is a newer feature. I looked over the code, and thought adding some design patterns might make adding additional LLM's in the future an easier process.

I've written several classes using the Strategy and Factory design patterns. The idea is to make adding a new LLM (or updating an existing one) as painless as possible, without running the risk of introducing errors to code you already know works. I've added some unit tests as well.

I ran the code in this PR through a full pipeline using deepseek, but do not have much access to OpenAI APIs, so have not been able to test that.

*Note: I have ideas for additional refactoring that could make running, debugging and testing the code easier, but figure this is a good place to start.
**Note: Much of the refactored code was originally generated using Claude 3.5 Sonnet, though I reviewed it and made several edits as per the needs of the program.

@CCranney
Copy link
Author

I should also note I found a pre-existing bug in the report refinement portion of the code. The model defaults to chatGPT-4 in the get_score, I believe, though I haven't discovered why the model provided as a function parameter does not override it yet. I bypassed the error by hard-coding 'deepseek-chat' right before the reviewers take their turn for the time being, but that's something to look out for.

@CCranney
Copy link
Author

I did run through the open PRs briefly, and ones like #10, #29, #35, #40 would all be helped with this kind of change.

@CCranney
Copy link
Author

Also #64

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant