Skip to content

ci(guardrails): style gates — deprecated-pattern scanner, allowlist ratchet, PR checklist (FO-03)#364

Merged
lmoresi merged 4 commits into
developmentfrom
feature/guardrails
Jul 15, 2026
Merged

ci(guardrails): style gates — deprecated-pattern scanner, allowlist ratchet, PR checklist (FO-03)#364
lmoresi merged 4 commits into
developmentfrom
feature/guardrails

Conversation

@lmoresi

@lmoresi lmoresi commented Jul 15, 2026

Copy link
Copy Markdown
Member

Phase 4 of the 2026-07 quality campaign (FO-03): machine-enforce the cheapest-to-check rules of docs/developer/UW3_STYLE_CHARTER.md so the cleaned-up state cannot silently drift back.

What each gate checks

scripts/check_deprecated_patterns.py (stdlib only, line-based, heuristics documented in its module docstring) scans src/ for:

Pattern id Charter Flags
access-context §7 with mesh.access(...) / with swarm.access(...)
mesh-data §7 mesh.data coordinate reads
hedging-name §3 def maybe_* / def try_* / def do_* (incl. _-private)
except-pass §4 except ...: + bare pass with no comment on the adjacent lines

Comment lines are skipped (commented-out legacy .access() references in the .pyx files do not trip the gate). Documented exclusions: self.data inside Mesh methods (not reliably distinguishable by a line scanner) and general string/docstring mentions.

.github/workflows/style-gates.yml runs the scanner on every PR to development and main — checkout + setup-python only, no PETSc build, well under a minute — and fails with the offending lines plus pointers to the Charter, docs/developer/guides/style-gates.md, and the allowlist policy. A second step prints a report-only docs/ inventory (currently 254 hits: 161 access-context, 76 mesh-data, 17 except-pass) without affecting the result.

Allowlist inventory (baseline development @ 9f5ed9e)

87 legacy hits across 33 files, recorded in scripts/deprecated_pattern_allowlist.txt as path:pattern-id lines. The allowlist may only shrink — the header, the scanner output, and the CI failure message all state this; the scanner reports stale entries so cleanups delete them.

  • access-context (13 hits, 1 file): src/underworld3/tests/test_adaptivity_metrics.py — legacy test written against the old context-manager API.
  • mesh-data (2 hits, 1 file): the deprecated property's own docstring and DeprecationWarning message in discretisation_mesh.py — the shim implementation, not usage.
  • hedging-name (5 hits, 4 files): _maybe_install_snes_update (the Charter §3 example itself) and four _do_move() mover closures. Renaming them is library-code churn outside this PR's scope (Charter §9).
  • except-pass (67 hits, 30 files): uncommented swallows concentrated in units.py (8), discretisation_mesh.py (8), coordinates.py (7), petsc_maths.pyx (4), smoothing/monge_ampere.py (4). Each needs a one-line sanctioned-failure-mode comment; the intended workflow is to add it in files you already touch and delete the allowlist entry.

The shim-warning regression net (tests/test_0641_wave_c_api_shims.py, already in the level_1 + tier_a CI gate) complements these static checks by covering the deprecation warnings the shimmed APIs must keep emitting.

Black investigation and recommendation

black --check src/underworld3 (black 24.10.0, already present in the amr-dev env — nothing installed): 81 of 95 files would be reformatted. A format gate would therefore fail essentially every PR that touches library code, and a changed-files-only advisory would be permanently red for the same reason — pure noise. Recommendation: no format gate now. If the maintainers want black later, adopt it as one dedicated tree-wide reformat commit (with a .git-blame-ignore-revs entry) plus a gate in the same PR. This is recorded in docs/developer/guides/style-gates.md so nobody "fixes" it piecemeal. Nothing was reformatted in this PR.

Local red/green demonstration

  • Green at tip: python scripts/check_deprecated_patterns.py → exit 0, OK: no new deprecated patterns (87 known legacy hit(s) allowlisted).
  • Red on a synthetic violation: a scratch file containing with mesh.access(var): pointed at via python scripts/check_deprecated_patterns.py <scratch-dir> → exit 1, hit reported with Charter pointer (file not committed).
  • Raw inventory (--no-allowlist) → exit 1 with all 87 hits, confirming the allowlist is load-bearing.

Other deliverables

  • .github/pull_request_template.md: nine-line Charter checklist (no template existed before).
  • Charter §10: four-line machine-enforcement pointer (only Charter change).
  • docs/developer/guides/release-process.md: style gates must be green on the release branch (the development → main PR runs them automatically).
  • docs/developer/guides/style-gates.md: compact guide (checks, shrink-only policy, local usage, tripped-gate procedure), linked from the developer guides toctree. Docs build passes; no new warnings.

Left for the maintainer

  • .claude/commands/check-patterns.md should point at the scanner as the single source of truth for the src/ pattern list (keeping its manual notebook/docs sweeps); the session's permission policy blocks edits under .claude/, so that one-paragraph edit is left to a maintainer.
  • The 254 docs/ hits stay report-only; promoting docs to a gated root is a separate cleanup.
  • No pixi.toml/pixi.lock or library-code changes anywhere in this PR.

Underworld development team with AI support from Claude Code

lmoresi added 3 commits July 14, 2026 14:53
…lowlist ratchet

Scans src/ for the Charter's cheapest-to-check banned patterns:
access-context (S7), mesh.data (S7), hedging names maybe_/try_/do_ (S3),
and uncommented except/pass swallows (S4). Line-based, no dependencies;
comment lines skipped; heuristics and exclusions documented in the module
docstring. --include-docs adds a report-only docs/ inventory.

The allowlist records the 87 legacy hits at baseline (development@9f5ed9e9)
as path:pattern-id lines and may only shrink; the scanner reports stale
entries so cleanups delete them.

Part of FO-03 (quality campaign 2026-07, Phase 4).

Underworld development team with AI support from Claude Code
style-gates.yml runs the deprecated-pattern scanner on every PR to
development and main (checkout + python only, no PETSc build, <1 min),
failing with the offending lines and a pointer to the Charter and the
shrink-only allowlist policy. A second step prints the docs/ inventory
report-only. No formatting gate: src/underworld3 is not black-clean
(81 of 95 files), so a format check would fail every library PR.

The PR template distils the Charter into a nine-line checklist.

Part of FO-03 (quality campaign 2026-07, Phase 4).

Underworld development team with AI support from Claude Code
…ters

Adds guides/style-gates.md (what the gates check, shrink-only allowlist
policy, local usage, what to do when a PR trips the gate, why there is
no formatting gate) and links it from the developer guides toctree.
Charter S10 gains a four-line machine-enforcement pointer; the release
process notes that style gates must be green on the release branch.

Part of FO-03 (quality campaign 2026-07, Phase 4).

Underworld development team with AI support from Claude Code
Copilot AI review requested due to automatic review settings July 15, 2026 01:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

…ide reference

The /check-patterns skill now points at scripts/check_deprecated_patterns.py as
the single source of truth for the src/ pattern list, and its reference list is
aligned with the Charter authority map (the .qmd guide no longer exists).

Underworld development team with AI support from Claude Code
@lmoresi lmoresi merged commit 045eac0 into development Jul 15, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants