From 9ded096839d8374d8ef387debd7be1e0c109757e Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Thu, 11 Dec 2025 10:11:15 +0000 Subject: [PATCH] Create meaningful lines calculations. --- README.md | 27 ++ config.example.yaml | 2 + internal/aggregator/aggregator.go | 31 +- internal/config/schema.go | 33 +- internal/diff/analyzer.go | 137 +++++++ internal/diff/analyzer_test.go | 431 +++++++++++++++++++++ internal/domain/models/commit.go | 4 + internal/domain/models/metrics.go | 12 + internal/domain/models/pullrequest.go | 4 + internal/domain/scoring/calculator.go | 14 +- internal/domain/scoring/calculator_test.go | 130 +++++++ internal/git/repository.go | 105 ++--- internal/github/client.go | 45 ++- 13 files changed, 866 insertions(+), 109 deletions(-) create mode 100644 internal/diff/analyzer.go create mode 100644 internal/diff/analyzer_test.go diff --git a/README.md b/README.md index 6665091..214b109 100644 --- a/README.md +++ b/README.md @@ -47,6 +47,7 @@ $ git-velocity serve --port 8080 - **Pull Requests**: Opened, merged, closed, average size, time to merge - **Code Reviews**: Reviews given, comments, approvals, response time - **Issues**: Opened, closed, comments +- **Meaningful Lines**: Filter out comments, whitespace, and documentation changes from line counts ### 🎮 Gamification Engine - **Scoring System**: Earn points for every contribution @@ -305,6 +306,7 @@ scoring: commit_with_tests: 15 lines_added: 0.1 lines_deleted: 0.05 + use_meaningful_lines: true # Exclude comments/whitespace from line scoring pr_opened: 25 pr_merged: 50 pr_reviewed: 30 @@ -386,6 +388,31 @@ options: - "*-ci" # Suffix match ``` +### Meaningful Lines Filtering + +By default, Git Velocity filters out non-meaningful code changes when scoring line additions and deletions. This provides a more accurate measure of actual code contributions. + +**What's filtered out:** +- **Comments**: Single-line (`//`, `#`, `--`), block (`/* */`, ``), docstrings (`"""`, `'''`) +- **Whitespace**: Empty lines, whitespace-only lines +- **Documentation files**: `.md`, `.rst`, `.txt`, `README`, `CHANGELOG`, `LICENSE`, files in `docs/` directories + +**Supported comment styles:** +- C-style: `//`, `/* */`, `*` (block continuation) +- Python/Shell: `#`, `"""`, `'''` +- SQL/Lua/Haskell: `--` +- Assembly/Lisp/INI: `;` +- VB: `'` +- HTML/XML: `` + +To disable this filtering and score raw line counts: + +```yaml +scoring: + points: + use_meaningful_lines: false # Score all lines including comments/whitespace +``` + ### Environment Variables All configuration values support environment variable expansion: diff --git a/config.example.yaml b/config.example.yaml index 58de391..84f6f40 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -89,6 +89,8 @@ scoring: commit_with_tests: 15 lines_added: 0.1 lines_deleted: 0.05 + # Use meaningful lines (excludes comments/whitespace) for scoring + use_meaningful_lines: true pr_opened: 25 pr_merged: 50 pr_reviewed: 30 diff --git a/internal/aggregator/aggregator.go b/internal/aggregator/aggregator.go index cfe4118..a9b11f1 100644 --- a/internal/aggregator/aggregator.go +++ b/internal/aggregator/aggregator.go @@ -137,6 +137,8 @@ func (a *Aggregator) Aggregate(data *models.RawData, dateRange *config.ParsedDat cm.CommitCount++ cm.LinesAdded += commit.Additions cm.LinesDeleted += commit.Deletions + cm.MeaningfulLinesAdded += commit.MeaningfulAdditions + cm.MeaningfulLinesDeleted += commit.MeaningfulDeletions cm.FilesChanged += commit.FilesChanged // Update per-repo contributor stats @@ -144,6 +146,8 @@ func (a *Aggregator) Aggregate(data *models.RawData, dateRange *config.ParsedDat rcm.CommitCount++ rcm.LinesAdded += commit.Additions rcm.LinesDeleted += commit.Deletions + rcm.MeaningfulLinesAdded += commit.MeaningfulAdditions + rcm.MeaningfulLinesDeleted += commit.MeaningfulDeletions rcm.FilesChanged += commit.FilesChanged // Track activity patterns based on commit time @@ -203,6 +207,8 @@ func (a *Aggregator) Aggregate(data *models.RawData, dateRange *config.ParsedDat rm.TotalCommits++ rm.TotalLinesAdded += commit.Additions rm.TotalLinesDeleted += commit.Deletions + rm.TotalMeaningfulLinesAdded += commit.MeaningfulAdditions + rm.TotalMeaningfulLinesDeleted += commit.MeaningfulDeletions } // Calculate active days and streaks for each contributor @@ -535,28 +541,33 @@ func (a *Aggregator) Aggregate(data *models.RawData, dateRange *config.ParsedDat // Calculate totals var totalCommits, totalPRs, totalReviews, totalLinesAdded, totalLinesDeleted int + var totalMeaningfulLinesAdded, totalMeaningfulLinesDeleted int for _, rm := range repositories { totalCommits += rm.TotalCommits totalPRs += rm.TotalPRs totalReviews += rm.TotalReviews totalLinesAdded += rm.TotalLinesAdded totalLinesDeleted += rm.TotalLinesDeleted + totalMeaningfulLinesAdded += rm.TotalMeaningfulLinesAdded + totalMeaningfulLinesDeleted += rm.TotalMeaningfulLinesDeleted } // Build velocity timeline (weekly aggregation) velocityTimeline := buildVelocityTimeline(data, period, a.config.Scoring) return &models.GlobalMetrics{ - Period: period, - Repositories: repositories, - Teams: teams, - TotalContributors: len(contributors), - TotalCommits: totalCommits, - TotalPRs: totalPRs, - TotalReviews: totalReviews, - TotalLinesAdded: totalLinesAdded, - TotalLinesDeleted: totalLinesDeleted, - VelocityTimeline: velocityTimeline, + Period: period, + Repositories: repositories, + Teams: teams, + TotalContributors: len(contributors), + TotalCommits: totalCommits, + TotalPRs: totalPRs, + TotalReviews: totalReviews, + TotalLinesAdded: totalLinesAdded, + TotalLinesDeleted: totalLinesDeleted, + TotalMeaningfulLinesAdded: totalMeaningfulLinesAdded, + TotalMeaningfulLinesDeleted: totalMeaningfulLinesDeleted, + VelocityTimeline: velocityTimeline, }, nil } diff --git a/internal/config/schema.go b/internal/config/schema.go index 9092094..88c8881 100644 --- a/internal/config/schema.go +++ b/internal/config/schema.go @@ -88,6 +88,10 @@ type PointsConfig struct { FastReview4h int `yaml:"fast_review_4h"` FastReview24h int `yaml:"fast_review_24h"` OutOfHours int `yaml:"out_of_hours"` // Bonus per commit outside 9am-5pm + + // UseMeaningfulLines determines whether scoring uses meaningful lines (excluding comments/whitespace) + // or raw line counts. Default is true for more accurate contribution scoring. + UseMeaningfulLines bool `yaml:"use_meaningful_lines"` } // AchievementConfig defines an achievement badge @@ -185,20 +189,21 @@ func DefaultConfig() *Config { Scoring: ScoringConfig{ Enabled: true, Points: PointsConfig{ - Commit: 10, - CommitWithTests: 15, - LinesAdded: 0.1, - LinesDeleted: 0.05, - PROpened: 25, - PRMerged: 50, - PRReviewed: 30, - ReviewComment: 5, - IssueOpened: 15, - IssueClosed: 20, - FastReview1h: 50, - FastReview4h: 25, - FastReview24h: 10, - OutOfHours: 2, + Commit: 10, + CommitWithTests: 15, + LinesAdded: 0.1, + LinesDeleted: 0.05, + PROpened: 25, + PRMerged: 50, + PRReviewed: 30, + ReviewComment: 5, + IssueOpened: 15, + IssueClosed: 20, + FastReview1h: 50, + FastReview4h: 25, + FastReview24h: 10, + OutOfHours: 2, + UseMeaningfulLines: true, // Default to meaningful lines for accurate contribution scoring }, }, Output: OutputConfig{ diff --git a/internal/diff/analyzer.go b/internal/diff/analyzer.go new file mode 100644 index 0000000..d21fee4 --- /dev/null +++ b/internal/diff/analyzer.go @@ -0,0 +1,137 @@ +package diff + +import ( + "strings" +) + +// IsCommentLine checks if a line is a code comment (should not count as meaningful contribution) +func IsCommentLine(line string) bool { + trimmed := strings.TrimSpace(line) + if trimmed == "" { + return true // Empty lines don't count + } + + // Common comment patterns across languages + commentPrefixes := []string{ + "//", // C, C++, Java, Go, JS, TS, Swift, Kotlin, etc. + "#", // Python, Ruby, Shell, YAML, Perl, etc. + "/*", // C-style block comment start + "*/", // C-style block comment end + "*", // C-style block comment continuation + "", // HTML/XML comment end + "--", // SQL, Lua, Haskell + ";", // Assembly, Lisp, INI files + "'", // VB comment + "\"\"\"", // Python docstring + "'''", // Python docstring + } + + for _, prefix := range commentPrefixes { + if strings.HasPrefix(trimmed, prefix) { + return true + } + } + + return false +} + +// IsWhitespaceLine checks if a line contains only whitespace characters +func IsWhitespaceLine(line string) bool { + return strings.TrimSpace(line) == "" +} + +// IsDocumentationFile checks if a file is documentation-only +func IsDocumentationFile(filename string) bool { + // Documentation file extensions and patterns + docPatterns := []string{ + ".md", ".markdown", ".rst", ".txt", ".adoc", + "README", "CHANGELOG", "LICENSE", "CONTRIBUTING", + "docs/", "documentation/", "/doc/", + } + + lowerFilename := strings.ToLower(filename) + for _, pattern := range docPatterns { + if strings.Contains(lowerFilename, strings.ToLower(pattern)) { + return true + } + } + return false +} + +// PatchStats holds the results of analyzing a diff patch +type PatchStats struct { + TotalAdditions int + TotalDeletions int + MeaningfulAdditions int + MeaningfulDeletions int + CommentAdditions int + CommentDeletions int + WhitespaceAdditions int + WhitespaceDeletions int +} + +// AnalyzePatch analyzes a unified diff patch and returns both raw and meaningful line counts. +// It parses diff hunks and categorizes each changed line as meaningful, comment, or whitespace. +func AnalyzePatch(patch string) PatchStats { + stats := PatchStats{} + + lines := strings.Split(patch, "\n") + for _, line := range lines { + if len(line) == 0 { + continue + } + + // Check if this is an addition or deletion line + isAddition := strings.HasPrefix(line, "+") && !strings.HasPrefix(line, "+++") + isDeletion := strings.HasPrefix(line, "-") && !strings.HasPrefix(line, "---") + + if !isAddition && !isDeletion { + continue // Context line or header + } + + // Remove the diff prefix to get actual content + content := line[1:] + + // Categorize the line + if IsWhitespaceLine(content) { + if isAddition { + stats.TotalAdditions++ + stats.WhitespaceAdditions++ + } else { + stats.TotalDeletions++ + stats.WhitespaceDeletions++ + } + } else if IsCommentLine(content) { + if isAddition { + stats.TotalAdditions++ + stats.CommentAdditions++ + } else { + stats.TotalDeletions++ + stats.CommentDeletions++ + } + } else { + // Meaningful code line + if isAddition { + stats.TotalAdditions++ + stats.MeaningfulAdditions++ + } else { + stats.TotalDeletions++ + stats.MeaningfulDeletions++ + } + } + } + + return stats +} + +// AnalyzePatchSimple returns just the meaningful additions and deletions +func AnalyzePatchSimple(patch string) (meaningfulAdds, meaningfulDels int) { + stats := AnalyzePatch(patch) + return stats.MeaningfulAdditions, stats.MeaningfulDeletions +} + +// IsMeaningfulLine checks if a line of code is meaningful (not a comment or whitespace) +func IsMeaningfulLine(line string) bool { + return !IsWhitespaceLine(line) && !IsCommentLine(line) +} diff --git a/internal/diff/analyzer_test.go b/internal/diff/analyzer_test.go new file mode 100644 index 0000000..ad34b28 --- /dev/null +++ b/internal/diff/analyzer_test.go @@ -0,0 +1,431 @@ +package diff + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestIsCommentLine(t *testing.T) { + tests := []struct { + name string + line string + expected bool + }{ + // Empty and whitespace + {"empty string", "", true}, + {"whitespace only", " ", true}, + {"tab only", "\t", true}, + {"mixed whitespace", " \t ", true}, + + // C-style comments (Go, Java, JS, C++, etc.) + {"C single line comment", "// this is a comment", true}, + {"C single line with leading space", " // this is a comment", true}, + {"C block start", "/* block comment", true}, + {"C block end", "*/", true}, + {"C block continuation", "* continuation", true}, + {"C block continuation with space", " * continuation", true}, + + // Python/Shell comments + {"Python comment", "# python comment", true}, + {"Shell comment", "#!/bin/bash", true}, + {"Python comment with space", " # comment", true}, + + // Python docstrings + {"Python docstring double", "\"\"\"docstring", true}, + {"Python docstring single", "'''docstring", true}, + + // SQL/Lua/Haskell comments + {"SQL comment", "-- SQL comment", true}, + + // Assembly/Lisp/INI comments + {"Assembly comment", "; assembly comment", true}, + {"INI comment", "; ini comment", true}, + + // VB comments + {"VB comment", "' VB comment", true}, + + // HTML/XML comments + {"HTML comment start", "", true}, + + // Actual code - NOT comments + {"Go code", "func main() {", false}, + {"Python code", "def main():", false}, + {"JS code", "const x = 5;", false}, + {"Variable assignment", "x = 10", false}, + {"Return statement", "return nil", false}, + {"Import statement", "import fmt", false}, + {"Package declaration", "package main", false}, + {"Struct field", "Name string", false}, + {"Function call", "fmt.Println(x)", false}, + {"String with slash", `"http://example.com"`, false}, + {"Code after whitespace", " x := 5", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := IsCommentLine(tt.line) + assert.Equal(t, tt.expected, result, "IsCommentLine(%q)", tt.line) + }) + } +} + +func TestIsWhitespaceLine(t *testing.T) { + tests := []struct { + name string + line string + expected bool + }{ + {"empty string", "", true}, + {"single space", " ", true}, + {"multiple spaces", " ", true}, + {"single tab", "\t", true}, + {"multiple tabs", "\t\t\t", true}, + {"mixed whitespace", " \t \t ", true}, + {"newline only", "\n", true}, + {"carriage return", "\r", true}, + {"code line", "x := 5", false}, + {"code with leading whitespace", " x := 5", false}, + {"comment line", "// comment", false}, + {"single character", "x", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := IsWhitespaceLine(tt.line) + assert.Equal(t, tt.expected, result, "IsWhitespaceLine(%q)", tt.line) + }) + } +} + +func TestIsDocumentationFile(t *testing.T) { + tests := []struct { + name string + filename string + expected bool + }{ + // Documentation files + {"readme markdown", "README.md", true}, + {"readme uppercase", "README", true}, + {"readme lowercase", "readme.md", true}, + {"changelog", "CHANGELOG.md", true}, + {"license", "LICENSE", true}, + {"license txt", "LICENSE.txt", true}, + {"contributing", "CONTRIBUTING.md", true}, + {"markdown file", "docs.md", true}, + {"rst file", "index.rst", true}, + {"txt file", "notes.txt", true}, + {"adoc file", "guide.adoc", true}, + {"docs directory", "docs/api.md", true}, + {"documentation directory", "documentation/guide.md", true}, + {"doc directory", "/doc/api.md", true}, + + // Code files - NOT documentation + {"go file", "main.go", false}, + {"python file", "app.py", false}, + {"js file", "index.js", false}, + {"ts file", "app.ts", false}, + {"java file", "App.java", false}, + {"c file", "main.c", false}, + {"cpp file", "main.cpp", false}, + {"rust file", "main.rs", false}, + {"yaml file", "config.yaml", false}, + {"json file", "package.json", false}, + {"html file", "index.html", false}, + {"css file", "style.css", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := IsDocumentationFile(tt.filename) + assert.Equal(t, tt.expected, result, "IsDocumentationFile(%q)", tt.filename) + }) + } +} + +func TestAnalyzePatch(t *testing.T) { + tests := []struct { + name string + patch string + expected PatchStats + }{ + { + name: "simple additions", + patch: `@@ -1,3 +1,5 @@ + context line ++func main() { ++ x := 5 ++}`, + expected: PatchStats{ + TotalAdditions: 3, + MeaningfulAdditions: 3, + }, + }, + { + name: "simple deletions", + patch: `@@ -1,5 +1,3 @@ + context line +-func main() { +- x := 5 +-}`, + expected: PatchStats{ + TotalDeletions: 3, + MeaningfulDeletions: 3, + }, + }, + { + name: "mixed additions and deletions", + patch: `@@ -1,3 +1,3 @@ +-old code ++new code`, + expected: PatchStats{ + TotalAdditions: 1, + TotalDeletions: 1, + MeaningfulAdditions: 1, + MeaningfulDeletions: 1, + }, + }, + { + name: "comment only changes", + patch: `@@ -1,3 +1,5 @@ + func main() { ++// This is a comment ++// Another comment + }`, + expected: PatchStats{ + TotalAdditions: 2, + CommentAdditions: 2, + }, + }, + { + name: "whitespace only changes", + patch: `@@ -1,3 +1,5 @@ + func main() { ++ ++ + }`, + expected: PatchStats{ + TotalAdditions: 2, + WhitespaceAdditions: 2, + }, + }, + { + name: "mixed meaningful and non-meaningful", + patch: `@@ -1,5 +1,10 @@ + func main() { ++// Add logging ++ x := 5 ++ ++ // Calculate result ++ result := x * 2 ++ + }`, + expected: PatchStats{ + TotalAdditions: 6, + MeaningfulAdditions: 2, // x := 5 and result := x * 2 + CommentAdditions: 2, // two comments + WhitespaceAdditions: 2, // two empty lines + }, + }, + { + name: "deleted comments", + patch: `@@ -1,5 +1,2 @@ + func main() { +-// Old comment +-/* Block comment */ + }`, + expected: PatchStats{ + TotalDeletions: 2, + CommentDeletions: 2, + }, + }, + { + name: "python style comments", + patch: `@@ -1,3 +1,6 @@ + def main(): ++# This is a python comment ++"""This is a docstring""" ++ x = 5`, + expected: PatchStats{ + TotalAdditions: 3, + MeaningfulAdditions: 1, // x = 5 + CommentAdditions: 2, // # comment and docstring + }, + }, + { + name: "sql comments", + patch: `@@ -1,2 +1,4 @@ + SELECT * FROM users ++-- This is a SQL comment ++WHERE id = 1`, + expected: PatchStats{ + TotalAdditions: 2, + MeaningfulAdditions: 1, // WHERE clause + CommentAdditions: 1, // SQL comment + }, + }, + { + name: "empty patch", + patch: "", + expected: PatchStats{ + TotalAdditions: 0, + TotalDeletions: 0, + MeaningfulAdditions: 0, + MeaningfulDeletions: 0, + }, + }, + { + name: "context only patch", + patch: `@@ -1,3 +1,3 @@ + line 1 + line 2 + line 3`, + expected: PatchStats{ + TotalAdditions: 0, + TotalDeletions: 0, + MeaningfulAdditions: 0, + MeaningfulDeletions: 0, + }, + }, + { + name: "header lines should be ignored", + patch: `--- a/file.go ++++ b/file.go +@@ -1,3 +1,4 @@ + context ++new line`, + expected: PatchStats{ + TotalAdditions: 1, + MeaningfulAdditions: 1, + }, + }, + { + name: "c-style block comment continuation", + patch: `@@ -1,2 +1,5 @@ + code ++/* ++ * Block comment ++ */`, + expected: PatchStats{ + TotalAdditions: 3, + CommentAdditions: 3, + }, + }, + { + name: "html comments", + patch: `@@ -1,2 +1,4 @@ +
++ ++

