Testing AI-enhanced reviews for Linux patches

> Brandeburg said that there was something else that LLMs could, in theory, do that would be "very, very hard" for him as a reviewer: go back and look at previous revisions of a patch series to see if previous comments had been addressed. It would take him "hours and hours" for each series to look at all of the comments he had made. Sometimes "little stuff sneaks through because the reviewer's tired, or you switch reviewers mid-series". An LLM could be much better at going back to review previous discussions about a patch to take into account for the latest patch series.

It baffles me that Linux developers insist that email is superior to forge-based PR reviews, and then complain about problems that forges solved years ago. On GitHub,[1] it explicitly tracks which comments are requesting changes to a PR (as opposed to merely being general chatter), and whether or not each conversation has been "marked resolved" (i.e. whether anybody has pushed a specific button to "resolve" the discussion, meaning to hide it because it is no longer relevant). It can also show warnings indicating that there are unresolved comments, so that you don't merge something that isn't finished (which you can override, of course). Gerrit has a similar thing but it works slightly differently, and most other forges probably do too. The idea of using an LLM to retrospectively re-discover this information, instead of explicitly tracking it up-front is... confusing. It's like deciding not to use a VCS, and then using an LLM to try and guesstimate what the git blame output would've looked like if you had used git, based on random patches it found on your mailing list that may or may not have been applied.

I use a forge-based workflow, similar to Gerrit, every single day, and I have never had any difficulty figuring out which comments have been addressed and which have not, because the forge tells me. Obviously, Linux is not moving to a forge any time soon... but it would not be that difficult to recreate this tracking with simple text parsing. Just require developers and maintainers to prefix specific parts of their emails with specific magic strings that can be parsed out by some simple text extraction tool, and then that tool can keep track of which comments have been explicitly marked resolved and which have not. Maintainers, to my understanding, can unilaterally impose such requirements now if they so choose (by refusing to accept merges unless the tool is green).

(Yes, that is an obnoxious amount of work. Such is the price of not using a forge with first-class review support.)

[1]: https://docs.github.com/en/pull-requests/collaborating-wi...

I see at least two problems here. There's likely more, but two major ones.

First, there's no LLM out there that's FOSS. No, not even Llama. It's under a non-compete business license that's neither open nor free. There will be fundamental objections to using proprietary solutions like this for FOSS projects, and rightfully so.

Second, LLMs don't 'understand' anything. This is a anthropomorphism. They take input, do some fuzzy logic and statistics to tie it within a margin of error to a given set of rules. They make a decision based on that. They go off the rails when a question arises outside of their limited finite model. They're really bad at determining reality from non-reality. They're also bad at simply saying a query is outside of their given rule set.

So the real question becomes: Within these limitations, without giving into the hype train in either direction by praising nor by demonizing them, do LLMs actually help a reviewer, or will it end up taking more time to check audit the model's output making sure it's not contributing well formatted garbage in addition to the code the reviewer themselves is working through? Because if that's the case, it may be just easier to let the AI handle formatting issues and error tagging (for stuff that's obviously broken- it won't even compile for example) then leave all the actual code review to the human. Having properly formatted and at least superficially functional code before it reaches the human reviewer would probably be a big help without the landmine of AIs clearing what may be bad code when you look deeper.

I'd say there are fundamentally two different approaches for using LLMs to do this kind of stuff, with different strengths and weaknesses: Prompt engineering and fine tuning.

Prompt engineering, as done here, is in my experience the one it's the most reasonable to start with. Provide sets of rules, augment or modify the prompt on misbehavior. Incidentally, I'd agree that the kind of stuff done here is where current LLMs shine: Understanding text and fairly simple rules, not knowing the intricacies of the kernel.

Having said that, I think fine tuning would likely make it somewhat more powerful, limit the undesired behavior and make it spot better things it misses sometimes. (A good test is to run it three times on the same input and see if it spots the same things. You may even use it to synthesize a final answer from three drafts.)

Fine tuning can be done either alone, or perhaps more powerfully together with prompt engineering. Once you have collected a corpus of, say, 100+ good reviews (they may well be AI-generated and then just reviewed and edited by humans for sanity), you can use those to fine tune the model to further encourage desired behavior. I actually think that approach could lead to it being a bit more aware of things. You may consider asking it to add any other points it has below some delimiter just so you can track how hallucinatory or useful it is without actually pestering people with them.

