From 8612ecaa23fef730145afef37009eb1f1cd29d77 Mon Sep 17 00:00:00 2001 From: Matthias Lehner <143808484+matthiaslehnertum@users.noreply.github.com> Date: Wed, 3 Apr 2024 11:10:53 +0200 Subject: [PATCH] `Modeling Exercises`: Filter out non-referenceable element IDs (#237) --- .../generate_suggestions.py | 36 +++++++++++++++---- .../helpers/serializers/bpmn_serializer.py | 10 ++++++ .../module_modelling_llm/helpers/utils.py | 13 +++++++ 3 files changed, 52 insertions(+), 7 deletions(-) diff --git a/module_modelling_llm/module_modelling_llm/generate_suggestions.py b/module_modelling_llm/module_modelling_llm/generate_suggestions.py index ce662a166..547a69d76 100644 --- a/module_modelling_llm/module_modelling_llm/generate_suggestions.py +++ b/module_modelling_llm/module_modelling_llm/generate_suggestions.py @@ -15,7 +15,7 @@ ) from module_modelling_llm.helpers.models.diagram_types import DiagramType from module_modelling_llm.helpers.serializers.diagram_model_serializer import DiagramModelSerializer -from module_modelling_llm.helpers.utils import format_grading_instructions +from module_modelling_llm.helpers.utils import format_grading_instructions, get_elements from module_modelling_llm.prompts.submission_format.submission_format_remarks import get_submission_format_remarks @@ -41,8 +41,28 @@ class Config: title = "Assessment" +def filter_ids_for_model(ids: List[str], model: dict) -> List[str]: + """ + Filter a list of element ids based on whether a corresponding element is present in a given diagram model. + :param ids: List of ids that should be filtered + :param model: Diagram model in which elements with the given ids should be contained + :return The filtered list of IDs + """ + elements: list[dict] = get_elements(model) + model_ids: set[str] = {str(element.get("id")) for element in elements} + return list(filter(lambda id: id in model_ids, ids)) + + async def generate_suggestions(exercise: Exercise, submission: Submission, config: BasicApproachConfig, debug: bool) -> \ List[Feedback]: + """ + Generate feedback suggestions for modeling exercise submissions + :param exercise: The exercise for which a submission is assessed + :param submission: The submission that is assessed + :param config: A configuration object for the feedback module + :param debug: Indicates whether additional debugging information should be provided + :return: A list of feedback items for the assessed submission + """ model = config.model.get_model() # type: ignore[attr-defined] serialized_example_solution = None @@ -52,7 +72,7 @@ async def generate_suggestions(exercise: Exercise, submission: Submission, confi serialized_example_solution, _ = DiagramModelSerializer.serialize_model(example_solution_diagram) submission_diagram = json.loads(submission.model) - submission_format_remarks = get_submission_format_remarks(DiagramType[submission_diagram.get("type")]) + submission_format_remarks = get_submission_format_remarks(submission_diagram.get("type")) # Having the LLM reference IDs that a specific feedback item applies to seems to work a lot more reliable with # shorter IDs, especially if they are prefixed with "id_". We therefore map the UUIDs used in Apollon diagrams to @@ -125,16 +145,18 @@ async def generate_suggestions(exercise: Exercise, submission: Submission, confi feedbacks = [] for feedback in result.feedbacks: grading_instruction_id = feedback.grading_instruction_id if feedback.grading_instruction_id in grading_instruction_ids else None + element_ids = list( + map(lambda element_id: reverse_id_map[ + element_id.strip() + ] if reverse_id_map else element_id.strip(), feedback.element_ids.split(",")) + ) if feedback.element_ids else [] + feedbacks.append(Feedback( exercise_id=exercise.id, submission_id=submission.id, title=feedback.title, description=feedback.description, - element_ids=list( - map(lambda element_id: reverse_id_map[ - element_id.strip() - ] if reverse_id_map else element_id.strip(), feedback.element_ids.split(",")) - ) if feedback.element_ids else [], + element_ids=filter_ids_for_model(element_ids, submission_diagram), credits=feedback.credits, structured_grading_instruction_id=grading_instruction_id, meta={} diff --git a/module_modelling_llm/module_modelling_llm/helpers/serializers/bpmn_serializer.py b/module_modelling_llm/module_modelling_llm/helpers/serializers/bpmn_serializer.py index ad10bc182..8a5f31934 100644 --- a/module_modelling_llm/module_modelling_llm/helpers/serializers/bpmn_serializer.py +++ b/module_modelling_llm/module_modelling_llm/helpers/serializers/bpmn_serializer.py @@ -120,6 +120,12 @@ def get_reverse_id_map(self) -> dict[str, str]: """ return {value: key for key, value in self.id_map.items()} + def reset(self): + """ + Reset the IDShortener's internal ID map + """ + self.id_map = {} + class BPMNSerializer: __start_event_definition_map = { @@ -774,6 +780,10 @@ def serialize(self, model: dict, omit_layout_info: bool = False) -> ElementTree. :return: An Elementtree Element representing the serialized BPMN diagram """ + if self.id_shortener: + # The IDShortener is reset to ensure no prior ID mappings are cached + self.id_shortener.reset() + definitions: ElementTree.Element = ElementTree.Element(self.__prefix_tag("definitions", self.__bpmn_prefix)) definitions.set("id", "Definition") diff --git a/module_modelling_llm/module_modelling_llm/helpers/utils.py b/module_modelling_llm/module_modelling_llm/helpers/utils.py index 5ca295589..a7078dc6f 100644 --- a/module_modelling_llm/module_modelling_llm/helpers/utils.py +++ b/module_modelling_llm/module_modelling_llm/helpers/utils.py @@ -46,3 +46,16 @@ def format_grading_instructions(grading_instructions: Optional[str], result += "\n" return result.strip() + + +def get_elements(model: dict) -> list[dict]: + """ + Helper method to retrieve the elements of a model backwards compatible with Apollon version 2 diagrams + """ + + elements: list | dict = model.get("elements", {}) + + if isinstance(elements, list): + return elements + + return list(elements.values())