revert: "fix(native): read mtime via BigInt nanoseconds (#1079)" (#1080)

* revert: "fix(native): read mtime via BigInt nanoseconds to match Rust truncation (#1079)"

This reverts commit 3575880.

#1079 paid a real perf cost (BigInt-mode statSync on every file in change
detection) for what is in practice a no-op: at large epoch values, JS
Math.floor(stat.mtimeMs) can read N+1 where Rust wrote N, busting the
fast-skip optimization. But fast-skip only gates whether to re-hash the
file — when it misses, the hash is recomputed from contents and matches
what's stored, so the graph is byte-identical either way. The fix changes
optimization hit rate, not correctness.

The 0.026% miss-rate window the fix targeted does not justify the
constant-cost BigInt overhead applied to 100% of stat calls on the
change-detection hot path. Post-merge benchmark gate confirmed the
regression: no-op rebuild went from +70% (warning) to +108% (fail).

Reopens #1075. The right path is documenting that fast-skip is
best-effort (the on-disk hash is the source of truth for graph
correctness) rather than chasing exact integer parity with Rust.

* refactor(builder): floor mtime inside fileStat helper (#1080)

Encapsulate `Math.floor(stat.mtimeMs)` inside the `fileStat` helper so
every consumer of the integer DB column gets a pre-floored value by
default. Eliminates the risk that a future call site reads
`stat.mtimeMs` and stores it un-floored, which would silently write a
non-integer (or rounded-up integer) into the DB and cause spurious
fast-skip misses on the next build.

All six existing call sites simplified from `Math.floor(stat.mtimeMs)`
to `stat.mtime`. Behaviour unchanged.

Addresses Greptile P2 feedback on the revert PR.

Author: carlos-alm

src/domain/graph/builder/helpers.ts

@@ -222,18 +222,17 @@ export function fileHash(content: string): string {
 }
 
 /**
- * Stat a file, returning integer-truncated mtime in ms (and size).
+ * Stat a file, returning { mtime, size } or null on error.
  *
- * Reads via BigInt nanoseconds and truncates with integer math so the value
- * matches Rust's `Duration::as_millis() as i64` exactly. `Math.floor(stat.mtimeMs)`
- * cannot be substituted: at large epoch values the f64 `mtimeMs` rounds, so a
- * Rust-written `file_hashes.mtime` of N can read back as N+1 in JS and bust the
- * fast-skip path on every native→JS handoff.
+ * `mtime` is `Math.floor(stat.mtimeMs)` so it matches the integer column
+ * stored in the DB. Floor-once-here keeps every consumer honest: storing or
+ * comparing a non-floored `mtimeMs` against the integer DB column would cause
+ * spurious fast-skip misses on the next build.
  */
 export function fileStat(filePath: string): { mtime: number; size: number } | null {
   try {
-    const s = fs.statSync(filePath, { bigint: true });
-    return { mtime: Number(s.mtimeNs / 1_000_000n), size: Number(s.size) };
+    const s = fs.statSync(filePath);
+    return { mtime: Math.floor(s.mtimeMs), size: s.size };
   } catch {
     return null;
   }

src/domain/graph/builder/stages/insert-nodes.ts

@@ -152,7 +152,7 @@ export function buildFileHashes(
 
   // Also include metadata-only updates (self-heal mtime/size without re-parse)
   for (const item of metadataUpdates) {
-    const mtime = item.stat ? Math.floor(item.stat.mtime) : 0;
+    const mtime = item.stat ? item.stat.mtime : 0;
     const size = item.stat ? item.stat.size : 0;
     fileHashes.push({ file: item.relPath, hash: item.hash, mtime, size });
   }
@@ -389,7 +389,7 @@ function updateFileHashes(
 
   // Also update metadata-only entries (self-heal mtime/size without re-parse)
   for (const item of metadataUpdates) {
-    const mtime = item.stat ? Math.floor(item.stat.mtime) : 0;
+    const mtime = item.stat ? item.stat.mtime : 0;
     const size = item.stat ? item.stat.size : 0;
     upsertHash.run(item.relPath, item.hash, mtime, size);
   }

tests/builder/detect-changes.test.ts

@@ -4,10 +4,9 @@
 import fs from 'node:fs';
 import os from 'node:os';
 import path from 'node:path';
-import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest';
+import { afterAll, beforeAll, describe, expect, it } from 'vitest';
 import { closeDb, initSchema, openDb } from '../../src/db/index.js';
 import { PipelineContext } from '../../src/domain/graph/builder/context.js';
-import { fileStat } from '../../src/domain/graph/builder/helpers.js';
 import {
   detectChanges,
   detectNoChanges,
@@ -63,12 +62,12 @@ describe('detectChanges stage', () => {
     const content = fs.readFileSync(path.join(dir, 'a.js'), 'utf-8');
     const { createHash } = await import('node:crypto');
     const hash = createHash('md5').update(content).digest('hex');
-    const stat = fs.statSync(path.join(dir, 'a.js'), { bigint: true });
+    const stat = fs.statSync(path.join(dir, 'a.js'));
     db.prepare('INSERT INTO file_hashes (file, hash, mtime, size) VALUES (?, ?, ?, ?)').run(
       'a.js',
       hash,
-      Number(stat.mtimeNs / 1_000_000n),
-      Number(stat.size),
+      Math.floor(stat.mtimeMs),
+      stat.size,
     );
 
     // Write journal header so journal check doesn't confuse things
@@ -159,16 +158,15 @@ describe('detectNoChanges fast-skip', () => {
     relPath: string,
     filePath: string,
   ): { mtime: number; size: number } {
-    const stat = fs.statSync(filePath, { bigint: true });
-    const mtime = Number(stat.mtimeNs / 1_000_000n);
-    const size = Number(stat.size);
+    const stat = fs.statSync(filePath);
+    const mtime = Math.floor(stat.mtimeMs);
     db.prepare('INSERT INTO file_hashes (file, hash, mtime, size) VALUES (?, ?, ?, ?)').run(
       relPath,
       'deadbeef',
       mtime,
-      size,
+      stat.size,
     );
-    return { mtime, size };
+    return { mtime, size: stat.size };
   }
 
   it('returns false when file_hashes is empty (first build)', () => {
@@ -223,12 +221,12 @@ describe('detectNoChanges fast-skip', () => {
     const db = openDb(path.join(dbDir, 'graph.db'));
     initSchema(db);
     const file = seedFile(dir, 'a.js', 'export const a = 1;');
-    const stat = fs.statSync(file, { bigint: true });
+    const stat = fs.statSync(file);
     db.prepare('INSERT INTO file_hashes (file, hash, mtime, size) VALUES (?, ?, ?, ?)').run(
       'a.js',
       'deadbeef',
-      Number(stat.mtimeNs / 1_000_000n) + 1000, // skewed mtime
-      Number(stat.size),
+      Math.floor(stat.mtimeMs) + 1000, // skewed mtime
+      stat.size,
     );
 
     expect(detectNoChanges(db, [file], dir)).toBe(false);
@@ -278,56 +276,4 @@ describe('detectNoChanges fast-skip', () => {
     closeDb(db);
     fs.rmSync(dir, { recursive: true, force: true });
   });
-
-  // Pins down the BigInt-nanosecond truncation the helper uses to match Rust's
-  // `Duration::as_millis() as i64`. We can't trigger the f64 ULP rounding bug
-  // with a freshly-created file (the failure window is ~256 ns out of every ms,
-  // ~0.026% of values), so instead we stub `fs.statSync` to return a hand-picked
-  // BigInt `mtimeNs` whose f64-mtimeMs path diverges from the BigInt path:
-  //   ns = 1748400000000999808n  (≈ 2025-05-28 epoch ns)
-  //   BigInt:        Number(ns / 1_000_000n)         === 1748400000000
-  //   f64 (broken):  Math.floor(Number(ns) / 1e6)    === 1748400000001
-  // Reverting `fileStat` to `Math.floor(stat.mtimeMs)` would flip the result to
-  // N+1 and fail the assertion deterministically — re-introducing #1075 (the
-  // Rust-written `file_hashes.mtime` of N reading back as N+1 in JS, busting
-  // the fast-skip path on every native→JS handoff).
-  describe('fileStat #1075 mtime truncation', () => {
-    afterEach(() => {
-      vi.restoreAllMocks();
-    });
-
-    it('matches Rust Duration::as_millis() truncation at f64-rounding boundary', () => {
-      // Hand-picked epoch ns where Number(ns)/1e6 rounds up across a ms boundary.
-      const badMtimeNs = 1748400000000999808n;
-      const truncatedMs = 1748400000000;
-      const roundedMs = 1748400000001;
-
-      // Sanity: confirm the chosen value actually triggers the divergence; if a
-      // future Node.js release changes f64 rounding, this baseline assertion
-      // catches it before we trust the spy-based test below.
-      expect(Number(badMtimeNs / 1_000_000n)).toBe(truncatedMs);
-      expect(Math.floor(Number(badMtimeNs) / 1e6)).toBe(roundedMs);
-
-      const stubStats = {
-        mtimeNs: badMtimeNs,
-        mtimeMs: Number(badMtimeNs) / 1e6,
-        size: 42n,
-      } as unknown as fs.BigIntStats;
-      vi.spyOn(fs, 'statSync').mockReturnValue(stubStats);
-
-      // BigInt path must win: N, not N+1.
-      expect(fileStat('/fake/path.js')?.mtime).toBe(truncatedMs);
-    });
-
-    it('returns the BigInt-truncated mtime for a real file on disk', () => {
-      const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-fileStat-trunc-'));
-      const file = seedFile(dir, 'a.js', 'export const a = 1;');
-
-      const big = fs.statSync(file, { bigint: true });
-      const expected = Number(big.mtimeNs / 1_000_000n);
-      expect(fileStat(file)?.mtime).toBe(expected);
-
-      fs.rmSync(dir, { recursive: true, force: true });
-    });
-  });
 });