LAB: AI, Review Yourself

Some LLMs are very good at reviewing and refining code. It can be generated code, like we have, or code you or team have written.

The first thing it will do is to examine the style guides in the instructions, or in any documents you attach as context. That means you'll need to be sure you've spelled out what is expected in terms of review and code quality, which is where we'll start.

Step 1: Update the Instructions

If you don't have review guidelines set up for your project, you can have Copilot help you out (we'll be using Copilot for this last lab but use whatever tool you like).

Ghost text (code completions) is particularly helpful for this kind of thing. You can enable it for markdown by clicking on the Copilot icon in the bottom status bar of VS Code.

To get started:

• Create a new file called review-guidelines.md and put it in the /docs folder. This isn't something Copilot will need to reference on every chat request - only when we're doing a review.
• Start typing # Code Review Guidelines at the top, and let code completion take over…

Tab through what comes back, and see if it makes sense to you. Some of it will, but some things will no doubt be repetitive or useless.

For the sake of time, let's work with this set of guidelines that Claude Sonnet created for us:

<span class="hljs-section"># Code Review Guidelines</span>

This document outlines the guidelines for conducting code reviews in our project. The goal is to ensure high-quality code, maintainability, and collaboration among team members.

<span class="hljs-section">## General Guidelines</span>

<span class="hljs-bullet">  -</span> All code should adhere to the project&#x27;s coding standards and style guide.
<span class="hljs-bullet">  -</span> Code should be well-structured, readable, and maintainable.
<span class="hljs-bullet">  -</span> All code should be properly tested, with sufficient test coverage for new features or changes.
<span class="hljs-bullet">  -</span> Code should be documented, including comments and JSDoc annotations.
<span class="hljs-bullet">  -</span> Pull requests (PRs) should be used for all code changes, and they should include a clear description of the changes made, the purpose of the changes, and any relevant issue numbers.
<span class="hljs-bullet">  -</span> Code should be reviewed by at least one other team member before merging into the main branch.
<span class="hljs-bullet">  -</span> All code should be written in English to ensure clarity for all team members.

<span class="hljs-section">## Documentation</span>

<span class="hljs-bullet">  -</span> All code should be well-documented with comments explaining the purpose and functionality of complex logic.
<span class="hljs-bullet">  -</span> API endpoints should be documented in the <span class="hljs-code">`docs/api.md`</span> file.
<span class="hljs-bullet">  -</span> Any significant architectural decisions should be documented in the <span class="hljs-code">`docs/architecture.md`</span> file.
<span class="hljs-bullet">  -</span> All changes should be accompanied by relevant documentation updates to ensure clarity for future developers.
<span class="hljs-bullet">  -</span> All public methods and classes should have JSDoc comments to provide clear API documentation.
<span class="hljs-bullet">  -</span> The README file should provide an overview of the project, setup instructions, and usage examples.
<span class="hljs-bullet">  -</span> All documentation should be written in Markdown format and follow the project&#x27;s style guide for consistency.
<span class="hljs-bullet">  -</span> Documentation should be kept up-to-date with code changes to avoid discrepancies.

<span class="hljs-section">## Comment Guidelines</span>

<span class="hljs-bullet">  -</span> Comments should be used to clarify complex logic, explain the purpose of functions or classes, and provide context for why certain decisions were made.
<span class="hljs-bullet">  -</span> Comments should not be used to explain simple or self-explanatory code.
<span class="hljs-bullet">  -</span> Comments should be concise and relevant, avoiding unnecessary verbosity.
<span class="hljs-bullet">  -</span> Comments should be written in English to ensure clarity for all team members.
<span class="hljs-bullet">  -</span> Comments should follow the project&#x27;s style guide for consistency in formatting and tone.
<span class="hljs-bullet">  -</span> Comments should be placed above the code they refer to, using a consistent format (e.g. <span class="hljs-code">`//`</span> for single-line comments, <span class="hljs-code">`/* ... */`</span> for multi-line comments).
<span class="hljs-bullet">  -</span> Avoid using comments to disable code; instead, remove or refactor the code as necessary.
<span class="hljs-bullet">  -</span> Comments should not be used to explain the &quot;what&quot; of the code, as this should be clear from the code itself; instead, focus on the &quot;why&quot; and any important context.

<span class="hljs-section">## Code Review Process</span>

<span class="hljs-bullet">  1.</span> <span class="hljs-strong">**Pull Request Creation**</span>:
   Developers should create a pull request (PR) for any changes they wish to merge into the main branch. The PR should include a clear description of the changes made, the purpose of the changes, and any relevant issue numbers.
<span class="hljs-bullet">  2.</span> <span class="hljs-strong">**Assign Reviewers**</span>:
   The developer should assign at least one reviewer to the PR. Reviewers should be familiar with the codebase and the specific area of the changes.
