Skip to content

fix(core): keep run() from raising on invalid UTF-8 exercises - #134

Draft
Huoyanlifusu wants to merge 2 commits into
abhiksark:devfrom
Huoyanlifusu:fix/raise-invalid-utf8
Draft

fix(core): keep run() from raising on invalid UTF-8 exercises#134
Huoyanlifusu wants to merge 2 commits into
abhiksark:devfrom
Huoyanlifusu:fix/raise-invalid-utf8

Conversation

@Huoyanlifusu

Copy link
Copy Markdown
Contributor

Summary

Fixes #72

run() documents a no-raise contract: the CLI and TUI rely on receiving a structured RunResult. An invalidly encoded learner exercise currently escapes that boundary as UnicodeDecodeError from read_text().

Catch the decode error and return a failing RunResult with an actionable stderr message instead.

Tests

(.venv) yy@yy-desktop:~/opensource/pythonlings$ python -m pytest tests/unit/test_runner.py tests/integration/test_cli_run.py tests/integration/test_cli_verify.py -q
python -m pytest -q
pythonlings --root tests/fixtures/passing_curriculum verify
..........................                                                                                                         [100%]
26 passed in 1.93s
.................................................................................................................................. [ 60%]
......................................................................................                                             [100%]
216 passed in 26.43s
✓ passing1
✓ passing2

Screenshots

Checklist

  • Updated docs when behavior changed
  • Added or updated tests
  • Verified python -m pytest -q

run() documents a no-raise contract: the CLI and TUI rely on receiving
a structured RunResult. An invalidly encoded learner exercise currently
escapes that boundary as UnicodeDecodeError from read_text().

Catch the decode error and return a failing RunResult with an
actionable stderr message instead.
@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 5e75e687-0a00-40ac-80dc-a67e53cf7662

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@abhiksark

Copy link
Copy Markdown
Owner

Thanks for tackling this — the run() change is going in the right direction, and the current test suite is green. I found one blocker before we can merge it.

An invalid UTF-8 exercise still crashes the TUI on startup before run() gets a chance to return the new RunResult. I reproduced it with an invalid a1.py: TrackScreen._load_current() calls EditorPane.load_exercise(), which decodes the file directly and raises UnicodeDecodeError. The output panel also reads the exercise directly when building the goal text. The app stops instead of showing the failure through its normal output path, which is one of #72’s acceptance requirements.

Could you handle that TUI load path and add a regression test showing the app stays open and displays the encoding error? The runner error should also include the exercise name or path; right now it only says exercise is not valid UTF-8, so it does not identify which exercise failed.

Validation on the current branch:

  • targeted runner/CLI tests — 26 passed
  • full suite — 216 passed
  • passing curriculum verification — passed
  • manual CLI check — exits cleanly without a traceback
  • manual TUI check — still raises UnicodeDecodeError and stops the app

@abhiksark abhiksark left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The core runner fix works, but #72 is not fully satisfied yet. Invalid UTF-8 still raises from EditorPane.load_exercise() before the TUI reaches run(), so the app stops instead of rendering a normal failure. Please cover the TUI read path with regression coverage, and include the affected exercise name or path in the runner error.

Render invalid encoding errors through the normal output panel,
include the exercise name and path, and add TUI regression coverage.

Signed-off-by: zhangyuyang <zhangyuyang821@163.com>
@Huoyanlifusu
Huoyanlifusu marked this pull request as draft September 2, 2026 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants