streamline more code snippets, hopefully to make them shorter as well as easier to maintain #1010
Conversation
BBC-Esq
changed the title
BBC-Esq
changed the title
BBC-Esq
changed the title
| if arg_value.endswith("?download=true"): | ||
| arg_value = arg_value[:-14] # Remove "?download=true" | ||
|
|
||
| is_url = arg_value.startswith(("http://", "https://")) | ||
| valid_extensions = (".gguf", ".bin", ".safetensors") |
There was a problem hiding this comment.
This is WRONG!!! For example, mmproj cannot be a safetensors!
Are you using an LLM to refactor the code? Please do not do that without going through what the code does.
Language models CANNOT refactor code an accurate manner without human review..
Also why would you truncate a string by and offset when just replacing it is so much clearer and more intuitive?
| ("Help", "#992222", "#bb3333", display_help, 60, 1, 1, "sw", 135) | ||
| ] | ||
|
|
||
| for text, fg, hover, command, width, row, col, stick, padx in buttons: |
There was a problem hiding this comment.
Doing it this way is significantly less readable. In the original code, it's very clear what width means, e.g. 80 is referring to width. Now, by splitting it into an array of unnamed values, you even have a width but not the height. You have the x padding but not the y padding. It becomes super confusing.
| def display_updates(): | ||
| try: | ||
| import webbrowser as wb | ||
| wb.open("https://github.com/LostRuins/koboldcpp/releases/latest") | ||
| import webbrowser |
|
imho there is no need for such extensive changes through the entire file. Especially if using an LLM to rewrite the code - I can use Claude or other language models to do rewrites as well, but I don't usually do that, because that would necessitate extensive testing to ensure readability and functionality is not compromised. In this case, there are a lot of unnecessary and confusing changes, that have not been tested for correctness. And I mean tested - you can't just send the code to a language model and ask if it's correct. Thank you for your effort, but can I kindly request no more using LLMs to refactor the code, please. |
|
Yes, this pull request was created with AI assistance. I've been using AI primarily to write my code since I started with Python just over a year ago. All my repositories were created with it and they all work. :-) Also, just FYI, the three pull requests that were accepted by you were also written primarily by AI: #1007, #1006, and #1005. However, if you wouldn't appreciate any pull requests unless they're written completely free from AI assistance I'm afraid I won't be able to contribute...it's up to you. I do extensive testing on my own code and fully realize the effort it takes to some times make correct mistakes made by AI, and by no means to I simply feed code to AI without trying to understand and test what's produced...Let me know. Thanks! [EDIT]...Just to clarify, I could dial it back and not submit such omnibus pull requests, focus on smaller more testable changes, etc., but if you just categorically just don't want any AI assisted code I'm afraid I wouldn't be able to contribute...Thanks, let me know! |
|
I know, but the problem is that AI refactored code often contains subtle bugs that are not apparent without a detailed understanding of the code. For example, the suggested change assumes the same 3 accepted formats are allowed for all downloads when combining them into 1. This is not the case - each model type has it's own supported formats.
Yes, I was aware, and I also highlighted issues in two of them, which I accepted and fixed because they were relatively trivial ones.
I would prefer not to have AI written code, but there might be other ways AI could come in useful. For example, detecting and correcting phrasing or spelling errors in https://github.com/LostRuins/koboldcpp/wiki or the readme, or using it to (just) detect and highlight possible bugs in code.
Generally yes, I would keep pull requests focused on one specific feature or fix. For example now, henk is testing a new method of calculating the layers automatically for a model in #1012 , that requires a lot of manual testing and is not something you can ask a LLM to just do, and it will require further review and testing before merging. If you just tell ChatGPT or Claude to "refactor this into a better calculation for layers in a GGUF model" the resulting algorithm will likely not work as desired. |
|
I appreciate the explanation. Different people different feelings on AI generated code. You actually remind me of a "poker night" acquaintance of mind where we have good debates about AI and other issues. He's a 30 year SQL programmer who makes the big bucks. Ultimately, I don't want to contribute if it's not wanted and I understand that people like to develop in different ways. I, for example, am still stuck using Notepad++ and really should finally start using an IDE... ;-) I can still contribute if I see something that might be interesting, but will severely limit it to stuff where I know AI won't be making any mistakes like the ones you're describing. I might feel more confident in the future if I learn to compile the program so as to test any proposed modifications as well...Yet another thing I need to teach myself. Believe it or not I do appreciate the explanation though! Looking forward to creating this vectorDB plugin. Should have a rough draft out later today but I "plead the fifth" as to whether it'll be AI-assisted code. ;-) |
|
@BBC-Esq If you have a linux machine to test on koboldcpp.sh handles all the compiling for you on auto pilot. Its the simplest way to get it compiled, otherwise if you git cloned the repo it can build it for you in the actions tab. |
Thanks @henk717. Unfortunately, I don't have linux but do plan on attempting to compile again. I've failed every other time I've tried (not with kobold, other programs) but the instructions on kobold's repo are fairly good so...will try again. Good to meet you and nice to know that you and others are willing to help and share information either here or on the Discord! |
I strove to ensure that all modifications kept the same exact functionality, just hopefully streamlined. No new functions/helper functions were created. Tried to keep the flow of the script overall the same to avoid bugs.