-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor Feedback Reference #354
base: develop
Are you sure you want to change the base?
Conversation
…apping and uniqueness resolution
…tum/Athena into feature/modeling/reference
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, left a couple of comments
from .feedback import Feedback | ||
|
||
|
||
class ModelingFeedback(Feedback): | ||
"""Feedback on a modeling exercise.""" | ||
|
||
element_ids: Optional[List[str]] = Field([], description="referenced diagram element IDs", example=["id_1"]) | ||
reference: Optional[str] = Field(None, description="reference to the diagram element", example="ClassAttribute:5a337bdf-da00-4bd0-a6f0-78ba5b84330e") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason for the type none?
""" | ||
# Remove visibility symbols and leading/trailing spaces | ||
name = full_name.lstrip('+-#~ ').strip() | ||
# For attributes, split on ':' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can it be the case that both are present?
from pydantic import BaseModel, Field | ||
|
||
class FeedbackModel(BaseModel): | ||
title: str = Field(description="Very short title, i.e. feedback category or similar", example="Logic Error") | ||
description: str = Field(description="Feedback description") | ||
element_names: Optional[List[str]] = Field(description="Referenced diagram element names, and relations (R<number>) or empty if unreferenced") | ||
element_name: Optional[str] = Field(description="Referenced diagram element, attribute names, and relations (use format: <ClassName>, <ClassName>.<AttributeName>, <ClassName>.<MethodName>, R<number>), or leave empty if unreferenced") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we don't have a list of feedbacks any more, do we use multiple instances of FeedbackModel somehow now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code lgtm generally. Is there a reason it does not work for the BPMN exercises on TS1 example data?
@EneaGore This should be related to the playground token config, if you increase the |
…tum/Athena into feature/modeling/reference
Motivation and Context
This PR introduces two changes:
Simplified Feedback Schema: The feedback schema has been simplified by replacing the list of
element_ids
with a singlereference
field. This change avoids unnecessary conversions on the Artemis side and aligns the reference format with how manual references are handled in Artemis.Enabled Detailed Element Referencing: Previously, feedback could only reference entire elements. With this update, feedback can now reference specific attributes, methods within elements and also references between elements.
Description
1. Simplified Feedback Schema
DBModelingFeedback
andModelingFeedback
models to replace theelement_ids
field with areference
field that accepts a single string in the formatelementType:elementId
(e.g.,ClassAttribute:5a337bdf-da00-4bd0-a6f0-78ba5b84330e
).2. Enabled Detailed Element Referencing
Enhanced Parsers and Transformers:
ApollonJSONTransformer
and its parsers to extract IDs and types of attributes, methods, and relations.Element
andUMLParser
classes to support detailed mapping between element names and their IDs/types.Adjusted Prompts and Utility Functions:
convert_to_athana_feedback_model
andget_exercise_model
to handle the newreference
field and support the enhanced referencing capabilities.Steps for Testing
API Testing:
/feedback_suggestions
endpoint of the module.Verify Feedback Generation:
reference
field in the formatelementType:elementId
.Check Granular Referencing:
ClassAttribute:[attribute-id]
orClassMethod:[method-id]
.Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Screenshots
Example of how attribute and method referencing is displayed in the feedback UI on Artemis: