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: Cursorpull/6/head
parent
c47892c782
commit
85d3070969
|
|
@ -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.
|
- [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.
|
- [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
|
## 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.
|
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).
|
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.
|
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)
|
## 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`.
|
1. **Verdict** — One line: `Approve` / `Approve with nits` / `Request changes`.
|
||||||
2. **Summary** — 2–5 sentences on what the change does and overall risk.
|
2. **Summary** — 2–5 sentences on what the change does and overall risk.
|
||||||
3. **Blocking issues** — Numbered list; empty section if none.
|
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. **Suggestions** — Non-blocking improvements.
|
4. **Blocking issues** — Numbered list; empty section if none.
|
||||||
5. **Nits** — Optional; prefix with “Nit:”.
|
5. **Suggestions** — Non-blocking improvements.
|
||||||
6. **Verification** — Commands or manual steps the author should run before merge (e.g. `dotnet test`, Godot scenario).
|
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.
|
In **chat only**, you may use Cursor line-number **code citations** when referencing existing code. For hypothetical fixes, use normal fenced code blocks everywhere.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -4,6 +4,6 @@ Optional **personas** for Cursor. Enable by **@ mentioning** the rule in chat or
|
||||||
|
|
||||||
| Agent | Rule file | When to use |
|
| 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`).
|
Project-wide conventions live under [`.cursor/rules/`](.cursor/rules/) (many are `alwaysApply`).
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue