Skip to content

Commit 2e7dbc3

Browse files
ShreeBoharaclaude
andcommitted
Add a GraphQL surface alongside REST for the learner read path
Mounted at /graphql. Additive, not a migration: every REST route still works and there are tests asserting they do. WHY The learn page has a measured five-round-trip waterfall on lesson completion. The POST already returns {xp_gained, stats}; the client discards them and calls refreshStats(), which fires four more GETs (stats, achievements, activity, completed lessons) against the same SQLite file for the same repository. learnerDashboard collapses the four reads into one request, and completeLesson returns the post-mutation dashboard inline so the refresh is unnecessary. BE PRECISE ABOUT THE BENEFIT GraphQL does NOT reduce database work here, and measuring it disproved the intuitive claim: the combined resolver issues MORE SQL statements than the four REST handlers (6 vs 4 on an empty repo), because it performs the same four reads plus session overhead. What it removes is four HTTP round trips, four dependency-injection cycles and four session open/close pairs. The test and docstrings say this rather than claiming a query-count win that does not exist. THE STRAWBERRY LANDMINE Strawberry documents that it "processes sync and async fields using the event loop, which means that using a sync def will block the entire worker" -- unlike FastAPI there is no automatic threadpool. dependencies.get_db hands out a synchronous SQLAlchemy Session, so a single sync resolver would serialize blocking SQLite calls on the loop and stall in-flight chat streams. Every resolver is therefore async and offloads via run_in_threadpool, and two tests enforce it: one reflects over Query/Mutation asserting no resolver is a sync def, the other asserts each blocking helper is only reached through run_in_threadpool. AsyncSession is not the alternative -- a single AsyncSession is documented as unsafe across concurrent tasks, which is how DataLoader batches, and greenlet is not installed. SCOPE Chat deliberately stays REST/SSE. GraphQL incremental delivery (@defer/@stream) is not ratified: absent from the September 2025 spec edition, RFC open since 2024-09-18, and Strawberry's support is experimental requiring graphql-core>=3.3.0a9 against 3.2.11 stable. A test asserts the schema exposes no chat or stream field. strawberry-graphql is pinned with an upper bound (>=0.240,<1.0) -- it is a weekly-releasing 0.x with a documented breaking-change history, and this file otherwise uses open lower bounds. Resolvers reuse GamificationService rather than reimplementing anything, so GraphQL and REST cannot diverge in behaviour. ALSO: removed three iCloud-duplicated files ("neo4j_store 2.py", "__init__ 2.py", "test_neo4j_graph_store 2.py") that macOS had created after the previous commit, and added a .gitignore rule for the "* 2.*" pattern. The duplicated test file was being collected by pytest as a second copy of the same 15 tests -- the suite reported 157 with it present and 142 without, which is how it was caught. Verified: 10 new tests, 142 total (132 + 10; the earlier 157 was the inflated count), ruff clean, 43 routes, schema builds and mounts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 2a15ec6 commit 2e7dbc3

7 files changed

Lines changed: 598 additions & 0 deletions

File tree

.gitignore

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,3 +84,12 @@ htmlcov/
8484
.cache/
8585
Documents/
8686
/repos/
87+
88+
# -----------------------
89+
# macOS / iCloud duplicate artifacts
90+
# -----------------------
91+
# iCloud Drive appends " 2"/" 3" to filenames it duplicates. These are never real
92+
# source files, and a duplicated test_*.py would be collected by pytest as a second copy
93+
# of the same tests.
94+
* 2.*
95+
* 3.*

README.md

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -455,3 +455,43 @@ pnpm web:verify-css
455455
## License
456456

