brooktrails/pr-reviewer

Watch

Fork 0

fix(diffanalysis): don't count deleted tests as tests included #89

Merged

rcsheets merged 1 commit from fix/test-signal-deletions into main

2026-07-28 04:36:43 +00:00

rcsheets commented

2026-07-28 03:49:18 +00:00

Owner

TestFilesChanged counted every touched test file regardless of whether the diff added it or removed it, and HasTests keyed off that total.

A PR whose only test-file change was a deletion rendered:

- **Tests included**: 1 test files changed

Ripping tests out reported as the reassuring signal, on the line the model most likely reads as a proxy for whether the change was tested.

HasTests is now net of deletions, the count reads "added or modified", and deleted test files get their own line so the removal is visible instead of absorbed into a positive number. A PR that adds one test and deletes another reports both.

🤖 Generated with Claude Code

`TestFilesChanged` counted every touched test file regardless of whether the diff added it or removed it, and `HasTests` keyed off that total. A PR whose only test-file change was a deletion rendered: ``` - **Tests included**: 1 test files changed ``` Ripping tests out reported as the reassuring signal, on the line the model most likely reads as a proxy for whether the change was tested. `HasTests` is now net of deletions, the count reads "added or modified", and deleted test files get their own line so the removal is visible instead of absorbed into a positive number. A PR that adds one test and deletes another reports both. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

rcsheets added 1 commit

2026-07-28 03:49:18 +00:00

fix(diffanalysis): don't count deleted tests as tests included

ci / check (pull_request) Successful in 43s

Details

97fa477ed0

TestFilesChanged counted every touched test file regardless of whether
the diff added it or removed it, and HasTests keyed off that total. A PR
whose only test-file change was a deletion rendered "**Tests included**:
1 test files changed" — ripping tests out reported as the reassuring
signal, on the line the model most likely reads as a proxy for whether
the change was tested.

HasTests is now net of deletions, the count says "added or modified", and
deleted test files get their own line so the removal is visible rather
than absorbed into a positive number.

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

pr-reviewer-bot commented

2026-07-28 03:49:19 +00:00

Collaborator

Automated review by pr-reviewer v0.41.3 | Safety Check | Claude Sonnet 5 | tracking id r-6826be-ecee5f
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 — Small, correct, well-tested bugfix with no security or breaking-change concerns.

The change is a small, well-targeted logic fix: TestFilesChanged no longer conflates deletions with additions for the HasTests/"Tests included" signal, and deleted test files now get their own line. Logic looks correct:

HasTests = TestFilesChanged - TestFilesDeleted > 0 correctly nets out deletions.
Format's "Tests included" line now reports the net count with clearer wording.
Deleted test files get a separate line, shown regardless of HasTests, so mixed add+delete scenarios report both.
New tests cover both the pure-deletion case and the mixed add/delete case, and they pass logically based on the diff.

No security issues, no accidentally committed files, no breaking API changes beyond an added struct field (TestFilesDeleted) and a wording change in Format's output, which is the intended fix. Nothing alarming here.

*Automated review by [pr-reviewer](https://git.brooktrails.org/brooktrails/pr-reviewer) v0.41.3 | Safety Check | Claude Sonnet 5 | tracking id `r-6826be-ecee5f`* *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** — Small, correct, well-tested bugfix with no security or breaking-change concerns. The change is a small, well-targeted logic fix: `TestFilesChanged` no longer conflates deletions with additions for the `HasTests`/"Tests included" signal, and deleted test files now get their own line. Logic looks correct: - `HasTests = TestFilesChanged - TestFilesDeleted > 0` correctly nets out deletions. - Format's "Tests included" line now reports the net count with clearer wording. - Deleted test files get a separate line, shown regardless of `HasTests`, so mixed add+delete scenarios report both. - New tests cover both the pure-deletion case and the mixed add/delete case, and they pass logically based on the diff. No security issues, no accidentally committed files, no breaking API changes beyond an added struct field (`TestFilesDeleted`) and a wording change in Format's output, which is the intended fix. Nothing alarming here.

rcsheets force-pushed fix/test-signal-deletions from 97fa477ed0

ci / check (pull_request) Successful in 43s

Details

to 7f596d4e19

ci / check (pull_request) Successful in 42s

Details

2026-07-28 04:35:52 +00:00

Compare

pr-reviewer-bot commented

2026-07-28 04:35:53 +00:00

Collaborator

Automated review by pr-reviewer v0.41.4 | Safety Check | Claude Sonnet 5 | tracking id r-6831a9-76028b
This is an AI-generated review and may contain mistkaes.

Status: ✅ Completed

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

✅ Verdict: LGTM — Straightforward, well-tested bugfix with no signs of unrelated or risky changes.

This is a small, well-scoped fix: HasTests/TestFilesChanged now correctly net out deleted test files, and the format output surfaces deletions separately instead of silently counting them as positive test signal. Logic looks correct (TestFilesChanged - TestFilesDeleted > 0), tests cover the deleted-only and mixed add+delete cases, and no unrelated changes or secrets are present. Nothing alarming here.

*Automated review by [pr-reviewer](https://git.brooktrails.org/brooktrails/pr-reviewer) v0.41.4 | Safety Check | Claude Sonnet 5 | tracking id `r-6831a9-76028b`* *This is an AI-generated review and may contain mistkaes.* **Status:** ✅ Completed --- > ⚠️ The primary review (**Mistral Small**) failed. The review below is from **Claude Sonnet 5**, which reviewed the same diff. **✅ Verdict: LGTM** — Straightforward, well-tested bugfix with no signs of unrelated or risky changes. This is a small, well-scoped fix: `HasTests`/`TestFilesChanged` now correctly net out deleted test files, and the format output surfaces deletions separately instead of silently counting them as positive test signal. Logic looks correct (`TestFilesChanged - TestFilesDeleted > 0`), tests cover the deleted-only and mixed add+delete cases, and no unrelated changes or secrets are present. Nothing alarming here.

rcsheets merged commit b9fdd7da72 into main

2026-07-28 04:36:43 +00:00

rcsheets deleted branch fix/test-signal-deletions

2026-07-28 04:36:43 +00:00

rcsheets referenced this pull request from a commit

2026-07-28 04:36:43 +00:00

Merge pull request 'fix(diffanalysis): don't count deleted tests as tests included' (#89) from fix/test-signal-deletions into main