Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions pkg/cli/token_usage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -414,10 +414,13 @@ func TestFormatDurationMs(t *testing.T) {
{1000, "1.0s"},
{1500, "1.5s"},
{6383, "6.4s"},
{59999, "60.0s"},
{60000, "1m0s"},
{90000, "1m30s"},
{125000, "2m5s"},
{59999, "1.0m"},
{60000, "1.0m"},
{90000, "1.5m"},
{119999, "2.0m"},
{125000, "2.1m"},
{3599999, "1.0h"},
{3600000, "1.0h"},
}

for _, tt := range tests {
Expand Down
21 changes: 11 additions & 10 deletions pkg/timeutil/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,28 +41,29 @@ timeutil.FormatDuration(90 * time.Second) // "1.5m"

### `FormatDurationMs(ms int) string`

Formats a duration given in **milliseconds** as a human-readable string.
Formats a duration given in **milliseconds** as a human-readable string. Values that round up to the next unit at a boundary (e.g. `59999` → `60.0s`) roll over into that next unit instead (`59999` → `1.0m`). Values beyond `time.Duration`'s representable nanosecond range are formatted directly in hours to avoid silent overflow.

| Range | Example |
|-------|---------|
| `< 1000ms` | `"500ms"` |
| `1000ms – < 60s` | `"1.5s"` |
| `≥ 60s` | `"1m30s"` |
| `60s – < 1h` | `"1.5m"` |
| `≥ 1h` | `"2.0h"` |

```go
timeutil.FormatDurationMs(500) // "500ms"
timeutil.FormatDurationMs(1500) // "1.5s"
timeutil.FormatDurationMs(90000) // "1m30s"
timeutil.FormatDurationMs(90000) // "1.5m"
```

### `FormatDurationNs(ns int64) string`

Formats a duration given in **nanoseconds** as a human-readable string. Returns `"—"` for zero or negative values. Uses Go's standard `time.Duration.Round(time.Second)` for output.
Formats a duration given in **nanoseconds** as a human-readable string. Returns `"—"` for zero or negative values.

```go
timeutil.FormatDurationNs(0) // "—"
timeutil.FormatDurationNs(2_000_000_000) // "2s"
timeutil.FormatDurationNs(90_000_000_000) // "1m30s"
timeutil.FormatDurationNs(2_000_000_000) // "2.0s"
timeutil.FormatDurationNs(90_000_000_000) // "1.5m"
```

## Usage Examples
Expand All @@ -77,11 +78,11 @@ timeutil.FormatDuration(90 * time.Second) // "1.5m"

// Format a duration given in milliseconds (e.g. from GitHub Actions)
timeutil.FormatDurationMs(1500) // "1.5s"
timeutil.FormatDurationMs(90000) // "1m30s"
timeutil.FormatDurationMs(90000) // "1.5m"

// Format a duration given in nanoseconds (e.g. billing duration)
timeutil.FormatDurationNs(2_000_000_000) // "2s"
timeutil.FormatDurationNs(90_000_000_000) // "1m30s"
timeutil.FormatDurationNs(2_000_000_000) // "2.0s"
timeutil.FormatDurationNs(90_000_000_000) // "1.5m"
```

## Dependencies
Expand All @@ -90,7 +91,7 @@ timeutil.FormatDurationNs(90_000_000_000) // "1m30s"
- None

**External**:
- None beyond the Go standard library (`fmt`, `math`, `time`).
- None beyond the Go standard library (`fmt`, `time`).

## Design Decisions

Expand Down
41 changes: 29 additions & 12 deletions pkg/timeutil/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,19 @@ import (
"time"
)

// maxMsForDuration is the largest millisecond value that can be safely
// converted to a time.Duration (nanoseconds) without overflowing int64.
const maxMsForDuration = math.MaxInt64 / int64(time.Millisecond)

// msPerHour is the number of milliseconds in one hour, used to format
// millisecond values that would overflow time.Duration's nanosecond range.
const msPerHour = float64(time.Hour / time.Millisecond)

// round1 rounds v to one decimal place using standard rounding.
func round1(v float64) float64 {
return math.Round(v*10) / 10
}

// FormatDuration formats a duration for display like the debug npm package.
// It provides granular formatting from nanoseconds to hours.
func FormatDuration(d time.Duration) string {
Expand All @@ -19,35 +32,39 @@ func FormatDuration(d time.Duration) string {
return fmt.Sprintf("%dms", d.Milliseconds())
}
if d < time.Minute {
return fmt.Sprintf("%.1fs", d.Seconds())
if v := round1(d.Seconds()); v < 60 {
return fmt.Sprintf("%.1fs", v)
}
// Rounding pushed the value into the next unit (e.g. 59999ms -> 1.0m).
}
if d < time.Hour {
return fmt.Sprintf("%.1fm", d.Minutes())
if v := round1(d.Minutes()); v < 60 {
return fmt.Sprintf("%.1fm", v)
}
// Rounding pushed the value into the next unit (e.g. 3599999ms -> 1.0h).
}
return fmt.Sprintf("%.1fh", d.Hours())
}

// FormatDurationMs formats a duration given in milliseconds as a human-readable string.
// Examples: 500 -> "500ms", 1500 -> "1.5s", 90000 -> "1m30s"
// Examples: 500 -> "500ms", 1500 -> "1.5s", 90000 -> "1.5m"
// Values that would overflow time.Duration's nanosecond range are formatted
// directly in hours to avoid silent wraparound.
func FormatDurationMs(ms int) string {
if ms < 1000 {
return fmt.Sprintf("%dms", ms)
}
seconds := float64(ms) / 1000.0
if seconds < 60 {
return fmt.Sprintf("%.1fs", seconds)
if int64(ms) > maxMsForDuration {
return fmt.Sprintf("%.1fh", float64(ms)/msPerHour)
}
minutes := int(seconds) / 60
secs := math.Mod(seconds, 60)
return fmt.Sprintf("%dm%.0fs", minutes, secs)
return FormatDuration(time.Duration(ms) * time.Millisecond)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 60cdd64: FormatDurationMs now guards against overflow, formatting values above MaxInt64 / int64(time.Millisecond) directly in hours instead of converting to time.Duration.

}

// FormatDurationNs formats a duration given in nanoseconds as a human-readable string.
// Returns "—" for zero or negative values. Uses Go's standard duration rounding to seconds.
// Returns "—" for zero or negative values.
func FormatDurationNs(ns int64) string {
if ns <= 0 {
return "—"
}
d := time.Duration(ns)
return d.Round(time.Second).String()
return FormatDuration(time.Duration(ns))
}
90 changes: 65 additions & 25 deletions pkg/timeutil/format_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
package timeutil

import (
"math"
"strconv"
"testing"
"time"

Expand Down Expand Up @@ -142,19 +144,19 @@ func TestFormatDuration(t *testing.T) {
expected: "1.0s",
},
{
name: "just under minute",
name: "just under minute rolls over",
duration: 59*time.Second + 999*time.Millisecond,
expected: "60.0s",
expected: "1.0m",
},
{
name: "exactly 1 minute",
duration: 1 * time.Minute,
expected: "1.0m",
},
{
name: "just under hour",
name: "just under hour rolls over",
duration: 59*time.Minute + 59*time.Second,
expected: "60.0m",
expected: "1.0h",
},
{
name: "exactly 1 hour",
Expand Down Expand Up @@ -207,32 +209,32 @@ func TestFormatDurationMs(t *testing.T) {
expected: "1.5s",
},
{
name: "just under one minute rounds up to 60.0s",
name: "just under one minute rolls over to minutes",
ms: 59999,
// 59_999 ms is still in the seconds branch (ms < 60_000), and
// one-decimal formatting rounds 59.999s to "60.0s" before minute formatting applies.
expected: "60.0s",
// 59_999 ms rounds to 60.0s, which rolls over into the minute
// range instead of rendering the "60.0s" artifact.
expected: "1.0m",
},
// Minute range
{
name: "exactly one minute",
ms: 60000,
expected: "1m0s",
expected: "1.0m",
},
{
name: "one minute thirty seconds",
ms: 90000,
expected: "1m30s",
expected: "1.5m",
},
{
name: "multi-minute composition",
ms: 125000,
expected: "2m5s",
expected: "2.1m",
},
{
name: "multi-hour value stays in minutes",
name: "multi-hour value rolls over to hours",
ms: 3_600_000,
expected: "60m0s",
expected: "1.0h",
},
// Negative input is passed through the millisecond branch as-is.
{
Expand All @@ -251,6 +253,44 @@ func TestFormatDurationMs(t *testing.T) {
}
}

// TestFormatDurationMsOverflowGuard verifies that millisecond values beyond
// time.Duration's representable nanosecond range are formatted directly in
// hours instead of silently overflowing. Skipped on platforms where int is
// narrower than 64 bits, since the tested values cannot be represented there.
func TestFormatDurationMsOverflowGuard(t *testing.T) {
if strconv.IntSize < 64 {
t.Skip("requires a 64-bit int to represent the tested values")
}

var beyondRange int64 = 9_223_372_036_855
atBoundary := math.MaxInt64 / int64(time.Millisecond)

tests := []struct {
name string
ms int64
expected string
}{
{
name: "value beyond time.Duration range formats in hours",
ms: beyondRange,
expected: "2562047.8h",
},
{
name: "value at time.Duration boundary uses standard path",
ms: atBoundary,
expected: "2562047.8h",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
result := FormatDurationMs(int(tt.ms))
assert.Equal(t, tt.expected, result, "FormatDurationMs(%d) mismatch", tt.ms)
})
}
}

func TestFormatDurationNs(t *testing.T) {
t.Parallel()
tests := []struct {
Expand All @@ -276,45 +316,45 @@ func TestFormatDurationNs(t *testing.T) {
},
// Rounding boundaries
{
name: "just under half a second rounds down to zero",
name: "just under half a second stays in milliseconds",
ns: 499_999_999,
expected: "0s",
expected: "499ms",
},
{
name: "exactly half a second rounds up",
name: "exactly half a second stays in milliseconds",
ns: 500_000_000,
expected: "1s",
expected: "500ms",
},
{
name: "one and a half seconds rounds up",
name: "one and a half seconds",
ns: 1_500_000_000,
expected: "2s",
expected: "1.5s",
},
{
name: "just under one and a half seconds rounds down",
name: "just under one and a half seconds",
ns: 1_499_999_999,
expected: "1s",
expected: "1.5s",
},
// Composition
{
name: "two seconds",
ns: 2_000_000_000,
expected: "2s",
expected: "2.0s",
},
{
name: "one minute thirty seconds",
ns: 90_000_000_000,
expected: "1m30s",
expected: "1.5m",
},
{
name: "multi-hour duration",
ns: 7_265_000_000_000,
expected: "2h1m5s",
expected: "2.0h",
},
{
name: "multi-hour duration with sub-second rounding",
ns: 3_600_500_000_000,
expected: "1h0m1s",
expected: "1.0h",
},
}

Expand Down
14 changes: 9 additions & 5 deletions pkg/timeutil/spec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,16 @@ func TestSpec_PublicAPI_FormatDurationMs(t *testing.T) {
{name: "sub-second range outputs ms", inputMs: 500, expected: "500ms"},
// From spec range table: 1000ms – < 60s → e.g. "1.5s"
{name: "second range outputs s with one decimal", inputMs: 1500, expected: "1.5s"},
// From spec range table: ≥ 60s → e.g. "1m30s"
{name: "minute-and-seconds range outputs m and s", inputMs: 90000, expected: "1m30s"},
// From spec range table: 60s – < 1h → e.g. "1.5m"
{name: "minute range outputs m with decimal", inputMs: 90000, expected: "1.5m"},
{name: "minute boundary rounds to next minute", inputMs: 119999, expected: "2.0m"},
{name: "just under minute rolls over to next minute", inputMs: 59999, expected: "1.0m"},
{name: "just under hour rolls over to next hour", inputMs: 3599999, expected: "1.0h"},
{name: "hour boundary rolls over to hours", inputMs: 3600000, expected: "1.0h"},
// From spec code examples
{name: "spec example: 500 → 500ms", inputMs: 500, expected: "500ms"},
{name: "spec example: 1500 → 1.5s", inputMs: 1500, expected: "1.5s"},
{name: "spec example: 90000 → 1m30s", inputMs: 90000, expected: "1m30s"},
{name: "spec example: 90000 → 1.5m", inputMs: 90000, expected: "1.5m"},
}

for _, tt := range tests {
Expand All @@ -95,8 +99,8 @@ func TestSpec_PublicAPI_FormatDurationNs(t *testing.T) {
{name: "zero returns em-dash", inputNs: 0, expected: "—"},
{name: "negative returns em-dash", inputNs: -1, expected: "—"},
// From spec code examples
{name: "spec example: 2 billion ns → 2s", inputNs: 2_000_000_000, expected: "2s"},
{name: "spec example: 90 billion ns → 1m30s", inputNs: 90_000_000_000, expected: "1m30s"},
{name: "spec example: 2 billion ns → 2.0s", inputNs: 2_000_000_000, expected: "2.0s"},
{name: "spec example: 90 billion ns → 1.5m", inputNs: 90_000_000_000, expected: "1.5m"},
}

for _, tt := range tests {
Expand Down
Loading