diff --git a/tutorial/tests/testsuite/ai_helpers.py b/tutorial/tests/testsuite/ai_helpers.py index fce40504..def805fa 100644 --- a/tutorial/tests/testsuite/ai_helpers.py +++ b/tutorial/tests/testsuite/ai_helpers.py @@ -37,6 +37,13 @@ logger = logging.getLogger() +# Utility: Markdown to HTML converter +def to_html(text: t.Any) -> str: + """Markdown to HTML converter""" + return md.markdown(str(text)) + + +# Base models for an AI Explanation class ExplanationStep(BaseModel): """A single step in the explanation""" @@ -60,6 +67,25 @@ class Explanation(BaseModel): hints: t.List[str] +# Base model for an AI Feedback +class CodeReviewPoint(BaseModel): + """A single point in the code review feedback""" + + category: str # e.g. "Readability", "Style", "Performance" + title: str + details: str + hint: t.Optional[str] + + +class SuccessFeedback(BaseModel): + """A structured code review feedback""" + + overview: str # General assessment + strengths: t.List[str] # What's good about your solution + review_points: t.List[CodeReviewPoint] # Areas for improvement + suggestions: t.List[str] # Suggestions for improvement + + class OpenAIWrapper: """A simple API wrapper adapted for IPython environments""" @@ -193,7 +219,7 @@ def change_model(self, model: str) -> None: wait=wait_fixed(10) + wait_random(0, 5), ) def get_chat_response( - self, query: str, *args, **kwargs + self, query: str, response_format: t.Type[BaseModel], *args, **kwargs ) -> ParsedChatCompletionMessage | ChatCompletionMessage: """Fetch a completion from the chat model""" system_prompt = ( @@ -216,7 +242,7 @@ def get_chat_response( response = self.client.beta.chat.completions.parse( model=self.model, messages=messages, - response_format=Explanation, + response_format=response_format, **kwargs, ) except openai.APIError: @@ -240,6 +266,8 @@ class ButtonState(Enum): class AIExplanation: """Class representing an AI-generated explanation""" + reponse_format: t.ClassVar[t.Type[BaseModel]] = Explanation + _STYLES = """ """ + _BUTTON_TITLE = "Get AI Explanation" + _WIDGET_TITLE = "🤖 Explain With AI" def __init__( self, @@ -352,7 +382,7 @@ def __init__( # The button widget for fetching the explanation self._button_styles = { ButtonState.READY: { - "description": "Get AI Explanation", + "description": f"{self._BUTTON_TITLE}", "icon": "search", "disabled": False, }, @@ -414,7 +444,7 @@ def render(self) -> widgets.Widget: header_html = widgets.HTML( '
' - '🤖 Explain With AI' + f'{self._WIDGET_TITLE}' "
" ) @@ -524,7 +554,6 @@ def update_timer(): def _fetch_explanation(self) -> None: """Fetch the explanation from OpenAI API""" - from .helpers import IPytestOutcome logger.debug("Attempting to fetch explanation from OpenAI API.") @@ -533,33 +562,15 @@ def _fetch_explanation(self) -> None: self._update_button_state(ButtonState.LOADING) - if self.exception: - traceback_str = "".join(traceback.format_exception_only(self.exception)) - logger.debug("Formatted traceback: %s", traceback_str) - else: - traceback_str = "No traceback available." - with self._output: self._output.clear_output() try: - # assert self.ipytest_result.function is not None - match self.ipytest_result.status: - case IPytestOutcome.FINISHED if self.ipytest_result.function is not None: - self.query_params( - function_code=self.ipytest_result.function.source_code, - docstring=self.ipytest_result.function.implementation.__doc__, - traceback=traceback_str, - ) - case _: - self.query_params( - function_code=self.ipytest_result.cell_content, - docstring="(Find it in the function's definition above.)", - traceback=traceback_str, - ) + self._prepare_query() response = self.openai_client.get_chat_response( self.query, + response_format=self.reponse_format, temperature=0.2, ) @@ -582,16 +593,34 @@ def _fetch_explanation(self) -> None: else: self._update_button_state(ButtonState.READY) + def _prepare_query(self) -> None: + """Prepare the query parameters before fetching the explanation""" + from .helpers import IPytestOutcome + + if self.exception: + traceback_str = "".join(traceback.format_exception_only(self.exception)) + logger.debug("Formatted traceback: %s", traceback_str) + else: + traceback_str = "No traceback available." + + match self.ipytest_result.status: + case IPytestOutcome.FINISHED if self.ipytest_result.function is not None: + self.query_params( + function_code=self.ipytest_result.function.source_code, + docstring=self.ipytest_result.function.implementation.__doc__, + traceback=traceback_str, + ) + case _: + self.query_params( + function_code=self.ipytest_result.cell_content, + docstring="(Find it in the function's definition above.)", + traceback=traceback_str, + ) + def _format_explanation( self, chat_response: ParsedChatCompletionMessage | ChatCompletionMessage ) -> t.Optional[t.List[t.Any]]: """Format the explanation response for display""" - - # Initialize the Markdown to HTML converter - def to_html(text: t.Any) -> str: - """Markdown to HTML converter""" - return md.markdown(str(text)) - # Reset the explanation object explanation = None @@ -674,3 +703,186 @@ def to_html(text: t.Any) -> str: logger.debug("Failed to parse explanation.") return None + + +class AISuccessFeedback(AIExplanation): + """A subclass of AIExplanation for successful feedback""" + + reponse_format = SuccessFeedback + + __STYLES = ( + AIExplanation._STYLES + + """ + + """ + ) + _WIDGET_TITLE = "🎯 Code Review" + _BUTTON_TITLE = "Get Code Review" + + def __init__( + self, + ipytest_result: "IPytestResult", + openai_client: "OpenAIWrapper", + reference_solution: t.Optional[str] = None, + wait_time: int = 30, + ) -> None: + super().__init__( + ipytest_result=ipytest_result, + openai_client=openai_client, + wait_time=wait_time, + ) + + self.reference_solution = reference_solution + + # Different query templates based on whether we have a valid reference solution + if reference_solution: + self._query_template = ( + "I wrote the following Python function:\n\n" + "{function_code}\n\n" + "Here's the reference solution:\n\n" + "{reference_solution}\n\n" + "All tests have passed. Please review my solution, analyzing these distinct aspects:\n" + "1. Implementation: choice of data structures, algorithms, built-in functions\n" + "2. Performance: time/space complexity, resource usage\n" + "3. Design Pattern: code organization, abstraction, reusability\n\n" + "For each aspect:\n" + "- Highlight what works well\n" + "- Point out potential improvements\n" + "- Provide concrete examples where relevant\n" + "- Compare with the reference solution when meaningful\n\n" + "Include specific suggestions that could make the code more efficient or maintainable." + ) + else: + self._query_template = ( + "I wrote the following Python function:\n\n" + "{function_code}\n\n" + "All tests have passed. Please review my solution, analyzing these distinct aspects:\n" + "1. Implementation: choice of data structures, algorithms, built-in functions\n" + "2. Performance: time/space complexity, resource usage\n" + "3. Design Pattern: code organization, abstraction, reusability\n\n" + "For each aspect:\n" + "- Highlight what works well\n" + "- Point out potential improvements\n" + "- Provide concrete examples where relevant\n\n" + "Include specific suggestions that could make the code more efficient or maintainable." + ) + + def _prepare_query(self) -> None: + """Include the reference solution in the query, if available""" + super()._prepare_query() + if self.reference_solution: + self.query_params(reference_solution=self.reference_solution) + + def _format_explanation( + self, chat_response: ParsedChatCompletionMessage | ChatCompletionMessage + ) -> t.Optional[t.List[t.Any]]: + """Format the success feedback response for display""" + widgets_list = [] + + if isinstance(chat_response, ParsedChatCompletionMessage): + feedback = chat_response.parsed + if not isinstance(feedback, SuccessFeedback): + return super()._format_explanation(chat_response) + + # Overview in alert-style block + overview_html = ( + '
' + f'

Overview

{to_html(feedback.overview)}' + "
" + ) + + widgets_list.append(widgets.HTML(overview_html)) + + # Strengths in alert-style block + if feedback.strengths: + strengths_html = ( + '
' + '

✨ Strengths

' + '
" + ) + + widgets_list.append(widgets.HTML(strengths_html)) + + # Review points as accordions + for point in feedback.review_points: + point_output = widgets.Output() + point_output.append_display_data( + widgets.HTML( + f'
{to_html(point.details)}
' + ) + ) + + if point.hint: + hint_html = ( + '
' + "💡 Hint: " + f"{to_html(point.hint)}" + "
" + ) + point_output.append_display_data(widgets.HTML(hint_html)) + + accordion = widgets.Accordion( + children=[point_output], + titles=(f"{point.category}: {point.title}",), + ) + widgets_list.append(accordion) + + # Suggestions as list + if feedback.suggestions: + suggestions_html = ( + '
' + '

💡 Suggestions

' + '
" + ) + widgets_list.append(widgets.HTML(suggestions_html)) + + elif ( + isinstance(chat_response, ChatCompletionMessage) + and chat_response.content is not None + ): + # Fallback to unstructured response + return super()._format_explanation(chat_response) + + return widgets_list if widgets_list else None diff --git a/tutorial/tests/testsuite/helpers.py b/tutorial/tests/testsuite/helpers.py index 5c7f17c5..0636ac87 100644 --- a/tutorial/tests/testsuite/helpers.py +++ b/tutorial/tests/testsuite/helpers.py @@ -12,7 +12,7 @@ from IPython.display import display as ipython_display from ipywidgets import HTML -from .ai_helpers import AIExplanation, OpenAIWrapper +from .ai_helpers import AIExplanation, AISuccessFeedback, OpenAIWrapper def strip_ansi_codes(text: str) -> str: @@ -577,8 +577,8 @@ def prepare_solution_cell(self) -> ipywidgets.Widget: ipython_display( HTML( '
' - '👉' - 'Proposed solution' + '👉' + 'Proposed solution' "
" ) ) @@ -598,7 +598,7 @@ def prepare_solution_cell(self) -> ipywidgets.Widget: return ipywidgets.VBox( children=[header_output, accordion], layout=ipywidgets.Layout( - margin="0", + margin="1rem 0 0 0", padding="0", ), ) @@ -678,20 +678,35 @@ def prepare_output_cell(self) -> ipywidgets.Output: for test in self.ipytest_result.test_results: output_cell.append_display_data(HTML(test.to_html())) - failed_tests = [ - test + all_passed = all( + test.outcome == TestOutcome.PASS for test in self.ipytest_result.test_results - if test.outcome != TestOutcome.PASS - ] - - if self.openai_client and failed_tests: - ai_explains = AIExplanation( - ipytest_result=self.ipytest_result, - exception=failed_tests[0].exception, - openai_client=self.openai_client, - ) + ) - output_cell.append_display_data(ai_explains.render()) + if self.openai_client: + if all_passed and self.solution: + # Add success feedback widget + success_feedback = AISuccessFeedback( + ipytest_result=self.ipytest_result, + openai_client=self.openai_client, + reference_solution=self.solution, + ) + output_cell.append_display_data(success_feedback.render()) + else: + failed_tests = [ + test + for test in self.ipytest_result.test_results + if test.outcome != TestOutcome.PASS + ] + + if failed_tests: + ai_explains = AIExplanation( + ipytest_result=self.ipytest_result, + exception=failed_tests[0].exception, + openai_client=self.openai_client, + ) + + output_cell.append_display_data(ai_explains.render()) case IPytestOutcome.SOLUTION_FUNCTION_MISSING: output_cell.append_display_data(