Skip to content

question.validate() assert statements bypass node validation error collection #659

@rtibbles

Description

@rtibbles

This issue is not open for contribution. Visit Contributing guidelines to learn about the contributing process and how to find suitable issues.

Target branch: main

Observed behavior

BaseQuestion.validate() and its subclass overrides (PerseusQuestion, MultipleSelectQuestion, SingleSelectQuestion, InputQuestion) use bare assert statements for validation. When a question is invalid, these raise AssertionError directly.

Callers like ExerciseNode._validate() and UnitNode._validate() call question.validate() inside _validate_values(not question.validate(), ...), expecting a boolean return value. But the AssertionError propagates before _validate_values gets a chance to collect the error — so invalid questions produce raw AssertionError stack traces instead of structured InvalidNodeException.

Errors and logs

# Example: SingleSelectQuestion with no correct answer
Traceback (most recent call last):
  File "...", line ...
    assert correct_answers == 1, "Assumption Failed: Single selection question should have only one correct answer"
AssertionError: Assumption Failed: Single selection question should have only one correct answer

Instead of the expected InvalidNodeException with collected validation errors.

Expected behavior

Invalid questions should produce the same structured InvalidNodeException that other validation failures produce via _validate_values(). The error should be collected into the node's validation error list alongside any other validation issues, rather than immediately crashing with a raw AssertionError.

User-facing consequences

Chef authors debugging invalid questions see raw Python stack traces instead of the clear, aggregated validation error messages that ricecooker produces for all other node validation failures. This makes it harder to identify and fix content issues.

Steps to reproduce

  1. Create an ExerciseNode with a SingleSelectQuestion that has zero correct answers
  2. Add it to a channel and trigger validation
  3. Observe AssertionError instead of InvalidNodeException

Context

Affected files:

  • ricecooker/classes/questions.pyBaseQuestion.validate() (line ~226), PerseusQuestion.validate(), MultipleSelectQuestion.validate(), SingleSelectQuestion.validate(), InputQuestion.validate()
  • ricecooker/classes/nodes.pyExerciseNode._validate() (line ~1247), UnitNode._validate() (line ~1644)

Acceptance Criteria

  • question.validate() methods return False for invalid questions instead of raising AssertionError
  • Node _validate() methods collect question validation errors through _validate_values() like all other validation checks
  • Invalid questions produce InvalidNodeException with descriptive messages
  • Existing tests continue to pass

AI usage

Issue drafted by Claude during code review of the of_course branch. Description verified against source code in questions.py and nodes.py.

Metadata

Metadata

Assignees

No one assigned

    Type

    No fields configured for Bug.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions