learningequality / learningequality/ricecooker
question.validate() assert statements bypass node validation error collection
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 31
- Forks
- 77
- Avg merge
- 5d 4h
- Merged PRs (30d)
- 3
Description
❌ 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
- Create an
ExerciseNodewith aSingleSelectQuestionthat has zero correct answers - Add it to a channel and trigger validation
- Observe
AssertionErrorinstead ofInvalidNodeException
Context
Affected files:
ricecooker/classes/questions.py—BaseQuestion.validate()(line ~226),PerseusQuestion.validate(),MultipleSelectQuestion.validate(),SingleSelectQuestion.validate(),InputQuestion.validate()ricecooker/classes/nodes.py—ExerciseNode._validate()(line ~1247),UnitNode._validate()(line ~1644)
Acceptance Criteria
-
question.validate()methods returnFalsefor invalid questions instead of raisingAssertionError - Node
_validate()methods collect question validation errors through_validate_values()like all other validation checks - Invalid questions produce
InvalidNodeExceptionwith 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.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the validate() methods in ricecooker/classes/questions.py and the _validate() methods in ricecooker/classes/nodes.py. Reproduce the SingleSelectQuestion case with no correct answer, then inspect how _validate_values() collects errors. Done means invalid question validation returns False, node validation produces descriptive InvalidNodeException errors, and existing tests still pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 25/100