feat(read_file): enhance file reading capabilities with multi-file support and improved parameter handling

[samhvw8]

Context

Implementation

Screenshots

before	after

How to Test

Get in Touch

---

Important

Enhances read_file tool with multi-file support, adds maxConcurrentFileReads setting, and updates UI components and tests accordingly.

• Behavior:
  • read_file tool now supports reading multiple files with line ranges, improving efficiency.
  • Introduces maxConcurrentFileReads setting in global-settings.ts to limit concurrent file reads.
• UI Components:
  • Adds BatchFilePermission component for handling multi-file read permissions in ChatView.
  • Updates ContextManagementSettings and SettingsView to include maxConcurrentFileReads slider.
• Tests:
  • Adds tests for BatchFilePermission in BatchFilePermission.test.tsx.
  • Updates existing tests to handle new multi-file read logic in readFileTool.test.ts.
• Localization:
  • Updates settings.json for English and Japanese locales to include new settings descriptions.

^{This description was created by for 62330ebfae61e7c2c1c4d669ab5cef93e46a00d0. You can customize this summary. It will automatically update as commits are pushed.}

[changeset-bot]

⚠️ No Changeset found

Latest commit: 940c543

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[ellipsis-dev]

The changes in this pull request seem to be related to enhancing the file reading capabilities and updating the tool usage format. The changes across different files appear to be interconnected, focusing on a single feature enhancement. Therefore, it doesn't seem necessary to split the pull request into smaller ones. If you have any questions or need further clarification, feel free to reach out!

[ellipsis-dev]

Range read is handled only when both start_line and end_line are provided. Confirm if this is the intended behavior. If a user provides only one of these parameters (e.g., just start_line), consider whether it should read from start_line to the end of the file.

[KJ7LNW]

• please convert this PR to a draft so it does not get merged accidentally
• @cte this will need run through an eval before merging, but I think it has great promise to reduce token usage (ie, the n^2/2 for sequential file reads), and increase focus because all the information is provided through a single request.

[samhvw8]

@KJ7LNW converted, @mrubens @cte we need an threshold of maximum file reading, should we based on model information for threshold ?

[KJ7LNW]

@KJ7LNW converted

Excellent please force push so I can look when you get a chance.

we need an threshold of maximum file reading, should we based on model information for threshold ?

I think first pass should respect existing line limit handling per file to keep the implementation simple . After we have been using it for a while we may discover different ways to handle length issues.

Please make sure that "list code definitions" triggers when the length of the file is less than the maximum line limit as it does now

[samhvw8]

@KJ7LNW i just update the test, now it all pass

[KJ7LNW]

<args>
:path:src/app.ts
:start_line:1
:end_line:50
======+++======
:path:src/utils.ts
:start_line:100
:end_line:150
======+++======
:path:package.json
</args>

It looks like you have custom argument formats which do not allow us to programmatically change the parser or automate tool XML structure integration as part of the tool refactor that @bramburn is working on in #2467 . Ultimately all tools will provide standardized argument structure so that no tool has special parsing for trivial data types (int, string, etc). The long term goal is to automatically generate system instructions, parsing, and tool documentation based on per-tool JSON-looking XML schema definitions standardized in a static attribute for each tool class.

Please choose a standard XML format, so we can standardize all tool arguments as part of the Tool base class in the future and avoid much fine your code to conform. Something like this would be appropriate:

<read_file>
<file path="/path1">
<range lines="1-100"/>
<range lines="210-22"/>
</file>
<file path="/path2">
<range lines="11-111"/>
<range lines="222-333"/>
</file>
</read_file>

or if you do not like attributes:

<read_file>
<file>
<path>/path/to/file1</path>
<line_range>111-222</line_range>
<line_range>333-444</line_range>
</file>
<file>
<path>/path/to/file2</path>
<line_range>1111-2222</line_range>
<line_range>3333-4444</line_range>
</file>
</read_file>

[samhvw8]

@KJ7LNW if we have format like that another llm can do it ?
I will make another branch for xml style

[KJ7LNW]

if we have format like that another llm can do it ?

LLMs work quite well with XML at the moment, I am not sure if I understood your question, can you elaborate?

[samhvw8]

@KJ7LNW i make it work with all xml, fixing test then push to branch 💪🏻

[KJ7LNW]

💪🏻!!!

I'll check it out

[bramburn]

🏋️‍♀️

…

On Sun, 27 Apr 2025, 04:30 KJ7LNW, ***@***.***> wrote: *KJ7LNW* left a comment (RooCodeInc/Roo-Code#2886) <#2886 (comment)> 💪🏻!!! I'll check it out — Reply to this email directly, view it on GitHub <#2886 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACUTT3JKUBPPBZARXW25LML23RFNLAVCNFSM6AAAAAB3XC6R66VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQMZSHEZTOMJXG4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[samhvw8]

@mrubens @cte @KJ7LNW i just updated to all xml

[KJ7LNW]

the code looks good, I am going to try it out in real life and see what I discover ...

[KJ7LNW]

FYI, I have been running this on my desk for a couple of weeks for everything that I do, this is the feedback of my experience:

Performance issues to monitor:

* If users report slowdowns when reading 10+ files at once, we might need to consider adding concurrency limiting

I have not found any slowdown, and I have had times where 15-20 files were loaded at once. it is probably 10x faster than loading them individually.

* Watch for memory spikes - the current implementation loads all files into memory simultaneously

Fortunately, this happens on the backend, not in the web view.

* Keep an eye on VS Code becoming unresponsive during large batch reads

I have never seen that problem it has been nice and snappy.

Things that might break:

* Very large files (100MB+) being read in batches could cause OOM errors

That is an issue for single file reads, too, because a file that big would exceed available context for all existing models today; if you are using read-size limits, then this is not an issue. Read limits default to 500 lines, but it may have changed recently.

* No validation on total bytes being read - someone could accidentally read gigabytes (while this is also a potential issue with our current `read_file`, this implementation makes it easier for this to happen)

If this is going to be addressed it needs to be addressed per-file. We have line-limit constraints available to handle this already there is probably not a better way to do it.

Quick fixes to consider post-merge:

1. Add a simple concurrency limit (even just 5 concurrent file ops would help)

There already is. I keep mine at 100 and have never had a problem:

2. Track total bytes being read and warn/limit if it's too much

I disagree with this because we already have line limit handling that users can enforce. "too much" is too subjective, it will very by user model and use case so let's keep the existing line-limit behavior.

3. Make the error messages clearer when batch operations fail

is there a specific error message you are concerned about?

4. Add telemetry to see how people actually use this feature

Good idea.

Can this be merged and add telemetry in a separate PR?

[KJ7LNW]

When checked for the first time this should probably default to a higher number?

@samhvw8 can you set this to 15 by default immediately when experimental flag is enabled? Users can then adjust it from there.

[daniel-lxs]

@KJ7LNW, Can you check again the default value for concurrent reads? I made a change regarding that specifically.

The value is set to 15 for me when I enable the feature.

[KJ7LNW]

@KJ7LNW, Can you check again the default value for concurrent reads? I made a change regarding that specifically.

The value is set to 15 for me when I enable the feature.

If that is what it does for you, then it must be correct, as I am going off slightly older information.

sorry for any confusion

[mrubens]

Nice!