Add a RuboCop metrics baseline ratchet

[justin808]

Why

The repository globally disables eight default RuboCop Metrics cops because the existing backlog is too large for one safe refactor. Without a machine-readable allowance, new complexity can accumulate while maintainers reduce that backlog.

Refs #309.

What changed

• Added a pinned, deterministic check and refresh helper. It groups offenses by tracked Ruby file and cop and stores sorted observed metric values.
• Made bin/lint reject new offenses, larger existing values, inline-disable bypasses, RuboCop version drift, and cop-set drift while preserving ordinary RuboCop diagnostics in the same run.
• Captured the RuboCop 1.87.0 baseline: 5,702 existing offenses across 153 files. Upstream thresholds and the temporary global disables are unchanged.
• Documented current totals, production-first priorities, deliberate refresh rules, and the path for removing each disable. The new Ruby helper and tests add no Metrics offenses.

How to review and verify

1. Review bin/rubocop-metrics-baseline for deterministic collection and rank-by-rank comparison, then inspect the JSON fixture as generated debt rather than hand-authored policy.
2. Run ruby bin/rubocop-metrics-baseline-test.rb, ruby bin/lint-test.rb, bin/rubocop-metrics-baseline check, and bin/lint.
3. Confirm that later baseline refreshes only reduce intended buckets and that each zero-total cop loses its .rubocop.yml disable.

Agent details

Commands and results

• ruby bin/rubocop-metrics-baseline-test.rb — 8 runs, 34 assertions, passed.
• ruby bin/lint-test.rb — 16 runs, 146 assertions, passed.
• ruby bin/validate-doc-links-test.rb — 16 runs, 20 assertions, passed.
• bin/rubocop-metrics-baseline refresh followed by check — passed; the review refactor tightened bin/lint-test.rb class length from 226 to 213, and a second refresh produced no fixture diff.
• bin/lint — passed all five linters with pinned yamllint 1.37.1 installed in an isolated temporary virtual environment.
• git diff --check — passed.
• Local bin/validate was intentionally not run under the implementation-lane constraint. Exact-head hosted Validate is the full-validation authority.

Exact-head and replay evidence

• Trusted base: db989c34d6910095a066beef07b67fb6be657c94.
• Current head: 27b3261bf4d90d7b25b8f9dab77ae7ee65633646.
• Base-drift check: GitHub main and the merge base still equal the trusted base.
• Stale-base race control: checklist+replay. A GitHub file-map sweep covered all 56 open main PRs and found 50, including this PR, that touch Ruby. The 49 other affected PRs must update to post-ratchet main and rerun the current lint/CI before merge.
• Motivating miss: a pre-ratchet PR head does not contain the new baseline gate.
• Replay evidence: the sweep used each open PR's complete changed-file list and a conservative Ruby-surface predicate: known Ruby suffixes and entrypoints plus every extensionless potential script.
• Non-goal: this PR does not mass-update, rebase, or otherwise mutate those independent lanes.

QA Evidence

• QA lane: implementation owner codex-m5-issue309-metrics-ratchet; focused local developer-workflow replay complete; exact-head hosted Lint and Claude review passed. Validate is blocked by an inherited prompt-budget failure reproduced on the trusted base.
• Scope checked: baseline generation and comparison, bin/lint propagation, documentation links, all repository lint surfaces, and baseline metadata drift.
• Tested at: PR Add a RuboCop metrics baseline ratchet #692 head 27b3261bf4d90d7b25b8f9dab77ae7ee65633646.
• Automated checks: focused commands above; exact-head hosted Lint https://github.com/shakacode/agent-workflows/actions/runs/33621379321 and Claude review https://github.com/shakacode/agent-workflows/actions/runs/33621379278 passed. Validate https://github.com/shakacode/agent-workflows/actions/runs/33621379277 failed only at the unchanged plan-pr-batch 299-character prompt headroom check; the same command fails identically on trusted base db989c34d6910095a066beef07b67fb6be657c94.
• Manual checks: direct bin/lint smoke test exercised the developer workflow and failure propagation is covered by a focused process test.
• User-visible UI change: no.
• Visual evidence: not applicable: no rendered output changed.
• Interaction change: no; not applicable because no UI interaction changed.
• Interaction evidence: not applicable: no interaction changed.
• Visual fix: no; not applicable because no visual behavior changed.
• Negative control: not applicable: no visual fix.
• Performance evidence: not applicable: no app/runtime or bundle surface changed; lint intentionally adds one pinned Metrics scan.
• Findings: all first-wave feedback was dispositioned in 3406dc4a; the exact-head Claude review found no correctness, security, or coverage issues. Hosted Validate remains blocked by the inherited 299/300 prompt-headroom failure.
• QA required: yes.
• QA required rationale: this changes the repository-wide developer lint workflow; focused process replay plus exact-head hosted validation covers it without a separate broad local validation root.
• QA lane status: blocked.
• Release-blocking status: blocked by the inherited plan-pr-batch 299/300 prompt-headroom failure.
• Process-gap disposition: checklist+replay.

