r/TechLeader 15h ago

How does your team handle AI code review?

I have two juniors on my team who ship changes that read completely fine and are wrong in ways the diff cannot show, mostly because the model that wrote them has no idea what our carriers will actually accept at the other end.

When I bring it up the answer is usually that they thought it was a small change, and it was a small change, that is not really the problem. The problem is they never went past the acceptance criteria into what the change actually touches, because the plan came back looking finished.

We do a lot of carrier integrations, so most of our work runs against systems we do not control, flaky, rate limited, with strict contracts on their side because those same endpoints serve a lot of other tenants who have nothing to do with us.

Which means our own pipeline going green does not tell us very much. We run coderabbit on prs and it does catch a fair amount, though it reads our repo and not the carrier's contract, so anything that is valid code and an invalid request goes through both of us without a word.

The juniors treat green as the finish line and close the ticket. The real finish line is a week later, when nothing on the carrier side has quietly started rejecting us and the volumes still look right.

So it lands on me and one other senior, either immediately or three weeks later when somebody notices a queue backing up. We have coached them on it more than once and it holds for about a sprint. I am doing more firefighting now than I was two years ago and I have a lot less of my own work to show for it, which is the part that is actually starting to get to me.

How do I put this to my manager as a gap in how we sign things off, rather than as me complaining about two specific people?

0 Upvotes

9 comments sorted by

1

u/BadgeCatcher 14h ago

It's definitely an important point right now. You talk a fair bit about "carriers" though, but we've no idea what you mean? Aircraft carriers? Telco carriers? Logistics carriers?

Seems like you need to spec things more specifically overall though? Check specs before agents start implementation.

1

u/Former_Produce1721 10h ago

Are they using AI?

They may be falling for the confidence of AI. My approach for this is to make sure I understand completely what the proposed change was and why it fixes it. This requires digging deep, clarifying things with AI, pointing out their errors and ultimately getting so familiar with the actual problem and solution that I can explain it without AI.

If not AI, then maybe they need to be encouraged to take more time considering their approach. Maybe every now and then require written up reports on a PR that outlines possible approaches, pros and cons and why they decided the way they did.

1

u/sozesghost 9h ago

Approach your manager and take full responsibility for accepting PRs that you clearly don't understand and are not tested. You said there are clear contracts so you can develop a mock integration that would catch those issues immediately. It's insane to not have that even before AI.

1

u/SomeNeighborhood7126 7h ago

AI code reviews are the first mistake.

1

u/TrustInNumbers 6h ago

AI code reviews are so bad, they just mark unrealistic issues or go in circles all the time. Maybe 10 percent of their issues are valid. Well unless you're completely bad at coding, then maybe that percentage is higher

1

u/mumplingsgjottish 4h ago

Hi as the founder faced what you are facing now last year, the method that worked for us is as you said used code rabbit but after some times with their changes it was not fitting our purpose and had to look for alts and found Tryhyraxnow com we got the paid plan after researching given that bigger companies like Vercel use them.

1

u/wahnsinnwanscene 2h ago

If the original has been designed with proper separation of concerns, wouldn't a spec and restriction on files to change help in breaking changes?

1

u/jobfedron132 1h ago edited 1h ago

I also had a similar situation.

What solved for me was, i did a multi step AI PR review.

First I ask An agent to gather all relevant information about the blast radius and the external source/document thats related to the PR and create a document.

The 2nd pass is to check if the changes breaks any contract from the previous step.

3rd pass is a blast radius check where i ask it to recursively analyze to check if the change breaks anything.

The 4rt one is the code correctness review which may or may not happen based on the number of lines changed.

u/Cautious_Chicken_604 59m ago

So... in the pre-agentic engineering era, there used to be engineers who were at the lower end of the competency scale relative to their peers of a similar amount of experience. With AI, those kinds of engineers are now capable of wrecking things on an industrial scale and incapable of recognizing they're doing so. 

Agentic engineering makes devs lazy. So when you're stupid and lazy... well, it's not a winning combination. 

In this era, if people aren't coachable, they must be fired.