Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions pythonlings/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,16 @@ def __init__(
self._force_picker = force_picker

def on_mount(self) -> None:
first_launch = not self.state.seen_intro
self.push_screen(TopicPickerScreen())
target = self._startup_target()
if target is not None:
topic, exercise = target
self.push_screen(TrackScreen(topic, start_exercise=exercise))
if first_launch and target is not None:
from pythonlings.screens.welcome import WelcomeScreen

self.push_screen(WelcomeScreen())

def _startup_target(self) -> tuple[str, str | None] | None:
if self._start_topic is not None:
Expand Down
10 changes: 10 additions & 0 deletions pythonlings/core/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import json
import sys
from collections.abc import Iterable
from dataclasses import dataclass, field
from pathlib import Path

Expand Down Expand Up @@ -84,3 +85,12 @@ def next_pending(exercises: list[Exercise], completed: set[str]) -> str | None:
if ex.name not in completed:
return ex.name
return None


def completed_count(exercise_names: Iterable[str], completed: set[str]) -> int:
"""How many of `exercise_names` are completed.

Counts only names that exist in the curriculum, so stale `completed`
entries (e.g. exercises renamed or removed) cannot inflate the total.
"""
return sum(1 for name in exercise_names if name in completed)
27 changes: 27 additions & 0 deletions pythonlings/pythonlings.tcss
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,30 @@ DocsScreen {
height: 1;
color: $text;
}

WelcomeScreen {
align: center middle;
}

#welcome-window {
width: 72%;
max-width: 76;
height: auto;
padding: 1 2;
border: heavy $primary;
background: $surface;
color: $text;
}

#welcome-title {
height: 1;
}

#welcome-body {
margin: 1 0;
}

#welcome-footer {
height: 1;
color: $text-muted;
}
33 changes: 28 additions & 5 deletions pythonlings/screens/track.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

from pythonlings.core.exercise import Exercise, RunResult
from pythonlings.core.runner import run as run_exercise
from pythonlings.core.state import next_pending, save as save_state
from pythonlings.core.state import completed_count, next_pending, save as save_state
from pythonlings.widgets.editor_pane import EditorPane
from pythonlings.widgets.exercise_tree import ExerciseTree
from pythonlings.widgets.output_panel import OutputPanel
Expand All @@ -19,6 +19,17 @@
_DEBOUNCE_SECONDS = 0.6


def celebration_message(total: int) -> str:
"""Message shown when every exercise in the curriculum is complete."""
return (
f"🎉 You finished all {total} pythonlings exercises! 🎉\n\n"
"That's the whole curriculum — nicely done.\n"
f"Share it: \"I just completed all {total} pythonlings Python exercises 🎉\"\n"
"If pythonlings helped, a ⭐ on GitHub or a contribution is hugely appreciated.\n\n"
"Press Ctrl+Q to quit, or F4 to revisit topics."
)


class TrackScreen(Screen[None]):
"""One topic's linear track: editor + output + auto-save loop."""

Expand Down Expand Up @@ -82,7 +93,13 @@ def _initial_exercise(self) -> str | None:
def _render_state(self) -> None:
exs = self._exercises()
done = sum(1 for ex in exs if ex.name in self.app.state.completed)
self.query_one(ProgressBar).update_progress(done, len(exs))
all_exercises = self.app.manifest.exercises
overall_done = completed_count(
(ex.name for ex in all_exercises), self.app.state.completed
)
self.query_one(ProgressBar).update_progress(
done, len(exs), overall_done, len(all_exercises)
)
self.query_one(ExerciseTree).render_topic(
self.topic, exs, self.app.state.completed, self.current
)
Expand Down Expand Up @@ -175,9 +192,15 @@ def _apply_result(self, exercise: Exercise, result: RunResult) -> None:
self._render_state()
if self.current is None:
self._record_resume(None)
self.query_one(OutputPanel).show_final(
f"Topic '{self.topic}' complete — press F4 for topics."
)
all_exercises = self.app.manifest.exercises
if next_pending(all_exercises, self.app.state.completed) is None:
self.query_one(OutputPanel).show_final(
celebration_message(len(all_exercises))
)
else:
self.query_one(OutputPanel).show_final(
f"Topic '{self.topic}' complete — press F4 for topics."
)
Comment on lines +195 to +203

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Topic-only completion still renders a global “All exercises complete” header.

In the else branch (not fully complete curriculum), show_final(...) is still used. That method hardcodes the global completion header in pythonlings/widgets/output_panel.py, so users can see a misleading completion state when only the current topic is done.