Coordination and reviewer telemetry

• Coordination batch aw-medium-wave9-20260901; live claim holder codex-m5-issue309-metrics-ratchet; instance 284734b7-45a2-4cf1-b125-6f51f35d493c.
• Exact-target security preflight passed for this head with no untrusted participants, suspicious text, or GitHub API coverage gaps.
• Both Claude waves were triaged through the address-review workflow. Exact-head review passed with no correctness, security, or coverage issues, and all 22 inline threads have explicit outcomes and are resolved. Exact-head Lint passed; Validate is red only for the inherited 299/300 prompt-headroom check. CodeRabbit reports success but does not review draft PRs by default.

Decision log

• Non-blocking: Bind allowances to stable method identities or per-file ranked values.
  • Decision: Compare descending values per file and cop.
  • Why: This catches offense-count and severity growth without noise from renames or line movement.
  • Review later: Revisit only if real refactors expose masking that the rank distribution does not catch.
• Non-blocking: Honor inline disable comments during diagnostic collection.
  • Decision: Ignore inline disable comments so new code cannot bypass the ratchet; configuration-level exclusions remain reviewable.
  • Why: The gate must observe new debt even while the cops remain globally disabled.
  • Review later: None.

Merge confidence

Draft. Exact-head Lint and Claude review passed with no unresolved threads. Validate is blocked by the inherited plan-pr-batch 299/300 prompt-headroom failure, reproduced unchanged on the trusted base. No merge is authorized in this lane.

Audit receipts

No merge or completed-batch audit is in scope. Changelog classification: deferred_to_update_changelog.

[coderabbitai]

Important

Approval pending

CodeRabbit has no unresolved comments, but it skipped the latest review.

Use the checkbox below to review the latest commit. CodeRabbit will approve the changes if it finds no blocking issues.

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

Review summary

This is a solid, well-documented draft implementing a metrics ratchet for the disabled RuboCop Metrics cops. I posted inline comments on the specific issues found; overview below.

Correctness / robustness

• bin/rubocop-metrics-baseline's rubocop_arguments has no guard for an empty tracked-Ruby-files list (unlike the equivalent guard in bin/lint), so an empty list would fall through to RuboCop's default Include patterns instead of skipping the scan.
• bin/lint's check_metrics_baseline invokes the helper via system(...) with a direct path; if the script is missing/non-executable, system returns nil silently rather than surfacing the "install X" style diagnostic every other tool gets via check_version!.
• The bin/lint-test.rb split of the combined rubocop-command assertion dropped file-selection coverage (assert_includes/refute_includes) for the new metrics invocation, weakening the safety net for the baseline script's independent file-selection logic.

Design / maintenance overhead

