feat: Add file read caching to prevent redundant reads in conversation history

[Mnehmos]

Related GitHub Issue

Closes: #4009

Description

This pull request introduces a file read caching mechanism to prevent re-reading file content that is already present in the conversation history. This improves performance and reduces redundant operations.

The core implementation includes:

• A new fileReadCacheService.ts that tracks file content and modification times (mtime) within the conversation history.
• Logic to check the cache before executing a read_file tool request. If the requested file content is fresh (i.e., mtime has not changed), the tool returns a notice instead of reading the file again.
• Updates to the readFileTool, applyDiffTool, and writeToFileTool to include file metadata (modification time and line ranges) in their XML output, which is used to populate the cache.
• The addition of vitest and related testing dependencies to the package.json files, which was necessary to write tests for the new caching service in accordance with the project's contribution guidelines.

Reviewers should pay close attention to the caching logic in fileReadCacheService.ts and how the mtime is used to invalidate the cache.

Test Procedure

This implementation was validated through a combination of new and updated unit tests:

1. New Tests for Caching Logic: A new test suite, src/core/services/__tests__/fileReadCacheService.spec.ts, was created to cover the caching logic, including cache hits, misses, partial hits, and invalidation based on file modification times.
2. Updated Tool Tests: The existing test suite for the read file tool, src/core/tools/__tests__/readFileTool.test.ts, was significantly updated to mock the caching service and verify that the tool correctly interacts with the cache (e.g., skipping file reads on a cache hit).
3. Running Tests: All tests can be verified by running pnpm test from the root of the repository.

Type of Change

• 🐛 Bug Fix: Non-breaking change that fixes an issue.
• ✨ New Feature: Non-breaking change that adds functionality.
• 💥 Breaking Change: Fix or feature that would cause existing functionality to not work as expected.
• ♻️ Refactor: Code change that neither fixes a bug nor adds a feature.
• 💅 Style: Changes that do not affect the meaning of the code (white-space, formatting, etc.).
• 📚 Documentation: Updates to documentation files.
• ⚙️ Build/CI: Changes to the build process or CI configuration.
• 🧹 Chore: Other changes that don't modify src or test files.

Pre-Submission Checklist

• Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
• Scope: My changes are focused on the linked issue (one major feature/fix per PR).
• Self-Review: I have performed a thorough self-review of my code.
• Code Quality:
  • My code adheres to the project's style guidelines.
  • There are no new linting errors or warnings (npm run lint).
  • All debug code (e.g., console.log) has been removed.
• Testing:
  • New and/or updated tests have been added to cover my changes.
  • All tests pass locally (npm test).
  • The application builds successfully with my changes.
• Branch Hygiene: My branch is up-to-date (rebased) with the main branch.
• Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
• Changeset: A changeset has been created using npm run changeset if this PR includes user-facing changes or dependency updates.
• Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

N/A. This is a non-UI change.

Documentation Updates

The internal developer documentation should be updated to reflect the new file read caching behavior and the importance of the metadata now included in tool responses.

Additional Notes

The updates to package.json and pnpm-lock.yaml are a result of adding vitest as a testing framework for the new service, which is in line with the project's testing standards.

Get in Touch

Mnehmos

[hannesrudolph]

Good implementation of file read caching. The architecture is sound and test coverage is comprehensive. Main concerns are memory management and error handling - see specific comments below.

[hannesrudolph]

Memory management concern: Consider implementing cache size limits and eviction policies to prevent memory issues with large files or long conversations.

[hannesrudolph]

Error handling: Add proper error handling for file system operations when checking mtime. Consider what happens if a file is deleted between cache check and actual read.

[hannesrudolph]

Consider separating the dependency updates (package.json, pnpm-lock.yaml changes) into a separate PR. This makes the feature changes easier to review and track.

[Mnehmos]

Will do so soon, thanks Hannes

[hannesrudolph]

Closing this in favor of the new PR #4501