feat: add configurable target and stop-loss percentages#7
Conversation
|
Warning Review limit reached
More reviews will be available in 55 minutes and 53 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. 📝 WalkthroughWalkthroughThis PR implements configurable target and stop-loss risk percentages. Users select values from the Streamlit sidebar, which are passed through the analysis pipeline and used to calculate dynamic target and stop-loss price levels during trade signal generation. ChangesConfigurable Risk Percentages Pipeline
Sequence DiagramsequenceDiagram
participant User as User
participant UI as Sidebar UI
participant AnalyzeCache as _analyze_cached
participant AnalyzeSymbols as analyze_symbols
participant CalcSignal as calculate_orb_signal
participant ActionSignal as _action_signal
participant TradeSignal as TradeSignal
User->>UI: Select target_percent & stop_percent
UI->>AnalyzeCache: Pass percentages with symbols
AnalyzeCache->>AnalyzeSymbols: Forward target_percent, stop_percent
AnalyzeSymbols->>CalcSignal: Call with target_percent, stop_percent
CalcSignal->>ActionSignal: Forward percentages to BUY/SELL branch
ActionSignal->>ActionSignal: Calculate target/stop_loss from percentages
ActionSignal->>TradeSignal: Return configured target and stop_loss
TradeSignal-->>User: Display dynamic price levels
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app.py (1)
98-98: ⚡ Quick winAdd spaces after commas for PEP 8 compliance.
The function call is missing spaces after commas.
📝 Proposed formatting fix
- signals, charts, errors, trades, stats = _analyze_cached(tuple(symbols),target_percent,stop_percent) + signals, charts, errors, trades, stats = _analyze_cached(tuple(symbols), target_percent, stop_percent)🤖 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` at line 98, The call to _analyze_cached is missing spaces after commas (PEP8 formatting); update the call to include spaces after each comma when passing arguments (symbols, target_percent, stop_percent) so it reads with normal spacing and matches PEP 8 style around the tuple conversion and subsequent arguments in the _analyze_cached(...) invocation.
🤖 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 192-193: The cached helper _analyze_cached currently omits type
hints and has spacing issues; update its signature (the function named
_analyze_cached) to include proper spacing and types for target_percent and
stop_percent (e.g. target_percent: float, stop_percent: float) and add a return
type annotation consistent with analyze_symbols' return (or use -> Any) so the
`@st.cache_data` cache key is generated correctly; keep the function body calling
analyze_symbols(list(symbols), target_percent, stop_percent) unchanged.
---
Nitpick comments:
In `@app.py`:
- Line 98: The call to _analyze_cached is missing spaces after commas (PEP8
formatting); update the call to include spaces after each comma when passing
arguments (symbols, target_percent, stop_percent) so it reads with normal
spacing and matches PEP 8 style around the tuple conversion and subsequent
arguments in the _analyze_cached(...) invocation.
🪄 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: cf85bc47-1e2c-44f5-8547-6bc5fae6cbeb
⛔ Files ignored due to path filters (7)
__pycache__/data_fetcher.cpython-314.pycis excluded by!**/*.pyc__pycache__/strategy.cpython-314.pycis excluded by!**/*.pyc__pycache__/symbol_resolver.cpython-314.pycis excluded by!**/*.pyc__pycache__/trade_tracker.cpython-314.pycis excluded by!**/*.pyc__pycache__/trader.cpython-314.pycis excluded by!**/*.pyclogs/trades.csvis excluded by!**/*.csvlogs/tradex.logis excluded by!**/*.log
📒 Files selected for processing (4)
app.pydata/trades.jsonstrategy.pytrader.py
|
Hi @sreerevanth , I've implemented the configurable target and stop-loss percentage feature and tested it locally. The values now update dynamically in the signal cards and signal matrix while keeping the existing ORB signal generation logic unchanged. I'd appreciate any feedback or review when you have time. Thanks! |
feat: make target and stop-loss percentages configurable
Summary
This PR adds configurable Target % and Stop Loss % inputs to the Streamlit sidebar, allowing users to customize risk management settings without modifying source code.
Previously, target and stop-loss values were hardcoded in the ORB strategy:
With this change, users can adjust these values directly from the dashboard.
Changes Made
UI (Streamlit)
Added a new Risk Settings section in the sidebar:
Analysis Pipeline
Passed risk settings through:
Strategy Logic
Updated target and stop-loss calculations to use user-defined percentages.
Previous implementation:
New implementation:
Equivalent logic was applied for SELL signals.
Example
Current Defaults
Entry = 100
Target % = 1
Stop Loss % = 0.5
Result:
Custom Settings
Entry = 100
Target % = 3
Stop Loss % = 1
Result:
Benefits
Testing
Tested with multiple symbols and verified:
Closes #6
Summary by CodeRabbit
New Features
Chores