pal-e-docs

Review: feat: add Postgres trigger to bump projects.updated_at

review-1344-2026-06-06 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- Standalone, discovery context provided
  • [x] Repo -- ldraney/paldocs
  • [x] User Story -- properly formatted As/I want/So that
  • [x] Context -- clear motivation, solution approach explained
  • [x] File Targets -- 3 files to modify/create, 1 exclusion documented
  • [x] Acceptance Criteria -- 8 items, well-scoped
  • [x] Test Expectations -- 3 manual checks + run command
  • [x] Constraints -- shared DB, NULL handling, plpgsql noted
  • [x] Checklist -- present
  • [x] Related -- project + related issue #14 linked

Traceability

  • [x] story:project-pages label -- present on board item
  • [ ] story note MISSING -- project-paldocs user-stories section exists but has no entries ("No user-story notes created yet"). [SCOPE] Create user story entry for story:project-pages on project-paldocs user-stories section.
  • [x] arch:rails-db label -- present on board item
  • [ ] arch note MISSING -- search for "arch-rails-db" returned zero results. [SCOPE] Create architecture note arch-rails-db for the Rails database integration component.
  • [x] Forgejo issue -- ldraney/paldocs#13, state: open

File Targets

  • [ ] db/migrate/YYYYMMDD_add_project_activity_trigger.rb -- ISSUE: The db/migrate/ directory does not exist. Zero migrations exist in this repo. The docs/data-model.md explicitly states: "No db/schema.rb or db/migrate/ in this repo" and "all DDL is managed by the FastAPI pal-e-docs service." Creating a Rails migration contradicts the established shared-DB architecture. The trigger DDL should be managed by the Python pal-e-docs layer (Alembic or raw SQL), not by a Rails migration.
  • [x] bin/docker-entrypoint -- verified exists (6 lines). Currently contains comment "Shared database -- Rails does not manage migrations. Skip db:prepare; tables already exist (managed by pal-e-docs FastAPI)." Adding db:migrate here contradicts the architecture. However, if the DDL ownership question is resolved in favor of Rails managing triggers, this change makes sense.
  • [x] docs/data-model.md -- verified exists (129 lines). Contains ER diagram and shared-DB documentation. Section about triggers would be appropriate here.
  • [x] app/controllers/projects_controller.rb -- verified: sort query is Project.order(updated_at: :desc) on line 3, correctly excluded from modification targets.

Repo Placement

Issue is filed on ldraney/paldocs, which is the Rails app. However, the core question is whether this trigger DDL belongs in the Rails repo or in the Python pal-e-docs service that currently owns all DDL. The issue assumes Rails ownership without addressing this architectural decision. The docs/data-model.md in the same repo explicitly says DDL is managed by FastAPI. This is an internal contradiction that must be resolved before implementation.

Dependencies

  • Issue #14 (spike: audit infra/pipeline gaps) is referenced as a dependency for the entrypoint pattern decision. Issue #14 is open and in backlog on the same board (item #1345). The ticket says it "can proceed independently" but modifying the entrypoint without the spike's findings risks rework.
  • The Python pal-e-docs service (separate repo) owns all DDL. If the trigger is added via Rails migration, this breaks the DDL ownership contract. The Python service's Alembic/migration system is not accounted for.

Acceptance Criteria

ACs are well-structured and testable by an agent, with one exception: AC "Projects index sorts by actual activity after deploy" is vague -- it depends on deploy happening and data existing. The manual test expectations are reasonable. The bin/rails db:migrate run command is valid if the migration approach is approved.
Missing AC: No criterion for verifying the trigger doesn't conflict with the Python layer's writes to the notes table. Should verify that Python INSERT/UPDATE on notes still works after trigger is added.

Blast Radius

  • The Python pal-e-docs API performs heavy writes to the notes table (create, update, bulk operations via MCP). A Postgres trigger on notes INSERT/UPDATE will fire for every Python write. While the trigger is simple (bump parent project's updated_at), this is an invisible side effect for the Python layer.
  • Board items also reference projects (indirectly through board notes). Should the trigger also fire on board_item changes? The ticket only covers notes, but board activity is also "project activity."
  • The blocks table (child of notes) is also a signal of project activity. Not covered.

Decomposition Assessment

3 file targets in 1 repo, 8 acceptance criteria. The file count is fine, but the 8 ACs are at the threshold. More importantly, the architectural question (DDL ownership) must be resolved before implementation can proceed. No decomposition needed for the implementation itself -- this is a single-pass task once the approach is decided. However, the DDL ownership decision is a prerequisite that may change the file targets entirely.

Recommendations

  • [SCOPE] Resolve DDL ownership: should this trigger be a Rails migration or managed via the Python pal-e-docs service? The current architecture (documented in docs/data-model.md) says all DDL is Python-managed. If Rails takes ownership of triggers, update docs/data-model.md to document the exception. If Python keeps ownership, the file targets change entirely (no Rails migration, no entrypoint change).
  • [BODY] Fix file target: db/migrate/YYYYMMDD_add_project_activity_trigger.rb -- db/migrate/ directory does not exist and the architecture forbids Rails migrations. Either justify the exception or move DDL to the Python layer.
  • [BODY] Add acceptance criterion: verify Python-layer writes to notes table still function correctly after trigger is installed.
  • [SCOPE] Create user story entry for story:project-pages on project-paldocs user-stories section.
  • [SCOPE] Create architecture note arch-rails-db for the Rails database integration component.