Conversation
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
I also updated the black version to 26.1 as this was causing issues with github actions (I think actions is configured to use the latest stable version of black) |
There was a problem hiding this comment.
Pull request overview
This PR aims to fix missing/blank “source traceback” output when KIRIN_PYTHON_STACKTRACE=0 by preserving interpreter frames on exceptions so the exception handler can identify the originating kernel statement.
Changes:
- Modify
InterpreterABC.new_frame()to avoid popping frames on exceptions, preserving the full frame chain for traceback formatting. - Update a few formatting/style items (assignment tests, generator script formatting, Binding error string formatting).
- Update dev tooling/dependencies (Black, lockfile changes; plus new optional deps/extra entries in the lockfile).
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/kirin/interp/abc.py |
Preserve interpreter frame chain on exceptions to enable correct source traceback output. |
src/kirin/lowering/python/binding.py |
Reformat NotImplementedError raising code (string layout changed). |
test/lowering/test_assign.py |
Adjust tuple-unpacking assignment syntax to match formatter expectations. |
src/kirin/dialects/math/_gen.py |
Refactor string writes into more compact calls (no functional intent indicated). |
pyproject.toml |
Bump Black version requirement. |
.pre-commit-config.yaml |
Bump Black pre-commit hook revision. |
uv.lock |
Lockfile updates: toolchain version bump, Black/pathspec/pytokens updates, and new optional dependency entries (e.g., vmath extra with numpy/scipy). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| except BaseException: | ||
| # NOTE: Don't pop frames on exception so that eval_context | ||
| # can capture the full frame chain for the stack trace. | ||
| # The state is re-initialized on the next eval_context call. | ||
| raise |
There was a problem hiding this comment.
new_frame now catches BaseException and deliberately leaves frames on the interpreter state. This means KeyboardInterrupt/SystemExit will also leave the interpreter stack dirty, and it also doesn't line up with eval_context, which only annotates Exception instances with KIRIN_INTERP_STATE. Consider catching Exception here (or alternatively updating eval_context to handle BaseException and resetting self.state after attaching it to the raised exception) so non-error control-flow exceptions don't leak frames.
| # NOTE: Don't pop frames on exception so that eval_context | ||
| # can capture the full frame chain for the stack trace. | ||
| # The state is re-initialized on the next eval_context call. | ||
| raise | ||
| else: | ||
| self.state.pop_frame() |
There was a problem hiding this comment.
Because frames are no longer popped on exception, there’s currently no test coverage ensuring the source traceback includes the expected statement when KIRIN_PYTHON_STACKTRACE=0. Adding a regression test that triggers an interpreter error and asserts the printed stack trace contains the originating stmt/source would prevent this from silently regressing.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
3 participants
When KIRIN_PYTHON_STACKTRACE is set to 0, the source traceback should identify the statement in the original kernel that's causing the error. This is currently blank due to a bug in the base interpreter error handling; this PR fixes the issue.