From cdf233328fe0052965b94bd7473e7517236b6efa Mon Sep 17 00:00:00 2001 From: Steve Geinitz Date: Tue, 12 May 2026 10:53:16 -0600 Subject: [PATCH 1/2] Cache recent submissions and broaden follow-up audience - Add 10-minute mtime cache for the three submission CSVs written by getAllSubmissionsAndEvents. send-quiz-reminder and send-follow-up-question now skip the Canvas refetch when CSVs from a prior run (often a dry-run minutes earlier) are still fresh; other callers of getAllSubmissionsAndEvents are unchanged. - send-follow-up-question now sends to every student who attempted the quiz, with intro wording tailored to whether their latest attempt on the corresponding original question was correct (nice job, slightly more challenging follow-up) or missed (existing wording). - Rename _findStudentsWhoMissed to _classifyStudentsByQuestionResult; the helper now returns {student_id: 'missed' | 'correct'} for every attempter rather than a list of missed IDs. Co-Authored-By: Claude Opus 4.7 --- canvigator_course.py | 5 +- canvigator_help.py | 19 ++++-- canvigator_quiz.py | 149 +++++++++++++++++++++++++++++++++---------- test_canvigator.py | 112 ++++++++++++++++++++++++++++---- 4 files changed, 231 insertions(+), 54 deletions(-) diff --git a/canvigator_course.py b/canvigator_course.py index 74030a6..1f4ed0e 100644 --- a/canvigator_course.py +++ b/canvigator_course.py @@ -96,8 +96,9 @@ def sendAllQuizReminders(self, dry_run=False): logger.warning(msg) continue - print(" Fetching latest submissions...") - quiz.getAllSubmissionsAndEvents() + if not quiz._tryLoadRecentSubmissions(): + print(" Fetching latest submissions...") + quiz.getAllSubmissionsAndEvents() subs_by_q_df = quiz.all_subs_by_question events_df = quiz.all_subs_and_events quiz_scores = quiz._buildQuizScores() diff --git a/canvigator_help.py b/canvigator_help.py index 35766da..46ed42b 100644 --- a/canvigator_help.py +++ b/canvigator_help.py @@ -466,7 +466,10 @@ "message; imperfect-score students get a bulleted list of the " "questions they missed on their most recent attempt. With --all, " "sends ONE consolidated message per student covering every " - "published, future-due quiz." + "published, future-due quiz. If the quiz's submission CSVs were " + "written within the last 10 minutes, they are reused instead of " + "refetching from Canvas — making the typical dry-run-then-real " + "workflow fast on the second run." ), 'prerequisites': [ "get-quiz-questions --tag must have produced " @@ -499,10 +502,16 @@ 'description': ( "Send the instructor-selected open-ended follow-up question (the " "row marked selected_question=1 in *_open_ended_*.csv) via Canvas " - "message to each student who missed the corresponding original " - "quiz question on their latest attempt. Mode-aware instructions: " - "'explain' asks for a voice recording, 'draw' asks for an attached " - "photo. Uses force_new=True so the thread is dedicated." + "message to every student who has attempted the quiz. The intro " + "sentence is tailored to whether the student got the corresponding " + "original question right on their latest attempt ('nice job, here " + "is a more challenging follow-up to confirm mastery') or wrong " + "('here is a follow-up to reinforce your understanding'). " + "Mode-aware instructions: 'explain' asks for a voice recording, " + "'draw' asks for an attached photo. Uses force_new=True so the " + "thread is dedicated. Reuses the quiz's submission CSVs if they " + "were written within the last 10 minutes (e.g. just after a " + "send-quiz-reminder run) instead of refetching from Canvas." ), 'prerequisites': [ "get-quiz-questions --tag must have produced " diff --git a/canvigator_quiz.py b/canvigator_quiz.py index 0d2e5a9..f29188b 100644 --- a/canvigator_quiz.py +++ b/canvigator_quiz.py @@ -42,6 +42,31 @@ def _parse_points_possible_from_student_analysis(csv_path): return points_possible +def _findRecentSubmissionCsvs(data_path, quiz_prefix, quiz_id, max_age_minutes=10): + """Return paths to the three submission CSVs if all are recent, else None. + + Looks up the latest dated ``*_all_submissions_*.csv``, + ``*_all_subs_by_question_*.csv``, and ``*_all_subs_and_events_*.csv`` for + the given quiz. Returns the tuple ``(submissions, by_q, events)`` only when + all three exist AND each file's mtime is within ``max_age_minutes``. Pure + disk inspection — no DataFrame loading or side effects. Lets the + dry-run-then-real ``send-quiz-reminder`` workflow skip a Canvas refetch. + """ + prefix_id = f"{quiz_prefix}{quiz_id}" + paths = ( + find_latest_csv(data_path, f"{prefix_id}_all_submissions_"), + find_latest_csv(data_path, f"{prefix_id}_all_subs_by_question_"), + find_latest_csv(data_path, f"{prefix_id}_all_subs_and_events_"), + ) + if any(p is None for p in paths): + return None + + cutoff_ts = (datetime.now() - timedelta(minutes=max_age_minutes)).timestamp() + if not all(p.stat().st_mtime >= cutoff_ts for p in paths): + return None + return paths + + def _annotate_stats(ax, mean, std): """Render a small mean/sd text box in the upper-left of a matplotlib axis.""" text = f'mean: {_fmt_stat(mean)}\nsd: {_fmt_stat(std)}' @@ -719,9 +744,11 @@ def sendQuizReminders(self, dry_run=False): # Fail fast with a clear instruction if not. question_info = self._loadQuestionInfo() - # Refresh submission data — may be minutes old, so always re-fetch. - print("\nFetching latest submissions for missed-question details...") - self.getAllSubmissionsAndEvents() + # Refresh submission data, reusing CSVs from a recent run when present + # (the dry-run-then-real workflow re-runs minutes apart). + if not self._tryLoadRecentSubmissions(): + print("\nFetching latest submissions for missed-question details...") + self.getAllSubmissionsAndEvents() subs_by_q_df = self.all_subs_by_question events_df = self.all_subs_and_events @@ -821,14 +848,15 @@ def _sendOrPreviewMessages(self, messages, subject_str, quiz_name, points_possib _sendOrPreviewMessages(messages, subject_str, header_label) def sendFollowUpQuestions(self, dry_run=False): - """Send the instructor-selected open-ended follow-up question to students who missed it. + """Send the instructor-selected open-ended follow-up question to every student who attempted the quiz. Picks the first row in the *_open_ended_*.csv with `selected_question == 1` as the question to send (the instructor curates the choice offline). - Recipients are still students who missed the corresponding original quiz - question on their latest attempt. Requires that get-quiz-questions --tag - and generate-follow-up-questions have both been run for this quiz. - Auto-refreshes submission data to pick up recent attempts. + Every student who has at least one attempt receives the follow-up — the + message wording differs based on whether their latest attempt got the + corresponding original question right or wrong. Requires that + get-quiz-questions --tag and generate-follow-up-questions have both been + run for this quiz. Auto-refreshes submission data to pick up recent attempts. """ quiz_name = self.canvas_quiz.title @@ -860,22 +888,28 @@ def sendFollowUpQuestions(self, dry_run=False): print(f" Mode: {question_mode}") print(f" Follow-up: {open_ended_text[:120]}{'...' if len(open_ended_text) > 120 else ''}") - # Refresh submission data so the recipient list reflects the latest attempts - print("\nFetching latest submissions to identify students who missed this question...") - self.getAllSubmissionsAndEvents() + # Refresh submission data so the recipient list reflects the latest attempts, + # reusing CSVs from a recent run when present (e.g. send-quiz-reminder a few minutes ago). + if not self._tryLoadRecentSubmissions(): + print("\nFetching latest submissions to identify students who missed this question...") + self.getAllSubmissionsAndEvents() subs_by_q_df = self.all_subs_by_question if subs_by_q_df is None or subs_by_q_df.empty: print("No submission data available — cannot determine recipients.") return - # Build the list of students who missed this question on their latest attempt - students_who_missed = self._findStudentsWhoMissed(selected_qid, subs_by_q_df, question_info) + # Classify every attempter as 'missed' or 'correct' on this question + classification = self._classifyStudentsByQuestionResult(selected_qid, subs_by_q_df, question_info) - if not students_who_missed: - print("No students missed this question on their latest attempt — no follow-up needed!") + if not classification: + print("No students have attempted this quiz yet — no follow-up to send.") return + n_missed = sum(1 for v in classification.values() if v == 'missed') + n_correct = sum(1 for v in classification.values() if v == 'correct') + print(f" Sending follow-up to {len(classification)} students ({n_correct} correct, {n_missed} missed).") + # Compose messages if question_mode == 'draw': instructions = ( @@ -899,24 +933,36 @@ def sendFollowUpQuestions(self, dry_run=False): enrolled = self.canvas_course.students enrolled_map = {s['id']: s['name'] for s in enrolled} + tail = ( + f"{open_ended_text}\n\n" + f"{instructions}\n\n" + f"You can reply as many times as you'd like — only your most recent " + f"response will be assessed.\n\n" + f"NOTE: This is an auto-generated message, please let me know if you " + f"have any questions/concerns/suggestions about it." + ) + messages = [] - for student_id in students_who_missed: + for student_id, status in classification.items(): student_name = enrolled_map.get(student_id) if student_name is None: continue first_name = student_name.split()[0] - message_str = ( - f"Hello {first_name}, based on your recent attempt on {quiz_name}, " - f"here is a follow-up question to help reinforce your understanding " - f"of the topic ({keywords}):\n\n" - f"{open_ended_text}\n\n" - f"{instructions}\n\n" - f"You can reply as many times as you'd like — only your most recent " - f"response will be assessed.\n\n" - f"NOTE: This is an auto-generated message, please let me know if you " - f"have any questions/concerns/suggestions about it." - ) - messages.append((student_id, student_name, message_str, f"missed Q{position}")) + if status == 'correct': + intro = ( + f"Hello {first_name}, nice job on {quiz_name}! Here is a slightly " + f"more challenging follow-up question corresponding to one that " + f"you answered correctly — this will be good practice to ensure " + f"you have mastered the topic ({keywords})" + ) + else: + intro = ( + f"Hello {first_name}, based on your recent attempt on {quiz_name}, " + f"here is a follow-up question to help reinforce your understanding " + f"of the topic ({keywords})" + ) + message_str = f"{intro}:\n\n{tail}" + messages.append((student_id, student_name, message_str, f"{status} Q{position}")) if dry_run: self._sendOrPreviewMessages(messages, subject_str, quiz_name, self.canvas_quiz.points_possible) @@ -935,22 +981,29 @@ def _interactiveSend(self, messages, subject_str, quiz_name, points_possible): header_label = f"Quiz: {quiz_name} ({points_possible} points possible)" return _interactiveSend(self.canvas, messages, subject_str, header_label) - def _findStudentsWhoMissed(self, question_id, subs_by_q_df, question_info): - """Return a list of student IDs who scored below points_possible on the given question in their latest attempt.""" + def _classifyStudentsByQuestionResult(self, question_id, subs_by_q_df, question_info): + """Return ``{student_id: 'missed' | 'correct'}`` for every student who attempted the quiz. + + Classification is by the student's **latest** attempt on the given question: + ``'missed'`` if ``points < points_possible``, ``'correct'`` otherwise. + Students with no attempts are absent from the result entirely (the + follow-up question presupposes they saw the original). + """ pp = question_info.get(question_id, {}).get('points_possible') if pp is None: - return [] + return {} max_attempt_per_student = subs_by_q_df.groupby('id')['attempt'].transform('max') latest_attempts = subs_by_q_df[subs_by_q_df['attempt'] == max_attempt_per_student] q_rows = latest_attempts[latest_attempts['question_id'] == question_id] - missed_ids = [] + result = {} for _, row in q_rows.iterrows(): points = row.get('points') - if points is not None and pd.notna(points) and float(points) < float(pp): - missed_ids.append(row['id']) - return missed_ids + if points is None or pd.isna(points): + continue + result[row['id']] = 'missed' if float(points) < float(pp) else 'correct' + return result def _loadOpenEndedQuestions(self): """Load the latest *_open_ended_*.csv and return a dict keyed by question_id. @@ -2292,6 +2345,32 @@ def _buildSubByQuestionRow(self, user_id, attempt_num, q_idx, qdata): 'correct': qdata['correct'], } + def _tryLoadRecentSubmissions(self, max_age_minutes=10): + """Load cached submission CSVs if all three are recent; return ``True`` on hit. + + On a cache hit, populates ``self.all_subs_by_question`` and + ``self.all_subs_and_events`` from disk so the caller can skip + ``getAllSubmissionsAndEvents``. The third CSV (``all_submissions``) + is checked for freshness but not loaded — it has no in-memory + consumer in the reminder flow. + """ + paths = _findRecentSubmissionCsvs( + self.config.data_path, + self.config.quiz_prefix, + self.canvas_quiz.id, + max_age_minutes=max_age_minutes, + ) + if paths is None: + return False + _submissions_csv, by_q_csv, events_csv = paths + age_secs = int(datetime.now().timestamp() - min(p.stat().st_mtime for p in paths)) + msg = f"Reusing submissions fetched {age_secs}s ago (within {max_age_minutes}-min cache)." + print(msg) + logger.info(msg) + self.all_subs_by_question = pd.read_csv(by_q_csv) + self.all_subs_and_events = pd.read_csv(events_csv) + return True + def getAllSubmissionsAndEvents(self): """Collect per-attempt submission history and events into three CSVs.""" quiz_takers = self.quiz_df[['name', 'id']].copy() diff --git a/test_canvigator.py b/test_canvigator.py index 465b277..8014ee2 100644 --- a/test_canvigator.py +++ b/test_canvigator.py @@ -1080,17 +1080,17 @@ def _make_quiz_stub(): return CanvigatorQuiz -class TestFindStudentsWhoMissed: - """Tests for CanvigatorQuiz._findStudentsWhoMissed.""" +class TestClassifyStudentsByQuestionResult: + """Tests for CanvigatorQuiz._classifyStudentsByQuestionResult.""" def _call(self, question_id, subs_rows, question_info): - """Helper to call _findStudentsWhoMissed without a full quiz instance.""" + """Helper to call the classifier without a full quiz instance.""" cls = _make_quiz_stub() df = _make_subs_by_question_df(subs_rows) - return cls._findStudentsWhoMissed(None, question_id, df, question_info) + return cls._classifyStudentsByQuestionResult(None, question_id, df, question_info) - def test_returns_students_who_missed(self): - """Only students who scored below points_possible are returned.""" + def test_classifies_correct_and_missed(self): + """Each attempter is labeled 'missed' (points < pp) or 'correct' (points == pp).""" question_info = {100: {'position': 1, 'keywords': 'topic a', 'points_possible': 1.0}} rows = [ ('A', 1, 1, 1, 100, 0.0, 1.0, False), @@ -1098,26 +1098,39 @@ def test_returns_students_who_missed(self): ('C', 3, 1, 1, 100, 0.5, 1.0, False), ] result = self._call(100, rows, question_info) - assert sorted(result) == [1, 3] + assert result == {1: 'missed', 2: 'correct', 3: 'missed'} def test_uses_latest_attempt(self): - """Student who fixed the question on a later attempt is excluded.""" + """A later attempt overrides the earlier one (fixed on retry → 'correct').""" question_info = {100: {'position': 1, 'keywords': 'topic a', 'points_possible': 1.0}} rows = [ ('A', 1, 1, 1, 100, 0.0, 1.0, False), ('A', 1, 2, 1, 100, 1.0, 1.0, True), ] result = self._call(100, rows, question_info) - assert result == [] + assert result == {1: 'correct'} - def test_empty_when_no_misses(self): - """Returns empty list when all students scored perfectly.""" + def test_all_correct(self): + """When every latest attempt scored perfectly, every student is 'correct'.""" question_info = {100: {'position': 1, 'keywords': 'topic a', 'points_possible': 1.0}} rows = [ ('A', 1, 1, 1, 100, 1.0, 1.0, True), + ('B', 2, 1, 1, 100, 1.0, 1.0, True), ] result = self._call(100, rows, question_info) - assert result == [] + assert result == {1: 'correct', 2: 'correct'} + + def test_question_missing_from_info_returns_empty(self): + """Without a points_possible entry the classifier can't label anyone.""" + rows = [('A', 1, 1, 1, 100, 1.0, 1.0, True)] + result = self._call(100, rows, question_info={}) + assert result == {} + + def test_no_attempts_returns_empty(self): + """Students who never attempted the quiz are absent from the result.""" + question_info = {100: {'position': 1, 'keywords': 'topic a', 'points_possible': 1.0}} + result = self._call(100, [], question_info) + assert result == {} # --------------------------------------------------------------------------- @@ -3016,6 +3029,81 @@ def test_results_sorted_ascending_by_date(self, tmp_path): assert [p.name for p in result] == ['foo_20260424.csv', 'foo_20260427.csv', 'foo_20260501.csv'] +class TestFindRecentSubmissionCsvs: + """Tests for canvigator_quiz._findRecentSubmissionCsvs.""" + + SUFFIXES = ('all_submissions', 'all_subs_by_question', 'all_subs_and_events') + + def _write_csv(self, tmp_path, name, age_seconds=0): + """Write a CSV in ``tmp_path``; backdate its mtime by ``age_seconds`` if positive.""" + import os + import time + path = tmp_path / name + path.write_text('id\n1\n') + if age_seconds > 0: + ts = time.time() - age_seconds + os.utime(path, (ts, ts)) + + def _write_all_three(self, tmp_path, prefix='quiz', quiz_id=123, date_str='20260510', age_seconds=0): + """Write all three submission CSVs for ``prefix{quiz_id}`` with the given mtime offset.""" + for suffix in self.SUFFIXES: + self._write_csv(tmp_path, f"{prefix}{quiz_id}_{suffix}_{date_str}.csv", age_seconds=age_seconds) + + def test_all_three_fresh_returns_paths(self, tmp_path): + """When all three CSVs exist with current mtimes, the helper returns a 3-tuple of Paths.""" + from canvigator_quiz import _findRecentSubmissionCsvs + self._write_all_three(tmp_path) + result = _findRecentSubmissionCsvs(tmp_path, 'quiz', 123, max_age_minutes=10) + assert result is not None + assert len(result) == 3 + for p, suffix in zip(result, self.SUFFIXES): + assert suffix in p.name + + def test_missing_one_returns_none(self, tmp_path): + """If any of the three CSVs is absent, the helper returns None.""" + from canvigator_quiz import _findRecentSubmissionCsvs + self._write_csv(tmp_path, 'quiz123_all_submissions_20260510.csv') + self._write_csv(tmp_path, 'quiz123_all_subs_by_question_20260510.csv') + # all_subs_and_events deliberately missing + assert _findRecentSubmissionCsvs(tmp_path, 'quiz', 123, max_age_minutes=10) is None + + def test_one_stale_returns_none(self, tmp_path): + """A single file outside the freshness window invalidates the cache.""" + from canvigator_quiz import _findRecentSubmissionCsvs + self._write_csv(tmp_path, 'quiz123_all_submissions_20260510.csv') + self._write_csv(tmp_path, 'quiz123_all_subs_by_question_20260510.csv') + # 15 minutes is past the 10-minute window + self._write_csv(tmp_path, 'quiz123_all_subs_and_events_20260510.csv', age_seconds=15 * 60) + assert _findRecentSubmissionCsvs(tmp_path, 'quiz', 123, max_age_minutes=10) is None + + def test_all_stale_returns_none(self, tmp_path): + """All-stale CSVs return None.""" + from canvigator_quiz import _findRecentSubmissionCsvs + self._write_all_three(tmp_path, age_seconds=20 * 60) + assert _findRecentSubmissionCsvs(tmp_path, 'quiz', 123, max_age_minutes=10) is None + + def test_respects_quiz_prefix(self, tmp_path): + """A non-default quiz_prefix is honored when looking up files.""" + from canvigator_quiz import _findRecentSubmissionCsvs + self._write_all_three(tmp_path, prefix='midterm') + # Default prefix should miss + assert _findRecentSubmissionCsvs(tmp_path, 'quiz', 123, max_age_minutes=10) is None + # Matching prefix should hit + result = _findRecentSubmissionCsvs(tmp_path, 'midterm', 123, max_age_minutes=10) + assert result is not None + + def test_picks_latest_dated_per_pattern(self, tmp_path): + """When multiple dated copies exist, the helper inherits ``find_latest_csv``'s newest-wins semantics.""" + from canvigator_quiz import _findRecentSubmissionCsvs + # Older copies are stale; newer copies are fresh — newest dated should win and pass freshness. + self._write_all_three(tmp_path, date_str='20260401', age_seconds=60 * 60) + self._write_all_three(tmp_path, date_str='20260510') + result = _findRecentSubmissionCsvs(tmp_path, 'quiz', 123, max_age_minutes=10) + assert result is not None + for p in result: + assert '20260510' in p.name + + class TestLoadQuizMisses: """Tests for canvigator_digest._loadQuizMisses.""" From 87436427a4b360832142dd03c25e366b462c65f8 Mon Sep 17 00:00:00 2001 From: Steve Geinitz Date: Tue, 12 May 2026 10:55:17 -0600 Subject: [PATCH 2/2] Prompt [y/N] per row in send-follow-up-assessments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Real-send mode now previews each feedback message and prompts the instructor to confirm before posting. Default is SKIP — only an exact lowercase 'y' sends, matching the convention already used by send-quiz-reminder / send-follow-up-question. Skipped rows stay at sent_assessment=0 so they can be revisited on a subsequent run. --dry-run is unchanged (previews every row without prompting). Co-Authored-By: Claude Opus 4.7 --- canvigator_help.py | 14 ++++++++------ canvigator_quiz.py | 10 +++++++++- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/canvigator_help.py b/canvigator_help.py index 46ed42b..37c5e9d 100644 --- a/canvigator_help.py +++ b/canvigator_help.py @@ -584,12 +584,14 @@ 'send-follow-up-assessments': { 'description': ( "For every row in the persistent *_followup_assessments.csv with " - "sent_assessment=0 and a non-empty 'feedback' value, post the " - "feedback as a reply on the existing follow-up conversation using " - "the row's conversation_id. On success, sets sent_assessment=1 and " - "stamps sent_at. The instructor edits the 'feedback' column " - "between assess-replies and this task — feedback already sent is " - "never overwritten." + "sent_assessment=0 and a non-empty 'feedback' value, preview the " + "feedback and prompt the instructor [y/N] before posting it as a " + "reply on the existing follow-up conversation (using the row's " + "conversation_id). Default is SKIP — only an exact 'y' sends. " + "On a confirmed send, sets sent_assessment=1 and stamps sent_at. " + "Skipped rows stay at sent_assessment=0 so they can be revisited. " + "The instructor edits the 'feedback' column between assess-replies " + "and this task — feedback already sent is never overwritten." ), 'prerequisites': [ "assess-replies must have produced " diff --git a/canvigator_quiz.py b/canvigator_quiz.py index f29188b..9427c84 100644 --- a/canvigator_quiz.py +++ b/canvigator_quiz.py @@ -1597,6 +1597,8 @@ def sendFollowUpAssessments(self, dry_run=False): return print(f"\n{'[DRY-RUN] ' if dry_run else ''}Sending feedback for {len(to_send)} student(s).") + if not dry_run: + print("For each student, enter 'y' + Enter to send, or just Enter to skip (default).") n_sent = 0 for idx, convo_id, row in to_send: if self._sendOneAssessment(df, idx, convo_id, row, dry_run): @@ -1642,7 +1644,9 @@ def _sendOneAssessment(self, df, idx, convo_id, row, dry_run): """Send (or preview) feedback for a single assessment row. Updates ``df`` in place on a successful real send and returns True; - returns False on dry-run or send failure. + returns False on dry-run, skip, or send failure. Real sends require + per-message [y/N] confirmation — default (bare Enter or anything + other than ``y``) is SKIP. """ student_name = row.get('student_name', '?') feedback = str(row['feedback']).strip() @@ -1654,6 +1658,10 @@ def _sendOneAssessment(self, df, idx, convo_id, row, dry_run): print(f" {line}") if dry_run: return False + choice = input(f" Send to {student_name}? [y/N]: ").strip().lower() + if choice != 'y': + print(f" Skipped {student_name}.") + return False try: convo = self.canvas.get_conversation(convo_id, auto_mark_as_read=False) convo.add_message(message_body)