Skip to content

Fix added_lines and deleted_lines dropping "++" and "--" content - #324

Merged
ishepard merged 1 commit into
ishepard:masterfrom
MaxFreedomPollard:fix-added-deleted-lines-count
Sep 11, 2026
Merged

ishepard merged 1 commit into
ishepard:masterfrom
MaxFreedomPollard:fix-added-deleted-lines-count

Conversation

@MaxFreedomPollard

Copy link
Copy Markdown
Contributor

ModifiedFile.added_lines skips every diff line starting with +++, and ModifiedFile.deleted_lines skips every one starting with ---, to avoid counting the +++ b/file and --- a/file patch headers. Those headers are never in the string being scanned: GitPython's Diff.re_header matches them as part of the header block and assigns Diff.diff only the text that follows, so ModifiedFile.diff always starts at the first @@. The guards can therefore only match real content. An added line ++i; appears as +++i; in the patch and a deleted line --i; appears as ---i;, and both go uncounted.

So the counts disagree with Commit.insertions and Commit.deletions, which come from numstat, and with ModifiedFile.diff_parsed, which classifies the same lines with no header guard, as does HunksCount at pydriller/metrics/process/hunks_count.py:45. There is already a case in the bundled repos: in test-repos/diff, commit 156111a7 deletes 9 lines from docs/reference.rst, one of them the reST underline ----------------, and deleted_lines returns 8 while git diff --numstat and diff_parsed["deleted"] both say 9. Everything built on these two properties inherits the undercount, including CodeChurn, LinesCount, ContributorsCount, ContributorsExperience and HistoryComplexity.

The fix drops both guards in pydriller/domain/commit.py, so every + and - line in the hunk body is counted.

Two tests in tests/test_commit.py: one over test-repos/diff that pins the reference.rst count at 9 and checks the per-file sums now match Commit.insertions and Commit.deletions, and one that runs a small patch containing +++i; and ---i; through a mocked Diff. On master they fail with assert 8 == 9 and assert 0 == 1. With the change, mypy --ignore-missing-imports pydriller/ tests/, flake8 and pytest tests/ all pass locally on Python 3.11 on macOS.

ModifiedFile.added_lines skipped every diff line starting with "+++",
and ModifiedFile.deleted_lines skipped every one starting with "---",
to avoid counting the "+++ b/file" and "--- a/file" patch headers.
GitPython's Diff.re_header already consumes both headers, so Diff.diff
starts at the first "@@" hunk header and those guards could only match
real content: an added line "++i;" appears as "+++i;" in the patch, and
a deleted line "--i;" appears as "---i;".

The counts therefore disagreed with ModifiedFile.diff_parsed and with
Commit.insertions and Commit.deletions, which come from numstat. In
test-repos/diff, commit 156111a deletes 9 lines from docs/reference.rst,
one of them the reST underline "----------------", and deleted_lines
returned 8.

pydriller/domain/commit.py: drop both guards so every "+" and "-" line
in the hunk body is counted.
@codecov

codecov Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.39%. Comparing base (cf19096) to head (061fd73).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #324   +/-   ##
=======================================
  Coverage   97.39%   97.39%           
=======================================
  Files          16       16           
  Lines        1150     1150           
=======================================
  Hits         1120     1120           
  Misses         30       30           
Files with missing lines Coverage Δ
pydriller/domain/commit.py 97.38% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ishepard

Copy link
Copy Markdown
Owner

Amazing finding 😄 crazy that the bug has been there for all these years! thanks for fixing it!

@ishepard
ishepard merged commit 2c884df into ishepard:master Sep 11, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants