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

[kantwalamonil]

I reviewed both CONTRIBUTING.md and AGENTS.md before opening this PR.

Summary

• What changed: The Read tool's "File unchanged since last read" stub now also covers two cases that used to resend the whole file:
  1. The mtime moved but the content didn't. Common causes are git checkout or git stash pop back to the same content, a formatter writing identical output, and touch. On an mtime mismatch, the tool re-reads the range stored in readFileState and compares it byte-for-byte with the cached content. If they match, it returns the stub and refreshes the cached timestamp.
  2. A sub-range of a file that was already read in full. For example, offset: 40, limit: 20 after a full Read. The earlier tool_result already has those lines with line numbers, so they are treated as already seen.
• The eligibility check moves out of FileReadTool.call into a small pure helper, src/tools/FileReadTool/readDedup.ts, so it can be unit-tested without building a full ToolUseContext. The stub text, the tengu_file_read_dedup event, and the killswitch are unchanged.
• Why: both cases put a second full copy of the file into context. That costs input and cache-creation tokens on every later turn, even though the model already has the content.

These cases still do a normal read:

• Edit/Write entries (offset === undefined), as before.
• isPartialView entries, as before.
• A start line past the end of a prior full read, so the "file is shorter than offset" warning still fires.
• Notebooks. Their state stores cell JSON, so neither new path matches.
• Any stat or read failure.

Impact

• user-facing impact: none visible. Fewer duplicate file bodies in context after git operations, formatting, or narrowing reads, which lowers token use and cache-creation cost.
• developer/maintainer impact: the dedup eligibility rules now live in one documented helper with direct unit tests (13 cases) instead of inline in call().

Testing

• I ran the required local preflight.
• exact commands and results:
  • bun install --frozen-lockfile: ok
  • bun run check: exit 0. Smoke, any-budget and knip pass. See below for test failures in the log.
  • bun run typecheck: pass
  • bun run typecheck:type-tests: pass (10 files)
  • node bin/openclaude --version and NODE_DISABLE_COMPILE_CACHE=1 node bin/openclaude --version: 0.31.0 (OpenClaude)
  • bun run test:provider: 1703 pass, 1 fail. The failure already exists on main; see below.
  • npm run test:provider-recommendation: 165 pass, 0 fail
  • git fetch https://github.com/Gitlawb/openclaude.git main && bun run security:pr-scan -- --base FETCH_HEAD --head HEAD: "no suspicious additions found"
• focused tests: bun test ./src/tools/FileReadTool/ passes 22 tests, including the new readDedup.test.ts.
• documented skipped checks, platform limitations, or verified pre-existing failures (macOS, Bun 1.3.11, Node 24.11.1, base 53c7ed9c). All of these were reproduced with this change stashed:
  • src/services/api/claude.streamWatchdog.test.ts: "falls back when the top-level stream iterator never settles" fails 3 out of 3 runs both with and without this change.
  • src/memdir/autoExtractFacts.test.ts has 20 failures and src/utils/sessionStorage.atomicReplace.test.ts has 1. The counts are identical with and without this change.
  • Seven more files fail only during the full test:full run and pass on their own both with and without this change, which points to cross-test state leakage: fastMode, context, modelCost.modelGate, sideQuery.attribution, attachments.ultracode, http, knowledgeGraph. None of these touch the Read path.

Notes

• provider/model path tested: n/a. This is a client-side change in the Read tool, with no request-shape or provider differences.
• screenshots attached (if UI changed): n/a, no UI change.
• follow-up work or known limitations: time-based microcompact can replace an old Read result with [Old tool result content cleared] without evicting the matching readFileState entry. A later Read can then return the "unchanged" stub that points at content no longer in context. That gap predates this PR, but because this change returns the stub more often, it makes the gap more likely to show up. I'd suggest a separate fix that clears the readFileState entries for the Read results microcompact clears, and I'm happy to open an issue for it.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Improvements
  • Repeated file reads can now be recognized as unchanged when the requested content matches a previous read, even if the file’s modification time has changed.
  • Reads with different content or ranges continue to return fresh results.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: Twigpine/openclaude/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 12ddb42d-d5f5-4914-a265-d6dc7401e3a6

