Return feedback instead of crashing on submissions with no notes by peterbjohnson · Pull Request #19 · lambda-feedback/compareMusic

peterbjohnson · 2026-09-09T20:35:23Z

Closes #13

Problem

An empty note list raised IndexError, which reaches the student as a 500 rather than a feedback message.

compare_performance_ED({"notes": []}, reference)
# IndexError: list index out of range

This 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.

Approach

Tests first, added to evaluation_test.py as section 11, alongside the existing tests for compare_performance_ED (section 7) and evaluation_function (section 8). Before the change:

9 failed, 4 passed

The four passes are the deliberate control: one- and two-note submissions already worked, and they are covered so they stay working. All three empty combinations crashed.

What was actually broken

Two distinct faults, the second only visible once the first was fixed.

1. Indexing element zero of an empty list. event_alignment_ED inspected response_events[0] to decide whether its input was already grouped into events. The edit-distance boundary conditions already handle a zero-length side correctly, so the check just needed to be skipped when the list is empty.

2. Float arrays from empty lists. With the first fault fixed, build_cost_matrix failed differently:

TypeError: ufunc 'bitwise_and' not supported for the input types

np.array([]) defaults to float64, so the chord flags came back as floats and the & mask raised. The dtypes are now given explicitly.

Reporting the degenerate cases

Structurally the pipeline now produces the right numbers: an empty response marks all four reference notes missing. But the standard wording would be misleading, so both degenerate cases get their own message.

Empty response:

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.

Empty reference (a misconfigured question, not a student error):

Practice Summary
This question has no reference notes to compare your performance against, so it
could not be evaluated. Please let your teacher know.

is_correct is now also guarded, because an empty response against an empty reference satisfied every count and was being reported as correct.

Verification

Check	Result
New tests, before	9 failed, 4 passed
New tests, after	13 passed
Full suite	86 passed
CI lint gate (E9,F63,F7,F82)	0

🤖 Generated with Claude Code

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 <noreply@anthropic.com>

ada-jz7125 · 2026-09-10T06:37:08Z

+    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()


text assertion may fail if the feedback messages change in future

Thanks good point. Quick fix for now: move the two messages into named constants in compare_MIDI.py and assert against those:

from .compare_MIDI import NO_RESPONSE_NOTES_MESSAGE assert result.feedback_message == NO_RESPONSE_NOTES_MESSAGE

The same applies to the copy of this assertion in test_through_the_platform_entry_point just below, which you did not flag but has the identical problem.

A global collection of strings should also be added - this is captured in #22 #22

ada-jz7125 · 2026-09-10T06:38:09Z

+    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()


text assertion

Same fix here. This one is additionally weak as a test: "reference" appears in plenty of unrelated feedback, so it would keep passing even if the empty-reference branch stopped firing and the student got the ordinary "you missed every note" message instead.

Asserting result.feedback_message == NO_REFERENCE_NOTES_MESSAGE fixes both problems at once.

ada-jz7125

some text assertion may need changes

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 <noreply@anthropic.com>

Follows the same review point raised on #19. The two fixed preview messages are now named constants, so tests assert which case was hit rather than how it happens to be worded. The pitch-range test no longer hard-codes note names either. Note naming is a fact about MIDI rather than a wording choice, so it gets its own tests, and the range test now checks that the lowest and highest pitches are the ones reported, and that a middle pitch is not. The remaining text assertions cover note count, duration and pluralisation, which are the preview's actual contract rather than incidental phrasing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

peterbjohnson force-pushed the empty_input_guard branch from a8343d8 to 4fd1e1e Compare September 9, 2026 20:38

peterbjohnson assigned ada-jz7125 Sep 9, 2026

ada-jz7125 reviewed Sep 10, 2026

View reviewed changes

Navigation Menu

Uh oh!

Return feedback instead of crashing on submissions with no notes - #19

Return feedback instead of crashing on submissions with no notes#19
peterbjohnson wants to merge 2 commits into
mainfrom
empty_input_guard

peterbjohnson commented Sep 9, 2026 •

edited

Loading

Uh oh!

ada-jz7125 Sep 10, 2026

Uh oh!

peterbjohnson Sep 10, 2026 •

edited

Loading

Uh oh!

ada-jz7125 Sep 10, 2026

Uh oh!

peterbjohnson Sep 10, 2026

Uh oh!

ada-jz7125 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

peterbjohnson commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Approach

What was actually broken

Reporting the degenerate cases

Verification

Uh oh!

ada-jz7125 Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

peterbjohnson Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ada-jz7125 Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

peterbjohnson Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

ada-jz7125 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

peterbjohnson commented Sep 9, 2026 •

edited

Loading

peterbjohnson Sep 10, 2026 •

edited

Loading