• bin/rubocop-metrics-baseline re-implements roughly what RuboCop's own --auto-gen-config/.rubocop_todo.yml mechanism already provides, via bespoke JSON-fixture + offense-message regex parsing. The per-rank (sorted value) comparison does add something .rubocop_todo.yml doesn't (catching value increases hidden behind an unchanged offense count), but it's worth weighing whether that benefit justifies ~225 lines of custom parsing/versioning logic plus a 219-line test file and a large committed fixture.
• tracked_ruby_files/ruby_file?/first_line in the new script duplicate near-identical logic already in bin/lint. If bin/lint's definition of "tracked Ruby file" changes later, the two copies can silently diverge.
• verify_version! re-checks a RuboCop version that's already pinned via the "_#{version}_" CLI argument convention — appears to be a redundant guarantee.
• The test's hardcoded copy of the 8-cop COPS list can drift from the production constant without any test failure.

Performance

• bin/lint now runs two full RuboCop subprocess scans over the same tracked-file set on every invocation (once for the metrics gate, once for the normal lint pass), roughly doubling RuboCop's cost per run (local and CI). The metrics gate also re-scans the git tree independently instead of reusing the file list bin/lint already computed.

Behavioral note

• The metrics gate runs before the normal lint pass and short-circuits on failure, so a commit that both regresses a tracked metric and has an unrelated style offense will now only show the metrics error in one run, where the old single combined RuboCop invocation surfaced both at once.

None of these are blockers for a draft — flagging them since the PR description says the baseline/CI-integration/tests are still in progress. The double-RuboCop-scan cost and the reuse-vs-.rubocop_todo.yml design question seem worth resolving before this leaves draft, since both affect the shape of the final implementation.

[claude]

Review summary

Solid, well-scoped addition — a deterministic ratchet that prevents new RuboCop Metrics complexity while leaving the existing backlog and global disables untouched. Tests, docs, and the bin/lint integration are thorough. Left 4 inline comments, none blocking:

• bin/lint:43 — check_metrics_baseline has no error handling if bin/rubocop-metrics-baseline is missing/non-executable (unlike check_version!, which rescues Errno::ENOENT with a clear message). Kernel#system returns nil silently in that case, so the failure would be opaque.
• bin/rubocop-metrics-baseline:211 — metric_increases_for compares values by sorted rank position rather than offense identity, so an increase in one method can be masked by an unrelated decrease elsewhere in the same file/cop. Likely an intentional tradeoff per the PR description, but worth double-checking the maintenance doc states the guarantee precisely ("distribution doesn't grow," not "no offense grows").
• bin/rubocop-metrics-baseline:80 — ruby_file?/first_line detection logic is duplicated verbatim from bin/lint. Worth extracting to a shared helper so the two scripts can't drift out of sync on which files count as Ruby.
• bin/lint:27 — the metrics-baseline check re-runs git ls-files and a second RuboCop process independently of bin/lint's own tracked-file scan and RuboCop invocation, doubling subprocess/parse overhead on every bin/lint run. Probably an acceptable tradeoff for keeping the scripts independently runnable — flagging for awareness.

No security concerns; this only shells out to pinned local tooling (rubocop, git) with no untrusted input reaching the shell.

[justin808]

Address-review summary

Scan scope: full review history; no previous address-review checkpoint.

Status: cutoff-safe summary. All 20 inline threads were replied to and resolved; no review item remains pending.

Detailed review outcomes

Mattered

• Fixed the missing/non-executable helper diagnostic in 3406dc4a and covered it with a focused test: Add a RuboCop metrics baseline ratchet #692 (comment) and Add a RuboCop metrics baseline ratchet #692 (comment)
• Fixed the empty tracked-Ruby fallback in 3406dc4a; the helper now writes an empty profile without invoking RuboCop: Add a RuboCop metrics baseline ratchet #692 (comment) and Add a RuboCop metrics baseline ratchet #692 (comment)
• Fixed loss of ordinary lint diagnostics in 3406dc4a; the normal RuboCop pass now runs even when the metrics check fails: Add a RuboCop metrics baseline ratchet #692 (comment)
• Declined replacing the value ratchet with .rubocop_todo.yml: excluded files can still grow unchecked, so that mechanism does not meet issue Refactor oversized Ruby files and re-enable default RuboCop metrics #309 acceptance: Add a RuboCop metrics baseline ratchet #692 (comment) and Add a RuboCop metrics baseline ratchet #692 (comment)
• Declined a single-pass lint redesign for this slice. The second scan deliberately preserves an independently runnable machine-readable check/refresh contract and unchanged streaming output from the ordinary RuboCop pass: Add a RuboCop metrics baseline ratchet #692 (comment), Add a RuboCop metrics baseline ratchet #692 (comment), Add a RuboCop metrics baseline ratchet #692 (comment), and Add a RuboCop metrics baseline ratchet #692 (comment)

