Can Cursor's HARDCORE Review Skill Stop The Slop?

Why automated code review matters

00:00Automated code review is one of the most impactful ways that you can improve the code quality coming out of your agent. I've known this for a while, but it's taking me a while to kind of implement it and figure out a reusable skill that I can give to people to review their code.

00:16I've got this review skill here in my skills repo which is currently sitting at whoa 1,000 no sorry 109,000 stars and it is currently marked as in progress. I'm sort of okay with it, but I'm not terribly happy with it. So, I've been looking around for inspiration in other skills that I can copy from, steal ideas from, and one crossed my path I want to

Discovering thermonuclear code quality skill

00:39show you. It is this one from the Cursed team. It is the thermonuclear code quality review. Use this skill for an unusually strict review focused on implementation quality, maintainability, abstraction quality, and codebase health. And one thing I think is notable about this skill is how ambitious it asks the reviewer to be.

00:54It's asking it to be very ambitious and look for code judo moves throughout the review. The skill itself is simply one file. It's just a skill.md up here. And what I thought I'd do is I would copy it to my local system, try it out on some actual code of mine and see what it comes up

Testing the skill on Sandcastle

01:12with. Yesterday I spent a lot of time working on Sand Castle, my open- source software factory. And so I figure I would review the last x number of commits and see what it thought about them. So I'm going to be pretty loose here. I'm just going to say thermonuclear code quality review.

01:25Review the last five PRs that made it to main. I'm going to stick it on auto mode. And while it's doing this, let's go and actually read the skill because that should explain the skill a bit more. So, it starts from this baseline.

Breaking down the skill's instructions

01:37Perform a deep code quality audit of the current branch's changes. Rethink how to structure implement the changes to meaningfully improve code quality without impacting behavior. Work to improve abstractions, modularity, reduce spaghetti code.

01:48Improve succinctness and legibility. Be ambitious. If there is a clear path to improving the implementation that involves restructuring some of the codebase, go for it. Be extremely thorough and rigorous. Measure twice, cut once. What I've often found with review skills like this is that the agent is not ambitious enough.

02:06If you pass an agent a diff, then it will usually treat that diff as its bounds within which it can work. Whereas this prompt appears to be going beyond that. It's essentially saying look throughout the entire codebase for opportunities but starting from this current branch's changes.

02:24It also goes on to add a bunch of non-negot non-negotiable additional standards. Be ambitious about structural simplification. Again, the ambition, do not let a PR push a file from under 1k lines to over 1k lines without a very strong reason.

02:39This is really interesting. I've actually reached this conclusion myself as well. Large files are just quite hard for agents to navigate because they need to ingest the entire file into their context window in order to find the thing that's actually useful within it.

02:53A much better way to structure that is to split them into multiple files and let the kind of the file name of the file be the context pointer that tells it what's in that file and whether it might need to open it. This ends up being a lot more context efficient.

03:08I have generally split my files if they go over 5k tokens, but this 1k lines is sort of I guess a similar rubric. Do not allow random spaghetti growth in existing code. Okay, I see. It's sort of arguing against nesting here. If a change adds weird if statements in random places, treat that as a design problem, not a stylistic knit.

03:27Prefer pushing the logic into a dedicated abstraction helper state machine policy object or separate module instead of tangling an existing path. Interesting. This is another way of telling it to be aggressive about, you know, if there's a bunch of nested if statements and weird conditionals, maybe abstract that into a cleaner abstraction or a helper or something.

03:46It's arguable whether I prefer that. I suppose sometimes I do, sometimes I don't, but let's assume it's a good thing for now. Bias towards cleaning the design, not just accepting working code. Again, pushing it to be ambitious. Prefer direct, boring, maintainable code over hacky and magical code.

03:58This is like a classic one in these prompts. This comes from, I think, simplify in Claude Code. Not the same uh wording I think, but a similar idea that you want simple direct code that's easy to read. I really like this one actually. Push hard on type and boundary cleanliness when they affect maintainability.

