diff --git a/src/lpi/models.py b/src/lpi/models.py index 9347429..ccc00a5 100644 --- a/src/lpi/models.py +++ b/src/lpi/models.py @@ -175,6 +175,7 @@ class Signal(SignalCreate): id: str user_id: str timestamp: datetime + # ── Recommendation model ────────────────────────────────────────────────────── diff --git a/src/lpi/routers/signals.py b/src/lpi/routers/signals.py index edb0b61..9aee50f 100644 --- a/src/lpi/routers/signals.py +++ b/src/lpi/routers/signals.py @@ -58,6 +58,7 @@ # ── Wave 2: POST /api/v1/signals/ ──────────────────────────────────────────── + @router.post( "/", response_model=Signal, @@ -106,9 +107,9 @@ def ingest_signal( # fields (id, user_id, timestamp) on top. now = datetime.now(UTC) new_signal = Signal( - id=str(uuid.uuid4()), # UUID generated here, not by Postgres - user_id=user_id, # "default_user" in Phase 3 - timestamp=now, # always UTC + id=str(uuid.uuid4()), # UUID generated here, not by Postgres + user_id=user_id, # "default_user" in Phase 3 + timestamp=now, # always UTC **signal.model_dump(), # stream, event_type, payload, source ) @@ -137,11 +138,13 @@ def ingest_signal( # Log to stdout — visible in uvicorn logs. Never breaks the endpoint. print(f"[ingest_signal] WARNING: logging failed for signal {new_signal.id}: {exc}") + print(new_signal.model_dump()) return new_signal # ── Wave 3: GET /api/v1/signals/ ───────────────────────────────────────────── + @router.get( "/", response_model=list[Signal], @@ -169,10 +172,18 @@ def list_signals( "exclude simulated signals." ), ), + start: datetime | None = Query( + default=None, + description="Return signals created at or after this UTC timestamp.", + ), + end: datetime | None = Query( + default=None, + description="Return signals created at or before this UTC timestamp.", + ), limit: int = Query( default=50, - ge=1, # minimum 1 row - le=200, # maximum 200 rows — prevents accidentally huge responses + ge=1, # minimum 1 row + le=200, # maximum 200 rows — prevents accidentally huge responses description=( "Max rows per page (1–200). Use with offset for pagination. " "Default 50 is enough for dashboards and the rec engine." @@ -180,7 +191,7 @@ def list_signals( ), offset: int = Query( default=0, - ge=0, # cannot be negative + ge=0, # cannot be negative description=( "Number of rows to skip. Page 1 = offset 0. " "Page 2 = offset 50 (if limit=50). " @@ -226,6 +237,8 @@ def list_signals( stream=stream, event_type=event_type, source=source, + start=start, + end=end, limit=limit, offset=offset, ) @@ -233,6 +246,7 @@ def list_signals( # ── Bonus: GET /api/v1/signals/{signal_id} ─────────────────────────────────── + @router.get( "/{signal_id}", response_model=Signal, diff --git a/src/lpi/store.py b/src/lpi/store.py index 423d12c..57a6d46 100644 --- a/src/lpi/store.py +++ b/src/lpi/store.py @@ -57,6 +57,7 @@ """ import threading +from datetime import datetime from typing import TYPE_CHECKING, cast from lpi.config import settings @@ -78,6 +79,7 @@ def _get_client() -> "Client": authenticated server-side operations bypass RLS as designed. """ from supabase import create_client # type: ignore[attr-defined] + key = settings.supabase_service_role_key or settings.supabase_key if not key: raise RuntimeError( @@ -99,6 +101,7 @@ def _get_client() -> "Client": # ── Goals ────────────────────────────────────────────────────────────────────── + def get_goal(goal_id: str) -> Goal | None: """Fetch a single goal by its UUID. Returns None if not found.""" result = _get_client().table("goals").select("*").eq("id", goal_id).execute() @@ -133,9 +136,7 @@ def insert_goal(goal: Goal) -> Goal: def update_goal(goal_id: str, updates: dict) -> Goal: """Apply a partial update dict to a goal row. Returns the updated goal.""" - result = ( - _get_client().table("goals").update(updates).eq("id", goal_id).execute() - ) + result = _get_client().table("goals").update(updates).eq("id", goal_id).execute() return Goal(**cast(dict, result.data[0])) @@ -153,6 +154,7 @@ def delete_goal(goal_id: str) -> None: # 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: """Persist a new signal row to the activity_signals Supabase table. @@ -163,9 +165,7 @@ def insert_signal(signal: Signal) -> Signal: Returns the original signal unchanged (Supabase returns the inserted row but we already have it — no need to re-parse it). """ - _get_client().table("activity_signals").insert( - signal.model_dump(mode="json") - ).execute() + _get_client().table("activity_signals").insert(signal.model_dump(mode="json")).execute() return signal @@ -174,6 +174,8 @@ def list_signals( stream: str | None = None, event_type: str | None = None, source: str | None = None, + start: datetime | None = None, + end: datetime | None = None, limit: int = 50, offset: int = 0, ) -> list[Signal]: @@ -232,6 +234,11 @@ def list_signals( query = query.eq("event_type", event_type) if source: query = query.eq("source", source) + if start is not None: + query = query.gte("timestamp", start.isoformat()) + + if end is not None: + query = query.lte("timestamp", end.isoformat()) # Sort newest-first, then apply pagination. # .order("timestamp", desc=True) → ORDER BY timestamp DESC @@ -253,13 +260,7 @@ def get_signal(signal_id: str) -> Signal | None: 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. """ - result = ( - _get_client() - .table("activity_signals") - .select("*") - .eq("id", signal_id) - .execute() - ) + result = _get_client().table("activity_signals").select("*").eq("id", signal_id).execute() if not result.data: return None return Signal(**cast(dict, result.data[0])) @@ -267,6 +268,7 @@ def get_signal(signal_id: str) -> Signal | None: # ── Test helper ─────────────────────────────────────────────────────────────── + def clear_all() -> None: """Wipe all data from goals and activity_signals. Call ONLY from tests. @@ -284,9 +286,7 @@ def clear_all() -> None: in conftest.py, so tests never see each other's data. """ # Wipe all goals rows - _get_client().table("goals").delete().neq( - "user_id", "__sentinel_never_exists__" - ).execute() + _get_client().table("goals").delete().neq("user_id", "__sentinel_never_exists__").execute() # Wipe all activity_signals rows (Phase 3 addition) _get_client().table("activity_signals").delete().neq( diff --git a/tests/conftest.py b/tests/conftest.py index 4b8c7a4..1022f5e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -61,7 +61,8 @@ def _supabase_available() -> bool: try: store.list_goals() return True - except Exception: + except Exception as e: + print("SUPABASE CHECK FAILED:", repr(e)) return False @@ -107,6 +108,10 @@ def client() -> TestClient: test_client.headers.update({"Authorization": f"Bearer {_make_token()}"}) return test_client +@pytest.fixture +def unauthenticated_client() -> TestClient: + """FastAPI test client without Authorization header.""" + return TestClient(app) @pytest.fixture def sample_goal() -> dict: @@ -129,5 +134,6 @@ def sample_signal() -> dict: return { "stream": "boardy", "event_type": "match_created", + "timestamp": "2026-06-11T10:00:00Z", "payload": {"person_a": "Alice", "person_b": "Bob", "score": 0.85}, } diff --git a/tests/test_activity_signals.py b/tests/test_activity_signals.py index 551cce7..93128be 100644 --- a/tests/test_activity_signals.py +++ b/tests/test_activity_signals.py @@ -24,6 +24,8 @@ pytest tests/test_activity_signals.py -v """ +from unittest.mock import patch + class TestIngestSignal: """Tests for POST /api/v1/signals/ @@ -85,6 +87,23 @@ def test_ingest_with_explicit_source(self, client) -> None: data = response.json() assert data["source"] == "github_api" # must be preserved, not overwritten + def test_ingest_requires_auth( + self, + unauthenticated_client, + ) -> None: + signal = { + "stream": "boardy", + "event_type": "match_created", + "payload": {}, + } + + response = unauthenticated_client.post( + "/api/v1/signals/", + json=signal, + ) + + assert response.status_code == 401 + def test_ingest_from_different_streams(self, client) -> None: """Should accept signals from any stream — no allowlist enforced. @@ -104,6 +123,181 @@ def test_ingest_from_different_streams(self, client) -> None: f"got {response.status_code}: {response.text}" ) + def test_missing_stream_field(self, client) -> None: + """Signal ingestion should reject payloads missing the stream field. + + Current Phase 3 behavior: + - router still returns HTTP 501 (stub implementation) + + Future expected behavior: + - FastAPI/Pydantic validation should return HTTP 422 + """ + signal = { + "event_type": "match_created", + "timestamp": "2026-06-11T10:00:00Z", + "payload": {}, + } + + response = client.post("/api/v1/signals/", json=signal) + + # Accept both for now while implementation is incomplete + assert response.status_code == 422 + + def test_missing_event_type(self, client) -> None: + """Signal ingestion should reject payloads missing event_type. + + event_type is required for downstream timeline processing + and recommendation-engine event classification. + """ + signal = { + "stream": "boardy", + "timestamp": "2026-06-11T10:00:00Z", + "payload": {}, + } + + response = client.post("/api/v1/signals/", json=signal) + + assert response.status_code == 422 + + def test_invalid_payload_type(self, client) -> None: + """Signal payload should eventually require a dictionary object. + + Current implementation is still a Phase 3 stub, but this test + prepares validation coverage for malformed payload structures. + """ + signal = { + "stream": "boardy", + "event_type": "match_created", + "timestamp": "2026-06-11T10:00:00Z", + "payload": "invalid_payload", + } + + response = client.post("/api/v1/signals/", json=signal) + + assert response.status_code == 422 + + def test_empty_payload(self, client) -> None: + """Empty payloads should not crash the ingestion endpoint. + + Empty payloads may still be considered valid depending on + stream-specific event schemas in later phases. + """ + signal = { + "stream": "boardy", + "event_type": "match_created", + "timestamp": "2026-06-11T10:00:00Z", + "payload": {}, + } + + response = client.post("/api/v1/signals/", json=signal) + + # Stub currently returns 501 until implementation is wired + assert response.status_code == 201 + + def test_extra_timestamp_field_is_ignored(self, client) -> None: + """Client-supplied timestamp should be ignored. + + The server generates its own timestamp during ingestion. + """ + + signal = { + "stream": "boardy", + "event_type": "match_created", + "timestamp": "not-a-timestamp", + "payload": {}, + } + + response = client.post("/api/v1/signals/", json=signal) + + assert response.status_code == 201 + + data = response.json() + + # Server-generated timestamp should still exist + assert "timestamp" in data + assert data["timestamp"] != signal["timestamp"] + + def test_timestamp_not_required(self, client) -> None: + """Timestamp should not be required in requests. + + The server assigns the ingestion timestamp automatically. + """ + + signal = { + "stream": "boardy", + "event_type": "match_created", + "payload": {}, + } + + response = client.post("/api/v1/signals/", json=signal) + + assert response.status_code in (200, 201) + + data = response.json() + + assert "timestamp" in data + + def test_valid_signal_ingestion(self, client, sample_signal) -> None: + """A valid signal should be accepted and persisted.""" + + response = client.post( + "/api/v1/signals/", + json=sample_signal, + ) + + assert response.status_code == 201 + + data = response.json() + + assert data["stream"] == sample_signal["stream"] + assert data["event_type"] == sample_signal["event_type"] + assert "id" in data + assert "timestamp" in data + + def test_signal_ingestion_logs_activity( + self, + client, + sample_signal, + ) -> None: + """Signal ingestion should log a user activity event.""" + + with patch("lpi.routers.signals.log_user_activity") as mock_log: + response = client.post( + "/api/v1/signals/", + json=sample_signal, + ) + + assert response.status_code == 201 + + mock_log.assert_called_once() + + _, kwargs = mock_log.call_args + + assert kwargs["action"] == "signal_ingested" + assert kwargs["user_id"] == "00000000-0000-0000-0000-000000000001" + + assert kwargs["metadata"]["stream"] == sample_signal["stream"] + assert kwargs["metadata"]["event_type"] == sample_signal["event_type"] + assert kwargs["metadata"]["source"] == "api" + + def test_signal_ingestion_succeeds_when_logging_fails( + self, + client, + sample_signal, + ) -> None: + """Signal ingestion should still succeed if activity logging fails.""" + + with patch( + "lpi.routers.signals.log_user_activity", + side_effect=Exception("Logging failed"), + ): + response = client.post( + "/api/v1/signals/", + json=sample_signal, + ) + + assert response.status_code == 201 + class TestQuerySignals: """Tests for GET /api/v1/signals/ @@ -146,6 +340,31 @@ def test_list_signals(self, client, sample_signal) -> None: for signal in data: assert signal["user_id"] == "00000000-0000-0000-0000-000000000001" + def test_get_signal_by_id( + self, + client, + sample_signal, + ) -> None: + create_response = client.post( + "/api/v1/signals/", + json=sample_signal, + ) + + signal_id = create_response.json()["id"] + + response = client.get(f"/api/v1/signals/{signal_id}") + + assert response.status_code == 200 + assert response.json()["id"] == signal_id + + def test_get_nonexistent_signal_returns_404( + self, + client, + ) -> None: + response = client.get("/api/v1/signals/00000000-0000-0000-0000-000000000000") + + assert response.status_code == 404 + def test_filter_by_stream(self, client) -> None: """?stream=boardy should return only boardy signals. @@ -172,6 +391,28 @@ def test_filter_by_stream(self, client) -> None: # Filter by boardy only response = client.get("/api/v1/signals/?stream=boardy") assert response.status_code == 200 + data = response.json() + + # Should return at least one signal + assert len(data) >= 1 + + # Every returned signal must belong to the boardy stream + for signal in data: + assert signal["stream"] == "boardy" + + # Ensure the datapro signal is not included + 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. + + Current Phase 3 implementation intentionally returns [] + until real querying and persistence are implemented. + """ + response = client.get("/api/v1/signals/") + + assert response.status_code == 200 + assert response.json() == [] data = response.json() assert isinstance(data, list) @@ -215,3 +456,77 @@ def test_filter_by_source(self, client) -> None: assert signal["source"] == "github_api", ( f"Filter source=github_api returned a signal with source='{signal['source']}'" ) + + def test_filter_by_event_type(self, client) -> None: + """?event_type=pr_merged should return only matching event types.""" + + # Insert a PR merged signal + pr_signal = { + "stream": "lpi", + "event_type": "pr_merged", + "payload": {}, + } + client.post("/api/v1/signals/", json=pr_signal) + + # Insert a different event type + match_signal = { + "stream": "boardy", + "event_type": "match_created", + "payload": {}, + } + client.post("/api/v1/signals/", json=match_signal) + + response = client.get("/api/v1/signals/?event_type=pr_merged") + + assert response.status_code == 200 + + data = response.json() + + assert len(data) >= 1 + + for signal in data: + assert signal["event_type"] == "pr_merged" + + assert all(signal["event_type"] != "match_created" for signal in data) + + def test_filter_by_time_range(self, client) -> None: + """Signals should be filtered by timestamp range.""" + + client.post( + "/api/v1/signals/", + json={ + "stream": "lpi", + "event_type": "pr_merged", + "payload": {}, + }, + ) + + response = client.get( + "/api/v1/signals/?start=2000-01-01T00:00:00Z&end=2100-01-01T00:00:00Z" + ) + + assert response.status_code == 200 + + data = response.json() + + assert isinstance(data, list) + assert len(data) >= 1 + + def test_limit_parameter( + self, + client, + ) -> None: + for i in range(5): + client.post( + "/api/v1/signals/", + json={ + "stream": "boardy", + "event_type": f"event_{i}", + "payload": {}, + }, + ) + + response = client.get("/api/v1/signals/?limit=2") + + assert response.status_code == 200 + assert len(response.json()) == 2