-
Notifications
You must be signed in to change notification settings - Fork 0
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
Pipeline
: Add a pipeline for course level inquiries
#101
Conversation
WalkthroughThe recent changes involve enhancing the educational system's data representation and management by introducing new data transfer objects (DTOs) and models related to competencies, exercises, lecture units, and student metrics. These changes aim to provide a more comprehensive view of student progress, competency levels, and exercise completion within the system. Changes
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (9)
Files skipped from review due to trivial changes (1)
Files skipped from review as they are similar to previous changes (8)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 0
Out of diff range and nitpick comments (1)
app/domain/__init__.py (1)
Line range hint
1-9
: Consider managing unused imports to prevent potential confusion and maintain cleaner code.+ __all__ = [ + "IrisErrorResponseDTO", + "PipelineExecutionDTO", + "PipelineExecutionSettingsDTO", + "TutorChatPipelineExecutionDTO", + "CourseChatPipelineExecutionDTO", + "PyrisMessage", + "IrisMessageRole" + ]
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.
Actionable comments posted: 4
Out of diff range and nitpick comments (3)
app/domain/__init__.py (3)
Line range hint
1-1
: Consider removing or using theIrisErrorResponseDTO
import.This import is currently unused. If it's not needed, it should be removed to clean up the codebase. If it's intended for external use, consider adding it to the
__all__
list to explicitly indicate its purpose.
Line range hint
2-2
: Consider removing or using thePipelineExecutionDTO
import.Similar to the previous comment, this import is unused. Removing it or adding it to
__all__
would help clarify its role in the module.
Line range hint
3-3
: Consider removing or using thePipelineExecutionSettingsDTO
import.This import is unused and should either be removed or added to
__all__
if it's meant to be available for external modules.
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.
Actionable comments posted: 3
Out of diff range and nitpick comments (3)
app/domain/pipeline_execution_dto.py (3)
Line range hint
1-1
: Remove unused imports (List
,Optional
) to clean up the code.- from typing import List, Optional
Line range hint
3-3
: Remove unused import (Field
) to clean up the code.- from pydantic import BaseModel, Field + from pydantic import BaseModel
Line range hint
5-6
: Remove unused imports (PipelineExecutionSettingsDTO
,StageDTO
) to clean up the code.- from app.domain.pipeline_execution_settings_dto import PipelineExecutionSettingsDTO - from app.domain.status.stage_dto import StageDTO
from .error_response_dto import IrisErrorResponseDTO | ||
from .pipeline_execution_dto import PipelineExecutionDTO | ||
from .pyris_message import PyrisMessage | ||
from .pipeline_execution_settings_dto import PipelineExecutionSettingsDTO | ||
from app.domain.tutor_chat.tutor_chat_pipeline_execution_dto import ( | ||
from .chat.chat_pipeline_execution_dto import ChatPipelineExecutionDTO | ||
from .chat.chat_pipeline_execution_base_data_dto import ChatPipelineExecutionBaseDataDTO | ||
from app.domain.chat.tutor_chat.tutor_chat_pipeline_execution_dto import ( | ||
TutorChatPipelineExecutionDTO, | ||
) | ||
from app.domain.chat.course_chat.course_chat_pipeline_execution_dto import ( | ||
CourseChatPipelineExecutionDTO, | ||
) | ||
from .pyris_message import PyrisMessage, IrisMessageRole | ||
from app.domain.data import image_message_content_dto |
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.
Several imports in this file are marked as unused. Consider either removing these or explicitly including them in __all__
if they are intended for external use. This will clarify their purpose and maintain clean code.
- from .error_response_dto import IrisErrorResponseDTO
- from .pipeline_execution_dto import PipelineExecutionDTO
- from .pipeline_execution_settings_dto import PipelineExecutionSettingsDTO
- from .chat.chat_pipeline_execution_dto import ChatPipelineExecutionDTO
- from .chat.chat_pipeline_execution_base_data_dto import ChatPipelineExecutionBaseDataDTO
- from app.domain.chat.tutor_chat.tutor_chat_pipeline_execution_dto import TutorChatPipelineExecutionDTO
- from app.domain.chat.course_chat.course_chat_pipeline_execution_dto import CourseChatPipelineExecutionDTO
- from .pyris_message import PyrisMessage, IrisMessageRole
- from app.domain.data import image_message_content_dto
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
from .error_response_dto import IrisErrorResponseDTO | |
from .pipeline_execution_dto import PipelineExecutionDTO | |
from .pyris_message import PyrisMessage | |
from .pipeline_execution_settings_dto import PipelineExecutionSettingsDTO | |
from app.domain.tutor_chat.tutor_chat_pipeline_execution_dto import ( | |
from .chat.chat_pipeline_execution_dto import ChatPipelineExecutionDTO | |
from .chat.chat_pipeline_execution_base_data_dto import ChatPipelineExecutionBaseDataDTO | |
from app.domain.chat.tutor_chat.tutor_chat_pipeline_execution_dto import ( | |
TutorChatPipelineExecutionDTO, | |
) | |
from app.domain.chat.course_chat.course_chat_pipeline_execution_dto import ( | |
CourseChatPipelineExecutionDTO, | |
) | |
from .pyris_message import PyrisMessage, IrisMessageRole | |
from app.domain.data import image_message_content_dto | |
from .pyris_message import PyrisMessage |
from fastapi import FastAPI, Request, status | ||
from fastapi.exceptions import RequestValidationError |
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.
Remove the redundant imports of FastAPI
and RequestValidationError
as they are already imported at the beginning of the file. This will prevent confusion and keep the import section clean.
- from fastapi import FastAPI, Request, status
- from fastapi.exceptions import RequestValidationError
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
from fastapi import FastAPI, Request, status | |
from fastapi.exceptions import RequestValidationError |
from urllib.request import Request | ||
|
||
from fastapi import APIRouter, status, Response, Depends, FastAPI | ||
from fastapi.exceptions import RequestValidationError | ||
from starlette.responses import JSONResponse |
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.
Remove the unused imports to clean up the code. These imports are not utilized in the file and clutter the import section.
- from urllib.request import Request
- from fastapi import FastAPI
- from fastapi.exceptions import RequestValidationError
- from starlette.responses import JSONResponse
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
from urllib.request import Request | |
from fastapi import APIRouter, status, Response, Depends, FastAPI | |
from fastapi.exceptions import RequestValidationError | |
from starlette.responses import JSONResponse | |
from fastapi import APIRouter, status, Response, Depends |
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.
Actionable comments posted: 1
# app/domain/data/metrics/competency_student_metrics_dto.py | ||
|
||
from pydantic import BaseModel, Field | ||
from typing import Dict, Set, Optional | ||
|
||
from app.domain.data.metrics.competency_information_dto import CompetencyInformationDTO | ||
|
||
|
||
class CompetencyStudentMetricsDTO(BaseModel): | ||
competency_information: Dict[int, CompetencyInformationDTO] = Field(alias="competencyInformation") | ||
exercises: Dict[int, Set[int]] | ||
lecture_units: Dict[int, Set[int]] = Field(alias="lectureUnits") | ||
progress: Dict[int, float] | ||
confidence: Dict[int, float] | ||
|
||
class Config: | ||
allow_population_by_field_name = True |
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.
LGTM! The CompetencyStudentMetricsDTO
class is well-defined and follows best practices. However, the Optional
import is unused and can be removed.
- from typing import Dict, Set, Optional
+ from typing import Dict, Set
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
# app/domain/data/metrics/competency_student_metrics_dto.py | |
from pydantic import BaseModel, Field | |
from typing import Dict, Set, Optional | |
from app.domain.data.metrics.competency_information_dto import CompetencyInformationDTO | |
class CompetencyStudentMetricsDTO(BaseModel): | |
competency_information: Dict[int, CompetencyInformationDTO] = Field(alias="competencyInformation") | |
exercises: Dict[int, Set[int]] | |
lecture_units: Dict[int, Set[int]] = Field(alias="lectureUnits") | |
progress: Dict[int, float] | |
confidence: Dict[int, float] | |
class Config: | |
allow_population_by_field_name = True | |
# app/domain/data/metrics/competency_student_metrics_dto.py | |
from pydantic import BaseModel, Field | |
from typing import Dict, Set | |
from app.domain.data.metrics.competency_information_dto import CompetencyInformationDTO | |
class CompetencyStudentMetricsDTO(BaseModel): | |
competency_information: Dict[int, CompetencyInformationDTO] = Field(alias="competencyInformation") | |
exercises: Dict[int, Set[int]] | |
lecture_units: Dict[int, Set[int]] = Field(alias="lectureUnits") | |
progress: Dict[int, float] | |
confidence: Dict[int, float] | |
class Config: | |
allow_population_by_field_name = True |
Motivation
In the future we want IRIS to answer course-related questions
Description
CourseChatPipeline
receives information about the course and answers students' questions about organizational aspects of the course and the course content.Summary by CodeRabbit