<span class="hljs-bullet">  3.</span> <span class="hljs-strong">**Review Guidelines**</span>:
   Reviewers should follow these guidelines when reviewing code:

<span class="hljs-bullet">     -</span> Check for adherence to the project&#x27;s coding standards and style guide.
<span class="hljs-bullet">     -</span> Ensure that the code is well-structured, readable, and maintainable.
<span class="hljs-bullet">     -</span> Verify that the code is properly tested, with sufficient test coverage for new features or changes.
<span class="hljs-bullet">     -</span> Look for potential bugs, performance issues, or security vulnerabilities.
<span class="hljs-bullet">     -</span> Ensure that the code is properly documented, including comments and JSDoc annotations.
<span class="hljs-bullet">     -</span> Check that the code does not introduce any breaking changes unless explicitly intended and documented.
<span class="hljs-bullet">  4.</span> <span class="hljs-strong">**Feedback and Discussion**</span>:
   Reviewers should provide constructive feedback on the PR. This can include suggestions for improvements, questions about the implementation, or requests for additional tests or documentation. Discussion should be respectful and focused on improving the code quality.
<span class="hljs-bullet">  5.</span> <span class="hljs-strong">**Addressing Feedback**</span>:
   The developer should address the feedback provided by the reviewers. This may involve making changes to the code, adding tests, or updating documentation. Once the feedback has been addressed, the developer should push the changes to the PR branch.
<span class="hljs-bullet">  6.</span> <span class="hljs-strong">**Approval and Merging**</span>:
   Once all feedback has been addressed and the reviewers are satisfied with the changes, they should approve the PR. The developer can then merge the PR into the main branch. If there are any merge conflicts, they should be resolved before merging.
<span class="hljs-bullet">  7.</span> <span class="hljs-strong">**Post-Merge Actions**</span>:
   After merging, the developer should ensure that the main branch is stable and that all tests pass. If necessary, the developer should also update any relevant documentation or notify the team about the changes made.

Now that we have this, let's have Copilot do a code review.

Step 2: Executing the Review

VS Code has a built-in code review tool that uses Copilot, but I have always struggled with it. The UX is barely usable and it's very slow, at least at the time of this writing.

What I find to be more useful is to run a quick report, review it, optionally save it to a markdown file, and then have Copilot make the changes.

It's easiest to just do it, rather than read about it:

• Open a Copilot chat pane, select “Edit” as the mode. We want edit mode just in case we want to save the file. We can't do that in Ask mode, which is odd.
• Open lib/cart.js in the editor, which will automatically add it to the chat context.
• Drag in the docs/review-guidelines.md to the chat pane.
• Enter the prompt: review this code. text only for review

Claude is very thorough when it comes to code reviews, and you should see a list of issues, including validations, error handling, and more.

Let's save this to our project so we can use it as reference:

save this as a markdown task list file to the project

You should see a result like this:

Step 3: Fixing the Problems

Go through this list and see if it makes sense to you. Did you catch these errors in the code?

It's not easy to spot silent failures (which are so annoying), and dealing with side effects can be mind-numbing.

Let's give this document to Claude and tell it to fix the problems. We could also continue the chat session, but it's nice to have this document output so we can go over it with others on the team, if needed.

• Flip into Agent mode. You'll notice that we lose the context from the previous chat session, which is odd.
• Keep the cart.js file open, for context.
• Drag in the review document that we just made. Mine is titled cart-review-tasks.md.

Now have Copilot fix the code: fix the cart code. Nice and terse. It should have all the context it needs!

You'll see Claude do it's thing and update the code files:

This is a great first step. When the refactor is done, Claude will ask you if it can run tests to ensure it didn't mess anything up - say yes.

I have never been through a review/refactor session where these testing errors didn't happen. As you'll see, Claude will keep running the tests, changing things, trying again and again.

Warning: make sure your laptop is plugged in when you do this. I've had Copilot send itself into a hole trying to get tests to pass. The reason is because it would make a correction that would fail and try something else. That would fail and then, eventually, it would make the same mistake again… basically walking itself in circles.

You can open the VS Code terminal and watch what's happening in real time if you're worried about an open loop. Sometimes it takes a few minutes … but then…

Step 4: Review the Review

We want to be sure we have a record of what happened and why any time an AI tool refactors code. A lot just went on here, so let's make sure we have a record!

I want an audit record of what just happend. Summarize this conversation for later review and add it to the docs/reviews directory. Give it a name and a date as well.

Wait for just a moment, and boom:

It's a big document with a lot to read. And yes, there might be some “hallucinations” in there, so read through to make sure everything is captured properly.

The bottom of the document is wonderful:

I love how Claude gives itself an A+ on Quality 🤣. Why not?