ci(lint): enforce type-discipline budget for casts and type guards - #30498
ci(lint): enforce type-discipline budget for casts and type guards#30498mateo-berri wants to merge 1 commit into
Conversation
Add a ratcheted gate that blocks net-new typing.cast() usage and bans TypeGuard/TypeIs outright, layered on the existing ruff-strict budget setup. - ruff-strict.toml: ban cast/TypeGuard/TypeIs (typing + typing_extensions) via flake8-tidy-imports banned-api (TID251) for a coarse import-level freeze. - ruff-strict-budget.json: bump TID251 baseline 2404 -> 2662 to absorb the ~258 pre-existing usages now matched by the new banned-api entries. - scripts/check_type_discipline.py: AST checker adding LIT006 (cast call sites, suppress with `# cast-ok: <reason>`) and LIT007 (TypeGuard/TypeIs annotations, suppress with `# guard-ok: <reason>`) for per-call-site granularity. - scripts/type_discipline_gate.py: baseline+slack gate with delta-vs-base, mirroring ruff_strict_gate.py. - type-discipline-budget.json: LIT006 baseline 1013 (slack 10), LIT007 0/0. - test-linting.yml: run the gate in CI against the PR base SHA.
|
Superseded by #30500 (branch renamed to litellm_type_discipline_gate). |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryAdds a two-layer CI ratchet that freezes net-new
Confidence Score: 4/5Safe to merge — the gate tooling is additive CI infrastructure that doesn't touch production code paths and fails closed on error. The core gate logic (worktree-based base comparison, budget evaluation, breach reporting) is solid and correctly mirrors the existing ruff gate. Two usability issues in the checker script — the no-op scripts/check_type_discipline.py — the suppression comment line-matching and the undocumented flag behavior deserve a second look before the checker is widely used.
|
| Filename | Overview |
|---|---|
| scripts/check_type_discipline.py | New AST-based checker for LIT006/LIT007 rules; has two usability issues: --changed-only flag is documented but silently ignored, and multi-line cast suppression comments not on the call's opening line have no effect. |
| scripts/type_discipline_gate.py | New budget gate mirroring ruff_strict_gate.py; logic for base worktree comparison, cap evaluation, and introduced-violation display is correct. |
| ruff-strict.toml | Adds cast/TypeGuard/TypeIs to banned-api with explanatory messages; missing trailing newline. |
| ruff-strict-budget.json | TID251 baseline bumped 2404→2662 to absorb the ~258 pre-existing import-level violations introduced by the new banned-api entries. |
| type-discipline-budget.json | New budget file with LIT006 baseline 1013/slack 10 and LIT007 at 0/0 (hard ban); values align with the PR description. |
| .github/workflows/test-linting.yml | Adds the type-discipline gate step in the same pattern as the existing ruff_strict_gate step, using BASE_SHA from pull_request.base.sha. |
Reviews (1): Last reviewed commit: "ci(lint): enforce type-discipline budget..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| Validate into a concrete frozen type at the boundary instead. | ||
| Suppress with `# cast-ok: <reason>` on the call's first line. |
There was a problem hiding this comment.
--changed-only flag documented but not implemented
The module docstring shows python check_type_discipline.py --changed-only file1.py file2.py as a valid usage, but main() discards all arguments starting with - via paths = [a for a in argv if not a.startswith("-")]. The flag is silently ignored — passing --changed-only has no effect beyond being dropped. Callers who believe the flag restricts output to changed files will be surprised when all passed files are checked unconditionally. Either implement the flag or remove it from the docstring.
| def iter_cast_violations(path: Path, tree: ast.AST, comments: Comments) -> Iterator[Violation]: | ||
| for node in ast.walk(tree): | ||
| if isinstance(node, ast.Call) and _is_cast_call(node) and node.lineno not in comments.cast_ok_lines: | ||
| yield Violation( | ||
| path, node.lineno, "LIT006", | ||
| "cast() is an unchecked assertion (the type checker takes it on faith); " | ||
| "validate into a frozen dataclass/NamedTuple/ReadOnly TypedDict at the " | ||
| "boundary instead (suppress: `# cast-ok: <reason>`)", | ||
| ) |
There was a problem hiding this comment.
cast() suppression silently has no effect
iter_cast_violations checks node.lineno not in comments.cast_ok_lines, where node.lineno is the first line of the ast.Call node. A suppression comment placed on any line other than the cast('s opening line is not matched and the violation is still counted. The module docstring mentions "on the call's first line" but this constraint is easy to miss for long multi-line casts. Consider checking all lines spanned by the call (node.lineno through node.end_lineno) for a suppression comment.
| "typing.TypeGuard".msg = "Unverified narrowing. Parse into a concrete type, or use isinstance for a runtime-checked narrowing." | ||
| "typing_extensions.TypeGuard".msg = "Same as typing.TypeGuard." | ||
| "typing.TypeIs".msg = "Unverified narrowing (the body is trusted). Parse into a concrete type instead." | ||
| "typing_extensions.TypeIs".msg = "Same as typing.TypeIs." No newline at end of file |
There was a problem hiding this comment.
| "typing_extensions.TypeIs".msg = "Same as typing.TypeIs." | |
| "typing_extensions.TypeIs".msg = "Same as typing.TypeIs." |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Summary
Adds a ratcheted CI gate that blocks net-new
typing.cast()usage and bansTypeGuard/TypeIsoutright, layered on top of the existingruff-strictbudget infrastructure. Nothing existing is forced to change — the gate freezes today's counts as a baseline and fails only on additions beyondbaseline + slack.Two enforcement layers:
banned-api/ TID251).cast,TypeGuard, andTypeIs(from bothtypingandtyping_extensions) are added toflake8-tidy-imports.banned-apiinruff-strict.toml. Because TID251 is a single rule code covering all banned-api matches, theruff-strict-budget.jsonbaseline is bumped2404 → 2662to absorb the ~258 pre-existing usages these entries now match (otherwise the gate would breach immediately on existing code).scripts/check_type_discipline.pygains:cast(...)call sites. Suppress with# cast-ok: <reason>.TypeGuard[...]/TypeIs[...]annotations. Suppress with# guard-ok: <reason>.scripts/type_discipline_gate.pyruns the checker with abaseline + slackbudget and delta-vs-base comparison, mirroringruff_strict_gate.py. Budgets intype-discipline-budget.json:LIT006baseline1013/ slack10,LIT0070/0(hard ban — there are none today).CI wiring:
test-linting.ymlrunsscripts/type_discipline_gate.py --base "$BASE_SHA"alongside the existingruff_strict_gate.pystep.Rationale:
typing.cast()is the moral equivalent of TypeScript'sas— an unchecked assertion with zero runtime guarantee — andTypeGuard/TypeIspredicate bodies are never verified by the checker. The intent is to validate into concrete frozen types at boundaries instead, and to ratchet the cast count down over time (python scripts/type_discipline_gate.py --updatere-captures a lower baseline as casts are removed).Files
ruff-strict.toml— ban cast/TypeGuard/TypeIs via banned-apiruff-strict-budget.json— TID251 baseline 2404 → 2662scripts/check_type_discipline.py— new vendored AST checker (LIT006/LIT007)scripts/type_discipline_gate.py— new baseline+slack gatetype-discipline-budget.json— LIT006/LIT007 budgets.github/workflows/test-linting.yml— run the gate in CITest plan
test-lintingworkflow passes on this PR (no net-new casts/guards introduced here).cast()(verified locally).# cast-ok: <reason>/# guard-ok: <reason>suppressions are honored.