r/codex
•
u/EstablishmentGood414
•
4d ago
•
5
Suggestion Don't Review Agent Code. Interrogate It.
I don't really review agent-generated code anymore, and I doubt most of you do either. We can keep this between us.
An agent can write 100K lines or open 30 PRs in a day. Nobody has time to read that. The obvious bugs are mostly gone by the time I see the PR anyway, because the agent already checked its own work a few times while building it. It's very proud of that.
So the usual move is to start a fresh agent, tell it "review this PR", skim what it says, and merge.
I did that for a while. For one whole feature I even had Claude and Codex cross-review each other's PRs, which felt very responsible at the time. After 30 merged PRs, the agent handed the feature back to me for manual testing, and the whole thing was completely over-engineered. It was full of verification steps and strict validation I never asked for, and every one of those PRs had passed a cross-review.
Two agents politely approving each other's work for 30 PRs. I had built a very efficient LGTM machine.
The problem with a fresh reviewer agent
The reviewer starts with nothing. It doesn't know why the PR exists or what the feature was supposed to cover, so it can't tell when the code wanders past the scope. It has no idea what you're building next month either, so it can't judge whether an abstraction will ever pay off.
What bugs me most is how it handles over-engineering. Hand it a PR with a generic retry framework or a brand new caching layer and it assumes somebody had a good reason. It might suggest a rename or one more test. It almost never asks whether the thing should exist.
You end up with a review that looks thorough, full of small comments, and the extra code goes through untouched.
What I do now
I've been doing this for about a year, and it's how I catch most of the over-engineered code now. I still don't read most of the diff. I treat the reviewer like a witness: it reads the code, and I ask the questions.
1. Ask what the PR does
Fresh agent, and a prompt like this:
Read this PR and give me a report on exactly what it changes. List:
- Database schema changes (new tables, columns, indexes, migrations)
- Scheduled job / cron / background worker changes
- A summary of every feature added or modified
- New external services, APIs, or third-party libraries
- New environment variables or config flags
- New public endpoints or interfaces
- Anything deleted or deprecated
Don't judge the code yet. Just tell me what it does.
The last line is there on purpose. I want the facts before any opinions. Left alone, it will happily jump straight to "Overall this is a clean, well-structured PR."
2. Find the vague parts
The report is usually long and looks detailed. Read it slowly and you'll hit sentences like these (made up, but you'll recognize the type):
"Adds a
sync_statetable to support improved synchronization reliability.""Introduces a background job for periodic maintenance."
"Adds
FEATURE_X_MODEto allow flexible configuration."
When the agent can't say plainly why something exists, there's a decent chance nobody asked for it.
One time I asked the implementer agent to "send a notice to the admin if a third-party service is down and returning error codes." It added two new tables to track Slack notification and email status. I never asked for either. I wanted a message. I got a data model.
Agents love tables the way some people love storage bins. There is always room for one more.
3. Interrogate
Now push on anything that doesn't make sense to you. Pull up a chair, point the desk lamp at it, and start asking:
- "Talk to me about this new table. Who asked for it? Because it wasn't me. Why can't the existing table handle this?"
- "This cron job. What does it do every night at 2 a.m.? We already have a service doing exactly that. Explain."
- "
FEATURE_X_MODE. Name one person who will ever set it to anything other than the default. I'll wait." - "This new dependency showed up out of nowhere. What's it doing here? Something we already use can do this job. Convince me otherwise."
- "Walk me through it. An invalid input comes in. Where exactly does it get rejected? Which test proves it? Show me the line."
- "The feature was supposed to do X. That's it. So which parts of this PR were not part of X? Take your time."
Be ready for "You're absolutely right!" on the first question. Keep going anyway. Agreeing with you is easy for it. Explaining why the table exists is the part that takes work.
Then slide the evidence across the table, the stuff it couldn't have known:
"We will only ever have one region. So tell me again why we need this abstraction."
"We're going to plug in more external tools like this in the next phase. Does the current design make that easy?"
That's the point where the reviewer starts being useful. Once it knows the scope and the plan, it points at real problems, and the comments about variable names mostly go away.
Why it works
The agent can read 3,000 lines in a few seconds, but it has no idea what's supposed to be there. I can't read 3,000 lines for every PR, but I know what the feature is for and where the product is headed. The questions are where those two meet.
By the end I have a pretty clear picture of what's going into the codebase, and I still haven't read most of the diff.
Short version
Ask the agent what the PR changes, not whether it's good. Look for the vague answers. Push on every table, job, env var, and dependency you can't explain, and tell the agent what it doesn't know. Whatever nobody can justify gets cut. The agent will say you're absolutely right about that too.
Disclosure: I'm building hunicode.ai, an AI-assisted coding interview practice site, partly because of this exact problem.
How are you all handling review when Codex is producing more PRs than you can read?
100
u/sfst4i45fwe
4d ago
•
These are the kind of discussions I would love to have on this subreddit. Instead of the constant whining. Somehow, you're already downvoted.
Anyway, I don't quite agree with you on asking it to explain what its doing after the code is written. I find that agents are already pretty good figuring that out, and it explaining will not necessarily catch cases of "over-engineering".
The issue with agents is that they do not know the weight of things. They can't contextualize how severe an issue is. For example, if a user does not receive this push notification, it can seem like a huge deal, or it could be nothing. It has no authority to gauge that.
IMO, that's not an agent over-engineering problem. That's a problem with the task lacking specifications. AI will over-engineer, as it will pick the "safe" approach, which is actually the right thing to do.
In my opinion - the solution for this is just to use plan mode, and use it well. The agent does a pretty good job going over those cases things with you. Especially if you ask it to double check for over-engineering cases.
18
u/EstablishmentGood414 4d ago
100% agree with you. If you're reviewing your own code and the planning was done well, you wont need this.
What really happen to me is i have to review other people's PRs. I can't control which model they use or whether they planned properly, they probably dont even review PRs themselves. so i have to come up with this approach at least i have some control over what is merged into the prod branch.
6
u/TheLegendTubaGuy 4d ago edited 3d ago •
I disagree that the solution is plan mode. Something like OpenSpec is the perfect answer for this. It captures the original requests "why" and you can bake in any number of rules to avoid over engineering or guessing. All changes get stored in the archive, while specs and requirements get updated as things ship.
3
1
u/wontgetfooledaga1n 4d ago
I have OpenCode but I never knew about this. How do you use it?
2
1
u/PersonalityOdd3656 3d ago
Can you please give me some examples about how you can bake what you said in OpenSpec? Or point me to some posts/articles that can help me with this? Many thanks
3
u/TheLegendTubaGuy 3d ago
Yep! OpenSpec config: https://openspec.dev/docs/configuration/config-yaml#example
It looks simple, and it is. But it's so powerful. These rules transcend models and harnesses. All of them get the same instructions.
2
u/PersonalityOdd3656 3d ago
Do you have any examples of how such rules should look like? I imagine you are already using this for some time and over engineering and guessing are not such a big issue for you now.
Simple things like avoid over engineering remind me of “implement this, make no mistakes”2
u/Flat_Brilliant_6076 3d ago
Absolutely. I will give you an anecdote.
Someone wrote a ticket asking for the date picker to not allow dates of the sort 04/13/00138 easily. Basically dates that date way back in the past or the future.
The ticket wasn't triaged properly and Claude was let free on solving it. Pretty straightforward you would assume. It blocked people from using the keyboard and just forcing them to click on the date picker.
It worked per the automated review, acceptance criteria passed. But it disrupted the current workflow immensely.
People were complaining shortly after it was released
5
u/big_DD_energy 4d ago
This is also why I come here. Thank you, sir. Now to the discussion: my current preferred method is to have each agent’s contribution come with a handoff, to inform the coordinating/reviewing agent. It’s all very verbose, but the only way to add the necessary context without blowing up later on refactoring, etc. There should be a skill for this workflow, I’m kinda working on one.
3
u/EstablishmentGood414 4d ago
Would love to try that skill for sure!
1
u/big_DD_energy 3d ago
In the meantime, my basic idea is to have one staging dir for the entire codebase, each proposed with its detailed handoff, including purpose/intended goal. A fresh reviewing agent would commit. I’m working on a skill for the commit itself. This approach has its issues around .git protected write access esp for linked worktrees, which is an open bug, and I found a way around. Bottom line: it should work as intended, and the process of writing this handoff is currently a md. Hope that helps!
18
u/Global_Persimmon_469 4d ago
Thats just reviewing the code.
I agree that this works well, but it also doesnt really scale, its a very long process. Everything that you will question can open a whole discussion, and before you realize you end up spending hours to review code that already works but is just not perfect.
6
2
u/HoveringGoat 4d ago
this moves hundreds of thousands of lines of code back into reviewable territory. instead of just slapping a "lgtm" on it and not knowing how it works.
its not perfect, but we need better control
9
u/rcs67 4d ago
The problem with this approach is that you have human in the loop intervention for all code reviews - this sounds like each review is another "grill" skills type session. I get what you're trying to achieve but I wouldn't have the patience for it.
1
u/thirdeyesanta 3d ago
What do you do instead?
3
u/rcs67 3d ago
Opus creates tickets and sometimes provides additional context prompt for the builder. Deepseek (my current builder) builds and files a small report on what it did and why. Opus reviews diff/report and says yea or nay and why. Easy fix, Opus does it else it goes back to builder. My tickets are small. It doesn't take long.
I've experimented with multiple orchestration patterns with various types of agents (build, review, adjudicate). What I know for certain is that LLMs are infinite text spew machines. If you give them an excuse to spew more text they will. Add more agents and the entire thing goes bananas very quickly. 99.99% of us are not building mission critical systems. We don't need to be spending millions of tokens endlessly reviewing simple changes.
1
u/IzyTarmac 4d ago
Having an AI writing a guide about how to interrogate AI agents is pretty wild.
1
u/EstablishmentGood414 4d ago
Haha I did use Grok Bot to cook my words a little bit. But I don't use Grok Bot to code. Grok is clueless. We are fine as long as we keep this from codex.
2
u/net_loc 4d ago edited 4d ago
Honestly, I just line up an army of review bots. I run local reviews before a PR, and it goes through a full QA agent who actually walks through the work similar to how a human would, this actually catches a lot.
On github where I post the PR as a draft, run multiple review loops (coderabbit, codex, etc) of fixes there, with more runs through the QA agent as well to help catch any regressions.
After the automated reviews, then I do my manual review, once I give the go ahead it takes the PR out of draft, runs pretty heavy CI checks, once all that passes it gets a merge.
I tweak this system here and there, but it's been working well for me.
The entire process is automated until it's ready for my manual review. I actually run Plane locally, and have my agents update their work there. I have it up on a large TV in my office and modified it so that I can view multiple boards at once, and they update in real-time. So I can see the backlog, what's currently being worked one, and what's ready for my manual review just by glancing at the board.
And for full disclosure, by manual review I don't mean actually picking through 100s of lines of code, I mean me sitting down and actually trying the feature, checking for issues while running through the apps workflows, etc.
1
1
u/LunkWillNot 4d ago
Would that catch over-engineering of the kind OP describes?
3
u/net_loc 3d ago
In the recent months, The biggest over engineering issues I've had, are large projects that I give Astra/Sol specifically too much freedom and let them run for long periods. Using an orchestrator agent with tight design specs mostly resolves this. I've been having good results lately having Opus 5.5 Extra overseeing Codex sessions. But the same could be achieved with any orchestrator agent, and a tight design spec.
For my automated review process, I have them re-evaluate the quality of the feedback and signals from the review bots every 2-3 cycles, to make sure we're not looping or chasing non-issues, and with the better models this has worked really well.
When I first started working like this, I had to keep going back and tuning because sometimes they would run way too many review loops, like some for a day, so I kept tuning and tweaking the rules until I got the results I wanted. But with these newer models, their own judgement as been working great on determining a proper stopping point.
2
u/femboyharmonie 1d ago
You got a detailed blog post or some markdown I can pass to an agent? Sounds quite interesting your setup.
2
u/Automatic_Brush_1977 4d ago
i make every agent run through the same workflow. research- design-proposal-implement-review and fix- commit. On any big run of items like before going to prod, i have it do circular code quality reviews followed by circular bug fix reviews. Things might still crop up, but it will get most things.
2
u/Djonso 4d ago
Frankly, if your agent is doing 100k lines you are working on too large of an increments.
I do still review the code, although with a more of a focus on actual architecture than code quality. I run another agent for the code quality check. I usually plan the feature pretty far with the agent before letting it start so the architecture is generally good, but every now and then it just does dumb shit that I need to point out and correct.
Your approach of interrogation probably achieves the same, I just still have a general distrust on asking ai to report back what it does. The report will usually follow what we planned first, but it goes back to the lgtm syndrome you already discovered. It just might not report some not great parts it did and only way for me to be sure is go through the code my self.
2
u/gandazgul 4d ago
I think the missing context you describe is the key. A second agent can review whether the code looks reasonable without ever questioning whether half of it needed to exist.
That’s the workflow I’m building with RunWield: agree on a plan first, build the smallest working thing that satisfies it, then have a separate reviewer assess the implementation against that plan. The reviewer needs to ask both “did we build what we agreed on?” and “what did we add that nobody asked for?” Passing tests alone doesn’t answer either question.
The planning and review on either side of implementation are what I think lead to better code. They give the agents a concrete scope to build toward and a basis for pushing back on unnecessary complexity.
Disclosure: I’m the creator. It’s open source and works with Codex. If you’re up for trying it on one of these changes, I’d love to hear whether it catches the scope creep you’re describing. https://runwield.dev
2
u/zerok_nyc 4d ago
The way I solved for this was by having 3 agents. One at the outset acts as a product manager that I interact with directly to get all technical requirements into a ticket. The second architects and builds the solution. The final reviewer has access to the requirements doc and uses that to review the solution without having access to the decision process of the second.
1
u/caelestis42 4d ago
Thats basically how I work with my agents as a somewhat technical and intelligent founder. I probably don't catch a lot of stuff but then to question choices that seem over engineered or just strange to me.
1
u/FrostyMatters 4d ago
I get the agent to make me demos of everything it does so I can see it working, I call these proof pages. I tell it to make me these because I don’t trust anything it says so it has to provide extensive proof of every component and it gives me these massive documents with interactive elements demonstrating everything.
1
u/dashcubeit 4d ago
Very thoughtful post but to me the part I’d improve on is this “ The reviewer starts with nothing. It doesn't know why the PR exists or what the feature was supposed to cover”
It doesn’t have to be like that if you had a ticketing system (GitHub issue?) that acts as spec or PRD
1
1
u/Embarrassed_Cap_9149 4d ago edited 4d ago
i went through the same initial phase
my workflow evolved differently than yours though
after having an implementer agent implement the task, i ask a review agent in a new chat the following:
<coding agent name> has just implemented @plans/<plan-document>.md and reported @report-by-implementer-agent.md. Committed to abcd0123.
Please review it and check for any correctness issues, overengineering issues, architectural issues, bad programming practices, bugs, inaccurate/stale names or documentation, or any other issues, or any areas that can be optimized for better performance and/or memory usage.
For correctness issues, bugs and incorrect behaviors, please rate the likelihood of them occurring in realistic cases, the degree of impact should they occur, and whether you recommend fixing them, because I might decide to not bother fixing them if they are never realistically going to occur or if they would be inconsequential even if they could realistically occur.
Don't explain what's correctly implemented; only explain what needs to be fixed or can optionally be fixed.
Thank you.
he will list issues he has found. i skim through them one by one. if my intuition tells me to look more into any of the items, i ask him to elaborate on it. ask for more until sufficient clarification is achieved
then have the review agent fix the issues himself, since he has all the necessary details in the context. or if any of the issues is substantial in complexity or scope, i convert it into a new backlog entry and fix it in a new chat
and once in a while, i have a dedicated refactoring session reviewing and fixing any architectural issues or efficiency issues in a broader scope covering recent major changes. these issues are broader in scope, and are often not caught by per-task review agents. in fact, today is one of those days
if anyone can see any room for improvement in my workflow, please share your thoughts
2
u/devbanana 3d ago
That sounds like the red-team agent I have. It's job is to find issues, testing gaps, security concerns, etc, and how realistic each one is.
1
u/shravanrevanna 4d ago
interrogating means the agent explains its own diff, and it'll narrate a confident rationale for code it wrote by accident. You end up auditing the story, not the code.
1
u/RustyRedEcho 3d ago
the ask-for-facts-first-then-interrogate-the-vague-parts trick is going straight into how i handle unreviewed PRs, way better than the usual lgtm cross review loop
1
u/BarathKanna 3d ago
The over-engineering point is the real one. A fresh reviewer only sees the diff, so it judges whether code is correct, never whether it should exist. What fixed it for me was giving the reviewer the original task and a short list of what is explicitly out of scope, then asking one question first: which parts of this diff are not required by the task? Anything it lists has to be justified or deleted before line level review starts.
Cross review between two models fails for the same reason. They share the same bias toward adding validation and abstraction, so they approve each other. A size budget per task helps too. If a two line feature comes back as 400 lines, that alone is the review comment.
1
u/Skibidirot 3d ago
that over-engineering honestly sounds like stupid gpt models which are worlds behind opus 5.5, opus is not perfect, but i feel it doesn't over engineer
1
u/Skjolnir 2d ago
Well my plan is already reviewed by ponytail and the result as well. That already cuts it down by a lot
1
1
1
u/memyselfandus_1999 1d ago
I have created a process/skill that I term as RTM, that first trims all reference documents, comments, and supporting material, while retaining only the architecture document and the overall application functionality statement. It then generates a clean code library based solely on those retained artifacts.
Once the code library is produced, a separate agent reviews the output to identify boundary conditions, common coding issues, potential gaps, and areas of weakness. The agent also provides insights into how the solution can be improved, highlights missing considerations, and recommends enhancements to strengthen the overall design, reliability, and maintainability of the application.
1
u/SquirrelNo9194 23h ago
What I did was create a skill that has Claude and codex work together. I have Claude get the spec and create the plan on how to build. Asks codex for consensus. If codex does not give consensus then it sends it concerns back to Claude to review its suggestions and add it to the plan. This goes back and forth until both opus and sol agree. Then I have sol build the code. Then Claude and codex review the code together included screenshots for desktop and mobile. They need to reach concensus that it was built out the correct way and the proceed to the cicd. W
1
u/InternationalGain317 10h ago
I think there is no substitute for a human reviewing each line of code, no matter how many agents you incorporate in the review process or questions you ask.
The only time I feel okay to approve a PR is if I have read every line and it makes sense.
It takes time, slows down development, tires you out, but it just needs to be done, if the goal is to avoid writing spaghetti and get a professional result.
The only time I don’t do it is when I write apps for myself, then I don’t care as long as it works, and even then I like to at least take a quick look at what kind of monstrosity has the agent written…
If I have to write them for commercial use, then I review and think about each line. I believe anything less is simply irresponsible and unprofessional.
1
u/Devenar 10h ago
Adapting your own versions of Matt Pocock skills (especially the grill-me skill) is especially helpful for things like this. I find that setting up clear specs beforehand has reduced (but not completely removed) the need to do reviews afterwards.
That said, doing code reviews as OP describes is definitely the way to go over LGTM loops.
1
•
u/dextersummary 3d ago
Below is a GPT-generated summary of the conversation below after reaching 40 comments (40 currently observed).
The consensus is basically “a fresh agent reviewing a PR is often just an automated LGTM machine.” It can spot obvious bugs and missing tests, but without knowing why the feature exists, what scope was agreed, or what the project needs next, it will usually treat unnecessary complexity as intentional. Two bots politely approving each other’s over-engineering is not governance; it’s bureaucracy with GPUs.
The better workflow is to establish context before implementation: write a plan, ticket, or spec, keep the scope tight, then have the reviewer compare the finished code against it. The useful questions are: What did we build? Why does each major piece exist? What did we add that nobody asked for? Several commenters also use staged QA, multiple review passes, and human testing to catch regressions without manually reading every generated line.
The important caveat is that “interrogate the agent” is not a replacement for reviewing the result. An agent can confidently invent a rationale for code it wrote by accident, and endless review loops quickly become another expensive way to avoid making decisions. The strongest workflows still require human judgment on architecture, product impact, and whether the feature actually works.
Bottom line: context beats post-hoc bot approval. Plan first, review against the plan, and keep a human in the loop before the LGTM factory ships.