Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -75,6 +75,7 @@ ruff format . # formatter — available, not yet CI-gated (see
- Display names are resolved via `services/profiles.py` (`get_display_name`/`get_display_names`), which decrypts off `user_profiles` — don't read name columns off `users`.
- All current LLM calls route through `services/gemini_service.py` (`call_gemini`, `call_gemini_json`, `call_gemini_multiturn`). New LLM-driven code should be written as Pydantic AI agents in `backend/agents/` rather than extending `gemini_service.py`.
- Knowledge-graph mutations go through `services/graph_service.py::apply_graph_update` — routes never write `graph_nodes`/`graph_edges` directly.
- `functools.lru_cache` is reserved for **deterministic, per-process reads** (#98) — either immutable mappings that never need invalidation (e.g. `academics.offering_course_id`) or reads with a matching `clear_*_cache()` hook that every mutator calls (e.g. `course_context_service.get_course_context` is cleared by `update_course_context`). Cache only hashable-arg functions; return a deep copy if the cached value is mutable; never cache without a clear invalidation story. The autouse `_clear_lru_caches` fixture in `tests/conftest.py` resets these between tests.
- Backend tests live in `backend/tests/` and run via `pytest`; shared fixtures (mock Supabase, mock Gemini) are in `tests/conftest.py`.
- Routers are mounted in `main.py` with `/api/<name>` prefixes; new routes follow that pattern.

Expand Down
31 changes: 28 additions & 3 deletions backend/services/academics.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,8 +15,10 @@
"""
from __future__ import annotations

import copy
import uuid
from datetime import date
from functools import lru_cache

from db.connection import table

Expand DownExpand Up@@ -93,8 +95,13 @@ def resolve_offering(
return any_off[0]["id"] if any_off else None


@lru_cache(maxsize=4096)
def offering_course_id(offering_id: str) -> str | None:
"""The abstract course id an offering belongs to (offering → graph bridge)."""
"""The abstract course id an offering belongs to (offering → graph bridge).

Cached per-process (#98): an offering's ``course_id`` is set at creation and
never changes, so this is a deterministic immutable mapping — no invalidation
hook needed. Returns an immutable ``str``/``None`` (safe to share)."""
if not offering_id:
return None
rows = table("course_offerings").select(
Expand All@@ -121,8 +128,8 @@ def user_offering_ids_for_course(user_id: str, course_id: str) -> list[str]:
return [e["offering_id"] for e in enr if e.get("offering_id") in off_ids]


def term_for_offering(offering_id: str) -> dict | None:
"""The term row for an offering (for semester labels)."""
@lru_cache(maxsize=4096)
def _term_for_offering_cached(offering_id: str) -> dict | None:
if not offering_id:
return None
rows = table("course_offerings").select(
Expand All@@ -137,6 +144,24 @@ def term_for_offering(offering_id: str) -> dict | None:
return terms[0] if terms else None


def term_for_offering(offering_id: str) -> dict | None:
"""The term row for an offering (for semester labels).

Cached per-process (#98): the offering→term mapping is immutable and terms
are seeded reference data that don't change at runtime. Returns a deep copy
so callers can't mutate the shared cached row."""
cached = _term_for_offering_cached(offering_id)
return copy.deepcopy(cached) if cached is not None else None


def clear_academics_caches() -> None:
"""Clear the per-process academics caches. Called from test setup (so mocked
DB state doesn't leak across tests); rarely needed at runtime since the
cached mappings are immutable."""
offering_course_id.cache_clear()
_term_for_offering_cached.cache_clear()


def user_enrollment_ids(user_id: str) -> list[dict]:
"""The user's enrollments as ``{id, offering_id}`` rows (read + scoping helper)."""
if not user_id:
Expand Down
36 changes: 28 additions & 8 deletions backend/services/course_context_service.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,9 +11,11 @@
- offering_summary: class-wide summary with Gemini-generated text (per offering)
"""

import copy
import json
import hashlib
from datetime import datetime, timezone
from functools import lru_cache

from db.connection import table
from services.gemini_service import call_gemini
Expand DownExpand Up@@ -65,14 +67,8 @@ def _generate_summary_with_gemini(
)


def get_course_context(offering_id: str) -> dict:
"""
Return the cached class context for an offering: summary + concept stats.
Offering-scoped (one class instance in one term). Returns {} if not found.
"""
if not offering_id:
return {}

@lru_cache(maxsize=512)
def _get_course_context_cached(offering_id: str) -> dict:
try:
# Get the offering summary
summary_rows = table("offering_summary").select(
Expand DownExpand Up@@ -106,6 +102,26 @@ def get_course_context(offering_id: str) -> dict:
return {}


def get_course_context(offering_id: str) -> dict:
"""
Return the cached class context for an offering: summary + concept stats.
Offering-scoped (one class instance in one term). Returns {} if not found.

Cached per-process (#98) and invalidated by ``update_course_context`` (which
runs after graph updates). Returns a deep copy so callers can't mutate the
shared cached value.
"""
if not offering_id:
return {}
return copy.deepcopy(_get_course_context_cached(offering_id))


def clear_course_context_cache() -> None:
"""Drop the per-process course-context cache. Called whenever the underlying
aggregates change (``update_course_context``) and from test setup."""
_get_course_context_cached.cache_clear()


def update_course_context(offering_id: str) -> None:
"""
Aggregate mastery + quiz data for all students enrolled in an **offering**
Expand All@@ -127,6 +143,7 @@ def update_course_context(offering_id: str) -> None:
# No students enrolled — purge any stale aggregates
table("offering_concept_stats").delete({"offering_id": f"eq.{offering_id}"})
table("offering_summary").delete({"offering_id": f"eq.{offering_id}"})
clear_course_context_cache() # #98: aggregates changed → drop cached read
return

user_ids = [r["user_id"] for r in enrollment_rows]
Expand DownExpand Up@@ -335,3 +352,6 @@ def _parse_quiz_context_to_arrays(ctx_rows: list) -> tuple[list, list, list]:
},
on_conflict="offering_id",
)

# #98: the aggregates this offering's context reads from just changed.
clear_course_context_cache()
12 changes: 12 additions & 0 deletions backend/tests/conftest.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,18 @@ def pytest_configure(config):
)


@pytest.fixture(autouse=True)
def _clear_lru_caches():
"""#98: reset the per-process lru_caches around every test so one test's
mocked DB state can't leak into another via a cached read."""
from services import academics, course_context_service
academics.clear_academics_caches()
course_context_service.clear_course_context_cache()
yield
academics.clear_academics_caches()
course_context_service.clear_course_context_cache()


