# 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

```bash
# 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.