Content

`, + expected: PatchStats{ + TotalAdditions: 2, + MeaningfulAdditions: 1, //

tag + CommentAdditions: 1, // HTML comment + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := AnalyzePatch(tt.patch) + assert.Equal(t, tt.expected.TotalAdditions, result.TotalAdditions, "TotalAdditions") + assert.Equal(t, tt.expected.TotalDeletions, result.TotalDeletions, "TotalDeletions") + assert.Equal(t, tt.expected.MeaningfulAdditions, result.MeaningfulAdditions, "MeaningfulAdditions") + assert.Equal(t, tt.expected.MeaningfulDeletions, result.MeaningfulDeletions, "MeaningfulDeletions") + assert.Equal(t, tt.expected.CommentAdditions, result.CommentAdditions, "CommentAdditions") + assert.Equal(t, tt.expected.CommentDeletions, result.CommentDeletions, "CommentDeletions") + assert.Equal(t, tt.expected.WhitespaceAdditions, result.WhitespaceAdditions, "WhitespaceAdditions") + assert.Equal(t, tt.expected.WhitespaceDeletions, result.WhitespaceDeletions, "WhitespaceDeletions") + }) + } +} + +func TestAnalyzePatchSimple(t *testing.T) { + patch := `@@ -1,3 +1,6 @@ + func main() { ++// comment ++ x := 5 ++ ++ y := 10 + }` + + adds, dels := AnalyzePatchSimple(patch) + assert.Equal(t, 2, adds, "meaningful additions (x := 5 and y := 10)") + assert.Equal(t, 0, dels, "meaningful deletions") +} + +func TestIsMeaningfulLine(t *testing.T) { + tests := []struct { + name string + line string + expected bool + }{ + {"code line", "x := 5", true}, + {"function definition", "func main() {", true}, + {"return statement", "return nil", true}, + {"comment line", "// comment", false}, + {"empty line", "", false}, + {"whitespace line", " ", false}, + {"python comment", "# comment", false}, + {"code with leading whitespace", " x := 5", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := IsMeaningfulLine(tt.line) + assert.Equal(t, tt.expected, result, "IsMeaningfulLine(%q)", tt.line) + }) + } +} + +func TestAnalyzePatch_RealWorldExample(t *testing.T) { + // Simulate a real-world Go file change + patch := `diff --git a/main.go b/main.go +index 1234567..abcdefg 100644 +--- a/main.go ++++ b/main.go +@@ -10,6 +10,15 @@ package main + import "fmt" + ++// ProcessData handles data processing ++// It takes input and returns processed output + func ProcessData(input string) string { ++ // Validate input ++ if input == "" { ++ return "" ++ } ++ ++ // Transform the data ++ result := strings.ToUpper(input) +- return input ++ return result + }` + + stats := AnalyzePatch(patch) + + // Count what's actually in the patch: + // Additions (lines starting with +, not +++): + // 1. +// ProcessData handles data processing -> comment + // 2. +// It takes input and returns processed output -> comment + // 3. + // Validate input -> comment + // 4. + if input == "" -> meaningful + // 5. + return "" -> meaningful + // 6. + } -> meaningful + // 7. + (empty line) -> whitespace + // 8. + // Transform the data -> comment + // 9. + result := strings.ToUpper(input) -> meaningful + // 10. + return result -> meaningful + // Total: 10 additions, 5 meaningful, 4 comments, 1 whitespace + + // Deletions (lines starting with -, not ---): + // 1. - return input -> meaningful + // Total: 1 deletion, 1 meaningful + + assert.Equal(t, 10, stats.TotalAdditions, "Total additions") + assert.Equal(t, 1, stats.TotalDeletions, "Total deletions") + assert.Equal(t, 5, stats.MeaningfulAdditions, "Meaningful additions") + assert.Equal(t, 1, stats.MeaningfulDeletions, "Meaningful deletions") + assert.Equal(t, 4, stats.CommentAdditions, "Comment additions") + assert.Equal(t, 1, stats.WhitespaceAdditions, "Whitespace additions") +} diff --git a/internal/domain/models/commit.go b/internal/domain/models/commit.go index da35f87..d3aa987 100644 --- a/internal/domain/models/commit.go +++ b/internal/domain/models/commit.go @@ -15,6 +15,10 @@ type Commit struct { Repository string `json:"repository"` // owner/repo format URL string `json:"url"` + // Meaningful line counts (excludes comments and whitespace) + MeaningfulAdditions int `json:"meaningful_additions"` + MeaningfulDeletions int `json:"meaningful_deletions"` + // Derived fields HasTests bool `json:"has_tests"` } diff --git a/internal/domain/models/metrics.go b/internal/domain/models/metrics.go index a2a657e..040ab9e 100644 --- a/internal/domain/models/metrics.go +++ b/internal/domain/models/metrics.go @@ -23,6 +23,10 @@ type ContributorMetrics struct { LinesDeleted int `json:"lines_deleted"` FilesChanged int `json:"files_changed"` + // Meaningful line counts (excludes comments and whitespace) + MeaningfulLinesAdded int `json:"meaningful_lines_added"` + MeaningfulLinesDeleted int `json:"meaningful_lines_deleted"` + // PR metrics PRsOpened int `json:"prs_opened"` PRsMerged int `json:"prs_merged"` @@ -97,6 +101,10 @@ type RepositoryMetrics struct { ActiveContributors int `json:"active_contributors"` TotalLinesAdded int `json:"total_lines_added"` TotalLinesDeleted int `json:"total_lines_deleted"` + + // Meaningful line counts (excludes comments and whitespace) + TotalMeaningfulLinesAdded int `json:"total_meaningful_lines_added"` + TotalMeaningfulLinesDeleted int `json:"total_meaningful_lines_deleted"` } // TeamMetrics holds aggregated metrics for a team @@ -127,6 +135,10 @@ type GlobalMetrics struct { TotalLinesAdded int `json:"total_lines_added"` TotalLinesDeleted int `json:"total_lines_deleted"` + // Meaningful line counts (excludes comments and whitespace) + TotalMeaningfulLinesAdded int `json:"total_meaningful_lines_added"` + TotalMeaningfulLinesDeleted int `json:"total_meaningful_lines_deleted"` + // Velocity timeline (weekly granularity) VelocityTimeline *VelocityTimeline `json:"velocity_timeline,omitempty"` } diff --git a/internal/domain/models/pullrequest.go b/internal/domain/models/pullrequest.go index 99f7349..b4a530a 100644 --- a/internal/domain/models/pullrequest.go +++ b/internal/domain/models/pullrequest.go @@ -32,6 +32,10 @@ type PullRequest struct { Reviews []Review `json:"reviews,omitempty"` URL string `json:"url"` + // Meaningful line counts (excludes comments and whitespace) + MeaningfulAdditions int `json:"meaningful_additions"` + MeaningfulDeletions int `json:"meaningful_deletions"` + // Derived fields TimeToMerge *time.Duration `json:"time_to_merge,omitempty"` TimeToFirstReview *time.Duration `json:"time_to_first_review,omitempty"` diff --git a/internal/domain/scoring/calculator.go b/internal/domain/scoring/calculator.go index ac7c917..fb04527 100644 --- a/internal/domain/scoring/calculator.go +++ b/internal/domain/scoring/calculator.go @@ -40,6 +40,8 @@ func (c *Calculator) Calculate(metrics *models.GlobalMetrics) *models.GlobalMetr existing.CommitCount += cm.CommitCount existing.LinesAdded += cm.LinesAdded existing.LinesDeleted += cm.LinesDeleted + existing.MeaningfulLinesAdded += cm.MeaningfulLinesAdded + existing.MeaningfulLinesDeleted += cm.MeaningfulLinesDeleted existing.PRsOpened += cm.PRsOpened existing.PRsMerged += cm.PRsMerged existing.ReviewsGiven += cm.ReviewsGiven @@ -157,9 +159,15 @@ func (c *Calculator) calculateScore(cm *models.ContributorMetrics) models.Score // Commit points breakdown.Commits = cm.CommitCount * points.Commit - // Line change points - breakdown.LineChanges = int(float64(cm.LinesAdded)*points.LinesAdded + - float64(cm.LinesDeleted)*points.LinesDeleted) + // Line change points - use meaningful lines if configured, otherwise raw counts + linesAdded := cm.LinesAdded + linesDeleted := cm.LinesDeleted + if points.UseMeaningfulLines { + linesAdded = cm.MeaningfulLinesAdded + linesDeleted = cm.MeaningfulLinesDeleted + } + breakdown.LineChanges = int(float64(linesAdded)*points.LinesAdded + + float64(linesDeleted)*points.LinesDeleted) // PR points breakdown.PRs = cm.PRsOpened*points.PROpened + cm.PRsMerged*points.PRMerged diff --git a/internal/domain/scoring/calculator_test.go b/internal/domain/scoring/calculator_test.go index 3dc1b32..156332e 100644 --- a/internal/domain/scoring/calculator_test.go +++ b/internal/domain/scoring/calculator_test.go @@ -748,3 +748,133 @@ func TestContains(t *testing.T) { assert.False(t, contains(slice, "d")) assert.False(t, contains([]string{}, "a")) } + +func TestCalculator_MeaningfulLinesScoring(t *testing.T) { + t.Parallel() + + t.Run("uses meaningful lines when enabled", func(t *testing.T) { + t.Parallel() + + cfg := config.DefaultConfig() + cfg.Scoring.Enabled = true + cfg.Scoring.Points = config.PointsConfig{ + Commit: 10, + LinesAdded: 0.1, + LinesDeleted: 0.05, + UseMeaningfulLines: true, // Use meaningful lines + } + calc := NewCalculator(cfg) + + metrics := &models.GlobalMetrics{ + Repositories: []models.RepositoryMetrics{ + { + FullName: "owner/repo", + Contributors: []models.ContributorMetrics{ + { + Login: "user1", + CommitCount: 10, + LinesAdded: 1000, // Raw lines + LinesDeleted: 500, + MeaningfulLinesAdded: 800, // Meaningful lines (excluding comments/whitespace) + MeaningfulLinesDeleted: 400, + RepositoriesContributed: []string{"owner/repo"}, + }, + }, + }, + }, + } + + result := calc.Calculate(metrics) + + contributor := result.Repositories[0].Contributors[0] + // Line change points should use meaningful lines: + // Meaningful: 800 * 0.1 + 400 * 0.05 = 80 + 20 = 100 + // (Not raw: 1000 * 0.1 + 500 * 0.05 = 100 + 25 = 125) + assert.Equal(t, 100, contributor.Score.Breakdown.LineChanges) + // Total: Commits (10 * 10 = 100) + Lines (100) = 200 + assert.Equal(t, 200, contributor.Score.Total) + }) + + t.Run("uses raw lines when disabled", func(t *testing.T) { + t.Parallel() + + cfg := config.DefaultConfig() + cfg.Scoring.Enabled = true + cfg.Scoring.Points = config.PointsConfig{ + Commit: 10, + LinesAdded: 0.1, + LinesDeleted: 0.05, + UseMeaningfulLines: false, // Use raw lines + } + calc := NewCalculator(cfg) + + metrics := &models.GlobalMetrics{ + Repositories: []models.RepositoryMetrics{ + { + FullName: "owner/repo", + Contributors: []models.ContributorMetrics{ + { + Login: "user1", + CommitCount: 10, + LinesAdded: 1000, // Raw lines + LinesDeleted: 500, + MeaningfulLinesAdded: 800, // Meaningful lines (should be ignored) + MeaningfulLinesDeleted: 400, + RepositoriesContributed: []string{"owner/repo"}, + }, + }, + }, + }, + } + + result := calc.Calculate(metrics) + + contributor := result.Repositories[0].Contributors[0] + // Line change points should use raw lines: + // Raw: 1000 * 0.1 + 500 * 0.05 = 100 + 25 = 125 + assert.Equal(t, 125, contributor.Score.Breakdown.LineChanges) + // Total: Commits (10 * 10 = 100) + Lines (125) = 225 + assert.Equal(t, 225, contributor.Score.Total) + }) + + t.Run("comment-only changes score zero meaningful lines", func(t *testing.T) { + t.Parallel() + + cfg := config.DefaultConfig() + cfg.Scoring.Enabled = true + cfg.Scoring.Points = config.PointsConfig{ + Commit: 10, + LinesAdded: 0.1, + LinesDeleted: 0.05, + UseMeaningfulLines: true, + } + calc := NewCalculator(cfg) + + metrics := &models.GlobalMetrics{ + Repositories: []models.RepositoryMetrics{ + { + FullName: "owner/repo", + Contributors: []models.ContributorMetrics{ + { + Login: "commenter", + CommitCount: 5, + LinesAdded: 100, // All comment lines + LinesDeleted: 50, + MeaningfulLinesAdded: 0, // No meaningful code + MeaningfulLinesDeleted: 0, + RepositoriesContributed: []string{"owner/repo"}, + }, + }, + }, + }, + } + + result := calc.Calculate(metrics) + + contributor := result.Repositories[0].Contributors[0] + // Line change points should be 0 since all lines were comments + assert.Equal(t, 0, contributor.Score.Breakdown.LineChanges) + // Total: Commits (5 * 10 = 50) + Lines (0) = 50 + assert.Equal(t, 50, contributor.Score.Total) + }) +} diff --git a/internal/git/repository.go b/internal/git/repository.go index 706092b..d6aaa54 100644 --- a/internal/git/repository.go +++ b/internal/git/repository.go @@ -14,6 +14,7 @@ import ( "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" "github.com/go-git/go-git/v5/plumbing/transport/http" + "github.com/lukaszraczylo/git-velocity/internal/diff" "github.com/lukaszraczylo/git-velocity/internal/domain/models" ) @@ -128,56 +129,6 @@ func (r *Repository) fetch(ctx context.Context, repoPath, token string) error { return nil } -// isCommentLine checks if a line is a code comment (should not count as contribution) -func isCommentLine(line string) bool { - trimmed := strings.TrimSpace(line) - if trimmed == "" { - return true // Empty lines don't count - } - - // Common comment patterns across languages - commentPrefixes := []string{ - "//", // C, C++, Java, Go, JS, etc. - "#", // Python, Ruby, Shell, YAML - "/*", // C-style block comment start - "*/", // C-style block comment end - "*", // C-style block comment continuation - "", // HTML/XML comment end - "--", // SQL, Lua, Haskell - ";", // Assembly, Lisp, INI files - "'", // VB comment - "\"\"\"", // Python docstring - "'''", // Python docstring - } - - for _, prefix := range commentPrefixes { - if strings.HasPrefix(trimmed, prefix) { - return true - } - } - - return false -} - -// isDocumentationFile checks if a file is documentation-only -func isDocumentationFile(filename string) bool { - // Documentation file extensions and patterns - docPatterns := []string{ - ".md", ".markdown", ".rst", ".txt", ".adoc", - "README", "CHANGELOG", "LICENSE", "CONTRIBUTING", - "docs/", "documentation/", "/doc/", - } - - lowerFilename := strings.ToLower(filename) - for _, pattern := range docPatterns { - if strings.Contains(lowerFilename, strings.ToLower(pattern)) { - return true - } - } - return false -} - // FetchCommits retrieves commits from the local repository using go-git func (r *Repository) FetchCommits(ctx context.Context, owner, name string, since, until *time.Time) ([]models.Commit, error) { repoPath := r.repoPath(owner, name) @@ -242,7 +193,7 @@ func (r *Repository) FetchCommits(ctx context.Context, owner, name string, since } // Get file stats for this commit - additions, deletions, filesChanged, hasTests := r.getCommitStats(c, testPatterns) + stats := r.getCommitStats(c, testPatterns) // Extract login from email authorLogin := extractLoginFromEmail(c.Author.Email, c.Author.Name) @@ -261,13 +212,15 @@ func (r *Repository) FetchCommits(ctx context.Context, owner, name string, since Name: c.Committer.Name, Email: c.Committer.Email, }, - Date: commitTime, - Additions: additions, - Deletions: deletions, - FilesChanged: filesChanged, - Repository: fmt.Sprintf("%s/%s", owner, name), - URL: fmt.Sprintf("https://github.com/%s/%s/commit/%s", owner, name, c.Hash.String()), - HasTests: hasTests, + Date: commitTime, + Additions: stats.Additions, + Deletions: stats.Deletions, + MeaningfulAdditions: stats.MeaningfulAdditions, + MeaningfulDeletions: stats.MeaningfulDeletions, + FilesChanged: stats.FilesChanged, + Repository: fmt.Sprintf("%s/%s", owner, name), + URL: fmt.Sprintf("https://github.com/%s/%s/commit/%s", owner, name, c.Hash.String()), + HasTests: stats.HasTests, } commits = append(commits, commit) @@ -286,8 +239,20 @@ func (r *Repository) FetchCommits(ctx context.Context, owner, name string, since return commits, nil } +// commitStats holds the statistics for a commit +type commitStats struct { + Additions int + Deletions int + MeaningfulAdditions int + MeaningfulDeletions int + FilesChanged int + HasTests bool +} + // getCommitStats calculates additions, deletions, files changed for a commit -func (r *Repository) getCommitStats(c *object.Commit, testPatterns []string) (additions, deletions, filesChanged int, hasTests bool) { +func (r *Repository) getCommitStats(c *object.Commit, testPatterns []string) commitStats { + stats := commitStats{} + // Get parent commit for diff parentIter := c.Parents() parent, err := parentIter.Next() @@ -299,7 +264,7 @@ func (r *Repository) getCommitStats(c *object.Commit, testPatterns []string) (ad currentTree, err := c.Tree() if err != nil { - return 0, 0, 0, false + return stats } // Get changes between parent and current @@ -312,7 +277,7 @@ func (r *Repository) getCommitStats(c *object.Commit, testPatterns []string) (ad } if err != nil { - return 0, 0, 0, false + return stats } filesSet := make(map[string]bool) @@ -327,19 +292,19 @@ func (r *Repository) getCommitStats(c *object.Commit, testPatterns []string) (ad } // Skip documentation files - if isDocumentationFile(filePath) { + if diff.IsDocumentationFile(filePath) { continue } // Count unique files if !filesSet[filePath] { filesSet[filePath] = true - filesChanged++ + stats.FilesChanged++ // Check for test files for _, pattern := range testPatterns { if strings.Contains(filePath, pattern) { - hasTests = true + stats.HasTests = true break } } @@ -359,14 +324,16 @@ func (r *Repository) getCommitStats(c *object.Commit, testPatterns []string) (ad switch chunk.Type() { case 1: // Add for _, line := range lines { - if !isCommentLine(line) { - additions++ + stats.Additions++ + if diff.IsMeaningfulLine(line) { + stats.MeaningfulAdditions++ } } case 2: // Delete for _, line := range lines { - if !isCommentLine(line) { - deletions++ + stats.Deletions++ + if diff.IsMeaningfulLine(line) { + stats.MeaningfulDeletions++ } } } @@ -374,7 +341,7 @@ func (r *Repository) getCommitStats(c *object.Commit, testPatterns []string) (ad } } - return additions, deletions, filesChanged, hasTests + return stats } // extractLoginFromEmail tries to extract GitHub login from email diff --git a/internal/github/client.go b/internal/github/client.go index 34b7fd8..71a127c 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -13,6 +13,7 @@ import ( "github.com/google/go-github/v68/github" "github.com/lukaszraczylo/git-velocity/internal/config" + "github.com/lukaszraczylo/git-velocity/internal/diff" "github.com/lukaszraczylo/git-velocity/internal/domain/models" "github.com/lukaszraczylo/git-velocity/internal/github/cache" ) @@ -726,10 +727,14 @@ func convertCommit(c *github.RepositoryCommit, owner, repo string) models.Commit } filesChanged = len(c.Files) - // Detect if commit includes tests + // Detect if commit includes tests and calculate meaningful line counts hasTests := false + var meaningfulAdditions, meaningfulDeletions int + for _, f := range c.Files { filename := f.GetFilename() + + // Check for test files if strings.Contains(filename, "_test.go") || strings.Contains(filename, ".test.") || strings.Contains(filename, ".spec.") || @@ -737,7 +742,19 @@ func convertCommit(c *github.RepositoryCommit, owner, repo string) models.Commit strings.Contains(filename, "/test/") || strings.Contains(filename, "__tests__") { hasTests = true - break + } + + // Skip documentation files for meaningful line calculation + if diff.IsDocumentationFile(filename) { + continue + } + + // Analyze file patch to get meaningful line counts + patch := f.GetPatch() + if patch != "" { + stats := diff.AnalyzePatch(patch) + meaningfulAdditions += stats.MeaningfulAdditions + meaningfulDeletions += stats.MeaningfulDeletions } } @@ -747,17 +764,19 @@ func convertCommit(c *github.RepositoryCommit, owner, repo string) models.Commit } return models.Commit{ - SHA: c.GetSHA(), - Message: message, - Author: author, - Committer: committer, - Date: date, - Additions: additions, - Deletions: deletions, - FilesChanged: filesChanged, - Repository: fmt.Sprintf("%s/%s", owner, repo), - URL: c.GetHTMLURL(), - HasTests: hasTests, + SHA: c.GetSHA(), + Message: message, + Author: author, + Committer: committer, + Date: date, + Additions: additions, + Deletions: deletions, + MeaningfulAdditions: meaningfulAdditions, + MeaningfulDeletions: meaningfulDeletions, + FilesChanged: filesChanged, + Repository: fmt.Sprintf("%s/%s", owner, repo), + URL: c.GetHTMLURL(), + HasTests: hasTests, } }