fix(native): read mtime via BigInt nanoseconds to match Rust truncation

[carlos-alm]

Closes #1075

Summary

• Rust writes file_hashes.mtime via Duration::as_millis() (truncates) while JS read via Math.floor(stat.mtimeMs) (rounds because mtimeMs is f64). At large epoch values JS reads N+1 for what Rust stored as N — busting the fast-skip path on every native→JS handoff and producing spurious re-parses on no-op and 1-file rebuilds.
• Switch fileStat() to fs.statSync(path, { bigint: true }) and compute mtime = Number(s.mtimeNs / 1_000_000n). Integer math truncates identically to Duration::as_millis() as i64 so writer and reader land on the same value.
• Update all consumers (detect-changes, insert-nodes, pipeline backfill) and the FileStat interface to drop the now-redundant Math.floor wrapper.
• Bring the test seeds onto the same BigInt path so they don't go flaky on the f64 round-up case they're meant to guard against.
• Add a focused regression test pinning fileStat().mtime === Number(stat.mtimeNs / 1_000_000n) so a future revert to Math.floor(stat.mtimeMs) re-introduces the bug visibly.

Why this unblocks the publish gate

Pre-publish benchmark gate failed v3.10.0 with:

[native] No-op rebuild:  13 → 21  (+62%,  threshold 15%)   warning
[native] No-op rebuild:  10 → 17  (+70%,  threshold 15%)   warning
[native] 1-file rebuild: 54 → 132 (+144%, threshold 15%)   FAIL

#1075 explicitly enumerated this as one of two residuals after #1074 unmasked the precision drift; the other residual was the query benchmark regression closed by #1076. The same root cause explains the 1-file rebuild blow-up: fast-skip is also walked over the unchanged neighbors of the edited file, and every one falls out due to the 1ms drift and gets re-parsed.

Notes for reviewers

• Stat sites outside the file_hashes/fast-skip path are intentionally untouched: features/owners.ts, domain/graph/watcher.ts, domain/graph/journal.ts all compare JS-written values to JS-read values, so they have no Rust round-trip and no drift.
• The fast-skip diagnostic line drops (mtimeMs=...) since the float value is no longer consulted by the comparison.

Test plan

• npm run typecheck
• npm run lint
• npx vitest run tests/builder/ — 27 passed
• npx vitest run tests/integration/build.test.ts tests/integration/build-parity.test.ts — 23 passed
• CI pre-publish bench gate run on this branch confirms 1-file rebuild and No-op rebuild deltas back under threshold

Dependency

Local Node 24 verification depended on #1078 (vitest --strip-types injection fix). CI runs on Node 22 so this PR's tests run regardless of merge order.

[claude]

Claude finished @carlos-alm's task —— View job

---

PR Review Analysis

Conducting comprehensive review of PR #1079: fix(native): read mtime via BigInt nanoseconds to match Rust truncation

Todo List:

• Phase 1: Root Cause Analysis - Understand the mtime precision drift problem
• Phase 2: Configuration Change Detection - Check for rule relaxation
• Phase 3: Test Integrity - Verify test changes are justified
• Phase 4: Code Quality - Review implementation details
• Phase 5: Backlog Compliance - Rate against zero-dep/foundation/problem-fit criteria
• Final Assessment - Provide rating and recommendation

Phase 1: Root Cause Analysis ✅

Problem Being Solved: Legitimate precision drift bug where:

• Rust writes file_hashes.mtime via Duration::as_millis() (truncates nanoseconds to milliseconds)
• JS reads via Math.floor(stat.mtimeMs) where mtimeMs is f64 that can round at large epoch values
• At large timestamps, JS reads N+1 for what Rust stored as N, busting fast-skip cache
• This caused 62-144% performance regression on no-op and 1-file rebuilds

Why This Approach: Switching to BigInt nanosecond math (Number(s.mtimeNs / 1_000_000n)) ensures identical truncation behavior to Rust's integer math. This is the correct fix.

Phase 2: Configuration Change Detection ✅

No configuration or rule relaxation detected. This is a pure algorithmic fix with no changes to linting rules, test thresholds, or build configuration.

Phase 3: Test Integrity ✅

Test changes are justified and improve accuracy:

