Semantic Review & PR Description
Reviewing against the spec catches wrong-but-correct code — and a PR description that maps to acceptance criteria is self-reviewing.
Two Kinds of Review
There are two fundamentally different ways to review a pull request. The first opens the diff and evaluates whether the code is correct — logical, readable, tested, consistent with the codebase style. This is diff review. It is valuable and necessary.
The second opens the PRD alongside the diff and evaluates whether the implementation matches the specification — not just whether it is correct, but whether it is complete and bounded. This is semantic review. It catches wrong-but-correct code: code that is technically sound but implements the wrong thing.
The Difference in Practice
The gap between these two review modes becomes most visible when they diverge:
The "finds wrong implementation" row is the critical one. An agent can write syntactically correct, well-tested code that implements the wrong behavior. It can implement AC-1 and AC-2 while completely missing AC-3. Diff review may not catch this — the code that was written looks correct. Semantic review catches it because the reviewer is checking criteria, not just correctness.
The scope creep case is equally important. An agent adding an unrequested caching layer is doing the "right thing" by the model's evaluation of the problem. The agent is trying to be thorough. The semantic reviewer compares the addition against the non-goals section and finds it absent from scope. The question is not "is this good code?" but "did we ask for this?"
Writing a Spec-Driven PR Description
The PR description in a spec-driven workflow maps to acceptance criteria, not to a summary of what changed. The structure:
## Implements
- AC-1: Lesson announced within 5 min of merge ✅
- Verified: announce-lessons cron log timestamp < 5 min after merge
- AC-2: Message includes title, excerpt, and link ✅
- Verified: Discord #lessons screenshot attached
- AC-3: Retry on Discord API failure — deferred → ticket #43
## Non-Goals Honored
- No per-user DMs (explicitly not implemented)
- Telegram bridge untouched (no files in the messaging-bridge service modified)
- Enrollment tracking not added
## Out of Scope Tickets Created
- #43: Retry logic on Discord API failure
- #44: Per-lesson announcement customization
A reviewer with the PRD can verify completeness in two minutes. The "Implements" section tells them which criteria are met. The "Non-Goals Honored" section confirms scope didn't drift. The tickets at the bottom confirm that deferred work won't fall through the cracks.
The CodeRabbit Workflow
Knox's PR workflow uses CodeRabbit for automated code review. CodeRabbit fires automatically on PR creation and re-review on new pushes. The triage workflow:
The most important rule in the triage workflow is the P3 queue rule. A P3 finding is advisory and doesn't block merge. But from Knox's MEMORY.md: "Low-priority CR findings frequently become THE binding constraint after the top issue resolves." If you dismiss P3s without tracking them, you will encounter the same finding in a more inconvenient context next sprint.
The workflow: every P3 becomes a ticket. Not "I'll remember to fix this." A ticket in kanban-state.json with a reference to the PR that generated it. When the next related PR opens, the ticket is already in the queue.
When a Merge Is Blocked by Branch Protection
Repos may have branch protection that requires reviews, signatures, or passing checks before merge. When a gh pr merge attempt is rejected, the disciplined response is to diagnose the exact rule that fired and satisfy it through a sanctioned path — obtain the missing review, add the required signature, or unblock the failing check — rather than handing the PR back as "can't be done" at the first error.
The discipline rule: try the legitimate CLI paths before declaring a limit, and escalate to a human reviewer only once those paths are genuinely exhausted. The opposite failure is just as costly: never defeat the protection itself — no force-push over the branch, no disabling the rule to slip a change in. The guardrail exists because the team chose it; the goal is to work with it efficiently, not around it.
The Review as Spec Verification
The semantic review is the last formal check that the implementation matches the spec before it enters the compounding loop (the next lesson). Once merged, the retro evaluates how well the process went — but the review is the last chance to catch spec gaps before they become production behavior.
The question the reviewer asks: "If I know only what's in the PRD and I look at this diff, can I confirm that every acceptance criterion is implemented and no non-goal was violated?" If the answer is yes on all criteria and no on all non-goals, the PR is ready to merge.
What's Next
The next lesson closes the loop: the post-ship retro, how to extract lessons from each delivery cycle, the Semantic Memory Layer memory storage that makes lessons cross-session, and the rule that spec completion must queue its implementation task to prevent the coverage gap.