Skip to content

refactor: split metrics.render and the D-ranked functions, pinned by a golden exposition (#39) - #46

Merged
tschm merged 1 commit into
mainfrom
rhiza_fix_39_20260925
Sep 25, 2026
Merged

tschm merged 1 commit into
mainfrom
rhiza_fix_39_20260925

Conversation

@tschm

@tschm tschm commented Sep 25, 2026

Copy link
Copy Markdown
Member

Fixes #39

Acceptance criterion (verbatim):

uvx radon cc jq_collector -nc reports no block worse than C, and render is below C. The existing 447 tests stay green, and the /metrics output is byte-identical for the same snapshot.

Route: the issue left the split open; the maintainer asked for it with fix #39. The approach: pin the output first, then refactor under that pin.

1. The pin: tests/test_metrics_golden.py + tests/golden/metrics.prom

  • One fixture fleet that takes every branch of render:
    • a fully populated repo, with duplicate and inconclusive workflows, a draft and a red PR, and a merged PR reported twice
    • a remote-only GitLab repo with a nested namespace, unknown protection, alerts off and a cancelled CI run
    • an unprotected repo whose clone is on a feature branch and out of sync
    • a local-only detached clone with no tags
    • an excluded repo
  • The test compares the whole exposition, # HELP/# TYPE lines included, as one string. It covers 222 lines and all 51 families.
  • The golden file was generated from the code on main before any refactor. The module docstring gives the one-liner that regenerates it after a deliberate exposition change, so the diff becomes the review.

2. The refactor (no behaviour change)

Block Before After
metrics.render F (50) A (4)
repos.load D (24) C (14)
GitHub.latest_runs D (22) C (11)
  • metrics.py: render now yields the fleet-wide families from _health, then fills a _Families holder repo by repo. The fillers are _add_identity, _add_protection, _add_alerts, _add_drift, _add_ci, _add_pulls, _add_merged, _add_local and _add_size, and in_exposition_order() yields them. The 44 family definitions were moved by script, not retyped, so no HELP string or label list could drift. The yield order is taken from the old yield from (...) tuple. Every comment moved with its code. The largest new block is _add_local at C (11).
  • repos.py: file reading and validation move to _read_entries, and the "listed twice" message moves to _listed_twice. The error messages are unchanged.
  • github.py: the feed scan becomes _newest_per_workflow (pure), and one backfill call becomes _newest_conclusive. The insertion order into newest, and so the returned order, is unchanged.

uvx radon cc jq_collector -nc now lists only C blocks, 11 in all, the worst at 18.

Checks (run locally from collector/, as CI does)

  • ruff check / ruff format --check / mypy: pass
  • 3.12: 451 passed (the 447 from the criterion, 3 doctests, and the golden test), 100.00% coverage, floor met
  • 3.11: 451 passed, 100.00%
  • Byte-identity, both directions:
    • with origin/main's metrics.py in a scratch copy, the golden test passes. So the golden file is the old output, and the new code reproduces it.
    • a one-character change to a HELP string ("Stash entries." → "Stash entries") fails it.

Not addressed

  • The remaining C blocks: localgit.scan_repo at 18, and _folder and _entry in repos.py at 17. The criterion stops at C.
  • A return annotation on render.

Closes #39

🤖 Generated with Claude Code

@tschm
tschm merged commit 1ee4a52 into main Sep 25, 2026
5 checks passed
@tschm
tschm deleted the rhiza_fix_39_20260925 branch September 25, 2026 11:39
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.

Split metrics.render (CC 50) and the other D-ranked functions

1 participant