• Updates test seed functions to use the same BigInt truncation path as production code
• Adds a regression test that specifically validates fileStat().mtime === Number(stat.mtimeNs / 1_000_000n)
• No weakened assertions or relaxed thresholds - tests maintain same rigor but use correct precision

Phase 4: Code Quality ✅

Excellent implementation quality:

✅ Consistent Change Pattern: All stat call sites updated identically

• helpers.ts:235: Core fileStat() function updated to use { bigint: true }
• detect-changes.ts:300,310: Healing functions drop Math.floor() wrapper
• insert-nodes.ts:155,392: Metadata update functions use integer mtime values
• pipeline.ts:940: Backfill function updated to match pattern

✅ Proper Documentation: fileStat() function has comprehensive comment explaining the precision requirement and why Math.floor(stat.mtimeMs) cannot be substituted

✅ Error Handling: All fileStat() calls properly handle null returns for missing files

✅ Type Safety: All consumers properly handle the integer mtime values

Root Cause Analysis

Problem Being Solved: Critical performance regression caused by Rust/JS mtime precision mismatch that was blocking the v3.10.0 publish gate with 62-144% slower rebuilds.
Why This Approach: Aligns JS behavior with Rust's integer truncation math to eliminate the precision drift that caused cache misses.
Risk Assessment: Very low - internal change-detection optimization with comprehensive test coverage.

Backlog Compliance

• Zero-dep: ✓ — Uses built-in Node.js BigInt functionality, no new dependencies
• Foundation-aligned: ✓ — Pure performance optimization aligned with P4 (zero-cost core)
• Problem-fit: 5 — Directly addresses AI coding assistant efficiency by eliminating spurious 62-144% rebuild slowdowns that waste compute cycles
• Breaking: No — Internal change-detection logic, no user-facing API changes
• Tier: 1

Critical Concerns

None. This is a well-implemented precision fix that addresses a real performance regression.

Final Recommendation

• Rating: ⭐⭐⭐⭐⭐ (5/5)
• Action: APPROVE
• Reasoning: Exceptional PR that fixes a critical performance regression with surgical precision, comprehensive documentation, proper test coverage, and zero risk to user-facing functionality. The BigInt truncation approach correctly matches Rust behavior and the implementation is consistent across all affected code paths.

---

[greptile-apps]

Greptile Summary

Fixes the Rust↔JS mtime precision mismatch that caused the fast-skip path to miss on every native→JS handoff: fileStat() now reads nanoseconds via { bigint: true } and truncates with integer BigInt division (Number(s.mtimeNs / 1_000_000n)), exactly matching Rust's Duration::as_millis() truncation that Math.floor(stat.mtimeMs) could not reproduce due to f64 rounding at large epoch values.

• helpers.ts — fileStat switched to BigInt stat; return type renamed from mtimeMs to mtime (integer-truncated ms).
• detect-changes.ts, insert-nodes.ts, pipeline.ts — all direct callers updated to drop the now-redundant Math.floor wrapper and use the new stat.mtime field.
• Tests — seed data migrated to the BigInt path; a new deterministic regression test uses vi.spyOn with a hand-picked mtimeNs that diverges on the f64 path, guaranteeing a visible failure if the implementation reverts.

Confidence Score: 5/5

Safe to merge — the fix is narrow, well-reasoned, and correctly propagated across all callers.

The BigInt truncation path is mathematically equivalent to Rust's Duration::as_millis(), the result stays well within Number.MAX_SAFE_INTEGER (~1.748 × 10¹² at today's epoch vs the ~9 × 10¹⁵ safe limit), and the deterministic spy-based test will catch any future revert. No data-loss or logic errors are introduced; remaining Math.floor wrappers in the unchanged metadataUpdates loop are no-ops on already-integer values.

No files require special attention.

Important Files Changed

Filename	Overview
src/domain/graph/builder/helpers.ts	Core fix: fileStat now uses { bigint: true } and computes mtime = Number(s.mtimeNs / 1_000_000n) for exact integer truncation matching Rust's Duration::as_millis(); return type renamed from mtimeMs to mtime.
src/domain/graph/builder/stages/detect-changes.ts	All direct Math.floor(c.stat.mtimeMs) call sites updated to c.stat.mtime; local FileStat interface updated from mtimeMs to mtime. The healMetadata path still wraps with Math.floor(item.stat.mtime) but that is pre-existing code and now harmless since the value is already a truncated integer.
src/domain/graph/builder/stages/insert-nodes.ts	Six Math.floor(rawStat.mtimeMs) / Math.floor(stat.mtimeMs) call sites updated to stat.mtime; the metadataUpdates loop still has Math.floor(item.stat.mtime) (pre-existing, now a no-op since mtime is already a truncated integer).
src/domain/graph/builder/pipeline.ts	Single backfillNativeDroppedFiles call site updated from Math.floor(stat.mtimeMs) to stat.mtime; clean and correct.
tests/builder/detect-changes.test.ts	Test seeds migrated to the BigInt path; new deterministic regression test uses vi.spyOn with a hand-picked mtimeNs that exercises the exact f64 rounding boundary, replacing the previous probabilistic approach flagged in review.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["fs.statSync(path, { bigint: true })"] --> B["s.mtimeNs: BigInt (nanoseconds)"]
    B --> C["s.mtimeNs / 1_000_000n\n(integer BigInt division — truncates)"]
    C --> D["Number(result) → integer ms\nMatches Rust Duration::as_millis()"]

    subgraph OLD ["Old path (broken)"]
        E["fs.statSync(path)"] --> F["s.mtimeMs: f64 (float ms)"]
        F --> G["Math.floor(s.mtimeMs)\nf64 ULP ~256 ns → rounds up at boundary"]
        G --> H["N+1 instead of N\n→ fast-skip cache miss on every native→JS handoff"]
    end

    D --> I["fileStat() returns { mtime, size }"]
    I --> J["detect-changes: mtime comparison\n stat.mtime === storedMtime"]
    I --> K["insert-nodes: buildFileHashes / updateFileHashes"]
    I --> L["pipeline: backfillNativeDroppedFiles"]

Loading

_{Reviews (3): Last reviewed commit: "test(builder): make #1075 mtime regressi..." | Re-trigger Greptile}

[greptile-apps]

Regression test only catches the bug probabilistically

The test comment says "Reverting to Math.floor(stat.mtimeMs) re-introduces the bug visibly," but that's only true when the freshly-created file's mtimeNs happens to land near a millisecond boundary where f64 rounding flips. At the current epoch (~1.748 × 10¹⁸ ns), the f64 ULP is ~256 ns, so only values with a sub-ms remainder in the ~[999744 ns, 1000000 ns) window trigger the discrepancy — roughly a 0.026% chance per run. In the common case the test passes even with Math.floor.

A deterministic version would use fs.utimesSync to stamp a known-bad mtime (one where Math.floor(Number(badNs) / 1e6) !== Number(badNs / 1_000_000n)) and assert the two values diverge, then confirm fileStat returns the BigInt-truncated value. As written the test is still useful as a sanity check, but the regression-guard claim in the comment overstates it.

[carlos-alm]

Fixed in 1a44aa4 — replaced the probabilistic test with a deterministic vi.spyOn(fs, 'statSync') stub that injects a hand-picked mtimeNs (1748400000000999808n) where the BigInt path lands on N and the f64 path lands on N+1. Verified locally by reverting fileStat() to Math.floor(stat.mtimeMs) — the test now fails deterministically (1748400000000 vs 1748400000001) instead of mostly passing. Kept the original fresh-file-on-disk variant alongside as a sanity check that the helper still works against real filesystem stats.

[github-actions]

Codegraph Impact Analysis

8 functions changed → 17 callers affected across 7 files

• fileStat in src/domain/graph/builder/helpers.ts:233 (15 transitive callers)
• backfillNativeDroppedFiles in src/domain/graph/builder/pipeline.ts:774 (4 transitive callers)
• FileStat.mtime in src/domain/graph/builder/stages/detect-changes.ts:30 (0 transitive callers)
• mtimeAndHashTiers in src/domain/graph/builder/stages/detect-changes.ts:165 (3 transitive callers)
• detectNoChanges in src/domain/graph/builder/stages/detect-changes.ts:537 (6 transitive callers)
• detectChanges in src/domain/graph/builder/stages/detect-changes.ts:641 (5 transitive callers)
• buildFileHashes in src/domain/graph/builder/stages/insert-nodes.ts:105 (3 transitive callers)
• updateFileHashes in src/domain/graph/builder/stages/insert-nodes.ts:338 (3 transitive callers)

[carlos-alm]

@greptileai