diff --git a/lib/context-bill.ts b/lib/context-bill.ts index e13270961..568e0d079 100644 --- a/lib/context-bill.ts +++ b/lib/context-bill.ts @@ -276,6 +276,9 @@ export interface WalkedFile { /** The file's identity, taken without following links when it was walked; --exact reads only that same file. */ dev: number; ino: number; + /** Its size and mtime when it was walked; a measurement is reused only for a file that still has them. */ + size: number; + mtimeMs: number; } /** @@ -364,7 +367,7 @@ export function walkMdFiles(dir: string, opts: { billedRoot?: string; skipNested } if (!st.isFile()) continue; seen.add(real); - out.push({ path: p, real, dev: st.dev, ino: st.ino }); + out.push({ path: p, real, dev: st.dev, ino: st.ino, size: st.size, mtimeMs: st.mtimeMs }); continue; } if (encloses(real, rootReal) || encloses(real, billedReal)) continue; @@ -909,7 +912,7 @@ export function exactSendSet(root: string): ExactSendSet { * walk can be sent. The open does not block, so a file swapped for a FIFO is * refused as not a regular file instead of hanging the run. */ -function readWalkedFile(real: string, dev: number, ino: number, file: string, root: string): string { +function readWalkedFile({ path: file, real, dev, ino, size, mtimeMs }: WalkedFile, root: string): string { const changed = () => new Error(`${file} changed after it was walked; nothing from ${root} was sent. Run again.`); let fd: number; try { @@ -919,13 +922,26 @@ function readWalkedFile(real: string, dev: number, ino: number, file: string, ro } try { const st = fs.fstatSync(fd); - if (!st.isFile() || st.dev !== dev || st.ino !== ino || fs.realpathSync(real) !== real) throw changed(); + if ( + !st.isFile() || + st.dev !== dev || + st.ino !== ino || + st.size !== size || + st.mtimeMs !== mtimeMs || + fs.realpathSync(real) !== real + ) { + throw changed(); + } return fs.readFileSync(fd, "utf8"); } finally { fs.closeSync(fd); } } +function fileIdentity(real: string, { size, mtimeMs }: { size: number; mtimeMs: number }): string { + return `${real}\0${size}\0${mtimeMs}`; +} + export interface ExactMeasurement { tokenSource: string; tokensOf: TokensOf; @@ -972,15 +988,17 @@ export async function measureExactTokens( const texts = new Map(); // The send set holds each real file once, so each is sent once. The bill keys - // the same file by every lexical path it is billed under; lexicalOf maps a - // real file back to the one key that was measured. - const lexicalOf = new Map(); + // the same file by every lexical path it is billed under; measuredAs maps a + // real file, at the size and mtime it was walked with, back to the one key + // that was measured. + const measuredAs = new Map(); const realOf = new Map(); const { files, frontmatter } = sendSet ?? exactSendSet(root); - for (const { path: file, real, dev, ino } of files) { - const text = readWalkedFile(real, dev, ino, file, root); + for (const walked of files) { + const { path: file, real } = walked; + const text = readWalkedFile(walked, root); texts.set(file, text); - lexicalOf.set(real, file); + measuredAs.set(fileIdentity(real, walked), file); realOf.set(file, real); if (!frontmatter.has(real)) continue; const fm = parseFrontmatter(text); @@ -1026,23 +1044,27 @@ export async function measureExactTokens( // A key the send set does not hold (a non-.md foreign-host file, or a file // refused as outside the root and the checkout) falls back to the - // estimate rather than billing zero. Misses are counted, not swallowed: a bill - // that is part-measured and part-estimated must not present itself as measured. + // estimate rather than billing zero. So does a key whose file is no longer + // the one measured (a link retargeted, or the file rewritten, since the walk). + // Misses are counted, not swallowed: a bill that is part-measured and + // part-estimated must not present itself as measured. const missed = new Set(); const fmSuffix = frontmatterKey(""); const measuredKey = (key: string): string | undefined => { const isFm = key.endsWith(fmSuffix); let real: string; + let st: fs.Stats; try { real = realpathStepwise(isFm ? key.slice(0, -fmSuffix.length) : key); + st = fs.lstatSync(real); } catch { return undefined; } - const lexical = lexicalOf.get(real); + const lexical = measuredAs.get(fileIdentity(real, st)); return lexical === undefined ? undefined : isFm ? frontmatterKey(lexical) : lexical; }; const tokensOf: TokensOf = (key, bytes) => { - const alias = counts.has(key) ? key : measuredKey(key); + const alias = measuredKey(key); const exact = alias === undefined ? undefined : counts.get(alias); if (exact !== undefined) return exact; missed.add(key); diff --git a/test/context-bill.test.ts b/test/context-bill.test.ts index 7856175ea..522113229 100644 --- a/test/context-bill.test.ts +++ b/test/context-bill.test.ts @@ -1195,6 +1195,29 @@ describe("--exact (opt-in measurement; offline here via an injected fetch)", () expect(sameFileMoved.err).toContain("changed after it was walked"); }); + it("never reuses a measurement for a file retargeted or rewritten after it was measured", async () => { + if (process.platform === "win32") return; + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "context-bill-stale-")); + const skills = path.join(tmp, "skills"); + fs.mkdirSync(path.join(skills, "qa"), { recursive: true }); + fs.writeFileSync(path.join(tmp, "small.md"), "---\nname: qa\ndescription: d\n---\n# qa\n"); + fs.writeFileSync(path.join(tmp, "big.md"), "---\nname: qa\ndescription: d\n---\n" + "z".repeat(60_000)); + fs.symlinkSync(path.join(tmp, "small.md"), path.join(skills, "qa", "SKILL.md")); + fs.writeFileSync(path.join(skills, "qa", "ref.md"), "r"); + const measured = await measureExactTokens(skills, { model: "m", apiKey: "sk-test", fetchImpl: fakeFetch(), egressHome }); + fs.unlinkSync(path.join(skills, "qa", "SKILL.md")); + fs.symlinkSync(path.join(tmp, "big.md"), path.join(skills, "qa", "SKILL.md")); + fs.writeFileSync(path.join(skills, "qa", "ref.md"), "r".repeat(9_000)); + const bill = buildBill(skills, { tokensOf: measured.tokensOf, tokenSource: measured.tokenSource }); + fs.rmSync(tmp, { recursive: true, force: true }); + const skillMd = path.join(skills, "qa", "SKILL.md"); + const ref = path.join(skills, "qa", "ref.md"); + expect(measured.missedKeys.has(skillMd)).toBe(true); + expect(measured.missedKeys.has(ref)).toBe(true); + expect(bill.skills[0].skillMdTokens).toBeGreaterThan(10_000); + expect(bill.skills[0].totalMdTokens).toBeGreaterThan(12_000); + }); + it("refuses a walked file swapped for a FIFO instead of hanging", () => { if (process.platform === "win32") return; const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "context-bill-fifo-"));