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 bugs that none of the three approvals had caught. The review that checked the fix turned up 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 draft out of a model call, and publishes nothing until a human has read and substantially revised it. Building the pipeline itself follows a fixed shape. A plan breaks a feature into a handful of independently testable tasks, and each task goes to a fresh implementer with no memory of the tasks before it, carrying only the plan and the one piece it owns. Every task gets its own reviewer before the next one starts. A narrow, well-defined job should catch what accumulated context and creeping scope would let slide. That is the bet, anyway. Once every task is done and approved, one more review reads the finished result as a single whole change.
net.b12n.publish.snippet went through exactly that process while being wired into the draft-writing workflow. The bugs it turned up are concrete enough to be worth writing down, and the arc doubles as a test of the extra layer. Does reviewing the whole thing, after every piece has already passed on its own, find anything the pieces missed? Twice over, here.
What the feature does, and why it exists
A marker in a draft is an HTML comment naming a repo, a file, a named region in that file, and a pinned commit. It resolves to the code at that commit, so a post never describes code that has since shifted underneath it. The whole feature reacts to a rule this project's design doc states plainly: never let a model retype code into a draft. The first post this pipeline published broke that rule anyway. Every code block in it was hand-copied during revision, because the resolver existed and nothing called it. This post doesn't repeat that. Every code block below is quoted through the 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-textswaps each marker for the pinned code it points to.drifted-markers-in-textflags markers whose pinned region has changed at HEAD. Drift is a signal to review before publishing rather than a failure to stop on.
They were built as three separate tasks from a written plan, each with its own tests and its own reviewer.
What the per-task reviews caught
Before any task ran, a pass over the plan itself caught an editing artifact. Someone had fixed an arithmetic mistake in one task's instructions and left a "wait, let me recount" aside sitting in the text. Small, and exactly what a pre-dispatch scan is for.
Every task review approved cleanly. Two of the three still flagged something worth noting on the way past.
Task 1's reviewer pointed out that the tests only covered a marker sitting alone on its own line, never one embedded mid-line, and suggested folding that case into a later task if it wasn't picked up naturally there. It wasn't. That gap matters a lot, two sections down.
Task 3's reviewer caught something sharper. The plan's own prescribed code called keep, which hands back a lazy sequence. Two of that task's tests assert that a broken marker throws:
(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, so wrapping (is (thrown-with-msg? ...)) around a bare keep call just returns an unrealized lazy-seq and runs nothing. As prescribed, that test would have passed without ever touching the throw path it claims to cover. The implementer spotted it independently, forced the sequence with into [], and the reviewer confirmed the deviation was a fix rather than something to revert. A plan can be wrong in a way that only surfaces once someone runs its tests.
What only the final review caught
A whole-branch review ran on a more capable model after all three tasks were green. It found two things no task-scoped review was positioned to see, because neither one lives inside a single task.
The first is the case Task 1's reviewer flagged and deferred. resolve-markers-in-text finds a marker anywhere in a line, because parse-marker matches with re-find rather than re-matches. Then it replaces the entire line. Put a marker inside a sentence instead of alone on its own line, and the surrounding prose silently disappears. No error, no warning. Gone. Here is the fixed version, which makes that loud:
(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, which is 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, because 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 finding is about how the thing breaks in the field. The original error handling checked whether the resolved region came back nil, which covers a file that exists without the requested #region and #endregion markers in it. The likeliest mistake is a typo'd commit sha or a typo'd file path, though, and that doesn't produce nil at all. It makes the underlying git show exit non-zero, so the throw happens before the nil-check runs, carrying an empty message and an internal library error type that has nothing to do with this project's own error contract. The documented promise of a clear error didn't hold for the most common way a human gets a marker wrong. The fix wraps the call and rethrows with a message that says something:
(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, where 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})))
Checked against a temp git repo and a deliberately bad sha, the test quoted earlier now passes for the right reason instead of never running at all.
A third finding from the same review wasn't about the code. This feature exists because the first post hand-copied its code instead of using markers, and the review noticed that the operational playbook for running a publish session still said nothing about snippet markers anywhere. The resolver could be flawless and the next post would still hand-copy its code, because nothing told the human running that session 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 then confirmed each finding was addressed, and turned up something new on the way. A diagnostic field added during the error-handling fix, meant to carry the underlying git error text for debugging, was holding an unread process stream object instead of a string. Nothing in the codebase reads that field yet, so nothing is broken today. The fix's own report claimed the field carried the error text, though, and it didn't. Parked as a known gap rather than blocked on, since nothing depends on it until something consumes it.
Four checks, four different questions
Each check caught something different, because each was asking a different question.
- A pre-dispatch scan of the plan, before any code existed, catching an artifact in the plan's own text.
- Per-task review, asking whether each task's tests exercised what they claimed to. That one caught prescribed code that 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 was too permissive met a resolver that was too literal, and an error path had only ever been 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 the code didn't support.
None of those questions is a subset of another. A task review that also asked "does this compose correctly with everything else" would stop being a task review and turn into the whole-branch review, run four times over with three-quarters of the context missing each time. So the pipeline keeps all four passes, and the snippet module went out with four findings against it instead of the clean bill its three task reviews had already signed off on.