ci: stabilize lint and benchmark checks#5
Merged
Conversation
CI was red on every PR for two reasons, both fixed here: - Lint: code was not `ruff format`-clean, and pre-commit (ruff 0.8.4) drifted from CI (latest, 0.15.20). Pin ruff to 0.15.20 across CI, pre-commit, and nox; apply `ruff format` (line-rewraps only, no logic change); ignore UP042 (LLMProvider deliberately uses (str, Enum) with an explicit __str__ — StrEnum migration deferred, it changes a public base). - Tests: wall-clock microbenchmarks ran inside the gating matrix and flaked on shared runners (p99 < 5ms). Deselect benchmarks from the matrix (`-m "not benchmark"`, coverage still 95.6%) and run them once in a dedicated ubuntu `benchmark` job with median-based assertions; p99 is now reported, not gated. Lint now also covers noxfile.py and scripts/. Verified locally with ruff 0.15.20: ruff check + format --check pass; 124 tests + 95.62% coverage; benchmarks 3 passed.
The lint fix unblocked two jobs that gate on it (`benchmark`, `security`), revealing two more failures — both false-positives from the gating config, not real problems: - Benchmarks: pyproject `addopts` applies `--cov-fail-under=90` to every pytest run, so the benchmark-only job (covers ~64%) failed the coverage gate even though all 3 benchmarks passed. Add `--no-cov`; coverage stays enforced in the matrix `test` job. - Security audit: `pip-audit --strict` hard-fails on any dependency it can't audit, and `pip install -e .` makes `tokenhelm` (not yet on PyPI) one of them. Use `--skip-editable` and drop `--strict`; pip-audit still exits non-zero on a real CVE, so the vulnerability gate is intact. Verified in a clean venv: benchmarks 3 passed; pip-audit "No known vulnerabilities found" (exit 0).
The Security audit job's gitleaks step failed with:
fatal: ambiguous argument '463c523^..d45eae0'
gitleaks-action scans the PR commit range (<first-commit>^..<head>), but
actions/checkout defaults to a shallow clone (fetch-depth: 1), which lacks the
parent of the PR's first commit — so git can't resolve the range and fails
before any scanning happens. The repo is clean (verified locally with gitleaks
8.30.1: detect + dir both report no leaks); this was purely a checkout-depth bug.
Set fetch-depth: 0 on the security job only (the sole job whose step needs full
history). Keeps the existing gitleaks-action; no scanner change.
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.
CI was red on every PR for two reasons, both fixed here:
ruff format-clean, and pre-commit (ruff 0.8.4) drifted from CI (latest, 0.15.20). Pin ruff to 0.15.20 across CI, pre-commit, and nox; applyruff format(line-rewraps only, no logic change); ignore UP042 (LLMProvider deliberately uses (str, Enum) with an explicit str — StrEnum migration deferred, it changes a public base).-m "not benchmark", coverage still 95.6%) and run them once in a dedicated ubuntubenchmarkjob with median-based assertions; p99 is now reported, not gated.Lint now also covers noxfile.py and scripts/. Verified locally with ruff 0.15.20: ruff check + format --check pass; 124 tests + 95.62% coverage; benchmarks 3 passed.
Summary
What does this PR do and why?
Type of change
fix— bug fix (no API change)feat— additive feature (new adapter / logger / storage / pricing / kwarg)docs— documentation onlyrefactor/perf/test/choreChecklist
pytestpasses with coverage ≥ 90%.ruff check src testsandruff format --check src testspass.docs/.CHANGELOG.mdupdated under[Unreleased].sdk).Notes for reviewers
Anything that needs special attention, trade-offs, or follow-ups.