Code review summaries

Code reviews are always "fun" to do - you have minimal context of what the person was trying to do (without sitting down with them), but you are the gatekeeper of them merging their code.

Or the code is a tiny change which doesn't really need an approval (eg, a flag flip). Or it's a monster change, over 100's of files, and following it is like trying to work out the string lines on the board, but without any context.

We've been trying to get engineers to look at their code change and asses if they think it's risky or not - and if not, just merge it. (sure, post a link in the #team-engineering channel, but just as a "hey I'm doing this" type thing)

While I was watching the Data and AI keynote at re:invent this morning, I found this thread from Simon Willison and Corey Quinn, talking about the newly announced Nova models from AWS, which are quick, cheap and well resourced.

Both are worth a follow on Bluesky if you are interested in either LLMs/AI/Data (Simon) or AWS and Snark (Corey). Simon's blog is especially good.

I wanted to try the new Nova LLMs out, and I ended up down the rabbit hole to Simon's llm command line tool. This is a wrapper around a number of different LLMs - ChatGPT, Claude, Bedrock, Ollama if you run it locally. But it's a unix/mac command line tool, so you can run things like:

llm -m nova-pro 'a happy poem about a pelican with a secret'

and it'll fire up bedrock, use the Nova Pro model, and spit out a poem.

Simon's comments on bluesky about AWS IAM setup for a new account are very valid, but not something I want to cover here - for Tend, ours are setup already, so I just had to request access to the models. AWS needs to address this - it's overly complex and very fraught with footguns.

However, because this is a command line tool, you can do all sorts of fun things with your local data. For example, I now have an alias for

git diff -W --no-prefix -U1000 master | llm -m nova-pro -s 'Describe these changes' | pbcopy

Which will diff the current branch against master (you want to be branched directly off the current master, so rebase first), and have Nova describe the change. You could do this with Claude, ChatGPT or others just as easily.

I'm still working on the input - I'm not sure if this is enough context - but the results indicate that it might be VERY useful for automatic generation when you make a PR, so it's easier for someone to review the code.

It's not perfect yet, and I want to get it running during our normal Danger run which injects ticket numbers and links. But thats a hell of a result for basically no investment of my time.

... and a short time later....

The main feedback from the team was "cool, but it's a bit wordy!". Which is right. So I reworked the prompt a little

Before:

'Describe these changes'

Output:
Here are the changes made in the `slack.ts` file:

1. Added a new constant `HIDDEN_CHANNELS`:
   - This constant is an array that contains hidden channel names. In a development environment, it is an empty array. In other environments, it includes the channels 'notifications-appointments-ashburton' and 'notifications-appointments-linwood'.

2. Modified the `getNotificationChannels` function:
   - The function now filters channels by checking if their name starts with the `channelPrefix`.
   - The result of the filter operation is now ensured to be an array using the nullish coalescing operator `?? []`.
   - The return value of the function is updated to include both the filtered channel names and the `HIDDEN_CHANNELS`. This is done using the spread operator to concatenate the two arrays.

These changes ensure that the `getNotificationChannels` function returns a list of notification channels that includes both dynamically fetched channels and any hidden channels specified in the configuration.

After:

'Describe these changes which are part of a pull request for a larger project. The project is in typescript and uses graphql for the api, and postgres for the backend. Be concise, especially on short changesets.'

Output:
- Added `HIDDEN_CHANNELS` array that includes hidden notification channels in production.
- Modified `getNotificationChannels` function to return both visible and hidden notification channels.

This was on a tiny PR, but it shows that changing the prompt is going to be super useful.

Right now, my aliases are (ZSH)

makenotes() {
  git diff -W --no-prefix -U1000 master | llm -m nova-pro -s 'Describe these changes' | pbcopy
}

makeshortnote() {
  git diff -W --no-prefix -U1000 master | llm -m nova-pro -s 'Describe these changes which are part of a pull request for a larger project. The project is in typescript and uses graphql for the api, and postgres for the backend. Be concise, especially on short changesets.' | pbcopy
}

So I can just run makenote and the comment is then on the clipboard