CodeGPT icon indicating copy to clipboard operation
CodeGPT copied to clipboard

chore: Convert actions to Kotlin

Open reneleonhardt opened this issue 1 year ago • 9 comments

I suggest streamlining all actionPerformed:

  • Send Telemetry if nothing happened?
  • Show dialog if action is not possible?
  • Throw NPE with requireNonNull() or handle null accordingly?

reneleonhardt avatar Apr 20 '24 19:04 reneleonhardt

@carlrobertoh I converted conversations on top of this branch, any chance that you can merge this first? Another 29 files changed, still 120 to go 😅

reneleonhardt avatar Apr 21 '24 13:04 reneleonhardt

I haven't had a chance to review this PR yet, but I'll try to do it sometime early next week. Also, I'll probably do the next release without these refactorings because our test coverage is bad, and I don't want to risk breaking anything.

carlrobertoh avatar Apr 21 '24 14:04 carlrobertoh

Sounds good 👍

reneleonhardt avatar Apr 21 '24 14:04 reneleonhardt

Speaking of which, please try Llama 3 locally first before releasing 😅

reneleonhardt avatar Apr 21 '24 14:04 reneleonhardt

The response seems a bit off. Also, the success callback isn't being triggered once the server is up and running, so it looks like it's booting up forever.

llama3

carlrobertoh avatar Apr 21 '24 14:04 carlrobertoh

Was able to fix the response by modifying the prompt and adding a stop token.

llama3_update

carlrobertoh avatar Apr 21 '24 19:04 carlrobertoh

Thank you for testing and fixing! Stream was not the problem, begin_of_text isn't needed, but assistant, stopTokens and server hack (?). Are other models running with msg instead of message, is the hack "official"?

reneleonhardt avatar Apr 22 '24 05:04 reneleonhardt

Thank you for testing and fixing! Stream was not the problem, begin_of_text isn't needed, but assistant, stopTokens and server hack (?).

I followed the template format from their official guide. Even if it's not required, I'd still like to have it included unless it breaks something.

Are other models running with msg instead of message, is the hack "official"?

I called it a hack because the way we detect if the server is up and running is through the server logs. I assume they changed their logging structure (from message to msg), and that broke our trigger condition.

carlrobertoh avatar Apr 22 '24 08:04 carlrobertoh

Hey, I'm putting this refactoring on pause for a bit until I have run a compatibility comparison between different IDE builds. Most likely, this won't cause any issues, but I just want to be sure, since I'm slowly considering supporting older versions again.

carlrobertoh avatar Apr 25 '24 10:04 carlrobertoh