fix(diff): don't overcount new-file additions by the trailing newline - #1873
Conversation
countLinesChanged's new-file branch counted added lines as newFileContent.split(/\r?\n/).length. Content that ends in a newline (the normal case) yields a trailing empty element, so a 2-line file "a\nb\n" counted as 3 additions instead of 2. That inflated the tengu_file_changed telemetry and the running total-lines-changed counter for essentially every new file, and disagreed with the diff-based update path (which counts `+` hunk lines git-accurately). Drop the phantom trailing line when the content is newline-terminated so the count matches git and the update path.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🧰 Additional context used📓 Path-based instructions (4)src/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**⚙️ CodeRabbit configuration file
Files:
**/*⚙️ CodeRabbit configuration file
Files:
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (2)
📝 WalkthroughWalkthroughThis PR fixes an off-by-one error in ChangesDiff line counting fix
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I do not see any actionable issues from my review.
@kevincodex1 LGTM
…Gitlawb#1873) countLinesChanged's new-file branch counted added lines as newFileContent.split(/\r?\n/).length. Content that ends in a newline (the normal case) yields a trailing empty element, so a 2-line file "a\nb\n" counted as 3 additions instead of 2. That inflated the tengu_file_changed telemetry and the running total-lines-changed counter for essentially every new file, and disagreed with the diff-based update path (which counts `+` hunk lines git-accurately). Drop the phantom trailing line when the content is newline-terminated so the count matches git and the update path. (cherry picked from commit 8d90849) (cherry picked from commit 63e057b13ca994335ba0c2d934144767a9b9aba3)
Summary
countLinesChangedhas a dedicated branch for new files (used byFileWriteToolwhen writing a file that did not exist), which counts every content line as an addition:Splitting on newlines produces a trailing empty element when the content ends in a newline — which is the normal case for a text file. So a 2-line file
"a\nb\n"is counted as 3 additions instead of 2,"line1\nline2\nline3\n"as 4 instead of 3, and so on. Git reports 2+lines for a 2-line new file, and the diff-based update path in the same function counts+-prefixed hunk lines git-accurately — so the create path is off by one against both.The inflated count feeds the
tengu_file_changedanalytics event (lines_added) and the process-wide total-lines-changed counter (addToTotalLinesChanged→ session stats), so nearly every new file overstates added lines by one.Fix
Drop the phantom trailing element when the content is newline-terminated, matching git and the update path. Content without a trailing newline (and CRLF endings) are unaffected.
Testing
New
src/utils/diff.test.tsmeasures the deltacountLinesChangedcontributes to the running total:"a\nb\n"→ 2,"line1\nline2\nline3\n"→ 3,"only\n"→ 1 (newline-terminated)."a\nb"→ 2,"single"→ 1 (no trailing newline, already correct)."a\r\nb\r\n") counts the same as LF.Reverting to the plain
.split(...).lengthre-fails the newline-terminated and CRLF assertions.Summary by CodeRabbit