Skip to content

fix(validation): catch bad input at the public boundary with specific error messages #592

Closed

Description

@murray-ds

Noticed while watching coding agents work from the installed package. These errors come from 240 agent runs over 15 retail-analysis tasks, plus a source audit. Checked on main @ c5f4b2a; every message below was reproduced.

What: when a caller passes the wrong input, ORS often doesn't catch it. The mistake surfaces later as an ibis, DuckDB or pandas error that names internals rather than the cause, or as an ORS message that states a rule but not what was passed or how to fix it. Each one cost the agents retries. The worst took 6 to 17 turns, and some runs never recovered.

Errors seen in the runs, ranked by runs affected:

Error the caller sees Runs Real mistake Where to catch it Suggested message
ValueError: Unknown option: transaction_date (also column, column.category_0_name, store_id) 18 The option name is missing its column. prefix, or doesn't exist options.py:287, 307, 320 Unknown option 'transaction_date'. Did you mean 'column.transaction_date'? See list_options(). (difflib.get_close_matches)
AttributeError: 'StringColumn' object has no attribute 'truncate' 15 transaction_date is a string, e.g. straight from read_csv cohort.py:174, with no dtype check transaction_date must be a date or datetime column, got string. Convert it with pd.to_datetime first.
ValueError: The number of thresholds must match the number of segments 14 thresholds=[0.5, 0.8] for 3 segments; the last threshold must be 1.0 threshold.py:65-66 Got 2 thresholds for 3 segments. thresholds are ascending percentile upper bounds, one per segment, ending at 1.0, e.g. [0.5, 0.8, 1.0].
ValueError: The name customer_id occurs multiple times, use a level number 6 GainLoss(group_col="customer_id"); the class already groups by customer gain_loss.py:76-78 group_col must not be the customer column; GainLoss already groups by customer.
KeyError: 'customers_p2', or ValueError: Length mismatch: Expected axis has 1 elements, new values have 2 elements 7 p1_value/p2_value matches no row of period_col (wrong format or type, or a raw date column) revenue_tree.py:279-313 p2_value='2023-Q3' matches no rows of period_col 'quarter'. Values present: '2023Q1', '2023Q2', …
IbisInputError: Duplicate column name 'customer_id' in result set 4 ThresholdSegmentation(group_col="customer_id") threshold.py:72-85 group_col must not include the customer column; segmentation is already per customer.
TypeError: Unable to normalize 'Total' to Int64(nullable=True) 3 SegTransactionStats on an integer segment_col: the default rollup_value="Total" can't be cast to it segstats.py:379 rollup_value 'Total' can't be stored in integer column 'customer_id'. Pass calc_total=False, a numeric rollup_value, or cast the column to str.
Columns ['Skincare'] in grouping_sets not found in segment_col; Each element must be a tuple. Got: str 3 Segment values or bare strings passed in place of tuples of column names segstats.py:440, 568-571 Add the form: grouping_sets entries are tuples of segment_col names, e.g. [("category_1_name",), ()], or "total"/"rollup"/"cube".
AttributeError: module 'ibis.expr.datatypes' has no attribute 'Period' 2 A pandas period[M] column validation.py:57 (ibis.memtable) Column 'month' has dtype period[M], which isn't supported. Convert it with .astype(str) or .dt.to_timestamp().
ValueError: p1_index and p2_index should not overlap 1 (17 turns) df indexed by a repeated customer_id; the masks themselves are disjoint gain_loss.py:65-69 compares index labels Check the masks with (p1_index & p2_index).any(), and require a unique index: df.index must be unique; call df.reset_index() first.
ConversionException: Could not convert string 'La Roche-Posay' to INT64; target_item must contain only str or float values. Got numpy.int64 1 target_item doesn't match value_col's type, and int ids are rejected product_association.py:163 Accept ints. Otherwise say target_item 'La Roche-Posay' doesn't match value_col 'product_id' (int64).

