-
-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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
Improved chat parsing with no AI logic #120
Conversation
thanks a lot for this PR. I would like to merge this but I need confirmation/feedback from @AntonOsika and anyone else who wants to chime in. I am thinking that we can merge this and remove it later if #94 gets done. I think #94 fixes this with functions though |
Looks good to me! |
The only thing is that I always want simpler solutions
Less code -> easier to change the repository later. What we could do: Use a system prompt as a final message that explicitly asks for the format we prefer. Let me know what you think @goncalomoita, but I'm ready to merge. |
I had a variety of issues with chat parsing, I made similar improvements including supporting directory creation in #140 |
Merging now since you agree, we can always simplify if possible later. Just to be clear I think #94 is the way to go, it is simpler imo and uses the model itself via functions to solve this issue. Until that is merged (if ever) this provides a solution for an issue that many users have at the moment. |
Thank you @goncalomoita |
This reverts commit 8facedd.
This reverts commit 8facedd.
This fixes compatibility issues in PR #87 that have arisen from recent changes to the repository.
Additionally, an improvement to the parser unit tests has been done.
The goal of the this parser is to make sense of what the LLM has provided without requiring another inference call which adds time and cost overhead. My remarks on this matter were stated in #35.
Further debate has been conducted in PR #94