fix(lib): carry rounded seconds in formatDuration

[Nitjsefnie]

Root cause

formatDuration decided which bucket (seconds / minutes / hours) the value belonged to using the raw millisecond value, and only rounded the remainder after that decision. When the rounded remainder landed exactly on 60, it was emitted as an invalid component:

• 59999 ms → 59.999 s rounded to 60.0 s → "60.0s"
• 119600 ms → bucketed as 1 m, then 59.6 s rounded to 60 s → "1m 60s"
• 3599600 ms → bucketed as 59 m, then 59.6 s rounded to 60 s → "59m 60s"

Fix

Round first, to the precision the final output actually uses, then bucket the rounded total:

Range	Output precision	Rounding
< 60 s	0.1 s	round ms to nearest 100 ms
60 s – 1 h	whole seconds	round ms to nearest 1000 ms
≥ 1 h	whole minutes	round ms to nearest 60 000 ms

This makes carries propagate naturally through every boundary.

Boundary coverage

The new tests cover the reported cases plus the exact edges and one step on each side:

ms	old output	new output
59_949	59.9s	59.9s
59_950	60.0s (bug)	1m 0s
59_999	60.0s (bug)	1m 0s
60_000	1m 0s	1m 0s
119_600	1m 60s (bug)	2m 0s
120_000	2m 0s	2m 0s
3_599_499	59m 59s	59m 59s
3_599_500	59m 60s (bug)	1h 0m
3_599_600	59m 60s (bug)	1h 0m
3_600_000	1h 0m	1h 0m
3_660_000	1h 1m	1h 1m

Existing output for already-correct inputs is preserved byte-for-byte (verified by the untouched existing tests and additional regression pins for 1_499 ms, 312_000 ms, etc.).

Gates

bun run lint && bunx tsc --noEmit && bun run test:run && bun run build

• bun run lint — pass (warnings only, pre-existing)
• bunx tsc --noEmit — pass
• bun run test:run — pass for all related tests; one unrelated pre-existing failure in __tests__/hooks/integrations.test.ts > writeHookEntries adds a packages-array entry to a fresh settings.json (also fails on unmodified main)
• bun run build — pass

Closes#521

Prepared with AI assistance (Kimi K2.7 Code), human-reviewed before submission.

Summary by CodeRabbit

• Bug Fixes
  • Improved duration formatting by applying rounding before bucketing into seconds, minutes, and hours.
  • Fixed rollover behavior when rounding lands exactly on 60 seconds or 60 minutes, preventing invalid outputs.
  • Eliminated display issues such as 60.0s, 1m 60s, and 59m 60s.
  • Added regression and boundary tests to verify correct carry propagation across seconds→minutes→hours (including threshold and near-threshold cases).

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 047de091-fb34-44f0-963d-bac0183e254b

📥 Commits

Reviewing files that changed from the base of the PR and between 6ecd22e and e92ff86.

📒 Files selected for processing (3)

• CHANGELOG.md
• __tests__/lib/format-duration.test.ts
• lib/format-duration.ts

🚧 Files skipped from review as they are similar to previous changes (3)

• tests/lib/format-duration.test.ts
• lib/format-duration.ts
• CHANGELOG.md

---

📝 Walkthrough

Walkthrough

formatDuration now rounds values before selecting seconds, minutes, or hours, allowing carries across unit boundaries. Tests cover rollover thresholds and existing outputs, and the changelog documents the fix.

Changes

Duration normalization

Layer / File(s)	Summary
Rounding-first duration formatting lib/format-duration.ts	formatDuration rounds at the output unit’s precision before bucketing values into seconds, minutes, or hours.
Carry boundary regression coverage __tests__/lib/format-duration.test.ts, CHANGELOG.md	Tests cover carry thresholds and preserved outputs; the changelog records the normalization change.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• FailproofAI/failproofai#530: Updates the same formatter logic and boundary tests.
• FailproofAI/failproofai#552: Addresses the same rounding and carry behavior with regression coverage.

Poem

I’m a bunny with a clock in my paw,
Rounding each tick without a flaw.
Sixty seconds hop to a minute bright,
Sixty minutes leap to an hour’s light.
No crooked durations remain in sight!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description explains the bug, fix, tests, and verification, but it does not follow the required template sections.	Add the template's Description, Type of Change, and Checklist sections, including the required checkboxes.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately summarizes the main fix in formatDuration.
Linked Issues check	✅ Passed	The changes normalize boundary rounding and add tests matching #521's expected outputs.
Out of Scope Changes check	✅ Passed	The PR stays focused on formatDuration, its tests, and the changelog entry, with no clear unrelated code changes.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.

---

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.}

[hermes-exosphere]

Automated code review started - full review. Results will be posted here.

[hermes-exosphere]

✅ Build & test complete. Results:

• TypeScript (tsc --noEmit): pass (0 errors)
• Lint (bun run lint): pass (0 errors, 5 pre-existing warnings unrelated to this PR)
• Tests (bun run test:run): 121 test files, 2074 tests all passing (0 failures)
• Build (bun run build): pass
• formatDuration tests specifically: 21/21 passing (11 original + 10 new boundary/carry tests)

[hermes-exosphere]

🔍 Automated Code Review — PR #529

📋 Executive Summary

This PR fixes a rounding bug in formatDuration where values near second/minute boundaries were displayed with invalid components like "60.0s" or "1m 60s". The fix is elegant: round to the output precision before bucketing, allowing carries to propagate naturally. The change is surgical (2 files, +64/-9 lines), well-tested (10 new boundary tests + 2 regression pins), and verified clean — 2074 tests pass, TypeScript compiles, lint is clean.

---

📊 Change Architecture

graph TD
A["formatDuration(ms)"] -->|"old: bucket first, round after"| B["Bug: 60.0s / 1m 60s"]
A -->|"new: round first, then bucket"| C["Correct carries"]
C --> D["deciseconds bucket<br/>(< 600 → X.Xs)"]
C --> E["seconds bucket<br/>(600-3599 → Xm Ys)"]
C --> F["minutes bucket<br/>(≥ 3600 → Xh Ym)"]
G["formatDuration.test.ts"] -->|"+10 carry/boundary tests<br/>+2 regression pins"| H["21/21 passing"]
style C fill:#90EE90
style B fill:#FF6B6B
style G fill:#87CEEB

Loading

Legend: 🟢 New/Fixed | 🔴 Bug (fixed) | 🔵 Test changes

---

🔴 Breaking Changes

✅ No breaking changes detected. The function signature is unchanged, all existing callers across 12 session files remain backward-compatible, and regression pins confirm existing output for correct inputs is preserved.

---

⚠️ Issues Found

1. 🔵 Info — lib/format-duration.ts:18 — Negative values fall through to return \${ms}ms`` (e.g. -1 → "-1ms"). This is pre-existing behavior preserved by this PR, but semantically a negative duration is impossible and should arguably be clamped to 0ms or throw. Not blocking — outside the scope of this fix.

---

🔬 Logical / Bug Analysis

The bug: The old code computed seconds = ms / 1000, checked if (seconds < 60), then returned seconds.toFixed(1). When seconds = 59.999, .toFixed(1) rounds to 60.0, producing "60.0s" — a mathematically nonsensical output. The same issue affected minute-to-hour carries.

The fix — three-tier rounding-before-bucketing:

1. Sub-minute range (deciseconds = Math.round(ms / 100) < 600): Rounds to nearest 100ms (0.1s precision), so 59.999s → 600 deciseconds → bumps to minute bucket → "1m 0s" ✓
2. Minute range (totalSeconds = Math.round(ms / 1000) < 3600): Rounds to nearest second, so 59.6s → 60s → carries to "1m 0s" ✓
3. Hour+ range (totalMinutes = Math.round(ms / 60000)): Rounds to nearest minute, so 59m 60s → 60m → carries to "1h 0m" ✓

Correctness verification:

• Math.round(59949 / 100) = 599 < 600 → "59.9s" (just below carry) ✓
• Math.round(59950 / 100) = 600 >= 600 → drops to minute bucket → "1m 0s" ✓
• Math.round(3599499 / 1000) = 3599 < 3600 → "59m 59s" (just below hour carry) ✓
• Math.round(3599500 / 1000) = 3600 >= 3600 → drops to hour bucket → "1h 0m" ✓

Caller impact:formatDuration is imported by 12 session modules (codex-sessions, opencode-sessions, copilot-sessions, hermes-sessions, goose-sessions, openclaw-sessions, devin-sessions, cursor-sessions, antigravity-sessions, factory-sessions, log-stats, log-entries) plus a separate private copy in session-hooks-panel.tsx. This change makes all callers correctly display rounded durations — a net improvement with zero risk, as the only changed outputs are the previously-buggy ones.

---

🧪 Evidence — Build & Test Results

Test Results (formatDuration specifically)

 ✓ formatDuration > sub-second: 0ms
✓ formatDuration > sub-second: 42ms
✓ formatDuration > sub-second: 999ms
✓ formatDuration > boundary: exactly 1000ms
✓ formatDuration > seconds: 1500ms
✓ formatDuration > seconds: 3200ms
✓ formatDuration > boundary: exactly 60000ms
✓ formatDuration > minutes: 312000ms
✓ formatDuration > boundary: exactly 3600000ms
✓ formatDuration > hours: 8100000ms
✓ formatDuration > large: 86400000ms (24h)
✓ formatDuration > carries rounded seconds to minutes (59999ms -> 1m 0s)
✓ formatDuration > carries rounded seconds to minutes (119600ms -> 2m 0s)
✓ formatDuration > carries rounded seconds to hours (3599600ms -> 1h 0m)
✓ formatDuration > boundary: just below second-to-minute carry (59949ms)
✓ formatDuration > boundary: second-to-minute carry (59950ms -> 1m 0s)
✓ formatDuration > boundary: just below minute-to-hour carry (3599499ms)
✓ formatDuration > boundary: minute-to-hour carry (3599500ms -> 1h 0m)
✓ formatDuration > boundary: just above hour bucket (3660000ms -> 1h 1m)
✓ formatDuration > regression pin: seconds rounding still formats 1499ms as 1.5s
✓ formatDuration > regression pin: minutes rounding still formats 312000ms as 5m 12s
Test Files 1 passed (1)
Tests 21 passed (21)

Full Test Suite

Test Files 121 passed (121)
Tests 2074 passed (2074)

TypeScript Compilation

bunx tsc --noEmit → exit 0 (no errors)

Build

bun run build → exit 0

---

🔗 Issue Linkage

This PR closes #521 — the bug report for invalid formatted durations at rounding boundaries. The fix directly addresses all cases described in the issue and the test table in the PR body.

---

👥 Human Review Feedback

No human review comments exist on this PR. (CodeRabbit was rate-limited and did not review; hermes-exosphere had a prior review started.)

---

💡 Suggestions

1. 🔵 Minor — app/components/session-hooks-panel.tsx:28: Contains a private formatDuration function that produces "1.0s" → "59.9s" without the carry fix. Consider importing @/lib/format-duration instead to converge on a single implementation and eliminate the private copy.
2. 🔵 Minor — lib/format-duration.ts:18: Consider adding Math.max(0, ms) at the function top to silently clamp negative durations (currently returns -1ms for negative inputs). Outside scope of this PR but worth tracking as a follow-up.

---

🏆 Verdict

VERDICT: APPROVED

---

Automated code review · 2026-07-16 12:36 UTC

[hermes-exosphere]

Automated code review: Approved. Clean fix for #521 — round-before-bucket eliminates invalid formatted durations. 21/21 tests pass, TypeScript clean, build succeeds. No breaking changes.

[hermes-exosphere]

Automated review: Approved. ✅

[hermes-exosphere]

Your PR is awaiting review by a moderator. Till then you can join the Discord for conversation: https://discord.gg/qaFM2uYFb

[hermes-exosphere]

Your PR is awaiting review by a moderator. Till then you can join the Discord for conversation: https://discord.befailproof.ai

[hermes-exosphere]

Your PR is awaiting review by a reviewer. Till then you can join the Discord for conversation: https://discord.befailproof.ai

[chhhee10]

Ran this across a spread from 0 up to 1e15 and it matches on everything realistic. The 59949/59950 boundary tests are the right instinct too — most people just lift the three examples straight out of the issue and call it done.

Only thing missing is a CHANGELOG.md entry. That requirement is buried in CLAUDE.md rather than CONTRIBUTING.md, which is on us rather than you — add a line and this looks good from my side.

[Nitjsefnie]

Appreciate you running the spread to 1e15! CHANGELOG entry added (fbd6475) under a new 0.0.14-beta.1 — 2026-07-17 section per the CLAUDE.md rule.

[Nitjsefnie]

Rebased onto current main — this needed a force-push, so unfortunately your approval got dismissed along with it. Sorry for the re-review tax.

Cause: main picked up its own ## 0.0.14-beta.1 — 2026-07-17 CHANGELOG heading this morning (#563/#564/#567), and this branch had added that same heading independently, so the two collided at the top of the file. Resolved by dropping our duplicate heading and folding the entry into the existing section's ### Fixes list instead — the changelog text itself is unchanged.

No source changes in the rebase. Re-verified on the new head: bun run lint clean, bunx tsc --noEmit clean, bun run test:run green, bun run build green. The regression tests were re-mutation-checked against upstream/main's version of the production file (they fail there, pass on this branch).

[Nitjsefnie]

Rebased onto current main again — apologies for the repeated re-review tax.

main moved once more since the last rebase (it merged #560, #562, #566, #568 and a couple of skill bumps), and the collision was the same shape: the new merges added entries to the top of the ### Fixes list under ## 0.0.14-beta.1 — 2026-07-17, right where this PR's entry sits. Resolved by keeping every one of main's new entries and leaving the formatDuration line directly below them — the changelog wording is unchanged.

No source changes in the rebase; lib/format-duration.ts and the test are byte-identical to the previous head. Re-verified on the new head: lint clean, tsc --noEmit clean, bun run test:run green (2211 tests), build green. The regression tests were re-mutation-checked against upstream/main's copy of the production file — they fail there (rendering the invalid 60.0s / 1m 60s / 59m 60s components) and pass on this branch.

Disclosure: this rebase and verification were done with AI assistance.

[chhhee10]

Approving. Clean fix.

Full gate on top of main is green — tsc, lint, 2243 tests, build — with only the pre-existing #569Pi integration failure you flagged.

Your boundary table covers the reported cases, so I went at it from the other side and brute-forced the function looking for any output with a component ≥ 60 or a shape that doesn't parse: every millisecond from 0–200s, every millisecond across 3500–3700s, a coarse sweep out to 48h, and ±3s around every hour boundary up to 100h. Around 1.5M values, zero invalid outputs. I also checked the opposite failure mode — round-then-bucket could in principle undershoot and emit 0m 30s or 0h 45m — but the thresholds line up so it can't.

The reason I like this over the obvious patch: the tempting fix is if (seconds === 60) { minutes++; seconds = 0 } at each level, which is three special cases that all have to be right, and becomes four the moment someone adds a days bucket. Rounding to the output's own precision first makes the entire class impossible instead of handling its instances. The per-range precision table in the description made that easy to check.

Also happens to be slightly cheaper than what it replaces — at most three Math.round calls instead of division plus toFixed plus floor plus modulo.