Hello.
So our next speaker is Marco Castellucho.
He's a self-described engineer, which is a hacker and an engineer.
He is also an engineering manager at Modzilla, where he is leading the CLN quality tools,
PDFs and translation teams.
Today, he'll be talking to us about how Modzilla is using machine learning and large language
models to develop an experimental tool to make code review experience more pleasant and easy
for developers.
He would also be talking about the architecture and the iterative development process of
the tool and the future plans for this tool.
Welcome, Marco.
Thank you for the introduction.
Thanks.
So I will skip the introduction since I missed your name.
The Nika did it for me.
So the slides are not visible.
Where is the... I forgot to, of course.
Perfect.
Sorry.
So, yes, I'm Marco Castellucho, as Danica said, and as you can guess from the slides, I'm working
at Modzilla.
I'm an engineering manager managing the CI and Quality Tools team, translations team and PDF team.
In particular, this talk is more related to CI and Quality Tools, where we use both classical
tools like static analysis, LinkedIn, and also machine learning based tools.
We've been using them for a few years since before they were cool.
Now, we're continuing to using them even with the additional tools like LLMs.
Before we dive in the topic, I want to first give you an idea of the Firefox scale.
We have hundreds of bug reports, actually, they are not all defect.
We use bug reports for everything like tasks or announcements, future requests, so two million
bug reports are not actually two million defect, but still, we celebrate when we arrive at
these kinds of numbers.
We release every four weeks, with thousands of changes, we have 12 major releases in 2024.
Firefox is one of the biggest and most complex software we recently celebrated it's
20th birthday in November, and the NetScape was open, source 27 years ago, but was
started even before then, so there's a lot of legacy and tech that.
We have 950,000 commits made by 10,000 contributors with 32 million lines of code,
46,000 commits only last year with 1,100 unique contributors every year, this number keeps increasing,
which is nice.
We use lots of languages starting from C++, JavaScript, and Rust is taking more and more space over time.
So the idea we had was not to automate reviews, it was not even to make reviews faster.
It was to try to identify more issues and improve code quality through code review,
by supporting developers. We don't want to automate the code review process.
We want to support developers in this process.
This was a collaboration between us, Ubisoft, and two universities, Queen School of Computing,
and University of Montreal, we all worked together in open source project.
We didn't have access to the Ubisoft game source code, but they could work together with us
in the open source project, and we're working on a paper to show the results.
So let's dive in. First of all, I tripped down prompt engineering lane.
So we started with a simple prompt. We just asked LLM,
please find issues in the patch, and we showed the patch to the LLM.
Of course, this generated lots of weird comments, comments that usually developers don't really
want to see. So we added the human examples. Initially, there were comments that we identified as good
by default. Then later on, we tried to use code similarity to find comments that were made
previously by humans on code that is similar to the patch. Then we had to fight against the
urge of the LLM to be nice. As you might have noticed, they always want you to be happy.
So we saw comments like, this is a good addition to the code. Of course, reviewers don't really care
about this kind of comments. So we had to explicitly say, please don't add a praising comments.
Then we had to ask it to focus. It was commenting on everything. So sometimes it was generating
comments that were related to things that the patch was already doing. So it was suggesting to
do things that the patch was doing. So we had to ask it to focus on addition only, on the other lines.
Then we had to add more and more and more and more filters. For example, there were many comments
focusing on making sure that something exists. But of course, if a developer is writing a patch,
they are using functions that exist. They will not use things that do not exist in the source code.
So these kind of comments are kind of useless. This already gave us some initial interesting results,
but we needed to do a bit more. We had to try to elicit reasoning from the LLM. So we initially
before asking to review the patch, we asked the LLM to summarize the patch. So they could not
understand what the changes were and then review based on the summary. Then we noticed it was still
confused about usage of variables. For example, if it couldn't see where the variable was defined,
in case of patches that for example, they are changing function. The variable definition is way earlier
and you cannot see it unless you go back to the initial part of the function. So basically,
we gave a tool to the LLM to be able to see more context around the changes if it wanted to see more
context. Then we also gave it a tool to search code using our search fox tool, which is a code
search tool for a Firefox source code. So we gave it the ability if it wanted to see
function functions that were being used in the patch. We gave it the ability to search for those
functions and see their implementation in the source code. This was particularly useful for
Ubisoft, for example, because the LLM are already trained on Firefox source code, while for Ubisoft
their code is called source. So they really needed the ways for the LLM to see what
the prints of Perseya functions were doing. We also added the bad examples to
other bad examples to teach the LLM not to add them. For example, still praising comments like
overall the patch seems to be well implemented with no major concerns. And again, here,
just like the human comments, initially we used predefined examples that we identified by ourselves,
and then later on we started using data from from the field. I will talk about that later.
So it seems linear. It actually wasn't this linear. It went more like, okay, go back,
go forward, go back, refine, refine, refine until we got something that we were more or less happy with.
And we were satisfied enough to offer it to a set of developers. So what it looks like?
This is an example before we deployed the tool. So there was already a review comment.
The LM didn't have access to it. And it generated the comment which was saying more or less the
same thing in other words. So it was very promising. And you can see the interface here. The developers
had the ability to approve comments. Mark them has invalid or Mark them has trivial the UI
changed a little later. But I will show you an example later on. This is another example from when
the tool was actually deployed. It was actually maybe from two days ago.
It was nice so I added it. The LM identified that potentially potential overflow in this code
and so the developer fixed it by adding a check for the overflow.
This is another interesting example. In this case the LM identified a possible division by zero
and it was actually true. But it wasn't clear how to fix it because there were tests that were
actually using images with size zero. So the developer didn't exactly know how to fix it.
The patch landed as it is and the developer will investigate more to figure out what to do exactly.
And this is an example with JavaScript. You can see actually the first part of the patch
was in another hand of the patch. It's line 15 and 16 and the second part of the patch is
we later. And the LM was able to identify and problem with the accessing of this variable.
It was a different language before it was a simpler class. Now it's a JavaScript
of course it's able to identify issues in both languages and it cannot identify issues
in different parts of the patch. Not only it has the full context basically.
And this is UI when users reject comments. So when they approve it, it looks like this.
The comment was generated the contents of the comment and then we say this comment was generated
automatically and was approved by this person. Or if they added the comment as I will show later
it will say it was generated automatically and was edited by. We want to make sure that developers
that receive these kinds of comments know that they were generated automatically.
This is the interface. This is actually from Ubisoft side. What happens when you try to reject comments
so users could select the reason for rejection. Most of the cases it was a trivial comment
when they rejected. But there were many cases also of useful in the development phase,
not in the review phase or useful as a tip for reviewers, not a review comment.
So do people like it? How many comments are accepted? 8% of comments are accepted
from Modzilla, 7% of Ubisoft. It's not that huge number but it's also very promising because
it means it is finding valuable, highly valuable cases. There is still a lot of noise that we need
to address but still it's very promising as you saw in the examples before.
Some developers said it detects perfectly possible flows in the code but most of the suggestions
are more in the development realm than the review realm and that's why we added those two
cases. Useful as a tip for reviewers, not as a review comment. Useful development phase, not in the
review phase. There are not considered here between the 8% or the 7% because they were not accepted
were rejected but they were still useful in some way. So we had another percentage of appreciated
which contains both the accepted ones which were published and the ones that were considered
as useful even though not useful as comments in the review. It was more or less the fourth of the
comments, almost the third of the comments that Ubisoft. It also depended on the seniority of the
developers. Some more general developers appreciated this review tips more than more senior developers
for example. The other thing we were wondering is if the reviews would be slower or faster with
this kind of tool we were not as I said at the beginning we were not aiming to make reviews faster
but we also didn't want to make them slower and developers said things like often looking at
the suggestion helped me think more about the code and reflect a bit more on what is done and why
it is done. Overall I think it is a bit slower to do reviews but my reviews are actually better
now that I used the tool. We didn't track the review time at Modzilla but Ubisoft
tracked the review time and we saw that the additional time for this kind of comments was not that
long. So for accepted comments it was around 28 seconds for rejected it was 16 seconds. Probably
it was obvious that they were bad comments and in general it took 42 seconds more per review.
So it didn't add too much review time. Of course there are some cases where it took way longer
so towards 150 seconds and this is probably because sometimes the LLM could hallucinate
problems that do not actually exist but then if you're not familiar enough with the code you
need to actually go and try to find out if it is actually the case or not. It actually happened
to me once. I was not familiar with the API that the developer was using so I had to search for
documentation, try to figure out if it was real or not etc etc and I think it turned out to be real
but still it was a long rabbit hole and our comments edited we were we were wondering if they were
edited or not and what happens when they are edited we found that most comments are just accepted
as they are or they are shortened and this is because LLM is often two verbose so in the most
reason prompt we are asking it to be super concise. There were very few examples of length and
comments at the time we wrote the paper it was only one. Now there are a few more.
You can see in the first case it was just because the developer wanted to be nice to the developer
and it added that suggestion on how to fix the comment. In the second case instead the reviewer
was not sure about the comment but it thought it could be valuable so they said this was suggested
by the review helper Adon however maybe this should handle errors but please make sure.
I guess this is a case where you could consider it might have been a waste of time for
if it turned out not to be true we don't know I need to look into it.
The other thing we wanted to see is are accepted comments valuable enough
as human comments so we set out to see if they led to the same number of changes and it turned out
that yes more or less 74% of LLM comments lead to changes and 73% of human comments lead to
changes so they lead to changes more or less in the same way.
Next steps so we saw a few promising results at the same time we saw there is a noise in the results
so we want to improve them. So the first step improved bug detection capabilities
so right now we're using as I said in the prompt engineering phase we're using past human
comments plus the knowledge of DLLM from pre-training and we're thinking of building a data set
of synthetic comments so basically on Bugzilla a few years ago we were smart enough to think
of adding a new field called regressed by where developers can say which patch caused a bug so
this has been used for metadata purposes to know how many regressions there are instead of
over time but it can also be used in this case for example for building a data set of
fake comments to teach DLM about very highly valuable cases because basically we have the original
bug we have the patch associated to the original bug we know this patch causes another bug which
is the regression and we have the patch that fixes the regression so by giving to the LLM the original
patch where the bug was not caught by the reviewer and the fixed patch which fixes the regression
caused by the original patch we can generate fake comments that would have prevented these
regressions of course it's not always possible because sometimes that my regression might be due
to external reasons it was it would have been possible to find it by a reviewer but there are
many cases where it is possible and these are extremely highly valuable because they were missed
during the review by humans and this is to improve the acceptance rate but we also want to
reduce the rejection rate due to noise because the LLM is still even though we added a lot of
filtering etc to remove praise comments and stuff like that it's still making some of them
so we want to reduce trivial and unwanted comments first of all we want to allow customization
by type as I said there are some developers who like to see the review tips some other developers
don't want to see review tips so we want to categorize them and give the option to the reviewers to
see some of them and other reviewers to see only a subset and we want to introduce more filtering
using past trajectory example so with the implementation of the tool as I showed we collect
feedback from developers and we have a lot of accepted and rejected comments we can use
the rejected comments in just them into a vector database so whenever we generate new comments
with LLM we can find for similarly looking comments in the rejected comments vector database
by similarity and we can add them into the prompt and ask the LLM not to generate
things that look like that we're still working on it so I don't know exactly how
the solution will look like I don't know exactly how the solution okay how the solution
yes I think it's probably this maybe let's try to remove it
maybe let's try to remove it
oh it's working sorry about that so yeah we don't know exactly how the solution will look like
but we have a big new not big but big enough data set of rejected comments from developers
which we can use for clustering filtering and so on and so forth and we hope this will greatly
reduce the number of noise comments the other thing we want to do is auto fixing comments a bit
like the developer did to help sorry the regular did to help the reviewer this is an example
here on the left there is a comment via reviewer of course we only want to do these in simple
because probably the LLM won't be able to address very complex cases at least for now
but for very simple cases where developers can just accept without having to
edit the code locally and push the patch again ask for review again etc etc we could generate
suggestions directly in the review system and they could just accept them and the land the patch
so there's an example here where the reviewer said variables is actually required
pointed to some documentation and but can be an empty dicked and so the LLM just added variables
attribute with an empty dicked
we also want to there are also many developers who said this is very useful
but I would like to have it while I'm working on the patch and not after I submit the patch for
review so we are thinking of building an ID integration so that they can use it before
so this kind of issues especially the tips for example use useful in development phase
they can actually use them and avoid wasting the time of the reviewers in this case we could actually
speed up the review we wouldn't see it because the initial review by the LLM would happen before
submission but it would still be a win overall all right this is more or less it when it comes to
using LLMs for code review but I also wanted to show a bit what we're doing in addition to code
review to still support developers with various tasks first of all QA test case generation so LLMs
can often have hallucinations so you could say that they're very creative so we thought
maybe we could use them for test case generation where you need a lot of creativity to break
the software so we are getting to the LLM the role of expert quality assurance engineer
we're giving examples of test cases we specified the feature that the LLM should test
with the details of the feature the scope of the testing and so on and so forth and then we
ask it to generate test cases and we tested it on one two three four five six seven eight five
fox features which were both UI features and more backend features it went very well in general
it generated 54 cases so 27% of the test cases were novel so the no QA QA thought of them
one under the one already existed so 50% so which means they were good but already existed
29 were invalid and 16 other reasons for rejection but in general it was very valuable for the QA teams
you can see in in some cases where the feature is more obvious and there are less chances for
corner cases like reader view comfort for example there was only one novel case but in other
features which could lead to more corner cases like image to gen AI in PDF there were way more
novel cases found in in case of the back button intervention which was mostly back and stuff
there was no novel test case found another aspect is smart documentation search so we're building
a ragged architecture to ingest the Firefox source code the back from baczilla the history of
development basically and then building a chatbot for developers this was mainly thought for
new contributors who might come in or new employees etc and we started testing it with the PDF
by taking questions and answers from metrics from the PDFGS metrics channel and comparing the
human answers with the LLM answers so we have an example here which was good where the
person was asking how to download things in parallel etc the human answer it was not our answer
I guess it was somebody else from the channel just said download download it at user
another viewer well the LLM answer was this is already handled automatically by the browser
network stack and we had the developer from the PDF team who helped evaluate the cases and
he preferred the second the LLM answer to the human answer in the second case instead it was a
wrong so it was a question related to offsets of the page etc the human answer was very short but to the
point the LLM answer was wrong because it got the offsets wrong but then we actually found that the
problem was in the documentation because the documentation was wrong there are many other cases
were actually the LLM is wrong but in this specific case the LL the documentation was wrong
another interesting thing for I think for us but also for other open source
project we have lots of commits and it's very hard to identify what is important
for end users or what is important for other developers and so on and so forth so we're experimenting
with using an LLM to ingest the committee story from the software and identifying what is important
for various reasons so in this case we're starting with the user release notes so we're focusing
we're asking LLM to focus on user phasing changes but in the future we're also planning to do the same
for web developers so we could identify commits which are changing APIs in a way that
and developers care about and same for identifying new valuable contributions from new
contributors for example another aspect which is very very important for Modzilla it's web
compat tissue detection so we have many issues due to differences between browsers in some cases
due to websites which only test in one browser compared to the other and so there are many cases
where the websites might be working well in a Chrome for example Mod not in Firefox this is an
example here where you can see the graphs on the Chrome side and in Firefox the content is almost
empty basically and we're using a multi model model to identify potential issues we're still
starting this project now but it's already showing some promising results of course there are
lots of difficulties in this province not just a matter of comparing screenshots because you might
have differences just by for example different advertisement when you reload the page or if you have
a carousel for example if you take a screenshot now it's not going to be the same in two seconds because
the carousel will move so you need to identify things that are actually important versus things that
are just part of the website normally we also want to expand autofixing what
I talked about before regarding comments we want to expand it to additional cases for example when
we get crash crashes from users we can identify potential causes for the crash and suggest
potential fix the other thing we would like to try is analyze build failure logs for a given patch
to try to determine and resolve the build failure and both of these in theory could have a feedback
cycle so that we can for example in the build case we can build get the error log
try to fix it through the LLM if it still fails give it the new error log and so on and so forth
just try a few times until the issues fixed and this gets us to the end if you're interested in
experimenting with with our stuff any of the things that I mentioned and see where it could go on
Firefox or if you want to extend support for this kind of tools to other review systems right now
we only support our review system plus the Ubisoft one which is maybe only used by users of
and our satisfoing this maybe only used by us or other projects if you want to adopt
our tools in your project come talk to us in either later or in our bedroom tomorrow
modila devram thank you
thanks great talk maybe I missed it but is Mozilla hosting the LLM or are using an external service
we're using an external service we're actually running multiple LLMs at the same time
to compare results across models both size in size and type
if you thought about boosting it yourself yes we haven't done it yet but the plan would be actually to
use an open source model fine tune it with the data that we are collecting so that we can
host it ourselves and it can be an open source model
what are the costs for running those services I haven't looked at the costs recently they were not
super high but if you want I can share the numbers with you later okay and especially the
environment costs please the environmental costs yeah
thanks for this great talk so I was interested I suppose you selected Ubisoft as a partner
because you have common points with them so for instance they code in C++ with
without divulging in these secrets of course I would like to ask you how the general public
could make use of your research do they have to be programming what they're specifically
or is there something that can be used generally sorry I missed the last part of the question
the question is how reusable is your work for people who work with C++ code bases
I think it should be pretty reusable the collaboration we Ubisoft was because we already were collaborating
with them we have a lot of touchpoints like you mentioned because they also have a big large
code bases in C++ and legacy code bases and the collaboration was both because we had similar
problems but also because we wanted to try to make the two generic enough so that it could be applied
also by other projects hello when generating the prompt of you know the steps of
generating the context and creating all the prompt is there any limitation you you faced like
the size of the poor requests for example like at what when you have like multiple issues right
correlated with that patch how would you decide which context you the injector not and what was
the limitation there yeah when we started we had way more limitations because of the content
of the context size so we kept over time increasing and increasing the limitation on the so we
have a limitation on the patch size if it is too large we don't review it and this limitation was
pretty small at the beginning and then we kept increasing it and increasing it over time
because of the larger context sizes that new models support it's certainly important especially
as we build more and more tools right now we have only two tools which is the asking for more
context around the changes lines and searching for code in the future if we add more and
more tools for example searching for documentation running test or whatever we will have to choose
what is most valuable because we will have limitations on the on the context anyway at some point
out I was surprised by the little or the low acceptance rate of those comments my assumption
would have been it would have been a lot higher and I was wondering if the AI is kind of forced to
comment on every PR or is it free to just say I got nothing on this one and I'm you know I don't
have to add a comment just curious about that can you repeat sorry does the AI have to comment
on every PR or can it say I have nothing to add to this PR and just that maybe you know contribute to
the lowest so in general even in cases where there are some issues it will still generate some
trivial comments so that's why we're working on reducing that but yes what what you say is through
the LLM has a urge to say something all the time so in some cases it will generate when it
doesn't find it when it finds something it still generates something that is not so interesting
but many of them are interesting but when it doesn't find anything it still really wants to say
something and so it will fall back to its behavior even if we say please don't say this please
don't say that it will still fall back to its behavior of saying something and yes so there are still
many of these cases and we're working on reducing them I have a question as well I think I have
to maybe like do you dog feed the AI tool to also review the code that is being used to generate
the two? No not currently right now we are only using it on Firefox source code not on any other
modzilla project so we're using we have a small group of developers who are participating in this
experiment and they're all Firefox developers working on Firefox source code currently we only
show the results to the reviewer so that because we want to collect as much feedback as possible if we
showed it to the developer the developer could just fix the issue without clicking on the button
accept or clicking on the button reject it it wouldn't make sense for the developer to accept the
comment publish the comment and then fix it so that's why in the experimentation phase we're
just showing it to reviewers and only Firefox reviewers. Hello yes another question so I may have
misunderstood but like right now the tool only goes over the added lines the lines that were added
in the patch what about the lines that were removed why are they not being reviewed by the tool?
The issue was that before we asked it to focus on added lines and it was focusing on changes that
were already made by the patch so it was suggesting to do what the patch was already doing so it was
kind of a hack to avoid the LLM focusing on things that the patch was already doing which
wouldn't make sense to comment about. I think it's a limitation that we should resolve
while building the prompt engineering process it sounds easy but it's extremely complex like I said
we go back and forth back and forth back and forth we build some tools to validate changes so whenever
we make a change we have a tool that compares the we run the tool on many past reviews and we
compare the comments that are generated by the new version of the prompt with the old version
of the prompt automatically and then we manually label the ones that are new completely new
compared to before so we can see okay is the new version of the prompt
generating the same comments as before is it generating more accept more comments that are accepted
that were previously accepted is it generating fewer comments that were previously rejected so
now we have all of these tools around the tool itself and we can more easily iterate on the prompt
so this is one of the things that I want to try next try to remove this or refresh it to ask
it to focus on new changes not only on added lines also on the removed lines but we need to find
a good way so that it doesn't fall back to saying to adding comments on things that are already
done by the pat thanks any more questions you have described some comments that were
good but not good review comments but good tips for development how can we have a comment given
during a review which is good for development but not be a good review comment is a little bit
serious to me so basically there were things like I could find some examples later and share them
with you but one example that comes to mind is things like ensure that this code path is not
leading to a crash for example this is something that the reviewer should do it's not for the developer
to do so it's not a comment it's something like ensure this is a good like please double check this
but it's a suggestion to the reviewer actually I mean it could be considered as a suggestion for
both but usually when you are reviewing a patch you don't you don't add a comment to say please
make sure this is correct you will make sure that this is correct and then if it is not correct
sure if it is correct you will say something but you will never comment saying please make sure
that this is correct
okay I wonder does your system support iterative development and review is the element comment
only done once per review and will it have context on previous comments it did
at the moment it's one review and that's it we are thinking of
re-architecting re-architecting it to be a bit to take into account the previous review
as well but also to maybe make it conversational in case of those useful review tips for
example if the element says please make sure that this is correct you could ask why are you saying
that's and you could ask why are you saying that or can you look this function can you look
up this function for me you know you could interact with the comment itself to dig deeper
the other thing that comes to mind when I see this is we're also planning to
right now we're generating the comment and that's it like you saw before we're planning to also
generate reasoning behind the comment so this is this has to there's two reasons why we're
we're doing that one is so that the element maybe will realize by itself that comment is
wrong by generating the reasoning behind it and the other reason is if we generate both the
comment and reasoning the developer does not have to spend as much time validating it because
it will see the reasoning and it will be easier to see if there is something wrong in the
in the reasoning there is an inflow okay any last questions there is one there
yeah awesome talk at a question about tokens like how many tokens generally are being
computed during like each review and is it a multi-step review to get the final result or is it just
like a single like conversation that happens once it's so I don't know about the tokens any to
looking to that it's a multi-step process so it's not just a single prompt and the result it's
summarized the changes okay now please find issues okay now please filter these issues based on
these other results so there are multiple steps to get to the answer there is the link to the project
in the presentation so you can anybody can have a look at how the prompt prompting system prompting
process works thanks just checking to see if anyone has any more questions okay looks like we
are done thank you so much Marka that was really nice presentation