diff --git a/apps/api/src/controllers/gameController.test.ts b/apps/api/src/controllers/gameController.test.ts index d8115bd..cfbe065 100644 --- a/apps/api/src/controllers/gameController.test.ts +++ b/apps/api/src/controllers/gameController.test.ts @@ -110,15 +110,6 @@ describe("POST /api/v1/game/start", () => { expect(res.status).toBe(400); expect(body.success).toBe(false); }); - - it("returns 404 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(body.success).toBe(false); - }); }); describe("POST /api/v1/game/answer", () => { diff --git a/apps/api/src/services/gameService.test.ts b/apps/api/src/services/gameService.test.ts index d5b56de..65d21d7 100644 --- a/apps/api/src/services/gameService.test.ts +++ b/apps/api/src/services/gameService.test.ts @@ -85,22 +85,20 @@ describe("createGameSession", () => { } }); - it("correct answer appears exactly once even if getDistractors returns a duplicate", async () => { - mockGetDistractors.mockResolvedValueOnce([ - "cane", - "wrong2", - "wrong3", - "wrong4", - "wrong5", - "wrong6", - ]); - + it("distractors are never the correct answer", async () => { const session = await createGameSession(validRequest, store, "user-1"); - const question = session.questions[0]!; - const optionTexts = question.options.map((o) => o.text); - expect(optionTexts.filter((t) => t === "cane")).toHaveLength(1); - expect(question.options).toHaveLength(4); + for (let i = 0; i < session.questions.length; i++) { + const question = session.questions[i]!; + const correctText = fakeTerms[i]!.targetText; + const distractorTexts = question.options + .map((o) => o.text) + .filter((t) => t !== correctText); + + for (const text of distractorTexts) { + expect(text).not.toBe(correctText); + } + } }); it("sets the prompt from the source text", async () => { @@ -143,14 +141,6 @@ describe("createGameSession", () => { createGameSession(validRequest, store, "user-1"), ).rejects.toThrow("connection refused"); }); - - it("propagates getDistractors failure", async () => { - mockGetDistractors.mockRejectedValue(new Error("db timeout")); - - await expect( - createGameSession(validRequest, store, "user-1"), - ).rejects.toThrow("db timeout"); - }); }); describe("evaluateAnswer", () => { @@ -283,12 +273,4 @@ describe("evaluateAnswer", () => { ), ).rejects.toThrow("Game session not found"); }); - - it("throws NotFoundError when getGameTerms returns no terms", async () => { - mockGetGameTerms.mockResolvedValue([]); - - await expect( - createGameSession(validRequest, store, "user-1"), - ).rejects.toThrow("No terms found"); - }); }); diff --git a/apps/api/src/services/gameService.ts b/apps/api/src/services/gameService.ts index a64b4e7..b892986 100644 --- a/apps/api/src/services/gameService.ts +++ b/apps/api/src/services/gameService.ts @@ -25,10 +25,6 @@ export const createGameSession = async ( request.rounds, ); - if (terms.length === 0) { - throw new NotFoundError("No terms found for the given filters"); - } - const answerKey = new Map(); const questions: GameQuestion[] = await Promise.all( @@ -39,13 +35,10 @@ export const createGameSession = async ( request.target_language, request.pos, request.difficulty, - 6, + 3, ); - const uniqueDistractors = distractorTexts.filter( - (t) => t !== term.targetText, - ); - const optionTexts = [term.targetText, ...uniqueDistractors.slice(0, 3)]; + const optionTexts = [term.targetText, ...distractorTexts]; const shuffledTexts = shuffleArray(optionTexts); const correctOptionId = shuffledTexts.indexOf(term.targetText); diff --git a/documentation/tickets/t00007.md b/documentation/tickets/t00007.md deleted file mode 100644 index 5469049..0000000 --- a/documentation/tickets/t00007.md +++ /dev/null @@ -1,41 +0,0 @@ -# feat: guard against empty terms in createGameSession - -## Problem - -If `getGameTerms` returned an empty array — no vocabulary data matched the requested language, difficulty, and part of speech combination — `createGameSession` would create a session with zero questions and return it. The frontend would receive an empty `questions` array, attempt to render the first question, find nothing, and crash with no useful error message shown to the user. - -## Options considered - -### Option A — `NotFoundError` (404) ✅ - -Throw when `terms.length === 0` before any session is created. The combination of filters yielded no data — that's a "not found" situation. - -Chosen because: the request is technically valid (all filter values are recognised), but the combination has no matching data. 404 is the correct semantic response. - -### Option B — `ValidationError` (400) - -Treat empty results as a bad request. - -Rejected because: the client sent valid input. The problem is missing data, not invalid input. 400 would be misleading. - -## Solution - -Added a guard in `createGameSession` immediately after `getGameTerms`: - -```ts -if (terms.length === 0) { - throw new NotFoundError("No terms found for the given filters"); -} -``` - -The error propagates through the controller's `try/catch` to the error handler, which returns a clean 404 response. No session is created. - -## Files changed - -- `apps/api/src/services/gameService.ts` — empty terms guard added -- `apps/api/src/services/gameService.test.ts` — pinning test added -- `apps/api/src/controllers/gameController.test.ts` — pinning test added at HTTP layer - -## Commit - -`feat: guard against empty terms in createGameSession` diff --git a/documentation/tickets/t00008.md b/documentation/tickets/t00008.md deleted file mode 100644 index b171abc..0000000 --- a/documentation/tickets/t00008.md +++ /dev/null @@ -1,54 +0,0 @@ -# fix: deduplicate distractors, replace tautological test - -## Problem - -Two issues in `createGameSession` and its test suite: - -1. If `getDistractors` returned the correct answer as one of the distractors, `createGameSession` would include it in the options array without filtering it out. `indexOf` would then find the first occurrence, which might not be the one intended as the correct answer — producing a question where the correct answer appears twice and the stored `correctOptionId` is wrong. - -2. The test `"distractors are never the correct answer"` was tautological — it filtered the correct answer out of the options array, then asserted the remaining items were not the correct answer. It was testing that `Array.filter()` works. It could never fail. - -## Options considered - -### Option A — Filter duplicates after fetching, request extra distractors as buffer ✅ - -Filter out any distractor that matches the correct answer after fetching. Request 6 distractors instead of 3 to ensure enough remain after deduplication. Take the first 3 valid ones with `slice(0, 3)`. - -Chosen because: deduplication at the service layer is the right place — `getDistractors` shouldn't need to know what the correct answer is. Requesting extra provides a buffer against collisions. - -### Option B — Fix `getDistractors` to never return the correct answer - -Add a NOT filter in the database query. - -Not chosen for this ticket — the database query is in `@lila/db` and is a separate concern. The service layer should be defensive regardless of what the model layer returns. - -## Solution - -- Filter distractors against the correct answer before building options: - - ```ts - const uniqueDistractors = distractorTexts.filter((t) => t !== term.targetText); - const optionTexts = [term.targetText, ...uniqueDistractors.slice(0, 3)]; - ``` - -- Request 6 distractors instead of 3 to account for potential duplicates -- Replaced tautological test with a test that actually exercises the duplicate case: - - ```ts - it("correct answer appears exactly once even if getDistractors returns a duplicate", ...) - ``` - -- Added distractor failure propagation test: - - ```ts - it("propagates getDistractors failure", ...) - ``` - -## Files changed - -- `apps/api/src/services/gameService.ts` — deduplication logic, distractor count increased to 6 -- `apps/api/src/services/gameService.test.ts` — tautological test replaced, failure test added - -## Commit - -`fix: deduplicate distractors, replace tautological test, add distractor failure test`