From 4fd1e1ea873ae274e2f6202357c06a880bb29eb7 Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Wed, 9 Sep 2026 21:35:05 +0100 Subject: [PATCH 1/2] Return feedback instead of crashing on submissions with no notes An empty note list raised IndexError, which reached the student as a 500 rather than a feedback message. This is reachable in production: a student submits nothing, uploads a silent or failed recording, or plays so quietly that transcription returns no notes. Three changes: - event_alignment_ED indexed element zero to decide whether its input was already grouped into events. Skip that check when the list is empty; the cost matrix boundary conditions already handle a zero-length side. - build_cost_matrix built its arrays without explicit dtypes, so an empty list produced float arrays and the boolean chord mask raised TypeError. Give the dtypes explicitly. - Report the degenerate cases plainly. Describing every reference note as "missed" is misleading when nothing was submitted, and an empty response against an empty reference must not be marked correct. Tests go in evaluation_test.py as section 11, alongside the other tests for compare_performance_ED and evaluation_function. Short submissions of one or two notes already worked and are covered so they stay working. Co-Authored-By: Claude Opus 5 --- evaluation_function/compare_MIDI.py | 55 ++++++++++++--- evaluation_function/evaluation_test.py | 94 +++++++++++++++++++++++++- 2 files changed, 140 insertions(+), 9 deletions(-) diff --git a/evaluation_function/compare_MIDI.py b/evaluation_function/compare_MIDI.py index 8d72cc4..d546b55 100644 --- a/evaluation_function/compare_MIDI.py +++ b/evaluation_function/compare_MIDI.py @@ -285,18 +285,25 @@ def build_cost_matrix(response_events, ref_events, gap_penalty=DEFAULT_GAP_PENAL # Build simple per-event arrays: is this event a chord, and (if it is a # single note) what is its pitch. - res_is_chord = np.array([event["event_type"] == "chord" for event in response_events]) - ref_is_chord = np.array([event["event_type"] == "chord" for event in ref_events]) + # The dtypes are given explicitly so that an empty event list still + # produces bool/int arrays; numpy would otherwise default them to float + # and the boolean masks below would fail. + res_is_chord = np.array( + [event["event_type"] == "chord" for event in response_events], dtype=bool + ) + ref_is_chord = np.array( + [event["event_type"] == "chord" for event in ref_events], dtype=bool + ) # For note events, extract the pitch; for chords, use 0 as a placeholder. res_pitch = np.array([ event["notes"][0]["pitch"] if event["event_type"] == "note" else 0 for event in response_events - ]) + ], dtype=int) ref_pitch = np.array([ event["notes"][0]["pitch"] if event["event_type"] == "note" else 0 for event in ref_events - ]) + ], dtype=int) # Note-vs-note cost: vectorised absolute pitch difference for every pair. # Shape (N, 1) - shape (1, M) broadcasts to (N, M) @@ -351,10 +358,12 @@ def event_alignment_ED(response_events, ref_events, gap_penalty=DEFAULT_GAP_PENA D: accumulated cost matrix, shape (N+1, M+1) """ # if a raw note dict with "pitch"/"start"/"duration" but no "event_type" is - # passed in, group them into events first. - if "event_type" not in response_events[0]: + # passed in, group them into events first. + # An empty list has nothing to inspect, and nothing to group either, so + # skip the check rather than indexing into it. + if response_events and "event_type" not in response_events[0]: response_events = group_notes_into_events(response_events) - if "event_type" not in ref_events[0]: + if ref_events and "event_type" not in ref_events[0]: ref_events = group_notes_into_events(ref_events) # the rows of D correspond to response events @@ -1087,6 +1096,31 @@ def polished_feedback_message(event_details, response_events, ref_events, stats, Returns: feedback_message (str) """ + # Degenerate submissions, handled before the tiered messages below. + # The usual wording would be actively misleading here: telling a student + # who submitted nothing that they "missed" every note, or praising a + # perfect match against a reference that contains no notes at all. + if len(ref_events) == 0: + return "\n".join([ + "Practice Summary", + "This question has no reference notes to compare your performance " + "against, so it could not be evaluated. Please let your teacher know.", + ]) + + if len(response_events) == 0: + return "\n".join([ + "Practice Summary", + "No notes were detected in your submission, so there was nothing " + "to compare against the reference.", + "", + "What to check", + "If you submitted a recording, check that it is not silent and that " + "your instrument can be heard clearly. If you submitted MIDI, check " + "that it contains notes.", + "", + "Have another go when you are ready.", + ]) + note_events = [n for n in event_details if n["event_type"] == "note"] chord_events = [ch for ch in event_details if ch["event_type"] == "chord"] @@ -1434,8 +1468,13 @@ def compare_performance_ED(responseMIDI, refMIDI, ) # Step 6: Overall pass/fail judgement + # A submission with no notes on either side cannot be correct, even though + # the counts below are all trivially satisfied when there is nothing to + # compare. is_correct = ( - stats["total_notes_missing"] == 0 + len(response_events) > 0 + and len(ref_events) > 0 + and stats["total_notes_missing"] == 0 and stats["total_notes_extra"] == 0 and stats["total_chords_missing"] == 0 and stats["total_chords_extra"] == 0 diff --git a/evaluation_function/evaluation_test.py b/evaluation_function/evaluation_test.py index 9aca78e..9b1210e 100755 --- a/evaluation_function/evaluation_test.py +++ b/evaluation_function/evaluation_test.py @@ -19,6 +19,7 @@ 8. Tests for evaluation_function (Lambda Feedback integration) 9. Tests for parameter overrides 10. Bulk tests using longer MIDI sequences +11. Tests for submissions that contain no notes """ @@ -705,4 +706,95 @@ def test_realistic_scenario(case): ] assert len(matching_notes) == 1 flagged_note = matching_notes[0] - assert flagged_note["timing_correct"] is False \ No newline at end of file + assert flagged_note["timing_correct"] is False + +# 11. Tests for submissions that contain no notes +# ------------------------------------------------------------------------------ +# An empty note list is reachable in production: a student submits nothing, +# uploads a silent or failed recording, or plays so quietly that transcription +# returns no notes at all. These cases used to raise IndexError, which reaches +# the student as a 500 rather than a feedback message. +# +# Short-but-not-empty submissions already worked, and are covered here so they +# stay working. + +EMPTY_MIDI = {"notes": []} + + +def short_melody(note_count): + """The first note_count notes of a simple four-note melody.""" + pitches = [60, 62, 64, 65][:note_count] + starts = [0.0, 0.5, 1.0, 1.5][:note_count] + return make_midi(pitches, starts, [0.4] * note_count) + + +FOUR_NOTE_REFERENCE = short_melody(4) + + +class TestEmptyResponse(unittest.TestCase): + + def test_does_not_raise(self): + compare_performance_ED(EMPTY_MIDI, FOUR_NOTE_REFERENCE) + + def test_is_not_correct(self): + result = compare_performance_ED(EMPTY_MIDI, FOUR_NOTE_REFERENCE) + assert result.is_correct is False + + def test_every_reference_note_counted_as_missing(self): + result = compare_performance_ED(EMPTY_MIDI, FOUR_NOTE_REFERENCE) + assert result.stats["total_notes_missing"] == 4 + assert result.stats["total_notes_extra"] == 0 + + def test_feedback_says_no_notes_were_detected(self): + # "You missed four notes" is technically true but unhelpful when the + # student submitted nothing at all. The message should say so plainly. + result = compare_performance_ED(EMPTY_MIDI, FOUR_NOTE_REFERENCE) + assert "no notes" in result.feedback_message.lower() + + def test_through_the_platform_entry_point(self): + result = evaluation_function(EMPTY_MIDI, FOUR_NOTE_REFERENCE, {}) + assert result["is_correct"] is False + assert "no notes" in result["feedback"].lower() + + +class TestEmptyReference(unittest.TestCase): + """An empty reference is a misconfigured question, not a student error.""" + + def test_does_not_raise(self): + compare_performance_ED(FOUR_NOTE_REFERENCE, EMPTY_MIDI) + + def test_is_not_correct(self): + result = compare_performance_ED(FOUR_NOTE_REFERENCE, EMPTY_MIDI) + assert result.is_correct is False + + def test_feedback_points_at_the_question_not_the_student(self): + result = compare_performance_ED(FOUR_NOTE_REFERENCE, EMPTY_MIDI) + assert "reference" in result.feedback_message.lower() + + +class TestBothEmpty(unittest.TestCase): + + def test_does_not_raise(self): + compare_performance_ED(EMPTY_MIDI, EMPTY_MIDI) + + def test_is_not_correct(self): + # A submission with nothing to compare cannot be correct, even though + # an empty response trivially "matches" an empty reference. + result = compare_performance_ED(EMPTY_MIDI, EMPTY_MIDI) + assert result.is_correct is False + + +class TestShortSubmissions(unittest.TestCase): + """One and two note submissions already worked; keep them working.""" + + def test_one_note_against_four_note_reference(self): + result = compare_performance_ED(short_melody(1), FOUR_NOTE_REFERENCE) + assert result.stats["total_notes_missing"] == 3 + + def test_two_notes_against_four_note_reference(self): + result = compare_performance_ED(short_melody(2), FOUR_NOTE_REFERENCE) + assert result.stats["total_notes_missing"] == 2 + + def test_single_note_matching_single_note_reference_is_correct(self): + one_note = short_melody(1) + assert compare_performance_ED(one_note, one_note).is_correct is True From b45482952f39b9f97694c8a5fe87fea9ba41f9fb Mon Sep 17 00:00:00 2001 From: "Peter B. Johnson" Date: Thu, 10 Sep 2026 08:28:17 +0100 Subject: [PATCH 2/2] Assert the degenerate feedback by constant, not by substring Review feedback: the tests matched on message text, so rewording the feedback would fail a test whose behaviour had not changed. Lift the two degenerate messages into named constants and compare against those instead. Rewording now means editing the constant, and the tests follow automatically. This also makes the assertions stronger rather than merely more stable. "reference" appears in ordinary feedback too, so the old substring check passed even when the empty-reference branch did not fire at all. Disabling that branch now fails two tests, where before it failed none. Add a test that both sides being empty reports the misconfigured question rather than the empty submission, which was previously unpinned. Co-Authored-By: Claude Opus 5 --- evaluation_function/compare_MIDI.py | 41 +++++++++++++++----------- evaluation_function/evaluation_test.py | 17 ++++++++--- 2 files changed, 37 insertions(+), 21 deletions(-) diff --git a/evaluation_function/compare_MIDI.py b/evaluation_function/compare_MIDI.py index d546b55..41c65fe 100644 --- a/evaluation_function/compare_MIDI.py +++ b/evaluation_function/compare_MIDI.py @@ -40,6 +40,28 @@ # Default threshold: notes starting within 50ms are grouped as one chord. DEFAULT_CHORD_ONSET_WINDOW = 0.05 +# Feedback for the two degenerate cases, where there is nothing to compare. +# These are named rather than written inline so that tests can assert which +# case was hit without depending on the wording, which is free to change. +NO_REFERENCE_NOTES_MESSAGE = "\n".join([ + "Practice Summary", + "This question has no reference notes to compare your performance " + "against, so it could not be evaluated. Please let your teacher know.", +]) + +NO_RESPONSE_NOTES_MESSAGE = "\n".join([ + "Practice Summary", + "No notes were detected in your submission, so there was nothing " + "to compare against the reference.", + "", + "What to check", + "If you submitted a recording, check that it is not silent and that " + "your instrument can be heard clearly. If you submitted MIDI, check " + "that it contains notes.", + "", + "Have another go when you are ready.", +]) + # template and helper functions for chords # ------------------------------------------------------------------------------ # Chord template dictionary. @@ -1101,25 +1123,10 @@ def polished_feedback_message(event_details, response_events, ref_events, stats, # who submitted nothing that they "missed" every note, or praising a # perfect match against a reference that contains no notes at all. if len(ref_events) == 0: - return "\n".join([ - "Practice Summary", - "This question has no reference notes to compare your performance " - "against, so it could not be evaluated. Please let your teacher know.", - ]) + return NO_REFERENCE_NOTES_MESSAGE if len(response_events) == 0: - return "\n".join([ - "Practice Summary", - "No notes were detected in your submission, so there was nothing " - "to compare against the reference.", - "", - "What to check", - "If you submitted a recording, check that it is not silent and that " - "your instrument can be heard clearly. If you submitted MIDI, check " - "that it contains notes.", - "", - "Have another go when you are ready.", - ]) + return NO_RESPONSE_NOTES_MESSAGE note_events = [n for n in event_details if n["event_type"] == "note"] chord_events = [ch for ch in event_details if ch["event_type"] == "chord"] diff --git a/evaluation_function/evaluation_test.py b/evaluation_function/evaluation_test.py index 9b1210e..9cb3468 100755 --- a/evaluation_function/evaluation_test.py +++ b/evaluation_function/evaluation_test.py @@ -38,6 +38,8 @@ event_level_feedback, compute_stats, compare_performance_ED, + NO_REFERENCE_NOTES_MESSAGE, + NO_RESPONSE_NOTES_MESSAGE, DEFAULT_GAP_PENALTY, TIMING_RELATIVE_THRESHOLD, DURATION_RELATIVE_THRESHOLD, @@ -747,14 +749,15 @@ def test_every_reference_note_counted_as_missing(self): def test_feedback_says_no_notes_were_detected(self): # "You missed four notes" is technically true but unhelpful when the - # student submitted nothing at all. The message should say so plainly. + # student submitted nothing at all. Compare against the constant the + # code returns, so that rewording the message does not break this. result = compare_performance_ED(EMPTY_MIDI, FOUR_NOTE_REFERENCE) - assert "no notes" in result.feedback_message.lower() + assert result.feedback_message == NO_RESPONSE_NOTES_MESSAGE def test_through_the_platform_entry_point(self): result = evaluation_function(EMPTY_MIDI, FOUR_NOTE_REFERENCE, {}) assert result["is_correct"] is False - assert "no notes" in result["feedback"].lower() + assert result["feedback"] == NO_RESPONSE_NOTES_MESSAGE class TestEmptyReference(unittest.TestCase): @@ -769,7 +772,7 @@ def test_is_not_correct(self): def test_feedback_points_at_the_question_not_the_student(self): result = compare_performance_ED(FOUR_NOTE_REFERENCE, EMPTY_MIDI) - assert "reference" in result.feedback_message.lower() + assert result.feedback_message == NO_REFERENCE_NOTES_MESSAGE class TestBothEmpty(unittest.TestCase): @@ -783,6 +786,12 @@ def test_is_not_correct(self): result = compare_performance_ED(EMPTY_MIDI, EMPTY_MIDI) assert result.is_correct is False + def test_reports_the_missing_reference_rather_than_the_empty_response(self): + # With nothing on either side, the misconfigured question is the more + # useful thing to report, so that branch must win. + result = compare_performance_ED(EMPTY_MIDI, EMPTY_MIDI) + assert result.feedback_message == NO_REFERENCE_NOTES_MESSAGE + class TestShortSubmissions(unittest.TestCase): """One and two note submissions already worked; keep them working."""