Refactor reporting functions to cut cognitive complexity (S3776, batch 1/4) - #15
Conversation
report_summary.py:_print_top_consumers had cognitive complexity 54 (limit 15). Extract each output section into a focused helper — _print_actions_minutes_top, _print_actions_cost_top, _print_repo_consumer_breakdowns, _print_copilot_by_model, _print_lfs_storage — leaving the parent as a flat sequence of calls. Pure extract-method; output is byte-for-byte unchanged (33 report-summary tests assert exact output). Also gitignore the local .sonar_cloud_token helper file. 732 tests OK, ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- export_csv._write_sections (complexity 44 -> ~2): extract each CSV section into its own _write_*_section helper; parent is now a flat call sequence. - report_products.show_base_costs and render_base_costs (32 each): these were near-duplicate per-product pricing printers. Extract four shared block helpers (_print_actions_compute_costs, _print_actions_storage_costs, _print_copilot_base_costs, _print_lfs_base_costs) that both call, removing ~80 lines of duplication (567 -> 513 lines) and dropping both parents well under the threshold. Pure extract-method; output byte-for-byte unchanged. 732 tests OK, ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- report_data._fetch_sections (20): extract the repeated try/except-RuntimeError->errors[key] pattern into a _try_section helper so each section fetch is a single call. - report_actions.show_actions_os_breakdown (21): extract per-repo OS breakdown printing (_print_repo_os_breakdown) and the totals block (_print_os_totals). - report_account.render_rate_limits (22): split into _print_standard_rate_limits and _print_premium_rate_limits; drop a dead `used` assignment in the standard loop. Pure extract-method; output unchanged. 732 tests OK, typecheck clean, ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe pull request extracts reporting, data-fetching, pricing, summary, and CSV export logic into smaller private helpers. It preserves existing output and error behavior and adds coverage for missing billing item data. ChangesReporting refactor
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
# Conflicts: # CHANGELOG.md
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/github_usage/report_products.py`:
- Line 230: Resolve the Ruff RUF001 warnings in the four output strings near the
product-report formatting prints by replacing the ambiguous “×” character with
ASCII “x” (or adding a targeted RUF001 suppression if the symbol is
intentionally required), while preserving the existing output formatting.
- Around line 506-509: Add unit-test coverage for render_base_costs in the
relevant test module, exercising both billing data containing the "items" key
and the missing-"items" path, while preserving existing show_base_costs coverage
and following the established src/github_usage test patterns.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 68153c7e-3c4f-41cc-900d-c1fafd7c552a
📒 Files selected for processing (8)
CHANGELOG.mdsrc/github_usage/export_csv.pysrc/github_usage/report_account.pysrc/github_usage/report_actions.pysrc/github_usage/report_data.pysrc/github_usage/report_products.pysrc/github_usage/report_summary.pytests/test_report_products.py
| price = item.get("pricePerUnit", 0) | ||
| qty = item.get("grossQuantity", 0) | ||
| net = item.get("netAmount", 0) | ||
| print(f" {sku:<40} {fmt_price(price)}/min × {qty:.1f} min = {fmt_price(net)}") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Resolve the Ruff RUF001 warnings.
Ruff flags the × character in these four output strings as ambiguous. If the character is intentional, add a narrow # noqa: RUF001 suppression. Otherwise, replace it with ASCII x.
Also applies to: 252-252, 274-274, 295-295
🧰 Tools
🪛 Ruff (0.16.1)
[warning] 230-230: String contains ambiguous × (MULTIPLICATION SIGN). Did you mean x (LATIN SMALL LETTER X)?
(RUF001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/github_usage/report_products.py` at line 230, Resolve the Ruff RUF001
warnings in the four output strings near the product-report formatting prints by
replacing the ambiguous “×” character with ASCII “x” (or adding a targeted
RUF001 suppression if the symbol is intentionally required), while preserving
the existing output formatting.
Source: Linters/SAST tools
| _print_actions_compute_costs(actions_sku) | ||
| _print_actions_storage_costs(actions_sku) | ||
| _print_copilot_base_costs(copilot_billing.get("items") if copilot_billing else None) | ||
| _print_lfs_base_costs(lfs_billing.get("items") if lfs_billing else None) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Expect: a test that calls render_base_costs.
rg -n -C 4 '\brender_base_costs\b' tests src/github_usageRepository: kgrizz-git/github-usage
Length of output: 1962
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate tests related to products/show_base_costs/render_base_costs =="
rg -n -C 3 'show_base_costs|render_base_costs|base_costs|missing.*items|items.*missing' tests || true
echo
echo "== report_products outline / relevant source =="
ast-grep outline src/github_usage/report_products.py --match render_base_costs --view expanded || true
sed -n '480,515p' src/github_usage/report_products.py
echo
echo "== find public exports for report_products =="
rg -n 'from \.report_products import|import report_products|show_base_costs|render_base_costs|__all__' src/github_usage tests || trueRepository: kgrizz-git/github-usage
Length of output: 3907
Add coverage for render_base_costs.
render_base_costs is defined in src/github_usage/report_products.py, but the regression test only covers show_base_costs. Add a unit test for render_base_costs, including the missing "items" path, per the coverage guideline for src/github_usage/**/*.py.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/github_usage/report_products.py` around lines 506 - 509, Add unit-test
coverage for render_base_costs in the relevant test module, exercising both
billing data containing the "items" key and the missing-"items" path, while
preserving existing show_base_costs coverage and following the established
src/github_usage test patterns.
Source: Coding guidelines
|
2 participants
Summary
First themed batch of SonarCloud S3776 (cognitive complexity) refactors — the reporting area. All are pure extract-method: behavior and output are unchanged, verified by the existing suite (many reporting tests assert exact printed output byte-for-byte).
report_summary._print_top_consumersexport_csv._write_sections_write_*_sectionhelperreport_products.show_base_costsrender_base_costs(dedup)report_products.render_base_costsreport_account.render_rate_limitsusedvarreport_actions.show_actions_os_breakdownreport_data._fetch_sectionstry/except→errors[key]into_try_sectionEach parent is now a flat sequence of calls, well under the 15 threshold.
Verification
unittest discover)Follow-up batches (not in this PR)
Setup/wizard (8), GUI (2), Core/CLI (8) — separate PRs.
🤖 Generated with Claude Code
Summary by CodeRabbit
Refactor
Bug Fixes
Tests