Coverage for server / services / common / question_bank.py: 96%

70 statements  

« prev     ^ index     » next       coverage.py v7.13.4, created at 2026-10-04 09:33 +0000

1"""Resolving an assignment's questions from the live question banks. 

2 

3WHY THIS MODULE EXISTS 

4---------------------- 

5Question documents live in three places, checked in order: 

6 

71. ``teacher_questionbank`` — the live bank (teacher-authored items) 

82. ``global_questionbank`` — staff-authored items, in the ADMIN-STAFF database 

93. ``question_collection`` — a legacy collection that no longer exists on any 

10 EruditionTX database; kept as a last-resort lookup for historical ids only 

11 

12``server/services/student/student_assignment.py`` already resolved questions this 

13way, but ``assignment_view_fetch`` / ``assignment_review_fetch`` in 

14``server/services/common/assignments.py`` read ``question_collection`` DIRECTLY — 

15so they answered 200 with ``"questions": []`` for every assignment, however many 

16questions it actually held. The lookup lives here now so there is one 

17implementation rather than a third copy. 

18 

19Developer: Allan Ninal 

20Date: 2026-09-23 

21""" 

22 

23import json 

24 

25from bson.objectid import ObjectId 

26 

27from server.connection.database import db, staff_admin_db 

28from server.utilities import model_parser 

29 

30# Defense-in-depth: bound the cross-bank $in fan-out. Assignments cap at 100 

31# questions, so this never bites a legitimate assignment — it only guards 

32# against a corrupt or abusive oversized id set. 

33MAX_QUESTION_FETCH = 500 

34 

35 

36def question_object_ids(assignment) -> list: 

37 """The ObjectIds of an assignment's questions, in the assignment's own order. 

38 

39 ``Assignment.questions`` is a ``List[Union[QuestionModel, str]]`` — each item 

40 is a QuestionModel (``.id`` is a PydanticObjectId), a ``{"id": ...}`` dict, or 

41 a bare id string. There is no ``question_ids`` field on the model; code that 

42 referenced one raised AttributeError and 500'd. 

43 """ 

44 questions = ( 

45 assignment.get("questions", []) 

46 if isinstance(assignment, dict) 

47 else (getattr(assignment, "questions", None) or []) 

48 ) 

49 ids = [] 

50 for entry in questions: 

51 raw = ( 

52 entry.get("id") if isinstance(entry, dict) else getattr(entry, "id", entry) 

53 ) 

54 if raw is None: 

55 continue 

56 if isinstance(raw, ObjectId): 

57 ids.append(raw) 

58 elif ObjectId.is_valid(str(raw)): 

59 ids.append(ObjectId(str(raw))) 

60 return ids 

61 

62 

63async def fetch_questions_by_ids(question_ids) -> dict: 

64 """Map ``str(question id) -> question document``, across all three banks. 

65 

66 Ids that exist in none of them are simply absent from the map: a question a 

67 teacher deleted must not break the whole assignment view. 

68 """ 

69 if not question_ids: 

70 return {} 

71 

72 if len(question_ids) > MAX_QUESTION_FETCH: 

73 print( 

74 f"[question_bank] question_ids count {len(question_ids)} exceeds cap " 

75 f"{MAX_QUESTION_FETCH}; truncating cross-bank fetch." 

76 ) 

77 question_ids = question_ids[:MAX_QUESTION_FETCH] 

78 

79 question_map = {} 

80 

81 results = ( 

82 await db["teacher_questionbank"] 

83 .aggregate( 

84 [ 

85 {"$match": {"_id": {"$in": question_ids}}}, 

86 {"$addFields": {"_id_str": {"$toString": "$_id"}}}, 

87 ] 

88 ) 

89 .to_list(length=len(question_ids)) 

90 ) 

91 for q in results: 

92 question_map[str(q["_id"])] = q 

93 

94 missing = [qid for qid in question_ids if str(qid) not in question_map] 

95 

96 # The GLOBAL bank lives in the admin-staff database, not this service's own. 

97 if missing and staff_admin_db is not None: 

98 global_results = ( 

99 await staff_admin_db["global_questionbank"] 

100 .aggregate( 

101 [ 

102 {"$match": {"_id": {"$in": missing}}}, 

103 {"$addFields": {"_id_str": {"$toString": "$_id"}}}, 

104 ] 

105 ) 

106 .to_list(length=len(missing)) 

107 ) 

108 for q in global_results: 

109 question_map[str(q["_id"])] = q 

110 missing = [qid for qid in question_ids if str(qid) not in question_map] 

111 

112 if missing: 

113 legacy_results = ( 

114 await db["question_collection"] 

115 .aggregate( 

116 [ 

117 {"$match": {"_id": {"$in": missing}}}, 

118 {"$addFields": {"_id_str": {"$toString": "$_id"}}}, 

119 ] 

120 ) 

121 .to_list(length=len(missing)) 

122 ) 

123 for q in legacy_results: 

124 question_map[str(q["_id"])] = q 

125 

126 return question_map 

127 

128 

129def project_question(document: dict, points_override=None) -> dict: 

130 """One question document, shaped for an assignment response. 

131 

132 The same field set the student assignment-taking path returns 

133 (``StudentAssignmentsService._prepare_questions``), minus the shuffling, 

134 which belongs to taking an attempt rather than viewing an assignment. 

135 

136 `points_override` (from the assignment's `question_points_overrides`) wins 

137 over the bank document's own `points`, so a teacher's per-assignment point 

138 edit is reflected without mutating the shared question document. 

139 """ 

