News for hackers. Stuff that matters.
Re:Show HN: Patchwork – Open-source framework to automate de... by meiraleal on Friday July 26, 2024 @11:05PM (#41084187)
PR reviews are the one thing you sure don't want a LLM doing.
Re:Show HN: Patchwork – Open-source framework to automate de... by Carrok on Friday July 26, 2024 @11:50PM (#41084316)
Please elaborate.

While obviously a LLM might miss functional problems, it feels extremely well suited for catching “stupid mistakes”.

I don’t think anyone is advocating for LLMs merging and approving PRs on their own, they can certainly provide value to the human reviewer.

Re:Show HN: Patchwork – Open-source framework to automate de... by cuu508 on Saturday July 27, 2024 @02:47AM (#41084867)
They can lull the human reviewer into a false sense of security.

"Computer already looked at it so I only need to glance at it"

Re:Show HN: Patchwork – Open-source framework to automate de... by throwthrowuknow on Saturday July 27, 2024 @05:55AM (#41085502)
I don’t know what your process is but if someone else has reviewed a PR before I take my turn I don’t ignore the code they’ve looked at. In fact I take the time to review both the original code as well as their comments or suggestions. That’s the point of review after all, to verify the thinking behind the code as well as the code itself and that applies equally to thoughts or code added by a reviewer.
Re:Show HN: Patchwork – Open-source framework to automate de... by spartanatreyu on Sunday July 28, 2024 @09:08PM (#41097230)
> LLM [...] feels extremely well suited for catching “stupid mistakes”.

No.

Linters are extremely well suited for catching stupid mistakes.

LLMs are extremely well suited for the appearance of catching stupid mistakes.

Linters will catch things like this because they can go through checking and evaluating things logically:

> if (

> isValid(primaryValue, "strict") || isValid(secondaryValue, "strict") ||

> isValid(primaryValue, "loose" || isValid(secondaryValue, "loose"))

> //...............................^^^^ Did we forget a closing ')'?

> ) {

> ...

> }

LLMs will only highlight exact problems they've seen before, miss other problems that linters would immediately find, and hallucinate new problems altogether.

Re:Show HN: Patchwork – Open-source framework to automate de... by rohansood15 on Monday July 29, 2024 @12:35AM (#41097913)
Linters are great at finding syntactical errors like the case you mentioned. But LLMs do a better job at finding logical flaws or enforcing things like non-syntactic naming conventions. The idea is not to replace linters, but to complement them. In fact, one of the flows we're building next is fixing linting issues that linters struggle to fix automatically.
Re:Show HN: Patchwork – Open-source framework to automate de... by luckilydiscrete on Monday July 29, 2024 @04:02AM (#41098717)
While true in a subset of problems, linters will also miss stupid mistakes because not everything is syntactical.

AI for example can catch the fact that `phone.match(/\d{10}/)` might break because of spaces, while a linter has no concept of a correct "regex" as long as it matches the regex syntax.

I don't think anyone is arguing that replacing linters with AI is the answer, instead a combination of both is useful.

Re:Show HN: Patchwork – Open-source framework to automate de... by rohansood15 on Friday July 26, 2024 @11:36PM (#41084276)
I agree and disagree. You definitely need someone competent to take a look before merging in code, but you can do a first pass with an LLM to provide immediate feedback on any obvious issues as defined in your internal engineering standards.

Especially helpful if you're a team with where there's a wide variance in competency/experience levels.

Re:Show HN: Patchwork – Open-source framework to automate de... by aaomidi on Saturday July 27, 2024 @12:11AM (#41084408)
Until that immediate feedback is outright wrong feedback and now you’ve sent them down a goose chase.
Re:Show HN: Patchwork – Open-source framework to automate de... by rohansood15 on Saturday July 27, 2024 @12:45AM (#41084506)
This is where prompting and context is key - you need to keep the scope of the review limited and well-defined. And ideally, you want to validate the review with another LLM before passing it to the dev.

Still won't be perfect, but you'll definitely get to a point where it's a net positive overall - especially with frontier models.

Re:Show HN: Patchwork – Open-source framework to automate de... by throwthrowuknow on Saturday July 27, 2024 @06:00AM (#41085521)
That happens with human review too and often serves as an opportunity to clarify your reasoning to both the reviewer and yourself. If the code is easily misunderstood then you should take a second look at it and do something to make it easier to understand. Sometimes that process even turns up a problem that isn’t a bug now but could become one later when the code is modified by someone in the future.
Re:Show HN: Patchwork – Open-source framework to automate de... by datashaman on Saturday July 27, 2024 @12:47AM (#41084513)
We're trialing ellipsis.dev for exactly this, and it's pretty good most of the time.
Re:Show HN: Patchwork – Open-source framework to automate de... by meiraleal on Saturday July 27, 2024 @08:03AM (#41086015)
I stand corrected: LLMs are great to block PRs by raising issues. A lack of issues should not be taken as a good PR tho.