Add test-stats: history-based test-to-source ratio - #13
Merged
Conversation
New `--stat tests` (plus an HTML report section and a per-dev figure) that classifies every tracked file as test/source/other by path convention and reports the test-to-source ratio — files and churn — overall, per language, and as a ratio-over-time trend. It is a history-only proxy for test investment, not coverage: gitcortex never reads file contents or runs a suite. - internal/stats/tests.go (new): classifier (language-embedded test naming, code-gated .test./.spec./test_ and test dirs, vendor/generated exclusion reusing the suspect table), ComputeTestSummary, TestRatioOverTime, and NewTestConfig/ValidateTestGlobs for --test-glob. - CLI: --stat tests (table/csv/json) and --test-glob on stats/report/scan. - HTML report: Tests section (summary, by-language, yearly trend) and a test-share figure on developer profiles (full report + single-dev). - docs: METRICS.md Tests section, README catalog row + usage example. --test-glob matches via path.Match (forward-slash, Windows-safe — same reasoning as extract.ShouldIgnore) and overrides the suspect gate; malformed patterns are rejected up front. Ratios are test-over-source so docs/config/vendor sit out of both numerator and denominator. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f8c7696fb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Four changes to the extract/load path (2 correctness, 2 perf): - git/stream.go: pass `-c core.quotePath=false` to `git log` so non-ASCII pathnames arrive as clean UTF-8 instead of C-quoted/octal-escaped. Without it, files with accented/unicode names were stored with quotes+escapes in every path-based stat, and a renamed unicode file's numstat key stopped matching its raw path — resolving that file's churn to 0. - stats/reader.go: a commit whose author date does not parse is now excluded from a bounded --from/--to/--since window instead of slipping past the zero-time guard and being counted in every window. - stats/reader.go: cache the decay weight and month key per commit, keyed on SHA change across a commit's contiguous file lines (loop-local, not retained on commitEntry), instead of re-running math.Exp and time.Format per file. - stats/reader.go: truncate the retained commit message at ingest on a rune boundary (only the first ~77 chars are ever displayed), saving hundreds of MB on large repos. Regression tests: a unicode add+rename LogStreamer test (gitconfig-isolated so it reliably catches flag removal) and a windowed undated-commit exclusion test. Verified byte-identical stats output vs baseline on pi-hole, kubernetes, WordPress, and praat fixtures (full JSON + windowed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ComputeTestSummary/TestRatioOverTime classified each file by its canonical
(post-rename) path, so a file renamed across the test/source boundary —
src/foo.go → foo_test.go, or a move into tests/ or under vendor/ — had its
entire history reclassified by the final name. Pre-rename production churn
was counted as test, and the trend mislabeled old months, corrupting the
metric for test migrations.
Track the role per era instead:
- reader.go: add fileEntry.byPath (path → {churn, monthChurn}), populated at
ingest from the path-at-commit (pre-rename, like byExt), unioned in
mergeFileEntry. finalizeDataset drops it for files that only ever held one
path, so unrenamed files (the majority) carry no extra map and fall back to
the canonical path — exact for a single era. New fileEntry.eachEra helper
yields (path, churn, monthChurn) per era over byPath or the fallback.
- tests.go: both stat functions classify each era by its own path; a lineage
counts once per role it ever held (and once per (ext,role) per language),
matching ExtensionStats' per-bucket semantics.
Adds TestRenameAcrossTestBoundary. Verified non-test stats stay byte-identical
on the fixtures (byPath is read only by the test-stat functions); the tests
stat correctly shifts on pi-hole/WordPress/praat as vendored/moved pre-rename
history stops being miscounted.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99d8c454bd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A math-correctness audit of the stat computations surfaced one real bug plus a few smaller ones; the core (sums, ratios, Herfindahl, bus factor, coupling, decay, percentiles, test-stats per-era) checked out. - report.go: the directories Pareto ranked by file-touches, not churn — ComputePareto iterated DirectoryStats (sorted by file-touches) and ran the cumulative-80%-of-churn loop in that order, so TopChurnDirs over-counted and the concentration label could flip. Re-sort dirs by churn desc first (mirrors the Files path). On real repos this was large: WordPress 55→16 dirs, praat relabeled "moderately"→"extremely concentrated". - stats.go ExtensionCount: counted only canonical-path extensions, so the "Top N of M" header under-counted M on cross-extension renames (M could be smaller than the rows shown). Count per-era byExt buckets, matching ExtensionStats (WordPress 41→43). - repo_breakdown.go: under an email filter (the only mode it renders in) the Devs column collapsed to a constant 1 because dev accumulation sat behind the email gate. Count authors repo-wide before the filter; the column documents itself as repo-wide. - stats.go DevProfiles: a dev with deletions but zero additions was labeled "growth"; classify as refactor (del/add is unbounded cleanup). - docs/METRICS.md + TestLangStat comment: the test-stats churn/by-language prose still described the pre-fix whole-file/canonical-path attribution; update to per-era. Adds regression tests for the Pareto-dirs ranking, per-era ExtensionCount, and repo-wide breakdown devs. Verified the JSON stats diff vs baseline is confined to the pareto-dirs fields on the fixtures. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
DevProfiles classified a developer's whole churn on a file by its canonical (post-rename) path, so a file renamed across the test/source boundary (foo.go → foo_test.go) reported the dev's pre-rename production edits as test churn — the same misattribution ComputeTestSummary already avoids via eachEra, but at the per-dev level. Per-dev per-era needs a dev dimension on the era data, which byPath's churn and monthChurn don't carry. Add pathEra.devChurn (email → churn), captured only during rename merges (captureEraDevChurn) from each era's pre-merge devLines — so unrenamed files (the majority, byPath dropped at finalize) pay nothing, and only renamed lineages carry the per-dev map. DevProfiles then splits the dev's churn per era, falling back to the canonical path when byPath is nil (exact for a single era). Subtlety fixed during validation: a pure-rename era (0 churn → empty devLines) must still be MARKED captured (with an empty map), otherwise a later merge in a rename chain re-captured it from the by-then-summed devLines, producing an order-dependent over-count. Verified on the fixtures that the per-dev aggregate now reconciles exactly and deterministically with the repo-level summary (pi-hole 9988/97318, kubernetes 5437195/8779305) and that non-profile stats stay byte-identical. Adds tests for the boundary split, the pure-rename chain (with a per-dev == repo-level reconciliation assert), and the end-to-end load path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The per-era test-stats split (byPath) was populated for every commit_file during load — a map plus a nested per-month map on every fileEntry, including the unrenamed majority and runs that never read test stats — then dropped for single-path files only at finalizeDataset, after the whole JSONL was loaded. That spiked peak heap during load (e.g. a plain `stats --stat summary` paid the full cost) and could risk OOM before the drop. Build byPath only in applyRenames, for entries that actually merge (renamed lineages), via a new ensureEra that snapshots each pre-merge entry's own-era churn/monthChurn/devChurn before the merge sums them. Unrenamed files never allocate byPath and the test stats fall back to the canonical path (exact for a single era). This drops the ingest-time allocation and the finalizeDataset cleanup entirely, and folds the old merge-time captureEraDevChurn into ensureEra. Behavior is unchanged: `--stat tests` is byte-identical and the per-dev churn still reconciles exactly with the repo-level summary on the fixtures; peak load RSS on kubernetes dropped ~657→613 MB (scales with repo size). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
herfindahl summed Σpᵢ² over `values` in the order the caller built the slice — which is a map range (per-directory file counts), so the iteration order is randomized. Float addition is non-associative, so the result's low-order bits varied run-to-run, making the JSON Specialization field non-deterministic (the %.3f display and the label bands were unaffected, but reports/diffs weren't reproducible). Sort `values` ascending before the float loop: a canonical order makes the result deterministic (and ascending also minimizes rounding error). The integer sum is order-independent and the displayed value is unchanged; only the now-stable low-order bits differ. Verified on the fixtures that the full stats JSON is byte-stable across runs and the %.3f Specialization is identical to before. Adds TestHerfindahlOrderInvariant. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Update the docs to match behavior changed in this branch: - Per-Repository Breakdown `Devs` is now repo-wide (counts all of a repo's authors, not just the profiled dev) — note it as the deliberate exception to the otherwise email-filtered columns, in METRICS.md and the two README spots that describe the section. - Contribution type: prose said refactor ">0.8" but the code and threshold table use ">=0.8"; align it, and document that a pure-deletion dev (zero additions) classifies as refactor. - Per-dev test churn (profiles) is now split per rename era like the repo-level metric; say so. No code change. The Pareto/Extensions/Tests sections were already correct (the code fixes made the code match the docs). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
New
--stat tests(plus an HTML report section and a per-dev figure) that classifies every tracked file as test/source/other by path convention and reports the test-to-source ratio — files and churn — overall, per language, and as a ratio-over-time trend. It is a history-only proxy for test investment, not coverage: gitcortex never reads file contents or runs a suite.--test-glob matches via path.Match (forward-slash, Windows-safe — same reasoning as extract.ShouldIgnore) and overrides the suspect gate; malformed patterns are rejected up front. Ratios are test-over-source so docs/config/vendor sit out of both numerator and denominator.