Repository navigation
feat: report content-duplicate commits instead of silently counting a change twice - #116
Merged
Merged
Conversation
code-quality-metrics-7ccq: local-code-metrics.js dedupes analyzed commits by SHA only, which does not catch a squash-merge duplicate -- the squash commit on the default branch and the feature-branch commits it squashed are different SHAs carrying the same change, so both get analyzed when the branch survives the merge. findContentDuplicateGroups (lib/git.js) groups commits sharing a normalized subject (trailing "(#N)"/"(GH-N)" stripped, reusing the existing PR_REFERENCE_PATTERN), total_additions and total_deletions, surfacing any group with more than one distinct SHA. This is a floor, not an exact test: it will miss a multi-commit branch whose squash diff differs from the sum of its parts, and documented as detection-only in its own comment -- not wired into anything yet. TDD: wrote a stub returning [] unconditionally (compiles, behaviorally absent), then a test asserting two same-signature, different-SHA commits group together. Predicted RED: stub returns [] so the assertion fails comparing [] against the expected one-group array. Actual RED matched exactly. Implemented the real grouping and got GREEN.
…dContentDuplicateGroups Three more tests for the function added in the prior commit: PR-reference suffix stripping, a false-positive guard (same subject, different diff size must not group), and a same-SHA-input guard. All three passed on arrival -- the prior commit implementation already stripped PR_REFERENCE_PATTERN and already compared additions/deletions as part of the key, so no production code changed here. Disclosing this rather than presenting it as a red/green cycle: predicted RED for the PR-suffix test specifically, expecting it to need production code, ran it, got GREEN immediately, and stopped to diagnose before writing anything. Diagnosis was that the generalization already existed, not that the test was wrong.
code-quality-metrics-7ccq: wires findContentDuplicateGroups (lib/git.js) into collectLocalMetrics. After metrics is built, groups it by content signature and adds three summary fields: content_duplicate_group_count, content_duplicate_redundant_entries_count, and the full content_duplicate_groups detail (subject, total_additions, total_deletions, shas). A non-empty result also logs a console line pointing at the summary field. Deliberately report-only: total_commits, filtered_from and every rate metric/distribution are untouched. The detection is a floor (documented on findContentDuplicateGroups itself) that can miss a divergent squash diff and could in principle coincide on two unrelated commits, so silently dropping an entry on this signal would trade a visible distortion for an invisible one. This is the "report rather than correct" branch the issue explicitly allows. TDD: added two tests to collectLocalMetrics.test.js -- a two-branch fixture (a "(#42)"-suffixed squash-style commit and its un-suffixed original, same diff size, different SHA) asserting content_duplicate_group_count: 1, content_duplicate_redundant_entries_count: 1, and the group detail; plus a guard asserting 0/0/[] when no duplicate exists. Predicted RED: both new summary fields are undefined before this commit, so expect(summary.content_duplicate_group_count).toBe(1) (and .toBe(0) in the guard) fail with "Received: undefined". Actual RED matched exactly for both. Implemented the wiring and got GREEN; total_commits stayed 2 in the duplicate fixture, confirming no silent correction. Fixed an accidental extra closing brace introduced while appending this describe block to the end of collectLocalMetrics.test.js (caught by node -c before running the suite).
PR AnalysisSize: small (based on production code) Strengths
Commit AnalysisTotal Commits: 3
Commit Detailsa895f5d Ken Judy (8/20/2026) d1a9dd1 Ken Judy (8/20/2026) acd5d2a Ken Judy (8/20/2026) Test CoverageTest-to-Production Ratio: 1.68:1 (no calibrated target for this ratio; reported descriptively) DORA Capability AssessmentArchetype: harmonious-high-achiever
|
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.
Addresses code-quality-metrics-7ccq (P1). Reports rather than corrects, deliberately; the issue permits either and the reasoning is below.
The defect
local-code-metrics.js dedupes analysed commits by SHA. That does not remove a squash-merge duplicate: the squash commit on the default branch and the feature-branch commits it squashed are different SHAs carrying the same change. When the branch survives the merge, both are analysed.
Reproduced on this repository: 15 groups covering 17 redundant entries out of 531, grouping by normalised subject plus total_additions plus total_deletions. The issue predicted 14 of 528. The gap is window drift, since the run is HEAD-anchored and this session added commits between the issue being written and the reproduction; the extra group is the newest commit duplicated with its own surviving branch. Diagnosed before implementing rather than after.
Measured effect, which is why reasoning about it was not enough
No verdict flips on this sample, and the direction is not uniform: test_isolation_rate rose while everything else fell. A duplicate lands in both numerator and denominator, so the bias cannot be predicted from first principles, only measured.
The distortion scales with how many merged branches survive, which is branch hygiene rather than engineering practice. That is the opposite of what these metrics claim to measure.
Report, not correct
findContentDuplicateGroups (lib/git.js) groups by normalised subject, reusing the existing PR_REFERENCE_PATTERN, plus total_additions and total_deletions. The summary gains content_duplicate_group_count, content_duplicate_redundant_entries_count and content_duplicate_groups, and the console prints a line when any are found. total_commits, every rate metric and every distribution are untouched.
The content signature is documented as a floor: it misses a multi-commit branch whose squash net diff differs from the sum of its parts. The reliable alternative is the merged pull request head and base via gh, validated on the deferred code-quality-metrics-76ar and GitHub #107, and it was rejected here for three reasons. It would put network and authentication into local-code-metrics.js, which CLAUDE.md keeps deliberately local-only. Two of the six repositories this tool measures have no API access. And the acceptance criterion asks for a false-negative rate validated across all six, which the sandbox restriction on running the tool outside the worktree made impossible to produce.
Silently deduping a number this central on a heuristic that sometimes misfires would be worse than disclosing it. The issue stays open for the validation the acceptance criteria ask for.
The calibration data is unaffected, and this was checked rather than assumed
Every observation in calibration/observations.json records workflow_type: trunk, with zero occurrences of feature_branch anywhere in the file, consistent with the README mandated --single-branch clone protocol for calibration runs. A single-branch clone never fetches the pre-squash branch, so this duplicate shape cannot occur in that data. No observation needs a reservation.
Verification
720 tests across 43 suites, lint and typecheck clean at every commit. Three cycles with predicted and actual RED quoted in their commit messages; one cycle is disclosed as having passed on arrival because the previous implementation already generalised, rather than being presented as a clean cycle.
🤖 Generated with Claude Code