chore(lint): emit backend implicit/explicit Any counts for the trend dashboard - #32822
chore(lint): emit backend implicit/explicit Any counts for the trend dashboard#32822yuneng-berri wants to merge 1 commit into
Conversation
Greptile SummaryThis PR adds
Confidence Score: 4/5Safe to merge; the change is purely additive build/CI tooling with no proxy or SDK surface and no gate side effects. The core logic (count extraction, vacuous-run guard, zero-fill, sort order) is correct and well-tested. Three minor issues exist: the test file's module-level exec_module mutates sys.path at collection time making import failures harder to diagnose; the workflow's date-only branch name will collide if the automation is triggered twice in one day; and the flat merge in measured_counts silently overwrites on a namespace collision rather than asserting. None affect the correctness of the committed metrics file or block any developer workflow. .github/workflows/update-backend-lint-metrics.yml (branch name collision on same-day reruns) and tests/test_litellm/test_lint_metrics.py (collection-time sys.path mutation).
|
| Filename | Overview |
|---|---|
| scripts/lint_metrics.py | New script that generates backend-lint-metrics.json; logic is sound — reuses gate helpers, vacuous-run guard works correctly. sys.path.insert at module level is a side-effect worth noting for test isolation. |
| tests/test_litellm/test_lint_metrics.py | New test file; tests cover zero-fill, tracked-only filtering, sort order, and dashboard-key guarantees. Module-level exec_module mutates sys.path at collection time — minor but worth noting. No network calls. |
| .github/workflows/update-backend-lint-metrics.yml | Weekly scheduled workflow; actions are pinned by commit SHA, persist-credentials: false + explicit PAT push URL pattern is correct. Branch naming is date-only, so a second run on the same day would fail the push step. |
| Makefile | Adds lint-metrics target; correctly depends on LINT_DEP_INSTALL, is kept out of the main lint/lint-checks targets, and matches the workflow pipeline exactly. |
| backend-lint-metrics.json | Committed snapshot of 179 rules; all keys are sorted alphabetically matching the generator output, format is consistent, and the counts align with the verification data in the PR description. |
Reviews (1): Last reviewed commit: "Merge origin/litellm_internal_staging in..." | Re-trigger Greptile
| _MODULE_PATH = _ROOT / "scripts" / "lint_metrics.py" | ||
| _spec = importlib.util.spec_from_file_location("lint_metrics", _MODULE_PATH) | ||
| metrics_mod = importlib.util.module_from_spec(_spec) | ||
| _spec.loader.exec_module(metrics_mod) |
There was a problem hiding this comment.
Module-level exec_module mutates sys.path at collection time
exec_module is called at the top level of the test file, so pytest runs it during collection (before any test executes). lint_metrics.py does sys.path.insert(0, str(Path(__file__).resolve().parent)) at module scope, which prepends scripts/ to the test process's sys.path for the rest of the session. If any gate module (ruff_strict_gate, type_check_gate, type_discipline_gate) fails to import, the entire test file surfaces as a collection error rather than a test failure, making the cause harder to diagnose. A pytest.fixture or a module-scoped fixture that calls exec_module would scope the side effect and produce a cleaner failure message.
| DATE=$(date +'%Y-%m-%d') | ||
| BRANCH="backend-lint-metrics-$DATE" | ||
| git config user.name "github-actions[bot]" | ||
| git config user.email "github-actions[bot]@users.noreply.github.com" | ||
| git checkout -b "$BRANCH" | ||
| git add backend-lint-metrics.json | ||
| git commit -m "chore(lint): refresh backend-lint-metrics.json ($DATE)" | ||
| git push "https://x-access-token:${GH_TOKEN}@github.com/${GITHUB_REPOSITORY}.git" "HEAD:$BRANCH" |
There was a problem hiding this comment.
Same-day reruns will fail on duplicate branch name
BRANCH is keyed only by date (backend-lint-metrics-YYYY-MM-DD). If the scheduled run and a manual workflow_dispatch (with dry_run=false) both execute on the same Monday, or if the workflow is manually re-triggered after a partial failure, the second git push will be rejected because the remote branch already exists. Adding a short timestamp (e.g., $(date +'%Y-%m-%d-%H%M')) or checking for an existing remote branch before pushing would prevent this failure. Since the workflow is non-required this only blocks the automation, not developers, but it will need manual cleanup of the orphaned branch.
| def measured_counts(basedpyright_counts: dict[str, int]) -> dict[str, int]: | ||
| """Real per-rule counts across all three tools. Rule namespaces are disjoint | ||
| (``report*`` vs ruff codes vs ``LIT*``), so the flat merge cannot collide.""" | ||
| return { | ||
| **basedpyright_counts, | ||
| **ruff_strict_gate.count_by_rule(ruff_strict_gate.head_violations()), | ||
| **type_discipline_gate.count_by_rule(type_discipline_gate.head_violations()), | ||
| } |
There was a problem hiding this comment.
Namespace disjointness is assumed but not verified at runtime
The flat merge in measured_counts relies on the comment that report*, ruff codes, and LIT* namespaces cannot collide. If a future budget addition were to introduce a rule code that exists in two tools (e.g., a hypothetical ruff rule spelled identically to a basedpyright rule), the second dict would silently overwrite the first with no warning. A cheap assertion like assert not (set(basedpyright_counts) & set(ruff_counts) & set(td_counts)) after construction would catch this during any run rather than producing a quietly wrong metric.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…dashboard Add backend-lint-metrics.json with the real reportAny (implicit Any) and reportExplicitAny (explicit Any) counts across the tree. The basedpyright budget's limit stopped tracking the real count after its limit collapse in #31883, so a trend dashboard reading that file went stale; this reports the real numbers instead. scripts/lint_metrics.py regenerates it, reusing type_check_gate's counting so the numbers match what CI measures, and a weekly workflow opens a PR when they drift. Report-only: nothing gates on the file
e0e7450 to
08d06aa
Compare
Relevant issues
Follow-up to the budget collapse in #31883, which replaced
{baseline, slack}with a single downward-onlylimitin the backend budget files.limitis now a loose ceiling (~1.5x the last real count), so anything readingbasedpyright-code-budget.jsonto track how manyanys exist reads a frozen ceiling, not the countLinear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
count_basedpyright; see Changes)@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
Build/CI tooling change with no proxy or LLM surface, so the proof is the generator producing the file and the numbers being the real count, not the budget ceiling.
These are the real count, not the
limitfrom the budget file, which carries ~1.5x headroom:Both line up with the pre-collapse baselines (~24,989 and ~6,931) plus drift since 2026-07-01, confirming they are measured, not derived. Nothing in the repo reads this file as a gate: the budget ratchet check reads a hardcoded list of the three
*-budget.jsonfiles and never looks at it, and this new workflow is not a required checkType
🚄 Infrastructure
Changes
Adds
backend-lint-metrics.jsonat the repo root: a flat{rule: count}file holding the realreportAny(implicit Any) andreportExplicitAny(explicit Any) counts across the tree, for a downstream trend dashboard to read.scripts/lint_metrics.pyregenerates it from pipedbasedpyright --outputjson, reusingtype_check_gate.count_basedpyrightso the numbers are exactly the ones CI measures. A weekly scheduled workflow (update-backend-lint-metrics.yml) opens a PR when the counts drift, mirroringauto_update_price_and_context_window.ymlfor the PR andtest-linting.ymlfor the environment; weekly because a whole-tree basedpyright pass takes minutes and the counts move slowly, so every PR is spared the cost.The file is report-only and never gates. The budget ceilings and their PR gates are unchanged, the workflow is not a required check, and it only opens a PR that a human merges, so a failed or skipped refresh never blocks anyone. On tests: the only new logic is a two-key extraction over
count_basedpyright, which is already covered bytests/test_litellm/test_type_check_gate.py, so I did not add a separate low-value test for the filter