BOT: Fix #454: Deduplicate interval coverage bounds-check logic#1097
BOT: Fix #454: Deduplicate interval coverage bounds-check logic#1097nikosbosse wants to merge 2 commits intomainfrom
Conversation
Extract shared bounds-check `(observed >= lower) & (observed <= upper)` into `check_interval_coverage()` helper, used by both `get_coverage()` and `interval_coverage()` to eliminate duplicated logic. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1097 +/- ##
==========================================
+ Coverage 97.83% 97.98% +0.15%
==========================================
Files 35 37 +2
Lines 1845 1985 +140
==========================================
+ Hits 1805 1945 +140
Misses 40 40 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
nikosbosse
left a comment
There was a problem hiding this comment.
CLAUDE: Approve — Clean, minimal refactoring that correctly extracts the duplicated bounds-check logic into a shared internal helper check_interval_coverage(). The change is purely mechanical with no behavioral impact. Both original implementations computed the same expression — the helper standardizes the operand order. Well-guarded by 6 new regression tests covering cross-validation between code paths, boundary cases, and full-dataset comparison against score() output. No issues found.
|
@seabbs minor refactoring, neither does much harm nor much good, I'd say |
- Fix indentation alignment (hanging indent and continuation lines) - Replace redundant `== TRUE` / `== FALSE` with direct logical usage - Replace `expect_equal(x, TRUE)` with `expect_true(x)` Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
(observed >= lower) & (observed <= upper)into a shared internal helpercheck_interval_coverage()get_coverage()(R/get-coverage.R) andinterval_coverage()(R/metrics-quantile.R) now call this helper instead of independently implementing the same logicFixes #454
Root cause
Both
get_coverage()andinterval_coverage()independently implemented the same interval bounds check after callingquantile_to_interval(). The logic was identical but the operand order differed (observed <= upper & observed >= lowervsobserved >= lower & observed <= upper).What the fix does
check_interval_coverage(observed, lower, upper)inR/helper-quantile-interval-range.Rget_coverage()andinterval_coverage()to call the shared helper(observed >= lower) & (observed <= upper)Test coverage added
get_coverage()interval coverage matchesinterval_coverage()for same dataget_coverage()vsscore()🤖 Generated with Claude Code