perf(read): dedup touched-but-identical files and covered sub-ranges (#2259)

The Read dedup stub only fired for the exact same range with an unchanged
mtime. Two common cases re-sent content the model already had:

- mtime moved but bytes did not (git checkout/stash pop, formatters, touch)
- a sub-range of a file that was already read in full

Extract the eligibility check into readDedup.ts. On an mtime mismatch, re-read
the stored range and compare it byte-for-byte before deduping, refreshing the
cached timestamp on a match. Treat an in-bounds range of a prior full Read as
already seen. Edit/Write entries, partial views, out-of-range offsets, and any
stat/read failure still fall through to a normal read.

src/tools/FileReadTool/FileReadTool.ts

@@ -89,6 +89,7 @@ import {
   OFFSET_INSTRUCTION_TARGETED,
   renderPromptTemplate,
 } from './prompt.js'
+import { checkReadDedup } from './readDedup.js'
 import {
   getToolUseSummary,
   renderToolResultMessage,
@@ -632,35 +633,36 @@ export const FileReadTool = buildTool({
     const existingState = dedupKillswitch
       ? undefined
       : readFileState.get(fullFilePath)
-    // Only dedup entries that came from a prior Read (offset is always set
-    // by Read). Edit/Write store offset=undefined — their readFileState
-    // entry reflects post-edit mtime, so deduping against it would wrongly
-    // point the model at the pre-edit Read content.
-    if (
-      existingState &&
-      !existingState.isPartialView &&
-      existingState.offset !== undefined
-    ) {
-      const rangeMatch =
-        existingState.offset === offset && existingState.limit === limit
-      if (rangeMatch) {
-        try {
-          const mtimeMs = await getFileModificationTimeAsync(fullFilePath)
-          if (mtimeMs === existingState.timestamp) {
-            const analyticsExt = getFileExtensionForAnalytics(fullFilePath)
-            logEvent('tengu_file_read_dedup', {
-              ...(analyticsExt !== undefined && { ext: analyticsExt }),
-            })
-            return {
-              data: {
-                type: 'file_unchanged' as const,
-                file: { filePath: file_path },
-              },
-            }
-          }
-        } catch {
-          // stat failed — fall through to full read
-        }
+    // Hits cover the exact same range, any in-bounds range of a prior full
+    // Read, and files whose mtime moved but whose bytes did not (see
+    // readDedup.ts for the eligibility rules).
+    const dedup = await checkReadDedup(existingState, offset, limit, {
+      getModificationTime: () => getFileModificationTimeAsync(fullFilePath),
+      readRange: (rangeOffset, rangeLimit) =>
+        readFileInRange(
+          fullFilePath,
+          rangeOffset === 0 ? 0 : rangeOffset - 1,
+          rangeLimit,
+          rangeLimit === undefined ? maxSizeBytes : undefined,
+          context.abortController.signal,
+        ),
+    })
+    if (dedup.kind !== 'miss' && existingState) {
+      if (dedup.kind === 'touched') {
+        readFileState.set(fullFilePath, {
+          ...existingState,
+          timestamp: dedup.timestamp,
+        })
+      }
+      const analyticsExt = getFileExtensionForAnalytics(fullFilePath)
+      logEvent('tengu_file_read_dedup', {
+        ...(analyticsExt !== undefined && { ext: analyticsExt }),
+      })
+      return {
+        data: {
+          type: 'file_unchanged' as const,
+          file: { filePath: file_path },
+        },
       }
     }
 

src/tools/FileReadTool/readDedup.test.ts

@@ -0,0 +1,113 @@
+import { describe, expect, test } from 'bun:test'
+
+import type { FileState } from '../../utils/fileStateCache.js'
+import { checkReadDedup, type ReadDedupDeps } from './readDedup.js'
+
+const FILE = 'line 1\nline 2\nline 3\nline 4\nline 5\n'
+
+function fullRead(overrides: Partial<FileState> = {}): FileState {
+  return { content: FILE, timestamp: 1000, offset: 1, limit: undefined, ...overrides }
+}
+
+function deps(
+  mtime: number,
+  disk: string = FILE,
+): ReadDedupDeps & { reads: Array<[number, number | undefined]> } {
+  const reads: Array<[number, number | undefined]> = []
+  return {
+    reads,
+    getModificationTime: async () => mtime,
+    readRange: async (offset, limit) => {
+      reads.push([offset, limit])
+      return { content: disk, mtimeMs: mtime + 0.4 }
+    },
+  }
+}
+
+describe('checkReadDedup', () => {
+  test('misses with no prior state', async () => {
+    expect(await checkReadDedup(undefined, 1, undefined, deps(1000))).toEqual({ kind: 'miss' })
+  })
+
+  test('misses for Edit/Write entries (offset undefined)', async () => {
+    const state = fullRead({ offset: undefined })
+    expect(await checkReadDedup(state, 1, undefined, deps(1000))).toEqual({ kind: 'miss' })
+  })
+
+  test('misses for partial-view entries', async () => {
+    const state = fullRead({ isPartialView: true })
+    expect(await checkReadDedup(state, 1, undefined, deps(1000))).toEqual({ kind: 'miss' })
+  })
+
+  test('same range and same mtime is unchanged without re-reading', async () => {
+    const d = deps(1000)
+    expect(await checkReadDedup(fullRead(), 1, undefined, d)).toEqual({ kind: 'unchanged' })
+    expect(d.reads).toEqual([])
+  })
+
+  test('mtime moved but bytes identical is touched with refreshed timestamp', async () => {
+    const d = deps(2000)
+    expect(await checkReadDedup(fullRead(), 1, undefined, d)).toEqual({
+      kind: 'touched',
+      timestamp: 2000,
+    })
+    expect(d.reads).toEqual([[1, undefined]])
+  })
+
+  test('mtime moved and bytes changed is a miss', async () => {
+    const d = deps(2000, FILE + 'line 6\n')
+    expect(await checkReadDedup(fullRead(), 1, undefined, d)).toEqual({ kind: 'miss' })
+  })
+
+  test('touched check re-reads the stored range, not the requested one', async () => {
+    const state = fullRead({ content: 'line 2\nline 3\n', offset: 2, limit: 2 })
+    const d = deps(2000, 'line 2\nline 3\n')
+    expect(await checkReadDedup(state, 2, 2, d)).toEqual({ kind: 'touched', timestamp: 2000 })
+    expect(d.reads).toEqual([[2, 2]])
+  })
+
+  test('sub-range of a prior full read is unchanged', async () => {
+    expect(await checkReadDedup(fullRead(), 2, 2, deps(1000))).toEqual({ kind: 'unchanged' })
+    expect(await checkReadDedup(fullRead(), 5, undefined, deps(1000))).toEqual({ kind: 'unchanged' })
+    expect(await checkReadDedup(fullRead(), 4, 100, deps(1000))).toEqual({ kind: 'unchanged' })
+  })
+
+  test('sub-range of a touched-but-identical full read is touched', async () => {
+    expect(await checkReadDedup(fullRead(), 3, 1, deps(2000))).toEqual({
+      kind: 'touched',
+      timestamp: 2000,
+    })
+  })
+
+  test('offset past the end of a full read misses so the normal warning fires', async () => {
+    expect(await checkReadDedup(fullRead(), 6, undefined, deps(1000))).toEqual({ kind: 'miss' })
+  })
+
+  test('a different range of a partial read misses', async () => {
+    const state = fullRead({ content: 'line 1\nline 2\n', offset: 1, limit: 2 })
+    expect(await checkReadDedup(state, 2, 1, deps(1000))).toEqual({ kind: 'miss' })
+  })
+
+  test('a range read cannot be served by an earlier full read of an empty file', async () => {
+    const state = fullRead({ content: '' })
+    expect(await checkReadDedup(state, 1, 10, deps(1000))).toEqual({ kind: 'miss' })
+  })
+
+  test('stat or read failure falls through to a miss', async () => {
+    const failingStat: ReadDedupDeps = {
+      getModificationTime: async () => {
+        throw new Error('EACCES')
+      },
+      readRange: async () => ({ content: FILE, mtimeMs: 1000 }),
+    }
+    expect(await checkReadDedup(fullRead(), 1, undefined, failingStat)).toEqual({ kind: 'miss' })
+
+    const failingRead: ReadDedupDeps = {
+      getModificationTime: async () => 2000,
+      readRange: async () => {
+        throw new Error('too large')
+      },
+    }
+    expect(await checkReadDedup(fullRead(), 1, undefined, failingRead)).toEqual({ kind: 'miss' })
+  })
+})

src/tools/FileReadTool/readDedup.ts

@@ -0,0 +1,88 @@
+import type { FileState } from '../../utils/fileStateCache.js'
+
+/**
+ * Outcome of the Read dedup check.
+ * - `miss`: send the file content as usual.
+ * - `unchanged`: the model already has this content; return the stub.
+ * - `touched`: mtime moved but the bytes are identical (git checkout, stash
+ *   pop, a formatter rewriting the same output, `touch`). Return the stub and
+ *   refresh the cached timestamp so later checks stay on the fast path.
+ */
+export type ReadDedupResult =
+  | { kind: 'miss' }
+  | { kind: 'unchanged' }
+  | { kind: 'touched'; timestamp: number }
+
+export interface ReadDedupDeps {
+  getModificationTime: () => Promise<number>
+  /** Re-read the given range with the same semantics as the original Read. */
+  readRange: (
+    offset: number,
+    limit: number | undefined,
+  ) => Promise<{ content: string; mtimeMs: number }>
+}
+
+function isFullRead(state: FileState): boolean {
+  return (state.offset === 1 || state.offset === 0) && state.limit === undefined
+}
+
+function countLines(content: string): number {
+  if (content === '') return 0
+  const lines = content.split('\n').length
+  return content.endsWith('\n') ? lines - 1 : lines
+}
+
+/**
+ * Whether the requested range is already in context from a prior full Read.
+ * Requires the start line to exist so an out-of-range offset still reaches
+ * the normal path and gets its "file is shorter than offset" warning.
+ */
+function isCoveredByFullRead(
+  state: FileState,
+  offset: number,
+  limit: number | undefined,
+): boolean {
+  if (!isFullRead(state)) return false
+  if (limit !== undefined && limit <= 0) return false
+  const startLine = offset === 0 ? 1 : offset
+  return startLine >= 1 && startLine <= countLines(state.content)
+}
+
+/**
+ * Decide whether a Read can be answered with the "file unchanged" stub.
+ *
+ * Only entries from a prior Read qualify (offset is always set by Read;
+ * Edit/Write store offset=undefined and reflect post-edit content the model
+ * never saw as a Read result). A hit is either the exact same range, or any
+ * in-bounds range of a file the model already read in full. When mtime has
+ * moved, the stored range is re-read and compared byte-for-byte before
+ * deduping, so a touched-but-identical file doesn't resend its content.
+ * Any failure falls through to a normal read.
+ */
+export async function checkReadDedup(
+  state: FileState | undefined,
+  offset: number,
+  limit: number | undefined,
+  deps: ReadDedupDeps,
+): Promise<ReadDedupResult> {
+  if (!state || state.isPartialView || state.offset === undefined) {
+    return { kind: 'miss' }
+  }
+  const exactRange = state.offset === offset && state.limit === limit
+  if (!exactRange && !isCoveredByFullRead(state, offset, limit)) {
+    return { kind: 'miss' }
+  }
+
+  try {
+    const mtimeMs = await deps.getModificationTime()
+    if (mtimeMs === state.timestamp) return { kind: 'unchanged' }
+
+    const current = await deps.readRange(state.offset, state.limit)
+    if (current.content === state.content) {
+      return { kind: 'touched', timestamp: Math.floor(current.mtimeMs) }
+    }
+  } catch {
+    // stat/read failed — fall through to full read
+  }
+  return { kind: 'miss' }
+}