Optional

• Added explicit helper-boundary coverage for tracked Ruby inclusion and non-Ruby exclusion: Add a RuboCop metrics baseline ratchet #692 (comment) and Add a RuboCop metrics baseline ratchet #692 (comment)
• Removed file-classifier drift by sharing RubyFileSurface between both commands: Add a RuboCop metrics baseline ratchet #692 (comment), Add a RuboCop metrics baseline ratchet #692 (comment), and Add a RuboCop metrics baseline ratchet #692 (comment)
• Kept JSON metadata version validation because it independently verifies the machine-readable producer; rationale posted with [auto-deferred]: Add a RuboCop metrics baseline ratchet #692 (comment)
• Removed duplicate .rubocop-version reads and the test-only literal cop list: Add a RuboCop metrics baseline ratchet #692 (comment) and Add a RuboCop metrics baseline ratchet #692 (comment)
• Documented the exact per-file/per-cop severity-distribution guarantee and its rank-offset tradeoff: Add a RuboCop metrics baseline ratchet #692 (comment)

Skipped

• CodeRabbit reported that draft PRs are not reviewed automatically; no code action was requested.
• Both Claude review-summary comments were status summaries whose actionable inline items are accounted for above.

Deferred-work tracking: none. The two broader redesign suggestions were explicitly declined for this first baseline slice, not deferred.

Next default scan starts after this comment. Say check all reviews to rescan the full PR.

[claude]

Review summary

Reviewed the RuboCop Metrics ratchet: bin/rubocop-metrics-baseline (check/refresh), the bin/lint integration, the extracted RubyFileSurface module, and the associated tests/docs.

Overall assessment: solid. The core logic is correct and well-tested:

• Exit-code wiring in bin/lint#run (compute the metrics result first, still run the regular linters, fail if either failed) is correct in all three branch combinations.
• The rank-by-rank descending-value comparison in metric_increases_for correctly rejects new offenses, larger values at any rank, and can't be gamed by shuffling offenses around within a file — verified against several synthetic scenarios (new offense, worst-offender regression, compensating rank shifts). The known imprecision (per-file/per-cop rather than per-method identity) is explicitly documented, which is the right call.
• Version and cop-set drift are both guarded (verify_version!, verify_cops!) before comparison, so a stale or mismatched baseline can't silently pass.
• --ignore-disable-comments correctly forces the check to see through inline # rubocop:disable bypasses.
• All subprocess calls use array-form system/Open3.capture3 (no shell interpolation), so no injection concern.
• Good deduplication: extracting RubyFileSurface out of bin/lint removes the previous copy-paste between the two scripts.

Two minor, non-blocking points left as inline comments:

1. Performance (bin/lint): the metrics-baseline check and the ordinary RuboCop pass each scan the full tracked Ruby surface, so RuboCop's contribution to lint runtime roughly doubles on every run. Worth being explicit about, since it's a recurring cost.
2. Docs nit (.rubocop.yml): the new explanatory comment sits above Metrics/AbcSize only but documents the whole 8-cop block below it.

No correctness, security, or test-coverage issues found.

[justin808]

Address-review summary

Scan scope: review activity after 2026-09-02T10:47:05Z.

Status: cutoff-safe summary. The exact-head Claude review found no correctness, security, or coverage issues; both non-blocking threads have explicit outcomes and are resolved.

Detailed review outcomes

Mattered

• None.

Optional

