Feature/better console log detection #413
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Watermelon AI SummaryThe Pull Request implements a refined approach to console log detection by evolving a prototype regex pattern to achieve a balanced level of sensitivity. It replaces an initial AI-based method, ensuring logs are identified correctly without being too inclusive or exclusive, while also streamlining and cleaning up the codebase for this feature. GitHub PRs
Click here to login to Jira No results found in Linear Tickets :( Click here to login to Asana |
![watermelon-copilot-for-code-review[bot]](https://avatars.githubusercontent.com/in/327523?s=60&v=4){kind=small}
| ) | ||
| .catch((err) => { | ||
| console.log(err); | ||
| }); |
There was a problem hiding this comment.
This PR contains console logs. Please review or remove them.
There was a problem hiding this comment.
Ty! This might be over-informative.
| const splitAdditions = additions.split("\n"); | ||
|
|
||
| // RegEx to detect console log equivalents in different languages | ||
| const consoleLogRegex = /(console\.log|print|printf|fmt\.Print|log\.Print|NSLog|puts|println|println!)\([^)]*\)(?![^]*?\/\/|[^]*?\/\*|#)/; |
There was a problem hiding this comment.
Could we make an external list?
Adding to a single line seems harder
There was a problem hiding this comment.
What do you mean by an external list, exactly?
There was a problem hiding this comment.
| const consoleLogRegex = /(console\.log|print|printf|fmt\.Print|log\.Print|NSLog|puts|println|println!)\([^)]*\)(?![^]*?\/\/|[^]*?\/\*|#)/; | |
| const consoleLogTexts = ["console.log", "printLn"] |
There was a problem hiding this comment.
And then we use a template string to add it to the Regex
There was a problem hiding this comment.
It's a valid suggestion. If I'm free of tasks during the holidays I'll improve the readability. Soft promise.
| if (!currentLine.match(commentRegex) && currentLine.match(consoleLogRegex)) { | ||
| const commentFileDiff = async () => { | ||
|
|
||
| const consoleLogPosition = i + 1; // The +1 is because IDEs and GitHub file diff view index LOC at 1, not 0 |
There was a problem hiding this comment.
I know this is fast, but should we do it concurrently?
There was a problem hiding this comment.
Also, this generates a comment in the last consle log, isn't it better to show it at the top of the PR?
There was a problem hiding this comment.
Steps to replicate?
I'm testing with this fizz buzz and it's commenting lines 6,8, and 10 respectively
There was a problem hiding this comment.
I know this is fast, but should we do it concurrently?
Why would we want to do it concurrently? It's a pretty straightforward iteration of an array in O(n) to me. But I'd like to read your opinion.
There was a problem hiding this comment.
For really long file diffs it will be a long running process, awaiting each time the posting of the comment.
As per the API we can send an object and never hit the rate limit.
There was a problem hiding this comment.
In the very rare edge case that we get a very long file diff where every single line should be commented, I still don't think we'd have problems.
My argument for this is that when doing this with GPT, we were doing the very same thing with the additional computational complexity of async/awaiting the http call to Open AI which is an expensive one.
If we ever get to the rate limit we can always refactor this.
Description
LLMs didn't prove to be the correct approach towards console log detection.
They were a fast way to test interest and some initial cases. But since this is a key need users have, we need to run these through RegExes to make the console log detection not too lax, and not too strict.
I tested this with PR #412
Type of change
Notes
Acceptance