@pytest.fixture(autouse=True)
def _hermetic_supabase_client(request, monkeypatch):
"""Hermetic safety net (#210): no test may make a real Supabase call.
Expand Down
112 changes: 112 additions & 0 deletions backend/tests/test_lru_cache.py
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
"""Tests for the per-process lru_caches (#98): academics resolvers and
course-context, including the invalidation hook.

The autouse `_clear_lru_caches` fixture in conftest clears these before each
test, so each starts with an empty cache."""
from unittest.mock import MagicMock, patch

from services import academics, course_context_service


class TestOfferingCourseIdCache:
def test_second_call_served_from_cache(self):
calls = {"n": 0}

def _table(name):
m = MagicMock()
def _select(*a, **k):
calls["n"] += 1
return [{"course_id": "c1"}]
m.select.side_effect = _select
return m

with patch("services.academics.table", side_effect=_table):
assert academics.offering_course_id("off1") == "c1"
assert academics.offering_course_id("off1") == "c1"
assert calls["n"] == 1 # only the first call hit the DB

def test_distinct_offerings_not_conflated(self):
def _table(name):
m = MagicMock()
def _select(cols, filters=None, **k):
off = filters["id"].split(".")[-1]
return [{"course_id": f"c-{off}"}]
m.select.side_effect = _select
return m

with patch("services.academics.table", side_effect=_table):
assert academics.offering_course_id("A") == "c-A"
assert academics.offering_course_id("B") == "c-B"


class TestTermForOfferingCache:
def _table(self, name):
m = MagicMock()
if name == "course_offerings":
m.select.return_value = [{"term_id": "t1"}]
elif name == "terms":
m.select.return_value = [{"id": "t1", "name": "Fall 2026"}]
else:
m.select.return_value = []
return m

def test_returns_defensive_copy(self):
with patch("services.academics.table", side_effect=self._table):
t1 = academics.term_for_offering("off1")
t1["name"] = "MUTATED" # caller mutates the returned dict
t2 = academics.term_for_offering("off1")
assert t2["name"] == "Fall 2026" # cache not corrupted


class TestCourseContextCacheInvalidation:
def _ctx_table(self, summary_text):
def _table(name):
m = MagicMock()
if name == "offering_summary":
m.select.return_value = [{
"offering_id": "o1", "student_count": 1,
"avg_class_mastery": 0.5, "summary_text": summary_text,
"updated_at": "t",
}]
else:
m.select.return_value = []
return m
return _table

def test_cached_until_explicitly_cleared(self):
with patch("services.course_context_service.table", side_effect=self._ctx_table("v1")):
c1 = course_context_service.get_course_context("o1")
assert c1["course_summary"]["summary_text"] == "v1"

# DB now returns v2, but the cache still serves v1 until cleared.
with patch("services.course_context_service.table", side_effect=self._ctx_table("v2")):
cached = course_context_service.get_course_context("o1")
assert cached["course_summary"]["summary_text"] == "v1"
course_context_service.clear_course_context_cache()
fresh = course_context_service.get_course_context("o1")
assert fresh["course_summary"]["summary_text"] == "v2"

def test_update_course_context_invalidates(self):
# Prime the cache with v1.
with patch("services.course_context_service.table", side_effect=self._ctx_table("v1")):
course_context_service.get_course_context("o1")

# update_course_context with no enrollments → purge path → cache_clear.
def _no_enrollments(name):
m = MagicMock()
m.select.return_value = []
return m
with patch("services.course_context_service.table", side_effect=_no_enrollments):
course_context_service.update_course_context("o1")

# Cache was invalidated, so the next read re-fetches (now v2).
with patch("services.course_context_service.table", side_effect=self._ctx_table("v2")):
after = course_context_service.get_course_context("o1")
assert after["course_summary"]["summary_text"] == "v2"

def test_returns_defensive_copy(self):
with patch("services.course_context_service.table", side_effect=self._ctx_table("v1")):
a = course_context_service.get_course_context("o1")
a["course_summary"]["summary_text"] = "MUTATED"
b = course_context_service.get_course_context("o1")
assert b["course_summary"]["summary_text"] == "v1" # cache intact
48 changes: 48 additions & 0 deletions specs/98-lru-cache.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
# Spec: functools.lru_cache for hot deterministic reads (#98)

## Scope

Milestone #2 (perf). Per-process (per-worker) caching of hot, deterministic reads with a clear
invalidation story. Composes with the HTTP cache (#99) and a future cross-worker Redis layer (#97).

## Requirements

### R1 — Cache immutable academics resolvers (no invalidation needed)
- `services/academics.offering_course_id(offering_id)` — `@lru_cache`. An offering's `course_id` is set
at creation and never changes → deterministic immutable mapping; returns an immutable `str`/`None`.
- `services/academics.term_for_offering(offering_id)` — `@lru_cache` on the private body; the public
function returns a `copy.deepcopy` (mutable dict) so callers can't corrupt the cache. Offering→term is
immutable and terms are seeded reference data.
- `services/academics.clear_academics_caches()` clears both (for tests).

### R2 — Cache course-context with an invalidation hook
- `services/course_context_service.get_course_context(offering_id)` — `@lru_cache` on the private body;
the public function returns a `copy.deepcopy`.
- `clear_course_context_cache()` is called from `update_course_context` on **every** write path (the
no-enrollment purge and the final upsert). `update_course_context` is the choke point that
`apply_graph_update` and the doc/grade post-rolls funnel through, so a graph/doc/grade change that
moves the aggregates always drops the stale cached read.

### R3 — Test isolation
- An autouse `_clear_lru_caches` fixture in `tests/conftest.py` clears all these caches around every test
so one test's mocked DB state can't leak into another via a cached read.

### R4 — Docs
- CLAUDE.md Conventions: `lru_cache` is reserved for deterministic per-process reads — immutable mappings
(no invalidation) or reads with a matching `clear_*_cache()` every mutator calls; hashable args only;
deep-copy mutable returns.

## Acceptance
1. 3+ functions cached, each provably safe (immutable, or hooked + deep-copied).
2. `clear_course_context_cache()` invoked from `update_course_context`'s write paths.
3. CLAUDE.md documents the convention.
4. Tests: cache hit avoids a 2nd DB read; distinct keys not conflated; deep-copy immunity; mutate via
`update_course_context` → next read returns fresh.
5. `pytest tests/ -q` shows no new failures vs `main` (proves the conftest fixture stops cache leakage);
`ruff` clean.

## Deliberately NOT cached (documented risk)
- Graph reads (`get_graph`) — large mutable structures with hot, per-turn invalidation; deep-copy cost +
invalidation surface outweigh the win here. Revisit with the Redis layer (#97).
- Token decode / `require_self` — security-sensitive and needs a TTL bounded to token lifetime; out of
scope for a plain `lru_cache`.
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -75,6 +75,7 @@ ruff format . # formatter — available, not yet CI-gated (see
- Display names are resolved via `services/profiles.py` (`get_display_name`/`get_display_names`), which decrypts off `user_profiles` — don't read name columns off `users`.
- All current LLM calls route through `services/gemini_service.py` (`call_gemini`, `call_gemini_json`, `call_gemini_multiturn`). New LLM-driven code should be written as Pydantic AI agents in `backend/agents/` rather than extending `gemini_service.py`.
- Knowledge-graph mutations go through `services/graph_service.py::apply_graph_update` — routes never write `graph_nodes`/`graph_edges` directly.
- `functools.lru_cache` is reserved for **deterministic, per-process reads** (#98) — either immutable mappings that never need invalidation (e.g. `academics.offering_course_id`) or reads with a matching `clear_*_cache()` hook that every mutator calls (e.g. `course_context_service.get_course_context` is cleared by `update_course_context`). Cache only hashable-arg functions; return a deep copy if the cached value is mutable; never cache without a clear invalidation story. The autouse `_clear_lru_caches` fixture in `tests/conftest.py` resets these between tests.
- Backend tests live in `backend/tests/` and run via `pytest`; shared fixtures (mock Supabase, mock Gemini) are in `tests/conftest.py`.
- Routers are mounted in `main.py` with `/api/<name>` prefixes; new routes follow that pattern.

Expand Down
31 changes: 28 additions & 3 deletions backend/services/academics.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,8 +15,10 @@
"""
from __future__ import annotations

import copy
import uuid
from datetime import date
from functools import lru_cache

from db.connection import table

Expand DownExpand Up@@ -93,8 +95,13 @@ def resolve_offering(
return any_off[0]["id"] if any_off else None


@lru_cache(maxsize=4096)
def offering_course_id(offering_id: str) -> str | None:
"""The abstract course id an offering belongs to (offering → graph bridge)."""
"""The abstract course id an offering belongs to (offering → graph bridge).

Cached per-process (#98): an offering's ``course_id`` is set at creation and
never changes, so this is a deterministic immutable mapping — no invalidation
hook needed. Returns an immutable ``str``/``None`` (safe to share)."""
if not offering_id:
return None
rows = table("course_offerings").select(
Expand All@@ -121,8 +128,8 @@ def user_offering_ids_for_course(user_id: str, course_id: str) -> list[str]:
return [e["offering_id"] for e in enr if e.get("offering_id") in off_ids]


def term_for_offering(offering_id: str) -> dict | None:
"""The term row for an offering (for semester labels)."""
@lru_cache(maxsize=4096)
def _term_for_offering_cached(offering_id: str) -> dict | None:
if not offering_id:
return None
rows = table("course_offerings").select(
Expand All@@ -137,6 +144,24 @@ def term_for_offering(offering_id: str) -> dict | None:
return terms[0] if terms else None


def term_for_offering(offering_id: str) -> dict | None:
"""The term row for an offering (for semester labels).

Cached per-process (#98): the offering→term mapping is immutable and terms
are seeded reference data that don't change at runtime. Returns a deep copy
so callers can't mutate the shared cached row."""
cached = _term_for_offering_cached(offering_id)
return copy.deepcopy(cached) if cached is not None else None


def clear_academics_caches() -> None:
"""Clear the per-process academics caches. Called from test setup (so mocked
DB state doesn't leak across tests); rarely needed at runtime since the
cached mappings are immutable."""
offering_course_id.cache_clear()
_term_for_offering_cached.cache_clear()


def user_enrollment_ids(user_id: str) -> list[dict]:
"""The user's enrollments as ``{id, offering_id}`` rows (read + scoping helper)."""
if not user_id:
Expand Down
36 changes: 28 additions & 8 deletions backend/services/course_context_service.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,9 +11,11 @@
- offering_summary: class-wide summary with Gemini-generated text (per offering)
"""

import copy
import json
import hashlib
from datetime import datetime, timezone
from functools import lru_cache

from db.connection import table
from services.gemini_service import call_gemini
Expand DownExpand Up@@ -65,14 +67,8 @@ def _generate_summary_with_gemini(
)


def get_course_context(offering_id: str) -> dict:
"""
Return the cached class context for an offering: summary + concept stats.
Offering-scoped (one class instance in one term). Returns {} if not found.
"""
if not offering_id:
return {}

@lru_cache(maxsize=512)
def _get_course_context_cached(offering_id: str) -> dict:
try:
# Get the offering summary
summary_rows = table("offering_summary").select(
Expand DownExpand Up@@ -106,6 +102,26 @@ def get_course_context(offering_id: str) -> dict:
return {}


def get_course_context(offering_id: str) -> dict:
"""
Return the cached class context for an offering: summary + concept stats.
Offering-scoped (one class instance in one term). Returns {} if not found.

Cached per-process (#98) and invalidated by ``update_course_context`` (which
runs after graph updates). Returns a deep copy so callers can't mutate the
shared cached value.
"""
if not offering_id:
return {}
return copy.deepcopy(_get_course_context_cached(offering_id))


def clear_course_context_cache() -> None:
"""Drop the per-process course-context cache. Called whenever the underlying
aggregates change (``update_course_context``) and from test setup."""
_get_course_context_cached.cache_clear()


def update_course_context(offering_id: str) -> None:
"""
Aggregate mastery + quiz data for all students enrolled in an **offering**
Expand All@@ -127,6 +143,7 @@ def update_course_context(offering_id: str) -> None:
# No students enrolled — purge any stale aggregates
table("offering_concept_stats").delete({"offering_id": f"eq.{offering_id}"})
table("offering_summary").delete({"offering_id": f"eq.{offering_id}"})
clear_course_context_cache() # #98: aggregates changed → drop cached read
return

user_ids = [r["user_id"] for r in enrollment_rows]
Expand DownExpand Up@@ -335,3 +352,6 @@ def _parse_quiz_context_to_arrays(ctx_rows: list) -> tuple[list, list, list]:
},
on_conflict="offering_id",
)

# #98: the aggregates this offering's context reads from just changed.
clear_course_context_cache()
12 changes: 12 additions & 0 deletions backend/tests/conftest.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,18 @@ def pytest_configure(config):
)


@pytest.fixture(autouse=True)
def _clear_lru_caches():
"""#98: reset the per-process lru_caches around every test so one test's
mocked DB state can't leak into another via a cached read."""
from services import academics, course_context_service
academics.clear_academics_caches()
course_context_service.clear_course_context_cache()
yield
academics.clear_academics_caches()
course_context_service.clear_course_context_cache()


@pytest.fixture(autouse=True)
def _hermetic_supabase_client(request, monkeypatch):
"""Hermetic safety net (#210): no test may make a real Supabase call.
Expand Down
112 changes: 112 additions & 0 deletions backend/tests/test_lru_cache.py
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
"""Tests for the per-process lru_caches (#98): academics resolvers and
course-context, including the invalidation hook.

The autouse `_clear_lru_caches` fixture in conftest clears these before each
test, so each starts with an empty cache."""
from unittest.mock import MagicMock, patch

from services import academics, course_context_service


class TestOfferingCourseIdCache:
def test_second_call_served_from_cache(self):
calls = {"n": 0}

def _table(name):
m = MagicMock()
def _select(*a, **k):
calls["n"] += 1
return [{"course_id": "c1"}]
m.select.side_effect = _select
return m

with patch("services.academics.table", side_effect=_table):
assert academics.offering_course_id("off1") == "c1"
assert academics.offering_course_id("off1") == "c1"
assert calls["n"] == 1 # only the first call hit the DB

def test_distinct_offerings_not_conflated(self):
def _table(name):
m = MagicMock()
def _select(cols, filters=None, **k):
off = filters["id"].split(".")[-1]
return [{"course_id": f"c-{off}"}]
m.select.side_effect = _select
return m

with patch("services.academics.table", side_effect=_table):
assert academics.offering_course_id("A") == "c-A"
assert academics.offering_course_id("B") == "c-B"


class TestTermForOfferingCache:
def _table(self, name):
m = MagicMock()
if name == "course_offerings":
m.select.return_value = [{"term_id": "t1"}]
elif name == "terms":
m.select.return_value = [{"id": "t1", "name": "Fall 2026"}]
else:
m.select.return_value = []
return m

def test_returns_defensive_copy(self):
with patch("services.academics.table", side_effect=self._table):
t1 = academics.term_for_offering("off1")
t1["name"] = "MUTATED" # caller mutates the returned dict
t2 = academics.term_for_offering("off1")
assert t2["name"] == "Fall 2026" # cache not corrupted


class TestCourseContextCacheInvalidation:
def _ctx_table(self, summary_text):
def _table(name):
m = MagicMock()
if name == "offering_summary":
m.select.return_value = [{
"offering_id": "o1", "student_count": 1,
"avg_class_mastery": 0.5, "summary_text": summary_text,
"updated_at": "t",
}]
else:
m.select.return_value = []
return m
return _table

def test_cached_until_explicitly_cleared(self):
with patch("services.course_context_service.table", side_effect=self._ctx_table("v1")):
c1 = course_context_service.get_course_context("o1")
assert c1["course_summary"]["summary_text"] == "v1"

# DB now returns v2, but the cache still serves v1 until cleared.
with patch("services.course_context_service.table", side_effect=self._ctx_table("v2")):
cached = course_context_service.get_course_context("o1")
assert cached["course_summary"]["summary_text"] == "v1"
course_context_service.clear_course_context_cache()
fresh = course_context_service.get_course_context("o1")
assert fresh["course_summary"]["summary_text"] == "v2"

def test_update_course_context_invalidates(self):
# Prime the cache with v1.
with patch("services.course_context_service.table", side_effect=self._ctx_table("v1")):
course_context_service.get_course_context("o1")

# update_course_context with no enrollments → purge path → cache_clear.
def _no_enrollments(name):
m = MagicMock()
m.select.return_value = []
return m
with patch("services.course_context_service.table", side_effect=_no_enrollments):
course_context_service.update_course_context("o1")

# Cache was invalidated, so the next read re-fetches (now v2).
with patch("services.course_context_service.table", side_effect=self._ctx_table("v2")):
after = course_context_service.get_course_context("o1")
assert after["course_summary"]["summary_text"] == "v2"

def test_returns_defensive_copy(self):
with patch("services.course_context_service.table", side_effect=self._ctx_table("v1")):
a = course_context_service.get_course_context("o1")
a["course_summary"]["summary_text"] = "MUTATED"
b = course_context_service.get_course_context("o1")
assert b["course_summary"]["summary_text"] == "v1" # cache intact
48 changes: 48 additions & 0 deletions specs/98-lru-cache.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
# Spec: functools.lru_cache for hot deterministic reads (#98)

## Scope

Milestone #2 (perf). Per-process (per-worker) caching of hot, deterministic reads with a clear
invalidation story. Composes with the HTTP cache (#99) and a future cross-worker Redis layer (#97).

## Requirements

### R1 — Cache immutable academics resolvers (no invalidation needed)
- `services/academics.offering_course_id(offering_id)` — `@lru_cache`. An offering's `course_id` is set
at creation and never changes → deterministic immutable mapping; returns an immutable `str`/`None`.
- `services/academics.term_for_offering(offering_id)` — `@lru_cache` on the private body; the public
function returns a `copy.deepcopy` (mutable dict) so callers can't corrupt the cache. Offering→term is
immutable and terms are seeded reference data.
- `services/academics.clear_academics_caches()` clears both (for tests).

### R2 — Cache course-context with an invalidation hook
- `services/course_context_service.get_course_context(offering_id)` — `@lru_cache` on the private body;
the public function returns a `copy.deepcopy`.
- `clear_course_context_cache()` is called from `update_course_context` on **every** write path (the
no-enrollment purge and the final upsert). `update_course_context` is the choke point that
`apply_graph_update` and the doc/grade post-rolls funnel through, so a graph/doc/grade change that
moves the aggregates always drops the stale cached read.

### R3 — Test isolation
- An autouse `_clear_lru_caches` fixture in `tests/conftest.py` clears all these caches around every test
so one test's mocked DB state can't leak into another via a cached read.

### R4 — Docs
- CLAUDE.md Conventions: `lru_cache` is reserved for deterministic per-process reads — immutable mappings
(no invalidation) or reads with a matching `clear_*_cache()` every mutator calls; hashable args only;
deep-copy mutable returns.

## Acceptance
1. 3+ functions cached, each provably safe (immutable, or hooked + deep-copied).
2. `clear_course_context_cache()` invoked from `update_course_context`'s write paths.
3. CLAUDE.md documents the convention.
4. Tests: cache hit avoids a 2nd DB read; distinct keys not conflated; deep-copy immunity; mutate via
`update_course_context` → next read returns fresh.
5. `pytest tests/ -q` shows no new failures vs `main` (proves the conftest fixture stops cache leakage);
`ruff` clean.

## Deliberately NOT cached (documented risk)
- Graph reads (`get_graph`) — large mutable structures with hot, per-turn invalidation; deep-copy cost +
invalidation surface outweigh the win here. Revisit with the Redis layer (#97).
- Token decode / `require_self` — security-sensitive and needs a TTL bounded to token lifetime; out of
scope for a plain `lru_cache`.
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -75,6 +75,7 @@ ruff format . # formatter — available, not yet CI-gated (see
- Display names are resolved via `services/profiles.py` (`get_display_name`/`get_display_names`), which decrypts off `user_profiles` — don't read name columns off `users`.
- All current LLM calls route through `services/gemini_service.py` (`call_gemini`, `call_gemini_json`, `call_gemini_multiturn`). New LLM-driven code should be written as Pydantic AI agents in `backend/agents/` rather than extending `gemini_service.py`.
- Knowledge-graph mutations go through `services/graph_service.py::apply_graph_update` — routes never write `graph_nodes`/`graph_edges` directly.
- `functools.lru_cache` is reserved for **deterministic, per-process reads** (#98) — either immutable mappings that never need invalidation (e.g. `academics.offering_course_id`) or reads with a matching `clear_*_cache()` hook that every mutator calls (e.g. `course_context_service.get_course_context` is cleared by `update_course_context`). Cache only hashable-arg functions; return a deep copy if the cached value is mutable; never cache without a clear invalidation story. The autouse `_clear_lru_caches` fixture in `tests/conftest.py` resets these between tests.
- Backend tests live in `backend/tests/` and run via `pytest`; shared fixtures (mock Supabase, mock Gemini) are in `tests/conftest.py`.
- Routers are mounted in `main.py` with `/api/<name>` prefixes; new routes follow that pattern.

Expand Down
31 changes: 28 additions & 3 deletions backend/services/academics.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,8 +15,10 @@
"""
from __future__ import annotations

import copy
import uuid
from datetime import date
from functools import lru_cache

from db.connection import table

Expand DownExpand Up@@ -93,8 +95,13 @@ def resolve_offering(
return any_off[0]["id"] if any_off else None


@lru_cache(maxsize=4096)
def offering_course_id(offering_id: str) -> str | None:
"""The abstract course id an offering belongs to (offering → graph bridge)."""
"""The abstract course id an offering belongs to (offering → graph bridge).

Cached per-process (#98): an offering's ``course_id`` is set at creation and
never changes, so this is a deterministic immutable mapping — no invalidation
hook needed. Returns an immutable ``str``/``None`` (safe to share)."""
if not offering_id:
return None
rows = table("course_offerings").select(
Expand All@@ -121,8 +128,8 @@ def user_offering_ids_for_course(user_id: str, course_id: str) -> list[str]:
return [e["offering_id"] for e in enr if e.get("offering_id") in off_ids]


def term_for_offering(offering_id: str) -> dict | None:
"""The term row for an offering (for semester labels)."""
@lru_cache(maxsize=4096)
def _term_for_offering_cached(offering_id: str) -> dict | None:
if not offering_id:
return None
rows = table("course_offerings").select(
Expand All@@ -137,6 +144,24 @@ def term_for_offering(offering_id: str) -> dict | None:
return terms[0] if terms else None


def term_for_offering(offering_id: str) -> dict | None:
"""The term row for an offering (for semester labels).

Cached per-process (#98): the offering→term mapping is immutable and terms
are seeded reference data that don't change at runtime. Returns a deep copy
so callers can't mutate the shared cached row."""
cached = _term_for_offering_cached(offering_id)
return copy.deepcopy(cached) if cached is not None else None


def clear_academics_caches() -> None:
"""Clear the per-process academics caches. Called from test setup (so mocked
DB state doesn't leak across tests); rarely needed at runtime since the
cached mappings are immutable."""
offering_course_id.cache_clear()
_term_for_offering_cached.cache_clear()


def user_enrollment_ids(user_id: str) -> list[dict]:
"""The user's enrollments as ``{id, offering_id}`` rows (read + scoping helper)."""
if not user_id:
Expand Down
36 changes: 28 additions & 8 deletions backend/services/course_context_service.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,9 +11,11 @@
- offering_summary: class-wide summary with Gemini-generated text (per offering)
"""

import copy
import json
import hashlib
from datetime import datetime, timezone
from functools import lru_cache

from db.connection import table
from services.gemini_service import call_gemini
Expand DownExpand Up@@ -65,14 +67,8 @@ def _generate_summary_with_gemini(
)


def get_course_context(offering_id: str) -> dict:
"""
Return the cached class context for an offering: summary + concept stats.
Offering-scoped (one class instance in one term). Returns {} if not found.
"""
if not offering_id:
return {}

@lru_cache(maxsize=512)
def _get_course_context_cached(offering_id: str) -> dict:
try:
# Get the offering summary
summary_rows = table("offering_summary").select(
Expand DownExpand Up@@ -106,6 +102,26 @@ def get_course_context(offering_id: str) -> dict:
return {}


def get_course_context(offering_id: str) -> dict:
"""
Return the cached class context for an offering: summary + concept stats.
Offering-scoped (one class instance in one term). Returns {} if not found.

Cached per-process (#98) and invalidated by ``update_course_context`` (which
runs after graph updates). Returns a deep copy so callers can't mutate the
shared cached value.
"""
if not offering_id:
return {}
return copy.deepcopy(_get_course_context_cached(offering_id))


def clear_course_context_cache() -> None:
"""Drop the per-process course-context cache. Called whenever the underlying
aggregates change (``update_course_context``) and from test setup."""
_get_course_context_cached.cache_clear()


def update_course_context(offering_id: str) -> None:
"""
Aggregate mastery + quiz data for all students enrolled in an **offering**
Expand All@@ -127,6 +143,7 @@ def update_course_context(offering_id: str) -> None:
# No students enrolled — purge any stale aggregates
table("offering_concept_stats").delete({"offering_id": f"eq.{offering_id}"})
table("offering_summary").delete({"offering_id": f"eq.{offering_id}"})
clear_course_context_cache() # #98: aggregates changed → drop cached read
return

user_ids = [r["user_id"] for r in enrollment_rows]
Expand DownExpand Up@@ -335,3 +352,6 @@ def _parse_quiz_context_to_arrays(ctx_rows: list) -> tuple[list, list, list]:
},
on_conflict="offering_id",
)

# #98: the aggregates this offering's context reads from just changed.
clear_course_context_cache()
12 changes: 12 additions & 0 deletions backend/tests/conftest.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,18 @@ def pytest_configure(config):
)


@pytest.fixture(autouse=True)
def _clear_lru_caches():
"""#98: reset the per-process lru_caches around every test so one test's
mocked DB state can't leak into another via a cached read."""
from services import academics, course_context_service
academics.clear_academics_caches()
course_context_service.clear_course_context_cache()
yield
academics.clear_academics_caches()
course_context_service.clear_course_context_cache()


@pytest.fixture(autouse=True)
def _hermetic_supabase_client(request, monkeypatch):
"""Hermetic safety net (#210): no test may make a real Supabase call.
Expand Down
112 changes: 112 additions & 0 deletions backend/tests/test_lru_cache.py
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
"""Tests for the per-process lru_caches (#98): academics resolvers and
course-context, including the invalidation hook.

The autouse `_clear_lru_caches` fixture in conftest clears these before each
test, so each starts with an empty cache."""
from unittest.mock import MagicMock, patch

from services import academics, course_context_service


class TestOfferingCourseIdCache:
def test_second_call_served_from_cache(self):
calls = {"n": 0}

def _table(name):
m = MagicMock()
def _select(*a, **k):
calls["n"] += 1
return [{"course_id": "c1"}]
m.select.side_effect = _select
return m

with patch("services.academics.table", side_effect=_table):
assert academics.offering_course_id("off1") == "c1"
assert academics.offering_course_id("off1") == "c1"
assert calls["n"] == 1 # only the first call hit the DB

def test_distinct_offerings_not_conflated(self):
def _table(name):
m = MagicMock()
def _select(cols, filters=None, **k):
off = filters["id"].split(".")[-1]
return [{"course_id": f"c-{off}"}]
m.select.side_effect = _select
return m

with patch("services.academics.table", side_effect=_table):
assert academics.offering_course_id("A") == "c-A"
assert academics.offering_course_id("B") == "c-B"


class TestTermForOfferingCache:
def _table(self, name):
m = MagicMock()
if name == "course_offerings":
m.select.return_value = [{"term_id": "t1"}]
elif name == "terms":
m.select.return_value = [{"id": "t1", "name": "Fall 2026"}]
else:
m.select.return_value = []
return m

def test_returns_defensive_copy(self):
with patch("services.academics.table", side_effect=self._table):
t1 = academics.term_for_offering("off1")
t1["name"] = "MUTATED" # caller mutates the returned dict
t2 = academics.term_for_offering("off1")
assert t2["name"] == "Fall 2026" # cache not corrupted


class TestCourseContextCacheInvalidation:
def _ctx_table(self, summary_text):
def _table(name):
m = MagicMock()
if name == "offering_summary":
m.select.return_value = [{
"offering_id": "o1", "student_count": 1,
"avg_class_mastery": 0.5, "summary_text": summary_text,
"updated_at": "t",
}]
else:
m.select.return_value = []
return m
return _table

def test_cached_until_explicitly_cleared(self):
with patch("services.course_context_service.table", side_effect=self._ctx_table("v1")):
c1 = course_context_service.get_course_context("o1")
assert c1["course_summary"]["summary_text"] == "v1"

# DB now returns v2, but the cache still serves v1 until cleared.
with patch("services.course_context_service.table", side_effect=self._ctx_table("v2")):
cached = course_context_service.get_course_context("o1")
assert cached["course_summary"]["summary_text"] == "v1"
course_context_service.clear_course_context_cache()
fresh = course_context_service.get_course_context("o1")
assert fresh["course_summary"]["summary_text"] == "v2"

def test_update_course_context_invalidates(self):
# Prime the cache with v1.
with patch("services.course_context_service.table", side_effect=self._ctx_table("v1")):
course_context_service.get_course_context("o1")

# update_course_context with no enrollments → purge path → cache_clear.
def _no_enrollments(name):
m = MagicMock()
m.select.return_value = []
return m
with patch("services.course_context_service.table", side_effect=_no_enrollments):
course_context_service.update_course_context("o1")

# Cache was invalidated, so the next read re-fetches (now v2).
with patch("services.course_context_service.table", side_effect=self._ctx_table("v2")):
after = course_context_service.get_course_context("o1")
assert after["course_summary"]["summary_text"] == "v2"

def test_returns_defensive_copy(self):
with patch("services.course_context_service.table", side_effect=self._ctx_table("v1")):
a = course_context_service.get_course_context("o1")
a["course_summary"]["summary_text"] = "MUTATED"
b = course_context_service.get_course_context("o1")
assert b["course_summary"]["summary_text"] == "v1" # cache intact
48 changes: 48 additions & 0 deletions specs/98-lru-cache.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
# Spec: functools.lru_cache for hot deterministic reads (#98)

## Scope

Milestone #2 (perf). Per-process (per-worker) caching of hot, deterministic reads with a clear
invalidation story. Composes with the HTTP cache (#99) and a future cross-worker Redis layer (#97).

## Requirements

### R1 — Cache immutable academics resolvers (no invalidation needed)
- `services/academics.offering_course_id(offering_id)` — `@lru_cache`. An offering's `course_id` is set
at creation and never changes → deterministic immutable mapping; returns an immutable `str`/`None`.
- `services/academics.term_for_offering(offering_id)` — `@lru_cache` on the private body; the public
function returns a `copy.deepcopy` (mutable dict) so callers can't corrupt the cache. Offering→term is
immutable and terms are seeded reference data.
- `services/academics.clear_academics_caches()` clears both (for tests).

### R2 — Cache course-context with an invalidation hook
- `services/course_context_service.get_course_context(offering_id)` — `@lru_cache` on the private body;
the public function returns a `copy.deepcopy`.
- `clear_course_context_cache()` is called from `update_course_context` on **every** write path (the
no-enrollment purge and the final upsert). `update_course_context` is the choke point that
`apply_graph_update` and the doc/grade post-rolls funnel through, so a graph/doc/grade change that
moves the aggregates always drops the stale cached read.

### R3 — Test isolation
- An autouse `_clear_lru_caches` fixture in `tests/conftest.py` clears all these caches around every test
so one test's mocked DB state can't leak into another via a cached read.

### R4 — Docs
- CLAUDE.md Conventions: `lru_cache` is reserved for deterministic per-process reads — immutable mappings
(no invalidation) or reads with a matching `clear_*_cache()` every mutator calls; hashable args only;
deep-copy mutable returns.

## Acceptance
1. 3+ functions cached, each provably safe (immutable, or hooked + deep-copied).
2. `clear_course_context_cache()` invoked from `update_course_context`'s write paths.
3. CLAUDE.md documents the convention.
4. Tests: cache hit avoids a 2nd DB read; distinct keys not conflated; deep-copy immunity; mutate via
`update_course_context` → next read returns fresh.
5. `pytest tests/ -q` shows no new failures vs `main` (proves the conftest fixture stops cache leakage);
`ruff` clean.

## Deliberately NOT cached (documented risk)
- Graph reads (`get_graph`) — large mutable structures with hot, per-turn invalidation; deep-copy cost +
invalidation surface outweigh the win here. Revisit with the Redis layer (#97).
- Token decode / `require_self` — security-sensitive and needs a TTL bounded to token lifetime; out of
scope for a plain `lru_cache`.
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -75,6 +75,7 @@ ruff format . # formatter — available, not yet CI-gated (see
- Display names are resolved via `services/profiles.py` (`get_display_name`/`get_display_names`), which decrypts off `user_profiles` — don't read name columns off `users`.
- All current LLM calls route through `services/gemini_service.py` (`call_gemini`, `call_gemini_json`, `call_gemini_multiturn`). New LLM-driven code should be written as Pydantic AI agents in `backend/agents/` rather than extending `gemini_service.py`.
- Knowledge-graph mutations go through `services/graph_service.py::apply_graph_update` — routes never write `graph_nodes`/`graph_edges` directly.
- `functools.lru_cache` is reserved for **deterministic, per-process reads** (#98) — either immutable mappings that never need invalidation (e.g. `academics.offering_course_id`) or reads with a matching `clear_*_cache()` hook that every mutator calls (e.g. `course_context_service.get_course_context` is cleared by `update_course_context`). Cache only hashable-arg functions; return a deep copy if the cached value is mutable; never cache without a clear invalidation story. The autouse `_clear_lru_caches` fixture in `tests/conftest.py` resets these between tests.
- Backend tests live in `backend/tests/` and run via `pytest`; shared fixtures (mock Supabase, mock Gemini) are in `tests/conftest.py`.
- Routers are mounted in `main.py` with `/api/<name>` prefixes; new routes follow that pattern.

Expand Down
31 changes: 28 additions & 3 deletions backend/services/academics.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,8 +15,10 @@
"""
from __future__ import annotations

import copy
import uuid
from datetime import date
from functools import lru_cache

from db.connection import table

Expand DownExpand Up@@ -93,8 +95,13 @@ def resolve_offering(
return any_off[0]["id"] if any_off else None


@lru_cache(maxsize=4096)
def offering_course_id(offering_id: str) -> str | None:
"""The abstract course id an offering belongs to (offering → graph bridge)."""
"""The abstract course id an offering belongs to (offering → graph bridge).

Cached per-process (#98): an offering's ``course_id`` is set at creation and
never changes, so this is a deterministic immutable mapping — no invalidation
hook needed. Returns an immutable ``str``/``None`` (safe to share)."""
if not offering_id:
return None
rows = table("course_offerings").select(
Expand All@@ -121,8 +128,8 @@ def user_offering_ids_for_course(user_id: str, course_id: str) -> list[str]:
return [e["offering_id"] for e in enr if e.get("offering_id") in off_ids]


def term_for_offering(offering_id: str) -> dict | None:
"""The term row for an offering (for semester labels)."""
@lru_cache(maxsize=4096)
def _term_for_offering_cached(offering_id: str) -> dict | None:
if not offering_id:
return None
rows = table("course_offerings").select(
Expand All@@ -137,6 +144,24 @@ def term_for_offering(offering_id: str) -> dict | None:
return terms[0] if terms else None


def term_for_offering(offering_id: str) -> dict | None:
"""The term row for an offering (for semester labels).

Cached per-process (#98): the offering→term mapping is immutable and terms
are seeded reference data that don't change at runtime. Returns a deep copy
so callers can't mutate the shared cached row."""
cached = _term_for_offering_cached(offering_id)
return copy.deepcopy(cached) if cached is not None else None


def clear_academics_caches() -> None:
"""Clear the per-process academics caches. Called from test setup (so mocked
DB state doesn't leak across tests); rarely needed at runtime since the
cached mappings are immutable."""
offering_course_id.cache_clear()
_term_for_offering_cached.cache_clear()


def user_enrollment_ids(user_id: str) -> list[dict]:
"""The user's enrollments as ``{id, offering_id}`` rows (read + scoping helper)."""
if not user_id:
Expand Down
36 changes: 28 additions & 8 deletions backend/services/course_context_service.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,9 +11,11 @@
- offering_summary: class-wide summary with Gemini-generated text (per offering)
"""

import copy
import json
import hashlib
from datetime import datetime, timezone
from functools import lru_cache

from db.connection import table
from services.gemini_service import call_gemini
Expand DownExpand Up@@ -65,14 +67,8 @@ def _generate_summary_with_gemini(
)


def get_course_context(offering_id: str) -> dict:
"""
Return the cached class context for an offering: summary + concept stats.
Offering-scoped (one class instance in one term). Returns {} if not found.
"""
if not offering_id:
return {}

@lru_cache(maxsize=512)
def _get_course_context_cached(offering_id: str) -> dict:
try:
# Get the offering summary
summary_rows = table("offering_summary").select(
Expand DownExpand Up@@ -106,6 +102,26 @@ def get_course_context(offering_id: str) -> dict:
return {}


def get_course_context(offering_id: str) -> dict:
"""
Return the cached class context for an offering: summary + concept stats.
Offering-scoped (one class instance in one term). Returns {} if not found.

Cached per-process (#98) and invalidated by ``update_course_context`` (which
runs after graph updates). Returns a deep copy so callers can't mutate the
shared cached value.
"""
if not offering_id:
return {}
return copy.deepcopy(_get_course_context_cached(offering_id))


def clear_course_context_cache() -> None:
"""Drop the per-process course-context cache. Called whenever the underlying
aggregates change (``update_course_context``) and from test setup."""
_get_course_context_cached.cache_clear()


def update_course_context(offering_id: str) -> None:
"""
Aggregate mastery + quiz data for all students enrolled in an **offering**
Expand All@@ -127,6 +143,7 @@ def update_course_context(offering_id: str) -> None:
# No students enrolled — purge any stale aggregates
table("offering_concept_stats").delete({"offering_id": f"eq.{offering_id}"})
table("offering_summary").delete({"offering_id": f"eq.{offering_id}"})
clear_course_context_cache() # #98: aggregates changed → drop cached read
return

user_ids = [r["user_id"] for r in enrollment_rows]
Expand DownExpand Up@@ -335,3 +352,6 @@ def _parse_quiz_context_to_arrays(ctx_rows: list) -> tuple[list, list, list]:
},
on_conflict="offering_id",
)

# #98: the aggregates this offering's context reads from just changed.
clear_course_context_cache()
12 changes: 12 additions & 0 deletions backend/tests/conftest.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,18 @@ def pytest_configure(config):
)


@pytest.fixture(autouse=True)
def _clear_lru_caches():
"""#98: reset the per-process lru_caches around every test so one test's
mocked DB state can't leak into another via a cached read."""
from services import academics, course_context_service
academics.clear_academics_caches()
course_context_service.clear_course_context_cache()
yield
academics.clear_academics_caches()
course_context_service.clear_course_context_cache()


@pytest.fixture(autouse=True)
def _hermetic_supabase_client(request, monkeypatch):
"""Hermetic safety net (#210): no test may make a real Supabase call.
Expand Down
112 changes: 112 additions & 0 deletions backend/tests/test_lru_cache.py
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
"""Tests for the per-process lru_caches (#98): academics resolvers and
course-context, including the invalidation hook.

The autouse `_clear_lru_caches` fixture in conftest clears these before each
test, so each starts with an empty cache."""
from unittest.mock import MagicMock, patch

from services import academics, course_context_service


class TestOfferingCourseIdCache:
def test_second_call_served_from_cache(self):
calls = {"n": 0}

def _table(name):
m = MagicMock()
def _select(*a, **k):
calls["n"] += 1
return [{"course_id": "c1"}]
m.select.side_effect = _select
return m

with patch("services.academics.table", side_effect=_table):
assert academics.offering_course_id("off1") == "c1"
assert academics.offering_course_id("off1") == "c1"
assert calls["n"] == 1 # only the first call hit the DB

def test_distinct_offerings_not_conflated(self):
def _table(name):
m = MagicMock()
def _select(cols, filters=None, **k):
off = filters["id"].split(".")[-1]
return [{"course_id": f"c-{off}"}]
m.select.side_effect = _select
return m

with patch("services.academics.table", side_effect=_table):
assert academics.offering_course_id("A") == "c-A"
assert academics.offering_course_id("B") == "c-B"


class TestTermForOfferingCache:
def _table(self, name):
m = MagicMock()
if name == "course_offerings":
m.select.return_value = [{"term_id": "t1"}]
elif name == "terms":
m.select.return_value = [{"id": "t1", "name": "Fall 2026"}]
else:
m.select.return_value = []
return m

def test_returns_defensive_copy(self):
with patch("services.academics.table", side_effect=self._table):
t1 = academics.term_for_offering("off1")
t1["name"] = "MUTATED" # caller mutates the returned dict
t2 = academics.term_for_offering("off1")
assert t2["name"] == "Fall 2026" # cache not corrupted


class TestCourseContextCacheInvalidation:
def _ctx_table(self, summary_text):
def _table(name):
m = MagicMock()
if name == "offering_summary":
m.select.return_value = [{
"offering_id": "o1", "student_count": 1,
"avg_class_mastery": 0.5, "summary_text": summary_text,
"updated_at": "t",
}]
else:
m.select.return_value = []
return m
return _table

def test_cached_until_explicitly_cleared(self):
with patch("services.course_context_service.table", side_effect=self._ctx_table("v1")):
c1 = course_context_service.get_course_context("o1")
assert c1["course_summary"]["summary_text"] == "v1"

# DB now returns v2, but the cache still serves v1 until cleared.
with patch("services.course_context_service.table", side_effect=self._ctx_table("v2")):
cached = course_context_service.get_course_context("o1")
assert cached["course_summary"]["summary_text"] == "v1"
course_context_service.clear_course_context_cache()
fresh = course_context_service.get_course_context("o1")
assert fresh["course_summary"]["summary_text"] == "v2"

def test_update_course_context_invalidates(self):
# Prime the cache with v1.
with patch("services.course_context_service.table", side_effect=self._ctx_table("v1")):
course_context_service.get_course_context("o1")

# update_course_context with no enrollments → purge path → cache_clear.
def _no_enrollments(name):
m = MagicMock()
m.select.return_value = []
return m
with patch("services.course_context_service.table", side_effect=_no_enrollments):
course_context_service.update_course_context("o1")

# Cache was invalidated, so the next read re-fetches (now v2).
with patch("services.course_context_service.table", side_effect=self._ctx_table("v2")):
after = course_context_service.get_course_context("o1")
assert after["course_summary"]["summary_text"] == "v2"

def test_returns_defensive_copy(self):
with patch("services.course_context_service.table", side_effect=self._ctx_table("v1")):
a = course_context_service.get_course_context("o1")
a["course_summary"]["summary_text"] = "MUTATED"
b = course_context_service.get_course_context("o1")
assert b["course_summary"]["summary_text"] == "v1" # cache intact
48 changes: 48 additions & 0 deletions specs/98-lru-cache.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
# Spec: functools.lru_cache for hot deterministic reads (#98)

## Scope

Milestone #2 (perf). Per-process (per-worker) caching of hot, deterministic reads with a clear
invalidation story. Composes with the HTTP cache (#99) and a future cross-worker Redis layer (#97).

## Requirements

### R1 — Cache immutable academics resolvers (no invalidation needed)
- `services/academics.offering_course_id(offering_id)` — `@lru_cache`. An offering's `course_id` is set
at creation and never changes → deterministic immutable mapping; returns an immutable `str`/`None`.
- `services/academics.term_for_offering(offering_id)` — `@lru_cache` on the private body; the public
function returns a `copy.deepcopy` (mutable dict) so callers can't corrupt the cache. Offering→term is
immutable and terms are seeded reference data.
- `services/academics.clear_academics_caches()` clears both (for tests).

### R2 — Cache course-context with an invalidation hook
- `services/course_context_service.get_course_context(offering_id)` — `@lru_cache` on the private body;
the public function returns a `copy.deepcopy`.
- `clear_course_context_cache()` is called from `update_course_context` on **every** write path (the
no-enrollment purge and the final upsert). `update_course_context` is the choke point that
`apply_graph_update` and the doc/grade post-rolls funnel through, so a graph/doc/grade change that
moves the aggregates always drops the stale cached read.

### R3 — Test isolation
- An autouse `_clear_lru_caches` fixture in `tests/conftest.py` clears all these caches around every test
so one test's mocked DB state can't leak into another via a cached read.

### R4 — Docs
- CLAUDE.md Conventions: `lru_cache` is reserved for deterministic per-process reads — immutable mappings
(no invalidation) or reads with a matching `clear_*_cache()` every mutator calls; hashable args only;
deep-copy mutable returns.

## Acceptance
1. 3+ functions cached, each provably safe (immutable, or hooked + deep-copied).
2. `clear_course_context_cache()` invoked from `update_course_context`'s write paths.
3. CLAUDE.md documents the convention.
4. Tests: cache hit avoids a 2nd DB read; distinct keys not conflated; deep-copy immunity; mutate via
`update_course_context` → next read returns fresh.
5. `pytest tests/ -q` shows no new failures vs `main` (proves the conftest fixture stops cache leakage);
`ruff` clean.

## Deliberately NOT cached (documented risk)
- Graph reads (`get_graph`) — large mutable structures with hot, per-turn invalidation; deep-copy cost +
invalidation surface outweigh the win here. Revisit with the Redis layer (#97).
- Token decode / `require_self` — security-sensitive and needs a TTL bounded to token lifetime; out of
scope for a plain `lru_cache`.
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -75,6 +75,7 @@ ruff format . # formatter — available, not yet CI-gated (see
- Display names are resolved via `services/profiles.py` (`get_display_name`/`get_display_names`), which decrypts off `user_profiles` — don't read name columns off `users`.
- All current LLM calls route through `services/gemini_service.py` (`call_gemini`, `call_gemini_json`, `call_gemini_multiturn`). New LLM-driven code should be written as Pydantic AI agents in `backend/agents/` rather than extending `gemini_service.py`.
- Knowledge-graph mutations go through `services/graph_service.py::apply_graph_update` — routes never write `graph_nodes`/`graph_edges` directly.
- `functools.lru_cache` is reserved for **deterministic, per-process reads** (#98) — either immutable mappings that never need invalidation (e.g. `academics.offering_course_id`) or reads with a matching `clear_*_cache()` hook that every mutator calls (e.g. `course_context_service.get_course_context` is cleared by `update_course_context`). Cache only hashable-arg functions; return a deep copy if the cached value is mutable; never cache without a clear invalidation story. The autouse `_clear_lru_caches` fixture in `tests/conftest.py` resets these between tests.
- Backend tests live in `backend/tests/` and run via `pytest`; shared fixtures (mock Supabase, mock Gemini) are in `tests/conftest.py`.
- Routers are mounted in `main.py` with `/api/<name>` prefixes; new routes follow that pattern.

Expand Down
31 changes: 28 additions & 3 deletions backend/services/academics.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,8 +15,10 @@
"""
from __future__ import annotations

import copy
import uuid
from datetime import date
from functools import lru_cache

from db.connection import table

Expand DownExpand Up@@ -93,8 +95,13 @@ def resolve_offering(
return any_off[0]["id"] if any_off else None


@lru_cache(maxsize=4096)
def offering_course_id(offering_id: str) -> str | None:
"""The abstract course id an offering belongs to (offering → graph bridge)."""
"""The abstract course id an offering belongs to (offering → graph bridge).

Cached per-process (#98): an offering's ``course_id`` is set at creation and
never changes, so this is a deterministic immutable mapping — no invalidation
hook needed. Returns an immutable ``str``/``None`` (safe to share)."""
if not offering_id:
return None
rows = table("course_offerings").select(
Expand All@@ -121,8 +128,8 @@ def user_offering_ids_for_course(user_id: str, course_id: str) -> list[str]:
return [e["offering_id"] for e in enr if e.get("offering_id") in off_ids]


def term_for_offering(offering_id: str) -> dict | None:
"""The term row for an offering (for semester labels)."""
@lru_cache(maxsize=4096)
def _term_for_offering_cached(offering_id: str) -> dict | None:
if not offering_id:
return None
rows = table("course_offerings").select(
Expand All@@ -137,6 +144,24 @@ def term_for_offering(offering_id: str) -> dict | None:
return terms[0] if terms else None


def term_for_offering(offering_id: str) -> dict | None:
"""The term row for an offering (for semester labels).

Cached per-process (#98): the offering→term mapping is immutable and terms
are seeded reference data that don't change at runtime. Returns a deep copy
so callers can't mutate the shared cached row."""
cached = _term_for_offering_cached(offering_id)
return copy.deepcopy(cached) if cached is not None else None


def clear_academics_caches() -> None:
"""Clear the per-process academics caches. Called from test setup (so mocked
DB state doesn't leak across tests); rarely needed at runtime since the
cached mappings are immutable."""
offering_course_id.cache_clear()
_term_for_offering_cached.cache_clear()


def user_enrollment_ids(user_id: str) -> list[dict]:
"""The user's enrollments as ``{id, offering_id}`` rows (read + scoping helper)."""
if not user_id:
Expand Down
36 changes: 28 additions & 8 deletions backend/services/course_context_service.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,9 +11,11 @@
- offering_summary: class-wide summary with Gemini-generated text (per offering)
"""

import copy
import json
import hashlib
from datetime import datetime, timezone
from functools import lru_cache

from db.connection import table
from services.gemini_service import call_gemini
Expand DownExpand Up@@ -65,14 +67,8 @@ def _generate_summary_with_gemini(
)


def get_course_context(offering_id: str) -> dict:
"""
Return the cached class context for an offering: summary + concept stats.
Offering-scoped (one class instance in one term). Returns {} if not found.
"""
if not offering_id:
return {}

@lru_cache(maxsize=512)
def _get_course_context_cached(offering_id: str) -> dict:
try:
# Get the offering summary
summary_rows = table("offering_summary").select(
Expand DownExpand Up@@ -106,6 +102,26 @@ def get_course_context(offering_id: str) -> dict:
return {}


def get_course_context(offering_id: str) -> dict:
"""
Return the cached class context for an offering: summary + concept stats.
Offering-scoped (one class instance in one term). Returns {} if not found.

Cached per-process (#98) and invalidated by ``update_course_context`` (which
runs after graph updates). Returns a deep copy so callers can't mutate the
shared cached value.
"""
if not offering_id:
return {}
return copy.deepcopy(_get_course_context_cached(offering_id))


def clear_course_context_cache() -> None:
"""Drop the per-process course-context cache. Called whenever the underlying
aggregates change (``update_course_context``) and from test setup."""
_get_course_context_cached.cache_clear()


def update_course_context(offering_id: str) -> None:
"""
Aggregate mastery + quiz data for all students enrolled in an **offering**
Expand All@@ -127,6 +143,7 @@ def update_course_context(offering_id: str) -> None:
# No students enrolled — purge any stale aggregates
table("offering_concept_stats").delete({"offering_id": f"eq.{offering_id}"})
table("offering_summary").delete({"offering_id": f"eq.{offering_id}"})
clear_course_context_cache() # #98: aggregates changed → drop cached read
return

user_ids = [r["user_id"] for r in enrollment_rows]
Expand DownExpand Up@@ -335,3 +352,6 @@ def _parse_quiz_context_to_arrays(ctx_rows: list) -> tuple[list, list, list]:
},
on_conflict="offering_id",
)

# #98: the aggregates this offering's context reads from just changed.
clear_course_context_cache()
12 changes: 12 additions & 0 deletions backend/tests/conftest.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,18 @@ def pytest_configure(config):
)


@pytest.fixture(autouse=True)
def _clear_lru_caches():
"""#98: reset the per-process lru_caches around every test so one test's
mocked DB state can't leak into another via a cached read."""
from services import academics, course_context_service
academics.clear_academics_caches()
course_context_service.clear_course_context_cache()
yield
academics.clear_academics_caches()
course_context_service.clear_course_context_cache()


@pytest.fixture(autouse=True)
def _hermetic_supabase_client(request, monkeypatch):
"""Hermetic safety net (#210): no test may make a real Supabase call.
Expand Down
112 changes: 112 additions & 0 deletions backend/tests/test_lru_cache.py
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
"""Tests for the per-process lru_caches (#98): academics resolvers and
course-context, including the invalidation hook.

The autouse `_clear_lru_caches` fixture in conftest clears these before each
test, so each starts with an empty cache."""
from unittest.mock import MagicMock, patch

from services import academics, course_context_service


class TestOfferingCourseIdCache:
def test_second_call_served_from_cache(self):
calls = {"n": 0}

def _table(name):
m = MagicMock()
def _select(*a, **k):
calls["n"] += 1
return [{"course_id": "c1"}]
m.select.side_effect = _select
return m

with patch("services.academics.table", side_effect=_table):
assert academics.offering_course_id("off1") == "c1"
assert academics.offering_course_id("off1") == "c1"
assert calls["n"] == 1 # only the first call hit the DB

def test_distinct_offerings_not_conflated(self):
def _table(name):
m = MagicMock()
def _select(cols, filters=None, **k):
off = filters["id"].split(".")[-1]
return [{"course_id": f"c-{off}"}]
m.select.side_effect = _select
return m

with patch("services.academics.table", side_effect=_table):
assert academics.offering_course_id("A") == "c-A"
assert academics.offering_course_id("B") == "c-B"


class TestTermForOfferingCache:
def _table(self, name):
m = MagicMock()
if name == "course_offerings":
m.select.return_value = [{"term_id": "t1"}]
elif name == "terms":
m.select.return_value = [{"id": "t1", "name": "Fall 2026"}]
else:
m.select.return_value = []
return m

def test_returns_defensive_copy(self):
with patch("services.academics.table", side_effect=self._table):
t1 = academics.term_for_offering("off1")
t1["name"] = "MUTATED" # caller mutates the returned dict
t2 = academics.term_for_offering("off1")
assert t2["name"] == "Fall 2026" # cache not corrupted


class TestCourseContextCacheInvalidation:
def _ctx_table(self, summary_text):
def _table(name):
m = MagicMock()
if name == "offering_summary":
m.select.return_value = [{
"offering_id": "o1", "student_count": 1,
"avg_class_mastery": 0.5, "summary_text": summary_text,
"updated_at": "t",
}]
else:
m.select.return_value = []
return m
return _table

def test_cached_until_explicitly_cleared(self):
with patch("services.course_context_service.table", side_effect=self._ctx_table("v1")):
c1 = course_context_service.get_course_context("o1")
assert c1["course_summary"]["summary_text"] == "v1"

# DB now returns v2, but the cache still serves v1 until cleared.
with patch("services.course_context_service.table", side_effect=self._ctx_table("v2")):
cached = course_context_service.get_course_context("o1")
assert cached["course_summary"]["summary_text"] == "v1"
course_context_service.clear_course_context_cache()
fresh = course_context_service.get_course_context("o1")
assert fresh["course_summary"]["summary_text"] == "v2"

def test_update_course_context_invalidates(self):
# Prime the cache with v1.
with patch("services.course_context_service.table", side_effect=self._ctx_table("v1")):
course_context_service.get_course_context("o1")

# update_course_context with no enrollments → purge path → cache_clear.
def _no_enrollments(name):
m = MagicMock()
m.select.return_value = []
return m
with patch("services.course_context_service.table", side_effect=_no_enrollments):
course_context_service.update_course_context("o1")

# Cache was invalidated, so the next read re-fetches (now v2).
with patch("services.course_context_service.table", side_effect=self._ctx_table("v2")):
after = course_context_service.get_course_context("o1")
assert after["course_summary"]["summary_text"] == "v2"

def test_returns_defensive_copy(self):
with patch("services.course_context_service.table", side_effect=self._ctx_table("v1")):
a = course_context_service.get_course_context("o1")
a["course_summary"]["summary_text"] = "MUTATED"
b = course_context_service.get_course_context("o1")
assert b["course_summary"]["summary_text"] == "v1" # cache intact
48 changes: 48 additions & 0 deletions specs/98-lru-cache.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
# Spec: functools.lru_cache for hot deterministic reads (#98)

## Scope

Milestone #2 (perf). Per-process (per-worker) caching of hot, deterministic reads with a clear
invalidation story. Composes with the HTTP cache (#99) and a future cross-worker Redis layer (#97).

## Requirements

### R1 — Cache immutable academics resolvers (no invalidation needed)
- `services/academics.offering_course_id(offering_id)` — `@lru_cache`. An offering's `course_id` is set
at creation and never changes → deterministic immutable mapping; returns an immutable `str`/`None`.
- `services/academics.term_for_offering(offering_id)` — `@lru_cache` on the private body; the public
function returns a `copy.deepcopy` (mutable dict) so callers can't corrupt the cache. Offering→term is
immutable and terms are seeded reference data.
- `services/academics.clear_academics_caches()` clears both (for tests).

### R2 — Cache course-context with an invalidation hook
- `services/course_context_service.get_course_context(offering_id)` — `@lru_cache` on the private body;
the public function returns a `copy.deepcopy`.
- `clear_course_context_cache()` is called from `update_course_context` on **every** write path (the
no-enrollment purge and the final upsert). `update_course_context` is the choke point that
`apply_graph_update` and the doc/grade post-rolls funnel through, so a graph/doc/grade change that
moves the aggregates always drops the stale cached read.

### R3 — Test isolation
- An autouse `_clear_lru_caches` fixture in `tests/conftest.py` clears all these caches around every test
so one test's mocked DB state can't leak into another via a cached read.

### R4 — Docs
- CLAUDE.md Conventions: `lru_cache` is reserved for deterministic per-process reads — immutable mappings
(no invalidation) or reads with a matching `clear_*_cache()` every mutator calls; hashable args only;
deep-copy mutable returns.

## Acceptance
1. 3+ functions cached, each provably safe (immutable, or hooked + deep-copied).
2. `clear_course_context_cache()` invoked from `update_course_context`'s write paths.
3. CLAUDE.md documents the convention.
4. Tests: cache hit avoids a 2nd DB read; distinct keys not conflated; deep-copy immunity; mutate via
`update_course_context` → next read returns fresh.
5. `pytest tests/ -q` shows no new failures vs `main` (proves the conftest fixture stops cache leakage);
`ruff` clean.

## Deliberately NOT cached (documented risk)
- Graph reads (`get_graph`) — large mutable structures with hot, per-turn invalidation; deep-copy cost +
invalidation surface outweigh the win here. Revisit with the Redis layer (#97).
- Token decode / `require_self` — security-sensitive and needs a TTL bounded to token lifetime; out of
scope for a plain `lru_cache`.
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -75,6 +75,7 @@ ruff format . # formatter — available, not yet CI-gated (see
- Display names are resolved via `services/profiles.py` (`get_display_name`/`get_display_names`), which decrypts off `user_profiles` — don't read name columns off `users`.
- All current LLM calls route through `services/gemini_service.py` (`call_gemini`, `call_gemini_json`, `call_gemini_multiturn`). New LLM-driven code should be written as Pydantic AI agents in `backend/agents/` rather than extending `gemini_service.py`.
- Knowledge-graph mutations go through `services/graph_service.py::apply_graph_update` — routes never write `graph_nodes`/`graph_edges` directly.
- `functools.lru_cache` is reserved for **deterministic, per-process reads** (#98) — either immutable mappings that never need invalidation (e.g. `academics.offering_course_id`) or reads with a matching `clear_*_cache()` hook that every mutator calls (e.g. `course_context_service.get_course_context` is cleared by `update_course_context`). Cache only hashable-arg functions; return a deep copy if the cached value is mutable; never cache without a clear invalidation story. The autouse `_clear_lru_caches` fixture in `tests/conftest.py` resets these between tests.
- Backend tests live in `backend/tests/` and run via `pytest`; shared fixtures (mock Supabase, mock Gemini) are in `tests/conftest.py`.
- Routers are mounted in `main.py` with `/api/<name>` prefixes; new routes follow that pattern.

Expand Down
31 changes: 28 additions & 3 deletions backend/services/academics.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,8 +15,10 @@
"""
from __future__ import annotations

import copy
import uuid
from datetime import date
from functools import lru_cache

from db.connection import table

Expand DownExpand Up@@ -93,8 +95,13 @@ def resolve_offering(
return any_off[0]["id"] if any_off else None


@lru_cache(maxsize=4096)
def offering_course_id(offering_id: str) -> str | None:
"""The abstract course id an offering belongs to (offering → graph bridge)."""
"""The abstract course id an offering belongs to (offering → graph bridge).

Cached per-process (#98): an offering's ``course_id`` is set at creation and
never changes, so this is a deterministic immutable mapping — no invalidation
hook needed. Returns an immutable ``str``/``None`` (safe to share)."""
if not offering_id:
return None
rows = table("course_offerings").select(
Expand All@@ -121,8 +128,8 @@ def user_offering_ids_for_course(user_id: str, course_id: str) -> list[str]:
return [e["offering_id"] for e in enr if e.get("offering_id") in off_ids]


def term_for_offering(offering_id: str) -> dict | None:
"""The term row for an offering (for semester labels)."""
@lru_cache(maxsize=4096)
def _term_for_offering_cached(offering_id: str) -> dict | None:
if not offering_id:
return None
rows = table("course_offerings").select(
Expand All@@ -137,6 +144,24 @@ def term_for_offering(offering_id: str) -> dict | None:
return terms[0] if terms else None


def term_for_offering(offering_id: str) -> dict | None:
"""The term row for an offering (for semester labels).

Cached per-process (#98): the offering→term mapping is immutable and terms
are seeded reference data that don't change at runtime. Returns a deep copy
so callers can't mutate the shared cached row."""
cached = _term_for_offering_cached(offering_id)
return copy.deepcopy(cached) if cached is not None else None


def clear_academics_caches() -> None:
"""Clear the per-process academics caches. Called from test setup (so mocked
DB state doesn't leak across tests); rarely needed at runtime since the
cached mappings are immutable."""
offering_course_id.cache_clear()
_term_for_offering_cached.cache_clear()


def user_enrollment_ids(user_id: str) -> list[dict]:
"""The user's enrollments as ``{id, offering_id}`` rows (read + scoping helper)."""
if not user_id:
Expand Down
36 changes: 28 additions & 8 deletions backend/services/course_context_service.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,9 +11,11 @@
- offering_summary: class-wide summary with Gemini-generated text (per offering)
"""

import copy
import json
import hashlib
from datetime import datetime, timezone
from functools import lru_cache

from db.connection import table
from services.gemini_service import call_gemini
Expand DownExpand Up@@ -65,14 +67,8 @@ def _generate_summary_with_gemini(
)


def get_course_context(offering_id: str) -> dict:
"""
Return the cached class context for an offering: summary + concept stats.
Offering-scoped (one class instance in one term). Returns {} if not found.
"""
if not offering_id:
return {}

@lru_cache(maxsize=512)
def _get_course_context_cached(offering_id: str) -> dict:
try:
# Get the offering summary
summary_rows = table("offering_summary").select(
Expand DownExpand Up@@ -106,6 +102,26 @@ def get_course_context(offering_id: str) -> dict:
return {}


def get_course_context(offering_id: str) -> dict:
"""
Return the cached class context for an offering: summary + concept stats.
Offering-scoped (one class instance in one term). Returns {} if not found.

Cached per-process (#98) and invalidated by ``update_course_context`` (which
runs after graph updates). Returns a deep copy so callers can't mutate the
shared cached value.
"""
if not offering_id:
return {}
return copy.deepcopy(_get_course_context_cached(offering_id))


def clear_course_context_cache() -> None:
"""Drop the per-process course-context cache. Called whenever the underlying
aggregates change (``update_course_context``) and from test setup."""
_get_course_context_cached.cache_clear()


def update_course_context(offering_id: str) -> None:
"""
Aggregate mastery + quiz data for all students enrolled in an **offering**
Expand All@@ -127,6 +143,7 @@ def update_course_context(offering_id: str) -> None:
# No students enrolled — purge any stale aggregates
table("offering_concept_stats").delete({"offering_id": f"eq.{offering_id}"})
table("offering_summary").delete({"offering_id": f"eq.{offering_id}"})
clear_course_context_cache() # #98: aggregates changed → drop cached read
return

user_ids = [r["user_id"] for r in enrollment_rows]
Expand DownExpand Up@@ -335,3 +352,6 @@ def _parse_quiz_context_to_arrays(ctx_rows: list) -> tuple[list, list, list]:
},
on_conflict="offering_id",
)

# #98: the aggregates this offering's context reads from just changed.
clear_course_context_cache()
12 changes: 12 additions & 0 deletions backend/tests/conftest.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,18 @@ def pytest_configure(config):
)


@pytest.fixture(autouse=True)
def _clear_lru_caches():
"""#98: reset the per-process lru_caches around every test so one test's
mocked DB state can't leak into another via a cached read."""
from services import academics, course_context_service
academics.clear_academics_caches()
course_context_service.clear_course_context_cache()
yield
academics.clear_academics_caches()
course_context_service.clear_course_context_cache()


@pytest.fixture(autouse=True)
def _hermetic_supabase_client(request, monkeypatch):
"""Hermetic safety net (#210): no test may make a real Supabase call.
Expand Down
112 changes: 112 additions & 0 deletions backend/tests/test_lru_cache.py
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
"""Tests for the per-process lru_caches (#98): academics resolvers and
course-context, including the invalidation hook.

The autouse `_clear_lru_caches` fixture in conftest clears these before each
test, so each starts with an empty cache."""
from unittest.mock import MagicMock, patch

from services import academics, course_context_service


class TestOfferingCourseIdCache:
def test_second_call_served_from_cache(self):
calls = {"n": 0}

def _table(name):
m = MagicMock()
def _select(*a, **k):
calls["n"] += 1
return [{"course_id": "c1"}]
m.select.side_effect = _select
return m

with patch("services.academics.table", side_effect=_table):
assert academics.offering_course_id("off1") == "c1"
assert academics.offering_course_id("off1") == "c1"
assert calls["n"] == 1 # only the first call hit the DB

def test_distinct_offerings_not_conflated(self):
def _table(name):
m = MagicMock()
def _select(cols, filters=None, **k):
off = filters["id"].split(".")[-1]
return [{"course_id": f"c-{off}"}]
m.select.side_effect = _select
return m

with patch("services.academics.table", side_effect=_table):
assert academics.offering_course_id("A") == "c-A"
assert academics.offering_course_id("B") == "c-B"


class TestTermForOfferingCache:
def _table(self, name):
m = MagicMock()
if name == "course_offerings":
m.select.return_value = [{"term_id": "t1"}]
elif name == "terms":
m.select.return_value = [{"id": "t1", "name": "Fall 2026"}]
else:
m.select.return_value = []
return m

def test_returns_defensive_copy(self):
with patch("services.academics.table", side_effect=self._table):
t1 = academics.term_for_offering("off1")
t1["name"] = "MUTATED" # caller mutates the returned dict
t2 = academics.term_for_offering("off1")
assert t2["name"] == "Fall 2026" # cache not corrupted


class TestCourseContextCacheInvalidation:
def _ctx_table(self, summary_text):
def _table(name):
m = MagicMock()
if name == "offering_summary":
m.select.return_value = [{
"offering_id": "o1", "student_count": 1,
"avg_class_mastery": 0.5, "summary_text": summary_text,
"updated_at": "t",
}]
else:
m.select.return_value = []
return m
return _table

def test_cached_until_explicitly_cleared(self):
with patch("services.course_context_service.table", side_effect=self._ctx_table("v1")):
c1 = course_context_service.get_course_context("o1")
assert c1["course_summary"]["summary_text"] == "v1"

# DB now returns v2, but the cache still serves v1 until cleared.
with patch("services.course_context_service.table", side_effect=self._ctx_table("v2")):
cached = course_context_service.get_course_context("o1")
assert cached["course_summary"]["summary_text"] == "v1"
course_context_service.clear_course_context_cache()
fresh = course_context_service.get_course_context("o1")
assert fresh["course_summary"]["summary_text"] == "v2"

def test_update_course_context_invalidates(self):
# Prime the cache with v1.
with patch("services.course_context_service.table", side_effect=self._ctx_table("v1")):
course_context_service.get_course_context("o1")

# update_course_context with no enrollments → purge path → cache_clear.
def _no_enrollments(name):
m = MagicMock()
m.select.return_value = []
return m
with patch("services.course_context_service.table", side_effect=_no_enrollments):
course_context_service.update_course_context("o1")

# Cache was invalidated, so the next read re-fetches (now v2).
with patch("services.course_context_service.table", side_effect=self._ctx_table("v2")):
after = course_context_service.get_course_context("o1")
assert after["course_summary"]["summary_text"] == "v2"

def test_returns_defensive_copy(self):
with patch("services.course_context_service.table", side_effect=self._ctx_table("v1")):
a = course_context_service.get_course_context("o1")
a["course_summary"]["summary_text"] = "MUTATED"
b = course_context_service.get_course_context("o1")
assert b["course_summary"]["summary_text"] == "v1" # cache intact
48 changes: 48 additions & 0 deletions specs/98-lru-cache.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
# Spec: functools.lru_cache for hot deterministic reads (#98)

## Scope

Milestone #2 (perf). Per-process (per-worker) caching of hot, deterministic reads with a clear
invalidation story. Composes with the HTTP cache (#99) and a future cross-worker Redis layer (#97).

## Requirements

### R1 — Cache immutable academics resolvers (no invalidation needed)
- `services/academics.offering_course_id(offering_id)` — `@lru_cache`. An offering's `course_id` is set
at creation and never changes → deterministic immutable mapping; returns an immutable `str`/`None`.
- `services/academics.term_for_offering(offering_id)` — `@lru_cache` on the private body; the public
function returns a `copy.deepcopy` (mutable dict) so callers can't corrupt the cache. Offering→term is
immutable and terms are seeded reference data.
- `services/academics.clear_academics_caches()` clears both (for tests).

### R2 — Cache course-context with an invalidation hook
- `services/course_context_service.get_course_context(offering_id)` — `@lru_cache` on the private body;
the public function returns a `copy.deepcopy`.
- `clear_course_context_cache()` is called from `update_course_context` on **every** write path (the
no-enrollment purge and the final upsert). `update_course_context` is the choke point that
`apply_graph_update` and the doc/grade post-rolls funnel through, so a graph/doc/grade change that
moves the aggregates always drops the stale cached read.

### R3 — Test isolation
- An autouse `_clear_lru_caches` fixture in `tests/conftest.py` clears all these caches around every test
so one test's mocked DB state can't leak into another via a cached read.

### R4 — Docs
- CLAUDE.md Conventions: `lru_cache` is reserved for deterministic per-process reads — immutable mappings
(no invalidation) or reads with a matching `clear_*_cache()` every mutator calls; hashable args only;
deep-copy mutable returns.

## Acceptance
1. 3+ functions cached, each provably safe (immutable, or hooked + deep-copied).
2. `clear_course_context_cache()` invoked from `update_course_context`'s write paths.
3. CLAUDE.md documents the convention.
4. Tests: cache hit avoids a 2nd DB read; distinct keys not conflated; deep-copy immunity; mutate via
`update_course_context` → next read returns fresh.
5. `pytest tests/ -q` shows no new failures vs `main` (proves the conftest fixture stops cache leakage);
`ruff` clean.

## Deliberately NOT cached (documented risk)
- Graph reads (`get_graph`) — large mutable structures with hot, per-turn invalidation; deep-copy cost +
invalidation surface outweigh the win here. Revisit with the Redis layer (#97).
- Token decode / `require_self` — security-sensitive and needs a TTL bounded to token lifetime; out of
scope for a plain `lru_cache`.
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -75,6 +75,7 @@ ruff format . # formatter — available, not yet CI-gated (see
- Display names are resolved via `services/profiles.py` (`get_display_name`/`get_display_names`), which decrypts off `user_profiles` — don't read name columns off `users`.
- All current LLM calls route through `services/gemini_service.py` (`call_gemini`, `call_gemini_json`, `call_gemini_multiturn`). New LLM-driven code should be written as Pydantic AI agents in `backend/agents/` rather than extending `gemini_service.py`.
- Knowledge-graph mutations go through `services/graph_service.py::apply_graph_update` — routes never write `graph_nodes`/`graph_edges` directly.
- `functools.lru_cache` is reserved for **deterministic, per-process reads** (#98) — either immutable mappings that never need invalidation (e.g. `academics.offering_course_id`) or reads with a matching `clear_*_cache()` hook that every mutator calls (e.g. `course_context_service.get_course_context` is cleared by `update_course_context`). Cache only hashable-arg functions; return a deep copy if the cached value is mutable; never cache without a clear invalidation story. The autouse `_clear_lru_caches` fixture in `tests/conftest.py` resets these between tests.
- Backend tests live in `backend/tests/` and run via `pytest`; shared fixtures (mock Supabase, mock Gemini) are in `tests/conftest.py`.
- Routers are mounted in `main.py` with `/api/<name>` prefixes; new routes follow that pattern.

Expand Down
31 changes: 28 additions & 3 deletions backend/services/academics.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,8 +15,10 @@
"""
from __future__ import annotations

import copy
import uuid
from datetime import date
from functools import lru_cache

from db.connection import table

Expand DownExpand Up@@ -93,8 +95,13 @@ def resolve_offering(
return any_off[0]["id"] if any_off else None


@lru_cache(maxsize=4096)
def offering_course_id(offering_id: str) -> str | None:
"""The abstract course id an offering belongs to (offering → graph bridge)."""
"""The abstract course id an offering belongs to (offering → graph bridge).

Cached per-process (#98): an offering's ``course_id`` is set at creation and
never changes, so this is a deterministic immutable mapping — no invalidation
hook needed. Returns an immutable ``str``/``None`` (safe to share)."""
if not offering_id:
return None
rows = table("course_offerings").select(
Expand All@@ -121,8 +128,8 @@ def user_offering_ids_for_course(user_id: str, course_id: str) -> list[str]:
return [e["offering_id"] for e in enr if e.get("offering_id") in off_ids]


def term_for_offering(offering_id: str) -> dict | None:
"""The term row for an offering (for semester labels)."""
@lru_cache(maxsize=4096)
def _term_for_offering_cached(offering_id: str) -> dict | None:
if not offering_id:
return None
rows = table("course_offerings").select(
Expand All@@ -137,6 +144,24 @@ def term_for_offering(offering_id: str) -> dict | None:
return terms[0] if terms else None


def term_for_offering(offering_id: str) -> dict | None:
"""The term row for an offering (for semester labels).

Cached per-process (#98): the offering→term mapping is immutable and terms
are seeded reference data that don't change at runtime. Returns a deep copy
so callers can't mutate the shared cached row."""
cached = _term_for_offering_cached(offering_id)
return copy.deepcopy(cached) if cached is not None else None


def clear_academics_caches() -> None:
"""Clear the per-process academics caches. Called from test setup (so mocked
DB state doesn't leak across tests); rarely needed at runtime since the
cached mappings are immutable."""
offering_course_id.cache_clear()
_term_for_offering_cached.cache_clear()


def user_enrollment_ids(user_id: str) -> list[dict]:
"""The user's enrollments as ``{id, offering_id}`` rows (read + scoping helper)."""
if not user_id:
Expand Down
36 changes: 28 additions & 8 deletions backend/services/course_context_service.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,9 +11,11 @@
- offering_summary: class-wide summary with Gemini-generated text (per offering)
"""

import copy
import json
import hashlib
from datetime import datetime, timezone
from functools import lru_cache

from db.connection import table
from services.gemini_service import call_gemini
Expand DownExpand Up@@ -65,14 +67,8 @@ def _generate_summary_with_gemini(
)


def get_course_context(offering_id: str) -> dict:
"""
Return the cached class context for an offering: summary + concept stats.
Offering-scoped (one class instance in one term). Returns {} if not found.
"""
if not offering_id:
return {}

@lru_cache(maxsize=512)
def _get_course_context_cached(offering_id: str) -> dict:
try:
# Get the offering summary
summary_rows = table("offering_summary").select(
Expand DownExpand Up@@ -106,6 +102,26 @@ def get_course_context(offering_id: str) -> dict:
return {}


def get_course_context(offering_id: str) -> dict:
"""
Return the cached class context for an offering: summary + concept stats.
Offering-scoped (one class instance in one term). Returns {} if not found.

Cached per-process (#98) and invalidated by ``update_course_context`` (which
runs after graph updates). Returns a deep copy so callers can't mutate the
shared cached value.
"""
if not offering_id:
return {}
return copy.deepcopy(_get_course_context_cached(offering_id))


def clear_course_context_cache() -> None:
"""Drop the per-process course-context cache. Called whenever the underlying
aggregates change (``update_course_context``) and from test setup."""
_get_course_context_cached.cache_clear()


def update_course_context(offering_id: str) -> None:
"""
Aggregate mastery + quiz data for all students enrolled in an **offering**
Expand All@@ -127,6 +143,7 @@ def update_course_context(offering_id: str) -> None:
# No students enrolled — purge any stale aggregates
table("offering_concept_stats").delete({"offering_id": f"eq.{offering_id}"})
table("offering_summary").delete({"offering_id": f"eq.{offering_id}"})
clear_course_context_cache() # #98: aggregates changed → drop cached read
return

user_ids = [r["user_id"] for r in enrollment_rows]
Expand DownExpand Up@@ -335,3 +352,6 @@ def _parse_quiz_context_to_arrays(ctx_rows: list) -> tuple[list, list, list]:
},
on_conflict="offering_id",
)

# #98: the aggregates this offering's context reads from just changed.
clear_course_context_cache()
12 changes: 12 additions & 0 deletions backend/tests/conftest.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,18 @@ def pytest_configure(config):
)


@pytest.fixture(autouse=True)
def _clear_lru_caches():
"""#98: reset the per-process lru_caches around every test so one test's
mocked DB state can't leak into another via a cached read."""
from services import academics, course_context_service
academics.clear_academics_caches()
course_context_service.clear_course_context_cache()
yield
academics.clear_academics_caches()
course_context_service.clear_course_context_cache()


@pytest.fixture(autouse=True)
def _hermetic_supabase_client(request, monkeypatch):
"""Hermetic safety net (#210): no test may make a real Supabase call.
Expand Down
112 changes: 112 additions & 0 deletions backend/tests/test_lru_cache.py
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
"""Tests for the per-process lru_caches (#98): academics resolvers and
course-context, including the invalidation hook.

The autouse `_clear_lru_caches` fixture in conftest clears these before each
test, so each starts with an empty cache."""
from unittest.mock import MagicMock, patch

from services import academics, course_context_service


class TestOfferingCourseIdCache:
def test_second_call_served_from_cache(self):
calls = {"n": 0}

def _table(name):
m = MagicMock()
def _select(*a, **k):
calls["n"] += 1
return [{"course_id": "c1"}]
m.select.side_effect = _select
return m

with patch("services.academics.table", side_effect=_table):
assert academics.offering_course_id("off1") == "c1"
assert academics.offering_course_id("off1") == "c1"
assert calls["n"] == 1 # only the first call hit the DB

def test_distinct_offerings_not_conflated(self):
def _table(name):
m = MagicMock()
def _select(cols, filters=None, **k):
off = filters["id"].split(".")[-1]
return [{"course_id": f"c-{off}"}]
m.select.side_effect = _select
return m

with patch("services.academics.table", side_effect=_table):
assert academics.offering_course_id("A") == "c-A"
assert academics.offering_course_id("B") == "c-B"


class TestTermForOfferingCache:
def _table(self, name):
m = MagicMock()
if name == "course_offerings":
m.select.return_value = [{"term_id": "t1"}]
elif name == "terms":
m.select.return_value = [{"id": "t1", "name": "Fall 2026"}]
else:
m.select.return_value = []
return m

def test_returns_defensive_copy(self):
with patch("services.academics.table", side_effect=self._table):
t1 = academics.term_for_offering("off1")
t1["name"] = "MUTATED" # caller mutates the returned dict
t2 = academics.term_for_offering("off1")
assert t2["name"] == "Fall 2026" # cache not corrupted


class TestCourseContextCacheInvalidation:
def _ctx_table(self, summary_text):
def _table(name):
m = MagicMock()
if name == "offering_summary":
m.select.return_value = [{
"offering_id": "o1", "student_count": 1,
"avg_class_mastery": 0.5, "summary_text": summary_text,
"updated_at": "t",
}]
else:
m.select.return_value = []
return m
return _table

def test_cached_until_explicitly_cleared(self):
with patch("services.course_context_service.table", side_effect=self._ctx_table("v1")):
c1 = course_context_service.get_course_context("o1")
assert c1["course_summary"]["summary_text"] == "v1"

# DB now returns v2, but the cache still serves v1 until cleared.
with patch("services.course_context_service.table", side_effect=self._ctx_table("v2")):
cached = course_context_service.get_course_context("o1")
assert cached["course_summary"]["summary_text"] == "v1"
course_context_service.clear_course_context_cache()
fresh = course_context_service.get_course_context("o1")
assert fresh["course_summary"]["summary_text"] == "v2"

def test_update_course_context_invalidates(self):
# Prime the cache with v1.
with patch("services.course_context_service.table", side_effect=self._ctx_table("v1")):
course_context_service.get_course_context("o1")

# update_course_context with no enrollments → purge path → cache_clear.
def _no_enrollments(name):
m = MagicMock()
m.select.return_value = []
return m
with patch("services.course_context_service.table", side_effect=_no_enrollments):
course_context_service.update_course_context("o1")

# Cache was invalidated, so the next read re-fetches (now v2).
with patch("services.course_context_service.table", side_effect=self._ctx_table("v2")):
after = course_context_service.get_course_context("o1")
assert after["course_summary"]["summary_text"] == "v2"

def test_returns_defensive_copy(self):
with patch("services.course_context_service.table", side_effect=self._ctx_table("v1")):
a = course_context_service.get_course_context("o1")
a["course_summary"]["summary_text"] = "MUTATED"
b = course_context_service.get_course_context("o1")
assert b["course_summary"]["summary_text"] == "v1" # cache intact
48 changes: 48 additions & 0 deletions specs/98-lru-cache.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
# Spec: functools.lru_cache for hot deterministic reads (#98)

## Scope

Milestone #2 (perf). Per-process (per-worker) caching of hot, deterministic reads with a clear
invalidation story. Composes with the HTTP cache (#99) and a future cross-worker Redis layer (#97).

## Requirements

### R1 — Cache immutable academics resolvers (no invalidation needed)
- `services/academics.offering_course_id(offering_id)` — `@lru_cache`. An offering's `course_id` is set
at creation and never changes → deterministic immutable mapping; returns an immutable `str`/`None`.
- `services/academics.term_for_offering(offering_id)` — `@lru_cache` on the private body; the public
function returns a `copy.deepcopy` (mutable dict) so callers can't corrupt the cache. Offering→term is
immutable and terms are seeded reference data.
- `services/academics.clear_academics_caches()` clears both (for tests).

### R2 — Cache course-context with an invalidation hook
- `services/course_context_service.get_course_context(offering_id)` — `@lru_cache` on the private body;
the public function returns a `copy.deepcopy`.
- `clear_course_context_cache()` is called from `update_course_context` on **every** write path (the
no-enrollment purge and the final upsert). `update_course_context` is the choke point that
`apply_graph_update` and the doc/grade post-rolls funnel through, so a graph/doc/grade change that
moves the aggregates always drops the stale cached read.

### R3 — Test isolation
- An autouse `_clear_lru_caches` fixture in `tests/conftest.py` clears all these caches around every test
so one test's mocked DB state can't leak into another via a cached read.

### R4 — Docs
- CLAUDE.md Conventions: `lru_cache` is reserved for deterministic per-process reads — immutable mappings
(no invalidation) or reads with a matching `clear_*_cache()` every mutator calls; hashable args only;
deep-copy mutable returns.

## Acceptance
1. 3+ functions cached, each provably safe (immutable, or hooked + deep-copied).
2. `clear_course_context_cache()` invoked from `update_course_context`'s write paths.
3. CLAUDE.md documents the convention.
4. Tests: cache hit avoids a 2nd DB read; distinct keys not conflated; deep-copy immunity; mutate via
`update_course_context` → next read returns fresh.
5. `pytest tests/ -q` shows no new failures vs `main` (proves the conftest fixture stops cache leakage);
`ruff` clean.

## Deliberately NOT cached (documented risk)
- Graph reads (`get_graph`) — large mutable structures with hot, per-turn invalidation; deep-copy cost +
invalidation surface outweigh the win here. Revisit with the Redis layer (#97).
- Token decode / `require_self` — security-sensitive and needs a TTL bounded to token lifetime; out of
scope for a plain `lru_cache`.
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -75,6 +75,7 @@ ruff format . # formatter — available, not yet CI-gated (see
- Display names are resolved via `services/profiles.py` (`get_display_name`/`get_display_names`), which decrypts off `user_profiles` — don't read name columns off `users`.
- All current LLM calls route through `services/gemini_service.py` (`call_gemini`, `call_gemini_json`, `call_gemini_multiturn`). New LLM-driven code should be written as Pydantic AI agents in `backend/agents/` rather than extending `gemini_service.py`.
- Knowledge-graph mutations go through `services/graph_service.py::apply_graph_update` — routes never write `graph_nodes`/`graph_edges` directly.
- `functools.lru_cache` is reserved for **deterministic, per-process reads** (#98) — either immutable mappings that never need invalidation (e.g. `academics.offering_course_id`) or reads with a matching `clear_*_cache()` hook that every mutator calls (e.g. `course_context_service.get_course_context` is cleared by `update_course_context`). Cache only hashable-arg functions; return a deep copy if the cached value is mutable; never cache without a clear invalidation story. The autouse `_clear_lru_caches` fixture in `tests/conftest.py` resets these between tests.
- Backend tests live in `backend/tests/` and run via `pytest`; shared fixtures (mock Supabase, mock Gemini) are in `tests/conftest.py`.
- Routers are mounted in `main.py` with `/api/<name>` prefixes; new routes follow that pattern.

Expand Down
31 changes: 28 additions & 3 deletions backend/services/academics.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,8 +15,10 @@
"""
from __future__ import annotations

import copy
import uuid
from datetime import date
from functools import lru_cache

from db.connection import table

Expand DownExpand Up@@ -93,8 +95,13 @@ def resolve_offering(
return any_off[0]["id"] if any_off else None


@lru_cache(maxsize=4096)
def offering_course_id(offering_id: str) -> str | None:
"""The abstract course id an offering belongs to (offering → graph bridge)."""
"""The abstract course id an offering belongs to (offering → graph bridge).

Cached per-process (#98): an offering's ``course_id`` is set at creation and
never changes, so this is a deterministic immutable mapping — no invalidation
hook needed. Returns an immutable ``str``/``None`` (safe to share)."""
if not offering_id:
return None
rows = table("course_offerings").select(
Expand All@@ -121,8 +128,8 @@ def user_offering_ids_for_course(user_id: str, course_id: str) -> list[str]:
return [e["offering_id"] for e in enr if e.get("offering_id") in off_ids]


def term_for_offering(offering_id: str) -> dict | None:
"""The term row for an offering (for semester labels)."""
@lru_cache(maxsize=4096)
def _term_for_offering_cached(offering_id: str) -> dict | None:
if not offering_id:
return None
rows = table("course_offerings").select(
Expand All@@ -137,6 +144,24 @@ def term_for_offering(offering_id: str) -> dict | None:
return terms[0] if terms else None


def term_for_offering(offering_id: str) -> dict | None:
"""The term row for an offering (for semester labels).

Cached per-process (#98): the offering→term mapping is immutable and terms
are seeded reference data that don't change at runtime. Returns a deep copy
so callers can't mutate the shared cached row."""
cached = _term_for_offering_cached(offering_id)
return copy.deepcopy(cached) if cached is not None else None


def clear_academics_caches() -> None:
"""Clear the per-process academics caches. Called from test setup (so mocked
DB state doesn't leak across tests); rarely needed at runtime since the
cached mappings are immutable."""
offering_course_id.cache_clear()
_term_for_offering_cached.cache_clear()


def user_enrollment_ids(user_id: str) -> list[dict]:
"""The user's enrollments as ``{id, offering_id}`` rows (read + scoping helper)."""
if not user_id:
Expand Down
36 changes: 28 additions & 8 deletions backend/services/course_context_service.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,9 +11,11 @@
- offering_summary: class-wide summary with Gemini-generated text (per offering)
"""

import copy
import json
import hashlib
from datetime import datetime, timezone
from functools import lru_cache

from db.connection import table
from services.gemini_service import call_gemini
Expand DownExpand Up@@ -65,14 +67,8 @@ def _generate_summary_with_gemini(
)


def get_course_context(offering_id: str) -> dict:
"""
Return the cached class context for an offering: summary + concept stats.
Offering-scoped (one class instance in one term). Returns {} if not found.
"""
if not offering_id:
return {}

@lru_cache(maxsize=512)
def _get_course_context_cached(offering_id: str) -> dict:
try:
# Get the offering summary
summary_rows = table("offering_summary").select(
Expand DownExpand Up@@ -106,6 +102,26 @@ def get_course_context(offering_id: str) -> dict:
return {}


def get_course_context(offering_id: str) -> dict:
"""
Return the cached class context for an offering: summary + concept stats.
Offering-scoped (one class instance in one term). Returns {} if not found.

Cached per-process (#98) and invalidated by ``update_course_context`` (which
runs after graph updates). Returns a deep copy so callers can't mutate the
shared cached value.
"""
if not offering_id:
return {}
return copy.deepcopy(_get_course_context_cached(offering_id))


def clear_course_context_cache() -> None:
"""Drop the per-process course-context cache. Called whenever the underlying
aggregates change (``update_course_context``) and from test setup."""
_get_course_context_cached.cache_clear()


def update_course_context(offering_id: str) -> None:
"""
Aggregate mastery + quiz data for all students enrolled in an **offering**
Expand All@@ -127,6 +143,7 @@ def update_course_context(offering_id: str) -> None:
# No students enrolled — purge any stale aggregates
table("offering_concept_stats").delete({"offering_id": f"eq.{offering_id}"})
table("offering_summary").delete({"offering_id": f"eq.{offering_id}"})
clear_course_context_cache() # #98: aggregates changed → drop cached read
return

user_ids = [r["user_id"] for r in enrollment_rows]
Expand DownExpand Up@@ -335,3 +352,6 @@ def _parse_quiz_context_to_arrays(ctx_rows: list) -> tuple[list, list, list]:
},
on_conflict="offering_id",
)

# #98: the aggregates this offering's context reads from just changed.
clear_course_context_cache()
12 changes: 12 additions & 0 deletions backend/tests/conftest.py
Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,18 @@ def pytest_configure(config):
)


@pytest.fixture(autouse=True)
def _clear_lru_caches():
"""#98: reset the per-process lru_caches around every test so one test's
mocked DB state can't leak into another via a cached read."""
from services import academics, course_context_service
academics.clear_academics_caches()
course_context_service.clear_course_context_cache()
yield
academics.clear_academics_caches()
course_context_service.clear_course_context_cache()


@pytest.fixture(autouse=True)
def _hermetic_supabase_client(request, monkeypatch):
"""Hermetic safety net (#210): no test may make a real Supabase call.
Expand Down
112 changes: 112 additions & 0 deletions backend/tests/test_lru_cache.py
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
"""Tests for the per-process lru_caches (#98): academics resolvers and
course-context, including the invalidation hook.

The autouse `_clear_lru_caches` fixture in conftest clears these before each
test, so each starts with an empty cache."""
from unittest.mock import MagicMock, patch

from services import academics, course_context_service


class TestOfferingCourseIdCache:
def test_second_call_served_from_cache(self):
calls = {"n": 0}

def _table(name):
m = MagicMock()
def _select(*a, **k):
calls["n"] += 1
return [{"course_id": "c1"}]
m.select.side_effect = _select
return m

with patch("services.academics.table", side_effect=_table):
assert academics.offering_course_id("off1") == "c1"
assert academics.offering_course_id("off1") == "c1"
assert calls["n"] == 1 # only the first call hit the DB

def test_distinct_offerings_not_conflated(self):
def _table(name):
m = MagicMock()
def _select(cols, filters=None, **k):
off = filters["id"].split(".")[-1]
return [{"course_id": f"c-{off}"}]
m.select.side_effect = _select
return m

with patch("services.academics.table", side_effect=_table):
assert academics.offering_course_id("A") == "c-A"
assert academics.offering_course_id("B") == "c-B"


class TestTermForOfferingCache:
def _table(self, name):
m = MagicMock()
if name == "course_offerings":
m.select.return_value = [{"term_id": "t1"}]
elif name == "terms":
m.select.return_value = [{"id": "t1", "name": "Fall 2026"}]
else:
m.select.return_value = []
return m

def test_returns_defensive_copy(self):
with patch("services.academics.table", side_effect=self._table):
t1 = academics.term_for_offering("off1")
t1["name"] = "MUTATED" # caller mutates the returned dict
t2 = academics.term_for_offering("off1")
assert t2["name"] == "Fall 2026" # cache not corrupted


class TestCourseContextCacheInvalidation:
def _ctx_table(self, summary_text):
def _table(name):
m = MagicMock()
if name == "offering_summary":
m.select.return_value = [{
"offering_id": "o1", "student_count": 1,
"avg_class_mastery": 0.5, "summary_text": summary_text,
"updated_at": "t",
}]
else:
m.select.return_value = []
return m
return _table

def test_cached_until_explicitly_cleared(self):
with patch("services.course_context_service.table", side_effect=self._ctx_table("v1")):
c1 = course_context_service.get_course_context("o1")
assert c1["course_summary"]["summary_text"] == "v1"

# DB now returns v2, but the cache still serves v1 until cleared.
with patch("services.course_context_service.table", side_effect=self._ctx_table("v2")):
cached = course_context_service.get_course_context("o1")
assert cached["course_summary"]["summary_text"] == "v1"
course_context_service.clear_course_context_cache()
fresh = course_context_service.get_course_context("o1")
assert fresh["course_summary"]["summary_text"] == "v2"

def test_update_course_context_invalidates(self):
# Prime the cache with v1.
with patch("services.course_context_service.table", side_effect=self._ctx_table("v1")):
course_context_service.get_course_context("o1")

# update_course_context with no enrollments → purge path → cache_clear.
def _no_enrollments(name):
m = MagicMock()
m.select.return_value = []
return m
with patch("services.course_context_service.table", side_effect=_no_enrollments):
course_context_service.update_course_context("o1")

# Cache was invalidated, so the next read re-fetches (now v2).
with patch("services.course_context_service.table", side_effect=self._ctx_table("v2")):
after = course_context_service.get_course_context("o1")
assert after["course_summary"]["summary_text"] == "v2"

def test_returns_defensive_copy(self):
with patch("services.course_context_service.table", side_effect=self._ctx_table("v1")):
a = course_context_service.get_course_context("o1")
a["course_summary"]["summary_text"] = "MUTATED"
b = course_context_service.get_course_context("o1")
assert b["course_summary"]["summary_text"] == "v1" # cache intact
48 changes: 48 additions & 0 deletions specs/98-lru-cache.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
# Spec: functools.lru_cache for hot deterministic reads (#98)

## Scope

Milestone #2 (perf). Per-process (per-worker) caching of hot, deterministic reads with a clear
invalidation story. Composes with the HTTP cache (#99) and a future cross-worker Redis layer (#97).

## Requirements

### R1 — Cache immutable academics resolvers (no invalidation needed)
- `services/academics.offering_course_id(offering_id)` — `@lru_cache`. An offering's `course_id` is set
at creation and never changes → deterministic immutable mapping; returns an immutable `str`/`None`.
- `services/academics.term_for_offering(offering_id)` — `@lru_cache` on the private body; the public
function returns a `copy.deepcopy` (mutable dict) so callers can't corrupt the cache. Offering→term is
immutable and terms are seeded reference data.
- `services/academics.clear_academics_caches()` clears both (for tests).

### R2 — Cache course-context with an invalidation hook
- `services/course_context_service.get_course_context(offering_id)` — `@lru_cache` on the private body;
the public function returns a `copy.deepcopy`.
- `clear_course_context_cache()` is called from `update_course_context` on **every** write path (the
no-enrollment purge and the final upsert). `update_course_context` is the choke point that
`apply_graph_update` and the doc/grade post-rolls funnel through, so a graph/doc/grade change that
moves the aggregates always drops the stale cached read.

### R3 — Test isolation
- An autouse `_clear_lru_caches` fixture in `tests/conftest.py` clears all these caches around every test
so one test's mocked DB state can't leak into another via a cached read.

### R4 — Docs
- CLAUDE.md Conventions: `lru_cache` is reserved for deterministic per-process reads — immutable mappings
(no invalidation) or reads with a matching `clear_*_cache()` every mutator calls; hashable args only;
deep-copy mutable returns.

## Acceptance
1. 3+ functions cached, each provably safe (immutable, or hooked + deep-copied).
2. `clear_course_context_cache()` invoked from `update_course_context`'s write paths.
3. CLAUDE.md documents the convention.
4. Tests: cache hit avoids a 2nd DB read; distinct keys not conflated; deep-copy immunity; mutate via
`update_course_context` → next read returns fresh.
5. `pytest tests/ -q` shows no new failures vs `main` (proves the conftest fixture stops cache leakage);
`ruff` clean.

## Deliberately NOT cached (documented risk)
- Graph reads (`get_graph`) — large mutable structures with hot, per-turn invalidation; deep-copy cost +
invalidation surface outweigh the win here. Revisit with the Redis layer (#97).
- Token decode / `require_self` — security-sensitive and needs a TTL bounded to token lifetime; out of
scope for a plain `lru_cache`.
Loading