• Performance note: declined at the final-candidate gate because the recurring second Metrics scan is already explicit in the PR performance evidence and preserves the independently runnable check/refresh contract: Add a RuboCop metrics baseline ratchet #692 (comment)
• Metrics-block wording nit: declined at the final-candidate gate because the existing plural comment is already the header immediately above the contiguous eight-cop section: Add a RuboCop metrics baseline ratchet #692 (comment)

Skipped

• The Claude general review summary was status-only; its two inline notes are accounted for above.

Deferred-work tracking: none. No substantive follow-up was identified.

Next default scan starts after this comment. Say check all reviews to rescan the full PR.

[justin808]

Reconciled onto current main and pushed.

• Merge commit: 3afda038a9abcb19311e7bd66559df337859c933
• Main merged: f15211f985e30a85fd846c8ebc37c94bc22b90b7
• Checks: ruby bin/rubocop-metrics-baseline-test.rb, ruby bin/lint-test.rb, bin/rubocop-metrics-baseline check, git diff --check, bin/lint
• Review threads: 0 unresolved
• Hosted checks: queued (claude-review, Lint, validate)
• Worktree: clean

Next owner: GitHub Actions and reviewers.

[claude]

Doc counts don't match the committed fixture. The table says Metrics/AbcSize = 1,509, Metrics/MethodLength = 2,779, and a total of 5,702 offenses, but summing test/fixtures/rubocop-metrics-baseline.json gives 1,510 / 2,780 / 5,704:

jq '[.files[] | .["Metrics/AbcSize"] // [] | length] | add' test/fixtures/rubocop-metrics-baseline.json      # 1510
jq '[.files[] | .["Metrics/MethodLength"] // [] | length] | add' test/fixtures/rubocop-metrics-baseline.json  # 2780
jq '[.files[] | to_entries[] | .value | length] | add' test/fixtures/rubocop-metrics-baseline.json            # 5704

Not release-blocking, but a maintainer using these numbers to gauge remaining backlog or verify a refresh will be off by 1-2 in three places. Worth regenerating the doc numbers from the fixture (or a refresh-time doc-check) rather than hand-typing them.

[claude]

Review summary

Reviewed the RuboCop metrics-baseline ratchet (bin/rubocop-metrics-baseline, bin/lint integration, fixture, docs). Overall this is a solid, well-tested piece of infra:

• The rank-by-rank descending-value comparison in the baseline check correctly rejects new offenses and any regression in existing offense severity, and can't be gamed by shuffling offenses within a file.
• Version drift (verify_version!) and cop-set drift (verify_cops!) are both guarded before comparison.
• --ignore-disable-comments correctly prevents inline # rubocop:disable from being used to dodge the ratchet.
• All subprocess calls use array-form system/Open3.capture3 — no shell injection concern.
• Good dedup: RubyFileSurface removes the prior copy-paste between bin/lint and the new tool.
• Test coverage (bin/lint-test.rb, bin/rubocop-metrics-baseline-test.rb) is thorough.

Left two inline comments, both non-blocking:

1. docs/rubocop-metrics.md: the documented per-cop/total offense counts (1,509 / 2,779 / 5,702) don't match the committed fixture (1,510 / 2,780 / 5,704 per jq summation). Worth deriving the doc numbers from the fixture rather than hand-typing them so they can't drift.
2. bin/lint: the metrics-baseline check is invoked by direct executable path rather than via ruby, unlike every other helper script in the repo (see bin/validate). If the executable bit is ever lost in a checkout, this fails lint entirely rather than falling back to ruby bin/rubocop-metrics-baseline check.

No security or correctness issues found; the two prior review threads (recurring double RuboCop pass, .rubocop.yml comment placement) were already raised and knowingly declined, so not re-raising those.

[justin808]

Current-head triage at 3afda038a9abcb19311e7bd66559df337859c933 reduced the review queue to one real correction: the documentation totals are stale versus the committed fixture (5,702/1,509/2,779 documented; 5,704/1,510/2,780 actual). The executable-bit suggestion was optional portability hardening and is resolved without code churn. The remaining correction is mechanical and requires no product decision; mutation waits only for 692:high-risk-files acknowledgement.