📝 Add docstrings to `feat_patch` #131

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

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-06T20:28:34Z

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-06T20:28:34Z

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")

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

This comment was marked as outdated.

Sign in to view

laike9m reviewed Jan 22, 2026

View reviewed changes

This comment was marked as off-topic.

Sign in to view

laike9m reviewed Feb 6, 2026

View reviewed changes

laike9m requested changes Feb 6, 2026

View reviewed changes

laike9m reviewed Feb 6, 2026

View reviewed changes

		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 ''}")

📝 Add docstrings to feat_patch #131

Are you sure you want to change the base?

📝 Add docstrings to feat_patch #131

Uh oh!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

📝 Add docstrings to `feat_patch` #131

📝 Add docstrings to `feat_patch` #131