diff --git a/README.md b/README.md index 1225420..9410aa5 100644 --- a/README.md +++ b/README.md @@ -229,6 +229,11 @@ gitcortex stats --input data.jsonl --stat churn-risk --top 0 --format csv # Activity by week gitcortex stats --input data.jsonl --stat activity --granularity week +# Test-to-source ratio (history-based proxy, not coverage) +gitcortex stats --input data.jsonl --stat tests +# Mark a non-standard test layout so it counts (repeatable) +gitcortex stats --input data.jsonl --stat tests --test-glob 'tools/testing/selftests/*' --test-glob '*_kunit.c' + # Filter to recent period gitcortex stats --since 7d # last 7 days gitcortex stats --since 3m --stat contributors # last 3 months @@ -259,6 +264,7 @@ Available stats: | `pareto` | Concentration (80% threshold) across files, devs (two lenses: commits and churn), and directories | | `structure` | Repo layout as a `tree(1)`-style view, dirs sorted by aggregate churn, capped by `--tree-depth` (default 3) | | `extensions` | File extensions ranked by recent churn, with file count, unique devs, and first/last-seen — the historical lens on language distribution | +| `tests` | History-based test-investment proxy: test-to-source ratio (files and churn) overall and per language, plus a ratio-over-time trend. Files are classified by path convention (not coverage); customize with `--test-glob` | Output formats: `table` (default, human-readable), `csv` (single clean table per `--stat`, header row on line 1), `json` (unified object with all sections). @@ -457,7 +463,7 @@ vendor/ Directory rules, globs, `**/foo`, and `!path` negations all work. Globbed negations like `!vendor*/keep` are honored — discovery descends into any dir where a negation rule could match a descendant. If `--ignore-file` is not set, scan looks for `.gitcortex-ignore` in the first `--root`. -**Consolidated profile report.** When `scan --email me@company.com --report path.html` runs against a multi-repo dataset, the profile report renders a *Per-Repository Breakdown* section: commits, churn, files, active days, and share-of-total — all filtered to that developer's contributions (files count reflects only files the dev touched). This is the one report that legitimately aggregates across repos; team-level views live in `--report-dir` (one HTML per repo, never mixed). +**Consolidated profile report.** When `scan --email me@company.com --report path.html` runs against a multi-repo dataset, the profile report renders a *Per-Repository Breakdown* section: commits, churn, files, active days, and share-of-total — all filtered to that developer's contributions (files count reflects only files the dev touched). The **Devs** column is the deliberate exception: it counts every author of the repo (repo-wide), so the dev can see how crowded each repo they work in is. This is the one report that legitimately aggregates across repos; team-level views live in `--report-dir` (one HTML per repo, never mixed). **Flags worth knowing:** @@ -510,9 +516,9 @@ gitcortex report --input data.jsonl --output report.html --top 30 gitcortex report --input data.jsonl --email alice@company.com --output alice.html ``` -Includes: summary cards, activity heatmap (with table toggle), top contributors, file hotspots, churn risk (with full-dataset label distribution strip above the truncated table), bus factor, file coupling, working patterns heatmap, top commits, developer network, and developer profiles. A collapsible glossary at the top defines the terms (bus factor, churn, fading-silo, specialization, etc.) for readers who are not already familiar. Typical size: 50-500KB depending on number of contributors. +Includes: summary cards, activity heatmap (with table toggle), top contributors, file hotspots, churn risk (with full-dataset label distribution strip above the truncated table), bus factor, file coupling, extensions, the tests section (test-to-source ratio, per-language breakdown, and ratio-over-time trend), working patterns heatmap, top commits, developer network, and developer profiles (each carrying a test-share figure). A collapsible glossary at the top defines the terms (bus factor, churn, fading-silo, specialization, etc.) for readers who are not already familiar. Typical size: 50-500KB depending on number of contributors. -When the input is multi-repo (from `gitcortex scan` or multiple `--input` files) AND `--email` is set, the profile report renders a *Per-Repository Breakdown* with commit/churn/files/active-days per repo, filtered to that developer's contributions. The team-view report intentionally omits this section — per-repo aggregates on a consolidated dataset reduce to raw git-history distribution, which is more usefully inspected via `manifest.json` or `stats --input X.jsonl` per repo. +When the input is multi-repo (from `gitcortex scan` or multiple `--input` files) AND `--email` is set, the profile report renders a *Per-Repository Breakdown* with commit/churn/files/active-days per repo, filtered to that developer's contributions (the Devs column is repo-wide, counting all of each repo's authors). The team-view report intentionally omits this section — per-repo aggregates on a consolidated dataset reduce to raw git-history distribution, which is more usefully inspected via `manifest.json` or `stats --input X.jsonl` per repo. > The HTML activity heatmap is always monthly (year × 12 months grid). For day/week/year buckets, use `gitcortex stats --stat activity --granularity `. diff --git a/cmd/gitcortex/main.go b/cmd/gitcortex/main.go index 256ec87..3361b56 100644 --- a/cmd/gitcortex/main.go +++ b/cmd/gitcortex/main.go @@ -107,11 +107,24 @@ func isValidGranularity(s string) bool { return false } +// testTrendGranularity maps the --granularity flag to the resolution the +// test-ratio trend can actually deliver. The per-file time series backing +// it (fileEntry.monthChurn) is monthly, so only "year" rolls up; "day" +// and "week" have no finer data and collapse to "month". Used purely for +// the section header so the label doesn't promise a resolution the data +// can't support. +func testTrendGranularity(g string) string { + if g == "year" { + return "year" + } + return "month" +} + func isValidStat(s string) bool { switch s { case "summary", "contributors", "hotspots", "directories", "extensions", "activity", "busfactor", "coupling", "churn-risk", "working-patterns", - "dev-network", "profile", "top-commits", "pareto", "structure": + "dev-network", "profile", "top-commits", "pareto", "structure", "tests": return true } return false @@ -132,6 +145,7 @@ type statsFlags struct { networkMinFiles int email string treeDepth int + testGlobs []string } func addStatsFlags(cmd *cobra.Command, sf *statsFlags) { @@ -139,7 +153,7 @@ func addStatsFlags(cmd *cobra.Command, sf *statsFlags) { cmd.Flags().StringVar(&sf.format, "format", "table", "Output format: table, csv, json") cmd.Flags().IntVar(&sf.topN, "top", 10, "Number of top entries to show (0 = all)") cmd.Flags().StringVar(&sf.granularity, "granularity", "month", "Activity granularity: day, week, month, year") - cmd.Flags().StringVar(&sf.stat, "stat", "", "Show a specific stat: summary, contributors, hotspots, directories, extensions, activity, busfactor, coupling, churn-risk, working-patterns, dev-network, profile, top-commits, pareto, structure") + cmd.Flags().StringVar(&sf.stat, "stat", "", "Show a specific stat: summary, contributors, hotspots, directories, extensions, activity, busfactor, coupling, churn-risk, working-patterns, dev-network, profile, top-commits, pareto, structure, tests") cmd.Flags().IntVar(&sf.couplingMaxFiles, "coupling-max-files", 50, "Max files per commit for coupling analysis") cmd.Flags().IntVar(&sf.couplingMinChanges, "coupling-min-changes", 5, "Min co-changes for coupling results") cmd.Flags().IntVar(&sf.churnHalfLife, "churn-half-life", 90, "Half-life in days for churn decay (churn-risk)") @@ -149,6 +163,7 @@ func addStatsFlags(cmd *cobra.Command, sf *statsFlags) { cmd.Flags().StringVar(&sf.from, "from", "", "Window start date YYYY-MM-DD, inclusive (pair with --to for closed window; leave --to empty for open-ended)") cmd.Flags().StringVar(&sf.to, "to", "", "Window end date YYYY-MM-DD, inclusive (pair with --from; leave --from empty for 'up to this date')") cmd.Flags().IntVar(&sf.treeDepth, "tree-depth", 3, "Max depth for --stat structure (0 = unlimited)") + cmd.Flags().StringSliceVar(&sf.testGlobs, "test-glob", nil, "Extra glob(s) marking files as tests for --stat tests (repeatable), e.g. testdata/*, *.itest.go") } func validateStatsFlags(sf *statsFlags) error { @@ -159,7 +174,7 @@ func validateStatsFlags(sf *statsFlags) error { return fmt.Errorf("invalid --granularity %q; must be one of: day, week, month, year", sf.granularity) } if sf.stat != "" && !isValidStat(sf.stat) { - return fmt.Errorf("invalid --stat %q; valid: summary, contributors, hotspots, directories, extensions, activity, busfactor, coupling, churn-risk, working-patterns, dev-network, profile, top-commits, pareto, structure", sf.stat) + return fmt.Errorf("invalid --stat %q; valid: summary, contributors, hotspots, directories, extensions, activity, busfactor, coupling, churn-risk, working-patterns, dev-network, profile, top-commits, pareto, structure, tests", sf.stat) } if sf.since != "" && (sf.from != "" || sf.to != "") { return fmt.Errorf("--since cannot be combined with --from/--to; pick one window spec") @@ -173,6 +188,9 @@ func validateStatsFlags(sf *statsFlags) error { if sf.from != "" && sf.to != "" && sf.from > sf.to { return fmt.Errorf("--from (%s) must be on or before --to (%s)", sf.from, sf.to) } + if err := stats.ValidateTestGlobs(sf.testGlobs); err != nil { + return err + } return nil } @@ -302,6 +320,23 @@ func renderStats(ds *stats.Dataset, sf *statsFlags) error { return err } } + if showAll || sf.stat == "tests" { + cfg := stats.NewTestConfig(sf.testGlobs) + fmt.Fprintln(os.Stderr, "\n=== Test Stats ===") + if err := f.PrintTestSummary(stats.ComputeTestSummary(ds, cfg, sf.topN)); err != nil { + return err + } + // The trend is a second table. CSV keeps a single-table contract + // (downstream parsers tail one header), so the time series renders + // only for the human table format; JSON consumers get it nested + // under "tests" via renderStatsJSON. + if sf.format != "csv" { + fmt.Fprintf(os.Stderr, "\n=== Test Ratio Trend (%s) ===\n", testTrendGranularity(sf.granularity)) + if err := f.PrintTestTrend(stats.TestRatioOverTime(ds, cfg, sf.granularity)); err != nil { + return err + } + } + } if showAll || sf.stat == "activity" { fmt.Fprintf(os.Stderr, "\n=== Activity (%s) ===\n", sf.granularity) if err := f.PrintActivity(stats.ActivityOverTime(ds, sf.granularity)); err != nil { @@ -344,7 +379,7 @@ func renderStats(ds *stats.Dataset, sf *statsFlags) error { label = sf.email } fmt.Fprintf(os.Stderr, "\n=== Profile: %s ===\n", label) - if err := f.PrintProfiles(stats.DevProfiles(ds, sf.email, 0)); err != nil { + if err := f.PrintProfiles(stats.DevProfiles(ds, sf.email, 0, stats.NewTestConfig(sf.testGlobs))); err != nil { return err } } @@ -403,6 +438,13 @@ func renderStatsJSON(f *stats.Formatter, ds *stats.Dataset, sf *statsFlags) erro if showAll || sf.stat == "extensions" { report["extensions"] = stats.ExtensionStats(ds, sf.topN) } + if showAll || sf.stat == "tests" { + cfg := stats.NewTestConfig(sf.testGlobs) + report["tests"] = map[string]interface{}{ + "summary": stats.ComputeTestSummary(ds, cfg, sf.topN), + "trend": stats.TestRatioOverTime(ds, cfg, sf.granularity), + } + } if showAll || sf.stat == "activity" { report["activity"] = stats.ActivityOverTime(ds, sf.granularity) } @@ -422,7 +464,7 @@ func renderStatsJSON(f *stats.Formatter, ds *stats.Dataset, sf *statsFlags) erro report["dev_network"] = stats.DeveloperNetwork(ds, sf.topN, sf.networkMinFiles) } if sf.stat == "profile" { - report["profiles"] = stats.DevProfiles(ds, sf.email, 0) + report["profiles"] = stats.DevProfiles(ds, sf.email, 0, stats.NewTestConfig(sf.testGlobs)) } if showAll || sf.stat == "pareto" { report["pareto"] = reportpkg.ComputePareto(ds) @@ -785,6 +827,7 @@ func reportCmd() *cobra.Command { couplingMinChanges int churnHalfLife int networkMinFiles int + testGlobs []string ) cmd := &cobra.Command{ @@ -808,6 +851,9 @@ func reportCmd() *cobra.Command { if from != "" && to != "" && from > to { return fmt.Errorf("--from (%s) must be on or before --to (%s)", from, to) } + if err := stats.ValidateTestGlobs(testGlobs); err != nil { + return err + } fromDate := from if since != "" { @@ -837,6 +883,7 @@ func reportCmd() *cobra.Command { sf := stats.StatsFlags{ CouplingMinChanges: couplingMinChanges, NetworkMinFiles: networkMinFiles, + TestGlobs: testGlobs, } repoName := strings.TrimSuffix(filepath.Base(input), filepath.Ext(input)) @@ -845,7 +892,7 @@ func reportCmd() *cobra.Command { } if email != "" { - if err := reportpkg.GenerateProfile(f, ds, repoName, email); err != nil { + if err := reportpkg.GenerateProfile(f, ds, repoName, email, stats.NewTestConfig(testGlobs)); err != nil { return fmt.Errorf("generate profile: %w", err) } fmt.Fprintf(os.Stderr, "Profile report for %s written to %s\n", email, fileURL(output)) @@ -870,6 +917,7 @@ func reportCmd() *cobra.Command { cmd.Flags().StringVar(&since, "since", "", "Filter to recent period (e.g. 7d, 4w, 3m, 1y)") cmd.Flags().StringVar(&from, "from", "", "Window start date YYYY-MM-DD, inclusive (pair with --to for closed window; leave --to empty for open-ended)") cmd.Flags().StringVar(&to, "to", "", "Window end date YYYY-MM-DD, inclusive (pair with --from; leave --from empty for 'up to this date')") + cmd.Flags().StringSliceVar(&testGlobs, "test-glob", nil, "Extra glob(s) marking files as tests in the Tests section (repeatable), e.g. testdata/*") return cmd } @@ -1146,6 +1194,7 @@ func scanCmd() *cobra.Command { couplingMinChanges int churnHalfLife int networkMinFiles int + testGlobs []string ) cmd := &cobra.Command{ @@ -1171,6 +1220,9 @@ breakdown — handy for showing aggregated work across many repos.`, if from != "" && to != "" && from > to { return fmt.Errorf("--from (%s) must be on or before --to (%s)", from, to) } + if err := stats.ValidateTestGlobs(testGlobs); err != nil { + return err + } // Report-flag combinations. --report is reserved for the // only case that genuinely consolidates signal across repos // — the per-developer profile; cross-repo team aggregation @@ -1250,7 +1302,7 @@ breakdown — handy for showing aggregated work across many repos.`, filepath.Join(result.OutputDir, "manifest.json")) } - sf := stats.StatsFlags{CouplingMinChanges: couplingMinChanges, NetworkMinFiles: networkMinFiles} + sf := stats.StatsFlags{CouplingMinChanges: couplingMinChanges, NetworkMinFiles: networkMinFiles, TestGlobs: testGlobs} loadOpts := stats.LoadOptions{ From: fromDate, To: to, @@ -1277,7 +1329,7 @@ breakdown — handy for showing aggregated work across many repos.`, defer f.Close() repoLabel := profileScanLabel(cfg.Roots) - if err := reportpkg.GenerateProfile(f, ds, repoLabel, email); err != nil { + if err := reportpkg.GenerateProfile(f, ds, repoLabel, email, stats.NewTestConfig(testGlobs)); err != nil { return fmt.Errorf("generate profile report: %w", err) } fmt.Fprintf(os.Stderr, "Profile report for %s written to %s\n", email, fileURL(reportPath)) @@ -1322,6 +1374,7 @@ breakdown — handy for showing aggregated work across many repos.`, cmd.Flags().IntVar(&couplingMinChanges, "coupling-min-changes", 5, "Min co-changes for coupling results (consolidated report)") cmd.Flags().IntVar(&churnHalfLife, "churn-half-life", 90, "Half-life in days for churn decay (consolidated report)") cmd.Flags().IntVar(&networkMinFiles, "network-min-files", 5, "Min shared files for dev-network edges (consolidated report)") + cmd.Flags().StringSliceVar(&testGlobs, "test-glob", nil, "Extra glob(s) marking files as tests in each repo's Tests section (repeatable), e.g. testdata/*") return cmd } diff --git a/docs/METRICS.md b/docs/METRICS.md index 9e199b0..c23fb05 100644 --- a/docs/METRICS.md +++ b/docs/METRICS.md @@ -217,7 +217,7 @@ Per-developer report combining multiple metrics. | Scope | Top 5 directories by unique file count, as % of the dev's **authored** files — i.e. files where the dev added or removed at least one line. Pure renames (file appears in the dev's change set with zero line changes) are excluded from both numerator and denominator so the visible Pct values sum to 100% (modulo the top-5 truncation). Same denominator is used for Extensions and for the Herfindahl specialization index, keeping the three consistent. **Multi-repo:** the `:` prefix that `LoadMultiJSONL` adds to avoid filename collisions is stripped before bucketing, so `cmd/` in three repos aggregates into one `cmd` bucket instead of fragmenting into `repoA:cmd`, `repoB:cmd`, `repoC:cmd`. Without the strip, Scope burns the top-5 slots on repo-×-dir pairs and Specialization deflates toward "generalist" for anyone whose area of work happens to exist in several repos. The per-repo split is still surfaced by the Per-Repository Breakdown section when a profile report is multi-repo. | | Extensions | Top 5 file extensions the dev touched, sorted by **files desc** (tiebreak churn desc, then ext asc) so the displayed `Pct` is monotonic with the sort order and HTML bar widths read correctly. `Pct` is `Files / authored * 100` where `authored` is the count of files the dev added or removed at least one line on — same denominator as Scope, so Pcts sum to 100% modulo top-5 truncation. The raw dev-attributable `Churn` (sum of `devLines[email]` across bucket files) is kept on the struct for JSON consumers who want a churn-ranked view. Answers the "language/skill fingerprint" question (`.go` + `.yaml` → backend+infra; `.tsx` + `.ts` + `.css` → frontend). **Attribution caveat:** bucket is derived from the file's canonical (post-rename) path — a dev who worked on `foo.js` pre-migration still shows up under `.ts` if it was later renamed; per-era per-dev attribution would need `byExt` to carry a dev dimension, which isn't tracked. | | Specialization | Herfindahl index over the **full** per-directory file-count distribution: Σ pᵢ² where pᵢ is the share of the dev's files in directory i. 1 = all files in one directory (narrow specialist); 1/N for a uniform spread across N directories; approaches 0 as the distribution widens. Computed before the top-5 Scope truncation so it reflects actual breadth. Labels (see `specBroadGeneralistMax`, `specBalancedMax`, `specFocusedMax` constants): `< 0.15` broad generalist, `< 0.35` balanced, `< 0.7` focused specialist, `≥ 0.7` narrow specialist. Herfindahl, not Gini, because Gini would collapse "1 file in 1 dir" and "1 file in each of 5 dirs" to the same value (both have zero inequality among buckets), which misses the specialization distinction. **Measures file distribution, not domain expertise** — see caveat below. **Display vs raw:** CLI and HTML show the value rounded to 3 decimals (`%.3f`) for readability; JSON output preserves the full float64. Band classification runs against the raw float, so a value like 0.149 lands in `broad generalist` even though %.2f would have rounded it to `0.15`. JSON consumers that reproduce the banding must use the raw value, not a rounded version. | -| Contribution type | Based on del/add ratio: growth (<0.4), balanced (0.4-0.8), refactor (>0.8) | +| Contribution type | Based on del/add ratio: growth (<0.4), balanced (0.4-0.8), refactor (≥0.8). A dev with deletions but zero additions (pure cleanup, del/add unbounded) is classified refactor. | | Collaborators | Top 5 devs sharing code with this dev. Ranked by `shared_lines` (Σ min(linesA, linesB) across shared files), tiebreak `shared_files`, then email. Same `shared_lines` semantics as the Developer Network metric — discounts trivial one-line touches so "collaborator" reflects real overlap. | | Top commits | The dev's top 10 commits by `lines_changed` (additions + deletions), tiebreak `sha asc`. Same ranking key and tiebreak as the dataset-level Top Commits section so the two read consistently side by side. Messages follow the same 80-character truncation rule and are only populated when `extract` ran with `--include-commit-messages`. Rendered in the CLI `profile` stat and in the standalone `--email` HTML profile page; intentionally omitted from the main report's Developer Profiles cards to keep those compact. **Divergence from dataset-level Top Commits:** commits with a zero `author_date` are dropped from the per-dev list (they share the guard that protects grid/monthly bucketing); the dataset-level section renders them as `0001-01-01`. Negligible in practice — the JSONL extract always emits `author_date` — but worth knowing if you compare the two views. | @@ -281,6 +281,31 @@ File extensions aggregated from `ds.files`, ranked by **recent churn** (decay-we **What it does not do**: no language-family grouping (`.js`+`.ts`+`.tsx` stay distinct). Aggregate downstream if you need "frontend vs backend"; the tool does not prescribe the taxonomy. Generated-file buckets (`.lock`, `.pb.go`, `.min.js`) will dominate unless filtered via `--ignore` at extract time — the suspect-paths warning flags these. +## Tests + +A **history-based proxy** for test investment (`--stat tests`, plus a section in the HTML report and a per-dev figure in profiles). It is *not* code coverage: gitcortex never reads file contents or runs a suite. Every tracked file is classified by **path convention** into one of three roles (`classifyTestRole`), and the ratios are computed from commit churn already in the dataset. + +**Roles** (each file resolves to exactly one, in this precedence): +1. **`--test-glob` override** — an explicit user glob wins over everything (including the suspect gate below), so golden-file suites under `testdata/*` or any path you name count as tests. +2. **other (suspect)** — vendor/generated paths (`vendor/`, `node_modules/`, `dist/`, `build/`, `*.lock`, `*.min.js`, `*.pb.go`, …) are excluded as noise, so a dependency's own `foo.test.js` doesn't inflate your ratio. Shares the suspect-paths table with the extract warning. +3. **test** — filename conventions that embed the language (`_test.go`, `_spec.rb`, `*Test.java`, `*_test.{cc,cpp,rs}`, …) fire regardless of location. Extension-agnostic conventions (`test_*` prefix, `*.test.*` / `*.spec.*` infix) and test directories (`test/ tests/ spec/ specs/ __tests__/ e2e/`) only count when the file is also code, so `api.spec.yaml` or `test/fixtures/data.json` stay in **other**. +4. **source** — any remaining file with a recognized code extension (`codeExtensions`). +5. **other** — everything else (docs, config, data, assets). + +**Ratios** are deliberately test-over-**source**, not test-over-total: the denominator is code a test could plausibly cover, so docs/config/vendor sit out of both numerator and denominator (reported only as an "excluded" count for transparency). A ratio of `0.50` means half as much test churn as production-code churn. + +**Fields / surfaces**: +- `test_files` / `source_files` and their `file_ratio` — counts of distinct files per role. +- `test_churn` / `source_churn` and their `churn_ratio` — lifetime additions + deletions per role, attributed **per era** (like Extensions' `byExt`): when a file is renamed across the test/source boundary (`src/foo.go → foo_test.go`, or a move into `tests/` or under `vendor/`), each era's churn is classified by the path it held at the time, so pre-rename production churn is not miscounted as test. A lineage is counted once per role it ever held. +- **By language** — the same split sliced by `extractExtension` of the **era** path, so `foo_test.go` and `bar.go` both land under `.go`, and a `foo.js → foo.ts` migration splits across `.js`/`.ts`; read each row as that language's test-to-source balance. Bounded by `--top`. +- **Trend** (`TestRatioOverTime`) — the churn ratio per period. Resolution is **monthly** (the only per-file time series retained is `fileEntry.monthChurn`); `--granularity year` rolls months up, and `day`/`week` fall back to month. The HTML report renders the **yearly** trend, which smooths the low-volume-month noise that makes a ratio spike when a month has only a line or two of source churn. +- **Per developer** (profiles) — `TestChurn` / `SourceChurn` split a dev's authored line churn by file role, **per rename era** (like the repo-level metric above), so a dev's pre-rename production edits on a file later renamed into a test aren't counted as test. The profile shows both the test:source ratio and a "% of code churn is tests" share. + +**Caveats**: +- **Convention-based, so false positives exist.** A real source package literally named `spec`/`specs` is read as tests; a real source dir named `build`/`dist` is read as suspect noise and drops out of the source denominator (inflating the ratio). Use `--ignore` at extract time or accept the bounded skew. +- **Trend vs summary may not reconcile** for commits with no date: the summary counts all churn, the trend only churn from dated commits (the same `monthChurn` basis the Activity stat uses). +- **`--test-glob`** matches via `path.Match` against the full repo-relative path and every trailing sub-path, so `testdata/*` matches at any depth and `*.itest.go` matches the basename; `**` is unsupported (express deep intent as a basename glob). Available on `stats`, `report`, and `scan`. + ## Repo Structure A `tree(1)`-style view of the repository's directory layout, built from paths seen in history (`FileHotspots`), not from the filesystem at HEAD. Deleted files are included — the view answers "what shaped the codebase", not "what is present today". @@ -309,7 +334,7 @@ One row per repo: | `% Churn` | share of total churn | | `Files` | distinct files the developer touched in this repo (always email-filtered, since the section only renders on profile reports) | | `Active days` | distinct UTC author-dates | -| `Devs` | unique author emails in this repo | +| `Devs` | unique author emails in this repo — **repo-wide**, counting ALL of the repo's authors, NOT filtered to the profiled dev (unlike Commits/Churn/Files/Active days above). Lets a profile show how crowded each repo the dev works in is. | | `First → Last` | earliest and latest author-date | **How the repo label is derived**: `LoadMultiJSONL` prefixes every path in the dataset with `:` (so `WordPress.git.jsonl` contributes paths like `WordPress.git:wp-includes/foo.php`). The breakdown groups by that prefix. If only a single JSONL is loaded, no prefix is emitted and the breakdown collapses to a single `(repo)` row the HTML report hides. diff --git a/internal/git/stream.go b/internal/git/stream.go index 4c21ab5..3a1f578 100644 --- a/internal/git/stream.go +++ b/internal/git/stream.go @@ -42,7 +42,17 @@ func NewLogStreamer(ctx context.Context, repo, branch, resumeSHA string, firstPa format = fmt.Sprintf("%%x00%%x01%%H%%x1f%%T%%x1f%%P%%x1f%s%%x1f%s%%x1f%%aI%%x1f%s%%x1f%s%%x1f%%cI%%x1f%%x1e%%x02", an, ae, cn, ce) } - args := []string{"-C", repo, "log", + args := []string{ + // core.quotePath=false stops git from C-quoting/octal-escaping + // non-ASCII bytes in pathnames (e.g. "caf\303\251.txt"). The raw + // and numstat parsers read paths verbatim, so without this a file + // with a UTF-8/accented name lands in every path-based stat with + // quotes and escapes baked in, and a rename's numstat key stops + // matching its raw PathNew — resolving that file's churn to 0. + // Control characters (tab/newline) are still quoted by git even + // with this off, so the tab-delimited parsing stays safe. + "-c", "core.quotePath=false", + "-C", repo, "log", "--raw", "--numstat", "-M", "--abbrev=40", fmt.Sprintf("--format=%s", format), diff --git a/internal/git/stream_test.go b/internal/git/stream_test.go index e63d513..ed11a77 100644 --- a/internal/git/stream_test.go +++ b/internal/git/stream_test.go @@ -1,44 +1,130 @@ package git import ( + "context" + "os" + "os/exec" + "path/filepath" + "strings" "testing" ) +// TestLogStreamerUnquotesUTF8Paths is a regression test for the +// core.quotePath=false flag on the git log invocation: without it git +// C-quotes/octal-escapes non-ASCII pathnames ("caf\303\251.txt"), which +// the raw/numstat parsers store verbatim — corrupting path-based stats and +// resolving renamed-unicode-file churn to 0. The path must arrive as clean +// UTF-8 in both the raw entries and the numstat map. +func TestLogStreamerUnquotesUTF8Paths(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not installed") + } + dir := t.TempDir() + // Isolate from ambient gitconfig: a global core.quotePath=false on the + // dev/CI box would mask a removal of the production flag and defeat this + // regression guard, so force git's default (quotePath=true). + env := append(os.Environ(), + "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null", + "GIT_AUTHOR_NAME=T", "GIT_AUTHOR_EMAIL=t@t.com", + "GIT_COMMITTER_NAME=T", "GIT_COMMITTER_EMAIL=t@t.com") + runGit := func(args ...string) { + t.Helper() + cmd := exec.Command("git", append([]string{"-C", dir}, args...)...) + cmd.Env = env + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + } + runGit("init", "-q") // no -b: branch="" → HEAD, and -b needs git ≥2.28 + if err := os.WriteFile(filepath.Join(dir, "café.txt"), []byte("a\nb\nc\n"), 0o644); err != nil { + t.Fatal(err) + } + runGit("add", "-A") + runGit("commit", "-qm", "add café") + // Rename the unicode file (with a small edit) — this is the most severe + // failure mode: when the path is C-quoted, the numstat rename key stops + // matching the raw PathNew and the file's churn resolves to 0. + runGit("mv", "café.txt", "résumé.txt") + if err := os.WriteFile(filepath.Join(dir, "résumé.txt"), []byte("a\nb\nc\nd\n"), 0o644); err != nil { + t.Fatal(err) + } + runGit("commit", "-qam", "rename to résumé") + + ls, err := NewLogStreamer(context.Background(), dir, "", "", false, false, false) + if err != nil { + t.Fatalf("NewLogStreamer: %v", err) + } + defer ls.Close() + + // Newest-first: the first block is the rename commit. + c, err := ls.Next() + if err != nil { + t.Fatalf("Next: %v", err) + } + if c == nil { + t.Fatal("no commit streamed") + } + + // No path may carry surrounding quotes or escape backslashes. + for _, r := range c.Raw { + for _, p := range []string{r.PathOld, r.PathNew} { + if strings.ContainsAny(p, "\"\\") { + t.Errorf("path is quoted/escaped: %q", p) + } + } + } + // The rename must be detected with clean old/new UTF-8 paths... + sawRename := false + for _, r := range c.Raw { + if strings.HasPrefix(r.Status, "R") && r.PathOld == "café.txt" && r.PathNew == "résumé.txt" { + sawRename = true + } + } + if !sawRename { + t.Errorf("clean unicode rename café.txt→résumé.txt not found in raw: %+v", c.Raw) + } + // ...and its new path must have a resolvable numstat entry (else the + // file's churn would silently resolve to 0). + if _, ok := c.Numstats["résumé.txt"]; !ok { + t.Errorf("numstat key \"résumé.txt\" missing — churn would resolve to 0; keys=%v", c.Numstats) + } +} + func TestParseRawLine(t *testing.T) { tests := []struct { - name string - line string - want RawEntry - wantOK bool + name string + line string + want RawEntry + wantOK bool }{ { - name: "modify", - line: ":100644 100644 aaaa bbbb M\tsrc/main.go", - want: RawEntry{Status: "M", OldHash: "aaaa", NewHash: "bbbb", PathOld: "src/main.go", PathNew: "src/main.go"}, + name: "modify", + line: ":100644 100644 aaaa bbbb M\tsrc/main.go", + want: RawEntry{Status: "M", OldHash: "aaaa", NewHash: "bbbb", PathOld: "src/main.go", PathNew: "src/main.go"}, wantOK: true, }, { - name: "add", - line: ":000000 100644 0000 abcd A\tnew_file.txt", - want: RawEntry{Status: "A", OldHash: "0000", NewHash: "abcd", PathOld: "new_file.txt", PathNew: "new_file.txt"}, + name: "add", + line: ":000000 100644 0000 abcd A\tnew_file.txt", + want: RawEntry{Status: "A", OldHash: "0000", NewHash: "abcd", PathOld: "new_file.txt", PathNew: "new_file.txt"}, wantOK: true, }, { - name: "delete", - line: ":100644 000000 abcd 0000 D\told_file.txt", - want: RawEntry{Status: "D", OldHash: "abcd", NewHash: "0000", PathOld: "old_file.txt", PathNew: ""}, + name: "delete", + line: ":100644 000000 abcd 0000 D\told_file.txt", + want: RawEntry{Status: "D", OldHash: "abcd", NewHash: "0000", PathOld: "old_file.txt", PathNew: ""}, wantOK: true, }, { - name: "rename", - line: ":100644 100644 aaaa bbbb R100\told.go\tnew.go", - want: RawEntry{Status: "R100", OldHash: "aaaa", NewHash: "bbbb", PathOld: "old.go", PathNew: "new.go"}, + name: "rename", + line: ":100644 100644 aaaa bbbb R100\told.go\tnew.go", + want: RawEntry{Status: "R100", OldHash: "aaaa", NewHash: "bbbb", PathOld: "old.go", PathNew: "new.go"}, wantOK: true, }, { - name: "copy", - line: ":100644 100644 aaaa bbbb C075\tsrc.go\tdst.go", - want: RawEntry{Status: "C075", OldHash: "aaaa", NewHash: "bbbb", PathOld: "src.go", PathNew: "dst.go"}, + name: "copy", + line: ":100644 100644 aaaa bbbb C075\tsrc.go\tdst.go", + want: RawEntry{Status: "C075", OldHash: "aaaa", NewHash: "bbbb", PathOld: "src.go", PathNew: "dst.go"}, wantOK: true, }, { diff --git a/internal/report/profile_template.go b/internal/report/profile_template.go index add5b7b..87c5a46 100644 --- a/internal/report/profile_template.go +++ b/internal/report/profile_template.go @@ -78,6 +78,9 @@ footer { margin-top: 40px; padding-top: 16px; border-top: 1px solid #d0d7de; col
Active Days
{{humanize .Profile.ActiveDays}}
Pace
{{printf "%.1f" .Profile.Pace}}
commits/active day
Weekend
{{printf "%.1f" .Profile.WeekendPct}}%
+ {{if or .Profile.TestChurn .Profile.SourceChurn}} +
Tests
{{testShare .Profile.TestChurn .Profile.SourceChurn}}%
of code churn{{if .Profile.SourceChurn}} · test:source {{printf "%.2f" .Profile.TestRatio}}{{end}}
+ {{end}}
diff --git a/internal/report/report.go b/internal/report/report.go index acd4603..c583103 100644 --- a/internal/report/report.go +++ b/internal/report/report.go @@ -27,6 +27,13 @@ type ReportData struct { Hotspots []stats.FileStat Directories []stats.DirStat Extensions []stats.ExtensionStat + TestSummary stats.TestSummary + // TestTrend is the test:source churn ratio by year — compact enough + // to render the whole history and robust to the low-volume-month + // noise that makes a monthly ratio spike (e.g. 2 source lines in a + // month → a meaningless ratio). nil/empty when the repo has no + // classifiable code. + TestTrend []stats.TestRatioBucket ActivityRaw []stats.ActivityBucket ActivityYears []string ActivityGrid [][]ActivityCell // [year][month 0-11] @@ -226,7 +233,13 @@ func ComputePareto(ds *stats.Dataset) ParetoData { } // Dirs: % of dirs for 80% of churn + // DirectoryStats returns dirs sorted by file-touches; the Pareto count + // is "fewest dirs reaching 80% of CHURN", so re-sort by churn desc + // first (mirrors the Files path above). Without this the cumulative + // loop walks file-touches order and over-counts TopChurnDirs, inflating + // DirsPct80Churn and biasing the label toward "well distributed". dirs := stats.DirectoryStats(ds, 0) + sort.Slice(dirs, func(i, j int) bool { return dirs[i].Churn > dirs[j].Churn }) var totalDirChurn int64 for _, d := range dirs { totalDirChurn += d.Churn @@ -339,6 +352,8 @@ func Generate(w io.Writer, ds *stats.Dataset, repoName string, topN int, sf stat actRaw := stats.ActivityOverTime(ds, "month") actYears, actGrid, maxActCommits := buildActivityGrid(actRaw) + testCfg := stats.NewTestConfig(sf.TestGlobs) + now := time.Now().Format("2006-01-02 15:04") // Compute label distribution for the Churn Risk chip strip without @@ -357,6 +372,8 @@ func Generate(w io.Writer, ds *stats.Dataset, repoName string, topN int, sf stat Hotspots: stats.FileHotspots(ds, topN), Directories: stats.DirectoryStats(ds, topN), Extensions: stats.ExtensionStats(ds, topN), + TestSummary: stats.ComputeTestSummary(ds, testCfg, topN), + TestTrend: stats.TestRatioOverTime(ds, testCfg, "year"), ActivityRaw: actRaw, ActivityYears: actYears, ActivityGrid: actGrid, @@ -368,7 +385,7 @@ func Generate(w io.Writer, ds *stats.Dataset, repoName string, topN int, sf stat Patterns: patterns, TopCommits: stats.TopCommits(ds, topN), DevNetwork: stats.DeveloperNetwork(ds, topN, sf.NetworkMinFiles), - Profiles: stats.DevProfiles(ds, "", topN), + Profiles: stats.DevProfiles(ds, "", topN, testCfg), Pareto: ComputePareto(ds), PatternGrid: grid, MaxPattern: maxP, @@ -496,6 +513,21 @@ var funcMap = template.FuncMap{ "humanize": humanize, "thousands": thousands, "docRef": docRef, + "testShare": testShare, +} + +// testShare renders test churn as a whole-percent of a dev's total +// (test+source) code churn — "38" meaning 38% of their code work is +// tests. Returns "0" when the dev touched no classifiable code, so the +// template never divides by zero. Complements the test:source ratio +// shown alongside it: the share answers "how much of their work" while +// the ratio answers "how it compares to the production code they wrote". +func testShare(test, source int64) string { + tot := test + source + if tot == 0 { + return "0" + } + return fmt.Sprintf("%.0f", float64(test)/float64(tot)*100) } // docRef returns an anchor link to the Churn Risk / Bus Factor / etc. @@ -650,8 +682,8 @@ type ProfileReportData struct { Repos []stats.RepoStat } -func GenerateProfile(w io.Writer, ds *stats.Dataset, repoName, email string) error { - profiles := stats.DevProfiles(ds, email, 0) +func GenerateProfile(w io.Writer, ds *stats.Dataset, repoName, email string, cfg ...stats.TestConfig) error { + profiles := stats.DevProfiles(ds, email, 0, cfg...) if len(profiles) == 0 { return fmt.Errorf("developer %s not found", email) } diff --git a/internal/report/report_test.go b/internal/report/report_test.go index e82d7ff..bad3f9f 100644 --- a/internal/report/report_test.go +++ b/internal/report/report_test.go @@ -122,13 +122,13 @@ func TestGenerate_TeamReportOmitsPerRepoBreakdown(t *testing.T) { // End-to-end for the `gitcortex scan --email me@x.com --report …` // flow. Covers three assertions at once: -// 1. GenerateProfile emits the Per-Repository Breakdown section -// when the dataset is multi-repo (gated on len(Repos) > 1). -// 2. Counts per repo are filtered to the dev — a commit by -// someone-else@x.com in alpha doesn't bleed into my profile's -// alpha row. -// 3. Files counted per repo are only files THIS dev touched — a -// colleague-exclusive file in alpha must not inflate my scope. +// 1. GenerateProfile emits the Per-Repository Breakdown section +// when the dataset is multi-repo (gated on len(Repos) > 1). +// 2. Counts per repo are filtered to the dev — a commit by +// someone-else@x.com in alpha doesn't bleed into my profile's +// alpha row. +// 3. Files counted per repo are only files THIS dev touched — a +// colleague-exclusive file in alpha must not inflate my scope. func TestGenerateProfile_MultiRepoBreakdownFiltersByEmail(t *testing.T) { dir := t.TempDir() alpha := filepath.Join(dir, "alpha.jsonl") @@ -465,6 +465,41 @@ func TestComputeParetoFilesAndDirsZeroChurn(t *testing.T) { } } +// Regression: the dirs Pareto must rank by CHURN, not by file-touches. +// dirA is touched in more commits (higher file-touches) but holds little +// churn; dirB holds ~95% of churn in one commit. The correct "fewest dirs +// for 80% of churn" answer is 1 (dirB alone). The old code walked +// DirectoryStats' file-touches order (dirA first) and reported 2. +func TestComputeParetoDirsRankByChurn(t *testing.T) { + jsonl := `{"type":"commit","sha":"1111111111111111111111111111111111111111","author_name":"A","author_email":"a@x","author_date":"2024-01-01T10:00:00Z","additions":205,"deletions":0,"files_changed":2} +{"type":"commit_file","commit":"1111111111111111111111111111111111111111","path_current":"dirA/x.go","path_previous":"dirA/x.go","status":"M","additions":5,"deletions":0} +{"type":"commit_file","commit":"1111111111111111111111111111111111111111","path_current":"dirB/y.go","path_previous":"dirB/y.go","status":"M","additions":200,"deletions":0} +{"type":"commit","sha":"2222222222222222222222222222222222222222","author_name":"A","author_email":"a@x","author_date":"2024-01-02T10:00:00Z","additions":5,"deletions":0,"files_changed":1} +{"type":"commit_file","commit":"2222222222222222222222222222222222222222","path_current":"dirA/x.go","path_previous":"dirA/x.go","status":"M","additions":5,"deletions":0} +` + dir := t.TempDir() + path := filepath.Join(dir, "dirs.jsonl") + if err := os.WriteFile(path, []byte(jsonl), 0644); err != nil { + t.Fatal(err) + } + ds, err := stats.LoadJSONL(path) + if err != nil { + t.Fatal(err) + } + p := ComputePareto(ds) + // dirA: file-touches 2, churn 10; dirB: file-touches 1, churn 200. + // Total churn 210, 80% = 168 → dirB alone (200) crosses it. + if p.TopChurnDirs != 1 { + t.Errorf("TopChurnDirs = %d, want 1 (dirB holds 95%% of churn); file-touches order would give 2", p.TopChurnDirs) + } + if p.TotalDirs != 2 { + t.Errorf("TotalDirs = %d, want 2", p.TotalDirs) + } + if p.DirsPct80Churn != 50.0 { + t.Errorf("DirsPct80Churn = %.1f, want 50.0 (1 of 2 dirs)", p.DirsPct80Churn) + } +} + func TestComputePareto(t *testing.T) { ds := loadFixture(t) p := ComputePareto(ds) diff --git a/internal/report/template.go b/internal/report/template.go index b6bf907..b684ef3 100644 --- a/internal/report/template.go +++ b/internal/report/template.go @@ -260,6 +260,56 @@ footer { margin-top: 40px; padding-top: 16px; border-top: 1px solid #d0d7de; col {{end}} +{{if or .TestSummary.SourceFiles .TestSummary.TestFiles}} +

Tests test:source churn {{if .TestSummary.SourceChurn}}{{printf "%.2f" .TestSummary.ChurnRatio}}{{else}}n/a{{end}}

+

A history-based proxy for test investment — files are classified by path convention (e.g. _test.go, *.spec.ts, tests/), not by measuring coverage. Ratios are test-over-source: docs, config, and vendored/generated files are excluded so the denominator is code a test could plausibly cover. · {{docRef "tests"}}

+ + + + + + +
MetricFilesChurn
Test{{thousands .TestSummary.TestFiles}}{{thousands .TestSummary.TestChurn}}
Source{{thousands .TestSummary.SourceFiles}}{{thousands .TestSummary.SourceChurn}}
Test : Source ratio{{if .TestSummary.SourceFiles}}{{printf "%.2f" .TestSummary.FileRatio}}{{else}}n/a{{end}}{{if .TestSummary.SourceChurn}}{{printf "%.2f" .TestSummary.ChurnRatio}}{{else}}n/a{{end}}
Excluded (docs/config/vendor){{thousands .TestSummary.OtherFiles}}
+ +{{if .TestSummary.ByLanguage}} +

By language

+ + +{{$maxChurnRatio := 0.0}}{{range .TestSummary.ByLanguage}}{{if gt .ChurnRatio $maxChurnRatio}}{{$maxChurnRatio = .ChurnRatio}}{{end}}{{end}} +{{range .TestSummary.ByLanguage}} + + + + + + + + + + +{{end}} +
LangTest FilesSrc FilesFile RatioTest ChurnSrc ChurnChurn Ratio
{{.Ext}}{{thousands .TestFiles}}{{thousands .SourceFiles}}{{if .SourceFiles}}{{printf "%.2f" .FileRatio}}{{else}}n/a{{end}}{{thousands .TestChurn}}{{thousands .SourceChurn}}{{if .SourceChurn}}{{printf "%.2f" .ChurnRatio}}{{else}}n/a{{end}}
+{{end}} + +{{if .TestTrend}} +

Test ratio over time (by year)

+

Is the codebase getting more or less tested? Yearly resolution smooths the low-volume-month noise that makes a monthly ratio spike.

+ + +{{$maxTrend := 0.0}}{{range .TestTrend}}{{if gt .Ratio $maxTrend}}{{$maxTrend = .Ratio}}{{end}}{{end}} +{{range .TestTrend}} + + + + + + + +{{end}} +
YearTest ChurnSrc ChurnChurn Ratio
{{.Period}}{{thousands .TestChurn}}{{thousands .SourceChurn}}{{if .SourceChurn}}{{printf "%.2f" .Ratio}}{{else}}n/a{{end}}
+{{end}} +{{end}} + {{if .ChurnRisk}}

Churn Risk{{if lt (len .ChurnRisk) .Summary.TotalFiles}} {{thousands (len .ChurnRisk)}} of {{thousands .Summary.TotalFiles}}{{end}}

Files ranked by recent churn. Label classifies context so you can judge action: fading-silo (old code + concentrated + declining) is the urgent alarm; silo suggests knowledge transfer; active-core is young code with a single author (often fine); active is shared healthy work; cold is quiet.{{if (index .ChurnRisk 0).AgePercentile}} Age P__ / Trend P__ under the label show where this file sits in the repo's distribution: age P90 means older than 90% of tracked files; trend P10 means declining more sharply than 90%. Classification boundaries are the P75 age and P25 trend of this dataset (see {{docRef "churn-risk"}}).{{end}}

@@ -388,6 +438,11 @@ footer { margin-top: 40px; padding-top: 16px; border-top: 1px solid #d0d7de; col {{range $i, $e := .Extensions}}{{if $i}}, {{end}}{{$e.Ext}} ({{printf "%.0f" $e.Pct}}%){{end}}{{if gt .ExtensionsHidden 0}} (+{{.ExtensionsHidden}} more){{end}} {{end}} + {{if or .TestChurn .SourceChurn}} + Tests + {{testShare .TestChurn .SourceChurn}}% of code churn (test:source {{if .SourceChurn}}{{printf "%.2f" .TestRatio}}{{else}}n/a{{end}} · {{thousands .TestChurn}} test / {{thousands .SourceChurn}} src) + {{end}} + Specialization {{printf "%.3f" .Specialization}} ({{if lt .Specialization 0.15}}broad generalist{{else if lt .Specialization 0.35}}balanced{{else if lt .Specialization 0.7}}focused specialist{{else}}narrow specialist{{end}}) diff --git a/internal/stats/extension_test.go b/internal/stats/extension_test.go index 6a5d974..8fc3590 100644 --- a/internal/stats/extension_test.go +++ b/internal/stats/extension_test.go @@ -43,8 +43,8 @@ func TestExtractExtensionPolicy(t *testing.T) { // nested paths already discard the prefix via the slash split. {"repo.v1:Makefile", "(none)"}, {"repo.v1:LICENSE", "(none)"}, - {"repo.v1:foo.go", ".go"}, // real ext still wins after prefix strip - {"repo:Makefile", "(none)"}, // stem with no dots — same rule + {"repo.v1:foo.go", ".go"}, // real ext still wins after prefix strip + {"repo:Makefile", "(none)"}, // stem with no dots — same rule {"repo.v1:.gitignore", ".gitignore"}, // dotfile survives prefix {"repo.v1:src/foo.go", ".go"}, // nested path: slash strips prefix first {"repo.v1:", "(none)"}, // prefix with empty basename @@ -101,8 +101,8 @@ func TestBusFactorCountExcludesEmptyDevLines(t *testing.T) { ds := &Dataset{ UniqueFileCount: 2, // Summary total; includes both files: map[string]*fileEntry{ - "src/authored.go": {devLines: map[string]int64{"alice@x": 10}}, - "src/pure-rename-only": {devLines: map[string]int64{}}, // no authored lines + "src/authored.go": {devLines: map[string]int64{"alice@x": 10}}, + "src/pure-rename-only": {devLines: map[string]int64{}}, // no authored lines }, } if got := BusFactorCount(ds); got != 1 { @@ -194,6 +194,32 @@ func TestExtensionStatsAggregation(t *testing.T) { // and zero to .js — an ugly skew in migration-heavy repos. The fix // uses fileEntry.byExt (populated at per-change time) to split the // lineage back across both buckets. +// Regression: ExtensionCount (the "M" in a "Top N of M" header) must count +// per-era extension buckets like ExtensionStats, not just canonical-path +// extensions. A foo.js → foo.ts migration yields TWO ExtensionStats rows, +// so the header total must also be 2 — otherwise it reads "Top N of 1" with +// two rows shown. +func TestExtensionCountHonorsPerEraSplit(t *testing.T) { + ds := &Dataset{ + files: map[string]*fileEntry{ + "foo.ts": { // canonical .ts, but lineage held .js then .ts + byExt: map[string]*extContribution{ + ".js": {churn: 1000}, + ".ts": {churn: 500}, + }, + }, + "bar.go": {additions: 10}, // byExt nil → canonical fallback (.go) + }, + } + if got := ExtensionCount(ds); got != 3 { + t.Errorf("ExtensionCount = %d, want 3 (.js + .ts + .go)", got) + } + // Must agree with the number of rows ExtensionStats actually produces. + if got, want := ExtensionCount(ds), len(ExtensionStats(ds, 0)); got != want { + t.Errorf("ExtensionCount (%d) != len(ExtensionStats) (%d)", got, want) + } +} + func TestExtensionStatsHonorsPerEraSplit(t *testing.T) { ds := &Dataset{ Latest: time.Date(2024, 6, 1, 0, 0, 0, 0, time.UTC), @@ -422,8 +448,8 @@ func TestDevProfileExtensions(t *testing.T) { "deploy/prod.yaml": {devLines: map[string]int64{"alice@x": 20}, devCommits: map[string]int{"alice@x": 1}, additions: 15, deletions: 5}, "Makefile": {devLines: map[string]int64{"alice@x": 5}, devCommits: map[string]int{"alice@x": 1}, additions: 5, deletions: 0}, }, - commits: map[string]*commitEntry{}, - workGrid: [7][24]int{}, + commits: map[string]*commitEntry{}, + workGrid: [7][24]int{}, } profiles := DevProfiles(ds, "alice@x", 0) @@ -612,12 +638,12 @@ func TestDevProfileExtensionsTruncationSum(t *testing.T) { "alice@x": {Email: "alice@x", Commits: 6, FilesTouched: 6, ActiveDays: 1}, }, files: map[string]*fileEntry{ - "a.go": {devLines: map[string]int64{"alice@x": 60}, devCommits: map[string]int{"alice@x": 1}}, - "b.py": {devLines: map[string]int64{"alice@x": 50}, devCommits: map[string]int{"alice@x": 1}}, - "c.rs": {devLines: map[string]int64{"alice@x": 40}, devCommits: map[string]int{"alice@x": 1}}, - "d.ts": {devLines: map[string]int64{"alice@x": 30}, devCommits: map[string]int{"alice@x": 1}}, - "e.md": {devLines: map[string]int64{"alice@x": 20}, devCommits: map[string]int{"alice@x": 1}}, - "f.sh": {devLines: map[string]int64{"alice@x": 10}, devCommits: map[string]int{"alice@x": 1}}, + "a.go": {devLines: map[string]int64{"alice@x": 60}, devCommits: map[string]int{"alice@x": 1}}, + "b.py": {devLines: map[string]int64{"alice@x": 50}, devCommits: map[string]int{"alice@x": 1}}, + "c.rs": {devLines: map[string]int64{"alice@x": 40}, devCommits: map[string]int{"alice@x": 1}}, + "d.ts": {devLines: map[string]int64{"alice@x": 30}, devCommits: map[string]int{"alice@x": 1}}, + "e.md": {devLines: map[string]int64{"alice@x": 20}, devCommits: map[string]int{"alice@x": 1}}, + "f.sh": {devLines: map[string]int64{"alice@x": 10}, devCommits: map[string]int{"alice@x": 1}}, }, commits: map[string]*commitEntry{}, workGrid: [7][24]int{}, @@ -844,9 +870,9 @@ func TestDevProfileExtensionsTopFive(t *testing.T) { func TestExtensionStatsTopN(t *testing.T) { ds := &Dataset{ files: map[string]*fileEntry{ - "a.go": {recentChurn: 100, devLines: map[string]int64{"a": 1}}, - "b.py": {recentChurn: 50, devLines: map[string]int64{"a": 1}}, - "c.rs": {recentChurn: 10, devLines: map[string]int64{"a": 1}}, + "a.go": {recentChurn: 100, devLines: map[string]int64{"a": 1}}, + "b.py": {recentChurn: 50, devLines: map[string]int64{"a": 1}}, + "c.rs": {recentChurn: 10, devLines: map[string]int64{"a": 1}}, }, } result := ExtensionStats(ds, 2) diff --git a/internal/stats/format.go b/internal/stats/format.go index 223b16a..ac997a7 100644 --- a/internal/stats/format.go +++ b/internal/stats/format.go @@ -202,6 +202,107 @@ func (f *Formatter) PrintExtensions(exts []ExtensionStat) error { } } +// ratioCell renders a test:source ratio for display. Returns "n/a" when +// the denominator is zero (no source files / no source churn) so the +// table doesn't show a misleading "0.00" that reads as "tests exist but +// cover nothing" when really there's nothing to cover. CSV gets an empty +// cell in that case (machine consumers prefer absent to a sentinel). +func ratioCell(num, den int64, csv bool) string { + if den == 0 { + if csv { + return "" + } + return "n/a" + } + return fmt.Sprintf("%.2f", float64(num)/float64(den)) +} + +func (f *Formatter) PrintTestSummary(s TestSummary) error { + switch f.format { + case "json": + return f.writeJSON(s) + case "csv": + rows := make([][]string, 0, len(s.ByLanguage)+1) + for _, l := range s.ByLanguage { + rows = append(rows, []string{ + l.Ext, + fmt.Sprintf("%d", l.TestFiles), + fmt.Sprintf("%d", l.SourceFiles), + ratioCell(int64(l.TestFiles), int64(l.SourceFiles), true), + fmt.Sprintf("%d", l.TestChurn), + fmt.Sprintf("%d", l.SourceChurn), + ratioCell(l.TestChurn, l.SourceChurn, true), + }) + } + // A "(total)" row mirrors the table footer so a piped CSV carries + // the overall ratio without the consumer having to re-aggregate. + rows = append(rows, []string{ + "(total)", + fmt.Sprintf("%d", s.TestFiles), + fmt.Sprintf("%d", s.SourceFiles), + ratioCell(int64(s.TestFiles), int64(s.SourceFiles), true), + fmt.Sprintf("%d", s.TestChurn), + fmt.Sprintf("%d", s.SourceChurn), + ratioCell(s.TestChurn, s.SourceChurn, true), + }) + return f.writeCSV([]string{"lang", "test_files", "source_files", "file_ratio", "test_churn", "source_churn", "churn_ratio"}, rows) + default: + tw := tabwriter.NewWriter(f.w, 0, 4, 2, ' ', 0) + fmt.Fprintf(tw, "Test files\t%d\n", s.TestFiles) + fmt.Fprintf(tw, "Source files\t%d\n", s.SourceFiles) + fmt.Fprintf(tw, "Test:source files\t%s\n", ratioCell(int64(s.TestFiles), int64(s.SourceFiles), false)) + fmt.Fprintf(tw, "Test churn\t%d\n", s.TestChurn) + fmt.Fprintf(tw, "Source churn\t%d\n", s.SourceChurn) + fmt.Fprintf(tw, "Test:source churn\t%s\n", ratioCell(s.TestChurn, s.SourceChurn, false)) + fmt.Fprintf(tw, "Excluded (docs/config/vendor)\t%d\n", s.OtherFiles) + if err := tw.Flush(); err != nil { + return err + } + if len(s.ByLanguage) == 0 { + return nil + } + fmt.Fprintln(f.w, "\nBy language:") + ltw := tabwriter.NewWriter(f.w, 0, 4, 2, ' ', 0) + fmt.Fprintf(ltw, "LANG\tTEST FILES\tSRC FILES\tFILE RATIO\tTEST CHURN\tSRC CHURN\tCHURN RATIO\n") + fmt.Fprintf(ltw, "----\t----------\t---------\t----------\t----------\t---------\t-----------\n") + for _, l := range s.ByLanguage { + fmt.Fprintf(ltw, "%s\t%d\t%d\t%s\t%d\t%d\t%s\n", + l.Ext, l.TestFiles, l.SourceFiles, + ratioCell(int64(l.TestFiles), int64(l.SourceFiles), false), + l.TestChurn, l.SourceChurn, + ratioCell(l.TestChurn, l.SourceChurn, false)) + } + return ltw.Flush() + } +} + +func (f *Formatter) PrintTestTrend(buckets []TestRatioBucket) error { + switch f.format { + case "json": + return f.writeJSON(buckets) + case "csv": + rows := make([][]string, len(buckets)) + for i, b := range buckets { + rows[i] = []string{ + b.Period, + fmt.Sprintf("%d", b.TestChurn), + fmt.Sprintf("%d", b.SourceChurn), + ratioCell(b.TestChurn, b.SourceChurn, true), + } + } + return f.writeCSV([]string{"period", "test_churn", "source_churn", "churn_ratio"}, rows) + default: + tw := tabwriter.NewWriter(f.w, 0, 4, 2, ' ', 0) + fmt.Fprintf(tw, "PERIOD\tTEST CHURN\tSRC CHURN\tCHURN RATIO\n") + fmt.Fprintf(tw, "------\t----------\t---------\t-----------\n") + for _, b := range buckets { + fmt.Fprintf(tw, "%s\t%d\t%d\t%s\n", + b.Period, b.TestChurn, b.SourceChurn, ratioCell(b.TestChurn, b.SourceChurn, false)) + } + return tw.Flush() + } +} + func (f *Formatter) PrintActivity(buckets []ActivityBucket) error { switch f.format { case "json": @@ -462,9 +563,12 @@ func (f *Formatter) PrintProfiles(profiles []DevProfile) error { fmt.Sprintf("%d", p.ActiveDays), fmt.Sprintf("%.1f", p.WeekendPct), p.FirstDate, p.LastDate, + fmt.Sprintf("%d", p.TestChurn), + fmt.Sprintf("%d", p.SourceChurn), + ratioCell(p.TestChurn, p.SourceChurn, true), } } - return f.writeCSV([]string{"name", "email", "commits", "lines_changed", "files_touched", "active_days", "weekend_pct", "first_date", "last_date"}, rows) + return f.writeCSV([]string{"name", "email", "commits", "lines_changed", "files_touched", "active_days", "weekend_pct", "first_date", "last_date", "test_churn", "source_churn", "test_ratio"}, rows) default: for i, p := range profiles { if i > 0 { @@ -497,6 +601,18 @@ func (f *Formatter) PrintProfiles(profiles []DevProfile) error { } fmt.Fprintln(f.w) } + if p.TestChurn > 0 || p.SourceChurn > 0 { + share := 0.0 + if tot := p.TestChurn + p.SourceChurn; tot > 0 { + share = float64(p.TestChurn) / float64(tot) * 100 + } + ratioStr := "n/a" + if p.SourceChurn > 0 { + ratioStr = fmt.Sprintf("%.2f", p.TestRatio) + } + fmt.Fprintf(f.w, " Tests: %.0f%% of code churn is tests (test:source %s — %d test / %d src)\n", + share, ratioStr, p.TestChurn, p.SourceChurn) + } // %.3f (not %.2f): labels are assigned at thresholds 0.15 / 0.35 // / 0.7 using the unrounded float. With %.2f a value like // 0.149 displays as "0.15" and the "broad generalist" label diff --git a/internal/stats/reader.go b/internal/stats/reader.go index 4162249..88107b3 100644 --- a/internal/stats/reader.go +++ b/internal/stats/reader.go @@ -11,6 +11,7 @@ import ( "sort" "strings" "time" + "unicode/utf8" "github.com/lex0c/gitcortex/internal/model" ) @@ -29,6 +30,28 @@ type commitEntry struct { repo string } +// maxStoredMessageBytes caps the commit message retained per commit. The +// only consumers (TopCommits, per-dev TopCommits) show the first line +// truncated to 77 chars; retaining full multi-line bodies for every +// commit adds hundreds of MB on large repos (linux: 1.44M commits) for +// bytes nothing renders. The bound keeps ample headroom over the 80-char +// display cap. +const maxStoredMessageBytes = 200 + +func truncateMessage(s string) string { + if len(s) <= maxStoredMessageBytes { + return s + } + // Back up to a rune boundary so the retained string is always valid + // UTF-8 (current display re-slices to ~77 chars, but keep it clean for + // any future full-message consumer). + b := maxStoredMessageBytes + for b > 0 && !utf8.RuneStart(s[b]) { + b-- + } + return s[:b] +} + type fileEntry struct { commits int additions int64 @@ -47,6 +70,22 @@ type fileEntry struct { // ExtensionStats. nil for hand-built fileEntries in tests — the // aggregator falls back to the canonical path's extension when so. byExt map[string]*extContribution + + // byPath splits this file's churn (total + per month + per dev) across + // the distinct paths it occupied over its lifetime, so a rename that + // crosses the test/source boundary (src/foo.go → foo_test.go, or a move + // into tests/) keeps per-era ROLE attribution correct after applyRenames + // merges the lineage onto one canonical path — otherwise pre-rename + // production churn would be counted as test (and the trend would mislabel + // old months). Built ONLY by applyRenames, and only for entries that + // actually merge (renamed lineages), via ensureEra snapshotting each + // pre-merge entry's own-era totals. It is deliberately NOT populated at + // ingest: doing so allocated a map per file (including the unrenamed + // majority and runs that never read test stats), spiking peak heap. + // nil for unrenamed files and hand-built fileEntries — the test stats + // fall back to the canonical path with the file totals, exact for a + // single era. Consumed only by the test-stat functions. + byPath map[string]*pathEra } type extContribution struct { @@ -56,6 +95,37 @@ type extContribution struct { lastChange time.Time } +// pathEra is one path's contribution to a file lineage (see byPath): the +// churn attributed while the file lived at that path, the per-month +// breakdown the test-ratio trend needs to place each era in time, and the +// per-developer churn for that era so the dev profile can split a +// cross-boundary rename's churn by role per dev. All three are snapshotted +// by ensureEra (from applyRenames) for renamed lineages only; an un-merged +// (single-era) file has no pathEra and uses the canonical fallback, which +// is exact for one era. +type pathEra struct { + churn int64 + monthChurn map[string]int64 // "YYYY-MM" → churn + devChurn map[string]int64 // email → churn, captured at rename merge +} + +// eachEra invokes f once per (path, churn, monthChurn) era of the file so +// callers can attribute role per era. A renamed lineage iterates its +// byPath entries (the distinct pre-merge paths it held); an unrenamed file +// (byPath dropped in finalizeDataset) yields a single era for its +// canonical path with the file totals. canonical is the file's ds.files +// key. Used by the test-stat functions to keep a cross-boundary rename +// from mislabeling pre-rename history. +func (fe *fileEntry) eachEra(canonical string, f func(path string, churn int64, monthChurn map[string]int64)) { + if len(fe.byPath) == 0 { + f(canonical, fe.additions+fe.deletions, fe.monthChurn) + return + } + for p, pe := range fe.byPath { + f(p, pe.churn, pe.monthChurn) + } +} + type filePair struct{ a, b string } type renameEdge struct { @@ -204,6 +274,17 @@ func streamLoadInto(ds *Dataset, r io.Reader, opt LoadOptions, pathPrefix string var coupCurrentFiles []string var coupCurrentChurn int64 + // Per-commit decay weight + month key, recomputed only when the commit + // changes. A commit's file lines are emitted contiguously (no other + // commit record between them — extract.emitCommit writes the block + // atomically), so ds.Latest is constant across them and this collapses + // to one math.Exp + time.Format per commit instead of one per file. + // Kept loop-local (not on commitEntry) so the scratch isn't retained + // for the whole Dataset lifetime. + var weightSHA string + var commitWeight float64 + var commitMonthKey string + dayIndex := map[time.Weekday]int{ time.Monday: 0, time.Tuesday: 1, time.Wednesday: 2, time.Thursday: 3, time.Friday: 4, time.Saturday: 5, time.Sunday: 6, @@ -238,10 +319,17 @@ func streamLoadInto(ds *Dataset, r io.Reader, opt LoadOptions, pathPrefix string t := parseDate(c.AuthorDate) if hasFilter { - if !fromTime.IsZero() && !t.IsZero() && t.Before(fromTime) { + // A commit with no parseable date can't be placed inside a + // bounded window — exclude it (and, via commitInRange, its + // file/parent records) rather than letting the IsZero guard + // fall through and count it in every --since/--from/--to run. + if t.IsZero() { continue } - if !toTime.IsZero() && !t.IsZero() && t.After(toTime) { + if !fromTime.IsZero() && t.Before(fromTime) { + continue + } + if !toTime.IsZero() && t.After(toTime) { continue } commitInRange[c.SHA] = struct{}{} @@ -258,7 +346,7 @@ func streamLoadInto(ds *Dataset, r io.Reader, opt LoadOptions, pathPrefix string add: c.Additions, del: c.Deletions, files: c.FilesChanged, - message: c.Message, + message: truncateMessage(c.Message), repo: strings.TrimSuffix(pathPrefix, ":"), } ds.commits[c.SHA] = entry @@ -393,6 +481,14 @@ func streamLoadInto(ds *Dataset, r io.Reader, opt LoadOptions, pathPrefix string } ec.churn += cf.Additions + cf.Deletions + // byPath (the per-era test-stats split) is NOT built here: doing + // so would allocate a map + nested month map for EVERY file — + // including the unrenamed majority and runs that never touch test + // stats — spiking peak heap during load. Instead applyRenames + // builds it only for renamed lineages, from these same per-era + // fileEntries before they merge. Unrenamed files keep byPath nil + // and the test stats fall back to the canonical path. + cm := ds.commits[cf.Commit] if cm != nil { // Only record a devLines entry when the change actually @@ -417,10 +513,17 @@ func streamLoadInto(ds *Dataset, r io.Reader, opt LoadOptions, pathPrefix string ds.contribFiles[cm.email][path] = struct{}{} if !cm.date.IsZero() { - days := ds.Latest.Sub(cm.date).Hours() / 24 - weight := math.Exp(-lambda * days) - fe.recentChurn += float64(cf.Additions+cf.Deletions) * weight - ec.recentChurn += float64(cf.Additions+cf.Deletions) * weight + // Recompute the decay weight + month key only when the + // commit changes (see weightSHA decl): collapses to one + // math.Exp + time.Format per commit instead of per file. + if cf.Commit != weightSHA { + days := ds.Latest.Sub(cm.date).Hours() / 24 + commitWeight = math.Exp(-lambda * days) + commitMonthKey = cm.date.UTC().Format("2006-01") + weightSHA = cf.Commit + } + fe.recentChurn += float64(cf.Additions+cf.Deletions) * commitWeight + ec.recentChurn += float64(cf.Additions+cf.Deletions) * commitWeight if cm.date.After(fe.lastChange) { fe.lastChange = cm.date } @@ -433,7 +536,7 @@ func streamLoadInto(ds *Dataset, r io.Reader, opt LoadOptions, pathPrefix string if ec.firstChange.IsZero() || cm.date.Before(ec.firstChange) { ec.firstChange = cm.date } - fe.monthChurn[cm.date.UTC().Format("2006-01")] += cf.Additions + cf.Deletions + fe.monthChurn[commitMonthKey] += cf.Additions + cf.Deletions } } @@ -595,14 +698,22 @@ func applyRenames(ds *Dataset) { } } - // Re-key file entries, merging colliders. + // Re-key file entries, merging colliders. Only here — for entries that + // actually merge (renamed lineages) — do we build byPath, snapshotting + // each era's totals before they're summed. newFilePath remembers the + // original path of each survivor so its own era can be captured under + // that key on the first merge into it. newFiles := make(map[string]*fileEntry, len(ds.files)) + newFilePath := make(map[string]string, len(ds.files)) for path, fe := range ds.files { c := canonical(path) if existing, ok := newFiles[c]; ok { + ensureEra(existing, newFilePath[c]) // survivor's own era (once, pre-sum) + ensureEra(fe, path) // this era being merged in mergeFileEntry(existing, fe) } else { newFiles[c] = fe + newFilePath[c] = path } } ds.files = newFiles @@ -641,9 +752,46 @@ func applyRenames(ds *Dataset) { } } +// ensureEra records fe's current totals as the path-era for `path` in +// fe.byPath, so a renamed lineage keeps its per-era churn/month/dev split. +// applyRenames calls it for both the surviving entry (under its original +// path) and each entry merged into it, BEFORE the merge sums fe's totals — +// so the snapshot is that era's own contribution, not a running sum. The +// presence of fe.byPath[path] marks it captured: a second merge into the +// same survivor skips re-capturing its (now summed) totals. Maps are copied +// because fe's are about to be mutated/discarded. Called only for renamed +// lineages, so the per-era cost is paid only where it changes the answer — +// the unrenamed majority never allocate byPath. +func ensureEra(fe *fileEntry, path string) { + if fe.byPath == nil { + fe.byPath = make(map[string]*pathEra) + } + if _, ok := fe.byPath[path]; ok { + return + } + pe := &pathEra{churn: fe.additions + fe.deletions} + if len(fe.monthChurn) > 0 { + pe.monthChurn = make(map[string]int64, len(fe.monthChurn)) + for m, c := range fe.monthChurn { + pe.monthChurn[m] = c + } + } + if len(fe.devLines) > 0 { + pe.devChurn = make(map[string]int64, len(fe.devLines)) + for email, n := range fe.devLines { + pe.devChurn[email] = n + } + } + fe.byPath[path] = pe +} + // mergeFileEntry folds src into dst: sums scalars, unions maps, keeps the // widest firstChange→lastChange span. + func mergeFileEntry(dst, src *fileEntry) { + // byPath eras for both sides are snapshotted by ensureEra (in + // applyRenames) before this call, so the union below just transfers + // src's eras onto dst. dst.commits += src.commits dst.additions += src.additions dst.deletions += src.deletions @@ -701,6 +849,39 @@ func mergeFileEntry(dst, src *fileEntry) { } } } + + // byPath: union per-path eras. A rename gives src and dst distinct path + // keys, so this is normally a pointer transfer; the same-key branch + // (a path reused across a merged lineage) sums defensively. + if src.byPath != nil { + if dst.byPath == nil { + dst.byPath = make(map[string]*pathEra, len(src.byPath)) + } + for p, srcPE := range src.byPath { + if dstPE, ok := dst.byPath[p]; ok { + dstPE.churn += srcPE.churn + if dstPE.monthChurn == nil { + dstPE.monthChurn = make(map[string]int64, len(srcPE.monthChurn)) + } + for m, c := range srcPE.monthChurn { + dstPE.monthChurn[m] += c + } + // Both sides' devChurn were snapshotted by captureEraDevChurn + // above; sum them so a path reused across merged lineages + // keeps every era's per-dev churn. + if srcPE.devChurn != nil { + if dstPE.devChurn == nil { + dstPE.devChurn = make(map[string]int64, len(srcPE.devChurn)) + } + for e, c := range srcPE.devChurn { + dstPE.devChurn[e] += c + } + } + } else { + dst.byPath[p] = srcPE + } + } + } } // isMechanicalRefactor returns true when a commit's shape matches a likely diff --git a/internal/stats/reader_test.go b/internal/stats/reader_test.go new file mode 100644 index 0000000..0c4de17 --- /dev/null +++ b/internal/stats/reader_test.go @@ -0,0 +1,59 @@ +package stats + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestTruncateMessage(t *testing.T) { + short := "fix: one-line subject" + if got := truncateMessage(short); got != short { + t.Errorf("short message altered: %q", got) + } + long := strings.Repeat("x", maxStoredMessageBytes+50) + if got := truncateMessage(long); len(got) != maxStoredMessageBytes { + t.Errorf("len = %d, want %d", len(got), maxStoredMessageBytes) + } + exact := strings.Repeat("y", maxStoredMessageBytes) + if got := truncateMessage(exact); got != exact { + t.Errorf("at-bound message altered (len %d)", len(got)) + } +} + +// A commit whose author date does not parse must NOT be counted inside a +// bounded --from/--to (or --since) window — it can't be placed in time, so +// it should be excluded rather than slip past the zero-time guard into +// every window. Regression for reader.go's window filter. +func TestWindowExcludesUndatedCommits(t *testing.T) { + dated := `{"type":"commit","sha":"aaa","author_name":"A","author_email":"a@x","author_date":"2026-03-15T10:00:00Z","additions":10,"deletions":2,"files_changed":1}` + undated := `{"type":"commit","sha":"bbb","author_name":"B","author_email":"b@x","author_date":"","additions":99,"deletions":7,"files_changed":1}` + + dir := t.TempDir() + path := filepath.Join(dir, "git_data.jsonl") + if err := os.WriteFile(path, []byte(dated+"\n"+undated+"\n"), 0o644); err != nil { + t.Fatal(err) + } + + // No window: both commits load. + all, err := LoadJSONL(path) + if err != nil { + t.Fatal(err) + } + if all.CommitCount != 2 { + t.Fatalf("unfiltered CommitCount = %d, want 2", all.CommitCount) + } + + // Bounded window containing the dated commit: the undated one is dropped. + win, err := LoadJSONL(path, LoadOptions{From: "2026-03-01", To: "2026-03-31"}) + if err != nil { + t.Fatal(err) + } + if win.CommitCount != 1 { + t.Errorf("windowed CommitCount = %d, want 1 (undated commit excluded)", win.CommitCount) + } + if win.TotalAdditions != 10 { + t.Errorf("windowed TotalAdditions = %d, want 10 (undated 99 must not leak in)", win.TotalAdditions) + } +} diff --git a/internal/stats/repo_breakdown.go b/internal/stats/repo_breakdown.go index 57822e8..3db0526 100644 --- a/internal/stats/repo_breakdown.go +++ b/internal/stats/repo_breakdown.go @@ -18,6 +18,10 @@ type RepoStat struct { Churn int64 Files int ActiveDays int + // UniqueDevs is repo-wide: all authors who committed to the repo, not + // just the email-filtered dev — so the profile lens shows how crowded + // each of the dev's repos is. (Commits/Churn/Files/ActiveDays above + // ARE email-filtered when a filter is set.) UniqueDevs int FirstCommitDate string LastCommitDate string @@ -53,31 +57,40 @@ func RepoBreakdown(ds *Dataset, emailFilter string) []RepoStat { commits int add, del int64 days map[string]struct{} - devs map[string]struct{} first, last time.Time } repos := make(map[string]*acc) + // repoDevs is the REPO-WIDE author set per repo, recorded before the + // email filter so UniqueDevs answers "how many people work in this + // repo" even in the email-filtered profile lens. Accumulating it + // inside the filtered path (as before) collapsed the column to a + // constant 1 — the filtered dev was the only author ever recorded. + repoDevs := make(map[string]map[string]struct{}) emailLower := strings.ToLower(strings.TrimSpace(emailFilter)) visit := func(key string, c *commitEntry) { + if c.email != "" { + dv := repoDevs[key] + if dv == nil { + dv = make(map[string]struct{}) + repoDevs[key] = dv + } + dv[strings.ToLower(c.email)] = struct{}{} + } + // Commits/churn/active-days/dates below are the filtered dev's + // slice of the repo; only UniqueDevs (repoDevs) is repo-wide. if emailLower != "" && strings.ToLower(strings.TrimSpace(c.email)) != emailLower { return } a, ok := repos[key] if !ok { - a = &acc{ - days: make(map[string]struct{}), - devs: make(map[string]struct{}), - } + a = &acc{days: make(map[string]struct{})} repos[key] = a } a.commits++ a.add += c.add a.del += c.del - if c.email != "" { - a.devs[strings.ToLower(c.email)] = struct{}{} - } if !c.date.IsZero() { a.days[c.date.UTC().Format("2006-01-02")] = struct{}{} if a.first.IsZero() || c.date.Before(a.first) { @@ -161,7 +174,7 @@ func RepoBreakdown(ds *Dataset, emailFilter string) []RepoStat { Churn: a.add + a.del, Files: fileCounts[repo], ActiveDays: len(a.days), - UniqueDevs: len(a.devs), + UniqueDevs: len(repoDevs[repo]), } if !a.first.IsZero() { stat.FirstCommitDate = a.first.UTC().Format("2006-01-02") diff --git a/internal/stats/repo_breakdown_test.go b/internal/stats/repo_breakdown_test.go index 404a563..a21ac5a 100644 --- a/internal/stats/repo_breakdown_test.go +++ b/internal/stats/repo_breakdown_test.go @@ -61,6 +61,7 @@ func TestRepoBreakdown_EmailFilter(t *testing.T) { if len(breakdown) != 2 { t.Fatalf("want 2 repos for me@x.com, got %d", len(breakdown)) } + devsByRepo := map[string]int{} for _, b := range breakdown { if b.Commits != 1 { t.Errorf("repo %s: expected 1 commit (filtered), got %d", b.Repo, b.Commits) @@ -68,6 +69,16 @@ func TestRepoBreakdown_EmailFilter(t *testing.T) { if b.Files != 1 { t.Errorf("repo %s: expected 1 file (filtered), got %d", b.Repo, b.Files) } + devsByRepo[b.Repo] = b.UniqueDevs + } + // UniqueDevs is repo-wide even under the email filter: alpha has two + // authors (me@ + other@), beta has one. A regression to the filtered + // accumulation would report 1 for both. + if devsByRepo["alpha"] != 2 { + t.Errorf("alpha UniqueDevs = %d, want 2 (repo-wide: me@ + other@)", devsByRepo["alpha"]) + } + if devsByRepo["beta"] != 1 { + t.Errorf("beta UniqueDevs = %d, want 1", devsByRepo["beta"]) } } diff --git a/internal/stats/stats.go b/internal/stats/stats.go index f8fe1a6..300c038 100644 --- a/internal/stats/stats.go +++ b/internal/stats/stats.go @@ -184,6 +184,17 @@ func herfindahl(values []int) float64 { if len(values) == 0 { return 0 } + // Sort ascending so the floating-point Σpᵢ² below accumulates in a + // canonical order. Callers build `values` by ranging a map (e.g. a + // per-directory file-count map), whose iteration order is randomized; + // because float addition is non-associative, that made the result's + // low-order bits differ run-to-run (visible as jitter in the JSON + // Specialization field, though the %.3f display and label bands were + // unaffected). Ascending order also minimizes rounding error. The + // integer sum is order-independent; only the float loop needs this. + // Sorting in place is safe — `values` is a throwaway slice the caller + // does not reuse. + sort.Ints(values) var sum int64 for _, v := range values { if v < 0 { @@ -209,6 +220,10 @@ func herfindahl(values []int) float64 { type StatsFlags struct { CouplingMinChanges int NetworkMinFiles int + // TestGlobs are extra --test-glob patterns forwarded to the test-stat + // classifier so the HTML report's test section matches what + // `stats --stat tests --test-glob ...` would show on the CLI. + TestGlobs []string } // --- Stats from pre-aggregated Dataset --- @@ -407,13 +422,24 @@ func DirectoryCount(ds *Dataset) int { return len(dirs) } -// ExtensionCount returns the total number of distinct extension -// buckets ExtensionStats would produce. Same derivation via -// extractExtension so the count matches what ranking would show. +// ExtensionCount returns the total number of distinct extension buckets +// ExtensionStats would produce — the "M" in a "Top N of M" header. It must +// count buckets the SAME way ExtensionStats does: per-era via fileEntry.byExt +// (so a rename across extensions, foo.js → foo.ts, contributes BOTH ".js" +// and ".ts"), falling back to the canonical path's extension only when byExt +// is nil (hand-built fileEntries). Counting just canonical-path extensions +// would under-count M on migration-heavy repos and make the header read +// "Top N of M" with M smaller than the rows actually shown. func ExtensionCount(ds *Dataset) int { exts := make(map[string]struct{}) - for path := range ds.files { - exts[extractExtension(path)] = struct{}{} + for path, fe := range ds.files { + if fe.byExt == nil { + exts[extractExtension(path)] = struct{}{} + continue + } + for ext := range fe.byExt { + exts[ext] = struct{}{} + } } return len(exts) } @@ -1344,6 +1370,16 @@ type DevProfile struct { ScopeHidden int Extensions []DevExtContrib ExtensionsHidden int + // TestChurn / SourceChurn split this dev's authored line churn by the + // role of the file touched (test vs production code); TestRatio is + // their test:source — "how much of this person's work is test code", + // read the same way as the repo-level Tests section. Files that are + // neither (docs/config/vendor/generated) are excluded from both. + // TestRatio is 0 when the dev authored no source churn (guarded + // division), same convention as TestSummary. + TestChurn int64 + SourceChurn int64 + TestRatio float64 Specialization float64 // Gini over dir file-count distribution: 0 = broad generalist, 1 = single-dir specialist ContribRatio float64 // del/add — 0=growth, ~1=rewrite, >1=cleanup ContribType string // "growth", "balanced", "refactor" @@ -1425,7 +1461,17 @@ type DevExtContrib struct { // the pre-optimization behaviour — the CLI stats command wants this). // filterEmail is the stronger filter: when set, only that one profile // is built and n is ignored. -func DevProfiles(ds *Dataset, filterEmail string, n int) []DevProfile { +func DevProfiles(ds *Dataset, filterEmail string, n int, cfg ...TestConfig) []DevProfile { + // Test-detection config is optional (variadic) so the ~25 existing + // callers — almost all tests — need no change; the CLI and HTML + // report pass a config built from --test-glob so the per-dev test + // ratio matches what the Tests section shows. Mirrors the variadic + // LoadOptions convention on LoadJSONL. + testCfg := NewTestConfig(nil) + if len(cfg) > 0 { + testCfg = cfg[0] + } + // Determine the target set of emails. Three modes: // 1. filterEmail != "" → single dev, n is irrelevant // 2. n > 0 → top-N by commits (desc), email asc tiebreak @@ -1788,6 +1834,7 @@ func DevProfiles(ds *Dataset, filterEmail string, n int) []DevProfile { churn int64 } extCount := make(map[string]*extAccForDev) + var devTestChurn, devSourceChurn int64 if files, ok := devFiles[email]; ok { for path, fa := range files { ext := extractExtension(path) @@ -1798,6 +1845,36 @@ func DevProfiles(ds *Dataset, filterEmail string, n int) []DevProfile { } acc.files++ acc.churn += fa.churn + // Split the dev's authored churn by role. When the lineage + // was renamed across the test/source boundary, devLines (and + // thus fa.churn) is merged under the canonical path, so + // classify PER ERA using byPath's per-dev churn — mirroring + // ComputeTestSummary's eachEra — to keep pre-rename + // production churn from being reported as test. Unrenamed + // files (byPath nil) classify the canonical path with + // fa.churn, which is exact for a single era. RoleOther + // contributes to neither. + if fe := ds.files[path]; fe != nil && len(fe.byPath) > 0 { + for eraPath, pe := range fe.byPath { + c := pe.devChurn[email] + if c == 0 { + continue + } + switch classifyTestRole(eraPath, testCfg) { + case RoleTest: + devTestChurn += c + case RoleSource: + devSourceChurn += c + } + } + } else { + switch classifyTestRole(path, testCfg) { + case RoleTest: + devTestChurn += fa.churn + case RoleSource: + devSourceChurn += fa.churn + } + } } } var extensions []DevExtContrib @@ -1840,11 +1917,17 @@ func DevProfiles(ds *Dataset, filterEmail string, n int) []DevProfile { contribType := "growth" if cs.Additions > 0 { contribRatio = math.Round(float64(cs.Deletions)/float64(cs.Additions)*100) / 100 - } - if contribRatio >= contribRefactorRatio { + if contribRatio >= contribRefactorRatio { + contribType = "refactor" + } else if contribRatio >= contribBalancedRatio { + contribType = "balanced" + } + } else if cs.Deletions > 0 { + // Pure deletions (no additions) — del/add is unbounded, which is + // the strongest cleanup signal, not "growth". Leave contribRatio + // at 0 (an unbounded float can't round-trip through JSON) but + // classify as refactor so the label isn't the opposite of reality. contribType = "refactor" - } else if contribRatio >= contribBalancedRatio { - contribType = "balanced" } // Pace @@ -1887,6 +1970,8 @@ func DevProfiles(ds *Dataset, filterEmail string, n int) []DevProfile { TopCommits: topCommits, TopCommitsHidden: topCommitsHidden, Scope: scope, ScopeHidden: scopeHidden, Extensions: extensions, ExtensionsHidden: extensionsHidden, + TestChurn: devTestChurn, SourceChurn: devSourceChurn, + TestRatio: safeRatio(devTestChurn, devSourceChurn), Specialization: specialization, ContribRatio: contribRatio, ContribType: contribType, Pace: pace, Collaborators: collabs, CollaboratorsHidden: collabsHidden, diff --git a/internal/stats/stats_test.go b/internal/stats/stats_test.go index 3dd49c1..28d6142 100644 --- a/internal/stats/stats_test.go +++ b/internal/stats/stats_test.go @@ -2644,6 +2644,40 @@ func TestHerfindahlHelper(t *testing.T) { } } +// herfindahl must be invariant to input order: callers build `values` by +// ranging a map (randomized order), and because float Σpᵢ² is +// non-associative, an unsorted sum made the Specialization field jitter +// run-to-run. The internal sort makes the result deterministic. +func TestHerfindahlOrderInvariant(t *testing.T) { + // One dominant share plus many tiny ones — the case most sensitive to + // summation order in floating point. + base := []int{1000000} + for i := 0; i < 200; i++ { + base = append(base, 1) + } + cp := func() []int { return append([]int(nil), base...) } + + forward := herfindahl(cp()) + + rev := cp() + for i, j := 0, len(rev)-1; i < j; i, j = i+1, j-1 { + rev[i], rev[j] = rev[j], rev[i] + } + reversed := herfindahl(rev) + + // A deterministic permutation (stride coprime with len → bijection). + src := cp() + perm := make([]int, len(src)) + for i := range perm { + perm[i] = src[(i*7)%len(src)] + } + permuted := herfindahl(perm) + + if forward != reversed || forward != permuted { + t.Errorf("herfindahl not order-invariant: forward=%v reversed=%v permuted=%v", forward, reversed, permuted) + } +} + func TestPrintProfilesTopCommitsShortSHA(t *testing.T) { // Regression: the Top commits block used to slice tc.SHA[:12] // unconditionally, which panics when the dataset carries short diff --git a/internal/stats/tests.go b/internal/stats/tests.go new file mode 100644 index 0000000..f9a604a --- /dev/null +++ b/internal/stats/tests.go @@ -0,0 +1,534 @@ +package stats + +import ( + "fmt" + "path" + "sort" + "strings" +) + +// TestRole is the role a file plays for the test-coverage proxy metrics. +// Every tracked path resolves to exactly one role, so the three buckets +// partition the file set and "test / source" ratios have a well-defined +// denominator. +// +// - RoleTest — a test/spec file (hand-authored verification code). +// - RoleSource — hand-authored production code (the thing tests cover). +// - RoleOther — everything excluded from the ratio: vendor/generated +// output, lockfiles, docs, config, data, assets. Folding these into +// "source" would deflate the ratio with files no test could ever +// cover; folding them into "test" would inflate it. They get their +// own bucket so the ratio stays meaningful. +type TestRole string + +const ( + RoleTest TestRole = "test" + RoleSource TestRole = "source" + RoleOther TestRole = "other" +) + +// codeExtensions is the set of extensions treated as hand-authored +// production code. It defines the "source" denominator: a non-test, +// non-suspect file counts as source only when its extension is here. +// Anything else (docs, config, data, assets) lands in RoleOther. +// +// Deliberately curated rather than exhaustive — adding a language is a +// one-line change, and an unknown extension defaulting to "other" is the +// safe failure mode (it sits out of the ratio instead of silently +// inflating the source count with, say, a vendored binary blob). +// Extensions are stored lowercased to match extractExtension's output. +var codeExtensions = map[string]bool{ + ".go": true, + ".js": true, + ".jsx": true, + ".mjs": true, + ".cjs": true, + ".ts": true, + ".tsx": true, + ".py": true, + ".rb": true, + ".java": true, + ".kt": true, + ".kts": true, + ".scala": true, + ".cs": true, + ".c": true, + ".cc": true, + ".cpp": true, + ".cxx": true, + ".h": true, + ".hpp": true, + ".hxx": true, + ".rs": true, + ".swift": true, + ".m": true, + ".mm": true, + ".php": true, + ".pl": true, + ".pm": true, + ".sh": true, + ".bash": true, + ".lua": true, + ".dart": true, + ".ex": true, + ".exs": true, + ".erl": true, + ".clj": true, + ".cljs": true, + ".groovy": true, + ".vue": true, + ".svelte": true, + ".jl": true, + ".hs": true, + ".ml": true, + ".fs": true, + ".f90": true, + ".r": true, + ".sql": true, +} + +// isCodeExt reports whether ext (as returned by extractExtension, i.e. +// lowercased and dot-prefixed) names a production-code language. +func isCodeExt(ext string) bool { + return codeExtensions[ext] +} + +// testNameMatchers are the "this filename IS a test" rules whose pattern +// embeds the language extension (`_test.go`, `Test.java`), so they're +// unambiguous regardless of directory and need no code-extension gate — +// `foo_test.go` is a test wherever it lives. Kept conservative: only the +// dominant convention per ecosystem, so the rules don't fire on +// coincidental names. +var testNameMatchers = []func(string) bool{ + hasSuffixOf("_test.go"), // Go + hasSuffixOf("_test.py"), // Python (pytest/nose trailing form) + hasSuffixOf("_spec.rb"), // Ruby (RSpec) + hasSuffixOf("Test.java"), // JVM + hasSuffixOf("Tests.java"), + hasSuffixOf("Test.kt"), + hasSuffixOf("Tests.kt"), + hasSuffixOf("Test.scala"), + hasSuffixOf("Spec.scala"), + hasSuffixOf("Test.cs"), // .NET + hasSuffixOf("Tests.cs"), + hasSuffixOf("_test.cc"), // C++ (GoogleTest convention) + hasSuffixOf("_test.cpp"), + hasSuffixOf("_test.cxx"), + hasSuffixOf("_test.rs"), // Rust (separate-file convention) +} + +// testNameCodeMatchers are extension-agnostic filename conventions — a +// leading `test_` (Python) or an infix `.test.`/`.spec.` (JS/TS). They +// only count when the file is ALSO code, because otherwise data/config +// like `api.spec.yaml`, `users.test.json`, or `test_data.json` would be +// miscounted as tests. isTestFile applies them behind the isCodeExt gate. +var testNameCodeMatchers = []func(string) bool{ + basenamePrefix("test_"), // Python unittest/pytest leading form + basenameContains(".test."), // JS/TS foo.test.ts, foo.test.jsx + basenameContains(".spec."), // JS/TS/Angular foo.spec.ts +} + +// testDirSegments are directory names that conventionally hold tests. +// Unlike testNameMatchers, a directory hit alone is NOT enough — a +// `spec/openapi.yaml` or `test/fixtures/data.json` is data, not a test. +// classifyTestRole requires a directory match to ALSO be a code file +// before calling it a test, which keeps non-code payloads in test trees +// out of the test bucket. "it" (Maven Failsafe integration dir) is +// deliberately excluded: a 2-char generic segment collides with common +// non-test dirs (e.g. an `it/` Italian-locale tree), a false-positive +// risk that outweighs the niche convention. +var testDirSegments = []string{"test", "tests", "spec", "specs", "__tests__", "e2e"} + +// basenamePrefix matches when the final path segment (the filename) +// starts with pre. Mirrors the basename-scoped matchers in suspect.go so +// `a/test_dir/util.py` is not mistaken for a test on the directory name. +func basenamePrefix(pre string) func(string) bool { + return func(p string) bool { + base := p + if i := strings.LastIndex(p, "/"); i >= 0 { + base = p[i+1:] + } + return strings.HasPrefix(base, pre) + } +} + +// isSuspectPath reports whether p matches any vendor/generated heuristic. +// Reuses the suspect detector's pattern table so "what counts as +// generated noise" has one definition shared between the extract warning +// and the test ratio's exclusions. p must already be repo-prefix-stripped +// (the patterns assume bare repo-relative paths, same as +// DetectSuspectFiles). +func isSuspectPath(p string) bool { + for _, pat := range defaultSuspectPatterns { + if pat.Match(p) { + return true + } + } + return false +} + +// TestConfig carries the resolved test-detection rules for one stats run. +// It is built once per invocation from the built-in heuristics plus any +// user --test-glob overrides, then threaded into the test-stat functions +// so every metric (summary, trend, per-dev) classifies paths identically. +type TestConfig struct { + // extra holds matchers compiled from --test-glob. classifyTestRole + // checks them first — before the suspect gate and the code-extension + // gate — so an explicit user glob wins unconditionally: data files + // (golden-file suites under `testdata/*`) and even paths that match a + // vendor/generated pattern count as tests when the user names them. + extra []func(string) bool +} + +// NewTestConfig compiles user-supplied --test-glob patterns into a +// TestConfig. Each glob is matched (via path.Match) against the +// repo-relative path and every trailing sub-path, so `*_itest.go` +// (basename shape) and `integration/*` (path shape) both work. Empty +// strings are skipped. Malformed patterns are NOT rejected here (path.Match +// has no compile step — a bad glob simply never matches); call +// ValidateTestGlobs at the CLI layer to surface a typo before this runs. +func NewTestConfig(globs []string) TestConfig { + var cfg TestConfig + for _, g := range globs { + if g == "" { + continue + } + cfg.extra = append(cfg.extra, globMatcher(g)) + } + return cfg +} + +// ValidateTestGlobs returns the first --test-glob that is not a valid glob +// (path.Match syntax, e.g. an unterminated `[` class). Call it from the +// command layer before NewTestConfig so a typo fails fast with a clear +// error instead of silently matching nothing and skewing the test counts. +func ValidateTestGlobs(globs []string) error { + for _, g := range globs { + if g == "" { + continue + } + // path.Match validates the whole pattern even against an empty + // name (it walks the pattern to detect bad syntax), so this + // reliably surfaces ErrBadPattern. + if _, err := path.Match(g, ""); err != nil { + return fmt.Errorf("invalid --test-glob %q: %w", g, err) + } + } + return nil +} + +// globMatcher returns a predicate that matches glob against the full +// repo-relative path and every trailing sub-path. Testing each tail lets +// a path glob like `testdata/*` match at any depth (`pkg/testdata/x.json`) +// and a bare basename glob like `*.itest.go` match the filename. Uses +// path.Match (not filepath.Match): git paths are always forward-slash, and +// filepath.Match is OS-aware — on Windows its separator is `\`, so `*` +// would cross `/` and break the per-segment guarantee (same reasoning as +// extract.ShouldIgnore). path.Match never crosses `/`, so a glob with N +// slashes only matches a tail holding the same N segments — `testdata/*` +// matches direct children only; `**` is unsupported, express deep intent +// as a basename glob instead. +func globMatcher(glob string) func(string) bool { + return func(p string) bool { + if matchGlobOnce(glob, p) { + return true + } + for i := 0; i < len(p); i++ { + if p[i] == '/' && matchGlobOnce(glob, p[i+1:]) { + return true + } + } + return false + } +} + +func matchGlobOnce(glob, s string) bool { + ok, err := path.Match(glob, s) + return err == nil && ok +} + +// isTestFile reports whether p (an already repo-prefix-stripped path) is a +// test file by built-in convention. User --test-glob overrides are handled +// in classifyTestRole, not here, so this is purely the heuristic layer. +func isTestFile(p string) bool { + // Extension-embedded conventions fire regardless of location. + for _, m := range testNameMatchers { + if m(p) { + return true + } + } + // Extension-agnostic name conventions and test directories require the + // file to also be code, so fixtures/data/config (`api.spec.yaml`, + // `test/fixtures/x.json`) don't count as tests. + if isCodeExt(extractExtension(p)) { + for _, m := range testNameCodeMatchers { + if m(p) { + return true + } + } + for _, seg := range testDirSegments { + if hasPathSegment(seg)(p) { + return true + } + } + } + return false +} + +// classifyTestRole assigns path to exactly one role. Order matters: +// +// 1. Explicit --test-glob overrides win over everything, including the +// suspect gate — if a user names a path as a test, honor it even under +// vendor/ or *.lock (TestConfig.extra documents this contract). +// 2. Suspect (vendor/generated/lock/minified) → excluded as noise, so a +// dependency's own test (`node_modules/x/foo.test.js`) doesn't count +// toward this repo's test ratio. +// 3. Built-in test naming/dir conventions → test. +// 4. Code extension → source; anything left → other. +func classifyTestRole(path string, cfg TestConfig) TestRole { + p := stripRepoPrefix(path) + for _, m := range cfg.extra { + if m(p) { + return RoleTest + } + } + if isSuspectPath(p) { + return RoleOther + } + if isTestFile(p) { + return RoleTest + } + if isCodeExt(extractExtension(p)) { + return RoleSource + } + return RoleOther +} + +// TestSummary is the headline test-coverage proxy for a dataset: how much +// of the production code's footprint is matched by test code. The ratios +// are intentionally test-over-source (not test-over-total) so the +// denominator is "code a test could plausibly cover" — RoleOther files +// (docs/config/vendor/generated) sit out of both numerator and +// denominator. OtherFiles is reported only for transparency about what +// was excluded. +type TestSummary struct { + TestFiles int + SourceFiles int + OtherFiles int + FileRatio float64 // TestFiles / SourceFiles; 0 when SourceFiles == 0 + TestChurn int64 + SourceChurn int64 + ChurnRatio float64 // TestChurn / SourceChurn; 0 when SourceChurn == 0 + ByLanguage []TestLangStat +} + +// TestLangStat is the same ratio sliced to one language bucket (keyed by +// extractExtension of the per-era path). A test file's extension places +// it in its language's bucket — `foo_test.go` and `bar.go` both land +// under ".go" — so each row reads as "this language's test-to-source +// balance". Per-era keying means a `foo.js → foo.ts` migration splits +// across the ".js" and ".ts" buckets, matching ExtensionStats. +type TestLangStat struct { + Ext string + TestFiles int + SourceFiles int + TestChurn int64 + SourceChurn int64 + FileRatio float64 + ChurnRatio float64 +} + +// safeRatio is num/den as a float, guarding the zero-denominator case +// (no source code, or no source churn) by returning 0 instead of Inf/NaN. +// Callers that must distinguish "0 because empty" from "genuinely 0" +// check the underlying denominator themselves. +func safeRatio(num, den int64) float64 { + if den == 0 { + return 0 + } + return float64(num) / float64(den) +} + +// ComputeTestSummary aggregates the test/source ratio overall and per +// language. Churn is attributed PER ERA via fileEntry.eachEra: when a file +// was renamed across the test/source boundary (src/foo.go → foo_test.go), +// each era's churn is classified by the path it held at the time, so +// pre-rename production churn is not miscounted as test. A lineage is +// counted once per role it ever held — and once per (ext, role) in the +// language breakdown — matching ExtensionStats' "once per bucket"; a +// lineage that was only ever RoleOther counts toward OtherFiles. +// +// langTop bounds the ByLanguage slice (0 = all), ranking languages by +// combined test+source churn so the busiest languages surface first. +func ComputeTestSummary(ds *Dataset, cfg TestConfig, langTop int) TestSummary { + type langAcc struct { + testFiles, sourceFiles int + testChurn, sourceChurn int64 + } + langs := map[string]*langAcc{} + getLang := func(ext string) *langAcc { + la, ok := langs[ext] + if !ok { + la = &langAcc{} + langs[ext] = la + } + return la + } + + var s TestSummary + for path, fe := range ds.files { + var hadTest, hadSource bool + // seen dedups per-(ext,role) file counts within a multi-era lineage + // so one renamed file isn't counted twice in a language bucket; + // only allocated for the rare multi-path case. + var seen map[string]bool + if fe.byPath != nil { + seen = make(map[string]bool, len(fe.byPath)) + } + fe.eachEra(path, func(p string, churn int64, _ map[string]int64) { + role := classifyTestRole(p, cfg) + if role == RoleOther { + return + } + ext := extractExtension(p) + la := getLang(ext) + if role == RoleTest { + s.TestChurn += churn + la.testChurn += churn + hadTest = true + } else { + s.SourceChurn += churn + la.sourceChurn += churn + hadSource = true + } + // Per-language file count, deduped within the lineage. + if seen != nil { + k := ext + "\x00" + string(role) + if seen[k] { + return + } + seen[k] = true + } + if role == RoleTest { + la.testFiles++ + } else { + la.sourceFiles++ + } + }) + if hadTest { + s.TestFiles++ + } + if hadSource { + s.SourceFiles++ + } + if !hadTest && !hadSource { + s.OtherFiles++ + } + } + + s.FileRatio = safeRatio(int64(s.TestFiles), int64(s.SourceFiles)) + s.ChurnRatio = safeRatio(s.TestChurn, s.SourceChurn) + + s.ByLanguage = make([]TestLangStat, 0, len(langs)) + for ext, la := range langs { + s.ByLanguage = append(s.ByLanguage, TestLangStat{ + Ext: ext, + TestFiles: la.testFiles, + SourceFiles: la.sourceFiles, + TestChurn: la.testChurn, + SourceChurn: la.sourceChurn, + FileRatio: safeRatio(int64(la.testFiles), int64(la.sourceFiles)), + ChurnRatio: safeRatio(la.testChurn, la.sourceChurn), + }) + } + // Busiest language first; ext asc breaks ties so output is stable. + sortTestLangs(s.ByLanguage) + if langTop > 0 && langTop < len(s.ByLanguage) { + s.ByLanguage = s.ByLanguage[:langTop] + } + return s +} + +// TestRatioBucket is the test:source churn ratio for one time period. +// Periods are "YYYY-MM" (month) or "YYYY" (year). +type TestRatioBucket struct { + Period string + TestChurn int64 + SourceChurn int64 + Ratio float64 // TestChurn / SourceChurn; 0 when SourceChurn == 0 +} + +// TestRatioOverTime tracks how the test:source churn ratio moved over the +// life of the repo — "is this codebase getting more or less tested?". +// +// Resolution is monthly because the only per-file time series retained on +// the Dataset is the per-month churn (keyed "YYYY-MM"). granularity +// "year" rolls months up to "YYYY"; "month" passes through; finer values +// ("day"/"week") have no backing data and fall back to monthly buckets. +// RoleOther eras are skipped so the denominator matches ComputeTestSummary. +// Churn is split PER ERA (fileEntry.eachEra), so a file renamed across the +// test/source boundary contributes its pre-rename months to its old role +// and post-rename months to its new role, rather than labeling all of +// history by the final name. +func TestRatioOverTime(ds *Dataset, cfg TestConfig, granularity string) []TestRatioBucket { + type acc struct{ test, source int64 } + buckets := map[string]*acc{} + + periodKey := func(month string) string { + if granularity == "year" && len(month) >= 4 { + return month[:4] + } + return month + } + + for path, fe := range ds.files { + fe.eachEra(path, func(p string, _ int64, monthChurn map[string]int64) { + role := classifyTestRole(p, cfg) + if role == RoleOther { + return + } + for month, churn := range monthChurn { + k := periodKey(month) + b, ok := buckets[k] + if !ok { + b = &acc{} + buckets[k] = b + } + if role == RoleTest { + b.test += churn + } else { + b.source += churn + } + } + }) + } + + keys := make([]string, 0, len(buckets)) + for k := range buckets { + keys = append(keys, k) + } + sort.Strings(keys) // chronological: "YYYY-MM"/"YYYY" sort lexically + + out := make([]TestRatioBucket, 0, len(keys)) + for _, k := range keys { + b := buckets[k] + out = append(out, TestRatioBucket{ + Period: k, + TestChurn: b.test, + SourceChurn: b.source, + Ratio: safeRatio(b.test, b.source), + }) + } + return out +} + +func sortTestLangs(ls []TestLangStat) { + sort.Slice(ls, func(i, j int) bool { + ci := ls[i].TestChurn + ls[i].SourceChurn + cj := ls[j].TestChurn + ls[j].SourceChurn + if ci != cj { + return ci > cj + } + return ls[i].Ext < ls[j].Ext + }) +} diff --git a/internal/stats/tests_test.go b/internal/stats/tests_test.go new file mode 100644 index 0000000..ecb8b52 --- /dev/null +++ b/internal/stats/tests_test.go @@ -0,0 +1,324 @@ +package stats + +import ( + "os" + "path/filepath" + "testing" +) + +func TestClassifyTestRole(t *testing.T) { + cfg := NewTestConfig(nil) + cases := []struct { + path string + want TestRole + }{ + // --- Test naming conventions (fire regardless of directory) --- + {"internal/stats/stats_test.go", RoleTest}, // Go + {"foo_test.py", RoleTest}, // Python trailing + {"tests/test_login.py", RoleTest}, // Python leading + {"src/components/Button.test.tsx", RoleTest}, // JS/TS .test. + {"src/app/user.spec.ts", RoleTest}, // Angular .spec. + {"spec/models/user_spec.rb", RoleTest}, // Ruby RSpec + {"src/main/java/com/x/FooTest.java", RoleTest}, // JVM + {"src/main/java/com/x/FooTests.java", RoleTest}, // JVM plural + {"app/UserServiceTest.kt", RoleTest}, // Kotlin + {"Calc/CalcTests.cs", RoleTest}, // .NET + {"src/widget_test.cc", RoleTest}, // C++ GoogleTest + {"src/lib_test.rs", RoleTest}, // Rust separate file + + // --- Test directories: code in a test dir is a test --- + {"test/handler.go", RoleTest}, + {"src/test/java/com/x/Helper.java", RoleTest}, // Maven layout + {"e2e/checkout.ts", RoleTest}, + {"__tests__/reducer.js", RoleTest}, + + // --- Test directories with NON-code payload stay "other" --- + {"spec/openapi.yaml", RoleOther}, // API spec, not a test + {"test/fixtures/data.json", RoleOther}, // fixture data + {"tests/golden/output.txt", RoleOther}, + + // --- Name conventions on NON-code payload stay "other": the + // extension-agnostic .test./.spec./test_ rules are code-gated. + {"api.spec.yaml", RoleOther}, // OpenAPI spec, not a test + {"data/users.test.json", RoleOther}, // fixture data + {"test_data.json", RoleOther}, // fixture data, not a Python test + + // --- "it" is NOT a test dir segment (locale-collision guard) --- + {"web/i18n/it/strings.go", RoleSource}, + + // --- Source: code that is not a test --- + {"internal/stats/stats.go", RoleSource}, + {"src/components/Button.tsx", RoleSource}, + {"cmd/gitcortex/main.go", RoleSource}, + {"lib/user.rb", RoleSource}, + + // --- Other: docs, config, data --- + {"README.md", RoleOther}, + {"docs/guide.rst", RoleOther}, + {"config.yaml", RoleOther}, + {"data/seed.json", RoleOther}, + {"assets/logo.svg", RoleOther}, + {"Makefile", RoleOther}, + + // --- Suspect wins over test: a dependency's own test is noise --- + {"node_modules/lib/foo.test.js", RoleOther}, + {"vendor/pkg/util_test.go", RoleOther}, + {"dist/bundle.min.js", RoleOther}, + {"go.sum", RoleOther}, + {"package-lock.json", RoleOther}, + + // --- Multi-repo prefix is stripped before classification --- + {"myrepo:internal/stats/stats_test.go", RoleTest}, + {"myrepo:cmd/main.go", RoleSource}, + {"myrepo:vendor/x/a_test.go", RoleOther}, + } + + for _, c := range cases { + if got := classifyTestRole(c.path, cfg); got != c.want { + t.Errorf("classifyTestRole(%q) = %q, want %q", c.path, got, c.want) + } + } +} + +// The Python `test_` prefix rule must scope to the basename, not the +// path: util.go lives in a "test_utils" directory but is production code. +// "test_utils" is also not one of the test dir segments ("test"/"tests"), +// so the file should classify as plain source. +func TestTestNamePrefixScopesToBasename(t *testing.T) { + cfg := NewTestConfig(nil) + if got := classifyTestRole("test_utils/util.go", cfg); got != RoleSource { + t.Errorf("classifyTestRole(test_utils/util.go) = %q, want source", got) + } +} + +func TestComputeTestSummary(t *testing.T) { + ds := &Dataset{files: map[string]*fileEntry{ + "main.go": {additions: 100, deletions: 0}, + "util.go": {additions: 40, deletions: 10}, + "main_test.go": {additions: 50, deletions: 5}, + "util_test.go": {additions: 20, deletions: 0}, + "README.md": {additions: 30, deletions: 0}, // other + "app/user.ts": {additions: 80, deletions: 20}, + "app/user.spec.ts": {additions: 60, deletions: 0}, + "vendor/x_test.go": {additions: 999, deletions: 0}, // suspect → other + }} + + s := ComputeTestSummary(ds, NewTestConfig(nil), 0) + + if s.TestFiles != 3 || s.SourceFiles != 3 || s.OtherFiles != 2 { + t.Fatalf("counts: test=%d source=%d other=%d, want 3/3/2", s.TestFiles, s.SourceFiles, s.OtherFiles) + } + if s.TestChurn != 135 || s.SourceChurn != 250 { + t.Fatalf("churn: test=%d source=%d, want 135/250", s.TestChurn, s.SourceChurn) + } + if s.FileRatio != 1.0 { + t.Errorf("FileRatio = %v, want 1.0", s.FileRatio) + } + if s.ChurnRatio != 135.0/250.0 { + t.Errorf("ChurnRatio = %v, want %v", s.ChurnRatio, 135.0/250.0) + } + + // The vendored *_test.go must not leak into the test bucket. + if s.TestChurn >= 999 { + t.Errorf("vendored test churn leaked into TestChurn (%d)", s.TestChurn) + } + + // By-language: .go busiest (385 churn) before .ts (260). + if len(s.ByLanguage) != 2 { + t.Fatalf("ByLanguage len = %d, want 2", len(s.ByLanguage)) + } + if s.ByLanguage[0].Ext != ".go" || s.ByLanguage[1].Ext != ".ts" { + t.Fatalf("ByLanguage order = [%s %s], want [.go .ts]", s.ByLanguage[0].Ext, s.ByLanguage[1].Ext) + } + goLang := s.ByLanguage[0] + if goLang.TestFiles != 2 || goLang.SourceFiles != 2 || goLang.TestChurn != 75 || goLang.SourceChurn != 150 { + t.Errorf(".go lang = %+v, want test 2/75 source 2/150", goLang) + } +} + +func TestComputeTestSummaryNoSource(t *testing.T) { + // A repo of only docs/config: zero source means the ratio is + // undefined, not infinite. Guard the denominator. + ds := &Dataset{files: map[string]*fileEntry{ + "README.md": {additions: 10}, + "config.yaml": {additions: 5}, + }} + s := ComputeTestSummary(ds, NewTestConfig(nil), 0) + if s.SourceFiles != 0 || s.TestFiles != 0 { + t.Fatalf("counts: test=%d source=%d, want 0/0", s.TestFiles, s.SourceFiles) + } + if s.FileRatio != 0 || s.ChurnRatio != 0 { + t.Errorf("ratios should be 0 (guarded) when no source; got file=%v churn=%v", s.FileRatio, s.ChurnRatio) + } +} + +func TestTestRatioOverTime(t *testing.T) { + ds := &Dataset{files: map[string]*fileEntry{ + "main.go": {monthChurn: map[string]int64{"2024-01": 100, "2024-02": 50}}, + "main_test.go": {monthChurn: map[string]int64{"2024-02": 40, "2024-03": 20}}, + "README.md": {monthChurn: map[string]int64{"2024-01": 999}}, // other → skipped + }} + + got := TestRatioOverTime(ds, NewTestConfig(nil), "month") + if len(got) != 3 { + t.Fatalf("len=%d, want 3 (2024-01..03)", len(got)) + } + if got[0].Period != "2024-01" || got[0].SourceChurn != 100 || got[0].TestChurn != 0 { + t.Errorf("bucket[0] = %+v, want 2024-01 src=100 test=0", got[0]) + } + if got[1].Period != "2024-02" || got[1].SourceChurn != 50 || got[1].TestChurn != 40 || got[1].Ratio != 40.0/50.0 { + t.Errorf("bucket[1] = %+v, want 2024-02 src=50 test=40 ratio=0.8", got[1]) + } + if got[2].Period != "2024-03" || got[2].TestChurn != 20 || got[2].SourceChurn != 0 || got[2].Ratio != 0 { + t.Errorf("bucket[2] = %+v, want 2024-03 test=20 src=0 ratio=0(guarded)", got[2]) + } + if got[0].SourceChurn >= 999 { + t.Errorf("RoleOther file (README.md) leaked into the trend") + } + + // Year rollup folds every month into a single "2024" bucket. + yr := TestRatioOverTime(ds, NewTestConfig(nil), "year") + if len(yr) != 1 || yr[0].Period != "2024" || yr[0].SourceChurn != 150 || yr[0].TestChurn != 60 { + t.Fatalf("year rollup = %+v, want one 2024 bucket src=150 test=60", yr) + } +} + +// End-to-end (load → applyRenames merge → DevProfiles): a developer's +// per-profile test churn must be split per rename era too. A dev who wrote +// foo.go (production) before it was renamed to foo_test.go must have that +// pre-rename churn counted as SOURCE in their profile, not test — devLines +// is merged under the canonical path, so the split comes from byPath's +// per-dev churn captured at the merge. +func TestDevProfileTestChurnSplitsByEra(t *testing.T) { + jsonl := `{"type":"commit","sha":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","author_name":"D","author_email":"dev@x","author_date":"2024-01-01T10:00:00Z","additions":100,"deletions":0,"files_changed":1} +{"type":"commit_file","commit":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","path_current":"foo.go","path_previous":"foo.go","status":"A","additions":100,"deletions":0} +{"type":"commit","sha":"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb","author_name":"D","author_email":"dev@x","author_date":"2024-02-01T10:00:00Z","additions":30,"deletions":0,"files_changed":1} +{"type":"commit_file","commit":"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb","path_current":"foo_test.go","path_previous":"foo.go","status":"R090","additions":30,"deletions":0} +` + dir := t.TempDir() + path := filepath.Join(dir, "git_data.jsonl") + if err := os.WriteFile(path, []byte(jsonl), 0o644); err != nil { + t.Fatal(err) + } + ds, err := LoadJSONL(path) + if err != nil { + t.Fatal(err) + } + + profiles := DevProfiles(ds, "dev@x", 0, NewTestConfig(nil)) + if len(profiles) != 1 { + t.Fatalf("want 1 profile, got %d", len(profiles)) + } + p := profiles[0] + // Pre-rename foo.go (100) → source; post-rename foo_test.go (30) → test. + // Canonical-only classification would report all 130 as test. + if p.TestChurn != 30 || p.SourceChurn != 100 { + t.Errorf("profile churn: test=%d source=%d, want 30/100 (pre-rename stays source)", p.TestChurn, p.SourceChurn) + } + if p.TestRatio != 30.0/100.0 { + t.Errorf("TestRatio = %v, want 0.3", p.TestRatio) + } +} + +// Regression for the per-era capture: a rename CHAIN that includes a +// pure-rename era (0 churn → empty devLines) must not over-count or become +// order-dependent. foo.go (+100, source) → bar.go (pure rename, 0) → +// bar_test.go (+30, test). The per-dev split must reconcile exactly with +// the repo-level summary (Σ per-dev == repo-level), which a missed capture +// of the empty era broke. +func TestDevProfilePureRenameChainNoOvercount(t *testing.T) { + jsonl := `{"type":"commit","sha":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","author_name":"D","author_email":"dev@x","author_date":"2024-01-01T10:00:00Z","additions":100,"deletions":0,"files_changed":1} +{"type":"commit_file","commit":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","path_current":"foo.go","path_previous":"foo.go","status":"A","additions":100,"deletions":0} +{"type":"commit","sha":"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb","author_name":"D","author_email":"dev@x","author_date":"2024-02-01T10:00:00Z","additions":0,"deletions":0,"files_changed":1} +{"type":"commit_file","commit":"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb","path_current":"bar.go","path_previous":"foo.go","status":"R100","additions":0,"deletions":0} +{"type":"commit","sha":"cccccccccccccccccccccccccccccccccccccccc","author_name":"D","author_email":"dev@x","author_date":"2024-03-01T10:00:00Z","additions":30,"deletions":0,"files_changed":1} +{"type":"commit_file","commit":"cccccccccccccccccccccccccccccccccccccccc","path_current":"bar_test.go","path_previous":"bar.go","status":"R090","additions":30,"deletions":0} +` + dir := t.TempDir() + path := filepath.Join(dir, "git_data.jsonl") + if err := os.WriteFile(path, []byte(jsonl), 0o644); err != nil { + t.Fatal(err) + } + ds, err := LoadJSONL(path) + if err != nil { + t.Fatal(err) + } + + p := DevProfiles(ds, "dev@x", 0, NewTestConfig(nil))[0] + if p.TestChurn != 30 || p.SourceChurn != 100 { + t.Errorf("profile churn: test=%d source=%d, want 30/100 (no over-count from the pure-rename era)", p.TestChurn, p.SourceChurn) + } + // Reconciliation invariant: per-dev == repo-level. + s := ComputeTestSummary(ds, NewTestConfig(nil), 0) + if p.TestChurn != s.TestChurn || p.SourceChurn != s.SourceChurn { + t.Errorf("per-dev (%d/%d) != repo-level (%d/%d)", p.TestChurn, p.SourceChurn, s.TestChurn, s.SourceChurn) + } +} + +// A file renamed across the test/source boundary (src/foo.go → foo_test.go) +// must attribute pre-rename churn to source and post-rename churn to test, +// not classify its whole history by the final (test) name. Exercised via +// byPath, which the loader populates for renamed lineages. +func TestRenameAcrossTestBoundary(t *testing.T) { + ds := &Dataset{files: map[string]*fileEntry{ + // Canonical (final) path is a test file, but the lineage was + // production code for most of its life. + "foo_test.go": {byPath: map[string]*pathEra{ + "foo.go": {churn: 100, monthChurn: map[string]int64{"2024-01": 100}}, + "foo_test.go": {churn: 30, monthChurn: map[string]int64{"2024-06": 30}}, + }}, + "bar.go": {additions: 50, monthChurn: map[string]int64{"2024-03": 50}}, // unrenamed → fallback + }} + + s := ComputeTestSummary(ds, NewTestConfig(nil), 0) + // Pre-rename 100 stays SOURCE; only the 30 post-rename is test. + if s.TestChurn != 30 || s.SourceChurn != 150 { + t.Errorf("churn: test=%d source=%d, want 30/150 (pre-rename 100 must stay source)", s.TestChurn, s.SourceChurn) + } + // The renamed lineage counts as both a test file and a source file. + if s.TestFiles != 1 || s.SourceFiles != 2 { + t.Errorf("files: test=%d source=%d, want 1/2", s.TestFiles, s.SourceFiles) + } + + tr := TestRatioOverTime(ds, NewTestConfig(nil), "month") + got := map[string]TestRatioBucket{} + for _, b := range tr { + got[b.Period] = b + } + if b := got["2024-01"]; b.SourceChurn != 100 || b.TestChurn != 0 { + t.Errorf("2024-01 = %+v, want source=100 test=0 (pre-rename month must be source)", b) + } + if b := got["2024-06"]; b.TestChurn != 30 || b.SourceChurn != 0 { + t.Errorf("2024-06 = %+v, want test=30 source=0", b) + } +} + +// --test-glob overrides count their matches as tests unconditionally, +// even when the path is data the built-in rules would call "other". +func TestTestConfigExtraGlobs(t *testing.T) { + cfg := NewTestConfig([]string{"testdata/*", "*.itest.go", "fixtures/snap/*"}) + cases := []struct { + path string + want TestRole + }{ + {"pkg/testdata/golden.json", RoleTest}, // would be "other" by default + {"pkg/flow.itest.go", RoleTest}, // custom basename suffix + {"pkg/flow.go", RoleSource}, // unaffected + // A user glob wins even over the suspect (vendor/lock) gate. + {"vendor/fixtures/snap/x.json", RoleTest}, + } + for _, c := range cases { + if got := classifyTestRole(c.path, cfg); got != c.want { + t.Errorf("classifyTestRole(%q) with globs = %q, want %q", c.path, got, c.want) + } + } +} + +func TestValidateTestGlobs(t *testing.T) { + if err := ValidateTestGlobs([]string{"testdata/*", "*.go", ""}); err != nil { + t.Errorf("valid globs rejected: %v", err) + } + if err := ValidateTestGlobs([]string{"ok/*", "[unterminated"}); err == nil { + t.Errorf("malformed glob '[unterminated' was not rejected") + } +}