Skip to content

Conversation

@KJ7LNW
Copy link
Contributor

@KJ7LNW KJ7LNW commented Mar 21, 2025

Roo read_file/apply_diff Hackfest

Today @hannesrudolph and I we are troubleshooting strange Claude hallucinations related to line numbers and other issues. While troubleshooting that, I hacked together these few commits.

Do not merge this! --- at least not yet. I am sharing this with the community for feedback and for others who may wish to test the behavior.

what does it do?

This branch:

  1. Absolutely forces the model to use apply_diff if a file exists (write_to_file is denied)
  2. Prevents line numbers from being added to reads
  3. Relaxes line number checking in multi-diff

Background

While there are places where line numbers are useful, line numbers in file reads have a few issues:

  • Line numbers increase context usage
  • They can confuse the replacement algorithm, especially when a line legitimately starts with a | symbol because it interferes with the line number separator and confuses the model (just try to edit a markdown table, I dare you...)
  • Modifying the file content (ie, line numbers) gives an alternate perception to the model vs. reality, and then we have to trust the model (!?) to exclude those line numbers later for operations...
  • Line numbers can distract the model from the content (123 | Squirrel!):
    1. When trying to produce exact search content for replacement
    2. When trying to consider indentation, especially since our line number markers contains spaces: 123 |

There are places where line numbers are useful, here are a few:

  • Search results
  • Declaration details
  • read_file start/end ranges

So, I have built this into my daily driver and will report my findings and continue to refine this.

Long term goals

  • Configuration options to make this possible for use by others without breaking features
  • clean up the pr to make it not so hackish
  • others?

@changeset-bot
Copy link

changeset-bot bot commented Mar 21, 2025

⚠️ No Changeset found

Latest commit: 5ea1198

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

@KJ7LNW KJ7LNW self-assigned this Mar 26, 2025
@KJ7LNW
Copy link
Contributor Author

KJ7LNW commented Mar 26, 2025

Here is another problem with line numbers pointed out by @hannesrudolph:

Actual content in the file:

- GitHub Users page showing:
  - List of GitHub usernames
  - Link status
  - Option to create new user or link to existing user
  - Pagination for large datasets

read_file content result

35 | - GitHub Users page showing:
36 |   - List of GitHub usernames
37 |   - Link status
38 |   - Option to create new user or link to existing user
39 |   - Pagination for large datasets

Content match that AI tries to replace in apply_diff:

Notice that it thinks | is part of the tree-shaped input, but really it is the line number delimiter:

| - GitHub Users page showing:
|   - List of GitHub usernames
|   - Link status
|   - Option to create new user or link to existing user
|   - Pagination for large datasets

Solution

We might be able to find a heuristic to solve this but it may introduce edge cases. For example if the best match has all leading | symbols, try without those. Maybe this has side effects, or maybe it's just an ugly workaround, but it certainly increases complexity... I think the real fix is to get off of line numbers.

Try this number-free:

code --install-extension roo-cline-3.10.5-2025-03-25_17-47-47-build.vsix

# or
code-insiders --install-extension roo-cline-3.10.5-2025-03-25_17-47-47-build.vsix

@KJ7LNW KJ7LNW force-pushed the roo-hack-remove-line-numbers branch from 63ddbee to a660b24 Compare March 26, 2025 00:57
@hannesrudolph hannesrudolph moved this from New to PR [Pre Approval Review] in Roo Code Roadmap Mar 26, 2025
teddyOOXX pushed a commit to teddyOOXX/Roo-Code that referenced this pull request Mar 28, 2025
* timeouts for individual servers

* changeset

* remove logger

* use const and descriptive function for time settings
teddyOOXX pushed a commit to teddyOOXX/Roo-Code that referenced this pull request Mar 28, 2025
@KJ7LNW KJ7LNW force-pushed the roo-hack-remove-line-numbers branch from a660b24 to d6fe15b Compare April 24, 2025 21:53
Eric Wheeler added 5 commits April 24, 2025 16:51
this tool does not work without line numbers
The insert_content tool was intended for appending content only, but the code
allowed arbitrary line numbers which could corrupt files. This change enforces
that line_number must be 0 (append-only mode) to prevent file corruption.

Signed-off-by: Eric Wheeler <[email protected]>
Add documentation showing how to use insert_content in conjunction with
write_to_file to handle files that exceed output limits by breaking them
into multiple append operations.

Signed-off-by: Eric Wheeler <[email protected]>
Add line count check to writeToFileTool to only require apply_diff for files that are more than 25 lines long. This allows write_to_file to work on small existing files while still protecting larger files that need more careful modification.

Signed-off-by: Eric Wheeler <[email protected]>
@hannesrudolph hannesrudolph moved this from PR [Pre Approval Review] to PR [Draft/WIP] in Roo Code Roadmap May 10, 2025
@hannesrudolph hannesrudolph moved this from New to PR [Draft/WIP] in Roo Code Roadmap May 20, 2025
@hannesrudolph hannesrudolph moved this from PR [Draft / In Progress] to TEMP in Roo Code Roadmap May 26, 2025
@daniel-lxs
Copy link
Member

Hey @KJ7LNW,
Thank you for your contribution, We noticed this PR is stale and will be closed. If you plan to revisit this, please create an issue first as required by our issue-first approach before opening a new PR.

@KJ7LNW
Copy link
Contributor Author

KJ7LNW commented May 26, 2025

Hey @KJ7LNW, Thank you for your contribution, We noticed this PR is stale and will be closed. If you plan to revisit this, please create an issue first as required by our issue-first approach before opening a new PR.

I am still working on this and I maintain it every time I rebuild my local tree for further testing. I have been using this pull request for months, and it works great, but we will need an option so users can toggle the feature, which has not yet been implemented.

This pull request was created before the requirement of having an issue for each pull request, so here it is: #4008

@KJ7LNW KJ7LNW reopened this May 26, 2025
@github-project-automation github-project-automation bot moved this from Done to New in Roo Code Roadmap May 26, 2025
@github-project-automation github-project-automation bot moved this from Done to Triage in Roo Code Roadmap May 26, 2025
@KJ7LNW KJ7LNW changed the title Hack: remove line numbers from reads and diffs WIP: optionally omit line number from reads and apply_diff requirements May 26, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Draft / In Progress] in Roo Code Roadmap May 26, 2025
@hannesrudolph
Copy link
Collaborator

stale

@github-project-automation github-project-automation bot moved this from PR [Draft / In Progress] to Done in Roo Code Roadmap Jul 7, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jul 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants