fix: address PR review feedback for validation functions

1. Fix Friday logic edge case comment - Clarified get_default_backtest_dates() docstring - Removed confusing 'at least a week old' comment - Explicitly documented Friday behavior 2. Reorder validation checks in validate_date_range_for_symbol() - Now checks start > end first (most fundamental) - Then checks end > today (future dates) - Finally checks symbol-specific bounds - Users get most actionable error first 3. Add server-side numeric bounds validation - New validate_numeric_inputs() function - Validates units > 0, loan >= 0, 0 < LTV < 1 - Called in run_backtest() before service call 4. Add boundary tests - Test start_date exactly at SYMBOL_MIN_DATES boundary - Test same-day date range (start == end) - Test end_date exactly today - Test end_date tomorrow (future) - Test validation order returns most actionable error - Test near-zero and large values for units calculation - Test LTV at boundaries (0, 1, 0.01, 0.99) 5. Add tests for validate_numeric_inputs - Valid inputs, zero/negative values - LTV boundary conditions
2026-03-29 19:29:46 +02:00
parent f9ea7f0b67
commit 269745cd3e
2 changed files with 191 additions and 13 deletions
--- a/app/pages/backtests.py
+++ b/app/pages/backtests.py
@@ -49,13 +49,18 @@ SYMBOL_MIN_DATES = {
 }

 def get_default_backtest_dates() -> tuple[date, date]:
-    """Get default backtest date range (last 2 years excluding current week)."""
+    """Get default backtest date range (~2 years ending on most recent Friday or earlier).
+    
+    Returns dates (start, end) where:
+    - end is the most recent Friday (including today if today is Friday)
+    - start is ~730 days before end
+    """
    today = date.today()
-    # Find the most recent Friday that's at least a week old
+    # Find days since most recent Friday
    days_since_friday = (today.weekday() - 4) % 7
-    if days_since_friday == 0 and today.weekday() != 4:
-        # Not Friday yet, go back to previous Friday
-        days_since_friday = 7
+    # If today is Friday (weekday 4), days_since_friday is 0, so end = today
+    # If today is Saturday (weekday 5), days_since_friday is 1, so end = yesterday (Friday)
+    # etc.
    end = today - timedelta(days=days_since_friday)
    start = end - timedelta(days=730)  # ~2 years
    return start, end
@@ -69,14 +74,37 @@ def validate_date_range_for_symbol(start_date: date, end_date: date, symbol: str
    """Validate date range is within available data for symbol.

    Returns error message if invalid, None if valid.
+    
+    Validation order:
+    1. Logical order (start <= end)
+    2. End not in future
+    3. Symbol-specific data availability
    """
+    if start_date > end_date:
+        return "Start date must be before or equal to end date."
+    if end_date > date.today():
+        return "End date cannot be in the future."
    min_date = SYMBOL_MIN_DATES.get(symbol)
    if min_date and start_date < min_date:
        return f"Start date must be on or after {min_date.strftime('%Y-%m-%d')} for {symbol} (data availability)."
-    if end_date > date.today():
-        return "End date cannot be in the future."
-    if start_date > end_date:
-        return "Start date must be before or equal to end date."
+    return None
+
+
+def validate_numeric_inputs(
+    units: float,
+    loan_amount: float,
+    margin_call_ltv: float,
+) -> str | None:
+    """Validate numeric inputs for backtest scenario.
+
+    Returns error message if invalid, None if valid.
+    """
+    if units <= 0:
+        return "Underlying units must be positive."
+    if loan_amount < 0:
+        return "Loan amount cannot be negative."
+    if not (0 < margin_call_ltv < 1):
+        return "Margin call LTV must be between 0 and 1 (exclusive)."
    return None


@@ -781,6 +809,16 @@ def _render_backtests_page(workspace_id: str | None = None) -> None:
                    render_result_state("Scenario validation failed", date_range_error, tone="warning")
                    return

+                # Validate numeric inputs
+                units = float(units_input.value or 0.0)
+                loan = float(loan_input.value or 0.0)
+                ltv = float(ltv_input.value or 0.0)
+                numeric_error = validate_numeric_inputs(units, loan, ltv)
+                if numeric_error:
+                    validation_label.set_text(numeric_error)
+                    render_result_state("Input validation failed", numeric_error, tone="warning")
+                    return
+
                # Save settings before running
                save_backtest_settings()

@@ -789,9 +827,9 @@ def _render_backtests_page(workspace_id: str | None = None) -> None:
                    start_date=start_date,
                    end_date=end_date,
                    template_slug=str(template_select.value or ""),
-                    underlying_units=float(units_input.value or 0.0),
-                    loan_amount=float(loan_input.value or 0.0),
-                    margin_call_ltv=float(ltv_input.value or 0.0),
+                    underlying_units=units,
+                    loan_amount=loan,
+                    margin_call_ltv=ltv,
                )
                # Update cost in saved settings after successful run
                if str(data_source_select.value) == "databento":
--- a/tests/test_page_validation.py
+++ b/tests/test_page_validation.py
@@ -3,10 +3,13 @@ from __future__ import annotations

 from datetime import date, timedelta

+import pytest
+
 from app.pages.backtests import (
    SYMBOL_MIN_DATES,
    get_default_backtest_dates,
    validate_date_range_for_symbol,
+    validate_numeric_inputs,
 )
 from app.pages.event_comparison import validate_and_calculate_units

@@ -171,4 +174,141 @@ class TestSymbolMinDates:
        # GC (futures have longer history)
        assert SYMBOL_MIN_DATES["GC"].year <= 1980
        # XAU (index historical)
