I stumbled across Matt Pocock's improve-codebase-architecture skill on YouTube and thought it was worth a shot. What I really wanted was better test coverage on PeerTalk, a networking library for old Macs, and BomberTalk. The theory was that pulling the tangled parts into deep modules behind small interfaces would make them testable. This post is about what the skill found, what I decided to act on, and why.
#What the skill does
The skill reads the codebase and writes a report: which parts are pulling their weight, which aren't, and a before-and-after for each, in rough priority order. You read it and pick what to act on.
What counts as pulling its weight comes from John Ousterhout's book A Philosophy of Software Design. His central idea is the split between a "shallow" module, whose interface is nearly as fiddly as the code behind it, and a "deep" one, which hides a lot of work behind a small, simple interface, so the rest of the code doesn't have to deal with it. Deep is what you want. The skill reads your code for the shallow spots and suggests ways to deepen them. The book is worth reading whatever stage you're at. The skill takes this one idea from it and runs it over your code.
#PeerTalk: one place for the rules
PeerTalk talks to three networking stacks: modern Unix sockets, and two old Apple ones, MacTCP and Open Transport. Each backend carried its own copy of the rules for opening and closing a connection. That left three copies to keep in step, and I could only test each one on the hardware it drove, so a mistake in one didn't surface until I ran it on a real Mac.
The report drew a thin boundary it called a "seam". I moved the rules into one core above the line and left the backends below it doing nothing but reporting what happened: connected, data, closed. I also trimmed a dead slot from the backend interface and pulled the last timeout logic up into the core. After that there was one copy of the rules instead of three, and because it sat in plain code above the hardware, I could drive it from a fake backend on my laptop.
How did I end up with three copies? The backends were written in one go, in a spec-driven rewrite where each one was its own task. The interface I'd given a backend made it do two jobs: talk to its OS, and run the connection rules. The OS part really is different for POSIX, MacTCP and Open Transport, but the rules are the same, and I'd bundled them in with the OS code. So each backend task wrote the rules out again. This is easy to get when you build with an agent: it does what the interface asks in every task, and the copies land in separate files, so nobody's typing the same thing twice and noticing. I didn't spot it until the skill read all three next to each other.
#PeerTalk: who dials
When every machine connects to every other, something has to decide who dials whom. If both ends dialled at once I got two half-open connections and a coin-toss over which survived. A lost toss left a link that looked alive but delivered nothing, which showed up in the game as a player frozen on the spot. BomberTalk's lobby carried about seventy-five lines to stagger, retry and time out around it.
I moved that job into PeerTalk. Of any two machines, only the lower-address one dials, and the other waits. One comparison settles it, so both ends can't dial at once. I turned it on in one line and deleted BomberTalk's seventy-five. PeerTalk is the right home for it because it already knows both addresses, and the game shouldn't have to.
This one's less about AI. BomberTalk wanted every machine talking to every other, so it grew its own code to stagger and retry connections. PeerTalk already knew both addresses and could have decided who dials, but the game was patching something that belonged to the library underneath it. Those lines were the clue I'd missed: the wiring should have lived in PeerTalk all along.
#BomberTalk: pulling the rules out
BomberTalk had the same shape of problem. Its rules (collision, blast range, who's still alive, grid maths) were woven into the Mac drawing code. Running any of it needed a real Mac, so a bug in the pure logic survived all the way to the hardware before I could see it.
I pulled the rules into a "portable core": their own files that know nothing about Macs, with a thin Mac layer that only draws and reads input. That one boundary paid off twice. The core now tests on Linux in a fraction of a second, and the same rules picked up a modern desktop build (via SDL2) and a native Mac OS X build on both PowerPC and Intel. The pieces underneath are small enough to read on their own: collision, blast reach, who wins.
The rules got welded to the screen because I wrote them where I first needed them. The collision check went in next to the drawing code because that's where I was working, and nothing ever pushed it back out. I only noticed how stuck together they were when I tried to test the logic and couldn't do it without a whole Mac behind it.
#What it did for testing
The skill isn't a testing tool, but nearly every change it suggested made something testable that hadn't been before. Tangled code is the hard-to-test kind, so pulling it apart is what let the tests in.
PeerTalk went from 41 core checks to 120, driven by a fake backend that feeds the core made-up events so I can check the state logic without a network. BomberTalk went from none to 1,275 across eight files. BomberTalk's tests run on every push through CI. PeerTalk's I run on my own machine. Two of them pin down bugs that had only ever turned up on hardware.
#Which changes I kept
The report is a set of suggestions rather than a patch, so the last call is mine. It proposed six changes for PeerTalk. I took three and turned down three. One was a slot that looked dead, and fair enough: two of the three backends leave it empty. On MacTCP it re-arms UDP reception when discovery restarts, and deleting it would have scattered that timing into two other places and probably stopped discovery on the oldest machines. I only knew that because I'd been in that code. Another added machinery for a problem I don't have. The skill can spot that a module looks shallow, but it can't tell that one is quietly holding up the MacTCP path, so it's not something to run hands-off.
Code: peertalk, BomberTalk. Skill: Matt Pocock's.