Skip to content

Consolidate duplicate formatNumber() implementations#2690

Merged
pelikhan merged 3 commits intomainfrom
copilot/consolidate-format-number-function
Oct 28, 2025
Merged

Consolidate duplicate formatNumber() implementations#2690
pelikhan merged 3 commits intomainfrom
copilot/consolidate-format-number-function

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Oct 28, 2025

Two implementations of number formatting logic existed with 90% code overlap:

  • pkg/cli/logs.go:formatNumber() (41 lines, sophisticated precision thresholds)
  • pkg/console/render.go:formatNumberForDisplay() (25 lines, simpler logic)

Changes

  • Consolidated into console.FormatNumber(): Replaced simpler implementation with the sophisticated version featuring adaptive precision (2 decimals <10k, 1 decimal 10k-99k, 0 decimals 100k+)
  • Removed duplicate from logs.go: Deleted 41-line formatNumber() function
  • Updated call sites: Migrated 7 references across logs.go, audit.go, and audit_report.go to use console.FormatNumber()
  • Updated test expectations: Adjusted existing tests to match new precision behavior

Example

// Before: Two separate implementations
// pkg/cli/logs.go
func formatNumber(n int) string { /* 41 lines */ }

// pkg/console/render.go  
func formatNumberForDisplay(n int) string { /* 25 lines */ }

// After: Single shared implementation
console.FormatNumber(1234)    // "1.23k" (2 decimals)
console.FormatNumber(12345)   // "12.3k" (1 decimal)
console.FormatNumber(123456)  // "123k"  (0 decimals)

Removes 45 lines of duplicate code while providing consistent, higher-quality number formatting across the codebase.

Original prompt

This section details on the original issue you should resolve

<issue_title>[task] Consolidate formatNumber() duplicate implementations</issue_title>
<issue_description>## Objective
Consolidate two similar number formatting functions into a single shared implementation.

Context

Related to #2668 (Duplicate or Near-Duplicate Functions)

Currently there are two implementations with 90% code overlap:

  • pkg/cli/logs.go:1843-1883 (41 lines) - More sophisticated precision handling
  • pkg/console/render.go:542-566 (25 lines) - Simpler thresholds

Both format integers as human-readable strings (K/M/B suffixes) but use different precision logic.

Approach

  1. Consolidate into pkg/console/render.go as FormatNumber(n int) string
  2. Use the more sophisticated version from logs.go (better precision thresholds)
  3. Update logs.go to import from the console package
  4. Remove duplicate implementation from logs.go
  5. Ensure consistent number formatting across the entire application

Files to Modify

  • Primary: pkg/console/render.go
    • Replace existing formatNumberForDisplay() (lines 542-566) with enhanced version from logs.go
    • Export as FormatNumber()
  • Update: pkg/cli/logs.go
    • Remove formatNumber() function (lines 1843-1883)
    • Import and use console.FormatNumber()
    • Update all call sites
  • Update: Test files for both packages

Acceptance Criteria

  • Single FormatNumber() function exists in pkg/console/render.go
  • Function uses the sophisticated precision handling from logs.go
  • Function correctly formats: 0, <1000, 1K-999K, 1M-999M, 1B+
  • logs.go imports and uses console.FormatNumber()
  • All existing tests pass
  • Unit tests verify formatting for edge cases (0, 999, 1000, 1500, 999999, 1000000, etc.)
  • 25-40 lines of duplicate code removed

Estimated Effort

1 hour

Priority

P1 - High Impact (Quick Win)
Related to #2668

AI generated by Plan Command for #2668</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Consolidate duplicate formatNumber implementations Consolidate duplicate formatNumber() implementations Oct 28, 2025
Copilot AI requested a review from pelikhan October 28, 2025 13:49
@pelikhan pelikhan marked this pull request as ready for review October 28, 2025 13:52
Copilot AI review requested due to automatic review settings October 28, 2025 13:52
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR consolidates duplicate number formatting logic by replacing a simpler implementation with a more sophisticated one featuring adaptive precision based on magnitude (2 decimals for values under 10k/10M/10B, 1 decimal for 10-99k/10-99M/10-99B, and 0 decimals for 100k+/100M+/100B+).

Key changes:

  • Enhanced FormatNumber() in pkg/console/render.go with better precision thresholds from the logs.go implementation
  • Removed 45-line duplicate formatNumber() function from pkg/cli/logs.go
  • Updated 7 call sites across logs.go, audit.go, and audit_report.go to use the consolidated function
  • Adjusted test expectations to match the new precision behavior (e.g., "1.5k" → "1.50k")

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
pkg/console/render.go Enhanced FormatNumber() with adaptive precision logic for K/M/B formatting
pkg/cli/logs.go Removed duplicate formatNumber() and updated call sites to use console.FormatNumber()
pkg/cli/audit.go Updated 3 call sites to use console.FormatNumber()
pkg/cli/audit_report.go Updated 2 call sites to use console.FormatNumber()
pkg/console/render_test.go Updated expected values and added comprehensive test coverage for FormatNumber()
pkg/console/render_formatting_test.go Updated expected values to match new precision behavior
pkg/cli/logs_test.go Updated test to use console.FormatNumber()

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions
Copy link
Copy Markdown
Contributor

Agentic Changeset Generator triggered by this pull request.

@pelikhan pelikhan merged commit 48becf8 into main Oct 28, 2025
3 checks passed
@pelikhan pelikhan deleted the copilot/consolidate-format-number-function branch October 28, 2025 13:56
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.

[task] Consolidate formatNumber() duplicate implementations

3 participants