pal-e-docs

Review: Partial indexes for mermaid blocks + architecture notes

review-944-2026-04-10 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Scope is technically solid and executable in a single agent pass. Correct repo placement, verified file targets, idiomatic Alembic pattern, testable AC. Blocked on two missing backing architecture notes (traceability triangle) and one label-spelling reconciliation.

Template Completeness

  • [x] Type (Feature)
  • [x] Lineage
  • [x] Repo
  • [x] User Story
  • [x] Context (strong — explains semantic-documentation motivation vs performance)
  • [x] File Targets (with explicit exclusions)
  • [x] Acceptance Criteria (6 items, all verifiable)
  • [x] Test Expectations (up/down round-trip + EXPLAIN)
  • [x] Constraints (partial-index pattern, naming convention, jsonb out of scope)
  • [x] Checklist
  • [x] Related

Traceability

  • [x] story:superuser-query label — Superuser query path
  • [x] story note verified — found in project-pal-e-docs user-stories section (row: "I can query the knowledge base by meaning...")
  • [x] arch:blocks label present
  • [ ] arch-blocks note MISSING — [SCOPE] Create architecture note arch-blocks for the blocks-table component
  • [x] arch:notes label present
  • [ ] arch-notes note MISSING — [SCOPE] Create architecture note arch-notes for the notes-table component
  • [!] Label spelling inconsistency — sibling backlog ticket #908 (pal-e-api) uses arch:notes-api; this ticket uses arch:notes. Confirm canonical label before creating backing notes to avoid two arch notes for the same component.
  • [x] Forgejo issue — https://forgejo.tail5b443a.ts.net/forgejo_admin/pal-e-api/issues/252 (open)

File Targets

  • [x] alembic/versions/<next>_add_mermaid_and_architecture_partial_indexes.py — verified: alembic/versions/ exists in forgejo_admin/pal-e-api. Current head is t0o1p2q3r4s5 (drop_legacy_boards_table). New migration's down_revision must chain to this.
  • [x] Ticket explicitly excludes src/pal_e_docs/models.py edits — verified correct: SQLAlchemy's Index(...) in __table_args__ does not express WHERE clauses for partial indexes. Existing convention in models.py (e.g. ix_blocks_note_id_position, ix_blocks_anchor_id) matches the ix_<table>_<columns> naming the ticket prescribes.
  • [x] Block model has block_type: Mapped[str] at models.py:221; Note model has note_type: Mapped[str | None] at models.py:112. Both columns exist and the types match the ticket's assumptions.

Repo Placement

OK. Initially flagged as a potential repo-placement bug because the task brief said "models.py is in ~/pal-e-docs." Verified on Forgejo: schema source lives at forgejo_admin/pal-e-api/src/pal_e_docs/models.py (the Python package is named pal_e_docs inside the pal-e-api repo — a naming artifact from the pal-e-docs→pal-e-api repo rename, board item #439). Alembic also lives in pal-e-api. Ticket is filed against the correct repo. No split across repos needed.

Dependencies

  • [x] Current alembic head t0o1p2q3r4s5 — satisfied (no pending migrations on main)
  • [x] No blocking in_progress items on board-pal-e-docs touch the blocks or notes schema — satisfied
  • [!] arch-blocks and arch-notes backing notes — pending ([SCOPE] above)

Acceptance Criteria

All six AC are verifiable by an agent post-implementation:
  • Index existence: introspect pg_indexes or \d blocks / \d notes
  • EXPLAIN plans: verifiable, though at 23K blocks / 900 notes PostgreSQL may still prefer Seq Scan for the architecture-notes case (900 rows is tiny). Ticket body should note: if planner chooses Seq Scan, use SET enable_seqscan = off to force the partial index and confirm it is usable. Otherwise AC #4 and #5 may fail non-deterministically at current scale.
  • Round-trip: alembic upgrade head && alembic downgrade -1 && alembic upgrade head — real command, runnable in test DB.

Blast Radius

Minimal. Pure additive DDL. Partial indexes cost ~25 rows for mermaid, ~20 for architecture (near-zero bytes). One file created, one repo, no application code changes, no downstream consumers. Rollback is trivial (alembic downgrade -1 drops both indexes). blocks.content json→jsonb migration is explicitly deferred to separate scope. No sibling pattern exists elsewhere — this is a one-off declaration, not a bug being fixed in multiple places.

Decomposition Assessment

No decomposition needed.
  • Discrete changes: 1 (single migration file)
  • Files touched: 1 created, 0 modified
  • Repos touched: 1 (pal-e-api)
  • Acceptance criteria: 6 (under the 5-criterion caution threshold — all verifiable via a single agent session)
  • Estimated agent work: ~3 minutes (well under the 5-minute rule)
  • No independent subtasks to parallelize

Recommendation

  • [SCOPE] Create architecture note arch-blocks (or the canonical label spelling) documenting the blocks table as a first-class query surface. Should reference that mermaid is a partial-index query path.
  • [SCOPE] Create architecture note arch-notes (or arch-notes-api — reconcile with #908's arch:notes-api label first) documenting the notes table. Should reference that architecture-type lookups are a partial-index query path.
  • [LABEL] Reconcile canonical arch label spelling: arch:notes (this ticket) vs arch:notes-api (#908). If canonical is arch:notes-api, update this ticket's label to match before moving to todo.
  • [BODY] Add a sentence to Acceptance Criteria noting that if PostgreSQL planner chooses Seq Scan over the partial index at current scale, the agent should verify with SET enable_seqscan = off to confirm the index is usable (idiomatic partial-index verification).
Once the two arch notes exist and the label spelling is reconciled, this ticket is READY. File targets, decomposition, repo placement, and the story leg of traceability are already confirmed.