[kristjansson]

> to make sure

you've really got to be careful with absolute language like this in reference to LLMs. A review agent provides no guarantees whatsoever, just shifts the distribution of acceptable responses, hopefully in a direction the user prefers.

[jawiggins]

Fair, it's something like a semantic enforcement rather than a hard one. I think current AI agents are good enough that if you tell it, "Review this PR and request changes anytime a user uses a variable name that is a color", it will do a pretty good job. But for complex things I can still see them falling short.

[antihero]

Yep, I was gastowning some draft PRs to get a prototype built quickly. The polecat FE PR managed to request a review from @claude (normal ok), and then given that in isolation the PR was fine due to having hardcoded draft schemas (though it was deliberately only ever going to work against a PR deployed version of the API that was also draft), decided to enable auto-merge, such that the PR actually merged and luckily I caught the CI/CD run and locked deployments, however, it would have taken down the site and pointed it to staging.

[SR2Z]

I mean, having unit tests and not allowing PRs in unless they all pass is pretty easy (or requiring human review to remove a test!).

A software engineer takes a spec which "shifts the distribution of acceptable responses" for their output. If they're 100% accurate (snort), how good does an LLM have to be for you to accept its review as reasonable?

[59nadir]

We've seen public examples of where LLMs literally disable or remove tests in order to pass. I'm not sure having tests and asking LLMs to not merge things before passing them being "easy" matters much when the failure modes here are so plentiful and broad in nature.

[ElFitz]

My favourite so far was Claude "fixing" deployment checks with `continue-on-error: true`

[jawiggins]

You'd want to have the tests run as a github action and then fail the check if the tests don't pass. Optio will resume agents when the actions fail and tell them to fix the failures.

[SR2Z]

So... add another presubmit test that fails when a test is removed. Require human reviews.

It's not like a human being always pushes correct code, my risk assessment for an LLM reading a small bug and just making a PR is that thinking too hard is a waste of time. My risk assessment for a human is very similar, because actually catching issues during code review is best done by tests anyways. If the tests can't tell you if your code is good or not then it really doesn't matter if it's a human or an LLM, you're mostly just guessing if things are going to work and you WILL push bad code that gets caught in prod.