Skip to content

Commit 3575880

Browse files
authored
fix(native): read mtime via BigInt nanoseconds to match Rust truncation (#1079)
* fix(native): read mtime via BigInt nanoseconds to match Rust truncation Closes #1075 Rust writes file_hashes.mtime via Duration::as_millis() (truncates), JS read via Math.floor(stat.mtimeMs) (rounds because mtimeMs is f64). At large epoch values the f64 representation rounds up — 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 as Number(s.mtimeNs / 1_000_000n). Integer math truncates identically to Duration::as_millis() so writer and reader land on the same value. Updates all consumers (detect-changes, insert-nodes, pipeline backfill) and the FileStat interface to drop the now-redundant Math.floor wrapper. Test seeds use the same BigInt path so they don't regress on f64 round-up. docs check acknowledged: internal change-detection fix; no language, feature, command, architecture, or roadmap surface affected. * test(builder): make #1075 mtime regression test deterministic Greptile noted that the previous regression test was probabilistic: the f64 ULP rounding bug only triggers on ~0.026% of fresh-file mtimes (the ~256 ns window where Number(mtimeNs)/1e6 rounds across a ms boundary), so a future revert to Math.floor(stat.mtimeMs) would usually pass. Replace with a vi.spyOn stub that injects a hand-picked BigInt mtimeNs known to diverge between the BigInt path (N) and the f64 path (N+1). The test now fails deterministically against the broken implementation — verified locally by reverting fileStat() and watching the assertion flip 1748400000000 → 1748400000001. The fresh-file disk variant is kept alongside as a sanity check that the helper still works against real filesystem stats.
1 parent 09b7569 commit 3575880

5 files changed

Lines changed: 86 additions & 26 deletions

File tree

src/domain/graph/builder/helpers.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -222,12 +222,18 @@ export function fileHash(content: string): string {
222222
}
223223

224224
/**
225-
* Stat a file, returning { mtimeMs, size } or null on error.
225+
* Stat a file, returning integer-truncated mtime in ms (and size).
226+
*
227+
* Reads via BigInt nanoseconds and truncates with integer math so the value
228+
* matches Rust's `Duration::as_millis() as i64` exactly. `Math.floor(stat.mtimeMs)`
229+
* cannot be substituted: at large epoch values the f64 `mtimeMs` rounds, so a
230+
* Rust-written `file_hashes.mtime` of N can read back as N+1 in JS and bust the
231+
* fast-skip path on every native→JS handoff.
226232
*/
227-
export function fileStat(filePath: string): { mtimeMs: number; size: number } | null {
233+
export function fileStat(filePath: string): { mtime: number; size: number } | null {
228234
try {
229-
const s = fs.statSync(filePath);
230-
return { mtimeMs: s.mtimeMs, size: s.size };
235+
const s = fs.statSync(filePath, { bigint: true });
236+
return { mtime: Number(s.mtimeNs / 1_000_000n), size: Number(s.size) };
231237
} catch {
232238
return null;
233239
}

src/domain/graph/builder/pipeline.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -938,7 +938,7 @@ async function backfillNativeDroppedFiles(ctx: PipelineContext): Promise<void> {
938938
}
939939
if (code === null) continue;
940940
const stat = fileStat(absPath);
941-
const mtime = stat ? Math.floor(stat.mtimeMs) : 0;
941+
const mtime = stat ? stat.mtime : 0;
942942
const size = stat ? stat.size : 0;
943943
upsertHash.run(relPath, fileHash(code), mtime, size);
944944
}

src/domain/graph/builder/stages/detect-changes.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ interface FileHashRow {
2727
}
2828

2929
interface FileStat {
30-
mtimeMs: number;
30+
mtime: number;
3131
size: number;
3232
}
3333

@@ -182,7 +182,7 @@ function mtimeAndHashTiers(
182182
if (!stat) continue;
183183
const storedMtime = record.mtime || 0;
184184
const storedSize = record.size || 0;
185-
if (storedSize > 0 && Math.floor(stat.mtimeMs) === storedMtime && stat.size === storedSize) {
185+
if (storedSize > 0 && stat.mtime === storedMtime && stat.size === storedSize) {
186186
skipped.push(relPath);
187187
continue;
188188
}
@@ -596,9 +596,9 @@ export function detectNoChanges(
596596
log(`false: stored size <= 0 for ${relPath} (stored=${record.size})`);
597597
return false;
598598
}
599-
if (Math.floor(stat.mtimeMs) !== storedMtime || stat.size !== storedSize) {
599+
if (stat.mtime !== storedMtime || stat.size !== storedSize) {
600600
log(
601-
`false: mtime/size diff for ${relPath}: stat=${Math.floor(stat.mtimeMs)}/${stat.size} stored=${storedMtime}/${storedSize} (mtimeMs=${stat.mtimeMs})`,
601+
`false: mtime/size diff for ${relPath}: stat=${stat.mtime}/${stat.size} stored=${storedMtime}/${storedSize}`,
602602
);
603603
return false;
604604
}
@@ -663,7 +663,7 @@ export async function detectChanges(ctx: PipelineContext): Promise<void> {
663663
relPath: c.relPath,
664664
content: c.content,
665665
hash: c.hash,
666-
stat: c.stat ? { mtime: Math.floor(c.stat.mtimeMs), size: c.stat.size } : undefined,
666+
stat: c.stat ? { mtime: c.stat.mtime, size: c.stat.size } : undefined,
667667
_reverseDepOnly: c._reverseDepOnly,
668668
}));
669669
ctx.metadataUpdates = increResult.changed
@@ -674,7 +674,7 @@ export async function detectChanges(ctx: PipelineContext): Promise<void> {
674674
.map((c) => ({
675675
relPath: c.relPath,
676676
hash: c.hash,
677-
stat: { mtime: Math.floor(c.stat.mtimeMs), size: c.stat.size },
677+
stat: { mtime: c.stat.mtime, size: c.stat.size },
678678
}));
679679
if (!ctx.isFullBuild && ctx.parseChanges.length === 0 && ctx.removed.length === 0) {
680680
const ranAnalysis = await runPendingAnalysis(ctx);

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ export function buildFileHashes(
128128
size = precomputed.stat.size;
129129
} else {
130130
const rawStat = fileStat(path.join(rootDir, relPath));
131-
mtime = rawStat ? Math.floor(rawStat.mtimeMs) : 0;
131+
mtime = rawStat ? rawStat.mtime : 0;
132132
size = rawStat ? rawStat.size : 0;
133133
}
134134
fileHashes.push({ file: relPath, hash: precomputed.hash, mtime, size });
@@ -143,7 +143,7 @@ export function buildFileHashes(
143143
}
144144
if (code !== null) {
145145
const stat = fileStat(absPath);
146-
const mtime = stat ? Math.floor(stat.mtimeMs) : 0;
146+
const mtime = stat ? stat.mtime : 0;
147147
const size = stat ? stat.size : 0;
148148
fileHashes.push({ file: relPath, hash: fileHash(code), mtime, size });
149149
}
@@ -365,7 +365,7 @@ function updateFileHashes(
365365
size = precomputed.stat.size;
366366
} else {
367367
const rawStat = fileStat(path.join(rootDir, relPath));
368-
mtime = rawStat ? Math.floor(rawStat.mtimeMs) : 0;
368+
mtime = rawStat ? rawStat.mtime : 0;
369369
size = rawStat ? rawStat.size : 0;
370370
}
371371
upsertHash.run(relPath, precomputed.hash, mtime, size);
@@ -380,7 +380,7 @@ function updateFileHashes(
380380
}
381381
if (code !== null) {
382382
const stat = fileStat(absPath);
383-
const mtime = stat ? Math.floor(stat.mtimeMs) : 0;
383+
const mtime = stat ? stat.mtime : 0;
384384
const size = stat ? stat.size : 0;
385385
upsertHash.run(relPath, fileHash(code), mtime, size);
386386
}

tests/builder/detect-changes.test.ts

Lines changed: 65 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,10 @@
44
import fs from 'node:fs';
55
import os from 'node:os';
66
import path from 'node:path';
7-
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
7+
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest';
88
import { closeDb, initSchema, openDb } from '../../src/db/index.js';
99
import { PipelineContext } from '../../src/domain/graph/builder/context.js';
10+
import { fileStat } from '../../src/domain/graph/builder/helpers.js';
1011
import {
1112
detectChanges,
1213
detectNoChanges,
@@ -62,12 +63,12 @@ describe('detectChanges stage', () => {
6263
const content = fs.readFileSync(path.join(dir, 'a.js'), 'utf-8');
6364
const { createHash } = await import('node:crypto');
6465
const hash = createHash('md5').update(content).digest('hex');
65-
const stat = fs.statSync(path.join(dir, 'a.js'));
66+
const stat = fs.statSync(path.join(dir, 'a.js'), { bigint: true });
6667
db.prepare('INSERT INTO file_hashes (file, hash, mtime, size) VALUES (?, ?, ?, ?)').run(
6768
'a.js',
6869
hash,
69-
Math.floor(stat.mtimeMs),
70-
stat.size,
70+
Number(stat.mtimeNs / 1_000_000n),
71+
Number(stat.size),
7172
);
7273

7374
// Write journal header so journal check doesn't confuse things
@@ -158,15 +159,16 @@ describe('detectNoChanges fast-skip', () => {
158159
relPath: string,
159160
filePath: string,
160161
): { mtime: number; size: number } {
161-
const stat = fs.statSync(filePath);
162-
const mtime = Math.floor(stat.mtimeMs);
162+
const stat = fs.statSync(filePath, { bigint: true });
163+
const mtime = Number(stat.mtimeNs / 1_000_000n);
164+
const size = Number(stat.size);
163165
db.prepare('INSERT INTO file_hashes (file, hash, mtime, size) VALUES (?, ?, ?, ?)').run(
164166
relPath,
165167
'deadbeef',
166168
mtime,
167-
stat.size,
169+
size,
168170
);
169-
return { mtime, size: stat.size };
171+
return { mtime, size };
170172
}
171173

172174
it('returns false when file_hashes is empty (first build)', () => {
@@ -221,12 +223,12 @@ describe('detectNoChanges fast-skip', () => {
221223
const db = openDb(path.join(dbDir, 'graph.db'));
222224
initSchema(db);
223225
const file = seedFile(dir, 'a.js', 'export const a = 1;');
224-
const stat = fs.statSync(file);
226+
const stat = fs.statSync(file, { bigint: true });
225227
db.prepare('INSERT INTO file_hashes (file, hash, mtime, size) VALUES (?, ?, ?, ?)').run(
226228
'a.js',
227229
'deadbeef',
228-
Math.floor(stat.mtimeMs) + 1000, // skewed mtime
229-
stat.size,
230+
Number(stat.mtimeNs / 1_000_000n) + 1000, // skewed mtime
231+
Number(stat.size),
230232
);
231233

232234
expect(detectNoChanges(db, [file], dir)).toBe(false);
@@ -276,4 +278,56 @@ describe('detectNoChanges fast-skip', () => {
276278
closeDb(db);
277279
fs.rmSync(dir, { recursive: true, force: true });
278280
});
281+
282+
// Pins down the BigInt-nanosecond truncation the helper uses to match Rust's
283+
// `Duration::as_millis() as i64`. We can't trigger the f64 ULP rounding bug
284+
// with a freshly-created file (the failure window is ~256 ns out of every ms,
285+
// ~0.026% of values), so instead we stub `fs.statSync` to return a hand-picked
286+
// BigInt `mtimeNs` whose f64-mtimeMs path diverges from the BigInt path:
287+
// ns = 1748400000000999808n (≈ 2025-05-28 epoch ns)
288+
// BigInt: Number(ns / 1_000_000n) === 1748400000000
289+
// f64 (broken): Math.floor(Number(ns) / 1e6) === 1748400000001
290+
// Reverting `fileStat` to `Math.floor(stat.mtimeMs)` would flip the result to
291+
// N+1 and fail the assertion deterministically — re-introducing #1075 (the
292+
// Rust-written `file_hashes.mtime` of N reading back as N+1 in JS, busting
293+
// the fast-skip path on every native→JS handoff).
294+
describe('fileStat #1075 mtime truncation', () => {
295+
afterEach(() => {
296+
vi.restoreAllMocks();
297+
});
298+
299+
it('matches Rust Duration::as_millis() truncation at f64-rounding boundary', () => {
300+
// Hand-picked epoch ns where Number(ns)/1e6 rounds up across a ms boundary.
301+
const badMtimeNs = 1748400000000999808n;
302+
const truncatedMs = 1748400000000;
303+
const roundedMs = 1748400000001;
304+
305+
// Sanity: confirm the chosen value actually triggers the divergence; if a
306+
// future Node.js release changes f64 rounding, this baseline assertion
307+
// catches it before we trust the spy-based test below.
308+
expect(Number(badMtimeNs / 1_000_000n)).toBe(truncatedMs);
309+
expect(Math.floor(Number(badMtimeNs) / 1e6)).toBe(roundedMs);
310+
311+
const stubStats = {
312+
mtimeNs: badMtimeNs,
313+
mtimeMs: Number(badMtimeNs) / 1e6,
314+
size: 42n,
315+
} as unknown as fs.BigIntStats;
316+
vi.spyOn(fs, 'statSync').mockReturnValue(stubStats);
317+
318+
// BigInt path must win: N, not N+1.
319+
expect(fileStat('/fake/path.js')?.mtime).toBe(truncatedMs);
320+
});
321+
322+
it('returns the BigInt-truncated mtime for a real file on disk', () => {
323+
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-fileStat-trunc-'));
324+
const file = seedFile(dir, 'a.js', 'export const a = 1;');
325+
326+
const big = fs.statSync(file, { bigint: true });
327+
const expected = Number(big.mtimeNs / 1_000_000n);
328+
expect(fileStat(file)?.mtime).toBe(expected);
329+
330+
fs.rmSync(dir, { recursive: true, force: true });
331+
});
332+
});
279333
});

0 commit comments

Comments
 (0)