ci: gate complexity with xenon, track its trend with wily - #356
Draft
thodson-usgs wants to merge 3 commits into
Draft
ci: gate complexity with xenon, track its trend with wily#356thodson-usgs wants to merge 3 commits into
thodson-usgs wants to merge 3 commits into
Conversation
Complexity is the one structural property that degrades without anyone deciding to degrade it: every change adds a branch for a good local reason, and the aggregate is nobody's decision. That makes it worth a gate rather than a review habit -- especially where coding agents contribute, since an agent optimizing for a passing test suite has no reason to notice a function crossing into unmaintainable. Add xenon as a ratchet, in CI and as a pre-commit hook running the identical check, so a clean pre-commit run means CI agrees. The thresholds -- no block above rank C, no module below B, average at A -- are the tightest the package passes today. That is deliberate: a gate set where the code already is holds the line without demanding a refactor first, and tightening a letter later is a visible commit rather than a silent drift. It failed on an injected rank-D function and named the block, which is the property that makes the fix local. Scope is the package, not the tests, which legitimately contain long branchy fixtures. The path is in args rather than passed by pre-commit because --max-average and --max-modules are whole-package judgements; computed over only the changed files, the average could drift upward one commit at a time while every individual commit passed. wily is advisory and never blocks. On a pull request the job writes a complexity diff against the base into the run summary, so a reviewer sees direction rather than a verdict. Rising complexity in a file that gained a genuinely complex feature is information, not a failure -- which is exactly why it is separate from the gate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`wily build` writes a .wily/ index of per-revision metrics. It is a local cache, rebuildable from git history in one command, and large enough to be worth keeping out of diffs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The two remaining find_spec checks were as redundant as the eight removed with the adapter-split cleanup: the script imports dataretrieval.ogc.engine directly, and waterdata/__init__.py imports .api eagerly and unconditionally, so both modules are already in sys.modules by the time the assertions run. A module missing from the wheel raises at the import statement several lines earlier -- the find_spec call is unreachable as a failure mode. Verified by extracting the script and running it both ways: it still passes when the package resolves outside GITHUB_WORKSPACE, and still fails when it resolves from inside the checkout, which is the regression it exists to catch (source-tree imports masking a broken wheel, DOI-USGS#347). importlib.util goes with them; importlib.resources.files is a separate import and stays. Co-Authored-By: Claude Opus 5 <noreply@anthropic.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
1 participant
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.
Complexity is the one structural property that degrades without anyone deciding to degrade it. Every change adds a branch for a good local reason; the aggregate is nobody's decision. That makes it worth a gate rather than a review habit — especially where coding agents contribute, since an agent optimizing for a passing test suite has no reason to notice a function crossing into unmaintainable.
What this adds
xenon as a ratchet — in CI and as a pre-commit hook running the identical check, so a clean pre-commit run means CI agrees.
wily as advisory trend — on a PR, the job writes a complexity diff against the base into the run summary. It never fails the build.
Why these thresholds
They are the tightest the package passes today:
--max-absolute Cogc/filters.py:_split_top_level_or)--max-modules B--max-average ATightening any one letter fails immediately —
--max-absolute B,--max-modules A, and--max-averageat a stricter grade were each tested and each fails.That is deliberate. A gate set where the code already is holds the line without demanding a refactor first, and tightening a letter later becomes a visible commit rather than a silent drift. The alternative — an aspirational threshold — gets disabled the first time it blocks someone.
Verified it actually bites
Injecting a rank-D function produced:
Naming the block is the property that matters — the fix is local, usually extracting a branch rather than restructuring. A first attempt at rank C passed, which is the correct behaviour and confirmed the threshold is where I claimed.
Two scoping decisions worth reviewing
The package, not the tests. Test modules legitimately contain long, branchy fixtures; gating them would generate noise with no design signal.
The path is in
args, not passed by pre-commit.--max-averageand--max-modulesare whole-package judgements. Computed over only the changed files, the average could drift upward one commit at a time while every individual commit passed.files:decides whether to run;argsdecides what to scan.Not a merge gate
wilyis advisory on purpose. Rising complexity in a file that gained a genuinely complex feature is information, not a failure — the distinction the book calls essential vs accidental complexity, which no tool can make for you. The gate blocks; the trend informs.Local use
pip install -e .[metrics] # or: uv sync --extra metrics wily build dataretrieval --max-revisions 50 wily report dataretrieval wily diff dataretrieval --revision mainCONTRIBUTING.mddocuments both.🤖 Generated with Claude Code