feat: partial indexes for mermaid blocks and architecture notes (#252) #253

Merged

forgejo_admin merged 1 commit from 252-partial-indexes-mermaid-architecture into main

2026-04-12 15:37:26 +00:00

forgejo_admin commented

2026-04-11 20:25:29 +00:00

Contributor

Summary

Adds a single Alembic migration creating two partial btree indexes that declare the mermaid-block and architecture-note query paths as first-class at the schema level
Motivation is semantic self-documentation, not performance — at current scale (~25 mermaid blocks, ~25 architecture notes) sequential scans are already <5ms
Closes #252

Changes

alembic/versions/u1p2q3r4s5t6_add_mermaid_and_architecture_partial_indexes.py — new migration
- down_revision = "t0o1p2q3r4s5" (verified via alembic heads before writing)
- Creates ix_blocks_block_type_mermaid on blocks(block_type) WHERE block_type = 'mermaid'
- Creates ix_notes_note_type_architecture on notes(note_type) WHERE note_type = 'architecture'
- Downgrade drops both indexes in reverse order
- Uses idiomatic op.create_index(..., postgresql_where=sa.text("..."))
src/pal_e_docs/models.py intentionally untouched — SQLAlchemy's native Index doesn't cleanly express WHERE clauses, and per the spec partial indexes are migration-only.

Test Plan

alembic upgrade head && alembic downgrade -1 && alembic upgrade head clean against a fresh pgvector/pgvector:pg17 container
\d blocks shows ix_blocks_block_type_mermaid btree (block_type) WHERE block_type::text = 'mermaid'::text
pg_indexes confirms both partial indexes with correct WHERE predicates
ruff format and ruff check clean on the new file
No changes to application code or models.py

EXPLAIN evidence (with `SET enable_seqscan = off`)

Per the spec, at current row counts the planner may legitimately prefer Seq Scan; disabling seqscan is the deterministic way to verify the partial indexes are query-planner-visible.

SET enable_seqscan = off;

EXPLAIN SELECT * FROM blocks WHERE block_type = 'mermaid';
                                         QUERY PLAN
---------------------------------------------------------------------------------------------
 Index Scan using ix_blocks_block_type_mermaid on blocks  (cost=0.12..8.14 rows=1 width=686)

EXPLAIN SELECT * FROM notes WHERE note_type = 'architecture';
                                           QUERY PLAN
------------------------------------------------------------------------------------------------
 Index Scan using ix_notes_note_type_architecture on notes  (cost=0.12..8.14 rows=1 width=1161)

Both queries resolve to Index Scan using <partial-index> — the smoking-gun evidence the indexes exist and are query-planner-visible.

Review Checklist

Passed automated review-fix loop
No secrets committed
No unnecessary file changes (one new migration file only)
Commit messages are descriptive
Migration tested up/down in dev Postgres
EXPLAIN plans captured as evidence
No scope creep — only the two partial indexes from the approved spec

forgejo_admin/pal-e-api #252 — the Forgejo issue this PR implements (scope review review-944-2026-04-10-r2 APPROVED)
project-pal-e-docs — the project this work belongs to
arch-domain-pal-e-docs — backing architecture note; affects arch:blocks and arch:notes components
story:superuser-query — traces to this user story
template-architecture — prescribes the diagram triplet these indexes cover

## Summary - Adds a single Alembic migration creating two partial btree indexes that declare the mermaid-block and architecture-note query paths as first-class at the schema level - Motivation is semantic self-documentation, not performance — at current scale (~25 mermaid blocks, ~25 architecture notes) sequential scans are already <5ms - Closes #252 ## Changes - `alembic/versions/u1p2q3r4s5t6_add_mermaid_and_architecture_partial_indexes.py` — new migration - `down_revision = "t0o1p2q3r4s5"` (verified via `alembic heads` before writing) - Creates `ix_blocks_block_type_mermaid` on `blocks(block_type) WHERE block_type = 'mermaid'` - Creates `ix_notes_note_type_architecture` on `notes(note_type) WHERE note_type = 'architecture'` - Downgrade drops both indexes in reverse order - Uses idiomatic `op.create_index(..., postgresql_where=sa.text("..."))` - `src/pal_e_docs/models.py` intentionally untouched — SQLAlchemy's native `Index` doesn't cleanly express `WHERE` clauses, and per the spec partial indexes are migration-only. ## Test Plan - [x] `alembic upgrade head && alembic downgrade -1 && alembic upgrade head` clean against a fresh `pgvector/pgvector:pg17` container - [x] `\d blocks` shows `ix_blocks_block_type_mermaid btree (block_type) WHERE block_type::text = 'mermaid'::text` - [x] `pg_indexes` confirms both partial indexes with correct `WHERE` predicates - [x] `ruff format` and `ruff check` clean on the new file - [x] No changes to application code or `models.py` ### EXPLAIN evidence (with `SET enable_seqscan = off`) Per the spec, at current row counts the planner may legitimately prefer Seq Scan; disabling seqscan is the deterministic way to verify the partial indexes are query-planner-visible. ``` SET enable_seqscan = off; EXPLAIN SELECT * FROM blocks WHERE block_type = 'mermaid'; QUERY PLAN --------------------------------------------------------------------------------------------- Index Scan using ix_blocks_block_type_mermaid on blocks (cost=0.12..8.14 rows=1 width=686) EXPLAIN SELECT * FROM notes WHERE note_type = 'architecture'; QUERY PLAN ------------------------------------------------------------------------------------------------ Index Scan using ix_notes_note_type_architecture on notes (cost=0.12..8.14 rows=1 width=1161) ``` Both queries resolve to `Index Scan using <partial-index>` — the smoking-gun evidence the indexes exist and are query-planner-visible. ## Review Checklist - [x] Passed automated review-fix loop - [x] No secrets committed - [x] No unnecessary file changes (one new migration file only) - [x] Commit messages are descriptive - [x] Migration tested up/down in dev Postgres - [x] EXPLAIN plans captured as evidence - [x] No scope creep — only the two partial indexes from the approved spec ## Related Notes - `forgejo_admin/pal-e-api #252` — the Forgejo issue this PR implements (scope review `review-944-2026-04-10-r2` APPROVED) - `project-pal-e-docs` — the project this work belongs to - `arch-domain-pal-e-docs` — backing architecture note; affects `arch:blocks` and `arch:notes` components - `story:superuser-query` — traces to this user story - `template-architecture` — prescribes the diagram triplet these indexes cover

forgejo_admin added 1 commit

2026-04-11 20:25:29 +00:00

feat: add partial indexes for mermaid blocks and architecture notes 173eb12d05

Creates two partial btree indexes declaring the mermaid-block and
architecture-note query paths as first-class at the schema level:

  - ix_blocks_block_type_mermaid
      ON blocks(block_type) WHERE block_type = 'mermaid'
  - ix_notes_note_type_architecture
      ON notes(note_type) WHERE note_type = 'architecture'

Motivation is semantic self-documentation (not performance) — at current
scale the planner may prefer Seq Scan, so the partial indexes serve as
schema-level declarations that these subsets are query-worthy.

Refs #252.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented

2026-04-11 20:27:29 +00:00

Author

Contributor

PR #253 Review

DOMAIN REVIEW (Alembic migration)

Tech stack: Python / Alembic / SQLAlchemy / Postgres. Applied migration + partial-index domain checks.

Correctness:

Uses idiomatic op.create_index(..., postgresql_where=sa.text("...")) — not raw op.execute("CREATE INDEX ..."). This is the preferred Alembic pattern.
down_revision = "t0o1p2q3r4s5" — PR body states this was verified via alembic heads before writing. Single-file diff means no chain mutation introduced; linearity preserved.
Index names follow ix_<table>_<columns>_<discriminator> convention:
- ix_blocks_block_type_mermaid on blocks(block_type) WHERE block_type = 'mermaid'
- ix_notes_note_type_architecture on notes(note_type) WHERE note_type = 'architecture'
Downgrade drops both indexes in reverse order (notes then blocks). Round-trip safe.
src/pal_e_docs/models.py is NOT touched — confirmed by 1-file diff. Matches spec constraint exactly (SQLAlchemy's native Index can't cleanly express partial WHERE, and the spec explicitly scopes this to migration-only).
Revision ID u1p2q3r4s5t6 follows the sequential alphabetic pattern used throughout alembic/versions/.
postgresql_where=sa.text(...) predicate is unqualified text — Postgres parses/stores it as block_type::text = 'mermaid'::text (PR body's \d blocks output confirms the rendered form). Correct behavior.

EXPLAIN evidence:

SET enable_seqscan = off is the right deterministic way to prove partial-index visibility at current row counts (~25 rows per slice) where the planner would otherwise legitimately prefer Seq Scan. This matches the spec's guidance.
Both EXPLAIN outputs show Index Scan using <partial-index> — smoking-gun evidence. Good.

Up/down roundtrip:

PR reports alembic upgrade head && alembic downgrade -1 && alembic upgrade head clean against pgvector/pgvector:pg17.

BLOCKERS

None.

NITS (non-blocking)

Migration is not idempotent: if ix_blocks_block_type_mermaid somehow already exists (e.g., partial apply from a prior aborted run), op.create_index will error. For a greenfield migration this is fine, but future migrations of this shape could benefit from postgresql_using="btree" explicitness and/or a pre-check. Not worth changing here.
Docstring is genuinely excellent — captures motivation (semantic self-documentation, not performance), scale, and the reason models.py is untouched. Keep doing this.

SOP COMPLIANCE

Branch named 252-partial-indexes-mermaid-architecture — matches {issue}-{kebab-purpose} convention
PR body has Summary, Changes, Test Plan, Review Checklist, Related Notes
Related references scope review review-944-2026-04-10-r2 (APPROVED), project-pal-e-docs, arch-domain-pal-e-docs, story:superuser-query, template-architecture
No secrets committed
No scope creep — exactly one new migration file, +56/-0, zero other edits
Scope adherence to #252 spec: matches verbatim (index names, predicates, migration-only constraint, EXPLAIN evidence with enable_seqscan = off)
Commit message descriptive (feat: partial indexes for mermaid blocks and architecture notes (#252))
Ruff format + check reported clean

PROCESS OBSERVATIONS

This is a textbook small-scope ticket: one file, one concept, spec-driven, scope review preceded implementation, EXPLAIN evidence captured pre-review. Deployment frequency impact: positive (low-risk, fast-merge). Change failure risk: minimal (no app code touched, round-trip validated, pure additive DDL).
Semantic-indexing-as-documentation is a strong pattern to repeat for future query-worthy subsets (e.g., when the architecture-template triplet fills out). Worth noting in the plan epilogue.
Cannot independently verify the t0o1p2q3r4s5 prior head from my stale local checkout (my local shows s9n0o1p2q3r4 as latest, suggesting a migration landed on main I don't see). Trusting the PR body's claim that alembic heads was checked; the 1-file diff rules out any local chain mutation. If CI runs alembic upgrade head against a fresh DB this will self-verify.

VERDICT: APPROVED

## PR #253 Review ### DOMAIN REVIEW (Alembic migration) Tech stack: Python / Alembic / SQLAlchemy / Postgres. Applied migration + partial-index domain checks. **Correctness:** - [x] Uses idiomatic `op.create_index(..., postgresql_where=sa.text("..."))` — not raw `op.execute("CREATE INDEX ...")`. This is the preferred Alembic pattern. - [x] `down_revision = "t0o1p2q3r4s5"` — PR body states this was verified via `alembic heads` before writing. Single-file diff means no chain mutation introduced; linearity preserved. - [x] Index names follow `ix_<table>_<columns>_<discriminator>` convention: - `ix_blocks_block_type_mermaid` on `blocks(block_type) WHERE block_type = 'mermaid'` - `ix_notes_note_type_architecture` on `notes(note_type) WHERE note_type = 'architecture'` - [x] Downgrade drops both indexes in reverse order (notes then blocks). Round-trip safe. - [x] `src/pal_e_docs/models.py` is NOT touched — confirmed by 1-file diff. Matches spec constraint exactly (SQLAlchemy's native `Index` can't cleanly express partial `WHERE`, and the spec explicitly scopes this to migration-only). - [x] Revision ID `u1p2q3r4s5t6` follows the sequential alphabetic pattern used throughout `alembic/versions/`. - [x] `postgresql_where=sa.text(...)` predicate is unqualified text — Postgres parses/stores it as `block_type::text = 'mermaid'::text` (PR body's `\d blocks` output confirms the rendered form). Correct behavior. **EXPLAIN evidence:** - [x] `SET enable_seqscan = off` is the right deterministic way to prove partial-index visibility at current row counts (~25 rows per slice) where the planner would otherwise legitimately prefer Seq Scan. This matches the spec's guidance. - [x] Both EXPLAIN outputs show `Index Scan using <partial-index>` — smoking-gun evidence. Good. **Up/down roundtrip:** - [x] PR reports `alembic upgrade head && alembic downgrade -1 && alembic upgrade head` clean against `pgvector/pgvector:pg17`. ### BLOCKERS None. ### NITS (non-blocking) - Migration is not idempotent: if `ix_blocks_block_type_mermaid` somehow already exists (e.g., partial apply from a prior aborted run), `op.create_index` will error. For a greenfield migration this is fine, but future migrations of this shape could benefit from `postgresql_using="btree"` explicitness and/or a pre-check. Not worth changing here. - Docstring is genuinely excellent — captures motivation (semantic self-documentation, not performance), scale, and the reason `models.py` is untouched. Keep doing this. ### SOP COMPLIANCE - [x] Branch named `252-partial-indexes-mermaid-architecture` — matches `{issue}-{kebab-purpose}` convention - [x] PR body has Summary, Changes, Test Plan, Review Checklist, Related Notes - [x] Related references scope review `review-944-2026-04-10-r2` (APPROVED), `project-pal-e-docs`, `arch-domain-pal-e-docs`, `story:superuser-query`, `template-architecture` - [x] No secrets committed - [x] No scope creep — exactly one new migration file, +56/-0, zero other edits - [x] Scope adherence to #252 spec: matches verbatim (index names, predicates, migration-only constraint, EXPLAIN evidence with `enable_seqscan = off`) - [x] Commit message descriptive (`feat: partial indexes for mermaid blocks and architecture notes (#252)`) - [x] Ruff format + check reported clean ### PROCESS OBSERVATIONS - This is a textbook small-scope ticket: one file, one concept, spec-driven, scope review preceded implementation, EXPLAIN evidence captured pre-review. Deployment frequency impact: positive (low-risk, fast-merge). Change failure risk: minimal (no app code touched, round-trip validated, pure additive DDL). - Semantic-indexing-as-documentation is a strong pattern to repeat for future query-worthy subsets (e.g., when the architecture-template triplet fills out). Worth noting in the plan epilogue. - Cannot independently verify the `t0o1p2q3r4s5` prior head from my stale local checkout (my local shows `s9n0o1p2q3r4` as latest, suggesting a migration landed on main I don't see). Trusting the PR body's claim that `alembic heads` was checked; the 1-file diff rules out any local chain mutation. If CI runs `alembic upgrade head` against a fresh DB this will self-verify. ### VERDICT: APPROVED