Skip to content

Conversation

@KJ7LNW
Copy link
Contributor

@KJ7LNW KJ7LNW commented Jul 22, 2025

Context

This PR significantly improves the file history range tracking system to handle complex overlapping scenarios that were previously causing issues with redundant file reads and incorrect range calculations.

Implementation

Enhanced Range Detection:

  • Added computeModifiedRanges() function for precise line-by-line change detection
  • Added adjustHistoricalRanges() for proper range adjustment after content modifications
  • Added getChangedLineRanges() for comprehensive diff analysis

Improved calculateWriteRanges() Logic:

  • Now compares original vs modified content instead of marking entire files as valid
  • Properly preserves historical valid ranges while only invalidating actually modified ranges
  • Handles complex scenarios like range splitting, merging, and line number shifts

Comprehensive Test Coverage:

  • Partial Overlap Tests: Left/right edge overlaps, multiple range overlaps
  • Separation Tests: Insertions between ranges, boundary insertions
  • Complete Coverage Tests: Entire range replacements, extensions beyond ranges
  • Edge Cases: File start/end modifications, empty files, adjacent ranges
  • Complex Scenarios: Multi-range modifications, non-contiguous changes

How to Test

  1. Run the comprehensive test suite:

    cd src && npx vitest core/file-history/__tests__/fileHistoryUtils.spec.ts
  2. Test various file modification scenarios:

    • Modify portions of previously read files
    • Insert content between existing ranges
    • Make overlapping modifications across multiple ranges
    • Verify only modified ranges are invalidated

Benefits

  • Prevents redundant reads: Only re-reads actually modified content ranges
  • Accurate range tracking: Preserves valid historical ranges through complex modifications
  • Robust overlap handling: Correctly handles all types of range intersections and splits
  • Mathematical precision: Range calculations validated through comprehensive test scenarios

Fixes #5871 #1374


Important

Enhances file history range tracking with precise overlap handling and updates tools to utilize these improvements.

  • Behavior:
    • Adds computeModifiedRanges(), adjustHistoricalRanges(), and getChangedLineRanges() in fileHistoryUtils.ts for precise range detection and adjustment.
    • Improves calculateWriteRanges() to compare original vs modified content, preserving valid ranges and handling complex scenarios.
    • Updates tools like writeToFileTool, insertContentTool, and readFileTool to use enhanced range tracking.
  • Tests:
    • Adds comprehensive tests in fileHistoryUtils.spec.ts for partial overlaps, complete coverage, edge cases, and complex scenarios.
  • Misc:
    • Adds FileLineRange and FileMetadata interfaces to apiMessages.ts.
    • Updates Task.ts to track file modification times and manage pending file metadata.

This description was created by Ellipsis for 79fcc89b2986484f1760f08ab14d12b511cc7429. You can customize this summary. It will automatically update as commits are pushed.

@KJ7LNW KJ7LNW requested review from cte, jr and mrubens as code owners July 22, 2025 07:15
@KJ7LNW KJ7LNW moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Jul 22, 2025
@KJ7LNW
Copy link
Contributor Author

KJ7LNW commented Jul 22, 2025

@daniel-lxs this fixes the underlying readfile problem.

you should probably run an eval and make sure there are no surprises

@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. enhancement New feature or request labels Jul 22, 2025
Eric Wheeler added 3 commits July 28, 2025 15:36
- Add file history tracking system to avoid redundant reads
- Optimize readFileTool to only read missing ranges from disk
- Show informative notices when content already available in history
- Integrate history tracking into writeToFile, applyDiff, insertContent tools
- Add comprehensive tests for file history utilities
- Support partial read rejection with detailed error messages

Resolves issue where overlapping read requests would re-read entire ranges
from disk instead of using previously read content from conversation history.

Signed-off-by: Eric Wheeler <[email protected]>
…handling

Improved calculateWriteRanges to properly detect modified content ranges by
comparing original and modified content line-by-line instead of marking
entire files as valid.

- Added computeModifiedRanges function for precise change detection
- Added adjustHistoricalRanges for proper range adjustment after modifications
- Added comprehensive test coverage for all overlap scenarios
- Tests validate range splitting, merging, and shift calculations
- Handles partial overlaps, insertions, deletions, and edge cases

The range tracking now accurately preserves historical valid ranges while
only invalidating ranges that were actually modified.

Fixes #5871 #1374

Signed-off-by: Eric Wheeler <[email protected]>
This fixes an issue where file metadata from read_file operations was not being
properly attached to the API conversation history stream. The pendingFileMetadata
was being set correctly in the Task object but never incorporated into the
conversation stream, causing subsequent reads of the same file to not recognize
previously read ranges.

The fix modifies addToApiConversationHistory to check for and include any
pending file metadata and tool metadata in the message before adding it to
the conversation stream, ensuring proper tracking of file reads across
multiple operations.

Signed-off-by: Eric Wheeler <[email protected]>
@daniel-lxs daniel-lxs force-pushed the file-history-range-optimization branch from 67abb33 to 881fd20 Compare July 28, 2025 20:37
@daniel-lxs
Copy link
Member

I fixed the merge conflicts but it seems like something is wrong with the file detection if the file is changed:

image

I'm guessing it has to do with empty lines in the file remaining unchanged, how do you think we should handle this?

@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Changes Requested] in Roo Code Roadmap Jul 28, 2025
@KJ7LNW
Copy link
Contributor Author

KJ7LNW commented Jul 28, 2025

I'm guessing it has to do with empty lines in the file remaining unchanged, how do you think we should handle this?

if the line did not actually change, then they can be omitted. I have a feeling that the code that scans to see if lines change is somehow misbehaving on lines that are "falsy"

@KJ7LNW KJ7LNW closed this by deleting the head repository Sep 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request PR - Changes Requested size:XXL This PR changes 1000+ lines, ignoring generated files.

Projects

Status: PR [Changes Requested]

Development

Successfully merging this pull request may close these issues.

3 participants