3 ms·

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

by Carrok 2y ago

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.

cuu508 2y ago
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"
- throwthrowuknow 2y ago
  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.
spartanatreyu 2y ago
> 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.
- rohansood15 2y ago
  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.
- luckilydiscrete 2y ago
  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.

by Carrok 2y ago

cuu508 2y ago

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"

throwthrowuknow 2y ago
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.

spartanatreyu 2y ago

> 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.

rohansood15 2y ago
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.
luckilydiscrete 2y ago
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.