Repository navigation
Release 3.0.0: explicit thresholds, complete CRAP scoring, deterministic file identity - #78
Conversation
…tic file identity Three gate contracts change: check, test, the Fallout target and the badge action have no default threshold; the CRAP gate needs every method scored; a report with several source roots no longer triggers a disk probe, and unresolved files make every gate NODATA until --probe-sources or --source-root resolves them. --exclude-test-code joins --exclude-generated, and an active exclusion is listed on stderr or in JSON. Seven counting fixes: ParseAsync finishes method-only classes; CRAP rows stay per file and per overload; condition identity no longer depends on XML packaging; contradictory branch data warns instead of inflating rates; an empty class summary decides the file; and ./src/C.cs and src/C.cs are one file. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 03ae1f238e
ℹ️ 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".
| .DistinctBy(PathIdentity.NormalizeRoot, StringComparer.Ordinal).ToList() | ||
| .DistinctBy(PathIdentity.NormalizeRoot, StringComparer.Ordinal).ToList(), | ||
| AmbiguousFiles = AmbiguousFile.Union(a.AmbiguousFiles.Concat(b.AmbiguousFiles)), | ||
| Exclusion = a.Exclusion.Union(b.Exclusion) |
There was a problem hiding this comment.
When reports were filtered differently before being merged, unioning their exclusion records can claim that a file was excluded even though the other report reintroduces that file into merged.Values. For example, merging a report that excluded tests/A.Tests/ATests.cs with one that retained it includes the file in coverage totals while JSON and diagnostics still list it under ExcludedFiles. Reconcile the combined exclusion metadata against the final merged file paths, or retain only exclusions that remain absent from the result.
Useful? React with 👍 / 👎.
| { | ||
| logical = new LogicalMethod(typeKey, typeIdentity, methodKey, arity, folded, mc.File); | ||
| int? arity = origin is null ? MethodIdentity.SignatureArity(mc.Signature) : null; | ||
| logical = new LogicalMethod(name, typeKey, arity, origin); |
There was a problem hiding this comment.
When a lambda from an async overload appears before that overload's state-machine entry, both share the same folded ordinal and this first-created LogicalMethod permanently retains the lambda's OwnBody=false. The later state machine is merged without promoting that flag, so if a covered synchronous overload with the same name is the sole direct candidate, FoldSyntheticGroups incorrectly folds the async overload into it; reversing the XML/list order keeps them separate. This can dilute the async method's CRAP score into a false pass, so the grouped origin must retain OwnBody=true if any merged entry represents its own state-machine body.
Useful? React with 👍 / 👎.
| if (Unscored.Count > 0) | ||
| return new CrapGateResult(GateOutcome.NoData, maxCrap, Methods.Count, above, worst, | ||
| Invariant($"{Unscored.Count} of {Methods.Count + Unscored.Count} methods could not be scored - ") + | ||
| "supply --metrics <file> (dotnet msbuild /t:Metrics); a verdict needs every method scored"); |
There was a problem hiding this comment.
When --metrics contains a method or accessor with no coverage counterpart, CollectUnmatched records it but this completeness check considers only Unscored; if every covered method is below the limit, the gate still returns PASS. An unmatched member can be an uninstrumented method with no coverage measurement at all—and may be the riskiest method—so the new “every method must be scored” contract should return NODATA when UnmatchedMetricsMembers is nonempty rather than merely listing it.
AGENTS.md reference: AGENTS.md:L21-L23
Useful? React with 👍 / 👎.
Summary
Three gate contracts change; seven counting bugs are fixed. Every number a gate decides on may move.
Brief decisions taken with the owner: cross-file folding never happens (the A2 test wins over the prose); synthetic groups that cannot be attributed stay scored rows; A5(b) warns on 2-way-shaped detail that disagrees, while non-2-way detail (coverlet switch, grcov per-outcome counts) stays silent as before.
Evidence
ParseAsync_MethodOnlyClass_YieldsTheSameReportAsSync,SameMethodInTwoFiles_IsTwoRows_NotOneAveragedRow,AsyncOverloads_KeepTheirOwnRowsAndScores,ConditionIdentity_OneDocumentWithTwoBlocks_EqualsTwoMergedDocuments,ConditionDetailClaimingMoreThanItsLine_IsDroppedWithAWarning,EmptyClassSummary_..._FileIsNoData(<lines/>),DotSegments_InFilenames_FoldIntoOneFile.After:
fallout Coverage→PASS: line 100.0% (min 100%), branch 100.0% (min 100%), 1547/1547 tests;fallout Crap→PASS: worst CRAP 30.0 (max 30) - all 411 methods at or below threshold;git grep -n '"80"' src→ nothing.ThresholdContractTests,ExcludeTestCodeTests,FileIdentityCliTests(real process, two working directories,dotcov testagainst a stand-in dotnet),CliCrapTests,FalloutBuildTests,RepositoryBuildTests,BadgeActionTests.Merge Danger
Door: two-way for the code; the
v3.0.0tag and NuGet packages that follow are one-way.Blast Radius: consumers
Gates that relied on the implicit 80 now exit 2; CRAP gates with unscored methods and coverage.py reports with several roots now exit 3 until
--metrics/--probe-sources/--source-root. CI's consumer and README-badge jobs opt into--probe-sources. Library:ParseAsynctakes the identity before the token,CrapAnalysis.ExcludeFilesis replaced byMethodCoverageReport.Exclude,ICoverageReport.MinLine/MinBrancharestring?.🤖 Generated with Claude Code