Show precise (non-rounded) durations in the new trace viewer#1983
Draft
mitul-s wants to merge 3 commits into
Draft
Show precise (non-rounded) durations in the new trace viewer#1983mitul-s wants to merge 3 commits into
mitul-s wants to merge 3 commits into
Conversation
The new trace viewer was using formatDuration() for hover labels, the event list, and the detail pane. formatDuration() rounds to whole seconds for any value >= 1s, so a 1.5s span renders as '2s' — which overstates the duration and is confusing on hover. Add formatDurationPrecise(), which preserves up to two decimals of seconds (and one decimal in minute/second form) without ever rounding up to the next-larger unit. Switch the new trace viewer's bar/segment hover labels, gap delta indicators, event list rows, and span detail pane (duration + offset) to use it.
🦋 Changeset detectedLatest commit: 1c8867e The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
Contributor
🧪 E2E Test Results❌ Some tests failed Summary
❌ Failed Tests🐘 Local Postgres (2 failed)nextjs-webpack-stable-lazy-discovery-disabled (1 failed):
nextjs-webpack-stable-lazy-discovery-enabled (1 failed):
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
❌ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
❌ Some E2E test jobs failed:
Check the workflow run for details. |
Contributor
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Nitro | Express workflow with 1 step💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro workflow with 25 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro workflow with 50 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro Promise.all with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express Promise.all with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.all with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | Nitro Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.race with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 10 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 25 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 50 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 10 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro workflow with 25 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 50 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Stream Benchmarks (includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | Nitro stream pipeline with 5 transform steps (1MB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) 10 parallel streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) fan-out fan-in 10 streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | Nitro SummaryFastest Framework by WorldWinner determined by most benchmark wins
Fastest World by FrameworkWinner determined by most benchmark wins
Column Definitions
Worlds:
|
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
…daries (e.g., "1000ms", "60s", "1m 60s") due to rounding overflow, violating its documented guarantee of never overstating duration. This commit fixes the issue reported at packages/web-shared/src/lib/utils.ts:106 ## Bug Analysis The `formatDurationPrecise` function in `packages/web-shared/src/lib/utils.ts` has three rounding overflow bugs at unit boundaries: 1. **`ms = 999.5`**: The `ms < MS_IN_SECOND` branch uses `Math.round(ms)` which yields `1000`, producing `"1000ms"` — a value that should have been formatted as seconds. The original `formatDuration` function has a guard for this exact case, but `formatDurationPrecise` lacks it. 2. **`ms = 59999`** (59.999s): The `ms < MS_IN_MINUTE` branch computes `s = 59.999` then calls `s.toFixed(2)` which rounds to `"60.00"`. After trimming trailing zeros this becomes `"60s"` — overstating the value and producing an output that belongs in the next unit bracket. 3. **`ms = 119950`** (1m 59.95s): The `ms < MS_IN_HOUR` branch computes `s = 59.95` then calls `s.toFixed(1)` which rounds to `"60.0"`, producing `"1m 60s"` — a nonsensical time representation. All three bugs were confirmed by executing the actual JavaScript logic and observing the incorrect outputs. ## Fix The fix uses truncation (floor) instead of rounding throughout the function, consistent with its documented purpose of "never overstating the underlying duration": 1. Changed `Math.round(ms)` to `Math.floor(ms)` in the sub-second branch. 2. Added a `truncateToFixed(value, decimals)` helper that uses `Math.floor(value * 10^decimals) / 10^decimals` to truncate to a fixed number of decimal places without rounding up. 3. Replaced `s.toFixed(2)` and `s.toFixed(1)` calls with `truncateToFixed(s, 2)` and `truncateToFixed(s, 1)` respectively. After the fix: - `ms=999.5` → `"999ms"` (truncated, not rounded) - `ms=59999` → `"59.99s"` (truncated to 2 decimal places) - `ms=119950` → `"1m 59.9s"` (truncated to 1 decimal place) - All normal cases continue to work correctly. Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com> Co-authored-by: mitul-s <mitulxshah@gmail.com>
Per review feedback, expand the existing formatDuration() with a new
`precise` option rather than introducing formatDurationPrecise() as a
parallel API. The second arg now accepts either an options object
({ compact?, precise? }) or the original boolean shorthand for
backwards compatibility — all existing call sites that pass `true`
keep working unchanged.
The precise branch carries over the truncation semantics from the
upstream boundary-overflow fix (e7c5f12): sub-second values floor to
integer ms, sub-minute and sub-hour values truncate seconds via a
`truncateToFixed` helper so 999.5ms / 59.999s / 119.95s never roll
over into the next-larger unit.
Update the new-trace-viewer call sites to pass { precise: true } and
drop the formatDurationPrecise export. Default and compact behavior is
unchanged; the test file now also covers the legacy compact form and
the three boundary cases identified by the upstream fix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The new trace viewer was formatting span durations with
formatDuration(), which rounds to whole seconds once the value crosses1s. That means a 1.5s span renders as2son hover — overstating the real duration and being confusing when the bar visually shows ~1.5 grid units.This PR adds a new
preciseoption toformatDuration(no separate function — the second arg now accepts either an options object{ compact?, precise? }or the original boolean for backwards compatibility) and switches the new trace viewer to{ precise: true }everywhere a duration / offset is shown to the user as an exact figure:DurationandOffsetfieldsThe precise branch keeps up to two decimals of seconds (e.g.
1.5s,12.34s), one decimal insideXm Y.Zsform, and truncates rather than rounds — so 999.5ms / 59.999s / 1m 59.95s never roll over into the next-larger unit at the boundary. Default and compact (true/{ compact: true }) behavior is unchanged, so all existing call sites — the old trace viewer, the workflow graph execution viewer, the status badge, and the new viewer's own timeline tick / ruler labels — keep working without changes.How did you test your changes?
packages/web-shared/test/format-duration.test.tscovering sub-second / sub-minute / sub-hour / multi-day cases for the precise mode, an explicit "never rounds up" guard, the three boundary cases identified by the upstream fix (999.5ms →999ms, 59 999ms →59.99s, 119 950ms →1m 59.9s), and the unchanged default / compact behavior including the legacy two-argformatDuration(ms, true)form.pnpm --filter @workflow/web-shared run test(46 / 46 pass)pnpm --filter @workflow/web-shared run buildandpnpm --filter @workflow/web run buildboth succeed.PR Checklist - Required to merge
pnpm changesetwas run to create a changelog for this PRgit commit --signoffon your commits)@vercel/workflowin a comment once the PR is ready, and the above checklist is completeSlack Thread