140 return { 

141 "_id": document["_id"], 

142 "points": ( 

143 points_override if points_override is not None else document.get("points") 

144 ), 

145 "choices": document.get("choices"), 

146 "question": document.get("question"), 

147 "questionType": document.get("questionType"), 

148 "correctAnswer": document.get("correctAnswer"), 

149 "category": document.get("category"), 

150 "groups": document.get("groups"), 

151 "rows": document.get("rows"), 

152 "rowHeaderLabel": document.get("rowHeaderLabel"), 

153 } 

154 

155 

156async def resolve_assignment_questions(assignment) -> list: 

157 """An assignment's questions, resolved and projected, in the assignment's order. 

158 

159 Ids missing from every bank are skipped rather than raising — one deleted 

160 question must not take the whole response down with it. 

161 """ 

162 ids = question_object_ids(assignment) 

163 question_map = await fetch_questions_by_ids(ids) 

164 

165 overrides = ( 

166 assignment.get("question_points_overrides") 

167 if isinstance(assignment, dict) 

168 else getattr(assignment, "question_points_overrides", None) 

169 ) or {} 

170 

171 resolved = [] 

172 for qid in ids: 

173 document = question_map.get(str(qid)) 

174 if document is None: 

175 continue 

176 resolved.append(project_question(document, overrides.get(str(qid)))) 

177 return resolved 

178 

179 

180def id_match_forms(raw_id) -> list: 

181 """The same id in both shapes Mongo may be holding it in. 

182 

183 Ownership columns here are inconsistent by history — `createdBy` on questions 

184 and `created_by` on assignments are a string on most rows and an ObjectId on 

185 thousands of older ones. A filter that matches only one shape silently misses 

186 the other. 

187 """ 

188 forms = [str(raw_id)] 

189 if ObjectId.is_valid(str(raw_id)): 

190 forms.append(ObjectId(str(raw_id))) 

191 return forms 

192 

193 

194def _json_safe(document: dict) -> dict: 

195 """A question document FastAPI can actually serialise. 

196 

197 Added by Allan Ninal — 2026-09-23 (EI-T115) 

198 WHAT: run the raw Mongo document through the house JSONEncoder before it 

199 leaves this module. 

200 WHY: a real question carries `_id` and `createdBy` as **ObjectId** and 

201 `createdDate` as a datetime. The adaptive routes return the document 

202 straight to the client, and FastAPI's jsonable_encoder cannot handle a 

203 bare ObjectId — it died with 

204 `TypeError: 'ObjectId' object is not iterable` AFTER the picker had 

205 correctly found a question, so the endpoint still answered 500. 

206 Doing it here covers both service copies and any future caller. 

207 

208 My own picker tests missed this because they used simplified fixtures 

209 with no ObjectId fields; test_adaptive_question_picker.py now builds a 

210 document with the real field types. 

211 """ 

212 return json.loads(model_parser.JSONEncoder().encode(document)) 

213 

214 

215async def pick_adaptive_question( 

216 *, difficulty: str, classification: str, exclude_ids: list, creator_id 

217) -> dict | None: 

218 """One unseen question at `difficulty` and `classification`, or None. 

219 

220 WHERE IT LOOKS, and why only there 

221 ---------------------------------- 

222 1. The assignment CREATOR's own questions in `teacher_questionbank`. 

223 2. The curated `global_questionbank` (in the admin-staff database), which is 

224 shared by design. 

225 

226 It never reaches another teacher's private questions. That bank holds 40,279 

227 rows from **1,166 distinct authors**; sampling it unscoped would hand one 

228 teacher's material to another teacher's students. It also never returns a 

229 soft-deleted question — 18,646 of those rows are `deleted: true`, and the 

230 previous picker filtered neither. 

231 

232 The old implementation read `db["question_collection"]` matching a 

233 `classification` field. That collection exists in NO database on this 

234 cluster, and no question document carries a `classification` field, so it 

235 could never return anything — the endpoint answered 400 "Something wrong 

236 fetching a new question." for every input that got that far (and 500 before 

237 PR #345). The live banks carry `assignmentType` (STAAR / SAT / TSI / ACT / 

238 TEACHER TEST / Quiz / Homework), which is what the route's 

239 `question_classification` parameter actually selects on. 

240 

241 `difficulty` arrives lowercase from the ladder ("easy"/"average"/"advance") 

242 and is title-cased to match the stored values. 

243 

244 Developer: Allan Ninal — 2026-09-23 (EI-T115) 

245 """ 

246 match = { 

247 "difficulty": (difficulty or "").title(), 

248 "assignmentType": classification, 

249 "deleted": {"$ne": True}, 

250 } 

251 if exclude_ids: 

252 match["_id"] = {"$nin": list(exclude_ids)} 

253 

254 # 1. the creator's own bank 

255 owned = await ( 

256 db["teacher_questionbank"] 

257 .aggregate( 

258 [ 

259 {"$match": {**match, "createdBy": {"$in": id_match_forms(creator_id)}}}, 

260 {"$sample": {"size": 1}}, 

261 ] 

262 ) 

263 .to_list(length=1) 

264 ) 

265 if owned: 

266 return _json_safe(owned[0]) 

267 

268 # 2. the curated global bank, shared by design 

269 if staff_admin_db is not None: 

270 shared = await ( 

271 staff_admin_db["global_questionbank"] 

272 .aggregate([{"$match": match}, {"$sample": {"size": 1}}]) 

273 .to_list(length=1) 

274 ) 

275 if shared: 

276 return _json_safe(shared[0]) 

277 

278 return None