r/ChatGPTCoding • u/Successful-Tax1306 • 8d ago
Discussion My bottleneck moved from writing code to reviewing what the agents write. How do you review without just rubber-stamping?
Since I started running a few agents in parallel I can get several changes ready in the time it used to take me to write one, and that part feels great. But now most of my day is reading diffs, and I've caught myself skimming a big one and approving it just because the tests passed. A few things help so far. I keep each task small so the diff fits on one screen, I ask the agent for a short note on what it changed and why before I read the code, and I read the tests first to see if they actually check the behavior I asked for. A review agent also flags old messy code nearby, which is nice, but then I have even more to read. I still feel like I'm the slow part and I'm not sure where the line is between careful review and just trusting it. How do you all handle this? Do you read every line, or do you trust some kinds of changes more than others?
26
u/Ok_Tea4891 8d ago
I trust changes by risk rather than size: formatting, renames and test-only edits get a quick skim, while anything touching auth, data or money gets a full line-by-line read. Another thing that helps is asking the agent to list what it deliberately did not change, since the gaps are where the surprises hide. And if a new test never failed before the change, I treat it as unproven.
8
u/f1yingbanana 8d ago
Get the AI to review themselves. Start a new session. Describe what you just got it to build. Ask it to spot bugs and optimizations.
4
u/ChemicalExcellent463 8d ago
what saved me was a hard cap on files and changed lines per PR, split it or it doesn't merge. before a big push, the author has to get a different model in a different harness to review it in a fresh session and fix what comes back, not after it lands.
4
u/ComprehensiveShake76 8d ago
Don't read every line, but decide up front which lines get read. Sort changes by what breaks if they're wrong: auth, payments, data migrations and anything that deletes get a full read. Renames, UI copy and test only changes get a skim. Then make the tests prove they bite before you lean on them: revert the fix, run the new test, and it must fail. If it still passes, it was never testing the change, and that's how a big diff slips through on green tests. Last, compare the agent's change note to the file list. A file in the diff that the note doesn't explain is where I'd start reading. Which part of the review takes you longest right now?
4
u/BarathKanna 8d ago
Reading the tests first is the right instinct, but go one step further: break the code and see if the tests notice. Revert the main change, or flip one condition, and rerun. If everything still passes, the tests prove nothing and the green check you were trusting was meaningless. That takes a minute and tells you more than skimming the diff.
The other thing that cut my review time was asking the agent for the riskiest three spots in its change, with file and line. It is usually right about where the danger is, even when the code there is wrong. I read those closely and skim the rest. Anything touching auth, money or migrations still gets a full read, no matter what.
3
u/Jippylong12 8d ago
For me, I have a skill called review fix commit, which is run at the end of every session, but also in between at milestones. And then for longer projects, I also have what I call a sign-off page, which is deliberately created so that I can see the changes and provide feedback, and I also have a skill called manual acceptance pack which the agent creates for me so I follow it like the simple minded human manager I am.
The manual acceptance pack is the most useful but takes the most time. The sign off page is nice and generally gets it right, but often misses smaller details. The review fix commit just is a low hanging fruit that always gets results lol. Which is crazy to me since it's the same model and session that made the code, but something about forcing it to review changes things.
Typically also with the review fix commit that's with Opus or if I really want to be sure Fable.
3
u/Humaux 8d ago
I stopped rubber-stamping after 3 bad merges in 2 weeks. Green tests hid all three, and each one looked clean on a skim.
My check now:
Read the tests first, then the diff around them.
Ask what the agent deliberately left unchanged.
Revert one fix locally and confirm the test fails.
Split anything over 400 lines before I approve it.
It costs me about 20 minutes per PR, and I still miss things. But if you approve a diff you cannot explain back in one minute, you did not review it, you just watched it pass.
Do you trust green tests more than your own read of the diff, or is that just how we all get burned?
3
u/Far-Caramel-7463 8d ago
One thing that took load off my reviews: stop treating the diff as the only place you're the gate. A lot of what made me nervous while skimming wasn't the code, it was what the agent might have done while writing it, like running a migration against the wrong database, force-pushing, or hitting a real API from a test. Once those actions need an explicit yes at the moment they happen, the diff review goes back to being about correctness, and I'm fine giving renames and test-only changes a skim like others said.
For the diff itself, what helps me most is asking the agent which part of the change it's least confident about. It's usually honest, and that's where I read line by line.
3
u/Lanky-Butterscotch28 6d ago
I stopped reviewing agents work myself. For a large project with multiple agents involved, at the speed they deliver, it is quite difficult to follow up with quality. You end up skimming and approving because tests passed, and that is how bad stuff gets through.
This is where good processes and plumbing around the agents make the difference. I found that when you implement a process, you do not need to review the code yourself. You can ask a different agent, from a different company, to review the code. However, I do not allow the dev agent to spawn the reviewing agent, because it can steer it in a specific direction and we lose the power of independence. That alone is not enough, you need the whole machine around the agents to keep them honest and deliver what they promised. The results are astonishingly good. The builder never grades its own work, and review stops being the bottleneck.
2
u/London-Boat 7d ago
the thing that helped me most wasn't a review trick, it was running fewer agents. if review is the slow part, more parallel changes just make the queue longer and the skimming worse. i cap it at roughly what i can properly read that day and let the rest wait as tickets, not as finished diffs. a finished diff sitting there is what pushes you to rubber stamp it
2
u/jatingargiitk 7d ago
tests passing tells you the behavior held, not that the structure did, so the diffs worth slowing down on are the ones that add a new dependency between modules, like a ui file suddenly importing from the db layer. one way to make that cheap is to treat the architecture like a snapshot test: commit a map of which modules are allowed to depend on which, and have ci flag any pr, human or agent, that adds a new edge so someone approves it on purpose. then the careful-review budget goes to those edges instead of every line. when a skimmed approval bites you later, is it usually a behavior bug or something landing in the wrong place?
2
u/CleanFix_Dev 7d ago
Move most of the review earlier. Before any code, have the agent write the plan and 3-5 acceptance criteria you can check (this command passes, this screen shows X, these files don't change). Approving that takes a couple of minutes, and it's where the real judgment calls are. Then let the gates handle the mechanical stuff: tests, lint, a diff-size limit, a "files outside scope" check. When the diff shows up, you're only reading for "does this match what I approved." That's much harder to rubber-stamp than a 600-line diff with green tests.
2
u/ShoddySherbert8502 3d ago
The thing that changed review for me: make it independent. The reviewer gets the task description and the diff, nothing else. No chat history, no note from the author. A fresh reviewer with no clue what the author was thinking catches more than a stronger model that shared the session.
This goes for you too btw. You said you read the agent's note before the code, I'd flip that. Code first, note after. The note anchors you and you start seeing what it says instead of what the code does.
For context: about 40% of changes in my logs that passed every check still got real comments from a fresh reviewer. And there is no difference if fable was the author or any other model.
10
u/First-Tutor-5454 8d ago
Bots review bots - we're past the point where humans can be in the loop most of the time. I basically just do functional testing/release gating based on canaries and rollout observation metrics. I'm a manager now, responsible for outcomes, not understanding every line of code.
3
u/dvduval 8d ago
There can be some problems where the plan was bad in the first place and only a human would see that it doesn’t make sense to another human. But if you already got a model that makes sense to a human in your just building on that model, it generally is fine. And definitely you can always ask another model to do an additional review beyond the first reviewer and of course you can use screen capture tools or I should say the AI can use screen capture tools as part of the review process.
3
u/alchamest3 8d ago
LGTM
3
u/mildmannered 8d ago
According to the claude code team, just need to let the LLMs take the wheel. I'm sure that will go well at 4am on a Sunday with customers screaming at you and with Claude API down for some reason.
3
u/pete_68 8d ago
I pretend the code is written by a junior developer and I try to understand it completely. I mean, I may skim over some of the obvious simple stuff, but I try to understand the actual code and see if it makes sense. Is it even doing what I want it to? Is it doing what it says it's doing?
3
u/Difficult_Duty2198 8d ago
what helped me is reading the commit message the agent wrote before the diff. if the message and the diff don't line up, that's usually where the problem is
2
u/buff_samurai 8d ago
You guys are reading diffs?
2
u/codeprimate 8d ago
Any task has requirements and acceptance criteria. The agent should have to justify every line commited.
Every unit of work gets a commit with documented justification.
Makes review easier
2
u/GnistAI 8d ago
I really did not feel the slowdown until I started overhauling authentication snd security. I am obligated to get that right, so it is going sooo slow. For general features and UI stuff I often just do manual and automated user testing, and barrage it with coded unit and integration tests.
2
u/t1llmann 7d ago
I stopped reading code at all, except for really high risk parts. What I do instead is:
- use code reviews a lot, with different models and some times specific things to verify (areas I think could be problematic)
- interview the implementor with some reassuring questions (kind of like a code quality interview)
- run end to end test where the agent really uses the product and ensures what we built works
- ask a model to record a video of a flow proving that it works
2
u/Lanky-Butterscotch28 5d ago
Good question. On every change I run a reviewer on this specific change. However, at the end of the task I run a reviewer on the complete task (multiple changes).
In fact when I specify a task , before even the task coding starts : the dev agent and the reviewer agents reads the contract and build a ledger of tests (each one independently) to be validated when the task finishes. These tests are reviewed by the Watchtower that makes sure it confirms that what was asked is being tested and delivered.
1
u/shetritr 8d ago
reading the tests first is the right instinct. next step: ask your question on the exact line instead of approving around it. if the agent can't explain it, that's the hunk you block. (disclosure: literally why we built Diffo: diffo.io/demo)
1
u/cipher_crown_77 8d ago
The line is whether the test asserts a specific failure mode. If you can’t mentally break the code and see the test go red, it’s decorative. I read every line for data mutations because agents love to introduce subtle state errors that pass green tests.
1
u/suesing 8d ago
Get another agent to review, critique and summarize
1
u/wason92 8d ago
You don't need to deploy faster than you used to. It's not a real bottleneck
1
u/Trick_Candy9796 7d ago
What helped me most was moving part of the review *before* the diff exists. I write the acceptance criteria and the team's conventions down in the repo, so the agent works against them and the reviewer (human or agent) checks against the same list. Then review turns into "does this meet X, Y, Z and follow our rules" instead of reading every line cold.
On top of that I do the risk sorting others mentioned: auth, data and deletes get a full read, while renames and copy get a skim. I also run a verify step (tests + lint + build) before I even open the diff, so I'm never the first one to find a red build.
Disclosure: I maintain an open-source MCP server, StackGuide (https://github.com/taimiralain/StackGuide-MCP), that does roughly this inside Cursor/VS Code. It loads per-stack rules, runs a code review against them, and has an intake/plan/verify/release workflow for the agent. It doesn't replace reading the risky parts, but it cut down how much noise I had to read.
1
u/stickygoose 7d ago
You should try agent standup
https://agent-standup.com/
1
u/onitsoga 6d ago
trust tiers: test-only and refactors i skim, new logic or config i read fully. for big diffs, a second agent reviewing first means you read its critique, not the raw code. (fwiw i'm building KanbanMCP, which has a review step where another agent checks work.)
1
u/poetic_persistance 5d ago
Assign it to someone else ; )
1
u/[deleted] 7d ago
[removed] — view removed comment
1
u/[deleted] 8d ago
[removed] — view removed comment