04:15So we're specifically talking about types here. Question unnecessary optionality, unknown, any, or cast heavy code when a clearer type boundary could exist. This is kind of TypeScript focused here. unknown and any are specifically TypeScript um terms and the unnecessary optionality is one that always gets me.

04:37Whenever an agent adds a prop onto a React component, let's say, it always adds it as optional. I don't know why. I don't know why it's so stupid. Even when it's always required, it will add it as optional just to make it backwards compatible or something or to lessen the blast radius of the change.

04:52So yeah, question unnecessary optionality is a great one. Keep logic in the canonical layer and reuse existing helpers. Prefer existing canonical utilities and helpers over bespoke one-offs. Yes, I suppose it's basically just telling it to look for places where this has already been solved in the codebase and use those instead.

05:11Makes sense. Treat unnecessary sequential orchestration and non-atomic updates as design smells when the cleaner structure is obvious. If independent work is serialized for no good reason, ask whether the flow should run in parallel. I said, I see.

05:21This is about performance essentially. Obviously, when two things that are independent, if they run in parallel, then it's going to be faster than if they run sequentially. So, that's kind of what it's going for here. But it's also saying do not overindex on micro optimizations.

05:36Okay, so it's basically telling it don't go too far. I think if this was my skill, I would definitely rewrite this to be a lot more direct. Treat unnecessary sequential orchestration, non-atomic updates as design smells. That's just word salad to me.

05:48I don't know what that means. So, this is really cool. Primary review questions. For every meaningful change, ask, is there a code judo move that would make this dramatically simpler? That's great. I love that. Can this be reframed so that fewer concepts, branch, or helper layers are needed?

05:59Lovely. I don't love this. Does this improve or worsen the local architecture? You've got to say exactly what good and bad looks like to an agent in order for improve or worsen to mean anything. Overall, this set of questions along with the kind of rules above give the agent a nice kind of uh way in to talking about the code, which is what you need.

06:19And now it talks about really bad stuff. Escalate findings when you see a complicated implementation where a cleaner reframing could delete whole categories of complexity. Refactors that move code around but fail to reduce the number of concepts a reader must hold in their head.

06:30Yeah, there's a bit of repetition going on here, unnecessary casts, any unknown or optional params. What sort of scares me about these big reviewbased prompts is that this is a huge ball of mud for the agent to read. Like there's a lot of instructions in here and it's hard to know what to prioritize for the agent.

06:46So I don't know. This this makes me a little bit nervous. I do like this though. When you identify a code quality problem, perverse suggestions like delete a whole layer of indirection rather than polishing it. Again, ambition. Split a large file into smaller focus modules.

06:59Again, you know, making things easier to navigate for the agent. Again, duplication. Make type boundaries more explicit so the control flow gets simpler. There's a lot of duplication throughout a lot of this. This could be cut down, I think, quite a lot.

07:08Review tone. I don't know why this is here. This is just sort of saying um choose your tone. I suppose be direct, serious, and demanding about quality. Do not be rude. This seems like a crazy thing to add to a skill. I don't know why that's here.

07:21What this does do is it does um really punch the language that the agent should be using. So, we really emphasizing code judo. Uh saying decompose pushes the file past. Makes the surrounding code more spaghetti. I like that. Further down, we can say output expectations.

07:35Right, this is nice. It's saying to prioritize findings in this order. It's saying to float the important stuff to the top and legibility and maintainability concerns are at the bottom. Structural code quality regressions right at the top. Right.

07:47And it is asking for an approval here. So it's approving or rejecting the PR. And again, tons and tons of repetition here. This skill could be a lot shorter. So what we have is a large block of text that basically says be more ambitious. Here are some specific things that you can focus on in your review.

08:03Really go nuts here and like uh propose a ton of structural changes. Make sure that you uh prioritize your findings in a certain order so you don't flood it with useless crap. and then approve or reject based on these conditions. What I don't like here is there's no mention of testing.

08:18There's no mention of seams. There's no mention of any kind of like improving the feedback loops to make future runs better, which in my view is the entire point now of having a good codebase or having a code base that's easy to change and modular and easy to navigate.

08:33All of this appears to be focused on actual source code. None of it on tests.

