Skip to content

docs(code-review): retire the plan, keep the decisions it held - #83

Merged
pedromvgomes merged 2 commits into
mainfrom
chore/retire-the-code-review-plan
Sep 8, 2026
Merged

docs(code-review): retire the plan, keep the decisions it held#83
pedromvgomes merged 2 commits into
mainfrom
chore/retire-the-code-review-plan

Conversation

@pedromvgomes

Copy link
Copy Markdown
Owner

Two pieces of housekeeping now that approve has landed.

The plan is retired

docs/plans/code-review.md described work that is now built, all eight sections of it. A plan that describes shipped code is a second account of it — one nobody updates, and one a reader has to reconcile against the code to learn which is current. The ADRs carry what is hard to reverse and CONTEXT.md carries the vocabulary; the plan carried what was next, and there is no next.

Its rejected alternatives are not disposable

CLAUDE.md says ADRs are the one place where recording the rejected alternative is the decision's content. §6 held five, and four had no ADR to fall back on — deleting the file would have left them recoverable only from git history, which is not where anyone looks before re-proposing an idea.

ADR 0010 — Escalation is rules a person reads, not a ladder a model climbs takes them:

  • The rung ladder. Why deep-code-review's size-class-plus-signal-count arithmetic does not survive: it exists because a model invokes that skill from a session where nobody chose a depth, and here a person typed the command having just decided the change is ready. Its inputs survive as escalation conditions; the arithmetic does not.
  • Repo-declared content patterns. Regex zones, so a repo could define concurrency itself. A repo that writes sync\.Mutex gets nothing the day it adds another language.
  • Nested condition combinators. Rejected on Semgrep's own published evidence for why it shipped a structured editor.
  • Implicit AND across a rule's keys. The Renovate shape — two invisible combinators in adjacent lines.

The fifth, reviewers as a ninth definition category, already traced to ADR 0004 and is cross-referenced rather than restated.

The ADR is written forward, as a decision rather than as a summary of a deleted document, and its consequences section carries the one that matters next: a skill wrapping this engine does not size the review.

Two comments that pointed at the document

internal/reviewrun/prompt.go said "for the reason the plan gives" and internal/reviewrun/thread_test.go said "The table in the plan, whole". Both now state the reason instead of pointing at it — which is what CLAUDE.md asks of a comment anyway, and what made this removal load-bearing rather than mechanical.

Memory curation

Separate commit. Backlog to zero: four notes promoted, two re-verified with drifted line pointers corrected, one deleted and replaced because its name asserted a claim that had gone false, three candidates rejected. agtk memory lint: 28 ok, index current.

One promoted note is about the PR that preceded this one — credential-guards-are-hand-maintained-lists records that three of the guards in credential_surface_test.go are scoped to hand-typed package slices, so a new sibling package is not walked and nothing fails.

make check green.

Four notes promoted, two re-verified with their drifted pointers corrected,
one deleted and replaced because its name asserted a claim that had gone
false, three candidates rejected.

The curator runs in its own process against the candidates and the matching
slice of the index, never a session transcript, so this is its judgement
rather than the judgement of whichever session happened to be exhausted when
the backlog was cleared.
Everything the plan described is built, and a plan that describes shipped code
is a second account of it — one nobody updates, and one a reader has to
reconcile against the code to find out which is current. The ADRs and
CONTEXT.md carry what is hard to reverse; the plan carried what was next.

Its rejected alternatives are not disposable, though: recording the rejected
alternative is the decision's content, and four of them had no ADR. ADR 0010
takes them — the rung ladder, repo-declared content patterns, nested condition
combinators, and implicit AND across a rule's keys — with the reasoning and the
outside evidence intact. The fifth already traced to ADR 0004.

Two comments pointed at the document rather than stating what they meant, which
is what made the removal load-bearing rather than mechanical. Both now say the
thing itself.
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown

bulwark

  • scan — no findings

  • coverage — go: 81.1% (baseline 81.1%)

📦 Full bulwark output — complete scan and coverage logs.

@pedromvgomes
pedromvgomes merged commit a5b41b8 into main Sep 8, 2026
10 checks passed
@pedromvgomes
pedromvgomes deleted the chore/retire-the-code-review-plan branch September 8, 2026 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant