feat(pybind): register exception translator for cytnx::error -> CytnxError

[yingjerkao]

Problem

Every cytnx C++ error surfaced in Python as a bare RuntimeError (pybind11's fallback for std::logic_error), indistinguishable from any other failure — user code could not catch cytnx errors specifically.

Fix

Registers a pybind11 exception translator in PYBIND11_MODULE mapping cytnx::error (thrown by cytnx_error_msg since the error-header rewrite) to a new cytnx.CytnxError. It subclasses RuntimeError, so all existing except RuntimeError: code keeps working. Uses the non-deprecated py::set_error API with the static-lifetime pattern pybind11 3.x itself uses internally; non-cytnx exceptions fall through to default translation untouched (pinned by test). The exception carries a docstring for help().

Testing

pytests/binding_exception_test.py (4 tests): type + subclass relationship, message content, a submodule error path (linalg.InvM) proving module-global coverage, and negative fall-through pinning (a pybind TypeError and a plain pybind-cast RuntimeError both stay un-reclassified).

Stacking

Base branch is fix/error-macro-rewrite (PR #983) — cytnx::error only exists there. Merge #983 first, then retarget this PR to master.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request introduces a custom Python exception class CytnxError (subclassing RuntimeError) and registers a module-global exception translator in pybind/cytnx.cpp to map C++ cytnx::error exceptions to Python. It also adds comprehensive unit tests in pytests/binding_exception_test.py to verify this behavior. The reviewer suggested simplifying the exception registration in pybind/cytnx.cpp by using pybind11's built-in py::register_exception helper instead of manually defining a static exception object and translator.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

Instead of manually registering a custom exception translator and declaring a static py::exception object, you can use pybind11's built-in py::register_exception helper. It automatically creates the custom Python exception class, registers the exception translator to map the C++ exception to the Python exception using e.what(), and returns a reference to the registered exception object so you can set its docstring. This simplifies the code and avoids manual exception translation boilerplate.

Suggested change

	static py::exception<cytnx::error> cytnx_error_exc(m, "CytnxError", PyExc_RuntimeError);
	cytnx_error_exc.attr("__doc__") =
	"Raised when a cytnx C++ operation fails (via cytnx_error_msg).";
	py::register_exception_translator([](std::exception_ptr p) {
	try {
	if (p) std::rethrow_exception(p);
	} catch (const cytnx::error &e) {
	py::set_error(cytnx_error_exc, e.what());
	}
	});
	py::register_exception<cytnx::error>(m, "CytnxError", PyExc_RuntimeError)
	.attr("__doc__") = "Raised when a cytnx C++ operation fails (via cytnx_error_msg).";

[yingjerkao]

Done in e9d0687 — switched to py::register_exception<cytnx::error>(m, "CytnxError", PyExc_RuntimeError) and set the docstring on the returned object, dropping the manual py::exception/register_exception_translator/set_error boilerplate. Behavior is unchanged: the 4 tests (type + subclass, message body, submodule InvM path, and the negative fall-through pinning that a pybind TypeError/RuntimeError stays un-reclassified) all pass against a freshly built module.

[pcchen]

Code Review

Maps the C++ cytnx::error (from #983) to a dedicated Python cytnx.CytnxError via a pybind11 translator, subclassing RuntimeError so existing except RuntimeError code keeps working. The genuinely new content is small and good: the 14-line translator in pybind/cytnx.cpp and pytests/binding_exception_test.py; the rest of the diff is inherited from the #983 base.

The new code — correct and idiomatic

• Static-lifetime py::exception referenced by a capture-free lambda — the documented pybind11 pattern; cytnx_error_exc is valid whenever the translator fires. ✅
• py::set_error is the non-deprecated pybind11-3.x API, and pyproject.toml requires pybind11 >=3.0, so it's available. ✅
• Fall-through preserved: the lambda catches only const cytnx::error &; anything else rethrows to the default translator. Pinned by test_non_cytnx_errors_are_not_reclassified, which correctly checks that a pybind TypeError and a pybind-cast RuntimeError are not reclassified — a valuable negative test given CytnxError subclasses RuntimeError. ✅
• No pre-existing translator to conflict with. ✅

Issues & Risks

1. Stale copy of the #983 base — rebase before merge (medium, process).
The diff carries the entire #983 header rewrite, and this branch's copy is older than the current fix/error-macro-rewrite head:

• Missing the null-safety guards already on the base: if (format == nullptr) in vformat_message, and the kind ? kind : "error" / func ? func : "unknown" fallbacks in compose_report.
• Missing name ? name : "value" in checked_cast.hpp, and 3 tests (NullFormatDoesNotCrash, ComposeReportToleratesNulls, CheckedCastNullNameDoesNotCrash).
• Re-introduces the "...support!%s", "\n" mis-formatting in Generator.cpp (base has the cleaned "...support!"); — the extra %s prints literally and "\n" is silently dropped).

A 3-way merge into the base will likely keep the base's newer lines, so it may not regress in practice, but the diverged snapshot makes the review scope confusing and is fragile. Recommendation (matching the PR's own stacking note): merge #983 first, then rebase this onto master — the diff should then be just pybind/cytnx.cpp + pytests/binding_exception_test.py.

2. Comment slightly misstates the mechanism (nit).

"Registered before any submodule bindings so the translator is in effect for every binding below"

Exception translators are consulted at exception-raise time, not binding time — registration order relative to symmetry_binding(m) etc. doesn't affect coverage. Being early is harmless, but the rationale is inaccurate; consider "translators are global and consulted when an exception propagates, so order relative to bindings is irrelevant."

3. Scope of CytnxError (low).
Only cytnx::error translates. Bare throw std::logic_error(...) sites — e.g. the still-untouched GPU error macros in cytnx_error.hpp — and other std:: exceptions stay as RuntimeError. Expected and fine, but worth a one-line note (docstring or PR) that CytnxError == cytnx_error_msg-originated only.

4. Test brittleness (nit).
match="reshape" / match="InvM" depend on the function name appearing in the message (via CYTNX_FUNC_NAME/__PRETTY_FUNCTION__). Fine today, but coupled to that formatting; a matcher on a stable substring of the error body would be more robust.

Summary

The actual contribution — the translator and its tests — is correct, idiomatic for pybind11 3.x, and well-tested (including the important negative fall-through). The one real action item is rebasing onto the merged #983 so the diff stops carrying a stale copy of that PR's files; the rest are nits.

Posted by Claude Code on behalf of @pcchen

[pcchen]

Follow-up: #983 is now approved

Heads-up on stacking. Since this branch was cut, base PR #983 has advanced further (it dropped the _MSC_VER/__FUNCSIG__ branch, moved User_debug out of Type.hpp, and added more tests), so this branch's bundled copy of the error-header files is now further behind the approved #983 — e.g. it still carries the #ifdef _MSC_VER … __FUNCSIG__ branch #983 deleted, the duplicate extern bool User_debug; in Type.hpp #983 removed, and the stale Generator.cpp formatting.

None of that needs hand-fixing here: once #983 merges, rebase this onto master and the rebase absorbs all of it automatically, collapsing this PR's diff to just its real contribution — pybind/cytnx.cpp + pytests/binding_exception_test.py.

The new code itself is unaffected and still good: #983's changes don't touch cytnx::error or the translator, and the shared m.attr("User_debug") = cytnx::User_debug; line still compiles (User_debug is now declared in cytnx_error.hpp, transitively included).

Posted by Claude Code on behalf of @pcchen

[codecov]

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 30.83%. Comparing base (7b4d7dc) to head (e9d0687).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
pybind/cytnx.cpp	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #989   +/-   ##
=======================================
  Coverage   30.82%   30.83%           
=======================================
  Files         229      229           
  Lines       33348    33349    +1     
  Branches    14069    14070    +1     
=======================================
+ Hits        10281    10283    +2     
  Misses      15795    15795           
+ Partials     7272     7271    -1     

Flag	Coverage Δ
cpp	30.46% <0.00%> (+<0.01%)	⬆️
python	61.44% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
C++ backend	32.26% <ø> (+<0.01%)	⬆️
Python bindings	18.79% <0.00%> (+0.01%)	⬆️
Python package	61.44% <ø> (ø)

Files with missing lines	Coverage Δ
pybind/cytnx.cpp	18.18% <0.00%> (-0.57%)	⬇️

... and 2 files with indirect coverage changes

---

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7b4d7dc...e9d0687. Read the comment docs.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[yingjerkao]

Thanks @pcchen — all points addressed.

1. Rebase (main action item). Already resolved: the branch is now based on post-#983 master, so the diff no longer carries the stale copy of the error-header files — it's just pybind/cytnx.cpp + pytests/binding_exception_test.py. Rebased again onto the current tip (through #1004) so it's fully current and fast-forwardable.

2. Comment misstates the mechanism. Fixed in e9d0687. The comment now says translators are module-global and consulted when an exception propagates (not at binding time), so registration order relative to the submodule bindings is irrelevant.

3. Scope of CytnxError. Added a one-line note to both the comment and the docstring that only cytnx_error_msg-originated errors (i.e. cytnx::error) are reclassified; other std:: exceptions keep their default pybind11 mapping.

4. Test brittleness. Fixed. test_message_content now matches "number of elements" — a stable substring of the error body — instead of "reshape", which (verified empirically) appears only in the __PRETTY_FUNCTION__ section. The InvM matcher I left as-is: [InvM] is a literal in the message body, not function-name-coupled, so it's already robust.

Separately I took the Gemini suggestion and collapsed the manual py::exception/register_exception_translator/set_error into py::register_exception<cytnx::error>(...) — same behavior, less boilerplate.

Verification: pybind/cytnx.cpp compiles clean with the project flags, and all 4 tests pass against a freshly built module (including the negative fall-through: a pybind TypeError and a pybind-cast RuntimeError both stay un-reclassified).

[pcchen]

Approving. The translator now uses the canonical py::register_exception<cytnx::error>(m, "CytnxError", PyExc_RuntimeError) one-liner — cleaner than the manual py::exception + register_exception_translator, and the updated comment accurately describes translator semantics (module-global, consulted when an exception propagates, order-independent relative to bindings), resolving my earlier note. CytnxError subclasses RuntimeError so existing except RuntimeError code keeps working, non-cytnx exceptions still fall through to default translation, and the tests pin all of it (type/subclass, message content, submodule InvM path, and the negative not-reclassified case). The message-content test now matches a stable error-body substring rather than the function name — nice.

Verified the branch is exactly master + the two translator commits (touching only pybind/cytnx.cpp + pytests/binding_exception_test.py), and Linux BuildAndTest (compile + pytest) is green. codecov/patch is advisory. Good to merge once the remaining macOS/wheel jobs finish.

Posted by Claude Code on behalf of @pcchen