From 241c3cbf7a2ed8760d3320f58a20bfdf9d7a8b69 Mon Sep 17 00:00:00 2001 From: Adilislam0 Date: Fri, 19 Jun 2026 11:26:16 +0530 Subject: [PATCH 1/7] doc --- LPI_Goal_to_Recommendation_Flow.md | 418 ++++++++++++++ ..._Module2_Activity_Signals_Documentation.md | 511 ++++++++++++++++++ 2 files changed, 929 insertions(+) create mode 100644 LPI_Goal_to_Recommendation_Flow.md create mode 100644 Phase3_Module2_Activity_Signals_Documentation.md diff --git a/LPI_Goal_to_Recommendation_Flow.md b/LPI_Goal_to_Recommendation_Flow.md new file mode 100644 index 0000000..99b4c1b --- /dev/null +++ b/LPI_Goal_to_Recommendation_Flow.md @@ -0,0 +1,418 @@ +# LPI Platform: Goal → Activity Signal → Recommendation Integration Flow + +**Source repo:** `lpi-platform` (Life-Atlas org) · **Document scope:** Module 1 (Goals) → Module 2 (Activity Signals) → Module 3 (Recommendations) +**Verified against:** `src/lpi/`, `supabase/migrations/`, `scripts/ingest_github_events.py`, `tests/` + +This document traces a single piece of data — a user's goal — from the moment it's typed into a form to the moment a recommendation about it comes back out the other side. Every payload, endpoint, and schema snippet below is pulled directly from the current codebase, not idealized. Where the running code diverges from the diagram or from the intended design, that's called out explicitly rather than smoothed over — those gaps are exactly what doesn't show up in an architecture diagram. + +--- + +## 1. System Overview + +### 1.1 The three modules + +| Module | Router | Owner | Status in current codebase | +|---|---|---|---| +| Goals (Module 1) | `routers/goals.py` | Adil Islam (Phase 2) | Fully implemented — CRUD + SMILE phase tracking + composite scoring | +| Activity Signals (Module 2) | `routers/signals.py` | Adil Islam (Phase 3) | Fully implemented — ingest + filter + paginate, Supabase-backed | +| Recommendations (Module 3) | `routers/recommendations.py` | Jaivardhan Singh (Phase 4) | **Stub** — route is wired and returns `[]`; LangGraph reasoning not yet implemented | + +This matters for reading the rest of the document: Stages 1–6 below describe code that runs today. Stage 7 (the recommendation engine) describes the *designed* contract — the shape Module 2 was deliberately built to support — not yet a working implementation. That distinction is preserved throughout. + +### 1.2 Repo layout (the parts this flow touches) + +``` +lpi-platform/ +├── src/lpi/ +│ ├── models.py ← GoalCreate/Goal, SignalCreate/Signal, Recommendation +│ ├── smile.py ← 6-phase SMILE state machine +│ ├── scoring.py ← composite priority score +│ ├── store.py ← all Supabase reads/writes go through here +│ ├── middleware/auth.py ← Supabase JWT verification → user_id +│ └── routers/ +│ ├── goals.py +│ ├── signals.py +│ └── recommendations.py +├── scripts/ +│ └── ingest_github_events.py ← pulls GitHub Events API, POSTs signals +└── supabase/migrations/ + ├── 20260604000000_create_goals.sql + ├── 20260611000000_create_activity_signals.sql + ├── 20260607000000_create_log_tables.sql + └── 20260615000000_signals_rls_and_log_action.sql +``` + +There is **no `frontend/` directory in this repo**. The README states it directly: *"Frontend — Yet to be implemented by Jahanvi."* Section 4 of this document describes the integration contract the frontend needs to honor, derived from the API as it exists — not a description of code that was found, since none exists yet. + +--- + +## 2. Data Contracts + +These three Pydantic model pairs are the backbone of the whole flow. Each follows the same pattern: a `*Create` model defines what the caller sends, and the full model adds server-assigned fields on top. + +### 2.1 Goal + +```python +class GoalCreate(BaseModel): + title: str + description: str = "" + priority: int = 5 # 1 (low) – 10 (high) + smile_phase: SmilePhase = SmilePhase.REALITY_EMULATION + urgency_flag: bool = False # +0.20 to composite score when True + +class Goal(GoalCreate): + id: str # server-assigned uuid4 + user_id: str # from JWT `sub` claim + created_at: datetime + updated_at: datetime +``` + +### 2.2 Activity Signal + +```python +class SignalCreate(BaseModel): + stream: str # business domain: 'lpi', 'boardy', 'datapro', 'vsab'... + event_type: str # 'pr_merged', 'commit_pushed', 'match_created'... + payload: dict = {} # event-specific JSON, structure varies by event_type + source: str = "api" # 'github_api' | 'manual' | 'simulated' | 'api' + +class Signal(SignalCreate): + id: str + user_id: str + timestamp: datetime +``` + +**Important structural detail:** there is no `goal_id` field on `Signal`. Signals are *not* foreign-keyed to a specific goal at the database level. The only link between a goal and the signals that justify "progress" toward it is `user_id` plus whatever semantic correlation a reasoning layer performs at read time (matching `stream`, `payload.repo`, or text similarity against goal titles). This is a deliberate flexibility/strictness trade-off, covered in Section 6. + +### 2.3 Recommendation (target shape — not yet produced) + +```python +class Recommendation(BaseModel): + id: str + user_id: str + action: str + reasoning: str + smile_phase: SmilePhase + priority: float + source_goals: list[str] = [] # goal ids the recommendation references + source_signals: list[str] = [] # signal ids used as evidence + created_at: datetime +``` + +`source_goals` and `source_signals` are exactly how the goal↔signal link described above is meant to materialize — not as a database constraint, but as an output of the recommendation engine's reasoning step. + +--- + +## 3. End-to-End Flow + +This walks the diagram top to bottom, stage by stage, showing what enters, what the backend adds or transforms, and what exits. + +### Stage 1 — User creates a goal + +**Frontend → Backend:** +``` +POST /api/v1/goals/ +Authorization: Bearer +Content-Type: application/json + +{ + "title": "Ship Phase 3 activity signals", + "description": "Wire signal ingestion end-to-end before demo day", + "priority": 8, + "smile_phase": "contextual-intelligence", + "urgency_flag": true +} +``` + +**What `create_goal()` does (`routers/goals.py`):** +```python +new_goal = Goal( + id=str(uuid.uuid4()), + user_id=user_id, # injected by Depends(get_current_user) + created_at=now, updated_at=now, + **goal.model_dump(), # spreads title/description/priority/smile_phase/urgency_flag +) +store.insert_goal(new_goal) +log_user_activity(user_id=..., action="goal_created", resource_id=new_goal.id, metadata={...}) +``` + +| | Enters | Backend adds | Exits | +|---|---|---|---| +| Fields | `title, description, priority, smile_phase, urgency_flag` | `id` (uuid4), `user_id` (JWT `sub`), `created_at`, `updated_at` | Full `Goal` JSON, HTTP 201 | + +The `**goal.model_dump()` spread is why `urgency_flag` "just works" without any special-case code in the router — because `Goal` inherits from `GoalCreate`, any field added to the request model automatically flows through. + +### Stage 2 — Supabase `goals` table + +```sql +CREATE TABLE goals ( + id TEXT PRIMARY KEY, + user_id TEXT NOT NULL DEFAULT 'default_user', + title TEXT NOT NULL, + description TEXT NOT NULL DEFAULT '', + priority INTEGER NOT NULL DEFAULT 5, + smile_phase TEXT NOT NULL DEFAULT 'reality-emulation' + CHECK (smile_phase IN ('reality-emulation', 'concurrent-engineering', + 'collective-intelligence', 'contextual-intelligence', + 'continuous-intelligence', 'perpetual-wisdom')), + urgency_flag BOOLEAN NOT NULL DEFAULT FALSE, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); +``` +RLS (`20260612000000_goals_rls.sql`) restricts row visibility to `auth.uid()::text = user_id`. This is defense-in-depth only — the FastAPI backend writes using the **service-role key**, which bypasses RLS by design (`store.py::_get_client()`). RLS only matters if something queries Supabase directly (e.g. a future frontend using `supabase-js` against the table instead of going through the API). + +### Stage 3 — Work happens on the linked GitHub repository + +This is the step with no API call at all — a developer merges a PR or pushes a commit to `Life-Atlas/lpi-platform` on GitHub. GitHub itself records this as a public event, retrievable from `GET https://api.github.com/repos/{owner}/{repo}/events` (the **GitHub Events API**, not a webhook — see Section 6 for why that distinction matters). + +There is no automatic trigger here. Nothing in this repo subscribes to GitHub webhooks; ingestion is **pull-based and currently manual** (Stage 4 is a script someone runs, not a listener that fires on push). + +### Stage 4 — GitHub ingestion script converts events to signals + +`scripts/ingest_github_events.py` is invoked manually (`python scripts/ingest_github_events.py`) and does four things: + +1. **Fetch:** `GET /repos/Life-Atlas/lpi-platform/events` (last ~30 public events; 60 req/hr unauthenticated, 5000 req/hr with `GITHUB_TOKEN`). +2. **Filter + map:** only event types representing *completed* work are kept. + +```python +# PullRequestEvent → only if action == "closed" and pr["merged"] is True +{ + "stream": "lpi", "event_type": "pr_merged", "source": "github_api", + "payload": { + "repo": repo, "pr_number": pr["number"], "title": pr["title"], + "author": actor, "merged_at": pr["merged_at"], + "additions": pr["additions"], "deletions": pr["deletions"], + "changed_files": pr["changed_files"], + "body_snippet": pr["body"][:200], + }, +} +``` +Other mapped types: `PushEvent → commit_pushed`, `PullRequestReviewEvent → pr_reviewed`, `IssuesEvent → issue_closed` (closed only), `CreateEvent → branch_created` (branches only). Everything else (`WatchEvent`, `ForkEvent`, etc.) is dropped. + +3. **POST each mapped event:** +```python +def post_signal(signal_payload: dict) -> bool: + response = requests.post(f"{LPI_API_BASE}/api/v1/signals/", json=signal_payload, timeout=5) + return response.status_code in (200, 201) +``` + +| | Enters (per GitHub event) | Transform | Exits (per signal) | +|---|---|---|---| +| Source | Raw GitHub event JSON (`type`, `payload`, `actor`, `repo`) | Event-type-specific field extraction | `SignalCreate`-shaped dict, `source="github_api"` | + +> **Gap worth flagging:** `post_signal()` calls `requests.post()` with no `Authorization` header. Since `signals.py::ingest_signal` requires `Depends(get_current_user)`, running this script today against an auth-protected deployment returns `401 Unauthorized` for every event. The script was written before JWT auth was wired onto the signals router and hasn't been updated to attach a service token — this needs a fix before it can run against any environment with auth enabled. + +### Stage 5 — Signals router persists the event + +```python +new_signal = Signal( + id=str(uuid.uuid4()), + user_id=user_id, + timestamp=datetime.now(UTC), + **signal.model_dump(), # stream, event_type, payload, source +) +store.insert_signal(new_signal) + +try: + log_user_activity(user_id=user_id, action="signal_ingested", resource_id=new_signal.id, ...) +except Exception as exc: + print(f"[ingest_signal] WARNING: logging failed for signal {new_signal.id}: {exc}") +``` + +| | Enters | Backend adds | Exits | +|---|---|---|---| +| Fields | `stream, event_type, payload, source` | `id`, `user_id`, `timestamp` | Full `Signal` JSON, HTTP 201 | + +The `try/except` around logging exists because of a real schema bug: `user_activity_logs` originally had a `CHECK` constraint allowing only `'goal_created' | 'goal_updated' | 'goal_deleted'`. Calling `log_user_activity(action="signal_ingested")` against that constraint raises a Postgres exception. The signal still gets stored in `activity_signals` (the insert that matters), but the activity-log write silently fails and prints a warning instead of breaking the endpoint. + +The fix already exists as a migration — `20260615000000_signals_rls_and_log_action.sql` — but only takes effect once it's actually applied: +```sql +ALTER TABLE user_activity_logs DROP CONSTRAINT IF EXISTS user_activity_logs_action_check; +ALTER TABLE user_activity_logs ADD CONSTRAINT user_activity_logs_action_check + CHECK (action IN ('goal_created', 'goal_updated', 'goal_deleted', 'signal_ingested')); +``` +Until `supabase db push` runs this migration against the target environment, every signal ingestion logs a warning to stdout instead of recording a clean audit trail entry — functionally harmless (signals still land in the table), but it means `user_activity_logs` undercounts signal activity until applied. + +### Stage 6 — Supabase `activity_signals` table + +```sql +CREATE TABLE activity_signals ( + id TEXT PRIMARY KEY, + user_id TEXT NOT NULL DEFAULT 'default_user', + stream TEXT NOT NULL, + event_type TEXT NOT NULL, + payload JSONB NOT NULL DEFAULT '{}', + source TEXT NOT NULL DEFAULT 'api', + timestamp TIMESTAMPTZ NOT NULL DEFAULT NOW() +); +CREATE INDEX idx_as_user_id ON activity_signals (user_id); +CREATE INDEX idx_as_stream ON activity_signals (stream); +CREATE INDEX idx_as_event_type ON activity_signals (event_type); +CREATE INDEX idx_as_source ON activity_signals (source); +CREATE INDEX idx_as_timestamp ON activity_signals (timestamp DESC); +``` +No `CHECK` constraint on `stream` or `event_type` — intentionally. New business streams (`vsab`, `altiostar`, etc.) or new event types can be added without a migration. `source` is also unconstrained at the DB layer despite having well-known values (`github_api`, `manual`, `simulated`, `api`) — validation, if any, is application-level. + +### Stage 7 — Recommendation engine reads signals + goals *(designed, not yet built)* + +The intended read pattern, per the code's own docstrings (`signals.py`, `scoring.py`): +``` +GET /api/v1/signals/?source=github_api&limit=20 # exclude simulated/test signals +GET /api/v1/goals/ # already sorted by composite score +``` +`store.list_signals(source="github_api", ...)` runs a server-side `WHERE source = 'github_api'` — the rec engine never has to filter simulated data in Python. The composite goal score (Section 2.1's `priority/phase/urgency` weighting, computed in `scoring.py`) means goals are already ranked by importance before the rec engine even looks at signals. + +What's *not* built: the actual reasoning step that correlates a `pr_merged` signal with `payload.repo == "lpi-platform"` to a specific goal like "Ship Phase 3 activity signals," and produces `Recommendation.reasoning` text plus `source_goals`/`source_signals` references. `recommendations.py` today: +```python +@router.get("/{user_id}", response_model=list[Recommendation]) +def get_recommendations(user_id: str, limit: int = Query(default=3, ge=1, le=10)) -> list[Recommendation]: + return [] # Phase 4 placeholder — confirms the route is wired, nothing more +``` + +### Stage 8 — Recommendation surfaced to the frontend + +``` +GET /api/v1/recommendations/{user_id}?limit=3 +→ [] today, eventually: +[ + { + "action": "Open a PR closing the activity_signals CHECK constraint gap", + "reasoning": "3 github_api signals show signal-ingestion work in progress; this is the blocking issue before demo day.", + "smile_phase": "contextual-intelligence", + "priority": 6.8, + "source_goals": [""], + "source_signals": ["", ""] + } +] +``` + +--- + +## Scenario 1 — Production Flow (full lifecycle, all components operational) + +``` +┌──────────────┐ POST /api/v1/goals/ ┌─────────────────┐ +│ Frontend │ ─────────────────────────▶│ FastAPI goals │ +│ goal form │ GoalCreate JSON │ router │ +└──────────────┘ └────────┬────────┘ + │ insert_goal() + ▼ + ┌─────────────────┐ + │ Supabase: goals │ + └────────┬────────┘ + │ (informational — + │ no automated trigger) + ▼ +┌──────────────┐ PR merged / push ┌─────────────────┐ +│ GitHub repo │ ─────────────────────────▶│ GitHub Events │ +│ │ │ API (pull-based) │ +└──────────────┘ └────────┬────────┘ + │ python scripts/ingest_github_events.py + ▼ + ┌─────────────────┐ + │ ingestion script│ + │ maps event→signal│ + └────────┬────────┘ + │ POST /api/v1/signals/ + ▼ + ┌─────────────────┐ + │ signals router │ + │ ingest_signal() │ + └────────┬────────┘ + │ insert_signal() + ▼ + ┌─────────────────────┐ + │ Supabase: │ + │ activity_signals │ + └────────┬─────────────┘ + │ GET ?source=github_api + ▼ + ┌─────────────────────┐ + GET /api/v1/goals/ ───────▶│ Recommendation │ + (goals context) │ engine (Phase 4) │ + └────────┬─────────────┘ + │ GET /api/v1/recommendations/{user_id} + ▼ + ┌─────────────────┐ + │ Frontend: "next │ + │ step" + phase │ + └─────────────────┘ +``` + +**Concrete trace:** + +1. A user creates `Goal{title: "Ship Phase 3 activity signals", priority: 8, smile_phase: "contextual-intelligence", urgency_flag: true}`. Composite score = `8×0.5 + 4×0.3 + 1×0.2 = 5.40` — this goal now sorts near the top of `GET /api/v1/goals/`. +2. Over the next few days the team merges PR #18 ("Supabase dual-sync logging") into `lpi-platform`. +3. Someone runs `python scripts/ingest_github_events.py`. It fetches GitHub events, finds the merged PR, and POSTs: + ```json + {"stream": "lpi", "event_type": "pr_merged", "source": "github_api", + "payload": {"repo": "lpi-platform", "pr_number": 18, "title": "Supabase dual-sync logging", "author": "Adilislam0"}} + ``` +4. The signal lands in `activity_signals` with a server-assigned `id`, `user_id`, and `timestamp`. +5. The (future) recommendation engine queries `GET /api/v1/signals/?source=github_api&limit=20` and `GET /api/v1/goals/`, semantically matches the `pr_merged` signal's `payload.title` against the goal's title, and emits a `Recommendation` referencing both ids. +6. The frontend calls `GET /api/v1/recommendations/{user_id}` and renders: *"Recent merge confirms progress on 'Ship Phase 3 activity signals' — consider advancing to continuous-intelligence."* + +--- + +## Scenario 2 — Demo / MVP (3–5 day timeline) + +The full chain above has one component that doesn't exist yet (Stage 7's reasoning step) and one known bug blocking a clean demo (Stage 5's `CHECK` constraint, plus Stage 4's missing auth header). A 3–5 day MVP should **prove Stages 1–6 work end-to-end with real data**, and present the recommendation step as a thin, honest placeholder rather than trying to ship the LangGraph reasoning layer under time pressure. + +``` +Day 1 Day 2 Day 3 Day 4-5 +────── ────── ────── ─────── +Apply pending → Fix ingestion → Build minimal → Rehearse + +migration script auth + "Activity Feed" buffer +(supabase db run it against UI: list raw +push) so signal_ a real merged PR signals chrono- +ingested logging ologically, no +stops warning reasoning layer +``` + +**Day 1 — Unblock the data path** +- Run `supabase db push` to apply `20260615000000_signals_rls_and_log_action.sql`. This silences the `CHECK` constraint warning and gives `user_activity_logs` a clean record of `signal_ingested` events. +- Verify with: `SELECT * FROM user_activity_logs WHERE action = 'signal_ingested' ORDER BY logged_at DESC;` + +**Day 2 — Make the ingestion script actually work against auth** +- Add a service-role (or test-user) bearer token to `post_signal()`'s request headers. Without this, every POST from the script 401s the moment auth is enforced. +- Run the script against the real repo, confirm signals land: `GET /api/v1/signals/?source=github_api`. + +**Day 3 — Build the smallest honest UI** +- Skip Module 3 entirely for the demo. Instead of recommendations, build a simple "Recent Activity" panel that calls `GET /api/v1/signals/?stream=lpi&source=github_api&limit=10` and renders each row as `"{author} merged PR #{pr_number}: {title}"` or `"{author} pushed {commit_count} commit(s) to {branch}"`. This is real, verifiable data — not a simulated recommendation — which is a stronger demo artifact than a stubbed `[]` recommendations call or fabricated reasoning text. +- Pair it with the existing `GET /api/v1/goals/` list (already fully functional, already sorted by composite score) so the demo shows *goals* and *evidence of work* side by side, even without the connecting reasoning layer. + +**Day 4–5 — Buffer** +- Re-run the full test suite (`pytest tests/ -v`) against a freshly migrated local Supabase instance to catch any regression from the constraint fix. +- Rehearse the narrative: "here's the goal, here's the real GitHub activity feeding it, the recommendation layer that connects them is Phase 4 — in progress." + +**What this scenario deliberately excludes:** LangGraph reasoning, automatic goal-signal correlation, and webhook-based (push) ingestion. All three are real Phase 4 work, not a 3–5 day scope. + +--- + +## 4. Frontend Integration Contract + +No frontend code exists in this repo yet — this section describes the contract a frontend implementation needs to satisfy, derived from the API surface above, not a description of existing UI code. + +**Authentication.** Every endpoint except `/health` requires `Authorization: Bearer `. The README's stated pattern is `supabase-js`'s `auth.signUp` / `signInWithPassword`, with the resulting `access_token` attached to every backend call. `middleware/auth.py` accepts both HS256 (shared-secret) and ES256/RS256 (JWKS-verified) tokens depending on the Supabase project's signing configuration — the frontend doesn't need to know which; it just forwards whatever Supabase's client SDK issues. + +**Displaying goals.** `GET /api/v1/goals/` already returns results sorted by composite score (`sort_goals_by_score()` runs server-side) — the frontend should *not* re-sort client-side, or it'll fight the intended urgency/phase weighting. Optional filter: `?smile_phase=reality-emulation` to scope a view to one phase. There is currently no field on `Goal` linking it to a specific GitHub repo — "linked repositories" as a UI concept would need to be inferred from `payload.repo` on associated signals (matched by `stream`), since no structural link exists yet. Flag this for whoever owns the frontend goal-detail view. + +**Displaying activity signals.** `GET /api/v1/signals/?stream=lpi&source=github_api&limit=20` for a real-only, paginated activity feed. `source=github_api` specifically excludes `simulated` and `manual` test entries — useful for a "verified activity" view distinct from a raw/debug view. Pagination is `limit`/`offset`, not cursor-based; page 2 is `?offset=50` with the same `limit`. + +**Displaying recommendations.** `GET /api/v1/recommendations/{user_id}?limit=3` currently always returns `[]`. The frontend should build against the `Recommendation` schema (Section 2.3) now so that when Phase 4 ships, no contract changes are needed — but should render gracefully on an empty array rather than treating it as an error state. + +**Triggering ingestion.** There is no API endpoint or button-triggered action that runs `ingest_github_events.py` — it's a manually-invoked Python script today, not something the frontend can call. If a "Sync GitHub Activity" button is wanted in the UI, that script would need to be wrapped behind a new authenticated endpoint first; right now it only runs from a developer's terminal. + +--- + +## 5. Known Gaps & Architectural Decisions Summary + +| Item | Type | Detail | +|---|---|---| +| `user_activity_logs` CHECK constraint | Bug (fix exists, not yet applied) | Migration `20260615000000` adds `'signal_ingested'`; needs `supabase db push` | +| `ingest_github_events.py` missing auth header | Bug | `post_signal()` sends no `Authorization` header; will 401 against an auth-enforced deployment | +| No `goal_id` FK on `Signal` | Deliberate design | Correlation is meant to happen via reasoning (Phase 4), not a rigid foreign key — keeps signal ingestion source-agnostic | +| Recommendation engine returns `[]` | Known incomplete (Phase 4 in progress) | Route is wired so dependent code doesn't 500; reasoning logic not yet written | +| GitHub ingestion is pull/manual, not webhook-driven | Deliberate (for now) | Simpler to build and demo; no public endpoint needed for GitHub to call back to | +| RLS on `goals`/`activity_signals` | Defense-in-depth | Backend uses service-role key and bypasses RLS by design; RLS only protects against direct (non-backend) Supabase access | diff --git a/Phase3_Module2_Activity_Signals_Documentation.md b/Phase3_Module2_Activity_Signals_Documentation.md new file mode 100644 index 0000000..6d4f6c7 --- /dev/null +++ b/Phase3_Module2_Activity_Signals_Documentation.md @@ -0,0 +1,511 @@ +# Phase 3, Module 2: Activity Signals +## Technical Documentation — Data Ingestion, Implementation & Testing/Validation + +**Repo:** `lpi-platform` · **Owner:** Adil Islam · **QA:** Daksh Garg / Jaivardhan Singh · **Demo Day:** June 27, 2026 + +> This document is written directly against the current state of `lpi-platform` (commit history through `20260615000000_signals_rls_and_log_action.sql`), not an idealized spec. Where the running code diverges from the Module 2 gate sheet, that gap is called out explicitly rather than smoothed over — you need to know what's actually shippable vs. what's still aspirational before demo day. + +--- + +## 1. Phase Overview + +### 1.1 Purpose + +Activity Signals is the **evidence layer** of the LPI platform. A Goal (Phase 2) is a stated intention — *"I want to build a startup."* A Signal is proof of work toward it — *"Alice merged a PR."* Phase 4's recommendation engine (owned by Jaivardhan) reads Goals and Signals together to decide what a user should do next, give feedback, and watch for follow-up signals. Without this module, Phase 4 has goals with no evidence to reason against — it's a recommendation engine staring at an empty evidence locker. + +### 1.2 Gate Condition (from the Module 2 spec sheet) + +> **Gate:** API ingests simulated events. Activity timeline queryable by user and time range. Core path uses simulated data — real cross-stream data is bonus territory. + +The spec sheet is explicit about *why*: other streams (Boardy, DataPro, VSAB, Altiostar) won't have stable APIs until their own Phase 3–4, so cross-stream ingestion is architecturally a *bonus*, not a blocker. The module has to stand on its own with **simulated events + intern profile data**. + +Breaking the gate into testable conditions: + +| Gate clause | Testable as | +|---|---| +| API ingests simulated events | `POST /api/v1/signals/` accepts a payload with `source: "simulated"` and persists it | +| Timeline queryable by user | `GET /api/v1/signals/` returns only the authenticated caller's rows | +| Timeline queryable by time range | `GET /api/v1/signals/?start=...&end=...` filters by `timestamp` | +| Core path = simulated | A generator script produces realistic signals without depending on any external stream | + +The third row is flagged because, as covered in §4.3, **it is not yet implemented in the running code** — this is the single largest gap between the spec sheet and what currently ships. + +### 1.3 Downstream Relevance + +``` +Goals (Phase 2) ──┐ + ├──→ Recommendation Engine (Phase 4) ──→ Instructions, feedback, +Signals (Phase 3) ──┘ follow-up monitoring +``` + +Two design decisions in this module exist *specifically* to make Phase 4 easier later: + +1. **`source` is a first-class field, added now rather than in Phase 4.** It lets the recommendation engine later run `GET /signals/?source=github_api` to weight verified real activity above simulated test data, without a breaking schema migration. +2. **There is intentionally no `goal_id` foreign key on signals.** Correlating "this signal is evidence for that goal" is a Phase 4 reasoning problem (semantic matching between a goal's description and a signal's payload), not a database constraint. Locking it to an FK now would force every signal to map to exactly one goal at ingest time, which doesn't reflect reality — one commit can be evidence for two goals, or none yet. + +--- + +## 2. Data Model + +### 2.1 Supabase Schema + +```sql +-- supabase/migrations/20260611000000_create_activity_signals.sql + +CREATE TABLE IF NOT EXISTS activity_signals ( + id TEXT PRIMARY KEY, -- uuid4(), generated in Python + user_id TEXT NOT NULL DEFAULT 'default_user', + stream TEXT NOT NULL, -- WHICH domain: 'lpi', 'boardy', 'intern_proxy'... + event_type TEXT NOT NULL, -- WHAT happened: 'pr_merged', 'match_created'... + payload JSONB NOT NULL DEFAULT '{}', -- event-specific data, schema varies by event_type + source TEXT NOT NULL DEFAULT 'api', -- HOW it arrived: 'github_api'|'manual'|'simulated'|'api' + timestamp TIMESTAMPTZ NOT NULL DEFAULT NOW() +); + +CREATE INDEX IF NOT EXISTS idx_as_user_id ON activity_signals (user_id); +CREATE INDEX IF NOT EXISTS idx_as_stream ON activity_signals (stream); +CREATE INDEX IF NOT EXISTS idx_as_event_type ON activity_signals (event_type); +CREATE INDEX IF NOT EXISTS idx_as_source ON activity_signals (source); +CREATE INDEX IF NOT EXISTS idx_as_timestamp ON activity_signals (timestamp DESC); +``` + +**Why `stream` and `source` are separate columns** — this is the single most important modeling decision in the table, and it's easy to conflate the two: + +- `stream` = the **business domain** the event is *about* (lpi, boardy, datapro). This is what the recommendation engine groups by when it asks "what's happened in the area this goal cares about?" +- `source` = **how the signal got into the database** (github_api, simulated, manual, api). This is what the recommendation engine uses to *weight confidence* — a `github_api` PR merge is stronger evidence than a `simulated` test event, even if both have `stream: "lpi"`. + +Deliberately **no CHECK constraint** on either column. New streams (a future Boardy integration) or new event types onboard without a schema migration — the table grows with the product instead of gating it. The trade-off: nothing stops a typo (`"boardy"` vs `"boardy "`) from silently creating an orphan bucket; validation for known streams, if ever needed, belongs in the application layer, not the DB. + +**No RLS at table creation** — RLS was added three days later in a follow-up migration (§2.3) after the auth PR landed. Worth noting because it's a real example of schema evolving alongside the rest of the system rather than being designed perfectly up front. + +### 2.2 Pydantic Models + +```python +# src/lpi/models.py + +class SignalCreate(BaseModel): + """Request body for POST /api/v1/signals/.""" + stream: str + event_type: str + payload: dict = {} + source: str = "api" # defaults to 'api' so untagged callers don't break + + +class Signal(SignalCreate): + """Full signal — inherits SignalCreate fields + server-assigned ones.""" + id: str # uuid.uuid4(), assigned in the router, not by Postgres + user_id: str # from the JWT 'sub' claim via get_current_user() + timestamp: datetime # datetime.now(UTC), assigned in the router +``` + +`Signal` inheriting from `SignalCreate` (rather than redefining all five fields) means anything added to `SignalCreate` — like `source` was — automatically shows up in `Signal` and in the `**signal.model_dump()` spread used to build the full object in the router. One inheritance edge, one field addition, zero duplicated maintenance. + +### 2.3 RLS as Defense-in-Depth (not the primary gate) + +```sql +-- supabase/migrations/20260615000000_signals_rls_and_log_action.sql +ALTER TABLE activity_signals ENABLE ROW LEVEL SECURITY; + +CREATE POLICY "Users read own signals" ON activity_signals FOR SELECT USING (auth.uid()::text = user_id); +CREATE POLICY "Users insert own signals" ON activity_signals FOR INSERT WITH CHECK (auth.uid()::text = user_id); +CREATE POLICY "Users update own signals" ON activity_signals FOR UPDATE USING (auth.uid()::text = user_id); +CREATE POLICY "Users delete own signals" ON activity_signals FOR DELETE USING (auth.uid()::text = user_id); +``` + +It's worth being precise about what this actually protects against: the FastAPI backend connects with the **service-role key**, which bypasses RLS entirely. The real authorization boundary is `Depends(get_current_user)` in the router (§3.3). RLS here only matters if the frontend (or anything else) ever queries Supabase **directly**, skipping the FastAPI layer — in that scenario, RLS is the only thing stopping cross-user reads. Don't mistake "RLS is enabled" for "the backend is enforcing per-user isolation" — those are two separate mechanisms protecting two separate attack surfaces. + +--- + +## 3. Implementation + +### 3.1 Ingest Endpoint — `POST /api/v1/signals/` + +```python +# src/lpi/routers/signals.py + +@router.post("/", response_model=Signal, status_code=status.HTTP_201_CREATED) +def ingest_signal( + signal: SignalCreate, + user_id: str = Depends(get_current_user), # ① auth happens before the body even runs +) -> Signal: + now = datetime.now(UTC) + new_signal = Signal( + id=str(uuid.uuid4()), # ② server assigns the ID — caller never sets it + user_id=user_id, # ③ from the verified JWT, not from the request body + timestamp=now, + **signal.model_dump(), # ④ stream, event_type, payload, source from the validated body + ) + + store.insert_signal(new_signal) # ⑤ Supabase INSERT + + try: + log_user_activity( # ⑥ best-effort audit log — see §6.2 for the known issue here + user_id=user_id, action="signal_ingested", resource_id=new_signal.id, + metadata={"stream": new_signal.stream, "event_type": new_signal.event_type, "source": new_signal.source}, + ) + except Exception as exc: + print(f"[ingest_signal] WARNING: logging failed for signal {new_signal.id}: {exc}") + + return new_signal +``` + +Walking the validation chain on a single request: + +1. FastAPI deserializes the JSON body against `SignalCreate` *before* the function body runs. Missing `stream` or `event_type` → automatic `422 Unprocessable Entity`, no custom code needed. +2. `Depends(get_current_user)` resolves before the handler executes — an unauthenticated request never reaches the insert logic at all (§3.3). +3. **The `id`, `user_id`, and `timestamp` are never trusted from the client.** Even if a malicious caller puts `"user_id": "someone-else"` in the body, `SignalCreate` doesn't have a `user_id` field, so Pydantic silently drops it. This is the actual enforcement mechanism for "you can't write to another user's account" — it's not a runtime check, it's that the field doesn't exist on the input schema. +4. Logging is wrapped in `try/except` rather than allowed to propagate — see §6.2, this is currently masking a real schema bug rather than handling a genuinely optional side-effect. + +### 3.2 Simulated Event Generation + +This is the part of the gate the spec sheet calls the **core path**, and it's worth being direct about its current state: as of this writing, the simulated generator described in the Module 2 task table is a **design document, not yet a script**. The plan (`docs/aditi-proxy-data-plan.md`) defines the approach clearly: + +- Real Deri/cross-stream data is blocked, so **intern profile data** (each intern's stated 3-year goal, interests, and skills) stands in as proxy ground truth. +- Each intern's skills/interests become signals with `stream: "intern_proxy"`: + - one signal per skill → `event_type: "skill_demonstrated"`, `payload: {"skill": ""}` + - one signal per interest → `event_type: "interest_identified"`, `payload: {"interest": ""}` +- Each intern's stated goals become `Goal` rows so the recommendation engine has something to correlate the signals against. + +This is a genuinely better design than a naive random-event generator: it produces a feed that's internally consistent (an intern with `skills: ["python", "tensorflow"]` gets exactly those two `skill_demonstrated` signals, not arbitrary noise), which makes Phase 4's output far easier to sanity-check during a demo than fully synthetic data would be. The implementation gap is real, though — until that script lands, the "simulated event generator" row on the gate sheet is plan, not proof. Pseudocode for the conversion, matching the documented plan: + +```python +# Planned: scripts/generate_intern_signals.py (not yet implemented) + +def intern_to_signals(intern: dict) -> list[dict]: + """Convert one intern profile into SignalCreate-shaped dicts.""" + signals = [] + for skill in intern["skills"]: + signals.append({ + "stream": "intern_proxy", + "event_type": "skill_demonstrated", + "payload": {"skill": skill}, + "source": "simulated", + }) + for interest in intern["interests"]: + signals.append({ + "stream": "intern_proxy", + "event_type": "interest_identified", + "payload": {"interest": interest}, + "source": "simulated", + }) + return signals + +# Then for each generated dict: requests.post(f"{API_BASE}/api/v1/signals/", json=signal, headers=auth_header) +``` + +### 3.3 Auth Middleware + +```python +# src/lpi/middleware/auth.py + +def get_current_user(credentials: HTTPAuthorizationCredentials | None = Depends(bearer_scheme)) -> str: + if credentials is None: + raise HTTPException(status_code=401, detail="Missing bearer token") + + token = credentials.credentials + alg = jwt.get_unverified_header(token).get("alg") + + if alg == "HS256": + key, algorithms = settings.supabase_jwt_secret, ["HS256"] # legacy Supabase projects + else: + signing_key = _get_jwks_client().get_signing_key_from_jwt(token) + key, algorithms = signing_key.key, ["ES256", "RS256"] # current Supabase projects, via JWKS + + payload = jwt.decode(token, key, algorithms=algorithms, audience="authenticated") + return payload["sub"] # the Supabase auth user's UUID +``` + +This is **real JWT verification**, not a stub — it's the upgrade from the Phase 2 `"default_user"` placeholder. Two things worth understanding: + +- It branches on the token's own `alg` header because Supabase signs tokens differently depending on project/CLI version (HS256 shared-secret on older projects, ES256/RS256 asymmetric on current ones, verified against the project's JWKS endpoint). Hardcoding one algorithm would silently break for whichever project type wasn't tested. +- `audience="authenticated"` is a deliberate check, not boilerplate — it rejects tokens issued for a different audience (e.g., a service-role token), so a leaked admin credential of the wrong type doesn't accidentally pass as a regular user. + +### 3.4 Rate Limiting + +```python +# src/lpi/middleware/rate_limit.py — fixed-window, per client IP, in-memory + +_HEALTH_LIMIT, _WRITE_LIMIT, _READ_LIMIT = 120, 30, 60 # requests per 60s window + +class RateLimitMiddleware(BaseHTTPMiddleware): + async def dispatch(self, request, call_next): + if request.method == "OPTIONS": + return await call_next(request) # CORS preflight is exempt + + limit_type, limit = ( + ("health", _HEALTH_LIMIT) if request.url.path == "/health" else + ("write", _WRITE_LIMIT) if request.method in {"POST", "PATCH", "PUT", "DELETE"} else + ("read", _READ_LIMIT) + ) + allowed, retry_after = _check(_client_ip(request), limit_type, limit) + if not allowed: + return JSONResponse(status_code=429, content={"detail": "Rate limit exceeded. Please slow down."}, + headers={"Retry-After": str(retry_after)}) + return await call_next(request) +``` + +`POST /signals/` falls under the **write** bucket (30/min/IP) — meaningfully tighter than reads, because writes are the expensive side (DB insert + audit log) and the side most exposed to a runaway ingestion script hammering the endpoint in a loop. The known limitation, called out directly in the module's own docstring: state is an in-memory dict, single-process. It works correctly for the demo's single-instance deployment; it silently stops being a real limit the moment the service runs behind more than one worker process, since each worker would track its own counters. That's a Redis-backed-counter problem for whenever the platform scales past one process — not a Phase 3 blocker, but worth flagging now so it doesn't get rediscovered the hard way under load. + +### 3.5 External (Bonus) Ingestion — GitHub Events + +```python +# scripts/ingest_github_events.py + +def map_github_event(event: dict) -> dict | None: + """GitHub raw event → SignalCreate dict. Returns None for events we don't care about.""" + event_type = event.get("type", "") + payload = event.get("payload", {}) + + if event_type == "PullRequestEvent": + pr = payload.get("pull_request", {}) + if payload.get("action") == "closed" and pr.get("merged", False): # only MERGED PRs count + return { + "stream": "lpi", "event_type": "pr_merged", "source": "github_api", + "payload": {"repo": event["repo"]["name"], "pr_number": pr.get("number"), + "title": pr.get("title", ""), "author": event["actor"]["login"]}, + } + return None + # ... PushEvent → commit_pushed, PullRequestReviewEvent → pr_reviewed, + # IssuesEvent (closed only) → issue_closed, CreateEvent (branch) → branch_created +``` + +This is the bonus path the gate sheet describes as "real cross-stream data... Boardy match events preferred." For demo day, real GitHub activity on `lpi-platform` itself stands in as the first real (non-simulated) stream, tagged `source: "github_api"` so it's distinguishable from `intern_proxy` simulated signals at query time. + +**A real bug to fix before this script is demo-safe:** `post_signal()` POSTs with no `Authorization` header at all: + +```python +def post_signal(signal_payload: dict) -> bool: + url = f"{LPI_API_BASE}/api/v1/signals/" + response = requests.post(url, json=signal_payload, timeout=5) # ← no Authorization header +``` + +This worked fine against the Phase 2 `"default_user"` stub, but now that `ingest_signal` requires `Depends(get_current_user)`, every call from this script will 401 against an auth-enforced deployment. Fix is small (mint or reuse a service-account JWT and pass it as a bearer header) but it has to happen before this script is run against staging — right now it would *appear* to work locally only because local dev may still be running against an unenforced auth state, which is exactly the kind of gap that surfaces as a surprise during a live demo. + +--- + +## 4. Query Layer — `GET /api/v1/signals/` + +```python +@router.get("/", response_model=list[Signal]) +def list_signals( + stream: str | None = Query(default=None), + event_type: str | None = Query(default=None), + source: str | None = Query(default=None), + limit: int = Query(default=50, ge=1, le=200), + offset: int = Query(default=0, ge=0), + user_id: str = Depends(get_current_user), +) -> list[Signal]: + return store.list_signals(user_id=user_id, stream=stream, event_type=event_type, + source=source, limit=limit, offset=offset) +``` + +```python +# src/lpi/store.py + +def list_signals(user_id=None, stream=None, event_type=None, source=None, limit=50, offset=0) -> list[Signal]: + query = _get_client().table("activity_signals").select("*") + if user_id: query = query.eq("user_id", user_id) + if stream: query = query.eq("stream", stream) + if event_type: query = query.eq("event_type", event_type) + if source: query = query.eq("source", source) + query = query.order("timestamp", desc=True).limit(limit).offset(offset) + return [Signal(**row) for row in query.execute().data] +``` + +### 4.1 Why filtering is server-side, not client-side + +```python +# BAD — fetches every row, filters in Python +all_rows = query.execute().data +return [s for s in all_rows if s["stream"] == stream] + +# GOOD — Postgres only sends back matching rows; this is what the code does +query = query.eq("stream", stream) +``` + +At 50 signals the difference is invisible. At 10,000 signals, client-side filtering pulls all 10,000 rows over the network to discard 9,950 of them; server-side filtering, backed by `idx_as_stream`, returns exactly the 50 that match in roughly logarithmic time. This is the entire reason the migration adds one index per filterable column (§2.1) — an index without a corresponding `.eq()` filter in the store is dead weight, and a filter without a matching index is a full table scan waiting to happen at scale. + +### 4.2 Pagination + +`limit` and `offset` translate directly to SQL `LIMIT`/`OFFSET`. Querying page 2 of a `stream=boardy` filter (`?stream=boardy&limit=50&offset=50`) costs the same as page 1 regardless of total row count — Postgres never reads rows outside the requested window. The `idx_as_timestamp DESC` index means "sort newest-first" is also free; Postgres doesn't sort after fetching, it reads the index in its existing order. + +### 4.3 Gap: Time-Range Filtering Is Not Yet Implemented + +This is the most important discrepancy to flag against the gate sheet. The spec table lists *"Query endpoints: by user, by stream, by time range, by event type"* with done-state *"Flexible filtering, paginated results."* The running `list_signals` accepts `stream`, `event_type`, `source`, `limit`, `offset` — **there is no `start`/`end` query parameter, and `store.list_signals()` has no corresponding `.gte()`/`.lte()` call on `timestamp`.** The `idx_as_timestamp` index exists and would make a range filter fast once added, but the filter itself isn't wired up. + +Concretely, this is what's missing and what closing it looks like: + +```python +# Needed addition to GET /api/v1/signals/ +start: datetime | None = Query(default=None, description="ISO-8601, inclusive lower bound"), +end: datetime | None = Query(default=None, description="ISO-8601, inclusive upper bound"), + +# Needed addition to store.list_signals() +if start: query = query.gte("timestamp", start.isoformat()) +if end: query = query.lte("timestamp", end.isoformat()) +``` + +Given the gate sheet explicitly names "queryable by user and time range" as the success condition (not just by stream/event_type), this is a pre-demo blocker, not a nice-to-have — the timeline view (Jahanvi's frontend task) will likely want a "last 7 days" or "last 24 hours" view, which has no endpoint to call without this. + +### 4.4 Bonus Endpoint — Single Signal Lookup + +```python +@router.get("/{signal_id}", response_model=Signal) +def get_signal(signal_id: str, user_id: str = Depends(get_current_user)) -> Signal: + signal = store.get_signal(signal_id) + if signal is None or signal.user_id != user_id: + raise HTTPException(status_code=404, detail=f"Signal '{signal_id}' not found.") + return signal +``` + +Note the **404, not 403**, when the signal belongs to someone else. This mirrors the pattern already established in `goals.py`: returning 403 ("forbidden") leaks the fact that a resource with that ID exists at all; 404 keeps that information from a caller probing IDs they don't own. Small detail, but it's the kind of consistency worth preserving as new endpoints get added in Phase 4. + +--- + +## 5. Execution Flow + +```mermaid +sequenceDiagram + participant Gen as Simulated Generator
(intern_proxy) / GitHub Script + participant API as FastAPI
POST /api/v1/signals/ + participant Auth as Auth Middleware
(JWT verify) + participant RL as Rate Limiter
(30 writes/min/IP) + participant Store as store.insert_signal() + participant DB as Supabase
activity_signals + participant Log as user_activity_logs
(best-effort) + participant Q as GET /api/v1/signals/
(stream/event_type/source filters) + participant FE as Frontend Timeline
(not yet built) + + Gen->>API: POST {stream, event_type, payload, source} + API->>Auth: verify Bearer JWT + Auth-->>API: user_id (sub claim) + API->>RL: check write-bucket counter + RL-->>API: allowed + API->>API: build Signal(id=uuid4(), user_id, timestamp=now) + API->>Store: insert_signal(signal) + Store->>DB: INSERT INTO activity_signals + DB-->>Store: ack + API->>Log: log_user_activity(action="signal_ingested") + Note over Log: try/except — CHECK constraint
fix exists but unapplied (§6.1) + API-->>Gen: 201 Created, full Signal JSON + + FE->>Q: GET ?stream=...&event_type=...&source=... + Q->>Auth: verify Bearer JWT + Auth-->>Q: user_id + Q->>Store: list_signals(user_id, filters, limit, offset) + Store->>DB: SELECT ... WHERE user_id=? AND filters ORDER BY timestamp DESC LIMIT/OFFSET + DB-->>Store: matching rows + Store-->>Q: list[Signal] + Q-->>FE: 200, JSON array + Note over FE: Chronological scrollable feed —
component does not exist in repo yet +``` + +**Step-by-step, in prose:** + +1. **Generation** — either the (planned) intern-proxy script or the GitHub events script produces a `SignalCreate`-shaped payload and POSTs it. +2. **Auth gate** — `get_current_user` runs before any business logic. A bad or missing token never reaches the database. +3. **Rate gate** — the write-bucket counter is checked next; a script in a tight retry loop gets throttled with a `429` + `Retry-After` rather than hammering Supabase. +4. **Server-assigned fields** — `id`, `user_id`, `timestamp` are stamped in the router, never trusted from the caller. +5. **Persistence** — one `INSERT` into `activity_signals`, with the indexes from §2.1 already in place to make future reads fast. +6. **Audit log (best-effort)** — wrapped in `try/except` so a logging failure never blocks ingestion (currently masking the issue in §6.1, not just being defensive). +7. **Query side** — the frontend (or, eventually, Phase 4) calls `GET /signals/` with whatever filters it needs; everything happens server-side in one round trip. +8. **Frontend timeline** — this is where the flow currently ends on paper. The component itself isn't in the repo yet (Jahanvi's deliverable); the backend contract it would consume is otherwise ready, modulo §4.3. + +--- + +## 6. Testing & Validation Strategy + +### 6.1 Test Infrastructure + +```python +# tests/conftest.py — the pattern every signals test relies on + +@pytest.fixture(autouse=True) +def clear_store() -> Generator[None, None, None]: + if not _supabase_available(): + pytest.skip("Local Supabase is not running. Start it with `supabase start`.") + store.clear_all() + clear_all_logs() + yield + store.clear_all() + clear_all_logs() + +@pytest.fixture +def client() -> TestClient: + test_client = TestClient(app) + test_client.headers.update({"Authorization": f"Bearer {_make_token()}"}) # real JWT, fixed test secret + return test_client +``` + +Two things make this a solid foundation rather than a brittle one: + +- **`autouse=True` on `clear_store`** means every test gets a clean table without remembering to call a cleanup helper — order-dependent test pollution (a classic flaky-suite cause) is structurally prevented. +- **The `client` fixture issues a real HS256 JWT** signed with a fixed test secret (`monkeypatch`-injected into `settings.supabase_jwt_secret`), rather than mocking `get_current_user` away. This means the auth middleware's actual decode/verify path runs in every test — a regression in JWT handling would be caught by the existing signal tests, not just a dedicated auth test file. + +### 6.2 Current Coverage — `tests/test_activity_signals.py` + +| Test | Validates | +|---|---| +| `test_ingest_returns_signal` | 201 status, server-assigned `id`/`timestamp`/`user_id`, `source` defaults to `"api"` | +| `test_ingest_with_explicit_source` | explicit `source` is preserved, not overwritten by the default | +| `test_ingest_from_different_streams` | no stream allowlist — `boardy`, `datapro`, `vsab`, `altiostar`, `security` all accepted | +| `test_list_signals_empty` | clean store → `[]`, not stale data from a prior test | +| `test_list_signals` | results scoped to the authenticated `user_id` — no cross-user leakage | +| `test_filter_by_stream` | `?stream=boardy` excludes a `datapro` signal inserted in the same test | +| `test_filter_by_source` | `?source=github_api` excludes a `simulated` signal — the exact filter Phase 4 needs | + +### 6.3 Coverage Gaps to Close Before the Gate Is Truly Met + +- **No `event_type` filter test**, even though the router exposes the parameter and the migration indexes it. Trivial to add by mirroring `test_filter_by_stream`. +- **No time-range test** — can't exist yet because the feature itself doesn't exist (§4.3). Once the `start`/`end` params are added, the test should insert signals with manually-set or mocked timestamps spanning a boundary and assert the boundary is inclusive/exclusive as documented. +- **No pagination test** — nothing currently asserts that `limit`/`offset` actually bound the result set (e.g., insert 60 signals, `limit=50`, assert exactly 50 come back and the 51st appears on `offset=50`). +- **No unauthenticated-request test specific to signals** — `test_rate_limit.py` exercises rate limiting, but there's no `test_activity_signals.py` case asserting a missing/invalid bearer token returns 401 *for this router specifically* (as opposed to relying on shared middleware tests elsewhere). +- **No test for the logging side-effect path** — given §6.1's known CHECK-constraint issue, a test that intentionally exercises the `signal_ingested` log write (rather than relying on the `try/except` to silently swallow it) would have caught the bug before it shipped to a migration fix. + +### 6.4 The Known Blocker (Already Diagnosed, Not Yet Applied) + +```sql +-- Already written in 20260615000000_signals_rls_and_log_action.sql, NOT yet pushed: +ALTER TABLE user_activity_logs DROP CONSTRAINT IF EXISTS user_activity_logs_action_check; +ALTER TABLE user_activity_logs ADD CONSTRAINT user_activity_logs_action_check + CHECK (action IN ('goal_created', 'goal_updated', 'goal_deleted', 'signal_ingested')); +``` + +The original `user_activity_logs` CHECK constraint (from the Phase 2 logging migration) only permits `goal_created | goal_updated | goal_deleted`. Every `signal_ingested` log write currently violates that constraint and is silently caught by the `try/except` in `ingest_signal` — meaning **the audit trail for signal ingestion does not currently exist in the database**, even though the ingest endpoint itself works correctly. The fix is already written; it just needs `supabase db push` run against the target environment before this is considered closed. This is exactly the kind of "endpoint works, but a downstream side-effect silently fails" bug that integration tests against a real local Supabase instance (as this suite already does) are positioned to catch, once a test specifically exercises the log write rather than letting the try/except absorb it. + +--- + +## 7. Completion Checklist + +| Deliverable | Owner | Status | Notes | +|---|---|---|---| +| Activity signal model (schema + Pydantic) | Aditi | ✅ Done | `activity_signals` table + `SignalCreate`/`Signal` match the spec exactly | +| Ingest endpoint with validation | Adil | ✅ Done | Pydantic validation, JWT auth, server-assigned fields all in place | +| Simulated event generator (all 3 module types) | Aditi | 🟡 Planned, not coded | `docs/aditi-proxy-data-plan.md` defines the intern-proxy approach; script not yet written | +| Query endpoints — by user | Adil | ✅ Done | enforced via `Depends(get_current_user)` scoping, not optional | +| Query endpoints — by stream | Adil | ✅ Done | server-side `.eq()`, indexed | +| Query endpoints — by event type | Adil | ✅ Done | server-side `.eq()`, indexed, but no dedicated test yet | +| Query endpoints — by time range | Adil | 🔴 Not started | no `start`/`end` params on the router or store — see §4.3 | +| Query endpoints — pagination | Adil | ✅ Done | `limit`/`offset`, capped at 200, but untested | +| Timeline view (frontend) | Jahanvi | 🔴 Not started | no frontend code in the `lpi-platform` repo yet | +| Auth middleware on all endpoints | Jaivardhan | ✅ Done | real Supabase JWT verification (HS256 + ES256/RS256 via JWKS) | +| Rate limiting on all endpoints | Jaivardhan | ✅ Done | fixed-window per-IP, 30 writes/60 reads per minute | +| Webhook/polling design doc | Aditi | ⚪ Not reviewed in this pass | not located under `docs/` as of this writing | +| Unit + integration tests | Yashika | 🟡 Partial | ingest + stream/source filtering covered; event_type, pagination, time-range, auth-failure not yet covered | +| **Bonus:** real event data from another stream | Adil | 🟡 Partial, blocked | GitHub events script works end-to-end *except* it sends no auth header (§3.5) — will 401 once run against an auth-enforced deployment | + +✅ Done · 🟡 Partial / in progress · 🔴 Not started · ⚪ Unverified in this review + +--- + +## 8. Key Considerations + +**Why simulated data is the right core path, not a shortcut.** Every other stream (Boardy, DataPro, VSAB, Altiostar) is on its own Phase 3–4 timeline — their APIs are explicitly described as unstable until then. Gating Module 2's success on real cross-stream integration would make this module's demo-readiness hostage to four other teams' schedules. The `source` field (§2.1) is the architectural hedge that makes this safe: simulated and real signals share one schema, one ingest path, and one query surface. When a real stream does come online, nothing about the ingestion pipeline changes — only the `source` value on incoming payloads does. This is the textbook case for designing the seam before you have both sides of it. + +**Risk mitigation for unstable external APIs.** The GitHub ingestion script (§3.5) is the current external dependency, and it's structured defensively: GitHub API failures (`404`, `403` rate-limit) cause a clean exit with a printed cause rather than a stack trace, and each signal POST is wrapped so one failed insert doesn't abort the batch — the script reports a final `succeeded/failed` count and keeps going. The one gap that *isn't* defensive yet is the missing auth header (§3.5) — that's not a "what if GitHub is flaky" risk, it's a guaranteed failure the moment auth enforcement is live in the target environment, and it should be fixed before the script is pointed at anything but a local, auth-disabled instance. + +**How the signal structure supports downstream goals analysis.** The lack of a `goal_id` FK (§1.3) is a deliberate bet that correlation belongs in Phase 4's reasoning layer, not the schema. The cost of that bet is that `recommendations.py` currently returns `[]` unconditionally — the correlation logic doesn't exist yet, so there's no way to verify in this phase that the bet pays off as intended. What *is* verifiable now: the `stream`/`event_type`/`source` triple gives Phase 4 enough to query "all real LPI-stream signals from the last week" without needing the FK at all, which is the access pattern the recommendation engine actually needs first (recent evidence in a domain) before it needs the finer-grained "evidence for this specific goal" correlation. + +--- + +*Reviewed against the running `lpi-platform` codebase (models.py, store.py, routers/signals.py, middleware/auth.py, middleware/rate_limit.py, scripts/ingest_github_events.py, tests/test_activity_signals.py, and the four signals-related migrations) rather than written from the spec alone. The three items most worth fixing before demo day, in priority order: (1) wire up time-range filtering — it's named explicitly in the gate condition; (2) run `supabase db push` to apply the already-written CHECK constraint fix; (3) add an auth header to `ingest_github_events.py` before running it against anything but local/unauthenticated dev.* From 672f529725746a6fdbced2eb0876b7c381ecef96 Mon Sep 17 00:00:00 2001 From: Adilislam0 Date: Fri, 19 Jun 2026 13:33:22 +0530 Subject: [PATCH 2/7] Create Phase4_Module3_Recommendation_Engine_Plan.md --- Phase4_Module3_Recommendation_Engine_Plan.md | 490 +++++++++++++++++++ 1 file changed, 490 insertions(+) create mode 100644 Phase4_Module3_Recommendation_Engine_Plan.md diff --git a/Phase4_Module3_Recommendation_Engine_Plan.md b/Phase4_Module3_Recommendation_Engine_Plan.md new file mode 100644 index 0000000..8d6f162 --- /dev/null +++ b/Phase4_Module3_Recommendation_Engine_Plan.md @@ -0,0 +1,490 @@ +# Phase 4, Module 3: Recommendation Engine +## Implementation Plan — Data Model, Build Sequence & Testing/Validation + +**Repo:** `lpi-platform` · **Owners:** Jaivardhan (algorithm + reasoning) · Adil (endpoint) · Jahanvi (frontend) · Daksh (agent orchestration) · Yashika (feedback + tests) + +> **Status: not started.** Unlike the Phase 3 document, this is not a description of running code — `recommendations.py` is still the Phase 1/2 stub (`return []`), and `test_recommendations.py` is fully skipped. This document is a build plan: what has to be created, in what order, and — critically — using which pieces Phase 2 and Phase 3 already built so this phase isn't starting from zero. Every "how to build it" section below names the actual existing function it should call rather than describing a new one to write from scratch. + +--- + +## 1. Phase Overview + +### 1.1 Purpose + +Phase 4 is where the platform stops being a system of record (goals + signals) and becomes a system of **advice**. Given a user's Goals (Phase 2) and Activity Signals (Phase 3), the engine has to surface three concrete next actions, explain *why* each one matters in SMILE terms, and learn from whether the user accepts or dismisses them. + +### 1.2 Gate Condition + +> **Gate:** Given a user's goals + activity signals → system suggests 3 next actions with SMILE-based reasoning. Frontend shows recommendation cards. Daksh's agent orchestration pipeline handles multi-step queries. + +> **Bonus:** LLM-powered explanations (not just template-based), with per-call cost tracked. $10/day cap enforced in code. + +Breaking the gate into testable conditions, the same way the Phase 3 document did: + +| Gate clause | Testable as | +|---|---| +| Suggests 3 next actions | `GET /recommendations/{user_id}` returns a list of length ≤ 3, sorted by relevance | +| SMILE-based reasoning, specific not generic | Each `reasoning` string names the actual goal title and/or signal evidence it's based on — not a templated sentence with no per-user detail | +| Frontend shows recommendation cards | A card component renders `action`, `reasoning`, `smile_phase`, `priority` per item | +| Agent orchestration handles multi-step queries | A separate orchestration layer can chain goals→signals→actions for queries broader than "give me 3 actions" (e.g. "what should I focus on across all my goals this week") | +| Feedback loop | Accept/dismiss is persisted and measurably changes future output for that user | + +### 1.3 Direct Continuity From Phase 3 + +This phase inherits two unresolved items from the Phase 3 document rather than starting clean, and both block specific Phase 4 tasks: + +1. **Time-range query filtering on signals does not exist yet** (Phase 3 doc, §4.3). The recommendation algorithm's most natural question — *"what has this user done recently?"* — has no server-side way to ask "signals from the last 7 days." Closing that gap is now a **Phase 4 prerequisite**, not just a Phase 3 nice-to-have, because §4.1 below depends on it directly. +2. **The `recommendations.py` stub currently has no auth dependency at all** — `get_recommendations(user_id: str, limit: int = ...)` takes `user_id` as a path parameter with no `Depends(get_current_user)` anywhere. Once this returns real data instead of `[]`, that's an open endpoint serving anyone's recommendations to anyone who guesses their UUID. This has to be fixed as part of the endpoint work in §4.3, not treated as a separate hardening pass later. + +--- + +## 2. What Already Exists vs. What Must Be Built + +This is the most important table in this document — it's the difference between a 4-week build and a 1-week build. + +| Already exists (reuse, don't rewrite) | Where | What's missing (must build) | +|---|---|---| +| `Recommendation` Pydantic model — `id, user_id, action, reasoning, smile_phase, priority, source_goals, source_signals, created_at` | `models.py` | A `store.insert_recommendation()` / `list_recommendations()` pair — recommendations are currently never persisted anywhere | +| SMILE-weighted goal scoring (`score_goal`, `score_explanation`, `sort_goals_by_score`) | `scoring.py` | Wiring this into a candidate-action generator — it currently only ranks goals, not actions | +| SMILE phase descriptions + key questions (`get_phase_description`, `get_phase_key_question`) | `smile.py` | Using these as the actual text source for "specific, not generic" reasoning, instead of writing new copy | +| Forward-one-step phase transition validation (`validate_phase_transition`) | `smile.py` | Reusing this to decide *when* "move to the next SMILE phase" is a valid recommendation vs. a skip | +| `store.list_goals(user_id, smile_phase)` and `store.list_signals(user_id, stream, event_type, source, limit, offset)` | `store.py` | A combined fetch step that pulls both per request, and a workaround for the missing time-range filter (§4.1) | +| `Recommendation.limit` query param already shaped for "top N" (`default=3, ge=1, le=10`) on the route stub | `routers/recommendations.py` | Auth dependency, real body, and persistence | +| `llm_provider`, `llm_model`, `daily_cost_cap_usd=10.0` settings fields | `config.py` | **These exist but are read by nothing.** No LLM client, no cost tracker, no enforcement — the bonus's hardest requirement is a config placeholder today | +| `X-Process-Time` response header, added specifically — per its own docstring — "to verify the <3s recommendation target" | `middleware/__init__.py` | An actual recommendation endpoint to measure; the SLA is already implied in the infra, it just has nothing to time yet | +| Dual-sync logging pattern (`log_user_activity`, `log_system_event`) — Supabase + in-memory, try/except wrapped | `utils/logging.py` | A fourth log type, or reuse of `log_user_activity`, for feedback events (§4.6) | +| Skipped test skeleton naming the exact gate behaviors to verify | `tests/test_recommendations.py` | The `phase_gate_enabled` fixture these tests reference **does not exist in `conftest.py` today** — un-skipping them will fail on a missing fixture until that's added (§6.1) | + +--- + +## 3. Data Model Additions Needed + +### 3.1 Recommendation — Already Modeled, Needs Persistence + +```python +# models.py — already exists, no changes needed +class Recommendation(BaseModel): + id: str + user_id: str + action: str + reasoning: str + smile_phase: SmilePhase + priority: float + source_goals: list[str] = [] + source_signals: list[str] = [] + created_at: datetime +``` + +The model is gate-ready as written. What's missing is a place for it to live. Right now there is no `recommendations` table and no `store.insert_recommendation()` — every call to the endpoint would have to recompute from scratch with no record of what was shown before. That's a problem the moment feedback enters the picture: §4.6 needs to say "the user dismissed *recommendation X*," which requires recommendation X to have a durable `id` a feedback row can point to. **Decision to make explicitly before building:** persist every generated recommendation (cheap — it's a small JSON row) rather than trying to make feedback reference an ephemeral, never-stored object. + +```sql +-- New migration needed: 2026MMDD000000_create_recommendations.sql +CREATE TABLE IF NOT EXISTS recommendations ( + id TEXT PRIMARY KEY, + user_id TEXT NOT NULL, + action TEXT NOT NULL, + reasoning TEXT NOT NULL, + smile_phase TEXT NOT NULL, + priority DOUBLE PRECISION NOT NULL, + source_goals JSONB NOT NULL DEFAULT '[]', + source_signals JSONB NOT NULL DEFAULT '[]', + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); +CREATE INDEX IF NOT EXISTS idx_rec_user_id ON recommendations (user_id); +CREATE INDEX IF NOT EXISTS idx_rec_created_at ON recommendations (created_at DESC); +``` + +Mirrors the `activity_signals` migration pattern exactly (§2.1 of the Phase 3 document) — same column-naming convention, same one-index-per-filter-column approach, same `TEXT` primary key generated in Python. + +### 3.2 Feedback — Net New + +The gate names this explicitly: *"accept/dismiss recommendation → influences future suggestions... Feedback stored, affects next recommendations."* + +```sql +-- New migration: 2026MMDD000001_create_recommendation_feedback.sql +CREATE TABLE IF NOT EXISTS recommendation_feedback ( + id TEXT PRIMARY KEY, + recommendation_id TEXT NOT NULL REFERENCES recommendations(id), + user_id TEXT NOT NULL, + action TEXT NOT NULL CHECK (action IN ('accepted', 'dismissed')), + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); +CREATE INDEX IF NOT EXISTS idx_rf_user_id ON recommendation_feedback (user_id); +``` + +```python +# models.py addition needed +class FeedbackCreate(BaseModel): + action: Literal["accepted", "dismissed"] + +class Feedback(FeedbackCreate): + id: str + recommendation_id: str + user_id: str + created_at: datetime +``` + +This *is* allowed a `recommendation_id` foreign key — unlike the deliberate choice to leave `goal_id` off `activity_signals` (Phase 3 doc, §1.3). The reasoning is different here: a signal can be evidence for zero or several goals, which is exactly why that correlation was left to application logic. Feedback, by contrast, is *always* about exactly one specific recommendation that was actually shown — there's no ambiguity to defer, so the FK is the right call. + +### 3.3 LLM Usage Log — Bonus Only + +Needed only if the LLM-powered-explanations bonus is attempted. Without this table, `daily_cost_cap_usd` stays an unenforced number in `config.py`. + +```sql +-- New migration, bonus scope only +CREATE TABLE IF NOT EXISTS llm_usage_log ( + id TEXT PRIMARY KEY, + user_id TEXT NOT NULL, + model TEXT NOT NULL, + input_tokens INTEGER NOT NULL, + output_tokens INTEGER NOT NULL, + cost_usd DOUBLE PRECISION NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); +CREATE INDEX IF NOT EXISTS idx_llm_created_at ON llm_usage_log (created_at); +``` + +--- + +## 4. Implementation Plan, By Gate Task + +### 4.1 Recommendation Algorithm — Owner: Jaivardhan + +**What it has to do:** turn a user's goals + recent signals into ranked candidate actions. + +**How to build it, reusing what exists:** + +```python +# Proposed: src/lpi/recommendation_engine.py (new module) + +from lpi import store +from lpi.scoring import sort_goals_by_score, score_goal +from lpi.smile import PHASE_ORDER, validate_phase_transition, get_phase_description + +def generate_candidates(user_id: str) -> list[dict]: + """Produce ranked candidate actions for one user. + + Step 1 — fetch ranked goals. Reuses Phase 2's scoring exactly as-is; + no new ranking logic needed for "which goal matters most right now." + """ + goals = store.list_goals(user_id=user_id) + ranked_goals = sort_goals_by_score(goals) # existing function, zero changes + + """ + Step 2 — fetch recent signals. + KNOWN GAP (Phase 3 doc §4.3): there is no server-side time-range filter yet. + Interim workaround until that's closed: pull a generously large page, + sorted newest-first (already the default order from store.list_signals), + and filter client-side on `timestamp` here. This is the client-side + anti-pattern the Phase 3 doc warns against for production scale — it is + acceptable ONLY as a temporary measure, and should be replaced with a + real `start=`/`end=` query the moment that gap is closed. + """ + all_recent = store.list_signals(user_id=user_id, limit=200) + cutoff = datetime.now(UTC) - timedelta(days=7) + recent_signals = [s for s in all_recent if s.timestamp >= cutoff] + + candidates = [] + for goal in ranked_goals: + # Has this goal seen ANY signal evidence at all (any stream/event_type)? + # A real implementation matches signal.payload content against + # goal.description — left intentionally simple here; this is the + # single biggest place to invest follow-up engineering effort. + has_recent_evidence = any( + True for s in recent_signals # naive: any recent signal counts as activity + ) + + if not has_recent_evidence: + # No evidence in the window → recommend an action, not a phase change + candidates.append({ + "goal": goal, + "action": f"Log progress on '{goal.title}' — no activity signal in 7 days", + "smile_phase": goal.smile_phase, + "priority": score_goal(goal), + "source_goals": [goal.id], + "source_signals": [], + }) + else: + # Evidence exists → is advancing to the next SMILE phase valid? + current_idx = PHASE_ORDER.index(goal.smile_phase) + if current_idx + 1 < len(PHASE_ORDER): + next_phase = PHASE_ORDER[current_idx + 1] + if validate_phase_transition(goal.smile_phase, next_phase): + candidates.append({ + "goal": goal, + "action": f"Consider advancing '{goal.title}' to {next_phase.value}", + "smile_phase": next_phase, + "priority": score_goal(goal), + "source_goals": [goal.id], + "source_signals": [s.id for s in recent_signals][:5], + }) + + # Step 3 — rank candidates, return top N (default 3, matches the gate) + candidates.sort(key=lambda c: -c["priority"]) + return candidates[:3] +``` + +This is deliberately scoped as a **starting algorithm, not a finished one** — the "has this goal seen evidence" check above is naive on purpose (it doesn't yet match a signal's `payload`/`stream` against a goal's actual subject matter). That matching step is the real intellectual core of the recommendation engine and the part most worth iterating on; everything else in this function is plumbing that already existed in Phase 2/3 and just needed to be called in sequence. + +### 4.2 SMILE-Based Reasoning — Owner: Jaivardhan + +**The gate's actual bar:** *"Explanations are specific, not generic."* This is exactly what `scoring.score_explanation()` was already built for — its own docstring says so directly: *"Used by the Phase 4 recommendation engine to produce transparent reasoning."* Phase 2 wrote this for Phase 4 to use; Phase 4 should use it rather than inventing a parallel explanation system. + +```python +from lpi.scoring import score_explanation +from lpi.smile import get_phase_description, get_phase_key_question + +def build_reasoning(candidate: dict) -> str: + goal = candidate["goal"] + # Reuse Phase 2's per-goal score breakdown verbatim — it already names + # the goal's actual priority, phase, and urgency numbers, which is what + # makes it "specific" rather than templated boilerplate. + base = score_explanation(goal) + + # Layer the SMILE phase's key question on top — ties the recommendation + # back to the methodology's own framing, not just a numeric score. + question = get_phase_key_question(candidate["smile_phase"]) + return f"{base} Next-phase question to consider: {question}" +``` + +This single change — calling existing Phase 2 functions instead of writing new copy — is what turns "generic" into "specific" without any new design work: `score_explanation()` already interpolates the real goal title, real priority number, and real phase weight into its sentence. + +### 4.3 Recommendation Endpoint — Owner: Adil + +```python +# routers/recommendations.py — replacing the [] stub + +@router.get("/{user_id}", response_model=list[Recommendation]) +def get_recommendations( + user_id: str, + limit: int = Query(default=3, ge=1, le=10), + caller_id: str = Depends(get_current_user), # ← MUST be added; absent today +) -> list[Recommendation]: + """Return up to `limit` ranked recommendations for `user_id`. + + SECURITY FIX vs. current stub: the path `user_id` and the authenticated + `caller_id` are two different things. Without this check, any + authenticated user could request any OTHER user's recommendations by + guessing their UUID in the URL — there is currently nothing stopping + that once this returns real data instead of []. + """ + if user_id != caller_id: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Not found") + # 404, not 403 — same information-hiding pattern goals.py and + # signals.py already use (Phase 3 doc §4.4): don't confirm or deny + # that a different user_id exists. + + candidates = generate_candidates(user_id) + recommendations = [ + Recommendation( + id=str(uuid.uuid4()), user_id=user_id, action=c["action"], + reasoning=build_reasoning(c), smile_phase=c["smile_phase"], + priority=c["priority"], source_goals=c["source_goals"], + source_signals=c["source_signals"], created_at=datetime.now(UTC), + ) + for c in candidates[:limit] + ] + for rec in recommendations: + store.insert_recommendation(rec) # persist — feedback needs the id (§3.1, §4.6) + + return recommendations +``` + +`store.insert_recommendation()` doesn't exist yet either — it's a one-function addition to `store.py`, identical in shape to `insert_signal()` (Phase 3 doc §3.1): build a dict via `model_dump(mode="json")`, `.table("recommendations").insert(...)`. + +### 4.4 Frontend Recommendation Cards — Owner: Jahanvi + +The contract this depends on is already fully specified by the `Recommendation` model — `action`, `reasoning`, `smile_phase`, `priority` map directly onto the gate's "Cards show: action, why, SMILE phase, priority" requirement with no translation layer needed. As with the Phase 3 timeline view, **no frontend code exists in this repo yet** — this is the second frontend deliverable stacked on the same unstarted foundation, so the two should likely be planned together rather than as fully separate efforts once frontend work begins. + +### 4.5 Agent Orchestration — Owner: Daksh + +**Open dependency to resolve first:** the gate references "chosen framework from Phase 1," but `pyproject.toml` currently has no agent/LLM orchestration dependency at all (no LangGraph, no equivalent). This needs to be confirmed against whatever Phase 1 planning artifact made that choice before any code is written here — building against the wrong assumption is more expensive to unwind than spending an hour confirming it now. + +**Scope boundary worth setting explicitly**, because the gate's own wording draws it: §4.1's `generate_candidates()` is the deterministic core that satisfies *"given goals + signals → 3 actions."* Agent orchestration is described as handling *"complex multi-step queries"* — a different, broader job (e.g. "what should I prioritize across all my goals this week, accounting for what I dismissed last time"). The clean design is **the algorithm as a callable tool the agent can invoke**, not a rewrite of the algorithm in agent form: + +```python +# Conceptual shape — concrete implementation depends on the confirmed framework +tools = [ + Tool(name="get_ranked_goals", fn=lambda uid: sort_goals_by_score(store.list_goals(user_id=uid))), + Tool(name="get_recent_signals", fn=lambda uid, days: [...]), # same gap as §4.1 + Tool(name="generate_candidates", fn=generate_candidates), # §4.1, reused directly + Tool(name="explain_smile_phase", fn=get_phase_description), +] +``` + +This keeps the single-call gate path (§4.3) fast and dependency-free, while the multi-step path can compose the same building blocks through whichever framework Phase 1 settled on. + +### 4.6 User Feedback Loop — Owner: Yashika + +```python +# New endpoint, e.g. POST /api/v1/recommendations/{recommendation_id}/feedback + +@router.post("/{recommendation_id}/feedback", response_model=Feedback, status_code=201) +def submit_feedback( + recommendation_id: str, feedback: FeedbackCreate, + user_id: str = Depends(get_current_user), +) -> Feedback: + new_feedback = Feedback(id=str(uuid.uuid4()), recommendation_id=recommendation_id, + user_id=user_id, action=feedback.action, created_at=datetime.now(UTC)) + store.insert_feedback(new_feedback) + # Same dual-sync, never-break-the-endpoint pattern as log_user_activity (Phase 3 doc §3.1) + log_user_activity(user_id=user_id, action="recommendation_feedback", + resource_id=recommendation_id, metadata={"action": feedback.action}) + return new_feedback +``` + +**"Influences future suggestions" — the simplest mechanism that actually satisfies this:** in `generate_candidates()` (§4.1), before ranking, look up recent dismissals for the user and down-weight (or skip) candidates whose `action` text matches a recently-dismissed one closely enough. This doesn't need to be sophisticated to pass the gate — it needs to be *demonstrable*: dismiss a recommendation, call the endpoint again, show the same suggestion doesn't reappear immediately. A more sophisticated version (adjusting `PHASE_WEIGHT`/`PRIORITY_WEIGHT`-style learned weights per user) is a reasonable post-gate iteration, not a Phase 4 demo requirement. Note that `user_activity_logs`' CHECK constraint (Phase 3 doc §6.4) will need a similar follow-up if `"recommendation_feedback"` is added as an action value there too — same class of bug, worth fixing in the same migration pass rather than rediscovering it the same way. + +### 4.7 Unit Tests for Recommendation Logic — Owner: Yashika + +Covered in depth in §6 below — the test file already exists with the right test names; it's the implementation and one missing fixture that are blocking it. + +### 4.8 Bonus — LLM-Powered Explanations With Cost Cap + +**Why this is correctly scoped as bonus, not core:** the deterministic reasoning in §4.2 already satisfies "specific, not generic" using real numbers and real goal/phase text — it does not require an LLM call to pass the gate. The bonus is specifically about *quality of prose*, not correctness, which is exactly the kind of enhancement that should degrade gracefully rather than become a dependency. + +```python +# Proposed shape — wraps §4.2's deterministic output as the floor, never the ceiling's only path + +DAILY_CAP = settings.daily_cost_cap_usd # 10.0 — already in config.py, currently unused anywhere + +def build_reasoning(candidate: dict, allow_llm: bool = True) -> str: + deterministic = _deterministic_reasoning(candidate) # §4.2, always computed first + + if not allow_llm or _today_cost_usd() >= DAILY_CAP: + return deterministic # cap hit, or LLM disabled → fall back, never fail the request + + try: + response = llm_client.rewrite(deterministic) # one call, one user-facing rewrite + _log_llm_usage(tokens_in=response.usage.input_tokens, + tokens_out=response.usage.output_tokens, + cost_usd=response.cost) # writes to llm_usage_log (§3.3) + return response.text + except Exception: + return deterministic # same "logging/LLM failure never breaks the endpoint" pattern + # already established in logging.py and signals.py +``` + +`_today_cost_usd()` sums `llm_usage_log.cost_usd` for the current UTC day — a single `SELECT SUM(cost_usd) WHERE created_at >= today` query, checked **before** the LLM call, not after, so the cap is actually preventative rather than just an after-the-fact reading. This is also where the `<3s` target from `TimingMiddleware`'s own docstring (§2) becomes a real constraint: an LLM call needs a tight timeout (suggest ≤1.5s) with the deterministic fallback firing on timeout, not just on cap-exceeded — a slow LLM call should never be the reason a recommendation request blows its latency budget. + +--- + +## 5. Execution Flow + +```mermaid +sequenceDiagram + participant FE as Frontend Cards
(not yet built) + participant API as GET /recommendations/{user_id} + participant Auth as Auth Middleware
(must be added — §4.3) + participant Eng as generate_candidates()
(new module, §4.1) + participant Goals as store.list_goals()
+ sort_goals_by_score() + participant Sig as store.list_signals()
⚠ no time-range filter yet + participant SMILE as scoring.score_explanation()
+ smile.get_phase_description() + participant LLM as LLM rewrite
(bonus, cost-capped) + participant DB as Supabase
recommendations table (new) + participant FB as POST .../feedback
(new, §4.6) + + FE->>API: GET /recommendations/{user_id}?limit=3 + API->>Auth: verify JWT, check user_id == caller + Auth-->>API: caller_id + API->>Eng: generate_candidates(user_id) + Eng->>Goals: list_goals(user_id) → sort_goals_by_score() + Eng->>Sig: list_signals(user_id, limit=200), filter client-side ≤7d + Note over Sig: Replace with real start=/end= query
once Phase 3 gap (§1.3) is closed + Eng-->>API: top-3 ranked candidates + API->>SMILE: build_reasoning(candidate) per item + SMILE->>LLM: optional rewrite, if under $10/day cap + LLM-->>SMILE: rewritten text, or deterministic fallback + API->>DB: insert_recommendation() per item + API-->>FE: 201, list[Recommendation] + + FE->>FB: POST {recommendation_id} {action: accepted|dismissed} + FB->>DB: insert_feedback() + Note over Eng: Next call to generate_candidates()
down-weights recently dismissed actions +``` + +--- + +## 6. Testing & Validation Strategy + +### 6.1 Fix Before Anything Else: the Missing Fixture + +`tests/test_recommendations.py` already references `phase_gate_enabled` as a fixture argument: + +```python +def test_get_recommendations(self, client, phase_gate_enabled: bool) -> None: +``` + +**`phase_gate_enabled` does not exist in `conftest.py` today.** Until it's added, un-skipping these tests will fail at fixture resolution before the test body even runs — a pytest error, not a clean skip. The fix is small and should land before any other Phase 4 test work: + +```python +# conftest.py addition needed +import os + +@pytest.fixture +def phase_gate_enabled() -> bool: + return os.environ.get("LPI_RUN_PHASE_GATES") == "1" +``` + +This matches the env var name (`LPI_RUN_PHASE_GATES`) already referenced in the test docstrings and in `docs/notes/phase3_signals_prep.md`'s implementation notes — the convention was established, just never wired into a fixture. + +### 6.2 Existing Test Skeleton — What Each One Will Actually Require + +| Test | Currently | To make it pass | +|---|---|---| +| `test_get_recommendations` | skipped unless `LPI_RUN_PHASE_GATES=1` | Needs §4.3's endpoint, plus `sample_goal`/`sample_signal` fixtures seeded beforehand (the existing fixtures from Phase 2/3's `conftest.py` work as-is) | +| `test_recommendations_have_reasoning` | `pytest.skip("Implement after goals + signals exist")` | Goals and signals already exist (Phase 2/3 shipped) — this skip reason is now stale; the real blocker is §4.2's `build_reasoning()` | +| `test_recommendations_reference_smile_phase` | `pytest.skip("Implement after recommendation engine built")` | Assert `recommendation.smile_phase` is a valid `SmilePhase` enum value present in `PHASE_ORDER` | +| `test_max_3_recommendations` | gated, same pattern as the first | Trivial once §4.3 respects `limit` — already shaped correctly in the stub signature | + +### 6.3 New Tests Needed, Not Yet Skeletoned + +- **Auth enforcement on the new endpoint** — a test asserting `user_id != caller_id` returns 404, mirroring the existing pattern in `test_goal_crud.py`'s ownership checks. This is the one true new piece of attack-surface in this phase (§1.3) and deserves its own explicit test, not just incidental coverage. +- **Algorithm unit tests, isolated from Supabase** — `generate_candidates()` and `build_reasoning()` are pure-ish functions once goals/signals are passed in directly (not fetched). Following `test_scoring.py`'s existing pattern (pure functions, no DB, no client fixture needed) makes these fast and reliable: feed in a fixed list of `Goal`/`Signal` objects, assert on the returned candidates' `action` and `priority` fields. +- **Feedback influencing future output** — insert a recommendation, submit a `dismissed` feedback, call `generate_candidates()` again, assert the dismissed action doesn't reappear. This is the actual gate condition ("feedback... affects next recommendations") and currently has no test anywhere. +- **Cost-cap enforcement (bonus scope)** — seed `llm_usage_log` with rows summing to ≥ $10 for today, call `build_reasoning()` with `allow_llm=True`, assert the LLM path is skipped and the deterministic fallback is returned. Also test the timeout-fallback path independently of the cap path — these are two different failure modes and should not share one test. +- **Latency target** — given `TimingMiddleware` already exists specifically to surface `X-Process-Time` for this purpose, add a test asserting the header is present and under 3000ms on a recommendation call with the LLM path both enabled and disabled, so a slow bonus feature doesn't silently regress the core gate's performance bar. + +--- + +## 7. Completion Checklist + +| Deliverable | Owner | Status | Depends on | +|---|---|---|---| +| Recommendation algorithm (goals + signals → candidates) | Jaivardhan | 🔴 Not started | `store.list_goals`/`list_signals` (exist) + Phase 3 time-range gap (workaround viable, real fix preferred) | +| SMILE-based reasoning (specific, not generic) | Jaivardhan | 🔴 Not started | `scoring.score_explanation()`, `smile.get_phase_description()`/`get_phase_key_question()` — all exist, just need calling | +| `recommendations` table + `store.insert_recommendation`/`list_recommendations` | Jaivardhan / Adil | 🔴 Not started | New migration (§3.1) — no schema or store function exists yet | +| Recommendation endpoint with auth | Adil | 🔴 Not started | Must add `Depends(get_current_user)` — currently absent on this route entirely (§1.3) | +| Frontend recommendation cards | Jahanvi | 🔴 Not started | Endpoint contract (above); no frontend code exists in-repo for Phase 3's timeline either — same blocker class | +| Agent orchestration (multi-step queries) | Daksh | 🔴 Not started, blocked | Framework choice from Phase 1 not confirmed in this codebase — `pyproject.toml` has zero agent/LLM SDK dependencies today | +| User feedback loop | Yashika | 🔴 Not started | New `recommendation_feedback` table (§3.2); down-weighting logic in §4.1 | +| Unit tests for recommendation logic | Yashika | 🟡 Skeleton exists, all skipped | Missing `phase_gate_enabled` fixture (§6.1) blocks even running the gated ones | +| **Bonus:** LLM explanations, cost-tracked, $10/day cap | Jaivardhan (+Adil for wiring) | 🔴 Not started | `llm_provider`/`llm_model`/`daily_cost_cap_usd` already in `config.py` but read by nothing; needs `llm_usage_log` table + the cap-check-before-call logic in §4.8 | + +✅ Done · 🟡 Partial / in progress · 🔴 Not started + +**Recommended build order**, based purely on dependency chains above, not owner availability: + +1. `recommendations` table + `store` functions (§3.1) — everything else writes through this +2. Endpoint with auth fix (§4.3) + algorithm (§4.1) + reasoning (§4.2) — this alone satisfies the core gate +3. `phase_gate_enabled` fixture (§6.1) — unblocks running the existing test skeleton against step 2 +4. Feedback table + endpoint + down-weighting (§3.2, §4.6) — second gate clause, independent of frontend/agent work +5. Frontend cards (§4.4) — can start as soon as step 2's response shape is stable, doesn't need to wait for 3–4 +6. Agent orchestration (§4.5) — gated on confirming the Phase 1 framework choice; can proceed in parallel with 3–5 once unblocked +7. LLM bonus (§4.8) — last, by design; it's additive and every other step already works without it + +--- + +## 8. Key Considerations + +**This phase's biggest risk is invisible until step 2 ships: the missing auth check.** Every other endpoint in this codebase (`goals.py`, `signals.py`) enforces `user_id != caller_id → 404` from day one. The recommendation stub never got that treatment because it never returned real data to protect. The moment §4.1–4.3 land, this becomes a live cross-user data leak if the fix in §4.3 is skipped or deferred "for later" — it should be written in the same commit as the real algorithm, not as a follow-up PR. + +**The <3s latency target is not a new requirement invented for this phase — it's already encoded in infrastructure that's been live since Phase 2.** `TimingMiddleware`'s own docstring states its `X-Process-Time` header exists specifically to verify this target. That means the bar was set before this module existed, and the bonus LLM path (§4.8) is the one piece of this phase actually at risk of breaking it — which is exactly why §4.8 specifies a tight LLM timeout with deterministic fallback rather than letting a slow external API call become the response time. + +**The cost cap is the bonus's only hard requirement, and it's currently zero percent enforced despite looking configured.** `daily_cost_cap_usd: float = 10.0` sitting in `config.py` reads like the cap already exists — it doesn't; nothing in the codebase queries it. Treat that field as a placeholder the bonus work fulfills, not as evidence the bonus is partially done. + +**Connection back to Phase 3, restated plainly:** this phase cannot fully satisfy its own gate (3 actions grounded in goals *and signals*) until Phase 3's time-range filtering gap is closed, because "recent activity" is the natural lens for "what should I do next." The workaround in §4.1 (fetch a large page, filter client-side) is explicitly a stopgap — it works for a demo at current data volumes, and stops being acceptable the moment either module has real production traffic, for the same reason the Phase 3 document gives for preferring server-side filtering in the first place. + +--- + +*Reviewed against the running `lpi-platform` codebase (models.py, scoring.py, smile.py, store.py, routers/recommendations.py, routers/goals.py, middleware/__init__.py, config.py, tests/test_recommendations.py, tests/conftest.py) — none of which contain a working recommendation algorithm yet, which is why this document is a build plan rather than an as-built description. The three items most worth resolving before any other Phase 4 work starts: (1) add the missing auth check to the recommendation endpoint before it serves real data; (2) add the `phase_gate_enabled` fixture so the existing test skeleton can even run; (3) confirm the agent orchestration framework from Phase 1 with Daksh before writing any orchestration code against an assumed choice.* From 1ff4cf9a11d775aba8bd0fe434b4755e6a3f6bdc Mon Sep 17 00:00:00 2001 From: Adil Islam <93443758+Adilislam0@users.noreply.github.com> Date: Sat, 20 Jun 2026 21:01:18 +0530 Subject: [PATCH 3/7] Delete Phase4_Module3_Recommendation_Engine_Plan.md --- Phase4_Module3_Recommendation_Engine_Plan.md | 490 ------------------- 1 file changed, 490 deletions(-) delete mode 100644 Phase4_Module3_Recommendation_Engine_Plan.md diff --git a/Phase4_Module3_Recommendation_Engine_Plan.md b/Phase4_Module3_Recommendation_Engine_Plan.md deleted file mode 100644 index 8d6f162..0000000 --- a/Phase4_Module3_Recommendation_Engine_Plan.md +++ /dev/null @@ -1,490 +0,0 @@ -# Phase 4, Module 3: Recommendation Engine -## Implementation Plan — Data Model, Build Sequence & Testing/Validation - -**Repo:** `lpi-platform` · **Owners:** Jaivardhan (algorithm + reasoning) · Adil (endpoint) · Jahanvi (frontend) · Daksh (agent orchestration) · Yashika (feedback + tests) - -> **Status: not started.** Unlike the Phase 3 document, this is not a description of running code — `recommendations.py` is still the Phase 1/2 stub (`return []`), and `test_recommendations.py` is fully skipped. This document is a build plan: what has to be created, in what order, and — critically — using which pieces Phase 2 and Phase 3 already built so this phase isn't starting from zero. Every "how to build it" section below names the actual existing function it should call rather than describing a new one to write from scratch. - ---- - -## 1. Phase Overview - -### 1.1 Purpose - -Phase 4 is where the platform stops being a system of record (goals + signals) and becomes a system of **advice**. Given a user's Goals (Phase 2) and Activity Signals (Phase 3), the engine has to surface three concrete next actions, explain *why* each one matters in SMILE terms, and learn from whether the user accepts or dismisses them. - -### 1.2 Gate Condition - -> **Gate:** Given a user's goals + activity signals → system suggests 3 next actions with SMILE-based reasoning. Frontend shows recommendation cards. Daksh's agent orchestration pipeline handles multi-step queries. - -> **Bonus:** LLM-powered explanations (not just template-based), with per-call cost tracked. $10/day cap enforced in code. - -Breaking the gate into testable conditions, the same way the Phase 3 document did: - -| Gate clause | Testable as | -|---|---| -| Suggests 3 next actions | `GET /recommendations/{user_id}` returns a list of length ≤ 3, sorted by relevance | -| SMILE-based reasoning, specific not generic | Each `reasoning` string names the actual goal title and/or signal evidence it's based on — not a templated sentence with no per-user detail | -| Frontend shows recommendation cards | A card component renders `action`, `reasoning`, `smile_phase`, `priority` per item | -| Agent orchestration handles multi-step queries | A separate orchestration layer can chain goals→signals→actions for queries broader than "give me 3 actions" (e.g. "what should I focus on across all my goals this week") | -| Feedback loop | Accept/dismiss is persisted and measurably changes future output for that user | - -### 1.3 Direct Continuity From Phase 3 - -This phase inherits two unresolved items from the Phase 3 document rather than starting clean, and both block specific Phase 4 tasks: - -1. **Time-range query filtering on signals does not exist yet** (Phase 3 doc, §4.3). The recommendation algorithm's most natural question — *"what has this user done recently?"* — has no server-side way to ask "signals from the last 7 days." Closing that gap is now a **Phase 4 prerequisite**, not just a Phase 3 nice-to-have, because §4.1 below depends on it directly. -2. **The `recommendations.py` stub currently has no auth dependency at all** — `get_recommendations(user_id: str, limit: int = ...)` takes `user_id` as a path parameter with no `Depends(get_current_user)` anywhere. Once this returns real data instead of `[]`, that's an open endpoint serving anyone's recommendations to anyone who guesses their UUID. This has to be fixed as part of the endpoint work in §4.3, not treated as a separate hardening pass later. - ---- - -## 2. What Already Exists vs. What Must Be Built - -This is the most important table in this document — it's the difference between a 4-week build and a 1-week build. - -| Already exists (reuse, don't rewrite) | Where | What's missing (must build) | -|---|---|---| -| `Recommendation` Pydantic model — `id, user_id, action, reasoning, smile_phase, priority, source_goals, source_signals, created_at` | `models.py` | A `store.insert_recommendation()` / `list_recommendations()` pair — recommendations are currently never persisted anywhere | -| SMILE-weighted goal scoring (`score_goal`, `score_explanation`, `sort_goals_by_score`) | `scoring.py` | Wiring this into a candidate-action generator — it currently only ranks goals, not actions | -| SMILE phase descriptions + key questions (`get_phase_description`, `get_phase_key_question`) | `smile.py` | Using these as the actual text source for "specific, not generic" reasoning, instead of writing new copy | -| Forward-one-step phase transition validation (`validate_phase_transition`) | `smile.py` | Reusing this to decide *when* "move to the next SMILE phase" is a valid recommendation vs. a skip | -| `store.list_goals(user_id, smile_phase)` and `store.list_signals(user_id, stream, event_type, source, limit, offset)` | `store.py` | A combined fetch step that pulls both per request, and a workaround for the missing time-range filter (§4.1) | -| `Recommendation.limit` query param already shaped for "top N" (`default=3, ge=1, le=10`) on the route stub | `routers/recommendations.py` | Auth dependency, real body, and persistence | -| `llm_provider`, `llm_model`, `daily_cost_cap_usd=10.0` settings fields | `config.py` | **These exist but are read by nothing.** No LLM client, no cost tracker, no enforcement — the bonus's hardest requirement is a config placeholder today | -| `X-Process-Time` response header, added specifically — per its own docstring — "to verify the <3s recommendation target" | `middleware/__init__.py` | An actual recommendation endpoint to measure; the SLA is already implied in the infra, it just has nothing to time yet | -| Dual-sync logging pattern (`log_user_activity`, `log_system_event`) — Supabase + in-memory, try/except wrapped | `utils/logging.py` | A fourth log type, or reuse of `log_user_activity`, for feedback events (§4.6) | -| Skipped test skeleton naming the exact gate behaviors to verify | `tests/test_recommendations.py` | The `phase_gate_enabled` fixture these tests reference **does not exist in `conftest.py` today** — un-skipping them will fail on a missing fixture until that's added (§6.1) | - ---- - -## 3. Data Model Additions Needed - -### 3.1 Recommendation — Already Modeled, Needs Persistence - -```python -# models.py — already exists, no changes needed -class Recommendation(BaseModel): - id: str - user_id: str - action: str - reasoning: str - smile_phase: SmilePhase - priority: float - source_goals: list[str] = [] - source_signals: list[str] = [] - created_at: datetime -``` - -The model is gate-ready as written. What's missing is a place for it to live. Right now there is no `recommendations` table and no `store.insert_recommendation()` — every call to the endpoint would have to recompute from scratch with no record of what was shown before. That's a problem the moment feedback enters the picture: §4.6 needs to say "the user dismissed *recommendation X*," which requires recommendation X to have a durable `id` a feedback row can point to. **Decision to make explicitly before building:** persist every generated recommendation (cheap — it's a small JSON row) rather than trying to make feedback reference an ephemeral, never-stored object. - -```sql --- New migration needed: 2026MMDD000000_create_recommendations.sql -CREATE TABLE IF NOT EXISTS recommendations ( - id TEXT PRIMARY KEY, - user_id TEXT NOT NULL, - action TEXT NOT NULL, - reasoning TEXT NOT NULL, - smile_phase TEXT NOT NULL, - priority DOUBLE PRECISION NOT NULL, - source_goals JSONB NOT NULL DEFAULT '[]', - source_signals JSONB NOT NULL DEFAULT '[]', - created_at TIMESTAMPTZ NOT NULL DEFAULT NOW() -); -CREATE INDEX IF NOT EXISTS idx_rec_user_id ON recommendations (user_id); -CREATE INDEX IF NOT EXISTS idx_rec_created_at ON recommendations (created_at DESC); -``` - -Mirrors the `activity_signals` migration pattern exactly (§2.1 of the Phase 3 document) — same column-naming convention, same one-index-per-filter-column approach, same `TEXT` primary key generated in Python. - -### 3.2 Feedback — Net New - -The gate names this explicitly: *"accept/dismiss recommendation → influences future suggestions... Feedback stored, affects next recommendations."* - -```sql --- New migration: 2026MMDD000001_create_recommendation_feedback.sql -CREATE TABLE IF NOT EXISTS recommendation_feedback ( - id TEXT PRIMARY KEY, - recommendation_id TEXT NOT NULL REFERENCES recommendations(id), - user_id TEXT NOT NULL, - action TEXT NOT NULL CHECK (action IN ('accepted', 'dismissed')), - created_at TIMESTAMPTZ NOT NULL DEFAULT NOW() -); -CREATE INDEX IF NOT EXISTS idx_rf_user_id ON recommendation_feedback (user_id); -``` - -```python -# models.py addition needed -class FeedbackCreate(BaseModel): - action: Literal["accepted", "dismissed"] - -class Feedback(FeedbackCreate): - id: str - recommendation_id: str - user_id: str - created_at: datetime -``` - -This *is* allowed a `recommendation_id` foreign key — unlike the deliberate choice to leave `goal_id` off `activity_signals` (Phase 3 doc, §1.3). The reasoning is different here: a signal can be evidence for zero or several goals, which is exactly why that correlation was left to application logic. Feedback, by contrast, is *always* about exactly one specific recommendation that was actually shown — there's no ambiguity to defer, so the FK is the right call. - -### 3.3 LLM Usage Log — Bonus Only - -Needed only if the LLM-powered-explanations bonus is attempted. Without this table, `daily_cost_cap_usd` stays an unenforced number in `config.py`. - -```sql --- New migration, bonus scope only -CREATE TABLE IF NOT EXISTS llm_usage_log ( - id TEXT PRIMARY KEY, - user_id TEXT NOT NULL, - model TEXT NOT NULL, - input_tokens INTEGER NOT NULL, - output_tokens INTEGER NOT NULL, - cost_usd DOUBLE PRECISION NOT NULL, - created_at TIMESTAMPTZ NOT NULL DEFAULT NOW() -); -CREATE INDEX IF NOT EXISTS idx_llm_created_at ON llm_usage_log (created_at); -``` - ---- - -## 4. Implementation Plan, By Gate Task - -### 4.1 Recommendation Algorithm — Owner: Jaivardhan - -**What it has to do:** turn a user's goals + recent signals into ranked candidate actions. - -**How to build it, reusing what exists:** - -```python -# Proposed: src/lpi/recommendation_engine.py (new module) - -from lpi import store -from lpi.scoring import sort_goals_by_score, score_goal -from lpi.smile import PHASE_ORDER, validate_phase_transition, get_phase_description - -def generate_candidates(user_id: str) -> list[dict]: - """Produce ranked candidate actions for one user. - - Step 1 — fetch ranked goals. Reuses Phase 2's scoring exactly as-is; - no new ranking logic needed for "which goal matters most right now." - """ - goals = store.list_goals(user_id=user_id) - ranked_goals = sort_goals_by_score(goals) # existing function, zero changes - - """ - Step 2 — fetch recent signals. - KNOWN GAP (Phase 3 doc §4.3): there is no server-side time-range filter yet. - Interim workaround until that's closed: pull a generously large page, - sorted newest-first (already the default order from store.list_signals), - and filter client-side on `timestamp` here. This is the client-side - anti-pattern the Phase 3 doc warns against for production scale — it is - acceptable ONLY as a temporary measure, and should be replaced with a - real `start=`/`end=` query the moment that gap is closed. - """ - all_recent = store.list_signals(user_id=user_id, limit=200) - cutoff = datetime.now(UTC) - timedelta(days=7) - recent_signals = [s for s in all_recent if s.timestamp >= cutoff] - - candidates = [] - for goal in ranked_goals: - # Has this goal seen ANY signal evidence at all (any stream/event_type)? - # A real implementation matches signal.payload content against - # goal.description — left intentionally simple here; this is the - # single biggest place to invest follow-up engineering effort. - has_recent_evidence = any( - True for s in recent_signals # naive: any recent signal counts as activity - ) - - if not has_recent_evidence: - # No evidence in the window → recommend an action, not a phase change - candidates.append({ - "goal": goal, - "action": f"Log progress on '{goal.title}' — no activity signal in 7 days", - "smile_phase": goal.smile_phase, - "priority": score_goal(goal), - "source_goals": [goal.id], - "source_signals": [], - }) - else: - # Evidence exists → is advancing to the next SMILE phase valid? - current_idx = PHASE_ORDER.index(goal.smile_phase) - if current_idx + 1 < len(PHASE_ORDER): - next_phase = PHASE_ORDER[current_idx + 1] - if validate_phase_transition(goal.smile_phase, next_phase): - candidates.append({ - "goal": goal, - "action": f"Consider advancing '{goal.title}' to {next_phase.value}", - "smile_phase": next_phase, - "priority": score_goal(goal), - "source_goals": [goal.id], - "source_signals": [s.id for s in recent_signals][:5], - }) - - # Step 3 — rank candidates, return top N (default 3, matches the gate) - candidates.sort(key=lambda c: -c["priority"]) - return candidates[:3] -``` - -This is deliberately scoped as a **starting algorithm, not a finished one** — the "has this goal seen evidence" check above is naive on purpose (it doesn't yet match a signal's `payload`/`stream` against a goal's actual subject matter). That matching step is the real intellectual core of the recommendation engine and the part most worth iterating on; everything else in this function is plumbing that already existed in Phase 2/3 and just needed to be called in sequence. - -### 4.2 SMILE-Based Reasoning — Owner: Jaivardhan - -**The gate's actual bar:** *"Explanations are specific, not generic."* This is exactly what `scoring.score_explanation()` was already built for — its own docstring says so directly: *"Used by the Phase 4 recommendation engine to produce transparent reasoning."* Phase 2 wrote this for Phase 4 to use; Phase 4 should use it rather than inventing a parallel explanation system. - -```python -from lpi.scoring import score_explanation -from lpi.smile import get_phase_description, get_phase_key_question - -def build_reasoning(candidate: dict) -> str: - goal = candidate["goal"] - # Reuse Phase 2's per-goal score breakdown verbatim — it already names - # the goal's actual priority, phase, and urgency numbers, which is what - # makes it "specific" rather than templated boilerplate. - base = score_explanation(goal) - - # Layer the SMILE phase's key question on top — ties the recommendation - # back to the methodology's own framing, not just a numeric score. - question = get_phase_key_question(candidate["smile_phase"]) - return f"{base} Next-phase question to consider: {question}" -``` - -This single change — calling existing Phase 2 functions instead of writing new copy — is what turns "generic" into "specific" without any new design work: `score_explanation()` already interpolates the real goal title, real priority number, and real phase weight into its sentence. - -### 4.3 Recommendation Endpoint — Owner: Adil - -```python -# routers/recommendations.py — replacing the [] stub - -@router.get("/{user_id}", response_model=list[Recommendation]) -def get_recommendations( - user_id: str, - limit: int = Query(default=3, ge=1, le=10), - caller_id: str = Depends(get_current_user), # ← MUST be added; absent today -) -> list[Recommendation]: - """Return up to `limit` ranked recommendations for `user_id`. - - SECURITY FIX vs. current stub: the path `user_id` and the authenticated - `caller_id` are two different things. Without this check, any - authenticated user could request any OTHER user's recommendations by - guessing their UUID in the URL — there is currently nothing stopping - that once this returns real data instead of []. - """ - if user_id != caller_id: - raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Not found") - # 404, not 403 — same information-hiding pattern goals.py and - # signals.py already use (Phase 3 doc §4.4): don't confirm or deny - # that a different user_id exists. - - candidates = generate_candidates(user_id) - recommendations = [ - Recommendation( - id=str(uuid.uuid4()), user_id=user_id, action=c["action"], - reasoning=build_reasoning(c), smile_phase=c["smile_phase"], - priority=c["priority"], source_goals=c["source_goals"], - source_signals=c["source_signals"], created_at=datetime.now(UTC), - ) - for c in candidates[:limit] - ] - for rec in recommendations: - store.insert_recommendation(rec) # persist — feedback needs the id (§3.1, §4.6) - - return recommendations -``` - -`store.insert_recommendation()` doesn't exist yet either — it's a one-function addition to `store.py`, identical in shape to `insert_signal()` (Phase 3 doc §3.1): build a dict via `model_dump(mode="json")`, `.table("recommendations").insert(...)`. - -### 4.4 Frontend Recommendation Cards — Owner: Jahanvi - -The contract this depends on is already fully specified by the `Recommendation` model — `action`, `reasoning`, `smile_phase`, `priority` map directly onto the gate's "Cards show: action, why, SMILE phase, priority" requirement with no translation layer needed. As with the Phase 3 timeline view, **no frontend code exists in this repo yet** — this is the second frontend deliverable stacked on the same unstarted foundation, so the two should likely be planned together rather than as fully separate efforts once frontend work begins. - -### 4.5 Agent Orchestration — Owner: Daksh - -**Open dependency to resolve first:** the gate references "chosen framework from Phase 1," but `pyproject.toml` currently has no agent/LLM orchestration dependency at all (no LangGraph, no equivalent). This needs to be confirmed against whatever Phase 1 planning artifact made that choice before any code is written here — building against the wrong assumption is more expensive to unwind than spending an hour confirming it now. - -**Scope boundary worth setting explicitly**, because the gate's own wording draws it: §4.1's `generate_candidates()` is the deterministic core that satisfies *"given goals + signals → 3 actions."* Agent orchestration is described as handling *"complex multi-step queries"* — a different, broader job (e.g. "what should I prioritize across all my goals this week, accounting for what I dismissed last time"). The clean design is **the algorithm as a callable tool the agent can invoke**, not a rewrite of the algorithm in agent form: - -```python -# Conceptual shape — concrete implementation depends on the confirmed framework -tools = [ - Tool(name="get_ranked_goals", fn=lambda uid: sort_goals_by_score(store.list_goals(user_id=uid))), - Tool(name="get_recent_signals", fn=lambda uid, days: [...]), # same gap as §4.1 - Tool(name="generate_candidates", fn=generate_candidates), # §4.1, reused directly - Tool(name="explain_smile_phase", fn=get_phase_description), -] -``` - -This keeps the single-call gate path (§4.3) fast and dependency-free, while the multi-step path can compose the same building blocks through whichever framework Phase 1 settled on. - -### 4.6 User Feedback Loop — Owner: Yashika - -```python -# New endpoint, e.g. POST /api/v1/recommendations/{recommendation_id}/feedback - -@router.post("/{recommendation_id}/feedback", response_model=Feedback, status_code=201) -def submit_feedback( - recommendation_id: str, feedback: FeedbackCreate, - user_id: str = Depends(get_current_user), -) -> Feedback: - new_feedback = Feedback(id=str(uuid.uuid4()), recommendation_id=recommendation_id, - user_id=user_id, action=feedback.action, created_at=datetime.now(UTC)) - store.insert_feedback(new_feedback) - # Same dual-sync, never-break-the-endpoint pattern as log_user_activity (Phase 3 doc §3.1) - log_user_activity(user_id=user_id, action="recommendation_feedback", - resource_id=recommendation_id, metadata={"action": feedback.action}) - return new_feedback -``` - -**"Influences future suggestions" — the simplest mechanism that actually satisfies this:** in `generate_candidates()` (§4.1), before ranking, look up recent dismissals for the user and down-weight (or skip) candidates whose `action` text matches a recently-dismissed one closely enough. This doesn't need to be sophisticated to pass the gate — it needs to be *demonstrable*: dismiss a recommendation, call the endpoint again, show the same suggestion doesn't reappear immediately. A more sophisticated version (adjusting `PHASE_WEIGHT`/`PRIORITY_WEIGHT`-style learned weights per user) is a reasonable post-gate iteration, not a Phase 4 demo requirement. Note that `user_activity_logs`' CHECK constraint (Phase 3 doc §6.4) will need a similar follow-up if `"recommendation_feedback"` is added as an action value there too — same class of bug, worth fixing in the same migration pass rather than rediscovering it the same way. - -### 4.7 Unit Tests for Recommendation Logic — Owner: Yashika - -Covered in depth in §6 below — the test file already exists with the right test names; it's the implementation and one missing fixture that are blocking it. - -### 4.8 Bonus — LLM-Powered Explanations With Cost Cap - -**Why this is correctly scoped as bonus, not core:** the deterministic reasoning in §4.2 already satisfies "specific, not generic" using real numbers and real goal/phase text — it does not require an LLM call to pass the gate. The bonus is specifically about *quality of prose*, not correctness, which is exactly the kind of enhancement that should degrade gracefully rather than become a dependency. - -```python -# Proposed shape — wraps §4.2's deterministic output as the floor, never the ceiling's only path - -DAILY_CAP = settings.daily_cost_cap_usd # 10.0 — already in config.py, currently unused anywhere - -def build_reasoning(candidate: dict, allow_llm: bool = True) -> str: - deterministic = _deterministic_reasoning(candidate) # §4.2, always computed first - - if not allow_llm or _today_cost_usd() >= DAILY_CAP: - return deterministic # cap hit, or LLM disabled → fall back, never fail the request - - try: - response = llm_client.rewrite(deterministic) # one call, one user-facing rewrite - _log_llm_usage(tokens_in=response.usage.input_tokens, - tokens_out=response.usage.output_tokens, - cost_usd=response.cost) # writes to llm_usage_log (§3.3) - return response.text - except Exception: - return deterministic # same "logging/LLM failure never breaks the endpoint" pattern - # already established in logging.py and signals.py -``` - -`_today_cost_usd()` sums `llm_usage_log.cost_usd` for the current UTC day — a single `SELECT SUM(cost_usd) WHERE created_at >= today` query, checked **before** the LLM call, not after, so the cap is actually preventative rather than just an after-the-fact reading. This is also where the `<3s` target from `TimingMiddleware`'s own docstring (§2) becomes a real constraint: an LLM call needs a tight timeout (suggest ≤1.5s) with the deterministic fallback firing on timeout, not just on cap-exceeded — a slow LLM call should never be the reason a recommendation request blows its latency budget. - ---- - -## 5. Execution Flow - -```mermaid -sequenceDiagram - participant FE as Frontend Cards
(not yet built) - participant API as GET /recommendations/{user_id} - participant Auth as Auth Middleware
(must be added — §4.3) - participant Eng as generate_candidates()
(new module, §4.1) - participant Goals as store.list_goals()
+ sort_goals_by_score() - participant Sig as store.list_signals()
⚠ no time-range filter yet - participant SMILE as scoring.score_explanation()
+ smile.get_phase_description() - participant LLM as LLM rewrite
(bonus, cost-capped) - participant DB as Supabase
recommendations table (new) - participant FB as POST .../feedback
(new, §4.6) - - FE->>API: GET /recommendations/{user_id}?limit=3 - API->>Auth: verify JWT, check user_id == caller - Auth-->>API: caller_id - API->>Eng: generate_candidates(user_id) - Eng->>Goals: list_goals(user_id) → sort_goals_by_score() - Eng->>Sig: list_signals(user_id, limit=200), filter client-side ≤7d - Note over Sig: Replace with real start=/end= query
once Phase 3 gap (§1.3) is closed - Eng-->>API: top-3 ranked candidates - API->>SMILE: build_reasoning(candidate) per item - SMILE->>LLM: optional rewrite, if under $10/day cap - LLM-->>SMILE: rewritten text, or deterministic fallback - API->>DB: insert_recommendation() per item - API-->>FE: 201, list[Recommendation] - - FE->>FB: POST {recommendation_id} {action: accepted|dismissed} - FB->>DB: insert_feedback() - Note over Eng: Next call to generate_candidates()
down-weights recently dismissed actions -``` - ---- - -## 6. Testing & Validation Strategy - -### 6.1 Fix Before Anything Else: the Missing Fixture - -`tests/test_recommendations.py` already references `phase_gate_enabled` as a fixture argument: - -```python -def test_get_recommendations(self, client, phase_gate_enabled: bool) -> None: -``` - -**`phase_gate_enabled` does not exist in `conftest.py` today.** Until it's added, un-skipping these tests will fail at fixture resolution before the test body even runs — a pytest error, not a clean skip. The fix is small and should land before any other Phase 4 test work: - -```python -# conftest.py addition needed -import os - -@pytest.fixture -def phase_gate_enabled() -> bool: - return os.environ.get("LPI_RUN_PHASE_GATES") == "1" -``` - -This matches the env var name (`LPI_RUN_PHASE_GATES`) already referenced in the test docstrings and in `docs/notes/phase3_signals_prep.md`'s implementation notes — the convention was established, just never wired into a fixture. - -### 6.2 Existing Test Skeleton — What Each One Will Actually Require - -| Test | Currently | To make it pass | -|---|---|---| -| `test_get_recommendations` | skipped unless `LPI_RUN_PHASE_GATES=1` | Needs §4.3's endpoint, plus `sample_goal`/`sample_signal` fixtures seeded beforehand (the existing fixtures from Phase 2/3's `conftest.py` work as-is) | -| `test_recommendations_have_reasoning` | `pytest.skip("Implement after goals + signals exist")` | Goals and signals already exist (Phase 2/3 shipped) — this skip reason is now stale; the real blocker is §4.2's `build_reasoning()` | -| `test_recommendations_reference_smile_phase` | `pytest.skip("Implement after recommendation engine built")` | Assert `recommendation.smile_phase` is a valid `SmilePhase` enum value present in `PHASE_ORDER` | -| `test_max_3_recommendations` | gated, same pattern as the first | Trivial once §4.3 respects `limit` — already shaped correctly in the stub signature | - -### 6.3 New Tests Needed, Not Yet Skeletoned - -- **Auth enforcement on the new endpoint** — a test asserting `user_id != caller_id` returns 404, mirroring the existing pattern in `test_goal_crud.py`'s ownership checks. This is the one true new piece of attack-surface in this phase (§1.3) and deserves its own explicit test, not just incidental coverage. -- **Algorithm unit tests, isolated from Supabase** — `generate_candidates()` and `build_reasoning()` are pure-ish functions once goals/signals are passed in directly (not fetched). Following `test_scoring.py`'s existing pattern (pure functions, no DB, no client fixture needed) makes these fast and reliable: feed in a fixed list of `Goal`/`Signal` objects, assert on the returned candidates' `action` and `priority` fields. -- **Feedback influencing future output** — insert a recommendation, submit a `dismissed` feedback, call `generate_candidates()` again, assert the dismissed action doesn't reappear. This is the actual gate condition ("feedback... affects next recommendations") and currently has no test anywhere. -- **Cost-cap enforcement (bonus scope)** — seed `llm_usage_log` with rows summing to ≥ $10 for today, call `build_reasoning()` with `allow_llm=True`, assert the LLM path is skipped and the deterministic fallback is returned. Also test the timeout-fallback path independently of the cap path — these are two different failure modes and should not share one test. -- **Latency target** — given `TimingMiddleware` already exists specifically to surface `X-Process-Time` for this purpose, add a test asserting the header is present and under 3000ms on a recommendation call with the LLM path both enabled and disabled, so a slow bonus feature doesn't silently regress the core gate's performance bar. - ---- - -## 7. Completion Checklist - -| Deliverable | Owner | Status | Depends on | -|---|---|---|---| -| Recommendation algorithm (goals + signals → candidates) | Jaivardhan | 🔴 Not started | `store.list_goals`/`list_signals` (exist) + Phase 3 time-range gap (workaround viable, real fix preferred) | -| SMILE-based reasoning (specific, not generic) | Jaivardhan | 🔴 Not started | `scoring.score_explanation()`, `smile.get_phase_description()`/`get_phase_key_question()` — all exist, just need calling | -| `recommendations` table + `store.insert_recommendation`/`list_recommendations` | Jaivardhan / Adil | 🔴 Not started | New migration (§3.1) — no schema or store function exists yet | -| Recommendation endpoint with auth | Adil | 🔴 Not started | Must add `Depends(get_current_user)` — currently absent on this route entirely (§1.3) | -| Frontend recommendation cards | Jahanvi | 🔴 Not started | Endpoint contract (above); no frontend code exists in-repo for Phase 3's timeline either — same blocker class | -| Agent orchestration (multi-step queries) | Daksh | 🔴 Not started, blocked | Framework choice from Phase 1 not confirmed in this codebase — `pyproject.toml` has zero agent/LLM SDK dependencies today | -| User feedback loop | Yashika | 🔴 Not started | New `recommendation_feedback` table (§3.2); down-weighting logic in §4.1 | -| Unit tests for recommendation logic | Yashika | 🟡 Skeleton exists, all skipped | Missing `phase_gate_enabled` fixture (§6.1) blocks even running the gated ones | -| **Bonus:** LLM explanations, cost-tracked, $10/day cap | Jaivardhan (+Adil for wiring) | 🔴 Not started | `llm_provider`/`llm_model`/`daily_cost_cap_usd` already in `config.py` but read by nothing; needs `llm_usage_log` table + the cap-check-before-call logic in §4.8 | - -✅ Done · 🟡 Partial / in progress · 🔴 Not started - -**Recommended build order**, based purely on dependency chains above, not owner availability: - -1. `recommendations` table + `store` functions (§3.1) — everything else writes through this -2. Endpoint with auth fix (§4.3) + algorithm (§4.1) + reasoning (§4.2) — this alone satisfies the core gate -3. `phase_gate_enabled` fixture (§6.1) — unblocks running the existing test skeleton against step 2 -4. Feedback table + endpoint + down-weighting (§3.2, §4.6) — second gate clause, independent of frontend/agent work -5. Frontend cards (§4.4) — can start as soon as step 2's response shape is stable, doesn't need to wait for 3–4 -6. Agent orchestration (§4.5) — gated on confirming the Phase 1 framework choice; can proceed in parallel with 3–5 once unblocked -7. LLM bonus (§4.8) — last, by design; it's additive and every other step already works without it - ---- - -## 8. Key Considerations - -**This phase's biggest risk is invisible until step 2 ships: the missing auth check.** Every other endpoint in this codebase (`goals.py`, `signals.py`) enforces `user_id != caller_id → 404` from day one. The recommendation stub never got that treatment because it never returned real data to protect. The moment §4.1–4.3 land, this becomes a live cross-user data leak if the fix in §4.3 is skipped or deferred "for later" — it should be written in the same commit as the real algorithm, not as a follow-up PR. - -**The <3s latency target is not a new requirement invented for this phase — it's already encoded in infrastructure that's been live since Phase 2.** `TimingMiddleware`'s own docstring states its `X-Process-Time` header exists specifically to verify this target. That means the bar was set before this module existed, and the bonus LLM path (§4.8) is the one piece of this phase actually at risk of breaking it — which is exactly why §4.8 specifies a tight LLM timeout with deterministic fallback rather than letting a slow external API call become the response time. - -**The cost cap is the bonus's only hard requirement, and it's currently zero percent enforced despite looking configured.** `daily_cost_cap_usd: float = 10.0` sitting in `config.py` reads like the cap already exists — it doesn't; nothing in the codebase queries it. Treat that field as a placeholder the bonus work fulfills, not as evidence the bonus is partially done. - -**Connection back to Phase 3, restated plainly:** this phase cannot fully satisfy its own gate (3 actions grounded in goals *and signals*) until Phase 3's time-range filtering gap is closed, because "recent activity" is the natural lens for "what should I do next." The workaround in §4.1 (fetch a large page, filter client-side) is explicitly a stopgap — it works for a demo at current data volumes, and stops being acceptable the moment either module has real production traffic, for the same reason the Phase 3 document gives for preferring server-side filtering in the first place. - ---- - -*Reviewed against the running `lpi-platform` codebase (models.py, scoring.py, smile.py, store.py, routers/recommendations.py, routers/goals.py, middleware/__init__.py, config.py, tests/test_recommendations.py, tests/conftest.py) — none of which contain a working recommendation algorithm yet, which is why this document is a build plan rather than an as-built description. The three items most worth resolving before any other Phase 4 work starts: (1) add the missing auth check to the recommendation endpoint before it serves real data; (2) add the `phase_gate_enabled` fixture so the existing test skeleton can even run; (3) confirm the agent orchestration framework from Phase 1 with Daksh before writing any orchestration code against an assumed choice.* From 56d23a8313db3d9d5729b138549ab9b18ca4ef1b Mon Sep 17 00:00:00 2001 From: Adil Islam <93443758+Adilislam0@users.noreply.github.com> Date: Sat, 20 Jun 2026 21:01:35 +0530 Subject: [PATCH 4/7] Delete Phase3_Module2_Activity_Signals_Documentation.md --- ..._Module2_Activity_Signals_Documentation.md | 511 ------------------ 1 file changed, 511 deletions(-) delete mode 100644 Phase3_Module2_Activity_Signals_Documentation.md diff --git a/Phase3_Module2_Activity_Signals_Documentation.md b/Phase3_Module2_Activity_Signals_Documentation.md deleted file mode 100644 index 6d4f6c7..0000000 --- a/Phase3_Module2_Activity_Signals_Documentation.md +++ /dev/null @@ -1,511 +0,0 @@ -# Phase 3, Module 2: Activity Signals -## Technical Documentation — Data Ingestion, Implementation & Testing/Validation - -**Repo:** `lpi-platform` · **Owner:** Adil Islam · **QA:** Daksh Garg / Jaivardhan Singh · **Demo Day:** June 27, 2026 - -> This document is written directly against the current state of `lpi-platform` (commit history through `20260615000000_signals_rls_and_log_action.sql`), not an idealized spec. Where the running code diverges from the Module 2 gate sheet, that gap is called out explicitly rather than smoothed over — you need to know what's actually shippable vs. what's still aspirational before demo day. - ---- - -## 1. Phase Overview - -### 1.1 Purpose - -Activity Signals is the **evidence layer** of the LPI platform. A Goal (Phase 2) is a stated intention — *"I want to build a startup."* A Signal is proof of work toward it — *"Alice merged a PR."* Phase 4's recommendation engine (owned by Jaivardhan) reads Goals and Signals together to decide what a user should do next, give feedback, and watch for follow-up signals. Without this module, Phase 4 has goals with no evidence to reason against — it's a recommendation engine staring at an empty evidence locker. - -### 1.2 Gate Condition (from the Module 2 spec sheet) - -> **Gate:** API ingests simulated events. Activity timeline queryable by user and time range. Core path uses simulated data — real cross-stream data is bonus territory. - -The spec sheet is explicit about *why*: other streams (Boardy, DataPro, VSAB, Altiostar) won't have stable APIs until their own Phase 3–4, so cross-stream ingestion is architecturally a *bonus*, not a blocker. The module has to stand on its own with **simulated events + intern profile data**. - -Breaking the gate into testable conditions: - -| Gate clause | Testable as | -|---|---| -| API ingests simulated events | `POST /api/v1/signals/` accepts a payload with `source: "simulated"` and persists it | -| Timeline queryable by user | `GET /api/v1/signals/` returns only the authenticated caller's rows | -| Timeline queryable by time range | `GET /api/v1/signals/?start=...&end=...` filters by `timestamp` | -| Core path = simulated | A generator script produces realistic signals without depending on any external stream | - -The third row is flagged because, as covered in §4.3, **it is not yet implemented in the running code** — this is the single largest gap between the spec sheet and what currently ships. - -### 1.3 Downstream Relevance - -``` -Goals (Phase 2) ──┐ - ├──→ Recommendation Engine (Phase 4) ──→ Instructions, feedback, -Signals (Phase 3) ──┘ follow-up monitoring -``` - -Two design decisions in this module exist *specifically* to make Phase 4 easier later: - -1. **`source` is a first-class field, added now rather than in Phase 4.** It lets the recommendation engine later run `GET /signals/?source=github_api` to weight verified real activity above simulated test data, without a breaking schema migration. -2. **There is intentionally no `goal_id` foreign key on signals.** Correlating "this signal is evidence for that goal" is a Phase 4 reasoning problem (semantic matching between a goal's description and a signal's payload), not a database constraint. Locking it to an FK now would force every signal to map to exactly one goal at ingest time, which doesn't reflect reality — one commit can be evidence for two goals, or none yet. - ---- - -## 2. Data Model - -### 2.1 Supabase Schema - -```sql --- supabase/migrations/20260611000000_create_activity_signals.sql - -CREATE TABLE IF NOT EXISTS activity_signals ( - id TEXT PRIMARY KEY, -- uuid4(), generated in Python - user_id TEXT NOT NULL DEFAULT 'default_user', - stream TEXT NOT NULL, -- WHICH domain: 'lpi', 'boardy', 'intern_proxy'... - event_type TEXT NOT NULL, -- WHAT happened: 'pr_merged', 'match_created'... - payload JSONB NOT NULL DEFAULT '{}', -- event-specific data, schema varies by event_type - source TEXT NOT NULL DEFAULT 'api', -- HOW it arrived: 'github_api'|'manual'|'simulated'|'api' - timestamp TIMESTAMPTZ NOT NULL DEFAULT NOW() -); - -CREATE INDEX IF NOT EXISTS idx_as_user_id ON activity_signals (user_id); -CREATE INDEX IF NOT EXISTS idx_as_stream ON activity_signals (stream); -CREATE INDEX IF NOT EXISTS idx_as_event_type ON activity_signals (event_type); -CREATE INDEX IF NOT EXISTS idx_as_source ON activity_signals (source); -CREATE INDEX IF NOT EXISTS idx_as_timestamp ON activity_signals (timestamp DESC); -``` - -**Why `stream` and `source` are separate columns** — this is the single most important modeling decision in the table, and it's easy to conflate the two: - -- `stream` = the **business domain** the event is *about* (lpi, boardy, datapro). This is what the recommendation engine groups by when it asks "what's happened in the area this goal cares about?" -- `source` = **how the signal got into the database** (github_api, simulated, manual, api). This is what the recommendation engine uses to *weight confidence* — a `github_api` PR merge is stronger evidence than a `simulated` test event, even if both have `stream: "lpi"`. - -Deliberately **no CHECK constraint** on either column. New streams (a future Boardy integration) or new event types onboard without a schema migration — the table grows with the product instead of gating it. The trade-off: nothing stops a typo (`"boardy"` vs `"boardy "`) from silently creating an orphan bucket; validation for known streams, if ever needed, belongs in the application layer, not the DB. - -**No RLS at table creation** — RLS was added three days later in a follow-up migration (§2.3) after the auth PR landed. Worth noting because it's a real example of schema evolving alongside the rest of the system rather than being designed perfectly up front. - -### 2.2 Pydantic Models - -```python -# src/lpi/models.py - -class SignalCreate(BaseModel): - """Request body for POST /api/v1/signals/.""" - stream: str - event_type: str - payload: dict = {} - source: str = "api" # defaults to 'api' so untagged callers don't break - - -class Signal(SignalCreate): - """Full signal — inherits SignalCreate fields + server-assigned ones.""" - id: str # uuid.uuid4(), assigned in the router, not by Postgres - user_id: str # from the JWT 'sub' claim via get_current_user() - timestamp: datetime # datetime.now(UTC), assigned in the router -``` - -`Signal` inheriting from `SignalCreate` (rather than redefining all five fields) means anything added to `SignalCreate` — like `source` was — automatically shows up in `Signal` and in the `**signal.model_dump()` spread used to build the full object in the router. One inheritance edge, one field addition, zero duplicated maintenance. - -### 2.3 RLS as Defense-in-Depth (not the primary gate) - -```sql --- supabase/migrations/20260615000000_signals_rls_and_log_action.sql -ALTER TABLE activity_signals ENABLE ROW LEVEL SECURITY; - -CREATE POLICY "Users read own signals" ON activity_signals FOR SELECT USING (auth.uid()::text = user_id); -CREATE POLICY "Users insert own signals" ON activity_signals FOR INSERT WITH CHECK (auth.uid()::text = user_id); -CREATE POLICY "Users update own signals" ON activity_signals FOR UPDATE USING (auth.uid()::text = user_id); -CREATE POLICY "Users delete own signals" ON activity_signals FOR DELETE USING (auth.uid()::text = user_id); -``` - -It's worth being precise about what this actually protects against: the FastAPI backend connects with the **service-role key**, which bypasses RLS entirely. The real authorization boundary is `Depends(get_current_user)` in the router (§3.3). RLS here only matters if the frontend (or anything else) ever queries Supabase **directly**, skipping the FastAPI layer — in that scenario, RLS is the only thing stopping cross-user reads. Don't mistake "RLS is enabled" for "the backend is enforcing per-user isolation" — those are two separate mechanisms protecting two separate attack surfaces. - ---- - -## 3. Implementation - -### 3.1 Ingest Endpoint — `POST /api/v1/signals/` - -```python -# src/lpi/routers/signals.py - -@router.post("/", response_model=Signal, status_code=status.HTTP_201_CREATED) -def ingest_signal( - signal: SignalCreate, - user_id: str = Depends(get_current_user), # ① auth happens before the body even runs -) -> Signal: - now = datetime.now(UTC) - new_signal = Signal( - id=str(uuid.uuid4()), # ② server assigns the ID — caller never sets it - user_id=user_id, # ③ from the verified JWT, not from the request body - timestamp=now, - **signal.model_dump(), # ④ stream, event_type, payload, source from the validated body - ) - - store.insert_signal(new_signal) # ⑤ Supabase INSERT - - try: - log_user_activity( # ⑥ best-effort audit log — see §6.2 for the known issue here - user_id=user_id, action="signal_ingested", resource_id=new_signal.id, - metadata={"stream": new_signal.stream, "event_type": new_signal.event_type, "source": new_signal.source}, - ) - except Exception as exc: - print(f"[ingest_signal] WARNING: logging failed for signal {new_signal.id}: {exc}") - - return new_signal -``` - -Walking the validation chain on a single request: - -1. FastAPI deserializes the JSON body against `SignalCreate` *before* the function body runs. Missing `stream` or `event_type` → automatic `422 Unprocessable Entity`, no custom code needed. -2. `Depends(get_current_user)` resolves before the handler executes — an unauthenticated request never reaches the insert logic at all (§3.3). -3. **The `id`, `user_id`, and `timestamp` are never trusted from the client.** Even if a malicious caller puts `"user_id": "someone-else"` in the body, `SignalCreate` doesn't have a `user_id` field, so Pydantic silently drops it. This is the actual enforcement mechanism for "you can't write to another user's account" — it's not a runtime check, it's that the field doesn't exist on the input schema. -4. Logging is wrapped in `try/except` rather than allowed to propagate — see §6.2, this is currently masking a real schema bug rather than handling a genuinely optional side-effect. - -### 3.2 Simulated Event Generation - -This is the part of the gate the spec sheet calls the **core path**, and it's worth being direct about its current state: as of this writing, the simulated generator described in the Module 2 task table is a **design document, not yet a script**. The plan (`docs/aditi-proxy-data-plan.md`) defines the approach clearly: - -- Real Deri/cross-stream data is blocked, so **intern profile data** (each intern's stated 3-year goal, interests, and skills) stands in as proxy ground truth. -- Each intern's skills/interests become signals with `stream: "intern_proxy"`: - - one signal per skill → `event_type: "skill_demonstrated"`, `payload: {"skill": ""}` - - one signal per interest → `event_type: "interest_identified"`, `payload: {"interest": ""}` -- Each intern's stated goals become `Goal` rows so the recommendation engine has something to correlate the signals against. - -This is a genuinely better design than a naive random-event generator: it produces a feed that's internally consistent (an intern with `skills: ["python", "tensorflow"]` gets exactly those two `skill_demonstrated` signals, not arbitrary noise), which makes Phase 4's output far easier to sanity-check during a demo than fully synthetic data would be. The implementation gap is real, though — until that script lands, the "simulated event generator" row on the gate sheet is plan, not proof. Pseudocode for the conversion, matching the documented plan: - -```python -# Planned: scripts/generate_intern_signals.py (not yet implemented) - -def intern_to_signals(intern: dict) -> list[dict]: - """Convert one intern profile into SignalCreate-shaped dicts.""" - signals = [] - for skill in intern["skills"]: - signals.append({ - "stream": "intern_proxy", - "event_type": "skill_demonstrated", - "payload": {"skill": skill}, - "source": "simulated", - }) - for interest in intern["interests"]: - signals.append({ - "stream": "intern_proxy", - "event_type": "interest_identified", - "payload": {"interest": interest}, - "source": "simulated", - }) - return signals - -# Then for each generated dict: requests.post(f"{API_BASE}/api/v1/signals/", json=signal, headers=auth_header) -``` - -### 3.3 Auth Middleware - -```python -# src/lpi/middleware/auth.py - -def get_current_user(credentials: HTTPAuthorizationCredentials | None = Depends(bearer_scheme)) -> str: - if credentials is None: - raise HTTPException(status_code=401, detail="Missing bearer token") - - token = credentials.credentials - alg = jwt.get_unverified_header(token).get("alg") - - if alg == "HS256": - key, algorithms = settings.supabase_jwt_secret, ["HS256"] # legacy Supabase projects - else: - signing_key = _get_jwks_client().get_signing_key_from_jwt(token) - key, algorithms = signing_key.key, ["ES256", "RS256"] # current Supabase projects, via JWKS - - payload = jwt.decode(token, key, algorithms=algorithms, audience="authenticated") - return payload["sub"] # the Supabase auth user's UUID -``` - -This is **real JWT verification**, not a stub — it's the upgrade from the Phase 2 `"default_user"` placeholder. Two things worth understanding: - -- It branches on the token's own `alg` header because Supabase signs tokens differently depending on project/CLI version (HS256 shared-secret on older projects, ES256/RS256 asymmetric on current ones, verified against the project's JWKS endpoint). Hardcoding one algorithm would silently break for whichever project type wasn't tested. -- `audience="authenticated"` is a deliberate check, not boilerplate — it rejects tokens issued for a different audience (e.g., a service-role token), so a leaked admin credential of the wrong type doesn't accidentally pass as a regular user. - -### 3.4 Rate Limiting - -```python -# src/lpi/middleware/rate_limit.py — fixed-window, per client IP, in-memory - -_HEALTH_LIMIT, _WRITE_LIMIT, _READ_LIMIT = 120, 30, 60 # requests per 60s window - -class RateLimitMiddleware(BaseHTTPMiddleware): - async def dispatch(self, request, call_next): - if request.method == "OPTIONS": - return await call_next(request) # CORS preflight is exempt - - limit_type, limit = ( - ("health", _HEALTH_LIMIT) if request.url.path == "/health" else - ("write", _WRITE_LIMIT) if request.method in {"POST", "PATCH", "PUT", "DELETE"} else - ("read", _READ_LIMIT) - ) - allowed, retry_after = _check(_client_ip(request), limit_type, limit) - if not allowed: - return JSONResponse(status_code=429, content={"detail": "Rate limit exceeded. Please slow down."}, - headers={"Retry-After": str(retry_after)}) - return await call_next(request) -``` - -`POST /signals/` falls under the **write** bucket (30/min/IP) — meaningfully tighter than reads, because writes are the expensive side (DB insert + audit log) and the side most exposed to a runaway ingestion script hammering the endpoint in a loop. The known limitation, called out directly in the module's own docstring: state is an in-memory dict, single-process. It works correctly for the demo's single-instance deployment; it silently stops being a real limit the moment the service runs behind more than one worker process, since each worker would track its own counters. That's a Redis-backed-counter problem for whenever the platform scales past one process — not a Phase 3 blocker, but worth flagging now so it doesn't get rediscovered the hard way under load. - -### 3.5 External (Bonus) Ingestion — GitHub Events - -```python -# scripts/ingest_github_events.py - -def map_github_event(event: dict) -> dict | None: - """GitHub raw event → SignalCreate dict. Returns None for events we don't care about.""" - event_type = event.get("type", "") - payload = event.get("payload", {}) - - if event_type == "PullRequestEvent": - pr = payload.get("pull_request", {}) - if payload.get("action") == "closed" and pr.get("merged", False): # only MERGED PRs count - return { - "stream": "lpi", "event_type": "pr_merged", "source": "github_api", - "payload": {"repo": event["repo"]["name"], "pr_number": pr.get("number"), - "title": pr.get("title", ""), "author": event["actor"]["login"]}, - } - return None - # ... PushEvent → commit_pushed, PullRequestReviewEvent → pr_reviewed, - # IssuesEvent (closed only) → issue_closed, CreateEvent (branch) → branch_created -``` - -This is the bonus path the gate sheet describes as "real cross-stream data... Boardy match events preferred." For demo day, real GitHub activity on `lpi-platform` itself stands in as the first real (non-simulated) stream, tagged `source: "github_api"` so it's distinguishable from `intern_proxy` simulated signals at query time. - -**A real bug to fix before this script is demo-safe:** `post_signal()` POSTs with no `Authorization` header at all: - -```python -def post_signal(signal_payload: dict) -> bool: - url = f"{LPI_API_BASE}/api/v1/signals/" - response = requests.post(url, json=signal_payload, timeout=5) # ← no Authorization header -``` - -This worked fine against the Phase 2 `"default_user"` stub, but now that `ingest_signal` requires `Depends(get_current_user)`, every call from this script will 401 against an auth-enforced deployment. Fix is small (mint or reuse a service-account JWT and pass it as a bearer header) but it has to happen before this script is run against staging — right now it would *appear* to work locally only because local dev may still be running against an unenforced auth state, which is exactly the kind of gap that surfaces as a surprise during a live demo. - ---- - -## 4. Query Layer — `GET /api/v1/signals/` - -```python -@router.get("/", response_model=list[Signal]) -def list_signals( - stream: str | None = Query(default=None), - event_type: str | None = Query(default=None), - source: str | None = Query(default=None), - limit: int = Query(default=50, ge=1, le=200), - offset: int = Query(default=0, ge=0), - user_id: str = Depends(get_current_user), -) -> list[Signal]: - return store.list_signals(user_id=user_id, stream=stream, event_type=event_type, - source=source, limit=limit, offset=offset) -``` - -```python -# src/lpi/store.py - -def list_signals(user_id=None, stream=None, event_type=None, source=None, limit=50, offset=0) -> list[Signal]: - query = _get_client().table("activity_signals").select("*") - if user_id: query = query.eq("user_id", user_id) - if stream: query = query.eq("stream", stream) - if event_type: query = query.eq("event_type", event_type) - if source: query = query.eq("source", source) - query = query.order("timestamp", desc=True).limit(limit).offset(offset) - return [Signal(**row) for row in query.execute().data] -``` - -### 4.1 Why filtering is server-side, not client-side - -```python -# BAD — fetches every row, filters in Python -all_rows = query.execute().data -return [s for s in all_rows if s["stream"] == stream] - -# GOOD — Postgres only sends back matching rows; this is what the code does -query = query.eq("stream", stream) -``` - -At 50 signals the difference is invisible. At 10,000 signals, client-side filtering pulls all 10,000 rows over the network to discard 9,950 of them; server-side filtering, backed by `idx_as_stream`, returns exactly the 50 that match in roughly logarithmic time. This is the entire reason the migration adds one index per filterable column (§2.1) — an index without a corresponding `.eq()` filter in the store is dead weight, and a filter without a matching index is a full table scan waiting to happen at scale. - -### 4.2 Pagination - -`limit` and `offset` translate directly to SQL `LIMIT`/`OFFSET`. Querying page 2 of a `stream=boardy` filter (`?stream=boardy&limit=50&offset=50`) costs the same as page 1 regardless of total row count — Postgres never reads rows outside the requested window. The `idx_as_timestamp DESC` index means "sort newest-first" is also free; Postgres doesn't sort after fetching, it reads the index in its existing order. - -### 4.3 Gap: Time-Range Filtering Is Not Yet Implemented - -This is the most important discrepancy to flag against the gate sheet. The spec table lists *"Query endpoints: by user, by stream, by time range, by event type"* with done-state *"Flexible filtering, paginated results."* The running `list_signals` accepts `stream`, `event_type`, `source`, `limit`, `offset` — **there is no `start`/`end` query parameter, and `store.list_signals()` has no corresponding `.gte()`/`.lte()` call on `timestamp`.** The `idx_as_timestamp` index exists and would make a range filter fast once added, but the filter itself isn't wired up. - -Concretely, this is what's missing and what closing it looks like: - -```python -# Needed addition to GET /api/v1/signals/ -start: datetime | None = Query(default=None, description="ISO-8601, inclusive lower bound"), -end: datetime | None = Query(default=None, description="ISO-8601, inclusive upper bound"), - -# Needed addition to store.list_signals() -if start: query = query.gte("timestamp", start.isoformat()) -if end: query = query.lte("timestamp", end.isoformat()) -``` - -Given the gate sheet explicitly names "queryable by user and time range" as the success condition (not just by stream/event_type), this is a pre-demo blocker, not a nice-to-have — the timeline view (Jahanvi's frontend task) will likely want a "last 7 days" or "last 24 hours" view, which has no endpoint to call without this. - -### 4.4 Bonus Endpoint — Single Signal Lookup - -```python -@router.get("/{signal_id}", response_model=Signal) -def get_signal(signal_id: str, user_id: str = Depends(get_current_user)) -> Signal: - signal = store.get_signal(signal_id) - if signal is None or signal.user_id != user_id: - raise HTTPException(status_code=404, detail=f"Signal '{signal_id}' not found.") - return signal -``` - -Note the **404, not 403**, when the signal belongs to someone else. This mirrors the pattern already established in `goals.py`: returning 403 ("forbidden") leaks the fact that a resource with that ID exists at all; 404 keeps that information from a caller probing IDs they don't own. Small detail, but it's the kind of consistency worth preserving as new endpoints get added in Phase 4. - ---- - -## 5. Execution Flow - -```mermaid -sequenceDiagram - participant Gen as Simulated Generator
(intern_proxy) / GitHub Script - participant API as FastAPI
POST /api/v1/signals/ - participant Auth as Auth Middleware
(JWT verify) - participant RL as Rate Limiter
(30 writes/min/IP) - participant Store as store.insert_signal() - participant DB as Supabase
activity_signals - participant Log as user_activity_logs
(best-effort) - participant Q as GET /api/v1/signals/
(stream/event_type/source filters) - participant FE as Frontend Timeline
(not yet built) - - Gen->>API: POST {stream, event_type, payload, source} - API->>Auth: verify Bearer JWT - Auth-->>API: user_id (sub claim) - API->>RL: check write-bucket counter - RL-->>API: allowed - API->>API: build Signal(id=uuid4(), user_id, timestamp=now) - API->>Store: insert_signal(signal) - Store->>DB: INSERT INTO activity_signals - DB-->>Store: ack - API->>Log: log_user_activity(action="signal_ingested") - Note over Log: try/except — CHECK constraint
fix exists but unapplied (§6.1) - API-->>Gen: 201 Created, full Signal JSON - - FE->>Q: GET ?stream=...&event_type=...&source=... - Q->>Auth: verify Bearer JWT - Auth-->>Q: user_id - Q->>Store: list_signals(user_id, filters, limit, offset) - Store->>DB: SELECT ... WHERE user_id=? AND filters ORDER BY timestamp DESC LIMIT/OFFSET - DB-->>Store: matching rows - Store-->>Q: list[Signal] - Q-->>FE: 200, JSON array - Note over FE: Chronological scrollable feed —
component does not exist in repo yet -``` - -**Step-by-step, in prose:** - -1. **Generation** — either the (planned) intern-proxy script or the GitHub events script produces a `SignalCreate`-shaped payload and POSTs it. -2. **Auth gate** — `get_current_user` runs before any business logic. A bad or missing token never reaches the database. -3. **Rate gate** — the write-bucket counter is checked next; a script in a tight retry loop gets throttled with a `429` + `Retry-After` rather than hammering Supabase. -4. **Server-assigned fields** — `id`, `user_id`, `timestamp` are stamped in the router, never trusted from the caller. -5. **Persistence** — one `INSERT` into `activity_signals`, with the indexes from §2.1 already in place to make future reads fast. -6. **Audit log (best-effort)** — wrapped in `try/except` so a logging failure never blocks ingestion (currently masking the issue in §6.1, not just being defensive). -7. **Query side** — the frontend (or, eventually, Phase 4) calls `GET /signals/` with whatever filters it needs; everything happens server-side in one round trip. -8. **Frontend timeline** — this is where the flow currently ends on paper. The component itself isn't in the repo yet (Jahanvi's deliverable); the backend contract it would consume is otherwise ready, modulo §4.3. - ---- - -## 6. Testing & Validation Strategy - -### 6.1 Test Infrastructure - -```python -# tests/conftest.py — the pattern every signals test relies on - -@pytest.fixture(autouse=True) -def clear_store() -> Generator[None, None, None]: - if not _supabase_available(): - pytest.skip("Local Supabase is not running. Start it with `supabase start`.") - store.clear_all() - clear_all_logs() - yield - store.clear_all() - clear_all_logs() - -@pytest.fixture -def client() -> TestClient: - test_client = TestClient(app) - test_client.headers.update({"Authorization": f"Bearer {_make_token()}"}) # real JWT, fixed test secret - return test_client -``` - -Two things make this a solid foundation rather than a brittle one: - -- **`autouse=True` on `clear_store`** means every test gets a clean table without remembering to call a cleanup helper — order-dependent test pollution (a classic flaky-suite cause) is structurally prevented. -- **The `client` fixture issues a real HS256 JWT** signed with a fixed test secret (`monkeypatch`-injected into `settings.supabase_jwt_secret`), rather than mocking `get_current_user` away. This means the auth middleware's actual decode/verify path runs in every test — a regression in JWT handling would be caught by the existing signal tests, not just a dedicated auth test file. - -### 6.2 Current Coverage — `tests/test_activity_signals.py` - -| Test | Validates | -|---|---| -| `test_ingest_returns_signal` | 201 status, server-assigned `id`/`timestamp`/`user_id`, `source` defaults to `"api"` | -| `test_ingest_with_explicit_source` | explicit `source` is preserved, not overwritten by the default | -| `test_ingest_from_different_streams` | no stream allowlist — `boardy`, `datapro`, `vsab`, `altiostar`, `security` all accepted | -| `test_list_signals_empty` | clean store → `[]`, not stale data from a prior test | -| `test_list_signals` | results scoped to the authenticated `user_id` — no cross-user leakage | -| `test_filter_by_stream` | `?stream=boardy` excludes a `datapro` signal inserted in the same test | -| `test_filter_by_source` | `?source=github_api` excludes a `simulated` signal — the exact filter Phase 4 needs | - -### 6.3 Coverage Gaps to Close Before the Gate Is Truly Met - -- **No `event_type` filter test**, even though the router exposes the parameter and the migration indexes it. Trivial to add by mirroring `test_filter_by_stream`. -- **No time-range test** — can't exist yet because the feature itself doesn't exist (§4.3). Once the `start`/`end` params are added, the test should insert signals with manually-set or mocked timestamps spanning a boundary and assert the boundary is inclusive/exclusive as documented. -- **No pagination test** — nothing currently asserts that `limit`/`offset` actually bound the result set (e.g., insert 60 signals, `limit=50`, assert exactly 50 come back and the 51st appears on `offset=50`). -- **No unauthenticated-request test specific to signals** — `test_rate_limit.py` exercises rate limiting, but there's no `test_activity_signals.py` case asserting a missing/invalid bearer token returns 401 *for this router specifically* (as opposed to relying on shared middleware tests elsewhere). -- **No test for the logging side-effect path** — given §6.1's known CHECK-constraint issue, a test that intentionally exercises the `signal_ingested` log write (rather than relying on the `try/except` to silently swallow it) would have caught the bug before it shipped to a migration fix. - -### 6.4 The Known Blocker (Already Diagnosed, Not Yet Applied) - -```sql --- Already written in 20260615000000_signals_rls_and_log_action.sql, NOT yet pushed: -ALTER TABLE user_activity_logs DROP CONSTRAINT IF EXISTS user_activity_logs_action_check; -ALTER TABLE user_activity_logs ADD CONSTRAINT user_activity_logs_action_check - CHECK (action IN ('goal_created', 'goal_updated', 'goal_deleted', 'signal_ingested')); -``` - -The original `user_activity_logs` CHECK constraint (from the Phase 2 logging migration) only permits `goal_created | goal_updated | goal_deleted`. Every `signal_ingested` log write currently violates that constraint and is silently caught by the `try/except` in `ingest_signal` — meaning **the audit trail for signal ingestion does not currently exist in the database**, even though the ingest endpoint itself works correctly. The fix is already written; it just needs `supabase db push` run against the target environment before this is considered closed. This is exactly the kind of "endpoint works, but a downstream side-effect silently fails" bug that integration tests against a real local Supabase instance (as this suite already does) are positioned to catch, once a test specifically exercises the log write rather than letting the try/except absorb it. - ---- - -## 7. Completion Checklist - -| Deliverable | Owner | Status | Notes | -|---|---|---|---| -| Activity signal model (schema + Pydantic) | Aditi | ✅ Done | `activity_signals` table + `SignalCreate`/`Signal` match the spec exactly | -| Ingest endpoint with validation | Adil | ✅ Done | Pydantic validation, JWT auth, server-assigned fields all in place | -| Simulated event generator (all 3 module types) | Aditi | 🟡 Planned, not coded | `docs/aditi-proxy-data-plan.md` defines the intern-proxy approach; script not yet written | -| Query endpoints — by user | Adil | ✅ Done | enforced via `Depends(get_current_user)` scoping, not optional | -| Query endpoints — by stream | Adil | ✅ Done | server-side `.eq()`, indexed | -| Query endpoints — by event type | Adil | ✅ Done | server-side `.eq()`, indexed, but no dedicated test yet | -| Query endpoints — by time range | Adil | 🔴 Not started | no `start`/`end` params on the router or store — see §4.3 | -| Query endpoints — pagination | Adil | ✅ Done | `limit`/`offset`, capped at 200, but untested | -| Timeline view (frontend) | Jahanvi | 🔴 Not started | no frontend code in the `lpi-platform` repo yet | -| Auth middleware on all endpoints | Jaivardhan | ✅ Done | real Supabase JWT verification (HS256 + ES256/RS256 via JWKS) | -| Rate limiting on all endpoints | Jaivardhan | ✅ Done | fixed-window per-IP, 30 writes/60 reads per minute | -| Webhook/polling design doc | Aditi | ⚪ Not reviewed in this pass | not located under `docs/` as of this writing | -| Unit + integration tests | Yashika | 🟡 Partial | ingest + stream/source filtering covered; event_type, pagination, time-range, auth-failure not yet covered | -| **Bonus:** real event data from another stream | Adil | 🟡 Partial, blocked | GitHub events script works end-to-end *except* it sends no auth header (§3.5) — will 401 once run against an auth-enforced deployment | - -✅ Done · 🟡 Partial / in progress · 🔴 Not started · ⚪ Unverified in this review - ---- - -## 8. Key Considerations - -**Why simulated data is the right core path, not a shortcut.** Every other stream (Boardy, DataPro, VSAB, Altiostar) is on its own Phase 3–4 timeline — their APIs are explicitly described as unstable until then. Gating Module 2's success on real cross-stream integration would make this module's demo-readiness hostage to four other teams' schedules. The `source` field (§2.1) is the architectural hedge that makes this safe: simulated and real signals share one schema, one ingest path, and one query surface. When a real stream does come online, nothing about the ingestion pipeline changes — only the `source` value on incoming payloads does. This is the textbook case for designing the seam before you have both sides of it. - -**Risk mitigation for unstable external APIs.** The GitHub ingestion script (§3.5) is the current external dependency, and it's structured defensively: GitHub API failures (`404`, `403` rate-limit) cause a clean exit with a printed cause rather than a stack trace, and each signal POST is wrapped so one failed insert doesn't abort the batch — the script reports a final `succeeded/failed` count and keeps going. The one gap that *isn't* defensive yet is the missing auth header (§3.5) — that's not a "what if GitHub is flaky" risk, it's a guaranteed failure the moment auth enforcement is live in the target environment, and it should be fixed before the script is pointed at anything but a local, auth-disabled instance. - -**How the signal structure supports downstream goals analysis.** The lack of a `goal_id` FK (§1.3) is a deliberate bet that correlation belongs in Phase 4's reasoning layer, not the schema. The cost of that bet is that `recommendations.py` currently returns `[]` unconditionally — the correlation logic doesn't exist yet, so there's no way to verify in this phase that the bet pays off as intended. What *is* verifiable now: the `stream`/`event_type`/`source` triple gives Phase 4 enough to query "all real LPI-stream signals from the last week" without needing the FK at all, which is the access pattern the recommendation engine actually needs first (recent evidence in a domain) before it needs the finer-grained "evidence for this specific goal" correlation. - ---- - -*Reviewed against the running `lpi-platform` codebase (models.py, store.py, routers/signals.py, middleware/auth.py, middleware/rate_limit.py, scripts/ingest_github_events.py, tests/test_activity_signals.py, and the four signals-related migrations) rather than written from the spec alone. The three items most worth fixing before demo day, in priority order: (1) wire up time-range filtering — it's named explicitly in the gate condition; (2) run `supabase db push` to apply the already-written CHECK constraint fix; (3) add an auth header to `ingest_github_events.py` before running it against anything but local/unauthenticated dev.* From 85fcaa69debd03904049492d92d2512646390ee8 Mon Sep 17 00:00:00 2001 From: Adil Islam <93443758+Adilislam0@users.noreply.github.com> Date: Sat, 20 Jun 2026 21:01:51 +0530 Subject: [PATCH 5/7] Delete LPI_Goal_to_Recommendation_Flow.md --- LPI_Goal_to_Recommendation_Flow.md | 418 ----------------------------- 1 file changed, 418 deletions(-) delete mode 100644 LPI_Goal_to_Recommendation_Flow.md diff --git a/LPI_Goal_to_Recommendation_Flow.md b/LPI_Goal_to_Recommendation_Flow.md deleted file mode 100644 index 99b4c1b..0000000 --- a/LPI_Goal_to_Recommendation_Flow.md +++ /dev/null @@ -1,418 +0,0 @@ -# LPI Platform: Goal → Activity Signal → Recommendation Integration Flow - -**Source repo:** `lpi-platform` (Life-Atlas org) · **Document scope:** Module 1 (Goals) → Module 2 (Activity Signals) → Module 3 (Recommendations) -**Verified against:** `src/lpi/`, `supabase/migrations/`, `scripts/ingest_github_events.py`, `tests/` - -This document traces a single piece of data — a user's goal — from the moment it's typed into a form to the moment a recommendation about it comes back out the other side. Every payload, endpoint, and schema snippet below is pulled directly from the current codebase, not idealized. Where the running code diverges from the diagram or from the intended design, that's called out explicitly rather than smoothed over — those gaps are exactly what doesn't show up in an architecture diagram. - ---- - -## 1. System Overview - -### 1.1 The three modules - -| Module | Router | Owner | Status in current codebase | -|---|---|---|---| -| Goals (Module 1) | `routers/goals.py` | Adil Islam (Phase 2) | Fully implemented — CRUD + SMILE phase tracking + composite scoring | -| Activity Signals (Module 2) | `routers/signals.py` | Adil Islam (Phase 3) | Fully implemented — ingest + filter + paginate, Supabase-backed | -| Recommendations (Module 3) | `routers/recommendations.py` | Jaivardhan Singh (Phase 4) | **Stub** — route is wired and returns `[]`; LangGraph reasoning not yet implemented | - -This matters for reading the rest of the document: Stages 1–6 below describe code that runs today. Stage 7 (the recommendation engine) describes the *designed* contract — the shape Module 2 was deliberately built to support — not yet a working implementation. That distinction is preserved throughout. - -### 1.2 Repo layout (the parts this flow touches) - -``` -lpi-platform/ -├── src/lpi/ -│ ├── models.py ← GoalCreate/Goal, SignalCreate/Signal, Recommendation -│ ├── smile.py ← 6-phase SMILE state machine -│ ├── scoring.py ← composite priority score -│ ├── store.py ← all Supabase reads/writes go through here -│ ├── middleware/auth.py ← Supabase JWT verification → user_id -│ └── routers/ -│ ├── goals.py -│ ├── signals.py -│ └── recommendations.py -├── scripts/ -│ └── ingest_github_events.py ← pulls GitHub Events API, POSTs signals -└── supabase/migrations/ - ├── 20260604000000_create_goals.sql - ├── 20260611000000_create_activity_signals.sql - ├── 20260607000000_create_log_tables.sql - └── 20260615000000_signals_rls_and_log_action.sql -``` - -There is **no `frontend/` directory in this repo**. The README states it directly: *"Frontend — Yet to be implemented by Jahanvi."* Section 4 of this document describes the integration contract the frontend needs to honor, derived from the API as it exists — not a description of code that was found, since none exists yet. - ---- - -## 2. Data Contracts - -These three Pydantic model pairs are the backbone of the whole flow. Each follows the same pattern: a `*Create` model defines what the caller sends, and the full model adds server-assigned fields on top. - -### 2.1 Goal - -```python -class GoalCreate(BaseModel): - title: str - description: str = "" - priority: int = 5 # 1 (low) – 10 (high) - smile_phase: SmilePhase = SmilePhase.REALITY_EMULATION - urgency_flag: bool = False # +0.20 to composite score when True - -class Goal(GoalCreate): - id: str # server-assigned uuid4 - user_id: str # from JWT `sub` claim - created_at: datetime - updated_at: datetime -``` - -### 2.2 Activity Signal - -```python -class SignalCreate(BaseModel): - stream: str # business domain: 'lpi', 'boardy', 'datapro', 'vsab'... - event_type: str # 'pr_merged', 'commit_pushed', 'match_created'... - payload: dict = {} # event-specific JSON, structure varies by event_type - source: str = "api" # 'github_api' | 'manual' | 'simulated' | 'api' - -class Signal(SignalCreate): - id: str - user_id: str - timestamp: datetime -``` - -**Important structural detail:** there is no `goal_id` field on `Signal`. Signals are *not* foreign-keyed to a specific goal at the database level. The only link between a goal and the signals that justify "progress" toward it is `user_id` plus whatever semantic correlation a reasoning layer performs at read time (matching `stream`, `payload.repo`, or text similarity against goal titles). This is a deliberate flexibility/strictness trade-off, covered in Section 6. - -### 2.3 Recommendation (target shape — not yet produced) - -```python -class Recommendation(BaseModel): - id: str - user_id: str - action: str - reasoning: str - smile_phase: SmilePhase - priority: float - source_goals: list[str] = [] # goal ids the recommendation references - source_signals: list[str] = [] # signal ids used as evidence - created_at: datetime -``` - -`source_goals` and `source_signals` are exactly how the goal↔signal link described above is meant to materialize — not as a database constraint, but as an output of the recommendation engine's reasoning step. - ---- - -## 3. End-to-End Flow - -This walks the diagram top to bottom, stage by stage, showing what enters, what the backend adds or transforms, and what exits. - -### Stage 1 — User creates a goal - -**Frontend → Backend:** -``` -POST /api/v1/goals/ -Authorization: Bearer -Content-Type: application/json - -{ - "title": "Ship Phase 3 activity signals", - "description": "Wire signal ingestion end-to-end before demo day", - "priority": 8, - "smile_phase": "contextual-intelligence", - "urgency_flag": true -} -``` - -**What `create_goal()` does (`routers/goals.py`):** -```python -new_goal = Goal( - id=str(uuid.uuid4()), - user_id=user_id, # injected by Depends(get_current_user) - created_at=now, updated_at=now, - **goal.model_dump(), # spreads title/description/priority/smile_phase/urgency_flag -) -store.insert_goal(new_goal) -log_user_activity(user_id=..., action="goal_created", resource_id=new_goal.id, metadata={...}) -``` - -| | Enters | Backend adds | Exits | -|---|---|---|---| -| Fields | `title, description, priority, smile_phase, urgency_flag` | `id` (uuid4), `user_id` (JWT `sub`), `created_at`, `updated_at` | Full `Goal` JSON, HTTP 201 | - -The `**goal.model_dump()` spread is why `urgency_flag` "just works" without any special-case code in the router — because `Goal` inherits from `GoalCreate`, any field added to the request model automatically flows through. - -### Stage 2 — Supabase `goals` table - -```sql -CREATE TABLE goals ( - id TEXT PRIMARY KEY, - user_id TEXT NOT NULL DEFAULT 'default_user', - title TEXT NOT NULL, - description TEXT NOT NULL DEFAULT '', - priority INTEGER NOT NULL DEFAULT 5, - smile_phase TEXT NOT NULL DEFAULT 'reality-emulation' - CHECK (smile_phase IN ('reality-emulation', 'concurrent-engineering', - 'collective-intelligence', 'contextual-intelligence', - 'continuous-intelligence', 'perpetual-wisdom')), - urgency_flag BOOLEAN NOT NULL DEFAULT FALSE, - created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), - updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW() -); -``` -RLS (`20260612000000_goals_rls.sql`) restricts row visibility to `auth.uid()::text = user_id`. This is defense-in-depth only — the FastAPI backend writes using the **service-role key**, which bypasses RLS by design (`store.py::_get_client()`). RLS only matters if something queries Supabase directly (e.g. a future frontend using `supabase-js` against the table instead of going through the API). - -### Stage 3 — Work happens on the linked GitHub repository - -This is the step with no API call at all — a developer merges a PR or pushes a commit to `Life-Atlas/lpi-platform` on GitHub. GitHub itself records this as a public event, retrievable from `GET https://api.github.com/repos/{owner}/{repo}/events` (the **GitHub Events API**, not a webhook — see Section 6 for why that distinction matters). - -There is no automatic trigger here. Nothing in this repo subscribes to GitHub webhooks; ingestion is **pull-based and currently manual** (Stage 4 is a script someone runs, not a listener that fires on push). - -### Stage 4 — GitHub ingestion script converts events to signals - -`scripts/ingest_github_events.py` is invoked manually (`python scripts/ingest_github_events.py`) and does four things: - -1. **Fetch:** `GET /repos/Life-Atlas/lpi-platform/events` (last ~30 public events; 60 req/hr unauthenticated, 5000 req/hr with `GITHUB_TOKEN`). -2. **Filter + map:** only event types representing *completed* work are kept. - -```python -# PullRequestEvent → only if action == "closed" and pr["merged"] is True -{ - "stream": "lpi", "event_type": "pr_merged", "source": "github_api", - "payload": { - "repo": repo, "pr_number": pr["number"], "title": pr["title"], - "author": actor, "merged_at": pr["merged_at"], - "additions": pr["additions"], "deletions": pr["deletions"], - "changed_files": pr["changed_files"], - "body_snippet": pr["body"][:200], - }, -} -``` -Other mapped types: `PushEvent → commit_pushed`, `PullRequestReviewEvent → pr_reviewed`, `IssuesEvent → issue_closed` (closed only), `CreateEvent → branch_created` (branches only). Everything else (`WatchEvent`, `ForkEvent`, etc.) is dropped. - -3. **POST each mapped event:** -```python -def post_signal(signal_payload: dict) -> bool: - response = requests.post(f"{LPI_API_BASE}/api/v1/signals/", json=signal_payload, timeout=5) - return response.status_code in (200, 201) -``` - -| | Enters (per GitHub event) | Transform | Exits (per signal) | -|---|---|---|---| -| Source | Raw GitHub event JSON (`type`, `payload`, `actor`, `repo`) | Event-type-specific field extraction | `SignalCreate`-shaped dict, `source="github_api"` | - -> **Gap worth flagging:** `post_signal()` calls `requests.post()` with no `Authorization` header. Since `signals.py::ingest_signal` requires `Depends(get_current_user)`, running this script today against an auth-protected deployment returns `401 Unauthorized` for every event. The script was written before JWT auth was wired onto the signals router and hasn't been updated to attach a service token — this needs a fix before it can run against any environment with auth enabled. - -### Stage 5 — Signals router persists the event - -```python -new_signal = Signal( - id=str(uuid.uuid4()), - user_id=user_id, - timestamp=datetime.now(UTC), - **signal.model_dump(), # stream, event_type, payload, source -) -store.insert_signal(new_signal) - -try: - log_user_activity(user_id=user_id, action="signal_ingested", resource_id=new_signal.id, ...) -except Exception as exc: - print(f"[ingest_signal] WARNING: logging failed for signal {new_signal.id}: {exc}") -``` - -| | Enters | Backend adds | Exits | -|---|---|---|---| -| Fields | `stream, event_type, payload, source` | `id`, `user_id`, `timestamp` | Full `Signal` JSON, HTTP 201 | - -The `try/except` around logging exists because of a real schema bug: `user_activity_logs` originally had a `CHECK` constraint allowing only `'goal_created' | 'goal_updated' | 'goal_deleted'`. Calling `log_user_activity(action="signal_ingested")` against that constraint raises a Postgres exception. The signal still gets stored in `activity_signals` (the insert that matters), but the activity-log write silently fails and prints a warning instead of breaking the endpoint. - -The fix already exists as a migration — `20260615000000_signals_rls_and_log_action.sql` — but only takes effect once it's actually applied: -```sql -ALTER TABLE user_activity_logs DROP CONSTRAINT IF EXISTS user_activity_logs_action_check; -ALTER TABLE user_activity_logs ADD CONSTRAINT user_activity_logs_action_check - CHECK (action IN ('goal_created', 'goal_updated', 'goal_deleted', 'signal_ingested')); -``` -Until `supabase db push` runs this migration against the target environment, every signal ingestion logs a warning to stdout instead of recording a clean audit trail entry — functionally harmless (signals still land in the table), but it means `user_activity_logs` undercounts signal activity until applied. - -### Stage 6 — Supabase `activity_signals` table - -```sql -CREATE TABLE activity_signals ( - id TEXT PRIMARY KEY, - user_id TEXT NOT NULL DEFAULT 'default_user', - stream TEXT NOT NULL, - event_type TEXT NOT NULL, - payload JSONB NOT NULL DEFAULT '{}', - source TEXT NOT NULL DEFAULT 'api', - timestamp TIMESTAMPTZ NOT NULL DEFAULT NOW() -); -CREATE INDEX idx_as_user_id ON activity_signals (user_id); -CREATE INDEX idx_as_stream ON activity_signals (stream); -CREATE INDEX idx_as_event_type ON activity_signals (event_type); -CREATE INDEX idx_as_source ON activity_signals (source); -CREATE INDEX idx_as_timestamp ON activity_signals (timestamp DESC); -``` -No `CHECK` constraint on `stream` or `event_type` — intentionally. New business streams (`vsab`, `altiostar`, etc.) or new event types can be added without a migration. `source` is also unconstrained at the DB layer despite having well-known values (`github_api`, `manual`, `simulated`, `api`) — validation, if any, is application-level. - -### Stage 7 — Recommendation engine reads signals + goals *(designed, not yet built)* - -The intended read pattern, per the code's own docstrings (`signals.py`, `scoring.py`): -``` -GET /api/v1/signals/?source=github_api&limit=20 # exclude simulated/test signals -GET /api/v1/goals/ # already sorted by composite score -``` -`store.list_signals(source="github_api", ...)` runs a server-side `WHERE source = 'github_api'` — the rec engine never has to filter simulated data in Python. The composite goal score (Section 2.1's `priority/phase/urgency` weighting, computed in `scoring.py`) means goals are already ranked by importance before the rec engine even looks at signals. - -What's *not* built: the actual reasoning step that correlates a `pr_merged` signal with `payload.repo == "lpi-platform"` to a specific goal like "Ship Phase 3 activity signals," and produces `Recommendation.reasoning` text plus `source_goals`/`source_signals` references. `recommendations.py` today: -```python -@router.get("/{user_id}", response_model=list[Recommendation]) -def get_recommendations(user_id: str, limit: int = Query(default=3, ge=1, le=10)) -> list[Recommendation]: - return [] # Phase 4 placeholder — confirms the route is wired, nothing more -``` - -### Stage 8 — Recommendation surfaced to the frontend - -``` -GET /api/v1/recommendations/{user_id}?limit=3 -→ [] today, eventually: -[ - { - "action": "Open a PR closing the activity_signals CHECK constraint gap", - "reasoning": "3 github_api signals show signal-ingestion work in progress; this is the blocking issue before demo day.", - "smile_phase": "contextual-intelligence", - "priority": 6.8, - "source_goals": [""], - "source_signals": ["", ""] - } -] -``` - ---- - -## Scenario 1 — Production Flow (full lifecycle, all components operational) - -``` -┌──────────────┐ POST /api/v1/goals/ ┌─────────────────┐ -│ Frontend │ ─────────────────────────▶│ FastAPI goals │ -│ goal form │ GoalCreate JSON │ router │ -└──────────────┘ └────────┬────────┘ - │ insert_goal() - ▼ - ┌─────────────────┐ - │ Supabase: goals │ - └────────┬────────┘ - │ (informational — - │ no automated trigger) - ▼ -┌──────────────┐ PR merged / push ┌─────────────────┐ -│ GitHub repo │ ─────────────────────────▶│ GitHub Events │ -│ │ │ API (pull-based) │ -└──────────────┘ └────────┬────────┘ - │ python scripts/ingest_github_events.py - ▼ - ┌─────────────────┐ - │ ingestion script│ - │ maps event→signal│ - └────────┬────────┘ - │ POST /api/v1/signals/ - ▼ - ┌─────────────────┐ - │ signals router │ - │ ingest_signal() │ - └────────┬────────┘ - │ insert_signal() - ▼ - ┌─────────────────────┐ - │ Supabase: │ - │ activity_signals │ - └────────┬─────────────┘ - │ GET ?source=github_api - ▼ - ┌─────────────────────┐ - GET /api/v1/goals/ ───────▶│ Recommendation │ - (goals context) │ engine (Phase 4) │ - └────────┬─────────────┘ - │ GET /api/v1/recommendations/{user_id} - ▼ - ┌─────────────────┐ - │ Frontend: "next │ - │ step" + phase │ - └─────────────────┘ -``` - -**Concrete trace:** - -1. A user creates `Goal{title: "Ship Phase 3 activity signals", priority: 8, smile_phase: "contextual-intelligence", urgency_flag: true}`. Composite score = `8×0.5 + 4×0.3 + 1×0.2 = 5.40` — this goal now sorts near the top of `GET /api/v1/goals/`. -2. Over the next few days the team merges PR #18 ("Supabase dual-sync logging") into `lpi-platform`. -3. Someone runs `python scripts/ingest_github_events.py`. It fetches GitHub events, finds the merged PR, and POSTs: - ```json - {"stream": "lpi", "event_type": "pr_merged", "source": "github_api", - "payload": {"repo": "lpi-platform", "pr_number": 18, "title": "Supabase dual-sync logging", "author": "Adilislam0"}} - ``` -4. The signal lands in `activity_signals` with a server-assigned `id`, `user_id`, and `timestamp`. -5. The (future) recommendation engine queries `GET /api/v1/signals/?source=github_api&limit=20` and `GET /api/v1/goals/`, semantically matches the `pr_merged` signal's `payload.title` against the goal's title, and emits a `Recommendation` referencing both ids. -6. The frontend calls `GET /api/v1/recommendations/{user_id}` and renders: *"Recent merge confirms progress on 'Ship Phase 3 activity signals' — consider advancing to continuous-intelligence."* - ---- - -## Scenario 2 — Demo / MVP (3–5 day timeline) - -The full chain above has one component that doesn't exist yet (Stage 7's reasoning step) and one known bug blocking a clean demo (Stage 5's `CHECK` constraint, plus Stage 4's missing auth header). A 3–5 day MVP should **prove Stages 1–6 work end-to-end with real data**, and present the recommendation step as a thin, honest placeholder rather than trying to ship the LangGraph reasoning layer under time pressure. - -``` -Day 1 Day 2 Day 3 Day 4-5 -────── ────── ────── ─────── -Apply pending → Fix ingestion → Build minimal → Rehearse + -migration script auth + "Activity Feed" buffer -(supabase db run it against UI: list raw -push) so signal_ a real merged PR signals chrono- -ingested logging ologically, no -stops warning reasoning layer -``` - -**Day 1 — Unblock the data path** -- Run `supabase db push` to apply `20260615000000_signals_rls_and_log_action.sql`. This silences the `CHECK` constraint warning and gives `user_activity_logs` a clean record of `signal_ingested` events. -- Verify with: `SELECT * FROM user_activity_logs WHERE action = 'signal_ingested' ORDER BY logged_at DESC;` - -**Day 2 — Make the ingestion script actually work against auth** -- Add a service-role (or test-user) bearer token to `post_signal()`'s request headers. Without this, every POST from the script 401s the moment auth is enforced. -- Run the script against the real repo, confirm signals land: `GET /api/v1/signals/?source=github_api`. - -**Day 3 — Build the smallest honest UI** -- Skip Module 3 entirely for the demo. Instead of recommendations, build a simple "Recent Activity" panel that calls `GET /api/v1/signals/?stream=lpi&source=github_api&limit=10` and renders each row as `"{author} merged PR #{pr_number}: {title}"` or `"{author} pushed {commit_count} commit(s) to {branch}"`. This is real, verifiable data — not a simulated recommendation — which is a stronger demo artifact than a stubbed `[]` recommendations call or fabricated reasoning text. -- Pair it with the existing `GET /api/v1/goals/` list (already fully functional, already sorted by composite score) so the demo shows *goals* and *evidence of work* side by side, even without the connecting reasoning layer. - -**Day 4–5 — Buffer** -- Re-run the full test suite (`pytest tests/ -v`) against a freshly migrated local Supabase instance to catch any regression from the constraint fix. -- Rehearse the narrative: "here's the goal, here's the real GitHub activity feeding it, the recommendation layer that connects them is Phase 4 — in progress." - -**What this scenario deliberately excludes:** LangGraph reasoning, automatic goal-signal correlation, and webhook-based (push) ingestion. All three are real Phase 4 work, not a 3–5 day scope. - ---- - -## 4. Frontend Integration Contract - -No frontend code exists in this repo yet — this section describes the contract a frontend implementation needs to satisfy, derived from the API surface above, not a description of existing UI code. - -**Authentication.** Every endpoint except `/health` requires `Authorization: Bearer `. The README's stated pattern is `supabase-js`'s `auth.signUp` / `signInWithPassword`, with the resulting `access_token` attached to every backend call. `middleware/auth.py` accepts both HS256 (shared-secret) and ES256/RS256 (JWKS-verified) tokens depending on the Supabase project's signing configuration — the frontend doesn't need to know which; it just forwards whatever Supabase's client SDK issues. - -**Displaying goals.** `GET /api/v1/goals/` already returns results sorted by composite score (`sort_goals_by_score()` runs server-side) — the frontend should *not* re-sort client-side, or it'll fight the intended urgency/phase weighting. Optional filter: `?smile_phase=reality-emulation` to scope a view to one phase. There is currently no field on `Goal` linking it to a specific GitHub repo — "linked repositories" as a UI concept would need to be inferred from `payload.repo` on associated signals (matched by `stream`), since no structural link exists yet. Flag this for whoever owns the frontend goal-detail view. - -**Displaying activity signals.** `GET /api/v1/signals/?stream=lpi&source=github_api&limit=20` for a real-only, paginated activity feed. `source=github_api` specifically excludes `simulated` and `manual` test entries — useful for a "verified activity" view distinct from a raw/debug view. Pagination is `limit`/`offset`, not cursor-based; page 2 is `?offset=50` with the same `limit`. - -**Displaying recommendations.** `GET /api/v1/recommendations/{user_id}?limit=3` currently always returns `[]`. The frontend should build against the `Recommendation` schema (Section 2.3) now so that when Phase 4 ships, no contract changes are needed — but should render gracefully on an empty array rather than treating it as an error state. - -**Triggering ingestion.** There is no API endpoint or button-triggered action that runs `ingest_github_events.py` — it's a manually-invoked Python script today, not something the frontend can call. If a "Sync GitHub Activity" button is wanted in the UI, that script would need to be wrapped behind a new authenticated endpoint first; right now it only runs from a developer's terminal. - ---- - -## 5. Known Gaps & Architectural Decisions Summary - -| Item | Type | Detail | -|---|---|---| -| `user_activity_logs` CHECK constraint | Bug (fix exists, not yet applied) | Migration `20260615000000` adds `'signal_ingested'`; needs `supabase db push` | -| `ingest_github_events.py` missing auth header | Bug | `post_signal()` sends no `Authorization` header; will 401 against an auth-enforced deployment | -| No `goal_id` FK on `Signal` | Deliberate design | Correlation is meant to happen via reasoning (Phase 4), not a rigid foreign key — keeps signal ingestion source-agnostic | -| Recommendation engine returns `[]` | Known incomplete (Phase 4 in progress) | Route is wired so dependent code doesn't 500; reasoning logic not yet written | -| GitHub ingestion is pull/manual, not webhook-driven | Deliberate (for now) | Simpler to build and demo; no public endpoint needed for GitHub to call back to | -| RLS on `goals`/`activity_signals` | Defense-in-depth | Backend uses service-role key and bypasses RLS by design; RLS only protects against direct (non-backend) Supabase access | From cb4aa03f9b9531bc8e3226be2aafd05d51ab7277 Mon Sep 17 00:00:00 2001 From: Adilislam0 Date: Sun, 21 Jun 2026 13:20:46 +0530 Subject: [PATCH 6/7] Phase 3 fixes (time-range filter, audit logging) + Phase 4 Wave 1: recommendations endpoint --- .env.example | 1 + README.md | 70 +++ lpi_testing_guide.md | 789 +++++++++++++++++++++++++++++ src/lpi/recommendation_engine.py | 319 ++++++++++++ src/lpi/routers/recommendations.py | 112 +++- src/lpi/routers/signals.py | 89 ++-- src/lpi/store.py | 111 +++- src/lpi/utils/logging.py | 136 +++-- tests/test_activity_signals.py | 24 +- tests/test_recommendations.py | 478 ++++++++++++++++- 10 files changed, 1993 insertions(+), 136 deletions(-) create mode 100644 lpi_testing_guide.md create mode 100644 src/lpi/recommendation_engine.py diff --git a/.env.example b/.env.example index 7927998..d284fad 100644 --- a/.env.example +++ b/.env.example @@ -7,3 +7,4 @@ LLM_PROVIDER=anthropic LLM_MODEL=claude-sonnet-4-20250514 ANTHROPIC_API_KEY=sk-ant-... DAILY_COST_CAP_USD=10.0 +ADMIN_USER_IDS=your-admin-user-id \ No newline at end of file diff --git a/README.md b/README.md index cbca343..be89299 100644 --- a/README.md +++ b/README.md @@ -270,7 +270,77 @@ pytest tests/test_smoke.py -v # fast boot + invariant checks pytest tests/test_activity_signals.py -v # activity signa endpoints ``` +## Activity Signals — Time-Range Filtering +`GET /api/v1/signals/` supports filtering by a UTC timestamp window in +addition to `stream`, `event_type`, and `source`: + +```bash +# Everything since June 13 +GET /api/v1/signals/?start=2026-06-13T00:00:00Z + +# A closed one-week window +GET /api/v1/signals/?start=2026-06-13T00:00:00Z&end=2026-06-20T00:00:00Z +``` + +Both bounds are inclusive and optional independently. All filtering +happens server-side in Postgres via `idx_as_timestamp` — no client-side +filtering, so response time doesn't degrade as the table grows. + +--- + +## Recommendations Endpoint (Phase 4 — Wave 1) + +`GET /api/v1/recommendations/{user_id}` returns up to `limit` (default 3, +max 10) next-action recommendations, sorted by priority descending. + +**Status:** Wave 1 — deterministic mock data, real auth, stable contract. +Wave 2 (Jaivardhan) will replace the mock data source with real +goal/signal-grounded reasoning without changing the response shape. + +```bash +curl http://127.0.0.1:8000/api/v1/recommendations/intern-a-demo-profile \ + -H "Authorization: Bearer " +``` + +Response: +```json +[ + { + "id": "uuid", + "user_id": "intern-a-demo-profile", + "action": "Ingest this week's GitHub activity as a signal", + "reasoning": "...", + "smile_phase": "collective-intelligence", + "priority": 5.60, + "source_goals": [], + "source_signals": [], + "created_at": "2026-06-21T..." + } +] +``` + +`priority` uses the same `[0.80, 7.00]` scale as goals (`lpi/scoring.py`), +so frontend priority badges can be reused as-is. + +**Note:** `user_id` is a path parameter, not derived from the JWT — any +authenticated caller can request recommendations for any user_id. This is +intentional for the demo (one session, multiple seeded intern profiles) +and should be revisited before any real multi-tenant rollout. + +--- + +## Required setup before pulling this branch + +```bash +supabase db push +``` + +This applies `20260615000000_signals_rls_and_log_action.sql`, which adds +`signal_ingested` to the `user_activity_logs` CHECK constraint. Without +this migration, signal ingestion will still work but its audit log entry +will silently fail to write (now surfaced via `logger.exception()` instead +of a swallowed `print()`). --- diff --git a/lpi_testing_guide.md b/lpi_testing_guide.md new file mode 100644 index 0000000..50c1a16 --- /dev/null +++ b/lpi_testing_guide.md @@ -0,0 +1,789 @@ +# LPI Platform — Backend Testing & Verification Guide +> Scope: Authentication → Goal CRUD → Activity Signals +> Stack: FastAPI · Pydantic v2 · Supabase · pytest +> Base URL (local): `http://localhost:8000` + +--- + +## Part 1 — Implementation Review Against plan.md + +### What Should Be Done By Now (Phase 2 Module 1) + +Based on the plan gate criteria: + +| Gate Criterion | Expected Status | Verify With | +|---|---|---| +| 10+ seeded goals in DB | ✅ 15 seeded | `GET /goals` count | +| Priority scoring working | ✅ per PR #12 | `GET /goals?sort=priority` | +| SMILE transitions logged | ✅ `log_transition` in store.py | POST to signal endpoint | +| `test_goal_crud.py` 100% green | ⚠️ 4 failing | `pytest -v tests/test_goal_crud.py` | +| All PRs merged | ⚠️ PRs #14, #15 under review | Check GitHub | +| JWT auth working | ⚠️ Aditi implementing | `POST /auth/login` | + +### Known Deviations / Risk Areas + +1. **JWT auth (Aditi's PR)** — Not yet merged. All protected endpoints will return 401 until this lands. Test auth endpoints separately once merged. +2. **Signals endpoint (`signals.py`)** — Previously returned HTTP 501 (stub). Confirm it's been promoted to real implementation before testing ingestion. +3. **Pydantic v2 migration** — `model_config` fix applied in `config.py`. Watch for any remaining v1-style validators (`@validator`) in models not yet migrated. +4. **Supabase `.delete()` filter requirement** — `clear_all()` uses `.neq()` sentinel. Confirm this pattern holds in any new store methods added via PRs #14/#15. +5. **SMILE phase label** — Confirmed 6-phase lifecycle: `sense → model → intervene → learn → evolve` (plus the corrected 6th phase from PR #17). Verify conftest fixture uses `"reality-emulation"` not `"sense"` as the test phase. + +--- + +## Part 2 — Testing Commands + +> **Prerequisites** +> ```bash +> # From the lpi-platform directory +> cd C:\Users\Aadil_islam\Desktop\Projects\Internship\Winniio\lpi-platform +> +> # Start the server (one terminal) +> uvicorn app.main:app --reload --port 8000 +> +> # Confirm it's alive (second terminal) +> curl http://localhost:8000/health +> # Expected: {"status": "ok"} or {"status": "healthy"} +> ``` + +--- + +### 2A — Authentication + +#### 1. Register / Create User (if endpoint exists) +```bash +curl -X POST http://localhost:8000/auth/register \ + -H "Content-Type: application/json" \ + -d '{"email": "test@lpi.dev", "password": "Test1234!", "name": "Test User"}' +``` +**Tests:** User creation flow +**Expected:** `201 Created` with user object or `{"message": "User created"}` +**Failure signs:** `422 Unprocessable Entity` (schema mismatch), `500` (Supabase connection issue) + +#### 2. Login — Get JWT Token +```bash +curl -X POST http://localhost:8000/auth/login \ + -H "Content-Type: application/json" \ + -d '{"email": "test@lpi.dev", "password": "Test1234!"}' \ + -v +``` +**Tests:** JWT generation +**Expected:** `200 OK` with `{"access_token": "", "token_type": "bearer"}` +**Failure signs:** `401 Unauthorized`, `404 Not Found` (route missing), `500` (Supabase auth not configured) + +#### 3. Capture Token for Subsequent Calls +```bash +# Windows CMD +for /f "tokens=*" %i in ('curl -s -X POST http://localhost:8000/auth/login -H "Content-Type: application/json" -d "{\"email\": \"test@lpi.dev\", \"password\": \"Test1234!\"}" ^| python -c "import sys,json; print(json.load(sys.stdin)[\"access_token\"])"') do set TOKEN=%i +echo %TOKEN% + +# PowerShell +$resp = Invoke-RestMethod -Uri "http://localhost:8000/auth/login" -Method POST -ContentType "application/json" -Body '{"email":"test@lpi.dev","password":"Test1234!"}' +$TOKEN = $resp.access_token +echo $TOKEN + +# Git Bash / WSL +TOKEN=$(curl -s -X POST http://localhost:8000/auth/login \ + -H "Content-Type: application/json" \ + -d '{"email":"test@lpi.dev","password":"Test1234!"}' \ + | python -c "import sys,json; print(json.load(sys.stdin)['access_token'])") +echo $TOKEN +``` + +#### 4. Access Protected Route with Token +```bash +curl http://localhost:8000/goals \ + -H "Authorization: Bearer $TOKEN" +``` +**Tests:** JWT middleware is enforcing auth +**Expected:** `200 OK` with goals list +**Failure signs:** `401` with `{"detail": "Not authenticated"}` (token not passed), `403` (token invalid/expired) + +#### 5. Token with Invalid Credentials +```bash +curl -X POST http://localhost:8000/auth/login \ + -H "Content-Type: application/json" \ + -d '{"email": "test@lpi.dev", "password": "WrongPassword"}' +``` +**Tests:** Negative auth case +**Expected:** `401 Unauthorized` with error message +**Failure signs:** `200 OK` (auth bypass bug), `500` (unhandled exception on bad credentials) + +#### 6. Token Refresh (if implemented) +```bash +curl -X POST http://localhost:8000/auth/refresh \ + -H "Authorization: Bearer $TOKEN" +``` +**Tests:** Refresh token flow +**Expected:** New `access_token` in response +**Failure signs:** `404` (not implemented yet — acceptable if Aditi's PR not merged) + +--- + +### 2B — Goal CRUD Endpoints + +Replace `$TOKEN` with your captured token in all commands below. + +#### 7. Create a Goal (POST) +```bash +curl -X POST http://localhost:8000/goals \ + -H "Authorization: Bearer $TOKEN" \ + -H "Content-Type: application/json" \ + -d '{ + "title": "Complete LPI Module 1", + "description": "Finish Goal Registry implementation", + "priority": 1, + "smile_phase": "sense", + "due_date": "2026-06-27T00:00:00Z" + }' +``` +**Tests:** Goal creation, Pydantic v2 validation, Supabase insert +**Expected:** `201 Created` with goal object including `id`, `created_at` +**Failure signs:** +- `422` → schema mismatch (check field names vs your GoalCreate model) +- `500` → Supabase connection or RLS policy blocking insert + +#### 8. List All Goals (GET) +```bash +curl http://localhost:8000/goals \ + -H "Authorization: Bearer $TOKEN" +``` +**Tests:** Goal retrieval, confirms 15 seeded goals exist +**Expected:** `200 OK` with array of ≥15 goals +**Failure signs:** Empty array `[]` (seeding didn't run), `401` (auth middleware issue) + +#### 9. Get Goals Count (verify seeding) +```bash +curl http://localhost:8000/goals \ + -H "Authorization: Bearer $TOKEN" \ + | python -c "import sys,json; goals=json.load(sys.stdin); print(f'Total goals: {len(goals)}')" +``` +**Expected:** `Total goals: 15` (or more) + +#### 10. Get Single Goal by ID (GET) +```bash +# First grab an ID from the list +GOAL_ID="" + +curl http://localhost:8000/goals/$GOAL_ID \ + -H "Authorization: Bearer $TOKEN" +``` +**Tests:** Single-resource fetch, UUID routing +**Expected:** `200 OK` with single goal object +**Failure signs:** `404 Not Found` (ID doesn't exist or routing broken) + +#### 11. Update a Goal (PUT / PATCH) +```bash +curl -X PATCH http://localhost:8000/goals/$GOAL_ID \ + -H "Authorization: Bearer $TOKEN" \ + -H "Content-Type: application/json" \ + -d '{ + "title": "Complete LPI Module 1 — UPDATED", + "smile_phase": "model" + }' +``` +**Tests:** Partial update, SMILE phase transition +**Expected:** `200 OK` with updated goal; `smile_phase` changed to `"model"` +**Failure signs:** `422` (PATCH not accepting partial body — try PUT with full body instead), `404` (wrong ID) + +#### 12. Get Goals Sorted by Priority +```bash +curl "http://localhost:8000/goals?sort=priority&order=asc" \ + -H "Authorization: Bearer $TOKEN" +``` +**Tests:** Priority scoring feature (Phase 2 gate criterion) +**Expected:** Goals ordered by priority field ascending +**Failure signs:** `422` (query param not supported), unordered response + +#### 13. Filter Goals by SMILE Phase +```bash +curl "http://localhost:8000/goals?smile_phase=sense" \ + -H "Authorization: Bearer $TOKEN" +``` +**Tests:** Filtering by phase +**Expected:** Only goals in `sense` phase +**Failure signs:** All goals returned (filter ignored), `422` + +#### 14. Delete a Goal (DELETE) +```bash +# Create a throwaway goal first +THROWAWAY=$(curl -s -X POST http://localhost:8000/goals \ + -H "Authorization: Bearer $TOKEN" \ + -H "Content-Type: application/json" \ + -d '{"title": "DELETE ME", "priority": 99, "smile_phase": "sense"}' \ + | python -c "import sys,json; print(json.load(sys.stdin)['id'])") + +# Now delete it +curl -X DELETE http://localhost:8000/goals/$THROWAWAY \ + -H "Authorization: Bearer $TOKEN" \ + -v +``` +**Tests:** Goal deletion +**Expected:** `200 OK` or `204 No Content` +**Failure signs:** `405 Method Not Allowed` (DELETE not implemented), `500` + +#### 15. Delete Non-Existent Goal (Negative Case) +```bash +curl -X DELETE http://localhost:8000/goals/00000000-0000-0000-0000-000000000000 \ + -H "Authorization: Bearer $TOKEN" +``` +**Expected:** `404 Not Found` +**Failure signs:** `500` (unhandled exception), `200` (false success) + +--- + +### 2C — Activity Signals Endpoint + +#### 16. Check Signals Endpoint is Live +```bash +curl http://localhost:8000/signals \ + -H "Authorization: Bearer $TOKEN" +``` +**Tests:** Route exists and responds +**Expected:** `200 OK` (list of signals) or `405` (GET not allowed — signals may be POST-only) +**Failure signs:** `501 Not Implemented` → signals.py still stubbed; `404` → route not registered in main.py + +#### 17. Ingest a Single Activity Signal (POST) +```bash +curl -X POST http://localhost:8000/signals \ + -H "Authorization: Bearer $TOKEN" \ + -H "Content-Type: application/json" \ + -d '{ + "goal_id": "'$GOAL_ID'", + "actor": "test@lpi.dev", + "event_type": "goal_updated", + "payload": { + "field_changed": "smile_phase", + "old_value": "sense", + "new_value": "model" + }, + "timestamp": "2026-06-15T10:30:00Z" + }' +``` +**Tests:** Signal ingestion, Timestamp/Actor/Event schema +**Expected:** `201 Created` with signal ID +**Failure signs:** `422` (schema mismatch — check your SignalCreate model field names), `404` (goal_id not found if FK enforced) + +#### 18. Ingest Signal Without Auth (Negative Case) +```bash +curl -X POST http://localhost:8000/signals \ + -H "Content-Type: application/json" \ + -d '{"actor": "hacker", "event_type": "test"}' +``` +**Expected:** `401 Unauthorized` +**Failure signs:** `201` (auth not enforced on signals endpoint) + +#### 19. Retrieve Signals for a Goal +```bash +curl "http://localhost:8000/signals?goal_id=$GOAL_ID" \ + -H "Authorization: Bearer $TOKEN" +``` +**Tests:** Signal retrieval and filtering by goal +**Expected:** Array containing the signal from step 17 +**Failure signs:** Empty array (signal not persisted), `422` (query param not supported) + +#### 20. Ingest Signal with Missing Required Fields (Negative Case) +```bash +curl -X POST http://localhost:8000/signals \ + -H "Authorization: Bearer $TOKEN" \ + -H "Content-Type: application/json" \ + -d '{"actor": "test@lpi.dev"}' +``` +**Expected:** `422 Unprocessable Entity` with validation error details +**Failure signs:** `500` (Pydantic not catching the missing field — model may have Optional where it should be Required) + +#### 21. Verify SMILE Transition Logging +```bash +# Transition a goal through a phase change +curl -X PATCH http://localhost:8000/goals/$GOAL_ID \ + -H "Authorization: Bearer $TOKEN" \ + -H "Content-Type: application/json" \ + -d '{"smile_phase": "intervene"}' + +# Now check signals were auto-created by log_transition +curl "http://localhost:8000/signals?goal_id=$GOAL_ID" \ + -H "Authorization: Bearer $TOKEN" \ + | python -c "import sys,json; sigs=json.load(sys.stdin); print(f'Signals logged: {len(sigs)}')" +``` +**Tests:** `log_transition()` in store.py auto-creates signal on SMILE phase change +**Expected:** At least 1 signal with `event_type` like `"smile_transition"` or `"phase_changed"` + +--- + +## Part 3 — Addressing the 4 Failing pytest Tests + +### Step 1: Identify Which 4 Tests Are Failing + +```bash +cd C:\Users\Aadil_islam\Desktop\Projects\Internship\Winniio\lpi-platform + +# Run full suite and capture output +pytest tests/ -v --tb=short 2>&1 | tee test_output.txt + +# Show only failures +pytest tests/ -v --tb=short 2>&1 | grep -E "FAILED|ERROR" +``` + +### Step 2: Run Failing Tests in Isolation + +Once you know which 4 are failing, run each individually: + +```bash +# Pattern: pytest tests/test_file.py::TestClass::test_function -v --tb=long + +# Example — if test_goal_crud.py has failures: +pytest tests/test_goal_crud.py -v --tb=long -s + +# Run a specific test by name keyword: +pytest tests/ -v -k "test_create_goal" --tb=long -s + +# Run with full traceback and print statements: +pytest tests/ -v --tb=long -s --no-header -rN +``` + +### Step 3: Common Root Causes and Diagnostics + +#### Cause A: conftest.py fixture using wrong SMILE phase label + +```bash +# Check the fixture +grep -n "sense\|reality-emulation\|smile_phase" tests/conftest.py +``` +**Fix:** Ensure `"reality-emulation"` is used as the test phase name (from PR #17 correction). If `"sense"` is hardcoded in a fixture that then gets validated against the enum, it may fail if the enum was updated. + +#### Cause B: Pydantic v2 `model_config` not applied everywhere + +```bash +# Find any remaining v1-style validators +grep -rn "@validator\|class Config:" app/ +``` +**Fix:** Replace `class Config: orm_mode = True` with `model_config = ConfigDict(from_attributes=True)`. Replace `@validator` with `@field_validator`. + +#### Cause C: Supabase `.delete()` requires at least one filter + +```bash +grep -n "\.delete()" app/ +``` +**Fix:** Any `.delete()` without a `.eq()/.neq()/.in_()` filter will raise an error in supabase-py v2. Use the `.neq("id", "00000000-0000-0000-0000-000000000000")` sentinel for clear_all-style operations. + +#### Cause D: Test database isolation (tests polluting each other) + +```bash +# Run tests in isolation and compare pass/fail +pytest tests/test_goal_crud.py::test_create_goal -v --tb=long +pytest tests/test_goal_crud.py::test_list_goals -v --tb=long +pytest tests/test_goal_crud.py -v --tb=long # run together + +# If isolated pass but combined fail → fixture teardown issue +``` +**Fix:** Ensure each test fixture runs its own `clear_all()` in teardown, and that `clear_all()` itself works (see Cause C). + +#### Cause E: HTTP 501 still returned by signals.py + +```bash +curl -X POST http://localhost:8000/signals \ + -H "Content-Type: application/json" \ + -d '{"actor":"test","event_type":"test","timestamp":"2026-06-15T00:00:00Z"}' \ + | python -c "import sys,json; r=json.load(sys.stdin); print(r)" +``` +If any test POSTs to `/signals` and expects 201 but gets 501, the stub is still active. + +### Step 4: Address pytest Warnings + +```bash +# See all warnings in detail +pytest tests/ -v --tb=short -W always 2>&1 | grep -A3 "Warning\|DeprecationWarning\|PytestWarning" +``` + +**Likely Warning Categories:** + +| Warning | Category | Fix | +|---|---|---| +| `DeprecationWarning: @validator` | Pydantic v1→v2 | Replace with `@field_validator` | +| `PytestUnraisableExceptionWarning` | Async teardown not awaited | Add `asyncio_mode = "auto"` to pytest.ini or use `pytest-asyncio` properly | +| `ResourceWarning: unclosed socket` | Supabase client not closed | Add `client.close()` or use context manager in fixtures | +| `UserWarning: datetime naive` | Missing timezone on timestamps | Use `datetime.now(timezone.utc)` instead of `datetime.utcnow()` | +| `pytest.PytestConfigWarning` | Missing pytest.ini setting | Add `[pytest] asyncio_mode = auto` to `pytest.ini` or `pyproject.toml` | + +**Fix for asyncio warnings (add to `pytest.ini` or `pyproject.toml`):** +```ini +# pytest.ini +[pytest] +asyncio_mode = auto +filterwarnings = + ignore::DeprecationWarning:pydantic +``` + +--- + +## Part 4 — Dataset Evaluation for Activity Signal Testing + +### 4A — Loghub (LogPAI) + +**Repo:** https://github.com/logpai/loghub +**Format:** System logs (Apache, Hadoop, Linux) — Timestamp + Component + Message + +**Schema Mapping:** +| Loghub Field | Your Signal Field | Notes | +|---|---|---| +| `Timestamp` | `timestamp` | Direct map after ISO 8601 conversion | +| `Component` | `actor` | Maps well — represents the source system/service | +| `EventTemplate` | `event_type` | Use parsed template ID (e.g., `E42`) | +| `Content` | `payload.raw_log` | Store full message in payload | +| N/A | `goal_id` | Must inject synthetically — map by log source | + +**Suitability:** ⭐⭐⭐ (3/5) — Structurally perfect for testing ingestion pipelines but semantically irrelevant to intern goals. Best for **volume and format testing**, not semantic validation. + +**Download and Transform:** +```bash +# 1. Clone the repo +git clone https://github.com/logpai/loghub.git +cd loghub + +# 2. Use the small Apache dataset (~1MB, good for testing) +# File: loghub/Apache/Apache_2k.log + +# 3. Parse and transform to signal format +python - << 'EOF' +import json, re +from datetime import datetime + +LOG_FILE = "Apache/Apache_2k.log" +GOAL_ID = "YOUR-SEEDED-GOAL-UUID-HERE" # replace with a real goal ID +OUTPUT_FILE = "signals_apache.json" + +signals = [] +# Apache log pattern: [Day Mon DD HH:MM:SS YYYY] [level] message +pattern = re.compile(r'\[(.+?)\] \[(\w+)\] (.+)') + +with open(LOG_FILE, "r") as f: + for line in f: + m = pattern.match(line.strip()) + if m: + raw_ts, level, message = m.groups() + try: + ts = datetime.strptime(raw_ts, "%a %b %d %H:%M:%S %Y").isoformat() + "Z" + except: + ts = datetime.utcnow().isoformat() + "Z" + signals.append({ + "goal_id": GOAL_ID, + "actor": "apache-server", + "event_type": f"log_{level.lower()}", + "timestamp": ts, + "payload": {"raw_log": message[:500]} + }) + +with open(OUTPUT_FILE, "w") as f: + json.dump(signals[:50], f, indent=2) # first 50 for testing + +print(f"Wrote {min(50, len(signals))} signals to {OUTPUT_FILE}") +EOF + +# 4. Send signals to your endpoint +python - << 'EOF' +import json, requests + +TOKEN = "YOUR-JWT-TOKEN-HERE" +BASE_URL = "http://localhost:8000" + +with open("signals_apache.json") as f: + signals = json.load(f) + +headers = {"Authorization": f"Bearer {TOKEN}", "Content-Type": "application/json"} +success, fail = 0, 0 + +for sig in signals: + r = requests.post(f"{BASE_URL}/signals", json=sig, headers=headers) + if r.status_code in (200, 201): + success += 1 + else: + fail += 1 + print(f"FAILED: {r.status_code} — {r.text[:100]}") + +print(f"\nResults: {success} success, {fail} failed out of {len(signals)} signals") +EOF +``` + +--- + +### 4B — BPI Challenge 2013 (Volvo IT Incident Management) + +**Source:** https://www.tf-pm.org/competitions-awards/bpi-challenge +**Format:** XES event log — CaseID + Timestamp + Activity + Resource + +**Schema Mapping:** +| BPI 2013 Field | Your Signal Field | Notes | +|---|---|---| +| `time:timestamp` | `timestamp` | Direct map — already ISO 8601 | +| `org:resource` | `actor` | Maps directly — person handling the incident | +| `concept:name` (activity) | `event_type` | e.g., `"Accepted"`, `"Wait"`, `"Resolved"` | +| `case:concept:name` (case ID) | `goal_id` | Map 1 case → 1 goal (create goals from unique case IDs) | +| `impact`, `sub_status` | `payload.*` | Rich metadata for payload | + +**Suitability:** ⭐⭐⭐⭐⭐ (5/5) — This is the **ideal dataset**. IT incidents moving through statuses directly mirrors how an intern goal moves through SMILE phases. The lifecycle (Open → Accepted → In Progress → Resolved) maps conceptually to (sense → model → intervene → learn → evolve). + +**Download and Transform:** +```bash +# 1. Download from tf-pm.org (manual step — requires accepting terms) +# Navigate to: https://www.tf-pm.org/competitions-awards/bpi-challenge +# Find BPI Challenge 2013 and download the XES file +# File will be something like: VINST.xes or BPI_Challenge_2013_incidents.xes + +# 2. Install XES parser +pip install pm4py + +# 3. Parse XES and transform to signal format +python - << 'EOF' +import json +import pm4py +from datetime import datetime, timezone + +XES_FILE = "BPI_Challenge_2013_incidents.xes" # adjust filename +OUTPUT_FILE = "signals_bpi2013.json" +BASE_URL = "http://localhost:8000" +TOKEN = "YOUR-JWT-TOKEN-HERE" + +# Load the XES file +log = pm4py.read_xes(XES_FILE) + +signals = [] +case_to_goal = {} # we'll create one goal per unique case + +import requests +headers = {"Authorization": f"Bearer {TOKEN}", "Content-Type": "application/json"} + +# Create goals for unique cases (first 10 cases only for testing) +cases = list(log.groupby("case:concept:name"))[:10] + +for case_id, trace in cases: + # Create a goal for this incident case + goal_resp = requests.post(f"{BASE_URL}/goals", json={ + "title": f"Incident Case {case_id}", + "description": f"Volvo IT incident tracked from BPI 2013", + "priority": 3, + "smile_phase": "sense" + }, headers=headers) + + if goal_resp.status_code in (200, 201): + goal_id = goal_resp.json()["id"] + case_to_goal[case_id] = goal_id + print(f"Created goal {goal_id} for case {case_id}") + else: + print(f"Failed to create goal for case {case_id}: {goal_resp.text}") + +# Build signals from events +for case_id, trace in cases: + goal_id = case_to_goal.get(case_id) + if not goal_id: + continue + for _, event in trace.iterrows(): + ts = event.get("time:timestamp") + if hasattr(ts, "isoformat"): + ts_str = ts.isoformat() + else: + ts_str = datetime.now(timezone.utc).isoformat() + + signals.append({ + "goal_id": goal_id, + "actor": str(event.get("org:resource", "unknown")), + "event_type": str(event.get("concept:name", "unknown_event")).lower().replace(" ", "_"), + "timestamp": ts_str, + "payload": { + "impact": str(event.get("impact", "")), + "sub_status": str(event.get("sub_status", "")), + "case_id": str(case_id) + } + }) + +with open(OUTPUT_FILE, "w") as f: + json.dump(signals, f, indent=2) + +print(f"\nTransformed {len(signals)} events from {len(cases)} cases") +print(f"Written to {OUTPUT_FILE}") +EOF + +# 4. Send all signals in batch +python - << 'EOF' +import json, requests + +TOKEN = "YOUR-JWT-TOKEN-HERE" +BASE_URL = "http://localhost:8000" + +with open("signals_bpi2013.json") as f: + signals = json.load(f) + +headers = {"Authorization": f"Bearer {TOKEN}", "Content-Type": "application/json"} +success, fail = 0, 0 + +for i, sig in enumerate(signals): + r = requests.post(f"{BASE_URL}/signals", json=sig, headers=headers) + if r.status_code in (200, 201): + success += 1 + else: + fail += 1 + if fail <= 5: # only print first 5 failures + print(f"[{i}] FAILED {r.status_code}: {r.text[:150]}") + +print(f"\nFinal: {success}/{len(signals)} signals ingested successfully") +EOF +``` + +--- + +### 4C — BPI Challenge 2020 (Travel Administration) + +**Source:** https://www.tf-pm.org/resources/logs +**Format:** XES event log — travel requests moving through approval/rejection states + +**Schema Mapping:** +| BPI 2020 Field | Your Signal Field | Notes | +|---|---|---| +| `time:timestamp` | `timestamp` | Direct map | +| `org:resource` | `actor` | Approver/submitter name | +| `concept:name` | `event_type` | e.g., `"Declaration SUBMITTED"`, `"Declaration APPROVED"` | +| `case:concept:name` | `goal_id` | One travel request = one goal | +| `case:Amount` | `payload.amount` | Budget metadata | +| `case:org:role` | `payload.role` | Submitter role | + +**Suitability:** ⭐⭐⭐⭐ (4/5) — Very relevant since travel request approval mirrors intern goal approval/progression. Useful for testing multi-step workflows and rejection/re-submission loops. + +**Download and Transform:** +```bash +# 1. Download from tf-pm.org (manual — select BPI Challenge 2020) +# Multiple sublogs available: Prepaid Travel Costs, Declaration with pre-approval, etc. +# Recommended: "RequestForPayment" sublog (smaller, cleaner) + +pip install pm4py + +python - << 'EOF' +import json, pm4py, requests +from datetime import datetime, timezone + +XES_FILE = "RequestForPayment.xes" # adjust to your downloaded filename +OUTPUT_FILE = "signals_bpi2020.json" +TOKEN = "YOUR-JWT-TOKEN-HERE" +BASE_URL = "http://localhost:8000" + +log = pm4py.read_xes(XES_FILE) +headers = {"Authorization": f"Bearer {TOKEN}", "Content-Type": "application/json"} + +signals = [] +case_to_goal = {} +cases = list(log.groupby("case:concept:name"))[:10] + +for case_id, trace in cases: + amount = trace.get("case:Amount", [None]).iloc[0] if "case:Amount" in trace.columns else None + goal_resp = requests.post(f"{BASE_URL}/goals", json={ + "title": f"Travel Request {case_id}", + "description": f"BPI 2020 travel administration case", + "priority": 2, + "smile_phase": "sense", + "metadata": {"amount": str(amount) if amount else "unknown"} + }, headers=headers) + + if goal_resp.status_code in (200, 201): + goal_id = goal_resp.json()["id"] + case_to_goal[case_id] = goal_id + +for case_id, trace in cases: + goal_id = case_to_goal.get(case_id) + if not goal_id: + continue + for _, event in trace.iterrows(): + ts = event.get("time:timestamp") + ts_str = ts.isoformat() if hasattr(ts, "isoformat") else datetime.now(timezone.utc).isoformat() + event_name = str(event.get("concept:name", "unknown")).lower().replace(" ", "_") + + signals.append({ + "goal_id": goal_id, + "actor": str(event.get("org:resource", "system")), + "event_type": event_name, + "timestamp": ts_str, + "payload": { + "case_id": str(case_id), + "role": str(event.get("org:role", "")) + } + }) + +with open(OUTPUT_FILE, "w") as f: + json.dump(signals, f, indent=2) + +print(f"Generated {len(signals)} signals for {len(cases)} travel cases") +EOF +``` + +--- + +## Part 5 — Complete Testing Checklist (Sequential) + +Run these in order. Each section depends on the previous. + +### Phase 0: Environment Check +- [ ] Server running: `curl http://localhost:8000/health` → 200 +- [ ] Supabase local running: `npx supabase status` → shows running services +- [ ] 15 goals seeded: `curl http://localhost:8000/goals | python -c "import sys,json; print(len(json.load(sys.stdin)))"` + +### Phase 1: Authentication +- [ ] **Step 2** — POST `/auth/login` → 200 with JWT +- [ ] **Step 3** — Token captured in `$TOKEN` +- [ ] **Step 4** — GET `/goals` with token → 200 +- [ ] **Step 5** — POST `/auth/login` with wrong password → 401 + +### Phase 2: Goal CRUD +- [ ] **Step 7** — POST `/goals` → 201, goal created +- [ ] **Step 8** — GET `/goals` → 200, ≥15 goals +- [ ] **Step 9** — Count = 15+ +- [ ] **Step 10** — GET `/goals/:id` → 200, correct goal +- [ ] **Step 11** — PATCH `/goals/:id` → 200, updated +- [ ] **Step 12** — GET `/goals?sort=priority` → 200, ordered +- [ ] **Step 13** — GET `/goals?smile_phase=sense` → filtered +- [ ] **Step 14** — DELETE throwaway goal → 204/200 +- [ ] **Step 15** — DELETE non-existent → 404 + +### Phase 3: Activity Signals +- [ ] **Step 16** — GET `/signals` → 200 or 405 (not 501 or 404) +- [ ] **Step 17** — POST `/signals` with valid body → 201 +- [ ] **Step 18** — POST `/signals` without auth → 401 +- [ ] **Step 19** — GET `/signals?goal_id=X` → contains step 17's signal +- [ ] **Step 20** — POST `/signals` missing fields → 422 +- [ ] **Step 21** — PATCH goal phase → signals auto-created by `log_transition` + +### Phase 4: pytest +- [ ] `pytest tests/ -v` → identify the 4 failing tests +- [ ] Run each failing test in isolation with `--tb=long -s` +- [ ] Fix conftest.py fixture phase label if needed +- [ ] Fix Pydantic v2 validators if needed +- [ ] Verify `.delete()` uses `.neq()` filter +- [ ] `pytest tests/ -v` → all green ✅ + +### Phase 5: Dataset Testing (optional, for Demo Day evidence) +- [ ] Download Apache 2K log from Loghub +- [ ] Run Loghub transform script → `signals_apache.json` (50 signals) +- [ ] Send to `/signals` endpoint → ≥45/50 success rate +- [ ] Download BPI 2013 XES +- [ ] Install `pm4py`: `pip install pm4py` +- [ ] Run BPI 2013 script → goals created + signals ingested +- [ ] Verify via GET `/signals?goal_id=X` for each created goal +- [ ] Validate signal count matches expected event count from XES + +--- + +## Quick Reference: Signal Schema + +```json +{ + "goal_id": "uuid-string", + "actor": "user@email.com or system-name", + "event_type": "snake_case_event_name", + "timestamp": "2026-06-15T10:30:00Z", + "payload": { + "any": "additional", + "context": "here" + } +} +``` + +## Quick Reference: SMILE Phases (6-phase lifecycle) +``` +sense → model → intervene → learn → evolve → reality-emulation +``` +(Use `reality-emulation` in test fixtures per PR #17 correction) diff --git a/src/lpi/recommendation_engine.py b/src/lpi/recommendation_engine.py new file mode 100644 index 0000000..cc7d0ba --- /dev/null +++ b/src/lpi/recommendation_engine.py @@ -0,0 +1,319 @@ +"""Module 3 — Recommendation Engine, Wave 2: real reasoning core. + +Algorithm Owner : Jaivardhan Singh (Phase 4 — Wave 2) +This pass : Adil Islam + +WHY THIS FILE EXISTS +───────────────────── +routers/recommendations.py's own module docstring predicted this exact +path: "Jaivardhan's reasoning core (not yet built — will likely live in +a new lpi/recommendation_engine.py) owns mapping real goals + signals to +candidate actions with SMILE-grounded reasoning." + +Wave 1 shipped a stable endpoint contract backed by +_build_mock_recommendations() — three hardcoded Recommendation objects, +IDENTICAL for every user_id, never touching the goals or activity_signals +tables. That unblocked Jahanvi's frontend, but it meant the "recommendation +engine" wasn't reading any of the data Module 1 (goals) and Module 3a +(activity_signals) actually store. This file fixes that: every +recommendation is now derived from the caller's real rows in Supabase. + +WHAT CHANGED VS WAVE 1 +──────────────────────── + - generate_recommendations(user_id) replaces the hardcoded specs list. + It calls store.list_goals(user_id=...) and store.list_signals(user_id=...) + and turns the ACTUAL rows into Recommendation objects. + - Each goal produces one "advance to the next SMILE phase" recommendation. + priority = scoring.score_goal(goal) — reusing the EXACT formula + goals.py already uses, so a recommendation's priority badge means the + same thing as a goal's priority badge. That's the whole reason + Recommendation.priority lives on the [0.80, 7.00] scale in the first + place (see the Wave 1 docstring this replaces). + - The user's recent activity signals (not yet linked to any goal — + Signal has no goal_id column, see models.SignalCreate) produce one + "review your signals" recommendation, so a user who only logs + activity and has no goals yet still gets a useful nudge. + - source_goals / source_signals are populated with REAL ids instead of + always being []. + - Cold start (user has zero goals AND zero signals — true for a brand + new account, and also true for the Wave-1 demo profiles in a freshly + cleared test DB) falls back to build_cold_start_recommendations(), + which is the old _build_mock_recommendations() content, relocated + here. routers/recommendations.py re-exports the old private name so + nothing that already imports it from there breaks. + +WHAT THIS WAVE DELIBERATELY DOES NOT DO +────────────────────────────────────────── + - No LLM call. config.py already has llm_provider / llm_model / + daily_cost_cap_usd but nothing reads them yet — that's the LLM-bonus + wave in the Phase 4 build plan, intentionally last, after this + deterministic version is correct and tested. + - No persistence / accept-dismiss storage. Still Yashika's deliverable — + no Supabase table exists for it yet. + - No goal<->signal FK correlation. There's no schema link saying + "signal X helped advance goal Y" — that's a harder correlation + problem left for a later wave. This pass treats "the user's recent + signals" as one bucket per user, not scoped per goal. +""" + +import uuid +from datetime import UTC, datetime + +from lpi import store +from lpi.models import Goal, Recommendation, Signal, SmilePhase +from lpi.scoring import score_goal, sort_goals_by_score +from lpi.smile import PHASE_ORDER, get_phase_description, get_phase_key_question + +# How many of the user's most recent signals to pull when building the +# signal-driven recommendation. Mirrors the "small limit" guidance already +# documented in store.list_signals() — the engine needs recent activity, +# not the user's entire history, to stay fast (TimingMiddleware's +# X-Process-Time header exists specifically to catch a slow path here). +_SIGNALS_TO_CONSIDER = 20 + +# Priority assigned to the signal-driven recommendation. Grounded in +# scoring.py's own worked example for a brand-new default goal — +# "p=5, reality-emulation, urgent=False -> 2.80" — instead of an arbitrary +# number, so a fresh signal-based nudge ranks roughly as important as +# creating a brand-new default-priority goal would. +_SIGNAL_RECOMMENDATION_PRIORITY = 2.80 + + +def generate_recommendations(user_id: str) -> list[Recommendation]: + """Build this user's candidate recommendations from real stored data. + + Returns an UNSORTED, UNTRUNCATED list — routers/recommendations.py + owns sorting by priority descending and slicing to `limit`, exactly + like it did for the Wave 1 mock data. Keeping that contract here is + what lets every auth/shape/limit test in tests/test_recommendations.py + keep passing unmodified regardless of where the data comes from. + """ + goals = store.list_goals(user_id=user_id) + signals = store.list_signals(user_id=user_id, limit=_SIGNALS_TO_CONSIDER) + + if not goals and not signals: + # Cold start: brand-new account, or one of the Wave-1 demo + # profiles against a freshly cleared test DB. Real reasoning + # needs SOMETHING to reason about — fall back to the + # deterministic starter set so the endpoint never returns an + # unhelpful empty list on a user's first visit. + return build_cold_start_recommendations(user_id) + + candidates: list[Recommendation] = [] + candidates.extend(_goal_recommendations(user_id, goals)) + + signal_rec = _signal_recommendation(user_id, signals) + if signal_rec is not None: + candidates.append(signal_rec) + + return _diversify_by_phase(candidates) + + +def _phase_grounded_reasoning(phase: SmilePhase, lead_in: str) -> str: + """Build reasoning text that ALWAYS literally names its own SMILE phase. + + WHY EXPLICIT, NOT IMPLICIT + ──────────────────────────── + get_phase_description()'s prose doesn't always contain the phase's own + slug as a literal word — e.g. collective-intelligence's description + talks about sensors, KPIs, and ontologies; it never spells out the + words "collective" or "intelligence". Naming the phase outright here, + rather than hoping the description text happens to mention it, is + what makes every recommendation reliably SMILE-grounded instead of + grounded by coincidence (this is exactly the failure mode the + test_recommendation_reasoning_names_its_smile_phase test guards + against — it greps the reasoning string for the phase slug/words). + """ + return ( + f"{lead_in} This action targets the {phase.value} phase. " + f"Key question: {get_phase_key_question(phase)} " + f"{get_phase_description(phase)[:120]}..." + ) + + +def _next_phase(current: SmilePhase) -> SmilePhase | None: + """Return the phase one step forward from `current`, or None at the end. + + Mirrors the forward-step rule already enforced by + smile.validate_phase_transition() (forward exactly one step is the + only legal forward move) — just expressed as "what is that one step" + instead of "is this transition legal." + """ + idx = PHASE_ORDER.index(current) + if idx + 1 >= len(PHASE_ORDER): + return None + return PHASE_ORDER[idx + 1] + + +def _goal_recommendations(user_id: str, goals: list[Goal]) -> list[Recommendation]: + """One recommendation per goal: advance it to its next SMILE phase. + + Reuses sort_goals_by_score() purely so goal-derived candidates are + built in the same priority order goals.py already shows the user — + not required for correctness (the router re-sorts everything anyway) + but keeps debug/log output in a familiar order. + """ + now = datetime.now(UTC) + recommendations: list[Recommendation] = [] + + for goal in sort_goals_by_score(goals): + target_phase = _next_phase(goal.smile_phase) + priority = score_goal(goal) + + if target_phase is None: + # Goal is already at Perpetual Wisdom — there is no "next" + # phase to advance to. Recommend sustaining/sharing impact at + # the CURRENT phase instead of crashing on a missing + # PHASE_ORDER index (the bug a naive idx+1 lookup would hit). + target_phase = goal.smile_phase + action = f"Sustain and share the impact of '{goal.title}'" + lead_in = ( + f"'{goal.title}' has already reached {goal.smile_phase.value} — " + "the next concrete step is sustaining and sharing that impact, " + "not advancing further." + ) + else: + action = f"Advance '{goal.title}' to {target_phase.value}" + lead_in = ( + f"'{goal.title}' is currently at {goal.smile_phase.value}. " + "Advancing it forward is the next concrete step." + ) + + recommendations.append( + Recommendation( + id=str(uuid.uuid4()), + user_id=user_id, + action=action, + reasoning=_phase_grounded_reasoning(target_phase, lead_in), + smile_phase=target_phase, + priority=priority, + source_goals=[goal.id], + source_signals=[], + created_at=now, + ) + ) + + return recommendations + + +def _signal_recommendation(user_id: str, signals: list[Signal]) -> Recommendation | None: + """One recommendation summarising the user's recent activity signals. + + Activity signals are sensor/event data with no goal_id column (see + models.SignalCreate) — there's no FK telling us which goal, if any, a + given signal is "for". Rather than guess at that correlation, this + wave treats all of the user's recent signals as one bucket and nudges + them to fold that raw activity into a tracked goal. Smarter per-goal + correlation is future work (see module docstring). + + Returns None when the user has no signals — callers must handle that + (generate_recommendations() does, by skipping this candidate entirely). + """ + if not signals: + return None + + streams = sorted({s.stream for s in signals}) + + lead_in = ( + f"You have {len(signals)} recent activity signal(s) from: " + f"{', '.join(streams)}." + ) + + return Recommendation( + id=str(uuid.uuid4()), + user_id=user_id, + action=f"Review your {len(signals)} recent activity signal(s) and link them to a goal", + reasoning=_phase_grounded_reasoning(SmilePhase.COLLECTIVE_INTELLIGENCE, lead_in), + smile_phase=SmilePhase.COLLECTIVE_INTELLIGENCE, + priority=_SIGNAL_RECOMMENDATION_PRIORITY, + source_goals=[], + source_signals=[s.id for s in signals], + created_at=datetime.now(UTC), + ) + + +def _diversify_by_phase(candidates: list[Recommendation]) -> list[Recommendation]: + """Keep at most one candidate per SMILE phase — the highest-priority one. + + WHY THIS EXISTS + ────────────────── + A user with three goals all sitting in reality-emulation would + otherwise get three near-identical "advance to concurrent-engineering" + cards. That tells the user nothing about where they stand across the + SMILE lifecycle (the whole point per the Phase 4 gate criteria) and + burns two of Jahanvi's three card slots on duplicate advice. + + Does NOT sort or truncate — routers/recommendations.py still owns + that. This only drops same-phase duplicates so that sort+limit + naturally yields a phase-diverse top N whenever enough diversity + exists in the underlying goals/signals. + """ + best_per_phase: dict[SmilePhase, Recommendation] = {} + for rec in candidates: + existing = best_per_phase.get(rec.smile_phase) + if existing is None or rec.priority > existing.priority: + best_per_phase[rec.smile_phase] = rec + return list(best_per_phase.values()) + + +def build_cold_start_recommendations(user_id: str) -> list[Recommendation]: + """Deterministic fallback for a user with zero goals AND zero signals. + + This is the Wave 1 mock dataset's content, relocated here from + routers/recommendations.py (_build_mock_recommendations). Two reasons + it still exists post-Wave-2: + + 1. Cold start: a brand-new account (or a freshly seeded demo profile + / clean test DB) has nothing real to reason about yet. An empty + list is a worse first-run experience than three generic starter + actions. + 2. Daksh's orchestration pipeline documented a "guaranteed 3 cards" + fallback route for when the multi-module reasoning pipeline + doesn't finish in time — this function IS that safety net. + routers/recommendations.py re-exports it under its old private + name (_build_mock_recommendations) so existing callers don't break. + + Priority values intentionally land inside the existing goals scoring + scale (0.80-7.00, see lpi/scoring.py) instead of an arbitrary 1-3, so + the frontend can reuse the same "priority badge" component it already + builds for goals. + """ + now = datetime.now(UTC) + + # (action, smile_phase, priority) — priority hand-picked to land in + # the same band a real goal at that phase/priority would score via + # lpi/scoring.py's formula, so the fallback "looks like" real output. + specs: list[tuple[str, SmilePhase, float]] = [ + ( + "Ingest this week's GitHub activity as a signal", + SmilePhase.COLLECTIVE_INTELLIGENCE, + 5.60, + ), + ( + "Advance your active goal to Contextual Intelligence", + SmilePhase.CONTEXTUAL_INTELLIGENCE, + 4.30, + ), + ( + "Re-validate your reality canvas before the next demo", + SmilePhase.REALITY_EMULATION, + 2.90, + ), + ] + + lead_in = "Getting started — no goals or signals logged yet for this user." + + return [ + Recommendation( + id=str(uuid.uuid4()), + user_id=user_id, + action=action, + reasoning=_phase_grounded_reasoning(phase, lead_in), + smile_phase=phase, + priority=priority, + source_goals=[], + source_signals=[], + created_at=now, + ) + for action, phase, priority in specs + ] diff --git a/src/lpi/routers/recommendations.py b/src/lpi/routers/recommendations.py index ede9d42..51791fe 100644 --- a/src/lpi/routers/recommendations.py +++ b/src/lpi/routers/recommendations.py @@ -1,30 +1,114 @@ -"""Module 3 — Recommendation Engine. +"""Module 3 — Recommendation Engine: HTTP endpoint. -Owner : Jaivardhan Singh (Phase 4) -QA : Ankit Kumar Singh +Algorithm Owner : Jaivardhan Singh (Phase 4 — Wave 2) +Endpoint Owner : Adil Islam (Phase 4 — Wave 1, also QA) -Phase 1/2 gate: returns empty list [] to prove the route is wired and -the server starts without error. The NotImplementedError that was here -caused HTTP 500 on every call and broke test_recommendations.py. +═══════════════════════════════════════════════════════════ +WAVE 2 (this pass) — real data wired in +═══════════════════════════════════════════════════════════ +Wave 1 shipped this endpoint backed by deterministic mock data +(_build_mock_recommendations) so Jahanvi's frontend wasn't blocked on the +real reasoning core. That mock data NEVER read the goals or +activity_signals tables — every user_id got the same 3 hardcoded cards. -Phase 4: Jaivardhan replaces the body with LangGraph agent reasoning. +Wave 2 fixes exactly that: the data source is now +lpi.recommendation_engine.generate_recommendations(user_id), which reads +the caller's REAL goals + activity_signals from Supabase and reasons over +them (see lpi/recommendation_engine.py for the full algorithm writeup). + +This file is now a thin HTTP layer: + - auth (Depends(get_current_user)) + - validation (the `limit` Query bounds) + - the stable response contract (always <= limit Recommendation objects, + sorted by priority descending) + - delegating the actual "what should this user do next" decision to + lpi/recommendation_engine.py + +WHAT STAYS THE SAME AS WAVE 1 (unchanged on purpose) +─────────────────────────────────────────────────────── + - Real auth, same as goals.py / signals.py. + - Same response contract: Recommendation objects on the SAME 0.80-7.00 + priority scale goals use, sorted highest-priority first. + - user_id is still a path param, NOT derived from the JWT. This is + still BY DESIGN, not an oversight: it's what lets one authenticated + demo session pull up Aryan's seeded "Intern A/B/C" profiles, and it's + also what lets the recommendation engine read ANY user_id's goals/ + signals rather than only the caller's own. If this becomes a + production multi-tenant endpoint, add the same 404-on-mismatch + ownership check used in goals.py / signals.py + (`if resource.user_id != caller_id: raise 404`). + - No persistence. Accept/dismiss storage is still Yashika's deliverable. + +HOW THE SWAP WORKS (for anyone touching this file next) +─────────────────────────────────────────────────────────── +generate_recommendations(user_id) -> list[Recommendation] is the same +shape _build_mock_recommendations(user_id) always had, so improving the +reasoning core (smarter signal correlation, an LLM-backed bonus pass, +etc.) means editing lpi/recommendation_engine.py — this router shouldn't +need to change again for that. + +Daksh: your orchestration pipeline's "guaranteed 3 cards" fallback can +still call `_build_mock_recommendations()` (re-exported below from +lpi.recommendation_engine.build_cold_start_recommendations under its old +name) directly as the safety-net route if the multi-module reasoning +hasn't finished in time. """ -from fastapi import APIRouter, Query +from fastapi import APIRouter, Depends, Query +from lpi.middleware.auth import get_current_user from lpi.models import Recommendation +from lpi.recommendation_engine import build_cold_start_recommendations, generate_recommendations router = APIRouter() +# Backward-compatible alias. Wave 1's mock builder lived here under this +# private name; Wave 2 relocated its implementation to +# lpi/recommendation_engine.py (as build_cold_start_recommendations) so +# the real engine can call it directly for the cold-start case without a +# circular import between this router and the engine module. Anything +# that already imports `_build_mock_recommendations` from this module +# (e.g. Daksh's orchestration safety net) keeps working unchanged. +_build_mock_recommendations = build_cold_start_recommendations + -@router.get("/{user_id}", response_model=list[Recommendation]) +@router.get( + "/{user_id}", + response_model=list[Recommendation], + summary="Get recommended next actions for a user", + description=( + "Returns up to `limit` SMILE-grounded next-action recommendations, " + "highest priority first. Wave 2: derived from the user's real " + "goals + activity signals, with a deterministic 3-item fallback " + "for users with no goals or signals yet." + ), +) def get_recommendations( user_id: str, - limit: int = Query(default=3, ge=1, le=10), + limit: int = Query( + default=3, + ge=1, + le=10, + description="Max recommendations to return (1-10). Demo gate asks for 3.", + ), + _caller_id: str = Depends(get_current_user), ) -> list[Recommendation]: - """Return personalised next-action recommendations. + """Return up to `limit` recommended next actions for `user_id`. + + Auth: requires a valid Supabase JWT (any authenticated caller — see + module docstring for why user_id is NOT restricted to the caller's + own id). `_caller_id` is intentionally unused beyond the Depends() + call itself: its only job here is to reject unauthenticated requests + with 401 before this function body ever runs. - Phase 1/2: always returns [] — placeholder, route confirmed wired. - Phase 4: Jaivardhan implements SMILE-grounded LangGraph reasoning here. + Sorted by priority descending (highest-priority action first), then + truncated to `limit`. With a cold-start user (no goals/signals yet), + `limit` only has a visible effect for limit < 3, since the fallback + set always has exactly 3 items. With real goal/signal data, the + candidate count varies with how much the user has actually logged. """ - return [] + recommendations = sorted( + generate_recommendations(user_id), + key=lambda r: -r.priority, + ) + return recommendations[:limit] diff --git a/src/lpi/routers/signals.py b/src/lpi/routers/signals.py index 190d93a..5febcbf 100644 --- a/src/lpi/routers/signals.py +++ b/src/lpi/routers/signals.py @@ -14,6 +14,18 @@ GET / → queries Supabase with server-side filters + pagination [Wave 3] GET /{signal_id} → fetch a single signal by UUID [bonus] +PHASE 3 FOLLOW-UP (this pass) +──────────────────────────────── + - GET / gained `start`/`end` query params for time-range filtering. + This was the single largest gap between the gate sheet ("Timeline + queryable by user and time range") and the running code. + - The ingest logging comment below is corrected: log_user_activity() + in utils/logging.py already has its own internal try/except and + never raises, so the try/except previously wrapped around it here + was dead code for the CHECK-constraint failure mode. See + utils/logging.py for the real fix (logger.exception instead of + print) and the migration note below. + HOW THIS ROUTER FITS INTO THE SYSTEM ────────────────────────────────────── Signals are the input layer for the recommendation engine. @@ -22,7 +34,7 @@ → POST /api/v1/signals/ ← THIS FILE → store.insert_signal() → Supabase activity_signals table - → Phase 4: GET /api/v1/signals/?stream=... ← ALSO THIS FILE + → Phase 4: GET /api/v1/signals/?stream=...&start=...&end=... ← ALSO THIS FILE → Jaivardhan's recommendation engine reads signals here WHY source MATTERS @@ -36,11 +48,13 @@ ──────── Every POST calls log_user_activity() — same pattern as goals.py. The action string is 'signal_ingested'. -NOTE: The user_activity_logs table CHECK constraint currently only allows - 'goal_created', 'goal_updated', 'goal_deleted'. To log signals there, - either update the CHECK constraint or add a new 'signal_ingested' value. - For now, logging is wrapped in try/except so a constraint mismatch - never breaks the ingest endpoint — it just prints a warning. +NOTE: the user_activity_logs CHECK constraint originally only allowed + 'goal_created', 'goal_updated', 'goal_deleted'. The fix is already + written in supabase/migrations/20260615000000_signals_rls_and_log_action.sql + — apply it with `supabase db push` if not already applied. + log_user_activity() itself never raises (it catches and logs its own + Supabase errors internally via the standard `logging` module), so + no try/except is needed at this call site. """ import uuid @@ -118,25 +132,22 @@ def ingest_signal( # which runs an INSERT into the activity_signals table. store.insert_signal(new_signal) - # Log the ingest event. - # Wrapped in try/except because the user_activity_logs CHECK constraint - # may not yet include 'signal_ingested' — a constraint mismatch raises - # an exception in supabase-py. We log the warning but never break the - # ingest endpoint. Update the CHECK constraint migration to fix properly. - try: - log_user_activity( - user_id=user_id, - action="signal_ingested", - resource_id=new_signal.id, - metadata={ - "stream": new_signal.stream, - "event_type": new_signal.event_type, - "source": new_signal.source, - }, - ) - except Exception as exc: - # Log to stdout — visible in uvicorn logs. Never breaks the endpoint. - print(f"[ingest_signal] WARNING: logging failed for signal {new_signal.id}: {exc}") + # Log the ingest event. No try/except here: log_user_activity() already + # guarantees it never raises — it catches any Supabase-side failure + # internally and reports it via logger.exception() (see + # utils/logging.py). Wrapping it here again would be redundant and, + # worse, gives a false impression that THIS is where a logging failure + # gets caught — it isn't; this call simply cannot raise. + log_user_activity( + user_id=user_id, + action="signal_ingested", + resource_id=new_signal.id, + metadata={ + "stream": new_signal.stream, + "event_type": new_signal.event_type, + "source": new_signal.source, + }, + ) print(new_signal.model_dump()) return new_signal @@ -202,16 +213,18 @@ def list_signals( fetch_all: bool = Query(False, alias="all"), user_context: UserContext = Depends(get_current_user_context), ) -> list[Signal]: - """Return signals filtered by stream, event_type, and/or source. + """Return signals filtered by stream, event_type, source, and/or time range. HOW SERVER-SIDE FILTERING WORKS HERE ────────────────────────────────────── - Each query param (stream, event_type, source) is passed to - store.list_signals(), which chains .eq() calls on the Supabase - query builder. This translates to SQL WHERE clauses: + Each query param (stream, event_type, source, start, end) is passed to + store.list_signals(), which chains .eq()/.gte()/.lte() calls on the + Supabase query builder. This translates to SQL WHERE clauses: - ?stream=boardy → WHERE stream = 'boardy' - ?stream=boardy&source=manual → WHERE stream = 'boardy' AND source = 'manual' + ?stream=boardy → WHERE stream = 'boardy' + ?stream=boardy&source=manual → WHERE stream = 'boardy' AND source = 'manual' + ?start=2026-06-13T00:00:00Z → WHERE timestamp >= '2026-06-13T00:00:00Z' + ?start=...&end=... → WHERE timestamp BETWEEN start AND end (inclusive) Only rows matching ALL provided filters are returned. Postgres runs the filter using the indexes from the migration: @@ -227,11 +240,13 @@ def list_signals( Postgres never reads rows outside the requested window. Example calls: - GET /api/v1/signals/ → last 50 signals - GET /api/v1/signals/?stream=boardy → boardy signals - GET /api/v1/signals/?stream=lpi&event_type=pr_merged → LPI PRs only - GET /api/v1/signals/?source=github_api&limit=20 → 20 real GitHub events - GET /api/v1/signals/?stream=boardy&limit=50&offset=50 → boardy page 2 + GET /api/v1/signals/ → last 50 signals + GET /api/v1/signals/?stream=boardy → boardy signals + GET /api/v1/signals/?stream=lpi&event_type=pr_merged → LPI PRs only + GET /api/v1/signals/?source=github_api&limit=20 → 20 real GitHub events + GET /api/v1/signals/?start=2026-06-13T00:00:00Z → everything since June 13 + GET /api/v1/signals/?start=2026-06-13T00:00:00Z&end=2026-06-20T00:00:00Z → one week window + GET /api/v1/signals/?stream=boardy&limit=50&offset=50 → boardy page 2 """ target_user_id = None if (fetch_all and user_context.is_admin) else user_context.user_id return store.list_signals( @@ -277,4 +292,4 @@ def get_signal( detail=f"Signal '{signal_id}' not found.", ) - return signal + return signal \ No newline at end of file diff --git a/src/lpi/store.py b/src/lpi/store.py index 57a6d46..f584fac 100644 --- a/src/lpi/store.py +++ b/src/lpi/store.py @@ -15,6 +15,17 @@ - get_signal() → fetches a single signal by id - clear_all() → now also truncates 'activity_signals' +PHASE 3 FOLLOW-UP (this pass) +──────────────────────────────── + - list_signals() gained `start`/`end` params for time-range filtering + (closes the §4.3 gate gap — was the largest discrepancy between the + spec sheet and the running code). + - get_user_activity_logs() added: a thin read helper over the + `user_activity_logs` table, used by tests to verify the audit trail + for signal ingestion actually exists in Supabase (not just in the + in-memory mirror in utils/logging.py, which always succeeds even if + the real Supabase write silently failed). + HOW THE STORE CONNECTS TO SUPABASE ──────────────────────────────────── Every function calls _get_client() which creates a supabase-py client @@ -41,6 +52,8 @@ Why it matters: at 10,000 signals, client-side fetches 10,000 rows to return 50. Server-side fetches and returns 50. The indexes in the migration (idx_as_stream, idx_as_event_type, etc.) make this O(log n) instead of O(n). +The same logic applies to the new start/end range filter below — it uses +idx_as_timestamp, which already existed but had no filter wired to it. FOR TESTS ────────── @@ -150,9 +163,6 @@ def delete_goal(goal_id: str) -> None: # These replace the Phase 2 in-memory dict stub. # The table 'activity_signals' is created by: # supabase/migrations/20260611000000_create_activity_signals.sql -# -# Pattern: identical to the goals functions above. If you understand -# how insert_goal / list_goals / get_goal work, these are the same. def insert_signal(signal: Signal) -> Signal: @@ -186,7 +196,8 @@ def list_signals( other user's signals, which is a data isolation bug. ALL filtering happens inside Postgres (server-side), not in Python. - Each .eq() call adds a WHERE clause — only matching rows come back. + Each .eq()/.gte()/.lte() call adds a WHERE clause — only matching + rows come back. Args: user_id : Scope results to the authenticated user's signals. @@ -195,11 +206,40 @@ def list_signals( event_type : Filter to one event type (e.g. 'pr_merged'). source : Filter by ingestion origin (e.g. 'github_api'). Phase 4 rec engine uses this to exclude 'simulated'. + start : Inclusive lower bound on `timestamp`. Closes the + §4.3 gate gap — "Timeline queryable by user and time + range" was a named success criterion that previously + had no router/store wiring at all. + end : Inclusive upper bound on `timestamp`. limit : Max rows to return per page (default 50, max 200). Prevents accidentally fetching thousands of rows. offset : How many rows to skip (for pagination). Page 1 = offset 0, Page 2 = offset 50, etc. + TIME-RANGE FILTERING EXPLAINED + ───────────────────────────────── + `.gte("timestamp", start.isoformat())` → WHERE timestamp >= start + `.lte("timestamp", end.isoformat())` → WHERE timestamp <= end + Both bounds are inclusive. Either can be supplied alone: + - only `start` → "everything since X" + - only `end` → "everything up to X" + - both → a closed window + - neither → unchanged behavior (no time filter), so this is + fully backward-compatible with existing callers. + + `.isoformat()` is required because the Supabase REST/PostgREST layer + expects a string, not a Python datetime object, the same reason + insert_signal() calls signal.model_dump(mode="json") rather than + passing a raw Pydantic model. Incoming `start`/`end` should be + timezone-aware (the router enforces ISO-8601 with an offset/Z) so the + comparison against the TIMESTAMPTZ column in Postgres is unambiguous. + + This filter uses idx_as_timestamp DESC, which already existed in the + Phase 3 migration but had no corresponding .gte()/.lte() call until + now — an index without a matching filter is dead weight; a filter + without a matching index is a full table scan waiting to happen. + Adding the filter here is what actually makes the existing index useful. + PAGINATION EXPLAINED ───────────────────── If 500 rows match a filter and limit=50: @@ -227,17 +267,22 @@ def list_signals( # Add filters only when the caller provided them. # Each .eq() adds: WHERE column = 'value' - # Chaining two .eq() calls adds: WHERE col1 = 'v1' AND col2 = 'v2' + # Chaining calls adds: WHERE col1 = 'v1' AND col2 = 'v2' AND ... if stream: query = query.eq("stream", stream) if event_type: query = query.eq("event_type", event_type) if source: query = query.eq("source", source) - if start is not None: + + # Time-range filter (new). `if start:` / `if end:` works correctly here + # because datetime instances are always truthy in Python (no __bool__ + # override) — this is the same truthy-check idiom already used for + # stream/event_type/source above, just applied to a datetime instead + # of a string. + if start: query = query.gte("timestamp", start.isoformat()) - - if end is not None: + if end: query = query.lte("timestamp", end.isoformat()) # Sort newest-first, then apply pagination. @@ -257,8 +302,7 @@ def list_signals( def get_signal(signal_id: str) -> Signal | None: """Fetch a single signal by its UUID. Returns None if not found. - Used by GET /api/v1/signals/{signal_id} — not yet in the router - but defined here so Phase 4 can use it without touching the store. + Used by GET /api/v1/signals/{signal_id}. """ result = _get_client().table("activity_signals").select("*").eq("id", signal_id).execute() if not result.data: @@ -266,6 +310,45 @@ def get_signal(signal_id: str) -> Signal | None: return Signal(**cast(dict, result.data[0])) +# ── Audit log verification (new — used by tests, also useful for admin tooling) ─ + +def get_user_activity_logs( + resource_id: str | None = None, + action: str | None = None, +) -> list[dict]: + """Read rows directly from the user_activity_logs Supabase table. + + WHY THIS EXISTS + ───────────────── + utils/logging.py's in-memory `user_activity_logs` list is appended to + unconditionally, BEFORE the Supabase insert is attempted — so it + always "succeeds" even if the real database write silently fails + (e.g. a CHECK constraint mismatch, exactly what shipped with + signal_ingested logging). Asserting against that in-memory list in a + test therefore cannot catch that class of bug. + + This function queries Supabase directly, which is the only reliable + way to confirm a log row actually exists in the database — used by + the regression test in tests/test_activity_signals.py + (test_ingest_writes_audit_log) and available for any admin/debug + tooling that needs to inspect the real audit trail. + + Args: + resource_id : Filter to logs for one specific goal/signal UUID. + action : Filter to one action string, e.g. "signal_ingested". + + Returns: + Raw list of matching rows (dicts), newest behavior not enforced — + callers needing order/pagination should add it the same way + list_signals() does, if this grows beyond test/debug usage. + """ + query = _get_client().table("user_activity_logs").select("*") + if resource_id: + query = query.eq("resource_id", resource_id) + if action: + query = query.eq("action", action) + return cast(list[dict], query.execute().data) # ← always reached + # ── Test helper ─────────────────────────────────────────────────────────────── @@ -284,6 +367,12 @@ def clear_all() -> None: This is called before AND after every test by the autouse fixture in conftest.py, so tests never see each other's data. + + NOTE: this does NOT wipe user_activity_logs. If you add a test that + relies on a clean audit-log table between runs (e.g. counting rows + rather than filtering by resource_id), wipe it the same way here. + The current regression test avoids this by filtering on resource_id, + which is unique per signal and doesn't require a clean table. """ # Wipe all goals rows _get_client().table("goals").delete().neq("user_id", "__sentinel_never_exists__").execute() @@ -291,4 +380,4 @@ def clear_all() -> None: # Wipe all activity_signals rows (Phase 3 addition) _get_client().table("activity_signals").delete().neq( "user_id", "__sentinel_never_exists__" - ).execute() + ).execute() \ No newline at end of file diff --git a/src/lpi/utils/logging.py b/src/lpi/utils/logging.py index 10d7fb2..ba15336 100644 --- a/src/lpi/utils/logging.py +++ b/src/lpi/utils/logging.py @@ -2,9 +2,14 @@ Design spec : Yashika Verma Updated : Adil Islam — Phase 2 Supabase integration — all 3 log types - -WHY THIS FILE WAS CHANGED -───────────────────────── +Updated : Adil Islam — Phase 3 fix: log_user_activity() failures now go + through the standard `logging` module instead of print(), so + a constraint mismatch (the bug that shipped with signal + ingestion) shows up in real log output/alerting instead of + only in stdout that may or may not be captured. + +WHY THIS FILE WAS CHANGED (Phase 2) +───────────────────────────────────── Phase 2 stub wrote transitions only to an in-memory list. That meant: - Nothing appeared in Supabase Studio (Table Editor or Logs tab) - Transitions were lost on every server restart @@ -20,14 +25,32 @@ 1. In-memory list → tests inspect this; no Supabase needed in test env 2. Supabase table → visible in Studio → Table Editor -If the Supabase write fails for any reason, the error is caught, printed -to stdout, and the request continues normally. Logging must NEVER break -an API endpoint. +If the Supabase write fails for any reason, the error is caught, logged +via the `logging` module, and the request continues normally. Logging +must NEVER break an API endpoint. log_transition() → goal_phase_transitions (SMILE phase changes only) - log_user_activity() → user_activity_logs (create, update, delete) + log_user_activity() → user_activity_logs (create, update, delete, + signal_ingested) log_system_event() → system_logs (startup, errors, warnings) +KNOWN BUG THIS FILE PREVIOUSLY MASKED (now fixed in log_user_activity) +───────────────────────────────────────────────────────────────────── +The `user_activity_logs` CHECK constraint originally only allowed +'goal_created' | 'goal_updated' | 'goal_deleted'. When Phase 3's +signals router started calling log_user_activity(action="signal_ingested"), +every call violated that constraint. The Supabase insert raised, was +caught by the except block below, and was reported with print() — +which is easy to miss in production and wasn't being asserted on by any +test. The real fix is two parts: + 1. DB fix: supabase/migrations/20260615000000_signals_rls_and_log_action.sql + adds 'signal_ingested' to the CHECK constraint. Apply with + `supabase db push` if not already applied. + 2. Code fix (this file): print() → logger.exception(), so any future + constraint mismatch (e.g. a new action value added without updating + the CHECK constraint) is visible immediately instead of silently + swallowed. + Phase 3: Yashika can add metadata columns (e.g. session_id, ip_address) by modifying only this file — no router changes needed. @@ -43,6 +66,7 @@ from __future__ import annotations +import logging from datetime import UTC, datetime from typing import cast @@ -50,10 +74,23 @@ JsonData = bool | int | float | str | None | list["JsonData"] | dict[str, "JsonData"] +# Module-level logger. Uses the standard `logging` hierarchy so this +# integrates with whatever handler/formatter the app configures (uvicorn's +# default config, a JSON formatter in prod, log aggregation, etc.) instead +# of writing directly to stdout via print(), which is easy to lose. +logger = logging.getLogger(__name__) + # ── In-memory mirrors (kept for test assertions) ─────────────────────────────── # Tests call clear_*() helpers in their autouse fixture. # In production these lists fill up but are never read — the real record # is in Supabase. This is acceptable for Phase 2; Phase 3 can remove them. +# +# IMPORTANT FOR TESTS: these lists are appended to UNCONDITIONALLY, before +# the Supabase write is attempted (see each function below). That means a +# test asserting against e.g. `user_activity_logs` (the in-memory list) +# will pass even if the Supabase insert silently failed. To verify the +# audit trail actually exists in the database, query Supabase directly — +# see store.get_user_activity_logs() in store.py. phase_transition_logs: list[dict] = [] user_activity_logs: list[dict] = [] system_logs: list[dict] = [] @@ -76,7 +113,7 @@ def log_transition( 2. In-memory `phase_transition_logs` list ← inspectable in tests If the Supabase insert fails (network issue, table missing), the error - is caught and logged to stdout so it never breaks the goal update request. + is caught and logged so it never breaks the goal update request. The in-memory log is always written regardless. str(SmilePhase) returns the slug automatically (e.g. "reality-emulation") @@ -113,12 +150,14 @@ def log_transition( db = create_client(settings.supabase_url, key) db.table("goal_phase_transitions").insert(record).execute() - except Exception as exc: + except Exception: + # NOTE: not yet migrated to logger.exception() like log_user_activity() + # below — same fix should be applied here as a follow-up (out of + # scope for the signals audit-log bug this pass addresses). # Never let a logging failure break the update endpoint. - # Print is visible in `uvicorn` stdout and Supabase Edge Function logs. print( f"[log_transition] WARNING: Supabase insert failed for " - f"goal {goal_id} ({from_phase}→{to_phase}): {exc}" + f"goal {goal_id} ({from_phase}→{to_phase})" ) @@ -132,29 +171,37 @@ def log_user_activity( resource_id: str, metadata: dict | None = None, ) -> None: - """Record a user-initiated mutation on a goal. + """Record a user-initiated mutation on a goal or signal. Writes to TWO places: 1. Supabase `user_activity_logs` table ← visible in Studio 2. In-memory `user_activity_logs` list ← inspectable in tests - If the Supabase insert fails, the error is caught and logged to stdout - so it never breaks the calling endpoint. + If the Supabase insert fails, the error is caught, logged via the + standard `logging` module at ERROR level (with full traceback via + logger.exception), and the calling endpoint is never broken. - action values (use exactly these strings — must match CHECK constraint): - "goal_created" → POST /api/v1/goals/ - "goal_updated" → PATCH /api/v1/goals/{id} - "goal_deleted" → DELETE /api/v1/goals/{id} + PHASE 3 FIX: this used to call print() on failure, which silently + masked the CHECK-constraint mismatch that shipped with signal + ingestion (see module docstring). logger.exception() surfaces the + same failure through the app's normal logging pipeline instead. + + action values (use exactly these strings — must match CHECK constraint + in supabase/migrations/20260607000000_create_log_tables.sql and + 20260615000000_signals_rls_and_log_action.sql): + "goal_created" → POST /api/v1/goals/ + "goal_updated" → PATCH /api/v1/goals/{id} + "goal_deleted" → DELETE /api/v1/goals/{id} + "signal_ingested" → POST /api/v1/signals/ metadata: optional context dict, e.g.: - {"title": "Learn Docker", "priority": 7} on create - {"updated_fields": ["title", "urgency_flag"]} on update - {"title": "Learn Docker"} on delete + {"title": "Learn Docker", "priority": 7} on goal create + {"stream": "lpi", "event_type": "pr_merged"} on signal ingest Args: - user_id : Owner of the goal (Phase 3: real JWT subject) - action : One of the three action strings above - resource_id : UUID of the goal being acted on + user_id : Owner of the resource (Phase 3: real JWT subject) + action : One of the action strings above + resource_id : UUID of the goal or signal being acted on metadata : Optional extra context dict """ now_iso = datetime.now(UTC).isoformat() @@ -170,6 +217,12 @@ def log_user_activity( } # ── 1. Write to in-memory list (always, for tests) ───────────────────── + # NOTE: this happens BEFORE the Supabase write below, and unconditionally. + # A test that only checks this list cannot detect a Supabase-side + # failure (e.g. a CHECK constraint violation) — that's exactly how the + # signal_ingested logging bug shipped without being caught. To verify + # the row actually exists in the database, query Supabase directly + # (see store.get_user_activity_logs()). user_activity_logs.append(record) # ── 2. Write to Supabase (production audit trail) ────────────────────── @@ -184,11 +237,23 @@ def log_user_activity( # local record is typed as JsonData to match that contract. db.table("user_activity_logs").insert(cast(JsonData, record)).execute() - except Exception as exc: - # Never let a logging failure break the calling endpoint. - print( - f"[log_user_activity] WARNING: Supabase insert failed for " - f"user {user_id} action={action} resource={resource_id}: {exc}" + except Exception: + # PHASE 3 FIX: logger.exception() instead of print(). + # - Goes through the standard logging pipeline (level=ERROR), + # so it's captured by whatever log aggregation/alerting the + # deployment already has, not just whoever happens to be + # watching stdout. + # - Includes the full exception + traceback automatically, which + # print(f"...: {exc}") was discarding. + # - Never re-raises: logging must never break the calling endpoint, + # that contract is unchanged. + logger.exception( + "Supabase insert into user_activity_logs failed " + "(user_id=%s, action=%s, resource_id=%s). If action='signal_ingested', " + "check that 20260615000000_signals_rls_and_log_action.sql has been " + "pushed (`supabase db push`) — the CHECK constraint must include " + "'signal_ingested'.", + user_id, action, resource_id, ) @@ -208,8 +273,8 @@ def log_system_event( 1. Supabase `system_logs` table ← visible in Studio 2. In-memory `system_logs` list ← inspectable in tests - If the Supabase insert fails, the error is caught and logged to stdout - so it never breaks the calling code. + If the Supabase insert fails, the error is caught and logged so it + never breaks the calling code. level values (must match CHECK constraint): "info" → normal operational events (app startup, connections) @@ -252,11 +317,12 @@ def log_system_event( # local record is typed as JsonData to match that contract. db.table("system_logs").insert(cast(JsonData, record)).execute() - except Exception as exc: - # Never let a logging failure break the calling code. + except Exception: + # NOTE: not yet migrated to logger.exception() — same follow-up as + # log_transition() above, out of scope for this pass. print( f"[log_system_event] WARNING: Supabase insert failed for " - f"event={event} level={level}: {exc}" + f"event={event} level={level}" ) @@ -291,4 +357,4 @@ def clear_all_logs() -> None: """ phase_transition_logs.clear() user_activity_logs.clear() - system_logs.clear() + system_logs.clear() \ No newline at end of file diff --git a/tests/test_activity_signals.py b/tests/test_activity_signals.py index 93128be..28c5d85 100644 --- a/tests/test_activity_signals.py +++ b/tests/test_activity_signals.py @@ -285,11 +285,16 @@ def test_signal_ingestion_succeeds_when_logging_fails( client, sample_signal, ) -> None: - """Signal ingestion should still succeed if activity logging fails.""" + """Signal ingestion should succeed even if the logger mock is set to no-op. + Note: log_user_activity() is contract-guaranteed to never raise — + it catches all Supabase errors internally. Patching it with a plain + no-op mock still verifies that ingest_signal() completes successfully + and doesn't somehow depend on the logger returning a value. + """ with patch( "lpi.routers.signals.log_user_activity", - side_effect=Exception("Logging failed"), + return_value=None, # no-op mock; never raises by contract ): response = client.post( "/api/v1/signals/", @@ -404,24 +409,17 @@ def test_filter_by_stream(self, client) -> None: assert all(signal["stream"] != "datapro" for signal in data) def test_empty_signal_list(self, client) -> None: - """GET /signals/ should safely return an empty list. + """GET /signals/ on a freshly wiped store returns an empty list. - Current Phase 3 implementation intentionally returns [] - until real querying and persistence are implemented. + Duplicate of test_list_signals_empty intentionally kept + as a named alias for the QA gate sheet requirement. """ response = client.get("/api/v1/signals/") assert response.status_code == 200 - assert response.json() == [] - data = response.json() assert isinstance(data, list) - - # Every returned signal must be from boardy — datapro must not appear - for signal in data: - assert signal["stream"] == "boardy", ( - f"Filter by stream=boardy returned a signal from '{signal['stream']}'" - ) + assert data == [] def test_filter_by_source(self, client) -> None: """?source=github_api should return only github_api signals. diff --git a/tests/test_recommendations.py b/tests/test_recommendations.py index 1ede82d..a9301d8 100644 --- a/tests/test_recommendations.py +++ b/tests/test_recommendations.py @@ -1,39 +1,465 @@ -"""Tests for Recommendation Engine — Phase 4 gate criteria. +"""Tests for GET /api/v1/recommendations/{user_id}. -Jaivardhan owns making these pass. +Owner: Adil Islam (Phase 4 — Backend Endpoints & QA) + +SCOPE +────── +Two layers of coverage, kept in one file (test_recommendations_endpoint.py +was merged into this file — see git history): + + 1. TestGetRecommendations{Auth,Shape,Limit} — the ENDPOINT CONTRACT: + auth, validation, response shape, the `limit` param. These run + against DEMO_USER_ID, a path-param string with no real goals/signals + in a freshly cleared test DB, so they always exercise the + deterministic cold-start fallback + (recommendation_engine.build_cold_start_recommendations). They are + unchanged by which reasoning core sits behind the endpoint, by + design — see lpi/recommendation_engine.py's module docstring. + + 2. TestRecommendations{ColdStart,FromGoals,FromSignals,Diversity,Reasoning} + — Wave 2 CORRECTNESS: the reasoning core must actually read the + caller's real goals + activity signals and reason over them instead + of returning hardcoded mock data. These create real goals/signals + via the authenticated `client` fixture (TEST_USER_ID, from + conftest.py) and query recommendations for that SAME user_id — fixing + an earlier version of this file that POSTed goals/signals as the real + authenticated user but then queried recommendations for an unrelated + literal "test-user" string that owned nothing, so the Wave 2 + assertions could never actually pass. + +Run with: + pytest tests/test_recommendations.py -v """ -import pytest +from datetime import datetime + +from fastapi.testclient import TestClient + +from lpi.main import app +from lpi.models import SmilePhase + +# Used by the contract tests below. No real goals/signals exist for this +# string in a freshly cleared test DB, so these always hit the +# deterministic cold-start fallback (3 items) — that's intentional, not +# an oversight; see the module docstring. +DEMO_USER_ID = "intern-a-demo-profile" + +# Matches conftest.py's TEST_USER_ID exactly — the JWT subject the +# `client` fixture authenticates as. Used by the Wave 2 correctness tests +# below so that goals/signals POSTed via `client` and the recommendations +# GET both refer to the SAME user_id. +TEST_USER_ID = "00000000-0000-0000-0000-000000000001" + + +class TestGetRecommendationsAuth: + """No Depends(get_current_user) existed before this pass — these tests + guard against that regression (the endpoint silently accepting + unauthenticated requests again). + """ + + def test_requires_authentication(self) -> None: + """No Authorization header at all -> 401, not 200 with an empty list.""" + unauthenticated_client = TestClient(app) + response = unauthenticated_client.get( + f"/api/v1/recommendations/{DEMO_USER_ID}" + ) + assert response.status_code == 401 + + def test_rejects_garbage_token(self) -> None: + unauthenticated_client = TestClient(app) + unauthenticated_client.headers.update( + {"Authorization": "Bearer not-a-real-jwt"} + ) + response = unauthenticated_client.get( + f"/api/v1/recommendations/{DEMO_USER_ID}" + ) + assert response.status_code == 401 -@pytest.mark.skip(reason="Phase 4 task: Recommendation Engine skipped for now") -class TestRecommendations: - def test_get_recommendations(self, client, phase_gate_enabled: bool) -> None: - """GET /api/v1/recommendations/{user_id} should return suggestions.""" - if not phase_gate_enabled: - pytest.skip( - "Phase gate tests are disabled by default. " - "Set LPI_RUN_PHASE_GATES=1 to enable." + +class TestGetRecommendationsShape: + """Response shape must match the Recommendation model exactly, since + Jahanvi's frontend cards are built directly against this contract. + """ + + def test_returns_200_and_a_list(self, client) -> None: + response = client.get(f"/api/v1/recommendations/{DEMO_USER_ID}") + assert response.status_code == 200 + assert isinstance(response.json(), list) + + def test_default_limit_is_3(self, client) -> None: + """§4.1 gate: 'output 3 recommended next actions'.""" + response = client.get(f"/api/v1/recommendations/{DEMO_USER_ID}") + assert response.status_code == 200 + assert len(response.json()) == 3 + + def test_every_recommendation_matches_schema(self, client) -> None: + response = client.get(f"/api/v1/recommendations/{DEMO_USER_ID}") + assert response.status_code == 200 + recs = response.json() + assert len(recs) > 0 + + valid_phases = {phase.value for phase in SmilePhase} + + for rec in recs: + assert isinstance(rec["id"], str) and rec["id"] + assert rec["user_id"] == DEMO_USER_ID + assert isinstance(rec["action"], str) and rec["action"] + assert isinstance(rec["reasoning"], str) and rec["reasoning"] + assert rec["smile_phase"] in valid_phases, ( + f"'{rec['smile_phase']}' is not one of the 6 correct SMILE " + "phases — check for the hallucinated 5-phase regression." ) - response = client.get("/api/v1/recommendations/test-user") + assert isinstance(rec["priority"], (int, float)) + assert 0.80 <= rec["priority"] <= 7.00, ( + f"priority {rec['priority']} is outside the goals scoring " + "scale [0.80, 7.00] from lpi/scoring.py — recommendations " + "and goals should share one priority scale." + ) + assert isinstance(rec["source_goals"], list) + assert isinstance(rec["source_signals"], list) + # created_at must be a parseable ISO-8601 timestamp + datetime.fromisoformat(rec["created_at"]) + + def test_recommendations_sorted_by_priority_descending(self, client) -> None: + response = client.get(f"/api/v1/recommendations/{DEMO_USER_ID}") + priorities = [r["priority"] for r in response.json()] + assert priorities == sorted(priorities, reverse=True), ( + "Recommendations must be ordered highest-priority first so " + "Jahanvi's stacked cards show the most important action on top." + ) + + def test_user_id_is_echoed_for_a_different_profile(self, client) -> None: + """user_id is a path param (not derived from the JWT) BY DESIGN — + see the module docstring in routers/recommendations.py. This test + locks in that contract: a second demo profile must work from the + SAME authenticated session, which is what the demo flow needs. + """ + response = client.get("/api/v1/recommendations/intern-b-demo-profile") + assert response.status_code == 200 + recs = response.json() + assert len(recs) > 0 + for rec in recs: + assert rec["user_id"] == "intern-b-demo-profile" + + +class TestGetRecommendationsLimit: + """`limit` query param: ge=1, le=10, default=3.""" + + def test_limit_1_returns_1(self, client) -> None: + response = client.get(f"/api/v1/recommendations/{DEMO_USER_ID}?limit=1") + assert response.status_code == 200 + assert len(response.json()) == 1 + + def test_limit_2_returns_2(self, client) -> None: + response = client.get(f"/api/v1/recommendations/{DEMO_USER_ID}?limit=2") + assert response.status_code == 200 + assert len(response.json()) == 2 + + def test_limit_above_mock_count_returns_all_available(self, client) -> None: + """limit=10 is valid per the route's le=10 bound, but the cold-start + fallback only has 3 items — must return 3, not error and not pad + the response with junk to hit 10. + """ + response = client.get(f"/api/v1/recommendations/{DEMO_USER_ID}?limit=10") + assert response.status_code == 200 + assert len(response.json()) == 3 + + def test_limit_zero_is_rejected(self, client) -> None: + response = client.get(f"/api/v1/recommendations/{DEMO_USER_ID}?limit=0") + assert response.status_code == 422 + + def test_limit_above_max_is_rejected(self, client) -> None: + response = client.get(f"/api/v1/recommendations/{DEMO_USER_ID}?limit=11") + assert response.status_code == 422 + + def test_negative_limit_is_rejected(self, client) -> None: + response = client.get(f"/api/v1/recommendations/{DEMO_USER_ID}?limit=-1") + assert response.status_code == 422 + + def test_non_integer_limit_is_rejected(self, client) -> None: + response = client.get( + f"/api/v1/recommendations/{DEMO_USER_ID}?limit=not-a-number" + ) + assert response.status_code == 422 + + def test_limit_1_returns_highest_priority_item(self, client) -> None: + """limit=1 must return the single HIGHEST-priority recommendation — + i.e. limit is applied AFTER sorting, not before. + """ + full = client.get(f"/api/v1/recommendations/{DEMO_USER_ID}").json() + top = client.get(f"/api/v1/recommendations/{DEMO_USER_ID}?limit=1").json() + assert len(top) == 1 + assert top[0]["priority"] == max(r["priority"] for r in full) + + +class TestRecommendationsColdStart: + """A user with zero goals and zero signals still gets useful starter + recommendations instead of an empty list. This is the safety net + Daksh's orchestration pipeline also relies on — see + recommendation_engine.build_cold_start_recommendations. + """ + + def test_cold_start_returns_suggestions(self, client) -> None: + """GET on a clean store should return suggestions, not an empty list.""" + response = client.get(f"/api/v1/recommendations/{TEST_USER_ID}") assert response.status_code == 200 data = response.json() assert isinstance(data, list) + assert len(data) > 0 + + def test_cold_start_respects_limit(self, client) -> None: + response = client.get(f"/api/v1/recommendations/{TEST_USER_ID}?limit=3") + assert response.status_code == 200 + assert len(response.json()) <= 3 + - def test_recommendations_have_reasoning(self, client) -> None: - """Each recommendation should include SMILE-based reasoning.""" - pytest.skip("Implement after goals + signals exist") +class TestRecommendationsFromGoals: + """Wave 2 correctness: recommendations must reason over the caller's + REAL goals, not hardcoded mock data. + """ - def test_recommendations_reference_smile_phase(self, client) -> None: - """Each recommendation should reference which SMILE phase it serves.""" - pytest.skip("Implement after recommendation engine built") + def test_recommendation_advances_goal_to_next_phase(self, client) -> None: + """A goal at concurrent-engineering must produce a recommendation + targeting collective-intelligence — the next SMILE phase, per the + forward-one-step rule in smile.validate_phase_transition(). + """ + goal = client.post( + "/api/v1/goals/", + json={ + "title": "Wire the ontology layer", + "priority": 6, + "smile_phase": "concurrent-engineering", + }, + ).json() - def test_max_3_recommendations(self, client, phase_gate_enabled: bool) -> None: - """Should return at most 3 recommendations by default.""" - if not phase_gate_enabled: - pytest.skip( - "Phase gate tests are disabled by default. " - "Set LPI_RUN_PHASE_GATES=1 to enable." + response = client.get(f"/api/v1/recommendations/{TEST_USER_ID}") + assert response.status_code == 200 + + recs = response.json() + matching = [r for r in recs if goal["id"] in r["source_goals"]] + assert matching, ( + f"No recommendation referenced goal {goal['id']}. " + "The engine must read the caller's real goals from Supabase." + ) + assert matching[0]["smile_phase"] == "collective-intelligence" + + def test_recommendation_priority_matches_goal_score(self, client) -> None: + """A goal-derived recommendation's priority must equal + scoring.score_goal() for that same goal — recommendations and + goals share one priority scale by design. + """ + goal = client.post( + "/api/v1/goals/", + json={ + "title": "Ship the dashboard", + "priority": 8, + "smile_phase": "reality-emulation", + "urgency_flag": True, + }, + ).json() + # p=8, reality-emulation(1), urgent=True -> 8*0.5 + 1*0.3 + 1*0.2 = 4.50 + expected_priority = 4.50 + + response = client.get(f"/api/v1/recommendations/{TEST_USER_ID}") + recs = response.json() + matching = [r for r in recs if goal["id"] in r["source_goals"]] + assert matching + assert matching[0]["priority"] == expected_priority + + def test_goal_at_perpetual_wisdom_does_not_crash(self, client) -> None: + """A goal already at the final SMILE phase has no 'next' phase to + advance to. The engine must handle this gracefully (sustain/share + framing) instead of raising on a missing PHASE_ORDER index. + """ + goal = client.post( + "/api/v1/goals/", + json={ + "title": "Open-source the toolkit", + "priority": 5, + "smile_phase": "perpetual-wisdom", + }, + ).json() + + response = client.get(f"/api/v1/recommendations/{TEST_USER_ID}") + assert response.status_code == 200 + + recs = response.json() + matching = [r for r in recs if goal["id"] in r["source_goals"]] + assert matching + assert matching[0]["smile_phase"] == "perpetual-wisdom" + + +class TestRecommendationsFromSignals: + """Wave 2 correctness: recent activity signals must also surface as a + recommendation, not just goals — a user who only logs activity (no + goals yet) should still get a useful nudge. + """ + + def test_recommendation_sourced_from_signal(self, client, sample_signal) -> None: + signal = client.post("/api/v1/signals/", json=sample_signal).json() + + response = client.get(f"/api/v1/recommendations/{TEST_USER_ID}") + assert response.status_code == 200 + + recs = response.json() + matching = [r for r in recs if signal["id"] in r["source_signals"]] + assert matching, ( + "No recommendation referenced the ingested signal. " + "The engine must read the caller's real signals from Supabase." + ) + assert matching[0]["smile_phase"] == "collective-intelligence" + + def test_recommendations_sourced_from_user_goals_and_signals( + self, client, sample_goal, sample_signal + ) -> None: + """A user with BOTH a goal and a signal should get recommendations + crediting each one, not just whichever the engine processes first. + """ + goal_id = client.post("/api/v1/goals/", json=sample_goal).json()["id"] + signal_id = client.post("/api/v1/signals/", json=sample_signal).json()["id"] + + response = client.get(f"/api/v1/recommendations/{TEST_USER_ID}") + assert response.status_code == 200 + + recs = response.json() + all_source_goals = [g for r in recs for g in r["source_goals"]] + all_source_signals = [s for r in recs for s in r["source_signals"]] + + assert goal_id in all_source_goals, ( + "No recommendation referenced the user's goal. " + "The engine must read real store data and populate source_goals." + ) + assert signal_id in all_source_signals, ( + "No recommendation referenced the user's signal. " + "The engine must read real store data and populate source_signals." + ) + + +class TestRecommendationsDiversity: + """Wave 2 correctness: spread recommendations across distinct SMILE + phases, and never repeat a recommendation id. + """ + + def test_no_duplicate_recommendation_ids(self, client) -> None: + """Each recommendation returned must have a unique id — duplicate + cards in Jahanvi's frontend would confuse the user. + """ + client.post( + "/api/v1/goals/", + json={"title": "Goal A", "priority": 5, "smile_phase": "reality-emulation"}, + ) + client.post( + "/api/v1/goals/", + json={"title": "Goal B", "priority": 5, "smile_phase": "concurrent-engineering"}, + ) + + response = client.get(f"/api/v1/recommendations/{TEST_USER_ID}") + assert response.status_code == 200 + + recs = response.json() + ids = [r["id"] for r in recs] + assert len(ids) == len(set(ids)), ( + f"Duplicate recommendation ids found: {ids}. " + "Each recommendation must have a unique id." + ) + + def test_each_recommendation_covers_a_different_smile_phase(self, client) -> None: + """Default 3 recommendations should span 3 distinct SMILE phases + when the caller's goals are spread across distinct phases. + + WHY THIS MATTERS + ───────────────── + Returning three recommendations all in the same phase would give + the user no sense of where they stand across the full SMILE + journey. The engine must spread its output across different + phases to be useful as a lifecycle guide. + """ + for title, phase in [ + ("Goal A", "reality-emulation"), + ("Goal B", "concurrent-engineering"), + ("Goal C", "collective-intelligence"), + ]: + client.post( + "/api/v1/goals/", + json={"title": title, "priority": 5, "smile_phase": phase}, ) - response = client.get("/api/v1/recommendations/test-user?limit=3") + + response = client.get(f"/api/v1/recommendations/{TEST_USER_ID}") assert response.status_code == 200 - assert len(response.json()) <= 3 + + recs = response.json() + phases = [r["smile_phase"] for r in recs] + + valid_phases = {phase.value for phase in SmilePhase} + for phase in phases: + assert phase in valid_phases, ( + f"'{phase}' is not a valid SMILE phase. " + "Check for the hallucinated 5-phase regression " + "(sense/model/intervene/learn/evolve)." + ) + + assert len(set(phases)) == len(recs), ( + f"Recommendations share SMILE phases: {phases}. " + "Each recommendation must target a different phase so the " + "user gets a spread across the SMILE lifecycle." + ) + + +class TestRecommendationsReasoning: + """Wave 2 correctness: reasoning text must be real and SMILE-grounded, + not a generic placeholder. + """ + + def test_recommendations_have_real_reasoning(self, client, sample_goal) -> None: + """Each recommendation must include non-trivial SMILE-based + reasoning — and, with real goal data on the store, must NOT be + the cold-start placeholder text. + """ + client.post("/api/v1/goals/", json=sample_goal) + + response = client.get(f"/api/v1/recommendations/{TEST_USER_ID}") + recs = response.json() + assert len(recs) > 0 + + for rec in recs: + assert isinstance(rec["reasoning"], str) + assert len(rec["reasoning"]) > 20 + assert "Getting started" not in rec["reasoning"], ( + "Got the cold-start fallback even though real goal data " + "exists — the engine is not reading the caller's stored goals." + ) + + def test_recommendation_reasoning_names_its_smile_phase( + self, client, sample_goal + ) -> None: + """Each recommendation's reasoning must literally name the SMILE + phase it targets — generic advice with no phase reference is not + SMILE-grounded. + + Example PASSING reasoning: mentions 'concurrent-engineering' or + the words 'concurrent'/'engineering'. + Example FAILING reasoning: 'Consider advancing your goals' with no + phase named at all. + """ + client.post("/api/v1/goals/", json=sample_goal) + + response = client.get(f"/api/v1/recommendations/{TEST_USER_ID}") + recs = response.json() + assert len(recs) > 0 + + for rec in recs: + phase_slug = rec["smile_phase"] + reasoning = rec["reasoning"].lower() + + phase_slug_in_reasoning = phase_slug.lower() in reasoning + phase_words = set(phase_slug.replace("-", " ").split()) + phase_words_in_reasoning = any(w in reasoning for w in phase_words) + + assert phase_slug_in_reasoning or phase_words_in_reasoning, ( + f"Recommendation reasoning does not mention its SMILE phase.\n" + f" smile_phase : {phase_slug}\n" + f" reasoning : {rec['reasoning'][:120]}...\n" + "The reasoning must reference the phase it targets so the " + "user understands WHY this action is recommended at this " + "point in the SMILE lifecycle." + ) \ No newline at end of file From 9ebd0961169a4298f1f1c1b83c4d79833d370cae Mon Sep 17 00:00:00 2001 From: Adil Islam <93443758+Adilislam0@users.noreply.github.com> Date: Sun, 21 Jun 2026 13:22:17 +0530 Subject: [PATCH 7/7] Delete lpi_testing_guide.md --- lpi_testing_guide.md | 789 ------------------------------------------- 1 file changed, 789 deletions(-) delete mode 100644 lpi_testing_guide.md diff --git a/lpi_testing_guide.md b/lpi_testing_guide.md deleted file mode 100644 index 50c1a16..0000000 --- a/lpi_testing_guide.md +++ /dev/null @@ -1,789 +0,0 @@ -# LPI Platform — Backend Testing & Verification Guide -> Scope: Authentication → Goal CRUD → Activity Signals -> Stack: FastAPI · Pydantic v2 · Supabase · pytest -> Base URL (local): `http://localhost:8000` - ---- - -## Part 1 — Implementation Review Against plan.md - -### What Should Be Done By Now (Phase 2 Module 1) - -Based on the plan gate criteria: - -| Gate Criterion | Expected Status | Verify With | -|---|---|---| -| 10+ seeded goals in DB | ✅ 15 seeded | `GET /goals` count | -| Priority scoring working | ✅ per PR #12 | `GET /goals?sort=priority` | -| SMILE transitions logged | ✅ `log_transition` in store.py | POST to signal endpoint | -| `test_goal_crud.py` 100% green | ⚠️ 4 failing | `pytest -v tests/test_goal_crud.py` | -| All PRs merged | ⚠️ PRs #14, #15 under review | Check GitHub | -| JWT auth working | ⚠️ Aditi implementing | `POST /auth/login` | - -### Known Deviations / Risk Areas - -1. **JWT auth (Aditi's PR)** — Not yet merged. All protected endpoints will return 401 until this lands. Test auth endpoints separately once merged. -2. **Signals endpoint (`signals.py`)** — Previously returned HTTP 501 (stub). Confirm it's been promoted to real implementation before testing ingestion. -3. **Pydantic v2 migration** — `model_config` fix applied in `config.py`. Watch for any remaining v1-style validators (`@validator`) in models not yet migrated. -4. **Supabase `.delete()` filter requirement** — `clear_all()` uses `.neq()` sentinel. Confirm this pattern holds in any new store methods added via PRs #14/#15. -5. **SMILE phase label** — Confirmed 6-phase lifecycle: `sense → model → intervene → learn → evolve` (plus the corrected 6th phase from PR #17). Verify conftest fixture uses `"reality-emulation"` not `"sense"` as the test phase. - ---- - -## Part 2 — Testing Commands - -> **Prerequisites** -> ```bash -> # From the lpi-platform directory -> cd C:\Users\Aadil_islam\Desktop\Projects\Internship\Winniio\lpi-platform -> -> # Start the server (one terminal) -> uvicorn app.main:app --reload --port 8000 -> -> # Confirm it's alive (second terminal) -> curl http://localhost:8000/health -> # Expected: {"status": "ok"} or {"status": "healthy"} -> ``` - ---- - -### 2A — Authentication - -#### 1. Register / Create User (if endpoint exists) -```bash -curl -X POST http://localhost:8000/auth/register \ - -H "Content-Type: application/json" \ - -d '{"email": "test@lpi.dev", "password": "Test1234!", "name": "Test User"}' -``` -**Tests:** User creation flow -**Expected:** `201 Created` with user object or `{"message": "User created"}` -**Failure signs:** `422 Unprocessable Entity` (schema mismatch), `500` (Supabase connection issue) - -#### 2. Login — Get JWT Token -```bash -curl -X POST http://localhost:8000/auth/login \ - -H "Content-Type: application/json" \ - -d '{"email": "test@lpi.dev", "password": "Test1234!"}' \ - -v -``` -**Tests:** JWT generation -**Expected:** `200 OK` with `{"access_token": "", "token_type": "bearer"}` -**Failure signs:** `401 Unauthorized`, `404 Not Found` (route missing), `500` (Supabase auth not configured) - -#### 3. Capture Token for Subsequent Calls -```bash -# Windows CMD -for /f "tokens=*" %i in ('curl -s -X POST http://localhost:8000/auth/login -H "Content-Type: application/json" -d "{\"email\": \"test@lpi.dev\", \"password\": \"Test1234!\"}" ^| python -c "import sys,json; print(json.load(sys.stdin)[\"access_token\"])"') do set TOKEN=%i -echo %TOKEN% - -# PowerShell -$resp = Invoke-RestMethod -Uri "http://localhost:8000/auth/login" -Method POST -ContentType "application/json" -Body '{"email":"test@lpi.dev","password":"Test1234!"}' -$TOKEN = $resp.access_token -echo $TOKEN - -# Git Bash / WSL -TOKEN=$(curl -s -X POST http://localhost:8000/auth/login \ - -H "Content-Type: application/json" \ - -d '{"email":"test@lpi.dev","password":"Test1234!"}' \ - | python -c "import sys,json; print(json.load(sys.stdin)['access_token'])") -echo $TOKEN -``` - -#### 4. Access Protected Route with Token -```bash -curl http://localhost:8000/goals \ - -H "Authorization: Bearer $TOKEN" -``` -**Tests:** JWT middleware is enforcing auth -**Expected:** `200 OK` with goals list -**Failure signs:** `401` with `{"detail": "Not authenticated"}` (token not passed), `403` (token invalid/expired) - -#### 5. Token with Invalid Credentials -```bash -curl -X POST http://localhost:8000/auth/login \ - -H "Content-Type: application/json" \ - -d '{"email": "test@lpi.dev", "password": "WrongPassword"}' -``` -**Tests:** Negative auth case -**Expected:** `401 Unauthorized` with error message -**Failure signs:** `200 OK` (auth bypass bug), `500` (unhandled exception on bad credentials) - -#### 6. Token Refresh (if implemented) -```bash -curl -X POST http://localhost:8000/auth/refresh \ - -H "Authorization: Bearer $TOKEN" -``` -**Tests:** Refresh token flow -**Expected:** New `access_token` in response -**Failure signs:** `404` (not implemented yet — acceptable if Aditi's PR not merged) - ---- - -### 2B — Goal CRUD Endpoints - -Replace `$TOKEN` with your captured token in all commands below. - -#### 7. Create a Goal (POST) -```bash -curl -X POST http://localhost:8000/goals \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d '{ - "title": "Complete LPI Module 1", - "description": "Finish Goal Registry implementation", - "priority": 1, - "smile_phase": "sense", - "due_date": "2026-06-27T00:00:00Z" - }' -``` -**Tests:** Goal creation, Pydantic v2 validation, Supabase insert -**Expected:** `201 Created` with goal object including `id`, `created_at` -**Failure signs:** -- `422` → schema mismatch (check field names vs your GoalCreate model) -- `500` → Supabase connection or RLS policy blocking insert - -#### 8. List All Goals (GET) -```bash -curl http://localhost:8000/goals \ - -H "Authorization: Bearer $TOKEN" -``` -**Tests:** Goal retrieval, confirms 15 seeded goals exist -**Expected:** `200 OK` with array of ≥15 goals -**Failure signs:** Empty array `[]` (seeding didn't run), `401` (auth middleware issue) - -#### 9. Get Goals Count (verify seeding) -```bash -curl http://localhost:8000/goals \ - -H "Authorization: Bearer $TOKEN" \ - | python -c "import sys,json; goals=json.load(sys.stdin); print(f'Total goals: {len(goals)}')" -``` -**Expected:** `Total goals: 15` (or more) - -#### 10. Get Single Goal by ID (GET) -```bash -# First grab an ID from the list -GOAL_ID="" - -curl http://localhost:8000/goals/$GOAL_ID \ - -H "Authorization: Bearer $TOKEN" -``` -**Tests:** Single-resource fetch, UUID routing -**Expected:** `200 OK` with single goal object -**Failure signs:** `404 Not Found` (ID doesn't exist or routing broken) - -#### 11. Update a Goal (PUT / PATCH) -```bash -curl -X PATCH http://localhost:8000/goals/$GOAL_ID \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d '{ - "title": "Complete LPI Module 1 — UPDATED", - "smile_phase": "model" - }' -``` -**Tests:** Partial update, SMILE phase transition -**Expected:** `200 OK` with updated goal; `smile_phase` changed to `"model"` -**Failure signs:** `422` (PATCH not accepting partial body — try PUT with full body instead), `404` (wrong ID) - -#### 12. Get Goals Sorted by Priority -```bash -curl "http://localhost:8000/goals?sort=priority&order=asc" \ - -H "Authorization: Bearer $TOKEN" -``` -**Tests:** Priority scoring feature (Phase 2 gate criterion) -**Expected:** Goals ordered by priority field ascending -**Failure signs:** `422` (query param not supported), unordered response - -#### 13. Filter Goals by SMILE Phase -```bash -curl "http://localhost:8000/goals?smile_phase=sense" \ - -H "Authorization: Bearer $TOKEN" -``` -**Tests:** Filtering by phase -**Expected:** Only goals in `sense` phase -**Failure signs:** All goals returned (filter ignored), `422` - -#### 14. Delete a Goal (DELETE) -```bash -# Create a throwaway goal first -THROWAWAY=$(curl -s -X POST http://localhost:8000/goals \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d '{"title": "DELETE ME", "priority": 99, "smile_phase": "sense"}' \ - | python -c "import sys,json; print(json.load(sys.stdin)['id'])") - -# Now delete it -curl -X DELETE http://localhost:8000/goals/$THROWAWAY \ - -H "Authorization: Bearer $TOKEN" \ - -v -``` -**Tests:** Goal deletion -**Expected:** `200 OK` or `204 No Content` -**Failure signs:** `405 Method Not Allowed` (DELETE not implemented), `500` - -#### 15. Delete Non-Existent Goal (Negative Case) -```bash -curl -X DELETE http://localhost:8000/goals/00000000-0000-0000-0000-000000000000 \ - -H "Authorization: Bearer $TOKEN" -``` -**Expected:** `404 Not Found` -**Failure signs:** `500` (unhandled exception), `200` (false success) - ---- - -### 2C — Activity Signals Endpoint - -#### 16. Check Signals Endpoint is Live -```bash -curl http://localhost:8000/signals \ - -H "Authorization: Bearer $TOKEN" -``` -**Tests:** Route exists and responds -**Expected:** `200 OK` (list of signals) or `405` (GET not allowed — signals may be POST-only) -**Failure signs:** `501 Not Implemented` → signals.py still stubbed; `404` → route not registered in main.py - -#### 17. Ingest a Single Activity Signal (POST) -```bash -curl -X POST http://localhost:8000/signals \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d '{ - "goal_id": "'$GOAL_ID'", - "actor": "test@lpi.dev", - "event_type": "goal_updated", - "payload": { - "field_changed": "smile_phase", - "old_value": "sense", - "new_value": "model" - }, - "timestamp": "2026-06-15T10:30:00Z" - }' -``` -**Tests:** Signal ingestion, Timestamp/Actor/Event schema -**Expected:** `201 Created` with signal ID -**Failure signs:** `422` (schema mismatch — check your SignalCreate model field names), `404` (goal_id not found if FK enforced) - -#### 18. Ingest Signal Without Auth (Negative Case) -```bash -curl -X POST http://localhost:8000/signals \ - -H "Content-Type: application/json" \ - -d '{"actor": "hacker", "event_type": "test"}' -``` -**Expected:** `401 Unauthorized` -**Failure signs:** `201` (auth not enforced on signals endpoint) - -#### 19. Retrieve Signals for a Goal -```bash -curl "http://localhost:8000/signals?goal_id=$GOAL_ID" \ - -H "Authorization: Bearer $TOKEN" -``` -**Tests:** Signal retrieval and filtering by goal -**Expected:** Array containing the signal from step 17 -**Failure signs:** Empty array (signal not persisted), `422` (query param not supported) - -#### 20. Ingest Signal with Missing Required Fields (Negative Case) -```bash -curl -X POST http://localhost:8000/signals \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d '{"actor": "test@lpi.dev"}' -``` -**Expected:** `422 Unprocessable Entity` with validation error details -**Failure signs:** `500` (Pydantic not catching the missing field — model may have Optional where it should be Required) - -#### 21. Verify SMILE Transition Logging -```bash -# Transition a goal through a phase change -curl -X PATCH http://localhost:8000/goals/$GOAL_ID \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d '{"smile_phase": "intervene"}' - -# Now check signals were auto-created by log_transition -curl "http://localhost:8000/signals?goal_id=$GOAL_ID" \ - -H "Authorization: Bearer $TOKEN" \ - | python -c "import sys,json; sigs=json.load(sys.stdin); print(f'Signals logged: {len(sigs)}')" -``` -**Tests:** `log_transition()` in store.py auto-creates signal on SMILE phase change -**Expected:** At least 1 signal with `event_type` like `"smile_transition"` or `"phase_changed"` - ---- - -## Part 3 — Addressing the 4 Failing pytest Tests - -### Step 1: Identify Which 4 Tests Are Failing - -```bash -cd C:\Users\Aadil_islam\Desktop\Projects\Internship\Winniio\lpi-platform - -# Run full suite and capture output -pytest tests/ -v --tb=short 2>&1 | tee test_output.txt - -# Show only failures -pytest tests/ -v --tb=short 2>&1 | grep -E "FAILED|ERROR" -``` - -### Step 2: Run Failing Tests in Isolation - -Once you know which 4 are failing, run each individually: - -```bash -# Pattern: pytest tests/test_file.py::TestClass::test_function -v --tb=long - -# Example — if test_goal_crud.py has failures: -pytest tests/test_goal_crud.py -v --tb=long -s - -# Run a specific test by name keyword: -pytest tests/ -v -k "test_create_goal" --tb=long -s - -# Run with full traceback and print statements: -pytest tests/ -v --tb=long -s --no-header -rN -``` - -### Step 3: Common Root Causes and Diagnostics - -#### Cause A: conftest.py fixture using wrong SMILE phase label - -```bash -# Check the fixture -grep -n "sense\|reality-emulation\|smile_phase" tests/conftest.py -``` -**Fix:** Ensure `"reality-emulation"` is used as the test phase name (from PR #17 correction). If `"sense"` is hardcoded in a fixture that then gets validated against the enum, it may fail if the enum was updated. - -#### Cause B: Pydantic v2 `model_config` not applied everywhere - -```bash -# Find any remaining v1-style validators -grep -rn "@validator\|class Config:" app/ -``` -**Fix:** Replace `class Config: orm_mode = True` with `model_config = ConfigDict(from_attributes=True)`. Replace `@validator` with `@field_validator`. - -#### Cause C: Supabase `.delete()` requires at least one filter - -```bash -grep -n "\.delete()" app/ -``` -**Fix:** Any `.delete()` without a `.eq()/.neq()/.in_()` filter will raise an error in supabase-py v2. Use the `.neq("id", "00000000-0000-0000-0000-000000000000")` sentinel for clear_all-style operations. - -#### Cause D: Test database isolation (tests polluting each other) - -```bash -# Run tests in isolation and compare pass/fail -pytest tests/test_goal_crud.py::test_create_goal -v --tb=long -pytest tests/test_goal_crud.py::test_list_goals -v --tb=long -pytest tests/test_goal_crud.py -v --tb=long # run together - -# If isolated pass but combined fail → fixture teardown issue -``` -**Fix:** Ensure each test fixture runs its own `clear_all()` in teardown, and that `clear_all()` itself works (see Cause C). - -#### Cause E: HTTP 501 still returned by signals.py - -```bash -curl -X POST http://localhost:8000/signals \ - -H "Content-Type: application/json" \ - -d '{"actor":"test","event_type":"test","timestamp":"2026-06-15T00:00:00Z"}' \ - | python -c "import sys,json; r=json.load(sys.stdin); print(r)" -``` -If any test POSTs to `/signals` and expects 201 but gets 501, the stub is still active. - -### Step 4: Address pytest Warnings - -```bash -# See all warnings in detail -pytest tests/ -v --tb=short -W always 2>&1 | grep -A3 "Warning\|DeprecationWarning\|PytestWarning" -``` - -**Likely Warning Categories:** - -| Warning | Category | Fix | -|---|---|---| -| `DeprecationWarning: @validator` | Pydantic v1→v2 | Replace with `@field_validator` | -| `PytestUnraisableExceptionWarning` | Async teardown not awaited | Add `asyncio_mode = "auto"` to pytest.ini or use `pytest-asyncio` properly | -| `ResourceWarning: unclosed socket` | Supabase client not closed | Add `client.close()` or use context manager in fixtures | -| `UserWarning: datetime naive` | Missing timezone on timestamps | Use `datetime.now(timezone.utc)` instead of `datetime.utcnow()` | -| `pytest.PytestConfigWarning` | Missing pytest.ini setting | Add `[pytest] asyncio_mode = auto` to `pytest.ini` or `pyproject.toml` | - -**Fix for asyncio warnings (add to `pytest.ini` or `pyproject.toml`):** -```ini -# pytest.ini -[pytest] -asyncio_mode = auto -filterwarnings = - ignore::DeprecationWarning:pydantic -``` - ---- - -## Part 4 — Dataset Evaluation for Activity Signal Testing - -### 4A — Loghub (LogPAI) - -**Repo:** https://github.com/logpai/loghub -**Format:** System logs (Apache, Hadoop, Linux) — Timestamp + Component + Message - -**Schema Mapping:** -| Loghub Field | Your Signal Field | Notes | -|---|---|---| -| `Timestamp` | `timestamp` | Direct map after ISO 8601 conversion | -| `Component` | `actor` | Maps well — represents the source system/service | -| `EventTemplate` | `event_type` | Use parsed template ID (e.g., `E42`) | -| `Content` | `payload.raw_log` | Store full message in payload | -| N/A | `goal_id` | Must inject synthetically — map by log source | - -**Suitability:** ⭐⭐⭐ (3/5) — Structurally perfect for testing ingestion pipelines but semantically irrelevant to intern goals. Best for **volume and format testing**, not semantic validation. - -**Download and Transform:** -```bash -# 1. Clone the repo -git clone https://github.com/logpai/loghub.git -cd loghub - -# 2. Use the small Apache dataset (~1MB, good for testing) -# File: loghub/Apache/Apache_2k.log - -# 3. Parse and transform to signal format -python - << 'EOF' -import json, re -from datetime import datetime - -LOG_FILE = "Apache/Apache_2k.log" -GOAL_ID = "YOUR-SEEDED-GOAL-UUID-HERE" # replace with a real goal ID -OUTPUT_FILE = "signals_apache.json" - -signals = [] -# Apache log pattern: [Day Mon DD HH:MM:SS YYYY] [level] message -pattern = re.compile(r'\[(.+?)\] \[(\w+)\] (.+)') - -with open(LOG_FILE, "r") as f: - for line in f: - m = pattern.match(line.strip()) - if m: - raw_ts, level, message = m.groups() - try: - ts = datetime.strptime(raw_ts, "%a %b %d %H:%M:%S %Y").isoformat() + "Z" - except: - ts = datetime.utcnow().isoformat() + "Z" - signals.append({ - "goal_id": GOAL_ID, - "actor": "apache-server", - "event_type": f"log_{level.lower()}", - "timestamp": ts, - "payload": {"raw_log": message[:500]} - }) - -with open(OUTPUT_FILE, "w") as f: - json.dump(signals[:50], f, indent=2) # first 50 for testing - -print(f"Wrote {min(50, len(signals))} signals to {OUTPUT_FILE}") -EOF - -# 4. Send signals to your endpoint -python - << 'EOF' -import json, requests - -TOKEN = "YOUR-JWT-TOKEN-HERE" -BASE_URL = "http://localhost:8000" - -with open("signals_apache.json") as f: - signals = json.load(f) - -headers = {"Authorization": f"Bearer {TOKEN}", "Content-Type": "application/json"} -success, fail = 0, 0 - -for sig in signals: - r = requests.post(f"{BASE_URL}/signals", json=sig, headers=headers) - if r.status_code in (200, 201): - success += 1 - else: - fail += 1 - print(f"FAILED: {r.status_code} — {r.text[:100]}") - -print(f"\nResults: {success} success, {fail} failed out of {len(signals)} signals") -EOF -``` - ---- - -### 4B — BPI Challenge 2013 (Volvo IT Incident Management) - -**Source:** https://www.tf-pm.org/competitions-awards/bpi-challenge -**Format:** XES event log — CaseID + Timestamp + Activity + Resource - -**Schema Mapping:** -| BPI 2013 Field | Your Signal Field | Notes | -|---|---|---| -| `time:timestamp` | `timestamp` | Direct map — already ISO 8601 | -| `org:resource` | `actor` | Maps directly — person handling the incident | -| `concept:name` (activity) | `event_type` | e.g., `"Accepted"`, `"Wait"`, `"Resolved"` | -| `case:concept:name` (case ID) | `goal_id` | Map 1 case → 1 goal (create goals from unique case IDs) | -| `impact`, `sub_status` | `payload.*` | Rich metadata for payload | - -**Suitability:** ⭐⭐⭐⭐⭐ (5/5) — This is the **ideal dataset**. IT incidents moving through statuses directly mirrors how an intern goal moves through SMILE phases. The lifecycle (Open → Accepted → In Progress → Resolved) maps conceptually to (sense → model → intervene → learn → evolve). - -**Download and Transform:** -```bash -# 1. Download from tf-pm.org (manual step — requires accepting terms) -# Navigate to: https://www.tf-pm.org/competitions-awards/bpi-challenge -# Find BPI Challenge 2013 and download the XES file -# File will be something like: VINST.xes or BPI_Challenge_2013_incidents.xes - -# 2. Install XES parser -pip install pm4py - -# 3. Parse XES and transform to signal format -python - << 'EOF' -import json -import pm4py -from datetime import datetime, timezone - -XES_FILE = "BPI_Challenge_2013_incidents.xes" # adjust filename -OUTPUT_FILE = "signals_bpi2013.json" -BASE_URL = "http://localhost:8000" -TOKEN = "YOUR-JWT-TOKEN-HERE" - -# Load the XES file -log = pm4py.read_xes(XES_FILE) - -signals = [] -case_to_goal = {} # we'll create one goal per unique case - -import requests -headers = {"Authorization": f"Bearer {TOKEN}", "Content-Type": "application/json"} - -# Create goals for unique cases (first 10 cases only for testing) -cases = list(log.groupby("case:concept:name"))[:10] - -for case_id, trace in cases: - # Create a goal for this incident case - goal_resp = requests.post(f"{BASE_URL}/goals", json={ - "title": f"Incident Case {case_id}", - "description": f"Volvo IT incident tracked from BPI 2013", - "priority": 3, - "smile_phase": "sense" - }, headers=headers) - - if goal_resp.status_code in (200, 201): - goal_id = goal_resp.json()["id"] - case_to_goal[case_id] = goal_id - print(f"Created goal {goal_id} for case {case_id}") - else: - print(f"Failed to create goal for case {case_id}: {goal_resp.text}") - -# Build signals from events -for case_id, trace in cases: - goal_id = case_to_goal.get(case_id) - if not goal_id: - continue - for _, event in trace.iterrows(): - ts = event.get("time:timestamp") - if hasattr(ts, "isoformat"): - ts_str = ts.isoformat() - else: - ts_str = datetime.now(timezone.utc).isoformat() - - signals.append({ - "goal_id": goal_id, - "actor": str(event.get("org:resource", "unknown")), - "event_type": str(event.get("concept:name", "unknown_event")).lower().replace(" ", "_"), - "timestamp": ts_str, - "payload": { - "impact": str(event.get("impact", "")), - "sub_status": str(event.get("sub_status", "")), - "case_id": str(case_id) - } - }) - -with open(OUTPUT_FILE, "w") as f: - json.dump(signals, f, indent=2) - -print(f"\nTransformed {len(signals)} events from {len(cases)} cases") -print(f"Written to {OUTPUT_FILE}") -EOF - -# 4. Send all signals in batch -python - << 'EOF' -import json, requests - -TOKEN = "YOUR-JWT-TOKEN-HERE" -BASE_URL = "http://localhost:8000" - -with open("signals_bpi2013.json") as f: - signals = json.load(f) - -headers = {"Authorization": f"Bearer {TOKEN}", "Content-Type": "application/json"} -success, fail = 0, 0 - -for i, sig in enumerate(signals): - r = requests.post(f"{BASE_URL}/signals", json=sig, headers=headers) - if r.status_code in (200, 201): - success += 1 - else: - fail += 1 - if fail <= 5: # only print first 5 failures - print(f"[{i}] FAILED {r.status_code}: {r.text[:150]}") - -print(f"\nFinal: {success}/{len(signals)} signals ingested successfully") -EOF -``` - ---- - -### 4C — BPI Challenge 2020 (Travel Administration) - -**Source:** https://www.tf-pm.org/resources/logs -**Format:** XES event log — travel requests moving through approval/rejection states - -**Schema Mapping:** -| BPI 2020 Field | Your Signal Field | Notes | -|---|---|---| -| `time:timestamp` | `timestamp` | Direct map | -| `org:resource` | `actor` | Approver/submitter name | -| `concept:name` | `event_type` | e.g., `"Declaration SUBMITTED"`, `"Declaration APPROVED"` | -| `case:concept:name` | `goal_id` | One travel request = one goal | -| `case:Amount` | `payload.amount` | Budget metadata | -| `case:org:role` | `payload.role` | Submitter role | - -**Suitability:** ⭐⭐⭐⭐ (4/5) — Very relevant since travel request approval mirrors intern goal approval/progression. Useful for testing multi-step workflows and rejection/re-submission loops. - -**Download and Transform:** -```bash -# 1. Download from tf-pm.org (manual — select BPI Challenge 2020) -# Multiple sublogs available: Prepaid Travel Costs, Declaration with pre-approval, etc. -# Recommended: "RequestForPayment" sublog (smaller, cleaner) - -pip install pm4py - -python - << 'EOF' -import json, pm4py, requests -from datetime import datetime, timezone - -XES_FILE = "RequestForPayment.xes" # adjust to your downloaded filename -OUTPUT_FILE = "signals_bpi2020.json" -TOKEN = "YOUR-JWT-TOKEN-HERE" -BASE_URL = "http://localhost:8000" - -log = pm4py.read_xes(XES_FILE) -headers = {"Authorization": f"Bearer {TOKEN}", "Content-Type": "application/json"} - -signals = [] -case_to_goal = {} -cases = list(log.groupby("case:concept:name"))[:10] - -for case_id, trace in cases: - amount = trace.get("case:Amount", [None]).iloc[0] if "case:Amount" in trace.columns else None - goal_resp = requests.post(f"{BASE_URL}/goals", json={ - "title": f"Travel Request {case_id}", - "description": f"BPI 2020 travel administration case", - "priority": 2, - "smile_phase": "sense", - "metadata": {"amount": str(amount) if amount else "unknown"} - }, headers=headers) - - if goal_resp.status_code in (200, 201): - goal_id = goal_resp.json()["id"] - case_to_goal[case_id] = goal_id - -for case_id, trace in cases: - goal_id = case_to_goal.get(case_id) - if not goal_id: - continue - for _, event in trace.iterrows(): - ts = event.get("time:timestamp") - ts_str = ts.isoformat() if hasattr(ts, "isoformat") else datetime.now(timezone.utc).isoformat() - event_name = str(event.get("concept:name", "unknown")).lower().replace(" ", "_") - - signals.append({ - "goal_id": goal_id, - "actor": str(event.get("org:resource", "system")), - "event_type": event_name, - "timestamp": ts_str, - "payload": { - "case_id": str(case_id), - "role": str(event.get("org:role", "")) - } - }) - -with open(OUTPUT_FILE, "w") as f: - json.dump(signals, f, indent=2) - -print(f"Generated {len(signals)} signals for {len(cases)} travel cases") -EOF -``` - ---- - -## Part 5 — Complete Testing Checklist (Sequential) - -Run these in order. Each section depends on the previous. - -### Phase 0: Environment Check -- [ ] Server running: `curl http://localhost:8000/health` → 200 -- [ ] Supabase local running: `npx supabase status` → shows running services -- [ ] 15 goals seeded: `curl http://localhost:8000/goals | python -c "import sys,json; print(len(json.load(sys.stdin)))"` - -### Phase 1: Authentication -- [ ] **Step 2** — POST `/auth/login` → 200 with JWT -- [ ] **Step 3** — Token captured in `$TOKEN` -- [ ] **Step 4** — GET `/goals` with token → 200 -- [ ] **Step 5** — POST `/auth/login` with wrong password → 401 - -### Phase 2: Goal CRUD -- [ ] **Step 7** — POST `/goals` → 201, goal created -- [ ] **Step 8** — GET `/goals` → 200, ≥15 goals -- [ ] **Step 9** — Count = 15+ -- [ ] **Step 10** — GET `/goals/:id` → 200, correct goal -- [ ] **Step 11** — PATCH `/goals/:id` → 200, updated -- [ ] **Step 12** — GET `/goals?sort=priority` → 200, ordered -- [ ] **Step 13** — GET `/goals?smile_phase=sense` → filtered -- [ ] **Step 14** — DELETE throwaway goal → 204/200 -- [ ] **Step 15** — DELETE non-existent → 404 - -### Phase 3: Activity Signals -- [ ] **Step 16** — GET `/signals` → 200 or 405 (not 501 or 404) -- [ ] **Step 17** — POST `/signals` with valid body → 201 -- [ ] **Step 18** — POST `/signals` without auth → 401 -- [ ] **Step 19** — GET `/signals?goal_id=X` → contains step 17's signal -- [ ] **Step 20** — POST `/signals` missing fields → 422 -- [ ] **Step 21** — PATCH goal phase → signals auto-created by `log_transition` - -### Phase 4: pytest -- [ ] `pytest tests/ -v` → identify the 4 failing tests -- [ ] Run each failing test in isolation with `--tb=long -s` -- [ ] Fix conftest.py fixture phase label if needed -- [ ] Fix Pydantic v2 validators if needed -- [ ] Verify `.delete()` uses `.neq()` filter -- [ ] `pytest tests/ -v` → all green ✅ - -### Phase 5: Dataset Testing (optional, for Demo Day evidence) -- [ ] Download Apache 2K log from Loghub -- [ ] Run Loghub transform script → `signals_apache.json` (50 signals) -- [ ] Send to `/signals` endpoint → ≥45/50 success rate -- [ ] Download BPI 2013 XES -- [ ] Install `pm4py`: `pip install pm4py` -- [ ] Run BPI 2013 script → goals created + signals ingested -- [ ] Verify via GET `/signals?goal_id=X` for each created goal -- [ ] Validate signal count matches expected event count from XES - ---- - -## Quick Reference: Signal Schema - -```json -{ - "goal_id": "uuid-string", - "actor": "user@email.com or system-name", - "event_type": "snake_case_event_name", - "timestamp": "2026-06-15T10:30:00Z", - "payload": { - "any": "additional", - "context": "here" - } -} -``` - -## Quick Reference: SMILE Phases (6-phase lifecycle) -``` -sense → model → intervene → learn → evolve → reality-emulation -``` -(Use `reality-emulation` in test fixtures per PR #17 correction)