feat: paper trading, backtesting module, and risk management overlays#4
Merged
sreerevanth merged 4 commits intoJun 7, 2026
Conversation
- paper_trader.py: logs simulated trades to data/paper_trades.json, tracks hypothetical P&L, win rate, auto-closes on new signal - backtester.py: replays ORB strategy on historical intraday data, computes win rate, total return, max drawdown, equity curve - app.py: Live/Paper mode toggle in sidebar, SL/TP sliders, chart overlays for stop loss and take profit levels, backtester UI in a second tab with equity curve visualization - Closes sreerevanth#1
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds file-backed paper trading (log, stats, clear), a daily ORB backtester returning trade logs and metrics, SL/TP risk inputs and chart annotations, PAPER-mode simulated trade logging and performance UI, and a Backtester tab with equity-curve visualization. ChangesPaper Trading and Backtesting Platform
Sequence DiagramsequenceDiagram
participant User
participant Dashboard
participant PaperTrader
participant JSONStorage
participant Backtester
participant yfinance
User->>Dashboard: Select PAPER mode, set SL/TP %
Note over Dashboard: Live signal detected
Dashboard->>PaperTrader: log_paper_trade(signal, sl_pct, tp_pct)
PaperTrader->>JSONStorage: write data/paper_trades.json (append/close)
Dashboard->>PaperTrader: get_paper_stats()
PaperTrader->>JSONStorage: read data/paper_trades.json
PaperTrader->>Dashboard: return stats
Dashboard->>User: Show paper stats, trades table, SL/TP on chart
User->>Dashboard: Click Backtester tab, set ticker and date range
Dashboard->>Backtester: run_backtest(ticker, dates, sl_pct, tp_pct)
Backtester->>yfinance: download daily OHLC
yfinance->>Backtester: return historical candles
Backtester->>Backtester: replay ORB, simulate SL/TP exits, build trade_log
Backtester->>Dashboard: return BacktestResult (metrics + trade_log)
Dashboard->>User: Display backtest metrics, trade log, equity curve
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Analysis CompleteGenerated ECC bundle from 1 commits | Confidence: 50% View Pull Request #5Repository Profile
Changed Files (3)
Top hotspots
Top directories
Analysis Depth Readiness (commit-history, 7%)ECC Tools uses this to decide whether recommendations should stay at commit-history/setup guidance or expand into CI, security, harness, reference-set, AI-routing, and team backlog work.
Reference Set Readiness (0/7, 0%)
Likely Future Issues (1)
Suggested Follow-up Work (1)
Copy-ready bodies test: add regression coverage for app.py + backtester.py ## Summary
- Add regression coverage for the recently touched code paths before more changes stack on top.
## Why
- Backfill regression coverage before another change set lands on the touched code paths.
## Touched paths
- `app.py`
- `backtester.py`
## Validation
- Add or extend focused tests that exercise the touched paths.
- Run the affected test suite and verify the new coverage closes the gap.Generated Instincts (14)
After merging, import with: Files
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app.py (1)
105-106:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake sidebar SL/TP drive the same target/stop values used in generated signals (not just overlays/paper trades).
app.pysliders (sl_pct,tp_pct) are used for chart overlays (_price_figure(..., sl_pct=..., tp_pct=...)) and paper trades (log_paper_trade(..., sl_pct=..., tp_pct=...)).- The displayed signals (“SIGNALS” card + signal matrix) use
TradeSignal.target/TradeSignal.stop_loss, which are hardcoded instrategy.calculate_orb_signal(_action_signal: BUY target=+1%, stop=-0.5%; SELL target=-1%, stop=+0.5%)._analyze_cachedis keyed only bysymbolsandtrader.analyze_symbols()takes onlysymbols, so changing the sliders won’t regenerate signals—UI can diverge from the strategy risk implied by the overlays/paper-trade rows.Plumb
sl_pct/tp_pctthroughanalyze_symbols -> calculate_orb_signal/_action_signaland include them in_analyze_cached’s cache key.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app.py` around lines 105 - 106, The signals are generated with hardcoded targets/stops while sliders (sl_pct, tp_pct) only affect overlays/paper trades; to fix, add sl_pct and tp_pct parameters to _analyze_cached, trader.analyze_symbols, strategy.calculate_orb_signal and its helper _action_signal and thread them through so calculate_orb_signal uses the slider values to set TradeSignal.target and TradeSignal.stop_loss; update the cache key for _analyze_cached to include sl_pct and tp_pct and change the call site (currently _analyze_cached(tuple(symbols))) to pass the current slider values so UI signals, overlays and paper trades stay in sync.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app.py`:
- Around line 113-117: The paper-trade logger currently calls log_paper_trade
for every cycle whenever trading_mode == "PAPER" and signal.signal is "BUY" or
"SELL", causing duplicate logs; fix by constructing a durable signal identity
(e.g. f"{signal.symbol}:{signal.signal}:{signal.timestamp}" or another unique
combination such as symbol+side+rounded-timestamp) and checking a persisted
store (in-memory cache with durable backing, DB table, or a file) for that id
before calling log_paper_trade; if the id is new, call log_paper_trade and
persist the id (update the store) so subsequent runs skip the same signal.
- Around line 220-221: The IndentationError at the st.markdown('<div
class="section-title">CHARTS</div>', unsafe_allow_html=True) line means the
statement's indentation doesn't match the surrounding block; fix it by aligning
this st.markdown call to the correct indentation level (remove stray leading
spaces/tabs or convert mixed tabs to spaces) so it matches the surrounding scope
(top-level or the enclosing function/block), and ensure there are no unclosed
blocks or multi-line strings before it that would change expected indentation.
In `@backtester.py`:
- Around line 194-208: The percent metrics are incorrectly normalized to the
first trade price (first_entry) — change the logic to normalize against an
explicit initial capital (e.g., initial_capital) derived from or passed into the
backtest and computed equity_curve instead: ensure equity_curve represents
per-period account equity based on initial_capital and trade P&L, compute
total_return_pct = (ending_equity - initial_capital) / initial_capital * 100 and
max_dd_pct = max_drawdown / initial_capital * 100 (use the existing max_dd calc
over equity_curve but divide by initial_capital), and keep total_return,
avg_trade_pnl, and trade_log computations intact but based on notional or
position sizing consistent with initial_capital.
In `@paper_trader.py`:
- Around line 46-66: The code currently loads, mutates and rewrites
PAPER_TRADES_FILE without synchronization and treats corrupted JSON as an empty
list; wrap all access to PAPER_TRADES_FILE (both reads in _load_trades() and
writes where existing.append(trade) and json.dump(...) are called) with a
process-wide file lock (e.g., filelock.FileLock or a module-level threading.Lock
used with a context manager) to serialize concurrent runs, and perform atomic
writes by writing JSON to a temporary file in the same directory and then
os.replace(temp_path, PAPER_TRADES_FILE) to atomically swap; also validate
parsed JSON in _load_trades() (raise/log and abort write or restore from a
backup rather than silently returning [] on truncation/corruption) so you don’t
silently drop trades.
---
Outside diff comments:
In `@app.py`:
- Around line 105-106: The signals are generated with hardcoded targets/stops
while sliders (sl_pct, tp_pct) only affect overlays/paper trades; to fix, add
sl_pct and tp_pct parameters to _analyze_cached, trader.analyze_symbols,
strategy.calculate_orb_signal and its helper _action_signal and thread them
through so calculate_orb_signal uses the slider values to set TradeSignal.target
and TradeSignal.stop_loss; update the cache key for _analyze_cached to include
sl_pct and tp_pct and change the call site (currently
_analyze_cached(tuple(symbols))) to pass the current slider values so UI
signals, overlays and paper trades stay in sync.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 77b26c55-5c93-4ccf-a751-3c98a6717359
📒 Files selected for processing (3)
app.pybacktester.pypaper_trader.py
Comment on lines
+194
to
+208
| total_return = round(sum(t["pnl"] for t in trade_log), 2) | ||
| first_entry = trade_log[0]["entry"] | ||
| total_return_pct = round(total_return / first_entry * 100, 2) if first_entry else 0.0 | ||
| avg_trade_pnl = round(total_return / len(trade_log), 2) | ||
|
|
||
| # Max drawdown from equity curve | ||
| peak = equity_curve[0] | ||
| max_dd = 0.0 | ||
| for val in equity_curve: | ||
| if val > peak: | ||
| peak = val | ||
| dd = (peak - val) | ||
| if dd > max_dd: | ||
| max_dd = dd | ||
| max_dd_pct = round(max_dd / first_entry * 100, 2) if first_entry else 0.0 |
There was a problem hiding this comment.
Normalize return and drawdown against a real capital base.
Line 196 and Line 208 divide cumulative multi-trade P&L by first_entry, so the reported percentages are anchored to the first trade's price instead of the backtest equity. That makes total_return_pct and max_drawdown misleading across multiple days/tickers. Track an initial capital or per-trade notional and derive both metrics from that equity curve instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backtester.py` around lines 194 - 208, The percent metrics are incorrectly
normalized to the first trade price (first_entry) — change the logic to
normalize against an explicit initial capital (e.g., initial_capital) derived
from or passed into the backtest and computed equity_curve instead: ensure
equity_curve represents per-period account equity based on initial_capital and
trade P&L, compute total_return_pct = (ending_equity - initial_capital) /
initial_capital * 100 and max_dd_pct = max_drawdown / initial_capital * 100 (use
the existing max_dd calc over equity_curve but divide by initial_capital), and
keep total_return, avg_trade_pnl, and trade_log computations intact but based on
notional or position sizing consistent with initial_capital.
Comment on lines
+46
to
+66
| existing = _load_trades() | ||
|
|
||
| # Auto-close any existing OPEN trade for same symbol | ||
| for t in existing: | ||
| if t["symbol"] == signal.symbol and t["status"] == "OPEN": | ||
| current_price = signal.last_price or entry | ||
| t["status"] = "CLOSED" | ||
| t["exit_price"] = current_price | ||
| t["exit_time"] = datetime.now().isoformat(timespec="seconds") | ||
| t["exit_reason"] = "New signal" | ||
| if t["side"] == "BUY": | ||
| t["pnl"] = round(current_price - t["entry_price"], 2) | ||
| t["pnl_pct"] = round((current_price - t["entry_price"]) / t["entry_price"] * 100, 2) | ||
| else: | ||
| t["pnl"] = round(t["entry_price"] - current_price, 2) | ||
| t["pnl_pct"] = round((t["entry_price"] - current_price) / t["entry_price"] * 100, 2) | ||
|
|
||
| existing.append(trade) | ||
|
|
||
| with PAPER_TRADES_FILE.open("w", encoding="utf-8") as f: | ||
| json.dump(existing, f, indent=2) |
There was a problem hiding this comment.
Guard paper-trade persistence with locking and atomic writes.
Line 46 does a full load/mutate/rewrite with no synchronization, and Line 114 treats any truncated/corrupted JSON as []. Two Streamlit reruns or users hitting this path together can drop trades or wipe the history entirely. Use a lock around all read/write paths and write to a temp file before replacing the real JSON file.
Also applies to: 102-115
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@paper_trader.py` around lines 46 - 66, The code currently loads, mutates and
rewrites PAPER_TRADES_FILE without synchronization and treats corrupted JSON as
an empty list; wrap all access to PAPER_TRADES_FILE (both reads in
_load_trades() and writes where existing.append(trade) and json.dump(...) are
called) with a process-wide file lock (e.g., filelock.FileLock or a module-level
threading.Lock used with a context manager) to serialize concurrent runs, and
perform atomic writes by writing JSON to a temporary file in the same directory
and then os.replace(temp_path, PAPER_TRADES_FILE) to atomically swap; also
validate parsed JSON in _load_trades() (raise/log and abort write or restore
from a backup rather than silently returning [] on truncation/corruption) so you
don’t silently drop trades.
sreerevanth
requested changes
Jun 5, 2026
sreerevanth
left a comment
sreerevanth
left a comment
Owner
There was a problem hiding this comment.
Thanks for the contribution! The feature set looks great and covers the requested enhancement well. Before merging, please address the paper-trading logging issue. Currently every Streamlit rerun logs a new trade and closes the previous OPEN trade for the same symbol, which inflates trade count and distorts P&L statistics. Also please verify the CHARTS section indentation since CodeRabbit flagged a possible parse error. Once those are fixed I'll be happy to review again.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
paper_trader.py (1)
11-15:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing error handling for corrupted JSON file.
If
paper_trades.jsonexists but contains invalid JSON (e.g., due to a previous partial write or manual edit),json.load()will raiseJSONDecodeErrorand crash the application. Add exception handling to either recover gracefully or surface the error explicitly.🛡️ Proposed fix
def _load_trades() -> list[dict]: if not os.path.exists(TRADES_FILE): return [] - with open(TRADES_FILE, "r") as f: - return json.load(f) + try: + with open(TRADES_FILE, "r") as f: + return json.load(f) + except (json.JSONDecodeError, ValueError): + # Log or notify user of corrupted file + return []🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@paper_trader.py` around lines 11 - 15, The _load_trades function currently calls json.load(TRADES_FILE) without handling corrupt JSON; wrap the file read/json.load in a try/except that catches json.JSONDecodeError (and optionally OSError) and handle it: either log the error with context (including TRADES_FILE) and return an empty list to recover, or re-raise a clearer exception (e.g., ValueError with filename and original error) so callers can surface it; update the code around _load_trades to implement this error handling and reference TRADES_FILE and _load_trades when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backtester.py`:
- Around line 81-94: The current intraday exit logic (variables side, day_low,
day_high, sl_price, tp_price, day_close, exit_price, exit_reason) always treats
STOP_LOSS as occurring before TAKE_PROFIT when both levels are within the day's
range, creating pessimistic bias; change the tie-breaking behavior when both
sl_price and tp_price are reachable on the same candle by implementing one clear
policy: either (A) randomize selection between SL and TP, (B) pick the level
closer to the open price (use open_price to compute distance), or (C) explicitly
document and expose the current assumption to users; update the branch in the
BUY and SELL blocks to detect the "both-hit" condition (day_low <= sl_price AND
day_high >= tp_price for BUY, the analogous condition for SELL) and apply your
chosen tie-breaker consistently, setting exit_price and exit_reason accordingly
and adding a comment or config flag to record the policy.
---
Duplicate comments:
In `@paper_trader.py`:
- Around line 11-15: The _load_trades function currently calls
json.load(TRADES_FILE) without handling corrupt JSON; wrap the file
read/json.load in a try/except that catches json.JSONDecodeError (and optionally
OSError) and handle it: either log the error with context (including
TRADES_FILE) and return an empty list to recover, or re-raise a clearer
exception (e.g., ValueError with filename and original error) so callers can
surface it; update the code around _load_trades to implement this error handling
and reference TRADES_FILE and _load_trades when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f97e9502-e1ba-47f3-bcdc-03f30cc3491b
📒 Files selected for processing (2)
backtester.pypaper_trader.py
Comment on lines
+81
to
+94
| if side == "BUY": | ||
| if day_low <= sl_price: | ||
| exit_price, exit_reason = sl_price, "STOP_LOSS" | ||
| elif day_high >= tp_price: | ||
| exit_price, exit_reason = tp_price, "TAKE_PROFIT" | ||
| else: | ||
| exit_price, exit_reason = day_close, "EOD" | ||
| else: | ||
| if day_high >= sl_price: | ||
| exit_price, exit_reason = sl_price, "STOP_LOSS" | ||
| elif day_low <= tp_price: | ||
| exit_price, exit_reason = tp_price, "TAKE_PROFIT" | ||
| else: | ||
| exit_price, exit_reason = day_close, "EOD" |
There was a problem hiding this comment.
Intraday exit order creates pessimistic bias in backtest results.
On volatile days where both SL and TP levels could have been breached within the same candle, the code always assumes stop-loss was hit first (lines 82-83, 89-90). This introduces a systematic pessimistic bias that can significantly understate win rate and total returns.
With daily OHLC data, it's impossible to determine which level was hit first. Consider either:
- Using a random choice when both could have triggered
- Using the level closer to the open price
- Documenting this assumption clearly for users
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backtester.py` around lines 81 - 94, The current intraday exit logic
(variables side, day_low, day_high, sl_price, tp_price, day_close, exit_price,
exit_reason) always treats STOP_LOSS as occurring before TAKE_PROFIT when both
levels are within the day's range, creating pessimistic bias; change the
tie-breaking behavior when both sl_price and tp_price are reachable on the same
candle by implementing one clear policy: either (A) randomize selection between
SL and TP, (B) pick the level closer to the open price (use open_price to
compute distance), or (C) explicitly document and expose the current assumption
to users; update the branch in the BUY and SELL blocks to detect the "both-hit"
condition (day_low <= sl_price AND day_high >= tp_price for BUY, the analogous
condition for SELL) and apply your chosen tie-breaker consistently, setting
exit_price and exit_reason accordingly and adding a comment or config flag to
record the policy.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app.py`:
- Around line 115-130: When clear_paper_trades() wipes the JSON paper-trade
store it doesn't reset the in-memory dedupe cache paper_logged_signals, so
previously-seen BUY/SELL keys remain marked and new trades aren't recorded;
update clear_paper_trades() to also remove or reset
st.session_state["paper_logged_signals"] (e.g., delete the key or set it to an
empty set) so the session dedupe cache is cleared whenever the paper history is
cleared, and ensure any code paths that clear paper history call that same
reset.
- Around line 114-130: There is a duplicated/pasted block causing an
IndentationError after the if trading_mode == "PAPER": check; remove the
duplicated second block and properly indent the remaining block so the for-loop
and subsequent st.session_state assignment are inside the if. Specifically, keep
a single block that checks trading_mode, reads/updates
st.session_state["paper_logged_signals"], iterates signals, computes signal_key,
calls log_paper_trade(signal, sl_pct=sl_pct, tp_pct=tp_pct) for new BUY/SELL
keys, adds keys to logged, and finally writes logged back to st.session_state;
ensure function/variable names referenced are trading_mode, signals,
log_paper_trade, sl_pct, tp_pct, and paper_logged_signals.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+114
to
+130
| if trading_mode == "PAPER": | ||
| logged = st.session_state.get("paper_logged_signals", set()) | ||
| for signal in signals: | ||
| if signal.signal in {"BUY", "SELL"}: | ||
| signal_key = f"{signal.symbol}_{signal.signal}_{signal.entry_price}" | ||
| if signal_key not in logged: | ||
| log_paper_trade(signal, sl_pct=sl_pct, tp_pct=tp_pct) | ||
| logged.add(signal_key) | ||
| st.session_state["paper_logged_signals"] = loggedif trading_mode == "PAPER": | ||
| logged = st.session_state.get("paper_logged_signals", set()) | ||
| for signal in signals: | ||
| if signal.signal in {"BUY", "SELL"}: | ||
| signal_key = f"{signal.symbol}_{signal.signal}_{signal.entry_price}" | ||
| if signal_key not in logged: | ||
| log_paper_trade(signal, sl_pct=sl_pct, tp_pct=tp_pct) | ||
| logged.add(signal_key) | ||
| st.session_state["paper_logged_signals"] = logged |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import ast
from pathlib import Path
source = Path("app.py").read_text(encoding="utf-8")
ast.parse(source)
print("app.py parses successfully")
PYRepository: sreerevanth/TRADEX
Length of output: 468
Fix release-blocking syntax error in app.py PAPER trade logging (remove duplicated block)
app.py fails to parse due to missing indentation after if trading_mode == "PAPER": (IndentationError at/around line 115) and a pasted/duplicated block that splices logged into the next if.
🐛 Proposed fix
# ── Paper trade logging ───────────────────────────────
if trading_mode == "PAPER":
- logged = st.session_state.get("paper_logged_signals", set())
- for signal in signals:
- if signal.signal in {"BUY", "SELL"}:
- signal_key = f"{signal.symbol}_{signal.signal}_{signal.entry_price}"
- if signal_key not in logged:
- log_paper_trade(signal, sl_pct=sl_pct, tp_pct=tp_pct)
- logged.add(signal_key)
- st.session_state["paper_logged_signals"] = loggedif trading_mode == "PAPER":
- logged = st.session_state.get("paper_logged_signals", set())
- for signal in signals:
- if signal.signal in {"BUY", "SELL"}:
- signal_key = f"{signal.symbol}_{signal.signal}_{signal.entry_price}"
- if signal_key not in logged:
- log_paper_trade(signal, sl_pct=sl_pct, tp_pct=tp_pct)
- logged.add(signal_key)
- st.session_state["paper_logged_signals"] = logged
+ logged = st.session_state.get("paper_logged_signals", set())
+ for signal in signals:
+ if signal.signal in {"BUY", "SELL"}:
+ signal_key = f"{signal.symbol}_{signal.signal}_{signal.entry_price}"
+ if signal_key not in logged:
+ log_paper_trade(signal, sl_pct=sl_pct, tp_pct=tp_pct)
+ logged.add(signal_key)
+ st.session_state["paper_logged_signals"] = logged📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if trading_mode == "PAPER": | |
| logged = st.session_state.get("paper_logged_signals", set()) | |
| for signal in signals: | |
| if signal.signal in {"BUY", "SELL"}: | |
| signal_key = f"{signal.symbol}_{signal.signal}_{signal.entry_price}" | |
| if signal_key not in logged: | |
| log_paper_trade(signal, sl_pct=sl_pct, tp_pct=tp_pct) | |
| logged.add(signal_key) | |
| st.session_state["paper_logged_signals"] = loggedif trading_mode == "PAPER": | |
| logged = st.session_state.get("paper_logged_signals", set()) | |
| for signal in signals: | |
| if signal.signal in {"BUY", "SELL"}: | |
| signal_key = f"{signal.symbol}_{signal.signal}_{signal.entry_price}" | |
| if signal_key not in logged: | |
| log_paper_trade(signal, sl_pct=sl_pct, tp_pct=tp_pct) | |
| logged.add(signal_key) | |
| st.session_state["paper_logged_signals"] = logged | |
| # ── Paper trade logging ─────────────────────────────── | |
| if trading_mode == "PAPER": | |
| logged = st.session_state.get("paper_logged_signals", set()) | |
| for signal in signals: | |
| if signal.signal in {"BUY", "SELL"}: | |
| signal_key = f"{signal.symbol}_{signal.signal}_{signal.entry_price}" | |
| if signal_key not in logged: | |
| log_paper_trade(signal, sl_pct=sl_pct, tp_pct=tp_pct) | |
| logged.add(signal_key) | |
| st.session_state["paper_logged_signals"] = logged |
🧰 Tools
🪛 Ruff (0.15.15)
[warning] 115-115: Expected an indented block after if statement
(invalid-syntax)
[warning] 122-122: Simple statements must be separated by newlines or semicolons
(invalid-syntax)
[warning] 122-123: Expected an expression
(invalid-syntax)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app.py` around lines 114 - 130, There is a duplicated/pasted block causing an
IndentationError after the if trading_mode == "PAPER": check; remove the
duplicated second block and properly indent the remaining block so the for-loop
and subsequent st.session_state assignment are inside the if. Specifically, keep
a single block that checks trading_mode, reads/updates
st.session_state["paper_logged_signals"], iterates signals, computes signal_key,
calls log_paper_trade(signal, sl_pct=sl_pct, tp_pct=tp_pct) for new BUY/SELL
keys, adds keys to logged, and finally writes logged back to st.session_state;
ensure function/variable names referenced are trading_mode, signals,
log_paper_trade, sl_pct, tp_pct, and paper_logged_signals.
Source: Linters/SAST tools
Comment on lines
+115
to
+130
| logged = st.session_state.get("paper_logged_signals", set()) | ||
| for signal in signals: | ||
| if signal.signal in {"BUY", "SELL"}: | ||
| signal_key = f"{signal.symbol}_{signal.signal}_{signal.entry_price}" | ||
| if signal_key not in logged: | ||
| log_paper_trade(signal, sl_pct=sl_pct, tp_pct=tp_pct) | ||
| logged.add(signal_key) | ||
| st.session_state["paper_logged_signals"] = loggedif trading_mode == "PAPER": | ||
| logged = st.session_state.get("paper_logged_signals", set()) | ||
| for signal in signals: | ||
| if signal.signal in {"BUY", "SELL"}: | ||
| signal_key = f"{signal.symbol}_{signal.signal}_{signal.entry_price}" | ||
| if signal_key not in logged: | ||
| log_paper_trade(signal, sl_pct=sl_pct, tp_pct=tp_pct) | ||
| logged.add(signal_key) | ||
| st.session_state["paper_logged_signals"] = logged |
There was a problem hiding this comment.
Clear the session dedupe cache with the paper-trade store.
This cache survives clear_paper_trades(), so after the JSON file is wiped the same BUY/SELL key is still treated as “already logged” and the next scan can show an empty paper-trades table without recording a fresh trade. Reset paper_logged_signals when paper history is cleared.
🧰 Tools
🪛 Ruff (0.15.15)
[warning] 115-115: Expected an indented block after if statement
(invalid-syntax)
[warning] 122-122: Simple statements must be separated by newlines or semicolons
(invalid-syntax)
[warning] 122-123: Expected an expression
(invalid-syntax)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app.py` around lines 115 - 130, When clear_paper_trades() wipes the JSON
paper-trade store it doesn't reset the in-memory dedupe cache
paper_logged_signals, so previously-seen BUY/SELL keys remain marked and new
trades aren't recorded; update clear_paper_trades() to also remove or reset
st.session_state["paper_logged_signals"] (e.g., delete the key or set it to an
empty set) so the session dedupe cache is cleared whenever the paper history is
cleared, and ensure any code paths that clear paper history call that same
reset.
Contributor
Author
|
@sreerevanth i have updated the codes please verify it |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds three major features to Tradex: Paper Trading mode, Historical Backtesting,
and Risk Management chart overlays — transforming it from a signal generator into
a complete trading environment.
Changes
New Files
paper_trader.py— Paper Trading Engine: logs simulated BUY/SELL entries todata/paper_trades.json, tracks hypothetical P&L without real capitalbacktester.py— Backtesting Module: fetches historical intraday data viayfinance, replays ORB strategy day-by-day, generates a performance report
Modified Files
app.pytrader.pystop_loss_pctandtake_profit_pctparams passed through signal logicFeatures in Detail
Paper Trading
LIVEandPAPERmode in the sidebardata/paper_trades.jsoninstead ofexecuting real trades
Backtesting
Risk Management Overlays
Testing Done
Closes #1
Summary by CodeRabbit