WEBVTT 00:00:12.020 --> 00:00:15.440 Welcome to the Bikeshed podcast where we dive into all things software engineering 00:00:16.150 --> 00:00:21.460 sip some copacetic coffee and ask the big question is the ai bubble finally starting to crack 00:00:22.380 --> 00:00:27.820 i'm your co-host whose entire workflow lives in floating windows raycast and the terminal 00:00:28.380 --> 00:00:34.040 Scott K. Alongside with me are my co-hosts. If it runs in JavaScript, he'll roast it, 00:00:34.400 --> 00:00:38.900 praise it, and make you rethink everything you ever knew about engineering. Matt Hamlin. 00:00:39.450 --> 00:00:43.100 And my other co-host. He doesn't just give takes. He forges statements, 00:00:43.600 --> 00:00:49.720 searing them into your memory hole. Dillon, spicy take, hurry. Fellas, what are we cooking up today? 00:00:51.760 --> 00:00:54.100 Yeah, today we're going to talk a little bit about AI code reviews. 00:00:56.020 --> 00:00:57.160 Yeah, I don't know. 00:00:57.950 --> 00:01:00.740 I think, Scott, you originally floated this as a topic idea. 00:01:02.000 --> 00:01:04.820 I'm actually curious to dig into how you guys are using it 00:01:04.930 --> 00:01:11.100 or if you guys have internal tools set up to do AI code reviews at work. 00:01:11.860 --> 00:01:13.480 We do, and it's kind of funky. 00:01:13.780 --> 00:01:17.000 But yeah, I'm kind of curious to hear your guys' experience with it. 00:01:18.020 --> 00:01:19.840 All right, guys, I'll dive in first. 00:01:20.580 --> 00:01:22.940 I'm going to jump the line at Whoop. 00:01:23.600 --> 00:01:24.060 That's where I work. 00:01:26.020 --> 00:01:26.880 we're using like 00:01:27.000 --> 00:01:29.160 it feels like we're using three different tools at once 00:01:29.380 --> 00:01:30.160 I don't know what's going on 00:01:30.540 --> 00:01:32.180 I guess they're piloting a bunch of different ones 00:01:32.880 --> 00:01:34.240 we got something called Greptile 00:01:35.320 --> 00:01:36.700 we got something called Copilot 00:01:36.920 --> 00:01:38.080 which is GitHub's version of 00:01:38.600 --> 00:01:39.860 code reviews with AI 00:01:40.620 --> 00:01:41.940 and then I think there's one other one 00:01:42.200 --> 00:01:43.420 maybe CodeRabbit 00:01:44.160 --> 00:01:45.420 does that sound like a thing that's real? 00:01:46.260 --> 00:01:48.220 yeah it is, we know somebody who works there 00:01:48.320 --> 00:01:48.620 right Matt? 00:01:49.820 --> 00:01:51.660 whoa, you can't just name drop 00:01:51.840 --> 00:01:52.399 the episode 00:01:54.260 --> 00:01:59.520 wait who what i don't know someone that works there 00:01:54.260 --> 00:01:59.520 yes you do you worked with him 00:02:01.359 --> 00:02:04.460 who 00:02:01.359 --> 00:02:04.460 i believe he works at code rabbit i'm gonna look it up right now 00:02:06.899 --> 00:02:13.420 arvind arvind from fireworks uh 00:02:06.899 --> 00:02:13.420 okay maybe we cut this from the episode 00:02:13.920 --> 00:02:17.819 worked with them is a strong term choice of words 00:02:20.240 --> 00:02:21.580 I'm not caught in from the episode 00:02:21.960 --> 00:02:22.580 that's staying right 00:02:25.820 --> 00:02:26.600 yeah I feel like 00:02:27.660 --> 00:02:30.000 I don't know which one of those is going to show up in my PR 00:02:30.160 --> 00:02:31.900 half the time lately it's been Greptile 00:02:32.880 --> 00:02:35.980 and I didn't realize until recently that you could actually reply to its 00:02:36.230 --> 00:02:38.800 comments until I saw somebody else do it 00:02:39.360 --> 00:02:42.080 is it shipping your PRs for you or is it just leaving comments 00:02:42.940 --> 00:02:45.959 it's just leaving comments and it'll do changesets 00:02:45.980 --> 00:02:51.540 sometimes like it it reviewed somebody else's pr that i approved yesterday and it was like hey 00:02:51.860 --> 00:02:58.060 this is like a critical bug don't do this and i was like this thing is so dumb and then i like 00:02:58.280 --> 00:03:03.560 looked a little closer and i was like oh shit that's like an actual bug we need to fix that 00:03:05.120 --> 00:03:09.539 so you heard it here first like the ai code reviews are better than Dillon's code reviews 00:03:10.160 --> 00:03:17.360 yeah but my like my take is that we're getting so reliant on it that it's just making us dumber 00:03:17.800 --> 00:03:25.060 and then we're like oh the ai will catch it um i'm not gonna like review this as in depth as i used 00:03:25.160 --> 00:03:31.380 to maybe that's that's my hot take 00:03:25.160 --> 00:03:31.380 i totally feel that with ai in general but i i just 00:03:32.620 --> 00:03:38.199 not to float off topic too much i feel like maybe i was listening to syntax so i'm syntax pilled 00:03:38.840 --> 00:03:41.520 but it is how I've always felt. 00:03:41.660 --> 00:03:42.460 It's like we're becoming, 00:03:42.920 --> 00:03:44.700 like these smaller tasks are becoming like, 00:03:45.160 --> 00:03:46.860 I also read an article about this, I believe. 00:03:47.380 --> 00:03:50.960 Smaller tasks are becoming more AI driven 00:03:51.600 --> 00:03:54.000 and we're like kind of orchestrating things 00:03:54.080 --> 00:03:55.160 from a higher and higher level. 00:03:56.220 --> 00:03:57.100 Basically, like you remember, 00:03:58.060 --> 00:04:00.400 I started as like an HTML CSS developer 00:04:01.140 --> 00:04:03.500 and now like that's not really, 00:04:03.760 --> 00:04:05.480 I mean, it matters in some sense, 00:04:05.620 --> 00:04:06.900 like things that need to be really accessible, 00:04:07.780 --> 00:04:12.260 products that have to worry about it but like people aren't hiring for that like the scope is 00:04:12.380 --> 00:04:17.859 just getting larger so like a front end scope has gotten a lot larger let alone like even though 00:04:18.140 --> 00:04:24.300 we're still kind of front end um you just need to be able to ship your entire app nowadays you need 00:04:24.340 --> 00:04:27.800 to be able to build it from the ground up and have an understanding of how the pieces work together 00:04:27.980 --> 00:04:34.479 the more you know about different parts of the stack the more useful you still are matt's matt's 00:04:34.500 --> 00:04:36.620 That's just like giggling it up over here. 00:04:36.620 --> 00:04:38.620 I have no idea what's going on. 00:04:38.880 --> 00:04:39.740 I have no idea. 00:04:40.010 --> 00:04:42.280 So I kind of tuned out what you were saying, to be honest, 00:04:42.910 --> 00:04:45.420 because I have no idea how that relates to AI code reviews. 00:04:46.910 --> 00:04:50.260 So maybe you can connect the dots for me because I'm so confused. 00:04:50.590 --> 00:04:51.460 I'm lost in the sauce. 00:04:52.300 --> 00:04:54.140 It's not specifically code reviews. 00:04:54.380 --> 00:04:57.320 It's the fact that AI keeps taking smaller and smaller tasks, 00:04:58.200 --> 00:05:03.020 but the human still needs to do the larger and larger orchestration 00:05:03.130 --> 00:05:03.900 around those tasks. 00:05:04.880 --> 00:05:09.360 together that's kind of where things have been going sounds like we disagree which is great 00:05:09.490 --> 00:05:15.660 because i'm ready to i'm ready to throw down on this 00:05:09.490 --> 00:05:15.660 uh i don't know if we disagree it's just i 00:05:15.760 --> 00:05:19.420 don't know if i have like well enough formed thoughts on that thread yet i don't know but 00:05:19.470 --> 00:05:24.840 at the moment maybe i'm like really focused in on the code review thread like i want to like pick a 00:05:24.850 --> 00:05:28.580 little bit more at that and 00:05:24.850 --> 00:05:28.580 sure i just think that the code review is another piece of that pie like 00:05:28.720 --> 00:05:33.419 just because okay it can pick up some things it doesn't mean it picks up everything but at the 00:05:33.380 --> 00:05:38.920 same time what Dillon's talking about is like feeling of regression or not retaining memory 00:05:39.720 --> 00:05:45.580 or not feeling like you need to do as much um it's being a little too heavily reliant but you're 00:05:46.260 --> 00:05:49.820 it still matters elsewhere like you still should look at the PR review and have a fundamental 00:05:50.020 --> 00:05:52.800 understanding of how it's being built and if it's the right structure for what you need to do 00:05:53.260 --> 00:06:00.100 especially with AI it's can often be absolute crap and it could be convoluted like one of the 00:06:00.020 --> 00:06:07.340 biggest problems I have might be that just AI churns out extra lines of code when it could just 00:06:07.840 --> 00:06:12.240 reuse things that exist. So it doesn't really write things in a streamlined, concise manner. 00:06:13.200 --> 00:06:18.600 You know, sometimes adding extra code does make sense, but it kind of just creates band-aids on 00:06:18.680 --> 00:06:22.380 top of band-aids until it can get unwieldy. So those are things to look out for when you're 00:06:22.440 --> 00:06:29.440 doing PR reviews. All right. Well, back to talking about reviews. Anyone else want to talk about it 00:06:29.460 --> 00:06:32.980 before I tell you why AI code reviews are great. 00:06:34.040 --> 00:06:37.080 Since Dillon was sharing the three or so products 00:06:37.340 --> 00:06:38.500 that Whoop is trialing, 00:06:39.260 --> 00:06:41.140 we can talk a little bit about what HubSpot does. 00:06:42.040 --> 00:06:44.600 Right now we have this bot called Sidekick, 00:06:44.780 --> 00:06:47.080 which is both a general internal chat agent. 00:06:48.070 --> 00:06:49.760 So you can just go to it and ask questions 00:06:50.000 --> 00:06:50.880 about our code base or whatnot. 00:06:51.640 --> 00:06:53.660 But it's also the same thing that powers our code reviews. 00:06:56.000 --> 00:06:57.999 And it recently got rolled out 00:06:58.020 --> 00:06:59.080 to basically every repo. 00:06:59.830 --> 00:07:01.540 I think it was doing an initial trial 00:07:01.660 --> 00:07:03.720 of infra-based teams using it. 00:07:06.360 --> 00:07:10.060 But I would say maybe we saved the takes for it. 00:07:10.120 --> 00:07:11.220 But yeah, it's interesting. 00:07:12.860 --> 00:07:14.340 But then we also, even before that, 00:07:14.410 --> 00:07:17.300 even before AI, we had this review bot called Sparrow, 00:07:17.660 --> 00:07:19.780 which would go in and auto-approve PRs 00:07:19.900 --> 00:07:22.580 that don't have changes to quote-unquote 00:07:22.730 --> 00:07:23.940 user-facing things. 00:07:24.550 --> 00:07:26.280 So if you only change Markdown in a repo, 00:07:26.340 --> 00:07:30.640 Sparrow could auto-approve your PR, and you can merge without getting a human review, which is kind of nice. 00:07:32.660 --> 00:07:38.500 So, like, I don't know, there's kind of this spectrum of, quote-unquote, like, AI review tools. 00:07:38.650 --> 00:07:46.340 Like, there's sort of automated review cycles that maybe aren't very AI-y, but then there's also, like, actual agents reviewing code. 00:07:47.800 --> 00:07:50.080 Yeah, Scott, tell us how Airbnb works. 00:07:52.100 --> 00:07:54.820 We use Claude again. 00:07:55.500 --> 00:08:02.840 So we have a like Claude PR review checker that just like runs every time you push an update to your PR. 00:08:03.620 --> 00:08:05.000 I find that the most useful. 00:08:05.060 --> 00:08:10.100 I guess that's what I'm advocating for is it often finds things you might not notice. 00:08:10.500 --> 00:08:16.300 Sometimes they're not real problems or they are overly safe. 00:08:17.360 --> 00:08:22.680 I remember like specifically, like I had a handler that was like check toggling something on and off. 00:08:24.400 --> 00:08:28.700 And the handler lived in some conditional logic that could only happen based on the conditional logic. 00:08:29.020 --> 00:08:36.159 Claude review basically told me, like, you also need that condition that it lives in to make sure that it's safe. 00:08:36.550 --> 00:08:38.780 So, like, I thought it was, like, overkill. 00:08:38.909 --> 00:08:45.560 Like, I don't need to say this handler that only lives inside this condition should also check this condition. 00:08:46.150 --> 00:08:47.660 But I just did it to satisfy it. 00:08:48.360 --> 00:08:51.940 So there are some situations where it's not, where it's a little bit overkill. 00:08:52.160 --> 00:09:02.780 I've also seen this, I think, something I was doing last night where I'm working with this diagram, third-party app, and it has a repaint method that I just fire. 00:09:03.620 --> 00:09:17.460 And it didn't like or didn't notice that and expected it to run through a regular React re-render, although it's all singleton pattern, so it's a little strange as is. 00:09:17.680 --> 00:09:22.960 So I ended up making the change that it asked for, but it was not really necessary. 00:09:23.180 --> 00:09:28.260 So sometimes it's overkill, but it does find like a lot of little bugs that you might not 00:09:28.480 --> 00:09:28.560 see. 00:09:28.940 --> 00:09:32.280 And it gives you, it just basically does a code review and it's just like an extra layer. 00:09:32.570 --> 00:09:39.020 We also have in Claude Code, a way to run it on a review. 00:09:39.440 --> 00:09:44.340 So you can just have like a specific run and just get its opinion on it. 00:09:44.380 --> 00:09:49.420 So it's just like another similar run, but it'll run through locally a little bit better. 00:09:50.560 --> 00:09:52.180 So that's kind of how we're using it now. 00:09:52.460 --> 00:09:57.420 I believe before I started, they may have, they went back and forth on whether it should 00:09:57.420 --> 00:09:58.160 be used for code review. 00:09:58.900 --> 00:10:04.760 They might have used it to deploy or rather like stamp reviews, but we did not do that. 00:10:04.870 --> 00:10:09.220 So we still make a human be like the last line of defense, which I actually, as much 00:10:09.280 --> 00:10:13.320 as I advocate that it's good to have code review from AI, I don't think it should be 00:10:13.320 --> 00:10:20.520 approving changes at all. It should just be telling you its thoughts. I guess if you're 00:10:20.620 --> 00:10:25.640 talking about like low level changes, there could be things, but I always think the water 00:10:26.220 --> 00:10:32.660 waters get muddy here. Like, yeah, like it's a bug fix, but when you're in a really large code base 00:10:32.860 --> 00:10:36.440 and you do something that you think is trivial, like I think we've had this conversation 00:10:37.780 --> 00:10:43.280 about five minute deploys where you should be able to push these bug fixes so, so fast 00:10:43.580 --> 00:10:49.220 some like, or, or what people might call like a UI tweak, like some, some little quote unquote UI 00:10:49.400 --> 00:10:53.720 tweak, like CSS is more powerful today than it was when it was just, I mean, I guess it's always 00:10:53.920 --> 00:10:58.080 kind of been, but it's not just like moving elements on a page. Sometimes like you would 00:10:58.110 --> 00:11:02.360 have to like craft a list of things that are safe because you could, you put pointer events on 00:11:02.540 --> 00:11:09.160 something, right. And now you can't click a button. So there are things that can happen, uh, if not 00:11:09.180 --> 00:11:14.320 appropriately tested especially in the five minute deploy world where we have zero integration tests 00:11:14.560 --> 00:11:19.520 because we need to deploy immediately over anything else um so i i do think there are like 00:11:19.760 --> 00:11:24.020 some things that you might be able to filter for but ideally like a human just looks at it last 00:11:24.220 --> 00:11:31.540 to give it that overall approval 00:11:24.220 --> 00:11:31.540 Matt, it sounded like maybe this is going back to um having like ai 00:11:32.240 --> 00:11:36.940 auto-approved things are you it's not like Matt was in favor of that and i feel like 00:11:38.319 --> 00:11:40.880 that Matt's take on that is a hot take. 00:11:41.640 --> 00:11:43.380 Like allowing AI to approve code. 00:11:43.780 --> 00:11:44.440 To me, it's just like, 00:11:44.780 --> 00:11:46.560 just give your engineers more admin privileges. 00:11:46.680 --> 00:11:48.580 And if they really, really need to merge something, 00:11:48.780 --> 00:11:49.600 just let them merge it. 00:11:50.100 --> 00:11:51.220 I know that seems kind of crazy. 00:11:51.920 --> 00:11:52.720 I just think like, 00:11:52.840 --> 00:11:54.500 we need to just educate our engineers better 00:11:54.980 --> 00:11:55.900 so they can make good decisions. 00:11:57.020 --> 00:11:58.180 At least I feel like we're pulling on 00:11:58.200 --> 00:11:58.920 some very similar threads 00:11:59.060 --> 00:12:00.120 to the five minute deploy episode. 00:12:01.660 --> 00:12:02.640 Like my take is that 00:12:03.620 --> 00:12:06.220 you should basically never be held up 00:12:06.880 --> 00:12:09.040 from being able to merge changes in, 00:12:10.280 --> 00:12:12.220 as long as that is completely decoupled 00:12:12.400 --> 00:12:14.280 from automatically releasing your changes 00:12:14.560 --> 00:12:15.760 to customers or to users. 00:12:16.700 --> 00:12:20.200 So it works really well for platform teams 00:12:20.260 --> 00:12:22.100 that work on versioned libraries, for example. 00:12:22.420 --> 00:12:24.260 Internally, we cut releases. 00:12:26.000 --> 00:12:27.680 Technically, we do publish on every PR, 00:12:27.720 --> 00:12:31.440 but we don't actually stabilize or promote that release 00:12:31.680 --> 00:12:34.880 to a stable version automatically. 00:12:35.380 --> 00:12:40.020 So for us, there's a gap between when we merge changes in 00:12:40.020 --> 00:12:43.000 before it's released, which means that technically, 00:12:43.090 --> 00:12:45.340 we should be able to merge things faster. 00:12:46.500 --> 00:12:50.240 And so my general take there is, yeah, 00:12:50.680 --> 00:12:53.440 we should entrust developers to make the right decision 00:12:53.780 --> 00:12:59.280 and move fast, meaning in the case of AI code reviews, 00:13:00.420 --> 00:13:03.200 allow the AI to ship a PR if it thinks it's good enough. 00:13:05.120 --> 00:13:08.800 You can still have humans come in and review PRs and whatnot, 00:13:09.120 --> 00:13:14.540 but I think it's like you should empower the team to move fast 00:13:14.670 --> 00:13:17.620 in order to fix issues and like roll out features. 00:13:18.320 --> 00:13:21.200 So I'm going to be told that I'm just agreeing with Matt 00:13:21.360 --> 00:13:22.480 and I'm disagreeing, 00:13:22.860 --> 00:13:25.860 but I agree that you should be able to move fast, 00:13:25.950 --> 00:13:28.440 but there's like, it's the same argument, 00:13:28.590 --> 00:13:32.320 whether it's AI code reviews or it's deploy speed, 00:13:33.140 --> 00:13:36.520 It's stability versus speed. 00:13:37.200 --> 00:13:39.240 And at some level, there's a balance. 00:13:39.760 --> 00:13:43.240 Like I agree in the sense that we should make things fast, 00:13:43.940 --> 00:13:47.240 but stability comes with some slowing things down. 00:13:49.360 --> 00:13:53.280 And speed comes with not having every single gate in your way. 00:13:53.660 --> 00:13:56.080 So yes, I think there's a balance between the two, 00:13:57.160 --> 00:14:00.140 but I don't think that we should just yeet out things 00:14:00.800 --> 00:14:04.240 and find a bug that we may have put out there 00:14:05.960 --> 00:14:06.600 X long ago 00:14:06.800 --> 00:14:08.000 because we just wanted to get things out 00:14:08.000 --> 00:14:08.760 as fast as possible. 00:14:09.320 --> 00:14:11.200 Speed inherently introduces risk. 00:14:13.100 --> 00:14:16.240 I disagree that they're related to one another. 00:14:17.480 --> 00:14:18.520 I think they're mutually exclusive 00:14:18.720 --> 00:14:19.780 or can be mutually exclusive. 00:14:20.080 --> 00:14:20.560 But again, yeah, 00:14:20.800 --> 00:14:22.260 like I think maybe we're entering into 00:14:22.980 --> 00:14:24.560 five minute deploy episode two 00:14:25.900 --> 00:14:28.740 versus the code review aspect. 00:14:29.040 --> 00:14:31.140 I don't see them as mutually discreet. 00:14:31.150 --> 00:14:33.240 I mean, like maybe two different teams own them, 00:14:34.040 --> 00:14:37.600 but at some level they still work hand in hand. 00:14:38.220 --> 00:14:41.000 No, like, again, like this sort of gets back to my point 00:14:41.160 --> 00:14:43.880 from our fast deploy episode where it's like, 00:14:44.620 --> 00:14:45.920 just because you're deploying fast 00:14:46.180 --> 00:14:47.360 doesn't mean you're risking stability. 00:14:48.380 --> 00:14:50.360 Like those are unrelated things. 00:14:50.980 --> 00:14:54.080 You can have great fast deploys and great stability. 00:14:55.200 --> 00:14:55.980 That I disagree with. 00:14:56.030 --> 00:14:57.900 I think in a perfect world, that would be nice, 00:14:58.140 --> 00:15:00.500 but I don't think that that right now exists. 00:15:01.020 --> 00:15:02.860 Everything gets faster, everything gets bigger, 00:15:03.510 --> 00:15:06.240 but I don't think there's some perfect world currently 00:15:06.450 --> 00:15:08.600 where that exists, especially at a corporate level. 00:15:09.160 --> 00:15:10.240 But I know we're getting off topic, 00:15:10.500 --> 00:15:12.020 so I'll go back to the code reviews. 00:15:14.220 --> 00:15:15.660 I'll go back to the code reviews, how about that? 00:15:16.440 --> 00:15:19.500 Is it really beneficial to have an AI agent 00:15:19.810 --> 00:15:22.020 auto-approve your PR versus just not having 00:15:22.500 --> 00:15:25.220 a requirement for approvers on specific files? 00:15:25.430 --> 00:15:27.460 Like, what's the difference, and is it actually valuable? 00:15:28.260 --> 00:15:30.580 maybe also to like step back a little bit and provide some context so 00:15:31.839 --> 00:15:37.720 the what the the bot that i called sidekick at hubspot that can't approve prs currently 00:15:38.680 --> 00:15:43.580 i think they had an initial like phase early on where it could approve prs and then they realized 00:15:43.660 --> 00:15:48.140 that engineers could prompt sidekick to create a pr and then have it review the pr and approve it 00:15:48.320 --> 00:15:53.839 and then be able to merge it without any human looking at it or writing a code so that was quickly 00:15:53.860 --> 00:16:00.620 squashed. 00:15:53.860 --> 00:16:00.620 That was a hack that they discovered is what it sounds like. 00:15:53.860 --> 00:16:00.620 Yeah, exactly. So yeah, 00:16:00.620 --> 00:16:05.540 I believe it can't approve PRs right now. I think that was turned off. But we do have this other bot, 00:16:06.260 --> 00:16:12.320 Sparrow, but that's not an agent. That's just a very basic static analysis of the changes in a PR, 00:16:12.790 --> 00:16:15.620 right? If it sees that the changes in the PR only change markdown, for example, 00:16:16.220 --> 00:16:21.980 it can approve that. Because collectively, as an organization, we've said changes to markdown in 00:16:21.980 --> 00:16:25.920 repos won't have a user facing impact. 00:16:26.630 --> 00:16:30.020 Um, you know, and certainly in some cases, like maybe more for platform 00:16:30.240 --> 00:16:33.140 teams where you might use markdown as source content for a documentation 00:16:33.440 --> 00:16:37.820 website that faces like that's internal facing, like that gets a little bit 00:16:37.900 --> 00:16:41.280 murkier, but generally speaking, you know, a change to a markdown is not 00:16:41.410 --> 00:16:46.500 going to break your application or, um, you know, lead to bugs. 00:16:47.080 --> 00:16:50.619 So like that, that, that was sort of like chosen to support the approval 00:16:50.660 --> 00:16:50.840 workflow. 00:16:53.360 --> 00:16:55.640 Dillon, in terms of your question, what's 00:16:55.640 --> 00:16:58.120 the difference between having agents that can approve PRs 00:16:58.290 --> 00:17:00.920 versus just removing the code review requirement? 00:17:01.980 --> 00:17:04.600 I think it still has a little bit of friction 00:17:05.089 --> 00:17:08.660 to the release workflow, where it's like you have at least 00:17:08.860 --> 00:17:11.699 something evaluating the change that's 00:17:11.959 --> 00:17:13.600 other than the person that authored the change 00:17:14.680 --> 00:17:16.439 in order to determine if it's safe to merge or not. 00:17:17.140 --> 00:17:19.120 Whereas if you remove the requirement, 00:17:19.339 --> 00:17:21.319 then it's like, yeah, there is no gate. 00:17:23.400 --> 00:17:25.900 I'm not saying it's equivalent to a human reviewer, 00:17:26.260 --> 00:17:30.800 but it is a little bit more than no review at all is my take. 00:17:31.000 --> 00:17:32.160 I mean, the more I think about it, 00:17:32.200 --> 00:17:35.760 it's like an additional linting step in a sense 00:17:35.980 --> 00:17:40.060 that can sometimes like lint for control flow being correct. 00:17:41.700 --> 00:17:42.720 Or if that makes sense. 00:17:43.640 --> 00:17:45.640 Yeah, this is the way I've been thinking about it also, 00:17:45.840 --> 00:17:49.620 where it's like, I think that the real, 00:17:49.860 --> 00:17:52.640 like the sort of the bright spot of agents doing code review 00:17:52.780 --> 00:17:55.640 is like a little bit smarter than a generic linter 00:17:55.920 --> 00:17:59.480 to validate code patterns that are being added in a PR. 00:18:01.780 --> 00:18:04.260 Like you probably could author lint rules 00:18:04.400 --> 00:18:07.240 to do some of that stuff, but like at certain points, 00:18:07.320 --> 00:18:10.020 it gets very tedious to try to author like a lint rule 00:18:10.200 --> 00:18:12.680 or static analysis rule that will, you know, 00:18:12.880 --> 00:18:15.800 assert that you're architecting your application 00:18:15.820 --> 00:18:21.200 way, right? Whereas an agent can kind of internalize some of those concepts for you and be able to provide feedback on it. 00:18:21.580 --> 00:18:27.520 One of the things this made me think of is if I'm using Claude in my editor 00:18:27.820 --> 00:18:34.320 to write my code, why do I need Claude to also review my code? But sort of my counterpoint to my 00:18:34.400 --> 00:18:39.580 own thought was some people aren't even using AI to write their code. So it's kind of nice to have 00:18:39.600 --> 00:18:46.180 like a final check that's like pseudo mandatory not really mandatory but just happens for free 00:18:46.920 --> 00:18:51.760 and it can catch issues that maybe would have been caught if you'd used ai before i'm not saying 00:18:52.250 --> 00:18:57.180 that ai is always good like i've i have an example of yesterday when i was trying to use it to to 00:18:57.180 --> 00:19:02.820 plan something out and it was using docs that were just completely outdated and giving me like 00:19:02.960 --> 00:19:11.140 really bad ideas so but that's another episode 00:19:11.300 --> 00:19:17.760 that's a good point and like it's another reason 00:19:11.300 --> 00:19:17.760 like a lot of people have multiple instances running or you know multiple agents running 00:19:18.120 --> 00:19:27.280 and asking um like iterative process questions and prompts basically one agent or or just one 00:19:27.440 --> 00:19:34.040 instance of chat might not do all the work or might not catch everything. And weirdly, 00:19:34.210 --> 00:19:41.780 we have this channel called Shit That Claude Says. Claude gets lazy sometimes, which is 00:19:42.630 --> 00:19:50.140 very on trend for developers. But it's funny to me because essentially sometimes it takes shortcuts 00:19:50.620 --> 00:19:54.360 if you don't prompt it otherwise in some spots. 00:19:54.740 --> 00:19:57.940 So it checking its work is very similar to that, 00:19:58.010 --> 00:20:00.080 having multiple instances running 00:20:01.839 --> 00:20:05.740 and keep drilling down on what you need. 00:20:05.970 --> 00:20:07.460 I guess it can only... 00:20:08.240 --> 00:20:09.740 I noticed issues with context, 00:20:09.950 --> 00:20:14.400 and I've seen people build multiple instances 00:20:14.920 --> 00:20:17.200 where it's constantly checking what it's doing. 00:20:17.880 --> 00:20:20.620 So it's like, I think it's problems that personally, 00:20:20.890 --> 00:20:22.100 and I'm speaking about Claude, 00:20:22.470 --> 00:20:24.340 but Claude should eventually be able to do, 00:20:24.520 --> 00:20:27.600 but like it can only handle so much context at once, 00:20:27.730 --> 00:20:28.680 I guess context bloat. 00:20:29.440 --> 00:20:30.760 So when you have multiple instances running, 00:20:30.870 --> 00:20:32.040 they can keep each other in check. 00:20:33.060 --> 00:20:37.520 So this is like an example of you're running this one, 00:20:37.880 --> 00:20:42.420 this one instance locally and another instance whose job is like more siloed 00:20:42.540 --> 00:20:44.900 to checking certain things can, 00:20:45.140 --> 00:20:46.660 can make sure that it's doing a better job. 00:20:47.300 --> 00:20:54.940 One instance, one big model that is really good is going to miss a few things here and there, for now at least. 00:20:55.720 --> 00:21:01.020 I had experimented a little bit with doing like quote unquote like kind of like red teaming with AI locally. 00:21:01.450 --> 00:21:04.220 So I would have Claude, you know, implement the change. 00:21:04.710 --> 00:21:09.840 And then I would basically stop that agent and like start a new one without preserve. 00:21:10.040 --> 00:21:12.620 Or, you know, nowadays I would do like slash clear with new. 00:21:13.880 --> 00:21:23.720 And then I would say, okay, I have changes locally, you know, review these changes, make sure they accomplish this problem and or like, you know, serve to fit this problem and whatnot. 00:21:24.680 --> 00:21:27.660 So kind of as like a pre-code review code review in a sense. 00:21:28.820 --> 00:21:29.760 And I actually found it pretty useful. 00:21:29.940 --> 00:21:35.780 Like it was able to catch some like logical bugs that the initial agent wasn't able to find itself. 00:21:36.500 --> 00:21:40.740 It's like, give me someone or give me an agent with a fresh perspective. 00:21:42.220 --> 00:21:44.840 And sort of a different mission in mind. 00:21:45.740 --> 00:21:46.440 Exactly, yeah. 00:21:46.940 --> 00:21:48.400 You net better results, potentially. 00:21:49.380 --> 00:21:53.320 Yeah, which is, I think, the whole value proposition 00:21:53.620 --> 00:21:56.200 behind the mixture of experts, mixture of agents 00:21:56.420 --> 00:22:00.320 sort of architecture or pattern, right? 00:22:00.440 --> 00:22:03.680 You have a collection of different specialized things 00:22:03.980 --> 00:22:07.800 that can handle different tasks better than others. 00:22:10.120 --> 00:22:17.380 But I'm not suited to, like, that's my, like, maybe the limit of my understanding of those concepts or implementation. 00:22:17.740 --> 00:22:19.500 So I'm not suited to talk more about that. 00:22:20.860 --> 00:22:28.340 I think ironically, you guys maybe made me argue for the opposite approach of what I was thinking originally coming in here. 00:22:28.840 --> 00:22:39.740 So I think it's valuable to have AI agents review code, but I don't think it's at the point where it's an equivalent review of a human reviewing a code. 00:22:40.180 --> 00:22:41.840 So I think it's not that valuable. 00:22:42.340 --> 00:22:43.080 We agree. 00:22:43.780 --> 00:22:50.240 I mean, I had this stance before the podcast, but I feel like you forced me down the alternative path. 00:22:50.240 --> 00:22:51.540 Well, that's the goal. 00:22:51.720 --> 00:22:53.040 We need to make our two... 00:22:53.200 --> 00:22:53.600 Oh, sorry. 00:22:54.000 --> 00:22:57.460 Our three regular listeners need some controversy. 00:22:58.700 --> 00:23:03.600 who's the third 00:22:58.700 --> 00:23:03.600 i mean i don't know who they are i just know we have three in the last like 00:23:03.920 --> 00:23:11.300 four episodes 00:23:03.920 --> 00:23:11.300 it's just us 00:23:03.920 --> 00:23:11.300 it's just us all right let's talk about this like 00:23:11.960 --> 00:23:20.020 what are the cons of having ai do code review like is there a con here i mean i think my only con is 00:23:20.120 --> 00:23:29.920 Sometimes it makes potential frivolous, more explicit code be written that might not necessarily be necessary. 00:23:30.380 --> 00:23:32.060 It doesn't really slow down the process. 00:23:32.440 --> 00:23:35.160 It's one extra level added gate. 00:23:35.440 --> 00:23:39.000 Maybe cost, if we're worried about cost. 00:23:39.460 --> 00:23:41.100 Is this the best way to spend your tokens? 00:23:42.339 --> 00:23:43.460 Maybe? Maybe not. 00:23:45.620 --> 00:23:49.100 But I don't see a con in adding an extra set of eyes. 00:23:49.660 --> 00:23:57.860 I get that there's no eyes yet for AI, but like more caution that runs as a pipeline sounds great to me. 00:23:58.280 --> 00:24:09.300 It's just like one, it's just AI basically being some sort of lint-like feature that you don't have to build or find and maintain. 00:24:11.560 --> 00:24:18.180 One example that comes to mind, and we didn't necessarily see this in the code review aspect of Sidekick, 00:24:18.800 --> 00:24:26.780 But when adopting it as like a coding agent for our use cases, we had like maybe, we don't 00:24:27.080 --> 00:24:34.140 do any like post training or anything like that to customize the agents for our code bases. 00:24:34.620 --> 00:24:38.700 But we do have a set of one pagers that we basically say like, okay, if this looks like 00:24:38.800 --> 00:24:43.820 a front end code repo, then fetch this one pager about front end coding at HubSpot. 00:24:44.120 --> 00:24:48.280 If this looks like a Java repo, like a backend repo, then fetch this one pager about Java apps 00:24:49.560 --> 00:24:51.800 And we have a number of these different one pagers that we fetch. 00:24:52.340 --> 00:24:53.740 Tell me more about the one pager. 00:24:53.760 --> 00:24:55.840 I'm curious, like what are people getting from this? 00:24:56.780 --> 00:24:58.340 It's not for people, it's for agents. 00:24:59.520 --> 00:25:02.160 It's, it's expl, I mean, a human can read it. 00:25:02.240 --> 00:25:03.280 Obviously it's just a markdown doc. 00:25:03.560 --> 00:25:08.440 So a human can read it, but it helps, helps tell the agent, here's how we write front end 00:25:08.580 --> 00:25:14.880 code, or here's how we use CHIRP, our like RPC framework, or here's how you scaffold a 00:25:14.980 --> 00:25:16.420 Java service, et cetera. 00:25:18.260 --> 00:25:31.000 Yeah. But one thing that we found, at least on my team, is that we're simultaneously not a front-end repo, but also not a back-end repo, at least in the front-end tooling that we maintain. 00:25:32.880 --> 00:25:42.240 And so it kind of got a little bit confused on patterns to recommend or how to write code in our repo, how to run the repo, how to validate changes, stuff like that. 00:25:43.440 --> 00:25:50.060 So I think that's maybe one risk of the, or like con of like using the AI agent, depending on how 00:25:50.160 --> 00:25:57.960 well you've like configured it to maintain context about your code base, right? Like not all, you 00:25:57.960 --> 00:26:02.400 know, maybe, maybe a good example of potentially, right? Like taking whoops code base, for example, 00:26:02.520 --> 00:26:07.060 as far as I know it, like where it's a monorepo of packages, like you might have a package in there 00:26:07.140 --> 00:26:12.300 that's a little bit more frameworky versus a package that's a little bit more front, like 00:26:12.300 --> 00:26:17.380 focused and like knowing the difference between those two things can be easy maybe easy for a 00:26:17.500 --> 00:26:23.700 human to spot but you know an agent might not be I don't know aware of the those distinctions and 00:26:23.740 --> 00:26:28.460