updating documentation

README.md

@@ -30,7 +30,7 @@ Live at [lilastudy.com](https://lilastudy.com).
 
 ## Repository Structure
 
-```
+```tree
 lila/
 ├── apps/
 │   ├── api/        — Express backend
@@ -50,7 +50,7 @@ lila/
 
 Requests flow through a strict layered architecture:
 
-```
+```text
 HTTP Request → Router → Controller → Service → Model → Database
 ```
 
@@ -71,7 +71,7 @@ Vocabulary data is sourced from WordNet and the Open Multilingual Wordnet (OMW).
 
 ## API
 
-```
+```text
 POST /api/v1/game/start     — start a quiz session (auth required)
 POST /api/v1/game/answer    — submit an answer (auth required)
 GET  /api/v1/health         — health check (public)
@@ -90,7 +90,7 @@ Rooms are created via REST, then managed over WebSockets. Messages are typed via
 
 ## Infrastructure
 
-```
+```tree
 Internet → Caddy (HTTPS)
             ├── lilastudy.com      → web (nginx, static files)
             ├── api.lilastudy.com  → api (Express)

apps/api/src/controllers/gameController.test.ts

@@ -111,14 +111,25 @@ describe("POST /api/v1/game/start", () => {
     expect(body.success).toBe(false);
   });
 
-  it("returns 404 when no terms are found for the given filters", async () => {
+  it("returns 422 when no terms are found for the given filters", async () => {
     mockGetGameTerms.mockResolvedValue([]);
 
     const res = await request(app).post("/api/v1/game/start").send(validBody);
     const body = res.body as ErrorResponse;
-    expect(res.status).toBe(404);
+    expect(res.status).toBe(422);
     expect(body.success).toBe(false);
   });
+
+  it("returns a sanitised error message when the body is invalid", async () => {
+    const res = await request(app)
+      .post("/api/v1/game/start")
+      .send({ ...validBody, difficulty: "impossible" });
+    const body = res.body as ErrorResponse;
+    expect(res.status).toBe(400);
+    expect(body.error).toBe("Invalid game settings");
+    expect(body.error).not.toContain("Invalid literal value");
+    expect(body.error).not.toContain("Invalid enum value");
+  });
 });
 
 describe("POST /api/v1/game/answer", () => {
@@ -167,7 +178,7 @@ describe("POST /api/v1/game/answer", () => {
     expect(body.error).toContain("Game session not found");
   });
 
-  it("returns 404 when the question does not exist in the session", async () => {
+  it("returns 409 when the question does not exist in the session", async () => {
     const startRes = await request(app)
       .post("/api/v1/game/start")
       .send(validBody);
@@ -182,11 +193,11 @@ describe("POST /api/v1/game/answer", () => {
         selectedOptionId: 0,
       });
     const body = res.body as ErrorResponse;
-    expect(res.status).toBe(404);
+    expect(res.status).toBe(409);
     expect(body.success).toBe(false);
-    expect(body.error).toContain("Question not found");
+    expect(body.error).toContain("Question already answered");
   });
-
+  
   it("returns 400 when a field has an invalid value", async () => {
     const res = await request(app)
       .post("/api/v1/game/start")

apps/api/src/controllers/gameController.ts

@@ -14,7 +14,7 @@ export const createGameController = (store: GameSessionStore) => ({
     try {
       const gameSettings = GameRequestSchema.safeParse(req.body);
       if (!gameSettings.success) {
-        throw new ValidationError(gameSettings.error.message);
+        throw new ValidationError("Invalid game settings");
       }
       const gameQuestions = await createGameSession(
         gameSettings.data,
@@ -35,7 +35,7 @@ export const createGameController = (store: GameSessionStore) => ({
     try {
       const submission = AnswerSubmissionSchema.safeParse(req.body);
       if (!submission.success) {
-        throw new ValidationError(submission.error.message);
+        throw new ValidationError("Invalid answer submission");
       }
       const result = await evaluateAnswer(
         submission.data,

apps/api/src/errors/AppError.ts

@@ -25,3 +25,9 @@ export class ConflictError extends AppError {
     super(message, 409);
   }
 }
+
+export class UnprocessableEntityError extends AppError {
+  constructor(message: string) {
+    super(message, 422);
+  }
+}

apps/api/src/gameSessionStore/GameSessionStore.ts

@@ -1,4 +1,7 @@
-export type GameSessionData = { answers: Map<string, number>; userId: string };
+export type GameSessionData = {
+  answers: Map<string, { correctOptionId: number }>;
+  userId: string;
+};
 
 export interface GameSessionStore {
   create(
@@ -7,5 +10,6 @@ export interface GameSessionStore {
     ttlMs: number,
   ): Promise<void>;
   get(sessionId: string): Promise<GameSessionData | null>;
+  update(sessionId: string, data: GameSessionData): Promise<void>;
   delete(sessionId: string): Promise<void>;
 }

apps/api/src/gameSessionStore/InMemoryGameSessionStore.test.ts

@@ -14,14 +14,20 @@ describe("InMemoryGameSessionStore", () => {
   });
 
   it("returns session data after creation", async () => {
-    const data = { answers: new Map([["q1", 2]]), userId: "user-1" };
+    const data = {
+      answers: new Map([["q1", { correctOptionId: 2 }]]),
+      userId: "user-1",
+    };
     await store.create("session-1", data, 60_000);
     const result = await store.get("session-1");
     expect(result).toEqual(data);
   });
 
   it("returns null after the session is deleted", async () => {
-    const data = { answers: new Map([["q1", 2]]), userId: "user-1" };
+    const data = {
+      answers: new Map([["q1", { correctOptionId: 2 }]]),
+      userId: "user-1",
+    };
     await store.create("session-1", data, 60_000);
     await store.delete("session-1");
     const result = await store.get("session-1");
@@ -29,7 +35,10 @@ describe("InMemoryGameSessionStore", () => {
   });
 
   it("returns null after TTL expires", async () => {
-    const data = { answers: new Map([["q1", 2]]), userId: "user-1" };
+    const data = {
+      answers: new Map([["q1", { correctOptionId: 2 }]]),
+      userId: "user-1",
+    };
     await store.create("session-1", data, 1);
     await new Promise((resolve) => setTimeout(resolve, 10));
     const result = await store.get("session-1");
@@ -37,9 +46,30 @@ describe("InMemoryGameSessionStore", () => {
   });
 
   it("returns session data before TTL expires", async () => {
-    const data = { answers: new Map([["q1", 2]]), userId: "user-1" };
+    const data = {
+      answers: new Map([["q1", { correctOptionId: 2 }]]),
+      userId: "user-1",
+    };
     await store.create("session-1", data, 60_000);
     const result = await store.get("session-1");
     expect(result).not.toBeNull();
   });
+
+  it("update persists modified session data", async () => {
+    const data = {
+      answers: new Map([["q1", { correctOptionId: 2 }]]),
+      userId: "user-1",
+    };
+
+    await store.create("session-1", data, 60_000);
+
+    const updated = {
+      answers: new Map([["q2", { correctOptionId: 1 }]]),
+      userId: "user-1",
+    };
+    await store.update("session-1", updated);
+
+    const result = await store.get("session-1");
+    expect(result).toEqual(updated);
+  });
 });

apps/api/src/gameSessionStore/InMemoryGameSessionStore.ts

@@ -24,6 +24,13 @@ export class InMemoryGameSessionStore implements GameSessionStore {
     return Promise.resolve(entry.data);
   }
 
+  update(sessionId: string, data: GameSessionData): Promise<void> {
+    const entry = this.sessions.get(sessionId);
+    if (!entry) return Promise.resolve();
+    this.sessions.set(sessionId, { data, expiresAt: entry.expiresAt });
+    return Promise.resolve();
+  }
+
   delete(sessionId: string): Promise<void> {
     this.sessions.delete(sessionId);
     return Promise.resolve();

apps/api/src/services/gameService.test.ts

@@ -32,7 +32,14 @@ const fakeTerms = [
 beforeEach(() => {
   vi.clearAllMocks();
   mockGetGameTerms.mockResolvedValue(fakeTerms);
-  mockGetDistractors.mockResolvedValue(["wrong1", "wrong2", "wrong3"]);
+  mockGetDistractors.mockResolvedValue([
+    "wrong1",
+    "wrong2",
+    "wrong3",
+    "wrong4",
+    "wrong5",
+    "wrong6",
+  ]);
 });
 
 describe("createGameSession", () => {
@@ -151,6 +158,39 @@ describe("createGameSession", () => {
       createGameSession(validRequest, store, "user-1"),
     ).rejects.toThrow("db timeout");
   });
+
+  it("throws when fewer than 3 unique distractors remain after deduplication", async () => {
+    mockGetDistractors.mockResolvedValueOnce([
+      "cane",
+      "cane",
+      "cane",
+      "cane",
+      "cane",
+      "cane",
+    ]);
+
+    await expect(
+      createGameSession(validRequest, store, "user-1"),
+    ).rejects.toThrow("Not enough unique distractors");
+  });
+
+  it("duplicate distractors are deduplicated against each other", async () => {
+    mockGetDistractors.mockResolvedValueOnce([
+      "wrong1",
+      "wrong1",
+      "wrong1",
+      "wrong2",
+      "wrong3",
+      "wrong4",
+    ]);
+
+    const session = await createGameSession(validRequest, store, "user-1");
+    const question = session.questions[0]!;
+    const optionTexts = question.options.map((o) => o.text);
+
+    expect(new Set(optionTexts).size).toBe(4);
+    expect(question.options).toHaveLength(4);
+  });
 });
 
 describe("evaluateAnswer", () => {
@@ -215,7 +255,7 @@ describe("evaluateAnswer", () => {
     );
   });
 
-  it("throws NotFoundError for a non-existent question", async () => {
+  it("throws ConflictError for a non-existent question", async () => {
     const session = await createGameSession(validRequest, store, "user-1");
 
     const submission: AnswerSubmission = {
@@ -224,12 +264,12 @@ describe("evaluateAnswer", () => {
       selectedOptionId: 0,
     };
 
-    await expect(evaluateAnswer(submission, store, "user-1")).rejects.toThrow(
-      "Question not found",
-    );
+    await expect(
+      evaluateAnswer(submission, store, "user-1"),
+    ).rejects.toMatchObject({ statusCode: 409 });
   });
 
-  it("throws NotFoundError when the same question is submitted twice", async () => {
+  it("throws ConflictError when the same question is submitted twice", async () => {
     const session = await createGameSession(validRequest, store, "user-1");
     const question = session.questions[0]!;
 
@@ -253,7 +293,7 @@ describe("evaluateAnswer", () => {
         store,
         "user-1",
       ),
-    ).rejects.toThrow("Question not found");
+    ).rejects.toMatchObject({ statusCode: 409 });
   });
 
   it("deletes the session after the last question is answered", async () => {
@@ -284,11 +324,11 @@ describe("evaluateAnswer", () => {
     ).rejects.toThrow("Game session not found");
   });
 
-  it("throws NotFoundError when getGameTerms returns no terms", async () => {
+  it("throws UnprocessableEntityError when getGameTerms returns no terms", async () => {
     mockGetGameTerms.mockResolvedValue([]);
 
     await expect(
       createGameSession(validRequest, store, "user-1"),
-    ).rejects.toThrow("No terms found");
+    ).rejects.toMatchObject({ statusCode: 422 });
   });
 });

apps/api/src/services/gameService.ts

@@ -9,7 +9,11 @@ import type {
   AnswerResult,
 } from "@lila/shared";
 import type { GameSessionStore } from "../gameSessionStore/index.js";
-import { NotFoundError } from "../errors/AppError.js";
+import {
+  NotFoundError,
+  ConflictError,
+  UnprocessableEntityError,
+} from "../errors/AppError.js";
 import { shuffleArray } from "../lib/utils.js";
 
 export const createGameSession = async (
@@ -26,10 +30,10 @@ export const createGameSession = async (
   );
 
   if (terms.length === 0) {
-    throw new NotFoundError("No terms found for the given filters");
+    throw new UnprocessableEntityError("No terms found for the given filters");
   }
 
-  const answerKey = new Map<string, number>();
+  const answerKey = new Map<string, { correctOptionId: number }>();
 
   const questions: GameQuestion[] = await Promise.all(
     terms.map(async (term) => {
@@ -42,9 +46,16 @@ export const createGameSession = async (
         6,
       );
 
-      const uniqueDistractors = distractorTexts.filter(
-        (t) => t !== term.targetText,
-      );
+      const uniqueDistractors = [
+        ...new Set(distractorTexts.filter((t) => t !== term.targetText)),
+      ];
+
+      if (uniqueDistractors.length < 3) {
+        throw new Error(
+          `Not enough unique distractors for term: ${term.targetText}`,
+        );
+      }
+
       const optionTexts = [term.targetText, ...uniqueDistractors.slice(0, 3)];
       const shuffledTexts = shuffleArray(optionTexts);
       const correctOptionId = shuffledTexts.indexOf(term.targetText);
@@ -55,7 +66,7 @@ export const createGameSession = async (
       }));
 
       const questionId = randomUUID();
-      answerKey.set(questionId, correctOptionId);
+      answerKey.set(questionId, { correctOptionId });
 
       return {
         questionId,
@@ -83,22 +94,30 @@ export const evaluateAnswer = async (
     throw new NotFoundError(`Game session not found: ${submission.sessionId}`);
   }
 
-  const correctOptionId = session.answers.get(submission.questionId);
+  const answer = session.answers.get(submission.questionId);
 
-  if (correctOptionId === undefined) {
-    throw new NotFoundError(`Question not found: ${submission.questionId}`);
+  if (answer === undefined) {
+    throw new ConflictError(
+      `Question already answered: ${submission.questionId}`,
+    );
   }
 
-  session.answers.delete(submission.questionId);
+  const updatedAnswers = new Map(session.answers);
+  updatedAnswers.delete(submission.questionId);
 
-  if (session.answers.size === 0) {
+  if (updatedAnswers.size === 0) {
     await store.delete(submission.sessionId);
+  } else {
+    await store.update(submission.sessionId, {
+      answers: updatedAnswers,
+      userId: session.userId,
+    });
   }
 
   return {
     questionId: submission.questionId,
-    isCorrect: submission.selectedOptionId === correctOptionId,
-    correctOptionId,
+    isCorrect: submission.selectedOptionId === answer.correctOptionId,
+    correctOptionId: answer.correctOptionId,
     selectedOptionId: submission.selectedOptionId,
   };
 };

documentation/backlog.md

@@ -8,21 +8,21 @@ Labels: `[feature]` `[infra]` `[security]` `[ux]` `[debt]`
 
 Things that are actively in progress or should be picked up immediately. Mostly operational risk and the remaining phase 7 hardening work.
 
-- **Google OAuth publishing** `[infra]`
-  Only test users can currently log in via Google. Publish the OAuth consent screen so any Google user can sign in — requires branding verification in Google Cloud Console.
-
 - **Hetzner domain migration check** `[infra]`
   Verify whether the lilastudy.com domain needs to be migrated following a Hetzner DNS change. Check Hetzner dashboard for any pending migration notice.
 
-- **Conditionally register OAuth providers** `[debt]`
-  Better Auth logs warnings when social providers are registered without credentials (`Social provider google is missing clientId or clientSecret`). Instead of registering all providers unconditionally, only add a provider to the config when its credentials are present in the environment. Keeps local dev clean for contributors who don't have OAuth apps set up.
-
 ---
 
 ## next
 
 Clearly planned work, not yet started. No hard ordering — sequence based on what unblocks real users first.
 
+- **Batch distractor queries to eliminate N+1** `[debt]`
+  createGameSession calls getDistractors once per term in parallel — 3 queries for 3 rounds, 10 for 10. Each query does ORDER BY RANDOM() which can't use an index and gets slower as the translations table grows. Fix: add a getDistractorsForTerms(termIds[], ...) function to @lila/db that batches all distractor fetches into a single query and returns results grouped by term. The service distributes the results per question. Prerequisite: none. Blocked by: nothing, but coordinate with any ongoing @lila/db changes.
+
+- **Atomic session creation** `[debt]`
+  createGameSession reads from Postgres (getGameTerms, getDistractors) then writes to the session store (in-memory/Valkey). A crash between the two leaves the terms consumed with no session created — the user gets an error and retries, no data is corrupted, but the work is wasted. A true transaction boundary isn't achievable across two different systems (Postgres + Valkey have no shared coordinator). Options when revisiting: store sessions in Postgres instead of Valkey (full transactionality, higher latency), or accept the current behaviour and add retry logic on the client. Revisit after Valkey is in production and actual failure rates are observable.
+
 - **Guest / try-now flow** `[feature]`
   Allow users to play a quiz without signing in so they can see what the app offers before creating an account. Make auth middleware optional on game routes, add a "Try without account" button on the landing/login page.
 
@@ -63,6 +63,9 @@ Clearly planned work, not yet started. No hard ordering — sequence based on wh
 - **Tighten CSP to remove unsafe-inline** `[security]`
   Current script-src uses 'unsafe-inline' to accommodate framework-injected inline scripts (likely TanStack Router hydration). Tightening this would require nonce-based CSP, which needs server-rendered HTML or a Caddy layer that injects per-request nonces. Not urgent — pragmatic CSP with 'unsafe-inline' is mainstream for SPAs at this scale. Revisit if the app handles more sensitive data or grows a meaningful user base
 
+- **Publish Google OAuth consent screen** `[infra]`
+  App is currently in testing mode, which caps OAuth sign-ins at 100 users. Before hitting that limit, publish the consent screen in Google Cloud Console. Basic scopes (email, profile, openid) require no Google review — just fill in branding fields (app name, logo, support email, privacy policy URL) and click publish. Trigger: do this before reaching 80 users.
+
 ---
 
 ## later

documentation/roasts/gameService.md

@@ -1,366 +1,348 @@
-# `gameService.ts` — Code Review & Fixes
+# 🔥 GameService Roast: `apps/api/src/services/gameService.ts`
+
+> *"It works on my machine" is not a scalability strategy.*
+
+**Project:** lila — Vocabulary Trainer  
+**File Roasted:** `gameService.ts`  
+**Date:** $(date)  
+**Roaster:** Qwen3.6  
 
 ---
 
-## 1. Hardcoded singleton kills the abstraction
+## 📋 Executive Summary
 
-**Problem**
-
-A `GameSessionStore` interface exists, an `InMemoryGameSessionStore` implements it, and then the concrete class is immediately hardcoded as a module-level singleton. The interface is decorative — nothing can inject an alternative implementation without editing this file.
-
-```ts
-// ❌ current — store is unreachable from outside
-const gameSessionStore = new InMemoryGameSessionStore();
-
-export const createGameSession = async (request: GameRequest) => { ... };
-export const evaluateAnswer = async (submission: AnswerSubmission) => { ... };
-```
-
-**Fix — inject the store**
-
-Accept the store as a parameter (or use a factory). The simplest approach that requires no framework:
-
-```ts
-// ✅ inject the store
-export const createGameSession = async (
-  request: GameRequest,
-  store: GameSessionStore,
-): Promise<GameSession> => { ... };
-
-export const evaluateAnswer = async (
-  submission: AnswerSubmission,
-  store: GameSessionStore,
-): Promise<AnswerResult> => { ... };
-```
-
-The call site (controller) owns the store instance and passes it in. Tests can pass a fresh `InMemoryGameSessionStore` per test — no mocking required, no shared state.
-
-```ts
-// gameController.ts
-const store = new InMemoryGameSessionStore();
-
-// later, swap for ValKeyGameSessionStore with one line change
-```
+| Metric        | Score    | Notes                                                |
+| ------------- | -------- | ---------------------------------------------------- |
+| Code Quality  | 8/10     | Clean layering, good types, consistent style         |
+| Correctness   | 6/10     | Race condition + N+1 query are critical              |
+| Test Coverage | 7/10     | Good happy-path tests, missing concurrency tests     |
+| Scalability   | 5/10     | Will choke at ~100 concurrent users without fixes    |
+| **Overall**   | **7/10** | Solid foundation, but fix the footguns before launch |
 
 ---
 
-## 2. Sessions are never deleted — memory leak
+## 🚨 Critical Issues (Fix Before Production)
 
-**Problem**
+### 1. Race Condition: Lost Update in `evaluateAnswer`
 
-`GameSessionStore.delete()` is defined and implemented but never called. Every session ever created stays in the Map until the process restarts. Under real traffic this is a slow memory leak; under a spike it's a fast one.
+**Location:** `gameService.ts:45-58` + `InMemoryGameSessionStore.ts:update()`
 
-**Fix — delete after answer, or add a TTL**
+// Current flow (VULNERABLE):
+const session = await store.get(submission.sessionId);  // READ
+const updatedAnswers = new Map(session.answers);         // MODIFY (local copy)
+updatedAnswers.delete(submission.questionId);
+await store.update(submission.sessionId, { answers: updatedAnswers }); // WRITE
 
-The simplest fix: delete the session once the last question is answered. If partial completion is needed, add a TTL on creation instead.
+The Attack:
 
-```ts
-// ✅ option A — delete on answer
-export const evaluateAnswer = async (
-  submission: AnswerSubmission,
-  store: GameSessionStore,
-): Promise<AnswerResult> => {
-  const session = await store.get(submission.sessionId);
-  if (!session)
-    throw new NotFoundError(`Game session not found: ${submission.sessionId}`);
+    Client submits answer A and answer B for the same question (network retry, bug, or malice)
+    Both requests read the same session.answers Map (question still present)
+    Both delete the question from their local copy
+    Both write back → second write overwrites first
+    Result: One answer is silently lost, session state desyncs
 
-  const correctOptionId = session.answers.get(submission.questionId);
-  if (correctOptionId === undefined)
-    throw new NotFoundError(`Question not found: ${submission.questionId}`);
+Why Tests Missed It: Vitest runs tests synchronously. Race conditions require deliberate concurrency testing.
+Fix Options:
 
-  // delete answered question; delete session when all questions are answered
-  session.answers.delete(submission.questionId);
-  if (session.answers.size === 0) {
-    await store.delete(submission.sessionId);
-  }
+// Option A: Add atomic operation to store interface
+interface GameSessionStore {
+  deleteAnswer(sessionId: string, questionId: string): Promise<boolean>;
+}
 
+// Option B: Use Valkey Lua script for atomic read-modify-write
+// Option C: Optimistic locking with version numbers
+
+Priority: 🔴 CRITICAL — Data integrity issue
+2. N+1 Query: Database Performance Bomb
+Location: gameService.ts:24-26 + termModel.ts:getDistractors()
+
+// For each of N terms, we call getDistractors():
+const questions: GameQuestion[] = await Promise.all(
+  terms.map(async (term) => {
+    const distractorTexts = await getDistractors(term.termId, ...); // 🚩 N queries!
+  })
+);
+
+Impact Analysis:
+Rounds
+	
+DB Queries
+	
+At 50 concurrent users
+3
+	
+1 + 3 = 4
+	
+200 queries/min
+10
+	
+1 + 10 = 11
+	
+550 queries/min
+20
+	
+1 + 20 = 21
+	
+1,050 queries/min
+Each getDistractors() runs:
+
+SELECT text FROM terms 
+JOIN translations ON ... 
+WHERE pos = $1 AND difficulty = $2 AND term_id != $3 AND text != $4 
+ORDER BY RANDOM() LIMIT 6
+
+Fix: Batch Fetch Distractors
+
+// Fetch all distractors in ONE query
+const allDistractors = await db
+  .select({ termId: terms.id, text: translations.text })
+  .from(terms)
+  .innerJoin(translations, /* ... */)
+  .where(and(
+    eq(terms.pos, pos),
+    eq(translations.difficulty, difficulty),
+    inArray(terms.id, termIds), // Batch!
+  ))
+  .limit(DISTRACTOR_FETCH_COUNT * termIds.length);
+
+// Group by termId in JS, then slice to 3 unique distractors per term
+const distractorsByTerm = groupByTermId(allDistractors);
+
+Priority: 🔴 CRITICAL — Performance/scalability issue
+
+3. Error Handling Inconsistency
+Location: gameService.ts:33-36
+
+if (uniqueDistractors.length < 3) {
+  throw new Error(`Not enough unique distractors for term: ${term.targetText}`); // 🚩
+}
+
+Problem: Raw Error bypasses your errorHandler middleware:
+
+    No HTTP status mapping (defaults to 500)
+    No structured logging
+    Inconsistent API responses
+
+Fix:
+import { UnprocessableEntityError } from "../errors/AppError.js";
+
+if (uniqueDistractors.length < 3) {
+  logger.warn({ termId: term.termId, uniqueCount: uniqueDistractors.length }, 
+              "insufficient_distractors");
+  throw new UnprocessableEntityError(
+    `Not enough unique distractors for term: ${term.targetText}`
+  );
+}
+Priority: 🟡 HIGH — Observability & UX issue
+⚠️ High-Severity Smells
+4. Code Duplication: Singleplayer vs Multiplayer
+Compare: gameService.ts vs multiplayerGameService.ts
+// gameService.ts
+const optionTexts = [term.targetText, ...uniqueDistractors.slice(0, 3)];
+const shuffledTexts = shuffleArray(optionTexts);
+const correctOptionId = shuffledTexts.indexOf(term.targetText);
+
+// multiplayerGameService.ts (lines 35-45)
+const optionTexts = [correctAnswer.targetText, ...distractorTexts];
+const shuffledTexts = shuffle(optionTexts); // Different function, same logic!
+const correctOptionId = shuffledTexts.indexOf(correctAnswer.targetText);
+
+Risks:
+
+    Fix shuffle bias in one place, forget the other
+    Add new option type (e.g., etymology hint), update one service only
+    Harder to test core game logic in isolation
+
+Fix: Extract pure function to @lila/shared or new @lila/game-logic:
+
+// packages/shared/src/game-logic.ts
+export const buildQuestionOptions = (
+  correctAnswer: string,
+  distractors: string[],
+  optionCount: number = 4
+): { options: AnswerOption[]; correctOptionId: number } => {
+  const uniqueDistractors = [...new Set(distractors.filter(d => d !== correctAnswer))];
+  const optionTexts = [correctAnswer, ...uniqueDistractors.slice(0, optionCount - 1)];
+  const shuffled = shuffleSecure(optionTexts);
+  const correctOptionId = shuffled.indexOf(correctAnswer);
+  
   return {
-    questionId: submission.questionId,
-    isCorrect: submission.selectedOptionId === correctOptionId,
-    correctOptionId,
-    selectedOptionId: submission.selectedOptionId,
+    options: shuffled.map((text, idx) => ({ optionId: idx, text })),
+    correctOptionId
   };
 };
-```
 
-```ts
-// ✅ option B — TTL in InMemoryGameSessionStore
-export class InMemoryGameSessionStore implements GameSessionStore {
-  private sessions = new Map<
-    string,
-    { data: GameSessionData; expiresAt: number }
-  >();
-  private readonly ttlMs: number;
+Priority: 🟡 HIGH — Maintainability issue
+5. Shuffle Bias: Math.random() Trap
+Location: utils.ts:shuffleArray() + multiplayerGameService.ts:shuffle()
 
-  constructor(ttlMs = 30 * 60 * 1000) {
-    // 30 minutes default
-    this.ttlMs = ttlMs;
+export const shuffleArray = <T>(array: T[]): T[] => {
+  for (let i = result.length - 1; i > 0; i--) {
+    const j = Math.floor(Math.random() * (i + 1)); // 🚩 Modulo bias + non-crypto RNG
+    // ...
   }
+};
 
-  create(sessionId: string, data: GameSessionData): Promise<void> {
-    this.sessions.set(sessionId, { data, expiresAt: Date.now() + this.ttlMs });
-    return Promise.resolve();
+The Math:
+
+    Math.random() has ~53 bits of entropy (fine for vocab)
+    Math.floor(rand * n) has modulo bias when n isn't a power of 2
+    For n=4: bias is ~0.01% (tiny, but non-zero)
+
+When It Matters:
+
+    Competitive leaderboards ("option 0 is correct 26% of the time")
+    Achievement systems based on answer patterns
+    Security-sensitive features (not applicable here, but principle matters)
+
+Fix (if needed):
+import { randomBytes } from "crypto";
+
+const shuffleSecure = <T>(array: T[]): T[] => {
+  const result = [...array];
+  for (let i = result.length - 1; i > 0; i--) {
+    // Use crypto.getRandomValues for better randomness
+    const rand = randomBytes(4).readUInt32LE(0);
+    const j = rand % (i + 1);
+    [result[i], result[j]] = [result[j], result[i]];
   }
+  return result;
+};
 
-  get(sessionId: string): Promise<GameSessionData | null> {
-    const entry = this.sessions.get(sessionId);
-    if (!entry) return Promise.resolve(null);
-    if (Date.now() > entry.expiresAt) {
-      this.sessions.delete(sessionId);
-      return Promise.resolve(null);
-    }
-    return Promise.resolve(entry.data);
-  }
+Priority: 🟢 LOW — Document tradeoff and move on for now
 
-  delete(sessionId: string): Promise<void> {
-    this.sessions.delete(sessionId);
-    return Promise.resolve();
-  }
-}
-```
+6. Test Coverage Gaps
+File: gameService.test.ts
+✅ Well Tested:
 
----
+    Happy path: session creation, answer evaluation
+    Edge cases: duplicate distractors, empty results, invalid inputs
+    Error propagation from DB layer
 
-**Problem**
+❌ Missing Tests:
 
-`GameRequest.rounds` is typed as `string` in `@lila/shared`, forcing the service to cast it every time:
-
-```ts
-// ❌ why is a round count a string?
-Number(request.rounds);
-```
-
-**Fix — fix the schema in `@lila/shared`**
-
-```ts
-// ✅ in packages/shared
-export const GameRequestSchema = z.object({
-  source_language: z.string(),
-  target_language: z.string(),
-  pos: z.string(),
-  difficulty: z.string(),
-  rounds: z.coerce.number().int().min(1).max(50), // coerce handles form inputs, validates range
-});
-
-export type GameRequest = z.infer<typeof GameRequestSchema>;
-```
-
-The `z.coerce.number()` handles the case where the value arrives as a string from a query param or form — Zod does the conversion at the boundary so the rest of the system never sees a string.
-
----
-
-**Problem**
-
-The variable holds `terms` — word pairs fetched from the database. Calling them `correctAnswers` jumps ahead semantically; they only become "correct answers" once options are constructed around them.
-
-```ts
-// ❌ these are terms, not answers yet
-const correctAnswers = await getGameTerms(...);
-```
-
-**Fix**
-
-```ts
-// ✅
-const terms = await getGameTerms(...);
-
-// and inside the map:
-terms.map(async (term) => {
-  const distractorTexts = await getDistractors(
-    term.termId,
-    term.targetText,
-    ...
-  );
-  ...
-  const correctOptionId = shuffledTexts.indexOf(term.targetText);
-  ...
-});
-```
-
----
-
-## 6. Tautological test: `"distractors are never the correct answer"`
-
-**Problem**
-
-The test filters the correct answer out of the options array, then asserts the remaining items are not the correct answer. It is testing that `Array.filter` works.
-
-```ts
-// ❌ this cannot fail
-it("distractors are never the correct answer", async () => {
-  const distractorTexts = question.options
-    .map((o) => o.text)
-    .filter((t) => t !== correctText); // removes correct answer...
-
-  for (const text of distractorTexts) {
-    expect(text).not.toBe(correctText); // ...then checks they're not the correct answer
-  }
-});
-```
-
-**What to actually test**
-
-The real concern is that `getDistractors` doesn't return the target word. Test that the service handles it correctly if it does:
-
-```ts
-// ✅ test that the correct answer appears exactly once even if a distractor collides
-it("correct answer appears exactly once in options even if distractor matches", async () => {
-  // simulate getDistractors returning the correct answer as one of the distractors
-  mockGetDistractors.mockResolvedValueOnce(["cane", "wrong2", "wrong3"]);
-
-  const session = await createGameSession(
-    validRequest,
-    new InMemoryGameSessionStore(),
-  );
+// 1. Concurrency test (race condition)
+it("rejects duplicate answers for same question under concurrent load", async () => {
+  const session = await createGameSession(validRequest, store, "user-1");
   const question = session.questions[0]!;
-  const optionTexts = question.options.map((o) => o.text);
-
-  // "cane" should only appear once regardless of the duplicate from getDistractors
-  expect(optionTexts.filter((t) => t === "cane")).toHaveLength(1);
-  expect(question.options).toHaveLength(4);
+  
+  // Submit two answers simultaneously
+  const [result1, result2] = await Promise.allSettled([
+    evaluateAnswer({ sessionId, questionId, selectedOptionId: 0 }, store, "user-1"),
+    evaluateAnswer({ sessionId, questionId, selectedOptionId: 1 }, store, "user-1"),
+  ]);
+  
+  // Exactly one should succeed, one should throw ConflictError
+  expect([result1, result2].filter(r => r.status === "fulfilled")).toHaveLength(1);
 });
-```
 
-> **Note:** the current implementation doesn't actually handle this case — a duplicate distractor would produce a 4-option list where the correct answer appears twice and one distractor slot is wasted. Worth fixing in `createGameSession` alongside the test.
-
----
-
-## 7. Store not reset between tests
-
-**Problem**
-
-`beforeEach` calls `vi.clearAllMocks()` which resets mock functions, but the `gameSessionStore` module-level singleton is never cleared. Ghost sessions from earlier tests persist for the entire test run.
-
-It doesn't bite today because each session gets a unique UUID and tests don't share IDs — but it's one non-UUID lookup away from a very confusing afternoon.
-
-**Fix — a consequence of fixing issue #1**
-
-Once the store is injected rather than module-level, each test creates its own instance:
-
-```ts
-// ✅ no shared state, no ghost sessions
-describe("evaluateAnswer", () => {
-  it("returns isCorrect: true for correct option", async () => {
-    const store = new InMemoryGameSessionStore();
-    const session = await createGameSession(validRequest, store);
-    ...
-    const result = await evaluateAnswer({ ... }, store);
-    ...
-  });
+// 2. TTL expiration test
+it("deletes session after TTL expires", async () => {
+  vi.useFakeTimers();
+  const session = await createGameSession(validRequest, store, "user-1");
+  
+  vi.advanceTimersByTime(31 * 60 * 1000); // 31 minutes
+  
+  await expect(store.get(session.sessionId)).resolves.toBeNull();
 });
-```
 
-No `beforeEach` cleanup needed — the store simply doesn't outlive the test that created it.
+// 3. Distractor fallback strategy test
+it("uses fallback when <3 unique distractors available", async () => {
+  mockGetDistractors.mockResolvedValue(["same", "same", "same", "same"]);
+  // Should either: (a) fetch from broader pool, or (b) reduce rounds gracefully
+});
 
----
+Priority: 🟡 HIGH — Prevents regression on critical fixes
+🧼 Code Quality Nitpicks
+7. Magic Numbers
 
-## 8. No answer replay protection
+// gameService.ts:52
+await store.create(sessionId, {...}, 30 * 60 * 1000); // What is this?
 
-**Problem**
+// termModel.ts:65
+.limit(count); // count=6, but why?
 
-`evaluateAnswer` can be called multiple times with the same `questionId`. The
-service will evaluate it every time. In multiplayer this could be abused to
-farm points or desync state.
+// shared/schemas/game.ts:15
+optionId: z.number().int().min(0).max(3), // Why 4 options?
 
-**Fix — delete the question from the answer key after first evaluation**
+Fix: Centralize in @lila/shared/constants.ts:
 
-```ts
-// ✅ inside evaluateAnswer, after retrieving correctOptionId
-session.answers.delete(submission.questionId);
+export const GAME_SESSION_TTL_MS = 30 * 60 * 1000;
+export const DISTRACTOR_FETCH_COUNT = 6;
+export const GAME_OPTION_COUNT = 4;
+export const MIN_UNIQUE_DISTRACTORS = 3;
 
-if (submission.selectedOptionId !== correctOptionId) {
-  // already removed — can't retry
+8. Mutable Reference Leakage
+Location: InMemoryGameSessionStore.ts:get()
+
+get(sessionId: string): Promise<GameSessionData | null> {
+  return Promise.resolve(entry.data); // 🚩 Returns mutable reference to internal state
 }
-```
 
-Once the question key is deleted, a second submission hits the
-`correctOptionId === undefined` branch and throws `NotFoundError`. One shot
-per question.
+Risk: Any code that does session.answers.delete(...) mutates the store's internal Map directly.
+Fix:
 
----
+// Option A: Deep clone (simple, works for this data shape)
+return Promise.resolve(structuredClone(entry.data));
 
-## 9. No ownership check in `evaluateAnswer`
+// Option B: Return readonly view (TypeScript-only protection)
+return Promise.resolve(entry.data as Readonly<GameSessionData>);
 
-**Problem**
+// Option C: Use immutable data structures (overkill for now)
 
-The service accepts any `sessionId` without verifying it belongs to the
-requesting user. If auth middleware doesn't tie sessions to users at a higher
-layer, Alice can submit answers for Bob's session by guessing or intercepting
-his `sessionId`.
+9. Zero Observability
+Problem: No logging, no metrics. You're flying blind in production.
+Minimal Fix (5 minutes):
 
-**Fix — store `userId` alongside the session and assert it on retrieval**
 
-```ts
-// GameSessionStore.ts
-export type GameSessionData = { answers: Map<string, number>; userId: string };
 
-// evaluateAnswer
-const session = await store.get(submission.sessionId);
-
-if (!session) throw new NotFoundError(`Game session not found`);
-if (session.userId !== requestingUserId)
-  throw new NotFoundError(`Game session not found`);
-//                                        ^^^ same error — don't confirm the session exists to the wrong user
-```
-
-Pass `requestingUserId` in from the controller, where it's already available
-via auth middleware.
-
----
-
-## 10. No test for empty `getGameTerms` result
-
-**Problem**
-
-If the database returns zero terms (no words match the difficulty/language/pos
-filter), `createGameSession` happily returns a session with an empty
-`questions` array. The frontend receives it, tries to render question 1, and
-crashes. The user sees nothing useful.
-
-**Fix — guard in the service and add a test**
-
-```ts
-// ✅ inside createGameSession, after fetching terms
-if (terms.length === 0) {
-  throw new AppError("No terms found for the given filters", 404);
-}
-```
-
-```ts
-// ✅ test
-it("throws when getGameTerms returns no terms", async () => {
-  mockGetGameTerms.mockResolvedValue([]);
-
-  await expect(
-    createGameSession(validRequest, new InMemoryGameSessionStore()),
-  ).rejects.toThrow("No terms found");
+// apps/api/src/lib/logger.ts
+import pino from "pino";
+export const logger = pino({ 
+  level: process.env.LOG_LEVEL || "info",
+  transport: process.env.NODE_ENV === "production" 
+    ? { target: "pino-pretty" } 
+    : undefined 
 });
-```
 
----
+// In gameService.ts:
+import { logger } from "../lib/logger.js";
 
-## 11. No test for `getDistractors` rejection
+logger.info(
+  { userId, sourceLang, targetLang, termCount: terms.length },
+  "game_session_created"
+);
 
-**Problem**
+logger.debug(
+  { sessionId, questionId, isCorrect, responseTimeMs },
+  "answer_evaluated"
+);
 
-`createGameSession` uses `Promise.all` over the terms array. If
-`getDistractors` rejects for any single term, the entire `Promise.all` rejects
-— no session is created, no partial recovery, the user gets a 500 with
-"connection refused" leaking through.
+Bonus: Export a Prometheus histogram for game_service_duration_seconds.
 
-**Fix — test the failure path and consider a fallback**
+10. ORDER BY RANDOM() Time Bomb
+Location: termModel.ts:getGameTerms() + getDistractors()
 
-```ts
-// ✅ test
-it("propagates getDistractors failure", async () => {
-  mockGetDistractors.mockRejectedValue(new Error("db timeout"));
+.orderBy(sql`RANDOM()`) // 🚩 Fine for 10k rows, slow for 1M
 
-  await expect(
-    createGameSession(validRequest, new InMemoryGameSessionStore()),
-  ).rejects.toThrow("db timeout");
-});
-```
+The Comment Admits It:
 
-For resilience, consider catching per-term distractor failures and falling back
-to random terms from the already-fetched set rather than collapsing the whole
-session.
+// TODO(post-mvp): ORDER BY RANDOM() sorts the entire filtered result set...
+
+Reality Check: "Post-MVP" never comes without a ticket.
+Fix Options:
+
+-- Option A: Pre-computed random_seed column (updated nightly)
+WHERE ... AND random_seed >= random() 
+ORDER BY random_seed 
+LIMIT $1
+
+-- Option B: TABLESAMPLE for approximate sampling (Postgres 9.5+)
+FROM terms TABLESAMPLE SYSTEM(10) 
+WHERE ... 
+LIMIT $1
+
+-- Option C: Random offset (simple, but still scans)
+OFFSET floor(random() * (SELECT count(*) FROM terms WHERE ...))
+
+Action: Add a ticket to documentation/tickets/t00009.md now.