-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: accept identical diffs as no-op instead of error #7184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Modified multi-search-replace.ts and multi-file-search-replace.ts to treat identical search/replace content as successful no-op operations - Added comprehensive tests for the new behavior - Fixes #7183 where Gemini and other AI providers sometimes generate identical diffs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own code is like debugging in a mirror - everything looks backwards but somehow still broken.
| // This is a no-op diff, skip it without error | ||
| // Some AI providers (like Gemini) sometimes generate identical diffs | ||
| // We treat these as successful no-ops rather than errors | ||
| appliedCount++ // Count it as applied since it's intentionally a no-op |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding telemetry here to track how often identical diffs occur. This data could help us understand AI provider behavior patterns and potentially improve the user experience further.
| // This is a no-op diff, skip it without error | ||
| // Some AI providers (like Gemini) sometimes generate identical diffs | ||
| // We treat these as successful no-ops rather than errors | ||
| appliedCount++ // Count it as applied since it's intentionally a no-op |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same telemetry suggestion applies here for consistency. Also, could we add debug-level logging when skipping identical diffs? It might help with troubleshooting without affecting production performance.
| // Content should remain unchanged | ||
| expect(result.content).toBe(originalContent) | ||
| } | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test coverage is comprehensive, but consider adding an explicit test that verifies the is correctly incremented for no-op diffs. This would ensure we're properly counting them as successful operations rather than just skipping them.
Fixes #7183
Summary
This PR modifies the diff application logic to treat identical search and replace content as successful no-op operations instead of throwing errors. This addresses the issue where AI providers like Gemini sometimes generate identical diffs.
Changes
multi-search-replace.tsto skip identical diffs silently and count them as successful no-opsmulti-file-search-replace.tswith the same logic for consistencyidentical-diff.spec.tsTesting
Impact
This change makes Roo Code more tolerant of AI-generated diffs, improving the user experience when working with providers that occasionally produce identical search/replace blocks.
Important
Identical diffs are now treated as no-ops in
multi-search-replace.tsandmulti-file-search-replace.ts, with tests added inidentical-diff.spec.ts.multi-search-replace.tsandmulti-file-search-replace.tsnow treat identical search and replace content as successful no-ops, incrementingappliedCountwithout error.identical-diff.spec.tscover scenarios with identical diffs, including single and multiple diff cases.This description was created by
for 8a86af0. You can customize this summary. It will automatically update as commits are pushed.