Fixing the PR Bottleneck — Matt Pocock, AIHero https://www.youtube.com/watch?v=LlgiOCmFG_w AI Engineer · 22 min · 295 sentences · sentence-level transcript from YouTube's caption track; each line begins at the time shown [00:01] OK. [00:02] I'm here to talk about solving the PR bottleneck, and that's a bit of a big name for trying to fix something that I think most organizations struggle with, and that they've historically struggled with since before AI came along. [00:16] We've always had a huge amount of PR that was just sitting around with no one checking it, and now that workload has massively increased due to the new challenges that AI is putting before us , imposing all these strange limitations. [00:28] And to do that, I'll use a rubric of my skills that you may have heard of or even used. [00:35] I have a couple of new skills I want to announce that I hope will improve your approach to PR, improve the speed at which you review and execute them. [00:46] Speed. [00:48] Now we are being forced to do more with fewer resources, or essentially, more PR, more work, etc. And that's the main promise of AI: we'll be able to use [00:59] these agents to scale and do more work. [01:02] This led to the emergence of the “software factory”—perhaps the biggest buzzword today. [01:07] Everyone I talk to talks about the software factory. [01:12] I think of a software factory as something where instead of a human initiating all of this work, we outsource some of these processes. [01:19] Part of the initiation will be performed by agents. [01:23] And I think about it this way: Maybe from there you have a classifier like Jeff who says, " Okay, let's turn this into a fix or a replay." [01:56] Or maybe I'll notify people right away. [01:58] Maybe you have something else. [02:00] You may have Planet Scale connected to get reports on slow database queries. [02:05] Maybe this triggers another link in your software factory. [02:09] All this is not run by people, but by deterministic code, right? [02:13] So, they speed up your software factory. [02:16] They push more code through it . [02:19] But then you need brakes, right? [02:21] If you have constant acceleration pushing things through a factory, you get a " garbage cannon," right? [02:27] You'll just get a bunch of worthless, terrible PR that you won't even be able to touch, review, or even look at. [02:35] So you need brakes. [02:36] These are mechanisms that slow down, improve quality, and ensure that your codebase doesn't turn into a nightmare of software entropy, because code is the environment in which your agent runs. [02:48] And if you have bad code in your codebase, it will generate even more bad code. [02:54] So, in this talk, I'll talk about these three brakes and how we can use them to, paradoxically, move faster. [03:02] So , automated checks. [03:03] These are deterministic checks in your repository that have been around for thousands of years, well, or since the 50s. [03:11] Deterministic checks, where we have linting, tests, type checking, and code quality—all working together and the same way every time. [03:19] On top of that, we have an automated review. [03:23] So we have agents that look at our code and say, "Okay, this is for things that the tests didn't catch, or this is an analysis of the structure of the codebase in general ." [03:32] And then on top of that , the third level, the final level—is human verification. [03:37] That is, people who view PR. [03:39] And these three levels form a kind of "cake" that we get when we get to human verification. [03:45] So higher speed, of course, means more PR. [03:48] So the goal here is to make human verification faster by building on these first two steps. [03:54] And we are going to stop the negligence. [03:57] This is the first principle: if you improve the quality of the code you release, you will spend less time on human review because it is simply a better job, and therefore you will need to do less intervention. [04:09] So, automated checks. [04:12] Nowadays, checks are cheap. [04:14] The cool thing about them is that they don't cost tokens like an automated review, they don't require human effort, they only cost processor time. [04:22] For example, you run tests for every code change. [04:25] Perhaps these tests require some tokens, because let's say the agent created a bug and the test detected it—then you'll have to spend some tokens to fix it. [04:34] But, in my opinion, these are tokens spent quite appropriately. [04:38] So, inspections are cheap. [04:39] This means you can add a lot of them to your repositories. [04:43] And you're probably not using them enough or creatively enough. [04:49] But tests can lie. [04:51] Does a green CI status mean that the code is ready for merging ? [04:55] No, it doesn't mean. [04:56] So we always needed an extra layer on top of these checks to find out if there was something catastrophically wrong with the code before we released it . [05:05] So all the other stages, human verification and automated verification—they're lie detectors. [05:10] They are needed to detect lies in automated checks. [05:13] Now I want to show you some examples of such lies, because it helps us , when we think about code and automated testing, to understand how bad it is and what it can lead to. [05:24] The first is tautological tests. [05:26] A test that simply confirms the implementation. [05:29] Opus 5 got hooked on them. [05:31] I don't quite understand why. [05:33] For example, it would be: the character limit in post X is 280. [05:37] Can anyone guess the test written to check for this behavior? [05:40] Truth? [05:41] You've probably seen this a thousand times. [05:43] This is real code from agents or what I found out agents do. [05:47] It says: " We expect the character limit for post X to be 280." [05:51] So, the implementation looked exactly like this. [05:54] And the test essentially just repeated the implementation. [05:56] This is the tautological test. [06:00] Tautological tests are bad because they are extremely sensitive to structure. [06:05] They are very dependent on the internal workings of the system. [06:09] This means I can't change the constant without failing the test. [06:14] Moreover, I can't rename a constant without failing the test. [06:17] This is so closely tied to the structure of my system. [06:21] I found another one that I would say is even more blatant. [06:25] This is an incredible e-e test. [06:29] It essentially checks whether two elements in the interface appear in the correct order. [06:35] So, it checks whether the pitch details page , or more specifically, the video section, comes after the content plan. [06:42] It doesn't render it to the screen. [06:45] It simply reads the file, the module into its own memory, and then finds the right thing. [06:51] So, it finds the content plan, finds the video, and then expects the videos to come after it in the source material. [06:58] Which is crazy when you think about it, because I can just change the look of the source material and this test will fail. [07:05] It is too sensitive to the structure of my codebase. [07:07] So this is another way that automated checks can go wrong. [07:11] And there's also Oh , sorry. [07:13] Automated checks can lie. [07:15] There are also tests that literally cannot fail. [07:18] This will sound familiar to you if you have abused mochas or used different things before. [07:24] For example , here we have the use audio boost function. [07:28] And this function use audio boost, its internal logic uses the audio context API in the DOM. [07:34] Don't worry if you don't know any of this . [07:36] But here we are simply creating " stubs" with dummy methods. [07:40] It turns out that the audio context has complex error modes and crashes under strange conditions. [07:47] Therefore, such actions mean that our test cannot fail in these modes, and we will encounter strange errors in operation that the tests will not fix. [07:57] So the question arises: is it possible to bypass automated checks, even with the best of intentions? [08:03] AI doesn't try to write bad tests. [08:07] It simply follows our instructions and writes tests that are too tied to the structure, instead of actually running the code. [08:14] So how do you make automated checks harder to bypass? [08:18] If we do this, we will raise the quality of inspections, and therefore our overall quality standard. [08:24] And the first thing I really like here is the code-based design . [08:28] You can literally design solutions to avoid these bad checks. [08:33] So what does good code-based design look like? [08:36] I've talked about this before in previous talks for AI engineers—these are deep modules. [08:40] These are modules that hide complex behavior behind simple interfaces. [08:43] This is an idea from John Osterhout on software design philosophy . [08:48] If we look at these two modules, we see A, where the large implementation is hidden behind a small interface on top. [08:56] Of course? [08:57] And module B has a large interface, many functions that can be called, but each of them does very little. [09:03] Does this make sense? [09:05] Yes? [09:06] If you have a deep module like A, you will have fewer structure-sensitive tests because you are hiding more of the implementation behind the interface. [09:17] If testing only happens through this interface, you will get better tests. [09:21] Your task here is to get agents to use this little interface, not to dig into the implementation to test weird details. [09:30] I have a skill for this. [09:35] You can take the weirdest, worst codebase you've ever seen, apply this skill, and it becomes better. [09:43] It essentially gives you opportunities to deepen the modules. [09:47] It looks something like this . [09:50] Raise your hands if you've used this skill before. [09:52] Not sure how many of my people are in this room. [09:54] Yes, okay. [09:55] This is really really cool. [09:57] Essentially, it gives you an HTML document. [09:58] I'll step aside so you can see. [10:00] With all the different, um, potential possibilities she sees. [10:04] So here we see a “before” and “after” state where we reduce duplication and create a beautiful, deep, and tested module. [10:12] And then you can safely implement it. [10:15] This is also coupled with a language that I developed to describe modules, because if you ever try to learn how to structure a codebase, you'll find 20 different approaches, and they'll all be called [10:28] DDD, and you need a consistent language that you can use within a team to discuss these things. [10:33] So I have a small set of codebase design skills that define what locality is, what leverage is, and what seam is. [10:41] I used joints before it became mainstream. [10:45] Locality means how well all the code is placed together. [10:51] How can you change one small part in a module without causing a chain reaction? [10:56] And leverage is what you get by having a deep module, because the caller of that module gets a lot of benefit from calling a simple function. [11:06] Both of these concepts are very useful in codebases. [11:09] It turns out they are also useful for agents. [11:12] But I'm describing all these high coding standards, but how do we actually get an agent to adhere to them? [11:20] How do you make sure that the agent creates deep modules, writes good tests, and doesn't create those worthless tautological or structure-dependent tests? [11:30] Well, I think most people do it wrong. [11:34] And my first piece of advice: don't implement coding standards directly into implementing agents . [11:42] Of course? [11:43] Let me explain. [11:45] If you imagine that the implementing agent looks something like this, then these are all the tasks that it has to perform within one context window. [11:52] He must be able to investigate, that is, look for code that needs to be changed. [11:56] Then he has to modify it directly, i.e. update the files marked in green. [12:01] And then he needs the time to adjust himself. [12:04] So, to run automated checks, to actually check and confirm that everything is working. [12:10] As you can see, this is quite a large amount of work. [12:13] And if you try to put coding standards enforcement on it , it will perform worse. [12:20] So, the implementation is overloaded. [12:22] This is the mental model I want you to adopt. [12:25] So can we find a way to implement these coding standards in a way that doesn't overload the agent? [12:30] Well, this is my attempt to do so. [12:32] This is my code review skill. [12:34] She gets a diff file. [12:36] It reads a file in the repository called " coding standards" which you can write and customize. [12:43] Then it checks whether the code meets these standards. [12:47] So, if we look at the review agent, he also importantly works as a subagent. [12:52] Therefore, it has its own context window for processing tasks. [12:55] He has his own budget. [12:57] He needs to do some research, right? [13:01] Because of course, he gets a diff file, so he knows exactly where the code is, but he should explore the system a little for a broader context to better understand the code. [13:10] However, he does not need to deal with implementation. [13:12] He doesn't need to do any debugging. [13:15] So while the implementation is overloaded, the view is actually underloaded. [13:19] So, he doesn't have much work to do. [13:22] This means you can add a bunch of coding standards to it , and it will handle it much better than if you tried to do it during implementation. [13:29] I think about it this way, and it's a little uncomfortable, right? [13:32] Because we all want to get quality code the first time . [13:36] But I see it as a two-step process of writing good code: implementation—to make it work, and code review—to make it good quality. [13:45] You apply your own coding standards. [13:48] And for the “ old school” developers among us, this is essentially a red-green-refactor approach. [13:53] We use one context window to make the code work by going through the red-green stage, and then another for refactoring. [14:00] At least that's how it works in my head, and it's what has brought me success. [14:03] This means that when you have coding standards, you don't add them to the global scope . [14:08] You don't put them in agents.md because then they overload your implementation tool. [14:13] He may or may not read them . [14:16] You place them in coding standards.md, and that way they are only processed by the code review agent . [14:21] Now I have another idea: I talked to a lot of people today. [14:26] Many say I was talking about review, automated review, and eliminating bottlenecks in the PR process. [14:31] So many people say, "Oh, yeah, we just use a third-party service." [14:35] We use Cursor Bug Bot. [14:36] We use Code Rabbit. [14:38] Or something like that. [14:39] I think I was really trying to create a universal code review skill that would find all the bugs, perform security audits, etc. before. [14:49] It turns out that this is very, very difficult because you either make it too general and get false positives that are irrelevant to your case, or you make it too specific. [14:59] You say, "Okay, find everything related to TypeScript." [15:01] And then Rust developers can't take advantage of it. [15:04] Therefore, I would say: you should not outsource automated testing. [15:07] Create your own. [15:08] Develop your own coding standards over time. [15:11] Share them with your team. [15:12] And if you have the ability to implement coding standards, and you have some documents that no one reads, this is exactly the place to put them. [15:21] Also , when you have an automated reviewer like this, many people have a natural desire to say, "Oh, yeah, my code review agent reads the code and then leaves comments [15:32] on the PR." [15:34] So your code review agent in this case is just adding work to the human reviewer. [15:39] A human reviewer then has to read all these lengthy comments and decide: do we implement this or do we implement that? [15:46] The reviewer must make commits. [15:48] He has to make corrections. [15:50] That is, he must actually change what he finds. [15:52] Because when a person comes, he or she checks a finished, high-quality artifact. [15:57] If he finds something that raises questions, he can of course leave a comment, but the default should be a commit. [16:03] Stop trying to get the perfect code the first time. [16:06] Stop trying to make the executive agent a mere tool and thinking, “Okay, I’ll make it work perfectly.” [16:13] It takes some time, you know, to make sense of it, but once you get it, it's just incredible. [16:19] So, okay. [16:19] With this whole process: we've done our automated checks, now we've done an automated audit to make sure the checks aren't lying. [16:27] How do we maximize the quality of PR from a human verification perspective? [16:31] How to make it work as best as possible? [16:34] We need human-friendly PR. [16:36] And this is a new skill that's coming to the repository, it's currently in development, but I'll be releasing it soon—it's the PR skill. [16:45] I thought about this for a long, long time. [16:47] I still don't fully understand what the modern standard is. [16:50] I realized that the best way to master the skill of creating PR is to simply borrow all the best ideas that everyone has. [16:56] And that's very good. [16:57] So, what does a good PR description look like? [17:01] How can you recreate this skill yourself? [17:04] Well, the first principle is that some checks are more important than others. [17:09] Not every inspection is mandatory, right? [17:12] And once you understand that, you realize: okay, this means I can focus my energy on the really important checks. [17:20] But how do we classify them? [17:23] You need to think about PR using AWS terminology: is it a one-way door or a two-way door? [17:30] Most of your PR will be a two-way door. [17:34] You can merge a PR and then you can always roll it back. [17:37] That's a great advantage of working as a software engineer instead of a civil engineer, right? [17:42] Usually when you're a civil engineer, it's always a one-way door, right? [17:45] If you make a mistake , the bridge will collapse, but if you have doors on both sides, it means you can easily undo the changes. [17:54] This may be a little more complicated than you expect. [17:57] It could happen that a very simple change accidentally sends an email to 60,000 people or something like that, in which case it's a one-way door. [18:04] You have to check this very, very carefully. [18:06] If it involves expensive migrations or data loss, it's a one-way door . [18:10] Check such PR as carefully as possible. [18:13] But, in addition, we need to understand the radius of this PR. [18:17] What could go wrong? [18:18] And if things go wrong, how serious is it? [18:22] This means that at the end of all my PRs there is a nice short summary—the level of merger risk. [18:28] I see that this PR is a two-way door . [18:30] Its radius of effect is local, so I see: great, I don't need to pay much attention to it. [18:35] I'll just take a look at it. [18:38] This is very important. [18:40] Next, we need to understand what this PR is actually doing, right? [18:43] I've tried many ways to figure this out, and the best I've come up with is using pseudocode. [18:50] I want to express my great gratitude for the "show me" skill from the human layer skills repository by Dex Hadley. [18:58] This is a phenomenal skill that essentially eliminates most of the text and shows everything through images and diagrams. [19:06] This makes it much easier to understand what exactly is changing and why it is happening. [19:13] So, you get a standard set of mermaid and UML diagrams to show the sequence of events. [19:20] You also get great simple examples like this one. [19:23] We can look at this and say, "Okay, we're working in the command line interface. [19:26] We see that a new command has been added ." [19:27] And we have two new things, these little flags at the top. [19:31] Just a brief summary. [19:32] This really makes a huge difference. [19:34] It is difficult to overestimate its importance. [19:36] So, you try to make understanding the reasons as fast as possible. [19:42] And I think the third principle here is what you have to think about when you're doing a human verification. [19:49] Because we don't do like...Because our processes are so streamlined now, and we all collaborate around these same skill files, these same management files. [20:03] We are all building an environment in which our agents will work together. [20:09] You should think of the process that creates your code as just as important as the code itself. [20:15] In other words, when you do human verification, you are not only checking the code, but also the system that generates it. [20:21] And the theory here is that you never want to write the same comment twice. [20:26] Is n't that right? [20:27] You never want to see an agent doing the same thing in two different merge requests (PRs). [20:31] So, what mechanism allows you to make your verification of a person significant? [20:35] Well, it's a new skill. [20:37] It's called retro. [20:38] Retro— from "retrospective." [20:40] You are essentially taking the session you had. [20:42] It could be either a single agent session, or a PR along with a session, or you could just have her review it: "Okay, look at all the PRs we've done in the last week, all the reviews, put them together." [20:54] Let's take a look back at them. [20:56] And it will offer automated checks and coding standards to make next time better. [21:03] So, it's a cumulative effect, where by doing a human review, you actually increase the quality of the next review and reduce the amount of work for yourself in the future. [21:12] And retro is a really smart skill. [21:15] She adds a bunch of everything. [21:17] Of course, it offers automated checks. [21:19] It suggests updating the coding standards.md file. [21:22] She does other smart things too. [21:24] For example, it analyzes navigation indicators. [21:26] How easily did the agent find the information? [21:29] Can we provide a hint in agents.md to help him next time? [21:33] It analyzes the effectiveness of using tools. [21:35] Are there other tools we use in the session that could be more effective with tokens? [21:41] It's amazing how much this reveals, because such things are often very difficult to debug from the outside. [21:45] This also checks the system for redundancy. [21:47] So, are there overloaded control files? [21:49] Are there excessive skills that lead to such poor results? [21:53] Can we make them more orderly? [21:56] So, that's the goal. [21:57] This is to make human verification faster. [22:00] We achieve this by overlaying automated verification to make human review as painless, simple, and optional as possible. [22:10] You really don't need to test every decision that can be changed. [22:13] But every irreversible decision is mandatory. [22:16] So, these are my skills. [22:18] I'm here at hero.dev/skills, and this week we're releasing version 1.3. It was great spending time with you. [22:25] It was a really enjoyable conference. [22:26] I'll be in the lobby if anyone wants to chat. [22:30] Thank you very much for the invitation. [22:32] Thank you, Paris.