The Review That Caught What the Reviews Missed
Three tasks, each with its own tests, each individually reviewed and approved. Then one more review looked at the whole thing at once and found two real bugs that none of the three approvals had caught. Then the review that checked the fix found a smaller fourth thing.
b12n-publish is the pipeline behind this blog: it watches a handful of repos for activity worth writing about, gets a real draft from a real model call, and only after a human reads and substantially revises it does anything get published. Building the pipeline itself happens in a specific shape. Write a plan that breaks a feature into a handful of independently testable tasks. Work through the plan one task at a time, and give each task a fresh implementer with no memory of the tasks before it, just the plan and the one piece it's responsible for, plus its own dedicated reviewer before the next task starts. The bet is that a narrow, well-defined job catches things that accumulated context and creeping scope would let slide. Once every task is done and individually approved, one more review looks at the finished result as a single, whole change, not a task at a time.
That's the process net.b12n.publish.snippet, a module in this pipeline, went through while being wired into the real draft-writing workflow. It's worth writing up in detail, both because the bugs are concrete and because the story is a real test of the extra layer: does reviewing the whole thing, after every piece already passed on its own, actually find anything the pieces didn't? In this case, yes, twice over.
What the feature does, and why it exists
A marker in a draft, an HTML comment naming a repo, a file, a named region in that file, and a pinned commit, resolves to the real code at that commit, so a blog post never describes code that's since changed underneath it. It's a direct reaction to a rule this project's own design doc states plainly: never let a model retype code into a draft. The first real post this pipeline published broke that rule anyway. Every code block in it was hand-copied during revision, because the resolver existed but nothing called it. This post doesn't make that mistake: every code block below is quoted through the real marker system, pinned to the commit where it landed.
Three functions do the work:
snippet-markers-in-textfinds every marker in a block of text.resolve-markers-in-textreplaces each marker with the real, pinned code it points to.drifted-markers-in-textflags markers whose pinned region has since changed at HEAD. Drift isn't an error, it's a signal to review before publishing.
They were built as three separate tasks from a written plan, each with its own tests and its own dedicated reviewer.
What the per-task reviews actually caught
Before any task ran, a pass over the plan itself caught an editing artifact: a leftover "wait, let me recount" aside left in one task's instructions while fixing an arithmetic mistake. Small, but exactly what a pre-dispatch self-scan is for.
Each task's own review approved cleanly, and two of the three flagged something worth noting even though neither blocked merge.
Task 1's reviewer noted the tests didn't cover a marker embedded mid-line, only a marker alone on its own line, and suggested folding that case into a later task's tests if it wasn't naturally covered there. It wasn't. That gap turns out to matter a lot, two sections down.
Task 3's reviewer caught something sharper. The plan's own prescribed code used a bare keep call, which returns a lazy sequence. Two of that task's tests assert that a broken marker throws an exception:
(deftest resolve-markers-in-text-throws-for-a-bad-sha
(let [{:keys [repo-path]} (git-repo-with-region-change!)
text "<!-- snippet: agent-toolkit harness.clj#run-to-pause @deadbeef -->"]
(is (thrown-with-msg? Exception #"snippet source unreadable"
(snippet/resolve-markers-in-text {:text text :repo->path (constantly repo-path)})))))
A lazy keep doesn't invoke its function until something forces the sequence, and evaluating (is (thrown-with-msg? ...)) around a bare (keep ...) call just returns an unrealized lazy-seq object without running it. As literally prescribed, that test would very likely have passed without ever exercising the throw path it claims to test. The implementer caught this independently, wrapped the call in into [] to force it eagerly, and the reviewer verified the deviation was a necessary fix, not a bug to revert. A plan can be wrong in a way that only shows up once someone tries to run its tests for real.
What only the final review caught
A review of the whole branch, at once, on a more capable model, ran after all three tasks were individually green. It found two things no task-scoped review was positioned to see, because neither one lives inside a single task.
The first is exactly what Task 1's reviewer had flagged and deferred. resolve-markers-in-text finds a marker anywhere in a line, because parse-marker matches with re-find, not re-matches. But it replaces the entire line. Given a marker sitting inside a sentence rather than alone on its own line, the surrounding prose silently disappears. Not an error, not a warning. Gone. Here's the fixed version, which makes that loud instead of silent:
(defn resolve-markers-in-text
"Replace every snippet marker line in `text` with a fenced code block
containing that marker's pinned-sha region text. `repo->path` resolves
a marker's :repo keyword to a filesystem path — the caller's job
(registry lookup + scan/expand-path), matching resolve-snippet's
existing division of labor. Throws (see resolved-marker-text) rather
than shipping a post with missing code. Also throws if a marker
shares its line with other text — a fenced code block can't sit
inline in a sentence, so a line with prose before or after a marker
(or two markers on one line) is a marker misuse, not a resolvable
case."
[{:keys [text repo->path]}]
(->> (str/split-lines text)
(map (fn [line]
(if-let [marker (parse-marker line)]
(do
(when-not (re-matches marker-re (str/trim line))
(throw (ex-info (str "snippet marker must be alone on its line: " line)
{:type ::marker-not-alone-on-line :line line})))
(let [{:keys [text]} (resolved-marker-text repo->path marker)]
(str "```" (fence-lang (:file marker)) "\n" text "\n```")))
line)))
(str/join "\n")))
The second is about what actually breaks in practice, not what a hand-written fixture predicted. The original error handling checked whether the resolved region came back nil, the case where a file exists but the requested #region/#endregion markers aren't in it. But the most likely real mistake, a typo'd commit sha or a typo'd file path, doesn't produce nil. It makes the underlying git show process exit non-zero, which throws before the nil-check ever runs, with an empty exception message and an internal library error type that has nothing to do with this project's own error contract. The documented "throws a clear error" promise didn't hold for the single most likely way a human actually gets a marker wrong. The fix wraps the call and rethrows with a real message:
(defn- resolved-marker-text
"repo-path + pinned-sha region text for `marker`. Throws ex-info if
`repo->path` can't resolve `marker`'s :repo, if the pinned sha/file
can't be read at all (a typo'd sha or file path — extract-region's
underlying `git show` fails loudly, not with nil), or if the region
can't be found within an otherwise-readable file — no silent blank-
code fallback."
[repo->path {:keys [repo file region sha] :as marker}]
(let [repo-path (repo->path repo)]
(when-not repo-path
(throw (ex-info (str "unresolvable snippet repo: " repo)
{:type ::unresolvable-repo :marker marker})))
(let [text (try
(extract-region {:repo-path repo-path :file file :region region :sha sha})
(catch Exception e
(throw (ex-info (str "snippet source unreadable: " file " @ " sha)
{:type ::unreadable-source :marker marker
:git-error (or (:err (ex-data e)) (ex-message e))}))))]
(when-not text
(throw (ex-info (str "snippet region not found: " file "#" region)
{:type ::missing-region :marker marker})))
{:repo-path repo-path :text text})))
Verified against a real temp git repo with a real bad sha, the same test quoted above now passes for the right reason instead of never actually running.
A third finding from the same review wasn't a bug in the code at all. The whole point of building this feature was that the first real post hand-copied its code instead of using markers, and the review found that the operational playbook for running a publish session still didn't mention snippet markers anywhere. The resolver could be perfectly correct and the next real post would still hand-copy code, because nothing told the human running the next session that a better option existed.
The review that checked the fix found a fourth thing
One subagent fixed all three findings in a single pass. A scoped re-review, checking only that fix, verified every finding was actually addressed, and then found something new: a diagnostic field added as part of the error-handling fix, meant to carry the underlying git error text for debugging, was actually holding an unread process stream object instead of a string. Nothing in the codebase reads that field yet, so it's not a live bug, but the fix's own report claimed the field carried the error text, and it didn't. Parked as a known gap rather than blocking on it, since nothing depends on it being correct until something actually consumes it.
The shape of it
Four different checks, four different things caught, because each one was looking at a different question:
- A pre-dispatch scan of the plan itself, before any code existed, catching an artifact in the plan's own text.
- Per-task review, catching whether each task's own tests actually tested what they claimed to, including a case where the plan's own prescribed code would have shipped a vacuous test.
- A whole-branch review, catching what only shows up once independently-correct pieces are composed: a marker-finder that's too permissive meeting a resolver that's too literal, and an error path that was only ever tested with fixtures that happened not to trigger it.
- A re-review of the fix, catching a claim in the fix's own report that didn't match what the code actually did.
None of the four questions is a subset of another. A task review that tried to also ask "does this compose correctly with everything else" wouldn't be a task review anymore, it would be the whole-branch review, done four times over with three-quarters of the context missing each time. The value is in keeping the questions separate, not in finding one review thorough enough to ask all of them at once.