I'm already using an LLM (mistral-7b) to evaluate the need to backport patches based on the instructions given in them (e.g. backport till this version, wait a little bit, only backport if XXX is also backported etc). It took a while to tune but it's fairly accurate now (~95-99% similar to the human's choice), but I still want the human to have the last word so it's only used for offloading their work and has no interaction with the rest of the world, and I would definitely do the same for reviews.

I've been trying to do this a little bit as well, to see if there could be some hope that an LLM could help maintainers with trivial reviews.

I'm still at a stage where I definitely don't want the bot to automatically respond, but instead the reviewer would use it to offload some of their work. One pain point in this case is the time it takes to analyze a message, which can be too long of the reviewer has to start the process automatically. And doing it on all incoming messages can quickly result in overloading, though that might be the only solution. One interesting aspect I noticed along some manual tests is that doing it automatically on message receipt can allow to feed it a whole thread. LLMs are very good at reading emails even their structure, and with good prompt engineering you can teach them to read the whole thread of comments and responses, and indicate what is still problematic in the last message. My first attempts were limited by the context size, but with newer models going to 32 or even 128k, it's more accessible.

I'm going to watch these slides to gauge if it's still too early or if it's worth experimenting with all this again. I notably found that Microsoft's Phi and Mistral's NEMO models are both fast and very obedient, and that makes them nice for prompt engineering, when you combine this with their large contexts.

I was probably one of the people (or the main person?) who told someone to stop complaining about stupid stuff.

Good kernel developers are always people who are very obsessed with details. They're not going to agree which details are important or even which ways are correct. You have to take the good with the bad. Most maintainers are very sensible people, but you can't hire the most detail oriented people you can find and then ask them to change their personalities. You have to accommodate maintainers and their foibles. The slides talk about "the first letter in the subject should be capitalized" and that's true sometimes. Other times it is the opposite. Some maintainers have claimed that it's American to capitalize the first letter and not how they were taught in school. As a developer, it's fine. Adapt.

The person I was talking to has been banned from a lot of subsystems. He's obsessed with the wrong parts like minor typos in the commit message or imperative tense. He never points out bugs in the patches. His comments are so confusing so it ends up generating a pointless thread. We all have to read this pointless thread and do damage control because a lot of his instructions are weird and wrong.

Occasionally, he will say things which are correct but even there, what he's pointing out is obvious. Everyone with eyeballs can see there is a missing Fixes tag. It's only newbies who leave off the Fixes tag and they assume he knows what he's talking about so they send a v2 of the patch. Now the discussion about the actual code is spread over two different threads. So even when he's correct, it's not helpful. Why doesn't he look up the code which introduced the bug and add that person to the CC list? A useful comment about Fixes tags would be, "I'm not qualified to review this patch, however you're missing a Fixes tag. Fixes: 123412341234 ("blah blah blah"). I have added Jane Doe to the CC list because she wrote that commit. After the maintainer has reviewed this patch, then please, resend with the Fixes tag."

I think that newbie reviewers are excited to spot a mistake in someone's patch. But once you've reviewed enough code, you realize that none of it is perfect. There are magic numbers. People hard code error codes instead of propagating them. There are if statements which would be more readable if they were reversed. There are checks for things which can never happen. The code uses "rc" in some functions and "ret" in other functions. There are header files which don't need to be included or the header files not in alphabetical order. The label names could suck.

Obviously bugs are not allowed. And there is a minimum level of good style which is required. But I'm not going to nitpick the code to death. I'm mostly looking for effort. If people are trying and they stick around to fix bugs then we're going to move in a good direction.

We're currently exploring a new approach related, which is using LLMs to better understand kernel code, and I'd love to get your thoughts on it.

- Can we understand the high-level design and evolution of complex
systems like the Linux kernel better?
- Can AI help us with what's never possible before?

Instead of forcing stupid AI to do buggy kernel coding, or just using RAG or
fine-tuned models to give wrong answers.

We are doing a completely different way:

- By carefully designing a survey, you can use LLM to transform
unstructured data like commits, mails into well organized, structured
and easy-to-analyze data. Then you can do quantitative analysis on it
with traditional methods to gain meaningful insights. AI can also
help you analyze data and give insights quickly, it's already a
feature of ChatGPT.

Imagine if you can ask every entry-level kernel developer, or a
Graduate Student who is studying kernel, to do a survey and answer
questions about every commit/patch/email, what can you find with the
results?

- Arxiv: https://arxiv.org/abs/2410.01837
- The Github repo: https://github.com/eunomia-bpf/code-survey
- Some early experiments and reports related to BPF subsystem:
https://github.com/eunomia-bpf/code-survey/blob/main/docs...
- The commit dataset:
https://github.com/eunomia-bpf/code-survey/blob/main/data...