https://www.augmentcode.com/blog/we-benchmarked-7-ai-code-re...
has anyone tried it?
231–240 of 265 posts
https://www.augmentcode.com/blog/we-benchmarked-7-ai-code-re...
has anyone tried it?
Greptile is a great product and I hope you succeed. However, I disagree that independence is a competitive advantage. If it’s true that having a “firewall” between the coding agent and review agent leads to better code, I don’t see why a company like Cursor can’t create full independence between their coding and review products but still bundle them together for distribution. Furthermore, there might well be benefits…
Earlier quoted context omitted.
They’re both equally bad to me, I don’t see the improvement over just using item.count. I may be nitpicking a toy example though.
I think in this case itemCount had application in a couple of conditions later in the function, so there was value in extracting the count. In my recollection I might be missing some nuance, lets say for the sake of argument it was: var relevantCount = items.Where(x => x.SomeValue > 5); vs var numberOfRelevantItems = items.Where(x => x.SomeValue > 5); so it wasn't necessarily cheap enough to want to repeat.
Here though you're missing the ".Count", so
var relevantItems = items.Where(x => x.SomeValue > 5);
relevantItems.Count
As long as it's not a property that's calculated every time it's accessed, this still seems better than pulling the count into its own variable to me.My experience with using AI tools for code review is that they do find critical bugs (from my retrospective analysis, maybe 80% of the time), but the signal to noise ratio is poor. It's really hard to get it not to tell you 20 highly speculative reasons why the code is problematic along with the one critical error. And in almost all cases, sufficient human attention would also have identified the critical bug - so hu…
The signal-to-noise ratio problem is unexpectedly difficult. We wrote about our approach to it some time ago here - https://www.greptile.com/blog/make-llms-shut-up Much has changed on our approach since then, so we'll probably write a a new blog post. The tl;dr of what makes it hard is - different people have different ideas of what a nitpick is - it's not a spectrum, the differences are qualitative - LLMs are reluct…
Earlier quoted context omitted.
Human comments tend to be short and sweet like "nit: rename creatorOfWidgets to widgetFactory". Whereas AI code review comments are long winded not as precise. So even if there are 20 humans comments, I can easily see which are important and which aren't.
it "nit" short for nitpick? I think prefixing PR comments with prefixes like that is very helpful for dealing with this problem.
It may feels to many. I mostly use suggestion, thought, and todo. When I type down "nit..." I realized it usually does not worth it. I'd rather make comment about higher level of the changes.
Earlier quoted context omitted.
You mention the tools you can use to make it happen. I think we're at the point where you need concrete examples to talk about whether it's worth it or not. If you have functions that can't be called twice, then you have no other option to test details in the implementation like that. Yeah there's a tradeoff between torturing your code to make everything about it testable and enforce certain behavior or keeping it si…
In functions that you write, that might be possible. How would you assert that a given std::vector only was filtered by std::ranges::copy_if once? And how would you test that the code that was in the predicate for it wasn't duplicated? How would you write a failing test for this function keeping the constraint that you are working with std::vector? std::vector doThing(const std::vector & nums) { std::vector tmp1; std…
Maybe dependency injection and function pointers for the copy if function. Then you can check the call counts in your tests. But idk the cpp eco system and what's available there to do it.
Earlier quoted context omitted.
at my last job code review was done directly in your editor (with tooling to show you diffs as well). What this meant was that instead of leaving nitpicky comments, people would just change things that were nitpicky but clear improvements. They'd only leave comments (which blocked release) for stuff that was interesting enough to discuss. This was typically a big shock for new hires who were used to the "comment for…
Was that Jane Street? I remember watching a presentation from someone there about such a system. If not, any chance this tooling is openly available?
Earlier quoted context omitted.
In functions that you write, that might be possible. How would you assert that a given std::vector only was filtered by std::ranges::copy_if once? And how would you test that the code that was in the predicate for it wasn't duplicated? How would you write a failing test for this function keeping the constraint that you are working with std::vector? std::vector doThing(const std::vector & nums) { std::vector tmp1; std…
I know how I would do it in python. This is built into the stdlibs testing library, with mocks. Maybe dependency injection and function pointers for the copy if function. Then you can check the call counts in your tests. But idk the cpp eco system and what's available there to do it.
def some_call():
return [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
def print_evens(nums):
for n in filter(lambda n: n % 2 == 0, nums):
print(n)
def func():
filtered = list(filter(lambda n: n % 2 == 0, some_call()))
print_evens(filtered)
if __name__ == "__main__":
func()
How would you write a failing test that prevents the list from some_call() from having the same filter applied to it twice?Earlier quoted context omitted.
at my last job code review was done directly in your editor (with tooling to show you diffs as well). What this meant was that instead of leaving nitpicky comments, people would just change things that were nitpicky but clear improvements. They'd only leave comments (which blocked release) for stuff that was interesting enough to discuss. This was typically a big shock for new hires who were used to the "comment for…
That sounds great. Was that proprietary tooling? I'd be interested in some such thing.
Gitlab has this functionality right in the web UI. Reviewers can suggest changes, and if the PR author approves, a commit is created with the suggested change. One issue with this flow it that's it doesn't run any tests on the change before it's actually in the PR branch, so... Really best for typos and other tiny changes.
Alternatively you actually, you know, _collaborate_ with the PR author, work it out, run tests locally and/or on another pushed branch, and someone then pushes a change directly to the PR.
The complaints about nitpicks slowing things down too much or breaking things sound like solo-hero devs who assume their god-like PRs should be effectively auto-approved because how could their code even contain problems... No wonder they love working with "Dr Flattery the Always Wrong Bot".
*(Hilarious name borrowed from Angela Collier)
My experience with using AI tools for code review is that they do find critical bugs (from my retrospective analysis, maybe 80% of the time), but the signal to noise ratio is poor. It's really hard to get it not to tell you 20 highly speculative reasons why the code is problematic along with the one critical error. And in almost all cases, sufficient human attention would also have identified the critical bug - so hu…
For the signal to noise reason, I start with Claude Code reviewing a PR. Then I selectively choose what I want to bubble up to the actual review. Often times, there's additional context not available to the model or it's just nit picky.
Sounds incredibly pointless. But at least you're spending those tokens your boss was forced to buy so the board can tell the investors that they've jumped on the bandwagon, hooray!