pal-e-docs

Review: Partial indexes for mermaid blocks + architecture notes (r2)

review-944-2026-04-10-r2 Review

review approved
Project

Verdict: APPROVED

Round 2 re-review after refinement. All four round-1 blockers resolved. Ticket is READY to move backlog → todo.

Round 1 Blocker Resolution

  • [x] [SCOPE] arch-blocks backing note — RESOLVED. arch-domain-pal-e-docs created 2026-04-10 with a blocks row in the Components table covering SQLAlchemy Block, halfvec embeddings, mermaid-skip policy, and the (note_id, anchor_id) uniqueness that enables get_section. Ticket Architecture section now points at this note.
  • [x] [SCOPE] arch-notes backing note — RESOLVED. Same arch-domain-pal-e-docs note has a notes row covering SQLAlchemy Note, the html_content legacy vs blocks-authoritative split, and explicitly calls out the missing note_type index as the opportunity this ticket fills (“opportunity for partial index — see ticket pal-e-api#252”). Forward reference is wired.
  • [x] [LABEL] arch:notes vs arch:notes-api — RESOLVED. Verified sibling backlog item #908 points at Forgejo issue pal-e-api#251 titled “Create arch-landing-site architecture note for westside-landing”. Its arch:notes-api label refers to the API-routes layer (westside landing-site API). This ticket’s arch:notes refers to the notes DB entity row in arch-domain-pal-e-docs. Different components at different abstraction layers, not a spelling conflict. Ticket body documents the distinction explicitly.
  • [x] [BODY] Non-deterministic EXPLAIN ACs — RESOLVED. AC #5 and #6 now prescribe SET enable_seqscan = off; before the EXPLAIN, with a paragraph explaining why (planner may legitimately prefer Seq Scan at 25-row scale; disabling seqscan is the deterministic idiom to verify the partial index is query-planner-visible). Checklist item updated to match.

Template Completeness

  • [x] Type (Feature)
  • [x] Lineage (includes round-1 review reference — good provenance)
  • [x] Repo (with rename-artifact note preserved)
  • [x] User Story
  • [x] Architecture (new section, names both arch:blocks and arch:notes with pointer to arch-domain-pal-e-docs)
  • [x] Context
  • [x] File Targets (with explicit exclusions)
  • [x] Acceptance Criteria (7 items, all verifiable)
  • [x] Test Expectations
  • [x] Constraints
  • [x] Checklist
  • [x] Related

Traceability

  • [x] story:superuser-query label — Superuser query path
  • [x] story note verified — row present in project-pal-e-docs user-stories table: “I can query the knowledge base by meaning (semantic search)…”
  • [x] arch:blocks label — backing component verified in arch-domain-pal-e-docs Components table
  • [x] arch:notes label — backing component verified in arch-domain-pal-e-docs Components table (and forward-references this ticket by number)
  • [x] area:db label — appropriate
  • [x] Forgejo issue — pal-e-api#252 open, refined body matches ticket brief

File Targets

  • [x] alembic/versions/<next>_add_mermaid_and_architecture_partial_indexes.py — verified. Current Alembic head is t0o1p2q3r4s5_drop_legacy_boards_table.py (confirmed via Forgejo API listing of alembic/versions/). New migration’s down_revision must chain to this slug. Ticket explicitly instructs the agent to re-verify via alembic heads before writing, which is the right belt-and-suspenders.
  • [x] Exclusion of src/pal_e_docs/models.py preserved from round 1 — still correct (SQLAlchemy Index can’t express partial WHERE clauses cleanly).

Repo Placement

OK. pal-e-api is correct; the pal_e_docs package name inside pal-e-api is a known rename artifact, documented in the ticket body.

Dependencies

  • [x] Current Alembic head satisfied (t0o1p2q3r4s5, no pending migrations).
  • [x] No in_progress board items on board-pal-e-docs touch the blocks or notes schema.
  • [x] Backing arch note now exists (arch-domain-pal-e-docs) — no longer blocked.

Acceptance Criteria

7 criteria. All verifiable by an agent post-implementation:
  • Two DDL statements with exact index names matching the ix_<table>_<columns> convention and partial predicates.
  • Downgrade symmetry (drops both).
  • alembic heads chain verification.
  • Two EXPLAIN assertions with SET enable_seqscan = off — now deterministic at any data volume.
  • Migration applies against current prod schema (dev overlay gate).
Slightly over the 5-AC caution threshold (7), but all 7 are tightly coupled to a single migration file with no independent subtasks. Splitting would produce pointless ceremony. Holding the round-1 judgment: single-pass work.

Blast Radius

Minimal. Pure additive DDL. Partial indexes cost near-zero bytes (~25+25 rows). One file created, one repo, no application code changes, no downstream consumers. Rollback is trivial. blocks.content json→jsonb still explicitly deferred. Unchanged from round 1.

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: 7 (over the 5-AC threshold, but cohesive — one file, one concern, no independent subtasks)
  • Estimated agent work: ~3–5 minutes
  • No parallelizable subtasks

Recommendation

No action needed. Move backlog → todo. Ticket is READY for dev dispatch.

Drift Note (not a blocker)

Round 1 noted that skill-review-ticket format says “Decomposition” but the template-review hook requires “Decomposition Assessment.” This review uses “Decomposition Assessment” to match the hook. Skill-vs-hook drift is a separate dogfooding ticket; not in scope for this review.