Skip to content

fix(fs): preserve sub-millisecond precision in stat *Ms properties - #28018

Merged
Jarred-Sumner merged 4 commits into
mainfrom
claude/fix-stat-ms-precision
Mar 12, 2026
Merged

Jarred-Sumner merged 4 commits into
mainfrom
claude/fix-stat-ms-precision

Conversation

@robobun

@robobun robobun commented Mar 11, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Fix fs.stat truncating sub-millisecond precision from mtimeMs, atimeMs, ctimeMs, and birthtimeMs properties
  • The non-BigInt path in toTimeMS() was using bun.timespec.ms() which performs integer division, losing the nanosecond remainder
  • Switch to floating-point arithmetic so fractional milliseconds are preserved, matching Node.js behavior

Before

$ bun mtime-test.js
mtimeMs: 1773248895434

After

$ bun mtime-test.js
mtimeMs: 1773248895434.0544

Test plan

  • Added regression test test/regression/issue/28017.test.ts that verifies *Ms properties have fractional components
  • Test fails with system bun (USE_SYSTEM_BUN=1) and passes with debug build
  • Existing stat tests (fs-stats-truncate.test.ts) still pass

Closes #28017
Closes #21943
🤖 Generated with Claude Code

The non-BigInt path in `toTimeMS()` was using integer division via
`bun.timespec.ms()`, which truncated the nanosecond remainder when
converting to milliseconds. This caused `mtimeMs`, `atimeMs`, `ctimeMs`,
and `birthtimeMs` to always be whole numbers, whereas Node.js preserves
fractional milliseconds (e.g. `1773248895434.0544` vs `1773248895434`).

Switch to floating-point arithmetic to preserve sub-millisecond precision.

Closes #28017

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@robobun

robobun commented Mar 11, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 12:01 PM PT - Mar 11th, 2026

❌ Your commit db774fb6 has 2 failures in Build #39311 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 28018

That installs a local version of the PR into your bun-28018 executable, so you can run:

bun-28018 --bun

@coderabbitai

coderabbitai Bot commented Mar 11, 2026 •

Copy link
Copy Markdown
Contributor

Walkthrough

Replaces truncating millisecond computation with a floating-point calculation in Stat.toTimeMS to preserve sub-millisecond precision; adds a regression test that verifies fractional atimeMs and mtimeMs for both synchronous and asynchronous stat results.

Changes

Cohort / File(s) Summary
Stat time conversion & regression test
src/bun.js/node/Stat.zig, test/regression/issue/28017.test.ts
Replaces integer/truncating ms calculation in toTimeMS with f64 computation (sec * 1000 + nsec / 1_000_000) to preserve fractional milliseconds and match Node.js; adds a test asserting non-integer atimeMs/mtimeMs for both sync and async stat.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(fs): preserve sub-millisecond precision in stat *Ms properties' clearly and specifically describes the main change in the PR.
Linked Issues check ✅ Passed The code changes directly address issue #28017 by implementing floating-point arithmetic in toTimeMS to preserve sub-millisecond precision in fs.stat *Ms properties, matching Node.js behavior.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the stated objective: the fix modifies the time-conversion logic and adds a regression test, with no extraneous modifications.
Description check ✅ Passed The PR description comprehensively covers the issue, solution, before/after examples, and test plan with checkboxes.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@test/regression/issue/28017.test.ts`:
- Around line 5-24: Replace the nondeterministic use of import.meta.path with a
deterministic temp file created via harness.tempDir: create a tempDir, write a
test file into it, set known fractional atime/mtime using fs.utimes or
fs.utimesSync (so the timestamps include a fractional millisecond component you
control), then call statSync and await stat on that temp file and assert that
the returned mtimeMs/atimeMs (and other *Ms values if needed) exactly match the
expected fractional values; update the test named "fs.stat *Ms properties
preserve sub-millisecond precision" to remove reliance on import.meta.path and
use the tempDir-created file and controlled utimes instead.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ad912789-03f3-45c5-9c9e-5df1056baa0b

📥 Commits

Reviewing files that changed from the base of the PR and between fbfd069 and 7b502c6.

📒 Files selected for processing (2)
  • src/bun.js/node/Stat.zig
  • test/regression/issue/28017.test.ts

Comment thread test/regression/issue/28017.test.ts Resolved
@github-actions

Copy link
Copy Markdown
Contributor

Found 1 issue this PR may fix:

  1. fs.stat.birthtimeMs returns an integer instead of a float #21943 - fs.stat.birthtimeMs returns an integer instead of a float with sub-millisecond precision

If this is helpful, consider adding 'Fixes #21943' to the PR description to auto-close the issue on merge.

🤖 Generated with Claude Code

Use tempDir and utimesSync to set known fractional timestamps instead of
relying on the filesystem's timestamps of import.meta.path. This makes
the test deterministic and verifiable against exact expected values.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@test/regression/issue/28017.test.ts`:
- Around line 26-29: The test currently uses
expect(s.atimeMs).toBeCloseTo(expectedMs, 0) and
expect(s.mtimeMs).toBeCloseTo(expectedMs, 0) which permits ±0.5ms; tighten this
to assert microsecond-level determinism by changing the matcher to use a higher
precision (e.g. toBeCloseTo(expectedMs, 3)) so s.atimeMs and s.mtimeMs are
compared to expectedMs to within 0.001ms (reference symbols: s.atimeMs,
s.mtimeMs, expectedMs, fractionalTime in the test).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5319039a-c27a-4fa8-a488-32a39aff6bef

📥 Commits

Reviewing files that changed from the base of the PR and between 7b502c6 and 85ce1a5.

📒 Files selected for processing (1)
  • test/regression/issue/28017.test.ts

Comment thread test/regression/issue/28017.test.ts Resolved
Claude Bot and others added 2 commits March 11, 2026 17:41
Change toBeCloseTo from precision 0 (±0.5ms) to precision 3 (±0.001ms)
since utimes preserves the exact value on this filesystem.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use a timestamp with 0.5ms fractional part (500 microseconds) and
toBeCloseTo precision 1 (±0.05ms) instead of precision 3 (±0.001ms).
This ensures the test passes across different filesystem timestamp
resolutions (APFS nanosecond vs utimes microsecond precision).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Jarred-Sumner
Jarred-Sumner merged commit a305e99 into main Mar 12, 2026
60 of 64 checks passed
@Jarred-Sumner
Jarred-Sumner deleted the claude/fix-stat-ms-precision branch March 12, 2026 00:08
structwafel pushed a commit to structwafel/bun that referenced this pull request Apr 25, 2026
…ven-sh#28018)

## Summary
- Fix `fs.stat` truncating sub-millisecond precision from `mtimeMs`,
`atimeMs`, `ctimeMs`, and `birthtimeMs` properties
- The non-BigInt path in `toTimeMS()` was using `bun.timespec.ms()`
which performs integer division, losing the nanosecond remainder
- Switch to floating-point arithmetic so fractional milliseconds are
preserved, matching Node.js behavior

## Before
```
$ bun mtime-test.js
mtimeMs: 1773248895434
```

## After
```
$ bun mtime-test.js
mtimeMs: 1773248895434.0544
```

## Test plan
- [x] Added regression test `test/regression/issue/28017.test.ts` that
verifies `*Ms` properties have fractional components
- [x] Test fails with system bun (`USE_SYSTEM_BUN=1`) and passes with
debug build
- [x] Existing stat tests (`fs-stats-truncate.test.ts`) still pass

Closes oven-sh#28017
Closes oven-sh#21943 
🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Bot <claude-bot@bun.sh>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
xhjkl pushed a commit to xhjkl/bun that referenced this pull request May 14, 2026
…ven-sh#28018)

## Summary
- Fix `fs.stat` truncating sub-millisecond precision from `mtimeMs`,
`atimeMs`, `ctimeMs`, and `birthtimeMs` properties
- The non-BigInt path in `toTimeMS()` was using `bun.timespec.ms()`
which performs integer division, losing the nanosecond remainder
- Switch to floating-point arithmetic so fractional milliseconds are
preserved, matching Node.js behavior

## Before
```
$ bun mtime-test.js
mtimeMs: 1773248895434
```

## After
```
$ bun mtime-test.js
mtimeMs: 1773248895434.0544
```

## Test plan
- [x] Added regression test `test/regression/issue/28017.test.ts` that
verifies `*Ms` properties have fractional components
- [x] Test fails with system bun (`USE_SYSTEM_BUN=1`) and passes with
debug build
- [x] Existing stat tests (`fs-stats-truncate.test.ts`) still pass

Closes oven-sh#28017
Closes oven-sh#21943 
🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Bot <claude-bot@bun.sh>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

2 participants