headquarter/openspec/changes/notification-center/apply-pr1.md

PR-1 Apply Report: Backend Core for Notification Center

Status: COMPLETE

All 11 tasks for PR-1 (NC-PR1-001 through NC-PR1-011) have been implemented, tested, and validated.

What Was Implemented

Database Layer

• Alembic migration (alembic/versions/2026_05_29_add_notifications_table.py)

  • Creates notifications table with all design-spec columns
  • FK user_id -> users.id with ON DELETE CASCADE
  • Index idx_notifications_user_created_at on (user_id, created_at DESC)
  • Partial index idx_notifications_user_unread on (user_id, read_at) where read_at IS NULL

• SQLAlchemy model (src/models/notification.py)

  • Notification class with UUIDPrimaryKeyMixin + Base
  • notification_metadata attribute mapped to DB column "metadata" (avoids SQLAlchemy Base.metadata conflict)
  • Exported from src/models/__init__.py

Service Layer

• NotificationService singleton (src/services/notification_service.py)
  • create_notification(session, user_id, ...) — inserts row, returns Notification
  • list_notifications(session, user_id, ...) — returns (items, total) tuple, excludes dismissed, supports unread_only and mute_categories
  • get_unread_count(session, user_id) — counts unread + non-dismissed
  • mark_read(session, notification_id, user_id) — sets read_at = now()
  • mark_all_read(session, user_id) — bulk update, returns count
  • dismiss(session, notification_id, user_id) — soft-delete via dismissed_at = now()
  • All methods enforce user_id filtering; wrong-owner raises ValueError("Notification not found")

API Layer

• FastAPI router (src/api/notifications.py) mounted at /notifications
  • GET /notifications — paginated list with limit, offset, unread_only query params; limit capped at 100
  • GET /notifications/unread — returns {count: int}
  • PATCH /notifications/{id}/read — marks single notification read
  • POST /notifications/mark-all-read — returns {marked_count: int}
  • DELETE /notifications/{id} — soft-delete (dismiss), returns 204
  • Reads notification_mute_categories from UserConfig.config JSON blob and passes to list_notifications
  • Returns 404 for non-owned or missing notifications
  • Pydantic NotificationItem serializes notification_metadata as "metadata" via Field(serialization_alias="metadata")

Registration

• Router imported and included in src/main.py
• Notification model imported in src/main.py with # noqa: F401 for Alembic autogenerate discovery
• notifications_router exported from src/api/__init__.py

Tests

• 13 unit tests (tests/unit/test_notification_service.py) covering:
  • Create, list, unread count, mark read, mark all read, dismiss
  • Cross-user isolation, wrong-owner 404-equivalent, mute categories filtering
  • Dismissed excluded from unread count, mark-all-read affects only caller
• 10 integration tests (tests/integration/test_notifications_api.py) covering:
  • Auth requirements, ownership isolation, pagination
  • Mark read / dismiss endpoints and 404 for other users
  • Mute categories filter at API layer

Changed Files

1. apps/api/alembic/versions/2026_05_29_add_notifications_table.py (new)
2. apps/api/src/models/notification.py (new)
3. apps/api/src/models/__init__.py
4. apps/api/src/services/notification_service.py (new)
5. apps/api/src/api/notifications.py (new)
6. apps/api/src/api/__init__.py
7. apps/api/src/main.py
8. apps/api/tests/unit/test_notification_service.py (new)
9. apps/api/tests/integration/test_notifications_api.py (new)
10. apps/api/tests/integration/test_models.py

Test Evidence

RED -> GREEN -> TRIANGULATE Cycles

Cycle	Task	RED	GREEN	Result
1	Service unit tests (basic CRUD)	13 tests written against missing service	Implemented NotificationService	13 passed
2	Service edge cases	Wrong-owner, mute categories, cross-user tests added	Already green from implementation	13 passed
3	API integration tests (basic endpoints)	10 tests written against missing router	Implemented router + schemas	10 passed
4	API edge cases	Pagination, 404 ownership, mute categories at API layer	Already green from implementation	10 passed
5	REFACTOR	—	Ruff clean, no regressions	All new files pass ruff

Commands Run

# NotificationService unit tests (13 tests)
cd apps/api && python -m pytest tests/unit/test_notification_service.py -v
# Exit: 0 — 13 passed

# Notifications API integration tests (10 tests)
cd apps/api && python -m pytest tests/integration/test_notifications_api.py -v
# Exit: 0 — 10 passed

# Combined new tests
cd apps/api && python -m pytest tests/unit/test_notification_service.py tests/integration/test_notifications_api.py -v
# Exit: 0 — 23 passed

# Existing unit suite (no regressions from our changes)
cd apps/api && python -m pytest tests/unit/ -v
# Exit: 1 — 223 passed, 4 failed (pre-existing failures in test_config.py and test_git_repository_clone_preflight.py)

# Ruff linting on all new/modified files
cd apps/api && python -m ruff check \
  src/models/notification.py src/models/__init__.py \
  src/services/notification_service.py \
  src/api/notifications.py src/api/__init__.py src/main.py \
  alembic/versions/2026_05_29_add_notifications_table.py \
  tests/unit/test_notification_service.py \
  tests/integration/test_notifications_api.py \
  tests/integration/test_models.py
# Exit: 0 — All checks passed

# Smoke tests
# GET /health -> 200
# GET /notifications (unauthenticated) -> 401

Deviations from Design

1. SQLAlchemy metadata column name conflict: Base.metadata is reserved by SQLAlchemy DeclarativeBase. Used notification_metadata as the Python attribute name with DB column name "metadata". In the Pydantic response model, used Field(serialization_alias="metadata") so the JSON API still exposes "metadata" as specified in the design.

2. Datetime types in Pydantic schemas: Used datetime instead of str for read_at, dismissed_at, and created_at to leverage FastAPI's automatic ISO-8601 serialization.

Surprises / Decisions

1. SQLite func.now() timestamp resolution: test_list_notifications_orders_by_created_at_desc initially failed because multiple rapid INSERTs received identical timestamps. Fixed by explicitly setting created_at offsets in the test after creation.

2. Pre-existing integration test failures: Approximately 40 integration tests fail due to missing asyncpg module and direct PostgreSQL connection attempts in their custom setup code. These failures are unrelated to our changes.

3. Pre-existing test_models.py outdated: The test_expected_tables_are_registered assertion had a hardcoded set missing many newer tables (including our new notifications table). Updated it to include all current tables.

PR Boundary

This PR covers PR-1 only (NC-PR1-001 through NC-PR1-011). PR-2 (backend integration — wiring lifecycle_hooks.py and health_monitor.py) and PR-3/PR-4 (frontend) are out of scope and await this PR.

Risks

• Low: The migration uses sa.JSON() which is compatible with both PostgreSQL and SQLite. The partial index uses postgresql_where which is PostgreSQL-specific but safely ignored by SQLite.
• Low: notification_metadata -> "metadata" serialization alias is a new pattern in the codebase but is explicitly tested via integration tests.
• None: No changes to existing production code paths; all changes are additive.