Suggested direction
-                self.query_one(OutputPanel).show_final(
+                self.query_one(OutputPanel).show_topic_complete(
                     f"Topic '{self.topic}' complete — press F4 for topics."
                 )
# pythonlings/widgets/output_panel.py (new helper)
def show_topic_complete(self, 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]Topic complete[/bold green]")
    self.query_one("`#goal`", Static).update("")
    self.query_one("`#docs`", Static).update("")
    self.query_one("`#status`", Static).update(message)
    self.query_one("`#next-step`", Static).update("")
    self.query_one("`#details`", Static).update("")
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pythonlings/screens/track.py` around lines 195 - 203, The else branch in
track.py calls show_final() which displays a global completion header, creating
misleading messaging when only the current topic is complete. Create a new
method show_topic_complete() in the OutputPanel class in
pythonlings/widgets/output_panel.py that matches the behavior of show_final()
but sets the header to "Topic complete" instead of the global completion header.
Then replace the show_final() call in the else branch with a call to this new
show_topic_complete() method, passing the topic completion message.

return
self._load_current()
self._run_current()
Expand Down
45 changes: 45 additions & 0 deletions pythonlings/screens/welcome.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# pythonlings/screens/welcome.py
from __future__ import annotations

from textual.app import ComposeResult
from textual.binding import Binding
from textual.containers import Vertical
from textual.screen import ModalScreen
from textual.widgets import Static

from pythonlings.core.state import save as save_state


def welcome_text() -> str:
"""The first-launch explainer for the edit/save/advance loop."""
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"
" 3. Remove the `# I AM NOT DONE` marker to advance to the next one.\n\n"
Comment on lines +18 to +19

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clarify the onboarding loop sentence to remove contradictory behavior.

Line 18 says users should save, then says checks rerun “as you type,” which conflicts with the save-driven loop described in the PR objectives.

Proposed copy tweak
-        "  2. Save -- the check reruns automatically as you type.\n"
+        "  2. Save -- checks rerun automatically.\n"
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
" 2. Save -- the check reruns automatically as you type.\n"
" 3. Remove the `# I AM NOT DONE` marker to advance to the next one.\n\n"
" 2. Save -- checks rerun automatically.\n"
" 3. Remove the `# I AM NOT DONE` marker to advance to the next one.\n\n"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pythonlings/screens/welcome.py` around lines 18 - 19, The onboarding message
on line 18 contains contradictory instructions that say to save but then state
checks rerun "as you type", which conflicts with the save-driven loop workflow.
Update the welcome screen message text to clarify that checks rerun when you
save (remove the "as you type" phrase) so the instructions are consistent with
the actual save-driven behavior and align with the PR objectives. The message is
part of a string literal in the welcome.py file that instructs users on the
workflow.

"Handy keys: F1 hint - F3 exercise list - F4 topics - "
"F5 local docs - Ctrl+Q quit.\n\n"
"Press Enter to start."
)


class WelcomeScreen(ModalScreen[None]):
"""First-launch onboarding overlay explaining the core loop."""

BINDINGS = [
Binding("enter", "start", "Start", priority=True),
Binding("escape", "start", "Start"),
]
Comment on lines +29 to +32

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cat -n pythonlings/screens/welcome.py | sed -n '25,35p'

Repository: abhiksark/pythonlings

Length of output: 444


🌐 Web query:

Ruff RUF012 rule mutable class attribute list comprehension

💡 Result:

The Ruff rule RUF012, titled "mutable-class-default," identifies mutable default values assigned to class attributes [1]. Such attributes are shared across all instances of the class, which can lead to unintended side effects where changes in one instance unexpectedly impact others [1]. While the rule is commonly triggered by simple assignments like my_list: list[int] = [], it also applies to list comprehensions or any other expressions that result in a mutable data structure [1][2]. To resolve RUF012 violations, consider the following approaches: 1. Initialize in init: Move the assignment into the class constructor (init) so that each instance gets its own unique, mutable object [1][3]. 2. Use typing.ClassVar: If the attribute is intentionally meant to be shared across all instances, annotate it with typing.ClassVar to explicitly signal this intent [1]. 3. Use immutable types: Replace the mutable structure with an immutable alternative, such as a tuple instead of a list [1]. 4. Use Optional: Assign None as the default value and handle initialization within the class logic [1][3]. Note that RUF012 generally excludes certain frameworks—such as Pydantic or msgspec—that have built-in copy semantics for their model fields, as these frameworks automatically handle the instantiation of unique values [2][4]. Diagnostic messages for this rule typically suggest these fixes to help you choose the most appropriate pattern for your code [5][6].

Citations:


🏁 Script executed:

rg "BINDINGS\s*=" pythonlings/ -A 2

Repository: abhiksark/pythonlings

Length of output: 912


🌐 Web query:

Textual framework BINDINGS class attribute mutable RUF012

💡 Result:

In the Ruff linter, RUF012 is a rule that identifies mutable default values in class attributes [1][2]. What it does: It flags class attributes that are assigned mutable data structures (such as lists, sets, or dictionaries) directly in the class body [1][2]. Because these objects are created when the class is defined, they are shared across all instances of that class, which often leads to unintended side effects when one instance modifies the shared state [1][2]. Why it is flagged: Mutable default values in class bodies are generally considered an anti-pattern because they create implicit shared state between instances [1]. Common fixes: - Initialize the attribute in the class's init method, which ensures each instance gets its own unique copy of the mutable structure [1][3]. - Annotate the attribute with typing.ClassVar if you explicitly intend for the variable to be shared across all instances [1]. - Use an immutable type, such as a tuple instead of a list, or types.MappingProxyType for dictionaries [1][3]. - Use None as the default value and initialize the object within init [1][3]. Note: The diagnostic message and documentation for RUF012 were improved to clarify these options, as previous versions primarily suggested ClassVar even when initialization in init was the preferred architectural fix [4][5][6]. The rule generally excludes specific frameworks or patterns where default values are intentionally copied on instance creation (e.g., certain uses of Pydantic or msgspec) [7].

Citations:


Use a tuple instead of a list for the BINDINGS class attribute to avoid Ruff RUF012 and prevent accidental mutation.

The list-based BINDINGS is a mutable class attribute shared across all instances, which triggers the RUF012 rule and can lead to unintended side effects.

Suggested change
-    BINDINGS = [
+    BINDINGS = (
         Binding("enter", "start", "Start", priority=True),
         Binding("escape", "start", "Start"),
-    ]
+    )
🧰 Tools
🪛 Ruff (0.15.17)

[warning] 29-32: Mutable default value for class attribute

(RUF012)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pythonlings/screens/welcome.py` around lines 29 - 32, The BINDINGS class
attribute is defined as a list, which is a mutable shared class attribute that
triggers the RUF012 linting rule and can cause unintended mutations. Convert the
BINDINGS list to a tuple by replacing the square brackets with parentheses
around the Binding declarations, keeping all the Binding elements unchanged.

Source: Linters/SAST tools


def compose(self) -> ComposeResult:
yield Vertical(
Static("[bold]Welcome to pythonlings[/bold]", id="welcome-title"),
Static(welcome_text(), id="welcome-body"),
Static("Enter Start", id="welcome-footer"),
id="welcome-window",
)

def action_start(self) -> None:
self.app.state.seen_intro = True
save_state(self.app.root, self.app.state)
self.dismiss()
36 changes: 30 additions & 6 deletions pythonlings/widgets/progress.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,37 @@

from textual.widgets import Static

_BAR_WIDTH = 20


def _bar(completed: int, total: int) -> tuple[str, float]:
pct = (completed / total * 100) if total else 0
filled = int(_BAR_WIDTH * pct / 100)
return "█" * filled + "░" * (_BAR_WIDTH - filled), pct


def format_progress(
completed: int, total: int, overall_completed: int, overall_total: int
) -> str:
"""Render a one-line display of topic progress and overall progress."""
topic_bar, topic_pct = _bar(completed, total)
overall_bar, overall_pct = _bar(overall_completed, overall_total)
return (
f"Topic: {topic_bar} {completed}/{total} ({topic_pct:.0f}%)"
f" Overall: {overall_bar} {overall_completed}/{overall_total} ({overall_pct:.0f}%)"
)


class ProgressBar(Static):
DEFAULT_CSS = ""

def update_progress(self, completed: int, total: int) -> None:
pct = (completed / total * 100) if total else 0
bar_width = 20
filled = int(bar_width * pct / 100)
bar = "█" * filled + "░" * (bar_width - filled)
self.update(f"Progress: {bar} {completed}/{total} ({pct:.0f}%)")
def update_progress(
self,
completed: int,
total: int,
overall_completed: int,
overall_total: int,
) -> None:
self.update(
format_progress(completed, total, overall_completed, overall_total)
)
8 changes: 7 additions & 1 deletion tests/tui/test_app_pilot.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
def _work_copy(tmp_path: Path) -> Path:
work = tmp_path / "work"
shutil.copytree(MULTI, work, ignore=shutil.ignore_patterns(".pythonlings"))
# These tests exercise the returning-user flow, so start past the
# first-launch welcome overlay (covered in test_welcome_pilot.py).
save_state(work, State(seen_intro=True))
return work


Expand Down Expand Up @@ -69,7 +72,10 @@ async def test_enter_on_picker_opens_selected_topic(tmp_path: Path) -> None:

@pytest.mark.asyncio
async def test_picker_shows_first_run_start_banner(tmp_path: Path) -> None:
app = PythonlingsApp(root=_work_copy(tmp_path), force_picker=True)
# Needs genuine first-run state (no seen_intro seed) for the banner.
work = tmp_path / "work"
shutil.copytree(MULTI, work, ignore=shutil.ignore_patterns(".pythonlings"))
app = PythonlingsApp(root=work, force_picker=True)
async with app.run_test() as pilot:
await _settle(pilot)
banner = str(app.screen.query_one("#topic-banner").content)
Expand Down
62 changes: 62 additions & 0 deletions tests/tui/test_welcome_pilot.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import shutil
from pathlib import Path

Comment on lines +1 to +3

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add from __future__ import annotations at the module top.

This new Python module is missing the required future-annotations import.

Proposed fix
+from __future__ import annotations
+
 import shutil
 from pathlib import Path

As per coding guidelines, **/*.py: Include from __future__ import annotations at the top of Python modules.

📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import shutil
from pathlib import Path
from __future__ import annotations
import shutil
from pathlib import Path
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/tui/test_welcome_pilot.py` around lines 1 - 3, The
test_welcome_pilot.py module is missing the required future-annotations import
at the top. Add `from __future__ import annotations` as the very first line in
the file, before the existing imports of shutil and pathlib.Path, to comply with
the coding guidelines that require this import in all Python modules.

Source: Coding guidelines

import pytest
from textual.worker import WorkerCancelled

from pythonlings.app import PythonlingsApp
from pythonlings.core.state import State, load as load_state, save as save_state
from pythonlings.screens.track import TrackScreen
from pythonlings.screens.welcome import WelcomeScreen

MULTI = Path(__file__).parent.parent / "fixtures" / "multi_topic"


def _work_copy(tmp_path: Path) -> Path:
work = tmp_path / "work"
shutil.copytree(MULTI, work, ignore=shutil.ignore_patterns(".pythonlings"))
return work


async def _settle(pilot) -> None:
for _ in range(6):
try:
await pilot.app.workers.wait_for_complete()
except WorkerCancelled:
pass
await pilot.pause()


@pytest.mark.asyncio
async def test_welcome_shown_on_first_launch(tmp_path: Path) -> None:
app = PythonlingsApp(root=_work_copy(tmp_path))
async with app.run_test() as pilot:
await _settle(pilot)
assert isinstance(app.screen, WelcomeScreen)


@pytest.mark.asyncio
async def test_welcome_not_shown_after_intro_seen(tmp_path: Path) -> None:
work = _work_copy(tmp_path)
save_state(work, State(seen_intro=True))

app = PythonlingsApp(root=work)
async with app.run_test() as pilot:
await _settle(pilot)
assert not isinstance(app.screen, WelcomeScreen)
assert isinstance(app.screen, TrackScreen)


@pytest.mark.asyncio
async def test_dismissing_welcome_persists_seen_intro(tmp_path: Path) -> None:
work = _work_copy(tmp_path)

app = PythonlingsApp(root=work)
async with app.run_test() as pilot:
await _settle(pilot)
assert isinstance(app.screen, WelcomeScreen)
await pilot.press("enter")
await _settle(pilot)
assert not isinstance(app.screen, WelcomeScreen)

assert load_state(work).seen_intro is True
10 changes: 10 additions & 0 deletions tests/unit/test_celebration.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
from __future__ import annotations

from pythonlings.screens.track import celebration_message


def test_celebration_message_includes_count_and_is_celebratory() -> None:
msg = celebration_message(292)

assert "292" in msg
assert "🎉" in msg
30 changes: 30 additions & 0 deletions tests/unit/test_progress.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
from __future__ import annotations

from pythonlings.core.state import completed_count
from pythonlings.widgets.progress import format_progress


def test_format_progress_shows_topic_and_overall() -> None:
line = format_progress(2, 5, 10, 100)

assert "2/5" in line
assert "40%" in line # topic 2/5
assert "10/100" in line
assert "10%" in line # overall 10/100
assert "Overall" in line


def test_format_progress_handles_zero_totals() -> None:
line = format_progress(0, 0, 0, 0)

assert "0/0" in line
assert "0%" in line


def test_completed_count_ignores_names_not_in_curriculum() -> None:
# Stale state (renamed/removed exercises) must not inflate the count.
assert completed_count(["a", "b", "c"], {"a", "b", "ghost"}) == 2


def test_completed_count_empty() -> None:
assert completed_count([], set()) == 0
11 changes: 11 additions & 0 deletions tests/unit/test_welcome.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
from __future__ import annotations

from pythonlings.screens.welcome import welcome_text


def test_welcome_text_explains_the_loop() -> None:
text = welcome_text()

assert "I AM NOT DONE" in text
assert "save" in text.lower()
assert "F1" in text
Loading