fix(repoanalysis): only claim test coverage for files we actually probed #87

Merged

rcsheets merged 1 commit from fix/test-coverage-overclaim into main

2026-07-28 03:55:39 +00:00

rcsheets commented

2026-07-28 00:26:39 +00:00

Owner

RepoHasTests was derived as len(UntestedFiles) < len(sources), and
Format only reached that branch when UntestedFiles was empty. Files that
findUntestedFiles skips — non-code, or a language with no convention in
testFileCandidates — landed in neither the numerator nor the denominator,
so a PR touching only Terraform, Rust, or Markdown made zero HEAD probes
and still told the model "Changed files appear to have test coverage".

Split the probe into explicit tested/untested lists so unprobed files are
represented as unknown rather than tested, and drop the bullet entirely
when nothing was probed. Also reword the surviving claim: what we verify
is that a matching test file exists on the head branch, which is not the
same as the change being covered.

Co-Authored-By: Claude Opus 5 noreply@anthropic.com

RepoHasTests was derived as `len(UntestedFiles) < len(sources)`, and Format only reached that branch when UntestedFiles was empty. Files that findUntestedFiles skips — non-code, or a language with no convention in testFileCandidates — landed in neither the numerator nor the denominator, so a PR touching only Terraform, Rust, or Markdown made zero HEAD probes and still told the model "Changed files appear to have test coverage". Split the probe into explicit tested/untested lists so unprobed files are represented as unknown rather than tested, and drop the bullet entirely when nothing was probed. Also reword the surviving claim: what we verify is that a matching test file exists on the head branch, which is not the same as the change being covered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

rcsheets added 1 commit

2026-07-28 00:26:39 +00:00

fix(repoanalysis): only claim test coverage for files we actually probed

ci / check (pull_request) Successful in 42s

Details

16df018286

RepoHasTests was derived as `len(UntestedFiles) < len(sources)`, and
Format only reached that branch when UntestedFiles was empty. Files that
findUntestedFiles skips — non-code, or a language with no convention in
testFileCandidates — landed in neither the numerator nor the denominator,
so a PR touching only Terraform, Rust, or Markdown made zero HEAD probes
and still told the model "Changed files appear to have test coverage".

Split the probe into explicit tested/untested lists so unprobed files are
represented as unknown rather than tested, and drop the bullet entirely
when nothing was probed. Also reword the surviving claim: what we verify
is that a matching test file exists on the head branch, which is not the
same as the change being covered.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

pr-reviewer-bot commented

2026-07-28 00:26:41 +00:00

Collaborator

Automated review by pr-reviewer v0.41.3 | Safety Check | Claude Sonnet 5 | tracking id r-67f741-269b21
This is an AI-generated review and may contain mistakes.

Status: ✅ Completed

⚠️ The primary review (Mistral Small) failed. The review below is from Claude Sonnet 5, which reviewed the same diff.

✅ Verdict: LGTM — Bug-fix PR is logically sound, appropriately tested, and introduces no security or stability concerns.

This is a straightforward, well-scoped fix. The change correctly addresses the described bug: previously RepoHasTests was inferred as len(UntestedFiles) < len(sources), which silently treated unprobed files (non-code, or languages without a test convention) as "tested" simply because they weren't in the untested list. The fix replaces this with explicit TestedFiles/UntestedFiles lists, only emitting the test-coverage claim when something was actually probed, and softens the wording to reflect that file existence isn't proof of coverage.

The logic change in probeTestFiles (formerly findUntestedFiles) looks correct — files skipped via isNonCode or lacking testFileCandidates now land in neither list, which is the intended behavior. The Format switch was updated to check len(r.TestedFiles) > 0 instead of the old RepoHasTests field, consistent with the struct change.

Tests were added for both the new "no claim when nothing probed" case and the reworded output, which is good given the bug was subtle and behavior-order dependent (silent success case).

No secrets, no unsafe injected input into HEAD requests (paths are file paths, not raw user text executed anywhere), and no breaking API changes beyond an internal struct field rename (RepoHasTests → TestedFiles), which is safe since this is an internal type.

Nothing here looks alarming or accidentally committed.

*Automated review by [pr-reviewer](https://git.brooktrails.org/brooktrails/pr-reviewer) v0.41.3 | Safety Check | Claude Sonnet 5 | tracking id `r-67f741-269b21`* *This is an AI-generated review and may contain mistakes.* **Status:** ✅ Completed --- > ⚠️ The primary review (**Mistral Small**) failed. The review below is from **Claude Sonnet 5**, which reviewed the same diff. **✅ Verdict: LGTM** — Bug-fix PR is logically sound, appropriately tested, and introduces no security or stability concerns. This is a straightforward, well-scoped fix. The change correctly addresses the described bug: previously `RepoHasTests` was inferred as `len(UntestedFiles) < len(sources)`, which silently treated unprobed files (non-code, or languages without a test convention) as "tested" simply because they weren't in the untested list. The fix replaces this with explicit `TestedFiles`/`UntestedFiles` lists, only emitting the test-coverage claim when something was actually probed, and softens the wording to reflect that file existence isn't proof of coverage. The logic change in `probeTestFiles` (formerly `findUntestedFiles`) looks correct — files skipped via `isNonCode` or lacking `testFileCandidates` now land in neither list, which is the intended behavior. The `Format` switch was updated to check `len(r.TestedFiles) > 0` instead of the old `RepoHasTests` field, consistent with the struct change. Tests were added for both the new "no claim when nothing probed" case and the reworded output, which is good given the bug was subtle and behavior-order dependent (silent success case). No secrets, no unsafe injected input into HEAD requests (paths are file paths, not raw user text executed anywhere), and no breaking API changes beyond an internal struct field rename (`RepoHasTests` → `TestedFiles`), which is safe since this is an internal type. Nothing here looks alarming or accidentally committed.

rcsheets merged commit 33181b0b94 into main

2026-07-28 03:55:39 +00:00

rcsheets deleted branch fix/test-coverage-overclaim

2026-07-28 03:55:39 +00:00

rcsheets referenced this pull request from a commit

2026-07-28 03:55:41 +00:00

Merge pull request 'fix(repoanalysis): only claim test coverage for files we actually probed' (#87) from fix/test-coverage-overclaim into main