Skip to content

Ruff depth limit#391

Open
samuelcolvin wants to merge 5 commits intomainfrom
ruff-depth-limit
Open

Ruff depth limit#391
samuelcolvin wants to merge 5 commits intomainfrom
ruff-depth-limit

Conversation

@samuelcolvin
Copy link
Copy Markdown
Member

Use ruff parsing depth limits as per astral-sh/ruff#24810

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 24, 2026

Merging this PR will not alter performance

✅ 16 untouched benchmarks
⏩ 15 skipped benchmarks1


Comparing ruff-depth-limit (30be6aa) with main (5c7cf2b)

Open in CodSpeed

Footnotes

  1. 15 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 3 potential issues.

View 2 additional findings in Devin Review.

Open in Devin Review

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🚩 Stale check_types references in unchanged docstrings

The docstring at lines 68-69 of crates/monty-type-checking/src/db.rs still references check_types ("Program settings needed by check_types") even though the PR replaces all usage with check_file_unwrap. Similarly, crates/monty-type-checking/src/pool.rs:125 says "e.g. for check_types or rendering". These are pre-existing context lines not in the diff hunks, but they became stale as a direct result of this PR's changes. Per the CLAUDE.md rule about updating stale comments, these should be updated in a follow-up.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread crates/monty-type-checking/src/db.rs
Comment thread Cargo.toml Outdated
devin-ai-integration[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 2 new potential issues.

View 5 additional findings in Devin Review.

Open in Devin Review

Comment thread crates/monty-python/src/exceptions.rs
Comment on lines +846 to 849
def display(self, format: Literal['traceback', 'type-msg', 'msg'] = 'traceback') -> str:
"""Returns formatted exception string.

Args:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🚩 MontySyntaxError.display() pyi docstring incomplete for new 'traceback' format

The type stub at crates/monty-python/python/pydantic_monty/_monty.pyi:846 correctly lists 'traceback' in the Literal type and as the default, but the docstring at lines 849-851 only documents 'type-msg' and 'msg' formats — it doesn't document 'traceback'. Compare with MontyRuntimeError.display() at line 891-897 which documents all three formats. This is a documentation gap in the public API stub.

(Refers to lines 846-852)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 24, 2026

Codecov Results 📊

❌ Patch coverage is 65.79%. Project has 23799 uncovered lines.
❌ Project coverage is 64.55%. Comparing base (base) to head (head).

Files with missing lines (4)
File Patch % Lines
crates/monty-python/src/exceptions.rs 78.72% ⚠️ 10 Missing
crates/monty-type-checking/src/db.rs 11.11% ⚠️ 8 Missing
crates/monty/src/exception_public.rs 56.25% ⚠️ 7 Missing and 1 partials
crates/monty/src/parse.rs 66.67% ⚠️ 1 Missing
Coverage diff
@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
- Coverage    64.56%    64.55%    -0.01%
==========================================
  Files          258       258         —
  Lines        67096     67126       +30
  Branches    143260    143312       +52
==========================================
+ Hits         43314     43327       +13
- Misses       23782     23799       +17
- Partials      3241      3272       +31

Generated by Codecov Action

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 24, 2026

Codecov Report

❌ Patch coverage is 75.00000% with 19 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/monty-python/src/exceptions.rs 78.72% 10 Missing ⚠️
crates/monty-type-checking/src/db.rs 11.11% 8 Missing ⚠️
crates/monty/src/exception_public.rs 93.75% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 6 additional findings in Devin Review.

Open in Devin Review

Comment on lines 847 to 849
"""Returns formatted exception string.

Args:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 MontySyntaxError.display() docstring missing documentation for 'traceback' format (now the default)

The .pyi docstring for MontySyntaxError.display() at crates/monty-python/python/pydantic_monty/_monty.pyi:846-852 does not document the 'traceback' format option, despite it being the new default. The docstring only lists 'type-msg' and 'msg'. Compare with MontyRuntimeError.display() at crates/monty-python/python/pydantic_monty/_monty.pyi:891-898, which correctly documents all three formats including 'traceback' - full traceback with exception. This violates CLAUDE.md: "If you encounter a comment or docstring that's out of date - you MUST update it to be correct."

(Refers to lines 847-852)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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.

1 participant