Audit and clean up review notes: type consistency, naming, and lifecycle #246

New issue

Open

opened 2026-03-28 22:57:40 +00:00 by forgejo_admin · 1 comment

forgejo_admin commented

2026-03-28 22:57:40 +00:00

Owner

Type

Bug

Lineage

Discovered during westside schedule feature scoping — review agents created notes with wrong slugs (Forgejo issue number instead of board item ID), wrong note_type (doc instead of review), blocking the check-board-advance hook.

Repo

forgejo_admin/pal-e-api (notes + skill-review-ticket)

What Broke

Review notes are inconsistent and accumulating without cleanup:

Slug naming: skill-review-ticket step 12 says review-{board_item_id}-YYYY-MM-DD but agents frequently use Forgejo issue numbers instead. The check-board-advance hook queries for review-{board_item_id}-*, so misnamed notes fail the gate silently.
note_type: Skill says review but most notes were created as doc or null. 50+ notes tagged review exist with mixed types.
No lifecycle: Review notes accumulate forever. No convention for cleanup after tickets reach done.
Active types list: review is not in feedback_active_note_types (sop, convention, doc, project-page, board). Either add it or decide reviews shouldn't be notes.

Repro Steps

Run /review-ticket board-westside-basketball#629
Review agent creates review-232-2026-03-28 (using Forgejo issue #232 instead of board item #629)
Attempt to advance item #629 from todo to next_up
check-board-advance hook blocks because it looks for review-629-*

Expected Behavior

Review notes use review-{board_item_id}-* slugs consistently
Hook finds them on first attempt
Old review notes get cleaned up when tickets reach done

Environment

pal-e-docs API: production
Hook: ~/.claude/hooks/check-board-advance.sh
Skill: skill-review-ticket note in pal-e-docs

Acceptance Criteria

All existing review notes audited: fix note_type to review where tagged review
Orphaned/duplicate review notes identified (notes where the board item is in done or no longer exists)
Convention note created: convention-review-note-lifecycle defining creation, naming, type, and cleanup rules
review added to active note types list (or decision documented to remove review notes entirely)
skill-review-ticket step 12 verified — if the naming convention is clear, note WHY agents drift (possible: agents see existing misnamed notes and copy the pattern)

skill-review-ticket — the skill that guides review agents
check-board-advance.sh — hook in claude-custom that enforces the gate (code change is a separate ticket if needed)
feedback_active_note_types — memory noting active types
convention-todo-lifecycle — related lifecycle convention

### Type Bug ### Lineage Discovered during westside schedule feature scoping — review agents created notes with wrong slugs (Forgejo issue number instead of board item ID), wrong note_type (`doc` instead of `review`), blocking the `check-board-advance` hook. ### Repo `forgejo_admin/pal-e-api` (notes + skill-review-ticket) ### What Broke Review notes are inconsistent and accumulating without cleanup: 1. **Slug naming:** `skill-review-ticket` step 12 says `review-{board_item_id}-YYYY-MM-DD` but agents frequently use Forgejo issue numbers instead. The `check-board-advance` hook queries for `review-{board_item_id}-*`, so misnamed notes fail the gate silently. 2. **note_type:** Skill says `review` but most notes were created as `doc` or `null`. 50+ notes tagged `review` exist with mixed types. 3. **No lifecycle:** Review notes accumulate forever. No convention for cleanup after tickets reach `done`. 4. **Active types list:** `review` is not in `feedback_active_note_types` (sop, convention, doc, project-page, board). Either add it or decide reviews shouldn't be notes. ### Repro Steps 1. Run `/review-ticket board-westside-basketball#629` 2. Review agent creates `review-232-2026-03-28` (using Forgejo issue #232 instead of board item #629) 3. Attempt to advance item #629 from todo to next_up 4. `check-board-advance` hook blocks because it looks for `review-629-*` ### Expected Behavior - Review notes use `review-{board_item_id}-*` slugs consistently - Hook finds them on first attempt - Old review notes get cleaned up when tickets reach `done` ### Environment - pal-e-docs API: production - Hook: `~/.claude/hooks/check-board-advance.sh` - Skill: `skill-review-ticket` note in pal-e-docs ### Acceptance Criteria - [ ] All existing review notes audited: fix note_type to `review` where tagged `review` - [ ] Orphaned/duplicate review notes identified (notes where the board item is in `done` or no longer exists) - [ ] Convention note created: `convention-review-note-lifecycle` defining creation, naming, type, and cleanup rules - [ ] `review` added to active note types list (or decision documented to remove review notes entirely) - [ ] `skill-review-ticket` step 12 verified — if the naming convention is clear, note WHY agents drift (possible: agents see existing misnamed notes and copy the pattern) ### Related - `skill-review-ticket` — the skill that guides review agents - `check-board-advance.sh` — hook in claude-custom that enforces the gate (code change is a separate ticket if needed) - `feedback_active_note_types` — memory noting active types - `convention-todo-lifecycle` — related lifecycle convention

forgejo_admin commented

2026-03-28 23:02:01 +00:00

Author

Owner

Review Note Audit -- Dottie Report

Summary

Metric	Count
Total review notes found	21
Correct note_type (`review`)	18
Wrong note_type (`doc` instead of `review`)	3 (fixed)
Duplicate reviews (same board item, multiple reviews)	5 board items with review chains
Orphaned (board item in `done` or superseded)	Needs board-level check per item (see below)

Review Notes Inventory

Slug	Board Item	Verdict	note_type (before)	note_type (after)	Project
`review-241-2026-03-28`	#241	BLOCK	review	--	pal-e-docs
`review-317-2026-03-24`	#317	NEEDS_REFINEMENT	review	--	pal-e-docs
`review-356-2026-03-25`	#356	NEEDS_REFINEMENT	review	--	westside-basketball
`review-356-2026-03-25-r2`	#356	READY	review	--	westside-basketball
`review-364-2026-03-27`	#364	NEEDS_REFINEMENT	review	--	pal-e-agency
`review-364-2026-03-27-v2`	#364	READY	review	--	pal-e-agency
`review-376-2026-03-25`	#376	NEEDS_REFINEMENT	review	--	svelte-playground
`review-376-2026-03-25-r2`	#376	READY	review	--	svelte-playground
`review-398-2026-03-26`	#398	READY	review	--	pal-e-agency
`review-416-2026-03-25`	#416	NEEDS_REFINEMENT	review	--	westside-basketball
`review-416-2026-03-26`	#416	READY	review	--	westside-basketball
`review-418-2026-03-25`	#418	NEEDS_REFINEMENT	review	--	pal-e-agency
`review-418-2026-03-25-r2`	#418	NEEDS_REFINEMENT	review	--	pal-e-agency
`review-418-2026-03-27`	#418	NEEDS_REFINEMENT	review	--	pal-e-agency
`review-424-2026-03-26`	#424	READY	review	--	pal-e-docs
`review-464-2026-03-27`	#464	NEEDS_REFINEMENT	review	--	pal-e-platform
`review-464-2026-03-28`	#464	READY	review	--	pal-e-platform
`review-479-2026-03-27`	#479	NEEDS_REFINEMENT	review	--	pal-e-agency
`review-181-2026-03-27`	#181	NEEDS_REFINEMENT	review	--	pal-e-agency
`review-595-2026-03-28`	#595	NEEDS_REFINEMENT	doc	review (fixed)	westside-ai-assistant
`review-595-2026-03-28-r2`	#595	READY	review	--	westside-ai-assistant
`review-597-2026-03-28`	#597	NEEDS_REFINEMENT	doc	review (fixed)	westside-ai-assistant
`review-597-2026-03-28-r2`	#597	READY	doc	review (fixed)	westside-ai-assistant

Fixes Applied

Fixed note_type on 3 notes: review-595-2026-03-28, review-597-2026-03-28, review-597-2026-03-28-r2 changed from doc to review.
Root cause: template-review naming convention section said note_type: doc (until review is added to NoteType enum) -- but review IS a valid NoteType now. Stale guidance caused the westside-ai-assistant session to use doc.

Duplicate Reviews (Review Chains)

These are expected -- they are the review-fix-re-review loop in action:

#356: 2 reviews (NEEDS_REFINEMENT -> READY)
#364: 2 reviews (NEEDS_REFINEMENT -> READY)
#376: 2 reviews (NEEDS_REFINEMENT -> READY)
#416: 2 reviews (NEEDS_REFINEMENT -> READY)
#418: 3 reviews (all NEEDS_REFINEMENT -- no final READY yet)
#464: 2 reviews (NEEDS_REFINEMENT -> READY)
#595: 2 reviews (NEEDS_REFINEMENT -> READY)
#597: 2 reviews (NEEDS_REFINEMENT -> READY)

Naming Convention Conflict Found

Three sources gave conflicting slug guidance:

skill-review-ticket step 12: review-{board_item_id}-{YYYY-MM-DD} (authoritative)
template-review: review-{issueNumber}-{YYYY-MM-DD} (stale)
note-conventions: review-{repo}-{pr-number} (describes PR reviews, different artifact)

Resolution: skill-review-ticket is authoritative. All existing review notes already follow the board-item-ID pattern. template-review naming convention section needs updating to match.

Convention Note Created

convention-review-note-lifecycle -- covers naming, note_type, tags, status, cleanup rules, and documents the source conflict resolution.

Remaining Work (for Betty Sue)

Update template-review naming convention section to say review-{board_item_id} and note_type: review (currently says {issueNumber} and doc).
Board item #418 has 3 NEEDS_REFINEMENT reviews with no READY -- the ticket refinement loop may be stuck.
Orphan cleanup requires checking each board item's current column. Review notes for items in done older than 7 days are cleanup candidates per the new convention.

## Review Note Audit -- Dottie Report ### Summary | Metric | Count | |--------|-------| | Total review notes found | 21 | | Correct note_type (`review`) | 18 | | Wrong note_type (`doc` instead of `review`) | 3 (fixed) | | Duplicate reviews (same board item, multiple reviews) | 5 board items with review chains | | Orphaned (board item in `done` or superseded) | Needs board-level check per item (see below) | ### Review Notes Inventory | Slug | Board Item | Verdict | note_type (before) | note_type (after) | Project | |------|-----------|---------|--------------------|--------------------|---------| | `review-241-2026-03-28` | #241 | BLOCK | review | -- | pal-e-docs | | `review-317-2026-03-24` | #317 | NEEDS_REFINEMENT | review | -- | pal-e-docs | | `review-356-2026-03-25` | #356 | NEEDS_REFINEMENT | review | -- | westside-basketball | | `review-356-2026-03-25-r2` | #356 | READY | review | -- | westside-basketball | | `review-364-2026-03-27` | #364 | NEEDS_REFINEMENT | review | -- | pal-e-agency | | `review-364-2026-03-27-v2` | #364 | READY | review | -- | pal-e-agency | | `review-376-2026-03-25` | #376 | NEEDS_REFINEMENT | review | -- | svelte-playground | | `review-376-2026-03-25-r2` | #376 | READY | review | -- | svelte-playground | | `review-398-2026-03-26` | #398 | READY | review | -- | pal-e-agency | | `review-416-2026-03-25` | #416 | NEEDS_REFINEMENT | review | -- | westside-basketball | | `review-416-2026-03-26` | #416 | READY | review | -- | westside-basketball | | `review-418-2026-03-25` | #418 | NEEDS_REFINEMENT | review | -- | pal-e-agency | | `review-418-2026-03-25-r2` | #418 | NEEDS_REFINEMENT | review | -- | pal-e-agency | | `review-418-2026-03-27` | #418 | NEEDS_REFINEMENT | review | -- | pal-e-agency | | `review-424-2026-03-26` | #424 | READY | review | -- | pal-e-docs | | `review-464-2026-03-27` | #464 | NEEDS_REFINEMENT | review | -- | pal-e-platform | | `review-464-2026-03-28` | #464 | READY | review | -- | pal-e-platform | | `review-479-2026-03-27` | #479 | NEEDS_REFINEMENT | review | -- | pal-e-agency | | `review-181-2026-03-27` | #181 | NEEDS_REFINEMENT | review | -- | pal-e-agency | | `review-595-2026-03-28` | #595 | NEEDS_REFINEMENT | **doc** | **review** (fixed) | westside-ai-assistant | | `review-595-2026-03-28-r2` | #595 | READY | review | -- | westside-ai-assistant | | `review-597-2026-03-28` | #597 | NEEDS_REFINEMENT | **doc** | **review** (fixed) | westside-ai-assistant | | `review-597-2026-03-28-r2` | #597 | READY | **doc** | **review** (fixed) | westside-ai-assistant | ### Fixes Applied 1. **Fixed note_type on 3 notes:** `review-595-2026-03-28`, `review-597-2026-03-28`, `review-597-2026-03-28-r2` changed from `doc` to `review`. 2. **Root cause:** `template-review` naming convention section said `note_type: doc (until review is added to NoteType enum)` -- but `review` IS a valid NoteType now. Stale guidance caused the westside-ai-assistant session to use `doc`. ### Duplicate Reviews (Review Chains) These are expected -- they are the review-fix-re-review loop in action: - **#356**: 2 reviews (NEEDS_REFINEMENT -> READY) - **#364**: 2 reviews (NEEDS_REFINEMENT -> READY) - **#376**: 2 reviews (NEEDS_REFINEMENT -> READY) - **#416**: 2 reviews (NEEDS_REFINEMENT -> READY) - **#418**: 3 reviews (all NEEDS_REFINEMENT -- no final READY yet) - **#464**: 2 reviews (NEEDS_REFINEMENT -> READY) - **#595**: 2 reviews (NEEDS_REFINEMENT -> READY) - **#597**: 2 reviews (NEEDS_REFINEMENT -> READY) ### Naming Convention Conflict Found Three sources gave conflicting slug guidance: - `skill-review-ticket` step 12: `review-{board_item_id}-{YYYY-MM-DD}` (authoritative) - `template-review`: `review-{issueNumber}-{YYYY-MM-DD}` (stale) - `note-conventions`: `review-{repo}-{pr-number}` (describes PR reviews, different artifact) **Resolution:** `skill-review-ticket` is authoritative. All existing review notes already follow the board-item-ID pattern. `template-review` naming convention section needs updating to match. ### Convention Note Created `convention-review-note-lifecycle` -- covers naming, note_type, tags, status, cleanup rules, and documents the source conflict resolution. ### Remaining Work (for Betty Sue) 1. **Update `template-review` naming convention section** to say `review-{board_item_id}` and `note_type: review` (currently says `{issueNumber}` and `doc`). 2. **Board item #418 has 3 NEEDS_REFINEMENT reviews with no READY** -- the ticket refinement loop may be stuck. 3. **Orphan cleanup** requires checking each board item's current column. Review notes for items in `done` older than 7 days are cleanup candidates per the new convention.