Three Modules, One Node Shape

I recently added Matt Pocock's mattpocock-skills plugin to my Claude Code setup, mostly out of curiosity about improve-codebase-architecture: a skill that doesn't wait for you to ask for a feature. Instead it goes looking for places where the existing code is quietly costing you, and hands back a report instead of a diff.

The skill borrows its vocabulary from John Ousterhout's A Philosophy of Software Design: a module is deep when a lot of behavior sits behind a small interface, and shallow when the interface is nearly as complicated as what's behind it. The test it applies to anything that looks shallow is simple: imagine deleting it. If the complexity just vanishes, it was dead weight. If it reappears at every caller, it was earning its keep.

Pointed at this blog's own codebase, a Next.js app on top of Payload CMS, it walked the areas that had seen the most recent churn (the Posts collection turned out to be the single most-edited file in the whole repo), and came back with four candidates, each with a before/after diagram and a strength rating from "Strong" to "Speculative." This series is that report, one candidate at a time, plus what actually happened once I built each one.

First up: three files that had each independently reinvented the same dozen or so lines.

The shape being copied three times

The blog's content field is Lexical rich text, and one thing I do with it is rewrite plain URLs into internal Payload relationships. A link that points at /blog/some-slug becomes a real reference to that post instead of a string that breaks if the slug ever changes. Three different scripts needed a version of this:

• one converts links while a post is first authored,
• one migrates links in already-published content when the scheme changes, and
• one just audits existing content for links it can't safely rewrite yet.

Each of the three had its own copy of the same tree-walk: recurse through a Lexical node tree, find anything shaped like a custom link, and, for the two that rewrite, replace its fields with an internal relationship. Each file also had its own private type guard to make the walk type-safe. None of the three imported from the others.

That's the "shallow module, times three" shape the skill was built to catch: the actual interesting logic (recognizing a link node, deciding what to do with it) was a few lines, and the ceremony of a type guard plus a recursive walk was reimplemented at least three times to get there. Delete any one copy and the complexity doesn't disappear. It reappears the moment you need the same walk for a fourth thing.

Pulling it into one place

The fix was mechanical once it was named: one module owning the walk and the rewrite, parameterized by a callback that only the three original call sites still need to supply.

export function collectClassifiedLinks<T>(
  node: unknown,
  classify: (url: string) => T | undefined,
  found: ClassifiedLink<T>[] = [],
): ClassifiedLink<T>[] {
  // one walk, however many callers need it
}

export function rewriteToInternalLink(link: ClassifiedLink, postId: number | string): void {
  // one rewrite, mutating the node in place
}

The three original files shrank down to supplying just their own rule for what counts as a match (a URL scheme here, a slug pattern there) and calling into the shared module for everything else. Their combined test suites, which had been re-proving the same walk-and-rewrite behavior three separate times, collapsed into one fixture-backed suite for the shared module, with each caller's tests trimmed down to just its own classification rule.

What the second pass of review caught

I ran an automated adversarial review against every pull request on this repo, a pass whose whole job was to try to break the "this is just an extraction, nothing changed" claim. On the follow-up here, it found something real: the slimmed-down tests only asserted on the rewritten node's metadata, the new internal relationship, and not on its visible text, which lives in a different part of the node entirely. Nothing was actually broken; the rewrite function only ever touches the metadata fields today. But that function is now the one place both the live "someone pastes a link in the editor" path and a one-time script that rewrites already-published posts in the database go through. A future change to it that got a little too enthusiastic, replacing the whole node instead of just its fields, would silently blank the visible text of every converted link in the database, and every existing test would still pass.

I restored the missing assertion, then proved it actually mattered: temporarily changed the rewrite to replace the node wholesale, watched the new test fail exactly as predicted, and put the working version back.

That's the pattern for the rest of this series, too. The report tells you what's shallow. It's the second pass of review (adversarial, and specifically looking for what the first fix missed) that tends to tell you whether the fix was actually complete. Next up: nine copies of the same try/catch block.