Reviewing the feedback results

08:39Interesting. But okay, let's read what it said here. So, it's taken the last five PRs to main and it has found some blocker class structural issues. Okay, it's found that an init service is now a big file. So, it's over 1,000 lines and it mixes a bunch of stuff here and it should have been preceded by a split and it's proposed a nice split there.

08:58Oh, it's also trying to create an abstraction here. A little make registry generic function returning this would delete 20 lines of duplicated boiler plate at the same time. Feels good. That's nice. So, we now go to the next one. The feature specific if issue tracker name custom scattered across three layers.

09:17Interesting. It's basically saying that instead of this being a special case if statement here, we should instead do a bit of code judo and push the custom tracker variations into a type itself and then it can be read later. I think in terms of suggestions here, I happen to know this code quite well.

09:32I think we are at two out of two here. That's um seems like two really good suggestions. Down here we have an inconsistent contract. Template args carries both shell commands and pros markers. It's basically saying that some of these are runnable but some of these are not runnable here.

09:47And it's saying that maybe we should widen the type to either a command or a to-do marker discriminated union or use a different field entirely for unfilled markers. So it's basically it's basically trying to strengthen the type boundary here so that we don't later pass in a pros marker into something else.

10:05That's interesting. I think that this comes from an inaccurate understanding of the whole system, which is okay. You're going to get some false positives. I suppose the false positive in any review prompt. So, this is the kind of thing if it came up in a PR, I would say this is fine.

10:21Don't worry about it. So, two out of three, not bad. Let's go look at the strong code quality issues. Aha, we do have a weird uh bug here that it's not a bug, just a weird bit of code design. We essentially have different templates in sand castle that each declare the dependencies that they need and mostly they declare zod as their dependency.

10:38So we have this weird code path in here that looks like it just hardcodes zod and then interesting. What? Yeah, overall this is quite hard to explain, but it's definitely pulled up something weird here. So I think we're at three out of four, which is good.

10:53Oh, it's found some swallowed errors here. Excync inside effect.sync with swallowed errors. Interesting. We can see it's trying something inside here and if it fails then it just like returns false inside here. So yeah, this is definitely another thing that I would like the reviewer to look at.

11:09Looks like we started decomposing a large file into small files but only half finished. So this again is a good one. This is five out of six so far. And it's now saying there is a bit of prompt duplication within some prompts that were changed here.

11:24So the change is bite identical for two different prompts here. It's saying that we should refactor those into a issue list preamble. I don't think that's right. I think the prompts should just be independently um changeable. So, not bad though.

11:37Five out of seven. Then it's got a list of smaller items worth fixing here. I've done a quick scan and I would say most of those look pretty good. And interesting, I'm kind of intrigued by the approval bar here under the skills stated bar. A couple of the PRs should not have landed in their current shape.

11:54The behavior is correct in all three substantive PRs, but the codebase is meaningfully messier than it was a week ago. Well, cool. I mean, we got some really good feedback from this skill. I think I

Lessons learned and next steps

12:06think what this is teaching me is that actually getting the review to be super ambitious and getting it to push a lot of different options will give you more false positives. But those false positives are pretty easy just to say no to, right?

12:19It's the ones that you miss that you never know about. The opportunities for improvement that you never see. Those are the dangerous ones. Overall, I would clean up this skill so it's not quite so um duplicative, so there's, you know, a bit more dry.

12:36And I would also just get it to focus a lot more on tests as well. Think about the seams in your codebase, kind of like what my uh improved codebase architecture does. But overall, I think this is worth pulling down, experimenting with, and just seeing what comes out of it.

12:51Now, if you dig this stuff, then I'm running a cohort starting next week, starting June 1st, on AI coding for real engineers. This has been my most subscribed to course ever. People are going nuts for this. Uh we're going to have I think around 4,000 4,500 people in there.

13:07So, yeah, it's absolutely wild. But if you're enjoying this stuff, if there's a skill that you want me to review, I really like making that content because it lets me steal ideas from other people's great skills. Then let me know. Nice work and I will see you very