docs: compound code review findings
- Add verify-lint-rule-scope-before-relying-on-it: guidance on confirming lint rule coverage before trusting it for enforcement. Documents the P2-002 incident where ruff BLE only catches bare not . - Update working-tree-hygiene: add dev-tool-in-venv check and ruff dependency example.
This commit is contained in:
+80
@@ -0,0 +1,80 @@
|
|||||||
|
---
|
||||||
|
title: Verify Lint Rule Scope Before Relying on It for Enforcement
|
||||||
|
date: 2026-05-01
|
||||||
|
category: docs/solutions/best-practices
|
||||||
|
module: stemwijzer
|
||||||
|
date: "2026-05-01"
|
||||||
|
problem_type: best_practice
|
||||||
|
component: development_workflow
|
||||||
|
severity: medium
|
||||||
|
applies_when:
|
||||||
|
- Adding a new lint rule to catch a specific pattern
|
||||||
|
- Refactoring code to eliminate an anti-pattern
|
||||||
|
- Writing new modules while an anti-pattern cleanup is in progress
|
||||||
|
symptoms:
|
||||||
|
- Lint rule appears to enforce a rule but misses the variant you actually use
|
||||||
|
- New code re-introduces the anti-pattern you are actively removing
|
||||||
|
- False confidence that CI/pre-commit will catch regressions
|
||||||
|
root_cause: missing_tooling
|
||||||
|
resolution_type: workflow_improvement
|
||||||
|
related_components:
|
||||||
|
- ruff
|
||||||
|
- pre-commit
|
||||||
|
tags:
|
||||||
|
- linting
|
||||||
|
- ruff
|
||||||
|
- exception-handling
|
||||||
|
- self-review
|
||||||
|
- workflow
|
||||||
|
---
|
||||||
|
|
||||||
|
# Verify Lint Rule Scope Before Relying on It for Enforcement
|
||||||
|
|
||||||
|
## Context
|
||||||
|
|
||||||
|
During a refactor to tighten exception handling (P2-002), I added the ruff `BLE` (blind exception) rule to `.pre-commit-config.yaml` and `pyproject.toml`, believing it would catch `except Exception:` blocks. I then assumed the rule would prevent regressions and wrote new modules (`health/`, `scheduler.py`, `scripts/health_check.py`) that contained `except Exception:` — the exact pattern the refactor was meant to eliminate.
|
||||||
|
|
||||||
|
The BLE rule **only catches bare `except:`** (no exception type at all). It does **not** flag `except Exception:` or `except Exception as e:`. The result: the new modules passed linting while re-introducing the anti-pattern.
|
||||||
|
|
||||||
|
## Guidance
|
||||||
|
|
||||||
|
When adding a lint rule to enforce a pattern:
|
||||||
|
|
||||||
|
1. **Read the rule documentation** to confirm it catches the exact variant you care about. Do not assume the rule name or description covers all forms of the anti-pattern.
|
||||||
|
|
||||||
|
2. **Test the rule against your codebase** before relying on it. Run the linter on a file with the target pattern and verify it actually flags it.
|
||||||
|
|
||||||
|
3. **Check for rule gaps.** If the rule misses a variant you use (e.g., `except Exception:` but the rule only catches bare `except:`), either:
|
||||||
|
- Find a complementary rule that covers the gap, or
|
||||||
|
- Add a custom rule/regex check, or
|
||||||
|
- Rely on code review instead of automation for that variant.
|
||||||
|
|
||||||
|
4. **When cleaning up an anti-pattern, do not write new code that uses it.** This sounds obvious but is easy to violate when working across multiple files over a long session. Maintain a mental (or literal) checklist: "I am currently removing X; any new code I write must not contain X."
|
||||||
|
|
||||||
|
5. **Run the full linter on new files before committing.** Do not assume pre-commit catches everything — verify manually if the rule set is new or recently changed.
|
||||||
|
|
||||||
|
## Why This Matters
|
||||||
|
|
||||||
|
Lint rules create false confidence. If you believe a rule enforces a standard, you stop looking for violations manually. When the rule has a narrower scope than you assumed, violations slip through and accumulate. This is especially dangerous during refactors where new code is written alongside cleanup — the new code inherits the "old" bad habits because the safety net has a hole in it.
|
||||||
|
|
||||||
|
## When to Apply
|
||||||
|
|
||||||
|
- Adding any new lint rule (ruff, pylint, flake8, etc.)
|
||||||
|
- Refactoring to remove an anti-pattern across multiple files
|
||||||
|
- Setting up CI enforcement for a new code-quality rule
|
||||||
|
- Working on long-running refactor branches where new modules are created
|
||||||
|
|
||||||
|
## Examples
|
||||||
|
|
||||||
|
**Actual incident — ruff BLE rule scope mismatch:**
|
||||||
|
|
||||||
|
- Problem: `database.py` had broad `except Exception:` blocks that silently swallowed errors
|
||||||
|
- Assumption: ruff `BLE` rule would catch these and prevent regressions
|
||||||
|
- Reality: BLE only flags bare `except:` (no exception type). `except Exception:` passes silently.
|
||||||
|
- Result: New `health/checks.py`, `scheduler.py`, and `scripts/health_check.py` all contained `except Exception:` blocks. They passed pre-commit and CI.
|
||||||
|
- Fix: Remove or tighten the `except Exception:` blocks in new code. Consider adding a custom check or a stricter rule (e.g., `TRY` rules from `tryceratops` or a custom AST grep) if `except Exception:` must be banned project-wide.
|
||||||
|
|
||||||
|
## Related
|
||||||
|
|
||||||
|
- `docs/solutions/best-practices/working-tree-hygiene-dependency-groups-and-gitignore-2026-04-24.md` — related workflow hygiene guidance (pre-commit configuration, dev-tool availability)
|
||||||
|
- `docs/solutions/workflow-issues/verify-session-artifacts-against-canonical-sources-2026-04-24.md` — same principle applied to documentation: verify before trusting
|
||||||
+24
-2
@@ -59,8 +59,9 @@ For every set of uncommitted changes:
|
|||||||
|
|
||||||
1. **Dependencies**: Any new packages in the right group?
|
1. **Dependencies**: Any new packages in the right group?
|
||||||
2. **Gitignore**: Any duplicates or redundant patterns?
|
2. **Gitignore**: Any duplicates or redundant patterns?
|
||||||
3. **Blog/docs**: Any hardcoded numbers without canonical sources? (see `blog-numbers-from-pipeline-outputs`)
|
3. **Dev tools in venv**: Is the linter/formatter also listed in `[dependency-groups] dev` so `uv run <tool>` works? (A tool in pre-commit but not in `pyproject.toml` will fail for developers who run it manually.)
|
||||||
4. **Config**: Any secrets or local paths committed by accident?
|
4. **Blog/docs**: Any hardcoded numbers without canonical sources? (see `blog-numbers-from-pipeline-outputs`)
|
||||||
|
5. **Config**: Any secrets or local paths committed by accident?
|
||||||
|
|
||||||
## Why This Matters
|
## Why This Matters
|
||||||
|
|
||||||
@@ -110,6 +111,27 @@ dev = [
|
|||||||
-
|
-
|
||||||
- # Stray temp files
|
- # Stray temp files
|
||||||
- .worktrees # ← duplicate, remove
|
- .worktrees # ← duplicate, remove
|
||||||
|
**Dev tool missing from venv:**
|
||||||
|
|
||||||
|
A linter was configured in `.pre-commit-config.yaml` and `pyproject.toml`'s `[tool.ruff]` section, but it was not listed in `[dependency-groups] dev`. Developers running `uv run ruff check .` got a "command not found" error even though pre-commit worked.
|
||||||
|
|
||||||
|
```toml
|
||||||
|
# ❌ Before — tool configured but not installable
|
||||||
|
[tool.ruff.lint]
|
||||||
|
select = ["T20", "BLE"]
|
||||||
|
|
||||||
|
[dependency-groups]
|
||||||
|
dev = ["pytest>=9.0.2"]
|
||||||
|
|
||||||
|
# ✅ After — tool is both configured and installable
|
||||||
|
[tool.ruff.lint]
|
||||||
|
select = ["T20", "BLE"]
|
||||||
|
|
||||||
|
[dependency-groups]
|
||||||
|
dev = [
|
||||||
|
"pytest>=9.0.2",
|
||||||
|
"ruff>=0.11.0",
|
||||||
|
]
|
||||||
```
|
```
|
||||||
|
|
||||||
## Related
|
## Related
|
||||||
|
|||||||
Reference in New Issue
Block a user