Skip to content
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

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

LeonWehrhahn
Copy link
Contributor

@LeonWehrhahn LeonWehrhahn commented Nov 16, 2024

Motivation and Context

This PR introduces two changes:

  1. Simplified Feedback Schema: The feedback schema has been simplified by replacing the list of element_ids with a single reference field. This change avoids unnecessary conversions on the Artemis side and aligns the reference format with how manual references are handled in Artemis.

  2. 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

  • Updated Data Models: Modified the DBModelingFeedback and ModelingFeedback models to replace the element_ids field with a reference field that accepts a single string in the format elementType:elementId (e.g., ClassAttribute:5a337bdf-da00-4bd0-a6f0-78ba5b84330e).

2. Enabled Detailed Element Referencing

  • Enhanced Parsers and Transformers:

    • Updated the ApollonJSONTransformer and its parsers to extract IDs and types of attributes, methods, and relations.
    • Modified the Element and UMLParser classes to support detailed mapping between element names and their IDs/types.
  • Adjusted Prompts and Utility Functions:

    • Updated the feedback generation prompt to instruct the LLM to reference specific attributes, methods, and relations using the new format.
    • Modified utility functions like convert_to_athana_feedback_model and get_exercise_model to handle the new reference field and support the enhanced referencing capabilities.

Steps for Testing

  1. API Testing:

    • Make direct API calls to the /feedback_suggestions endpoint of the module.
    • Use submissions that include classes with attributes, methods, and relations.
  2. Verify Feedback Generation:

    • Ensure that feedback is generated correctly without errors.
    • Check that the feedback includes the new reference field in the format elementType:elementId.
  3. Check Granular Referencing:

    • Verify that feedback can reference specific attributes, methods, and relations.
      • For example, feedback might reference ClassAttribute:[attribute-id] or ClassMethod:[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:
image

@github-actions github-actions bot removed the deploy:athena-test1 Athena Test Server 1 label Nov 16, 2024
@LeonWehrhahn LeonWehrhahn added the deploy:athena-test1 Athena Test Server 1 label Nov 16, 2024
@github-actions github-actions bot added the lock:athena-test1 Is currently deployed to Athena Test Server 1 label Nov 16, 2024
@LeonWehrhahn LeonWehrhahn temporarily deployed to athena-test1.ase.cit.tum.de November 16, 2024 19:02 — with GitHub Actions Inactive
@github-actions github-actions bot removed the deploy:athena-test1 Athena Test Server 1 label Nov 16, 2024
@LeonWehrhahn LeonWehrhahn added deploy:athena-test1 Athena Test Server 1 and removed lock:athena-test1 Is currently deployed to Athena Test Server 1 labels Nov 18, 2024
@github-actions github-actions bot removed the deploy:athena-test1 Athena Test Server 1 label Nov 18, 2024
Copy link

⚠️ Unable to deploy to test servers ⚠️

The docker build needs to run through before deploying.

@github-actions github-actions bot added the deployment-error Added by deployment workflows if an error occured label Nov 18, 2024
@LeonWehrhahn LeonWehrhahn changed the title Refactor feedback reference Refactor Feedback Reference Nov 18, 2024
@LeonWehrhahn LeonWehrhahn added deploy:athena-test1 Athena Test Server 1 and removed deployment-error Added by deployment workflows if an error occured labels Nov 19, 2024
@LeonWehrhahn LeonWehrhahn temporarily deployed to athena-test1.ase.cit.tum.de November 19, 2024 10:07 — with GitHub Actions Inactive
@github-actions github-actions bot added lock:athena-test1 Is currently deployed to Athena Test Server 1 and removed deploy:athena-test1 Athena Test Server 1 labels Nov 19, 2024
@github-actions github-actions bot removed the lock:athena-test1 Is currently deployed to Athena Test Server 1 label Nov 22, 2024
DominikRemo
DominikRemo previously approved these changes Nov 24, 2024
Copy link
Contributor

@DominikRemo DominikRemo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM

Copy link
Contributor

@dmytropolityka dmytropolityka left a 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")
Copy link
Contributor

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 ':'
Copy link
Contributor

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")
Copy link
Contributor

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?

Copy link
Contributor

@EneaGore EneaGore left a 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?

@LeonWehrhahn
Copy link
Contributor Author

LeonWehrhahn commented Nov 25, 2024

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 Max Tokens the error should go away.

@LeonWehrhahn LeonWehrhahn added the deploy:athena-test1 Athena Test Server 1 label Nov 28, 2024
@github-actions github-actions bot added lock:athena-test1 Is currently deployed to Athena Test Server 1 and removed deploy:athena-test1 Athena Test Server 1 labels Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lock:athena-test1 Is currently deployed to Athena Test Server 1 ready to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants