docs: add Cython gotchas section to CLAUDE.md#1679
Merged
Conversation
Mirror of esphome/aioesphomeapi#1653, adapted to python-zeroconf's code layout (`_protocol/`, `_cache`, `_handlers/`, `_listener`, `const.py`). Captures non-obvious `.py` + `.pxd` traps that work fine in pure- Python mode but break or silently misbehave in the shipped Cython wheels: - `cdef`-typed module constants are not Python-importable; the pattern is to keep both a public `MAX_X` and a `_MAX_X` alias for cdef use. - `noexcept` cdef paths must be pure C; raising from inside a `noexcept` function is undefined / lossy (Cython prints via `WriteUnraisable` and continues). - `unsigned int` returned through `cdef int` can flip sign for values with bit 31 set, silently hitting `< 0` sentinel branches and bypassing length / offset checks — the parser hot path in `_protocol/incoming` is exactly where this class of bug bites. - `except *` / `except? -N` adds per-call `PyErr_Occurred()` checks; measurable on hot paths. - Module-level Python int constants force `PyLong_AsLong` on every hot-path comparison; declare `cdef int` in the `.pxd` to get a native C compare. - Sign-compare warnings in generated C predict the unsigned -> signed overflow class of bug. - CodSpeed regressions only show in the Cython build — pure- Python tests can pass while production hot paths regress. Docs only; no production code touched.
Trim and reshape the section added in the previous commit to
match python-zeroconf's reality rather than aioesphomeapi's:
- Drop the `noexcept` cdef bullet and the `except *` / `except? -N`
bullet. The authored .pxd files in this repo don't use either —
warning about traps the codebase doesn't reach is noise.
- Fix the `unsigned int` -> `cdef int` bullet. DNS RR length and
name compression pointer fields are 16-bit and 14-bit, so they
don't cross the bit-31 signed boundary. Cite TTL instead — it
is 32-bit (RFC 1035 §3.2.1) and the only field where the trap
applies — and frame the rule as "stay with `unsigned int` end-
to-end" since that is the convention `_protocol/incoming.pxd`,
`_cache.pxd`, and `_handlers/*.pxd` already follow.
- Ground the `cdef`-typed module constants bullet in an actual
example: `_ANSWER_STRATEGY_POINTER` in
`_handlers/query_handler.{py,pxd}`.
- Ground the PyLong-conversion bullet in actual examples:
`_TYPE_PTR`, `_DNS_PTR_MIN_TTL`, `_MIN_SCHEDULED_RECORD_EXPIRATION`
already use the `cdef unsigned int` alias pattern.
- Drop "(things that have bitten us)" from the heading — most of
the surviving bullets are preventative for this repo, not
historical incidents.
Docs only.
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a "Cython gotchas" section to CLAUDE.md documenting non-obvious traps in the .py + .pxd setup that work in pure-Python mode but fail or misbehave in shipped Cython wheels.
Changes:
- New documentation section between "Build conventions" and "Reporting security issues" listing five Cython traps adapted to python-zeroconf's code layout.
- No production code or tests changed.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1679 +/- ##
=======================================
Coverage 99.76% 99.76%
=======================================
Files 33 33
Lines 3401 3401
Branches 461 461
=======================================
Hits 3393 3393
Misses 5 5
Partials 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add a Cython gotchas section to
CLAUDE.md, mirroring esphome/aioesphomeapi#1653 and adapted to python-zeroconf's code layout. Documents the non-obvious.py+.pxdtraps that work in pure-Python mode but break or silently misbehave in shipped Cython wheels — exactly the class of bug whereSKIP_CYTHON=1tests pass whileuse_cythonCI catches it (or worse, it ships).Details
New section sits between
## Build conventionsand## Reporting security issues, capturing seven traps:cdef-typed module constants are not Python-importable. Pattern: keep both a publicMAX_Xand acdef-typed_MAX_X = MAX_Xalias sofrom module import _MAX_Xdoesn't silently break under Cython.noexceptcdef paths must be pure C. Raising from insidecdef ... noexceptis undefined; Cython prints viaWriteUnraisableand continues. Keepnoexceptpaths to sentinel returns.unsigned intreturned throughcdef intcan flip sign for values with bit 31 set — silently hitting< 0sentinel branches. Called out specifically for parsers in_protocol/incomingwhere DNS name pointers and RR length fields cross the signed-int boundary.except */except? -Nadds per-callPyErr_Occurred()checks — negligible cold, measurable hot.PyLong_AsLongin hot-path comparisons;cdef intin the.pxdmakes it a native C compare. Called out for constants inconst.pyconsumed by_protocol/,_cache,_handlers/,_listener.REQUIRE_CYTHON=1 poetry install --only=main,devto rebuild in place locally.Aioesphomeapi-specific references in the original section (issue / PR numbers, BLE function names,
_handle_error_and_close,_read_varuint) are dropped or generalized. Each trap is grounded in this repo's actual hot-path modules instead.Docs only; no production code, no tests touched.
Test plan
lintandcommitlintjobs are green (commit subject isdocs:— semantic-release will skip it from the changelog, which is the right behaviour for a docs-only contributor-guide change).testmatrix continues to pass unchanged (no code touched).