457457
MIT
458+
459+
## GraphQL (optional, alongside REST)
460+
461+
A GraphQL surface is mounted at `/graphql` **in addition to** the REST API — nothing was
462+
migrated, and every REST route still works.
463+
464+
It exists for one measured reason: completing a lesson used to be five HTTP round trips
465+
(the POST already returned `{xp_gained, stats}`, the client discarded them and fired four
466+
more GETs for stats, achievements, activity and completed lessons). Two operations
467+
collapse that:
468+
469+
```graphql
470+
query { learnerDashboard(repoId: "...") {
471+
stats { totalXp level { level title } }
472+
achievements { key unlocked }
473+
activity { date count }
474+
completedLessons
475+
} }
476+
477+
mutation { completeLesson(repoId: "...", lessonId: "...", timeSpentSeconds: 120) {
478+
xpGained { amount reason }
479+
dashboard { stats { totalXp } completedLessons } # post-mutation state inline
480+
} }
481+
```
482+
483+
Two deliberate boundaries:
484+
485+
- **Chat stays on REST/SSE.** GraphQL's incremental delivery (`@defer`/`@stream`) is not
486+
ratified — absent from the September 2025 spec edition, RFC open since 2024-09-18, and
487+
Strawberry's support is experimental requiring `graphql-core>=3.3.0a9` against 3.2.11
488+
stable. Token streaming over GraphQL would mean betting on an unratified extension.
489+
- **Every resolver is `async` and offloads DB work via `run_in_threadpool`.** Strawberry
490+
has no threadpool for sync resolvers (unlike FastAPI), and this app uses a synchronous
491+
SQLAlchemy `Session` — one sync resolver would serialize blocking SQLite calls on the
492+
event loop and stall in-flight chat streams. A test enforces this.
493+
494+
Note on the benefit: GraphQL does **not** reduce database work here. Measured, the
495+
combined resolver issues slightly more SQL than the four REST handlers. What it removes
496+
is four network round trips, four dependency-injection cycles and four session
497+
open/close pairs.

apps/api/requirements.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ tree-sitter-ruby>=0.21.0
4646
# HTTP Client
4747
httpx>=0.27.0
4848

49+
# GraphQL surface, mounted alongside REST. Upper bound is deliberate: this is a
50+
# weekly-releasing 0.x with a documented breaking-change history.
51+
strawberry-graphql[fastapi]>=0.240,<1.0
52+
4953
# Graph read model (optional; only imported when NEO4J_ENABLED=true)
5054
neo4j>=5.28
5155

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
"""GraphQL surface, mounted alongside the REST routes (see schema.py for scope)."""

apps/api/src/api/graphql/schema.py

