Compare commits

...

2 commits

5 changed files with 143 additions and 14 deletions

View file

@ -110,6 +110,15 @@ describe("POST /api/v1/game/start", () => {
expect(res.status).toBe(400); expect(res.status).toBe(400);
expect(body.success).toBe(false); 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", () => { describe("POST /api/v1/game/answer", () => {

View file

@ -85,20 +85,22 @@ describe("createGameSession", () => {
} }
}); });
it("distractors are never the correct answer", async () => { it("correct answer appears exactly once even if getDistractors returns a duplicate", async () => {
mockGetDistractors.mockResolvedValueOnce([
"cane",
"wrong2",
"wrong3",
"wrong4",
"wrong5",
"wrong6",
]);
const session = await createGameSession(validRequest, store, "user-1"); const session = await createGameSession(validRequest, store, "user-1");
const question = session.questions[0]!;
const optionTexts = question.options.map((o) => o.text);
for (let i = 0; i < session.questions.length; i++) { expect(optionTexts.filter((t) => t === "cane")).toHaveLength(1);
const question = session.questions[i]!; expect(question.options).toHaveLength(4);
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 () => { it("sets the prompt from the source text", async () => {
@ -141,6 +143,14 @@ describe("createGameSession", () => {
createGameSession(validRequest, store, "user-1"), createGameSession(validRequest, store, "user-1"),
).rejects.toThrow("connection refused"); ).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", () => { describe("evaluateAnswer", () => {
@ -273,4 +283,12 @@ describe("evaluateAnswer", () => {
), ),
).rejects.toThrow("Game session not found"); ).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");
});
}); });

View file

@ -25,6 +25,10 @@ export const createGameSession = async (
request.rounds, request.rounds,
); );
if (terms.length === 0) {
throw new NotFoundError("No terms found for the given filters");
}
const answerKey = new Map<string, number>(); const answerKey = new Map<string, number>();
const questions: GameQuestion[] = await Promise.all( const questions: GameQuestion[] = await Promise.all(
@ -35,10 +39,13 @@ export const createGameSession = async (
request.target_language, request.target_language,
request.pos, request.pos,
request.difficulty, request.difficulty,
3, 6,
); );
const optionTexts = [term.targetText, ...distractorTexts]; const uniqueDistractors = distractorTexts.filter(
(t) => t !== term.targetText,
);
const optionTexts = [term.targetText, ...uniqueDistractors.slice(0, 3)];
const shuffledTexts = shuffleArray(optionTexts); const shuffledTexts = shuffleArray(optionTexts);
const correctOptionId = shuffledTexts.indexOf(term.targetText); const correctOptionId = shuffledTexts.indexOf(term.targetText);

View file

@ -0,0 +1,41 @@
# 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`

View file

@ -0,0 +1,54 @@
# 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`