From e6508716a1733c7271482d70eeacc348f5160375 Mon Sep 17 00:00:00 2001 From: Samuel Schlesinger Date: Thu, 20 Aug 2026 18:47:12 -0400 Subject: [PATCH 1/2] test: Pin range clamping at both ends of a frame range clips the start against [0, rows] and the length against [0, rows] independently, instead of clipping the length against the rows that remain after the start. An end past the last row slices out of bounds (range (8, 20) on ten rows dies with "invalid slice (8,10,10)"), and a negative start stretches the slice by the underflow (range (-5, 3) returns eight rows instead of three). Subtracting before clamping also wraps: range (1, minBound) reopens the range instead of emptying it. The property covers arbitrary endpoints, with an oracle that clamps before it subtracts for the same reason. --- tests/Main.hs | 5 ++++ tests/Operations/Subset.hs | 51 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/tests/Main.hs b/tests/Main.hs index 7689e2f7..e8adcce4 100644 --- a/tests/Main.hs +++ b/tests/Main.hs @@ -143,6 +143,10 @@ main = do mapM (quickCheckWithResult stdArgs) Operations.Subset.tests + subsetPropRes <- + mapM + (quickCheckWithResult stdArgs) + Operations.Subset.properties monadRes <- mapM (quickCheckWithResult stdArgs) Monad.tests cbRes <- mapM @@ -151,6 +155,7 @@ main = do propsRes <- mapM (quickCheckWithResult stdArgs) Properties.tests catRes <- mapM (quickCheckWithResult stdArgs) Properties.Categorical.tests if not (all isSuccessful propRes) + || not (all isSuccessful subsetPropRes) || not (all isSuccessful cbRes) || not (all isSuccessful monadRes) || not (all isSuccessful propsRes) diff --git a/tests/Operations/Subset.hs b/tests/Operations/Subset.hs index cef527f0..aa47fb9b 100644 --- a/tests/Operations/Subset.hs +++ b/tests/Operations/Subset.hs @@ -9,8 +9,10 @@ import qualified DataFrame as D import qualified DataFrame.Internal.Column as Col import DataFrame.Internal.DataFrame import DataFrame.Operations.Merge () +import GenDataFrame () import System.Random import Test.HUnit +import Test.QuickCheck (Property, property) prop_dropZero :: DataFrame -> Bool prop_dropZero df = D.drop 0 df == df @@ -53,6 +55,17 @@ prop_rangeFull df = let rows = fst (dataframeDimensions df) in D.range (0, rows) df == df +prop_rangeClampsToBounds :: DataFrame -> Int -> Int -> Bool +prop_rangeClampsToBounds df a b = + fst (dataframeDimensions (D.range (a, b) df)) == expected + where + rows = fst (dataframeDimensions df) + -- Rows in [a, b) that actually exist. Clamps before subtracting so the + -- oracle itself cannot overflow on extreme endpoints. + lo = min (max a 0) rows + hi = min (max b lo) rows + expected = hi - lo + prop_selectAll :: DataFrame -> Bool prop_selectAll df = D.select (D.columnNames df) df == df @@ -177,6 +190,37 @@ unit_stratifiedSplit_proportions = ) (abs (vaProp - origProp) < tol) +tenRows :: DataFrame +tenRows = fromNamedColumns [("x", Col.fromList ([0 .. 9] :: [Int]))] + +-- Endpoints that overflow Int if the length is computed before clamping. +unit_rangeExtremeEndpoints :: Test +unit_rangeExtremeEndpoints = + TestCase + ( assertEqual + "range (1, minBound) is empty, not a wrapped-around full range" + 0 + (fst (dataframeDimensions (D.range (1, minBound) tenRows))) + ) + +unit_rangeEndPastEnd :: Test +unit_rangeEndPastEnd = + TestCase + ( assertEqual + "range (8, 20) on a 10-row frame yields rows 8 and 9" + (Just (Col.fromList ([8, 9] :: [Int]))) + (getColumn "x" (D.range (8, 20) tenRows)) + ) + +unit_rangeStartBeforeZero :: Test +unit_rangeStartBeforeZero = + TestCase + ( assertEqual + "range (-5, 3) on a 10-row frame yields rows 0 to 2" + (Just (Col.fromList ([0, 1, 2] :: [Int]))) + (getColumn "x" (D.range (-5, 3) tenRows)) + ) + hunitTests :: [Test] hunitTests = [ TestLabel "unit_stratifiedSample_full" unit_stratifiedSample_full @@ -185,8 +229,15 @@ hunitTests = "unit_stratifiedSplit_singleRowStratum" unit_stratifiedSplit_singleRowStratum , TestLabel "unit_stratifiedSplit_proportions" unit_stratifiedSplit_proportions + , TestLabel "unit_rangeEndPastEnd" unit_rangeEndPastEnd + , TestLabel "unit_rangeExtremeEndpoints" unit_rangeExtremeEndpoints + , TestLabel "unit_rangeStartBeforeZero" unit_rangeStartBeforeZero ] +-- Properties whose shape does not fit [DataFrame -> Bool]. +properties :: [Property] +properties = [property prop_rangeClampsToBounds] + tests :: [DataFrame -> Bool] tests = [ prop_dropZero From a981c8901f195f508b1f9612e14300b7629054c0 Mon Sep 17 00:00:00 2001 From: Samuel Schlesinger Date: Thu, 20 Aug 2026 18:47:12 -0400 Subject: [PATCH 2/2] fix: Clamp both of range's endpoints before subtracting The start and the length were clipped against the row count independently, so an end past the last row produced a slice running off the end of the column and a negative start widened the slice by the underflow. Subtracting first also let a very negative end wrap to a large positive length. Clamp the start, then the end against it, and take the difference. --- dataframe-operations/src/DataFrame/Operations/Subset.hs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/dataframe-operations/src/DataFrame/Operations/Subset.hs b/dataframe-operations/src/DataFrame/Operations/Subset.hs index d5a53b51..a1b4d5b7 100644 --- a/dataframe-operations/src/DataFrame/Operations/Subset.hs +++ b/dataframe-operations/src/DataFrame/Operations/Subset.hs @@ -176,12 +176,16 @@ dropLast n d = range :: (Int, Int) -> DataFrame -> DataFrame range (start, end) d = d - { columns = V.map (sliceColumn (clip start 0 r) n') (columns d) + { columns = V.map (sliceColumn start' n') (columns d) , dataframeDimensions = (n', c) } where (r, c) = dataframeDimensions d - n' = clip (end - start) 0 r + start' = clip start 0 r + -- Clamp both endpoints before subtracting: end - start' on an unclamped + -- end wraps for very negative values and reopens the range. + end' = clip end start' r + n' = end' - start' clip :: Int -> Int -> Int -> Int clip n left right = min right $ max n left