Skip to content

fix: create the review policy AGENTS.md already promised - #380

Merged
markcallen merged 1 commit into
mainfrom
fix/issue-340-code-review-doc
Sep 29, 2026
Merged

markcallen merged 1 commit into
mainfrom
fix/issue-340-code-review-doc

Conversation

@markcallen

Copy link
Copy Markdown
Contributor

Closes #340. Phases 2 and 4 of plans/plan-spec-kit-development-process.md.

The same question was open in two places

AGENTS.md has said "Follow docs/code_review.md for code reviews" for a file that does not exist. That is #340, and it is also the spec-kit plan's second open question. Answered once, here.

Created the doc rather than repointing

The reference sits directly above an orphaned <!-- END CODEX REVIEWER INSTALLER --> marker — that line is written by an external installer, not by Ballast and not by hand. Editing the pointer would be overwritten on the installer's next run; supplying the target is durable.

Worth noting it is repo-local: the section falls outside both Ballast-managed regions of AGENTS.md, so unlike #363 this never shipped to consuming repositories.

docs/code_review.md makes explicit the guidance already inline in AGENTS.md — what reviews look for in priority order, when each severity level blocks a merge, and the gates a change passes before review is requested. It carries the evidence standard from ADR-002, including that agent review findings are claims to verify, not verified findings.

Phase 4: the check that would have caught it

packages/ballast-typescript/src/docs-links.test.ts:

  • every relative markdown link resolves
  • every docs/*.md path named in backticks exists

The second check exists because a markdown-link check alone would have missed #340 — the reference was inline code, not a link.

Verified by deleting docs/code_review.md: both checks fire, naming AGENTS.md and docs/README.md.

Where the boundary got drawn

The check asserts current state, so it covers README.md, AGENTS.md, CLAUDE.md, docs/ and adr/, and excludes plans/ and tasks/.

That distinction was not designed up front — the check found two references while being written (docs/development-process.md, named by the spec-kit plan and then by my own todo). Both are correct: a plan or todo naming a file it intends to create is describing intent, not drift. Documents that assert what the repository has must be true; documents that describe what it will have must not be held to that.

Verification

385/385 tests, eslint and prettier clean, hooks enabled throughout.

Still open on the plan

Phase 1 is blocked on its placement question — does the end-to-end process doc live at docs/development-process.md, under docs/agents/spec-kit.md, or both? Phases 3 and 5 follow from it. Asking rather than guessing, since it determines the structure of the main deliverable.

🤖 Generated with Claude Code

Closes #340. Phases 2 and 4 of plan-spec-kit-development-process.

AGENTS.md has said "Follow `docs/code_review.md` for code reviews" for a
file that does not exist. The plan's open question and #340 were the same
question asked twice, so it is answered once here.

Created the doc rather than repointing the reference. That line sits
directly above an orphaned `END CODEX REVIEWER INSTALLER` marker and is
written by an external installer, not by Ballast and not by hand — so
supplying the target is durable where editing the pointer would be
overwritten on the installer's next run. It is repo-local, outside both
Ballast-managed sections, so unlike #363 it never shipped to consumers.

The policy itself is the review guidance already inline in AGENTS.md,
made explicit: what reviews look for in priority order, when each
severity level blocks a merge, and the gates a change passes before
review is requested. It carries the evidence standard from ADR-002,
including that agent review findings are claims to verify rather than
verified findings.

Phase 4 adds docs-links.test.ts, which would have caught this: every
relative markdown link must resolve, and every docs/*.md path named in
backticks must exist. A markdown-link check alone would have missed
#340, because the reference was inline code.

The check asserts current state, so it covers README.md, AGENTS.md,
CLAUDE.md, docs/ and adr/, and excludes plans/ and tasks/ — a plan or
todo naming a file it intends to create is describing intent, not drift.
It found two such references while being written, which is how that
boundary got drawn.

Verified by deleting docs/code_review.md: both checks fire, naming
AGENTS.md and docs/README.md.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@markcallen
markcallen merged commit 4b0ce6f into main Sep 29, 2026
47 checks passed
@markcallen
markcallen deleted the fix/issue-340-code-review-doc branch September 29, 2026 23:14
markcallen added a commit that referenced this pull request Sep 29, 2026
All five phases landed via #380 and #381, so the plan graduates.

ADR-004 records the decision rather than the document: each artifact
owns exactly one kind of truth, and docs/development-process.md
sequences the moves between them. The problem it solves is concrete --
one decision could plausibly live in spec.md, tasks.md, tasks/todo.md, a
plan, an issue and a PR comment, so it gets written in several, updated
in one, and the rest decay into confident contradictions.

It records the two invariants that hold it together (intent is written
before implementation, not reconstructed after it; work that will not
finish on this branch leaves the branch), and that the process is scaled
to the change, since a process with no skip rule gets skipped entirely.

Negative consequences are recorded, including the honest one: the
process is documented but not yet exercised here. This repository has
not bootstrapped Spec Kit, so steps 1-3 are untested. That is #303.

plans/ is now empty; its README says so explicitly rather than rendering
an empty table.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

AGENTS.md references docs/code_review.md, which does not exist

1 participant