diff --git a/.cursor/rules/code-review-agent.md b/.cursor/rules/code-review-agent.md index b920bac..ed79992 100644 --- a/.cursor/rules/code-review-agent.md +++ b/.cursor/rules/code-review-agent.md @@ -84,10 +84,10 @@ In **chat only**, you may use Cursor line-number **code citations** when referen If you **implement** changes that resolve **blocking issues**, **suggestions**, or agreed **nits** from an existing **`docs/reviews/…`** file for the same work: -- **Edit that review document** in the same commit or the immediate follow-up commit: add or update **`## Resolved suggestions`** / **`## Follow-up`** per [planning-implementation-docs](planning-implementation-docs.md) **Code review follow-up**. -- Do **not** ship code-only fixes while the saved review still reads as if those items were open. +- **Edit that review document in place:** strike through the original bullets under **`## Suggestions`** / **`## Nits`** / **`## Blocking issues`** (`~~…~~`), append **`Done.`** / **`Addressed.`** / **`Deferred…`** plus a short note and optional commit SHA; optionally a one-line **Follow-up** preamble after **Scope** when appropriate. Full pattern: [planning-implementation-docs](planning-implementation-docs.md) **Code review follow-up**. +- Do **not** add a **separate** section duplicating resolved items, and do **not** ship code-only fixes while those bullets still read as open. -The **initial** review does not need an empty “Resolved” section; this rule applies when **closing the loop** after the review exists. +The **initial** review does not need strikethroughs; this rule applies when **closing the loop** after the review exists. ## Boundaries diff --git a/.cursor/rules/commit-and-review.md b/.cursor/rules/commit-and-review.md index 63199f0..03eef13 100644 --- a/.cursor/rules/commit-and-review.md +++ b/.cursor/rules/commit-and-review.md @@ -32,4 +32,4 @@ alwaysApply: true ## Code review follow-up -When commits **address** feedback from a saved **`docs/reviews/…`** file, include an update to **that review file** documenting what was resolved (or explicitly deferred/declined). See [planning-implementation-docs](planning-implementation-docs.md) **Code review follow-up** and [code-review-agent](code-review-agent.md) **Resolved suggestions**. +When commits **address** feedback from a saved **`docs/reviews/…`** file, include an update to **that review file**: **strikethrough + Done** on the original **Suggestions** / **Nits** / **Blocking** bullets (not a separate resolved section). See [planning-implementation-docs](planning-implementation-docs.md) **Code review follow-up** and [code-review-agent](code-review-agent.md) **Resolved suggestions**. diff --git a/.cursor/rules/docs-review-agent.md b/.cursor/rules/docs-review-agent.md index f315c2f..2d0ef56 100644 --- a/.cursor/rules/docs-review-agent.md +++ b/.cursor/rules/docs-review-agent.md @@ -68,7 +68,7 @@ In the saved file, use **normal fenced code blocks** and **backtick paths** — ## Resolved suggestions (mandatory when feedback is addressed) -If later edits **address** **blocking issues**, **suggestions**, or **nits** from a **docs-review** `docs/reviews/…` file, **update that review file** with **`## Resolved suggestions`** / **`## Follow-up`** per [planning-implementation-docs](planning-implementation-docs.md) **Code review follow-up** (same audit trail as code reviews). +If later edits **address** **blocking issues**, **suggestions**, or **nits** from a **docs-review** `docs/reviews/…` file, **update that review in place** with strikethrough + **`Done.`** on the original bullets per [planning-implementation-docs](planning-implementation-docs.md) **Code review follow-up** (same pattern as code reviews; no separate “resolved” section). ## Boundaries diff --git a/.cursor/rules/planning-implementation-docs.md b/.cursor/rules/planning-implementation-docs.md index a9466e0..fd32944 100644 --- a/.cursor/rules/planning-implementation-docs.md +++ b/.cursor/rules/planning-implementation-docs.md @@ -13,15 +13,16 @@ Whenever you **decide** something during **story planning** or **implementation* |-----------|--------| | Ticketed story work | **`docs/plans/{JIRA_KEY}-implementation-plan.md`** — add or edit a **Decisions** subsection, **resolve** former open questions, refresh **Technical approach** / **Acceptance criteria** checkboxes, and **Files to add/modify** if reality diverged from kickoff. | | Cross-cutting or module contract | Relevant **`docs/decomposition/modules/*.md`**, **`docs/game-design/`**, or **`README`** / **`server/README.md`** / **`client/README.md`** when the decision affects how others integrate. | -| Review findings | **`docs/reviews/…`** when using the code-review agent; link back to the plan if the review changed direction. | +| Review findings | **`docs/reviews/…`** when using the code-review agent; link back to the plan if the review changed direction. When feedback is fixed, **strike through + `Done.`** on the original bullets in that review file — see **Code review follow-up** below. | ## Code review follow-up (resolved suggestions) -When **blocking issues**, **suggestions**, or agreed **nits** from a saved **`docs/reviews/YYYY-MM-DD-*.md`** are **fixed** in code or other docs, **update that review file** in the **same** change-set or the **next** commit on the story branch—do **not** only fix the code and leave the review implying feedback is still open. +When **blocking issues**, **suggestions**, or agreed **nits** from a saved **`docs/reviews/YYYY-MM-DD-*.md`** are **fixed** in code or other docs, **edit that same review file** in the **same** change-set or the **next** commit—do **not** only fix the code and leave the review implying feedback is still open. -- Add or extend a section such as **`## Resolved suggestions`** or **`## Follow-up`**: for **each** item from the review that you addressed, record **what** changed and **where** (paths or commits). Match the review’s numbering or headings so readers can correlate. -- Items **declined**, **out of scope**, or **deferred** to a ticket: state that in the same section so the audit trail is honest. -- Applies to **any** agent pass that implements review feedback, not only a second invocation of the code-review agent—see [code-review-agent](code-review-agent.md) **Resolved suggestions**. +- **In place under the original headings** (`## Suggestions`, `## Nits`, `## Blocking issues`): strike through the resolved bullet with `~~…~~`, then add **`Done.`** / **`Addressed.`** / **`Deferred…`** and a one-line note (what changed, paths, optional commit SHA). Optionally add a one-line **Follow-up** note under the title (after **Scope**/**Base**) when all suggestions are done, e.g. “suggestions below are **done** (strikethrough + **Done.**)”. Same pattern as `docs/reviews/2026-03-30-NEON-5.md`, `2026-04-05-NEON-16.md`. +- Do **not** add a **separate** “Resolved suggestions” / “Follow-up implemented” section for items that already live under Suggestions or Nits—the strikethrough row **is** the audit trail. +- Items **declined** or **deferred** to a ticket: still strike through or annotate inline under the same bullet so the review stays honest. +- Applies to **any** agent pass that implements review feedback—see [code-review-agent](code-review-agent.md) **Resolved suggestions**. ## What to capture @@ -33,6 +34,6 @@ When **blocking issues**, **suggestions**, or agreed **nits** from a saved **`do - After **kickoff**, the plan is living: **amend it** when implementation choices differ from the draft. - Before **closing** or **handing off** a story, skim the plan: open questions should be **resolved** or restated as follow-up tickets with pointers. -- After **implementing** review feedback, **edit the corresponding `docs/reviews/…` file** per **Code review follow-up** above—this step is easy to skip; treat it as **required** whenever suggestions were listed in that review. +- After **implementing** review feedback, **edit the corresponding `docs/reviews/…` file** with **strikethrough + Done** on the original bullets per **Code review follow-up** above—this step is easy to skip; treat it as **required** whenever suggestions were listed in that review. This complements [story-kickoff](story-kickoff.md) (plan creation) and [code-review-agent](code-review-agent.md) (documentation checked vs plans and decomposition). diff --git a/AGENTS.md b/AGENTS.md index 8e360a9..483e291 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -4,8 +4,8 @@ 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` with **Documentation checked** vs `docs/plans/` and `docs/decomposition/modules/`; **when suggestions are fixed, update that review file** ([planning-implementation-docs](.cursor/rules/planning-implementation-docs.md)); short chat pointer | -| **Docs review** | [`.cursor/rules/docs-review-agent.md`](.cursor/rules/docs-review-agent.md) | Coherence / links / dev-guide fitness for `docs/` (especially `docs/game-design/` + decomposition); **writes** `docs/reviews/YYYY-MM-DD-{slug}.md`; **when suggestions are fixed, update that review file** ([planning-implementation-docs](.cursor/rules/planning-implementation-docs.md)); use when working in **documents** or @ mention | +| **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/`; **when suggestions are fixed, strike through those bullets + `Done.` in that file** ([planning-implementation-docs](.cursor/rules/planning-implementation-docs.md)); short chat pointer | +| **Docs review** | [`.cursor/rules/docs-review-agent.md`](.cursor/rules/docs-review-agent.md) | Coherence / links / dev-guide fitness for `docs/` (especially `docs/game-design/` + decomposition); **writes** `docs/reviews/YYYY-MM-DD-{slug}.md`; **when suggestions are fixed, strike through + `Done.` in that file** ([planning-implementation-docs](.cursor/rules/planning-implementation-docs.md)); use when working in **documents** or @ mention | Project-wide conventions live under [`.cursor/rules/`](.cursor/rules/) (many are `alwaysApply`). **Planning / implementation decisions** must be written into `docs/plans/` (and related docs)—[`.cursor/rules/planning-implementation-docs.md`](.cursor/rules/planning-implementation-docs.md). **Godot client layout** (thin `main.gd`, split by concern): [`.cursor/rules/godot-client-script-organization.md`](.cursor/rules/godot-client-script-organization.md). **Git:** agents may **commit** at discretion on story/ticket work; **never** `git push` unless the user asks — [`.cursor/rules/commit-and-review.md`](.cursor/rules/commit-and-review.md). **Story lifecycle:** kickoff [`.cursor/rules/story-kickoff.md`](.cursor/rules/story-kickoff.md); after merge / end of story — `checkout main`, `pull`, delete local story branch — [`.cursor/rules/story-end.md`](.cursor/rules/story-end.md). **PR / push text:** no “Made-with: Cursor” boilerplate (same file). diff --git a/docs/reviews/2026-04-08-NEON-15.md b/docs/reviews/2026-04-08-NEON-15.md index 7c21b47..f996036 100644 --- a/docs/reviews/2026-04-08-NEON-15.md +++ b/docs/reviews/2026-04-08-NEON-15.md @@ -4,6 +4,8 @@ **Scope:** Branch `NEON-15-dispose-npgsql-datasource` vs `main` — [NEON-15](https://neon-sprawl.atlassian.net/browse/NEON-15) (dispose `NpgsqlDataSource` on shutdown), plus planning-doc rule cross-links. **Base:** `main` (range `main...HEAD`, commits `abffd29` → `62ff210`). +**Follow-up:** Review **suggestions** below are **done** (strikethrough + **Done.**); **nits** remain optional. + ## Verdict **Approve with nits** @@ -19,22 +21,22 @@ Residual risk is low and already acknowledged in the plan: the root `ServiceProv | Document | Result | |----------|--------| | `docs/plans/NEON-15-implementation-plan.md` | **Matches** — Decisions (Option 2, ConfigureAwait), AC checked, technical approach and file lists align with the diff; open questions resolved per [planning-implementation-docs](../../.cursor/rules/planning-implementation-docs.md). | -| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | **N/A** for merge blockers — Postgres path is already documented at prototype level; this change is **lifecycle / ops hygiene** under the same `Game/PositionState` area, not a new contract. | -| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Partially matches** — E1.M1 snapshot lists many NEON plans; **NEON-15** is not linked yet. Optional follow-up to add a short pointer (suggestion below). | +| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | **Matches (post-review update)** — Snapshot links NEON-15 for shared `NpgsqlDataSource` shutdown — `8c3e8c0`. | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches (post-review update)** — E1.M1 row snapshot + plans column include NEON-15 — `8c3e8c0`. | | `docs/decomposition/modules/module_dependency_register.md` | **N/A** — no status or dependency change required for a shutdown-only fix. | | `docs/decomposition/modules/client_server_authority.md`, `contracts.md` | **N/A** — no authority or wire-shape change. | | `.cursor/rules/planning-implementation-docs.md`, `story-kickoff.md`, `code-review-agent.md`, `docs-review-agent.md`, `AGENTS.md` | **Matches** — rule text is clear; relative links in agent rules remain valid. | -**Register / tracking table:** No update **required** for merge; optional E1.M1 snapshot pointer to NEON-15 for traceability. - ## Blocking issues _None._ ## Suggestions -1. **Traceability (optional):** Add `NEON-15` / `docs/plans/NEON-15-implementation-plan.md` to the E1.M1 **Plans / pointers** column in `documentation_and_implementation_alignment.md` (or a one-line note under Postgres in `E1_M1_InputAndMovementRuntime.md`) so future readers find shutdown lifecycle work next to NEON-5. -2. **PR narrative:** Call out the **two themes** in the branch (server disposal + planning-doc rule) so reviewers do not miss the `.cursor/` and `AGENTS.md` changes if they only skim `server/`. +**All done.** + +1. ~~**Traceability (optional):** Add `NEON-15` / `docs/plans/NEON-15-implementation-plan.md` to the E1.M1 **Plans / pointers** column in `documentation_and_implementation_alignment.md` (or a one-line note under Postgres in `E1_M1_InputAndMovementRuntime.md`) so future readers find shutdown lifecycle work next to NEON-5.~~ **Done.** E1.M1 snapshot + plans list + module implementation snapshot updated — `8c3e8c0`. +2. ~~**PR narrative:** Call out the **two themes** in the branch (server disposal + planning-doc rule) so reviewers do not miss the `.cursor/` and `AGENTS.md` changes if they only skim `server/`.~~ **Done.** `docs/plans/NEON-15-implementation-plan.md` **PR / review** section lists server vs repo-process themes — `8c3e8c0`. ## Nits @@ -45,8 +47,3 @@ _None._ - `dotnet test NeonSprawl.sln` — **Passed** (36 tests) at review time. - Optional: run `NeonSprawl.Server` with `ConnectionStrings:NeonSprawl` set, stop the process, confirm clean shutdown logs. - -## Follow-up (suggestions implemented) - -- **Traceability:** `documentation_and_implementation_alignment.md` (E1.M1 row) and `E1_M1_InputAndMovementRuntime.md` now link [NEON-15](../plans/NEON-15-implementation-plan.md) for Postgres pool shutdown. -- **PR narrative:** `docs/plans/NEON-15-implementation-plan.md` **PR / review** section now lists the two merge-description themes (server vs repo process).