diff --git a/.planning/REQUIREMENTS.md b/.planning/REQUIREMENTS.md index d14e04e..3e19393 100644 --- a/.planning/REQUIREMENTS.md +++ b/.planning/REQUIREMENTS.md @@ -23,9 +23,9 @@ Requirements for initial release. Each maps to roadmap phases. ### Processing Analytics -- [ ] **ANLY-01**: Document processing events persist to Supabase at write time (not in-memory only) +- [x] **ANLY-01**: Document processing events persist to Supabase at write time (not in-memory only) - [ ] **ANLY-02**: Admin can view processing summary: upload counts, success/failure rates, avg processing time -- [ ] **ANLY-03**: Analytics instrumentation is non-blocking (fire-and-forget, never delays processing pipeline) +- [x] **ANLY-03**: Analytics instrumentation is non-blocking (fire-and-forget, never delays processing pipeline) ### Infrastructure @@ -83,8 +83,8 @@ Which phases cover which requirements. Updated during roadmap creation. | ALRT-01 | Phase 2 | Pending | | ALRT-02 | Phase 2 | Pending | | ALRT-04 | Phase 2 | Pending | -| ANLY-01 | Phase 2 | Pending | -| ANLY-03 | Phase 2 | Pending | +| ANLY-01 | Phase 2 | Complete | +| ANLY-03 | Phase 2 | Complete | | INFR-03 | Phase 2 | Pending | | INFR-02 | Phase 3 | Pending | | HLTH-01 | Phase 3 | Pending | diff --git a/.planning/STATE.md b/.planning/STATE.md index 136eea9..37d4959 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -59,6 +59,7 @@ Recent decisions affecting current work: - 02-02: Firebase Auth probe: verifyIdToken always throws; 'INVALID'/'Decoding'/'argument' in message = SDK alive = healthy - 02-02: Promise.allSettled for probe orchestration — all 4 probes run even if one throws outside its own try/catch - 02-02: Per-probe HealthCheckModel.create failure swallowed with logger.error — probe results still returned to caller +- [Phase 02-backend-services]: 02-01: recordProcessingEvent return type is void (not Promise) — type system prevents accidental await on critical path ### Pending Todos diff --git a/.planning/phases/02-backend-services/02-01-SUMMARY.md b/.planning/phases/02-backend-services/02-01-SUMMARY.md new file mode 100644 index 0000000..24c1f55 --- /dev/null +++ b/.planning/phases/02-backend-services/02-01-SUMMARY.md @@ -0,0 +1,139 @@ +--- +phase: 02-backend-services +plan: 01 +subsystem: analytics +tags: [supabase, vitest, fire-and-forget, analytics, postgresql, migrations] + +# Dependency graph +requires: + - phase: 01-data-foundation/01-01 + provides: getSupabaseServiceClient per-method call pattern, migration file format + - phase: 01-data-foundation/01-02 + provides: makeSupabaseChain() Vitest mock pattern, vi.mock hoisting rules + +provides: + - Migration 013: document_processing_events table DDL with indexes and RLS + - analyticsService.recordProcessingEvent(): fire-and-forget void write to Supabase + - analyticsService.deleteProcessingEventsOlderThan(): retention delete returning row count + - Unit tests for both exports (6 tests) + +affects: + - 02-backend-services/02-02 (monitoring services) + - 02-backend-services/02-03 (health probe scheduler) + - 03-api (callers that instrument processing pipeline events) + +# Tech tracking +tech-stack: + added: [] + patterns: + - "Fire-and-forget analytics: void return type (not Promise) prevents accidental await on critical path" + - "void supabase.from(...).insert(...).then(callback) pattern for non-blocking writes with error logging" + - "getSupabaseServiceClient() called per-method inside each exported function, never cached at module level" + +key-files: + created: + - backend/src/models/migrations/013_create_processing_events_table.sql + - backend/src/services/analyticsService.ts + - backend/src/__tests__/unit/analyticsService.test.ts + modified: [] + +key-decisions: + - "recordProcessingEvent return type is void (not Promise) — prevents callers from accidentally awaiting analytics writes on the critical processing path" + - "Optional fields (duration_ms, error_message, stage) coalesce to null in insert payload — consistent nullability in DB" + - "created_at set explicitly in insert payload (not relying on DB DEFAULT) so it matches the event occurrence time" + +patterns-established: + - "Analytics void function test: expect(result).toBeUndefined() + expect(typeof result).toBe('undefined') — toHaveProperty throws on undefined, use typeof check instead" + - "Fire-and-forget error path test: mock .insert().then() directly to control the resolved value, flush microtask queue with await Promise.resolve() before asserting logger call" + +requirements-completed: [ANLY-01, ANLY-03] + +# Metrics +duration: 3min +completed: 2026-02-24 +--- + +# Phase 02 Plan 01: Analytics Service Summary + +**Fire-and-forget analyticsService with document_processing_events migration — void recordProcessingEvent that logs errors without throwing, and deleteProcessingEventsOlderThan returning row count** + +## Performance + +- **Duration:** 3 min +- **Started:** 2026-02-24T19:21:16Z +- **Completed:** 2026-02-24T19:24:17Z +- **Tasks:** 2 +- **Files modified:** 3 + +## Accomplishments + +- Migration 013 creates `document_processing_events` table with `event_type` CHECK constraint, indexes on `created_at` and `document_id`, and RLS enabled — follows migration 012 pattern exactly +- `recordProcessingEvent()` has `void` return type (not `Promise`) and uses `void supabase.from(...).insert(...).then(callback)` to ensure errors are logged but never thrown, never blocking the processing pipeline +- `deleteProcessingEventsOlderThan()` computes cutoff via `Date.now() - days * 86400000`, deletes with `.lt('created_at', cutoff)`, returns `data.length` as row count +- 6 unit tests covering all exports: insert payload, void return type, error swallowing + logging, null coalescing for optional fields, cutoff date math, and row count return + +## Task Commits + +Each task was committed atomically: + +1. **Task 1: Create analytics migration and analyticsService** - `ef88541` (feat) +2. **Task 2: Create analyticsService unit tests** - `cf30811` (test) + +**Plan metadata:** (docs commit to follow) + +## Files Created/Modified + +- `backend/src/models/migrations/013_create_processing_events_table.sql` - document_processing_events DDL with UUID PK, CHECK constraint on event_type, indexes on created_at + document_id, RLS enabled +- `backend/src/services/analyticsService.ts` - recordProcessingEvent (void, fire-and-forget) and deleteProcessingEventsOlderThan (Promise) +- `backend/src/__tests__/unit/analyticsService.test.ts` - 6 unit tests using established makeSupabaseChain() pattern + +## Decisions Made + +- `recordProcessingEvent` return type is `void` (not `Promise`) — the type system itself prevents accidental `await`, matching the architecture decision in STATE.md ("Analytics writes are always fire-and-forget") +- Optional fields coalesce to `null` in the insert payload rather than omitting them — keeps the DB row shape consistent and predictable +- `created_at` is set explicitly in the insert payload (not via DB DEFAULT) to accurately reflect event occurrence time rather than DB write time + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 1 - Bug] Fixed toHaveProperty assertion on undefined return value** +- **Found during:** Task 2 (first test run) +- **Issue:** `expect(result).not.toHaveProperty('then')` throws `TypeError: Cannot convert undefined or null to object` when `result` is `undefined` — Vitest's `toHaveProperty` cannot introspect `undefined` +- **Fix:** Replaced with `expect(typeof result).toBe('undefined')` which correctly verifies the return is not a thenable without requiring the value to be an object +- **Files modified:** `backend/src/__tests__/unit/analyticsService.test.ts` +- **Verification:** All 6 tests pass after fix +- **Committed in:** cf30811 (Task 2 commit) + +--- + +**Total deviations:** 1 auto-fixed (Rule 1 — runtime error in test assertion) +**Impact on plan:** Fix required for tests to run. The replacement assertion is semantically equivalent and more idiomatic for checking void returns. + +## Issues Encountered + +- Vitest `toHaveProperty` throws on `undefined`/`null` values rather than returning false — use `typeof result` checks when verifying void returns instead. + +## User Setup Required + +None - no external service configuration required. + +## Next Phase Readiness + +- `analyticsService` is ready for callers — import `recordProcessingEvent` from `'../services/analyticsService'` and call it without `await` at instrumentation points +- Migration 013 SQL ready to run against Supabase (requires manual `psql` or Dashboard execution) +- `makeSupabaseChain()` pattern from Phase 1 confirmed working for service-layer tests (not just model-layer tests) +- Ready for Phase 2 plan 02: monitoring services that will call `recordProcessingEvent` during health probe lifecycle + +--- +*Phase: 02-backend-services* +*Completed: 2026-02-24* + +## Self-Check: PASSED + +- FOUND: backend/src/models/migrations/013_create_processing_events_table.sql +- FOUND: backend/src/services/analyticsService.ts +- FOUND: backend/src/__tests__/unit/analyticsService.test.ts +- FOUND: .planning/phases/02-backend-services/02-01-SUMMARY.md +- FOUND commit ef88541 (Task 1: analytics migration + service) +- FOUND commit cf30811 (Task 2: unit tests)