Deep review agent

[fbmcipher]

Our investigatory code style review agent is trained on 4000+ PR comments from over the years. It's designed to automate away repetitive code style, architectural and comments that enforce good coding standards – potentially saving @raineorshine time from "first-pass" busywork writing comments that have already been written before.

In contrast, a deep review agent would be much less guided. Rather than being explicitly trained to pick up on repeated comments and patterns, it would act independently as an "AI senior developer", making nuanced suggestions that humans may not necessarily even pick up on. In this way, we rely more on the "intuition" of the model.

We've had discussions about consistency – that an agent correct 70% of the time is wrong 30% of the time – and how that can have a net-negative impact on productivity, create noise and slow down the team.

To avoid that issue, we will need to test and scrutinize these review agents to understand how they work, and ensure that where they do make mistakes, that it can be guided away from them for future reviews using custom instructions.

From now on, I will assign my PRs to Copilot for a first review. I think there's a great opportunity in this – when a PR hits Raine's table for review, we could feasibly already have a pass or two of PRs done – just pending a final review by Raine.

In this thread, we can share experiences using AI code review agents.

[fbmcipher]

Initial experiences

Users can assign Copilot to a PR to have it review the submission. It'll post review comments on specific lines of code, just like a human does.

In general, I felt the comments it left were all useful. In the first PR I tried this with, #4003, I either felt its comments were important and actionable, or felt that the comments opened useful discussion and helped me feel more confident about some of the implicit decisions and assumptions within the code.

You can directly ask Copilot to make changes to the code by @'ing it in a comment thread, which is a useful timesaver. However, this only works for branches in cybersemics/em. In other words, because my pull request was for a branch in my fork (fbmcipher/em), I couldn't ask Copilot to make changes.

This is a major shame, as in my experience a lot of time could be saved if comments like these could be resolved inline without having to check out the latest code or open my IDE. All contributors are currently pushing branches to their own forks and then opening PRs.

For this reason, to continue my investigation I had to push a branch to cybersemics/em and open a second PR that way: #4039. That worked and allowed me to use Copilot inline, and I was able to fix some important issues with just a few clicks.

It's interesting to note that though the contents of #4003 and #4039 are identical, the comments it left were completely different with very little crossover. Both sets of comments of useful. I guess this makes sense given the non-deterministic nature of AI, but it still surprised me and felt worth mentioning.

Key points

• Recommendation: Allow other contributors to push branches to cybersemics directly so that they can directly mention Copilot in PRs.

  • To avoid mess, enforce a standard on branch naming: prefix with the contributors' name. My branch was named fbmcipher/https-dev-server – keeps things organized and prevents naming crossover
  • Keep main protected to ensure it's safe from accidental commits/deletion

• Copilot review agent is powerful, yet somewhat inconsistent

  • Very different comments left on two PRs with identical changes (Use https dev server instead of http to enable access to new web APIs in development environment #4003, [cybersemics:fbmcipher/https-dev-server] Use https dev server instead of http to enable access to new web APIs in development environment #4039)
  • Potential solution: assign Copilot to 2-3 rounds of code review before passing over to a human

[raineorshine]

• Recommendation: Allow other contributors to push branches to cybersemics directly so that they can directly mention Copilot in PRs.

Yes, sounds good. Everyone has write access currently so this should be possible now.

• Copilot review agent is powerful, yet somewhat inconsistent

Yeah, I would expect a given PR to yield consistent feedback. I think if we focus on the quality of the code review that will naturally address the inconsistency. i.e. an inconsistent code review is just a set of false positives and false negatives. If it misses certain recommendations then we will need to design a new rule or skill so that it consistently delivers the appropriate feedback.

[fbmcipher]

If it misses certain recommendations then we will need to design a new rule or skill so that it consistently delivers the appropriate feedback.

Agreed. Some high level code review principles in a rule file could be a lightweight way to drive the agent's behaviour to make reviews more consistent – without getting too granular in a way that takes away the agent's ability to apply its own independent judgment to a review.

For future reference, this doc explains how to create custom instructions that only apply to the review agent using the excludeAgent property.