📝 Add docstrings to `feat_patch` by coderabbitai[bot] · Pull Request #131 · laike9m/Python-Type-Challenges

coderabbitai · 2026-01-12T05:42:00Z

Docstrings generation was requested by @laike9m.

Support test_code to add pyright-config comment #98 (comment)

The following files were modified:

tests/assets/challenges/basic-foo-pyright-config/question.py
tests/conftest.py
tests/test_identical.py
views/challenge.py
views/views.py

These files were kept as they were

tests/test_challenge.py

These file types are not supported

docs/Contribute.md

ℹ️ Note

CodeRabbit cannot perform edits on its own pull requests yet.

@laike9m

Docstrings generation was requested by @laike9m. * #98 (comment) The following files were modified: * `tests/assets/challenges/basic-foo-pyright-config/question.py` * `tests/conftest.py` * `tests/test_identical.py` * `views/challenge.py` * `views/views.py`

coderabbitai · 2026-01-12T05:42:06Z

Important

Review skipped

CodeRabbit bot authored PR detected.

To trigger a single review, invoke the @coderabbitai review command.

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

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

laike9m

Great work on refactoring the type-checking logic and handling Pyright configurations!

I found one bug in tests/test_identical.py.

laike9m · 2026-01-22T22:11:05Z

tests/test_identical.py

+        challenge = Challenge(
+            name=challenge_name, level=Level(level), code=challenge_code
+        )
+


This line should use path.open() instead of solution_file.open(). Currently, it always reads the content of solution_file regardless of the path argument.

laike9m

The pull request introduces docstrings and refactors the Pyright type-checking logic to support custom configurations within challenge files.

I found a bug in tests/test_identical.py where the helper function incorrectly reads from the solution_file fixture instead of the provided path argument, which would cause the test to always pass incorrectly.

I also suggested a few improvements in views/challenge.py to restore the "All tests passed" message and ensure consistent line number reporting.

laike9m · 2026-02-06T07:31:21Z

tests/test_identical.py

+        with solution_file.open() as f:
+            challenge_code = f.read()


The helper function get_test_code should use the path argument passed to it instead of the solution_file fixture from the outer scope. Using solution_file here causes the test to compare the solution's test code with itself when called for the question file, making the test ineffective.

Suggested change

with solution_file.open() as f:

challenge_code = f.read()

with path.open() as f:

challenge_code = f.read()

laike9m · 2026-02-06T07:31:21Z

views/challenge.py

+        passed = True
+        for error_line in error_lines:
+            if error_line.startswith("[pyright-config]"):
+                continue
+            passed = False
+
+        error_lines.append(f"\nFound {len(error_lines)} errors")


The previous implementation provided a clear "All tests passed" message when the challenge was successful. The new logic always shows "Found X errors", which is less encouraging for users when they succeed. Restoring the success message improves the user experience.

Suggested change

passed = True

for error_line in error_lines:

if error_line.startswith("[pyright-config]"):

continue

passed = False

error_lines.append(f"\nFound {len(error_lines)} errors")

# Error for pyright-config will not fail the challenge

passed = not any(

not line.startswith("[pyright-config]") for line in error_lines

)

if passed:

error_lines.append("\nAll tests passed")

else:

error_lines.append(f"\nFound {len(error_lines)} errors")

laike9m

The pull request introduces docstrings and refactors the Pyright type-checking logic to support custom configurations within challenge files.

I found a bug in tests/test_identical.py where the helper function incorrectly reads from the solution_file fixture instead of the provided path argument, which would cause the test to always pass incorrectly.

I also suggested a few improvements in views/challenge.py to restore the "All tests passed" message and ensure consistent line number reporting.

laike9m · 2026-02-06T07:45:00Z

tests/test_identical.py

+        with solution_file.open() as f:
+            challenge_code = f.read()


The helper function get_test_code should use the path argument passed to it instead of the solution_file fixture from the outer scope. Using solution_file here causes the test to compare the solution's test code with itself when called for the question file, making the test ineffective.

Suggested change

with solution_file.open() as f:

challenge_code = f.read()

with path.open() as f:

challenge_code = f.read()

laike9m · 2026-02-06T07:45:00Z

views/challenge.py

+        passed = True
+        for error_line in error_lines:
+            if error_line.startswith("[pyright-config]"):
+                continue
+            passed = False
+
+        error_lines.append(f"\nFound {len(error_lines)} errors")


The previous implementation provided a clear "All tests passed" message when the challenge was successful. The new logic always shows "Found X errors", which is less encouraging for users when they succeed. Restoring the success message improves the user experience.

Suggested change

passed = True

for error_line in error_lines:

if error_line.startswith("[pyright-config]"):

continue

passed = False

error_lines.append(f"\nFound {len(error_lines)} errors")

# Error for pyright-config will not fail the challenge

passed = not any(

not line.startswith("[pyright-config]") for line in error_lines

)

if passed:

error_lines.append("\nAll tests passed")

else:

error_lines.append(f"\nFound {len(error_lines)} errors")

laike9m

The PR adds docstrings and refactors some logic to support per-challenge Pyright configuration. I found a bug in the test suite and some opportunities for optimization and UX improvement.

laike9m · 2026-02-06T07:46:28Z

tests/test_identical.py

+        with solution_file.open() as f:
+            challenge_code = f.read()


The nested function get_test_code should use its parameter path to open the file. Currently, it always reads solution_file, which makes the comparison between solution and question test code ineffective.

Suggested change

with solution_file.open() as f:

challenge_code = f.read()

with path.open() as f:

challenge_code = f.read()

laike9m · 2026-02-06T07:46:28Z

views/challenge.py

+        user_code_lines_len = len(user_code.splitlines())
        for line in stdout.splitlines():


Calculate the number of lines in test_code once outside the loop to avoid redundant calculations.

Suggested change

user_code_lines_len = len(user_code.splitlines())

for line in stdout.splitlines():

user_code_lines_len = len(user_code.splitlines())

test_code_lines_len = len(test_code.splitlines())

for line in stdout.splitlines():

laike9m · 2026-02-06T07:46:28Z

views/challenge.py

+
+            if line_number <= user_code_lines_len:
+                error_lines.append(error_line % line_number)
+            elif line_number <= user_code_lines_len + len(test_code.splitlines()):


Use the pre-calculated line count here.

Suggested change

elif line_number <= user_code_lines_len + len(test_code.splitlines()):

elif line_number <= user_code_lines_len + test_code_lines_len:

laike9m · 2026-02-06T07:46:29Z

views/challenge.py

+                continue
+            passed = False
+
+        error_lines.append(f"\nFound {len(error_lines)} errors")


Restore the 'All tests passed' message when the challenge is successful, and fix the pluralization of 'errors'.

Suggested change

error_lines.append(f"\nFound {len(error_lines)} errors")

if passed:

error_lines.append("\nAll tests passed")

else:

error_lines.append(f"\nFound {len(error_lines)} error{'s' if len(error_lines) > 1 else ''}")

laike9m

The pull request adds valuable docstrings and refactors the type-checking logic to support custom Pyright configurations. However, there is a critical bug in the test suite and some UX/efficiency improvements that should be addressed.

laike9m · 2026-02-06T07:47:24Z

tests/test_identical.py

+        with solution_file.open() as f:
+            challenge_code = f.read()


Bug: The helper function get_test_code should use the path argument to open the file. Currently, it always reads from solution_file (the fixture), which means it compares the solution's test code with itself, making the test ineffective.

Suggested change

with solution_file.open() as f:

challenge_code = f.read()

with path.open() as f:

challenge_code = f.read()

laike9m · 2026-02-06T07:47:24Z

views/challenge.py

+
+            if line_number <= user_code_lines_len:
+                error_lines.append(error_line % line_number)
+            elif line_number <= user_code_lines_len + len(test_code.splitlines()):


Optimization: Calculate test_code_lines_len = len(test_code.splitlines()) once outside the loop to avoid redundant calculations for every error line.

Suggested change

elif line_number <= user_code_lines_len + len(test_code.splitlines()):

elif line_number <= user_code_lines_len + test_code_lines_len:

laike9m · 2026-02-06T07:47:24Z

views/challenge.py

+                continue
+            passed = False
+
+        error_lines.append(f"\nFound {len(error_lines)} errors")


UX: Restore the 'All tests passed' message for successful challenges and improve the pluralization of the error count.

Suggested change

error_lines.append(f"\nFound {len(error_lines)} errors")

if passed:

error_lines.append("\nAll tests passed")

if error_lines:

error_lines.append(f"\nFound {len(error_lines)} error{'s' if len(error_lines) > 1 else ''}")

laike9m

The pull request introduces docstrings and refactors the Pyright type-checking logic to support custom configurations within challenge files.

I found a bug in tests/test_identical.py where the helper function incorrectly reads from the solution_file fixture instead of the provided path argument, which would cause the test to always pass incorrectly.

I also suggested a few improvements in views/challenge.py to restore the "All tests passed" message and ensure consistent line number reporting.

laike9m · 2026-02-06T19:23:57Z

tests/test_identical.py

+        with solution_file.open() as f:
+            challenge_code = f.read()


The helper function get_test_code should use the path argument passed to it instead of the solution_file fixture from the outer scope. Using solution_file here causes the test to compare the solution's test code with itself when called for the question file, making the test ineffective.

Suggested change

with solution_file.open() as f:

challenge_code = f.read()

with path.open() as f:

challenge_code = f.read()

laike9m · 2026-02-06T19:23:57Z

views/challenge.py

+        passed = True
+        for error_line in error_lines:
+            if error_line.startswith("[pyright-config]"):
+                continue
+            passed = False
+
+        error_lines.append(f"\nFound {len(error_lines)} errors")


The previous implementation provided a clear "All tests passed" message when the challenge was successful. The new logic always shows "Found X errors", which is less encouraging for users when they succeed. Restoring the success message improves the user experience.

Suggested change

passed = True

for error_line in error_lines:

if error_line.startswith("[pyright-config]"):

continue

passed = False

error_lines.append(f"\nFound {len(error_lines)} errors")

# Error for pyright-config will not fail the challenge

passed = not any(

not line.startswith("[pyright-config]") for line in error_lines

)

if passed:

error_lines.append("\nAll tests passed")

else:

error_lines.append(f"\nFound {len(error_lines)} errors")

laike9m

Great work adding docstrings and refining the Pyright configuration handling. I found a critical bug in the test refactoring in tests/test_identical.py where the wrong file is being opened, invalidating the test. I also noted a few minor points for improvement.

laike9m · 2026-02-17T19:36:58Z

tests/test_identical.py

+        TEST_SPLITTER = "\n## End of test code ##\n"
+        level, challenge_name = path.parent.name.split("-", maxsplit=1)
+
+        with solution_file.open() as f:


It appears solution_file (from the outer scope) is used here instead of the path argument. This causes get_test_code to always read the solution file, making the test compare the solution code against itself (which will always pass) instead of comparing it against the question code.

Suggested change

with solution_file.open() as f:

with path.open() as f:

laike9m · 2026-02-17T19:36:58Z

views/challenge.py

        error_lines: list[str] = []

-        # Substract lineno in merged code by lineno_delta, so that the lineno in
+        # Substract lineno in merged code by user_code_line_len, so that the lineno in


Typo: "Substract" should be "Subtract".

Suggested change

# Substract lineno in merged code by user_code_line_len, so that the lineno in

# Subtract lineno in merged code by user_code_line_len, so that the lineno in

laike9m · 2026-02-17T19:36:59Z

tests/conftest.py



+@pytest.fixture()
+def mgr(assets_dir: Path):


The mgr fixture appears to be unused in the current changes. Consider removing it if it's not needed, or update the tests to utilize it.

coderabbitai bot assigned laike9m Jan 12, 2026

coderabbitai bot mentioned this pull request Jan 12, 2026

Support test_code to add pyright-config comment #98

Open

		user_code_lines_len = len(user_code.splitlines())
		for line in stdout.splitlines():

	elif line_number <= user_code_lines_len + len(test_code.splitlines()):
	elif line_number <= user_code_lines_len + test_code_lines_len:

-        error_lines.append(f"\nFound {len(error_lines)} errors")
+       if passed:
+           error_lines.append("\nAll tests passed")
+       else:
+           error_lines.append(f"\nFound {len(error_lines)} error{'s' if len(error_lines) > 1 else ''}")

	# Substract lineno in merged code by user_code_line_len, so that the lineno in
	# Subtract lineno in merged code by user_code_line_len, so that the lineno in



		@pytest.fixture()
		def mgr(assets_dir: Path):

Comments

Conversation

coderabbitai bot commented Jan 12, 2026

Uh oh!

coderabbitai bot commented Jan 12, 2026

Review skipped

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

laike9m left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Uh oh!

laike9m left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laike9m left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laike9m left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laike9m left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laike9m left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as duplicate.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as abuse.

Uh oh!

laike9m left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees