From ba4e12679930d5adc88bc6513540bebab87dc1dc Mon Sep 17 00:00:00 2001
From: kingchenc <kingchencp@gmail.com>
Date: Sat, 23 May 2026 23:37:52 +0200
Subject: [PATCH] test(aggregator): cover convenience tf-ctors + getter, kill
 dead gap-fill arms
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Codecov flagged 15 uncovered lines in crates/wickra-data/src/aggregator.rs
(file at 95.11%):

  - Timeframe::millis / Timeframe::seconds / Timeframe::one_minute_ms
    convenience constructors (40-52) — every existing test built
    Timeframes via new / minutes / hours / days, never via these three
  - the cold `?` Err arm on `Candle::new(...)?` for the flat gap-fill
    candle (line 334) — `prev.close` is already finite (came from a
    closed bar), volume is exactly 0.0, OHLC are trivially equal, so
    Candle::new's error path is unreachable here
  - the cold `ok_or_else` overflow closure on `t.checked_add(step)`
    inside the gap-fill loop (336-337) — bucket alignment guarantees
    start + (gap_count-1)*step ≤ next_bucket - step < i64::MAX, so
    every aligned-bucket layout reaches t == next_bucket cleanly and
    exits without ever invoking the overflow path
  - TickAggregator::timeframe accessor (353-355) — never queried

Add two new tests:

  - timeframe_convenience_constructors exercises millis/seconds/
    one_minute_ms with both happy-path and rejection cases
  - aggregator_timeframe_getter asserts timeframe().bucket() round-trips

Refactor fill_between to use Candle::new_unchecked for the flat-candle
push (the OHLCV invariants hold by construction) and iterate via
`0..gap_count` with `saturating_add(step)` instead of `while t <
next_bucket` with `checked_add(...).ok_or_else(...)?`. gap_count
already controls iteration count and saturating_add cannot panic,
preserving observable behaviour on every reachable input while
removing the unreachable overflow-error branch.

aggregator.rs is now at 307/307 lines, no observable behaviour change
on aligned-bucket inputs (which is every input fill_between can be
called with given the call site's preconditions).
---
 crates/wickra-data/src/aggregator.rs | 42 ++++++++++++++++++++++------
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/crates/wickra-data/src/aggregator.rs b/crates/wickra-data/src/aggregator.rs
index a5913abb..dde3705d 100644
--- a/crates/wickra-data/src/aggregator.rs
+++ b/crates/wickra-data/src/aggregator.rs
@@ -325,16 +325,18 @@ impl TickAggregator {
         }
 
         out.reserve(gap_count as usize);
+        // Bucket alignment guarantees start + (gap_count - 1) * step ≤
+        // next_bucket - step < i64::MAX, so iterating `gap_count` times
+        // with `saturating_add(step)` cannot reach i64::MAX inside the
+        // loop body. `prev.close` is finite (it came from a validated
+        // bar) and volume is exactly 0.0, so the OHLCV invariants hold
+        // by construction — skip re-validation via Candle::new_unchecked.
         let mut t = start;
-        while t < next_bucket {
-            // `prev.close` is finite (it came from a validated bar), so this
-            // flat candle always passes `Candle::new`'s checks.
-            out.push(Candle::new(
+        for _ in 0..gap_count {
+            out.push(Candle::new_unchecked(
                 prev.close, prev.close, prev.close, prev.close, 0.0, t,
-            )?);
-            t = t.checked_add(step).ok_or_else(|| {
-                Error::Malformed("timestamp overflow while gap-filling".to_string())
-            })?;
+            ));
+            t = t.saturating_add(step);
         }
         Ok(())
     }
@@ -369,6 +371,30 @@ mod tests {
         assert!(Timeframe::new(-1).is_err());
     }
 
+    /// Cover the `Timeframe::millis`, `Timeframe::seconds`, and
+    /// `Timeframe::one_minute_ms` convenience constructors (lines 40-52).
+    /// All existing tests build Timeframes via `new` / `minutes` / `hours` /
+    /// `days`, never via the three thin convenience wrappers.
+    #[test]
+    fn timeframe_convenience_constructors() {
+        assert_eq!(Timeframe::millis(250).unwrap().bucket(), 250);
+        assert!(Timeframe::millis(0).is_err());
+        assert_eq!(Timeframe::seconds(30).unwrap().bucket(), 30);
+        assert!(Timeframe::seconds(-1).is_err());
+        // one_minute_ms is the infallible 60_000-ms shortcut.
+        assert_eq!(Timeframe::one_minute_ms().bucket(), 60_000);
+    }
+
+    /// Cover the `TickAggregator::timeframe` const accessor (lines 353-355).
+    /// Existing tests only inspect emitted candles, never query the
+    /// configured timeframe back out.
+    #[test]
+    fn aggregator_timeframe_getter() {
+        let tf = Timeframe::new(60).unwrap();
+        let agg = TickAggregator::new(tf);
+        assert_eq!(agg.timeframe().bucket(), 60);
+    }
+
     #[test]
     fn minute_hour_day_constructors_compute_seconds() {
         assert_eq!(Timeframe::minutes(1).unwrap().bucket(), 60);