Skip to content

Fix backtesting crash for non-SMA strategies, division by zero, and alert logic#1

Open
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1774737178-fix-backtest-crash
Open

Fix backtesting crash for non-SMA strategies, division by zero, and alert logic#1
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1774737178-fix-backtest-crash

Conversation

@devin-ai-integration

Copy link
Copy Markdown

Summary

Fixes three bugs in app.py:

  1. Backtesting crash (critical): Selecting "RSI Oversold/Overbought", "Bollinger Band Breakout", or "Custom" in the backtesting module caused a KeyError on df['Position'] because only the SMA Crossover strategy was implemented. Now all four strategy options work:

    • RSI: Buys when RSI drops below oversold, sells when RSI rises above overbought. Uses simple rolling-window RSI.
    • Bollinger Band: Buys on upper band breakout, sells on lower band breakout.
    • Custom: Falls back to buy-and-hold with a warning.
  2. ZeroDivisionError in MAE/MFE analysis: avg_mfe / avg_mae crashed when avg_mae was 0. Added a guard.

  3. Alert threshold only fired on gains: daily_pnl > threshold never alerted on large losses. Changed to abs(daily_pnl) > threshold.

Review & Testing Checklist for Human

  • Verify RSI calculation correctness: The RSI implementation uses a simple rolling mean (SMA) for average gain/loss rather than Wilder's exponential smoothing. This is a valid approach but will produce slightly different values than most charting platforms. Decide if this is acceptable for your use case.
  • Test all 4 backtesting strategies end-to-end in the Streamlit UI: Select each strategy type, run a backtest with real ticker data, and confirm the equity curve and metrics render without errors.
  • Verify the alert behavior change is desired: The alert threshold now fires on large losses too (via abs()). Previously it only alerted when P&L was positive and exceeded the threshold. Confirm this is the intended behavior.
  • Check signal persistence logic for RSI/BB: Both strategies use ffill() to hold a position until the opposite signal fires. Verify this matches your intended trading logic (vs. exiting to flat when the signal is neutral).

Notes

  • No automated tests exist in this repo, so all verification must be manual via the Streamlit UI.
  • The avg_loss.replace(0, np.nan) in RSI guards against division by zero when computing relative strength.

Link to Devin session: https://app.devin.ai/sessions/a97d0c2bf8584905b198c44220a50eac
Requested by: @Philip-2007

…/MFE, and alert threshold logic

- Implement RSI Oversold/Overbought backtesting strategy
- Implement Bollinger Band Breakout backtesting strategy
- Add fallback for Custom strategy (buy-and-hold default)
- Fix ZeroDivisionError when avg_mae is 0 in MAE/MFE analysis
- Fix alert threshold to check absolute P&L value (alerts on losses too)

Co-Authored-By: eleraobariphilipaaron@gmail.com <philipaaron62@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant