I Had AI Write My PR. Now I Need to Understand the Review.

I’m going to be honest about something that happened this week, because I think a lot of people are doing this and nobody’s talking about it.

I had Claude generate a PR. A real one — migrating mai_kernels to Bazel with GPU test support. CI workflow changes, new macros, BUILD files, the whole thing. I reviewed the diff, it looked reasonable, I pushed it. Then my teammate Ryan left five review comments.

And I sat there reading them thinking: I don’t actually know what half of this code does.

The situation

Let me back up. I’ve been working on migrating packages to Bazel in our monorepo. mai_kernels is a Python package with custom CUDA kernels — it can’t be auto-generated by gazelle because of the native C++/CUDA parts, so the BUILD files need to be manually maintained. I’ve done enough Bazel migration work to know the shape of what’s needed, but the specific CI plumbing for GPU tests in Docker? That’s new territory.

So I described what I wanted to Claude, it generated a PR with a py_gpu_test macro, BUILD files, and a bazel-gpu-docker CI job. I read through the diff. Nothing looked obviously wrong. I pushed it.

Ryan reviewed it within a day. Five comments. All reasonable. All things I should have caught or at least thought about.

The problem isn’t that the comments were hard. The problem is that I couldn’t respond to them on autopilot, because I didn’t write the code. I had to go back and actually understand what was generated before I could have an opinion about the feedback.

That’s the part nobody warns you about with AI-generated PRs. The code is the easy part. The review is where you find out whether you actually know what you shipped.

Comment 1: the image tag that doesn’t exist yet

Ryan pointed at the bazel-gpu-image-tag job, which computes a Docker image tag like this:

main-${{ hashFiles('docker/Dockerfile', 'docker/pretraining/Dockerfile', 'docker-bake.hcl') }}

His comment: “This will fail if we change these files on the same branch?”

I stared at this for a minute before it clicked.

The tag is based on the hash of the Dockerfiles. But the actual image only gets built from main. So if you’re on a feature branch and you modify any of those Dockerfiles, the hash changes, the tag changes — and now you’re pointing at an image that was never built. The tag literally doesn’t exist in ACR. The job fails.

This is one of those things that’s completely obvious once someone says it, and completely invisible when you’re reading a diff. The code looks correct. The logic is correct for the happy path. It just has an edge case that matters in practice — because people do modify Dockerfiles on feature branches.

Ryan’s follow-up was generous: “Probably fine for now, but maybe leave a comment?” Which is reviewer code for “I know this isn’t a blocker, but future-you is going to be confused.”

I wouldn’t have caught this. Not because it’s hard, but because I didn’t think through the relationship between the image build pipeline and the feature branch workflow. Claude generated the hash-based tagging because that’s the pattern used elsewhere in the repo. It’s the right pattern. It just has a known limitation that deserves a comment.

The fix: Add a comment explaining the assumption. Two lines.

What I learned: The image tagging scheme is coupled to the build pipeline’s trigger branch. Same pattern, different failure mode depending on where you’re running.

Comment 2: the DNS retry loop

The bazel-gpu-docker job starts with a DNS retry loop:

- name: Wait for DNS Ready
  run: |
    for i in $(seq 1 30); do
      getent hosts github.com && exit 0
      sleep 2
    done
    exit 1

Ryan’s comment: “Do we know that we need this? Is it due to some container startup ordering issue?”

This one I actually knew about — but only because I’d seen it in other GPU workflows. The gpu-condor2-rss-dind runners use Docker-in-Docker, and there’s a race condition where the container’s network stack isn’t fully initialized when the job starts. Without this loop, actions/checkout tries to reach GitHub, DNS resolution fails, and the whole job errors out.

But here’s the thing — I knew the what but not the why until Ryan’s question made me think about it. “Container startup ordering issue” is exactly right. The runner’s networking initializes asynchronously, and GitHub Actions doesn’t wait for it.

If I’d written this code myself, I’d have added a comment explaining the race condition. Claude included the pattern because it appears in similar workflows, but without the context of why it’s needed.

The fix: Add a comment explaining the DinD DNS race condition. Point to the other workflows that use the same pattern as precedent.

What I learned: Sometimes the AI copies a pattern correctly without knowing why the pattern exists. That’s fine — until someone asks “why?”

Comment 3: use the standard action

The PR installs uv with a raw curl:

curl -LsSf https://astral.sh/uv/install.sh | sh

Ryan’s comment: “Can we use this instead?” — with a link to .github/actions/setup-uv-wrapper/.

Yeah. We should.

The repo has a standardized composite action that wraps astral-sh/setup-uv@v5 with a pinned version, retry logic, and caching support. Claude didn’t know about it because the wrapper is a local composite action, not something that shows up when you’re generating CI config from general knowledge. Claude reached for the universal approach — curl | sh — which works but ignores the team’s existing tooling.

This is the most common failure mode I see with AI-generated CI code. It produces something that works but doesn’t match the team’s conventions. Functionally correct, culturally wrong. And that’s exactly what code review is for.

The fix: Replace the curl install with uses: ./.github/actions/setup-uv-wrapper.

What I learned: AI doesn’t know about your local composite actions, your wrapper scripts, your team’s preferred way of doing things. It defaults to the internet’s way. Your reviewer knows the team’s way. That gap is the entire value of code review.

Comment 4: one big glob

The mai_kernels/BUILD.bazel file has one py_library target:

py_library(
    name = "mai_kernels",
    srcs = glob(["src/mai_kernels/**/*.py"]),
    ...
)

Ryan’s comment: “Can we eventually break these into smaller targets?”

This is less a bug and more a design question. One monolithic target means any change to any file in mai_kernels invalidates the entire package in Bazel’s cache. Smaller targets — per subpackage, maybe — would give better cache granularity and faster incremental builds.

But here’s why it’s one big glob right now: mai_kernels is manually maintained because gazelle can’t handle the C++/CUDA parts. Breaking it into subpackages means manually mapping the internal dependency graph — which Python files import which, which ones depend on the CUDA bindings, which ones are pure Python. That’s a chunk of work, and it’s not clear it’s worth it until we know how often this package changes relative to its consumers.

Claude generated the simple version because a single glob is the obvious first step for a manual BUILD file. It’s correct. It’s just not optimal.

The fix: Add a TODO comment. This is a follow-up, not a PR blocker.

What I learned: The reviewer isn’t just checking correctness — they’re checking trajectory. “Does this get us where we want to go, or does it create tech debt we’ll regret?” A single glob works today. The question is whether it’ll still work when 50 packages depend on it.

The pattern I’m noticing

Five comments. None of them were “this is broken.” All of them were in the space between “this works” and “this is how we do things here.”

Comment 1: Works on main, breaks on feature branches (edge case awareness)
Comment 2: Works, but nobody explained why (documentation gap)
Comment 3: Works, but we have a standard tool for this (convention gap)
Comment 4: Works, but won’t scale (design trajectory)

And then Ryan’s follow-up on comment 1 — “probably fine for now, but maybe leave a comment?” — which is the most human thing in the entire review. It’s saying: I understand the tradeoff, I’m not blocking you, but help the next person who reads this.

AI doesn’t generate comments like that. AI doesn’t say “probably fine for now.” AI either generates the code or doesn’t. The nuance of “this is acceptable as-is but here’s how to make it good” — that’s a human skill.

What I’m actually learning

I want to be clear: I’m not embarrassed that Claude wrote this PR. Claude is good at this. The macro definition is clean, the CI job structure is correct, the BUILD files follow the right patterns. I would have spent two days writing what Claude generated in twenty minutes.

What I’m reckoning with is the gap between generating code and owning code. When Ryan asks “do we need the DNS retry?” I can’t answer that from the diff. I need to know about DinD networking on condor2 runners. When he asks about the image tag, I need to understand the relationship between hash-based tagging and the build pipeline. When he suggests the uv wrapper, I need to know it exists.

None of that knowledge comes from reading the generated code. It comes from understanding the system the code lives in.

The AI handled the syntax of Bazel migration — BUILD files, macros, CI YAML. The review tested the semantics — does this fit our system, our conventions, our failure modes? Syntax is automatable. Semantics require you to actually be present.

I think the honest workflow looks like this:

Let AI generate the PR. It’s faster and often cleaner than what I’d write from scratch.
Read the diff like you wrote it. Understand every line before pushing.
When review comes back, treat it as a learning session, not just a fix list.
The comments that reveal gaps in your understanding? Those are the valuable ones. Those are the things you now know that you didn’t before.

Step 2 is where I cut corners this time. I read the diff but I didn’t interrogate it. I didn’t ask “why this pattern?” for the DNS loop. I didn’t check for existing composite actions. I didn’t think about the feature-branch edge case for image tags. I trusted the output because it looked right.

It was right. But “right” and “good” aren’t the same thing. Code review is where “right” becomes “good,” and you have to be present for that part. The AI can’t do it for you — because the reviewer isn’t reviewing the code, they’re reviewing your understanding of the system. And if you don’t have that understanding, the review is the place to build it.

Ryan’s five comments taught me more about our CI infrastructure than the PR itself did. That’s not a failure of the AI. That’s code review working exactly as designed.