From 2ce93a029496b262c6439c41a309f2a854a6bce9 Mon Sep 17 00:00:00 2001 From: Abhik Sarkar Date: Sun, 21 Jun 2026 19:45:41 +0530 Subject: [PATCH] fix: correct topic-complete header and welcome copy (CodeRabbit on #31) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the real findings from CodeRabbit's review of #31: - Major: topic-only completion called OutputPanel.show_final(), which hardcodes the 'All exercises complete' header — misleading when only the current topic is done. Added show_topic_complete() (shared body factored into _show_complete) with a 'Topic complete' header, and used it in both the on-mount already-complete path and the post-pass else branch. The whole-curriculum celebration still uses show_final(). - Minor: welcome copy said 'Save -- ... as you type', which is contradictory and wrong for v0.4.0 (the built-in editor reruns checks as you type, no manual save). Copy and its test updated. - Minor: added 'from __future__ import annotations' to test_welcome_pilot.py per repo convention. Skipped the RUF012 nit on WelcomeScreen.BINDINGS: a plain list matches every other screen in the codebase (Textual idiom) and the rule is not enforced. Tests: 2 new output-panel header tests; full suite 147 passed on 3.9 and 3.13. --- pythonlings/screens/track.py | 4 ++-- pythonlings/screens/welcome.py | 2 +- pythonlings/widgets/output_panel.py | 11 +++++++++-- tests/tui/test_output_panel.py | 26 ++++++++++++++++++++++++++ tests/tui/test_welcome_pilot.py | 2 ++ tests/unit/test_welcome.py | 5 ++++- 6 files changed, 44 insertions(+), 6 deletions(-) diff --git a/pythonlings/screens/track.py b/pythonlings/screens/track.py index fc303c1..4f6bafa 100644 --- a/pythonlings/screens/track.py +++ b/pythonlings/screens/track.py @@ -68,7 +68,7 @@ def on_mount(self) -> None: self.current = self._initial_exercise() self._render_state() if self.current is None: - self.query_one(OutputPanel).show_final( + self.query_one(OutputPanel).show_topic_complete( f"Topic '{self.topic}' complete." ) return @@ -198,7 +198,7 @@ def _apply_result(self, exercise: Exercise, result: RunResult) -> None: celebration_message(len(all_exercises)) ) else: - self.query_one(OutputPanel).show_final( + self.query_one(OutputPanel).show_topic_complete( f"Topic '{self.topic}' complete — press F4 for topics." ) return diff --git a/pythonlings/screens/welcome.py b/pythonlings/screens/welcome.py index f043285..2cf7bd1 100644 --- a/pythonlings/screens/welcome.py +++ b/pythonlings/screens/welcome.py @@ -15,7 +15,7 @@ def welcome_text() -> str: return ( "You learn Python here by fixing small broken programs. The loop is:\n\n" " 1. Edit the current exercise in the built-in editor.\n" - " 2. Save -- the check reruns automatically as you type.\n" + " 2. Checks rerun automatically as you type.\n" " 3. Remove the `# I AM NOT DONE` marker to advance to the next one.\n\n" "Handy keys: F1 hint - F3 exercise list - F4 topics - " "F5 local docs - Ctrl+Q quit.\n\n" diff --git a/pythonlings/widgets/output_panel.py b/pythonlings/widgets/output_panel.py index b12b5df..2e0c573 100644 --- a/pythonlings/widgets/output_panel.py +++ b/pythonlings/widgets/output_panel.py @@ -113,12 +113,19 @@ def render_result( ) def show_final(self, message: str) -> None: - """Render the curriculum-complete screen.""" + """Render the whole-curriculum-complete screen.""" + self._show_complete("All exercises complete", message) + + def show_topic_complete(self, message: str) -> None: + """Render a single-topic-complete screen (not the whole curriculum).""" + self._show_complete("Topic complete", message) + + def _show_complete(self, header: str, message: str) -> None: self.remove_class("failed", "pending") self.add_class("passed") self.query_one("#hint", Static).remove_class("visible") self.query_one("#output-header", Static).update( - "[bold green]All exercises complete[/bold green]" + f"[bold green]{header}[/bold green]" ) self.query_one("#goal", Static).update("") self.query_one("#docs", Static).update("") diff --git a/tests/tui/test_output_panel.py b/tests/tui/test_output_panel.py index 4dcfb83..31f6ed1 100644 --- a/tests/tui/test_output_panel.py +++ b/tests/tui/test_output_panel.py @@ -126,3 +126,29 @@ async def test_full_hint_toggles_on_f1(tmp_path: Path) -> None: panel.toggle_hint("Full hint text.") await pilot.pause() assert "Full hint text." in str(panel.query_one("#hint", Static).content) + + +@pytest.mark.asyncio +async def test_show_topic_complete_uses_topic_header_not_global(tmp_path: Path) -> None: + app = _Harness() + async with app.run_test() as pilot: + await pilot.pause() + panel = app.query_one(OutputPanel) + panel.show_topic_complete("Topic 'variables' complete — press F4 for topics.") + await pilot.pause() + header = str(panel.query_one("#output-header", Static).content) + assert "Topic complete" in header + assert "All exercises complete" not in header + assert "Topic 'variables' complete" in panel.renderable_text() + + +@pytest.mark.asyncio +async def test_show_final_keeps_global_header(tmp_path: Path) -> None: + app = _Harness() + async with app.run_test() as pilot: + await pilot.pause() + panel = app.query_one(OutputPanel) + panel.show_final("🎉 done") + await pilot.pause() + header = str(panel.query_one("#output-header", Static).content) + assert "All exercises complete" in header diff --git a/tests/tui/test_welcome_pilot.py b/tests/tui/test_welcome_pilot.py index c98d5bb..ad30032 100644 --- a/tests/tui/test_welcome_pilot.py +++ b/tests/tui/test_welcome_pilot.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import shutil from pathlib import Path diff --git a/tests/unit/test_welcome.py b/tests/unit/test_welcome.py index 2786488..cc74126 100644 --- a/tests/unit/test_welcome.py +++ b/tests/unit/test_welcome.py @@ -7,5 +7,8 @@ def test_welcome_text_explains_the_loop() -> None: text = welcome_text() assert "I AM NOT DONE" in text - assert "save" in text.lower() + # v0.4.0 reruns checks as you type in the built-in editor — no manual save, + # so the copy must not tell users to "save". + assert "as you type" in text.lower() + assert "save" not in text.lower() assert "F1" in text