Found in the source audit, reproduced:

  • Wrong agg_func name. It is checked in NLRSegmentation and CompositeRank only. CohortAnalysis, ThresholdSegmentation, CrossShop and GainLoss give a raw AttributeError: 'FloatingColumn' object has no attribute 'summ'.

  • Non-date transaction_date in RFMSegmentation.

    • Integer dates dump SQL: ConversionException: ... (BIGINT -> DATE) ... LINE 1: ...make_date(2024, 12, 31)) AS INTEGER) AS recency_days.
    • ISO strings silently work.
    • Only customer.py calls ensure_tznaive_datetime.
  • RFMSegmentation(current_date="31/12/2024") gives ValueError: Invalid isoformat string: '31/12/2024', which doesn't name current_date.

  • GainLoss mask length. The length check runs after the masks are used, so it never fires. A short list gives Item wrong length 59 instead of 60.

  • SegTransactionStats with a string unit_spend gives 'StringColumn' object has no attribute 'sum'.

  • ThresholdSegmentation input it doesn't check, accepted silently:

    • thresholds that don't end at 1.0 leave the top customers with segment None;
    • unsorted thresholds put everyone in one segment;
    • a value above 1 is accepted;
    • a typo in zero_value_customers is treated as include_with_light.
  • Rules without the value passed.

    • product_association.py L108-116: "Minimum support must be between 0 and 1".
    • rfm.py L156-189: "min_frequency must be an integer".
    • customer_decision_hierarchy.py L125, L129, L210.
    • cross_shop.py L92, L155.

    These should add Got {value!r}. The ensure_* helpers in core/validation.py already do this.

  • CohortAnalysis given a non-DataFrame fails with 'list' object has no attribute 'columns', because it checks columns before ensure_ibis_table.

  • option_context("column.customer_id") says "requires an even number of arguments" without saying how many were passed.

Ask: validate at the public boundary, in a few shared helpers, so every class gets the same messages:

  1. ensure_date_column(df, col): date or datetime, with the pd.to_datetime remedy. Call it from every class that reads transaction_date: CohortAnalysis, RFMSegmentation, CLVStats, … (ensure_tznaive_datetime already has the message).
  2. ensure_agg_func(table, col, func): reuse the probe from ensure_valid_extra_aggs. Name the parameter, the value and the valid options.
  3. A guard that group_col doesn't include the customer column, for the classes that already group by customer.
  4. A check that p1_value/p2_value (and target_item) appear in the data, echoing sample values.
  5. One raise site for unknown options, with a did-you-mean suggestion.
  6. ensure_ibis_table first in every constructor.
  7. Every rule message names the parameter and the value passed.

One parametrized test per helper, over every class that uses it, keeps new classes covered.

Related: #536 (NULL/NaT dates), #590 (exports), #591 (docstring contracts).

Activity

  1. murray-ds commented on Oct 22, 2026

    @murray-ds
    ContributorAuthor

    Failing tests for the error-message cases above. All 12 fail on main @ c5f4b2a.

    The two RFMSegmentation date cases don't raise at construction at all: the DuckDB ConversionException only appears when .df is read, which is a further reason to validate at the boundary.

    The bugs these checks turned up are filed separately: #593, #594, #595, #600, #601. The suspected SegTransactionStats failure with no customer column did not reproduce: without a customer column it works and drops the customer metrics. Only an integer segment_col with the default rollup_value fails.

    import pandas as pd
    import pytest
    
    from openretailscience.analysis.cohort import CohortAnalysis
    from openretailscience.analysis.gain_loss import GainLoss
    from openretailscience.options import ColumnHelper
    from openretailscience.segmentation.rfm import RFMSegmentation
    from openretailscience.segmentation.segstats import SegTransactionStats
    from openretailscience.segmentation.threshold import ThresholdSegmentation
    
    cols = ColumnHelper()
    
    
    def _transactions(dates: list) -> pd.DataFrame:
        return pd.DataFrame(
            {
                cols.customer_id: [101, 101, 102, 103],
                cols.transaction_id: [9001, 9002, 9003, 9004],
                cols.unit_spend: [24.99, 61.50, 12.75, 149.00],
                cols.store_id: [11, 11, 24, 24],
                cols.transaction_date: dates,
            },
        )
    
    
    DATES = pd.to_datetime(["2024-12-30", "2024-11-02", "2024-06-15", "2024-12-31"])
    
    
    class TestDateColumnErrors:
        """A non-date transaction_date is rejected with an error naming the column."""
    
        @pytest.mark.parametrize(
            "dates",
            [
                pytest.param(["2024-01-05", "2024-02-11", "2024-02-20", "2024-03-02"], id="iso_string"),
                pytest.param([20240105, 20240211, 20240220, 20240302], id="integer_yyyymmdd"),
            ],
        )
        def test_cohort_analysis(self, dates):
            with pytest.raises((TypeError, ValueError), match=cols.transaction_date):
                CohortAnalysis(_transactions(dates), aggregation_column=cols.customer_id, period="month")
    
        @pytest.mark.parametrize(
            "dates",
            [
                pytest.param([20241230, 20241102, 20240615, 20241231], id="integer_yyyymmdd"),
                pytest.param(["12/30/2024", "11/02/2024", "06/15/2024", "12/31/2024"], id="non_iso_string"),
            ],
        )
        def test_rfm_segmentation(self, dates):
            with pytest.raises((TypeError, ValueError), match=cols.transaction_date):
                RFMSegmentation(_transactions(dates), current_date="2024-12-31", r_segments=2, f_segments=2, m_segments=2)
    
    
    class TestParameterNamedInError:
        """Errors name the parameter the caller got wrong."""
    
        @pytest.mark.parametrize("current_date", ["31/12/2024", "2024-13-01"])
        def test_rfm_invalid_current_date(self, current_date):
            with pytest.raises(ValueError, match="current_date"):
                RFMSegmentation(_transactions(DATES), current_date=current_date, r_segments=2, f_segments=2, m_segments=2)
    
        def test_threshold_segmentation_invalid_agg_func(self):
            with pytest.raises(ValueError, match="agg_func"):
                ThresholdSegmentation(_transactions(DATES), thresholds=[0.5, 1.0], segments=["Light", "Heavy"], agg_func="summ")
    
        def test_cohort_analysis_invalid_agg_func(self):
            with pytest.raises(ValueError, match="agg_func"):
                CohortAnalysis(_transactions(DATES), aggregation_column=cols.unit_spend, agg_func="summ")
    
        @pytest.mark.parametrize("segment_col", [cols.store_id, cols.customer_id])
        def test_seg_transaction_stats_integer_segment_default_rollup(self, segment_col):
            with pytest.raises(ValueError, match="rollup_value"):
                SegTransactionStats(_transactions(DATES), segment_col=segment_col, calc_total=True)
    
        def test_gain_loss_group_col_is_customer(self):
            df = _transactions(DATES).assign(brand=["Coke", "Pepsi", "Coke", "Pepsi"])
            with pytest.raises(ValueError, match="group_col"):
                GainLoss(
                    df=df,
                    p1_index=df[cols.transaction_date] < "2024-07-01",
                    p2_index=df[cols.transaction_date] >= "2024-07-01",
                    focus_group_index=df["brand"] == "Coke",
                    focus_group_name="Coke",
                    comparison_group_index=df["brand"] == "Pepsi",
                    comparison_group_name="Pepsi",
                    group_col=cols.customer_id,
                )
    
    
    class TestInputTypeErrors:
        def test_cohort_analysis_rejects_non_dataframe(self):
            with pytest.raises(TypeError, match="DataFrame"):
                CohortAnalysis([1, 2, 3], aggregation_column=cols.unit_spend)
  2. added a commit that references this issue on Oct 29, 2026
    9fe8f14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    type:enhancementImprovement to existing functionality

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions