From 85d30709699ce76179957e182db1d518a7f205d7 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 30 Mar 2026 19:04:58 -0400 Subject: [PATCH] rules: code review must cite plans and decomposition docs Require identifying docs/plans and docs/decomposition/modules (register, module pages, cross-cutting policies, implementation alignment); verify implementation against them; add Documentation checked to review output. Extend checklist; AGENTS.md code review row summarizes requirement. Made-with: Cursor --- .cursor/rules/code-review-agent.md | 23 ++++++++++++++++++----- AGENTS.md | 2 +- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/.cursor/rules/code-review-agent.md b/.cursor/rules/code-review-agent.md index c42b2eb..8380f32 100644 --- a/.cursor/rules/code-review-agent.md +++ b/.cursor/rules/code-review-agent.md @@ -25,7 +25,18 @@ Align recommendations with repo rules and docs, including: - [jira-git-naming](jira-git-naming.md) — branch/commit prefixes when the work is ticketed. - [git-workflow](git-workflow.md) — branch vs `main`, story-scoped plans. -If the change contradicts an adopted plan under `docs/plans/`, call that out. +### Plan and decomposition documentation (required to reference) + +For every review, **identify and cite** the documentation that defines intent for the change, then **check the implementation against it** (not only style and correctness). + +1. **Story / implementation plans** — If the work maps to a ticket or plan under `docs/plans/` (e.g. `NS-*-implementation-plan.md`), treat that document as acceptance criteria. If the diff diverges without an updated plan or an explicit “out of scope” note, call it out (blocking or non-blocking by severity). +2. **Module docs** — Map the change to one or more modules in [`docs/decomposition/modules/module_dependency_register.md`](../../docs/decomposition/modules/module_dependency_register.md) and the corresponding `docs/decomposition/modules/E*_*.md` pages. Verify behavior matches **Purpose**, **Responsibilities**, **Key contracts**, and **Authority** / linked policy sections where relevant. +3. **Cross-cutting policies** — When applicable, align with `docs/decomposition/modules/` policy docs (e.g. [`contracts.md`](../../docs/decomposition/modules/contracts.md), [`client_server_authority.md`](../../docs/decomposition/modules/client_server_authority.md), [`pvp_combat_integration.md`](../../docs/decomposition/modules/pvp_combat_integration.md), [`data_and_ops_policy.md`](../../docs/decomposition/modules/data_and_ops_policy.md)). +4. **Implementation status** — If the register or [`documentation_and_implementation_alignment.md`](../../docs/decomposition/modules/documentation_and_implementation_alignment.md) tracks the module, note whether **Status** / the implementation tracking table should be updated after this merge. + +In the **saved review document**, include a subsection **Documentation checked**: bullet list of paths consulted and whether the diff **matches**, **partially matches**, or **conflicts** with them. If no decomposition doc applies (e.g. pure tooling), state that explicitly. + +If the change contradicts an adopted plan under `docs/plans/` or documented module/policy intent without justification, treat that as at least a **should fix** (or **blocking** if it breaks contracts or authority). ## Review checklist @@ -38,6 +49,7 @@ Work through what applies to the diff (skip irrelevant sections briefly). 5. **Tests** — Happy path, failure modes, integration boundaries per [testing-expectations](testing-expectations.md); note gaps. 6. **Observability** — Logging/metrics where failures would be hard to diagnose (server/runtime code). 7. **Docs** — README or plan updates when behavior or run instructions change. +8. **Plan & decomposition alignment** — Per **Plan and decomposition documentation** above: relevant `docs/plans/` and module/policy docs cited; implementation checked against them; **Documentation checked** section in the saved review file. ## Code review document (required) @@ -60,10 +72,11 @@ Use this structure in **both** the saved document and (abbreviated if you want) 1. **Verdict** — One line: `Approve` / `Approve with nits` / `Request changes`. 2. **Summary** — 2–5 sentences on what the change does and overall risk. -3. **Blocking issues** — Numbered list; empty section if none. -4. **Suggestions** — Non-blocking improvements. -5. **Nits** — Optional; prefix with “Nit:”. -6. **Verification** — Commands or manual steps the author should run before merge (e.g. `dotnet test`, Godot scenario). +3. **Documentation checked** — Paths under `docs/plans/` and `docs/decomposition/modules/` (and related) consulted; **matches** / **partially matches** / **conflicts** / **N/A** per item; note if register/tracking table updates are needed. +4. **Blocking issues** — Numbered list; empty section if none. +5. **Suggestions** — Non-blocking improvements. +6. **Nits** — Optional; prefix with “Nit:”. +7. **Verification** — Commands or manual steps the author should run before merge (e.g. `dotnet test`, Godot scenario). In **chat only**, you may use Cursor line-number **code citations** when referencing existing code. For hypothetical fixes, use normal fenced code blocks everywhere. diff --git a/AGENTS.md b/AGENTS.md index 1838faf..9c5804d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -4,6 +4,6 @@ Optional **personas** for Cursor. Enable by **@ mentioning** the rule in chat or | Agent | Rule file | When to use | |--------|-----------|-------------| -| **Code review** | [`.cursor/rules/code-review-agent.md`](.cursor/rules/code-review-agent.md) | PR / diff / pre-merge review; **always writes** `docs/reviews/YYYY-MM-DD-{slug}.md` plus a short chat pointer | +| **Code review** | [`.cursor/rules/code-review-agent.md`](.cursor/rules/code-review-agent.md) | PR / diff / pre-merge review; **always writes** `docs/reviews/YYYY-MM-DD-{slug}.md` with **Documentation checked** vs `docs/plans/` and `docs/decomposition/modules/`; short chat pointer | Project-wide conventions live under [`.cursor/rules/`](.cursor/rules/) (many are `alwaysApply`).