-        assert SYMBOL_MIN_DATES["XAU"].year <= 1980
+        assert SYMBOL_MIN_DATES["XAU"].year <= 1980
+
+
+class TestValidateDateRangeForSymbolBoundaryCases:
+    """Boundary tests for validate_date_range_for_symbol.
+
+    These tests verify edge cases like exact boundary dates and same-day ranges.
+    """
+
+    def test_start_date_exactly_at_min_date_is_valid(self) -> None:
+        """Test start_date exactly at SYMBOL_MIN_DATES passes validation."""
+        # GLD min date is 2004-11-18, use that exact date
+        start = SYMBOL_MIN_DATES["GLD"]
+        end = date(2006, 11, 18)
+        error = validate_date_range_for_symbol(start, end, "GLD")
+        assert error is None
+
+    def test_same_day_date_range_is_valid(self) -> None:
+        """Test start_date == end_date is valid (single-day range)."""
+        # Use a valid date within GLD range
+        single_day = date(2020, 6, 15)
+        error = validate_date_range_for_symbol(single_day, single_day, "GLD")
+        assert error is None
+
+    def test_end_date_exactly_today_is_valid(self) -> None:
+        """Test end_date == today passes validation."""
+        today = date.today()
+        start = today - timedelta(days=365)
+        error = validate_date_range_for_symbol(start, today, "GLD")
+        assert error is None
+
+    def test_end_date_tomorrow_is_invalid(self) -> None:
+        """Test end_date == tomorrow + 1 is caught as future date."""
+        today = date.today()
+        start = today - timedelta(days=30)
+        end = today + timedelta(days=1)
+        error = validate_date_range_for_symbol(start, end, "GLD")
+        assert error is not None
+        assert "future" in error.lower()
+
+    def test_validation_order_returns_most_actionable_error(self) -> None:
+        """Test that start > end is caught before symbol-specific bounds."""
+        # Inverted range that's also before GLD launch
+        start = date(2000, 1, 1)  # Before GLD launch
+        end = date(1999, 1, 1)   # Before start
+        error = validate_date_range_for_symbol(start, end, "GLD")
+        # Should get "start > end" error, not "before min date" error
+        assert error is not None
+        assert "before" in error.lower() or "equal to" in error.lower()
+        assert "future" not in error.lower()  # Not the future date error
+
+
+class TestValidateAndCalculateUnitsBoundaryCases:
+    """Boundary tests for validate_and_calculate_units.
+
+    These tests verify edge cases like near-zero values and precision.
+    """
+
+    def test_very_small_positive_values_calculate_correctly(self) -> None:
+        """Test near-zero positive values don't underflow."""
+        units, error = validate_and_calculate_units(1e-10, 1.0)
+        assert error is None
+        assert units == pytest.approx(1e-10)
+
+    def test_small_dividend_large_divisor(self) -> None:
+        """Test precision with very small quotient."""
+        units, error = validate_and_calculate_units(0.001, 1_000_000.0)
+        assert error is None
+        assert units == pytest.approx(1e-9)
+
+    def test_very_large_values_calculate_correctly(self) -> None:
+        """Test large portfolio values don't cause issues."""
+        units, error = validate_and_calculate_units(1e15, 1.0)
+        assert error is None
+        assert units == pytest.approx(1e15)
+
+    def test_fractional_entry_spot(self) -> None:
+        """Test fractional entry spot calculates correctly."""
+        units, error = validate_and_calculate_units(1000.0, 3.14159)
+        assert error is None
+        assert units == pytest.approx(318.31, abs=0.01)
+
+
+class TestValidateNumericInputs:
+    """Tests for validate_numeric_inputs from backtests.py."""
+
+    def test_valid_inputs_return_none(self) -> None:
+        """Test valid inputs return None (no error)."""
+        error = validate_numeric_inputs(1000.0, 50000.0, 0.75)
+        assert error is None
+
+    def test_zero_units_returns_error(self) -> None:
+        """Test zero units returns error."""
+        error = validate_numeric_inputs(0.0, 50000.0, 0.75)
+        assert error is not None
+        assert "positive" in error.lower()
+
+    def test_negative_units_returns_error(self) -> None:
+        """Test negative units returns error."""
+        error = validate_numeric_inputs(-100.0, 50000.0, 0.75)
+        assert error is not None
+        assert "positive" in error.lower()
+
+    def test_negative_loan_returns_error(self) -> None:
+        """Test negative loan amount returns error."""
+        error = validate_numeric_inputs(1000.0, -50000.0, 0.75)
+        assert error is not None
+        assert "negative" in error.lower()
+
+    def test_zero_loan_is_valid(self) -> None:
+        """Test zero loan is valid (no loan scenario)."""
+        error = validate_numeric_inputs(1000.0, 0.0, 0.75)
+        assert error is None
+
+    def test_ltv_at_zero_is_invalid(self) -> None:
+        """Test LTV at 0 returns error."""
+        error = validate_numeric_inputs(1000.0, 50000.0, 0.0)
+        assert error is not None
+        assert "ltv" in error.lower()
+
+    def test_ltv_at_one_is_invalid(self) -> None:
+        """Test LTV at 1.0 returns error."""
+        error = validate_numeric_inputs(1000.0, 50000.0, 1.0)
+        assert error is not None
+        assert "ltv" in error.lower()
+
+    def test_ltv_above_one_is_invalid(self) -> None:
+        """Test LTV above 1.0 returns error."""
+        error = validate_numeric_inputs(1000.0, 50000.0, 1.5)
+        assert error is not None
+        assert "ltv" in error.lower()
+
+    def test_ltv_at_valid_bounds(self) -> None:
+        """Test LTV at valid bounds (0.01 and 0.99)."""
+        error_low = validate_numeric_inputs(1000.0, 50000.0, 0.01)
+        assert error_low is None
+        error_high = validate_numeric_inputs(1000.0, 50000.0, 0.99)
+        assert error_high is None