📥 Commits

Reviewing files that changed from the base of the PR and between 53c7ed9 and 949774e.

📒 Files selected for processing (3)

• src/tools/FileReadTool/FileReadTool.ts
• src/tools/FileReadTool/readDedup.test.ts
• src/tools/FileReadTool/readDedup.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📜 Recent review details

🧰 Additional context used

📓 Path-based instructions (3)

Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive.

⚙️ CodeRabbit configuration file

Files:

• src/tools/FileReadTool/readDedup.test.ts
• src/tools/FileReadTool/FileReadTool.ts
• src/tools/FileReadTool/readDedup.ts

Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions.

⚙️ CodeRabbit configuration file

Files:

• src/tools/FileReadTool/readDedup.test.ts

Apply the OpenClaude maintainer review rubric from AGENTS.md.

⚙️ CodeRabbit configuration file

Files:

• src/tools/FileReadTool/readDedup.test.ts
• src/tools/FileReadTool/FileReadTool.ts
• src/tools/FileReadTool/readDedup.ts

🔇 Additional comments (3)

src/tools/FileReadTool/readDedup.ts (1)

1-88: LGTM!

src/tools/FileReadTool/readDedup.test.ts (1)

1-113: LGTM!

src/tools/FileReadTool/FileReadTool.ts (1)

92-92: LGTM!

Also applies to: 636-665

---

📝 Walkthrough

Walkthrough

The Read tool now uses a dedicated check to compare a requested range with prior read state. The check returns unchanged, touched, or miss. The tool returns file_unchanged for non-miss results when prior state exists.

Changes

File Read Deduplication

Layer / File(s)	Summary
Deduplication eligibility and outcomes src/tools/FileReadTool/readDedup.ts, src/tools/FileReadTool/readDedup.test.ts	The check evaluates eligible prior ranges, modification times, and reread contents. Tests cover unchanged and touched results, misses, range boundaries, and stat or read failures.
Read tool integration src/tools/FileReadTool/FileReadTool.ts	The Read tool calls checkReadDedup. For a non-miss result with existing state, it refreshes the timestamp when touched, logs the event, and returns file_unchanged.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to 94977

No issue identified here prevents merging. When the inspected edge cases cannot safely reuse a prior Read, they fall back to a normal read.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 94977

The optimization preserves existing file-read permissions and does not grant additional access. Remaining uncertainty concerns content freshness and concurrent cache updates, rather than new privileges or demonstrated data exposure.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The demonstrated scope remains the existing model-selected local file and shared conversation cache. The added filesystem operation rereads that cached range for comparison; a deduplication hit returns the existing stub, not newly read bytes.

Trust Boundaries and Controls

• observed — The normal tool entrypoint retains path expansion and read-deny validation before execution. Reuse excludes partial/injected views and post-Edit/Write states, preserving the distinction between content presented as a Read result and other cache producers.

Resilience and Maintainability Implications

• observed — Stat/read exceptions cannot produce a touched refresh and instead fall through to normal reading. The reread receives the existing abort signal. Edit and Write retain their pre-write freshness guards, while full and partial compaction explicitly clear the Read cache; complete overlapping invalidation behavior remains unverified.

Hardening Proposals

• proposed — If cache ownership must remain monotonic across concurrent operations, an entry-identity or generation check could prevent a late timestamp refresh from replacing newer or invalidated state. This is a hardening proposal, not a verified introduced security finding.

🚥 Pre-merge checks | ✅ 6 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (6 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise, scoped, and accurately describes both read deduplication changes in the diff.
Description check	✅ Passed	The description follows the repository template and covers the changes, rationale, impact, testing commands and results, known failures, and limitations.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Risk Surface Disclosed	✅ Passed	PASS. The PR changes only local Read-tool deduplication and its unit tests in src/tools/FileReadTool/. The diff does not change auth, provider routing, permissions, outbound network behavior, startu…
No Hidden Policy Change	✅ Passed	PASS. The PR has an explicit Read-product change, not hidden cleanup: it documents touched-but-identical files and in-bounds sub-ranges, and the diff implements those cases in checkReadDedup. The ex…

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

[kevincodex1]

LGTM, thanks for contributing