[COA-2971] Fix Union interval bounds validation to prevent panic#3
[COA-2971] Fix Union interval bounds validation to prevent panic#3
Conversation
The Union function computes intersection intervals by taking max(start) and min(end) of two intervals. However, when intervals don't actually overlap (e.g., [90,95] and [80,88]), this produces invalid intervals where start > end (e.g., [90,88]). This causes a panic in TotalWhitespace when it tries to slice with invalid bounds: `lines[90:88]` -> "slice bounds out of range [90:88]" The fix validates that start <= end before appending to the result, which correctly handles non-overlapping intervals by excluding them. Added test cases for non-overlapping and adjacent intervals.
|
Coverage on new code: 75% |
VerificationThis fix has been tested on rewardStyle/engx#4. ❌ Before fix (using
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a panic in the interval.Union() function that occurred when processing non-overlapping intervals. The function computes interval intersections using max(a.Start, b.Start) for the start and min(a.End, b.End) for the end. When intervals don't actually overlap, this can produce invalid intervals where start > end, which causes panics in downstream functions like TotalWhitespace().
The fix adds defensive validation to check start <= end before appending intervals, and includes test cases for non-overlapping and adjacent interval scenarios that previously caused panics.
Changes:
- Added bounds validation in
interval.Union()to prevent creating invalid intervals wherestart > end - Added test case for non-overlapping intervals
- Added test case for adjacent but non-overlapping intervals
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| interval/interval.go | Added defensive check to validate start <= end before appending intervals in Union function |
| interval/interval_test.go | Added two test cases to verify non-overlapping intervals produce empty results |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
What
interval.Union()to checkstart <= endbefore appending intervals to the resultWhy
The Problem
The
Unionfunction panics when processing certain file/coverage combinations:This was discovered in rewardStyle/engx#4.
Root Cause
The
Unionfunction computes interval intersections by independently calculating:start = max(a.Start, b.Start)end = min(a.End, b.End)When two intervals don't actually overlap, this produces an invalid interval where
start > end:This invalid interval is passed to
TotalWhitespace()which attemptslines[90:88], causing the panic.Why This Bug Hasn't Been Seen Before
Every repo using build-utils has these default coverage filters:
These filters are applied when creating
parsed_coverage.out:This means
/cmd/files are excluded from coverage analysis in every existing repo.The engx repo is a CLI tool that lives entirely in
cmd/ktl/. It doesn't use build-utils, so it passed unfiltered coverage data to cov-diff. This is the first time cov-diff has ever processed/cmd/files, which happen to have interval patterns that trigger this edge case.The Fix
Add validation to ensure
start <= endbefore appending:This correctly handles non-overlapping intervals by excluding them from the result.