Lines changed: 292 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,292 @@
1+
"""
2+
GraphQL schema, mounted alongside the REST routes rather than replacing them.
3+
4+
WHY THIS EXISTS
5+
The learn page has a measured request waterfall. Completing a lesson is five round
6+
trips: the POST already returns {xp_gained, stats}, the client discards the stats and
7+
then calls refreshStats(), which fires four more GETs (stats, achievements, activity,
8+
completed lessons). Every one of those hits the same SQLite file for the same repository.
9+
`learnerDashboard` collapses the four reads into one request, and `completeLesson`
10+
returns the post-mutation dashboard inline so the client never needs the refresh.
11+
12+
WHAT STAYS ON REST
13+
Chat. It is SSE token streaming (routes/chat.py), and GraphQL's incremental delivery
14+
(@defer/@stream) is not ratified -- it is absent from the September 2025 spec edition,
15+
its RFC has been open since 2024-09-18, and Strawberry's support is experimental and
16+
requires graphql-core>=3.3.0a9 while the installed stable is 3.2.11. Streaming tokens
17+
over GraphQL here would mean betting on an unratified extension for no gain.
18+
19+
THE THREADING RULE -- READ BEFORE ADDING A RESOLVER
20+
Strawberry documents that it "processes sync and async fields using the event loop, which
21+
means that using a sync def will block the entire worker". Unlike FastAPI, there is NO
22+
automatic threadpool for sync resolvers. dependencies.get_db hands out a synchronous
23+
SQLAlchemy Session, so every resolver that touches the database MUST go through
24+
run_in_threadpool. A single sync resolver here would serialize blocking SQLite calls on
25+
the event loop and stall in-flight chat streams.
26+
27+
Switching to AsyncSession is not the fix: a single AsyncSession is documented as unsafe
28+
across concurrent tasks, which is exactly how a DataLoader batches, and greenlet (which
29+
SQLAlchemy's async bridge requires) is not installed.
30+
"""
31+
32+
from __future__ import annotations
33+
34+
from typing import Dict, List, Optional
35+
36+
import strawberry
37+
from starlette.concurrency import run_in_threadpool
38+
39+
from src.core.demo_mode import assert_demo_repo_access
40+
from src.dependencies import get_session_factory
41+
from src.models.database import Repository
42+
from src.services.gamification import GamificationService
43+
44+
# --- types -----------------------------------------------------------------------
45+
46+
@strawberry.type
47+
class Level:
48+
level: int
49+
title: str
50+
icon: str
51+
current_xp: int
52+
xp_for_next_level: int
53+
xp_progress: float
54+
55+
56+
@strawberry.type
57+
class Streak:
58+
current: int
59+
longest: int
60+
active_today: bool
61+
62+
63+
@strawberry.type
64+
class UserStats:
65+
total_xp: int
66+
level: Level
67+
streak: Streak
68+
lessons_completed: int
69+
quizzes_passed: int
70+
challenges_completed: int
71+
perfect_quizzes: int
72+
73+
74+
@strawberry.type
75+
class Achievement:
76+
key: str
77+
name: str
78+
description: str
79+
icon: str
80+
category: str
81+
xp_reward: int
82+
unlocked: bool
83+
requirement: Optional[int] = None
84+
85+
86+
@strawberry.type
87+
class ActivityDay:
88+
"""Activity history as a list rather than a map: GraphQL has no arbitrary-key type."""
89+
date: str
90+
count: int
91+
92+
93+
@strawberry.type
94+
class XPGain:
95+
amount: int
96+
reason: str
97+
bonus: Optional[int] = None
98+
bonus_reason: Optional[str] = None
99+
100+
101+
@strawberry.type
102+
class RepoSummary:
103+
id: str
104+
github_owner: str
105+
github_name: str
106+
status: str
107+
total_files: int
108+
total_chunks: int
109+
primary_language: Optional[str] = None
110+
111+
112+
@strawberry.type
113+
class LearnerDashboard:
114+
"""
115+
Everything the learn page needs after any progress event.
116+
117+
This is the shape that replaces four separate GETs. Keeping it one type (rather than
118+
four top-level fields) means the mutation can return it inline, which is what removes
119+
the fifth round trip.
120+
"""
121+
repo_id: str
122+
stats: UserStats
123+
achievements: List[Achievement]
124+
activity: List[ActivityDay]
125+
completed_lessons: List[str]
126+
127+
128+
@strawberry.type
129+
class CompleteLessonResult:
130+
xp_gained: XPGain
131+
dashboard: LearnerDashboard
132+
133+
134+
# --- mapping from the existing service layer -------------------------------------
135+
# Deliberately reuses GamificationService so GraphQL and REST cannot diverge in
136+
# behaviour. These are pure functions over already-fetched data -- no I/O.
137+
138+
def _to_stats(raw) -> UserStats:
139+
d = raw if isinstance(raw, dict) else raw.model_dump()
140+
lvl, stk = d["level"], d["streak"]
141+
return UserStats(
142+
total_xp=d["total_xp"],
143+
level=Level(
144+
level=lvl["level"], title=lvl["title"], icon=lvl["icon"],
145+
current_xp=lvl["current_xp"], xp_for_next_level=lvl["xp_for_next_level"],
146+
xp_progress=lvl["xp_progress"],
147+
),
148+
streak=Streak(
149+
current=stk["current"], longest=stk["longest"], active_today=stk["active_today"]
150+
),
151+
lessons_completed=d["lessons_completed"],
152+
quizzes_passed=d["quizzes_passed"],
153+
challenges_completed=d["challenges_completed"],
154+
perfect_quizzes=d["perfect_quizzes"],
155+
)
156+
157+
158+
def _to_achievements(raw) -> List[Achievement]:
159+
out = []
160+
for a in raw:
161+
d = a if isinstance(a, dict) else a.model_dump()
162+
out.append(Achievement(
163+
key=d["key"], name=d["name"], description=d["description"], icon=d["icon"],
164+
category=d["category"], xp_reward=d["xp_reward"],
165+
unlocked=bool(d.get("unlocked", False)), requirement=d.get("requirement"),
166+
))
167+
return out
168+
169+
170+
def _to_activity(raw: Dict[str, int]) -> List[ActivityDay]:
171+
return [ActivityDay(date=k, count=v) for k, v in sorted((raw or {}).items())]
172+
173+
174+
def _to_xp_gain(raw) -> XPGain:
175+
d = raw if isinstance(raw, dict) else raw.model_dump()
176+
return XPGain(
177+
amount=d["amount"], reason=d["reason"],
178+
bonus=d.get("bonus"), bonus_reason=d.get("bonus_reason"),
179+
)
180+
181+
182+
# --- blocking work, always off the event loop ------------------------------------
183+
184+
def _load_dashboard_sync(repo_id: str, persona: Optional[str]) -> LearnerDashboard:
185+
"""
186+
All four reads in one session, on a worker thread.
187+
188+
A fresh Session per call, not a request-scoped one: Session is not thread-safe, and
189+
this runs on a threadpool worker.
190+
"""
191+
db = get_session_factory()()
192+
try:
193+
assert_demo_repo_access(db, repo_id)
194+
service = GamificationService(db)
195+
return LearnerDashboard(
196+
repo_id=repo_id,
197+
stats=_to_stats(service.get_user_stats(repo_id)),
198+
achievements=_to_achievements(service.get_all_achievements(repo_id)),
199+
activity=_to_activity(service.get_activity_history(repo_id)),
200+
completed_lessons=list(service.get_completed_lessons(repo_id, persona=persona)),
201+
)
202+
finally:
203+
db.close()
204+
205+
206+
def _complete_lesson_sync(
207+
repo_id: str, lesson_id: str, time_spent_seconds: int,
208+
persona: Optional[str], module_id: Optional[str],
209+
) -> CompleteLessonResult:
210+
db = get_session_factory()()
211+
try:
212+
assert_demo_repo_access(db, repo_id)
213+
service = GamificationService(db)
214+
xp_gain = service.record_lesson_complete(
215+
repo_id, lesson_id, time_spent_seconds, persona=persona, module_id=module_id
216+
)
217+
# Read the dashboard back in the SAME session, after the write, so the client
218+
# cannot observe a state that predates its own mutation.
219+
dashboard = LearnerDashboard(
220+
repo_id=repo_id,
221+
stats=_to_stats(service.get_user_stats(repo_id)),
222+
achievements=_to_achievements(service.get_all_achievements(repo_id)),
223+
activity=_to_activity(service.get_activity_history(repo_id)),
224+
completed_lessons=list(service.get_completed_lessons(repo_id, persona=persona)),
225+
)
226+
return CompleteLessonResult(xp_gained=_to_xp_gain(xp_gain), dashboard=dashboard)
227+
finally:
228+
db.close()
229+
230+
231+
def _load_repo_sync(repo_id: str) -> Optional[RepoSummary]:
232+
db = get_session_factory()()
233+
try:
234+
assert_demo_repo_access(db, repo_id)
235+
repo = db.query(Repository).filter(Repository.id == repo_id).first()
236+
if not repo:
237+
return None
238+
return RepoSummary(
239+
id=repo.id,
240+
github_owner=repo.github_owner,
241+
github_name=repo.github_name,
242+
status=repo.status.value if hasattr(repo.status, "value") else str(repo.status),
243+
total_files=repo.total_files or 0,
244+
total_chunks=repo.total_chunks or 0,
245+
primary_language=repo.primary_language,
246+
)
247+
finally:
248+
db.close()
249+
250+
251+
# --- schema ----------------------------------------------------------------------
252+
253+
@strawberry.type
254+
class Query:
255+
@strawberry.field(description="Repository summary.")
256+
async def repo(self, repo_id: str) -> Optional[RepoSummary]:
257+
return await run_in_threadpool(_load_repo_sync, repo_id)
258+
259+
@strawberry.field(
260+
description=(
261+
"Stats, achievements, activity and completed lessons in one request. "
262+
"Replaces four separate REST GETs."
263+
)
264+
)
265+
async def learner_dashboard(
266+
self, repo_id: str, persona: Optional[str] = None
267+
) -> LearnerDashboard:
268+
return await run_in_threadpool(_load_dashboard_sync, repo_id, persona)
269+
270+
271+
@strawberry.type
272+
class Mutation:
273+
@strawberry.mutation(
274+
description=(
275+
"Complete a lesson and return the post-mutation dashboard inline, so the "
276+
"client does not need a follow-up refresh."
277+
)
278+
)
279+
async def complete_lesson(
280+
self,
281+
repo_id: str,
282+
lesson_id: str,
283+
time_spent_seconds: int = 0,
284+
persona: Optional[str] = None,
285+
module_id: Optional[str] = None,
286+
) -> CompleteLessonResult:
287+
return await run_in_threadpool(
288+
_complete_lesson_sync, repo_id, lesson_id, time_spent_seconds, persona, module_id
289+
)
290+
291+
292+
schema = strawberry.Schema(query=Query, mutation=Mutation)

apps/api/src/main.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,14 @@ async def lifespan(app: FastAPI):
119119
app.include_router(learning.router, prefix="/api/learning", tags=["learning"])
120120
app.include_router(platform.router, prefix="/api/platform", tags=["platform"])
121121

122+
# GraphQL, additive rather than a migration: every REST route above still works.
123+
# Chat deliberately stays REST/SSE -- see src/api/graphql/schema.py.
124+
from strawberry.fastapi import GraphQLRouter # noqa: E402
125+
126+
from src.api.graphql.schema import schema as graphql_schema # noqa: E402
127+
128+
app.include_router(GraphQLRouter(graphql_schema), prefix="/graphql", tags=["graphql"])
129+
122130

123131
# Health check endpoint
124132
@app.get("/health")

0 commit comments

Comments
 (0)