Repository navigation
lint(ci): add check-py39-union-annotations.py gate (#961 Option A) - #1199
bassilkhilo-ag2 wants to merge 1 commit into
Conversation
…n A) AST-based CI gate that flags PEP-604 `X | Y` union annotations in `src/` and `skills/` without `from __future__ import annotations`, which crash Python 3.9 on import (root cause of sonichi#960 telegram-bridge outage). Exits 1 on any violation. Also adds `from __future__ import annotations` to skills/deal-finder/scripts/scan.py which had 5 violations. 14/14 structural tests added in tests/check-py39-union-annotations.test.py. Closes sonichi#961 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
VasiliyRad
left a comment
There was a problem hiding this comment.
Clean. Script is stdlib-only AST-based, correctly distinguishes annotation BitOr from expression BitOr, file-level future guard works as expected. CI wiring is a pure additive step in the right slot (pre-Node, no deps), no conflict with #1181's cwd-lint step. Fixing the deal-finder violation in the same PR is the right call. 14/14 tests cover the meaningful edge cases. Good to go.
VasiliyRad
left a comment
There was a problem hiding this comment.
Solid CI gate. AST walk correctly scopes BitOr detection to annotation positions (AnnAssign, function returns, all arg kinds), early-exits on from __future__ import annotations, and surfaces file:line violations with a fix hint. Bundled deal-finder/scripts/scan.py patch presumably adds the __future__ line to clear the gate — appropriate companion change.
Nit (non-blocking): hits.append(node.col_offset and node.lineno or node.lineno) in annotation_union_lines for the AnnAssign case is equivalent to plain hits.append(node.lineno) — col_offset and … only filters when col_offset is 0, which can't happen for a real statement. Won't affect output, just dead idiom. Otherwise clean.
Scope: only src/ + skills/ scanned. Defensible since the original outage was in src/, but worth widening to scripts/ + tests/ later if a similar regression slips through.
LGTM 👍
Summary
scripts/check-py39-union-annotations.py: AST-based CI gate that scanssrc/andskills/for PEP-604X | Yunion annotations used in annotation positions (variable annotations, function return types, argument annotations) withoutfrom __future__ import annotations— the root cause of the fix(telegram-bridge): from __future__ import annotations — fixes py3.9 crash #960 telegram-bridge outage on Python 3.9.github/workflows/ci.ymlalongside the existing UTC timestamp gate (runs before Node deps install — no deps needed)skills/deal-finder/scripts/scan.pyby adding the__future__importtests/check-py39-union-annotations.test.pycovering AST detection, file-level guard, CLI exit codesWhat the gate catches
Test plan
python3 scripts/check-py39-union-annotations.py→Results: 52 Python files checked, 0 violations.python3 tests/check-py39-union-annotations.test.py -v→ 14/14 passCloses #961
🤖 Generated with Claude Code