pal-e-docs

Review: Issue #225 -- Remove deprecated NoteTypes from enum

review-225-2026-03-27 Review

review needs-refinement
Project

Review: Issue #225 -- Remove deprecated NoteTypes from enum

Issue: forgejo_admin/pal-e-api #225
Title: Remove deprecated NoteTypes from enum (reference, journal, incident, post, todo, issue, milestone)
Reviewed: 2026-03-27
Reviewer: Dottie
Verdict: NEEDS_REFINEMENT

1. Template Compliance (template-issue-feature)

Section Present? Notes
Type Yes Feature
Lineage Yes Links to claude-custom #180
Repo Yes forgejo_admin/pal-e-api
User Story Yes As a developer / minimal type system
Context Yes References audit + enforcement chain
File Targets Yes <strong>ERRORS -- see finding F1</strong>
Acceptance Criteria Yes <strong>INCOMPLETE -- see finding F2</strong>
Test Expectations Yes Adequate
Constraints Yes <strong>DATA ERRORS -- see findings F3, F4</strong>
Checklist Yes Standard 3-item
Related Yes Links project + parent spike
Template sections: all present. Content accuracy: 4 critical errors.

2. Findings

F1: File Targets reference wrong repo (BLOCKER)

The issue lists two frontend files as targets:
  • src/pal_e_docs/static/colors.ts -- does not exist
  • src/pal_e_docs/static/app.css -- does not exist
There is no static/ directory under src/pal_e_docs/ at all. The actual files live in a completely different repo:
  • pal-e-app/src/lib/colors.ts -- contains type color tokens for: issue, todo, reference, journal, post, incident
  • pal-e-app/src/app.css -- contains CSS custom properties: --type-issue, --type-todo, --type-reference, --type-journal, --type-post, --type-incident, plus badge and card selectors referencing them
Fix: Either (a) remove these from the pal-e-api issue and create a separate pal-e-app issue for frontend cleanup, or (b) re-scope this as a cross-repo ticket and document both repos. Option (a) is cleaner -- one ticket per repo per convention.

F2: Acceptance Criteria only cover 4 of 7 types (BLOCKER)

The issue removes 7 types: reference, journal, incident, post, todo, issue, milestone. But the acceptance criteria only test 4:
  • reference -- 422 on create (tested)
  • todo -- 422 on create (tested)
  • issue -- 422 on create (tested)
  • milestone -- 422 on create (tested)
  • journal -- NOT tested
  • incident -- NOT tested
  • post -- NOT tested
Fix: Add acceptance criteria for all 7 types, or explicitly document why 4 is sufficient (it is not).

F3: "11 open todo notes" claim is wrong (BLOCKER)

The Constraints section states: "11 open todo notes must become Forgejo issues first (manual triage)."
Actual count from pal-e-docs query (list_notes(note_type="todo", include_cold=True)):
  • Total todo notes: 57
  • Status "open": 7 (not 11)
  • Status "done": 48
  • Other statuses: 2
The 7 open todo notes are:
  • todo-pre-merge-infra-validation (pal-e-agency)
  • todo-capacitor-audit-agent (pal-e-agency)
  • todo-playground-auto-deploy (westside-basketball)
  • todo-westside-app-pr11-qa-nits (westside-basketball)
  • todo-monitoring-stack-mcp-api (pal-e-platform)
  • todo-gpg-physical-backup (pal-e-platform)
  • todo-token-metrics-dora-correlation (pal-e-docs)
Additionally, 3 notes have status "open" but tag "done" (stale tag state): todo-jinja2-plan-templates, todo-mcp-clear-points-labels, todo-archived-status-for-todos. These need triage too but are likely just status/tag drift.
Fix: Update the constraint to reference the actual count (7 open + 3 with tag/status drift = 10, not 11). The prerequisite triage work itself is valid -- just the number is wrong.

F4: "6 milestone notes" claim is fabricated (BLOCKER)

The Constraints section states: "6 milestone notes need children re-parented (12 plans with parent_slug pointing to milestones)."
Actual count from pal-e-docs query (list_notes(note_type="milestone", include_cold=True)):
  • Total milestone notes: 0
There are zero notes with note_type "milestone" in the entire database. This prerequisite does not exist. The constraint is fabricated data -- likely an LLM hallucination during issue creation.
Fix: Remove this constraint entirely. No milestone re-parenting is needed because no milestone notes exist.

3. Prerequisite Documentation Check

Prerequisite Documented? Accurate?
Must run AFTER #224 data migration Yes (Constraints + Lineage) Yes -- valid dependency
11 open todo notes triage Yes (Constraints) <strong>No -- count is 7, not 11</strong>
6 milestone re-parenting Yes (Constraints) <strong>No -- 0 milestones exist</strong>

4. Scope Fitness (5-minute rule)

The actual backend work (remove 7 values from NoteType Literal + remove 7 entries from VALID_STATUSES dict in routes/notes.py) is a small, mechanical change. Two files, well-scoped deletions. This fits the 5-minute rule easily -- IF the frontend files are split to a separate ticket.
With the cross-repo frontend work included, this becomes a multi-repo ticket violating one-ticket-one-repo convention.

5. Existing Notes Impact Assessment

Notes currently using the 7 deprecated types that must be migrated BEFORE this issue executes:
Type Count Migration needed
todo 57 Yes -- all 57 need retyping (most to "doc" or deletion)
reference 27 Yes -- retype to "doc" or new type
journal 3 Yes -- retype (private notes)
incident 3 Yes -- retype to "doc" or new type
post 3 Yes -- retype to "doc" or new type
issue 1 Yes -- retype to "doc"
milestone 0 None needed
Total notes requiring migration: 94. This is the scope of the sibling issue #224. This issue (#225) correctly depends on #224 completing first.

6. Verdict: NEEDS_REFINEMENT

Four blockers must be resolved before this ticket moves to next_up:
  • F1: Remove colors.ts and app.css from File Targets. Create separate pal-e-app issue for frontend type color cleanup.
  • F2: Add acceptance criteria for journal, incident, and post (all 7 types must have 422 test).
  • F3: Correct todo count from 11 to 7 (+ 3 tag/status drift).
  • F4: Remove fabricated milestone constraint entirely (0 milestones exist).