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

Jump to bottom

Initial plugins code cleanup #3120

Merged
merged 13 commits into from
May 19, 2023
Merged

Initial plugins code cleanup #3120

merged 13 commits into from
May 19, 2023

Conversation

olliestanley
Copy link
Collaborator

@olliestanley olliestanley commented May 10, 2023 •
edited
Loading

The primary contributions of this PR are:

  • Introduce PromptedLLM class to manage the repeated process of preparing prompt with memory and template, calling LLM, and postprocessing output
  • Separating out functions in openapi_parser
  • General improvements to clarity/readability

@olliestanley olliestanley marked this pull request as ready for review May 11, 2023 18:53
@olliestanley olliestanley mentioned this pull request May 11, 2023
Closed
@olliestanley olliestanley marked this pull request as draft May 12, 2023 19:05
@olliestanley olliestanley marked this pull request as ready for review May 12, 2023 20:15
AbdBarho
AbdBarho reviewed May 13, 2023
View reviewed changes
Copy link
Collaborator

@AbdBarho AbdBarho left a comment

Choose a reason for hiding this comment

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

I feel abit unqualified to review the internals of the plugin system, but overall it looks good.

@draganjovanovich
Copy link
Collaborator

Nice job!
LGTM,
But I would love to manually test this branch a bit more before merging if possible?

@olliestanley
Copy link
Collaborator Author

Nice job! LGTM, But I would love to manually test this branch a bit more before merging if possible?

Sure, no problem. Let me know when you're happy for it to be merged

@olliestanley olliestanley linked an issue May 13, 2023 that may be closed by this pull request
Use of phrases including "Open Assistant" causes generation to stop with plugins enabled #3146
Closed
@olliestanley
Copy link
Collaborator Author

@draganjovanovich the last commit I have pushed to this branch should address #3146. Please let me know if you think there is any problem with this solution.

@draganjovanovich
Copy link
Collaborator

@draganjovanovich the last commit I have pushed to this branch should address #3146. Please let me know if you think there is any problem with this solution.

yes, yes. I had that in my local repo, but left it uncommitted. Thanks.

@draganjovanovich
Copy link
Collaborator

I tested it on a local setup, and everything seems fine.

@olliestanley olliestanley enabled auto-merge (squash) May 15, 2023 15:58
@draganjovanovich
Copy link
Collaborator

I have some more improvements for plugins that would like to open PR, but It would be best if that could be merged before that?

@olliestanley olliestanley merged commit 1a14d9d into main May 19, 2023
@olliestanley olliestanley deleted the cleanup-plugins branch May 19, 2023 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@draganjovanovich draganjovanovich draganjovanovich left review comments

@AbdBarho AbdBarho AbdBarho left review comments

@yk yk yk approved these changes

@andreaskoepf andreaskoepf Awaiting requested review from andreaskoepf andreaskoepf is a code owner

@melvinebenezer melvinebenezer Awaiting requested review from melvinebenezer melvinebenezer is a code owner

Assignees
No one assigned
Labels
inference
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use of phrases including "Open Assistant" causes generation to stop with plugins enabled
4 participants
@olliestanley @draganjovanovich @yk @AbdBarho