From 1bb2efcb655c046091036061e7cf58ccd655ea5b Mon Sep 17 00:00:00 2001 From: Peter Derias Date: Tue, 21 Apr 2026 20:08:31 +1000 Subject: [PATCH 1/2] fix(conditions): improve OR condition handling in CompositeCondition class test: add regression test for nested AND/OR condition enforcement --- backend/algorithms/objects/conditions.py | 28 +++--- backend/algorithms/tests/test_autoplanning.py | 36 +++++++ .../server/tests/planner/test_autoplanning.py | 95 +++++++++++++++++++ 3 files changed, 145 insertions(+), 14 deletions(-) diff --git a/backend/algorithms/objects/conditions.py b/backend/algorithms/objects/conditions.py index 53063b1e8..ef549e8b2 100644 --- a/backend/algorithms/objects/conditions.py +++ b/backend/algorithms/objects/conditions.py @@ -5,6 +5,7 @@ import json import re from abc import ABC, abstractmethod +from itertools import chain from typing import Optional, Tuple, TypedDict from algorithms.objects.categories import AnyCategory, Category @@ -579,22 +580,21 @@ def condition_to_model(self, model: cp_model.CpModel, user: User, courses: list[ # just aggregate the conditions together, they are already all simultaneously asserted return sum((condition.condition_to_model(model, user, courses, course_variable) for condition in self.conditions), []) case Logic.OR: - or_constraints: list[cp_model.Constraint] = sum(( + condition_truths = [model.new_bool_var(f"condition_{index}") for index, _ in enumerate(self.conditions)] + + # Build each child branch independently, then flatten the result. + branch_constraints = [ condition.condition_to_model(model, user, courses, course_variable) for condition in self.conditions - ), []) - or_opposite_constraints: list[cp_model.Constraint] = sum((condition.condition_negation(model, user, courses, course_variable) for condition in self.conditions), []) - boolean_vars = [model.new_bool_var(f"condition_{i}") for i in range(len(or_constraints))] - # Create a boolean for each condition in the OR group. This channeling constraint ensures: - # - If condition_satisfied=true, the condition's constraint is enforced - # - If condition_satisfied=false, the condition's negation is enforced - # Then add_bool_or ensures at least one of these booleans is true (OR logic). - # https://developers.google.com/optimization/cp/channeling - for constraint, negation, boolean in zip(or_constraints, or_opposite_constraints, boolean_vars): - constraint.only_enforce_if(boolean) - negation.only_enforce_if(boolean.Not()) - model.add_bool_or(boolean_vars) - return or_constraints + ] + + for condition_truth, constraints in zip(condition_truths, branch_constraints): + for constraint in constraints: + constraint.only_enforce_if(condition_truth) + + # one of our constraints must be met + model.add_bool_or(condition_truths) + return list(chain.from_iterable(branch_constraints)) def condition_negation(self, model: cp_model.CpModel, user: User, courses: list[Tuple[cp_model.IntVar, Course]], course_variable: cp_model.IntVar) -> list[cp_model.Constraint]: if len(self.conditions) == 0: diff --git a/backend/algorithms/tests/test_autoplanning.py b/backend/algorithms/tests/test_autoplanning.py index 2c0bf3c08..bbb1d1c70 100644 --- a/backend/algorithms/tests/test_autoplanning.py +++ b/backend/algorithms/tests/test_autoplanning.py @@ -1,6 +1,7 @@ from typing import Optional from pytest import raises from algorithms.autoplanning import autoplan, terms_between +from algorithms.create import create_condition from algorithms.objects.course import Course from algorithms.objects.user import User from algorithms.validate_term_planner import RawUserPlan, validate_terms @@ -80,6 +81,41 @@ def test_more_complex_prereqs(): ["COMPBH"] ) + +def test_comp4128_requires_comp3121_ordering_when_no_comp3821(): + """Regression: OR branches with nested AND must be enforced as whole branches.""" + prereq_code = "COMP9998" + target_code = "COMP9999" + target_condition = create_condition([ + "(", + "COMP8888", + "||", + "(", + prereq_code, + "&&", + "75WAM", + ")", + ")", + ]) + + results = autoplan( + [ + Course(prereq_code, create_condition(["(", ")"]), 80, 6, {2026: [1, 2, 3]}), + Course(target_code, target_condition, 80, 6, {2026: [1, 2, 3]}), + ], + User({ + "program": "3778", + "specialisations": ["COMPA1"], + "courses": {}, + }), + (2026, 0), + (2026, 3), + [12, 12, 12, 12], + ) + + placements = {course_name: term for course_name, term in results} + assert terms_between((2026, 0), placements[prereq_code]) < terms_between((2026, 0), placements[target_code]) + def test_infeasable(): with raises(Exception): # no terms have space diff --git a/backend/server/tests/planner/test_autoplanning.py b/backend/server/tests/planner/test_autoplanning.py index 256fcd3a1..3210846c1 100644 --- a/backend/server/tests/planner/test_autoplanning.py +++ b/backend/server/tests/planner/test_autoplanning.py @@ -71,3 +71,98 @@ def test_autoplanning_generic(): assert x.status_code == 200 body = x.json() assert 'plan' in body + + +def _find_course_term_index(planner_years: list[dict[str, list[str]]], course_code: str) -> int: + for year_index, year in enumerate(planner_years): + for term_index in range(4): + if course_code in year[f'T{term_index}']: + return year_index * 4 + term_index + raise AssertionError(f"{course_code} was not found in planner years") + + +def test_autoplanning_jason_screenshot_ordering(): + clear() + token = get_token() + headers = get_token_headers(token) + + import_res = requests.put('http://127.0.0.1:8000/user/import', json={ + "degree": { + "programCode": "3778", + "specs": [ + "COMPA1" + ] + }, + "courses": { + "MATH1081": {"mark": None, "ignoreFromProgression": False}, + "COMP1511": {"mark": None, "ignoreFromProgression": False}, + "COMP1521": {"mark": None, "ignoreFromProgression": False}, + "COMP2521": {"mark": None, "ignoreFromProgression": False}, + "COMP2041": {"mark": None, "ignoreFromProgression": False}, + "COMP3121": {"mark": None, "ignoreFromProgression": False}, + "COMP4128": {"mark": None, "ignoreFromProgression": False}, + "MATH2901": {"mark": None, "ignoreFromProgression": False}, + "MATH3856": {"mark": None, "ignoreFromProgression": False}, + "SCIF0000": {"mark": None, "ignoreFromProgression": False} + }, + "planner": { + "years": [ + { + "T0": [], + "T1": ["MATH1081", "COMP1511"], + "T2": ["COMP2521", "COMP1521"], + "T3": [] + }, + { + "T0": [], + "T1": [], + "T2": [], + "T3": [] + }, + { + "T0": [], + "T1": [], + "T2": [], + "T3": [] + }, + { + "T0": [], + "T1": [], + "T2": [], + "T3": [] + } + ], + "unplanned": [ + "COMP2041", + "COMP3121", + "COMP4128", + "MATH2901", + "MATH3856", + "SCIF0000" + ], + "startYear": 2023, + "isSummerEnabled": False, + "lockedTerms": {} + }, + "settings": { + "showMarks": True, + "hiddenYears": [] + } + }, headers=headers) + assert import_res.status_code == 200 + + autoplan_res = requests.post( + 'http://127.0.0.1:8000/planner/autoplan', + json={'endTime': [2026, 3]}, + headers=headers, + ) + assert autoplan_res.status_code == 200 + + user_res = requests.get('http://127.0.0.1:8000/user/data/all', headers=headers) + assert user_res.status_code == 200 + + planner_years = user_res.json()['planner']['years'] + comp3121_term = _find_course_term_index(planner_years, 'COMP3121') + comp4128_term = _find_course_term_index(planner_years, 'COMP4128') + + assert comp3121_term < comp4128_term From 5c0bd8cca20e4898112deeac6619aacea34ee9ef Mon Sep 17 00:00:00 2001 From: Peter Derias Date: Tue, 21 Apr 2026 20:08:31 +1000 Subject: [PATCH 2/2] fix(conditions): improve OR condition handling in CompositeCondition class test: add regression test for nested AND/OR condition enforcement --- backend/algorithms/objects/conditions.py | 28 +++--- backend/algorithms/tests/test_autoplanning.py | 36 ++++++++ .../server/tests/planner/test_autoplanning.py | 91 +++++++++++++++++++ 3 files changed, 141 insertions(+), 14 deletions(-) diff --git a/backend/algorithms/objects/conditions.py b/backend/algorithms/objects/conditions.py index 53063b1e8..ef549e8b2 100644 --- a/backend/algorithms/objects/conditions.py +++ b/backend/algorithms/objects/conditions.py @@ -5,6 +5,7 @@ import json import re from abc import ABC, abstractmethod +from itertools import chain from typing import Optional, Tuple, TypedDict from algorithms.objects.categories import AnyCategory, Category @@ -579,22 +580,21 @@ def condition_to_model(self, model: cp_model.CpModel, user: User, courses: list[ # just aggregate the conditions together, they are already all simultaneously asserted return sum((condition.condition_to_model(model, user, courses, course_variable) for condition in self.conditions), []) case Logic.OR: - or_constraints: list[cp_model.Constraint] = sum(( + condition_truths = [model.new_bool_var(f"condition_{index}") for index, _ in enumerate(self.conditions)] + + # Build each child branch independently, then flatten the result. + branch_constraints = [ condition.condition_to_model(model, user, courses, course_variable) for condition in self.conditions - ), []) - or_opposite_constraints: list[cp_model.Constraint] = sum((condition.condition_negation(model, user, courses, course_variable) for condition in self.conditions), []) - boolean_vars = [model.new_bool_var(f"condition_{i}") for i in range(len(or_constraints))] - # Create a boolean for each condition in the OR group. This channeling constraint ensures: - # - If condition_satisfied=true, the condition's constraint is enforced - # - If condition_satisfied=false, the condition's negation is enforced - # Then add_bool_or ensures at least one of these booleans is true (OR logic). - # https://developers.google.com/optimization/cp/channeling - for constraint, negation, boolean in zip(or_constraints, or_opposite_constraints, boolean_vars): - constraint.only_enforce_if(boolean) - negation.only_enforce_if(boolean.Not()) - model.add_bool_or(boolean_vars) - return or_constraints + ] + + for condition_truth, constraints in zip(condition_truths, branch_constraints): + for constraint in constraints: + constraint.only_enforce_if(condition_truth) + + # one of our constraints must be met + model.add_bool_or(condition_truths) + return list(chain.from_iterable(branch_constraints)) def condition_negation(self, model: cp_model.CpModel, user: User, courses: list[Tuple[cp_model.IntVar, Course]], course_variable: cp_model.IntVar) -> list[cp_model.Constraint]: if len(self.conditions) == 0: diff --git a/backend/algorithms/tests/test_autoplanning.py b/backend/algorithms/tests/test_autoplanning.py index 2c0bf3c08..bbb1d1c70 100644 --- a/backend/algorithms/tests/test_autoplanning.py +++ b/backend/algorithms/tests/test_autoplanning.py @@ -1,6 +1,7 @@ from typing import Optional from pytest import raises from algorithms.autoplanning import autoplan, terms_between +from algorithms.create import create_condition from algorithms.objects.course import Course from algorithms.objects.user import User from algorithms.validate_term_planner import RawUserPlan, validate_terms @@ -80,6 +81,41 @@ def test_more_complex_prereqs(): ["COMPBH"] ) + +def test_comp4128_requires_comp3121_ordering_when_no_comp3821(): + """Regression: OR branches with nested AND must be enforced as whole branches.""" + prereq_code = "COMP9998" + target_code = "COMP9999" + target_condition = create_condition([ + "(", + "COMP8888", + "||", + "(", + prereq_code, + "&&", + "75WAM", + ")", + ")", + ]) + + results = autoplan( + [ + Course(prereq_code, create_condition(["(", ")"]), 80, 6, {2026: [1, 2, 3]}), + Course(target_code, target_condition, 80, 6, {2026: [1, 2, 3]}), + ], + User({ + "program": "3778", + "specialisations": ["COMPA1"], + "courses": {}, + }), + (2026, 0), + (2026, 3), + [12, 12, 12, 12], + ) + + placements = {course_name: term for course_name, term in results} + assert terms_between((2026, 0), placements[prereq_code]) < terms_between((2026, 0), placements[target_code]) + def test_infeasable(): with raises(Exception): # no terms have space diff --git a/backend/server/tests/planner/test_autoplanning.py b/backend/server/tests/planner/test_autoplanning.py index 256fcd3a1..4f6e25c5b 100644 --- a/backend/server/tests/planner/test_autoplanning.py +++ b/backend/server/tests/planner/test_autoplanning.py @@ -71,3 +71,94 @@ def test_autoplanning_generic(): assert x.status_code == 200 body = x.json() assert 'plan' in body + + +def _find_course_term_index(planner_years: list[dict[str, list[str]]], course_code: str) -> int: + for year_index, year in enumerate(planner_years): + for term_index in range(4): + if course_code in year[f'T{term_index}']: + return year_index * 4 + term_index + raise AssertionError(f"{course_code} was not found in planner years") + + +def test_autoplanning_places_comp3121_before_comp4128(): + clear() + token = get_token() + headers = get_token_headers(token) + + import_res = requests.put('http://127.0.0.1:8000/user/import', json={ + "degree": { + "programCode": "3778", + "specs": [ + "COMPA1" + ] + }, + "courses": { + "COMP1511": {"mark": None, "ignoreFromProgression": False}, + "COMP1521": {"mark": None, "ignoreFromProgression": False}, + "COMP2521": {"mark": None, "ignoreFromProgression": False}, + "COMP3121": {"mark": None, "ignoreFromProgression": False}, + "COMP4128": {"mark": None, "ignoreFromProgression": False}, + "MATH1081": {"mark": None, "ignoreFromProgression": False} + }, + "planner": { + "years": [ + { + "T0": [], + "T1": [], + "T2": [], + "T3": [] + }, + { + "T0": [], + "T1": [], + "T2": [], + "T3": [] + }, + { + "T0": [], + "T1": [], + "T2": [], + "T3": [] + }, + { + "T0": [], + "T1": [], + "T2": [], + "T3": [] + } + ], + "unplanned": [ + "COMP1511", + "COMP1521", + "COMP2521", + "COMP3121", + "COMP4128", + "MATH1081" + ], + "startYear": 2020, + "isSummerEnabled": False, + "lockedTerms": {} + }, + "settings": { + "showMarks": True, + "hiddenYears": [] + } + }, headers=headers) + assert import_res.status_code == 200 + + autoplan_res = requests.post( + 'http://127.0.0.1:8000/planner/autoplan', + json={'endTime': [2026, 3]}, + headers=headers, + ) + assert autoplan_res.status_code == 200 + + user_res = requests.get('http://127.0.0.1:8000/user/data/all', headers=headers) + assert user_res.status_code == 200 + + planner_years = user_res.json()['planner']['years'] + comp3121_term = _find_course_term_index(planner_years, 'COMP3121') + comp4128_term = _find_course_term_index(planner_years, 'COMP4128') + + assert comp3121_term < comp4128_term