TP2000-1584 Fix duty sentence parser error with MAX expression

conftest.py

@@ -1777,15 +1777,15 @@ def percent_or_amount() -> DutyExpression:
 
 
 @pytest.fixture
-def plus_percent_or_amount() -> DutyExpression:
-    return factories.DutyExpressionFactory(
-        sid=4,
-        prefix="+",
-        description=f"+ % or amount",
-        duty_amount_applicability_code=ApplicabilityCode.MANDATORY,
-        measurement_unit_applicability_code=ApplicabilityCode.PERMITTED,
-        monetary_unit_applicability_code=ApplicabilityCode.PERMITTED,
-    )
+def plus_percent_or_amount() -> list[DutyExpression]:
+    kwargs = {
+        "prefix": "+",
+        "description": f"+ % or amount",
+        "duty_amount_applicability_code": ApplicabilityCode.MANDATORY,
+        "measurement_unit_applicability_code": ApplicabilityCode.PERMITTED,
+        "monetary_unit_applicability_code": ApplicabilityCode.PERMITTED,
+    }
+    return [factories.DutyExpressionFactory(sid=sid, **kwargs) for sid in [4, 19, 20]]
 
 
 @pytest.fixture
@@ -1800,6 +1800,18 @@ def plus_agri_component() -> DutyExpression:
     )
 
 
+@pytest.fixture
+def maximum() -> list[DutyExpression]:
+    kwargs = {
+        "prefix": "MAX",
+        "description": "Maximum",
+        "duty_amount_applicability_code": ApplicabilityCode.MANDATORY,
+        "measurement_unit_applicability_code": ApplicabilityCode.PERMITTED,
+        "monetary_unit_applicability_code": ApplicabilityCode.PERMITTED,
+    }
+    return [factories.DutyExpressionFactory(sid=sid, **kwargs) for sid in [17, 35]]
+
+
 @pytest.fixture
 def plus_amount_only() -> DutyExpression:
     return factories.DutyExpressionFactory(
@@ -1838,39 +1850,26 @@ def supplementary_unit() -> DutyExpression:
 
 @pytest.fixture
 def duty_expressions(
-    percent_or_amount: DutyExpression,
-    plus_percent_or_amount: DutyExpression,
-    plus_agri_component: DutyExpression,
-    plus_amount_only: DutyExpression,
-    supplementary_unit: DutyExpression,
-    nothing: DutyExpression,
+    duty_expressions_list: list[DutyExpression],
 ) -> Dict[int, DutyExpression]:
-    return {
-        d.sid: d
-        for d in [
-            percent_or_amount,
-            plus_percent_or_amount,
-            plus_agri_component,
-            plus_amount_only,
-            supplementary_unit,
-            nothing,
-        ]
-    }
+    return {d.sid: d for d in duty_expressions_list}
 
 
 @pytest.fixture
 def duty_expressions_list(
     percent_or_amount: DutyExpression,
-    plus_percent_or_amount: DutyExpression,
+    plus_percent_or_amount: list[DutyExpression],
     plus_agri_component: DutyExpression,
+    maximum: list[DutyExpression],
     plus_amount_only: DutyExpression,
     supplementary_unit: DutyExpression,
     nothing: DutyExpression,
 ) -> Sequence[DutyExpression]:
     return [
         percent_or_amount,
-        plus_percent_or_amount,
+        *plus_percent_or_amount,
         plus_agri_component,
+        *maximum,
         plus_amount_only,
         supplementary_unit,
         nothing,

measures/duty_sentence_parser.py

@@ -8,6 +8,7 @@
 from lark import Transformer
 from lark import UnexpectedInput
 
+from common.models import TrackedModel
 from measures import models
 from measures.validators import ApplicabilityCode
 
@@ -261,8 +262,8 @@ def validate_duty_expressions(self, transformed):
             .exclude(prefix__isnull=True)
             .order_by("sid")
         )
-
         duty_expression_sids = [d.sid for d in duty_expressions]
+        matched_sids = set()
         supplementary_unit = models.DutyExpression.objects.as_at(self.date).get(sid=99)
 
         if len(transformed) == 1:
@@ -287,7 +288,8 @@ def validate_duty_expressions(self, transformed):
                 .order_by("sid")
                 .first()
             )
-            if match is None:
+
+            if not self.is_valid_match(match, supplementary_unit, phrase):
                 potential_match = (
                     models.DutyExpression.objects.as_at(self.date)
                     .filter(
@@ -296,25 +298,57 @@ def validate_duty_expressions(self, transformed):
                     .order_by("sid")
                     .first()
                 )
+
+                if potential_match.sid in matched_sids:
+                    raise ValidationError(
+                        f"A duty expression cannot be used more than once in a duty sentence. Matching expression: {potential_match.description} ({potential_match.prefix})",
+                    )
+
                 raise ValidationError(
-                    f"A duty expression cannot be used more than once in a duty sentence. Matching expression: {potential_match.description} ({potential_match.prefix})",
+                    f"Duty expressions must be used in the duty sentence in ascending order of SID. Matching expression: {potential_match.description} ({potential_match.prefix}).",
                 )
 
-            # Each duty expression can only be used once in a sentence and in order of increasing
-            # SID so once we have a match, remove it from the list of duty expression sids
-            duty_expression_sids.remove(match.sid)
+            # Each duty expression can only be used once in a sentence and in ascending SID order
+            matched_sids.add(match.sid)
+            duty_expression_sids[:] = [
+                sid for sid in duty_expression_sids if sid > match.sid
+            ]
             # Update with the matching DutyExpression we found
             phrase["duty_expression"] = match
 
         transformed_duty_expression_sids = [
             phrase["duty_expression"].sid for phrase in transformed
         ]
-
         if transformed_duty_expression_sids != sorted(transformed_duty_expression_sids):
             raise ValidationError(
                 "Duty expressions must be used in the duty sentence in ascending order of SID.",
             )
 
+    def is_valid_match(
+        self,
+        match: models.DutyExpression | None,
+        supplementary_unit: models.DutyExpression,
+        phrase: dict,
+    ) -> bool:
+        """
+        Checks whether a match has been found and is valid, returning `True` if
+        so and `False` if not.
+
+        A duty amount may be mistakenly matched by prefix as a supplementary unit if all possible duty amount expression SIDs have already been applied,
+        in which case `False` is returned.
+        """
+        if match is None:
+            return False
+
+        if (
+            match == supplementary_unit
+            and "duty_amount" in phrase
+            and "measurement_unit" not in phrase
+        ):
+            return False
+
+        return True
+
     @staticmethod
     def validate_according_to_applicability_code(
         code,
@@ -327,7 +361,7 @@ def validate_according_to_applicability_code(
                 f"Duty expression {duty_expression.description} ({duty_expression.prefix}) requires a {item_name}.",
             )
         if code == ApplicabilityCode.NOT_PERMITTED and item:
-            if isinstance(item, object):
+            if isinstance(item, TrackedModel):
                 message = f"{item_name.capitalize()} {item.abbreviation} ({item.code}) cannot be used with duty expression {duty_expression.description} ({duty_expression.prefix})."
             else:
                 message = f"{item_name.capitalize()} cannot be used with duty expression {duty_expression.description} ({duty_expression.prefix})."