From f98ef9b83e93f29b7614df1be9b30495df02bc54 Mon Sep 17 00:00:00 2001 From: Thomas Schmelzer Date: Thu, 8 Oct 2026 18:00:25 +0400 Subject: [PATCH] chore: gate docstring coverage at 100% with interrogate Adds [tool.interrogate] to collector/pyproject.toml (fail-under = 100, exempting __init__, dunders and nested functions) and the interrogate hook, pointed at that table, to .pre-commit-config.yaml so CI enforces it. Writes the 47 docstrings the package was missing to get there from 76.2%. Co-Authored-By: Claude Opus 5.5 --- .pre-commit-config.yaml | 15 ++++++++++++--- collector/jq_collector/__main__.py | 2 ++ collector/jq_collector/config.py | 3 +++ collector/jq_collector/forge.py | 6 ++++-- collector/jq_collector/github_api.py | 7 +++++++ collector/jq_collector/gitlab.py | 6 ++++++ collector/jq_collector/localgit.py | 1 + collector/jq_collector/metrics.py | 16 ++++++++++++++++ collector/jq_collector/origin.py | 2 ++ collector/jq_collector/repos.py | 1 + collector/jq_collector/state.py | 8 ++++++++ collector/pyproject.toml | 12 ++++++++++++ 12 files changed, 74 insertions(+), 5 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 2afc8a3..6444e1a 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -7,9 +7,10 @@ # this repo does not have. # - check-jsonschema for template-bundles.yml and check-renovate: neither file # exists here. -# - interrogate: rhiza's hook reads [tool.interrogate], and without that table -# interrogate falls back to its own defaults - a standard nobody here chose. -# Add the table first if docstring coverage should be gated. +# - interrogate: rhiza's hook reads [tool.interrogate] from the root; here the +# table lives in collector/pyproject.toml, so the hook is pointed at it and +# at the package, and runs over the whole package rather than the changed +# files - fail-under is a share of everything, not of one commit's files. # - bandit: rhiza passes --ini .bandit; here it is scoped to the package by path # instead, since the tests assert freely and that is not a finding. # - ruff is pinned to the version in collector/uv.lock, which is what CI runs, @@ -76,6 +77,14 @@ repos: - id: validate-pyproject files: ^collector/pyproject\.toml$ + - repo: https://github.com/econchick/interrogate + rev: 1.7.0 + hooks: + - id: interrogate + args: [--config, collector/pyproject.toml, collector/jq_collector] + pass_filenames: false + files: ^collector/(jq_collector/.*\.py|pyproject\.toml)$ + - repo: https://github.com/PyCQA/bandit rev: 1.9.4 hooks: diff --git a/collector/jq_collector/__main__.py b/collector/jq_collector/__main__.py index 082cd27..c921c0f 100644 --- a/collector/jq_collector/__main__.py +++ b/collector/jq_collector/__main__.py @@ -33,6 +33,7 @@ def _refresh_local(cfg: Config, store: Store) -> None: + """Rescan every working copy and publish it, reusing measurements that have not moved.""" snap = store.snapshot() branches = {name: repo.default_branch for name, repo in snap.remote.items()} # The previous scan is the measurement cache: a repo that has not moved @@ -186,6 +187,7 @@ def _loop( def main() -> None: + """Seed both sources, then serve ``/metrics`` while refreshing each on its own interval.""" logging.basicConfig( level=os.environ.get("JQ_LOG_LEVEL", "INFO").upper(), format="%(asctime)s %(levelname)-7s %(name)s %(message)s", diff --git a/collector/jq_collector/config.py b/collector/jq_collector/config.py index d5c690d..788265b 100644 --- a/collector/jq_collector/config.py +++ b/collector/jq_collector/config.py @@ -13,6 +13,7 @@ def _int(name: str, default: int) -> int: + """The integer in environment variable ``name``, or ``default`` when unset or blank.""" raw = os.environ.get(name) if raw is None or raw.strip() == "": return default @@ -20,6 +21,7 @@ def _int(name: str, default: int) -> int: def _csv(name: str, default: tuple[str, ...] = ()) -> tuple[str, ...]: + """Environment variable ``name`` as a tuple of non-empty, stripped comma-separated parts.""" raw = os.environ.get(name) if raw is None or raw.strip() == "": return default @@ -201,6 +203,7 @@ def __post_init__(self) -> None: ) def is_ignored(self, owner: str, name: str) -> bool: + """Whether JQ_IGNORE names this repo, by bare name or as ``owner/name``.""" return name in self.ignore or f"{owner}/{name}" in self.ignore def forge_for(self, full_name: str) -> str: diff --git a/collector/jq_collector/forge.py b/collector/jq_collector/forge.py index af0df10..4b661c6 100644 --- a/collector/jq_collector/forge.py +++ b/collector/jq_collector/forge.py @@ -93,9 +93,11 @@ def collect( self, ref_cache: dict[str, tuple[str, str]], coverage_cache: dict[str, tuple[int, tuple[float, int] | None]], - ) -> tuple[dict[str, RemoteRepo], frozenset[str]]: ... + ) -> tuple[dict[str, RemoteRepo], frozenset[str]]: + """This forge's repos keyed by ``namespace/name``, and the ones it dropped.""" - def close(self) -> None: ... + def close(self) -> None: + """Release the client's connections.""" # -- shared by both collectors ----------------------------------------------- diff --git a/collector/jq_collector/github_api.py b/collector/jq_collector/github_api.py index e61325f..55c10e1 100644 --- a/collector/jq_collector/github_api.py +++ b/collector/jq_collector/github_api.py @@ -42,6 +42,8 @@ class GitHub: + """A GitHub REST client for one refresh, tracking the rate limit off every response.""" + def __init__(self, cfg: Config) -> None: self._cfg = cfg headers = {"Accept": _ACCEPT, "X-GitHub-Api-Version": "2022-11-28"} @@ -58,14 +60,17 @@ def __init__(self, cfg: Config) -> None: self.rate_reset = 0.0 def close(self) -> None: + """Release the client's connections.""" self._client.close() def _get(self, path: str, **params: str | int) -> httpx.Response: + """GET ``path``, noting the rate-limit headers whatever the status.""" response = self._client.get(path, params=params or None) self._note_rate_limit(response) return response def _note_rate_limit(self, response: httpx.Response) -> None: + """Copy the ``x-ratelimit-*`` headers onto the client; a malformed one is skipped.""" for header, attr in ( ("x-ratelimit-remaining", "rate_remaining"), ("x-ratelimit-limit", "rate_limit"), @@ -105,6 +110,7 @@ def _bytes(self, path: str) -> bytes | None: return response.content def _paginate(self, path: str, **params: str | int) -> list[dict[str, Any]]: + """Every item of a list endpoint, 100 a page, stopping at an empty or short page or page 10.""" items: list[dict[str, Any]] = [] page = 1 while True: @@ -195,6 +201,7 @@ def release_tags(self, full_name: str) -> list[str]: # -- per repo -------------------------------------------------------- def branch_sha(self, full_name: str, branch: str) -> str: + """The commit sha at the tip of ``branch``, or ``""`` if it cannot be read.""" data = self._json(f"/repos/{full_name}/branches/{branch}") if not isinstance(data, dict): return "" diff --git a/collector/jq_collector/gitlab.py b/collector/jq_collector/gitlab.py index 26ddd4f..9b6cca5 100644 --- a/collector/jq_collector/gitlab.py +++ b/collector/jq_collector/gitlab.py @@ -80,6 +80,8 @@ def _pid(full_name: str) -> str: class GitLab: + """A GitLab REST client for one refresh, authenticated with GITLAB_TOKEN when set.""" + def __init__(self, cfg: Config) -> None: self._cfg = cfg headers = {"Accept": "application/json"} @@ -95,6 +97,7 @@ def __init__(self, cfg: Config) -> None: ) def close(self) -> None: + """Release the client's connections.""" self._client.close() def _json(self, path: str, **params: str | int) -> object | None: @@ -130,6 +133,7 @@ def _text(self, path: str, **params: str | int) -> str | None: return response.text if response is not None else None def _paginate(self, path: str, **params: str | int) -> list[dict[str, Any]]: + """Every item of a list endpoint, stopping at an empty or short page or page 10.""" items: list[dict[str, Any]] = [] page = 1 while True: @@ -183,6 +187,7 @@ def protected_branch(self, full_name: str, branch: str) -> dict[str, Any] | None return raw if isinstance(raw, dict) else None def branch_sha(self, full_name: str, branch: str) -> str: + """The commit sha at the tip of ``branch``, or ``""`` if it cannot be read.""" data = self._json( f"/projects/{_pid(full_name)}/repository/branches/{quote(branch, safe='')}" ) @@ -295,6 +300,7 @@ def open_merge_requests(self, full_name: str) -> tuple[int, list[PullRequest]]: return total, pulls def recent_merges(self, full_name: str, limit: int) -> list[MergedPull]: + """The ``limit`` most recently merged merge requests, newest first; any without a merge time are dropped.""" raw = self._json( f"/projects/{_pid(full_name)}/merge_requests", state="merged", diff --git a/collector/jq_collector/localgit.py b/collector/jq_collector/localgit.py index a810d0e..2050440 100644 --- a/collector/jq_collector/localgit.py +++ b/collector/jq_collector/localgit.py @@ -234,6 +234,7 @@ def _commit_counts(path: str, ref: str) -> tuple[int, int | None, str]: def _ahead_behind(path: str) -> tuple[int | None, int | None]: + """``(ahead, behind)`` against the branch's upstream, or ``(None, None)`` with no upstream.""" counts = _git(path, "rev-list", "--left-right", "--count", "@{upstream}...HEAD") if not counts: return None, None diff --git a/collector/jq_collector/metrics.py b/collector/jq_collector/metrics.py index 39cb029..16094b7 100644 --- a/collector/jq_collector/metrics.py +++ b/collector/jq_collector/metrics.py @@ -40,10 +40,12 @@ def _gauge(name: str, doc: str, labels: list[str] | None = None) -> GaugeMetricFamily: + """A gauge family with ``labels`` (none by default).""" return GaugeMetricFamily(name, doc, labels=labels or []) def render(snap: Snapshot) -> Iterator[Metric]: + """The whole exposition for one snapshot: collector health first, then every repo's series.""" yield from _health(snap) f = _Families() @@ -358,6 +360,7 @@ def __init__(self) -> None: ) def in_exposition_order(self) -> tuple[GaugeMetricFamily, ...]: + """Every family, in the order the exposition lists them.""" return ( self.repo_info, self.cloned, @@ -409,6 +412,7 @@ def in_exposition_order(self) -> tuple[GaugeMetricFamily, ...]: def _add_identity( f: _Families, key: str, remote: RemoteRepo | None, local: LocalRepo | None ) -> None: + """The repo's info series (owner, branch, visibility, forge, URLs) and whether it is cloned here.""" # rpartition, not partition: a GitLab namespace nests, so the owner is # everything before the last slash rather than the first segment. owner = key.rpartition("/")[0] @@ -428,10 +432,12 @@ def _add_identity( def _default_branch(remote: RemoteRepo | None) -> str: + """The forge's default branch, or ``main`` when the forge has not answered.""" return remote.default_branch if remote else "main" def _add_remote(f: _Families, ident: list[str], remote: RemoteRepo) -> None: + """Every series the forge's answer feeds.""" f.pushed.add_metric(ident, remote.pushed_at) _add_protection(f, ident, remote) _add_alerts(f, ident, remote) @@ -442,6 +448,7 @@ def _add_remote(f: _Families, ident: list[str], remote: RemoteRepo) -> None: def _add_protection(f: _Families, ident: list[str], remote: RemoteRepo) -> None: + """Branch-protection series, left out entirely when the forge would not say.""" if remote.protected is not None: f.protected.add_metric(ident, 1 if remote.protected else 0) f.required_reviews.add_metric(ident, remote.required_reviews) @@ -449,6 +456,7 @@ def _add_protection(f: _Families, ident: list[str], remote: RemoteRepo) -> None: def _add_alerts(f: _Families, ident: list[str], remote: RemoteRepo) -> None: + """Whether alerts are on and, if so, the open count per severity.""" f.alerts_enabled.add_metric(ident, 1 if remote.alerts_enabled else 0) if remote.alerts_enabled: # Zero-fill the severities GitHub uses, so a repo that has just @@ -462,6 +470,7 @@ def _add_alerts(f: _Families, ident: list[str], remote: RemoteRepo) -> None: def _add_drift(f: _Families, ident: list[str], remote: RemoteRepo) -> None: + """Template series: managed or not, the pinned ref, and how many releases it is behind.""" f.managed.add_metric(ident, 1 if remote.rhiza_managed else 0) if remote.rhiza_ref: f.ref_info.add_metric([*ident, remote.rhiza_ref], 1) @@ -470,6 +479,7 @@ def _add_drift(f: _Families, ident: list[str], remote: RemoteRepo) -> None: def _latest_per_workflow(workflows: tuple[WorkflowRun, ...]) -> dict[str, WorkflowRun]: + """The newest conclusive run of each workflow name, so no label set appears twice.""" # Collapse workflows sharing a name, newest run winning. github.py # already guarantees one per name, but this layer owns the exposition # contract: duplicate label sets are silently dropped by Prometheus @@ -485,6 +495,7 @@ def _latest_per_workflow(workflows: tuple[WorkflowRun, ...]) -> dict[str, Workfl def _add_ci(f: _Families, ident: list[str], remote: RemoteRepo) -> None: + """Per-workflow verdicts and times, and the repo-level CI verdict.""" bad = 0 for wf in _latest_per_workflow(remote.workflows).values(): good = wf.conclusion in _GOOD_CONCLUSIONS @@ -512,6 +523,7 @@ def _add_ci(f: _Families, ident: list[str], remote: RemoteRepo) -> None: def _add_pulls(f: _Families, ident: list[str], remote: RemoteRepo) -> None: + """Open pull request and issue counts, and one info series per open pull request.""" f.pr_count.add_metric(ident, remote.open_pulls_total) f.issue_count.add_metric(ident, remote.open_issues) # Red means red. A cancelled check is no verdict - the same rule the @@ -536,6 +548,7 @@ def _add_pulls(f: _Families, ident: list[str], remote: RemoteRepo) -> None: def _add_merged(f: _Families, ident: list[str], remote: RemoteRepo) -> None: + """One series per recently merged pull request, valued at its merge time.""" # One series per recently merged PR. The value is the merge time, so # the board can take topk() across the fleet rather than needing a # per-repo view. Deduped on number because a repo occasionally @@ -550,6 +563,7 @@ def _add_merged(f: _Families, ident: list[str], remote: RemoteRepo) -> None: def _add_local(f: _Families, ident: list[str], local: LocalRepo, remote: RemoteRepo | None) -> None: + """Every series the working copy feeds.""" _add_working_copy(f, ident, local, remote) _add_upstream(f, ident, local, remote) _add_size(f, ident, local) @@ -584,6 +598,7 @@ def _add_upstream( def _add_size(f: _Families, ident: list[str], local: LocalRepo) -> None: + """Line counts, recent commits and the release series for the checkout.""" f.code_lines.add_metric(ident, local.code_lines) f.test_lines.add_metric(ident, local.test_lines) f.commits_30d.add_metric(ident, local.commits_30d) @@ -602,4 +617,5 @@ def __init__(self, store: Store) -> None: self._store = store def collect(self) -> Iterator[Metric]: + """The current snapshot, rendered. prometheus_client calls this on every scrape.""" yield from render(self._store.snapshot()) diff --git a/collector/jq_collector/origin.py b/collector/jq_collector/origin.py index 7e1bf96..3358aec 100644 --- a/collector/jq_collector/origin.py +++ b/collector/jq_collector/origin.py @@ -43,10 +43,12 @@ class Origin: @property def full_name(self) -> str: + """``namespace/name``, the repo's key everywhere else in the collector.""" return f"{self.namespace}/{self.name}" @property def forge(self) -> str: + """Which forge the origin's host is.""" return forge_for_host(self.host) diff --git a/collector/jq_collector/repos.py b/collector/jq_collector/repos.py index cd2c488..e444115 100644 --- a/collector/jq_collector/repos.py +++ b/collector/jq_collector/repos.py @@ -82,6 +82,7 @@ def resolve_path(raw: str, host_root: str) -> str: def _is_checkout(path: str) -> bool: + """Whether ``path`` is a git checkout, plain or a worktree.""" # `.git` is a directory in a plain checkout and a file in a worktree. return os.path.exists(os.path.join(path, ".git")) diff --git a/collector/jq_collector/state.py b/collector/jq_collector/state.py index 18d2079..dfceac1 100644 --- a/collector/jq_collector/state.py +++ b/collector/jq_collector/state.py @@ -13,6 +13,8 @@ @dataclass(frozen=True) class PullRequest: + """An open pull (or merge) request, with its checks rolled up to one word.""" + number: int title: str author: str @@ -171,6 +173,8 @@ class LocalRepo: @dataclass(frozen=True) class SourceHealth: + """How one source's refreshes are going: last success, its duration, and the errors since.""" + last_success: float = 0.0 last_duration: float = 0.0 errors: int = 0 @@ -209,18 +213,22 @@ def __init__(self) -> None: self._snapshot = Snapshot() def snapshot(self) -> Snapshot: + """The current snapshot. It is immutable, so a caller may keep it after the store moves on.""" with self._lock: return self._snapshot def update(self, **changes: object) -> None: + """Replace the given snapshot fields at once, so a reader never sees half an update.""" with self._lock: self._snapshot = replace(self._snapshot, **changes) # type: ignore[arg-type] def record_health(self, source: str, health: SourceHealth) -> None: + """Set one source's health, leaving the other sources' alone.""" with self._lock: health_map = dict(self._snapshot.health) health_map[source] = health self._snapshot = replace(self._snapshot, health=health_map) def health_for(self, source: str) -> SourceHealth: + """One source's health, or a fresh SourceHealth if it has never reported.""" return self.snapshot().health.get(source, SourceHealth()) diff --git a/collector/pyproject.toml b/collector/pyproject.toml index 9400b41..095ae75 100644 --- a/collector/pyproject.toml +++ b/collector/pyproject.toml @@ -44,6 +44,18 @@ branch = true # CI's test step fails on a drop without a --cov-fail-under of its own. fail_under = 100 +[tool.interrogate] +# Every module, class and function in the package carries a docstring, and this +# keeps it so - the hook in .pre-commit-config.yaml reads this table, and CI runs +# that hook. Three kinds are exempt because something else already documents +# them: __init__ (the class docstring), dunder methods (the data model), and +# functions nested in another (the enclosing function, which is what a reader +# looks at - these are the per-repo closures handed to a thread pool). +fail-under = 100 +ignore-init-method = true +ignore-magic = true +ignore-nested-functions = true + [tool.mypy] # The package only, checked against the oldest Python it claims to run on, and # strict: every signature typed, every generic parameterised. JSON payloads are